From 92675efde9b7ce442c0555c68ea3065997fa7a00 Mon Sep 17 00:00:00 2001 From: Marcin Kielar Date: Sat, 27 Jan 2024 08:58:21 +0100 Subject: [PATCH] Improved handling of throwables in message formatting. https://github.com/qos-ch/slf4j/issues/390 Signed-off-by: Marcin Kielar --- .../org/slf4j/helpers/AbstractLogger.java | 10 +- .../org/slf4j/helpers/FormattingTuple.java | 10 +- .../org/slf4j/helpers/MessageFormatter.java | 62 +-- .../slf4j/helpers/NormalizedParameters.java | 116 ---- .../eventTest/EventRecordingLoggerTest.java | 513 +----------------- .../slf4j/helpers/MessageFormatterTest.java | 14 +- .../org/slf4j/jul/JDK14LoggerAdapter.java | 8 +- .../slf4j/reload4j/Reload4jLoggerAdapter.java | 11 +- .../java/org/slf4j/simple/SimpleLogger.java | 12 +- 9 files changed, 44 insertions(+), 712 deletions(-) delete mode 100644 slf4j-api/src/main/java/org/slf4j/helpers/NormalizedParameters.java diff --git a/slf4j-api/src/main/java/org/slf4j/helpers/AbstractLogger.java b/slf4j-api/src/main/java/org/slf4j/helpers/AbstractLogger.java index 0caaf8103..40430c9e1 100644 --- a/slf4j-api/src/main/java/org/slf4j/helpers/AbstractLogger.java +++ b/slf4j-api/src/main/java/org/slf4j/helpers/AbstractLogger.java @@ -395,13 +395,7 @@ private void handle2ArgsCall(Level level, Marker marker, String msg, Object arg1 } private void handleArgArrayCall(Level level, Marker marker, String msg, Object[] args) { - Throwable throwableCandidate = MessageFormatter.getThrowableCandidate(args); - if (throwableCandidate != null) { - Object[] trimmedCopy = MessageFormatter.trimmedCopy(args); - handleNormalizedLoggingCall(level, marker, msg, trimmedCopy, throwableCandidate); - } else { - handleNormalizedLoggingCall(level, marker, msg, args, null); - } + handleNormalizedLoggingCall(level, marker, msg, args, null); } abstract protected String getFullyQualifiedCallerName(); @@ -411,6 +405,8 @@ private void handleArgArrayCall(Level level, Marker marker, String msg, Object[] * *

