minor cleanups

diff --git a/lib/router/index.js b/lib/router/index.js
index 0e2a4e374e339ad2c6790a50f93badc4d4179aff..4976cd97e954b69236912d92282acadeddce7bc4 100644 (file)
--- a/lib/router/index.js
+++ b/lib/router/index.js
@@ -6,19 +6,10 @@
  */
 
 const { METHODS: httpMethods } = require('node:http');
-const common = require('../common');
+const { setOptions } = require('../common');
 const { DingusError, RouterNoPathError, RouterNoMethodError } = require('../errors');
 const PathParameter = require('./path-parameter');
 
-// Internal identifiers for route entries.
-const kPathMethods = Symbol('kSqueepDingusRouterPathMethods');
-const kPathName = Symbol('kSqueepDingusRouterPathName');
-
-const defaultOptions = {
-  ignoreTrailingSlash: false,
-  paramPrefix: ':',
-};
-
 /**
  * @typedef {Array<string|PathParameter>} RoutePath
  * @property {{[method: string]: PathHandler}} kPathMethods (symbol key)
@@ -49,9 +40,19 @@ const defaultOptions = {
  * mapping of methods to handler functions.
  */
 class Router {
-  static kPathMethods = kPathMethods;
-  static kPathName = kPathName;
+  // Internal identifiers for route entries.
+  static kPathMethods = Symbol('kSqueepDingusRouterPathMethods');
+  static kPathName = Symbol('kSqueepDingusRouterPathName');
+
+  static defaultOptions = {
+    ignoreTrailingSlash: false,
+    paramPrefix: ':',
+  };
+
+  ignoreTrailingSlash;
+  paramPrefix;
   pathsByLength;
+  pathsByName;
 
   /**
    * @param {object} options options object
@@ -59,7 +60,7 @@ class Router {
    * @param {string} options.paramPrefix prefix of a path part denoting a named parameter when registering paths (default: ':')
    */
   constructor(options = {}) {
-    common.setOptions(this, defaultOptions, options);
+    setOptions(this, this.constructor.defaultOptions, options);
 
     /**
      * Keep lists of registered paths to match, indexed by number of path parts.
@@ -90,13 +91,13 @@ class Router {
       .map((p) => this._pathPartMunge(p));
 
     if (this.ignoreTrailingSlash
-    &&  routePath[routePath.length - 1] === '') {
+    &&  (routePath[routePath.length - 1] === '')) {
       routePath.pop();
     }
 
-    routePath[kPathMethods] = {};
+    routePath[this.constructor.kPathMethods] = {};
     if (name) {
-      routePath[kPathName] = name;
+      routePath[this.constructor.kPathName] = name;
     }
 
     Object.defineProperty(routePath, 'path', {
@@ -129,6 +130,7 @@ class Router {
 
   /**
    * Compare checkPath to fixedPath, no param substitution, params must match.
+   * Used when adding routes to find existing.
    * @param {RoutePath} routePath route path 
    * @param {RoutePath} checkPath path to match
    * @returns {boolean} matched
@@ -141,7 +143,7 @@ class Router {
     for (let i = 0; i < routePath.length; i++) {
       const fixedPart = routePath[i];
       const checkPart = checkPath[i];
-      if (fixedPart instanceof PathParameter && checkPart instanceof PathParameter) {
+      if ((fixedPart instanceof PathParameter) && (checkPart instanceof PathParameter)) {
         if (fixedPart.parameter !== checkPart.parameter) {
           return false;
         }
@@ -157,15 +159,14 @@ class Router {
    * Compare routePath to fixedPath, populating params.
    * @param {RoutePath} routePath route path
    * @param {Array<string>} checkPath request path to check
-   * @param {object} returnParams populated param components on match
-   * @returns {boolean} matched
+   * @returns {[boolean, object]} [matched, params]
    * @private
    */
-  static _pathCompareParam(routePath, checkPath, returnParams = {}) {
+  static _pathCompareParam(routePath, checkPath) {
     const params = {};
 
     if (routePath.length !== checkPath.length) {
-      return false;
+      return [false, undefined];
     }
     for (let i = 0; i < routePath.length; i++) {
       const fixedPart = routePath[i];
@@ -173,11 +174,10 @@ class Router {
       if (fixedPart instanceof PathParameter) {
         params[fixedPart.parameter] = checkPart;
       } else if (fixedPart !== checkPart) {
-        return false;
+        return [false, undefined];
       }
     }
-    Object.assign(returnParams, params);
-    return true;
+    return [true, params];
   }
 
 
@@ -195,11 +195,12 @@ class Router {
     const pathsByLength = this.pathsByLength[matchParts.length];
     if (pathsByLength) {
       for (const p of pathsByLength) {
-        if (Router._pathCompareParam(p, matchParts, result.pathParams)) {
+        const [isMatch, pathParams] = this.constructor._pathCompareParam(p, matchParts, result.pathParams);
+        if (isMatch) {
           result.matchedPath = p;
+          result.pathParams = pathParams;
           break;
         }
-        result.pathParams = {}; // Reset after potential population from failed match.
       }
     }
     return result;
@@ -252,14 +253,14 @@ class Router {
     if (!Array.isArray(methods)) {
       methods = [methods];
     }
-    if (typeof handlerArgs !== 'undefined' && !Array.isArray(handlerArgs)) {
+    if (((typeof handlerArgs) !== 'undefined') && (!Array.isArray(handlerArgs))) {
       throw new TypeError(`handlerArgs must be an Array, not '${typeof handlerArgs}'`);
     }
     methods.forEach((method) => {
-      if (!httpMethods.includes(method) && method !== '*') {
+      if ((!httpMethods.includes(method)) && (method !== '*')) {
         throw new DingusError(`invalid method '${method}'`);
       }
-      existingPath[kPathMethods][method] = { handler, handlerArgs: handlerArgs ?? [] };
+      existingPath[this.constructor.kPathMethods][method] = { handler, handlerArgs: handlerArgs ?? [] };
     });
   }
 
@@ -277,20 +278,20 @@ class Router {
   lookup(method, urlPath, ctx = {}) {
     const pathParts = urlPath.split('/').map((part) => decodeURIComponent(part));
     if (this.ignoreTrailingSlash
-    &&  pathParts[pathParts.length - 1] === '') {
+    &&  (pathParts[pathParts.length - 1] === '')) {
       pathParts.pop();
     }  
     const { matchedPath, pathParams } = this._pathFind(pathParts);
     ctx.params = pathParams;
     if (matchedPath) {
       ctx.matchedPath = matchedPath.path;
-      if (method in matchedPath[kPathMethods]) {
-        return matchedPath[kPathMethods][method];
+      if (method in matchedPath[this.constructor.kPathMethods]) {
+        return matchedPath[this.constructor.kPathMethods][method];
       }
-      if ('*' in matchedPath[kPathMethods]) {
-        return matchedPath[kPathMethods]['*'];
+      if ('*' in matchedPath[this.constructor.kPathMethods]) {
+        return matchedPath[this.constructor.kPathMethods]['*'];
       }
-      throw new RouterNoMethodError(Object.keys(matchedPath[kPathMethods]));
+      throw new RouterNoMethodError(Object.keys(matchedPath[this.constructor.kPathMethods]));
     }
     ctx.unmatchedPath = pathParts;
     throw new RouterNoPathError();

diff --git a/lib/router/path-parameter.js b/lib/router/path-parameter.js
index 7728295250b182f88323a524d48a37f93b43c8cc..b618e6d7f9546842bd15d6f77096414ab3f3eed0 100644 (file)
--- a/lib/router/path-parameter.js
+++ b/lib/router/path-parameter.js
@@ -1,11 +1,10 @@
 'use strict';
 
-const parameters = new Map();
 /**
  * De-duplicating factory of minimal-objects to track the named-parameter parts of path definitions.
  */
 class PathParameter extends null {
-  static #parameters = parameters;
+  static #parameters = new Map();
 
   /**
    * 
@@ -13,7 +12,7 @@ class PathParameter extends null {
    * @returns {PathParameter} frozen parameter object
    */
   constructor(parameter) {
-    if (!parameter || typeof(parameter) !== 'string') {
+    if ((!parameter) || ((typeof parameter) !== 'string')) {
       throw new RangeError('parameter must be string');
     }
 
@@ -21,6 +20,7 @@ class PathParameter extends null {
       return PathParameter.#parameters.get(parameter); // NOSONAR
     }
 
+    // Because null super.
     const pathParameter = Object.create(PathParameter.prototype);
     pathParameter.parameter = parameter;
     Object.freeze(pathParameter);

diff --git a/test/lib/router.js b/test/lib/router.js
index c5a51303c33079512701e4adfa26f16aa26f4aa7..d19ae89bc4af7d370ed5b18e80edd4a75c996898 100644 (file)
--- a/test/lib/router.js
+++ b/test/lib/router.js
@@ -101,52 +101,47 @@ describe('Router', function () {
     let fixedPath, checkPath;
 
     it('compares static paths which match', function () {
-      const params = {};
       const expectedParams = {};
       fixedPath = router._pathToRoutePath('/a/b/c');
       checkPath = router._pathToRoutePath('/a/b/c');
-      const r = Router._pathCompareParam(fixedPath, checkPath);
+      const [r, params] = Router._pathCompareParam(fixedPath, checkPath);
       assert.strictEqual(r, true);
       assert.deepStrictEqual(params, expectedParams);
     });
     it('compares static paths which do not match', function () {
-      const params = {};
-      const expectedParams = {};
+      const expectedParams = undefined;
       fixedPath = router._pathToRoutePath('/a/b/c');
       checkPath = router._pathToRoutePath('/a/b/d');
-      const r = Router._pathCompareParam(fixedPath, checkPath, params);
+      const [r, params] = Router._pathCompareParam(fixedPath, checkPath);
       assert.strictEqual(r, false);
-      assert.deepStrictEqual(params, expectedParams);
+      assert.strictEqual(params, expectedParams);
     });
     it('compares unequal static paths', function () {
-      const params = {};
-      const expectedParams = {};
+      const expectedParams = undefined;
       fixedPath = router._pathToRoutePath('/a/b/c');
       checkPath = router._pathToRoutePath('/a/b');
-      const r = Router._pathCompareParam(fixedPath, checkPath, params);
+      const [r, params] = Router._pathCompareParam(fixedPath, checkPath);
       assert.strictEqual(r, false);
       assert.deepStrictEqual(params, expectedParams);
     });
     it('compares param paths which match', function () {
-      const params = {};
       const expectedParams = {
         b: 'bar',
       };
       fixedPath = router._pathToRoutePath('/a/:b/c');
       checkPath = router._pathToRoutePath('/a/bar/c');
-      const r = Router._pathCompareParam(fixedPath, checkPath, params);
+      const [r, params] = Router._pathCompareParam(fixedPath, checkPath);
       assert.strictEqual(r, true);
       assert.deepStrictEqual(params, expectedParams);
     });
     it('compares multi param path which match', function () {
-      const params = {};
       const expectedParams = {
         b: 'gaz',
         c: '123',
       };
       fixedPath = router._pathToRoutePath('/a/:b/:c');
       checkPath = router._pathToRoutePath('/a/gaz/123');
-      const r = Router._pathCompareParam(fixedPath, checkPath, params);
+      const [r, params] = Router._pathCompareParam(fixedPath, checkPath);
       assert.strictEqual(r, true);
       assert.deepStrictEqual(params, expectedParams);
     });