Ref: Drop Endpoint::_setSiblingMethods() (#1539)

Getting rid of some ugliness.

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## Version 16
 
+### v16.6.2
+
+- Internal method `Endpoint::_setSiblingMethods()` removed (since v8.4.1);
+- The public property `DependsOnMethod::endpoints` is deprecated and will be removed in v17.
+
 ### v16.6.1
 
 - Performance fix for uploads processing (since v16.1.0).

src/depends-on-method.ts

@@ -1,8 +1,24 @@
+import { head, tail, toPairs } from "ramda";
 import { AbstractEndpoint } from "./endpoint";
 import { Method } from "./method";
 
 export class DependsOnMethod {
+  public readonly pairs: [Method, AbstractEndpoint][];
+  public readonly firstEndpoint: AbstractEndpoint | undefined;
+  public readonly siblingMethods: Method[];
+
   constructor(
+    /**
+     * @deprecated use pairs instead
+     * @todo remove from public in v17
+     * */
     public readonly endpoints: Partial<Record<Method, AbstractEndpoint>>,
-  ) {}
+  ) {
+    this.pairs = toPairs(endpoints).filter(
+      (pair): pair is [Method, AbstractEndpoint] =>
+        pair !== undefined && pair[1] !== undefined,
+    );
+    this.firstEndpoint = head(this.pairs)?.[1];
+    this.siblingMethods = tail(this.pairs).map(([method]) => method);
+  }
 }

src/endpoint.ts

@@ -47,6 +47,7 @@ export abstract class AbstractEndpoint {
     response: Response;
     logger: AbstractLogger;
     config: CommonConfig;
+    siblingMethods?: Method[];
   }): Promise<void>;
   public abstract getDescription(
     variant: DescriptionVariant,
@@ -59,7 +60,6 @@ export abstract class AbstractEndpoint {
   public abstract getSecurity(): LogicalContainer<Security>;
   public abstract getScopes(): string[];
   public abstract getTags(): string[];
-  public abstract _setSiblingMethods(methods: Method[]): void;
   public abstract getOperationId(method: Method): string | undefined;
 }
 
@@ -81,7 +81,6 @@ export class Endpoint<
   readonly #scopes: SCO[];
   readonly #tags: TAG[];
   readonly #getOperationId: (method: Method) => string | undefined;
-  #siblingMethods: Method[] = [];
 
   constructor({
     methods,
@@ -155,14 +154,6 @@ export class Endpoint<
     };
   }
 
-  /**
-   * @desc Sets the other methods supported by the same path. Used by Routing in DependsOnMethod case, for options.
-   * @deprecated This method is for internal needs of the library, please avoid using it.
-   * */
-  public override _setSiblingMethods(methods: Method[]): void {
-    this.#siblingMethods = methods;
-  }
-
   public override getDescription(variant: DescriptionVariant) {
     return this.#descriptions[variant];
   }
@@ -211,9 +202,9 @@ export class Endpoint<
     return this.#getOperationId(method);
   }
 
