From 0901794b3532987af967413eb607cb0429c2badd Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 31 May 2012 07:50:01 +1000 Subject: [PATCH] Moved abspath option checking into file appender, log4js options now passed to appenders --- lib/appenders/file.js | 7 ++- lib/log4js.js | 28 ++-------- test/logging.js | 14 +++-- test/test-log-abspath.js | 117 +++++++++++++++------------------------ 4 files changed, 66 insertions(+), 100 deletions(-) diff --git a/lib/appenders/file.js b/lib/appenders/file.js index fdc5c18..0a0267f 100644 --- a/lib/appenders/file.js +++ b/lib/appenders/file.js @@ -50,11 +50,16 @@ function fileAppender (file, layout, logSize, numBackups) { }; } -function configure(config) { +function configure(config, options) { var layout; if (config.layout) { layout = layouts.layout(config.layout.type, config.layout); } + + if (options && options.cwd && !config.absolute) { + config.filename = path.join(options.cwd, config.filename); + } + return fileAppender(config.filename, layout, config.maxLogSize, config.backups); } diff --git a/lib/log4js.js b/lib/log4js.js index 771c571..e7f20fd 100644 --- a/lib/log4js.js +++ b/lib/log4js.js @@ -138,14 +138,14 @@ function clearAppenders () { } } -function configureAppenders(appenderList) { +function configureAppenders(appenderList, options) { clearAppenders(); if (appenderList) { appenderList.forEach(function(appenderConfig) { loadAppender(appenderConfig.type); var appender; appenderConfig.makers = appenderMakers; - appender = appenderMakers[appenderConfig.type](appenderConfig); + appender = appenderMakers[appenderConfig.type](appenderConfig, options); if (appender) { addAppender(appender, appenderConfig.category); } else { @@ -259,10 +259,10 @@ function loadConfigurationFile(filename) { return undefined; } -function configureOnceOff(config) { +function configureOnceOff(config, options) { if (config) { try { - configureAppenders(config.appenders); + configureAppenders(config.appenders, options); configureLevels(config.levels); if (config.replaceConsole) { @@ -321,25 +321,7 @@ function configure(configurationFileOrObject, options) { getLogger('log4js').warn('Ignoring configuration reload parameter for "object" configuration.'); } } - - if (options.hasOwnProperty('cwd')) { - if (config.hasOwnProperty('appenders')) { - config.appenders.forEach(function(appender) { - if (!appender.hasOwnProperty('type')) return; - - if (appender.type === 'file') { - if(!appender.hasOwnProperty('filename')) return; - if(appender.hasOwnProperty('absolute')) { - if(appender.absolute) return; - } - - appender.filename = path.join(options.cwd, appender.filename); - } - }); - } - } - - configureOnceOff(config); + configureOnceOff(config, options); } var originalConsoleFunctions = { diff --git a/test/logging.js b/test/logging.js index a24c437..990f630 100644 --- a/test/logging.js +++ b/test/logging.js @@ -86,7 +86,7 @@ vows.describe('log4js').addBatch({ 'invalid configuration': { 'should throw an exception': function() { assert.throws(function() { - log4js.configure({ "type": "invalid" }); + require('log4js').configure({ "type": "invalid" }); }); } }, @@ -393,7 +393,7 @@ vows.describe('log4js').addBatch({ 'configuration': { topic: function(test) { test.log4js.replaceConsole(); - test.log4js.configure({ doNotReplaceConsole: true }); + test.log4js.configure({ replaceConsole: false }); try { test.fakeConsole.log("This should cause the error described in the setup"); } catch (e) { @@ -407,14 +407,20 @@ vows.describe('log4js').addBatch({ } }, 'configuration persistence' : { - 'should maintain appenders between requires': function () { - var logEvent, firstLog4js = require('../lib/log4js'), secondLog4js; + topic: function() { + var logEvent, + firstLog4js = require('../lib/log4js'), + secondLog4js; + firstLog4js.clearAppenders(); firstLog4js.addAppender(function(evt) { logEvent = evt; }); secondLog4js = require('../lib/log4js'); secondLog4js.getLogger().info("This should go to the appender defined in firstLog4js"); + return logEvent; + }, + 'should maintain appenders between requires': function (logEvent) { assert.equal(logEvent.data[0], "This should go to the appender defined in firstLog4js"); } }, diff --git a/test/test-log-abspath.js b/test/test-log-abspath.js index 94bc564..b0db18b 100644 --- a/test/test-log-abspath.js +++ b/test/test-log-abspath.js @@ -3,93 +3,66 @@ var vows = require('vows') , sandbox = require('sandboxed-module'); vows.describe('log4js-abspath').addBatch({ - 'configuration is passed as object with options.cwd': { + 'options': { topic: function() { - var appenderConfig - , log4js = sandbox.require( - '../lib/log4js' - , { requires: - { './appenders/file': - { - name: "file" - , appender: function() {} - , configure: function(configuration) { - appenderConfig = configuration; - return function() {}; + var appenderOptions, + log4js = sandbox.require( + '../lib/log4js', + { requires: + { './appenders/fake': + { + name: "fake", + appender: function() {}, + configure: function(configuration, options) { + appenderOptions = options; + return function() {}; + } + } } - } - } - } - ) - , config = { - "appenders": [ - { - "type" : "file", - "filename" : "cheesy-wotsits.log", - "maxLogSize" : 1024, - "backups" : 3, - "pollInterval" : 15 } - ] - }; + ), + config = { + "appenders": [ + { + "type" : "fake", + "filename" : "cheesy-wotsits.log" + } + ] + }; + log4js.configure(config, { cwd: '/absolute/path/to' }); - return appenderConfig; + return appenderOptions; }, - 'should be an absolute path': function(configuration) { - assert.equal(configuration.filename, '/absolute/path/to/cheesy-wotsits.log'); + 'should be passed to appenders during configuration': function(options) { + assert.equal(options.cwd, '/absolute/path/to'); } }, - 'configuration passed as filename with options.cwd': { + 'file appender': { topic: function() { - var appenderConfig - , configFilename - , log4js = sandbox.require( - '../lib/log4js' - , { requires: - { 'fs': - { - statSync: function() { - return { mtime: Date.now() }; - }, - readFileSync: function(filename) { - configFilename = filename; - return JSON.stringify({ - appenders: [ - { type: "file" - , filename: "whatever.log" - } - ] - }); - }, - readdirSync: function() { - return ['file']; + var fileOpened, + fileAppender = sandbox.require( + '../lib/appenders/file', + { requires: + { '../streams': + { + RollingFileStream: function(file) { + fileOpened = file; + }, + BufferedWriteStream: function(other) { + return { on: function() { }, end: function() {} } + } } - } - , './appenders/file': - { - name: "file" - , appender: function() {} - , configure: function(configuration) { - appenderConfig = configuration; - return function() {}; } } - } - } - ); - log4js.configure("/path/to/cheese.json", { - cwd: '/absolute/path/to' - }); - return [ configFilename, appenderConfig ]; + ); + fileAppender.configure({ filename: "whatever.log", maxLogSize: 10 }, { cwd: '/absolute/path/to' }); + return fileOpened; }, - 'should read the config from a file': function(args) { - assert.equal(args[0], '/path/to/cheese.json'); - }, - 'should be an absolute path': function(args) { - assert.equal(args[1].filename, "/absolute/path/to/whatever.log"); + 'should prepend options.cwd to config.filename': function(fileOpened) { + assert.equal(fileOpened, "/absolute/path/to/whatever.log"); } }, }).export(module); \ No newline at end of file