Moved abspath option checking into file appender, log4js options now passed to appenders

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);
 }
 

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 = {

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");
         }
     },

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 ];
-        },
-        'should read the config from a file': function(args) {
-            assert.equal(args[0], '/path/to/cheese.json');
+                );
+            fileAppender.configure({ filename: "whatever.log", maxLogSize: 10 }, { cwd: '/absolute/path/to' });
+            return fileOpened;
         },
-        '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);