diff --git a/package-lock.json b/package-lock.json index c8abb07d..ebe428e0 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.11", "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 2bed38b7..8338e84a 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.11", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", 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; 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 })); } }; diff --git a/src/listeners/__tests__/browser.spec.ts b/src/listeners/__tests__/browser.spec.ts index 8af13e8b..5ec91eff 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/logger/constants.ts b/src/logger/constants.ts index ada74e13..10b45254 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -68,6 +68,9 @@ 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 USER_CONSENT_NOT_UPDATED = 123; +export const USER_CONSENT_INITIAL = 124; export const ENGINE_VALUE_INVALID = 200; export const ENGINE_VALUE_NO_ATTRIBUTES = 201; @@ -115,10 +118,11 @@ 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; +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/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 1d0fe00e..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,8 +28,9 @@ 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: provided param must be a boolean value.'], // 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 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.'], [c.ERROR_STORAGE_INVALID, c.LOG_PREFIX_SETTINGS+': The provided storage is invalid.%s Fallbacking into default MEMORY storage'], ]; diff --git a/src/logger/messages/info.ts b/src/logger/messages/info.ts index a8746dec..96703540 100644 --- a/src/logger/messages/info.ts +++ b/src/logger/messages/info.ts @@ -10,10 +10,13 @@ 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, '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/sdkClient/__tests__/sdkClientMethodCS.spec.ts b/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts index b5b6b21e..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 }[] = []; @@ -220,31 +216,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/client.ts b/src/sdkClient/client.ts index 5b0b7de2..15ea6193 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -16,7 +16,8 @@ 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) => { @@ -124,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/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 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/sdkFactory/__tests__/userConsentProps.spec.ts b/src/sdkFactory/__tests__/userConsentProps.spec.ts new file mode 100644 index 00000000..e7f4d0fc --- /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..898bb1a0 100644 --- a/src/sdkFactory/index.ts +++ b/src/sdkFactory/index.ts @@ -12,14 +12,15 @@ 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, - syncManagerFactory, SignalListener, impressionsObserverFactory, impressionListener, + const { settings, platform, storageFactory, splitApiFactory, extraProps, + syncManagerFactory, SignalListener, impressionsObserverFactory, integrationsManagerFactory, sdkManagerFactory, sdkClientMethodFactory } = params; const log = settings.log; @@ -73,8 +74,8 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO. // trackers const observer = impressionsObserverFactory && impressionsObserverFactory(); - const impressionsTracker = impressionsTrackerFactory(log, storage.impressions, settings, impressionListener, integrationsManager, observer, storage.impressionCounts); - const eventTracker = eventTrackerFactory(log, storage.events, integrationsManager); + const impressionsTracker = impressionsTrackerFactory(settings, storage.impressions, integrationsManager, observer, storage.impressionCounts); + const eventTracker = eventTrackerFactory(settings, storage.events, integrationsManager); // signal listener const signalListener = SignalListener && new SignalListener(syncManager, settings, storage, splitApi); @@ -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..2689af9a 100644 --- a/src/sdkFactory/types.ts +++ b/src/sdkFactory/types.ts @@ -64,10 +64,11 @@ export interface ISdkFactoryParams { serviceApi: ISplitApi | undefined) => ISignalListener, // Used by BrowserSignalListener // @TODO review impressionListener and integrations interfaces. What about handling impressionListener as an integration ? - impressionListener?: SplitIO.IImpressionListener, integrationsManagerFactory?: (params: IIntegrationFactoryParams) => IIntegrationManager | undefined, // Impression observer factory. If provided, will be used for impressions dedupe 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..b275406c --- /dev/null +++ b/src/sdkFactory/userConsentProps.ts @@ -0,0 +1,42 @@ +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'; + +// Extend client-side factory instances with user consent getter/setter +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 + if (!isBoolean(consent)) { + log.warn(ERROR_NOT_BOOLEAN, ['setUserConsent']); + return false; + } + + const newConsentStatus = consent ? CONSENT_GRANTED : CONSENT_DECLINED; + + 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 + } else { + log.info(USER_CONSENT_NOT_UPDATED, [newConsentStatus]); + } + + return true; + }, + + getUserConsent() { + return settings.userConsent; + } + }; +} 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 { 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(); +}); diff --git a/src/storages/inRedis/RedisAdapter.ts b/src/storages/inRedis/RedisAdapter.ts index 05ef94f7..0c374f23 100644 --- a/src/storages/inRedis/RedisAdapter.ts +++ b/src/storages/inRedis/RedisAdapter.ts @@ -164,6 +164,12 @@ 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 }); + } return result; } @@ -171,9 +177,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); 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', () => { diff --git a/src/sync/__tests__/syncManagerOnline.spec.ts b/src/sync/__tests__/syncManagerOnline.spec.ts new file mode 100644 index 00000000..c47cad86 --- /dev/null +++ b/src/sync/__tests__/syncManagerOnline.spec.ts @@ -0,0 +1,42 @@ +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 = { ...fullSettings }; + + // @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/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/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 151607ea..7c84374b 100644 --- a/src/sync/submitters/eventsSyncTask.ts +++ b/src/sync/submitters/eventsSyncTask.ts @@ -22,24 +22,35 @@ 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 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(); + } + // 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 5947c40c..f3fdb20c 100644 --- a/src/sync/submitters/impressionsSyncTask.ts +++ b/src/sync/submitters/impressionsSyncTask.ts @@ -57,8 +57,12 @@ 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(); + } + // 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/syncManagerOnline.ts b/src/sync/syncManagerOnline.ts index 1bbfe948..b95be139 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 { isConsentGranted } from '../utils/consent'; /** * Online SyncManager factory. @@ -25,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); @@ -39,7 +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); - /** Sync Manager logic */ function startPolling() { @@ -69,12 +69,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 +96,21 @@ export function syncManagerOnlineFactory( } // start periodic data recording (events, impressions, telemetry). - if (submitter) submitter.start(); - running = true; + if (isConsentGranted(settings)) 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 +118,7 @@ export function syncManagerOnlineFactory( }, flush() { - if (submitter) return submitter.execute(); + if (isConsentGranted(settings)) 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/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]); diff --git a/src/types.ts b/src/types.ts index 1e3654a8..7d523223 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 + readonly userConsent?: ConsentStatus } /** * Log levels. @@ -392,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. 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; +} diff --git a/src/utils/constants/index.ts b/src/utils/constants/index.ts index 4a4d8303..243accd6 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 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 { 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. + + // validate on a different VM context + const ctx = vm.createContext({ isObject }); + 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/__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/index.ts b/src/utils/lang/index.ts index f894f652..bd7c7a51 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)) { @@ -151,10 +151,14 @@ export function isNaNNumber(val: any): boolean { } /** - * 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). + * 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; +export function isObject(obj: any) { + return obj !== null && typeof obj === 'object' && ( + obj.constructor === Object || + (obj.constructor != null && obj.constructor.name === 'Object') + ); } /** @@ -164,6 +168,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/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(); diff --git a/src/utils/settingsValidation/__tests__/index.spec.ts b/src/utils/settingsValidation/__tests__/index.spec.ts index be0087b5..3f799970 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', () => { @@ -188,6 +189,56 @@ 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, acceptKey: true, acceptTT: 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('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/__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 = { diff --git a/src/utils/settingsValidation/consent.ts b/src/utils/settingsValidation/consent.ts new file mode 100644 index 00000000..98b4112b --- /dev/null +++ b/src/utils/settingsValidation/consent.ts @@ -0,0 +1,16 @@ +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 }): ConsentStatus { + userConsent = stringToUpperCase(userConsent); + + if (userConsentValues.indexOf(userConsent) > -1) return userConsent; + + log.error(ERROR_INVALID_CONFIG_PARAM, ['userConsent', userConsentValues, CONSENT_GRANTED]); + return CONSENT_GRANTED; +} diff --git a/src/utils/settingsValidation/impressionsMode.ts b/src/utils/settingsValidation/impressionsMode.ts index ddcda23d..9fdc3e09 100644 --- a/src/utils/settingsValidation/impressionsMode.ts +++ b/src/utils/settingsValidation/impressionsMode.ts @@ -1,14 +1,14 @@ -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'; +import { stringToUpperCase } from '../lang'; -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 { + impressionsMode = stringToUpperCase(impressionsMode); - 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..5bb1125b 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 } = 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; @@ -129,9 +131,25 @@ 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'; + // 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) { + 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'); + } + + if (validationParams.acceptTT) { + const maybeTT = withDefaults.core.trafficType; + if (maybeTT !== undefined) { // @ts-ignore + withDefaults.core.trafficType = validateTrafficType(log, maybeTT, 'Client instantiation'); + } + } } // Current ip/hostname information @@ -161,5 +179,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 = consent(withDefaults); + return withDefaults; } diff --git a/src/utils/settingsValidation/types.ts b/src/utils/settingsValidation/types.ts index b305d440..2322d042 100644 --- a/src/utils/settingsValidation/types.ts +++ b/src/utils/settingsValidation/types.ts @@ -10,7 +10,11 @@ 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 */ + acceptKey?: boolean, + /** If true, validates core.trafficType */ + acceptTT?: boolean, + /** Define runtime values (`settings.runtime`) */ runtime: (settings: ISettings) => ISettings['runtime'], /** Storage validator (`settings.storage`) */ storage?: (settings: ISettings) => ISettings['storage'], @@ -20,4 +24,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`) */ + consent: (settings: ISettings) => ISettings['userConsent'], }