This is an automated email from the ASF dual-hosted git repository. pkarwasz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit b0e977f44ca05b0a59e489b75b9b237f5ea7e3e5 Author: Piotr P. Karwasz <[email protected]> AuthorDate: Mon Feb 21 19:44:47 2022 +0100 Provide a uniform Log4j 1.x message factory This PR provides a factory method that decides the type of message based on the runtime type of the logged object (cf. [LOG4J2-3080]( https://issues.apache.org/jira/browse/LOG4J2-3080)). These are applied to both `maybeLog` and `forceLog`. Conflicts: log4j-1.2-api/src/test/java/org/apache/log4j/CategoryTest.java --- .../src/main/java/org/apache/log4j/Category.java | 49 ++++++++----------- .../test/java/org/apache/log4j/CategoryTest.java | 56 ++++++++++++++++++---- 2 files changed, 69 insertions(+), 36 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 90857cb..82a6e1b 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 @@ -307,17 +307,28 @@ public class Category implements AppenderAttachable { } } + private static Message createMessage(Object message) { + if (message instanceof String) { + return new SimpleMessage((String) message); + } + if (message instanceof CharSequence) { + return new SimpleMessage((CharSequence) message); + } + if (message instanceof Map) { + return new MapMessage<>((Map<String, ?>) message); + } + if (message instanceof Message) { + return (Message) message; + } + return new ObjectMessage(message); + } + public void forcedLog(final String fqcn, final Priority level, final Object message, final Throwable t) { final org.apache.logging.log4j.Level lvl = org.apache.logging.log4j.Level.toLevel(level.toString()); + final Message msg = createMessage(message); if (logger instanceof ExtendedLogger) { - @SuppressWarnings("unchecked") - final Message msg = message instanceof Message ? (Message) message - : message instanceof Map ? new MapMessage((Map) message) : new ObjectMessage(message); ((ExtendedLogger) logger).logMessage(fqcn, lvl, null, msg, t); } else { - final ObjectRenderer renderer = get(message.getClass()); - final Message msg = message instanceof Message ? (Message) message - : renderer != null ? new RenderedMessage(renderer, message) : new ObjectMessage(message); logger.log(lvl, msg, t); } } @@ -537,43 +548,25 @@ public class Category implements AppenderAttachable { public void log(final Priority priority, final Object message) { if (isEnabledFor(priority)) { - @SuppressWarnings("unchecked") - final Message msg = message instanceof Map ? new MapMessage((Map) message) : new ObjectMessage(message); - forcedLog(FQCN, priority, msg, null); + forcedLog(FQCN, priority, message, null); } } public void log(final Priority priority, final Object message, final Throwable t) { if (isEnabledFor(priority)) { - @SuppressWarnings("unchecked") - final Message msg = message instanceof Map ? new MapMessage((Map) message) : new ObjectMessage(message); - forcedLog(FQCN, priority, msg, t); + forcedLog(FQCN, priority, message, t); } } public void log(final String fqcn, final Priority priority, final Object message, final Throwable t) { if (isEnabledFor(priority)) { - final Message msg = new ObjectMessage(message); - forcedLog(fqcn, priority, msg, t); + forcedLog(fqcn, priority, message, t); } } void maybeLog(final String fqcn, final org.apache.logging.log4j.Level level, final Object message, final Throwable throwable) { if (logger.isEnabled(level)) { - 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); - } + final Message msg = createMessage(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 700f572..e490df3 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 @@ -27,6 +27,7 @@ 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.plugins.util.TypeUtil; +import org.apache.logging.log4j.util.Constants; import org.apache.logging.log4j.util.Strings; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -79,26 +80,65 @@ public class CategoryTest { final MockCategory category = new MockCategory("org.example.foo"); category.setAdditivity(false); ((org.apache.logging.log4j.core.Logger) category.getLogger()).addAppender(appender); + // Logging a String category.info("Hello, World"); - final List<LogEvent> list = appender.getEvents(); + List<LogEvent> list = appender.getEvents(); int events = list.size(); assertEquals(1, events, "Number of events should be 1, was " + events); LogEvent event = list.get(0); Message msg = event.getMessage(); assertNotNull(msg, "No message"); - assertTrue(msg instanceof ObjectMessage, "Incorrect Message type"); + // LOG4J2-3080: use message type consistently + assertTrue(msg instanceof SimpleMessage, "Incorrect Message type"); + assertEquals("Hello, World", msg.getFormat()); + appender.clear(); + // Logging a String map + category.info(Collections.singletonMap("hello", "world")); + list = appender.getEvents(); + events = list.size(); + assertTrue(events == 1, "Number of events should be 1, was " + events); + event = list.get(0); + msg = event.getMessage(); + assertNotNull(msg, "No message"); + assertTrue(msg instanceof MapMessage, "Incorrect Message type"); Object[] objects = msg.getParameters(); - assertTrue(objects[0] instanceof String, "Incorrect Object type"); + assertEquals("world", objects[0]); appender.clear(); - category.log(Priority.INFO, "Hello, World"); + // Logging a generic map + category.info(Collections.singletonMap(1234L, "world")); + list = appender.getEvents(); events = list.size(); - assertEquals(1, events, "Number of events should be 1, was " + events); + assertTrue(events == 1, "Number of events should be 1, was " + events); + event = list.get(0); + msg = event.getMessage(); + assertNotNull(msg, "No message"); + assertTrue(msg instanceof MapMessage, "Incorrect Message type"); + objects = msg.getParameters(); + assertEquals("world", objects[0]); + appender.clear(); + // Logging an object + final Object obj = new Object(); + category.info(obj); + list = appender.getEvents(); + events = list.size(); + assertTrue(events == 1, "Number of events should be 1, was " + events); event = list.get(0); msg = event.getMessage(); assertNotNull(msg, "No message"); assertTrue(msg instanceof ObjectMessage, "Incorrect Message type"); objects = msg.getParameters(); - assertTrue(objects[0] instanceof String, "Incorrect Object type"); + assertEquals(obj, objects[0]); + appender.clear(); + + category.log(Priority.INFO, "Hello, World"); + list = appender.getEvents(); + events = list.size(); + assertTrue(events == 1, "Number of events should be 1, was " + events); + event = list.get(0); + msg = event.getMessage(); + assertNotNull(msg, "No message"); + assertTrue(msg instanceof SimpleMessage, "Incorrect Message type"); + assertEquals("Hello, World", msg.getFormat()); appender.clear(); } @@ -133,7 +173,7 @@ public class CategoryTest { final Logger logger = Logger.getLogger("org.example.foo"); logger.setLevel(Level.ERROR); final Priority debug = Level.DEBUG; - logger.l7dlog(debug, "Hello, World", new Object[0], null); + logger.l7dlog(debug, "Hello, World", Constants.EMPTY_OBJECT_ARRAY, null); assertTrue(appender.getEvents().isEmpty()); } @@ -152,7 +192,7 @@ public class CategoryTest { final Priority debug = Level.DEBUG; // 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); + logger.l7dlog(debug, "Hello, World", Constants.EMPTY_OBJECT_ARRAY, null); assertTrue(appender.getEvents().isEmpty()); }
