[logback-dev] svn commit: r2322 - in logback/trunk: logback-core/src/main/java/ch/qos/logback/core logback-core/src/main/java/ch/qos/logback/core/rolling logback-core/src/test/java/ch/qos/logback/core/rolling logback-core/src/test/java/ch/qos/logback/core/status logback-site/src/site/pages logback-site/src/site/pages/manual

noreply.ceki at qos.ch noreply.ceki at qos.ch
Wed Jul 8 10:24:28 CEST 2009


Author: ceki
Date: Wed Jul  8 10:24:27 2009
New Revision: 2322

Modified:
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/CoreConstants.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/FileAppender.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/FixedWindowRollingPolicy.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.java
   logback/trunk/logback-core/src/test/java/ch/qos/logback/core/rolling/RollingFileAppenderTest.java
   logback/trunk/logback-core/src/test/java/ch/qos/logback/core/status/StatusChecker.java
   logback/trunk/logback-site/src/site/pages/codes.html
   logback/trunk/logback-site/src/site/pages/manual/appenders.html
   logback/trunk/logback-site/src/site/pages/news.html

Log:
Fix LBCORE-94  Within A RollingFileAppender, if the File property is declared after 
any rolling or triggering policies, then generate an error status message.

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/CoreConstants.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/CoreConstants.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/CoreConstants.java	Wed Jul  8 10:24:27 2009
@@ -81,4 +81,6 @@
   static public final char DOT = '.';
 
   static public final char TAB = '\t';
+  
+  static public final String SEE_FNP_NOT_SET = "See also http://logback.qos.ch/codes.html#tbr_fnp_not_set";
 }

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/FileAppender.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/FileAppender.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/FileAppender.java	Wed Jul  8 10:24:27 2009
@@ -73,9 +73,7 @@
       // trailing spaces in file names.
       String val = file.trim();
       fileName = val;      
-
     }
-
   }
 
   /**

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/FixedWindowRollingPolicy.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/FixedWindowRollingPolicy.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/FixedWindowRollingPolicy.java	Wed Jul  8 10:24:27 2009
@@ -11,6 +11,7 @@
 
 import java.io.File;
 
+import ch.qos.logback.core.CoreConstants;
 import ch.qos.logback.core.rolling.helper.CompressionMode;
 import ch.qos.logback.core.rolling.helper.Compressor;
 import ch.qos.logback.core.rolling.helper.FileNamePattern;
@@ -28,7 +29,6 @@
  */
 public class FixedWindowRollingPolicy extends RollingPolicyBase {
   static final String FNP_NOT_SET = "The \"FileNamePattern\" property must be set before using FixedWindowRollingPolicy. ";
-  static final String SEE_FNP_NOT_SET = "See also http://logback.qos.ch/codes.html#tbr_fnp_not_set";
   static final String PRUDENT_MODE_UNSUPPORTED = "See also http://logback.qos.ch/codes.html#tbr_fnp_prudent_unsupported";
   static final String SEE_PARENT_FN_NOT_SET = "Please refer to http://logback.qos.ch/codes.html#fwrp_parentFileName_not_set";
   int maxIndex;
@@ -54,8 +54,8 @@
       determineCompressionMode();
     } else {
       addError(FNP_NOT_SET);
-      addError(SEE_FNP_NOT_SET);
-      throw new IllegalStateException(FNP_NOT_SET + SEE_FNP_NOT_SET);
+      addError(CoreConstants.SEE_FNP_NOT_SET);
+      throw new IllegalStateException(FNP_NOT_SET + CoreConstants.SEE_FNP_NOT_SET);
     }
 
     if(isParentPrudent()) {

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java	Wed Jul  8 10:24:27 2009
@@ -70,6 +70,18 @@
   }
 
   @Override
+  public void setFile(String file) {
+    // http://jira.qos.ch/browse/LBCORE-94
+    // allow setting the file name to null if mandated by prudent mode
+    if(file != null && ((triggeringPolicy != null) || (rollingPolicy != null))) {
+      addError("File property must be set before any triggeringPolicy or rollingPolicy properties");
+      addError("Visit http://logback.qos.ch/codes.html#rfa_file_after for more information");
+    }
+    super.setFile(file);
+  }
+
+  
+  @Override
   public String getFile() {
     return rollingPolicy.getActiveFileName();
   }

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.java	Wed Jul  8 10:24:27 2009
@@ -13,6 +13,7 @@
 import java.util.Date;
 import java.util.concurrent.Future;
 