-  #getDefaultCorsHeaders(): Record<string, string> {
+  #getDefaultCorsHeaders(siblingMethods: Method[]): Record<string, string> {
     const accessMethods = (this.#methods as Array<Method | AuxMethod>)
-      .concat(this.#siblingMethods)
+      .concat(siblingMethods)
       .concat("options")
       .join(", ")
       .toUpperCase();
@@ -350,17 +341,19 @@ export class Endpoint<
     response,
     logger,
     config,
+    siblingMethods = [],
   }: {
     request: Request;
     response: Response;
     logger: AbstractLogger;
     config: CommonConfig;
+    siblingMethods?: Method[];
   }) {
     const method = getActualMethod(request);
     let output: FlatObject | null = null;
     let error: Error | null = null;
     if (config.cors) {
-      let headers = this.#getDefaultCorsHeaders();
+      let headers = this.#getDefaultCorsHeaders(siblingMethods);
       if (typeof config.cors === "function") {
         headers = await config.cors({
           request,

src/routing-walker.ts

@@ -12,6 +12,7 @@ export interface RoutingWalkerParams {
     endpoint: AbstractEndpoint,
     path: string,
     method: Method | AuxMethod,
+    siblingMethods?: Method[],
   ) => void;
   onStatic?: (path: string, handler: StaticHandler) => void;
   parentPath?: string;
@@ -48,22 +49,22 @@ export const walkRouting = ({
         element.apply(path, onStatic);
       }
     } else if (element instanceof DependsOnMethod) {
-      Object.entries(element.endpoints).forEach(([method, endpoint]) => {
+      for (const [method, endpoint] of element.pairs) {
         assert(
-          endpoint.getMethods().includes(method as Method),
+          endpoint.getMethods().includes(method),
           new RoutingError(
             `Endpoint assigned to ${method} method of ${path} must support ${method} method.`,
           ),
         );
-        onEndpoint(endpoint, path, method as Method);
-      });
-      if (hasCors && Object.keys(element.endpoints).length > 0) {
-        const [firstMethod, ...siblingMethods] = Object.keys(
-          element.endpoints,
-        ) as Method[];
-        const firstEndpoint = element.endpoints[firstMethod]!;
-        firstEndpoint._setSiblingMethods(siblingMethods);
-        onEndpoint(firstEndpoint, path, "options");
+        onEndpoint(endpoint, path, method);
+      }
+      if (hasCors && element.firstEndpoint) {
+        onEndpoint(
+          element.firstEndpoint,
+          path,
+          "options",
+          element.siblingMethods,
+        );
       }
     } else {
       walkRouting({

src/routing.ts

@@ -29,13 +29,19 @@ export const initRouting = ({
   walkRouting({
     routing,
     hasCors: !!config.cors,
-    onEndpoint: (endpoint, path, method) => {
+    onEndpoint: (endpoint, path, method, siblingMethods) => {
       app[method](path, async (request, response) => {
         const logger = config.childLoggerProvider
           ? await config.childLoggerProvider({ request, parent: rootLogger })
           : rootLogger;
         logger.info(`${request.method}: ${path}`);
-        await endpoint.execute({ request, response, logger, config });
+        await endpoint.execute({
+          request,
+          response,
+          logger,
+          config,
+          siblingMethods,
+        });
       });
     },
     onStatic: (path, handler) => {

tests/unit/depends-on-method.spec.ts

@@ -1,5 +1,6 @@
 import { z } from "zod";
 import {
+  AbstractEndpoint,
   DependsOnMethod,
   EndpointsFactory,
   defaultResultHandler,
@@ -10,7 +11,9 @@ describe("DependsOnMethod", () => {
   test("should accept empty object", () => {
     const instance = new DependsOnMethod({});
     expect(instance).toBeInstanceOf(DependsOnMethod);
-    expect(instance.endpoints).toEqual({});
+    expect(instance.firstEndpoint).toBeUndefined();
+    expect(instance.siblingMethods).toEqual([]);
+    expect(instance.pairs).toEqual([]);
   });
 
   test("should accept an endpoint with a corresponding method", () => {
@@ -23,7 +26,9 @@ describe("DependsOnMethod", () => {
       }),
     });
     expect(instance).toBeInstanceOf(DependsOnMethod);
-    expect(instance.endpoints).toHaveProperty("post");
+    expect(instance.firstEndpoint).toBeInstanceOf(AbstractEndpoint);
+    expect(instance.siblingMethods).toEqual([]);
+    expect(instance.pairs).toHaveLength(1);
   });
 
   test("should accept an endpoint with additional methods", () => {
@@ -38,7 +43,18 @@ describe("DependsOnMethod", () => {
       post: endpoint,
     });
     expect(instance).toBeInstanceOf(DependsOnMethod);
-    expect(instance.endpoints).toHaveProperty("get");
-    expect(instance.endpoints).toHaveProperty("post");
+    expect(instance.firstEndpoint).toBe(endpoint);
+    expect(instance.siblingMethods).toEqual(["post"]);
+    expect(instance.pairs).toHaveLength(2);
+  });
+
+  test("should reject empty assignments", () => {
+    const instance = new DependsOnMethod({
+      get: undefined,
+      post: undefined,
+    });
+    expect(instance.pairs).toEqual([]);
+    expect(instance.firstEndpoint).toBeUndefined();
+    expect(instance.siblingMethods).toEqual([]);
   });
 });