Skip to content

Commit

Permalink
feat(rest): sort routes based on verb/path to match better
Browse files Browse the repository at this point in the history
  • Loading branch information
raymondfeng committed Sep 28, 2018
1 parent 6bb4e5d commit 267017d
Show file tree
Hide file tree
Showing 3 changed files with 202 additions and 1 deletion.
87 changes: 86 additions & 1 deletion packages/rest/src/router/routing-table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
ValueOrPromise,
} from '@loopback/context';
import * as HttpErrors from 'http-errors';
import * as pathToRegExp from 'path-to-regexp';
import {inspect} from 'util';

import {
Expand Down Expand Up @@ -127,6 +128,8 @@ export class RoutingTable {
);
}
this._routes.push(route);
// Sort routes
this._routes.sort(compareRoute);
}

describeApiPaths(): PathObject {
Expand Down Expand Up @@ -247,7 +250,12 @@ export abstract class BaseRoute implements RouteEntry {
}

match(request: Request): ResolvedRoute | undefined {
debug('trying endpoint %s', inspect(this, {depth: 5}));
debug(
'trying endpoint %s for %s %s',
inspect(this, {depth: 5}),
request.method,
request.path,
);
if (this.verb !== request.method!.toLowerCase()) {
debug(' -> verb mismatch');
return undefined;
Expand Down Expand Up @@ -469,3 +477,80 @@ export function createControllerFactoryForInstance<T>(
): ControllerFactory<T> {
return ctx => controllerInst;
}

/**
* Sorting order for http verbs
*/
const HTTP_VERBS: {[name: string]: number} = {
post: 1,
put: 2,
patch: 3,
get: 4,
head: 5,
delete: 6,
options: 7,
};

/**
* Compare two routes by verb/path for sorting
* @param route1 First route entry
* @param route2 Second route entry
*/
export function compareRoute(
route1: Pick<RouteEntry, 'verb' | 'path'>,
route2: Pick<RouteEntry, 'verb' | 'path'>,
): number {
// First check verb
const verb1 = HTTP_VERBS[route1.verb.toLowerCase()] || HTTP_VERBS.get;
const verb2 = HTTP_VERBS[route2.verb.toLowerCase()] || HTTP_VERBS.get;
if (verb1 !== verb2) return verb1 - verb2;

// Then check the path tokens
const path1 = route1.path.replace(/{([^}]*)}(\/|$)/g, ':$1$2');
const path2 = route2.path.replace(/{([^}]*)}(\/|$)/g, ':$1$2');
const tokensForPath1: pathToRegExp.Token[] = parse(path1);
const tokensForPath2: pathToRegExp.Token[] = parse(path2);

// Longer path comes before shorter path
if (tokensForPath1.length < tokensForPath2.length) {
return 1;
} else if (tokensForPath1.length > tokensForPath2.length) {
return -1;
}

// Now check token by token
for (let i = 0; i < tokensForPath1.length; i++) {
const token1 = tokensForPath1[i];
const token2 = tokensForPath2[i];
if (typeof token1 === 'string' && typeof token2 === 'string') {
if (token1 < token2) return -1;
else if (token1 > token2) return 1;
} else if (typeof token1 === 'string' && typeof token2 === 'object') {
// token 1 is a literal while token 2 is a parameter
return -1;
} else if (typeof token1 === 'object' && typeof token2 === 'string') {
// token 1 is a parameter while token 2 is a literal
return 1;
} else {
// Both token are parameters. Treat as equal weight.
}
}
return 0;
}

/**
*
* @param path Parse a path template into tokens
*/
function parse(path: string) {
const tokens: pathToRegExp.Token[] = [];
pathToRegExp.parse(path).forEach(p => {
if (typeof p === 'string') {
// The string can be /orders/count
tokens.push(...p.split('/').filter(Boolean));
} else {
tokens.push(p);
}
});
return tokens;
}
42 changes: 42 additions & 0 deletions packages/rest/test/acceptance/routing/routing.acceptance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,48 @@ describe('Routing', () => {
.get('/')
.expect(200, 'hello');
});

it('sorts routes based on their specifics', async () => {
const app = new RestApplication();

class MyController {
greet(name: string) {
return `hello ${name}`;
}

greetWorld() {
return `hello WORLD`;
}
}

const spec = anOpenApiSpec()
.withOperation(
'get',
'/greet/{name}',
anOperationSpec()
.withParameter({name: 'name', in: 'path', type: 'string'})
.withExtension('x-operation-name', 'greet')
.withExtension('x-controller-name', 'MyController'),
)
.withOperation(
'get',
'/greet/world',
anOperationSpec()
.withExtension('x-operation-name', 'greetWorld')
.withExtension('x-controller-name', 'MyController'),
)
.build();

app.api(spec);
app.controller(MyController);

await whenIMakeRequestTo(app)
.get('/greet/john')
.expect(200, 'hello john');
await whenIMakeRequestTo(app)
.get('/greet/world')
.expect(200, 'hello WORLD');
});
});

/* ===== HELPERS ===== */
Expand Down
74 changes: 74 additions & 0 deletions packages/rest/test/unit/router/sort-routes.unit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// Copyright IBM Corp. 2018. All Rights Reserved.
// Node module: @loopback/openapi-v3.
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {expect} from '@loopback/testlab';
import {compareRoute} from '../../../src/router/routing-table';

describe('route sorter', () => {
it('sorts routes', () => {
const routes = givenRoutes();
// Sort endpoints by their verb/path templates
const sortedEndpoints = Object.entries(routes).sort((a, b) =>
compareRoute(a[1], b[1]),
);
expect(sortedEndpoints).to.eql([
['create', {verb: 'post', path: '/orders'}],
['replaceById', {verb: 'put', path: '/orders/{id}'}],
['updateById', {verb: 'patch', path: '/orders/{id}'}],
['updateAll', {verb: 'patch', path: '/orders'}],
['exists', {verb: 'get', path: '/orders/{id}/exists'}],
['count', {verb: 'get', path: '/orders/count'}],
['findById', {verb: 'get', path: '/orders/{id}'}],
['findAll', {verb: 'get', path: '/orders'}],
['deleteById', {verb: 'delete', path: '/orders/{id}'}],
['deleteAll', {verb: 'delete', path: '/orders'}],
]);
});

function givenRoutes() {
return {
create: {
verb: 'post',
path: '/orders',
},
findAll: {
verb: 'get',
path: '/orders',
},
findById: {
verb: 'get',
path: '/orders/{id}',
},
updateById: {
verb: 'patch',
path: '/orders/{id}',
},
replaceById: {
verb: 'put',
path: '/orders/{id}',
},
count: {
verb: 'get',
path: '/orders/count',
},
exists: {
verb: 'get',
path: '/orders/{id}/exists',
},
deleteById: {
verb: 'delete',
path: '/orders/{id}',
},
deleteAll: {
verb: 'delete',
path: '/orders',
},
updateAll: {
verb: 'patch',
path: '/orders',
},
};
}
});

0 comments on commit 267017d

Please sign in to comment.