Skip to content

Commit 6be89dd

Browse files
authored
fix: disable cache was not working (#1187)
**Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions **Related issues** sdk-2023 **Describe the solution you've provided** - added an `disableCache` option - fixed the overall cache pruning behavior BEGIN_COMMIT_OVERRIDE fix: Max cached context enforcement wasn't working for 0. feat: Add explicit disableCache setting. END_COMMIT_OVERRIDE <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes persistent flag caching behavior (read/write and eviction) and introduces a new `disableCache` option, which can affect offline fallback/initialization latency if misconfigured. Logic is covered by new unit tests but touches core persistence paths. > > **Overview** > Fixes persistent flag caching so it can be **explicitly disabled** via a new `disableCache` option (plumbed through `LDOptions` → `Configuration` → `LDClientImpl` → `DefaultFlagManager` → `FlagPersistence`) and validated in configuration parsing. > > Updates `FlagPersistence` to skip all cache reads/writes when `disableCache` is true or `maxCachedContexts <= 0`, while still pruning/clearing previously cached entries and ensuring pruned contexts (including equal-timestamp edge cases) are not re-written. Adds targeted tests covering `maxCachedContexts=0`, `disableCache=true`, and pruning behavior. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 431fce0. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/launchdarkly/js-core/pull/1187" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end -->
1 parent 327a7f7 commit 6be89dd

8 files changed

Lines changed: 266 additions & 2 deletions

File tree

packages/shared/sdk-client/__tests__/flag-manager/FlagManager.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ describe('FlagManager override tests', () => {
2525
mockPlatform,
2626
TEST_SDK_KEY,
2727
TEST_MAX_CACHED_CONTEXTS,
28+
false,
2829
mockLogger,
2930
);
3031
});

packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ describe('FlagPersistence tests', () => {
2828
makeMockPlatform(makeMemoryStorage(), makeMockCrypto()),
2929
TEST_NAMESPACE,
3030
5,
31+
false,
3132
flagStore,
3233
createFlagUpdater(flagStore, mockLogger),
3334
mockLogger,
@@ -45,6 +46,7 @@ describe('FlagPersistence tests', () => {
4546
makeMockPlatform(makeCorruptStorage(), makeMockCrypto()),
4647
TEST_NAMESPACE,
4748
5,
49+
false,
4850
flagStore,
4951
createFlagUpdater(flagStore, mockLogger),
5052
mockLogger,
@@ -72,6 +74,7 @@ describe('FlagPersistence tests', () => {
7274
makeMockPlatform(makeMemoryStorage(), makeMockCrypto()),
7375
TEST_NAMESPACE,
7476
5,
77+
false,
7578
flagStore,
7679
flagUpdater,
7780
mockLogger,
@@ -100,6 +103,7 @@ describe('FlagPersistence tests', () => {
100103
makeMockPlatform(memoryStorage, makeMockCrypto()),
101104
TEST_NAMESPACE,
102105
5,
106+
false,
103107
flagStore,
104108
flagUpdater,
105109
mockLogger,
@@ -129,6 +133,7 @@ describe('FlagPersistence tests', () => {
129133
mockPlatform,
130134
TEST_NAMESPACE,
131135
5,
136+
false,
132137
flagStore,
133138
flagUpdater,
134139
mockLogger,
@@ -165,6 +170,7 @@ describe('FlagPersistence tests', () => {
165170
mockPlatform,
166171
TEST_NAMESPACE,
167172
1,
173+
false,
168174
flagStore,
169175
flagUpdater,
170176
mockLogger,
@@ -213,6 +219,7 @@ describe('FlagPersistence tests', () => {
213219
mockPlatform,
214220
TEST_NAMESPACE,
215221
1,
222+
false,
216223
flagStore,
217224
flagUpdater,
218225
mockLogger,
@@ -244,6 +251,7 @@ describe('FlagPersistence tests', () => {
244251
mockPlatform,
245252
TEST_NAMESPACE,
246253
5,
254+
false,
247255
flagStore,
248256
flagUpdater,
249257
mockLogger,
@@ -277,6 +285,7 @@ describe('FlagPersistence tests', () => {
277285
mockPlatform,
278286
TEST_NAMESPACE,
279287
5,
288+
false,
280289
flagStore,
281290
flagUpdater,
282291
mockLogger,
@@ -305,6 +314,173 @@ describe('FlagPersistence tests', () => {
305314
expect(await memoryStorage.get(contextDataKey)).toContain('"version":2');
306315
});
307316

317+
it('does not write to storage when maxCachedContexts is 0', async () => {
318+
const memoryStorage = makeMemoryStorage();
319+
const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto());
320+
const flagStore = createDefaultFlagStore();
321+
const mockLogger = makeMockLogger();
322+
const flagUpdater = createFlagUpdater(flagStore, mockLogger);
323+
324+
const fpUnderTest = new FlagPersistence(
325+
mockPlatform,
326+
TEST_NAMESPACE,
327+
0,
328+
false,
329+
flagStore,
330+
flagUpdater,
331+
mockLogger,
332+
);
333+
334+
const context = Context.fromLDContext({ kind: 'org', key: 'TestyPizza' });
335+
const flags = { flagA: { version: 1, flag: makeMockFlag() } };
336+
337+
await fpUnderTest.init(context, flags);
338+
339+
const contextDataKey = await namespaceForContextData(
340+
mockPlatform.crypto,
341+
TEST_NAMESPACE,
342+
context,
343+
);
344+
expect(await memoryStorage.get(contextDataKey)).toBeNull();
345+
});
346+
347+
it('clears previously cached data when maxCachedContexts is 0', async () => {
348+
const memoryStorage = makeMemoryStorage();
349+
const crypto = makeMockCrypto();
350+
const mockPlatform = makeMockPlatform(memoryStorage, crypto);
351+
const flagStore = createDefaultFlagStore();
352+
const mockLogger = makeMockLogger();
353+
const flagUpdater = createFlagUpdater(flagStore, mockLogger);
354+
355+
const contextA = Context.fromLDContext({ kind: 'org', key: 'TestyPizza' });
356+
const storageKeyA = await namespaceForContextData(crypto, TEST_NAMESPACE, contextA);
357+
const indexKey = await namespaceForContextIndex(TEST_NAMESPACE);
358+
359+
// Pre-populate storage as if a prior session had maxCachedContexts > 0
360+
const indexJson = JSON.stringify({ index: [{ id: storageKeyA, timestamp: 1 }] });
361+
await memoryStorage.set(indexKey, indexJson);
362+
await memoryStorage.set(storageKeyA, JSON.stringify({ flagA: makeMockFlag() }));
363+
364+
const fpUnderTest = new FlagPersistence(
365+
mockPlatform,
366+
TEST_NAMESPACE,
367+
0,
368+
false,
369+
flagStore,
370+
flagUpdater,
371+
mockLogger,
372+
);
373+
374+
const flags = { flagA: { version: 1, flag: makeMockFlag() } };
375+
await fpUnderTest.init(contextA, flags);
376+
377+
// Existing entry must have been evicted
378+
expect(await memoryStorage.get(storageKeyA)).toBeNull();
379+
// Index must be saved as empty
380+
const savedIndex = JSON.parse((await memoryStorage.get(indexKey))!);
381+
expect(savedIndex.index).toHaveLength(0);
382+
});
383+
384+
it('does not load from storage when maxCachedContexts is 0', async () => {
385+
const memoryStorage = makeMemoryStorage();
386+
const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto());
387+
const flagStore = createDefaultFlagStore();
388+
const mockLogger = makeMockLogger();
389+
const flagUpdater = createFlagUpdater(flagStore, mockLogger);
390+
391+
// First write data to storage using a normal FlagPersistence
392+
const writeFp = new FlagPersistence(
393+
mockPlatform,
394+
TEST_NAMESPACE,
395+
5,
396+
false,
397+
flagStore,
398+
flagUpdater,
399+
mockLogger,
400+
);
401+
const context = Context.fromLDContext({ kind: 'org', key: 'TestyPizza' });
402+
const flags = { flagA: { version: 1, flag: makeMockFlag() } };
403+
await writeFp.init(context, flags);
404+
405+
// Now try to load with maxCachedContexts: 0
406+
const fpUnderTest = new FlagPersistence(
407+
mockPlatform,
408+
TEST_NAMESPACE,
409+
0,
410+
false,
411+
flagStore,
412+
flagUpdater,
413+
mockLogger,
414+
);
415+
const didLoad = await fpUnderTest.loadCached(context);
416+
expect(didLoad).toEqual(false);
417+
});
418+
419+
it('does not write to storage when disableCache is true', async () => {
420+
const memoryStorage = makeMemoryStorage();
421+
const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto());
422+
const flagStore = createDefaultFlagStore();
423+
const mockLogger = makeMockLogger();
424+
const flagUpdater = createFlagUpdater(flagStore, mockLogger);
425+
426+
const fpUnderTest = new FlagPersistence(
427+
mockPlatform,
428+
TEST_NAMESPACE,
429+
5,
430+
true,
431+
flagStore,
432+
flagUpdater,
433+
mockLogger,
434+
);
435+
436+
const context = Context.fromLDContext({ kind: 'org', key: 'TestyPizza' });
437+
const flags = { flagA: { version: 1, flag: makeMockFlag() } };
438+
439+
await fpUnderTest.init(context, flags);
440+
441+
const contextDataKey = await namespaceForContextData(
442+
mockPlatform.crypto,
443+
TEST_NAMESPACE,
444+
context,
445+
);
446+
expect(await memoryStorage.get(contextDataKey)).toBeNull();
447+
});
448+
449+
it('does not load from storage when disableCache is true', async () => {
450+
const memoryStorage = makeMemoryStorage();
451+
const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto());
452+
const flagStore = createDefaultFlagStore();
453+
const mockLogger = makeMockLogger();
454+
const flagUpdater = createFlagUpdater(flagStore, mockLogger);
455+
456+
// First write data to storage using a normal FlagPersistence
457+
const writeFp = new FlagPersistence(
458+
mockPlatform,
459+
TEST_NAMESPACE,
460+
5,
461+
false,
462+
flagStore,
463+
flagUpdater,
464+
mockLogger,
465+
);
466+
const context = Context.fromLDContext({ kind: 'org', key: 'TestyPizza' });
467+
const flags = { flagA: { version: 1, flag: makeMockFlag() } };
468+
await writeFp.init(context, flags);
469+
470+
// Now try to load with disableCache: true
471+
const fpUnderTest = new FlagPersistence(
472+
mockPlatform,
473+
TEST_NAMESPACE,
474+
5,
475+
true,
476+
flagStore,
477+
flagUpdater,
478+
mockLogger,
479+
);
480+
const didLoad = await fpUnderTest.loadCached(context);
481+
expect(didLoad).toEqual(false);
482+
});
483+
308484
test('upsert ignores inactive context', async () => {
309485
const memoryStorage = makeMemoryStorage();
310486
const mockPlatform = makeMockPlatform(memoryStorage, makeMockCrypto());
@@ -316,6 +492,7 @@ describe('FlagPersistence tests', () => {
316492
mockPlatform,
317493
TEST_NAMESPACE,
318494
5,
495+
false,
319496
flagStore,
320497
flagUpdater,
321498
mockLogger,
@@ -350,6 +527,55 @@ describe('FlagPersistence tests', () => {
350527
expect(await memoryStorage.get(activeContextDataKey)).not.toBeNull();
351528
expect(await memoryStorage.get(inactiveContextDataKey)).toBeNull();
352529
});
530+
531+
it('does not write to storage when current context is pruned due to equal timestamps', async () => {
532+
const memoryStorage = makeMemoryStorage();
533+
const crypto = makeMockCrypto();
534+
const mockPlatform = makeMockPlatform(memoryStorage, crypto);
535+
const flagStore = createDefaultFlagStore();
536+
const mockLogger = makeMockLogger();
537+
const flagUpdater = createFlagUpdater(flagStore, mockLogger);
538+
539+
const contextA = Context.fromLDContext({ kind: 'org', key: 'TestyPizza' });
540+
const contextB = Context.fromLDContext({ kind: 'user', key: 'TestyUser' });
541+
542+
const storageKeyA = await namespaceForContextData(crypto, TEST_NAMESPACE, contextA);
543+
const storageKeyB = await namespaceForContextData(crypto, TEST_NAMESPACE, contextB);
544+
const indexKey = await namespaceForContextIndex(TEST_NAMESPACE);
545+
546+
// Pre-populate storage: index with A before B (same timestamp t=1), and B's flag data
547+
const indexJson = JSON.stringify({
548+
index: [
549+
{ id: storageKeyA, timestamp: 1 },
550+
{ id: storageKeyB, timestamp: 1 },
551+
],
552+
});
553+
await memoryStorage.set(indexKey, indexJson);
554+
await memoryStorage.set(storageKeyB, JSON.stringify({ flagB: makeMockFlag() }));
555+
556+
const fpUnderTest = new FlagPersistence(
557+
mockPlatform,
558+
TEST_NAMESPACE,
559+
1,
560+
false,
561+
flagStore,
562+
flagUpdater,
563+
mockLogger,
564+
() => 1,
565+
);
566+
567+
const flags = { flagA: { version: 1, flag: makeMockFlag() } };
568+
await fpUnderTest.init(contextA, flags);
569+
570+
// A was in the pruned list — must not be re-written to storage
571+
expect(await memoryStorage.get(storageKeyA)).toBeNull();
572+
// B was not pruned — its existing data should be untouched
573+
expect(await memoryStorage.get(storageKeyB)).not.toBeNull();
574+
// Index should contain only B
575+
const savedIndex = JSON.parse((await memoryStorage.get(indexKey))!);
576+
expect(savedIndex.index).toHaveLength(1);
577+
expect(savedIndex.index[0].id).toBe(storageKeyB);
578+
});
353579
});
354580

355581
describe('FlagPersistence freshness', () => {
@@ -363,6 +589,7 @@ describe('FlagPersistence freshness', () => {
363589
makeMockPlatform(memoryStorage, crypto),
364590
TEST_NAMESPACE,
365591
5,
592+
false,
366593
flagStore,
367594
createFlagUpdater(flagStore, mockLogger),
368595
mockLogger,

packages/shared/sdk-client/src/LDClientImpl.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult {
129129
this.platform,
130130
sdkKey,
131131
this._config.maxCachedContexts,
132+
this._config.disableCache ?? false,
132133
this._config.logger,
133134
);
134135
this._diagnosticsManager = createDiagnosticsManager(sdkKey, this._config, platform);

packages/shared/sdk-client/src/api/LDOptions.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,4 +295,13 @@ export interface LDOptions {
295295
* protocol.
296296
*/
297297
dataSystem?: LDClientDataSystemOptions;
298+
299+
/**
300+
* Set to true to completely disable the persistent flag cache. When disabled,
301+
* flags are never read from or written to local storage. This takes precedence
302+
* over `maxCachedContexts`.
303+
*
304+
* @defaultValue false
305+
*/
306+
disableCache?: boolean;
298307
}

packages/shared/sdk-client/src/configuration/Configuration.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export interface LDClientInternalOptions extends internal.LDInternalOptions {
3131
export interface Configuration {
3232
readonly logger: LDLogger;
3333
readonly maxCachedContexts: number;
34+
readonly disableCache?: boolean;
3435
readonly capacity: number;
3536
readonly diagnosticRecordingInterval: number;
3637
readonly flushInterval: number;
@@ -93,6 +94,7 @@ export default class ConfigurationImpl implements Configuration {
9394
private readonly streamUri = DEFAULT_STREAM;
9495

9596
public readonly maxCachedContexts = 5;
97+
public readonly disableCache: boolean = false;
9698

9799
public readonly capacity = 100;
98100
public readonly diagnosticRecordingInterval = 900;

packages/shared/sdk-client/src/configuration/validators.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export default function createValidators(
3737

3838
privateAttributes: TypeValidators.StringArray,
3939

40+
disableCache: TypeValidators.Boolean,
4041
applicationInfo: TypeValidators.Object,
4142
wrapperName: TypeValidators.String,
4243
wrapperVersion: TypeValidators.String,

0 commit comments

Comments
 (0)