Skip to content

Commit 395a8f4

Browse files
ppkarwaszppkarwasz
committed
Fix handling of LoggerContextAware lookups
Due to the changes in #2278 `LoggerContextAware` lookups stopped working in `2.23.0`. This PR: * fixes the NPE in `Interpolator` that occurs if `Interpolator#setLoggerContext` was **not** called after instantiation. * Calls `Interpolator#setConfiguration` and `Interpolator#setLoggerContext` wherever it is possible. * Changes the way `Interpolator` propagates `Configuration` and `LoggerContext` to child lookups. Previously this occurred at each evaluation, now it occurs only in the setters. Closes #2309.
1 parent 28b70d1 commit 395a8f4

File tree

7 files changed

+202
-78
lines changed

7 files changed

+202
-78
lines changed

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
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.config;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
import static org.mockito.Mockito.mock;
21+
22+
import java.util.Arrays;
23+
import java.util.Collections;
24+
import java.util.Map;
25+
import java.util.Map.Entry;
26+
import org.apache.logging.log4j.core.LoggerContext;
27+
import org.apache.logging.log4j.core.lookup.Interpolator;
28+
import org.apache.logging.log4j.core.lookup.InterpolatorTest;
29+
import org.apache.logging.log4j.core.lookup.StrSubstitutor;
30+
import org.junit.jupiter.api.Test;
31+
import org.junit.jupiter.params.ParameterizedTest;
32+
import org.junit.jupiter.params.provider.ValueSource;
33+
import org.junitpioneer.jupiter.Issue;
34+
35+
class AbstractConfigurationTest {
36+
37+
@Test
38+
void propertiesCanComeLast() {
39+
final Configuration config = new TestConfiguration(null, Collections.singletonMap("console.name", "CONSOLE"));
40+
config.initialize();
41+
final StrSubstitutor substitutor = config.getStrSubstitutor();
42+
assertThat(substitutor.replace("${console.name}"))
43+
.as("No interpolation for '${console.name}'")
44+
.isEqualTo("CONSOLE");
45+
}
46+
47+
@ParameterizedTest
48+
@ValueSource(booleans = {false, true})
49+
@Issue("https://github.com/apache/logging-log4j2/issues/2309")
50+
void substitutorHasConfigurationAndLoggerContext(final boolean hasProperties) {
51+
final LoggerContext context = mock(LoggerContext.class);
52+
final Configuration config = new TestConfiguration(context, hasProperties ? Collections.emptyMap() : null);
53+
config.initialize();
54+
final Interpolator runtime = (Interpolator) config.getStrSubstitutor().getVariableResolver();
55+
final Interpolator configTime =
56+
(Interpolator) config.getConfigurationStrSubstitutor().getVariableResolver();
57+
for (final Interpolator interpolator : Arrays.asList(runtime, configTime)) {
58+
assertThat(InterpolatorTest.getConfiguration(interpolator)).isEqualTo(config);
59+
assertThat(InterpolatorTest.getLoggerContext(interpolator)).isEqualTo(context);
60+
}
61+
}
62+
63+
private static class TestConfiguration extends AbstractConfiguration {
64+
65+
private final Map<String, String> map;
66+
67+
public TestConfiguration(final LoggerContext context, final Map<String, String> map) {
68+
super(context, ConfigurationSource.NULL_SOURCE);
69+
this.map = map;
70+
}
71+
72+
@Override
73+
public void setup() {
74+
// Nodes
75+
final Node loggers = newNode(rootNode, "Loggers");
76+
rootNode.getChildren().add(loggers);
77+
78+
final Node rootLogger = newNode(loggers, "Root");
79+
rootLogger.getAttributes().put("level", "INFO");
80+
loggers.getChildren().add(rootLogger);
81+
82+
if (map != null) {
83+
final Node properties = newNode(rootNode, "Properties");
84+
rootNode.getChildren().add(properties);
85+
86+
for (final Entry<String, String> entry : map.entrySet()) {
87+
final Node property = newNode(properties, "Property");
88+
property.getAttributes().put("name", entry.getKey());
89+
property.getAttributes().put("value", entry.getValue());
90+
properties.getChildren().add(property);
91+
}
92+
}
93+
}
94+
95+
private Node newNode(final Node parent, final String name) {
96+
return new Node(parent, name, pluginManager.getPluginType(name));
97+
}
98+
}
99+
}

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

Lines changed: 0 additions & 67 deletions
This file was deleted.

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,14 @@
1616
*/
1717
package org.apache.logging.log4j.core.lookup;
1818

