The core.Logger#setLevel method should work like (#2282)

* The core.Logger#setLevel method should work like Configurator#setLevel #2281 * Fix `Logger#setLevel` to work as in Log4j 1.2 --------- Co-authored-by: Piotr P. Karwasz <[email protected]>
apache · Feb 14, 2024 · 0ecf6e1 · 0ecf6e1
1 parent 6a3f8cd
commit 0ecf6e1
Show file tree

Hide file tree

Showing 7 changed files with 115 additions and 39 deletions.
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/Category.java b/log4j-1.2-api/src/main/java/org/apache/log4j/Category.java
@@ -29,6 +29,7 @@
 import org.apache.log4j.bridge.LogEventWrapper;
 import org.apache.log4j.helpers.AppenderAttachableImpl;
 import org.apache.log4j.helpers.NullEnumeration;
+import org.apache.log4j.helpers.OptionConverter;
 import org.apache.log4j.legacy.core.CategoryUtil;
 import org.apache.log4j.spi.AppenderAttachable;
 import org.apache.log4j.spi.HierarchyEventListener;
@@ -384,25 +385,7 @@ public Priority getChainedPriority() {
     }
 
     public Level getEffectiveLevel() {
-        switch (logger.getLevel().getStandardLevel()) {
-            case ALL:
-                return Level.ALL;
-            case TRACE:
-                return Level.TRACE;
-            case DEBUG:
-                return Level.DEBUG;
-            case INFO:
-                return Level.INFO;
-            case WARN:
-                return Level.WARN;
-            case ERROR:
-                return Level.ERROR;
-            case FATAL:
-                return Level.FATAL;
-            default:
-                // TODO Should this be an IllegalStateException?
-                return Level.OFF;
-        }
+        return OptionConverter.convertLevel(logger.getLevel());
     }
 
     /**
@@ -417,7 +400,8 @@ public LoggerRepository getHierarchy() {
     }
 
     public final Level getLevel() {
-        return getEffectiveLevel();
+        final org.apache.logging.log4j.Level v2Level = CategoryUtil.getExplicitLevel(logger);
+        return v2Level != null ? OptionConverter.convertLevel(v2Level) : null;
     }
 
     private String getLevelStr(final Priority priority) {
@@ -460,7 +444,7 @@ public final Category getParent() {
     }
 
     public final Level getPriority() {
-        return getEffectiveLevel();
+        return getLevel();
     }
 
     public ResourceBundle getResourceBundle() {

diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/helpers/OptionConverter.java b/log4j-1.2-api/src/main/java/org/apache/log4j/helpers/OptionConverter.java
@@ -630,16 +630,36 @@ public static Level toLevel(final String clazz, final String levelName, final Le
             return null;
         }
 
-        LOGGER.debug("toLevel" + ":class=[" + clazz + "]" + ":pri=[" + levelName + "]");
+        LOGGER.debug("toLevel:class=[{}]:pri=[{}]", clazz, levelName);
 
         // Support for levels defined in Log4j2.
         if (LOG4J2_LEVEL_CLASS.equals(clazz)) {
             final org.apache.logging.log4j.Level v2Level =
                     org.apache.logging.log4j.Level.getLevel(toRootUpperCase(levelName));
             if (v2Level != null) {
-                return new LevelWrapper(v2Level);
+                switch (v2Level.name()) {
+                    case "ALL":
+                        return Level.ALL;
+                    case "DEBUG":
+                        return Level.DEBUG;
+                    case "ERROR":
+                        return Level.ERROR;
+                    case "FATAL":
+                        return Level.FATAL;
+                    case "INFO":
+                        return Level.INFO;
+                    case "OFF":
+                        return Level.OFF;
+                    case "WARN":
+                        return Level.WARN;
+                    case "TRACE":
+                        return Level.TRACE;
+                    default:
+                        return new LevelWrapper(v2Level);
+                }
+            } else {
+                return defaultValue;
             }
-            return defaultValue;
         }
         try {
             final Class<?> customLevel = LoaderUtil.loadClass(clazz);

diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/legacy/core/CategoryUtil.java b/log4j-1.2-api/src/main/java/org/apache/log4j/legacy/core/CategoryUtil.java
@@ -27,6 +27,7 @@
 import org.apache.logging.log4j.core.Appender;
 import org.apache.logging.log4j.core.Filter;
 import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.config.Configurator;
 import org.apache.logging.log4j.core.config.LoggerConfig;
 import org.apache.logging.log4j.spi.LoggerContext;
 
@@ -127,10 +128,25 @@ public static void setAdditivity(final Logger logger, final boolean additive) {
      */
     public static void setLevel(final Logger logger, final Level level) {
         if (isCore(logger)) {
-            asCore(logger).setLevel(level);
+            Configurator.setLevel(asCore(logger), level);
         }
     }
 
