fix(rest): remove trailing slash for routing match

See #2188
loopbackio · Feb 8, 2019 · 88ec137 · 88ec137
1 parent 5042698
commit 88ec137
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 5 deletions.
diff --git a/packages/rest/src/__tests__/unit/router/routing-table.unit.ts b/packages/rest/src/__tests__/unit/router/routing-table.unit.ts
@@ -19,6 +19,7 @@ import {
   TrieRouter,
 } from '../../..';
 import * as HttpErrors from 'http-errors';
+import {ResolvedRoute} from '../../../router';
 
 describe('RoutingTable', () => {
   it('joins basePath and path', () => {
@@ -214,6 +215,44 @@ function runTestsWithRouter(router: RestRouter) {
     expect(route.pathParams).to.containEql({userId: '1', format: 'json'});
   });
 
+  it('finds "GET /orders/ and GET /orders/{id}" endpoints', () => {
+    class TestController {
+      @get('/orders/{id}')
+      async getOrderById(@param.path.number('id') id: number): Promise<object> {
+        return {id};
+      }
+      // With trailing `/`
+      @get('/orders/')
+      async findOrders(): Promise<object[]> {
+        return [];
+      }
+      // Without trailing `/`
+      @get('/pendingOrders')
+      async findPendingOrders(): Promise<object[]> {
+        return [];
+      }
+    }
+
+    const table = givenRoutingTable();
+    const spec = getControllerSpec(TestController);
+    table.registerController(spec, TestController);
+
+    const findAndCheckRoute = (url: string, expectedPath: string) => {
+      let request = givenRequest({
+        method: 'get',
+        url,
+      });
+      const route = table.find(request);
+      expect(route.path).to.eql(expectedPath);
+    };
+
+    findAndCheckRoute('/orders/1', '/orders/{id}');
+    findAndCheckRoute('/orders', '/orders');
+    findAndCheckRoute('/orders/', '/orders');
+    findAndCheckRoute('/pendingOrders', '/pendingOrders');
+    findAndCheckRoute('/pendingOrders/', '/pendingOrders');
+  });
+
   it('throws if router is not found', () => {
     const table = givenRoutingTable();
 

diff --git a/packages/rest/src/router/router-base.ts b/packages/rest/src/router/router-base.ts
@@ -18,12 +18,29 @@ export abstract class BaseRouter implements RestRouter {
    */
   protected routesWithoutPathVars: {[path: string]: RouteEntry} = {};
 
-  protected getKeyForRoute(route: RouteEntry) {
-    const path = route.path.startsWith('/') ? route.path : `/${route.path}`;
-    const verb = route.verb.toLowerCase() || 'get';
+  /**
+   * Build a key for verb+path
+   * @param verb HTTP verb/method
+   * @param path URL path
+   */
+  private getKey(verb: string, path: string) {
+    // Use lower case
+    verb = (verb && verb.toLowerCase()) || 'get';
+    // Prepend `/` if needed
+    path = path || '/';
+    path = path.startsWith('/') ? path : `/${path}`;
+    if (path !== '/' && path.endsWith('/')) {
+      // Remove trailing `/`
+      // See https://github.com/strongloop/loopback-next/issues/2188
+      path = path.substring(0, path.length - 1);
+    }
     return `/${verb}${path}`;
   }
 
+  protected getKeyForRoute(route: RouteEntry) {
+    return this.getKey(route.verb, route.path);
+  }
+
   add(route: RouteEntry) {
     if (!getPathVariables(route.path)) {
       const key = this.getKeyForRoute(route);
@@ -34,8 +51,7 @@ export abstract class BaseRouter implements RestRouter {
   }
 
   protected getKeyForRequest(request: Request) {
-    const method = request.method.toLowerCase();
-    return `/${method}${request.path}`;
+    return this.getKey(request.method, request.path);
   }
 
   find(request: Request) {