diff --git a/.changeset/quick-routes-rank.md b/.changeset/quick-routes-rank.md new file mode 100644 index 00000000..f6f2d3df --- /dev/null +++ b/.changeset/quick-routes-rank.md @@ -0,0 +1,6 @@ +--- +'@cleverbrush/server': patch +--- + +Prefer the most specific matching HTTP or WebSocket route instead of allowing +an earlier generic dynamic route to shadow a more literal route. diff --git a/libs/server-integration-tests/tests/integration.test.ts b/libs/server-integration-tests/tests/integration.test.ts index 12c00944..22bd709f 100644 --- a/libs/server-integration-tests/tests/integration.test.ts +++ b/libs/server-integration-tests/tests/integration.test.ts @@ -903,3 +903,38 @@ describe('withHealthcheck()', () => { expect(builder.withHealthcheck()).toBe(builder); }); }); + +// =========================================================================== +// Route specificity +// =========================================================================== + +describe('Route specificity', () => { + let server: Server; + + afterEach(async () => { + await server?.close(); + }); + + it('dispatches to the most specific handler regardless of registration order', async () => { + const SessionPath = route({ id: string() })`/${t => t.id}`; + const QuestionPath = route({ id: string() })`/${t => t.id}/question`; + const session = endpoint.get('/sessions', SessionPath); + const question = endpoint.get('/sessions', QuestionPath); + + server = await createServer() + .handle(session, ({ params }) => ({ + handler: 'session', + id: params.id + })) + .handle(question, ({ params }) => ({ + handler: 'question', + id: params.id + })) + .listen(0); + + const res = await request(server, 'GET', '/sessions/984/question'); + + expect(res.status).toBe(200); + expect(json(res)).toEqual({ handler: 'question', id: '984' }); + }); +}); diff --git a/libs/server/README.md b/libs/server/README.md index 481328f2..b4660b0d 100644 --- a/libs/server/README.md +++ b/libs/server/README.md @@ -97,6 +97,12 @@ server.handle(GetUser, ({ params }) => { }); ``` +When several route schemas validate the same URL, the server selects the most +specific route independently of registration order. Exact static routes win, +followed by routes with more literal path segments and then fewer dynamic +segments. Registration order only breaks ties between equally specific routes. +The same precedence applies to WebSocket subscriptions. + ### Authorization ```ts diff --git a/libs/server/src/Router.ts b/libs/server/src/Router.ts index bb407728..be12f1b7 100644 --- a/libs/server/src/Router.ts +++ b/libs/server/src/Router.ts @@ -5,20 +5,31 @@ import type { SubscriptionRegistration } from './types.js'; -interface RegisteredRoute { - readonly basePath: string; - readonly routePath: - | string - | ParseStringSchemaBuilder; - readonly registration: EndpointRegistration; +type RoutePath = string | ParseStringSchemaBuilder; + +/** Structural route precedence used to rank successful path matches. */ +interface RouteSpecificity { + readonly isStatic: boolean; + readonly literalSegments: number; + readonly dynamicSegments: number; } -interface RegisteredSubscriptionRoute { +interface RegisteredRoute { readonly basePath: string; - readonly routePath: - | string - | ParseStringSchemaBuilder; - readonly registration: SubscriptionRegistration; + readonly routePath: RoutePath; + readonly specificity: RouteSpecificity; + readonly exactPath: string | null; + readonly registration: TRegistration; +} + +interface RegisteredRouteMatch { + readonly registration: TRegistration; + readonly parsedPath: Record | null; +} + +interface RouteCollection { + readonly exactRoutes: Map>; + readonly dynamicRoutes: RegisteredRoute[]; } function normalizePath(p: string): string { @@ -34,21 +45,142 @@ function normalizePath(p: string): string { } function isParseStringSchema( - p: string | ParseStringSchemaBuilder + p: RoutePath ): p is ParseStringSchemaBuilder { return typeof p !== 'string' && typeof (p as any).validate === 'function'; } /** - * Radix-style HTTP router that maps method + path to endpoint registrations. + * Counts slash-delimited segments that contain literal route content. + * Interpolations do not add literal content, even when they can consume `/`. + */ +function countLiteralSegments(literals: readonly string[]): number { + let count = 0; + let currentSegmentHasLiteral = false; + + for (const literal of literals) { + for (const character of literal) { + if (character === '/') { + if (currentSegmentHasLiteral) count++; + currentSegmentHasLiteral = false; + } else { + currentSegmentHasLiteral = true; + } + } + } + + return count + (currentSegmentHasLiteral ? 1 : 0); +} + +/** Derives registration-order-independent precedence from route structure. */ +function getSpecificity( + basePath: string, + routePath: RoutePath +): RouteSpecificity { + if (!isParseStringSchema(routePath)) { + const suffix = normalizePath(routePath); + const fullPath = suffix === '/' ? basePath || '/' : basePath + suffix; + return { + isStatic: true, + literalSegments: countLiteralSegments([fullPath]), + dynamicSegments: 0 + }; + } + + const { literals, segments } = routePath.introspect().templateDefinition; + const fullLiterals = [basePath + (literals[0] ?? ''), ...literals.slice(1)]; + + return { + isStatic: segments.length === 0, + literalSegments: countLiteralSegments(fullLiterals), + dynamicSegments: segments.length + }; +} + +/** + * Returns a positive value when `candidate` is more specific than `current`. + * Equal scores deliberately preserve registration order. + */ +function compareSpecificity( + candidate: RouteSpecificity, + current: RouteSpecificity +): number { + if (candidate.isStatic !== current.isStatic) { + return candidate.isStatic ? 1 : -1; + } + if (candidate.literalSegments !== current.literalSegments) { + return candidate.literalSegments - current.literalSegments; + } + return current.dynamicSegments - candidate.dynamicSegments; +} + +/** Returns the complete exact path for routes without interpolations. */ +function getExactPath(basePath: string, routePath: RoutePath): string | null { + if (!isParseStringSchema(routePath)) { + const suffix = normalizePath(routePath); + return suffix === '/' ? basePath || '/' : basePath + suffix; + } + + const { literals, segments } = routePath.introspect().templateDefinition; + if (segments.length > 0) return null; + return basePath + (literals[0] ?? ''); +} + +function createRegisteredRoute( + basePath: string, + routePath: RoutePath, + registration: TRegistration +): RegisteredRoute { + const normalizedBase = normalizePath(basePath); + return { + basePath: normalizedBase, + routePath, + specificity: getSpecificity(normalizedBase, routePath), + exactPath: getExactPath(normalizedBase, routePath), + registration + }; +} + +function createRouteCollection< + TRegistration +>(): RouteCollection { + return { + exactRoutes: new Map(), + dynamicRoutes: [] + }; +} + +/** Adds a route while preserving the first registration for exact-path ties. */ +function addRegisteredRoute( + collection: RouteCollection, + route: RegisteredRoute +): void { + if (route.exactPath !== null) { + if (!collection.exactRoutes.has(route.exactPath)) { + collection.exactRoutes.set(route.exactPath, route); + } + return; + } + + collection.dynamicRoutes.push(route); +} + +/** + * HTTP and WebSocket router that maps paths to endpoint registrations. * * Both static string paths (exact-match only) and `ParseStringSchemaBuilder` * typed path templates are supported. For dynamic path parameters use * `route()` / `parseString()` templates rather than colon-param strings. + * When multiple templates validate the same URL, static routes win, followed + * by routes with more literal segments and then fewer dynamic segments. + * Registration order breaks equal-specificity ties. */ export class Router { - readonly #routes: Map = new Map(); - readonly #subscriptionRoutes: RegisteredSubscriptionRoute[] = []; + readonly #routes: Map> = + new Map(); + readonly #subscriptionRoutes: RouteCollection = + createRouteCollection(); + #finalized = true; /** * Register an endpoint with the router. @@ -56,18 +188,36 @@ export class Router { addRoute(registration: EndpointRegistration): void { const { method, basePath, pathTemplate } = registration.endpoint; const upperMethod = method.toUpperCase(); - const normalizedBase = normalizePath(basePath); - - const route: RegisteredRoute = { - basePath: normalizedBase, - routePath: pathTemplate, + const route = createRegisteredRoute( + basePath, + pathTemplate, registration - }; + ); - if (!this.#routes.has(upperMethod)) { - this.#routes.set(upperMethod, []); + let collection = this.#routes.get(upperMethod); + if (!collection) { + collection = createRouteCollection(); + this.#routes.set(upperMethod, collection); } - this.#routes.get(upperMethod)!.push(route); + addRegisteredRoute(collection, route); + this.#finalized = false; + } + + /** + * Prepare all dynamic route collections for request-time matching. + * + * Sorting is stable, so registration order remains the tie-breaker for + * routes with equal specificity. The method is idempotent and is also + * called lazily by match operations if routes have changed. + */ + finalize(): void { + if (this.#finalized) return; + + for (const collection of this.#routes.values()) { + this.#sortDynamicRoutes(collection.dynamicRoutes); + } + this.#sortDynamicRoutes(this.#subscriptionRoutes.dynamicRoutes); + this.#finalized = true; } /** @@ -80,6 +230,9 @@ export class Router { * - `{ match: null, methodNotAllowed: false }` — no match at all (404). * - `{ match: null, methodNotAllowed: false, badRequest: true }` — the URL * contains malformed percent-encoding (caller should respond with 400). + * + * Exact paths are indexed directly. Dynamic candidates are ordered by + * specificity during finalization and evaluated until the first match. */ match( method: string, @@ -98,13 +251,14 @@ export class Router { return { match: null, methodNotAllowed: false, badRequest: true }; } const upperMethod = method.toUpperCase(); + this.finalize(); // Try exact method match first const methodRoutes = this.#routes.get(upperMethod); if (methodRoutes) { - for (const route of methodRoutes) { - const result = this.#tryMatch(route, normalized); - if (result) return { match: result, methodNotAllowed: false }; + const result = this.#findFirstMatch(methodRoutes, normalized); + if (result) { + return { match: result, methodNotAllowed: false }; } } @@ -112,11 +266,8 @@ export class Router { const allowedMethods: string[] = []; for (const [m, routes] of this.#routes) { if (m === upperMethod) continue; - for (const route of routes) { - if (this.#tryMatch(route, normalized)) { - allowedMethods.push(m); - break; - } + if (this.#findFirstMatch(routes, normalized)) { + allowedMethods.push(m); } } @@ -127,10 +278,35 @@ export class Router { return { match: null, methodNotAllowed: false }; } - #tryMatch( - route: RegisteredRoute, + /** Returns the first successful match from an indexed route collection. */ + #findFirstMatch( + collection: RouteCollection, normalizedUrl: string - ): RouteMatch | null { + ): RegisteredRouteMatch | null { + const exactRoute = collection.exactRoutes.get(normalizedUrl); + if (exactRoute) { + const match = this.#tryMatch(exactRoute, normalizedUrl); + if (match) return match; + } + + for (const route of collection.dynamicRoutes) { + const match = this.#tryMatch(route, normalizedUrl); + if (match) return match; + } + + return null; + } + + #sortDynamicRoutes( + routes: RegisteredRoute[] + ): void { + routes.sort((a, b) => compareSpecificity(b.specificity, a.specificity)); + } + + #tryMatch( + route: RegisteredRoute, + normalizedUrl: string + ): RegisteredRouteMatch | null { const { basePath, routePath } = route; // Check basePath prefix @@ -177,19 +353,18 @@ export class Router { */ addSubscriptionRoute(registration: SubscriptionRegistration): void { const { basePath, pathTemplate } = registration.endpoint; - const normalizedBase = normalizePath(basePath); - - this.#subscriptionRoutes.push({ - basePath: normalizedBase, - routePath: pathTemplate, - registration - }); + addRegisteredRoute( + this.#subscriptionRoutes, + createRegisteredRoute(basePath, pathTemplate, registration) + ); + this.#finalized = false; } /** * Match an incoming WebSocket upgrade URL to a registered subscription. * - * Returns the matched registration and parsed path params, or `null`. + * Returns the most specific matched registration and parsed path params, + * or `null`. Registration order breaks equal-specificity ties. */ matchSubscription(url: string): { registration: SubscriptionRegistration; @@ -202,40 +377,7 @@ export class Router { return null; } - for (const route of this.#subscriptionRoutes) { - const { basePath, routePath } = route; - - if (basePath && !normalized.startsWith(basePath)) { - continue; - } - - const remainder = basePath - ? normalized.slice(basePath.length) - : normalized; - - if (isParseStringSchema(routePath)) { - const result = routePath.validate(remainder); - if (result.valid) { - return { - registration: route.registration, - parsedPath: result.object as Record - }; - } - continue; - } - - const normalizedRoutePath = normalizePath(routePath); - const normalizedRemainder = - remainder.length === 0 ? '/' : remainder; - - if (normalizedRemainder === normalizedRoutePath) { - return { - registration: route.registration, - parsedPath: null - }; - } - } - - return null; + this.finalize(); + return this.#findFirstMatch(this.#subscriptionRoutes, normalized); } } diff --git a/libs/server/src/Server.ts b/libs/server/src/Server.ts index 28f557af..6da69ffd 100644 --- a/libs/server/src/Server.ts +++ b/libs/server/src/Server.ts @@ -421,6 +421,7 @@ export class ServerBuilder { for (const reg of this.#subscriptionRegistrations) { router.addSubscriptionRoute(reg); } + router.finalize(); const serviceProvider = this.#serviceCollection.buildServiceProvider({ validateScopes: false diff --git a/libs/server/src/route.ts b/libs/server/src/route.ts index 39e2cf2c..719eadd7 100644 --- a/libs/server/src/route.ts +++ b/libs/server/src/route.ts @@ -88,6 +88,12 @@ export function route< /** * Concise shorthand for defining a typed path template. * + * When multiple registered templates validate the same URL, the server ranks + * them by specificity: exact static routes first, then routes with more + * literal path segments, then routes with fewer dynamic segments. + * Registration order breaks ties between equally specific routes. This + * precedence applies to both HTTP endpoints and WebSocket subscriptions. + * * @example With parameters * ```ts * const TodoById = route({ id: number().coerce() })`/${t => t.id}`; diff --git a/libs/server/tests/Router.test.ts b/libs/server/tests/Router.test.ts index 17e20ed3..a48df760 100644 --- a/libs/server/tests/Router.test.ts +++ b/libs/server/tests/Router.test.ts @@ -1,8 +1,12 @@ import { number, object, parseString, string } from '@cleverbrush/schema'; -import { describe, expect, it } from 'vitest'; -import type { EndpointMetadata } from '../src/Endpoint.js'; +import { describe, expect, it, vi } from 'vitest'; +import { type EndpointMetadata, endpoint } from '../src/Endpoint.js'; import { Router } from '../src/Router.js'; -import type { EndpointRegistration } from '../src/types.js'; +import type { SubscriptionMetadata } from '../src/Subscription.js'; +import type { + EndpointRegistration, + SubscriptionRegistration +} from '../src/types.js'; function makeRegistration( method: string, @@ -30,6 +34,16 @@ function makeRegistration( }; } +function makeSubscriptionRegistration( + basePath: string, + pathTemplate: SubscriptionMetadata['pathTemplate'] = '/' +): SubscriptionRegistration { + return { + endpoint: endpoint.subscription(basePath, pathTemplate).introspect(), + handler: async function* () {} + }; +} + describe('Router', () => { it('matches static routes', () => { const router = new Router(); @@ -80,6 +94,211 @@ describe('Router', () => { expect(result.match!.parsedPath).toEqual({ userId: 5, postId: 42 }); }); + it('prefers a nested dynamic route over an earlier generic route', () => { + const GenericPath = parseString( + object({ id: string() }), + $t => $t`/${t => t.id}` + ); + const QuestionPath = parseString( + object({ id: string() }), + $t => $t`/${t => t.id}/question` + ); + const genericValidate = vi.spyOn(GenericPath, 'validate'); + const router = new Router(); + const generic = makeRegistration('GET', '/sessions', GenericPath); + const question = makeRegistration('GET', '/sessions', QuestionPath); + router.addRoute(generic); + router.addRoute(question); + + const result = router.match('GET', '/sessions/984/question'); + + expect(result.match?.registration).toBe(question); + expect(result.match?.parsedPath).toEqual({ id: '984' }); + expect(genericValidate).not.toHaveBeenCalled(); + }); + + it('falls back when a more-specific route fails validation', () => { + const GenericPath = parseString( + object({ id: string() }), + $t => $t`/${t => t.id}` + ); + const NumericQuestionPath = parseString( + object({ id: number().coerce() }), + $t => $t`/${t => t.id}/question` + ); + const specificValidate = vi.spyOn(NumericQuestionPath, 'validate'); + const genericValidate = vi.spyOn(GenericPath, 'validate'); + const router = new Router(); + const generic = makeRegistration('GET', '/sessions', GenericPath); + const numericQuestion = makeRegistration( + 'GET', + '/sessions', + NumericQuestionPath + ); + router.addRoute(generic); + router.addRoute(numericQuestion); + + const result = router.match('GET', '/sessions/not-a-number/question'); + + expect(result.match?.registration).toBe(generic); + expect(result.match?.parsedPath).toEqual({ + id: 'not-a-number/question' + }); + expect(specificValidate).toHaveBeenCalledOnce(); + expect(genericValidate).toHaveBeenCalledOnce(); + }); + + it('re-finalizes after a route is added following a match', () => { + const GenericPath = parseString( + object({ id: string() }), + $t => $t`/${t => t.id}` + ); + const QuestionPath = parseString( + object({ id: string() }), + $t => $t`/${t => t.id}/question` + ); + const router = new Router(); + const generic = makeRegistration('GET', '/sessions', GenericPath); + const question = makeRegistration('GET', '/sessions', QuestionPath); + router.addRoute(generic); + + expect( + router.match('GET', '/sessions/984/question').match?.registration + ).toBe(generic); + + router.addRoute(question); + + const result = router.match('GET', '/sessions/984/question'); + expect(result.match?.registration).toBe(question); + expect(result.match?.parsedPath).toEqual({ id: '984' }); + }); + + it('prefers a static-leading dynamic route over a generic route', () => { + const GenericPath = parseString( + object({ id: string() }), + $t => $t`/${t => t.id}` + ); + const ActivePath = parseString( + object({ telegramUserId: string() }), + $t => $t`/active/${t => t.telegramUserId}` + ); + const router = new Router(); + const generic = makeRegistration('GET', '/sessions', GenericPath); + const active = makeRegistration('GET', '/sessions', ActivePath); + router.addRoute(generic); + router.addRoute(active); + + const result = router.match('GET', '/sessions/active/123'); + + expect(result.match?.registration).toBe(active); + expect(result.match?.parsedPath).toEqual({ + telegramUserId: '123' + }); + }); + + it('prefers an exact static route over a dynamic route', () => { + const GenericPath = parseString( + object({ id: string() }), + $t => $t`/${t => t.id}` + ); + const genericValidate = vi.spyOn(GenericPath, 'validate'); + const router = new Router(); + const generic = makeRegistration('GET', '/sessions', GenericPath); + const exact = makeRegistration('GET', '/sessions/984/question'); + router.addRoute(generic); + router.addRoute(exact); + + const result = router.match('GET', '/sessions/984/question'); + + expect(result.match?.registration).toBe(exact); + expect(result.match?.parsedPath).toBeNull(); + expect(genericValidate).not.toHaveBeenCalled(); + }); + + it('indexes zero-interpolation templates as exact routes', () => { + const GenericPath = parseString( + object({ id: string() }), + $t => $t`/${t => t.id}` + ); + const StaticPath = parseString(object({}), $t => $t`/health`); + const genericValidate = vi.spyOn(GenericPath, 'validate'); + const router = new Router(); + const generic = makeRegistration('GET', '', GenericPath); + const exact = makeRegistration('GET', '', StaticPath); + router.addRoute(generic); + router.addRoute(exact); + + const result = router.match('GET', '/health'); + + expect(result.match?.registration).toBe(exact); + expect(result.match?.parsedPath).toEqual({}); + expect(genericValidate).not.toHaveBeenCalled(); + }); + + it('prefers fewer dynamic segments when literal counts are equal', () => { + const TwoParamsPath = parseString( + object({ first: string(), second: string() }), + $t => $t`/${t => t.first}/${t => t.second}/fixed` + ); + const OneParamPath = parseString( + object({ value: string() }), + $t => $t`/${t => t.value}/fixed` + ); + const router = new Router(); + const twoParams = makeRegistration('GET', '/routes', TwoParamsPath); + const oneParam = makeRegistration('GET', '/routes', OneParamPath); + router.addRoute(twoParams); + router.addRoute(oneParam); + + const result = router.match('GET', '/routes/one/two/fixed'); + + expect(result.match?.registration).toBe(oneParam); + expect(result.match?.parsedPath).toEqual({ value: 'one/two' }); + }); + + it('uses registration order for equal-specificity dynamic routes', () => { + const TrailingLiteralPath = parseString( + object({ value: string() }), + $t => $t`/${t => t.value}/fixed` + ); + const LeadingLiteralPath = parseString( + object({ value: string() }), + $t => $t`/fixed/${t => t.value}` + ); + const secondValidate = vi.spyOn(LeadingLiteralPath, 'validate'); + const router = new Router(); + const first = makeRegistration('GET', '/routes', TrailingLiteralPath); + const second = makeRegistration('GET', '/routes', LeadingLiteralPath); + router.addRoute(first); + router.addRoute(second); + + const result = router.match('GET', '/routes/fixed/fixed'); + + expect(result.match?.registration).toBe(first); + expect(secondValidate).not.toHaveBeenCalled(); + }); + + it('preserves trailing-slash handling when ranking dynamic routes', () => { + const GenericPath = parseString( + object({ id: string() }), + $t => $t`/${t => t.id}` + ); + const QuestionPath = parseString( + object({ id: string() }), + $t => $t`/${t => t.id}/question` + ); + const router = new Router(); + const generic = makeRegistration('GET', '/sessions', GenericPath); + const question = makeRegistration('GET', '/sessions', QuestionPath); + router.addRoute(generic); + router.addRoute(question); + + const result = router.match('GET', '/sessions/984/question/'); + + expect(result.match?.registration).toBe(question); + expect(result.match?.parsedPath).toEqual({ id: '984' }); + }); + it('returns 405 for wrong method', () => { const router = new Router(); const reg = makeRegistration('GET', '/api/users'); @@ -204,4 +423,53 @@ describe('Router', () => { // the router should not crash or misinterpret the path structure. expect(result.badRequest).not.toBe(true); }); + + it('ranks WebSocket subscription routes by specificity', () => { + const GenericPath = parseString( + object({ id: string() }), + $t => $t`/${t => t.id}` + ); + const QuestionPath = parseString( + object({ id: string() }), + $t => $t`/${t => t.id}/question` + ); + const genericValidate = vi.spyOn(GenericPath, 'validate'); + const router = new Router(); + const generic = makeSubscriptionRegistration( + '/ws/sessions', + GenericPath + ); + const question = makeSubscriptionRegistration( + '/ws/sessions', + QuestionPath + ); + router.addSubscriptionRoute(generic); + router.addSubscriptionRoute(question); + + const result = router.matchSubscription('/ws/sessions/984/question/'); + + expect(result?.registration).toBe(question); + expect(result?.parsedPath).toEqual({ id: '984' }); + expect(genericValidate).not.toHaveBeenCalled(); + }); + + it('uses registration order for equal-specificity subscriptions', () => { + const FirstPath = parseString( + object({ value: string() }), + $t => $t`/${t => t.value}/fixed` + ); + const SecondPath = parseString( + object({ value: string() }), + $t => $t`/fixed/${t => t.value}` + ); + const router = new Router(); + const first = makeSubscriptionRegistration('/ws', FirstPath); + const second = makeSubscriptionRegistration('/ws', SecondPath); + router.addSubscriptionRoute(first); + router.addSubscriptionRoute(second); + + const result = router.matchSubscription('/ws/fixed/fixed'); + + expect(result?.registration).toBe(first); + }); }); diff --git a/websites/docs/app/server/page.tsx b/websites/docs/app/server/page.tsx index 554c7a82..cacc013f 100644 --- a/websites/docs/app/server/page.tsx +++ b/websites/docs/app/server/page.tsx @@ -178,6 +178,14 @@ server.handle(GetUser, ({ params }) => { }} /> +

+ If several route schemas validate the same URL, the + server selects the most specific route regardless of + registration order. Static routes win, followed by + routes with more literal path segments and then fewer + dynamic segments. Registration order only breaks ties; + WebSocket subscriptions use the same precedence. +

{/* ── Action Results ───────────────────────────────── */} diff --git a/websites/docs/public/api-docs/index.html b/websites/docs/public/api-docs/index.html index 089fc982..6b715c33 100644 --- a/websites/docs/public/api-docs/index.html +++ b/websites/docs/public/api-docs/index.html @@ -40,6 +40,8 @@

Previous Versions

+ + @@ -61,6 +63,7 @@

Previous Versions