Another idea that I had while looking through CDI is supporting a unit of work logging feature LOG4J2-1630 [1] might be supportable with something similar to @ConversationScoped in CDI [2].
[1]: https://issues.apache.org/jira/browse/LOG4J2-1630 [2]: https://docs.jboss.org/cdi/api/2.0/javax/enterprise/context/ConversationScoped.html On Sat, 9 Nov 2019 at 11:46, Matt Sicker <boa...@gmail.com> wrote: > > ScriptRef is a fairly simple conversion: you'd either add @Inject on > its current constructor (we'd make ScriptManager a singleton or > similar scope) and potentially adding a configuration scope annotation > on the name parameter (the default scope in javax.inject is supposed > to be effectively a prototype scope), or you'd change the > Configuration parameter in the factory method to be ScriptManager > instead. No scope annotation would likely be required because there > would only be a single ScriptManager to choose from the available > scopes. > > CronTriggeringPolicy could be likewise update the same way. This would > be even easier to support by adding a TypeConverter for > CronExpression. > > ScriptPatternSelector is slightly more interesting. The StrSubstitutor > can be injected directly (probably a configuration-scope object). I'm > not currently sure how to model the configuration properties since > it's just a string->string map currently, though we could always add a > wrapper class there if necessary. > > For AbstractLayout, the configuration can be replaced by the injected > StrSubstitutor as well. AbstractStringLayout relies on the root logger > config, and this is one of those scenarios where I expect the > injection API to really show off features. This would be a > LoggerConfig injection with some sort of qualifier. > > As for Lookup overhead, I don't imagine this system to introduce much > overhead (if any at all once JIT'd) since constructors and such are > assembled at startup to an extent. The static code generation version > should further avoid overhead as it would just be directly setting up > objects without reflection. > > On Tue, 5 Nov 2019 at 11:25, Matt Sicker <boa...@gmail.com> wrote: > > > > Yes, these sound like great starting points. I'll work on a proof of > > concept of what I imagine the classes can look like after implementing > > the new injection system. > > > > On Mon, 4 Nov 2019 at 22:48, Ralph Goers <ralph.go...@dslextreme.com> wrote: > > > > > > PatternLayout and RFC5424Layout use the Configuration to pass to > > > Converters, primarily so they can use the Interpolator. > > > CronTriggeringPolicy and IdlePurgePolicy access the Configuration to get > > > to the ConfigurationScheduler. ScriptPatternSelector, and all other > > > Script plugins, accesses the Configuration to ge to the ScriptManager. > > > > > > Note that Lookups all have no-arg constructors but some of them, such as > > > Log4jLookup, need access to the Configuration. They do that by extending > > > AbstractConfigurationAwareLookup. That could obviously be replaced with > > > an annotation to perform the injection, although checking for the > > > interface probably involves less overhead. > > > > > > Hopefully this is what you are looking for. I am sure there are other > > > cases though. > > > > > > Ralph > > > > > > > > > > > > > On Nov 4, 2019, at 1:18 PM, Matt Sicker <boa...@gmail.com> wrote: > > > > > > > > Do you have any example plugins that might be a good example to showcase > > > > the scopes and qualifiers model? Preferably a plugin that injects a > > > > Configuration in order to obtain some other services or other ad hoc > > > > type > > > > plugins. Any plugin using just attributes (and elements and values) > > > > would > > > > be trivial to convert to the updated system (they’d likely be unchanged > > > > depending on how I model stereotypes; this is one area I intend to > > > > significantly diverge from CDI and act more like Spring where you can > > > > make > > > > stereotypes out of almost any annotation instead of just qualifiers and > > > > a > > > > single scope), so a plugin that needs injected values from elsewhere > > > > would > > > > probably showcase this idea best. > > > > > > > > On Sun, Nov 3, 2019 at 23:30 Matt Sicker <boa...@gmail.com> wrote: > > > > > > > >> I see what you did with the annotation processor. I figured I’d be > > > >> able to > > > >> add more metadata in the service class generated basically. If we can > > > >> identify the various logical scopes in the library, I think that could > > > >> lead > > > >> to less potential bugs. > > > >> > > > >> On Sun, Nov 3, 2019 at 14:14 Ralph Goers <ralph.go...@dslextreme.com> > > > >> wrote: > > > >> > > > >>> I have a couple of comments: > > > >>> It isn’t really possible for me to know what you are suggesting > > > >>> without > > > >>> seeing some examples of how plugins would be defined. > > > >>> I have a suspicion the scopes could get nasty. Some plugins would be > > > >>> tied > > > >>> to the LoggerContext, which essentially has a lifetime of the > > > >>> ClassLoader > > > >>> (or Bundle, or Module), while others are tied to the configuration so > > > >>> have > > > >>> that lifetime. I am afraid of the problems that would be introduced > > > >>> if a > > > >>> Plugin tied to the LoggerContext suddenly has a configuration scope. > > > >>> That > > > >>> would mean having to worry about thread safety in places we currently > > > >>> may > > > >>> not have to. But again, I would want to see examples to be sure. > > > >>> Using the annotation processor in 3.0 is currently required when using > > > >>> OSGi or JPMS. This is because OSGi requires bundle loading and JPMS > > > >>> requires the use of the ServiceLoader. We do not want to have to load > > > >>> each > > > >>> and every plugin via the ServiceLoader so instead the PluginService > > > >>> class > > > >>> is loaded, which contains a Map of all the plugins in the > > > >>> bundle/module. I > > > >>> don’t see how you can avoid this. That said, you are certainly free > > > >>> to > > > >>> make the PluginService smarter and more efficient. > > > >>> > > > >>> Ralph > > > >>> > > > >>>> On Nov 3, 2019, at 12:18 PM, Matt Sicker <boa...@gmail.com> wrote: > > > >>>> > > > >>>> Oh, almost forgot to mention my other main goal here: I have a > > > >>>> hypothesis that if designed our APIs with inversion of control in > > > >>>> mind, programmatic configuration and extension should be _much_ > > > >>>> easier > > > >>>> to support while maintaining backward compatibility, too. While the > > > >>>> configuration framework itself is extremely powerful for creating > > > >>>> fairly dynamic configurations, a non-trivial amount of users want > > > >>>> more > > > >>>> programmatic control than any of the APIs we support. > > > >>>> > > > >>>> On Sat, 2 Nov 2019 at 16:04, Matt Sicker <boa...@gmail.com> wrote: > > > >>>>> > > > >>>>> I've been working on this part time over the weekends lately, and > > > >>>>> after exploring a bit of Guice and CDI, I've come to the conclusion > > > >>>>> that if we were to rebase the plugin system on any annotation style, > > > >>>>> it seems as though CDI provides a fairly good set of extensions to > > > >>>>> javax.inject which would potentially work well with our existing > > > >>>>> annotations for backward compatibility. I wouldn't want to introduce > > > >>>>> an actual dependency on it, but a plugin SPI inspired by it could > > > >>>>> potentially work. A nice advantage to adopting a CDI-like system is > > > >>>>> that it should be simpler to use by other developers who aren't > > > >>>>> familiar with our custom framework. > > > >>>>> > > > >>>>> General overview of potential changes: > > > >>>>> > > > >>>>> * In addition to scopes and qualifiers, introduce stereotype > > > >>>>> annotations which are used for combining multiple annotations. This > > > >>>>> will help a lot in making old annotations still work properly. > > > >>>>> * Introduce a Produces and Disposes annotation that work similarly > > > >>>>> to > > > >>> CDI. > > > >>>>> * Introduce a PostConstruct and PreDestroy annotation because later > > > >>>>> versions of Java removed them from the base JDK (they're in a > > > >>>>> separate > > > >>>>> module from java.base), and javax.inject is supposed to support > > > >>>>> those > > > >>>>> annotations. > > > >>>>> * Introduce a "Configuration" scope which lives for the life of a > > > >>>>> configuration. This would essentially replace the concept of a > > > >>>>> "core" > > > >>>>> plugin category as they map to the lifecycle of a configuration > > > >>>>> source. > > > >>>>> * Use the "Singleton" scope for anything we'd want to live for the > > > >>>>> entire application instance (generally replaces use of system > > > >>>>> property > > > >>>>> singletons where possible). > > > >>>>> * Integrate some common scopes like request-scoped, session-scoped, > > > >>>>> etc., to allow for a more pluggable system for introducing scopes > > > >>>>> for > > > >>>>> Lookups or other plugins. > > > >>>>> * Use the PostConstruct/PreDestroy callbacks to replace initialize > > > >>>>> and > > > >>>>> destroy methods in plugins. This might work well toward generic > > > >>>>> lifecycle management which is currently done ad hoc. > > > >>>>> * I have a vague notion that a "LogEvent" scope would be really > > > >>>>> neat, > > > >>>>> but such a system would likely not work well with GC-free code, so > > > >>>>> I'm > > > >>>>> not really considering this one at the moment. > > > >>>>> * PluginAttribute/PluginBuilderAttribute/PluginElement/PluginValue > > > >>>>> all > > > >>>>> become stereotype qualifiers with some annotation compatibility > > > >>>>> checks > > > >>>>> to support old annotations. > > > >>>>> * PluginFactory becomes a stereotype producer annotation. > > > >>>>> * PluginBuilderFactory (previously deleted) would also be a > > > >>>>> stereotype > > > >>>>> producer annotation. > > > >>>>> > > > >>>>> One thing I'm experimenting with right now that should really help > > > >>>>> inform me whether or not this effort will be worth it is breaking > > > >>>>> this > > > >>>>> down into an SPI that will have two default implementations: one > > > >>>>> based > > > >>>>> on runtime reflection, and another based on generated code from the > > > >>>>> annotation processor (a more advanced version of the existing > > > >>>>> PluginService code generator). I hope to avoid the use of actual > > > >>>>> reflection APIs in the generated code to more easily support GraalVM > > > >>>>> as well as to hopefully offload a bit of the reflective > > > >>>>> initialization > > > >>>>> to compile-time rather than at runtime (fairly useful for > > > >>>>> microservices where startup time is more important). > > > >>>>> > > > >>>>> I do have an overarching purpose behind all this: I'd like to > > > >>>>> dramatically reduce the tight coupling between Configuration, Node, > > > >>>>> and plugin classes. Some of this is in the form of a more generic > > > >>>>> dependency injection API to avoid the need for manually calling > > > >>>>> Configuration methods in plugin factories. Another is to make > > > >>>>> various > > > >>>>> services provided by Configuration to instead be injected directly > > > >>>>> as > > > >>>>> parameters to the plugin factories. Reducing the coupling between > > > >>>>> plugins and other running parts of the system should help reduce the > > > >>>>> complexity of many tests as well, and it may also help reduce the > > > >>>>> time > > > >>>>> taken running tests, though that will still likely be affected by > > > >>>>> the > > > >>>>> various integration tests regardless. > > > >>>>> > > > >>>>> On Wed, 9 Oct 2019 at 16:52, Matt Sicker <boa...@gmail.com> wrote: > > > >>>>>> > > > >>>>>> Part of this is to make it simpler to access globally configured > > > >>>>>> things without having to pass around a Configuration everywhere. If > > > >>>>>> plugins can declare the explicit parts of the API they need access > > > >>>>>> to > > > >>>>>> rather than the full Configuration, that should make tests a bit > > > >>>>>> lighter and easier to write. > > > >>>>>> > > > >>>>>> On Wed, 9 Oct 2019 at 16:11, Ralph Goers > > > >>>>>> <ralph.go...@dslextreme.com> > > > >>> wrote: > > > >>>>>>> > > > >>>>>>> FWIW, I don’t think @Inject is a good replacement for > > > >>> @PluginFactory. @Inject is essentially the same as @Autowired. It > > > >>> should > > > >>> be placed on fields where you want the implementation to be injected. > > > >>> @Inject specified on a method implies that the method parameters > > > >>> should be > > > >>> injected. I don’t think that is what you are intending. > > > >>>>>>> > > > >>>>>>> Ralph > > > >>>>>>> > > > >>>>>>>> On Oct 9, 2019, at 1:38 PM, Ralph Goers > > > >>>>>>>> <ralph.go...@dslextreme.com> > > > >>> wrote: > > > >>>>>>>> > > > >>>>>>>> Ok, but given @Scope and @Qualifier can only be used to annotate > > > >>> the existing PluginElement, PluginAttribute and PluginValue > > > >>> annotations I > > > >>> am not sure what they buy you. They can’t be used to replace the > > > >>> existing > > > >>> annotations as they are only allowed to be used on annotation > > > >>> declarations. I agree that @PluginConfiguration is somewhat > > > >>> redundant. All > > > >>> we are really doing with it is saying that the existing Configuration > > > >>> object should be passed to the method. Using @PluginConfiguration for > > > >>> that > > > >>> is easy but using @Inject as an indicator and checking the type of the > > > >>> parameter could just as easily be done. > > > >>>>>>>> > > > >>>>>>>> I guess I would like to see what you think an example plugin > > > >>>>>>>> would > > > >>> look like before going further. > > > >>>>>>>> > > > >>>>>>>> Ralph > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>>> On Oct 9, 2019, at 11:46 AM, Matt Sicker <boa...@gmail.com> > > > >>>>>>>>> wrote: > > > >>>>>>>>> > > > >>>>>>>>> I want to make it simpler to write plugins. Increase > > > >>>>>>>>> testability, > > > >>> reuse > > > >>>>>>>>> more standard APIs that others would be more familiar with > > > >>>>>>>>> (easier > > > >>>>>>>>> onboarding), and make it simpler to implement some tangentially > > > >>> related > > > >>>>>>>>> feature ideas. > > > >>>>>>>>> > > > >>>>>>>>> As for the builder versus factory annotation, there was already > > > >>> special > > > >>>>>>>>> support for collections and maps that weren’t explicit > > > >>> annotations, so > > > >>>>>>>>> determining what to do based on the return type of the factory > > > >>> method was > > > >>>>>>>>> already an established idea. I think it made sense to combine > > > >>>>>>>>> them > > > >>> all into > > > >>>>>>>>> @PluginFactory, and because of that, I noticed the > > > >>>>>>>>> correspondence > > > >>> between > > > >>>>>>>>> that and @Inject. > > > >>>>>>>>> > > > >>>>>>>>> On Wed, Oct 9, 2019 at 13:28, Ralph Goers < > > > >>> ralph.go...@dslextreme.com> > > > >>>>>>>>> wrote: > > > >>>>>>>>> > > > >>>>>>>>>> I still don’t understand what the benefit is. IIUC your plan is > > > >>> to add the > > > >>>>>>>>>> @Scope and @Qualifier annotations to the @PluginElement and > > > >>>>>>>>>> @PluginAttribute. If that somehow helps, great, as it is will > > > >>>>>>>>>> be > > > >>> invisible > > > >>>>>>>>>> to plugin developers. > > > >>>>>>>>>> > > > >>>>>>>>>> One thing I find odd is that Builders were annotated with > > > >>>>>>>>>> @PluginBuilderFactory and are now annotated with > > > >>>>>>>>>> @PluginFactory. > > > >>>>>>>>>> Previously, only factory methods were annotated with > > > >>> @PluginFactory. What > > > >>>>>>>>>> is odd is that the methods previously annotated with > > > >>> @PluginBuilderFactory > > > >>>>>>>>>> create a Builder. They builder then creates the plugin object. > > > >>> However the > > > >>>>>>>>>> methods annotated with @PluginFactory directly created the > > > >>>>>>>>>> plugin > > > >>> object. > > > >>>>>>>>>> Now you have both annotated with @PluginFactory, which means > > > >>>>>>>>>> some > > > >>> methods > > > >>>>>>>>>> behave one way and others behave another way. How is that > > > >>>>>>>>>> clearer? > > > >>>>>>>>>> > > > >>>>>>>>>> That said changing both to @Inject has some merit although I am > > > >>> not sure > > > >>>>>>>>>> how you are determining that one is creating a Builder that > > > >>>>>>>>>> then > > > >>> creates > > > >>>>>>>>>> the object vs a method that creates the object. > > > >>>>>>>>>> > > > >>>>>>>>>> But again, what problem is this trying to solve? How will this > > > >>> make things > > > >>>>>>>>>> easier for users? > > > >>>>>>>>>> > > > >>>>>>>>>> Ralph > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>>> On Oct 9, 2019, at 10:42 AM, Matt Sicker <boa...@gmail.com> > > > >>> wrote: > > > >>>>>>>>>>> > > > >>>>>>>>>>> To clarify on the mapping between javax.inject and the > > > >>>>>>>>>>> existing > > > >>>>>>>>>>> annotations, I believe this would work: > > > >>>>>>>>>>> > > > >>>>>>>>>>> @PluginFactory can be replaced by @Inject > > > >>>>>>>>>>> @PluginElement is a @Scope annotation > > > >>>>>>>>>>> @PluginAttribute is a @Qualifier annotation > > > >>>>>>>>>>> @PluginConfiguration could be a scope or singleton, but it's > > > >>> redundant > > > >>>>>>>>>>> @PluginNode could be a scope, but it's somewhat redundant > > > >>>>>>>>>>> @PluginValue would be a @Qualifier > > > >>>>>>>>>>> > > > >>>>>>>>>>> On Wed, 9 Oct 2019 at 10:05, Matt Sicker <boa...@gmail.com> > > > >>> wrote: > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> Ok, I see the issue. I won’t add a dependency on the API. I > > > >>>>>>>>>>>> do > > > >>> want to > > > >>>>>>>>>> try refactoring the 3.x API to use an annotation model similar > > > >>>>>>>>>> to > > > >>> that. > > > >>>>>>>>>> I’ll show a proof of concept sometime soon. > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> On Tue, Oct 8, 2019 at 18:50, Ralph Goers < > > > >>> ralph.go...@dslextreme.com> > > > >>>>>>>>>> wrote: > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> I don’t understand. You can’t add javax.inject stuff into > > > >>>>>>>>>>>>> our > > > >>>>>>>>>> namespace without changing the package name. And if you change > > > >>> the package > > > >>>>>>>>>> name I don’t see any benefit at all as the current names are > > > >>>>>>>>>> much > > > >>> clearer. > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> I have no problem with Configurations being a plugin except > > > >>>>>>>>>>>>> it > > > >>> will > > > >>>>>>>>>> currently cause an endless loop as plugins are captured during > > > >>>>>>>>>> configuration. So any change you make here is going to be huge. > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> Ralph > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>>> On Oct 8, 2019, at 2:23 PM, Matt Sicker <boa...@gmail.com> > > > >>> wrote: > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> I'm thinking that the old annotations can be supported in > > > >>> terms of the > > > >>>>>>>>>>>>>> javax.inject API. As for requiring a jar, that's why I've > > > >>>>>>>>>>>>>> also > > > >>>>>>>>>>>>>> suggested just adopting the annotations into our own > > > >>>>>>>>>>>>>> package > > > >>>>>>>>>>>>>> somewhere. > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> Either way this is done, my general goal is to untangle > > > >>>>>>>>>>>>>> other > > > >>> areas in > > > >>>>>>>>>>>>>> the core API that could benefit from generic DI support. > > > >>>>>>>>>>>>>> See > > > >>> for > > > >>>>>>>>>>>>>> example turning Configuration into a plugin. > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> On Tue, 8 Oct 2019 at 15:40, Ralph Goers < > > > >>> ralph.go...@dslextreme.com> > > > >>>>>>>>>> wrote: > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> I don’t see how that relates. The proposal as I understand > > > >>> it is to > > > >>>>>>>>>> replace the existing annotations with annotations from > > > >>> javax.inject, which > > > >>>>>>>>>> would require a JEE jar. > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> Ralph > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>> On Oct 8, 2019, at 1:31 PM, Jochen Wiedmann < > > > >>>>>>>>>> jochen.wiedm...@gmail.com> wrote: > > > >>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>> On Tue, Oct 8, 2019 at 10:26 PM Ralph Goers < > > > >>>>>>>>>> ralph.go...@dslextreme.com> wrote: > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> IIUC this will require a dependency on a Java EE jar? > > > >>>>>>>>>>>>>>>>> For > > > >>> that > > > >>>>>>>>>> reason alone, no. > > > >>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>> Don't think so. A simple (mostly JSR 330 compliant) > > > >>> provider can be > > > >>>>>>>>>>>>>>>> implemented in a few classes: > > > >>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>> > > > >>>>>>>>>> > > > >>> https://github.com/jochenw/afw/tree/master/afw-core/src/main/java/com/github/jochenw/afw/core/inject/simple > > > >>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> -- > > > >>>>>>>>>>>>>> Matt Sicker <boa...@gmail.com> > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>> -- > > > >>>>>>>>>>>> Matt Sicker <boa...@gmail.com> > > > >>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>>> -- > > > >>>>>>>>>>> Matt Sicker <boa...@gmail.com> > > > >>>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> -- > > > >>>>>>>>> Matt Sicker <boa...@gmail.com> > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>> > > > >>>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>>> -- > > > >>>>>> Matt Sicker <boa...@gmail.com> > > > >>>>> > > > >>>>> > > > >>>>> > > > >>>>> -- > > > >>>>> Matt Sicker <boa...@gmail.com> > > > >>>> > > > >>>> > > > >>>> > > > >>>> -- > > > >>>> Matt Sicker <boa...@gmail.com> > > > >>>> > > > >>> > > > >>> -- > > > >> Matt Sicker <boa...@gmail.com> > > > >> > > > > -- > > > > Matt Sicker <boa...@gmail.com> > > > > > > > > > > > > -- > > Matt Sicker <boa...@gmail.com> > > > > -- > Matt Sicker <boa...@gmail.com> -- Matt Sicker <boa...@gmail.com>
