fix: prevent removal of analytics and auth when being depended on

packages/amplify-category-analytics/package.json

@@ -17,7 +17,8 @@
   "scripts": {
     "build": "tsc",
     "clean": "rimraf lib tsconfig.tsbuildinfo node_modules",
-    "watch": "tsc --watch"
+    "watch": "tsc --watch",
+    "test": "jest --logHeapUsage"
   },
   "dependencies": {
     "@aws-amplify/amplify-environment-parameters": "1.1.3",
@@ -26,5 +27,21 @@
     "fs-extra": "^8.1.0",
     "inquirer": "^7.3.3",
     "uuid": "^8.3.2"
+  },
+  "jest": {
+    "collectCoverage": true,
+    "transform": {
+      "^.+\\.tsx?$": "ts-jest"
+    },
+    "testURL": "http://localhost",
+    "testRegex": "((\\.|/)(test|spec))\\.(jsx?|tsx?)$",
+    "moduleFileExtensions": [
+      "ts",
+      "tsx",
+      "js",
+      "jsx",
+      "json",
+      "node"
+    ]
   }
-}
+}

packages/amplify-category-analytics/src/__tests__/commands/analytics/remove.test.ts

@@ -0,0 +1,29 @@
+import { $TSContext } from 'amplify-cli-core';
+import { run as runRemove } from '../../../commands/analytics/remove';
+import { checkResourceInUseByNotifications } from '../../../plugin-client-api-notifications';
+
+jest.mock('../../../plugin-client-api-notifications');
+
+const checkResourceInUseByNotificationsMock = checkResourceInUseByNotifications as jest.MockedFunction<
+  typeof checkResourceInUseByNotifications
+>;
+
+checkResourceInUseByNotificationsMock.mockResolvedValue(true);
+
+describe('remove analytics handler', () => {
+  it('throws error if notifications exists', async () => {
+    const stubContext = ({
+      amplify: {
+        removeResource: jest.fn().mockImplementation(async (__context, __category, resourceName, __config, callback) => {
+          await callback(resourceName);
+        }),
+      },
+      parameters: {
+        first: 'testing',
+      },
+    } as unknown) as $TSContext;
+    await expect(() => runRemove(stubContext)).rejects.toThrowErrorMatchingInlineSnapshot(
+      `"Analytics resource testing is being used by the notifications category and cannot be removed"`,
+    );
+  });
+});

packages/amplify-category-analytics/src/commands/analytics/remove.ts

@@ -1,30 +1,12 @@
 import {
-  $TSContext, AmplifyCategories, amplifyFaultWithTroubleshootingLink,
+  $TSContext, AmplifyError,
 } from 'amplify-cli-core';
-import { printer } from 'amplify-prompts';
-import { checkResourceInUseByNotifications, invokeNotificationsAPIRecursiveRemoveApp } from '../../plugin-client-api-notifications';
+import { checkResourceInUseByNotifications } from '../../plugin-client-api-notifications';
 
 const subcommand = 'remove';
 const category = 'analytics';
