You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by gg...@apache.org on 2022/01/16 14:43:39 UTC

[logging-log4j2] 01/02: Use some symmetry in naming: We already have 'configurationStrSubstitutor', so rename 'subst' to 'runtimeStrSubstitutor'.

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

ggregory pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit 327089b7d3b78449b458b244d5be0d228598809f
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sun Jan 16 09:42:02 2022 -0500

    Use some symmetry in naming: We already have
    'configurationStrSubstitutor', so rename 'subst' to
    'runtimeStrSubstitutor'.
---
 .../builders/appender/FileAppenderBuilder.java     |  1 -
 .../apache/log4j/config/Log4j1Configuration.java   | 32 ++++++++++--
 .../log4j/config/XmlRollingWithPropertiesTest.java | 57 ++++++++++++++++++++++
 .../test/resources/log4j1-rolling-properties.xml   | 31 ++++++++++++
 .../log4j/core/config/AbstractConfiguration.java   | 14 +++---
 5 files changed, 123 insertions(+), 12 deletions(-)

diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java
index 8682d35..3390762 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java
@@ -131,7 +131,6 @@ public class FileAppenderBuilder extends AbstractBuilder implements AppenderBuil
                 immediateFlush.get(), append.get(), bufferedIo.get(), bufferSize.get());
     }
 
