From 023d90568c0b2ad51738959e2de75258a9e72e4b Mon Sep 17 00:00:00 2001 From: Harry Brundage Date: Mon, 1 Feb 2021 11:21:44 -0500 Subject: [PATCH] Ensure constrained routes are matched before unconstrained routes regardless of insertion order --- README.md | 3 ++ node.js | 2 + test/constraint.host.test.js | 18 +------- test/constraints.test.js | 90 ++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 17 deletions(-) create mode 100644 test/constraints.test.js diff --git a/README.md b/README.md index e302d464..f8cf1527 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,7 @@ findMyWay.on('GET', '/', { constraints: { host: 'example.com' } }, (req, res) => Constraints can be combined, and route handlers will only match if __all__ of the constraints for the handler match the request. `find-my-way` does a boolean AND with each route constraint, not an OR. +`find-my-way` will try to match the most constrained handlers first before handler with fewer or no constraints. ### Custom Constraint Strategies @@ -306,6 +307,8 @@ and the URL of the incoming request is /33/foo/bar, the second route will be matched because the first chunk (33) matches the static chunk. If the URL would have been /32/foo/bar, the first route would have been matched. +Once a url has been matched, `find-my-way` will figure out which handler registered for that path matches the request if there are any constraints. `find-my-way` will check the most constrained handlers first, which means the handlers with the most keys in the `constraints` object. + ##### Supported methods The router is able to route all HTTP methods defined by [`http` core module](https://nodejs.org/api/http.html#http_http_methods). diff --git a/node.js b/node.js index 2ab2ece0..d8f9e484 100644 --- a/node.js +++ b/node.js @@ -175,6 +175,8 @@ Node.prototype.addHandler = function (handler, params, store, constraints) { } this.handlers.push(handlerObject) + // Sort the most constrained handlers to the front of the list of handlers so they are tested first. + this.handlers.sort((a, b) => Object.keys(a.constraints).length - Object.keys(b.constraints).length) if (Object.keys(constraints).length > 0) { this.hasConstraints = true diff --git a/test/constraint.host.test.js b/test/constraint.host.test.js index 54de2932..0ce568cc 100644 --- a/test/constraint.host.test.js +++ b/test/constraint.host.test.js @@ -36,22 +36,6 @@ test('A route supports wildcard host constraints', t => { t.notOk(findMyWay.find('GET', '/', { host: 'example.com' })) }) -test('A route could support multiple host constraints while versioned', t => { - t.plan(6) - - const findMyWay = FindMyWay() - - findMyWay.on('GET', '/', { constraints: { host: 'fastify.io', version: '1.1.0' } }, beta) - findMyWay.on('GET', '/', { constraints: { host: 'fastify.io', version: '2.1.0' } }, gamma) - - t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '1.x' }).handler, beta) - t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '1.1.x' }).handler, beta) - t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '2.x' }).handler, gamma) - t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '2.1.x' }).handler, gamma) - t.notOk(findMyWay.find('GET', '/', { host: 'fastify.io', version: '3.x' })) - t.notOk(findMyWay.find('GET', '/', { host: 'something-else.io', version: '1.x' })) -}) - test('A route supports multiple host constraints (lookup)', t => { t.plan(4) @@ -89,4 +73,4 @@ test('A route supports multiple host constraints (lookup)', t => { url: '/', headers: { host: 'bar.fancy.ca' } }) -}) +}) \ No newline at end of file diff --git a/test/constraints.test.js b/test/constraints.test.js new file mode 100644 index 00000000..e1bf5e10 --- /dev/null +++ b/test/constraints.test.js @@ -0,0 +1,90 @@ +'use strict' + +const t = require('tap') +const test = t.test +const FindMyWay = require('..') +const alpha = () => { } +const beta = () => { } +const gamma = () => { } + +test('A route could support multiple host constraints while versioned', t => { + t.plan(6) + + const findMyWay = FindMyWay() + + findMyWay.on('GET', '/', { constraints: { host: 'fastify.io', version: '1.1.0' } }, beta) + findMyWay.on('GET', '/', { constraints: { host: 'fastify.io', version: '2.1.0' } }, gamma) + + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '1.x' }).handler, beta) + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '1.1.x' }).handler, beta) + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '2.x' }).handler, gamma) + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '2.1.x' }).handler, gamma) + t.notOk(findMyWay.find('GET', '/', { host: 'fastify.io', version: '3.x' })) + t.notOk(findMyWay.find('GET', '/', { host: 'something-else.io', version: '1.x' })) +}) + +test('Constrained routes are matched before unconstrainted routes when the constrained route is added last', t => { + t.plan(3) + + const findMyWay = FindMyWay() + + findMyWay.on('GET', '/', {}, alpha) + findMyWay.on('GET', '/', { constraints: { host: 'fastify.io' } }, beta) + + t.strictEqual(findMyWay.find('GET', '/', {}).handler, alpha) + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io' }).handler, beta) + t.strictEqual(findMyWay.find('GET', '/', { host: 'example.com' }).handler, alpha) +}) + +test('Constrained routes are matched before unconstrainted routes when the constrained route is added first', t => { + t.plan(3) + + const findMyWay = FindMyWay() + + findMyWay.on('GET', '/', { constraints: { host: 'fastify.io' } }, beta) + findMyWay.on('GET', '/', {}, alpha) + + t.strictEqual(findMyWay.find('GET', '/', {}).handler, alpha) + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io' }).handler, beta) + t.strictEqual(findMyWay.find('GET', '/', { host: 'example.com' }).handler, alpha) +}) + +test('Routes with multiple constraints are matched before routes with one constraint when the doubly-constrained route is added last', t => { + t.plan(3) + + const findMyWay = FindMyWay() + + findMyWay.on('GET', '/', { constraints: { host: 'fastify.io' } }, alpha) + findMyWay.on('GET', '/', { constraints: { host: 'fastify.io', version: '1.0.0' } }, beta) + + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io' }).handler, alpha) + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '1.0.0' }).handler, beta) + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '2.0.0' }).handler, alpha) +}) + +test('Routes with multiple constraints are matched before routes with one constraint when the doubly-constrained route is added first', t => { + t.plan(3) + + const findMyWay = FindMyWay() + + findMyWay.on('GET', '/', { constraints: { host: 'fastify.io', version: '1.0.0' } }, beta) + findMyWay.on('GET', '/', { constraints: { host: 'fastify.io' } }, alpha) + + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io' }).handler, alpha) + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '1.0.0' }).handler, beta) + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '2.0.0' }).handler, alpha) +}) + +test('Routes with multiple constraints are matched before routes with one constraint before unconstrained routes', t => { + t.plan(3) + + const findMyWay = FindMyWay() + + findMyWay.on('GET', '/', { constraints: { host: 'fastify.io', version: '1.0.0' } }, beta) + findMyWay.on('GET', '/', { constraints: { host: 'fastify.io' } }, alpha) + findMyWay.on('GET', '/', { constraints: {} }, gamma) + + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '1.0.0' }).handler, beta) + t.strictEqual(findMyWay.find('GET', '/', { host: 'fastify.io', version: '2.0.0' }).handler, alpha) + t.strictEqual(findMyWay.find('GET', '/', { host: 'example.io' }).handler, gamma) +})