From 6124cc1a477cc2cba91eb3c82fab36d23cdec6ff Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Fri, 13 Mar 2026 15:38:27 -0500 Subject: [PATCH] fix: disable cache was not working --- .../flag-manager/FlagManager.test.ts | 1 + .../flag-manager/FlagPersistence.test.ts | 227 ++++++++++++++++++ .../shared/sdk-client/src/LDClientImpl.ts | 1 + .../shared/sdk-client/src/api/LDOptions.ts | 9 + .../src/configuration/Configuration.ts | 2 + .../src/configuration/validators.ts | 1 + .../src/flag-manager/FlagManager.ts | 5 + .../src/flag-manager/FlagPersistence.ts | 22 +- 8 files changed, 266 insertions(+), 2 deletions(-) diff --git a/packages/shared/sdk-client/__tests__/flag-manager/FlagManager.test.ts b/packages/shared/sdk-client/__tests__/flag-manager/FlagManager.test.ts index c0bc009504..5cb1fb0c76 100644 --- a/packages/shared/sdk-client/__tests__/flag-manager/FlagManager.test.ts +++ b/packages/shared/sdk-client/__tests__/flag-manager/FlagManager.test.ts @@ -25,6 +25,7 @@ describe('FlagManager override tests', () => { mockPlatform, TEST_SDK_KEY, TEST_MAX_CACHED_CONTEXTS, + false, mockLogger, ); }); diff --git a/packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts b/packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts index 542ae23291..515ac2a6c9 100644 --- a/packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts +++ b/packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts @@ -28,6 +28,7 @@ describe('FlagPersistence tests', () => { makeMockPlatform(makeMemoryStorage(), makeMockCrypto()), TEST_NAMESPACE, 5, + false, flagStore, createFlagUpdater(flagStore, mockLogger), mockLogger, @@ -45,6 +46,7 @@ describe('FlagPersistence tests', () => { makeMockPlatform(makeCorruptStorage(), makeMockCrypto()), TEST_NAMESPACE, 5, + false, flagStore, createFlagUpdater(flagStore, mockLogger), mockLogger, @@ -72,6 +74,7 @@ describe('FlagPersistence tests', () => { makeMockPlatform(makeMemoryStorage(), makeMockCrypto()), TEST_NAMESPACE, 5, + false, flagStore, flagUpdater, mockLogger, @@ -100,6 +103,7 @@ describe('FlagPersistence tests', () => { makeMockPlatform(memoryStorage, makeMockCrypto()), TEST_NAMESPACE, 5, + false, flagStore, flagUpdater, mockLogger, @@ -129,6 +133,7 @@ describe('FlagPersistence tests', () => { mockPlatform, TEST_NAMESPACE, 5, + false, flagStore, flagUpdater, mockLogger, @@ -165,6 +170,7 @@ describe('FlagPersistence tests', () => { mockPlatform, TEST_NAMESPACE, 1, + false, flagStore, flagUpdater, mockLogger, @@ -213,6 +219,7 @@ describe('FlagPersistence tests', () => { mockPlatform, TEST_NAMESPACE, 1, + false, flagStore, flagUpdater, mockLogger, @@ -244,6 +251,7 @@ describe('FlagPersistence tests', () => { mockPlatform, TEST_NAMESPACE, 5, + false, flagStore, flagUpdater, mockLogger, @@ -277,6 +285,7 @@ describe('FlagPersistence tests', () => { mockPlatform, TEST_NAMESPACE, 5, + false, flagStore, flagUpdater, mockLogger, @@ -305,6 +314,173 @@ describe('FlagPersistence tests', () => { expect(await memoryStorage.get(contextDataKey)).toContain('"version":2'); }); + it('does not write to storage when maxCachedContexts is 0', async () => { + const memoryStorage = makeMemoryStorage(); + const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); + const flagStore = createDefaultFlagStore(); + const mockLogger = makeMockLogger(); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); + + const fpUnderTest = new FlagPersistence( + mockPlatform, + TEST_NAMESPACE, + 0, + false, + flagStore, + flagUpdater, + mockLogger, + ); + + const context = Context.fromLDContext({ kind: 'org', key: 'TestyPizza' }); + const flags = { flagA: { version: 1, flag: makeMockFlag() } }; + + await fpUnderTest.init(context, flags); + + const contextDataKey = await namespaceForContextData( + mockPlatform.crypto, + TEST_NAMESPACE, + context, + ); + expect(await memoryStorage.get(contextDataKey)).toBeNull(); + }); + + it('clears previously cached data when maxCachedContexts is 0', async () => { + const memoryStorage = makeMemoryStorage(); + const crypto = makeMockCrypto(); + const mockPlatform = makeMockPlatform(memoryStorage, crypto); + const flagStore = createDefaultFlagStore(); + const mockLogger = makeMockLogger(); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); + + const contextA = Context.fromLDContext({ kind: 'org', key: 'TestyPizza' }); + const storageKeyA = await namespaceForContextData(crypto, TEST_NAMESPACE, contextA); + const indexKey = await namespaceForContextIndex(TEST_NAMESPACE); + + // Pre-populate storage as if a prior session had maxCachedContexts > 0 + const indexJson = JSON.stringify({ index: [{ id: storageKeyA, timestamp: 1 }] }); + await memoryStorage.set(indexKey, indexJson); + await memoryStorage.set(storageKeyA, JSON.stringify({ flagA: makeMockFlag() })); + + const fpUnderTest = new FlagPersistence( + mockPlatform, + TEST_NAMESPACE, + 0, + false, + flagStore, + flagUpdater, + mockLogger, + ); + + const flags = { flagA: { version: 1, flag: makeMockFlag() } }; + await fpUnderTest.init(contextA, flags); + + // Existing entry must have been evicted + expect(await memoryStorage.get(storageKeyA)).toBeNull(); + // Index must be saved as empty + const savedIndex = JSON.parse((await memoryStorage.get(indexKey))!); + expect(savedIndex.index).toHaveLength(0); + }); + + it('does not load from storage when maxCachedContexts is 0', async () => { + const memoryStorage = makeMemoryStorage(); + const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); + const flagStore = createDefaultFlagStore(); + const mockLogger = makeMockLogger(); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); + + // First write data to storage using a normal FlagPersistence + const writeFp = new FlagPersistence( + mockPlatform, + TEST_NAMESPACE, + 5, + false, + flagStore, + flagUpdater, + mockLogger, + ); + const context = Context.fromLDContext({ kind: 'org', key: 'TestyPizza' }); + const flags = { flagA: { version: 1, flag: makeMockFlag() } }; + await writeFp.init(context, flags); + + // Now try to load with maxCachedContexts: 0 + const fpUnderTest = new FlagPersistence( + mockPlatform, + TEST_NAMESPACE, + 0, + false, + flagStore, + flagUpdater, + mockLogger, + ); + const didLoad = await fpUnderTest.loadCached(context); + expect(didLoad).toEqual(false); + }); + + it('does not write to storage when disableCache is true', async () => { + const memoryStorage = makeMemoryStorage(); + const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); + const flagStore = createDefaultFlagStore(); + const mockLogger = makeMockLogger(); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); + + const fpUnderTest = new FlagPersistence( + mockPlatform, + TEST_NAMESPACE, + 5, + true, + flagStore, + flagUpdater, + mockLogger, + ); + + const context = Context.fromLDContext({ kind: 'org', key: 'TestyPizza' }); + const flags = { flagA: { version: 1, flag: makeMockFlag() } }; + + await fpUnderTest.init(context, flags); + + const contextDataKey = await namespaceForContextData( + mockPlatform.crypto, + TEST_NAMESPACE, + context, + ); + expect(await memoryStorage.get(contextDataKey)).toBeNull(); + }); + + it('does not load from storage when disableCache is true', async () => { + const memoryStorage = makeMemoryStorage(); + const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); + const flagStore = createDefaultFlagStore(); + const mockLogger = makeMockLogger(); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); + + // First write data to storage using a normal FlagPersistence + const writeFp = new FlagPersistence( + mockPlatform, + TEST_NAMESPACE, + 5, + false, + flagStore, + flagUpdater, + mockLogger, + ); + const context = Context.fromLDContext({ kind: 'org', key: 'TestyPizza' }); + const flags = { flagA: { version: 1, flag: makeMockFlag() } }; + await writeFp.init(context, flags); + + // Now try to load with disableCache: true + const fpUnderTest = new FlagPersistence( + mockPlatform, + TEST_NAMESPACE, + 5, + true, + flagStore, + flagUpdater, + mockLogger, + ); + const didLoad = await fpUnderTest.loadCached(context); + expect(didLoad).toEqual(false); + }); + test('upsert ignores inactive context', async () => { const memoryStorage = makeMemoryStorage(); const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto()); @@ -316,6 +492,7 @@ describe('FlagPersistence tests', () => { mockPlatform, TEST_NAMESPACE, 5, + false, flagStore, flagUpdater, mockLogger, @@ -350,6 +527,55 @@ describe('FlagPersistence tests', () => { expect(await memoryStorage.get(activeContextDataKey)).not.toBeNull(); expect(await memoryStorage.get(inactiveContextDataKey)).toBeNull(); }); + + it('does not write to storage when current context is pruned due to equal timestamps', async () => { + const memoryStorage = makeMemoryStorage(); + const crypto = makeMockCrypto(); + const mockPlatform = makeMockPlatform(memoryStorage, crypto); + const flagStore = createDefaultFlagStore(); + const mockLogger = makeMockLogger(); + const flagUpdater = createFlagUpdater(flagStore, mockLogger); + + const contextA = Context.fromLDContext({ kind: 'org', key: 'TestyPizza' }); + const contextB = Context.fromLDContext({ kind: 'user', key: 'TestyUser' }); + + const storageKeyA = await namespaceForContextData(crypto, TEST_NAMESPACE, contextA); + const storageKeyB = await namespaceForContextData(crypto, TEST_NAMESPACE, contextB); + const indexKey = await namespaceForContextIndex(TEST_NAMESPACE); + + // Pre-populate storage: index with A before B (same timestamp t=1), and B's flag data + const indexJson = JSON.stringify({ + index: [ + { id: storageKeyA, timestamp: 1 }, + { id: storageKeyB, timestamp: 1 }, + ], + }); + await memoryStorage.set(indexKey, indexJson); + await memoryStorage.set(storageKeyB, JSON.stringify({ flagB: makeMockFlag() })); + + const fpUnderTest = new FlagPersistence( + mockPlatform, + TEST_NAMESPACE, + 1, + false, + flagStore, + flagUpdater, + mockLogger, + () => 1, + ); + + const flags = { flagA: { version: 1, flag: makeMockFlag() } }; + await fpUnderTest.init(contextA, flags); + + // A was in the pruned list — must not be re-written to storage + expect(await memoryStorage.get(storageKeyA)).toBeNull(); + // B was not pruned — its existing data should be untouched + expect(await memoryStorage.get(storageKeyB)).not.toBeNull(); + // Index should contain only B + const savedIndex = JSON.parse((await memoryStorage.get(indexKey))!); + expect(savedIndex.index).toHaveLength(1); + expect(savedIndex.index[0].id).toBe(storageKeyB); + }); }); describe('FlagPersistence freshness', () => { @@ -363,6 +589,7 @@ describe('FlagPersistence freshness', () => { makeMockPlatform(memoryStorage, crypto), TEST_NAMESPACE, 5, + false, flagStore, createFlagUpdater(flagStore, mockLogger), mockLogger, diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index beecf992fb..78d5170db5 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -129,6 +129,7 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { this.platform, sdkKey, this._config.maxCachedContexts, + this._config.disableCache ?? false, this._config.logger, ); this._diagnosticsManager = createDiagnosticsManager(sdkKey, this._config, platform); diff --git a/packages/shared/sdk-client/src/api/LDOptions.ts b/packages/shared/sdk-client/src/api/LDOptions.ts index b06f1959cd..6c566c4e3f 100644 --- a/packages/shared/sdk-client/src/api/LDOptions.ts +++ b/packages/shared/sdk-client/src/api/LDOptions.ts @@ -295,4 +295,13 @@ export interface LDOptions { * protocol. */ dataSystem?: LDClientDataSystemOptions; + + /** + * Set to true to completely disable the persistent flag cache. When disabled, + * flags are never read from or written to local storage. This takes precedence + * over `maxCachedContexts`. + * + * @defaultValue false + */ + disableCache?: boolean; } diff --git a/packages/shared/sdk-client/src/configuration/Configuration.ts b/packages/shared/sdk-client/src/configuration/Configuration.ts index d8a4e6b5fe..ac79e9d22c 100644 --- a/packages/shared/sdk-client/src/configuration/Configuration.ts +++ b/packages/shared/sdk-client/src/configuration/Configuration.ts @@ -31,6 +31,7 @@ export interface LDClientInternalOptions extends internal.LDInternalOptions { export interface Configuration { readonly logger: LDLogger; readonly maxCachedContexts: number; + readonly disableCache?: boolean; readonly capacity: number; readonly diagnosticRecordingInterval: number; readonly flushInterval: number; @@ -93,6 +94,7 @@ export default class ConfigurationImpl implements Configuration { private readonly streamUri = DEFAULT_STREAM; public readonly maxCachedContexts = 5; + public readonly disableCache: boolean = false; public readonly capacity = 100; public readonly diagnosticRecordingInterval = 900; diff --git a/packages/shared/sdk-client/src/configuration/validators.ts b/packages/shared/sdk-client/src/configuration/validators.ts index 5e6254cd31..2350878f34 100644 --- a/packages/shared/sdk-client/src/configuration/validators.ts +++ b/packages/shared/sdk-client/src/configuration/validators.ts @@ -37,6 +37,7 @@ export default function createValidators( privateAttributes: TypeValidators.StringArray, + disableCache: TypeValidators.Boolean, applicationInfo: TypeValidators.Object, wrapperName: TypeValidators.String, wrapperVersion: TypeValidators.String, diff --git a/packages/shared/sdk-client/src/flag-manager/FlagManager.ts b/packages/shared/sdk-client/src/flag-manager/FlagManager.ts index 094819bf56..273b623a08 100644 --- a/packages/shared/sdk-client/src/flag-manager/FlagManager.ts +++ b/packages/shared/sdk-client/src/flag-manager/FlagManager.ts @@ -126,6 +126,7 @@ export default class DefaultFlagManager implements FlagManager { * @param platform implementation of various platform provided functionality * @param sdkKey that will be used to distinguish different environments * @param maxCachedContexts that specifies the max number of contexts that will be cached in persistence + * @param disableCache set to true to completely disable the persistent flag cache * @param logger used for logging various messages * @param timeStamper exists for testing purposes */ @@ -133,6 +134,7 @@ export default class DefaultFlagManager implements FlagManager { platform: Platform, sdkKey: string, maxCachedContexts: number, + disableCache: boolean, logger: LDLogger, timeStamper: () => number = () => Date.now(), ) { @@ -141,6 +143,7 @@ export default class DefaultFlagManager implements FlagManager { platform, sdkKey, maxCachedContexts, + disableCache, logger, timeStamper, ); @@ -150,6 +153,7 @@ export default class DefaultFlagManager implements FlagManager { platform: Platform, sdkKey: string, maxCachedContexts: number, + disableCache: boolean, logger: LDLogger, timeStamper: () => number = () => Date.now(), ): Promise { @@ -159,6 +163,7 @@ export default class DefaultFlagManager implements FlagManager { platform, environmentNamespace, maxCachedContexts, + disableCache, this._flagStore, this._flagUpdater, logger, diff --git a/packages/shared/sdk-client/src/flag-manager/FlagPersistence.ts b/packages/shared/sdk-client/src/flag-manager/FlagPersistence.ts index b6723f98ed..df41882309 100644 --- a/packages/shared/sdk-client/src/flag-manager/FlagPersistence.ts +++ b/packages/shared/sdk-client/src/flag-manager/FlagPersistence.ts @@ -28,6 +28,7 @@ export default class FlagPersistence { private readonly _platform: Platform, private readonly _environmentNamespace: string, private readonly _maxCachedContexts: number, + private readonly _disableCache: boolean, private readonly _flagStore: FlagStore, private readonly _flagUpdater: FlagUpdater, private readonly _logger: LDLogger, @@ -63,6 +64,9 @@ export default class FlagPersistence { * {@link FlagUpdater} this {@link FlagPersistence} was constructed with. */ async loadCached(context: Context): Promise { + if (this._disableCache || this._maxCachedContexts <= 0) { + return false; + } if (!this._platform.storage) { return false; } @@ -137,6 +141,9 @@ export default class FlagPersistence { } private async _storeCache(context: Context): Promise { + if (this._disableCache) { + return; + } const now = this._timeStamper(); const index = await this._loadIndex(); const storageKey = await namespaceForContextData( @@ -144,9 +151,16 @@ export default class FlagPersistence { this._environmentNamespace, context, ); - index.notice(storageKey, now); + + if (this._maxCachedContexts > 0) { + index.notice(storageKey, now); + } const pruned = index.prune(this._maxCachedContexts); + // If maxCachedContexts <= 0, current context was never added, so always skip flag write + const currentContextWasPruned = + this._maxCachedContexts <= 0 || pruned.some((it) => it.id === storageKey); + await Promise.all( pruned.flatMap((it) => [ this._platform.storage?.clear(it.id), @@ -154,8 +168,12 @@ export default class FlagPersistence { ]), ); - // store index await this._platform.storage?.set(await this._indexKeyPromise, index.toJson()); + + if (currentContextWasPruned) { + return; + } + const allFlags = this._flagStore.getAll(); // mapping item descriptors to flags