From 754ac2c5ac4a471569824501090da58bfde3e5bf Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Tue, 29 May 2012 15:50:35 +1000 Subject: [PATCH] changed config loading to be more predictable --- lib/log4js.js | 41 +++++++++------------ test/logging.js | 94 ++++++++++++++++++------------------------------- 2 files changed, 50 insertions(+), 85 deletions(-) diff --git a/lib/log4js.js b/lib/log4js.js index 6c42627..771c571 100644 --- a/lib/log4js.js +++ b/lib/log4js.js @@ -55,7 +55,13 @@ var events = require('events') , ALL_CATEGORIES = '[all]' , appenders = {} , loggers = {} -, appenderMakers = {}; +, appenderMakers = {} +, defaultConfig = { + appenders: [ + { type: "console" } + ], + replaceConsole: false +}; /** * Get a logger instance. Instance is cached on categoryName level. @@ -146,8 +152,6 @@ function configureAppenders(appenderList) { throw new Error("log4js configuration problem for "+util.inspect(appenderConfig)); } }); - } else { - addAppender(consoleAppender()); } } @@ -243,22 +247,9 @@ function getDefaultLogger () { return getLogger(DEFAULT_CATEGORY); } -function findConfiguration(filename) { - var path; - try { - path = require.resolve(filename || 'log4js.json'); - } catch (e) { - //file not found. default to the one in the log4js module. - path = filename || __dirname + '/log4js.json'; - } - - return path; -} - var configState = {}; function loadConfigurationFile(filename) { - filename = findConfiguration(filename); if (filename && (!configState.lastFilename || filename !== configState.lastFilename || !configState.lastMTime || fs.statSync(filename).mtime !== configState.lastMTime)) { configState.lastFilename = filename; @@ -274,10 +265,10 @@ function configureOnceOff(config) { configureAppenders(config.appenders); configureLevels(config.levels); - if (config.doNotReplaceConsole) { - restoreConsole(); - } else { + if (config.replaceConsole) { replaceConsole(); + } else { + restoreConsole(); } } catch (e) { throw new Error("Problem reading log4js config " + util.inspect(config) + ". Error was \"" + e.message + "\" ("+e.stack+")"); @@ -286,7 +277,7 @@ function configureOnceOff(config) { } function reloadConfiguration() { - var filename = findConfiguration(configState.filename), + var filename = configState.filename, mtime; if (!filename) { // can't find anything to reload @@ -316,7 +307,7 @@ function initReloadConfiguration(filename, options) { configState.timerId = setInterval(reloadConfiguration, options.reloadSecs*1000); } -function configure (configurationFileOrObject, options) { +function configure(configurationFileOrObject, options) { var config = configurationFileOrObject; options = options || {}; @@ -324,7 +315,7 @@ function configure (configurationFileOrObject, options) { if (options.reloadSecs) { initReloadConfiguration(config, options); } - config = loadConfigurationFile(config); + config = loadConfigurationFile(config) || defaultConfig; } else { if (options.reloadSecs) { getLogger('log4js').warn('Ignoring configuration reload parameter for "object" configuration.'); @@ -332,7 +323,7 @@ function configure (configurationFileOrObject, options) { } if (options.hasOwnProperty('cwd')) { - if(config.hasOwnProperty('appenders')) { + if (config.hasOwnProperty('appenders')) { config.appenders.forEach(function(appender) { if (!appender.hasOwnProperty('type')) return; @@ -409,8 +400,8 @@ module.exports = { loadAppender(appender); }); -//set ourselves up if we can find a default log4js.json -configure(findConfiguration()); +//set ourselves up +configure(); //keep the old-style layouts ['basicLayout','messagePassThroughLayout','colouredLayout','coloredLayout'].forEach(function(item) { diff --git a/test/logging.js b/test/logging.js index 71df36a..a24c437 100644 --- a/test/logging.js +++ b/test/logging.js @@ -295,72 +295,46 @@ vows.describe('log4js').addBatch({ 'default setup': { topic: function() { - var pathLoaded, - logger, - modulePath = require('path').normalize(__dirname + '/../lib/log4js.json'), - mtime = Date.now(), - fakeFS = { - readdirSync: function(dir) { - return require('fs').readdirSync(dir); - }, - readFileSync: function (file, encoding) { - pathLoaded = file; - assert.equal(encoding, 'utf8'); - return '{ "appenders" : [ { "type": "console", "layout": { "type": "messagePassThrough" }} ] }'; - }, - statSync: function (path) { - if (path === modulePath) { - return { mtime: mtime }; - } else { - throw new Error("no such file"); + var appenderEvents = [], + fakeConsole = { + 'name': 'console' + , 'appender': function () { + return function(evt) { + appenderEvents.push(evt); + } } - } - }, - appenderEvents = [], - fakeConsole = { - 'name': 'console' - , 'appender': function () { - return function(evt) { appenderEvents.push(evt); } - } - , 'configure': function (config) { - return fakeConsole.appender(); - } - }, - globalConsole = { - log: function() { throw new Error("I should not be called."); } - }, - log4js = sandbox.require( - '../lib/log4js', - { - requires: { - 'fs': fakeFS - , './appenders/console': fakeConsole - }, - globals: { - console: globalConsole + , 'configure': function (config) { + return fakeConsole.appender(); } - } - ); + }, + globalConsole = { + log: function() { } + }, + log4js = sandbox.require( + '../lib/log4js', + { + requires: { + './appenders/console': fakeConsole + }, + globals: { + console: globalConsole + } + } + ), + logger = log4js.getLogger('a-test'); - logger = log4js.getLogger('a-test'); logger.debug("this is a test"); - globalConsole.log("this should be logged"); - return [ pathLoaded, appenderEvents, modulePath ]; + globalConsole.log("this should not be logged"); + + return appenderEvents; }, - 'should use require.resolve to find log4js.json': function(args) { - var pathLoaded = args[0], modulePath = args[2]; - assert.equal(pathLoaded, modulePath); - }, - - 'should configure log4js from first log4js.json found': function(args) { - var appenderEvents = args[1]; + 'should configure a console appender': function(appenderEvents) { assert.equal(appenderEvents[0].data[0], 'this is a test'); }, - 'should replace console.log with log4js version': function(args) { - var appenderEvents = args[1]; - assert.equal(appenderEvents[1].data[0], 'this should be logged'); + 'should not replace console.log with log4js version': function(appenderEvents) { + assert.equal(appenderEvents.length, 1); } }, @@ -449,7 +423,7 @@ vows.describe('log4js').addBatch({ var pathsChecked = [], logEvents = [], logger, - modulePath = require('path').normalize(__dirname + '/../lib/log4js.json'), + modulePath = 'path/to/log4js.json', fakeFS = { lastMtime: Date.now(), config: { appenders: [ { type: 'console', layout: { type: 'messagePassThrough' } } ], @@ -499,7 +473,7 @@ vows.describe('log4js').addBatch({ } ); - log4js.configure(undefined, { reloadSecs: 30 }); + log4js.configure('path/to/log4js.json', { reloadSecs: 30 }); logger = log4js.getLogger('a-test'); logger.info("info1"); logger.debug("debug2 - should be ignored"); @@ -511,10 +485,10 @@ vows.describe('log4js').addBatch({ return logEvents; }, 'should configure log4js from first log4js.json found': function(logEvents) { - assert.equal(logEvents.length, 3); assert.equal(logEvents[0].data[0], 'info1'); assert.equal(logEvents[1].data[0], 'info3'); assert.equal(logEvents[2].data[0], 'debug4'); + assert.equal(logEvents.length, 3); } },