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();