Generalize DI usage #2366
Merged
Generalize DI usage #2366
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ppkarwasz
commented
Mar 11, 2024
9 tasks
ppkarwasz
force-pushed
the
fix/generalize-di-usage
branch
from
e0ba274 to
0c3b74b
Compare
This commit ensures that: * an `InstanceFactory` is created in the Log4j `Provider`, * the same factory is injected as parameters to all the components of the chain `Log4jContextFactory`, `ContextSelector`, `LoggerContext` (child), `Configuration` (grand-child). * `DI.createInstanceFactory` is called only when strictly necessary.
ppkarwasz
force-pushed
the
fix/generalize-di-usage
branch
from
0c3b74b to
68e22a7
Compare
Contributor
Author
|
@jvz, I am closing this as "partially reviewed", but I am waiting patiently to replace the 3 fully-fledged and configured |
Member
|
Sounds good to me! |
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit ensures that:
InstanceFactoryis created in the Log4jProvider,Log4jContextFactory,ContextSelector,LoggerContext(child),Configuration(grand-child).DI.createInstanceFactoryis called only when strictly necessary.Review kit
The most important changes are in
log4j-core,log4j-async-logger:ConfigurationFactoryis split into an interface (ConfigurationFactory) and an abstract class (AbstractConfigurationFactory). TheConfigurationFactorytype becomes an interface, which might be a non-acceptable change,Provider->LoggerContextFactory->ContextSelector->LoggerContext->Configurationis created using anInstanceFactory,DefaultConfigurationandNullConfiguration(the tests are filled with these), their usage is limited to tests,LoggerConfigis refactored to require aConfigurationparameter to retrieve the services it need (e.g.LogEventFactory),log4j-async-loggerartifact has been refactored to create the necessary disruptors using the DI. Since theAsyncLoggerConfigDisruptorrequires some configuration data only available in aConfiguration, this forced a change in the way bundles of beans are registered with the DI: each child instance factory applies all availableConfigurableInstanceFactoryPostProcessorservices to itself. Previously on the first instance factory did it.The changes in
log4j-apican probably be ignored: the end goal is to remove this artifact.Part of #2290.