Stop false sharing, make addMethodResponse merge response objects

aws · Aug 11, 2023 · 69c5847 · 69c5847
1 parent 870a143
commit 69c5847
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 21 deletions.
diff --git a/packages/aws-cdk-lib/aws-apigateway/lib/method.ts b/packages/aws-cdk-lib/aws-apigateway/lib/method.ts
@@ -173,7 +173,7 @@ export class Method extends Resource {
    */
   public readonly api: IRestApi;
 
-  private readonly methodResponses: MethodResponse[];
+  private readonly methodResponses: MethodResponse[] = [];
 
   constructor(scope: Construct, id: string, props: MethodProps) {
     super(scope, id);
@@ -203,7 +203,9 @@ export class Method extends Resource {
       authorizer._attachToApi(this.api);
     }
 
-    this.methodResponses = options.methodResponses ?? defaultMethodOptions.methodResponses ?? [];
+    for (const mr of options.methodResponses ?? defaultMethodOptions.methodResponses ?? []) {
+      this.addMethodResponse(mr);
+    }
 
     const integration = props.integration ?? this.resource.defaultIntegration ?? new MockIntegration();
     const bindResult = integration.bind(this);
@@ -242,22 +244,6 @@ export class Method extends Resource {
         },
       });
     }
-
-    this.node.addValidation({
-      validate: () => {
-        const responses = this.methodResponses;
-
-        // Validate that all method response codes are unique
-        const counts: Record<string, number> = {};
-        for (const resp of responses) {
-          counts[resp.statusCode] = (counts[resp.statusCode] ?? 0) + 1;
-        }
-        const dupes = Object.entries(counts).filter(([_, n]) => n > 1).map(([c, _]) => c);
-        return dupes.map((code) =>
-          `${counts[code]} methodResponses for code ${code}: ${this.methodResponses.filter((r) => r.statusCode === code).map((x) => JSON.stringify(x)).join(', ')}`,
-        );
-      },
-    });
   }
 
   /**
@@ -294,9 +280,23 @@ export class Method extends Resource {
 
   /**
    * Add a method response to this method
+   *
+   * If a method response for the same status code already exists, the `responseModels`
+   * and `responseParameters` maps will be merged.
    */
   public addMethodResponse(methodResponse: MethodResponse): void {
-    this.methodResponses.push(methodResponse);
+    const i = this.methodResponses.findIndex((mr) => mr.statusCode === methodResponse.statusCode);
+    if (i >= 0) {
+      // Need to do a splice because MethodResponses are immutable
+      const existing = this.methodResponses[i];
+      this.methodResponses.splice(i, 1, {
+        statusCode: methodResponse.statusCode,
+        responseModels: mergeDicts(existing.responseModels, methodResponse.responseModels),
+        responseParameters: mergeDicts(existing.responseParameters, methodResponse.responseParameters),
+      });
+    } else {
+      this.methodResponses.push(methodResponse);
+    }
   }
 
   private renderIntegration(bindResult: IntegrationConfig): CfnMethod.IntegrationProperty {
@@ -519,3 +519,7 @@ export enum AuthorizationType {
 function pathForArn(path: string): string {
   return path.replace(/\{[^\}]*\}/g, '*'); // replace path parameters (like '{bookId}') with asterisk
 }
+
+function mergeDicts<T>(xs?: Record<string, T>, ys?: Record<string, T>): Record<string, T> | undefined {
+  return xs || ys ? Object.assign(xs ?? {}, ys) : undefined;
+}
diff --git a/packages/aws-cdk-lib/aws-apigateway/test/integrations/stepfunctions.test.ts b/packages/aws-cdk-lib/aws-apigateway/test/integrations/stepfunctions.test.ts
@@ -373,7 +373,7 @@ describe('StepFunctionsIntegration', () => {
     });
   });
 
-  test('with custom methodOptions and integrationOptions', () => {
+  test('merging methodOptions.methodResponses, and not susceptible to false sharing of arrays', () => {
     //GIVEN
     const { stack, api, stateMachine } = givenSetup();
 
@@ -402,7 +402,7 @@ describe('StepFunctionsIntegration', () => {
 
     const integ = apigw.StepFunctionsIntegration.startExecution(stateMachine, integrationOptions);
     api.root.addMethod('GET', integ, methodOptions);
-    // api.root.addMethod('POST', integ, methodOptions);
+    api.root.addMethod('POST', integ, methodOptions);
 
     //THEN
     Template.fromStack(stack).resourceCountIs('AWS::ApiGateway::Method', 2);