Repository: logging-log4j2 Updated Branches: refs/heads/master 19d74eaff -> 7fd641a06
[LOG4J2-1180] Logger cache does not account for message factory. Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/7fd641a0 Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/7fd641a0 Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/7fd641a0 Branch: refs/heads/master Commit: 7fd641a06550eed1f0b86f84a09d4124a08386d7 Parents: 19d74ea Author: ggregory <ggreg...@apache.org> Authored: Sun Nov 1 19:14:17 2015 -0800 Committer: ggregory <ggreg...@apache.org> Committed: Sun Nov 1 19:14:17 2015 -0800 ---------------------------------------------------------------------- .../log4j/simple/SimpleLoggerContext.java | 31 ++++++++++++--- .../apache/logging/log4j/spi/LoggerContext.java | 16 ++++++++ .../logging/log4j/spi/LoggerContextKey.java | 40 ++++++++++++++++++++ .../apache/logging/log4j/TestLoggerContext.java | 14 ++++++- .../logging/log4j/core/LoggerContext.java | 36 ++++++++++++++++-- .../apache/logging/log4j/core/LoggerTest.java | 16 ++++++-- .../log4j/taglib/Log4jTaglibLoggerContext.java | 15 +++++++- .../logging/slf4j/SLF4JLoggerContext.java | 15 +++++++- src/changes/changes.xml | 3 ++ 9 files changed, 169 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/7fd641a0/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java ---------------------------------------------------------------------- diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java b/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java index 6839290..d11d73f 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java @@ -27,6 +27,7 @@ import org.apache.logging.log4j.message.MessageFactory; import org.apache.logging.log4j.spi.AbstractLogger; import org.apache.logging.log4j.spi.ExtendedLogger; import org.apache.logging.log4j.spi.LoggerContext; +import org.apache.logging.log4j.spi.LoggerContextKey; import org.apache.logging.log4j.util.PropertiesUtil; /** @@ -44,8 +45,9 @@ public class SimpleLoggerContext implements LoggerContext { /** Include the instance name in the log message? */ private final boolean showLogName; + /** - * Include the short name ( last component ) of the logger in the log message. Defaults to true - otherwise we'll be + * Include the short name (last component) of the logger in the log message. Defaults to true - otherwise we'll be * lost in a flood of messages without knowing who sends them. */ private final boolean showShortName; @@ -99,14 +101,22 @@ public class SimpleLoggerContext implements LoggerContext { @Override public ExtendedLogger getLogger(final String name, final MessageFactory messageFactory) { - final ExtendedLogger extendedLogger = loggers.get(name); + // The loggers map key is the logger name plus the messageFactory FQCN (if any). + String key = name; + if (messageFactory != null) { + key = LoggerContextKey.create(name, messageFactory); + } + final ExtendedLogger extendedLogger = loggers.get(key); if (extendedLogger != null) { AbstractLogger.checkMessageFactory(extendedLogger, messageFactory); return extendedLogger; } - loggers.putIfAbsent(name, new SimpleLogger(name, defaultLevel, showLogName, showShortName, showDateTime, - showContextMap, dateTimeFormat, messageFactory, props, stream)); - return loggers.get(name); + final SimpleLogger simpleLogger = new SimpleLogger(name, defaultLevel, showLogName, showShortName, showDateTime, + showContextMap, dateTimeFormat, messageFactory, props, stream); + // If messageFactory was null then we need to pull it out of the logger now + key = LoggerContextKey.create(name, simpleLogger.getMessageFactory()); + loggers.putIfAbsent(key, simpleLogger); + return loggers.get(key); } @Override @@ -115,7 +125,18 @@ public class SimpleLoggerContext implements LoggerContext { } @Override + public boolean hasLogger(String name, MessageFactory messageFactory) { + return false; + } + + @Override + public boolean hasLogger(String name, Class<? extends MessageFactory> messageFactoryClass) { + return false; + } + + @Override public Object getExternalContext() { return null; } + } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/7fd641a0/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContext.java ---------------------------------------------------------------------- diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContext.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContext.java index 5cf3eb8..320c8e0 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContext.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContext.java @@ -51,4 +51,20 @@ public interface LoggerContext { * @return true if the Logger exists, false otherwise. */ boolean hasLogger(String name); + + /** + * Detects if a Logger with the specified name and MessageFactory exists. + * @param name The Logger name to search for. + * @return true if the Logger exists, false otherwise. + * @since 2.5 + */ + boolean hasLogger(String name, MessageFactory messageFactory); + + /** + * Detects if a Logger with the specified name and MessageFactory type exists. + * @param name The Logger name to search for. + * @return true if the Logger exists, false otherwise. + * @since 2.5 + */ + boolean hasLogger(String name, Class<? extends MessageFactory> messageFactoryClass); } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/7fd641a0/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextKey.java ---------------------------------------------------------------------- diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextKey.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextKey.java new file mode 100644 index 0000000..bff50ed --- /dev/null +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextKey.java @@ -0,0 +1,40 @@ +/* + * 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.logging.log4j.spi; + +import org.apache.logging.log4j.message.MessageFactory; + +public class LoggerContextKey { + + public static String create(final String name) { + return create(name, AbstractLogger.DEFAULT_MESSAGE_FACTORY_CLASS); + } + + public static String create(final String name, final MessageFactory messageFactory) { + final Class<? extends MessageFactory> messageFactoryClass = messageFactory != null ? messageFactory.getClass() + : AbstractLogger.DEFAULT_MESSAGE_FACTORY_CLASS; + return create(name, messageFactoryClass); + } + + public static String create(final String name, final Class<? extends MessageFactory> messageFactoryClass) { + final Class<? extends MessageFactory> mfClass = messageFactoryClass != null ? messageFactoryClass + : AbstractLogger.DEFAULT_MESSAGE_FACTORY_CLASS; + return name + "." + mfClass.getName(); + } + +} http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/7fd641a0/log4j-api/src/test/java/org/apache/logging/log4j/TestLoggerContext.java ---------------------------------------------------------------------- diff --git a/log4j-api/src/test/java/org/apache/logging/log4j/TestLoggerContext.java b/log4j-api/src/test/java/org/apache/logging/log4j/TestLoggerContext.java index 5aac379..b0a5a82 100644 --- a/log4j-api/src/test/java/org/apache/logging/log4j/TestLoggerContext.java +++ b/log4j-api/src/test/java/org/apache/logging/log4j/TestLoggerContext.java @@ -46,13 +46,23 @@ public class TestLoggerContext implements LoggerContext { } @Override + public Object getExternalContext() { + return null; + } + + @Override public boolean hasLogger(final String name) { return false; } @Override - public Object getExternalContext() { - return null; + public boolean hasLogger(String name, MessageFactory messageFactory) { + return false; + } + + @Override + public boolean hasLogger(String name, Class<? extends MessageFactory> messageFactoryClass) { + return false; } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/7fd641a0/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java index 6605129..d7c44fd 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java @@ -46,6 +46,7 @@ import org.apache.logging.log4j.core.util.ShutdownCallbackRegistry; import org.apache.logging.log4j.message.MessageFactory; import org.apache.logging.log4j.spi.AbstractLogger; import org.apache.logging.log4j.spi.LoggerContextFactory; +import org.apache.logging.log4j.spi.LoggerContextKey; import static org.apache.logging.log4j.core.util.ShutdownCallbackRegistry.*; @@ -382,14 +383,21 @@ public class LoggerContext extends AbstractLifeCycle implements org.apache.loggi */ @Override public Logger getLogger(final String name, final MessageFactory messageFactory) { - Logger logger = loggers.get(name); + // The loggers map key is the logger name plus the messageFactory FQCN (if any). + String key = name; + if (messageFactory != null) { + key = LoggerContextKey.create(name, messageFactory); + } + Logger logger = loggers.get(key); if (logger != null) { AbstractLogger.checkMessageFactory(logger, messageFactory); return logger; } logger = newInstance(this, name, messageFactory); - final Logger prev = loggers.putIfAbsent(name, logger); + // If messageFactory was null then we need to pull it out of the logger now + key = LoggerContextKey.create(name, logger.getMessageFactory()); + final Logger prev = loggers.putIfAbsent(key, logger); return prev == null ? logger : prev; } @@ -401,7 +409,29 @@ public class LoggerContext extends AbstractLifeCycle implements org.apache.loggi */ @Override public boolean hasLogger(final String name) { - return loggers.containsKey(name); + return loggers.containsKey(LoggerContextKey.create(name)); + } + + /** + * Determines if the specified Logger exists. + * + * @param name The Logger name to search for. + * @return True if the Logger exists, false otherwise. + */ + @Override + public boolean hasLogger(final String name, MessageFactory messageFactory) { + return loggers.containsKey(LoggerContextKey.create(name, messageFactory)); + } + + /** + * Determines if the specified Logger exists. + * + * @param name The Logger name to search for. + * @return True if the Logger exists, false otherwise. + */ + @Override + public boolean hasLogger(final String name, Class<? extends MessageFactory> messageFactoryClass) { + return loggers.containsKey(LoggerContextKey.create(name, messageFactoryClass)); } /** http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/7fd641a0/log4j-core/src/test/java/org/apache/logging/log4j/core/LoggerTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/LoggerTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/LoggerTest.java index ecc1f22..8e48ea2 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/LoggerTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/LoggerTest.java @@ -41,6 +41,7 @@ import org.apache.logging.log4j.message.MessageFactory; import org.apache.logging.log4j.message.ParameterizedMessageFactory; import org.apache.logging.log4j.message.StringFormatterMessageFactory; import org.apache.logging.log4j.message.StructuredDataMessage; +import org.apache.logging.log4j.spi.AbstractLogger; import org.apache.logging.log4j.test.appender.ListAppender; import org.junit.Before; import org.junit.Rule; @@ -271,15 +272,24 @@ public class LoggerTest { private static Logger testMessageFactoryMismatch(final String name, final MessageFactory messageFactory1, - final MessageFactory messageFactory2) { + final MessageFactory messageFactory2) { final Logger testLogger1 = (Logger) LogManager.getLogger(name, messageFactory1); assertNotNull(testLogger1); - assertEquals(messageFactory1, testLogger1.getMessageFactory()); + checkMessageFactory(messageFactory1, testLogger1); final Logger testLogger2 = (Logger) LogManager.getLogger(name, messageFactory2); - assertEquals(messageFactory1, testLogger2.getMessageFactory()); + assertNotNull(testLogger2); + checkMessageFactory(messageFactory2, testLogger2); return testLogger1; } + private static void checkMessageFactory(final MessageFactory messageFactory1, final Logger testLogger1) { + if (messageFactory1 == null) { + assertEquals(AbstractLogger.DEFAULT_MESSAGE_FACTORY_CLASS, testLogger1.getMessageFactory().getClass()); + } else { + assertEquals(messageFactory1, testLogger1.getMessageFactory()); + } + } + @Test public void debugObject() { logger.debug(new Date()); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/7fd641a0/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java ---------------------------------------------------------------------- diff --git a/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java b/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java index a6b9a1d..f25ffc1 100644 --- a/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java +++ b/log4j-taglib/src/main/java/org/apache/logging/log4j/taglib/Log4jTaglibLoggerContext.java @@ -23,8 +23,9 @@ import javax.servlet.ServletContext; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.message.MessageFactory; import org.apache.logging.log4j.spi.AbstractLogger; -import org.apache.logging.log4j.spi.LoggerContext; import org.apache.logging.log4j.spi.ExtendedLogger; +import org.apache.logging.log4j.spi.LoggerContext; +import org.apache.logging.log4j.spi.LoggerContextKey; /** * This bridge between the tag library and the Log4j API ensures that instances of {@link Log4jTaglibLogger} are @@ -82,7 +83,17 @@ final class Log4jTaglibLoggerContext implements LoggerContext { @Override public boolean hasLogger(final String name) { - return this.loggers.containsKey(name); + return loggers.containsKey(LoggerContextKey.create(name)); + } + + @Override + public boolean hasLogger(String name, MessageFactory messageFactory) { + return loggers.containsKey(LoggerContextKey.create(name, messageFactory)); + } + + @Override + public boolean hasLogger(String name, Class<? extends MessageFactory> messageFactoryClass) { + return loggers.containsKey(LoggerContextKey.create(name, messageFactoryClass)); } static synchronized Log4jTaglibLoggerContext getInstance(final ServletContext servletContext) { http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/7fd641a0/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContext.java ---------------------------------------------------------------------- diff --git a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContext.java b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContext.java index 95fb776..c4e6a99 100644 --- a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContext.java +++ b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContext.java @@ -20,8 +20,9 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import org.apache.logging.log4j.message.MessageFactory; -import org.apache.logging.log4j.spi.LoggerContext; import org.apache.logging.log4j.spi.ExtendedLogger; +import org.apache.logging.log4j.spi.LoggerContext; +import org.apache.logging.log4j.spi.LoggerContextKey; import org.slf4j.LoggerFactory; /** @@ -53,6 +54,16 @@ public class SLF4JLoggerContext implements LoggerContext { @Override public boolean hasLogger(final String name) { - return loggers.containsKey(name); + return loggers.containsKey(LoggerContextKey.create(name)); + } + + @Override + public boolean hasLogger(String name, MessageFactory messageFactory) { + return loggers.containsKey(LoggerContextKey.create(name, messageFactory)); + } + + @Override + public boolean hasLogger(String name, Class<? extends MessageFactory> messageFactoryClass) { + return loggers.containsKey(LoggerContextKey.create(name, messageFactoryClass)); } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/7fd641a0/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index a19805b..50f0673 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -42,6 +42,9 @@ <action issue="LOG4J2-1187" dev="ggregory" type="add"> Support use case for java.sql.DriverManager.setLogStream(PrintStream). </action> + <action issue="LOG4J2-1180" dev="ggregory" type="fix" due-to="Mikael Ståldal"> + Logger cache does not account for message factory. + </action> <action issue="LOG4J2-879" dev="rpopma" type="fix"> Documentation: fixed minor issues with the site and manual pages. </action>
