carterkozak commented on a change in pull request #733:
URL: https://github.com/apache/logging-log4j2/pull/733#discussion_r794523439
##########
File path:
log4j-core/src/main/java/org/apache/logging/log4j/core/config/properties/PropertiesConfigurationBuilder.java
##########
@@ -190,6 +193,17 @@ public PropertiesConfiguration build() {
return builder.build(false);
}
+ private static void useSyntheticLevelAndAppenderRefs(final String
propertyName, final Properties loggerProps, final Properties context) {
+ final String propertyValue = (String) context.remove(propertyName);
+ if (propertyValue != null) {
+ final String[] parts = propertyValue.split(",");
+ loggerProps.setProperty("level", parts[0]);
+ for (int i = 1; i < parts.length; i++) {
+ loggerProps.setProperty("appenderRef.$" + i + ".ref",
parts[i]);
Review comment:
It may be worthwhile to add some StatusLogger debug logging here, I
defer to your judgement :-)
##########
File path:
log4j-core/src/main/java/org/apache/logging/log4j/core/config/properties/PropertiesConfigurationBuilder.java
##########
@@ -190,6 +193,17 @@ public PropertiesConfiguration build() {
return builder.build(false);
}
+ private static void useSyntheticLevelAndAppenderRefs(final String
propertyName, final Properties loggerProps, final Properties context) {
+ final String propertyValue = (String) context.remove(propertyName);
+ if (propertyValue != null) {
+ final String[] parts = propertyValue.split(",");
+ loggerProps.setProperty("level", parts[0]);
Review comment:
We should check that `parts` is not empty to avoid an
`ArrayIndexOutOfBoundsException`. If memory serves, `",".split(",")` will
result in an empty array.
##########
File path:
log4j-core/src/main/java/org/apache/logging/log4j/core/config/properties/PropertiesConfigurationBuilder.java
##########
@@ -207,7 +221,12 @@ private ScriptFileComponentBuilder createScriptFile(final
Properties properties)
}
private AppenderComponentBuilder createAppender(final String key, final
Properties properties) {
- final String name = (String) properties.remove(CONFIG_NAME);
+ final String name;
+ if (properties.containsKey(CONFIG_NAME)) {
+ name = (String) properties.remove(CONFIG_NAME);
+ } else {
+ name = key;
+ }
Review comment:
I think we may be slightly better off retaining the original `remove`
logic, only traversing the map once:
```suggestion
final String name = Objects.toString(properties.remove(CONFIG_NAME),
key);
```
##########
File path:
log4j-core/src/main/java/org/apache/logging/log4j/core/config/properties/PropertiesConfigurationBuilder.java
##########
@@ -190,6 +193,17 @@ public PropertiesConfiguration build() {
return builder.build(false);
}
+ private static void useSyntheticLevelAndAppenderRefs(final String
propertyName, final Properties loggerProps, final Properties context) {
+ final String propertyValue = (String) context.remove(propertyName);
+ if (propertyValue != null) {
+ final String[] parts = propertyValue.split(",");
Review comment:
Do we need to trim each segment of `parts` to avoid whitespace issues? I
suspect the level piece may handle it gracefully, but I'm less sure about the
appenderRefs. Could be worth a test.
--
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]
