Skip to content

Commit

Permalink
fix(api): handle attribute type change on gsi
Browse files Browse the repository at this point in the history
  • Loading branch information
sundersc committed May 9, 2024
1 parent 5c91370 commit 43cbb3d
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 42 deletions.
28 changes: 14 additions & 14 deletions codebuild_specs/e2e_workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1107,66 +1107,66 @@ batch:
USE_PARENT_ACCOUNT: 1
depend-on:
- publish_to_local_registry
- identifier: add_resources_custom_logic_data_construct_ddb_iam_access
- identifier: add_resources_amplify_table_5_custom_logic_data_construct
buildspec: codebuild_specs/run_cdk_tests.yml
env:
compute-type: BUILD_GENERAL1_MEDIUM
variables:
TEST_SUITE: >-
src/__tests__/add-resources.test.ts|src/__tests__/custom-logic.test.ts|src/__tests__/data-construct.test.ts|src/__tests__/ddb-iam-access.test.ts
CLI_REGION: sa-east-1
src/__tests__/add-resources.test.ts|src/__tests__/amplify-table-5.test.ts|src/__tests__/custom-logic.test.ts|src/__tests__/data-construct.test.ts
CLI_REGION: ap-east-1
depend-on:
- publish_to_local_registry
- identifier: >-
3_gsis_10k_records_3_gsis_1k_records_3_gsis_empty_table_3_gsis_single_record
- identifier: ddb_iam_access_3_gsis_10k_records_3_gsis_1k_records_3_gsis_empty_table
buildspec: codebuild_specs/run_cdk_tests.yml
env:
compute-type: BUILD_GENERAL1_MEDIUM
variables:
TEST_SUITE: >-
src/__tests__/deploy-velocity-temporarily-disabled/3-gsis-10k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/3-gsis-1k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/3-gsis-empty-table.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/3-gsis-single-record.test.ts
src/__tests__/ddb-iam-access.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/3-gsis-10k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/3-gsis-1k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/3-gsis-empty-table.test.ts
CLI_REGION: eu-north-1
depend-on:
- publish_to_local_registry
- identifier: >-
replace_2_gsis_10k_records_replace_2_gsis_1k_records_replace_2_gsis_empty_table_replace_2_gsis_single_record
3_gsis_single_record_replace_2_gsis_10k_records_replace_2_gsis_1k_records_replace_2_gsis_empty_table
buildspec: codebuild_specs/run_cdk_tests.yml
env:
compute-type: BUILD_GENERAL1_MEDIUM
variables:
TEST_SUITE: >-
src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-10k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-1k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-empty-table.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-single-record.test.ts
src/__tests__/deploy-velocity-temporarily-disabled/3-gsis-single-record.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-10k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-1k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-empty-table.test.ts
CLI_REGION: eu-west-1
depend-on:
- publish_to_local_registry
- identifier: >-
replace_2_gsis_update_attr_10k_records_replace_2_gsis_update_attr_1k_records_replace_2_gsis_update_attr_empty_table_replace_2_g
replace_2_gsis_single_record_replace_2_gsis_update_attr_10k_records_replace_2_gsis_update_attr_1k_records_replace_2_gsis_update
buildspec: codebuild_specs/run_cdk_tests.yml
env:
compute-type: BUILD_GENERAL1_MEDIUM
variables:
TEST_SUITE: >-
src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-update-attr-10k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-update-attr-1k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-update-attr-empty-table.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-update-attr-single-record.test.ts
src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-single-record.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-update-attr-10k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-update-attr-1k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-update-attr-empty-table.test.ts
CLI_REGION: eu-west-2
depend-on:
- publish_to_local_registry
- identifier: >-
single_gsi_10k_records_single_gsi_1k_records_single_gsi_empty_table_single_gsi_single_record
replace_2_gsis_update_attr_single_record_single_gsi_10k_records_single_gsi_1k_records_single_gsi_empty_table
buildspec: codebuild_specs/run_cdk_tests.yml
env:
compute-type: BUILD_GENERAL1_MEDIUM
variables:
TEST_SUITE: >-
src/__tests__/deploy-velocity-temporarily-disabled/single-gsi-10k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/single-gsi-1k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/single-gsi-empty-table.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/single-gsi-single-record.test.ts
src/__tests__/deploy-velocity-temporarily-disabled/replace-2-gsis-update-attr-single-record.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/single-gsi-10k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/single-gsi-1k-records.test.ts|src/__tests__/deploy-velocity-temporarily-disabled/single-gsi-empty-table.test.ts
CLI_REGION: eu-west-3
depend-on:
- publish_to_local_registry
- identifier: utils
- identifier: single_gsi_single_record_utils
buildspec: codebuild_specs/run_cdk_tests.yml
env:
compute-type: BUILD_GENERAL1_MEDIUM
variables:
TEST_SUITE: src/__tests__/utils.test.ts
TEST_SUITE: >-
src/__tests__/deploy-velocity-temporarily-disabled/single-gsi-single-record.test.ts|src/__tests__/utils.test.ts
CLI_REGION: me-south-1
depend-on:
- publish_to_local_registry
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import * as path from 'path';
import { createNewProjectDir, deleteProjectDir, getDDBTable } from 'amplify-category-api-e2e-core';
import { cdkDestroy, initCDKProject, cdkDeploy, updateCDKAppWithTemplate } from '../commands';

