From 33291c76f68a7837b2f90ce5c760d507213d05e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 24 Apr 2024 11:45:36 +0200 Subject: [PATCH 1/4] Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties --- .../logging/log4j/spi/AbstractLogger.java | 66 +++++++------ .../log4j/core/LoggerMessageFactoryTest.java | 94 +++++++++++++++++++ .../org/apache/logging/log4j/core/Logger.java | 94 +++++++++++++++++-- .../2379_fix_message_factory_properties.xml | 8 ++ 4 files changed, 226 insertions(+), 36 deletions(-) create mode 100644 log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryTest.java create mode 100644 src/changelog/.2.x.x/2379_fix_message_factory_properties.xml diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java index 30d0cffef02..ecc499ac586 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java @@ -106,34 +106,57 @@ public abstract class AbstractLogger implements ExtendedLogger, LocationAwareLog private static final ThreadLocal logBuilder = ThreadLocal.withInitial(DefaultLogBuilder::new); /** - * Creates a new logger named after this class (or subclass). + * Constructs an instance named after this class. */ public AbstractLogger() { - final String canonicalName = getClass().getCanonicalName(); - this.name = canonicalName != null ? canonicalName : getClass().getName(); - this.messageFactory = createDefaultMessageFactory(); - this.flowMessageFactory = createDefaultFlowMessageFactory(); + this(null, null, null); } /** - * Creates a new named logger. + * Constructs an instance using the provided name. * - * @param name the logger name + * @param name the logger name (if null, will be derived from this class or subclass) */ public AbstractLogger(final String name) { - this(name, createDefaultMessageFactory()); + this(name, null, null); } /** - * Creates a new named logger with a particular {@link MessageFactory}. + * Constructs an instance using the provided name and {@link MessageFactory}. * - * @param name the logger name - * @param messageFactory the message factory, if null then use the default message factory. + * @param name the logger name (if null, will be derived from this class) + * @param messageFactory the {@link Message} factory (if null, {@link ParameterizedMessageFactory} will be used) */ public AbstractLogger(final String name, final MessageFactory messageFactory) { - this.name = name; - this.messageFactory = messageFactory == null ? createDefaultMessageFactory() : narrow(messageFactory); - this.flowMessageFactory = createDefaultFlowMessageFactory(); + this(name, messageFactory, null); + } + + /** + * The canonical constructor. + * + * @param name the logger name (if null, will be derived from this class) + * @param messageFactory the {@link Message} factory (if null, {@link ParameterizedMessageFactory} will be used) + * @param flowMessageFactory the {@link org.apache.logging.log4j.message.FlowMessage} factory (if null, {@link DefaultFlowMessageFactory} will be used) + */ + protected AbstractLogger( + final String name, final MessageFactory messageFactory, final FlowMessageFactory flowMessageFactory) { + if (name != null) { + this.name = name; + } else { + final Class clazz = getClass(); + final String canonicalName = clazz.getCanonicalName(); + this.name = canonicalName != null ? canonicalName : clazz.getName(); + } + this.messageFactory = + messageFactory != null ? adaptMessageFactory(messageFactory) : ParameterizedMessageFactory.INSTANCE; + this.flowMessageFactory = flowMessageFactory != null ? flowMessageFactory : DefaultFlowMessageFactory.INSTANCE; + } + + private static MessageFactory2 adaptMessageFactory(final MessageFactory result) { + if (result instanceof MessageFactory2) { + return (MessageFactory2) result; + } + return new MessageFactory2Adapter(result); } /** @@ -196,21 +219,6 @@ protected Message catchingMsg(final Throwable throwable) { return messageFactory.newMessage(CATCHING); } - private static MessageFactory2 createDefaultMessageFactory() { - return ParameterizedMessageFactory.INSTANCE; - } - - private static MessageFactory2 narrow(final MessageFactory result) { - if (result instanceof MessageFactory2) { - return (MessageFactory2) result; - } - return new MessageFactory2Adapter(result); - } - - private static FlowMessageFactory createDefaultFlowMessageFactory() { - return DefaultFlowMessageFactory.INSTANCE; - } - @Override public void debug(final Marker marker, final CharSequence message) { logIfEnabled(FQCN, Level.DEBUG, marker, message, null); diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryTest.java new file mode 100644 index 00000000000..26a8ed82f42 --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryTest.java @@ -0,0 +1,94 @@ +/* + * 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.core; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.apache.logging.log4j.message.AbstractMessageFactory; +import org.apache.logging.log4j.message.DefaultFlowMessageFactory; +import org.apache.logging.log4j.message.Message; +import org.apache.logging.log4j.message.MessageFactory; +import org.apache.logging.log4j.message.ParameterizedMessageFactory; +import org.apache.logging.log4j.message.ReusableMessageFactory; +import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ClearSystemProperty; +import org.junitpioneer.jupiter.SetSystemProperty; + +class LoggerMessageFactoryTest { + + @Test + @SetSystemProperty(key = "log4j2.is.webapp", value = "false") + @SetSystemProperty(key = "log4j2.enableThreadLocals", value = "true") + @ClearSystemProperty(key = "log4j2.messageFactory") + @ClearSystemProperty(key = "log4j2.flowMessageFactory") + void defaults_should_match_when_thread_locals_enabled() { + assertDefaultsMatch(ReusableMessageFactory.INSTANCE); + } + + @Test + @SetSystemProperty(key = "log4j2.enableThreadLocals", value = "false") + @ClearSystemProperty(key = "log4j2.messageFactory") + @ClearSystemProperty(key = "log4j2.flowMessageFactory") + void defaults_should_match_when_thread_locals_disabled() { + assertDefaultsMatch(ParameterizedMessageFactory.INSTANCE); + } + + private static void assertDefaultsMatch(final MessageFactory expectedMessageFactory) { + final LoggerContext loggerContext = new LoggerContext(LoggerMessageFactoryTest.class.getSimpleName()); + final Logger logger = new Logger(loggerContext, "assertDefaultsMatch", null, null); + assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(expectedMessageFactory); + assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE); + } + + @Test + @ClearSystemProperty(key = "log4j2.messageFactory") + @ClearSystemProperty(key = "log4j2.flowMessageFactory") + void arguments_should_be_honored() { + final LoggerContext loggerContext = new LoggerContext(LoggerMessageFactoryTest.class.getSimpleName()); + final Logger logger = new Logger( + loggerContext, "arguments_should_be_honored", new TestMessageFactory(), new TestFlowMessageFactory()); + assertTestMessageFactories(logger); + } + + @Test + @SetSystemProperty( + key = "log4j2.messageFactory", + value = "org.apache.logging.log4j.core.LoggerMessageFactoryTest$TestMessageFactory") + @SetSystemProperty( + key = "log4j2.flowMessageFactory", + value = "org.apache.logging.log4j.core.LoggerMessageFactoryTest$TestFlowMessageFactory") + void properties_should_be_honored() { + final LoggerContext loggerContext = new LoggerContext(LoggerMessageFactoryTest.class.getSimpleName()); + final Logger logger = new Logger(loggerContext, "properties_should_be_honored", null, null); + assertTestMessageFactories(logger); + } + + private static void assertTestMessageFactories(Logger logger) { + assertThat((MessageFactory) logger.getMessageFactory()).isInstanceOf(TestMessageFactory.class); + assertThat(logger.getFlowMessageFactory()).isInstanceOf(TestFlowMessageFactory.class); + } + + public static final class TestMessageFactory extends AbstractMessageFactory { + + @Override + public Message newMessage(final String message, final Object... params) { + return ParameterizedMessageFactory.INSTANCE.newMessage(message, params); + } + } + + public static final class TestFlowMessageFactory extends DefaultFlowMessageFactory {} +} diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java index 81813629ebe..ca573afc273 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java @@ -16,6 +16,8 @@ */ package org.apache.logging.log4j.core; +import static java.util.Objects.requireNonNull; + import java.io.ObjectStreamException; import java.io.Serializable; import java.util.ArrayList; @@ -31,10 +33,18 @@ import org.apache.logging.log4j.core.config.LoggerConfig; import org.apache.logging.log4j.core.config.ReliabilityStrategy; import org.apache.logging.log4j.core.filter.CompositeFilter; +import org.apache.logging.log4j.core.util.Constants; +import org.apache.logging.log4j.message.DefaultFlowMessageFactory; +import org.apache.logging.log4j.message.FlowMessageFactory; import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.message.MessageFactory; +import org.apache.logging.log4j.message.ParameterizedMessageFactory; +import org.apache.logging.log4j.message.ReusableMessageFactory; import org.apache.logging.log4j.message.SimpleMessage; import org.apache.logging.log4j.spi.AbstractLogger; +import org.apache.logging.log4j.status.StatusLogger; +import org.apache.logging.log4j.util.LoaderUtil; +import org.apache.logging.log4j.util.PropertiesUtil; import org.apache.logging.log4j.util.Strings; import org.apache.logging.log4j.util.Supplier; @@ -54,6 +64,10 @@ public class Logger extends AbstractLogger implements Supplier { private static final long serialVersionUID = 1L; + private static final String MESSAGE_FACTORY_PROPERTY_NAME = "log4j2.messageFactory"; + + private static final String FLOW_MESSAGE_FACTORY_PROPERTY_NAME = "log4j2.flowMessageFactory"; + /** * Config should be consistent across threads. */ @@ -63,16 +77,82 @@ public class Logger extends AbstractLogger implements Supplier { private final LoggerContext context; /** - * The constructor. + * Constructs an instance using the given {@link LoggerContext}, logger name, and {@link MessageFactory}. * - * @param context The LoggerContext this Logger is associated with. - * @param messageFactory The message factory. - * @param name The name of the Logger. + * @param context the {@link LoggerContext} this logger is associated with + * @param messageFactory The message factory to be used. + * If null, first the {@value #MESSAGE_FACTORY_PROPERTY_NAME} property will be used to instantiate the message factory. + * If the property is missing and the {@code log4j2.enableThreadLocals} property is not {@code false}, {@link ReusableMessageFactory} will be used. + * Otherwise, we will fall back to {@link ParameterizedMessageFactory}. + * @param name the logger name */ protected Logger(final LoggerContext context, final String name, final MessageFactory messageFactory) { - super(name, messageFactory); - this.context = context; - privateConfig = new PrivateConfig(context.getConfiguration(), this); + this(context, name, messageFactory, null); + } + + /** + * The canonical constructor. + * + * @param context the {@link LoggerContext} this logger is associated with + * @param messageFactory The message factory to be used. + * If null, first the {@value #MESSAGE_FACTORY_PROPERTY_NAME} property will be used to instantiate the message factory. + * If the property is missing and the {@code log4j2.enableThreadLocals} property is not {@code false}, {@link ReusableMessageFactory} will be used. + * Otherwise, we will fall back to {@link ParameterizedMessageFactory}. + * @param flowMessageFactory The flow message factory to be used. + * If null, first the {@value #FLOW_MESSAGE_FACTORY_PROPERTY_NAME} property will be used to instantiate the flow message factory. + * If the property is missing, {@link DefaultFlowMessageFactory} will be used. + * @param name the logger name + */ + protected Logger( + final LoggerContext context, + final String name, + final MessageFactory messageFactory, + final FlowMessageFactory flowMessageFactory) { + super(name, getEffectiveMessageFactory(messageFactory), getEffectiveFlowMessageFactory(flowMessageFactory)); + this.context = requireNonNull(context, "context"); + this.privateConfig = new PrivateConfig(context.getConfiguration(), this); + } + + private static MessageFactory getEffectiveMessageFactory(final MessageFactory messageFactory) { + return createInstanceFromFactoryProperty( + MessageFactory.class, + messageFactory, + MESSAGE_FACTORY_PROPERTY_NAME, + () -> Constants.ENABLE_THREADLOCALS + ? ReusableMessageFactory.INSTANCE + : ParameterizedMessageFactory.INSTANCE); + } + + private static FlowMessageFactory getEffectiveFlowMessageFactory(final FlowMessageFactory flowMessageFactory) { + return createInstanceFromFactoryProperty( + FlowMessageFactory.class, + flowMessageFactory, + FLOW_MESSAGE_FACTORY_PROPERTY_NAME, + () -> DefaultFlowMessageFactory.INSTANCE); + } + + private static V createInstanceFromFactoryProperty( + final Class instanceType, + final V providedInstance, + final String propertyName, + final java.util.function.Supplier fallbackInstanceSupplier) { + if (providedInstance != null) { + return providedInstance; + } + final String className = PropertiesUtil.getProperties().getStringProperty(propertyName); + if (Strings.isNotBlank(className)) { + try { + return LoaderUtil.newCheckedInstanceOf(className, instanceType); + } catch (final Throwable error) { + StatusLogger.getLogger() + .error( + "failed instantiating the `{}` class obtained from the `{}` property", + className, + propertyName, + error); + } + } + return fallbackInstanceSupplier.get(); } protected Object writeReplace() throws ObjectStreamException { diff --git a/src/changelog/.2.x.x/2379_fix_message_factory_properties.xml b/src/changelog/.2.x.x/2379_fix_message_factory_properties.xml new file mode 100644 index 00000000000..95aa7723d4d --- /dev/null +++ b/src/changelog/.2.x.x/2379_fix_message_factory_properties.xml @@ -0,0 +1,8 @@ + + + + Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties + From 587c2485fc2cd8c78d06e064a71184fe96d9b23f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 24 Apr 2024 13:20:24 +0200 Subject: [PATCH 2/4] Fix property issues --- ...oggerMessageFactoryCustomizationTest.java} | 37 ++++------------- ...MessageFactoryDefaultsTlaDisabledTest.java | 41 +++++++++++++++++++ ...rMessageFactoryDefaultsTlaEnabledTest.java | 41 +++++++++++++++++++ 3 files changed, 89 insertions(+), 30 deletions(-) rename log4j-core-test/src/test/java/org/apache/logging/log4j/core/{LoggerMessageFactoryTest.java => LoggerMessageFactoryCustomizationTest.java} (63%) create mode 100644 log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaDisabledTest.java create mode 100644 log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaEnabledTest.java diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java similarity index 63% rename from log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryTest.java rename to log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java index 26a8ed82f42..9043c47b1c7 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java @@ -23,42 +23,18 @@ import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.message.MessageFactory; import org.apache.logging.log4j.message.ParameterizedMessageFactory; -import org.apache.logging.log4j.message.ReusableMessageFactory; import org.junit.jupiter.api.Test; import org.junitpioneer.jupiter.ClearSystemProperty; import org.junitpioneer.jupiter.SetSystemProperty; -class LoggerMessageFactoryTest { - - @Test - @SetSystemProperty(key = "log4j2.is.webapp", value = "false") - @SetSystemProperty(key = "log4j2.enableThreadLocals", value = "true") - @ClearSystemProperty(key = "log4j2.messageFactory") - @ClearSystemProperty(key = "log4j2.flowMessageFactory") - void defaults_should_match_when_thread_locals_enabled() { - assertDefaultsMatch(ReusableMessageFactory.INSTANCE); - } - - @Test - @SetSystemProperty(key = "log4j2.enableThreadLocals", value = "false") - @ClearSystemProperty(key = "log4j2.messageFactory") - @ClearSystemProperty(key = "log4j2.flowMessageFactory") - void defaults_should_match_when_thread_locals_disabled() { - assertDefaultsMatch(ParameterizedMessageFactory.INSTANCE); - } - - private static void assertDefaultsMatch(final MessageFactory expectedMessageFactory) { - final LoggerContext loggerContext = new LoggerContext(LoggerMessageFactoryTest.class.getSimpleName()); - final Logger logger = new Logger(loggerContext, "assertDefaultsMatch", null, null); - assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(expectedMessageFactory); - assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE); - } +class LoggerMessageFactoryCustomizationTest { @Test @ClearSystemProperty(key = "log4j2.messageFactory") @ClearSystemProperty(key = "log4j2.flowMessageFactory") void arguments_should_be_honored() { - final LoggerContext loggerContext = new LoggerContext(LoggerMessageFactoryTest.class.getSimpleName()); + final LoggerContext loggerContext = + new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName()); final Logger logger = new Logger( loggerContext, "arguments_should_be_honored", new TestMessageFactory(), new TestFlowMessageFactory()); assertTestMessageFactories(logger); @@ -67,12 +43,13 @@ void arguments_should_be_honored() { @Test @SetSystemProperty( key = "log4j2.messageFactory", - value = "org.apache.logging.log4j.core.LoggerMessageFactoryTest$TestMessageFactory") + value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$TestMessageFactory") @SetSystemProperty( key = "log4j2.flowMessageFactory", - value = "org.apache.logging.log4j.core.LoggerMessageFactoryTest$TestFlowMessageFactory") + value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$TestFlowMessageFactory") void properties_should_be_honored() { - final LoggerContext loggerContext = new LoggerContext(LoggerMessageFactoryTest.class.getSimpleName()); + final LoggerContext loggerContext = + new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName()); final Logger logger = new Logger(loggerContext, "properties_should_be_honored", null, null); assertTestMessageFactories(logger); } diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaDisabledTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaDisabledTest.java new file mode 100644 index 00000000000..af3c8624645 --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaDisabledTest.java @@ -0,0 +1,41 @@ +/* + * 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.core; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.apache.logging.log4j.core.util.Constants; +import org.apache.logging.log4j.message.DefaultFlowMessageFactory; +import org.apache.logging.log4j.message.MessageFactory; +import org.apache.logging.log4j.message.ParameterizedMessageFactory; +import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.SetSystemProperty; + +class LoggerMessageFactoryDefaultsTlaDisabledTest { + + @Test + @SetSystemProperty(key = "log4j2.enableThreadLocals", value = "false") + void defaults_should_match_when_thread_locals_disabled() { + assertThat(Constants.ENABLE_THREADLOCALS).isFalse(); + final LoggerContext loggerContext = + new LoggerContext(LoggerMessageFactoryDefaultsTlaDisabledTest.class.getSimpleName()); + final Logger logger = + new Logger(loggerContext, "defaults_should_match_when_thread_locals_disabled", null, null); + assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(ParameterizedMessageFactory.INSTANCE); + assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE); + } +} diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaEnabledTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaEnabledTest.java new file mode 100644 index 00000000000..3255566710c --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaEnabledTest.java @@ -0,0 +1,41 @@ +/* + * 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.core; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.apache.logging.log4j.core.util.Constants; +import org.apache.logging.log4j.message.DefaultFlowMessageFactory; +import org.apache.logging.log4j.message.MessageFactory; +import org.apache.logging.log4j.message.ReusableMessageFactory; +import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.SetSystemProperty; + +class LoggerMessageFactoryDefaultsTlaEnabledTest { + + @Test + @SetSystemProperty(key = "log4j2.is.webapp", value = "false") + @SetSystemProperty(key = "log4j2.enableThreadLocals", value = "true") + void defaults_should_match_when_thread_locals_enabled() { + assertThat(Constants.ENABLE_THREADLOCALS).isTrue(); + final LoggerContext loggerContext = + new LoggerContext(LoggerMessageFactoryDefaultsTlaEnabledTest.class.getSimpleName()); + final Logger logger = new Logger(loggerContext, "defaults_should_match_when_thread_locals_enabled", null, null); + assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(ReusableMessageFactory.INSTANCE); + assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE); + } +} From 5fc9c80beee1c6e52f5e34e10d29e264a4a45c2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 24 Apr 2024 13:24:23 +0200 Subject: [PATCH 3/4] Fix issue link --- src/changelog/.2.x.x/2379_fix_message_factory_properties.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/changelog/.2.x.x/2379_fix_message_factory_properties.xml b/src/changelog/.2.x.x/2379_fix_message_factory_properties.xml index 95aa7723d4d..f86978422f1 100644 --- a/src/changelog/.2.x.x/2379_fix_message_factory_properties.xml +++ b/src/changelog/.2.x.x/2379_fix_message_factory_properties.xml @@ -3,6 +3,6 @@ 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"> - + Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties From 097853933f8a19aa6333dd62d7fa5c06250452be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 24 Apr 2024 15:02:58 +0200 Subject: [PATCH 4/4] Use `LoaderUtil.newCheckedInstanceOfProperty()` --- .../org/apache/logging/log4j/core/Logger.java | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java index ca573afc273..777d627115d 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java @@ -42,9 +42,7 @@ import org.apache.logging.log4j.message.ReusableMessageFactory; import org.apache.logging.log4j.message.SimpleMessage; import org.apache.logging.log4j.spi.AbstractLogger; -import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.LoaderUtil; -import org.apache.logging.log4j.util.PropertiesUtil; import org.apache.logging.log4j.util.Strings; import org.apache.logging.log4j.util.Supplier; @@ -139,20 +137,13 @@ private static V createInstanceFromFactoryProperty( if (providedInstance != null) { return providedInstance; } - final String className = PropertiesUtil.getProperties().getStringProperty(propertyName); - if (Strings.isNotBlank(className)) { - try { - return LoaderUtil.newCheckedInstanceOf(className, instanceType); - } catch (final Throwable error) { - StatusLogger.getLogger() - .error( - "failed instantiating the `{}` class obtained from the `{}` property", - className, - propertyName, - error); - } + try { + return LoaderUtil.newCheckedInstanceOfProperty(propertyName, instanceType, fallbackInstanceSupplier); + } catch (final Exception error) { + final String message = + String.format("failed instantiating the class pointed by the `%s` property", propertyName); + throw new RuntimeException(message, error); } - return fallbackInstanceSupplier.get(); } protected Object writeReplace() throws ObjectStreamException {