Skip to content

Commit

Permalink
fix: Prevent overwriting with fallbacks when updating endpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
cshfang committed May 2, 2024
1 parent 5f34186 commit d904325
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

import {
appId,
clientDemographic,
endpointId as defaultEndpointId,
uuid,
} from '../../testUtils/data';
Expand All @@ -12,7 +11,7 @@ export const getExpectedInput = ({
address,
attributes,
channelType,
demographic = clientDemographic as any,
demographic,
endpointId = defaultEndpointId,
location,
metrics,
Expand All @@ -28,30 +27,36 @@ export const getExpectedInput = ({
EffectiveDate: expect.any(String),
ChannelType: channelType,
Address: address,
Attributes: attributes,
Demographic: {
AppVersion: demographic.appVersion,
Locale: demographic.locale,
Make: demographic.make,
Model: demographic.model,
ModelVersion: demographic.modelVersion ?? demographic.version,
Platform: demographic.platform,
PlatformVersion: demographic.platformVersion,
Timezone: demographic.timezone,
},
Location: {
City: location?.city,
Country: location?.country,
Latitude: location?.latitude,
Longitude: location?.longitude,
PostalCode: location?.postalCode,
Region: location?.region,
},
...(attributes && { Attributes: attributes }),
...(demographic && {
Demographic: {
AppVersion: demographic.appVersion,
Locale: demographic.locale,
Make: demographic.make,
Model: demographic.model,
ModelVersion: demographic.modelVersion ?? demographic.version,
Platform: demographic.platform,
PlatformVersion: demographic.platformVersion,
Timezone: demographic.timezone,
},
}),
...(location && {
Location: {
City: location?.city,
Country: location?.country,
Latitude: location?.latitude,
Longitude: location?.longitude,
PostalCode: location?.postalCode,
Region: location?.region,
},
}),
Metrics: metrics,
OptOut: optOut,
User: {
UserId: userId,
UserAttributes: userAttributes,
},
...((userId || userAttributes) && {
User: {
UserId: userId,
UserAttributes: userAttributes,
},
}),
}),
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import {
clientDemographic,
credentials,
endpointId,
identityId,
region,
userAttributes,
userId,
userProfile,
uuid,
Expand Down Expand Up @@ -130,7 +132,8 @@ describe('Pinpoint Provider API: updateEndpoint', () => {
);
});

it('merges demographics', async () => {
it('merges demographics with client info on endpoint creation', async () => {
mockGetEndpointId.mockReturnValue(undefined);
const partialDemographic = { ...demographic } as any;
delete partialDemographic.make;
delete partialDemographic.model;
Expand All @@ -146,6 +149,7 @@ describe('Pinpoint Provider API: updateEndpoint', () => {
expect(mockClientUpdateEndpoint).toHaveBeenCalledWith(
{ credentials, region },
getExpectedInput({
endpointId: createdEndpointId,
demographic: {
...demographic,
make: clientDemographic.make,
Expand All @@ -155,6 +159,58 @@ describe('Pinpoint Provider API: updateEndpoint', () => {
);
});

it('does not merge demographics with client info on endpoint update', async () => {
const partialDemographic = { ...demographic } as any;
delete partialDemographic.make;
delete partialDemographic.model;
await updateEndpoint({
appId,
category,
credentials,
region,
userProfile: {
demographic: partialDemographic,
},
});
expect(mockClientUpdateEndpoint).toHaveBeenCalledWith(
{ credentials, region },
getExpectedInput({ demographic: partialDemographic }),
);
});

it('falls back to idenity id on endpoint creation', async () => {
mockGetEndpointId.mockReturnValue(undefined);
await updateEndpoint({
appId,
category,
credentials,
identityId,
region,
});
expect(mockClientUpdateEndpoint).toHaveBeenCalledWith(
{ credentials, region },
getExpectedInput({
endpointId: createdEndpointId,
userId: identityId,
}),
);
});

it('does not fall back to idenity id on endpoint update', async () => {
await updateEndpoint({
appId,
category,
credentials,
identityId,
region,
userAttributes,
});
expect(mockClientUpdateEndpoint).toHaveBeenCalledWith(
{ credentials, region },
getExpectedInput({ userAttributes }),
);
});

it('creates an endpoint if one is not already cached', async () => {
mockGetEndpointId.mockReturnValue(undefined);
await updateEndpoint({ appId, category, credentials, region });
Expand Down
3 changes: 3 additions & 0 deletions packages/core/__tests__/providers/pinpoint/testUtils/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ export const event = {
};
export const identityId = 'identity-id';
export const region = 'region';
export const userAttributes = {
attr: ['attr-value-one', 'attr-value-two'],
};
export const userId = 'user-id';
export const userProfile = {
customProperties: {
Expand Down
83 changes: 51 additions & 32 deletions packages/core/src/providers/pinpoint/apis/updateEndpoint.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import { amplifyUuid } from '../../../utils/amplifyUuid';
import { getClientInfo } from '../../../utils/getClientInfo';
import {
UpdateEndpointInput,
updateEndpoint as clientUpdateEndpoint,
} from '../../../awsClients/pinpoint';
import { UserProfile } from '../../../types';
import { amplifyUuid } from '../../../utils/amplifyUuid';
import { getClientInfo } from '../../../utils/getClientInfo';
import { PinpointUpdateEndpointInput } from '../types';
import { cacheEndpointId } from '../utils/cacheEndpointId';
import {
Expand Down Expand Up @@ -46,22 +47,34 @@ export const updateEndpoint = async ({
name,
plan,
} = userProfile ?? {};
const clientInfo = getClientInfo();

// only automatically populate the endpoint with client info and identity id upon endpoint creation to
// avoid overwriting the endpoint with these values every time the endpoint is updated
const demographicsFromClientInfo: UserProfile['demographic'] = {};
const resolvedUserId = createdEndpointId ? userId ?? identityId : userId;
if (createdEndpointId) {
const clientInfo = getClientInfo();
demographicsFromClientInfo.appVersion = clientInfo.appVersion;
demographicsFromClientInfo.make = clientInfo.make;
demographicsFromClientInfo.model = clientInfo.model;
demographicsFromClientInfo.modelVersion = clientInfo.version;
demographicsFromClientInfo.platform = clientInfo.platform;
}
const mergedDemographic = {
appVersion: clientInfo.appVersion,
make: clientInfo.make,
model: clientInfo.model,
modelVersion: clientInfo.version,
platform: clientInfo.platform,
...demographicsFromClientInfo,
...demographic,
};
const shouldAddAttributes = email || customProperties || name || plan;
const attributes = {
...(email && { email: [email] }),
...(name && { name: [name] }),
...(plan && { plan: [plan] }),
...customProperties,
};

const shouldAddDemographics = createdEndpointId || demographic;
const shouldAddAttributes = email || customProperties || name || plan;
const shouldAddUser = resolvedUserId || userAttributes;

const input: UpdateEndpointInput = {
ApplicationId: appId,
EndpointId: endpointId ?? createdEndpointId,
Expand All @@ -70,31 +83,37 @@ export const updateEndpoint = async ({
EffectiveDate: new Date().toISOString(),
ChannelType: channelType,
Address: address,
Attributes: shouldAddAttributes ? attributes : undefined,
Demographic: {
AppVersion: mergedDemographic.appVersion,
Locale: mergedDemographic.locale,
Make: mergedDemographic.make,
Model: mergedDemographic.model,
ModelVersion: mergedDemographic.modelVersion,
Platform: mergedDemographic.platform,
PlatformVersion: mergedDemographic.platformVersion,
Timezone: mergedDemographic.timezone,
},
Location: {
City: location?.city,
Country: location?.country,
Latitude: location?.latitude,
Longitude: location?.longitude,
PostalCode: location?.postalCode,
Region: location?.region,
},
...(shouldAddAttributes && { Attributes: attributes }),
...(shouldAddDemographics && {
Demographic: {
AppVersion: mergedDemographic.appVersion,
Locale: mergedDemographic.locale,
Make: mergedDemographic.make,
Model: mergedDemographic.model,
ModelVersion: mergedDemographic.modelVersion,
Platform: mergedDemographic.platform,
PlatformVersion: mergedDemographic.platformVersion,
Timezone: mergedDemographic.timezone,
},
}),
...(location && {
Location: {
City: location.city,
Country: location.country,
Latitude: location.latitude,
Longitude: location.longitude,
PostalCode: location.postalCode,
Region: location.region,
},
}),
Metrics: metrics,
OptOut: optOut,
User: {
UserId: userId ?? identityId,
UserAttributes: userAttributes,
},
...(shouldAddUser && {
User: {
UserId: resolvedUserId,
UserAttributes: userAttributes,
},
}),
},
};
try {
Expand Down

0 comments on commit d904325

Please sign in to comment.