#63047 - Make POILogger subclassable

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1850040 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/java/org/apache/poi/util/CommonsLogger.java b/src/java/org/apache/poi/util/CommonsLogger.java
index 1825deaacd30923f645cef988623963ebd084b02..c5a4374fc8eab2a9a25b51470bfd8d936b932b8e 100644 (file)
--- a/src/java/org/apache/poi/util/CommonsLogger.java
+++ b/src/java/org/apache/poi/util/CommonsLogger.java
@@ -28,14 +28,13 @@ import org.apache.commons.logging.LogFactory;
  * calls as cheap as possible by performing lazy evaluation of the log
  * message.<p>
  */
-public class CommonsLogger extends POILogger
+public class CommonsLogger implements POILogger
 {
     private static final LogFactory   _creator = LogFactory.getFactory();
-    private Log             log;
+    private Log log;
 
     @Override
-    public void initialize(final String cat)
-    {
+    public void initialize(final String cat) {
         this.log = _creator.getInstance(cat);
     }   
      
@@ -46,51 +45,40 @@ public class CommonsLogger extends POILogger
      * @param obj1 The object to log.
      */
     @Override
-    protected void _log(final int level, final Object obj1)
-    {
+    public void _log(final int level, final Object obj1) {
         // FIXME: What happens if level is in between two levels (an even number)?
         // Should this be `if (level >= FATAL) ...`?
-        if(level==FATAL)
-        {
-          if(log.isFatalEnabled())
-          {
-            log.fatal(obj1);
-          }
-        }
-        else if(level==ERROR)
-        {
-          if(log.isErrorEnabled())
-          {
-            log.error(obj1);
-          }
-        }
-        else if(level==WARN)
-        {
-          if(log.isWarnEnabled())
-          {
-            log.warn(obj1);
-          }
-        }
-        else if(level==INFO)
-        {
-          if(log.isInfoEnabled())
-          {
-            log.info(obj1);
-          }
-        }
-        else if(level==DEBUG)
-        {
-          if(log.isDebugEnabled())
-          {
-            log.debug(obj1);
-          }
-        }
-        else
-        {
-          if(log.isTraceEnabled())
-          {
-            log.trace(obj1);
-          }
+        switch (level) {
+            case FATAL:
+                if (log.isFatalEnabled()) {
+                    log.fatal(obj1);
+                }
+                break;
+            case ERROR:
+                if (log.isErrorEnabled()) {
+                    log.error(obj1);
+                }
+                break;
+            case WARN:
+                if (log.isWarnEnabled()) {
+                    log.warn(obj1);
+                }
+                break;
+            case INFO:
+                if (log.isInfoEnabled()) {
+                    log.info(obj1);
+                }
+                break;
+            case DEBUG:
+                if (log.isDebugEnabled()) {
+                    log.debug(obj1);
+                }
+                break;
+            default:
+                if (log.isTraceEnabled()) {
+                    log.trace(obj1);
+                }
+                break;
         }
     }
     
@@ -102,72 +90,65 @@ public class CommonsLogger extends POILogger
      * @param exception An exception to be logged
      */
     @Override
-    protected void _log(final int level, final Object obj1,
-                    final Throwable exception) 
-    {
+    public void _log(final int level, final Object obj1, final Throwable exception) {
         // FIXME: What happens if level is in between two levels (an even number)?
         // Should this be `if (level >= FATAL) ...`?
-        if(level==FATAL)
-        {
-          if(log.isFatalEnabled())
-          {
-            if(obj1 != null)
-               log.fatal(obj1, exception);
-            else
-               log.fatal(exception);
-          }
-        }
-        else if(level==ERROR)
-        {
-          if(log.isErrorEnabled())
-          {
-            if(obj1 != null)
-               log.error(obj1, exception);
-            else
-               log.error(exception);
-          }
-        }
-        else if(level==WARN)
-        {
-          if(log.isWarnEnabled())
-          {
-            if(obj1 != null)
-               log.warn(obj1, exception);
-            else
-               log.warn(exception);
-          }
-        }
-        else if(level==INFO)
-        {
-          if(log.isInfoEnabled())
-          {
-               if(obj1 != null)
-               log.info(obj1, exception);
-               else
-                  log.info(exception);
-          }
+        switch (level) {
+            case FATAL:
+                if (log.isFatalEnabled()) {
+                    if (obj1 != null) {
+                        log.fatal(obj1, exception);
+                    } else {
+                        log.fatal(exception);
+                    }
+                }
+                break;
+            case ERROR:
+                if (log.isErrorEnabled()) {
+                    if (obj1 != null) {
+                        log.error(obj1, exception);
+                    } else {
+                        log.error(exception);
+                    }
+                }
+                break;
+            case WARN:
+                if (log.isWarnEnabled()) {
+                    if (obj1 != null) {
+                        log.warn(obj1, exception);
+                    } else {
+                        log.warn(exception);
+                    }
+                }
+                break;
+            case INFO:
+                if (log.isInfoEnabled()) {
+                    if (obj1 != null) {
+                        log.info(obj1, exception);
+                    } else {
+                        log.info(exception);
+                    }
+                }
+                break;
+            case DEBUG:
+                if (log.isDebugEnabled()) {
+                    if (obj1 != null) {
+                        log.debug(obj1, exception);
+                    } else {
+                        log.debug(exception);
+                    }
+                }
+                break;
+            default:
+                if (log.isTraceEnabled()) {
+                    if (obj1 != null) {
+                        log.trace(obj1, exception);
+                    } else {
+                        log.trace(exception);
+                    }
+                }
+                break;
         }
-        else if(level==DEBUG)
-        {
-          if(log.isDebugEnabled())
-          {
-               if(obj1 != null)
-               log.debug(obj1, exception);
-               else
-                  log.debug(exception);
-          }
-        }
-        else
-        {
-          if(log.isTraceEnabled())
-          {
-               if(obj1 != null)
-               log.trace(obj1, exception);
-               else
-                  log.trace(exception);
-          }
-        }
-
     }
 
     /**
@@ -180,46 +161,20 @@ public class CommonsLogger extends POILogger
     {
         // FIXME: What happens if level is in between two levels (an even number)?
         // Should this be `if (level >= FATAL) ...`?
-        if(level==FATAL)
-        {
-          if(log.isFatalEnabled())
-          {
-            return true;
-          }
-        }
-        else if(level==ERROR)
-        {
-          if(log.isErrorEnabled())
-          {
-            return true;
-          }
-        }
-        else if(level==WARN)
-        {
-          if(log.isWarnEnabled())
-          {
-            return true;
-          }
+        switch (level) {
+            case FATAL:
+                return log.isFatalEnabled();
+            case ERROR:
+                return log.isErrorEnabled();
+            case WARN:
+                return log.isWarnEnabled();
+            case INFO:
+                return log.isInfoEnabled();
+            case DEBUG:
+                return log.isDebugEnabled();
+            default:
+                return false;
         }
-        else if(level==INFO)
-        {
-          if(log.isInfoEnabled())
-          {
-            return true;
-          }
-        }
-        else if(level==DEBUG)
-        {
-          if(log.isDebugEnabled())
-          {
-            return true;
-          }
-        }
-
-        return false;
-
     }
-
- 
-}   // end package scope class POILogger
+}
 

diff --git a/src/java/org/apache/poi/util/NullLogger.java b/src/java/org/apache/poi/util/NullLogger.java
index e21adfaa7f46d91d6f92ecc47341e05016a44d6e..e4fab3903bdcbfacb22672ae13deed5b236eaa19 100644 (file)
--- a/src/java/org/apache/poi/util/NullLogger.java
+++ b/src/java/org/apache/poi/util/NullLogger.java
@@ -24,7 +24,7 @@ package org.apache.poi.util;
  * message.<p>
  */
 @Internal
-public class NullLogger extends POILogger {
+public class NullLogger implements POILogger {
     @Override
     public void initialize(final String cat) {
        // do nothing
@@ -38,7 +38,7 @@ public class NullLogger extends POILogger {
      */
 
     @Override
-    protected void _log(final int level, final Object obj1) {
+    public void _log(final int level, final Object obj1) {
         // do nothing
     }
 
@@ -50,7 +50,7 @@ public class NullLogger extends POILogger {
      * @param exception An exception to be logged
      */
     @Override
-    protected void _log(int level, Object obj1, final Throwable exception) {
+    public void _log(int level, Object obj1, final Throwable exception) {
         // do nothing
     }
 

diff --git a/src/java/org/apache/poi/util/POILogger.java b/src/java/org/apache/poi/util/POILogger.java
index 5603b2d07447617ee362f95829f9fbb301ef9373..627f83743e22cd7db419bde8c3e843ff230d06ba 100644 (file)
--- a/src/java/org/apache/poi/util/POILogger.java
+++ b/src/java/org/apache/poi/util/POILogger.java
@@ -21,53 +21,41 @@ package org.apache.poi.util;
  * A logger interface that strives to make it as easy as possible for
  * developers to write log calls, while simultaneously making those
  * calls as cheap as possible by performing lazy evaluation of the log
- * message.<p>
+ * message.
  */
 @Internal
-public abstract class POILogger {
+public interface POILogger {
 
-    public static final int DEBUG = 1;
-    public static final int INFO  = 3;
-    public static final int WARN  = 5;
-    public static final int ERROR = 7;
-    public static final int FATAL = 9;
+    int DEBUG = 1;
+    int INFO  = 3;
+    int WARN  = 5;
+    int ERROR = 7;
+    int FATAL = 9;
 
     /**
-     * Short strings for numeric log level. Use level as array index.
+     * Initialize the Logger - belongs to the SPI, called from the POILogFactory
+     * @param cat the String that defines the log
      */
-    protected static final String[] LEVEL_STRINGS_SHORT = {"?", "D", "?", "I", "?", "W", "?", "E", "?", "F", "?"};
-    /**
-     * Long strings for numeric log level. Use level as array index.
-     */
-    protected static final String[] LEVEL_STRINGS = {"?0?", "DEBUG", "?2?", "INFO", "?4?", "WARN", "?6?", "ERROR", "?8?", "FATAL", "?10+?"};
-
-
-    /**
-     * package scope so it cannot be instantiated outside of the util
-     * package. You need a POILogger? Go to the POILogFactory for one
-     */
-    POILogger() {
-        // no fields to initialize
-    }
-
-    abstract public void initialize(String cat);
+    void initialize(String cat);
 
     /**
-     * Log a message
+     * Log a message - belongs to the SPI, usually not called from user code
      *
      * @param level One of DEBUG, INFO, WARN, ERROR, FATAL
      * @param obj1 The object to log.  This is converted to a string.
      */
-    abstract protected void _log(int level, Object obj1);
+    @Internal
+    void _log(int level, Object obj1);
 
     /**
-     * Log a message
+     * Log a message - belongs to the SPI, usually not called from user code
      *
      * @param level One of DEBUG, INFO, WARN, ERROR, FATAL
      * @param obj1 The object to log.  This is converted to a string.
      * @param exception An exception to be logged
      */
-    abstract protected void _log(int level, Object obj1, final Throwable exception);
+    @Internal
+    void _log(int level, Object obj1, final Throwable exception);
 
 
     /**
@@ -84,7 +72,7 @@ public abstract class POILogger {
      *
      * @param level One of DEBUG, INFO, WARN, ERROR, FATAL
      */
-    abstract public boolean check(int level);
+    boolean check(int level);
 
    /**
      * Log a message. Lazily appends Object parameters together.
@@ -93,7 +81,7 @@ public abstract class POILogger {
      * @param level One of DEBUG, INFO, WARN, ERROR, FATAL
      * @param objs the objects to place in the message
      */
-    public void log(int level, Object... objs) {
+    default void log(int level, Object... objs) {
         if (!check(level)) return;
         StringBuilder sb = new StringBuilder(32);
         Throwable lastEx = null;
@@ -106,10 +94,9 @@ public abstract class POILogger {
         }
         
         String msg = sb.toString();
-        msg = msg.replaceAll("[\r\n]+", " ");  // log forging escape
+        // log forging escape
+        msg = msg.replaceAll("[\r\n]+", " ");
         
-        // somehow this ambiguity works and doesn't lead to a loop,
-        // but it's confusing ...
         if (lastEx == null) {
             _log(level, msg);
         } else {

diff --git a/src/java/org/apache/poi/util/SystemOutLogger.java b/src/java/org/apache/poi/util/SystemOutLogger.java
index 457f25410c9daa8a513f4e96ea1d9f73f3a83708..8eba4b00638ef836c463bf408a08fdba6f8c7de4 100644 (file)
--- a/src/java/org/apache/poi/util/SystemOutLogger.java
+++ b/src/java/org/apache/poi/util/SystemOutLogger.java
@@ -25,13 +25,16 @@ package org.apache.poi.util;
  * calls as cheap as possible by performing lazy evaluation of the log
  * message.
  */
-public class SystemOutLogger extends POILogger
-{
+public class SystemOutLogger implements POILogger {
+    /**
+     * Short strings for numeric log level. Use level as array index.
+     */
+    private static final String LEVEL_STRINGS_SHORT = "?D?I?W?E?F?";
+
     private String _cat;
 
     @Override
-    public void initialize(final String cat)
-    {
+    public void initialize(final String cat) {
        this._cat=cat;
     }
 
@@ -42,8 +45,7 @@ public class SystemOutLogger extends POILogger
      * @param obj1 The object to log.
      */
     @Override
-    protected void _log(final int level, final Object obj1)
-    {
+    public void _log(final int level, final Object obj1) {
        _log(level, obj1, null);
     }
 
@@ -56,13 +58,13 @@ public class SystemOutLogger extends POILogger
      */
     @Override
     @SuppressForbidden("uses printStackTrace")
-    protected void _log(final int level, final Object obj1,
-                    final Throwable exception) {
-        if (check(level)) {
-            System.out.println("[" + _cat + "]" + LEVEL_STRINGS_SHORT[Math.min(LEVEL_STRINGS_SHORT.length-1, level)] + " " + obj1);
-            if (exception != null) {
-               exception.printStackTrace(System.out);
-            }
+    public void _log(final int level, final Object obj1, final Throwable exception) {
+        if (!check(level)) {
+            return;
+        }
+        System.out.println("[" + _cat + "]" + LEVEL_STRINGS_SHORT.charAt(Math.min(LEVEL_STRINGS_SHORT.length()-1, level)) + " " + obj1);
+        if (exception != null) {
+            exception.printStackTrace(System.out);
         }
     }
 
@@ -77,8 +79,7 @@ public class SystemOutLogger extends POILogger
      * @see #FATAL
      */
     @Override
-    public boolean check(final int level)
-    {
+    public boolean check(final int level) {
         int currentLevel;
         try {
             currentLevel = Integer.parseInt(System.getProperty("poi.log.level", WARN + ""));
@@ -88,7 +89,5 @@ public class SystemOutLogger extends POILogger
 
         return level >= currentLevel;
     }
-
-
-}   // end package scope class POILogger
+}
 

diff --git a/src/testcases/org/apache/poi/util/DummyPOILogger.java b/src/testcases/org/apache/poi/util/DummyPOILogger.java
index ca8b2ca61570aab6cef94f990c418ebbf4c82a48..e220cc0a0c0666c1dc80325900cba8bbd942a1d3 100644 (file)
--- a/src/testcases/org/apache/poi/util/DummyPOILogger.java
+++ b/src/testcases/org/apache/poi/util/DummyPOILogger.java
@@ -24,7 +24,7 @@ import java.util.List;
  *  tests can see what got logged
  */
 @Internal
-public class DummyPOILogger extends POILogger {
+public class DummyPOILogger implements POILogger {
        public List<String>logged = new ArrayList<>();
 
        public void reset() {
@@ -40,12 +40,12 @@ public class DummyPOILogger extends POILogger {
        public void initialize(String cat) {}
 
     @Override
-       protected void _log(int level, Object obj1) {
+       public void _log(int level, Object obj1) {
                logged.add(level + " - " + obj1);
        }
 
     @Override
-       protected void _log(int level, Object obj1, Throwable exception) {
+       public void _log(int level, Object obj1, Throwable exception) {
                logged.add(level + " - " + obj1 + " - " + exception);
        }
 }

diff --git a/src/testcases/org/apache/poi/util/TestPOILogger.java b/src/testcases/org/apache/poi/util/TestPOILogger.java
index 7f6e6a06e5c200803f56fd169297601b4528821b..0c6765cf144cd475beae7addef0301be94ff3d6a 100644 (file)
--- a/src/testcases/org/apache/poi/util/TestPOILogger.java
+++ b/src/testcases/org/apache/poi/util/TestPOILogger.java
@@ -27,7 +27,7 @@ import org.junit.Test;
 /**
  * Tests the log class.
  */
-public final class TestPOILogger extends POILogger {
+public final class TestPOILogger implements POILogger {
     private String lastLog = "";
     private Throwable lastEx;
     
@@ -65,13 +65,13 @@ public final class TestPOILogger extends POILogger {
     }
 
     @Override
-    protected void _log(int level, Object obj1) {
+    public void _log(int level, Object obj1) {
         lastLog = (obj1 == null) ? "" : obj1.toString();
         lastEx = null;
     }
 
     @Override
-    protected void _log(int level, Object obj1, Throwable exception) {
+    public void _log(int level, Object obj1, Throwable exception) {
         lastLog = (obj1 == null) ? "" : obj1.toString();
         lastEx = exception;
     }