From d8fb0b3ef79470971c7da92ea53b51141379bdf7 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Tue, 30 Jun 2020 12:44:20 +0100 Subject: [PATCH] chore(apigateway): modernize Integration.bind() Currently, the Method construct reaches into the internals of the Integration class to construct itself, via the _props() internal method. Change to a more recent pattern where the bind() returns a result that contains all of the information that the Method class requires to finalize the bind. Motivation for the change This change - https://github.com/aws/aws-cdk/pull/8813 - requires a property returned to the Method that is not user specified. The change already introduces this pattern. This PR takes it a bit further and applies the same logic to existing properties, so that there is now only one way for the Method to get the result of the bind. --- .../aws-apigateway/lib/integration.ts | 60 +++++++++++++++---- .../aws-apigateway/lib/integrations/aws.ts | 6 +- .../aws-apigateway/lib/integrations/lambda.ts | 7 ++- .../@aws-cdk/aws-apigateway/lib/method.ts | 25 ++------ 4 files changed, 63 insertions(+), 35 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/integration.ts b/packages/@aws-cdk/aws-apigateway/lib/integration.ts index d7a9ec74f3b34..9324542eec631 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/integration.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/integration.ts @@ -134,6 +134,35 @@ export interface IntegrationProps { readonly options?: IntegrationOptions; } +/** + * Result of binding an Integration to the Method. + */ +export interface IntegrationConfig { + /** + * Integration options. + * @default - no integration options + */ + readonly options?: IntegrationOptions; + + /** + * Specifies an API method integration type. + */ + readonly type: IntegrationType; + + /** + * The Uniform Resource Identifier (URI) for the integration. + * @see https://docs.aws.amazon.com/apigateway/api-reference/resource/integration/#uri + * @default - no URI. Usually applies to MOCK integration + */ + readonly uri?: string; + + /** + * The integration's HTTP method type. + * @default - no integration method specified. + */ + readonly integrationHttpMethod?: string; +} + /** * Base class for backend integrations for an API Gateway method. * @@ -141,23 +170,32 @@ export interface IntegrationProps { * or implement on your own by specifying the set of props. */ export class Integration { - constructor(private readonly props: IntegrationProps) { } - - /** - * Allows `Method` to access the integration props. - * - * @internal - */ - public get _props() { - return this.props; + constructor(private readonly props: IntegrationProps) { + const options = this.props.options || { }; + if (options.credentialsPassthrough !== undefined && options.credentialsRole !== undefined) { + throw new Error('\'credentialsPassthrough\' and \'credentialsRole\' are mutually exclusive'); + } + + if (options.connectionType === ConnectionType.VPC_LINK && options.vpcLink === undefined) { + throw new Error('\'connectionType\' of VPC_LINK requires \'vpcLink\' prop to be set'); + } + + if (options.connectionType === ConnectionType.INTERNET && options.vpcLink !== undefined) { + throw new Error('cannot set \'vpcLink\' where \'connectionType\' is INTERNET'); + } } /** * Can be overridden by subclasses to allow the integration to interact with the method * being integrated, access the REST API object, method ARNs, etc. */ - public bind(_method: Method) { - return; + public bind(_method: Method): IntegrationConfig { + return { + options: this.props.options, + type: this.props.type, + uri: this.props.uri, + integrationHttpMethod: this.props.integrationHttpMethod, + }; } } diff --git a/packages/@aws-cdk/aws-apigateway/lib/integrations/aws.ts b/packages/@aws-cdk/aws-apigateway/lib/integrations/aws.ts index cd3854e2abd22..1f95b88233ab8 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/integrations/aws.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/integrations/aws.ts @@ -1,5 +1,5 @@ import * as cdk from '@aws-cdk/core'; -import { Integration, IntegrationOptions, IntegrationType } from '../integration'; +import { Integration, IntegrationConfig, IntegrationOptions, IntegrationType } from '../integration'; import { Method } from '../method'; import { parseAwsApiCall } from '../util'; @@ -92,7 +92,9 @@ export class AwsIntegration extends Integration { }); } - public bind(method: Method) { + public bind(method: Method): IntegrationConfig { + const bindResult = super.bind(method); this.scope = method; + return bindResult; } } diff --git a/packages/@aws-cdk/aws-apigateway/lib/integrations/lambda.ts b/packages/@aws-cdk/aws-apigateway/lib/integrations/lambda.ts index fe5e81678e85e..1025734b0ed60 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/integrations/lambda.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/integrations/lambda.ts @@ -1,7 +1,7 @@ import * as iam from '@aws-cdk/aws-iam'; import * as lambda from '@aws-cdk/aws-lambda'; import { Lazy } from '@aws-cdk/core'; -import { IntegrationOptions } from '../integration'; +import { IntegrationConfig, IntegrationOptions } from '../integration'; import { Method } from '../method'; import { AwsIntegration } from './aws'; @@ -52,8 +52,8 @@ export class LambdaIntegration extends AwsIntegration { this.enableTest = options.allowTestInvoke === undefined ? true : false; } - public bind(method: Method) { - super.bind(method); + public bind(method: Method): IntegrationConfig { + const bindResult = super.bind(method); const principal = new iam.ServicePrincipal('apigateway.amazonaws.com'); const desc = `${method.api.node.uniqueId}.${method.httpMethod}.${method.resource.path.replace(/\//g, '.')}`; @@ -72,5 +72,6 @@ export class LambdaIntegration extends AwsIntegration { sourceArn: method.testMethodArn, }); } + return bindResult; } } diff --git a/packages/@aws-cdk/aws-apigateway/lib/method.ts b/packages/@aws-cdk/aws-apigateway/lib/method.ts index 8721d9e9da114..a2d96572bb1f0 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/method.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/method.ts @@ -1,7 +1,7 @@ import { Construct, Resource, Stack } from '@aws-cdk/core'; import { CfnMethod, CfnMethodProps } from './apigateway.generated'; import { Authorizer, IAuthorizer } from './authorizer'; -import { ConnectionType, Integration } from './integration'; +import { Integration } from './integration'; import { MockIntegration } from './integrations/mock'; import { MethodResponse } from './methodresponse'; import { IModel } from './model'; @@ -266,23 +266,10 @@ export class Method extends Resource { return this.renderIntegration(new MockIntegration()); } - integration.bind(this); - - const options = integration._props.options || { }; + const bindResult = integration.bind(this); + const options = bindResult.options || { }; let credentials; - if (options.credentialsPassthrough !== undefined && options.credentialsRole !== undefined) { - throw new Error('\'credentialsPassthrough\' and \'credentialsRole\' are mutually exclusive'); - } - - if (options.connectionType === ConnectionType.VPC_LINK && options.vpcLink === undefined) { - throw new Error('\'connectionType\' of VPC_LINK requires \'vpcLink\' prop to be set'); - } - - if (options.connectionType === ConnectionType.INTERNET && options.vpcLink !== undefined) { - throw new Error('cannot set \'vpcLink\' where \'connectionType\' is INTERNET'); - } - if (options.credentialsRole) { credentials = options.credentialsRole.roleArn; } else if (options.credentialsPassthrough) { @@ -292,12 +279,12 @@ export class Method extends Resource { } return { - type: integration._props.type, - uri: integration._props.uri, + type: bindResult.type, + uri: bindResult.uri, cacheKeyParameters: options.cacheKeyParameters, cacheNamespace: options.cacheNamespace, contentHandling: options.contentHandling, - integrationHttpMethod: integration._props.integrationHttpMethod, + integrationHttpMethod: bindResult.integrationHttpMethod, requestParameters: options.requestParameters, requestTemplates: options.requestTemplates, passthroughBehavior: options.passthroughBehavior,