Skip to content

Commit

Permalink
chore(all): reduce cognitive complexity, distribute prop checking code (
Browse files Browse the repository at this point in the history
#1016)

* Finish emptying CheckProps()

* Remove CheckProps

* Typo

* remove stray imports

* Distribute and remove input-valdation.test.ts

* Remove redundant comments

* minor eslint format error
  • Loading branch information
biffgaut authored Sep 25, 2023
1 parent 035ea20 commit 1a15512
Show file tree
Hide file tree
Showing 141 changed files with 1,587 additions and 1,588 deletions.
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

0 comments on commit 1a15512

Please sign in to comment.