I’d like to add more ConditionalOn annotations for the @Factory methods, or at 
least something like “if no binding exists for this key already, here it is” 
which is analogous to @ConditionalOnMissingBean in Spring. That would make the 
DefaultCallback code easier to write without using the Injector API directly. 
That will also make a good example for users to copy from when making their own 
InjectorCallback customizations.

I do need to look more closely at the API module to see how injection can work 
without pulling DI APIs up. Might need to define some ServiceLoader stuff there 
like an ordering annotation.

—
Matt Sicker

> On May 19, 2022, at 10:47, Ralph Goers <ralph.go...@dslextreme.com> wrote:
> 
> 
> 
>> On May 19, 2022, at 12:15 AM, Volkan Yazıcı <vol...@yazi.ci> wrote:
>> 
>> In the last couple of weeks, I have been interrogating Matt on why and how
>> of the 3.x plugin infra. This inevitably led to some code archaeology. I
>> will try to share my take out of this exercise.
>> 
>> 1.x required users to type the fully-qualified class names of components
>> (appenders, layouts, etc.) in either configuration files or properties:
>> 
>> log4j.appender.A1=org.apache.log4j.ConsoleAppender
>> log4j.appender.A1.layout=org.apache.log4j.PatternLayout
>> 
>> Plugins were first introduced in 2.x to address this concern:
> 
> It wasn’t a concern. It was an annoyance. 
> 
>> providing a
>> mechanism to alias components. Later on it also got used to glue the rest
>> of the configuration components together. Consequently maintainers started
>> exercising this throughout the entire code base, whenever something needed
>> to be indirectly injected somewhere.
> 
> Only where we thought it made sense. Some things use system properties, 
> some things use ServiceLoader.
> 
> 
>> 
>> That was a great epiphany and I am in love with the plugins! It feels like
>> Log4j is composed of simple beans beautifully practicing
>> separation-of-concerns while running on a marvellous Spring-like
>> fully-fledged dependency injection (DI) framework... almost... sort of...
>> The reality is sadly a little bit different than that. In particular, I see
>> two major issues:
>> 
>>  1. Missing `@ConditionalOn*` support
> 
> I am certain I added the ability to do this in 3.0. I added constraint 
> checking 
> at the class level where previously it was only available on parameters. That 
> said, we currently only have RequiredClass and RequiredProperty validators.
> 
>>  2. Static access to DI
>> 
>> I guess Matt is already working on issue #1. He is trying to make sure
>> `@Required` et al. annotations are executed the same way at every injection
>> site.
>> 
>> What do I mean with the static access to DI? In a `@Bean`-annotated method
>> of a Spring application, do you create your own `ApplicationContext`
>> instance and access other beans from that? Do you statically access
>> `ApplicationContext.getBean("foo")`? Certainly not! Though these two
>> examples are what we exactly do in Log4j. We create single-use
>> `PluginManager` instances and use it to collect plugins. We call
>> `DI.createInjector().getInstance("foo")`. What we should be rather doing is
>> to inject the `ApplicationContext` and/or the beans we are interested in,
>> that is, in Log4j terms, inject the `Injector` and/or the plugins we are
>> interested in.
>> 
>> Thoughts?
> 
> What you are suggesting is that the injector should just be another bean 
> that can be injected. I have no problem with that.
> 
> I should mention that I asked Matt to look into an issue I have with the 
> Spring 
> support. Spring needs access to its Environment. We currently save that in 
> the LoggerContex. However, the SpringPropertySource has a circularity 
> problem. 
> PropertySources are created before anything else in Log4j, including the 
> Injector. 
> The Spring support currently uses an EnvironmentHolder singleton. I’d really 
> like 
> to do a PR for Spring to move the Spring Boot code there but I am not 
> comfortable 
> doing it with the EnvironmentHolder.
> 
> What happens how is that the EnvironmentHolder checks to see if Log4j has 
> initialized. 
> If it hasn’t it returns null. If it has then it accesses the LoggerContext to 
> get the Environment.
> This just feels clunky. What I would prefer is to have the PropertySource be 
> injected 
> with the Environment when it becomes available. In essence, deferred 
> injection. 
> 
> Ralph
> 
> 
> 
> 
> 
> 

Reply via email to