Skip to content

fix: disable cache was not working #1187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
joker23 merged 1 commit into from
Mar 16, 2026
Merged

fix: disable cache was not working #1187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/shared/sdk-client/__tests__/flag-manager/FlagManager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ describe('FlagManager override tests', () => {
mockPlatform,
TEST_SDK_KEY,
TEST_MAX_CACHED_CONTEXTS,
false,
mockLogger,
);
});
Expand Down
227 changes: 227 additions & 0 deletions packages/shared/sdk-client/__tests__/flag-manager/FlagPersistence.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ describe('FlagPersistence tests', () => {
makeMockPlatform(makeMemoryStorage(), makeMockCrypto()),
TEST_NAMESPACE,
5,
false,
flagStore,
createFlagUpdater(flagStore, mockLogger),
mockLogger,
Expand All @@ -45,6 +46,7 @@ describe('FlagPersistence tests', () => {
makeMockPlatform(makeCorruptStorage(), makeMockCrypto()),
TEST_NAMESPACE,
5,
false,
flagStore,
createFlagUpdater(flagStore, mockLogger),
mockLogger,
Expand Down Expand Up @@ -72,6 +74,7 @@ describe('FlagPersistence tests', () => {
makeMockPlatform(makeMemoryStorage(), makeMockCrypto()),
TEST_NAMESPACE,
5,
false,
flagStore,
flagUpdater,
mockLogger,
Expand Down Expand Up @@ -100,6 +103,7 @@ describe('FlagPersistence tests', () => {
makeMockPlatform(memoryStorage, makeMockCrypto()),
TEST_NAMESPACE,
5,
false,
flagStore,
flagUpdater,
mockLogger,
Expand Down Expand Up @@ -129,6 +133,7 @@ describe('FlagPersistence tests', () => {
mockPlatform,
TEST_NAMESPACE,
5,
false,
flagStore,
flagUpdater,
mockLogger,
Expand Down Expand Up @@ -165,6 +170,7 @@ describe('FlagPersistence tests', () => {
mockPlatform,
TEST_NAMESPACE,
1,
false,
flagStore,
flagUpdater,
mockLogger,
Expand Down Expand Up @@ -213,6 +219,7 @@ describe('FlagPersistence tests', () => {
mockPlatform,
TEST_NAMESPACE,
1,
false,
flagStore,
flagUpdater,
mockLogger,
Expand Down Expand Up @@ -244,6 +251,7 @@ describe('FlagPersistence tests', () => {
mockPlatform,
TEST_NAMESPACE,
5,
false,
flagStore,
flagUpdater,
mockLogger,
Expand Down Expand Up @@ -277,6 +285,7 @@ describe('FlagPersistence tests', () => {
mockPlatform,
TEST_NAMESPACE,
5,
false,
flagStore,
flagUpdater,
mockLogger,
Expand Down Expand Up @@ -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());
Expand All @@ -316,6 +492,7 @@ describe('FlagPersistence tests', () => {
mockPlatform,
TEST_NAMESPACE,
5,
false,
flagStore,
flagUpdater,
mockLogger,
Expand Down Expand Up @@ -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', () => {
Expand All @@ -363,6 +589,7 @@ describe('FlagPersistence freshness', () => {
makeMockPlatform(memoryStorage, crypto),
TEST_NAMESPACE,
5,
false,
flagStore,
createFlagUpdater(flagStore, mockLogger),
mockLogger,
Expand Down
1 change: 1 addition & 0 deletions packages/shared/sdk-client/src/LDClientImpl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 9 additions & 0 deletions packages/shared/sdk-client/src/api/LDOptions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor Author

@joker23 joker23 Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to separate this one out for a feature release?

Copy link
Member

@kinyoklion kinyoklion Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could put:

BEGIN_COMMIT_OVERRIDE
fix: Max cached context enforcement wasn't working for 0.
feat: Add explicit disableCache setting.
END_COMMIT_OVERRIDE

Copy link
Member

@kinyoklion kinyoklion Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will probably want to use the disable cache setting in gonfalon as well.

}
2 changes: 2 additions & 0 deletions packages/shared/sdk-client/src/configuration/Configuration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/sdk-client/src/configuration/validators.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export default function createValidators(

privateAttributes: TypeValidators.StringArray,

disableCache: TypeValidators.Boolean,
applicationInfo: TypeValidators.Object,
wrapperName: TypeValidators.String,
wrapperVersion: TypeValidators.String,
Expand Down
Loading
Loading