From 893833455985edf25367457e6b9281bc203d5ecb Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 13:54:21 -0300 Subject: [PATCH 1/6] 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 2/6] 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 3/6] 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 4/6] 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 5/6] 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 6605bfc56e5a4ba11378847fff667ba3243cc278 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 18 Dec 2024 15:39:58 -0300 Subject: [PATCH 6/6] 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