Author: seb
Date: Wed Nov 1 11:01:47 2006
New Revision: 843
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/RollingPolicyBase.java
logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.java
Log:
This version of these classes pass all the tests.
Still wondering if something better can be done, especially about the getActiveFileName method of RollingPolicy, that serves more
the purpose of a "makeInitialFileName" than just a getter to a value.
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 Nov 1 11:01:47 2006
@@ -78,24 +78,16 @@
return;
}
- if (rollingPolicy != null) {
- //rollingPolicy.setParent(this);
- //String afn = rollingPolicy.getActiveFileName();
- // the activeFile variable is used by the triggeringPolicy.isTriggeringEvent method
-
-// if (getFile() == null) {
-// setFile(rollingPolicy.getActiveFileName());
-// }
+ if (rollingPolicy != null) {
+ //if no active file name was set, then it's the responsability of the
+ //rollingPolicy to create one.
+ if (getFile() == null) {
+ setFile(rollingPolicy.getActiveFileName());
+ }
activeFileCache = new File(getFile());
addInfo("Active log file name: "+ getFile());
-
- // The local setFile throws an exception, so we use the parent's version.
- // This is to prevent the user from configuring both the RollingFileAppender with
- // an activeFileName _and_ the FileAppender's file attribute, causing confusion
- // on the attributes' uses.
- //super.setFile(rollingPolicy.getActiveFileName());
super.start();
} else {
addWarn("No RollingPolicy was set for the RollingFileAppender named "+ getName());
@@ -136,14 +128,6 @@
// we failed to roll-over, let us not truncate and risk data loss
this.append = true;
}
-
- // Although not certain, the active file name may change after roll over.
- //fileName = rollingPolicy.getActiveFileName();
- fileName = getFile();
- addInfo("Active file name is now ["+ fileName+"].");
-
- // the activeFile variable is used by the triggeringPolicy.isTriggeringEvent method
- ////activeFile = new File(fileName);
try {
// This will also close the file. This is OK since multiple
@@ -197,10 +181,4 @@
rollingPolicy = (RollingPolicy) policy;
}
}
-
-// @Override
-// public void setFile(String filename) {
-// throw new UnsupportedOperationException("With RollingFileAppender please use activeFileName " +
-// "option instead of File");
-// }
}
Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingPolicyBase.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingPolicyBase.java (original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingPolicyBase.java Wed Nov 1 11:01:47 2006
@@ -27,12 +27,7 @@
protected int compressionMode = Compress.NONE;
protected FileNamePattern fileNamePattern;
protected String fileNamePatternStr;
- /*
- * It would have been nice to merge 'activeFileName' into filename of
- * FileAppender. Unfortunately, a child component must be self contained
- * as it is started before its parent.
- */
- //protected String activeFileName = null;
+
private FileAppender parent;
private boolean started;
@@ -86,4 +81,7 @@
return parent.getFile();
}
+ protected void setParentFileName(String newFileName) {
+ parent.setFile(newFileName);
+ }
}
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 Nov 1 11:01:47 2006
@@ -157,6 +157,7 @@
FileNamePattern activeFileNamePattern;
RenameUtil util = new RenameUtil();
Compress compress = new Compress();
+ String lastGeneratedFileName;
public void start() {
// set the LR for our utility object
@@ -209,7 +210,6 @@
//Date nc = new Date();
//nc.setTime(nextCheck);
- //getLogger().debug("Next check set to: " + nc);
}
public void rollover() throws RolloverFailure {
@@ -245,31 +245,53 @@
break;
}
}
+
+ //if we already generated a name, then we have to update
+ //the fileAppender with a new active file name.
+ if (getParentFileName() == lastGeneratedFileName) {
+ String newFile = activeFileNamePattern.convertDate(lastCheck);
+ setParentFileName(newFile);
+ lastGeneratedFileName = newFile;
+ }
}
/**
- *
- * The active log file is determined by the value of the activeFileName
- * option if it is set. However, in case the activeFileName is left blank,
- * then, the active log file equals the file name for the current period
- * as computed by the <b>FileNamePattern</b> option.
- *
- */
+ *
+ * The active log file is determined by the value of the parent's filename
+ * option. However, in case the file name is left blank,
+ * then, the active log file equals the file name for the current period
+ * as computed by the <b>FileNamePattern</b> option.
+ *
+ * The RollingPolicy must know wether it is responsible for
+ * changing the name of the active file or not. If the active file
+ * name is set by the user via the configuration file, then the
+ * RollingPolicy must let it like it is. If the user does not
+ * specify an active file name, then the RollingPolicy generates one.
+ *
+ * To be sure that the file name used by the parent class has been
+ * generated by the RollingPolicy and not specified by the user, we
+ * keep track of the last generated name object and compare its
+ * reference to the parent file name. If they match, then
+ * the RollingPolicy knows it's responsible for the change of the
+ * file name.
+ *
+ *
+ * tmp note: !!!!! This is called only once at the start of the appender,
+ * and only if the parent file name is null, so no test is required.
+ *
+ */
public String getActiveFileName() {
- if (getParentFileName() == null) {
- return activeFileNamePattern.convertDate(lastCheck);
- } else {
- return getParentFileName();
- }
+ String newName = activeFileNamePattern.convertDate(lastCheck);
+ addInfo("Generated a new name for RollingFileAppender: " + newName);
+ lastGeneratedFileName = newName;
+ return newName;
}
public boolean isTriggeringEvent(File file, final Object event) {
- //getLogger().debug("Is triggering event called");
long n = System.currentTimeMillis();
if (n >= nextCheck) {
addInfo("Time to trigger roll-over");
-
// We set the elapsedPeriodsFileName before we set the 'lastCheck' variable
// The elapsedPeriodsFileName corresponds to the file name of the period
// that just elapsed.
@@ -277,7 +299,6 @@
addInfo("elapsedPeriodsFileName set to "+elapsedPeriodsFileName);
lastCheck.setTime(n);
- //getLogger().debug("ActiveLogFileName will return " + getActiveLogFileName());
nextCheck = rc.getNextCheckMillis(lastCheck);
Date x = new Date();
