Fix handling of log4j2.messageFactory and log4j2.flowMessageFactory properties

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<DefaultLogBuilder> 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<? extends AbstractLogger> 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);

log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java

@@ -0,0 +1,71 @@
+/*
+ * 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.junit.jupiter.api.Test;
+import org.junitpioneer.jupiter.ClearSystemProperty;
+import org.junitpioneer.jupiter.SetSystemProperty;
+
+class LoggerMessageFactoryCustomizationTest {
+
+    @Test
+    @ClearSystemProperty(key = "log4j2.messageFactory")
+    @ClearSystemProperty(key = "log4j2.flowMessageFactory")
+    void arguments_should_be_honored() {
+        final LoggerContext loggerContext =
+                new LoggerContext(LoggerMessageFactoryCustomizationTest.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.LoggerMessageFactoryCustomizationTest$TestMessageFactory")
+    @SetSystemProperty(
+            key = "log4j2.flowMessageFactory",
+            value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$TestFlowMessageFactory")
+    void properties_should_be_honored() {
+        final LoggerContext loggerContext =
+                new LoggerContext(LoggerMessageFactoryCustomizationTest.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 {}
+}

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);
+    }
+}

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);
+    }
+}

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,16 @@
 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.util.LoaderUtil;
 import org.apache.logging.log4j.util.Strings;
 import org.apache.logging.log4j.util.Supplier;
 
@@ -54,6 +62,10 @@ public class Logger extends AbstractLogger implements Supplier<LoggerConfig> {
 
     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 +75,75 @@ public class Logger extends AbstractLogger implements Supplier<LoggerConfig> {
     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> V createInstanceFromFactoryProperty(
+            final Class<V> instanceType,
+            final V providedInstance,
+            final String propertyName,
+            final java.util.function.Supplier<V> fallbackInstanceSupplier) {
+        if (providedInstance != null) {
+            return providedInstance;
+        }
+        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);
+        }
     }
 
     protected Object writeReplace() throws ObjectStreamException {

src/changelog/.2.x.x/2379_fix_message_factory_properties.xml

@@ -0,0 +1,8 @@
+<?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="2505" link="https://github.com/apache/logging-log4j2/pull/2505"/>
+  <description format="asciidoc">Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties</description>
+</entry>