Express route order

packages/express/src/route-utils.test.ts

@@ -0,0 +1,196 @@
+import { compareRoutes } from './route-utils';
+
+const sortRoutes = (routes: string[]) => [...routes].sort(compareRoutes);
+
+const expectBefore = (a: string, b: string) => {
+  expect(compareRoutes(a, b)).toBeLessThan(0);
+  expect(compareRoutes(b, a)).toBeGreaterThan(0);
+};
+
+const permutations = <T>(items: T[]): T[][] => {
+  if (items.length < 2) return [items];
+
+  return items.flatMap((item, index) =>
+    permutations([...items.slice(0, index), ...items.slice(index + 1)]).map(
+      (rest) => [item, ...rest],
+    ),
+  );
+};
+
+const expectEveryPermutationSortsTo = (expected: string[]) => {
+  for (const input of permutations(expected)) {
+    expect(sortRoutes(input)).toEqual(expected);
+  }
+};
+
+describe('compareRoutes', () => {
+  describe('static-vs-parameter specificity', () => {
+    it.each<[string, string]>([
+      ['/charts/folder-tree', '/charts/:id'],
+      ['/charts/folder-tree', '/charts/:id/children'],
+      ['/charts/folder-tree/children', '/charts/:id/children'],
+      ['/orgs/:orgId/settings', '/orgs/:orgId/:section'],
+      ['/orgs/:orgId/settings', '/orgs/:orgId/:section/details'],
+      ['/orgs/:orgId/members/me', '/orgs/:orgId/members/:memberId'],
+      [
+        '/orgs/:orgId/members/me/profile',
+        '/orgs/:orgId/members/:memberId/profile',
+      ],
+    ])(
+      'sorts literal route %s before parameterized route %s',
+      (literal, parameterized) => {
+        expectBefore(literal, parameterized);
+      },
+    );
+
+    it('prevents the reported /charts/folder-tree route from being shadowed by /charts/:id', () => {
+      expect(sortRoutes(['/charts/:id', '/charts/folder-tree'])).toEqual([
+        '/charts/folder-tree',
+        '/charts/:id',
+      ]);
+    });
+  });
+
+  describe('static sibling ordering', () => {
+    it('sorts static sibling routes lexicographically', () => {
+      expect(
+        sortRoutes([
+          '/charts/recent',
+          '/charts/folder-tree',
+          '/charts/archive',
+        ]),
+      ).toEqual(['/charts/archive', '/charts/folder-tree', '/charts/recent']);
+    });
+
+    it('sorts static sibling branches lexicographically before comparing later segments', () => {
+      expect(
+        sortRoutes([
+          '/charts/b/detail',
+          '/charts/a/detail',
+          '/charts/c/detail',
+        ]),
+      ).toEqual(['/charts/a/detail', '/charts/b/detail', '/charts/c/detail']);
+    });
+  });
+
+  describe('dynamic segment behavior', () => {
+    it('treats different parameter names as equivalent route shapes', () => {
+      expect(compareRoutes('/charts/:id', '/charts/:chartId')).toBe(0);
+
+      expect(
+        compareRoutes(
+          '/orgs/:orgId/members/:memberId',
+          '/orgs/:teamId/members/:userId',
+        ),
+      ).toBe(0);
+    });
+
+    it('preserves input order for equivalent dynamic route shapes because sort is stable', () => {
+      const routes = ['/charts/:id', '/charts/:chartId', '/charts/:slug'];
+
+      expect(sortRoutes(routes)).toEqual(routes);
+    });
+  });
+
+  describe('nested route ordering', () => {
+    it('keeps an exact parent route before its deeper child route', () => {
+      expect(sortRoutes(['/users/me/profile', '/users/me'])).toEqual([
+        '/users/me',
+        '/users/me/profile',
+      ]);
+    });
+
+    it('keeps parameterized parent routes before their deeper child routes', () => {
+      expect(sortRoutes(['/users/:id/profile', '/users/:id'])).toEqual([
+        '/users/:id',
+        '/users/:id/profile',
+      ]);
+    });
+
+    it('sorts a mixed user route set deterministically', () => {
+      const routes = [
+        '/users/:id',
+        '/users/me',
+        '/users/:id/profile',
+        '/users/me/profile',
+      ];
+
+      expect(sortRoutes(routes)).toEqual([
+        '/users/me',
+        '/users/me/profile',
+        '/users/:id',
+        '/users/:id/profile',
+      ]);
+    });
+  });
+
+  describe('normalization-equivalent paths', () => {
+    it('treats trailing slashes as equivalent', () => {
+      expect(compareRoutes('/users/me/', '/users/me')).toBe(0);
+      expect(compareRoutes('/users/:id/', '/users/:id')).toBe(0);
+    });
+
+    it('treats repeated slashes as equivalent after empty segments are filtered', () => {
+      expect(compareRoutes('//users//me', '/users/me')).toBe(0);
+      expect(compareRoutes('/users//:id', '/users/:id')).toBe(0);
+    });
+  });
+
+  describe('comparator contract', () => {
+    const representativeRoutes = [
+      '/',
+      '/health',
+      '/charts/a',
+      '/charts/b/a',
+      '/charts/folder-tree',
+      '/charts/:id',
+      '/charts/:id/a',
+      '/orgs/:orgId/members/me',
+      '/orgs/:orgId/members/:memberId',
+      '/orgs/:orgId/settings',
+      '/orgs/:orgId/:section',
+      '/users/me',
+      '/users/me/profile',
+      '/users/:id',
+      '/users/:id/profile',
+    ];
+
+    it('is antisymmetric for representative routes', () => {
+      for (const a of representativeRoutes) {
+        for (const b of representativeRoutes) {
+          const ab = Math.sign(compareRoutes(a, b));
+          const ba = Math.sign(compareRoutes(b, a));
+
+          if (ab === 0 || ba === 0) {
+            expect(ab).toBe(0);
+            expect(ba).toBe(0);
+          } else {
+            expect(ab).toBe(-ba);
+          }
+        }
+      }
+    });
+
+    it('is transitive for representative routes', () => {
+      for (const a of representativeRoutes) {
+        for (const b of representativeRoutes) {
+          for (const c of representativeRoutes) {
+            const ab = compareRoutes(a, b);
+            const bc = compareRoutes(b, c);
+            const ac = compareRoutes(a, c);
+
+            if (ab <= 0 && bc <= 0) {
+              expect(ac).toBeLessThanOrEqual(0);
+            }
+          }
+        }
+      }
+    });
+
+    it('does not produce input-order-dependent sorting for the known non-transitive shape', () => {
+      const expected = ['/charts/a', '/charts/b/a', '/charts/:id/a'];
+
+      expectEveryPermutationSortsTo(expected);
+    });
+  });
+});