+    /**
+     * Returns the level explicitly set on the logger.
+     * <p>
+     *     If the Log4j API implementation does not support it, returns the effective level instead.
+     * </p>
+     */
+    public static Level getExplicitLevel(final Logger logger) {
+        return isCore(logger) ? getExplicitLevel(asCore(logger)) : logger.getLevel();
+    }
+
+    private static Level getExplicitLevel(final org.apache.logging.log4j.core.Logger logger) {
+        final LoggerConfig config = logger.get();
+        return config.getName().equals(logger.getName()) ? config.getExplicitLevel() : null;
+    }
+
     /**
      * Adds an appender to the logger. This method requires a check for the presence
      * of Log4j Core or it will cause a {@code ClassNotFoundException}.

diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/LoggerTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/LoggerTest.java
@@ -16,6 +16,7 @@
  */
 package org.apache.log4j;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -25,17 +26,17 @@
 
 import java.util.List;
 import java.util.Locale;
+import java.util.Objects;
 import java.util.ResourceBundle;
 import org.apache.logging.log4j.core.Appender;
 import org.apache.logging.log4j.core.LogEvent;
-import org.apache.logging.log4j.core.LoggerContext;
 import org.apache.logging.log4j.core.appender.AbstractAppender;
 import org.apache.logging.log4j.core.config.ConfigurationFactory;
 import org.apache.logging.log4j.core.config.Property;
 import org.apache.logging.log4j.core.layout.PatternLayout;
 import org.apache.logging.log4j.core.test.appender.ListAppender;
-import org.junit.After;
 import org.junit.AfterClass;
+import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -75,9 +76,9 @@ public static void tearDownClass() {
         ConfigurationFactory.removeConfigurationFactory(configurationFactory);
     }
 
-    @After
-    public void tearDown() {
-        LoggerContext.getContext().reconfigure();
+    @Before
+    public void resetTest() {
+        Objects.requireNonNull(LogManager.getHierarchy()).resetConfiguration();
         a1 = null;
         a2 = null;
     }
@@ -493,6 +494,53 @@ public void testLog() {
         }
     }
 
