Fix handling of onMatch and onMismatch attributes in the properties configuration format#4116
Conversation
…Attributte (apache#2791) + more appropriate method name
+ made all Default***Builder implementations @ProviderrType + added setXXXAttribute method to some component builder interfaces with default implementations + added JSpecify @NullMarked to packages and @nullable where approrpiate to fields/mehods (per last PR review) + deprecated ComponentBuilder#addAttribute(...) methods and replaced with ComponentBuilder#setAttribute(...) methods in line with other builders (i.e. Plugins) - also since they don't just add but also remove if the value is null + reorganized ConfigurationBuilder to group getters, setters, builder factory methods + incremented package-info.java @Version annoations + added some null-guards in BuiltConfiguration#setup() + moved DeffaultConfigurationBuilder XML generation code from top to bottom of source file + removed "extra" attributes in Default***Builder constructors and replaced with setXXXAttribute chained calls in DefaultConfigurrationBuilder#newXXX(...) convenience methods + javadoc'd both packages (fixing documentation errors)
…instead of void (apache#2791) + added @BaselineIgnore due to API change (but breaks no code)
…lve builder conflicts
onMatch and onMismatch attributes in the properties configuration format
onMatch and onMismatch attributes in the properties configuration formatonMatch and onMismatch attributes in the properties configuration format
| * @param level the level | ||
| * @return this builder (for chaining) | ||
| */ | ||
| default AppenderRefComponentBuilder setLevelAttribute(@Nullable String level) { |
There was a problem hiding this comment.
As @ppkarwasz remarked earlier, I was expecting no set*Attribute methods. Am I missing something?
| * @param newValue the new value | ||
| * @return the previous value or {@code null} if none was set | ||
| */ | ||
| protected @Nullable String putAttribute(final String key, final @Nullable String newValue) { |
There was a problem hiding this comment.
Please see @ppkarwasz's earlier remark:
Why is this protected?
It has a better sounding name than
addAttribute. MaybeaddAttributeshould be deprecated (possibly with an Error Prone @InlineMe annotation) and this should bepublic?
There was a problem hiding this comment.
These changes are redundant, please revert 'em.
| * @return this builder (for chaining) | ||
| */ | ||
| ConfigurationBuilder<T> addProperty(String key, String value); | ||
| ConfigurationBuilder<T> add(PropertyComponentBuilder builder); |
There was a problem hiding this comment.
Don't we need a @throws NPE ... documentation here too?
| * @param packages a comma separated list of packages | ||
| * @return this builder (for chaining) | ||
| * @deprecated use package metadata and plugin descriptors instead of runtime package scanning | ||
| */ |
| @Override | ||
| public T addAttribute(final String key, final int value) { | ||
| return put(key, Integer.toString(value)); | ||
| public @NonNull CB getBuilder() { |
There was a problem hiding this comment.
See @ppkarwasz's earlier remark:
Isn't
@NonNullthe default?
| <issue id="2791" link="https://github.com/apache/logging-log4j2/issues/2791"/> | ||
| <description format="asciidoc"> | ||
| Fix problem when null attribute values are set on DefaultComponentBuilder. GitHub issue #2791. | ||
| Added JSpecify annotations to config.builder.* packages. | ||
| Updated Builder APIs and Default***Builder implementations. |
There was a problem hiding this comment.
| <issue id="2791" link="https://github.com/apache/logging-log4j2/issues/2791"/> | |
| <description format="asciidoc"> | |
| Fix problem when null attribute values are set on DefaultComponentBuilder. GitHub issue #2791. | |
| Added JSpecify annotations to config.builder.* packages. | |
| Updated Builder APIs and Default***Builder implementations. | |
| <issue id="4116" link="https://github.com/apache/logging-log4j2/pull/4116"/> | |
| <description format="asciidoc"> | |
| Rework programmatic configuration API and its implementations. |
There was a problem hiding this comment.
@ramanathan1504, would you mind creating one more changelog entry file with the following content, please?
- Issues: Fix handling of
onMatchandonMismatchattributes in the properties configuration format #2791, 2791 Fix handling ofonMatchandonMismatchattributes in the properties configuration format #3505, Fix handling ofonMatchandonMismatchattributes in the properties configuration format #4116 - Type:
fixed - Description: Fix handling of
onMatchandonMismatchattributes in the properties configuration format
There was a problem hiding this comment.
ok i will do that
Fixes #3505 Issue #2791
Addressed review feedback in this update:
Properties builder robustness (
log4j-core/.../PropertiesConfigurationBuilder.java)rootPropertiesinbuild().Long.parseLong(...)forshutdownTimeoutwith validated parsing (parseShutdownTimeout(...)) that throws aConfigurationExceptionwith a clear message.setLevelAttribute(...),setAdditivityAttribute(...)) to avoid regressions.Properties consumption correctness (
log4j-core/.../PropertiesConfigurationBuilder.java)levelAndRefsreads to remove the consumed key (properties.remove("")) to avoid stale/unprocessed entries.processRemainingProperties(...)before returning, so extra nested attributes/components are not dropped.Null/empty check cleanup (
log4j-core/.../PropertiesConfigurationBuilder.java)Strings.isEmpty(type)increateComponent(...).Changelog typo fix (
src/changelog/.2.x.x/2791_rework_ComponentBuilder_API.xml)JVerify->JSpecify.