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.

Sign up for GitHub

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

fix(dynamodb): allow using PhysicalName.GENERATE_IF_NEEDED as the Table name #9377

Merged
merged 2 commits into from
Aug 2, 2020
Merged

fix(dynamodb): allow using PhysicalName.GENERATE_IF_NEEDED as the Table name #9377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ca5e2d
fix(dynamodb): allow using PhysicalName.GENERATE_IF_NEEDED as he Tabl…
skinny85 Jul 31, 2020
6253108
Merge branch 'master' into fix/dynamodb-physical-name-if-required-as-…
mergify[bot] Aug 2, 2020
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
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-dynamodb/lib/table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -913,15 +913,15 @@ export class Table extends TableBase {

this.encryptionKey = encryptionKey;

if (props.tableName) { this.node.addMetadata('aws:cdk:hasPhysicalName', props.tableName); }

this.tableArn = this.getResourceArnAttribute(this.table.attrArn, {
service: 'dynamodb',
resource: 'table',
resourceName: this.physicalName,
});
this.tableName = this.getResourceNameAttribute(this.table.ref);

if (props.tableName) { this.node.addMetadata('aws:cdk:hasPhysicalName', this.tableName); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my own understanding, what is this? This is the only place in the CDK this pattern is used

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was an old idea to mark things with physical names set explicitly in our L2s. I don't think it was adapted widely.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it was adapted widely.
Screen Shot 2020-08-03 at 8 02 06 AM


this.tableStreamArn = streamSpecification ? this.table.attrStreamArn : undefined;

this.scalingRole = this.makeScalingRole();
Expand Down
37 changes: 20 additions & 17 deletions packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { ResourcePart, SynthUtils } from '@aws-cdk/assert';
import { ABSENT, ResourcePart, SynthUtils } from '@aws-cdk/assert';
import '@aws-cdk/assert/jest';
import * as appscaling from '@aws-cdk/aws-applicationautoscaling';
import * as iam from '@aws-cdk/aws-iam';
import * as kms from '@aws-cdk/aws-kms';
import { App, CfnDeletionPolicy, ConstructNode, Duration, RemovalPolicy, Stack, Tag } from '@aws-cdk/core';
import { App, CfnDeletionPolicy, ConstructNode, Duration, PhysicalName, RemovalPolicy, Stack, Tag } from '@aws-cdk/core';
import {
Attribute,
AttributeType,
Expand Down Expand Up @@ -67,8 +67,12 @@ function* LSI_GENERATOR(): Generator<LocalSecondaryIndexProps, never> {
}

describe('default properties', () => {
let stack: Stack;
beforeEach(() => {
stack = new Stack();
});

test('hash key only', () => {
const stack = new Stack();
new Table(stack, CONSTRUCT_NAME, { partitionKey: TABLE_PARTITION_KEY });

expect(stack).toHaveResource('AWS::DynamoDB::Table', {
Expand All @@ -82,15 +86,13 @@ describe('default properties', () => {
});

test('removalPolicy is DESTROY', () => {
const stack = new Stack();
new Table(stack, CONSTRUCT_NAME, { partitionKey: TABLE_PARTITION_KEY, removalPolicy: RemovalPolicy.DESTROY });

expect(stack).toHaveResource('AWS::DynamoDB::Table', { DeletionPolicy: CfnDeletionPolicy.DELETE }, ResourcePart.CompleteDefinition);

});

test('hash + range key', () => {
const stack = new Stack();
new Table(stack, CONSTRUCT_NAME, {
partitionKey: TABLE_PARTITION_KEY,
sortKey: TABLE_SORT_KEY,
Expand All @@ -110,8 +112,6 @@ describe('default properties', () => {
});

test('hash + range key can also be specified in props', () => {
const stack = new Stack();

new Table(stack, CONSTRUCT_NAME, {
partitionKey: TABLE_PARTITION_KEY,
sortKey: TABLE_SORT_KEY,
Expand All @@ -132,7 +132,6 @@ describe('default properties', () => {
});

test('point-in-time recovery is not enabled', () => {
const stack = new Stack();
new Table(stack, CONSTRUCT_NAME, {
partitionKey: TABLE_PARTITION_KEY,
sortKey: TABLE_SORT_KEY,
Expand All @@ -154,7 +153,6 @@ describe('default properties', () => {
});

test('server-side encryption is not enabled', () => {
const stack = new Stack();
new Table(stack, CONSTRUCT_NAME, {
partitionKey: TABLE_PARTITION_KEY,
sortKey: TABLE_SORT_KEY,
Expand All @@ -176,7 +174,6 @@ describe('default properties', () => {
});

test('stream is not enabled', () => {
const stack = new Stack();
new Table(stack, CONSTRUCT_NAME, {
partitionKey: TABLE_PARTITION_KEY,
sortKey: TABLE_SORT_KEY,
Expand All @@ -198,7 +195,6 @@ describe('default properties', () => {
});

test('ttl is not enabled', () => {
const stack = new Stack();
new Table(stack, CONSTRUCT_NAME, {
partitionKey: TABLE_PARTITION_KEY,
sortKey: TABLE_SORT_KEY,
Expand All @@ -220,8 +216,6 @@ describe('default properties', () => {
});

test('can specify new and old images', () => {
const stack = new Stack();

new Table(stack, CONSTRUCT_NAME, {
tableName: TABLE_NAME,
readCapacity: 42,
Expand Down Expand Up @@ -249,8 +243,6 @@ describe('default properties', () => {
});

test('can specify new images only', () => {
const stack = new Stack();

new Table(stack, CONSTRUCT_NAME, {
tableName: TABLE_NAME,
readCapacity: 42,
Expand Down Expand Up @@ -278,8 +270,6 @@ describe('default properties', () => {
});

test('can specify old images only', () => {
const stack = new Stack();

new Table(stack, CONSTRUCT_NAME, {
tableName: TABLE_NAME,
readCapacity: 42,
Expand All @@ -305,6 +295,19 @@ describe('default properties', () => {
},
);
});

test('can use PhysicalName.GENERATE_IF_NEEDED as the Table name', () => {
new Table(stack, CONSTRUCT_NAME, {
tableName: PhysicalName.GENERATE_IF_NEEDED,
partitionKey: TABLE_PARTITION_KEY,
});

// since the resource has not been used in a cross-environment manner,
// so the name should not be filled
expect(stack).toHaveResourceLike('AWS::DynamoDB::Table', {
TableName: ABSENT,
});
});
});

test('when specifying every property', () => {
Expand Down