19+
import static org.assertj.core.api.Assertions.assertThat;
1920
import static org.junit.Assert.assertFalse;
2021
import static org.junit.Assert.assertNotNull;
2122
import static org.junit.Assert.assertSame;
23+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
2224
import static org.junit.jupiter.api.Assertions.assertEquals;
2325
import static org.junit.jupiter.api.Assertions.assertNull;
26+
import static org.mockito.Mockito.mock;
2427

2528
import java.text.SimpleDateFormat;
2629
import java.util.Collections;
@@ -31,18 +34,24 @@
3134
import org.apache.logging.log4j.ThreadContext;
3235
import org.apache.logging.log4j.core.LogEvent;
3336
import org.apache.logging.log4j.core.Logger;
37+
import org.apache.logging.log4j.core.LoggerContext;
38+
import org.apache.logging.log4j.core.config.Configuration;
39+
import org.apache.logging.log4j.core.config.LoggerContextAware;
40+
import org.apache.logging.log4j.core.config.plugins.Plugin;
3441
import org.apache.logging.log4j.core.impl.Log4jLogEvent;
3542
import org.apache.logging.log4j.core.test.junit.JndiRule;
3643
import org.apache.logging.log4j.message.StringMapMessage;
3744
import org.junit.ClassRule;
3845
import org.junit.Test;
3946
import org.junit.rules.ExternalResource;
4047
import org.junit.rules.RuleChain;
48+
import org.junitpioneer.jupiter.Issue;
4149