jest.setTimeout(1000 * 60 * 60 /* 1 hour */);

describe('CDK amplify table 5', () => {
let projRoot: string;
let projFolderName: string;

beforeEach(async () => {
projFolderName = 'cdkamplifytable5';
projRoot = await createNewProjectDir(projFolderName);
});

afterEach(async () => {
try {
await cdkDestroy(projRoot, '--all');
} catch (_) {
/* No-op */
}

deleteProjectDir(projRoot);
});

test('datatype change on indexed field should replace the index', async () => {
const templatePath = path.resolve(path.join(__dirname, 'backends', 'amplify-table', 'simple-todo'));
const name = await initCDKProject(projRoot, templatePath);
const outputs = await cdkDeploy(projRoot, '--all');
const { awsAppsyncApiId: apiId, awsAppsyncRegion: region } = outputs[name];
const tableName = `Todo-${apiId}-NONE`;
const table = await getDDBTable(tableName, region);
expect(table.Table.AttributeDefinitions).toHaveLength(2);
const nameAttr = table.Table.AttributeDefinitions.find((attr) => attr.AttributeName === 'name');
expect(nameAttr.AttributeType).toEqual('S');
expect(table.Table.GlobalSecondaryIndexes).toHaveLength(1);
expect(table.Table.GlobalSecondaryIndexes[0].IndexName).toEqual('byName');

// deploy with modified attribute type
const updateTemplatePath = path.resolve(path.join(__dirname, 'backends', 'amplify-table', 'simple-todo', 'attributeTypeChange'));
updateCDKAppWithTemplate(projRoot, updateTemplatePath);
await expect(cdkDeploy(projRoot, '--all')).resolves.not.toThrow();
const modifiedTable = await getDDBTable(tableName, region);
expect(modifiedTable.Table.AttributeDefinitions).toHaveLength(3);
const modifiedNameAttr = modifiedTable.Table.AttributeDefinitions.find((attr) => attr.AttributeName === 'name');
expect(modifiedNameAttr.AttributeType).toEqual('N');
expect(modifiedTable.Table.GlobalSecondaryIndexes).toHaveLength(2);
expect(modifiedTable.Table.GlobalSecondaryIndexes).toEqual(
expect.arrayContaining([
expect.objectContaining({
IndexName: 'byNameDescription',
}),
expect.objectContaining({
IndexName: 'byName',
}),
]),
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/usr/bin/env node
import 'source-map-support/register';
import { App, Stack, Duration } from 'aws-cdk-lib';
// @ts-ignore
import { AmplifyGraphqlApi, AmplifyGraphqlDefinition } from '@aws-amplify/graphql-api-construct';

const packageJson = require('../package.json');

const app = new App();
const stack = new Stack(app, packageJson.name.replace(/_/g, '-'), {
env: { region: process.env.CLI_REGION || 'us-west-2' },
});

new AmplifyGraphqlApi(stack, 'GraphqlApi', {
apiName: 'MyGraphQLApi',
definition: AmplifyGraphqlDefinition.fromString(
/* GraphQL */ `
type Todo @model @auth(rules: [{ allow: public }]) {
id: ID!
description: String!
name: Int! @index(name: "byName") @index(name: "byNameDescription", sortKeyFields: ["description"])
}
`,
{
dbType: 'DYNAMODB',
provisionStrategy: 'AMPLIFY_TABLE',
},
),
authorizationModes: {
apiKeyConfig: { expires: Duration.days(7) },
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -537,13 +537,19 @@ const getNextGSIUpdate = (currentState: TableDescription, endState: CustomDDB.In
const endStateGSIs = endState.globalSecondaryIndexes || [];
const endStateGSINames = endStateGSIs.map((gsi) => gsi.indexName);

// Retrieve the attributes whose type has been modified
const modifiedAttributes = getTypeModifiedAttributes(currentState.AttributeDefinitions, endState.attributeDefinitions);
const indexesWithModifiedAttributeType = getIndexesContainingAttributes(currentState.GlobalSecondaryIndexes, modifiedAttributes);

const currentStateGSIs = currentState.GlobalSecondaryIndexes || [];
const currentStateGSINames = currentStateGSIs.map((gsi) => gsi.IndexName);

// function to identify any GSIs that need to be removed
const gsiRequiresReplacementPredicate = (currentGSI: GlobalSecondaryIndexDescription): boolean => {
// check if the index has been removed entirely
if (!endStateGSINames.includes(currentGSI.IndexName!)) return true;
// check if the index attributes type has been modified
if (indexesWithModifiedAttributeType.includes(currentGSI.IndexName!)) return true;
// get the end state of this GSI
const respectiveEndStateGSI = endStateGSIs.find((endStateGSI) => endStateGSI.indexName === currentGSI.IndexName)!;
// detect if projection has changed
Expand Down Expand Up @@ -1105,6 +1111,48 @@ const isKeySchemaModified = (currentSchema: Array<KeySchemaElement>, endSchema:
return false;
};

/**
* Util function to get a list of attributes with modified type
* @param currentSchema current state of attributes
* @param endSchema end state of key attributes
* @returns string[] indicates the list of attributes name with modified type
*/
const getTypeModifiedAttributes = (
currentSchema?: Array<AttributeDefinition>,
endSchema?: Array<CustomDDB.AttributeDefinitionProperty>,
): string[] => {
const result: string[] = [];
if (!currentSchema || !endSchema) return result;
for (const attribute of currentSchema) {
const endAttribute = endSchema.find((endAttr) => endAttr.attributeName === attribute.AttributeName);
// If an attribute is not found in the end schema, no need to handle it here.
// The attribute will be removed once we delete the corresponding GSI.
if (!endAttribute) continue;
if (attribute.AttributeType !== endAttribute.attributeType) {
result.push(attribute.AttributeName!);
}
}
return result;
};

/**
* Util function to get a list of indexes containing the given attributes
* @param currentSchema current state of GSIs
* @param attributes list of attribute names
* @returns string[] indicates the list of index names containing the given attributes
*/
const getIndexesContainingAttributes = (
currentSchema: Array<GlobalSecondaryIndexDescription> | undefined,
attributes: string[],
): string[] => {
if (!currentSchema) return [];
const result = currentSchema
.filter((index) => index.IndexStatus === 'ACTIVE') // This is important. You do not want to update a GSI that is not active.
.filter((index) => index.KeySchema?.some((key) => attributes.includes(key.AttributeName!)))
.map((index) => index.IndexName!);
return result ?? [];
};

/**
* Util function to check if the time to live is modified between old and new table definitions
* @param oldTtl TTL config from old table properties
Expand Down
31 changes: 3 additions & 28 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -21314,7 +21314,7 @@ string-length@^4.0.1:
char-regex "^1.0.2"
strip-ansi "^6.0.0"

"string-width-cjs@npm:string-width@^4.2.0":
"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
Expand All @@ -21332,15 +21332,6 @@ string-width@^1.0.1:
is-fullwidth-code-point "^1.0.0"
strip-ansi "^3.0.0"

"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
dependencies:
emoji-regex "^8.0.0"
is-fullwidth-code-point "^3.0.0"
strip-ansi "^6.0.1"

string-width@^2.1.0, string-width@^2.1.1:
version "2.1.1"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-2.1.1.tgz#ab93f27a8dc13d28cac815c462143a6d9012ae9e"
Expand Down Expand Up @@ -21433,7 +21424,7 @@ stringify-object@^3.3.0:
is-obj "^1.0.1"
is-regexp "^1.0.0"

"strip-ansi-cjs@npm:strip-ansi@^6.0.1":
"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1:
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
Expand Down Expand Up @@ -21461,13 +21452,6 @@ strip-ansi@^5.1.0, strip-ansi@^5.2.0:
dependencies:
ansi-regex "^4.1.0"

strip-ansi@^6.0.0, strip-ansi@^6.0.1:
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
dependencies:
ansi-regex "^5.0.1"

strip-ansi@^7.0.1, strip-ansi@^7.1.0:
version "7.1.0"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45"
Expand Down Expand Up @@ -22815,7 +22799,7 @@ workerpool@^6.5.0:
resolved "https://registry.yarnpkg.com/workerpool/-/workerpool-6.5.1.tgz#060f73b39d0caf97c6db64da004cd01b4c099544"
integrity sha512-Fs4dNYcsdpYSAfVxhnl1L5zTksjvOJxtC5hzMNl+1t9B8hTJTdKDyZ5ju7ztgPy+ft9tBFXoOlDNiOT9WUXZlA==

"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0":
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
Expand All @@ -22841,15 +22825,6 @@ wrap-ansi@^6.0.1, wrap-ansi@^6.2.0:
string-width "^4.1.0"
strip-ansi "^6.0.0"

wrap-ansi@^7.0.0:
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
dependencies:
ansi-styles "^4.0.0"
string-width "^4.1.0"
strip-ansi "^6.0.0"

wrap-ansi@^8.0.1, wrap-ansi@^8.1.0:
version "8.1.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214"
Expand Down

0 comments on commit 43cbb3d

Please sign in to comment.