packages/express/src/route-utils.ts

@@ -0,0 +1,37 @@
+export function compareRoutes(a: string, b: string): number {
+  const aSegs = a.split('/').filter(Boolean);
+  const bSegs = b.split('/').filter(Boolean);
+
+  const steps = Math.max(aSegs.length, bSegs.length);
+
+  for (let i = 0; i < steps; i++) {
+    if (i >= aSegs.length) {
+      return -1;
+    }
+
+    if (i >= bSegs.length) {
+      return 1;
+    }
+
+    const aSeg = aSegs[i];
+    const bSeg = bSegs[i];
+    const aParam = aSeg.startsWith(':');
+    const bParam = bSeg.startsWith(':');
+
+    if (!aParam && !bParam) {
+      const cmp = aSeg.localeCompare(bSeg);
+
+      if (cmp !== 0) {
+        return cmp;
+      }
+    } else if (aParam && bParam) {
+      continue;
+    } else if (!aParam) {
+      return -1;
+    } else {
+      return 1;
+    }
+  }
+
+  return 0;
+}

packages/express/src/router-factory-factory.ts

@@ -4,6 +4,7 @@ import { NamespacedExpressOptions } from './types';
 import { format } from '@basketry/typescript/lib/utils';
 import { buildFilePath, buildInterfaceName } from '@basketry/typescript';
 import { camel, pascal, snake } from 'case';
