This is an automated email from the ASF dual-hosted git repository. pkarwasz pushed a commit to branch fix/configuration-properties-order in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit ff5d0be10c503a4e34204f4abde5d0f1b70e576d Author: Piotr P. Karwasz <[email protected]> AuthorDate: Fri Feb 9 22:51:22 2024 +0100 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. --- .../config/ConfigurationPropertiesOrderTest.java | 67 ++++++++++++++++++++++ .../log4j/core/config/AbstractConfiguration.java | 33 ++++++----- .../.2.x.x/allow_arbitrary_properties_order.xml | 9 +++ 3 files changed, 94 insertions(+), 15 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationPropertiesOrderTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationPropertiesOrderTest.java new file mode 100644 index 0000000000..a725283da9 --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationPropertiesOrderTest.java @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.logging.log4j.core.config; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.apache.logging.log4j.core.Appender; +import org.apache.logging.log4j.core.appender.ConsoleAppender; +import org.junit.jupiter.api.Test; + +public class ConfigurationPropertiesOrderTest { + + @Test + void propertiesCanComeLast() { + final Configuration config = new AbstractConfiguration(null, ConfigurationSource.NULL_SOURCE) { + @Override + public void setup() { + // Nodes + final Node appenders = newNode(rootNode, "Appenders"); + rootNode.getChildren().add(appenders); + + final Node console = newNode(appenders, "Console"); + console.getAttributes().put("name", "${console.name}"); + appenders.getChildren().add(console); + + final Node loggers = newNode(rootNode, "Loggers"); + rootNode.getChildren().add(loggers); + + final Node rootLogger = newNode(loggers, "Root"); + rootLogger.getAttributes().put("level", "INFO"); + loggers.getChildren().add(rootLogger); + + final Node properties = newNode(rootNode, "Properties"); + rootNode.getChildren().add(properties); + + final Node property = newNode(properties, "Property"); + property.getAttributes().put("name", "console.name"); + property.getAttributes().put("value", "CONSOLE"); + properties.getChildren().add(property); + } + + private Node newNode(final Node parent, final String name) { + return new Node(rootNode, name, pluginManager.getPluginType(name)); + } + }; + config.initialize(); + + final Appender noInterpolation = config.getAppender("${console.name}"); + assertThat(noInterpolation).as("No interpolation for '${console.name}'").isNull(); + final Appender console = config.getAppender("CONSOLE"); + assertThat(console).isInstanceOf(ConsoleAppender.class); + } +} diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java index d95202be2a..0345358796 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java @@ -647,18 +647,21 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement processConditionals(rootNode); preConfigure(rootNode); configurationScheduler.start(); - if (rootNode.hasChildren() && rootNode.getChildren().get(0).getName().equalsIgnoreCase("Properties")) { - final Node first = rootNode.getChildren().get(0); - createConfiguration(first, null); - if (first.getObject() != null) { - final StrLookup lookup = (StrLookup) first.getObject(); - if (lookup instanceof LoggerContextAware) { - ((LoggerContextAware) lookup).setLoggerContext(loggerContext.get()); + // Find the "Properties" node first + boolean hasProperties = false; + for (final Node node : rootNode.getChildren()) { + if ("Properties".equalsIgnoreCase(node.getName())) { + hasProperties = true; + createConfiguration(node, null); + if (node.getObject() != null) { + final StrLookup lookup = node.getObject(); + runtimeStrSubstitutor.setVariableResolver(lookup); + configurationStrSubstitutor.setVariableResolver(lookup); } - runtimeStrSubstitutor.setVariableResolver(lookup); - configurationStrSubstitutor.setVariableResolver(lookup); + break; } - } else { + } + if (!hasProperties) { final Map<String, String> map = this.getComponent(CONTEXT_PROPERTIES); final StrLookup lookup = map == null ? null : new PropertiesLookup(map); final Interpolator interpolator = new Interpolator(lookup, pluginPackages); @@ -670,7 +673,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement boolean setLoggers = false; boolean setRoot = false; for (final Node child : rootNode.getChildren()) { - if (child.getName().equalsIgnoreCase("Properties")) { + if ("Properties".equalsIgnoreCase(child.getName())) { if (tempLookup == runtimeStrSubstitutor.getVariableResolver()) { LOGGER.error("Properties declaration must be the first element in the configuration"); } @@ -680,7 +683,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement if (child.getObject() == null) { continue; } - if (child.getName().equalsIgnoreCase("Scripts")) { + if ("Scripts".equalsIgnoreCase(child.getName())) { for (final AbstractScript script : child.getObject(AbstractScript[].class)) { if (script instanceof ScriptRef) { LOGGER.error( @@ -690,11 +693,11 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement scriptManager.addScript(script); } } - } else if (child.getName().equalsIgnoreCase("Appenders")) { + } else if ("Appenders".equalsIgnoreCase(child.getName())) { appenders = child.getObject(); } else if (child.isInstanceOf(Filter.class)) { addFilter(child.getObject(Filter.class)); - } else if (child.getName().equalsIgnoreCase("Loggers")) { + } else if (child.isInstanceOf(Loggers.class)) { final Loggers l = child.getObject(); loggerConfigs = l.getMap(); setLoggers = true; @@ -702,7 +705,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement root = l.getRoot(); setRoot = true; } - } else if (child.getName().equalsIgnoreCase("CustomLevels")) { + } else if (child.isInstanceOf(CustomLevels.class)) { customLevels = child.getObject(CustomLevels.class).getCustomLevels(); } else if (child.isInstanceOf(CustomLevelConfig.class)) { final List<CustomLevelConfig> copy = new ArrayList<>(customLevels); diff --git a/src/changelog/.2.x.x/allow_arbitrary_properties_order.xml b/src/changelog/.2.x.x/allow_arbitrary_properties_order.xml new file mode 100644 index 0000000000..825b147cc3 --- /dev/null +++ b/src/changelog/.2.x.x/allow_arbitrary_properties_order.xml @@ -0,0 +1,9 @@ +<?xml version="1.0" encoding="UTF-8"?> +<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xmlns="http://logging.apache.org/log4j/changelog"; + xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.3.xsd"; + type="fixed"> + <description format="asciidoc"> + Allow the <Properties> node to appear in any position in the configuration element. + </description> +</entry>