+import ch.qos.logback.core.CoreConstants;
 import ch.qos.logback.core.rolling.helper.AsynchronousCompressor;
 import ch.qos.logback.core.rolling.helper.CompressionMode;
 import ch.qos.logback.core.rolling.helper.Compressor;
@@ -35,7 +36,6 @@
 public class TimeBasedRollingPolicy<E> extends RollingPolicyBase implements
     TriggeringPolicy<E> {
   static final String FNP_NOT_SET = "The FileNamePattern option must be set before using TimeBasedRollingPolicy. ";
-  static final String SEE_FNP_NOT_SET = "See also http://logback.qos.ch/codes.html#tbr_fnp_not_set";
   static final int NO_DELETE_HISTORY = 0;
 
   RollingCalendar rc;
@@ -77,8 +77,8 @@
       determineCompressionMode();
     } else {
       addWarn(FNP_NOT_SET);
-      addWarn(SEE_FNP_NOT_SET);
-      throw new IllegalStateException(FNP_NOT_SET + SEE_FNP_NOT_SET);
+      addWarn(CoreConstants.SEE_FNP_NOT_SET);
+      throw new IllegalStateException(FNP_NOT_SET + CoreConstants.SEE_FNP_NOT_SET);
     }
 
     DateTokenConverter dtc = fileNamePattern.getDateTokenConverter();

Modified: logback/trunk/logback-core/src/test/java/ch/qos/logback/core/rolling/RollingFileAppenderTest.java
==============================================================================
--- logback/trunk/logback-core/src/test/java/ch/qos/logback/core/rolling/RollingFileAppenderTest.java	(original)
+++ logback/trunk/logback-core/src/test/java/ch/qos/logback/core/rolling/RollingFileAppenderTest.java	Wed Jul  8 10:24:27 2009
@@ -9,11 +9,11 @@
  */
 package ch.qos.logback.core.rolling;
 
-
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import org.junit.After;
 import org.junit.Before;
@@ -25,7 +25,9 @@
 import ch.qos.logback.core.appender.AbstractAppenderTest;
 import ch.qos.logback.core.layout.DummyLayout;
 import ch.qos.logback.core.status.Status;
+import ch.qos.logback.core.status.StatusChecker;
 import ch.qos.logback.core.status.StatusManager;
