You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2024/02/09 21:53:14 UTC

(logging-log4j2) branch fix/configuration-properties-order created (now ff5d0be10c)

This is an automated email from the ASF dual-hosted git repository.

pkarwasz pushed a change to branch fix/configuration-properties-order
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git


      at ff5d0be10c Allow arbitrary position of `<Properties>` element

This branch includes the following new commits:

     new ff5d0be10c Allow arbitrary position of `<Properties>` element

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(logging-log4j2) 01/01: Allow arbitrary position of `` element

Posted by pk...@apache.org.
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 <pi...@karwasz.org>
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 &lt;Properties&gt; node to appear in any position in the configuration element.
+  </description>
+</entry>