chore: improved connect-logger test coverage

lib/connect-logger.js

@@ -146,20 +146,18 @@ function format(str, tokens) {
 function createNoLogCondition(nolog) {
   let regexp = null;
 
-  if (nolog) {
-    if (nolog instanceof RegExp) {
-      regexp = nolog;
-    }
+  if (nolog instanceof RegExp) {
+    regexp = nolog;
+  }
 
-    if (typeof nolog === 'string') {
-      regexp = new RegExp(nolog);
-    }
+  if (typeof nolog === 'string') {
+    regexp = new RegExp(nolog);
+  }
 
-    if (Array.isArray(nolog)) {
-      // convert to strings
-      const regexpsAsStrings = nolog.map(reg => (reg.source ? reg.source : reg));
-      regexp = new RegExp(regexpsAsStrings.join('|'));
-    }
+  if (Array.isArray(nolog)) {
+    // convert to strings
+    const regexpsAsStrings = nolog.map(reg => (reg.source ? reg.source : reg));
+    regexp = new RegExp(regexpsAsStrings.join('|'));
   }
 
   return regexp;
@@ -189,7 +187,7 @@ function matchRules(statusCode, currentLevel, ruleSet) {
       let ruleMatched = false;
       if (rule.from && rule.to) {
         ruleMatched = statusCode >= rule.from && statusCode <= rule.to;
-      } else if (rule.codes) {
+      } else {
         ruleMatched = rule.codes.indexOf(statusCode) !== -1;
       }
       return ruleMatched;
@@ -232,18 +230,16 @@ function matchRules(statusCode, currentLevel, ruleSet) {
    */
 module.exports = function getLogger(logger4js, options) {
   /* eslint no-underscore-dangle:0 */
-  if (typeof options === 'object') {
-    options = options || {};
-  } else if (options) {
+  if (typeof options === 'string' || typeof options === 'function') {
     options = { format: options };
   } else {
-    options = {};
+    options = options || {};
   }
 
   const thisLogger = logger4js;
   let level = levels.getLevel(options.level, levels.INFO);
   const fmt = options.format || DEFAULT_FORMAT;
-  const nolog = options.nolog ? createNoLogCondition(options.nolog) : null;
+  const nolog = createNoLogCondition(options.nolog);
 
   return (req, res, next) => {
     // mount safety
@@ -289,15 +285,13 @@ module.exports = function getLogger(logger4js, options) {
         }
         level = matchRules(res.statusCode, level, options.statusRules);
 
-        if (thisLogger.isLevelEnabled(level)) {
-          const combinedTokens = assembleTokens(req, res, options.tokens || []);
+        const combinedTokens = assembleTokens(req, res, options.tokens || []);
 
-          if (typeof fmt === 'function') {
-            const line = fmt(req, res, str => format(str, combinedTokens));
-            if (line) thisLogger.log(level, line);
-          } else {
-            thisLogger.log(level, format(fmt, combinedTokens));
-          }
+        if (typeof fmt === 'function') {
+          const line = fmt(req, res, str => format(str, combinedTokens));
+          if (line) thisLogger.log(level, line);
+        } else {
+          thisLogger.log(level, format(fmt, combinedTokens));
         }
       });
     }

test/tap/connect-logger-test.js

@@ -19,9 +19,10 @@ class MockLogger {
   }
 }
 
-function MockRequest(remoteAddr, method, originalUrl, headers) {
+function MockRequest(remoteAddr, method, originalUrl, headers, url) {
   this.socket = { remoteAddress: remoteAddr };
   this.originalUrl = originalUrl;
+  this.url = url;
   this.method = method;
   this.httpVersionMajor = '5';
   this.httpVersionMinor = '0';
@@ -48,11 +49,15 @@ class MockResponse extends EE {
   }
 }
 
-function request(cl, method, url, code, reqHeaders, resHeaders) {
-  const req = new MockRequest('my.remote.addr', method, url, reqHeaders);
+function request(cl, method, originalUrl, code, reqHeaders, resHeaders, next, url) {
+  const req = new MockRequest('my.remote.addr', method, originalUrl, reqHeaders, url);
   const res = new MockResponse();
-  cl(req, res, () => {
-  });
+  if (next) {
+    next = next.bind(null, req, res, () => {});
+  } else {
+    next = () => {};
+  }
+  cl(req, res, next);
   res.writeHead(code, resHeaders);
   res.end('chunk', 'encoding');
 }
@@ -100,16 +105,32 @@ test('log4js connect logger', (batch) => {
     t.test('log events with non-default level and custom format', (assert) => {
       const ml = new MockLogger();
       ml.level = levels.INFO;
-      const cl = clm(ml, { level: levels.INFO, format: ':method :url' });
+      const cl = clm(ml, { level: levels.WARN, format: ':method :url' });
       request(cl, 'GET', 'http://url', 200);
 
       const messages = ml.messages;
       assert.type(messages, Array);
       assert.equal(messages.length, 1);
-      assert.ok(levels.INFO.isEqualTo(messages[0].level));
+      assert.ok(levels.WARN.isEqualTo(messages[0].level));
       assert.equal(messages[0].message, 'GET http://url');
       assert.end();
     });
+
+    t.test('adding multiple loggers should only log once', (assert) => {
+      const ml = new MockLogger();
+      ml.level = levels.INFO;
+      const cl = clm(ml, { level: levels.WARN, format: ':method :url' });
+      const nextLogger = clm(ml, { level: levels.INFO, format: ':method' });
+      request(cl, 'GET', 'http://url', 200, null, null, nextLogger);
+
+      const messages = ml.messages;
+      assert.type(messages, Array);
+      assert.equal(messages.length, 1);
+      assert.ok(levels.WARN.isEqualTo(messages[0].level));
+      assert.equal(messages[0].message, 'GET http://url');
+
+      assert.end();
+    });
     t.end();
   });
 
@@ -209,6 +230,26 @@ test('log4js connect logger', (batch) => {
     t.end();
   });
 
+  batch.test('format using a function that also uses tokens', (t) => {
+    const ml = new MockLogger();
+    ml.level = levels.INFO;
+    const cl = clm(ml, (req, res, tokenReplacer) => `${req.method} ${tokenReplacer(':status')}`);
+    request(cl, 'GET', 'http://blah', 200);
+
+    t.equal(ml.messages[0].message, 'GET 200');
+    t.end();
+  });
+
+  batch.test('format using a function, but do not log anything if the function returns nothing', (t) => {
+    const ml = new MockLogger();
+    ml.level = levels.INFO;
+    const cl = clm(ml, () => null);
+    request(cl, 'GET', 'http://blah', 200);
+
+    t.equal(ml.messages.length, 0);
+    t.end();
+  });
+
   batch.test('format that includes request headers', (t) => {
     const ml = new MockLogger();
     ml.level = levels.INFO;
@@ -238,6 +279,15 @@ test('log4js connect logger', (batch) => {
     t.end();
   });
 
+  batch.test('url token should check originalUrl and url', (t) => {
+    const ml = new MockLogger();
+    const cl = clm(ml, ':url');
+    request(cl, 'GET', null, 200, null, null, null, 'http://cheese');
+
+    t.equal(ml.messages[0].message, 'http://cheese');
+    t.end();
+  });
+
   batch.test('log events with custom token', (t) => {
     const ml = new MockLogger();
     ml.level = levels.INFO;

test/tap/connect-nolog-test.js

@@ -221,5 +221,52 @@ test('log4js connect logger', (batch) => {
     t.end();
   });
 
+  batch.test('nolog Array<RegExp>', (t) => {
+    const ml = new MockLogger();
+    const cl = clm(ml, { nolog: [/\.gif/, /\.jpe?g/] });
+
+    t.beforeEach((done) => { ml.messages = []; done(); });
+
+    t.test('check unmatch url request (png)', (assert) => {
+      const messages = ml.messages;
+      const req = new MockRequest('my.remote.addr', 'GET', 'http://url/hoge.png'); // not gif
+      const res = new MockResponse(200);
+      cl(req, res, () => { });
+      res.end('chunk', 'encoding');
+
+      assert.equal(messages.length, 1);
+      assert.ok(levels.INFO.isEqualTo(messages[0].level));
+      assert.include(messages[0].message, 'GET');
+      assert.include(messages[0].message, 'http://url');
+      assert.include(messages[0].message, 'my.remote.addr');
+      assert.include(messages[0].message, '200');
+      assert.end();
+    });
+
+    t.test('check match url request (gif)', (assert) => {
+      const messages = ml.messages;
+      const req = new MockRequest('my.remote.addr', 'GET', 'http://url/hoge.gif'); // gif
+      const res = new MockResponse(200);
+      cl(req, res, () => {});
+      res.end('chunk', 'encoding');
+
+      assert.equal(messages.length, 0);
+      assert.end();
+    });
+
+    t.test('check match url request (jpeg)', (assert) => {
+      const messages = ml.messages;
+      const req = new MockRequest('my.remote.addr', 'GET', 'http://url/hoge.jpeg'); // gif
+      const res = new MockResponse(200);
+      cl(req, res, () => {});
+      res.end('chunk', 'encoding');
+
+      assert.equal(messages.length, 0);
+      assert.end();
+    });
+
+    t.end();
+  });
+
   batch.end();
 });