-/**
- * Check if Notifications is using the given Analytics resource.
- * note:- TBD: This function will be replaced by a generic pre-remove hook handler in the future.
- * The eventual goal is to remove all explicit binding in the code between categories and abstract them out
- * by role (capabilities, providerCategory and subscriberCategory ).
- */
-const removeResourceDependencies = async (context:$TSContext, resourceName: string): Promise<void> => {
-  const isResourceInUse = await checkResourceInUseByNotifications(context, resourceName);
-  if (isResourceInUse) {
-    // Pinpoint App is in use by Notifications.
-    printer.warn(`Disabling all notifications on ${resourceName}`);
-    const result = await invokeNotificationsAPIRecursiveRemoveApp(context, resourceName);
-    if (!result.status) {
-      throw amplifyFaultWithTroubleshootingLink('ResourceRemoveFault', {
-        message: result.reasonMsg || `Failed to remove ${resourceName} from ${AmplifyCategories.NOTIFICATIONS} category`,
-      });
-    }
-  }
-};
+
+export const name = subcommand;
 
 /**
  * Analytics remove resource handler.
@@ -35,8 +17,16 @@ export const run = async (context: $TSContext): Promise<void> => {
   const { amplify, parameters } = context;
   const resourceName = parameters.first;
 
-  await amplify.removeResource(context, category, resourceName, { headless: false });
-  return removeResourceDependencies(context, resourceName);
-};
+  const throwIfUsedByNotifications = async (selectedAnalyticsResource: string): Promise<void> => {
+    const isResourceInUse = await checkResourceInUseByNotifications(context, selectedAnalyticsResource);
+    if (isResourceInUse) {
+      throw new AmplifyError('ResourceInUseError', {
+        message: `Analytics resource ${selectedAnalyticsResource} is being used by the notifications category and cannot be removed`,
+        resolution: `Run 'amplify remove notifications', then retry removing analytics`,
+      });
+    }
+  };
 
-export const name = subcommand;
+  // remove resource with a resourceName callback that will block removal if selecting an analytics resource that notifications depends on
+  await amplify.removeResource(context, category, resourceName, { headless: false }, throwIfUsedByNotifications);
+};

packages/amplify-category-analytics/src/plugin-client-api-notifications.ts

@@ -31,6 +31,7 @@ export const invokeNotificationsAPIRecursiveRemoveApp = async (
  */
 export const checkResourceInUseByNotifications = async (context: $TSContext, resourceName: string): Promise<boolean> => {
   const notificationsResource = await invokeNotificationsAPIGetResource(context);
+  if (!notificationsResource?.resourceName) return false;
   return (notificationsResource?.resourceName === resourceName);
 };
 

packages/amplify-category-auth/src/__tests__/commands/remove.test.ts

@@ -1,130 +1,115 @@
+import { stateManager, $TSContext, AmplifyError } from 'amplify-cli-core';
+import { printer } from 'amplify-prompts';
 import * as remove from '../../commands/auth/remove';
 import { messages } from '../../provider-utils/awscloudformation/assets/string-maps';
-import { $TSContext } from 'amplify-cli-core';
-import { printer } from 'amplify-prompts';
 
-jest.mock('amplify-prompts', () => ({
-  printer: {
-    info: jest.fn(),
-  },
-}));
+jest.mock('amplify-prompts');
 
-const saveCLIInputPayload_mock = jest.fn();
+const saveCLIInputPayloadMock = jest.fn();
 
 jest.mock('fs-extra');
 
-jest.mock('../../provider-utils/awscloudformation/auth-inputs-manager/auth-input-state', () => {
-  return {
-    AuthInputState: jest.fn().mockImplementation(() => {
-      return {
-        isCLIInputsValid: jest.fn(),
-        getCLIInputPayload: jest.fn().mockImplementation(() => ({
-          cognitoConfig: {
-            userPoolGroupList: ['admin'],
-          },
-        })),
-        saveCLIInputPayload: saveCLIInputPayload_mock,
-      };
-    }),
-  };
-});
+jest.mock('../../provider-utils/awscloudformation/auth-inputs-manager/auth-input-state', () => ({
+  AuthInputState: jest.fn().mockImplementation(() => ({
+    isCLIInputsValid: jest.fn(),
+    getCLIInputPayload: jest.fn().mockImplementation(() => ({
+      cognitoConfig: {
+        userPoolGroupList: ['admin'],
+      },
+    })),
+    saveCLIInputPayload: saveCLIInputPayloadMock,
+  })),
+}));
 
