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 15:04:59 UTC

[logging-log4j2] branch master updated (0fc8421 -> f590109)

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

ggregory pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git.


    from 0fc8421  Add PropertiesLookup#getProperties().
     new 766d8b1  Make the test use a system property in addition to a local property.
     new f590109  Use some symmetry in naming: We already have 'configurationStrSubstitutor', so rename 'subst' to 'runtimeStrSubstitutor'.

The 2 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.


Summary of changes:
 .../builders/appender/FileAppenderBuilder.java     |  1 -
 .../PropertiesRollingWithPropertiesTest.java       | 22 +++++++++++----
 .../resources/log4j1-rolling-properties.properties |  4 +--
 .../log4j/core/config/AbstractConfiguration.java   | 32 ++++++++++------------
 4 files changed, 32 insertions(+), 27 deletions(-)

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

Posted by gg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit f590109706bd5fc0a48b2efc811c3bb2d0f1a6ce
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sun Jan 16 09:49:17 2022 -0500

    Use some symmetry in naming: We already have
    'configurationStrSubstitutor', so rename 'subst' to
    'runtimeStrSubstitutor'.
    
    - No need to initialize to default values.
    - Remove dead whitespace.
    - Remove useless parentheses.
    - Simplify exception handling.
    - No need to nest code in else clause.
---
 .../builders/appender/FileAppenderBuilder.java     |  1 -
 .../log4j/core/config/AbstractConfiguration.java   | 32 ++++++++++------------
 2 files changed, 14 insertions(+), 19 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 8b7f467..a7c4002 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
@@ -130,7 +130,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-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 77bc1ec..677b591 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
@@ -114,7 +114,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
     /**
      * Shutdown timeout in milliseconds.
      */
-    protected long shutdownTimeoutMillis = 0;
+    protected long shutdownTimeoutMillis;
 
     /**
      * The Script manager.
@@ -125,7 +125,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
      * The Advertiser which exposes appender configurations to external systems.
      */
     private Advertiser advertiser = new DefaultAdvertiser();
-    private Node advertiserNode = null;
+    private Node advertiserNode;
     private Object advertisement;
     private String name;
     private ConcurrentMap<String, Appender> appenders = new ConcurrentHashMap<>();
@@ -133,8 +133,8 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
     private List<CustomLevelConfig> customLevels = Collections.emptyList();
     private final ConcurrentMap<String, String> properties = new ConcurrentHashMap<>();
     private final StrLookup tempLookup = new Interpolator(properties);
-    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;
@@ -156,7 +156,6 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
         pluginManager = new PluginManager(Node.CATEGORY);
         rootNode = new Node();
         setState(State.INITIALIZING);
-
     }
 
     @Override
@@ -221,11 +220,11 @@ 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 {
             ServiceLoaderUtil.loadServices(ScriptManagerFactory.class,
-                            (layer) -> ServiceLoader.load(layer, ScriptManagerFactory.class),
+                            layer -> ServiceLoader.load(layer, ScriptManagerFactory.class),
                             null).stream().findFirst().ifPresent(scriptManagerFactory ->
                     scriptManager = scriptManagerFactory.createScriptManager(this, watchManager));
         } catch (final LinkageError | Exception e) {
@@ -490,10 +489,8 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
                 try {
                     advertiser = clazz.newInstance();
                     advertisement = advertiser.advertise(advertiserNode.getAttributes());
-                } catch (final InstantiationException e) {
-                    LOGGER.error("InstantiationException attempting to instantiate advertiser: {}", nodeName, e);
-                } catch (final IllegalAccessException e) {
-                    LOGGER.error("IllegalAccessException attempting to instantiate advertiser: {}", nodeName, e);
+                } catch (final ReflectiveOperationException e) {
+                    LOGGER.error("{} attempting to instantiate advertiser: {}", e.getClass().getSimpleName(), nodeName, e);
                 }
             }
         }
@@ -628,14 +625,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 +640,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;
@@ -804,7 +801,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
 
     @Override
     public StrSubstitutor getStrSubstitutor() {
-        return subst;
+        return runtimeStrSubstitutor;
     }
 
     @Override
@@ -1051,10 +1048,9 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
     public Object createPluginObject(final PluginType<?> type, final Node node) {
         if (this.getState().equals(State.INITIALIZING)) {
             return createPluginObject(type, node, null);
-        } else {
-            LOGGER.warn("Plugin Object creation is not allowed after initialization");
-            return null;
         }
+        LOGGER.warn("Plugin Object creation is not allowed after initialization");
+        return null;
     }
 
     /**

[logging-log4j2] 01/02: Make the test use a system property in addition to a local property.

Posted by gg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 766d8b19153c5d1ef23f371d46b6d7f971bcee74
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sat Jan 15 18:51:32 2022 -0500

    Make the test use a system property in addition to a local property.
---
 .../PropertiesRollingWithPropertiesTest.java       | 22 ++++++++++++++++------
 .../resources/log4j1-rolling-properties.properties |  4 ++--
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesRollingWithPropertiesTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesRollingWithPropertiesTest.java
index 87326cf..c149ab9 100644
--- a/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesRollingWithPropertiesTest.java
+++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/PropertiesRollingWithPropertiesTest.java
@@ -18,13 +18,16 @@ package org.apache.log4j.config;
 
 import static org.junit.Assert.assertTrue;
 
-import java.io.File;
+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.core.test.SystemPropertyTestRule;
 import org.junit.ClassRule;
 import org.junit.Test;
+import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 
 /**
@@ -32,16 +35,23 @@ import org.junit.rules.TestRule;
  */
 public class PropertiesRollingWithPropertiesTest {
 
+    private static final String TEST_DIR = "target/" + PropertiesRollingWithPropertiesTest.class.getSimpleName();
+
     @ClassRule
-    public static TestRule SP_RULE = SystemPropertyTestRule.create("log4j.configuration", "target/test-classes/log4j1-rolling-properties.properties");
+    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.properties"));
+    //@formatter:on
 
     @Test
     public void testProperties() throws Exception {
-        Logger logger = LogManager.getLogger("test");
+        final Logger logger = LogManager.getLogger("test");
         logger.debug("This is a test of the root logger");
-        File file = new File("target/rolling/somefile.log");
-        assertTrue("Log file was not created", file.exists());
-        assertTrue("Log file is empty", file.length() > 0);
+        final Path path = Paths.get(TEST_DIR, "somefile.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.properties b/log4j-1.2-api/src/test/resources/log4j1-rolling-properties.properties
index 4ce0e43..41d8453 100644
--- a/log4j-1.2-api/src/test/resources/log4j1-rolling-properties.properties
+++ b/log4j-1.2-api/src/test/resources/log4j1-rolling-properties.properties
@@ -16,7 +16,7 @@
 log4j.debug=true
 
 # Properties for substitution
-somelogfile=target/rolling/somefile.log
+somelogfile=somefile.log
 maxfilesize=256MB
 maxbackupindex=20
 
@@ -24,7 +24,7 @@ log4j.rootLogger=TRACE, RFA
 
 # Appender configuration with variables
 log4j.appender.RFA=org.apache.log4j.RollingFileAppender
-log4j.appender.RFA.File=${somelogfile}
+log4j.appender.RFA.File=${test.directory}/${somelogfile}
 log4j.appender.RFA.MaxFileSize=${maxfilesize}
 log4j.appender.RFA.MaxBackupIndex=${maxbackupindex}
 log4j.appender.RFA.layout=org.apache.log4j.PatternLayout