4250
/**
4351
* Tests {@link Interpolator}.
4452
*/
4553
public class InterpolatorTest {
54+
public static final String TEST_LOOKUP = "interpolator_test";
4655

4756
private static final String TESTKEY = "TestKey";
4857
private static final String TESTKEY2 = "TestKey2";
@@ -177,4 +186,66 @@ public void testInterpolatorMapMessageWithMapPrefix() {
177186
.build();
178187
assertEquals("mapMessage", interpolator.lookup(event, "map:key"));
179188
}
189+
190+
@Test
191+
@Issue("https://github.com/apache/logging-log4j2/issues/2309")
192+
public void testContextAndConfigurationPropagation() {
193+
final Interpolator interpolator = new Interpolator();
194+
assertThat(getConfiguration(interpolator)).isNull();
195+
assertThat(getLoggerContext(interpolator)).isNull();
196+
197+
final Lookup lookup = (Lookup) interpolator.getStrLookupMap().get(TEST_LOOKUP);
198+
assertThat(lookup)
199+
.isNotNull()
200+
.as("Configuration and logger context are null")
201+
.extracting(Lookup::getConfiguration, Lookup::getLoggerContext)
202+
.containsExactly(null, null);
203+
204+
// Evaluation does not throw, even if config and context are null.
205+
assertDoesNotThrow(() -> interpolator.evaluate(null, TEST_LOOKUP + ":any_key"));
206+
207+
final Configuration config = mock(Configuration.class);
208+
interpolator.setConfiguration(config);
209+
assertThat(getConfiguration(interpolator)).isEqualTo(config);
210+
assertThat(lookup.getConfiguration()).as("Configuration propagates").isEqualTo(config);
211+
212+
final LoggerContext context = mock(LoggerContext.class);
213+
interpolator.setLoggerContext(context);
214+
assertThat(getLoggerContext(interpolator)).isEqualTo(context);
215+
assertThat(lookup.getLoggerContext()).as("Logger context propagates").isEqualTo(context);
216+
}
217+
218+
// Used in tests from other packages
219+
public static Configuration getConfiguration(final Interpolator interpolator) {
220+
return interpolator.configuration;
221+
}
222+
223+
// Used in tests from other packages
224+
public static LoggerContext getLoggerContext(final Interpolator interpolator) {
225+
return interpolator.loggerContext.get();
226+
}
227+
228+
@Plugin(name = TEST_LOOKUP, category = StrLookup.CATEGORY)
229+
public static class Lookup extends AbstractConfigurationAwareLookup implements LoggerContextAware {
230+
231+
private LoggerContext loggerContext;
232+
233+
public Configuration getConfiguration() {
234+
return configuration;
235+
}
236+
237+
public LoggerContext getLoggerContext() {
238+
return loggerContext;
239+
}
240+
241+
@Override
242+
public void setLoggerContext(final LoggerContext loggerContext) {
243+
this.loggerContext = loggerContext;
244+
}
245+
246+
@Override
247+
public String lookup(final LogEvent event, final String key) {
248+
return null;
249+
}
250+
}
180251
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,7 @@ protected void doConfigure() {
665665
final Map<String, String> map = this.getComponent(CONTEXT_PROPERTIES);
666666
final StrLookup lookup = map == null ? null : new PropertiesLookup(map);
667667
final Interpolator interpolator = new Interpolator(lookup, pluginPackages);
668+
interpolator.setConfiguration(this);
668669
interpolator.setLoggerContext(loggerContext.get());
669670
runtimeStrSubstitutor.setVariableResolver(interpolator);
670671
configurationStrSubstitutor.setVariableResolver(interpolator);

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,11 @@ public static StrLookup configureSubstitutor(
5252
for (int i = 0; i < unescapedProperties.length; i++) {
5353
unescapedProperties[i] = unescape(properties[i]);
5454
}
55-
return new Interpolator(
55+
final Interpolator interpolator = new Interpolator(
5656
new PropertiesLookup(unescapedProperties, config.getProperties()), config.getPluginPackages());
57+
interpolator.setConfiguration(config);
58+
interpolator.setLoggerContext(config.getLoggerContext());
59+
return interpolator;
5760
}
5861

5962
private static Property unescape(final Property input) {

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

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.apache.logging.log4j.Logger;
2727
import org.apache.logging.log4j.core.LogEvent;
2828
import org.apache.logging.log4j.core.LoggerContext;
29+
import org.apache.logging.log4j.core.config.Configuration;
2930
import org.apache.logging.log4j.core.config.ConfigurationAware;
3031
import org.apache.logging.log4j.core.config.LoggerContextAware;
3132
import org.apache.logging.log4j.core.config.plugins.util.PluginManager;
@@ -60,7 +61,7 @@ public class Interpolator extends AbstractConfigurationAwareLookup implements Lo
6061

6162
private final StrLookup defaultLookup;
6263

63-
protected WeakReference<LoggerContext> loggerContext;
64+
protected WeakReference<LoggerContext> loggerContext = new WeakReference<>(null);
6465

6566
public Interpolator(final StrLookup defaultLookup) {
6667
this(defaultLookup, null);
@@ -191,12 +192,6 @@ public LookupResult evaluate(final LogEvent event, String var) {
191192
final String prefix = toRootLowerCase(var.substring(0, prefixPos));
192193
final String name = var.substring(prefixPos + 1);
193194
final StrLookup lookup = strLookupMap.get(prefix);
194-
if (lookup instanceof ConfigurationAware) {
195-
((ConfigurationAware) lookup).setConfiguration(configuration);
196-
}
197-
if (lookup instanceof LoggerContextAware) {
198-
((LoggerContextAware) lookup).setLoggerContext(loggerContext.get());
199-
}
200195
LookupResult value = null;
201196
if (lookup != null) {
202197
value = event == null ? lookup.evaluate(name) : lookup.evaluate(event, name);
@@ -214,11 +209,25 @@ public LookupResult evaluate(final LogEvent event, String var) {
214209
}
215210

216211
@Override
217-
public void setLoggerContext(final LoggerContext loggerContext) {
218-
if (loggerContext == null) {
219-
return;
212+
public void setConfiguration(final Configuration configuration) {
213+
super.setConfiguration(configuration);
214+
// Propagate
215+
for (final StrLookup lookup : strLookupMap.values()) {
216+
if (lookup instanceof ConfigurationAware) {
217+
((ConfigurationAware) lookup).setConfiguration(configuration);
218+
}
220219
}
220+
}
221+
222+
@Override
223+
public void setLoggerContext(final LoggerContext loggerContext) {
221224
this.loggerContext = new WeakReference<>(loggerContext);
225+
// Propagate
226+
for (final StrLookup lookup : strLookupMap.values()) {
227+
if (lookup instanceof LoggerContextAware) {
228+
((LoggerContextAware) lookup).setLoggerContext(loggerContext);
229+
}
230+
}
222231
}
223232

224233
@Override

‎src/changelog/.2.x.x/2309_fix_logger_context_aware_lookups.xml

Lines changed: 8 additions & 0 deletions
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="http://logging.apache.org/log4j/changelog"
4+
xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.3.xsd"
5+
type="changed">
6+
<issue id="2309" link="https://github.com/apache/logging-log4j2/pull/2309"/>
7+
<description format="asciidoc">Fix handling of `LoggerContextAware` lookups.</description>
8+
</entry>

0 commit comments

Comments
 (0)