vpelikh commented on PR #4153: URL: https://github.com/apache/logging-log4j2/pull/4153#issuecomment-4990620550
Thanks @ramanathan1504 — I've pulled @JWT007's original port commits from #3510 into this PR (authorship preserved) and addressed the open review feedback that @vy and @ppkarwasz left there: **@ppkarwasz — `setText(null)` should throw NPE (not `IllegalArgumentException`)** - `setText(@NonNull final String text)` now calls `Objects.requireNonNull(text, ...)` first, so a `null` argument throws `NullPointerException` (the `@NonNull` contract violation). - An empty string still throws `IllegalArgumentException` via `Assert.requireNonEmpty`, keeping the two validation levels distinct (null → NPE, empty → IAE). The corresponding test now asserts `NullPointerException`. **@ppkarwasz — the misspelled-plugin-name test didn't belong here** - The `nok` fixture (`StringMatchFfilter`, a typo) is replaced with a correct `<StringMatchFilter/>` element that omits the required `text` attribute, so the test now exercises `StringMatchFilter`'s own `@Required` validation (the plugin is skipped → `getFilter()` is `null`) rather than `PluginBuilder` name resolution. **@vy / @ppkarwasz — avoid cosmetic changes on existing code** - Reverted the method reordering and the rewritten `filter()` javadocs back to the original `main` form; only the validation behavior was added. The `filter()` method bodies are unchanged. - Dropped the out-of-scope `equalsImpl` refactors in `AbstractFilter`/`AbstractLifeCycle` and the `Assert` tweaks into a separate follow-up change, keeping this PR focused on `StringMatchFilter` validation. All 6 `StringMatchFilterTest` cases pass and Spotless is clean. PTAL — once approved I'll close #3510. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
