Skip to content

Commit 3efd59e

Browse files
vyvy
authored
Fix handling of log4j2.messageFactory and log4j2.flowMessageFactory properties (#2505)
1 parent 9a04f35 commit 3efd59e

File tree

6 files changed

+276
-36
lines changed

6 files changed

+276
-36
lines changed

‎log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java

+37-29
Original file line numberDiff line numberDiff line change
@@ -106,34 +106,57 @@ public abstract class AbstractLogger implements ExtendedLogger, LocationAwareLog
106106
private static final ThreadLocal<DefaultLogBuilder> logBuilder = ThreadLocal.withInitial(DefaultLogBuilder::new);
107107

108108
/**
109-
* Creates a new logger named after this class (or subclass).
109+
* Constructs an instance named after this class.
110110
*/
111111
public AbstractLogger() {
112-
final String canonicalName = getClass().getCanonicalName();
113-
this.name = canonicalName != null ? canonicalName : getClass().getName();
114-
this.messageFactory = createDefaultMessageFactory();
115-
this.flowMessageFactory = createDefaultFlowMessageFactory();
112+
this(null, null, null);
116113
}
117114

118115
/**
119-
* Creates a new named logger.
116+
* Constructs an instance using the provided name.
120117
*
121-
* @param name the logger name
118+
* @param name the logger name (if null, will be derived from this class or subclass)
122119
*/
123120
public AbstractLogger(final String name) {
124-
this(name, createDefaultMessageFactory());
121+
this(name, null, null);
125122
}
126123

127124
/**
128-
* Creates a new named logger with a particular {@link MessageFactory}.
125+
* Constructs an instance using the provided name and {@link MessageFactory}.
129126
*
130-
* @param name the logger name
131-
* @param messageFactory the message factory, if null then use the default message factory.
127+
* @param name the logger name (if null, will be derived from this class)
128+
* @param messageFactory the {@link Message} factory (if null, {@link ParameterizedMessageFactory} will be used)
132129
*/
133130
public AbstractLogger(final String name, final MessageFactory messageFactory) {
134-
this.name = name;
135-
this.messageFactory = messageFactory == null ? createDefaultMessageFactory() : narrow(messageFactory);
136-
this.flowMessageFactory = createDefaultFlowMessageFactory();
131+
this(name, messageFactory, null);
132+
}
133+
134+
/**
135+
* The canonical constructor.
136+
*
137+
* @param name the logger name (if null, will be derived from this class)
138+
* @param messageFactory the {@link Message} factory (if null, {@link ParameterizedMessageFactory} will be used)
139+
* @param flowMessageFactory the {@link org.apache.logging.log4j.message.FlowMessage} factory (if null, {@link DefaultFlowMessageFactory} will be used)
140+
*/
141+
protected AbstractLogger(
142+
final String name, final MessageFactory messageFactory, final FlowMessageFactory flowMessageFactory) {
143+
if (name != null) {
144+
this.name = name;
145+
} else {
146+
final Class<? extends AbstractLogger> clazz = getClass();
147+
final String canonicalName = clazz.getCanonicalName();
148+
this.name = canonicalName != null ? canonicalName : clazz.getName();
149+
}
150+
this.messageFactory =
151+
messageFactory != null ? adaptMessageFactory(messageFactory) : ParameterizedMessageFactory.INSTANCE;
152+
this.flowMessageFactory = flowMessageFactory != null ? flowMessageFactory : DefaultFlowMessageFactory.INSTANCE;
153+
}
154+
155+
private static MessageFactory2 adaptMessageFactory(final MessageFactory result) {
156+
if (result instanceof MessageFactory2) {
157+
return (MessageFactory2) result;
158+
}
159+
return new MessageFactory2Adapter(result);
137160
}
138161

139162
/**
@@ -196,21 +219,6 @@ protected Message catchingMsg(final Throwable throwable) {
196219
return messageFactory.newMessage(CATCHING);
197220
}
198221

199-
private static MessageFactory2 createDefaultMessageFactory() {
200-
return ParameterizedMessageFactory.INSTANCE;
201-
}
202-
203-
private static MessageFactory2 narrow(final MessageFactory result) {
204-
if (result instanceof MessageFactory2) {
205-
return (MessageFactory2) result;
206-
}
207-
return new MessageFactory2Adapter(result);
208-
}
209-
210-
private static FlowMessageFactory createDefaultFlowMessageFactory() {
211-
return DefaultFlowMessageFactory.INSTANCE;
212-
}
213-
214222
@Override
215223
public void debug(final Marker marker, final CharSequence message) {
216224
logIfEnabled(FQCN, Level.DEBUG, marker, message, null);

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

+71
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.core;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
21+
import org.apache.logging.log4j.message.AbstractMessageFactory;
22+
import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
23+
import org.apache.logging.log4j.message.Message;
24+
import org.apache.logging.log4j.message.MessageFactory;
25+
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
26+
import org.junit.jupiter.api.Test;
27+
import org.junitpioneer.jupiter.ClearSystemProperty;
28+
import org.junitpioneer.jupiter.SetSystemProperty;
29+
30+
class LoggerMessageFactoryCustomizationTest {
31+
32+
@Test
33+
@ClearSystemProperty(key = "log4j2.messageFactory")
34+
@ClearSystemProperty(key = "log4j2.flowMessageFactory")
35+
void arguments_should_be_honored() {
36+
final LoggerContext loggerContext =
37+
new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName());
38+
final Logger logger = new Logger(
39+
loggerContext, "arguments_should_be_honored", new TestMessageFactory(), new TestFlowMessageFactory());
40+
assertTestMessageFactories(logger);
41+
}
42+
43+
@Test
44+
@SetSystemProperty(
45+
key = "log4j2.messageFactory",
46+
value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$TestMessageFactory")
47+
@SetSystemProperty(
48+
key = "log4j2.flowMessageFactory",
49+
value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$TestFlowMessageFactory")
50+
void properties_should_be_honored() {
51+
final LoggerContext loggerContext =
52+
new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName());
53+
final Logger logger = new Logger(loggerContext, "properties_should_be_honored", null, null);
54+
assertTestMessageFactories(logger);
55+
}
56+
57+
private static void assertTestMessageFactories(Logger logger) {
58+
assertThat((MessageFactory) logger.getMessageFactory()).isInstanceOf(TestMessageFactory.class);
59+
assertThat(logger.getFlowMessageFactory()).isInstanceOf(TestFlowMessageFactory.class);
60+
}
61+
62+
public static final class TestMessageFactory extends AbstractMessageFactory {
63+
64+
@Override
65+
public Message newMessage(final String message, final Object... params) {
66+
return ParameterizedMessageFactory.INSTANCE.newMessage(message, params);
67+
}
68+
}
69+
70+
public static final class TestFlowMessageFactory extends DefaultFlowMessageFactory {}
71+
}

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaDisabledTest.java

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.core;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
21+
import org.apache.logging.log4j.core.util.Constants;
22+
import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
23+
import org.apache.logging.log4j.message.MessageFactory;
24+
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
25+
import org.junit.jupiter.api.Test;
26+
import org.junitpioneer.jupiter.SetSystemProperty;
27+
28+
class LoggerMessageFactoryDefaultsTlaDisabledTest {
29+
30+
@Test
31+
@SetSystemProperty(key = "log4j2.enableThreadLocals", value = "false")
32+
void defaults_should_match_when_thread_locals_disabled() {
33+
assertThat(Constants.ENABLE_THREADLOCALS).isFalse();
34+
final LoggerContext loggerContext =
35+
new LoggerContext(LoggerMessageFactoryDefaultsTlaDisabledTest.class.getSimpleName());
36+
final Logger logger =
37+
new Logger(loggerContext, "defaults_should_match_when_thread_locals_disabled", null, null);
38+
assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(ParameterizedMessageFactory.INSTANCE);
39+
assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE);
40+
}
41+
}

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaEnabledTest.java

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.core;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
21+
import org.apache.logging.log4j.core.util.Constants;
22+
import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
23+
import org.apache.logging.log4j.message.MessageFactory;
24+
import org.apache.logging.log4j.message.ReusableMessageFactory;
25+
import org.junit.jupiter.api.Test;
26+
import org.junitpioneer.jupiter.SetSystemProperty;
27+
28+
class LoggerMessageFactoryDefaultsTlaEnabledTest {
29+
30+
@Test
31+
@SetSystemProperty(key = "log4j2.is.webapp", value = "false")
32+
@SetSystemProperty(key = "log4j2.enableThreadLocals", value = "true")
33+
void defaults_should_match_when_thread_locals_enabled() {
34+
assertThat(Constants.ENABLE_THREADLOCALS).isTrue();
35+
final LoggerContext loggerContext =
36+
new LoggerContext(LoggerMessageFactoryDefaultsTlaEnabledTest.class.getSimpleName());
37+
final Logger logger = new Logger(loggerContext, "defaults_should_match_when_thread_locals_enabled", null, null);
38+
assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(ReusableMessageFactory.INSTANCE);
39+
assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE);
40+
}
41+
}

‎log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java

+78-7
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
*/
1717
package org.apache.logging.log4j.core;
1818

19+
import static java.util.Objects.requireNonNull;
20+
1921
import java.io.ObjectStreamException;
2022
import java.io.Serializable;
2123
import java.util.ArrayList;
@@ -31,10 +33,16 @@
3133
import org.apache.logging.log4j.core.config.LoggerConfig;
3234
import org.apache.logging.log4j.core.config.ReliabilityStrategy;
3335
import org.apache.logging.log4j.core.filter.CompositeFilter;
36+
import org.apache.logging.log4j.core.util.Constants;
37+
import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
38+
import org.apache.logging.log4j.message.FlowMessageFactory;
3439
import org.apache.logging.log4j.message.Message;
3540
import org.apache.logging.log4j.message.MessageFactory;
41+
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
42+
import org.apache.logging.log4j.message.ReusableMessageFactory;
3643
import org.apache.logging.log4j.message.SimpleMessage;
3744
import org.apache.logging.log4j.spi.AbstractLogger;
45+
import org.apache.logging.log4j.util.LoaderUtil;
3846
import org.apache.logging.log4j.util.Strings;
3947
import org.apache.logging.log4j.util.Supplier;
4048

@@ -54,6 +62,10 @@ public class Logger extends AbstractLogger implements Supplier<LoggerConfig> {
5462

5563
private static final long serialVersionUID = 1L;
5664

65+
private static final String MESSAGE_FACTORY_PROPERTY_NAME = "log4j2.messageFactory";
66+
67+
private static final String FLOW_MESSAGE_FACTORY_PROPERTY_NAME = "log4j2.flowMessageFactory";
68+
5769
/**
5870
* Config should be consistent across threads.
5971
*/
@@ -63,16 +75,75 @@ public class Logger extends AbstractLogger implements Supplier<LoggerConfig> {
6375
private final LoggerContext context;
6476

6577
/**
66-
* The constructor.
78+
* Constructs an instance using the given {@link LoggerContext}, logger name, and {@link MessageFactory}.
6779
*
68-
* @param context The LoggerContext this Logger is associated with.
69-
* @param messageFactory The message factory.
70-
* @param name The name of the Logger.
80+
* @param context the {@link LoggerContext} this logger is associated with
81+
* @param messageFactory The message factory to be used.
82+
* If null, first the {@value #MESSAGE_FACTORY_PROPERTY_NAME} property will be used to instantiate the message factory.
83+
* If the property is missing and the {@code log4j2.enableThreadLocals} property is not {@code false}, {@link ReusableMessageFactory} will be used.
84+
* Otherwise, we will fall back to {@link ParameterizedMessageFactory}.
85+
* @param name the logger name
7186
*/
7287
protected Logger(final LoggerContext context, final String name, final MessageFactory messageFactory) {
73-
super(name, messageFactory);
74-
this.context = context;
75-
privateConfig = new PrivateConfig(context.getConfiguration(), this);
88+
this(context, name, messageFactory, null);
89+
}
90+
91+
/**
92+
* The canonical constructor.
93+
*
94+
* @param context the {@link LoggerContext} this logger is associated with
95+
* @param messageFactory The message factory to be used.
96+
* If null, first the {@value #MESSAGE_FACTORY_PROPERTY_NAME} property will be used to instantiate the message factory.
97+
* If the property is missing and the {@code log4j2.enableThreadLocals} property is not {@code false}, {@link ReusableMessageFactory} will be used.
98+
* Otherwise, we will fall back to {@link ParameterizedMessageFactory}.
99+
* @param flowMessageFactory The flow message factory to be used.
100+
* If null, first the {@value #FLOW_MESSAGE_FACTORY_PROPERTY_NAME} property will be used to instantiate the flow message factory.
101+
* If the property is missing, {@link DefaultFlowMessageFactory} will be used.
102+
* @param name the logger name
103+
*/
104+
protected Logger(
105+
final LoggerContext context,
106+
final String name,
107+
final MessageFactory messageFactory,
108+
final FlowMessageFactory flowMessageFactory) {
109+
super(name, getEffectiveMessageFactory(messageFactory), getEffectiveFlowMessageFactory(flowMessageFactory));
110+
this.context = requireNonNull(context, "context");
111+
this.privateConfig = new PrivateConfig(context.getConfiguration(), this);
112+
}
113+
114+
private static MessageFactory getEffectiveMessageFactory(final MessageFactory messageFactory) {
115+
return createInstanceFromFactoryProperty(
116+
MessageFactory.class,
117+
messageFactory,
118+
MESSAGE_FACTORY_PROPERTY_NAME,
119+
() -> Constants.ENABLE_THREADLOCALS
120+
? ReusableMessageFactory.INSTANCE
121+
: ParameterizedMessageFactory.INSTANCE);
122+
}
123+
124+
private static FlowMessageFactory getEffectiveFlowMessageFactory(final FlowMessageFactory flowMessageFactory) {
125+
return createInstanceFromFactoryProperty(
126+
FlowMessageFactory.class,
127+
flowMessageFactory,
128+
FLOW_MESSAGE_FACTORY_PROPERTY_NAME,
129+
() -> DefaultFlowMessageFactory.INSTANCE);
130+
}
131+
132+
private static <V> V createInstanceFromFactoryProperty(
133+
final Class<V> instanceType,
134+
final V providedInstance,
135+
final String propertyName,
136+
final java.util.function.Supplier<V> fallbackInstanceSupplier) {
137+
if (providedInstance != null) {
138+
return providedInstance;
139+
}
140+
try {
141+
return LoaderUtil.newCheckedInstanceOfProperty(propertyName, instanceType, fallbackInstanceSupplier);
142+
} catch (final Exception error) {
143+
final String message =
144+
String.format("failed instantiating the class pointed by the `%s` property", propertyName);
145+
throw new RuntimeException(message, error);
146+
}
76147
}
77148

78149
protected Object writeReplace() throws ObjectStreamException {

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

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3+
xmlns="https://logging.apache.org/xml/ns"
4+
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
5+
type="fixed">
6+
<issue id="2505" link="https://github.com/apache/logging-log4j2/pull/2505"/>
7+
<description format="asciidoc">Fix handling of `log4j2.messageFactory` and `log4j2.flowMessageFactory` properties</description>
8+
</entry>

0 commit comments

Comments
 (0)