Improved handling of throwables in message formatting.

qos-ch#390
Signed-off-by: Marcin Kielar <[email protected]>

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[]
      * 
      * <p>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.

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;
     }

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);
-    }
-
 }