fix(rest): handle overlapping paths with different vars

For example:

- GET /users/{id}/products
- GET /users/{id}
- GET /users/{userId}/orders

Author: raymondfeng

packages/rest/src/router/routing-table.ts

@@ -138,7 +138,7 @@ export class RoutingTable {
    * @param request
    */
   find(request: Request): ResolvedRoute {
-    debug('Finding route %s for %s %s', request.method, request.path);
+    debug('Finding route for %s %s', request.method, request.path);
 
     const found = this._router.find(request);
 

packages/rest/src/router/trie.ts

@@ -103,20 +103,19 @@ function traverse<T>(root: Node<T>, visitor: (node: NodeWithValue<T>) => void) {
 }
 
 /**
- * Match the given key to a child of the parent node
+ * Match the given key to one or more children of the parent node
  * @param key Key
  * @param parent Parent node
  */
-function matchChild<T>(
-  key: string,
-  parent: Node<T>,
-): ResolvedNode<T> | undefined {
+function matchChildren<T>(key: string, parent: Node<T>): ResolvedNode<T>[] {
+  const resolvedNodes: ResolvedNode<T>[] = [];
   // Match key literal first
   let child = parent.children[key];
   if (child) {
-    return {
+    resolvedNodes.push({
       node: child,
-    };
+    });
+    return resolvedNodes;
   }
 
   // Match templates
@@ -131,9 +130,10 @@ function matchChild<T>(
         const val = match[++i];
         resolved.params![n] = decodeURIComponent(val);
       }
-      return resolved;
+      resolvedNodes.push(resolved);
     }
   }
+  return resolvedNodes;
 }
 
 /**
@@ -154,10 +154,15 @@ function search<T>(
   const resolved: ResolvedNode<T> = {node: parent, params};
   if (key === undefined) return resolved;
 
-  const child = matchChild(key, parent);
-  if (child) {
-    Object.assign(params, child.params);
-    return search(keys, index + 1, params, child.node);
+  const children = matchChildren(key, parent);
+  if (children.length === 0) return undefined;
+  // There might be multiple matches, such as `/users/{id}` and `/users/{userId}`
+  for (const child of children) {
+    const result = search(keys, index + 1, params, child.node);
+    if (result) {
+      Object.assign(params, child.params);
+      return result;
+    }
   }
   // no matches found
   return undefined;

packages/rest/test/unit/router/trie-router.unit.ts

@@ -3,9 +3,14 @@
 // This file is licensed under the MIT License.
 // License text available at https://opensource.org/licenses/MIT
 
-import {expect} from '@loopback/testlab';
-import {TrieRouter, RouteEntry} from '../../..';
+import {
+  expect,
+  ShotRequestOptions,
+  stubExpressContext,
+} from '@loopback/testlab';
+import {TrieRouter, RouteEntry, Request} from '../../..';
 import {anOperationSpec} from '@loopback/openapi-spec-builder';
+import {ResolvedRoute} from '../../../src';
 
 describe('trie router', () => {
   class TestTrieRouter extends TrieRouter {
@@ -14,10 +19,15 @@ describe('trie router', () => {
     }
   }
 
-  const getVerbAndPath = (r: RouteEntry) => ({verb: r.verb, path: r.path});
+  let router: TestTrieRouter;
+  beforeEach(givenTrieRouter);
+
+  const getVerbAndPath = (r?: RouteEntry) => ({
+    verb: r && r.verb,
+    path: r && r.path,
+  });
 
   it('adds routes to routesWithoutPathVars', () => {
-    const router = givenTrieRouter();
     const staticRoutes = router.staticRoutes.map(getVerbAndPath);
 
     for (const r of [
@@ -36,30 +46,76 @@ describe('trie router', () => {
       {verb: 'get', path: '/orders/{id}/exists'},
       {verb: 'get', path: '/orders/{id}'},
       {verb: 'delete', path: '/orders/{id}'},
+      {verb: 'get', path: '/users/{id}'},
+      {verb: 'get', path: '/users/{userId}/orders'},
+      {verb: 'get', path: '/users/{id}/products'},
     ]) {
       expect(staticRoutes).to.not.containEql(r);
     }
   });
 
-  it('list routes by order', () => {
-    const router = givenTrieRouter();
-
+  it('lists routes by order', () => {
     expect(router.list().map(getVerbAndPath)).to.eql([
       {verb: 'post', path: '/orders'},
       {verb: 'put', path: '/orders/{id}'},
       {verb: 'patch', path: '/orders/{id}'},
       {verb: 'patch', path: '/orders'},
       {verb: 'get', path: '/orders/{id}/exists'},
+      {verb: 'get', path: '/users/{userId}/orders'},
+      {verb: 'get', path: '/users/{id}/products'},
       {verb: 'get', path: '/orders/count'},
       {verb: 'get', path: '/orders/{id}'},
+      {verb: 'get', path: '/users/{id}'},
       {verb: 'get', path: '/orders'},
       {verb: 'delete', path: '/orders/{id}'},
       {verb: 'delete', path: '/orders'},
     ]);
   });
 
+  it('finds route for GET /users/{id}', () => {
+    const req = givenRequest({method: 'get', url: '/users/123'});
+    const route = router.find(req);
+    expect(getRouteInfo(route)).to.containEql({
+      verb: 'get',
+      path: '/users/{id}',
+      params: {id: '123'},
+    });
+  });
+
+  it('finds route for GET /users/{id}/products', () => {
+    const req = givenRequest({method: 'get', url: '/users/123/products'});
+    const route = router.find(req);
+    expect(getRouteInfo(route)).to.containEql({
+      verb: 'get',
+      path: '/users/{id}/products',
+      params: {id: '123'},
+    });
+  });
+
+  it('finds route for GET /users/{userId}/orders', () => {
+    const req = givenRequest({method: 'get', url: '/users/123/orders'});
+    const route = router.find(req);
+    expect(getRouteInfo(route)).to.containEql({
+      verb: 'get',
+      path: '/users/{userId}/orders',
+      params: {userId: '123'},
+    });
+  });
+
+  function getRouteInfo(r?: ResolvedRoute) {
+    return {
+      verb: r && r.verb,
+      path: r && r.path,
+      params: r && r.pathParams,
+    };
+  }
+
+  function givenRequest(options?: ShotRequestOptions): Request {
+    return stubExpressContext(options).request;
+  }
+
   function givenTrieRouter() {
-    const router = new TestTrieRouter();
+    router = new TestTrieRouter();
     for (const r of givenRoutes()) {
       router.add(r);
     }
@@ -92,6 +148,10 @@ describe('trie router', () => {
     addRoute('deleteAll', 'delete', '/orders');
     addRoute('updateAll', 'patch', '/orders');
 
+    addRoute('getUserById', 'get', '/users/{id}');
+    addRoute('getUserOrders', 'get', '/users/{userId}/orders');
+    addRoute('getUserRecommendation', 'get', '/users/{id}/products');
+
     return routes;
   }
 });