Fix LOGBACK-1194

qos-ch · Jan 22, 2018 · d992543 · d992543 · matsandreassen · Feb 23, 2018
1 parent 3b9d598
commit d992543
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 10 deletions.
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
@@ -39,7 +39,8 @@ public class ConfigurationAction extends Action {
     static final String SCAN_ATTR = "scan";
     static final String SCAN_PERIOD_ATTR = "scanPeriod";
     static final String DEBUG_SYSTEM_PROPERTY_KEY = "logback.debug";
-
+    static final Duration SCAN_PERIOD_DEFAULT = Duration.buildByMinutes(1);
+
     long threshold = 0;
 
     public void begin(InterpretationContext ic, String name, Attributes attributes) {
@@ -103,11 +104,7 @@ void processScanAttrib(InterpretationContext ic, Attributes attributes) {
             context.putObject(CoreConstants.RECONFIGURE_ON_CHANGE_TASK, rocTask);
 
             String scanPeriodAttrib = ic.subst(attributes.getValue(SCAN_PERIOD_ATTR));
-            Duration duration = getDuration(scanAttrib, scanPeriodAttrib);
-
-            if (duration == null) {
-                return;
-            }
+            Duration duration = getDurationOfScanPeriodAttribute(scanPeriodAttrib, SCAN_PERIOD_DEFAULT);
 
             addInfo("Will scan for changes in [" + mainURL + "] ");
             // Given that included files are encountered at a later phase, the complete list of files 
@@ -122,17 +119,22 @@ void processScanAttrib(InterpretationContext ic, Attributes attributes) {
         }
     }
 
-    private Duration getDuration(String scanAttrib, String scanPeriodAttrib) {
+    private Duration getDurationOfScanPeriodAttribute(String scanPeriodAttrib, Duration defaultDuration) {
         Duration duration = null;
 
         if (!OptionHelper.isEmpty(scanPeriodAttrib)) {
             try {
                 duration = Duration.valueOf(scanPeriodAttrib);
-
-            } catch (NumberFormatException nfe) {
-                addError("Error while converting [" + scanAttrib + "] to long", nfe);
+            } catch(IllegalStateException|IllegalArgumentException e) {
+               addWarn("Failed to parse 'scanPeriod' attribute ["+scanPeriodAttrib+"]", e);
+                // default duration will be set below
             }
         }
+
+        if(duration == null) {
+            addInfo("No 'scanPeriod' specified. Defaulting to " + defaultDuration.toString());
+            duration = defaultDuration;
+        }
         return duration;
     }
 

diff --git a/logback-classic/src/test/input/joran/roct/scan_period_default.xml b/logback-classic/src/test/input/joran/roct/scan_period_default.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!DOCTYPE configuration>
+
+<configuration scan="true">
+
+  <root level="ERROR"/>    
+
+</configuration> 
diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/joran/ReconfigureOnChangeTaskTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/joran/ReconfigureOnChangeTaskTest.java
@@ -19,6 +19,7 @@
 import static ch.qos.logback.classic.joran.ReconfigureOnChangeTask.RE_REGISTERING_PREVIOUS_SAFE_CONFIGURATION;
 import static ch.qos.logback.core.CoreConstants.RECONFIGURE_ON_CHANGE_TASK;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -75,6 +76,8 @@ public class ReconfigureOnChangeTaskTest {
 
     final static String INCLUSION_SCAN_INNER1_AS_STR = "target/test-classes/asResource/inner1.xml";
 
+    private static final String SCAN_PERIOD_DEFAULT_FILE_AS_STR =  JORAN_INPUT_PREFIX + "roct/scan_period_default.xml";
+
     LoggerContext loggerContext = new LoggerContext();
     Logger logger = loggerContext.getLogger(this.getClass());
     StatusChecker statusChecker = new StatusChecker(loggerContext);
@@ -309,6 +312,16 @@ private RunnableWithCounterAndDone[] buildRunnableArray(File configFile, UpdateT
         return rArray;
     }
 
+    @Test
+    public void checkReconfigureTaskScheduledWhenDefaultScanPeriodUsed() throws JoranException {
+        File file = new File(SCAN_PERIOD_DEFAULT_FILE_AS_STR);
+        configure(file);
+
+        final List<ScheduledFuture<?>> scheduledFutures = loggerContext.getScheduledFutures();
+        assertFalse(scheduledFutures.isEmpty());
+        StatusPrinter.print(loggerContext);
+}
+
     // check for deadlocks
     @Test(timeout = 4000L)
     public void scan_LOGBACK_474() throws JoranException, IOException, InterruptedException {

diff --git a/logback-site/src/site/pages/news.html b/logback-site/src/site/pages/news.html
@@ -58,6 +58,13 @@ <h3>, 2018, Release of version 1.3.0-alpha1</h3>
     <p>Logback's internal <code>ThreadPoolExecutor</code> now has an
     initial pool of 1 thread instead of 8 previously.</p>
 
+    <p>The <span class="option">scanPeriod</span> attribute in
+    configuration file defaults to 1 minute. The documentation stated
+    as much but the code had not followed through. This issue was
+    reported in <a
+    href="https://jira.qos.ch/browse/LOGBACK-1194">LOGBACK-1194</a>
+    with F. Buehler providing the appropriate patch.</p>
+
     <p>Removed "final" modifier for the
     <code>LoggerContext.getLogger(String)</code> method. This fixes <a
     href="https://jira.qos.ch/browse/LOGBACK-1308">LOGBACK-1308</a>