Skip to content

Commit

Permalink
Use longest match for ctx._matchedRoute
Browse files Browse the repository at this point in the history
Use most recently added layer as a tiebreaker
  • Loading branch information
jcao219 authored and Jimmy Cao committed Jan 12, 2019
1 parent 590df78 commit 1ad877d
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 13 deletions.
22 changes: 16 additions & 6 deletions lib/layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ function Layer(path, methods, middleware, opts) {
}, this);

this.path = path;
this.plainPath = path instanceof RegExp ? path.source :
path.replace(/\(\.\*\)/g, '');

this.regexp = pathToRegExp(path, this.paramNames, this.opts);
this.regexp = new RegExp(this.regexp.source, 'gi');

debug('defined route %s %s', this.methods, this.opts.prefix + this.path);
};
Expand All @@ -57,8 +61,11 @@ function Layer(path, methods, middleware, opts) {
* @private
*/

Layer.prototype.match = function (path) {
return this.regexp.test(path);
Layer.prototype.matchLength = function (path) {
var result = this.regexp.test(path);
var lastIndex = this.regexp.lastIndex;
this.regexp.lastIndex = 0;
return result ? lastIndex : -1;
};

/**
Expand Down Expand Up @@ -94,7 +101,9 @@ Layer.prototype.params = function (path, captures, existingParams) {

Layer.prototype.captures = function (path) {
if (this.opts.ignoreCaptures) return [];
return path.match(this.regexp).slice(1);
var result = this.regexp.exec(path).slice(1);
this.regexp.lastIndex = 0;
return result;
};

/**
Expand All @@ -115,8 +124,7 @@ Layer.prototype.captures = function (path) {

Layer.prototype.url = function (params, options) {
var args = params;
var url = this.path.replace(/\(\.\*\)/g, '');
var toPath = pathToRegExp.compile(url);
var toPath = pathToRegExp.compile(this.plainPath);
var replaced;

if (typeof params != 'object') {
Expand All @@ -127,7 +135,7 @@ Layer.prototype.url = function (params, options) {
}
}

var tokens = pathToRegExp.parse(url);
var tokens = pathToRegExp.parse(this.plainPath);
var replace = {};

if (args instanceof Array) {
Expand Down Expand Up @@ -214,8 +222,10 @@ Layer.prototype.param = function (param, fn) {
Layer.prototype.setPrefix = function (prefix) {
if (this.path) {
this.path = prefix + this.path;
this.plainPath = this.path.replace(/\(\.\*\)/g, '');
this.paramNames = [];
this.regexp = pathToRegExp(this.path, this.paramNames, this.opts);
this.regexp = new RegExp(this.regexp.source, 'gi');
}

return this;
Expand Down
25 changes: 20 additions & 5 deletions lib/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ Router.prototype.routes = Router.prototype.middleware = function () {
if (!matched.route) return next();

var matchedLayers = matched.pathAndMethod
var mostSpecificLayer = matchedLayers[matchedLayers.length - 1]
var mostSpecificLayer = matched.longestMatch;
ctx._matchedRoute = mostSpecificLayer.path;
if (mostSpecificLayer.name) {
ctx._matchedRouteName = mostSpecificLayer.name;
Expand Down Expand Up @@ -651,21 +651,36 @@ Router.prototype.url = function (name, params) {
Router.prototype.match = function (path, method) {
var layers = this.stack;
var layer;
var maxMatchLength = -1;
var maxMatchPathLength = -1;
var matched = {
path: [],
pathAndMethod: [],
route: false
route: false,
longestMatch: null
};

for (var len = layers.length, i = 0; i < len; i++) {
layer = layers[i];

debug('test %s %s', layer.path, layer.regexp);
var matchLength = layer.matchLength(path);

if (layer.match(path)) {
if (~matchLength) {
matched.path.push(layer);

if (layer.methods.length === 0 || ~layer.methods.indexOf(method)) {
var matchPathLength = layer.plainPath.length;
if (matchLength > maxMatchLength) {
maxMatchLength = matchLength;
maxMatchPathLength = matchPathLength;
matched.longestMatch = layer;
} else if (matchLength === maxMatchLength &&
matchPathLength >= maxMatchPathLength) {
// tiebreak based on pattern length
maxMatchPathLength = matchPathLength;
matched.longestMatch = layer;
}
matched.pathAndMethod.push(layer);
if (layer.methods.length) matched.route = true;
}
Expand Down Expand Up @@ -728,6 +743,6 @@ Router.prototype.param = function (param, middleware) {
* @returns {String}
*/
Router.url = function (path, params) {
var args = Array.prototype.slice.call(arguments, 1);
return Layer.prototype.url.apply({ path: path }, args);
var args = Array.prototype.slice.call(arguments, 1);
return Layer.prototype.url.apply({ plainPath: path }, args);
};
4 changes: 2 additions & 2 deletions test/lib/layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ var Koa = require('koa')
, Layer = require('../../lib/layer');

describe('Layer', function() {
it('composes multiple callbacks/middlware', function(done) {
it('composes multiple callbacks/middleware', function(done) {
var app = new Koa();
var router = new Router();
app.use(router.routes());
Expand All @@ -34,7 +34,7 @@ describe('Layer', function() {
});
});

describe('Layer#match()', function() {
describe('Layer#matchLength()', function() {
it('captures URL path parameters', function(done) {
var app = new Koa();
var router = new Router();
Expand Down
56 changes: 56 additions & 0 deletions test/lib/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -1534,6 +1534,62 @@ describe('Router', function () {
});
});

it('places the longest match which is a subrouter route for `_matchedRoute` value on context', function (done) {
var app = new Koa();
var subrouter = Router()
.get('/sub', function (ctx) {
ctx.body.route1 = ctx._matchedRoute;
});
var router = Router()
.prefix('/prefix')
.use(function (ctx, next) {
ctx.body = {};
ctx.body.route2 = ctx._matchedRoute;
return next();
})
.use('/parent', subrouter.routes(), subrouter.allowedMethods());
request(http.createServer(app.use(router.routes()).callback()))
.get('/prefix/parent/sub')
.expect(200)
.end(function (err, res) {
if (err) return done(err);
expect(res.body).to.have.property('route1', '/prefix/parent/sub');
expect(res.body).to.have.property('route2', '/prefix/parent/sub');
done();
});
});

it('uses the longest match for `_matchedRoute` value on context', function(done) {
var app = new Koa();
var router = new Router();

router.get('/users/:id', function (ctx, next) {
ctx.body = { route1: ctx._matchedRoute };
should.exist(ctx.params.id);
ctx.status = 200;
return next();
});

// Matches (.*) so it will require a tie breaker
router.use(function(ctx, next) {
ctx.body.route2 = ctx._matchedRoute;
return next();
});

request(http.createServer(
app
.use(router.routes())
.callback()))
.get('/users/1')
.expect(200)
.end(function(err, res) {
if (err) return done(err);
expect(res.body).to.have.property('route1', '/users/:id');
expect(res.body).to.have.property('route2', '/users/:id');
done();
});
});

it('places a `_matchedRouteName` value on the context for a named route', function(done) {
var app = new Koa();
var router = new Router();
Expand Down

0 comments on commit 1ad877d

Please sign in to comment.