diff --git a/packages/express/src/route-utils.test.ts b/packages/express/src/route-utils.test.ts new file mode 100644 index 0000000..d4ceed9 --- /dev/null +++ b/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 = (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); + }); + }); +}); diff --git a/packages/express/src/route-utils.ts b/packages/express/src/route-utils.ts new file mode 100644 index 0000000..924cd75 --- /dev/null +++ b/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; +} diff --git a/packages/express/src/router-factory-factory.ts b/packages/express/src/router-factory-factory.ts index 7accf19..02abb5a 100644 --- a/packages/express/src/router-factory-factory.ts +++ b/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( 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; -} diff --git a/packages/express/src/snapshot/zod/v1/express/router-factory.ts b/packages/express/src/snapshot/zod/v1/express/router-factory.ts index 766a674..29ba289 100644 --- a/packages/express/src/snapshot/zod/v1/express/router-factory.ts +++ b/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) => {