Issue #21: fixed reloading of config when config has not changed.

lib/log4js.js

@@ -265,9 +265,14 @@ function findConfiguration() {
     return undefined;
 }
 
+var configState = {};
+
 function loadConfigurationFile(filename) {
     filename = filename || findConfiguration();
-    if (filename) {
+    if (filename && (!configState.lastFilename || filename !== configState.lastFilename || 
+                     !configState.lastMTime || fs.statSync(filename).mtime !== configState.lastMTime)) {
+        configState.lastFilename = filename;
+        configState.lastMTime = fs.statSync(filename).mtime;
         return JSON.parse(fs.readFileSync(filename, "utf8"));
     }
     return undefined;
@@ -284,8 +289,6 @@ function configureOnceOff(config) {
     }
 }
 
-var configState = {};
-
 function reloadConfiguration() {
     var filename = configState.filename || findConfiguration(),
     mtime;
@@ -301,15 +304,11 @@ function reloadConfiguration() {
     }
     if (configState.lastFilename && configState.lastFilename === filename) {
         if (mtime.getTime() > configState.lastMTime.getTime()) {
-            configState.lastMTime = mtime;
+            configureOnceOff(loadConfigurationFile(filename));
-        } else {
-            filename = null;
         }
     } else {
-        configState.lastFilename = filename;
+        configureOnceOff(loadConfigurationFile(filename));
-        configState.lastMTime = mtime;
     }
-    configureOnceOff(loadConfigurationFile(filename));
 }
 
 function initReloadConfiguration(filename, options) {

test/logging.js

@@ -109,6 +109,9 @@ vows.describe('log4js').addBatch({
               , { requires:
                   { 'fs':
                     {
+                        statSync: function() {
+                            return { mtime: Date.now() };
+                        },
                         readFileSync: function(filename) {
                             configFilename = filename;
                             return JSON.stringify({
@@ -268,6 +271,7 @@ vows.describe('log4js').addBatch({
             appenderEvent,
             logger,
             modulePath = require('path').normalize(__dirname + '/../lib/log4js.json'),
+            mtime = Date.now(),
             fakeFS = {
                 readdirSync: function(dir) {
                     return require('fs').readdirSync(dir);
@@ -280,7 +284,7 @@ vows.describe('log4js').addBatch({
                 statSync: function (path) {
                     pathsChecked.push(path);
                     if (path === modulePath) {
-                        return true;
+                        return { mtime: mtime };
                     } else {
                         throw new Error("no such file");
                     }
@@ -316,7 +320,8 @@ vows.describe('log4js').addBatch({
                   require.paths.map(function(item) {
                       return item + '/log4js.json';
                   }),
-                  args[2]
+                  args[2],
+                  args[2] // stat will be called twice on the selected file
               );
             assert.deepEqual(pathsChecked, expectedPaths);
         },
@@ -391,13 +396,14 @@ vows.describe('log4js').addBatch({
             assert.equal(logEvent.data[0], "This should go to the appender defined in firstLog4js");
         }
     },
-    'configuration reload' : {
+    'configuration reload with configuration changing' : {
         topic: function() {
             var pathsChecked = [],
             logEvents = [],
             logger,
             modulePath = require('path').normalize(__dirname + '/../lib/log4js.json'),
             fakeFS = {
+                lastMtime: Date.now(),
                 config: { appenders: [ { type: 'console', layout: { type: 'messagePassThrough' } } ],
                           levels: { 'a-test' : 'INFO' } },
                 readdirSync: function(dir) {
@@ -411,7 +417,8 @@ vows.describe('log4js').addBatch({
                 statSync: function (path) {
                     pathsChecked.push(path);
                     if (path === modulePath) {
-                        return { mtime: new Date() };
+                        fakeFS.lastMtime += 1;
+                        return { mtime: new Date(fakeFS.lastMtime) };
                     } else {
                         throw new Error("no such file");
                     }
@@ -444,6 +451,7 @@ vows.describe('log4js').addBatch({
                 }
             );
 
+            log4js.configure();
             logger = log4js.getLogger('a-test');
             logger.info("info1");
             logger.debug("debug2 - should be ignored");
@@ -461,6 +469,85 @@ vows.describe('log4js').addBatch({
             assert.equal(logEvents[1].data[0], 'info3'); 
             assert.equal(logEvents[2].data[0], 'debug4');
         }
+    },
+    
+    'configuration reload with configuration staying the same' : {
+        topic: function() {
+            var pathsChecked = [],
+            fileRead = 0,
+            logEvents = [],
+            logger,
+            modulePath = require('path').normalize(__dirname + '/../lib/log4js.json'),
+            mtime = new Date(),
+            fakeFS = {
+                config: { appenders: [ { type: 'console', layout: { type: 'messagePassThrough' } } ],
+                          levels: { 'a-test' : 'INFO' } },
+                readdirSync: function(dir) {
+                    return require('fs').readdirSync(dir);
+                },
+                readFileSync: function (file, encoding) {
+                    fileRead += 1;
+                    assert.isString(file);
+                    assert.equal(file, modulePath);
+                    assert.equal(encoding, 'utf8');
+                    return JSON.stringify(fakeFS.config);
+                },
+                statSync: function (path) {
+                    pathsChecked.push(path);
+                    if (path === modulePath) {
+                        return { mtime: mtime };
+                    } else {
+                        throw new Error("no such file");
+                    }
+                }
+            },
+            fakeConsole = {
+                'name': 'console', 
+                'appender': function () {
+                    return function(evt) { logEvents.push(evt); };
+                }, 
+                'configure': function (config) {
+                    return fakeConsole.appender();
+                }
+            },
+            setIntervalCallback,
+            fakeSetInterval = function(cb, timeout) {
+                setIntervalCallback = cb;
+            },
+            log4js = sandbox.require(
+                '../lib/log4js',
+                {
+                    requires: {
+                        'fs': fakeFS, 
+                        './appenders/console.js': fakeConsole
+                    },
+                    globals: {
+                        'console': fakeConsole,
+                        'setInterval' : fakeSetInterval,
+                    }
+                }
+            );
+
+            log4js.configure(modulePath, { reloadSecs: 3 });
+            logger = log4js.getLogger('a-test');
+            logger.info("info1");
+            logger.debug("debug2 - should be ignored");
+            setIntervalCallback();
+            logger.info("info3");
+            logger.debug("debug4");
+
+            return [ pathsChecked, logEvents, modulePath, fileRead ];
+        },
+        'should only read the configuration file once': function(args) {
+            var fileRead = args[3];
+            assert.equal(fileRead, 1);
+        },
+        'should configure log4js from first log4js.json found': function(args) {
+            var logEvents = args[1];
+            assert.length(logEvents, 2);
+            assert.equal(logEvents[0].data[0], 'info1');
+            assert.equal(logEvents[1].data[0], 'info3'); 
+        }
     }
 
 }).export(module);