fix(logging): Upgrade to log4js 2.x API. (#2868)

johnjbarton · web-flow · commit f6f8707efaff · 2017-11-20T16:00:47.000-08:00
Support log4js 2.x as well as 1.x for configuration. Fixes #2858
diff --git a/lib/logger.js b/lib/logger.js
@@ -17,29 +17,45 @@ var constant = require('./constants')
 //
 // * `logLevel`: *String* Defines the global log level.
 // * `colors`: *Boolean* Use colors in the stdout or not.
-// * `appenders`: *Array* This will be passed as appenders to log4js
+// * `appenders`: *Object*  This will be passed as appenders to log4js
 //   to allow for fine grained configuration of log4js. For more information
 //   see https://github.com/nomiddlename/log4js-node.
+//   *Array* is also accepted for backwards compatibility.
 var setup = function (level, colors, appenders) {
   // Turn color on/off on the console appenders with pattern layout
   var pattern = colors ? constant.COLOR_PATTERN : constant.NO_COLOR_PATTERN
-
-  // If there are no appenders use the default one
-  appenders = helper.isDefined(appenders) ? appenders : [constant.CONSOLE_APPENDER]
-
-  appenders = appenders.map(function (appender) {
-    if (appender.type === 'console') {
-      if (helper.isDefined(appender.layout) && appender.layout.type === 'pattern') {
-        appender.layout.pattern = pattern
+  if (appenders) {
+    // Convert Array to Object for backwards compatibility.
+    if (appenders['map']) {
+      if (appenders.length === 0) {
+        appenders = [constant.CONSOLE_APPENDER]
       }
+      const v1Appenders = appenders
+      appenders = {}
+      v1Appenders.forEach(function (appender, index) {
+        if (appender.type === 'console') {
+          appenders['console'] = appender
+          if (helper.isDefined(appender.layout) && appender.layout.type === 'pattern') {
+            appender.layout.pattern = pattern
+          }
+        } else {
+          appenders[index + ''] = appender
+        }
+        return appender
+      })
     }
-    return appender
-  })
+  } else {
+    appenders = {'console' : constant.CONSOLE_APPENDER}
+  }
 
-  // Pass the values to log4js
-  log4js.setGlobalLogLevel(level)
   log4js.configure({
-    appenders: appenders
+    appenders: appenders,
+    categories: {
+      'default': {
+        'appenders': Object.keys(appenders),
+        'level': level
+      }
+    }
   })
 }
 
@@ -49,9 +65,10 @@ var setup = function (level, colors, appenders) {
 //     setupFromConfig(config, appenders)
 //
 // * `config`: *Object* The configuration object.
-// * `appenders`: *Array* This will be passed as appenders to log4js
+// * `appenders`: *Object*  This will be passed as appenders to log4js
 //   to allow for fine grained configuration of log4js. For more information
 //   see https://github.com/nomiddlename/log4js-node.
+//   *Array* is also accepted for backwards compatibility.
 var setupFromConfig = function (config, appenders) {
   var useColors = true
   var logLevel = constant.LOG_INFO
@@ -66,13 +83,22 @@ var setupFromConfig = function (config, appenders) {
   setup(logLevel, useColors, appenders)
 }
 
+const loggerCache = {}
+
 // Create a new logger. There are two optional arguments
 // * `name`, which defaults to `karma` and
 //   If the `name = 'socket.io'` this will create a special wrapper
 //   to be used as a logger for socket.io.
 // * `level`, which defaults to the global level.
 var create = function (name, level) {
-  var logger = log4js.getLogger(name || 'karma')
+  name = name || 'karma'
+  var logger
+  if (loggerCache.hasOwnProperty(name)) {
+    logger = loggerCache[name]
+  } else {
+    logger = log4js.getLogger(name)
+    loggerCache[name] = logger
+  }
   if (helper.isDefined(level)) {
     logger.setLevel(level)
   }
@@ -84,3 +110,6 @@ var create = function (name, level) {
 exports.create = create
 exports.setup = setup
 exports.setupFromConfig = setupFromConfig
+exports._rebindLog4js4testing = function(mockLog4js) {
+  log4js = mockLog4js
+}
diff --git a/package.json b/package.json
@@ -342,7 +342,7 @@
     "http-proxy": "^1.13.0",
     "isbinaryfile": "^3.0.0",
     "lodash": "^4.17.4",
-    "log4js": "^1.1.1",
+    "log4js": "^2.3.9",
     "mime": "^1.3.4",
     "minimatch": "^3.0.2",
     "optimist": "^0.6.1",
diff --git a/test/unit/config.spec.js b/test/unit/config.spec.js
@@ -71,8 +71,7 @@ describe('config', () => {
     var logSpy
 
     beforeEach(() => {
-      logSpy = sinon.spy()
-      logger.create('config').on('log', logSpy)
+      logSpy = sinon.spy(logger.create('config'), 'error')
     })
 
     it('should resolve relative basePath to config directory', () => {
@@ -114,19 +113,17 @@ describe('config', () => {
       e.parseConfig('/conf/not-exist.js', {})
 
       expect(logSpy).to.have.been.called
-      var event = logSpy.lastCall.args[0]
-      expect(event.level.toString()).to.be.equal('ERROR')
-      expect(event.data).to.be.deep.equal(['File %s does not exist!', '/conf/not-exist.js'])
+      var event = logSpy.lastCall.args
+      expect(event).to.be.deep.equal(['File %s does not exist!', '/conf/not-exist.js'])
       expect(mocks.process.exit).to.have.been.calledWith(1)
     })
 
     it('should throw and log error if invalid file', () => {
       e.parseConfig('/conf/invalid.js', {})
 
       expect(logSpy).to.have.been.called
-      var event = logSpy.lastCall.args[0]
-      expect(event.level.toString()).to.be.equal('ERROR')
-      expect(event.data).to.be.deep.equal([
+      var event = logSpy.lastCall.args
+      expect(event).to.be.deep.equal([
         'Error in config file!\n',
         new SyntaxError('Unexpected token =')
       ])
diff --git a/test/unit/logger.spec.js b/test/unit/logger.spec.js
@@ -3,19 +3,43 @@ import path from 'path'
 
 describe('logger', () => {
   var m
+  let configuration
 
   beforeEach(() => {
-    m = loadFile(path.join(__dirname, '/../../lib/logger.js'))
+    const mockLog4Js = {
+      configure: function (config) {
+        configuration = config
+      }
+    }
+    m = loadFile(path.join(__dirname, '/../../lib/logger.js'), {'log4js': mockLog4Js})
   })
 
   describe('setup', () => {
-    it('should allow for configuration via setup() using an array', () => {
+    it('should allow for configuration via setup() using an array for back-compat', () => {
       m.setup('INFO', true, [{
         type: 'file',
         filename: 'test/unit/test.log'
       }])
-
-      expect(m.log4js.appenders).to.have.keys(['console', 'file', 'stdout'])
+      expect(configuration).to.have.keys(['appenders', 'categories'])
+      expect(configuration.appenders).to.have.keys(['0'])
+      expect(configuration.appenders['0'].type).to.equal('file')
+      expect(configuration.categories).to.have.keys(['default'])
+      expect(configuration.categories.default.appenders).to.have.keys(['0'])
+      expect(configuration.categories.default.level).to.equal('INFO')
+    })
+    it('should allow setup() using log4js v2 object', () => {
+      m.setup('WARN', true, {
+        'fileAppender': {
+          type: 'file',
+          filename: 'test/unit/test.log'
+        }
+      })
+      expect(configuration).to.have.keys(['appenders', 'categories'])
+      expect(configuration.appenders).to.have.keys(['fileAppender'])
+      expect(configuration.appenders['fileAppender'].type).to.equal('file')
+      expect(configuration.categories).to.have.keys(['default'])
+      expect(configuration.categories.default.appenders[0]).to.equal('fileAppender')
+      expect(configuration.categories.default.level).to.equal('WARN')
     })
   })
 })
