From 0e67f8ae03fe7daa8e883dbdaab021fc8537a2a6 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 23 Feb 2022 13:00:53 -0300 Subject: [PATCH 01/32] implementation and unit tests --- src/logger/constants.ts | 1 + src/logger/messages/warn.ts | 1 + src/sdkClient/__tests__/client.spec.ts | 39 ++++++++++++++++++ src/sdkClient/client.ts | 12 +++--- src/sync/__tests__/syncManagerOnline.spec.ts | 43 ++++++++++++++++++++ src/sync/submitters/eventsSyncTask.ts | 11 +++-- src/sync/submitters/impressionsSyncTask.ts | 11 +++-- src/sync/syncManagerOnline.ts | 22 +++++++--- src/sync/types.ts | 5 ++- src/utils/constants/index.ts | 5 +++ 10 files changed, 133 insertions(+), 17 deletions(-) create mode 100644 src/sdkClient/__tests__/client.spec.ts create mode 100644 src/sync/__tests__/syncManagerOnline.spec.ts diff --git a/src/logger/constants.ts b/src/logger/constants.ts index ada74e13..f9e5041b 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -93,6 +93,7 @@ export const WARN_SPLITS_FILTER_INVALID = 220; export const WARN_SPLITS_FILTER_EMPTY = 221; export const WARN_API_KEY = 222; export const STREAMING_PARSING_MY_SEGMENTS_UPDATE_V2 = 223; +export const SUBMITTERS_PUSH_FULL_QUEUE_DROPPED = 224; export const ERROR_ENGINE_COMBINER_IFELSEIF = 300; export const ERROR_LOGLEVEL_INVALID = 301; diff --git a/src/logger/messages/warn.ts b/src/logger/messages/warn.ts index f6a66eec..ccda06b8 100644 --- a/src/logger/messages/warn.ts +++ b/src/logger/messages/warn.ts @@ -13,6 +13,7 @@ export const codesWarn: [number, string][] = codesError.concat([ [c.STREAMING_FALLBACK, c.LOG_PREFIX_SYNC_STREAMING + 'Falling back to polling mode. Reason: %s'], [c.SUBMITTERS_PUSH_FAILS, c.LOG_PREFIX_SYNC_SUBMITTERS + 'Droping %s %s after retry. Reason: %s.'], [c.SUBMITTERS_PUSH_RETRY, c.LOG_PREFIX_SYNC_SUBMITTERS + 'Failed to push %s %s, keeping data to retry on next iteration. Reason: %s.'], + [c.SUBMITTERS_PUSH_FULL_QUEUE_DROPPED, c.LOG_PREFIX_SYNC_SUBMITTERS + 'Droping %s from full queue. Reason: user consent declined.'], // client status [c.CLIENT_NOT_READY, '%s: the SDK is not ready, results may be incorrect. Make sure to wait for SDK readiness before using this method.'], [c.CLIENT_NO_LISTENER, 'No listeners for SDK Readiness detected. Incorrect control treatments could have been logged if you called getTreatment/s while the SDK was not yet ready.'], diff --git a/src/sdkClient/__tests__/client.spec.ts b/src/sdkClient/__tests__/client.spec.ts new file mode 100644 index 00000000..f447d6d3 --- /dev/null +++ b/src/sdkClient/__tests__/client.spec.ts @@ -0,0 +1,39 @@ +import { ISettings } from '../../types'; +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: ISettings = { ...fullSettings, userConsent: undefined }; + 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 5b0b7de2..7d5ab72c 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 { CONTROL } from '../utils/constants'; +import { CONSENT_DECLINED, CONTROL } from '../utils/constants'; import { IClientFactoryParams } from './types'; import { IEvaluationResult } from '../evaluator/types'; import { SplitIO, ImpressionDTO } from '../types'; @@ -16,13 +16,14 @@ import { IMPRESSION, IMPRESSION_QUEUEING } from '../logger/constants'; */ // @TODO missing time tracking to collect telemetry export function clientFactory(params: IClientFactoryParams): SplitIO.IClient | SplitIO.IAsyncClient { - const { sdkReadinessManager: { readinessManager }, storage, settings: { log, mode }, impressionsTracker, eventTracker } = params; + const { sdkReadinessManager: { readinessManager }, storage, settings, impressionsTracker, eventTracker } = params; + const { log, mode } = settings; function getTreatment(key: SplitIO.SplitKey, splitName: string, attributes: SplitIO.Attributes | undefined, withConfig = false) { const wrapUp = (evaluationResult: IEvaluationResult) => { const queue: ImpressionDTO[] = []; const treatment = processEvaluation(evaluationResult, splitName, key, attributes, withConfig, `getTreatment${withConfig ? 'withConfig' : ''}`, queue); - impressionsTracker.track(queue, attributes); + if (settings.userConsent !== CONSENT_DECLINED) impressionsTracker.track(queue, attributes); return treatment; }; @@ -42,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); }); - impressionsTracker.track(queue, attributes); + if (settings.userConsent !== CONSENT_DECLINED) impressionsTracker.track(queue, attributes); return treatments; }; @@ -115,7 +116,8 @@ 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'); - return eventTracker.track(eventData, size); + if (settings.userConsent !== CONSENT_DECLINED) return eventTracker.track(eventData, size); + else return false; } return { diff --git a/src/sync/__tests__/syncManagerOnline.spec.ts b/src/sync/__tests__/syncManagerOnline.spec.ts new file mode 100644 index 00000000..0e8f2a64 --- /dev/null +++ b/src/sync/__tests__/syncManagerOnline.spec.ts @@ -0,0 +1,43 @@ +import { ISettings } from '../../types'; +import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.mocks'; +import { syncTaskFactory } from './syncTask.mock'; + +jest.mock('../submitters/submitterManager', () => { + return { + submitterManagerFactory: syncTaskFactory + }; +}); + +import { syncManagerOnlineFactory } from '../syncManagerOnline'; + +test('syncManagerOnline should start or not the submitter depending on user consent status', () => { + const settings: ISettings = { ...fullSettings, userConsent: undefined }; + + // @ts-ignore + const syncManager = syncManagerOnlineFactory()({ settings }); + const submitter = syncManager.submitter!; + + syncManager.start(); + expect(submitter.start).toBeCalledTimes(1); // Submitter should be started if userConsent is undefined + syncManager.stop(); + expect(submitter.stop).toBeCalledTimes(1); + + settings.userConsent = 'unknown'; + syncManager.start(); + expect(submitter.start).toBeCalledTimes(1); // Submitter should not be started if userConsent is unknown + syncManager.stop(); + expect(submitter.stop).toBeCalledTimes(2); + + settings.userConsent = 'granted'; + syncManager.start(); + expect(submitter.start).toBeCalledTimes(2); // Submitter should be started if userConsent is granted + syncManager.stop(); + expect(submitter.stop).toBeCalledTimes(3); + + settings.userConsent = 'declined'; + syncManager.start(); + expect(submitter.start).toBeCalledTimes(2); // Submitter should not be started if userConsent is declined + syncManager.stop(); + expect(submitter.stop).toBeCalledTimes(4); + +}); diff --git a/src/sync/submitters/eventsSyncTask.ts b/src/sync/submitters/eventsSyncTask.ts index 151607ea..fcd5b3b8 100644 --- a/src/sync/submitters/eventsSyncTask.ts +++ b/src/sync/submitters/eventsSyncTask.ts @@ -3,7 +3,7 @@ import { IPostEventsBulk } from '../../services/types'; import { ISyncTask, ITimeTracker } from '../types'; import { submitterSyncTaskFactory } from './submitterSyncTask'; import { ILogger } from '../../logger/types'; -import { SUBMITTERS_PUSH_FULL_QUEUE } from '../../logger/constants'; +import { SUBMITTERS_PUSH_FULL_QUEUE, SUBMITTERS_PUSH_FULL_QUEUE_DROPPED } from '../../logger/constants'; const DATA_NAME = 'events'; @@ -38,8 +38,13 @@ export function eventsSyncTaskFactory( // register events submitter to be executed when events cache is full eventsCache.setOnFullQueueCb(() => { - log.info(SUBMITTERS_PUSH_FULL_QUEUE, [DATA_NAME]); - syncTask.execute(); + if (syncTask.isRunning()) { + log.info(SUBMITTERS_PUSH_FULL_QUEUE, [DATA_NAME]); + syncTask.execute(); + } else { // If submitter is stopped (e.g., user consent declined, or app state offline), we drop items + log.warn(SUBMITTERS_PUSH_FULL_QUEUE_DROPPED); + eventsCache.clear(); + } }); return syncTask; diff --git a/src/sync/submitters/impressionsSyncTask.ts b/src/sync/submitters/impressionsSyncTask.ts index 5947c40c..9f225fdd 100644 --- a/src/sync/submitters/impressionsSyncTask.ts +++ b/src/sync/submitters/impressionsSyncTask.ts @@ -6,7 +6,7 @@ import { ImpressionDTO } from '../../types'; import { submitterSyncTaskFactory } from './submitterSyncTask'; import { ImpressionsPayload } from './types'; import { ILogger } from '../../logger/types'; -import { SUBMITTERS_PUSH_FULL_QUEUE } from '../../logger/constants'; +import { SUBMITTERS_PUSH_FULL_QUEUE, SUBMITTERS_PUSH_FULL_QUEUE_DROPPED } from '../../logger/constants'; const DATA_NAME = 'impressions'; @@ -57,8 +57,13 @@ export function impressionsSyncTaskFactory( // register impressions submitter to be executed when impressions cache is full impressionsCache.setOnFullQueueCb(() => { - log.info(SUBMITTERS_PUSH_FULL_QUEUE, [DATA_NAME]); - syncTask.execute(); + if (syncTask.isRunning()) { + log.info(SUBMITTERS_PUSH_FULL_QUEUE, [DATA_NAME]); + syncTask.execute(); + } else { // If submitter is stopped (e.g., user consent declined, or app state offline), we drop items + log.warn(SUBMITTERS_PUSH_FULL_QUEUE_DROPPED); + impressionsCache.clear(); + } }); return syncTask; diff --git a/src/sync/syncManagerOnline.ts b/src/sync/syncManagerOnline.ts index 1bbfe948..425b9d9f 100644 --- a/src/sync/syncManagerOnline.ts +++ b/src/sync/syncManagerOnline.ts @@ -6,6 +6,7 @@ import { IPushManager } from './streaming/types'; import { IPollingManager, IPollingManagerCS } from './polling/types'; import { PUSH_SUBSYSTEM_UP, PUSH_SUBSYSTEM_DOWN } from './streaming/constants'; import { SYNC_START_POLLING, SYNC_CONTINUE_POLLING, SYNC_STOP_POLLING } from '../logger/constants'; +import { CONSENT_GRANTED } from '../utils/constants'; /** * Online SyncManager factory. @@ -39,6 +40,11 @@ export function syncManagerOnlineFactory( // It is not inyected as push and polling managers, because at the moment it is required const submitter = submitterManagerFactory(params); + function isUserConsentGranted() { + const userConsent = params.settings.userConsent; + // undefined userConsent is handled as granted to avoid a breaking change with users that don't use this features + return !userConsent || userConsent === CONSENT_GRANTED; + } /** Sync Manager logic */ @@ -69,12 +75,18 @@ export function syncManagerOnlineFactory( let startFirstTime = true; // flag to distinguish calling the `start` method for the first time, to support pausing and resuming the synchronization return { + // Exposed for fine-grained control of synchronization. + // E.g.: user consent, app state changes (Page hide, Foreground/Background, Online/Offline). + pollingManager, pushManager, + submitter, /** * Method used to start the syncManager for the first time, or resume it after being stopped. */ start() { + running = true; + // start syncing splits and segments if (pollingManager) { if (pushManager) { @@ -90,21 +102,21 @@ export function syncManagerOnlineFactory( } // start periodic data recording (events, impressions, telemetry). - if (submitter) submitter.start(); - running = true; + if (isUserConsentGranted()) submitter.start(); }, /** * Method used to stop/pause the syncManager. */ stop() { + running = false; + // stop syncing if (pushManager) pushManager.stop(); if (pollingManager && pollingManager.isRunning()) pollingManager.stop(); // stop periodic data recording (events, impressions, telemetry). - if (submitter) submitter.stop(); - running = false; + submitter.stop(); }, isRunning() { @@ -112,7 +124,7 @@ export function syncManagerOnlineFactory( }, flush() { - if (submitter) return submitter.execute(); + if (isUserConsentGranted()) return submitter.execute(); else return Promise.resolve(); }, diff --git a/src/sync/types.ts b/src/sync/types.ts index 0f0588d9..1a70a5c6 100644 --- a/src/sync/types.ts +++ b/src/sync/types.ts @@ -3,6 +3,7 @@ import { IPlatform } from '../sdkFactory/types'; import { ISplitApi } from '../services/types'; import { IStorageSync } from '../storages/types'; import { ISettings } from '../types'; +import { IPollingManager } from './polling/types'; import { IPushManager } from './streaming/types'; export interface ITask { @@ -43,7 +44,9 @@ export interface ITimeTracker { export interface ISyncManager extends ITask { flush(): Promise, - pushManager?: IPushManager + pushManager?: IPushManager, + pollingManager?: IPollingManager, + submitter?: ISyncTask } export interface ISyncManagerCS extends ISyncManager { diff --git a/src/utils/constants/index.ts b/src/utils/constants/index.ts index 4a4d8303..9b3412a5 100644 --- a/src/utils/constants/index.ts +++ b/src/utils/constants/index.ts @@ -32,3 +32,8 @@ export const STORAGE_MEMORY: StorageType = 'MEMORY'; export const STORAGE_LOCALSTORAGE: StorageType = 'LOCALSTORAGE'; export const STORAGE_REDIS: StorageType = 'REDIS'; export const STORAGE_PLUGGABLE: StorageType = 'PLUGGABLE'; + +// User consent +export const CONSENT_GRANTED = 'granted'; // The user has granted consent for tracking events and impressions +export const CONSENT_DECLINED = 'declined'; // The user has declined consent for tracking events and impressions +export const CONSENT_UNKNOWN = 'unknown'; // The user has neither granted nor declined consent for tracking events and impressions From 58620798732acbb4c2828612ec32819dfe472a3d Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 23 Feb 2022 13:29:14 -0300 Subject: [PATCH 02/32] updated settings interface --- src/types.ts | 6 ++++++ src/utils/settingsValidation/__tests__/settings.mocks.ts | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/types.ts b/src/types.ts index 1e3654a8..9f67b027 100644 --- a/src/types.ts +++ b/src/types.ts @@ -54,6 +54,11 @@ type EventConsts = { * @typedef {string} SDKMode */ export type SDKMode = 'standalone' | 'consumer' | 'localhost' | 'consumer_partial'; +/** + * User consent status. + * @typedef {string} ConsentStatus + */ +export type ConsentStatus = 'granted' | 'declined' | 'unknown'; /** * Settings interface. This is a representation of the settings the SDK expose, that's why * most of it's props are readonly. Only features should be rewritten when localhost mode is active. @@ -111,6 +116,7 @@ export interface ISettings { }, readonly log: ILogger readonly impressionListener?: unknown + userConsent?: ConsentStatus } /** * Log levels. diff --git a/src/utils/settingsValidation/__tests__/settings.mocks.ts b/src/utils/settingsValidation/__tests__/settings.mocks.ts index 3cb458cb..ff160297 100644 --- a/src/utils/settingsValidation/__tests__/settings.mocks.ts +++ b/src/utils/settingsValidation/__tests__/settings.mocks.ts @@ -88,7 +88,8 @@ export const fullSettings: ISettings = { auth: 'auth', streaming: 'streaming' }, - log: loggerMock + log: loggerMock, + userConsent: undefined }; export const fullSettingsServerSide = { From b2dfd2e494993eea48f0da4fdaf6c11cd2394579 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 23 Feb 2022 14:11:29 -0300 Subject: [PATCH 03/32] update submitters to not drop data if queues are full but consent has not been granted yet --- src/logger/constants.ts | 1 - src/logger/messages/warn.ts | 1 - src/sync/submitters/eventsSyncTask.ts | 7 +++---- src/sync/submitters/impressionsSyncTask.ts | 7 +++---- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/logger/constants.ts b/src/logger/constants.ts index f9e5041b..ada74e13 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -93,7 +93,6 @@ export const WARN_SPLITS_FILTER_INVALID = 220; export const WARN_SPLITS_FILTER_EMPTY = 221; export const WARN_API_KEY = 222; export const STREAMING_PARSING_MY_SEGMENTS_UPDATE_V2 = 223; -export const SUBMITTERS_PUSH_FULL_QUEUE_DROPPED = 224; export const ERROR_ENGINE_COMBINER_IFELSEIF = 300; export const ERROR_LOGLEVEL_INVALID = 301; diff --git a/src/logger/messages/warn.ts b/src/logger/messages/warn.ts index ccda06b8..f6a66eec 100644 --- a/src/logger/messages/warn.ts +++ b/src/logger/messages/warn.ts @@ -13,7 +13,6 @@ export const codesWarn: [number, string][] = codesError.concat([ [c.STREAMING_FALLBACK, c.LOG_PREFIX_SYNC_STREAMING + 'Falling back to polling mode. Reason: %s'], [c.SUBMITTERS_PUSH_FAILS, c.LOG_PREFIX_SYNC_SUBMITTERS + 'Droping %s %s after retry. Reason: %s.'], [c.SUBMITTERS_PUSH_RETRY, c.LOG_PREFIX_SYNC_SUBMITTERS + 'Failed to push %s %s, keeping data to retry on next iteration. Reason: %s.'], - [c.SUBMITTERS_PUSH_FULL_QUEUE_DROPPED, c.LOG_PREFIX_SYNC_SUBMITTERS + 'Droping %s from full queue. Reason: user consent declined.'], // client status [c.CLIENT_NOT_READY, '%s: the SDK is not ready, results may be incorrect. Make sure to wait for SDK readiness before using this method.'], [c.CLIENT_NO_LISTENER, 'No listeners for SDK Readiness detected. Incorrect control treatments could have been logged if you called getTreatment/s while the SDK was not yet ready.'], diff --git a/src/sync/submitters/eventsSyncTask.ts b/src/sync/submitters/eventsSyncTask.ts index fcd5b3b8..b87ee24e 100644 --- a/src/sync/submitters/eventsSyncTask.ts +++ b/src/sync/submitters/eventsSyncTask.ts @@ -3,7 +3,7 @@ import { IPostEventsBulk } from '../../services/types'; import { ISyncTask, ITimeTracker } from '../types'; import { submitterSyncTaskFactory } from './submitterSyncTask'; import { ILogger } from '../../logger/types'; -import { SUBMITTERS_PUSH_FULL_QUEUE, SUBMITTERS_PUSH_FULL_QUEUE_DROPPED } from '../../logger/constants'; +import { SUBMITTERS_PUSH_FULL_QUEUE } from '../../logger/constants'; const DATA_NAME = 'events'; @@ -41,10 +41,9 @@ export function eventsSyncTaskFactory( if (syncTask.isRunning()) { log.info(SUBMITTERS_PUSH_FULL_QUEUE, [DATA_NAME]); syncTask.execute(); - } else { // If submitter is stopped (e.g., user consent declined, or app state offline), we drop items - log.warn(SUBMITTERS_PUSH_FULL_QUEUE_DROPPED); - eventsCache.clear(); } + // If submitter is stopped (e.g., user consent declined or unknown, or app state offline), we don't send the data. + // Data will be sent when submitter is resumed. }); return syncTask; diff --git a/src/sync/submitters/impressionsSyncTask.ts b/src/sync/submitters/impressionsSyncTask.ts index 9f225fdd..f3fdb20c 100644 --- a/src/sync/submitters/impressionsSyncTask.ts +++ b/src/sync/submitters/impressionsSyncTask.ts @@ -6,7 +6,7 @@ import { ImpressionDTO } from '../../types'; import { submitterSyncTaskFactory } from './submitterSyncTask'; import { ImpressionsPayload } from './types'; import { ILogger } from '../../logger/types'; -import { SUBMITTERS_PUSH_FULL_QUEUE, SUBMITTERS_PUSH_FULL_QUEUE_DROPPED } from '../../logger/constants'; +import { SUBMITTERS_PUSH_FULL_QUEUE } from '../../logger/constants'; const DATA_NAME = 'impressions'; @@ -60,10 +60,9 @@ export function impressionsSyncTaskFactory( if (syncTask.isRunning()) { log.info(SUBMITTERS_PUSH_FULL_QUEUE, [DATA_NAME]); syncTask.execute(); - } else { // If submitter is stopped (e.g., user consent declined, or app state offline), we drop items - log.warn(SUBMITTERS_PUSH_FULL_QUEUE_DROPPED); - impressionsCache.clear(); } + // If submitter is stopped (e.g., user consent declined or unknown, or app state offline), we don't send the data. + // Data will be sent when submitter is resumed. }); return syncTask; From 73423894a45b59e946883d8790dd808b2b5ead8d Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 23 Feb 2022 16:14:52 -0300 Subject: [PATCH 04/32] Modify browser listener flow to avoid flushing data without consent --- src/listeners/__tests__/browser.spec.ts | 36 +++++++++++++++++++++++-- src/listeners/browser.ts | 22 ++++++++------- src/sync/syncManagerOnline.ts | 14 +++------- src/utils/consent.ts | 8 ++++++ 4 files changed, 59 insertions(+), 21 deletions(-) create mode 100644 src/utils/consent.ts diff --git a/src/listeners/__tests__/browser.spec.ts b/src/listeners/__tests__/browser.spec.ts index 8af13e8b..730c19de 100644 --- a/src/listeners/__tests__/browser.spec.ts +++ b/src/listeners/__tests__/browser.spec.ts @@ -85,11 +85,12 @@ beforeAll(() => { }); }); -// clean mocks -afterEach(() => { +// clear mocks +beforeEach(() => { (global.window.addEventListener as jest.Mock).mockClear(); (global.window.removeEventListener as jest.Mock).mockClear(); if (global.window.navigator.sendBeacon) (global.window.navigator.sendBeacon as jest.Mock).mockClear(); + Object.values(fakeSplitApi).forEach(method => method.mockClear()); }); // delete mocks from global @@ -227,3 +228,34 @@ test('Browser JS listener / standalone mode / Impressions debug mode without sen // restore sendBeacon API global.navigator.sendBeacon = sendBeacon; }); + +test('Browser JS listener / standalone mode / user consent status', () => { + const syncManagerMock = {}; + const settings = { ...fullSettings }; + + // @ts-expect-error + const listener = new BrowserSignalListener(syncManagerMock, settings, fakeStorageOptimized as IStorageSync, fakeSplitApi); + + listener.start(); + + settings.userConsent = 'unknown'; + triggerUnloadEvent(); + settings.userConsent = 'declined'; + triggerUnloadEvent(); + + // Unload event was triggered when user consent was unknown and declined. Thus sendBeacon and post services should not be called + expect(global.window.navigator.sendBeacon).toBeCalledTimes(0); + expect(fakeSplitApi.postTestImpressionsBulk).not.toBeCalled(); + expect(fakeSplitApi.postEventsBulk).not.toBeCalled(); + expect(fakeSplitApi.postTestImpressionsCount).not.toBeCalled(); + + settings.userConsent = 'granted'; + triggerUnloadEvent(); + settings.userConsent = undefined; + triggerUnloadEvent(); + + // Unload event was triggered when user consent was granted and undefined. Thus sendBeacon should be called 6 times (3 times per event in optimized mode). + expect(global.window.navigator.sendBeacon).toBeCalledTimes(6); + + listener.stop(); +}); diff --git a/src/listeners/browser.ts b/src/listeners/browser.ts index 9705af2c..502c4e43 100644 --- a/src/listeners/browser.ts +++ b/src/listeners/browser.ts @@ -11,6 +11,7 @@ import { OPTIMIZED, DEBUG } from '../utils/constants'; import { objectAssign } from '../utils/lang/objectAssign'; import { CLEANUP_REGISTERING, CLEANUP_DEREGISTERING } from '../logger/constants'; import { ISyncManager } from '../sync/types'; +import { isConsentGranted } from '../utils/consent'; // 'unload' event is used instead of 'beforeunload', since 'unload' is not a cancelable event, so no other listeners can stop the event from occurring. const UNLOAD_DOM_EVENT = 'unload'; @@ -65,15 +66,18 @@ export class BrowserSignalListener implements ISignalListener { flushData() { if (!this.syncManager) return; // In consumer mode there is not sync manager and data to flush - const eventsUrl = this.settings.urls.events; - const extraMetadata = { - // sim stands for Sync/Split Impressions Mode - sim: this.settings.sync.impressionsMode === OPTIMIZED ? OPTIMIZED : DEBUG - }; + // Flush data if there is user consent + if (isConsentGranted(this.settings)) { + const eventsUrl = this.settings.urls.events; + const extraMetadata = { + // sim stands for Sync/Split Impressions Mode + sim: this.settings.sync.impressionsMode === OPTIMIZED ? OPTIMIZED : DEBUG + }; - this._flushData(eventsUrl + '/testImpressions/beacon', this.storage.impressions, this.serviceApi.postTestImpressionsBulk, this.fromImpressionsCollector, extraMetadata); - this._flushData(eventsUrl + '/events/beacon', this.storage.events, this.serviceApi.postEventsBulk); - if (this.storage.impressionCounts) this._flushData(eventsUrl + '/testImpressions/count/beacon', this.storage.impressionCounts, this.serviceApi.postTestImpressionsCount, fromImpressionCountsCollector); + this._flushData(eventsUrl + '/testImpressions/beacon', this.storage.impressions, this.serviceApi.postTestImpressionsBulk, this.fromImpressionsCollector, extraMetadata); + this._flushData(eventsUrl + '/events/beacon', this.storage.events, this.serviceApi.postEventsBulk); + if (this.storage.impressionCounts) this._flushData(eventsUrl + '/testImpressions/count/beacon', this.storage.impressionCounts, this.serviceApi.postTestImpressionsCount, fromImpressionCountsCollector); + } // Close streaming connection if (this.syncManager.pushManager) this.syncManager.pushManager.stop(); @@ -84,7 +88,7 @@ export class BrowserSignalListener implements ISignalListener { if (!cache.isEmpty()) { const dataPayload = fromCacheToPayload ? fromCacheToPayload(cache.state()) : cache.state(); if (!this._sendBeacon(url, dataPayload, extraMetadata)) { - postService(JSON.stringify(dataPayload)).catch(() => { }); // no-op just to catch a possible exceptions + postService(JSON.stringify(dataPayload)).catch(() => { }); // no-op just to catch a possible exception } cache.clear(); } diff --git a/src/sync/syncManagerOnline.ts b/src/sync/syncManagerOnline.ts index 425b9d9f..b95be139 100644 --- a/src/sync/syncManagerOnline.ts +++ b/src/sync/syncManagerOnline.ts @@ -6,7 +6,7 @@ import { IPushManager } from './streaming/types'; import { IPollingManager, IPollingManagerCS } from './polling/types'; import { PUSH_SUBSYSTEM_UP, PUSH_SUBSYSTEM_DOWN } from './streaming/constants'; import { SYNC_START_POLLING, SYNC_CONTINUE_POLLING, SYNC_STOP_POLLING } from '../logger/constants'; -import { CONSENT_GRANTED } from '../utils/constants'; +import { isConsentGranted } from '../utils/consent'; /** * Online SyncManager factory. @@ -26,7 +26,7 @@ export function syncManagerOnlineFactory( */ return function (params: ISyncManagerFactoryParams): ISyncManagerCS { - const { log, streamingEnabled } = params.settings; + const { settings, settings: { log, streamingEnabled } } = params; /** Polling Manager */ const pollingManager = pollingManagerFactory && pollingManagerFactory(params); @@ -40,12 +40,6 @@ export function syncManagerOnlineFactory( // It is not inyected as push and polling managers, because at the moment it is required const submitter = submitterManagerFactory(params); - function isUserConsentGranted() { - const userConsent = params.settings.userConsent; - // undefined userConsent is handled as granted to avoid a breaking change with users that don't use this features - return !userConsent || userConsent === CONSENT_GRANTED; - } - /** Sync Manager logic */ function startPolling() { @@ -102,7 +96,7 @@ export function syncManagerOnlineFactory( } // start periodic data recording (events, impressions, telemetry). - if (isUserConsentGranted()) submitter.start(); + if (isConsentGranted(settings)) submitter.start(); }, /** @@ -124,7 +118,7 @@ export function syncManagerOnlineFactory( }, flush() { - if (isUserConsentGranted()) return submitter.execute(); + if (isConsentGranted(settings)) return submitter.execute(); else return Promise.resolve(); }, diff --git a/src/utils/consent.ts b/src/utils/consent.ts new file mode 100644 index 00000000..20ca745c --- /dev/null +++ b/src/utils/consent.ts @@ -0,0 +1,8 @@ +import { ISettings } from '../types'; +import { CONSENT_GRANTED } from './constants'; + +export function isConsentGranted(settings: ISettings) { + const userConsent = settings.userConsent; + // undefined userConsent is handled as granted (default) + return !userConsent || userConsent === CONSENT_GRANTED; +} From bae3e5ee12695c7a77ef2b869a4d1eb9450e1091 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 24 Feb 2022 16:25:56 -0300 Subject: [PATCH 05/32] added getter & setter for user consent in factory --- src/logger/constants.ts | 2 + src/logger/messages/error.ts | 1 + src/logger/messages/info.ts | 1 + src/sdkClient/__tests__/client.spec.ts | 2 +- .../__tests__/userConsentProps.spec.ts | 39 +++++++++++++++++++ src/sdkFactory/index.ts | 9 +++-- src/sdkFactory/types.ts | 2 + src/sdkFactory/userConsentProps.ts | 37 ++++++++++++++++++ src/sync/__tests__/syncManagerOnline.spec.ts | 2 +- 9 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 src/sdkFactory/__tests__/userConsentProps.spec.ts create mode 100644 src/sdkFactory/userConsentProps.ts diff --git a/src/logger/constants.ts b/src/logger/constants.ts index ada74e13..92428e5b 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -68,6 +68,7 @@ export const SYNC_CONTINUE_POLLING = 118; export const SYNC_STOP_POLLING = 119; export const EVENTS_TRACKER_SUCCESS = 120; export const IMPRESSIONS_TRACKER_SUCCESS = 121; +export const USER_CONSENT_UPDATED = 122; export const ENGINE_VALUE_INVALID = 200; export const ENGINE_VALUE_NO_ATTRIBUTES = 201; @@ -119,6 +120,7 @@ export const ERROR_INVALID_IMPRESSIONS_MODE = 321; export const ERROR_HTTP = 322; export const ERROR_LOCALHOST_MODULE_REQUIRED = 323; export const ERROR_STORAGE_INVALID = 324; +export const ERROR_NOT_BOOLEAN = 325; // Log prefixes (a.k.a. tags or categories) export const LOG_PREFIX_SETTINGS = 'settings'; diff --git a/src/logger/messages/error.ts b/src/logger/messages/error.ts index 1d0fe00e..4c2b3d70 100644 --- a/src/logger/messages/error.ts +++ b/src/logger/messages/error.ts @@ -28,6 +28,7 @@ export const codesError: [number, string][] = [ [c.ERROR_INVALID, '%s: you passed an invalid %s. It must be a non-empty string.'], [c.ERROR_EMPTY, '%s: you passed an empty %s. It must be a non-empty string.'], [c.ERROR_EMPTY_ARRAY, '%s: %s must be a non-empty array.'], + [c.ERROR_NOT_BOOLEAN, '%s: param must be boolean.'], // initialization / settings validation [c.ERROR_INVALID_IMPRESSIONS_MODE, c.LOG_PREFIX_SETTINGS + ': you passed an invalid "impressionsMode". It should be one of the following values: %s. Defaulting to "%s" mode.'], [c.ERROR_LOCALHOST_MODULE_REQUIRED, c.LOG_PREFIX_SETTINGS + ': an invalid value was received for "sync.localhostMode" config. A valid entity should be provided for localhost mode.'], diff --git a/src/logger/messages/info.ts b/src/logger/messages/info.ts index a8746dec..bac83ea9 100644 --- a/src/logger/messages/info.ts +++ b/src/logger/messages/info.ts @@ -14,6 +14,7 @@ export const codesInfo: [number, string][] = codesWarn.concat([ [c.NEW_FACTORY, ' New Split SDK instance created.'], [c.EVENTS_TRACKER_SUCCESS, c.LOG_PREFIX_EVENTS_TRACKER + 'Successfully queued %s'], [c.IMPRESSIONS_TRACKER_SUCCESS, c.LOG_PREFIX_IMPRESSIONS_TRACKER + 'Successfully stored %s impression(s).'], + [c.USER_CONSENT_UPDATED, ' User consent changed from %s to %s.'], // synchronizer [c.POLLING_SMART_PAUSING, c.LOG_PREFIX_SYNC_POLLING + 'Turning segments data polling %s.'], diff --git a/src/sdkClient/__tests__/client.spec.ts b/src/sdkClient/__tests__/client.spec.ts index f447d6d3..a46be232 100644 --- a/src/sdkClient/__tests__/client.spec.ts +++ b/src/sdkClient/__tests__/client.spec.ts @@ -6,7 +6,7 @@ test('client should track or not events and impressions depending on user consen const sdkReadinessManager = { readinessManager: { isReady: () => true } }; const storage = { splits: { trafficTypeExists: () => true } }; - const settings: ISettings = { ...fullSettings, userConsent: undefined }; + const settings: ISettings = { ...fullSettings }; const eventTracker = { track: jest.fn() }; const impressionsTracker = { track: jest.fn() }; diff --git a/src/sdkFactory/__tests__/userConsentProps.spec.ts b/src/sdkFactory/__tests__/userConsentProps.spec.ts new file mode 100644 index 00000000..7a173387 --- /dev/null +++ b/src/sdkFactory/__tests__/userConsentProps.spec.ts @@ -0,0 +1,39 @@ +import { userConsentProps } from '../userConsentProps'; +import { syncTaskFactory } from '../../sync/__tests__/syncTask.mock'; +import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.mocks'; + +test('userConsentProps', () => { + const settings = { ...fullSettings }; + const syncManager = { submitter: syncTaskFactory() }; + + // @ts-ignore + const props = userConsentProps(settings, syncManager); + + // getUserConsent returns settings.userConsent + expect(props.getUserConsent()).toBe(settings.userConsent); + + // setting user consent to 'granted' + expect(props.setUserConsent(true)).toBe(true); + expect(props.setUserConsent(true)).toBe(true); // calling again has no affect + expect(syncManager.submitter.start).toBeCalledTimes(1); // submitter resumed + expect(syncManager.submitter.stop).toBeCalledTimes(0); + expect(props.getUserConsent()).toBe('granted'); + + // setting user consent to 'declined' + expect(props.setUserConsent(false)).toBe(true); + expect(props.setUserConsent(false)).toBe(true); // calling again has no affect + expect(syncManager.submitter.start).toBeCalledTimes(1); + expect(syncManager.submitter.stop).toBeCalledTimes(1); // submitter paused + expect(props.getUserConsent()).toBe('declined'); + + // Invalid values have no effect + expect(props.setUserConsent('declined')).toBe(false); // strings are not valid + expect(props.setUserConsent('granted')).toBe(false); + expect(props.setUserConsent(undefined)).toBe(false); + expect(props.setUserConsent({})).toBe(false); + + expect(syncManager.submitter.start).toBeCalledTimes(1); + expect(syncManager.submitter.stop).toBeCalledTimes(1); + expect(props.getUserConsent()).toBe('declined'); + +}); diff --git a/src/sdkFactory/index.ts b/src/sdkFactory/index.ts index 5290d5c7..2d645796 100644 --- a/src/sdkFactory/index.ts +++ b/src/sdkFactory/index.ts @@ -12,13 +12,14 @@ import { createLoggerAPI } from '../logger/sdkLogger'; import { NEW_FACTORY, RETRIEVE_MANAGER } from '../logger/constants'; import { metadataBuilder } from '../storages/metadataBuilder'; import { SDK_SPLITS_ARRIVED, SDK_SEGMENTS_ARRIVED } from '../readiness/constants'; +import { objectAssign } from '../utils/lang/objectAssign'; /** * Modular SDK factory */ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO.ISDK | SplitIO.IAsyncSDK { - const { settings, platform, storageFactory, splitApiFactory, + const { settings, platform, storageFactory, splitApiFactory, extraProps, syncManagerFactory, SignalListener, impressionsObserverFactory, impressionListener, integrationsManagerFactory, sdkManagerFactory, sdkClientMethodFactory } = params; const log = settings.log; @@ -88,12 +89,12 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO. log.info(NEW_FACTORY); - return { + // @ts-ignore + return objectAssign({ // Split evaluation and event tracking engine client: clientMethod, // Manager API to explore available information - // @ts-ignore manager() { log.debug(RETRIEVE_MANAGER); return managerInstance; @@ -103,5 +104,5 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO. Logger: createLoggerAPI(settings.log), settings, - }; + }, extraProps && extraProps(settings, syncManager)); } diff --git a/src/sdkFactory/types.ts b/src/sdkFactory/types.ts index d907d2d0..842a6068 100644 --- a/src/sdkFactory/types.ts +++ b/src/sdkFactory/types.ts @@ -70,4 +70,6 @@ export interface ISdkFactoryParams { // Impression observer factory. If provided, will be used for impressions dedupe impressionsObserverFactory?: () => IImpressionObserver + // Optional function to assign additional properties to the factory instance + extraProps?: (settings: ISettings, syncManager?: ISyncManager) => object } diff --git a/src/sdkFactory/userConsentProps.ts b/src/sdkFactory/userConsentProps.ts new file mode 100644 index 00000000..b9498360 --- /dev/null +++ b/src/sdkFactory/userConsentProps.ts @@ -0,0 +1,37 @@ +import { ERROR_NOT_BOOLEAN, USER_CONSENT_UPDATED } from '../logger/constants'; +import { ISyncManager } from '../sync/types'; +import { ISettings } from '../types'; +import { CONSENT_GRANTED, CONSENT_DECLINED } from '../utils/constants'; + +// Extend client-side factory instances with user consent getter/setter +export function userConsentProps(settings: ISettings, syncManager?: ISyncManager) { + + const log = settings.log; + + return { + setUserConsent(consent: unknown) { + // validate input param + if (typeof consent !== 'boolean') { + log.error(ERROR_NOT_BOOLEAN, ['setUserConsent']); + return false; + } + + const newConsentStatus = consent ? CONSENT_GRANTED : CONSENT_DECLINED; + + if (settings.userConsent !== newConsentStatus) { + // resume/pause submitters + if (consent) syncManager?.submitter?.start(); + else syncManager?.submitter?.stop(); + + log.info(USER_CONSENT_UPDATED, [settings.userConsent, newConsentStatus]); + settings.userConsent = newConsentStatus; + } + + return true; + }, + + getUserConsent() { + return settings.userConsent; + } + }; +} diff --git a/src/sync/__tests__/syncManagerOnline.spec.ts b/src/sync/__tests__/syncManagerOnline.spec.ts index 0e8f2a64..cd94c9bd 100644 --- a/src/sync/__tests__/syncManagerOnline.spec.ts +++ b/src/sync/__tests__/syncManagerOnline.spec.ts @@ -11,7 +11,7 @@ jest.mock('../submitters/submitterManager', () => { import { syncManagerOnlineFactory } from '../syncManagerOnline'; test('syncManagerOnline should start or not the submitter depending on user consent status', () => { - const settings: ISettings = { ...fullSettings, userConsent: undefined }; + const settings: ISettings = { ...fullSettings }; // @ts-ignore const syncManager = syncManagerOnlineFactory()({ settings }); From 961f56c5bdcf5e7a82bf18a80852e5728858666a Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 24 Feb 2022 16:35:12 -0300 Subject: [PATCH 06/32] constants to uppercase --- src/sdkClient/__tests__/client.spec.ts | 9 ++++----- src/sync/__tests__/syncManagerOnline.spec.ts | 9 ++++----- src/types.ts | 4 ++-- src/utils/constants/index.ts | 6 +++--- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/sdkClient/__tests__/client.spec.ts b/src/sdkClient/__tests__/client.spec.ts index f447d6d3..4112d7d5 100644 --- a/src/sdkClient/__tests__/client.spec.ts +++ b/src/sdkClient/__tests__/client.spec.ts @@ -1,4 +1,3 @@ -import { ISettings } from '../../types'; import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.mocks'; import { clientFactory } from '../client'; @@ -6,7 +5,7 @@ test('client should track or not events and impressions depending on user consen const sdkReadinessManager = { readinessManager: { isReady: () => true } }; const storage = { splits: { trafficTypeExists: () => true } }; - const settings: ISettings = { ...fullSettings, userConsent: undefined }; + const settings = { ...fullSettings }; const eventTracker = { track: jest.fn() }; const impressionsTracker = { track: jest.fn() }; @@ -18,19 +17,19 @@ test('client should track or not events and impressions depending on user consen client.track('some_key', 'some_tt', 'some_event'); expect(eventTracker.track).toBeCalledTimes(1); // event should be tracked if userConsent is undefined - settings.userConsent = 'unknown'; + 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'; + 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'; + 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'); diff --git a/src/sync/__tests__/syncManagerOnline.spec.ts b/src/sync/__tests__/syncManagerOnline.spec.ts index 0e8f2a64..c47cad86 100644 --- a/src/sync/__tests__/syncManagerOnline.spec.ts +++ b/src/sync/__tests__/syncManagerOnline.spec.ts @@ -1,4 +1,3 @@ -import { ISettings } from '../../types'; import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.mocks'; import { syncTaskFactory } from './syncTask.mock'; @@ -11,7 +10,7 @@ jest.mock('../submitters/submitterManager', () => { import { syncManagerOnlineFactory } from '../syncManagerOnline'; test('syncManagerOnline should start or not the submitter depending on user consent status', () => { - const settings: ISettings = { ...fullSettings, userConsent: undefined }; + const settings = { ...fullSettings }; // @ts-ignore const syncManager = syncManagerOnlineFactory()({ settings }); @@ -22,19 +21,19 @@ test('syncManagerOnline should start or not the submitter depending on user cons syncManager.stop(); expect(submitter.stop).toBeCalledTimes(1); - settings.userConsent = 'unknown'; + settings.userConsent = 'UNKNOWN'; syncManager.start(); expect(submitter.start).toBeCalledTimes(1); // Submitter should not be started if userConsent is unknown syncManager.stop(); expect(submitter.stop).toBeCalledTimes(2); - settings.userConsent = 'granted'; + settings.userConsent = 'GRANTED'; syncManager.start(); expect(submitter.start).toBeCalledTimes(2); // Submitter should be started if userConsent is granted syncManager.stop(); expect(submitter.stop).toBeCalledTimes(3); - settings.userConsent = 'declined'; + settings.userConsent = 'DECLINED'; syncManager.start(); expect(submitter.start).toBeCalledTimes(2); // Submitter should not be started if userConsent is declined syncManager.stop(); diff --git a/src/types.ts b/src/types.ts index 9f67b027..c449ff37 100644 --- a/src/types.ts +++ b/src/types.ts @@ -58,7 +58,7 @@ export type SDKMode = 'standalone' | 'consumer' | 'localhost' | 'consumer_partia * User consent status. * @typedef {string} ConsentStatus */ -export type ConsentStatus = 'granted' | 'declined' | 'unknown'; +export type ConsentStatus = 'GRANTED' | 'DECLINED' | 'UNKNOWN'; /** * Settings interface. This is a representation of the settings the SDK expose, that's why * most of it's props are readonly. Only features should be rewritten when localhost mode is active. @@ -116,7 +116,7 @@ export interface ISettings { }, readonly log: ILogger readonly impressionListener?: unknown - userConsent?: ConsentStatus + readonly userConsent?: ConsentStatus } /** * Log levels. diff --git a/src/utils/constants/index.ts b/src/utils/constants/index.ts index 9b3412a5..243accd6 100644 --- a/src/utils/constants/index.ts +++ b/src/utils/constants/index.ts @@ -34,6 +34,6 @@ export const STORAGE_REDIS: StorageType = 'REDIS'; export const STORAGE_PLUGGABLE: StorageType = 'PLUGGABLE'; // User consent -export const CONSENT_GRANTED = 'granted'; // The user has granted consent for tracking events and impressions -export const CONSENT_DECLINED = 'declined'; // The user has declined consent for tracking events and impressions -export const CONSENT_UNKNOWN = 'unknown'; // The user has neither granted nor declined consent for tracking events and impressions +export const CONSENT_GRANTED = 'GRANTED'; // The user has granted consent for tracking events and impressions +export const CONSENT_DECLINED = 'DECLINED'; // The user has declined consent for tracking events and impressions +export const CONSENT_UNKNOWN = 'UNKNOWN'; // The user has neither granted nor declined consent for tracking events and impressions From 37cc9fb076ffe2c75597e049efa97f2704eb785b Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 24 Feb 2022 16:42:35 -0300 Subject: [PATCH 07/32] constants to uppercase --- src/listeners/__tests__/browser.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/listeners/__tests__/browser.spec.ts b/src/listeners/__tests__/browser.spec.ts index 730c19de..5ec91eff 100644 --- a/src/listeners/__tests__/browser.spec.ts +++ b/src/listeners/__tests__/browser.spec.ts @@ -238,9 +238,9 @@ test('Browser JS listener / standalone mode / user consent status', () => { listener.start(); - settings.userConsent = 'unknown'; + settings.userConsent = 'UNKNOWN'; triggerUnloadEvent(); - settings.userConsent = 'declined'; + settings.userConsent = 'DECLINED'; triggerUnloadEvent(); // Unload event was triggered when user consent was unknown and declined. Thus sendBeacon and post services should not be called @@ -249,7 +249,7 @@ test('Browser JS listener / standalone mode / user consent status', () => { expect(fakeSplitApi.postEventsBulk).not.toBeCalled(); expect(fakeSplitApi.postTestImpressionsCount).not.toBeCalled(); - settings.userConsent = 'granted'; + settings.userConsent = 'GRANTED'; triggerUnloadEvent(); settings.userConsent = undefined; triggerUnloadEvent(); From 9669ad87d1af146b713aa9fe8f0fea0c48b19f43 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 24 Feb 2022 16:51:46 -0300 Subject: [PATCH 08/32] constants to uppercase --- src/logger/messages/error.ts | 2 +- src/logger/messages/info.ts | 2 +- src/sdkFactory/__tests__/userConsentProps.spec.ts | 14 +++++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/logger/messages/error.ts b/src/logger/messages/error.ts index 4c2b3d70..caa2936d 100644 --- a/src/logger/messages/error.ts +++ b/src/logger/messages/error.ts @@ -28,7 +28,7 @@ export const codesError: [number, string][] = [ [c.ERROR_INVALID, '%s: you passed an invalid %s. It must be a non-empty string.'], [c.ERROR_EMPTY, '%s: you passed an empty %s. It must be a non-empty string.'], [c.ERROR_EMPTY_ARRAY, '%s: %s must be a non-empty array.'], - [c.ERROR_NOT_BOOLEAN, '%s: param must be boolean.'], + [c.ERROR_NOT_BOOLEAN, '%s: you must provide a boolean param.'], // initialization / settings validation [c.ERROR_INVALID_IMPRESSIONS_MODE, c.LOG_PREFIX_SETTINGS + ': you passed an invalid "impressionsMode". It should be one of the following values: %s. Defaulting to "%s" mode.'], [c.ERROR_LOCALHOST_MODULE_REQUIRED, c.LOG_PREFIX_SETTINGS + ': an invalid value was received for "sync.localhostMode" config. A valid entity should be provided for localhost mode.'], diff --git a/src/logger/messages/info.ts b/src/logger/messages/info.ts index bac83ea9..4facaf24 100644 --- a/src/logger/messages/info.ts +++ b/src/logger/messages/info.ts @@ -14,7 +14,7 @@ export const codesInfo: [number, string][] = codesWarn.concat([ [c.NEW_FACTORY, ' New Split SDK instance created.'], [c.EVENTS_TRACKER_SUCCESS, c.LOG_PREFIX_EVENTS_TRACKER + 'Successfully queued %s'], [c.IMPRESSIONS_TRACKER_SUCCESS, c.LOG_PREFIX_IMPRESSIONS_TRACKER + 'Successfully stored %s impression(s).'], - [c.USER_CONSENT_UPDATED, ' User consent changed from %s to %s.'], + [c.USER_CONSENT_UPDATED, 'User consent changed from %s to %s.'], // synchronizer [c.POLLING_SMART_PAUSING, c.LOG_PREFIX_SYNC_POLLING + 'Turning segments data polling %s.'], diff --git a/src/sdkFactory/__tests__/userConsentProps.spec.ts b/src/sdkFactory/__tests__/userConsentProps.spec.ts index 7a173387..e7f4d0fc 100644 --- a/src/sdkFactory/__tests__/userConsentProps.spec.ts +++ b/src/sdkFactory/__tests__/userConsentProps.spec.ts @@ -12,28 +12,28 @@ test('userConsentProps', () => { // getUserConsent returns settings.userConsent expect(props.getUserConsent()).toBe(settings.userConsent); - // setting user consent to 'granted' + // setting user consent to 'GRANTED' expect(props.setUserConsent(true)).toBe(true); expect(props.setUserConsent(true)).toBe(true); // calling again has no affect expect(syncManager.submitter.start).toBeCalledTimes(1); // submitter resumed expect(syncManager.submitter.stop).toBeCalledTimes(0); - expect(props.getUserConsent()).toBe('granted'); + expect(props.getUserConsent()).toBe('GRANTED'); - // setting user consent to 'declined' + // setting user consent to 'DECLINED' expect(props.setUserConsent(false)).toBe(true); expect(props.setUserConsent(false)).toBe(true); // calling again has no affect expect(syncManager.submitter.start).toBeCalledTimes(1); expect(syncManager.submitter.stop).toBeCalledTimes(1); // submitter paused - expect(props.getUserConsent()).toBe('declined'); + expect(props.getUserConsent()).toBe('DECLINED'); // Invalid values have no effect - expect(props.setUserConsent('declined')).toBe(false); // strings are not valid - expect(props.setUserConsent('granted')).toBe(false); + expect(props.setUserConsent('DECLINED')).toBe(false); // strings are not valid + expect(props.setUserConsent('GRANTED')).toBe(false); expect(props.setUserConsent(undefined)).toBe(false); expect(props.setUserConsent({})).toBe(false); expect(syncManager.submitter.start).toBeCalledTimes(1); expect(syncManager.submitter.stop).toBeCalledTimes(1); - expect(props.getUserConsent()).toBe('declined'); + expect(props.getUserConsent()).toBe('DECLINED'); }); From fb7f3f02837e9aaee0a316f221b48f23c3926bd9 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 24 Feb 2022 18:40:51 -0300 Subject: [PATCH 09/32] validate config.userConsent --- src/logger/constants.ts | 2 +- src/logger/messages/error.ts | 2 +- src/sdkFactory/userConsentProps.ts | 2 +- src/utils/settingsValidation/impressionsMode.ts | 15 +++++++-------- src/utils/settingsValidation/index.ts | 6 +++++- src/utils/settingsValidation/types.ts | 2 ++ src/utils/settingsValidation/userConsent.ts | 14 ++++++++++++++ 7 files changed, 31 insertions(+), 12 deletions(-) create mode 100644 src/utils/settingsValidation/userConsent.ts diff --git a/src/logger/constants.ts b/src/logger/constants.ts index 92428e5b..36002715 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -116,7 +116,7 @@ export const ERROR_INVALID_KEY_OBJECT = 317; export const ERROR_INVALID = 318; export const ERROR_EMPTY = 319; export const ERROR_EMPTY_ARRAY = 320; -export const ERROR_INVALID_IMPRESSIONS_MODE = 321; +export const ERROR_INVALID_CONFIG_PARAM = 321; export const ERROR_HTTP = 322; export const ERROR_LOCALHOST_MODULE_REQUIRED = 323; export const ERROR_STORAGE_INVALID = 324; diff --git a/src/logger/messages/error.ts b/src/logger/messages/error.ts index caa2936d..db97a25a 100644 --- a/src/logger/messages/error.ts +++ b/src/logger/messages/error.ts @@ -30,7 +30,7 @@ export const codesError: [number, string][] = [ [c.ERROR_EMPTY_ARRAY, '%s: %s must be a non-empty array.'], [c.ERROR_NOT_BOOLEAN, '%s: you must provide a boolean param.'], // initialization / settings validation - [c.ERROR_INVALID_IMPRESSIONS_MODE, c.LOG_PREFIX_SETTINGS + ': you passed an invalid "impressionsMode". It should be one of the following values: %s. Defaulting to "%s" mode.'], + [c.ERROR_INVALID_CONFIG_PARAM, c.LOG_PREFIX_SETTINGS + ': you passed an invalid "%s" config param. It must be one of the following values: %s. Defaulting to "%s".'], [c.ERROR_LOCALHOST_MODULE_REQUIRED, c.LOG_PREFIX_SETTINGS + ': an invalid value was received for "sync.localhostMode" config. A valid entity should be provided for localhost mode.'], [c.ERROR_STORAGE_INVALID, c.LOG_PREFIX_SETTINGS+': The provided storage is invalid.%s Fallbacking into default MEMORY storage'], ]; diff --git a/src/sdkFactory/userConsentProps.ts b/src/sdkFactory/userConsentProps.ts index b9498360..027cef57 100644 --- a/src/sdkFactory/userConsentProps.ts +++ b/src/sdkFactory/userConsentProps.ts @@ -23,7 +23,7 @@ export function userConsentProps(settings: ISettings, syncManager?: ISyncManager if (consent) syncManager?.submitter?.start(); else syncManager?.submitter?.stop(); - log.info(USER_CONSENT_UPDATED, [settings.userConsent, newConsentStatus]); + log.info(USER_CONSENT_UPDATED, [settings.userConsent, newConsentStatus]); // @ts-ignore, modify readonly prop settings.userConsent = newConsentStatus; } diff --git a/src/utils/settingsValidation/impressionsMode.ts b/src/utils/settingsValidation/impressionsMode.ts index ddcda23d..891f92b9 100644 --- a/src/utils/settingsValidation/impressionsMode.ts +++ b/src/utils/settingsValidation/impressionsMode.ts @@ -1,14 +1,13 @@ -import { ERROR_INVALID_IMPRESSIONS_MODE } from '../../logger/constants'; +import { ERROR_INVALID_CONFIG_PARAM } from '../../logger/constants'; import { ILogger } from '../../logger/types'; import { SplitIO } from '../../types'; import { DEBUG, OPTIMIZED } from '../constants'; -export function validImpressionsMode(log: ILogger, impressionsMode: string): SplitIO.ImpressionsMode { - impressionsMode = impressionsMode.toUpperCase(); - if ([DEBUG, OPTIMIZED].indexOf(impressionsMode) === -1) { - log.error(ERROR_INVALID_IMPRESSIONS_MODE, [[DEBUG, OPTIMIZED], OPTIMIZED]); - impressionsMode = OPTIMIZED; - } +export function validImpressionsMode(log: ILogger, impressionsMode: any): SplitIO.ImpressionsMode { + if (typeof impressionsMode === 'string') impressionsMode = impressionsMode.toUpperCase(); - return impressionsMode as SplitIO.ImpressionsMode; + if ([DEBUG, OPTIMIZED].indexOf(impressionsMode) > -1) return impressionsMode; + + log.error(ERROR_INVALID_CONFIG_PARAM, ['impressionsMode', [DEBUG, OPTIMIZED], OPTIMIZED]); + return OPTIMIZED; } diff --git a/src/utils/settingsValidation/index.ts b/src/utils/settingsValidation/index.ts index 20bcd1bc..75c0872e 100644 --- a/src/utils/settingsValidation/index.ts +++ b/src/utils/settingsValidation/index.ts @@ -97,7 +97,7 @@ function fromSecondsToMillis(n: number) { */ export function settingsValidation(config: unknown, validationParams: ISettingsValidationParams) { - const { defaults, runtime, storage, integrations, logger, localhost } = validationParams; + const { defaults, runtime, storage, integrations, logger, localhost, userConsent } = validationParams; // creates a settings object merging base, defaults and config objects. const withDefaults = merge({}, base, defaults, config) as ISettings; @@ -161,5 +161,9 @@ export function settingsValidation(config: unknown, validationParams: ISettingsV // ensure a valid impressionsMode withDefaults.sync.impressionsMode = validImpressionsMode(log, withDefaults.sync.impressionsMode); + // ensure a valid user consent value + // @ts-ignore, modify readonly prop + withDefaults.userConsent = userConsent(withDefaults); + return withDefaults; } diff --git a/src/utils/settingsValidation/types.ts b/src/utils/settingsValidation/types.ts index b305d440..e8f954b5 100644 --- a/src/utils/settingsValidation/types.ts +++ b/src/utils/settingsValidation/types.ts @@ -20,4 +20,6 @@ export interface ISettingsValidationParams { logger: (settings: ISettings) => ISettings['log'], /** Localhost mode validator (`settings.sync.localhostMode`) */ localhost?: (settings: ISettings) => ISettings['sync']['localhostMode'], + /** User consent validator (`settings.userConsent`) */ + userConsent: (settings: ISettings) => ISettings['userConsent'], } diff --git a/src/utils/settingsValidation/userConsent.ts b/src/utils/settingsValidation/userConsent.ts new file mode 100644 index 00000000..7f16b351 --- /dev/null +++ b/src/utils/settingsValidation/userConsent.ts @@ -0,0 +1,14 @@ +import { ERROR_INVALID_CONFIG_PARAM } from '../../logger/constants'; +import { ILogger } from '../../logger/types'; +import { CONSENT_DECLINED, CONSENT_GRANTED, CONSENT_UNKNOWN } from '../constants'; + +const userConsentValues = [CONSENT_DECLINED, CONSENT_GRANTED, CONSENT_UNKNOWN]; + +export function validateUserConsent({ userConsent, log }: { userConsent: any, log: ILogger }) { + if (typeof userConsent === 'string') userConsent = userConsent.toUpperCase(); + + if (userConsentValues.indexOf(userConsent) > -1) return userConsent; + + log.error(ERROR_INVALID_CONFIG_PARAM, ['userConsent', userConsentValues, CONSENT_GRANTED]); + return CONSENT_GRANTED; +} From 14105d8a96ad7c273c0ec5cb25b7b365d9eff7f5 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 24 Feb 2022 18:44:54 -0300 Subject: [PATCH 10/32] fixed type check issue --- src/sdkFactory/userConsentProps.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sdkFactory/userConsentProps.ts b/src/sdkFactory/userConsentProps.ts index b9498360..027cef57 100644 --- a/src/sdkFactory/userConsentProps.ts +++ b/src/sdkFactory/userConsentProps.ts @@ -23,7 +23,7 @@ export function userConsentProps(settings: ISettings, syncManager?: ISyncManager if (consent) syncManager?.submitter?.start(); else syncManager?.submitter?.stop(); - log.info(USER_CONSENT_UPDATED, [settings.userConsent, newConsentStatus]); + log.info(USER_CONSENT_UPDATED, [settings.userConsent, newConsentStatus]); // @ts-ignore, modify readonly prop settings.userConsent = newConsentStatus; } From 9e71ee2af4da837cb41f28ff4ee385d038bbb182 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 24 Feb 2022 19:10:52 -0300 Subject: [PATCH 11/32] fixed test --- src/utils/settingsValidation/__tests__/index.spec.ts | 3 ++- src/utils/settingsValidation/{userConsent.ts => consent.ts} | 2 +- src/utils/settingsValidation/index.ts | 4 ++-- src/utils/settingsValidation/types.ts | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) rename src/utils/settingsValidation/{userConsent.ts => consent.ts} (85%) diff --git a/src/utils/settingsValidation/__tests__/index.spec.ts b/src/utils/settingsValidation/__tests__/index.spec.ts index be0087b5..50b960a9 100644 --- a/src/utils/settingsValidation/__tests__/index.spec.ts +++ b/src/utils/settingsValidation/__tests__/index.spec.ts @@ -19,7 +19,8 @@ const minimalSettingsParams = { version: 'javascript-test', }, runtime: () => ({ ip: false, hostname: false } as ISettings['runtime']), - logger: () => (loggerMock as ISettings['log']) + logger: () => (loggerMock as ISettings['log']), + consent: () => undefined }; describe('settingsValidation', () => { diff --git a/src/utils/settingsValidation/userConsent.ts b/src/utils/settingsValidation/consent.ts similarity index 85% rename from src/utils/settingsValidation/userConsent.ts rename to src/utils/settingsValidation/consent.ts index 7f16b351..1cf9233b 100644 --- a/src/utils/settingsValidation/userConsent.ts +++ b/src/utils/settingsValidation/consent.ts @@ -4,7 +4,7 @@ import { CONSENT_DECLINED, CONSENT_GRANTED, CONSENT_UNKNOWN } from '../constants const userConsentValues = [CONSENT_DECLINED, CONSENT_GRANTED, CONSENT_UNKNOWN]; -export function validateUserConsent({ userConsent, log }: { userConsent: any, log: ILogger }) { +export function validateConsent({ userConsent, log }: { userConsent: any, log: ILogger }) { if (typeof userConsent === 'string') userConsent = userConsent.toUpperCase(); if (userConsentValues.indexOf(userConsent) > -1) return userConsent; diff --git a/src/utils/settingsValidation/index.ts b/src/utils/settingsValidation/index.ts index 75c0872e..d7a31c10 100644 --- a/src/utils/settingsValidation/index.ts +++ b/src/utils/settingsValidation/index.ts @@ -97,7 +97,7 @@ function fromSecondsToMillis(n: number) { */ export function settingsValidation(config: unknown, validationParams: ISettingsValidationParams) { - const { defaults, runtime, storage, integrations, logger, localhost, userConsent } = validationParams; + const { defaults, runtime, storage, integrations, logger, localhost, consent } = validationParams; // creates a settings object merging base, defaults and config objects. const withDefaults = merge({}, base, defaults, config) as ISettings; @@ -163,7 +163,7 @@ export function settingsValidation(config: unknown, validationParams: ISettingsV // ensure a valid user consent value // @ts-ignore, modify readonly prop - withDefaults.userConsent = userConsent(withDefaults); + withDefaults.userConsent = consent(withDefaults); return withDefaults; } diff --git a/src/utils/settingsValidation/types.ts b/src/utils/settingsValidation/types.ts index e8f954b5..40cd155c 100644 --- a/src/utils/settingsValidation/types.ts +++ b/src/utils/settingsValidation/types.ts @@ -21,5 +21,5 @@ export interface ISettingsValidationParams { /** Localhost mode validator (`settings.sync.localhostMode`) */ localhost?: (settings: ISettings) => ISettings['sync']['localhostMode'], /** User consent validator (`settings.userConsent`) */ - userConsent: (settings: ISettings) => ISettings['userConsent'], + consent: (settings: ISettings) => ISettings['userConsent'], } From 3dc3638150df006358395c1bc1f5fc38f02484a6 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 25 Feb 2022 19:20:53 -0300 Subject: [PATCH 12/32] localstorage update --- src/storages/KeyBuilderCS.ts | 14 +++++++- .../inLocalStorage/MySegmentsCacheInLocal.ts | 26 +++++++++++++-- .../__tests__/MySegmentsCacheInLocal.spec.ts | 32 +++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/storages/KeyBuilderCS.ts b/src/storages/KeyBuilderCS.ts index d4f9e902..58c19fbb 100644 --- a/src/storages/KeyBuilderCS.ts +++ b/src/storages/KeyBuilderCS.ts @@ -16,10 +16,22 @@ export class KeyBuilderCS extends KeyBuilder { * @override */ buildSegmentNameKey(segmentName: string) { - return `${this.matchingKey}.${this.prefix}.segment.${segmentName}`; + return `${this.prefix}.${this.matchingKey}.segment.${segmentName}`; } extractSegmentName(builtSegmentKeyName: string) { + const prefix = `${this.prefix}.${this.matchingKey}.segment.`; + + if (startsWith(builtSegmentKeyName, prefix)) + return builtSegmentKeyName.substr(prefix.length); + } + + // @BREAKING: The key used to start with the matching key instead of the prefix, this was changed on version 10.17.3 + buildOldSegmentNameKey(segmentName: string) { + return `${this.matchingKey}.${this.prefix}.segment.${segmentName}`; + } + // @BREAKING: The key used to start with the matching key instead of the prefix, this was changed on version 10.17.3 + extractOldSegmentKey(builtSegmentKeyName: string) { const prefix = `${this.matchingKey}.${this.prefix}.segment.`; if (startsWith(builtSegmentKeyName, prefix)) diff --git a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts index 7bc6690a..627d1fcf 100644 --- a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts @@ -67,9 +67,29 @@ export class MySegmentsCacheInLocal extends AbstractSegmentsCacheSync { // Scan current values from localStorage const storedSegmentNames = Object.keys(localStorage).reduce((accum, key) => { - const name = this.keys.extractSegmentName(key); - - if (name) accum.push(name); + let segmentName = this.keys.extractSegmentName(key); + + if (segmentName) { + accum.push(segmentName); + } else { + // @BREAKING: This is only to clean up "old" keys. Remove this whole else code block. + segmentName = this.keys.extractOldSegmentKey(key); + + if (segmentName) { // this was an old segment key, let's clean up. + const newSegmentKey = this.keys.buildSegmentNameKey(segmentName); + try { + // If the new format key is not there, create it. + if (!localStorage.getItem(newSegmentKey) && names.indexOf(segmentName) > -1) { + localStorage.setItem(newSegmentKey, DEFINED); + // we are migrating a segment, let's track it. + accum.push(segmentName); + } + localStorage.removeItem(key); // we migrated the current key, let's delete it. + } catch (e) { + this.log.error(e); + } + } + } return accum; }, [] as string[]); diff --git a/src/storages/inLocalStorage/__tests__/MySegmentsCacheInLocal.spec.ts b/src/storages/inLocalStorage/__tests__/MySegmentsCacheInLocal.spec.ts index 36072550..67c9efcc 100644 --- a/src/storages/inLocalStorage/__tests__/MySegmentsCacheInLocal.spec.ts +++ b/src/storages/inLocalStorage/__tests__/MySegmentsCacheInLocal.spec.ts @@ -17,3 +17,35 @@ test('SEGMENT CACHE / in LocalStorage', () => { expect(cache.isInSegment('mocked-segment') === false).toBe(true); }); + +// @BREAKING: REMOVE when removing this backwards compatibility. +test('SEGMENT CACHE / in LocalStorage migration for mysegments keys', () => { + + const keys = new KeyBuilderCS('LS_BC_test.SPLITIO', 'test_nico'); + const cache = new MySegmentsCacheInLocal(loggerMock, keys); + + const oldKey1 = 'test_nico.LS_BC_test.SPLITIO.segment.segment1'; + const oldKey2 = 'test_nico.LS_BC_test.SPLITIO.segment.segment2'; + const newKey1 = keys.buildSegmentNameKey('segment1'); + const newKey2 = keys.buildSegmentNameKey('segment2'); + + cache.clear(); // cleanup before starting. + + // Not adding a full suite for LS keys now, testing here + expect(oldKey1).toBe(keys.buildOldSegmentNameKey('segment1')); + expect('segment1').toBe(keys.extractOldSegmentKey(oldKey1)); + + // add two segments, one we don't want to send on reset, should only be cleared, other one will be migrated. + localStorage.setItem(oldKey1, '1'); + localStorage.setItem(oldKey2, '1'); + expect(localStorage.getItem(newKey1)).toBe(null); // control assertion + + cache.resetSegments(['segment1']); + + expect(localStorage.getItem(newKey1)).toBe('1'); // The segment key for segment1, as is part of the new list, should be migrated. + expect(localStorage.getItem(newKey2)).toBe(null); // The segment key for segment2 should not be migrated. + expect(localStorage.getItem(oldKey1)).toBe(null); // Old keys are removed. + expect(localStorage.getItem(oldKey2)).toBe(null); // Old keys are removed. + + cache.clear(); +}); From a16b6032517792aeb88fb057c18e8a246fcf96f2 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 2 Mar 2022 13:26:59 -0300 Subject: [PATCH 13/32] prepare rc --- package-lock.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index c8abb07d..ea2edb24 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.3", + "version": "1.2.1-rc.5", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 2bed38b7..719eb3f6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.3", + "version": "1.2.1-rc.5", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", From a4bf0faf04d1d2645eed9193d53723dc375ea94f Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 3 Mar 2022 12:14:44 -0300 Subject: [PATCH 14/32] update validatePrefix function, for consistency with JS SDK --- src/storages/KeyBuilder.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/storages/KeyBuilder.ts b/src/storages/KeyBuilder.ts index 080ed3d9..3c54d403 100644 --- a/src/storages/KeyBuilder.ts +++ b/src/storages/KeyBuilder.ts @@ -1,15 +1,11 @@ -import { endsWith, startsWith } from '../utils/lang'; +import { startsWith } from '../utils/lang'; const everythingAtTheEnd = /[^.]+$/; const DEFAULT_PREFIX = 'SPLITIO'; export function validatePrefix(prefix: unknown) { - return prefix && typeof prefix === 'string' ? - endsWith(prefix, '.' + DEFAULT_PREFIX) ? - prefix : // suffix already appended - prefix + '.' + DEFAULT_PREFIX : // append suffix - DEFAULT_PREFIX; // use default prefix if none is provided + return prefix ? prefix + '.SPLITIO' : 'SPLITIO'; } export class KeyBuilder { From 0e36a90df20e0d3afde09f6b35f8d6913a8bf43c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 3 Mar 2022 16:55:38 -0300 Subject: [PATCH 15/32] feedback and polishing --- src/logger/constants.ts | 1 + src/logger/messages/debug.ts | 6 +++--- src/logger/messages/error.ts | 4 ++-- src/logger/messages/info.ts | 7 ++++--- src/sdkFactory/userConsentProps.ts | 21 +++++++++++-------- src/utils/lang/index.ts | 9 +++++++- src/utils/settingsValidation/consent.ts | 3 ++- .../settingsValidation/impressionsMode.ts | 3 ++- 8 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/logger/constants.ts b/src/logger/constants.ts index 36002715..68259646 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -69,6 +69,7 @@ export const SYNC_STOP_POLLING = 119; export const EVENTS_TRACKER_SUCCESS = 120; export const IMPRESSIONS_TRACKER_SUCCESS = 121; export const USER_CONSENT_UPDATED = 122; +export const USER_CONSENT_NOT_UPDATED = 123; export const ENGINE_VALUE_INVALID = 200; export const ENGINE_VALUE_NO_ATTRIBUTES = 201; diff --git a/src/logger/messages/debug.ts b/src/logger/messages/debug.ts index ed8281ed..c57ea6e0 100644 --- a/src/logger/messages/debug.ts +++ b/src/logger/messages/debug.ts @@ -31,9 +31,9 @@ export const codesDebug: [number, string][] = codesInfo.concat([ // SDK [c.CLEANUP_REGISTERING, c.LOG_PREFIX_CLEANUP + 'Registering cleanup handler %s'], [c.CLEANUP_DEREGISTERING, c.LOG_PREFIX_CLEANUP + 'Deregistering cleanup handler %s'], - [c.RETRIEVE_CLIENT_DEFAULT, ' Retrieving default SDK client.'], - [c.RETRIEVE_CLIENT_EXISTING, ' Retrieving existing SDK client.'], - [c.RETRIEVE_MANAGER, ' Retrieving manager instance.'], + [c.RETRIEVE_CLIENT_DEFAULT, 'Retrieving default SDK client.'], + [c.RETRIEVE_CLIENT_EXISTING, 'Retrieving existing SDK client.'], + [c.RETRIEVE_MANAGER, 'Retrieving manager instance.'], // synchronizer [c.SYNC_OFFLINE_DATA, c.LOG_PREFIX_SYNC_OFFLINE + 'Splits data: \n%s'], [c.SYNC_SPLITS_FETCH, c.LOG_PREFIX_SYNC_SPLITS + 'Spin up split update using since = %s'], diff --git a/src/logger/messages/error.ts b/src/logger/messages/error.ts index 555bd1d6..8ef9065b 100644 --- a/src/logger/messages/error.ts +++ b/src/logger/messages/error.ts @@ -5,7 +5,7 @@ export const codesError: [number, string][] = [ [c.ERROR_ENGINE_COMBINER_IFELSEIF, c.LOG_PREFIX_ENGINE_COMBINER + 'Invalid Split, no valid rules found'], // SDK [c.ERROR_LOGLEVEL_INVALID, 'logger: Invalid Log Level - No changes to the logs will be applied.'], - [c.ERROR_CLIENT_CANNOT_GET_READY, ' The SDK will not get ready. Reason: %s'], + [c.ERROR_CLIENT_CANNOT_GET_READY, 'The SDK will not get ready. Reason: %s'], [c.ERROR_IMPRESSIONS_TRACKER, c.LOG_PREFIX_IMPRESSIONS_TRACKER + 'Could not store impressions bulk with %s impression(s). Error: %s'], [c.ERROR_IMPRESSIONS_LISTENER, c.LOG_PREFIX_IMPRESSIONS_TRACKER + 'Impression listener logImpression method threw: %s.'], [c.ERROR_EVENTS_TRACKER, c.LOG_PREFIX_EVENTS_TRACKER + 'Failed to queue %s'], @@ -28,7 +28,7 @@ export const codesError: [number, string][] = [ [c.ERROR_INVALID, '%s: you passed an invalid %s. It must be a non-empty string.'], [c.ERROR_EMPTY, '%s: you passed an empty %s. It must be a non-empty string.'], [c.ERROR_EMPTY_ARRAY, '%s: %s must be a non-empty array.'], - [c.ERROR_NOT_BOOLEAN, '%s: you must provide a boolean param.'], + [c.ERROR_NOT_BOOLEAN, '%s: provided param must be a boolean value.'], // initialization / settings validation [c.ERROR_INVALID_CONFIG_PARAM, c.LOG_PREFIX_SETTINGS + ': you passed an invalid "%s" config param. It should be one of the following values: %s. Defaulting to "%s".'], [c.ERROR_LOCALHOST_MODULE_REQUIRED, c.LOG_PREFIX_SETTINGS + ': an invalid value was received for "sync.localhostMode" config. A valid entity should be provided for localhost mode.'], diff --git a/src/logger/messages/info.ts b/src/logger/messages/info.ts index 4facaf24..9f4a4254 100644 --- a/src/logger/messages/info.ts +++ b/src/logger/messages/info.ts @@ -10,11 +10,12 @@ export const codesInfo: [number, string][] = codesWarn.concat([ // SDK [c.IMPRESSION, c.LOG_PREFIX_IMPRESSIONS_TRACKER +'Split: %s. Key: %s. Evaluation: %s. Label: %s'], [c.IMPRESSION_QUEUEING, c.LOG_PREFIX_IMPRESSIONS_TRACKER +'Queueing corresponding impression.'], - [c.NEW_SHARED_CLIENT, ' New shared client instance created.'], - [c.NEW_FACTORY, ' New Split SDK instance created.'], + [c.NEW_SHARED_CLIENT, 'New shared client instance created.'], + [c.NEW_FACTORY, 'New Split SDK instance created.'], [c.EVENTS_TRACKER_SUCCESS, c.LOG_PREFIX_EVENTS_TRACKER + 'Successfully queued %s'], [c.IMPRESSIONS_TRACKER_SUCCESS, c.LOG_PREFIX_IMPRESSIONS_TRACKER + 'Successfully stored %s impression(s).'], - [c.USER_CONSENT_UPDATED, 'User consent changed from %s to %s.'], + [c.USER_CONSENT_UPDATED, 'setUserConsent: consent status changed from %s to %s.'], + [c.USER_CONSENT_NOT_UPDATED, 'setUserConsent: call had no effect because it was the current consent status (%s).'], // synchronizer [c.POLLING_SMART_PAUSING, c.LOG_PREFIX_SYNC_POLLING + 'Turning segments data polling %s.'], diff --git a/src/sdkFactory/userConsentProps.ts b/src/sdkFactory/userConsentProps.ts index 027cef57..da715c33 100644 --- a/src/sdkFactory/userConsentProps.ts +++ b/src/sdkFactory/userConsentProps.ts @@ -1,7 +1,8 @@ -import { ERROR_NOT_BOOLEAN, USER_CONSENT_UPDATED } from '../logger/constants'; +import { ERROR_NOT_BOOLEAN, USER_CONSENT_UPDATED, USER_CONSENT_NOT_UPDATED } from '../logger/constants'; import { ISyncManager } from '../sync/types'; import { ISettings } from '../types'; import { CONSENT_GRANTED, CONSENT_DECLINED } from '../utils/constants'; +import { isBoolean } from '../utils/lang'; // Extend client-side factory instances with user consent getter/setter export function userConsentProps(settings: ISettings, syncManager?: ISyncManager) { @@ -11,20 +12,22 @@ export function userConsentProps(settings: ISettings, syncManager?: ISyncManager return { setUserConsent(consent: unknown) { // validate input param - if (typeof consent !== 'boolean') { - log.error(ERROR_NOT_BOOLEAN, ['setUserConsent']); + if (!isBoolean(consent)) { + log.warn(ERROR_NOT_BOOLEAN, ['setUserConsent']); return false; } const newConsentStatus = consent ? CONSENT_GRANTED : CONSENT_DECLINED; - if (settings.userConsent !== newConsentStatus) { - // resume/pause submitters - if (consent) syncManager?.submitter?.start(); - else syncManager?.submitter?.stop(); - - log.info(USER_CONSENT_UPDATED, [settings.userConsent, newConsentStatus]); // @ts-ignore, modify readonly prop + if (settings.userConsent !== newConsentStatus) { // @ts-ignore, modify readonly prop settings.userConsent = newConsentStatus; + + if (consent) syncManager?.submitter?.start(); // resumes submitters if transitioning to GRANTED + else syncManager?.submitter?.stop(); // pauses submitters if transitioning to DECLINED + + log.info(USER_CONSENT_UPDATED, [settings.userConsent, newConsentStatus]); + } else { + log.info(USER_CONSENT_NOT_UPDATED, [newConsentStatus]); } return true; diff --git a/src/utils/lang/index.ts b/src/utils/lang/index.ts index f894f652..2a7332ff 100644 --- a/src/utils/lang/index.ts +++ b/src/utils/lang/index.ts @@ -89,7 +89,7 @@ export function get(obj: any, prop: any, val: any): any { /** * Parses an array into a map of different arrays, grouping by the specified prop value. */ -export function groupBy >(source: T[], prop: string): Record { +export function groupBy>(source: T[], prop: string): Record { const map: Record = {}; if (Array.isArray(source) && isString(prop)) { @@ -164,6 +164,13 @@ export function isString(val: any): val is string { return typeof val === 'string' || val instanceof String; } +/** + * String sanitizer. Returns the provided value converted to uppercase if it is a string. + */ +export function stringToUpperCase(val: any) { + return isString(val) ? val.toUpperCase() : val; +} + /** * Deep copy version of Object.assign using recursion. * There are some assumptions here. It's for internal use and we don't need verbose errors diff --git a/src/utils/settingsValidation/consent.ts b/src/utils/settingsValidation/consent.ts index 1cf9233b..c68cf222 100644 --- a/src/utils/settingsValidation/consent.ts +++ b/src/utils/settingsValidation/consent.ts @@ -1,11 +1,12 @@ import { ERROR_INVALID_CONFIG_PARAM } from '../../logger/constants'; import { ILogger } from '../../logger/types'; import { CONSENT_DECLINED, CONSENT_GRANTED, CONSENT_UNKNOWN } from '../constants'; +import { stringToUpperCase } from '../lang'; const userConsentValues = [CONSENT_DECLINED, CONSENT_GRANTED, CONSENT_UNKNOWN]; export function validateConsent({ userConsent, log }: { userConsent: any, log: ILogger }) { - if (typeof userConsent === 'string') userConsent = userConsent.toUpperCase(); + userConsent = stringToUpperCase(userConsent); if (userConsentValues.indexOf(userConsent) > -1) return userConsent; diff --git a/src/utils/settingsValidation/impressionsMode.ts b/src/utils/settingsValidation/impressionsMode.ts index 891f92b9..9fdc3e09 100644 --- a/src/utils/settingsValidation/impressionsMode.ts +++ b/src/utils/settingsValidation/impressionsMode.ts @@ -2,9 +2,10 @@ import { ERROR_INVALID_CONFIG_PARAM } from '../../logger/constants'; import { ILogger } from '../../logger/types'; import { SplitIO } from '../../types'; import { DEBUG, OPTIMIZED } from '../constants'; +import { stringToUpperCase } from '../lang'; export function validImpressionsMode(log: ILogger, impressionsMode: any): SplitIO.ImpressionsMode { - if (typeof impressionsMode === 'string') impressionsMode = impressionsMode.toUpperCase(); + impressionsMode = stringToUpperCase(impressionsMode); if ([DEBUG, OPTIMIZED].indexOf(impressionsMode) > -1) return impressionsMode; From 002912f2678c57490a1e100e5511f1157d94a5de Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 4 Mar 2022 16:26:23 -0300 Subject: [PATCH 16/32] fixed issue with event submitter push window --- package-lock.json | 2 +- package.json | 2 +- .../__tests__/eventsSyncTask.spec.ts | 61 +++++++++++++++++++ src/sync/submitters/eventsSyncTask.ts | 9 ++- 4 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 src/sync/submitters/__tests__/eventsSyncTask.spec.ts diff --git a/package-lock.json b/package-lock.json index ea2edb24..9c0208f7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.5", + "version": "1.2.1-rc.6", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 719eb3f6..3bef4bea 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.5", + "version": "1.2.1-rc.6", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/sync/submitters/__tests__/eventsSyncTask.spec.ts b/src/sync/submitters/__tests__/eventsSyncTask.spec.ts new file mode 100644 index 00000000..59b647c1 --- /dev/null +++ b/src/sync/submitters/__tests__/eventsSyncTask.spec.ts @@ -0,0 +1,61 @@ +import { eventsSyncTaskFactory } from '../eventsSyncTask'; +import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; + + + +describe('Events submitter (eventsSyncTask)', () => { + + let __onFullQueueCb: () => void; + const postEventsBulkMock = jest.fn(); + const eventsCacheMock = { + isEmpty: jest.fn(() => true), + setOnFullQueueCb: jest.fn(function (onFullQueueCb) { __onFullQueueCb = onFullQueueCb; }) + }; + + beforeEach(() => { + eventsCacheMock.isEmpty.mockClear(); + }); + + test('with eventsFirstPushWindow', async () => { + const eventsFirstPushWindow = 20; // @ts-ignore + const eventsSubmitter = eventsSyncTaskFactory(loggerMock, postEventsBulkMock, eventsCacheMock, 30000, eventsFirstPushWindow); + + eventsSubmitter.start(); + expect(eventsSubmitter.isRunning()).toEqual(true); // Submitter should be flagged as running + expect(eventsSubmitter.isExecuting()).toEqual(false); // but not executed immediatelly if there is a push window + expect(eventsCacheMock.isEmpty).not.toBeCalled(); + + // If queue is full, submitter should be executed + __onFullQueueCb(); + expect(eventsSubmitter.isExecuting()).toEqual(true); + expect(eventsCacheMock.isEmpty).toBeCalledTimes(1); + + // Await first push window + await new Promise(res => setTimeout(res, eventsFirstPushWindow + 10)); + expect(eventsCacheMock.isEmpty).toBeCalledTimes(2); // after the push window, submitter should have been executed + + expect(eventsSubmitter.isRunning()).toEqual(true); + eventsSubmitter.stop(); + expect(eventsSubmitter.isRunning()).toEqual(false); + }); + + test('without eventsFirstPushWindow', async () => { + // @ts-ignore + const eventsSubmitter = eventsSyncTaskFactory(loggerMock, postEventsBulkMock, eventsCacheMock, 30000); + + eventsSubmitter.start(); + expect(eventsSubmitter.isRunning()).toEqual(true); // Submitter should be flagged as running + expect(eventsSubmitter.isExecuting()).toEqual(true); // and executes immediatelly if there isn't a push window + expect(eventsCacheMock.isEmpty).toBeCalledTimes(1); + + // If queue is full, submitter should be executed + __onFullQueueCb(); + expect(eventsSubmitter.isExecuting()).toEqual(true); + expect(eventsCacheMock.isEmpty).toBeCalledTimes(2); + + expect(eventsSubmitter.isRunning()).toEqual(true); + eventsSubmitter.stop(); + expect(eventsSubmitter.isRunning()).toEqual(false); + }); + +}); diff --git a/src/sync/submitters/eventsSyncTask.ts b/src/sync/submitters/eventsSyncTask.ts index b87ee24e..7c84374b 100644 --- a/src/sync/submitters/eventsSyncTask.ts +++ b/src/sync/submitters/eventsSyncTask.ts @@ -22,18 +22,25 @@ export function eventsSyncTaskFactory( // don't retry events. const syncTask = submitterSyncTaskFactory(log, postEventsBulk, eventsCache, eventsPushRate, DATA_NAME, latencyTracker); - // Set a timer for the first push of events, + // Set a timer for the first push window of events. + // Not implemented in the base submitter or sync task, since this feature is only used by the events submitter. if (eventsFirstPushWindow > 0) { + let running = false; let stopEventPublisherTimeout: ReturnType; const originalStart = syncTask.start; syncTask.start = () => { + running = true; stopEventPublisherTimeout = setTimeout(originalStart, eventsFirstPushWindow); }; const originalStop = syncTask.stop; syncTask.stop = () => { + running = false; clearTimeout(stopEventPublisherTimeout); originalStop(); }; + syncTask.isRunning = () => { + return running; + }; } // register events submitter to be executed when events cache is full From 8255eaeab8075a606d264318b7449aff7116f8de Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 8 Mar 2022 12:08:38 -0300 Subject: [PATCH 17/32] validate undefined authorizationKey in SSEClient --- package-lock.json | 2 +- package.json | 2 +- src/sync/streaming/SSEClient/index.ts | 3 ++- src/utils/settingsValidation/consent.ts | 3 ++- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9c0208f7..36876610 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.6", + "version": "1.2.1-rc.7", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 3bef4bea..03f9ec7a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.6", + "version": "1.2.1-rc.7", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/sync/streaming/SSEClient/index.ts b/src/sync/streaming/SSEClient/index.ts index 378ac9c0..7c0dfb6e 100644 --- a/src/sync/streaming/SSEClient/index.ts +++ b/src/sync/streaming/SSEClient/index.ts @@ -1,5 +1,6 @@ import { IEventSourceConstructor } from '../../../services/types'; import { ISettings } from '../../../types'; +import { isString } from '../../../utils/lang'; import { IAuthTokenPushEnabled } from '../AuthClient/types'; import { ISSEClient, ISseEventHandler } from './types'; @@ -15,7 +16,7 @@ const CONTROL_CHANNEL_REGEX = /^control_/; */ function buildSSEHeaders(settings: ISettings) { const headers: Record = { - SplitSDKClientKey: settings.core.authorizationKey.slice(-4), + SplitSDKClientKey: isString(settings.core.authorizationKey) ? settings.core.authorizationKey.slice(-4) : '', SplitSDKVersion: settings.version, }; diff --git a/src/utils/settingsValidation/consent.ts b/src/utils/settingsValidation/consent.ts index c68cf222..98b4112b 100644 --- a/src/utils/settingsValidation/consent.ts +++ b/src/utils/settingsValidation/consent.ts @@ -1,11 +1,12 @@ import { ERROR_INVALID_CONFIG_PARAM } from '../../logger/constants'; import { ILogger } from '../../logger/types'; +import { ConsentStatus } from '../../types'; import { CONSENT_DECLINED, CONSENT_GRANTED, CONSENT_UNKNOWN } from '../constants'; import { stringToUpperCase } from '../lang'; const userConsentValues = [CONSENT_DECLINED, CONSENT_GRANTED, CONSENT_UNKNOWN]; -export function validateConsent({ userConsent, log }: { userConsent: any, log: ILogger }) { +export function validateConsent({ userConsent, log }: { userConsent?: any, log: ILogger }): ConsentStatus { userConsent = stringToUpperCase(userConsent); if (userConsentValues.indexOf(userConsent) > -1) return userConsent; From 9ab79d2b2dc478787df0c38cea11c4742771cc3c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 8 Mar 2022 17:04:03 -0300 Subject: [PATCH 18/32] 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 19/32] 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 20/32] 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 21/32] 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 })); } }; From 7620e08f87e4a37d155ba532de7d13eb0049148d Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 15 Mar 2022 16:23:08 -0300 Subject: [PATCH 22/32] in-transit encryption support added --- src/storages/inRedis/RedisAdapter.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/storages/inRedis/RedisAdapter.ts b/src/storages/inRedis/RedisAdapter.ts index 05ef94f7..c07d9bb0 100644 --- a/src/storages/inRedis/RedisAdapter.ts +++ b/src/storages/inRedis/RedisAdapter.ts @@ -164,6 +164,9 @@ export class RedisAdapter extends ioredis { } else { // If it IS the string URL, that'll be the first param for ioredis. result.unshift(options.url); } + if (options.tls) { + merge(opts, { tls: options.tls }); + } return result; } @@ -171,9 +174,9 @@ export class RedisAdapter extends ioredis { /** * Parses the options into what we care about. */ - static _defineOptions({ connectionTimeout, operationTimeout, url, host, port, db, pass }: Record) { + static _defineOptions({ connectionTimeout, operationTimeout, url, host, port, db, pass, tls }: Record) { const parsedOptions = { - connectionTimeout, operationTimeout, url, host, port, db, pass + connectionTimeout, operationTimeout, url, host, port, db, pass, tls }; return merge({}, DEFAULT_OPTIONS, parsedOptions); From 05637d0224e48d24b043b92f41bd0cf5ed2760af Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 17 Mar 2022 16:45:41 -0300 Subject: [PATCH 23/32] forwarding connectionTimeout to ioredis connectTimeout --- src/storages/inRedis/RedisAdapter.ts | 3 +++ .../inRedis/__tests__/RedisAdapter.spec.ts | 17 ++++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/storages/inRedis/RedisAdapter.ts b/src/storages/inRedis/RedisAdapter.ts index c07d9bb0..0c374f23 100644 --- a/src/storages/inRedis/RedisAdapter.ts +++ b/src/storages/inRedis/RedisAdapter.ts @@ -164,6 +164,9 @@ export class RedisAdapter extends ioredis { } else { // If it IS the string URL, that'll be the first param for ioredis. result.unshift(options.url); } + if (options.connectionTimeout) { + merge(opts, { connectTimeout: options.connectionTimeout }); + } if (options.tls) { merge(opts, { tls: options.tls }); } diff --git a/src/storages/inRedis/__tests__/RedisAdapter.spec.ts b/src/storages/inRedis/__tests__/RedisAdapter.spec.ts index c9e789f6..1e9fd512 100644 --- a/src/storages/inRedis/__tests__/RedisAdapter.spec.ts +++ b/src/storages/inRedis/__tests__/RedisAdapter.spec.ts @@ -93,33 +93,36 @@ describe('STORAGE Redis Adapter', () => { new RedisAdapter(loggerMock, { url: redisUrl, connectionTimeout: 123, - operationTimeout: 124 + operationTimeout: 124, + tls: { ca: 'ca' } }); // Keep in mind we're storing the arguments object, not a true array. expect(constructorParams.length).toBe(2); // In this signature, the constructor receives two params. expect(constructorParams[0]).toBe(redisUrl); // When we use the Redis URL, that should be the first parameter passed to ioredis constructor - expect(constructorParams[1]).toEqual({ enableOfflineQueue: false, connectTimeout: 10000, lazyConnect: false }); // and the second parameter would be the default settings for the lib. + expect(constructorParams[1]).toEqual({ enableOfflineQueue: false, connectTimeout: 123, lazyConnect: false, tls: { ca: 'ca' } }); // and the second parameter would be the default settings for the lib. new RedisAdapter(loggerMock, { ...redisParams, connectionTimeout: 123, - operationTimeout: 124 + operationTimeout: 124, + tls: { ca: 'ca' } }); expect(constructorParams.length).toBe(1); // In this signature, the constructor receives one param. // we keep "pass" instead of "password" on our settings API to be backwards compatible. - expect(constructorParams[0]).toEqual({ host: redisParams.host, port: redisParams.port, db: redisParams.db, password: redisParams.pass, enableOfflineQueue: false, connectTimeout: 10000, lazyConnect: false }); // If we send all the redis params separate, it will pass one object to the library containing that and the rest of the options. + expect(constructorParams[0]).toEqual({ host: redisParams.host, port: redisParams.port, db: redisParams.db, password: redisParams.pass, enableOfflineQueue: false, connectTimeout: 123, lazyConnect: false, tls: { ca: 'ca' } }); // If we send all the redis params separate, it will pass one object to the library containing that and the rest of the options. new RedisAdapter(loggerMock, { ...redisParams, url: redisUrl, - connectionTimeout: 123, - operationTimeout: 124 + // Using default connectionTimeout + operationTimeout: 124, + tls: { ca: 'ca' } }); expect(constructorParams.length).toBe(2); // In this signature, the constructor receives two params. expect(constructorParams[0]).toBe(redisUrl); // When we use the Redis URL, even if we specified all the other params one by one the URL takes precedence, so that should be the first parameter passed to ioredis constructor - expect(constructorParams[1]).toEqual({ enableOfflineQueue: false, connectTimeout: 10000, lazyConnect: false }); // and the second parameter would be the default settings for the lib. + expect(constructorParams[1]).toEqual({ enableOfflineQueue: false, connectTimeout: 10000, lazyConnect: false, tls: { ca: 'ca' } }); // and the second parameter would be the default settings for the lib. }); test('static method - _defineOptions', () => { From e1e6698e83128b86924affae97b7e3ab78dd4421 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 25 Mar 2022 19:06:33 -0300 Subject: [PATCH 24/32] implementation and tests --- package-lock.json | 8 ++-- package.json | 2 +- .../__tests__/sdkClientMethodCS.spec.ts | 25 ------------- src/sdkClient/sdkClientMethodCS.ts | 7 +--- src/sdkClient/sdkClientMethodCSWithTT.ts | 13 +------ .../__tests__/index.spec.ts | 37 +++++++++++++++++++ src/utils/settingsValidation/index.ts | 24 ++++++++++-- src/utils/settingsValidation/types.ts | 4 +- 8 files changed, 68 insertions(+), 52 deletions(-) diff --git a/package-lock.json b/package-lock.json index 0a014819..46d193a6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.8", + "version": "1.2.1-rc.9", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -4984,9 +4984,9 @@ } }, "minimist": { - "version": "1.2.5", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.5.tgz", - "integrity": "sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw==", + "version": "1.2.6", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.6.tgz", + "integrity": "sha512-Jsjnk4bw3YJqYzbdyBiNsPWHPfO++UGG749Cxs6peCu5Xg4nrena6OVxOYxrQTqww0Jmwt+Ref8rggumkTLz9Q==", "dev": true }, "ms": { diff --git a/package.json b/package.json index 63b45dc7..609120dc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.8", + "version": "1.2.1-rc.9", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts b/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts index b5b6b21e..4e445d6d 100644 --- a/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts +++ b/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts @@ -220,31 +220,6 @@ describe('sdkClientMethodCSFactory', () => { if (!ignoresTT) expect(() => sdkClientMethod('valid-key', ['invalid-TT'])).toThrow('Shared Client needs a valid traffic type or no traffic type at all.'); }); - test.each(testTargets)('invalid key/TT binds a false key/TT in the default client', (sdkClientMethodCSFactory, ignoresTT) => { - const paramsWithInvalidKeyAndTT = { - ...params, - settings: { - ...params.settings, - core: { - key: true, // invalid key - trafficType: '' // invalid TT - } - } - }; - - (clientCSDecoratorSpy as jest.Mock).mockClear(); - // @ts-expect-error - const sdkClientMethod = sdkClientMethodCSFactory(paramsWithInvalidKeyAndTT); - - // calling the function should return a client instance - const client = sdkClientMethod(); - assertClientApi(client, params.sdkReadinessManager.sdkStatus); - - // but with false as binded key and TT - if (ignoresTT) expect(clientCSDecoratorSpy).toHaveBeenCalledWith(expect.anything(), expect.anything(), false); - else expect(clientCSDecoratorSpy).toHaveBeenCalledWith(expect.anything(), expect.anything(), false, false); - }); - test.each(testTargets)('attributes binding - main client', (sdkClientMethodCSFactory) => { // @ts-expect-error const sdkClientMethod = sdkClientMethodCSFactory(params); diff --git a/src/sdkClient/sdkClientMethodCS.ts b/src/sdkClient/sdkClientMethodCS.ts index 2abd7a20..32f6f3fb 100644 --- a/src/sdkClient/sdkClientMethodCS.ts +++ b/src/sdkClient/sdkClientMethodCS.ts @@ -23,15 +23,10 @@ const method = 'Client instantiation'; export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key?: SplitIO.SplitKey) => SplitIO.ICsClient { const { storage, syncManager, sdkReadinessManager, settings: { core: { key }, startup: { readyTimeout }, log } } = params; - // Keeping similar behaviour as in the isomorphic JS SDK: if settings key is invalid, - // `false` value is used as binded key of the default client, but trafficType is ignored - // @TODO handle as a non-recoverable error - const validKey = validateKey(log, key, method); - const mainClientInstance = clientCSDecorator( log, sdkClientFactory(params) as SplitIO.IClient, // @ts-ignore - validKey + key ); const parsedDefaultKey = keyParser(key); diff --git a/src/sdkClient/sdkClientMethodCSWithTT.ts b/src/sdkClient/sdkClientMethodCSWithTT.ts index 7c5427e2..22a49ac6 100644 --- a/src/sdkClient/sdkClientMethodCSWithTT.ts +++ b/src/sdkClient/sdkClientMethodCSWithTT.ts @@ -25,20 +25,11 @@ const method = 'Client instantiation'; export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key?: SplitIO.SplitKey, trafficType?: string) => SplitIO.ICsClient { const { storage, syncManager, sdkReadinessManager, settings: { core: { key, trafficType }, startup: { readyTimeout }, log } } = params; - // Keeping the behaviour as in the isomorphic JS SDK: if settings key or TT are invalid, - // `false` value is used as binded key/TT of the default client, which leads to several issues. - // @TODO update when supporting non-recoverable errors - const validKey = validateKey(log, key, method); - let validTrafficType; - if (trafficType !== undefined) { - validTrafficType = validateTrafficType(log, trafficType, method); - } - const mainClientInstance = clientCSDecorator( log, sdkClientFactory(params) as SplitIO.IClient, // @ts-ignore - validKey, - validTrafficType + key, + trafficType ); const parsedDefaultKey = keyParser(key); diff --git a/src/utils/settingsValidation/__tests__/index.spec.ts b/src/utils/settingsValidation/__tests__/index.spec.ts index 50b960a9..7f110c86 100644 --- a/src/utils/settingsValidation/__tests__/index.spec.ts +++ b/src/utils/settingsValidation/__tests__/index.spec.ts @@ -189,6 +189,43 @@ describe('settingsValidation', () => { expect(settings.integrations).toBe(integrationsValidatorResult); expect(integrationsValidatorMock).toBeCalledWith(settings); }); + + test('validates and sanitizes key and traffic type in client-side', () => { + const clientSideValidationParams = { ...minimalSettingsParams, isClientSide: true }; + + const samples = [{ + key: ' valid-key ', settingsKey: 'valid-key', // key string is trimmed + trafficType: 'VALID-TT', settingsTrafficType: 'valid-tt', // TT is converted to lowercase + }, { + key: undefined, settingsKey: false, // undefined key is not valid in client-side + trafficType: undefined, settingsTrafficType: undefined, + }, { + key: null, settingsKey: false, + trafficType: null, settingsTrafficType: false, + }, { + key: true, settingsKey: false, + trafficType: true, settingsTrafficType: false, + }, { + key: 1.5, settingsKey: '1.5', // finite number as key is parsed + trafficType: 100, settingsTrafficType: false, + }, { + key: { matchingKey: 100, bucketingKey: ' BUCK ' }, settingsKey: { matchingKey: '100', bucketingKey: 'BUCK' }, + trafficType: {}, settingsTrafficType: false, + }]; + + samples.forEach(({ key, trafficType, settingsKey, settingsTrafficType }) => { + const settings = settingsValidation({ + core: { + authorizationKey: 'dummy token', + key, + trafficType + } + }, clientSideValidationParams); + + expect(settings.core.key).toEqual(settingsKey); + expect(settings.core.trafficType).toEqual(settingsTrafficType); + }); + }); }); test('SETTINGS / urls should be correctly assigned', () => { diff --git a/src/utils/settingsValidation/index.ts b/src/utils/settingsValidation/index.ts index d7a31c10..c4de8fbc 100644 --- a/src/utils/settingsValidation/index.ts +++ b/src/utils/settingsValidation/index.ts @@ -5,6 +5,8 @@ import { STANDALONE_MODE, OPTIMIZED, LOCALHOST_MODE } from '../constants'; import { validImpressionsMode } from './impressionsMode'; import { ISettingsValidationParams } from './types'; import { ISettings } from '../../types'; +import { validateKey } from '../inputValidation/key'; +import { validateTrafficType } from '../inputValidation/trafficType'; const base = { // Define which kind of object you want to retrieve from SplitFactory @@ -97,7 +99,7 @@ function fromSecondsToMillis(n: number) { */ export function settingsValidation(config: unknown, validationParams: ISettingsValidationParams) { - const { defaults, runtime, storage, integrations, logger, localhost, consent } = validationParams; + const { defaults, isClientSide, runtime, storage, integrations, logger, localhost, consent } = validationParams; // creates a settings object merging base, defaults and config objects. const withDefaults = merge({}, base, defaults, config) as ISettings; @@ -129,9 +131,23 @@ export function settingsValidation(config: unknown, validationParams: ISettingsV // @ts-ignore, modify readonly prop if (storage) withDefaults.storage = storage(withDefaults); - // Although `key` is mandatory according to TS declaration files, it can be omitted in LOCALHOST mode. In that case, the value `localhost_key` is used. - if (withDefaults.mode === LOCALHOST_MODE && withDefaults.core.key === undefined) { - withDefaults.core.key = 'localhost_key'; + // In client-side, validate key and TT + if (isClientSide) { + const maybeKey = withDefaults.core.key; + // Although `key` is required in client-side, it can be omitted in LOCALHOST mode. In that case, the value `localhost_key` is used. + if (withDefaults.mode === LOCALHOST_MODE && maybeKey === undefined) { + withDefaults.core.key = 'localhost_key'; + } else { + // Keeping same behaviour than JS SDK: if settings key or TT are invalid, + // `false` value is used as binded key/TT of the default client, which leads to some issues. + // @ts-ignore, @TODO handle invalid keys as a non-recoverable error? + withDefaults.core.key = validateKey(log, maybeKey, 'Client instantiation'); + } + + const maybeTT = withDefaults.core.trafficType; + if (maybeTT !== undefined) { // @ts-ignore, assigning false + withDefaults.core.trafficType = validateTrafficType(log, maybeTT, 'Client instantiation'); + } } // Current ip/hostname information diff --git a/src/utils/settingsValidation/types.ts b/src/utils/settingsValidation/types.ts index 40cd155c..f6ead8e4 100644 --- a/src/utils/settingsValidation/types.ts +++ b/src/utils/settingsValidation/types.ts @@ -10,7 +10,9 @@ export interface ISettingsValidationParams { * Version and startup properties are required, because they are not defined in the base settings. */ defaults: Partial & { version: string } & { startup: ISettings['startup'] }, - /** Function to define runtime values (`settings.runtime`) */ + /** If true, validates core.key and core.trafficType */ + isClientSide?: boolean, + /** Define runtime values (`settings.runtime`) */ runtime: (settings: ISettings) => ISettings['runtime'], /** Storage validator (`settings.storage`) */ storage?: (settings: ISettings) => ISettings['storage'], From c5aee70c4a58512583587d520519854648fb321e Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 25 Mar 2022 19:13:08 -0300 Subject: [PATCH 25/32] fixed type issue --- src/sdkClient/__tests__/sdkClientMethodCS.spec.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts b/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts index 4e445d6d..7eabeb7c 100644 --- a/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts +++ b/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts @@ -2,10 +2,6 @@ import { sdkClientMethodCSFactory as sdkClientMethodCSWithTTFactory } from '../s import { sdkClientMethodCSFactory } from '../sdkClientMethodCS'; import { assertClientApi } from './testUtils'; -/** Mocks */ -import * as clientCS from '../clientCS'; -const clientCSDecoratorSpy = jest.spyOn(clientCS, 'clientCSDecorator'); - import { settingsWithKey, settingsWithKeyAndTT, settingsWithKeyObject } from '../../utils/settingsValidation/__tests__/settings.mocks'; const partialStorages: { destroy: jest.Mock }[] = []; From 4621c01afac2f425a6a699919f22fb222786f9cc Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 28 Mar 2022 16:17:18 -0300 Subject: [PATCH 26/32] updated isObject util function --- src/utils/lang/__tests__/index.spec.ts | 34 +++++++++++++------------- src/utils/lang/index.ts | 30 ++++++++++++++++++++--- 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/src/utils/lang/__tests__/index.spec.ts b/src/utils/lang/__tests__/index.spec.ts index e6e724a1..145d4fed 100644 --- a/src/utils/lang/__tests__/index.spec.ts +++ b/src/utils/lang/__tests__/index.spec.ts @@ -231,26 +231,26 @@ test('LANG UTILS / isIntegerNumber', () => { test('LANG UTILS / isObject', () => { // positive - expect(isObject({})).toBe(true); // Should return true for map objects. - expect(isObject({ a: true })).toBe(true); // Should return true for map objects. - expect(isObject(new Object())).toBe(true); // Should return true for map objects. - expect(isObject(Object.create({}))).toBe(true); // Should return true for map objects. - expect(isObject(Object.create(Object.prototype))).toBe(true); // Should return true for map objects. + expect(isObject({})).toBe(true); // Should return true for plain objects (objects created by the Object built-in constructor). + expect(isObject({ a: true })).toBe(true); // Should return true for plain objects. + expect(isObject(new Object())).toBe(true); // Should return true for plain objects. + expect(isObject(Object.create({}))).toBe(true); // Should return true for plain objects. + expect(isObject(Object.create(Object.prototype))).toBe(true); // Should return true for plain objects. // negative - expect(isObject([])).toBe(false); // Should return false for anything that is not a map object. - expect(isObject(() => { })).toBe(false); // Should return false for anything that is not a map object. - expect(isObject(true)).toBe(false); // Should return false for anything that is not a map object. - expect(isObject(false)).toBe(false); // Should return false for anything that is not a map object. - expect(isObject(null)).toBe(false); // Should return false for anything that is not a map object. - expect(isObject(undefined)).toBe(false); // Should return false for anything that is not a map object. - expect(isObject(1)).toBe(false); // Should return false for anything that is not a map object. - expect(isObject('asd')).toBe(false); // Should return false for anything that is not a map object. - expect(isObject(function () { })).toBe(false); // Should return false for anything that is not a map object. - expect(isObject(Symbol('test'))).toBe(false); // Should return false for anything that is not a map object. - expect(isObject(new Promise(res => res()))).toBe(false); // Should return false for anything that is not a map object. + expect(isObject([])).toBe(false); // Should return false for anything that is not a plain object. + expect(isObject(() => { })).toBe(false); // Should return false for anything that is not a plain object. + expect(isObject(true)).toBe(false); // Should return false for anything that is not a plain object. + expect(isObject(false)).toBe(false); // Should return false for anything that is not a plain object. + expect(isObject(null)).toBe(false); // Should return false for anything that is not a plain object. + expect(isObject(undefined)).toBe(false); // Should return false for anything that is not a plain object. + expect(isObject(1)).toBe(false); // Should return false for anything that is not a plain object. + expect(isObject('asd')).toBe(false); // Should return false for anything that is not a plain object. + expect(isObject(function () { })).toBe(false); // Should return false for anything that is not a plain object. + expect(isObject(Symbol('test'))).toBe(false); // Should return false for anything that is not a plain object. + expect(isObject(new Promise(res => res()))).toBe(false); // Should return false for anything that is not a plain object. // Object.create(null) creates an object with no prototype which may be tricky to handle. Filtering that out too. - expect(isObject(Object.create(null))).toBe(false); // Should return false for anything that is not a map object. + expect(isObject(Object.create(null))).toBe(false); // Should return false for anything that is not a plain object. }); diff --git a/src/utils/lang/index.ts b/src/utils/lang/index.ts index 2a7332ff..94efa40d 100644 --- a/src/utils/lang/index.ts +++ b/src/utils/lang/index.ts @@ -150,11 +150,35 @@ export function isNaNNumber(val: any): boolean { return val !== val; } +function _isObject(o: any) { + return Object.prototype.toString.call(o) === '[object Object]'; +} + /** - * Validates if a value is an object with the Object prototype (map object). + * Validates if a value is an object created by the Object constructor (plain object). + * Adapted from: + * + * is-plain-object + * + * Copyright (c) 2014-2017, Jon Schlinkert. + * Released under the MIT License. */ -export function isObject(obj: any): boolean { - return obj !== null && typeof obj === 'object' && obj.constructor === Object; +export function isObject(o: any): boolean { + if (_isObject(o) === false) return false; + + // If has modified constructor + const ctor = o.constructor; + if (ctor === undefined) return false; // `Object.create(null)` + + // If has modified prototype + const prot = ctor.prototype; + if (_isObject(prot) === false) return false; + + // If constructor does not have an Object-specific method + if (Object.prototype.hasOwnProperty.call(prot, 'isPrototypeOf') === false) return false; + + // Most likely a plain Object + return true; } /** From cb32d4e836064128de9b3b3b93702cb3e2f19106 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 29 Mar 2022 12:43:27 -0300 Subject: [PATCH 27/32] performance improvement --- src/utils/inputValidation/attributes.ts | 3 +-- src/utils/lang/__tests__/index.spec.ts | 7 +++++++ src/utils/lang/index.ts | 15 ++++++++++----- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/utils/inputValidation/attributes.ts b/src/utils/inputValidation/attributes.ts index 7a06ee90..e9824113 100644 --- a/src/utils/inputValidation/attributes.ts +++ b/src/utils/inputValidation/attributes.ts @@ -6,7 +6,7 @@ import { ERROR_NOT_PLAIN_OBJECT } from '../../logger/constants'; export function validateAttributes(log: ILogger, maybeAttrs: any, method: string): SplitIO.Attributes | undefined | false { // Attributes are optional - if (isObject(maybeAttrs) || maybeAttrs == undefined) // eslint-disable-line eqeqeq + if (maybeAttrs == undefined || isObject(maybeAttrs)) // eslint-disable-line eqeqeq return maybeAttrs; log.error(ERROR_NOT_PLAIN_OBJECT, [method, 'attributes']); @@ -23,5 +23,4 @@ export function validateAttributesDeep(log: ILogger, maybeAttributes: Record { // Object.create(null) creates an object with no prototype which may be tricky to handle. Filtering that out too. expect(isObject(Object.create(null))).toBe(false); // Should return false for anything that is not a plain object. + // validate on a different VM context + const ctx = vm.createContext({ isObject }); + vm.runInContext('var plainObjectResult = isObject({}); var arrayResult = isObject([]); var nullResult = isObject(null);', ctx); + expect(ctx.plainObjectResult).toBe(true); + expect(ctx.arrayResult).toBe(false); + expect(ctx.nullResult).toBe(false); }); test('LANG UTILS / uniqueId', () => { diff --git a/src/utils/lang/index.ts b/src/utils/lang/index.ts index 94efa40d..83a35f6a 100644 --- a/src/utils/lang/index.ts +++ b/src/utils/lang/index.ts @@ -154,16 +154,13 @@ function _isObject(o: any) { return Object.prototype.toString.call(o) === '[object Object]'; } -/** - * Validates if a value is an object created by the Object constructor (plain object). - * Adapted from: - * +/* * is-plain-object * * Copyright (c) 2014-2017, Jon Schlinkert. * Released under the MIT License. */ -export function isObject(o: any): boolean { +function _isPlainObject(o: any) { if (_isObject(o) === false) return false; // If has modified constructor @@ -181,6 +178,14 @@ export function isObject(o: any): boolean { return true; } +/** + * Validates if a value is an object created by the Object constructor (plain object). + * It uses `_isPlainObject` to avoid false negatives when validating values on a separate VM context. + */ +export function isObject(obj: any): boolean { + return obj !== null && typeof obj === 'object' && (obj.constructor === Object || _isPlainObject(obj)); +} + /** * Checks if a given value is a string. */ From b9161b2559792012cce0a6d9d9dc27658d1471d4 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 29 Mar 2022 12:44:50 -0300 Subject: [PATCH 28/32] size improvement --- src/utils/lang/__tests__/index.spec.ts | 21 ++++++++++++--- src/utils/lang/index.ts | 37 +++++--------------------- 2 files changed, 23 insertions(+), 35 deletions(-) diff --git a/src/utils/lang/__tests__/index.spec.ts b/src/utils/lang/__tests__/index.spec.ts index b4c3ae24..ae502e86 100644 --- a/src/utils/lang/__tests__/index.spec.ts +++ b/src/utils/lang/__tests__/index.spec.ts @@ -255,10 +255,23 @@ test('LANG UTILS / isObject', () => { // validate on a different VM context const ctx = vm.createContext({ isObject }); - vm.runInContext('var plainObjectResult = isObject({}); var arrayResult = isObject([]); var nullResult = isObject(null);', ctx); - expect(ctx.plainObjectResult).toBe(true); - expect(ctx.arrayResult).toBe(false); - expect(ctx.nullResult).toBe(false); + vm.runInContext(` + var positives = + isObject({}) && + isObject({ a: true }) && + isObject(new Object()) && + isObject(Object.create({})) && + isObject(Object.create(Object.prototype)); + var negatives = + isObject([]) || + isObject(() => { }) || + isObject(null) || + isObject(undefined) || + isObject(new Promise(res => res())) || + isObject(Object.create(null)); + `, ctx); + expect(ctx.positives).toBe(true); + expect(ctx.negatives).toBe(false); }); test('LANG UTILS / uniqueId', () => { diff --git a/src/utils/lang/index.ts b/src/utils/lang/index.ts index 83a35f6a..bd7c7a51 100644 --- a/src/utils/lang/index.ts +++ b/src/utils/lang/index.ts @@ -150,40 +150,15 @@ export function isNaNNumber(val: any): boolean { return val !== val; } -function _isObject(o: any) { - return Object.prototype.toString.call(o) === '[object Object]'; -} - -/* - * is-plain-object - * - * Copyright (c) 2014-2017, Jon Schlinkert. - * Released under the MIT License. - */ -function _isPlainObject(o: any) { - if (_isObject(o) === false) return false; - - // If has modified constructor - const ctor = o.constructor; - if (ctor === undefined) return false; // `Object.create(null)` - - // If has modified prototype - const prot = ctor.prototype; - if (_isObject(prot) === false) return false; - - // If constructor does not have an Object-specific method - if (Object.prototype.hasOwnProperty.call(prot, 'isPrototypeOf') === false) return false; - - // Most likely a plain Object - return true; -} - /** * Validates if a value is an object created by the Object constructor (plain object). - * It uses `_isPlainObject` to avoid false negatives when validating values on a separate VM context. + * It checks `constructor.name` to avoid false negatives when validating values on a separate VM context, which has its own global built-ins. */ -export function isObject(obj: any): boolean { - return obj !== null && typeof obj === 'object' && (obj.constructor === Object || _isPlainObject(obj)); +export function isObject(obj: any) { + return obj !== null && typeof obj === 'object' && ( + obj.constructor === Object || + (obj.constructor != null && obj.constructor.name === 'Object') + ); } /** From 5bcf761fb6f700ada2191f2dabf94e2debe1b946 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 29 Mar 2022 15:06:33 -0300 Subject: [PATCH 29/32] updated some type signatures --- src/evaluator/parser/index.ts | 2 +- src/evaluator/types.ts | 4 ++-- src/evaluator/value/index.ts | 4 ++-- src/evaluator/value/sanitize.ts | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/evaluator/parser/index.ts b/src/evaluator/parser/index.ts index 8e53a321..25c1e7f1 100644 --- a/src/evaluator/parser/index.ts +++ b/src/evaluator/parser/index.ts @@ -31,7 +31,7 @@ export function parser(log: ILogger, conditions: ISplitCondition[], storage: ISt const matcher = matcherFactory(log, matcherDto, storage); // Evaluator function. - return (key: string, attributes: SplitIO.Attributes, splitEvaluator: ISplitEvaluator) => { + return (key: string, attributes: SplitIO.Attributes | undefined, splitEvaluator: ISplitEvaluator) => { const value = sanitizeValue(log, key, matcherDto, attributes); const result = value !== undefined && matcher ? matcher(value, splitEvaluator) : false; diff --git a/src/evaluator/types.ts b/src/evaluator/types.ts index ccab4db8..54078b5b 100644 --- a/src/evaluator/types.ts +++ b/src/evaluator/types.ts @@ -6,7 +6,7 @@ import { ILogger } from '../logger/types'; export interface IDependencyMatcherValue { key: SplitIO.SplitKey, - attributes: SplitIO.Attributes + attributes?: SplitIO.Attributes } export interface IMatcherDto { @@ -27,7 +27,7 @@ export interface IEvaluation { export type IEvaluationResult = IEvaluation & { treatment: string } -export type ISplitEvaluator = (log: ILogger, key: SplitIO.SplitKey, splitName: string, attributes: SplitIO.Attributes, storage: IStorageSync | IStorageAsync) => MaybeThenable +export type ISplitEvaluator = (log: ILogger, key: SplitIO.SplitKey, splitName: string, attributes: SplitIO.Attributes | undefined, storage: IStorageSync | IStorageAsync) => MaybeThenable export type IEvaluator = (key: SplitIO.SplitKey, seed: number, trafficAllocation?: number, trafficAllocationSeed?: number, attributes?: SplitIO.Attributes, splitEvaluator?: ISplitEvaluator) => MaybeThenable diff --git a/src/evaluator/value/index.ts b/src/evaluator/value/index.ts index 319e05e2..c564a68f 100644 --- a/src/evaluator/value/index.ts +++ b/src/evaluator/value/index.ts @@ -4,7 +4,7 @@ import { ILogger } from '../../logger/types'; import { sanitize } from './sanitize'; import { ENGINE_VALUE, ENGINE_VALUE_NO_ATTRIBUTES, ENGINE_VALUE_INVALID } from '../../logger/constants'; -function parseValue(log: ILogger, key: string, attributeName: string | null, attributes: SplitIO.Attributes) { +function parseValue(log: ILogger, key: string, attributeName: string | null, attributes?: SplitIO.Attributes) { let value = undefined; if (attributeName) { if (attributes) { @@ -23,7 +23,7 @@ function parseValue(log: ILogger, key: string, attributeName: string | null, att /** * Defines value to be matched (key / attribute). */ -export function sanitizeValue(log: ILogger, key: string, matcherDto: IMatcherDto, attributes: SplitIO.Attributes) { +export function sanitizeValue(log: ILogger, key: string, matcherDto: IMatcherDto, attributes?: SplitIO.Attributes) { const attributeName = matcherDto.attribute; const valueToMatch = parseValue(log, key, attributeName, attributes); const sanitizedValue = sanitize(log, matcherDto.type, valueToMatch, matcherDto.dataType, attributes); diff --git a/src/evaluator/value/sanitize.ts b/src/evaluator/value/sanitize.ts index 6a36a59d..d12de8ed 100644 --- a/src/evaluator/value/sanitize.ts +++ b/src/evaluator/value/sanitize.ts @@ -41,7 +41,7 @@ function sanitizeBoolean(val: any): boolean | undefined { return undefined; } -function dependencyProcessor(sanitizedValue: string, attributes: SplitIO.Attributes): IDependencyMatcherValue { +function dependencyProcessor(sanitizedValue: string, attributes?: SplitIO.Attributes): IDependencyMatcherValue { return { key: sanitizedValue, attributes @@ -69,7 +69,7 @@ function getProcessingFunction(matcherTypeID: number, dataType: string) { /** * Sanitize matcher value */ -export function sanitize(log: ILogger, matcherTypeID: number, value: string | number | boolean | Array | undefined, dataType: string, attributes: SplitIO.Attributes) { +export function sanitize(log: ILogger, matcherTypeID: number, value: string | number | boolean | Array | undefined, dataType: string, attributes?: SplitIO.Attributes) { const processor = getProcessingFunction(matcherTypeID, dataType); let sanitizedValue: string | number | boolean | Array | IDependencyMatcherValue | undefined; From 5020990f796239893ddb40ba99cb9c7918be941b Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 29 Mar 2022 15:13:58 -0300 Subject: [PATCH 30/32] handle key and TT validation separately --- package-lock.json | 2 +- package.json | 2 +- .../settingsValidation/__tests__/index.spec.ts | 15 ++++++++++++++- src/utils/settingsValidation/index.ts | 14 ++++++++------ src/utils/settingsValidation/types.ts | 6 ++++-- 5 files changed, 28 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index 46d193a6..1e0f7f3d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.9", + "version": "1.2.1-rc.10", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 609120dc..c74fbf0e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.9", + "version": "1.2.1-rc.10", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/utils/settingsValidation/__tests__/index.spec.ts b/src/utils/settingsValidation/__tests__/index.spec.ts index 7f110c86..3f799970 100644 --- a/src/utils/settingsValidation/__tests__/index.spec.ts +++ b/src/utils/settingsValidation/__tests__/index.spec.ts @@ -191,7 +191,7 @@ describe('settingsValidation', () => { }); test('validates and sanitizes key and traffic type in client-side', () => { - const clientSideValidationParams = { ...minimalSettingsParams, isClientSide: true }; + const clientSideValidationParams = { ...minimalSettingsParams, acceptKey: true, acceptTT: true }; const samples = [{ key: ' valid-key ', settingsKey: 'valid-key', // key string is trimmed @@ -226,6 +226,19 @@ describe('settingsValidation', () => { expect(settings.core.trafficType).toEqual(settingsTrafficType); }); }); + + test('validates and sanitizes key, while traffic type is ignored', () => { + const settings = settingsValidation({ + core: { + authorizationKey: 'dummy token', + key: true, + trafficType: true + } + }, { ...minimalSettingsParams, acceptKey: true }); + + expect(settings.core.key).toEqual(false); // key is validated + expect(settings.core.trafficType).toEqual(true); // traffic type is ignored + }); }); test('SETTINGS / urls should be correctly assigned', () => { diff --git a/src/utils/settingsValidation/index.ts b/src/utils/settingsValidation/index.ts index c4de8fbc..5bb1125b 100644 --- a/src/utils/settingsValidation/index.ts +++ b/src/utils/settingsValidation/index.ts @@ -99,7 +99,7 @@ function fromSecondsToMillis(n: number) { */ export function settingsValidation(config: unknown, validationParams: ISettingsValidationParams) { - const { defaults, isClientSide, runtime, storage, integrations, logger, localhost, consent } = validationParams; + const { defaults, runtime, storage, integrations, logger, localhost, consent } = validationParams; // creates a settings object merging base, defaults and config objects. const withDefaults = merge({}, base, defaults, config) as ISettings; @@ -131,8 +131,8 @@ export function settingsValidation(config: unknown, validationParams: ISettingsV // @ts-ignore, modify readonly prop if (storage) withDefaults.storage = storage(withDefaults); - // In client-side, validate key and TT - if (isClientSide) { + // Validate key and TT (for client-side) + if (validationParams.acceptKey) { const maybeKey = withDefaults.core.key; // Although `key` is required in client-side, it can be omitted in LOCALHOST mode. In that case, the value `localhost_key` is used. if (withDefaults.mode === LOCALHOST_MODE && maybeKey === undefined) { @@ -144,9 +144,11 @@ export function settingsValidation(config: unknown, validationParams: ISettingsV withDefaults.core.key = validateKey(log, maybeKey, 'Client instantiation'); } - const maybeTT = withDefaults.core.trafficType; - if (maybeTT !== undefined) { // @ts-ignore, assigning false - withDefaults.core.trafficType = validateTrafficType(log, maybeTT, 'Client instantiation'); + if (validationParams.acceptTT) { + const maybeTT = withDefaults.core.trafficType; + if (maybeTT !== undefined) { // @ts-ignore + withDefaults.core.trafficType = validateTrafficType(log, maybeTT, 'Client instantiation'); + } } } diff --git a/src/utils/settingsValidation/types.ts b/src/utils/settingsValidation/types.ts index f6ead8e4..2322d042 100644 --- a/src/utils/settingsValidation/types.ts +++ b/src/utils/settingsValidation/types.ts @@ -10,8 +10,10 @@ export interface ISettingsValidationParams { * Version and startup properties are required, because they are not defined in the base settings. */ defaults: Partial & { version: string } & { startup: ISettings['startup'] }, - /** If true, validates core.key and core.trafficType */ - isClientSide?: boolean, + /** If true, validates core.key */ + acceptKey?: boolean, + /** If true, validates core.trafficType */ + acceptTT?: boolean, /** Define runtime values (`settings.runtime`) */ runtime: (settings: ISettings) => ISettings['runtime'], /** Storage validator (`settings.storage`) */ From 0ca9ff5fca2a36a367a66f53838acfde9b8cd78b Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 31 Mar 2022 13:01:03 -0300 Subject: [PATCH 31/32] fixed issue with a info log --- package-lock.json | 2 +- package.json | 2 +- src/sdkFactory/userConsentProps.ts | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1e0f7f3d..ebe428e0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.10", + "version": "1.2.1-rc.11", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index c74fbf0e..8338e84a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.2.1-rc.10", + "version": "1.2.1-rc.11", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/sdkFactory/userConsentProps.ts b/src/sdkFactory/userConsentProps.ts index da715c33..893a10ca 100644 --- a/src/sdkFactory/userConsentProps.ts +++ b/src/sdkFactory/userConsentProps.ts @@ -19,13 +19,12 @@ export function userConsentProps(settings: ISettings, syncManager?: ISyncManager const newConsentStatus = consent ? CONSENT_GRANTED : CONSENT_DECLINED; - if (settings.userConsent !== newConsentStatus) { // @ts-ignore, modify readonly prop + if (settings.userConsent !== newConsentStatus) { + log.info(USER_CONSENT_UPDATED, [settings.userConsent, newConsentStatus]); // @ts-ignore, modify readonly prop settings.userConsent = newConsentStatus; if (consent) syncManager?.submitter?.start(); // resumes submitters if transitioning to GRANTED else syncManager?.submitter?.stop(); // pauses submitters if transitioning to DECLINED - - log.info(USER_CONSENT_UPDATED, [settings.userConsent, newConsentStatus]); } else { log.info(USER_CONSENT_NOT_UPDATED, [newConsentStatus]); } From f053eaf4163ac2e96378702758d6cba75787a77e Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 31 Mar 2022 13:40:46 -0300 Subject: [PATCH 32/32] added info log for initial user consent --- src/logger/constants.ts | 1 + src/logger/messages/info.ts | 1 + src/sdkFactory/userConsentProps.ts | 5 ++++- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/logger/constants.ts b/src/logger/constants.ts index 68259646..10b45254 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -70,6 +70,7 @@ export const EVENTS_TRACKER_SUCCESS = 120; export const IMPRESSIONS_TRACKER_SUCCESS = 121; export const USER_CONSENT_UPDATED = 122; export const USER_CONSENT_NOT_UPDATED = 123; +export const USER_CONSENT_INITIAL = 124; export const ENGINE_VALUE_INVALID = 200; export const ENGINE_VALUE_NO_ATTRIBUTES = 201; diff --git a/src/logger/messages/info.ts b/src/logger/messages/info.ts index 9f4a4254..96703540 100644 --- a/src/logger/messages/info.ts +++ b/src/logger/messages/info.ts @@ -16,6 +16,7 @@ export const codesInfo: [number, string][] = codesWarn.concat([ [c.IMPRESSIONS_TRACKER_SUCCESS, c.LOG_PREFIX_IMPRESSIONS_TRACKER + 'Successfully stored %s impression(s).'], [c.USER_CONSENT_UPDATED, 'setUserConsent: consent status changed from %s to %s.'], [c.USER_CONSENT_NOT_UPDATED, 'setUserConsent: call had no effect because it was the current consent status (%s).'], + [c.USER_CONSENT_INITIAL, 'Starting the SDK with %s user consent. No data will be sent.'], // synchronizer [c.POLLING_SMART_PAUSING, c.LOG_PREFIX_SYNC_POLLING + 'Turning segments data polling %s.'], diff --git a/src/sdkFactory/userConsentProps.ts b/src/sdkFactory/userConsentProps.ts index 893a10ca..b275406c 100644 --- a/src/sdkFactory/userConsentProps.ts +++ b/src/sdkFactory/userConsentProps.ts @@ -1,6 +1,7 @@ -import { ERROR_NOT_BOOLEAN, USER_CONSENT_UPDATED, USER_CONSENT_NOT_UPDATED } from '../logger/constants'; +import { ERROR_NOT_BOOLEAN, USER_CONSENT_UPDATED, USER_CONSENT_NOT_UPDATED, USER_CONSENT_INITIAL } from '../logger/constants'; import { ISyncManager } from '../sync/types'; import { ISettings } from '../types'; +import { isConsentGranted } from '../utils/consent'; import { CONSENT_GRANTED, CONSENT_DECLINED } from '../utils/constants'; import { isBoolean } from '../utils/lang'; @@ -9,6 +10,8 @@ export function userConsentProps(settings: ISettings, syncManager?: ISyncManager const log = settings.log; + if (!isConsentGranted(settings)) log.info(USER_CONSENT_INITIAL, [settings.userConsent]); + return { setUserConsent(consent: unknown) { // validate input param