Merge pull request #552 from syjer/jdklogging-formatting-opt

optimize the log formatter
danfickle · Sep 14, 2020 · 242a541 · 242a541
2 parents 4a0612f + 14f26f8
commit 242a541
Show file tree

Hide file tree

Showing 5 changed files with 161 additions and 43 deletions.
diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/Diagnostic.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/Diagnostic.java
@@ -36,11 +36,7 @@ public Throwable getError() {
     }
 
     public String getFormattedMessage() {
-        if (logMessageId instanceof LogMessageId.LogMessageId0Param) {
-            return logMessageId.getMessageFormat();
-        } else {
-            return String.format(MESSAGE_FORMAT_PLACEHOLDER.matcher(logMessageId.getMessageFormat()).replaceAll("%s"), args);
-        }
+        return logMessageId.formatMessage(args);
     }
 
     public Object[] getArgs() {

diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/JDKXRLogger.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/JDKXRLogger.java
@@ -21,14 +21,14 @@
  */
 package com.openhtmltopdf.util;
 
+import java.util.HashMap;
+import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import java.util.stream.Collectors;
 import java.util.logging.Handler;
 import java.util.logging.ConsoleHandler;
 import java.util.logging.Formatter;
 import java.util.Arrays;
-import java.util.List;
 
 /**
  * An {@link XRLogger} interface that uses <code>java.util.logging</code>.
@@ -37,9 +37,9 @@
 public class JDKXRLogger implements XRLogger {
     private boolean initPending = true;
 
-    // Keep a list of Loggers so they are not garbage collected
+    // Keep a map of Loggers so they are not garbage collected
     // which makes them lose their settings we have applied.
-    private List<Logger> loggers;
+    private Map<String, Logger> loggers;
 
     private final boolean useParent;
     private final Level level;
@@ -54,7 +54,7 @@ public JDKXRLogger() {
     }
 
     public JDKXRLogger(boolean useParent, Level level, Handler handler, Formatter formatter) {
-        this.useParent = false;
+        this.useParent = useParent;
         this.level = level;
         this.handler = handler;
         this.formatter = formatter;
@@ -100,7 +100,7 @@ public void setLevel(String logger, Level level) {
      */
     private Logger getLogger(String log) {
         checkInitPending();
-        return Logger.getLogger(log);
+        return loggers.get(log);
     }
 
     private void init(boolean useParent, Level level, Handler handler, Formatter formatter) {
@@ -122,27 +122,30 @@ private void initializeJDKLogManager(boolean useParent, Level level, Handler han
     }
 
     private void configureLoggerHandlerForwarding(boolean useParentHandlers) {
-        loggers.forEach(logger -> logger.setUseParentHandlers(useParentHandlers));
+        loggers.forEach((name, logger) -> logger.setUseParentHandlers(useParentHandlers));
     }
 
     /**
      * Returns a List of all Logger instances used by this project from the JDK LogManager; these will
      * be automatically created if they aren't already available.
      */
-    private List<Logger> retrieveLoggers() {
-        return XRLog.listRegisteredLoggers().stream()
-                .map(Logger::getLogger).collect(Collectors.toList());
+    private Map<String, Logger> retrieveLoggers() {
+        Map<String, Logger> loggers = new HashMap<>();
+        for (String name : XRLog.listRegisteredLoggers()) {
+            loggers.put(name, Logger.getLogger(name));
+        }
+        return loggers;
     }
 
     private void configureLogHandlers(Handler handler, Formatter formatter) {
         handler.setFormatter(formatter);
         // Note Logger::removeLogger doesn't throw if the handler isn't found
         // so there are no sync issues here.
-        loggers.forEach(logger -> Arrays.stream(logger.getHandlers()).forEach(logger::removeHandler));
-        loggers.forEach(logger -> logger.addHandler(handler));
+        loggers.forEach((name, logger) -> Arrays.stream(logger.getHandlers()).forEach(logger::removeHandler));
+        loggers.forEach((name, logger) -> logger.addHandler(handler));
     }
 
     private void configureLogLevels(Level level) {
-        loggers.forEach(logger -> logger.setLevel(level));
+        loggers.forEach((name, logger) -> logger.setLevel(level));
     }
 }
diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/LogMessageId.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/LogMessageId.java
@@ -5,6 +5,7 @@ public interface LogMessageId {
     Enum<?> getEnum();
     String getWhere();
     String getMessageFormat();
+    String formatMessage(Object[] args);
 
     enum LogMessageId0Param implements LogMessageId {
         CSS_PARSE_MUST_PROVIDE_AT_LEAST_A_FONT_FAMILY_AND_SRC_IN_FONT_FACE_RULE(XRLog.CSS_PARSE, "Must provide at least a font-family and src in @font-face rule"),
@@ -81,6 +82,10 @@ public String getWhere() {
             return where;
         }
 
+        @Override
+        public String formatMessage(Object[] args) {
+            return getMessageFormat();
+        }
     }
 
     enum LogMessageId1Param implements LogMessageId {
@@ -160,10 +165,12 @@ enum LogMessageId1Param implements LogMessageId {
 
         private final String where;
         private final String messageFormat;
+        private final LogMessageIdFormat logMessageIdFormat;
 
         LogMessageId1Param(String where, String messageFormat) {
             this.where = where;
             this.messageFormat = messageFormat;
+            this.logMessageIdFormat = new LogMessageIdFormat(messageFormat);
         }
 
         @Override
@@ -180,6 +187,11 @@ public String getMessageFormat() {
         public String getWhere() {
             return where;
         }
+
+        @Override
+        public String formatMessage(Object[] args) {
+            return logMessageIdFormat.formatMessage(args);
+        }
     }
 
     enum LogMessageId2Param implements LogMessageId {
@@ -216,10 +228,12 @@ enum LogMessageId2Param implements LogMessageId {
 
         private final String where;
         private final String messageFormat;
+        private final LogMessageIdFormat logMessageIdFormat;
 
         LogMessageId2Param(String where, String messageFormat) {
             this.where = where;
             this.messageFormat = messageFormat;
+            this.logMessageIdFormat = new LogMessageIdFormat(messageFormat);
         }
 
         @Override
@@ -232,6 +246,11 @@ public String getMessageFormat() {
             return messageFormat;
         }
 
+        @Override
+        public String formatMessage(Object[] args) {
+            return logMessageIdFormat.formatMessage(args);
+        }
+
         @Override
         public String getWhere() {
             return where;
@@ -252,10 +271,12 @@ enum LogMessageId3Param implements LogMessageId {
 
         private final String where;
         private final String messageFormat;
+        private final LogMessageIdFormat logMessageIdFormat;
 
         LogMessageId3Param(String where, String messageFormat) {
             this.where = where;
             this.messageFormat = messageFormat;
+            this.logMessageIdFormat = new LogMessageIdFormat(messageFormat);
         }
 
         @Override
@@ -272,6 +293,11 @@ public String getMessageFormat() {
         public String getWhere() {
             return where;
         }
+
+        @Override
+        public String formatMessage(Object[] args) {
+            return logMessageIdFormat.formatMessage(args);
+        }
     }
 
     enum LogMessageId4Param implements LogMessageId {
@@ -286,10 +312,12 @@ enum LogMessageId4Param implements LogMessageId {
 
         private final String where;
         private final String messageFormat;
+        private final LogMessageIdFormat logMessageIdFormat;
 
         LogMessageId4Param(String where, String messageFormat) {
             this.where = where;
             this.messageFormat = messageFormat;
+            this.logMessageIdFormat = new LogMessageIdFormat(messageFormat);
         }
 
         @Override
@@ -306,6 +334,11 @@ public String getMessageFormat() {
         public String getWhere() {
             return where;
         }
+
+        @Override
+        public String formatMessage(Object[] args) {
+            return logMessageIdFormat.formatMessage(args);
+        }
     }
 
     enum LogMessageId5Param implements LogMessageId {
@@ -314,10 +347,12 @@ enum LogMessageId5Param implements LogMessageId {
 
         private final String where;
         private final String messageFormat;
+        private final LogMessageIdFormat logMessageIdFormat;
 
         LogMessageId5Param(String where, String messageFormat) {
             this.where = where;
             this.messageFormat = messageFormat;
+            this.logMessageIdFormat = new LogMessageIdFormat(messageFormat);
         }
 
         @Override
@@ -335,5 +370,10 @@ public String getWhere() {
             return where;
         }
 
+        @Override
+        public String formatMessage(Object[] args) {
+            return logMessageIdFormat.formatMessage(args);
+        }
+
     }
 }
diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/LogMessageIdFormat.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/LogMessageIdFormat.java
@@ -0,0 +1,51 @@
+package com.openhtmltopdf.util;
+
+import java.util.ArrayList;
+import java.util.List;
+
+class LogMessageIdFormat {
+
+    private final Object PLACEHOLDER = new Object();
+
+
+    private final List<Object> tokens;
+
+    LogMessageIdFormat(String message) {
+        this.tokens = prepareFormatter(message);
+    }
+
+    private List<Object> prepareFormatter(String messageFormat) {
+        List<Object> v = new ArrayList<>();
+        int idx = 0;
+        while(true) {
+            int newIdx = messageFormat.indexOf("{}", idx);
+            String messageSegment = newIdx == -1 ? messageFormat.substring(idx) : messageFormat.substring(idx, newIdx);
+            if (!messageSegment.isEmpty()) {
+                v.add(messageSegment);
+            }
+            if (newIdx == -1) {
+                break;
+            }
+            idx = newIdx + 2;
+            v.add(PLACEHOLDER);
+        }
+        return v;
+    }
+
+    String formatMessage(Object[] args) {
+        StringBuilder sb = new StringBuilder();
+        int size = tokens.size();
+        int argsUse = 0;
+        for (int i = 0; i < size; i++) {
+            Object f = tokens.get(i);
+            if (f == PLACEHOLDER) {
+                Object argument = args[argsUse];
+                sb.append(argument);
+                argsUse++;
+            } else {
+                sb.append(f);
+            }
+        }
+        return sb.toString();
+    }
+}