Skip to content

Commit f87c5a7

Browse files
committed
Allow arbitrary position of <Properties> element
Until now the `<Properties>` element had to be the first child of `<Configuration>`. In the current architecture this restriction is no longer necessary and can be lifted.
1 parent 9e71d44 commit f87c5a7

File tree

3 files changed

+94
-15
lines changed

3 files changed

+94
-15
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.core.config;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
21+
import org.apache.logging.log4j.core.Appender;
22+
import org.apache.logging.log4j.core.appender.ConsoleAppender;
23+
import org.junit.jupiter.api.Test;
24+
25+
public class ConfigurationPropertiesOrderTest {
26+
27+
@Test
28+
void propertiesCanComeLast() {
29+
final Configuration config = new AbstractConfiguration(null, ConfigurationSource.NULL_SOURCE) {
30+
@Override
31+
public void setup() {
32+
// Nodes
33+
final Node appenders = newNode(rootNode, "Appenders");
34+
rootNode.getChildren().add(appenders);
35+
36+
final Node console = newNode(appenders, "Console");
37+
console.getAttributes().put("name", "${console.name}");
38+
appenders.getChildren().add(console);
39+
40+
final Node loggers = newNode(rootNode, "Loggers");
41+
rootNode.getChildren().add(loggers);
42+
43+
final Node rootLogger = newNode(loggers, "Root");
44+
rootLogger.getAttributes().put("level", "INFO");
45+
loggers.getChildren().add(rootLogger);
46+
47+
final Node properties = newNode(rootNode, "Properties");
48+
rootNode.getChildren().add(properties);
49+
50+
final Node property = newNode(properties, "Property");
51+
property.getAttributes().put("name", "console.name");
52+
property.getAttributes().put("value", "CONSOLE");
53+
properties.getChildren().add(property);
54+
}
55+
56+
private Node newNode(final Node parent, final String name) {
57+
return new Node(rootNode, name, pluginManager.getPluginType(name));
58+
}
59+
};
60+
config.initialize();
61+
62+
final Appender noInterpolation = config.getAppender("${console.name}");
63+
assertThat(noInterpolation).as("No interpolation for '${console.name}'").isNull();
64+
final Appender console = config.getAppender("CONSOLE");
65+
assertThat(console).isInstanceOf(ConsoleAppender.class);
66+
}
67+
}

log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -647,18 +647,21 @@ protected void doConfigure() {
647647
processConditionals(rootNode);
648648
preConfigure(rootNode);
649649
configurationScheduler.start();
650-
if (rootNode.hasChildren() && rootNode.getChildren().get(0).getName().equalsIgnoreCase("Properties")) {
651-
final Node first = rootNode.getChildren().get(0);
652-
createConfiguration(first, null);
653-
if (first.getObject() != null) {
654-
final StrLookup lookup = (StrLookup) first.getObject();
655-
if (lookup instanceof LoggerContextAware) {
656-
((LoggerContextAware) lookup).setLoggerContext(loggerContext.get());
650+
// Find the "Properties" node first
651+
boolean hasProperties = false;
652+
for (final Node node : rootNode.getChildren()) {
653+
if ("Properties".equalsIgnoreCase(node.getName())) {
654+
hasProperties = true;
655+
createConfiguration(node, null);
656+
if (node.getObject() != null) {
657+
final StrLookup lookup = node.getObject();
658+
runtimeStrSubstitutor.setVariableResolver(lookup);
659+
configurationStrSubstitutor.setVariableResolver(lookup);
657660
}
658-
runtimeStrSubstitutor.setVariableResolver(lookup);
659-
configurationStrSubstitutor.setVariableResolver(lookup);
661+
break;
660662
}
661-
} else {
663+
}
664+
if (!hasProperties) {
662665
final Map<String, String> map = this.getComponent(CONTEXT_PROPERTIES);
663666
final StrLookup lookup = map == null ? null : new PropertiesLookup(map);
664667
final Interpolator interpolator = new Interpolator(lookup, pluginPackages);
@@ -670,7 +673,7 @@ protected void doConfigure() {
670673
boolean setLoggers = false;
671674
boolean setRoot = false;
672675
for (final Node child : rootNode.getChildren()) {
673-
if (child.getName().equalsIgnoreCase("Properties")) {
676+
if ("Properties".equalsIgnoreCase(child.getName())) {
674677
if (tempLookup == runtimeStrSubstitutor.getVariableResolver()) {
675678
LOGGER.error("Properties declaration must be the first element in the configuration");
676679
}
@@ -680,7 +683,7 @@ protected void doConfigure() {
680683
if (child.getObject() == null) {
681684
continue;
682685
}
683-
if (child.getName().equalsIgnoreCase("Scripts")) {
686+
if ("Scripts".equalsIgnoreCase(child.getName())) {
684687
for (final AbstractScript script : child.getObject(AbstractScript[].class)) {
685688
if (script instanceof ScriptRef) {
686689
LOGGER.error(
@@ -690,19 +693,19 @@ protected void doConfigure() {
690693
scriptManager.addScript(script);
691694
}
692695
}
693-
} else if (child.getName().equalsIgnoreCase("Appenders")) {
696+
} else if ("Appenders".equalsIgnoreCase(child.getName())) {
694697
appenders = child.getObject();
695698
} else if (child.isInstanceOf(Filter.class)) {
696699
addFilter(child.getObject(Filter.class));
697-
} else if (child.getName().equalsIgnoreCase("Loggers")) {
700+
} else if (child.isInstanceOf(Loggers.class)) {
698701
final Loggers l = child.getObject();
699702
loggerConfigs = l.getMap();
700703
setLoggers = true;
701704
if (l.getRoot() != null) {
702705
root = l.getRoot();
703706
setRoot = true;
704707
}
705-
} else if (child.getName().equalsIgnoreCase("CustomLevels")) {
708+
} else if (child.isInstanceOf(CustomLevels.class)) {
706709
customLevels = child.getObject(CustomLevels.class).getCustomLevels();
707710
} else if (child.isInstanceOf(CustomLevelConfig.class)) {
708711
final List<CustomLevelConfig> copy = new ArrayList<>(customLevels);
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3+
xmlns="http://logging.apache.org/log4j/changelog"
4+
xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.3.xsd"
5+
type="fixed">
6+
<description format="asciidoc">
7+
Allow the &lt;Properties&gt; node to appear in any position in the configuration element.
8+
</description>
9+
</entry>

0 commit comments

Comments
 (0)