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 <ralph.go...@dslextreme.com> wrote:
>
>
>
> > On May 27, 2022, at 6:54 AM, Volkan Yazıcı <vol...@yazi.ci> 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
