Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix plugin subclassing in Log4j Docgen #120

Merged
merged 6 commits into from
May 7, 2024
Merged

Conversation

vy
Copy link
Member

@vy vy commented May 7, 2024

This change fixes #117, which misses loggers > logger entry.

Why is the entry missing?

log4j-core-plugins.xml contains two entries for LoggerConfig:

  1. <abstractType className="org.apache.logging.log4j.core.config.LoggerConfig">
  2. <plugin name="logger" className="org.apache.logging.log4j.core.config.LoggerConfig">

TypeLookup encounters the <abstractType first and consequently discards the <plugin, hence logger doesn't show up in the LoggerConfig XSD group.

How shall we solve the problem?

If an FQCN is a plugin and is also subclassed by other plugins:

  1. DescriptorGenerator should not generate an AbstractType, but only a PluginType.
  2. TypeLookup should promote an AbstractType to a PluginType if the associated FQCN is later on found to be a plugin. (This can still be the case even after the DescriptorGenerator fix mentioned above. That is, the plugin and the subclassed plugin can be provided in different descriptor files.)

@vy vy added the bug label May 7, 2024
@vy vy added this to the 0.9.0 milestone May 7, 2024
@vy vy requested a review from ppkarwasz May 7, 2024 08:48
@vy vy self-assigned this May 7, 2024
Copy link
Contributor

@ppkarwasz ppkarwasz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

I think that in the long run we could remove the ArtifactSourcedType class and move its additional fields to Type. These fields would have a value of null by default, which would mean that they are equal to the same fields in the PluginSet that contains them, but would be explicitly filled up by TypeLookup.

@vy vy merged commit bb0347a into main May 7, 2024
9 checks passed
@vy vy deleted the fix/docgen-plugin-subclass branch May 7, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Log4j XSD doesn't define loggers > logger element
2 participants