As usual you ask really good questions. I made the builders Plugins primarily because it was an easy way to identify them along with the class they are emulating. Other than that, the log4j-plugins builder type support really can’t be used as is because it operates on a Node hierarchy while we are still dealing directly with the properties or XML document.
I suspect if we simply extract the values and pass them to the Log4j 2 component’s Builder that it won’t be sufficient (I believe the PluginVisitor logic actually performs the validation) or the error produced would end up being confusing since it would be coming from a Log4j 2 component, not what the configuration specified. But the Builders I looked at don’t perform the validation. As I said, I am almost certain that happens in the Visitor that calls the builder methods. So I took the path of manually doing the validation because it didn’t seem like creating another layer of injection was worth the effort for the small number of builders needed in the 1.2 emulation. Ralph > On Feb 20, 2022, at 12:54 PM, Piotr P. Karwasz <[email protected]> > wrote: > > Hello, > > Inspired by the StackOverflow question [1], where `log4j-1.2-api` > provided the user with a totally useless error message, I was looking > at the validation of components from the perspective of a Log4j 1.x > bridge `Builder` plugin. From what I found there are 3 ways to > validate parameters: > > 1. There are nice annotations that are checked if the `PluginBuilder` > is used (Log4j 1.x bridge `Builder`s don't use it), > 2. The same constraints are also checked in the `Builder#build()` > methods of each component. This is not very consistent: the > `RollingFileAppender.Builder#build()` checks the `name` and > `filePattern` attribute again, but the `FileAppender.Builder#build()` > method does no such thing. > 3. Some Log4j 1.x bridge builders like the > `RollingFileAppenderBuilder` and `FileAppenderBuilder` perform > constraint checks themselves before calling the > `RollingFileAppender.Builder` and `FileAppender.Builder` and return > `null` if the check fails. Without those checks the Log4j 1.x bridge > `BuilderManager` would build an `AppenderWrapper(null)` and would not > fall back to direct instantiation of the > `org.apache.log4j.RollingFileAppender` class. > > I would like to put some order in the attribute validation, but what > would be the best approach? Should each core `Builder#build()` method > call the `PluginBuilder`'s annotation-driven validation code and only > deal directly with complex constraints? > > Piotr > > [1] https://stackoverflow.com/q/71149004/11748454
