LOG4J2-3080 Use SimpleMessage in Log4j 1 Category whenever possible. (#504)
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/Category.java b/log4j-1.2-api/src/main/java/org/apache/log4j/Category.java
index 755cd73..a17697a 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/Category.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/Category.java
@@ -30,6 +30,7 @@
import org.apache.log4j.spi.LoggerFactory;
import org.apache.log4j.spi.LoggingEvent;
import org.apache.logging.log4j.message.MapMessage;
+import org.apache.logging.log4j.message.SimpleMessage;
import org.apache.logging.log4j.spi.ExtendedLogger;
import org.apache.logging.log4j.spi.LoggerContext;
import org.apache.logging.log4j.message.LocalizedMessage;
@@ -504,11 +505,27 @@
}
}
- private void maybeLog(final String fqcn, final org.apache.logging.log4j.Level level,
- final Object message, final Throwable throwable) {
+ private void maybeLog(
+ final String fqcn,
+ final org.apache.logging.log4j.Level level,
+ final Object message,
+ final Throwable throwable) {
if (logger.isEnabled(level)) {
- @SuppressWarnings("unchecked")
- Message msg = message instanceof Map ? new MapMessage((Map) message) : new ObjectMessage(message);
+ final Message msg;
+ if (message instanceof String) {
+ msg = new SimpleMessage((String) message);
+ }
+ // SimpleMessage treats String and CharSequence differently, hence
+ // this else-if block.
+ else if (message instanceof CharSequence) {
+ msg = new SimpleMessage((CharSequence) message);
+ } else if (message instanceof Map) {
+ @SuppressWarnings("unchecked")
+ final Map<String, ?> map = (Map<String, ?>) message;
+ msg = new MapMessage<>(map);
+ } else {
+ msg = new ObjectMessage(message);
+ }
if (logger instanceof ExtendedLogger) {
((ExtendedLogger) logger).logMessage(fqcn, level, null, msg, throwable);
} else {
diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/CategoryTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/CategoryTest.java
index 1adbafc..9bc72e3 100644
--- a/log4j-1.2-api/src/test/java/org/apache/log4j/CategoryTest.java
+++ b/log4j-1.2-api/src/test/java/org/apache/log4j/CategoryTest.java
@@ -17,20 +17,15 @@
package org.apache.log4j;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-
-import java.lang.reflect.Method;
-import java.util.List;
-
import org.apache.logging.log4j.core.Layout;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.LoggerContext;
import org.apache.logging.log4j.core.config.ConfigurationFactory;
import org.apache.logging.log4j.core.layout.PatternLayout;
+import org.apache.logging.log4j.message.MapMessage;
import org.apache.logging.log4j.message.Message;
import org.apache.logging.log4j.message.ObjectMessage;
+import org.apache.logging.log4j.message.SimpleMessage;
import org.apache.logging.log4j.core.test.appender.ListAppender;
import org.apache.logging.log4j.util.Strings;
import org.junit.AfterClass;
@@ -38,6 +33,15 @@
import org.junit.BeforeClass;
import org.junit.Test;
+import java.lang.reflect.Method;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Consumer;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
/**
* Tests of Category.
@@ -65,7 +69,7 @@
public void before() {
appender.clear();
}
-
+
/**
* Tests Category.forcedLog.
*/
@@ -118,7 +122,7 @@
logger.setLevel(Level.ERROR);
final Priority debug = Level.DEBUG;
logger.l7dlog(debug, "Hello, World", null);
- assertTrue(appender.getEvents().size() == 0);
+ assertTrue(appender.getEvents().isEmpty());
}
/**
@@ -130,7 +134,7 @@
logger.setLevel(Level.ERROR);
final Priority debug = Level.DEBUG;
logger.l7dlog(debug, "Hello, World", new Object[0], null);
- assertTrue(appender.getEvents().size() == 0);
+ assertTrue(appender.getEvents().isEmpty());
}
/**
@@ -149,7 +153,7 @@
// the next line will throw an exception if the LogManager loggers
// aren't supported by 1.2 Logger/Category
logger.l7dlog(debug, "Hello, World", new Object[0], null);
- assertTrue(appender.getEvents().size() == 0);
+ assertTrue(appender.getEvents().isEmpty());
}
/**
@@ -175,7 +179,7 @@
public void testSetPriorityNull() {
Logger.getLogger("org.example.foo").setPriority(null);
}
-
+
@Test
public void testClassName() {
final Category category = Category.getInstance("TestCategory");
@@ -194,6 +198,97 @@
assertTrue("Incorrect message " + Strings.dquote(msg) + " expected " + Strings.dquote(expected), msg.endsWith(expected));
}
+ @Test
+ public void testStringLog() {
+ final String payload = "payload";
+ testMessageImplementation(
+ payload,
+ SimpleMessage.class,
+ message -> assertEquals(message.getFormattedMessage(), payload));
+ }
+
+ @Test
+ public void testCharSequenceLog() {
+ final CharSequence payload = new CharSequence() {
+
+ @Override
+ public int length() {
+ return 3;
+ }
+
+ @Override
+ public char charAt(final int index) {
+ return "abc".charAt(index);
+ }
+
+ @Override
+ public CharSequence subSequence(final int start, final int end) {
+ return "abc".subSequence(start, end);
+ }
+
+ @Override
+ public String toString() {
+ return "abc";
+ }
+
+ };
+ testMessageImplementation(
+ payload,
+ SimpleMessage.class,
+ message -> assertEquals(message.getFormattedMessage(), payload.toString()));
+ }
+
+ @Test
+ public void testMapLog() {
+ final String key = "key";
+ final Object value = 0xDEADBEEF;
+ final Map<String, Object> payload = Collections.singletonMap(key, value);
+ testMessageImplementation(
+ payload,
+ MapMessage.class,
+ message -> assertEquals(message.getData(), payload));
+ }
+
+ @Test
+ public void testObjectLog() {
+ final Object payload = new Object();
+ testMessageImplementation(
+ payload,
+ ObjectMessage.class,
+ message -> assertEquals(message.getParameter(), payload));
+ }
+
+ private <M extends Message> void testMessageImplementation(
+ final Object messagePayload,
+ final Class<M> expectedMessageClass,
+ final Consumer<M> messageTester) {
+
+ // Setup the logger and the appender.
+ final Category category = Category.getInstance("TestCategory");
+ final org.apache.logging.log4j.core.Logger logger =
+ (org.apache.logging.log4j.core.Logger) category.getLogger();
+ logger.addAppender(appender);
+
+ // Log the message payload.
+ category.info(messagePayload);
+
+ // Verify collected log events.
+ final List<LogEvent> events = appender.getEvents();
+ assertEquals("was expecting a single event", 1, events.size());
+ final LogEvent logEvent = events.get(0);
+
+ // Verify the collected message.
+ final Message message = logEvent.getMessage();
+ final Class<? extends Message> actualMessageClass = message.getClass();
+ assertTrue(
+ "was expecting message to be instance of " + expectedMessageClass + ", found: " + actualMessageClass,
+ expectedMessageClass.isAssignableFrom(actualMessageClass));
+ @SuppressWarnings("unchecked")
+ final M typedMessage = (M) message;
+ messageTester.accept(typedMessage);
+
+ }
+
/**
* Derived category to check method signature of forcedLog.
*/
diff --git a/log4j-layout-template-json/pom.xml b/log4j-layout-template-json/pom.xml
index 356bbe2..42b9199 100644
--- a/log4j-layout-template-json/pom.xml
+++ b/log4j-layout-template-json/pom.xml
@@ -15,9 +15,7 @@
~ See the license for the specific language governing permissions and
~ limitations under the license.
-->
-<project xmlns="http://maven.apache.org/POM/4.0.0"
- xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
- xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
@@ -64,6 +62,13 @@
</dependency>
<dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-1.2-api</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
<groupId>org.jctools</groupId>
<artifactId>jctools-core</artifactId>
<optional>true</optional>
diff --git a/log4j-layout-template-json/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/MessageResolverTest.java b/log4j-layout-template-json/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/MessageResolverTest.java
new file mode 100644
index 0000000..7aa8b24
--- /dev/null
+++ b/log4j-layout-template-json/src/test/java/org/apache/logging/log4j/layout/template/json/resolver/MessageResolverTest.java
@@ -0,0 +1,77 @@
+/*
+ * 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.layout.template.json.resolver;
+
+import org.apache.logging.log4j.core.test.junit.LoggerContextSource;
+import org.apache.logging.log4j.core.test.junit.Named;
+import org.apache.logging.log4j.layout.template.json.util.JsonReader;
+import org.apache.logging.log4j.core.test.appender.ListAppender;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+class MessageResolverTest {
+
+ /**
+ * @see <a href="https://issues.apache.org/jira/browse/LOG4J2-3080">LOG4J2-3080</a>
+ */
+ @Test
+ @LoggerContextSource("messageFallbackKeyUsingJsonTemplateLayout.xml")
+ void log4j1_logger_calls_should_use_fallbackKey(
+ final @Named(value = "List") ListAppender appender) {
+
+ // Log using legacy Log4j 1 API.
+ final String log4j1Message =
+ "Message logged using org.apache.log4j.Category.info(Object)";
+ org.apache.log4j.LogManager
+ .getLogger(MessageResolverTest.class)
+ .info(log4j1Message);
+
+ // Log using Log4j 2 API.
+ final String log4j2Message =
+ "Message logged using org.apache.logging.log4j.Logger.info(String)";
+ org.apache.logging.log4j.LogManager
+ .getLogger(MessageResolverTest.class)
+ .info(log4j2Message);
+
+ // Collect and parse logged messages.
+ final List<Object> actualLoggedEvents = appender
+ .getData()
+ .stream()
+ .map(jsonBytes -> {
+ final String json = new String(jsonBytes, StandardCharsets.UTF_8);
+ return JsonReader.read(json);
+ })
+ .collect(Collectors.toList());
+
+ // Verify logged messages.
+ final List<Object> expectedLoggedEvents = Stream
+ .of(log4j1Message, log4j2Message)
+ .map(message -> Collections.singletonMap(
+ "message", Collections.singletonMap(
+ "fallback", message)))
+ .collect(Collectors.toList());
+ Assertions.assertThat(actualLoggedEvents).isEqualTo(expectedLoggedEvents);
+
+ }
+
+}
diff --git a/log4j-layout-template-json/src/test/resources/messageFallbackKeyUsingJsonTemplateLayout.xml b/log4j-layout-template-json/src/test/resources/messageFallbackKeyUsingJsonTemplateLayout.xml
new file mode 100644
index 0000000..81e116c
--- /dev/null
+++ b/log4j-layout-template-json/src/test/resources/messageFallbackKeyUsingJsonTemplateLayout.xml
@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+<Configuration status="OFF">
+ <Appenders>
+ <List name="List" raw="true">
+ <JsonTemplateLayout eventTemplate=
+'{
+ "message": {
+ "$resolver": "message",
+ "fallbackKey": "fallback"
+ }
+}'
+ />
+ </List>
+ </Appenders>
+ <Loggers>
+ <Root level="TRACE">
+ <AppenderRef ref="List"/>
+ </Root>
+ </Loggers>
+</Configuration>
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index f7cc337..d355983 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -208,6 +208,9 @@
Allow a PatternSelector to be specified on GelfLayout.
</action>
<!-- FIXES -->
+ <action issue="LOG4J2-3080" dev="vy" type="fix">
+ Use SimpleMessage in Log4j 1 Category whenever possible.
+ </action>
<action issue="LOG4J2-3102" dev="ckozak" type="fix">
Fix a regression in 2.14.1 which allowed the AsyncAppender background thread to keep the JVM alive because
the daemon flag was not set.