From 981a1c0c92c9e616a4390615ea5caa453d72a788 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 4 Nov 2021 17:48:03 -0300 Subject: [PATCH 01/18] implementation --- README.md | 2 +- src/sdkClient/sdkClientMethodCS.ts | 12 +++- src/sdkClient/sdkClientMethodCSWithTT.ts | 14 +++-- src/sdkFactory/index.ts | 13 ++--- .../inLocalStorage/__tests__/index.spec.ts | 2 +- src/storages/inLocalStorage/index.ts | 16 ++++-- src/storages/inMemory/InMemoryStorage.ts | 2 +- src/storages/inMemory/InMemoryStorageCS.ts | 6 +- src/storages/inRedis/index.ts | 2 +- .../pluggable/__tests__/index.spec.ts | 2 +- src/storages/pluggable/index.ts | 10 +++- src/storages/types.ts | 40 ++++++-------- src/sync/submitters/submitterManager.ts | 22 ++++++++ src/sync/syncManagerOnline.ts | 55 ++++++++----------- src/types.ts | 7 ++- src/utils/settingsValidation/index.ts | 3 +- .../settingsValidation/storage/storageCS.ts | 8 +-- 17 files changed, 122 insertions(+), 94 deletions(-) create mode 100644 src/sync/submitters/submitterManager.ts diff --git a/README.md b/README.md index 51caf41d..5ebcc78a 100644 --- a/README.md +++ b/README.md @@ -30,8 +30,8 @@ Split has built and maintains SDKs for: * iOS [Github](https://github.com/splitio/ios-client) [Docs](https://help.split.io/hc/en-us/articles/360020401491-iOS-SDK) * Java [Github](https://github.com/splitio/java-client) [Docs](https://help.split.io/hc/en-us/articles/360020405151-Java-SDK) * Javascript [Github](https://github.com/splitio/javascript-client) [Docs](https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK) -* Node [Github](https://github.com/splitio/javascript-client) [Docs](https://help.split.io/hc/en-us/articles/360020564931-Node-js-SDK) * Javascript for Browser [Github](https://github.com/splitio/javascript-browser-client) [Docs](https://help.split.io/hc/en-us/articles/360058730852-Browser-SDK) +* Node [Github](https://github.com/splitio/javascript-client) [Docs](https://help.split.io/hc/en-us/articles/360020564931-Node-js-SDK) * PHP [Github](https://github.com/splitio/php-client) [Docs](https://help.split.io/hc/en-us/articles/360020350372-PHP-SDK) * Python [Github](https://github.com/splitio/python-client) [Docs](https://help.split.io/hc/en-us/articles/360020359652-Python-SDK) * React [Github](https://github.com/splitio/react-client) [Docs](https://help.split.io/hc/en-us/articles/360038825091-React-SDK) diff --git a/src/sdkClient/sdkClientMethodCS.ts b/src/sdkClient/sdkClientMethodCS.ts index aeb29720..8939b8af 100644 --- a/src/sdkClient/sdkClientMethodCS.ts +++ b/src/sdkClient/sdkClientMethodCS.ts @@ -4,7 +4,7 @@ import { SplitIO } from '../types'; import { validateKey } from '../utils/inputValidation/key'; import { getMatching, keyParser } from '../utils/key'; import { sdkClientFactory } from './sdkClient'; -import { IStorageSyncCS } from '../storages/types'; +import { IStorageSync } from '../storages/types'; import { ISyncManagerCS } from '../sync/types'; import objectAssign from 'object-assign'; import { RETRIEVE_CLIENT_DEFAULT, NEW_SHARED_CLIENT, RETRIEVE_CLIENT_EXISTING } from '../logger/constants'; @@ -58,8 +58,14 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? const matchingKey = getMatching(validKey); const sharedSdkReadiness = sdkReadinessManager.shared(readyTimeout); - const sharedStorage = (storage as IStorageSyncCS).shared(matchingKey); - const sharedSyncManager = syncManager && (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage); + // @TODO remove shared method to unify storage interfaces + const sharedStorage = storage.shared ? storage.shared(matchingKey) : undefined; + + // Following type assertions are safe: if syncManager and sharedStorage are defined (standalone mode), they implement ISyncManagerCS and IStorageSync respectively + // Other options are: + // - Consumer mode: both syncManager and sharedStorage are defined + // - Consumer mode with submitters: only syncManager is defined + const sharedSyncManager = syncManager && sharedStorage ? (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage as IStorageSync) : undefined; // As shared clients reuse all the storage information, we don't need to check here if we // will use offline or online mode. We should stick with the original decision. diff --git a/src/sdkClient/sdkClientMethodCSWithTT.ts b/src/sdkClient/sdkClientMethodCSWithTT.ts index 684f3a5b..c2f3ef4b 100644 --- a/src/sdkClient/sdkClientMethodCSWithTT.ts +++ b/src/sdkClient/sdkClientMethodCSWithTT.ts @@ -5,7 +5,7 @@ import { validateKey } from '../utils/inputValidation/key'; import { validateTrafficType } from '../utils/inputValidation/trafficType'; import { getMatching, keyParser } from '../utils/key'; import { sdkClientFactory } from './sdkClient'; -import { IStorageSyncCS } from '../storages/types'; +import { IStorageSync } from '../storages/types'; import { ISyncManagerCS } from '../sync/types'; import objectAssign from 'object-assign'; import { RETRIEVE_CLIENT_DEFAULT, NEW_SHARED_CLIENT, RETRIEVE_CLIENT_EXISTING } from '../logger/constants'; @@ -72,8 +72,14 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? const matchingKey = getMatching(validKey); const sharedSdkReadiness = sdkReadinessManager.shared(readyTimeout); - const sharedStorage = (storage as IStorageSyncCS).shared(matchingKey); - const sharedSyncManager = (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage); + // @TODO remove shared method to unify storage interfaces + const sharedStorage = storage.shared ? storage.shared(matchingKey) : undefined; + + // Following type assertions are safe: if syncManager and sharedStorage are defined (standalone mode), they implement ISyncManagerCS and IStorageSync respectively + // Other options are: + // - Consumer mode: both syncManager and sharedStorage are defined + // - Consumer mode with submitters: only syncManager is defined + const sharedSyncManager = syncManager && sharedStorage ? (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage as IStorageSync) : undefined; // As shared clients reuse all the storage information, we don't need to check here if we // will use offline or online mode. We should stick with the original decision. @@ -89,7 +95,7 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? validTrafficType ); - sharedSyncManager.start(); + sharedSyncManager && sharedSyncManager.start(); log.info(NEW_SHARED_CLIENT); } else { diff --git a/src/sdkFactory/index.ts b/src/sdkFactory/index.ts index 0f89f516..295b3dac 100644 --- a/src/sdkFactory/index.ts +++ b/src/sdkFactory/index.ts @@ -31,14 +31,13 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO. const sdkReadinessManager = sdkReadinessManagerFactory(log, platform.EventEmitter, settings.startup.readyTimeout); const readinessManager = sdkReadinessManager.readinessManager; - // @TODO consider passing the settings object, so that each storage access only what it needs + const matchingKey = getMatching(settings.core.key); + const storageFactoryParams: IStorageFactoryParams = { - eventsQueueSize: settings.scheduler.eventsQueueSize, + settings, optimize: shouldBeOptimized(settings), - - // ATM, only used by InLocalStorage - matchingKey: getMatching(settings.core.key), - splitFiltersValidation: settings.sync.__splitFiltersValidation, + matchingKey, + metadata: metadataBuilder(settings), // Callback used in consumer mode (`syncManagerFactory` is undefined) to emit SDK_READY onReadyCb: !syncManagerFactory ? (error) => { @@ -46,8 +45,6 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO. readinessManager.splits.emit(SDK_SPLITS_ARRIVED); readinessManager.segments.emit(SDK_SEGMENTS_ARRIVED); } : undefined, - metadata: metadataBuilder(settings), - log }; const storage = storageFactory(storageFactoryParams); diff --git a/src/storages/inLocalStorage/__tests__/index.spec.ts b/src/storages/inLocalStorage/__tests__/index.spec.ts index 417c8727..faa5157b 100644 --- a/src/storages/inLocalStorage/__tests__/index.spec.ts +++ b/src/storages/inLocalStorage/__tests__/index.spec.ts @@ -16,7 +16,7 @@ import { InLocalStorage } from '../index'; describe('IN LOCAL STORAGE', () => { // @ts-ignore - const internalSdkParams: IStorageFactoryParams = { log: loggerMock }; + const internalSdkParams: IStorageFactoryParams = { settings: { log: loggerMock, scheduler: {}, sync: {} } }; afterEach(() => { fakeInMemoryStorageFactory.mockClear(); diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 6587208b..72e8f8be 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -1,7 +1,7 @@ import ImpressionsCacheInMemory from '../inMemory/ImpressionsCacheInMemory'; import ImpressionCountsCacheInMemory from '../inMemory/ImpressionCountsCacheInMemory'; import EventsCacheInMemory from '../inMemory/EventsCacheInMemory'; -import { IStorageFactoryParams, IStorageSyncCS, IStorageSyncFactory } from '../types'; +import { IStorageFactoryParams, IStorageSync, IStorageSyncFactory } from '../types'; import { validatePrefix } from '../KeyBuilder'; import KeyBuilderCS from '../KeyBuilderCS'; import { isLocalStorageAvailable } from '../../utils/env/isLocalStorageAvailable'; @@ -25,24 +25,28 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn const prefix = validatePrefix(options.prefix); - function InLocalStorageCSFactory(params: IStorageFactoryParams): IStorageSyncCS { + function InLocalStorageCSFactory(params: IStorageFactoryParams): IStorageSync { + const { + log, + scheduler: { eventsQueueSize }, + sync: { __splitFiltersValidation } + } = params.settings; // Fallback to InMemoryStorage if LocalStorage API is not available if (!isLocalStorageAvailable()) { - params.log.warn(LOG_PREFIX + 'LocalStorage API is unavailable. Fallbacking into default MEMORY storage'); + log.warn(LOG_PREFIX + 'LocalStorage API is unavailable. Fallbacking into default MEMORY storage'); return InMemoryStorageCSFactory(params); } - const log = params.log; const keys = new KeyBuilderCS(prefix, params.matchingKey as string); const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS; return { - splits: new SplitsCacheInLocal(log, keys, expirationTimestamp, params.splitFiltersValidation), + splits: new SplitsCacheInLocal(log, keys, expirationTimestamp, __splitFiltersValidation), segments: new MySegmentsCacheInLocal(log, keys), impressions: new ImpressionsCacheInMemory(), impressionCounts: params.optimize ? new ImpressionCountsCacheInMemory() : undefined, - events: new EventsCacheInMemory(params.eventsQueueSize), + events: new EventsCacheInMemory(eventsQueueSize), destroy() { this.splits = new SplitsCacheInMemory(); diff --git a/src/storages/inMemory/InMemoryStorage.ts b/src/storages/inMemory/InMemoryStorage.ts index 349ac6e1..2ae9c75a 100644 --- a/src/storages/inMemory/InMemoryStorage.ts +++ b/src/storages/inMemory/InMemoryStorage.ts @@ -21,7 +21,7 @@ export function InMemoryStorageFactory(params: IStorageFactoryParams): IStorageS segments: new SegmentsCacheInMemory(), impressions: new ImpressionsCacheInMemory(), impressionCounts: params.optimize ? new ImpressionCountsCacheInMemory() : undefined, - events: new EventsCacheInMemory(params.eventsQueueSize), + events: new EventsCacheInMemory(params.settings.scheduler.eventsQueueSize), // When using MEMORY we should clean all the caches to leave them empty destroy() { diff --git a/src/storages/inMemory/InMemoryStorageCS.ts b/src/storages/inMemory/InMemoryStorageCS.ts index d59e4c04..a4538a9b 100644 --- a/src/storages/inMemory/InMemoryStorageCS.ts +++ b/src/storages/inMemory/InMemoryStorageCS.ts @@ -2,7 +2,7 @@ import SplitsCacheInMemory from './SplitsCacheInMemory'; import MySegmentsCacheInMemory from './MySegmentsCacheInMemory'; import ImpressionsCacheInMemory from './ImpressionsCacheInMemory'; import EventsCacheInMemory from './EventsCacheInMemory'; -import { IStorageSyncCS, IStorageFactoryParams } from '../types'; +import { IStorageSync, IStorageFactoryParams } from '../types'; import ImpressionCountsCacheInMemory from './ImpressionCountsCacheInMemory'; import { STORAGE_MEMORY } from '../../utils/constants'; @@ -11,14 +11,14 @@ import { STORAGE_MEMORY } from '../../utils/constants'; * * @param params parameters required by EventsCacheSync */ -export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorageSyncCS { +export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorageSync { return { splits: new SplitsCacheInMemory(), segments: new MySegmentsCacheInMemory(), impressions: new ImpressionsCacheInMemory(), impressionCounts: params.optimize ? new ImpressionCountsCacheInMemory() : undefined, - events: new EventsCacheInMemory(params.eventsQueueSize), + events: new EventsCacheInMemory(params.settings.scheduler.eventsQueueSize), // When using MEMORY we should clean all the caches to leave them empty destroy() { diff --git a/src/storages/inRedis/index.ts b/src/storages/inRedis/index.ts index b13413ee..8a5ab832 100644 --- a/src/storages/inRedis/index.ts +++ b/src/storages/inRedis/index.ts @@ -23,7 +23,7 @@ export function InRedisStorage(options: InRedisStorageOptions = {}): IStorageAsy const prefix = validatePrefix(options.prefix); - function InRedisStorageFactory({ log, metadata, onReadyCb }: IStorageFactoryParams): IStorageAsync { + function InRedisStorageFactory({ settings: { log }, metadata, onReadyCb }: IStorageFactoryParams): IStorageAsync { const keys = new KeyBuilderSS(prefix, metadata); const redisClient = new RedisAdapter(log, options.options || {}); diff --git a/src/storages/pluggable/__tests__/index.spec.ts b/src/storages/pluggable/__tests__/index.spec.ts index 8356a3ad..fbfe7121 100644 --- a/src/storages/pluggable/__tests__/index.spec.ts +++ b/src/storages/pluggable/__tests__/index.spec.ts @@ -15,7 +15,7 @@ import { assertStorageInterface } from '../../__tests__/testUtils'; describe('PLUGGABLE STORAGE', () => { const internalSdkParams: IStorageFactoryParams = { - log: loggerMock, + settings: { sync: {}, log: loggerMock }, metadata, onReadyCb: jest.fn() }; diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index 2afb1bf4..71b9fb46 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -9,6 +9,8 @@ import { wrapperAdapter, METHODS_TO_PROMISE_WRAP } from './wrapperAdapter'; import { isObject } from '../../utils/lang'; import { validatePrefix } from '../KeyBuilder'; import { 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,7 +43,9 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn const prefix = validatePrefix(options.prefix); - function PluggableStorageFactory({ log, metadata, onReadyCb }: IStorageFactoryParams): IStorageAsync { + function PluggableStorageFactory({ settings, metadata, onReadyCb }: IStorageFactoryParams): IStorageAsync { + const log = settings.log; + const onlyImpressionsAndEvents = settings.sync.onlyImpressionsAndEvents; const keys = new KeyBuilderSS(prefix, metadata); const wrapper = wrapperAdapter(log, options.wrapper); @@ -55,8 +59,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: onlyImpressionsAndEvents ? new ImpressionsCacheInMemory() : new ImpressionsCachePluggable(log, keys.buildImpressionsKey(), wrapper, metadata), + events: onlyImpressionsAndEvents ? new EventsCacheInMemory() : 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 288e1fea..080074a9 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -1,7 +1,6 @@ -import { MaybeThenable, IMetadata, ISplitFiltersValidation } from '../dtos/types'; -import { ILogger } from '../logger/types'; +import { MaybeThenable, IMetadata } from '../dtos/types'; import { StoredEventWithMetadata, StoredImpressionWithMetadata } from '../sync/submitters/types'; -import { SplitIO, ImpressionDTO } from '../types'; +import { SplitIO, ImpressionDTO, ISettings } from '../types'; /** * Interface of a custom wrapper storage. @@ -28,7 +27,7 @@ export interface ICustomStorageWrapper { * @returns {Promise} A promise that resolves if the operation success, whether the key was added or updated. * The promise rejects if the operation fails. */ - set: (key: string, value: string) => Promise + set: (key: string, value: string) => Promise /** * Add or update an item with a specified `key` and `value`. * @@ -47,7 +46,7 @@ export interface ICustomStorageWrapper { * @returns {Promise} A promise that resolves if the operation success, whether the key existed and was removed or it didn't exist. * The promise rejects if the operation fails, for example, if there is a connection error. */ - del: (key: string) => Promise + del: (key: string) => Promise /** * Returns all keys matching the given prefix. * @@ -140,20 +139,20 @@ export interface ICustomStorageWrapper { * @function addItems * @param {string} key Set key * @param {string} items Items to add - * @returns {Promise} A promise that resolves if the operation success. + * @returns {Promise} A promise that resolves if the operation success. * The promise rejects if the operation fails, for example, if there is a connection error or the key holds a value that is not a set. */ - addItems: (key: string, items: string[]) => Promise + addItems: (key: string, items: string[]) => Promise /** * Remove the specified `items` from the set stored at `key`. Those items that are not part of the set are ignored. * * @function removeItems * @param {string} key Set key * @param {string} items Items to remove - * @returns {Promise} A promise that resolves if the operation success. If key does not exist, the promise also resolves. + * @returns {Promise} A promise that resolves if the operation success. If key does not exist, the promise also resolves. * The promise rejects if the operation fails, for example, if there is a connection error or the key holds a value that is not a set. */ - removeItems: (key: string, items: string[]) => Promise + removeItems: (key: string, items: string[]) => Promise /** * Returns all the items of the `key` set. * @@ -271,7 +270,7 @@ export interface ISegmentsCacheSync extends ISegmentsCacheBase { export interface ISegmentsCacheAsync extends ISegmentsCacheBase { addToSegment(name: string, segmentKeys: string[]): Promise removeFromSegment(name: string, segmentKeys: string[]): Promise - isInSegment(name: string, key?: string): Promise + isInSegment(name: string, key: string): Promise registerSegments(names: string[]): Promise getRegisteredSegments(): Promise setChangeNumber(name: string, changeNumber: number): Promise @@ -396,6 +395,7 @@ export interface IStorageBase< latencies?: TLatenciesCache, counts?: TCountsCache, destroy(): void, + shared?(matchingKey: string): this } export type IStorageSync = IStorageBase< @@ -407,15 +407,11 @@ export type IStorageSync = IStorageBase< ICountsCacheSync > -export interface IStorageSyncCS extends IStorageSync { - shared(matchingKey: string): IStorageSync -} - export type IStorageAsync = IStorageBase< ISplitsCacheAsync, ISegmentsCacheAsync, - IImpressionsCacheAsync, - IEventsCacheAsync, + IImpressionsCacheAsync | IImpressionsCacheSync, + IEventsCacheAsync | IEventsCacheSync, ILatenciesCacheAsync, ICountsCacheAsync > @@ -425,19 +421,17 @@ export type IStorageAsync = IStorageBase< export type DataLoader = (storage: IStorageSync, matchingKey: string) => void export interface IStorageFactoryParams { - log: ILogger, - eventsQueueSize?: number, - optimize?: boolean /* whether create the `impressionCounts` cache (OPTIMIZED impression mode) or not (DEBUG impression mode) */, + settings: ISettings, - // ATM, only used by InLocalStorage - matchingKey?: string, /* undefined on server-side SDKs */ - splitFiltersValidation?: ISplitFiltersValidation, + // Properties derived from settings + optimize?: boolean /* whether create the `impressionCounts` cache (OPTIMIZED impression mode) or not (DEBUG impression mode) */, + matchingKey?: string, /* undefined on server-side SDKs. ATM, only used by InLocalStorage */ + metadata: IMetadata, // 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. onReadyCb?: (error?: any) => void, - metadata: IMetadata, } export type StorageType = 'MEMORY' | 'LOCALSTORAGE' | 'REDIS' | 'CUSTOM'; 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..d10323d3 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 { 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 mandatory + 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(); @@ -129,8 +122,8 @@ export function syncManagerOnlineFactory( else return Promise.resolve(); }, - // [Only used for client-side] - // It assumes that polling and push managers implement the interfaces for client-side + // Only used for client-side in standalone mode. + // Therefore, polling manager is defined and both polling and push managers implement the interfaces for client-side shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync): ISyncManager { const mySegmentsSyncTask = (pollingManager as IPollingManagerCS).add(matchingKey, readinessManager, storage); @@ -139,7 +132,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/types.ts b/src/types.ts index 691c5beb..c77bd861 100644 --- a/src/types.ts +++ b/src/types.ts @@ -3,7 +3,7 @@ import { IIntegration, IIntegrationFactoryParams } from './integrations/types'; import { ILogger } from './logger/types'; /* eslint-disable no-use-before-define */ -import { IStorageFactoryParams, IStorageSyncCS, IStorageSync, IStorageAsync, IStorageSyncFactory } from './storages/types'; +import { IStorageFactoryParams, IStorageSync, IStorageAsync, IStorageSyncFactory } from './storages/types'; import { ISyncManagerFactoryParams, ISyncManagerCS } from './sync/types'; /** @@ -101,7 +101,8 @@ export interface ISettings { splitFilters: SplitIO.SplitFilter[], impressionsMode: SplitIO.ImpressionsMode, __splitFiltersValidation: ISplitFiltersValidation, - localhostMode?: SplitIO.LocalhostFactory + localhostMode?: SplitIO.LocalhostFactory, + onlyImpressionsAndEvents?: boolean }, readonly runtime: { ip: string | false @@ -841,7 +842,7 @@ export namespace SplitIO { * Defines which kind of storage we should instanciate. * @property {Object} storage */ - storage?: (params: IStorageFactoryParams) => IStorageSyncCS, + storage?: (params: IStorageFactoryParams) => IStorageSync | IStorageAsync, /** * List of URLs that the SDK will use as base for it's synchronization functionalities, applicable only when running as standalone. * Do not change these settings unless you're working an advanced use case, like connecting to the Split proxy. diff --git a/src/utils/settingsValidation/index.ts b/src/utils/settingsValidation/index.ts index 1840fdb5..af75bb76 100644 --- a/src/utils/settingsValidation/index.ts +++ b/src/utils/settingsValidation/index.ts @@ -75,7 +75,8 @@ const base = { splitFilters: undefined, // impressions collection mode impressionsMode: OPTIMIZED, - localhostMode: undefined + localhostMode: undefined, + onlyImpressionsAndEvents: false }, runtime: { diff --git a/src/utils/settingsValidation/storage/storageCS.ts b/src/utils/settingsValidation/storage/storageCS.ts index 9cb762e2..841ca959 100644 --- a/src/utils/settingsValidation/storage/storageCS.ts +++ b/src/utils/settingsValidation/storage/storageCS.ts @@ -2,10 +2,10 @@ 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_LOCALSTORAGE, STORAGE_MEMORY } from '../../../utils/constants'; -import { IStorageFactoryParams, IStorageSyncCS } from '../../../storages/types'; +import { LOCALHOST_MODE, STORAGE_CUSTOM, STORAGE_LOCALSTORAGE, STORAGE_MEMORY } from '../../../utils/constants'; +import { IStorageFactoryParams, IStorageSync } from '../../../storages/types'; -export function __InLocalStorageMockFactory(params: IStorageFactoryParams): IStorageSyncCS { +export function __InLocalStorageMockFactory(params: IStorageFactoryParams): IStorageSync { const result = InMemoryStorageCSFactory(params); result.splits.checkCache = () => true; // to emit SDK_READY_FROM_CACHE return result; @@ -23,7 +23,7 @@ export function validateStorageCS(settings: { log: ILogger, storage?: any, mode? let { storage = InMemoryStorageCSFactory, log, mode } = settings; // If an invalid storage is provided, fallback into MEMORY - if (typeof storage !== 'function' || storage.type !== STORAGE_MEMORY && storage.type !== STORAGE_LOCALSTORAGE) { + if (typeof storage !== 'function' || [STORAGE_MEMORY, STORAGE_LOCALSTORAGE, STORAGE_CUSTOM].indexOf(storage.type) === -1) { storage = InMemoryStorageCSFactory; log.warn(WARN_STORAGE_INVALID); } From a4584ec38c666d5964effe4cf44eb88ecec0e09e Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 5 Nov 2021 14:01:01 -0300 Subject: [PATCH 02/18] polishing --- src/sdkClient/sdkClientMethodCS.ts | 12 ++++++------ src/sdkClient/sdkClientMethodCSWithTT.ts | 10 +++++----- src/storages/inLocalStorage/index.ts | 4 ++-- src/storages/inMemory/InMemoryStorageCS.ts | 4 ++-- src/storages/types.ts | 5 ++++- 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/sdkClient/sdkClientMethodCS.ts b/src/sdkClient/sdkClientMethodCS.ts index 8939b8af..4e818cde 100644 --- a/src/sdkClient/sdkClientMethodCS.ts +++ b/src/sdkClient/sdkClientMethodCS.ts @@ -4,7 +4,7 @@ import { SplitIO } from '../types'; import { validateKey } from '../utils/inputValidation/key'; import { getMatching, keyParser } from '../utils/key'; import { sdkClientFactory } from './sdkClient'; -import { IStorageSync } from '../storages/types'; +import { IStorageSyncCS } from '../storages/types'; import { ISyncManagerCS } from '../sync/types'; import objectAssign from 'object-assign'; import { RETRIEVE_CLIENT_DEFAULT, NEW_SHARED_CLIENT, RETRIEVE_CLIENT_EXISTING } from '../logger/constants'; @@ -58,14 +58,14 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? const matchingKey = getMatching(validKey); const sharedSdkReadiness = sdkReadinessManager.shared(readyTimeout); - // @TODO remove shared method to unify storage interfaces - const sharedStorage = storage.shared ? storage.shared(matchingKey) : undefined; + // @ts-ignore @TODO remove shared method to unify storage interfaces + const sharedStorage = storage.shared ? (storage as IStorageSyncCS).shared(matchingKey) : undefined; - // Following type assertions are safe: if syncManager and sharedStorage are defined (standalone mode), they implement ISyncManagerCS and IStorageSync respectively + // Next assertions are safe: if syncManager and sharedStorage are defined (standalone mode), they implement ISyncManagerCS and IStorageSyncCS respectively // Other options are: - // - Consumer mode: both syncManager and sharedStorage are defined + // - Consumer mode: both syncManager and sharedStorage are undefined // - Consumer mode with submitters: only syncManager is defined - const sharedSyncManager = syncManager && sharedStorage ? (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage as IStorageSync) : undefined; + const sharedSyncManager = syncManager && sharedStorage ? (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage) : undefined; // As shared clients reuse all the storage information, we don't need to check here if we // will use offline or online mode. We should stick with the original decision. diff --git a/src/sdkClient/sdkClientMethodCSWithTT.ts b/src/sdkClient/sdkClientMethodCSWithTT.ts index c2f3ef4b..12c1d880 100644 --- a/src/sdkClient/sdkClientMethodCSWithTT.ts +++ b/src/sdkClient/sdkClientMethodCSWithTT.ts @@ -5,7 +5,7 @@ import { validateKey } from '../utils/inputValidation/key'; import { validateTrafficType } from '../utils/inputValidation/trafficType'; import { getMatching, keyParser } from '../utils/key'; import { sdkClientFactory } from './sdkClient'; -import { IStorageSync } from '../storages/types'; +import { IStorageSyncCS } from '../storages/types'; import { ISyncManagerCS } from '../sync/types'; import objectAssign from 'object-assign'; import { RETRIEVE_CLIENT_DEFAULT, NEW_SHARED_CLIENT, RETRIEVE_CLIENT_EXISTING } from '../logger/constants'; @@ -72,14 +72,14 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? const matchingKey = getMatching(validKey); const sharedSdkReadiness = sdkReadinessManager.shared(readyTimeout); - // @TODO remove shared method to unify storage interfaces - const sharedStorage = storage.shared ? storage.shared(matchingKey) : undefined; + // @ts-ignore @TODO remove shared method to unify storage interfaces + const sharedStorage = storage.shared ? (storage as IStorageSyncCS).shared(matchingKey) : undefined; // Following type assertions are safe: if syncManager and sharedStorage are defined (standalone mode), they implement ISyncManagerCS and IStorageSync respectively // Other options are: - // - Consumer mode: both syncManager and sharedStorage are defined + // - Consumer mode: both syncManager and sharedStorage are undefined // - Consumer mode with submitters: only syncManager is defined - const sharedSyncManager = syncManager && sharedStorage ? (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage as IStorageSync) : undefined; + const sharedSyncManager = syncManager && sharedStorage ? (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage) : undefined; // As shared clients reuse all the storage information, we don't need to check here if we // will use offline or online mode. We should stick with the original decision. diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 72e8f8be..87cd6dd1 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -1,7 +1,7 @@ import ImpressionsCacheInMemory from '../inMemory/ImpressionsCacheInMemory'; import ImpressionCountsCacheInMemory from '../inMemory/ImpressionCountsCacheInMemory'; import EventsCacheInMemory from '../inMemory/EventsCacheInMemory'; -import { IStorageFactoryParams, IStorageSync, IStorageSyncFactory } from '../types'; +import { IStorageFactoryParams, IStorageSyncCS, IStorageSyncFactory } from '../types'; import { validatePrefix } from '../KeyBuilder'; import KeyBuilderCS from '../KeyBuilderCS'; import { isLocalStorageAvailable } from '../../utils/env/isLocalStorageAvailable'; @@ -25,7 +25,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn const prefix = validatePrefix(options.prefix); - function InLocalStorageCSFactory(params: IStorageFactoryParams): IStorageSync { + function InLocalStorageCSFactory(params: IStorageFactoryParams): IStorageSyncCS { const { log, scheduler: { eventsQueueSize }, diff --git a/src/storages/inMemory/InMemoryStorageCS.ts b/src/storages/inMemory/InMemoryStorageCS.ts index a4538a9b..ba7a1510 100644 --- a/src/storages/inMemory/InMemoryStorageCS.ts +++ b/src/storages/inMemory/InMemoryStorageCS.ts @@ -2,7 +2,7 @@ import SplitsCacheInMemory from './SplitsCacheInMemory'; import MySegmentsCacheInMemory from './MySegmentsCacheInMemory'; import ImpressionsCacheInMemory from './ImpressionsCacheInMemory'; import EventsCacheInMemory from './EventsCacheInMemory'; -import { IStorageSync, IStorageFactoryParams } from '../types'; +import { IStorageSyncCS, IStorageFactoryParams } from '../types'; import ImpressionCountsCacheInMemory from './ImpressionCountsCacheInMemory'; import { STORAGE_MEMORY } from '../../utils/constants'; @@ -11,7 +11,7 @@ import { STORAGE_MEMORY } from '../../utils/constants'; * * @param params parameters required by EventsCacheSync */ -export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorageSync { +export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorageSyncCS { return { splits: new SplitsCacheInMemory(), diff --git a/src/storages/types.ts b/src/storages/types.ts index 080074a9..7d722942 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -395,7 +395,6 @@ export interface IStorageBase< latencies?: TLatenciesCache, counts?: TCountsCache, destroy(): void, - shared?(matchingKey: string): this } export type IStorageSync = IStorageBase< @@ -407,6 +406,10 @@ export type IStorageSync = IStorageBase< ICountsCacheSync > +export interface IStorageSyncCS extends IStorageSync { + shared(matchingKey: string): IStorageSync +} + export type IStorageAsync = IStorageBase< ISplitsCacheAsync, ISegmentsCacheAsync, From f4ebb8fb56b8d038b3690e6dbfba4378ecb18f79 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 5 Nov 2021 15:20:48 -0300 Subject: [PATCH 03/18] revert some refactors --- src/sdkClient/sdkClientMethodCS.ts | 15 ++++++------ src/sdkClient/sdkClientMethodCSWithTT.ts | 15 ++++++------ src/sdkFactory/index.ts | 23 ++++++++++++------- .../inLocalStorage/__tests__/index.spec.ts | 2 +- src/storages/inLocalStorage/index.ts | 12 ++++------ src/storages/inMemory/InMemoryStorage.ts | 5 +--- src/storages/inMemory/InMemoryStorageCS.ts | 2 +- src/storages/inRedis/index.ts | 2 +- .../pluggable/__tests__/index.spec.ts | 2 +- src/storages/pluggable/index.ts | 8 +++---- src/storages/types.ts | 21 +++++++++++------ src/sync/syncManagerOnline.ts | 4 ++-- src/sync/types.ts | 2 ++ src/types.ts | 4 ++-- src/utils/settingsValidation/index.ts | 3 +-- .../settingsValidation/storage/storageCS.ts | 4 ++-- 16 files changed, 64 insertions(+), 60 deletions(-) diff --git a/src/sdkClient/sdkClientMethodCS.ts b/src/sdkClient/sdkClientMethodCS.ts index 4e818cde..0b60f307 100644 --- a/src/sdkClient/sdkClientMethodCS.ts +++ b/src/sdkClient/sdkClientMethodCS.ts @@ -58,14 +58,13 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? const matchingKey = getMatching(validKey); const sharedSdkReadiness = sdkReadinessManager.shared(readyTimeout); - // @ts-ignore @TODO remove shared method to unify storage interfaces - const sharedStorage = storage.shared ? (storage as IStorageSyncCS).shared(matchingKey) : undefined; - - // Next assertions are safe: if syncManager and sharedStorage are defined (standalone mode), they implement ISyncManagerCS and IStorageSyncCS respectively - // Other options are: - // - Consumer mode: both syncManager and sharedStorage are undefined - // - Consumer mode with submitters: only syncManager is defined - const sharedSyncManager = syncManager && sharedStorage ? (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage) : undefined; + + // 3 combinations are possible: + // - Standalone mode: both syncManager and storage.shared are defined + // - Consumer mode: both syncManager and storage.shared are undefined + // - Consumer mode with submitters: syncManager is defined but not storage.shared + const sharedStorage = (storage as IStorageSyncCS).shared && (storage as IStorageSyncCS).shared(matchingKey); + const sharedSyncManager = syncManager && sharedStorage && (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage); // As shared clients reuse all the storage information, we don't need to check here if we // will use offline or online mode. We should stick with the original decision. diff --git a/src/sdkClient/sdkClientMethodCSWithTT.ts b/src/sdkClient/sdkClientMethodCSWithTT.ts index 12c1d880..10e30586 100644 --- a/src/sdkClient/sdkClientMethodCSWithTT.ts +++ b/src/sdkClient/sdkClientMethodCSWithTT.ts @@ -72,14 +72,13 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? const matchingKey = getMatching(validKey); const sharedSdkReadiness = sdkReadinessManager.shared(readyTimeout); - // @ts-ignore @TODO remove shared method to unify storage interfaces - const sharedStorage = storage.shared ? (storage as IStorageSyncCS).shared(matchingKey) : undefined; - - // Following type assertions are safe: if syncManager and sharedStorage are defined (standalone mode), they implement ISyncManagerCS and IStorageSync respectively - // Other options are: - // - Consumer mode: both syncManager and sharedStorage are undefined - // - Consumer mode with submitters: only syncManager is defined - const sharedSyncManager = syncManager && sharedStorage ? (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage) : undefined; + + // 3 combinations are possible: + // - Standalone mode: both syncManager and storage.shared are defined + // - Consumer mode: both syncManager and storage.shared are undefined + // - Consumer mode with submitters: syncManager is defined but not storage.shared + const sharedStorage = (storage as IStorageSyncCS).shared && (storage as IStorageSyncCS).shared(matchingKey); + const sharedSyncManager = syncManager && sharedStorage && (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage); // As shared clients reuse all the storage information, we don't need to check here if we // will use offline or online mode. We should stick with the original decision. diff --git a/src/sdkFactory/index.ts b/src/sdkFactory/index.ts index 295b3dac..550952f8 100644 --- a/src/sdkFactory/index.ts +++ b/src/sdkFactory/index.ts @@ -31,20 +31,27 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO. const sdkReadinessManager = sdkReadinessManagerFactory(log, platform.EventEmitter, settings.startup.readyTimeout); const readinessManager = sdkReadinessManager.readinessManager; - const matchingKey = getMatching(settings.core.key); - + // @TODO consider passing the settings object, so that each storage access only what it needs const storageFactoryParams: IStorageFactoryParams = { - settings, + log, + eventsQueueSize: settings.scheduler.eventsQueueSize, optimize: shouldBeOptimized(settings), - matchingKey, - metadata: metadataBuilder(settings), - // Callback used in consumer mode (`syncManagerFactory` is undefined) to emit SDK_READY - onReadyCb: !syncManagerFactory ? (error) => { + // ATM, only used by InLocalStorage + matchingKey: getMatching(settings.core.key), + splitFiltersValidation: settings.sync.__splitFiltersValidation, + + // ATM, only used by CustomStorage. true for partial consumer mode + trackInMemory: settings.sync.onlyImpressionsAndEvents, + + // 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) => { if (error) return; // don't emit SDK_READY if storage failed to connect. readinessManager.splits.emit(SDK_SPLITS_ARRIVED); readinessManager.segments.emit(SDK_SEGMENTS_ARRIVED); - } : undefined, + }, + metadata: metadataBuilder(settings) }; const storage = storageFactory(storageFactoryParams); diff --git a/src/storages/inLocalStorage/__tests__/index.spec.ts b/src/storages/inLocalStorage/__tests__/index.spec.ts index faa5157b..417c8727 100644 --- a/src/storages/inLocalStorage/__tests__/index.spec.ts +++ b/src/storages/inLocalStorage/__tests__/index.spec.ts @@ -16,7 +16,7 @@ import { InLocalStorage } from '../index'; describe('IN LOCAL STORAGE', () => { // @ts-ignore - const internalSdkParams: IStorageFactoryParams = { settings: { log: loggerMock, scheduler: {}, sync: {} } }; + const internalSdkParams: IStorageFactoryParams = { log: loggerMock }; afterEach(() => { fakeInMemoryStorageFactory.mockClear(); diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 87cd6dd1..6587208b 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -26,27 +26,23 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn const prefix = validatePrefix(options.prefix); function InLocalStorageCSFactory(params: IStorageFactoryParams): IStorageSyncCS { - const { - log, - scheduler: { eventsQueueSize }, - sync: { __splitFiltersValidation } - } = params.settings; // Fallback to InMemoryStorage if LocalStorage API is not available if (!isLocalStorageAvailable()) { - log.warn(LOG_PREFIX + 'LocalStorage API is unavailable. Fallbacking into default MEMORY storage'); + params.log.warn(LOG_PREFIX + 'LocalStorage API is unavailable. Fallbacking into default MEMORY storage'); return InMemoryStorageCSFactory(params); } + const log = params.log; const keys = new KeyBuilderCS(prefix, params.matchingKey as string); const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS; return { - splits: new SplitsCacheInLocal(log, keys, expirationTimestamp, __splitFiltersValidation), + splits: new SplitsCacheInLocal(log, keys, expirationTimestamp, params.splitFiltersValidation), segments: new MySegmentsCacheInLocal(log, keys), impressions: new ImpressionsCacheInMemory(), impressionCounts: params.optimize ? new ImpressionCountsCacheInMemory() : undefined, - events: new EventsCacheInMemory(eventsQueueSize), + events: new EventsCacheInMemory(params.eventsQueueSize), destroy() { this.splits = new SplitsCacheInMemory(); diff --git a/src/storages/inMemory/InMemoryStorage.ts b/src/storages/inMemory/InMemoryStorage.ts index 2ae9c75a..173fbf3a 100644 --- a/src/storages/inMemory/InMemoryStorage.ts +++ b/src/storages/inMemory/InMemoryStorage.ts @@ -13,15 +13,12 @@ import { STORAGE_MEMORY } from '../../utils/constants'; */ export function InMemoryStorageFactory(params: IStorageFactoryParams): IStorageSync { - // InMemory storage is always ready - if (params.onReadyCb) setTimeout(params.onReadyCb); - return { splits: new SplitsCacheInMemory(), segments: new SegmentsCacheInMemory(), impressions: new ImpressionsCacheInMemory(), impressionCounts: params.optimize ? new ImpressionCountsCacheInMemory() : undefined, - events: new EventsCacheInMemory(params.settings.scheduler.eventsQueueSize), + events: new EventsCacheInMemory(params.eventsQueueSize), // When using MEMORY we should clean all the caches to leave them empty destroy() { diff --git a/src/storages/inMemory/InMemoryStorageCS.ts b/src/storages/inMemory/InMemoryStorageCS.ts index ba7a1510..d59e4c04 100644 --- a/src/storages/inMemory/InMemoryStorageCS.ts +++ b/src/storages/inMemory/InMemoryStorageCS.ts @@ -18,7 +18,7 @@ export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorag segments: new MySegmentsCacheInMemory(), impressions: new ImpressionsCacheInMemory(), impressionCounts: params.optimize ? new ImpressionCountsCacheInMemory() : undefined, - events: new EventsCacheInMemory(params.settings.scheduler.eventsQueueSize), + events: new EventsCacheInMemory(params.eventsQueueSize), // When using MEMORY we should clean all the caches to leave them empty destroy() { diff --git a/src/storages/inRedis/index.ts b/src/storages/inRedis/index.ts index 8a5ab832..b13413ee 100644 --- a/src/storages/inRedis/index.ts +++ b/src/storages/inRedis/index.ts @@ -23,7 +23,7 @@ export function InRedisStorage(options: InRedisStorageOptions = {}): IStorageAsy const prefix = validatePrefix(options.prefix); - function InRedisStorageFactory({ settings: { log }, metadata, onReadyCb }: IStorageFactoryParams): IStorageAsync { + function InRedisStorageFactory({ log, metadata, onReadyCb }: IStorageFactoryParams): IStorageAsync { const keys = new KeyBuilderSS(prefix, metadata); const redisClient = new RedisAdapter(log, options.options || {}); diff --git a/src/storages/pluggable/__tests__/index.spec.ts b/src/storages/pluggable/__tests__/index.spec.ts index fbfe7121..8356a3ad 100644 --- a/src/storages/pluggable/__tests__/index.spec.ts +++ b/src/storages/pluggable/__tests__/index.spec.ts @@ -15,7 +15,7 @@ import { assertStorageInterface } from '../../__tests__/testUtils'; describe('PLUGGABLE STORAGE', () => { const internalSdkParams: IStorageFactoryParams = { - settings: { sync: {}, log: loggerMock }, + log: loggerMock, metadata, onReadyCb: jest.fn() }; diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index 71b9fb46..724dd18a 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -43,9 +43,7 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn const prefix = validatePrefix(options.prefix); - function PluggableStorageFactory({ settings, metadata, onReadyCb }: IStorageFactoryParams): IStorageAsync { - const log = settings.log; - const onlyImpressionsAndEvents = settings.sync.onlyImpressionsAndEvents; + function PluggableStorageFactory({ log, metadata, onReadyCb, trackInMemory }: IStorageFactoryParams): IStorageAsync { const keys = new KeyBuilderSS(prefix, metadata); const wrapper = wrapperAdapter(log, options.wrapper); @@ -59,8 +57,8 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn return { splits: new SplitsCachePluggable(log, keys, wrapper), segments: new SegmentsCachePluggable(log, keys, wrapper), - impressions: onlyImpressionsAndEvents ? new ImpressionsCacheInMemory() : new ImpressionsCachePluggable(log, keys.buildImpressionsKey(), wrapper, metadata), - events: onlyImpressionsAndEvents ? new EventsCacheInMemory() : new EventsCachePluggable(log, keys.buildEventsKey(), wrapper, metadata), + impressions: trackInMemory ? new ImpressionsCacheInMemory() : new ImpressionsCachePluggable(log, keys.buildImpressionsKey(), wrapper, metadata), + events: trackInMemory ? new EventsCacheInMemory() : 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 7d722942..e3e6f846 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -1,6 +1,7 @@ -import { MaybeThenable, IMetadata } from '../dtos/types'; +import { MaybeThenable, IMetadata, ISplitFiltersValidation } from '../dtos/types'; +import { ILogger } from '../logger/types'; import { StoredEventWithMetadata, StoredImpressionWithMetadata } from '../sync/submitters/types'; -import { SplitIO, ImpressionDTO, ISettings } from '../types'; +import { SplitIO, ImpressionDTO } from '../types'; /** * Interface of a custom wrapper storage. @@ -406,6 +407,7 @@ export type IStorageSync = IStorageBase< ICountsCacheSync > +// @TODO remove shared method to unify storage interfaces export interface IStorageSyncCS extends IStorageSync { shared(matchingKey: string): IStorageSync } @@ -424,17 +426,22 @@ export type IStorageAsync = IStorageBase< export type DataLoader = (storage: IStorageSync, matchingKey: string) => void export interface IStorageFactoryParams { - settings: ISettings, - - // Properties derived from settings + log: ILogger, + eventsQueueSize?: number, optimize?: boolean /* whether create the `impressionCounts` cache (OPTIMIZED impression mode) or not (DEBUG impression mode) */, - matchingKey?: string, /* undefined on server-side SDKs. ATM, only used by InLocalStorage */ - metadata: IMetadata, + + // ATM, only used by InLocalStorage + matchingKey?: string, /* undefined on server-side SDKs */ + splitFiltersValidation?: ISplitFiltersValidation, + + // ATM, only used by CustomStorage. true for partial consumer mode + trackInMemory?: boolean, // 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. onReadyCb?: (error?: any) => void, + metadata: IMetadata, } export type StorageType = 'MEMORY' | 'LOCALSTORAGE' | 'REDIS' | 'CUSTOM'; diff --git a/src/sync/syncManagerOnline.ts b/src/sync/syncManagerOnline.ts index d10323d3..c7d1ee7b 100644 --- a/src/sync/syncManagerOnline.ts +++ b/src/sync/syncManagerOnline.ts @@ -122,8 +122,8 @@ export function syncManagerOnlineFactory( else return Promise.resolve(); }, - // Only used for client-side in standalone mode. - // Therefore, polling manager is defined and both polling and push managers implement the interfaces for client-side + // Only used for client-side in standalone mode: polling manager is defined and + // both polling and push managers implement the interfaces for client-side shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync): ISyncManager { const mySegmentsSyncTask = (pollingManager as IPollingManagerCS).add(matchingKey, readinessManager, storage); diff --git a/src/sync/types.ts b/src/sync/types.ts index 55566ef1..ebb4c22e 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,6 +44,7 @@ export interface ITimeTracker { export interface ISyncManager extends ITask { flush(): Promise, + pollingManager?: IPollingManager pushManager?: IPushManager } diff --git a/src/types.ts b/src/types.ts index c77bd861..8b9b1288 100644 --- a/src/types.ts +++ b/src/types.ts @@ -3,7 +3,7 @@ import { IIntegration, IIntegrationFactoryParams } from './integrations/types'; import { ILogger } from './logger/types'; /* eslint-disable no-use-before-define */ -import { IStorageFactoryParams, IStorageSync, IStorageAsync, IStorageSyncFactory } from './storages/types'; +import { IStorageFactoryParams, IStorageSyncCS, IStorageSync, IStorageAsync, IStorageSyncFactory } from './storages/types'; import { ISyncManagerFactoryParams, ISyncManagerCS } from './sync/types'; /** @@ -842,7 +842,7 @@ export namespace SplitIO { * Defines which kind of storage we should instanciate. * @property {Object} storage */ - storage?: (params: IStorageFactoryParams) => IStorageSync | IStorageAsync, + storage?: (params: IStorageFactoryParams) => IStorageSyncCS | IStorageAsync, /** * List of URLs that the SDK will use as base for it's synchronization functionalities, applicable only when running as standalone. * Do not change these settings unless you're working an advanced use case, like connecting to the Split proxy. diff --git a/src/utils/settingsValidation/index.ts b/src/utils/settingsValidation/index.ts index af75bb76..1840fdb5 100644 --- a/src/utils/settingsValidation/index.ts +++ b/src/utils/settingsValidation/index.ts @@ -75,8 +75,7 @@ const base = { splitFilters: undefined, // impressions collection mode impressionsMode: OPTIMIZED, - localhostMode: undefined, - onlyImpressionsAndEvents: false + localhostMode: undefined }, runtime: { diff --git a/src/utils/settingsValidation/storage/storageCS.ts b/src/utils/settingsValidation/storage/storageCS.ts index 841ca959..19b5e29e 100644 --- a/src/utils/settingsValidation/storage/storageCS.ts +++ b/src/utils/settingsValidation/storage/storageCS.ts @@ -3,9 +3,9 @@ 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 { IStorageFactoryParams, IStorageSync } from '../../../storages/types'; +import { IStorageFactoryParams, IStorageSyncCS } from '../../../storages/types'; -export function __InLocalStorageMockFactory(params: IStorageFactoryParams): IStorageSync { +export function __InLocalStorageMockFactory(params: IStorageFactoryParams): IStorageSyncCS { const result = InMemoryStorageCSFactory(params); result.splits.checkCache = () => true; // to emit SDK_READY_FROM_CACHE return result; From 9dfccd76c0516184838b6be3732cf7e9de51bf04 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 5 Nov 2021 15:26:35 -0300 Subject: [PATCH 04/18] renamed PluggableStorage --> CustomStorage --- src/index.ts | 2 +- src/storages/pluggable/__tests__/index.spec.ts | 16 ++++++++-------- src/storages/pluggable/index.ts | 18 +++++++++--------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/index.ts b/src/index.ts index 33f4217a..179c3585 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,7 +8,7 @@ export { InMemoryStorageCSFactory } from './storages/inMemory/InMemoryStorageCS' export { InLocalStorage } from './storages/inLocalStorage'; // Pluggable storage factory for consumer/producer mode -export { PluggableStorage } from './storages/pluggable'; +export { CustomStorage } from './storages/pluggable'; // Pluggable storage factory for consumer/producer mode export { InRedisStorage } from './storages/inRedis'; diff --git a/src/storages/pluggable/__tests__/index.spec.ts b/src/storages/pluggable/__tests__/index.spec.ts index 8356a3ad..3ebf6d58 100644 --- a/src/storages/pluggable/__tests__/index.spec.ts +++ b/src/storages/pluggable/__tests__/index.spec.ts @@ -9,7 +9,7 @@ const metadata = { s: 'version', i: 'ip', n: 'hostname' }; const prefix = 'some_prefix'; // Test target -import { PluggableStorage } from '../index'; +import { CustomStorage } from '../index'; import { assertStorageInterface } from '../../__tests__/testUtils'; describe('PLUGGABLE STORAGE', () => { @@ -25,7 +25,7 @@ describe('PLUGGABLE STORAGE', () => { }); test('creates a storage instance', async () => { - const storageFactory = PluggableStorage({ prefix, wrapper: wrapperMock }); + const storageFactory = CustomStorage({ prefix, wrapper: wrapperMock }); const storage = storageFactory(internalSdkParams); assertStorageInterface(storage); // the instance must implement the storage interface @@ -42,21 +42,21 @@ describe('PLUGGABLE STORAGE', () => { test('throws an exception if wrapper doesn\'t implement the expected interface', async () => { - expect(() => PluggableStorage({ wrapper: wrapperMock })).not.toThrow(); // not prefix but valid wrapper is OK + expect(() => CustomStorage({ wrapper: wrapperMock })).not.toThrow(); // not prefix but valid wrapper is OK // Throws exception if no object is passed as wrapper const errorNoValidWrapper = 'Expecting custom storage `wrapper` in options, but no valid wrapper instance was provided.'; - expect(() => PluggableStorage()).toThrow(errorNoValidWrapper); - expect(() => PluggableStorage({ wrapper: undefined })).toThrow(errorNoValidWrapper); - expect(() => PluggableStorage({ wrapper: 'invalid wrapper' })).toThrow(errorNoValidWrapper); + expect(() => CustomStorage()).toThrow(errorNoValidWrapper); + expect(() => CustomStorage({ wrapper: undefined })).toThrow(errorNoValidWrapper); + expect(() => CustomStorage({ wrapper: 'invalid wrapper' })).toThrow(errorNoValidWrapper); // 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'; 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); + expect(() => CustomStorage({ wrapper: invalidWrapper })).toThrow(errorNoValidWrapperInterface); + expect(() => CustomStorage({ wrapper: {} })).toThrow(errorNoValidWrapperInterface); }); }); diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index 724dd18a..337ea94e 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -15,18 +15,18 @@ 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.'; -export interface PluggableStorageOptions { +export interface CustomStorageOptions { prefix?: string wrapper: ICustomStorageWrapper } /** - * Validate pluggable storage factory options. + * Validate custom storage factory options. * * @param options user options * @throws Will throw an error if the options are invalid. Example: wrapper is not provided or doesn't have some methods. */ -function validatePluggableStorageOptions(options: any) { +function validateCustomStorageOptions(options: any) { if (!isObject(options) || !isObject(options.wrapper)) throw new Error(NO_VALID_WRAPPER); const wrapper = options.wrapper; @@ -35,15 +35,15 @@ function validatePluggableStorageOptions(options: any) { } /** - * Pluggable storage factory for consumer server-side & client-side SplitFactory. + * Custom storage factory for consumer server-side & client-side SplitFactory. */ -export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyncFactory { +export function CustomStorage(options: CustomStorageOptions): IStorageAsyncFactory { - validatePluggableStorageOptions(options); + validateCustomStorageOptions(options); const prefix = validatePrefix(options.prefix); - function PluggableStorageFactory({ log, metadata, onReadyCb, trackInMemory }: IStorageFactoryParams): IStorageAsync { + function CustomStorageFactory({ log, metadata, onReadyCb, trackInMemory }: IStorageFactoryParams): IStorageAsync { const keys = new KeyBuilderSS(prefix, metadata); const wrapper = wrapperAdapter(log, options.wrapper); @@ -68,6 +68,6 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn }; } - PluggableStorageFactory.type = STORAGE_CUSTOM; - return PluggableStorageFactory; + CustomStorageFactory.type = STORAGE_CUSTOM; + return CustomStorageFactory; } From 6b208b51733fa3ad937005ed4ca97ee7f9940af2 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 5 Nov 2021 16:17:40 -0300 Subject: [PATCH 05/18] Revert "renamed PluggableStorage --> CustomStorage" This reverts commit 9dfccd76c0516184838b6be3732cf7e9de51bf04. --- src/index.ts | 2 +- src/storages/pluggable/__tests__/index.spec.ts | 16 ++++++++-------- src/storages/pluggable/index.ts | 18 +++++++++--------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/index.ts b/src/index.ts index 179c3585..33f4217a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,7 +8,7 @@ export { InMemoryStorageCSFactory } from './storages/inMemory/InMemoryStorageCS' export { InLocalStorage } from './storages/inLocalStorage'; // Pluggable storage factory for consumer/producer mode -export { CustomStorage } from './storages/pluggable'; +export { PluggableStorage } from './storages/pluggable'; // Pluggable storage factory for consumer/producer mode export { InRedisStorage } from './storages/inRedis'; diff --git a/src/storages/pluggable/__tests__/index.spec.ts b/src/storages/pluggable/__tests__/index.spec.ts index 3ebf6d58..8356a3ad 100644 --- a/src/storages/pluggable/__tests__/index.spec.ts +++ b/src/storages/pluggable/__tests__/index.spec.ts @@ -9,7 +9,7 @@ const metadata = { s: 'version', i: 'ip', n: 'hostname' }; const prefix = 'some_prefix'; // Test target -import { CustomStorage } from '../index'; +import { PluggableStorage } from '../index'; import { assertStorageInterface } from '../../__tests__/testUtils'; describe('PLUGGABLE STORAGE', () => { @@ -25,7 +25,7 @@ describe('PLUGGABLE STORAGE', () => { }); test('creates a storage instance', async () => { - const storageFactory = CustomStorage({ prefix, wrapper: wrapperMock }); + const storageFactory = PluggableStorage({ prefix, wrapper: wrapperMock }); const storage = storageFactory(internalSdkParams); assertStorageInterface(storage); // the instance must implement the storage interface @@ -42,21 +42,21 @@ describe('PLUGGABLE STORAGE', () => { test('throws an exception if wrapper doesn\'t implement the expected interface', async () => { - expect(() => CustomStorage({ wrapper: wrapperMock })).not.toThrow(); // not prefix but valid wrapper is OK + expect(() => PluggableStorage({ wrapper: wrapperMock })).not.toThrow(); // not prefix but valid wrapper is OK // Throws exception if no object is passed as wrapper const errorNoValidWrapper = 'Expecting custom storage `wrapper` in options, but no valid wrapper instance was provided.'; - expect(() => CustomStorage()).toThrow(errorNoValidWrapper); - expect(() => CustomStorage({ wrapper: undefined })).toThrow(errorNoValidWrapper); - expect(() => CustomStorage({ wrapper: 'invalid wrapper' })).toThrow(errorNoValidWrapper); + expect(() => PluggableStorage()).toThrow(errorNoValidWrapper); + expect(() => PluggableStorage({ wrapper: undefined })).toThrow(errorNoValidWrapper); + expect(() => PluggableStorage({ wrapper: 'invalid wrapper' })).toThrow(errorNoValidWrapper); // 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'; const errorNoValidWrapperInterface = 'The provided wrapper instance doesn’t follow the expected interface. Check our docs.'; - expect(() => CustomStorage({ wrapper: invalidWrapper })).toThrow(errorNoValidWrapperInterface); - expect(() => CustomStorage({ wrapper: {} })).toThrow(errorNoValidWrapperInterface); + expect(() => PluggableStorage({ wrapper: invalidWrapper })).toThrow(errorNoValidWrapperInterface); + expect(() => PluggableStorage({ wrapper: {} })).toThrow(errorNoValidWrapperInterface); }); }); diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index 337ea94e..724dd18a 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -15,18 +15,18 @@ 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.'; -export interface CustomStorageOptions { +export interface PluggableStorageOptions { prefix?: string wrapper: ICustomStorageWrapper } /** - * Validate custom storage factory options. + * Validate pluggable storage factory options. * * @param options user options * @throws Will throw an error if the options are invalid. Example: wrapper is not provided or doesn't have some methods. */ -function validateCustomStorageOptions(options: any) { +function validatePluggableStorageOptions(options: any) { if (!isObject(options) || !isObject(options.wrapper)) throw new Error(NO_VALID_WRAPPER); const wrapper = options.wrapper; @@ -35,15 +35,15 @@ function validateCustomStorageOptions(options: any) { } /** - * Custom storage factory for consumer server-side & client-side SplitFactory. + * Pluggable storage factory for consumer server-side & client-side SplitFactory. */ -export function CustomStorage(options: CustomStorageOptions): IStorageAsyncFactory { +export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyncFactory { - validateCustomStorageOptions(options); + validatePluggableStorageOptions(options); const prefix = validatePrefix(options.prefix); - function CustomStorageFactory({ log, metadata, onReadyCb, trackInMemory }: IStorageFactoryParams): IStorageAsync { + function PluggableStorageFactory({ log, metadata, onReadyCb, trackInMemory }: IStorageFactoryParams): IStorageAsync { const keys = new KeyBuilderSS(prefix, metadata); const wrapper = wrapperAdapter(log, options.wrapper); @@ -68,6 +68,6 @@ export function CustomStorage(options: CustomStorageOptions): IStorageAsyncFacto }; } - CustomStorageFactory.type = STORAGE_CUSTOM; - return CustomStorageFactory; + PluggableStorageFactory.type = STORAGE_CUSTOM; + return PluggableStorageFactory; } From 1e668a50ee2c8f488e483c7e76d69b933039ab75 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 10 Nov 2021 16:08:26 -0300 Subject: [PATCH 06/18] polishing --- src/listeners/browser.ts | 4 +- src/sdkClient/sdkClientMethodCS.ts | 20 +++++---- src/sdkClient/sdkClientMethodCSWithTT.ts | 20 +++++---- src/storages/inLocalStorage/index.ts | 4 +- src/storages/inMemory/InMemoryStorageCS.ts | 4 +- src/storages/pluggable/inMemoryWrapper.ts | 18 +++++++- src/storages/pluggable/index.ts | 41 +++++++++++++++---- src/storages/types.ts | 16 +++----- src/sync/syncManagerOnline.ts | 9 ++-- src/sync/types.ts | 2 +- src/types.ts | 4 +- .../settingsValidation/storage/storageCS.ts | 4 +- 12 files changed, 98 insertions(+), 48 deletions(-) diff --git a/src/listeners/browser.ts b/src/listeners/browser.ts index c5c4fdfd..37429224 100644 --- a/src/listeners/browser.ts +++ b/src/listeners/browser.ts @@ -63,6 +63,8 @@ export default class BrowserSignalListener implements ISignalListener { * using beacon API if possible, or falling back to regular post transport. */ flushData() { + if (!this.syncManager) return; // In consumer mode there is not sync manager + const eventsUrl = this.settings.urls.events; const extraMetadata = { // sim stands for Sync/Split Impressions Mode @@ -74,7 +76,7 @@ export default class BrowserSignalListener implements ISignalListener { if (this.storage.impressionCounts) this._flushData(eventsUrl + '/testImpressions/count/beacon', this.storage.impressionCounts, this.serviceApi.postTestImpressionsCount, fromImpressionCountsCollector); // Close streaming connection - if (this.syncManager && this.syncManager.pushManager) this.syncManager.pushManager.stop(); + if (this.syncManager.pushManager) this.syncManager.pushManager.stop(); } private _flushData(url: string, cache: IRecorderCacheProducerSync, postService: (body: string) => Promise, fromCacheToPayload?: (cacheData: TState) => any, extraMetadata?: {}) { diff --git a/src/sdkClient/sdkClientMethodCS.ts b/src/sdkClient/sdkClientMethodCS.ts index 0b60f307..14de634d 100644 --- a/src/sdkClient/sdkClientMethodCS.ts +++ b/src/sdkClient/sdkClientMethodCS.ts @@ -4,10 +4,10 @@ import { SplitIO } from '../types'; import { validateKey } from '../utils/inputValidation/key'; import { getMatching, keyParser } from '../utils/key'; import { sdkClientFactory } from './sdkClient'; -import { IStorageSyncCS } from '../storages/types'; import { ISyncManagerCS } from '../sync/types'; import objectAssign from 'object-assign'; import { RETRIEVE_CLIENT_DEFAULT, NEW_SHARED_CLIENT, RETRIEVE_CLIENT_EXISTING } from '../logger/constants'; +import { SDK_SEGMENTS_ARRIVED } from '../readiness/constants'; function buildInstanceId(key: SplitIO.SplitKey) { // @ts-ignore @@ -59,11 +59,17 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? const sharedSdkReadiness = sdkReadinessManager.shared(readyTimeout); - // 3 combinations are possible: - // - Standalone mode: both syncManager and storage.shared are defined - // - Consumer mode: both syncManager and storage.shared are undefined - // - Consumer mode with submitters: syncManager is defined but not storage.shared - const sharedStorage = (storage as IStorageSyncCS).shared && (storage as IStorageSyncCS).shared(matchingKey); + const sharedStorage = storage.shared && storage.shared(matchingKey, (err) => { + // Emit SDK_READY in consumer mode for shared clients + if (err) return; // don't emit SDK_READY if storage failed to connect. + 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); // As shared clients reuse all the storage information, we don't need to check here if we @@ -71,7 +77,7 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? clientInstances[instanceId] = clientCSDecorator( sdkClientFactory(objectAssign({}, params, { sdkReadinessManager: sharedSdkReadiness, - storage: sharedStorage, + storage: sharedStorage || storage, syncManager: sharedSyncManager, signalListener: undefined, // only the main client "destroy" method stops the signal listener sharedClient: true diff --git a/src/sdkClient/sdkClientMethodCSWithTT.ts b/src/sdkClient/sdkClientMethodCSWithTT.ts index 10e30586..7de70f98 100644 --- a/src/sdkClient/sdkClientMethodCSWithTT.ts +++ b/src/sdkClient/sdkClientMethodCSWithTT.ts @@ -5,10 +5,10 @@ import { validateKey } from '../utils/inputValidation/key'; import { validateTrafficType } from '../utils/inputValidation/trafficType'; import { getMatching, keyParser } from '../utils/key'; import { sdkClientFactory } from './sdkClient'; -import { IStorageSyncCS } from '../storages/types'; import { ISyncManagerCS } from '../sync/types'; import objectAssign from 'object-assign'; import { RETRIEVE_CLIENT_DEFAULT, NEW_SHARED_CLIENT, RETRIEVE_CLIENT_EXISTING } from '../logger/constants'; +import { SDK_SEGMENTS_ARRIVED } from '../readiness/constants'; function buildInstanceId(key: SplitIO.SplitKey, trafficType?: string) { // @ts-ignore @@ -73,11 +73,17 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? const sharedSdkReadiness = sdkReadinessManager.shared(readyTimeout); - // 3 combinations are possible: - // - Standalone mode: both syncManager and storage.shared are defined - // - Consumer mode: both syncManager and storage.shared are undefined - // - Consumer mode with submitters: syncManager is defined but not storage.shared - const sharedStorage = (storage as IStorageSyncCS).shared && (storage as IStorageSyncCS).shared(matchingKey); + const sharedStorage = storage.shared && storage.shared(matchingKey, (err) => { + // Emit SDK_READY in consumer mode for shared clients + if (err) return; // don't emit SDK_READY if storage failed to connect. + 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); // As shared clients reuse all the storage information, we don't need to check here if we @@ -85,7 +91,7 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? clientInstances[instanceId] = clientCSDecorator( sdkClientFactory(objectAssign({}, params, { sdkReadinessManager: sharedSdkReadiness, - storage: sharedStorage, + storage: sharedStorage || storage, syncManager: sharedSyncManager, signalListener: undefined, // only the main client "destroy" method stops the signal listener sharedClient: true diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 6587208b..795fd95b 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -1,7 +1,7 @@ import ImpressionsCacheInMemory from '../inMemory/ImpressionsCacheInMemory'; import ImpressionCountsCacheInMemory from '../inMemory/ImpressionCountsCacheInMemory'; import EventsCacheInMemory from '../inMemory/EventsCacheInMemory'; -import { IStorageFactoryParams, IStorageSyncCS, IStorageSyncFactory } from '../types'; +import { IStorageFactoryParams, IStorageSync, IStorageSyncFactory } from '../types'; import { validatePrefix } from '../KeyBuilder'; import KeyBuilderCS from '../KeyBuilderCS'; import { isLocalStorageAvailable } from '../../utils/env/isLocalStorageAvailable'; @@ -25,7 +25,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn const prefix = validatePrefix(options.prefix); - function InLocalStorageCSFactory(params: IStorageFactoryParams): IStorageSyncCS { + function InLocalStorageCSFactory(params: IStorageFactoryParams): IStorageSync { // Fallback to InMemoryStorage if LocalStorage API is not available if (!isLocalStorageAvailable()) { diff --git a/src/storages/inMemory/InMemoryStorageCS.ts b/src/storages/inMemory/InMemoryStorageCS.ts index d59e4c04..8f0eff19 100644 --- a/src/storages/inMemory/InMemoryStorageCS.ts +++ b/src/storages/inMemory/InMemoryStorageCS.ts @@ -2,7 +2,7 @@ import SplitsCacheInMemory from './SplitsCacheInMemory'; import MySegmentsCacheInMemory from './MySegmentsCacheInMemory'; import ImpressionsCacheInMemory from './ImpressionsCacheInMemory'; import EventsCacheInMemory from './EventsCacheInMemory'; -import { IStorageSyncCS, IStorageFactoryParams } from '../types'; +import { IStorageSync, IStorageFactoryParams } from '../types'; import ImpressionCountsCacheInMemory from './ImpressionCountsCacheInMemory'; import { STORAGE_MEMORY } from '../../utils/constants'; @@ -11,7 +11,7 @@ import { STORAGE_MEMORY } from '../../utils/constants'; * * @param params parameters required by EventsCacheSync */ -export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorageSyncCS { +export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorageSync { return { splits: new SplitsCacheInMemory(), diff --git a/src/storages/pluggable/inMemoryWrapper.ts b/src/storages/pluggable/inMemoryWrapper.ts index efb2c6db..b72de2ff 100644 --- a/src/storages/pluggable/inMemoryWrapper.ts +++ b/src/storages/pluggable/inMemoryWrapper.ts @@ -6,10 +6,13 @@ import { ISet, setToArray, _Set } from '../../utils/lang/sets'; * Creates a ICustomStorageWrapper implementation that stores items in memory. * The `_cache` property is the object were items are stored. * Intended for testing purposes. + * + * @param connDelay delay in millis for `connect` resolve. If not provided, `connect` resolves inmediatelly. */ -export function inMemoryWrapperFactory(): ICustomStorageWrapper & { _cache: Record> } { +export function inMemoryWrapperFactory(connDelay?: number): ICustomStorageWrapper & { _cache: Record>, _setConnDelay(connDelay: number): void } { let _cache: Record> = {}; + let _connDelay = connDelay; return { /** Holds items (for key-value operations), list of items (for list operations) and set of items (for set operations) */ @@ -110,7 +113,18 @@ export function inMemoryWrapperFactory(): ICustomStorageWrapper & { _cache: Reco }, // always connects and close - connect() { return Promise.resolve(); }, + connect() { + if (typeof _connDelay === 'number') { + return new Promise(res => setTimeout(res, _connDelay)); + } else { + return Promise.resolve(); + } + }, close() { return Promise.resolve(); }, + + // for testing + _setConnDelay(connDelay: number) { + _connDelay = connDelay; + } }; } diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index 724dd18a..4a592d6f 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -34,6 +34,25 @@ function validatePluggableStorageOptions(options: any) { if (missingMethods.length) throw new Error(`${NO_VALID_WRAPPER_INTERFACE} The following methods are missing or invalid: ${missingMethods}`); } +// subscription to wrapper connect event in order to emit SDK_READY event +function wrapperConnect(wrapper: ICustomStorageWrapper, onReadyCb: (error?: any) => void) { + wrapper.connect().then(() => { + onReadyCb(); + }).catch((e) => { + onReadyCb(e || new Error('Error connecting wrapper')); + }); +} + +// For consumer partial mode to have async return type in `client.track` method +// 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. */ @@ -43,27 +62,33 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn const prefix = validatePrefix(options.prefix); - function PluggableStorageFactory({ log, metadata, onReadyCb, trackInMemory }: IStorageFactoryParams): IStorageAsync { + function PluggableStorageFactory({ log, metadata, onReadyCb, trackInMemory, eventsQueueSize }: IStorageFactoryParams): IStorageAsync { const keys = new KeyBuilderSS(prefix, metadata); const wrapper = wrapperAdapter(log, options.wrapper); - // subscription to Wrapper connect event in order to emit SDK_READY event - wrapper.connect().then(() => { - if (onReadyCb) onReadyCb(); - }).catch((e) => { - if (onReadyCb) onReadyCb(e || new Error('Error connecting wrapper')); - }); + // emit SDK_READY event on main client + wrapperConnect(wrapper, onReadyCb); return { splits: new SplitsCachePluggable(log, keys, wrapper), segments: new SegmentsCachePluggable(log, keys, wrapper), impressions: trackInMemory ? new ImpressionsCacheInMemory() : new ImpressionsCachePluggable(log, keys.buildImpressionsKey(), wrapper, metadata), - events: trackInMemory ? new EventsCacheInMemory() : new EventsCachePluggable(log, keys.buildEventsKey(), wrapper, metadata), + events: trackInMemory ? 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). destroy() { return wrapper.close(); + }, + + // emits SDK_READY event on shared clients and returns a reference to the storage + shared(_, onReadyCb) { + wrapperConnect(wrapper, onReadyCb); + return { + ...this, + // no-op destroy, to close the wrapper only when the main client is destroyed + destroy() { } + }; } }; } diff --git a/src/storages/types.ts b/src/storages/types.ts index e3e6f846..c3e5dfd9 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -4,7 +4,7 @@ import { StoredEventWithMetadata, StoredImpressionWithMetadata } from '../sync/s import { SplitIO, ImpressionDTO } from '../types'; /** - * Interface of a custom wrapper storage. + * Interface of a custom storage wrapper. */ export interface ICustomStorageWrapper { @@ -169,7 +169,7 @@ export interface ICustomStorageWrapper { /** * Connects to the underlying storage. * It is meant for storages that requires to be connected to some database or server. Otherwise it can just return a resolved promise. - * Note: will be called once on SplitFactory instantiation. + * Note: will be called once on SplitFactory instantiation and once per each shared client instantiation. * * @function connect * @returns {Promise} A promise that resolves when the wrapper successfully connect to the underlying storage. @@ -179,7 +179,7 @@ export interface ICustomStorageWrapper { /** * Disconnects 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 client destroy. + * Note: will be called once on SplitFactory main client destroy. * * @function close * @returns {Promise} A promise that resolves when the operation ends. @@ -396,6 +396,7 @@ export interface IStorageBase< latencies?: TLatenciesCache, counts?: TCountsCache, destroy(): void, + shared?: (matchingKey: string, onReadyCb: (error?: any) => void) => this } export type IStorageSync = IStorageBase< @@ -407,11 +408,6 @@ export type IStorageSync = IStorageBase< ICountsCacheSync > -// @TODO remove shared method to unify storage interfaces -export interface IStorageSyncCS extends IStorageSync { - shared(matchingKey: string): IStorageSync -} - export type IStorageAsync = IStorageBase< ISplitsCacheAsync, ISegmentsCacheAsync, @@ -434,13 +430,13 @@ 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. True for partial consumer mode trackInMemory?: boolean, // 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. - onReadyCb?: (error?: any) => void, + onReadyCb: (error?: any) => void, metadata: IMetadata, } diff --git a/src/sync/syncManagerOnline.ts b/src/sync/syncManagerOnline.ts index c7d1ee7b..b793f845 100644 --- a/src/sync/syncManagerOnline.ts +++ b/src/sync/syncManagerOnline.ts @@ -1,4 +1,4 @@ -import { ISyncManager, ISyncManagerCS, ISyncManagerFactoryParams } from './types'; +import { ISyncManagerCS, ISyncManagerFactoryParams } from './types'; import { submitterManagerFactory } from './submitters/submitterManager'; import { IReadinessManager } from '../readiness/types'; import { IStorageSync } from '../storages/types'; @@ -122,9 +122,10 @@ export function syncManagerOnlineFactory( else return Promise.resolve(); }, - // Only used for client-side in standalone mode: polling manager is defined and - // both polling and push managers implement the interfaces for client-side - shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync): ISyncManager { + // Only used for client-side in standalone mode: if polling and push managers + // are defined, 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); diff --git a/src/sync/types.ts b/src/sync/types.ts index ebb4c22e..d7349535 100644 --- a/src/sync/types.ts +++ b/src/sync/types.ts @@ -49,7 +49,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 8b9b1288..c77bd861 100644 --- a/src/types.ts +++ b/src/types.ts @@ -3,7 +3,7 @@ import { IIntegration, IIntegrationFactoryParams } from './integrations/types'; import { ILogger } from './logger/types'; /* eslint-disable no-use-before-define */ -import { IStorageFactoryParams, IStorageSyncCS, IStorageSync, IStorageAsync, IStorageSyncFactory } from './storages/types'; +import { IStorageFactoryParams, IStorageSync, IStorageAsync, IStorageSyncFactory } from './storages/types'; import { ISyncManagerFactoryParams, ISyncManagerCS } from './sync/types'; /** @@ -842,7 +842,7 @@ export namespace SplitIO { * Defines which kind of storage we should instanciate. * @property {Object} storage */ - storage?: (params: IStorageFactoryParams) => IStorageSyncCS | IStorageAsync, + storage?: (params: IStorageFactoryParams) => IStorageSync | IStorageAsync, /** * List of URLs that the SDK will use as base for it's synchronization functionalities, applicable only when running as standalone. * Do not change these settings unless you're working an advanced use case, like connecting to the Split proxy. diff --git a/src/utils/settingsValidation/storage/storageCS.ts b/src/utils/settingsValidation/storage/storageCS.ts index 19b5e29e..841ca959 100644 --- a/src/utils/settingsValidation/storage/storageCS.ts +++ b/src/utils/settingsValidation/storage/storageCS.ts @@ -3,9 +3,9 @@ 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 { IStorageFactoryParams, IStorageSyncCS } from '../../../storages/types'; +import { IStorageFactoryParams, IStorageSync } from '../../../storages/types'; -export function __InLocalStorageMockFactory(params: IStorageFactoryParams): IStorageSyncCS { +export function __InLocalStorageMockFactory(params: IStorageFactoryParams): IStorageSync { const result = InMemoryStorageCSFactory(params); result.splits.checkCache = () => true; // to emit SDK_READY_FROM_CACHE return result; From c5ad75c52e7e90eaba84f156062e9fcf26a43165 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 10 Nov 2021 16:16:44 -0300 Subject: [PATCH 07/18] handling storage.destroy async --- src/sdkClient/sdkClient.ts | 6 +++--- src/storages/types.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sdkClient/sdkClient.ts b/src/sdkClient/sdkClient.ts index dd325dcb..e3051864 100644 --- a/src/sdkClient/sdkClient.ts +++ b/src/sdkClient/sdkClient.ts @@ -37,11 +37,11 @@ export function sdkClientFactory(params: ISdkClientFactoryParams): SplitIO.IClie sdkReadinessManager.readinessManager.destroy(); signalListener && signalListener.stop(); - // Cleanup storage - storage.destroy(); - // Release the API Key if it is the main client if (!sharedClient) releaseApiKey(settings.core.authorizationKey); + + // Cleanup storage + return storage.destroy(); }); } } diff --git a/src/storages/types.ts b/src/storages/types.ts index c3e5dfd9..5cf07b1a 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -395,7 +395,7 @@ export interface IStorageBase< events: TEventsCache, latencies?: TLatenciesCache, counts?: TCountsCache, - destroy(): void, + destroy(): void | Promise, shared?: (matchingKey: string, onReadyCb: (error?: any) => void) => this } From 7fe07ccbbd86a174ba8541f90f24af9fc0653de6 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 10 Nov 2021 16:23:13 -0300 Subject: [PATCH 08/18] polishing --- src/sdkFactory/index.ts | 3 --- src/storages/pluggable/index.ts | 18 +++--------------- src/storages/types.ts | 3 --- src/types.ts | 3 +-- 4 files changed, 4 insertions(+), 23 deletions(-) diff --git a/src/sdkFactory/index.ts b/src/sdkFactory/index.ts index 550952f8..e55db71b 100644 --- a/src/sdkFactory/index.ts +++ b/src/sdkFactory/index.ts @@ -41,9 +41,6 @@ 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 - trackInMemory: settings.sync.onlyImpressionsAndEvents, - // 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/pluggable/index.ts b/src/storages/pluggable/index.ts index 4a592d6f..e00f87a5 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -9,8 +9,6 @@ import { wrapperAdapter, METHODS_TO_PROMISE_WRAP } from './wrapperAdapter'; import { isObject } from '../../utils/lang'; import { validatePrefix } from '../KeyBuilder'; import { 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.'; @@ -43,16 +41,6 @@ function wrapperConnect(wrapper: ICustomStorageWrapper, onReadyCb: (error?: any) }); } -// For consumer partial mode to have async return type in `client.track` method -// 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. */ @@ -62,7 +50,7 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn const prefix = validatePrefix(options.prefix); - function PluggableStorageFactory({ log, metadata, onReadyCb, trackInMemory, eventsQueueSize }: IStorageFactoryParams): IStorageAsync { + function PluggableStorageFactory({ log, metadata, onReadyCb }: IStorageFactoryParams): IStorageAsync { const keys = new KeyBuilderSS(prefix, metadata); const wrapper = wrapperAdapter(log, options.wrapper); @@ -72,8 +60,8 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn return { splits: new SplitsCachePluggable(log, keys, wrapper), segments: new SegmentsCachePluggable(log, keys, wrapper), - impressions: trackInMemory ? new ImpressionsCacheInMemory() : new ImpressionsCachePluggable(log, keys.buildImpressionsKey(), wrapper, metadata), - events: trackInMemory ? promisifyEventsTrack(new EventsCacheInMemory(eventsQueueSize)) : new EventsCachePluggable(log, keys.buildEventsKey(), wrapper, metadata), + impressions: new ImpressionsCachePluggable(log, keys.buildImpressionsKey(), wrapper, metadata), + events: 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 5cf07b1a..c26bc936 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -430,9 +430,6 @@ export interface IStorageFactoryParams { matchingKey?: string, /* undefined on server-side SDKs */ splitFiltersValidation?: ISplitFiltersValidation, - // ATM, only used by CustomStorage. True for partial consumer mode - trackInMemory?: boolean, - // 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/types.ts b/src/types.ts index c77bd861..c12ced90 100644 --- a/src/types.ts +++ b/src/types.ts @@ -101,8 +101,7 @@ export interface ISettings { splitFilters: SplitIO.SplitFilter[], impressionsMode: SplitIO.ImpressionsMode, __splitFiltersValidation: ISplitFiltersValidation, - localhostMode?: SplitIO.LocalhostFactory, - onlyImpressionsAndEvents?: boolean + localhostMode?: SplitIO.LocalhostFactory }, readonly runtime: { ip: string | false From 09689dcba0913cfdd54755a6f8855a54e3b8e1e2 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 10 Nov 2021 16:45:55 -0300 Subject: [PATCH 09/18] polishing --- src/sdkClient/sdkClientMethodCS.ts | 8 +--- src/sdkClient/sdkClientMethodCSWithTT.ts | 8 +--- src/sdkFactory/index.ts | 4 +- src/storages/types.ts | 4 +- src/sync/submitters/submitterManager.ts | 22 --------- src/sync/syncManagerOnline.ts | 60 +++++++++++++----------- src/sync/types.ts | 4 +- 7 files changed, 40 insertions(+), 70 deletions(-) delete mode 100644 src/sync/submitters/submitterManager.ts diff --git a/src/sdkClient/sdkClientMethodCS.ts b/src/sdkClient/sdkClientMethodCS.ts index 14de634d..b741c235 100644 --- a/src/sdkClient/sdkClientMethodCS.ts +++ b/src/sdkClient/sdkClientMethodCS.ts @@ -58,17 +58,11 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? const matchingKey = getMatching(validKey); const sharedSdkReadiness = sdkReadinessManager.shared(readyTimeout); - const sharedStorage = storage.shared && storage.shared(matchingKey, (err) => { + if (err) return; // Emit SDK_READY in consumer mode for shared clients - if (err) return; // don't emit SDK_READY if storage failed to connect. 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 7de70f98..82aa544d 100644 --- a/src/sdkClient/sdkClientMethodCSWithTT.ts +++ b/src/sdkClient/sdkClientMethodCSWithTT.ts @@ -72,17 +72,11 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? const matchingKey = getMatching(validKey); const sharedSdkReadiness = sdkReadinessManager.shared(readyTimeout); - const sharedStorage = storage.shared && storage.shared(matchingKey, (err) => { + if (err) return; // Emit SDK_READY in consumer mode for shared clients - if (err) return; // don't emit SDK_READY if storage failed to connect. 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 e55db71b..0c116895 100644 --- a/src/sdkFactory/index.ts +++ b/src/sdkFactory/index.ts @@ -33,7 +33,6 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO. // @TODO consider passing the settings object, so that each storage access only what it needs const storageFactoryParams: IStorageFactoryParams = { - log, eventsQueueSize: settings.scheduler.eventsQueueSize, optimize: shouldBeOptimized(settings), @@ -48,7 +47,8 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO. readinessManager.splits.emit(SDK_SPLITS_ARRIVED); readinessManager.segments.emit(SDK_SEGMENTS_ARRIVED); }, - metadata: metadataBuilder(settings) + metadata: metadataBuilder(settings), + log }; const storage = storageFactory(storageFactoryParams); diff --git a/src/storages/types.ts b/src/storages/types.ts index c26bc936..b9c7b0e6 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -411,8 +411,8 @@ export type IStorageSync = IStorageBase< export type IStorageAsync = IStorageBase< ISplitsCacheAsync, ISegmentsCacheAsync, - IImpressionsCacheAsync | IImpressionsCacheSync, - IEventsCacheAsync | IEventsCacheSync, + IImpressionsCacheAsync, + IEventsCacheAsync, ILatenciesCacheAsync, ICountsCacheAsync > diff --git a/src/sync/submitters/submitterManager.ts b/src/sync/submitters/submitterManager.ts deleted file mode 100644 index 7828d947..00000000 --- a/src/sync/submitters/submitterManager.ts +++ /dev/null @@ -1,22 +0,0 @@ -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 b793f845..b0199a9c 100644 --- a/src/sync/syncManagerOnline.ts +++ b/src/sync/syncManagerOnline.ts @@ -1,5 +1,8 @@ -import { ISyncManagerCS, ISyncManagerFactoryParams } from './types'; -import { submitterManagerFactory } from './submitters/submitterManager'; +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 { IReadinessManager } from '../readiness/types'; import { IStorageSync } from '../storages/types'; import { IPushManagerFactoryParams, IPushManager, IPushManagerCS } from './streaming/types'; @@ -16,7 +19,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 { @@ -34,24 +37,30 @@ export function syncManagerOnlineFactory( const log = settings.log; /** Polling Manager */ - const pollingManager = pollingManagerFactory && pollingManagerFactory(splitApi, storage, readiness, settings); + const pollingManager = pollingManagerFactory(splitApi, storage, readiness, settings); /** Push Manager */ - const pushManager = settings.streamingEnabled && pollingManager && pushManagerFactory ? + const pushManager = settings.streamingEnabled && pushManagerFactory ? pushManagerFactory(pollingManager, storage, readiness, splitApi.fetchAuth, platform, settings) : undefined; /** Submitter Manager */ - // It is not inyected as push and polling managers, because at the moment it is mandatory - const submitter = submitterManagerFactory(settings, storage, splitApi); + // 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); /** 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); } @@ -60,10 +69,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) { @@ -82,17 +91,15 @@ export function syncManagerOnlineFactory( */ start() { // start syncing splits and segments - if (pollingManager) { - if (pushManager) { - // Doesn't call `syncAll` when the syncManager is resuming - if (startFirstTime) { - pollingManager.syncAll(); - startFirstTime = false; - } - pushManager.start(); - } else { - pollingManager.start(); + if (pushManager) { + // Doesn't call `syncAll` when the syncManager is resuming + if (startFirstTime) { + pollingManager.syncAll(); + startFirstTime = false; } + pushManager.start(); + } else { + pollingManager.start(); } // start periodic data recording (events, impressions, telemetry). @@ -106,7 +113,7 @@ export function syncManagerOnlineFactory( stop() { // stop syncing if (pushManager) pushManager.stop(); - if (pollingManager && pollingManager.isRunning()) pollingManager.stop(); + if (pollingManager.isRunning()) pollingManager.stop(); // stop periodic data recording (events, impressions, telemetry). if (submitter) submitter.stop(); @@ -122,10 +129,9 @@ export function syncManagerOnlineFactory( else return Promise.resolve(); }, - // Only used for client-side in standalone mode: if polling and push managers - // are defined, they implement the interfaces for client-side - shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync) { - if (!pollingManager) return; + // [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 { const mySegmentsSyncTask = (pollingManager as IPollingManagerCS).add(matchingKey, readinessManager, storage); @@ -133,7 +139,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 d7349535..55566ef1 100644 --- a/src/sync/types.ts +++ b/src/sync/types.ts @@ -3,7 +3,6 @@ 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 { @@ -44,12 +43,11 @@ export interface ITimeTracker { export interface ISyncManager extends ITask { flush(): Promise, - pollingManager?: IPollingManager pushManager?: IPushManager } export interface ISyncManagerCS extends ISyncManager { - shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync): ISyncManager | undefined + shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync): ISyncManager } export interface ISyncManagerFactoryParams { From d87220348d824bf63100e2158d0860ca28cbea41 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 10 Nov 2021 21:46:57 -0300 Subject: [PATCH 10/18] added some tests --- src/listeners/browser.ts | 2 +- src/storages/pluggable/__tests__/index.spec.ts | 11 +++++++++++ src/types.ts | 4 ++-- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/listeners/browser.ts b/src/listeners/browser.ts index 37429224..4ac90bfc 100644 --- a/src/listeners/browser.ts +++ b/src/listeners/browser.ts @@ -63,7 +63,7 @@ export default class BrowserSignalListener implements ISignalListener { * using beacon API if possible, or falling back to regular post transport. */ flushData() { - if (!this.syncManager) return; // In consumer mode there is not sync manager + if (!this.syncManager) return; // In consumer mode there is not sync manager and data to flush const eventsUrl = this.settings.urls.events; const extraMetadata = { diff --git a/src/storages/pluggable/__tests__/index.spec.ts b/src/storages/pluggable/__tests__/index.spec.ts index 8356a3ad..10b0f043 100644 --- a/src/storages/pluggable/__tests__/index.spec.ts +++ b/src/storages/pluggable/__tests__/index.spec.ts @@ -31,13 +31,24 @@ describe('PLUGGABLE STORAGE', () => { assertStorageInterface(storage); // the instance must implement the storage interface expect(wrapperMock.connect).toBeCalledTimes(1); // wrapper connect method should be called once when storage is created + // shared storage has the same cache items than storage instance, but a no-op destroy + const sharedOnReadyCb = jest.fn(); + const sharedStorage = storage.shared('some_key', sharedOnReadyCb); + assertStorageInterface(sharedStorage); + expect(sharedStorage.splits).toBe(storage.splits); + expect(wrapperMock.connect).toBeCalledTimes(2); + expect(await storage.splits.getSplit('some_split')).toBe(null); + expect(await sharedStorage.splits.getSplit('some_split')).toBe(null); + expect(wrapperMock.get).toBeCalledTimes(2); expect(wrapperMock.get).toBeCalledWith(`${prefix}.SPLITIO.split.some_split`); // keys prefix should be the provided one await storage.destroy(); + await sharedStorage.destroy(); expect(wrapperMock.close).toBeCalledTimes(1); // wrapper close 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); }); test('throws an exception if wrapper doesn\'t implement the expected interface', async () => { diff --git a/src/types.ts b/src/types.ts index c12ced90..a2641970 100644 --- a/src/types.ts +++ b/src/types.ts @@ -3,7 +3,7 @@ import { IIntegration, IIntegrationFactoryParams } from './integrations/types'; import { ILogger } from './logger/types'; /* eslint-disable no-use-before-define */ -import { IStorageFactoryParams, IStorageSync, IStorageAsync, IStorageSyncFactory } from './storages/types'; +import { IStorageFactoryParams, IStorageSync, IStorageAsync, IStorageSyncFactory, IStorageAsyncFactory } from './storages/types'; import { ISyncManagerFactoryParams, ISyncManagerCS } from './sync/types'; /** @@ -85,7 +85,7 @@ export interface ISettings { retriesOnFailureBeforeReady: number, eventsFirstPushWindow: number }, - readonly storage: IStorageSyncFactory, + readonly storage: IStorageSyncFactory | IStorageAsyncFactory, readonly integrations?: Array<(params: IIntegrationFactoryParams) => IIntegration | void>, readonly urls: { events: string, From a9774f3a81fc6c8eb6f5195cce0d6f2c48562038 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 10 Nov 2021 23:36:51 -0300 Subject: [PATCH 11/18] fixed listener test --- src/listeners/__tests__/browser.spec.ts | 35 ++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/listeners/__tests__/browser.spec.ts b/src/listeners/__tests__/browser.spec.ts index 3bd83169..f232b482 100644 --- a/src/listeners/__tests__/browser.spec.ts +++ b/src/listeners/__tests__/browser.spec.ts @@ -105,8 +105,8 @@ function triggerUnloadEvent() { /* Mocks end */ -test('Browser JS listener / Impressions optimized mode', () => { - +test('Browser JS listener / consumer mode', () => { + // No SyncManager ==> consumer mode const listener = new BrowserSignalListener(undefined, fullSettings, fakeStorageOptimized as IStorageSync, fakeSplitApi); listener.start(); @@ -116,6 +116,33 @@ test('Browser JS listener / Impressions optimized mode', () => { triggerUnloadEvent(); + // Unload event was triggered, but 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(); + + // pre-check and call stop + expect(global.window.removeEventListener).not.toBeCalled(); + listener.stop(); + + // removed correct listener from correct signal on stop. + expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); +}); + +test('Browser JS listener / standalone mode / Impressions optimized mode', () => { + const syncManagerMock = {}; + + // @ts-expect-error + const listener = new BrowserSignalListener(syncManagerMock, fullSettings, fakeStorageOptimized as IStorageSync, fakeSplitApi); + + listener.start(); + + // Assigned right function to right signal. + expect((global.window.addEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); + + triggerUnloadEvent(); + // Unload event was triggered. Thus sendBeacon method should have been called three times. expect(global.window.navigator.sendBeacon).toBeCalledTimes(3); @@ -132,7 +159,7 @@ test('Browser JS listener / Impressions optimized mode', () => { expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); }); -test('Browser JS listener / Impressions debug mode', () => { +test('Browser JS listener / standalone mode / Impressions debug mode', () => { const syncManagerMockWithPushManager = { pushManager: { stop: jest.fn() } }; // @ts-expect-error @@ -166,7 +193,7 @@ test('Browser JS listener / Impressions debug mode', () => { expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); }); -test('Browser JS listener / Impressions debug mode without sendBeacon API', () => { +test('Browser JS listener / standalone mode / Impressions debug mode without sendBeacon API', () => { // remove sendBeacon API const sendBeacon = global.navigator.sendBeacon; // @ts-expect-error global.navigator.sendBeacon = undefined; From bfe62da149cceb57ccbf88019fbf13d5f5bc87f7 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 11 Nov 2021 10:08:23 -0300 Subject: [PATCH 12/18] 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 13/18] 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 14/18] 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 15/18] 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 16/18] 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 17/18] 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 18/18] 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 */