Skip to content

Commit c369ac2

Browse files
ppkarwaszppkarwasz
committed
Merge branch '2.24.x' into 2.x
2 parents 0f48f60 + a76a83f commit c369ac2

File tree

17 files changed

+350
-153
lines changed

17 files changed

+350
-153
lines changed

‎log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilTest.java

+68-1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@
3838
import java.util.Map;
3939
import java.util.Properties;
4040
import java.util.stream.Stream;
41+
import org.apache.logging.log4j.Level;
42+
import org.apache.logging.log4j.test.ListStatusListener;
43+
import org.apache.logging.log4j.test.junit.UsingStatusListener;
4144
import org.junit.jupiter.api.BeforeEach;
4245
import org.junit.jupiter.api.Test;
4346
import org.junit.jupiter.api.parallel.ResourceAccessMode;
@@ -193,16 +196,56 @@ void testPublish() {
193196
@Test
194197
@ResourceLock(value = Resources.SYSTEM_PROPERTIES, mode = ResourceAccessMode.READ)
195198
@Issue("https://github.com/spring-projects/spring-boot/issues/33450")
196-
void testBadPropertySource() {
199+
@UsingStatusListener
200+
void testErrorPropertySource(ListStatusListener statusListener) {
197201
final String key = "testKey";
198202
final Properties props = new Properties();
199203
props.put(key, "test");
200204
final PropertiesUtil util = new PropertiesUtil(props);
201205
final ErrorPropertySource source = new ErrorPropertySource();
202206
util.addPropertySource(source);
203207
try {
208+
statusListener.clear();
204209
assertEquals("test", util.getStringProperty(key));
205210
assertTrue(source.exceptionThrown);
211+
assertThat(statusListener.findStatusData(Level.WARN))
212+
.anySatisfy(data ->
213+
assertThat(data.getMessage().getFormattedMessage()).contains("Failed"));
214+
} finally {
215+
util.removePropertySource(source);
216+
}
217+
}
218+
219+
@Test
220+
@ResourceLock(value = Resources.SYSTEM_PROPERTIES, mode = ResourceAccessMode.READ)
221+
@Issue("https://github.com/apache/logging-log4j2/issues/3252")
222+
@UsingStatusListener
223+
void testRecursivePropertySource(ListStatusListener statusListener) {
224+
final String key = "testKey";
225+
final Properties props = new Properties();
226+
props.put(key, "test");
227+
final PropertiesUtil util = new PropertiesUtil(props);
228+
final PropertySource source = new RecursivePropertySource(util);
229+
util.addPropertySource(source);
230+
try {
231+
// We ignore the recursive source
232+
statusListener.clear();
233+
assertThat(util.getStringProperty(key)).isEqualTo("test");
234+
assertThat(statusListener.findStatusData(Level.WARN))
235+
.anySatisfy(data -> assertThat(data.getMessage().getFormattedMessage())
236+
.contains("Recursive call", "getProperty"));
237+
238+
statusListener.clear();
239+
// To check for existence, the sources are looked up in a random order.
240+
assertThat(util.hasProperty(key)).isTrue();
241+
// To find a missing key, all the sources must be used.
242+
assertThat(util.hasProperty("noSuchKey")).isFalse();
243+
assertThat(statusListener.findStatusData(Level.WARN))
244+
.anySatisfy(data -> assertThat(data.getMessage().getFormattedMessage())
245+
.contains("Recursive call", "containsProperty"));
246+
// We check that the source is recursive
247+
assertThat(source.getProperty(key)).isEqualTo("test");
248+
assertThat(source.containsProperty(key)).isTrue();
206249
} finally {
207250
util.removePropertySource(source);
208251
}
@@ -289,4 +332,28 @@ public boolean containsProperty(final String key) {
289332
throw new IllegalStateException("Test");
290333
}
291334
}
335+
336+
private static class RecursivePropertySource implements PropertySource {
337+
338+
private final PropertiesUtil propertiesUtil;
339+
340+
private RecursivePropertySource(PropertiesUtil propertiesUtil) {
341+
this.propertiesUtil = propertiesUtil;
342+
}
343+
344+
@Override
345+
public int getPriority() {
346+
return Integer.MIN_VALUE;
347+
}
348+
349+
@Override
350+
public String getProperty(String key) {
351+
return propertiesUtil.getStringProperty(key);
352+
}
353+
354+
@Override
355+
public boolean containsProperty(String key) {
356+
return propertiesUtil.hasProperty(key);
357+
}
358+
}
292359
}

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

+8-8
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import static java.util.Objects.requireNonNull;
2020

21-
import java.util.ArrayList;
2221
import java.util.Collection;
2322
import java.util.Collections;
2423
import java.util.HashMap;
@@ -28,6 +27,7 @@
2827
import java.util.concurrent.locks.Lock;
2928
import java.util.concurrent.locks.ReadWriteLock;
3029
import java.util.concurrent.locks.ReentrantReadWriteLock;
30+
import java.util.stream.Collectors;
3131
import org.apache.logging.log4j.Logger;
3232
import org.apache.logging.log4j.message.MessageFactory;
3333
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
@@ -171,19 +171,19 @@ public LoggerRegistry(@Nullable final MapFactory<T> mapFactory) {
171171
}
172172

173173
public Collection<T> getLoggers() {
174-
return getLoggers(new ArrayList<>());
175-
}
176-
177-
public Collection<T> getLoggers(final Collection<T> destination) {
178-
requireNonNull(destination, "destination");
179174
readLock.lock();
180175
try {
181-
loggerByMessageFactoryByName.values().stream()
176+
return loggerByMessageFactoryByName.values().stream()
182177
.flatMap(loggerByMessageFactory -> loggerByMessageFactory.values().stream())
183-
.forEach(destination::add);
178+
.collect(Collectors.toList());
184179
} finally {
185180
readLock.unlock();
186181
}
182+
}
183+
184+
public Collection<T> getLoggers(final Collection<T> destination) {
185+
requireNonNull(destination, "destination");
186+
destination.addAll(getLoggers());
187187
return destination;
188188
}
189189

‎log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java

+25-10
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,7 @@ public void reload() {}
496496
private static final class Environment {
497497

498498
private final Set<PropertySource> sources = ConcurrentHashMap.newKeySet();
499+
private final ThreadLocal<PropertySource> CURRENT_PROPERTY_SOURCE = new ThreadLocal<>();
499500

500501
private Environment(final PropertySource propertySource) {
501502
final PropertySource sysProps = new PropertyFilePropertySource(LOG4J_SYSTEM_PROPERTIES_FILE_NAME, false);
@@ -547,21 +548,35 @@ private String get(final String key) {
547548
}
548549

549550
private boolean sourceContainsProperty(final PropertySource source, final String key) {
550-
try {
551-
return source.containsProperty(key);
552-
} catch (final Exception e) {
553-
LOGGER.warn("Failed to retrieve Log4j property {} from property source {}.", key, source, e);
554-
return false;
551+
PropertySource recursiveSource = CURRENT_PROPERTY_SOURCE.get();
552+
if (recursiveSource == null) {
553+
CURRENT_PROPERTY_SOURCE.set(source);
554+
try {
555+
return source.containsProperty(key);
556+
} catch (final Exception e) {
557+
LOGGER.warn("Failed to retrieve Log4j property {} from property source {}.", key, source, e);
558+
} finally {
559+
CURRENT_PROPERTY_SOURCE.remove();
560+
}
555561
}
562+
LOGGER.warn("Recursive call to `containsProperty()` from property source {}.", recursiveSource);
563+
return false;
556564
}
557565

558566
private String sourceGetProperty(final PropertySource source, final String key) {
559-
try {
560-
return source.getProperty(key);
561-
} catch (final Exception e) {
562-
LOGGER.warn("Failed to retrieve Log4j property {} from property source {}.", key, source, e);
563-
return null;
567+
PropertySource recursiveSource = CURRENT_PROPERTY_SOURCE.get();
568+
if (recursiveSource == null) {
569+
CURRENT_PROPERTY_SOURCE.set(source);
570+
try {
571+
return source.getProperty(key);
572+
} catch (final Exception e) {
573+
LOGGER.warn("Failed to retrieve Log4j property {} from property source {}.", key, source, e);
574+
} finally {
575+
CURRENT_PROPERTY_SOURCE.remove();
576+
}
564577
}
578+
LOGGER.warn("Recursive call to `getProperty()` from property source {}.", recursiveSource);
579+
return null;
565580
}
566581

567582
private boolean containsKey(final String key) {

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

+38
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,23 @@
1919
import static org.assertj.core.api.Assertions.assertThat;
2020
import static org.mockito.Mockito.mock;
2121

22+
import java.util.Collection;
23+
import java.util.concurrent.ExecutorService;
24+
import java.util.concurrent.Executors;
25+
import java.util.concurrent.Future;
26+
import java.util.stream.Collectors;
27+
import java.util.stream.IntStream;
2228
import org.apache.logging.log4j.message.MessageFactory;
2329
import org.apache.logging.log4j.message.MessageFactory2;
30+
import org.junit.jupiter.api.Assertions;
2431
import org.junit.jupiter.api.Test;
2532
import org.junit.jupiter.api.TestInfo;
2633

2734
class LoggerContextTest {
2835

36+
private static final int LOGGER_COUNT = 1024;
37+
private static final int CONCURRENCY_LEVEL = 16;
38+
2939
@Test
3040
void newInstance_should_honor_name_and_message_factory(final TestInfo testInfo) {
3141
final String testName = testInfo.getDisplayName();
@@ -37,4 +47,32 @@ void newInstance_should_honor_name_and_message_factory(final TestInfo testInfo)
3747
assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(messageFactory);
3848
}
3949
}
50+
51+
@Test
52+
void getLoggers_can_be_updated_concurrently(final TestInfo testInfo) {
53+
final String testName = testInfo.getDisplayName();
54+
final ExecutorService executorService = Executors.newFixedThreadPool(CONCURRENCY_LEVEL);
55+
try (LoggerContext loggerContext = new LoggerContext(testName)) {
56+
// Create a logger
57+
Collection<Future<?>> tasks = IntStream.range(0, CONCURRENCY_LEVEL)
58+
.mapToObj(i -> executorService.submit(() -> {
59+
// Iterates over loggers
60+
loggerContext.updateLoggers();
61+
// Create some loggers
62+
for (int j = 0; j < LOGGER_COUNT; j++) {
63+
loggerContext.getLogger(testName + "-" + i + "-" + j);
64+
}
65+
// Iterate over loggers again
66+
loggerContext.updateLoggers();
67+
}))
68+
.collect(Collectors.toList());
69+
Assertions.assertDoesNotThrow(() -> {
70+
for (Future<?> task : tasks) {
71+
task.get();
72+
}
73+
});
74+
} finally {
75+
executorService.shutdown();
76+
}
77+
}
4078
}

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

+34-25
Original file line numberDiff line numberDiff line change
@@ -20,52 +20,61 @@
2020

2121
import org.apache.logging.log4j.message.AbstractMessageFactory;
2222
import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
23+
import org.apache.logging.log4j.message.FlowMessageFactory;
2324
import org.apache.logging.log4j.message.Message;
2425
import org.apache.logging.log4j.message.MessageFactory;
2526
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
2627
import org.junit.jupiter.api.Test;
27-
import org.junitpioneer.jupiter.ClearSystemProperty;
28+
import org.junit.jupiter.api.TestInfo;
2829
import org.junitpioneer.jupiter.SetSystemProperty;
2930

31+
@SetSystemProperty(
32+
key = "log4j2.messageFactory",
33+
value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$AlternativeTestMessageFactory")
34+
@SetSystemProperty(
35+
key = "log4j2.flowMessageFactory",
36+
value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$AlternativeTestFlowMessageFactory")
3037
class LoggerMessageFactoryCustomizationTest {
3138

3239
@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);
40+
void arguments_should_be_honored(TestInfo testInfo) {
41+
try (LoggerContext loggerContext =
42+
new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName())) {
43+
Logger logger = new Logger(
44+
loggerContext, testInfo.getDisplayName(), new TestMessageFactory(), new TestFlowMessageFactory());
45+
assertTestMessageFactories(logger, TestMessageFactory.class, TestFlowMessageFactory.class);
46+
}
4147
}
4248

4349
@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);
50+
void properties_should_be_honored(TestInfo testInfo) {
51+
try (LoggerContext loggerContext =
52+
new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName())) {
53+
Logger logger = loggerContext.getLogger(testInfo.getDisplayName());
54+
assertTestMessageFactories(
55+
logger, AlternativeTestMessageFactory.class, AlternativeTestFlowMessageFactory.class);
56+
}
5557
}
5658

57-
private static void assertTestMessageFactories(Logger logger) {
58-
assertThat((MessageFactory) logger.getMessageFactory()).isInstanceOf(TestMessageFactory.class);
59-
assertThat(logger.getFlowMessageFactory()).isInstanceOf(TestFlowMessageFactory.class);
59+
private static void assertTestMessageFactories(
60+
Logger logger,
61+
Class<? extends MessageFactory> messageFactoryClass,
62+
Class<? extends FlowMessageFactory> flowMessageFactoryClass) {
63+
assertThat(logger.getMessageFactory().getClass()).isEqualTo(messageFactoryClass);
64+
assertThat(logger.getFlowMessageFactory().getClass()).isEqualTo(flowMessageFactoryClass);
6065
}
6166

62-
public static final class TestMessageFactory extends AbstractMessageFactory {
67+
public static class TestMessageFactory extends AbstractMessageFactory {
6368

6469
@Override
6570
public Message newMessage(final String message, final Object... params) {
6671
return ParameterizedMessageFactory.INSTANCE.newMessage(message, params);
6772
}
6873
}
6974

70-
public static final class TestFlowMessageFactory extends DefaultFlowMessageFactory {}
75+
public static class AlternativeTestMessageFactory extends TestMessageFactory {}
76+
77+
public static class TestFlowMessageFactory extends DefaultFlowMessageFactory {}
78+
79+
public static class AlternativeTestFlowMessageFactory extends TestFlowMessageFactory {}
7180
}

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

+8-7
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,20 @@
2323
import org.apache.logging.log4j.message.MessageFactory;
2424
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
2525
import org.junit.jupiter.api.Test;
26+
import org.junit.jupiter.api.TestInfo;
2627
import org.junitpioneer.jupiter.SetSystemProperty;
2728

2829
class LoggerMessageFactoryDefaultsTlaDisabledTest {
2930

3031
@Test
3132
@SetSystemProperty(key = "log4j2.enableThreadLocals", value = "false")
32-
void defaults_should_match_when_thread_locals_disabled() {
33+
void defaults_should_match_when_thread_locals_disabled(TestInfo testInfo) {
3334
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);
35+
try (LoggerContext loggerContext =
36+
new LoggerContext(LoggerMessageFactoryDefaultsTlaDisabledTest.class.getSimpleName())) {
37+
final Logger logger = loggerContext.getLogger(testInfo.getDisplayName());
38+
assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(ParameterizedMessageFactory.INSTANCE);
39+
assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE);
40+
}
4041
}
4142
}

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

+10-8
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,21 @@
2323
import org.apache.logging.log4j.message.MessageFactory;
2424
import org.apache.logging.log4j.message.ReusableMessageFactory;
2525
import org.junit.jupiter.api.Test;
26+
import org.junit.jupiter.api.TestInfo;
2627
import org.junitpioneer.jupiter.SetSystemProperty;
2728

2829
class LoggerMessageFactoryDefaultsTlaEnabledTest {
2930

3031
@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() {
32+
@SetSystemProperty(key = "log4j2.isWebapp", value = "false")
33+
@SetSystemProperty(key = "log4j2.enableThreadlocals", value = "true")
34+
void defaults_should_match_when_thread_locals_enabled(TestInfo testInfo) {
3435
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);
36+
try (LoggerContext loggerContext =
37+
new LoggerContext(LoggerMessageFactoryDefaultsTlaEnabledTest.class.getSimpleName())) {
38+
Logger logger = loggerContext.getLogger(testInfo.getDisplayName());
39+
assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(ReusableMessageFactory.INSTANCE);
40+
assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE);
41+
}
4042
}
4143
}

0 commit comments

Comments
 (0)