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
>>>
>>>
>>>
>>>
>>>
>>>
>>
