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

Issue:
Add constraint outside the constructor 
Currently, constraints can only be added when creating the FindMyWay instance. It would be very useful to be able to add more constraints later via a method like ``addConstraint``. With regard to Fastify, this would allow plugins to add constraints without the developer having to specify the constraint itself when creating the Fastify instance.

```js
const router = require('find-my-way')()

router.addConstraint(...)
```

Is this even implementable and if so is it desirable to have such functionality?

Pull request:
Add addConstraintStrategies method
Close #272

Patch:
diff --git a/README.md b/README.md
index 6485d4f..1c7c51e 100644
--- a/README.md
+++ b/README.md
@@ -165,7 +165,9 @@ Constraints can be combined, and route handlers will only match if __all__ of th
 <a name="custom-constraint-strategies"></a>
 ### Custom Constraint Strategies
 
-Custom constraining strategies can be added and are matched against incoming requests while trying to maintain `find-my-way`'s high performance. To register a new type of constraint, you must add a new constraint strategy that knows how to match values to handlers, and that knows how to get the constraint value from a request. Register strategies when constructing a router:
+Custom constraining strategies can be added and are matched against incoming requests while trying to maintain `find-my-way`'s high performance. To register a new type of constraint, you must add a new constraint strategy that knows how to match values to handlers, and that knows how to get the constraint value from a request. Register strategies when constructing a router or use the addCustomConstraintStrategy method.
+
+Add a custom constrain strategy when constructing a router:
 
 ```js
 const customResponseTypeStrategy = {
@@ -190,6 +192,32 @@ const customResponseTypeStrategy = {
 const router = FindMyWay({ constraints: { accept: customResponseTypeStrategy } });
 ```
 
+Add a custom constraint strategy using the addCustomConstraintStrategy method:
+
+```js
+const customResponseTypeStrategy = {
+  // strategy name for referencing in the route handler `constraints` options
+  name: 'accept',
+  // storage factory for storing routes in the find-my-way route tree
+  storage: function () {
+    let handlers = {}
+    return {
+      get: (type) => { return handlers[type] || null },
+      set: (type, store) => { handlers[type] = store }
+    }
+  },
+  // function to get the value of the constraint from each incoming request
+  deriveConstraint: (req, ctx) => {
+    return req.headers['accept']
+  },
+  // optional flag marking if handlers without constraints can match requests that have a value for this constraint
+  mustMatchWhenDerived: true
+}
+
+const router = FindMyWay();
+router.addCustomConstraintStrategy(customResponseTypeStrategy);
+```
+
 Once a custom constraint strategy is registered, routes can be added that are constrained using it:
 
 
diff --git a/index.d.ts b/index.d.ts
index 03a83ed..7f01d53 100644
--- a/index.d.ts
+++ b/index.d.ts
@@ -139,7 +139,6 @@ declare namespace Router {
       handler: Handler<V>,
       store: any
     ): void;
