Merge pull request #216 from cdimascio/path-order

path templates matched incorrectly

src/framework/openapi.context.ts

@@ -49,15 +49,6 @@ export class OpenApiContext {
     return openApiRouteMethods;
   }
 
-  // private schema(route: string, method: string) {
-  //   const methods = this.methods(route);
-  //   if (methods) {
-  //     const schema = methods[method];
-  //     return schema;
-  //   }
-  //   return null;
-  // }
-
   // side-effecting builds express/openapi route maps
   private buildRouteMaps(routes: RouteMetadata[]): void {
     for (const route of routes) {

src/framework/openapi.spec.loader.ts

@@ -1,3 +1,4 @@
+import * as path from 'path';
 import { OpenAPIFramework } from './index';
 import {
   OpenAPIFrameworkAPIContext,
@@ -51,7 +52,7 @@ export class OpenApiSpecLoader {
 
       // Deasync should be used here any nowhere else!
       // it is an optional peer dep
-      // Only necessary for those looking to use a blocking 
+      // Only necessary for those looking to use a blocking
       // intial openapi parse to resolve json-schema-refs
       require('deasync').loopWhile(() => !done);
 
@@ -110,6 +111,9 @@ export class OpenApiSpecLoader {
         }
       },
     });
+
+    routes.sort(this.sortRoutes.bind(this));
+
     return {
       apiDoc,
       basePaths,
@@ -120,4 +124,25 @@ export class OpenApiSpecLoader {
   private toExpressParams(part: string): string {
     return part.replace(/\{([^}]+)}/g, ':$1');
   }
+
+  // Sort routes by most specific to least specific i.e. static routes before dynamic
+  // e.g. /users/my_route before /users/{id}
+  private sortRoutes(r1: RouteMetadata, r2: RouteMetadata) {
+    const e1 = r1.expressRoute;
+    const e2 = r2.expressRoute;
+    const a = {
+      dirname: path.dirname(e1).replace(/^\./i, ''),
+      basename: path.basename(e1).replace(/\:/i, '~'),
+    };
+    const b = {
+      dirname: path.dirname(e2).replace(/^\./i, ''),
+      basename: path.basename(e2).replace(/\:/i, '~'),
+    };
+
+    if (a.dirname === b.dirname) {
+      return a.basename > b.basename ? 1 : -1;
+    } else {
+      return a.dirname < b.dirname ? -1 : 1;
+    }
+  }
 }

test/path.order.spec.ts

@@ -0,0 +1,43 @@
+import * as path from 'path';
+import * as express from 'express';
+import * as request from 'supertest';
+import { createApp } from './common/app';
+import * as packageJson from '../package.json';
+
+describe(packageJson.name, () => {
+  let app = null;
+
+  before(async () => {
+    // Set up the express app
+    const apiSpec = path.join('test', 'resources', 'path.order.yaml');
+    app = await createApp({ apiSpec }, 3005, app =>
+      app.use(
+        `${app.basePath}`,
+        express
+          .Router()
+          .get(`/users/:id`, (req, res) => res.json({ path: req.path }))
+          .post(`/users/jimmy`, (req, res) =>
+            res.json({ ...req.body, path: req.path }),
+          ),
+      ),
+    );
+  });
+
+  after(() => {
+    app.server.close();
+  });
+
+  it('should match on users test', async () =>
+    request(app)
+      .get(`${app.basePath}/users/test`)
+      .expect(200));
+
+  it('static routes should be matched before dynamic routes', async () =>
+    request(app)
+      .post(`${app.basePath}/users/jimmy`)
+      .send({
+        id: 'some_id',
+        name: 'sally',
+      })
+      .expect(200));
+});

test/resources/path.order.yaml

@@ -0,0 +1,61 @@
+openapi: "3.0.0"
+info:
+  version: 1.0.0
+  title: Swagger Petstore
+  description: A sample API
+  termsOfService: http://swagger.io/terms/
+  license:
+    name: Apache 2.0
+    url: https://www.apache.org/licenses/LICENSE-2.0.html
+servers:
+  - url: /v1
+
+paths:
+  /users/{id}:
+    get:
+      description: get user
+      operationId: getUser
+      parameters:
+        - name: id
+          in: path
+          required: true
+          schema:
+            type: string
+      responses:
+        "200":
+          description: user response
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/User"
+
+  /users/jimmy:
+    post:
+      description: get user
+      operationId: modifyUser
+      requestBody:
+        required: true
+        content:
+          application/json:
+            schema:
+              $ref: "#/components/schemas/User"
+      responses:
+        "200":
+          description: user response
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/User"
+
+components:
+  schemas:
+    User:
+      description: default
+      type: object
+      required:
+        - id
+      properties:
+        id:
+          type: string
+        name:
+          type: string