From 0cb3a1f51c938c4cb51fb397b4f10364996eb76a Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 00:20:10 -0300 Subject: [PATCH 01/24] Move validateCache call from splitChangesUpdater to syncManagerOnline --- src/sync/__tests__/syncManagerOnline.spec.ts | 32 ++++++++++++++++--- .../polling/updaters/splitChangesUpdater.ts | 13 ++------ src/sync/syncManagerOnline.ts | 14 ++++++-- 3 files changed, 41 insertions(+), 18 deletions(-) diff --git a/src/sync/__tests__/syncManagerOnline.spec.ts b/src/sync/__tests__/syncManagerOnline.spec.ts index 7fda853b..11346c98 100644 --- a/src/sync/__tests__/syncManagerOnline.spec.ts +++ b/src/sync/__tests__/syncManagerOnline.spec.ts @@ -2,6 +2,7 @@ import { fullSettings } from '../../utils/settingsValidation/__tests__/settings. import { syncTaskFactory } from './syncTask.mock'; import { syncManagerOnlineFactory } from '../syncManagerOnline'; import { IReadinessManager } from '../../readiness/types'; +import { SDK_SPLITS_CACHE_LOADED } from '../../readiness/constants'; jest.mock('../submitters/submitterManager', () => { return { @@ -45,8 +46,10 @@ const pushManagerFactoryMock = jest.fn(() => pushManagerMock); test('syncManagerOnline should start or not the submitter depending on user consent status', () => { const settings = { ...fullSettings }; - // @ts-ignore - const syncManager = syncManagerOnlineFactory()({ settings }); + const syncManager = syncManagerOnlineFactory()({ + settings, // @ts-ignore + storage: { splits: { checkCache: () => false } }, + }); const submitterManager = syncManager.submitterManager!; syncManager.start(); @@ -95,7 +98,10 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', () // @ts-ignore // Test pushManager for main client - const syncManager = syncManagerOnlineFactory(() => pollingManagerMock, pushManagerFactoryMock)({ settings }); + const syncManager = syncManagerOnlineFactory(() => pollingManagerMock, pushManagerFactoryMock)({ + settings, // @ts-ignore + storage: { splits: { checkCache: () => false } }, + }); expect(pushManagerFactoryMock).not.toBeCalled(); @@ -161,7 +167,10 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', () settings.sync.enabled = true; // @ts-ignore // pushManager instantiation control test - const testSyncManager = syncManagerOnlineFactory(() => pollingManagerMock, pushManagerFactoryMock)({ settings }); + const testSyncManager = syncManagerOnlineFactory(() => pollingManagerMock, pushManagerFactoryMock)({ + settings, // @ts-ignore + storage: { splits: { checkCache: () => false } }, + }); expect(pushManagerFactoryMock).toBeCalled(); @@ -173,3 +182,18 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', () testSyncManager.stop(); }); + +test('syncManagerOnline should emit SDK_SPLITS_CACHE_LOADED if validateCache returns true', async () => { + const params = { + settings: fullSettings, + storage: { splits: { checkCache: () => true } }, + readiness: { splits: { emit: jest.fn() } } + }; // @ts-ignore + const syncManager = syncManagerOnlineFactory()(params); + + await syncManager.start(); + + expect(params.readiness.splits.emit).toBeCalledWith(SDK_SPLITS_CACHE_LOADED); + + syncManager.stop(); +}); diff --git a/src/sync/polling/updaters/splitChangesUpdater.ts b/src/sync/polling/updaters/splitChangesUpdater.ts index e8153987..e447227d 100644 --- a/src/sync/polling/updaters/splitChangesUpdater.ts +++ b/src/sync/polling/updaters/splitChangesUpdater.ts @@ -3,7 +3,7 @@ import { ISplitChangesFetcher } from '../fetchers/types'; import { ISplit, ISplitChangesResponse, ISplitFiltersValidation } from '../../../dtos/types'; import { ISplitsEventEmitter } from '../../../readiness/types'; import { timeout } from '../../../utils/promise/timeout'; -import { SDK_SPLITS_ARRIVED, SDK_SPLITS_CACHE_LOADED } from '../../../readiness/constants'; +import { SDK_SPLITS_ARRIVED } from '../../../readiness/constants'; import { ILogger } from '../../../logger/types'; import { SYNC_SPLITS_FETCH, SYNC_SPLITS_NEW, SYNC_SPLITS_REMOVED, SYNC_SPLITS_SEGMENTS, SYNC_SPLITS_FETCH_FAILS, SYNC_SPLITS_FETCH_RETRY } from '../../../logger/constants'; import { startsWith } from '../../../utils/lang'; @@ -153,7 +153,7 @@ export function splitChangesUpdaterFactory( */ function _splitChangesUpdater(since: number, retry = 0): Promise { log.debug(SYNC_SPLITS_FETCH, [since]); - const fetcherPromise = Promise.resolve(splitUpdateNotification ? + return Promise.resolve(splitUpdateNotification ? { splits: [splitUpdateNotification.payload], till: splitUpdateNotification.changeNumber } : splitChangesFetcher(since, noCache, till, _promiseDecorator) ) @@ -200,15 +200,6 @@ export function splitChangesUpdaterFactory( } return false; }); - - // After triggering the requests, if we have cached splits information let's notify that to emit SDK_READY_FROM_CACHE. - // Wrapping in a promise since checkCache can be async. - if (splitsEventEmitter && startingUp) { - Promise.resolve(splits.checkCache()).then(isCacheReady => { - if (isCacheReady) splitsEventEmitter.emit(SDK_SPLITS_CACHE_LOADED); - }); - } - return fetcherPromise; } let sincePromise = Promise.resolve(splits.getChangeNumber()); // `getChangeNumber` never rejects or throws error diff --git a/src/sync/syncManagerOnline.ts b/src/sync/syncManagerOnline.ts index 5410c17f..777fe41a 100644 --- a/src/sync/syncManagerOnline.ts +++ b/src/sync/syncManagerOnline.ts @@ -9,6 +9,7 @@ import { SYNC_START_POLLING, SYNC_CONTINUE_POLLING, SYNC_STOP_POLLING } from '.. import { isConsentGranted } from '../consent'; import { POLLING, STREAMING, SYNC_MODE_UPDATE } from '../utils/constants'; import { ISdkFactoryContextSync } from '../sdkFactory/types'; +import { SDK_SPLITS_CACHE_LOADED } from '../readiness/constants'; /** * Online SyncManager factory. @@ -28,7 +29,7 @@ export function syncManagerOnlineFactory( */ return function (params: ISdkFactoryContextSync): ISyncManagerCS { - const { settings, settings: { log, streamingEnabled, sync: { enabled: syncEnabled } }, telemetryTracker } = params; + const { settings, settings: { log, streamingEnabled, sync: { enabled: syncEnabled } }, telemetryTracker, storage, readiness } = params; /** Polling Manager */ const pollingManager = pollingManagerFactory && pollingManagerFactory(params); @@ -87,6 +88,13 @@ export function syncManagerOnlineFactory( start() { running = true; + if (startFirstTime) { + const isCacheLoaded = storage.splits.checkCache(); + Promise.resolve().then(() => { + if (isCacheLoaded) readiness.splits.emit(SDK_SPLITS_CACHE_LOADED); + }); + } + // start syncing splits and segments if (pollingManager) { @@ -96,7 +104,6 @@ export function syncManagerOnlineFactory( // Doesn't call `syncAll` when the syncManager is resuming if (startFirstTime) { pollingManager.syncAll(); - startFirstTime = false; } pushManager.start(); } else { @@ -105,13 +112,14 @@ export function syncManagerOnlineFactory( } else { if (startFirstTime) { pollingManager.syncAll(); - startFirstTime = false; } } } // start periodic data recording (events, impressions, telemetry). submitterManager.start(!isConsentGranted(settings)); + + startFirstTime = false; }, /** From d392cc88cc4ee1c22aefa20eb77ec85629d744f7 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 00:43:02 -0300 Subject: [PATCH 02/24] Replace splits::checkCache method with storage::validateCache method --- src/storages/AbstractSplitsCacheAsync.ts | 8 -------- src/storages/AbstractSplitsCacheSync.ts | 8 -------- src/storages/inLocalStorage/SplitsCacheInLocal.ts | 11 +---------- .../__tests__/SplitsCacheInLocal.spec.ts | 6 ------ src/storages/inLocalStorage/index.ts | 5 +++++ src/storages/types.ts | 5 +---- src/sync/__tests__/syncManagerOnline.spec.ts | 8 ++++---- src/sync/offline/syncTasks/fromObjectSyncTask.ts | 9 +++++---- src/sync/syncManagerOnline.ts | 6 ++---- src/utils/settingsValidation/storage/storageCS.ts | 2 +- 10 files changed, 19 insertions(+), 49 deletions(-) diff --git a/src/storages/AbstractSplitsCacheAsync.ts b/src/storages/AbstractSplitsCacheAsync.ts index dcf059ed..56664b22 100644 --- a/src/storages/AbstractSplitsCacheAsync.ts +++ b/src/storages/AbstractSplitsCacheAsync.ts @@ -27,14 +27,6 @@ export abstract class AbstractSplitsCacheAsync implements ISplitsCacheAsync { return Promise.resolve(true); } - /** - * Check if the splits information is already stored in cache. - * Noop, just keeping the interface. This is used by client-side implementations only. - */ - checkCache(): Promise { - return Promise.resolve(false); - } - /** * Kill `name` split and set `defaultTreatment` and `changeNumber`. * Used for SPLIT_KILL push notifications. diff --git a/src/storages/AbstractSplitsCacheSync.ts b/src/storages/AbstractSplitsCacheSync.ts index f82ebbd6..09c131ba 100644 --- a/src/storages/AbstractSplitsCacheSync.ts +++ b/src/storages/AbstractSplitsCacheSync.ts @@ -47,14 +47,6 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync { abstract clear(): void - /** - * Check if the splits information is already stored in cache. This data can be preloaded. - * It is used as condition to emit SDK_SPLITS_CACHE_LOADED, and then SDK_READY_FROM_CACHE. - */ - checkCache(): boolean { - return false; - } - /** * Kill `name` split and set `defaultTreatment` and `changeNumber`. * Used for SPLIT_KILL push notifications. diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index 93eb6f32..92057d4a 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -212,15 +212,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { } } - /** - * Check if the splits information is already stored in browser LocalStorage. - * In this function we could add more code to check if the data is valid. - * @override - */ - checkCache(): boolean { - return this.getChangeNumber() > -1; - } - /** * Clean Splits cache if its `lastUpdated` timestamp is older than the given `expirationTimestamp`, * @@ -245,7 +236,7 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { this.updateNewFilter = true; // if there is cache, clear it - if (this.checkCache()) this.clear(); + if (this.getChangeNumber() > -1) this.clear(); } catch (e) { this.log.error(LOG_PREFIX + e); diff --git a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts index 4d8ec076..17b2584d 100644 --- a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts +++ b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts @@ -30,16 +30,10 @@ test('SPLIT CACHE / LocalStorage', () => { expect(cache.getSplit('lol1')).toEqual(null); expect(cache.getSplit('lol2')).toEqual(somethingElse); - expect(cache.checkCache()).toBe(false); // checkCache should return false until localstorage has data. - expect(cache.getChangeNumber() === -1).toBe(true); - expect(cache.checkCache()).toBe(false); // checkCache should return false until localstorage has data. - cache.setChangeNumber(123); - expect(cache.checkCache()).toBe(true); // checkCache should return true once localstorage has data. - expect(cache.getChangeNumber() === 123).toBe(true); }); diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 871be592..1d16cd62 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -53,6 +53,11 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn telemetry: shouldRecordTelemetry(params) ? new TelemetryCacheInMemory(splits, segments) : undefined, uniqueKeys: impressionsMode === NONE ? new UniqueKeysCacheInMemoryCS() : undefined, + // @TODO implement + validateCache() { + return splits.getChangeNumber() > -1; + }, + destroy() { }, // When using shared instantiation with MEMORY we reuse everything but segments (they are customer per key). diff --git a/src/storages/types.ts b/src/storages/types.ts index 5d7b40b6..8c63f452 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -191,8 +191,6 @@ export interface ISplitsCacheBase { // only for Client-Side. Returns true if the storage is not synchronized yet (getChangeNumber() === -1) or contains a FF using segments or large segments usesSegments(): MaybeThenable, clear(): MaybeThenable, - // should never reject or throw an exception. Instead return false by default, to avoid emitting SDK_READY_FROM_CACHE. - checkCache(): MaybeThenable, killLocally(name: string, defaultTreatment: string, changeNumber: number): MaybeThenable, getNamesByFlagSets(flagSets: string[]): MaybeThenable[]> } @@ -209,7 +207,6 @@ export interface ISplitsCacheSync extends ISplitsCacheBase { trafficTypeExists(trafficType: string): boolean, usesSegments(): boolean, clear(): void, - checkCache(): boolean, killLocally(name: string, defaultTreatment: string, changeNumber: number): boolean, getNamesByFlagSets(flagSets: string[]): Set[] } @@ -226,7 +223,6 @@ export interface ISplitsCacheAsync extends ISplitsCacheBase { trafficTypeExists(trafficType: string): Promise, usesSegments(): Promise, clear(): Promise, - checkCache(): Promise, killLocally(name: string, defaultTreatment: string, changeNumber: number): Promise, getNamesByFlagSets(flagSets: string[]): Promise[]> } @@ -457,6 +453,7 @@ export interface IStorageSync extends IStorageBase< IUniqueKeysCacheSync > { // Defined in client-side + validateCache?: () => boolean, // @TODO support async largeSegments?: ISegmentsCacheSync, } diff --git a/src/sync/__tests__/syncManagerOnline.spec.ts b/src/sync/__tests__/syncManagerOnline.spec.ts index 11346c98..c7dba96e 100644 --- a/src/sync/__tests__/syncManagerOnline.spec.ts +++ b/src/sync/__tests__/syncManagerOnline.spec.ts @@ -48,7 +48,7 @@ test('syncManagerOnline should start or not the submitter depending on user cons const syncManager = syncManagerOnlineFactory()({ settings, // @ts-ignore - storage: { splits: { checkCache: () => false } }, + storage: {}, }); const submitterManager = syncManager.submitterManager!; @@ -100,7 +100,7 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', () // Test pushManager for main client const syncManager = syncManagerOnlineFactory(() => pollingManagerMock, pushManagerFactoryMock)({ settings, // @ts-ignore - storage: { splits: { checkCache: () => false } }, + storage: { validateCache: () => false }, }); expect(pushManagerFactoryMock).not.toBeCalled(); @@ -169,7 +169,7 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', () // pushManager instantiation control test const testSyncManager = syncManagerOnlineFactory(() => pollingManagerMock, pushManagerFactoryMock)({ settings, // @ts-ignore - storage: { splits: { checkCache: () => false } }, + storage: { validateCache: () => false }, }); expect(pushManagerFactoryMock).toBeCalled(); @@ -186,7 +186,7 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', () test('syncManagerOnline should emit SDK_SPLITS_CACHE_LOADED if validateCache returns true', async () => { const params = { settings: fullSettings, - storage: { splits: { checkCache: () => true } }, + storage: { validateCache: () => true }, readiness: { splits: { emit: jest.fn() } } }; // @ts-ignore const syncManager = syncManagerOnlineFactory()(params); diff --git a/src/sync/offline/syncTasks/fromObjectSyncTask.ts b/src/sync/offline/syncTasks/fromObjectSyncTask.ts index 84805110..1b3f97f8 100644 --- a/src/sync/offline/syncTasks/fromObjectSyncTask.ts +++ b/src/sync/offline/syncTasks/fromObjectSyncTask.ts @@ -1,6 +1,6 @@ import { forOwn } from '../../../utils/lang'; import { IReadinessManager } from '../../../readiness/types'; -import { ISplitsCacheSync } from '../../../storages/types'; +import { ISplitsCacheSync, IStorageSync } from '../../../storages/types'; import { ISplitsParser } from '../splitsParser/types'; import { ISplit, ISplitPartial } from '../../../dtos/types'; import { syncTaskFactory } from '../../syncTask'; @@ -15,7 +15,7 @@ import { SYNC_OFFLINE_DATA, ERROR_SYNC_OFFLINE_LOADING } from '../../../logger/c */ export function fromObjectUpdaterFactory( splitsParser: ISplitsParser, - storage: { splits: ISplitsCacheSync }, + storage: Pick, readiness: IReadinessManager, settings: ISettings, ): () => Promise { @@ -60,9 +60,10 @@ export function fromObjectUpdaterFactory( if (startingUp) { startingUp = false; - Promise.resolve(splitsCache.checkCache()).then(cacheReady => { + const isCacheLoaded = storage.validateCache ? storage.validateCache() : false; + Promise.resolve().then(() => { // Emits SDK_READY_FROM_CACHE - if (cacheReady) readiness.splits.emit(SDK_SPLITS_CACHE_LOADED); + if (isCacheLoaded) readiness.splits.emit(SDK_SPLITS_CACHE_LOADED); // Emits SDK_READY readiness.segments.emit(SDK_SEGMENTS_ARRIVED); }); diff --git a/src/sync/syncManagerOnline.ts b/src/sync/syncManagerOnline.ts index 777fe41a..aed32493 100644 --- a/src/sync/syncManagerOnline.ts +++ b/src/sync/syncManagerOnline.ts @@ -89,10 +89,8 @@ export function syncManagerOnlineFactory( running = true; if (startFirstTime) { - const isCacheLoaded = storage.splits.checkCache(); - Promise.resolve().then(() => { - if (isCacheLoaded) readiness.splits.emit(SDK_SPLITS_CACHE_LOADED); - }); + const isCacheLoaded = storage.validateCache ? storage.validateCache() : false; + if (isCacheLoaded) Promise.resolve().then(() => { readiness.splits.emit(SDK_SPLITS_CACHE_LOADED); }); } // start syncing splits and segments diff --git a/src/utils/settingsValidation/storage/storageCS.ts b/src/utils/settingsValidation/storage/storageCS.ts index f7b531fc..7d58af3d 100644 --- a/src/utils/settingsValidation/storage/storageCS.ts +++ b/src/utils/settingsValidation/storage/storageCS.ts @@ -8,7 +8,7 @@ import { IStorageFactoryParams, IStorageSync } from '../../../storages/types'; export function __InLocalStorageMockFactory(params: IStorageFactoryParams): IStorageSync { const result = InMemoryStorageCSFactory(params); - result.splits.checkCache = () => true; // to emit SDK_READY_FROM_CACHE + result.validateCache = () => true; // to emit SDK_READY_FROM_CACHE return result; } __InLocalStorageMockFactory.type = STORAGE_MEMORY; From 893833455985edf25367457e6b9281bc203d5ecb Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 13:54:21 -0300 Subject: [PATCH 03/24] Extract validation logic into SplitsCacheInLocal::validateCache method --- .../inLocalStorage/SplitsCacheInLocal.ts | 70 +++++++++---------- .../__tests__/SplitsCacheInLocal.spec.ts | 31 +++++--- src/storages/inLocalStorage/index.ts | 4 +- 3 files changed, 57 insertions(+), 48 deletions(-) diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index 92057d4a..1fa739aa 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -20,16 +20,47 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { private hasSync?: boolean; private updateNewFilter?: boolean; - constructor(settings: ISettings, keys: KeyBuilderCS, expirationTimestamp?: number) { + constructor(settings: ISettings, keys: KeyBuilderCS) { super(); this.keys = keys; this.log = settings.log; this.storageHash = getStorageHash(settings); this.flagSetsFilter = settings.sync.__splitFiltersValidation.groupedFilters.bySet; + } - this._checkExpiration(expirationTimestamp); + /** + * Clean Splits cache if its `lastUpdated` timestamp is older than the given `expirationTimestamp`, + * + * @param expirationTimestamp - if the value is not a number, data will not be cleaned + */ + public validateCache(expirationTimestamp?: number) { + // _checkExpiration + let value: string | number | null = localStorage.getItem(this.keys.buildLastUpdatedKey()); + if (value !== null) { + value = parseInt(value, 10); + if (!isNaNNumber(value) && expirationTimestamp && value < expirationTimestamp) this.clear(); + } - this._checkFilterQuery(); + // @TODO eventually remove `_checkFilterQuery`. Cache should be cleared at the storage level, reusing same logic than PluggableStorage + // _checkFilterQuery + const storageHashKey = this.keys.buildHashKey(); + const storageHash = localStorage.getItem(storageHashKey); + + if (storageHash !== this.storageHash) { + try { + // mark cache to update the new query filter on first successful splits fetch + this.updateNewFilter = true; + + // if there is cache, clear it + if (this.getChangeNumber() > -1) this.clear(); + + } catch (e) { + this.log.error(LOG_PREFIX + e); + } + } + // if the filter didn't change, nothing is done + + return this.getChangeNumber() > -1; } private _decrementCount(key: string) { @@ -212,39 +243,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { } } - /** - * Clean Splits cache if its `lastUpdated` timestamp is older than the given `expirationTimestamp`, - * - * @param expirationTimestamp - if the value is not a number, data will not be cleaned - */ - private _checkExpiration(expirationTimestamp?: number) { - let value: string | number | null = localStorage.getItem(this.keys.buildLastUpdatedKey()); - if (value !== null) { - value = parseInt(value, 10); - if (!isNaNNumber(value) && expirationTimestamp && value < expirationTimestamp) this.clear(); - } - } - - // @TODO eventually remove `_checkFilterQuery`. Cache should be cleared at the storage level, reusing same logic than PluggableStorage - private _checkFilterQuery() { - const storageHashKey = this.keys.buildHashKey(); - const storageHash = localStorage.getItem(storageHashKey); - - if (storageHash !== this.storageHash) { - try { - // mark cache to update the new query filter on first successful splits fetch - this.updateNewFilter = true; - - // if there is cache, clear it - if (this.getChangeNumber() > -1) this.clear(); - - } catch (e) { - this.log.error(LOG_PREFIX + e); - } - } - // if the filter didn't change, nothing is done - } - getNamesByFlagSets(flagSets: string[]): Set[] { return flagSets.map(flagSet => { const flagSetKey = this.keys.buildFlagSetKey(flagSet); diff --git a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts index 17b2584d..26dcbaf1 100644 --- a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts +++ b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts @@ -7,6 +7,7 @@ import { fullSettings } from '../../../utils/settingsValidation/__tests__/settin test('SPLIT CACHE / LocalStorage', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); + cache.validateCache(); cache.clear(); @@ -40,6 +41,7 @@ test('SPLIT CACHE / LocalStorage', () => { test('SPLIT CACHE / LocalStorage / Get Keys', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); + cache.validateCache(); cache.addSplit('lol1', something); cache.addSplit('lol2', somethingElse); @@ -52,6 +54,7 @@ test('SPLIT CACHE / LocalStorage / Get Keys', () => { test('SPLIT CACHE / LocalStorage / Add Splits', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); + cache.validateCache(); cache.addSplits([ ['lol1', something], @@ -66,6 +69,7 @@ test('SPLIT CACHE / LocalStorage / Add Splits', () => { test('SPLIT CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); + cache.validateCache(); cache.addSplits([ // loop of addSplit ['split1', splitWithUserTT], @@ -104,6 +108,8 @@ test('SPLIT CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => { test('SPLIT CACHE / LocalStorage / killLocally', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); + cache.validateCache(); + cache.addSplit('lol1', something); cache.addSplit('lol2', somethingElse); const initialChangeNumber = cache.getChangeNumber(); @@ -136,6 +142,7 @@ test('SPLIT CACHE / LocalStorage / killLocally', () => { test('SPLIT CACHE / LocalStorage / usesSegments', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); + cache.validateCache(); expect(cache.usesSegments()).toBe(true); // true initially, until data is synchronized cache.setChangeNumber(1); // to indicate that data has been synced. @@ -167,6 +174,8 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => { } } }, new KeyBuilderCS('SPLITIO', 'user')); + cache.validateCache(); + const emptySet = new Set([]); cache.addSplits([ @@ -206,25 +215,27 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => { // if FlagSets are not defined, it should store all FlagSets in memory. test('SPLIT CACHE / LocalStorage / flag set cache tests without filters', () => { - const cacheWithoutFilters = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); + const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); + cache.validateCache(); + const emptySet = new Set([]); - cacheWithoutFilters.addSplits([ + cache.addSplits([ [featureFlagOne.name, featureFlagOne], [featureFlagTwo.name, featureFlagTwo], [featureFlagThree.name, featureFlagThree], ]); - cacheWithoutFilters.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); + cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS); - expect(cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]); - expect(cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]); - expect(cacheWithoutFilters.getNamesByFlagSets(['e'])).toEqual([new Set(['ff_one', 'ff_three'])]); - expect(cacheWithoutFilters.getNamesByFlagSets(['t'])).toEqual([new Set(['ff_two', 'ff_three'])]); - expect(cacheWithoutFilters.getNamesByFlagSets(['y'])).toEqual([emptySet]); - expect(cacheWithoutFilters.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new Set(['ff_one', 'ff_two']), new Set(['ff_one']), new Set(['ff_one', 'ff_three'])]); + expect(cache.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]); + expect(cache.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]); + expect(cache.getNamesByFlagSets(['e'])).toEqual([new Set(['ff_one', 'ff_three'])]); + expect(cache.getNamesByFlagSets(['t'])).toEqual([new Set(['ff_two', 'ff_three'])]); + expect(cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); + expect(cache.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new Set(['ff_one', 'ff_two']), new Set(['ff_one']), new Set(['ff_one', 'ff_three'])]); // Validate that the feature flag cache is cleared when calling `clear` method - cacheWithoutFilters.clear(); + cache.clear(); expect(localStorage.length).toBe(1); // only 'SPLITIO.hash' should remain in localStorage expect(localStorage.key(0)).toBe('SPLITIO.hash'); }); diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 1d16cd62..cce6568c 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -39,7 +39,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn const keys = new KeyBuilderCS(prefix, matchingKey); const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS; - const splits = new SplitsCacheInLocal(settings, keys, expirationTimestamp); + const splits = new SplitsCacheInLocal(settings, keys); const segments = new MySegmentsCacheInLocal(log, keys); const largeSegments = new MySegmentsCacheInLocal(log, myLargeSegmentsKeyBuilder(prefix, matchingKey)); @@ -55,7 +55,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn // @TODO implement validateCache() { - return splits.getChangeNumber() > -1; + return splits.validateCache(expirationTimestamp); }, destroy() { }, From 1b061eb06b855914cdec2bc05fc94ac13ba80850 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 13:57:33 -0300 Subject: [PATCH 04/24] Simplify SplitsCacheInLocal::setChangeNumber moving clear logic to SplitsCacheInLocal::validateCache --- .../inLocalStorage/SplitsCacheInLocal.ts | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index 1fa739aa..42a1cb75 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -18,7 +18,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { private readonly storageHash: string; private readonly flagSetsFilter: string[]; private hasSync?: boolean; - private updateNewFilter?: boolean; constructor(settings: ISettings, keys: KeyBuilderCS) { super(); @@ -47,13 +46,12 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { const storageHash = localStorage.getItem(storageHashKey); if (storageHash !== this.storageHash) { + this.log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version was modified. Updating cache'); try { - // mark cache to update the new query filter on first successful splits fetch - this.updateNewFilter = true; - // if there is cache, clear it if (this.getChangeNumber() > -1) this.clear(); + localStorage.setItem(storageHashKey, this.storageHash); } catch (e) { this.log.error(LOG_PREFIX + e); } @@ -169,19 +167,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { } setChangeNumber(changeNumber: number): boolean { - - // when using a new split query, we must update it at the store - if (this.updateNewFilter) { - this.log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version was modified. Updating cache'); - const storageHashKey = this.keys.buildHashKey(); - try { - localStorage.setItem(storageHashKey, this.storageHash); - } catch (e) { - this.log.error(LOG_PREFIX + e); - } - this.updateNewFilter = false; - } - try { localStorage.setItem(this.keys.buildSplitsTillKey(), changeNumber + ''); // update "last updated" timestamp with current time From ffd724dd3fdfec27695b6d4449420427494d6684 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 14:03:47 -0300 Subject: [PATCH 05/24] Remove SplitsCacheInLocal::storageHash --- .../inLocalStorage/SplitsCacheInLocal.ts | 9 ++++----- .../__tests__/SplitsCacheInLocal.spec.ts | 16 ++++++++-------- src/storages/inLocalStorage/index.ts | 2 +- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index 42a1cb75..dd28bd41 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -15,7 +15,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { private readonly keys: KeyBuilderCS; private readonly log: ILogger; - private readonly storageHash: string; private readonly flagSetsFilter: string[]; private hasSync?: boolean; @@ -23,7 +22,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { super(); this.keys = keys; this.log = settings.log; - this.storageHash = getStorageHash(settings); this.flagSetsFilter = settings.sync.__splitFiltersValidation.groupedFilters.bySet; } @@ -32,7 +30,7 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { * * @param expirationTimestamp - if the value is not a number, data will not be cleaned */ - public validateCache(expirationTimestamp?: number) { + public validateCache(settings: ISettings, expirationTimestamp?: number) { // _checkExpiration let value: string | number | null = localStorage.getItem(this.keys.buildLastUpdatedKey()); if (value !== null) { @@ -44,14 +42,15 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { // _checkFilterQuery const storageHashKey = this.keys.buildHashKey(); const storageHash = localStorage.getItem(storageHashKey); + const currentStorageHash = getStorageHash(settings); - if (storageHash !== this.storageHash) { + if (storageHash !== currentStorageHash) { this.log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version was modified. Updating cache'); try { // if there is cache, clear it if (this.getChangeNumber() > -1) this.clear(); - localStorage.setItem(storageHashKey, this.storageHash); + localStorage.setItem(storageHashKey, currentStorageHash); } catch (e) { this.log.error(LOG_PREFIX + e); } diff --git a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts index 26dcbaf1..62dad318 100644 --- a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts +++ b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts @@ -7,7 +7,7 @@ import { fullSettings } from '../../../utils/settingsValidation/__tests__/settin test('SPLIT CACHE / LocalStorage', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(); + cache.validateCache(fullSettings); cache.clear(); @@ -41,7 +41,7 @@ test('SPLIT CACHE / LocalStorage', () => { test('SPLIT CACHE / LocalStorage / Get Keys', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(); + cache.validateCache(fullSettings); cache.addSplit('lol1', something); cache.addSplit('lol2', somethingElse); @@ -54,7 +54,7 @@ test('SPLIT CACHE / LocalStorage / Get Keys', () => { test('SPLIT CACHE / LocalStorage / Add Splits', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(); + cache.validateCache(fullSettings); cache.addSplits([ ['lol1', something], @@ -69,7 +69,7 @@ test('SPLIT CACHE / LocalStorage / Add Splits', () => { test('SPLIT CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(); + cache.validateCache(fullSettings); cache.addSplits([ // loop of addSplit ['split1', splitWithUserTT], @@ -108,7 +108,7 @@ test('SPLIT CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => { test('SPLIT CACHE / LocalStorage / killLocally', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(); + cache.validateCache(fullSettings); cache.addSplit('lol1', something); cache.addSplit('lol2', somethingElse); @@ -142,7 +142,7 @@ test('SPLIT CACHE / LocalStorage / killLocally', () => { test('SPLIT CACHE / LocalStorage / usesSegments', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(); + cache.validateCache(fullSettings); expect(cache.usesSegments()).toBe(true); // true initially, until data is synchronized cache.setChangeNumber(1); // to indicate that data has been synced. @@ -174,7 +174,7 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => { } } }, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(); + cache.validateCache(fullSettings); const emptySet = new Set([]); @@ -216,7 +216,7 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => { // if FlagSets are not defined, it should store all FlagSets in memory. test('SPLIT CACHE / LocalStorage / flag set cache tests without filters', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(); + cache.validateCache(fullSettings); const emptySet = new Set([]); diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index cce6568c..a42e6a12 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -55,7 +55,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn // @TODO implement validateCache() { - return splits.validateCache(expirationTimestamp); + return splits.validateCache(settings, expirationTimestamp); }, destroy() { }, From e13f81980bfcd3945fdb0ec799152d3a5ad7226c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 14:12:22 -0300 Subject: [PATCH 06/24] Move expirationTimestamp logic inside validateCache method --- src/storages/inLocalStorage/SplitsCacheInLocal.ts | 12 +++++++----- src/storages/inLocalStorage/index.ts | 4 +--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index dd28bd41..bd5ea343 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -7,6 +7,7 @@ import { LOG_PREFIX } from './constants'; import { ISettings } from '../../types'; import { getStorageHash } from '../KeyBuilder'; import { setToArray } from '../../utils/lang/sets'; +import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../../utils/constants/browser'; /** * ISplitsCacheSync implementation that stores split definitions in browser LocalStorage. @@ -26,16 +27,17 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { } /** - * Clean Splits cache if its `lastUpdated` timestamp is older than the given `expirationTimestamp`, - * - * @param expirationTimestamp - if the value is not a number, data will not be cleaned + * Clean Splits cache if: + * - it has expired, i.e., its `lastUpdated` timestamp is older than the given `expirationTimestamp` + * - hash has changes, i.e., the SDK key, flags filter criteria or flags spec version was modified */ - public validateCache(settings: ISettings, expirationTimestamp?: number) { + public validateCache(settings: ISettings) { // _checkExpiration + const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS; let value: string | number | null = localStorage.getItem(this.keys.buildLastUpdatedKey()); if (value !== null) { value = parseInt(value, 10); - if (!isNaNNumber(value) && expirationTimestamp && value < expirationTimestamp) this.clear(); + if (!isNaNNumber(value) && value < expirationTimestamp) this.clear(); } // @TODO eventually remove `_checkFilterQuery`. Cache should be cleared at the storage level, reusing same logic than PluggableStorage diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index a42e6a12..58085e17 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -7,7 +7,6 @@ import { KeyBuilderCS, myLargeSegmentsKeyBuilder } from '../KeyBuilderCS'; import { isLocalStorageAvailable } from '../../utils/env/isLocalStorageAvailable'; import { SplitsCacheInLocal } from './SplitsCacheInLocal'; import { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal'; -import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../../utils/constants/browser'; import { InMemoryStorageCSFactory } from '../inMemory/InMemoryStorageCS'; import { LOG_PREFIX } from './constants'; import { DEBUG, NONE, STORAGE_LOCALSTORAGE } from '../../utils/constants'; @@ -37,7 +36,6 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn const { settings, settings: { log, scheduler: { impressionsQueueSize, eventsQueueSize, }, sync: { impressionsMode } } } = params; const matchingKey = getMatching(settings.core.key); const keys = new KeyBuilderCS(prefix, matchingKey); - const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS; const splits = new SplitsCacheInLocal(settings, keys); const segments = new MySegmentsCacheInLocal(log, keys); @@ -55,7 +53,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn // @TODO implement validateCache() { - return splits.validateCache(settings, expirationTimestamp); + return splits.validateCache(settings); }, destroy() { }, From b32e3eeb3841dd01e2f20beaa99847614a4861e3 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 15:00:33 -0300 Subject: [PATCH 07/24] Move validateCache logic outside SplitsCacheInLocal --- .../inLocalStorage/SplitsCacheInLocal.ts | 38 ---------------- .../__tests__/SplitsCacheInLocal.spec.ts | 11 +---- src/storages/inLocalStorage/index.ts | 4 +- src/storages/inLocalStorage/validateCache.ts | 43 +++++++++++++++++++ src/storages/pluggable/index.ts | 3 +- 5 files changed, 48 insertions(+), 51 deletions(-) create mode 100644 src/storages/inLocalStorage/validateCache.ts diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index bd5ea343..14767d72 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -5,9 +5,7 @@ import { KeyBuilderCS } from '../KeyBuilderCS'; import { ILogger } from '../../logger/types'; import { LOG_PREFIX } from './constants'; import { ISettings } from '../../types'; -import { getStorageHash } from '../KeyBuilder'; import { setToArray } from '../../utils/lang/sets'; -import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../../utils/constants/browser'; /** * ISplitsCacheSync implementation that stores split definitions in browser LocalStorage. @@ -26,42 +24,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { this.flagSetsFilter = settings.sync.__splitFiltersValidation.groupedFilters.bySet; } - /** - * Clean Splits cache if: - * - it has expired, i.e., its `lastUpdated` timestamp is older than the given `expirationTimestamp` - * - hash has changes, i.e., the SDK key, flags filter criteria or flags spec version was modified - */ - public validateCache(settings: ISettings) { - // _checkExpiration - const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS; - let value: string | number | null = localStorage.getItem(this.keys.buildLastUpdatedKey()); - if (value !== null) { - value = parseInt(value, 10); - if (!isNaNNumber(value) && value < expirationTimestamp) this.clear(); - } - - // @TODO eventually remove `_checkFilterQuery`. Cache should be cleared at the storage level, reusing same logic than PluggableStorage - // _checkFilterQuery - const storageHashKey = this.keys.buildHashKey(); - const storageHash = localStorage.getItem(storageHashKey); - const currentStorageHash = getStorageHash(settings); - - if (storageHash !== currentStorageHash) { - this.log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version was modified. Updating cache'); - try { - // if there is cache, clear it - if (this.getChangeNumber() > -1) this.clear(); - - localStorage.setItem(storageHashKey, currentStorageHash); - } catch (e) { - this.log.error(LOG_PREFIX + e); - } - } - // if the filter didn't change, nothing is done - - return this.getChangeNumber() > -1; - } - private _decrementCount(key: string) { const count = toNumber(localStorage.getItem(key)) - 1; // @ts-expect-error diff --git a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts index 62dad318..1b3c8183 100644 --- a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts +++ b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts @@ -7,7 +7,6 @@ import { fullSettings } from '../../../utils/settingsValidation/__tests__/settin test('SPLIT CACHE / LocalStorage', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(fullSettings); cache.clear(); @@ -41,7 +40,6 @@ test('SPLIT CACHE / LocalStorage', () => { test('SPLIT CACHE / LocalStorage / Get Keys', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(fullSettings); cache.addSplit('lol1', something); cache.addSplit('lol2', somethingElse); @@ -54,7 +52,6 @@ test('SPLIT CACHE / LocalStorage / Get Keys', () => { test('SPLIT CACHE / LocalStorage / Add Splits', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(fullSettings); cache.addSplits([ ['lol1', something], @@ -69,7 +66,6 @@ test('SPLIT CACHE / LocalStorage / Add Splits', () => { test('SPLIT CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(fullSettings); cache.addSplits([ // loop of addSplit ['split1', splitWithUserTT], @@ -108,7 +104,6 @@ test('SPLIT CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => { test('SPLIT CACHE / LocalStorage / killLocally', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(fullSettings); cache.addSplit('lol1', something); cache.addSplit('lol2', somethingElse); @@ -142,7 +137,6 @@ test('SPLIT CACHE / LocalStorage / killLocally', () => { test('SPLIT CACHE / LocalStorage / usesSegments', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(fullSettings); expect(cache.usesSegments()).toBe(true); // true initially, until data is synchronized cache.setChangeNumber(1); // to indicate that data has been synced. @@ -174,7 +168,6 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => { } } }, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(fullSettings); const emptySet = new Set([]); @@ -216,7 +209,6 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => { // if FlagSets are not defined, it should store all FlagSets in memory. test('SPLIT CACHE / LocalStorage / flag set cache tests without filters', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - cache.validateCache(fullSettings); const emptySet = new Set([]); @@ -236,6 +228,5 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests without filters', () => // Validate that the feature flag cache is cleared when calling `clear` method cache.clear(); - expect(localStorage.length).toBe(1); // only 'SPLITIO.hash' should remain in localStorage - expect(localStorage.key(0)).toBe('SPLITIO.hash'); + expect(localStorage.length).toBe(0); }); diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 58085e17..cb14a235 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -13,6 +13,7 @@ import { DEBUG, NONE, STORAGE_LOCALSTORAGE } from '../../utils/constants'; import { shouldRecordTelemetry, TelemetryCacheInMemory } from '../inMemory/TelemetryCacheInMemory'; import { UniqueKeysCacheInMemoryCS } from '../inMemory/UniqueKeysCacheInMemoryCS'; import { getMatching } from '../../utils/key'; +import { validateCache } from './validateCache'; export interface InLocalStorageOptions { prefix?: string @@ -51,9 +52,8 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn telemetry: shouldRecordTelemetry(params) ? new TelemetryCacheInMemory(splits, segments) : undefined, uniqueKeys: impressionsMode === NONE ? new UniqueKeysCacheInMemoryCS() : undefined, - // @TODO implement validateCache() { - return splits.validateCache(settings); + return validateCache(settings, keys, splits); }, destroy() { }, diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts new file mode 100644 index 00000000..3fa5f5ae --- /dev/null +++ b/src/storages/inLocalStorage/validateCache.ts @@ -0,0 +1,43 @@ +import { ISettings } from '../../types'; +import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../../utils/constants/browser'; +import { isNaNNumber } from '../../utils/lang'; +import { getStorageHash } from '../KeyBuilder'; +import { LOG_PREFIX } from './constants'; +import type { SplitsCacheInLocal } from './SplitsCacheInLocal'; +import { KeyBuilderCS } from '../KeyBuilderCS'; + +/** + * Clean cache if: + * - it has expired, i.e., its `lastUpdated` timestamp is older than the given `expirationTimestamp` + * - hash has changed, i.e., the SDK key, flags filter criteria or flags spec version was modified + */ +export function validateCache(settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal): boolean { + const { log } = settings; + + // Check expiration and clear cache if needed + const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS; + let value: string | number | null = localStorage.getItem(keys.buildLastUpdatedKey()); + if (value !== null) { + value = parseInt(value, 10); + if (!isNaNNumber(value) && value < expirationTimestamp) splits.clear(); + } + + // Check hash and clear cache if needed + const storageHashKey = keys.buildHashKey(); + const storageHash = localStorage.getItem(storageHashKey); + const currentStorageHash = getStorageHash(settings); + + if (storageHash !== currentStorageHash) { + log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version was modified. Updating cache'); + try { + if (splits.getChangeNumber() > -1) splits.clear(); + + localStorage.setItem(storageHashKey, currentStorageHash); + } catch (e) { + log.error(LOG_PREFIX + e); + } + } + + // Check if the cache is ready + return splits.getChangeNumber() > -1; +} diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index 372eeeb4..67f92b05 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -92,7 +92,8 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn // Connects to wrapper and emits SDK_READY event on main client const connectPromise = wrapper.connect().then(() => { if (isSyncronizer) { - // In standalone or producer mode, clear storage if SDK key or feature flag filter has changed + // @TODO reuse InLocalStorage::validateCache logic + // In standalone or producer mode, clear storage if SDK key, flags filter criteria or flags spec version was modified return wrapper.get(keys.buildHashKey()).then((hash) => { const currentHash = getStorageHash(settings); if (hash !== currentHash) { From 679f841c0389da146b8ccdeaf094f3b44c35d875 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 15:17:45 -0300 Subject: [PATCH 08/24] Polishing --- src/sync/offline/syncTasks/fromObjectSyncTask.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sync/offline/syncTasks/fromObjectSyncTask.ts b/src/sync/offline/syncTasks/fromObjectSyncTask.ts index 1b3f97f8..d7184fcc 100644 --- a/src/sync/offline/syncTasks/fromObjectSyncTask.ts +++ b/src/sync/offline/syncTasks/fromObjectSyncTask.ts @@ -1,6 +1,6 @@ import { forOwn } from '../../../utils/lang'; import { IReadinessManager } from '../../../readiness/types'; -import { ISplitsCacheSync, IStorageSync } from '../../../storages/types'; +import { IStorageSync } from '../../../storages/types'; import { ISplitsParser } from '../splitsParser/types'; import { ISplit, ISplitPartial } from '../../../dtos/types'; import { syncTaskFactory } from '../../syncTask'; @@ -81,7 +81,7 @@ export function fromObjectUpdaterFactory( */ export function fromObjectSyncTaskFactory( splitsParser: ISplitsParser, - storage: { splits: ISplitsCacheSync }, + storage: Pick, readiness: IReadinessManager, settings: ISettings ): ISyncTask<[], boolean> { From 6605bfc56e5a4ba11378847fff667ba3243cc278 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 15:39:58 -0300 Subject: [PATCH 09/24] Refactor validateCache function --- src/storages/inLocalStorage/validateCache.ts | 28 ++++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts index 3fa5f5ae..331a8df9 100644 --- a/src/storages/inLocalStorage/validateCache.ts +++ b/src/storages/inLocalStorage/validateCache.ts @@ -6,23 +6,18 @@ import { LOG_PREFIX } from './constants'; import type { SplitsCacheInLocal } from './SplitsCacheInLocal'; import { KeyBuilderCS } from '../KeyBuilderCS'; -/** - * Clean cache if: - * - it has expired, i.e., its `lastUpdated` timestamp is older than the given `expirationTimestamp` - * - hash has changed, i.e., the SDK key, flags filter criteria or flags spec version was modified - */ -export function validateCache(settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal): boolean { +function validateExpiration(settings: ISettings, keys: KeyBuilderCS) { const { log } = settings; - // Check expiration and clear cache if needed + // Check expiration const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS; let value: string | number | null = localStorage.getItem(keys.buildLastUpdatedKey()); if (value !== null) { value = parseInt(value, 10); - if (!isNaNNumber(value) && value < expirationTimestamp) splits.clear(); + if (!isNaNNumber(value) && value < expirationTimestamp) return true; } - // Check hash and clear cache if needed + // Check hash const storageHashKey = keys.buildHashKey(); const storageHash = localStorage.getItem(storageHashKey); const currentStorageHash = getStorageHash(settings); @@ -30,12 +25,23 @@ export function validateCache(settings: ISettings, keys: KeyBuilderCS, splits: S if (storageHash !== currentStorageHash) { log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version was modified. Updating cache'); try { - if (splits.getChangeNumber() > -1) splits.clear(); - localStorage.setItem(storageHashKey, currentStorageHash); } catch (e) { log.error(LOG_PREFIX + e); } + return true; + } +} + +/** + * Clean cache if: + * - it has expired, i.e., its `lastUpdated` timestamp is older than the given `expirationTimestamp` + * - hash has changed, i.e., the SDK key, flags filter criteria or flags spec version was modified + */ +export function validateCache(settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal): boolean { + + if (validateExpiration(settings, keys)) { + splits.clear(); } // Check if the cache is ready From 9b8d36af77c55ead5cfafd835d25aae8a3ea4b16 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 15:48:49 -0300 Subject: [PATCH 10/24] Clear segments and largeSegments caches --- src/storages/inLocalStorage/index.ts | 9 +++------ src/storages/inLocalStorage/validateCache.ts | 5 ++++- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index cb14a235..f32cc014 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -14,15 +14,12 @@ import { shouldRecordTelemetry, TelemetryCacheInMemory } from '../inMemory/Telem import { UniqueKeysCacheInMemoryCS } from '../inMemory/UniqueKeysCacheInMemoryCS'; import { getMatching } from '../../utils/key'; import { validateCache } from './validateCache'; - -export interface InLocalStorageOptions { - prefix?: string -} +import SplitIO from '../../../types/splitio'; /** * InLocal storage factory for standalone client-side SplitFactory */ -export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyncFactory { +export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): IStorageSyncFactory { const prefix = validatePrefix(options.prefix); @@ -53,7 +50,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn uniqueKeys: impressionsMode === NONE ? new UniqueKeysCacheInMemoryCS() : undefined, validateCache() { - return validateCache(settings, keys, splits); + return validateCache(settings, keys, splits, segments, largeSegments); }, destroy() { }, diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts index 331a8df9..f76b77ca 100644 --- a/src/storages/inLocalStorage/validateCache.ts +++ b/src/storages/inLocalStorage/validateCache.ts @@ -4,6 +4,7 @@ import { isNaNNumber } from '../../utils/lang'; import { getStorageHash } from '../KeyBuilder'; import { LOG_PREFIX } from './constants'; import type { SplitsCacheInLocal } from './SplitsCacheInLocal'; +import type { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal'; import { KeyBuilderCS } from '../KeyBuilderCS'; function validateExpiration(settings: ISettings, keys: KeyBuilderCS) { @@ -38,10 +39,12 @@ function validateExpiration(settings: ISettings, keys: KeyBuilderCS) { * - it has expired, i.e., its `lastUpdated` timestamp is older than the given `expirationTimestamp` * - hash has changed, i.e., the SDK key, flags filter criteria or flags spec version was modified */ -export function validateCache(settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal): boolean { +export function validateCache(settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): boolean { if (validateExpiration(settings, keys)) { splits.clear(); + segments.clear(); + largeSegments.clear(); } // Check if the cache is ready From 87fbc4f6fc77791e3868152674c80c14667975a4 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 16:20:16 -0300 Subject: [PATCH 11/24] expirationDays configuration --- src/storages/dataLoader.ts | 4 ++- src/storages/inLocalStorage/index.ts | 2 +- src/storages/inLocalStorage/validateCache.ts | 23 +++++++++++------ src/utils/constants/browser.ts | 2 -- src/utils/lang/index.ts | 2 +- types/splitio.d.ts | 26 +++++++++++++++++++- 6 files changed, 45 insertions(+), 14 deletions(-) delete mode 100644 src/utils/constants/browser.ts diff --git a/src/storages/dataLoader.ts b/src/storages/dataLoader.ts index ce288868..55535cfd 100644 --- a/src/storages/dataLoader.ts +++ b/src/storages/dataLoader.ts @@ -1,7 +1,9 @@ import { PreloadedData } from '../types'; -import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../utils/constants/browser'; import { DataLoader, ISegmentsCacheSync, ISplitsCacheSync } from './types'; +// This value might be eventually set via a config parameter +const DEFAULT_CACHE_EXPIRATION_IN_MILLIS = 864000000; // 10 days + /** * Factory of client-side storage loader * diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index f32cc014..bb01bd7d 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -50,7 +50,7 @@ export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): ISt uniqueKeys: impressionsMode === NONE ? new UniqueKeysCacheInMemoryCS() : undefined, validateCache() { - return validateCache(settings, keys, splits, segments, largeSegments); + return validateCache(options, settings, keys, splits, segments, largeSegments); }, destroy() { }, diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts index f76b77ca..d2da969a 100644 --- a/src/storages/inLocalStorage/validateCache.ts +++ b/src/storages/inLocalStorage/validateCache.ts @@ -1,21 +1,28 @@ import { ISettings } from '../../types'; -import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../../utils/constants/browser'; -import { isNaNNumber } from '../../utils/lang'; +import { isFiniteNumber, isNaNNumber } from '../../utils/lang'; import { getStorageHash } from '../KeyBuilder'; import { LOG_PREFIX } from './constants'; import type { SplitsCacheInLocal } from './SplitsCacheInLocal'; import type { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal'; import { KeyBuilderCS } from '../KeyBuilderCS'; +import SplitIO from '../../../types/splitio'; -function validateExpiration(settings: ISettings, keys: KeyBuilderCS) { +// milliseconds in a day +const DEFAULT_CACHE_EXPIRATION_IN_DAYS = 10; +const MILLIS_IN_A_DAY = 86400000; + +function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS) { const { log } = settings; // Check expiration - const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS; + const expirationTimestamp = Date.now() - MILLIS_IN_A_DAY * (isFiniteNumber(options.expirationDays) && options.expirationDays >= 1 ? options.expirationDays : DEFAULT_CACHE_EXPIRATION_IN_DAYS); let value: string | number | null = localStorage.getItem(keys.buildLastUpdatedKey()); if (value !== null) { value = parseInt(value, 10); - if (!isNaNNumber(value) && value < expirationTimestamp) return true; + if (!isNaNNumber(value) && value < expirationTimestamp) { + log.info(LOG_PREFIX + 'Cache expired. Cleaning up cache'); + return true; + } } // Check hash @@ -24,7 +31,7 @@ function validateExpiration(settings: ISettings, keys: KeyBuilderCS) { const currentStorageHash = getStorageHash(settings); if (storageHash !== currentStorageHash) { - log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version was modified. Updating cache'); + log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version was modified. Cleaning up cache'); try { localStorage.setItem(storageHashKey, currentStorageHash); } catch (e) { @@ -39,9 +46,9 @@ function validateExpiration(settings: ISettings, keys: KeyBuilderCS) { * - it has expired, i.e., its `lastUpdated` timestamp is older than the given `expirationTimestamp` * - hash has changed, i.e., the SDK key, flags filter criteria or flags spec version was modified */ -export function validateCache(settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): boolean { +export function validateCache(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): boolean { - if (validateExpiration(settings, keys)) { + if (validateExpiration(options, settings, keys)) { splits.clear(); segments.clear(); largeSegments.clear(); diff --git a/src/utils/constants/browser.ts b/src/utils/constants/browser.ts deleted file mode 100644 index d627f780..00000000 --- a/src/utils/constants/browser.ts +++ /dev/null @@ -1,2 +0,0 @@ -// This value might be eventually set via a config parameter -export const DEFAULT_CACHE_EXPIRATION_IN_MILLIS = 864000000; // 10 days diff --git a/src/utils/lang/index.ts b/src/utils/lang/index.ts index 11b6afd0..b1a7e35a 100644 --- a/src/utils/lang/index.ts +++ b/src/utils/lang/index.ts @@ -120,7 +120,7 @@ export function isBoolean(val: any): boolean { * Unlike `Number.isFinite`, it also tests Number object instances. * Unlike global `isFinite`, it returns false if the value is not a number or Number object instance. */ -export function isFiniteNumber(val: any): boolean { +export function isFiniteNumber(val: any): val is number { if (val instanceof Number) val = val.valueOf(); return typeof val === 'number' ? Number.isFinite ? Number.isFinite(val) : isFinite(val) : diff --git a/types/splitio.d.ts b/types/splitio.d.ts index bb108c1c..4aa59db3 100644 --- a/types/splitio.d.ts +++ b/types/splitio.d.ts @@ -906,6 +906,18 @@ declare namespace SplitIO { * @defaultValue `'SPLITIO'` */ prefix?: string; + /** + * Number of days before cached data expires if it was not updated. If cache expires, it is cleared on initialization. + * + * @defaultValue `10` + */ + expirationDays?: number; + /** + * Optional settings to clear the cache. If set to `true`, the SDK clears the cached data on initialization, unless the cache was cleared within the last 24 hours. + * + * @defaultValue `false` + */ + clearOnInit?: boolean; } /** * Storage for asynchronous (consumer) SDK. @@ -1229,11 +1241,23 @@ declare namespace SplitIO { */ type?: BrowserStorage; /** - * Optional prefix to prevent any kind of data collision between SDK versions. + * Optional prefix to prevent any kind of data collision between SDK versions when using 'LOCALSTORAGE'. * * @defaultValue `'SPLITIO'` */ prefix?: string; + /** + * Optional settings for the 'LOCALSTORAGE' storage type. It specifies the number of days before cached data expires if it was not updated. If cache expires, it is cleared on initialization. + * + * @defaultValue `10` + */ + expirationDays?: number; + /** + * Optional settings for the 'LOCALSTORAGE' storage type. If set to `true`, the SDK clears the cached data on initialization, unless the cache was cleared within the last 24 hours. + * + * @defaultValue `false` + */ + clearOnInit?: boolean; }; } /** From aca35fe3fea2171e532c43b3de79c535cae4731c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 16:37:47 -0300 Subject: [PATCH 12/24] clearOnInit configuration --- src/storages/KeyBuilderCS.ts | 4 ++++ src/storages/inLocalStorage/validateCache.ts | 21 +++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/storages/KeyBuilderCS.ts b/src/storages/KeyBuilderCS.ts index a59d7208..20372358 100644 --- a/src/storages/KeyBuilderCS.ts +++ b/src/storages/KeyBuilderCS.ts @@ -43,6 +43,10 @@ export class KeyBuilderCS extends KeyBuilder implements MySegmentsKeyBuilder { buildTillKey() { return `${this.prefix}.${this.matchingKey}.segments.till`; } + + buildLastClear() { + return `${this.prefix}.lastClear`; + } } export function myLargeSegmentsKeyBuilder(prefix: string, matchingKey: string): MySegmentsKeyBuilder { diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts index d2da969a..7c1fa31d 100644 --- a/src/storages/inLocalStorage/validateCache.ts +++ b/src/storages/inLocalStorage/validateCache.ts @@ -39,6 +39,18 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS } return true; } + + // Clear on init + if (options.clearOnInit) { + let value: string | number | null = localStorage.getItem(keys.buildLastClear()); + if (value !== null) { + value = parseInt(value, 10); + if (!isNaNNumber(value) && value < Date.now() - MILLIS_IN_A_DAY) { + log.info(LOG_PREFIX + 'Clear on init was set and cache was cleared more than a day ago. Cleaning up cache'); + return true; + } + } + } } /** @@ -52,8 +64,15 @@ export function validateCache(options: SplitIO.InLocalStorageOptions, settings: splits.clear(); segments.clear(); largeSegments.clear(); + + // Update last clear timestamp + try { + localStorage.setItem(keys.buildLastClear(), Date.now() + ''); + } catch (e) { + settings.log.error(LOG_PREFIX + e); + } } - // Check if the cache is ready + // Check if ready from cache return splits.getChangeNumber() > -1; } From 534d6ca8808662c6ad735f323694db49bfc92a1a Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 19 Dec 2024 11:47:33 -0300 Subject: [PATCH 13/24] Reuse Date.now() result --- src/storages/inLocalStorage/validateCache.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts index 7c1fa31d..d7690828 100644 --- a/src/storages/inLocalStorage/validateCache.ts +++ b/src/storages/inLocalStorage/validateCache.ts @@ -11,16 +11,17 @@ import SplitIO from '../../../types/splitio'; const DEFAULT_CACHE_EXPIRATION_IN_DAYS = 10; const MILLIS_IN_A_DAY = 86400000; -function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS) { +function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, currentTimestamp: number) { const { log } = settings; // Check expiration - const expirationTimestamp = Date.now() - MILLIS_IN_A_DAY * (isFiniteNumber(options.expirationDays) && options.expirationDays >= 1 ? options.expirationDays : DEFAULT_CACHE_EXPIRATION_IN_DAYS); + const cacheExpirationInDays = isFiniteNumber(options.expirationDays) && options.expirationDays >= 1 ? options.expirationDays : DEFAULT_CACHE_EXPIRATION_IN_DAYS; + const expirationTimestamp = currentTimestamp - MILLIS_IN_A_DAY * cacheExpirationInDays; let value: string | number | null = localStorage.getItem(keys.buildLastUpdatedKey()); if (value !== null) { value = parseInt(value, 10); if (!isNaNNumber(value) && value < expirationTimestamp) { - log.info(LOG_PREFIX + 'Cache expired. Cleaning up cache'); + log.info(LOG_PREFIX + 'Cache expired more than ' + cacheExpirationInDays + ' days ago. Cleaning up cache'); return true; } } @@ -45,7 +46,7 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS let value: string | number | null = localStorage.getItem(keys.buildLastClear()); if (value !== null) { value = parseInt(value, 10); - if (!isNaNNumber(value) && value < Date.now() - MILLIS_IN_A_DAY) { + if (!isNaNNumber(value) && value < currentTimestamp - MILLIS_IN_A_DAY) { log.info(LOG_PREFIX + 'Clear on init was set and cache was cleared more than a day ago. Cleaning up cache'); return true; } @@ -60,14 +61,16 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS */ export function validateCache(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): boolean { - if (validateExpiration(options, settings, keys)) { + const currentTimestamp = Date.now(); + + if (validateExpiration(options, settings, keys, currentTimestamp)) { splits.clear(); segments.clear(); largeSegments.clear(); // Update last clear timestamp try { - localStorage.setItem(keys.buildLastClear(), Date.now() + ''); + localStorage.setItem(keys.buildLastClear(), currentTimestamp + ''); } catch (e) { settings.log.error(LOG_PREFIX + e); } From 6451cda87ee72fe71f506ca0224c46e8ccf5d372 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 19 Dec 2024 12:48:08 -0300 Subject: [PATCH 14/24] Handle clearOnInit case with older version of the SDK where lastClear item is not available --- src/storages/inLocalStorage/validateCache.ts | 27 ++++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts index d7690828..b8676cb7 100644 --- a/src/storages/inLocalStorage/validateCache.ts +++ b/src/storages/inLocalStorage/validateCache.ts @@ -15,12 +15,11 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS const { log } = settings; // Check expiration - const cacheExpirationInDays = isFiniteNumber(options.expirationDays) && options.expirationDays >= 1 ? options.expirationDays : DEFAULT_CACHE_EXPIRATION_IN_DAYS; - const expirationTimestamp = currentTimestamp - MILLIS_IN_A_DAY * cacheExpirationInDays; - let value: string | number | null = localStorage.getItem(keys.buildLastUpdatedKey()); - if (value !== null) { - value = parseInt(value, 10); - if (!isNaNNumber(value) && value < expirationTimestamp) { + const lastUpdatedTimestamp = parseInt(localStorage.getItem(keys.buildLastUpdatedKey()) as string, 10); + if (!isNaNNumber(lastUpdatedTimestamp)) { + const cacheExpirationInDays = isFiniteNumber(options.expirationDays) && options.expirationDays >= 1 ? options.expirationDays : DEFAULT_CACHE_EXPIRATION_IN_DAYS; + const expirationTimestamp = currentTimestamp - MILLIS_IN_A_DAY * cacheExpirationInDays; + if (lastUpdatedTimestamp < expirationTimestamp) { log.info(LOG_PREFIX + 'Cache expired more than ' + cacheExpirationInDays + ' days ago. Cleaning up cache'); return true; } @@ -32,7 +31,7 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS const currentStorageHash = getStorageHash(settings); if (storageHash !== currentStorageHash) { - log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version was modified. Cleaning up cache'); + log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version has changed. Cleaning up cache'); try { localStorage.setItem(storageHashKey, currentStorageHash); } catch (e) { @@ -43,13 +42,11 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS // Clear on init if (options.clearOnInit) { - let value: string | number | null = localStorage.getItem(keys.buildLastClear()); - if (value !== null) { - value = parseInt(value, 10); - if (!isNaNNumber(value) && value < currentTimestamp - MILLIS_IN_A_DAY) { - log.info(LOG_PREFIX + 'Clear on init was set and cache was cleared more than a day ago. Cleaning up cache'); - return true; - } + const lastClearTimestamp = parseInt(localStorage.getItem(keys.buildLastClear()) as string, 10); + + if (isNaNNumber(lastClearTimestamp) || lastClearTimestamp < currentTimestamp - MILLIS_IN_A_DAY) { + log.info(LOG_PREFIX + 'clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache'); + return true; } } } @@ -58,6 +55,8 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS * Clean cache if: * - it has expired, i.e., its `lastUpdated` timestamp is older than the given `expirationTimestamp` * - hash has changed, i.e., the SDK key, flags filter criteria or flags spec version was modified + * + * @returns `true` if cache is ready to be used, `false` otherwise (cache was cleared or there is no cache) */ export function validateCache(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): boolean { From 956c1df61ebec9f4df14dc040970026faae97884 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 19 Dec 2024 13:22:25 -0300 Subject: [PATCH 15/24] Handle no cache: cache should not be clearer --- src/storages/inLocalStorage/validateCache.ts | 24 +++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts index b8676cb7..7c0c8ae9 100644 --- a/src/storages/inLocalStorage/validateCache.ts +++ b/src/storages/inLocalStorage/validateCache.ts @@ -11,7 +11,12 @@ import SplitIO from '../../../types/splitio'; const DEFAULT_CACHE_EXPIRATION_IN_DAYS = 10; const MILLIS_IN_A_DAY = 86400000; -function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, currentTimestamp: number) { +/** + * Validates if cache should be cleared and sets the cache `hash` if needed. + * + * @returns `true` if cache should be cleared, `false` otherwise + */ +function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, currentTimestamp: number, isThereCache: boolean) { const { log } = settings; // Check expiration @@ -31,13 +36,16 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS const currentStorageHash = getStorageHash(settings); if (storageHash !== currentStorageHash) { - log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version has changed. Cleaning up cache'); try { localStorage.setItem(storageHashKey, currentStorageHash); } catch (e) { log.error(LOG_PREFIX + e); } - return true; + if (isThereCache) { + log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version has changed. Cleaning up cache'); + return true; + } + return false; // No cache to clear } // Clear on init @@ -54,15 +62,17 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS /** * Clean cache if: * - it has expired, i.e., its `lastUpdated` timestamp is older than the given `expirationTimestamp` - * - hash has changed, i.e., the SDK key, flags filter criteria or flags spec version was modified + * - its hash has changed, i.e., the SDK key, flags filter criteria or flags spec version was modified + * - `clearOnInit` was set and cache was not cleared in the last 24 hours * * @returns `true` if cache is ready to be used, `false` otherwise (cache was cleared or there is no cache) */ export function validateCache(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): boolean { const currentTimestamp = Date.now(); + const isThereCache = splits.getChangeNumber() > -1; - if (validateExpiration(options, settings, keys, currentTimestamp)) { + if (validateExpiration(options, settings, keys, currentTimestamp, isThereCache)) { splits.clear(); segments.clear(); largeSegments.clear(); @@ -73,8 +83,10 @@ export function validateCache(options: SplitIO.InLocalStorageOptions, settings: } catch (e) { settings.log.error(LOG_PREFIX + e); } + + return false; } // Check if ready from cache - return splits.getChangeNumber() > -1; + return isThereCache; } From 28b7fb994217f92c55c3819b810d50e159c1b60b Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 19 Dec 2024 17:24:30 -0300 Subject: [PATCH 16/24] Add unit test --- .../inLocalStorage/SplitsCacheInLocal.ts | 2 - .../__tests__/validateCache.spec.ts | 125 ++++++++++++++++++ 2 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 src/storages/inLocalStorage/__tests__/validateCache.spec.ts diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index 14767d72..7d4c002f 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -71,8 +71,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { * We cannot simply call `localStorage.clear()` since that implies removing user items from the storage. */ clear() { - this.log.info(LOG_PREFIX + 'Flushing Splits data from localStorage'); - // collect item keys const len = localStorage.length; const accum = []; diff --git a/src/storages/inLocalStorage/__tests__/validateCache.spec.ts b/src/storages/inLocalStorage/__tests__/validateCache.spec.ts new file mode 100644 index 00000000..a1f1784d --- /dev/null +++ b/src/storages/inLocalStorage/__tests__/validateCache.spec.ts @@ -0,0 +1,125 @@ +import { validateCache } from '../validateCache'; + +import { KeyBuilderCS } from '../../KeyBuilderCS'; +import { fullSettings } from '../../../utils/settingsValidation/__tests__/settings.mocks'; +import { SplitsCacheInLocal } from '../SplitsCacheInLocal'; +import { nearlyEqual } from '../../../__tests__/testUtils'; +import { MySegmentsCacheInLocal } from '../MySegmentsCacheInLocal'; + +const FULL_SETTINGS_HASH = '404832b3'; + +describe('validateCache', () => { + const keys = new KeyBuilderCS('SPLITIO', 'user'); + const logSpy = jest.spyOn(fullSettings.log, 'info'); + const segments = new MySegmentsCacheInLocal(fullSettings.log, keys); + const largeSegments = new MySegmentsCacheInLocal(fullSettings.log, keys); + const splits = new SplitsCacheInLocal(fullSettings, keys); + + jest.spyOn(splits, 'clear'); + jest.spyOn(splits, 'getChangeNumber'); + jest.spyOn(segments, 'clear'); + jest.spyOn(largeSegments, 'clear'); + + beforeEach(() => { + jest.clearAllMocks(); + localStorage.clear(); + }); + + test('if there is no cache, it should return false', () => { + expect(validateCache({}, fullSettings, keys, splits, segments, largeSegments)).toBe(false); + + expect(logSpy).not.toHaveBeenCalled(); + + expect(splits.clear).not.toHaveBeenCalled(); + expect(segments.clear).not.toHaveBeenCalled(); + expect(largeSegments.clear).not.toHaveBeenCalled(); + expect(splits.getChangeNumber).toHaveBeenCalledTimes(1); + + expect(localStorage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH); + expect(localStorage.getItem(keys.buildLastClear())).toBeNull(); + }); + + test('if there is cache and it must not be cleared, it should return true', () => { + localStorage.setItem(keys.buildSplitsTillKey(), '1'); + localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); + + expect(validateCache({}, fullSettings, keys, splits, segments, largeSegments)).toBe(true); + + expect(logSpy).not.toHaveBeenCalled(); + + expect(splits.clear).not.toHaveBeenCalled(); + expect(segments.clear).not.toHaveBeenCalled(); + expect(largeSegments.clear).not.toHaveBeenCalled(); + expect(splits.getChangeNumber).toHaveBeenCalledTimes(1); + + expect(localStorage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH); + expect(localStorage.getItem(keys.buildLastClear())).toBeNull(); + }); + + test('if there is cache and it has expired, it should clear cache and return false', () => { + localStorage.setItem(keys.buildSplitsTillKey(), '1'); + localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); + localStorage.setItem(keys.buildLastUpdatedKey(), Date.now() - 1000 * 60 * 60 * 24 * 2 + ''); // 2 days ago + + expect(validateCache({ expirationDays: 1 }, fullSettings, keys, splits, segments, largeSegments)).toBe(false); + + expect(logSpy).toHaveBeenCalledWith('storage:localstorage: Cache expired more than 1 days ago. Cleaning up cache'); + + expect(splits.clear).toHaveBeenCalledTimes(1); + expect(segments.clear).toHaveBeenCalledTimes(1); + expect(largeSegments.clear).toHaveBeenCalledTimes(1); + + expect(localStorage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH); + expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true); + }); + + test('if there is cache and its hash has changed, it should clear cache and return false', () => { + localStorage.setItem(keys.buildSplitsTillKey(), '1'); + localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); + + expect(validateCache({}, { ...fullSettings, core: { ...fullSettings.core, authorizationKey: 'another' } }, keys, splits, segments, largeSegments)).toBe(false); + + expect(logSpy).toHaveBeenCalledWith('storage:localstorage: SDK key, flags filter criteria or flags spec version has changed. Cleaning up cache'); + + expect(splits.clear).toHaveBeenCalledTimes(1); + expect(segments.clear).toHaveBeenCalledTimes(1); + expect(largeSegments.clear).toHaveBeenCalledTimes(1); + + expect(localStorage.getItem(keys.buildHashKey())).toBe('aa4877c2'); + expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true); + }); + + test('if there is cache and clearOnInit is true, it should clear cache and return false', () => { + // Older cache version (without last clear) + localStorage.setItem(keys.buildSplitsTillKey(), '1'); + localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); + + expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, segments, largeSegments)).toBe(false); + + expect(logSpy).toHaveBeenCalledWith('storage:localstorage: clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache'); + + expect(splits.clear).toHaveBeenCalledTimes(1); + expect(segments.clear).toHaveBeenCalledTimes(1); + expect(largeSegments.clear).toHaveBeenCalledTimes(1); + + expect(localStorage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH); + const lastClear = localStorage.getItem(keys.buildLastClear()); + expect(nearlyEqual(parseInt(lastClear as string), Date.now())).toBe(true); + + // If cache is cleared, it should not clear again until a day has passed + logSpy.mockClear(); + localStorage.setItem(keys.buildSplitsTillKey(), '1'); + expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, segments, largeSegments)).toBe(true); + expect(logSpy).not.toHaveBeenCalled(); + expect(localStorage.getItem(keys.buildLastClear())).toBe(lastClear); // Last clear should not have changed + + // If a day has passed, it should clear again + localStorage.setItem(keys.buildLastClear(), (Date.now() - 1000 * 60 * 60 * 24 - 1) + ''); + expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, segments, largeSegments)).toBe(false); + expect(logSpy).toHaveBeenCalledWith('storage:localstorage: clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache'); + expect(splits.clear).toHaveBeenCalledTimes(2); + expect(segments.clear).toHaveBeenCalledTimes(2); + expect(largeSegments.clear).toHaveBeenCalledTimes(2); + expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true); + }); +}); From 6dc1e613c74cadf1aef770dac902a22c2a846e3a Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 19 Dec 2024 18:16:33 -0300 Subject: [PATCH 17/24] rc --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 971d1486..160c9c26 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-commons", - "version": "2.0.2", + "version": "2.1.0-rc.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-commons", - "version": "2.0.2", + "version": "2.1.0-rc.0", "license": "Apache-2.0", "dependencies": { "@types/ioredis": "^4.28.0", diff --git a/package.json b/package.json index 936c23bd..37317c20 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "2.0.2", + "version": "2.1.0-rc.0", "description": "Split JavaScript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", From 5ea10ad81efaa55570df61c8f1ac6b89b18b16a3 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 19 Dec 2024 18:31:35 -0300 Subject: [PATCH 18/24] Add changelog entry --- CHANGES.txt | 5 +++++ src/storages/inLocalStorage/validateCache.ts | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 02a7bd61..6988dc05 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,8 @@ +2.1.0 (January XX, 2025) + - Added two new configuration options for the SDK storage in browsers when using storage type `LOCALSTORAGE`: + - `storage.expirationDays` to specify the validity period of the rollout cache. + - `storage.clearOnInit` to clear the rollout cache on SDK initialization. + 2.0.2 (December 3, 2024) - Updated the factory `init` and `destroy` methods to support re-initialization after destruction. This update ensures compatibility of the React SDK with React Strict Mode, where the factory's `init` and `destroy` effects are executed an extra time to validate proper resource cleanup. - Bugfixing - Sanitize the `SplitSDKMachineName` header value to avoid exceptions on HTTP/S requests when it contains non ISO-8859-1 characters (Related to issue https://github.com/splitio/javascript-client/issues/847). diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts index 7c0c8ae9..c11e8d90 100644 --- a/src/storages/inLocalStorage/validateCache.ts +++ b/src/storages/inLocalStorage/validateCache.ts @@ -7,7 +7,6 @@ import type { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal'; import { KeyBuilderCS } from '../KeyBuilderCS'; import SplitIO from '../../../types/splitio'; -// milliseconds in a day const DEFAULT_CACHE_EXPIRATION_IN_DAYS = 10; const MILLIS_IN_A_DAY = 86400000; From 71007342f9b44180518021f2efc7a379a94f1226 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 20 Dec 2024 13:53:44 -0300 Subject: [PATCH 19/24] Fix typo --- src/storages/inLocalStorage/__tests__/validateCache.spec.ts | 2 +- src/storages/inLocalStorage/validateCache.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storages/inLocalStorage/__tests__/validateCache.spec.ts b/src/storages/inLocalStorage/__tests__/validateCache.spec.ts index a1f1784d..27050a56 100644 --- a/src/storages/inLocalStorage/__tests__/validateCache.spec.ts +++ b/src/storages/inLocalStorage/__tests__/validateCache.spec.ts @@ -79,7 +79,7 @@ describe('validateCache', () => { expect(validateCache({}, { ...fullSettings, core: { ...fullSettings.core, authorizationKey: 'another' } }, keys, splits, segments, largeSegments)).toBe(false); - expect(logSpy).toHaveBeenCalledWith('storage:localstorage: SDK key, flags filter criteria or flags spec version has changed. Cleaning up cache'); + expect(logSpy).toHaveBeenCalledWith('storage:localstorage: SDK key, flags filter criteria, or flags spec version has changed. Cleaning up cache'); expect(splits.clear).toHaveBeenCalledTimes(1); expect(segments.clear).toHaveBeenCalledTimes(1); diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts index c11e8d90..c9bd78d2 100644 --- a/src/storages/inLocalStorage/validateCache.ts +++ b/src/storages/inLocalStorage/validateCache.ts @@ -41,7 +41,7 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: IS log.error(LOG_PREFIX + e); } if (isThereCache) { - log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version has changed. Cleaning up cache'); + log.info(LOG_PREFIX + 'SDK key, flags filter criteria, or flags spec version has changed. Cleaning up cache'); return true; } return false; // No cache to clear From 4c7e7812d2798d8601a81eedfdd4a634d44421fe Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 2 Jan 2025 13:50:57 -0300 Subject: [PATCH 20/24] Updated SDK_READY_FROM_CACHE event when using LOCALSTORAGE storage type to be emitted alongside SDK_READY event in case it has not been emitted --- CHANGES.txt | 1 + src/readiness/__tests__/readinessManager.spec.ts | 14 +++++++++++++- src/readiness/readinessManager.ts | 5 +++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index db363080..0b3a5fec 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -2,6 +2,7 @@ - Added two new configuration options for the SDK storage in browsers when using storage type `LOCALSTORAGE`: - `storage.expirationDays` to specify the validity period of the rollout cache. - `storage.clearOnInit` to clear the rollout cache on SDK initialization. + - Updated SDK_READY_FROM_CACHE event when using `LOCALSTORAGE` storage type to be emitted alongside SDK_READY event in case it has not been emitted. - Bugfixing - Properly handle rejected promises when using targeting rules with segment matchers in consumer modes (e.g., Redis and Pluggable storages). 2.0.2 (December 3, 2024) diff --git a/src/readiness/__tests__/readinessManager.spec.ts b/src/readiness/__tests__/readinessManager.spec.ts index 9e2cf34a..174f1373 100644 --- a/src/readiness/__tests__/readinessManager.spec.ts +++ b/src/readiness/__tests__/readinessManager.spec.ts @@ -3,10 +3,14 @@ import { EventEmitter } from '../../utils/MinEvents'; import { IReadinessManager } from '../types'; import { SDK_READY, SDK_UPDATE, SDK_SPLITS_ARRIVED, SDK_SEGMENTS_ARRIVED, SDK_READY_FROM_CACHE, SDK_SPLITS_CACHE_LOADED, SDK_READY_TIMED_OUT } from '../constants'; import { ISettings } from '../../types'; +import { STORAGE_LOCALSTORAGE } from '../../utils/constants'; const settings = { startup: { readyTimeout: 0, + }, + storage: { + type: STORAGE_LOCALSTORAGE } } as unknown as ISettings; @@ -67,7 +71,14 @@ test('READINESS MANAGER / Ready event should be fired once', () => { const readinessManager = readinessManagerFactory(EventEmitter, settings); let counter = 0; + readinessManager.gate.on(SDK_READY_FROM_CACHE, () => { + expect(readinessManager.isReadyFromCache()).toBe(true); + expect(readinessManager.isReady()).toBe(true); + counter++; + }); + readinessManager.gate.on(SDK_READY, () => { + expect(readinessManager.isReadyFromCache()).toBe(true); expect(readinessManager.isReady()).toBe(true); counter++; }); @@ -79,7 +90,7 @@ test('READINESS MANAGER / Ready event should be fired once', () => { readinessManager.splits.emit(SDK_SPLITS_ARRIVED); readinessManager.segments.emit(SDK_SEGMENTS_ARRIVED); - expect(counter).toBe(1); // should be called once + expect(counter).toBe(2); // should be called once }); test('READINESS MANAGER / Ready from cache event should be fired once', (done) => { @@ -88,6 +99,7 @@ test('READINESS MANAGER / Ready from cache event should be fired once', (done) = readinessManager.gate.on(SDK_READY_FROM_CACHE, () => { expect(readinessManager.isReadyFromCache()).toBe(true); + expect(readinessManager.isReady()).toBe(false); counter++; }); diff --git a/src/readiness/readinessManager.ts b/src/readiness/readinessManager.ts index 6f46474d..c69eedce 100644 --- a/src/readiness/readinessManager.ts +++ b/src/readiness/readinessManager.ts @@ -3,6 +3,7 @@ import { ISettings } from '../types'; import SplitIO from '../../types/splitio'; import { SDK_SPLITS_ARRIVED, SDK_SPLITS_CACHE_LOADED, SDK_SEGMENTS_ARRIVED, SDK_READY_TIMED_OUT, SDK_READY_FROM_CACHE, SDK_UPDATE, SDK_READY } from './constants'; import { IReadinessEventEmitter, IReadinessManager, ISegmentsEventEmitter, ISplitsEventEmitter } from './types'; +import { STORAGE_LOCALSTORAGE } from '../utils/constants'; function splitsEventEmitterFactory(EventEmitter: new () => SplitIO.IEventEmitter): ISplitsEventEmitter { const splitsEventEmitter = objectAssign(new EventEmitter(), { @@ -114,6 +115,10 @@ export function readinessManagerFactory( isReady = true; try { syncLastUpdate(); + if (!isReadyFromCache && settings.storage?.type === STORAGE_LOCALSTORAGE) { + isReadyFromCache = true; + gate.emit(SDK_READY_FROM_CACHE); + } gate.emit(SDK_READY); } catch (e) { // throws user callback exceptions in next tick From d5a5eaa053dd8169aeca66f1ddcc9b61675889a2 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 2 Jan 2025 13:52:08 -0300 Subject: [PATCH 21/24] rc --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 160c9c26..7b2773fe 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-commons", - "version": "2.1.0-rc.0", + "version": "2.1.0-rc.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-commons", - "version": "2.1.0-rc.0", + "version": "2.1.0-rc.1", "license": "Apache-2.0", "dependencies": { "@types/ioredis": "^4.28.0", diff --git a/package.json b/package.json index 37317c20..1e4eb233 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "2.1.0-rc.0", + "version": "2.1.0-rc.1", "description": "Split JavaScript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", From d854156fcf1a9b862bb618710a69484abdd948bb Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 2 Jan 2025 14:40:02 -0300 Subject: [PATCH 22/24] Update changelog entry --- CHANGES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 0b3a5fec..b20ac98a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -2,7 +2,7 @@ - Added two new configuration options for the SDK storage in browsers when using storage type `LOCALSTORAGE`: - `storage.expirationDays` to specify the validity period of the rollout cache. - `storage.clearOnInit` to clear the rollout cache on SDK initialization. - - Updated SDK_READY_FROM_CACHE event when using `LOCALSTORAGE` storage type to be emitted alongside SDK_READY event in case it has not been emitted. + - Updated SDK_READY_FROM_CACHE event when using the `LOCALSTORAGE` storage type to be emitted alongside the SDK_READY event if it has not already been emitted. - Bugfixing - Properly handle rejected promises when using targeting rules with segment matchers in consumer modes (e.g., Redis and Pluggable storages). 2.0.2 (December 3, 2024) From 20d97aa7f155273e9436a0ac2e4287530a80d067 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 13 Jan 2025 14:07:12 -0300 Subject: [PATCH 23/24] rc --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7b2773fe..e774db65 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-commons", - "version": "2.1.0-rc.1", + "version": "2.1.0-rc.2", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-commons", - "version": "2.1.0-rc.1", + "version": "2.1.0-rc.2", "license": "Apache-2.0", "dependencies": { "@types/ioredis": "^4.28.0", diff --git a/package.json b/package.json index 1e4eb233..a955ce56 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "2.1.0-rc.1", + "version": "2.1.0-rc.2", "description": "Split JavaScript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", From 1cc5ad6f7d8b35ce7ab07bc1e85727294ee66348 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 4 Feb 2025 23:36:59 -0300 Subject: [PATCH 24/24] Remove unnecessary comment --- src/sync/polling/updaters/splitChangesUpdater.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sync/polling/updaters/splitChangesUpdater.ts b/src/sync/polling/updaters/splitChangesUpdater.ts index e447227d..43f471a2 100644 --- a/src/sync/polling/updaters/splitChangesUpdater.ts +++ b/src/sync/polling/updaters/splitChangesUpdater.ts @@ -169,7 +169,6 @@ export function splitChangesUpdaterFactory( // Write into storage // @TODO call `setChangeNumber` only if the other storage operations have succeeded, in order to keep storage consistency return Promise.all([ - // calling first `setChangenumber` method, to perform cache flush if split filter queryString changed splits.setChangeNumber(splitChanges.till), splits.addSplits(mutation.added), splits.removeSplits(mutation.removed),