Repository: delvedor/find-my-way
Pull request #305 (2022-09-25 11:48:52)

feat: add async custom constraints support
Close #303

mcollina: Can you add docs?

Patch:
diff --git a/README.md b/README.md
index 0cf19c9..c79a0b6 100644
--- a/README.md
+++ b/README.md
@@ -22,7 +22,7 @@ Do you need a real-world example that uses this router? Check out [Fastify](http
   - [off(method, path)](#offmethod-path)
     - [off(methods[], path)](#offmethods-path)
     - [off(methods, path, [constraints])](#offmethods-path-constraints)
-  - [lookup(request, response, [context])](#lookuprequest-response-context)
+  - [lookup(request, response, [context], [done])](#lookuprequest-response-context)
   - [find(method, path, [constraints])](#findmethod-path-constraints)
   - [prettyPrint([{ commonPrefix: false, includeMeta: true || [] }])](#prettyprint-commonprefix-false-includemeta-true---)
   - [reset()](#reset)
@@ -378,7 +378,7 @@ router.off(['POST', 'GET'], '/example', { host: 'fastify.io' })
 router.off(['POST', 'GET'], '/example', {})
 ```
 
-#### lookup(request, response, [context])
+#### lookup(request, response, [context], [done])
 Start a new search, `request` and `response` are the server req/res objects.<br>
 If a route is found it will automatically call the handler, otherwise the default route will be called.<br>
 The url is sanitized internally, all the parameters and wildcards are decoded automatically.
@@ -394,6 +394,19 @@ router.on('GET', '*', function(req, res) {
 router.lookup(req, res, { greeting: 'Hello, World!' })
 ```
 
+`lookup` accepts an optional `done` callback for case when you use an async `deriveConstraint` function.
+```js
+router.on('GET', '*', function(req, res) {
+  res.end({ hello: 'world' });
+})
+router.lookup(req, res, (err) => {
+  if (err !== null) {
+    // handle error
+  }
+  console.log('Handler executed!!!'); 
+})
+```
+
 <a name="find"></a>
 #### find(method, path, [constraints])
 Return (if present) the route registered in *method:path*.<br>
@@ -602,7 +615,30 @@ const router = FindMyWay({ constraints: { accept: customResponseTypeStrategy } }
 ```
 
 Add a custom constraint strategy using the addConstraintStrategy method:
+```js
+const asyncCustomResponseTypeStrategy = {
+  // 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, done) => {
+    done(null, 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({ constraints: { accept: asyncCustomResponseTypeStrategy } });
+```
 
+Add an async custom constrain strategy when constructing a router:
 ```js
 const customResponseTypeStrategy = {
   // strategy name for referencing in the route handler `constraints` options
diff --git a/index.js b/index.js
index 32fd008..d60d648 100644
--- a/index.js
+++ b/index.js
@@ -267,6 +267,10 @@ Router.prototype._on = function _on (method, path, opts, handler, store) {
       params.push('*')
       currentNode = currentNode.createWildcardChild()
       parentNodePathIndex = i + 1
+
+      if (i !== path.length - 1) {
+        throw new Error('Wildcard must be the last character in the route')
+      }
     }
   }
 
@@ -370,8 +374,35 @@ Router.prototype._off = function _off (method, path, constraints) {
   this._rebuild(newRoutes)
 }
 
-Router.prototype.lookup = function lookup (req, res, ctx) {
-  var handle = this.find(req.method, req.url, this.constrainer.deriveConstraints(req, ctx))
+Router.prototype.lookup = function lookup (req, res, ctx, done) {
+  if (typeof ctx === 'function') {
+    done = ctx
+    ctx = undefined
+  }
+
+  if (done === undefined) {
+    const constraints = this.constrainer.deriveConstraints(req, ctx)
+    const handle = this.find(req.method, req.url, constraints)
+    return this.callHandler(handle, req, res, ctx)
+  }
+
+  this.constrainer.deriveConstraints(req, ctx, (err, constraints) => {
+    if (err !== null) {
+      done(err)
+      return
+    }
+
+    try {
+      const handle = this.find(req.method, req.url, constraints)
+      const result = this.callHandler(handle, req, res, ctx)
+      done(null, result)
+    } catch (err) {
+      done(err)
+    }
+  })
+}
+
+Router.prototype.callHandler = function callHandler (handle, req, res, ctx) {
   if (handle === null) return this._defaultRoute(req, res, ctx)
   return ctx === undefined
     ? handle.handler(req, res, handle.params, handle.store, handle.searchParams)
diff --git a/lib/constrainer.js b/lib/constrainer.js
index 947fe3d..b605ab2 100644
--- a/lib/constrainer.js
+++ b/lib/constrainer.js
@@ -12,6 +12,7 @@ class Constrainer {
     }
 
     this.strategiesInUse = new Set()
+    this.asyncStrategiesInUse = new Set()
 
     // validate and optimize prototypes of given custom strategies
     if (customStrategies) {
@@ -21,10 +22,16 @@ class Constrainer {
     }
   }
 
+  isStrategyUsed (strategyName) {
+    return this.strategiesInUse.has(strategyName) ||
+      this.asyncStrategiesInUse.has(strategyName)
+  }
+
   hasConstraintStrategy (strategyName) {
     const customConstraintStrategy = this.strategies[strategyName]
     if (customConstraintStrategy !== undefined) {
-      return customConstraintStrategy.isCustom || this.strategiesInUse.has(strategyName)
+      return customConstraintStrategy.isCustom ||
+        this.isStrategyUsed(strategyName)
     }
     return false
   }
@@ -38,11 +45,12 @@ class Constrainer {
       throw new Error(`There already exists a custom constraint with the name ${strategy.name}.`)
     }
 
-    if (this.strategiesInUse.has(strategy.name)) {
+    if (this.isStrategyUsed(strategy.name)) {
       throw new Error(`There already exists a route with ${strategy.name} constraint.`)
     }
 
     strategy.isCustom = true
+    strategy.isAsync = strategy.deriveConstraint.length === 3
     this.strategies[strategy.name] = strategy
 
     if (strategy.mustMatchWhenDerived) {
@@ -50,7 +58,17 @@ class Constrainer {
     }
   }
 
-  deriveConstraints (req, ctx) {
+  deriveConstraints (req, ctx, done) {
+    const constraints = this.deriveSyncConstraints(req, ctx)
+
+    if (done === undefined) {
+      return constraints
+    }
+
+    this.deriveAsyncConstraints(constraints, req, ctx, done)
+  }
+
+  deriveSyncConstraints (req, ctx) {
     return undefined
   }
 
@@ -59,7 +77,12 @@ class Constrainer {
     if (constraints) {
       const beforeSize = this.strategiesInUse.size
       for (const key in constraints) {
-        this.strategiesInUse.add(key)
+        const strategy = this.strategies[key]
+        if (strategy.isAsync) {
+          this.asyncStrategiesInUse.add(key)
+        } else {
+          this.strategiesInUse.add(key)
+        }
       }
       if (beforeSize !== this.strategiesInUse.size) {
         this._buildDeriveConstraints()
@@ -90,6 +113,32 @@ class Constrainer {
     }
   }
 
+  deriveAsyncConstraints (constraints, req, ctx, done) {
+    let asyncConstraintsCount = this.asyncStrategiesInUse.size
+
+    if (asyncConstraintsCount === 0) {
+      done(null, constraints)
+      return
+    }
+
+    constraints = constraints || {}
+    for (const key of this.asyncStrategiesInUse) {
+      const strategy = this.strategies[key]
+      strategy.deriveConstraint(req, ctx, (err, constraintValue) => {
+        if (err !== null) {
+          done(err)
+          return
+        }
+
+        constraints[key] = constraintValue
+
+        if (--asyncConstraintsCount === 0) {
+          done(null, constraints)
+        }
+      })
+    }
+  }
+
   // Optimization: build a fast function for deriving the constraints for all the strategies at once. We inline the definitions of the version constraint and the host constraint for performance.
   // If no constraining strategies are in use (no routes constrain on host, or version, or any custom strategies) then we don't need to derive constraints for each route match, so don't do anything special, and just return undefined
   // This allows us to not allocate an object to hold constraint values if no constraints are defined.
@@ -116,7 +165,7 @@ class Constrainer {
 
     lines.push('}')
 
-    this.deriveConstraints = new Function('req', 'ctx', lines.join('\n')).bind(this) // eslint-disable-line
+    this.deriveSyncConstraints = new Function('req', 'ctx', lines.join('\n')).bind(this) // eslint-disable-line
   }
 }
 
diff --git a/package.json b/package.json
index aa57973..2c62b00 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "find-my-way",
-  "version": "7.2.0",
+  "version": "7.2.1",
   "description": "Crazy fast http radix based router",
   "main": "index.js",
   "types": "index.d.ts",


Test patch:
diff --git a/test/constraint.custom.async.test.js b/test/constraint.custom.async.test.js
new file mode 100644
index 0000000..3aef949
--- /dev/null
+++ b/test/constraint.custom.async.test.js
@@ -0,0 +1,108 @@
+'use strict'
+
+const t = require('tap')
+const test = t.test
+const FindMyWay = require('..')
+
+const customHeaderConstraint = {
+  name: 'requestedBy',
+  storage: function () {
+    const requestedBys = {}
+    return {
+      get: (requestedBy) => { return requestedBys[requestedBy] || null },
+      set: (requestedBy, store) => { requestedBys[requestedBy] = store }
+    }
+  },
+  deriveConstraint: (req, ctx, done) => {
+    if (req.headers['user-agent'] === 'wrong') {
+      done(new Error('wrong user-agent'))
+      return
+    }
+
+    done(null, req.headers['user-agent'])
+  }
+}
+
+test('should derive async constraint', t => {
+  t.plan(2)
+
+  const router = FindMyWay({ constraints: { requestedBy: customHeaderConstraint } })
+  router.on('GET', '/', { constraints: { requestedBy: 'node' } }, () => 'asyncHandler')
+
+  router.lookup(
+    {
+      method: 'GET',
+      url: '/',
+      headers: {
+        'user-agent': 'node'
+      }
+    },
+    null,
+    (err, result) => {
+      t.equal(err, null)
+      t.equal(result, 'asyncHandler')
+    }
+  )
+})
+
+test('lookup should return an error from deriveConstraint', t => {
+  t.plan(2)
+
+  const router = FindMyWay({ constraints: { requestedBy: customHeaderConstraint } })
+  router.on('GET', '/', { constraints: { requestedBy: 'node' } }, () => 'asyncHandler')
+
+  router.lookup(
+    {
+      method: 'GET',
+      url: '/',
+      headers: {
+        'user-agent': 'wrong'
+      }
+    },
+    null,
+    (err, result) => {
+      t.same(err, new Error('wrong user-agent'))
+      t.equal(result, undefined)
+    }
+  )
+})
+
+test('should derive sync and async constraints', t => {
+  t.plan(4)
+
+  const router = FindMyWay({ constraints: { requestedBy: customHeaderConstraint } })
+  router.on('GET', '/', { constraints: { version: '1.0.0', requestedBy: 'node' } }, () => 'asyncHandlerV1')
+  router.on('GET', '/', { constraints: { version: '2.0.0', requestedBy: 'node' } }, () => 'asyncHandlerV2')
+
+  router.lookup(
+    {
+      method: 'GET',
+      url: '/',
+      headers: {
+        'user-agent': 'node',
+        'accept-version': '1.0.0'
+      }
+    },
+    null,
+    (err, result) => {
+      t.equal(err, null)
+      t.equal(result, 'asyncHandlerV1')
+    }
+  )
+
+  router.lookup(
+    {
+      method: 'GET',
+      url: '/',
+      headers: {
+        'user-agent': 'node',
+        'accept-version': '2.0.0'
+      }
+    },
+    null,
+    (err, result) => {
+      t.equal(err, null)
+      t.equal(result, 'asyncHandlerV2')
+    }
+  )
+})
diff --git a/test/issue-28.test.js b/test/issue-28.test.js
index fb901e6..c25d3ef 100644
--- a/test/issue-28.test.js
+++ b/test/issue-28.test.js
@@ -601,3 +601,19 @@ test('Wildcard node with constraints', t => {
     null
   )
 })
+
+test('Wildcard must be the last character in the route', (t) => {
+  t.plan(6)
+
+  const expectedError = new Error('Wildcard must be the last character in the route')
+
+  const findMyWay = FindMyWay()
+
+  t.throws(() => findMyWay.on('GET', '*1', () => {}), expectedError)
+  t.throws(() => findMyWay.on('GET', '*/', () => {}), expectedError)
+  t.throws(() => findMyWay.on('GET', '*?', () => {}), expectedError)
+
+  t.throws(() => findMyWay.on('GET', '/foo*123', () => {}), expectedError)
+  t.throws(() => findMyWay.on('GET', '/foo*?', () => {}), expectedError)
+  t.throws(() => findMyWay.on('GET', '/foo*/', () => {}), expectedError)
+})
diff --git a/test/lookup-async.test.js b/test/lookup-async.test.js
new file mode 100644
index 0000000..a56203e
--- /dev/null
+++ b/test/lookup-async.test.js
@@ -0,0 +1,30 @@
+'use strict'
+
+const t = require('tap')
+const test = t.test
+const FindMyWay = require('..')
+
+test('should return result in the done callback', t => {
+  t.plan(2)
+
+  const router = FindMyWay()
+  router.on('GET', '/', () => 'asyncHandlerResult')
+
+  router.lookup({ method: 'GET', url: '/' }, null, (err, result) => {
+    t.equal(err, null)
+    t.equal(result, 'asyncHandlerResult')
+  })
+})
+
+test('should return an error in the done callback', t => {
+  t.plan(2)
+
+  const router = FindMyWay()
+  const error = new Error('ASYNC_HANDLER_ERROR')
+  router.on('GET', '/', () => { throw error })
+
+  router.lookup({ method: 'GET', url: '/' }, null, (err, result) => {
+    t.equal(err, error)
+    t.equal(result, undefined)
+  })
+})