+    @Test
+    public void testSetLevel() {
+        final Logger a = Logger.getLogger("a");
+        final Logger a_b = Logger.getLogger("a.b");
+        final Logger a_b_c = Logger.getLogger("a.b.c");
+        // test default for this test
+        assertThat(a.getLevel()).isNull();
+        assertThat(a_b.getLevel()).isNull();
+        assertThat(a_b_c.getLevel()).isNull();
+        assertThat(a.getEffectiveLevel()).isEqualTo(Level.DEBUG);
+        assertThat(a_b.getEffectiveLevel()).isEqualTo(Level.DEBUG);
+        assertThat(a_b_c.getEffectiveLevel()).isEqualTo(Level.DEBUG);
+        // all
+        for (final Level level :
+                new Level[] {Level.DEBUG, Level.ERROR, Level.FATAL, Level.INFO, Level.TRACE, Level.WARN}) {
+            a.setLevel(level);
+            assertThat(a.getLevel()).isEqualTo(level);
+            assertThat(a_b.getLevel()).isNull();
+            assertThat(a_b.getEffectiveLevel()).isEqualTo(level);
+            assertThat(a_b.getLevel()).isNull();
+            assertThat(a_b_c.getEffectiveLevel()).isEqualTo(level);
+        }
+    }
+
+    @Test
+    public void testSetPriority() {
+        final Logger a = Logger.getLogger("a");
+        final Logger a_b = Logger.getLogger("a.b");
+        final Logger a_b_c = Logger.getLogger("a.b.c");
+        // test default for this test
+        assertThat(a.getPriority()).isNull();
+        assertThat(a_b.getPriority()).isNull();
+        assertThat(a_b_c.getPriority()).isNull();
+        assertThat(a.getEffectiveLevel()).isEqualTo(Level.DEBUG);
+        assertThat(a_b.getEffectiveLevel()).isEqualTo(Level.DEBUG);
+        assertThat(a_b_c.getEffectiveLevel()).isEqualTo(Level.DEBUG);
+        // all
+        for (final Priority level : Level.getAllPossiblePriorities()) {
+            a.setPriority(level);
+            assertThat(a.getPriority()).isEqualTo(level);
+            assertThat(a_b.getPriority()).isNull();
+            assertThat(a_b.getEffectiveLevel()).isEqualTo(level);
+            assertThat(a_b.getPriority()).isNull();
+            assertThat(a_b_c.getEffectiveLevel()).isEqualTo(level);
+        }
+    }
+
     private static class MyLogger {
 
         private final Logger logger;

diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/helpers/OptionConverterLevelTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/helpers/OptionConverterLevelTest.java
@@ -18,9 +18,10 @@
 
 import static org.apache.log4j.helpers.OptionConverter.toLog4j1Level;
 import static org.apache.log4j.helpers.OptionConverter.toLog4j2Level;
-import static org.junit.Assert.assertNull;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 
 import java.util.Arrays;
 import java.util.stream.Stream;
@@ -43,22 +44,18 @@ static Stream<Arguments> standardLevels() {
 
     /**
      * Test if the standard levels are transformed correctly.
-     *
-     * @param log4j1Level
-     * @param log4j2Level
      */
     @ParameterizedTest
     @MethodSource("standardLevels")
     public void testStandardLevelConversion(final Level log4j1Level, final org.apache.logging.log4j.Level log4j2Level) {
-        assertEquals(log4j2Level, OptionConverter.convertLevel(log4j1Level));
-        assertEquals(log4j1Level, OptionConverter.convertLevel(log4j2Level));
+        assertThat(log4j2Level).isSameAs(OptionConverter.convertLevel(log4j1Level));
+        assertThat(log4j1Level).isSameAs(OptionConverter.convertLevel(log4j2Level));
+        assertThat(OptionConverter.toLevel(org.apache.logging.log4j.Level.class.getName(), log4j2Level.name(), null))
+                .isSameAs(OptionConverter.convertLevel(log4j2Level));
     }
 
     /**
      * Test if the conversion works at an integer level.
-     *
-     * @param log4j1Level
-     * @param log4j2Level
      */
     @ParameterizedTest
     @MethodSource("standardLevels")

diff --git a/pom.xml b/pom.xml
@@ -542,6 +542,7 @@
       <plugin>
         <groupId>org.apache.rat</groupId>
         <artifactId>apache-rat-plugin</artifactId>
+        <version>0.16.1</version>
         <configuration>
           <consoleOutput>true</consoleOutput>
           <excludes combine.children="append">

diff --git a/src/changelog/.2.x.x/2282_fix_1_2_set_level.xml b/src/changelog/.2.x.x/2282_fix_1_2_set_level.xml
@@ -0,0 +1,10 @@
+<?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.2.xsd"
+       type="fixed">
+  <issue id="2282" link="https://github.com/apache/logging-log4j2/issues/2282"/>
+  <description format="asciidoc">
+    Fix the behavior of `Logger#setLevel` and `Logger#getLevel` in the Log4j 1.2 bridge.
+  </description>
+</entry>