From bfe62da149cceb57ccbf88019fbf13d5f5bc87f7 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 11 Nov 2021 10:08:23 -0300 Subject: [PATCH 1/7] implementation and tests --- package-lock.json | 2 +- package.json | 2 +- .../__tests__/sdkClientMethod.spec.ts | 12 ++-- src/sdkClient/sdkClient.ts | 5 +- src/sdkClient/sdkClientMethodCS.ts | 5 ++ src/sdkClient/sdkClientMethodCSWithTT.ts | 5 ++ src/sdkFactory/index.ts | 3 + src/storages/__tests__/testUtils.ts | 9 ++- .../pluggable/__tests__/index.spec.ts | 22 ++++++- src/storages/pluggable/index.ts | 21 +++++-- src/storages/types.ts | 9 ++- src/sync/submitters/submitterManager.ts | 22 +++++++ src/sync/syncManagerOnline.ts | 58 +++++++++---------- src/sync/types.ts | 2 +- src/types.ts | 2 +- src/utils/constants/index.ts | 8 ++- src/utils/settingsValidation/mode.ts | 4 +- .../storage/__tests__/storageCS.spec.ts | 29 +++++++--- .../settingsValidation/storage/storageCS.ts | 13 ++++- 19 files changed, 165 insertions(+), 68 deletions(-) create mode 100644 src/sync/submitters/submitterManager.ts diff --git a/package-lock.json b/package-lock.json index f32024b1..f912ec47 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.0.0", + "version": "1.0.1-rc.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 01e594a7..bd4a8851 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.0.0", + "version": "1.0.1-rc.0", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/sdkClient/__tests__/sdkClientMethod.spec.ts b/src/sdkClient/__tests__/sdkClientMethod.spec.ts index 5adc1d88..604f20f6 100644 --- a/src/sdkClient/__tests__/sdkClientMethod.spec.ts +++ b/src/sdkClient/__tests__/sdkClientMethod.spec.ts @@ -8,11 +8,11 @@ const errorMessage = 'Shared Client not supported by the storage mechanism. Crea const paramMocks = [ // No SyncManager (i.e., Async SDK) and No signal listener { - storage: { destroy: jest.fn() }, + storage: { destroy: jest.fn(() => Promise.resolve()) }, syncManager: undefined, sdkReadinessManager: { sdkStatus: jest.fn(), readinessManager: { destroy: jest.fn() } }, signalListener: undefined, - settings: { mode: CONSUMER_MODE, log: loggerMock } + settings: { mode: CONSUMER_MODE, log: loggerMock, core: { authorizationKey: 'api key '} } }, // SyncManager (i.e., Sync SDK) and Signal listener { @@ -20,11 +20,11 @@ const paramMocks = [ syncManager: { stop: jest.fn(), flush: jest.fn(() => Promise.resolve()) }, sdkReadinessManager: { sdkStatus: jest.fn(), readinessManager: { destroy: jest.fn() } }, signalListener: { stop: jest.fn() }, - settings: { mode: STANDALONE_MODE, log: loggerMock } + settings: { mode: STANDALONE_MODE, log: loggerMock, core: { authorizationKey: 'api key '} } } ]; -test.each(paramMocks)('sdkClientMethodFactory', (params) => { +test.each(paramMocks)('sdkClientMethodFactory', (params, done: any) => { // @ts-expect-error const sdkClientMethod = sdkClientMethodFactory(params); @@ -38,7 +38,7 @@ test.each(paramMocks)('sdkClientMethodFactory', (params) => { // multiple calls should return the same instance expect(sdkClientMethod()).toBe(client); - // `client.destroy` method should stop internal components (other client methods where validated in `client.spec.ts`) + // `client.destroy` method should stop internal components (other client methods are validated in `client.spec.ts`) client.destroy().then(() => { expect(params.sdkReadinessManager.readinessManager.destroy).toBeCalledTimes(1); expect(params.storage.destroy).toBeCalledTimes(1); @@ -48,6 +48,8 @@ test.each(paramMocks)('sdkClientMethodFactory', (params) => { expect(params.syncManager.flush).toBeCalledTimes(1); } if (params.signalListener) expect(params.signalListener.stop).toBeCalledTimes(1); + + done(); }); // calling the function with parameters should throw an error diff --git a/src/sdkClient/sdkClient.ts b/src/sdkClient/sdkClient.ts index e3051864..635dd976 100644 --- a/src/sdkClient/sdkClient.ts +++ b/src/sdkClient/sdkClient.ts @@ -1,6 +1,6 @@ import objectAssign from 'object-assign'; import { IStatusInterface, SplitIO } from '../types'; -import { CONSUMER_MODE } from '../utils/constants'; +import { CONSUMER_MODE, CONSUMER_PARTIAL_MODE } from '../utils/constants'; import { releaseApiKey } from '../utils/inputValidation/apiKey'; import clientFactory from './client'; import clientInputValidationDecorator from './clientInputValidation'; @@ -21,8 +21,7 @@ export function sdkClientFactory(params: ISdkClientFactoryParams): SplitIO.IClie settings.log, clientFactory(params), sdkReadinessManager.readinessManager, - // @TODO isStorageSync could be extracted from the storage itself (e.g. `storage.isSync`) to simplify interfaces - settings.mode === CONSUMER_MODE ? false : true, + [CONSUMER_MODE, CONSUMER_PARTIAL_MODE].indexOf(settings.mode) === -1 ? true : false, ), // Sdk destroy diff --git a/src/sdkClient/sdkClientMethodCS.ts b/src/sdkClient/sdkClientMethodCS.ts index b741c235..22254aaf 100644 --- a/src/sdkClient/sdkClientMethodCS.ts +++ b/src/sdkClient/sdkClientMethodCS.ts @@ -63,6 +63,11 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? // Emit SDK_READY in consumer mode for shared clients sharedSdkReadiness.readinessManager.segments.emit(SDK_SEGMENTS_ARRIVED); }); + + // 3 possibilities: + // - Standalone mode: both syncManager and sharedSyncManager are defined + // - Consumer mode: both syncManager and sharedSyncManager are undefined + // - Consumer partial mode: syncManager is defined (only for submitters) but sharedSyncManager is undefined // @ts-ignore const sharedSyncManager = syncManager && sharedStorage && (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage); diff --git a/src/sdkClient/sdkClientMethodCSWithTT.ts b/src/sdkClient/sdkClientMethodCSWithTT.ts index 82aa544d..6b8fa9ef 100644 --- a/src/sdkClient/sdkClientMethodCSWithTT.ts +++ b/src/sdkClient/sdkClientMethodCSWithTT.ts @@ -77,6 +77,11 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? // Emit SDK_READY in consumer mode for shared clients sharedSdkReadiness.readinessManager.segments.emit(SDK_SEGMENTS_ARRIVED); }); + + // 3 possibilities: + // - Standalone mode: both syncManager and sharedSyncManager are defined + // - Consumer mode: both syncManager and sharedSyncManager are undefined + // - Consumer partial mode: syncManager is defined (only for submitters) but sharedSyncManager is undefined // @ts-ignore const sharedSyncManager = syncManager && sharedStorage && (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage); diff --git a/src/sdkFactory/index.ts b/src/sdkFactory/index.ts index 0c116895..55359017 100644 --- a/src/sdkFactory/index.ts +++ b/src/sdkFactory/index.ts @@ -40,6 +40,9 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO. matchingKey: getMatching(settings.core.key), splitFiltersValidation: settings.sync.__splitFiltersValidation, + // ATM, only used by CustomStorage. true for partial consumer mode + mode: settings.mode, + // Callback used to emit SDK_READY in consumer mode, where `syncManagerFactory` is undefined // or only instantiates submitters, and therefore it is not able to emit readiness events. onReadyCb: (error) => { diff --git a/src/storages/__tests__/testUtils.ts b/src/storages/__tests__/testUtils.ts index b09e9c1b..4905adb2 100644 --- a/src/storages/__tests__/testUtils.ts +++ b/src/storages/__tests__/testUtils.ts @@ -1,4 +1,4 @@ -import { IStorageSync, IStorageAsync } from '../types'; +import { IStorageSync, IStorageAsync, IImpressionsCacheSync, IEventsCacheSync } from '../types'; // Assert that instances created by storage factories have the expected interface export function assertStorageInterface(storage: IStorageSync | IStorageAsync) { @@ -12,6 +12,13 @@ export function assertStorageInterface(storage: IStorageSync | IStorageAsync) { expect(!storage.impressionCounts || typeof storage.impressionCounts === 'object').toBeTruthy; } +export function assertSyncRecorderCacheInterface(cache: IEventsCacheSync | IImpressionsCacheSync) { + expect(typeof cache.isEmpty).toBe('function'); + expect(typeof cache.clear).toBe('function'); + expect(typeof cache.state).toBe('function'); + expect(typeof cache.track).toBe('function'); +} + // Split mocks export const splitWithUserTT = '{ "trafficTypeName": "user_tt", "conditions": [] }'; diff --git a/src/storages/pluggable/__tests__/index.spec.ts b/src/storages/pluggable/__tests__/index.spec.ts index 10b0f043..65bd7856 100644 --- a/src/storages/pluggable/__tests__/index.spec.ts +++ b/src/storages/pluggable/__tests__/index.spec.ts @@ -10,7 +10,8 @@ const prefix = 'some_prefix'; // Test target import { PluggableStorage } from '../index'; -import { assertStorageInterface } from '../../__tests__/testUtils'; +import { assertStorageInterface, assertSyncRecorderCacheInterface } from '../../__tests__/testUtils'; +import { CONSUMER_PARTIAL_MODE } from '../../../utils/constants'; describe('PLUGGABLE STORAGE', () => { @@ -70,4 +71,23 @@ describe('PLUGGABLE STORAGE', () => { expect(() => PluggableStorage({ wrapper: {} })).toThrow(errorNoValidWrapperInterface); }); + test('creates a storage instance for partial consumer mode (events and impressions cache in memory)', async () => { + const storageFactory = PluggableStorage({ prefix, wrapper: wrapperMock }); + const storage = storageFactory({ ...internalSdkParams, mode: CONSUMER_PARTIAL_MODE }); + + assertStorageInterface(storage); + expect(wrapperMock.connect).toBeCalledTimes(1); + + // Sync cache + assertSyncRecorderCacheInterface(storage.events); + assertSyncRecorderCacheInterface(storage.impressions); + + // But event track is async + const eventResult = storage.events.track('some data'); + expect(typeof eventResult.then === 'function').toBeTruthy(); + expect(await eventResult).toBe(true); + + const impResult = storage.impressions.track(['some data']); + expect(impResult).toBe(undefined); + }); }); diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index e00f87a5..fefb9cdc 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -8,7 +8,9 @@ import { EventsCachePluggable } from './EventsCachePluggable'; import { wrapperAdapter, METHODS_TO_PROMISE_WRAP } from './wrapperAdapter'; import { isObject } from '../../utils/lang'; import { validatePrefix } from '../KeyBuilder'; -import { STORAGE_CUSTOM } from '../../utils/constants'; +import { CONSUMER_PARTIAL_MODE, STORAGE_CUSTOM } from '../../utils/constants'; +import ImpressionsCacheInMemory from '../inMemory/ImpressionsCacheInMemory'; +import EventsCacheInMemory from '../inMemory/EventsCacheInMemory'; const NO_VALID_WRAPPER = 'Expecting custom storage `wrapper` in options, but no valid wrapper instance was provided.'; const NO_VALID_WRAPPER_INTERFACE = 'The provided wrapper instance doesn’t follow the expected interface. Check our docs.'; @@ -41,6 +43,16 @@ function wrapperConnect(wrapper: ICustomStorageWrapper, onReadyCb: (error?: any) }); } +// Async return type in `client.track` method on consumer partial mode +// No need to promisify impressions cache +function promisifyEventsTrack(events: any) { + const origTrack = events.track; + events.track = function () { + return Promise.resolve(origTrack.apply(this, arguments)); + }; + return events; +} + /** * Pluggable storage factory for consumer server-side & client-side SplitFactory. */ @@ -50,9 +62,10 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn const prefix = validatePrefix(options.prefix); - function PluggableStorageFactory({ log, metadata, onReadyCb }: IStorageFactoryParams): IStorageAsync { + function PluggableStorageFactory({ log, metadata, onReadyCb, mode, eventsQueueSize }: IStorageFactoryParams): IStorageAsync { const keys = new KeyBuilderSS(prefix, metadata); const wrapper = wrapperAdapter(log, options.wrapper); + const isPartialConsumer = mode === CONSUMER_PARTIAL_MODE; // emit SDK_READY event on main client wrapperConnect(wrapper, onReadyCb); @@ -60,8 +73,8 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn return { splits: new SplitsCachePluggable(log, keys, wrapper), segments: new SegmentsCachePluggable(log, keys, wrapper), - impressions: new ImpressionsCachePluggable(log, keys.buildImpressionsKey(), wrapper, metadata), - events: new EventsCachePluggable(log, keys.buildEventsKey(), wrapper, metadata), + impressions: isPartialConsumer ? new ImpressionsCacheInMemory() : new ImpressionsCachePluggable(log, keys.buildImpressionsKey(), wrapper, metadata), + events: isPartialConsumer ? promisifyEventsTrack(new EventsCacheInMemory(eventsQueueSize)) : new EventsCachePluggable(log, keys.buildEventsKey(), wrapper, metadata), // @TODO add telemetry cache when required // Disconnect the underlying storage, to release its resources (such as open files, database connections, etc). diff --git a/src/storages/types.ts b/src/storages/types.ts index b9c7b0e6..05c6d651 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -1,7 +1,7 @@ import { MaybeThenable, IMetadata, ISplitFiltersValidation } from '../dtos/types'; import { ILogger } from '../logger/types'; import { StoredEventWithMetadata, StoredImpressionWithMetadata } from '../sync/submitters/types'; -import { SplitIO, ImpressionDTO } from '../types'; +import { SplitIO, ImpressionDTO, SDKMode } from '../types'; /** * Interface of a custom storage wrapper. @@ -411,8 +411,8 @@ export type IStorageSync = IStorageBase< export type IStorageAsync = IStorageBase< ISplitsCacheAsync, ISegmentsCacheAsync, - IImpressionsCacheAsync, - IEventsCacheAsync, + IImpressionsCacheAsync | IImpressionsCacheSync, + IEventsCacheAsync | IEventsCacheSync, ILatenciesCacheAsync, ICountsCacheAsync > @@ -430,6 +430,9 @@ export interface IStorageFactoryParams { matchingKey?: string, /* undefined on server-side SDKs */ splitFiltersValidation?: ISplitFiltersValidation, + // ATM, only used by CustomStorage. True for partial consumer mode + mode?: SDKMode, + // This callback is invoked when the storage is ready to be used. Error-first callback style: if an error is passed, // it means that the storge fail to connect and shouldn't be used. // It is meant for emitting SDK_READY event in consumer mode, and for synchronizer to wait before using the storage. diff --git a/src/sync/submitters/submitterManager.ts b/src/sync/submitters/submitterManager.ts new file mode 100644 index 00000000..7828d947 --- /dev/null +++ b/src/sync/submitters/submitterManager.ts @@ -0,0 +1,22 @@ +import { syncTaskComposite } from '../syncTaskComposite'; +import { eventsSyncTaskFactory } from './eventsSyncTask'; +import { impressionsSyncTaskFactory } from './impressionsSyncTask'; +import { impressionCountsSyncTaskFactory } from './impressionCountsSyncTask'; +import { ISplitApi } from '../../services/types'; +import { IStorageSync } from '../../storages/types'; +import { ISettings } from '../../types'; + +export function submitterManagerFactory( + settings: ISettings, + storage: IStorageSync, + splitApi: ISplitApi, +) { + const log = settings.log; + const submitters = [ + impressionsSyncTaskFactory(log, splitApi.postTestImpressionsBulk, storage.impressions, settings.scheduler.impressionsRefreshRate, settings.core.labelsEnabled), + eventsSyncTaskFactory(log, splitApi.postEventsBulk, storage.events, settings.scheduler.eventsPushRate, settings.startup.eventsFirstPushWindow) + // @TODO add telemetry submitter + ]; + if (storage.impressionCounts) submitters.push(impressionCountsSyncTaskFactory(log, splitApi.postTestImpressionsCount, storage.impressionCounts)); + return syncTaskComposite(submitters); +} diff --git a/src/sync/syncManagerOnline.ts b/src/sync/syncManagerOnline.ts index b0199a9c..c8264106 100644 --- a/src/sync/syncManagerOnline.ts +++ b/src/sync/syncManagerOnline.ts @@ -1,8 +1,5 @@ -import { ISyncManager, ISyncManagerCS, ISyncManagerFactoryParams } from './types'; -import { syncTaskComposite } from './syncTaskComposite'; -import { eventsSyncTaskFactory } from './submitters/eventsSyncTask'; -import { impressionsSyncTaskFactory } from './submitters/impressionsSyncTask'; -import { impressionCountsSyncTaskFactory } from './submitters/impressionCountsSyncTask'; +import { ISyncManagerCS, ISyncManagerFactoryParams } from './types'; +import { submitterManagerFactory } from './submitters/submitterManager'; import { IReadinessManager } from '../readiness/types'; import { IStorageSync } from '../storages/types'; import { IPushManagerFactoryParams, IPushManager, IPushManagerCS } from './streaming/types'; @@ -19,7 +16,7 @@ import { SYNC_START_POLLING, SYNC_CONTINUE_POLLING, SYNC_STOP_POLLING } from '.. * @param pushManagerFactory optional to build a SyncManager with or without streaming support */ export function syncManagerOnlineFactory( - pollingManagerFactory: (...args: IPollingManagerFactoryParams) => IPollingManager, + pollingManagerFactory?: (...args: IPollingManagerFactoryParams) => IPollingManager, pushManagerFactory?: (...args: IPushManagerFactoryParams) => IPushManager | undefined ): (params: ISyncManagerFactoryParams) => ISyncManagerCS { @@ -37,30 +34,24 @@ export function syncManagerOnlineFactory( const log = settings.log; /** Polling Manager */ - const pollingManager = pollingManagerFactory(splitApi, storage, readiness, settings); + const pollingManager = pollingManagerFactory && pollingManagerFactory(splitApi, storage, readiness, settings); /** Push Manager */ - const pushManager = settings.streamingEnabled && pushManagerFactory ? + const pushManager = settings.streamingEnabled && pollingManager && pushManagerFactory ? pushManagerFactory(pollingManager, storage, readiness, splitApi.fetchAuth, platform, settings) : undefined; /** Submitter Manager */ - // It is not inyected via a factory as push and polling managers, because at the moment it is mandatory and the same for server-side and client-side variants - const submitters = [ - impressionsSyncTaskFactory(log, splitApi.postTestImpressionsBulk, storage.impressions, settings.scheduler.impressionsRefreshRate, settings.core.labelsEnabled), - eventsSyncTaskFactory(log, splitApi.postEventsBulk, storage.events, settings.scheduler.eventsPushRate, settings.startup.eventsFirstPushWindow) - // @TODO add telemetry submitter - ]; - if (storage.impressionCounts) submitters.push(impressionCountsSyncTaskFactory(log, splitApi.postTestImpressionsCount, storage.impressionCounts)); - const submitter = syncTaskComposite(submitters); + // It is not inyected as push and polling managers, because at the moment it is required + const submitter = submitterManagerFactory(settings, storage, splitApi); /** Sync Manager logic */ function startPolling() { - if (!pollingManager.isRunning()) { + if (!pollingManager!.isRunning()) { log.info(SYNC_START_POLLING); - pollingManager.start(); + pollingManager!.start(); } else { log.info(SYNC_CONTINUE_POLLING); } @@ -69,10 +60,10 @@ export function syncManagerOnlineFactory( function stopPollingAndSyncAll() { log.info(SYNC_STOP_POLLING); // if polling, stop - if (pollingManager.isRunning()) pollingManager.stop(); + if (pollingManager!.isRunning()) pollingManager!.stop(); // fetch splits and segments. There is no need to catch this promise (it is always resolved) - pollingManager.syncAll(); + pollingManager!.syncAll(); } if (pushManager) { @@ -91,15 +82,17 @@ export function syncManagerOnlineFactory( */ start() { // start syncing splits and segments - if (pushManager) { - // Doesn't call `syncAll` when the syncManager is resuming - if (startFirstTime) { - pollingManager.syncAll(); - startFirstTime = false; + if (pollingManager) { + if (pushManager) { + // Doesn't call `syncAll` when the syncManager is resuming + if (startFirstTime) { + pollingManager.syncAll(); + startFirstTime = false; + } + pushManager.start(); + } else { + pollingManager.start(); } - pushManager.start(); - } else { - pollingManager.start(); } // start periodic data recording (events, impressions, telemetry). @@ -113,7 +106,7 @@ export function syncManagerOnlineFactory( stop() { // stop syncing if (pushManager) pushManager.stop(); - if (pollingManager.isRunning()) pollingManager.stop(); + if (pollingManager && pollingManager.isRunning()) pollingManager.stop(); // stop periodic data recording (events, impressions, telemetry). if (submitter) submitter.stop(); @@ -130,8 +123,9 @@ export function syncManagerOnlineFactory( }, // [Only used for client-side] - // It assumes that polling and push managers implement the interfaces for client-side - shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync): ISyncManager { + // If polling and push managers are defined (standalone mode), they implement the interfaces for client-side + shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync) { + if (!pollingManager) return; const mySegmentsSyncTask = (pollingManager as IPollingManagerCS).add(matchingKey, readinessManager, storage); @@ -139,7 +133,7 @@ export function syncManagerOnlineFactory( isRunning: mySegmentsSyncTask.isRunning, start() { if (pushManager) { - if (pollingManager.isRunning()) { + if (pollingManager!.isRunning()) { // if doing polling, we must start the periodic fetch of data if (storage.splits.usesSegments()) mySegmentsSyncTask.start(); } else { diff --git a/src/sync/types.ts b/src/sync/types.ts index 55566ef1..0f0588d9 100644 --- a/src/sync/types.ts +++ b/src/sync/types.ts @@ -47,7 +47,7 @@ export interface ISyncManager extends ITask { } export interface ISyncManagerCS extends ISyncManager { - shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync): ISyncManager + shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync): ISyncManager | undefined } export interface ISyncManagerFactoryParams { diff --git a/src/types.ts b/src/types.ts index a2641970..94e6d461 100644 --- a/src/types.ts +++ b/src/types.ts @@ -52,7 +52,7 @@ type EventConsts = { * SDK Modes. * @typedef {string} SDKMode */ -export type SDKMode = 'standalone' | 'consumer' | 'localhost'; +export type SDKMode = 'standalone' | 'consumer' | 'localhost' | 'consumer_partial'; /** * 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. diff --git a/src/utils/constants/index.ts b/src/utils/constants/index.ts index 51bcafca..e632938f 100644 --- a/src/utils/constants/index.ts +++ b/src/utils/constants/index.ts @@ -1,4 +1,5 @@ import { StorageType } from '../../storages/types'; +import { SDKMode } from '../../types'; // Special treatments export const CONTROL = 'control'; @@ -20,10 +21,11 @@ export const DEBUG = 'DEBUG'; export const OPTIMIZED = 'OPTIMIZED'; // SDK Modes -export const LOCALHOST_MODE = 'localhost'; -export const STANDALONE_MODE = 'standalone'; +export const LOCALHOST_MODE: SDKMode = 'localhost'; +export const STANDALONE_MODE: SDKMode = 'standalone'; export const PRODUCER_MODE = 'producer'; -export const CONSUMER_MODE = 'consumer'; +export const CONSUMER_MODE: SDKMode = 'consumer'; +export const CONSUMER_PARTIAL_MODE: SDKMode = 'consumer_partial'; // Storage types export const STORAGE_MEMORY: StorageType = 'MEMORY'; diff --git a/src/utils/settingsValidation/mode.ts b/src/utils/settingsValidation/mode.ts index 0b2464b2..dd11fe06 100644 --- a/src/utils/settingsValidation/mode.ts +++ b/src/utils/settingsValidation/mode.ts @@ -1,10 +1,10 @@ -import { LOCALHOST_MODE, STANDALONE_MODE, PRODUCER_MODE, CONSUMER_MODE } from '../constants'; +import { LOCALHOST_MODE, STANDALONE_MODE, PRODUCER_MODE, CONSUMER_MODE, CONSUMER_PARTIAL_MODE } from '../constants'; export default function mode(key: string, mode: string) { // Leaving the comparison as is, in case we change the mode name but not the setting. if (key === 'localhost') return LOCALHOST_MODE; - if ([STANDALONE_MODE, PRODUCER_MODE, CONSUMER_MODE].indexOf(mode) === -1) throw Error('Invalid mode provided'); + if ([STANDALONE_MODE, PRODUCER_MODE, CONSUMER_MODE, CONSUMER_PARTIAL_MODE].indexOf(mode) === -1) throw Error('Invalid mode provided'); return mode; } diff --git a/src/utils/settingsValidation/storage/__tests__/storageCS.spec.ts b/src/utils/settingsValidation/storage/__tests__/storageCS.spec.ts index 72456b1f..c2dbc9eb 100644 --- a/src/utils/settingsValidation/storage/__tests__/storageCS.spec.ts +++ b/src/utils/settingsValidation/storage/__tests__/storageCS.spec.ts @@ -5,6 +5,9 @@ import { loggerMock as log } from '../../../../logger/__tests__/sdkLogger.mock'; const mockInLocalStorageFactory = () => { }; mockInLocalStorageFactory.type = 'LOCALSTORAGE'; +const mockCustomStorageFactory = () => { }; +mockCustomStorageFactory.type = 'CUSTOM'; + describe('storage validator for pluggable storage (client-side)', () => { afterEach(() => { @@ -13,24 +16,34 @@ describe('storage validator for pluggable storage (client-side)', () => { // Check different types, since `storage` param is defined by the user test('fallbacks to default InMemory storage if the storage is invalid or not provided', () => { - expect(validateStorageCS({ log })).toBe(InMemoryStorageCSFactory); - expect(validateStorageCS({ log, storage: undefined })).toBe(InMemoryStorageCSFactory); + expect(validateStorageCS({ log, mode: 'standalone' })).toBe(InMemoryStorageCSFactory); + expect(validateStorageCS({ log, mode: 'standalone', storage: undefined })).toBe(InMemoryStorageCSFactory); expect(log.warn).not.toBeCalled(); - expect(validateStorageCS({ log, storage: {} })).toBe(InMemoryStorageCSFactory); - expect(validateStorageCS({ log, storage: 'invalid storage' })).toBe(InMemoryStorageCSFactory); - expect(validateStorageCS({ log, storage: () => { } })).toBe(InMemoryStorageCSFactory); - expect(validateStorageCS({ log, storage: () => { }, mode: 'localhost' })).toBe(InMemoryStorageCSFactory); + expect(validateStorageCS({ log, mode: 'standalone', storage: {} })).toBe(InMemoryStorageCSFactory); + expect(validateStorageCS({ log, mode: 'standalone', storage: 'invalid storage' })).toBe(InMemoryStorageCSFactory); + expect(validateStorageCS({ log, mode: 'standalone', storage: () => { } })).toBe(InMemoryStorageCSFactory); + expect(validateStorageCS({ log, mode: 'localhost', storage: () => { } })).toBe(InMemoryStorageCSFactory); expect(log.warn).toBeCalledTimes(4); }); test('returns the provided storage factory if it is valid', () => { - expect(validateStorageCS({ log, storage: mockInLocalStorageFactory })).toBe(mockInLocalStorageFactory); + expect(validateStorageCS({ log, mode: 'standalone', storage: mockInLocalStorageFactory })).toBe(mockInLocalStorageFactory); expect(log.warn).not.toBeCalled(); }); test('fallbacks to mock InLocalStorage storage if the storage is InLocalStorage and the mode localhost', () => { - expect(validateStorageCS({ log, storage: mockInLocalStorageFactory, mode: 'localhost' })).toBe(__InLocalStorageMockFactory); + expect(validateStorageCS({ log, mode: 'localhost', storage: mockInLocalStorageFactory })).toBe(__InLocalStorageMockFactory); + expect(log.warn).not.toBeCalled(); + }); + + test('throws error if the provided storage factory is not compatible with the mode', () => { + expect(() => { validateStorageCS({ log, mode: 'consumer', storage: mockInLocalStorageFactory }); }).toThrow('A CustomStorage instance is required on consumer modes'); + expect(() => { validateStorageCS({ log, mode: 'consumer_partial', storage: mockInLocalStorageFactory }); }).toThrow('A CustomStorage instance is required on consumer modes'); + + expect(() => { validateStorageCS({ log, mode: 'standalone', storage: mockCustomStorageFactory }); }).toThrow('A CustomStorage instance cannot be used on standalone and localhost modes'); + expect(() => { validateStorageCS({ log, mode: 'localhost', storage: mockCustomStorageFactory }); }).toThrow('A CustomStorage instance cannot be used on standalone and localhost modes'); + expect(log.warn).not.toBeCalled(); }); diff --git a/src/utils/settingsValidation/storage/storageCS.ts b/src/utils/settingsValidation/storage/storageCS.ts index 841ca959..ad5d42fe 100644 --- a/src/utils/settingsValidation/storage/storageCS.ts +++ b/src/utils/settingsValidation/storage/storageCS.ts @@ -2,7 +2,7 @@ import { InMemoryStorageCSFactory } from '../../../storages/inMemory/InMemorySto import { ISettings, SDKMode } from '../../../types'; import { ILogger } from '../../../logger/types'; import { WARN_STORAGE_INVALID } from '../../../logger/constants'; -import { LOCALHOST_MODE, STORAGE_CUSTOM, STORAGE_LOCALSTORAGE, STORAGE_MEMORY } from '../../../utils/constants'; +import { LOCALHOST_MODE, STANDALONE_MODE, STORAGE_CUSTOM, STORAGE_LOCALSTORAGE, STORAGE_MEMORY } from '../../../utils/constants'; import { IStorageFactoryParams, IStorageSync } from '../../../storages/types'; export function __InLocalStorageMockFactory(params: IStorageFactoryParams): IStorageSync { @@ -19,7 +19,7 @@ __InLocalStorageMockFactory.type = STORAGE_MEMORY; * * @returns {Object} valid storage factory. It might be the default `InMemoryStorageCSFactory` if the provided storage is invalid. */ -export function validateStorageCS(settings: { log: ILogger, storage?: any, mode?: SDKMode }): ISettings['storage'] { +export function validateStorageCS(settings: { log: ILogger, storage?: any, mode: SDKMode }): ISettings['storage'] { let { storage = InMemoryStorageCSFactory, log, mode } = settings; // If an invalid storage is provided, fallback into MEMORY @@ -33,6 +33,15 @@ export function validateStorageCS(settings: { log: ILogger, storage?: any, mode? return __InLocalStorageMockFactory; } + // @TODO check behaviour + if ([LOCALHOST_MODE, STANDALONE_MODE].indexOf(mode) === -1) { + // Consumer modes require an async storage + if (storage.type !== STORAGE_CUSTOM) throw new Error('A CustomStorage instance is required on consumer modes'); + } else { + // Standalone and localhost modes require a sync storage + if (storage.type === STORAGE_CUSTOM) throw new Error('A CustomStorage instance cannot be used on standalone and localhost modes'); + } + // return default InMemory storage if provided one is not valid return storage; } From e7918dd65fd163495e3e8b8534c9ce3439ad6d53 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 11 Nov 2021 10:25:52 -0300 Subject: [PATCH 2/7] added comment --- src/sdkClient/sdkClient.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sdkClient/sdkClient.ts b/src/sdkClient/sdkClient.ts index 635dd976..e0401347 100644 --- a/src/sdkClient/sdkClient.ts +++ b/src/sdkClient/sdkClient.ts @@ -21,6 +21,7 @@ export function sdkClientFactory(params: ISdkClientFactoryParams): SplitIO.IClie settings.log, clientFactory(params), sdkReadinessManager.readinessManager, + // storage is async if and only if mode is consumer or partial consumer [CONSUMER_MODE, CONSUMER_PARTIAL_MODE].indexOf(settings.mode) === -1 ? true : false, ), From ee3b142e314131c95ba71c2c57c31181f0ff8033 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 12 Nov 2021 10:02:14 -0300 Subject: [PATCH 3/7] impressions dedup --- package-lock.json | 2 +- package.json | 2 +- src/storages/pluggable/__tests__/index.spec.ts | 3 ++- src/storages/pluggable/index.ts | 4 +++- src/trackers/impressionObserver/utils.ts | 4 ++-- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index f912ec47..7f35f186 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.0.1-rc.0", + "version": "1.0.1-rc.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index bd4a8851..75501e66 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.0.1-rc.0", + "version": "1.0.1-rc.1", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/storages/pluggable/__tests__/index.spec.ts b/src/storages/pluggable/__tests__/index.spec.ts index 65bd7856..2ee8bc70 100644 --- a/src/storages/pluggable/__tests__/index.spec.ts +++ b/src/storages/pluggable/__tests__/index.spec.ts @@ -73,7 +73,7 @@ describe('PLUGGABLE STORAGE', () => { test('creates a storage instance for partial consumer mode (events and impressions cache in memory)', async () => { const storageFactory = PluggableStorage({ prefix, wrapper: wrapperMock }); - const storage = storageFactory({ ...internalSdkParams, mode: CONSUMER_PARTIAL_MODE }); + const storage = storageFactory({ ...internalSdkParams, mode: CONSUMER_PARTIAL_MODE, optimize: true }); assertStorageInterface(storage); expect(wrapperMock.connect).toBeCalledTimes(1); @@ -81,6 +81,7 @@ describe('PLUGGABLE STORAGE', () => { // Sync cache assertSyncRecorderCacheInterface(storage.events); assertSyncRecorderCacheInterface(storage.impressions); + assertSyncRecorderCacheInterface(storage.impressionCounts); // But event track is async const eventResult = storage.events.track('some data'); diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index fefb9cdc..9f4c3b30 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -11,6 +11,7 @@ import { validatePrefix } from '../KeyBuilder'; import { CONSUMER_PARTIAL_MODE, STORAGE_CUSTOM } from '../../utils/constants'; import ImpressionsCacheInMemory from '../inMemory/ImpressionsCacheInMemory'; import EventsCacheInMemory from '../inMemory/EventsCacheInMemory'; +import ImpressionCountsCacheInMemory from '../inMemory/ImpressionCountsCacheInMemory'; const NO_VALID_WRAPPER = 'Expecting custom storage `wrapper` in options, but no valid wrapper instance was provided.'; const NO_VALID_WRAPPER_INTERFACE = 'The provided wrapper instance doesn’t follow the expected interface. Check our docs.'; @@ -62,7 +63,7 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn const prefix = validatePrefix(options.prefix); - function PluggableStorageFactory({ log, metadata, onReadyCb, mode, eventsQueueSize }: IStorageFactoryParams): IStorageAsync { + function PluggableStorageFactory({ log, metadata, onReadyCb, mode, eventsQueueSize, optimize }: IStorageFactoryParams): IStorageAsync { const keys = new KeyBuilderSS(prefix, metadata); const wrapper = wrapperAdapter(log, options.wrapper); const isPartialConsumer = mode === CONSUMER_PARTIAL_MODE; @@ -74,6 +75,7 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn splits: new SplitsCachePluggable(log, keys, wrapper), segments: new SegmentsCachePluggable(log, keys, wrapper), impressions: isPartialConsumer ? new ImpressionsCacheInMemory() : new ImpressionsCachePluggable(log, keys.buildImpressionsKey(), wrapper, metadata), + impressionCounts: optimize ? new ImpressionCountsCacheInMemory() : undefined, events: isPartialConsumer ? promisifyEventsTrack(new EventsCacheInMemory(eventsQueueSize)) : new EventsCachePluggable(log, keys.buildEventsKey(), wrapper, metadata), // @TODO add telemetry cache when required diff --git a/src/trackers/impressionObserver/utils.ts b/src/trackers/impressionObserver/utils.ts index 3b921a8f..7fb73b85 100644 --- a/src/trackers/impressionObserver/utils.ts +++ b/src/trackers/impressionObserver/utils.ts @@ -1,11 +1,11 @@ -import { OPTIMIZED, PRODUCER_MODE, STANDALONE_MODE } from '../../utils/constants'; +import { CONSUMER_PARTIAL_MODE, OPTIMIZED, PRODUCER_MODE, STANDALONE_MODE } from '../../utils/constants'; import { ISettings } from '../../types'; /** * Checks if impressions previous time should be added or not. */ export function shouldAddPt(settings: ISettings) { - return [PRODUCER_MODE, STANDALONE_MODE].indexOf(settings.mode) > -1 ? true : false; + return [PRODUCER_MODE, STANDALONE_MODE, CONSUMER_PARTIAL_MODE].indexOf(settings.mode) > -1 ? true : false; } /** From 1e1b3be45f35f93b47170e8527f46e525b682447 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 12 Nov 2021 15:59:52 -0300 Subject: [PATCH 4/7] handling log errors in storage validation --- src/logger/constants.ts | 6 ++--- src/logger/messages/error.ts | 3 ++- src/logger/messages/warn.ts | 1 - .../storage/__tests__/storageCS.spec.ts | 22 ++++++++++--------- .../settingsValidation/storage/storageCS.ts | 16 +++++++++----- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/logger/constants.ts b/src/logger/constants.ts index 5ca7317c..ab711cac 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -91,9 +91,8 @@ export const WARN_INTEGRATION_INVALID = 218; export const WARN_SPLITS_FILTER_IGNORED = 219; export const WARN_SPLITS_FILTER_INVALID = 220; export const WARN_SPLITS_FILTER_EMPTY = 221; -export const WARN_STORAGE_INVALID = 222; -export const WARN_API_KEY = 223; -export const STREAMING_PARSING_MY_SEGMENTS_UPDATE_V2 = 224; +export const WARN_API_KEY = 222; +export const STREAMING_PARSING_MY_SEGMENTS_UPDATE_V2 = 223; export const ERROR_ENGINE_COMBINER_IFELSEIF = 300; export const ERROR_LOGLEVEL_INVALID = 301; @@ -119,6 +118,7 @@ export const ERROR_EMPTY_ARRAY = 320; 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; // 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 75269bc9..2ce2d3a6 100644 --- a/src/logger/messages/error.ts +++ b/src/logger/messages/error.ts @@ -30,5 +30,6 @@ export const codesError: [number, string][] = [ [c.ERROR_EMPTY_ARRAY, '%s: %s must be a non-empty array.'], // 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.'] + [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. Fallbacking into default MEMORY storage%s'], ]; diff --git a/src/logger/messages/warn.ts b/src/logger/messages/warn.ts index 350eeb43..8c35e311 100644 --- a/src/logger/messages/warn.ts +++ b/src/logger/messages/warn.ts @@ -29,7 +29,6 @@ export const codesWarn: [number, string][] = codesError.concat([ [c.WARN_SPLITS_FILTER_IGNORED, c.LOG_PREFIX_SETTINGS+': split filters have been configured but will have no effect if mode is not "%s", since synchronization is being deferred to an external tool.'], [c.WARN_SPLITS_FILTER_INVALID, c.LOG_PREFIX_SETTINGS+': split filter at position %s is invalid. It must be an object with a valid filter type ("byName" or "byPrefix") and a list of "values".'], [c.WARN_SPLITS_FILTER_EMPTY, c.LOG_PREFIX_SETTINGS+': splitFilters configuration must be a non-empty array of filter objects.'], - [c.WARN_STORAGE_INVALID, c.LOG_PREFIX_SETTINGS+': The provided storage is invalid. Fallbacking into default MEMORY storage'], [c.WARN_API_KEY, c.LOG_PREFIX_SETTINGS+': You already have %s. We recommend keeping only one instance of the factory at all times (Singleton pattern) and reusing it throughout your application'], [c.STREAMING_PARSING_MY_SEGMENTS_UPDATE_V2, c.LOG_PREFIX_SYNC_STREAMING + 'Fetching MySegments due to an error processing %s notification: %s'], diff --git a/src/utils/settingsValidation/storage/__tests__/storageCS.spec.ts b/src/utils/settingsValidation/storage/__tests__/storageCS.spec.ts index c2dbc9eb..6d270946 100644 --- a/src/utils/settingsValidation/storage/__tests__/storageCS.spec.ts +++ b/src/utils/settingsValidation/storage/__tests__/storageCS.spec.ts @@ -11,40 +11,42 @@ mockCustomStorageFactory.type = 'CUSTOM'; describe('storage validator for pluggable storage (client-side)', () => { afterEach(() => { - log.warn.mockClear(); + log.error.mockClear(); }); // Check different types, since `storage` param is defined by the user test('fallbacks to default InMemory storage if the storage is invalid or not provided', () => { expect(validateStorageCS({ log, mode: 'standalone' })).toBe(InMemoryStorageCSFactory); expect(validateStorageCS({ log, mode: 'standalone', storage: undefined })).toBe(InMemoryStorageCSFactory); - expect(log.warn).not.toBeCalled(); + expect(log.error).not.toBeCalled(); expect(validateStorageCS({ log, mode: 'standalone', storage: {} })).toBe(InMemoryStorageCSFactory); expect(validateStorageCS({ log, mode: 'standalone', storage: 'invalid storage' })).toBe(InMemoryStorageCSFactory); expect(validateStorageCS({ log, mode: 'standalone', storage: () => { } })).toBe(InMemoryStorageCSFactory); expect(validateStorageCS({ log, mode: 'localhost', storage: () => { } })).toBe(InMemoryStorageCSFactory); - expect(log.warn).toBeCalledTimes(4); + expect(log.error).toBeCalledTimes(4); }); test('returns the provided storage factory if it is valid', () => { expect(validateStorageCS({ log, mode: 'standalone', storage: mockInLocalStorageFactory })).toBe(mockInLocalStorageFactory); - expect(log.warn).not.toBeCalled(); + expect(log.error).not.toBeCalled(); }); test('fallbacks to mock InLocalStorage storage if the storage is InLocalStorage and the mode localhost', () => { expect(validateStorageCS({ log, mode: 'localhost', storage: mockInLocalStorageFactory })).toBe(__InLocalStorageMockFactory); - expect(log.warn).not.toBeCalled(); + expect(log.error).not.toBeCalled(); }); test('throws error if the provided storage factory is not compatible with the mode', () => { - expect(() => { validateStorageCS({ log, mode: 'consumer', storage: mockInLocalStorageFactory }); }).toThrow('A CustomStorage instance is required on consumer modes'); - expect(() => { validateStorageCS({ log, mode: 'consumer_partial', storage: mockInLocalStorageFactory }); }).toThrow('A CustomStorage instance is required on consumer modes'); + expect(() => { validateStorageCS({ log, mode: 'consumer', storage: mockInLocalStorageFactory }); }).toThrow('A CustomStorage instance is required on consumer mode'); + expect(() => { validateStorageCS({ log, mode: 'consumer_partial', storage: mockInLocalStorageFactory }); }).toThrow('A CustomStorage instance is required on consumer mode'); - expect(() => { validateStorageCS({ log, mode: 'standalone', storage: mockCustomStorageFactory }); }).toThrow('A CustomStorage instance cannot be used on standalone and localhost modes'); - expect(() => { validateStorageCS({ log, mode: 'localhost', storage: mockCustomStorageFactory }); }).toThrow('A CustomStorage instance cannot be used on standalone and localhost modes'); + expect(log.error).not.toBeCalled(); - expect(log.warn).not.toBeCalled(); + expect(validateStorageCS({ log, mode: 'standalone', storage: mockCustomStorageFactory })).toBe(InMemoryStorageCSFactory); + expect(validateStorageCS({ log, mode: 'localhost', storage: mockCustomStorageFactory })).toBe(InMemoryStorageCSFactory); + + expect(log.error).toBeCalledTimes(2); }); }); diff --git a/src/utils/settingsValidation/storage/storageCS.ts b/src/utils/settingsValidation/storage/storageCS.ts index ad5d42fe..dc47fb6a 100644 --- a/src/utils/settingsValidation/storage/storageCS.ts +++ b/src/utils/settingsValidation/storage/storageCS.ts @@ -1,7 +1,7 @@ import { InMemoryStorageCSFactory } from '../../../storages/inMemory/InMemoryStorageCS'; import { ISettings, SDKMode } from '../../../types'; import { ILogger } from '../../../logger/types'; -import { WARN_STORAGE_INVALID } from '../../../logger/constants'; +import { ERROR_STORAGE_INVALID } from '../../../logger/constants'; import { LOCALHOST_MODE, STANDALONE_MODE, STORAGE_CUSTOM, STORAGE_LOCALSTORAGE, STORAGE_MEMORY } from '../../../utils/constants'; import { IStorageFactoryParams, IStorageSync } from '../../../storages/types'; @@ -17,7 +17,9 @@ __InLocalStorageMockFactory.type = STORAGE_MEMORY; * * @param {any} settings config object provided by the user to initialize the sdk * - * @returns {Object} valid storage factory. It might be the default `InMemoryStorageCSFactory` if the provided storage is invalid. + * @returns {Object} valid storage factory. Default to `InMemoryStorageCSFactory` if the provided storage is invalid or not compatible with the sdk mode if mode is standalone or localhost + * + * @throws error if mode is consumer and the provided storage is not compatible */ export function validateStorageCS(settings: { log: ILogger, storage?: any, mode: SDKMode }): ISettings['storage'] { let { storage = InMemoryStorageCSFactory, log, mode } = settings; @@ -25,7 +27,7 @@ export function validateStorageCS(settings: { log: ILogger, storage?: any, mode: // If an invalid storage is provided, fallback into MEMORY if (typeof storage !== 'function' || [STORAGE_MEMORY, STORAGE_LOCALSTORAGE, STORAGE_CUSTOM].indexOf(storage.type) === -1) { storage = InMemoryStorageCSFactory; - log.warn(WARN_STORAGE_INVALID); + log.error(ERROR_STORAGE_INVALID); } // In localhost mode with InLocalStorage, fallback to a mock InLocalStorage to emit SDK_READY_FROM_CACHE @@ -33,13 +35,15 @@ export function validateStorageCS(settings: { log: ILogger, storage?: any, mode: return __InLocalStorageMockFactory; } - // @TODO check behaviour if ([LOCALHOST_MODE, STANDALONE_MODE].indexOf(mode) === -1) { // Consumer modes require an async storage - if (storage.type !== STORAGE_CUSTOM) throw new Error('A CustomStorage instance is required on consumer modes'); + if (storage.type !== STORAGE_CUSTOM) throw new Error('A CustomStorage instance is required on consumer mode'); } else { // Standalone and localhost modes require a sync storage - if (storage.type === STORAGE_CUSTOM) throw new Error('A CustomStorage instance cannot be used on standalone and localhost modes'); + if (storage.type === STORAGE_CUSTOM) { + storage = InMemoryStorageCSFactory; + log.error(ERROR_STORAGE_INVALID, ['. The provided storage requires to set consumer mode']); + } } // return default InMemory storage if provided one is not valid From e655d718c163dd417aafb062f3a5c97763c4ea8e Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 12 Nov 2021 16:21:58 -0300 Subject: [PATCH 5/7] polishing --- src/logger/messages/error.ts | 2 +- src/utils/settingsValidation/storage/storageCS.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/logger/messages/error.ts b/src/logger/messages/error.ts index 2ce2d3a6..fee6c423 100644 --- a/src/logger/messages/error.ts +++ b/src/logger/messages/error.ts @@ -31,5 +31,5 @@ export const codesError: [number, string][] = [ // 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.'], - [c.ERROR_STORAGE_INVALID, c.LOG_PREFIX_SETTINGS+': The provided storage is invalid. Fallbacking into default MEMORY storage%s'], + [c.ERROR_STORAGE_INVALID, c.LOG_PREFIX_SETTINGS+': The provided storage is invalid.%s Fallbacking into default MEMORY storage'], ]; diff --git a/src/utils/settingsValidation/storage/storageCS.ts b/src/utils/settingsValidation/storage/storageCS.ts index dc47fb6a..3dadb31a 100644 --- a/src/utils/settingsValidation/storage/storageCS.ts +++ b/src/utils/settingsValidation/storage/storageCS.ts @@ -42,7 +42,7 @@ export function validateStorageCS(settings: { log: ILogger, storage?: any, mode: // Standalone and localhost modes require a sync storage if (storage.type === STORAGE_CUSTOM) { storage = InMemoryStorageCSFactory; - log.error(ERROR_STORAGE_INVALID, ['. The provided storage requires to set consumer mode']); + log.error(ERROR_STORAGE_INVALID, [' It requires consumer mode.']); } } From 510b03a7937dd04a8001081c4e1cd9b95fda5a5f Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 17 Nov 2021 11:33:59 -0300 Subject: [PATCH 6/7] polishing --- src/sdkFactory/index.ts | 2 +- src/services/splitApi.ts | 5 ++++- src/storages/inRedis/index.ts | 2 +- src/storages/pluggable/index.ts | 2 +- src/storages/types.ts | 2 +- src/trackers/impressionObserver/ImpressionObserver.ts | 2 +- src/types.ts | 2 +- 7 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/sdkFactory/index.ts b/src/sdkFactory/index.ts index 55359017..136cc064 100644 --- a/src/sdkFactory/index.ts +++ b/src/sdkFactory/index.ts @@ -40,7 +40,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO. matchingKey: getMatching(settings.core.key), splitFiltersValidation: settings.sync.__splitFiltersValidation, - // ATM, only used by CustomStorage. true for partial consumer mode + // ATM, only used by CustomStorage mode: settings.mode, // Callback used to emit SDK_READY in consumer mode, where `syncManagerFactory` is undefined diff --git a/src/services/splitApi.ts b/src/services/splitApi.ts index a080be33..a1856a8e 100644 --- a/src/services/splitApi.ts +++ b/src/services/splitApi.ts @@ -16,7 +16,10 @@ function userKeyToQueryParam(userKey: string) { * @param settings validated settings object * @param platform object containing environment-specific `getFetch` and `getOptions` dependencies */ -export function splitApiFactory(settings: ISettings, platform: Pick): ISplitApi { +export function splitApiFactory( + settings: Pick, + platform: Pick +): ISplitApi { const urls = settings.urls; const filterQueryString = settings.sync.__splitFiltersValidation && settings.sync.__splitFiltersValidation.queryString; diff --git a/src/storages/inRedis/index.ts b/src/storages/inRedis/index.ts index b13413ee..68b0a85b 100644 --- a/src/storages/inRedis/index.ts +++ b/src/storages/inRedis/index.ts @@ -30,7 +30,7 @@ export function InRedisStorage(options: InRedisStorageOptions = {}): IStorageAsy // subscription to Redis connect event in order to emit SDK_READY event on consumer mode redisClient.on('connect', () => { - if (onReadyCb) onReadyCb(); + onReadyCb(); }); return { diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index 9f4c3b30..e1ddaf67 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -68,7 +68,7 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn const wrapper = wrapperAdapter(log, options.wrapper); const isPartialConsumer = mode === CONSUMER_PARTIAL_MODE; - // emit SDK_READY event on main client + // Connects to wrapper and emits SDK_READY event on main client wrapperConnect(wrapper, onReadyCb); return { diff --git a/src/storages/types.ts b/src/storages/types.ts index 05c6d651..5fdb3d31 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -430,7 +430,7 @@ export interface IStorageFactoryParams { matchingKey?: string, /* undefined on server-side SDKs */ splitFiltersValidation?: ISplitFiltersValidation, - // ATM, only used by CustomStorage. True for partial consumer mode + // ATM, only used by CustomStorage mode?: SDKMode, // This callback is invoked when the storage is ready to be used. Error-first callback style: if an error is passed, diff --git a/src/trackers/impressionObserver/ImpressionObserver.ts b/src/trackers/impressionObserver/ImpressionObserver.ts index 666fa463..8ec9d260 100644 --- a/src/trackers/impressionObserver/ImpressionObserver.ts +++ b/src/trackers/impressionObserver/ImpressionObserver.ts @@ -2,7 +2,7 @@ import { ImpressionDTO } from '../../types'; import LRUCache from '../../utils/LRUCache'; import { IImpressionObserver } from './types'; -export default class ImpressionObserver implements IImpressionObserver { +export default class ImpressionObserver implements IImpressionObserver { private cache: LRUCache; private hasher: (impression: ImpressionDTO) => K; diff --git a/src/types.ts b/src/types.ts index 94e6d461..c7fbc62b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -93,7 +93,7 @@ export interface ISettings { auth: string, streaming: string }, - readonly debug: boolean | LogLevel, + readonly debug: boolean | LogLevel | ILogger, readonly version: string, features: SplitIO.MockedFeaturesFilePath | SplitIO.MockedFeaturesMap, readonly streamingEnabled: boolean, From 133d0573745bc3f0e4d60ff3e5c07957322cc659 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 17 Nov 2021 18:00:58 -0300 Subject: [PATCH 7/7] renamed wrapper.close to wrapper.disconnect --- src/storages/pluggable/__tests__/index.spec.ts | 4 ++-- src/storages/pluggable/__tests__/wrapper.mock.ts | 6 +++--- src/storages/pluggable/__tests__/wrapperAdapter.spec.ts | 4 ++-- src/storages/pluggable/inMemoryWrapper.ts | 4 ++-- src/storages/pluggable/index.ts | 6 +++--- src/storages/pluggable/wrapperAdapter.ts | 2 +- src/storages/types.ts | 6 +++--- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/storages/pluggable/__tests__/index.spec.ts b/src/storages/pluggable/__tests__/index.spec.ts index 2ee8bc70..34a95fca 100644 --- a/src/storages/pluggable/__tests__/index.spec.ts +++ b/src/storages/pluggable/__tests__/index.spec.ts @@ -46,7 +46,7 @@ describe('PLUGGABLE STORAGE', () => { await storage.destroy(); await sharedStorage.destroy(); - expect(wrapperMock.close).toBeCalledTimes(1); // wrapper close method should be called once when storage is destroyed + expect(wrapperMock.disconnect).toBeCalledTimes(1); // wrapper disconnect method should be called once when storage is destroyed expect(internalSdkParams.onReadyCb).toBeCalledTimes(1); // onReady callback should be called when the wrapper connect resolved with true expect(sharedOnReadyCb).toBeCalledTimes(1); @@ -65,7 +65,7 @@ describe('PLUGGABLE STORAGE', () => { // Throws exception if the given object is not a valid wrapper, informing which methods are missing const invalidWrapper = wrapperMockFactory(); invalidWrapper.connect = undefined; - invalidWrapper.close = 'invalid function'; + invalidWrapper.disconnect = 'invalid function'; const errorNoValidWrapperInterface = 'The provided wrapper instance doesn’t follow the expected interface. Check our docs.'; expect(() => PluggableStorage({ wrapper: invalidWrapper })).toThrow(errorNoValidWrapperInterface); expect(() => PluggableStorage({ wrapper: {} })).toThrow(errorNoValidWrapperInterface); diff --git a/src/storages/pluggable/__tests__/wrapper.mock.ts b/src/storages/pluggable/__tests__/wrapper.mock.ts index da4b2122..3301ef5c 100644 --- a/src/storages/pluggable/__tests__/wrapper.mock.ts +++ b/src/storages/pluggable/__tests__/wrapper.mock.ts @@ -103,9 +103,9 @@ export function wrapperMockFactory() { return Promise.reject('key is not a set'); }), - // always connects and close + // always connects and disconnects connect: jest.fn(() => Promise.resolve()), - close: jest.fn(() => Promise.resolve()), + disconnect: jest.fn(() => Promise.resolve()), mockClear() { this._cache = {}; @@ -117,7 +117,7 @@ export function wrapperMockFactory() { this.decr.mockClear(); this.getMany.mockClear(); this.connect.mockClear(); - this.close.mockClear(); + this.disconnect.mockClear(); this.getAndSet.mockClear(); this.pushItems.mockClear(); this.popItems.mockClear(); diff --git a/src/storages/pluggable/__tests__/wrapperAdapter.spec.ts b/src/storages/pluggable/__tests__/wrapperAdapter.spec.ts index dd081b6f..366ad1e4 100644 --- a/src/storages/pluggable/__tests__/wrapperAdapter.spec.ts +++ b/src/storages/pluggable/__tests__/wrapperAdapter.spec.ts @@ -28,7 +28,7 @@ export const wrapperWithIssues = { decr: 'invalid value', getMany: throwsException, connect: rejectedPromise, - close: invalidThenable, + disconnect: invalidThenable, getAndSet: 'invalid value', pushItems: rejectedPromise, popItems: invalidThenable, @@ -48,7 +48,7 @@ const VALID_METHOD_CALLS = { 'decr': ['some_key'], 'getMany': [['some_key_1', 'some_key_2']], 'connect': [], - 'close': [], + 'disconnect': [], 'getAndSet': ['some_key', 'some_value'], 'pushItems': ['some_key_list', ['item1', 'item2']], 'popItems': ['some_key_list'], diff --git a/src/storages/pluggable/inMemoryWrapper.ts b/src/storages/pluggable/inMemoryWrapper.ts index b72de2ff..3f050a16 100644 --- a/src/storages/pluggable/inMemoryWrapper.ts +++ b/src/storages/pluggable/inMemoryWrapper.ts @@ -112,7 +112,7 @@ export function inMemoryWrapperFactory(connDelay?: number): ICustomStorageWrappe return Promise.reject('key is not a set'); }, - // always connects and close + // always connects and disconnects connect() { if (typeof _connDelay === 'number') { return new Promise(res => setTimeout(res, _connDelay)); @@ -120,7 +120,7 @@ export function inMemoryWrapperFactory(connDelay?: number): ICustomStorageWrappe return Promise.resolve(); } }, - close() { return Promise.resolve(); }, + disconnect() { return Promise.resolve(); }, // for testing _setConnDelay(connDelay: number) { diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index e1ddaf67..f0d202f1 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -79,9 +79,9 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn events: isPartialConsumer ? promisifyEventsTrack(new EventsCacheInMemory(eventsQueueSize)) : new EventsCachePluggable(log, keys.buildEventsKey(), wrapper, metadata), // @TODO add telemetry cache when required - // Disconnect the underlying storage, to release its resources (such as open files, database connections, etc). + // Disconnect the underlying storage destroy() { - return wrapper.close(); + return wrapper.disconnect(); }, // emits SDK_READY event on shared clients and returns a reference to the storage @@ -89,7 +89,7 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn wrapperConnect(wrapper, onReadyCb); return { ...this, - // no-op destroy, to close the wrapper only when the main client is destroyed + // no-op destroy, to disconnect the wrapper only when the main client is destroyed destroy() { } }; } diff --git a/src/storages/pluggable/wrapperAdapter.ts b/src/storages/pluggable/wrapperAdapter.ts index 0b93f494..3b530d21 100644 --- a/src/storages/pluggable/wrapperAdapter.ts +++ b/src/storages/pluggable/wrapperAdapter.ts @@ -19,7 +19,7 @@ export const METHODS_TO_PROMISE_WRAP: string[] = [ 'removeItems', 'getItems', 'connect', - 'close' + 'disconnect' ]; /** diff --git a/src/storages/types.ts b/src/storages/types.ts index 5fdb3d31..bbd1c2c7 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -177,15 +177,15 @@ export interface ICustomStorageWrapper { */ connect: () => Promise /** - * Disconnects the underlying storage. + * Disconnects from the underlying storage. * It is meant for storages that requires to be closed, in order to release resources. Otherwise it can just return a resolved promise. * Note: will be called once on SplitFactory main client destroy. * - * @function close + * @function disconnect * @returns {Promise} A promise that resolves when the operation ends. * The promise never rejects. */ - close: () => Promise + disconnect: () => Promise } /** Splits cache */