+import { compareRoutes } from './route-utils';
 
 export class ExpressRouterFactoryFactory extends BaseFactory {
   constructor(service: Service, options: NamespacedExpressOptions) {
@@ -228,38 +229,3 @@ function getHandlers<TRequestHandler extends RequestHandler>(
     yield `.all(methodNotAllowed('GET, HEAD, OPTIONS'))`;
   }
 }
-
-function getRouteScore(route: string): number {
-  return route
-    .split('/')
-    .filter(Boolean)
-    .reduce((acc, seg) => acc * 3 + (seg.startsWith(':') ? 1 : 2), 0);
-}
-
-function compareRoutes(a: string, b: string): number {
-  const scoreA = getRouteScore(a);
-  const scoreB = getRouteScore(b);
-
-  const aSegs = a.split('/').filter(Boolean);
-  const bSegs = b.split('/').filter(Boolean);
-  const steps = Math.min(aSegs.length, bSegs.length);
-
-  for (let i = 0; i < steps; i++) {
-    const aSeg = aSegs[i];
-    const bSeg = bSegs[i];
-
-    if (!aSeg.startsWith(':') && !bSeg.startsWith(':')) {
-      const cmp = aSeg.localeCompare(bSeg);
-
-      if (cmp !== 0) {
-        return cmp;
-      }
-    } else if (aSeg.startsWith(':') && bSeg.startsWith(':')) {
-      continue;
-    } else {
-      break;
-    }
-  }
-
-  return scoreB - scoreA;
-}

packages/express/src/snapshot/zod/v1/express/router-factory.ts

@@ -68,13 +68,11 @@ export function getRouter({
     .all(methodNotAllowed('GET, HEAD, OPTIONS, PUT'));
 
   router
-    .route(
-      '/exhaustive/{path-string}/{path-enum}/{path-number}/{path-integer}/{path-boolean}/{path-string-array}/{path-enum-array}/{path-number-array}/{path-integer-array}/{path-boolean-array}',
-    )
+    .route('/exhaustive')
     .get(
       handlersFor(
-        'exhaustiveParams',
-        handlers.handleExhaustiveParams(getExhaustiveService),
+        'exhaustiveFormats',
+        handlers.handleExhaustiveFormats(getExhaustiveService),
       ),
     )
     .options((_, res) => {
@@ -83,11 +81,13 @@ export function getRouter({
     .all(methodNotAllowed('GET, HEAD, OPTIONS'));
 
   router
-    .route('/exhaustive')
+    .route(
+      '/exhaustive/{path-string}/{path-enum}/{path-number}/{path-integer}/{path-boolean}/{path-string-array}/{path-enum-array}/{path-number-array}/{path-integer-array}/{path-boolean-array}',
+    )
     .get(
       handlersFor(
-        'exhaustiveFormats',
-        handlers.handleExhaustiveFormats(getExhaustiveService),
+        'exhaustiveParams',
+        handlers.handleExhaustiveParams(getExhaustiveService),
       ),
     )
     .options((_, res) => {
@@ -120,6 +120,21 @@ export function getRouter({
     })
     .all(methodNotAllowed('GET, HEAD, OPTIONS, POST'));
 
+  router
+    .route('/widgets')
+    .get(handlersFor('getWidgets', handlers.handleGetWidgets(getWidgetService)))
+    .post(
+      handlersFor(
+        'createWidget',
+        handlers.handleCreateWidget(getWidgetService),
+      ),
+    )
+    .put(handlersFor('putWidget', handlers.handlePutWidget(getWidgetService)))
+    .options((_, res) => {
+      res.set('Allow', 'GET, HEAD, OPTIONS, POST, PUT').sendStatus(204);
+    })
+    .all(methodNotAllowed('GET, HEAD, OPTIONS, POST, PUT'));
+
   router
     .route('/widgets/:id/foo')
     .get(
@@ -139,21 +154,6 @@ export function getRouter({
     })
     .all(methodNotAllowed('DELETE, GET, HEAD, OPTIONS'));
 
-  router
-    .route('/widgets')
-    .get(handlersFor('getWidgets', handlers.handleGetWidgets(getWidgetService)))
-    .post(
-      handlersFor(
-        'createWidget',
-        handlers.handleCreateWidget(getWidgetService),
-      ),
-    )
-    .put(handlersFor('putWidget', handlers.handlePutWidget(getWidgetService)))
-    .options((_, res) => {
-      res.set('Allow', 'GET, HEAD, OPTIONS, POST, PUT').sendStatus(204);
-    })
-    .all(methodNotAllowed('GET, HEAD, OPTIONS, POST, PUT'));
-
   router
     .route('/')
     .get(...getMiddleware(middleware, '_onlySwaggerUI'), (_, res) => {