Merge pull request #202 from dmlloyd/logmgr-207

[LOGMGR-207] Move message formatting responsibilities to the Formatter

src/main/java/org/jboss/logmanager/ExtFormatter.java

@@ -19,6 +19,9 @@
 
 package org.jboss.logmanager;
 
+import java.text.MessageFormat;
+import java.util.MissingResourceException;
+import java.util.ResourceBundle;
 import java.util.logging.Formatter;
 import java.util.logging.LogRecord;
 
@@ -35,10 +38,40 @@ public final String format(final LogRecord record) {
     /**
      * Format a message using an extended log record.
      *
-     * @param extLogRecord the log record
+     * @param record the log record
      * @return the formatted message
      */
-    public abstract String format(final ExtLogRecord extLogRecord);
+    public abstract String format(ExtLogRecord record);
+
+    @Override
+    public String formatMessage(LogRecord record) {
+        final ResourceBundle bundle = record.getResourceBundle();
+        String msg = record.getMessage();
+        if (msg == null) {
+            return null;
+        }
+        if (bundle != null) {
+            try {
+                msg = bundle.getString(msg);
+            } catch (MissingResourceException ex) {
+                // ignore
+            }
+        }
+        final Object[] parameters = record.getParameters();
+        if (parameters == null || parameters.length == 0) {
+            return formatMessageNone(record);
+        }
+        if (record instanceof ExtLogRecord) {
+            final ExtLogRecord extLogRecord = (ExtLogRecord) record;
+            final ExtLogRecord.FormatStyle formatStyle = extLogRecord.getFormatStyle();
+            if (formatStyle == ExtLogRecord.FormatStyle.PRINTF) {
+                return formatMessagePrintf(record);
+            } else if (formatStyle == ExtLogRecord.FormatStyle.NO_FORMAT) {
+                return formatMessageNone(record);
+            }
+        }
+        return msg.indexOf('{') >= 0 ? formatMessageLegacy(record) : formatMessageNone(record);
+    }
 
     /**
      * Determines whether or not this formatter will require caller, source level, information when a log record is
@@ -54,4 +87,37 @@ public final String format(final LogRecord record) {
     public boolean isCallerCalculationRequired() {
         return true;
     }
-}
+
+    /**
+     * Format the message text as if there are no parameters.  The default implementation delegates to
+     * {@link LogRecord#getMessage() record.getMessage()}.
+     *
+     * @param record the record to format
+     * @return the formatted string
+     */
+    protected String formatMessageNone(LogRecord record) {
+        return record.getMessage();
+    }
+
+    /**
+     * Format the message text as if there are no parameters.  The default implementation delegates to
+     * {@link MessageFormat#format(String, Object[]) MessageFormat.format(record.getMessage(),record.getParameters())}.
+     *
+     * @param record the record to format
+     * @return the formatted string
+     */
+    protected String formatMessageLegacy(LogRecord record) {
+        return MessageFormat.format(record.getMessage(), record.getParameters());
+    }
+
+    /**
+     * Format the message text as if there are no parameters.  The default implementation delegates to
+     * {@link String#format(String, Object[]) String.format(record.getMessage(),record.getParameters())}.
+     *
+     * @param record the record to format
+     * @return the formatted string
+     */
+    protected String formatMessagePrintf(LogRecord record) {
+        return String.format(record.getMessage(), record.getParameters());
+    }
+}

src/main/java/org/jboss/logmanager/ExtHandler.java

@@ -45,6 +45,7 @@ public abstract class ExtHandler extends Handler implements FlushableCloseable,
     private volatile boolean closeChildren;
     private static final ErrorManager DEFAULT_ERROR_MANAGER = new OnlyOnceErrorManager();
 
+    @SuppressWarnings("unused")
     private volatile Object protectKey;
     private final ThreadLocal<Boolean> granted = new InheritableThreadLocal<Boolean>();
 
