From 4d4ebb395ed994263b848e300cd3e282db5fd795 Mon Sep 17 00:00:00 2001 From: Ceki Gulcu Date: Mon, 29 Apr 2019 22:35:08 +0200 Subject: [PATCH] initial backward compatible implementation of fluent API --- slf4j-api/src/main/java/org/slf4j/Logger.java | 10 +- .../org/slf4j/event/DefaultLoggingEvent.java | 13 +- .../org/slf4j/event/LoggingEventAware.java | 13 + .../org/slf4j/helpers/MessageFormatter.java | 19 +- .../slf4j/spi/DefaultLoggingEventBuilder.java | 77 ++++-- .../slf4j/jul/FluentApiInvocationTest.java | 92 +++++++ .../java/org/slf4j/jul/InvocationTest.java | 225 +++++++++--------- .../java/org/slf4j/simple/InvocationTest.java | 1 - 8 files changed, 304 insertions(+), 146 deletions(-) create mode 100755 slf4j-api/src/main/java/org/slf4j/event/LoggingEventAware.java create mode 100755 slf4j-jdk14/src/test/java/org/slf4j/jul/FluentApiInvocationTest.java diff --git a/slf4j-api/src/main/java/org/slf4j/Logger.java b/slf4j-api/src/main/java/org/slf4j/Logger.java index d916ed70..a2cae5da 100644 --- a/slf4j-api/src/main/java/org/slf4j/Logger.java +++ b/slf4j-api/src/main/java/org/slf4j/Logger.java @@ -175,7 +175,7 @@ public interface Logger { */ default public LoggingEventBuilder atTrace() { if(isTraceEnabled()) { - return new DefaultLoggingEventBuilder(TRACE, this); + return new DefaultLoggingEventBuilder(this, TRACE); } else { return NOPLoggingEventBuilder.singleton(); } @@ -371,7 +371,7 @@ public interface Logger { */ default public LoggingEventBuilder atDebug() { if(isDebugEnabled()) { - return new DefaultLoggingEventBuilder(DEBUG, this); + return new DefaultLoggingEventBuilder(this, DEBUG); } else { return NOPLoggingEventBuilder.singleton(); } @@ -509,7 +509,7 @@ public interface Logger { */ default public LoggingEventBuilder atInfo() { if(isInfoEnabled()) { - return new DefaultLoggingEventBuilder(INFO, this); + return new DefaultLoggingEventBuilder(this, INFO); } else { return NOPLoggingEventBuilder.singleton(); } @@ -650,7 +650,7 @@ public interface Logger { */ default public LoggingEventBuilder atWarn() { if(isWarnEnabled()) { - return new DefaultLoggingEventBuilder(WARN, this); + return new DefaultLoggingEventBuilder(this, WARN); } else { return NOPLoggingEventBuilder.singleton(); } @@ -793,7 +793,7 @@ public interface Logger { */ default public LoggingEventBuilder atError() { if(isErrorEnabled()) { - return new DefaultLoggingEventBuilder(ERROR, this); + return new DefaultLoggingEventBuilder(this, ERROR); } else { return NOPLoggingEventBuilder.singleton(); } diff --git a/slf4j-api/src/main/java/org/slf4j/event/DefaultLoggingEvent.java b/slf4j-api/src/main/java/org/slf4j/event/DefaultLoggingEvent.java index caaeb095..ac8f21a3 100755 --- a/slf4j-api/src/main/java/org/slf4j/event/DefaultLoggingEvent.java +++ b/slf4j-api/src/main/java/org/slf4j/event/DefaultLoggingEvent.java @@ -23,7 +23,7 @@ public class DefaultLoggingEvent implements LoggingEvent { List arguments; List keyValuePairs; - Throwable cause; + Throwable throwable; String threadName; long timeStamp; @@ -84,8 +84,8 @@ public class DefaultLoggingEvent implements LoggingEvent { return keyValuePairs; } - public void setCause(Throwable cause) { - this.cause = cause; + public void setThrowable(Throwable cause) { + this.throwable = cause; } @Override @@ -103,9 +103,14 @@ public class DefaultLoggingEvent implements LoggingEvent { return message; } + public void setMessage(String message) { + this.message = message; + } + + @Override public Throwable getThrowable() { - return cause; + return throwable; } public String getThreadName() { return threadName; diff --git a/slf4j-api/src/main/java/org/slf4j/event/LoggingEventAware.java b/slf4j-api/src/main/java/org/slf4j/event/LoggingEventAware.java new file mode 100755 index 00000000..9d875985 --- /dev/null +++ b/slf4j-api/src/main/java/org/slf4j/event/LoggingEventAware.java @@ -0,0 +1,13 @@ +package org.slf4j.event; + +/** + * + * + * @author Ceki Gülcü + * @since 2.0.0 + */ +public interface LoggingEventAware { + + void log(LoggingEvent event); + +} diff --git a/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java b/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java index bdbfb190..e5df70e9 100755 --- a/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java +++ b/slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java @@ -152,6 +152,15 @@ final public class MessageFormatter { } + final public static FormattingTuple arrayFormat(final String messagePattern, final Object[] argArray) { + Throwable throwableCandidate = getThrowableCandidate(argArray); + Object[] args = argArray; + if (throwableCandidate != null) { + args = trimmedCopy(argArray); + } + return arrayFormat(messagePattern, args, throwableCandidate); + } + static final Throwable getThrowableCandidate(Object[] argArray) { if (argArray == null || argArray.length == 0) { return null; @@ -163,16 +172,6 @@ final public class MessageFormatter { } return null; } - - final public static FormattingTuple arrayFormat(final String messagePattern, final Object[] argArray) { - Throwable throwableCandidate = getThrowableCandidate(argArray); - Object[] args = argArray; - if (throwableCandidate != null) { - args = trimmedCopy(argArray); - } - return arrayFormat(messagePattern, args, throwableCandidate); - } - private static Object[] trimmedCopy(Object[] argArray) { if (argArray == null || argArray.length == 0) { throw new IllegalStateException("non-sensical empty or null argument array"); diff --git a/slf4j-api/src/main/java/org/slf4j/spi/DefaultLoggingEventBuilder.java b/slf4j-api/src/main/java/org/slf4j/spi/DefaultLoggingEventBuilder.java index 280a35ec..d9e624b2 100755 --- a/slf4j-api/src/main/java/org/slf4j/spi/DefaultLoggingEventBuilder.java +++ b/slf4j-api/src/main/java/org/slf4j/spi/DefaultLoggingEventBuilder.java @@ -6,16 +6,18 @@ import org.slf4j.Logger; import org.slf4j.Marker; import org.slf4j.event.DefaultLoggingEvent; import org.slf4j.event.Level; +import org.slf4j.event.LoggingEventAware; public class DefaultLoggingEventBuilder implements LoggingEventBuilder { DefaultLoggingEvent logggingEvent; - - - public DefaultLoggingEventBuilder(Level level, Logger logger) { + Logger logger; + + public DefaultLoggingEventBuilder(Logger logger, Level level) { + this.logger = logger; logggingEvent = new DefaultLoggingEvent(level, logger); } - + /** * Add a marker to the current logging event being built. * @@ -28,11 +30,10 @@ public class DefaultLoggingEventBuilder implements LoggingEventBuilder { logggingEvent.addMarker(marker); return this; } - - + @Override - public LoggingEventBuilder setCause(Throwable cause) { - logggingEvent.setCause(cause); + public LoggingEventBuilder setCause(Throwable t) { + logggingEvent.setThrowable(t); return this; } @@ -41,24 +42,72 @@ public class DefaultLoggingEventBuilder implements LoggingEventBuilder { logggingEvent.addArgument(p); return this; } - + @Override public LoggingEventBuilder addArgument(Supplier objectSupplier) { logggingEvent.addArgument(objectSupplier.get()); return this; } - + @Override public void log(String message) { - + logggingEvent.setMessage(message); + + if (logger instanceof LoggingEventAware) { + ((LoggingEventAware) logger).log(logggingEvent); + } else { + logViaPublicLoggerAPI(); + } + + } + + private void logViaPublicLoggerAPI() { + Object[] argArray = logggingEvent.getArgumentArray(); + int argLen = argArray == null ? 0 : argArray.length; + + Throwable t = logggingEvent.getThrowable(); + int tLen = t == null ? 0 : 1; + + String msg = logggingEvent.getMessage(); + + Object[] combinedArguments = new Object[argLen + tLen]; + + if (argArray != null) { + System.arraycopy(argArray, 0, combinedArguments, 0, argLen); + } + if (t != null) { + combinedArguments[argLen] = t; + } + + switch (logggingEvent.getLevel()) { + case TRACE: + logger.trace(msg, combinedArguments); + break; + case DEBUG: + logger.debug(msg, combinedArguments); + break; + case INFO: + logger.info(msg, combinedArguments); + break; + case WARN: + logger.warn(msg, combinedArguments); + break; + case ERROR: + logger.error(msg, combinedArguments); + break; + } + } @Override public void log(Supplier messageSupplier) { + if (messageSupplier == null) { + log((String) null); + } else { + log(messageSupplier.get()); + } } - - - + @Override public LoggingEventBuilder addKeyValue(String key, Object value) { logggingEvent.addKeyValue(key, value); diff --git a/slf4j-jdk14/src/test/java/org/slf4j/jul/FluentApiInvocationTest.java b/slf4j-jdk14/src/test/java/org/slf4j/jul/FluentApiInvocationTest.java new file mode 100755 index 00000000..6cd91d37 --- /dev/null +++ b/slf4j-jdk14/src/test/java/org/slf4j/jul/FluentApiInvocationTest.java @@ -0,0 +1,92 @@ +package org.slf4j.jul; + +import static org.junit.Assert.assertEquals; + +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogRecord; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class FluentApiInvocationTest { + + ListHandler listHandler = new ListHandler(); + java.util.logging.Logger root = java.util.logging.Logger.getLogger(""); + Level oldLevel; + Logger logger = LoggerFactory.getLogger(this.getClass()); + + @Before + public void setUp() throws Exception { + oldLevel = root.getLevel(); + root.setLevel(Level.FINE); + // removeAllHandlers(root); + root.addHandler(listHandler); + } + + @After + public void tearDown() throws Exception { + root.setLevel(oldLevel); + removeListHandlers(root); + } + + void removeListHandlers(java.util.logging.Logger logger) { + Handler[] handlers = logger.getHandlers(); + for (Handler h : handlers) { + if (h instanceof ListHandler) + logger.removeHandler(h); + } + } + + @Test + public void singleMessage() { + String msg = "Hello world."; + logger.atDebug().log(msg); + assertLogMessage(msg, 0); + } + + @Test + public void messageWithArguments() { + String msg = "Hello {}."; + logger.atDebug().addArgument("world").log(msg); + assertLogMessage("Hello world.", 0); + } + + + @Test + public void messageWithThrowable() { + String msg = "Hello world."; + Throwable t = new IllegalStateException(); + logger.atDebug().setCause(t).log(msg); + assertLogMessage("Hello world.", 0); + assertThrowable(t, 0); + } + + @Test + public void messageWithArgumentsAndThrowable() { + String msg = "Hello {}."; + Throwable t = new IllegalStateException(); + + logger.atDebug().setCause(t).addArgument("world").log(msg); + assertLogMessage("Hello world.", 0); + assertThrowable(t, 0); + } + + + + private void assertLogMessage(String expected, int index) { + LogRecord logRecord = listHandler.recordList.get(index); + Assert.assertNotNull(logRecord); + assertEquals(expected, logRecord.getMessage()); + } + + private void assertThrowable(Throwable expected, int index) { + LogRecord logRecord = listHandler.recordList.get(index); + Assert.assertNotNull(logRecord); + assertEquals(expected, logRecord.getThrown()); + } +} diff --git a/slf4j-jdk14/src/test/java/org/slf4j/jul/InvocationTest.java b/slf4j-jdk14/src/test/java/org/slf4j/jul/InvocationTest.java index 4e940a27..6ef3700f 100755 --- a/slf4j-jdk14/src/test/java/org/slf4j/jul/InvocationTest.java +++ b/slf4j-jdk14/src/test/java/org/slf4j/jul/InvocationTest.java @@ -46,139 +46,140 @@ import static org.junit.Assert.fail; */ public class InvocationTest { - Level oldLevel; - java.util.logging.Logger root = java.util.logging.Logger.getLogger(""); + Level oldLevel; + java.util.logging.Logger root = java.util.logging.Logger.getLogger(""); - ListHandler listHandler = new ListHandler(); + ListHandler listHandler = new ListHandler(); - @Before - public void setUp() throws Exception { - oldLevel = root.getLevel(); - root.setLevel(Level.FINE); - // removeAllHandlers(root); - root.addHandler(listHandler); - } + @Before + public void setUp() throws Exception { + oldLevel = root.getLevel(); + root.setLevel(Level.FINE); + // removeAllHandlers(root); + root.addHandler(listHandler); + } - @After - public void tearDown() throws Exception { - root.setLevel(oldLevel); - removeListHandlers(root); - } + @After + public void tearDown() throws Exception { + root.setLevel(oldLevel); + removeListHandlers(root); + } - @Test - public void smoke() { - Logger logger = LoggerFactory.getLogger("test1"); - logger.debug("Hello world."); - assertLogMessage("Hello world.", 0); - } + void removeListHandlers(java.util.logging.Logger logger) { + Handler[] handlers = logger.getHandlers(); + for (Handler h : handlers) { + if (h instanceof ListHandler) + logger.removeHandler(h); + } + } - @Test - public void verifyMessageFormatting() { - Integer i1 = new Integer(1); - Integer i2 = new Integer(2); - Integer i3 = new Integer(3); - Exception e = new Exception("This is a test exception."); - Logger logger = LoggerFactory.getLogger("test2"); + @Test + public void smoke() { + Logger logger = LoggerFactory.getLogger("test1"); + logger.debug("Hello world."); + assertLogMessage("Hello world.", 0); + } - int index = 0; - logger.debug("Hello world"); - assertLogMessage("Hello world", index++); + @Test + public void verifyMessageFormatting() { + Integer i1 = new Integer(1); + Integer i2 = new Integer(2); + Integer i3 = new Integer(3); + Exception e = new Exception("This is a test exception."); + Logger logger = LoggerFactory.getLogger("test2"); - logger.debug("Hello world {}", i1); - assertLogMessage("Hello world " + i1, index++); + int index = 0; + logger.debug("Hello world"); + assertLogMessage("Hello world", index++); - logger.debug("val={} val={}", i1, i2); - assertLogMessage("val=1 val=2", index++); + logger.debug("Hello world {}", i1); + assertLogMessage("Hello world " + i1, index++); - logger.debug("val={} val={} val={}", new Object[] { i1, i2, i3 }); - assertLogMessage("val=1 val=2 val=3", index++); + logger.debug("val={} val={}", i1, i2); + assertLogMessage("val=1 val=2", index++); - logger.debug("Hello world 2", e); - assertLogMessage("Hello world 2", index); - assertException(e.getClass(), index++); - logger.info("Hello world 2."); + logger.debug("val={} val={} val={}", new Object[] { i1, i2, i3 }); + assertLogMessage("val=1 val=2 val=3", index++); - logger.warn("Hello world 3."); - logger.warn("Hello world 3", e); + logger.debug("Hello world 2", e); + assertLogMessage("Hello world 2", index); + assertException(e.getClass(), index++); + logger.info("Hello world 2."); - logger.error("Hello world 4."); - logger.error("Hello world {}", new Integer(3)); - logger.error("Hello world 4.", e); - } + logger.warn("Hello world 3."); + logger.warn("Hello world 3", e); - @Test - public void testNull() { - Logger logger = LoggerFactory.getLogger("testNull"); - logger.debug(null); - logger.info(null); - logger.warn(null); - logger.error(null); + logger.error("Hello world 4."); + logger.error("Hello world {}", new Integer(3)); + logger.error("Hello world 4.", e); + } - Exception e = new Exception("This is a test exception."); - logger.debug(null, e); - logger.info(null, e); - logger.warn(null, e); - logger.error(null, e); - } + @Test + public void testNull() { + Logger logger = LoggerFactory.getLogger("testNull"); + logger.debug(null); + logger.info(null); + logger.warn(null); + logger.error(null); - @Test - public void testMarker() { - Logger logger = LoggerFactory.getLogger("testMarker"); - Marker blue = MarkerFactory.getMarker("BLUE"); - logger.debug(blue, "hello"); - logger.info(blue, "hello"); - logger.warn(blue, "hello"); - logger.error(blue, "hello"); + Exception e = new Exception("This is a test exception."); + logger.debug(null, e); + logger.info(null, e); + logger.warn(null, e); + logger.error(null, e); + } - logger.debug(blue, "hello {}", "world"); - logger.info(blue, "hello {}", "world"); - logger.warn(blue, "hello {}", "world"); - logger.error(blue, "hello {}", "world"); + @Test + public void testMarker() { + Logger logger = LoggerFactory.getLogger("testMarker"); + Marker blue = MarkerFactory.getMarker("BLUE"); + logger.debug(blue, "hello"); + logger.info(blue, "hello"); + logger.warn(blue, "hello"); + logger.error(blue, "hello"); - logger.debug(blue, "hello {} and {} ", "world", "universe"); - logger.info(blue, "hello {} and {} ", "world", "universe"); - logger.warn(blue, "hello {} and {} ", "world", "universe"); - logger.error(blue, "hello {} and {} ", "world", "universe"); - } + logger.debug(blue, "hello {}", "world"); + logger.info(blue, "hello {}", "world"); + logger.warn(blue, "hello {}", "world"); + logger.error(blue, "hello {}", "world"); - @Test - public void testMDC() { - MDC.put("k", "v"); - assertNotNull(MDC.get("k")); - assertEquals("v", MDC.get("k")); + logger.debug(blue, "hello {} and {} ", "world", "universe"); + logger.info(blue, "hello {} and {} ", "world", "universe"); + logger.warn(blue, "hello {} and {} ", "world", "universe"); + logger.error(blue, "hello {} and {} ", "world", "universe"); + } - MDC.remove("k"); - assertNull(MDC.get("k")); + @Test + public void testMDC() { + MDC.put("k", "v"); + assertNotNull(MDC.get("k")); + assertEquals("v", MDC.get("k")); - MDC.put("k1", "v1"); - assertEquals("v1", MDC.get("k1")); - MDC.clear(); - assertNull(MDC.get("k1")); + MDC.remove("k"); + assertNull(MDC.get("k")); - try { - MDC.put(null, "x"); - fail("null keys are invalid"); - } catch (IllegalArgumentException e) { - } - } - - private void assertLogMessage(String expected, int index) { - LogRecord logRecord = listHandler.recordList.get(index); - Assert.assertNotNull(logRecord); - assertEquals(expected, logRecord.getMessage()); - } + MDC.put("k1", "v1"); + assertEquals("v1", MDC.get("k1")); + MDC.clear(); + assertNull(MDC.get("k1")); - private void assertException(Class exceptionType, int index) { - LogRecord logRecord = listHandler.recordList.get(index); - Assert.assertNotNull(logRecord); - assertEquals(exceptionType, logRecord.getThrown().getClass()); - } + try { + MDC.put(null, "x"); + fail("null keys are invalid"); + } catch (IllegalArgumentException e) { + } + } + + private void assertLogMessage(String expected, int index) { + LogRecord logRecord = listHandler.recordList.get(index); + Assert.assertNotNull(logRecord); + assertEquals(expected, logRecord.getMessage()); + } + + private void assertException(Class exceptionType, int index) { + LogRecord logRecord = listHandler.recordList.get(index); + Assert.assertNotNull(logRecord); + assertEquals(exceptionType, logRecord.getThrown().getClass()); + } - void removeListHandlers(java.util.logging.Logger logger) { - Handler[] handlers = logger.getHandlers(); - for (Handler h : handlers) { - if (h instanceof ListHandler) - logger.removeHandler(h); - } - } } diff --git a/slf4j-simple/src/test/java/org/slf4j/simple/InvocationTest.java b/slf4j-simple/src/test/java/org/slf4j/simple/InvocationTest.java index 709d4e00..38f69367 100644 --- a/slf4j-simple/src/test/java/org/slf4j/simple/InvocationTest.java +++ b/slf4j-simple/src/test/java/org/slf4j/simple/InvocationTest.java @@ -54,7 +54,6 @@ public class InvocationTest { @After public void tearDown() throws Exception { - System.setErr(old); }