Revert "fix: use existing rather than deleted models (#9769)" (#9884)

This reverts commit c4b7487.
aws-amplify · Mar 1, 2022 · 94547e0 · 94547e0
1 parent b22b67d
commit 94547e0
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 160 deletions.
diff --git a/...unction/src/__tests__/provider-utils/awscloudformation/utils/getDependentFunction.test.ts b/...unction/src/__tests__/provider-utils/awscloudformation/utils/getDependentFunction.test.ts
@@ -1,6 +1,4 @@
-import {
-  lambdasWithMissingApiDependency,
-} from '../../../../provider-utils/awscloudformation/utils/getDependentFunction';
+import { lambdasWithApiDependency } from '../../../../provider-utils/awscloudformation/utils/getDependentFunction';
 import { loadFunctionParameters } from '../../../../provider-utils/awscloudformation/utils/loadFunctionParameters';
 import { $TSContext } from 'amplify-cli-core';
 
@@ -68,50 +66,46 @@ const allResources = [
 const backendDir = 'randomPath';
 const loadResourceParameters_mock = loadFunctionParameters as jest.MockedFunction<typeof loadFunctionParameters>;
 
-describe('get dependent functions', () => {
-  it('using one out of three models', async () => {
-    jest.clearAllMocks();
-    const existingModels = ['model1'];
-    const FunctionMetaExpected = ['fn2', 'fn3'];
-    loadResourceParameters_mock
-      .mockReturnValueOnce({
-        permissions: {
-          storage: {
-            model1: ['create'],
-            model2: ['create'],
-            model3: ['create'],
-          },
+test('get dependent functions', async () => {
+  jest.clearAllMocks();
+  const modelsDeleted = ['model3', 'model2'];
+  const FunctionMetaExpected = ['fn2', 'fn3'];
+  loadResourceParameters_mock
+    .mockReturnValueOnce({
+      permissions: {
+        storage: {
+          model1: ['create'],
+          model2: ['create'],
+          model3: ['create'],
         },
-      })
-      .mockReturnValueOnce({
-        permissions: {
-          storage: {
-            model3: ['create'],
-          },
+      },
+    })
+    .mockReturnValueOnce({
+      permissions: {
+        storage: {
+          model3: ['create'],
         },
-      });
-    const fnMetaToBeUpdated = await lambdasWithMissingApiDependency((contextStub as unknown) as $TSContext, allResources, backendDir, existingModels);
-    expect(fnMetaToBeUpdated.map(resource => resource.resourceName).toString()).toBe(FunctionMetaExpected.toString());
-  });
+      },
+    });
+  const fnMetaToBeUpdated = await lambdasWithApiDependency((contextStub as unknown) as $TSContext, allResources, backendDir, modelsDeleted);
+  expect(fnMetaToBeUpdated.map(resource => resource.resourceName).toString()).toBe(FunctionMetaExpected.toString());
 });
 
-describe('get dependent functions with empty permissions', () => {
-  it('using two out of three models', async () => {
-    jest.clearAllMocks();
-    const existingModels = ['model1', 'model2'];
-    const FunctionMetaExpected = ['fn2'];
-    loadResourceParameters_mock
-      .mockReturnValueOnce({
-        permissions: {
-          storage: {
-            model1: ['create'],
-            model2: ['create'],
-            model3: ['create'],
-          },
+test('get dependent functions with empty permissions', async () => {
+  jest.clearAllMocks();
+  const modelsDeleted = ['model3'];
+  const FunctionMetaExpected = ['fn2'];
+  loadResourceParameters_mock
+    .mockReturnValueOnce({
+      permissions: {
+        storage: {
+          model1: ['create'],
+          model2: ['create'],
+          model3: ['create'],
         },
-      })
-      .mockReturnValueOnce({});
-    const fnMetaToBeUpdated = await lambdasWithMissingApiDependency((contextStub as unknown) as $TSContext, allResources, backendDir, existingModels);
-    expect(fnMetaToBeUpdated.map(resource => resource.resourceName).toString()).toBe(FunctionMetaExpected.toString());
-  });
+      },
+    })
+    .mockReturnValueOnce({});
+  const fnMetaToBeUpdated = await lambdasWithApiDependency((contextStub as unknown) as $TSContext, allResources, backendDir, modelsDeleted);
+  expect(fnMetaToBeUpdated.map(resource => resource.resourceName).toString()).toBe(FunctionMetaExpected.toString());
 });
diff --git a/...n/src/__tests__/provider-utils/awscloudformation/utils/updateDependentFunctionCfn.test.ts b/...n/src/__tests__/provider-utils/awscloudformation/utils/updateDependentFunctionCfn.test.ts
@@ -1,6 +1,4 @@
-import {
-  updateMissingDependencyFunctionsCfn,
-} from '../../../../provider-utils/awscloudformation/utils/updateDependentFunctionCfn';
+import { updateDependentFunctionsCfn } from '../../../../provider-utils/awscloudformation/utils/updateDependentFunctionCfn';
 import { loadFunctionParameters } from '../../../../provider-utils/awscloudformation/utils/loadFunctionParameters';
 import {
   getResourcesForCfn,
@@ -88,7 +86,7 @@ generateEnvVariablesforCFN_mock.mockResolvedValue({ dependsOn, environmentMap, e
 
 test('update dependent functions', async () => {
   jest.clearAllMocks();
-  const existingModels = ['model1', 'model2'];
+  const modelsDeleted = ['model3'];
   loadResourceParameters_mock
     .mockReturnValueOnce({
       permissions: {
@@ -106,13 +104,13 @@ test('update dependent functions', async () => {
         },
       },
     });
-  await updateMissingDependencyFunctionsCfn(contextStub as unknown as $TSContext, allResources, backendDir, existingModels, apiResourceName);
+  await updateDependentFunctionsCfn(contextStub as unknown as $TSContext, allResources, backendDir, modelsDeleted, apiResourceName);
   expect(updateCFNFileForResourcePermissions_mock.mock.calls[0][1]).toMatchSnapshot();
 });
 
 test('update dependent functions', async () => {
   jest.clearAllMocks();
-  const existingModels = ['model3'];
+  const modelsDeleted = ['model1', 'model2'];
   loadResourceParameters_mock
     .mockReturnValueOnce({
       permissions: {
@@ -144,6 +142,6 @@ test('update dependent functions', async () => {
         },
       ],
     });
-  await updateMissingDependencyFunctionsCfn(contextStub as unknown as $TSContext, allResources, backendDir, existingModels, apiResourceName);
+  await updateDependentFunctionsCfn(contextStub as unknown as $TSContext, allResources, backendDir, modelsDeleted, apiResourceName);
   expect(updateCFNFileForResourcePermissions_mock.mock.calls[0][1]).toMatchSnapshot();
 });
diff --git a/packages/amplify-category-function/src/index.ts b/packages/amplify-category-function/src/index.ts
@@ -25,12 +25,12 @@ export { askExecRolePermissionsQuestions } from './provider-utils/awscloudformat
 export { buildResource } from './provider-utils/awscloudformation/utils/build';
 export { buildTypeKeyMap } from './provider-utils/awscloudformation/utils/buildFunction';
 export { ServiceName } from './provider-utils/awscloudformation/utils/constants';
-export { lambdasWithMissingApiDependency } from './provider-utils/awscloudformation/utils/getDependentFunction';
+export { lambdasWithApiDependency } from './provider-utils/awscloudformation/utils/getDependentFunction';
 export { hashLayerResource } from './provider-utils/awscloudformation/utils/layerHelpers';
 export { migrateLegacyLayer } from './provider-utils/awscloudformation/utils/layerMigrationUtils';
 export { packageResource } from './provider-utils/awscloudformation/utils/package';
 export {
-  updateMissingDependencyFunctionsCfn,
+  updateDependentFunctionsCfn,
   addAppSyncInvokeMethodPermission,
 } from './provider-utils/awscloudformation/utils/updateDependentFunctionCfn';
 export { loadFunctionParameters } from './provider-utils/awscloudformation/utils/loadFunctionParameters';

diff --git a/...lify-category-function/src/provider-utils/awscloudformation/utils/getDependentFunction.ts b/...lify-category-function/src/provider-utils/awscloudformation/utils/getDependentFunction.ts
@@ -1,25 +1,23 @@
 import { $TSContext, $TSObject } from 'amplify-cli-core';
 import * as path from 'path';
-import { loadFunctionParameters, loadFunctionStackAsJSON } from './loadFunctionParameters';
+import { loadFunctionParameters } from './loadFunctionParameters';
 import { ServiceName } from './constants';
 import { categoryName } from '../../../constants';
 
-
-export async function lambdasWithMissingApiDependency(
+export async function lambdasWithApiDependency(
   context: $TSContext,
   allResources: $TSObject[],
   backendDir: string,
-  existingModels: string[],
+  modelsDeleted: string[],
 ) {
-  //get the List of functions dependent on deleted models
+  //get the List of functions dependent on deleted model
   let dependentFunctions = [];
   const lambdaFuncResources = allResources.filter(
-    resource => {
-      return resource.service === ServiceName.LambdaFunction &&
-                resource.mobileHubMigrated !== true &&
-                resource.dependsOn !== undefined &&
-                resource.dependsOn.find(val => val.category === 'api');
-    },
+    resource =>
+      resource.service === ServiceName.LambdaFunction &&
+      resource.mobileHubMigrated !== true &&
+      resource.dependsOn !== undefined &&
+      resource.dependsOn.find(val => val.category === 'api'),
   );
 
   // initialize function parameters for update
@@ -31,26 +29,12 @@ export async function lambdasWithMissingApiDependency(
 
     if (typeof selectedCategories === 'object' && selectedCategories !== null) {
       for (const selectedResources of Object.values(selectedCategories)) {
-        deletedModelFound = Object.keys(selectedResources).some(r => !existingModels.includes(r));
+        deletedModelFound = Object.keys(selectedResources).some(r => modelsDeleted.includes(r));
         if (deletedModelFound) {
           dependentFunctions.push(lambda);
         }
       }
     }
-    else {
-      // In the case that the function-parameters.json does not have info on dependencies, we check the CFN stack.
-      // If the dependency is only a dynamo stream trigger, we don't have info in function-parameters.json to work with
-      const lambdaStackJson = loadFunctionStackAsJSON(resourceDirPath, lambda.resourceName);
-      if (lambdaStackJson?.Resources) {
-        const triggerPolicyTables = Object.keys(lambdaStackJson.Resources).filter(key => key.startsWith("LambdaTriggerPolicy"));
-        for (const triggerPolicy of triggerPolicyTables) {
-          const tableName = triggerPolicy.match(/(?<=LambdaTriggerPolicy)(.*)/g)?.[0];
-          if (tableName && !existingModels.includes(tableName)) {
-            dependentFunctions.push(lambda);
-          }
-        }
-      }
-    }
   }
   return dependentFunctions;
 }
diff --git a/...fy-category-function/src/provider-utils/awscloudformation/utils/loadFunctionParameters.ts b/...fy-category-function/src/provider-utils/awscloudformation/utils/loadFunctionParameters.ts
@@ -14,8 +14,3 @@ export const loadFunctionParameters = (resourcePath: string) => {
   }
   return funcParams;
 };
-
-export const loadFunctionStackAsJSON = (resourcePath: string, functionName: string) => {
-  const stackJSON = JSONUtilities.readJson<$TSAny>(path.join(resourcePath,`${functionName}-cloudformation-template.json`));
-  return stackJSON;
-}
diff --git a/...ategory-function/src/provider-utils/awscloudformation/utils/updateDependentFunctionCfn.ts b/...ategory-function/src/provider-utils/awscloudformation/utils/updateDependentFunctionCfn.ts
@@ -7,12 +7,11 @@ import * as path from 'path';
 import { functionParametersFileName } from './constants';
 import { categoryName } from '../../../constants';
 
-
-export async function updateMissingDependencyFunctionsCfn(
+export async function updateDependentFunctionsCfn(
   context: $TSContext,
   dependentFunctionResource: $TSObject[],
   backendDir: string,
-  existingModels: string[],
+  modelsDeleted: string[],
   apiResource: string,
 ) {
   // remove function parameters from if there is dependency
@@ -43,28 +42,26 @@ export async function updateMissingDependencyFunctionsCfn(
       },
     };
 
-    if (selectedCategories) {
-      for (const selectedCategory of Object.keys(selectedCategories)) {
-        // update function parameters resouce parameters with deleted models data
-        // remove the deleted @model
-        const selectedResources = selectedCategories[selectedCategory];
-        for (const resourceName of Object.keys(selectedResources)) {
-          if (existingModels.includes(resourceName)) {
-            const resourcePolicy = selectedResources[resourceName];
-            const { permissionPolicies, cfnResources } = await getResourcesForCfn(
-              context,
-              resourceName,
-              resourcePolicy,
-              apiResource,
-              selectedCategory,
-            );
-            categoryPolicies = categoryPolicies.concat(permissionPolicies);
-            if (!permissions[selectedCategory]) {
-              permissions[selectedCategory] = {};
-            }
-            permissions[selectedCategory][resourceName] = resourcePolicy;
-            resources = resources.concat(cfnResources);
+    for (const selectedCategory of Object.keys(selectedCategories)) {
+      // update function parameters resouce parameters with deleted models data
+      // remove the deleted @model
+      const selectedResources = selectedCategories[selectedCategory];
+      for (const resourceName of Object.keys(selectedResources)) {
+        if (!modelsDeleted.includes(resourceName)) {
+          const resourcePolicy = selectedResources[resourceName];
+          const { permissionPolicies, cfnResources } = await getResourcesForCfn(
+            context,
+            resourceName,
+            resourcePolicy,
+            apiResource,
+            selectedCategory,
+          );
+          categoryPolicies = categoryPolicies.concat(permissionPolicies);
+          if (!permissions[selectedCategory]) {
+            permissions[selectedCategory] = {};
           }
+          permissions[selectedCategory][resourceName] = resourcePolicy;
+          resources = resources.concat(cfnResources);
         }
       }
     }
@@ -75,8 +72,6 @@ export async function updateMissingDependencyFunctionsCfn(
     functionParameters.dependsOn = dependsOn;
     functionParameters.lambdaLayers = currentParameters.lambdaLayers;
     updateCFNFileForResourcePermissions(resourceDirPath, functionParameters, currentParameters, apiResource);
-    // In some cases, we need to ensure that API Dynamo event sources are removed when the model doesn't exist
-    removeDeletedApiDynamoEventSourcesFromCfn(resourceDirPath, lambda.resourceName, existingModels);
     // assign new permissions to current permissions to update function-parameters file
     currentParameters.permissions = permissions;
     // update function-parameters file
@@ -110,40 +105,4 @@ export function addAppSyncInvokeMethodPermission(
     };
   }
   JSONUtilities.writeJson(cfnFilePath, cfnContent);
-}
-
-function removeDeletedApiDynamoEventSourcesFromCfn(
-  resourceDirPath: string,
-  resourceName: string,
-  existingModels: string[],
-) {
-  const sanitizedExistingModels = existingModels.map(value => {
-    return value.slice(0, value.indexOf(':'));
-  });
-  const cfnFilePath = path.join(resourceDirPath, `${resourceName}-cloudformation-template.json`);
-  const functionCfn = JSONUtilities.readJson<$TSAny>(cfnFilePath);
-  let tableDeletions: string[] = [];
-  if (functionCfn?.Resources) {
-    const triggerPolicyTables = Object.keys(functionCfn.Resources).filter(key => key.startsWith('LambdaTriggerPolicy'));
-    for (let triggerPolicy of triggerPolicyTables) {
-      const tableName = triggerPolicy.match(/(?<=LambdaTriggerPolicy)(.*)/g)?.[0];
-      if (tableName && !sanitizedExistingModels.includes(tableName)) {
-        const triggerPolicyStatement: $TSAny = functionCfn?.Resources?.[triggerPolicy]?.Properties?.PolicyDocument?.
-          Statement;
-        for (const statement of triggerPolicyStatement ?? []) {
-          const apiIDCheckString: string = statement?.Resource?.['Fn::ImportValue']?.['Fn::Sub'];
-          if (apiIDCheckString && apiIDCheckString.includes('GraphQLAPIId')) {
-            tableDeletions.push(tableName);
-            break;
-          }
-        }
-      }
-    }
-  }
-
-  for (let tableName of tableDeletions) {
-    delete functionCfn?.Resources?.[`LambdaTriggerPolicy${tableName}`];
-    delete functionCfn?.Resources?.[`LambdaEventSourceMapping${tableName}`];
-  }
-  JSONUtilities.writeJson(cfnFilePath, functionCfn);
-}
+}
diff --git a/packages/amplify-provider-awscloudformation/src/push-resources.ts b/packages/amplify-provider-awscloudformation/src/push-resources.ts
@@ -114,11 +114,11 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re
     await createEnvLevelConstructs(context);
 
     // removing dependent functions if @model{Table} is deleted
-    const apiResourceTobeChanged = (resourcesToBeUpdated.concat(resourcesToBeDeleted).concat(resourcesToBeCreated)).filter(resource => resource.service === 'AppSync');
-    if (apiResourceTobeChanged.length) {
+    const apiResourceTobeUpdated = resourcesToBeUpdated.filter(resource => resource.service === 'AppSync');
+    if (apiResourceTobeUpdated.length) {
       const functionResourceToBeUpdated = await ensureValidFunctionModelDependencies(
         context,
-        apiResourceTobeChanged,
+        apiResourceTobeUpdated,
         allResources as $TSObject[],
       );
       // filter updated function to replace with existing updated ones(in case of duplicates)