feat(graphql-key-transformer): change default to add GSIs when using @…

…key (#5648)

packages/amplify-cli-core/src/feature-flags/featureFlags.ts

@@ -533,6 +533,12 @@ export class FeatureFlags {
         defaultValueForExistingProjects: false,
         defaultValueForNewProjects: false,
       },
+      {
+        name: 'secondaryKeyAsGSI',
+        type: 'boolean',
+        defaultValueForExistingProjects: false,
+        defaultValueForNewProjects: true,
+      },
     ]);
 
     this.registerFlag('frontend-ios', [
@@ -568,7 +574,7 @@ export class FeatureFlags {
         type: 'boolean',
         defaultValueForExistingProjects: false,
         defaultValueForNewProjects: true,
-      },
+      }
     ]);
   };
 }

packages/amplify-e2e-core/src/categories/api.ts

@@ -138,9 +138,12 @@ export function addApiWithSchemaAndConflictDetection(cwd: string, schemaFile: st
   });
 }
 
-export function updateApiSchema(cwd: string, projectName: string, schemaName: string) {
+export function updateApiSchema(cwd: string, projectName: string, schemaName: string, forceUpdate: boolean = false) {
   const testSchemaPath = getSchemaPath(schemaName);
-  const schemaText = fs.readFileSync(testSchemaPath).toString();
+  let schemaText = fs.readFileSync(testSchemaPath).toString();
+  if (forceUpdate) {
+    schemaText += '  ';
+  }
   updateSchema(cwd, projectName, schemaText);
 }
 

packages/amplify-migration-tests/src/__tests__/migration_tests/transformer_migration/api.key.migration.test.ts

@@ -1,6 +1,16 @@
-import { initJSProjectWithProfile, deleteProject, amplifyPush, amplifyPushForce } from 'amplify-e2e-core';
-import { addApiWithSchema, apiGqlCompile } from 'amplify-e2e-core';
-import { createNewProjectDir, deleteProjectDir } from 'amplify-e2e-core';
+import {
+  initJSProjectWithProfile,
+  deleteProject,
+  amplifyPush,
+  amplifyPushForce,
+  addApiWithSchema,
+  updateApiSchema,
+  apiGqlCompile,
+  createNewProjectDir,
+  deleteProjectDir,
+  amplifyPushUpdate,
+  addFeatureFlag
+} from 'amplify-e2e-core';
 
 describe('amplify key force push', () => {
   let projRoot: string;
@@ -24,4 +34,25 @@ describe('amplify key force push', () => {
     await apiGqlCompile(projRoot, true);
     await amplifyPushForce(projRoot, true);
   });
+
+  it('init project, add lsi key and force push expect error', async () => {
+    const projectName = 'keyforce';
+    const initialSchema = 'migrations_key/initial_schema.graphql';
+    // init, add api and push with installed cli
+    await initJSProjectWithProfile(projRoot, { name: projectName });
+    await addApiWithSchema(projRoot, initialSchema);
+    await amplifyPush(projRoot);
+    // add feature flag
+    addFeatureFlag(projRoot, 'graphqltransformer', 'secondaryKeyAsGSI', true);
+    // forceUpdateSchema
+    updateApiSchema(projRoot, projectName, initialSchema, true);
+    // gql-compile and force push with codebase cli
+    await expect(
+      amplifyPushUpdate(
+        projRoot,
+        /Attempting to remove a local secondary index on the TodoTable table in the Todo stack.*/,
+        true,
+      ),
+    ).rejects.toThrowError(/Attempting to remove a local secondary index on the TodoTable table in the Todo stack.*/);
+  });
 });

packages/amplify-provider-awscloudformation/src/transform-graphql-schema.ts

@@ -31,6 +31,7 @@ import {
   migrateAPIProject,
   readProjectConfiguration,
   buildAPIProject,
+  TransformConfig,
 } from 'graphql-transformer-core';
 
 import { print } from 'graphql';
@@ -72,7 +73,7 @@ function getTransformerFactory(context, resourceDir, authConfig?) {
       transformerList.push(new SearchableModelTransformer());
     }
 
-    const customTransformersConfig = await readTransformerConfiguration(resourceDir);
+    const customTransformersConfig: TransformConfig = await readTransformerConfiguration(resourceDir);
     const customTransformers = (customTransformersConfig && customTransformersConfig.transformers
       ? customTransformersConfig.transformers
       : []

packages/graphql-key-transformer/src/KeyTransformer.ts

@@ -646,6 +646,7 @@ export class KeyTransformer extends Transformer {
     const tableResource = ctx.getResource(tableLogicalID);
     const primaryKeyDirective = getPrimaryKey(definition);
     const primaryPartitionKeyName = primaryKeyDirective ? getDirectiveArguments(primaryKeyDirective).fields[0] : 'id';
+    const defaultGSI = ctx.featureFlags.getBoolean('secondaryKeyAsGSI', false);
     if (!tableResource) {
       throw new InvalidDirectiveError(`The @key directive may only be added to object definitions annotated with @model.`);
     } else {
@@ -656,7 +657,7 @@ export class KeyTransformer extends Transformer {
           ProjectionType: 'ALL',
         }),
       };
-      if (primaryPartitionKeyName === ks[0].AttributeName) {
+      if (primaryPartitionKeyName === ks[0].AttributeName && !defaultGSI) {
         // This is an LSI.
         // Add the new secondary index and update the table's attribute definitions.
         tableResource.Properties.LocalSecondaryIndexes = append(

packages/graphql-transformer-core/src/util/amplifyUtils.ts

@@ -11,6 +11,7 @@ import { FeatureFlagProvider } from '../FeatureFlags';
 import {
   cantAddAndRemoveGSIAtSameTimeRule,
   cantAddLSILaterRule,
+  cantRemoveLSILater,
   cantEditGSIKeySchemaRule,
   cantEditKeySchemaRule,
   cantEditLSIKeySchemaRule,
@@ -64,6 +65,7 @@ export async function buildProject(opts: ProjectOptions) {
           // LSI
           cantEditKeySchemaRule,
           cantAddLSILaterRule,
+          cantRemoveLSILater,
           cantEditLSIKeySchemaRule,
         ];
 
@@ -74,6 +76,7 @@ export async function buildProject(opts: ProjectOptions) {
           // LSI
           cantEditKeySchemaRule,
           cantAddLSILaterRule,
+          cantRemoveLSILater,
           cantEditLSIKeySchemaRule,
           // GSI
           cantEditGSIKeySchemaRule,

packages/graphql-transformer-core/src/util/sanity-check.ts

@@ -333,6 +333,28 @@ export const cantEditLSIKeySchemaRule = (diff: Diff, currentBuild: DiffableProje
   }
 };
 
+export function cantRemoveLSILater(diff: Diff, currentBuild: DiffableProject, nextBuild: DiffableProject) {
+  const throwError = (stackName: string, tableName: string): void => {
+    throw new InvalidMigrationError(
+      `Attempting to remove a local secondary index on the ${tableName} table in the ${stackName} stack.`,
+      'A local secondary index cannot be removed after deployment.',
+      'In order to remove the local secondary index you need to delete or rename the table.',
+    );
+  };
+  // if removing more than one lsi
+  if (diff.kind === 'D' && diff.lhs && diff.path.length === 6 && diff.path[5] === 'LocalSecondaryIndexes') {
+    const tableName = diff.path[3];
+    const stackName = path.basename(diff.path[1], '.json');
+    throwError(stackName, tableName);
+  }
+ // if removing one lsi
+  if(diff.kind === 'A' && diff.item.kind === 'D' && diff.path.length === 6 && diff.path[5] === 'LocalSecondaryIndexes') {
+    const tableName = diff.path[3];
+    const stackName = path.basename(diff.path[1], '.json');
+    throwError(stackName, tableName);
+  }
+}
+
 export const cantHaveMoreThan500ResourcesRule = (diffs: Diff[], currentBuild: DiffableProject, nextBuild: DiffableProject): void => {
   const stackKeys = Object.keys(nextBuild.stacks);
 

packages/graphql-transformers-e2e-tests/src/__tests__/KeyTransformerLocal.e2e.test.ts

@@ -1,4 +1,4 @@
-import { GraphQLTransform } from 'graphql-transformer-core';
+import { GraphQLTransform, FeatureFlagProvider } from 'graphql-transformer-core';
 import { DynamoDBModelTransformer } from 'graphql-dynamodb-transformer';
 import { KeyTransformer } from 'graphql-key-transformer';
 import { parse, FieldDefinitionNode, ObjectTypeDefinitionNode, Kind, InputObjectTypeDefinitionNode } from 'graphql';
@@ -315,7 +315,7 @@ test('Test that a secondary @key with a multiple field adds an GSI.', () => {
   expect(deleteInput.fields).toHaveLength(2);
 });
 
-test('Test that a secondary @key with a multiple field adds an LSI.', () => {
+test('Test that a secondary @key with a multiple field adds an LSI with GSI FF turned off', () => {
   const validSchema = `
     type Test
         @model @key(fields: ["email", "createdAt"])
@@ -326,9 +326,15 @@ test('Test that a secondary @key with a multiple field adds an LSI.', () => {
     }
     `;
 
-  const transformer = new GraphQLTransform({
-    transformers: [new DynamoDBModelTransformer(), new KeyTransformer()],
-  });
+    const transformer = new GraphQLTransform({
+      transformers: [new DynamoDBModelTransformer(), new KeyTransformer()],
+      featureFlags: {
+        getBoolean: (featureName: string, defaultValue: boolean) => {
+          if (featureName === 'secondaryKeyAsGSI') return false;
+          return defaultValue || false;
+        },
+      } as unknown as FeatureFlagProvider
+    });
 
   const out = transformer.transform(validSchema);
   let tableResource = out.stacks.Test.Resources.TestTable;
@@ -352,6 +358,49 @@ test('Test that a secondary @key with a multiple field adds an LSI.', () => {
   expectArguments(listTestsField, ['email', 'createdAt', 'filter', 'nextToken', 'limit', 'sortDirection']);
 });
 
+test('Test that a secondary @key with a multiple field adds an GSI based on enabled feature flag.', () => {
+  const validSchema = `
+    type Test
+        @model @key(fields: ["email", "createdAt"])
+        @key(name: "GSI_Email_UpdatedAt", fields: ["email", "updatedAt"], queryField: "testsByEmailByUpdatedAt") {
+        email: String!
+        createdAt: AWSDateTime!
+        updatedAt: AWSDateTime!
+    }
+    `;
+
+    const transformer = new GraphQLTransform({
+      transformers: [new DynamoDBModelTransformer(), new KeyTransformer()],
+      featureFlags: {
+        getBoolean: (featureName: string, defaultValue: boolean) => {
+          if (featureName === 'secondaryKeyAsGSI') return true;
+          return defaultValue || false;
+        },
+      } as unknown as FeatureFlagProvider
+    });
+
+  const out = transformer.transform(validSchema);
+  let tableResource = out.stacks.Test.Resources.TestTable;
+  expect(tableResource).toBeDefined();
+  expect(tableResource.Properties.GlobalSecondaryIndexes[0].KeySchema[0].AttributeName).toEqual('email');
+  expect(tableResource.Properties.GlobalSecondaryIndexes[0].KeySchema[0].KeyType).toEqual('HASH');
+  expect(tableResource.Properties.GlobalSecondaryIndexes[0].KeySchema[1].AttributeName).toEqual('updatedAt');
+  expect(tableResource.Properties.GlobalSecondaryIndexes[0].KeySchema[1].KeyType).toEqual('RANGE');
+  expect(tableResource.Properties.AttributeDefinitions.find(ad => ad.AttributeName === 'email').AttributeType).toEqual('S');
+  expect(tableResource.Properties.AttributeDefinitions.find(ad => ad.AttributeName === 'updatedAt').AttributeType).toEqual('S');
+  expect(tableResource.Properties.AttributeDefinitions.find(ad => ad.AttributeName === 'createdAt').AttributeType).toEqual('S');
+  const schema = parse(out.schema);
+  const queryType = schema.definitions.find((def: any) => def.name && def.name.value === 'Query') as ObjectTypeDefinitionNode;
+  const queryIndexField = queryType.fields.find(f => f.name && f.name.value === 'testsByEmailByUpdatedAt') as FieldDefinitionNode;
+  expect(queryIndexField.arguments).toHaveLength(6);
+  expectArguments(queryIndexField, ['email', 'updatedAt', 'filter', 'nextToken', 'limit', 'sortDirection']);
+
+  // When using a complex primary key args are added to the list field. They are optional and if provided, will use a Query instead of a Scan.
+  const listTestsField = queryType.fields.find(f => f.name && f.name.value === 'listTests') as FieldDefinitionNode;
+  expect(listTestsField.arguments).toHaveLength(6);
+  expectArguments(listTestsField, ['email', 'createdAt', 'filter', 'nextToken', 'limit', 'sortDirection']);
+});
+
 test('Test that a primary @key with complex fields will update the input objects.', () => {
   const validSchema = `
     type Test @model @key(fields: ["email"]) {