From 4411c5d81b72096421089ffb08bccc265ab83d3d Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 27 May 2025 15:42:02 -0300 Subject: [PATCH 01/14] Implement basic storageAdapter to support async storages with getItem and setItem methods --- .../inLocalStorage/MySegmentsCacheInLocal.ts | 6 +- .../inLocalStorage/RBSegmentsCacheInLocal.ts | 6 +- .../inLocalStorage/SplitsCacheInLocal.ts | 6 +- src/storages/inLocalStorage/index.ts | 67 +++++++++++++++++-- src/storages/inLocalStorage/validateCache.ts | 7 +- types/splitio.d.ts | 49 +++----------- 6 files changed, 83 insertions(+), 58 deletions(-) diff --git a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts index 4d565531..0b296531 100644 --- a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts @@ -1,17 +1,17 @@ +import { StorageAdapter } from '.'; import { ILogger } from '../../logger/types'; import { isNaNNumber } from '../../utils/lang'; import { AbstractMySegmentsCacheSync } from '../AbstractMySegmentsCacheSync'; import type { MySegmentsKeyBuilder } from '../KeyBuilderCS'; import { LOG_PREFIX, DEFINED } from './constants'; -import SplitIO from '../../../types/splitio'; export class MySegmentsCacheInLocal extends AbstractMySegmentsCacheSync { private readonly keys: MySegmentsKeyBuilder; private readonly log: ILogger; - private readonly localStorage: SplitIO.Storage; + private readonly localStorage: StorageAdapter; - constructor(log: ILogger, keys: MySegmentsKeyBuilder, localStorage: SplitIO.Storage) { + constructor(log: ILogger, keys: MySegmentsKeyBuilder, localStorage: StorageAdapter) { super(); this.log = log; this.keys = keys; diff --git a/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts b/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts index a9744912..d68e17b2 100644 --- a/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts @@ -7,15 +7,15 @@ import { usesSegments } from '../AbstractSplitsCacheSync'; import { KeyBuilderCS } from '../KeyBuilderCS'; import { IRBSegmentsCacheSync } from '../types'; import { LOG_PREFIX } from './constants'; -import SplitIO from '../../../types/splitio'; +import { StorageAdapter } from '.'; export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync { private readonly keys: KeyBuilderCS; private readonly log: ILogger; - private readonly localStorage: SplitIO.Storage; + private readonly localStorage: StorageAdapter; - constructor(settings: ISettings, keys: KeyBuilderCS, localStorage: SplitIO.Storage) { + constructor(settings: ISettings, keys: KeyBuilderCS, localStorage: StorageAdapter) { this.keys = keys; this.log = settings.log; this.localStorage = localStorage; diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index da411048..2d0dec55 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -6,7 +6,7 @@ import { ILogger } from '../../logger/types'; import { LOG_PREFIX } from './constants'; import { ISettings } from '../../types'; import { setToArray } from '../../utils/lang/sets'; -import SplitIO from '../../../types/splitio'; +import { StorageAdapter } from '.'; /** * ISplitsCacheSync implementation that stores split definitions in browser LocalStorage. @@ -17,9 +17,9 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { private readonly log: ILogger; private readonly flagSetsFilter: string[]; private hasSync?: boolean; - private readonly localStorage: SplitIO.Storage; + private readonly localStorage: StorageAdapter; - constructor(settings: ISettings, keys: KeyBuilderCS, localStorage: SplitIO.Storage) { + constructor(settings: ISettings, keys: KeyBuilderCS, localStorage: StorageAdapter) { super(); this.keys = keys; this.log = settings.log; diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 5872adaa..7c04440f 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -18,9 +18,67 @@ import { validateCache } from './validateCache'; import { ILogger } from '../../logger/types'; import SplitIO from '../../../types/splitio'; -function validateStorage(log: ILogger, storage?: SplitIO.Storage) { +export interface StorageAdapter { + // Methods to support async storages + load?: () => Promise; + save?: () => Promise; + // Methods based on https://developer.mozilla.org/en-US/docs/Web/API/Storage + readonly length: number; + getItem(key: string): string | null; + key(index: number): string | null; + removeItem(key: string): void; + setItem(key: string, value: string): void; +} + +function isTillKey(key: string) { + return key.endsWith('.till'); +} + +function storageAdapter(log: ILogger, prefix: string, storage: SplitIO.Storage): StorageAdapter { + let cache: Record = {}; + + let connectPromise: Promise | undefined; + let disconnectPromise = Promise.resolve(); + + return { + load() { + return connectPromise || (connectPromise = storage.getItem(prefix).then((storedCache) => { + cache = JSON.parse(storedCache || '{}'); + }).catch((e) => { + log.error(LOG_PREFIX + 'Rejected promise calling storage getItem, with error: ' + e); + })); + }, + save() { + return disconnectPromise = disconnectPromise.then(() => { + return storage.setItem(prefix, JSON.stringify(cache)).catch((e) => { + log.error(LOG_PREFIX + 'Rejected promise calling storage setItem, with error: ' + e); + }); + }); + }, + + get length() { + return Object.keys(cache).length; + }, + getItem(key: string) { + return cache[key] || null; + }, + key(index: number) { + return Object.keys(cache)[index] || null; + }, + removeItem(key: string) { + delete cache[key]; + if (isTillKey(key)) this.save!(); + }, + setItem(key: string, value: string) { + cache[key] = value; + if (isTillKey(key)) this.save!(); + } + }; +} + +function validateStorage(log: ILogger, prefix: string, storage?: SplitIO.Storage): StorageAdapter | undefined { if (storage) { - if (isStorageValid(storage)) return storage; + if (isStorageValid(storage)) return storageAdapter(log, prefix, storage); log.warn(LOG_PREFIX + 'Invalid storage provided. Falling back to LocalStorage API'); } @@ -39,7 +97,7 @@ export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): ISt function InLocalStorageCSFactory(params: IStorageFactoryParams): IStorageSync { const { settings, settings: { log, scheduler: { impressionsQueueSize, eventsQueueSize } } } = params; - const storage = validateStorage(log, options.storage); + const storage = validateStorage(log, prefix, options.storage); if (!storage) return InMemoryStorageCSFactory(params); const matchingKey = getMatching(settings.core.key); @@ -67,8 +125,7 @@ export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): ISt }, destroy() { - // @TODO return `storageWrapper.disconnect()` - return Promise.resolve(); + return storage.save && storage.save(); }, // When using shared instantiation with MEMORY we reuse everything but segments (they are customer per key). diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts index 8aec3814..d25f9f3e 100644 --- a/src/storages/inLocalStorage/validateCache.ts +++ b/src/storages/inLocalStorage/validateCache.ts @@ -7,6 +7,7 @@ import type { RBSegmentsCacheInLocal } from './RBSegmentsCacheInLocal'; import type { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal'; import { KeyBuilderCS } from '../KeyBuilderCS'; import SplitIO from '../../../types/splitio'; +import { StorageAdapter } from '.'; const DEFAULT_CACHE_EXPIRATION_IN_DAYS = 10; const MILLIS_IN_A_DAY = 86400000; @@ -16,7 +17,7 @@ const MILLIS_IN_A_DAY = 86400000; * * @returns `true` if cache should be cleared, `false` otherwise */ -function validateExpiration(options: SplitIO.InLocalStorageOptions, storage: SplitIO.Storage, settings: ISettings, keys: KeyBuilderCS, currentTimestamp: number, isThereCache: boolean) { +function validateExpiration(options: SplitIO.InLocalStorageOptions, storage: StorageAdapter, settings: ISettings, keys: KeyBuilderCS, currentTimestamp: number, isThereCache: boolean) { const { log } = settings; // Check expiration @@ -67,9 +68,9 @@ function validateExpiration(options: SplitIO.InLocalStorageOptions, storage: Spl * * @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, storage: SplitIO.Storage, settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, rbSegments: RBSegmentsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): Promise { +export function validateCache(options: SplitIO.InLocalStorageOptions, storage: StorageAdapter, settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, rbSegments: RBSegmentsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): Promise { - return Promise.resolve().then(() => { + return Promise.resolve(storage.load && storage.load()).then(() => { const currentTimestamp = Date.now(); const isThereCache = splits.getChangeNumber() > -1; diff --git a/types/splitio.d.ts b/types/splitio.d.ts index 3ae98912..c9e8fbf9 100644 --- a/types/splitio.d.ts +++ b/types/splitio.d.ts @@ -451,50 +451,17 @@ declare namespace SplitIO { interface Storage { /** - * Returns the number of key/value pairs. - * - * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage/length) - */ - readonly length: number; - /** - * Removes all key/value pairs, if there are any. - * - * Dispatches a storage event on Window objects holding an equivalent Storage object. - * - * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage/clear) - */ - clear(): void; - /** - * Returns the current value associated with the given key, or null if the given key does not exist. - * - * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage/getItem) - */ - getItem(key: string): string | null; - /** - * Returns the name of the nth key, or null if n is greater than or equal to the number of key/value pairs. - * - * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage/key) + * Returns a promise that resolves to the current value associated with the given key, or null if the given key does not exist. */ - key(index: number): string | null; + getItem(key: string): Promise; /** - * Removes the key/value pair with the given key, if a key/value pair with the given key exists. - * - * Dispatches a storage event on Window objects holding an equivalent Storage object. - * - * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage/removeItem) - */ - removeItem(key: string): void; - /** - * Sets the value of the pair identified by key to value, creating a new key/value pair if none existed for key previously. - * - * Throws a "QuotaExceededError" DOMException exception if the new value couldn't be set. (Setting could fail if, e.g., the user has disabled storage for the site, or if the quota has been exceeded.) - * - * Dispatches a storage event on Window objects holding an equivalent Storage object. - * - * [MDN Reference](https://developer.mozilla.org/docs/Web/API/Storage/setItem) + * Returns a promise that resolves when the value of the pair identified by key is set to value, creating a new key/value pair if none existed for key previously. */ - setItem(key: string, value: string): void; - [name: string]: any; + setItem(key: string, value: string): Promise; + // /** + // * Returns a promise that resolves when the key/value pair with the given key is removed, if a key/value pair with the given key exists. + // */ + // removeItem(key: string): Promise; } /** From b550042c5490dede290b2e2f0274ac21c5640065 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 30 May 2025 12:49:42 -0300 Subject: [PATCH 02/14] Tests --- .../__tests__/MySegmentsCacheInLocal.spec.ts | 15 +- .../__tests__/SplitsCacheInLocal.spec.ts | 319 +++++++++--------- .../inLocalStorage/__tests__/index.spec.ts | 24 +- .../__tests__/validateCache.spec.ts | 75 ++-- .../inLocalStorage/__tests__/wrapper.mock.ts | 27 ++ src/storages/inLocalStorage/index.ts | 2 +- types/splitio.d.ts | 10 +- 7 files changed, 257 insertions(+), 215 deletions(-) create mode 100644 src/storages/inLocalStorage/__tests__/wrapper.mock.ts diff --git a/src/storages/inLocalStorage/__tests__/MySegmentsCacheInLocal.spec.ts b/src/storages/inLocalStorage/__tests__/MySegmentsCacheInLocal.spec.ts index a3246dab..dfb98522 100644 --- a/src/storages/inLocalStorage/__tests__/MySegmentsCacheInLocal.spec.ts +++ b/src/storages/inLocalStorage/__tests__/MySegmentsCacheInLocal.spec.ts @@ -1,11 +1,12 @@ import { MySegmentsCacheInLocal } from '../MySegmentsCacheInLocal'; import { KeyBuilderCS, myLargeSegmentsKeyBuilder } from '../../KeyBuilderCS'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; +import { storages, PREFIX } from './wrapper.mock'; -test('SEGMENT CACHE / in LocalStorage', () => { +test.each(storages)('SEGMENT CACHE / in LocalStorage', (storage) => { const caches = [ - new MySegmentsCacheInLocal(loggerMock, new KeyBuilderCS('SPLITIO', 'user'), localStorage), - new MySegmentsCacheInLocal(loggerMock, myLargeSegmentsKeyBuilder('SPLITIO', 'user'), localStorage) + new MySegmentsCacheInLocal(loggerMock, new KeyBuilderCS(PREFIX, 'user'), storage), + new MySegmentsCacheInLocal(loggerMock, myLargeSegmentsKeyBuilder(PREFIX, 'user'), storage) ]; caches.forEach(cache => { @@ -33,8 +34,8 @@ test('SEGMENT CACHE / in LocalStorage', () => { expect(cache.getKeysCount()).toBe(1); }); - expect(localStorage.getItem('SPLITIO.user.segment.mocked-segment-2')).toBe('1'); - expect(localStorage.getItem('SPLITIO.user.segment.mocked-segment')).toBe(null); - expect(localStorage.getItem('SPLITIO.user.largeSegment.mocked-segment-2')).toBe('1'); - expect(localStorage.getItem('SPLITIO.user.largeSegment.mocked-segment')).toBe(null); + expect(storage.getItem(PREFIX + '.user.segment.mocked-segment-2')).toBe('1'); + expect(storage.getItem(PREFIX + '.user.segment.mocked-segment')).toBe(null); + expect(storage.getItem(PREFIX + '.user.largeSegment.mocked-segment-2')).toBe('1'); + expect(storage.getItem(PREFIX + '.user.largeSegment.mocked-segment')).toBe(null); }); diff --git a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts index c8f938bd..8f561542 100644 --- a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts +++ b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts @@ -3,225 +3,228 @@ import { KeyBuilderCS } from '../../KeyBuilderCS'; import { splitWithUserTT, splitWithAccountTT, splitWithAccountTTAndUsesSegments, something, somethingElse, featureFlagOne, featureFlagTwo, featureFlagThree, featureFlagWithEmptyFS, featureFlagWithoutFS } from '../../__tests__/testUtils'; import { ISplit } from '../../../dtos/types'; import { fullSettings } from '../../../utils/settingsValidation/__tests__/settings.mocks'; +import { storages, PREFIX } from './wrapper.mock'; -test('SPLITS CACHE / LocalStorage', () => { - const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'), localStorage); +describe.each(storages)('SPLITS CACHE', (storage) => { + test('LocalStorage', () => { + const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS(PREFIX, 'user'), storage); - cache.clear(); + cache.clear(); - cache.update([something, somethingElse], [], -1); + cache.update([something, somethingElse], [], -1); - let values = cache.getAll(); + let values = cache.getAll(); - expect(values).toEqual([something, somethingElse]); + expect(values).toEqual([something, somethingElse]); - cache.removeSplit(something.name); + cache.removeSplit(something.name); - const splits = cache.getSplits([something.name, somethingElse.name]); - expect(splits[something.name]).toEqual(null); - expect(splits[somethingElse.name]).toEqual(somethingElse); + const splits = cache.getSplits([something.name, somethingElse.name]); + expect(splits[something.name]).toEqual(null); + expect(splits[somethingElse.name]).toEqual(somethingElse); - values = cache.getAll(); + values = cache.getAll(); - expect(values).toEqual([somethingElse]); + expect(values).toEqual([somethingElse]); - expect(cache.getSplit(something.name)).toEqual(null); - expect(cache.getSplit(somethingElse.name)).toEqual(somethingElse); + expect(cache.getSplit(something.name)).toEqual(null); + expect(cache.getSplit(somethingElse.name)).toEqual(somethingElse); - expect(cache.getChangeNumber()).toBe(-1); + expect(cache.getChangeNumber()).toBe(-1); - cache.setChangeNumber(123); + cache.setChangeNumber(123); - expect(cache.getChangeNumber()).toBe(123); -}); + expect(cache.getChangeNumber()).toBe(123); + }); -test('SPLITS CACHE / LocalStorage / Get Keys', () => { - const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'), localStorage); + test('LocalStorage / Get Keys', () => { + const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS(PREFIX, 'user'), storage); - cache.update([something, somethingElse], [], 1); + cache.update([something, somethingElse], [], 1); - const keys = cache.getSplitNames(); + const keys = cache.getSplitNames(); - expect(keys.indexOf(something.name) !== -1).toBe(true); - expect(keys.indexOf(somethingElse.name) !== -1).toBe(true); -}); + expect(keys.indexOf(something.name) !== -1).toBe(true); + expect(keys.indexOf(somethingElse.name) !== -1).toBe(true); + }); -test('SPLITS CACHE / LocalStorage / Update Splits', () => { - const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'), localStorage); + test('LocalStorage / Update Splits', () => { + const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS(PREFIX, 'user'), storage); - cache.update([something, somethingElse], [], 1); + cache.update([something, somethingElse], [], 1); - cache.update([], [something, somethingElse], 1); + cache.update([], [something, somethingElse], 1); - expect(cache.getSplit(something.name)).toBe(null); - expect(cache.getSplit(somethingElse.name)).toBe(null); -}); + expect(cache.getSplit(something.name)).toBe(null); + expect(cache.getSplit(somethingElse.name)).toBe(null); + }); -test('SPLITS CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => { - const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'), localStorage); + test('LocalStorage / trafficTypeExists and ttcache tests', () => { + const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS(PREFIX, 'user'), storage); - cache.update([ - { ...splitWithUserTT, name: 'split1' }, - { ...splitWithAccountTT, name: 'split2' }, - { ...splitWithUserTT, name: 'split3' }, - ], [], 1); - cache.addSplit({ ...splitWithUserTT, name: 'split4' }); + cache.update([ + { ...splitWithUserTT, name: 'split1' }, + { ...splitWithAccountTT, name: 'split2' }, + { ...splitWithUserTT, name: 'split3' }, + ], [], 1); + cache.addSplit({ ...splitWithUserTT, name: 'split4' }); - expect(cache.trafficTypeExists('user_tt')).toBe(true); - expect(cache.trafficTypeExists('account_tt')).toBe(true); - expect(cache.trafficTypeExists('not_existent_tt')).toBe(false); + expect(cache.trafficTypeExists('user_tt')).toBe(true); + expect(cache.trafficTypeExists('account_tt')).toBe(true); + expect(cache.trafficTypeExists('not_existent_tt')).toBe(false); - cache.removeSplit('split4'); + cache.removeSplit('split4'); - expect(cache.trafficTypeExists('user_tt')).toBe(true); - expect(cache.trafficTypeExists('account_tt')).toBe(true); + expect(cache.trafficTypeExists('user_tt')).toBe(true); + expect(cache.trafficTypeExists('account_tt')).toBe(true); - cache.removeSplit('split3'); - cache.removeSplit('split2'); + cache.removeSplit('split3'); + cache.removeSplit('split2'); - expect(cache.trafficTypeExists('user_tt')).toBe(true); - expect(cache.trafficTypeExists('account_tt')).toBe(false); + expect(cache.trafficTypeExists('user_tt')).toBe(true); + expect(cache.trafficTypeExists('account_tt')).toBe(false); - cache.removeSplit('split1'); + cache.removeSplit('split1'); - expect(cache.trafficTypeExists('user_tt')).toBe(false); - expect(cache.trafficTypeExists('account_tt')).toBe(false); + expect(cache.trafficTypeExists('user_tt')).toBe(false); + expect(cache.trafficTypeExists('account_tt')).toBe(false); - cache.addSplit({ ...splitWithUserTT, name: 'split1' }); - expect(cache.trafficTypeExists('user_tt')).toBe(true); + cache.addSplit({ ...splitWithUserTT, name: 'split1' }); + expect(cache.trafficTypeExists('user_tt')).toBe(true); - cache.addSplit({ ...splitWithAccountTT, name: 'split1' }); - expect(cache.trafficTypeExists('account_tt')).toBe(true); - expect(cache.trafficTypeExists('user_tt')).toBe(false); + cache.addSplit({ ...splitWithAccountTT, name: 'split1' }); + expect(cache.trafficTypeExists('account_tt')).toBe(true); + expect(cache.trafficTypeExists('user_tt')).toBe(false); -}); + }); -test('SPLITS CACHE / LocalStorage / killLocally', () => { - const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'), localStorage); + test('LocalStorage / killLocally', () => { + const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS(PREFIX, 'user'), storage); - cache.addSplit(something); - cache.addSplit(somethingElse); - const initialChangeNumber = cache.getChangeNumber(); + cache.addSplit(something); + cache.addSplit(somethingElse); + const initialChangeNumber = cache.getChangeNumber(); - // kill an non-existent split - let updated = cache.killLocally('nonexistent_split', 'other_treatment', 101); - const nonexistentSplit = cache.getSplit('nonexistent_split'); + // kill an non-existent split + let updated = cache.killLocally('nonexistent_split', 'other_treatment', 101); + const nonexistentSplit = cache.getSplit('nonexistent_split'); - expect(updated).toBe(false); // killLocally resolves without update if split doesn't exist - expect(nonexistentSplit).toBe(null); // non-existent split keeps being non-existent + expect(updated).toBe(false); // killLocally resolves without update if split doesn't exist + expect(nonexistentSplit).toBe(null); // non-existent split keeps being non-existent - // kill an existent split - updated = cache.killLocally(something.name, 'some_treatment', 100); - let lol1Split = cache.getSplit(something.name) as ISplit; + // kill an existent split + updated = cache.killLocally(something.name, 'some_treatment', 100); + let lol1Split = cache.getSplit(something.name) as ISplit; - expect(updated).toBe(true); // killLocally resolves with update if split is changed - expect(lol1Split.killed).toBe(true); // existing split must be killed - expect(lol1Split.defaultTreatment).toBe('some_treatment'); // existing split must have new default treatment - expect(lol1Split.changeNumber).toBe(100); // existing split must have the given change number - expect(cache.getChangeNumber()).toBe(initialChangeNumber); // cache changeNumber is not changed + expect(updated).toBe(true); // killLocally resolves with update if split is changed + expect(lol1Split.killed).toBe(true); // existing split must be killed + expect(lol1Split.defaultTreatment).toBe('some_treatment'); // existing split must have new default treatment + expect(lol1Split.changeNumber).toBe(100); // existing split must have the given change number + expect(cache.getChangeNumber()).toBe(initialChangeNumber); // cache changeNumber is not changed - // not update if changeNumber is old - updated = cache.killLocally(something.name, 'some_treatment_2', 90); - lol1Split = cache.getSplit(something.name) as ISplit; + // not update if changeNumber is old + updated = cache.killLocally(something.name, 'some_treatment_2', 90); + lol1Split = cache.getSplit(something.name) as ISplit; - expect(updated).toBe(false); // killLocally resolves without update if changeNumber is old - expect(lol1Split.defaultTreatment).not.toBe('some_treatment_2'); // existing split is not updated if given changeNumber is older + expect(updated).toBe(false); // killLocally resolves without update if changeNumber is old + expect(lol1Split.defaultTreatment).not.toBe('some_treatment_2'); // existing split is not updated if given changeNumber is older -}); + }); -test('SPLITS CACHE / LocalStorage / usesSegments', () => { - const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'), localStorage); + test('LocalStorage / usesSegments', () => { + const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS(PREFIX, 'user'), storage); - expect(cache.usesSegments()).toBe(true); // true initially, until data is synchronized - cache.setChangeNumber(1); // to indicate that data has been synced. + expect(cache.usesSegments()).toBe(true); // true initially, until data is synchronized + cache.setChangeNumber(1); // to indicate that data has been synced. - cache.update([splitWithUserTT, splitWithAccountTT], [], 1); - expect(cache.usesSegments()).toBe(false); // 0 splits using segments + cache.update([splitWithUserTT, splitWithAccountTT], [], 1); + expect(cache.usesSegments()).toBe(false); // 0 splits using segments - cache.addSplit({ ...splitWithAccountTTAndUsesSegments, name: 'split3' }); - expect(cache.usesSegments()).toBe(true); // 1 split using segments + cache.addSplit({ ...splitWithAccountTTAndUsesSegments, name: 'split3' }); + expect(cache.usesSegments()).toBe(true); // 1 split using segments - cache.addSplit({ ...splitWithAccountTTAndUsesSegments, name: 'split4' }); - expect(cache.usesSegments()).toBe(true); // 2 splits using segments + cache.addSplit({ ...splitWithAccountTTAndUsesSegments, name: 'split4' }); + expect(cache.usesSegments()).toBe(true); // 2 splits using segments - cache.removeSplit('split3'); - expect(cache.usesSegments()).toBe(true); // 1 split using segments + cache.removeSplit('split3'); + expect(cache.usesSegments()).toBe(true); // 1 split using segments - cache.removeSplit('split4'); - expect(cache.usesSegments()).toBe(false); // 0 splits using segments -}); + cache.removeSplit('split4'); + expect(cache.usesSegments()).toBe(false); // 0 splits using segments + }); -test('SPLITS CACHE / LocalStorage / flag set cache tests', () => { - // @ts-ignore - const cache = new SplitsCacheInLocal({ - ...fullSettings, - sync: { // @ts-expect-error - __splitFiltersValidation: { - groupedFilters: { bySet: ['o', 'n', 'e', 'x'], byName: [], byPrefix: [] }, - queryString: '&sets=e,n,o,x', + test('LocalStorage / flag set cache tests', () => { + // @ts-ignore + const cache = new SplitsCacheInLocal({ + ...fullSettings, + sync: { // @ts-expect-error + __splitFiltersValidation: { + groupedFilters: { bySet: ['o', 'n', 'e', 'x'], byName: [], byPrefix: [] }, + queryString: '&sets=e,n,o,x', + } } - } - }, new KeyBuilderCS('SPLITIO', 'user'), localStorage); + }, new KeyBuilderCS(PREFIX, 'user'), storage); - const emptySet = new Set([]); + const emptySet = new Set([]); - cache.update([ - featureFlagOne, - featureFlagTwo, - featureFlagThree, - ], [], -1); - cache.addSplit(featureFlagWithEmptyFS); + cache.update([ + featureFlagOne, + featureFlagTwo, + featureFlagThree, + ], [], -1); + cache.addSplit(featureFlagWithEmptyFS); - 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([emptySet]); // 't' not in filter - expect(cache.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([emptySet]); // 't' not in filter + expect(cache.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new Set(['ff_one', 'ff_two']), new Set(['ff_one']), new Set(['ff_one', 'ff_three'])]); - cache.addSplit({ ...featureFlagOne, sets: ['1'] }); + cache.addSplit({ ...featureFlagOne, sets: ['1'] }); - expect(cache.getNamesByFlagSets(['1'])).toEqual([emptySet]); // '1' not in filter - expect(cache.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_two'])]); - expect(cache.getNamesByFlagSets(['n'])).toEqual([emptySet]); + expect(cache.getNamesByFlagSets(['1'])).toEqual([emptySet]); // '1' not in filter + expect(cache.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_two'])]); + expect(cache.getNamesByFlagSets(['n'])).toEqual([emptySet]); - cache.addSplit({ ...featureFlagOne, sets: ['x'] }); - expect(cache.getNamesByFlagSets(['x'])).toEqual([new Set(['ff_one'])]); - expect(cache.getNamesByFlagSets(['o', 'e', 'x'])).toEqual([new Set(['ff_two']), new Set(['ff_three']), new Set(['ff_one'])]); + cache.addSplit({ ...featureFlagOne, sets: ['x'] }); + expect(cache.getNamesByFlagSets(['x'])).toEqual([new Set(['ff_one'])]); + expect(cache.getNamesByFlagSets(['o', 'e', 'x'])).toEqual([new Set(['ff_two']), new Set(['ff_three']), new Set(['ff_one'])]); - cache.removeSplit(featureFlagOne.name); - expect(cache.getNamesByFlagSets(['x'])).toEqual([emptySet]); + cache.removeSplit(featureFlagOne.name); + expect(cache.getNamesByFlagSets(['x'])).toEqual([emptySet]); - cache.removeSplit(featureFlagOne.name); - expect(cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); // 'y' not in filter - expect(cache.getNamesByFlagSets([])).toEqual([]); + cache.removeSplit(featureFlagOne.name); + expect(cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); // 'y' not in filter + expect(cache.getNamesByFlagSets([])).toEqual([]); - cache.addSplit(featureFlagWithoutFS); - expect(cache.getNamesByFlagSets([])).toEqual([]); -}); + cache.addSplit(featureFlagWithoutFS); + expect(cache.getNamesByFlagSets([])).toEqual([]); + }); + + // if FlagSets are not defined, it should store all FlagSets in memory. + test('LocalStorage / flag set cache tests without filters', () => { + const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS(PREFIX, 'user'), storage); + + const emptySet = new Set([]); + + cache.update([ + featureFlagOne, + featureFlagTwo, + featureFlagThree, + ], [], -1); + cache.addSplit(featureFlagWithEmptyFS); + + 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'])]); -// 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'), localStorage); - - const emptySet = new Set([]); - - cache.update([ - featureFlagOne, - featureFlagTwo, - featureFlagThree, - ], [], -1); - cache.addSplit(featureFlagWithEmptyFS); - - 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 - cache.clear(); - expect(localStorage.length).toBe(0); + // Validate that the feature flag cache is cleared when calling `clear` method + cache.clear(); + expect(storage.length).toBe(0); + }); }); diff --git a/src/storages/inLocalStorage/__tests__/index.spec.ts b/src/storages/inLocalStorage/__tests__/index.spec.ts index b9d0fc1d..5d11f514 100644 --- a/src/storages/inLocalStorage/__tests__/index.spec.ts +++ b/src/storages/inLocalStorage/__tests__/index.spec.ts @@ -23,19 +23,29 @@ describe('IN LOCAL STORAGE', () => { fakeInMemoryStorageFactory.mockClear(); }); - test('calls InMemoryStorage factory if LocalStorage API is not available', () => { - + test('calls InMemoryStorage factory if LocalStorage API is not available or the provided storage wrapper is invalid', () => { + // Delete global localStorage property const originalLocalStorage = Object.getOwnPropertyDescriptor(global, 'localStorage'); - Object.defineProperty(global, 'localStorage', {}); // delete global localStorage property - - const storageFactory = InLocalStorage({ prefix: 'prefix' }); - const storage = storageFactory(internalSdkParams); + Object.defineProperty(global, 'localStorage', {}); + // LocalStorage API is not available + let storageFactory = InLocalStorage({ prefix: 'prefix' }); + let storage = storageFactory(internalSdkParams); expect(fakeInMemoryStorageFactory).toBeCalledWith(internalSdkParams); // calls InMemoryStorage factory expect(storage).toBe(fakeInMemoryStorage); - Object.defineProperty(global, 'localStorage', originalLocalStorage as PropertyDescriptor); // restore original localStorage + // @ts-expect-error Provided storage is invalid + storageFactory = InLocalStorage({ prefix: 'prefix', storage: {} }); + storage = storageFactory(internalSdkParams); + expect(storage).toBe(fakeInMemoryStorage); + + // Provided storage is valid + storageFactory = InLocalStorage({ prefix: 'prefix', storage: { getItem: () => Promise.resolve(null), setItem: () => Promise.resolve(), removeItem: () => Promise.resolve() } }); + storage = storageFactory(internalSdkParams); + expect(storage).not.toBe(fakeInMemoryStorage); + // Restore original localStorage + Object.defineProperty(global, 'localStorage', originalLocalStorage as PropertyDescriptor); }); test('calls its own storage factory if LocalStorage API is available', () => { diff --git a/src/storages/inLocalStorage/__tests__/validateCache.spec.ts b/src/storages/inLocalStorage/__tests__/validateCache.spec.ts index 2699d184..102444ba 100644 --- a/src/storages/inLocalStorage/__tests__/validateCache.spec.ts +++ b/src/storages/inLocalStorage/__tests__/validateCache.spec.ts @@ -6,16 +6,17 @@ import { SplitsCacheInLocal } from '../SplitsCacheInLocal'; import { nearlyEqual } from '../../../__tests__/testUtils'; import { MySegmentsCacheInLocal } from '../MySegmentsCacheInLocal'; import { RBSegmentsCacheInLocal } from '../RBSegmentsCacheInLocal'; +import { storages, PREFIX } from './wrapper.mock'; const FULL_SETTINGS_HASH = 'dc1f9817'; -describe('validateCache', () => { - const keys = new KeyBuilderCS('SPLITIO', 'user'); +describe.each(storages)('validateCache', (storage) => { + const keys = new KeyBuilderCS(PREFIX, 'user'); const logSpy = jest.spyOn(fullSettings.log, 'info'); - const segments = new MySegmentsCacheInLocal(fullSettings.log, keys, localStorage); - const largeSegments = new MySegmentsCacheInLocal(fullSettings.log, keys, localStorage); - const splits = new SplitsCacheInLocal(fullSettings, keys, localStorage); - const rbSegments = new RBSegmentsCacheInLocal(fullSettings, keys, localStorage); + const segments = new MySegmentsCacheInLocal(fullSettings.log, keys, storage); + const largeSegments = new MySegmentsCacheInLocal(fullSettings.log, keys, storage); + const splits = new SplitsCacheInLocal(fullSettings, keys, storage); + const rbSegments = new RBSegmentsCacheInLocal(fullSettings, keys, storage); jest.spyOn(splits, 'getChangeNumber'); jest.spyOn(splits, 'clear'); @@ -25,11 +26,11 @@ describe('validateCache', () => { beforeEach(() => { jest.clearAllMocks(); - localStorage.clear(); + for (let i = 0; i < storage.length; i++) storage.removeItem(storage.key(i) as string); }); test('if there is no cache, it should return false', async () => { - expect(await validateCache({}, localStorage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false); + expect(await validateCache({}, storage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false); expect(logSpy).not.toHaveBeenCalled(); @@ -39,15 +40,15 @@ describe('validateCache', () => { 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(); + expect(storage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH); + expect(storage.getItem(keys.buildLastClear())).toBeNull(); }); test('if there is cache and it must not be cleared, it should return true', async () => { - localStorage.setItem(keys.buildSplitsTillKey(), '1'); - localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); + storage.setItem(keys.buildSplitsTillKey(), '1'); + storage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); - expect(await validateCache({}, localStorage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(true); + expect(await validateCache({}, storage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(true); expect(logSpy).not.toHaveBeenCalled(); @@ -57,16 +58,16 @@ describe('validateCache', () => { 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(); + expect(storage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH); + expect(storage.getItem(keys.buildLastClear())).toBeNull(); }); test('if there is cache and it has expired, it should clear cache and return false', async () => { - 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 + storage.setItem(keys.buildSplitsTillKey(), '1'); + storage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); + storage.setItem(keys.buildLastUpdatedKey(), Date.now() - 1000 * 60 * 60 * 24 * 2 + ''); // 2 days ago - expect(await validateCache({ expirationDays: 1 }, localStorage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false); + expect(await validateCache({ expirationDays: 1 }, storage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false); expect(logSpy).toHaveBeenCalledWith('storage:localstorage: Cache expired more than 1 days ago. Cleaning up cache'); @@ -75,15 +76,15 @@ describe('validateCache', () => { 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); + expect(storage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH); + expect(nearlyEqual(parseInt(storage.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', async () => { - localStorage.setItem(keys.buildSplitsTillKey(), '1'); - localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); + storage.setItem(keys.buildSplitsTillKey(), '1'); + storage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); - expect(await validateCache({}, localStorage, { ...fullSettings, core: { ...fullSettings.core, authorizationKey: 'another-sdk-key' } }, keys, splits, rbSegments, segments, largeSegments)).toBe(false); + expect(await validateCache({}, storage, { ...fullSettings, core: { ...fullSettings.core, authorizationKey: 'another-sdk-key' } }, keys, splits, rbSegments, segments, largeSegments)).toBe(false); expect(logSpy).toHaveBeenCalledWith('storage:localstorage: SDK key, flags filter criteria, or flags spec version has changed. Cleaning up cache'); @@ -92,16 +93,16 @@ describe('validateCache', () => { expect(segments.clear).toHaveBeenCalledTimes(1); expect(largeSegments.clear).toHaveBeenCalledTimes(1); - expect(localStorage.getItem(keys.buildHashKey())).toBe('45c6ba5d'); - expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true); + expect(storage.getItem(keys.buildHashKey())).toBe('45c6ba5d'); + expect(nearlyEqual(parseInt(storage.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', async () => { // Older cache version (without last clear) - localStorage.setItem(keys.buildSplitsTillKey(), '1'); - localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); + storage.setItem(keys.buildSplitsTillKey(), '1'); + storage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); - expect(await validateCache({ clearOnInit: true }, localStorage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false); + expect(await validateCache({ clearOnInit: true }, storage, fullSettings, keys, splits, rbSegments, 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'); @@ -110,25 +111,25 @@ describe('validateCache', () => { 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(storage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH); + const lastClear = storage.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(await validateCache({ clearOnInit: true }, localStorage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(true); + storage.setItem(keys.buildSplitsTillKey(), '1'); + expect(await validateCache({ clearOnInit: true }, storage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(true); expect(logSpy).not.toHaveBeenCalled(); - expect(localStorage.getItem(keys.buildLastClear())).toBe(lastClear); // Last clear should not have changed + expect(storage.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(await validateCache({ clearOnInit: true }, localStorage, fullSettings, keys, splits, rbSegments, segments, largeSegments)).toBe(false); + storage.setItem(keys.buildLastClear(), (Date.now() - 1000 * 60 * 60 * 24 - 1) + ''); + expect(await validateCache({ clearOnInit: true }, storage, fullSettings, keys, splits, rbSegments, 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(rbSegments.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); + expect(nearlyEqual(parseInt(storage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true); }); }); diff --git a/src/storages/inLocalStorage/__tests__/wrapper.mock.ts b/src/storages/inLocalStorage/__tests__/wrapper.mock.ts new file mode 100644 index 00000000..f1d3b90c --- /dev/null +++ b/src/storages/inLocalStorage/__tests__/wrapper.mock.ts @@ -0,0 +1,27 @@ +import { storageAdapter } from '..'; +import SplitIO from '../../../../types/splitio'; +import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; + +export const PREFIX = 'SPLITIO'; + +export function createMemoryStorage(): SplitIO.Storage { + let cache: Record = {}; + return { + getItem(key: string) { + return Promise.resolve(cache[key] || null); + }, + setItem(key: string, value: string) { + cache[key] = value; + return Promise.resolve(); + }, + removeItem(key: string) { + delete cache[key]; + return Promise.resolve(); + } + }; +} + +export const storages = [ + localStorage, + storageAdapter(loggerMock, PREFIX, createMemoryStorage()) +]; diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 7c04440f..b1e94254 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -34,7 +34,7 @@ function isTillKey(key: string) { return key.endsWith('.till'); } -function storageAdapter(log: ILogger, prefix: string, storage: SplitIO.Storage): StorageAdapter { +export function storageAdapter(log: ILogger, prefix: string, storage: SplitIO.Storage): StorageAdapter { let cache: Record = {}; let connectPromise: Promise | undefined; diff --git a/types/splitio.d.ts b/types/splitio.d.ts index c9e8fbf9..3600e682 100644 --- a/types/splitio.d.ts +++ b/types/splitio.d.ts @@ -457,11 +457,11 @@ declare namespace SplitIO { /** * Returns a promise that resolves when the value of the pair identified by key is set to value, creating a new key/value pair if none existed for key previously. */ - setItem(key: string, value: string): Promise; - // /** - // * Returns a promise that resolves when the key/value pair with the given key is removed, if a key/value pair with the given key exists. - // */ - // removeItem(key: string): Promise; + setItem(key: string, value: string): Promise; + /** + * Returns a promise that resolves when the key/value pair with the given key is removed, if a key/value pair with the given key exists. + */ + removeItem(key: string): Promise; } /** From 4b0988df659c060e636bb3ace587939ed53fc543 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 23 Jun 2025 17:51:59 -0300 Subject: [PATCH 03/14] Update types --- types/splitio.d.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/types/splitio.d.ts b/types/splitio.d.ts index 3600e682..9f979e7a 100644 --- a/types/splitio.d.ts +++ b/types/splitio.d.ts @@ -1324,6 +1324,12 @@ declare namespace SplitIO { * @defaultValue `false` */ clearOnInit?: boolean; + /** + * Optional storage API to persist rollout plan related data. If not provided, the SDK will use the default localStorage Web API. + * + * @defaultValue `window.localStorage` + */ + storage?: Storage; }; } /** From 299585b18880038c125c0a1c78d80262f7981f79 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 18 Jul 2025 15:07:16 -0300 Subject: [PATCH 04/14] refactor: rename Storage interface to StorageWrapper for clarity and consistency --- .../inLocalStorage/__tests__/index.spec.ts | 4 ++-- .../inLocalStorage/__tests__/wrapper.mock.ts | 2 +- src/storages/inLocalStorage/index.ts | 16 ++++++++-------- src/utils/env/isLocalStorageAvailable.ts | 10 +++++----- types/splitio.d.ts | 10 +++++----- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/storages/inLocalStorage/__tests__/index.spec.ts b/src/storages/inLocalStorage/__tests__/index.spec.ts index 5d11f514..545c532f 100644 --- a/src/storages/inLocalStorage/__tests__/index.spec.ts +++ b/src/storages/inLocalStorage/__tests__/index.spec.ts @@ -35,12 +35,12 @@ describe('IN LOCAL STORAGE', () => { expect(storage).toBe(fakeInMemoryStorage); // @ts-expect-error Provided storage is invalid - storageFactory = InLocalStorage({ prefix: 'prefix', storage: {} }); + storageFactory = InLocalStorage({ prefix: 'prefix', wrapper: {} }); storage = storageFactory(internalSdkParams); expect(storage).toBe(fakeInMemoryStorage); // Provided storage is valid - storageFactory = InLocalStorage({ prefix: 'prefix', storage: { getItem: () => Promise.resolve(null), setItem: () => Promise.resolve(), removeItem: () => Promise.resolve() } }); + storageFactory = InLocalStorage({ prefix: 'prefix', wrapper: { getItem: () => Promise.resolve(null), setItem: () => Promise.resolve(), removeItem: () => Promise.resolve() } }); storage = storageFactory(internalSdkParams); expect(storage).not.toBe(fakeInMemoryStorage); diff --git a/src/storages/inLocalStorage/__tests__/wrapper.mock.ts b/src/storages/inLocalStorage/__tests__/wrapper.mock.ts index f1d3b90c..dbfcbf6a 100644 --- a/src/storages/inLocalStorage/__tests__/wrapper.mock.ts +++ b/src/storages/inLocalStorage/__tests__/wrapper.mock.ts @@ -4,7 +4,7 @@ import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; export const PREFIX = 'SPLITIO'; -export function createMemoryStorage(): SplitIO.Storage { +export function createMemoryStorage(): SplitIO.StorageWrapper { let cache: Record = {}; return { getItem(key: string) { diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index b1e94254..6e8c41d3 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -4,7 +4,7 @@ import { EventsCacheInMemory } from '../inMemory/EventsCacheInMemory'; import { IStorageFactoryParams, IStorageSync, IStorageSyncFactory } from '../types'; import { validatePrefix } from '../KeyBuilder'; import { KeyBuilderCS, myLargeSegmentsKeyBuilder } from '../KeyBuilderCS'; -import { isLocalStorageAvailable, isStorageValid } from '../../utils/env/isLocalStorageAvailable'; +import { isLocalStorageAvailable, isStorageWrapperValid } from '../../utils/env/isLocalStorageAvailable'; import { SplitsCacheInLocal } from './SplitsCacheInLocal'; import { RBSegmentsCacheInLocal } from './RBSegmentsCacheInLocal'; import { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal'; @@ -34,7 +34,7 @@ function isTillKey(key: string) { return key.endsWith('.till'); } -export function storageAdapter(log: ILogger, prefix: string, storage: SplitIO.Storage): StorageAdapter { +export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.StorageWrapper): StorageAdapter { let cache: Record = {}; let connectPromise: Promise | undefined; @@ -42,7 +42,7 @@ export function storageAdapter(log: ILogger, prefix: string, storage: SplitIO.St return { load() { - return connectPromise || (connectPromise = storage.getItem(prefix).then((storedCache) => { + return connectPromise || (connectPromise = wrapper.getItem(prefix).then((storedCache) => { cache = JSON.parse(storedCache || '{}'); }).catch((e) => { log.error(LOG_PREFIX + 'Rejected promise calling storage getItem, with error: ' + e); @@ -50,7 +50,7 @@ export function storageAdapter(log: ILogger, prefix: string, storage: SplitIO.St }, save() { return disconnectPromise = disconnectPromise.then(() => { - return storage.setItem(prefix, JSON.stringify(cache)).catch((e) => { + return wrapper.setItem(prefix, JSON.stringify(cache)).catch((e) => { log.error(LOG_PREFIX + 'Rejected promise calling storage setItem, with error: ' + e); }); }); @@ -76,9 +76,9 @@ export function storageAdapter(log: ILogger, prefix: string, storage: SplitIO.St }; } -function validateStorage(log: ILogger, prefix: string, storage?: SplitIO.Storage): StorageAdapter | undefined { - if (storage) { - if (isStorageValid(storage)) return storageAdapter(log, prefix, storage); +function validateStorage(log: ILogger, prefix: string, wrapper?: SplitIO.StorageWrapper): StorageAdapter | undefined { + if (wrapper) { + if (isStorageWrapperValid(wrapper)) return storageAdapter(log, prefix, wrapper); log.warn(LOG_PREFIX + 'Invalid storage provided. Falling back to LocalStorage API'); } @@ -97,7 +97,7 @@ export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): ISt function InLocalStorageCSFactory(params: IStorageFactoryParams): IStorageSync { const { settings, settings: { log, scheduler: { impressionsQueueSize, eventsQueueSize } } } = params; - const storage = validateStorage(log, prefix, options.storage); + const storage = validateStorage(log, prefix, options.wrapper); if (!storage) return InMemoryStorageCSFactory(params); const matchingKey = getMatching(settings.core.key); diff --git a/src/utils/env/isLocalStorageAvailable.ts b/src/utils/env/isLocalStorageAvailable.ts index 5e98b87d..b01efec0 100644 --- a/src/utils/env/isLocalStorageAvailable.ts +++ b/src/utils/env/isLocalStorageAvailable.ts @@ -1,18 +1,18 @@ export function isLocalStorageAvailable(): boolean { try { // eslint-disable-next-line no-undef - return isStorageValid(localStorage); + return isStorageWrapperValid(localStorage); } catch (e) { return false; } } -export function isStorageValid(storage: any): boolean { +export function isStorageWrapperValid(wrapper: any): boolean { var mod = '__SPLITSOFTWARE__'; try { - storage.setItem(mod, mod); - storage.getItem(mod); - storage.removeItem(mod); + wrapper.setItem(mod, mod); + wrapper.getItem(mod); + wrapper.removeItem(mod); return true; } catch (e) { return false; diff --git a/types/splitio.d.ts b/types/splitio.d.ts index 9f979e7a..6b71c204 100644 --- a/types/splitio.d.ts +++ b/types/splitio.d.ts @@ -449,7 +449,7 @@ interface IClientSideSyncSharedSettings extends IClientSideSharedSettings, ISync */ declare namespace SplitIO { - interface Storage { + interface StorageWrapper { /** * Returns a promise that resolves to the current value associated with the given key, or null if the given key does not exist. */ @@ -979,11 +979,11 @@ declare namespace SplitIO { */ clearOnInit?: boolean; /** - * Optional storage API to persist rollout plan related data. If not provided, the SDK will use the default localStorage Web API. + * Optional storage wrapper to persist rollout plan related data. If not provided, the SDK will use the default localStorage Web API. * * @defaultValue `window.localStorage` */ - storage?: Storage; + wrapper?: StorageWrapper; } /** * Storage for asynchronous (consumer) SDK. @@ -1325,11 +1325,11 @@ declare namespace SplitIO { */ clearOnInit?: boolean; /** - * Optional storage API to persist rollout plan related data. If not provided, the SDK will use the default localStorage Web API. + * Optional storage wrapper to persist rollout plan related data. If not provided, the SDK will use the default localStorage Web API. * * @defaultValue `window.localStorage` */ - storage?: Storage; + wrapper?: StorageWrapper; }; } /** From 70ea6d424d08928e81b496ce6ffed3e262b19cf6 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 30 Jul 2025 16:32:46 -0300 Subject: [PATCH 05/14] Revert "Break the PR into smaller PRs" This reverts commit 96eaea06d2f68fa59f9fc383bdb830140cd0305b. --- src/storages/AbstractMySegmentsCacheSync.ts | 46 +++++++++++-------- src/storages/AbstractSplitsCacheSync.ts | 5 +- .../inLocalStorage/RBSegmentsCacheInLocal.ts | 5 +- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/storages/AbstractMySegmentsCacheSync.ts b/src/storages/AbstractMySegmentsCacheSync.ts index 7d3dc304..5b72aaf9 100644 --- a/src/storages/AbstractMySegmentsCacheSync.ts +++ b/src/storages/AbstractMySegmentsCacheSync.ts @@ -49,12 +49,10 @@ export abstract class AbstractMySegmentsCacheSync implements ISegmentsCacheSync * For client-side synchronizer: it resets or updates the cache. */ resetSegments(segmentsData: MySegmentsData | IMySegmentsResponse): boolean { - this.setChangeNumber(segmentsData.cn); - const { added, removed } = segmentsData as MySegmentsData; + let isDiff = false; if (added && removed) { - let isDiff = false; added.forEach(segment => { isDiff = this.addSegment(segment) || isDiff; @@ -63,32 +61,40 @@ export abstract class AbstractMySegmentsCacheSync implements ISegmentsCacheSync removed.forEach(segment => { isDiff = this.removeSegment(segment) || isDiff; }); + } else { - return isDiff; - } + const names = ((segmentsData as IMySegmentsResponse).k || []).map(s => s.n).sort(); + const storedSegmentKeys = this.getRegisteredSegments().sort(); - const names = ((segmentsData as IMySegmentsResponse).k || []).map(s => s.n).sort(); - const storedSegmentKeys = this.getRegisteredSegments().sort(); + // Extreme fast => everything is empty + if (!names.length && !storedSegmentKeys.length) { + isDiff = false; + } else { - // Extreme fast => everything is empty - if (!names.length && !storedSegmentKeys.length) return false; + let index = 0; - let index = 0; + while (index < names.length && index < storedSegmentKeys.length && names[index] === storedSegmentKeys[index]) index++; - while (index < names.length && index < storedSegmentKeys.length && names[index] === storedSegmentKeys[index]) index++; + // Quick path => no changes + if (index === names.length && index === storedSegmentKeys.length) { + isDiff = false; + } else { - // Quick path => no changes - if (index === names.length && index === storedSegmentKeys.length) return false; + // Slowest path => add and/or remove segments + for (let removeIndex = index; removeIndex < storedSegmentKeys.length; removeIndex++) { + this.removeSegment(storedSegmentKeys[removeIndex]); + } - // Slowest path => add and/or remove segments - for (let removeIndex = index; removeIndex < storedSegmentKeys.length; removeIndex++) { - this.removeSegment(storedSegmentKeys[removeIndex]); - } + for (let addIndex = index; addIndex < names.length; addIndex++) { + this.addSegment(names[addIndex]); + } - for (let addIndex = index; addIndex < names.length; addIndex++) { - this.addSegment(names[addIndex]); + isDiff = true; + } + } } - return true; + this.setChangeNumber(segmentsData.cn); + return isDiff; } } diff --git a/src/storages/AbstractSplitsCacheSync.ts b/src/storages/AbstractSplitsCacheSync.ts index 2a4b9b78..64194561 100644 --- a/src/storages/AbstractSplitsCacheSync.ts +++ b/src/storages/AbstractSplitsCacheSync.ts @@ -14,9 +14,10 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync { protected abstract setChangeNumber(changeNumber: number): boolean | void update(toAdd: ISplit[], toRemove: ISplit[], changeNumber: number): boolean { + let updated = toAdd.map(addedFF => this.addSplit(addedFF)).some(result => result); + updated = toRemove.map(removedFF => this.removeSplit(removedFF.name)).some(result => result) || updated; this.setChangeNumber(changeNumber); - const updated = toAdd.map(addedFF => this.addSplit(addedFF)).some(result => result); - return toRemove.map(removedFF => this.removeSplit(removedFF.name)).some(result => result) || updated; + return updated; } abstract getSplit(name: string): ISplit | null diff --git a/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts b/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts index 312787bc..b0b2aba5 100644 --- a/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts @@ -26,9 +26,10 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync { } update(toAdd: IRBSegment[], toRemove: IRBSegment[], changeNumber: number): boolean { + let updated = toAdd.map(toAdd => this.add(toAdd)).some(result => result); + updated = toRemove.map(toRemove => this.remove(toRemove.name)).some(result => result) || updated; this.setChangeNumber(changeNumber); - const updated = toAdd.map(toAdd => this.add(toAdd)).some(result => result); - return toRemove.map(toRemove => this.remove(toRemove.name)).some(result => result) || updated; + return updated; } private setChangeNumber(changeNumber: number) { From 60aa25d72ceb8a537dcc586ee5d83e9707f0c0c7 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 31 Jul 2025 12:01:36 -0300 Subject: [PATCH 06/14] refactor: move storageAdapter to dedicated file and update imports --- .../inLocalStorage/__tests__/wrapper.mock.ts | 2 +- src/storages/inLocalStorage/index.ts | 47 +---------------- src/storages/inLocalStorage/storageAdapter.ts | 50 +++++++++++++++++++ 3 files changed, 52 insertions(+), 47 deletions(-) create mode 100644 src/storages/inLocalStorage/storageAdapter.ts diff --git a/src/storages/inLocalStorage/__tests__/wrapper.mock.ts b/src/storages/inLocalStorage/__tests__/wrapper.mock.ts index dbfcbf6a..9cbcbf02 100644 --- a/src/storages/inLocalStorage/__tests__/wrapper.mock.ts +++ b/src/storages/inLocalStorage/__tests__/wrapper.mock.ts @@ -1,4 +1,4 @@ -import { storageAdapter } from '..'; +import { storageAdapter } from '../storageAdapter'; import SplitIO from '../../../../types/splitio'; import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index d6b0691f..68f5fe79 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -17,52 +17,7 @@ import { getMatching } from '../../utils/key'; import { validateCache } from './validateCache'; import { ILogger } from '../../logger/types'; import SplitIO from '../../../types/splitio'; - -function isTillKey(key: string) { - return key.endsWith('.till'); -} - -export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.StorageWrapper): StorageAdapter { - let cache: Record = {}; - - let connectPromise: Promise | undefined; - let disconnectPromise = Promise.resolve(); - - return { - load() { - return connectPromise || (connectPromise = Promise.resolve(wrapper.getItem(prefix)).then((storedCache) => { - cache = JSON.parse(storedCache || '{}'); - }).catch((e) => { - log.error(LOG_PREFIX + 'Rejected promise calling storage getItem, with error: ' + e); - })); - }, - save() { - return disconnectPromise = disconnectPromise.then(() => { - return Promise.resolve(wrapper.setItem(prefix, JSON.stringify(cache))).catch((e) => { - log.error(LOG_PREFIX + 'Rejected promise calling storage setItem, with error: ' + e); - }); - }); - }, - - get length() { - return Object.keys(cache).length; - }, - getItem(key: string) { - return cache[key] || null; - }, - key(index: number) { - return Object.keys(cache)[index] || null; - }, - removeItem(key: string) { - delete cache[key]; - if (isTillKey(key)) this.save!(); - }, - setItem(key: string, value: string) { - cache[key] = value; - if (isTillKey(key)) this.save!(); - } - }; -} +import { storageAdapter } from './storageAdapter'; function validateStorage(log: ILogger, prefix: string, wrapper?: SplitIO.StorageWrapper): StorageAdapter | undefined { if (wrapper) { diff --git a/src/storages/inLocalStorage/storageAdapter.ts b/src/storages/inLocalStorage/storageAdapter.ts new file mode 100644 index 00000000..f0fbd21a --- /dev/null +++ b/src/storages/inLocalStorage/storageAdapter.ts @@ -0,0 +1,50 @@ +import { ILogger } from '../../logger/types'; +import SplitIO from '../../../types/splitio'; +import { LOG_PREFIX } from './constants'; +import { StorageAdapter } from '../types'; + +function isTillKey(key: string) { + return key.endsWith('.till'); +} + +export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.StorageWrapper): StorageAdapter { + let cache: Record = {}; + + let connectPromise: Promise | undefined; + let disconnectPromise = Promise.resolve(); + + return { + load() { + return connectPromise || (connectPromise = Promise.resolve(wrapper.getItem(prefix)).then((storedCache) => { + cache = JSON.parse(storedCache || '{}'); + }).catch((e) => { + log.error(LOG_PREFIX + 'Rejected promise calling storage getItem, with error: ' + e); + })); + }, + save() { + return disconnectPromise = disconnectPromise.then(() => { + return Promise.resolve(wrapper.setItem(prefix, JSON.stringify(cache))).catch((e) => { + log.error(LOG_PREFIX + 'Rejected promise calling storage setItem, with error: ' + e); + }); + }); + }, + + get length() { + return Object.keys(cache).length; + }, + getItem(key: string) { + return cache[key] || null; + }, + key(index: number) { + return Object.keys(cache)[index] || null; + }, + removeItem(key: string) { + delete cache[key]; + if (isTillKey(key)) this.save!(); + }, + setItem(key: string, value: string) { + cache[key] = value; + if (isTillKey(key)) this.save!(); + } + }; +} From cb22017d84d46b8817247c20bc88cde4e50991dc Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 1 Aug 2025 18:56:02 -0300 Subject: [PATCH 07/14] Add unit test --- .../inLocalStorage/MySegmentsCacheInLocal.ts | 2 +- .../__tests__/storageAdapter.spec.ts | 58 +++++++++++++++++++ src/storages/inLocalStorage/storageAdapter.ts | 28 +++++---- 3 files changed, 76 insertions(+), 12 deletions(-) create mode 100644 src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts diff --git a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts index 5fc176cb..fd038a07 100644 --- a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts @@ -51,7 +51,7 @@ export class MySegmentsCacheInLocal extends AbstractMySegmentsCacheSync { getRegisteredSegments(): string[] { const registeredSegments: string[] = []; - for (let i = 0; i < this.storage.length; i++) { + for (let i = 0, len = this.storage.length; i < len; i++) { const segmentName = this.keys.extractSegmentName(this.storage.key(i)!); if (segmentName) registeredSegments.push(segmentName); } diff --git a/src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts b/src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts new file mode 100644 index 00000000..5b1924c4 --- /dev/null +++ b/src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts @@ -0,0 +1,58 @@ +import { storageAdapter } from '../storageAdapter'; +import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; + + +const syncWrapper = { + getItem: jest.fn(() => JSON.stringify({ key1: 'value1' })), + setItem: jest.fn(), + removeItem: jest.fn(), +}; + +const asyncWrapper = { + getItem: jest.fn(() => Promise.resolve(JSON.stringify({ key1: 'value1' }))), + setItem: jest.fn(() => Promise.resolve()), + removeItem: jest.fn(() => Promise.resolve()), +}; + +test.each([ + [syncWrapper], + [asyncWrapper], +])('storageAdapter', async (wrapper) => { + + const storage = storageAdapter(loggerMock, 'prefix', wrapper); + + await storage.load!(); + + expect(wrapper.getItem).toHaveBeenCalledWith('prefix'); + + expect(storage.length).toBe(1); + expect(storage.key(0)).toBe('key1'); + expect(storage.getItem('key1')).toBe('value1'); + + storage.setItem('key2', 'value2'); + expect(storage.getItem('key2')).toBe('value2'); + expect(storage.length).toBe(2); + + storage.removeItem('key1'); + expect(storage.getItem('key1')).toBe(null); + expect(storage.length).toBe(1); + + await storage.save!(); + expect(wrapper.setItem).not.toHaveBeenCalled(); + + storage.setItem('.till', '1'); + expect(storage.length).toBe(2); + expect(storage.key(0)).toBe('key2'); + expect(storage.key(1)).toBe('.till'); + + await storage.save!(); + expect(wrapper.setItem).toHaveBeenCalledWith('prefix', JSON.stringify({ key2: 'value2', '.till': '1' })); + + storage.removeItem('.till'); + + await storage.save!(); + expect(wrapper.setItem).toHaveBeenCalledWith('prefix', JSON.stringify({ key2: 'value2' })); + + await storage.save!(); + expect(wrapper.setItem).toHaveBeenCalledTimes(2); +}); diff --git a/src/storages/inLocalStorage/storageAdapter.ts b/src/storages/inLocalStorage/storageAdapter.ts index f0fbd21a..970d1812 100644 --- a/src/storages/inLocalStorage/storageAdapter.ts +++ b/src/storages/inLocalStorage/storageAdapter.ts @@ -10,23 +10,29 @@ function isTillKey(key: string) { export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.StorageWrapper): StorageAdapter { let cache: Record = {}; - let connectPromise: Promise | undefined; - let disconnectPromise = Promise.resolve(); + let loadPromise: Promise | undefined; + let savePromise = Promise.resolve(); + + function _save() { + return savePromise = savePromise.then(() => { + return Promise.resolve(wrapper.setItem(prefix, JSON.stringify(cache))); + }).catch((e) => { + log.error(LOG_PREFIX + 'Rejected promise calling wrapper `setItem` method, with error: ' + e); + }); + } return { load() { - return connectPromise || (connectPromise = Promise.resolve(wrapper.getItem(prefix)).then((storedCache) => { + return loadPromise || (loadPromise = Promise.resolve().then(() => { + return wrapper.getItem(prefix); + }).then((storedCache) => { cache = JSON.parse(storedCache || '{}'); }).catch((e) => { - log.error(LOG_PREFIX + 'Rejected promise calling storage getItem, with error: ' + e); + log.error(LOG_PREFIX + 'Rejected promise calling wrapper `getItem` method, with error: ' + e); })); }, save() { - return disconnectPromise = disconnectPromise.then(() => { - return Promise.resolve(wrapper.setItem(prefix, JSON.stringify(cache))).catch((e) => { - log.error(LOG_PREFIX + 'Rejected promise calling storage setItem, with error: ' + e); - }); - }); + return savePromise; }, get length() { @@ -40,11 +46,11 @@ export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.St }, removeItem(key: string) { delete cache[key]; - if (isTillKey(key)) this.save!(); + if (isTillKey(key)) _save(); }, setItem(key: string, value: string) { cache[key] = value; - if (isTillKey(key)) this.save!(); + if (isTillKey(key)) _save(); } }; } From 6bb29305af36d5c73e639cfae60703b5977e3c0d Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 1 Aug 2025 19:13:42 -0300 Subject: [PATCH 08/14] optimize StorageAdapter's length and key methods by using arrays instead of object cache --- src/storages/inLocalStorage/storageAdapter.ts | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/storages/inLocalStorage/storageAdapter.ts b/src/storages/inLocalStorage/storageAdapter.ts index 970d1812..9f10cf3f 100644 --- a/src/storages/inLocalStorage/storageAdapter.ts +++ b/src/storages/inLocalStorage/storageAdapter.ts @@ -8,13 +8,18 @@ function isTillKey(key: string) { } export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.StorageWrapper): StorageAdapter { - let cache: Record = {}; + let keys: string[] = []; + let values: string[] = []; let loadPromise: Promise | undefined; let savePromise = Promise.resolve(); function _save() { return savePromise = savePromise.then(() => { + const cache = keys.reduce((acc, key, index) => { + acc[key] = values[index]; + return acc; + }, {} as Record); return Promise.resolve(wrapper.setItem(prefix, JSON.stringify(cache))); }).catch((e) => { log.error(LOG_PREFIX + 'Rejected promise calling wrapper `setItem` method, with error: ' + e); @@ -26,7 +31,9 @@ export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.St return loadPromise || (loadPromise = Promise.resolve().then(() => { return wrapper.getItem(prefix); }).then((storedCache) => { - cache = JSON.parse(storedCache || '{}'); + const cache = JSON.parse(storedCache || '{}'); + keys = Object.keys(cache); + values = keys.map(key => cache[key]); }).catch((e) => { log.error(LOG_PREFIX + 'Rejected promise calling wrapper `getItem` method, with error: ' + e); })); @@ -36,20 +43,29 @@ export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.St }, get length() { - return Object.keys(cache).length; + return keys.length; }, getItem(key: string) { - return cache[key] || null; + const index = keys.indexOf(key); + if (index === -1) return null; + return values[index]; }, key(index: number) { - return Object.keys(cache)[index] || null; + return keys[index] || null; }, removeItem(key: string) { - delete cache[key]; + const index = keys.indexOf(key); + if (index === -1) return; + keys.splice(index, 1); + values.splice(index, 1); + if (isTillKey(key)) _save(); }, setItem(key: string, value: string) { - cache[key] = value; + let index = keys.indexOf(key); + if (index === -1) index = keys.push(key) - 1; + values[index] = value; + if (isTillKey(key)) _save(); } }; From a7d3246b73811b6e214c639db7397a3a96cc3d49 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Sat, 2 Aug 2025 10:43:35 -0300 Subject: [PATCH 09/14] refactor: rename save method to whenSaved --- .../__tests__/storageAdapter.spec.ts | 19 +++++++++++++------ src/storages/inLocalStorage/index.ts | 2 +- src/storages/inLocalStorage/storageAdapter.ts | 4 ++-- src/storages/types.ts | 4 ++-- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts b/src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts index 5b1924c4..0112e592 100644 --- a/src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts +++ b/src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts @@ -21,38 +21,45 @@ test.each([ const storage = storageAdapter(loggerMock, 'prefix', wrapper); - await storage.load!(); + expect(storage.length).toBe(0); - expect(wrapper.getItem).toHaveBeenCalledWith('prefix'); + // Load cache from storage wrapper + await storage.load(); + expect(wrapper.getItem).toHaveBeenCalledWith('prefix'); expect(storage.length).toBe(1); expect(storage.key(0)).toBe('key1'); expect(storage.getItem('key1')).toBe('value1'); + // Set item storage.setItem('key2', 'value2'); expect(storage.getItem('key2')).toBe('value2'); expect(storage.length).toBe(2); + // Remove item storage.removeItem('key1'); expect(storage.getItem('key1')).toBe(null); expect(storage.length).toBe(1); - await storage.save!(); + // Until a till key is set/removed, changes should not be saved/persisted + await storage.whenSaved(); expect(wrapper.setItem).not.toHaveBeenCalled(); + // When setting a till key, changes should be saved/persisted immediately storage.setItem('.till', '1'); expect(storage.length).toBe(2); expect(storage.key(0)).toBe('key2'); expect(storage.key(1)).toBe('.till'); - await storage.save!(); + await storage.whenSaved(); expect(wrapper.setItem).toHaveBeenCalledWith('prefix', JSON.stringify({ key2: 'value2', '.till': '1' })); + // When removing a till key, changes should be saved/persisted immediately storage.removeItem('.till'); - await storage.save!(); + await storage.whenSaved(); expect(wrapper.setItem).toHaveBeenCalledWith('prefix', JSON.stringify({ key2: 'value2' })); - await storage.save!(); + await storage.whenSaved(); expect(wrapper.setItem).toHaveBeenCalledTimes(2); }); diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 68f5fe79..9cf1ae0c 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -68,7 +68,7 @@ export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): ISt }, destroy() { - return storage.save && storage.save(); + return storage.whenSaved && storage.whenSaved(); }, // When using shared instantiation with MEMORY we reuse everything but segments (they are customer per key). diff --git a/src/storages/inLocalStorage/storageAdapter.ts b/src/storages/inLocalStorage/storageAdapter.ts index 9f10cf3f..9eec6ec1 100644 --- a/src/storages/inLocalStorage/storageAdapter.ts +++ b/src/storages/inLocalStorage/storageAdapter.ts @@ -7,7 +7,7 @@ function isTillKey(key: string) { return key.endsWith('.till'); } -export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.StorageWrapper): StorageAdapter { +export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.StorageWrapper): Required { let keys: string[] = []; let values: string[] = []; @@ -38,7 +38,7 @@ export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.St log.error(LOG_PREFIX + 'Rejected promise calling wrapper `getItem` method, with error: ' + e); })); }, - save() { + whenSaved() { return savePromise; }, diff --git a/src/storages/types.ts b/src/storages/types.ts index d91a0cab..c59950d7 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -11,11 +11,11 @@ import { ISettings } from '../types'; export interface StorageAdapter { // Methods to support async storages load?: () => Promise; - save?: () => Promise; + whenSaved?: () => Promise; // Methods based on https://developer.mozilla.org/en-US/docs/Web/API/Storage readonly length: number; - getItem(key: string): string | null; key(index: number): string | null; + getItem(key: string): string | null; removeItem(key: string): void; setItem(key: string, value: string): void; } From c594a6a223ab0c9a27bf8ee96028972f7ad2a04d Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 4 Aug 2025 15:47:27 -0300 Subject: [PATCH 10/14] optimize StorageAdapter using an internal keys array and cache object --- src/storages/inLocalStorage/storageAdapter.ts | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/storages/inLocalStorage/storageAdapter.ts b/src/storages/inLocalStorage/storageAdapter.ts index 9eec6ec1..5e068590 100644 --- a/src/storages/inLocalStorage/storageAdapter.ts +++ b/src/storages/inLocalStorage/storageAdapter.ts @@ -9,17 +9,13 @@ function isTillKey(key: string) { export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.StorageWrapper): Required { let keys: string[] = []; - let values: string[] = []; + let cache: Record = {}; let loadPromise: Promise | undefined; let savePromise = Promise.resolve(); function _save() { return savePromise = savePromise.then(() => { - const cache = keys.reduce((acc, key, index) => { - acc[key] = values[index]; - return acc; - }, {} as Record); return Promise.resolve(wrapper.setItem(prefix, JSON.stringify(cache))); }).catch((e) => { log.error(LOG_PREFIX + 'Rejected promise calling wrapper `setItem` method, with error: ' + e); @@ -31,9 +27,8 @@ export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.St return loadPromise || (loadPromise = Promise.resolve().then(() => { return wrapper.getItem(prefix); }).then((storedCache) => { - const cache = JSON.parse(storedCache || '{}'); + cache = JSON.parse(storedCache || '{}'); keys = Object.keys(cache); - values = keys.map(key => cache[key]); }).catch((e) => { log.error(LOG_PREFIX + 'Rejected promise calling wrapper `getItem` method, with error: ' + e); })); @@ -46,9 +41,7 @@ export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.St return keys.length; }, getItem(key: string) { - const index = keys.indexOf(key); - if (index === -1) return null; - return values[index]; + return cache[key] || null; }, key(index: number) { return keys[index] || null; @@ -57,14 +50,13 @@ export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.St const index = keys.indexOf(key); if (index === -1) return; keys.splice(index, 1); - values.splice(index, 1); + delete cache[key]; if (isTillKey(key)) _save(); }, setItem(key: string, value: string) { - let index = keys.indexOf(key); - if (index === -1) index = keys.push(key) - 1; - values[index] = value; + if (keys.indexOf(key) === -1) keys.push(key); + cache[key] = value; if (isTillKey(key)) _save(); } From a1c091bef65b8b12caeec047126ce46aa387a4b1 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 4 Aug 2025 15:59:10 -0300 Subject: [PATCH 11/14] refactor: split StorageWrapper into sync and async interfaces for better type safety --- .../inLocalStorage/__tests__/wrapper.mock.ts | 2 +- src/storages/inLocalStorage/index.ts | 2 +- src/storages/inLocalStorage/storageAdapter.ts | 2 +- types/splitio.d.ts | 33 ++++++++++++++----- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/storages/inLocalStorage/__tests__/wrapper.mock.ts b/src/storages/inLocalStorage/__tests__/wrapper.mock.ts index 9cbcbf02..897c13a0 100644 --- a/src/storages/inLocalStorage/__tests__/wrapper.mock.ts +++ b/src/storages/inLocalStorage/__tests__/wrapper.mock.ts @@ -4,7 +4,7 @@ import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock'; export const PREFIX = 'SPLITIO'; -export function createMemoryStorage(): SplitIO.StorageWrapper { +export function createMemoryStorage(): SplitIO.AsyncStorageWrapper { let cache: Record = {}; return { getItem(key: string) { diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index 9cf1ae0c..ed9bb1a2 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -19,7 +19,7 @@ import { ILogger } from '../../logger/types'; import SplitIO from '../../../types/splitio'; import { storageAdapter } from './storageAdapter'; -function validateStorage(log: ILogger, prefix: string, wrapper?: SplitIO.StorageWrapper): StorageAdapter | undefined { +function validateStorage(log: ILogger, prefix: string, wrapper?: SplitIO.SyncStorageWrapper | SplitIO.AsyncStorageWrapper): StorageAdapter | undefined { if (wrapper) { if (isValidStorageWrapper(wrapper)) return storageAdapter(log, prefix, wrapper); log.warn(LOG_PREFIX + 'Invalid storage provided. Falling back to LocalStorage API'); diff --git a/src/storages/inLocalStorage/storageAdapter.ts b/src/storages/inLocalStorage/storageAdapter.ts index 5e068590..7c508de1 100644 --- a/src/storages/inLocalStorage/storageAdapter.ts +++ b/src/storages/inLocalStorage/storageAdapter.ts @@ -7,7 +7,7 @@ function isTillKey(key: string) { return key.endsWith('.till'); } -export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.StorageWrapper): Required { +export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.SyncStorageWrapper | SplitIO.AsyncStorageWrapper): Required { let keys: string[] = []; let cache: Record = {}; diff --git a/types/splitio.d.ts b/types/splitio.d.ts index ed832258..93635db4 100644 --- a/types/splitio.d.ts +++ b/types/splitio.d.ts @@ -458,19 +458,34 @@ interface IClientSideSyncSharedSettings extends IClientSideSharedSettings, ISync */ declare namespace SplitIO { - interface StorageWrapper { + interface SyncStorageWrapper { /** - * Returns a promise that resolves to the current value associated with the given key, or null if the given key does not exist. + * Returns the value associated with the given key, or null if the key does not exist. */ - getItem(key: string): Promise | string | null; + getItem(key: string): string | null; /** - * Returns a promise that resolves when the value of the pair identified by key is set to value, creating a new key/value pair if none existed for key previously. + * Sets the value for the given key, creating a new key/value pair if key does not exist. */ - setItem(key: string, value: string): Promise | void; + setItem(key: string, value: string): void; /** - * Returns a promise that resolves when the key/value pair with the given key is removed, if a key/value pair with the given key exists. + * Removes the key/value pair for the given key, if the key exists. */ - removeItem(key: string): Promise | void; + removeItem(key: string): void; + } + + interface AsyncStorageWrapper { + /** + * Returns a promise that resolves to the value associated with the given key, or null if the key does not exist. + */ + getItem(key: string): Promise; + /** + * Returns a promise that resolves when the value of the pair identified by key is set to value, creating a new key/value pair if key does not exist. + */ + setItem(key: string, value: string): Promise; + /** + * Returns a promise that resolves when the key/value pair for the given key is removed, if the key exists. + */ + removeItem(key: string): Promise; } /** @@ -992,7 +1007,7 @@ declare namespace SplitIO { * * @defaultValue `window.localStorage` */ - wrapper?: StorageWrapper; + wrapper?: SyncStorageWrapper | AsyncStorageWrapper; } /** * Storage for asynchronous (consumer) SDK. @@ -1338,7 +1353,7 @@ declare namespace SplitIO { * * @defaultValue `window.localStorage` */ - wrapper?: StorageWrapper; + wrapper?: SyncStorageWrapper | AsyncStorageWrapper; }; } /** From 1c09903f8acd6a86bbe8d2d1a62c08910303aac9 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 11 Aug 2025 20:32:25 -0300 Subject: [PATCH 12/14] Revert commit 70ea6d424d08928e81b496ce6ffed3e262b19cf6. --- src/storages/AbstractMySegmentsCacheSync.ts | 46 ++++++++----------- src/storages/AbstractSplitsCacheSync.ts | 5 +- .../inLocalStorage/RBSegmentsCacheInLocal.ts | 5 +- 3 files changed, 24 insertions(+), 32 deletions(-) diff --git a/src/storages/AbstractMySegmentsCacheSync.ts b/src/storages/AbstractMySegmentsCacheSync.ts index 5b72aaf9..7d3dc304 100644 --- a/src/storages/AbstractMySegmentsCacheSync.ts +++ b/src/storages/AbstractMySegmentsCacheSync.ts @@ -49,10 +49,12 @@ export abstract class AbstractMySegmentsCacheSync implements ISegmentsCacheSync * For client-side synchronizer: it resets or updates the cache. */ resetSegments(segmentsData: MySegmentsData | IMySegmentsResponse): boolean { + this.setChangeNumber(segmentsData.cn); + const { added, removed } = segmentsData as MySegmentsData; - let isDiff = false; if (added && removed) { + let isDiff = false; added.forEach(segment => { isDiff = this.addSegment(segment) || isDiff; @@ -61,40 +63,32 @@ export abstract class AbstractMySegmentsCacheSync implements ISegmentsCacheSync removed.forEach(segment => { isDiff = this.removeSegment(segment) || isDiff; }); - } else { - const names = ((segmentsData as IMySegmentsResponse).k || []).map(s => s.n).sort(); - const storedSegmentKeys = this.getRegisteredSegments().sort(); + return isDiff; + } - // Extreme fast => everything is empty - if (!names.length && !storedSegmentKeys.length) { - isDiff = false; - } else { + const names = ((segmentsData as IMySegmentsResponse).k || []).map(s => s.n).sort(); + const storedSegmentKeys = this.getRegisteredSegments().sort(); - let index = 0; + // Extreme fast => everything is empty + if (!names.length && !storedSegmentKeys.length) return false; - while (index < names.length && index < storedSegmentKeys.length && names[index] === storedSegmentKeys[index]) index++; + let index = 0; - // Quick path => no changes - if (index === names.length && index === storedSegmentKeys.length) { - isDiff = false; - } else { + while (index < names.length && index < storedSegmentKeys.length && names[index] === storedSegmentKeys[index]) index++; - // Slowest path => add and/or remove segments - for (let removeIndex = index; removeIndex < storedSegmentKeys.length; removeIndex++) { - this.removeSegment(storedSegmentKeys[removeIndex]); - } + // Quick path => no changes + if (index === names.length && index === storedSegmentKeys.length) return false; - for (let addIndex = index; addIndex < names.length; addIndex++) { - this.addSegment(names[addIndex]); - } + // Slowest path => add and/or remove segments + for (let removeIndex = index; removeIndex < storedSegmentKeys.length; removeIndex++) { + this.removeSegment(storedSegmentKeys[removeIndex]); + } - isDiff = true; - } - } + for (let addIndex = index; addIndex < names.length; addIndex++) { + this.addSegment(names[addIndex]); } - this.setChangeNumber(segmentsData.cn); - return isDiff; + return true; } } diff --git a/src/storages/AbstractSplitsCacheSync.ts b/src/storages/AbstractSplitsCacheSync.ts index 64194561..2a4b9b78 100644 --- a/src/storages/AbstractSplitsCacheSync.ts +++ b/src/storages/AbstractSplitsCacheSync.ts @@ -14,10 +14,9 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync { protected abstract setChangeNumber(changeNumber: number): boolean | void update(toAdd: ISplit[], toRemove: ISplit[], changeNumber: number): boolean { - let updated = toAdd.map(addedFF => this.addSplit(addedFF)).some(result => result); - updated = toRemove.map(removedFF => this.removeSplit(removedFF.name)).some(result => result) || updated; this.setChangeNumber(changeNumber); - return updated; + const updated = toAdd.map(addedFF => this.addSplit(addedFF)).some(result => result); + return toRemove.map(removedFF => this.removeSplit(removedFF.name)).some(result => result) || updated; } abstract getSplit(name: string): ISplit | null diff --git a/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts b/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts index b0b2aba5..312787bc 100644 --- a/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts @@ -26,10 +26,9 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync { } update(toAdd: IRBSegment[], toRemove: IRBSegment[], changeNumber: number): boolean { - let updated = toAdd.map(toAdd => this.add(toAdd)).some(result => result); - updated = toRemove.map(toRemove => this.remove(toRemove.name)).some(result => result) || updated; this.setChangeNumber(changeNumber); - return updated; + const updated = toAdd.map(toAdd => this.add(toAdd)).some(result => result); + return toRemove.map(toRemove => this.remove(toRemove.name)).some(result => result) || updated; } private setChangeNumber(changeNumber: number) { From afdf656e30fd1af162450b9ac46031a90f0935f9 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 11 Aug 2025 20:34:45 -0300 Subject: [PATCH 13/14] Add and use StorageAdapter::save method --- .../inLocalStorage/MySegmentsCacheInLocal.ts | 8 ++++++ .../inLocalStorage/RBSegmentsCacheInLocal.ts | 6 ++-- .../inLocalStorage/SplitsCacheInLocal.ts | 6 ++++ .../__tests__/storageAdapter.spec.ts | 13 ++++----- src/storages/inLocalStorage/storageAdapter.ts | 28 +++++++++---------- src/storages/inLocalStorage/validateCache.ts | 3 ++ src/storages/types.ts | 1 + 7 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts index fd038a07..361ca61f 100644 --- a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts @@ -4,6 +4,8 @@ import { AbstractMySegmentsCacheSync } from '../AbstractMySegmentsCacheSync'; import type { MySegmentsKeyBuilder } from '../KeyBuilderCS'; import { LOG_PREFIX, DEFINED } from './constants'; import { StorageAdapter } from '../types'; +import { IMySegmentsResponse } from '../../dtos/types'; +import { MySegmentsData } from '../../sync/polling/types'; export class MySegmentsCacheInLocal extends AbstractMySegmentsCacheSync { @@ -19,6 +21,12 @@ export class MySegmentsCacheInLocal extends AbstractMySegmentsCacheSync { // There is not need to flush segments cache like splits cache, since resetSegments receives the up-to-date list of active segments } + resetSegments(segmentsData: MySegmentsData | IMySegmentsResponse): boolean { + const result = super.resetSegments(segmentsData); + if (this.storage.save) this.storage.save(); + return result; + } + protected addSegment(name: string): boolean { const segmentKey = this.keys.buildSegmentNameKey(name); diff --git a/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts b/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts index 312787bc..c22d790e 100644 --- a/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts @@ -27,8 +27,10 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync { update(toAdd: IRBSegment[], toRemove: IRBSegment[], changeNumber: number): boolean { this.setChangeNumber(changeNumber); - const updated = toAdd.map(toAdd => this.add(toAdd)).some(result => result); - return toRemove.map(toRemove => this.remove(toRemove.name)).some(result => result) || updated; + let updated = toAdd.map(toAdd => this.add(toAdd)).some(result => result); + updated = toRemove.map(toRemove => this.remove(toRemove.name)).some(result => result) || updated; + if (this.storage.save) this.storage.save(); + return updated; } private setChangeNumber(changeNumber: number) { diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index 3aa08452..25fa21a8 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -79,6 +79,12 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { this.hasSync = false; } + update(toAdd: ISplit[], toRemove: ISplit[], changeNumber: number): boolean { + const result = super.update(toAdd, toRemove, changeNumber); + if (this.storage.save) this.storage.save(); + return result; + } + addSplit(split: ISplit) { try { const name = split.name; diff --git a/src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts b/src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts index 0112e592..84be924a 100644 --- a/src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts +++ b/src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts @@ -41,25 +41,22 @@ test.each([ expect(storage.getItem('key1')).toBe(null); expect(storage.length).toBe(1); - // Until a till key is set/removed, changes should not be saved/persisted + // Until `save` is called, changes should not be saved/persisted await storage.whenSaved(); expect(wrapper.setItem).not.toHaveBeenCalled(); - // When setting a till key, changes should be saved/persisted immediately storage.setItem('.till', '1'); expect(storage.length).toBe(2); expect(storage.key(0)).toBe('key2'); expect(storage.key(1)).toBe('.till'); + // When `save` is called, changes should be saved/persisted immediately + storage.save(); await storage.whenSaved(); expect(wrapper.setItem).toHaveBeenCalledWith('prefix', JSON.stringify({ key2: 'value2', '.till': '1' })); - // When removing a till key, changes should be saved/persisted immediately - storage.removeItem('.till'); + expect(wrapper.setItem).toHaveBeenCalledTimes(1); await storage.whenSaved(); - expect(wrapper.setItem).toHaveBeenCalledWith('prefix', JSON.stringify({ key2: 'value2' })); - - await storage.whenSaved(); - expect(wrapper.setItem).toHaveBeenCalledTimes(2); + expect(wrapper.setItem).toHaveBeenCalledTimes(1); }); diff --git a/src/storages/inLocalStorage/storageAdapter.ts b/src/storages/inLocalStorage/storageAdapter.ts index 7c508de1..af92df22 100644 --- a/src/storages/inLocalStorage/storageAdapter.ts +++ b/src/storages/inLocalStorage/storageAdapter.ts @@ -3,9 +3,6 @@ import SplitIO from '../../../types/splitio'; import { LOG_PREFIX } from './constants'; import { StorageAdapter } from '../types'; -function isTillKey(key: string) { - return key.endsWith('.till'); -} export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.SyncStorageWrapper | SplitIO.AsyncStorageWrapper): Required { let keys: string[] = []; @@ -14,14 +11,6 @@ export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.Sy let loadPromise: Promise | undefined; let savePromise = Promise.resolve(); - function _save() { - return savePromise = savePromise.then(() => { - return Promise.resolve(wrapper.setItem(prefix, JSON.stringify(cache))); - }).catch((e) => { - log.error(LOG_PREFIX + 'Rejected promise calling wrapper `setItem` method, with error: ' + e); - }); - } - return { load() { return loadPromise || (loadPromise = Promise.resolve().then(() => { @@ -33,6 +22,15 @@ export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.Sy log.error(LOG_PREFIX + 'Rejected promise calling wrapper `getItem` method, with error: ' + e); })); }, + + save() { + return savePromise = savePromise.then(() => { + return Promise.resolve(wrapper.setItem(prefix, JSON.stringify(cache))); + }).catch((e) => { + log.error(LOG_PREFIX + 'Rejected promise calling wrapper `setItem` method, with error: ' + e); + }); + }, + whenSaved() { return savePromise; }, @@ -40,25 +38,25 @@ export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.Sy get length() { return keys.length; }, + getItem(key: string) { return cache[key] || null; }, + key(index: number) { return keys[index] || null; }, + removeItem(key: string) { const index = keys.indexOf(key); if (index === -1) return; keys.splice(index, 1); delete cache[key]; - - if (isTillKey(key)) _save(); }, + setItem(key: string, value: string) { if (keys.indexOf(key) === -1) keys.push(key); cache[key] = value; - - if (isTillKey(key)) _save(); } }; } diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts index 9973a76e..38df9899 100644 --- a/src/storages/inLocalStorage/validateCache.ts +++ b/src/storages/inLocalStorage/validateCache.ts @@ -87,6 +87,9 @@ export function validateCache(options: SplitIO.InLocalStorageOptions, storage: S settings.log.error(LOG_PREFIX + e); } + // Persist clear + if (storage.save) storage.save(); + return false; } diff --git a/src/storages/types.ts b/src/storages/types.ts index c59950d7..54533169 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -11,6 +11,7 @@ import { ISettings } from '../types'; export interface StorageAdapter { // Methods to support async storages load?: () => Promise; + save?: () => Promise; whenSaved?: () => Promise; // Methods based on https://developer.mozilla.org/en-US/docs/Web/API/Storage readonly length: number; From 1cc876b4091971f7e5c8be3665d67f29b35e6c1a Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Tue, 12 Aug 2025 12:31:06 -0300 Subject: [PATCH 14/14] refactor: call storage::save in updaters and remove from cache classes --- src/storages/inLocalStorage/MySegmentsCacheInLocal.ts | 8 -------- src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts | 6 ++---- src/storages/inLocalStorage/SplitsCacheInLocal.ts | 6 ------ src/storages/inLocalStorage/index.ts | 4 ++++ src/storages/types.ts | 1 + src/sync/polling/updaters/mySegmentsUpdater.ts | 2 ++ src/sync/polling/updaters/splitChangesUpdater.ts | 4 +++- 7 files changed, 12 insertions(+), 19 deletions(-) diff --git a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts index 361ca61f..fd038a07 100644 --- a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts @@ -4,8 +4,6 @@ import { AbstractMySegmentsCacheSync } from '../AbstractMySegmentsCacheSync'; import type { MySegmentsKeyBuilder } from '../KeyBuilderCS'; import { LOG_PREFIX, DEFINED } from './constants'; import { StorageAdapter } from '../types'; -import { IMySegmentsResponse } from '../../dtos/types'; -import { MySegmentsData } from '../../sync/polling/types'; export class MySegmentsCacheInLocal extends AbstractMySegmentsCacheSync { @@ -21,12 +19,6 @@ export class MySegmentsCacheInLocal extends AbstractMySegmentsCacheSync { // There is not need to flush segments cache like splits cache, since resetSegments receives the up-to-date list of active segments } - resetSegments(segmentsData: MySegmentsData | IMySegmentsResponse): boolean { - const result = super.resetSegments(segmentsData); - if (this.storage.save) this.storage.save(); - return result; - } - protected addSegment(name: string): boolean { const segmentKey = this.keys.buildSegmentNameKey(name); diff --git a/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts b/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts index c22d790e..312787bc 100644 --- a/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts @@ -27,10 +27,8 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync { update(toAdd: IRBSegment[], toRemove: IRBSegment[], changeNumber: number): boolean { this.setChangeNumber(changeNumber); - let updated = toAdd.map(toAdd => this.add(toAdd)).some(result => result); - updated = toRemove.map(toRemove => this.remove(toRemove.name)).some(result => result) || updated; - if (this.storage.save) this.storage.save(); - return updated; + const updated = toAdd.map(toAdd => this.add(toAdd)).some(result => result); + return toRemove.map(toRemove => this.remove(toRemove.name)).some(result => result) || updated; } private setChangeNumber(changeNumber: number) { diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index 25fa21a8..3aa08452 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -79,12 +79,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { this.hasSync = false; } - update(toAdd: ISplit[], toRemove: ISplit[], changeNumber: number): boolean { - const result = super.update(toAdd, toRemove, changeNumber); - if (this.storage.save) this.storage.save(); - return result; - } - addSplit(split: ISplit) { try { const name = split.name; diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index ed9bb1a2..ae77bb41 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -67,6 +67,10 @@ export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): ISt return validateCachePromise || (validateCachePromise = validateCache(options, storage, settings, keys, splits, rbSegments, segments, largeSegments)); }, + save() { + return storage.save && storage.save(); + }, + destroy() { return storage.whenSaved && storage.whenSaved(); }, diff --git a/src/storages/types.ts b/src/storages/types.ts index 54533169..307fee3e 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -483,6 +483,7 @@ export interface IStorageBase< uniqueKeys: TUniqueKeysCache, destroy(): void | Promise, shared?: (matchingKey: string, onReadyCb: (error?: any) => void) => this + save?: () => void | Promise, } export interface IStorageSync extends IStorageBase< diff --git a/src/sync/polling/updaters/mySegmentsUpdater.ts b/src/sync/polling/updaters/mySegmentsUpdater.ts index 5de512fa..4b6038c5 100644 --- a/src/sync/polling/updaters/mySegmentsUpdater.ts +++ b/src/sync/polling/updaters/mySegmentsUpdater.ts @@ -51,6 +51,8 @@ export function mySegmentsUpdaterFactory( shouldNotifyUpdate = largeSegments!.resetSegments((segmentsData as IMembershipsResponse).ls || {}) || shouldNotifyUpdate; } + if (storage.save) storage.save(); + // Notify update if required if (usesSegmentsSync(storage) && (shouldNotifyUpdate || readyOnAlreadyExistentState)) { readyOnAlreadyExistentState = false; diff --git a/src/sync/polling/updaters/splitChangesUpdater.ts b/src/sync/polling/updaters/splitChangesUpdater.ts index 0331bc43..6c6371e3 100644 --- a/src/sync/polling/updaters/splitChangesUpdater.ts +++ b/src/sync/polling/updaters/splitChangesUpdater.ts @@ -117,7 +117,7 @@ export function computeMutation(rules: Array, export function splitChangesUpdaterFactory( log: ILogger, splitChangesFetcher: ISplitChangesFetcher, - storage: Pick, + storage: Pick, splitFiltersValidation: ISplitFiltersValidation, splitsEventEmitter?: ISplitsEventEmitter, requestTimeoutBeforeReady: number = 0, @@ -185,6 +185,8 @@ export function splitChangesUpdaterFactory( // @TODO if at least 1 segment fetch fails due to 404 and other segments are updated in the storage, SDK_UPDATE is not emitted segments.registerSegments(setToArray(usedSegments)) ]).then(([ffChanged, rbsChanged]) => { + if (storage.save) storage.save(); + if (splitsEventEmitter) { // To emit SDK_SPLITS_ARRIVED for server-side SDK, we must check that all registered segments have been fetched return Promise.resolve(!splitsEventEmitter.splitsArrived || ((ffChanged || rbsChanged) && (isClientSide || checkAllSegmentsExist(segments))))