-
     @Override
     public Appender parseAppender(final String name, final String appenderPrefix, final String layoutPrefix,
             final String filterPrefix, final Properties props, final PropertiesConfiguration configuration) {
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1Configuration.java b/log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1Configuration.java
index ae733a1..cb36425 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1Configuration.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/config/Log4j1Configuration.java
@@ -16,15 +16,23 @@
  */
 package org.apache.log4j.config;
 
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+
 import org.apache.log4j.builders.BuilderManager;
 import org.apache.logging.log4j.core.LoggerContext;
 import org.apache.logging.log4j.core.config.AbstractConfiguration;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.ConfigurationSource;
 import org.apache.logging.log4j.core.config.Reconfigurable;
+import org.apache.logging.log4j.core.lookup.Interpolator;
+import org.apache.logging.log4j.core.lookup.PropertiesLookup;
+import org.apache.logging.log4j.core.lookup.StrLookup;
+import org.apache.logging.log4j.core.lookup.StrSubstitutor;
 
 /**
- * Class Description goes here.
+ * Log4j 1 Configuration.
  */
 public class Log4j1Configuration extends AbstractConfiguration implements Reconfigurable {
 
@@ -38,8 +46,7 @@ public class Log4j1Configuration extends AbstractConfiguration implements Reconf
 
     protected final BuilderManager manager = new BuilderManager();
 
-    public Log4j1Configuration(final LoggerContext loggerContext, final ConfigurationSource configurationSource,
-            int monitorIntervalSeconds) {
+    public Log4j1Configuration(final LoggerContext loggerContext, final ConfigurationSource configurationSource, final int monitorIntervalSeconds) {
         super(loggerContext, configurationSource);
         initializeWatchers(this, configurationSource, monitorIntervalSeconds);
     }
@@ -54,7 +61,24 @@ public class Log4j1Configuration extends AbstractConfiguration implements Reconf
     @Override
     public void initialize() {
         getStrSubstitutor().setConfiguration(this);
-        getConfigurationStrSubstitutor().setConfiguration(this);
+        final StrSubstitutor strSubstitutor = getConfigurationStrSubstitutor();
+        strSubstitutor.setConfiguration(this);
+        // Start: Load system properties as default properties.
+        final StrLookup variableResolver = strSubstitutor.getVariableResolver();
+        if (variableResolver instanceof Interpolator) {
+//            final Interpolator interpolator = (Interpolator) variableResolver;
+//            final StrLookup defaultLookup = interpolator.getDefaultLookup();
+//            if (defaultLookup instanceof PropertiesLookup) {
+//                final Map<String, String> properties = ((PropertiesLookup) defaultLookup).getProperties();
+//                final Properties systemProperties = System.getProperties();
+//                // Must lock to avoid edits from other threads
+//                synchronized (systemProperties) {
+//                    // Merge, don't replace!
+//                    systemProperties.forEach((k, v) -> properties.merge(Objects.toString(k), Objects.toString(v), (o, n) -> o));
+//                }
+//            }
+        }
+        // End: Load system properties as default properties.
         super.getScheduler().start();
         doConfigure();
         setState(State.INITIALIZED);
diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlRollingWithPropertiesTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlRollingWithPropertiesTest.java
new file mode 100644
index 0000000..d846f8d
--- /dev/null
+++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlRollingWithPropertiesTest.java
@@ -0,0 +1,57 @@
+/*
+ * 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.log4j.config;
+
+import static org.junit.Assert.assertTrue;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+import org.apache.logging.log4j.test.SystemPropertyTestRule;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.rules.RuleChain;
+import org.junit.rules.TestRule;
+
+/**
+ * Test configuration from Properties.
+ */
+public class XmlRollingWithPropertiesTest {
+
+    private static final String TEST_DIR = "target/" + XmlRollingWithPropertiesTest.class.getSimpleName();
+
+    @ClassRule
+    public static TestRule SP_RULE = RuleChain.emptyRuleChain()
+    //@formatter:off
+        .around(SystemPropertyTestRule.create("test.directory", TEST_DIR))
+        .around(SystemPropertyTestRule.create("log4j.configuration", "target/test-classes/log4j1-rolling-properties.xml"));
+    //@formatter:on
+
+    @Test
+    public void testProperties() throws Exception {
+        // ${test.directory}/logs/etl.log
+        Logger logger = LogManager.getLogger("test");
+        logger.debug("This is a test of the root logger");
+        Path path = Paths.get(TEST_DIR, "logs/etl.log");
+        assertTrue("Log file was not created", Files.exists(path));
+        assertTrue("Log file is empty", Files.size(path) > 0);
+    }
+
+}
diff --git a/log4j-1.2-api/src/test/resources/log4j1-rolling-properties.xml b/log4j-1.2-api/src/test/resources/log4j1-rolling-properties.xml
new file mode 100644
index 0000000..c138a25
--- /dev/null
+++ b/log4j-1.2-api/src/test/resources/log4j1-rolling-properties.xml
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!--
+  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.
+-->
+<!-- See https://issues.apache.org/jira/browse/LOG4J2-3328 -->
+<log4j:configuration>
+  <appender name="DAILY" class="org.apache.log4j.RollingFileAppender">
+    <param name="File" value="${sys:test.directory}/logs/etl.log" />
+    <param name="DatePattern" value="'.'yyyy-MM-dd" />
+    <layout class="org.apache.log4j.PatternLayout">
+      <param name="ConversionPattern" value="GATEWAY: %p %d [%t] %c{1}.%M(%L) | %m%n" />
+    </layout>
+  </appender>
+  <root>
+    <priority value="OFF" />
+    <appender-ref ref="DAILY" />
+  </root>
+</log4j:configuration>
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 d981768..591447e 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
@@ -131,8 +131,8 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
     private List<CustomLevelConfig> customLevels = Collections.emptyList();
     private final ConcurrentMap<String, String> propertyMap = new ConcurrentHashMap<>();
     private final StrLookup tempLookup = new Interpolator(propertyMap);
-    private final StrSubstitutor subst = new RuntimeStrSubstitutor(tempLookup);
-    private final StrSubstitutor configurationStrSubstitutor = new ConfigurationStrSubstitutor(subst);
+    private final StrSubstitutor runtimeStrSubstitutor = new RuntimeStrSubstitutor(tempLookup);
+    private final StrSubstitutor configurationStrSubstitutor = new ConfigurationStrSubstitutor(runtimeStrSubstitutor);
     private LoggerConfig root = new LoggerConfig();
     private final ConcurrentMap<String, Object> componentMap = new ConcurrentHashMap<>();
     private final ConfigurationSource configurationSource;
@@ -219,7 +219,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
     @Override
     public void initialize() {
         LOGGER.debug(Version.getProductString() + " initializing configuration {}", this);
-        subst.setConfiguration(this);
+        runtimeStrSubstitutor.setConfiguration(this);
         configurationStrSubstitutor.setConfiguration(this);
         try {
             scriptManager = new ScriptManager(this, watchManager);
@@ -628,14 +628,14 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
             createConfiguration(first, null);
             if (first.getObject() != null) {
                 StrLookup lookup = (StrLookup) first.getObject();
-                subst.setVariableResolver(lookup);
+                runtimeStrSubstitutor.setVariableResolver(lookup);
                 configurationStrSubstitutor.setVariableResolver(lookup);
             }
         } else {
             final Map<String, String> map = this.getComponent(CONTEXT_PROPERTIES);
             final StrLookup lookup = map == null ? null : new PropertiesLookup(map);
             Interpolator interpolator = new Interpolator(lookup, pluginPackages);
-            subst.setVariableResolver(interpolator);
+            runtimeStrSubstitutor.setVariableResolver(interpolator);
             configurationStrSubstitutor.setVariableResolver(interpolator);
         }
 
@@ -643,7 +643,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
         boolean setRoot = false;
         for (final Node child : rootNode.getChildren()) {
             if (child.getName().equalsIgnoreCase("Properties")) {
-                if (tempLookup == subst.getVariableResolver()) {
+                if (tempLookup == runtimeStrSubstitutor.getVariableResolver()) {
                     LOGGER.error("Properties declaration must be the first element in the configuration");
                 }
                 continue;
@@ -809,7 +809,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
 
     @Override
     public StrSubstitutor getStrSubstitutor() {
-        return subst;
+        return runtimeStrSubstitutor;
     }
 
     @Override