And I think one bit of confusion with plugin keys/names/etc. is that this is
mostly relevant for either Core plugins (since that determines the element name
used in configurations) or for any other plugin namespace that uses aliases for
plugin classes, though some of those plugin namespaces define their own
annotations for defining their keys (see for example @ConverterKeys which might
be able to be a generic naming-annotation instead). StrLookup plugins use the
plugin key as the key used in ${key:…}.
—
Matt Sicker
> On May 27, 2022, at 11:08, Matt Sicker <[email protected]> wrote:
>
> 1. Plugin key is separate from the plugin name due to automatically
> filling in some info from elementType, though I've rearranged the code
> a bit in 3.x to make this part less confusing.
>
> 2. It's only @Configurable (Core) plugins that use the element type,
> printable, and deferChildren options. I could potentially break up the
> classes, but that does complicate other areas like annotation
> processing.
>
> 3. This is somewhat of a relic from attempting to keep plugin loading
> lazy. The idea here was that we could find plugins by their
> implemented interface as a way to collect a subset of them without
> loading their classes, but this still hasn't replaced the use of
> elementType or namespace that already implicitly organize plugins by
> interface. I'll likely remove this.
>
> 4. Yeah, there's surface-level overlap, but they have opposite
> purposes: constraint validators will prevent invalid objects from
> being created, while conditionals will allow certain objects to be
> created based on some state. One is for raising errors, the other is
> for conditionally enabling things.
>
> 5 and 6. I'd be open to further organizing in the log4j-plugins module.
>
> On Fri, May 27, 2022 at 10:38 AM Ralph Goers <[email protected]>
> wrote:
>>
>>
>>
>>> On May 27, 2022, at 6:54 AM, Volkan Yazıcı <[email protected]> wrote:
>>>
>>> Matt, some more questions/remarks:
>>>
>>> 1. AFAIU, plugin `key` is the normalized `name`. Right? If so, what is
>>> the purpose of the `name`?
>>
>> See PluginElementVisitor (at least in 2.x)
>>
>> @Override
>> public PluginEntry visitType(final TypeElement e, final Plugin plugin) {
>> Objects.requireNonNull(plugin, "Plugin annotation is null.");
>> final PluginEntry entry = new PluginEntry();
>> entry.setKey(plugin.name().toLowerCase(Locale.US));
>> entry.setClassName(elements.getBinaryName(e).toString());
>> entry.setName(Plugin.EMPTY.equals(plugin.elementType()) ? plugin.name() :
>> plugin.elementType());
>> entry.setPrintable(plugin.printObject());
>> entry.setDefer(plugin.deferChildren());
>> entry.setCategory(plugin.category());
>> return entry;
>> }
>> Some elements (such as KeyValuePair) don’t specify an elementType. In that
>> case the name attribute is set to the plugin’s name attribute value.
>> Otherwise
>> it is the elementType. The key is always the Plugin’s name attribute.
>>
>>
>>
>>> 2. I was expecting `PluginEntry` to be simplified similar to `Plugin`
>>> and only contain a `String key`, `String name`, and `String className`.
>>
>> Why would you expect that? All the fields are used by the Plugin system and
>> need to be in the plugin metadata to load it. Remember, at this point we
>> don’t
>> use the annotations any more.
>>
>>
>>> For
>>> `@Configurable`, we could create a `ConfigurablePlugin` class extending
>>> from `Plugin` with extra `elementType`, `printable`, etc. fields. Generated
>>> `Log4jPlugins.java` could instantiate either `Plugin` or
>>> `ConfigurablePlugin` depending on the need. In its current state, `Plugin`
>>> looks pretty generic, though the rest still resembles the past
>>> architecture.
>>
>> IMO what you propose just makes things unnecessarily complicated.
>>
>>> 3. What is the purpose of populating interfaces (i.e.,
>>> `PluginEntry#interfaces`) at compile-time? What does use them and how?
>>
>> I’m interested in this too as PluginProcessor doesn’t seem to be setting any
>> values.
>>
>>> 4. I see quite some overlap between `plugins.condition` and
>>> `plugins.validation`. I would prefer the latter to be modernized and merged
>>> into the former.
>>> 5. I would prefer `Configurable`, `Node`, et al. to be moved to their
>>> own `plugins.config` package.
>>
>> Not sure I agree with this.
>>
>>> 6. I would prefer `di.Scope` et al. to be moved to their own
>>> `plugins.scope` package – or at least, be located in the same package. For
>>> instance, consider `plugins.di.Scope` vs. `plugins.ScopeType`.
>>
>> I agree it should be moved. It could be either plugins.scope or
>> plugins.di.scope.
>> I’d prefer the former since it is related to the Plugin annotation
>>
>> Ralph
