From 8db668438c9ef10d4b750ce7e4d2f229ef0feffc Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Mon, 31 Dec 2018 12:07:37 +0000 Subject: [PATCH] #63047 - Make POILogger subclassable git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1850040 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/poi/util/CommonsLogger.java | 255 ++++++++---------- src/java/org/apache/poi/util/NullLogger.java | 6 +- src/java/org/apache/poi/util/POILogger.java | 53 ++-- .../org/apache/poi/util/SystemOutLogger.java | 35 ++- .../org/apache/poi/util/DummyPOILogger.java | 6 +- .../org/apache/poi/util/TestPOILogger.java | 6 +- 6 files changed, 151 insertions(+), 210 deletions(-) diff --git a/src/java/org/apache/poi/util/CommonsLogger.java b/src/java/org/apache/poi/util/CommonsLogger.java index 1825deaacd..c5a4374fc8 100644 --- 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.

*/ -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 e21adfaa7f..e4fab3903b 100644 --- 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.

*/ @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 5603b2d074..627f83743e 100644 --- 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.

+ * 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 457f25410c..8eba4b0063 100644 --- 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 ca8b2ca615..e220cc0a0c 100644 --- 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 Listlogged = 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 7f6e6a06e5..0c6765cf14 100644 --- 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; } -- 2.39.5