From e043a4b00337b9becdb9cea6bdcff4ce207567ac Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Tue, 25 Jul 2023 23:00:32 +0200 Subject: [PATCH 1/8] fix(router): fallback for method-shadowed routes --- src/router.ts | 26 ++++++++++++++++++++++++-- test/router.test.ts | 21 +++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/router.ts b/src/router.ts index c554f031..e89d59f5 100644 --- a/src/router.ts +++ b/src/router.ts @@ -1,4 +1,8 @@ -import { createRouter as _createRouter } from "radix3"; +import { + createRouter as _createRouter, + toRouteMatcher, + RouteMatcher, +} from "radix3"; import type { HTTPMethod, EventHandler } from "./types"; import { createError } from "./error"; import { eventHandler, toEventHandler } from "./event"; @@ -42,6 +46,7 @@ export interface CreateRouterOptions { export function createRouter(opts: CreateRouterOptions = {}): Router { const _router = _createRouter({}); + let _matcher: RouteMatcher | undefined; const routes: Record = {}; const router: Router = {} as Router; @@ -100,7 +105,24 @@ export function createRouter(opts: CreateRouterOptions = {}): Router { const method = ( event.node.req.method || "get" ).toLowerCase() as RouterMethod; - const handler = matched.handlers[method] || matched.handlers.all; + let handler = matched.handlers[method] || matched.handlers.all; + + // Fallback to search for shadowed routes + if (!handler) { + if (!_matcher) { + _matcher = toRouteMatcher(_router); + } + // Default order is less specific to most specific + const matches: RouteNode[] = _matcher.matchAll(path).reverse(); + if (matches.length > 1) { + const _match = matches.find( + (m) => m.handlers[method] || m.handlers.all + ); + handler = _match?.handlers[method] || _match?.handlers.all; + } + } + + // Method not matched if (!handler) { if (opts.preemptive || opts.preemtive) { throw createError({ diff --git a/test/router.test.ts b/test/router.test.ts index 6a1f9975..2fe5f1ed 100644 --- a/test/router.test.ts +++ b/test/router.test.ts @@ -97,6 +97,27 @@ describe("router", () => { const res = await request.get("/404"); expect(res.status).toEqual(404); }); + + it("Handle shadowed route", async () => { + router.post( + "/test/123", + eventHandler((event) => `[${event.method}] ${event.path}`) + ); + + router.use( + "/test/**", + eventHandler((event) => `[${event.method}] ${event.path}`) + ); + + const postRed = await request.post("/test/123"); + expect(postRed.status).toEqual(200); + expect(postRed.text).toEqual("[POST] /test/123"); + + const getRes = await request.get("/test/123"); + console.log(getRes.text); + expect(getRes.status).toEqual(200); + expect(getRes.text).toEqual("[GET] /test/123"); + }); }); describe("router (preemptive)", () => { From 08d8e0ef0c57e2e98d4fad55933e52b8a1e30195 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Tue, 25 Jul 2023 23:02:39 +0200 Subject: [PATCH 2/8] fix types --- src/router.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/router.ts b/src/router.ts index e89d59f5..7ffb4742 100644 --- a/src/router.ts +++ b/src/router.ts @@ -46,7 +46,7 @@ export interface CreateRouterOptions { export function createRouter(opts: CreateRouterOptions = {}): Router { const _router = _createRouter({}); - let _matcher: RouteMatcher | undefined; + let _matcher: RouteMatcher | undefined; const routes: Record = {}; const router: Router = {} as Router; @@ -113,7 +113,7 @@ export function createRouter(opts: CreateRouterOptions = {}): Router { _matcher = toRouteMatcher(_router); } // Default order is less specific to most specific - const matches: RouteNode[] = _matcher.matchAll(path).reverse(); + const matches = _matcher.matchAll(path).reverse() as RouteNode[]; if (matches.length > 1) { const _match = matches.find( (m) => m.handlers[method] || m.handlers.all From e8e0f3bd7bd1d787cf95b99fd37ecde9acfb2263 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Tue, 25 Jul 2023 23:03:54 +0200 Subject: [PATCH 3/8] improve check --- src/router.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/router.ts b/src/router.ts index 7ffb4742..178c51d1 100644 --- a/src/router.ts +++ b/src/router.ts @@ -118,7 +118,9 @@ export function createRouter(opts: CreateRouterOptions = {}): Router { const _match = matches.find( (m) => m.handlers[method] || m.handlers.all ); - handler = _match?.handlers[method] || _match?.handlers.all; + if (_match) { + handler = _match.handlers[method] || _match.handlers.all; + } } } From 3d57e75cb68b799d1a8db523836da242738dd12e Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Tue, 25 Jul 2023 23:05:34 +0200 Subject: [PATCH 4/8] simplify --- src/router.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/router.ts b/src/router.ts index 178c51d1..f4a732e6 100644 --- a/src/router.ts +++ b/src/router.ts @@ -114,12 +114,10 @@ export function createRouter(opts: CreateRouterOptions = {}): Router { } // Default order is less specific to most specific const matches = _matcher.matchAll(path).reverse() as RouteNode[]; - if (matches.length > 1) { - const _match = matches.find( - (m) => m.handlers[method] || m.handlers.all - ); - if (_match) { - handler = _match.handlers[method] || _match.handlers.all; + for (const match of matches) { + handler = match.handlers[method] || match.handlers.all; + if (handler) { + break; } } } From 66b677034cff6c3ec7dbb991c279ad35b268bfef Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Tue, 25 Jul 2023 23:13:59 +0200 Subject: [PATCH 5/8] perf: cache result into upper tree --- src/router.ts | 15 +++++++++++---- test/router.test.ts | 18 ++++++++++-------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/router.ts b/src/router.ts index f4a732e6..01003f3e 100644 --- a/src/router.ts +++ b/src/router.ts @@ -109,14 +109,21 @@ export function createRouter(opts: CreateRouterOptions = {}): Router { // Fallback to search for shadowed routes if (!handler) { + console.log(">>>>>> Fallback to search for shadowed routes"); if (!_matcher) { _matcher = toRouteMatcher(_router); } // Default order is less specific to most specific - const matches = _matcher.matchAll(path).reverse() as RouteNode[]; - for (const match of matches) { - handler = match.handlers[method] || match.handlers.all; - if (handler) { + const _matches = _matcher.matchAll(path).reverse() as RouteNode[]; + for (const _match of _matches) { + if (_match.handlers[method]) { + handler = _match.handlers[method]; + matched.handlers[method] = handler; + break; + } + if (_match.handlers.all) { + handler = _match.handlers.all; + matched.handlers.all = handler; break; } } diff --git a/test/router.test.ts b/test/router.test.ts index 2fe5f1ed..566c4d60 100644 --- a/test/router.test.ts +++ b/test/router.test.ts @@ -109,14 +109,16 @@ describe("router", () => { eventHandler((event) => `[${event.method}] ${event.path}`) ); - const postRed = await request.post("/test/123"); - expect(postRed.status).toEqual(200); - expect(postRed.text).toEqual("[POST] /test/123"); - - const getRes = await request.get("/test/123"); - console.log(getRes.text); - expect(getRes.status).toEqual(200); - expect(getRes.text).toEqual("[GET] /test/123"); + // Loop to validate cached behavior + for (let i = 0; i < 5; i++) { + const postRed = await request.post("/test/123"); + expect(postRed.status).toEqual(200); + expect(postRed.text).toEqual("[POST] /test/123"); + + const getRes = await request.get("/test/123"); + expect(getRes.status).toEqual(200); + expect(getRes.text).toEqual("[GET] /test/123"); + } }); }); From 5dff7f863af8bbbb515ccad31f9199a165f071f0 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Tue, 25 Jul 2023 23:24:22 +0200 Subject: [PATCH 6/8] use safer cache --- src/router.ts | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/router.ts b/src/router.ts index 01003f3e..80cbfdf4 100644 --- a/src/router.ts +++ b/src/router.ts @@ -46,9 +46,11 @@ export interface CreateRouterOptions { export function createRouter(opts: CreateRouterOptions = {}): Router { const _router = _createRouter({}); - let _matcher: RouteMatcher | undefined; const routes: Record = {}; + let _matcher: RouteMatcher | undefined; + const _shadowedRoutes: Record = {}; + const router: Router = {} as Router; // Utilities to add a new route @@ -105,7 +107,11 @@ export function createRouter(opts: CreateRouterOptions = {}): Router { const method = ( event.node.req.method || "get" ).toLowerCase() as RouterMethod; - let handler = matched.handlers[method] || matched.handlers.all; + + let handler: EventHandler | undefined = + matched.handlers[method] || + matched.handlers.all || + _shadowedRoutes[`${path}?${method}`]; // Fallback to search for shadowed routes if (!handler) { @@ -116,14 +122,9 @@ export function createRouter(opts: CreateRouterOptions = {}): Router { // Default order is less specific to most specific const _matches = _matcher.matchAll(path).reverse() as RouteNode[]; for (const _match of _matches) { - if (_match.handlers[method]) { - handler = _match.handlers[method]; - matched.handlers[method] = handler; - break; - } - if (_match.handlers.all) { - handler = _match.handlers.all; - matched.handlers.all = handler; + handler = _match.handlers[method] || _match.handlers.all; + if (handler) { + _shadowedRoutes[`${path}?${method}`] = handler; break; } } From e660db41d695f6e939a451d4046075477a827011 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Tue, 25 Jul 2023 23:25:20 +0200 Subject: [PATCH 7/8] remove log --- src/router.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/router.ts b/src/router.ts index 80cbfdf4..46bd36eb 100644 --- a/src/router.ts +++ b/src/router.ts @@ -115,7 +115,6 @@ export function createRouter(opts: CreateRouterOptions = {}): Router { // Fallback to search for shadowed routes if (!handler) { - console.log(">>>>>> Fallback to search for shadowed routes"); if (!_matcher) { _matcher = toRouteMatcher(_router); } From 0d49698476a5eeefad49aa63fee3e3b9bf3c6c54 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Wed, 26 Jul 2023 11:40:50 +0200 Subject: [PATCH 8/8] switch back to rule cache --- src/router.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/router.ts b/src/router.ts index 46bd36eb..37312be2 100644 --- a/src/router.ts +++ b/src/router.ts @@ -49,7 +49,6 @@ export function createRouter(opts: CreateRouterOptions = {}): Router { const routes: Record = {}; let _matcher: RouteMatcher | undefined; - const _shadowedRoutes: Record = {}; const router: Router = {} as Router; @@ -109,9 +108,7 @@ export function createRouter(opts: CreateRouterOptions = {}): Router { ).toLowerCase() as RouterMethod; let handler: EventHandler | undefined = - matched.handlers[method] || - matched.handlers.all || - _shadowedRoutes[`${path}?${method}`]; + matched.handlers[method] || matched.handlers.all; // Fallback to search for shadowed routes if (!handler) { @@ -121,9 +118,14 @@ export function createRouter(opts: CreateRouterOptions = {}): Router { // Default order is less specific to most specific const _matches = _matcher.matchAll(path).reverse() as RouteNode[]; for (const _match of _matches) { - handler = _match.handlers[method] || _match.handlers.all; - if (handler) { - _shadowedRoutes[`${path}?${method}`] = handler; + if (_match.handlers[method]) { + handler = _match.handlers[method]; + matched.handlers[method] = matched.handlers[method] || handler; + break; + } + if (_match.handlers.all) { + handler = _match.handlers.all; + matched.handlers.all = matched.handlers.all || handler; break; } }