-jest.mock('amplify-cli-core', () => ({
-  AmplifySupportedService: {
-    AUTH: 'Cognito',
-    COGNITOUSERPOOLGROUPS: 'Cognito-UserPool-Groups',
+jest.mock('amplify-cli-core');
+
+const stateManagerMock = stateManager as jest.Mocked<typeof stateManager>;
+stateManagerMock.getMeta.mockReturnValue({
+  api: {
+    mockResource1: {},
   },
-  stateManager: {
-    getMeta: jest
-      .fn()
-      .mockReturnValueOnce({
-        analytics: {
-          mockResource1: {},
-        },
-        api: {
-          mockResource1: {},
-        },
-        function: {
-          mockResource1: {},
-        },
-        storage: {
-          mockResource1: {},
-        },
-        auth: {
-          mockResource1: {
-            service: 'Cognito',
-          },
-        },
-      })
-      .mockReturnValueOnce({
-        auth: {
-          mockResource1: {
-            service: 'Cognito',
-          },
-          mockResource2: {
-            service: 'Cognito-UserPool-Groups',
-          },
-        },
-      })
-      .mockReturnValueOnce({
-        analytics: {
-          mockResource1: {},
-        },
-        api: {
-          mockResource1: {},
-        },
-        function: {
-          mockResource1: {},
-        },
-        storage: {
-          mockResource1: {},
-        },
-        auth: {
-          mockResource1: {
-            service: 'Cognito',
-          },
-        },
-      }),
+  function: {
+    mockResource1: {},
   },
-}));
+  storage: {
+    mockResource1: {},
+  },
+  auth: {
+    mockResource1: {
+      service: 'Cognito',
+    },
+  },
+});
 
-const removeResource_mock = jest
-  .fn()
-  .mockResolvedValue({
-    service: 'Cognito',
-  })
-  .mockResolvedValueOnce({
-    service: 'Cognito',
-  })
-  .mockResolvedValueOnce({
-    service: 'Cognito-UserPool-Groups',
-  });
+const AmplifyErrorMock = AmplifyError as jest.MockedClass<typeof AmplifyError>;
+AmplifyErrorMock.mockImplementation(() => (new Error('test error') as unknown) as any);
+
+const removeResourceMock = jest.fn().mockResolvedValue({
+  service: 'Cognito',
+});
 
 const warningString = messages.dependenciesExists;
 const mockContext = {
   amplify: {
-    removeResource: removeResource_mock,
+    removeResource: removeResourceMock,
   },
   parameters: {
     first: 'mockFirst',
   },
 };
-const context_stub_typed = mockContext as unknown as $TSContext;
+const ContextStubTyped = (mockContext as unknown) as $TSContext;
+
+beforeEach(() => jest.clearAllMocks());
 
 test(`remove method should detect existing categories metadata and display warning with no userPoolGroup`, async () => {
-  await remove.run(context_stub_typed);
-  expect(printer.info).toBeCalledWith(warningString);
-  expect(context_stub_typed.amplify.removeResource).toBeCalled();
+  await remove.run(ContextStubTyped);
+  expect(printer.warn).toBeCalledWith(warningString);
+  expect(ContextStubTyped.amplify.removeResource).toBeCalled();
 });
 
 test(`remove method should  not  display warning when there is no dependency with no userPoolGroup`, async () => {
-  jest.clearAllMocks();
-  await remove.run(context_stub_typed);
-  expect(printer.info).not.toBeCalledWith(warningString);
-  expect(context_stub_typed.amplify.removeResource).toBeCalled();
+  stateManagerMock.getMeta.mockReturnValueOnce({
+    auth: {
+      mockResource1: {
+        service: 'Cognito',
+      },
+      mockResource2: {
+        service: 'Cognito-UserPool-Groups',
+      },
+    },
+  });
+  await remove.run(ContextStubTyped);
+  expect(printer.warn).not.toBeCalledWith(warningString);
+  expect(ContextStubTyped.amplify.removeResource).toBeCalled();
 });
 
-test(`remove method should still be called even when warning displayed for existing category resource and remmoves userPool group`, async () => {
-  await remove.run(context_stub_typed);
-  expect(saveCLIInputPayload_mock).toBeCalledWith({ cognitoConfig: { userPoolGroupList: [] } });
+test(`remove called when warning displayed for existing category resource and removes userPool group`, async () => {
+  removeResourceMock.mockResolvedValueOnce({
+    service: 'Cognito-UserPool-Groups',
+  });
+  await remove.run(ContextStubTyped);
+  expect(saveCLIInputPayloadMock).toBeCalledWith({ cognitoConfig: { userPoolGroupList: [] } });
+});
+
+test('remove throws error if project has analytics', async () => {
+  stateManagerMock.getMeta.mockReturnValueOnce({
+    analytics: {
+      mockAnalytics1: {},
+    },
+    api: {
+      mockResource1: {},
+    },
+    function: {
+      mockResource1: {},
+    },
+    storage: {
+      mockResource1: {},
+    },
+    auth: {
+      mockResource1: {
+        service: 'Cognito',
+      },
+    },
+  });
+  await expect(() => remove.run(ContextStubTyped)).rejects.toThrowErrorMatchingInlineSnapshot(`"test error"`);
 });