From acea2e1c34b0c8c4d275b973e1799caaf72f29a4 Mon Sep 17 00:00:00 2001 From: Oscar Bazaldua <511911+oscb@users.noreply.github.com> Date: Wed, 24 May 2023 16:32:18 -0700 Subject: [PATCH 1/3] fix: fix flush policies reference copy, add BackgroundPolicy --- packages/core/src/__mocks__/react-native.ts | 1 + packages/core/src/__tests__/analytics.test.ts | 77 +++++++++++++++++++ packages/core/src/analytics.ts | 49 +++++++----- packages/core/src/constants.ts | 5 +- .../__tests__/background-flush-policy.test.ts | 35 +++++++++ .../flushPolicies/background-flush-policy.ts | 38 +++++++++ .../flushPolicies/flush-policy-executer.ts | 2 +- packages/core/src/flushPolicies/index.ts | 1 + 8 files changed, 186 insertions(+), 22 deletions(-) create mode 100644 packages/core/src/flushPolicies/__tests__/background-flush-policy.test.ts create mode 100644 packages/core/src/flushPolicies/background-flush-policy.ts diff --git a/packages/core/src/__mocks__/react-native.ts b/packages/core/src/__mocks__/react-native.ts index c82011be..d79bdb32 100644 --- a/packages/core/src/__mocks__/react-native.ts +++ b/packages/core/src/__mocks__/react-native.ts @@ -3,6 +3,7 @@ import type { PlatformOSType } from 'react-native'; export const AppState = { addEventListener: jest.fn(), removeEventListener: jest.fn(), + currentState: 'active', }; export const Linking = { diff --git a/packages/core/src/__tests__/analytics.test.ts b/packages/core/src/__tests__/analytics.test.ts index e06398b2..f91efbdf 100644 --- a/packages/core/src/__tests__/analytics.test.ts +++ b/packages/core/src/__tests__/analytics.test.ts @@ -2,6 +2,7 @@ import type { AppStateStatus } from 'react-native'; import { AppState } from 'react-native'; import { SegmentClient } from '../analytics'; import { ErrorType, SegmentError } from '../errors'; +import { CountFlushPolicy, TimerFlushPolicy } from '../flushPolicies'; import { getMockLogger } from './__helpers__/mockLogger'; import { MockSegmentStore } from './__helpers__/mockSegmentStore'; @@ -172,4 +173,80 @@ describe('SegmentClient', () => { expect(errorHandler).toHaveBeenCalledWith(error); }); }); + + describe('Flush Policies', () => { + it('creates the default flush policies when config is empty', () => { + client = new SegmentClient({ + ...clientArgs, + config: { + ...clientArgs.config, + flushAt: undefined, + flushInterval: undefined, + }, + }); + const flushPolicies = client.getFlushPolicies(); + expect(flushPolicies.length).toBe(2); + }); + + it('setting flush policies is mutually exclusive with flushAt/Interval', () => { + client = new SegmentClient({ + ...clientArgs, + config: { + ...clientArgs.config, + flushAt: 5, + flushInterval: 30, + flushPolicies: [new CountFlushPolicy(1)], + }, + }); + const flushPolicies = client.getFlushPolicies(); + expect(flushPolicies.length).toBe(1); + }); + + it('setting flushAt/Interval to 0 should make the client have no uploads', () => { + client = new SegmentClient({ + ...clientArgs, + config: { + ...clientArgs.config, + flushAt: 0, + flushInterval: 0, + }, + }); + const flushPolicies = client.getFlushPolicies(); + expect(flushPolicies.length).toBe(0); + }); + + it('setting an empty array of policies should make the client have no uploads', () => { + client = new SegmentClient({ + ...clientArgs, + config: { + ...clientArgs.config, + flushAt: undefined, + flushInterval: undefined, + flushPolicies: [], + }, + }); + const flushPolicies = client.getFlushPolicies(); + expect(flushPolicies.length).toBe(0); + }); + + it('can add and remove policies, does not mutate original array', () => { + const policies = [new CountFlushPolicy(1), new TimerFlushPolicy(200)]; + client = new SegmentClient({ + ...clientArgs, + config: { + ...clientArgs.config, + flushAt: undefined, + flushInterval: undefined, + flushPolicies: policies, + }, + }); + expect(client.getFlushPolicies().length).toBe(policies.length); + + client.removeFlushPolicy(...policies); + expect(client.getFlushPolicies().length).toBe(0); + + client.addFlushPolicy(...policies); + expect(client.getFlushPolicies().length).toBe(policies.length); + }); + }); }); diff --git a/packages/core/src/analytics.ts b/packages/core/src/analytics.ts index f006a04a..adab5a88 100644 --- a/packages/core/src/analytics.ts +++ b/packages/core/src/analytics.ts @@ -5,7 +5,12 @@ import { AppStateStatus, NativeEventSubscription, } from 'react-native'; -import { settingsCDN, workspaceDestinationFilterKey } from './constants'; +import { + settingsCDN, + workspaceDestinationFilterKey, + defaultFlushInterval, + defaultFlushAt, +} from './constants'; import { getContext } from './context'; import { applyRawEventData, @@ -703,25 +708,31 @@ export class SegmentClient { * trigger flush */ private setupFlushPolicies() { - const flushPolicies = this.config.flushPolicies ?? []; - - // Compatibility with older arguments - if ( - this.config.flushAt !== undefined && - this.config.flushAt !== null && - this.config.flushAt > 0 - ) { - flushPolicies.push(new CountFlushPolicy(this.config.flushAt)); - } + const flushPolicies = []; - if ( - this.config.flushInterval !== undefined && - this.config.flushInterval !== null && - this.config.flushInterval > 0 - ) { - flushPolicies.push( - new TimerFlushPolicy(this.config.flushInterval * 1000) - ); + // If there are zero policies or flushAt/flushInterval use the defaults: + if (this.config.flushPolicies !== undefined) { + flushPolicies.push(...this.config.flushPolicies); + } else { + if ( + this.config.flushAt === undefined || + (this.config.flushAt !== null && this.config.flushAt > 0) + ) { + flushPolicies.push( + new CountFlushPolicy(this.config.flushAt ?? defaultFlushAt) + ); + } + + if ( + this.config.flushInterval === undefined || + (this.config.flushInterval !== null && this.config.flushInterval > 0) + ) { + flushPolicies.push( + new TimerFlushPolicy( + (this.config.flushInterval ?? defaultFlushInterval) * 1000 + ) + ); + } } this.flushPolicyExecuter = new FlushPolicyExecuter(flushPolicies, () => { diff --git a/packages/core/src/constants.ts b/packages/core/src/constants.ts index 43fffa55..a9dd5b87 100644 --- a/packages/core/src/constants.ts +++ b/packages/core/src/constants.ts @@ -6,8 +6,6 @@ export const settingsCDN = 'https://cdn-settings.segment.com/v1/projects'; export const defaultConfig: Config = { writeKey: '', - flushAt: 20, - flushInterval: 30, maxBatchSize: 1000, trackDeepLinks: false, trackAppLifecycleEvents: false, @@ -15,3 +13,6 @@ export const defaultConfig: Config = { }; export const workspaceDestinationFilterKey = ''; + +export const defaultFlushAt = 20; +export const defaultFlushInterval = 30; diff --git a/packages/core/src/flushPolicies/__tests__/background-flush-policy.test.ts b/packages/core/src/flushPolicies/__tests__/background-flush-policy.test.ts new file mode 100644 index 00000000..45ab22c7 --- /dev/null +++ b/packages/core/src/flushPolicies/__tests__/background-flush-policy.test.ts @@ -0,0 +1,35 @@ +import { AppState, AppStateStatus } from 'react-native'; +import { BackgroundFlushPolicy } from '../background-flush-policy'; + +describe('BackgroundFlushPolicy', () => { + it('triggers a flush when reaching limit', () => { + let updateCallback = (_val: AppStateStatus) => { + return; + }; + + const addSpy = jest + .spyOn(AppState, 'addEventListener') + .mockImplementation((_action, callback) => { + updateCallback = callback; + return { remove: jest.fn() }; + }); + + const policy = new BackgroundFlushPolicy(); + policy.start(); + const observer = jest.fn(); + + policy.shouldFlush.onChange(observer); + + expect(addSpy).toHaveBeenCalledTimes(1); + + updateCallback('background'); + expect(observer).toHaveBeenCalledWith(true); + observer.mockClear(); + + updateCallback('active'); + expect(observer).not.toHaveBeenCalled(); + + updateCallback('inactive'); + expect(observer).toHaveBeenCalledWith(true); + }); +}); diff --git a/packages/core/src/flushPolicies/background-flush-policy.ts b/packages/core/src/flushPolicies/background-flush-policy.ts new file mode 100644 index 00000000..397a5142 --- /dev/null +++ b/packages/core/src/flushPolicies/background-flush-policy.ts @@ -0,0 +1,38 @@ +import { + AppState, + AppStateStatus, + NativeEventSubscription, +} from 'react-native'; +import type { SegmentEvent } from '../types'; +import { FlushPolicyBase } from './types'; + +/** + * StatupFlushPolicy triggers a flush right away on client startup + */ +export class BackgroundFlushPolicy extends FlushPolicyBase { + private appStateSubscription?: NativeEventSubscription; + private appState: AppStateStatus = AppState.currentState; + + start() { + this.appStateSubscription = AppState.addEventListener( + 'change', + (nextAppState) => { + if ( + this.appState === 'active' && + ['inactive', 'background'].includes(nextAppState) + ) { + // When the app goes into the background we will trigger a flush + this.shouldFlush.value = true; + } + } + ); + } + + onEvent(_event: SegmentEvent): void { + // Nothing to do + } + + end(): void { + this.appStateSubscription?.remove(); + } +} diff --git a/packages/core/src/flushPolicies/flush-policy-executer.ts b/packages/core/src/flushPolicies/flush-policy-executer.ts index acb27ff1..92319c74 100644 --- a/packages/core/src/flushPolicies/flush-policy-executer.ts +++ b/packages/core/src/flushPolicies/flush-policy-executer.ts @@ -8,7 +8,7 @@ export class FlushPolicyExecuter { private onFlush: () => void; constructor(policies: FlushPolicy[], onFlush: () => void) { - this.policies = policies; + this.policies = [...policies]; this.onFlush = onFlush; // Now listen to changes on the flush policies shouldFlush diff --git a/packages/core/src/flushPolicies/index.ts b/packages/core/src/flushPolicies/index.ts index b7fd43f6..e340aaf0 100644 --- a/packages/core/src/flushPolicies/index.ts +++ b/packages/core/src/flushPolicies/index.ts @@ -2,3 +2,4 @@ export * from './types'; export * from './count-flush-policy'; export * from './timer-flush-policy'; export * from './startup-flush-policy'; +export * from './background-flush-policy'; From 0caaa0166914cdc2f83df798775dd183c77a7a05 Mon Sep 17 00:00:00 2001 From: Oscar Bazaldua <511911+oscb@users.noreply.github.com> Date: Wed, 24 May 2023 16:39:02 -0700 Subject: [PATCH 2/3] chore: update readme --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index fc16d940..5157bc45 100644 --- a/README.md +++ b/README.md @@ -124,7 +124,7 @@ You must pass at least the `writeKey`. Additional configuration options are list | `logger` | undefined | Custom logger instance to expose internal Segment client logging. | | `flushAt` | 20 | How many events to accumulate before sending events to the backend. | | `flushInterval` | 30 | In seconds, how often to send events to the backend. | -| `flushPolicies` | undefined | Add more granular control for when to flush, see [Adding or removing policies](#adding-or-removing-policies) | +| `flushPolicies` | undefined | Add more granular control for when to flush, see [Adding or removing policies](#adding-or-removing-policies). **Mutually exclusive with flushAt/flushInterval** | | `maxBatchSize` | 1000 | How many events to send to the API at once | | `trackAppLifecycleEvents` | false | Enable automatic tracking for [app lifecycle events](https://segment.com/docs/connections/spec/mobile/#lifecycle-events): application installed, opened, updated, backgrounded) | | `trackDeepLinks` | false | Enable automatic tracking for when the user opens the app via a deep link (Note: Requires additional setup on iOS, [see instructions](#ios-deep-link-tracking-setup)) | @@ -601,7 +601,7 @@ Refer to the following table for Plugins you can use to meet your tracking needs ## Controlling Upload With Flush Policies -To more granurily control when events are uploaded you can use `FlushPolicies` +To more granurily control when events are uploaded you can use `FlushPolicies`. **This will override any setting on `flushAt` and `flushInterval`, but you can use `CountFlushPolicy` and `TimerFlushPolicy` to have the same behaviour respectively.** A Flush Policy defines the strategy for deciding when to flush, this can be on an interval, on a certain time of day, after receiving a certain number of events or even after receiving a particular event. This gives you even more flexibility on when to send event to Segment. @@ -626,6 +626,7 @@ We have several standard FlushPolicies: - `CountFlushPolicy` triggers whenever a certain number of events is reached - `TimerFlushPolicy` triggers on an interval of milliseconds - `StartupFlushPolicy` triggers on client startup only +- `BackgroundFlushPolicy` triggers when the app goes into the background/inactive. ## Adding or removing policies From a7b021002ea3ed03df93767b159563f7d2ed02df Mon Sep 17 00:00:00 2001 From: Oscar Bazaldua <511911+oscb@users.noreply.github.com> Date: Thu, 25 May 2023 09:04:44 -0700 Subject: [PATCH 3/3] chore: remove timer from tests --- packages/core/src/__tests__/__helpers__/setupSegmentClient.ts | 1 + packages/core/src/__tests__/internal/fetchSettings.test.ts | 1 + packages/core/src/__tests__/methods/flush.test.ts | 1 + packages/core/src/__tests__/methods/group.test.ts | 1 + packages/core/src/__tests__/methods/screen.test.ts | 1 + packages/core/src/__tests__/methods/track.test.ts | 1 + packages/core/src/__tests__/timeline.test.ts | 1 + packages/core/src/plugins/__tests__/SegmentDestination.test.ts | 1 + .../plugin-clevertap/src/__tests__/ClevertapPlugin.test.ts | 1 + .../src/methods/___tests__/DeviceTokenPlugin.test.ts | 1 + .../plugin-mixpanel/src/methods/__tests__/MixpanelPlugin.test.ts | 1 + .../plugins/plugin-mixpanel/src/methods/__tests__/alias.test.ts | 1 + 12 files changed, 12 insertions(+) diff --git a/packages/core/src/__tests__/__helpers__/setupSegmentClient.ts b/packages/core/src/__tests__/__helpers__/setupSegmentClient.ts index 25ab6906..e6e86e85 100644 --- a/packages/core/src/__tests__/__helpers__/setupSegmentClient.ts +++ b/packages/core/src/__tests__/__helpers__/setupSegmentClient.ts @@ -23,6 +23,7 @@ export const createTestClient = ( config: { writeKey: 'mock-write-key', autoAddSegmentDestination: false, + flushInterval: 0, ...config, }, logger: getMockLogger(), diff --git a/packages/core/src/__tests__/internal/fetchSettings.test.ts b/packages/core/src/__tests__/internal/fetchSettings.test.ts index dc575fb1..1f385840 100644 --- a/packages/core/src/__tests__/internal/fetchSettings.test.ts +++ b/packages/core/src/__tests__/internal/fetchSettings.test.ts @@ -20,6 +20,7 @@ describe('internal #getSettings', () => { config: { writeKey: '123-456', defaultSettings: defaultIntegrationSettings, + flushInterval: 0, }, logger: getMockLogger(), store: store, diff --git a/packages/core/src/__tests__/methods/flush.test.ts b/packages/core/src/__tests__/methods/flush.test.ts index 0bcaba89..a16ba5ee 100644 --- a/packages/core/src/__tests__/methods/flush.test.ts +++ b/packages/core/src/__tests__/methods/flush.test.ts @@ -16,6 +16,7 @@ describe('methods #flush', () => { writeKey: '123-456', autoAddSegmentDestination: false, trackAppLifecycleEvents: false, + flushInterval: 0, }, logger: getMockLogger(), store: store, diff --git a/packages/core/src/__tests__/methods/group.test.ts b/packages/core/src/__tests__/methods/group.test.ts index 2b68470f..a4006c80 100644 --- a/packages/core/src/__tests__/methods/group.test.ts +++ b/packages/core/src/__tests__/methods/group.test.ts @@ -19,6 +19,7 @@ describe('methods #group', () => { const clientArgs = { config: { writeKey: 'mock-write-key', + flushInterval: 0, }, logger: getMockLogger(), store: store, diff --git a/packages/core/src/__tests__/methods/screen.test.ts b/packages/core/src/__tests__/methods/screen.test.ts index a97eda10..c2e44dce 100644 --- a/packages/core/src/__tests__/methods/screen.test.ts +++ b/packages/core/src/__tests__/methods/screen.test.ts @@ -19,6 +19,7 @@ describe('methods #screen', () => { const clientArgs = { config: { writeKey: 'mock-write-key', + flushInterval: 0, }, logger: getMockLogger(), store: store, diff --git a/packages/core/src/__tests__/methods/track.test.ts b/packages/core/src/__tests__/methods/track.test.ts index 8d8addac..8d285f47 100644 --- a/packages/core/src/__tests__/methods/track.test.ts +++ b/packages/core/src/__tests__/methods/track.test.ts @@ -21,6 +21,7 @@ describe('methods #track', () => { const clientArgs = { config: { writeKey: 'mock-write-key', + flushInterval: 0, }, logger: getMockLogger(), store: store, diff --git a/packages/core/src/__tests__/timeline.test.ts b/packages/core/src/__tests__/timeline.test.ts index baa8db30..4569f226 100644 --- a/packages/core/src/__tests__/timeline.test.ts +++ b/packages/core/src/__tests__/timeline.test.ts @@ -32,6 +32,7 @@ describe('timeline', () => { const clientArgs = { config: { writeKey: 'mock-write-key', + flushInterval: 0, }, logger: getMockLogger(), store: store, diff --git a/packages/core/src/plugins/__tests__/SegmentDestination.test.ts b/packages/core/src/plugins/__tests__/SegmentDestination.test.ts index 9dd51ac6..b959a66e 100644 --- a/packages/core/src/plugins/__tests__/SegmentDestination.test.ts +++ b/packages/core/src/plugins/__tests__/SegmentDestination.test.ts @@ -28,6 +28,7 @@ describe('SegmentDestination', () => { config: { writeKey: '123-456', maxBatchSize: 2, + flushInterval: 0, }, store, }; diff --git a/packages/plugins/plugin-clevertap/src/__tests__/ClevertapPlugin.test.ts b/packages/plugins/plugin-clevertap/src/__tests__/ClevertapPlugin.test.ts index c58d0bf3..866fe111 100644 --- a/packages/plugins/plugin-clevertap/src/__tests__/ClevertapPlugin.test.ts +++ b/packages/plugins/plugin-clevertap/src/__tests__/ClevertapPlugin.test.ts @@ -18,6 +18,7 @@ describe('ClevertapPlugin ', () => { config: { writeKey: '123-456', trackApplicationLifecycleEvents: true, + flushInterval: 0, }, store, }; diff --git a/packages/plugins/plugin-device-token/src/methods/___tests__/DeviceTokenPlugin.test.ts b/packages/plugins/plugin-device-token/src/methods/___tests__/DeviceTokenPlugin.test.ts index cef16ec8..c4b066ed 100644 --- a/packages/plugins/plugin-device-token/src/methods/___tests__/DeviceTokenPlugin.test.ts +++ b/packages/plugins/plugin-device-token/src/methods/___tests__/DeviceTokenPlugin.test.ts @@ -26,6 +26,7 @@ describe('DeviceTokenPlugin', () => { config: { writeKey: '123-456', trackApplicationLifecycleEvents: true, + flushInterval: 0, }, store, }; diff --git a/packages/plugins/plugin-mixpanel/src/methods/__tests__/MixpanelPlugin.test.ts b/packages/plugins/plugin-mixpanel/src/methods/__tests__/MixpanelPlugin.test.ts index 7591cd45..61b361b2 100644 --- a/packages/plugins/plugin-mixpanel/src/methods/__tests__/MixpanelPlugin.test.ts +++ b/packages/plugins/plugin-mixpanel/src/methods/__tests__/MixpanelPlugin.test.ts @@ -15,6 +15,7 @@ describe('MixpanelPlugin', () => { config: { writeKey: '123-456', trackApplicationLifecycleEvents: true, + flushInterval: 0, }, store, }; diff --git a/packages/plugins/plugin-mixpanel/src/methods/__tests__/alias.test.ts b/packages/plugins/plugin-mixpanel/src/methods/__tests__/alias.test.ts index 40e52bb5..3f5a5ff1 100644 --- a/packages/plugins/plugin-mixpanel/src/methods/__tests__/alias.test.ts +++ b/packages/plugins/plugin-mixpanel/src/methods/__tests__/alias.test.ts @@ -16,6 +16,7 @@ describe('#alias', () => { config: { writeKey: '123-456', trackApplicationLifecycleEvents: true, + flushInterval: 0, }, store, };