+import ch.qos.logback.core.util.StatusPrinter;
 
 public class RollingFileAppenderTest extends AbstractAppenderTest<Object> {
 
@@ -36,10 +38,11 @@
 
   @Before
   public void setUp() throws Exception {
+    // noStartTest fails if the context is set in setUp
+    // rfa.setContext(context);
+
     rfa.setLayout(new DummyLayout<Object>());
     rfa.setName("test");
-    rfa.setRollingPolicy(tbrp);
-    
     tbrp.setContext(context);
     tbrp.setParent(rfa);
   }
@@ -48,7 +51,6 @@
   public void tearDown() throws Exception {
   }
 
-  
   @Override
   protected Appender<Object> getAppender() {
     return rfa;
@@ -57,20 +59,16 @@
   @Override
   protected Appender<Object> getConfiguredAppender() {
     rfa.setContext(context);
-
     tbrp.setFileNamePattern("toto-%d.log");
     tbrp.start();
-    
+    rfa.setRollingPolicy(tbrp);
+
     rfa.start();
     return rfa;
   }
 
-
   @Test
   public void testPrudentModeLogicalImplications() {
-    tbrp.setFileNamePattern("toto-%d.log");
-    tbrp.start();
-    
     rfa.setContext(context);
     // prudent mode will force "file" property to be null
     rfa.setFile("some non null value");
@@ -78,6 +76,11 @@
     rfa.setImmediateFlush(false);
     rfa.setBufferedIO(true);
     rfa.setPrudent(true);
+
+    tbrp.setFileNamePattern("toto-%d.log");
+    tbrp.start();
+    rfa.setRollingPolicy(tbrp);
+
     rfa.start();
 
     assertTrue(rfa.getImmediateFlush());
@@ -87,23 +90,43 @@
     assertTrue(rfa.isStarted());
   }
 
-  
   @Test
   public void testPrudentModeLogicalImplicationsOnCompression() {
-    tbrp.setFileNamePattern("toto-%d.log.zip");
-    tbrp.start();
-
     rfa.setContext(context);
     rfa.setAppend(false);
     rfa.setImmediateFlush(false);
     rfa.setBufferedIO(true);
     rfa.setPrudent(true);
+
+    tbrp.setFileNamePattern("toto-%d.log.zip");
+    tbrp.start();
+    rfa.setRollingPolicy(tbrp);
+
     rfa.start();
 
     StatusManager sm = context.getStatusManager();
     assertFalse(rfa.isStarted());
     assertEquals(Status.ERROR, sm.getLevel());
   }
-  
-  
+
+  @Test
+  public void testFilePropertyAfterRollingPolicy() {
+    rfa.setContext(context);
+    rfa.setRollingPolicy(tbrp);
+    rfa.setFile("x");
+    StatusPrinter.print(context);
+    StatusChecker statusChecker = new StatusChecker(context.getStatusManager());
+    statusChecker.containsMatch(Status.ERROR,
+        "File property must be set before any triggeringPolicy ");
+  }
+
+  @Test
+  public void testFilePropertyAfterTriggeringPolicy() {
+    rfa.setContext(context);
+    rfa.setTriggeringPolicy(new SizeBasedTriggeringPolicy<Object>());
+    rfa.setFile("x");
+    StatusChecker statusChecker = new StatusChecker(context.getStatusManager());
+    statusChecker.containsMatch(Status.ERROR,
+        "File property must be set before any triggeringPolicy ");
+  }
 }

Modified: logback/trunk/logback-core/src/test/java/ch/qos/logback/core/status/StatusChecker.java
==============================================================================
--- logback/trunk/logback-core/src/test/java/ch/qos/logback/core/status/StatusChecker.java	(original)
+++ logback/trunk/logback-core/src/test/java/ch/qos/logback/core/status/StatusChecker.java	Wed Jul  8 10:24:27 2009
@@ -24,21 +24,31 @@
     this.sm = sm;
   }
 
