From bbb7b6dcf8d6c98fdbb85a61550a2c458443b994 Mon Sep 17 00:00:00 2001 From: Swopnil Dangol Date: Wed, 24 Sep 2025 22:19:33 +0100 Subject: [PATCH 1/5] Added includeRouter function to split routes into files --- .../src/rest/ErrorHandlerRegistry.ts | 15 +++++++ .../src/rest/RouteHandlerRegistry.ts | 39 ++++++++++++++++- packages/event-handler/src/rest/Router.ts | 37 ++++++++++++++++ .../unit/rest/Router/basic-routing.test.ts | 43 +++++++++++++++++++ 4 files changed, 133 insertions(+), 1 deletion(-) diff --git a/packages/event-handler/src/rest/ErrorHandlerRegistry.ts b/packages/event-handler/src/rest/ErrorHandlerRegistry.ts index 2a68b72d53..39ef44c11a 100644 --- a/packages/event-handler/src/rest/ErrorHandlerRegistry.ts +++ b/packages/event-handler/src/rest/ErrorHandlerRegistry.ts @@ -71,4 +71,19 @@ export class ErrorHandlerRegistry { return null; } + + /** + * Merges another {@link ErrorHandlerRegistry | `ErrorHandlerRegistry`} instance into the current instance. + * It takes the handlers from the provided registry and adds them to the current registry. + * + * @param errorHandlerRegistry - The registry instance to merge with the current instance + */ + public merge(errorHandlerRegistry: ErrorHandlerRegistry): void { + for (const [ + errorConstructor, + errorHandler, + ] of errorHandlerRegistry.#handlers) { + this.register(errorConstructor, errorHandler); + } + } } diff --git a/packages/event-handler/src/rest/RouteHandlerRegistry.ts b/packages/event-handler/src/rest/RouteHandlerRegistry.ts index 0c338f141d..c19afd7f06 100644 --- a/packages/event-handler/src/rest/RouteHandlerRegistry.ts +++ b/packages/event-handler/src/rest/RouteHandlerRegistry.ts @@ -8,7 +8,7 @@ import type { ValidationResult, } from '../types/rest.js'; import { ParameterValidationError } from './errors.js'; -import type { Route } from './Route.js'; +import { Route } from './Route.js'; import { compilePath, validatePathPattern } from './utils.js'; class RouteHandlerRegistry { @@ -193,6 +193,43 @@ class RouteHandlerRegistry { return null; } + + /** + * Merges another {@link RouteHandlerRegistry | `RouteHandlerRegistry`} instance into the current instance. + * It takes the static and dynamic routes from the provided registry and adds them to the current registry. + * + * @param routeHandlerRegistry - The registry instance to merge with the current instance + * @param options.prefix - An optional prefix to be added to the paths defined in the router + */ + public merge( + routeHandlerRegistry: RouteHandlerRegistry, + options?: { prefix: Path } + ): void { + for (const route of routeHandlerRegistry.#staticRoutes.values()) { + this.register( + new Route( + route.method as HttpMethod, + options?.prefix + ? route.path === '/' + ? options.prefix + : `${options.prefix}${route.path}` + : route.path, + route.handler, + route.middleware + ) + ); + } + for (const route of routeHandlerRegistry.#dynamicRoutes) { + this.register( + new Route( + route.method as HttpMethod, + options?.prefix ? `${options.prefix}${route.path}` : route.path, + route.handler, + route.middleware + ) + ); + } + } } export { RouteHandlerRegistry }; diff --git a/packages/event-handler/src/rest/Router.ts b/packages/event-handler/src/rest/Router.ts index cbff596d09..0bf059a7df 100644 --- a/packages/event-handler/src/rest/Router.ts +++ b/packages/event-handler/src/rest/Router.ts @@ -551,6 +551,43 @@ class Router { handler ); } + + /** + * Merges the routes, context and middleware from the passed router instance into this router instance + * + * @example + * ```typescript + * import { Router } from '@aws-lambda-powertools/event-handler/experimental-rest'; + * + * const todosRouter = new Router(); + * + * todosRouter.get('/todos', async () => { + * // List Todos + * }); + * + * todosRouter.get('/todos/{todoId}', async () => { + * // Get Todo + * }); + * + * const app = new Router(); + * app.includeRouter(todosRouter); + * + * export const handler = async (event: unknown, context: Context) => { + * return app.resolve(event, context); + * }; + * ``` + * @param router - The Router from which to merge the routes, context and middleware + * @param options.prefix - An optional prefix to be added to the paths defined in the router + */ + public includeRouter(router: Router, options?: { prefix: Path }): void { + this.context = { + ...this.context, + ...router.context, + }; + this.routeRegistry.merge(router.routeRegistry, options); + this.errorHandlerRegistry.merge(router.errorHandlerRegistry); + this.middleware.push(...router.middleware); + } } export { Router }; diff --git a/packages/event-handler/tests/unit/rest/Router/basic-routing.test.ts b/packages/event-handler/tests/unit/rest/Router/basic-routing.test.ts index d4dffadb44..6445888cc5 100644 --- a/packages/event-handler/tests/unit/rest/Router/basic-routing.test.ts +++ b/packages/event-handler/tests/unit/rest/Router/basic-routing.test.ts @@ -144,4 +144,47 @@ describe('Class: Router - Basic Routing', () => { expect(JSON.parse(createResult.body).actualPath).toBe('/todos'); expect(JSON.parse(getResult.body).actualPath).toBe('/todos/1'); }); + + it('routes to the included router when using split routers', async () => { + // Prepare + const baseRouter = new Router(); + baseRouter.get('/', async () => ({ api: 'root' })); + baseRouter.get('/version', async () => ({ api: 'listVersions' })); + baseRouter.get('/version/:id', async () => ({ api: 'getVersion' })); + baseRouter.notFound(async () => ({ error: 'NotFound' })); + + const todoRouter = new Router(); + todoRouter.get('/', async () => ({ api: 'listTodos' })); + todoRouter.post('/create', async () => ({ api: 'createTodo' })); + todoRouter.get('/:id', async () => ({ api: 'getTodo' })); + + const taskRouter = new Router(); + taskRouter.get('/', async () => ({ api: 'listTasks' })); + taskRouter.post('/create', async () => ({ api: 'createTask' })); + taskRouter.get('/:taskId', async () => ({ api: 'getTask' })); + + const app = new Router(); + app.includeRouter(baseRouter); + app.includeRouter(todoRouter, { prefix: '/todos' }); + app.includeRouter(taskRouter, { prefix: '/todos/:id/tasks' }); + + // Act & Assess + const testCases = [ + ['/', 'GET', 'api', 'root'], + ['/version', 'GET', 'api', 'listVersions'], + ['/version/1', 'GET', 'api', 'getVersion'], + ['/todos', 'GET', 'api', 'listTodos'], + ['/todos/create', 'POST', 'api', 'createTodo'], + ['/todos/1', 'GET', 'api', 'getTodo'], + ['/todos/1/tasks', 'GET', 'api', 'listTasks'], + ['/todos/1/tasks/create', 'POST', 'api', 'createTask'], + ['/todos/1/tasks/1', 'GET', 'api', 'getTask'], + ['/non-existent', 'GET', 'error', 'NotFound'], + ] as const; + + for (const [path, method, key, expected] of testCases) { + const result = await app.resolve(createTestEvent(path, method), context); + expect(JSON.parse(result.body)[key]).toBe(expected); + } + }); }); From a7789dce634b358535c50292b64c84fa387f92e1 Mon Sep 17 00:00:00 2001 From: Swopnil Dangol Date: Wed, 24 Sep 2025 22:36:57 +0100 Subject: [PATCH 2/5] Refactored the merging code --- .../src/rest/RouteHandlerRegistry.ts | 29 +++++++++---------- packages/event-handler/src/rest/Router.ts | 4 +-- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/packages/event-handler/src/rest/RouteHandlerRegistry.ts b/packages/event-handler/src/rest/RouteHandlerRegistry.ts index c19afd7f06..04551499be 100644 --- a/packages/event-handler/src/rest/RouteHandlerRegistry.ts +++ b/packages/event-handler/src/rest/RouteHandlerRegistry.ts @@ -205,25 +205,22 @@ class RouteHandlerRegistry { routeHandlerRegistry: RouteHandlerRegistry, options?: { prefix: Path } ): void { - for (const route of routeHandlerRegistry.#staticRoutes.values()) { - this.register( - new Route( - route.method as HttpMethod, - options?.prefix - ? route.path === '/' - ? options.prefix - : `${options.prefix}${route.path}` - : route.path, - route.handler, - route.middleware - ) - ); - } - for (const route of routeHandlerRegistry.#dynamicRoutes) { + const routes = [ + ...routeHandlerRegistry.#staticRoutes.values(), + ...routeHandlerRegistry.#dynamicRoutes, + ]; + for (const route of routes) { + let path = route.path; + if (options?.prefix) { + path = + route.path === '/' + ? options.prefix + : `${options.prefix}${route.path}`; + } this.register( new Route( route.method as HttpMethod, - options?.prefix ? `${options.prefix}${route.path}` : route.path, + path, route.handler, route.middleware ) diff --git a/packages/event-handler/src/rest/Router.ts b/packages/event-handler/src/rest/Router.ts index 0bf059a7df..7c74627f5f 100644 --- a/packages/event-handler/src/rest/Router.ts +++ b/packages/event-handler/src/rest/Router.ts @@ -562,11 +562,11 @@ class Router { * const todosRouter = new Router(); * * todosRouter.get('/todos', async () => { - * // List Todos + * // List API * }); * * todosRouter.get('/todos/{todoId}', async () => { - * // Get Todo + * // Get API * }); * * const app = new Router(); From 2eac09d1b1cb0cf692c1290b3752aa4fa0714948 Mon Sep 17 00:00:00 2001 From: Swopnil Dangol Date: Thu, 25 Sep 2025 13:19:25 +0100 Subject: [PATCH 3/5] Improved jsdocs and added utility function for prefixed path resolution --- .../src/rest/RouteHandlerRegistry.ts | 16 ++-- packages/event-handler/src/rest/Router.ts | 17 +++-- packages/event-handler/src/rest/utils.ts | 13 ++++ .../unit/rest/Router/basic-routing.test.ts | 74 ++++++++++--------- .../tests/unit/rest/utils.test.ts | 20 ++++- 5 files changed, 89 insertions(+), 51 deletions(-) diff --git a/packages/event-handler/src/rest/RouteHandlerRegistry.ts b/packages/event-handler/src/rest/RouteHandlerRegistry.ts index 04551499be..7dffa091f2 100644 --- a/packages/event-handler/src/rest/RouteHandlerRegistry.ts +++ b/packages/event-handler/src/rest/RouteHandlerRegistry.ts @@ -9,7 +9,11 @@ import type { } from '../types/rest.js'; import { ParameterValidationError } from './errors.js'; import { Route } from './Route.js'; -import { compilePath, validatePathPattern } from './utils.js'; +import { + compilePath, + resolvePrefixedPath, + validatePathPattern, +} from './utils.js'; class RouteHandlerRegistry { readonly #staticRoutes: Map = new Map(); @@ -199,6 +203,7 @@ class RouteHandlerRegistry { * It takes the static and dynamic routes from the provided registry and adds them to the current registry. * * @param routeHandlerRegistry - The registry instance to merge with the current instance + * @param options - Configuration options for merging the router * @param options.prefix - An optional prefix to be added to the paths defined in the router */ public merge( @@ -210,17 +215,10 @@ class RouteHandlerRegistry { ...routeHandlerRegistry.#dynamicRoutes, ]; for (const route of routes) { - let path = route.path; - if (options?.prefix) { - path = - route.path === '/' - ? options.prefix - : `${options.prefix}${route.path}`; - } this.register( new Route( route.method as HttpMethod, - path, + resolvePrefixedPath(route.path, options?.prefix), route.handler, route.middleware ) diff --git a/packages/event-handler/src/rest/Router.ts b/packages/event-handler/src/rest/Router.ts index 7c74627f5f..8ce3f5e191 100644 --- a/packages/event-handler/src/rest/Router.ts +++ b/packages/event-handler/src/rest/Router.ts @@ -41,6 +41,7 @@ import { isAPIGatewayProxyEvent, isAPIGatewayProxyResult, isHttpMethod, + resolvePrefixedPath, } from './utils.js'; class Router { @@ -293,10 +294,7 @@ class Router { public route(handler: RouteHandler, options: RestRouteOptions): void { const { method, path, middleware = [] } = options; const methods = Array.isArray(method) ? method : [method]; - let resolvedPath = path; - if (this.prefix) { - resolvedPath = path === '/' ? this.prefix : `${this.prefix}${path}`; - } + const resolvedPath = resolvePrefixedPath(path, this.prefix); for (const method of methods) { this.routeRegistry.register( @@ -553,7 +551,13 @@ class Router { } /** - * Merges the routes, context and middleware from the passed router instance into this router instance + * Merges the routes, context and middleware from the passed router instance into this router instance. + * + * **Override Behaviors:** + * - **Context**: Properties from the included router override existing properties with the same key in the current router. A warning is logged when conflicts occur. + * - **Routes**: Routes from the included router are added to the current router's registry. If a route with the same method and path already exists, the included router's route takes precedence. + * - **Error Handlers**: Error handlers from the included router are merged with existing handlers. If handlers for the same error type exist in both routers, the included router's handler takes precedence. + * - **Middleware**: Middleware from the included router is appended to the current router's middleware array. All middleware executes in registration order (current router's middleware first, then included router's middleware). * * @example * ```typescript @@ -576,7 +580,8 @@ class Router { * return app.resolve(event, context); * }; * ``` - * @param router - The Router from which to merge the routes, context and middleware + * @param router - The `Router` from which to merge the routes, context and middleware + * @param options - Configuration options for merging the router * @param options.prefix - An optional prefix to be added to the paths defined in the router */ public includeRouter(router: Router, options?: { prefix: Path }): void { diff --git a/packages/event-handler/src/rest/utils.ts b/packages/event-handler/src/rest/utils.ts index e8fbd1e5d0..eb587a0b7d 100644 --- a/packages/event-handler/src/rest/utils.ts +++ b/packages/event-handler/src/rest/utils.ts @@ -196,3 +196,16 @@ export const composeMiddleware = (middleware: Middleware[]): Middleware => { return result; }; }; + +/** + * Resolves a prefixed path by combining the provided path and prefix. + * + * @param path - The path to resolve + * @param prefix - The prefix to prepend to the path + */ +export const resolvePrefixedPath = (path: Path, prefix?: Path): Path => { + if (prefix) { + return path === '/' ? prefix : `${prefix}${path}`; + } + return path; +}; diff --git a/packages/event-handler/tests/unit/rest/Router/basic-routing.test.ts b/packages/event-handler/tests/unit/rest/Router/basic-routing.test.ts index 6445888cc5..57af65e985 100644 --- a/packages/event-handler/tests/unit/rest/Router/basic-routing.test.ts +++ b/packages/event-handler/tests/unit/rest/Router/basic-routing.test.ts @@ -1,5 +1,5 @@ import context from '@aws-lambda-powertools/testing-utils/context'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { HttpStatusCodes, HttpVerbs, @@ -147,44 +147,48 @@ describe('Class: Router - Basic Routing', () => { it('routes to the included router when using split routers', async () => { // Prepare - const baseRouter = new Router(); - baseRouter.get('/', async () => ({ api: 'root' })); - baseRouter.get('/version', async () => ({ api: 'listVersions' })); - baseRouter.get('/version/:id', async () => ({ api: 'getVersion' })); - baseRouter.notFound(async () => ({ error: 'NotFound' })); - - const todoRouter = new Router(); + const todoRouter = new Router({ logger: console }); + todoRouter.use(async ({ next }) => { + console.log('todoRouter middleware'); + await next(); + }); todoRouter.get('/', async () => ({ api: 'listTodos' })); - todoRouter.post('/create', async () => ({ api: 'createTodo' })); - todoRouter.get('/:id', async () => ({ api: 'getTodo' })); - - const taskRouter = new Router(); - taskRouter.get('/', async () => ({ api: 'listTasks' })); - taskRouter.post('/create', async () => ({ api: 'createTask' })); - taskRouter.get('/:taskId', async () => ({ api: 'getTask' })); + todoRouter.notFound(async () => { + return { + error: 'Route not found', + }; + }); + const consoleLogSpy = vi.spyOn(console, 'log'); + const consoleWarnSpy = vi.spyOn(console, 'warn'); const app = new Router(); - app.includeRouter(baseRouter); + app.use(async ({ next }) => { + console.log('app middleware'); + await next(); + }); + app.get('/todos', async () => ({ api: 'rootTodos' })); + app.get('/', async () => ({ api: 'root' })); app.includeRouter(todoRouter, { prefix: '/todos' }); - app.includeRouter(taskRouter, { prefix: '/todos/:id/tasks' }); - // Act & Assess - const testCases = [ - ['/', 'GET', 'api', 'root'], - ['/version', 'GET', 'api', 'listVersions'], - ['/version/1', 'GET', 'api', 'getVersion'], - ['/todos', 'GET', 'api', 'listTodos'], - ['/todos/create', 'POST', 'api', 'createTodo'], - ['/todos/1', 'GET', 'api', 'getTodo'], - ['/todos/1/tasks', 'GET', 'api', 'listTasks'], - ['/todos/1/tasks/create', 'POST', 'api', 'createTask'], - ['/todos/1/tasks/1', 'GET', 'api', 'getTask'], - ['/non-existent', 'GET', 'error', 'NotFound'], - ] as const; - - for (const [path, method, key, expected] of testCases) { - const result = await app.resolve(createTestEvent(path, method), context); - expect(JSON.parse(result.body)[key]).toBe(expected); - } + // Act + const rootResult = await app.resolve(createTestEvent('/', 'GET'), context); + const listTodosResult = await app.resolve( + createTestEvent('/todos', 'GET'), + context + ); + const notFoundResult = await app.resolve( + createTestEvent('/non-existent', 'GET'), + context + ); + + expect(JSON.parse(rootResult.body).api).toEqual('root'); + expect(JSON.parse(listTodosResult.body).api).toEqual('listTodos'); + expect(JSON.parse(notFoundResult.body).error).toEqual('Route not found'); + expect(consoleLogSpy).toHaveBeenNthCalledWith(1, 'app middleware'); + expect(consoleLogSpy).toHaveBeenNthCalledWith(2, 'todoRouter middleware'); + expect(consoleWarnSpy).toHaveBeenNthCalledWith( + 1, + 'Handler for method: GET and path: /todos already exists. The previous handler will be replaced.' + ); }); }); diff --git a/packages/event-handler/tests/unit/rest/utils.test.ts b/packages/event-handler/tests/unit/rest/utils.test.ts index 5082eb6608..50de2f4f03 100644 --- a/packages/event-handler/tests/unit/rest/utils.test.ts +++ b/packages/event-handler/tests/unit/rest/utils.test.ts @@ -9,7 +9,11 @@ import { isAPIGatewayProxyEvent, isAPIGatewayProxyResult, } from '../../../src/rest/index.js'; -import { compilePath, validatePathPattern } from '../../../src/rest/utils.js'; +import { + compilePath, + resolvePrefixedPath, + validatePathPattern, +} from '../../../src/rest/utils.js'; import type { Middleware, Path, @@ -567,4 +571,18 @@ describe('Path Utilities', () => { expect(result).toBeUndefined(); }); }); + + describe('resolvePrefixedPath', () => { + it.each([ + { path: '/test', prefix: '/prefix', expected: '/prefix/test' }, + { path: '/', prefix: '/prefix', expected: '/prefix' }, + { path: '/test', expected: '/test' }, + ])('resolves prefixed path', ({ path, prefix, expected }) => { + // Prepare & Act + const resolvedPath = resolvePrefixedPath(path as Path, prefix as Path); + + // Assert + expect(resolvedPath).toBe(expected); + }); + }); }); From aef81b50ed13106118d9d4ccd820099c1c2fef6b Mon Sep 17 00:00:00 2001 From: Swopnil Dangol Date: Thu, 25 Sep 2025 13:26:35 +0100 Subject: [PATCH 4/5] Added docstrings about override in the router and error handlers as well --- packages/event-handler/src/rest/ErrorHandlerRegistry.ts | 2 ++ packages/event-handler/src/rest/RouteHandlerRegistry.ts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/packages/event-handler/src/rest/ErrorHandlerRegistry.ts b/packages/event-handler/src/rest/ErrorHandlerRegistry.ts index 39ef44c11a..3dd3bbc479 100644 --- a/packages/event-handler/src/rest/ErrorHandlerRegistry.ts +++ b/packages/event-handler/src/rest/ErrorHandlerRegistry.ts @@ -76,6 +76,8 @@ export class ErrorHandlerRegistry { * Merges another {@link ErrorHandlerRegistry | `ErrorHandlerRegistry`} instance into the current instance. * It takes the handlers from the provided registry and adds them to the current registry. * + * Error handlers from the included router are merged with existing handlers. If handlers for the same error type exist in both routers, the included router's handler takes precedence. + * * @param errorHandlerRegistry - The registry instance to merge with the current instance */ public merge(errorHandlerRegistry: ErrorHandlerRegistry): void { diff --git a/packages/event-handler/src/rest/RouteHandlerRegistry.ts b/packages/event-handler/src/rest/RouteHandlerRegistry.ts index 7dffa091f2..116b95b91e 100644 --- a/packages/event-handler/src/rest/RouteHandlerRegistry.ts +++ b/packages/event-handler/src/rest/RouteHandlerRegistry.ts @@ -202,6 +202,8 @@ class RouteHandlerRegistry { * Merges another {@link RouteHandlerRegistry | `RouteHandlerRegistry`} instance into the current instance. * It takes the static and dynamic routes from the provided registry and adds them to the current registry. * + * Routes from the included router are added to the current router's registry. If a route with the same method and path already exists, the included router's route takes precedence. + * * @param routeHandlerRegistry - The registry instance to merge with the current instance * @param options - Configuration options for merging the router * @param options.prefix - An optional prefix to be added to the paths defined in the router From 78f5a3c652fc7c44f7347126e8660340a28c1eec Mon Sep 17 00:00:00 2001 From: Swopnil Dangol Date: Thu, 25 Sep 2025 13:29:18 +0100 Subject: [PATCH 5/5] Added missing Assert comment on the test --- .../event-handler/tests/unit/rest/Router/basic-routing.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/event-handler/tests/unit/rest/Router/basic-routing.test.ts b/packages/event-handler/tests/unit/rest/Router/basic-routing.test.ts index 57af65e985..a17ce1eb46 100644 --- a/packages/event-handler/tests/unit/rest/Router/basic-routing.test.ts +++ b/packages/event-handler/tests/unit/rest/Router/basic-routing.test.ts @@ -181,6 +181,7 @@ describe('Class: Router - Basic Routing', () => { context ); + // Assert expect(JSON.parse(rootResult.body).api).toEqual('root'); expect(JSON.parse(listTodosResult.body).api).toEqual('listTodos'); expect(JSON.parse(notFoundResult.body).error).toEqual('Route not found');