You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by vy...@apache.org on 2021/06/07 07:32:26 UTC

[logging-log4j2] branch release-2.x updated: LOG4J2-3080 Use SimpleMessage in Log4j 1 Category whenever possible. (#504)

This is an automated email from the ASF dual-hosted git repository.

vy pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git


The following commit(s) were added to refs/heads/release-2.x by this push:
     new 73164d3  LOG4J2-3080 Use SimpleMessage in Log4j 1 Category whenever possible. (#504)
73164d3 is described below

commit 73164d38ef4124f864c1a567027f73e7dae75fa8
Author: Volkan Yazıcı <vo...@gmail.com>
AuthorDate: Mon Jun 7 09:30:52 2021 +0200

    LOG4J2-3080 Use SimpleMessage in Log4j 1 Category whenever possible. (#504)
---
 .../src/main/java/org/apache/log4j/Category.java   |  25 ++++-
 .../test/java/org/apache/log4j/CategoryTest.java   | 109 +++++++++++++++++++--
 log4j-layout-template-json/pom.xml                 |   6 ++
 .../json/resolver/MessageResolverTest.java         |  77 +++++++++++++++
 .../messageFallbackKeyUsingJsonTemplateLayout.xml  |  36 +++++++
 src/changes/changes.xml                            |   3 +
 6 files changed, 245 insertions(+), 11 deletions(-)

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 7b13e91..5c895a2 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.or.RendererSupport;
 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;
@@ -507,11 +508,27 @@ public class Category {
         }
     }
 
-    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 33e1e57..f73c22b 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.test.appender.ListAppender;
 import org.apache.logging.log4j.util.Strings;
 import org.junit.AfterClass;
@@ -38,6 +33,15 @@ import org.junit.Before;
 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.
@@ -194,6 +198,97 @@ public class CategoryTest {
         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 86a60b6..c14eca3 100644
--- a/log4j-layout-template-json/pom.xml
+++ b/log4j-layout-template-json/pom.xml
@@ -55,6 +55,12 @@
     </dependency>
 
     <dependency>
+      <groupId>org.apache.logging.log4j</groupId>
+      <artifactId>log4j-1.2-api</artifactId>
+      <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..5dccd47
--- /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.junit.LoggerContextSource;
+import org.apache.logging.log4j.junit.Named;
+import org.apache.logging.log4j.layout.template.json.util.JsonReader;
+import org.apache.logging.log4j.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 b15ac33..3a78751 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -69,6 +69,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.