From a9c9a56aa49ec50aaa2a6bbb14c4018cf2d5f0b0 Mon Sep 17 00:00:00 2001 From: Oscar Bazaldua <511911+oscb@users.noreply.github.com> Date: Wed, 20 Jul 2022 08:12:48 -0700 Subject: [PATCH 1/7] fix: upgrade sovran to v0.3.0, add concurrency safe getters --- example/package.json | 2 +- packages/core/package.json | 2 +- .../__tests__/__helpers__/mockEventStore.ts | 32 +++ .../__tests__/__helpers__/mockSegmentStore.ts | 48 ++--- .../__tests__/internal/trackDeepLinks.test.ts | 9 +- packages/core/src/analytics.ts | 23 +- packages/core/src/info.ts | 2 +- .../__tests__/QueueFlushingPlugin.test.ts | 89 ++++---- .../__tests__/SegmentDestination.test.ts | 7 +- packages/core/src/storage/helpers.ts | 25 +++ packages/core/src/storage/index.ts | 1 + packages/core/src/storage/sovranStorage.ts | 198 ++++++++++++------ packages/core/src/storage/types.ts | 7 +- .../src/AmplitudeSessionPlugin.tsx | 41 ++-- yarn.lock | 8 +- 15 files changed, 312 insertions(+), 182 deletions(-) create mode 100644 packages/core/src/__tests__/__helpers__/mockEventStore.ts create mode 100644 packages/core/src/storage/helpers.ts diff --git a/example/package.json b/example/package.json index 4264b13c5..07a782d03 100644 --- a/example/package.json +++ b/example/package.json @@ -24,7 +24,7 @@ "@react-native-community/masked-view": "^0.1.11", "@react-navigation/native": "^6.0.2", "@react-navigation/stack": "^6.0.7", - "@segment/sovran-react-native": "^0.2.8", + "@segment/sovran-react-native": "^0.3.0", "react": "17.0.2", "react-native": "0.67.3", "react-native-bootsplash": "^3.2.4", diff --git a/packages/core/package.json b/packages/core/package.json index 5254f22ee..73c9a2734 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -47,7 +47,7 @@ "homepage": "https://github.com/segmentio/analytics-react-native#readme", "dependencies": { "@react-native-async-storage/async-storage": "^1.15.17", - "@segment/sovran-react-native": "^0.2.8", + "@segment/sovran-react-native": "^0.3.0", "deepmerge": "^4.2.2", "js-base64": "^3.7.2", "nanoid": "^3.1.25", diff --git a/packages/core/src/__tests__/__helpers__/mockEventStore.ts b/packages/core/src/__tests__/__helpers__/mockEventStore.ts new file mode 100644 index 000000000..7b69aef2b --- /dev/null +++ b/packages/core/src/__tests__/__helpers__/mockEventStore.ts @@ -0,0 +1,32 @@ +import type { SegmentEvent } from '../../types'; +import { createMockStoreGetter } from './mockSegmentStore'; +// import { createMockStoreGetter } from './mockSegmentStore'; +import { createCallbackManager } from './utils'; + +export class MockEventStore { + private initialData: SegmentEvent[] = []; + private events: SegmentEvent[] = []; + + private callbackManager = createCallbackManager<{ events: SegmentEvent[] }>(); + + constructor(initialData?: SegmentEvent[]) { + this.events = [...(initialData ?? [])]; + this.initialData = JSON.parse(JSON.stringify(initialData ?? [])); + } + + reset = () => { + this.events = JSON.parse(JSON.stringify(this.initialData)); + }; + + getState = createMockStoreGetter(() => ({ events: this.events })); + + subscribe = (callback: (value: { events: SegmentEvent[] }) => void) => + this.callbackManager.register(callback); + + dispatch = ( + callback: (value: { events: SegmentEvent[] }) => { events: SegmentEvent[] } + ) => { + this.events = callback({ events: this.events }).events; + this.callbackManager.run({ events: this.events }); + }; +} diff --git a/packages/core/src/__tests__/__helpers__/mockSegmentStore.ts b/packages/core/src/__tests__/__helpers__/mockSegmentStore.ts index ab4bc91fa..0d523b43f 100644 --- a/packages/core/src/__tests__/__helpers__/mockSegmentStore.ts +++ b/packages/core/src/__tests__/__helpers__/mockSegmentStore.ts @@ -5,10 +5,10 @@ import type { DeepPartial, IntegrationSettings, SegmentAPIIntegrations, - SegmentEvent, UserInfoState, } from '../../types'; import { createCallbackManager } from './utils'; +import { createGetter } from '../../storage/helpers'; type Data = { isReady: boolean; @@ -35,32 +35,12 @@ const INITIAL_VALUES: Data = { }, }; -export class MockEventStore { - private initialData: SegmentEvent[] = []; - private events: SegmentEvent[] = []; - - private callbackManager = createCallbackManager<{ events: SegmentEvent[] }>(); - - constructor(initialData?: SegmentEvent[]) { - this.events = [...(initialData ?? [])]; - this.initialData = JSON.parse(JSON.stringify(initialData ?? [])); - } - - reset = () => { - this.events = JSON.parse(JSON.stringify(this.initialData)); - }; - - getState = () => this.events; - - subscribe = (callback: (value: { events: SegmentEvent[] }) => void) => - this.callbackManager.register(callback); - - dispatch = ( - callback: (value: { events: SegmentEvent[] }) => { events: SegmentEvent[] } - ) => { - this.events = callback({ events: this.events }).events; - this.callbackManager.run({ events: this.events }); - }; +export function createMockStoreGetter(fn: () => T) { + return createGetter(fn, () => { + return new Promise((resolve) => { + resolve(fn()); + }); + }); } export class MockSegmentStore implements Storage { @@ -86,9 +66,9 @@ export class MockSegmentStore implements Storage { }; readonly isReady = { - get: () => { + get: createMockStoreGetter(() => { return this.data.isReady; - }, + }), onChange: (_callback: (value: boolean) => void) => { // Not doing anything cause this mock store is always ready, this is just legacy from the redux persistor return () => {}; @@ -96,7 +76,7 @@ export class MockSegmentStore implements Storage { }; readonly context = { - get: () => ({ ...this.data.context }), + get: createMockStoreGetter(() => ({ ...this.data.context })), onChange: (callback: (value?: DeepPartial) => void) => this.callbacks.context.register(callback), set: (value: DeepPartial) => { @@ -107,7 +87,7 @@ export class MockSegmentStore implements Storage { }; readonly settings = { - get: () => this.data.settings, + get: createMockStoreGetter(() => this.data.settings), onChange: ( callback: (value?: SegmentAPIIntegrations | undefined) => void ) => this.callbacks.settings.register(callback), @@ -123,7 +103,7 @@ export class MockSegmentStore implements Storage { }; readonly userInfo = { - get: () => this.data.userInfo, + get: createMockStoreGetter(() => this.data.userInfo), onChange: (callback: (value: UserInfoState) => void) => this.callbacks.userInfo.register(callback), set: (value: UserInfoState) => { @@ -134,9 +114,9 @@ export class MockSegmentStore implements Storage { }; readonly deepLinkData = { - get: () => { + get: createMockStoreGetter(() => { return this.data.deepLinkData; - }, + }), set: (value: DeepLinkData) => { this.data.deepLinkData = value; this.callbacks.deepLinkData.run(value); diff --git a/packages/core/src/__tests__/internal/trackDeepLinks.test.ts b/packages/core/src/__tests__/internal/trackDeepLinks.test.ts index 445c5cc41..c920497ea 100644 --- a/packages/core/src/__tests__/internal/trackDeepLinks.test.ts +++ b/packages/core/src/__tests__/internal/trackDeepLinks.test.ts @@ -2,7 +2,10 @@ import { SegmentClient } from '../../analytics'; import { getMockLogger } from '../__helpers__/mockLogger'; import * as ReactNative from 'react-native'; import { EventType } from '../../types'; -import { MockSegmentStore } from '../__helpers__/mockSegmentStore'; +import { + createMockStoreGetter, + MockSegmentStore, +} from '../__helpers__/mockSegmentStore'; jest .spyOn(Date.prototype, 'toISOString') @@ -40,7 +43,9 @@ describe('#trackDeepLinks', () => { url: 'myapp://open', referring_application: 'Safari', }; - jest.spyOn(store.deepLinkData, 'get').mockReturnValue(deepLinkData); + jest + .spyOn(store.deepLinkData, 'get') + .mockImplementation(createMockStoreGetter(() => deepLinkData)); const client = new SegmentClient(clientArgs); jest.spyOn(client, 'process'); diff --git a/packages/core/src/analytics.ts b/packages/core/src/analytics.ts index 9f1b005ff..640407162 100644 --- a/packages/core/src/analytics.ts +++ b/packages/core/src/analytics.ts @@ -1,4 +1,5 @@ -import type { Unsubscribe } from '@segment/sovran-react-native'; +//@ts-ignore +import { Unsubscribe } from '@segment/sovran-react-native'; import deepmerge from 'deepmerge'; import allSettled from 'promise.allsettled'; import { AppState, AppStateStatus } from 'react-native'; @@ -16,7 +17,13 @@ import type { Logger } from './logger'; import type { DestinationPlugin, PlatformPlugin, Plugin } from './plugin'; import { InjectContext } from './plugins/Context'; import { SegmentDestination } from './plugins/SegmentDestination'; -import type { DeepLinkData, Settable, Storage, Watchable } from './storage'; +import { + createGetter, + DeepLinkData, + Settable, + Storage, + Watchable, +} from './storage'; import { Timeline } from './timeline'; import { Config, @@ -168,7 +175,13 @@ export class SegmentClient { }; this.adTrackingEnabled = { - get: () => this.store.context.get()?.device?.adTrackingEnabled ?? false, + get: createGetter( + () => this.store.context.get()?.device?.adTrackingEnabled ?? false, + async () => { + const context = await this.store.context.get(true); + return context?.device?.adTrackingEnabled ?? false; + } + ), onChange: (callback: (value: boolean) => void) => this.store.context.onChange((context?: DeepPartial) => { callback(context?.device?.adTrackingEnabled ?? false); @@ -244,7 +257,7 @@ export class SegmentClient { const resJson = await res.json(); const integrations = resJson.integrations; this.logger.info(`Received settings from Segment succesfully.`); - this.store.settings.set(integrations); + await this.store.settings.set(integrations); } catch { this.logger.warn( `Could not receive settings from Segment. ${ @@ -254,7 +267,7 @@ export class SegmentClient { }` ); if (this.config.defaultSettings) { - this.store.settings.set(this.config.defaultSettings); + await this.store.settings.set(this.config.defaultSettings); } } } diff --git a/packages/core/src/info.ts b/packages/core/src/info.ts index df958c944..77be43e94 100644 --- a/packages/core/src/info.ts +++ b/packages/core/src/info.ts @@ -1,4 +1,4 @@ export const libraryInfo = { name: '@segment/analytics-react-native', - version: '2.3.2', + version: '2.4.0', }; diff --git a/packages/core/src/plugins/__tests__/QueueFlushingPlugin.test.ts b/packages/core/src/plugins/__tests__/QueueFlushingPlugin.test.ts index 5d25233bf..30dd765aa 100644 --- a/packages/core/src/plugins/__tests__/QueueFlushingPlugin.test.ts +++ b/packages/core/src/plugins/__tests__/QueueFlushingPlugin.test.ts @@ -1,40 +1,44 @@ +import { MockEventStore } from '../../__tests__/__helpers__/mockEventStore'; +import type { SegmentClient } from '../../analytics'; import { QueueFlushingPlugin } from '../QueueFlushingPlugin'; -import { SegmentClient } from '../../analytics'; -import { getMockLogger } from '../../__tests__/__helpers__/mockLogger'; -import { - MockEventStore, - MockSegmentStore, -} from '../../__tests__/__helpers__/mockSegmentStore'; import { EventType, SegmentEvent } from '../../types'; +import { createStore } from '@segment/sovran-react-native'; -jest.mock('@segment/sovran-react-native', () => ({ - createStore: () => new MockEventStore(), -})); +jest.mock('@segment/sovran-react-native'); describe('QueueFlushingPlugin', () => { + function setupQueuePlugin( + onFlush: (events: SegmentEvent[]) => Promise, + flushAt: number + ) { + const queuePlugin = new QueueFlushingPlugin(onFlush); + // We override the createStore before the queue plugin is initialized to use our own mocked event store + (createStore as jest.Mock).mockReturnValue(new MockEventStore()); + queuePlugin.configure({ + getConfig: () => ({ + writeKey: 'SEGMENT_KEY', + flushAt, + }), + } as unknown as SegmentClient); + + // Mock the create store just before the queue plugin creates its store + return queuePlugin; + } + beforeEach(() => { jest.clearAllMocks(); + jest.resetModules(); }); it('should queue events when executed in the timeline', async () => { const onFlush = jest.fn(); - const queuePlugin = new QueueFlushingPlugin(onFlush); - - const client = new SegmentClient({ - config: { - writeKey: 'SEGMENT_KEY', - flushAt: 10, - }, - logger: getMockLogger(), - store: new MockSegmentStore(), - }); - queuePlugin.configure(client); + const queuePlugin = setupQueuePlugin(onFlush, 10); const result = queuePlugin.execute({ type: EventType.TrackEvent, - event: 'test', + event: 'test1', properties: { - test: 'test', + test: 'test1', }, } as SegmentEvent); @@ -44,7 +48,7 @@ describe('QueueFlushingPlugin', () => { await new Promise(process.nextTick); // @ts-ignore - expect(queuePlugin.queueStore?.getState()).toHaveLength(1); + expect(queuePlugin.queueStore?.getState().events).toHaveLength(1); // No flush called yet expect(onFlush).not.toHaveBeenCalled(); @@ -52,17 +56,7 @@ describe('QueueFlushingPlugin', () => { it('should call onFlush when queue reaches limit', async () => { const onFlush = jest.fn().mockResolvedValue(undefined); - const queuePlugin = new QueueFlushingPlugin(onFlush); - - const client = new SegmentClient({ - config: { - writeKey: 'SEGMENT_KEY', - flushAt: 1, - }, - logger: getMockLogger(), - store: new MockSegmentStore(), - }); - queuePlugin.configure(client); + const queuePlugin = setupQueuePlugin(onFlush, 1); const result = queuePlugin.execute({ type: EventType.TrackEvent, @@ -82,25 +76,17 @@ describe('QueueFlushingPlugin', () => { it('should dequeue events on demand', async () => { const onFlush = jest.fn().mockResolvedValue(undefined); - const queuePlugin = new QueueFlushingPlugin(onFlush); - - const client = new SegmentClient({ - config: { - writeKey: 'SEGMENT_KEY', - flushAt: 10, - }, - logger: getMockLogger(), - store: new MockSegmentStore(), - }); - queuePlugin.configure(client); + const queuePlugin = setupQueuePlugin(onFlush, 10); - const result = queuePlugin.execute({ + const event: SegmentEvent = { type: EventType.TrackEvent, - event: 'test', + event: 'test2', properties: { - test: 'test', + test: 'test2', }, - } as SegmentEvent); + }; + + const result = queuePlugin.execute(event); expect(result).not.toBeUndefined(); @@ -108,6 +94,9 @@ describe('QueueFlushingPlugin', () => { await new Promise(process.nextTick); // @ts-ignore - expect(queuePlugin.queueStore?.getState()).toHaveLength(1); + expect(queuePlugin.queueStore?.getState().events).toHaveLength(1); + queuePlugin.dequeue(event); + // @ts-ignore + expect(queuePlugin.queueStore?.getState().events).toHaveLength(0); }); }); diff --git a/packages/core/src/plugins/__tests__/SegmentDestination.test.ts b/packages/core/src/plugins/__tests__/SegmentDestination.test.ts index 24840ff5a..ecc766fa8 100644 --- a/packages/core/src/plugins/__tests__/SegmentDestination.test.ts +++ b/packages/core/src/plugins/__tests__/SegmentDestination.test.ts @@ -4,7 +4,10 @@ import { SEGMENT_DESTINATION_KEY, } from '../SegmentDestination'; import { SegmentClient } from '../../analytics'; -import { MockSegmentStore } from '../../__tests__/__helpers__/mockSegmentStore'; +import { + createMockStoreGetter, + MockSegmentStore, +} from '../../__tests__/__helpers__/mockSegmentStore'; import { getMockLogger } from '../../__tests__/__helpers__/mockLogger'; import * as api from '../../api'; @@ -232,7 +235,7 @@ describe('SegmentDestination', () => { jest // @ts-ignore .spyOn(plugin.queuePlugin.queueStore!, 'getState') - .mockReturnValue({ events }); + .mockImplementation(createMockStoreGetter(() => ({ events }))); const sendEventsSpy = jest.spyOn(api, 'uploadEvents').mockResolvedValue(); diff --git a/packages/core/src/storage/helpers.ts b/packages/core/src/storage/helpers.ts new file mode 100644 index 000000000..8b3d8f2c0 --- /dev/null +++ b/packages/core/src/storage/helpers.ts @@ -0,0 +1,25 @@ +import type { getStateFunc } from './types'; + +/** + * Helper to create a function that can execute both async and async. + * Used for supporting Sovran's getState signature. e.g. + * - Async => enforces consistency by executing inline with the reducers + * - Sync => returns immediately with the current value, not awaiting for any reducer + * @param syncFunction code to execute when called synchronously + * @param asyncFunction code to execute when called async/ concurrency safe + * @returns a getStateFunc that can support both async and sync modes + */ +export function createGetter( + syncFunction: () => T, + asyncFunction: () => Promise +): getStateFunc { + function getState(): T; + function getState(safe: true): Promise; + function getState(safe?: boolean): T | Promise { + if (safe) { + return asyncFunction(); + } + return syncFunction(); + } + return getState; +} diff --git a/packages/core/src/storage/index.ts b/packages/core/src/storage/index.ts index 1b88119ad..c75ff09ed 100644 --- a/packages/core/src/storage/index.ts +++ b/packages/core/src/storage/index.ts @@ -1,2 +1,3 @@ export * from './types'; +export * from './helpers'; export * from './sovranStorage'; diff --git a/packages/core/src/storage/sovranStorage.ts b/packages/core/src/storage/sovranStorage.ts index 0ca5f60ac..1512ee9fc 100644 --- a/packages/core/src/storage/sovranStorage.ts +++ b/packages/core/src/storage/sovranStorage.ts @@ -14,7 +14,16 @@ import type { UserInfoState, } from '..'; import { getUUID } from '../uuid'; -import type { Storage, StorageConfig, DeepLinkData } from './types'; +import { createGetter } from './helpers'; +import type { + Storage, + StorageConfig, + DeepLinkData, + getStateFunc, + Watchable, + Settable, + Dictionary, +} from './types'; // NOTE: Not exported from @segment/sovran-react-native. Must explicitly declare here. // Also this fallback is used in store.ts in @segment/sovran-react-native yet "storeId" is required. @@ -76,6 +85,57 @@ registerBridgeStore({ }, }); +// function createGetter< +// U, +// Z extends keyof U | undefined = undefined, +// V = undefined +// >(store: Store, key?: Z): getStateFunc { +// type X = Z extends keyof U ? V : U; +// function getState(): X; +// function getState(safe: true): Promise; +// function getState(safe?: boolean): X | Promise { +// if (safe === true) { +// const promise = store.getState(true); + +// if (key !== undefined) { +// return promise.then((state) => state[key!]) as Promise; +// } +// return promise as Promise; +// } +// const state = store.getState(); +// if (key !== undefined) { +// return state[key!] as unknown as X; +// } +// return state as X; +// } +// return getState; +// } + +function createStoreGetter< + U, + Z extends keyof U | undefined = undefined, + V = undefined +>(store: Store, key?: Z): getStateFunc { + type X = Z extends keyof U ? V : U; + return createGetter( + () => { + const state = store.getState(); + if (key !== undefined) { + return state[key!] as unknown as X; + } + return state as X; + }, + () => { + const promise = store.getState(true); + + if (key !== undefined) { + return promise.then((state) => state[key!]) as Promise; + } + return promise as Promise; + } + ); +} + export class SovranStorage implements Storage { private storeId: string; private storePersistor?: Persistor; @@ -85,6 +145,19 @@ export class SovranStorage implements Storage { private userInfoStore: Store<{ userInfo: UserInfoState }>; private deepLinkStore: Store = deepLinkStore; + readonly isReady: Watchable; + + readonly context: Watchable | undefined> & + Settable>; + + readonly settings: Watchable & + Settable & + Dictionary; + + readonly userInfo: Watchable & Settable; + + readonly deepLinkData: Watchable; + constructor(config: StorageConfig) { this.storeId = config.storeId; this.storePersistor = config.storePersistor; @@ -99,6 +172,18 @@ export class SovranStorage implements Storage { }, } ); + + this.isReady = { + get: createStoreGetter(this.readinessStore, 'hasLoadedContext'), + onChange: (callback: (value: boolean) => void) => { + return this.readinessStore.subscribe((store) => { + if (store.hasLoadedContext) { + callback(true); + } + }); + }, + }; + this.contextStore = createStore( { context: INITIAL_VALUES.context }, { @@ -108,6 +193,18 @@ export class SovranStorage implements Storage { }, } ); + this.context = { + get: createStoreGetter(this.contextStore, 'context'), + onChange: (callback: (value?: DeepPartial) => void) => + this.contextStore.subscribe((store) => callback(store.context)), + set: async (value: DeepPartial) => { + const { context } = await this.contextStore.dispatch((state) => { + return { context: deepmerge(state.context, value) }; + }); + return context; + }, + }; + this.settingsStore = createStore( { settings: INITIAL_VALUES.settings }, { @@ -117,6 +214,25 @@ export class SovranStorage implements Storage { }, } ); + + this.settings = { + get: createStoreGetter(this.settingsStore, 'settings'), + onChange: ( + callback: (value?: SegmentAPIIntegrations | undefined) => void + ) => this.settingsStore.subscribe((store) => callback(store.settings)), + set: async (value: SegmentAPIIntegrations) => { + const { settings } = await this.settingsStore.dispatch((state) => { + return { settings: { ...state.settings, ...value } }; + }); + return settings; + }, + add: (key: string, value: IntegrationSettings) => { + this.settingsStore.dispatch((state) => ({ + settings: { ...state.settings, [key]: value }, + })); + }, + }; + this.userInfoStore = createStore( { userInfo: INITIAL_VALUES.userInfo }, { @@ -127,6 +243,24 @@ export class SovranStorage implements Storage { } ); + this.userInfo = { + get: createStoreGetter(this.userInfoStore, 'userInfo'), + onChange: (callback: (value: UserInfoState) => void) => + this.userInfoStore.subscribe((store) => callback(store.userInfo)), + set: async (value: UserInfoState) => { + const { userInfo } = await this.userInfoStore.dispatch((state) => ({ + userInfo: { ...state.userInfo, ...value }, + })); + return userInfo; + }, + }; + + this.deepLinkData = { + get: createStoreGetter(this.deepLinkStore), + onChange: (callback: (value: DeepLinkData) => void) => + this.deepLinkStore.subscribe(callback), + }; + this.fixAnonymousId(); // Wait for context to be loaded @@ -156,66 +290,4 @@ export class SovranStorage implements Storage { fixUnsubscribe(); }); }; - - // Check for all things that need to be ready before sending events through the timeline - readonly isReady = { - get: () => { - const ready = this.readinessStore.getState(); - return ready.hasLoadedContext; - }, - onChange: (callback: (value: boolean) => void) => { - return this.readinessStore.subscribe((store) => { - if (store.hasLoadedContext) { - callback(true); - } - }); - }, - }; - - readonly context = { - get: () => this.contextStore.getState().context, - onChange: (callback: (value?: DeepPartial) => void) => - this.contextStore.subscribe((store) => callback(store.context)), - set: async (value: DeepPartial) => { - const { context } = await this.contextStore.dispatch((state) => { - return { context: deepmerge(state.context, value) }; - }); - return context; - }, - }; - readonly settings = { - get: () => this.settingsStore.getState().settings, - onChange: ( - callback: (value?: SegmentAPIIntegrations | undefined) => void - ) => this.settingsStore.subscribe((store) => callback(store.settings)), - set: async (value: SegmentAPIIntegrations) => { - const { settings } = await this.settingsStore.dispatch((state) => { - return { settings: { ...state.settings, ...value } }; - }); - return settings; - }, - add: (key: string, value: IntegrationSettings) => { - this.settingsStore.dispatch((state) => ({ - settings: { ...state.settings, [key]: value }, - })); - }, - }; - - readonly userInfo = { - get: () => this.userInfoStore.getState().userInfo, - onChange: (callback: (value: UserInfoState) => void) => - this.userInfoStore.subscribe((store) => callback(store.userInfo)), - set: async (value: UserInfoState) => { - const { userInfo } = await this.userInfoStore.dispatch((state) => ({ - userInfo: { ...state.userInfo, ...value }, - })); - return userInfo; - }, - }; - - readonly deepLinkData = { - get: () => this.deepLinkStore.getState(), - onChange: (callback: (value: DeepLinkData) => void) => - this.deepLinkStore.subscribe(callback), - }; } diff --git a/packages/core/src/storage/types.ts b/packages/core/src/storage/types.ts index c19977217..de7603c84 100644 --- a/packages/core/src/storage/types.ts +++ b/packages/core/src/storage/types.ts @@ -7,6 +7,11 @@ import type { UserInfoState, } from '../types'; +export interface getStateFunc { + (): T; + (safe: true): Promise; +} + /** * Implements a value that can be subscribed for changes */ @@ -14,7 +19,7 @@ export interface Watchable { /** * Get current value */ - get: () => T; + get: getStateFunc; /** * Register a callback to be called when the value changes * @returns a function to unsubscribe diff --git a/packages/plugins/plugin-amplitudeSession/src/AmplitudeSessionPlugin.tsx b/packages/plugins/plugin-amplitudeSession/src/AmplitudeSessionPlugin.tsx index eceaf91ba..755795277 100644 --- a/packages/plugins/plugin-amplitudeSession/src/AmplitudeSessionPlugin.tsx +++ b/packages/plugins/plugin-amplitudeSession/src/AmplitudeSessionPlugin.tsx @@ -12,18 +12,19 @@ import { AliasEventType, } from '@segment/analytics-react-native'; +const MAX_SESSION_TIME_IN_MS = 300000; export class AmplitudeSessionPlugin extends EventPlugin { type = PluginType.enrichment; key = 'Actions Amplitude'; active = false; - sessionId = Date.now(); - sessionTimer = false; + sessionId: number | undefined; + sessionTimer: ReturnType | undefined; update(settings: SegmentAPISettings, _: UpdateType) { let integrations = settings.integrations; if (this.key in integrations) { this.active = true; - this.startSession(); + this.refreshSession(); } } @@ -32,7 +33,7 @@ export class AmplitudeSessionPlugin extends EventPlugin { return event; } - this.handleTimer(); + this.refreshSession(); let result = event; switch (result.type) { @@ -75,6 +76,10 @@ export class AmplitudeSessionPlugin extends EventPlugin { return this.insertSession(event) as AliasEventType; } + reset() { + this.resetSession(); + } + private insertSession = (event: SegmentEvent) => { const returnEvent = event; const integrations = event.integrations; @@ -87,23 +92,23 @@ export class AmplitudeSessionPlugin extends EventPlugin { return returnEvent; }; - private resetTimer = () => { - this.sessionTimer = false; - this.sessionId = -1; - }; - - private startSession = () => { - const maxSessionTime = 300000; - - setTimeout(() => this.resetTimer(), maxSessionTime); + private resetSession = () => { this.sessionId = Date.now(); - this.sessionTimer = true; + this.sessionTimer = undefined; }; - private handleTimer = () => { - if (!this.sessionTimer) { - this.startSession(); + private refreshSession = () => { + if (this.sessionId === undefined) { + this.sessionId = Date.now(); + } + + if (this.sessionTimer !== undefined) { + clearTimeout(this.sessionTimer); } - return; + + this.sessionTimer = setTimeout( + () => this.resetSession(), + MAX_SESSION_TIME_IN_MS + ); }; } diff --git a/yarn.lock b/yarn.lock index 59425c89c..d270b1943 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2360,10 +2360,10 @@ resolved "https://registry.yarnpkg.com/@react-native/polyfills/-/polyfills-2.0.0.tgz#4c40b74655c83982c8cf47530ee7dc13d957b6aa" integrity sha512-K0aGNn1TjalKj+65D7ycc1//H9roAQ51GJVk5ZJQFb2teECGmzd86bYDC0aYdbRf7gtovescq4Zt6FR0tgXiHQ== -"@segment/sovran-react-native@^0.2.8": - version "0.2.8" - resolved "https://registry.yarnpkg.com/@segment/sovran-react-native/-/sovran-react-native-0.2.8.tgz#76a3c29011f9726a0fa2ac3942fb1d7715816d7e" - integrity sha512-b3a2vfEj2+jb8w/o+rNrJESWUhHEtrRZgydRNg1PEmMDlLeh42T3mDAap4mtGFICRDHU57w2zPeuw+wfs/sk7g== +"@segment/sovran-react-native@^0.3.0": + version "0.3.0" + resolved "https://registry.yarnpkg.com/@segment/sovran-react-native/-/sovran-react-native-0.3.0.tgz#42c5ffe36252ce1bd04c7b43351b8aa62f1536a3" + integrity sha512-RRrQF1MhDW+Je6byKzsaC5qHg0IfXiGy5q/HrdKPO+NCK/emoIaIiqzIxlNpz8Kd/agEGW3yW1jU143krq8jPg== dependencies: "@react-native-async-storage/async-storage" "^1.15.15" ansi-regex "5.0.1" From f69b907042d9d3cbe7a44fe11886d288afea6410 Mon Sep 17 00:00:00 2001 From: Oscar Bazaldua <511911+oscb@users.noreply.github.com> Date: Wed, 20 Jul 2022 08:51:54 -0700 Subject: [PATCH 2/7] fix: support async processing in timeline --- packages/core/src/__tests__/timeline.test.ts | 12 ++++---- packages/core/src/plugin.ts | 22 +++++++++----- packages/core/src/plugins/Context.ts | 7 +++-- .../core/src/plugins/SegmentDestination.ts | 2 +- .../__tests__/SegmentDestination.test.ts | 24 +++++++-------- packages/core/src/timeline.ts | 30 +++++++++++++------ 6 files changed, 60 insertions(+), 37 deletions(-) diff --git a/packages/core/src/__tests__/timeline.test.ts b/packages/core/src/__tests__/timeline.test.ts index 97382a5c6..6386e3f28 100644 --- a/packages/core/src/__tests__/timeline.test.ts +++ b/packages/core/src/__tests__/timeline.test.ts @@ -54,7 +54,7 @@ describe('timeline', () => { } } - it('processes each destination independently', () => { + it('processes each destination independently', async () => { const timeline = new Timeline(); const goodPlugin = jest.fn().mockImplementation((e) => e); @@ -70,14 +70,14 @@ describe('timeline', () => { }, }; - const result = timeline.process(expectedEvent); + const result = await timeline.process(expectedEvent); expect(result).toEqual(expectedEvent); expect(goodPlugin).toHaveBeenCalled(); expect(badPlugin).toHaveBeenCalled(); }); - it('handles errors from plugins execution', () => { + it('handles errors from plugins execution', async () => { const timeline = new Timeline(); const goodPlugin = jest.fn().mockImplementation((e) => e); @@ -95,14 +95,14 @@ describe('timeline', () => { }, }; - const result = timeline.process(expectedEvent); + const result = await timeline.process(expectedEvent); expect(result).toEqual(expectedEvent); expect(goodPlugin).toHaveBeenCalled(); expect(badPlugin).toHaveBeenCalled(); }); - it('shortcircuits plugin execution if a plugin return undefined', () => { + it('shortcircuits plugin execution if a plugin return undefined', async () => { const timeline = new Timeline(); const goodPlugin = jest.fn().mockImplementation((e) => e); @@ -119,7 +119,7 @@ describe('timeline', () => { }, }; - const result = timeline.process(expectedEvent); + const result = await timeline.process(expectedEvent); expect(result).toEqual(undefined); expect(goodPlugin).not.toHaveBeenCalled(); diff --git a/packages/core/src/plugin.ts b/packages/core/src/plugin.ts index ffb192977..e199a7280 100644 --- a/packages/core/src/plugin.ts +++ b/packages/core/src/plugin.ts @@ -29,7 +29,9 @@ export class Plugin { // do nothing by default, user can override. } - execute(event: SegmentEvent): SegmentEvent | undefined { + execute( + event: SegmentEvent + ): Promise | SegmentEvent | undefined { // do nothing. return event; } @@ -40,7 +42,9 @@ export class Plugin { } export class EventPlugin extends Plugin { - execute(event: SegmentEvent): SegmentEvent | undefined { + execute( + event: SegmentEvent + ): Promise | SegmentEvent | undefined { if (event === undefined) { return event; } @@ -153,13 +157,13 @@ export class DestinationPlugin extends EventPlugin { this.timeline.remove(plugin); } - execute(event: SegmentEvent): SegmentEvent | undefined { + async execute(event: SegmentEvent): Promise { if (!this.isEnabled(event)) { return; } // Apply before and enrichment plugins - const beforeResult = this.timeline.applyPlugins({ + const beforeResult = await this.timeline.applyPlugins({ type: PluginType.before, event, }); @@ -168,16 +172,20 @@ export class DestinationPlugin extends EventPlugin { return; } - const enrichmentResult = this.timeline.applyPlugins({ + const enrichmentResult = await this.timeline.applyPlugins({ type: PluginType.enrichment, event: beforeResult, }); + if (enrichmentResult === undefined) { + return; + } + // Now send the event to the destination by executing the normal flow of an EventPlugin - super.execute(enrichmentResult); + await super.execute(enrichmentResult); // apply .after plugins - let afterResult = this.timeline.applyPlugins({ + let afterResult = await this.timeline.applyPlugins({ type: PluginType.after, event: enrichmentResult, }); diff --git a/packages/core/src/plugins/Context.ts b/packages/core/src/plugins/Context.ts index 8b920d9c7..5175eaeb8 100644 --- a/packages/core/src/plugins/Context.ts +++ b/packages/core/src/plugins/Context.ts @@ -4,12 +4,15 @@ import { PluginType, SegmentEvent } from '../types'; export class InjectContext extends PlatformPlugin { type = PluginType.before; - execute(event: SegmentEvent) { + async execute(event: SegmentEvent): Promise { + // We need to get the Context in a concurrency safe mode to permit changes (e.g. identify) to make it in before we retrieve it + const context = this.analytics!.context.get(true); + return { ...event, context: { ...event.context, - ...this.analytics!.context.get(), + ...context, }, }; } diff --git a/packages/core/src/plugins/SegmentDestination.ts b/packages/core/src/plugins/SegmentDestination.ts index 4160477b6..3989e2b12 100644 --- a/packages/core/src/plugins/SegmentDestination.ts +++ b/packages/core/src/plugins/SegmentDestination.ts @@ -69,7 +69,7 @@ export class SegmentDestination extends DestinationPlugin { this.add(this.queuePlugin); } - execute(event: SegmentEvent): SegmentEvent | undefined { + execute(event: SegmentEvent): Promise { // Execute the internal timeline here, the queue plugin will pick up the event and add it to the queue automatically const enrichedEvent = super.execute(event); return enrichedEvent; diff --git a/packages/core/src/plugins/__tests__/SegmentDestination.test.ts b/packages/core/src/plugins/__tests__/SegmentDestination.test.ts index ecc766fa8..6a5831f82 100644 --- a/packages/core/src/plugins/__tests__/SegmentDestination.test.ts +++ b/packages/core/src/plugins/__tests__/SegmentDestination.test.ts @@ -27,7 +27,7 @@ describe('SegmentDestination', () => { jest.clearAllMocks(); }); - it('executes', () => { + it('executes', async () => { const plugin = new SegmentDestination(); // @ts-ignore plugin.analytics = new SegmentClient(clientArgs); @@ -43,11 +43,11 @@ describe('SegmentDestination', () => { Firebase: false, }, }; - const result = plugin.execute(event); + const result = await plugin.execute(event); expect(result).toEqual(event); }); - it('disables device mode plugins to prevent dups', () => { + it('disables device mode plugins to prevent dups', async () => { const plugin = new SegmentDestination(); const analytics = new SegmentClient({ ...clientArgs, @@ -80,7 +80,7 @@ describe('SegmentDestination', () => { integrations: {}, }; - const result = plugin.execute(event); + const result = await plugin.execute(event); expect(result).toEqual({ ...event, _metadata: { @@ -91,7 +91,7 @@ describe('SegmentDestination', () => { }); }); - it('marks unbundled plugins where the cloud mode is disabled', () => { + it('marks unbundled plugins where the cloud mode is disabled', async () => { const plugin = new SegmentDestination(); const analytics = new SegmentClient({ ...clientArgs, @@ -124,7 +124,7 @@ describe('SegmentDestination', () => { integrations: {}, }; - const result = plugin.execute(event); + const result = await plugin.execute(event); expect(result).toEqual({ ...event, _metadata: { @@ -135,7 +135,7 @@ describe('SegmentDestination', () => { }); }); - it('marks maybeBundled integrations to unbundled if they are not bundled', () => { + it('marks maybeBundled integrations to unbundled if they are not bundled', async () => { const plugin = new SegmentDestination(); const analytics = new SegmentClient({ ...clientArgs, @@ -161,7 +161,7 @@ describe('SegmentDestination', () => { integrations: {}, }; - const result = plugin.execute(event); + const result = await plugin.execute(event); expect(result).toEqual({ ...event, _metadata: { @@ -172,7 +172,7 @@ describe('SegmentDestination', () => { }); }); - it('lets plugins/events override destination settings', () => { + it('lets plugins/events override destination settings', async () => { const plugin = new SegmentDestination(); // @ts-ignore plugin.analytics = new SegmentClient({ @@ -207,7 +207,7 @@ describe('SegmentDestination', () => { }, }; - const result = plugin.execute(event); + const result = await plugin.execute(event); expect(result).toEqual(event); }); @@ -262,7 +262,7 @@ describe('SegmentDestination', () => { }); }); - it('lets plugins/events disable destinations individually', () => { + it('lets plugins/events disable destinations individually', async () => { const plugin = new SegmentDestination(); // @ts-ignore plugin.analytics = new SegmentClient({ @@ -287,7 +287,7 @@ describe('SegmentDestination', () => { }, }; - const result = plugin.execute(event); + const result = await plugin.execute(event); expect(result).toEqual(undefined); }); }); diff --git a/packages/core/src/timeline.ts b/packages/core/src/timeline.ts index e3a5bd0cf..9ed575521 100644 --- a/packages/core/src/timeline.ts +++ b/packages/core/src/timeline.ts @@ -57,9 +57,11 @@ export class Timeline { getAllPlugins(this).forEach((plugin) => closure(plugin)); } - process(incomingEvent: SegmentEvent) { + async process( + incomingEvent: SegmentEvent + ): Promise { // apply .before and .enrichment types first ... - const beforeResult = this.applyPlugins({ + const beforeResult = await this.applyPlugins({ type: PluginType.before, event: incomingEvent, }); @@ -68,21 +70,25 @@ export class Timeline { return; } // .enrichment here is akin to source middleware in the old analytics-ios. - const enrichmentResult = this.applyPlugins({ + const enrichmentResult = await this.applyPlugins({ type: PluginType.enrichment, event: beforeResult, }); + if (enrichmentResult === undefined) { + return; + } + // once the event enters a destination, we don't want // to know about changes that happen there. those changes // are to only be received by the destination. - this.applyPlugins({ + await this.applyPlugins({ type: PluginType.destination, event: enrichmentResult, }); // apply .after plugins ... - let afterResult = this.applyPlugins({ + let afterResult = await this.applyPlugins({ type: PluginType.after, event: enrichmentResult, }); @@ -90,18 +96,24 @@ export class Timeline { return afterResult; } - applyPlugins({ type, event }: { type: PluginType; event: SegmentEvent }) { + async applyPlugins({ + type, + event, + }: { + type: PluginType; + event: SegmentEvent; + }): Promise { let result: SegmentEvent | undefined = event; const plugins = this.plugins[type]; if (plugins) { - plugins.forEach((plugin) => { + for (const plugin of plugins) { if (result) { try { const pluginResult = plugin.execute(result); // Each destination is independent from each other, so we don't roll over changes caused internally in each one of their processing if (type !== PluginType.destination) { - result = pluginResult; + result = await pluginResult; } } catch (error) { console.warn( @@ -111,7 +123,7 @@ export class Timeline { ); } } - }); + } } return result; } From f454bc34685ddf9d2a5e930c11e920bf6b041f53 Mon Sep 17 00:00:00 2001 From: Oscar Bazaldua <511911+oscb@users.noreply.github.com> Date: Wed, 20 Jul 2022 11:49:47 -0700 Subject: [PATCH 3/7] fix: make all client methods awaitable, fix userInfo race condition --- example/.detoxrc.json | 4 +- example/ios/Podfile.lock | 14 ++-- example/package.json | 2 +- example/src/Home.tsx | 3 +- example/yarn.lock | 8 +- packages/core/package.json | 2 +- .../__tests__/__helpers__/mockSegmentStore.ts | 44 +++++++---- .../internal/handleAppStateChange.test.ts | 8 +- .../core/src/__tests__/methods/alias.test.ts | 8 +- .../src/__tests__/methods/identify.test.ts | 21 ++--- packages/core/src/analytics.ts | 68 ++++++++--------- packages/core/src/client.tsx | 14 ++-- packages/core/src/plugins/Context.ts | 5 +- packages/core/src/storage/sovranStorage.ts | 76 +++++++++---------- packages/core/src/storage/types.ts | 2 +- packages/core/src/timeline.ts | 2 + packages/core/src/types.ts | 12 +-- yarn.lock | 8 +- 18 files changed, 163 insertions(+), 138 deletions(-) diff --git a/example/.detoxrc.json b/example/.detoxrc.json index a8bda975a..f8ad9d2ab 100644 --- a/example/.detoxrc.json +++ b/example/.detoxrc.json @@ -29,8 +29,8 @@ "simulator": { "type": "ios.simulator", "device": { - "name": "iPhone 11", - "os": "iOS 14.5" + "name": "iPhone 13", + "os": "iOS 15.4" } }, "emulator": { diff --git a/example/ios/Podfile.lock b/example/ios/Podfile.lock index e55ca8f96..8230dd6b7 100644 --- a/example/ios/Podfile.lock +++ b/example/ios/Podfile.lock @@ -286,12 +286,12 @@ PODS: - React - RNGestureHandler (2.3.0): - React-Core - - segment-analytics-react-native (2.2.0): + - segment-analytics-react-native (2.4.0): - React-Core - sovran-react-native - - segment-analytics-react-native-plugin-idfa (0.2.1): + - segment-analytics-react-native-plugin-idfa (0.4.0): - React-Core - - sovran-react-native (0.2.8): + - sovran-react-native (0.4.0): - React-Core - Yoga (1.14.0) @@ -459,11 +459,11 @@ SPEC CHECKSUMS: RNCAsyncStorage: b49b4e38a1548d03b74b30e558a1d18465b94be7 RNCMaskedView: 0e1bc4bfa8365eba5fbbb71e07fbdc0555249489 RNGestureHandler: 77d59828d40838c9fabb76a12d2d0a80c006906f - segment-analytics-react-native: d0b24d7b7e6e6968a3558a2c41f61e94420b6797 - segment-analytics-react-native-plugin-idfa: 80e5d610f537156833eabea12a1804523355de95 - sovran-react-native: e4064b633fd8232055d003460d5816dff87ba8cc + segment-analytics-react-native: b8ef30c1fc4feae75d98e03226cfc64716f6e0e3 + segment-analytics-react-native-plugin-idfa: 9befc7ff398e8a1fc49d353b7d74dab62a23e519 + sovran-react-native: 81407f2b9d1171f7106e7c9a8ef34de7a412caae Yoga: 90dcd029e45d8a7c1ff059e8b3c6612ff409061a PODFILE CHECKSUM: 0c7eb82d495ca56953c50916b7b49e7512632eb6 -COCOAPODS: 1.11.2 +COCOAPODS: 1.11.3 diff --git a/example/package.json b/example/package.json index 07a782d03..de41b351f 100644 --- a/example/package.json +++ b/example/package.json @@ -24,7 +24,7 @@ "@react-native-community/masked-view": "^0.1.11", "@react-navigation/native": "^6.0.2", "@react-navigation/stack": "^6.0.7", - "@segment/sovran-react-native": "^0.3.0", + "@segment/sovran-react-native": "^0.4.0", "react": "17.0.2", "react-native": "0.67.3", "react-native-bootsplash": "^3.2.4", diff --git a/example/src/Home.tsx b/example/src/Home.tsx index e6ea6736d..5064a5029 100644 --- a/example/src/Home.tsx +++ b/example/src/Home.tsx @@ -40,7 +40,8 @@ const Home = ({ navigation }: { navigation: any }) => { name: 'Identify', testID: 'BUTTON_IDENTIFY', onPress: () => { - identify('user_2', { username: 'simplyTheBest' }); + identify('user_oberto', { username: 'oberto' }); + track('testing identify should be oberto'); }, }, { diff --git a/example/yarn.lock b/example/yarn.lock index 71cf6c52b..9a88913e0 100644 --- a/example/yarn.lock +++ b/example/yarn.lock @@ -1575,10 +1575,10 @@ color "^3.1.3" warn-once "^0.1.0" -"@segment/sovran-react-native@^0.2.8": - version "0.2.8" - resolved "https://registry.yarnpkg.com/@segment/sovran-react-native/-/sovran-react-native-0.2.8.tgz#76a3c29011f9726a0fa2ac3942fb1d7715816d7e" - integrity sha512-b3a2vfEj2+jb8w/o+rNrJESWUhHEtrRZgydRNg1PEmMDlLeh42T3mDAap4mtGFICRDHU57w2zPeuw+wfs/sk7g== +"@segment/sovran-react-native@^0.4.0": + version "0.4.0" + resolved "https://registry.yarnpkg.com/@segment/sovran-react-native/-/sovran-react-native-0.4.0.tgz#cdac0902657dbb0c16103601d7e3885f67979a69" + integrity sha512-dba3sEQteoZLZf3STAgl5xKjKQbeupvQxsCtp/9UcEDkhnfYEq9AIXXZ7iNVSFK+uAxfdlumZU4kfMBcsHzVUw== dependencies: "@react-native-async-storage/async-storage" "^1.15.15" ansi-regex "5.0.1" diff --git a/packages/core/package.json b/packages/core/package.json index 73c9a2734..17f2b5025 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -47,7 +47,7 @@ "homepage": "https://github.com/segmentio/analytics-react-native#readme", "dependencies": { "@react-native-async-storage/async-storage": "^1.15.17", - "@segment/sovran-react-native": "^0.3.0", + "@segment/sovran-react-native": "^0.4.0", "deepmerge": "^4.2.2", "js-base64": "^3.7.2", "nanoid": "^3.1.25", diff --git a/packages/core/src/__tests__/__helpers__/mockSegmentStore.ts b/packages/core/src/__tests__/__helpers__/mockSegmentStore.ts index 0d523b43f..1cd4d6c60 100644 --- a/packages/core/src/__tests__/__helpers__/mockSegmentStore.ts +++ b/packages/core/src/__tests__/__helpers__/mockSegmentStore.ts @@ -1,5 +1,11 @@ import { SEGMENT_DESTINATION_KEY } from '../../plugins/SegmentDestination'; -import type { DeepLinkData, Storage } from '../../storage'; +import type { + DeepLinkData, + Dictionary, + Settable, + Storage, + Watchable, +} from '../../storage'; import type { Context, DeepPartial, @@ -75,25 +81,34 @@ export class MockSegmentStore implements Storage { }, }; - readonly context = { + readonly context: Watchable | undefined> & + Settable> = { get: createMockStoreGetter(() => ({ ...this.data.context })), onChange: (callback: (value?: DeepPartial) => void) => this.callbacks.context.register(callback), - set: (value: DeepPartial) => { - this.data.context = { ...value }; - this.callbacks.context.run(value); + set: (value) => { + this.data.context = + value instanceof Function + ? value(this.data.context ?? {}) + : { ...value }; + this.callbacks.context.run(this.data.context); return this.data.context; }, }; - readonly settings = { + readonly settings: Watchable & + Settable & + Dictionary = { get: createMockStoreGetter(() => this.data.settings), onChange: ( callback: (value?: SegmentAPIIntegrations | undefined) => void ) => this.callbacks.settings.register(callback), - set: (value: SegmentAPIIntegrations) => { - this.data.settings = value; - this.callbacks.settings.run(value); + set: (value) => { + this.data.settings = + value instanceof Function + ? value(this.data.settings ?? {}) + : { ...value }; + this.callbacks.settings.run(this.data.settings); return this.data.settings; }, add: (key: string, value: IntegrationSettings) => { @@ -102,13 +117,16 @@ export class MockSegmentStore implements Storage { }, }; - readonly userInfo = { + readonly userInfo: Watchable & Settable = { get: createMockStoreGetter(() => this.data.userInfo), onChange: (callback: (value: UserInfoState) => void) => this.callbacks.userInfo.register(callback), - set: (value: UserInfoState) => { - this.data.userInfo = value; - this.callbacks.userInfo.run(value); + set: (value) => { + this.data.userInfo = + value instanceof Function + ? value(this.data.userInfo ?? {}) + : { ...value }; + this.callbacks.userInfo.run(this.data.userInfo); return this.data.userInfo; }, }; diff --git a/packages/core/src/__tests__/internal/handleAppStateChange.test.ts b/packages/core/src/__tests__/internal/handleAppStateChange.test.ts index 71939546b..8ddd8f032 100644 --- a/packages/core/src/__tests__/internal/handleAppStateChange.test.ts +++ b/packages/core/src/__tests__/internal/handleAppStateChange.test.ts @@ -86,8 +86,8 @@ describe('SegmentClient #handleAppStateChange', () => { event: 'Application Opened', properties: { from_background: true, - build: store.context.get().app?.build, - version: store.context.get().app?.version, + build: store.context.get()?.app?.build, + version: store.context.get()?.app?.version, }, type: EventType.TrackEvent, }); @@ -104,8 +104,8 @@ describe('SegmentClient #handleAppStateChange', () => { event: 'Application Opened', properties: { from_background: true, - build: store.context.get().app?.build, - version: store.context.get().app?.version, + build: store.context.get()?.app?.build, + version: store.context.get()?.app?.version, }, type: EventType.TrackEvent, }); diff --git a/packages/core/src/__tests__/methods/alias.test.ts b/packages/core/src/__tests__/methods/alias.test.ts index 362484206..425beef95 100644 --- a/packages/core/src/__tests__/methods/alias.test.ts +++ b/packages/core/src/__tests__/methods/alias.test.ts @@ -29,12 +29,12 @@ describe('methods #alias', () => { jest.clearAllMocks(); }); - it('adds the alias event correctly', () => { + it('adds the alias event correctly', async () => { const client = new SegmentClient(clientArgs); jest.spyOn(client, 'process'); - client.alias('new-user-id'); + await client.alias('new-user-id'); const expectedEvent = { previousId: 'current-user-id', @@ -52,7 +52,7 @@ describe('methods #alias', () => { }); }); - it('uses anonymousId in event if no userId in store', () => { + it('uses anonymousId in event if no userId in store', async () => { const client = new SegmentClient({ ...clientArgs, store: new MockSegmentStore({ @@ -64,7 +64,7 @@ describe('methods #alias', () => { }); jest.spyOn(client, 'process'); - client.alias('new-user-id'); + await client.alias('new-user-id'); const expectedEvent = { previousId: 'anonymousId', diff --git a/packages/core/src/__tests__/methods/identify.test.ts b/packages/core/src/__tests__/methods/identify.test.ts index b089b030f..5b6962196 100644 --- a/packages/core/src/__tests__/methods/identify.test.ts +++ b/packages/core/src/__tests__/methods/identify.test.ts @@ -34,11 +34,11 @@ describe('methods #identify', () => { jest.clearAllMocks(); }); - it('adds the identify event correctly', () => { + it('adds the identify event correctly', async () => { const client = new SegmentClient(clientArgs); jest.spyOn(client, 'process'); - client.identify('new-user-id', { name: 'Mary', age: 30 }); + await client.identify('new-user-id', { name: 'Mary', age: 30 }); const expectedEvent = { traits: { @@ -59,11 +59,11 @@ describe('methods #identify', () => { }); }); - it('does not update user traits when there are no new ones provided', () => { + it('does not update user traits when there are no new ones provided', async () => { const client = new SegmentClient(clientArgs); jest.spyOn(client, 'process'); - client.identify('new-user-id'); + await client.identify('new-user-id'); const expectedEvent = { traits: initialUserInfo.traits, @@ -80,15 +80,15 @@ describe('methods #identify', () => { }); }); - it('does not update userId when userId is undefined', () => { + it('does not update userId when userId is undefined', async () => { const client = new SegmentClient(clientArgs); jest.spyOn(client, 'process'); - client.identify(undefined, { name: 'Mary' }); + await client.identify(undefined, { name: 'Mary' }); const expectedEvent = { traits: { name: 'Mary', age: 30 }, - userId: undefined, + userId: 'current-user-id', type: 'identify', }; @@ -103,13 +103,13 @@ describe('methods #identify', () => { }); }); - it('does not persist identity traits accross events', () => { + it('does not persist identity traits accross events', async () => { const client = new SegmentClient(clientArgs); jest.spyOn(client, 'process'); // @ts-ignore accessing the internal timeline to check the processed events jest.spyOn(client.timeline, 'process'); - client.identify('new-user-id', { name: 'Mary', age: 30 }); + await client.identify('new-user-id', { name: 'Mary', age: 30 }); const expectedEvent = { traits: { @@ -131,6 +131,9 @@ describe('methods #identify', () => { client.track('track event'); + // Await all promises + await new Promise(process.nextTick); + // @ts-ignore expect(client.timeline.process).toHaveBeenLastCalledWith({ anonymousId: 'very-anonymous', diff --git a/packages/core/src/analytics.ts b/packages/core/src/analytics.ts index 640407162..39a05d923 100644 --- a/packages/core/src/analytics.ts +++ b/packages/core/src/analytics.ts @@ -388,8 +388,9 @@ export class SegmentClient { this.timeline.remove(plugin); } - process(incomingEvent: SegmentEvent) { - const event = applyRawEventData(incomingEvent, this.store.userInfo.get()); + async process(incomingEvent: SegmentEvent) { + const userData = await this.store.userInfo.get(true); + const event = applyRawEventData(incomingEvent, userData); if (this.store.isReady.get() === true) { this.timeline.process(event); } else { @@ -399,7 +400,7 @@ export class SegmentClient { private async trackDeepLinks() { if (this.getConfig().trackDeepLinks === true) { - const deepLinkProperties = this.store.deepLinkData.get(); + const deepLinkProperties = await this.store.deepLinkData.get(true); this.trackDeepLinkEvent(deepLinkProperties); this.store.deepLinkData.onChange((data) => { @@ -473,74 +474,71 @@ export class SegmentClient { return Promise.resolve(); } - screen(name: string, options?: JsonMap) { + async screen(name: string, options?: JsonMap) { const event = createScreenEvent({ name, properties: options, }); - this.process(event); + await this.process(event); this.logger.info('SCREEN event saved', event); } - track(eventName: string, options?: JsonMap) { + async track(eventName: string, options?: JsonMap) { const event = createTrackEvent({ event: eventName, properties: options, }); - this.process(event); + await this.process(event); this.logger.info('TRACK event saved', event); } - identify(userId?: string, userTraits?: UserTraits) { - const userInfo = this.store.userInfo.get(); - const { traits: currentUserTraits } = userInfo; - - const mergedTraits = { - ...currentUserTraits, - ...userTraits, - }; + async identify(userId?: string, userTraits?: UserTraits) { + const userData = await this.store.userInfo.set((state) => ({ + ...state, + userId: userId ?? state.userId, + traits: { + ...state.traits, + ...userTraits, + }, + })); const event = createIdentifyEvent({ - userId, - userTraits: mergedTraits, + userId: userData.userId, + userTraits: userData.traits, }); - this.store.userInfo.set({ - ...userInfo, - userId: userId ?? userInfo.userId, - traits: mergedTraits, - }); - - this.process(event); + await this.process(event); this.logger.info('IDENTIFY event saved', event); } - group(groupId: string, groupTraits?: GroupTraits) { + async group(groupId: string, groupTraits?: GroupTraits) { const event = createGroupEvent({ groupId, groupTraits, }); - this.process(event); + await this.process(event); this.logger.info('GROUP event saved', event); } - alias(newUserId: string) { - const { anonymousId, userId } = this.userInfo.get(); + async alias(newUserId: string) { + const { anonymousId, userId: previousUserId } = + await this.store.userInfo.get(true); + + await this.store.userInfo.set((state) => ({ + ...state, + userId: newUserId, + })); + const event = createAliasEvent({ anonymousId, - userId, + userId: previousUserId, newUserId, }); - this.store.userInfo.set({ - ...this.store.userInfo.get(), - userId: newUserId, - }); - - this.process(event); + await this.process(event); this.logger.info('ALIAS event saved', event); } diff --git a/packages/core/src/client.tsx b/packages/core/src/client.tsx index f9ed76ba1..3309986ed 100644 --- a/packages/core/src/client.tsx +++ b/packages/core/src/client.tsx @@ -61,12 +61,12 @@ export const useAnalytics = (): ClientMethods => { return {}; } return { - screen: (...args) => client.screen(...args), - track: (...args) => client.track(...args), - identify: (...args) => client.identify(...args), - flush: () => client.flush(), - group: (...args) => client.group(...args), - alias: (...args) => client.alias(...args), - reset: (...args) => client.reset(...args), + screen: async (...args) => client.screen(...args), + track: async (...args) => client.track(...args), + identify: async (...args) => client.identify(...args), + flush: async () => client.flush(), + group: async (...args) => client.group(...args), + alias: async (...args) => client.alias(...args), + reset: async (...args) => client.reset(...args), }; }; diff --git a/packages/core/src/plugins/Context.ts b/packages/core/src/plugins/Context.ts index 5175eaeb8..b5204f54d 100644 --- a/packages/core/src/plugins/Context.ts +++ b/packages/core/src/plugins/Context.ts @@ -5,8 +5,11 @@ export class InjectContext extends PlatformPlugin { type = PluginType.before; async execute(event: SegmentEvent): Promise { + console.warn('Inject Context!'); // We need to get the Context in a concurrency safe mode to permit changes (e.g. identify) to make it in before we retrieve it - const context = this.analytics!.context.get(true); + const context = await this.analytics!.context.get(true); + + console.log('-> context: ', context); return { ...event, diff --git a/packages/core/src/storage/sovranStorage.ts b/packages/core/src/storage/sovranStorage.ts index 1512ee9fc..1c0afb41c 100644 --- a/packages/core/src/storage/sovranStorage.ts +++ b/packages/core/src/storage/sovranStorage.ts @@ -85,32 +85,6 @@ registerBridgeStore({ }, }); -// function createGetter< -// U, -// Z extends keyof U | undefined = undefined, -// V = undefined -// >(store: Store, key?: Z): getStateFunc { -// type X = Z extends keyof U ? V : U; -// function getState(): X; -// function getState(safe: true): Promise; -// function getState(safe?: boolean): X | Promise { -// if (safe === true) { -// const promise = store.getState(true); - -// if (key !== undefined) { -// return promise.then((state) => state[key!]) as Promise; -// } -// return promise as Promise; -// } -// const state = store.getState(); -// if (key !== undefined) { -// return state[key!] as unknown as X; -// } -// return state as X; -// } -// return getState; -// } - function createStoreGetter< U, Z extends keyof U | undefined = undefined, @@ -125,13 +99,21 @@ function createStoreGetter< } return state as X; }, - () => { - const promise = store.getState(true); + async () => { + const promise = await store.getState(true); + console.log( + '[sovranStorage]', + 'createStoreGetter', + 'async', + key, + promise + ); if (key !== undefined) { - return promise.then((state) => state[key!]) as Promise; + return promise[key!] as unknown as X; + // return promise.then((state) => state[key!]) as Promise; } - return promise as Promise; + return promise as unknown as X; } ); } @@ -197,9 +179,15 @@ export class SovranStorage implements Storage { get: createStoreGetter(this.contextStore, 'context'), onChange: (callback: (value?: DeepPartial) => void) => this.contextStore.subscribe((store) => callback(store.context)), - set: async (value: DeepPartial) => { + set: async (value) => { const { context } = await this.contextStore.dispatch((state) => { - return { context: deepmerge(state.context, value) }; + let newState: typeof state.context; + if (value instanceof Function) { + newState = value(state.context); + } else { + newState = deepmerge(state.context, value); + } + return { context: newState }; }); return context; }, @@ -220,9 +208,15 @@ export class SovranStorage implements Storage { onChange: ( callback: (value?: SegmentAPIIntegrations | undefined) => void ) => this.settingsStore.subscribe((store) => callback(store.settings)), - set: async (value: SegmentAPIIntegrations) => { + set: async (value) => { const { settings } = await this.settingsStore.dispatch((state) => { - return { settings: { ...state.settings, ...value } }; + let newState: typeof state.settings; + if (value instanceof Function) { + newState = value(state.settings); + } else { + newState = { ...state.settings, ...value }; + } + return { settings: newState }; }); return settings; }, @@ -247,10 +241,16 @@ export class SovranStorage implements Storage { get: createStoreGetter(this.userInfoStore, 'userInfo'), onChange: (callback: (value: UserInfoState) => void) => this.userInfoStore.subscribe((store) => callback(store.userInfo)), - set: async (value: UserInfoState) => { - const { userInfo } = await this.userInfoStore.dispatch((state) => ({ - userInfo: { ...state.userInfo, ...value }, - })); + set: async (value) => { + const { userInfo } = await this.userInfoStore.dispatch((state) => { + let newState: typeof state.userInfo; + if (value instanceof Function) { + newState = value(state.userInfo); + } else { + newState = deepmerge(state.userInfo, value); + } + return { userInfo: newState }; + }); return userInfo; }, }; diff --git a/packages/core/src/storage/types.ts b/packages/core/src/storage/types.ts index de7603c84..d92fcc289 100644 --- a/packages/core/src/storage/types.ts +++ b/packages/core/src/storage/types.ts @@ -31,7 +31,7 @@ export interface Watchable { * Implements a value that can be set */ export interface Settable { - set: (value: T) => T | Promise; + set: (value: T | ((state: T) => T)) => T | Promise; } /** diff --git a/packages/core/src/timeline.ts b/packages/core/src/timeline.ts index 9ed575521..7b41f881a 100644 --- a/packages/core/src/timeline.ts +++ b/packages/core/src/timeline.ts @@ -113,7 +113,9 @@ export class Timeline { const pluginResult = plugin.execute(result); // Each destination is independent from each other, so we don't roll over changes caused internally in each one of their processing if (type !== PluginType.destination) { + console.log('[Timeline]', 'applyPlugins', plugin.type); result = await pluginResult; + console.log('[Timeline]', 'applyPlugins', plugin.type, result); } } catch (error) { console.warn( diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 4765797fc..3afcb30fd 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -130,13 +130,13 @@ export type Config = { }; export type ClientMethods = { - screen: (name: string, properties?: JsonMap) => void; - track: (event: string, properties?: JsonMap) => void; - identify: (userId?: string, userTraits?: UserTraits) => void; + screen: (name: string, properties?: JsonMap) => Promise; + track: (event: string, properties?: JsonMap) => Promise; + identify: (userId?: string, userTraits?: UserTraits) => Promise; flush: () => Promise; - group: (groupId: string, groupTraits?: GroupTraits) => void; - alias: (newUserId: string) => void; - reset: (resetAnonymousId?: boolean) => void; + group: (groupId: string, groupTraits?: GroupTraits) => Promise; + alias: (newUserId: string) => Promise; + reset: (resetAnonymousId?: boolean) => Promise; }; type ContextApp = { diff --git a/yarn.lock b/yarn.lock index d270b1943..e0febf8ce 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2360,10 +2360,10 @@ resolved "https://registry.yarnpkg.com/@react-native/polyfills/-/polyfills-2.0.0.tgz#4c40b74655c83982c8cf47530ee7dc13d957b6aa" integrity sha512-K0aGNn1TjalKj+65D7ycc1//H9roAQ51GJVk5ZJQFb2teECGmzd86bYDC0aYdbRf7gtovescq4Zt6FR0tgXiHQ== -"@segment/sovran-react-native@^0.3.0": - version "0.3.0" - resolved "https://registry.yarnpkg.com/@segment/sovran-react-native/-/sovran-react-native-0.3.0.tgz#42c5ffe36252ce1bd04c7b43351b8aa62f1536a3" - integrity sha512-RRrQF1MhDW+Je6byKzsaC5qHg0IfXiGy5q/HrdKPO+NCK/emoIaIiqzIxlNpz8Kd/agEGW3yW1jU143krq8jPg== +"@segment/sovran-react-native@^0.4.0": + version "0.4.0" + resolved "https://registry.yarnpkg.com/@segment/sovran-react-native/-/sovran-react-native-0.4.0.tgz#cdac0902657dbb0c16103601d7e3885f67979a69" + integrity sha512-dba3sEQteoZLZf3STAgl5xKjKQbeupvQxsCtp/9UcEDkhnfYEq9AIXXZ7iNVSFK+uAxfdlumZU4kfMBcsHzVUw== dependencies: "@react-native-async-storage/async-storage" "^1.15.15" ansi-regex "5.0.1" From bf7ee9a838a1a8f518873052d7bafb79b4393ea6 Mon Sep 17 00:00:00 2001 From: Oscar Bazaldua <511911+oscb@users.noreply.github.com> Date: Wed, 20 Jul 2022 12:03:19 -0700 Subject: [PATCH 4/7] chore: cleanup --- example/src/Home.tsx | 3 +-- packages/core/src/plugins/Context.ts | 6 +----- packages/core/src/storage/sovranStorage.ts | 8 -------- packages/core/src/timeline.ts | 2 -- 4 files changed, 2 insertions(+), 17 deletions(-) diff --git a/example/src/Home.tsx b/example/src/Home.tsx index 5064a5029..e6ea6736d 100644 --- a/example/src/Home.tsx +++ b/example/src/Home.tsx @@ -40,8 +40,7 @@ const Home = ({ navigation }: { navigation: any }) => { name: 'Identify', testID: 'BUTTON_IDENTIFY', onPress: () => { - identify('user_oberto', { username: 'oberto' }); - track('testing identify should be oberto'); + identify('user_2', { username: 'simplyTheBest' }); }, }, { diff --git a/packages/core/src/plugins/Context.ts b/packages/core/src/plugins/Context.ts index b5204f54d..c46957b0b 100644 --- a/packages/core/src/plugins/Context.ts +++ b/packages/core/src/plugins/Context.ts @@ -5,12 +5,8 @@ export class InjectContext extends PlatformPlugin { type = PluginType.before; async execute(event: SegmentEvent): Promise { - console.warn('Inject Context!'); - // We need to get the Context in a concurrency safe mode to permit changes (e.g. identify) to make it in before we retrieve it + // 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); - - console.log('-> context: ', context); - return { ...event, context: { diff --git a/packages/core/src/storage/sovranStorage.ts b/packages/core/src/storage/sovranStorage.ts index 1c0afb41c..8b81f4f69 100644 --- a/packages/core/src/storage/sovranStorage.ts +++ b/packages/core/src/storage/sovranStorage.ts @@ -101,14 +101,6 @@ function createStoreGetter< }, async () => { const promise = await store.getState(true); - console.log( - '[sovranStorage]', - 'createStoreGetter', - 'async', - key, - promise - ); - if (key !== undefined) { return promise[key!] as unknown as X; // return promise.then((state) => state[key!]) as Promise; diff --git a/packages/core/src/timeline.ts b/packages/core/src/timeline.ts index 7b41f881a..9ed575521 100644 --- a/packages/core/src/timeline.ts +++ b/packages/core/src/timeline.ts @@ -113,9 +113,7 @@ export class Timeline { const pluginResult = plugin.execute(result); // Each destination is independent from each other, so we don't roll over changes caused internally in each one of their processing if (type !== PluginType.destination) { - console.log('[Timeline]', 'applyPlugins', plugin.type); result = await pluginResult; - console.log('[Timeline]', 'applyPlugins', plugin.type, result); } } catch (error) { console.warn( From 85ff81413f904f58868b9f5090e77710596ddae0 Mon Sep 17 00:00:00 2001 From: Oscar Bazaldua <511911+oscb@users.noreply.github.com> Date: Wed, 20 Jul 2022 14:04:05 -0700 Subject: [PATCH 5/7] fix: revert detoxrc --- example/.detoxrc.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/example/.detoxrc.json b/example/.detoxrc.json index f8ad9d2ab..a8bda975a 100644 --- a/example/.detoxrc.json +++ b/example/.detoxrc.json @@ -29,8 +29,8 @@ "simulator": { "type": "ios.simulator", "device": { - "name": "iPhone 13", - "os": "iOS 15.4" + "name": "iPhone 11", + "os": "iOS 14.5" } }, "emulator": { From 93a3420d0e3f02ed7c3fdfda14f8e5aade084943 Mon Sep 17 00:00:00 2001 From: Oscar Bazaldua <511911+oscb@users.noreply.github.com> Date: Thu, 21 Jul 2022 09:09:25 -0700 Subject: [PATCH 6/7] fix: async methods for plugins --- packages/core/src/plugin.ts | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/core/src/plugin.ts b/packages/core/src/plugin.ts index e199a7280..50b307b62 100644 --- a/packages/core/src/plugin.ts +++ b/packages/core/src/plugin.ts @@ -48,7 +48,8 @@ export class EventPlugin extends Plugin { if (event === undefined) { return event; } - let result = event; + let result: Promise | SegmentEvent | undefined = + event; switch (result.type) { case EventType.IdentifyEvent: result = this.identify(result); @@ -71,23 +72,33 @@ export class EventPlugin extends Plugin { // Default implementations that forward the event. This gives plugin // implementors the chance to interject on an event. - identify(event: IdentifyEventType) { + identify( + event: IdentifyEventType + ): Promise | IdentifyEventType | undefined { return event; } - track(event: TrackEventType) { + track( + event: TrackEventType + ): Promise | TrackEventType | undefined { return event; } - screen(event: ScreenEventType) { + screen( + event: ScreenEventType + ): Promise | ScreenEventType | undefined { return event; } - alias(event: AliasEventType) { + alias( + event: AliasEventType + ): Promise | AliasEventType | undefined { return event; } - group(event: GroupEventType) { + group( + event: GroupEventType + ): Promise | GroupEventType | undefined { return event; } From 149101ec5a1e5d9854c73d50d1826d15eb3cf62e Mon Sep 17 00:00:00 2001 From: Oscar Bazaldua <511911+oscb@users.noreply.github.com> Date: Mon, 25 Jul 2022 10:08:59 -0700 Subject: [PATCH 7/7] chore: fix typos --- packages/core/src/storage/helpers.ts | 2 +- packages/core/src/storage/sovranStorage.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/src/storage/helpers.ts b/packages/core/src/storage/helpers.ts index 8b3d8f2c0..325a2ec53 100644 --- a/packages/core/src/storage/helpers.ts +++ b/packages/core/src/storage/helpers.ts @@ -1,7 +1,7 @@ import type { getStateFunc } from './types'; /** - * Helper to create a function that can execute both async and async. + * Helper to create a function that can execute both sync and async. * Used for supporting Sovran's getState signature. e.g. * - Async => enforces consistency by executing inline with the reducers * - Sync => returns immediately with the current value, not awaiting for any reducer diff --git a/packages/core/src/storage/sovranStorage.ts b/packages/core/src/storage/sovranStorage.ts index 8b81f4f69..833f4e22b 100644 --- a/packages/core/src/storage/sovranStorage.ts +++ b/packages/core/src/storage/sovranStorage.ts @@ -103,7 +103,6 @@ function createStoreGetter< const promise = await store.getState(true); if (key !== undefined) { return promise[key!] as unknown as X; - // return promise.then((state) => state[key!]) as Promise; } return promise as unknown as X; }