From e000aa2c9931194772e8cd4523a4c1fee538fdce Mon Sep 17 00:00:00 2001
From: Carmine DiMascio <cdimascio@tripadvisor.com>
Date: Fri, 10 Jan 2020 21:34:06 -0500
Subject: [PATCH] path templates matched incorrectly #214

---
 src/framework/openapi.context.ts     |  9 ----
 src/framework/openapi.spec.loader.ts | 27 +++++++++++-
 test/path.order.spec.ts              | 43 ++++++++++++++++++++
 test/resources/path.order.yaml       | 61 ++++++++++++++++++++++++++++
 4 files changed, 130 insertions(+), 10 deletions(-)
 create mode 100644 test/path.order.spec.ts
 create mode 100644 test/resources/path.order.yaml

diff --git a/src/framework/openapi.context.ts b/src/framework/openapi.context.ts
index a8a8ef78..138d8b7a 100644
--- a/src/framework/openapi.context.ts
+++ b/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) {
diff --git a/src/framework/openapi.spec.loader.ts b/src/framework/openapi.spec.loader.ts
index 4010a6ee..cd16ab7c 100644
--- a/src/framework/openapi.spec.loader.ts
+++ b/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;
+    }
+  }
 }
diff --git a/test/path.order.spec.ts b/test/path.order.spec.ts
new file mode 100644
index 00000000..bcb85022
--- /dev/null
+++ b/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));
+});
diff --git a/test/resources/path.order.yaml b/test/resources/path.order.yaml
new file mode 100644
index 00000000..b6578542
--- /dev/null
+++ b/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