-
     off(
       method: HTTPMethod | HTTPMethod[],
       path: string,
@@ -162,6 +161,9 @@ declare namespace Router {
     prettyPrint(): string;
     prettyPrint(opts: { commonPrefix?: boolean, includeMeta?: boolean | (string | symbol)[]  }): string;
 
+    hasCustomConstraintStrategy(strategyName: string): boolean;
+    addCustomConstraintStrategy(constraintStrategy: ConstraintStrategy<V>): void;
+
     all: ShortHandRoute<V>;
 
     acl: ShortHandRoute<V>;
diff --git a/index.js b/index.js
index 63c7062..1aad974 100644
--- a/index.js
+++ b/index.js
@@ -266,6 +266,15 @@ Router.prototype._on = function _on (method, path, opts, handler, store) {
   currentNode.handlerStorage.addHandler(handler, params, store, this.constrainer, constraints)
 }
 
+Router.prototype.hasCustomConstraintStrategy = function (strategyName) {
+  return this.constrainer.hasCustomConstraintStrategy(strategyName)
+}
+
+Router.prototype.addCustomConstraintStrategy = function (constraints) {
+  this.constrainer.addCustomConstraintStrategy(constraints)
+  this._rebuild(this.routes)
+}
+
 Router.prototype.reset = function reset () {
   this.trees = {}
   this.routes = []
@@ -313,13 +322,7 @@ Router.prototype._off = function _off (method, path, opts) {
 
   // Rebuild tree without the specific route
   const newRoutes = this.routes.filter((route) => method !== route.method || path !== route.path || !matcher(route.opts.constraints))
-  this.reset()
-
-  for (const route of newRoutes) {
-    const { method, path, opts, handler, store } = route
-    this._on(method, path, opts, handler, store)
-    this.routes.push({ method, path, opts, handler, store })
-  }
+  this._rebuild(newRoutes)
 }
 
 Router.prototype.lookup = function lookup (req, res, ctx) {
@@ -500,6 +503,16 @@ Router.prototype.find = function find (method, path, derivedConstraints) {
   }
 }
 
+Router.prototype._rebuild = function (routes) {
+  this.reset()
+
+  for (const route of routes) {
+    const { method, path, opts, handler, store } = route
+    this._on(method, path, opts, handler, store)
+    this.routes.push({ method, path, opts, handler, store })
+  }
+}
+
 Router.prototype._defaultRoute = function (req, res, ctx) {
   if (this.defaultRoute !== null) {
     return ctx === undefined
diff --git a/lib/constrainer.js b/lib/constrainer.js
index a0cef68..79f84c0 100644
--- a/lib/constrainer.js
+++ b/lib/constrainer.js
@@ -15,23 +15,38 @@ class Constrainer {
 
     // validate and optimize prototypes of given custom strategies
     if (customStrategies) {
-      var kCustomStrategies = Object.keys(customStrategies)
-      var strategy
-      for (var i = 0; i < kCustomStrategies.length; i++) {
-        strategy = customStrategies[kCustomStrategies[i]]
-        assert(typeof strategy.name === 'string' && strategy.name !== '', 'strategy.name is required.')
-        assert(strategy.storage && typeof strategy.storage === 'function', 'strategy.storage function is required.')
-        assert(strategy.deriveConstraint && typeof strategy.deriveConstraint === 'function', 'strategy.deriveConstraint function is required.')
-        strategy.isCustom = true
-        this.strategies[strategy.name] = strategy
-
-        if (strategy.mustMatchWhenDerived) {
-          this.noteUsage({ [kCustomStrategies[i]]: strategy })
-        }
+      for (const strategy of Object.values(customStrategies)) {
+        this.addCustomConstraintStrategy(strategy)
       }
     }
   }
 
+  hasCustomConstraintStrategy (strategyName) {
+    const customConstraintStrategy = this.strategies[strategyName]
+    return customConstraintStrategy !== undefined && customConstraintStrategy.isCustom === true
+  }
+
+  addCustomConstraintStrategy (strategy) {
+    assert(typeof strategy.name === 'string' && strategy.name !== '', 'strategy.name is required.')
+    assert(strategy.storage && typeof strategy.storage === 'function', 'strategy.storage function is required.')
+    assert(strategy.deriveConstraint && typeof strategy.deriveConstraint === 'function', 'strategy.deriveConstraint function is required.')
+
+    if (this.hasCustomConstraintStrategy(strategy.name)) {
+      throw new Error(`There is already exists a custom constraint with the name ${strategy.name}.`)
+    }
+
+    if (this.strategiesInUse.has(strategy.name)) {
+      throw new Error(`There is already exists a route with ${strategy.name} constraint.`)
+    }
+
+    strategy.isCustom = true
+    this.strategies[strategy.name] = strategy
+
+    if (strategy.mustMatchWhenDerived) {
+      this.noteUsage({ [strategy.name]: strategy })
+    }
+  }
+
   deriveConstraints (req, ctx) {
     return undefined
   }


Test patch:
diff --git a/test/constraint.custom-versioning.test.js b/test/constraint.custom-versioning.test.js
index 10b237f..67caf4a 100644
--- a/test/constraint.custom-versioning.test.js
+++ b/test/constraint.custom-versioning.test.js
@@ -37,6 +37,23 @@ test('A route could support multiple versions (find) / 1', t => {
   t.notOk(findMyWay.find('GET', '/', { version: 'application/vnd.example.api+json;version=6' }))
 })
 
+test('A route could support multiple versions (find) / 1 (add strategy outside constructor)', t => {
+  t.plan(5)
+
+  const findMyWay = FindMyWay()
+
+  findMyWay.addCustomConstraintStrategy(customVersioning)
+
+  findMyWay.on('GET', '/', { constraints: { version: 'application/vnd.example.api+json;version=2' } }, noop)
+  findMyWay.on('GET', '/', { constraints: { version: 'application/vnd.example.api+json;version=3' } }, noop)
+
+  t.ok(findMyWay.find('GET', '/', { version: 'application/vnd.example.api+json;version=2' }))
+  t.ok(findMyWay.find('GET', '/', { version: 'application/vnd.example.api+json;version=3' }))
+  t.notOk(findMyWay.find('GET', '/', { version: 'application/vnd.example.api+json;version=4' }))
+  t.notOk(findMyWay.find('GET', '/', { version: 'application/vnd.example.api+json;version=5' }))
+  t.notOk(findMyWay.find('GET', '/', { version: 'application/vnd.example.api+json;version=6' }))
+})
+
 test('Overriding default strategies uses the custom deriveConstraint function', t => {
   t.plan(2)
 
@@ -61,3 +78,42 @@ test('Overriding default strategies uses the custom deriveConstraint function',
     headers: { accept: 'application/vnd.example.api+json;version=3' }
   })
 })
+
+test('Overriding default strategies uses the custom deriveConstraint function (add strategy outside constructor)', t => {
+  t.plan(2)
+
+  const findMyWay = FindMyWay()
+
+  findMyWay.addCustomConstraintStrategy(customVersioning)
+
+  findMyWay.on('GET', '/', { constraints: { version: 'application/vnd.example.api+json;version=2' } }, (req, res, params) => {
+    t.equal(req.headers.accept, 'application/vnd.example.api+json;version=2')
+  })
+
+  findMyWay.on('GET', '/', { constraints: { version: 'application/vnd.example.api+json;version=3' } }, (req, res, params) => {
+    t.equal(req.headers.accept, 'application/vnd.example.api+json;version=3')
+  })
+
+  findMyWay.lookup({
+    method: 'GET',
+    url: '/',
+    headers: { accept: 'application/vnd.example.api+json;version=2' }
+  })
+  findMyWay.lookup({
+    method: 'GET',
+    url: '/',
+    headers: { accept: 'application/vnd.example.api+json;version=3' }
+  })
+})
+
+test('Overriding custom strategies throws as error (add strategy outside constructor)', t => {
+  t.plan(1)
+
+  const findMyWay = FindMyWay()
+
+  findMyWay.addCustomConstraintStrategy(customVersioning)
+
+  t.throws(() => findMyWay.addCustomConstraintStrategy(customVersioning),
+    'There is already exists a custom constraint with the name version.'
+  )
+})
diff --git a/test/constraint.custom.test.js b/test/constraint.custom.test.js
index 87b800a..9a0567f 100644
--- a/test/constraint.custom.test.js
+++ b/test/constraint.custom.test.js
@@ -37,6 +37,21 @@ test('A route could support a custom constraint strategy', t => {
   t.notOk(findMyWay.find('GET', '/', { requestedBy: 'chrome' }))
 })
 
+test('A route could support a custom constraint strategy (add strategy outside constructor)', t => {
+  t.plan(3)
+
+  const findMyWay = FindMyWay()
+
+  findMyWay.addCustomConstraintStrategy(customHeaderConstraint)
+
+  findMyWay.on('GET', '/', { constraints: { requestedBy: 'curl' } }, alpha)
+  findMyWay.on('GET', '/', { constraints: { requestedBy: 'wget' } }, beta)
+
+  t.equal(findMyWay.find('GET', '/', { requestedBy: 'curl' }).handler, alpha)
+  t.equal(findMyWay.find('GET', '/', { requestedBy: 'wget' }).handler, beta)
+  t.notOk(findMyWay.find('GET', '/', { requestedBy: 'chrome' }))
+})
+
 test('A route could support a custom constraint strategy while versioned', t => {
   t.plan(8)
 
@@ -59,6 +74,30 @@ test('A route could support a custom constraint strategy while versioned', t =>
   t.notOk(findMyWay.find('GET', '/', { requestedBy: 'wget', version: '1.x' }))
 })
 
+test('A route could support a custom constraint strategy while versioned (add strategy outside constructor)', t => {
+  t.plan(8)
+
+  const findMyWay = FindMyWay()
+
+  findMyWay.addCustomConstraintStrategy(customHeaderConstraint)
+
+  findMyWay.on('GET', '/', { constraints: { requestedBy: 'curl', version: '1.0.0' } }, alpha)
+  findMyWay.on('GET', '/', { constraints: { requestedBy: 'curl', version: '2.0.0' } }, beta)
+  findMyWay.on('GET', '/', { constraints: { requestedBy: 'wget', version: '2.0.0' } }, gamma)
+  findMyWay.on('GET', '/', { constraints: { requestedBy: 'wget', version: '3.0.0' } }, delta)
+
+  t.equal(findMyWay.find('GET', '/', { requestedBy: 'curl', version: '1.x' }).handler, alpha)
+  t.equal(findMyWay.find('GET', '/', { requestedBy: 'curl', version: '2.x' }).handler, beta)
+  t.equal(findMyWay.find('GET', '/', { requestedBy: 'wget', version: '2.x' }).handler, gamma)
+  t.equal(findMyWay.find('GET', '/', { requestedBy: 'wget', version: '3.x' }).handler, delta)
+
+  t.notOk(findMyWay.find('GET', '/', { requestedBy: 'chrome' }))
+  t.notOk(findMyWay.find('GET', '/', { requestedBy: 'chrome', version: '1.x' }))
+
+  t.notOk(findMyWay.find('GET', '/', { requestedBy: 'curl', version: '3.x' }))
+  t.notOk(findMyWay.find('GET', '/', { requestedBy: 'wget', version: '1.x' }))
+})
+
 test('A route could support a custom constraint strategy while versioned and host constrained', t => {
   t.plan(9)
 
@@ -80,6 +119,29 @@ test('A route could support a custom constraint strategy while versioned and hos
   t.notOk(findMyWay.find('GET', '/', { requestedBy: 'curl', version: '1.x', host: 'example.io' }))
 })
 
+test('A route could support a custom constraint strategy while versioned and host constrained (add strategy outside constructor)', t => {
+  t.plan(9)
+
+  const findMyWay = FindMyWay()
+
+  findMyWay.addCustomConstraintStrategy(customHeaderConstraint)
+
+  findMyWay.on('GET', '/', { constraints: { requestedBy: 'curl', version: '1.0.0', host: 'fastify.io' } }, alpha)
+  findMyWay.on('GET', '/', { constraints: { requestedBy: 'curl', version: '2.0.0', host: 'fastify.io' } }, beta)
+  findMyWay.on('GET', '/', { constraints: { requestedBy: 'curl', version: '2.0.0', host: 'example.io' } }, delta)
+
+  t.equal(findMyWay.find('GET', '/', { requestedBy: 'curl', version: '1.x', host: 'fastify.io' }).handler, alpha)
+  t.equal(findMyWay.find('GET', '/', { requestedBy: 'curl', version: '2.x', host: 'fastify.io' }).handler, beta)
+  t.equal(findMyWay.find('GET', '/', { requestedBy: 'curl', version: '2.x', host: 'example.io' }).handler, delta)
+
+  t.notOk(findMyWay.find('GET', '/', { requestedBy: 'chrome' }))
+  t.notOk(findMyWay.find('GET', '/', { requestedBy: 'chrome', version: '1.x' }))
+  t.notOk(findMyWay.find('GET', '/', { requestedBy: 'curl', version: '1.x' }))
+  t.notOk(findMyWay.find('GET', '/', { requestedBy: 'curl', version: '2.x' }))
+  t.notOk(findMyWay.find('GET', '/', { requestedBy: 'curl', version: '3.x', host: 'fastify.io' }))
+  t.notOk(findMyWay.find('GET', '/', { requestedBy: 'curl', version: '1.x', host: 'example.io' }))
+})
+
 test('Custom constraint strategies can set mustMatchWhenDerived flag to true which prevents matches to unconstrained routes when a constraint is derived and there are no other routes', t => {
   t.plan(1)
 
@@ -100,6 +162,25 @@ test('Custom constraint strategies can set mustMatchWhenDerived flag to true whi
   findMyWay.lookup({ method: 'GET', url: '/', headers: { 'user-agent': 'node' } }, null)
 })
 
+test('Custom constraint strategies can set mustMatchWhenDerived flag to true which prevents matches to unconstrained routes when a constraint is derived and there are no other routes (add strategy outside constructor)', t => {
+  t.plan(1)
+
+  const findMyWay = FindMyWay({
+    defaultRoute (req, res) {
+      t.pass()
+    }
+  })
+
+  findMyWay.addCustomConstraintStrategy({
+    ...customHeaderConstraint,
+    mustMatchWhenDerived: true
+  })
+
+  findMyWay.on('GET', '/', {}, () => t.fail())
+
+  findMyWay.lookup({ method: 'GET', url: '/', headers: { 'user-agent': 'node' } }, null)
+})
+
 test('Custom constraint strategies can set mustMatchWhenDerived flag to true which prevents matches to unconstrained routes when a constraint is derived when there are constrained routes', t => {
   t.plan(1)
 
@@ -122,6 +203,27 @@ test('Custom constraint strategies can set mustMatchWhenDerived flag to true whi
   findMyWay.lookup({ method: 'GET', url: '/', headers: { 'user-agent': 'node' } }, null)
 })
 
+test('Custom constraint strategies can set mustMatchWhenDerived flag to true which prevents matches to unconstrained routes when a constraint is derived when there are constrained routes (add strategy outside constructor)', t => {
+  t.plan(1)
+
+  const findMyWay = FindMyWay({
+    defaultRoute (req, res) {
+      t.pass()
+    }
+  })
+
+  findMyWay.addCustomConstraintStrategy({
+    ...customHeaderConstraint,
+    mustMatchWhenDerived: true
+  })
+
+  findMyWay.on('GET', '/', {}, () => t.fail())
+  findMyWay.on('GET', '/', { constraints: { requestedBy: 'curl' } }, () => t.fail())
+  findMyWay.on('GET', '/', { constraints: { requestedBy: 'wget' } }, () => t.fail())
+
+  findMyWay.lookup({ method: 'GET', url: '/', headers: { 'user-agent': 'node' } }, null)
+})
+
 test('Custom constraint strategies can set mustMatchWhenDerived flag to false which allows matches to unconstrained routes when a constraint is derived', t => {
   t.plan(1)
 
@@ -141,3 +243,35 @@ test('Custom constraint strategies can set mustMatchWhenDerived flag to false wh
 
   findMyWay.lookup({ method: 'GET', url: '/', headers: { 'user-agent': 'node' } }, null)
 })
+
+test('Custom constraint strategies can set mustMatchWhenDerived flag to false which allows matches to unconstrained routes when a constraint is derived (add strategy outside constructor)', t => {
+  t.plan(1)
+
+  const findMyWay = FindMyWay({
+    defaultRoute (req, res) {
+      t.pass()
+    }
+  })
+
+  findMyWay.addCustomConstraintStrategy({
+    ...customHeaderConstraint,
+    mustMatchWhenDerived: true
+  })
+
+  findMyWay.on('GET', '/', {}, () => t.pass())
+
+  findMyWay.lookup({ method: 'GET', url: '/', headers: { 'user-agent': 'node' } }, null)
+})
+
+test('Has constraint strategy method test', t => {
+  t.plan(1)
+
+  const findMyWay = FindMyWay({
+    defaultRoute (req, res) {
+      t.pass()
+    }
+  })
+
+  findMyWay.addCustomConstraintStrategy(customHeaderConstraint)
+  t.same(findMyWay.hasCustomConstraintStrategy(customHeaderConstraint.name), true)
+})
diff --git a/test/constraints.test.js b/test/constraints.test.js
index 02c8215..85cdad2 100644
--- a/test/constraints.test.js
+++ b/test/constraints.test.js
@@ -88,3 +88,12 @@ test('Routes with multiple constraints are matched before routes with one constr
   t.equal(findMyWay.find('GET', '/', { host: 'fastify.io', version: '2.0.0' }), null)
   t.equal(findMyWay.find('GET', '/', { host: 'example.io' }).handler, gamma)
 })
+
+test('Has constraint strategy method test', t => {
+  t.plan(2)
+
+  const findMyWay = FindMyWay()
+
+  t.same(findMyWay.hasCustomConstraintStrategy('version'), false)
+  t.same(findMyWay.hasCustomConstraintStrategy('host'), false)
+})
diff --git a/test/types/router.test-d.ts b/test/types/router.test-d.ts
index c2ad8b1..e6ce153 100644
--- a/test/types/router.test-d.ts
+++ b/test/types/router.test-d.ts
@@ -66,6 +66,23 @@ let http2Res!: Http2ServerResponse;
 
 // HTTP2
 {
+  const constraints: { [key: string]: Router.ConstraintStrategy<Router.HTTPVersion.V2, string> } = {
+    foo: {
+      name: 'foo',
+      mustMatchWhenDerived: true,
+      storage () {
+        return {
+          get (version) { return handler },
+          set (version, handler) {},
+          del (version) {},
+          empty () {}
+        }
+      },
+      deriveConstraint(req) { return '1.0.0' },
+      validate(value) { if (typeof value === "string") { throw new Error("invalid")} }
+    }
+  }
+
   let handler: Router.Handler<Router.HTTPVersion.V2>
   const router = Router<Router.HTTPVersion.V2>({
     ignoreTrailingSlash: true,
@@ -74,22 +91,7 @@ let http2Res!: Http2ServerResponse;
     maxParamLength: 42,
     defaultRoute (http1Req, http1Res) {},
     onBadUrl (path, http1Req, http1Res) {},
-    constraints: {
-      foo: {
-        name: 'foo',
-        mustMatchWhenDerived: true,
-        storage () {
-          return {
-            get (version) { return handler },
-            set (version, handler) {},
-            del (version) {},
-            empty () {}
-          }
-        },
-        deriveConstraint(req) { return '1.0.0' },
-        validate(value) { if (typeof value === "string") { throw new Error("invalid")} }
-      }
-    }
+    constraints
   })
   expectType<Router.Instance<Router.HTTPVersion.V2>>(router)
 
@@ -99,6 +101,8 @@ let http2Res!: Http2ServerResponse;
   expectType<void>(router.on('GET', '/', () => {}, {}))
   expectType<void>(router.on('GET', '/', { constraints: { version: '1.0.0' }}, () => {}, {}))
 
+  expectType<void>(router.addCustomConstraintStrategy(constraints.foo))
+
   expectType<void>(router.get('/', () => {}))
   expectType<void>(router.get('/', { constraints: { version: '1.0.0' }}, () => {}))
   expectType<void>(router.get('/', () => {}, {}))
@@ -140,6 +144,7 @@ let http2Res!: Http2ServerResponse;
       }
     }
   }
+
   const storageWithObject = customConstraintWithObject.storage()
   const acceptAndContentType: AcceptAndContentType = { accept: 'application/json', contentType: 'application/xml' }