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>