Merge pull request #217 from cdimascio/paths-2

improve route sorting to match openapi spec

src/framework/openapi.spec.loader.ts

@@ -1,4 +1,3 @@
-import * as path from 'path';
 import { OpenAPIFramework } from './index';
 import {
   OpenAPIFrameworkAPIContext,
@@ -25,6 +24,15 @@ interface DiscoveredRoutes {
   basePaths: string[];
   routes: RouteMetadata[];
 }
+// Sort routes by most specific to least specific i.e. static routes before dynamic
+// e.g. /users/my_route before /users/{id}
+// Exported for tests
+export const sortRoutes = (r1, r2) => {
+  const e1 = r1.expressRoute.replace(/\/:/g, '/~');
+  const e2 = r2.expressRoute.replace(/\/:/g, '/~');
+  return e1 > e2 ? 1 : -1;
+};
+
 export class OpenApiSpecLoader {
   private readonly framework: OpenAPIFramework;
   constructor(opts: OpenAPIFrameworkArgs) {
@@ -112,7 +120,7 @@ export class OpenApiSpecLoader {
       },
     });
 
-    routes.sort(this.sortRoutes.bind(this));
+    routes.sort(sortRoutes);
 
     return {
       apiDoc,
@@ -124,25 +132,4 @@ 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/paths.sort.spec.ts

@@ -0,0 +1,44 @@
+import {
+  sortRoutes,
+} from '../src/framework/openapi.spec.loader';
+import { expect } from 'chai';
+
+describe('url sorter', () => {
+  it('should sort dynamic leafs after static leafs', async () => {
+    const urls = asRouteMetadatas(['/my/:id', '/my/path']);
+    const expected = ['/my/path', '/my/:id'];
+
+    urls.sort(sortRoutes);
+
+    expect(urls[0].expressRoute).to.equal(expected[0]);
+    expect(urls[1].expressRoute).to.equal(expected[1]);
+  });
+
+  it('should sort dynamic inner paths after static inner paths', async () => {
+    const urls = asRouteMetadatas([
+      '/my/:id/test',
+      '/my/path/test',
+      '/a/:b/c/:d',
+      '/a/:b/c/d',
+    ]);
+    const expected = [
+      '/a/:b/c/d',
+      '/a/:b/c/:d',
+      '/my/path/test',
+      '/my/:id/test',
+    ];
+
+    urls.sort(sortRoutes);
+
+    expect(urls[0].expressRoute).to.equal(expected[0]);
+    expect(urls[1].expressRoute).to.equal(expected[1]);
+    expect(urls[2].expressRoute).to.equal(expected[2]);
+    expect(urls[3].expressRoute).to.equal(expected[3]);
+  });
+});
+
+function asRouteMetadatas(urls: string[]) {
+  return urls.map(u => ({
+    expressRoute: u,
+  }));
+}