@@ -54,7 +55,7 @@ public abstract class ExtHandler extends Handler implements FlushableCloseable,
      * The sub-handlers for this handler.  May only be updated using the {@link #handlersUpdater} atomic updater.  The array
      * instance should not be modified (treat as immutable).
      */
-    @SuppressWarnings({ "UnusedDeclaration" })
+    @SuppressWarnings("unused")
     protected volatile Handler[] handlers;
 
     /**
@@ -196,7 +197,7 @@ public boolean isAutoFlush() {
     /**
      * Change the autoflush setting for this handler.
      *
-     * @param autoFlush {@code true} to automatically flush after each write; false otherwise
+     * @param autoFlush {@code true} to automatically flush after each write; {@code false} otherwise
      *
      * @throws SecurityException if a security manager exists and if the caller does not have {@code
      *                           LoggingPermission(control)} or the handler is {@link #protect(Object) protected}.
@@ -400,9 +401,12 @@ public boolean isCallerCalculationRequired() {
         Formatter formatter = getFormatter();
         if (formatterRequiresCallerCalculation(formatter)) {
             return true;
-        } else {
-            final Handler[] handlers = getHandlers();
-            for (Handler handler : handlers) {
+        } else for (Handler handler : getHandlers()) {
+            if (handler instanceof ExtHandler) {
+                if (((ExtHandler) handler).isCallerCalculationRequired()) {
+                    return true;
+                }
+            } else {
                 formatter = handler.getFormatter();
                 if (formatterRequiresCallerCalculation(formatter)) {
                     return true;

src/main/java/org/jboss/logmanager/ExtLogRecord.java

@@ -118,8 +118,6 @@ public ExtLogRecord(final ExtLogRecord original) {
         ndc = original.ndc;
         loggerClassName = original.loggerClassName;
         threadName = original.threadName;
-        resourceKey = original.resourceKey;
-        formattedMessage = original.formattedMessage;
         hostName = original.hostName;
         processName = original.processName;
         processId = original.processId;
@@ -150,8 +148,6 @@ public static ExtLogRecord wrap(LogRecord rec) {
     private FastCopyHashMap<String, Object> mdcCopy;
     private int sourceLineNumber = -1;
     private String sourceFileName;
-    private String resourceKey;
-    private String formattedMessage;
     private String threadName;
     private String hostName;
     private String processName;
@@ -172,8 +168,6 @@ private void readObject(ObjectInputStream ois) throws IOException, ClassNotFound
         mdcCopy = (FastCopyHashMap<String, Object>) fields.get("mdcCopy", new FastCopyHashMap<>());
         sourceLineNumber = fields.get("sourceLineNumber", -1);
         sourceFileName = (String) fields.get("sourceFileName", null);
-        resourceKey = (String) fields.get("resourceKey", null);
-        formattedMessage = (String) fields.get("formattedMessage", null);
         threadName = (String) fields.get("threadName", null);
         hostName = (String) fields.get("hostName", null);
         processName = (String) fields.get("processName", null);
@@ -199,7 +193,6 @@ public void disableCallerCalculation() {
     public void copyAll() {
         copyMdc();
         calculateCaller();
-        getFormattedMessage();
     }
 
     /**
@@ -462,36 +455,17 @@ public void setSourceModuleVersion(final String sourceModuleVersion) {
      * Get the fully formatted log record, with resources resolved and parameters applied.
      *
      * @return the formatted log record
+     * @deprecated The formatter should normally be used to format the message contents.
      */
+    @Deprecated
     public String getFormattedMessage() {
-        if (formattedMessage == null) {
-            formattedMessage = formatRecord();
-        }
-        return formattedMessage;
-    }
-
-    /**
-     * Get the resource key, if any.  If the log message is not localized, then the key is {@code null}.
-     *
-     * @return the resource key
-     */
-    public String getResourceKey() {
-        if (formattedMessage == null) {
-            formatRecord();
-        }
-        return resourceKey;
-    }
-
-    private String formatRecord() {
         final ResourceBundle bundle = getResourceBundle();
         String msg = getMessage();
         if (msg == null)
             return null;
         if (bundle != null) {
             try {
-                String locMsg = bundle.getString(msg);
-                resourceKey = msg;
-                msg = locMsg;
+                msg = bundle.getString(msg);
             } catch (MissingResourceException ex) {
                 // ignore
             }
@@ -512,6 +486,18 @@ private String formatRecord() {
         return msg;
     }
 
+    /**
+     * Get the resource key, if any.  If the log message is not localized, then the key is {@code null}.
+     *
+     * @return the resource key
+     */
+    public String getResourceKey() {
+        final String msg = getMessage();
+        if (msg == null) return null;
+        if (getResourceBundleName() == null && getResourceBundle() == null) return null;
+        return msg;
+    }
+
     /**
      * Get the thread name of this logging event.
      *
@@ -603,7 +589,6 @@ public void setMessage(final String message) {
      */
     public void setMessage(final String message, final FormatStyle formatStyle) {
         this.formatStyle = formatStyle == null ? FormatStyle.MESSAGE_FORMAT : formatStyle;
-        formattedMessage = null;
         super.setMessage(message);
     }
 
@@ -613,7 +598,6 @@ public void setMessage(final String message, final FormatStyle formatStyle) {
      * @param parameters the log message parameters. (may be null)
      */
     public void setParameters(final Object[] parameters) {
-        formattedMessage = null;
         super.setParameters(parameters);
     }
 
@@ -623,7 +607,6 @@ public void setParameters(final Object[] parameters) {
      * @param bundle localization bundle (may be null)
      */
     public void setResourceBundle(final ResourceBundle bundle) {
-        formattedMessage = null;
         super.setResourceBundle(bundle);
     }
 
@@ -633,7 +616,6 @@ public void setResourceBundle(final ResourceBundle bundle) {
      * @param name localization bundle name (may be null)
      */
     public void setResourceBundleName(final String name) {
-        formattedMessage = null;
         super.setResourceBundleName(name);
     }
 }

src/main/java/org/jboss/logmanager/filters/RegexFilter.java

@@ -19,6 +19,7 @@
 
 package org.jboss.logmanager.filters;
 
+import java.text.MessageFormat;
 import java.util.logging.LogRecord;
 import java.util.regex.Pattern;
 
@@ -63,7 +64,7 @@ public boolean isLoggable(final LogRecord record) {
         if (record instanceof ExtLogRecord) {
             msg = ((ExtLogRecord) record).getFormattedMessage();
         } else {
-            msg = record.getMessage();
+            msg = MessageFormat.format(record.getMessage(), record.getParameters());
         }
         return pattern.matcher(String.valueOf(msg)).find();
     }

src/main/java/org/jboss/logmanager/filters/SubstituteFilter.java

@@ -19,6 +19,7 @@
 
 package org.jboss.logmanager.filters;
 
+import java.text.MessageFormat;
 import java.util.logging.LogRecord;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -77,7 +78,7 @@ public boolean isLoggable(final LogRecord record) {
         if (record instanceof ExtLogRecord) {
             currentMsg = ((ExtLogRecord) record).getFormattedMessage();
         } else {
-            currentMsg = record.getMessage();
+            currentMsg = MessageFormat.format(record.getMessage(), record.getParameters());
         }
         final Matcher matcher = pattern.matcher(String.valueOf(currentMsg));
         final String msg;

src/main/java/org/jboss/logmanager/formatters/FormatStep.java

@@ -21,6 +21,8 @@
 
 import org.jboss.logmanager.ExtLogRecord;
 
+import java.util.logging.Formatter;
+
 /**
  * A single format step which handles some part of rendering a log record.
  */
@@ -34,6 +36,17 @@ public interface FormatStep {
      */
     void render(StringBuilder builder, ExtLogRecord record);
 
+    /**
+     * Render a part of the log record to the given formatter.
+     *
+     * @param formatter the formatter to render to
+     * @param builder the string builder to append to
+     * @param record the record being rendered
+     */
+    default void render(Formatter formatter, StringBuilder builder, ExtLogRecord record) {
+        render(builder, record);
+    }
+
     /**
      * Emit an estimate of the length of data which this step will produce.  The more accurate the estimate, the
      * more likely the format operation will be performant.