From f9768eb56e68da79c9598174e4a5bce7b0540e61 Mon Sep 17 00:00:00 2001 From: Daniel Bell Date: Wed, 5 Oct 2011 12:22:31 +1100 Subject: [PATCH] Issue #21: fixed reloading of config when config has not changed. --- lib/log4js.js | 17 +++++---- test/logging.js | 95 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 99 insertions(+), 13 deletions(-) diff --git a/lib/log4js.js b/lib/log4js.js index 2623639..1f1fc83 100644 --- a/lib/log4js.js +++ b/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; - } else { - filename = null; + configureOnceOff(loadConfigurationFile(filename)); } } else { - configState.lastFilename = filename; - configState.lastMTime = mtime; + configureOnceOff(loadConfigurationFile(filename)); } - configureOnceOff(loadConfigurationFile(filename)); } function initReloadConfiguration(filename, options) { diff --git a/test/logging.js b/test/logging.js index b9821b3..f02c556 100644 --- a/test/logging.js +++ b/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);