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 index d9718194869..861eaeff2fe 100644 --- 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)}. + *

+ * This method should be called by all {@link #setLevel} implementations to check permissions. + *

* @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 index 75c4888f522..9ddf62adfa0 100644 --- 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 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 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 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 GH issue #3119 + */ + @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 index 431c6682004..6319e52292f 100644 --- 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 index 8880119ef9a..e6695c6fe02 100644 --- 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 new file mode 100644 index 00000000000..e8646874106 --- /dev/null +++ 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 new file mode 100644 index 00000000000..a1924d24462 --- /dev/null +++ b/src/changelog/.2.x.x/3119_set_level_call_parent.xml @@ -0,0 +1,10 @@ + + + + + Ensures synchronization between `j.u.l.Logger.getLevel()` and `j.u.l.Logger.setLevel()` methods. + +