This method assumes that the separation of the args array into actual * objects and a throwable has been already operated. + * + * TODO: I think it should accept formatted message rather than pattern and arguments. * * @param level the SLF4J level for this event * @param marker The marker to be used for this event, may be null. diff --git a/slf4j-api/src/main/java/org/slf4j/helpers/FormattingTuple.java b/slf4j-api/src/main/java/org/slf4j/helpers/FormattingTuple.java index a416e1d47..a08c7d765 100644 --- a/slf4j-api/src/main/java/org/slf4j/helpers/FormattingTuple.java +++ b/slf4j-api/src/main/java/org/slf4j/helpers/FormattingTuple.java @@ -35,26 +35,20 @@ public class FormattingTuple { private final String message; private final Throwable throwable; - private final Object[] argArray; public FormattingTuple(String message) { - this(message, null, null); + this(message, null); } - public FormattingTuple(String message, Object[] argArray, Throwable throwable) { + public FormattingTuple(String message, Throwable throwable) { this.message = message; this.throwable = throwable; - this.argArray = argArray; } public String getMessage() { return message; } - public Object[] getArgArray() { - return argArray; - } - public Throwable getThrowable() { return throwable; } diff --git a/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java b/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java index 47f95daaa..aa1b827a0 100755 --- a/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java +++ b/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java @@ -152,12 +152,7 @@ final public static FormattingTuple format(final String messagePattern, Object a } final public static FormattingTuple arrayFormat(final String messagePattern, final Object[] argArray) { - Throwable throwableCandidate = MessageFormatter.getThrowableCandidate(argArray); - Object[] args = argArray; - if (throwableCandidate != null) { - args = MessageFormatter.trimmedCopy(argArray); - } - return arrayFormat(messagePattern, args, throwableCandidate); + return arrayFormat(messagePattern, argArray, null); } /** @@ -166,26 +161,40 @@ final public static FormattingTuple arrayFormat(final String messagePattern, fin * @param messagePattern * @param argArray */ + @Deprecated final public static String basicArrayFormat(final String messagePattern, final Object[] argArray) { FormattingTuple ft = arrayFormat(messagePattern, argArray, null); return ft.getMessage(); } - public static String basicArrayFormat(NormalizedParameters np) { - return basicArrayFormat(np.getMessage(), np.getArguments()); + /** Resolves throwable to be used in log message. + * + * @param index index of first array entry that was not bound to format placeholder + */ + final static Throwable resolveThrowable(final Object[] argArray, final int index, final Throwable throwable) { + if (throwable != null) { + return throwable; + } + if(argArray != null && argArray.length > index) { + final Object lastEntry = argArray[argArray.length - 1]; + if (lastEntry instanceof Throwable) { + return (Throwable) lastEntry; + } + } + return null; } final public static FormattingTuple arrayFormat(final String messagePattern, final Object[] argArray, Throwable throwable) { if (messagePattern == null) { - return new FormattingTuple(null, argArray, throwable); + return new FormattingTuple(null, resolveThrowable(argArray, 0, throwable)); } if (argArray == null) { - return new FormattingTuple(messagePattern); + return new FormattingTuple(messagePattern, throwable); } - int i = 0; + int i = 0; // index in the pattern int j; // use string builder for better multicore performance StringBuilder sbuf = new StringBuilder(messagePattern.length() + 50); @@ -198,11 +207,11 @@ final public static FormattingTuple arrayFormat(final String messagePattern, fin if (j == -1) { // no more variables if (i == 0) { // this is a simple string - return new FormattingTuple(messagePattern, argArray, throwable); + return new FormattingTuple(messagePattern, resolveThrowable(argArray, L, throwable)); } else { // add the tail string which contains no variables and return // the result. sbuf.append(messagePattern, i, messagePattern.length()); - return new FormattingTuple(sbuf.toString(), argArray, throwable); + return new FormattingTuple(sbuf.toString(), resolveThrowable(argArray, L, throwable)); } } else { if (isEscapedDelimeter(messagePattern, j)) { @@ -229,7 +238,7 @@ final public static FormattingTuple arrayFormat(final String messagePattern, fin } // append the characters following the last {} pair. sbuf.append(messagePattern, i, messagePattern.length()); - return new FormattingTuple(sbuf.toString(), argArray, throwable); + return new FormattingTuple(sbuf.toString(), throwable); } final static boolean isEscapedDelimeter(String messagePattern, int delimeterStartIndex) { @@ -402,29 +411,4 @@ private static void doubleArrayAppend(StringBuilder sbuf, double[] a) { } sbuf.append(']'); } - - /** - * Helper method to determine if an {@link Object} array contains a {@link Throwable} as last element - * - * @param argArray - * The arguments off which we want to know if it contains a {@link Throwable} as last element - * @return if the last {@link Object} in argArray is a {@link Throwable} this method will return it, - * otherwise it returns null - */ - public static Throwable getThrowableCandidate(final Object[] argArray) { - return NormalizedParameters.getThrowableCandidate(argArray); - } - - /** - * Helper method to get all but the last element of an array - * - * @param argArray - * The arguments from which we want to remove the last element - * - * @return a copy of the array without the last element - */ - public static Object[] trimmedCopy(final Object[] argArray) { - return NormalizedParameters.trimmedCopy(argArray); - } - } diff --git a/slf4j-api/src/main/java/org/slf4j/helpers/NormalizedParameters.java b/slf4j-api/src/main/java/org/slf4j/helpers/NormalizedParameters.java deleted file mode 100644 index ec278f463..000000000 --- a/slf4j-api/src/main/java/org/slf4j/helpers/NormalizedParameters.java +++ /dev/null @@ -1,116 +0,0 @@ -package org.slf4j.helpers; - -import org.slf4j.event.LoggingEvent; - -/** - * Holds normalized call parameters. - * - * Includes utility methods such as {@link #normalize(String, Object[], Throwable)} to help the normalization of parameters. - * - * @author ceki - * @since 2.0 - */ -public class NormalizedParameters { - - final String message; - final Object[] arguments; - final Throwable throwable; - - public NormalizedParameters(String message, Object[] arguments, Throwable throwable) { - this.message = message; - this.arguments = arguments; - this.throwable = throwable; - } - - public NormalizedParameters(String message, Object[] arguments) { - this(message, arguments, null); - } - - public String getMessage() { - return message; - } - - public Object[] getArguments() { - return arguments; - } - - public Throwable getThrowable() { - return throwable; - } - - /** - * Helper method to determine if an {@link Object} array contains a - * {@link Throwable} as last element - * - * @param argArray The arguments off which we want to know if it contains a - * {@link Throwable} as last element - * @return if the last {@link Object} in argArray is a {@link Throwable} this - * method will return it, otherwise it returns null - */ - public static Throwable getThrowableCandidate(final Object[] argArray) { - if (argArray == null || argArray.length == 0) { - return null; - } - - final Object lastEntry = argArray[argArray.length - 1]; - if (lastEntry instanceof Throwable) { - return (Throwable) lastEntry; - } - - return null; - } - - /** - * Helper method to get all but the last element of an array - * - * @param argArray The arguments from which we want to remove the last element - * - * @return a copy of the array without the last element - */ - public static Object[] trimmedCopy(final Object[] argArray) { - if (argArray == null || argArray.length == 0) { - throw new IllegalStateException("non-sensical empty or null argument array"); - } - - final int trimmedLen = argArray.length - 1; - - Object[] trimmed = new Object[trimmedLen]; - - if (trimmedLen > 0) { - System.arraycopy(argArray, 0, trimmed, 0, trimmedLen); - } - - return trimmed; - } - - /** - * This method serves to normalize logging call invocation parameters. - * - * More specifically, if a throwable argument is not supplied directly, it - * attempts to extract it from the argument array. - */ - public static NormalizedParameters normalize(String msg, Object[] arguments, Throwable t) { - - if (t != null) { - return new NormalizedParameters(msg, arguments, t); - } - - if (arguments == null || arguments.length == 0) { - return new NormalizedParameters(msg, arguments, t); - } - - Throwable throwableCandidate = NormalizedParameters.getThrowableCandidate(arguments); - if (throwableCandidate != null) { - Object[] trimmedArguments = MessageFormatter.trimmedCopy(arguments); - return new NormalizedParameters(msg, trimmedArguments, throwableCandidate); - } else { - return new NormalizedParameters(msg, arguments); - } - - } - - public static NormalizedParameters normalize(LoggingEvent event) { - return normalize(event.getMessage(), event.getArgumentArray(), event.getThrowable()); - } - -} diff --git a/slf4j-api/src/test/java/org/slf4j/eventTest/EventRecordingLoggerTest.java b/slf4j-api/src/test/java/org/slf4j/eventTest/EventRecordingLoggerTest.java index e2ec1c05c..c259d93e4 100644 --- a/slf4j-api/src/test/java/org/slf4j/eventTest/EventRecordingLoggerTest.java +++ b/slf4j-api/src/test/java/org/slf4j/eventTest/EventRecordingLoggerTest.java @@ -15,517 +15,6 @@ import static org.junit.Assert.*; +/** TODO: This test should verify outcome, not the implementation. */ public class EventRecordingLoggerTest { - private Queue queue; - private EventRecordingLogger logger; - private String message; - private Object param1; - private Object param2; - private Object param3; - private Object[] oneParam; - private Object[] twoParams; - private Object[] threeParams; - private Throwable exception; - private Marker marker; - - @Before - public void setUp() { - queue = new LinkedBlockingQueue<>(); - logger = new EventRecordingLogger(new SubstituteLogger("testLogger", queue, true), queue); - message = "Test message with 3 parameters {} {} {} {}"; - param1 = 1; - param2 = 2; - param3 = 3; - oneParam = new Object[] { param1 }; - twoParams = new Object[] { param1, param2 }; - threeParams = new Object[] { param1, param2, param3 }; - exception = new IllegalStateException("We just need an exception"); - marker = new BasicMarkerFactory().getMarker("testMarker"); - } - - @After - public void tearDown() { - assertTrue(queue.isEmpty()); - } - - @Test - public void singleMessage() { - for (Level level : Level.values()) { - singleMessageCheck(level); - } - } - - private void singleMessageCheck(Level level) { - switch (level) { - case TRACE: - logger.trace(message); - break; - case DEBUG: - logger.debug(message); - break; - case INFO: - logger.info(message); - break; - case WARN: - logger.warn(message); - break; - case ERROR: - logger.error(message); - break; - } - verifyMessageWithoutMarker(level, null, null); - } - - @Test - public void oneParameter() { - for (Level level : Level.values()) { - oneParameterCheck(level); - } - } - - private void oneParameterCheck(Level level) { - switch (level) { - case TRACE: - logger.trace(message, param1); - break; - case DEBUG: - logger.debug(message, param1); - break; - case INFO: - logger.info(message, param1); - break; - case WARN: - logger.warn(message, param1); - break; - case ERROR: - logger.error(message, param1); - break; - } - verifyMessageWithoutMarker(level, oneParam, null); - } - - @Test - public void messageTwoParameters() { - for (Level level : Level.values()) { - messageTwoParametersCheck(level); - } - } - - private void messageTwoParametersCheck(Level level) { - switch (level) { - case TRACE: - logger.trace(message, param1, param2); - break; - case DEBUG: - logger.debug(message, param1, param2); - break; - case INFO: - logger.info(message, param1, param2); - break; - case WARN: - logger.warn(message, param1, param2); - break; - case ERROR: - logger.error(message, param1, param2); - break; - } - verifyMessageWithoutMarker(level, twoParams, null); - } - - @Test - public void traceMessageThreeParameters() { - for (Level level : Level.values()) { - threeParameterCheck(level); - } - } - - private void threeParameterCheck(Level level) { - switch (level) { - case TRACE: - logger.trace(message, param1, param2, param3); - break; - case DEBUG: - logger.debug(message, param1, param2, param3); - break; - case INFO: - logger.info(message, param1, param2, param3); - break; - case WARN: - logger.warn(message, param1, param2, param3); - break; - case ERROR: - logger.error(message, param1, param2, param3); - break; - } - verifyMessageWithoutMarker(level, threeParams, null); - } - - @Test - public void testMessageThrowable() { - for (Level level : Level.values()) { - throwableCheck(level); - } - } - - private void throwableCheck(Level level) { - switch (level) { - case TRACE: - logger.trace(message, exception); - break; - case DEBUG: - logger.debug(message, exception); - break; - case INFO: - logger.info(message, exception); - break; - case WARN: - logger.warn(message, exception); - break; - case ERROR: - logger.error(message, exception); - break; - } - verifyMessageWithoutMarker(level, null, exception); - } - - @Test - public void traceMessageOneParameterThrowable() { - for (Level level : Level.values()) { - oneParamThrowableCheck(level); - } - } - - private void oneParamThrowableCheck(Level level) { - switch (level) { - case TRACE: - logger.trace(message, param1, exception); - break; - case DEBUG: - logger.debug(message, param1, exception); - break; - case INFO: - logger.info(message, param1, exception); - break; - case WARN: - logger.warn(message, param1, exception); - break; - case ERROR: - logger.error(message, param1, exception); - break; - } - verifyMessageWithoutMarker(level, oneParam, exception); - } - - @Test - public void traceMessageTwoParametersThrowable() { - for (Level level : Level.values()) { - twoParamThrowableCheck(level); - } - } - - private void twoParamThrowableCheck(Level level) { - switch (level) { - case TRACE: - logger.trace(message, param1, param2, exception); - break; - case DEBUG: - logger.debug(message, param1, param2, exception); - break; - case INFO: - logger.info(message, param1, param2, exception); - break; - case WARN: - logger.warn(message, param1, param2, exception); - break; - case ERROR: - logger.error(message, param1, param2, exception); - break; - } - verifyMessageWithoutMarker(level, twoParams, exception); - } - - @Test - public void testMessageThreeParametersThrowable() { - for (Level level : Level.values()) { - messageWith3ArgsPlusException(level); - } - } - - private void messageWith3ArgsPlusException(Level level) { - switch (level) { - case TRACE: - logger.trace(message, param1, param2, param3, exception); - break; - case DEBUG: - logger.debug(message, param1, param2, param3, exception); - break; - case INFO: - logger.info(message, param1, param2, param3, exception); - break; - case WARN: - logger.warn(message, param1, param2, param3, exception); - break; - case ERROR: - logger.error(message, param1, param2, param3, exception); - break; - } - verifyMessageWithoutMarker(level, threeParams, exception); - } - - @Test - public void markerMessage() { - for (Level level : Level.values()) { - markerMessageCheck(level); - } - } - - private void markerMessageCheck(Level level) { - switch (level) { - case TRACE: - logger.trace(marker, message); - break; - case DEBUG: - logger.debug(marker, message); - break; - case INFO: - logger.info(marker, message); - break; - case WARN: - logger.warn(marker, message); - break; - case ERROR: - logger.error(marker, message); - break; - } - verifyMessage(level, marker, null, null); - } - - @Test - public void markerMessageOneParameter() { - for (Level level : Level.values()) { - markerMessageOneParameter(level); - } - } - - private void markerMessageOneParameter(Level level) { - switch (level) { - case TRACE: - logger.trace(marker, message, param1); - break; - case DEBUG: - logger.debug(marker, message, param1); - break; - case INFO: - logger.info(marker, message, param1); - break; - case WARN: - logger.warn(marker, message, param1); - break; - case ERROR: - logger.error(marker, message, param1); - break; - } - verifyMessage(level, marker, oneParam, null); - } - - @Test - public void traceMarkerMessageTwoParameters() { - for (Level level : Level.values()) { - markerMessageTwoParameters(level); - } - } - - private void markerMessageTwoParameters(Level level) { - switch (level) { - case TRACE: - logger.trace(marker, message, param1, param2); - break; - case DEBUG: - logger.debug(marker, message, param1, param2); - break; - case INFO: - logger.info(marker, message, param1, param2); - break; - case WARN: - logger.warn(marker, message, param1, param2); - break; - case ERROR: - logger.error(marker, message, param1, param2); - break; - } - verifyMessage(level, marker, twoParams, null); - } - - @Test - public void traceMarkerMessageThreeParameters() { - for (Level level : Level.values()) { - markerMessageThreeParameters(level); - } - } - - private void markerMessageThreeParameters(Level level) { - switch (level) { - case TRACE: - logger.trace(marker, message, param1, param2, param3); - break; - case DEBUG: - logger.debug(marker, message, param1, param2, param3); - break; - case INFO: - logger.info(marker, message, param1, param2, param3); - break; - case WARN: - logger.warn(marker, message, param1, param2, param3); - break; - case ERROR: - logger.error(marker, message, param1, param2, param3); - break; - } - verifyMessage(level, marker, threeParams, null); - } - - @Test - public void markerMessageThrowable() { - for (Level level : Level.values()) { - markerMessageThrowable(level); - } - } - - private void markerMessageThrowable(Level level) { - switch (level) { - case TRACE: - logger.trace(marker, message, exception); - break; - case DEBUG: - logger.debug(marker, message, exception); - break; - case INFO: - logger.info(marker, message, exception); - break; - case WARN: - logger.warn(marker, message, exception); - break; - case ERROR: - logger.error(marker, message, exception); - break; - } - verifyMessage(level, marker, null, exception); - } - - @Test - public void markerMessageOneParameterThrowable() { - for (Level level : Level.values()) { - markerMessageOneParameterThrowableCheck(level); - } - } - - private void markerMessageOneParameterThrowableCheck(Level level) { - switch (level) { - case TRACE: - logger.trace(marker, message, param1, exception); - break; - case DEBUG: - logger.debug(marker, message, param1, exception); - break; - case INFO: - logger.info(marker, message, param1, exception); - break; - case WARN: - logger.warn(marker, message, param1, exception); - break; - case ERROR: - logger.error(marker, message, param1, exception); - break; - } - verifyMessage(level, marker, oneParam, exception); - } - - @Test - public void traceMarkerMessageTwoParametersThrowable() { - for (Level level : Level.values()) { - markerMessageTwoParametersThrowableCheck(level); - } - } - - private void markerMessageTwoParametersThrowableCheck(Level level) { - switch (level) { - case TRACE: - logger.trace(marker, message, param1, param2, exception); - break; - case DEBUG: - logger.debug(marker, message, param1, param2, exception); - break; - case INFO: - logger.info(marker, message, param1, param2, exception); - break; - case WARN: - logger.warn(marker, message, param1, param2, exception); - break; - case ERROR: - logger.error(marker, message, param1, param2, exception); - break; - } - verifyMessage(level, marker, twoParams, exception); - } - - @Test - public void traceMarkerMessageThreeParametersThrowable() { - for (Level level : Level.values()) { - markerMessageThreeParametersThrowableCheck(level); - } - } - - private void markerMessageThreeParametersThrowableCheck(Level level) { - switch (level) { - case TRACE: - logger.trace(marker, message, param1, param2, param3, exception); - break; - case DEBUG: - logger.debug(marker, message, param1, param2, param3, exception); - break; - case INFO: - logger.info(marker, message, param1, param2, param3, exception); - break; - case WARN: - logger.warn(marker, message, param1, param2, param3, exception); - break; - case ERROR: - logger.error(marker, message, param1, param2, param3, exception); - break; - } - verifyMessage(level, marker, threeParams, exception); - } - - private void verifyMessageWithoutMarker(Level level, Object[] arguments, Throwable exception) { - verifyMessage(level, null, arguments, exception); - } - - private void verifyMessage(Level level, Marker marker, Object[] arguments, Throwable exception) { - - assertEquals("missing event: ", 1, queue.size()); - SubstituteLoggingEvent event = queue.poll(); - assertNotNull(event); - - if (marker == null) { - assertNull(event.getMarkers()); - } else { - assertEquals(marker, event.getMarkers().get(0)); - } - - assertEquals(message, event.getMessage()); - - if (arguments == null) { - assertNull(event.getArgumentArray()); - } else { - assertArrayEquals(arguments, event.getArgumentArray()); - } - - assertEquals("wrong level: ", level, event.getLevel()); - - if (exception == null) { - assertNull(event.getThrowable()); - } else { - assertEquals(exception, event.getThrowable()); - } - } } diff --git a/slf4j-api/src/test/java/org/slf4j/helpers/MessageFormatterTest.java b/slf4j-api/src/test/java/org/slf4j/helpers/MessageFormatterTest.java index 4edd59f5d..2a913158f 100755 --- a/slf4j-api/src/test/java/org/slf4j/helpers/MessageFormatterTest.java +++ b/slf4j-api/src/test/java/org/slf4j/helpers/MessageFormatterTest.java @@ -287,52 +287,42 @@ public void testArrayThrowable() { ft = MessageFormatter.arrayFormat("Value {} is smaller than {} and {}.", ia); assertEquals("Value 1 is smaller than 2 and 3.", ft.getMessage()); - assertTrue(Arrays.equals(iaWitness, ft.getArgArray())); assertEquals(t, ft.getThrowable()); ft = MessageFormatter.arrayFormat("{}{}{}", ia); assertEquals("123", ft.getMessage()); - assertTrue(Arrays.equals(iaWitness, ft.getArgArray())); assertEquals(t, ft.getThrowable()); ft = MessageFormatter.arrayFormat("Value {} is smaller than {}.", ia); assertEquals("Value 1 is smaller than 2.", ft.getMessage()); - assertTrue(Arrays.equals(iaWitness, ft.getArgArray())); assertEquals(t, ft.getThrowable()); ft = MessageFormatter.arrayFormat("Value {} is smaller than {}", ia); assertEquals("Value 1 is smaller than 2", ft.getMessage()); - assertTrue(Arrays.equals(iaWitness, ft.getArgArray())); assertEquals(t, ft.getThrowable()); ft = MessageFormatter.arrayFormat("Val={}, {, Val={}", ia); assertEquals("Val=1, {, Val=2", ft.getMessage()); - assertTrue(Arrays.equals(iaWitness, ft.getArgArray())); assertEquals(t, ft.getThrowable()); ft = MessageFormatter.arrayFormat("Val={}, \\{, Val={}", ia); assertEquals("Val=1, \\{, Val=2", ft.getMessage()); - assertTrue(Arrays.equals(iaWitness, ft.getArgArray())); assertEquals(t, ft.getThrowable()); ft = MessageFormatter.arrayFormat("Val1={}, Val2={", ia); assertEquals("Val1=1, Val2={", ft.getMessage()); - assertTrue(Arrays.equals(iaWitness, ft.getArgArray())); assertEquals(t, ft.getThrowable()); ft = MessageFormatter.arrayFormat("Value {} is smaller than {} and {}.", ia); assertEquals("Value 1 is smaller than 2 and 3.", ft.getMessage()); - assertTrue(Arrays.equals(iaWitness, ft.getArgArray())); assertEquals(t, ft.getThrowable()); ft = MessageFormatter.arrayFormat("{}{}{}{}", ia); - assertEquals("123{}", ft.getMessage()); - assertTrue(Arrays.equals(iaWitness, ft.getArgArray())); - assertEquals(t, ft.getThrowable()); + assertEquals("123java.lang.Throwable", ft.getMessage()); + assertNull(ft.getThrowable()); ft = MessageFormatter.arrayFormat("1={}", new Object[] { i1 }, t); assertEquals("1=1", ft.getMessage()); - assertTrue(Arrays.equals(new Object[] { i1 }, ft.getArgArray())); assertEquals(t, ft.getThrowable()); } diff --git a/slf4j-jdk14/src/main/java/org/slf4j/jul/JDK14LoggerAdapter.java b/slf4j-jdk14/src/main/java/org/slf4j/jul/JDK14LoggerAdapter.java index dbc52da52..6fe43fb19 100755 --- a/slf4j-jdk14/src/main/java/org/slf4j/jul/JDK14LoggerAdapter.java +++ b/slf4j-jdk14/src/main/java/org/slf4j/jul/JDK14LoggerAdapter.java @@ -35,7 +35,6 @@ import org.slf4j.helpers.FormattingTuple; import org.slf4j.helpers.LegacyAbstractLogger; import org.slf4j.helpers.MessageFormatter; -import org.slf4j.helpers.NormalizedParameters; import org.slf4j.helpers.SubstituteLogger; import org.slf4j.spi.DefaultLoggingEventBuilder; import org.slf4j.spi.LocationAwareLogger; @@ -143,8 +142,8 @@ protected void handleNormalizedLoggingCall(org.slf4j.event.Level level, Marker m private void innerNormalizedLoggingCallHandler(String fqcn, org.slf4j.event.Level level, Marker marker, String msg, Object[] args, Throwable throwable) { // millis and thread are filled by the constructor Level julLevel = slf4jLevelToJULLevel(level); - String formattedMessage = MessageFormatter.basicArrayFormat(msg, args); - LogRecord record = new LogRecord(julLevel, formattedMessage); + FormattingTuple formattedMessage = MessageFormatter.arrayFormat(msg, args, throwable); + LogRecord record = new LogRecord(julLevel, formattedMessage.getMessage()); // https://jira.qos.ch/browse/SLF4J-13 record.setLoggerName(getName()); @@ -168,8 +167,7 @@ public void log(Marker marker, String callerFQCN, int slf4jLevelInt, String mess Level julLevel = slf4jLevelIntToJULLevel(slf4jLevelInt); if (logger.isLoggable(julLevel)) { - NormalizedParameters np = NormalizedParameters.normalize(message, arguments, throwable); - innerNormalizedLoggingCallHandler(callerFQCN, slf4jLevel, marker, np.getMessage(), np.getArguments(), np.getThrowable()); + innerNormalizedLoggingCallHandler(callerFQCN, slf4jLevel, marker, message, arguments, throwable); } } diff --git a/slf4j-reload4j/src/main/java/org/slf4j/reload4j/Reload4jLoggerAdapter.java b/slf4j-reload4j/src/main/java/org/slf4j/reload4j/Reload4jLoggerAdapter.java index 90a305d3e..b6fdf7666 100644 --- a/slf4j-reload4j/src/main/java/org/slf4j/reload4j/Reload4jLoggerAdapter.java +++ b/slf4j-reload4j/src/main/java/org/slf4j/reload4j/Reload4jLoggerAdapter.java @@ -36,9 +36,9 @@ import org.slf4j.event.DefaultLoggingEvent; import org.slf4j.event.LoggingEvent; import org.slf4j.event.SubstituteLoggingEvent; +import org.slf4j.helpers.FormattingTuple; import org.slf4j.helpers.LegacyAbstractLogger; import org.slf4j.helpers.MessageFormatter; -import org.slf4j.helpers.NormalizedParameters; import org.slf4j.helpers.SubstituteLogger; import org.slf4j.spi.LocationAwareLogger; import org.slf4j.spi.LoggingEventAware; @@ -125,16 +125,15 @@ public boolean isErrorEnabled() { @Override public void log(Marker marker, String callerFQCN, int level, String msg, Object[] arguments, Throwable t) { Level log4jLevel = toLog4jLevel(level); - NormalizedParameters np = NormalizedParameters.normalize(msg, arguments, t); - String formattedMessage = MessageFormatter.basicArrayFormat(np.getMessage(), np.getArguments()); - logger.log(callerFQCN, log4jLevel, formattedMessage, np.getThrowable()); + FormattingTuple formattedMessage = MessageFormatter.arrayFormat(msg, arguments, t); + logger.log(callerFQCN, log4jLevel, formattedMessage.getMessage(), formattedMessage.getThrowable()); } @Override protected void handleNormalizedLoggingCall(org.slf4j.event.Level level, Marker marker, String msg, Object[] arguments, Throwable throwable) { Level log4jLevel = toLog4jLevel(level.toInt()); - String formattedMessage = MessageFormatter.basicArrayFormat(msg, arguments); - logger.log(getFullyQualifiedCallerName(), log4jLevel, formattedMessage, throwable); + FormattingTuple formattedMessage = MessageFormatter.arrayFormat(msg, arguments, throwable); + logger.log(getFullyQualifiedCallerName(), log4jLevel, formattedMessage.getMessage(), formattedMessage.getThrowable()); } /** diff --git a/slf4j-simple/src/main/java/org/slf4j/simple/SimpleLogger.java b/slf4j-simple/src/main/java/org/slf4j/simple/SimpleLogger.java index c1b6c7889..cfd8d6dfa 100644 --- a/slf4j-simple/src/main/java/org/slf4j/simple/SimpleLogger.java +++ b/slf4j-simple/src/main/java/org/slf4j/simple/SimpleLogger.java @@ -33,9 +33,9 @@ import org.slf4j.Marker; import org.slf4j.event.Level; import org.slf4j.event.LoggingEvent; +import org.slf4j.helpers.FormattingTuple; import org.slf4j.helpers.LegacyAbstractLogger; import org.slf4j.helpers.MessageFormatter; -import org.slf4j.helpers.NormalizedParameters; import org.slf4j.spi.LocationAwareLogger; /** @@ -431,12 +431,12 @@ private void innerHandleNormalizedLoggingCall(Level level, List markers, } } - String formattedMessage = MessageFormatter.basicArrayFormat(messagePattern, arguments); + FormattingTuple formattedMessage = MessageFormatter.arrayFormat(messagePattern, arguments, t); // Append the message - buf.append(formattedMessage); + buf.append(formattedMessage.getMessage()); - write(buf, t); + write(buf, formattedMessage.getThrowable()); } protected String renderLevel(int levelInt) { @@ -462,9 +462,7 @@ public void log(LoggingEvent event) { return; } - NormalizedParameters np = NormalizedParameters.normalize(event); - - innerHandleNormalizedLoggingCall(event.getLevel(), event.getMarkers(), np.getMessage(), np.getArguments(), event.getThrowable()); + innerHandleNormalizedLoggingCall(event.getLevel(), event.getMarkers(), event.getMessage(), event.getArgumentArray(), event.getThrowable()); } @Override