Merge pull request #1333 from log4js-node/refactor/LoggingEvent

refactor(LoggingEvent): loop through location keys instead of hard-coding one-by-one
log4js-node · Oct 1, 2022 · cfbc7a0 · cfbc7a0
2 parents 570ef53 + 6a8b706
commit cfbc7a0
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 31 deletions.
diff --git a/lib/LoggingEvent.js b/lib/LoggingEvent.js
@@ -1,4 +1,5 @@
 /* eslint max-classes-per-file: ["error", 2] */
+/* eslint no-underscore-dangle: ["error", { "allow": ["_getLocationKeys"] }] */
 
 const flatted = require('flatted');
 const levels = require('./levels');
@@ -63,18 +64,32 @@ class LoggingEvent {
     this.pid = process.pid;
     this.error = error;
 
-    if (location) {
-      this.fileName = location.fileName;
-      this.lineNumber = location.lineNumber;
-      this.columnNumber = location.columnNumber;
-      this.callStack = location.callStack;
-      this.className = location.className;
-      this.functionName = location.functionName;
-      this.functionAlias = location.functionAlias;
-      this.callerName = location.callerName;
+    if (typeof location !== 'undefined') {
+      if (!location || typeof location !== 'object' || Array.isArray(location))
+        throw new TypeError(
+          'Invalid location type passed to LoggingEvent constructor'
+        );
+
+      this.constructor._getLocationKeys().forEach((key) => {
+        if (typeof location[key] !== 'undefined') this[key] = location[key];
+      });
     }
   }
 
+  /** @private */
+  static _getLocationKeys() {
+    return [
+      'fileName',
+      'lineNumber',
+      'columnNumber',
+      'callStack',
+      'className',
+      'functionName',
+      'functionAlias',
+      'callerName',
+    ];
+  }
+
   serialise() {
     return flatted.stringify(this, (key, value) => {
       // JSON.stringify(new Error('test')) returns {}, which is not really useful for us.
@@ -107,27 +122,12 @@ class LoggingEvent {
         }
         return serde.deserialise(value);
       });
-      if (
-        rehydratedEvent.fileName ||
-        rehydratedEvent.lineNumber ||
-        rehydratedEvent.columnNumber ||
-        rehydratedEvent.callStack ||
-        rehydratedEvent.className ||
-        rehydratedEvent.functionName ||
-        rehydratedEvent.functionAlias ||
-        rehydratedEvent.callerName
-      ) {
-        rehydratedEvent.location = {
-          fileName: rehydratedEvent.fileName,
-          lineNumber: rehydratedEvent.lineNumber,
-          columnNumber: rehydratedEvent.columnNumber,
-          callStack: rehydratedEvent.callStack,
-          className: rehydratedEvent.className,
-          functionName: rehydratedEvent.functionName,
-          functionAlias: rehydratedEvent.functionAlias,
-          callerName: rehydratedEvent.callerName,
-        };
-      }
+      this._getLocationKeys().forEach((key) => {
+        if (typeof rehydratedEvent[key] !== 'undefined') {
+          if (!rehydratedEvent.location) rehydratedEvent.location = {};
+          rehydratedEvent.location[key] = rehydratedEvent[key];
+        }
+      });
       event = new LoggingEvent(
         rehydratedEvent.categoryName,
         levels.getLevel(rehydratedEvent.level.levelStr),

diff --git a/test/tap/LoggingEvent-test.js b/test/tap/LoggingEvent-test.js
@@ -4,6 +4,21 @@ const LoggingEvent = require('../../lib/LoggingEvent');
 const levels = require('../../lib/levels');
 
 test('LoggingEvent', (batch) => {
+  batch.test('should throw error for invalid location', (t) => {
+    t.throws(
+      () =>
+        new LoggingEvent(
+          'cheese',
+          levels.DEBUG,
+          ['log message'],
+          undefined,
+          []
+        ),
+      'Invalid location type passed to LoggingEvent constructor'
+    );
+    t.end();
+  });
+
   batch.test('should serialise to flatted', (t) => {
     const event = new LoggingEvent(
       'cheese',

diff --git a/test/tap/logger-test.js b/test/tap/logger-test.js
@@ -371,7 +371,10 @@ test('../../lib/logger', (batch) => {
     t.equal(events[0].fileName, initialEvent.fileName);
     t.equal(events[0].columnNumber, initialEvent.columnNumber);
 
-    t.throws(() => logger.setParseCallStackFunction('not a function'));
+    t.throws(
+      () => logger.setParseCallStackFunction('not a function'),
+      'Invalid type passed to setParseCallStackFunction'
+    );
 
     t.end();
   });