Emulate effects of j.u.l.Logger.setLevel (#3125)

ppkarwasz · web-flow · commit 52dbb47c25ca · 2024-11-21T11:14:56.000+01:00
Some libraries rely on `j.u.l.Logger.getLevel` returning the value that was set using `j.u.l.Logger.setLevel`. This PR changes #2353: - By default, the **effective** configuration of the logging backend is not modified by `log4j-jul` (same assumption as #2353). - All implementations of `j.u.l.Logger` mutator methods (`setLevel`, `setUseParentHandlers`, `addHandler`, …), must guarantee a change in the corresponding getter (`getLevel`, `getUseParentHandlers`, `getHandlers`, …). - Adds warnings in `ApiLogger` to inform users that calling those methods has no other side effect that change the result of the corresponding getter.
diff --git a/log4j-jul/src/main/java/org/apache/logging/log4j/jul/ApiLogger.java b/log4j-jul/src/main/java/org/apache/logging/log4j/jul/ApiLogger.java
@@ -16,7 +16,9 @@
  */
 package org.apache.logging.log4j.jul;
 
+import java.util.ResourceBundle;
 import java.util.logging.Filter;
+import java.util.logging.Handler;
 import java.util.logging.Level;
 import java.util.logging.LogRecord;
 import java.util.logging.Logger;
@@ -41,6 +43,12 @@
  */
 public class ApiLogger extends Logger {
 
+    private static final org.apache.logging.log4j.Logger LOGGER = StatusLogger.getLogger();
+    private static final String MUTATOR_DISABLED =
+            "Ignoring call to `j.u.l.Logger.{}({})`, since the Log4j API does not provide methods to modify the underlying implementation.\n"
+                    + "To modify the configuration using JUL, use an `AbstractLoggerAdapter` appropriate for your logging implementation.\n"
+                    + "See https://logging.apache.org/log4j/3.x/log4j-jul.html#log4j.jul.loggerAdapter for more information.";
+
     private final WrappedLogger logger;
     private static final String FQCN = ApiLogger.class.getName();
 
@@ -82,29 +90,65 @@ public String getName() {
 
     @Override
     public Level getLevel() {
-        // Returns the effective level instead of the configured one.
-        // The configured level is not accessible through Log4j API.
-        return LevelTranslator.toJavaLevel(logger.getLevel());
+        // The configured level is NOT available through the Log4j API.
+        // Some libraries, however, rely on the following assertion:
+        //
+        // logger.setLevel(level);
+        // assert level.equals(logger.getLevel());
+        //
+        // See https://github.com/apache/logging-log4j2/issues/3119 for more details.
+        return super.getLevel();
     }
 
     @Override
     public void setLevel(final Level newLevel) throws SecurityException {
-        StatusLogger.getLogger()
-                .error(
-                        "Cannot set JUL log level through log4j-api: " + "ignoring call to Logger.setLevel({})",
-                        newLevel);
+        LOGGER.warn(MUTATOR_DISABLED, "setLevel", newLevel);
+        // Some libraries rely on the following assertion:
+        //
+        // logger.setLevel(level);
+        // assert level.equals(logger.getLevel());
+        //
+        // See https://github.com/apache/logging-log4j2/issues/3119 for more details.
+        doSetLevel(newLevel);
     }
 
     /**
-     * Provides access to {@link Logger#setLevel(java.util.logging.Level)}. This method should only be used by child
-     * classes.
-     *
+     * Provides access to {@link Logger#setLevel(java.util.logging.Level)}.
+     * <p>
+     *   This method should be called by all {@link #setLevel} implementations to check permissions.
+     * </p>
      * @see Logger#setLevel(java.util.logging.Level)
      */
     protected void doSetLevel(final Level newLevel) throws SecurityException {
         super.setLevel(newLevel);
     }
 
+    @Override
+    public void setUseParentHandlers(boolean useParentHandlers) {
+        LOGGER.warn(MUTATOR_DISABLED, "setLevel", useParentHandlers);
+        super.setUseParentHandlers(useParentHandlers);
+    }
+
+    @Override
+    public void addHandler(Handler handler) throws SecurityException {
+        LOGGER.warn(MUTATOR_DISABLED, "addHandler", handler);
+        super.addHandler(handler);
+    }
+
+    @Override
+    public void removeHandler(Handler handler) throws SecurityException {
+        LOGGER.warn(MUTATOR_DISABLED, "removeHandler", handler);
+        super.removeHandler(handler);
+    }
+
+    @Override
+    public void setResourceBundle(ResourceBundle bundle) {
+        LOGGER.warn(
+                "Ignoring call to `j.u.l.Logger.setResourceBundle({})`, since `o.a.l.l.jul.LogManager` currently does not support resource bundles.",
+                bundle);
+        super.setResourceBundle(bundle);
+    }
+
     /**
      * Unsupported operation.
      *
diff --git a/log4j-jul/src/test/java/org/apache/logging/log4j/jul/test/AbstractLoggerTest.java b/log4j-jul/src/test/java/org/apache/logging/log4j/jul/test/AbstractLoggerTest.java
@@ -19,6 +19,9 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.util.List;
+import java.util.ResourceBundle;
+import java.util.logging.ConsoleHandler;
+import java.util.logging.Filter;
 import java.util.logging.Logger;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.core.LogEvent;
@@ -33,18 +36,40 @@
  */
 public abstract class AbstractLoggerTest {
     public static final String LOGGER_NAME = "Test";
+
+    static final java.util.logging.Level[] LEVELS = new java.util.logging.Level[] {
+        java.util.logging.Level.ALL,
+        java.util.logging.Level.FINEST,
+        java.util.logging.Level.FINER,
+        java.util.logging.Level.FINE,
+        java.util.logging.Level.CONFIG,
+        java.util.logging.Level.INFO,
+        java.util.logging.Level.WARNING,
+        java.util.logging.Level.SEVERE,
+        java.util.logging.Level.OFF
+    };
+
+    static java.util.logging.Level getEffectiveLevel(final Logger logger) {
+        for (final java.util.logging.Level level : LEVELS) {
+            if (logger.isLoggable(level)) {
+                return level;
+            }
+        }
+        throw new RuntimeException("No level is enabled.");
+    }
+
     protected Logger logger;
     protected ListAppender eventAppender;
     protected ListAppender flowAppender;
     protected ListAppender stringAppender;
 
     @Test
-    public void testGetName() throws Exception {
+    public void testGetName() {
         assertThat(logger.getName()).isEqualTo(LOGGER_NAME);
     }
 
     @Test
-    public void testGlobalLogger() throws Exception {
+    public void testGlobalLogger() {
         final Logger root = Logger.getLogger(Logger.GLOBAL_LOGGER_NAME);
         root.info("Test info message");
         root.config("Test info message");
@@ -58,18 +83,18 @@ public void testGlobalLogger() throws Exception {
     }
 
     @Test
-    public void testGlobalLoggerName() throws Exception {
+    public void testGlobalLoggerName() {
         final Logger root = Logger.getLogger(Logger.GLOBAL_LOGGER_NAME);
         assertThat(root.getName()).isEqualTo(Logger.GLOBAL_LOGGER_NAME);
     }
 
     @Test
-    public void testIsLoggable() throws Exception {
+    public void testIsLoggable() {
         assertThat(logger.isLoggable(java.util.logging.Level.SEVERE)).isTrue();
     }
 
     @Test
-    public void testLog() throws Exception {
+    public void testLog() {
         logger.info("Informative message here.");
         final List<LogEvent> events = eventAppender.getEvents();
         assertThat(events).hasSize(1);
@@ -82,7 +107,7 @@ public void testLog() throws Exception {
     }
 
     @Test
-    public void testLogFilter() throws Exception {
+    public void testLogFilter() {
         logger.setFilter(record -> false);
         logger.severe("Informative message here.");
         logger.warning("Informative message here.");
@@ -96,7 +121,7 @@ public void testLogFilter() throws Exception {
     }
 
     @Test
-    public void testAlteringLogFilter() throws Exception {
+    public void testAlteringLogFilter() {
         logger.setFilter(record -> {
             record.setMessage("This is not the message you are looking for.");
             return true;
@@ -121,7 +146,7 @@ public void testLogParamMarkers() {
     }
 
     @Test
-    public void testLogUsingCustomLevel() throws Exception {
+    public void testLogUsingCustomLevel() {
         logger.config("Config level");
         final List<LogEvent> events = eventAppender.getEvents();
         assertThat(events).hasSize(1);
@@ -130,7 +155,7 @@ public void testLogUsingCustomLevel() throws Exception {
     }
 
     @Test
-    public void testLogWithCallingClass() throws Exception {
+    public void testLogWithCallingClass() {
         final Logger log = Logger.getLogger("Test.CallerClass");
         log.config("Calling from LoggerTest");
         final List<String> messages = stringAppender.getMessages();
@@ -201,6 +226,84 @@ public void testLambdasPercentAndCurlyBraces() {
         testLambdaMessages("message{%s}");
     }
 
+    /**
+     * This assertion must be true, even if {@code setLevel} has no effect on the logging implementation.
+     *
+     * @see <a href="https://github.com/apache/logging-log4j2/issues/3119">GH issue #3119</a>
+     */
+    @Test
+    public void testSetAndGetLevel() {
+        final Logger logger = Logger.getLogger(AbstractLoggerTest.class.getName() + ".testSetAndGetLevel");
+        // The logger under test should have no explicit configuration
+        assertThat(logger.getLevel()).isNull();
+
+        for (java.util.logging.Level level : LEVELS) {
+            logger.setLevel(level);
+            assertThat(logger.getLevel()).as("Level set using `setLevel()`").isEqualTo(level);
+        }
+    }
+
+    /**
+     * The value of `useParentHandlers` should be recorded even if it is not effective.
+     */
+    @Test
+    public void testSetUseParentHandlers() {
+        final Logger logger = Logger.getLogger(AbstractLoggerTest.class.getName() + ".testSetUseParentHandlers");
+
+        for (boolean useParentHandlers : new boolean[] {false, true}) {
+            logger.setUseParentHandlers(useParentHandlers);
+            assertThat(logger.getUseParentHandlers()).isEqualTo(useParentHandlers);
+        }
+    }
+
+    /**
+     * The programmatically configured handlers should be recorded, even if they are not used.
+     */
+    @Test
+    public void testAddAndRemoveHandlers() {
+        final Logger logger = Logger.getLogger(AbstractLoggerTest.class.getName() + ".testAddAndRemoveHandlers");
+
+        assertThat(logger.getHandlers()).isEmpty();
+        // Add a handler
+        ConsoleHandler handler = new ConsoleHandler();
+        logger.addHandler(handler);
+        assertThat(logger.getHandlers()).hasSize(1).containsExactly(handler);
+        // Remove handler
+        logger.removeHandler(handler);
+        assertThat(logger.getHandlers()).isEmpty();
+    }
+
+    /**
+     * The programmatically configured filters should be recorded, even if they are not used.
+     */
+    @Test
+    public void testSetFilter() {
+        final Logger logger = Logger.getLogger(AbstractLoggerTest.class.getName() + ".testSetFilter");
+
+        assertThat(logger.getFilter()).isNull();
+        // Set filter
+        Filter denyAllFilter = record -> false;
+        logger.setFilter(denyAllFilter);
+        assertThat(logger.getFilter()).isEqualTo(denyAllFilter);
+        // Remove filter
+        logger.setFilter(null);
+        assertThat(logger.getFilter()).isNull();
+    }
+
+    /**
+     * The programmatically configured resource bundles should be recorded, even if they are not used.
+     */
+    @Test
+    public void testSetResourceBundle() {
+        final Logger logger = Logger.getLogger(AbstractLoggerTest.class.getName() + ".testSetResourceBundle");
+
+        assertThat(logger.getResourceBundle()).isNull();
+        // Set resource bundle
+        ResourceBundle bundle = ResourceBundle.getBundle("testResourceBundle");
+        logger.setResourceBundle(bundle);
+        assertThat(logger.getResourceBundle()).isSameAs(bundle);
+    }
+
     private void testLambdaMessages(final String string) {
         final Logger root = Logger.getLogger(Logger.GLOBAL_LOGGER_NAME);
         root.info(() -> "Test info " + string);
diff --git a/log4j-jul/src/test/java/org/apache/logging/log4j/jul/test/ApiLoggerTest.java b/log4j-jul/src/test/java/org/apache/logging/log4j/jul/test/ApiLoggerTest.java
@@ -16,8 +16,7 @@
  */
 package org.apache.logging.log4j.jul.test;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.equalTo;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
@@ -46,7 +45,7 @@ public static void tearDownClass() {
     public void setUp() {
         logger = Logger.getLogger(LOGGER_NAME);
         logger.setFilter(null);
-        assertThat(logger.getLevel(), equalTo(java.util.logging.Level.FINE));
+        assertThat(getEffectiveLevel(logger)).isEqualTo(java.util.logging.Level.FINE);
         eventAppender = ListAppender.getListAppender("TestAppender");
         flowAppender = ListAppender.getListAppender("FlowAppender");
         stringAppender = ListAppender.getListAppender("StringAppender");
diff --git a/log4j-jul/src/test/java/org/apache/logging/log4j/jul/test/CoreLoggerTest.java b/log4j-jul/src/test/java/org/apache/logging/log4j/jul/test/CoreLoggerTest.java
@@ -34,27 +34,6 @@
 
 public class CoreLoggerTest extends AbstractLoggerTest {
 
-    private static final Level[] LEVELS = new Level[] {
-        Level.ALL,
-        Level.FINEST,
-        Level.FINER,
-        Level.FINE,
-        Level.CONFIG,
-        Level.INFO,
-        Level.WARNING,
-        Level.SEVERE,
-        Level.OFF
-    };
-
-    private static Level getEffectiveLevel(final Logger logger) {
-        for (final Level level : LEVELS) {
-            if (logger.isLoggable(level)) {
-                return level;
-            }
-        }
-        throw new RuntimeException("No level is enabled.");
-    }
-
     @BeforeClass
     public static void setUpClass() {
         System.setProperty("java.util.logging.manager", LogManager.class.getName());
diff --git a/log4j-jul/src/test/resources/testResourceBundle.properties b/log4j-jul/src/test/resources/testResourceBundle.properties
@@ -0,0 +1,18 @@
+#
+# 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.
+#
+###
+# An empty resource bundle for tests
diff --git a/src/changelog/.2.x.x/3119_set_level_call_parent.xml b/src/changelog/.2.x.x/3119_set_level_call_parent.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns="https://logging.apache.org/xml/ns"
+       xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="fixed">
+  <issue id="3119" link="https://github.com/apache/logging-log4j2/issues/3119"/>
+  <description format="asciidoc">
+    Ensures synchronization between `j.u.l.Logger.getLevel()` and `j.u.l.Logger.setLevel()` methods.
+  </description>
+</entry>
