diff --git a/logback-access/src/main/java/ch/qos/logback/access/joran/action/ConfigurationAction.java b/logback-access/src/main/java/ch/qos/logback/access/joran/action/ConfigurationAction.java index bc530a1a035498c369180abfedb32366ab5a545f..0740dcf7f8835598a52d984f0f89d205141bdbda 100644 --- a/logback-access/src/main/java/ch/qos/logback/access/joran/action/ConfigurationAction.java +++ b/logback-access/src/main/java/ch/qos/logback/access/joran/action/ConfigurationAction.java @@ -15,6 +15,7 @@ package ch.qos.logback.access.joran.action; import ch.qos.logback.core.status.OnConsoleStatusListener; import ch.qos.logback.core.util.OptionHelper; +import ch.qos.logback.core.util.StatusListenerConfigHelper; import org.xml.sax.Attributes; @@ -41,7 +42,7 @@ public class ConfigurationAction extends Action { if (OptionHelper.isEmpty(debugAttrib) || debugAttrib.equals("false") || debugAttrib.equals("null")) { addInfo(INTERNAL_DEBUG_ATTR + " attribute not set"); } else { - OnConsoleStatusListener.addNewInstanceToContext(context); + StatusListenerConfigHelper.addOnConsoleListenerInstance(context, new OnConsoleStatusListener()); } new ContextUtil(context).addHostNameAsProperty(); diff --git a/logback-access/src/main/java/ch/qos/logback/access/tomcat/LogbackValve.java b/logback-access/src/main/java/ch/qos/logback/access/tomcat/LogbackValve.java index 84e116d105b2c610a6184fd4445a01799988afb9..02fcf2e56413807dc3707d20a2d548a8559cdabd 100644 --- a/logback-access/src/main/java/ch/qos/logback/access/tomcat/LogbackValve.java +++ b/logback-access/src/main/java/ch/qos/logback/access/tomcat/LogbackValve.java @@ -62,6 +62,7 @@ import ch.qos.logback.core.status.WarnStatus; import ch.qos.logback.core.util.ExecutorServiceUtil; import ch.qos.logback.core.util.Loader; import ch.qos.logback.core.util.OptionHelper; +import ch.qos.logback.core.util.StatusListenerConfigHelper; import ch.qos.logback.core.util.StatusPrinter; //import org.apache.catalina.Lifecycle; @@ -150,7 +151,7 @@ public class LogbackValve extends ValveBase implements Lifecycle, Context, Appen } if (!quiet) { - OnConsoleStatusListener.addNewInstanceToContext(this); + StatusListenerConfigHelper.addOnConsoleListenerInstance(this, new OnConsoleStatusListener()); } started = true; diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/joran/action/ConfigurationAction.java b/logback-classic/src/main/java/ch/qos/logback/classic/joran/action/ConfigurationAction.java index 87b9960812311d1684e88b2b42100d2e34e0276a..a3df544865e5ef24a9416218d964c144944fa338 100644 --- a/logback-classic/src/main/java/ch/qos/logback/classic/joran/action/ConfigurationAction.java +++ b/logback-classic/src/main/java/ch/qos/logback/classic/joran/action/ConfigurationAction.java @@ -14,16 +14,18 @@ package ch.qos.logback.classic.joran.action; import ch.qos.logback.classic.util.EnvUtil; -import ch.qos.logback.core.status.OnConsoleStatusListener; + import org.xml.sax.Attributes; import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.turbo.ReconfigureOnChangeFilter; import ch.qos.logback.core.joran.action.Action; import ch.qos.logback.core.joran.spi.InterpretationContext; +import ch.qos.logback.core.status.OnConsoleStatusListener; import ch.qos.logback.core.util.ContextUtil; import ch.qos.logback.core.util.Duration; import ch.qos.logback.core.util.OptionHelper; +import ch.qos.logback.core.util.StatusListenerConfigHelper; public class ConfigurationAction extends Action { static final String INTERNAL_DEBUG_ATTR = "debug"; @@ -49,7 +51,7 @@ public class ConfigurationAction extends Action { || debugAttrib.equalsIgnoreCase("null")) { addInfo(INTERNAL_DEBUG_ATTR + " attribute not set"); } else { - OnConsoleStatusListener.addNewInstanceToContext(context); + StatusListenerConfigHelper.addOnConsoleListenerInstance(context, new OnConsoleStatusListener()); } processScanAttrib(ic, attributes); diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/net/SMTPAppender_GreenTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/net/SMTPAppender_GreenTest.java index 2b76bb0eb16f8bdad271b02e0fd127cda3d27f4d..8d67bed38ffd0ca4dd1c5b657f743d1e7da02737 100755 --- a/logback-classic/src/test/java/ch/qos/logback/classic/net/SMTPAppender_GreenTest.java +++ b/logback-classic/src/test/java/ch/qos/logback/classic/net/SMTPAppender_GreenTest.java @@ -26,6 +26,7 @@ import ch.qos.logback.core.joran.spi.JoranException; import ch.qos.logback.core.status.OnConsoleStatusListener; import ch.qos.logback.core.testUtil.EnvUtilForTests; import ch.qos.logback.core.testUtil.RandomUtil; +import ch.qos.logback.core.util.StatusListenerConfigHelper; import com.icegreen.greenmail.util.GreenMail; import com.icegreen.greenmail.util.GreenMailUtil; @@ -71,7 +72,7 @@ public class SMTPAppender_GreenTest { @Before public void setUp() throws Exception { - OnConsoleStatusListener.addNewInstanceToContext(loggerContext); + StatusListenerConfigHelper.addOnConsoleListenerInstance(loggerContext, new OnConsoleStatusListener()); MDC.clear(); ServerSetup serverSetup = new ServerSetup(port, "localhost", ServerSetup.PROTOCOL_SMTP); diff --git a/logback-core/src/main/java/ch/qos/logback/core/BasicStatusManager.java b/logback-core/src/main/java/ch/qos/logback/core/BasicStatusManager.java index 0d53877f0a3e8d603482c24bd6f448c32e3daf3d..2b3a05129bb86844014b44aad116c33d9219ffbe 100755 --- a/logback-core/src/main/java/ch/qos/logback/core/BasicStatusManager.java +++ b/logback-core/src/main/java/ch/qos/logback/core/BasicStatusManager.java @@ -119,7 +119,7 @@ public class BasicStatusManager implements StatusManager { } statusListenerList.add(listener); } - return false; + return true; } private boolean checkForPresence(List statusListenerList, Class aClass) { diff --git a/logback-core/src/main/java/ch/qos/logback/core/joran/action/StatusListenerAction.java b/logback-core/src/main/java/ch/qos/logback/core/joran/action/StatusListenerAction.java index 2c446da189396fb104bdee8762709c0c3b47fb91..b3f207539f70e7e038de8cfb836335b96cb14e14 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/joran/action/StatusListenerAction.java +++ b/logback-core/src/main/java/ch/qos/logback/core/joran/action/StatusListenerAction.java @@ -27,10 +27,12 @@ public class StatusListenerAction extends Action { boolean inError = false; + Boolean effectivelyAdded = null; StatusListener statusListener = null; public void begin(InterpretationContext ec, String name, Attributes attributes) throws ActionException { inError = false; + effectivelyAdded = null; String className = attributes.getValue(CLASS_ATTRIBUTE); if (OptionHelper.isEmpty(className)) { addError("Missing class name for statusListener. Near [" @@ -42,7 +44,7 @@ public class StatusListenerAction extends Action { try { statusListener = (StatusListener) OptionHelper.instantiateByClassName( className, StatusListener.class, context); - ec.getContext().getStatusManager().add(statusListener); + effectivelyAdded = ec.getContext().getStatusManager().add(statusListener); if (statusListener instanceof ContextAware) { ((ContextAware) statusListener).setContext(context); } @@ -64,7 +66,7 @@ public class StatusListenerAction extends Action { if (inError) { return; } - if (statusListener instanceof LifeCycle) { + if (isEffectivelyAdded() && statusListener instanceof LifeCycle) { ((LifeCycle) statusListener).start(); } Object o = ec.peekObject(); @@ -74,4 +76,10 @@ public class StatusListenerAction extends Action { ec.popObject(); } } + + private boolean isEffectivelyAdded() { + if(effectivelyAdded == null) + return false; + return effectivelyAdded; + } } diff --git a/logback-core/src/main/java/ch/qos/logback/core/status/OnConsoleStatusListener.java b/logback-core/src/main/java/ch/qos/logback/core/status/OnConsoleStatusListener.java index 0cc46307c1959d3c0ce2f0fdb926a6144ef6103b..591766059b766290b6eff756b19c1bbf28d92b91 100755 --- a/logback-core/src/main/java/ch/qos/logback/core/status/OnConsoleStatusListener.java +++ b/logback-core/src/main/java/ch/qos/logback/core/status/OnConsoleStatusListener.java @@ -13,8 +13,6 @@ */ package ch.qos.logback.core.status; -import ch.qos.logback.core.Context; - import java.io.PrintStream; /** @@ -29,19 +27,5 @@ public class OnConsoleStatusListener extends OnPrintStreamStatusListenerBase { return System.out; } - /** - * This utility method adds a new OnConsoleStatusListener to the context - * passed as parameter. - * - * @param context - * @since 1.0.1 - */ - static public void addNewInstanceToContext(Context context) { - OnConsoleStatusListener onConsoleStatusListener = new OnConsoleStatusListener(); - onConsoleStatusListener.setContext(context); - onConsoleStatusListener.start(); - context.getStatusManager().add(onConsoleStatusListener); - } - } diff --git a/logback-core/src/main/java/ch/qos/logback/core/status/OnPrintStreamStatusListenerBase.java b/logback-core/src/main/java/ch/qos/logback/core/status/OnPrintStreamStatusListenerBase.java index fb531bf98ac90692488e787ed1e1d1f07ab4ff40..6635d138d3071fa91918fa7533b1cc5ba74197ec 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/status/OnPrintStreamStatusListenerBase.java +++ b/logback-core/src/main/java/ch/qos/logback/core/status/OnPrintStreamStatusListenerBase.java @@ -26,68 +26,76 @@ import java.util.List; */ abstract class OnPrintStreamStatusListenerBase extends ContextAwareBase implements StatusListener, LifeCycle { - boolean isStarted = false; - - static final long DEFAULT_RETROSPECTIVE = 300; - long retrospective = DEFAULT_RETROSPECTIVE; - - - /** - * The PrintStream used by derived classes - * @return - */ - abstract protected PrintStream getPrintStream(); - - private void print(Status status) { - StringBuilder sb = new StringBuilder(); - StatusPrinter.buildStr(sb, "", status); - getPrintStream().print(sb); - } - - public void addStatusEvent(Status status) { - if (!isStarted) - return; - print(status); - } - - /** - * Print status messages retrospectively - */ - private void retrospectivePrint() { - if(context == null) - return; - long now = System.currentTimeMillis(); - StatusManager sm = context.getStatusManager(); - List statusList = sm.getCopyOfStatusList(); - for (Status status : statusList) { - long timestamp = status.getDate(); - if (now - timestamp < retrospective) { + boolean isStarted = false; + + static final long DEFAULT_RETROSPECTIVE = 300; + long retrospectiveThresold = DEFAULT_RETROSPECTIVE; + + /** + * The PrintStream used by derived classes + * @return + */ + abstract protected PrintStream getPrintStream(); + + private void print(Status status) { + StringBuilder sb = new StringBuilder(); + StatusPrinter.buildStr(sb, "", status); + getPrintStream().print(sb); + } + + public void addStatusEvent(Status status) { + if (!isStarted) + return; print(status); - } } - } - public void start() { - isStarted = true; - if (retrospective > 0) { - retrospectivePrint(); + /** + * Print status messages retrospectively + */ + private void retrospectivePrint() { + if (context == null) + return; + long now = System.currentTimeMillis(); + StatusManager sm = context.getStatusManager(); + List statusList = sm.getCopyOfStatusList(); + for (Status status : statusList) { + long timestampOfStatusMesage = status.getDate(); + if (isElapsedTimeLongerThanThreshold(now, timestampOfStatusMesage)) { + print(status); + } + } } - } - public void setRetrospective(long retrospective) { - this.retrospective = retrospective; - } + private boolean isElapsedTimeLongerThanThreshold(long now, long timestamp) { + long elapsedTime = now - timestamp; + return elapsedTime < retrospectiveThresold; + } - public long getRetrospective() { - return retrospective; - } + /** + * Invoking the start method can cause the instance to print status messages created less than + * value of retrospectiveThresold. + */ + public void start() { + isStarted = true; + if (retrospectiveThresold > 0) { + retrospectivePrint(); + } + } - public void stop() { - isStarted = false; - } + public void setRetrospective(long retrospective) { + this.retrospectiveThresold = retrospective; + } + + public long getRetrospective() { + return retrospectiveThresold; + } - public boolean isStarted() { - return isStarted; - } + public void stop() { + isStarted = false; + } + + public boolean isStarted() { + return isStarted; + } } diff --git a/logback-core/src/main/java/ch/qos/logback/core/util/StatusListenerConfigHelper.java b/logback-core/src/main/java/ch/qos/logback/core/util/StatusListenerConfigHelper.java index c1e39ec228c6711d461338d8e7c4e6280925a8ec..1ad0c572151b7f75b913ab8f88659b3d9a489cd2 100755 --- a/logback-core/src/main/java/ch/qos/logback/core/util/StatusListenerConfigHelper.java +++ b/logback-core/src/main/java/ch/qos/logback/core/util/StatusListenerConfigHelper.java @@ -61,4 +61,19 @@ public class StatusListenerConfigHelper { return null; } } + + /** + * This utility method adds a new OnConsoleStatusListener to the context + * passed as parameter. + * + * @param context + * @since 1.0.1 + */ + static public void addOnConsoleListenerInstance(Context context, OnConsoleStatusListener onConsoleStatusListener) { + onConsoleStatusListener.setContext(context); + boolean effectivelyAdded = context.getStatusManager().add(onConsoleStatusListener); + if(effectivelyAdded) { + onConsoleStatusListener.start(); + } + } } diff --git a/logback-core/src/test/java/ch/qos/logback/core/BasicStatusManagerTest.java b/logback-core/src/test/java/ch/qos/logback/core/BasicStatusManagerTest.java index 3b6205e5fc3e5435e6d78c50706e75008ff6fc08..92d52eb429c34c0a0e1e0dbfa75eb47157470d55 100755 --- a/logback-core/src/test/java/ch/qos/logback/core/BasicStatusManagerTest.java +++ b/logback-core/src/test/java/ch/qos/logback/core/BasicStatusManagerTest.java @@ -15,14 +15,14 @@ package ch.qos.logback.core; import static ch.qos.logback.core.BasicStatusManager.MAX_HEADER_COUNT; import static ch.qos.logback.core.BasicStatusManager.TAIL_SIZE; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.*; import java.util.ArrayList; import java.util.List; import ch.qos.logback.core.status.OnConsoleStatusListener; import ch.qos.logback.core.status.StatusListener; + import org.junit.Test; import ch.qos.logback.core.status.ErrorStatus; @@ -73,14 +73,14 @@ public class BasicStatusManagerTest { OnConsoleStatusListener sl1 = new OnConsoleStatusListener(); sl1.start(); - bsm.add(sl0); + assertTrue(bsm.add(sl0)); { List listeners = bsm.getCopyOfStatusListenerList(); assertEquals(1, listeners.size()); } - bsm.add(sl1); + assertFalse(bsm.add(sl1)); { List listeners = bsm.getCopyOfStatusListenerList(); assertEquals(1, listeners.size()); diff --git a/logback-core/src/test/java/ch/qos/logback/core/util/StatusListenerConfigHelperTest.java b/logback-core/src/test/java/ch/qos/logback/core/util/StatusListenerConfigHelperTest.java new file mode 100755 index 0000000000000000000000000000000000000000..2694f7840e5f4cd1918bd8120e32c7c167910e0b --- /dev/null +++ b/logback-core/src/test/java/ch/qos/logback/core/util/StatusListenerConfigHelperTest.java @@ -0,0 +1,51 @@ +package ch.qos.logback.core.util; + +import static org.junit.Assert.*; + +import java.util.List; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import ch.qos.logback.core.Context; +import ch.qos.logback.core.ContextBase; +import ch.qos.logback.core.status.OnConsoleStatusListener; +import ch.qos.logback.core.status.StatusListener; +import ch.qos.logback.core.status.StatusManager; + +public class StatusListenerConfigHelperTest { + + Context context = new ContextBase(); + StatusManager sm = context.getStatusManager(); + + @Before + public void setUp() throws Exception { + } + + @After + public void tearDown() throws Exception { + } + + @Test + public void addOnConsoleListenerInstanceShouldNotStartSecondListener() { + OnConsoleStatusListener ocl0 = new OnConsoleStatusListener(); + OnConsoleStatusListener ocl1 = new OnConsoleStatusListener(); + + StatusListenerConfigHelper.addOnConsoleListenerInstance(context, ocl0); + { + List listeners = sm.getCopyOfStatusListenerList(); + assertEquals(1, listeners.size()); + assertTrue(ocl0.isStarted()); + } + + // second listener should not have been started + StatusListenerConfigHelper.addOnConsoleListenerInstance(context, ocl1); + { + List listeners = sm.getCopyOfStatusListenerList(); + assertEquals(1, listeners.size()); + assertFalse(ocl1.isStarted()); + } + } + +}