From 49ea615225b08914d22d2e2b21998d5c331a5cd0 Mon Sep 17 00:00:00 2001 From: svozza Date: Thu, 24 Jul 2025 17:05:22 +0100 Subject: [PATCH 1/7] feat(event-handler): add route management system for API Gateway event handler --- packages/event-handler/src/rest/BaseRouter.ts | 129 +++++----- packages/event-handler/src/rest/Route.ts | 19 ++ .../src/rest/RouteHandlerRegistry.ts | 53 +++++ packages/event-handler/src/rest/utils.ts | 60 +++++ packages/event-handler/src/types/rest.ts | 22 +- .../tests/unit/rest/BaseRouter.test.ts | 60 +++-- .../unit/rest/RouteHandlerRegistry.test.ts | 222 ++++++++++++++++++ .../tests/unit/rest/utils.test.ts | 204 ++++++++++++++++ 8 files changed, 666 insertions(+), 103 deletions(-) create mode 100644 packages/event-handler/src/rest/Route.ts create mode 100644 packages/event-handler/src/rest/RouteHandlerRegistry.ts create mode 100644 packages/event-handler/src/rest/utils.ts create mode 100644 packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts create mode 100644 packages/event-handler/tests/unit/rest/utils.test.ts diff --git a/packages/event-handler/src/rest/BaseRouter.ts b/packages/event-handler/src/rest/BaseRouter.ts index aa86d99552..cc8fc9dcfd 100644 --- a/packages/event-handler/src/rest/BaseRouter.ts +++ b/packages/event-handler/src/rest/BaseRouter.ts @@ -1,5 +1,4 @@ import type { GenericLogger } from '@aws-lambda-powertools/commons/types'; -import { isRecord } from '@aws-lambda-powertools/commons/typeutils'; import { getStringFromEnv, isDevMode, @@ -14,9 +13,14 @@ import type { RouterOptions, } from '../types/rest.js'; import { HttpVerbs } from './constatnts.js'; +import { Route } from './Route.js'; +import { RouteHandlerRegistry } from './RouteHandlerRegistry.js'; abstract class BaseRouter { protected context: Record; + + protected routeRegistry: RouteHandlerRegistry; + /** * A logger instance to be used for logging debug, warning, and error messages. * @@ -39,6 +43,7 @@ abstract class BaseRouter { error: console.error, warn: console.warn, }; + this.routeRegistry = new RouteHandlerRegistry({ logger: this.logger }); this.isDev = isDevMode(); } @@ -48,126 +53,98 @@ abstract class BaseRouter { options?: ResolveOptions ): Promise; - public abstract route(handler: RouteHandler, options: RouteOptions): void; + public route(handler: RouteHandler, options: RouteOptions): void { + const { method, path } = options; + const methods = Array.isArray(method) ? method : [method]; + + for (const method of methods) { + this.routeRegistry.register(new Route(method, path, handler)); + } + } #handleHttpMethod( method: HttpMethod, path: Path, - handler?: RouteHandler | RouteOptions, - options?: RouteOptions + handler?: RouteHandler ): MethodDecorator | undefined { if (handler && typeof handler === 'function') { - this.route(handler, { ...(options || {}), method, path }); + this.route(handler, { method, path }); return; } return (_target, _propertyKey, descriptor: PropertyDescriptor) => { - const routeOptions = isRecord(handler) ? handler : options; - this.route(descriptor.value, { ...(routeOptions || {}), method, path }); + this.route(descriptor.value, { method, path }); return descriptor; }; } - public get(path: string, handler: RouteHandler, options?: RouteOptions): void; - public get(path: string, options?: RouteOptions): MethodDecorator; - public get( - path: Path, - handler?: RouteHandler | RouteOptions, - options?: RouteOptions - ): MethodDecorator | undefined { - return this.#handleHttpMethod(HttpVerbs.GET, path, handler, options); + public get(path: Path, handler: RouteHandler): void; + public get(path: Path): MethodDecorator; + public get(path: Path, handler?: RouteHandler): MethodDecorator | undefined { + return this.#handleHttpMethod(HttpVerbs.GET, path, handler); } - public post(path: Path, handler: RouteHandler, options?: RouteOptions): void; - public post(path: Path, options?: RouteOptions): MethodDecorator; - public post( - path: Path, - handler?: RouteHandler | RouteOptions, - options?: RouteOptions - ): MethodDecorator | undefined { - return this.#handleHttpMethod(HttpVerbs.POST, path, handler, options); + public post(path: Path, handler: RouteHandler): void; + public post(path: Path): MethodDecorator; + public post(path: Path, handler?: RouteHandler): MethodDecorator | undefined { + return this.#handleHttpMethod(HttpVerbs.POST, path, handler); } - public put(path: Path, handler: RouteHandler, options?: RouteOptions): void; - public put(path: Path, options?: RouteOptions): MethodDecorator; - public put( - path: Path, - handler?: RouteHandler | RouteOptions, - options?: RouteOptions - ): MethodDecorator | undefined { - return this.#handleHttpMethod(HttpVerbs.PUT, path, handler, options); + public put(path: Path, handler: RouteHandler): void; + public put(path: Path): MethodDecorator; + public put(path: Path, handler?: RouteHandler): MethodDecorator | undefined { + return this.#handleHttpMethod(HttpVerbs.PUT, path, handler); } - public patch(path: Path, handler: RouteHandler, options?: RouteOptions): void; - public patch(path: Path, options?: RouteOptions): MethodDecorator; + public patch(path: Path, handler: RouteHandler): void; + public patch(path: Path): MethodDecorator; public patch( path: Path, - handler?: RouteHandler | RouteOptions, - options?: RouteOptions + handler?: RouteHandler ): MethodDecorator | undefined { - return this.#handleHttpMethod(HttpVerbs.PATCH, path, handler, options); + return this.#handleHttpMethod(HttpVerbs.PATCH, path, handler); } + public delete(path: Path, handler: RouteHandler): void; + public delete(path: Path): MethodDecorator; public delete( path: Path, - handler: RouteHandler, - options?: RouteOptions - ): void; - public delete(path: Path, options?: RouteOptions): MethodDecorator; - public delete( - path: Path, - handler?: RouteHandler | RouteOptions, - options?: RouteOptions + handler?: RouteHandler ): MethodDecorator | undefined { - return this.#handleHttpMethod(HttpVerbs.DELETE, path, handler, options); + return this.#handleHttpMethod(HttpVerbs.DELETE, path, handler); } - public head(path: Path, handler: RouteHandler, options?: RouteOptions): void; - public head(path: Path, options?: RouteOptions): MethodDecorator; - public head( - path: Path, - handler?: RouteHandler | RouteOptions, - options?: RouteOptions - ): MethodDecorator | undefined { - return this.#handleHttpMethod(HttpVerbs.HEAD, path, handler, options); + public head(path: Path, handler: RouteHandler): void; + public head(path: Path): MethodDecorator; + public head(path: Path, handler?: RouteHandler): MethodDecorator | undefined { + return this.#handleHttpMethod(HttpVerbs.HEAD, path, handler); } + public options(path: Path, handler: RouteHandler): void; + public options(path: Path): MethodDecorator; public options( path: Path, - handler: RouteHandler, - options?: RouteOptions - ): void; - public options(path: Path, options?: RouteOptions): MethodDecorator; - public options( - path: Path, - handler?: RouteHandler | RouteOptions, - options?: RouteOptions + handler?: RouteHandler ): MethodDecorator | undefined { - return this.#handleHttpMethod(HttpVerbs.OPTIONS, path, handler, options); + return this.#handleHttpMethod(HttpVerbs.OPTIONS, path, handler); } + public connect(path: Path, handler: RouteHandler): void; + public connect(path: Path): MethodDecorator; public connect( path: Path, - handler: RouteHandler, - options?: RouteOptions - ): void; - public connect(path: Path, options?: RouteOptions): MethodDecorator; - public connect( - path: Path, - handler?: RouteHandler | RouteOptions, - options?: RouteOptions + handler?: RouteHandler ): MethodDecorator | undefined { - return this.#handleHttpMethod(HttpVerbs.CONNECT, path, handler, options); + return this.#handleHttpMethod(HttpVerbs.CONNECT, path, handler); } - public trace(path: Path, handler: RouteHandler, options?: RouteOptions): void; - public trace(path: Path, options?: RouteOptions): MethodDecorator; + public trace(path: Path, handler: RouteHandler): void; + public trace(path: Path): MethodDecorator; public trace( path: Path, - handler?: RouteHandler | RouteOptions, - options?: RouteOptions + handler?: RouteHandler ): MethodDecorator | undefined { - return this.#handleHttpMethod(HttpVerbs.TRACE, path, handler, options); + return this.#handleHttpMethod(HttpVerbs.TRACE, path, handler); } } diff --git a/packages/event-handler/src/rest/Route.ts b/packages/event-handler/src/rest/Route.ts new file mode 100644 index 0000000000..50fd54d62f --- /dev/null +++ b/packages/event-handler/src/rest/Route.ts @@ -0,0 +1,19 @@ +import type { Path, RouteHandler } from '../types/rest.js'; + +class Route { + readonly id: string; + readonly method: string; + readonly path: Path; + readonly handler: RouteHandler; + readonly registeredAt: Date; + + constructor(method: string, path: Path, handler: RouteHandler) { + this.id = `${method}:${path}`; + this.method = method.toUpperCase(); + this.path = path; + this.handler = handler; + this.registeredAt = new Date(); + } +} + +export { Route }; diff --git a/packages/event-handler/src/rest/RouteHandlerRegistry.ts b/packages/event-handler/src/rest/RouteHandlerRegistry.ts new file mode 100644 index 0000000000..4c73299747 --- /dev/null +++ b/packages/event-handler/src/rest/RouteHandlerRegistry.ts @@ -0,0 +1,53 @@ +import type { GenericLogger } from '@aws-lambda-powertools/commons/types'; +import type { RouteRegistryOptions } from '../types/rest.js'; +import type { Route } from './Route.js'; +import { validatePathPattern } from './utils.js'; + +class RouteHandlerRegistry { + #routes: Map = new Map(); + #routesByMethod: Map = new Map(); + + readonly #logger: Pick; + + constructor(options: RouteRegistryOptions) { + this.#logger = options.logger; + } + + public register(route: Route): void { + const { isValid, issues } = validatePathPattern(route.path); + if (!isValid) { + for (const issue of issues) { + this.#logger.warn(issue); + } + return; + } + + if (this.#routes.has(route.id)) { + this.#logger.warn( + `Handler for method: ${route.method} and path: ${route.path} already exists. The previous handler will be replaced.` + ); + } + + this.#routes.set(route.id, route); + + if (!this.#routesByMethod.has(route.method)) { + this.#routesByMethod.set(route.method, []); + } + // biome-ignore lint/style/noNonNullAssertion: Map.set operation above ensures Map.get won't return undefined + this.#routesByMethod.get(route.method)!.push(route); + } + + public getRouteCount(): number { + return this.#routes.size; + } + + public getRoutesByMethod(method: string): Route[] { + return this.#routesByMethod.get(method.toUpperCase()) || []; + } + + public getAllRoutes(): Route[] { + return Array.from(this.#routes.values()); + } +} + +export { RouteHandlerRegistry }; diff --git a/packages/event-handler/src/rest/utils.ts b/packages/event-handler/src/rest/utils.ts new file mode 100644 index 0000000000..fad1a6ee79 --- /dev/null +++ b/packages/event-handler/src/rest/utils.ts @@ -0,0 +1,60 @@ +import type { Path } from '../types/rest.js'; + +const PARAM_PATTERN = /:([a-zA-Z_]\w*)(?=[/]|$)/g; +const SAFE_CHARS = "-._~()'!*:@,;=+&$"; +const UNSAFE_CHARS = '%<> \\[\\]{}|^'; + +export function compilePath(path: Path): CompiledRoute { + const paramNames: string[] = []; + + const regexPattern = path.replace(PARAM_PATTERN, (_match, paramName) => { + paramNames.push(paramName); + return `(?<${paramName}>[${SAFE_CHARS}${UNSAFE_CHARS}\\w]+)`; + }); + + const finalPattern = `^${regexPattern}$`; + + return { + originalPath: path, + regex: new RegExp(finalPattern), + paramNames, + isDynamic: paramNames.length > 0, + }; +} + +type ValidationResult = { + isValid: boolean; + issues: string[]; +}; + +export function validatePathPattern(path: Path): ValidationResult { + const issues: string[] = []; + + const matches = [...path.matchAll(PARAM_PATTERN)]; + if (path.includes(':')) { + const expectedParams = path.split(':').length; + if (matches.length !== expectedParams - 1) { + issues.push('Malformed parameter syntax. Use :paramName format.'); + } + + const paramNames = matches.map((match) => match[1]); + const duplicates = paramNames.filter( + (param, index) => paramNames.indexOf(param) !== index + ); + if (duplicates.length > 0) { + issues.push(`Duplicate parameter names: ${duplicates.join(', ')}`); + } + } + + return { + isValid: issues.length === 0, + issues, + }; +} + +interface CompiledRoute { + originalPath: string; + regex: RegExp; + paramNames: string[]; + isDynamic: boolean; +} diff --git a/packages/event-handler/src/types/rest.ts b/packages/event-handler/src/types/rest.ts index 78673dfc12..28a701d00f 100644 --- a/packages/event-handler/src/types/rest.ts +++ b/packages/event-handler/src/types/rest.ts @@ -22,8 +22,24 @@ type HttpMethod = keyof typeof HttpVerbs; type Path = `/${string}`; type RouteOptions = { - method?: HttpMethod; - path?: Path; + method: HttpMethod | HttpMethod[]; + path: Path; }; -export type { HttpMethod, Path, RouterOptions, RouteHandler, RouteOptions }; +type RouteRegistryOptions = { + /** + * A logger instance to be used for logging debug, warning, and error messages. + * + * When no logger is provided, we'll only log warnings and errors using the global `console` object. + */ + logger: Pick; +}; + +export type { + HttpMethod, + Path, + RouterOptions, + RouteHandler, + RouteOptions, + RouteRegistryOptions, +}; diff --git a/packages/event-handler/tests/unit/rest/BaseRouter.test.ts b/packages/event-handler/tests/unit/rest/BaseRouter.test.ts index facd099431..bf5e2c57e5 100644 --- a/packages/event-handler/tests/unit/rest/BaseRouter.test.ts +++ b/packages/event-handler/tests/unit/rest/BaseRouter.test.ts @@ -3,18 +3,14 @@ import type { Context } from 'aws-lambda'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { BaseRouter } from '../../../src/rest/BaseRouter.js'; import { HttpVerbs } from '../../../src/rest/constatnts.js'; -import type { ResolveOptions } from '../../../src/types/index.js'; import type { HttpMethod, RouteHandler, - RouteOptions, RouterOptions, } from '../../../src/types/rest.js'; describe('Class: BaseRouter', () => { class TestResolver extends BaseRouter { - public readonly handlers: Map = new Map(); - constructor(options?: RouterOptions) { super(options); this.logger.debug('test debug'); @@ -33,22 +29,13 @@ describe('Class: BaseRouter', () => { } } - public route(handler: RouteHandler, options: RouteOptions) { - if (options.path == null || options.method == null) - throw new Error('path or method cannot be null'); - this.handlers.set(options.path + options.method, handler); - } - - public resolve( - event: unknown, - context: Context, - options?: ResolveOptions - ): Promise { + public resolve(event: unknown, context: Context): Promise { this.#isEvent(event); const { method, path } = event; - const handler = this.handlers.get(path + method); - if (handler == null) throw new Error('404'); - return handler(event, context); + const routes = this.routeRegistry.getRoutesByMethod(method); + const route = routes.find((x) => x.path === path); + if (route == null) throw new Error('404'); + return route.handler(event, context); } } @@ -69,16 +56,41 @@ describe('Class: BaseRouter', () => { ])('routes %s requests', async (method, verb) => { // Prepare const app = new TestResolver(); - (app[verb as Lowercase] as Function)( - '/test', - () => `${verb}-test` - ); + ( + app[verb as Lowercase] as ( + path: string, + handler: RouteHandler + ) => void + )('/test', () => `${verb}-test`); // Act const actual = await app.resolve({ path: '/test', method }, context); // Assess expect(actual).toEqual(`${verb}-test`); }); + it('accepts multiple HTTP methods', async () => { + // Act + const app = new TestResolver(); + app.route(() => 'route-test', { + path: '/test', + method: [HttpVerbs.GET, HttpVerbs.POST], + }); + + // Act + const getResult = await app.resolve( + { path: '/test', method: HttpVerbs.GET }, + context + ); + const postResult = await app.resolve( + { path: '/test', method: HttpVerbs.POST }, + context + ); + + // Assess + expect(getResult).toEqual('route-test'); + expect(postResult).toEqual('route-test'); + }); + it('uses the global console when no logger is not provided', () => { // Act const app = new TestResolver(); @@ -128,7 +140,7 @@ describe('Class: BaseRouter', () => { const app = new TestResolver(); class Lambda { - @app.get('/test', {}) + @app.get('/test') public async getTest() { return 'get-test'; } @@ -174,7 +186,7 @@ describe('Class: BaseRouter', () => { } public async handler(event: unknown, context: Context) { - return app.resolve(event, context, {}); + return app.resolve(event, context); } } diff --git a/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts b/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts new file mode 100644 index 0000000000..839fb027f2 --- /dev/null +++ b/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts @@ -0,0 +1,222 @@ +import { describe, expect, it, vi } from 'vitest'; +import { HttpVerbs } from '../../../src/rest/constatnts.js'; +import { Route } from '../../../src/rest/Route.js'; +import { RouteHandlerRegistry } from '../../../src/rest/RouteHandlerRegistry.js'; +import type { Path } from '../../../src/types/rest.js'; + +describe('Class: RouteHandlerRegistry', () => { + it('should warn when registering a duplicate route', () => { + // Prepare + const mockLogger = { + debug: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + + const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const handler = () => 'test'; + const path = '/test'; + const method = HttpVerbs.GET; + + // Act + const route1 = new Route(method, path, handler); + registry.register(route1); + + const route2 = new Route(method, path, () => 'another handler'); + registry.register(route2); + + // Assert + expect(mockLogger.warn).toHaveBeenCalledWith( + `Handler for method: ${method} and path: ${path} already exists. The previous handler will be replaced.` + ); + expect(registry.getRouteCount()).toBe(1); + + const routes = registry.getAllRoutes(); + expect(routes).toHaveLength(1); + expect(routes[0]).toBe(route2); + }); + + it.each([ + { path: '/users/:id:', description: 'malformed parameter syntax' }, + { path: '/users/:12345id', description: 'parameter beginning with number' }, + { path: '/users/:', description: 'parameter without name' }, + { path: '/users/:id-name', description: 'parameter with hyphen' }, + { path: '/users/:id.name', description: 'parameter with dot' }, + { + path: '/users/:id:name', + description: 'consecutive parameters without separator', + }, + ])( + 'should not register routes with invalid path pattern: $description', + ({ path }) => { + // Prepare + const mockLogger = { + debug: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + + const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const handler = () => 'test'; + + const route = new Route(HttpVerbs.GET, path as Path, handler); + + // Act + registry.register(route); + + // Assert + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Malformed parameter syntax. Use :paramName format.' + ); + expect(registry.getRouteCount()).toBe(0); + expect(registry.getAllRoutes()).toHaveLength(0); + expect(registry.getRoutesByMethod(HttpVerbs.GET)).toHaveLength(0); + } + ); + + it('should not register routes with duplicate parameter names', () => { + // Prepare + const mockLogger = { + debug: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + + const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const handler = () => 'test'; + + // Create a route with duplicate parameter names + const invalidPath = '/users/:id/posts/:id'; + const route = new Route(HttpVerbs.GET, invalidPath, handler); + + // Act + registry.register(route); + + // Assert + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Duplicate parameter names: id' + ); + expect(registry.getRouteCount()).toBe(0); // Route should not be registered + expect(registry.getAllRoutes()).toHaveLength(0); + expect(registry.getRoutesByMethod(HttpVerbs.GET)).toHaveLength(0); + }); + + describe('getRouteCount', () => { + it('returns 0 for empty registry', () => { + // Prepare + const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; + const registry = new RouteHandlerRegistry({ logger: mockLogger }); + + // Act & Assert + expect(registry.getRouteCount()).toBe(0); + }); + + it('returns correct count after registering routes', () => { + // Prepare + const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; + const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const handler = () => 'test'; + + // Act & Assert + registry.register(new Route(HttpVerbs.GET, '/users', handler)); + expect(registry.getRouteCount()).toBe(1); + + registry.register(new Route(HttpVerbs.POST, '/users', handler)); + expect(registry.getRouteCount()).toBe(2); + + registry.register(new Route(HttpVerbs.GET, '/posts', handler)); + expect(registry.getRouteCount()).toBe(3); + }); + }); + + describe('getRoutesByMethod', () => { + it('returns empty array for non-existent method', () => { + // Prepare + const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; + const registry = new RouteHandlerRegistry({ logger: mockLogger }); + + // Act & Assert + expect(registry.getRoutesByMethod('GET')).toEqual([]); + expect(registry.getRoutesByMethod('POST')).toEqual([]); + }); + + it.each([ + { method: HttpVerbs.GET }, + { method: HttpVerbs.POST }, + { method: HttpVerbs.PUT }, + { method: HttpVerbs.DELETE }, + ])('returns routes for $method method', ({ method }) => { + // Prepare + const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; + const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const handler = () => 'test'; + + const route1 = new Route(method, '/users', handler); + const route2 = new Route(method, '/posts', handler); + const otherMethodRoute = new Route(HttpVerbs.PATCH, '/other', handler); + + // Act + registry.register(route1); + registry.register(route2); + registry.register(otherMethodRoute); + + // Assert + const routes = registry.getRoutesByMethod(method); + expect(routes).toHaveLength(2); + expect(routes).toContain(route1); + expect(routes).toContain(route2); + expect(routes).not.toContain(otherMethodRoute); + }); + + it('handles case-insensitive method lookup', () => { + // Prepare + const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; + const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const handler = () => 'test'; + + const getRoute = new Route(HttpVerbs.GET, '/users', handler); + + // Act + registry.register(getRoute); + + // Assert + expect(registry.getRoutesByMethod('get')).toContain(getRoute); + expect(registry.getRoutesByMethod('GET')).toContain(getRoute); + expect(registry.getRoutesByMethod('Get')).toContain(getRoute); + }); + }); + + describe('getAllRoutes', () => { + it('returns empty array for empty registry', () => { + // Prepare + const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; + const registry = new RouteHandlerRegistry({ logger: mockLogger }); + + // Act & Assert + expect(registry.getAllRoutes()).toEqual([]); + }); + + it('returns all registered routes', () => { + // Prepare + const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; + const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const handler = () => 'test'; + + const route1 = new Route(HttpVerbs.GET, '/users', handler); + const route2 = new Route(HttpVerbs.POST, '/users', handler); + const route3 = new Route(HttpVerbs.GET, '/posts', handler); + + // Act + registry.register(route1); + registry.register(route2); + registry.register(route3); + + // Assert + const allRoutes = registry.getAllRoutes(); + expect(allRoutes).toHaveLength(3); + expect(allRoutes).toContain(route1); + expect(allRoutes).toContain(route2); + expect(allRoutes).toContain(route3); + }); + }); +}); diff --git a/packages/event-handler/tests/unit/rest/utils.test.ts b/packages/event-handler/tests/unit/rest/utils.test.ts new file mode 100644 index 0000000000..5f71837967 --- /dev/null +++ b/packages/event-handler/tests/unit/rest/utils.test.ts @@ -0,0 +1,204 @@ +import { describe, expect, it } from 'vitest'; +import { compilePath, validatePathPattern } from '../../../src/rest/utils.js'; +import type { Path } from '../../../src/types/rest.js'; + +describe('Path Utilities', () => { + describe('validatePathPattern', () => { + it.each([ + { path: '/users/:id', expected: true, issues: [] }, + { path: '/users/:user_id/posts/:post_id', expected: true, issues: [] }, + { path: '/static/path', expected: true, issues: [] }, + { path: '/:param1/:param2/:param3', expected: true, issues: [] }, + { + path: '/users/:id:', + expected: false, + issues: ['Malformed parameter syntax. Use :paramName format.'], + }, + { + path: '/users/:id/:id', + expected: false, + issues: ['Duplicate parameter names: id'], + }, + { + path: '/:param/:param/:param%', + expected: false, + issues: [ + 'Malformed parameter syntax. Use :paramName format.', + 'Duplicate parameter names: param', + ], + }, + { + path: '/users/:id#', + expected: false, + issues: ['Malformed parameter syntax. Use :paramName format.'], + }, + { + path: '/users/:12345id', + expected: false, + issues: ['Malformed parameter syntax. Use :paramName format.'], + }, + { + path: '/users/:', + expected: false, + issues: ['Malformed parameter syntax. Use :paramName format.'], + }, + { + path: '/users/:id/posts/:id', + expected: false, + issues: ['Duplicate parameter names: id'], + }, + { + path: '/users/:name/:age/:name', + expected: false, + issues: ['Duplicate parameter names: name'], + }, + { + path: '/users/:id:name', + expected: false, + issues: ['Malformed parameter syntax. Use :paramName format.'], + }, + { + path: '/users/:id/posts/:name^', + expected: false, + issues: ['Malformed parameter syntax. Use :paramName format.'], + }, + { + path: '/post/:id#comment', + expected: false, + issues: ['Malformed parameter syntax. Use :paramName format.'], + }, + ])( + 'should validate path "$path" correctly', + ({ path, expected, issues }) => { + // Act + const result = validatePathPattern(path as Path); + + // Assert + expect(result.isValid).toBe(expected); + expect(result.issues).toEqual(issues); + } + ); + }); + + describe('compilePath', () => { + it.each([ + { + path: '/users/:id', + expectedParams: ['id'], + testMatches: [ + { url: '/users/123', shouldMatch: true, params: { id: '123' } }, + { url: '/users/abc', shouldMatch: true, params: { id: 'abc' } }, + { url: '/users/123/posts', shouldMatch: false, params: {} }, + { url: '/users', shouldMatch: false, params: {} }, + ], + }, + { + path: '/users/:userId/posts/:postId', + expectedParams: ['userId', 'postId'], + testMatches: [ + { + url: '/users/123/posts/456', + shouldMatch: true, + params: { userId: '123', postId: '456' }, + }, + { + url: '/users/abc/posts/def', + shouldMatch: true, + params: { userId: 'abc', postId: 'def' }, + }, + { url: '/users/123/posts', shouldMatch: false, params: {} }, + { url: '/users/123', shouldMatch: false, params: {} }, + ], + }, + { + path: '/static/path', + expectedParams: [], + testMatches: [ + { url: '/static/path', shouldMatch: true, params: {} }, + { url: '/static/other', shouldMatch: false, params: {} }, + { url: '/static/path/extra', shouldMatch: false, params: {} }, + ], + }, + { + path: '/:param1/:param2/:param3', + expectedParams: ['param1', 'param2', 'param3'], + testMatches: [ + { + url: '/a/b/c', + shouldMatch: true, + params: { param1: 'a', param2: 'b', param3: 'c' }, + }, + { + url: '/123/456/789', + shouldMatch: true, + params: { param1: '123', param2: '456', param3: '789' }, + }, + { url: '/a/b', shouldMatch: false, params: {} }, + { url: '/a/b/c/d', shouldMatch: false, params: {} }, + ], + }, + { + path: '/users/:id/profile', + expectedParams: ['id'], + testMatches: [ + { + url: '/users/123/profile', + shouldMatch: true, + params: { id: '123' }, + }, + { + url: '/users/abc/profile', + shouldMatch: true, + params: { id: 'abc' }, + }, + { url: '/users/profile', shouldMatch: false, params: {} }, + { url: '/users/123/settings', shouldMatch: false, params: {} }, + ], + }, + { + path: '/api/:version/users/:userId', + expectedParams: ['version', 'userId'], + testMatches: [ + { + url: '/api/v1/users/123', + shouldMatch: true, + params: { version: 'v1', userId: '123' }, + }, + { + url: '/api/v2/users/abc', + shouldMatch: true, + params: { version: 'v2', userId: 'abc' }, + }, + { url: '/api/users/123', shouldMatch: false, params: {} }, + { url: '/api/v1/users', shouldMatch: false, params: {} }, + ], + }, + ])( + 'should compile path "$path" correctly', + ({ path, expectedParams, testMatches }) => { + // Act + const compiled = compilePath(path as Path); + + // Assert + expect(compiled.originalPath).toBe(path); + expect(compiled.paramNames).toEqual(expectedParams); + expect(compiled.isDynamic).toBe(expectedParams.length > 0); + + // Test regex matching + for (const testCase of testMatches) { + if (testCase.shouldMatch) { + expect(testCase.url).toMatch(compiled.regex); + + // Test extracted parameters + const match = compiled.regex.exec(testCase.url); + if (match?.groups) { + expect(match.groups).toEqual(testCase.params); + } + } else { + expect(testCase.url).not.toMatch(compiled.regex); + } + } + } + ); + }); +}); From 09977fd0e5845a78bd645999cd66a6f0bb9cd3d3 Mon Sep 17 00:00:00 2001 From: svozza Date: Thu, 24 Jul 2025 17:42:47 +0100 Subject: [PATCH 2/7] address sonarqube finding --- packages/event-handler/src/rest/RouteHandlerRegistry.ts | 4 ++-- packages/event-handler/src/rest/utils.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/event-handler/src/rest/RouteHandlerRegistry.ts b/packages/event-handler/src/rest/RouteHandlerRegistry.ts index 4c73299747..09f6d90f89 100644 --- a/packages/event-handler/src/rest/RouteHandlerRegistry.ts +++ b/packages/event-handler/src/rest/RouteHandlerRegistry.ts @@ -4,8 +4,8 @@ import type { Route } from './Route.js'; import { validatePathPattern } from './utils.js'; class RouteHandlerRegistry { - #routes: Map = new Map(); - #routesByMethod: Map = new Map(); + readonly #routes: Map = new Map(); + readonly #routesByMethod: Map = new Map(); readonly #logger: Pick; diff --git a/packages/event-handler/src/rest/utils.ts b/packages/event-handler/src/rest/utils.ts index fad1a6ee79..38f0c7dff8 100644 --- a/packages/event-handler/src/rest/utils.ts +++ b/packages/event-handler/src/rest/utils.ts @@ -1,6 +1,6 @@ import type { Path } from '../types/rest.js'; -const PARAM_PATTERN = /:([a-zA-Z_]\w*)(?=[/]|$)/g; +const PARAM_PATTERN = /:([a-zA-Z_]\w*)(?=\/|$)/g; const SAFE_CHARS = "-._~()'!*:@,;=+&$"; const UNSAFE_CHARS = '%<> \\[\\]{}|^'; From 8bccf05a86b1954a7bb1cb3f87950994cd6f6a78 Mon Sep 17 00:00:00 2001 From: svozza Date: Fri, 25 Jul 2025 15:05:57 +0100 Subject: [PATCH 3/7] use console rather than mock logger in registry tests --- .../unit/rest/RouteHandlerRegistry.test.ts | 53 +++++-------------- 1 file changed, 13 insertions(+), 40 deletions(-) diff --git a/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts b/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts index 839fb027f2..dd13cf800d 100644 --- a/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts +++ b/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts @@ -7,13 +7,7 @@ import type { Path } from '../../../src/types/rest.js'; describe('Class: RouteHandlerRegistry', () => { it('should warn when registering a duplicate route', () => { // Prepare - const mockLogger = { - debug: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - }; - - const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const registry = new RouteHandlerRegistry({ logger: console }); const handler = () => 'test'; const path = '/test'; const method = HttpVerbs.GET; @@ -26,7 +20,7 @@ describe('Class: RouteHandlerRegistry', () => { registry.register(route2); // Assert - expect(mockLogger.warn).toHaveBeenCalledWith( + expect(console.warn).toHaveBeenCalledWith( `Handler for method: ${method} and path: ${path} already exists. The previous handler will be replaced.` ); expect(registry.getRouteCount()).toBe(1); @@ -50,13 +44,7 @@ describe('Class: RouteHandlerRegistry', () => { 'should not register routes with invalid path pattern: $description', ({ path }) => { // Prepare - const mockLogger = { - debug: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - }; - - const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const registry = new RouteHandlerRegistry({ logger: console }); const handler = () => 'test'; const route = new Route(HttpVerbs.GET, path as Path, handler); @@ -65,7 +53,7 @@ describe('Class: RouteHandlerRegistry', () => { registry.register(route); // Assert - expect(mockLogger.warn).toHaveBeenCalledWith( + expect(console.warn).toHaveBeenCalledWith( 'Malformed parameter syntax. Use :paramName format.' ); expect(registry.getRouteCount()).toBe(0); @@ -76,13 +64,7 @@ describe('Class: RouteHandlerRegistry', () => { it('should not register routes with duplicate parameter names', () => { // Prepare - const mockLogger = { - debug: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - }; - - const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const registry = new RouteHandlerRegistry({ logger: console }); const handler = () => 'test'; // Create a route with duplicate parameter names @@ -93,9 +75,7 @@ describe('Class: RouteHandlerRegistry', () => { registry.register(route); // Assert - expect(mockLogger.warn).toHaveBeenCalledWith( - 'Duplicate parameter names: id' - ); + expect(console.warn).toHaveBeenCalledWith('Duplicate parameter names: id'); expect(registry.getRouteCount()).toBe(0); // Route should not be registered expect(registry.getAllRoutes()).toHaveLength(0); expect(registry.getRoutesByMethod(HttpVerbs.GET)).toHaveLength(0); @@ -104,8 +84,7 @@ describe('Class: RouteHandlerRegistry', () => { describe('getRouteCount', () => { it('returns 0 for empty registry', () => { // Prepare - const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; - const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const registry = new RouteHandlerRegistry({ logger: console }); // Act & Assert expect(registry.getRouteCount()).toBe(0); @@ -113,8 +92,7 @@ describe('Class: RouteHandlerRegistry', () => { it('returns correct count after registering routes', () => { // Prepare - const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; - const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const registry = new RouteHandlerRegistry({ logger: console }); const handler = () => 'test'; // Act & Assert @@ -132,8 +110,7 @@ describe('Class: RouteHandlerRegistry', () => { describe('getRoutesByMethod', () => { it('returns empty array for non-existent method', () => { // Prepare - const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; - const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const registry = new RouteHandlerRegistry({ logger: console }); // Act & Assert expect(registry.getRoutesByMethod('GET')).toEqual([]); @@ -147,8 +124,7 @@ describe('Class: RouteHandlerRegistry', () => { { method: HttpVerbs.DELETE }, ])('returns routes for $method method', ({ method }) => { // Prepare - const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; - const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const registry = new RouteHandlerRegistry({ logger: console }); const handler = () => 'test'; const route1 = new Route(method, '/users', handler); @@ -170,8 +146,7 @@ describe('Class: RouteHandlerRegistry', () => { it('handles case-insensitive method lookup', () => { // Prepare - const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; - const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const registry = new RouteHandlerRegistry({ logger: console }); const handler = () => 'test'; const getRoute = new Route(HttpVerbs.GET, '/users', handler); @@ -189,8 +164,7 @@ describe('Class: RouteHandlerRegistry', () => { describe('getAllRoutes', () => { it('returns empty array for empty registry', () => { // Prepare - const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; - const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const registry = new RouteHandlerRegistry({ logger: console }); // Act & Assert expect(registry.getAllRoutes()).toEqual([]); @@ -198,8 +172,7 @@ describe('Class: RouteHandlerRegistry', () => { it('returns all registered routes', () => { // Prepare - const mockLogger = { debug: vi.fn(), warn: vi.fn(), error: vi.fn() }; - const registry = new RouteHandlerRegistry({ logger: mockLogger }); + const registry = new RouteHandlerRegistry({ logger: console }); const handler = () => 'test'; const route1 = new Route(HttpVerbs.GET, '/users', handler); From 7c2c3476d7c4fb6565be4ec1d20a77a68260efb9 Mon Sep 17 00:00:00 2001 From: svozza Date: Fri, 25 Jul 2025 15:10:28 +0100 Subject: [PATCH 4/7] move types and constants to their own files --- packages/event-handler/src/rest/BaseRouter.ts | 2 +- .../src/rest/{constatnts.ts => constants.ts} | 6 ++++++ packages/event-handler/src/rest/utils.ts | 19 ++----------------- packages/event-handler/src/types/rest.ts | 16 +++++++++++++++- .../tests/unit/rest/BaseRouter.test.ts | 2 +- .../unit/rest/RouteHandlerRegistry.test.ts | 2 +- 6 files changed, 26 insertions(+), 21 deletions(-) rename packages/event-handler/src/rest/{constatnts.ts => constants.ts} (56%) diff --git a/packages/event-handler/src/rest/BaseRouter.ts b/packages/event-handler/src/rest/BaseRouter.ts index cc8fc9dcfd..b3eb854166 100644 --- a/packages/event-handler/src/rest/BaseRouter.ts +++ b/packages/event-handler/src/rest/BaseRouter.ts @@ -12,7 +12,7 @@ import type { RouteOptions, RouterOptions, } from '../types/rest.js'; -import { HttpVerbs } from './constatnts.js'; +import { HttpVerbs } from './constants.js'; import { Route } from './Route.js'; import { RouteHandlerRegistry } from './RouteHandlerRegistry.js'; diff --git a/packages/event-handler/src/rest/constatnts.ts b/packages/event-handler/src/rest/constants.ts similarity index 56% rename from packages/event-handler/src/rest/constatnts.ts rename to packages/event-handler/src/rest/constants.ts index 7a62de7cb1..2f4abbd868 100644 --- a/packages/event-handler/src/rest/constatnts.ts +++ b/packages/event-handler/src/rest/constants.ts @@ -9,3 +9,9 @@ export const HttpVerbs = { HEAD: 'HEAD', OPTIONS: 'OPTIONS', } as const; + +export const PARAM_PATTERN = /:([a-zA-Z_]\w*)(?=\/|$)/g; + +export const SAFE_CHARS = "-._~()'!*:@,;=+&$"; + +export const UNSAFE_CHARS = '%<> \\[\\]{}|^'; diff --git a/packages/event-handler/src/rest/utils.ts b/packages/event-handler/src/rest/utils.ts index 38f0c7dff8..5d79396d77 100644 --- a/packages/event-handler/src/rest/utils.ts +++ b/packages/event-handler/src/rest/utils.ts @@ -1,8 +1,5 @@ -import type { Path } from '../types/rest.js'; - -const PARAM_PATTERN = /:([a-zA-Z_]\w*)(?=\/|$)/g; -const SAFE_CHARS = "-._~()'!*:@,;=+&$"; -const UNSAFE_CHARS = '%<> \\[\\]{}|^'; +import type { CompiledRoute, Path, ValidationResult } from '../types/rest.js'; +import { PARAM_PATTERN, SAFE_CHARS, UNSAFE_CHARS } from './constants.js'; export function compilePath(path: Path): CompiledRoute { const paramNames: string[] = []; @@ -22,11 +19,6 @@ export function compilePath(path: Path): CompiledRoute { }; } -type ValidationResult = { - isValid: boolean; - issues: string[]; -}; - export function validatePathPattern(path: Path): ValidationResult { const issues: string[] = []; @@ -51,10 +43,3 @@ export function validatePathPattern(path: Path): ValidationResult { issues, }; } - -interface CompiledRoute { - originalPath: string; - regex: RegExp; - paramNames: string[]; - isDynamic: boolean; -} diff --git a/packages/event-handler/src/types/rest.ts b/packages/event-handler/src/types/rest.ts index 28a701d00f..9b9144f3ff 100644 --- a/packages/event-handler/src/types/rest.ts +++ b/packages/event-handler/src/types/rest.ts @@ -1,6 +1,6 @@ import type { GenericLogger } from '@aws-lambda-powertools/commons/types'; import type { BaseRouter } from '../rest/BaseRouter.js'; -import type { HttpVerbs } from '../rest/constatnts.js'; +import type { HttpVerbs } from '../rest/constants.js'; /** * Options for the {@link BaseRouter} class @@ -14,6 +14,13 @@ type RouterOptions = { logger?: GenericLogger; }; +interface CompiledRoute { + originalPath: string; + regex: RegExp; + paramNames: string[]; + isDynamic: boolean; +} + // biome-ignore lint/suspicious/noExplicitAny: we want to keep arguments and return types as any to accept any type of function type RouteHandler = (...args: T[]) => R; @@ -35,11 +42,18 @@ type RouteRegistryOptions = { logger: Pick; }; +type ValidationResult = { + isValid: boolean; + issues: string[]; +}; + export type { + CompiledRoute, HttpMethod, Path, RouterOptions, RouteHandler, RouteOptions, RouteRegistryOptions, + ValidationResult, }; diff --git a/packages/event-handler/tests/unit/rest/BaseRouter.test.ts b/packages/event-handler/tests/unit/rest/BaseRouter.test.ts index bf5e2c57e5..621a96f525 100644 --- a/packages/event-handler/tests/unit/rest/BaseRouter.test.ts +++ b/packages/event-handler/tests/unit/rest/BaseRouter.test.ts @@ -2,7 +2,7 @@ import context from '@aws-lambda-powertools/testing-utils/context'; import type { Context } from 'aws-lambda'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { BaseRouter } from '../../../src/rest/BaseRouter.js'; -import { HttpVerbs } from '../../../src/rest/constatnts.js'; +import { HttpVerbs } from '../../../src/rest/constants.js'; import type { HttpMethod, RouteHandler, diff --git a/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts b/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts index dd13cf800d..41b2fafd17 100644 --- a/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts +++ b/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest'; -import { HttpVerbs } from '../../../src/rest/constatnts.js'; +import { HttpVerbs } from '../../../src/rest/constants.js'; import { Route } from '../../../src/rest/Route.js'; import { RouteHandlerRegistry } from '../../../src/rest/RouteHandlerRegistry.js'; import type { Path } from '../../../src/types/rest.js'; From 9c381496f851a1450cbe010913d192e07715ad18 Mon Sep 17 00:00:00 2001 From: svozza Date: Fri, 25 Jul 2025 15:28:37 +0100 Subject: [PATCH 5/7] remove unused vitest import --- .../event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts b/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts index 41b2fafd17..ff5f8de8be 100644 --- a/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts +++ b/packages/event-handler/tests/unit/rest/RouteHandlerRegistry.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest'; +import { describe, expect, it } from 'vitest'; import { HttpVerbs } from '../../../src/rest/constants.js'; import { Route } from '../../../src/rest/Route.js'; import { RouteHandlerRegistry } from '../../../src/rest/RouteHandlerRegistry.js'; From 28f46ff3e2d0082ee8768348a9fff038fa18070a Mon Sep 17 00:00:00 2001 From: svozza Date: Fri, 25 Jul 2025 16:41:08 +0100 Subject: [PATCH 6/7] remove null check conditional when pushing to routesByMethod --- packages/event-handler/src/rest/RouteHandlerRegistry.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/event-handler/src/rest/RouteHandlerRegistry.ts b/packages/event-handler/src/rest/RouteHandlerRegistry.ts index 09f6d90f89..2ebd0b45c3 100644 --- a/packages/event-handler/src/rest/RouteHandlerRegistry.ts +++ b/packages/event-handler/src/rest/RouteHandlerRegistry.ts @@ -30,11 +30,9 @@ class RouteHandlerRegistry { this.#routes.set(route.id, route); - if (!this.#routesByMethod.has(route.method)) { - this.#routesByMethod.set(route.method, []); - } - // biome-ignore lint/style/noNonNullAssertion: Map.set operation above ensures Map.get won't return undefined - this.#routesByMethod.get(route.method)!.push(route); + const routesByMethod = this.#routesByMethod.get(route.method) ?? []; + routesByMethod.push(route); + this.#routesByMethod.set(route.method, routesByMethod); } public getRouteCount(): number { From b07208a327be544707ba4798395d9f63a00aad58 Mon Sep 17 00:00:00 2001 From: svozza Date: Fri, 25 Jul 2025 17:29:56 +0100 Subject: [PATCH 7/7] remove registeredAt field in Route class --- packages/event-handler/src/rest/Route.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/event-handler/src/rest/Route.ts b/packages/event-handler/src/rest/Route.ts index 50fd54d62f..e5690787b3 100644 --- a/packages/event-handler/src/rest/Route.ts +++ b/packages/event-handler/src/rest/Route.ts @@ -5,14 +5,12 @@ class Route { readonly method: string; readonly path: Path; readonly handler: RouteHandler; - readonly registeredAt: Date; constructor(method: string, path: Path, handler: RouteHandler) { this.id = `${method}:${path}`; this.method = method.toUpperCase(); this.path = path; this.handler = handler; - this.registeredAt = new Date(); } }