By the way, I hope my latest commit renaming categories to namespaces
and moving some annotation metadata around should help clarify the
scope of things. In particular, I made an alias annotation for Core
category/namespace plugins called @Configurable which makes their use
case more obvious. Similarly, I moved the Core-specific annotation
data from @Plugin to @Configurable since they only apply there. Now
that the category is in the @Namespace annotation, @Plugin only has a
single value() string to set which is the name of the plugin (which
will default to using the simple class name if the plugin name is an
empty string; I've removed explicit plugin names on classes whose
simple names already match their plugin names). Note that you can
still define namespace alias annotations for different namespaces
(I've only made them for Core, Lookup, and TypeConverter namespaces so
far).
Some next steps to make the system more consistent:
* @ConditionalOnFoo annotations for injectable bundles (i.e., the
equivalent to a @Configuration class in Spring)
* Ability to inject PluginType<T> for plugin namespaces that shouldn't
eagerly load available plugins
* Further cleanup and potential supporting APIs to apply inversion of
control rather than calling Injector APIs
* Figuring out how Injector may interact with or otherwise set up
things like PropertySource services or other log4j-api services. This
might be doable via a sort of "DI promise" API where the DI system
will complete the promise or some variation of lazy async loading
>From there, hopefully any remaining limitations or code smells will be
obvious enough to fix up before 3.0.0.
On Thu, May 19, 2022 at 3:06 PM Matt Sicker <boa...@gmail.com> wrote:
>
> 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
>
>
>
>
>
>
