User consent refactors and fixes

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",

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 }));
     }
   };

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 {
@@ -126,6 +125,6 @@ export function clientFactory(params: IClientFactoryParams): SplitIO.IClient | S
     getTreatments,
     getTreatmentsWithConfig,
     track,
-    isBrowserClient: false
+    isClientSide: false
   } as SplitIO.IClient | SplitIO.IAsyncClient;
 }

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;
 }

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<TClient extends SplitIO.IClient | SplitIO.IAsyncClient>(log: ILogger, client: TClient, readinessManager: IReadinessManager, isStorageSync = false): TClient {
+export function clientInputValidationDecorator<TClient extends SplitIO.IClient | SplitIO.IAsyncClient>(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<TClient extends SplitIO.IClient |
   }
 
   function wrapResult<T>(value: T): MaybeThenable<T> {
-    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<TClient extends SplitIO.IClient |
     if (isOperational && key && tt && event && eventValue !== false && properties !== false) { // @ts-expect-error
       return client.track(key, tt, event, eventValue, properties, size);
     } else {
-      if (isStorageSync) return false;
-      return Promise.resolve(false);
+      return isSync ? false : Promise.resolve(false);
     }
   }
 

src/sdkClient/sdkClient.ts

@@ -1,6 +1,5 @@
 import { objectAssign } from '../utils/lang/objectAssign';
 import { IStatusInterface, SplitIO } from '../types';
-import { CONSUMER_MODE, CONSUMER_PARTIAL_MODE } from '../utils/constants';
 import { releaseApiKey } from '../utils/inputValidation/apiKey';
 import { clientFactory } from './client';
 import { clientInputValidationDecorator } from './clientInputValidation';
@@ -18,11 +17,9 @@ export function sdkClientFactory(params: ISdkClientFactoryParams): SplitIO.IClie
 
     // Client API (getTreatment* & track methods)
     clientInputValidationDecorator(
-      settings.log,
+      settings,
       clientFactory(params),
-      sdkReadinessManager.readinessManager,
-      // storage is async if and only if mode is consumer or partial consumer
-      [CONSUMER_MODE, CONSUMER_PARTIAL_MODE].indexOf(settings.mode) === -1 ? true : false,
+      sdkReadinessManager.readinessManager
     ),
 
     // Sdk destroy

src/sdkFactory/index.ts

@@ -20,7 +20,7 @@ import { objectAssign } from '../utils/lang/objectAssign';
 export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO.ISDK | SplitIO.IAsyncSDK {
 
   const { settings, platform, storageFactory, splitApiFactory, extraProps,
-    syncManagerFactory, SignalListener, impressionsObserverFactory, impressionListener,
+    syncManagerFactory, SignalListener, impressionsObserverFactory,
     integrationsManagerFactory, sdkManagerFactory, sdkClientMethodFactory } = params;
   const log = settings.log;
 
@@ -74,8 +74,8 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO.
 
   // trackers
   const observer = impressionsObserverFactory && impressionsObserverFactory();
-  const impressionsTracker = impressionsTrackerFactory(log, storage.impressions, settings, impressionListener, integrationsManager, observer, storage.impressionCounts);
-  const eventTracker = eventTrackerFactory(log, storage.events, integrationsManager);
+  const impressionsTracker = impressionsTrackerFactory(settings, storage.impressions, integrationsManager, observer, storage.impressionCounts);
+  const eventTracker = eventTrackerFactory(settings, storage.events, integrationsManager);
 
   // signal listener
   const signalListener = SignalListener && new SignalListener(syncManager, settings, storage, splitApi);

src/sdkFactory/types.ts

@@ -64,7 +64,6 @@ export interface ISdkFactoryParams {
     serviceApi: ISplitApi | undefined) => 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

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<boolean>;
 
-      const result2 = tracker.track(fakeEvent, 2) as Promise<boolean>;
+    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<boolean>;
+
+    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<boolean>;
+  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
   });
 
 });