Repository: delvedor/find-my-way (js)

Issue:
`constraints` will be overrided when multiple router created
The current code override the `HandlerStorage.prototype.constrainer` to get the `constrainter` in router.
It will be override when two store created first before any route trying to lookup.

```js
const findMyWay = require("find-my-way");

const constraint = {
  name: "secret",
  storage: function () {
    let secrets = {};
    return {
      get: (secret) => {
        return secrets[secret] || null;
      },
      set: (secret, store) => {
        secrets[secret] = store;
      },
      del: (secret) => {
        delete secrets[secret];
      },
      empty: () => {
        secrets = {};
      },
    };
  },
  deriveConstraint: (req, ctx) => {
    return req.headers["x-secret"];
  },
  validate() {
    return true;
  },
};

let router1 = findMyWay({ constraints: { secret: constraint } });

// it override the constraints of router1
const router2 = findMyWay({ constraints: undefined });

// it can registered constraints
router1.on("GET", "/", { constraints: { secret: "alpha" } }, () => {});
// it cannot find the constraints secret
router1.find("GET", "/", { constraints: { secret: "alpha" } });
```

Pull request description:
Fix constrainer overwrite
Fixed #254 

Patch:
diff --git a/handler_storage.js b/handler_storage.js
index 19e53a9..c699af7 100644
--- a/handler_storage.js
+++ b/handler_storage.js
@@ -5,8 +5,6 @@ const deepEqual = require('fast-deep-equal')
 
 class HandlerStorage {
   constructor () {
-    this.constrainer = HandlerStorage.prototype.constrainer
-
     this.handlers = [] // unoptimized list of handler objects for which the fast matcher function will be compiled
     this.hasConstraints = false
     this.compiledHandler = null
@@ -19,14 +17,14 @@ class HandlerStorage {
   }
 
   // This is the hot path for node handler finding -- change with care!
-  getMatchingHandler (derivedConstraints) {
+  getMatchingHandler (constrainer, derivedConstraints) {
     if (derivedConstraints === undefined) {
       return this.unconstrainedHandler
     }
 
     if (this.hasConstraints) {
       // This node is constrained, use the performant precompiled constraint matcher
-      return this._getHandlerMatchingConstraints(derivedConstraints)
+      return this._getHandlerMatchingConstraints(constrainer, derivedConstraints)
     }
 
     // This node doesn't have any handlers that are constrained, so it's handlers probably match. Some requests have constraint values that *must* match however, like version, so check for those before returning it.
@@ -63,9 +61,9 @@ class HandlerStorage {
   }
 
   // Slot for the compiled constraint matching function
-  _getHandlerMatchingConstraints (derivedConstraints) {
+  _getHandlerMatchingConstraints (constrainer, derivedConstraints) {
     if (this.compiledHandler === null) {
-      this.compiledHandler = this._compileGetHandlerMatchingConstraints()
+      this.compiledHandler = this._compileGetHandlerMatchingConstraints(constrainer)
     }
     return this.compiledHandler(derivedConstraints)
   }
@@ -73,8 +71,8 @@ class HandlerStorage {
   // Builds a store object that maps from constraint values to a bitmap of handler indexes which pass the constraint for a value
   // So for a host constraint, this might look like { "fastify.io": 0b0010, "google.ca": 0b0101 }, meaning the 3rd handler is constrainted to fastify.io, and the 2nd and 4th handlers are constrained to google.ca.
   // The store's implementation comes from the strategies provided to the Router.
-  _buildConstraintStore (constraint) {
-    const store = this.constrainer.newStoreForConstraint(constraint)
+  _buildConstraintStore (constrainer, constraint) {
+    const store = constrainer.newStoreForConstraint(constraint)
 
     for (let i = 0; i < this.handlers.length; i++) {
       const handler = this.handlers[i]
@@ -110,7 +108,7 @@ class HandlerStorage {
   // We do this by asking each constraint store which handler indexes match the given constraint value for each store. Trickily, the handlers that a store says match are the handlers constrained by that store, but handlers that aren't constrained at all by that store could still match just fine. So, each constraint store can only describe matches for it, and it won't have any bearing on the handlers it doesn't care about. For this reason, we have to ask each stores which handlers match and track which have been matched (or not cared about) by all of them.
   // We use bitmaps to represent these lists of matches so we can use bitwise operations to implement this efficiently. Bitmaps are cheap to allocate, let us implement this masking behaviour in one CPU instruction, and are quite compact in memory. We start with a bitmap set to all 1s representing every handler that is a match candidate, and then for each constraint, see which handlers match using the store, and then mask the result by the mask of handlers that that store applies to, and bitwise AND with the candidate list. Phew.
   // We consider all this compiling function complexity to be worth it, because the naive implementation that just loops over the handlers asking which stores match is quite a bit slower.
-  _compileGetHandlerMatchingConstraints () {
+  _compileGetHandlerMatchingConstraints (constrainer) {
     this.constrainedHandlerStores = {}
     let constraints = new Set()
     for (const handler of this.handlers) {
@@ -125,7 +123,7 @@ class HandlerStorage {
     constraints.sort((a, b) => a === 'version' ? 1 : 0)
 
     for (const constraint of constraints) {
-      this.constrainedHandlerStores[constraint] = this._buildConstraintStore(constraint)
+      this.constrainedHandlerStores[constraint] = this._buildConstraintStore(constrainer, constraint)
     }
 
     lines.push(`
diff --git a/index.js b/index.js
index 3e09d35..0cc14b3 100644
--- a/index.js
+++ b/index.js
@@ -32,7 +32,6 @@ const { flattenNode, compressFlattenedNode, prettyPrintFlattenedNode, prettyPrin
 const { StaticNode, NODE_TYPES } = require('./custom_node')
 const Constrainer = require('./lib/constrainer')
 const sanitizeUrl = require('./lib/url-sanitizer')
-const HandlerStorage = require('./handler_storage')
 
 const httpMethods = http.METHODS
 const FULL_PATH_REGEXP = /^https?:\/\/.*?\//
@@ -86,7 +85,6 @@ function Router (opts) {
   this.trees = {}
 
   this.constrainer = new Constrainer(opts.constraints)
-  HandlerStorage.prototype.constrainer = this.constrainer
 }
 
 Router.prototype.on = function on (method, path, opts, handler, store) {
@@ -352,7 +350,7 @@ Router.prototype.find = function find (method, path, derivedConstraints) {
   while (true) {
     // found the route
     if (pathIndex === pathLen) {
-      const handle = currentNode.handlerStorage.getMatchingHandler(derivedConstraints)
+      const handle = currentNode.handlerStorage.getMatchingHandler(this.constrainer, derivedConstraints)
 
       if (handle !== null && handle !== undefined) {
         const paramsObj = {}


Test patch:
diff --git a/test/issue-254.test.js b/test/issue-254.test.js
new file mode 100644
index 0000000..e8ea057
--- /dev/null
+++ b/test/issue-254.test.js
@@ -0,0 +1,32 @@
+'use strict'
+
+const t = require('tap')
+const test = t.test
+const FindMyWay = require('..')
+
+test('Constraints should not be overrided when multiple router is created', t => {
+  t.plan(1)
+
+  const constraint = {
+    name: 'secret',
+    storage: function () {
+      const secrets = {}
+      return {
+        get: (secret) => { return secrets[secret] || null },
+        set: (secret, store) => { secrets[secret] = store }
+      }
+    },
+    deriveConstraint: (req, ctx) => {
+      return req.headers['x-secret']
+    },
+    validate () { return true }
+  }
+
+  const router1 = FindMyWay({ constraints: { secret: constraint } })
+  FindMyWay()
+
+  router1.on('GET', '/', { constraints: { secret: 'alpha' } }, () => {})
+  router1.find('GET', '/', { constraints: { secret: 'alpha' } })
+
+  t.pass('constraints is not overrided')
+})