-  public boolean containsMatch(String regex) {
-
+  public boolean containsMatch(int level, String regex) {
     Pattern p = Pattern.compile(regex);
 
-    Iterator stati = sm.getCopyOfStatusList().iterator();
-    while (stati.hasNext()) {
-      Status status = (Status) stati.next();
+    for(Status status: sm.getCopyOfStatusList()) {
+      if(level != status.getLevel()) {
+        continue;
+      }
       String msg = status.getMessage();
       Matcher matcher = p.matcher(msg);
       if (matcher.lookingAt()) {
         return true;
-      } else {
-        System.out.println("no match:" + msg);
-        System.out.println("regex   :" + regex);
-      }
+      } 
+    }
+    return false;
+    
+  }
+  
+  public boolean containsMatch(String regex) {
+    Pattern p = Pattern.compile(regex);
+    for(Status status: sm.getCopyOfStatusList()) {
+      String msg = status.getMessage();
+      Matcher matcher = p.matcher(msg);
+      if (matcher.lookingAt()) {
+        return true;
+      } 
     }
     return false;
   }

Modified: logback/trunk/logback-site/src/site/pages/codes.html
==============================================================================
--- logback/trunk/logback-site/src/site/pages/codes.html	(original)
+++ logback/trunk/logback-site/src/site/pages/codes.html	Wed Jul  8 10:24:27 2009
@@ -40,62 +40,6 @@
   </p>
   <hr/>
 
-
-  <p>
-    <a name="tbr_fnp_not_set" href="#tbr_fnp_not_set">
-      The <b>FileNamePattern</b> option must be set before using
-      <code>TimeBasedRollingPolicy</code> or
-      <code>FixedWindowRollingPolicy</code>
-    </a>
-  </p>
-
-  
-  <p>The <b>FileNamePattern</b> option for both
-  <code>TimeBasedRollingPolicy</code> and
-  <code>FixedWindowRollingPolicy</code> is mandatory.
-  </p>
-  
-  <p>See the <a
-  href="http://logback.qos.ch/apidocs/ch/qos/logback/core/rolling/FixedWindowRollingPolicy.html">
-  FixedWindowRollingPolicy javadoc </a> for more information.
-  </p>
-					
-  <hr />
-
-  <p>
-    <a name="fwrp_parentFileName_not_set" href="#fwrp_parentFileName_not_set">
-      The <span class="option">File</span> option must be set before
-      <code>FixedWindowRollingPolicy</code>
-    </a>
-  </p>
-
-  <p>The <span class="option">File</span> option is mandatory with
-  <code>FixedWindowRollingPolicy</code>. Moreover, the <span
-  class="option">File</span> option must be set before the
-  <code>FixedWindowRollingPolicy</code> element.
-  </p>
-      
-  <p>Refer to the logback manual on
-  <a href="manual/appenders.html#FixedWindowRollingPolicy">
-    FixedWindowRollingPolicy </a> for more information.
-  </p>
-			
-  <hr />
-  <p>
-    <a name="tbr_fnp_prudent_unsupported"
-    href="#tbr_fnp_prudent_unsupported">Prudent mode is not supported
-    with <code>FixedWindowRollingPolicy</code>.</a>
-  </p>
-
-  <p>Given that <code>FixedWindowRollingPolicy</code> performs
-  multiple file rename operations suring roll over, and that these
-  operations cannot be guaranteed to be safe in a multi-JVM context,
-  prudent mode is not allowed in conjuction with a
-  <code>FixedWindowRollingPolicy</code>.
-  </p>
-
-  <hr/>
-
   <p>
     <a name="socket_no_host" href="#socket_no_host">No remote host or
     address is set for <code>SocketAppender</code>
@@ -191,9 +135,11 @@
   
   <hr />
       
-  <p><a name="rfa_no_tp" href="#rfa_no_tp">No <code>TriggeringPolicy</code> was set for
-  the <code>RollingFileAppender</code>.
-  </a>
+  <p>
+    <a name="rfa_no_tp" href="#rfa_no_tp">No
+    <code>TriggeringPolicy</code> was set for the
+    <code>RollingFileAppender</code>.
+    </a>
   </p>
     
   <p>The <code>RollingFileAppender</code> must be set up with a
@@ -208,12 +154,12 @@
   <ul>
     <li>
       <a
-       href="http://logback.qos.ch/apidocs/ch/qos/logback/core/rolling/SizeBasedTriggeringPolicy.html"><code>SizeBasedTriggeringPolicy</code>
+       href="manual/appenders.html#SizeBasedTriggeringPolicy"><code>SizeBasedTriggeringPolicy</code>
       </a>
     </li>
     <li>
       <a
-       href="http://logback.qos.ch/apidocs/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.html"><code>TimeBasedRollingPolicy</code>
+      href="manual/appenders.html#TimeBasedRollingPolicy"><code>TimeBasedRollingPolicy</code>
       </a>
     </li>
   </ul>
@@ -243,12 +189,12 @@
     
   <ul>
     <li>
-      <a href="http://logback.qos.ch/apidocs/ch/qos/logback/core/rolling/FixedWindowRollingPolicy.html">
+      <a href="apidocs/ch/qos/logback/core/rolling/FixedWindowRollingPolicy.html">
         <code>FixedWindowRollingPolicy</code>
       </a>
     </li>
     <li>
-      <a href="http://logback.qos.ch/apidocs/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.html">
+      <a href="apidocs/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.html">
         <code>TimeBasedRollingPolicy</code>
       </a>
     </li>
@@ -262,9 +208,82 @@
   
   <hr/>
   
+  <p>
+    <a name="tbr_fnp_not_set" href="#tbr_fnp_not_set">
+      The <span class="option">FileNamePattern</span> property is mandatory for both
+      <code>TimeBasedRollingPolicy</code> and
+      <code>FixedWindowRollingPolicy</code>.
+    </a>
+  </p>
+
+  
+  <p>The <span class="option">FileNamePattern</span> property for both
+  <code>TimeBasedRollingPolicy</code> and
+  <code>FixedWindowRollingPolicy</code> is mandatory.
+  </p>
+  
+  <p>Please refer to the documentation of <a
+  href="manual/appenders.html#TimeBasedRollingPolicy">TimeBasedRollingPolicy</a>
+  and <a
+  href="manual/appenders.html#FixedWindowRollingPolicy">FixedWindowRollingPolicy</a> for
+  examples.
+  </p>
+
+
+  <hr/>
+
+  <p>
+    <a name="rfa_file_after" href="#rfa_file_after">
+      The <span class="option">File</span> property must be set before
+      any rolling policy or triggering policy.
+    </a>
+  </p>
+
+  <p>The <span class="option">File</span> property, if present, must
+  be placed before any rolling policy or triggering policy. Thus, in a
+  configuration file, the <span class="option">File</span> property,
+  if present, must be declared declared before any rolling policy or
+  triggering policy declarations.
+  </p>
+
+  <hr/>
+
+  <p>
+    <a name="fwrp_parentFileName_not_set" href="#fwrp_parentFileName_not_set">
+      The <span class="option">File</span> property must be set before
+      <code>FixedWindowRollingPolicy</code>
+    </a>
+  </p>
+
+  <p>The <span class="option">File</span> property is mandatory with
+  <code>FixedWindowRollingPolicy</code>. Moreover, the <span
+  class="option">File</span> option must be set before the
+  <code>FixedWindowRollingPolicy</code> element.
+  </p>
+      
+  <p>Refer to the logback manual on
+  <a href="manual/appenders.html#FixedWindowRollingPolicy">
+    FixedWindowRollingPolicy </a> for more information.
+  </p>
+			
+  <hr />
+  <p>
+    <a name="tbr_fnp_prudent_unsupported"
+    href="#tbr_fnp_prudent_unsupported">Prudent mode is not supported
+    with <code>FixedWindowRollingPolicy</code>.</a>
+  </p>
+
+  <p>Given that <code>FixedWindowRollingPolicy</code> performs
+  multiple file rename operations suring roll over, and that these
+  operations cannot be guaranteed to be safe in a multi-JVM context,
+  prudent mode is not allowed in conjuction with a
+  <code>FixedWindowRollingPolicy</code>.
+  </p>
+
+  <hr />
   
   <p><a name="syslog_layout" href="#syslog_layout"> SyslogAppender
-  does not admit a layout parameter.</a>
+  does not admit a layout.</a>
   </p>
 
 

Modified: logback/trunk/logback-site/src/site/pages/manual/appenders.html
==============================================================================
--- logback/trunk/logback-site/src/site/pages/manual/appenders.html	(original)
+++ logback/trunk/logback-site/src/site/pages/manual/appenders.html	Wed Jul  8 10:24:27 2009
@@ -1166,8 +1166,10 @@
 
     
 
-		<a name="TriggeringPolicy"></a>
-		<h3>Triggering policies</h3>
+
+		<h3>
+      <a name="TriggeringPolicy" href="#TriggeringPolicy">Triggering policies</a>
+    </h3>
 		
 		<p><a
 		href="../xref/ch/qos/logback/core/rolling/TriggeringPolicy.html"><code>TriggeringPolicy</code></a>
@@ -1193,8 +1195,9 @@
 		rollover should occur or not, based on the said parameters.
 		</p>
 
-		<a name="SizeBasedTriggeringPolicy"></a>
-		<h4>SizeBasedTriggeringPolicy</h4>
+		
+		<h4><a name="SizeBasedTriggeringPolicy"
+		href="#SizeBasedTriggeringPolicy">SizeBasedTriggeringPolicy</a></h4>
 
 		<p><a
 		href="../xref/ch/qos/logback/core/rolling/SizeBasedTriggeringPolicy.html">

Modified: logback/trunk/logback-site/src/site/pages/news.html
==============================================================================
--- logback/trunk/logback-site/src/site/pages/news.html	(original)
+++ logback/trunk/logback-site/src/site/pages/news.html	Wed Jul  8 10:24:27 2009
@@ -79,6 +79,13 @@
     47465</a> Mark Thomas.
     </p>
 
+    <p>Fixed <a
+    href="http://jira.qos.ch/browse/LBCORE-94">LBCORE-94</a>. While
+    configuring a RollingFileAppender, if the File property is
+    declared after any rolling triggering policies, then logback will
+    now generate an error status message.
+    </p>
+
     <hr width="80%" align="center" />
 
     <h3>12th of February 2009 - Release of version 0.9.15</h3>


More information about the logback-dev mailing list