Improved jsdocs and added utility function for prefixed path resolution

Author: sdangol

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<string, Route> = 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
         )

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 {

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;
+};

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.'
+    );
   });
 });

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);
+    });
+  });
 });