From d392cc88cc4ee1c22aefa20eb77ec85629d744f7 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 00:43:02 -0300 Subject: [PATCH] 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;