Refactor impressions tracker to simplify reusability

src/sdkClient/__tests__/sdkClientMethod.spec.ts

@@ -18,7 +18,7 @@ const paramMocks = [
     settings: { mode: CONSUMER_MODE, log: loggerMock, core: { authorizationKey: 'sdk key '} },
     telemetryTracker: telemetryTrackerFactory(),
     clients: {},
-    uniqueKeysTracker: { start: jest.fn(), stop: jest.fn() },
+    impressionsTracker: { start: jest.fn(), stop: jest.fn(), track: jest.fn() },
     fallbackTreatmentsCalculator: new FallbackTreatmentsCalculator({})
   },
   // SyncManager (i.e., Sync SDK) and Signal listener
@@ -30,7 +30,7 @@ const paramMocks = [
     settings: { mode: STANDALONE_MODE, log: loggerMock, core: { authorizationKey: 'sdk key '} },
     telemetryTracker: telemetryTrackerFactory(),
     clients: {},
-    uniqueKeysTracker: { start: jest.fn(), stop: jest.fn() },
+    impressionsTracker: { start: jest.fn(), stop: jest.fn(), track: jest.fn() },
     fallbackTreatmentsCalculator: new FallbackTreatmentsCalculator({})
   }
 ];
@@ -75,7 +75,7 @@ test.each(paramMocks)('sdkClientMethodFactory', (params, done: any) => {
           client.destroy().then(() => {
             expect(params.sdkReadinessManager.readinessManager.destroy).toBeCalledTimes(1);
             expect(params.storage.destroy).toBeCalledTimes(1);
-            expect(params.uniqueKeysTracker.stop).toBeCalledTimes(1);
+            expect(params.impressionsTracker.stop).toBeCalledTimes(1);
 
             if (params.syncManager) {
               expect(params.syncManager.stop).toBeCalledTimes(1);

src/sdkClient/__tests__/sdkClientMethodCS.spec.ts

@@ -46,7 +46,7 @@ const params = {
   settings: settingsWithKey,
   telemetryTracker: telemetryTrackerFactory(),
   clients: {},
-  uniqueKeysTracker: { start: jest.fn(), stop: jest.fn() }
+  impressionsTracker: { start: jest.fn(), stop: jest.fn(), track: jest.fn() }
 };
 
 const invalidAttributes = [
@@ -96,7 +96,7 @@ describe('sdkClientMethodCSFactory', () => {
       expect(params.syncManager.stop).toBeCalledTimes(1);
       expect(params.syncManager.flush).toBeCalledTimes(1);
       expect(params.signalListener.stop).toBeCalledTimes(1);
-      expect(params.uniqueKeysTracker.stop).toBeCalledTimes(1);
+      expect(params.impressionsTracker.stop).toBeCalledTimes(1);
     });
 
   });

src/sdkClient/sdkClient.ts

@@ -11,7 +11,7 @@ const COOLDOWN_TIME_IN_MILLIS = 1000;
  * Creates an Sdk client, i.e., a base client with status, init, flush and destroy interface
  */
 export function sdkClientFactory(params: ISdkFactoryContext, isSharedClient?: boolean): SplitIO.IClient | SplitIO.IAsyncClient {
-  const { sdkReadinessManager, syncManager, storage, signalListener, settings, telemetryTracker, uniqueKeysTracker } = params;
+  const { sdkReadinessManager, syncManager, storage, signalListener, settings, telemetryTracker, impressionsTracker } = params;
 
   let hasInit = false;
   let lastActionTime = 0;
@@ -56,7 +56,7 @@ export function sdkClientFactory(params: ISdkFactoryContext, isSharedClient?: bo
         if (!isSharedClient) {
           validateAndTrackApiKey(settings.log, settings.core.authorizationKey);
           sdkReadinessManager.readinessManager.init();
-          uniqueKeysTracker.start();
+          impressionsTracker.start();
           syncManager && syncManager.start();
           signalListener && signalListener.start();
         }
@@ -77,7 +77,7 @@ export function sdkClientFactory(params: ISdkFactoryContext, isSharedClient?: bo
           releaseApiKey(settings.core.authorizationKey);
           telemetryTracker.sessionLength();
           signalListener && signalListener.stop();
-          uniqueKeysTracker.stop();
+          impressionsTracker.stop();
         }
 
         // Stop background jobs

src/sdkFactory/index.ts

@@ -8,11 +8,6 @@ import { createLoggerAPI } from '../logger/sdkLogger';
 import { NEW_FACTORY, RETRIEVE_MANAGER } from '../logger/constants';
 import { SDK_SPLITS_ARRIVED, SDK_SEGMENTS_ARRIVED, SDK_SPLITS_CACHE_LOADED } from '../readiness/constants';
 import { objectAssign } from '../utils/lang/objectAssign';
-import { strategyDebugFactory } from '../trackers/strategy/strategyDebug';
-import { strategyOptimizedFactory } from '../trackers/strategy/strategyOptimized';
-import { strategyNoneFactory } from '../trackers/strategy/strategyNone';
-import { uniqueKeysTrackerFactory } from '../trackers/uniqueKeysTracker';
-import { DEBUG, OPTIMIZED } from '../utils/constants';
 import { setRolloutPlan } from '../storages/setRolloutPlan';
 import { IStorageSync } from '../storages/types';
 import { getMatching } from '../utils/key';
@@ -24,10 +19,9 @@ import { FallbackTreatmentsCalculator } from '../evaluator/fallbackTreatmentsCal
 export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IAsyncSDK | SplitIO.IBrowserSDK | SplitIO.IBrowserAsyncSDK {
 
   const { settings, platform, storageFactory, splitApiFactory, extraProps,
-    syncManagerFactory, SignalListener, impressionsObserverFactory,
-    integrationsManagerFactory, sdkManagerFactory, sdkClientMethodFactory,
-    filterAdapterFactory, lazyInit } = params;
-  const { log, sync: { impressionsMode }, initialRolloutPlan, core: { key } } = settings;
+    syncManagerFactory, SignalListener,
+    integrationsManagerFactory, sdkManagerFactory, sdkClientMethodFactory, lazyInit } = params;
+  const { log, initialRolloutPlan, core: { key } } = settings;
 
   // @TODO handle non-recoverable errors, such as, global `fetch` not available, invalid SDK Key, etc.
   // On non-recoverable errors, we should mark the SDK as destroyed and not start synchronization.
@@ -62,23 +56,13 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
   const telemetryTracker = telemetryTrackerFactory(storage.telemetry, platform.now);
   const integrationsManager = integrationsManagerFactory && integrationsManagerFactory({ settings, storage, telemetryTracker });
 
-  const observer = impressionsObserverFactory();
-  const uniqueKeysTracker = uniqueKeysTrackerFactory(log, storage.uniqueKeys, filterAdapterFactory && filterAdapterFactory());
-
-  const noneStrategy = strategyNoneFactory(storage.impressionCounts, uniqueKeysTracker);
-  const strategy = impressionsMode === OPTIMIZED ?
-    strategyOptimizedFactory(observer, storage.impressionCounts) :
-    impressionsMode === DEBUG ?
-      strategyDebugFactory(observer) :
-      noneStrategy;
-
-  const impressionsTracker = impressionsTrackerFactory(settings, storage.impressions, noneStrategy, strategy, integrationsManager, storage.telemetry);
+  const impressionsTracker = impressionsTrackerFactory(params, storage, integrationsManager);
   const eventTracker = eventTrackerFactory(settings, storage.events, integrationsManager, storage.telemetry);
 
   // splitApi is used by SyncManager and Browser signal listener
   const splitApi = splitApiFactory && splitApiFactory(settings, platform, telemetryTracker);
 
-  const ctx: ISdkFactoryContext = { clients, splitApi, eventTracker, impressionsTracker, telemetryTracker, uniqueKeysTracker, sdkReadinessManager, readiness, settings, storage, platform, fallbackTreatmentsCalculator };
+  const ctx: ISdkFactoryContext = { clients, splitApi, eventTracker, impressionsTracker, telemetryTracker, sdkReadinessManager, readiness, settings, storage, platform, fallbackTreatmentsCalculator };
 
   const syncManager = syncManagerFactory && syncManagerFactory(ctx as ISdkFactoryContextSync);
   ctx.syncManager = syncManager;

src/sdkFactory/types.ts

@@ -8,7 +8,7 @@ import { IFetch, ISplitApi, IEventSourceConstructor } from '../services/types';
 import { IStorageAsync, IStorageSync, IStorageFactoryParams } from '../storages/types';
 import { ISyncManager } from '../sync/types';
 import { IImpressionObserver } from '../trackers/impressionObserver/types';
-import { IImpressionsTracker, IEventTracker, ITelemetryTracker, IFilterAdapter, IUniqueKeysTracker } from '../trackers/types';
+import { IImpressionsTracker, IEventTracker, ITelemetryTracker, IFilterAdapter } from '../trackers/types';
 import { ISettings } from '../types';
 import SplitIO from '../../types/splitio';
 
@@ -47,7 +47,6 @@ export interface ISdkFactoryContext {
   eventTracker: IEventTracker,
   telemetryTracker: ITelemetryTracker,
   storage: IStorageSync | IStorageAsync,
-  uniqueKeysTracker: IUniqueKeysTracker,
   signalListener?: ISignalListener
   splitApi?: ISplitApi
   syncManager?: ISyncManager,

src/trackers/__tests__/impressionsTracker.spec.ts

@@ -2,10 +2,8 @@ import { impressionsTrackerFactory } from '../impressionsTracker';
 import { ImpressionCountsCacheInMemory } from '../../storages/inMemory/ImpressionCountsCacheInMemory';
 import { impressionObserverSSFactory } from '../impressionObserver/impressionObserverSS';
 import { impressionObserverCSFactory } from '../impressionObserver/impressionObserverCS';
-import SplitIO from '../../../types/splitio';
+import SplitIO, { ImpressionsMode } from '../../../types/splitio';
 import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.mocks';
-import { strategyDebugFactory } from '../strategy/strategyDebug';
-import { strategyOptimizedFactory } from '../strategy/strategyOptimized';
 import { DEDUPED, QUEUED } from '../../utils/constants';
 
 /* Mocks */
@@ -22,20 +20,22 @@ const fakeListener = {
 const fakeIntegrationsManager = {
   handleImpression: jest.fn()
 };
-const fakeSettings = {
-  ...fullSettings,
-  runtime: {
-    hostname: 'fake-hostname',
-    ip: 'fake-ip'
-  },
-  version: 'jest-test'
+const fakeStorage = {
+  impressions: fakeImpressionsCache,
+  impressionCounts: new ImpressionCountsCacheInMemory(),
+  uniqueKeys: { track: jest.fn() },
+  telemetry: undefined
 };
-const fakeSettingsWithListener = {
-  ...fakeSettings,
-  impressionListener: fakeListener
+const fakeParams = {
+  settings: fullSettings,
+  impressionsObserverFactory: impressionObserverCSFactory,
 };
-const fakeNoneStrategy = {
-  process: jest.fn(() => false)
+const fakeParamsWithListener = {
+  settings: {
+    ...fullSettings,
+    impressionListener: fakeListener
+  },
+  impressionsObserverFactory: impressionObserverCSFactory,
 };
 
 /* Tests */
@@ -48,10 +48,8 @@ describe('Impressions Tracker', () => {
     fakeIntegrationsManager.handleImpression.mockClear();
   });
 
-  const strategy = strategyDebugFactory(impressionObserverCSFactory());
-
   test('Should be able to track impressions (in DEBUG mode without Previous Time).', () => {
-    const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategy);
+    const tracker = impressionsTrackerFactory(fakeParams, fakeStorage);
 
     const imp1 = {
       feature: '10',
@@ -70,8 +68,8 @@ describe('Impressions Tracker', () => {
     expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([imp1, imp2]); // Should call the storage track method once we invoke .track() method, passing impressions with `track` enabled
   });
 
-  test('Tracked impressions should be sent to impression listener and integration manager when we invoke .track()', (done) => {
-    const tracker = impressionsTrackerFactory(fakeSettingsWithListener, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeIntegrationsManager);
+  test('Tracked impressions should be sent to impression listener and integration manager when we invoke .track()', async () => {
+    const tracker = impressionsTrackerFactory(fakeParamsWithListener, fakeStorage, fakeIntegrationsManager);
 
     const fakeImpression = {
       feature: 'impression'
@@ -94,25 +92,23 @@ describe('Impressions Tracker', () => {
     expect(fakeListener.logImpression).not.toBeCalled(); // The listener should not be executed synchronously.
     expect(fakeIntegrationsManager.handleImpression).not.toBeCalled(); // The integrations manager handleImpression method should not be executed synchronously.
 
-    setTimeout(() => {
-      expect(fakeListener.logImpression).toBeCalledTimes(2); // The listener should be executed after the timeout wrapping make it to the queue stack, once per each tracked impression.
-      expect(fakeIntegrationsManager.handleImpression).toBeCalledTimes(2); // The integrations manager handleImpression method should be executed after the timeout wrapping make it to the queue stack, once per each tracked impression.
+    await new Promise(resolve => setTimeout(resolve, 0));
 
-      const impressionData1 = { impression: fakeImpression, attributes: fakeAttributes, sdkLanguageVersion: fakeSettings.version, ip: fakeSettings.runtime.ip, hostname: fakeSettings.runtime.hostname };
-      const impressionData2 = { impression: fakeImpression2, attributes: fakeAttributes, sdkLanguageVersion: fakeSettings.version, ip: fakeSettings.runtime.ip, hostname: fakeSettings.runtime.hostname };
+    expect(fakeListener.logImpression).toBeCalledTimes(2); // The listener should be executed after the timeout wrapping make it to the queue stack, once per each tracked impression.
+    expect(fakeIntegrationsManager.handleImpression).toBeCalledTimes(2); // The integrations manager handleImpression method should be executed after the timeout wrapping make it to the queue stack, once per each tracked impression.
 
-      expect(fakeListener.logImpression.mock.calls[0][0]).toEqual(impressionData1); // The listener should be executed with the corresponding map for each of the impressions.
-      expect(fakeListener.logImpression.mock.calls[1][0]).toEqual(impressionData2); // The listener should be executed with the corresponding map for each of the impressions.
-      expect(fakeListener.logImpression.mock.calls[0][0].impression).not.toBe(fakeImpression); // but impression should be a copy
-      expect(fakeListener.logImpression.mock.calls[1][0].impression).not.toBe(fakeImpression2); // but impression should be a copy
+    const impressionData1 = { impression: fakeImpression, attributes: fakeAttributes, sdkLanguageVersion: fullSettings.version, ip: fullSettings.runtime.ip, hostname: fullSettings.runtime.hostname };
+    const impressionData2 = { impression: fakeImpression2, attributes: fakeAttributes, sdkLanguageVersion: fullSettings.version, ip: fullSettings.runtime.ip, hostname: fullSettings.runtime.hostname };
 
-      expect(fakeIntegrationsManager.handleImpression.mock.calls[0][0]).toEqual(impressionData1); // The integration manager handleImpression method should be executed with the corresponding map for each of the impressions.
-      expect(fakeIntegrationsManager.handleImpression.mock.calls[1][0]).toEqual(impressionData2); // The integration manager handleImpression method should be executed with the corresponding map for each of the impressions.
-      expect(fakeIntegrationsManager.handleImpression.mock.calls[0][0].impression).not.toBe(fakeImpression); // but impression should be a copy
-      expect(fakeIntegrationsManager.handleImpression.mock.calls[1][0].impression).not.toBe(fakeImpression2); // but impression should be a copy
+    expect(fakeListener.logImpression.mock.calls[0][0]).toEqual(impressionData1); // The listener should be executed with the corresponding map for each of the impressions.
+    expect(fakeListener.logImpression.mock.calls[1][0]).toEqual(impressionData2); // The listener should be executed with the corresponding map for each of the impressions.
+    expect(fakeListener.logImpression.mock.calls[0][0].impression).not.toBe(fakeImpression); // but impression should be a copy
+    expect(fakeListener.logImpression.mock.calls[1][0].impression).not.toBe(fakeImpression2); // but impression should be a copy
 
-      done();
-    }, 0);
+    expect(fakeIntegrationsManager.handleImpression.mock.calls[0][0]).toEqual(impressionData1); // The integration manager handleImpression method should be executed with the corresponding map for each of the impressions.
+    expect(fakeIntegrationsManager.handleImpression.mock.calls[1][0]).toEqual(impressionData2); // The integration manager handleImpression method should be executed with the corresponding map for each of the impressions.
+    expect(fakeIntegrationsManager.handleImpression.mock.calls[0][0].impression).not.toBe(fakeImpression); // but impression should be a copy
+    expect(fakeIntegrationsManager.handleImpression.mock.calls[1][0].impression).not.toBe(fakeImpression2); // but impression should be a copy
   });
 
   const impression = {
@@ -145,8 +141,8 @@ describe('Impressions Tracker', () => {
     impression3.time = 1234567891;
 
     const trackers = [
-      impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyDebugFactory(impressionObserverSSFactory()), undefined),
-      impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyDebugFactory(impressionObserverCSFactory()), undefined)
+      impressionsTrackerFactory({ ...fakeParams, impressionsObserverFactory: impressionObserverSSFactory }, fakeStorage),
+      impressionsTrackerFactory(fakeParams, fakeStorage)
     ];
 
     expect(fakeImpressionsCache.track).not.toBeCalled(); // storage method should not be called until impressions are tracked.
@@ -173,7 +169,7 @@ describe('Impressions Tracker', () => {
     impression3.time = Date.now();
 
     const impressionCountsCache = new ImpressionCountsCacheInMemory();
-    const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyOptimizedFactory(impressionObserverCSFactory(), impressionCountsCache), undefined, fakeTelemetryCache as any);
+    const tracker = impressionsTrackerFactory(fakeParams, { ...fakeStorage, impressionCounts: impressionCountsCache, telemetry: fakeTelemetryCache } as any);
 
     expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker
 
@@ -194,9 +190,10 @@ describe('Impressions Tracker', () => {
   });
 
   test('Should track or not impressions depending on user consent status', () => {
-    const settings = { ...fullSettings };
+    const settings = { ...fullSettings, sync: { ...fullSettings.sync, impressionsMode: 'DEBUG' as ImpressionsMode } };
+    const params = { settings, impressionsObserverFactory: impressionObserverCSFactory };
 
-    const tracker = impressionsTrackerFactory(settings, fakeImpressionsCache, fakeNoneStrategy, strategy);
+    const tracker = impressionsTrackerFactory(params, fakeStorage);
 
     tracker.track([{ imp: impression }]);
     expect(fakeImpressionsCache.track).toBeCalledTimes(1); // impression should be tracked if userConsent is undefined