From dc632f47059988782a65358b4b738f485f8553bc Mon Sep 17 00:00:00 2001
From: Issac Goldstand <margol@beamartyr.net>
Date: Sat, 11 May 2013 23:01:28 +0300
Subject: [PATCH] Fixes bug introduced in github issue #132 where file rolling
 needs to be handled differently for alwaysIncludePattern streams

---
 lib/streams/DateRollingFileStream.js       | 39 ++++++++-------
 test/streams/DateRollingFileStream-test.js | 57 ++++++++++++++++++++++
 2 files changed, 78 insertions(+), 18 deletions(-)

diff --git a/lib/streams/DateRollingFileStream.js b/lib/streams/DateRollingFileStream.js
index 8632f66..69d6668 100644
--- a/lib/streams/DateRollingFileStream.js
+++ b/lib/streams/DateRollingFileStream.js
@@ -23,13 +23,13 @@ function DateRollingFileStream(filename, pattern, options, now) {
   this.pattern = pattern || '.yyyy-MM-dd';
   this.now = now || Date.now;
   this.lastTimeWeWroteSomething = format.asString(this.pattern, new Date(this.now()));
-  this.alwaysIncludePattern = false;
   this.baseFilename = filename;
+  this.alwaysIncludePattern = false;
     
   if (options) {
     if (options.alwaysIncludePattern) {
       this.alwaysIncludePattern = true;
-      filename = filename + this.lastTimeWeWroteSomething;
+      filename = this.baseFilename + this.lastTimeWeWroteSomething;
     }
     delete options.alwaysIncludePattern;
     if (Object.keys(options).length === 0) { 
@@ -55,18 +55,26 @@ DateRollingFileStream.prototype.shouldRoll = function() {
 };
 
 DateRollingFileStream.prototype.roll = function(filename, callback) {
-    var that = this,
-        newFilename = this.baseFilename + this.previousTime;
+    var that = this;
 
     debug("Starting roll");
 
-    async.series([
-      this.closeTheStream.bind(this),
-      deleteAnyExistingFile,
-      renameTheCurrentFile,
-      this.openTheStream.bind(this)
-    ], callback);
-
+    if (this.alwaysIncludePattern) {
+        this.filename = this.baseFilename + this.lastTimeWeWroteSomething;
+        async.series([
+          this.closeTheStream.bind(this),
+          this.openTheStream.bind(this)
+        ], callback);
+    } else {
+        var newFilename = this.baseFilename + this.previousTime;
+        async.series([
+          this.closeTheStream.bind(this),
+          deleteAnyExistingFile,
+          renameTheCurrentFile,
+          this.openTheStream.bind(this)
+        ], callback);
+    }
+    
     function deleteAnyExistingFile(cb) {
         //on windows, you can get a EEXIST error if you rename a file to an existing file
         //so, we'll try to delete the file we're renaming to first
@@ -77,13 +85,8 @@ DateRollingFileStream.prototype.roll = function(filename, callback) {
     }
 
     function renameTheCurrentFile(cb) {
-        if (this.alwaysIncludePattern) {
-            // no-op
-            cb();
-        } else {
-            debug("Renaming the " + filename + " -> " + newFilename);
-            fs.rename(filename, newFilename, cb);
-        }
+        debug("Renaming the " + filename + " -> " + newFilename);
+        fs.rename(filename, newFilename, cb);
     }
 
 };
diff --git a/test/streams/DateRollingFileStream-test.js b/test/streams/DateRollingFileStream-test.js
index 69e9ba8..a314170 100644
--- a/test/streams/DateRollingFileStream-test.js
+++ b/test/streams/DateRollingFileStream-test.js
@@ -125,6 +125,63 @@ vows.describe('DateRollingFileStream').addBatch({
                 }
             }
         }
+    },
+
+    'with alwaysIncludePattern': {
+        topic: function() {
+          var that = this,
+	      testTime = new Date(2012, 8, 12, 0, 10, 12),
+	      stream = new DateRollingFileStream(__dirname + '/test-date-rolling-file-stream-pattern', '.yyyy-MM-dd', {alwaysIncludePattern: true}, now);
+	      stream.write("First message\n", 'utf8', function() {
+	        that.callback(null, stream);
+	      });
+        },
+        teardown: cleanUp(__dirname + '/test-date-rolling-file-stream-pattern.2012-09-12'),
+
+        'should create a file with the pattern set': {
+            topic: function(stream) {
+                fs.readFile(__dirname + '/test-date-rolling-file-stream-pattern.2012-09-12', this.callback);
+            },
+            'file should contain first message': function(result) {
+                assert.equal(result.toString(), "First message\n");
+            }
+        },
+
+        'when the day changes': {
+            topic: function(stream) {
+              testTime = new Date(2012, 8, 13, 0, 10, 12);
+              stream.write("Second message\n", 'utf8', this.callback);
+            },
+            teardown: cleanUp(__dirname + '/test-date-rolling-file-stream-pattern.2012-09-13'),
+
+
+            'the number of files': {
+                topic: function() {
+                    fs.readdir(__dirname, this.callback);
+                },
+                'should be two': function(files) {
+                    assert.equal(files.filter(function(file) { return file.indexOf('test-date-rolling-file-stream-pattern') > -1; }).length, 2);
+                }
+            },
+
+            'the file with the later date': {
+                topic: function() {
+                    fs.readFile(__dirname + '/test-date-rolling-file-stream-pattern.2012-09-13', this.callback);
+                },
+                'should contain the second message': function(contents) {
+                    assert.equal(contents.toString(), "Second message\n");
+                }
+            },
+
+            'the file with the date': {
+                topic: function() {
+                    fs.readFile(__dirname + '/test-date-rolling-file-stream-pattern.2012-09-12', this.callback);
+                },
+                'should contain the first message': function(contents) {
+                    assert.equal(contents.toString(), "First message\n");
+                }
+            }
+        }
     }
 
 }).exportTo(module);