I added the annotations a while ago to work on reducing duplication of constraint validation littered throughout the codebase. However, given that it is only invoked during plugin object construction, there’s still plenty of manual checks for things (and not much in the way of validators implemented). Some validation checks require checking more than just the value itself (like seeing if another attribute was set which is incompatible, etc.), so those have to be done manually for now.
Validation can potentially be done more consistently in the dependency injection branch I’ve been working on recently, but that’s limited to 3.x. For 2.x, it might be useful if Builder provided a default method to run all the constraint validators on its fields. The difficulty here is that there’s no easy way to add the same for static factory methods, so those are only enforced when invoked reflectively. — Matt Sicker > On Feb 20, 2022, at 13:54, 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
