Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(all): reduce cognitive complexity, distribute prop checking code #1016

Merged
merged 7 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ export class AlbToFargate extends Construct {

constructor(scope: Construct, id: string, props: AlbToFargateProps) {
super(scope, id);
defaults.CheckProps(props);
defaults.CheckAlbProps(props);
defaults.CheckFargateProps(props);
defaults.CheckVpcProps(props);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ export class AlbToLambda extends Construct {

constructor(scope: Construct, id: string, props: AlbToLambdaProps) {
super(scope, id);
defaults.CheckProps(props);
defaults.CheckAlbProps(props);
defaults.CheckVpcProps(props);
defaults.CheckLambdaProps(props);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ export class ApiGatewayToDynamoDB extends Construct {
*/
constructor(scope: Construct, id: string, props: ApiGatewayToDynamoDBProps) {
super(scope, id);
defaults.CheckProps(props);
defaults.CheckDynamoDBProps(props);

if (this.CheckCreateRequestProps(props)) {
throw new Error(`The 'allowCreateOperation' property must be set to true when setting any of the following: ` +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -742,4 +742,35 @@ test('Construct throws error when deleteIntegrationResponses is set and allowDel
});

expect(app).toThrowError(/The 'allowDeleteOperation' property must be set to true when setting any of the following: 'deleteRequestTemplate', 'additionalDeleteRequestTemplates', 'deleteIntegrationResponses'/);
});
});

test('Test that CheckDynamoDBProps is getting called', () => {
const stack = new Stack();
const tableName = 'custom-table-name';

const existingTable = new ddb.Table(stack, 'MyTablet', {
tableName,
partitionKey: {
name: 'id',
type: ddb.AttributeType.STRING
}
});

const props: ApiGatewayToDynamoDBProps = {
existingTableObj: existingTable,
dynamoTableProps: {
tableName,
partitionKey: {
name: 'id',
type: ddb.AttributeType.STRING
},
},
};

const app = () => {
new ApiGatewayToDynamoDB(stack, 'test-apigateway-dynamodb-stack', props);
};

// Assertion
expect(app).toThrowError(/Error - Either provide existingTableObj or dynamoTableProps, but not both.\n/);
});
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ export class ApiGatewayToIot extends Construct {
*/
constructor(scope: Construct, id: string, props: ApiGatewayToIotProps) {
super(scope, id);
defaults.CheckProps(props);

// Assignment to local member variables to make these available to all member methods of the class.
// (Split the string just in case user supplies fully qualified endpoint eg. ab123cdefghij4l-ats.iot.ap-south-1.amazonaws.com)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ import * as api from 'aws-cdk-lib/aws-apigateway';
import * as iam from 'aws-cdk-lib/aws-iam';
import { Template } from "aws-cdk-lib/assertions";

// --------------------------------------------------------------
// Check for ApiGateway params
// --------------------------------------------------------------
test('Test for default Params construct props', () => {
// Initial Setup
const stack = new cdk.Stack();
Expand All @@ -35,9 +32,6 @@ test('Test for default Params construct props', () => {
expect(construct.apiGatewayRole).not.toBeNull();
});

// --------------------------------------------------------------
// Check for Default IAM Role
// --------------------------------------------------------------
test('Test for default IAM Role', () => {
// Initial Setup
const stack = new cdk.Stack();
Expand Down Expand Up @@ -122,9 +116,6 @@ test('Test for default IAM Role', () => {
});
});

// --------------------------------------------------------------
// Check for Request Validator
// --------------------------------------------------------------
test('Test for default Params request validator', () => {
// Initial Setup
const stack = new cdk.Stack();
Expand All @@ -140,9 +131,6 @@ test('Test for default Params request validator', () => {
});
});

// --------------------------------------------------------------
// Check for Integ Props and Method Props
// --------------------------------------------------------------
test('Test for default Params Integ Props and Method Props', () => {
// Initial Setup
const stack = new cdk.Stack();
Expand Down Expand Up @@ -234,10 +222,6 @@ test('Test for default Params Integ Props and Method Props', () => {
}
});
});

// --------------------------------------------------------------
// Check for valid IoT Endpoint
// --------------------------------------------------------------
test('Test for valid iot endpoint', () => {
// Initial Setup
const stack = new cdk.Stack();
Expand All @@ -252,9 +236,6 @@ test('Test for valid iot endpoint', () => {
expect(app).toThrowError();
});

// --------------------------------------------------------------
// Check for binaryMediaTypes
// --------------------------------------------------------------
test('Test for Binary Media types', () => {
// Stack
const stack = new cdk.Stack();
Expand All @@ -272,9 +253,6 @@ test('Test for Binary Media types', () => {
});
});

// --------------------------------------------------------------
// Check for Api Name and Desc
// --------------------------------------------------------------
test('Test for Api Name and Desc', () => {
// Stack
const stack = new cdk.Stack();
Expand All @@ -296,9 +274,6 @@ test('Test for Api Name and Desc', () => {
});
});

// --------------------------------------------------------------
// Check for Overridden IAM Role
// --------------------------------------------------------------
test('Test for overridden IAM Role', () => {
// Initial Setup
const stack = new cdk.Stack();
Expand Down Expand Up @@ -407,9 +382,6 @@ test('Test for overridden IAM Role', () => {
});
});

// --------------------------------------------------------------
// Check for Api Key Source
// --------------------------------------------------------------
test('Test for APi Key Source', () => {
// Stack
const stack = new cdk.Stack();
Expand All @@ -430,9 +402,6 @@ test('Test for APi Key Source', () => {
});
});

// --------------------------------------------------------------
// Check for Api Key Creation
// --------------------------------------------------------------
test('Test for Api Key Creation', () => {
// Initial Setup
const stack = new cdk.Stack();
Expand Down Expand Up @@ -467,9 +436,6 @@ test('Test for Api Key Creation', () => {
});
});

// -----------------------------------------------------------------
// Test deployment for ApiGateway endPointCongiurationOverride
// -----------------------------------------------------------------
test('Test for deployment ApiGateway AuthorizationType override', () => {
// Stack
const stack = new cdk.Stack();
Expand All @@ -491,9 +457,6 @@ test('Test for deployment ApiGateway AuthorizationType override', () => {
});
});

// -----------------------------------------------------------------
// Test deployment for override ApiGateway AuthorizationType to NONE
// -----------------------------------------------------------------
test('Test for deployment ApiGateway AuthorizationType override', () => {
// Stack
const stack = new cdk.Stack();
Expand All @@ -514,9 +477,6 @@ test('Test for deployment ApiGateway AuthorizationType override', () => {
});
});

// -----------------------------------------------------------------
// Test deployment for fully qualified iotEndpoint name
// -----------------------------------------------------------------
test('Test for handling fully qualified iotEndpoint', () => {
// Stack
const stack = new cdk.Stack();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export class ApiGatewayToKinesisStreams extends Construct {
*/
constructor(scope: Construct, id: string, props: ApiGatewayToKinesisStreamsProps) {
super(scope, id);
defaults.CheckProps(props);
defaults.CheckKinesisStreamProps(props);

// Setup the Kinesis stream
this.kinesisStream = defaults.buildKinesisStream(scope, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,10 @@

// Imports
import { Stack, Duration } from 'aws-cdk-lib';
import { ApiGatewayToKinesisStreams } from '../lib';
import { ApiGatewayToKinesisStreams, ApiGatewayToKinesisStreamsProps } from '../lib';
import * as kinesis from 'aws-cdk-lib/aws-kinesis';
import { Template } from 'aws-cdk-lib/assertions';

// --------------------------------------------------------------
// Test construct properties
// --------------------------------------------------------------
test('Test construct properties', () => {
const stack = new Stack();
const pattern = new ApiGatewayToKinesisStreams(stack, 'api-gateway-kinesis', {});
Expand All @@ -32,9 +29,6 @@ test('Test construct properties', () => {
expect(pattern.cloudwatchAlarms !== null);
});

// --------------------------------------------------------------
// Test deployment w/ overwritten properties
// --------------------------------------------------------------
test('Test deployment w/ overwritten properties', () => {
const stack = new Stack();

Expand Down Expand Up @@ -87,9 +81,6 @@ test('Test deployment w/ overwritten properties', () => {
template.resourceCountIs('AWS::CloudWatch::Alarm', 2);
});

// --------------------------------------------------------------
// Test deployment w/ existing stream without default cloudwatch alarms
// --------------------------------------------------------------
test('Test deployment w/ existing stream', () => {
const stack = new Stack();

Expand Down Expand Up @@ -232,4 +223,18 @@ test('Construct uses custom putRecordsIntegrationResponses property', () => {
]
}
});
});
});

test('Confirm that CheckKinesisStreamProps is called', () => {
const stack = new Stack();

const props: ApiGatewayToKinesisStreamsProps = {
existingStreamObj: new kinesis.Stream(stack, 'test', {}),
kinesisStreamProps: {}
};

const app = () => {
new ApiGatewayToKinesisStreams(stack, 'test-eventbridge-kinesisstreams', props);
};
expect(app).toThrowError();
});
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ export class ApiGatewayToLambda extends Construct {
*/
constructor(scope: Construct, id: string, props: ApiGatewayToLambdaProps) {
super(scope, id);
defaults.CheckProps(props);
defaults.CheckLambdaProps(props);

// Setup the Lambda function
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ import * as api from 'aws-cdk-lib/aws-apigateway';
import { Template } from "aws-cdk-lib/assertions";
import * as defaults from '@aws-solutions-constructs/core';

// --------------------------------------------------------------
// Test for error with existingLambdaObj=undefined (not supplied by user).
// --------------------------------------------------------------
test('Error on existingLambdaObj=undefined', () => {
// Initial Setup
const stack = new Stack();
Expand Down Expand Up @@ -63,9 +60,6 @@ test('Test with lambdaFunctionProps', () => {
});
});

// --------------------------------------------------------------
// Test getter methods
// --------------------------------------------------------------
test('Test properties', () => {
// Initial Setup
const stack = new Stack();
Expand All @@ -85,9 +79,6 @@ test('Test properties', () => {
expect(app.apiGatewayLogGroup !== null);
});

// --------------------------------------------------------------
// Test for error with lambdaFunctionProps=undefined (not supplied by user).
// --------------------------------------------------------------
test('Error on lambdaFunctionProps=undefined', () => {
// Initial Setup
const stack = new Stack();
Expand All @@ -100,9 +91,6 @@ test('Error on lambdaFunctionProps=undefined', () => {
expect(app).toThrowError();
});

// -----------------------------------------------------------------
// Test deployment for override ApiGateway AuthorizationType to NONE
// -----------------------------------------------------------------
test('Test deployment ApiGateway AuthorizationType override', () => {
// Stack
const stack = new Stack();
Expand All @@ -127,9 +115,6 @@ test('Test deployment ApiGateway AuthorizationType override', () => {
});
});

// -----------------------------------------------------------------
// Test deployment for override ApiGateway cloudWatchRole = false
// -----------------------------------------------------------------
test('Test deployment ApiGateway override cloudWatchRole = false', () => {
// Stack
const stack = new Stack();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export class ApiGatewayToSageMakerEndpoint extends Construct {
*/
constructor(scope: Construct, id: string, props: ApiGatewayToSageMakerEndpointProps) {
super(scope, id);
defaults.CheckProps(props);
// CheckSagemakerProps is not called because this construct can't create a Sagemaker resource

// Setup the API Gateway
const globalRestApiResponse = defaults.GlobalRestApi(this, props.apiGatewayProps, props.logGroupProps);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ import { ApiGatewayToSageMakerEndpoint } from '../lib';
import * as iam from 'aws-cdk-lib/aws-iam';
import { Template } from 'aws-cdk-lib/assertions';

// --------------------------------------------------------------
// Test construct properties
// --------------------------------------------------------------
test('Test construct properties', () => {
const stack = new Stack();
const pattern = new ApiGatewayToSageMakerEndpoint(stack, 'api-gateway-sagemakerendpoint', {
Expand All @@ -34,9 +31,6 @@ test('Test construct properties', () => {
expect(pattern.apiGatewayLogGroup !== null);
});

// --------------------------------------------------------------
// Test deployment w/ overwritten properties
// --------------------------------------------------------------
test('Test deployment w/ overwritten properties', () => {
const stack = new Stack();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,6 @@ export class ApiGatewayToSqs extends Construct {
*/
constructor(scope: Construct, id: string, props: ApiGatewayToSqsProps) {
super(scope, id);
defaults.CheckProps(props);
defaults.CheckSqsProps(props);

if (this.CheckCreateRequestProps(props)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ export class CloudFrontToApiGatewayToLambda extends Construct {
*/
constructor(scope: Construct, id: string, props: CloudFrontToApiGatewayToLambdaProps) {
super(scope, id);
defaults.CheckProps(props);
defaults.CheckLambdaProps(props);
// CheckCloudFrontProps() is called by internal aws-cloudfront-apigateway construct

this.lambdaFunction = defaults.buildLambdaFunction(this, {
existingLambdaObj: props.existingLambdaObj,
Expand Down
Loading