Configurable and Plugin were combined as a single annotation before,
but the Core-category/namespace-specific configurability options don't
apply to other types of plugins, so I separated them. The benefit of
separating them is so that people don't get the strange idea that it's
possible to use multiple namespaces of plugins to configure in a flat
namespace (i.e., we're not using XML namespaces, and the other formats
don't have a concept of namespaces). At this point, Plugin is
basically the same thing as Named, but we keep an index of Plugin
classes. Since we don't want to just scan and load every possible
bean-like class at startup, we need a way to distinguish them.
In a sense, Configurable could potentially have a value() or name()
element for the plugin name, and that can work via a NameProvider (or
maybe through the recent annotation stereotyping code I added), though
the annotation processor would need to be updated to index them, too
(and supporting annotation stereotypes there is harder as the
annotation processing API is far more verbose when dealing with
dynamic annotations like that). In a sense, a plugin is kind of like a
Guice module or Spring configuration class that you still need to
import into your main config or other classes first before they're
activated.
This also makes use of Plugin a bit simpler for non-configurable plugins.
Note that this is mainly explaining how things work as of now, not
necessarily how they must be. Thanks for continuing to examine the
architecture here! If you have ideas on how you'd like the use of an
ideal DI API would look, that also helps as I can look into how
feasible it is to support. Some things are simpler than others to port
over, too, from other DI systems, while others are beyond the scope
I'd feel comfortable shipping. For example, advanced DI libraries like
Spring tend to use bytecode manipulation to support proxying
non-interfaces at runtime (such as via cglib, asm, bytebuddy,
whatever) in order to support interceptors, decorators, and other AOP
stuff, while I don't want to add that type of dependency to
log4j-{api,plugins,core}. If it's possible to implement via Java 11
standard modules (including java.compiler for the annotation
processor, though perhaps even the jdk modules that go with it if we
want to do more advanced Lombok-like things), then that's something to
consider.
See also some interesting looking modules that come with Java that I
never really noticed before (I mean, they seem relatively new, but a
version of 9 could mean that was when the _module_ was introduced and
not the API itself):
-
https://docs.oracle.com/en/java/javase/11/docs/api/jdk.dynalink/module-summary.html
-
https://docs.oracle.com/en/java/javase/11/docs/api/jdk.compiler/module-summary.html
On Mon, May 23, 2022 at 2:54 PM Volkan Yazıcı <vol...@yazi.ci> wrote:
>
> Matt, maybe I am not getting it but... I understand your explanation about
> Configurable-vs-Plugin, yet my question still stands: Can something be
> `@Configurable` but not `@Plugin`? If I am not mistaken, every single
> `@Configurable` usage is followed by a `@Plugin`. Hence, I am inclined to
> make `Configurable` extend from `Plugin`. This will not only simplify the
> call-site, but will communicate a more clear message: this is a plugin that
> will be configured. I can also reverse my question: What would be the
> benefit of keeping `@Configurable` and `@Plugin` separate?
>
> On Sun, May 22, 2022 at 11:49 PM Matt Sicker <boa...@gmail.com> wrote:
>
> > Those lazy wrappers are an insignificant detail. Feel free to simplify.
> >
> > Configurable is a namespace annotation, not a name annotation. Plugin is a
> > name annotation that gets indexed.
> >
> > If you’d like fancier conditional annotations, please describe what parts
> > you want. I don’t want to port over every feature of Spring’s DI system as
> > it’s fairly extensive and mostly over complex for our use case.
> >
> > —
> > Matt Sicker
> >
> > > On May 22, 2022, at 15:36, Volkan Yazıcı <vol...@yazi.ci> wrote:
> > >
> > > Great work Matt! I have some questions/remarks:
> > >
> > > 1. Why did we introduce `LazyInt`, `LazyBoolean`, etc. rather than
> > > simply leveraging `Lazy<V>`? If the concern is nullability, we could
> > have
> > > checked the `supplier` response against `null` in `Lazy<V>` itself.
> > > 2. Why is `Configurable` not annotated with `Plugin`? Can something be
> > > `@Configurable` but not `@Plugin`?
> > > 3. `ConditionalOnProperty` is missing `havingValue` and
> > `matchIfMissing`.
> > >
> > >
> > >> On Fri, May 20, 2022 at 11:37 PM Matt Sicker <boa...@gmail.com> wrote:
> > >>
> > >> 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
> > >>>
> > >>>
> > >>>
> > >>>
> > >>>
> > >>>
> > >>
> >
