From 9ab79d2b2dc478787df0c38cea11c4742771cc3c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 8 Mar 2022 17:04:03 -0300 Subject: [PATCH 1/4] move consent check from client to trackers --- package-lock.json | 2 +- package.json | 2 +- src/sdkClient/__tests__/client.spec.ts | 38 -------- src/sdkClient/client.ts | 9 +- src/sdkClient/clientInputValidation.ts | 15 +-- src/sdkClient/sdkClient.ts | 7 +- src/sdkFactory/index.ts | 6 +- src/sdkFactory/types.ts | 1 - src/trackers/__tests__/eventTracker.spec.ts | 93 +++++++++++-------- .../__tests__/impressionsTracker.spec.ts | 89 +++++++++++------- src/trackers/eventTracker.ts | 14 ++- src/trackers/impressionObserver/utils.ts | 9 +- src/trackers/impressionsTracker.ts | 15 ++- 13 files changed, 154 insertions(+), 146 deletions(-) delete mode 100644 src/sdkClient/__tests__/client.spec.ts diff --git a/package-lock.json b/package-lock.json index 36876610..0a014819 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.7", + "version": "1.2.1-rc.8", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 03f9ec7a..63b45dc7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.7", + "version": "1.2.1-rc.8", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/sdkClient/__tests__/client.spec.ts b/src/sdkClient/__tests__/client.spec.ts deleted file mode 100644 index 4112d7d5..00000000 --- a/src/sdkClient/__tests__/client.spec.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.mocks'; -import { clientFactory } from '../client'; - -test('client should track or not events and impressions depending on user consent status', () => { - const sdkReadinessManager = { readinessManager: { isReady: () => true } }; - const storage = { splits: { trafficTypeExists: () => true } }; - - const settings = { ...fullSettings }; - const eventTracker = { track: jest.fn() }; - const impressionsTracker = { track: jest.fn() }; - - // @ts-ignore - const client = clientFactory({ settings, eventTracker, impressionsTracker, sdkReadinessManager, storage }); - - client.getTreatment('some_key', 'some_split'); - expect(impressionsTracker.track).toBeCalledTimes(1); // impression should be tracked if userConsent is undefined - client.track('some_key', 'some_tt', 'some_event'); - expect(eventTracker.track).toBeCalledTimes(1); // event should be tracked if userConsent is undefined - - settings.userConsent = 'UNKNOWN'; - client.getTreatment('some_key', 'some_split'); - expect(impressionsTracker.track).toBeCalledTimes(2); // impression should be tracked if userConsent is unknown - client.track('some_key', 'some_tt', 'some_event'); - expect(eventTracker.track).toBeCalledTimes(2); // event should be tracked if userConsent is unknown - - settings.userConsent = 'GRANTED'; - client.getTreatment('some_key', 'some_split'); - expect(impressionsTracker.track).toBeCalledTimes(3); // impression should be tracked if userConsent is granted - client.track('some_key', 'some_tt', 'some_event'); - expect(eventTracker.track).toBeCalledTimes(3); // event should be tracked if userConsent is granted - - settings.userConsent = 'DECLINED'; - client.getTreatment('some_key', 'some_split'); - expect(impressionsTracker.track).toBeCalledTimes(3); // impression should not be tracked if userConsent is declined - client.track('some_key', 'some_tt', 'some_event'); - expect(eventTracker.track).toBeCalledTimes(3); // event should not be tracked if userConsent is declined - -}); diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index 7d5ab72c..3ef41ec6 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -4,7 +4,7 @@ import { getMatching, getBucketing } from '../utils/key'; import { validateSplitExistance } from '../utils/inputValidation/splitExistance'; import { validateTrafficTypeExistance } from '../utils/inputValidation/trafficTypeExistance'; import { SDK_NOT_READY } from '../utils/labels'; -import { CONSENT_DECLINED, CONTROL } from '../utils/constants'; +import { CONTROL } from '../utils/constants'; import { IClientFactoryParams } from './types'; import { IEvaluationResult } from '../evaluator/types'; import { SplitIO, ImpressionDTO } from '../types'; @@ -23,7 +23,7 @@ export function clientFactory(params: IClientFactoryParams): SplitIO.IClient | S const wrapUp = (evaluationResult: IEvaluationResult) => { const queue: ImpressionDTO[] = []; const treatment = processEvaluation(evaluationResult, splitName, key, attributes, withConfig, `getTreatment${withConfig ? 'withConfig' : ''}`, queue); - if (settings.userConsent !== CONSENT_DECLINED) impressionsTracker.track(queue, attributes); + impressionsTracker.track(queue, attributes); return treatment; }; @@ -43,7 +43,7 @@ export function clientFactory(params: IClientFactoryParams): SplitIO.IClient | S Object.keys(evaluationResults).forEach(splitName => { treatments[splitName] = processEvaluation(evaluationResults[splitName], splitName, key, attributes, withConfig, `getTreatments${withConfig ? 'withConfig' : ''}`, queue); }); - if (settings.userConsent !== CONSENT_DECLINED) impressionsTracker.track(queue, attributes); + impressionsTracker.track(queue, attributes); return treatments; }; @@ -116,8 +116,7 @@ export function clientFactory(params: IClientFactoryParams): SplitIO.IClient | S // This may be async but we only warn, we don't actually care if it is valid or not in terms of queueing the event. validateTrafficTypeExistance(log, readinessManager, storage.splits, mode, trafficTypeName, 'track'); - if (settings.userConsent !== CONSENT_DECLINED) return eventTracker.track(eventData, size); - else return false; + return eventTracker.track(eventData, size); } return { diff --git a/src/sdkClient/clientInputValidation.ts b/src/sdkClient/clientInputValidation.ts index df0eaebc..de9f3594 100644 --- a/src/sdkClient/clientInputValidation.ts +++ b/src/sdkClient/clientInputValidation.ts @@ -15,14 +15,17 @@ import { startsWith } from '../utils/lang'; import { CONTROL, CONTROL_WITH_CONFIG } from '../utils/constants'; import { IReadinessManager } from '../readiness/types'; import { MaybeThenable } from '../dtos/types'; -import { SplitIO } from '../types'; -import { ILogger } from '../logger/types'; +import { ISettings, SplitIO } from '../types'; +import { isStorageSync } from '../trackers/impressionObserver/utils'; /** * Decorator that validates the input before actually executing the client methods. * We should "guard" the client here, while not polluting the "real" implementation of those methods. */ -export function clientInputValidationDecorator(log: ILogger, client: TClient, readinessManager: IReadinessManager, isStorageSync = false): TClient { +export function clientInputValidationDecorator(settings: ISettings, client: TClient, readinessManager: IReadinessManager): TClient { + + const log = settings.log; + const isSync = isStorageSync(settings); /** * Avoid repeating this validations code @@ -47,8 +50,7 @@ export function clientInputValidationDecorator(value: T): MaybeThenable { - if (isStorageSync) return value; - return Promise.resolve(value); + return isSync ? value : Promise.resolve(value); } function getTreatment(maybeKey: SplitIO.SplitKey, maybeSplit: string, maybeAttributes?: SplitIO.Attributes) { @@ -108,8 +110,7 @@ export function clientInputValidationDecorator ISignalListener, // Used by BrowserSignalListener // @TODO review impressionListener and integrations interfaces. What about handling impressionListener as an integration ? - impressionListener?: SplitIO.IImpressionListener, integrationsManagerFactory?: (params: IIntegrationFactoryParams) => IIntegrationManager | undefined, // Impression observer factory. If provided, will be used for impressions dedupe diff --git a/src/trackers/__tests__/eventTracker.spec.ts b/src/trackers/__tests__/eventTracker.spec.ts index aa2cd234..d770d899 100644 --- a/src/trackers/__tests__/eventTracker.spec.ts +++ b/src/trackers/__tests__/eventTracker.spec.ts @@ -1,5 +1,5 @@ -import { loggerMock } from '../../logger/__tests__/sdkLogger.mock'; import { SplitIO } from '../../types'; +import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.mocks'; import { eventTrackerFactory } from '../eventTracker'; /* Mocks */ @@ -12,30 +12,30 @@ const fakeIntegrationsManager = { handleEvent: jest.fn() }; +const fakeEvent = { + eventTypeId: 'eventTypeId', + trafficTypeName: 'trafficTypeName', + value: 0, + timestamp: Date.now(), + key: 'matchingKey', + properties: { + prop1: 'prop1', + prop2: 0, + } +}; + /* Tests */ describe('Event Tracker', () => { test('Tracker API', () => { expect(typeof eventTrackerFactory).toBe('function'); // The module should return a function which acts as a factory. - const instance = eventTrackerFactory(loggerMock, fakeEventsCache, fakeIntegrationsManager); + const instance = eventTrackerFactory(fullSettings, fakeEventsCache, fakeIntegrationsManager); expect(typeof instance.track).toBe('function'); // The instance should implement the track method. }); - test('Propagate the event into the event cache and integrations manager, and return its result (a boolean or a promise that resolves to boolean)', (done) => { - const fakeEvent = { - eventTypeId: 'eventTypeId', - trafficTypeName: 'trafficTypeName', - value: 0, - timestamp: Date.now(), - key: 'matchingKey', - properties: { - prop1: 'prop1', - prop2: 0, - } - }; - + test('Propagate the event into the event cache and integrations manager, and return its result (a boolean or a promise that resolves to boolean)', async () => { fakeEventsCache.track.mockImplementation((eventData: SplitIO.EventData, size: number) => { if (eventData === fakeEvent) { switch (size) { @@ -46,46 +46,61 @@ describe('Event Tracker', () => { } }); - const tracker = eventTrackerFactory(loggerMock, fakeEventsCache, fakeIntegrationsManager); + const tracker = eventTrackerFactory(fullSettings, fakeEventsCache, fakeIntegrationsManager); const result1 = tracker.track(fakeEvent, 1); expect(fakeEventsCache.track.mock.calls[0]).toEqual([fakeEvent, 1]); // Should be present in the event cache. expect(fakeIntegrationsManager.handleEvent).not.toBeCalled(); // The integration manager handleEvent method should not be executed synchronously. expect(result1).toBe(true); // Should return the value of the event cache. - setTimeout(() => { - expect(fakeIntegrationsManager.handleEvent.mock.calls[0]).toEqual([fakeEvent]); // A copy of the tracked event should be sent to integration manager after the timeout wrapping make it to the queue stack. - expect(fakeIntegrationsManager.handleEvent.mock.calls[0][0]).not.toBe(fakeEvent); // Should not send the original event. + await new Promise(res => setTimeout(res)); + expect(fakeIntegrationsManager.handleEvent.mock.calls[0]).toEqual([fakeEvent]); // A copy of the tracked event should be sent to integration manager after the timeout wrapping make it to the queue stack. + expect(fakeIntegrationsManager.handleEvent.mock.calls[0][0]).not.toBe(fakeEvent); // Should not send the original event. + + const result2 = tracker.track(fakeEvent, 2) as Promise; - const result2 = tracker.track(fakeEvent, 2) as Promise; + expect(fakeEventsCache.track.mock.calls[1]).toEqual([fakeEvent, 2]); // Should be present in the event cache. - expect(fakeEventsCache.track.mock.calls[1]).toEqual([fakeEvent, 2]); // Should be present in the event cache. + let tracked = await result2; + expect(tracked).toBe(false); // Should return the value of the event cache resolved promise. - result2.then(tracked => { - expect(tracked).toBe(false); // Should return the value of the event cache resolved promise. + await new Promise(res => setTimeout(res)); + expect(fakeIntegrationsManager.handleEvent).toBeCalledTimes(1); // Untracked event should not be sent to integration manager. - setTimeout(() => { - expect(fakeIntegrationsManager.handleEvent).toBeCalledTimes(1); // Untracked event should not be sent to integration manager. + const result3 = tracker.track(fakeEvent, 3) as Promise; + + expect(fakeEventsCache.track.mock.calls[2]).toEqual([fakeEvent, 3]); // Should be present in the event cache. + + tracked = await result3; + expect(fakeIntegrationsManager.handleEvent).toBeCalledTimes(1); // Tracked event should not be sent to integration manager synchronously + expect(tracked).toBe(true); // Should return the value of the event cache resolved promise. + + await new Promise(res => setTimeout(res)); + expect(fakeIntegrationsManager.handleEvent.mock.calls[1]).toEqual([fakeEvent]); // A copy of tracked event should be sent to integration manager after the timeout wrapping make it to the queue stack. + expect(fakeIntegrationsManager.handleEvent.mock.calls[1][0]).not.toBe(fakeEvent); // Should not send the original event. + + }); - const result3 = tracker.track(fakeEvent, 3) as Promise; + test('Should track or not events depending on user consent status', () => { + const settings = { ...fullSettings }; + const fakeEventsCache = { track: jest.fn(() => true) }; - expect(fakeEventsCache.track.mock.calls[2]).toEqual([fakeEvent, 3]); // Should be present in the event cache. + const tracker = eventTrackerFactory(settings, fakeEventsCache); - result3.then(tracked => { - expect(fakeIntegrationsManager.handleEvent).toBeCalledTimes(1); // Tracked event should not be sent to integration manager synchronously - expect(tracked).toBe(true); // Should return the value of the event cache resolved promise. + expect(tracker.track(fakeEvent)).toBe(true); + expect(fakeEventsCache.track).toBeCalledTimes(1); // event should be tracked if userConsent is undefined - setTimeout(() => { - expect(fakeIntegrationsManager.handleEvent.mock.calls[1]).toEqual([fakeEvent]); // A copy of tracked event should be sent to integration manager after the timeout wrapping make it to the queue stack. - expect(fakeIntegrationsManager.handleEvent.mock.calls[1][0]).not.toBe(fakeEvent); // Should not send the original event. + settings.userConsent = 'UNKNOWN'; + expect(tracker.track(fakeEvent)).toBe(true); + expect(fakeEventsCache.track).toBeCalledTimes(2); // event should be tracked if userConsent is unknown - done(); - }, 0); - }); + settings.userConsent = 'GRANTED'; + expect(tracker.track(fakeEvent)).toBe(true); + expect(fakeEventsCache.track).toBeCalledTimes(3); // event should be tracked if userConsent is granted - }, 0); - }); - }, 0); + settings.userConsent = 'DECLINED'; + expect(tracker.track(fakeEvent)).toBe(false); + expect(fakeEventsCache.track).toBeCalledTimes(3); // event should not be tracked if userConsent is declined }); }); diff --git a/src/trackers/__tests__/impressionsTracker.spec.ts b/src/trackers/__tests__/impressionsTracker.spec.ts index fe763d67..0a35db12 100644 --- a/src/trackers/__tests__/impressionsTracker.spec.ts +++ b/src/trackers/__tests__/impressionsTracker.spec.ts @@ -3,48 +3,51 @@ import { ImpressionCountsCacheInMemory } from '../../storages/inMemory/Impressio import { impressionObserverSSFactory } from '../impressionObserver/impressionObserverSS'; import { impressionObserverCSFactory } from '../impressionObserver/impressionObserverCS'; import { ImpressionDTO } from '../../types'; -import { loggerMock } from '../../logger/__tests__/sdkLogger.mock'; +import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.mocks'; /* Mocks */ -function generateMocks() { - const fakeImpressionsCache = { - track: jest.fn() - }; - const fakeSettings = { - runtime: { - hostname: 'fake-hostname', - ip: 'fake-ip' - }, - version: 'jest-test', - }; - const fakeListener = { - logImpression: jest.fn() - }; - const fakeIntegrationsManager = { - handleImpression: jest.fn() - }; - - return { - fakeImpressionsCache, fakeSettings, fakeListener, fakeIntegrationsManager - }; -} +const fakeImpressionsCache = { + track: jest.fn() +}; +const fakeListener = { + logImpression: jest.fn() +}; +const fakeIntegrationsManager = { + handleImpression: jest.fn() +}; +const fakeSettings = { + ...fullSettings, + runtime: { + hostname: 'fake-hostname', + ip: 'fake-ip' + }, + version: 'jest-test' +}; +const fakeSettingsWithListener = { + ...fakeSettings, + impressionListener: fakeListener +}; /* Tests */ describe('Impressions Tracker', () => { + beforeEach(() => { + fakeImpressionsCache.track.mockClear(); + fakeListener.logImpression.mockClear(); + fakeIntegrationsManager.handleImpression.mockClear(); + }); + test('Tracker API', () => { expect(typeof impressionsTrackerFactory).toBe('function'); // The module should return a function which acts as a factory. - const { fakeImpressionsCache, fakeSettings } = generateMocks(); - const instance = impressionsTrackerFactory(loggerMock, fakeImpressionsCache, fakeSettings); + const instance = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache); expect(typeof instance.track).toBe('function'); // The instance should implement the track method which will actually track queued impressions. }); test('Should be able to track impressions (in DEBUG mode without Previous Time).', () => { - const { fakeImpressionsCache, fakeSettings } = generateMocks(); - const tracker = impressionsTrackerFactory(loggerMock, fakeImpressionsCache, fakeSettings); + const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache); const imp1 = { feature: '10', @@ -64,8 +67,7 @@ describe('Impressions Tracker', () => { }); test('Tracked impressions should be sent to impression listener and integration manager when we invoke .track()', (done) => { - const { fakeImpressionsCache, fakeSettings, fakeListener, fakeIntegrationsManager } = generateMocks(); - const tracker = impressionsTrackerFactory(loggerMock, fakeImpressionsCache, fakeSettings, fakeListener, fakeIntegrationsManager); + const tracker = impressionsTrackerFactory(fakeSettingsWithListener, fakeImpressionsCache, fakeIntegrationsManager); const fakeImpression = { feature: 'impression' @@ -135,13 +137,12 @@ describe('Impressions Tracker', () => { } as ImpressionDTO; test('Should track 3 impressions with Previous Time.', () => { - const { fakeImpressionsCache, fakeSettings } = generateMocks(); impression.time = impression2.time = 123456789; impression3.time = 1234567891; const trackers = [ - impressionsTrackerFactory(loggerMock, fakeImpressionsCache, fakeSettings, undefined, undefined, impressionObserverSSFactory()), - impressionsTrackerFactory(loggerMock, fakeImpressionsCache, fakeSettings, undefined, undefined, impressionObserverCSFactory()) + impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, undefined, impressionObserverSSFactory()), + impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, undefined, impressionObserverCSFactory()) ]; expect(fakeImpressionsCache.track).not.toBeCalled(); // storage method should not be called until impressions are tracked. @@ -163,13 +164,12 @@ describe('Impressions Tracker', () => { }); test('Should track 2 impressions in OPTIMIZED mode (providing ImpressionCountsCache).', () => { - const { fakeImpressionsCache, fakeSettings } = generateMocks(); impression.time = Date.now(); impression2.time = Date.now(); impression3.time = Date.now(); const impressionCountsCache = new ImpressionCountsCacheInMemory(); - const tracker = impressionsTrackerFactory(loggerMock, fakeImpressionsCache, fakeSettings, undefined, undefined, impressionObserverCSFactory(), impressionCountsCache); + const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, undefined, impressionObserverCSFactory(), impressionCountsCache); expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker @@ -187,4 +187,25 @@ describe('Impressions Tracker', () => { expect(Object.keys(impressionCountsCache.state()).length).toBe(2); }); + test('Should track or not impressions depending on user consent status', () => { + const settings = { ...fullSettings }; + + const tracker = impressionsTrackerFactory(settings, fakeImpressionsCache); + + tracker.track([impression]); + expect(fakeImpressionsCache.track).toBeCalledTimes(1); // impression should be tracked if userConsent is undefined + + settings.userConsent = 'UNKNOWN'; + tracker.track([impression]); + expect(fakeImpressionsCache.track).toBeCalledTimes(2); // impression should be tracked if userConsent is unknown + + settings.userConsent = 'GRANTED'; + tracker.track([impression]); + expect(fakeImpressionsCache.track).toBeCalledTimes(3); // impression should be tracked if userConsent is granted + + settings.userConsent = 'DECLINED'; + tracker.track([impression]); + expect(fakeImpressionsCache.track).toBeCalledTimes(3); // impression should not be tracked if userConsent is declined + }); + }); diff --git a/src/trackers/eventTracker.ts b/src/trackers/eventTracker.ts index b3d63f6e..7ac851a9 100644 --- a/src/trackers/eventTracker.ts +++ b/src/trackers/eventTracker.ts @@ -2,9 +2,10 @@ import { objectAssign } from '../utils/lang/objectAssign'; import { thenable } from '../utils/promise/thenable'; import { IEventsCacheBase } from '../storages/types'; import { IEventsHandler, IEventTracker } from './types'; -import { SplitIO } from '../types'; -import { ILogger } from '../logger/types'; +import { ISettings, SplitIO } from '../types'; import { EVENTS_TRACKER_SUCCESS, ERROR_EVENTS_TRACKER } from '../logger/constants'; +import { CONSENT_DECLINED } from '../utils/constants'; +import { isStorageSync } from './impressionObserver/utils'; /** * Event tracker stores events in cache and pass them to the integrations manager if provided. @@ -13,11 +14,14 @@ import { EVENTS_TRACKER_SUCCESS, ERROR_EVENTS_TRACKER } from '../logger/constant * @param integrationsManager optional event handler used for integrations */ export function eventTrackerFactory( - log: ILogger, + settings: ISettings, eventsCache: IEventsCacheBase, integrationsManager?: IEventsHandler ): IEventTracker { + const log = settings.log; + const isSync = isStorageSync(settings); + function queueEventsCallback(eventData: SplitIO.EventData, tracked: boolean) { const { eventTypeId, trafficTypeName, key, value, timestamp, properties } = eventData; // Logging every prop would be too much. @@ -44,6 +48,10 @@ export function eventTrackerFactory( return { track(eventData: SplitIO.EventData, size?: number) { + if (settings.userConsent === CONSENT_DECLINED) { + return isSync ? false : Promise.resolve(false); + } + const tracked = eventsCache.track(eventData, size); if (thenable(tracked)) { diff --git a/src/trackers/impressionObserver/utils.ts b/src/trackers/impressionObserver/utils.ts index 7fb73b85..4ecccbe3 100644 --- a/src/trackers/impressionObserver/utils.ts +++ b/src/trackers/impressionObserver/utils.ts @@ -1,4 +1,4 @@ -import { CONSUMER_PARTIAL_MODE, OPTIMIZED, PRODUCER_MODE, STANDALONE_MODE } from '../../utils/constants'; +import { CONSUMER_MODE, CONSUMER_PARTIAL_MODE, OPTIMIZED, PRODUCER_MODE, STANDALONE_MODE } from '../../utils/constants'; import { ISettings } from '../../types'; /** @@ -15,3 +15,10 @@ export function shouldBeOptimized(settings: ISettings) { if (!shouldAddPt(settings)) return false; return settings.sync.impressionsMode === OPTIMIZED ? true : false; } + +/** + * Storage is async if mode is consumer or partial consumer + */ +export function isStorageSync(settings: ISettings) { + return [CONSUMER_MODE, CONSUMER_PARTIAL_MODE].indexOf(settings.mode) === -1 ? true : false; +} diff --git a/src/trackers/impressionsTracker.ts b/src/trackers/impressionsTracker.ts index c7c18e5b..ecd40a5d 100644 --- a/src/trackers/impressionsTracker.ts +++ b/src/trackers/impressionsTracker.ts @@ -5,8 +5,8 @@ import { IImpressionCountsCacheSync, IImpressionsCacheBase } from '../storages/t import { IImpressionsHandler, IImpressionsTracker } from './types'; import { SplitIO, ImpressionDTO, ISettings } from '../types'; import { IImpressionObserver } from './impressionObserver/types'; -import { ILogger } from '../logger/types'; import { IMPRESSIONS_TRACKER_SUCCESS, ERROR_IMPRESSIONS_TRACKER, ERROR_IMPRESSIONS_LISTENER } from '../logger/constants'; +import { CONSENT_DECLINED } from '../utils/constants'; /** * Impressions tracker stores impressions in cache and pass them to the listener and integrations manager if provided. @@ -19,22 +19,21 @@ import { IMPRESSIONS_TRACKER_SUCCESS, ERROR_IMPRESSIONS_TRACKER, ERROR_IMPRESSIO * @param countsCache optional cache to save impressions count. If provided, impressions will be deduped (OPTIMIZED mode) */ export function impressionsTrackerFactory( - log: ILogger, + settings: ISettings, impressionsCache: IImpressionsCacheBase, - - // @TODO consider passing only an optional integrationsManager to handle impressions - { runtime: { ip, hostname }, version }: Pick, - impressionListener?: SplitIO.IImpressionListener, integrationsManager?: IImpressionsHandler, - // if observer is provided, it implies `shouldAddPreviousTime` flag (i.e., if impressions previous time should be added or not) observer?: IImpressionObserver, // if countsCache is provided, it implies `isOptimized` flag (i.e., if impressions should be deduped or not) countsCache?: IImpressionCountsCacheSync ): IImpressionsTracker { + const { log, impressionListener, runtime: { ip, hostname }, version } = settings; + return { track(impressions: ImpressionDTO[], attributes?: SplitIO.Attributes) { + if (settings.userConsent === CONSENT_DECLINED) return; + const impressionsCount = impressions.length; const impressionsToStore: ImpressionDTO[] = []; // Track only the impressions that are going to be stored @@ -85,7 +84,7 @@ export function impressionsTrackerFactory( // integrationsManager.handleImpression does not throw errors if (integrationsManager) integrationsManager.handleImpression(impressionData); - try { // An exception on the listeners should not break the SDK. + try { // @ts-ignore. An exception on the listeners should not break the SDK. if (impressionListener) impressionListener.logImpression(impressionData); } catch (err) { log.error(ERROR_IMPRESSIONS_LISTENER, [err]); From 6bff1209cf177a8e08daf624bc63ba9732402f6f Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 9 Mar 2022 11:17:59 -0300 Subject: [PATCH 2/4] fixed map, to use ponyfill on browsers with partial support of Map constructor, like ie11 --- src/utils/lang/__tests__/maps.spec.ts | 16 ++++++++++++++++ src/utils/lang/maps.ts | 16 +++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 src/utils/lang/__tests__/maps.spec.ts diff --git a/src/utils/lang/__tests__/maps.spec.ts b/src/utils/lang/__tests__/maps.spec.ts new file mode 100644 index 00000000..02312d50 --- /dev/null +++ b/src/utils/lang/__tests__/maps.spec.ts @@ -0,0 +1,16 @@ +import { __getMapConstructor, MapPoly } from '../maps'; + +test('__getMapConstructor', () => { + + // should return global Map constructor if available + expect(__getMapConstructor()).toBe(global.Map); + + const originalMap = global.Map; // @ts-ignore + global.Map = undefined; // overwrite global Map + + // should return Map polyfill if global Map constructor is not available + expect(__getMapConstructor()).toBe(MapPoly); + + global.Map = originalMap; // restore original global Map + +}); diff --git a/src/utils/lang/maps.ts b/src/utils/lang/maps.ts index 25461615..a0f09cff 100644 --- a/src/utils/lang/maps.ts +++ b/src/utils/lang/maps.ts @@ -79,4 +79,18 @@ interface IMapConstructor { readonly prototype: IMap; } -export const _Map: IMapConstructor = typeof Map !== 'undefined' ? Map : MapPoly; +/** + * return the Map constructor to use. If native Map is not available or it doesn't support the required features (e.g., IE11), + * a ponyfill with minimal features is returned instead. + * + * Exported for testing purposes only. + */ +export function __getMapConstructor(): IMapConstructor { + // eslint-disable-next-line compat/compat + if (typeof Array.from === 'function' && typeof Map === 'function' && Map.prototype && Map.prototype.values) { + return Map; + } + return MapPoly; +} + +export const _Map = __getMapConstructor(); From bbaed1f4c60b5b719493e69261249d0ce9698ecc Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 14 Mar 2022 10:53:51 -0300 Subject: [PATCH 3/4] updated isClientSide flag --- src/sdkClient/client.ts | 2 +- src/sdkClient/clientCS.ts | 2 +- src/types.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index 3ef41ec6..15ea6193 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -125,6 +125,6 @@ export function clientFactory(params: IClientFactoryParams): SplitIO.IClient | S getTreatments, getTreatmentsWithConfig, track, - isBrowserClient: false + isClientSide: false } as SplitIO.IClient | SplitIO.IAsyncClient; } diff --git a/src/sdkClient/clientCS.ts b/src/sdkClient/clientCS.ts index 16648bc6..c66d1e04 100644 --- a/src/sdkClient/clientCS.ts +++ b/src/sdkClient/clientCS.ts @@ -25,6 +25,6 @@ export function clientCSDecorator(log: ILogger, client: SplitIO.IClient, key: Sp // Key is bound to the `track` method. Same thing happens with trafficType but only if provided track: trafficType ? clientCS.track.bind(clientCS, key, trafficType) : clientCS.track.bind(clientCS, key), - isBrowserClient: true + isClientSide: true }) as SplitIO.ICsClient; } diff --git a/src/types.ts b/src/types.ts index c449ff37..7d523223 100644 --- a/src/types.ts +++ b/src/types.ts @@ -398,7 +398,7 @@ interface IBasicClient extends IStatusInterface { // Whether the client implements the client-side API, i.e, with bound key, (true), or the server-side API (false). // Exposed for internal purposes only. Not considered part of the public API, and might be renamed eventually. - isBrowserClient: boolean + isClientSide: boolean } /** * Common definitions between SDK instances for different environments interface. From 6eb52fc0d381015709a6b4db6eb0d2a930037cd7 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 15 Mar 2022 11:56:15 -0300 Subject: [PATCH 4/4] code syntax refactor --- src/integrations/pluggable.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/integrations/pluggable.ts b/src/integrations/pluggable.ts index aed6806c..df4ccd21 100644 --- a/src/integrations/pluggable.ts +++ b/src/integrations/pluggable.ts @@ -29,10 +29,10 @@ export function pluggableIntegrationsManagerFactory( // Exception safe methods: each integration module is responsable for handling errors return { - handleImpression: function (impressionData: SplitIO.ImpressionData) { + handleImpression(impressionData: SplitIO.ImpressionData) { listeners.forEach(listener => listener.queue({ type: SPLIT_IMPRESSION, payload: impressionData })); }, - handleEvent: function (eventData: SplitIO.EventData) { + handleEvent(eventData: SplitIO.EventData) { listeners.forEach(listener => listener.queue({ type: SPLIT_EVENT, payload: eventData })); } };