fix: Prevent creating multiple endpoint ids (#13319)

packages/aws-amplify/package.json

@@ -317,7 +317,7 @@
 			"name": "[Analytics] identifyUser (Pinpoint)",
 			"path": "./dist/esm/analytics/index.mjs",
 			"import": "{ identifyUser }",
-			"limit": "15.52 kB"
+			"limit": "15.53 kB"
 		},
 		{
 			"name": "[Analytics] enable",

packages/core/__tests__/providers/pinpoint/apis/updateEndpoint.test.ts

@@ -1,13 +1,16 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-import { v4 } from 'uuid';
-
 import { getClientInfo } from '../../../../src/utils/getClientInfo';
 import { updateEndpoint as clientUpdateEndpoint } from '../../../../src/awsClients/pinpoint';
 import { cacheEndpointId } from '../../../../src/providers/pinpoint/utils/cacheEndpointId';
+import {
+	clearCreatedEndpointId,
+	createEndpointId,
+} from '../../../../src/providers/pinpoint/utils/createEndpointId';
 import { getEndpointId } from '../../../../src/providers/pinpoint/utils/getEndpointId';
 import { updateEndpoint } from '../../../../src/providers/pinpoint/apis';
+import { amplifyUuid } from '../../../../src/utils/amplifyUuid';
 import {
 	appId,
 	category,
@@ -22,10 +25,11 @@ import {
 
 import { getExpectedInput } from './testUtils/getExpectedInput';
 
-jest.mock('uuid');
 jest.mock('../../../../src/awsClients/pinpoint');
 jest.mock('../../../../src/providers/pinpoint/utils/cacheEndpointId');
+jest.mock('../../../../src/providers/pinpoint/utils/createEndpointId');
 jest.mock('../../../../src/providers/pinpoint/utils/getEndpointId');
+jest.mock('../../../../src/utils/amplifyUuid');
 jest.mock('../../../../src/utils/getClientInfo');
 
 describe('Pinpoint Provider API: updateEndpoint', () => {
@@ -41,14 +45,17 @@ describe('Pinpoint Provider API: updateEndpoint', () => {
 		timezone: 'user-timezone',
 	};
 	// assert mocks
+	const mockAmplifyUuid = amplifyUuid as jest.Mock;
 	const mockCacheEndpointId = cacheEndpointId as jest.Mock;
+	const mockClearCreatedEndpointId = clearCreatedEndpointId as jest.Mock;
+	const mockCreateEndpointId = createEndpointId as jest.Mock;
 	const mockClientUpdateEndpoint = clientUpdateEndpoint as jest.Mock;
 	const mockGetClientInfo = getClientInfo as jest.Mock;
 	const mockGetEndpointId = getEndpointId as jest.Mock;
-	const mockUuid = v4 as jest.Mock;
 
 	beforeAll(() => {
-		mockUuid.mockReturnValue(uuid);
+		mockAmplifyUuid.mockReturnValue(uuid);
+		mockCreateEndpointId.mockReturnValue(createdEndpointId);
 		mockGetClientInfo.mockReturnValue(clientDemographic);
 	});
 
@@ -58,7 +65,9 @@ describe('Pinpoint Provider API: updateEndpoint', () => {
 
 	afterEach(() => {
 		mockCacheEndpointId.mockClear();
-		mockClientUpdateEndpoint.mockClear();
+		mockClearCreatedEndpointId.mockClear();
+		mockCreateEndpointId.mockClear();
+		mockClientUpdateEndpoint.mockReset();
 		mockGetEndpointId.mockReset();
 	});
 
@@ -148,7 +157,6 @@ describe('Pinpoint Provider API: updateEndpoint', () => {
 
 	it('creates an endpoint if one is not already cached', async () => {
 		mockGetEndpointId.mockReturnValue(undefined);
-		mockUuid.mockReturnValueOnce(createdEndpointId);
 		await updateEndpoint({ appId, category, credentials, region });
 		expect(mockClientUpdateEndpoint).toHaveBeenCalledWith(
 			{ credentials, region },
@@ -159,10 +167,33 @@ describe('Pinpoint Provider API: updateEndpoint', () => {
 			category,
 			createdEndpointId,
 		);
+		expect(mockClearCreatedEndpointId).toHaveBeenCalledWith(appId, category);
+	});
+
+	it('does not create an endpoint if previously cached', async () => {
+		await updateEndpoint({ appId, category, credentials, region });
+		expect(mockCreateEndpointId).not.toHaveBeenCalled();
 	});
 
 	it('does not cache endpoint if previously cached', async () => {
 		await updateEndpoint({ appId, category, credentials, region });
 		expect(mockCacheEndpointId).not.toHaveBeenCalled();
+		expect(mockClearCreatedEndpointId).not.toHaveBeenCalled();
+	});
+
+	it('throws on update failure', async () => {
+		mockClientUpdateEndpoint.mockRejectedValue(new Error());
+		await expect(
+			updateEndpoint({ appId, category, credentials, region }),
+		).rejects.toThrow();
+	});
+
+	it('clears a created endpoint on update failure', async () => {
+		mockGetEndpointId.mockReturnValue(undefined);
+		mockClientUpdateEndpoint.mockRejectedValue(new Error());
+		await expect(
+			updateEndpoint({ appId, category, credentials, region }),
+		).rejects.toThrow();
+		expect(mockClearCreatedEndpointId).toHaveBeenCalledWith(appId, category);
 	});
 });

packages/core/__tests__/providers/pinpoint/utils/createEndpointId.test.ts

@@ -0,0 +1,54 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import {
+	clearCreatedEndpointId,
+	createEndpointId,
+} from '../../../../src/providers/pinpoint/utils/createEndpointId';
+import { amplifyUuid } from '../../../../src/utils/amplifyUuid';
+import { appId, category, uuid } from '../testUtils/data';
+
+jest.mock('../../../../src/utils/amplifyUuid');
+
+describe('Pinpoint Provider Util: createEndpointId', () => {
+	// assert mocks
+	const mockAmplifyUuid = amplifyUuid as jest.Mock;
+
+	afterEach(() => {
+		mockAmplifyUuid.mockReset();
+	});
+
+	it('returns a new endpoint id for a category', () => {
+		mockAmplifyUuid.mockReturnValue(uuid);
+
+		expect(createEndpointId(appId, category)).toBe(uuid);
+		expect(mockAmplifyUuid).toHaveBeenCalled();
+	});
+
+	it('returns the same endpoint id for a category', () => {
+		const newUuid = `new-${uuid}`;
+		mockAmplifyUuid.mockReturnValue(newUuid);
+
+		expect(createEndpointId(appId, category)).toBe(uuid);
+		expect(mockAmplifyUuid).not.toHaveBeenCalled();
+	});
+
+	it('returns a new endpoint id for a different category', () => {
+		const newUuid = `new-${uuid}`;
+		const newCategory = 'PushNotification';
+		mockAmplifyUuid.mockReturnValue(newUuid);
+
+		expect(createEndpointId(appId, newCategory)).toBe(newUuid);
+		expect(mockAmplifyUuid).toHaveBeenCalled();
+	});
+
+	describe('clearCreatedEndpointId()', () => {
+		it('can create a new endpoint id for a category after clearing', () => {
+			const newUuid = `new-${uuid}`;
+			mockAmplifyUuid.mockReturnValue(newUuid);
+			clearCreatedEndpointId(appId, category);
+
+			expect(createEndpointId(appId, category)).toBe(newUuid);
+		});
+	});
+});

packages/core/src/providers/pinpoint/apis/updateEndpoint.ts

@@ -9,6 +9,10 @@ import {
 } from '../../../awsClients/pinpoint';
 import { PinpointUpdateEndpointInput } from '../types';
 import { cacheEndpointId } from '../utils/cacheEndpointId';
+import {
+	clearCreatedEndpointId,
+	createEndpointId,
+} from '../utils/createEndpointId';
 import { getEndpointId } from '../utils/getEndpointId';
 
 /**
@@ -30,7 +34,9 @@ export const updateEndpoint = async ({
 }: PinpointUpdateEndpointInput): Promise<void> => {
 	const endpointId = await getEndpointId(appId, category);
 	// only generate a new endpoint id if one was not found in cache
-	const createdEndpointId = !endpointId ? amplifyUuid() : undefined;
+	const createdEndpointId = !endpointId
+		? createEndpointId(appId, category)
+		: undefined;
 	const {
 		customProperties,
 		demographic,
@@ -91,9 +97,17 @@ export const updateEndpoint = async ({
 			},
 		},
 	};
-	await clientUpdateEndpoint({ credentials, region, userAgentValue }, input);
-	// if we had to create an endpoint id, we need to now cache it
-	if (createdEndpointId) {
-		return cacheEndpointId(appId, category, createdEndpointId);
+	try {
+		await clientUpdateEndpoint({ credentials, region, userAgentValue }, input);
+		// if we had to create an endpoint id, we need to now cache it
+		if (createdEndpointId) {
+			await cacheEndpointId(appId, category, createdEndpointId);
+		}
+	} finally {
+		// at this point, we completely reset the behavior so even if the update was unsuccessful
+		// we can just start over with a newly created endpoint id
+		if (createdEndpointId) {
+			clearCreatedEndpointId(appId, category);
+		}
 	}
 };

packages/core/src/providers/pinpoint/utils/createEndpointId.ts

@@ -0,0 +1,39 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import { amplifyUuid } from '../../../utils/amplifyUuid';
+import { SupportedCategory } from '../types';
+
+import { getCacheKey } from './getCacheKey';
+
+const createdEndpointIds: Record<string, string> = {};
+
+/**
+ * Creates an endpoint id and guarantees multiple creations for a category returns the same uuid.
+ *
+ * @internal
+ */
+export const createEndpointId = (
+	appId: string,
+	category: SupportedCategory,
+): string => {
+	const cacheKey = getCacheKey(appId, category);
+	if (!createdEndpointIds[cacheKey]) {
+		createdEndpointIds[cacheKey] = amplifyUuid();
+	}
+
+	return createdEndpointIds[cacheKey];
+};
+
+/**
+ * Clears a created endpoint id for a category.
+ *
+ * @internal
+ */
+export const clearCreatedEndpointId = (
+	appId: string,
+	category: SupportedCategory,
+): void => {
+	const cacheKey = getCacheKey(appId, category);
+	delete createdEndpointIds[cacheKey];
+};

packages/core/src/providers/pinpoint/utils/index.ts

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 export { cacheEndpointId } from './cacheEndpointId';
+export { createEndpointId } from './createEndpointId';
 export { getCacheKey } from './getCacheKey';
 export { getEndpointId } from './getEndpointId';
 export { resolveEndpointId } from './resolveEndpointId';