From 77a24e10a9303c7e8f184cfca6bfaba88775ddd3 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 25 Sep 2026 15:57:20 -0700 Subject: [PATCH 1/9] feat: Add the file-based override source Adds the file-based override source described by the OVERRIDE specification, configured as { type: 'file', paths: [...] } in the overrides option of the data system options. The source reads one or more JSON or YAML files in the file data source document format and combines their entries in the configured order. Duplicate keys across files fail the reload by default or keep the first file's entry with the ignore handling. Change detection is one of two modes: polling, the default, examines the files once per second with a one second minimum, and watching reacts to change notifications for the files' directories. A configured file that does not exist contributes no overrides, so a file can be created later and deleting a file removes its overrides. A file that exists but cannot be read or parsed fails that reload, keeps the last good overrides, logs the failure, and retries. Every applied change is logged at Info level with the overrides in effect and what each file supplied. The initial load completes before the client evaluates anything. An invalid configuration, such as no file paths or an unrecognized change detection mode, fails client construction. An unrecognized duplicate keys handling and a polling interval below the minimum are logged and replaced by their defaults. The Node.js SDK reads YAML files without configuration through the yaml package, which it supplies to the shared code as a platform default. A configured yamlParser takes precedence. --- .../LDClientNode.fileOverrides.test.ts | 133 +++++++ packages/sdk/server-node/package.json | 3 +- packages/sdk/server-node/src/LDClientNode.ts | 3 + .../LDClientImpl.fileOverrides.test.ts | 139 +++++++ .../shared/sdk-server/__tests__/Logger.ts | 7 + .../overrides/FileOverrideSource.test.ts | 350 ++++++++++++++++++ .../overrides/createOverrideSource.test.ts | 173 +++++++++ .../fileOverrideSourcePolicy.test.ts | 64 ++++ .../overrides/overrideDocument.test.ts | 131 +++++++ .../shared/sdk-server/src/LDClientImpl.ts | 4 +- .../src/api/options/LDDataSystemOptions.ts | 76 +++- .../src/options/ServerInternalOptions.ts | 7 + .../src/overrides/FileOverrideSource.ts | 262 +++++++++++++ .../src/overrides/createOverrideSource.ts | 124 ++++++- .../shared/sdk-server/src/overrides/index.ts | 17 + .../src/overrides/overrideDocument.ts | 75 ++++ 16 files changed, 1562 insertions(+), 6 deletions(-) create mode 100644 packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts create mode 100644 packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts create mode 100644 packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts create mode 100644 packages/shared/sdk-server/__tests__/overrides/createOverrideSource.test.ts create mode 100644 packages/shared/sdk-server/__tests__/overrides/fileOverrideSourcePolicy.test.ts create mode 100644 packages/shared/sdk-server/__tests__/overrides/overrideDocument.test.ts create mode 100644 packages/shared/sdk-server/src/overrides/FileOverrideSource.ts create mode 100644 packages/shared/sdk-server/src/overrides/overrideDocument.ts diff --git a/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts b/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts new file mode 100644 index 0000000000..a191b0801f --- /dev/null +++ b/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts @@ -0,0 +1,133 @@ +import { mkdtemp, rm, unlink, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { FileOverrideSourceOptions } from '@launchdarkly/js-server-sdk-common'; + +import LDClientNode from '../src/LDClientNode'; + +const user = { key: 'user-key' }; + +// The client never obtains data from LaunchDarkly: its only initializer reads a file that does +// not exist. Overrides are served from the override layer alone. +function makeClient( + directory: string, + overrides: Omit, +): LDClientNode { + return new LDClientNode('sdk-key-file-overrides', { + sendEvents: false, + diagnosticOptOut: true, + logger: { error: () => {}, warn: () => {}, info: () => {}, debug: () => {} }, + dataSystem: { + dataSource: { + dataSourceOptionsType: 'custom', + initializers: [{ type: 'file', paths: [join(directory, 'no-launchdarkly-data.json')] }], + synchronizers: [], + }, + overrides: { type: 'file', ...overrides }, + }, + }); +} + +async function waitFor(condition: () => Promise, timeoutMs: number = 10000) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + // eslint-disable-next-line no-await-in-loop + if (await condition()) { + return; + } + // eslint-disable-next-line no-await-in-loop + await new Promise((resolve) => { + setTimeout(resolve, 50); + }); + } + throw new Error('timed out waiting for the condition'); +} + +const document = (value: string) => JSON.stringify({ flagValues: { 'overridden-flag': value } }); + +describe('given a temporary directory of override files', () => { + let directory: string; + let client: LDClientNode | undefined; + + beforeEach(async () => { + directory = await mkdtemp(join(tmpdir(), 'ld-file-overrides-')); + }); + + afterEach(async () => { + client?.close(); + client = undefined; + await rm(directory, { recursive: true, force: true }); + }); + + it('reads a YAML file with the built-in parser', async () => { + const path = join(directory, 'overrides.yaml'); + await writeFile(path, 'flagValues:\n yaml-flag: "override-value"\n'); + client = makeClient(directory, { paths: [path] }); + + const detail = await client.variationDetail('yaml-flag', user, 'default'); + + expect(detail.value).toEqual('override-value'); + expect(detail.reason).toEqual({ kind: 'OFF', overrideAffected: true }); + expect(client.initialized()).toBe(false); + }); + + it('reloads a changed file in polling mode', async () => { + const path = join(directory, 'overrides.json'); + await writeFile(path, document('b')); + client = makeClient(directory, { paths: [path], changeDetection: 'polling', pollInterval: 1 }); + expect(await client.variation('overridden-flag', user, 'default')).toEqual('b'); + + await writeFile(path, document('c')); + + await waitFor( + async () => (await client!.variation('overridden-flag', user, 'default')) === 'c', + ); + }); + + it('picks up a file that does not exist yet in watching mode', async () => { + const path = join(directory, 'overrides.json'); + client = makeClient(directory, { paths: [path], changeDetection: 'watching' }); + const before = await client.variationDetail('overridden-flag', user, 'default'); + expect(before.reason).toEqual({ kind: 'ERROR', errorKind: 'CLIENT_NOT_READY' }); + + await writeFile(path, document('b')); + + await waitFor( + async () => (await client!.variation('overridden-flag', user, 'default')) === 'b', + ); + }); + + it('removes the overrides of a deleted file', async () => { + const path = join(directory, 'overrides.json'); + await writeFile(path, document('b')); + client = makeClient(directory, { paths: [path] }); + expect(await client.variation('overridden-flag', user, 'default')).toEqual('b'); + + await unlink(path); + + await waitFor(async () => { + const detail = await client!.variationDetail('overridden-flag', user, 'default'); + return detail.reason.errorKind === 'CLIENT_NOT_READY'; + }); + }); + + it('keeps the last good overrides while the file is malformed', async () => { + const path = join(directory, 'overrides.json'); + await writeFile(path, document('b')); + client = makeClient(directory, { paths: [path] }); + expect(await client.variation('overridden-flag', user, 'default')).toEqual('b'); + + await writeFile(path, '{"flagValues"'); + await new Promise((resolve) => { + setTimeout(resolve, 1500); + }); + expect(await client.variation('overridden-flag', user, 'default')).toEqual('b'); + + await writeFile(path, document('c')); + + await waitFor( + async () => (await client!.variation('overridden-flag', user, 'default')) === 'c', + ); + }); +}); diff --git a/packages/sdk/server-node/package.json b/packages/sdk/server-node/package.json index c34f048029..853e3a21f4 100644 --- a/packages/sdk/server-node/package.json +++ b/packages/sdk/server-node/package.json @@ -49,7 +49,8 @@ "dependencies": { "@launchdarkly/js-server-sdk-common": "2.24.1", "https-proxy-agent": "^7.0.6", - "launchdarkly-eventsource": "2.3.0" + "launchdarkly-eventsource": "2.3.0", + "yaml": "2.9.1" }, "devDependencies": { "@types/jest": "^29.4.0", diff --git a/packages/sdk/server-node/src/LDClientNode.ts b/packages/sdk/server-node/src/LDClientNode.ts index 71aaf26d9c..dbc8509a54 100644 --- a/packages/sdk/server-node/src/LDClientNode.ts +++ b/packages/sdk/server-node/src/LDClientNode.ts @@ -1,5 +1,6 @@ import { EventEmitter } from 'events'; import { format } from 'util'; +import { parse as parseYaml } from 'yaml'; import { BasicLogger, @@ -89,6 +90,8 @@ class LDClientNode extends LDClientImpl implements LDClient { getImplementationHooks: (environmentMetadata: LDPluginEnvironmentMetadata) => internal.safeGetHooks(logger, environmentMetadata, plugins), instanceId, + // The Node platform reads YAML files without configuration from the application. + yamlParser: (data: string) => parseYaml(data), }, ); diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts new file mode 100644 index 0000000000..e8cfa9de0a --- /dev/null +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts @@ -0,0 +1,139 @@ +import { LDLogger } from '@launchdarkly/js-sdk-common'; + +import { FileOverrideSourceOptions } from '../src/api/options/LDDataSystemOptions'; +import { LDOptions } from '../src/api/options/LDOptions'; +import LDClientImpl from '../src/LDClientImpl'; +import MockFilesystem from './data_sources/filedata/MockFilesystem'; +import { makeCallbacks, makeFDv2Platform } from './overrides/overridesTestSupport'; + +const user = { key: 'user-key' }; +const directory = '/etc/launchdarkly'; +const jsonPath = `${directory}/overrides.json`; +const yamlPath = `${directory}/overrides.yaml`; + +function makeLogger(): LDLogger { + return { error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() }; +} + +async function waitFor(condition: () => Promise, timeoutMs: number = 3000) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + // eslint-disable-next-line no-await-in-loop + if (await condition()) { + return; + } + // eslint-disable-next-line no-await-in-loop + await new Promise((resolve) => { + setTimeout(resolve, 20); + }); + } + throw new Error('timed out waiting for the condition'); +} + +describe('given a client with a file override source over a mock filesystem', () => { + let filesystem: MockFilesystem; + let client: LDClientImpl | undefined; + let platformYamlParser: jest.Mock; + + const makeClient = ( + overrides: Omit, + options: LDOptions = {}, + ) => { + client = new LDClientImpl( + 'sdk-key-file-overrides', + { ...makeFDv2Platform(), fileSystem: filesystem }, + { + sendEvents: false, + diagnosticOptOut: true, + logger: makeLogger(), + ...options, + dataSystem: { + dataSource: { + dataSourceOptionsType: 'custom', + initializers: [], + synchronizers: [{ type: 'polling', pollInterval: 1000 }], + }, + overrides: { type: 'file', changeDetection: 'watching', ...overrides }, + }, + }, + makeCallbacks(), + { yamlParser: platformYamlParser }, + ); + return client; + }; + + beforeEach(() => { + filesystem = new MockFilesystem(); + platformYamlParser = jest.fn(() => ({ flagValues: { 'yaml-flag': 'from-platform-parser' } })); + }); + + afterEach(() => { + client?.close(); + client = undefined; + }); + + it('serves an override from a file that is present when the client is created', async () => { + filesystem.set(jsonPath, '{"flagValues": {"overridden-flag": true}}'); + makeClient({ paths: [jsonPath] }); + + const detail = await client!.boolVariationDetail('overridden-flag', user, false); + + expect(detail.value).toBe(true); + expect(detail.reason).toEqual({ kind: 'OFF', overrideAffected: true }); + expect(client!.initialized()).toBe(false); + }); + + it('reads a YAML file with the parser the platform supplies', async () => { + filesystem.set(yamlPath, 'flagValues:\n yaml-flag: from-platform-parser\n'); + makeClient({ paths: [yamlPath] }); + + const value = await client!.variation('yaml-flag', user, 'default'); + + expect(value).toEqual('from-platform-parser'); + expect(platformYamlParser).toHaveBeenCalledWith( + 'flagValues:\n yaml-flag: from-platform-parser\n', + ); + }); + + it('prefers a configured YAML parser over the one the platform supplies', async () => { + const yamlParser = jest.fn(() => ({ flagValues: { 'yaml-flag': 'from-configured-parser' } })); + filesystem.set(yamlPath, 'flagValues:\n yaml-flag: x\n'); + makeClient({ paths: [yamlPath], yamlParser }); + + expect(await client!.variation('yaml-flag', user, 'default')).toEqual('from-configured-parser'); + expect(platformYamlParser).not.toHaveBeenCalled(); + }); + + it('reloads when the directory reports a change', async () => { + filesystem.set(jsonPath, '{"flagValues": {"overridden-flag": "b"}}'); + makeClient({ paths: [jsonPath] }); + expect(await client!.variation('overridden-flag', user, 'default')).toEqual('b'); + + filesystem.set(jsonPath, '{"flagValues": {"overridden-flag": "c"}}'); + filesystem.emit(directory); + + await waitFor( + async () => (await client!.variation('overridden-flag', user, 'default')) === 'c', + ); + }); + + it('contributes no overrides for a file that does not exist until it appears', async () => { + makeClient({ paths: [jsonPath] }); + + const before = await client!.variationDetail('overridden-flag', user, 'default'); + expect(before.reason).toEqual({ kind: 'ERROR', errorKind: 'CLIENT_NOT_READY' }); + + filesystem.set(jsonPath, '{"flagValues": {"overridden-flag": "b"}}'); + filesystem.emit(directory, 'rename'); + + await waitFor( + async () => (await client!.variation('overridden-flag', user, 'default')) === 'b', + ); + }); + + it('fails client construction when no file paths are configured', () => { + expect(() => makeClient({ paths: [] })).toThrow( + 'The file-based override source requires at least one file path', + ); + }); +}); diff --git a/packages/shared/sdk-server/__tests__/Logger.ts b/packages/shared/sdk-server/__tests__/Logger.ts index 7340cf6d54..8b0dc2aca9 100644 --- a/packages/shared/sdk-server/__tests__/Logger.ts +++ b/packages/shared/sdk-server/__tests__/Logger.ts @@ -93,6 +93,13 @@ export default class TestLogger implements LDLogger { }); } + /** + * The messages received at a level, in order. + */ + getMessages(level: LogLevel): string[] { + return [...this._messages[level]]; + } + getCount(level?: LogLevel) { if (level === undefined) { return this._callCount; diff --git a/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts b/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts new file mode 100644 index 0000000000..06182dc5e8 --- /dev/null +++ b/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts @@ -0,0 +1,350 @@ +import { LDKeyedFeatureStoreItem, LDOverrideSink } from '../../src/api/subsystems'; +import { FileOverrideSource, FileOverrideSourceConfig } from '../../src/overrides'; +import MockFilesystem from '../data_sources/filedata/MockFilesystem'; +import TestLogger, { LogLevel } from '../Logger'; + +const first = '/overrides/first.json'; +const second = '/overrides/second.json'; +const directory = '/overrides'; + +interface Snapshot { + flags: LDKeyedFeatureStoreItem[]; + segments: LDKeyedFeatureStoreItem[]; +} + +class CapturingSink implements LDOverrideSink { + public snapshots: Snapshot[] = []; + + setOverrides(flags: LDKeyedFeatureStoreItem[], segments: LDKeyedFeatureStoreItem[]): void { + this.snapshots.push({ flags, segments }); + } + + get last(): Snapshot { + return this.snapshots[this.snapshots.length - 1]; + } + + flagKeys(): string[] { + return this.last.flags.map((flag) => flag.key).sort(); + } +} + +// The Info lines that report the overrides in effect. The reloader logs other Info lines. +function infoLines(logger: TestLogger): string[] { + return logger.getMessages(LogLevel.Info).filter((line) => line.startsWith('Flag overrides')); +} + +describe('given a file override source over a mock filesystem', () => { + let filesystem: MockFilesystem; + let logger: TestLogger; + let sink: CapturingSink; + let source: FileOverrideSource; + + beforeEach(() => { + jest.useFakeTimers(); + filesystem = new MockFilesystem(); + logger = new TestLogger(); + sink = new CapturingSink(); + }); + + afterEach(() => { + source?.close(); + jest.useRealTimers(); + }); + + const startSource = async (config: Partial & { paths: string[] }) => { + source = new FileOverrideSource( + { + duplicateKeysHandling: 'fail', + changeDetection: 'polling', + pollIntervalMs: 1000, + ...config, + }, + filesystem, + logger, + ); + await source.start(sink); + return source; + }; + + it('loads the initial data before start completes', async () => { + filesystem.set( + first, + '{"flagValues": {"flag1": true}, "flags": {"flag2": {"key": "flag2", "version": 3, "on": false}}}', + ); + + await startSource({ paths: [first] }); + + expect(sink.snapshots).toHaveLength(1); + expect(sink.flagKeys()).toEqual(['flag1', 'flag2']); + // The flag value entry was expanded into a full flag definition that is off. + const flag1 = sink.last.flags.find((flag) => flag.key === 'flag1')!; + expect(flag1.variations).toEqual([true]); + expect(flag1.on).toBe(false); + expect(sink.last.flags.find((flag) => flag.key === 'flag2')!.version).toEqual(3); + }); + + it('loads YAML with the configured parser', async () => { + const yamlParser = jest.fn(() => ({ flagValues: { flag1: true } })); + filesystem.set('/overrides/overrides.yaml', 'flagValues:\n flag1: true\n'); + + await startSource({ paths: ['/overrides/overrides.yaml'], yamlParser }); + + expect(yamlParser).toHaveBeenCalledTimes(1); + expect(sink.flagKeys()).toEqual(['flag1']); + }); + + it('merges files in the configured order', async () => { + filesystem.set(first, '{"flags": {"flag1": {"key": "flag1", "version": 1}}}'); + filesystem.set(second, '{"flags": {"flag1": {"key": "flag1", "version": 2}}}'); + + await startSource({ paths: [first, second], duplicateKeysHandling: 'ignore' }); + + expect(sink.snapshots).toHaveLength(1); + expect(sink.last.flags).toHaveLength(1); + expect(sink.last.flags[0].version).toEqual(1); + }); + + it('fails the load for duplicate keys by default', async () => { + filesystem.set(first, '{"flags": {"flag1": {"key": "flag1", "version": 1}}}'); + filesystem.set(second, '{"flags": {"flag1": {"key": "flag1", "version": 2}}}'); + + await startSource({ paths: [first, second] }); + await jest.advanceTimersByTimeAsync(200); + + expect(sink.snapshots).toHaveLength(0); + logger.expectMessages([{ level: LogLevel.Error, matches: /specified by multiple files/ }]); + }); + + it('starts with a missing file and picks it up when it appears', async () => { + await startSource({ paths: [first] }); + + // A missing file contributes no overrides. The initial snapshot is empty. + expect(sink.snapshots).toHaveLength(1); + expect(sink.last.flags).toHaveLength(0); + + filesystem.set(first, '{"flagValues": {"flag1": true}}', 1); + await jest.advanceTimersByTimeAsync(1100); + expect(sink.snapshots).toHaveLength(2); + expect(sink.flagKeys()).toEqual(['flag1']); + }); + + it('adds and removes the entries of a file as it appears and disappears', async () => { + filesystem.set(first, '{"flagValues": {"from-first": true}}', 1); + + // Step 1: one configured file exists and one does not. The existing file applies. + await startSource({ paths: [first, second] }); + expect(sink.flagKeys()).toEqual(['from-first']); + + // Step 2: the second file appears. Both apply. + filesystem.set(second, '{"flagValues": {"from-second": true}}', 1); + await jest.advanceTimersByTimeAsync(1100); + expect(sink.flagKeys()).toEqual(['from-first', 'from-second']); + + // Step 3: the second file is deleted. Its overrides are removed. + filesystem.remove(second); + await jest.advanceTimersByTimeAsync(1100); + expect(sink.flagKeys()).toEqual(['from-first']); + + // Step 4: the last file is deleted. The layer is cleared. + filesystem.remove(first); + await jest.advanceTimersByTimeAsync(1100); + expect(sink.last.flags).toHaveLength(0); + expect(sink.snapshots).toHaveLength(4); + }); + + it('logs the overrides in effect on each change', async () => { + filesystem.set( + first, + '{"flagValues": {"flag1": true, "flag2": false}, "segments": {"seg": {"key": "seg", "version": 1}}}', + 1, + ); + + // Step 1: at startup, one file supplies entries and the other is absent. + await startSource({ paths: [first, second] }); + expect(infoLines(logger)).toContain( + `Flag overrides in effect: 2 flags, 1 segment (${first}: 2 flags, 1 segment; ${second}: absent)`, + ); + + // Step 2: the absent file appears with one entry. + filesystem.set(second, '{"flagValues": {"flag3": true}}', 1); + await jest.advanceTimersByTimeAsync(1100); + expect(infoLines(logger)).toContain( + `Flag overrides in effect: 3 flags, 1 segment (${first}: 2 flags, 1 segment; ${second}: 1 flag)`, + ); + + // Step 3: both files are deleted. Nothing is in effect. + filesystem.remove(first); + filesystem.remove(second); + await jest.advanceTimersByTimeAsync(1100); + expect(infoLines(logger)).toContain( + `Flag overrides: none in effect (${first}: absent; ${second}: absent)`, + ); + }); + + it('logs a file with no entries', async () => { + filesystem.set(first, '{}'); + await startSource({ paths: [first] }); + expect(infoLines(logger)).toContain(`Flag overrides: none in effect (${first}: no entries)`); + }); + + it('logs none in effect at startup without files', async () => { + await startSource({ paths: [first] }); + expect(infoLines(logger)).toContain(`Flag overrides: none in effect (${first}: absent)`); + }); + + it('does not log an unchanged reload', async () => { + filesystem.set(first, '{"flagValues": {"flag1": true}}', 1); + await startSource({ paths: [first] }); + expect(infoLines(logger)).toHaveLength(1); + + // The poller sees a new modification time, but the content is the same. + filesystem.set(first, '{"flagValues": {"flag1": true}}', 2); + await jest.advanceTimersByTimeAsync(1100); + expect(sink.snapshots).toHaveLength(1); + expect(infoLines(logger)).toHaveLength(1); + }); + + it('is quiet in watching mode while the file is absent and loads it when it appears', async () => { + await startSource({ paths: [first], changeDetection: 'watching' }); + expect(sink.snapshots).toHaveLength(1); + expect(filesystem.activeWatches(directory)).toHaveLength(1); + + await jest.advanceTimersByTimeAsync(2500); + expect(logger.getCount(LogLevel.Error)).toEqual(0); + expect(logger.getCount(LogLevel.Warn)).toEqual(0); + + filesystem.set(first, '{"flagValues": {"flag1": true}}'); + filesystem.emit(directory, 'rename'); + await jest.advanceTimersByTimeAsync(100); + expect(sink.flagKeys()).toEqual(['flag1']); + }); + + it('reloads on a change in watching mode', async () => { + filesystem.set(first, '{"flagValues": {"flag1": true}}'); + await startSource({ paths: [first], changeDetection: 'watching' }); + expect(sink.snapshots).toHaveLength(1); + + filesystem.set(first, '{"flagValues": {"flag1": true, "flag2": false}}'); + filesystem.emit(directory); + await jest.advanceTimersByTimeAsync(100); + expect(sink.flagKeys()).toEqual(['flag1', 'flag2']); + + // Removing entries removes them from the snapshot. A reload is a full replacement. + filesystem.set(first, '{}'); + filesystem.emit(directory); + await jest.advanceTimersByTimeAsync(100); + expect(sink.last.flags).toHaveLength(0); + }); + + it('reloads on a change in polling mode', async () => { + filesystem.set(first, '{"flagValues": {"flag1": true}}', 1); + await startSource({ paths: [first], pollIntervalMs: 1000 }); + expect(sink.snapshots).toHaveLength(1); + + filesystem.set(first, '{"flagValues": {"flag1": true, "flag2": false}}', 2); + // The poll tick at one second starts the debounced reload, which runs 100 ms later. + await jest.advanceTimersByTimeAsync(1099); + expect(sink.snapshots).toHaveLength(1); + await jest.advanceTimersByTimeAsync(1); + expect(sink.flagKeys()).toEqual(['flag1', 'flag2']); + }); + + it('keeps the last good overrides across a malformed edit and recovers', async () => { + filesystem.set(first, '{"flagValues": {"flag1": true}}'); + await startSource({ paths: [first], changeDetection: 'watching' }); + expect(sink.snapshots).toHaveLength(1); + + // A malformed edit produces no snapshot. The previously applied overrides stay in effect + // because the sink is never called. + filesystem.set(first, '{"flagValues"'); + filesystem.emit(directory); + await jest.advanceTimersByTimeAsync(300); + expect(sink.snapshots).toHaveLength(1); + logger.expectMessages([{ level: LogLevel.Error, matches: /Unable to load flags/ }]); + + // Fixing the file without a further notification recovers through the retry. + filesystem.set(first, '{"flagValues": {"flag1": false}}'); + await jest.advanceTimersByTimeAsync(1000); + expect(sink.snapshots).toHaveLength(2); + expect(sink.last.flags[0].variations).toEqual([false]); + }); + + it('stops detecting changes when closed', async () => { + filesystem.set(first, '{"flagValues": {"flag1": true}}', 1); + await startSource({ paths: [first] }); + + source.close(); + source.close(); + + filesystem.set(first, '{"flagValues": {"flag1": false}}', 2); + await jest.advanceTimersByTimeAsync(5000); + expect(sink.snapshots).toHaveLength(1); + expect(jest.getTimerCount()).toEqual(0); + }); + + it('watches the directory again after it is deleted without an event and recreated', async () => { + filesystem.set(first, '{"flagValues": {"flag1": true}}'); + await startSource({ paths: [first], changeDetection: 'watching' }); + expect(sink.snapshots).toHaveLength(1); + + // A malformed edit arms the retry while the directory still exists. + filesystem.set(first, '{"flagValues"'); + filesystem.emit(directory); + await jest.advanceTimersByTimeAsync(100); + expect(logger.getCount(LogLevel.Error)).toEqual(1); + expect(filesystem.activeWatches(directory)).toHaveLength(1); + + // The directory is deleted and the platform delivers no event for it. The retry finds the + // file missing, which is a load with no entries, and the missing file makes the watcher + // check its directories. + filesystem.removeDirectory(directory); + await jest.advanceTimersByTimeAsync(1000); + expect(filesystem.activeWatches(directory)).toHaveLength(0); + expect(sink.snapshots).toHaveLength(2); + expect(sink.last.flags).toHaveLength(0); + + // The directory comes back with the file. Only the watch set up again can detect it, because + // the load with no entries succeeded and disarmed the retry. + filesystem.set(first, '{"flagValues": {"flag1": false}}'); + await jest.advanceTimersByTimeAsync(1100); + expect(filesystem.activeWatches(directory)).toHaveLength(1); + expect(sink.snapshots).toHaveLength(3); + expect(sink.last.flags[0].variations).toEqual([false]); + + // A later edit is detected through the new watch. + filesystem.set(first, '{"flagValues": {"flag1": true, "flag2": true}}'); + filesystem.emit(directory); + await jest.advanceTimersByTimeAsync(100); + expect(sink.flagKeys()).toEqual(['flag1', 'flag2']); + }); + + it('closes its watches when closed', async () => { + filesystem.set(first, '{"flagValues": {"flag1": true}}'); + await startSource({ paths: [first], changeDetection: 'watching' }); + expect(filesystem.activeWatches(directory)).toHaveLength(1); + + source.close(); + expect(filesystem.activeWatches(directory)).toHaveLength(0); + }); + + it('delivers nothing when closed before the initial load completes', async () => { + filesystem.set(first, '{"flagValues": {"flag1": true}}'); + source = new FileOverrideSource( + { + paths: [first], + duplicateKeysHandling: 'fail', + changeDetection: 'polling', + pollIntervalMs: 1000, + }, + filesystem, + logger, + ); + + const started = source.start(sink); + source.close(); + await started; + + expect(sink.snapshots).toHaveLength(0); + expect(jest.getTimerCount()).toEqual(0); + }); +}); diff --git a/packages/shared/sdk-server/__tests__/overrides/createOverrideSource.test.ts b/packages/shared/sdk-server/__tests__/overrides/createOverrideSource.test.ts new file mode 100644 index 0000000000..4a6ce08b4a --- /dev/null +++ b/packages/shared/sdk-server/__tests__/overrides/createOverrideSource.test.ts @@ -0,0 +1,173 @@ +import { ClientContext, LDLogger } from '@launchdarkly/js-sdk-common'; + +import Configuration from '../../src/options/Configuration'; +import { createOverrideSource, FileOverrideSource } from '../../src/overrides'; +import { createBasicPlatform } from '../createBasicPlatform'; +import MockFilesystem from '../data_sources/filedata/MockFilesystem'; +import TestOverrideSource from './TestOverrideSource'; + +function makeLogger(): LDLogger & { warn: jest.Mock } { + return { error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() }; +} + +function makeContext(logger: LDLogger, withFilesystem: boolean = true): ClientContext { + return new ClientContext('sdk-key', new Configuration({ logger }), { + ...createBasicPlatform(), + fileSystem: withFilesystem ? new MockFilesystem() : undefined, + }); +} + +const defaultYamlParser = () => ({}); + +describe('given a client context with filesystem support', () => { + let logger: ReturnType; + let context: ClientContext; + + beforeEach(() => { + logger = makeLogger(); + context = makeContext(logger); + }); + + it('creates a file source with polling once per second and fail handling by default', () => { + const source = createOverrideSource( + { type: 'file', paths: ['/a.json'] }, + context, + defaultYamlParser, + ) as FileOverrideSource; + + expect(source).toBeInstanceOf(FileOverrideSource); + expect(source.config).toEqual({ + paths: ['/a.json'], + duplicateKeysHandling: 'fail', + changeDetection: 'polling', + pollIntervalMs: 1000, + yamlParser: defaultYamlParser, + }); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('applies the configured handling, mode, interval, and parser', () => { + const yamlParser = () => ({ flags: {} }); + const source = createOverrideSource( + { + type: 'file', + paths: ['/a.json', '/b.yaml'], + duplicateKeysHandling: 'ignore', + changeDetection: 'watching', + pollInterval: 5, + yamlParser, + }, + context, + defaultYamlParser, + ) as FileOverrideSource; + + expect(source.config).toEqual({ + paths: ['/a.json', '/b.yaml'], + duplicateKeysHandling: 'ignore', + changeDetection: 'watching', + pollIntervalMs: 5000, + yamlParser, + }); + }); + + it('requires at least one file path', () => { + expect(() => createOverrideSource({ type: 'file', paths: [] }, context)).toThrow( + 'The file-based override source requires at least one file path', + ); + expect(() => createOverrideSource({ type: 'file', paths: [''] }, context)).toThrow( + 'The file-based override source requires at least one file path', + ); + expect(() => createOverrideSource({ type: 'file' } as any, context)).toThrow( + 'The file-based override source requires at least one file path', + ); + expect(() => createOverrideSource({ type: 'file', paths: 'a.json' as any }, context)).toThrow( + 'The file-based override source requires at least one file path', + ); + }); + + it('rejects an unrecognized change detection mode', () => { + expect(() => + createOverrideSource( + { type: 'file', paths: ['/a.json'], changeDetection: 'notify' as any }, + context, + ), + ).toThrow('Unrecognized change detection mode "notify" for the file-based override source'); + }); + + it('warns and uses fail for an unrecognized duplicate keys handling', () => { + const source = createOverrideSource( + { type: 'file', paths: ['/a.json'], duplicateKeysHandling: 'bogus' as any }, + context, + ) as FileOverrideSource; + + expect(source.config.duplicateKeysHandling).toEqual('fail'); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('dataSystem.overrides.duplicateKeysHandling'), + ); + }); + + it('warns and raises a polling interval below the minimum', () => { + const source = createOverrideSource( + { type: 'file', paths: ['/a.json'], pollInterval: 0.1 }, + context, + ) as FileOverrideSource; + + expect(source.config.pollIntervalMs).toEqual(1000); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('dataSystem.overrides.pollInterval'), + ); + }); + + it('warns and uses the default for a polling interval that is not a number', () => { + const source = createOverrideSource( + { type: 'file', paths: ['/a.json'], pollInterval: '5' as any }, + context, + ) as FileOverrideSource; + + expect(source.config.pollIntervalMs).toEqual(1000); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('dataSystem.overrides.pollInterval'), + ); + }); + + it('warns and keeps the default parser for a YAML parser that is not a function', () => { + const source = createOverrideSource( + { type: 'file', paths: ['/a.json'], yamlParser: 'yaml' as any }, + context, + defaultYamlParser, + ) as FileOverrideSource; + + expect(source.config.yamlParser).toBe(defaultYamlParser); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('dataSystem.overrides.yamlParser'), + ); + }); + + it('returns a source object as is', () => { + const source = new TestOverrideSource(); + expect(createOverrideSource(source, context)).toBe(source); + }); + + it('calls a factory with the client context', () => { + const source = new TestOverrideSource(); + const factory = jest.fn(() => source); + expect(createOverrideSource(factory, context)).toBe(source); + expect(factory).toHaveBeenCalledWith(context); + }); + + it('rejects a configuration that is not a source', () => { + expect(() => createOverrideSource({ type: 'other' } as any, context)).toThrow( + 'Unsupported override source configuration', + ); + expect(() => createOverrideSource({ start: 'x' } as any, context)).toThrow( + 'Unsupported override source configuration', + ); + }); +}); + +it('rejects the file source on a platform without filesystem support', () => { + const context = makeContext(makeLogger(), false); + expect(() => createOverrideSource({ type: 'file', paths: ['/a.json'] }, context)).toThrow( + 'The file-based override source requires a platform with filesystem support', + ); +}); diff --git a/packages/shared/sdk-server/__tests__/overrides/fileOverrideSourcePolicy.test.ts b/packages/shared/sdk-server/__tests__/overrides/fileOverrideSourcePolicy.test.ts new file mode 100644 index 0000000000..f23229d61e --- /dev/null +++ b/packages/shared/sdk-server/__tests__/overrides/fileOverrideSourcePolicy.test.ts @@ -0,0 +1,64 @@ +import { Flag } from '../../src/evaluation/data/Flag'; +import { fileOverrideSourcePolicy, makeOverrideFlagWithValue } from '../../src/overrides'; + +// The policy is the one place where the file-based override source differs from the file data +// sources in how documents become data. + +it('expands a flag value into a flag that is off and serves the value with version 1', () => { + expect(makeOverrideFlagWithValue('flag', 'a')).toEqual({ + key: 'flag', + version: 1, + on: false, + offVariation: 0, + fallthrough: { variation: 0 }, + variations: ['a'], + }); + // The previous flag plays no part: an override snapshot has no version history. + const previous = { ...makeOverrideFlagWithValue('flag', 'old'), version: 9 }; + expect(fileOverrideSourcePolicy('fail').makeFlagWithValue('flag', 'a', previous).version).toBe(1); +}); + +it('fails the load on a duplicate key with the fail handling', () => { + const policy = fileOverrideSourcePolicy('fail'); + expect(() => policy.resolveDuplicateKey('flag', 'flag1')).toThrow( + "flag 'flag1' is specified by multiple files", + ); + expect(() => policy.resolveDuplicateKey('segment', 'seg1')).toThrow( + "segment 'seg1' is specified by multiple files", + ); +}); + +it('keeps the first entry on a duplicate key with the ignore handling', () => { + const policy = fileOverrideSourcePolicy('ignore'); + expect(policy.resolveDuplicateKey('flag', 'flag1')).toEqual('keepFirst'); + expect(policy.resolveDuplicateKey('segment', 'seg1')).toEqual('keepFirst'); +}); + +it('treats a configured file that does not exist as a file with no content', () => { + expect(fileOverrideSourcePolicy('fail').missingFile).toEqual('skip'); +}); + +it('parses by file extension and validates the document', () => { + const yamlParser = jest.fn(() => ({ flagValues: { flag: 'yaml' } })); + const policy = fileOverrideSourcePolicy('fail', yamlParser); + + expect(policy.parseDocument('data.yaml', 'flagValues:\n flag: yaml\n')).toEqual({ + flagValues: { flag: 'yaml' }, + }); + expect(yamlParser).toHaveBeenCalledTimes(1); + // A file without a YAML extension is JSON, whatever its content looks like. + expect(() => policy.parseDocument('data.json', 'flagValues:\n flag: yaml\n')).toThrow( + SyntaxError, + ); + expect(() => policy.parseDocument('data.json', '{"flags": []}')).toThrow( + '"flags" must be an object keyed by flag key', + ); + expect(policy.parseDocument('data.json', '{"flags": {"a": {"version": 1}}}')).toEqual({ + flags: { a: { key: 'a', version: 1 } }, + }); +}); + +it('keys a flag or segment entry by its map key', () => { + const policy = fileOverrideSourcePolicy('fail'); + expect(policy.entryKey('map-key', { key: 'own-key' } as Flag)).toEqual('map-key'); +}); diff --git a/packages/shared/sdk-server/__tests__/overrides/overrideDocument.test.ts b/packages/shared/sdk-server/__tests__/overrides/overrideDocument.test.ts new file mode 100644 index 0000000000..642fbffc9a --- /dev/null +++ b/packages/shared/sdk-server/__tests__/overrides/overrideDocument.test.ts @@ -0,0 +1,131 @@ +import { parseOverrideDocument } from '../../src/overrides/overrideDocument'; + +const flag1 = { + key: 'flag1', + on: true, + fallthrough: { variation: 0 }, + variations: [true], + version: 2, +}; +const segment1 = { key: 'segment1', included: ['user1'], version: 3 }; +const jsonDocument = JSON.stringify({ + flags: { flag1 }, + flagValues: { flag2: 'value' }, + segments: { segment1 }, +}); +const yamlDocument = 'flagValues:\n flag1: true\n'; + +describe('given the override document parser', () => { + it('parses a JSON document with flags, flag values, and segments', () => { + const document = parseOverrideDocument()('data.json', jsonDocument); + expect(document.flags).toEqual({ flag1 }); + expect(document.flagValues).toEqual({ flag2: 'value' }); + expect(document.segments).toEqual({ segment1 }); + }); + + it('parses a JSON document that only has some members', () => { + const document = parseOverrideDocument()( + 'data.json', + JSON.stringify({ flagValues: { flag2: 'value' } }), + ); + expect(document.flags).toBeUndefined(); + expect(document.flagValues).toEqual({ flag2: 'value' }); + expect(document.segments).toBeUndefined(); + }); + + it('treats an empty document as a document with no members', () => { + expect(parseOverrideDocument()('data.json', '{}')).toEqual({}); + }); + + it.each(['data.yml', 'data.yaml'])( + 'parses a file with a YAML extension using the parser: %s', + (path) => { + const yamlParser = jest.fn(() => ({ flagValues: { flag1: true } })); + const document = parseOverrideDocument(yamlParser)(path, yamlDocument); + expect(yamlParser).toHaveBeenCalledWith(yamlDocument); + expect(document.flagValues).toEqual({ flag1: true }); + }, + ); + + it('parses a file without a YAML extension as JSON and does not consult the YAML parser', () => { + const yamlParser = jest.fn(); + const document = parseOverrideDocument(yamlParser)( + 'data.txt', + ' \n{"flagValues": {"flag1": true}}', + ); + expect(yamlParser).not.toHaveBeenCalled(); + expect(document.flagValues).toEqual({ flag1: true }); + }); + + it('reports a JSON error for a file without a YAML extension whose content is YAML', () => { + const yamlParser = jest.fn(); + expect(() => parseOverrideDocument(yamlParser)('data.json', yamlDocument)).toThrow(SyntaxError); + expect(yamlParser).not.toHaveBeenCalled(); + }); + + it('always parses a file with a YAML extension as YAML, even when the content looks like JSON', () => { + const yamlParser = jest.fn(() => ({ flagValues: { flag1: true } })); + parseOverrideDocument(yamlParser)('data.yaml', '{"flagValues": {"flag1": true}}'); + expect(yamlParser).toHaveBeenCalledTimes(1); + }); + + it('reports a YAML file when no parser is available', () => { + expect(() => parseOverrideDocument()('data.yml', '')).toThrow( + 'Attempted to parse yaml file (data.yml) without parser.', + ); + }); + + it('treats a parser result of null or undefined as an empty document', () => { + expect(parseOverrideDocument(() => null)('empty.yaml', '')).toEqual({}); + expect(parseOverrideDocument(() => undefined)('empty.yaml', '')).toEqual({}); + }); + + it('rejects malformed JSON', () => { + expect(() => parseOverrideDocument()('data.json', '{"flagValues"')).toThrow(/json/i); + }); + + it('rejects a document that is not an object', () => { + expect(() => parseOverrideDocument(() => ['a'])('data.yaml', '- a\n')).toThrow( + 'file data must be an object', + ); + expect(() => parseOverrideDocument(() => 'text')('data.yaml', 'text')).toThrow( + 'file data must be an object', + ); + }); + + it('rejects members that are not objects keyed by item key', () => { + const parse = parseOverrideDocument(); + expect(() => parse('data.json', '{"flags": []}')).toThrow( + '"flags" must be an object keyed by flag key', + ); + expect(() => parse('data.json', '{"segments": "x"}')).toThrow( + '"segments" must be an object keyed by segment key', + ); + expect(() => parse('data.json', '{"flagValues": [1]}')).toThrow( + '"flagValues" must be an object keyed by flag key', + ); + }); + + it('rejects a flag or segment entry that is not an object', () => { + const parse = parseOverrideDocument(); + expect(() => parse('data.json', '{"flags": {"flag1": "x"}}')).toThrow( + 'flag "flag1" must be an object', + ); + expect(() => parse('data.json', '{"segments": {"segment1": 7}}')).toThrow( + 'segment "segment1" must be an object', + ); + }); + + it('fills a missing entry key from the map key and keeps an existing key', () => { + const document = parseOverrideDocument()( + 'data.json', + JSON.stringify({ + flags: { 'flag-a': { on: false, version: 1 }, 'flag-b': { key: 'other', version: 1 } }, + segments: { 'segment-a': { version: 1 } }, + }), + ); + expect(document.flags?.['flag-a'].key).toEqual('flag-a'); + expect(document.flags?.['flag-b'].key).toEqual('other'); + expect(document.segments?.['segment-a'].key).toEqual('segment-a'); + }); +}); diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 961813aa5d..843c363d96 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -290,6 +290,7 @@ function constructFDv2( instanceId: string | undefined, userAgentHeaderName: 'user-agent' | 'x-launchdarkly-user-agent' | undefined, startEventProcessor: boolean, + defaultYamlParser: ((data: string) => any) | undefined, ): { config: Configuration; logger: LDLogger | undefined; @@ -337,7 +338,7 @@ function constructFDv2( // fails client construction before a persistent store has opened a connection. const overrideSource = dataSystem.overrides !== undefined && !config.offline - ? createOverrideSource(dataSystem.overrides, clientContext) + ? createOverrideSource(dataSystem.overrides, clientContext, defaultYamlParser) : undefined; const featureStore = dataSystem.featureStoreFactory(clientContext); @@ -796,6 +797,7 @@ export default class LDClientImpl implements LDClient { internalOptions?.instanceId, internalOptions?.userAgentHeaderName, startEventProcessor, + internalOptions?.yamlParser, )); this._featureStore = transactionalStore; this.bigSegmentStatusProviderInternal = this._bigSegmentsManager diff --git a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts index 23a110e867..21c54ff3e7 100644 --- a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts +++ b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts @@ -99,15 +99,87 @@ export interface LDDataSystemOptions { } /** - * The ways an override source can be configured: the source itself, or a factory function that - * creates it from the client context. + * Configuration of the file-based override source. Flag overrides are currently experimental and + * subject to change. + * + * The source reads flag and segment overrides from one or more local files and reloads them as + * the files change. The files use the same document format as the file data source: a JSON or + * YAML document with optional `flags`, `flagValues`, and `segments` members. `flagValues` entries + * expand into full flag definitions that return the given value for every context. + * + * A configured file that does not exist contributes no overrides. It can be created later, and + * deleting it removes its overrides. A file that exists but cannot be read or parsed fails that + * reload: the previously loaded overrides stay in effect, the failure is logged, and the source + * retries. Every applied change is logged at Info level with the overrides in effect and what + * each file supplied. + * + * @example + * ```typescript + * const client = init(sdkKey, { + * dataSystem: { + * overrides: { type: 'file', paths: ['/etc/launchdarkly/overrides.json'] }, + * }, + * }); + * ``` + */ +export interface FileOverrideSourceOptions { + type: 'file'; + + /** + * The paths of the files to read, in precedence order. At least one path is required. The + * order decides which file wins under the duplicate keys handling when the same key appears in + * more than one file. + */ + paths: string[]; + + /** + * What to do when the same flag or segment key appears in more than one file. `fail`, the + * default, treats the reload as failed and keeps the previously loaded overrides. `ignore` + * keeps the entry from the first configured file that defines the key and discards the others. + */ + duplicateKeysHandling?: 'fail' | 'ignore'; + + /** + * How the source detects file changes. The two modes are alternatives. + * + * `polling`, the default, examines the files on a fixed interval and reloads when the + * modification time or the size of a file changes. It works on every filesystem, including + * network mounts and directories whose contents are swapped through symbolic links. + * + * `watching` reloads in response to filesystem change notifications for the directories that + * contain the files. It reacts faster than polling. It depends on notifications, which some + * filesystems do not deliver reliably. + */ + changeDetection?: 'polling' | 'watching'; + + /** + * The interval between examinations of the files in polling mode, in seconds. The default is + * 1. An interval below 1 is raised to 1. Watching mode ignores it. + */ + pollInterval?: number; + + /** + * A YAML parser for YAML files. The parser must produce the same structure as `JSON.parse`. + * The Node.js SDK supplies one by default. Other platforms need one to read YAML files. + */ + yamlParser?: (data: string) => any; +} + +/** + * The ways an override source can be configured: the file-based source, a source object, or a + * factory function that creates a source from the client context. * * Flag overrides are currently experimental and subject to change. */ export type LDOverrideSourceOptions = + | FileOverrideSourceOptions | LDOverrideSource | ((clientContext: LDClientContext) => LDOverrideSource); +export function isFileOverrideSourceOptions(u: any): u is FileOverrideSourceOptions { + return typeof u === 'object' && u !== null && u.type === 'file'; +} + /** * Configuration options for the FDv1 Fallback Synchronizer. */ diff --git a/packages/shared/sdk-server/src/options/ServerInternalOptions.ts b/packages/shared/sdk-server/src/options/ServerInternalOptions.ts index 1583fe9191..ac60d751c2 100644 --- a/packages/shared/sdk-server/src/options/ServerInternalOptions.ts +++ b/packages/shared/sdk-server/src/options/ServerInternalOptions.ts @@ -25,4 +25,11 @@ export interface ServerInternalOptions extends internal.LDInternalOptions { * graph in memory, which for a per-request client is a leak. */ disableBackgroundEventFlush?: boolean; + + /** + * A YAML parser that the platform supplies for file based components, used when the + * application does not configure one. The parser must produce the same structure as + * `JSON.parse`. The Node.js SDK supplies one. Platforms without one cannot read YAML files. + */ + yamlParser?: (data: string) => any; } diff --git a/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts b/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts new file mode 100644 index 0000000000..5d14a5a5ec --- /dev/null +++ b/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts @@ -0,0 +1,262 @@ +import { Filesystem, LDLogger } from '@launchdarkly/js-sdk-common'; + +import { LDOverrideSink, LDOverrideSource } from '../api/subsystems'; +import { + DEFAULT_DEBOUNCE_DELAY_MS, + DEFAULT_RETRY_DELAY_MS, + FileDataPolicy, + FileDirectoryWatcher, + FilePoller, + FileReloader, + LoadFailure, + ReloadResult, + YamlParser, +} from '../data_sources/filedata'; +import { Flag } from '../evaluation/data/Flag'; +import { parseOverrideDocument } from './overrideDocument'; + +/** + * How the file-based override source learns that a file changed. + * + * @internal + */ +export type FileChangeDetection = 'polling' | 'watching'; + +/** + * What the file-based override source does when the same key appears in more than one file. + * `fail` treats the reload as failed and keeps the previously loaded overrides. `ignore` keeps the + * entry from the first configured file that defines the key. + * + * @internal + */ +export type OverrideDuplicateKeysHandling = 'fail' | 'ignore'; + +/** + * The interval, in seconds, at which the file-based override source examines the files for + * changes in polling mode when no interval was specified. The source reads local files rather + * than contacting a service, so a short interval keeps an override responsive during an incident + * at negligible cost. + * + * @internal + */ +export const DEFAULT_POLL_INTERVAL_SECONDS = 1; + +/** + * The shortest allowed polling interval, in seconds. A configured interval below it is raised to + * it. The minimum exists only to prevent a tight loop over the filesystem. + * + * @internal + */ +export const MINIMUM_POLL_INTERVAL_SECONDS = 1; + +/** + * The validated configuration of a file-based override source. + * + * @internal + */ +export interface FileOverrideSourceConfig { + /** + * The files to load, in precedence order. + */ + paths: string[]; + duplicateKeysHandling: OverrideDuplicateKeysHandling; + changeDetection: FileChangeDetection; + /** + * The interval between examinations of the files in polling mode. + */ + pollIntervalMs: number; + yamlParser?: YamlParser; +} + +/** + * Expands a flag value into a full flag definition that returns the given value for every + * context. The flag is off and serves its single variation as the off variation, so it + * evaluates with the OFF reason. + * + * @internal + */ +export function makeOverrideFlagWithValue(key: string, value: any): Flag { + return { + key, + version: 1, + on: false, + offVariation: 0, + fallthrough: { variation: 0 }, + variations: [value], + }; +} + +/** + * How the file-based override source translates its documents into data. This is the one place + * where its rules differ from the file data sources. + * + * - The parser is chosen by file extension, as for the file data sources, and the document + * is validated. + * - A flag or segment entry is keyed by its map key, as the Go SDK's override source keys it. + * - A `flagValues` entry becomes a flag that is off and serves the value, with version 1. + * - A key that appears more than once fails the load with the `fail` handling, or keeps the + * first configured file's entry with the `ignore` handling. + * - A configured file that does not exist contributes no entries. + * + * @internal + */ +export function fileOverrideSourcePolicy( + duplicateKeysHandling: OverrideDuplicateKeysHandling, + yamlParser?: YamlParser, +): FileDataPolicy { + return { + parseDocument: parseOverrideDocument(yamlParser), + makeFlagWithValue: (key, value) => makeOverrideFlagWithValue(key, value), + resolveDuplicateKey: (category, key) => { + if (duplicateKeysHandling === 'ignore') { + return 'keepFirst'; + } + throw new Error(`${category} '${key}' is specified by multiple files`); + }, + missingFile: 'skip', + entryKey: (mapKey) => mapKey, + }; +} + +function pluralize(count: number, noun: string): string { + return count === 1 ? `1 ${noun}` : `${count} ${noun}s`; +} + +/** + * Formats flag and segment counts, for example "2 flags, 1 segment". + */ +function countsText(flags: number, segments: number): string { + const parts: string[] = []; + if (flags > 0) { + parts.push(pluralize(flags, 'flag')); + } + if (segments > 0) { + parts.push(pluralize(segments, 'segment')); + } + return parts.join(', '); +} + +/** + * An override source that reads flag and segment overrides from one or more local files and + * reloads them as the files change. + * + * The files use the same document format as the file data sources: a JSON or YAML document with + * optional `flags`, `flagValues`, and `segments` members. `flagValues` entries expand into full + * flag definitions that return the given value for every context. When several files are + * configured, their entries are combined in the configured order, and the duplicate keys + * handling decides what happens when the same key appears in more than one file. + * + * A reload replaces the entire override set. A configured file that does not exist contributes + * no overrides, so deleting a file removes its overrides. A file that exists but cannot be read + * or parsed makes that whole reload fail. The previously loaded overrides stay in effect, the + * failure is logged, and the source retries after a short delay. Every applied change is logged + * at Info level with the overrides in effect and what each file supplied. + * + * Flag overrides are currently experimental and subject to change. + * + * @internal + */ +export default class FileOverrideSource implements LDOverrideSource { + private _reloader?: FileReloader; + + private _poller?: FilePoller; + + private _watcher?: FileDirectoryWatcher; + + constructor( + public readonly config: FileOverrideSourceConfig, + private readonly _filesystem: Filesystem, + private readonly _logger?: LDLogger, + ) {} + + /** + * Sets up change detection and performs the initial load. The returned promise settles when + * the initial load has completed. A file that does not exist yet contributes no overrides. A + * file that cannot be read or parsed is not fatal: the failure is logged, and the retry plus + * the change signal recover once the file is readable. + */ + async start(sink: LDOverrideSink): Promise { + const { paths, duplicateKeysHandling, changeDetection, pollIntervalMs, yamlParser } = + this.config; + this._reloader = new FileReloader({ + paths, + policy: fileOverrideSourcePolicy(duplicateKeysHandling, yamlParser), + filesystem: this._filesystem, + logger: this._logger, + apply: (result) => { + sink.setOverrides( + result.flags.map((flag) => flag.item), + result.segments.map((segment) => segment.item), + ); + this._logOverridesInEffect(result); + if (result.files.some((file) => !file.present)) { + // A missing file contributes nothing, so its load succeeds. It can also mean that the + // file's directory is gone. Let the watcher check its directories. + this._watcher?.verify(); + } + }, + onFailure: (failure) => this._handleFailure(failure), + debounceDelayMs: DEFAULT_DEBOUNCE_DELAY_MS, + retryDelayMs: DEFAULT_RETRY_DELAY_MS, + skipUnchanged: true, + }); + const trigger = () => this._reloader?.trigger(); + + // Change detection is in place before the initial load, so a change made between the two + // is not missed. The reloader absorbs the redundant reload this can cause. + if (changeDetection === 'watching') { + this._watcher = new FileDirectoryWatcher(this._filesystem, paths, trigger, this._logger); + this._watcher.start(); + } else { + this._poller = new FilePoller(this._filesystem, paths, pollIntervalMs, trigger); + await this._poller.start(); + } + + // A close during the load stops the poller or watcher and closes the reloader, so a load + // that completes afterward delivers nothing. + await this._reloader.reloadNow(); + } + + close(): void { + this._watcher?.close(); + this._poller?.close(); + this._reloader?.close(); + } + + /** + * Reports a failed load. A persistent failure that automatic retries keep hitting is reported + * once at error level and then at debug level. + */ + private _handleFailure(failure: LoadFailure): void { + const where = failure.path ? ` [${failure.path}]` : ''; + const message = `Unable to load flags: ${failure.error.message}${where}`; + if (failure.repeated) { + this._logger?.debug(message); + return; + } + this._logger?.error(message); + } + + /** + * Reports the overrides now in effect and what each file supplied. The reloader applies a + * snapshot only when the content changed, so this logs each change once. + */ + private _logOverridesInEffect(result: ReloadResult): void { + const details = result.files.map((file) => { + if (!file.present) { + return `${file.path}: absent`; + } + if (file.flags === 0 && file.segments === 0) { + return `${file.path}: no entries`; + } + return `${file.path}: ${countsText(file.flags, file.segments)}`; + }); + if (result.flags.length === 0 && result.segments.length === 0) { + this._logger?.info(`Flag overrides: none in effect (${details.join('; ')})`); + return; + } + this._logger?.info( + `Flag overrides in effect: ${countsText(result.flags.length, result.segments.length)} (${details.join('; ')})`, + ); + } +} diff --git a/packages/shared/sdk-server/src/overrides/createOverrideSource.ts b/packages/shared/sdk-server/src/overrides/createOverrideSource.ts index e43d99e7fe..a6ba24c897 100644 --- a/packages/shared/sdk-server/src/overrides/createOverrideSource.ts +++ b/packages/shared/sdk-server/src/overrides/createOverrideSource.ts @@ -1,7 +1,19 @@ -import { LDClientContext, TypeValidators } from '@launchdarkly/js-sdk-common'; +import { LDClientContext, OptionMessages, TypeValidators } from '@launchdarkly/js-sdk-common'; -import { LDOverrideSourceOptions } from '../api/options/LDDataSystemOptions'; +import { + FileOverrideSourceOptions, + isFileOverrideSourceOptions, + LDOverrideSourceOptions, +} from '../api/options/LDDataSystemOptions'; import { LDOverrideSource } from '../api/subsystems'; +import { YamlParser } from '../data_sources/filedata'; +import FileOverrideSource, { + DEFAULT_POLL_INTERVAL_SECONDS, + FileChangeDetection, + FileOverrideSourceConfig, + MINIMUM_POLL_INTERVAL_SECONDS, + OverrideDuplicateKeysHandling, +} from './FileOverrideSource'; function isOverrideSource(u: unknown): u is LDOverrideSource { const candidate = u as Partial | undefined; @@ -10,6 +22,102 @@ function isOverrideSource(u: unknown): u is LDOverrideSource { ); } +const duplicateKeysHandlingValues: OverrideDuplicateKeysHandling[] = ['fail', 'ignore']; +const changeDetectionValues: FileChangeDetection[] = ['polling', 'watching']; + +/** + * Validates the options of the file-based override source. A configuration error, such as no + * file paths or an unrecognized change detection mode, throws. A value that has a safe default, + * such as an unrecognized duplicate keys handling or a polling interval below the minimum, is + * logged and replaced. + */ +function validateFileOverrideSourceOptions( + options: FileOverrideSourceOptions, + clientContext: LDClientContext, + defaultYamlParser?: YamlParser, +): FileOverrideSourceConfig { + const { logger } = clientContext.basicConfiguration; + if ( + !TypeValidators.StringArray.is(options.paths) || + options.paths.length === 0 || + options.paths.some((path) => path === '') + ) { + throw new Error('The file-based override source requires at least one file path'); + } + + let duplicateKeysHandling: OverrideDuplicateKeysHandling = 'fail'; + if (options.duplicateKeysHandling !== undefined) { + if (duplicateKeysHandlingValues.includes(options.duplicateKeysHandling)) { + duplicateKeysHandling = options.duplicateKeysHandling; + } else { + logger?.warn( + OptionMessages.wrongOptionType( + 'dataSystem.overrides.duplicateKeysHandling', + duplicateKeysHandlingValues.map((value) => `'${value}'`).join(' | '), + String(options.duplicateKeysHandling), + ), + ); + } + } + + let changeDetection: FileChangeDetection = 'polling'; + if (options.changeDetection !== undefined) { + if (!changeDetectionValues.includes(options.changeDetection)) { + throw new Error( + `Unrecognized change detection mode "${options.changeDetection}" for the file-based override source`, + ); + } + changeDetection = options.changeDetection; + } + + let pollIntervalSeconds = DEFAULT_POLL_INTERVAL_SECONDS; + if (options.pollInterval !== undefined) { + if (!TypeValidators.Number.is(options.pollInterval)) { + logger?.warn( + OptionMessages.wrongOptionType( + 'dataSystem.overrides.pollInterval', + 'number', + typeof options.pollInterval, + ), + ); + } else if (options.pollInterval < MINIMUM_POLL_INTERVAL_SECONDS) { + logger?.warn( + OptionMessages.optionBelowMinimum( + 'dataSystem.overrides.pollInterval', + options.pollInterval, + MINIMUM_POLL_INTERVAL_SECONDS, + ), + ); + pollIntervalSeconds = MINIMUM_POLL_INTERVAL_SECONDS; + } else { + pollIntervalSeconds = options.pollInterval; + } + } + + let yamlParser = defaultYamlParser; + if (options.yamlParser !== undefined) { + if (TypeValidators.Function.is(options.yamlParser)) { + yamlParser = options.yamlParser; + } else { + logger?.warn( + OptionMessages.wrongOptionType( + 'dataSystem.overrides.yamlParser', + 'function', + typeof options.yamlParser, + ), + ); + } + } + + return { + paths: options.paths, + duplicateKeysHandling, + changeDetection, + pollIntervalMs: pollIntervalSeconds * 1000, + yamlParser, + }; +} + /** * Creates the override source described by the data system options. A configuration that does * not describe a source is an error, reported the way an unsupported data source configuration @@ -20,10 +128,22 @@ function isOverrideSource(u: unknown): u is LDOverrideSource { export default function createOverrideSource( options: LDOverrideSourceOptions, clientContext: LDClientContext, + defaultYamlParser?: YamlParser, ): LDOverrideSource { if (TypeValidators.Function.is(options)) { return (options as (clientContext: LDClientContext) => LDOverrideSource)(clientContext); } + if (isFileOverrideSourceOptions(options)) { + const { fileSystem } = clientContext.platform; + if (!fileSystem) { + throw new Error('The file-based override source requires a platform with filesystem support'); + } + return new FileOverrideSource( + validateFileOverrideSourceOptions(options, clientContext, defaultYamlParser), + fileSystem, + clientContext.basicConfiguration.logger, + ); + } if (isOverrideSource(options)) { return options; } diff --git a/packages/shared/sdk-server/src/overrides/index.ts b/packages/shared/sdk-server/src/overrides/index.ts index ae61223012..6e40b06ee5 100644 --- a/packages/shared/sdk-server/src/overrides/index.ts +++ b/packages/shared/sdk-server/src/overrides/index.ts @@ -1,13 +1,30 @@ import createOverrideSource from './createOverrideSource'; +import FileOverrideSource, { + DEFAULT_POLL_INTERVAL_SECONDS, + FileChangeDetection, + FileOverrideSourceConfig, + fileOverrideSourcePolicy, + makeOverrideFlagWithValue, + MINIMUM_POLL_INTERVAL_SECONDS, + OverrideDuplicateKeysHandling, +} from './FileOverrideSource'; import OverrideLayer, { LayerContents, LayerKindContents } from './OverrideLayer'; import OverrideSink from './OverrideSink'; import ReadStoreOverlay, { ReadStore } from './ReadStoreOverlay'; export { createOverrideSource, + DEFAULT_POLL_INTERVAL_SECONDS, + FileChangeDetection, + FileOverrideSource, + FileOverrideSourceConfig, + fileOverrideSourcePolicy, LayerContents, LayerKindContents, OverrideLayer, + makeOverrideFlagWithValue, + MINIMUM_POLL_INTERVAL_SECONDS, + OverrideDuplicateKeysHandling, OverrideSink, ReadStore, ReadStoreOverlay, diff --git a/packages/shared/sdk-server/src/overrides/overrideDocument.ts b/packages/shared/sdk-server/src/overrides/overrideDocument.ts new file mode 100644 index 0000000000..004c409190 --- /dev/null +++ b/packages/shared/sdk-server/src/overrides/overrideDocument.ts @@ -0,0 +1,75 @@ +import { + DocumentParser, + FileDataDocument, + parseDocumentByExtension, + YamlParser, +} from '../data_sources/filedata'; + +function isPlainObject(value: any): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** + * Checks that a member of the document is an object keyed by item key, and that every entry + * in it is an object. The key of an entry is filled from the map key when the entry omits it. + */ +function validateItems(document: Record, member: 'flags' | 'segments'): void { + const items = document[member]; + if (items === undefined || items === null) { + return; + } + if (!isPlainObject(items)) { + throw new Error(`"${member}" must be an object keyed by ${member.slice(0, -1)} key`); + } + Object.entries(items).forEach(([key, item]) => { + if (!isPlainObject(item)) { + throw new Error(`${member.slice(0, -1)} "${key}" must be an object`); + } + if (item.key === undefined) { + item.key = key; + } + }); +} + +function validateDocument(parsed: any): FileDataDocument { + if (parsed === undefined || parsed === null) { + // An empty file is an empty document. + return {}; + } + if (!isPlainObject(parsed)) { + throw new Error('file data must be an object'); + } + validateItems(parsed, 'flags'); + validateItems(parsed, 'segments'); + const { flagValues } = parsed; + if (flagValues !== undefined && flagValues !== null && !isPlainObject(flagValues)) { + throw new Error('"flagValues" must be an object keyed by flag key'); + } + const document: FileDataDocument = {}; + if (parsed.flags) { + document.flags = parsed.flags; + } + if (flagValues) { + document.flagValues = flagValues; + } + if (parsed.segments) { + document.segments = parsed.segments; + } + return document; +} + +/** + * The document parser of the file-based override source. The format is chosen by file extension, + * as for the file data sources: a `.yml` or `.yaml` file uses the YAML parser and every other file + * is JSON. YAML needs a parser. Without one, a YAML file is an error. + * + * The result is validated: the document and its `flags`, `flagValues`, and `segments` members + * must be objects, every flag and segment entry must be an object, and an entry that omits its + * `key` gets it from the map key. An empty document is a document with no members. + * + * @internal + */ +export function parseOverrideDocument(yamlParser?: YamlParser): DocumentParser { + const parse = parseDocumentByExtension(yamlParser); + return (path, data) => validateDocument(parse(path, data)); +} From 2fdac52c89d35b1cbee3a5b4b6e399365beaeb43 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:18:03 +0000 Subject: [PATCH 2/9] fix: Expand value-only overrides to a flag served by fallthrough --- .../__tests__/LDClientImpl.fileOverrides.test.ts | 2 +- .../__tests__/overrides/FileOverrideSource.test.ts | 6 ++++-- .../overrides/fileOverrideSourcePolicy.test.ts | 5 ++--- .../sdk-server/src/overrides/FileOverrideSource.ts | 10 +++++----- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts index e8cfa9de0a..4c1b1ad798 100644 --- a/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts @@ -79,7 +79,7 @@ describe('given a client with a file override source over a mock filesystem', () const detail = await client!.boolVariationDetail('overridden-flag', user, false); expect(detail.value).toBe(true); - expect(detail.reason).toEqual({ kind: 'OFF', overrideAffected: true }); + expect(detail.reason).toEqual({ kind: 'FALLTHROUGH', overrideAffected: true }); expect(client!.initialized()).toBe(false); }); diff --git a/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts b/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts index 06182dc5e8..b6068ee055 100644 --- a/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts @@ -76,10 +76,12 @@ describe('given a file override source over a mock filesystem', () => { expect(sink.snapshots).toHaveLength(1); expect(sink.flagKeys()).toEqual(['flag1', 'flag2']); - // The flag value entry was expanded into a full flag definition that is off. + // The flag value entry was expanded into a full flag definition that is on and serves the + // value by fallthrough. const flag1 = sink.last.flags.find((flag) => flag.key === 'flag1')!; expect(flag1.variations).toEqual([true]); - expect(flag1.on).toBe(false); + expect(flag1.on).toBe(true); + expect(flag1.fallthrough).toEqual({ variation: 0 }); expect(sink.last.flags.find((flag) => flag.key === 'flag2')!.version).toEqual(3); }); diff --git a/packages/shared/sdk-server/__tests__/overrides/fileOverrideSourcePolicy.test.ts b/packages/shared/sdk-server/__tests__/overrides/fileOverrideSourcePolicy.test.ts index f23229d61e..766119bbfd 100644 --- a/packages/shared/sdk-server/__tests__/overrides/fileOverrideSourcePolicy.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/fileOverrideSourcePolicy.test.ts @@ -4,12 +4,11 @@ import { fileOverrideSourcePolicy, makeOverrideFlagWithValue } from '../../src/o // The policy is the one place where the file-based override source differs from the file data // sources in how documents become data. -it('expands a flag value into a flag that is off and serves the value with version 1', () => { +it('expands a flag value into a flag that is on and serves the value by fallthrough with version 1', () => { expect(makeOverrideFlagWithValue('flag', 'a')).toEqual({ key: 'flag', version: 1, - on: false, - offVariation: 0, + on: true, fallthrough: { variation: 0 }, variations: ['a'], }); diff --git a/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts b/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts index 5d14a5a5ec..4c1a8e334b 100644 --- a/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts +++ b/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts @@ -70,8 +70,8 @@ export interface FileOverrideSourceConfig { /** * Expands a flag value into a full flag definition that returns the given value for every - * context. The flag is off and serves its single variation as the off variation, so it - * evaluates with the OFF reason. + * context. The flag is on, has the value as its only variation, and serves that variation as + * its fallthrough, so it evaluates with the FALLTHROUGH reason. * * @internal */ @@ -79,8 +79,7 @@ export function makeOverrideFlagWithValue(key: string, value: any): Flag { return { key, version: 1, - on: false, - offVariation: 0, + on: true, fallthrough: { variation: 0 }, variations: [value], }; @@ -93,7 +92,8 @@ export function makeOverrideFlagWithValue(key: string, value: any): Flag { * - The parser is chosen by file extension, as for the file data sources, and the document * is validated. * - A flag or segment entry is keyed by its map key, as the Go SDK's override source keys it. - * - A `flagValues` entry becomes a flag that is off and serves the value, with version 1. + * - A `flagValues` entry becomes a flag that is on and serves the value by fallthrough, with + * version 1. * - A key that appears more than once fails the load with the `fail` handling, or keeps the * first configured file's entry with the `ignore` handling. * - A configured file that does not exist contributes no entries. From e3d7474f45f70e1355cda5558a091bf0c59a0e89 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:28:11 +0000 Subject: [PATCH 3/9] fix: Treat a polling interval that a timer cannot honor as invalid and use the default --- .../overrides/createOverrideSource.test.ts | 19 +++++++++++++++++-- .../src/overrides/FileOverrideSource.ts | 10 ++++++++++ .../src/overrides/createOverrideSource.ts | 13 +++++++++++++ .../shared/sdk-server/src/overrides/index.ts | 2 ++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/overrides/createOverrideSource.test.ts b/packages/shared/sdk-server/__tests__/overrides/createOverrideSource.test.ts index 4a6ce08b4a..6a4ded9abe 100644 --- a/packages/shared/sdk-server/__tests__/overrides/createOverrideSource.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/createOverrideSource.test.ts @@ -106,9 +106,9 @@ describe('given a client context with filesystem support', () => { ); }); - it('warns and raises a polling interval below the minimum', () => { + it.each([0.1, -5])('warns and raises a polling interval of %s to the minimum', (pollInterval) => { const source = createOverrideSource( - { type: 'file', paths: ['/a.json'], pollInterval: 0.1 }, + { type: 'file', paths: ['/a.json'], pollInterval }, context, ) as FileOverrideSource; @@ -118,6 +118,21 @@ describe('given a client context with filesystem support', () => { ); }); + it.each([NaN, Infinity, 3000000])( + 'warns and uses the default for a polling interval of %s, which a timer cannot honor', + (pollInterval) => { + const source = createOverrideSource( + { type: 'file', paths: ['/a.json'], pollInterval }, + context, + ) as FileOverrideSource; + + expect(source.config.pollIntervalMs).toEqual(1000); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('dataSystem.overrides.pollInterval'), + ); + }, + ); + it('warns and uses the default for a polling interval that is not a number', () => { const source = createOverrideSource( { type: 'file', paths: ['/a.json'], pollInterval: '5' as any }, diff --git a/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts b/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts index 4c1a8e334b..669fbae6d6 100644 --- a/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts +++ b/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts @@ -49,6 +49,16 @@ export const DEFAULT_POLL_INTERVAL_SECONDS = 1; */ export const MINIMUM_POLL_INTERVAL_SECONDS = 1; +/** + * The longest allowed polling interval, in seconds. The platform's timers do not honor a delay + * above 2147483647 milliseconds and run it after a short delay instead, which would make the + * poller loop over the filesystem. A configured interval above the maximum, or one that is not a + * finite number, is invalid and replaced by the default. + * + * @internal + */ +export const MAXIMUM_POLL_INTERVAL_SECONDS = 2147483; + /** * The validated configuration of a file-based override source. * diff --git a/packages/shared/sdk-server/src/overrides/createOverrideSource.ts b/packages/shared/sdk-server/src/overrides/createOverrideSource.ts index a6ba24c897..c0f5287cf6 100644 --- a/packages/shared/sdk-server/src/overrides/createOverrideSource.ts +++ b/packages/shared/sdk-server/src/overrides/createOverrideSource.ts @@ -11,6 +11,7 @@ import FileOverrideSource, { DEFAULT_POLL_INTERVAL_SECONDS, FileChangeDetection, FileOverrideSourceConfig, + MAXIMUM_POLL_INTERVAL_SECONDS, MINIMUM_POLL_INTERVAL_SECONDS, OverrideDuplicateKeysHandling, } from './FileOverrideSource'; @@ -80,6 +81,18 @@ function validateFileOverrideSourceOptions( typeof options.pollInterval, ), ); + } else if ( + !Number.isFinite(options.pollInterval) || + options.pollInterval > MAXIMUM_POLL_INTERVAL_SECONDS + ) { + // A timer does not honor such a delay, so the value is invalid rather than clamped. + logger?.warn( + OptionMessages.wrongOptionType( + 'dataSystem.overrides.pollInterval', + `number of seconds up to ${MAXIMUM_POLL_INTERVAL_SECONDS}`, + String(options.pollInterval), + ), + ); } else if (options.pollInterval < MINIMUM_POLL_INTERVAL_SECONDS) { logger?.warn( OptionMessages.optionBelowMinimum( diff --git a/packages/shared/sdk-server/src/overrides/index.ts b/packages/shared/sdk-server/src/overrides/index.ts index 6e40b06ee5..860f8bba97 100644 --- a/packages/shared/sdk-server/src/overrides/index.ts +++ b/packages/shared/sdk-server/src/overrides/index.ts @@ -5,6 +5,7 @@ import FileOverrideSource, { FileOverrideSourceConfig, fileOverrideSourcePolicy, makeOverrideFlagWithValue, + MAXIMUM_POLL_INTERVAL_SECONDS, MINIMUM_POLL_INTERVAL_SECONDS, OverrideDuplicateKeysHandling, } from './FileOverrideSource'; @@ -23,6 +24,7 @@ export { LayerKindContents, OverrideLayer, makeOverrideFlagWithValue, + MAXIMUM_POLL_INTERVAL_SECONDS, MINIMUM_POLL_INTERVAL_SECONDS, OverrideDuplicateKeysHandling, OverrideSink, From 6addc74a1ef23241a1e48889501071763fa21ae7 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:28:11 +0000 Subject: [PATCH 4/9] test: Expect the fallthrough reason for a value-only YAML override --- .../server-node/__tests__/LDClientNode.fileOverrides.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts b/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts index a191b0801f..a7b6e783dc 100644 --- a/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts +++ b/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts @@ -68,7 +68,7 @@ describe('given a temporary directory of override files', () => { const detail = await client.variationDetail('yaml-flag', user, 'default'); expect(detail.value).toEqual('override-value'); - expect(detail.reason).toEqual({ kind: 'OFF', overrideAffected: true }); + expect(detail.reason).toEqual({ kind: 'FALLTHROUGH', overrideAffected: true }); expect(client.initialized()).toBe(false); }); From c8a3f352b3b93a280dfd2bf9559ea6b57c051135 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:39:51 +0000 Subject: [PATCH 5/9] docs: Describe the invalid polling interval values that fall back to the default --- .../shared/sdk-server/src/api/options/LDDataSystemOptions.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts index 21c54ff3e7..788c00b916 100644 --- a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts +++ b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts @@ -154,7 +154,8 @@ export interface FileOverrideSourceOptions { /** * The interval between examinations of the files in polling mode, in seconds. The default is - * 1. An interval below 1 is raised to 1. Watching mode ignores it. + * 1. An interval below 1 is raised to 1. A value that is not a finite number, or that exceeds + * 2147483 seconds, is invalid and the default is used. Watching mode ignores it. */ pollInterval?: number; From 673127da2f6ff09a2b9651a0ecf863edb30caf54 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:48:11 +0000 Subject: [PATCH 6/9] test: Wait for the initial override load before asserting on the file source Evaluation no longer waits for the override source's initial load, so the tests that evaluated right after creating the client now wait until the override shows, as the tests of later changes already did. --- .../LDClientImpl.fileOverrides.test.ts | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts index 4c1b1ad798..328a08fe36 100644 --- a/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts @@ -76,6 +76,11 @@ describe('given a client with a file override source over a mock filesystem', () filesystem.set(jsonPath, '{"flagValues": {"overridden-flag": true}}'); makeClient({ paths: [jsonPath] }); + // The initial load reads the file asynchronously and evaluation does not wait for it, so the + // test waits until the override shows. + await waitFor( + async () => (await client!.boolVariation('overridden-flag', user, false)) === true, + ); const detail = await client!.boolVariationDetail('overridden-flag', user, false); expect(detail.value).toBe(true); @@ -87,9 +92,10 @@ describe('given a client with a file override source over a mock filesystem', () filesystem.set(yamlPath, 'flagValues:\n yaml-flag: from-platform-parser\n'); makeClient({ paths: [yamlPath] }); - const value = await client!.variation('yaml-flag', user, 'default'); - - expect(value).toEqual('from-platform-parser'); + await waitFor( + async () => + (await client!.variation('yaml-flag', user, 'default')) === 'from-platform-parser', + ); expect(platformYamlParser).toHaveBeenCalledWith( 'flagValues:\n yaml-flag: from-platform-parser\n', ); @@ -100,14 +106,19 @@ describe('given a client with a file override source over a mock filesystem', () filesystem.set(yamlPath, 'flagValues:\n yaml-flag: x\n'); makeClient({ paths: [yamlPath], yamlParser }); - expect(await client!.variation('yaml-flag', user, 'default')).toEqual('from-configured-parser'); + await waitFor( + async () => + (await client!.variation('yaml-flag', user, 'default')) === 'from-configured-parser', + ); expect(platformYamlParser).not.toHaveBeenCalled(); }); it('reloads when the directory reports a change', async () => { filesystem.set(jsonPath, '{"flagValues": {"overridden-flag": "b"}}'); makeClient({ paths: [jsonPath] }); - expect(await client!.variation('overridden-flag', user, 'default')).toEqual('b'); + await waitFor( + async () => (await client!.variation('overridden-flag', user, 'default')) === 'b', + ); filesystem.set(jsonPath, '{"flagValues": {"overridden-flag": "c"}}'); filesystem.emit(directory); From ae07657d9667960e9d00730ce1e546f8e167d1a1 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 9 Oct 2026 21:00:40 +0000 Subject: [PATCH 7/9] test: Cover a definition of the wrong shape as a failed load of the file override source The override layer now rejects a definition that evaluation cannot read. For the file-based source that is a failed load like any other: the error names the entry and the field, the last good overrides stay in effect, the retry repeats at debug level, and a fix recovers. --- .../overrides/FileOverrideSource.test.ts | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts b/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts index b6068ee055..3bfa028d4a 100644 --- a/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts @@ -1,5 +1,7 @@ import { LDKeyedFeatureStoreItem, LDOverrideSink } from '../../src/api/subsystems'; import { FileOverrideSource, FileOverrideSourceConfig } from '../../src/overrides'; +import OverrideLayer from '../../src/overrides/OverrideLayer'; +import VersionedDataKinds from '../../src/store/VersionedDataKinds'; import MockFilesystem from '../data_sources/filedata/MockFilesystem'; import TestLogger, { LogLevel } from '../Logger'; @@ -271,6 +273,52 @@ describe('given a file override source over a mock filesystem', () => { expect(sink.last.flags[0].variations).toEqual([false]); }); + it('reports a definition of the wrong shape as a failed load and keeps the last good overrides', async () => { + // The override layer rejects a definition that evaluation cannot read. To the source that is + // a failed load like any other: logged once, the last good overrides stay, and a fix recovers. + const layer = new OverrideLayer(); + const layerSink: LDOverrideSink = { + setOverrides: (flags, segments) => { + layer.setAll(flags, segments); + }, + }; + filesystem.set(first, '{"flagValues": {"flag1": true}}'); + source = new FileOverrideSource( + { + paths: [first], + duplicateKeysHandling: 'fail', + changeDetection: 'watching', + pollIntervalMs: 1000, + }, + filesystem, + logger, + ); + await source.start(layerSink); + expect(layer.get(VersionedDataKinds.Features, 'flag1')?.variations).toEqual([true]); + + filesystem.set( + first, + '{"flags": {"flag1": {"key": "flag1", "version": 2, "on": true, "fallthrough": {"variation": 0}}}}', + ); + filesystem.emit(directory); + await jest.advanceTimersByTimeAsync(300); + logger.expectMessages([ + { + level: LogLevel.Error, + matches: /Unable to load flags: flag "flag1": "variations" must be an array/, + }, + ]); + expect(layer.get(VersionedDataKinds.Features, 'flag1')?.variations).toEqual([true]); + + // The retry repeats the same failure at debug level only. + await jest.advanceTimersByTimeAsync(1000); + expect(logger.getCount(LogLevel.Error)).toEqual(1); + + filesystem.set(first, '{"flagValues": {"flag1": false}}'); + await jest.advanceTimersByTimeAsync(1000); + expect(layer.get(VersionedDataKinds.Features, 'flag1')?.variations).toEqual([false]); + }); + it('stops detecting changes when closed', async () => { filesystem.set(first, '{"flagValues": {"flag1": true}}', 1); await startSource({ paths: [first] }); From 2a412574bbbed7f5becb1805340c3cd2b83e6f65 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 9 Oct 2026 22:35:56 +0000 Subject: [PATCH 8/9] refactor: Reuse existing helpers in the file override source and its tests The file override source expanded a flag value with its own copy of makeFlagWithValue; it now calls the file data source's with version 1, as the FDv2 file initializer does. The document parser shares the plain-object check with the definition validator and uses the common isNullish. The tests share one waitFor helper per package and the jest mock logger instead of per-file copies. The server-node override tests asserted on an evaluation made right after the client was constructed. Evaluation does not wait for the initial override load, so those assertions raced the file read and failed on a busy machine. They now wait for the override to be in effect before asserting, like the rest of the file's tests. --- .../LDClientNode.fileOverrides.test.ts | 46 ++++++++++--------- packages/sdk/server-node/__tests__/waitFor.ts | 28 +++++++++++ .../LDClientImpl.fileOverrides.test.ts | 25 ++-------- .../overrides/createOverrideSource.test.ts | 11 ++--- .../fileOverrideSourcePolicy.test.ts | 9 ++-- .../src/overrides/FileOverrideSource.ts | 22 ++------- .../shared/sdk-server/src/overrides/index.ts | 2 - .../src/overrides/overrideDocument.ts | 13 +++--- 8 files changed, 73 insertions(+), 83 deletions(-) create mode 100644 packages/sdk/server-node/__tests__/waitFor.ts diff --git a/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts b/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts index a7b6e783dc..331c8a340c 100644 --- a/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts +++ b/packages/sdk/server-node/__tests__/LDClientNode.fileOverrides.test.ts @@ -5,6 +5,7 @@ import { join } from 'node:path'; import { FileOverrideSourceOptions } from '@launchdarkly/js-server-sdk-common'; import LDClientNode from '../src/LDClientNode'; +import waitFor, { sleep } from './waitFor'; const user = { key: 'user-key' }; @@ -29,21 +30,6 @@ function makeClient( }); } -async function waitFor(condition: () => Promise, timeoutMs: number = 10000) { - const deadline = Date.now() + timeoutMs; - while (Date.now() < deadline) { - // eslint-disable-next-line no-await-in-loop - if (await condition()) { - return; - } - // eslint-disable-next-line no-await-in-loop - await new Promise((resolve) => { - setTimeout(resolve, 50); - }); - } - throw new Error('timed out waiting for the condition'); -} - const document = (value: string) => JSON.stringify({ flagValues: { 'overridden-flag': value } }); describe('given a temporary directory of override files', () => { @@ -65,6 +51,12 @@ describe('given a temporary directory of override files', () => { await writeFile(path, 'flagValues:\n yaml-flag: "override-value"\n'); client = makeClient(directory, { paths: [path] }); + // Evaluation does not wait for the initial load, so wait for the override to be in effect. + await waitFor( + async () => + (await client!.variationDetail('yaml-flag', user, 'default')).value === 'override-value', + 10000, + ); const detail = await client.variationDetail('yaml-flag', user, 'default'); expect(detail.value).toEqual('override-value'); @@ -76,12 +68,16 @@ describe('given a temporary directory of override files', () => { const path = join(directory, 'overrides.json'); await writeFile(path, document('b')); client = makeClient(directory, { paths: [path], changeDetection: 'polling', pollInterval: 1 }); - expect(await client.variation('overridden-flag', user, 'default')).toEqual('b'); + await waitFor( + async () => (await client!.variation('overridden-flag', user, 'default')) === 'b', + 10000, + ); await writeFile(path, document('c')); await waitFor( async () => (await client!.variation('overridden-flag', user, 'default')) === 'c', + 10000, ); }); @@ -95,6 +91,7 @@ describe('given a temporary directory of override files', () => { await waitFor( async () => (await client!.variation('overridden-flag', user, 'default')) === 'b', + 10000, ); }); @@ -102,32 +99,37 @@ describe('given a temporary directory of override files', () => { const path = join(directory, 'overrides.json'); await writeFile(path, document('b')); client = makeClient(directory, { paths: [path] }); - expect(await client.variation('overridden-flag', user, 'default')).toEqual('b'); + await waitFor( + async () => (await client!.variation('overridden-flag', user, 'default')) === 'b', + 10000, + ); await unlink(path); await waitFor(async () => { const detail = await client!.variationDetail('overridden-flag', user, 'default'); return detail.reason.errorKind === 'CLIENT_NOT_READY'; - }); + }, 10000); }); it('keeps the last good overrides while the file is malformed', async () => { const path = join(directory, 'overrides.json'); await writeFile(path, document('b')); client = makeClient(directory, { paths: [path] }); - expect(await client.variation('overridden-flag', user, 'default')).toEqual('b'); + await waitFor( + async () => (await client!.variation('overridden-flag', user, 'default')) === 'b', + 10000, + ); await writeFile(path, '{"flagValues"'); - await new Promise((resolve) => { - setTimeout(resolve, 1500); - }); + await sleep(1500); expect(await client.variation('overridden-flag', user, 'default')).toEqual('b'); await writeFile(path, document('c')); await waitFor( async () => (await client!.variation('overridden-flag', user, 'default')) === 'c', + 10000, ); }); }); diff --git a/packages/sdk/server-node/__tests__/waitFor.ts b/packages/sdk/server-node/__tests__/waitFor.ts new file mode 100644 index 0000000000..1fde825163 --- /dev/null +++ b/packages/sdk/server-node/__tests__/waitFor.ts @@ -0,0 +1,28 @@ +/** + * Resolves after the given number of milliseconds. + */ +export function sleep(ms: number): Promise { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} + +/** + * Polls a condition until it holds or the timeout elapses. + */ +export default async function waitFor( + condition: () => Promise | boolean, + timeoutMs: number = 5000, + intervalMs: number = 20, +): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + // eslint-disable-next-line no-await-in-loop + if (await condition()) { + return; + } + // eslint-disable-next-line no-await-in-loop + await sleep(intervalMs); + } + throw new Error('timed out waiting for the condition'); +} diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts index 328a08fe36..84f7abaa4f 100644 --- a/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.fileOverrides.test.ts @@ -1,35 +1,16 @@ -import { LDLogger } from '@launchdarkly/js-sdk-common'; - import { FileOverrideSourceOptions } from '../src/api/options/LDDataSystemOptions'; import { LDOptions } from '../src/api/options/LDOptions'; import LDClientImpl from '../src/LDClientImpl'; import MockFilesystem from './data_sources/filedata/MockFilesystem'; +import makeMockLogger from './mockLogger'; import { makeCallbacks, makeFDv2Platform } from './overrides/overridesTestSupport'; +import waitFor from './waitFor'; const user = { key: 'user-key' }; const directory = '/etc/launchdarkly'; const jsonPath = `${directory}/overrides.json`; const yamlPath = `${directory}/overrides.yaml`; -function makeLogger(): LDLogger { - return { error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() }; -} - -async function waitFor(condition: () => Promise, timeoutMs: number = 3000) { - const deadline = Date.now() + timeoutMs; - while (Date.now() < deadline) { - // eslint-disable-next-line no-await-in-loop - if (await condition()) { - return; - } - // eslint-disable-next-line no-await-in-loop - await new Promise((resolve) => { - setTimeout(resolve, 20); - }); - } - throw new Error('timed out waiting for the condition'); -} - describe('given a client with a file override source over a mock filesystem', () => { let filesystem: MockFilesystem; let client: LDClientImpl | undefined; @@ -45,7 +26,7 @@ describe('given a client with a file override source over a mock filesystem', () { sendEvents: false, diagnosticOptOut: true, - logger: makeLogger(), + logger: makeMockLogger(), ...options, dataSystem: { dataSource: { diff --git a/packages/shared/sdk-server/__tests__/overrides/createOverrideSource.test.ts b/packages/shared/sdk-server/__tests__/overrides/createOverrideSource.test.ts index 6a4ded9abe..a342c990da 100644 --- a/packages/shared/sdk-server/__tests__/overrides/createOverrideSource.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/createOverrideSource.test.ts @@ -4,12 +4,9 @@ import Configuration from '../../src/options/Configuration'; import { createOverrideSource, FileOverrideSource } from '../../src/overrides'; import { createBasicPlatform } from '../createBasicPlatform'; import MockFilesystem from '../data_sources/filedata/MockFilesystem'; +import makeMockLogger, { MockLogger } from '../mockLogger'; import TestOverrideSource from './TestOverrideSource'; -function makeLogger(): LDLogger & { warn: jest.Mock } { - return { error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() }; -} - function makeContext(logger: LDLogger, withFilesystem: boolean = true): ClientContext { return new ClientContext('sdk-key', new Configuration({ logger }), { ...createBasicPlatform(), @@ -20,11 +17,11 @@ function makeContext(logger: LDLogger, withFilesystem: boolean = true): ClientCo const defaultYamlParser = () => ({}); describe('given a client context with filesystem support', () => { - let logger: ReturnType; + let logger: MockLogger; let context: ClientContext; beforeEach(() => { - logger = makeLogger(); + logger = makeMockLogger(); context = makeContext(logger); }); @@ -181,7 +178,7 @@ describe('given a client context with filesystem support', () => { }); it('rejects the file source on a platform without filesystem support', () => { - const context = makeContext(makeLogger(), false); + const context = makeContext(makeMockLogger(), false); expect(() => createOverrideSource({ type: 'file', paths: ['/a.json'] }, context)).toThrow( 'The file-based override source requires a platform with filesystem support', ); diff --git a/packages/shared/sdk-server/__tests__/overrides/fileOverrideSourcePolicy.test.ts b/packages/shared/sdk-server/__tests__/overrides/fileOverrideSourcePolicy.test.ts index 766119bbfd..b24b63695c 100644 --- a/packages/shared/sdk-server/__tests__/overrides/fileOverrideSourcePolicy.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/fileOverrideSourcePolicy.test.ts @@ -1,11 +1,12 @@ import { Flag } from '../../src/evaluation/data/Flag'; -import { fileOverrideSourcePolicy, makeOverrideFlagWithValue } from '../../src/overrides'; +import { fileOverrideSourcePolicy } from '../../src/overrides'; // The policy is the one place where the file-based override source differs from the file data // sources in how documents become data. it('expands a flag value into a flag that is on and serves the value by fallthrough with version 1', () => { - expect(makeOverrideFlagWithValue('flag', 'a')).toEqual({ + const policy = fileOverrideSourcePolicy('fail'); + expect(policy.makeFlagWithValue('flag', 'a', undefined)).toEqual({ key: 'flag', version: 1, on: true, @@ -13,8 +14,8 @@ it('expands a flag value into a flag that is on and serves the value by fallthro variations: ['a'], }); // The previous flag plays no part: an override snapshot has no version history. - const previous = { ...makeOverrideFlagWithValue('flag', 'old'), version: 9 }; - expect(fileOverrideSourcePolicy('fail').makeFlagWithValue('flag', 'a', previous).version).toBe(1); + const previous = { ...policy.makeFlagWithValue('flag', 'old', undefined), version: 9 }; + expect(policy.makeFlagWithValue('flag', 'a', previous).version).toBe(1); }); it('fails the load on a duplicate key with the fail handling', () => { diff --git a/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts b/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts index 669fbae6d6..cca60452aa 100644 --- a/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts +++ b/packages/shared/sdk-server/src/overrides/FileOverrideSource.ts @@ -12,7 +12,7 @@ import { ReloadResult, YamlParser, } from '../data_sources/filedata'; -import { Flag } from '../evaluation/data/Flag'; +import { makeFlagWithValue } from '../data_sources/FileDataSource'; import { parseOverrideDocument } from './overrideDocument'; /** @@ -78,23 +78,6 @@ export interface FileOverrideSourceConfig { yamlParser?: YamlParser; } -/** - * Expands a flag value into a full flag definition that returns the given value for every - * context. The flag is on, has the value as its only variation, and serves that variation as - * its fallthrough, so it evaluates with the FALLTHROUGH reason. - * - * @internal - */ -export function makeOverrideFlagWithValue(key: string, value: any): Flag { - return { - key, - version: 1, - on: true, - fallthrough: { variation: 0 }, - variations: [value], - }; -} - /** * How the file-based override source translates its documents into data. This is the one place * where its rules differ from the file data sources. @@ -116,7 +99,8 @@ export function fileOverrideSourcePolicy( ): FileDataPolicy { return { parseDocument: parseOverrideDocument(yamlParser), - makeFlagWithValue: (key, value) => makeOverrideFlagWithValue(key, value), + // An override snapshot has no version history, so every expanded flag is version 1. + makeFlagWithValue: (key, value) => makeFlagWithValue(key, value, 1), resolveDuplicateKey: (category, key) => { if (duplicateKeysHandling === 'ignore') { return 'keepFirst'; diff --git a/packages/shared/sdk-server/src/overrides/index.ts b/packages/shared/sdk-server/src/overrides/index.ts index 860f8bba97..356f136230 100644 --- a/packages/shared/sdk-server/src/overrides/index.ts +++ b/packages/shared/sdk-server/src/overrides/index.ts @@ -4,7 +4,6 @@ import FileOverrideSource, { FileChangeDetection, FileOverrideSourceConfig, fileOverrideSourcePolicy, - makeOverrideFlagWithValue, MAXIMUM_POLL_INTERVAL_SECONDS, MINIMUM_POLL_INTERVAL_SECONDS, OverrideDuplicateKeysHandling, @@ -23,7 +22,6 @@ export { LayerContents, LayerKindContents, OverrideLayer, - makeOverrideFlagWithValue, MAXIMUM_POLL_INTERVAL_SECONDS, MINIMUM_POLL_INTERVAL_SECONDS, OverrideDuplicateKeysHandling, diff --git a/packages/shared/sdk-server/src/overrides/overrideDocument.ts b/packages/shared/sdk-server/src/overrides/overrideDocument.ts index 004c409190..4443a7efec 100644 --- a/packages/shared/sdk-server/src/overrides/overrideDocument.ts +++ b/packages/shared/sdk-server/src/overrides/overrideDocument.ts @@ -1,13 +1,12 @@ +import { isNullish } from '@launchdarkly/js-sdk-common'; + import { DocumentParser, FileDataDocument, parseDocumentByExtension, YamlParser, } from '../data_sources/filedata'; - -function isPlainObject(value: any): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} +import { isPlainObject } from './validateDefinition'; /** * Checks that a member of the document is an object keyed by item key, and that every entry @@ -15,7 +14,7 @@ function isPlainObject(value: any): value is Record { */ function validateItems(document: Record, member: 'flags' | 'segments'): void { const items = document[member]; - if (items === undefined || items === null) { + if (isNullish(items)) { return; } if (!isPlainObject(items)) { @@ -32,7 +31,7 @@ function validateItems(document: Record, member: 'flags' | 'segment } function validateDocument(parsed: any): FileDataDocument { - if (parsed === undefined || parsed === null) { + if (isNullish(parsed)) { // An empty file is an empty document. return {}; } @@ -42,7 +41,7 @@ function validateDocument(parsed: any): FileDataDocument { validateItems(parsed, 'flags'); validateItems(parsed, 'segments'); const { flagValues } = parsed; - if (flagValues !== undefined && flagValues !== null && !isPlainObject(flagValues)) { + if (!isNullish(flagValues) && !isPlainObject(flagValues)) { throw new Error('"flagValues" must be an object keyed by flag key'); } const document: FileDataDocument = {}; From 0329a1f272ffbe4c07dcfa644888cbf5fcb2d8c6 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 9 Oct 2026 23:15:36 +0000 Subject: [PATCH 9/9] fix: Key file override entries by their map key The file override source merged and de-duplicated entries by the map key under which a file listed them, but the layer stored each entry under its key field, which the parser kept when the file supplied one. A definition pasted under a new map key with its old key field then overrode the old flag, escaped the duplicate check in the default fail mode, and inverted the ignore mode. The parser now sets the key field to the map key, as the Go and Python SDKs key the layer by the map key. --- .../overrides/FileOverrideSource.test.ts | 14 ++++++++++++++ .../__tests__/overrides/overrideDocument.test.ts | 16 ++++++++++++---- .../sdk-server/src/overrides/overrideDocument.ts | 9 +++++---- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts b/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts index 3bfa028d4a..7b05b70785 100644 --- a/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/FileOverrideSource.test.ts @@ -108,6 +108,20 @@ describe('given a file override source over a mock filesystem', () => { expect(sink.last.flags[0].version).toEqual(1); }); + it('keys an entry by its map key when its key field names another flag', async () => { + // The merge de-duplicates by map key, so the layer is keyed by it too: the entry overrides + // the flag it is listed under, and the second file's flag is not a duplicate of it. + filesystem.set(first, '{"flags": {"flag1": {"key": "other", "version": 1}}}'); + filesystem.set(second, '{"flags": {"other": {"key": "other", "version": 2}}}'); + + await startSource({ paths: [first, second] }); + + expect(sink.snapshots).toHaveLength(1); + expect(sink.flagKeys()).toEqual(['flag1', 'other']); + expect(sink.last.flags.find((flag) => flag.key === 'flag1')!.version).toEqual(1); + expect(sink.last.flags.find((flag) => flag.key === 'other')!.version).toEqual(2); + }); + it('fails the load for duplicate keys by default', async () => { filesystem.set(first, '{"flags": {"flag1": {"key": "flag1", "version": 1}}}'); filesystem.set(second, '{"flags": {"flag1": {"key": "flag1", "version": 2}}}'); diff --git a/packages/shared/sdk-server/__tests__/overrides/overrideDocument.test.ts b/packages/shared/sdk-server/__tests__/overrides/overrideDocument.test.ts index 642fbffc9a..3cf4524b5f 100644 --- a/packages/shared/sdk-server/__tests__/overrides/overrideDocument.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/overrideDocument.test.ts @@ -116,16 +116,24 @@ describe('given the override document parser', () => { ); }); - it('fills a missing entry key from the map key and keeps an existing key', () => { + it('keys every entry by its map key, whatever its key field says', () => { + // The map key is what the merge across files orders and de-duplicates by, so a key field + // that differs, as after pasting a definition under a new key, is set to the map key. const document = parseOverrideDocument()( 'data.json', JSON.stringify({ - flags: { 'flag-a': { on: false, version: 1 }, 'flag-b': { key: 'other', version: 1 } }, - segments: { 'segment-a': { version: 1 } }, + flags: { + 'flag-a': { on: false, version: 1 }, + 'flag-b': { key: 'other', version: 1 }, + 'flag-c': { key: null, version: 1 }, + }, + segments: { 'segment-a': { version: 1 }, 'segment-b': { key: 'other', version: 1 } }, }), ); expect(document.flags?.['flag-a'].key).toEqual('flag-a'); - expect(document.flags?.['flag-b'].key).toEqual('other'); + expect(document.flags?.['flag-b'].key).toEqual('flag-b'); + expect(document.flags?.['flag-c'].key).toEqual('flag-c'); expect(document.segments?.['segment-a'].key).toEqual('segment-a'); + expect(document.segments?.['segment-b'].key).toEqual('segment-b'); }); }); diff --git a/packages/shared/sdk-server/src/overrides/overrideDocument.ts b/packages/shared/sdk-server/src/overrides/overrideDocument.ts index 4443a7efec..80459806b5 100644 --- a/packages/shared/sdk-server/src/overrides/overrideDocument.ts +++ b/packages/shared/sdk-server/src/overrides/overrideDocument.ts @@ -10,7 +10,10 @@ import { isPlainObject } from './validateDefinition'; /** * Checks that a member of the document is an object keyed by item key, and that every entry - * in it is an object. The key of an entry is filled from the map key when the entry omits it. + * in it is an object. The map key is the key of the entry: it is what the merge across files + * orders and de-duplicates by, and what the Go and Python SDKs key the layer by, so a key field + * inside the entry is set to it. Without that, an entry pasted under a new map key with its + * old key field would be merged under one key and stored under another. */ function validateItems(document: Record, member: 'flags' | 'segments'): void { const items = document[member]; @@ -24,9 +27,7 @@ function validateItems(document: Record, member: 'flags' | 'segment if (!isPlainObject(item)) { throw new Error(`${member.slice(0, -1)} "${key}" must be an object`); } - if (item.key === undefined) { - item.key = key; - } + item.key = key; }); }