From 30f923011d320aef5126d76c6a650787c6d116b1 Mon Sep 17 00:00:00 2001 From: Oscar Bazaldua <511911+oscb@users.noreply.github.com> Date: Wed, 24 May 2023 14:02:29 -0700 Subject: [PATCH] fix: removes Inject plugins --- example/src/plugins/InjectTraits.ts | 25 ---- .../src/__tests__/methods/identify.test.ts | 2 +- packages/core/src/analytics.ts | 109 ++++++++++++++--- packages/core/src/events.ts | 12 -- packages/core/src/plugins/InjectContext.ts | 18 --- packages/core/src/plugins/InjectUserInfo.ts | 69 ----------- .../plugins/__tests__/InjectUserInfo.test.ts | 111 ------------------ 7 files changed, 95 insertions(+), 251 deletions(-) delete mode 100644 example/src/plugins/InjectTraits.ts delete mode 100644 packages/core/src/plugins/InjectContext.ts delete mode 100644 packages/core/src/plugins/InjectUserInfo.ts delete mode 100644 packages/core/src/plugins/__tests__/InjectUserInfo.test.ts diff --git a/example/src/plugins/InjectTraits.ts b/example/src/plugins/InjectTraits.ts deleted file mode 100644 index c6c454f1..00000000 --- a/example/src/plugins/InjectTraits.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { - PluginType, - SegmentEvent, - PlatformPlugin, -} from '@segment/analytics-react-native'; - -/** - * Plugin that injects the user traits to every event - */ -export class InjectTraits extends PlatformPlugin { - type = PluginType.before; - - execute(event: SegmentEvent) { - return { - ...event, - context: { - ...event.context, - traits: { - ...event.context?.traits, - ...this.analytics!.userInfo.get().traits, - }, - }, - }; - } -} diff --git a/packages/core/src/__tests__/methods/identify.test.ts b/packages/core/src/__tests__/methods/identify.test.ts index b326d8a3..fb8ba627 100644 --- a/packages/core/src/__tests__/methods/identify.test.ts +++ b/packages/core/src/__tests__/methods/identify.test.ts @@ -110,7 +110,7 @@ describe('methods #identify', () => { }); }); - it('is concurrency safe', async () => { + it('adds userInfo to next events, concurrency safe', async () => { // We trigger an identify and do not await it, we do a track immediately and await. // The track call should have the correct values injected into it. void client.identify('new-user-id'); diff --git a/packages/core/src/analytics.ts b/packages/core/src/analytics.ts index f006a04a..7f81bd7d 100644 --- a/packages/core/src/analytics.ts +++ b/packages/core/src/analytics.ts @@ -8,7 +8,6 @@ import { import { settingsCDN, workspaceDestinationFilterKey } from './constants'; import { getContext } from './context'; import { - applyRawEventData, createAliasEvent, createGroupEvent, createIdentifyEvent, @@ -22,8 +21,6 @@ import { } from './flushPolicies'; import { FlushPolicyExecuter } from './flushPolicies/flush-policy-executer'; import type { DestinationPlugin, PlatformPlugin, Plugin } from './plugin'; -import { InjectContext } from './plugins/InjectContext'; -import { InjectUserInfo } from './plugins/InjectUserInfo'; import { SegmentDestination } from './plugins/SegmentDestination'; import { createGetter, @@ -33,7 +30,7 @@ import { Watchable, } from './storage'; import { Timeline } from './timeline'; -import type { DestinationFilters, SegmentAPISettings } from './types'; +import { DestinationFilters, EventType, SegmentAPISettings } from './types'; import { Config, Context, @@ -91,10 +88,7 @@ export class SegmentClient { private onPluginAddedObservers: OnPluginAddedCallback[] = []; - private readonly platformPlugins: PlatformPlugin[] = [ - new InjectUserInfo(), - new InjectContext(), - ]; + private readonly platformPlugins: PlatformPlugin[] = []; // Watchables /** @@ -256,9 +250,12 @@ export class SegmentClient { await this.storageReady(); } + // Get new settings from segment + // It's important to run this before checkInstalledVersion and trackDeeplinks to give time for destination plugins + // which make use of the settings object to initialize + await this.fetchSettings(); + await allSettled([ - // Get new settings from segment - this.fetchSettings(), // save the current installed version this.checkInstalledVersion(), // check if the app was opened from a deep link @@ -411,7 +408,8 @@ export class SegmentClient { } async process(incomingEvent: SegmentEvent) { - const event = applyRawEventData(incomingEvent); + const event = await this.applyRawEventData(incomingEvent); + if (this.isReady.value) { this.flushPolicyExecuter.notify(event); return this.timeline.process(event); @@ -663,10 +661,8 @@ export class SegmentClient { async reset(resetAnonymousId = true) { try { - const anonymousId = - resetAnonymousId === true - ? getUUID() - : this.store.userInfo.get().anonymousId; + const { anonymousId: currentId } = await this.store.userInfo.get(true); + const anonymousId = resetAnonymousId === true ? getUUID() : currentId; await this.store.userInfo.set({ anonymousId, @@ -766,4 +762,87 @@ export class SegmentClient { } this.config.errorHandler?.(error); } + + /** + * Injects context and userInfo data into the event, sets the messageId and timestamp + * This is handled outside of the timeline to prevent concurrency issues between plugins + * @param event Segment Event + * @returns event with data injected + */ + private applyRawEventData = async ( + event: SegmentEvent + ): Promise => { + const userInfo = await this.processUserInfo(event); + const context = await this.context.get(true); + + return { + ...event, + ...userInfo, + context: { + ...event.context, + ...context, + }, + messageId: getUUID(), + timestamp: new Date().toISOString(), + integrations: event.integrations ?? {}, + } as SegmentEvent; + }; + + /** + * Processes the userInfo to add to an event. + * For Identify and Alias: it saves the new userId and traits into the storage + * For all: set the userId and anonymousId from the current values + * @param event segment event + * @returns userInfo to inject to an event + */ + private processUserInfo = async ( + event: SegmentEvent + ): Promise> => { + // Order here is IMPORTANT! + // Identify and Alias userInfo set operations have to come as soon as possible + // Do not block the set by doing a safe get first as it might cause a race condition + // within events procesing in the timeline asyncronously + if (event.type === EventType.IdentifyEvent) { + const userInfo = await this.userInfo.set((state) => ({ + ...state, + userId: event.userId ?? state.userId, + traits: { + ...state.traits, + ...event.traits, + }, + })); + + return { + anonymousId: userInfo.anonymousId, + userId: event.userId ?? userInfo.userId, + traits: { + ...userInfo.traits, + ...event.traits, + }, + }; + } else if (event.type === EventType.AliasEvent) { + let previousUserId: string; + + const userInfo = await this.userInfo.set((state) => { + previousUserId = state.userId ?? state.anonymousId; + + return { + ...state, + userId: event.userId, + }; + }); + + return { + anonymousId: userInfo.anonymousId, + userId: event.userId, + previousId: previousUserId!, + }; + } + + const userInfo = await this.userInfo.get(true); + return { + anonymousId: userInfo.anonymousId, + userId: userInfo.userId, + }; + }; } diff --git a/packages/core/src/events.ts b/packages/core/src/events.ts index 78a63c7a..dc43fdde 100644 --- a/packages/core/src/events.ts +++ b/packages/core/src/events.ts @@ -1,5 +1,3 @@ -import { getUUID } from './uuid'; - import { GroupEventType, GroupTraits, @@ -10,7 +8,6 @@ import { UserTraits, AliasEventType, EventType, - SegmentEvent, } from './types'; export const createTrackEvent = ({ @@ -76,12 +73,3 @@ export const createAliasEvent = ({ userId: newUserId, previousId: userId ?? anonymousId, }); - -export const applyRawEventData = (event: SegmentEvent): SegmentEvent => { - return { - ...event, - messageId: getUUID(), - timestamp: new Date().toISOString(), - integrations: event.integrations ?? {}, - }; -}; diff --git a/packages/core/src/plugins/InjectContext.ts b/packages/core/src/plugins/InjectContext.ts deleted file mode 100644 index c46957b0..00000000 --- a/packages/core/src/plugins/InjectContext.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { PlatformPlugin } from '../plugin'; -import { PluginType, SegmentEvent } from '../types'; - -export class InjectContext extends PlatformPlugin { - type = PluginType.before; - - async execute(event: SegmentEvent): Promise { - // We need to get the Context in a concurrency safe mode to permit changes to make it in before we retrieve it - const context = await this.analytics!.context.get(true); - return { - ...event, - context: { - ...event.context, - ...context, - }, - }; - } -} diff --git a/packages/core/src/plugins/InjectUserInfo.ts b/packages/core/src/plugins/InjectUserInfo.ts deleted file mode 100644 index ef9a3864..00000000 --- a/packages/core/src/plugins/InjectUserInfo.ts +++ /dev/null @@ -1,69 +0,0 @@ -import { PlatformPlugin } from '../plugin'; -import { - AliasEventType, - EventType, - IdentifyEventType, - PluginType, - SegmentEvent, -} from '../types'; - -/** - * This plugin injects the userInfo data into the event and also stores the changes applied by identify and alias calls into the state - */ -export class InjectUserInfo extends PlatformPlugin { - type = PluginType.before; - - async execute(event: SegmentEvent): Promise { - // Order here is IMPORTANT! - // Identify and Alias userInfo set operations have to come as soon as possible - // Do not block the set by doing a safe get first as it might cause a race condition - // within events procesing in the timeline asyncronously - if (event.type === EventType.IdentifyEvent) { - const userInfo = await this.analytics!.userInfo.set((state) => ({ - ...state, - userId: event.userId ?? state.userId, - traits: { - ...state.traits, - ...event.traits, - }, - })); - - return { - ...event, - anonymousId: userInfo.anonymousId, - userId: event.userId ?? userInfo.userId, - traits: { - ...userInfo.traits, - ...event.traits, - }, - } as IdentifyEventType; - } else if (event.type === EventType.AliasEvent) { - let previousUserId: string; - - const userInfo = await this.analytics!.userInfo.set((state) => { - previousUserId = state.userId ?? state.anonymousId; - - return { - ...state, - userId: event.userId, - }; - }); - - return { - ...event, - anonymousId: userInfo.anonymousId, - userId: event.userId, - previousId: previousUserId!, - } as AliasEventType; - } - - const userInfo = await this.analytics!.userInfo.get(true); - const injectedEvent: SegmentEvent = { - ...event, - anonymousId: userInfo.anonymousId, - userId: userInfo.userId, - }; - - return injectedEvent; - } -} diff --git a/packages/core/src/plugins/__tests__/InjectUserInfo.test.ts b/packages/core/src/plugins/__tests__/InjectUserInfo.test.ts deleted file mode 100644 index d1d8abaf..00000000 --- a/packages/core/src/plugins/__tests__/InjectUserInfo.test.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { createIdentifyEvent, createTrackEvent } from '../../events'; -import { type SegmentEvent, EventType, UserTraits } from '../../types'; -import { createTestClient } from '../../__tests__/__helpers__/setupSegmentClient'; -import { InjectUserInfo } from '../InjectUserInfo'; - -describe('InjectContext', () => { - const currentUserId = 'current-user-id'; - const anonymousId = 'very-anonymous'; - - const initialUserInfo = { - traits: { - name: 'Stacy', - age: 30, - }, - userId: currentUserId, - anonymousId: anonymousId, - }; - const { store, client } = createTestClient({ - userInfo: initialUserInfo, - }); - - beforeEach(() => { - store.reset(); - jest.clearAllMocks(); - }); - - it('adds userId and anonymousId to events', async () => { - const expectedEvent: Partial = { - event: 'something', - userId: currentUserId, - anonymousId: anonymousId, - type: EventType.TrackEvent, - }; - - const plugin = new InjectUserInfo(); - plugin.configure(client); - const event = await plugin.execute( - createTrackEvent({ - event: 'something', - }) - ); - - expect(event).toMatchObject(expectedEvent); - }); - - it('updates userId and traits on identify', async () => { - const newUserId = 'new-user-id'; - const newTraits: UserTraits = { - age: 30, - }; - - const expectedEvent: Partial = { - userId: newUserId, - anonymousId: anonymousId, - traits: newTraits, - type: EventType.IdentifyEvent, - }; - - const plugin = new InjectUserInfo(); - plugin.configure(client); - const event = await plugin.execute( - createIdentifyEvent({ - userId: newUserId, - userTraits: newTraits, - }) - ); - - expect(event).toMatchObject(expectedEvent); - expect(client.userInfo.get()).toEqual({ - ...initialUserInfo, - userId: 'new-user-id', - }); - }); - - it('is concurrency safe', async () => { - const newUserId = 'new-user-id'; - const newTraits: UserTraits = { - age: 30, - }; - - const expectedEvent: Partial = { - userId: newUserId, - anonymousId: anonymousId, - type: EventType.TrackEvent, - }; - - const plugin = new InjectUserInfo(); - plugin.configure(client); - - // Do not wait the first execution - void plugin.execute( - createIdentifyEvent({ - userId: newUserId, - userTraits: newTraits, - }) - ); - - // But wait for the next one, which should contain the first changes - const event = await plugin.execute( - createTrackEvent({ - event: 'something', - }) - ); - - expect(event).toMatchObject(expectedEvent); - expect(client.userInfo.get()).toEqual({ - ...initialUserInfo, - userId: 'new-user-id', - }); - }); -});