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

Issue:
router.off method support host filter
When there are two identical paths in the route, but the host is different

```js
router.on('GET', '/a', { constraints: { host: '1' } }, () => {})
router.on('GET', '/a', { constraints: { host: '2' } }, () => {})
```

When executing the off method, want to filter by host, like:

```js
router.off('GET', '/a', {
  host: '1'
})
```

But now only the path parameter is supported，Run the following code and both routes will be deleted

```js
router.off('GET', '/a')

router.prettyPrint() // is empty
```


Pull request:
Add router.off method support filter
fix #247

Patch:
diff --git a/README.md b/README.md
index 46fa129..6f9d352 100644
--- a/README.md
+++ b/README.md
@@ -374,7 +374,7 @@ router.off('GET', '/example')
 // => null
 ```
 
-##### off(methods[], path, handler, [store])
+##### off(methods[], path)
 Deregister a route for each method specified in the `methods` array.
 It comes handy when you need to deregister multiple routes with the same path but different methods.
 ```js
@@ -383,6 +383,14 @@ router.off(['GET', 'POST'], '/example')
 // => null
 ```
 
+##### off(methods, path, [constraints])
+Deregister a route for each `constraints` key is matched, containing keys like the `host` for the request, the `version` for the route to be matched, or other custom constraint values. See the [constraints section](https://github.com/delvedor/find-my-way#constraints) to know more.
+```js
+router.off('GET', '/example', { host: 'fastify.io' })
+// => [{ handler: Function, params: Object, store: Object}]
+// => null
+```
+
 <a name="reset"></a>
 #### reset()
 Empty router.
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.d.ts b/index.d.ts
index fc7f26b..40be372 100644
--- a/index.d.ts
+++ b/index.d.ts
@@ -138,7 +138,11 @@ declare namespace Router {
       store: any
     ): void;
 
-    off(method: HTTPMethod | HTTPMethod[], path: string): void;
+    off(
+      method: HTTPMethod | HTTPMethod[],
+      path: string,
+      constraints?: { [key: string]: any }
+    ): void;
 
     lookup<Context>(
       req: Req<V>,
diff --git a/index.js b/index.js
index 3e09d35..2162d98 100644
--- a/index.js
+++ b/index.js
@@ -28,11 +28,11 @@
 const assert = require('assert')
 const http = require('http')
 const isRegexSafe = require('safe-regex2')
+const deepEqual = require('fast-deep-equal')
 const { flattenNode, compressFlattenedNode, prettyPrintFlattenedNode, prettyPrintRoutesArray } = require('./lib/pretty-print')
 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 +86,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) {
@@ -263,7 +262,7 @@ Router.prototype.reset = function reset () {
   this.routes = []
 }
 
-Router.prototype.off = function off (method, path) {
+Router.prototype.off = function off (method, path, opts) {
   // path validation
   assert(typeof path === 'string', 'Path should be a string')
   assert(path.length > 0, 'The path could not be empty')
@@ -277,8 +276,8 @@ Router.prototype.off = function off (method, path) {
     const pathFull = path.replace(OPTIONAL_PARAM_REGEXP, '$1$2')
     const pathOptional = path.replace(OPTIONAL_PARAM_REGEXP, '$2')
 
-    this.off(method, pathFull)
-    this.off(method, pathOptional)
+    this.off(method, pathFull, opts)
+    this.off(method, pathOptional, opts)
     return
   }
 
@@ -288,17 +287,23 @@ Router.prototype.off = function off (method, path) {
 
   const methods = Array.isArray(method) ? method : [method]
   for (const method of methods) {
-    this._off(method, path)
+    this._off(method, path, opts)
   }
 }
 
-Router.prototype._off = function _off (method, path) {
+Router.prototype._off = function _off (method, path, opts) {
   // method validation
   assert(typeof method === 'string', 'Method should be a string')
   assert(httpMethods.includes(method), `Method '${method}' is not an http method.`)
 
+  function matcher (currentConstraints) {
+    if (!opts || !currentConstraints) return true
+
+    return deepEqual(opts, currentConstraints)
+  }
+
   // Rebuild tree without the specific route
-  const newRoutes = this.routes.filter((route) => method !== route.method || path !== route.path)
+  const newRoutes = this.routes.filter((route) => method !== route.method || path !== route.path || !matcher(route.opts.constraints))
   this.reset()
 
   for (const route of newRoutes) {
@@ -352,7 +357,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 = {}
diff --git a/package.json b/package.json
index 0a62695..eebdd9e 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "find-my-way",
-  "version": "5.5.0",
+  "version": "5.5.1",
   "description": "Crazy fast http radix based router",
   "main": "index.js",
   "types": "index.d.ts",
@@ -38,7 +38,7 @@
     "pre-commit": "^1.2.2",
     "request": "^2.88.2",
     "standard": "^14.3.4",
-    "tap": "^15.0.9",
+    "tap": "^16.0.1",
     "tap-mocha-reporter": "^5.0.1",
     "tsd": "^0.13.1"
   },


Test patch:
diff --git a/test/issue-247.test.js b/test/issue-247.test.js
new file mode 100644
index 0000000..3b23a9e
--- /dev/null
+++ b/test/issue-247.test.js
@@ -0,0 +1,52 @@
+'use strict'
+
+const t = require('tap')
+const test = t.test
+const FindMyWay = require('..')
+
+test('If there are constraints param, router.off method support filter', t => {
+  t.plan(12)
+  const findMyWay = FindMyWay()
+
+  findMyWay.on('GET', '/a', { constraints: { host: '1' } }, () => {}, { name: 1 })
+  findMyWay.on('GET', '/a', { constraints: { host: '2', version: '1.0.0' } }, () => {}, { name: 2 })
+  findMyWay.on('GET', '/a', { constraints: { host: '2', version: '2.0.0' } }, () => {}, { name: 3 })
+
+  t.same(findMyWay.find('GET', '/a', { host: '1' }).store, { name: 1 })
+  t.same(findMyWay.find('GET', '/a', { host: '2', version: '1.0.0' }).store, { name: 2 })
+  t.same(findMyWay.find('GET', '/a', { host: '2', version: '2.0.0' }).store, { name: 3 })
+
+  findMyWay.off('GET', '/a', { host: '1' })
+
+  t.same(findMyWay.find('GET', '/a', { host: '1' }), null)
+  t.same(findMyWay.find('GET', '/a', { host: '2', version: '1.0.0' }).store, { name: 2 })
+  t.same(findMyWay.find('GET', '/a', { host: '2', version: '2.0.0' }).store, { name: 3 })
+
+  findMyWay.off('GET', '/a', { host: '2', version: '1.0.0' })
+
+  t.same(findMyWay.find('GET', '/a', { host: '1' }), null)
+  t.same(findMyWay.find('GET', '/a', { host: '2', version: '1.0.0' }), null)
+  t.same(findMyWay.find('GET', '/a', { host: '2', version: '2.0.0' }).store, { name: 3 })
+
+  findMyWay.off('GET', '/a', { host: '2', version: '2.0.0' })
+
+  t.same(findMyWay.find('GET', '/a', { host: '1' }), null)
+  t.same(findMyWay.find('GET', '/a', { host: '2', version: '1.0.0' }), null)
+  t.same(findMyWay.find('GET', '/a', { host: '2', version: '2.0.0' }), null)
+})
+
+test('If there are no constraints param, router.off method remove all matched router', t => {
+  t.plan(4)
+  const findMyWay = FindMyWay()
+
+  findMyWay.on('GET', '/a', { constraints: { host: '1' } }, () => {}, { name: 1 })
+  findMyWay.on('GET', '/a', { constraints: { host: '2' } }, () => {}, { name: 2 })
+
+  t.same(findMyWay.find('GET', '/a', { host: '1' }).store, { name: 1 })
+  t.same(findMyWay.find('GET', '/a', { host: '2' }).store, { name: 2 })
+
+  findMyWay.off('GET', '/a')
+
+  t.same(findMyWay.find('GET', '/a', { host: '1' }), null)
+  t.same(findMyWay.find('GET', '/a', { host: '2' }), null)
+})
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')
+})