Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ [Service Configuration] Service Configurations refactoring #4079

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

dseurotech
Copy link
Contributor

@dseurotech dseurotech commented Jul 9, 2024

This PR continues the work initiated during the DI introduction, separating the logic of service configuration management into the ServiceConfigurationManager implementations.
This also changes how ServiceConfigurationManagers are wired - managers are no longer annotated with @nAmed but are wired as contributors to a Map, e.g.:

    @ProvidesIntoMap
    @ClassMapKey(UserService.class)
    @Singleton
    ServiceConfigurationManager userServiceConfigurationManager(

Unfortunately Guice does not support overriding for Map binder entries, therefore each service configuration manager has been isolated in a separate Module wiring file. If you need to change the behaviour of any ServiceConfigurationManager, configure the locator.xml to ignore that full class name (including the package name) and provide a new implementation.

Having all service configuration managers in a map was functional to solve a vulnerability issue present in the Rest Apis, which were instantiating classes based on the name provided by the user. Now the value provided by the user is matched (at the string level) with the list of available keys.

Furthermore, within this PR the logic to retrieve the TMetadata for services has been isolated in its own provider, allowing for alternative implementations.

@dseurotech dseurotech force-pushed the ref-simplify_service_configuration branch from 0bcd81b to c3e8953 Compare July 9, 2024 13:23
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

Attention: Patch coverage is 62.19081% with 214 lines in your changes missing coverage. Please review.

Project coverage is 16.75%. Comparing base (ca5615b) to head (ccdca43).
Report is 4 commits behind head on develop.

Files Patch % Lines
...configuration/ServiceConfigurationManagerImpl.java 5.63% 67 Missing ⚠️
...configuration/ServiceConfigurationsFacadeImpl.java 0.00% 35 Missing ⚠️
...cation/credential/shiro/CredentialServiceImpl.java 4.76% 20 Missing ⚠️
...ion/ServiceConfigurationManagerCachingWrapper.java 0.00% 11 Missing ⚠️
...cation/shiro/SystemPasswordLengthProviderImpl.java 28.57% 10 Missing ⚠️
...ntial/shiro/AccountPasswordLengthProviderImpl.java 30.76% 9 Missing ⚠️
...ns/configuration/KapuaConfigurableServiceBase.java 0.00% 7 Missing ⚠️
...urceBasedServiceConfigurationMetadataProvider.java 30.00% 7 Missing ⚠️
.../resources/v1/resources/ServiceConfigurations.java 0.00% 6 Missing ⚠️
...pse/kapua/service/config/ServiceConfiguration.java 0.00% 4 Missing ⚠️
... and 18 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #4079   +/-   ##
==========================================
  Coverage      16.75%   16.75%           
  Complexity        22       22           
==========================================
  Files           2016     2023    +7     
  Lines          52357    52373   +16     
  Branches        4417     4417           
==========================================
+ Hits            8773     8776    +3     
- Misses         43184    43198   +14     
+ Partials         400      399    -1     
Files Coverage Δ
...ons/configuration/ServiceConfigurationManager.java 0.00% <ø> (ø)
...se/kapua/commons/configuration/ValueTokenizer.java 89.60% <100.00%> (ø)
...onfiguration/metatype/PasswordPropertyAdapter.java 13.63% <ø> (ø)
.../kapua/app/api/web/RestApiJAXBContextProvider.java 0.00% <ø> (ø)
.../kapua/service/account/internal/AccountModule.java 93.75% <100.00%> (-0.70%) ⬇️
...rnal/AccountServiceConfigurationManagerModule.java 100.00% <100.00%> (ø)
...eclipse/kapua/model/config/metatype/EmptyTocd.java 100.00% <ø> (ø)
.../eclipse/kapua/model/config/metatype/KapuaTad.java 100.00% <100.00%> (ø)
...e/kapua/model/config/metatype/KapuaTdesignate.java 100.00% <100.00%> (ø)
...se/kapua/model/config/metatype/KapuaTmetadata.java 100.00% <100.00%> (ø)
... and 52 more

... and 1 file with indirect coverage changes

@dseurotech dseurotech force-pushed the ref-simplify_service_configuration branch 2 times, most recently from 90927fa to 4390cfb Compare July 12, 2024 08:56
@dseurotech dseurotech force-pushed the ref-simplify_service_configuration branch 2 times, most recently from 0ee208a to e10cf96 Compare July 23, 2024 15:15
@dseurotech dseurotech force-pushed the ref-simplify_service_configuration branch 2 times, most recently from 5813eaf to 6ffd032 Compare August 1, 2024 07:26
@dseurotech dseurotech changed the title 🔥 removing split between configuration metadata interfaces and dto ♻️ Service Configurations refactoring Aug 1, 2024
@dseurotech dseurotech marked this pull request as ready for review August 1, 2024 08:29
@dseurotech dseurotech force-pushed the ref-simplify_service_configuration branch from 6ffd032 to 68817c4 Compare August 1, 2024 10:04
@Coduz Coduz changed the title ♻️ Service Configurations refactoring ♻️ [Service Configuration] Service Configurations refactoring Aug 1, 2024
@dseurotech dseurotech force-pushed the ref-simplify_service_configuration branch 2 times, most recently from 1f3d03e to ccdca43 Compare August 1, 2024 15:35
…trieve all of them together later on

Signed-off-by: dseurotech <[email protected]>
…o allow them to be ignored (overriding not supported)

Signed-off-by: dseurotech <[email protected]>
…n they operate in (but don't use it directly)

Signed-off-by: dseurotech <[email protected]>
…ture via Optionals instead of null values

Signed-off-by: dseurotech <[email protected]>
@dseurotech dseurotech force-pushed the ref-simplify_service_configuration branch from ccdca43 to a103d45 Compare December 4, 2024 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant