And to answer your initial question, something could be Configurable and not a Plugin, but that would only have the effect of putting it in the Core namespace. The rest of the annotation only has interpretation in Injector::configure.
— Matt Sicker > On May 23, 2022, at 15:48, Matt Sicker <[email protected]> wrote: > > 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ı <[email protected]> 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 <[email protected]> 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ı <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >>>>> wrote: >>>>>> >>>>>> >>>>>> >>>>>>> On May 19, 2022, at 12:15 AM, Volkan Yazıcı <[email protected]> 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 >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>
