Skip to content

Commit

Permalink
chore(apigateway): modernize Integration.bind()
Browse files Browse the repository at this point in the history
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 - #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.
  • Loading branch information
Niranjan Jayakar committed Jun 30, 2020
1 parent c337d4a commit d8fb0b3
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 35 deletions.
60 changes: 49 additions & 11 deletions packages/@aws-cdk/aws-apigateway/lib/integration.ts
Expand Up @@ -134,30 +134,68 @@ 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.
*
* Use one of the concrete classes such as `MockIntegration`, `AwsIntegration`, `LambdaIntegration`
* 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,
};
}
}

Expand Down
6 changes: 4 additions & 2 deletions 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';

Expand Down Expand Up @@ -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;
}
}
7 changes: 4 additions & 3 deletions 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';

Expand Down Expand Up @@ -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, '.')}`;
Expand All @@ -72,5 +72,6 @@ export class LambdaIntegration extends AwsIntegration {
sourceArn: method.testMethodArn,
});
}
return bindResult;
}
}
25 changes: 6 additions & 19 deletions 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';
Expand Down Expand Up @@ -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) {
Expand All @@ -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,
Expand Down

0 comments on commit d8fb0b3

Please sign in to comment.