diff --git a/CHANGES.txt b/CHANGES.txt index 29391a11..17789c07 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,9 @@ 2.2.0 (March 28, 2025) - Added new optional argument to the client `getTreatment` methods to allow passing additional evaluation options, such as a map of properties to append to the generated impression object sent to Split backend. + - Added two new configuration options for the SDK storage in browsers when using storage type `LOCALSTORAGE`: + - `storage.expirationDays` to specify the validity period of the rollout cache. + - `storage.clearOnInit` to clear the rollout cache on SDK initialization. + - Updated SDK_READY_FROM_CACHE event when using the `LOCALSTORAGE` storage type to be emitted alongside the SDK_READY event if it has not already been emitted. 2.1.0 (January 17, 2025) - Added support for the new impressions tracking toggle available on feature flags, both respecting the setting and including the new field being returned on `SplitView` type objects. Read more in our docs. diff --git a/src/readiness/__tests__/readinessManager.spec.ts b/src/readiness/__tests__/readinessManager.spec.ts index 9e2cf34a..174f1373 100644 --- a/src/readiness/__tests__/readinessManager.spec.ts +++ b/src/readiness/__tests__/readinessManager.spec.ts @@ -3,10 +3,14 @@ import { EventEmitter } from '../../utils/MinEvents'; import { IReadinessManager } from '../types'; import { SDK_READY, SDK_UPDATE, SDK_SPLITS_ARRIVED, SDK_SEGMENTS_ARRIVED, SDK_READY_FROM_CACHE, SDK_SPLITS_CACHE_LOADED, SDK_READY_TIMED_OUT } from '../constants'; import { ISettings } from '../../types'; +import { STORAGE_LOCALSTORAGE } from '../../utils/constants'; const settings = { startup: { readyTimeout: 0, + }, + storage: { + type: STORAGE_LOCALSTORAGE } } as unknown as ISettings; @@ -67,7 +71,14 @@ test('READINESS MANAGER / Ready event should be fired once', () => { const readinessManager = readinessManagerFactory(EventEmitter, settings); let counter = 0; + readinessManager.gate.on(SDK_READY_FROM_CACHE, () => { + expect(readinessManager.isReadyFromCache()).toBe(true); + expect(readinessManager.isReady()).toBe(true); + counter++; + }); + readinessManager.gate.on(SDK_READY, () => { + expect(readinessManager.isReadyFromCache()).toBe(true); expect(readinessManager.isReady()).toBe(true); counter++; }); @@ -79,7 +90,7 @@ test('READINESS MANAGER / Ready event should be fired once', () => { readinessManager.splits.emit(SDK_SPLITS_ARRIVED); readinessManager.segments.emit(SDK_SEGMENTS_ARRIVED); - expect(counter).toBe(1); // should be called once + expect(counter).toBe(2); // should be called once }); test('READINESS MANAGER / Ready from cache event should be fired once', (done) => { @@ -88,6 +99,7 @@ test('READINESS MANAGER / Ready from cache event should be fired once', (done) = readinessManager.gate.on(SDK_READY_FROM_CACHE, () => { expect(readinessManager.isReadyFromCache()).toBe(true); + expect(readinessManager.isReady()).toBe(false); counter++; }); diff --git a/src/readiness/readinessManager.ts b/src/readiness/readinessManager.ts index 6f46474d..c69eedce 100644 --- a/src/readiness/readinessManager.ts +++ b/src/readiness/readinessManager.ts @@ -3,6 +3,7 @@ import { ISettings } from '../types'; import SplitIO from '../../types/splitio'; import { SDK_SPLITS_ARRIVED, SDK_SPLITS_CACHE_LOADED, SDK_SEGMENTS_ARRIVED, SDK_READY_TIMED_OUT, SDK_READY_FROM_CACHE, SDK_UPDATE, SDK_READY } from './constants'; import { IReadinessEventEmitter, IReadinessManager, ISegmentsEventEmitter, ISplitsEventEmitter } from './types'; +import { STORAGE_LOCALSTORAGE } from '../utils/constants'; function splitsEventEmitterFactory(EventEmitter: new () => SplitIO.IEventEmitter): ISplitsEventEmitter { const splitsEventEmitter = objectAssign(new EventEmitter(), { @@ -114,6 +115,10 @@ export function readinessManagerFactory( isReady = true; try { syncLastUpdate(); + if (!isReadyFromCache && settings.storage?.type === STORAGE_LOCALSTORAGE) { + isReadyFromCache = true; + gate.emit(SDK_READY_FROM_CACHE); + } gate.emit(SDK_READY); } catch (e) { // throws user callback exceptions in next tick diff --git a/src/storages/AbstractSplitsCacheAsync.ts b/src/storages/AbstractSplitsCacheAsync.ts index a43a57f2..420b9202 100644 --- a/src/storages/AbstractSplitsCacheAsync.ts +++ b/src/storages/AbstractSplitsCacheAsync.ts @@ -37,14 +37,6 @@ export abstract class AbstractSplitsCacheAsync implements ISplitsCacheAsync { return Promise.resolve(true); } - /** - * Check if the splits information is already stored in cache. - * Noop, just keeping the interface. This is used by client-side implementations only. - */ - checkCache(): Promise { - return Promise.resolve(false); - } - /** * Kill `name` split and set `defaultTreatment` and `changeNumber`. * Used for SPLIT_KILL push notifications. diff --git a/src/storages/AbstractSplitsCacheSync.ts b/src/storages/AbstractSplitsCacheSync.ts index 512d990e..483deb42 100644 --- a/src/storages/AbstractSplitsCacheSync.ts +++ b/src/storages/AbstractSplitsCacheSync.ts @@ -43,14 +43,6 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync { abstract clear(): void - /** - * Check if the splits information is already stored in cache. This data can be preloaded. - * It is used as condition to emit SDK_SPLITS_CACHE_LOADED, and then SDK_READY_FROM_CACHE. - */ - checkCache(): boolean { - return false; - } - /** * Kill `name` split and set `defaultTreatment` and `changeNumber`. * Used for SPLIT_KILL push notifications. diff --git a/src/storages/KeyBuilderCS.ts b/src/storages/KeyBuilderCS.ts index 23961f89..d3404ed1 100644 --- a/src/storages/KeyBuilderCS.ts +++ b/src/storages/KeyBuilderCS.ts @@ -50,6 +50,10 @@ export class KeyBuilderCS extends KeyBuilder implements MySegmentsKeyBuilder { buildSplitsWithSegmentCountKey() { return `${this.prefix}.splits.usingSegments`; } + + buildLastClear() { + return `${this.prefix}.lastClear`; + } } export function myLargeSegmentsKeyBuilder(prefix: string, matchingKey: string): MySegmentsKeyBuilder { diff --git a/src/storages/dataLoader.ts b/src/storages/dataLoader.ts index 2ab542a9..49522bce 100644 --- a/src/storages/dataLoader.ts +++ b/src/storages/dataLoader.ts @@ -1,7 +1,9 @@ import { PreloadedData } from '../types'; -import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../utils/constants/browser'; import { DataLoader, ISegmentsCacheSync, ISplitsCacheSync } from './types'; +// This value might be eventually set via a config parameter +const DEFAULT_CACHE_EXPIRATION_IN_MILLIS = 864000000; // 10 days + /** * Factory of client-side storage loader * diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index 61988139..82be56cd 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -5,7 +5,6 @@ import { KeyBuilderCS } from '../KeyBuilderCS'; import { ILogger } from '../../logger/types'; import { LOG_PREFIX } from './constants'; import { ISettings } from '../../types'; -import { getStorageHash } from '../KeyBuilder'; import { setToArray } from '../../utils/lang/sets'; /** @@ -15,21 +14,14 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { private readonly keys: KeyBuilderCS; private readonly log: ILogger; - private readonly storageHash: string; private readonly flagSetsFilter: string[]; private hasSync?: boolean; - private updateNewFilter?: boolean; - constructor(settings: ISettings, keys: KeyBuilderCS, expirationTimestamp?: number) { + constructor(settings: ISettings, keys: KeyBuilderCS) { super(); this.keys = keys; this.log = settings.log; - this.storageHash = getStorageHash(settings); this.flagSetsFilter = settings.sync.__splitFiltersValidation.groupedFilters.bySet; - - this._checkExpiration(expirationTimestamp); - - this._checkFilterQuery(); } private _decrementCount(key: string) { @@ -79,8 +71,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { * We cannot simply call `localStorage.clear()` since that implies removing user items from the storage. */ clear() { - this.log.info(LOG_PREFIX + 'Flushing Splits data from localStorage'); - // collect item keys const len = localStorage.length; const accum = []; @@ -141,19 +131,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { } setChangeNumber(changeNumber: number): boolean { - - // when using a new split query, we must update it at the store - if (this.updateNewFilter) { - this.log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version was modified. Updating cache'); - const storageHashKey = this.keys.buildHashKey(); - try { - localStorage.setItem(storageHashKey, this.storageHash); - } catch (e) { - this.log.error(LOG_PREFIX + e); - } - this.updateNewFilter = false; - } - try { localStorage.setItem(this.keys.buildSplitsTillKey(), changeNumber + ''); // update "last updated" timestamp with current time @@ -215,48 +192,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { } } - /** - * Check if the splits information is already stored in browser LocalStorage. - * In this function we could add more code to check if the data is valid. - * @override - */ - checkCache(): boolean { - return this.getChangeNumber() > -1; - } - - /** - * Clean Splits cache if its `lastUpdated` timestamp is older than the given `expirationTimestamp`, - * - * @param expirationTimestamp - if the value is not a number, data will not be cleaned - */ - private _checkExpiration(expirationTimestamp?: number) { - let value: string | number | null = localStorage.getItem(this.keys.buildLastUpdatedKey()); - if (value !== null) { - value = parseInt(value, 10); - if (!isNaNNumber(value) && expirationTimestamp && value < expirationTimestamp) this.clear(); - } - } - - // @TODO eventually remove `_checkFilterQuery`. Cache should be cleared at the storage level, reusing same logic than PluggableStorage - private _checkFilterQuery() { - const storageHashKey = this.keys.buildHashKey(); - const storageHash = localStorage.getItem(storageHashKey); - - if (storageHash !== this.storageHash) { - try { - // mark cache to update the new query filter on first successful splits fetch - this.updateNewFilter = true; - - // if there is cache, clear it - if (this.checkCache()) this.clear(); - - } catch (e) { - this.log.error(LOG_PREFIX + e); - } - } - // if the filter didn't change, nothing is done - } - getNamesByFlagSets(flagSets: string[]): Set[] { return flagSets.map(flagSet => { const flagSetKey = this.keys.buildFlagSetKey(flagSet); diff --git a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts index db78c741..913d6a3b 100644 --- a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts +++ b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts @@ -29,18 +29,11 @@ test('SPLITS CACHE / LocalStorage', () => { expect(cache.getSplit(something.name)).toEqual(null); expect(cache.getSplit(somethingElse.name)).toEqual(somethingElse); - expect(cache.checkCache()).toBe(false); // checkCache should return false until localstorage has data. - expect(cache.getChangeNumber()).toBe(-1); - expect(cache.checkCache()).toBe(false); // checkCache should return false until localstorage has data. - cache.setChangeNumber(123); - expect(cache.checkCache()).toBe(true); // checkCache should return true once localstorage has data. - expect(cache.getChangeNumber()).toBe(123); - }); test('SPLITS CACHE / LocalStorage / Get Keys', () => { @@ -106,6 +99,7 @@ test('SPLITS CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => test('SPLITS CACHE / LocalStorage / killLocally', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); + cache.addSplit(something); cache.addSplit(somethingElse); const initialChangeNumber = cache.getChangeNumber(); @@ -169,6 +163,7 @@ test('SPLITS CACHE / LocalStorage / flag set cache tests', () => { } } }, new KeyBuilderCS('SPLITIO', 'user')); + const emptySet = new Set([]); cache.update([ @@ -208,25 +203,25 @@ test('SPLITS CACHE / LocalStorage / flag set cache tests', () => { // if FlagSets are not defined, it should store all FlagSets in memory. test('SPLIT CACHE / LocalStorage / flag set cache tests without filters', () => { - const cacheWithoutFilters = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); + const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); + const emptySet = new Set([]); - cacheWithoutFilters.update([ + cache.update([ featureFlagOne, featureFlagTwo, featureFlagThree, ], [], -1); - cacheWithoutFilters.addSplit(featureFlagWithEmptyFS); + cache.addSplit(featureFlagWithEmptyFS); - expect(cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]); - expect(cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]); - expect(cacheWithoutFilters.getNamesByFlagSets(['e'])).toEqual([new Set(['ff_one', 'ff_three'])]); - expect(cacheWithoutFilters.getNamesByFlagSets(['t'])).toEqual([new Set(['ff_two', 'ff_three'])]); - expect(cacheWithoutFilters.getNamesByFlagSets(['y'])).toEqual([emptySet]); - expect(cacheWithoutFilters.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new Set(['ff_one', 'ff_two']), new Set(['ff_one']), new Set(['ff_one', 'ff_three'])]); + expect(cache.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]); + expect(cache.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]); + expect(cache.getNamesByFlagSets(['e'])).toEqual([new Set(['ff_one', 'ff_three'])]); + expect(cache.getNamesByFlagSets(['t'])).toEqual([new Set(['ff_two', 'ff_three'])]); + expect(cache.getNamesByFlagSets(['y'])).toEqual([emptySet]); + expect(cache.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new Set(['ff_one', 'ff_two']), new Set(['ff_one']), new Set(['ff_one', 'ff_three'])]); // Validate that the feature flag cache is cleared when calling `clear` method - cacheWithoutFilters.clear(); - expect(localStorage.length).toBe(1); // only 'SPLITIO.hash' should remain in localStorage - expect(localStorage.key(0)).toBe('SPLITIO.hash'); + cache.clear(); + expect(localStorage.length).toBe(0); }); diff --git a/src/storages/inLocalStorage/__tests__/validateCache.spec.ts b/src/storages/inLocalStorage/__tests__/validateCache.spec.ts new file mode 100644 index 00000000..27050a56 --- /dev/null +++ b/src/storages/inLocalStorage/__tests__/validateCache.spec.ts @@ -0,0 +1,125 @@ +import { validateCache } from '../validateCache'; + +import { KeyBuilderCS } from '../../KeyBuilderCS'; +import { fullSettings } from '../../../utils/settingsValidation/__tests__/settings.mocks'; +import { SplitsCacheInLocal } from '../SplitsCacheInLocal'; +import { nearlyEqual } from '../../../__tests__/testUtils'; +import { MySegmentsCacheInLocal } from '../MySegmentsCacheInLocal'; + +const FULL_SETTINGS_HASH = '404832b3'; + +describe('validateCache', () => { + const keys = new KeyBuilderCS('SPLITIO', 'user'); + const logSpy = jest.spyOn(fullSettings.log, 'info'); + const segments = new MySegmentsCacheInLocal(fullSettings.log, keys); + const largeSegments = new MySegmentsCacheInLocal(fullSettings.log, keys); + const splits = new SplitsCacheInLocal(fullSettings, keys); + + jest.spyOn(splits, 'clear'); + jest.spyOn(splits, 'getChangeNumber'); + jest.spyOn(segments, 'clear'); + jest.spyOn(largeSegments, 'clear'); + + beforeEach(() => { + jest.clearAllMocks(); + localStorage.clear(); + }); + + test('if there is no cache, it should return false', () => { + expect(validateCache({}, fullSettings, keys, splits, segments, largeSegments)).toBe(false); + + expect(logSpy).not.toHaveBeenCalled(); + + expect(splits.clear).not.toHaveBeenCalled(); + expect(segments.clear).not.toHaveBeenCalled(); + expect(largeSegments.clear).not.toHaveBeenCalled(); + expect(splits.getChangeNumber).toHaveBeenCalledTimes(1); + + expect(localStorage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH); + expect(localStorage.getItem(keys.buildLastClear())).toBeNull(); + }); + + test('if there is cache and it must not be cleared, it should return true', () => { + localStorage.setItem(keys.buildSplitsTillKey(), '1'); + localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); + + expect(validateCache({}, fullSettings, keys, splits, segments, largeSegments)).toBe(true); + + expect(logSpy).not.toHaveBeenCalled(); + + expect(splits.clear).not.toHaveBeenCalled(); + expect(segments.clear).not.toHaveBeenCalled(); + expect(largeSegments.clear).not.toHaveBeenCalled(); + expect(splits.getChangeNumber).toHaveBeenCalledTimes(1); + + expect(localStorage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH); + expect(localStorage.getItem(keys.buildLastClear())).toBeNull(); + }); + + test('if there is cache and it has expired, it should clear cache and return false', () => { + localStorage.setItem(keys.buildSplitsTillKey(), '1'); + localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); + localStorage.setItem(keys.buildLastUpdatedKey(), Date.now() - 1000 * 60 * 60 * 24 * 2 + ''); // 2 days ago + + expect(validateCache({ expirationDays: 1 }, fullSettings, keys, splits, segments, largeSegments)).toBe(false); + + expect(logSpy).toHaveBeenCalledWith('storage:localstorage: Cache expired more than 1 days ago. Cleaning up cache'); + + expect(splits.clear).toHaveBeenCalledTimes(1); + expect(segments.clear).toHaveBeenCalledTimes(1); + expect(largeSegments.clear).toHaveBeenCalledTimes(1); + + expect(localStorage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH); + expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true); + }); + + test('if there is cache and its hash has changed, it should clear cache and return false', () => { + localStorage.setItem(keys.buildSplitsTillKey(), '1'); + localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); + + expect(validateCache({}, { ...fullSettings, core: { ...fullSettings.core, authorizationKey: 'another' } }, keys, splits, segments, largeSegments)).toBe(false); + + expect(logSpy).toHaveBeenCalledWith('storage:localstorage: SDK key, flags filter criteria, or flags spec version has changed. Cleaning up cache'); + + expect(splits.clear).toHaveBeenCalledTimes(1); + expect(segments.clear).toHaveBeenCalledTimes(1); + expect(largeSegments.clear).toHaveBeenCalledTimes(1); + + expect(localStorage.getItem(keys.buildHashKey())).toBe('aa4877c2'); + expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true); + }); + + test('if there is cache and clearOnInit is true, it should clear cache and return false', () => { + // Older cache version (without last clear) + localStorage.setItem(keys.buildSplitsTillKey(), '1'); + localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH); + + expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, segments, largeSegments)).toBe(false); + + expect(logSpy).toHaveBeenCalledWith('storage:localstorage: clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache'); + + expect(splits.clear).toHaveBeenCalledTimes(1); + expect(segments.clear).toHaveBeenCalledTimes(1); + expect(largeSegments.clear).toHaveBeenCalledTimes(1); + + expect(localStorage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH); + const lastClear = localStorage.getItem(keys.buildLastClear()); + expect(nearlyEqual(parseInt(lastClear as string), Date.now())).toBe(true); + + // If cache is cleared, it should not clear again until a day has passed + logSpy.mockClear(); + localStorage.setItem(keys.buildSplitsTillKey(), '1'); + expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, segments, largeSegments)).toBe(true); + expect(logSpy).not.toHaveBeenCalled(); + expect(localStorage.getItem(keys.buildLastClear())).toBe(lastClear); // Last clear should not have changed + + // If a day has passed, it should clear again + localStorage.setItem(keys.buildLastClear(), (Date.now() - 1000 * 60 * 60 * 24 - 1) + ''); + expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, segments, largeSegments)).toBe(false); + expect(logSpy).toHaveBeenCalledWith('storage:localstorage: clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache'); + expect(splits.clear).toHaveBeenCalledTimes(2); + expect(segments.clear).toHaveBeenCalledTimes(2); + expect(largeSegments.clear).toHaveBeenCalledTimes(2); + expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true); + }); +}); diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts index c621141d..616bb7d7 100644 --- a/src/storages/inLocalStorage/index.ts +++ b/src/storages/inLocalStorage/index.ts @@ -7,22 +7,19 @@ import { KeyBuilderCS, myLargeSegmentsKeyBuilder } from '../KeyBuilderCS'; import { isLocalStorageAvailable } from '../../utils/env/isLocalStorageAvailable'; import { SplitsCacheInLocal } from './SplitsCacheInLocal'; import { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal'; -import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../../utils/constants/browser'; import { InMemoryStorageCSFactory } from '../inMemory/InMemoryStorageCS'; import { LOG_PREFIX } from './constants'; import { STORAGE_LOCALSTORAGE } from '../../utils/constants'; import { shouldRecordTelemetry, TelemetryCacheInMemory } from '../inMemory/TelemetryCacheInMemory'; import { UniqueKeysCacheInMemoryCS } from '../inMemory/UniqueKeysCacheInMemoryCS'; import { getMatching } from '../../utils/key'; - -export interface InLocalStorageOptions { - prefix?: string -} +import { validateCache } from './validateCache'; +import SplitIO from '../../../types/splitio'; /** * InLocal storage factory for standalone client-side SplitFactory */ -export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyncFactory { +export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): IStorageSyncFactory { const prefix = validatePrefix(options.prefix); @@ -37,9 +34,8 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn const { settings, settings: { log, scheduler: { impressionsQueueSize, eventsQueueSize } } } = params; const matchingKey = getMatching(settings.core.key); const keys = new KeyBuilderCS(prefix, matchingKey); - const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS; - const splits = new SplitsCacheInLocal(settings, keys, expirationTimestamp); + const splits = new SplitsCacheInLocal(settings, keys); const segments = new MySegmentsCacheInLocal(log, keys); const largeSegments = new MySegmentsCacheInLocal(log, myLargeSegmentsKeyBuilder(prefix, matchingKey)); @@ -53,6 +49,10 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn telemetry: shouldRecordTelemetry(params) ? new TelemetryCacheInMemory(splits, segments) : undefined, uniqueKeys: new UniqueKeysCacheInMemoryCS(), + validateCache() { + return validateCache(options, settings, keys, splits, segments, largeSegments); + }, + destroy() { }, // 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 new file mode 100644 index 00000000..c9bd78d2 --- /dev/null +++ b/src/storages/inLocalStorage/validateCache.ts @@ -0,0 +1,91 @@ +import { ISettings } from '../../types'; +import { isFiniteNumber, isNaNNumber } from '../../utils/lang'; +import { getStorageHash } from '../KeyBuilder'; +import { LOG_PREFIX } from './constants'; +import type { SplitsCacheInLocal } from './SplitsCacheInLocal'; +import type { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal'; +import { KeyBuilderCS } from '../KeyBuilderCS'; +import SplitIO from '../../../types/splitio'; + +const DEFAULT_CACHE_EXPIRATION_IN_DAYS = 10; +const MILLIS_IN_A_DAY = 86400000; + +/** + * Validates if cache should be cleared and sets the cache `hash` if needed. + * + * @returns `true` if cache should be cleared, `false` otherwise + */ +function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, currentTimestamp: number, isThereCache: boolean) { + const { log } = settings; + + // Check expiration + const lastUpdatedTimestamp = parseInt(localStorage.getItem(keys.buildLastUpdatedKey()) as string, 10); + if (!isNaNNumber(lastUpdatedTimestamp)) { + const cacheExpirationInDays = isFiniteNumber(options.expirationDays) && options.expirationDays >= 1 ? options.expirationDays : DEFAULT_CACHE_EXPIRATION_IN_DAYS; + const expirationTimestamp = currentTimestamp - MILLIS_IN_A_DAY * cacheExpirationInDays; + if (lastUpdatedTimestamp < expirationTimestamp) { + log.info(LOG_PREFIX + 'Cache expired more than ' + cacheExpirationInDays + ' days ago. Cleaning up cache'); + return true; + } + } + + // Check hash + const storageHashKey = keys.buildHashKey(); + const storageHash = localStorage.getItem(storageHashKey); + const currentStorageHash = getStorageHash(settings); + + if (storageHash !== currentStorageHash) { + try { + localStorage.setItem(storageHashKey, currentStorageHash); + } catch (e) { + log.error(LOG_PREFIX + e); + } + if (isThereCache) { + log.info(LOG_PREFIX + 'SDK key, flags filter criteria, or flags spec version has changed. Cleaning up cache'); + return true; + } + return false; // No cache to clear + } + + // Clear on init + if (options.clearOnInit) { + const lastClearTimestamp = parseInt(localStorage.getItem(keys.buildLastClear()) as string, 10); + + if (isNaNNumber(lastClearTimestamp) || lastClearTimestamp < currentTimestamp - MILLIS_IN_A_DAY) { + log.info(LOG_PREFIX + 'clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache'); + return true; + } + } +} + +/** + * Clean cache if: + * - it has expired, i.e., its `lastUpdated` timestamp is older than the given `expirationTimestamp` + * - its hash has changed, i.e., the SDK key, flags filter criteria or flags spec version was modified + * - `clearOnInit` was set and cache was not cleared in the last 24 hours + * + * @returns `true` if cache is ready to be used, `false` otherwise (cache was cleared or there is no cache) + */ +export function validateCache(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): boolean { + + const currentTimestamp = Date.now(); + const isThereCache = splits.getChangeNumber() > -1; + + if (validateExpiration(options, settings, keys, currentTimestamp, isThereCache)) { + splits.clear(); + segments.clear(); + largeSegments.clear(); + + // Update last clear timestamp + try { + localStorage.setItem(keys.buildLastClear(), currentTimestamp + ''); + } catch (e) { + settings.log.error(LOG_PREFIX + e); + } + + return false; + } + + // Check if ready from cache + return isThereCache; +} diff --git a/src/storages/pluggable/index.ts b/src/storages/pluggable/index.ts index ee8b1872..cc16bceb 100644 --- a/src/storages/pluggable/index.ts +++ b/src/storages/pluggable/index.ts @@ -88,7 +88,8 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn // Connects to wrapper and emits SDK_READY event on main client const connectPromise = wrapper.connect().then(() => { if (isSynchronizer) { - // In standalone or producer mode, clear storage if SDK key or feature flag filter has changed + // @TODO reuse InLocalStorage::validateCache logic + // In standalone or producer mode, clear storage if SDK key, flags filter criteria or flags spec version was modified return wrapper.get(keys.buildHashKey()).then((hash) => { const currentHash = getStorageHash(settings); if (hash !== currentHash) { diff --git a/src/storages/types.ts b/src/storages/types.ts index 71962254..a0ede2f1 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -189,8 +189,6 @@ export interface ISplitsCacheBase { // only for Client-Side. Returns true if the storage is not synchronized yet (getChangeNumber() === -1) or contains a FF using segments or large segments usesSegments(): MaybeThenable, clear(): MaybeThenable, - // should never reject or throw an exception. Instead return false by default, to avoid emitting SDK_READY_FROM_CACHE. - checkCache(): MaybeThenable, killLocally(name: string, defaultTreatment: string, changeNumber: number): MaybeThenable, getNamesByFlagSets(flagSets: string[]): MaybeThenable[]> } @@ -205,7 +203,6 @@ export interface ISplitsCacheSync extends ISplitsCacheBase { trafficTypeExists(trafficType: string): boolean, usesSegments(): boolean, clear(): void, - checkCache(): boolean, killLocally(name: string, defaultTreatment: string, changeNumber: number): boolean, getNamesByFlagSets(flagSets: string[]): Set[] } @@ -220,7 +217,6 @@ export interface ISplitsCacheAsync extends ISplitsCacheBase { trafficTypeExists(trafficType: string): Promise, usesSegments(): Promise, clear(): Promise, - checkCache(): Promise, killLocally(name: string, defaultTreatment: string, changeNumber: number): Promise, getNamesByFlagSets(flagSets: string[]): Promise[]> } @@ -451,6 +447,7 @@ export interface IStorageSync extends IStorageBase< IUniqueKeysCacheSync > { // Defined in client-side + validateCache?: () => boolean, // @TODO support async largeSegments?: ISegmentsCacheSync, } diff --git a/src/sync/__tests__/syncManagerOnline.spec.ts b/src/sync/__tests__/syncManagerOnline.spec.ts index 7fda853b..c7dba96e 100644 --- a/src/sync/__tests__/syncManagerOnline.spec.ts +++ b/src/sync/__tests__/syncManagerOnline.spec.ts @@ -2,6 +2,7 @@ import { fullSettings } from '../../utils/settingsValidation/__tests__/settings. import { syncTaskFactory } from './syncTask.mock'; import { syncManagerOnlineFactory } from '../syncManagerOnline'; import { IReadinessManager } from '../../readiness/types'; +import { SDK_SPLITS_CACHE_LOADED } from '../../readiness/constants'; jest.mock('../submitters/submitterManager', () => { return { @@ -45,8 +46,10 @@ const pushManagerFactoryMock = jest.fn(() => pushManagerMock); test('syncManagerOnline should start or not the submitter depending on user consent status', () => { const settings = { ...fullSettings }; - // @ts-ignore - const syncManager = syncManagerOnlineFactory()({ settings }); + const syncManager = syncManagerOnlineFactory()({ + settings, // @ts-ignore + storage: {}, + }); const submitterManager = syncManager.submitterManager!; syncManager.start(); @@ -95,7 +98,10 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', () // @ts-ignore // Test pushManager for main client - const syncManager = syncManagerOnlineFactory(() => pollingManagerMock, pushManagerFactoryMock)({ settings }); + const syncManager = syncManagerOnlineFactory(() => pollingManagerMock, pushManagerFactoryMock)({ + settings, // @ts-ignore + storage: { validateCache: () => false }, + }); expect(pushManagerFactoryMock).not.toBeCalled(); @@ -161,7 +167,10 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', () settings.sync.enabled = true; // @ts-ignore // pushManager instantiation control test - const testSyncManager = syncManagerOnlineFactory(() => pollingManagerMock, pushManagerFactoryMock)({ settings }); + const testSyncManager = syncManagerOnlineFactory(() => pollingManagerMock, pushManagerFactoryMock)({ + settings, // @ts-ignore + storage: { validateCache: () => false }, + }); expect(pushManagerFactoryMock).toBeCalled(); @@ -173,3 +182,18 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', () testSyncManager.stop(); }); + +test('syncManagerOnline should emit SDK_SPLITS_CACHE_LOADED if validateCache returns true', async () => { + const params = { + settings: fullSettings, + storage: { validateCache: () => true }, + readiness: { splits: { emit: jest.fn() } } + }; // @ts-ignore + const syncManager = syncManagerOnlineFactory()(params); + + await syncManager.start(); + + expect(params.readiness.splits.emit).toBeCalledWith(SDK_SPLITS_CACHE_LOADED); + + syncManager.stop(); +}); diff --git a/src/sync/offline/syncTasks/fromObjectSyncTask.ts b/src/sync/offline/syncTasks/fromObjectSyncTask.ts index 14daccf2..acbb5f52 100644 --- a/src/sync/offline/syncTasks/fromObjectSyncTask.ts +++ b/src/sync/offline/syncTasks/fromObjectSyncTask.ts @@ -1,6 +1,6 @@ import { forOwn } from '../../../utils/lang'; import { IReadinessManager } from '../../../readiness/types'; -import { ISplitsCacheSync } from '../../../storages/types'; +import { IStorageSync } from '../../../storages/types'; import { ISplitsParser } from '../splitsParser/types'; import { ISplit, ISplitPartial } from '../../../dtos/types'; import { syncTaskFactory } from '../../syncTask'; @@ -15,7 +15,7 @@ import { SYNC_OFFLINE_DATA, ERROR_SYNC_OFFLINE_LOADING } from '../../../logger/c */ export function fromObjectUpdaterFactory( splitsParser: ISplitsParser, - storage: { splits: ISplitsCacheSync }, + storage: Pick, readiness: IReadinessManager, settings: ISettings, ): () => Promise { @@ -59,9 +59,10 @@ export function fromObjectUpdaterFactory( if (startingUp) { startingUp = false; - Promise.resolve(splitsCache.checkCache()).then(cacheReady => { + const isCacheLoaded = storage.validateCache ? storage.validateCache() : false; + Promise.resolve().then(() => { // Emits SDK_READY_FROM_CACHE - if (cacheReady) readiness.splits.emit(SDK_SPLITS_CACHE_LOADED); + if (isCacheLoaded) readiness.splits.emit(SDK_SPLITS_CACHE_LOADED); // Emits SDK_READY readiness.segments.emit(SDK_SEGMENTS_ARRIVED); }); @@ -79,7 +80,7 @@ export function fromObjectUpdaterFactory( */ export function fromObjectSyncTaskFactory( splitsParser: ISplitsParser, - storage: { splits: ISplitsCacheSync }, + storage: Pick, readiness: IReadinessManager, settings: ISettings ): ISyncTask<[], boolean> { diff --git a/src/sync/polling/updaters/splitChangesUpdater.ts b/src/sync/polling/updaters/splitChangesUpdater.ts index 065ecb89..7a341cd0 100644 --- a/src/sync/polling/updaters/splitChangesUpdater.ts +++ b/src/sync/polling/updaters/splitChangesUpdater.ts @@ -3,7 +3,7 @@ import { ISplitChangesFetcher } from '../fetchers/types'; import { ISplit, ISplitChangesResponse, ISplitFiltersValidation } from '../../../dtos/types'; import { ISplitsEventEmitter } from '../../../readiness/types'; import { timeout } from '../../../utils/promise/timeout'; -import { SDK_SPLITS_ARRIVED, SDK_SPLITS_CACHE_LOADED } from '../../../readiness/constants'; +import { SDK_SPLITS_ARRIVED } from '../../../readiness/constants'; import { ILogger } from '../../../logger/types'; import { SYNC_SPLITS_FETCH, SYNC_SPLITS_UPDATE, SYNC_SPLITS_FETCH_FAILS, SYNC_SPLITS_FETCH_RETRY } from '../../../logger/constants'; import { startsWith } from '../../../utils/lang'; @@ -143,7 +143,7 @@ export function splitChangesUpdaterFactory( */ function _splitChangesUpdater(since: number, retry = 0): Promise { log.debug(SYNC_SPLITS_FETCH, [since]); - const fetcherPromise = Promise.resolve(splitUpdateNotification ? + return Promise.resolve(splitUpdateNotification ? { splits: [splitUpdateNotification.payload], till: splitUpdateNotification.changeNumber } : splitChangesFetcher(since, noCache, till, _promiseDecorator) ) @@ -154,8 +154,6 @@ export function splitChangesUpdaterFactory( log.debug(SYNC_SPLITS_UPDATE, [mutation.added.length, mutation.removed.length, mutation.segments.length]); - // Write into storage - // @TODO call `setChangeNumber` only if the other storage operations have succeeded, in order to keep storage consistency return Promise.all([ splits.update(mutation.added, mutation.removed, splitChanges.till), segments.registerSegments(mutation.segments) @@ -185,15 +183,6 @@ export function splitChangesUpdaterFactory( } return false; }); - - // After triggering the requests, if we have cached splits information let's notify that to emit SDK_READY_FROM_CACHE. - // Wrapping in a promise since checkCache can be async. - if (splitsEventEmitter && startingUp) { - Promise.resolve(splits.checkCache()).then(isCacheReady => { - if (isCacheReady) splitsEventEmitter.emit(SDK_SPLITS_CACHE_LOADED); - }); - } - return fetcherPromise; } let sincePromise = Promise.resolve(splits.getChangeNumber()); // `getChangeNumber` never rejects or throws error diff --git a/src/sync/syncManagerOnline.ts b/src/sync/syncManagerOnline.ts index 5410c17f..aed32493 100644 --- a/src/sync/syncManagerOnline.ts +++ b/src/sync/syncManagerOnline.ts @@ -9,6 +9,7 @@ import { SYNC_START_POLLING, SYNC_CONTINUE_POLLING, SYNC_STOP_POLLING } from '.. import { isConsentGranted } from '../consent'; import { POLLING, STREAMING, SYNC_MODE_UPDATE } from '../utils/constants'; import { ISdkFactoryContextSync } from '../sdkFactory/types'; +import { SDK_SPLITS_CACHE_LOADED } from '../readiness/constants'; /** * Online SyncManager factory. @@ -28,7 +29,7 @@ export function syncManagerOnlineFactory( */ return function (params: ISdkFactoryContextSync): ISyncManagerCS { - const { settings, settings: { log, streamingEnabled, sync: { enabled: syncEnabled } }, telemetryTracker } = params; + const { settings, settings: { log, streamingEnabled, sync: { enabled: syncEnabled } }, telemetryTracker, storage, readiness } = params; /** Polling Manager */ const pollingManager = pollingManagerFactory && pollingManagerFactory(params); @@ -87,6 +88,11 @@ export function syncManagerOnlineFactory( start() { running = true; + if (startFirstTime) { + const isCacheLoaded = storage.validateCache ? storage.validateCache() : false; + if (isCacheLoaded) Promise.resolve().then(() => { readiness.splits.emit(SDK_SPLITS_CACHE_LOADED); }); + } + // start syncing splits and segments if (pollingManager) { @@ -96,7 +102,6 @@ export function syncManagerOnlineFactory( // Doesn't call `syncAll` when the syncManager is resuming if (startFirstTime) { pollingManager.syncAll(); - startFirstTime = false; } pushManager.start(); } else { @@ -105,13 +110,14 @@ export function syncManagerOnlineFactory( } else { if (startFirstTime) { pollingManager.syncAll(); - startFirstTime = false; } } } // start periodic data recording (events, impressions, telemetry). submitterManager.start(!isConsentGranted(settings)); + + startFirstTime = false; }, /** diff --git a/src/utils/constants/browser.ts b/src/utils/constants/browser.ts deleted file mode 100644 index d627f780..00000000 --- a/src/utils/constants/browser.ts +++ /dev/null @@ -1,2 +0,0 @@ -// This value might be eventually set via a config parameter -export const DEFAULT_CACHE_EXPIRATION_IN_MILLIS = 864000000; // 10 days diff --git a/src/utils/lang/index.ts b/src/utils/lang/index.ts index 11b6afd0..b1a7e35a 100644 --- a/src/utils/lang/index.ts +++ b/src/utils/lang/index.ts @@ -120,7 +120,7 @@ export function isBoolean(val: any): boolean { * Unlike `Number.isFinite`, it also tests Number object instances. * Unlike global `isFinite`, it returns false if the value is not a number or Number object instance. */ -export function isFiniteNumber(val: any): boolean { +export function isFiniteNumber(val: any): val is number { if (val instanceof Number) val = val.valueOf(); return typeof val === 'number' ? Number.isFinite ? Number.isFinite(val) : isFinite(val) : diff --git a/src/utils/settingsValidation/storage/storageCS.ts b/src/utils/settingsValidation/storage/storageCS.ts index f7b531fc..7d58af3d 100644 --- a/src/utils/settingsValidation/storage/storageCS.ts +++ b/src/utils/settingsValidation/storage/storageCS.ts @@ -8,7 +8,7 @@ import { IStorageFactoryParams, IStorageSync } from '../../../storages/types'; export function __InLocalStorageMockFactory(params: IStorageFactoryParams): IStorageSync { const result = InMemoryStorageCSFactory(params); - result.splits.checkCache = () => true; // to emit SDK_READY_FROM_CACHE + result.validateCache = () => true; // to emit SDK_READY_FROM_CACHE return result; } __InLocalStorageMockFactory.type = STORAGE_MEMORY; diff --git a/types/splitio.d.ts b/types/splitio.d.ts index 4102a7fc..d8a1e67d 100644 --- a/types/splitio.d.ts +++ b/types/splitio.d.ts @@ -947,6 +947,18 @@ declare namespace SplitIO { * @defaultValue `'SPLITIO'` */ prefix?: string; + /** + * Number of days before cached data expires if it was not updated. If cache expires, it is cleared on initialization. + * + * @defaultValue `10` + */ + expirationDays?: number; + /** + * Optional settings to clear the cache. If set to `true`, the SDK clears the cached data on initialization, unless the cache was cleared within the last 24 hours. + * + * @defaultValue `false` + */ + clearOnInit?: boolean; } /** * Storage for asynchronous (consumer) SDK. @@ -1270,11 +1282,23 @@ declare namespace SplitIO { */ type?: BrowserStorage; /** - * Optional prefix to prevent any kind of data collision between SDK versions. + * Optional prefix to prevent any kind of data collision between SDK versions when using 'LOCALSTORAGE'. * * @defaultValue `'SPLITIO'` */ prefix?: string; + /** + * Optional settings for the 'LOCALSTORAGE' storage type. It specifies the number of days before cached data expires if it was not updated. If cache expires, it is cleared on initialization. + * + * @defaultValue `10` + */ + expirationDays?: number; + /** + * Optional settings for the 'LOCALSTORAGE' storage type. If set to `true`, the SDK clears the cached data on initialization, unless the cache was cleared within the last 24 hours. + * + * @defaultValue `false` + */ + clearOnInit?: boolean; }; } /**