From 381a62326b4620b7a20d9b431390f9071ad5a0fb Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 25 Sep 2026 15:29:33 -0700 Subject: [PATCH 1/9] feat: Add the override store, overlay, and data system wiring Adds the override layer described by the OVERRIDE specification to the FDv2 data system of the server SDK. An override source is configured with the new overrides option of the data system options. It is not a data source: it takes no part in the initializer and synchronizer pipeline and has no effect on initialization status. The SDK starts it when the client is created, passes it a sink, and closes it with the client. The public LDOverrideSource and LDOverrideSink interfaces let an application supply a custom source. The source supplies complete snapshots to the override store. Each snapshot replaces the whole store in one assignment. The store holds prepared, marked copies of the definitions and never modifies the objects the source supplied. An overlay at the store read boundary returns the override entry for a key in preference to LaunchDarkly data. Evaluation, prerequisite and segment resolution, and the all-flags read go through the overlay. The client consults the override store before the not-initialized short circuit, so an overridden flag is served before the client has LaunchDarkly data while a non-overridden flag still gets the not-ready default. Evaluations wait for the source's asynchronous initial load, so an override present at startup takes effect from the first evaluation. The all-flags state keeps override-affected flags with their values and marked reasons but turns their event tracking fields off. A type mismatch keeps the marking of the evaluation it replaces. Adding, changing, or removing an override fires the normal flag change notifications, including for the flags that depend on a changed entry. The OVERRIDE specification's test vectors run as a unit test through the full client stack. --- .../LDClientImpl.overrideVectors.test.ts | 108 +++++ .../__tests__/LDClientImpl.overrides.test.ts | 428 ++++++++++++++++++ .../__tests__/overrides/OverrideLayer.test.ts | 124 +++++ .../__tests__/overrides/OverrideSink.test.ts | 199 ++++++++ .../overrides/ReadStoreOverlay.test.ts | 122 +++++ .../__tests__/overrides/TestOverrideSource.ts | 66 +++ .../overrides/override-vectors/vectors.json | 331 ++++++++++++++ .../overrides/overridesTestSupport.ts | 105 +++++ .../sdk-server/src/FlagsStateBuilder.ts | 10 +- .../shared/sdk-server/src/LDClientImpl.ts | 300 ++++++++---- packages/shared/sdk-server/src/api/index.ts | 1 + .../src/api/options/LDDataSystemOptions.ts | 31 +- .../src/api/subsystems/LDOverrideSource.ts | 64 +++ .../sdk-server/src/api/subsystems/index.ts | 1 + .../src/evaluation/data/overrideMarker.ts | 11 + .../sdk-server/src/options/Configuration.ts | 16 + .../sdk-server/src/overrides/OverrideLayer.ts | 129 ++++++ .../sdk-server/src/overrides/OverrideSink.ts | 163 +++++++ .../src/overrides/ReadStoreOverlay.ts | 60 +++ .../src/overrides/createOverrideSource.ts | 31 ++ .../shared/sdk-server/src/overrides/index.ts | 15 + 21 files changed, 2232 insertions(+), 83 deletions(-) create mode 100644 packages/shared/sdk-server/__tests__/LDClientImpl.overrideVectors.test.ts create mode 100644 packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts create mode 100644 packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts create mode 100644 packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts create mode 100644 packages/shared/sdk-server/__tests__/overrides/ReadStoreOverlay.test.ts create mode 100644 packages/shared/sdk-server/__tests__/overrides/TestOverrideSource.ts create mode 100644 packages/shared/sdk-server/__tests__/overrides/override-vectors/vectors.json create mode 100644 packages/shared/sdk-server/__tests__/overrides/overridesTestSupport.ts create mode 100644 packages/shared/sdk-server/src/api/subsystems/LDOverrideSource.ts create mode 100644 packages/shared/sdk-server/src/overrides/OverrideLayer.ts create mode 100644 packages/shared/sdk-server/src/overrides/OverrideSink.ts create mode 100644 packages/shared/sdk-server/src/overrides/ReadStoreOverlay.ts create mode 100644 packages/shared/sdk-server/src/overrides/createOverrideSource.ts create mode 100644 packages/shared/sdk-server/src/overrides/index.ts diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.overrideVectors.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.overrideVectors.test.ts new file mode 100644 index 0000000000..4a856fef1d --- /dev/null +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.overrideVectors.test.ts @@ -0,0 +1,108 @@ +import * as fs from 'fs'; +import * as path from 'path'; + +import { + fdv2FullPayload, + makeFDv2Client, + makeFDv2Platform, + singleValueFlag, +} from './overrides/overridesTestSupport'; +import TestOverrideSource from './overrides/TestOverrideSource'; + +// These vectors come from the OVERRIDE specification. Each vector sets up LaunchDarkly data, an +// override layer, and an initialization state. The test evaluates one flag through the full +// client stack and checks the value, the variation index, and the reason. +const vectorsPath = path.join(__dirname, 'overrides', 'override-vectors', 'vectors.json'); + +// The vectors' semantics are versioned. A schema change means this runner needs review. +const supportedVectorSchema = '0.4.0'; + +interface OverrideVector { + description: string; + group: string; + launchDarklyData: { + initialized: boolean; + flags: Record; + segments: Record; + }; + overrides: { + flags?: Record; + flagValues?: Record; + segments?: Record; + }; + evaluate: { + flagKey: string; + context: any; + defaultValue: any; + }; + expect: { + value: any; + variationIndex: number | null; + reason: Record; + summaryOverrideAffected?: boolean; + }; +} + +const file: { schemaVersion: string; vectors: OverrideVector[] } = JSON.parse( + fs.readFileSync(vectorsPath, 'utf8'), +); + +it('uses the supported vector schema', () => { + expect(file.schemaVersion).toEqual(supportedVectorSchema); + expect(file.vectors.length).toBeGreaterThan(0); +}); + +describe.each( + file.vectors.map((vector) => [`${vector.group}: ${vector.description}`, vector] as const), +)('%s', (_name, vector) => { + it('evaluates as the vector expects', async () => { + // The override layer is the same document shape the file-based source accepts. A flag value + // entry expands into a full flag that is off and serves the value. + const source = new TestOverrideSource({ + flags: [ + ...Object.values(vector.overrides.flags ?? {}), + ...Object.entries(vector.overrides.flagValues ?? {}).map(([key, value]) => + singleValueFlag(key, value), + ), + ], + segments: Object.values(vector.overrides.segments ?? {}), + }); + const platform = vector.launchDarklyData.initialized + ? makeFDv2Platform( + fdv2FullPayload(vector.launchDarklyData.flags, vector.launchDarklyData.segments), + ) + : makeFDv2Platform(); + const client = makeFDv2Client(platform, { dataSystem: { overrides: source } }); + try { + if (vector.launchDarklyData.initialized) { + await client.waitForInitialization({ timeout: 5 }); + } + + const detail = await client.variationDetail( + vector.evaluate.flagKey, + vector.evaluate.context, + vector.evaluate.defaultValue, + ); + + expect(detail.value).toEqual(vector.expect.value); + if (vector.expect.variationIndex === null) { + expect(detail.variationIndex ?? null).toBeNull(); + } else { + expect(detail.variationIndex).toEqual(vector.expect.variationIndex); + } + + // The reason is compared only on the fields the vector lists. The override indicator + // collapses tri-state: an expected reason that omits it requires the actual reason to + // report false or to omit it. + const actualReason = detail.reason as Record; + Object.entries(vector.expect.reason).forEach(([field, expected]) => { + expect(actualReason[field]).toEqual(expected); + }); + if (!('overrideAffected' in vector.expect.reason)) { + expect(actualReason.overrideAffected ?? false).toBe(false); + } + } finally { + client.close(); + } + }); +}); diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts new file mode 100644 index 0000000000..feed292e16 --- /dev/null +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts @@ -0,0 +1,428 @@ +import { LDClientContext, LDLogger } from '@launchdarkly/js-sdk-common'; + +import { LDOverrideSink } from '../src/api/subsystems'; +import LDClientImpl from '../src/LDClientImpl'; +import { + fdv2FullPayload, + makeCallbacks, + makeFDv2Client, + makeFDv2Platform, + singleValueFlag, +} from './overrides/overridesTestSupport'; +import TestOverrideSource from './overrides/TestOverrideSource'; + +const user = { key: 'user-key' }; + +function makeLogger(): LDLogger & { warn: jest.Mock; error: jest.Mock } { + return { error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() }; +} + +function warningsMatching(logger: { warn: jest.Mock }, pattern: RegExp): number { + return logger.warn.mock.calls.filter((call) => pattern.test(String(call[0]))).length; +} + +// Change notifications and asynchronous starts complete on later turns of the event loop. +const settle = () => + new Promise((resolve) => { + setTimeout(resolve, 10); + }); + +describe('given an uninitialized client with an override source', () => { + let source: TestOverrideSource; + let client: LDClientImpl; + let logger: ReturnType; + + beforeEach(() => { + logger = makeLogger(); + source = new TestOverrideSource({ flags: [singleValueFlag('overridden-flag', true)] }); + client = makeFDv2Client(makeFDv2Platform(), { logger, dataSystem: { overrides: source } }); + }); + + afterEach(() => { + client.close(); + }); + + it('serves an override before the client is initialized', async () => { + expect(client.initialized()).toBe(false); + + const detail = await client.boolVariationDetail('overridden-flag', user, false); + + expect(detail.value).toBe(true); + expect(detail.variationIndex).toEqual(0); + expect(detail.reason).toEqual({ kind: 'OFF', overrideAffected: true }); + expect(client.initialized()).toBe(false); + }); + + it('returns the not-ready default for a flag the override layer does not hold', async () => { + const detail = await client.boolVariationDetail('other-flag', user, false); + + expect(detail.value).toBe(false); + expect(detail.variationIndex).toBeNull(); + expect(detail.reason).toEqual({ kind: 'ERROR', errorKind: 'CLIENT_NOT_READY' }); + }); + + it('restores the not-ready short-circuit when the override is removed', async () => { + expect(await client.boolVariation('overridden-flag', user, false)).toBe(true); + + source.setOverrides([]); + + const detail = await client.boolVariationDetail('overridden-flag', user, false); + expect(detail.value).toBe(false); + expect(detail.reason).toEqual({ kind: 'ERROR', errorKind: 'CLIENT_NOT_READY' }); + }); + + it('keeps a wrong type result of an overridden flag marked', async () => { + source.setOverrides([singleValueFlag('overridden-flag', 'not-a-bool')]); + + const detail = await client.boolVariationDetail('overridden-flag', user, false); + + expect(detail.value).toBe(false); + expect(detail.variationIndex).toBeNull(); + expect(detail.reason).toEqual({ + kind: 'ERROR', + errorKind: 'WRONG_TYPE', + overrideAffected: true, + }); + }); + + it('reports only the override layer in the all flags state and warns once', async () => { + const state = await client.allFlagsState(user, { withReasons: true }); + + expect(state.valid).toBe(true); + expect(state.allValues()).toEqual({ 'overridden-flag': true }); + expect(state.getFlagReason('overridden-flag')).toEqual({ kind: 'OFF', overrideAffected: true }); + + await client.allFlagsState(user); + expect(warningsMatching(logger, /returning only flags from the override layer/)).toEqual(1); + }); + + it('reports an invalid empty all flags state when the override layer is empty', async () => { + source.setOverrides([]); + + const state = await client.allFlagsState(user); + + expect(state.valid).toBe(false); + expect(state.allValues()).toEqual({}); + expect(warningsMatching(logger, /Data store not available/)).toEqual(1); + }); + + it('does not become initialized because of overrides', async () => { + await expect(client.waitForInitialization({ timeout: 0.05 })).rejects.toThrow(/timed out/); + expect(client.initialized()).toBe(false); + }); +}); + +describe('given an initialized client with LaunchDarkly data and an override source', () => { + const debugUntil = Date.now() + 100000; + const plainTracked = { + ...singleValueFlag('plain-tracked', true, 1), + trackEvents: true, + debugEventsUntilDate: debugUntil, + }; + // The overridden flag is on and serves variation 0, so the dependent flag's prerequisite + // passes only through the override. The LaunchDarkly definition is off. + const dependentTracked = { + key: 'dependent-tracked', + version: 1, + on: true, + variations: [false, true], + fallthrough: { variation: 1 }, + offVariation: 0, + prerequisites: [{ key: 'overridden-flag', variation: 0 }], + trackEvents: true, + debugEventsUntilDate: debugUntil, + }; + const launchDarklyOverridden = singleValueFlag('overridden-flag', 'ld-value', 5); + const overriddenTracked = { + key: 'overridden-flag', + version: 7, + on: true, + fallthrough: { variation: 0 }, + offVariation: 0, + variations: [true], + trackEvents: true, + debugEventsUntilDate: debugUntil, + }; + const normal = singleValueFlag('flag-normal', 'normal-value', 100); + + let source: TestOverrideSource; + let client: LDClientImpl; + + beforeEach(async () => { + const platform = makeFDv2Platform( + fdv2FullPayload({ + 'plain-tracked': plainTracked, + 'dependent-tracked': dependentTracked, + 'overridden-flag': launchDarklyOverridden, + 'flag-normal': normal, + }), + ); + source = new TestOverrideSource({ flags: [overriddenTracked] }); + client = makeFDv2Client(platform, { dataSystem: { overrides: source } }); + await client.waitForInitialization({ timeout: 5 }); + }); + + afterEach(() => { + client.close(); + }); + + it('gives an override precedence over LaunchDarkly data for the same key', async () => { + const detail = await client.variationDetail('overridden-flag', user, 'default'); + + expect(detail.value).toBe(true); + expect(detail.reason).toEqual({ kind: 'FALLTHROUGH', overrideAffected: true }); + }); + + it('leaves a flag absent from the override layer unaffected', async () => { + const detail = await client.variationDetail('flag-normal', user, 'default'); + + expect(detail.value).toEqual('normal-value'); + expect(detail.reason).toEqual({ kind: 'OFF' }); + }); + + it('marks a flag whose prerequisite is overridden', async () => { + const detail = await client.variationDetail('dependent-tracked', user, 'default'); + + expect(detail.value).toBe(true); + expect(detail.reason).toEqual({ kind: 'FALLTHROUGH', overrideAffected: true }); + }); + + it('turns off event tracking for override-affected flags in the all flags state', async () => { + const state = await client.allFlagsState(user, { withReasons: true }); + const json = state.toJSON() as any; + const flagsState = json.$flagsState; + + // A flag with no override keeps its tracking fields. + expect(json['plain-tracked']).toBe(true); + expect(flagsState['plain-tracked'].trackEvents).toBe(true); + expect(flagsState['plain-tracked'].debugEventsUntilDate).toEqual(debugUntil); + + // The overridden flag and the flag that depends on it stay in the state with their values + // and marked reasons, but without tracking fields. + ['overridden-flag', 'dependent-tracked'].forEach((key) => { + expect(json[key]).toBe(true); + expect(flagsState[key].reason.overrideAffected).toBe(true); + expect(flagsState[key]).not.toHaveProperty('trackEvents'); + expect(flagsState[key]).not.toHaveProperty('trackReason'); + expect(flagsState[key]).not.toHaveProperty('debugEventsUntilDate'); + }); + expect(flagsState['overridden-flag'].version).toEqual(7); + }); + + it('returns LaunchDarkly data again when the override is removed', async () => { + source.setOverrides([]); + + const detail = await client.variationDetail('overridden-flag', user, 'default'); + + expect(detail.value).toEqual('ld-value'); + expect(detail.reason).toEqual({ kind: 'OFF' }); + }); +}); + +describe('given a client with flag change listeners', () => { + let updates: string[]; + let source: TestOverrideSource; + let client: LDClientImpl; + + beforeEach(async () => { + updates = []; + const dependent = { + key: 'dependent', + version: 1, + on: true, + fallthrough: { variation: 0 }, + variations: [true, false], + rules: [ + { + id: 'r', + variation: 1, + clauses: [{ attribute: '', op: 'segmentMatch', values: ['segment1'] }], + }, + ], + }; + const platform = makeFDv2Platform( + fdv2FullPayload( + { dependent, unrelated: singleValueFlag('unrelated', true) }, + { segment1: { key: 'segment1', version: 1 } }, + ), + ); + source = new TestOverrideSource(); + client = makeFDv2Client( + platform, + { dataSystem: { overrides: source } }, + makeCallbacks((key) => updates.push(key), true), + ); + await client.waitForInitialization({ timeout: 5 }); + await settle(); + updates = []; + }); + + afterEach(() => { + client.close(); + }); + + it('notifies listeners when an override is added and when it is removed', async () => { + source.setOverrides([singleValueFlag('overridden-flag', true)]); + await settle(); + expect(updates).toEqual(['overridden-flag']); + + updates = []; + source.setOverrides([]); + await settle(); + expect(updates).toEqual(['overridden-flag']); + }); + + it('notifies the flags that depend on an overridden segment', async () => { + source.setOverrides([], [{ key: 'segment1', version: 99 }]); + await settle(); + expect(updates).toEqual(['dependent']); + }); +}); + +describe('given an override source with an asynchronous initial load', () => { + let source: TestOverrideSource; + let client: LDClientImpl; + + beforeEach(() => { + source = new TestOverrideSource(undefined, true); + client = makeFDv2Client(makeFDv2Platform(), { dataSystem: { overrides: source } }); + }); + + afterEach(() => { + client.close(); + }); + + it('waits for the initial load before evaluating', async () => { + let resolved = false; + const pending = client.boolVariationDetail('overridden-flag', user, false).then((detail) => { + resolved = true; + return detail; + }); + await settle(); + expect(resolved).toBe(false); + + source.setOverrides([singleValueFlag('overridden-flag', true)]); + source.completeStart(); + + const detail = await pending; + expect(detail.value).toBe(true); + expect(detail.reason).toEqual({ kind: 'OFF', overrideAffected: true }); + }); + + it('waits for the initial load before reporting the all flags state', async () => { + let resolved = false; + const pending = client.allFlagsState(user).then((state) => { + resolved = true; + return state; + }); + await settle(); + expect(resolved).toBe(false); + + source.setOverrides([singleValueFlag('overridden-flag', true)]); + source.completeStart(); + + const state = await pending; + expect(state.valid).toBe(true); + expect(state.allValues()).toEqual({ 'overridden-flag': true }); + }); +}); + +describe('given override source lifecycle and configuration', () => { + let client: LDClientImpl | undefined; + + afterEach(() => { + client?.close(); + client = undefined; + }); + + it('starts the source when the client is created and closes it with the client', () => { + const source = new TestOverrideSource(); + client = makeFDv2Client(makeFDv2Platform(), { dataSystem: { overrides: source } }); + expect(source.started).toBe(true); + expect(source.closed).toBe(false); + + client.close(); + expect(source.closed).toBe(true); + }); + + it('does not start the source in offline mode', async () => { + const source = new TestOverrideSource({ flags: [singleValueFlag('overridden-flag', true)] }); + client = makeFDv2Client(makeFDv2Platform(), { + offline: true, + dataSystem: { overrides: source }, + }); + + expect(source.started).toBe(false); + expect(await client.boolVariation('overridden-flag', user, false)).toBe(false); + }); + + it('accepts a factory function for the source', () => { + const source = new TestOverrideSource(); + const factory = jest.fn((_clientContext: LDClientContext) => source); + client = makeFDv2Client(makeFDv2Platform(), { dataSystem: { overrides: factory } }); + + expect(factory).toHaveBeenCalledTimes(1); + expect(factory.mock.calls[0][0].basicConfiguration.sdkKey).toEqual('sdk-key-overrides'); + expect(source.started).toBe(true); + }); + + it('fails client construction for a configuration that is not a source', () => { + expect(() => + makeFDv2Client(makeFDv2Platform(), { + dataSystem: { overrides: { start: 'not a function' } as any }, + }), + ).toThrow('Unsupported override source configuration'); + }); + + it('warns and ignores an override option of the wrong type', async () => { + const logger = makeLogger(); + client = makeFDv2Client(makeFDv2Platform(), { + logger, + dataSystem: { overrides: 'not an override source' as any }, + }); + + expect(warningsMatching(logger, /dataSystem\.overrides/)).toEqual(1); + const detail = await client.boolVariationDetail('overridden-flag', user, false); + expect(detail.reason).toEqual({ kind: 'ERROR', errorKind: 'CLIENT_NOT_READY' }); + }); + + it('logs and continues when the initial load fails', async () => { + const logger = makeLogger(); + const failing = { + start: () => Promise.reject(new Error('load failed')), + close: () => {}, + }; + client = makeFDv2Client(makeFDv2Platform(), { logger, dataSystem: { overrides: failing } }); + + const detail = await client.boolVariationDetail('overridden-flag', user, false); + expect(detail.reason).toEqual({ kind: 'ERROR', errorKind: 'CLIENT_NOT_READY' }); + expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('load failed')); + }); + + it('logs and continues when start throws', async () => { + const logger = makeLogger(); + const throwing = { + start: (_sink: LDOverrideSink) => { + throw new Error('start failed'); + }, + close: () => {}, + }; + client = makeFDv2Client(makeFDv2Platform(), { logger, dataSystem: { overrides: throwing } }); + + const detail = await client.boolVariationDetail('overridden-flag', user, false); + expect(detail.reason).toEqual({ kind: 'ERROR', errorKind: 'CLIENT_NOT_READY' }); + expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('start failed')); + }); + + it('behaves as before when no override source is configured', async () => { + client = makeFDv2Client( + makeFDv2Platform(fdv2FullPayload({ flag: singleValueFlag('flag', 'x') })), + {}, + ); + await client.waitForInitialization({ timeout: 5 }); + + const detail = await client.variationDetail('flag', user, 'default'); + expect(detail.value).toEqual('x'); + expect(detail.reason).toEqual({ kind: 'OFF' }); + }); +}); diff --git a/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts b/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts new file mode 100644 index 0000000000..11c6413eda --- /dev/null +++ b/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts @@ -0,0 +1,124 @@ +/* eslint-disable no-underscore-dangle */ +import { AttributeReference } from '@launchdarkly/js-sdk-common'; + +import { Flag } from '../../src/evaluation/data/Flag'; +import { Segment } from '../../src/evaluation/data/Segment'; +import { OverrideLayer } from '../../src/overrides'; +import VersionedDataKinds from '../../src/store/VersionedDataKinds'; + +function rawFlag(key: string, version: number = 1): any { + return { + key, + version, + on: true, + fallthrough: { variation: 0 }, + variations: [false, true], + targets: [{ variation: 1, values: ['user-a', 'user-b'] }], + rules: [ + { + id: 'rule', + variation: 1, + clauses: [{ attribute: 'name', op: 'in', values: ['x', 'y'], negate: false }], + }, + ], + }; +} + +function rawSegment(key: string, version: number = 1): any { + return { + key, + version, + included: ['user-a'], + excluded: ['user-z'], + rules: [{ id: 'rule', clauses: [{ attribute: 'name', op: 'in', values: ['x', 'y'] }] }], + }; +} + +it('stores marked copies and does not modify the supplied definitions', () => { + const layer = new OverrideLayer(); + const flag = rawFlag('flag1', 2); + const segment = rawSegment('segment1', 3); + const pristineFlag = JSON.parse(JSON.stringify(flag)); + const pristineSegment = JSON.parse(JSON.stringify(segment)); + + layer.setAll([flag], [segment]); + + expect(flag).toEqual(pristineFlag); + expect(segment).toEqual(pristineSegment); + expect(flag).not.toHaveProperty('_sdk_override'); + expect(flag.rules[0].clauses[0]).not.toHaveProperty('attributeReference'); + + const storedFlag = layer.get(VersionedDataKinds.Features, 'flag1') as Flag; + expect(storedFlag._sdk_override).toBe(true); + expect(storedFlag.version).toEqual(2); + expect(storedFlag).not.toBe(flag); + + const storedSegment = layer.get(VersionedDataKinds.Segments, 'segment1') as Segment; + expect(storedSegment._sdk_override).toBe(true); + expect(storedSegment.version).toEqual(3); + expect(storedSegment).not.toBe(segment); +}); + +it('prepares the copies for evaluation like data from LaunchDarkly', () => { + const layer = new OverrideLayer(); + layer.setAll([rawFlag('flag1')], [rawSegment('segment1')]); + + const storedFlag = layer.get(VersionedDataKinds.Features, 'flag1') as Flag; + const flagClause = storedFlag.rules![0].clauses![0]; + expect(flagClause.attributeReference).toBeInstanceOf(AttributeReference); + expect(flagClause.attributeReference.isValid).toBe(true); + + const storedSegment = layer.get(VersionedDataKinds.Segments, 'segment1') as Segment; + const segmentClause = storedSegment.rules![0].clauses[0]; + expect(segmentClause.attributeReference).toBeInstanceOf(AttributeReference); +}); + +it('replaces the whole layer with each snapshot', () => { + const layer = new OverrideLayer(); + expect(layer.isEmpty()).toBe(true); + expect(layer.get(VersionedDataKinds.Features, 'flag1')).toBeUndefined(); + + layer.setAll([rawFlag('flag1')], []); + expect(layer.isEmpty()).toBe(false); + expect(layer.get(VersionedDataKinds.Features, 'flag1')).toBeDefined(); + + // A replacement is a full snapshot. Entries absent from it are removed. + layer.setAll([rawFlag('flag2')], []); + expect(layer.get(VersionedDataKinds.Features, 'flag1')).toBeUndefined(); + expect(layer.get(VersionedDataKinds.Features, 'flag2')).toBeDefined(); + + layer.setAll([], []); + expect(layer.isEmpty()).toBe(true); + expect(layer.get(VersionedDataKinds.Features, 'flag2')).toBeUndefined(); +}); + +it('reports only segments as non-empty too', () => { + const layer = new OverrideLayer(); + layer.setAll([], [rawSegment('segment1')]); + expect(layer.isEmpty()).toBe(false); + expect(layer.get(VersionedDataKinds.Segments, 'segment1')).toBeDefined(); + expect(layer.get(VersionedDataKinds.Features, 'segment1')).toBeUndefined(); +}); + +it('returns all entries of a kind keyed by key', () => { + const layer = new OverrideLayer(); + layer.setAll([rawFlag('flag1'), rawFlag('flag2')], [rawSegment('segment1')]); + + const flags = layer.all(VersionedDataKinds.Features); + expect(Object.keys(flags).sort()).toEqual(['flag1', 'flag2']); + expect((flags.flag1 as Flag)._sdk_override).toBe(true); + expect(Object.keys(layer.all(VersionedDataKinds.Segments))).toEqual(['segment1']); + expect(new OverrideLayer().all(VersionedDataKinds.Features)).toEqual({}); +}); + +it('returns the previous and the new contents with the supplied text of each entry', () => { + const layer = new OverrideLayer(); + const first = layer.setAll([rawFlag('flag1', 1)], []); + expect(first.previous.features).toEqual({}); + expect(first.current.features.flag1.json).toEqual(JSON.stringify(rawFlag('flag1', 1))); + + const second = layer.setAll([rawFlag('flag1', 2)], [rawSegment('segment1')]); + expect(second.previous).toBe(first.current); + expect(second.current.features.flag1.json).toEqual(JSON.stringify(rawFlag('flag1', 2))); + expect(second.current.segments.segment1.item.key).toEqual('segment1'); +}); diff --git a/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts b/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts new file mode 100644 index 0000000000..8860ba31cd --- /dev/null +++ b/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts @@ -0,0 +1,199 @@ +import { LDFeatureStoreDataStorage } from '../../src/api/subsystems'; +import { OverrideLayer, OverrideSink } from '../../src/overrides'; +import AsyncStoreFacade from '../../src/store/AsyncStoreFacade'; +import InMemoryFeatureStore from '../../src/store/InMemoryFeatureStore'; +import VersionedDataKinds from '../../src/store/VersionedDataKinds'; +import TestLogger from '../Logger'; +import { singleValueFlag } from './overridesTestSupport'; + +function flagWithSegmentRule(key: string, segmentKey: string, version: number = 1): any { + return { + key, + version, + on: true, + fallthrough: { variation: 0 }, + variations: [true, false], + rules: [ + { + id: 'r', + variation: 0, + clauses: [{ attribute: '', op: 'segmentMatch', values: [segmentKey] }], + }, + ], + }; +} + +function flagWithPrerequisite(key: string, prereqKey: string, version: number = 1): any { + return { + ...singleValueFlag(key, true, version), + on: true, + prerequisites: [{ key: prereqKey, variation: 0 }], + }; +} + +// Notifications follow the layer replacement asynchronously. Let them run. +const settle = () => + new Promise((resolve) => { + setTimeout(resolve, 0); + }); + +describe('given a sink over an initialized base store', () => { + let base: InMemoryFeatureStore; + let layer: OverrideLayer; + let notified: string[]; + let listening: boolean; + let sink: OverrideSink; + let logger: TestLogger; + + const takeNotified = () => { + const result = notified.sort(); + notified = []; + return result; + }; + + const setupBase = async (data: LDFeatureStoreDataStorage) => { + await new AsyncStoreFacade(base).init(data); + }; + + beforeEach(() => { + base = new InMemoryFeatureStore(); + layer = new OverrideLayer(); + notified = []; + listening = true; + logger = new TestLogger(); + sink = new OverrideSink( + layer, + base, + (key) => notified.push(key), + () => listening, + logger, + ); + }); + + it('notifies on add, change, and remove', async () => { + await setupBase({ features: { flag1: singleValueFlag('flag1', 'base', 1) } }); + + // Adding an override is a change even though flag1 also exists in base data. + sink.setOverrides([singleValueFlag('flag1', 'a', 1), singleValueFlag('flag2', 'a', 1)], []); + await settle(); + expect(takeNotified()).toEqual(['flag1', 'flag2']); + + // An identical replacement, rebuilt from scratch, changes nothing. + sink.setOverrides([singleValueFlag('flag1', 'a', 1), singleValueFlag('flag2', 'a', 1)], []); + await settle(); + expect(takeNotified()).toEqual([]); + + // Changing one entry notifies only that entry. + sink.setOverrides([singleValueFlag('flag1', 'a', 1), singleValueFlag('flag2', 'b', 1)], []); + await settle(); + expect(takeNotified()).toEqual(['flag2']); + + // Removing overrides notifies them: flag1 reverts to base data, flag2 to not found. + sink.setOverrides([], []); + await settle(); + expect(takeNotified()).toEqual(['flag1', 'flag2']); + }); + + it('makes the new layer contents visible before the notifications run', () => { + sink.setOverrides([singleValueFlag('flag1', 'a')], []); + expect(layer.get(VersionedDataKinds.Features, 'flag1')).toBeDefined(); + expect(notified).toEqual([]); + }); + + it('fans a prerequisite override out to the flags that depend on it', async () => { + await setupBase({ + features: { + parent: flagWithPrerequisite('parent', 'prereq'), + grandparent: flagWithPrerequisite('grandparent', 'parent'), + unrelated: singleValueFlag('unrelated', true), + prereq: singleValueFlag('prereq', true, 1), + }, + }); + + sink.setOverrides([singleValueFlag('prereq', true, 99)], []); + await settle(); + expect(takeNotified()).toEqual(['grandparent', 'parent', 'prereq']); + }); + + it('fans a segment override out to the flags that depend on it', async () => { + await setupBase({ + features: { + dependent: flagWithSegmentRule('dependent', 'segment1'), + unrelated: singleValueFlag('unrelated', true), + }, + segments: { segment1: { key: 'segment1', version: 1 } }, + }); + + sink.setOverrides([], [{ key: 'segment1', version: 99 }]); + await settle(); + // The segment itself is not a flag, so only the dependent flag is notified. + expect(takeNotified()).toEqual(['dependent']); + }); + + it('notifies a removed override together with the entries that changed', async () => { + // The override for parent declares a prerequisite on prereq. The base definition of parent + // has no prerequisites. When the override is removed while prereq changes in the same + // replacement, parent is notified because its own entry changed. + await setupBase({ + features: { + parent: singleValueFlag('parent', true, 1), + prereq: singleValueFlag('prereq', true, 1), + }, + }); + + sink.setOverrides([flagWithPrerequisite('parent', 'prereq')], []); + await settle(); + expect(takeNotified()).toEqual(['parent']); + + // Replace the layer with an override of the prerequisite only. The parent override is removed + // (a change) and prereq is added (a change). + sink.setOverrides([singleValueFlag('prereq', true, 99)], []); + await settle(); + expect(takeNotified()).toEqual(['parent', 'prereq']); + + // Now only prereq is overridden and nothing depends on it in the new view either. + sink.setOverrides([singleValueFlag('prereq', true, 100)], []); + await settle(); + expect(takeNotified()).toEqual(['prereq']); + }); + + it('skips the change computation when nothing is listening', async () => { + listening = false; + const all = jest.spyOn(base, 'all'); + + sink.setOverrides([singleValueFlag('flag1', 'a')], []); + await settle(); + + expect(layer.get(VersionedDataKinds.Features, 'flag1')).toBeDefined(); + expect(all).not.toHaveBeenCalled(); + expect(notified).toEqual([]); + }); + + it('computes notifications for successive snapshots in order', async () => { + await setupBase({ features: {} }); + + sink.setOverrides([singleValueFlag('flag1', 'a')], []); + sink.setOverrides([singleValueFlag('flag2', 'a')], []); + sink.setOverrides([], []); + await settle(); + + // flag1 added, then flag1 removed and flag2 added, then flag2 removed. + expect(notified).toEqual(['flag1', 'flag1', 'flag2', 'flag2']); + }); + + it('logs and continues when the change computation fails', async () => { + base.all = () => { + throw new Error('store failure'); + }; + + sink.setOverrides([singleValueFlag('flag1', 'a')], []); + await settle(); + expect(logger.getCount('error' as any)).toEqual(1); + + // The next snapshot is still applied and its notifications still run. + base = new InMemoryFeatureStore(); + sink.setOverrides([singleValueFlag('flag2', 'a')], []); + await settle(); + expect(layer.get(VersionedDataKinds.Features, 'flag2')).toBeDefined(); + }); +}); diff --git a/packages/shared/sdk-server/__tests__/overrides/ReadStoreOverlay.test.ts b/packages/shared/sdk-server/__tests__/overrides/ReadStoreOverlay.test.ts new file mode 100644 index 0000000000..c7fc2d4467 --- /dev/null +++ b/packages/shared/sdk-server/__tests__/overrides/ReadStoreOverlay.test.ts @@ -0,0 +1,122 @@ +/* eslint-disable no-underscore-dangle */ +import { OverrideLayer, ReadStoreOverlay } from '../../src/overrides'; +import AsyncStoreFacade from '../../src/store/AsyncStoreFacade'; +import InMemoryFeatureStore from '../../src/store/InMemoryFeatureStore'; +import VersionedDataKinds from '../../src/store/VersionedDataKinds'; +import { singleValueFlag } from './overridesTestSupport'; + +function promiseGet(overlay: ReadStoreOverlay, namespace: 'features' | 'segments', key: string) { + const kind = namespace === 'features' ? VersionedDataKinds.Features : VersionedDataKinds.Segments; + return new Promise((resolve) => { + overlay.get(kind, key, resolve); + }); +} + +function promiseAll(overlay: ReadStoreOverlay) { + return new Promise((resolve) => { + overlay.all(VersionedDataKinds.Features, resolve); + }); +} + +function promiseInitialized(overlay: ReadStoreOverlay) { + return new Promise((resolve) => { + overlay.initialized(resolve); + }); +} + +describe('given an initialized base store with flags', () => { + let base: InMemoryFeatureStore; + let layer: OverrideLayer; + let overlay: ReadStoreOverlay; + + beforeEach(async () => { + base = new InMemoryFeatureStore(); + await new AsyncStoreFacade(base).init({ + features: { + both: singleValueFlag('both', 'base', 1), + 'base-only': singleValueFlag('base-only', 'base', 1), + tombstone: { key: 'tombstone', version: 5, deleted: true }, + }, + segments: { + segment: { key: 'segment', version: 1 }, + }, + }); + layer = new OverrideLayer(); + overlay = new ReadStoreOverlay(base, layer); + }); + + it('returns the override entry for a key the layer holds and the base entry otherwise', async () => { + layer.setAll( + [singleValueFlag('both', 'override', 99), singleValueFlag('override-only', 'override', 1)], + [], + ); + + const both = await promiseGet(overlay, 'features', 'both'); + expect(both.version).toEqual(99); + expect(both._sdk_override).toBe(true); + + const baseOnly = await promiseGet(overlay, 'features', 'base-only'); + expect(baseOnly.version).toEqual(1); + expect(baseOnly).not.toHaveProperty('_sdk_override'); + + const overrideOnly = await promiseGet(overlay, 'features', 'override-only'); + expect(overrideOnly._sdk_override).toBe(true); + + expect(await promiseGet(overlay, 'features', 'nowhere')).toBeNull(); + }); + + it('applies precedence to segments', async () => { + layer.setAll([], [{ key: 'segment', version: 7 }]); + const segment = await promiseGet(overlay, 'segments', 'segment'); + expect(segment.version).toEqual(7); + expect(segment._sdk_override).toBe(true); + }); + + it('returns the union of base and layer entries with the override entry winning', async () => { + layer.setAll( + [ + singleValueFlag('both', 'override', 99), + singleValueFlag('tombstone', 'override', 1), + singleValueFlag('override-only', 'override', 1), + ], + [], + ); + + const all = await promiseAll(overlay); + expect(Object.keys(all).sort()).toEqual(['base-only', 'both', 'override-only', 'tombstone']); + expect(all.both.version).toEqual(99); + expect(all.both._sdk_override).toBe(true); + expect(all['base-only']).not.toHaveProperty('_sdk_override'); + expect(all.tombstone._sdk_override).toBe(true); + expect(all['override-only']._sdk_override).toBe(true); + }); + + it('passes reads through unchanged when the layer is empty', async () => { + const all = await promiseAll(overlay); + expect(Object.keys(all).sort()).toEqual(['base-only', 'both']); + expect(await promiseGet(overlay, 'features', 'tombstone')).toBeNull(); + const both = await promiseGet(overlay, 'features', 'both'); + expect(both).not.toHaveProperty('_sdk_override'); + }); + + it('delegates initialized to the base store', async () => { + expect(await promiseInitialized(overlay)).toBe(true); + layer.setAll([singleValueFlag('flag', 'override')], []); + expect(await promiseInitialized(overlay)).toBe(true); + }); +}); + +describe('given an uninitialized base store', () => { + it('serves override entries and reports the base as not initialized', async () => { + const base = new InMemoryFeatureStore(); + const layer = new OverrideLayer(); + const overlay = new ReadStoreOverlay(base, layer); + layer.setAll([singleValueFlag('flag1', 'override')], []); + + const flag = await promiseGet(overlay, 'features', 'flag1'); + expect(flag._sdk_override).toBe(true); + expect(await promiseGet(overlay, 'features', 'other')).toBeNull(); + expect(Object.keys(await promiseAll(overlay))).toEqual(['flag1']); + expect(await promiseInitialized(overlay)).toBe(false); + }); +}); diff --git a/packages/shared/sdk-server/__tests__/overrides/TestOverrideSource.ts b/packages/shared/sdk-server/__tests__/overrides/TestOverrideSource.ts new file mode 100644 index 0000000000..53ca26d839 --- /dev/null +++ b/packages/shared/sdk-server/__tests__/overrides/TestOverrideSource.ts @@ -0,0 +1,66 @@ +import { + LDKeyedFeatureStoreItem, + LDOverrideSink, + LDOverrideSource, +} from '../../src/api/subsystems'; + +export interface OverrideData { + flags?: LDKeyedFeatureStoreItem[]; + segments?: LDKeyedFeatureStoreItem[]; +} + +/** + * A programmatic override source for testing the override system without any file machinery. + * It also serves as a reference for the seam an override source implements. Start delivers the + * initial data synchronously. setOverrides pushes replacement snapshots to the sink at any time + * afterward. With a deferred start, start returns a promise that the test completes. + */ +export default class TestOverrideSource implements LDOverrideSource { + public sink?: LDOverrideSink; + + public closed = false; + + private _completeStart?: () => void; + + constructor( + private readonly _initialData?: OverrideData, + private readonly _deferStart: boolean = false, + ) {} + + start(sink: LDOverrideSink): void | Promise { + this.sink = sink; + if (this._initialData) { + sink.setOverrides(this._initialData.flags ?? [], this._initialData.segments ?? []); + } + if (this._deferStart) { + return new Promise((resolve) => { + this._completeStart = resolve; + }); + } + return undefined; + } + + /** + * Completes a deferred start. + */ + completeStart() { + this._completeStart?.(); + } + + /** + * Replaces the override layer contents, as if the source's backing data had changed. + */ + setOverrides(flags: LDKeyedFeatureStoreItem[], segments: LDKeyedFeatureStoreItem[] = []) { + if (this.sink && !this.closed) { + this.sink.setOverrides(flags, segments); + } + } + + close(): void { + this.closed = true; + } + + get started(): boolean { + return this.sink !== undefined; + } +} diff --git a/packages/shared/sdk-server/__tests__/overrides/override-vectors/vectors.json b/packages/shared/sdk-server/__tests__/overrides/override-vectors/vectors.json new file mode 100644 index 0000000000..0910eefb87 --- /dev/null +++ b/packages/shared/sdk-server/__tests__/overrides/override-vectors/vectors.json @@ -0,0 +1,331 @@ +{ + "schemaVersion": "0.4.0", + "vectors": [ + { + "description": "An override takes precedence over LaunchDarkly data for the same flag key", + "group": "precedence", + "launchDarklyData": { + "initialized": true, + "flags": { + "flag-a": { + "key": "flag-a", + "version": 7, + "on": true, + "variations": ["ld-value"], + "fallthrough": { "variation": 0 }, + "offVariation": 0, + "salt": "salt-a" + } + }, + "segments": {} + }, + "overrides": { + "flagValues": { "flag-a": "override-value" } + }, + "evaluate": { + "flagKey": "flag-a", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "override-value", + "variationIndex": 0, + "reason": { "kind": "OFF", "overrideAffected": true }, + "summaryOverrideAffected": true + } + }, + { + "description": "An overridden flag is served when the SDK has not initialized from LaunchDarkly", + "group": "uninitialized", + "launchDarklyData": { + "initialized": false, + "flags": {}, + "segments": {} + }, + "overrides": { + "flagValues": { "flag-a": "override-value" } + }, + "evaluate": { + "flagKey": "flag-a", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "override-value", + "variationIndex": 0, + "reason": { "kind": "OFF", "overrideAffected": true }, + "summaryOverrideAffected": true + } + }, + { + "description": "A flag absent from the override layer is evaluated from LaunchDarkly data and is not marked", + "group": "passthrough", + "launchDarklyData": { + "initialized": true, + "flags": { + "flag-b": { + "key": "flag-b", + "version": 3, + "on": true, + "variations": ["off-value", "ld-value"], + "fallthrough": { "variation": 1 }, + "offVariation": 0, + "salt": "salt-b" + } + }, + "segments": {} + }, + "overrides": { + "flagValues": { "flag-a": "override-value" } + }, + "evaluate": { + "flagKey": "flag-b", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "ld-value", + "variationIndex": 1, + "reason": { "kind": "FALLTHROUGH" }, + "summaryOverrideAffected": false + } + }, + { + "description": "A flag absent from the override layer still short-circuits when the SDK has not initialized", + "group": "passthrough", + "launchDarklyData": { + "initialized": false, + "flags": {}, + "segments": {} + }, + "overrides": { + "flagValues": { "flag-a": "override-value" } + }, + "evaluate": { + "flagKey": "flag-b", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "fallback", + "variationIndex": null, + "reason": { "kind": "ERROR", "errorKind": "CLIENT_NOT_READY" } + } + }, + { + "description": "A full flag override with a matching targeting rule serves the rule's variation and is marked", + "group": "full-evaluation", + "launchDarklyData": { + "initialized": true, + "flags": {}, + "segments": {} + }, + "overrides": { + "flags": { + "flag-c": { + "key": "flag-c", + "version": 1, + "on": true, + "variations": ["default-value", "eu-value"], + "fallthrough": { "variation": 0 }, + "offVariation": 0, + "rules": [ + { + "id": "rule-eu", + "variation": 1, + "clauses": [ + { + "contextKind": "user", + "attribute": "region", + "op": "in", + "values": ["eu"], + "negate": false + } + ] + } + ], + "salt": "salt-c" + } + } + }, + "evaluate": { + "flagKey": "flag-c", + "context": { "kind": "user", "key": "user-1", "region": "eu" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "eu-value", + "variationIndex": 1, + "reason": { + "kind": "RULE_MATCH", + "ruleIndex": 0, + "ruleId": "rule-eu", + "overrideAffected": true + }, + "summaryOverrideAffected": true + } + }, + { + "description": "An override flag resolves a referenced segment through the merged view while the SDK is uninitialized", + "group": "merged-view", + "launchDarklyData": { + "initialized": false, + "flags": {}, + "segments": {} + }, + "overrides": { + "flags": { + "flag-d": { + "key": "flag-d", + "version": 1, + "on": true, + "variations": ["default-value", "member-value"], + "fallthrough": { "variation": 0 }, + "offVariation": 0, + "rules": [ + { + "id": "rule-seg", + "variation": 1, + "clauses": [ + { + "attribute": "", + "op": "segmentMatch", + "values": ["seg-1"], + "negate": false + } + ] + } + ], + "salt": "salt-d" + } + }, + "segments": { + "seg-1": { + "key": "seg-1", + "version": 1, + "included": ["user-1"], + "excluded": [], + "salt": "seg-salt-1" + } + } + }, + "evaluate": { + "flagKey": "flag-d", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "member-value", + "variationIndex": 1, + "reason": { + "kind": "RULE_MATCH", + "ruleIndex": 0, + "ruleId": "rule-seg", + "overrideAffected": true + }, + "summaryOverrideAffected": true + } + }, + { + "description": "A flag is marked overrideAffected when it matches via an overridden segment", + "group": "marking", + "launchDarklyData": { + "initialized": true, + "flags": { + "flag-f": { + "key": "flag-f", + "version": 4, + "on": true, + "variations": ["default-value", "member-value"], + "fallthrough": { "variation": 0 }, + "offVariation": 0, + "rules": [ + { + "id": "rule-seg", + "variation": 1, + "clauses": [ + { + "attribute": "", + "op": "segmentMatch", + "values": ["seg-2"], + "negate": false + } + ] + } + ], + "salt": "salt-f" + } + }, + "segments": { + "seg-2": { + "key": "seg-2", + "version": 1, + "included": [], + "excluded": [], + "salt": "seg-salt-2" + } + } + }, + "overrides": { + "segments": { + "seg-2": { + "key": "seg-2", + "version": 2, + "included": ["user-1"], + "excluded": [], + "salt": "seg-salt-2" + } + } + }, + "evaluate": { + "flagKey": "flag-f", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "member-value", + "variationIndex": 1, + "reason": { + "kind": "RULE_MATCH", + "ruleIndex": 0, + "ruleId": "rule-seg", + "overrideAffected": true + }, + "summaryOverrideAffected": true + } + }, + { + "description": "A malformed override flag returns the caller default with an error reason and is marked overrideAffected", + "group": "error-handling", + "launchDarklyData": { + "initialized": true, + "flags": {}, + "segments": {} + }, + "overrides": { + "flags": { + "flag-e": { + "key": "flag-e", + "version": 1, + "on": true, + "variations": ["only-value"], + "fallthrough": { "variation": 5 }, + "offVariation": 0, + "salt": "salt-e" + } + } + }, + "evaluate": { + "flagKey": "flag-e", + "context": { "kind": "user", "key": "user-1" }, + "defaultValue": "fallback" + }, + "expect": { + "value": "fallback", + "variationIndex": null, + "reason": { "kind": "ERROR", "errorKind": "MALFORMED_FLAG", "overrideAffected": true }, + "summaryOverrideAffected": true + } + } + ] +} diff --git a/packages/shared/sdk-server/__tests__/overrides/overridesTestSupport.ts b/packages/shared/sdk-server/__tests__/overrides/overridesTestSupport.ts new file mode 100644 index 0000000000..34e5b3e227 --- /dev/null +++ b/packages/shared/sdk-server/__tests__/overrides/overridesTestSupport.ts @@ -0,0 +1,105 @@ +import { LDOptions } from '../../src/api/options/LDOptions'; +import LDClientImpl, { LDClientCallbacks } from '../../src/LDClientImpl'; +import { createBasicPlatform } from '../createBasicPlatform'; + +/** + * The body of an FDv2 polling response that transfers the given flags and segments as a full + * payload with a selector, which is what makes the client initialize. + */ +export function fdv2FullPayload( + flags: Record = {}, + segments: Record = {}, +): string { + const events: any[] = [ + { + event: 'server-intent', + data: { payloads: [{ intentCode: 'xfer-full', id: 'test-payload', target: 1 }] }, + }, + ]; + Object.entries(flags).forEach(([key, flag]) => { + events.push({ + event: 'put-object', + data: { kind: 'flag', key, version: flag.version, object: flag }, + }); + }); + Object.entries(segments).forEach(([key, segment]) => { + events.push({ + event: 'put-object', + data: { kind: 'segment', key, version: segment.version, object: segment }, + }); + }); + events.push({ event: 'payload-transferred', data: { state: 'test-state', version: 1 } }); + return JSON.stringify({ events }); +} + +/** + * A platform whose polling requests return the given body. With no body, requests never + * complete, so the client never initializes. + */ +export function makeFDv2Platform(body?: string) { + const platform = createBasicPlatform(); + platform.requests.fetch = jest.fn(() => { + if (body === undefined) { + return new Promise(() => {}); + } + return Promise.resolve({ + status: 200, + headers: new Headers(), + text: async () => body, + }); + }); + return platform; +} + +export function makeCallbacks( + onUpdate: (key: string) => void = () => {}, + hasEventListeners: boolean = false, +): LDClientCallbacks { + return { + onError: jest.fn(), + onFailed: jest.fn(), + onReady: jest.fn(), + onUpdate, + hasEventListeners: () => hasEventListeners, + }; +} + +/** + * Creates a client that uses the FDv2 data system with a polling synchronizer, so that the mock + * platform decides whether and with what data it initializes. + */ +export function makeFDv2Client( + platform: ReturnType, + options: LDOptions, + callbacks: LDClientCallbacks = makeCallbacks(), +): LDClientImpl { + return new LDClientImpl( + 'sdk-key-overrides', + platform, + { + sendEvents: false, + diagnosticOptOut: true, + ...options, + dataSystem: { + dataSource: { + dataSourceOptionsType: 'custom', + initializers: [], + synchronizers: [{ type: 'polling', pollInterval: 1000 }], + }, + ...options.dataSystem, + }, + }, + callbacks, + ); +} + +export function singleValueFlag(key: string, value: any, version: number = 1): any { + return { + key, + version, + on: false, + offVariation: 0, + fallthrough: { variation: 0 }, + variations: [value], + }; +} diff --git a/packages/shared/sdk-server/src/FlagsStateBuilder.ts b/packages/shared/sdk-server/src/FlagsStateBuilder.ts index 1ca0613ccd..a77e1e104f 100644 --- a/packages/shared/sdk-server/src/FlagsStateBuilder.ts +++ b/packages/shared/sdk-server/src/FlagsStateBuilder.ts @@ -30,6 +30,7 @@ export default class FlagsStateBuilder { reason: LDEvaluationReason, trackEvents: boolean, trackReason: boolean, + debugEventsUntilDate: number | undefined, detailsOnlyIfTracked: boolean, prerequisites?: string[], ) { @@ -39,10 +40,7 @@ export default class FlagsStateBuilder { meta.variation = variation; } const omitDetails = - detailsOnlyIfTracked && - !trackEvents && - !trackReason && - flag.debugEventsUntilDate === undefined; // 0 probably doesn't matter. + detailsOnlyIfTracked && !trackEvents && !trackReason && debugEventsUntilDate === undefined; // 0 probably doesn't matter. if (!omitDetails) { meta.version = flag.version; } @@ -55,8 +53,8 @@ export default class FlagsStateBuilder { if (trackReason) { meta.trackReason = true; } - if (flag.debugEventsUntilDate !== undefined) { - meta.debugEventsUntilDate = flag.debugEventsUntilDate; + if (debugEventsUntilDate !== undefined) { + meta.debugEventsUntilDate = debugEventsUntilDate; } if (prerequisites && prerequisites.length) { meta.prerequisites = prerequisites; diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 38329c97b7..5e3f1335e2 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -22,12 +22,14 @@ import { IsMigrationStage, LDClient, LDFeatureStore, + LDFeatureStoreKindData, LDFlagsState, LDFlagsStateOptions, LDMigrationOpEvent, LDMigrationStage, LDMigrationVariation, LDOptions, + LDOverrideSource, LDTransactionalFeatureStore, } from './api'; import { Hook } from './api/integrations/Hook'; @@ -72,6 +74,13 @@ import Configuration, { DEFAULT_STREAM_RECONNECT_DELAY, } from './options/Configuration'; import { ServerInternalOptions } from './options/ServerInternalOptions'; +import { + createOverrideSource, + OverrideLayer, + OverrideSink, + ReadStore, + ReadStoreOverlay, +} from './overrides'; import VersionedDataKinds from './store/VersionedDataKinds'; const { ClientMessages, ErrorKinds, NullEventProcessor } = internal; @@ -284,6 +293,8 @@ function constructFDv2( logger: LDLogger | undefined; evaluator: Evaluator; featureStore: LDTransactionalFeatureStore; + readStore: ReadStore; + overrides: OverrideComponents | undefined; dataSource: subsystem.DataSource | undefined; payloadListener: ((payload: any) => void) | undefined; eventProcessor: subsystem.LDEventProcessor; @@ -328,6 +339,22 @@ function constructFDv2( onUpdate, ); + // The override layer sits at the store read boundary. Evaluation, prerequisite and segment + // resolution, and the all-flags read go through the overlay, so an override entry wins over + // LaunchDarkly data for its key. The data system never sees the layer. The source is created + // here so that an invalid configuration fails client construction. + let readStore: ReadStore = featureStore; + let overrides: OverrideComponents | undefined; + if (dataSystem.overrides !== undefined && !config.offline) { + const layer = new OverrideLayer(); + readStore = new ReadStoreOverlay(featureStore, layer); + overrides = { + layer, + sink: new OverrideSink(layer, featureStore, onUpdate, hasEventListeners, logger), + source: createOverrideSource(dataSystem.overrides, clientContext), + }; + } + let diagnosticsManager: internal.DiagnosticsManager | undefined; if (config.sendEvents && !config.offline && !config.diagnosticOptOut) { diagnosticsManager = new internal.DiagnosticsManager( @@ -362,10 +389,10 @@ function constructFDv2( const queries: Queries = { getFlag(key: string, cb: (flag: Flag | undefined) => void): void { - featureStore.get(VersionedDataKinds.Features, key, (item) => cb(item as Flag)); + readStore.get(VersionedDataKinds.Features, key, (item) => cb(item as Flag)); }, getSegment(key: string, cb: (segment: Segment | undefined) => void): void { - featureStore.get(VersionedDataKinds.Segments, key, (item) => cb(item as Segment)); + readStore.get(VersionedDataKinds.Segments, key, (item) => cb(item as Segment)); }, getBigSegmentsMembership( userKey: string, @@ -567,6 +594,8 @@ function constructFDv2( logger, evaluator, featureStore, + readStore, + overrides, dataSource, payloadListener, eventProcessor, @@ -578,6 +607,15 @@ function constructFDv2( }; } +/** + * The parts of the override system that the client owns when an override source is configured. + */ +interface OverrideComponents { + layer: OverrideLayer; + sink: OverrideSink; + source: LDOverrideSource; +} + /** * @ignore */ @@ -590,8 +628,23 @@ export default class LDClientImpl implements LDClient { // The allFlagsState last-known-values warning is logged once per client. private _allFlagsStateLastKnownValuesWarningLogged = false; + // The allFlagsState overrides-only warning is logged once per client. + private _allFlagsStateOverridesOnlyWarningLogged = false; + private _featureStore: LDFeatureStore | LDTransactionalFeatureStore; + // The store read boundary. With an override source this is the overlay of the override layer + // over the feature store. Otherwise it is the feature store itself. + private _readStore: ReadStore; + + private _overrideLayer?: OverrideLayer; + + private _overrideSource?: LDOverrideSource; + + // Settles when the override source's initial load has completed. Undefined once it has, or when + // there is no override source. + private _overridesReady?: Promise; + private _updateProcessor?: subsystem.LDStreamProcessor; private _dataSource?: subsystem.DataSource; @@ -693,6 +746,7 @@ export default class LDClientImpl implements LDClient { internalOptions?.userAgentHeaderName, startEventProcessor, )); + this._readStore = this._featureStore; this.bigSegmentStatusProviderInternal = this._bigSegmentsManager .statusProvider as BigSegmentStoreStatusProvider; @@ -708,11 +762,14 @@ export default class LDClientImpl implements LDClient { // setup for FDv2 let transactionalStore: LDTransactionalFeatureStore; let payloadListener: ((payload: any) => void) | undefined; + let overrides: OverrideComponents | undefined; ({ config: this._config, logger: this._logger, evaluator: this._evaluator, featureStore: transactionalStore, + readStore: this._readStore, + overrides, dataSource: this._dataSource, payloadListener, eventProcessor: this._eventProcessor, @@ -736,6 +793,15 @@ export default class LDClientImpl implements LDClient { this.bigSegmentStatusProviderInternal = this._bigSegmentsManager .statusProvider as BigSegmentStoreStatusProvider; + if (overrides) { + // The source starts before the data source, so an override that loads synchronously is in + // place before the client evaluates anything. The evaluation methods wait for an + // asynchronous initial load to complete. + this._overrideLayer = overrides.layer; + this._overrideSource = overrides.source; + this._overridesReady = this._startOverrideSource(overrides.source, overrides.sink); + } + if (this._dataSource) { this._dataSource.start( (_, payload) => { @@ -1146,52 +1212,73 @@ export default class LDClientImpl implements LDClient { } return new Promise((resolve) => { - const doEval = (valid: boolean) => - this._featureStore.all(VersionedDataKinds.Features, (allFlags) => { - const builder = new FlagsStateBuilder(valid, !!options?.withReasons); - const clientOnly = !!options?.clientSideOnly; - const detailsOnlyIfTracked = !!options?.detailsOnlyForTrackedFlags; - - allAsync( - Object.values(allFlags), - (storeItem, iterCb) => { - const flag = storeItem as Flag; - if (clientOnly && !flag.clientSideAvailability?.usingEnvironmentId) { - iterCb(true); - return; - } - this._evaluator.evaluateCb(flag, evalContext, (res) => { - if (res.isError) { - this._onError( - new Error( - `Error for feature flag "${flag.key}" while evaluating all flags: ${res.message}`, - ), - ); - } - const requireExperimentData = isExperiment(flag, res.detail.reason); - builder.addFlag( - flag, - res.detail.value, - res.detail.variationIndex ?? undefined, - res.detail.reason, - flag.trackEvents || requireExperimentData, - requireExperimentData, - detailsOnlyIfTracked, - res.prerequisites, + const evaluateAll = (valid: boolean, allFlags: LDFeatureStoreKindData) => { + const builder = new FlagsStateBuilder(valid, !!options?.withReasons); + const clientOnly = !!options?.clientSideOnly; + const detailsOnlyIfTracked = !!options?.detailsOnlyForTrackedFlags; + + allAsync( + Object.values(allFlags), + (storeItem, iterCb) => { + const flag = storeItem as Flag; + if (clientOnly && !flag.clientSideAvailability?.usingEnvironmentId) { + iterCb(true); + return; + } + this._evaluator.evaluateCb(flag, evalContext, (res) => { + if (res.isError) { + this._onError( + new Error( + `Error for feature flag "${flag.key}" while evaluating all flags: ${res.message}`, + ), ); - iterCb(true); - }); - }, - () => { - const res = builder.build(); - callback?.(null, res); - resolve(res); - }, - ); - }); - if (!this.initialized()) { + } + const requireExperimentData = isExperiment(flag, res.detail.reason); + let trackEvents = flag.trackEvents || requireExperimentData; + let trackReason = requireExperimentData; + let { debugEventsUntilDate } = flag; + if (res.overrideAffected) { + // A consumer of this state sends individual events according to these fields. An + // override-affected evaluation produces no individual events, so the state turns + // them off for this flag. The flag, its value, and its reason stay. + trackEvents = false; + trackReason = false; + debugEventsUntilDate = undefined; + } + builder.addFlag( + flag, + res.detail.value, + res.detail.variationIndex ?? undefined, + res.detail.reason, + trackEvents, + trackReason, + debugEventsUntilDate, + detailsOnlyIfTracked, + res.prerequisites, + ); + iterCb(true); + }); + }, + () => { + const res = builder.build(); + callback?.(null, res); + resolve(res); + }, + ); + }; + // The read goes through the store read boundary, so it has override precedence and includes + // flags that exist only in the override layer. + const doEval = (valid: boolean) => + this._readStore.all(VersionedDataKinds.Features, (allFlags) => + evaluateAll(valid, allFlags), + ); + + this._afterOverridesReady(() => { + if (this.initialized()) { + doEval(true); + return; + } this._featureStore.initialized((storeInitialized) => { - let valid = true; if (storeInitialized) { if (!this._allFlagsStateLastKnownValuesWarningLogged) { this._allFlagsStateLastKnownValuesWarningLogged = true; @@ -1200,18 +1287,29 @@ export default class LDClientImpl implements LDClient { ' values from data store. This message is logged once.', ); } - } else { - this._logger?.warn( - 'Called allFlagsState before client initialization. Data store not available; ' + - 'returning empty state', - ); - valid = false; + doEval(true); + return; } - doEval(valid); + if (this._overrideLayer && !this._overrideLayer.isEmpty()) { + // No LaunchDarkly data is available. The state holds only the flags that the + // override layer holds. + if (!this._allFlagsStateOverridesOnlyWarningLogged) { + this._allFlagsStateOverridesOnlyWarningLogged = true; + this._logger?.warn( + 'Called allFlagsState before client initialization; returning only flags from' + + ' the override layer. This message is logged once.', + ); + } + doEval(true); + return; + } + this._logger?.warn( + 'Called allFlagsState before client initialization. Data store not available; ' + + 'returning empty state', + ); + doEval(false); }); - } else { - doEval(true); - } + }); }); } @@ -1232,6 +1330,7 @@ export default class LDClientImpl implements LDClient { } close(): void { + this._overrideSource?.close(); this._eventProcessor.close(); this._updateProcessor?.close(); this._dataSource?.stop(); @@ -1315,7 +1414,7 @@ export default class LDClientImpl implements LDClient { return; } - this._featureStore.get(VersionedDataKinds.Features, flagKey, (item) => { + this._readStore.get(VersionedDataKinds.Features, flagKey, (item) => { const flag = item as Flag; if (!flag) { const error = new LDClientError( @@ -1349,6 +1448,12 @@ export default class LDClientImpl implements LDClient { `Did not receive expected type (${type}) evaluating feature flag "${flagKey}"`, defaultValue, ); + if (evalRes.overrideAffected) { + // The type mismatch replaces the reason. The evaluation read the same definitions, + // so the new result keeps the override-affected marking. + errorRes.detail.reason = { ...errorRes.detail.reason, overrideAffected: true }; + errorRes.overrideAffected = true; + } this._sendEvalEvent(errorRes, eventFactory, flag, evalContext, defaultValue); cb(errorRes, flag); return; @@ -1386,29 +1491,72 @@ export default class LDClientImpl implements LDClient { cb: (res: EvalResult, flag?: Flag) => void, typeChecker?: (value: any) => [boolean, string], ): void { - if (!this.initialized()) { - this._featureStore.initialized((storeInitialized) => { - if (storeInitialized) { - if (!this._lastKnownValuesWarningLogged) { - this._lastKnownValuesWarningLogged = true; - this._logger?.warn( - 'Variation called before LaunchDarkly client initialization completed' + - " (did you wait for the 'ready' event?) - using last known values from feature store." + - ' This message is logged once.', - ); + this._afterOverridesReady(() => { + if (!this.initialized()) { + this._featureStore.initialized((storeInitialized) => { + if (storeInitialized) { + if (!this._lastKnownValuesWarningLogged) { + this._lastKnownValuesWarningLogged = true; + this._logger?.warn( + 'Variation called before LaunchDarkly client initialization completed' + + " (did you wait for the 'ready' event?) - using last known values from feature store." + + ' This message is logged once.', + ); + } + this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); + return; } - this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); - return; - } - this._logger?.warn( - 'Variation called before LaunchDarkly client initialization completed (did you wait for the' + - "'ready' event?) - using default value", - ); - cb(EvalResult.forError(ErrorKinds.ClientNotReady, undefined, defaultValue)); + if (this._overrideLayer?.get(VersionedDataKinds.Features, flagKey)) { + // No LaunchDarkly data is available, but the override layer holds this flag. The + // override is served before the not-initialized short-circuit. A flag the layer does + // not hold still gets the not-ready default. + this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); + return; + } + this._logger?.warn( + 'Variation called before LaunchDarkly client initialization completed (did you wait for the' + + "'ready' event?) - using default value", + ); + cb(EvalResult.forError(ErrorKinds.ClientNotReady, undefined, defaultValue)); + }); + return; + } + this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); + }); + } + + /** + * Starts the override source. The returned promise settles when the source's initial load has + * completed, also when it failed. A failure never prevents the client from operating: the + * layer stays as it was and the failure is logged. + */ + private _startOverrideSource(source: LDOverrideSource, sink: OverrideSink): Promise { + let started: Promise; + try { + started = Promise.resolve(source.start(sink)); + } catch (err) { + started = Promise.reject(err); + } + return started + .catch((err) => { + this._logger?.error(`Unable to start the override source: ${err}`); + }) + .then(() => { + this._overridesReady = undefined; }); + } + + /** + * Runs the action once the override source's initial load has completed, so an override that + * is present when the client is created takes effect from the first evaluation. Without an + * override source, or once the load has completed, the action runs immediately. + */ + private _afterOverridesReady(action: () => void): void { + if (this._overridesReady) { + this._overridesReady.then(action); return; } - this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); + action(); } private _dataSourceErrorHandler(e: any) { diff --git a/packages/shared/sdk-server/src/api/index.ts b/packages/shared/sdk-server/src/api/index.ts index 2e656ad5a9..28abb3385d 100644 --- a/packages/shared/sdk-server/src/api/index.ts +++ b/packages/shared/sdk-server/src/api/index.ts @@ -5,6 +5,7 @@ export * from './LDMigration'; export * from './interfaces/DataKind'; export * from './subsystems/LDFeatureStore'; export * from './subsystems/LDTransactionalFeatureStore'; +export * from './subsystems/LDOverrideSource'; export * from './LDWaitForInitializationOptions'; // These are items that should be less frequently used, and therefore they diff --git a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts index 2df869a2c5..23a110e867 100644 --- a/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts +++ b/packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts @@ -1,6 +1,6 @@ import { LDClientContext } from '@launchdarkly/js-sdk-common'; -import { LDFeatureStore } from '../subsystems'; +import { LDFeatureStore, LDOverrideSource } from '../subsystems'; /** * Configuration options for the Data System that the SDK uses to get and maintain flags and other @@ -77,8 +77,37 @@ export interface LDDataSystemOptions { * will transition to a terminal Closed state when the directive is received. */ fdv1Fallback?: FDv1FallbackConfiguration | null; + + /** + * Configures an override source. Flag overrides are currently experimental and subject to + * change. + * + * The source supplies flag and segment definitions that take precedence over data received + * from LaunchDarkly on a per-key basis. Overrides let an operator force one or more flags to a + * known state on a running client, whether or not the client can reach LaunchDarkly. Flags not + * present in the override data are unaffected. + * + * The override source is not a data source. It has no effect on the client's initialization + * status or data source status. Configuring it changes nothing until the source supplies an + * override. At most one override source can be configured. + * + * Some implementations provide the source object itself, while others provide a factory + * function that creates the source based on the SDK configuration. This property accepts + * either. + */ + overrides?: LDOverrideSourceOptions; } +/** + * The ways an override source can be configured: the source itself, or a factory function that + * creates it from the client context. + * + * Flag overrides are currently experimental and subject to change. + */ +export type LDOverrideSourceOptions = + | LDOverrideSource + | ((clientContext: LDClientContext) => LDOverrideSource); + /** * Configuration options for the FDv1 Fallback Synchronizer. */ diff --git a/packages/shared/sdk-server/src/api/subsystems/LDOverrideSource.ts b/packages/shared/sdk-server/src/api/subsystems/LDOverrideSource.ts new file mode 100644 index 0000000000..6d30a198ff --- /dev/null +++ b/packages/shared/sdk-server/src/api/subsystems/LDOverrideSource.ts @@ -0,0 +1,64 @@ +import { LDKeyedFeatureStoreItem } from './LDFeatureStore'; + +/** + * Receives the contents of the SDK's flag and segment override layer. The SDK implements this + * interface and passes it to {@link LDOverrideSource.start}. Override sources call it. They do + * not implement it. + * + * Flag overrides are currently experimental and subject to change. + */ +export interface LDOverrideSink { + /** + * Replaces the entire override layer with the given flag and segment definitions. Each call is + * a full snapshot. Entries absent from the call are removed from the layer, and empty arrays + * clear the layer. + * + * The definitions are plain objects in the flag and segment data model, in the form in which + * LaunchDarkly delivers them. The SDK copies them, prepares the copies for evaluation, and + * marks the copies as override entries. It never modifies the objects it was given. + * + * The new contents are visible to evaluations when the call returns. Flag change notifications + * for the affected flags follow. + * + * @param flags The full flag definitions of the layer. + * @param segments The full segment definitions of the layer. + */ + setOverrides(flags: LDKeyedFeatureStoreItem[], segments: LDKeyedFeatureStoreItem[]): void; +} + +/** + * Supplies flag and segment overrides that take precedence over LaunchDarkly data at evaluation + * time, on a per-key basis. Overrides exist for resilience during an incident. They let an + * operator force one or more flags to a known state on a running client, whether or not the + * client can reach LaunchDarkly. + * + * An override source is not a data source. It takes no part in the data system's initializer and + * synchronizer pipeline. The override layer it populates has no effect on the client's + * initialization status or data source status. + * + * Configure an override source with the `overrides` property of the data system options. + * + * Flag overrides are currently experimental and subject to change. + */ +export interface LDOverrideSource { + /** + * Starts supplying overrides to the sink. An implementation performs its initial load and then + * pushes a full replacement snapshot to the sink whenever its backing data changes, until + * {@link close} is called. A failed load leaves the previously supplied layer untouched by not + * calling the sink. + * + * The SDK calls this method once, when the client is created. When the initial load is + * asynchronous, return a promise that settles when the load completes. The SDK waits for it + * before it evaluates flags, so an override that is present at startup takes effect from the + * first evaluation. + * + * @param sink The sink that receives the snapshots. + */ + start(sink: LDOverrideSink): void | Promise; + + /** + * Stops the source and releases any resources it holds. The SDK calls this method when the + * client is closed. + */ + close(): void; +} diff --git a/packages/shared/sdk-server/src/api/subsystems/index.ts b/packages/shared/sdk-server/src/api/subsystems/index.ts index 56390adf33..4117e18af2 100644 --- a/packages/shared/sdk-server/src/api/subsystems/index.ts +++ b/packages/shared/sdk-server/src/api/subsystems/index.ts @@ -1,5 +1,6 @@ export * from './LDDataSourceUpdates'; export * from './LDFeatureRequestor'; export * from './LDFeatureStore'; +export * from './LDOverrideSource'; export * from './LDTransactionalDataSourceUpdates'; export * from './LDTransactionalFeatureStore'; diff --git a/packages/shared/sdk-server/src/evaluation/data/overrideMarker.ts b/packages/shared/sdk-server/src/evaluation/data/overrideMarker.ts index 75275c769b..1b5e8a900d 100644 --- a/packages/shared/sdk-server/src/evaluation/data/overrideMarker.ts +++ b/packages/shared/sdk-server/src/evaluation/data/overrideMarker.ts @@ -46,6 +46,17 @@ export function isOverrideEntry(item: OverrideMarkable | undefined | null): bool return ownMarker(item) === true; } +/** + * Marks an entity as an override entry. Only the override store calls this, on copies that it + * owns. + * + * @internal + */ +export function markOverrideEntry(item: T): T { + item._sdk_override = true; + return item; +} + /** * Reports whether a value carries the override marker key at all. * diff --git a/packages/shared/sdk-server/src/options/Configuration.ts b/packages/shared/sdk-server/src/options/Configuration.ts index 87617c14c0..ba2bd2ad7d 100644 --- a/packages/shared/sdk-server/src/options/Configuration.ts +++ b/packages/shared/sdk-server/src/options/Configuration.ts @@ -22,6 +22,7 @@ import { isStandardOptions, isStreamingOnlyOptions, LDDataSystemOptions, + LDOverrideSourceOptions, PollingDataSourceOptions, StandardDataSourceOptions, StreamingDataSourceOptions, @@ -312,6 +313,19 @@ function validateDataSystemOptions(options: Options): { } } + // The override source is either a source object or a factory function. Anything else is a + // misconfiguration. Drop it so the client runs without overrides, and warn. + if (options.overrides !== undefined && !TypeValidators.ObjectOrFactory.is(options.overrides)) { + validatedOptions.overrides = undefined; + allErrors.push( + OptionMessages.wrongOptionType( + 'dataSystem.overrides', + 'LDOverrideSourceOptions', + typeof options.overrides, + ), + ); + } + if (options.dataSource) { let errors: string[]; let validatedDataSourceOptions: Options; @@ -367,6 +381,7 @@ export interface DataSystemConfiguration { featureStoreFactory: (clientContext: LDClientContext) => LDTransactionalFeatureStore; useLdd?: boolean; fdv1Fallback?: FDv1FallbackConfiguration | null; + overrides?: LDOverrideSourceOptions; } /** @@ -478,6 +493,7 @@ export default class Configuration { dataSource: validatedDSOptions.dataSource, useLdd: validatedDSOptions.useLdd, fdv1Fallback: validatedDSOptions.fdv1Fallback, + overrides: validatedDSOptions.overrides, featureStoreFactory: (clientContext): LDTransactionalFeatureStore => { const { persistentStore } = validatedDSOptions; let store: LDFeatureStore; diff --git a/packages/shared/sdk-server/src/overrides/OverrideLayer.ts b/packages/shared/sdk-server/src/overrides/OverrideLayer.ts new file mode 100644 index 0000000000..931c6c88d7 --- /dev/null +++ b/packages/shared/sdk-server/src/overrides/OverrideLayer.ts @@ -0,0 +1,129 @@ +import { DataKind } from '../api/interfaces'; +import { + LDFeatureStoreItem, + LDFeatureStoreKindData, + LDKeyedFeatureStoreItem, +} from '../api/subsystems'; +import { Flag } from '../evaluation/data/Flag'; +import { markOverrideEntry } from '../evaluation/data/overrideMarker'; +import { Segment } from '../evaluation/data/Segment'; +import { processFlag, processSegment } from '../store/serialization'; +import VersionedDataKinds from '../store/VersionedDataKinds'; + +/** + * One entry of the layer: the prepared, marked definition, and the JSON text of the definition + * as the source supplied it. The text identifies the entry for change comparison, because the + * layer builds new copies on every snapshot. + * + * @internal + */ +export interface LayerEntry { + item: LDFeatureStoreItem; + json: string; +} + +/** + * The entries of one kind, keyed by item key. + * + * @internal + */ +export type LayerKindContents = Record; + +/** + * The entries of the layer, keyed by namespace and then by item key. + * + * @internal + */ +export type LayerContents = Record; + +/** + * Copies a definition the source supplied, prepares the copy for evaluation the way the SDK + * prepares LaunchDarkly data, and marks it. The source's object is never modified. + */ +function prepareFlag(item: LDKeyedFeatureStoreItem): LayerEntry { + const json = JSON.stringify(item); + const flag = JSON.parse(json) as Flag; + processFlag(flag); + markOverrideEntry(flag); + return { item: flag, json }; +} + +function prepareSegment(item: LDKeyedFeatureStoreItem): LayerEntry { + const json = JSON.stringify(item); + const segment = JSON.parse(json) as Segment; + processSegment(segment); + markOverrideEntry(segment); + return { item: segment, json }; +} + +function emptyContents(): LayerContents { + return { + [VersionedDataKinds.Features.namespace]: {}, + [VersionedDataKinds.Segments.namespace]: {}, + }; +} + +/** + * The override store. It holds the entries the override source has loaded, keyed by flag key or + * segment key, and is replaced wholesale on each snapshot from the source. Every entry carries + * the override marker. + * + * @internal + */ +export default class OverrideLayer { + private _contents: LayerContents = emptyContents(); + + private _empty = true; + + /** + * Replaces the entire contents of the layer in one assignment, so the layer holds exactly one + * snapshot at any instant. Empty arrays clear the layer. + * + * @returns The previous and the new contents, for change comparison. Neither may be modified. + */ + setAll( + flags: LDKeyedFeatureStoreItem[], + segments: LDKeyedFeatureStoreItem[], + ): { previous: LayerContents; current: LayerContents } { + const current = emptyContents(); + flags.forEach((flag) => { + current[VersionedDataKinds.Features.namespace][flag.key] = prepareFlag(flag); + }); + segments.forEach((segment) => { + current[VersionedDataKinds.Segments.namespace][segment.key] = prepareSegment(segment); + }); + const previous = this._contents; + this._contents = current; + this._empty = flags.length === 0 && segments.length === 0; + return { previous, current }; + } + + /** + * Returns the override entry for a key, if any. + */ + get(kind: DataKind, key: string): LDFeatureStoreItem | undefined { + if (this._empty) { + return undefined; + } + return this._contents[kind.namespace]?.[key]?.item; + } + + /** + * Returns the entries of the given kind, keyed by item key. + */ + all(kind: DataKind): LDFeatureStoreKindData { + const result: LDFeatureStoreKindData = {}; + Object.entries(this._contents[kind.namespace] ?? {}).forEach(([key, entry]) => { + result[key] = entry.item; + }); + return result; + } + + /** + * Reports whether the layer holds no entries. A configured but unpopulated layer costs one + * check per read. + */ + isEmpty(): boolean { + return this._empty; + } +} diff --git a/packages/shared/sdk-server/src/overrides/OverrideSink.ts b/packages/shared/sdk-server/src/overrides/OverrideSink.ts new file mode 100644 index 0000000000..025c8b16a2 --- /dev/null +++ b/packages/shared/sdk-server/src/overrides/OverrideSink.ts @@ -0,0 +1,163 @@ +import { LDLogger } from '@launchdarkly/js-sdk-common'; + +import { LDFeatureStoreKindData, LDKeyedFeatureStoreItem, LDOverrideSink } from '../api/subsystems'; +import { computeDependencies } from '../data_sources/DataSourceUpdates'; +import DependencyTracker from '../data_sources/DependencyTracker'; +import NamespacedDataSet from '../data_sources/NamespacedDataSet'; +import VersionedDataKinds from '../store/VersionedDataKinds'; +import OverrideLayer, { LayerContents } from './OverrideLayer'; +import { ReadStore } from './ReadStoreOverlay'; + +const diffNamespaces = [ + VersionedDataKinds.Features.namespace, + VersionedDataKinds.Segments.namespace, +]; + +/** + * The data visible at the store read boundary: base data with override entries overlaid. + */ +type MergedView = Record; + +/** + * Returns the keys whose override entry differs between two layer snapshots. An added or removed + * entry is always a change, even when its content matches the underlying LaunchDarkly data: the + * override marker alone changes the served entry. Entries present in both snapshots are compared + * by the text the source supplied, because the layer is rebuilt on every snapshot. + */ +function diffContents( + previous: LayerContents, + current: LayerContents, +): { seeds: NamespacedDataSet; count: number } { + const seeds = new NamespacedDataSet(); + let count = 0; + diffNamespaces.forEach((namespace) => { + const oldItems = previous[namespace] ?? {}; + const newItems = current[namespace] ?? {}; + Object.entries(oldItems).forEach(([key, oldEntry]) => { + const newEntry = newItems[key]; + if (!newEntry || newEntry.json !== oldEntry.json) { + seeds.set(namespace, key, true); + count += 1; + } + }); + Object.keys(newItems).forEach((key) => { + if (!oldItems[key]) { + seeds.set(namespace, key, true); + count += 1; + } + }); + }); + return { seeds, count }; +} + +function mergedView(base: MergedView, contents: LayerContents): MergedView { + const view: MergedView = {}; + diffNamespaces.forEach((namespace) => { + const items: LDFeatureStoreKindData = { ...base[namespace] }; + Object.entries(contents[namespace] ?? {}).forEach(([key, entry]) => { + items[key] = entry.item; + }); + view[namespace] = items; + }); + return view; +} + +function trackerFor(view: MergedView): DependencyTracker { + const tracker = new DependencyTracker(); + Object.entries(view).forEach(([namespace, items]) => { + Object.entries(items).forEach(([key, item]) => { + tracker.updateDependenciesFrom(namespace, key, computeDependencies(namespace, item)); + }); + }); + return tracker; +} + +/** + * Applies the snapshots that an override source supplies to the override layer, and notifies + * flag change listeners of the flags each snapshot affects. + * + * @internal + */ +export default class OverrideSink implements LDOverrideSink { + // Notifications for successive snapshots are computed one at a time, in order. + private _chain: Promise = Promise.resolve(); + + /** + * @param _layer The layer to write to. + * @param _base The store that holds LaunchDarkly data, without the overlay. The merged views + * for change computation are built from it plus the layer. + * @param _onChange Receives the key of each flag whose evaluation may have changed. + * @param _hasEventListeners Reports whether anything listens for flag changes. + * @param _logger + */ + constructor( + private readonly _layer: OverrideLayer, + private readonly _base: ReadStore, + private readonly _onChange: (key: string) => void, + private readonly _hasEventListeners: () => boolean, + private readonly _logger?: LDLogger, + ) {} + + /** + * Replaces the entire override layer in one assignment, then notifies listeners of every flag + * whose merged-view evaluation may have changed. The replacement is visible to evaluations + * when this method returns. The notifications follow, because they can need asynchronous + * reads of the base store. + */ + setOverrides(flags: LDKeyedFeatureStoreItem[], segments: LDKeyedFeatureStoreItem[]): void { + const { previous, current } = this._layer.setAll(flags, segments); + // Computing the affected flags needs the merged view before and after the replacement. Skip + // all of that work when nothing is listening. + if (!this._hasEventListeners()) { + return; + } + this._chain = this._chain + .then(() => this._notifyAffected(previous, current)) + .catch((err) => { + this._logger?.error(`Unable to compute the flags affected by an override change: ${err}`); + }); + } + + /** + * Notifies the flags whose override entries were added, removed, or changed, plus every flag + * that depends, directly or transitively, on any added, removed, or changed entry of either + * kind. Dependency edges are computed over the new merged view. A flag whose edges differed in + * the old view has a different definition in the new view, so it is already one of the changed + * entries. + */ + private async _notifyAffected(previous: LayerContents, current: LayerContents): Promise { + const { seeds, count } = diffContents(previous, current); + if (count === 0) { + return; + } + const base = await this._readBase(); + const tracker = trackerFor(mergedView(base, current)); + const affected = new NamespacedDataSet(); + seeds.enumerate((namespace, key) => { + tracker.updateModifiedItems(affected, namespace, key); + }); + const flagKeys: string[] = []; + affected.enumerate((namespace, key) => { + if (namespace === VersionedDataKinds.Features.namespace) { + flagKeys.push(key); + } + }); + if (flagKeys.length > 0) { + this._logger?.debug(`Override update affected ${flagKeys.length} flag(s)`); + } + flagKeys.forEach((key) => this._onChange(key)); + } + + private _readBase(): Promise { + return new Promise((resolve) => { + this._base.all(VersionedDataKinds.Features, (flags) => { + this._base.all(VersionedDataKinds.Segments, (segments) => { + resolve({ + [VersionedDataKinds.Features.namespace]: flags, + [VersionedDataKinds.Segments.namespace]: segments, + }); + }); + }); + }); + } +} diff --git a/packages/shared/sdk-server/src/overrides/ReadStoreOverlay.ts b/packages/shared/sdk-server/src/overrides/ReadStoreOverlay.ts new file mode 100644 index 0000000000..f6d3723e11 --- /dev/null +++ b/packages/shared/sdk-server/src/overrides/ReadStoreOverlay.ts @@ -0,0 +1,60 @@ +import { DataKind } from '../api/interfaces'; +import { LDFeatureStore, LDFeatureStoreItem, LDFeatureStoreKindData } from '../api/subsystems'; +import OverrideLayer from './OverrideLayer'; + +/** + * The store read boundary: the operations through which the client reads definitions. + * + * @internal + */ +export type ReadStore = Pick; + +/** + * Merges an override layer over a base store. A read for a key returns the override entry when + * one exists, and the base entry otherwise. The overlay sits at the store read boundary. That + * placement makes targeting rules, prerequisites, and segment matches behave identically for + * overridden and ordinary data: they are the same reads through the same boundary. + * + * @internal + */ +export default class ReadStoreOverlay implements ReadStore { + constructor( + private readonly _base: ReadStore, + private readonly _layer: OverrideLayer, + ) {} + + /** + * Returns the override entry for the key when one exists, and otherwise delegates to the base + * store. This works when the base store is not initialized, because an uninitialized base + * reports not found. + */ + get(kind: DataKind, key: string, callback: (res: LDFeatureStoreItem | null) => void): void { + const item = this._layer.get(kind, key); + if (item) { + callback(item); + return; + } + this._base.get(kind, key, callback); + } + + /** + * Returns the union of the base store's items and the layer's items. The override entry wins + * for any key present in both, including a key the base holds as a deleted item. + */ + all(kind: DataKind, callback: (res: LDFeatureStoreKindData) => void): void { + if (this._layer.isEmpty()) { + this._base.all(kind, callback); + return; + } + this._base.all(kind, (baseItems) => { + callback({ ...baseItems, ...this._layer.all(kind) }); + }); + } + + /** + * Delegates to the base store. The override layer never affects initialization status. + */ + initialized(callback: (isInitialized: boolean) => void): void { + this._base.initialized(callback); + } +} diff --git a/packages/shared/sdk-server/src/overrides/createOverrideSource.ts b/packages/shared/sdk-server/src/overrides/createOverrideSource.ts new file mode 100644 index 0000000000..e43d99e7fe --- /dev/null +++ b/packages/shared/sdk-server/src/overrides/createOverrideSource.ts @@ -0,0 +1,31 @@ +import { LDClientContext, TypeValidators } from '@launchdarkly/js-sdk-common'; + +import { LDOverrideSourceOptions } from '../api/options/LDDataSystemOptions'; +import { LDOverrideSource } from '../api/subsystems'; + +function isOverrideSource(u: unknown): u is LDOverrideSource { + const candidate = u as Partial | undefined; + return ( + TypeValidators.Function.is(candidate?.start) && TypeValidators.Function.is(candidate?.close) + ); +} + +/** + * 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 + * is reported: by throwing from client construction. + * + * @internal + */ +export default function createOverrideSource( + options: LDOverrideSourceOptions, + clientContext: LDClientContext, +): LDOverrideSource { + if (TypeValidators.Function.is(options)) { + return (options as (clientContext: LDClientContext) => LDOverrideSource)(clientContext); + } + if (isOverrideSource(options)) { + return options; + } + throw new Error('Unsupported override source configuration'); +} diff --git a/packages/shared/sdk-server/src/overrides/index.ts b/packages/shared/sdk-server/src/overrides/index.ts new file mode 100644 index 0000000000..23d3ba565d --- /dev/null +++ b/packages/shared/sdk-server/src/overrides/index.ts @@ -0,0 +1,15 @@ +import createOverrideSource from './createOverrideSource'; +import OverrideLayer, { LayerContents, LayerEntry, LayerKindContents } from './OverrideLayer'; +import OverrideSink from './OverrideSink'; +import ReadStoreOverlay, { ReadStore } from './ReadStoreOverlay'; + +export { + createOverrideSource, + LayerContents, + LayerEntry, + LayerKindContents, + OverrideLayer, + OverrideSink, + ReadStore, + ReadStoreOverlay, +}; From abf256105889599e1111968ce9bf59ff4bdfdda2 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:13:28 +0000 Subject: [PATCH 2/9] test: Model value-only overrides as flags served by fallthrough --- .../LDClientImpl.overrideVectors.test.ts | 2 +- .../__tests__/LDClientImpl.overrides.test.ts | 15 +++++++++------ .../overrides/override-vectors/vectors.json | 4 ++-- .../__tests__/overrides/overridesTestSupport.ts | 3 +-- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.overrideVectors.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.overrideVectors.test.ts index 4a856fef1d..7cefd381ec 100644 --- a/packages/shared/sdk-server/__tests__/LDClientImpl.overrideVectors.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.overrideVectors.test.ts @@ -57,7 +57,7 @@ describe.each( )('%s', (_name, vector) => { it('evaluates as the vector expects', async () => { // The override layer is the same document shape the file-based source accepts. A flag value - // entry expands into a full flag that is off and serves the value. + // entry expands into a full flag that is on and serves the value by fallthrough. const source = new TestOverrideSource({ flags: [ ...Object.values(vector.overrides.flags ?? {}), diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts index feed292e16..be0ee12801 100644 --- a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts @@ -49,7 +49,7 @@ describe('given an uninitialized client with an override source', () => { expect(detail.value).toBe(true); expect(detail.variationIndex).toEqual(0); - expect(detail.reason).toEqual({ kind: 'OFF', overrideAffected: true }); + expect(detail.reason).toEqual({ kind: 'FALLTHROUGH', overrideAffected: true }); expect(client.initialized()).toBe(false); }); @@ -90,7 +90,10 @@ describe('given an uninitialized client with an override source', () => { expect(state.valid).toBe(true); expect(state.allValues()).toEqual({ 'overridden-flag': true }); - expect(state.getFlagReason('overridden-flag')).toEqual({ kind: 'OFF', overrideAffected: true }); + expect(state.getFlagReason('overridden-flag')).toEqual({ + kind: 'FALLTHROUGH', + overrideAffected: true, + }); await client.allFlagsState(user); expect(warningsMatching(logger, /returning only flags from the override layer/)).toEqual(1); @@ -177,7 +180,7 @@ describe('given an initialized client with LaunchDarkly data and an override sou const detail = await client.variationDetail('flag-normal', user, 'default'); expect(detail.value).toEqual('normal-value'); - expect(detail.reason).toEqual({ kind: 'OFF' }); + expect(detail.reason).toEqual({ kind: 'FALLTHROUGH' }); }); it('marks a flag whose prerequisite is overridden', async () => { @@ -215,7 +218,7 @@ describe('given an initialized client with LaunchDarkly data and an override sou const detail = await client.variationDetail('overridden-flag', user, 'default'); expect(detail.value).toEqual('ld-value'); - expect(detail.reason).toEqual({ kind: 'OFF' }); + expect(detail.reason).toEqual({ kind: 'FALLTHROUGH' }); }); }); @@ -306,7 +309,7 @@ describe('given an override source with an asynchronous initial load', () => { const detail = await pending; expect(detail.value).toBe(true); - expect(detail.reason).toEqual({ kind: 'OFF', overrideAffected: true }); + expect(detail.reason).toEqual({ kind: 'FALLTHROUGH', overrideAffected: true }); }); it('waits for the initial load before reporting the all flags state', async () => { @@ -423,6 +426,6 @@ describe('given override source lifecycle and configuration', () => { const detail = await client.variationDetail('flag', user, 'default'); expect(detail.value).toEqual('x'); - expect(detail.reason).toEqual({ kind: 'OFF' }); + expect(detail.reason).toEqual({ kind: 'FALLTHROUGH' }); }); }); diff --git a/packages/shared/sdk-server/__tests__/overrides/override-vectors/vectors.json b/packages/shared/sdk-server/__tests__/overrides/override-vectors/vectors.json index 0910eefb87..38eab6c317 100644 --- a/packages/shared/sdk-server/__tests__/overrides/override-vectors/vectors.json +++ b/packages/shared/sdk-server/__tests__/overrides/override-vectors/vectors.json @@ -30,7 +30,7 @@ "expect": { "value": "override-value", "variationIndex": 0, - "reason": { "kind": "OFF", "overrideAffected": true }, + "reason": { "kind": "FALLTHROUGH", "overrideAffected": true }, "summaryOverrideAffected": true } }, @@ -53,7 +53,7 @@ "expect": { "value": "override-value", "variationIndex": 0, - "reason": { "kind": "OFF", "overrideAffected": true }, + "reason": { "kind": "FALLTHROUGH", "overrideAffected": true }, "summaryOverrideAffected": true } }, diff --git a/packages/shared/sdk-server/__tests__/overrides/overridesTestSupport.ts b/packages/shared/sdk-server/__tests__/overrides/overridesTestSupport.ts index 34e5b3e227..b7e2208c35 100644 --- a/packages/shared/sdk-server/__tests__/overrides/overridesTestSupport.ts +++ b/packages/shared/sdk-server/__tests__/overrides/overridesTestSupport.ts @@ -97,8 +97,7 @@ export function singleValueFlag(key: string, value: any, version: number = 1): a return { key, version, - on: false, - offVariation: 0, + on: true, fallthrough: { variation: 0 }, variations: [value], }; From 37439809d0b12acb741878f471f352c2021ff58d Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:24:15 +0000 Subject: [PATCH 3/9] fix: Keep the client shutdown and the all flags state correct around the override layer A throwing override source close is logged and the rest of the shutdown continues. Before initialization, the all flags state uses the override layer only when it holds at least one flag. --- .../__tests__/LDClientImpl.overrides.test.ts | 36 +++++++++++++++ .../shared/sdk-server/src/LDClientImpl.ts | 44 +++++++++++-------- 2 files changed, 62 insertions(+), 18 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts index be0ee12801..08bbd3d7c7 100644 --- a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts @@ -2,6 +2,7 @@ import { LDClientContext, LDLogger } from '@launchdarkly/js-sdk-common'; import { LDOverrideSink } from '../src/api/subsystems'; import LDClientImpl from '../src/LDClientImpl'; +import InMemoryFeatureStore from '../src/store/InMemoryFeatureStore'; import { fdv2FullPayload, makeCallbacks, @@ -99,6 +100,19 @@ describe('given an uninitialized client with an override source', () => { expect(warningsMatching(logger, /returning only flags from the override layer/)).toEqual(1); }); + it('reports an invalid empty all flags state when the override layer holds only segments', async () => { + source.setOverrides([], [{ key: 'segment1', version: 1 }]); + + const state = await client.allFlagsState(user); + + // A layer without flags gives the state nothing to report, so the state is the one reported + // when no data is available. + expect(state.valid).toBe(false); + expect(state.allValues()).toEqual({}); + expect(warningsMatching(logger, /Data store not available/)).toEqual(1); + expect(warningsMatching(logger, /returning only flags from the override layer/)).toEqual(0); + }); + it('reports an invalid empty all flags state when the override layer is empty', async () => { source.setOverrides([]); @@ -348,6 +362,28 @@ describe('given override source lifecycle and configuration', () => { expect(source.closed).toBe(true); }); + it('logs and continues the shutdown when the source fails to close', () => { + const logger = makeLogger(); + const throwing = { + start: () => {}, + close: () => { + throw new Error('close failed'); + }, + }; + const store = new InMemoryFeatureStore(); + const storeClose = jest.spyOn(store, 'close'); + const created = makeFDv2Client(makeFDv2Platform(), { + logger, + dataSystem: { overrides: throwing, persistentStore: store }, + }); + client = created; + + // The failure is logged, and the components that close after the source still close. + expect(() => created.close()).not.toThrow(); + expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('close failed')); + expect(storeClose).toHaveBeenCalledTimes(1); + }); + it('does not start the source in offline mode', async () => { const source = new TestOverrideSource({ flags: [singleValueFlag('overridden-flag', true)] }); client = makeFDv2Client(makeFDv2Platform(), { diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 5e3f1335e2..4fafdb5461 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -1290,24 +1290,27 @@ export default class LDClientImpl implements LDClient { doEval(true); return; } - if (this._overrideLayer && !this._overrideLayer.isEmpty()) { - // No LaunchDarkly data is available. The state holds only the flags that the - // override layer holds. - if (!this._allFlagsStateOverridesOnlyWarningLogged) { - this._allFlagsStateOverridesOnlyWarningLogged = true; - this._logger?.warn( - 'Called allFlagsState before client initialization; returning only flags from' + - ' the override layer. This message is logged once.', - ); + // No LaunchDarkly data is available. The read returns only the flags that the override + // layer holds, and the result decides the state: a layer without flags gives the state + // nothing to report. + this._readStore.all(VersionedDataKinds.Features, (allFlags) => { + if (this._overrideLayer && Object.keys(allFlags).length > 0) { + if (!this._allFlagsStateOverridesOnlyWarningLogged) { + this._allFlagsStateOverridesOnlyWarningLogged = true; + this._logger?.warn( + 'Called allFlagsState before client initialization; returning only flags from' + + ' the override layer. This message is logged once.', + ); + } + evaluateAll(true, allFlags); + return; } - doEval(true); - return; - } - this._logger?.warn( - 'Called allFlagsState before client initialization. Data store not available; ' + - 'returning empty state', - ); - doEval(false); + this._logger?.warn( + 'Called allFlagsState before client initialization. Data store not available; ' + + 'returning empty state', + ); + evaluateAll(false, allFlags); + }); }); }); }); @@ -1330,7 +1333,12 @@ export default class LDClientImpl implements LDClient { } close(): void { - this._overrideSource?.close(); + try { + this._overrideSource?.close(); + } catch (err) { + // A source that fails to close does not stop the rest of the shutdown. + this._logger?.error(`Unable to close the override source: ${err}`); + } this._eventProcessor.close(); this._updateProcessor?.close(); this._dataSource?.stop(); From bf64955817559e4c29d4cf790dbd7dcbe75327f0 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Sat, 3 Oct 2026 01:44:52 +0000 Subject: [PATCH 4/9] fix: Treat a null override source option as no override source --- .../__tests__/options/Configuration.test.ts | 26 +++++++++++++++++++ .../sdk-server/src/options/Configuration.ts | 12 ++++++--- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts index 57c05d6c2e..39d7704bd7 100644 --- a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts +++ b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts @@ -5,6 +5,7 @@ import { LDOptions, LDTransactionalFeatureStore, } from '../../src'; +import { LDOverrideSourceOptions } from '../../src/api/options/LDDataSystemOptions'; import Configuration from '../../src/options/Configuration'; import InMemoryFeatureStore from '../../src/store/InMemoryFeatureStore'; import TestLogger, { LogLevel } from '../Logger'; @@ -436,6 +437,31 @@ describe('when setting different options', () => { ]); }); + it('treats a null override source option as no override source', () => { + const config = new Configuration( + withLogger({ + dataSystem: { overrides: null as unknown as LDOverrideSourceOptions }, + }), + ); + expect(config.dataSystem!.overrides).toBeUndefined(); + expect(logger(config).getCount()).toEqual(0); + }); + + it('drops an override source option of the wrong type and warns', () => { + const config = new Configuration( + withLogger({ + dataSystem: { overrides: 'bogus type' as unknown as LDOverrideSourceOptions }, + }), + ); + expect(config.dataSystem!.overrides).toBeUndefined(); + logger(config).expectMessages([ + { + level: LogLevel.Warn, + matches: /Config option "dataSystem.overrides" should be of type LDOverrideSourceOptions/, + }, + ]); + }); + it('validates the datasystem persistent store is a factory or object', () => { const config1 = new Configuration( withLogger({ diff --git a/packages/shared/sdk-server/src/options/Configuration.ts b/packages/shared/sdk-server/src/options/Configuration.ts index ba2bd2ad7d..d35baa2228 100644 --- a/packages/shared/sdk-server/src/options/Configuration.ts +++ b/packages/shared/sdk-server/src/options/Configuration.ts @@ -313,9 +313,15 @@ function validateDataSystemOptions(options: Options): { } } - // The override source is either a source object or a factory function. Anything else is a - // misconfiguration. Drop it so the client runs without overrides, and warn. - if (options.overrides !== undefined && !TypeValidators.ObjectOrFactory.is(options.overrides)) { + // The override source is either a source object or a factory function. A null value means no + // override source, the same as leaving the option out. Anything else is a misconfiguration. + // Drop it so the client runs without overrides, and warn. + if (options.overrides === null) { + validatedOptions.overrides = undefined; + } else if ( + options.overrides !== undefined && + !TypeValidators.ObjectOrFactory.is(options.overrides) + ) { validatedOptions.overrides = undefined; allErrors.push( OptionMessages.wrongOptionType( From dd27afb6b4e9807851d2a4ab59cac479f81efcf0 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:44:53 +0000 Subject: [PATCH 5/9] fix: Never make an evaluation wait for the override source's initial load Every evaluation path, and the all-flags state, was chained onto the promise of the override source's start so that an override present at startup would apply from the first evaluation. Evaluation must not wait on I/O: a source whose start never settles, such as a read of a hung network file system or a custom source waiting on an unreachable endpoint, held every flag evaluation and every caller's promise forever, and close could not release them. The wait is removed. The initial load is part of starting the client, so the promise that waitForInitialization returns now resolves after the load as well, bounded by the caller's timeout, which is the asynchronous start operation the specification describes. An evaluation made before the load completes reads the layer as it is at that moment. A migration variation whose value is not a stage replaces the reason; the replacement now keeps the override-affected marking, as the typed variation methods do. Negative controls: the migration test fails when the marking is dropped from the replacement reason; the two waitForInitialization tests fail when the load is left out of the start handle; the evaluation tests fail against the previous gate. --- .../__tests__/LDClientImpl.overrides.test.ts | 133 ++++++++++-- .../shared/sdk-server/src/LDClientImpl.ts | 190 ++++++++++-------- .../src/api/subsystems/LDOverrideSource.ts | 8 +- 3 files changed, 224 insertions(+), 107 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts index 08bbd3d7c7..bf47edbbd6 100644 --- a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts @@ -1,8 +1,10 @@ import { LDClientContext, LDLogger } from '@launchdarkly/js-sdk-common'; +import { LDMigrationStage } from '../src/api/data/LDMigrationStage'; import { LDOverrideSink } from '../src/api/subsystems'; import LDClientImpl from '../src/LDClientImpl'; import InMemoryFeatureStore from '../src/store/InMemoryFeatureStore'; +import { TestHook } from './hooks/TestHook'; import { fdv2FullPayload, makeCallbacks, @@ -86,6 +88,35 @@ describe('given an uninitialized client with an override source', () => { }); }); + it('keeps a migration stage mismatch of an overridden flag marked', async () => { + // The migration variation replaces the reason when the value is not a stage. The replacement + // keeps the override-affected marking, like the typed variation methods do. + client.close(); + const hook = new TestHook(); + client = makeFDv2Client(makeFDv2Platform(), { + logger, + hooks: [hook], + dataSystem: { overrides: source }, + }); + source.setOverrides([singleValueFlag('overridden-flag', 'not-a-stage')]); + + const migration = await client.migrationVariation( + 'overridden-flag', + user, + LDMigrationStage.Off, + ); + + expect(migration.value).toBe(LDMigrationStage.Off); + const after = hook.captureAfter.find( + (capture) => capture.hookContext.method === 'LDClient.migrationVariation', + ); + expect(after?.detail?.reason).toEqual({ + kind: 'ERROR', + errorKind: 'WRONG_TYPE', + overrideAffected: true, + }); + }); + it('reports only the override layer in the all flags state and warns once', async () => { const state = await client.allFlagsState(user, { withReasons: true }); @@ -309,38 +340,108 @@ describe('given an override source with an asynchronous initial load', () => { client.close(); }); - it('waits for the initial load before evaluating', async () => { - let resolved = false; - const pending = client.boolVariationDetail('overridden-flag', user, false).then((detail) => { - resolved = true; - return detail; - }); + it('does not make an evaluation wait for the initial load', async () => { + // Evaluation never waits on the source. Before the load completes the layer is empty, so a + // flag the layer does not hold gets the not-ready default, and the call settles at once. + const detail = await client.boolVariationDetail('overridden-flag', user, false); + expect(detail.value).toBe(false); + expect(detail.reason).toEqual({ kind: 'ERROR', errorKind: 'CLIENT_NOT_READY' }); + + source.setOverrides([singleValueFlag('overridden-flag', true)]); + source.completeStart(); await settle(); - expect(resolved).toBe(false); + + const afterLoad = await client.boolVariationDetail('overridden-flag', user, false); + expect(afterLoad.value).toBe(true); + expect(afterLoad.reason).toEqual({ kind: 'FALLTHROUGH', overrideAffected: true }); + }); + + it('does not make the all flags state wait for the initial load', async () => { + const state = await client.allFlagsState(user); + expect(state.valid).toBe(false); + expect(state.allValues()).toEqual({}); source.setOverrides([singleValueFlag('overridden-flag', true)]); source.completeStart(); + await settle(); - const detail = await pending; - expect(detail.value).toBe(true); - expect(detail.reason).toEqual({ kind: 'FALLTHROUGH', overrideAffected: true }); + const afterLoad = await client.allFlagsState(user); + expect(afterLoad.valid).toBe(true); + expect(afterLoad.allValues()).toEqual({ 'overridden-flag': true }); }); - it('waits for the initial load before reporting the all flags state', async () => { + it('settles an evaluation promptly when the start never completes', async () => { + // A source whose initial load hangs, for example a hung read of a network file system or a + // custom source waiting on an unreachable endpoint, must not hold up flag evaluation. + client.close(); + const hanging = { + start: () => + new Promise(() => { + // Never settles. + }), + close: () => {}, + }; + client = makeFDv2Client(makeFDv2Platform(), { dataSystem: { overrides: hanging } }); + + const outcome = await Promise.race([ + client.boolVariation('some-flag', user, false).then(() => 'settled'), + new Promise((resolve) => { + setTimeout(() => resolve('hung'), 500); + }), + ]); + expect(outcome).toBe('settled'); + }); +}); + +describe('given an initialized client and an override source with an asynchronous initial load', () => { + let source: TestOverrideSource; + let client: LDClientImpl; + + beforeEach(() => { + source = new TestOverrideSource(undefined, true); + client = makeFDv2Client( + makeFDv2Platform(fdv2FullPayload({ 'ld-flag': singleValueFlag('ld-flag', true) })), + { + dataSystem: { overrides: source }, + }, + ); + }); + + afterEach(() => { + client.close(); + }); + + it('completes waitForInitialization only after the initial load', async () => { + // The initial load is part of starting the client, so the start handle resolves after it. let resolved = false; - const pending = client.allFlagsState(user).then((state) => { + const pending = client.waitForInitialization({ timeout: 5 }).then(() => { resolved = true; - return state; }); await settle(); + expect(client.initialized()).toBe(true); expect(resolved).toBe(false); source.setOverrides([singleValueFlag('overridden-flag', true)]); source.completeStart(); + await pending; + expect(resolved).toBe(true); + const detail = await client.boolVariationDetail('overridden-flag', user, false); + expect(detail.value).toBe(true); + }); - const state = await pending; - expect(state.valid).toBe(true); - expect(state.allValues()).toEqual({ 'overridden-flag': true }); + it('lets waitForInitialization time out when the initial load does not complete', async () => { + await expect(client.waitForInitialization({ timeout: 0.1 })).rejects.toThrow( + /waitForInitialization/, + ); + // Evaluation was never held up by the load. + const detail = await client.boolVariationDetail('ld-flag', user, false); + expect(detail.value).toBe(true); + }); + + it('completes waitForInitialization without waiting once the load has completed', async () => { + source.completeStart(); + await settle(); + await expect(client.waitForInitialization({ timeout: 1 })).resolves.toBe(client); }); }); diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 4fafdb5461..4bdcd8910b 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -9,6 +9,7 @@ import { LDContext, LDEvaluationDetail, LDEvaluationDetailTyped, + LDEvaluationReason, LDLogger, LDPluginEnvironmentMetadata, LDTimeoutError, @@ -641,8 +642,9 @@ export default class LDClientImpl implements LDClient { private _overrideSource?: LDOverrideSource; - // Settles when the override source's initial load has completed. Undefined once it has, or when - // there is no override source. + // Settles when the override source's initial load has completed, also when it failed, and is + // cleared then. waitForInitialization awaits it, so that the load is part of starting the + // client. Evaluation never waits on it: an evaluation reads the layer as it is. private _overridesReady?: Promise; private _updateProcessor?: subsystem.LDStreamProcessor; @@ -795,8 +797,8 @@ export default class LDClientImpl implements LDClient { if (overrides) { // The source starts before the data source, so an override that loads synchronously is in - // place before the client evaluates anything. The evaluation methods wait for an - // asynchronous initial load to complete. + // place before the client evaluates anything. An asynchronous initial load is part of + // starting the client: waitForInitialization resolves after it. Evaluation never waits. this._overrideLayer = overrides.layer; this._overrideSource = overrides.source; this._overridesReady = this._startOverrideSource(overrides.source, overrides.sink); @@ -859,15 +861,24 @@ export default class LDClientImpl implements LDClient { // Initialization promise was created by a previous call to waitForInitialization. if (this._initializedPromise) { // This promise may already be resolved/rejected, but it doesn't hurt to wrap it in a timeout. - return this._clientWithTimeout(this._initializedPromise, options?.timeout, this._logger); + return this._clientWithTimeout( + this._withOverridesLoaded(this._initializedPromise), + options?.timeout, + this._logger, + ); } // Initialization completed before waitForInitialization was called, so we have completed // and there was no promise. So we make a resolved promise and return it. if (this._initState === InitState.Initialized) { this._initializedPromise = Promise.resolve(this); - // Already initialized, no need to timeout. - return this._initializedPromise; + // Already initialized, so only the override source's initial load, if it is still in + // progress, is left to wait for. + return this._clientWithTimeout( + this._withOverridesLoaded(this._initializedPromise), + options?.timeout, + this._logger, + ); } // Initialization failed before waitForInitialization was called, so we have completed @@ -884,7 +895,25 @@ export default class LDClientImpl implements LDClient { this._initReject = reject; }); } - return this._clientWithTimeout(this._initializedPromise, options?.timeout, this._logger); + return this._clientWithTimeout( + this._withOverridesLoaded(this._initializedPromise), + options?.timeout, + this._logger, + ); + } + + /** + * Extends the initialization promise with the override source's initial load, when that load + * is still in progress. The initial load is part of starting the client, so the start handle + * that waitForInitialization returns resolves after it. The load never rejects this promise: + * a failed load is logged and the client runs without overrides. + */ + private _withOverridesLoaded(initialized: Promise): Promise { + const overridesReady = this._overridesReady; + if (!overridesReady) { + return initialized; + } + return Promise.all([initialized, overridesReady]).then(() => this); } variation( @@ -1133,10 +1162,11 @@ export default class LDClientImpl implements LDClient { `Unrecognized MigrationState for "${key}"; returning default value.`, ); this._onError(error); - const reason = { - kind: 'ERROR', - errorKind: ErrorKinds.WrongType, - }; + // The type mismatch replaces the reason. The evaluation read the same definitions, so + // the new reason keeps the override-affected marking. + const reason: LDEvaluationReason = detail.reason.overrideAffected + ? { kind: 'ERROR', errorKind: ErrorKinds.WrongType, overrideAffected: true } + : { kind: 'ERROR', errorKind: ErrorKinds.WrongType }; resolve({ detail: { value: defaultValue, @@ -1273,44 +1303,42 @@ export default class LDClientImpl implements LDClient { evaluateAll(valid, allFlags), ); - this._afterOverridesReady(() => { - if (this.initialized()) { + if (this.initialized()) { + doEval(true); + return; + } + this._featureStore.initialized((storeInitialized) => { + if (storeInitialized) { + if (!this._allFlagsStateLastKnownValuesWarningLogged) { + this._allFlagsStateLastKnownValuesWarningLogged = true; + this._logger?.warn( + 'Called allFlagsState before client initialization; using last known' + + ' values from data store. This message is logged once.', + ); + } doEval(true); return; } - this._featureStore.initialized((storeInitialized) => { - if (storeInitialized) { - if (!this._allFlagsStateLastKnownValuesWarningLogged) { - this._allFlagsStateLastKnownValuesWarningLogged = true; + // No LaunchDarkly data is available. The read returns only the flags that the override + // layer holds, and the result decides the state: a layer without flags gives the state + // nothing to report. + this._readStore.all(VersionedDataKinds.Features, (allFlags) => { + if (this._overrideLayer && Object.keys(allFlags).length > 0) { + if (!this._allFlagsStateOverridesOnlyWarningLogged) { + this._allFlagsStateOverridesOnlyWarningLogged = true; this._logger?.warn( - 'Called allFlagsState before client initialization; using last known' + - ' values from data store. This message is logged once.', + 'Called allFlagsState before client initialization; returning only flags from' + + ' the override layer. This message is logged once.', ); } - doEval(true); + evaluateAll(true, allFlags); return; } - // No LaunchDarkly data is available. The read returns only the flags that the override - // layer holds, and the result decides the state: a layer without flags gives the state - // nothing to report. - this._readStore.all(VersionedDataKinds.Features, (allFlags) => { - if (this._overrideLayer && Object.keys(allFlags).length > 0) { - if (!this._allFlagsStateOverridesOnlyWarningLogged) { - this._allFlagsStateOverridesOnlyWarningLogged = true; - this._logger?.warn( - 'Called allFlagsState before client initialization; returning only flags from' + - ' the override layer. This message is logged once.', - ); - } - evaluateAll(true, allFlags); - return; - } - this._logger?.warn( - 'Called allFlagsState before client initialization. Data store not available; ' + - 'returning empty state', - ); - evaluateAll(false, allFlags); - }); + this._logger?.warn( + 'Called allFlagsState before client initialization. Data store not available; ' + + 'returning empty state', + ); + evaluateAll(false, allFlags); }); }); }); @@ -1499,44 +1527,43 @@ export default class LDClientImpl implements LDClient { cb: (res: EvalResult, flag?: Flag) => void, typeChecker?: (value: any) => [boolean, string], ): void { - this._afterOverridesReady(() => { - if (!this.initialized()) { - this._featureStore.initialized((storeInitialized) => { - if (storeInitialized) { - if (!this._lastKnownValuesWarningLogged) { - this._lastKnownValuesWarningLogged = true; - this._logger?.warn( - 'Variation called before LaunchDarkly client initialization completed' + - " (did you wait for the 'ready' event?) - using last known values from feature store." + - ' This message is logged once.', - ); - } - this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); - return; - } - if (this._overrideLayer?.get(VersionedDataKinds.Features, flagKey)) { - // No LaunchDarkly data is available, but the override layer holds this flag. The - // override is served before the not-initialized short-circuit. A flag the layer does - // not hold still gets the not-ready default. - this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); - return; + if (!this.initialized()) { + this._featureStore.initialized((storeInitialized) => { + if (storeInitialized) { + if (!this._lastKnownValuesWarningLogged) { + this._lastKnownValuesWarningLogged = true; + this._logger?.warn( + 'Variation called before LaunchDarkly client initialization completed' + + " (did you wait for the 'ready' event?) - using last known values from feature store." + + ' This message is logged once.', + ); } - this._logger?.warn( - 'Variation called before LaunchDarkly client initialization completed (did you wait for the' + - "'ready' event?) - using default value", - ); - cb(EvalResult.forError(ErrorKinds.ClientNotReady, undefined, defaultValue)); - }); - return; - } - this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); - }); + this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); + return; + } + if (this._overrideLayer?.get(VersionedDataKinds.Features, flagKey)) { + // No LaunchDarkly data is available, but the override layer holds this flag. The + // override is served before the not-initialized short-circuit. A flag the layer does + // not hold still gets the not-ready default. + this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); + return; + } + this._logger?.warn( + 'Variation called before LaunchDarkly client initialization completed (did you wait for the' + + "'ready' event?) - using default value", + ); + cb(EvalResult.forError(ErrorKinds.ClientNotReady, undefined, defaultValue)); + }); + return; + } + this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); } /** * Starts the override source. The returned promise settles when the source's initial load has - * completed, also when it failed. A failure never prevents the client from operating: the - * layer stays as it was and the failure is logged. + * completed, also when it failed, and never rejects. A failure never prevents the client from + * operating: the layer stays as it was and the failure is logged. The load is part of starting + * the client, so waitForInitialization awaits this promise; evaluation does not. */ private _startOverrideSource(source: LDOverrideSource, sink: OverrideSink): Promise { let started: Promise; @@ -1554,19 +1581,6 @@ export default class LDClientImpl implements LDClient { }); } - /** - * Runs the action once the override source's initial load has completed, so an override that - * is present when the client is created takes effect from the first evaluation. Without an - * override source, or once the load has completed, the action runs immediately. - */ - private _afterOverridesReady(action: () => void): void { - if (this._overridesReady) { - this._overridesReady.then(action); - return; - } - action(); - } - private _dataSourceErrorHandler(e: any) { const error = e.code === 401 ? new Error('Authentication failed. Double check your SDK key.') : e; diff --git a/packages/shared/sdk-server/src/api/subsystems/LDOverrideSource.ts b/packages/shared/sdk-server/src/api/subsystems/LDOverrideSource.ts index 6d30a198ff..f42c973d8d 100644 --- a/packages/shared/sdk-server/src/api/subsystems/LDOverrideSource.ts +++ b/packages/shared/sdk-server/src/api/subsystems/LDOverrideSource.ts @@ -48,9 +48,11 @@ export interface LDOverrideSource { * calling the sink. * * The SDK calls this method once, when the client is created. When the initial load is - * asynchronous, return a promise that settles when the load completes. The SDK waits for it - * before it evaluates flags, so an override that is present at startup takes effect from the - * first evaluation. + * asynchronous, return a promise that settles when the load completes. The load is part of + * starting the client: the promise that `waitForInitialization` returns resolves after it, so + * an override that is present at startup takes effect by the time initialization is awaited. + * Evaluation never waits for the load; an evaluation made before it completes reads the layer + * as it is at that moment. * * @param sink The sink that receives the snapshots. */ From dc6c2f8e64079ce961726ee950009bd35bfbc3b3 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 9 Oct 2026 20:57:47 +0000 Subject: [PATCH 6/9] fix: Reject an override definition that does not have the shape evaluation requires The SDK stores flag and segment definitions in their JSON form and has no typed deserialization, so a hand-written override definition could reach the evaluator with a field of the wrong type: a flag without variations, a target or clause without values, rules that are not an array. The evaluator then threw instead of returning a malformed-flag result, and the all flags state could never resolve. SDKs with typed deserialization reject such a definition when they read it. The override layer now checks each definition before it prepares and stores it. The checks cover the fields that evaluation and the preparation read with an assumed type: lists are arrays, list entries are objects, and attribute names that become attribute references are strings. Only a flag's variations and the values of a target or clause are required, because those are the fields evaluation reads without a guard. Every other field may be absent or null, as it may be in LaunchDarkly data. A definition that fails the check throws an Error that names the kind, the key, and the field, and the layer keeps its previous contents, so a snapshot with one bad entry applies nothing. Negative controls: each rejection test fails when its check is removed or disabled; the acceptance tests fail when the optional lists are made required, when null stops counting as absent, or when the string check is inverted; the snapshot and client tests fail when the layer builds the new snapshot in its live contents. --- .../__tests__/LDClientImpl.overrides.test.ts | 19 ++ .../__tests__/overrides/OverrideLayer.test.ts | 277 ++++++++++++++++++ .../src/api/subsystems/LDOverrideSource.ts | 5 +- .../sdk-server/src/overrides/OverrideLayer.ts | 20 +- .../sdk-server/src/overrides/OverrideSink.ts | 3 +- .../src/overrides/validateDefinition.ts | 186 ++++++++++++ 6 files changed, 504 insertions(+), 6 deletions(-) create mode 100644 packages/shared/sdk-server/src/overrides/validateDefinition.ts diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts index bf47edbbd6..9670616ed7 100644 --- a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts @@ -158,6 +158,25 @@ describe('given an uninitialized client with an override source', () => { await expect(client.waitForInitialization({ timeout: 0.05 })).rejects.toThrow(/timed out/); expect(client.initialized()).toBe(false); }); + + it('keeps serving the previous overrides when a snapshot has a malformed definition', async () => { + // One definition of the wrong shape fails the whole snapshot. The source's call throws with + // the reason, and the layer keeps the earlier snapshot, so the flag it held keeps its earlier + // value and the valid new entry of the rejected snapshot is not served. + const withoutVariations = { + key: 'other-flag', + version: 1, + on: true, + fallthrough: { variation: 0 }, + }; + expect(() => + source.setOverrides([singleValueFlag('overridden-flag', false), withoutVariations]), + ).toThrow('flag "other-flag": "variations" must be an array'); + + expect(await client.boolVariation('overridden-flag', user, false)).toBe(true); + const detail = await client.boolVariationDetail('other-flag', user, false); + expect(detail.reason).toEqual({ kind: 'ERROR', errorKind: 'CLIENT_NOT_READY' }); + }); }); describe('given an initialized client with LaunchDarkly data and an override source', () => { diff --git a/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts b/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts index 11c6413eda..499d20942b 100644 --- a/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts @@ -1,8 +1,10 @@ /* eslint-disable no-underscore-dangle */ import { AttributeReference } from '@launchdarkly/js-sdk-common'; +import { makeFlagWithValue } from '../../src/data_sources/FileDataSource'; import { Flag } from '../../src/evaluation/data/Flag'; import { Segment } from '../../src/evaluation/data/Segment'; +import TestData from '../../src/integrations/test_data/TestData'; import { OverrideLayer } from '../../src/overrides'; import VersionedDataKinds from '../../src/store/VersionedDataKinds'; @@ -122,3 +124,278 @@ it('returns the previous and the new contents with the supplied text of each ent expect(second.current.features.flag1.json).toEqual(JSON.stringify(rawFlag('flag1', 2))); expect(second.current.segments.segment1.item.key).toEqual('segment1'); }); + +describe('given definitions with the shape evaluation requires', () => { + it('accepts full flag and segment definitions', () => { + // Every list the data model has is present and filled, so each check sees a well-formed field. + const flag = { + ...rawFlag('flag1'), + prerequisites: [{ key: 'other', variation: 0 }], + contextTargets: [{ contextKind: 'org', variation: 1, values: ['org-a'] }], + fallthrough: { + rollout: { + kind: 'experiment', + bucketBy: 'name', + contextKind: 'user', + seed: 1, + variations: [ + { variation: 0, weight: 50000 }, + { variation: 1, weight: 50000 }, + ], + }, + }, + rules: [ + { + id: 'rollout-rule', + rollout: { bucketBy: 'name', variations: [{ variation: 1, weight: 100000 }] }, + clauses: [{ attribute: 'name', op: 'in', values: ['x'] }], + }, + { + id: 'segment-rule', + variation: 0, + clauses: [{ attribute: '', op: 'segmentMatch', values: ['segment1'] }], + }, + ], + }; + const segment = { + ...rawSegment('segment1'), + includedContexts: [{ contextKind: 'org', values: ['org-a'] }], + excludedContexts: [{ contextKind: 'org', values: ['org-z'] }], + rules: [ + { + id: 'rule', + weight: 50000, + bucketBy: 'name', + rolloutContextKind: 'user', + clauses: [{ attribute: 'name', op: 'in', values: ['x'], contextKind: 'user' }], + }, + ], + }; + const layer = new OverrideLayer(); + + layer.setAll([flag], [segment]); + + expect(layer.get(VersionedDataKinds.Features, 'flag1')).toBeDefined(); + expect(layer.get(VersionedDataKinds.Segments, 'segment1')).toBeDefined(); + }); + + it('accepts absent optional lists, null fields, and rules without clauses', () => { + // Only a flag's variations and the values of a target or clause are required. Every other + // field may be absent, as it may be in LaunchDarkly data, and the preparation removes null + // fields, so null counts as absent. A rule without clauses evaluates as no match. + const flag = { + key: 'flag1', + version: 1, + on: true, + variations: [false, true], + targets: null, + contextTargets: null, + prerequisites: null, + fallthrough: null, + rules: [ + { id: 'no-clauses', variation: 1 }, + { id: 'null-clauses', clauses: null }, + ], + }; + const segment = { + key: 'segment1', + version: 1, + included: null, + includedContexts: null, + rules: [{ id: 'no-clauses' }], + }; + const layer = new OverrideLayer(); + + layer.setAll([flag], [segment]); + + expect(layer.get(VersionedDataKinds.Features, 'flag1')).toBeDefined(); + expect(layer.get(VersionedDataKinds.Segments, 'segment1')).toBeDefined(); + }); + + it('accepts the flag that the file data source expands from a value', () => { + const layer = new OverrideLayer(); + + layer.setAll([makeFlagWithValue('flag1', 'value', 1)], []); + + expect(layer.get(VersionedDataKinds.Features, 'flag1')).toBeDefined(); + }); + + it('accepts the flags that the TestData builders build', () => { + const flag = new TestData() + .flag('flag1') + .variationForContext('org', 'org-a', true) + .variationForContext('user', 'user-a', false) + .ifMatch('user', 'name', 'x') + .thenReturn(true) + .build(1); + const layer = new OverrideLayer(); + + layer.setAll([flag], []); + + expect(layer.get(VersionedDataKinds.Features, 'flag1')).toBeDefined(); + }); +}); + +describe('given a definition that does not have the shape evaluation requires', () => { + // Evaluation reads these fields with the types the data model declares and throws when a + // hand-written definition has another type, so the layer rejects the definition first. The + // error names the kind, the key, and the field, so an operator can find the mistake. + function flagWith(changes: Record): any { + return { ...rawFlag('flag1'), ...changes }; + } + + function flagWithRule(rule: Record): any { + return flagWith({ rules: [{ id: 'rule', variation: 1, ...rule }] }); + } + + function segmentWith(changes: Record): any { + return { ...rawSegment('segment1'), ...changes }; + } + + function segmentWithRule(rule: Record): any { + return segmentWith({ rules: [{ id: 'rule', ...rule }] }); + } + + const malformedFlags: [string, any, string][] = [ + [ + 'a flag without variations', + flagWith({ variations: undefined }), + '"variations" must be an array', + ], + [ + 'variations that are not an array', + flagWith({ variations: 'true' }), + '"variations" must be an array', + ], + ['rules that are not an array', flagWith({ rules: {} }), '"rules" must be an array'], + ['a rule that is not an object', flagWith({ rules: [null] }), 'rule 0 must be an object'], + [ + 'clauses that are not an array', + flagWithRule({ clauses: 'name in x' }), + 'rule 0: "clauses" must be an array', + ], + [ + 'a clause without values', + flagWithRule({ clauses: [{ attribute: 'name', op: 'in' }] }), + 'rule 0, clause 0: "values" must be an array', + ], + [ + 'a clause whose attribute is not a string', + flagWithRule({ clauses: [{ attribute: 5, op: 'in', values: ['x'] }] }), + 'rule 0, clause 0: "attribute" must be a string', + ], + [ + 'a target without values', + flagWith({ targets: [{ variation: 1 }] }), + 'target 0: "values" must be an array', + ], + [ + 'a context target without values', + flagWith({ contextTargets: [{ contextKind: 'org', variation: 1 }] }), + 'context target 0: "values" must be an array', + ], + [ + 'prerequisites that are not an array', + flagWith({ prerequisites: { key: 'other', variation: 0 } }), + '"prerequisites" must be an array', + ], + [ + 'a prerequisite that is not an object', + flagWith({ prerequisites: ['other'] }), + 'prerequisite 0 must be an object', + ], + [ + 'a fallthrough that is not an object', + flagWith({ fallthrough: 0 }), + '"fallthrough" must be an object', + ], + [ + 'a fallthrough rollout whose variations are not an array', + flagWith({ fallthrough: { rollout: { variations: { variation: 0, weight: 100000 } } } }), + 'fallthrough, rollout: "variations" must be an array', + ], + [ + 'a fallthrough rollout whose bucketBy is not a string', + flagWith({ + fallthrough: { rollout: { bucketBy: 5, variations: [{ variation: 0, weight: 100000 }] } }, + }), + 'fallthrough, rollout: "bucketBy" must be a string', + ], + [ + 'a rule rollout with a weighted variation that is not an object', + flagWithRule({ variation: undefined, rollout: { variations: [null] } }), + 'rule 0, rollout: variation 0 must be an object', + ], + ]; + + it.each(malformedFlags)('rejects %s', (_description, flag, message) => { + const layer = new OverrideLayer(); + + expect(() => layer.setAll([flag], [])).toThrow(`flag "flag1": ${message}`); + expect(layer.isEmpty()).toBe(true); + }); + + const malformedSegments: [string, any, string][] = [ + [ + 'an included list that is not an array', + segmentWith({ included: 'user-a' }), + '"included" must be an array', + ], + [ + 'an excluded list that is not an array', + segmentWith({ excluded: {} }), + '"excluded" must be an array', + ], + [ + 'an included context target without values', + segmentWith({ includedContexts: [{ contextKind: 'org' }] }), + 'included context 0: "values" must be an array', + ], + [ + 'an excluded context target whose values are not an array', + segmentWith({ excludedContexts: [{ contextKind: 'org', values: 'org-a' }] }), + 'excluded context 0: "values" must be an array', + ], + ['rules that are not an array', segmentWith({ rules: {} }), '"rules" must be an array'], + [ + 'clauses that are not an array', + segmentWithRule({ clauses: {} }), + 'rule 0: "clauses" must be an array', + ], + [ + 'a clause without values', + segmentWithRule({ clauses: [{ attribute: 'name', op: 'in' }] }), + 'rule 0, clause 0: "values" must be an array', + ], + [ + 'a rule whose bucketBy is not a string', + segmentWithRule({ bucketBy: ['name'], clauses: [] }), + 'rule 0: "bucketBy" must be a string', + ], + ]; + + it.each(malformedSegments)('rejects a segment with %s', (_description, segment, message) => { + const layer = new OverrideLayer(); + + expect(() => layer.setAll([], [segment])).toThrow(`segment "segment1": ${message}`); + expect(layer.isEmpty()).toBe(true); + }); + + it('rejects the whole snapshot and keeps the previous layer', () => { + const layer = new OverrideLayer(); + layer.setAll([rawFlag('flag1')], [rawSegment('segment1')]); + + // The valid entries of the snapshot come before and after the malformed one, and none of + // them is applied: a snapshot is all or nothing. + expect(() => + layer.setAll( + [rawFlag('flag2'), flagWith({ key: 'bad-flag', variations: undefined }), rawFlag('flag3')], + [rawSegment('segment2')], + ), + ).toThrow('flag "bad-flag": "variations" must be an array'); + + expect(Object.keys(layer.all(VersionedDataKinds.Features))).toEqual(['flag1']); + expect(Object.keys(layer.all(VersionedDataKinds.Segments))).toEqual(['segment1']); + expect(layer.isEmpty()).toBe(false); + }); +}); diff --git a/packages/shared/sdk-server/src/api/subsystems/LDOverrideSource.ts b/packages/shared/sdk-server/src/api/subsystems/LDOverrideSource.ts index f42c973d8d..dbe06e5192 100644 --- a/packages/shared/sdk-server/src/api/subsystems/LDOverrideSource.ts +++ b/packages/shared/sdk-server/src/api/subsystems/LDOverrideSource.ts @@ -15,7 +15,10 @@ export interface LDOverrideSink { * * The definitions are plain objects in the flag and segment data model, in the form in which * LaunchDarkly delivers them. The SDK copies them, prepares the copies for evaluation, and - * marks the copies as override entries. It never modifies the objects it was given. + * marks the copies as override entries. It never modifies the objects it was given. A + * definition that does not have the shape the SDK requires, for example a flag without a + * `variations` array, is rejected with an error that names the kind, the key, and the field, + * and no part of the snapshot is applied: the layer keeps its previous contents. * * The new contents are visible to evaluations when the call returns. Flag change notifications * for the affected flags follow. diff --git a/packages/shared/sdk-server/src/overrides/OverrideLayer.ts b/packages/shared/sdk-server/src/overrides/OverrideLayer.ts index 931c6c88d7..cc9c750c2c 100644 --- a/packages/shared/sdk-server/src/overrides/OverrideLayer.ts +++ b/packages/shared/sdk-server/src/overrides/OverrideLayer.ts @@ -9,6 +9,7 @@ import { markOverrideEntry } from '../evaluation/data/overrideMarker'; import { Segment } from '../evaluation/data/Segment'; import { processFlag, processSegment } from '../store/serialization'; import VersionedDataKinds from '../store/VersionedDataKinds'; +import { validateFlag, validateSegment } from './validateDefinition'; /** * One entry of the layer: the prepared, marked definition, and the JSON text of the definition @@ -37,12 +38,16 @@ export type LayerKindContents = Record; export type LayerContents = Record; /** - * Copies a definition the source supplied, prepares the copy for evaluation the way the SDK - * prepares LaunchDarkly data, and marks it. The source's object is never modified. + * Copies a definition the source supplied, checks that the copy has the shape evaluation + * requires, prepares the copy for evaluation the way the SDK prepares LaunchDarkly data, and + * marks it. The source's object is never modified. The check is on the copy, because the copy is + * what the layer stores. A definition of the wrong shape throws. */ function prepareFlag(item: LDKeyedFeatureStoreItem): LayerEntry { const json = JSON.stringify(item); - const flag = JSON.parse(json) as Flag; + const parsed = JSON.parse(json); + validateFlag(parsed); + const flag = parsed as Flag; processFlag(flag); markOverrideEntry(flag); return { item: flag, json }; @@ -50,7 +55,9 @@ function prepareFlag(item: LDKeyedFeatureStoreItem): LayerEntry { function prepareSegment(item: LDKeyedFeatureStoreItem): LayerEntry { const json = JSON.stringify(item); - const segment = JSON.parse(json) as Segment; + const parsed = JSON.parse(json); + validateSegment(parsed); + const segment = parsed as Segment; processSegment(segment); markOverrideEntry(segment); return { item: segment, json }; @@ -79,6 +86,11 @@ export default class OverrideLayer { * Replaces the entire contents of the layer in one assignment, so the layer holds exactly one * snapshot at any instant. Empty arrays clear the layer. * + * Every definition is checked and prepared before the assignment. When one definition does + * not have the shape evaluation requires, this method throws an Error that names the kind, the + * key, and the field, and the layer keeps its previous contents: no part of the snapshot is + * applied. + * * @returns The previous and the new contents, for change comparison. Neither may be modified. */ setAll( diff --git a/packages/shared/sdk-server/src/overrides/OverrideSink.ts b/packages/shared/sdk-server/src/overrides/OverrideSink.ts index 025c8b16a2..44f0e846af 100644 --- a/packages/shared/sdk-server/src/overrides/OverrideSink.ts +++ b/packages/shared/sdk-server/src/overrides/OverrideSink.ts @@ -102,7 +102,8 @@ export default class OverrideSink implements LDOverrideSink { * Replaces the entire override layer in one assignment, then notifies listeners of every flag * whose merged-view evaluation may have changed. The replacement is visible to evaluations * when this method returns. The notifications follow, because they can need asynchronous - * reads of the base store. + * reads of the base store. A snapshot with a definition of the wrong shape is rejected by the + * layer before any of it is applied, and the error reaches the caller. */ setOverrides(flags: LDKeyedFeatureStoreItem[], segments: LDKeyedFeatureStoreItem[]): void { const { previous, current } = this._layer.setAll(flags, segments); diff --git a/packages/shared/sdk-server/src/overrides/validateDefinition.ts b/packages/shared/sdk-server/src/overrides/validateDefinition.ts new file mode 100644 index 0000000000..7d969503b6 --- /dev/null +++ b/packages/shared/sdk-server/src/overrides/validateDefinition.ts @@ -0,0 +1,186 @@ +/** + * Shape checks for the definitions an override source supplies. + * + * The SDK stores flag and segment definitions in their JSON form, without a typed + * deserialization, so evaluation reads the fields of a definition with the types the data model + * declares. LaunchDarkly data has that shape. A hand-written override definition may not, and a + * field of the wrong type makes evaluation throw where a malformed-flag result is expected. The + * override layer is the one path that admits definitions from outside LaunchDarkly, so it checks + * the shape here, before a definition is prepared and stored. + * + * The checks cover the fields that evaluation and the preparation of a definition read with an + * assumed type: a list is an array, an entry of a list is an object, and an attribute name that + * becomes an attribute reference is a string. A field is required only where evaluation reads it + * without a guard: the variations of a flag and the values of a target or clause. Every other + * field may be absent, as it may be in LaunchDarkly data, and a null field counts as absent + * because the preparation removes null fields. The checks go no further. The content of a field, + * such as a variation index or an operator name, is evaluation's concern, and evaluation reports + * a malformed-flag result for it. + */ + +type DefinitionKind = 'flag' | 'segment'; + +type Definition = Record; + +function isObject(value: unknown): value is Definition { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** + * The part of a definition under examination: the kind and key of the definition, and the steps + * from its root to the part, for example "rule 2" then "clause 0". Each check throws an Error + * that names them and the field that failed. + */ +class Location { + constructor( + private readonly _kind: DefinitionKind, + private readonly _key: string, + private readonly _steps: string[] = [], + ) {} + + at(step: string): Location { + return new Location(this._kind, this._key, [...this._steps, step]); + } + + fail(message: string): never { + const where = this._steps.length > 0 ? `${this._steps.join(', ')}: ` : ''; + throw new Error(`${this._kind} "${this._key}": ${where}${message}`); + } + + /** + * Checks that a field is an array. An optional field may be absent. + * + * @returns The array, or undefined when an optional field is absent. + */ + array(parent: Definition, field: string, required: boolean = false): any[] | undefined { + const value = parent[field]; + if (value === undefined || value === null) { + if (required) { + this.fail(`"${field}" must be an array`); + } + return undefined; + } + if (!Array.isArray(value)) { + this.fail(`"${field}" must be an array`); + } + return value; + } + + /** + * Checks that an optional field is an object when it is present. + * + * @returns The object, or undefined when the field is absent. + */ + object(parent: Definition, field: string): Definition | undefined { + const value = parent[field]; + if (value === undefined || value === null) { + return undefined; + } + if (!isObject(value)) { + this.fail(`"${field}" must be an object`); + } + return value; + } + + /** + * Checks that an optional field is a string when it is present. + */ + string(parent: Definition, field: string): void { + const value = parent[field]; + if (value !== undefined && value !== null && typeof value !== 'string') { + this.fail(`"${field}" must be a string`); + } + } + + /** + * Checks that an optional field is an array of objects when it is present, and runs a check on + * each entry at the entry's location. The label names an entry in a message, for example + * "rule" for the entries of "rules". + */ + entries( + parent: Definition, + field: string, + label: string, + check: (entry: Definition, at: Location) => void = () => {}, + ): void { + this.array(parent, field)?.forEach((entry, index) => { + if (!isObject(entry)) { + this.fail(`${label} ${index} must be an object`); + } + check(entry, this.at(`${label} ${index}`)); + }); + } +} + +/** + * A target of a flag, or a context target of a segment, is read by its values. + */ +function checkTargets(parent: Definition, at: Location, field: string, label: string): void { + at.entries(parent, field, label, (target, targetAt) => { + targetAt.array(target, 'values', true); + }); +} + +/** + * A clause is read by its values. Its attribute becomes an attribute reference during + * preparation when it is present. + */ +function checkClauses(rule: Definition, at: Location): void { + at.entries(rule, 'clauses', 'clause', (clause, clauseAt) => { + clauseAt.array(clause, 'values', true); + clauseAt.string(clause, 'attribute'); + }); +} + +/** + * A rollout, of a fallthrough or of a rule, is read by its weighted variations. Its bucketBy + * becomes an attribute reference during preparation when it is present. + */ +function checkRollout(parent: Definition, at: Location): void { + const rollout = at.object(parent, 'rollout'); + if (rollout) { + const rolloutAt = at.at('rollout'); + rolloutAt.string(rollout, 'bucketBy'); + rolloutAt.entries(rollout, 'variations', 'variation'); + } +} + +/** + * Checks that a flag definition has the shape that evaluation requires. Throws an Error that + * names the flag and the field when it does not. + * + * @internal + */ +export function validateFlag(flag: Definition): void { + const at = new Location('flag', `${flag.key}`); + at.array(flag, 'variations', true); + at.entries(flag, 'prerequisites', 'prerequisite'); + checkTargets(flag, at, 'targets', 'target'); + checkTargets(flag, at, 'contextTargets', 'context target'); + const fallthrough = at.object(flag, 'fallthrough'); + if (fallthrough) { + checkRollout(fallthrough, at.at('fallthrough')); + } + at.entries(flag, 'rules', 'rule', (rule, ruleAt) => { + checkClauses(rule, ruleAt); + checkRollout(rule, ruleAt); + }); +} + +/** + * Checks that a segment definition has the shape that evaluation requires. Throws an Error that + * names the segment and the field when it does not. + * + * @internal + */ +export function validateSegment(segment: Definition): void { + const at = new Location('segment', `${segment.key}`); + at.array(segment, 'included'); + at.array(segment, 'excluded'); + checkTargets(segment, at, 'includedContexts', 'included context'); + checkTargets(segment, at, 'excludedContexts', 'excluded context'); + at.entries(segment, 'rules', 'rule', (rule, ruleAt) => { + checkClauses(rule, ruleAt); + ruleAt.string(rule, 'bucketBy'); + }); +} From 7933b13851fa77e68ee144899b9cd674b1ae7c13 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 9 Oct 2026 22:04:43 +0000 Subject: [PATCH 7/9] fix: Compare override entries by deep equality and copy them with the shared clone helper The override layer copied each supplied definition with an inline JSON round trip and kept the serialized text on the entry so the sink could tell which entries changed between snapshots. JSON.stringify is not canonical: the same definition supplied with its properties in another order, for example after a file was reformatted or by a source that builds objects in varying order, produced different text and a spurious flag change notification. The sink now compares the prepared entries with fastDeepEqual, which ignores property order, the same way the client SDK's flag manager computes changed keys. The layer no longer stores text: an entry is the prepared, marked copy itself, made with the clone helper from js-sdk-common instead of a hand-rolled round trip. A new test supplies a definition again with every object's properties reversed and expects no notification. --- .../__tests__/overrides/OverrideLayer.test.ts | 9 ++-- .../__tests__/overrides/OverrideSink.test.ts | 31 ++++++++++++ .../sdk-server/src/overrides/OverrideLayer.ts | 49 +++++++------------ .../sdk-server/src/overrides/OverrideSink.ts | 9 ++-- .../shared/sdk-server/src/overrides/index.ts | 3 +- 5 files changed, 59 insertions(+), 42 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts b/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts index 499d20942b..955cbea13e 100644 --- a/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts @@ -113,16 +113,17 @@ it('returns all entries of a kind keyed by key', () => { expect(new OverrideLayer().all(VersionedDataKinds.Features)).toEqual({}); }); -it('returns the previous and the new contents with the supplied text of each entry', () => { +it('returns the previous and the new contents', () => { const layer = new OverrideLayer(); const first = layer.setAll([rawFlag('flag1', 1)], []); expect(first.previous.features).toEqual({}); - expect(first.current.features.flag1.json).toEqual(JSON.stringify(rawFlag('flag1', 1))); + expect((first.current.features.flag1 as Flag).version).toEqual(1); + expect((first.current.features.flag1 as Flag)._sdk_override).toBe(true); const second = layer.setAll([rawFlag('flag1', 2)], [rawSegment('segment1')]); expect(second.previous).toBe(first.current); - expect(second.current.features.flag1.json).toEqual(JSON.stringify(rawFlag('flag1', 2))); - expect(second.current.segments.segment1.item.key).toEqual('segment1'); + expect((second.current.features.flag1 as Flag).version).toEqual(2); + expect((second.current.segments.segment1 as Segment).key).toEqual('segment1'); }); describe('given definitions with the shape evaluation requires', () => { diff --git a/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts b/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts index 8860ba31cd..85c9d867d8 100644 --- a/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts @@ -31,6 +31,22 @@ function flagWithPrerequisite(key: string, prereqKey: string, version: number = }; } +// Rebuilds a definition with the properties of every object in it in the opposite order, so +// that it is the same definition with a different serialization. +function withReversedProperties(value: any): any { + if (Array.isArray(value)) { + return value.map(withReversedProperties); + } + if (value && typeof value === 'object') { + return Object.fromEntries( + Object.entries(value) + .reverse() + .map(([key, item]) => [key, withReversedProperties(item)]), + ); + } + return value; +} + // Notifications follow the layer replacement asynchronously. Let them run. const settle = () => new Promise((resolve) => { @@ -94,6 +110,21 @@ describe('given a sink over an initialized base store', () => { expect(takeNotified()).toEqual(['flag1', 'flag2']); }); + it('does not notify when a definition is supplied again with its properties in another order', async () => { + const original = flagWithSegmentRule('flag1', 'segment1', 1); + const reordered = withReversedProperties(original); + expect(reordered).toEqual(original); + expect(JSON.stringify(reordered)).not.toEqual(JSON.stringify(original)); + + sink.setOverrides([original], []); + await settle(); + expect(takeNotified()).toEqual(['flag1']); + + sink.setOverrides([reordered], []); + await settle(); + expect(takeNotified()).toEqual([]); + }); + it('makes the new layer contents visible before the notifications run', () => { sink.setOverrides([singleValueFlag('flag1', 'a')], []); expect(layer.get(VersionedDataKinds.Features, 'flag1')).toBeDefined(); diff --git a/packages/shared/sdk-server/src/overrides/OverrideLayer.ts b/packages/shared/sdk-server/src/overrides/OverrideLayer.ts index cc9c750c2c..85936aa9be 100644 --- a/packages/shared/sdk-server/src/overrides/OverrideLayer.ts +++ b/packages/shared/sdk-server/src/overrides/OverrideLayer.ts @@ -1,3 +1,5 @@ +import { clone } from '@launchdarkly/js-sdk-common'; + import { DataKind } from '../api/interfaces'; import { LDFeatureStoreItem, @@ -12,23 +14,12 @@ import VersionedDataKinds from '../store/VersionedDataKinds'; import { validateFlag, validateSegment } from './validateDefinition'; /** - * One entry of the layer: the prepared, marked definition, and the JSON text of the definition - * as the source supplied it. The text identifies the entry for change comparison, because the - * layer builds new copies on every snapshot. - * - * @internal - */ -export interface LayerEntry { - item: LDFeatureStoreItem; - json: string; -} - -/** - * The entries of one kind, keyed by item key. + * The entries of one kind, keyed by item key. Each entry is the prepared, marked copy of a + * definition the source supplied. * * @internal */ -export type LayerKindContents = Record; +export type LayerKindContents = Record; /** * The entries of the layer, keyed by namespace and then by item key. @@ -43,24 +34,22 @@ export type LayerContents = Record; * marks it. The source's object is never modified. The check is on the copy, because the copy is * what the layer stores. A definition of the wrong shape throws. */ -function prepareFlag(item: LDKeyedFeatureStoreItem): LayerEntry { - const json = JSON.stringify(item); - const parsed = JSON.parse(json); - validateFlag(parsed); - const flag = parsed as Flag; +function prepareFlag(item: LDKeyedFeatureStoreItem): LDFeatureStoreItem { + const copy = clone>(item); + validateFlag(copy); + const flag = copy as Flag; processFlag(flag); markOverrideEntry(flag); - return { item: flag, json }; + return flag; } -function prepareSegment(item: LDKeyedFeatureStoreItem): LayerEntry { - const json = JSON.stringify(item); - const parsed = JSON.parse(json); - validateSegment(parsed); - const segment = parsed as Segment; +function prepareSegment(item: LDKeyedFeatureStoreItem): LDFeatureStoreItem { + const copy = clone>(item); + validateSegment(copy); + const segment = copy as Segment; processSegment(segment); markOverrideEntry(segment); - return { item: segment, json }; + return segment; } function emptyContents(): LayerContents { @@ -117,18 +106,14 @@ export default class OverrideLayer { if (this._empty) { return undefined; } - return this._contents[kind.namespace]?.[key]?.item; + return this._contents[kind.namespace]?.[key]; } /** * Returns the entries of the given kind, keyed by item key. */ all(kind: DataKind): LDFeatureStoreKindData { - const result: LDFeatureStoreKindData = {}; - Object.entries(this._contents[kind.namespace] ?? {}).forEach(([key, entry]) => { - result[key] = entry.item; - }); - return result; + return { ...this._contents[kind.namespace] }; } /** diff --git a/packages/shared/sdk-server/src/overrides/OverrideSink.ts b/packages/shared/sdk-server/src/overrides/OverrideSink.ts index 44f0e846af..7bcc6fbcc4 100644 --- a/packages/shared/sdk-server/src/overrides/OverrideSink.ts +++ b/packages/shared/sdk-server/src/overrides/OverrideSink.ts @@ -1,4 +1,4 @@ -import { LDLogger } from '@launchdarkly/js-sdk-common'; +import { fastDeepEqual, LDLogger } from '@launchdarkly/js-sdk-common'; import { LDFeatureStoreKindData, LDKeyedFeatureStoreItem, LDOverrideSink } from '../api/subsystems'; import { computeDependencies } from '../data_sources/DataSourceUpdates'; @@ -22,7 +22,8 @@ type MergedView = Record; * Returns the keys whose override entry differs between two layer snapshots. An added or removed * entry is always a change, even when its content matches the underlying LaunchDarkly data: the * override marker alone changes the served entry. Entries present in both snapshots are compared - * by the text the source supplied, because the layer is rebuilt on every snapshot. + * by deep equality of the prepared definitions, because the layer is rebuilt on every snapshot, + * so a reference comparison would report every retained entry as changed. */ function diffContents( previous: LayerContents, @@ -35,7 +36,7 @@ function diffContents( const newItems = current[namespace] ?? {}; Object.entries(oldItems).forEach(([key, oldEntry]) => { const newEntry = newItems[key]; - if (!newEntry || newEntry.json !== oldEntry.json) { + if (!newEntry || !fastDeepEqual(oldEntry, newEntry)) { seeds.set(namespace, key, true); count += 1; } @@ -55,7 +56,7 @@ function mergedView(base: MergedView, contents: LayerContents): MergedView { diffNamespaces.forEach((namespace) => { const items: LDFeatureStoreKindData = { ...base[namespace] }; Object.entries(contents[namespace] ?? {}).forEach(([key, entry]) => { - items[key] = entry.item; + items[key] = entry; }); view[namespace] = items; }); diff --git a/packages/shared/sdk-server/src/overrides/index.ts b/packages/shared/sdk-server/src/overrides/index.ts index 23d3ba565d..ae61223012 100644 --- a/packages/shared/sdk-server/src/overrides/index.ts +++ b/packages/shared/sdk-server/src/overrides/index.ts @@ -1,12 +1,11 @@ import createOverrideSource from './createOverrideSource'; -import OverrideLayer, { LayerContents, LayerEntry, LayerKindContents } from './OverrideLayer'; +import OverrideLayer, { LayerContents, LayerKindContents } from './OverrideLayer'; import OverrideSink from './OverrideSink'; import ReadStoreOverlay, { ReadStore } from './ReadStoreOverlay'; export { createOverrideSource, LayerContents, - LayerEntry, LayerKindContents, OverrideLayer, OverrideSink, From 7942c7f36f90b29202f42c69d9ff5e9927af16f7 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 9 Oct 2026 22:29:39 +0000 Subject: [PATCH 8/9] refactor: Reuse existing helpers in the override layer, sink, and client marking The override indicator helper that the evaluator kept private now lives with the other marker functions, and the client uses it for the two wrong-type error paths instead of re-implementing the spread. The sink's merged view is the existing feature store storage type. The definition shape checks use the common validators for the plain-object and nullish tests and export the plain-object check for the document parser. The tests use the common sleep helper and one shared jest mock logger. --- .../__tests__/LDClientImpl.overrides.test.ts | 24 +++++++------------ .../shared/sdk-server/__tests__/mockLogger.ts | 15 ++++++++++++ .../__tests__/overrides/OverrideSink.test.ts | 7 +++--- .../shared/sdk-server/src/LDClientImpl.ts | 13 ++++++---- .../sdk-server/src/evaluation/Evaluator.ts | 9 +------ .../src/evaluation/data/overrideMarker.ts | 8 +++++++ .../sdk-server/src/overrides/OverrideSink.ts | 9 +++++-- .../src/overrides/validateDefinition.ts | 21 ++++++++++------ 8 files changed, 66 insertions(+), 40 deletions(-) create mode 100644 packages/shared/sdk-server/__tests__/mockLogger.ts diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts index 9670616ed7..14d2d880c6 100644 --- a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts @@ -1,10 +1,11 @@ -import { LDClientContext, LDLogger } from '@launchdarkly/js-sdk-common'; +import { LDClientContext, sleep } from '@launchdarkly/js-sdk-common'; import { LDMigrationStage } from '../src/api/data/LDMigrationStage'; import { LDOverrideSink } from '../src/api/subsystems'; import LDClientImpl from '../src/LDClientImpl'; import InMemoryFeatureStore from '../src/store/InMemoryFeatureStore'; import { TestHook } from './hooks/TestHook'; +import makeMockLogger, { MockLogger } from './mockLogger'; import { fdv2FullPayload, makeCallbacks, @@ -16,27 +17,20 @@ import TestOverrideSource from './overrides/TestOverrideSource'; const user = { key: 'user-key' }; -function makeLogger(): LDLogger & { warn: jest.Mock; error: jest.Mock } { - return { error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() }; -} - function warningsMatching(logger: { warn: jest.Mock }, pattern: RegExp): number { return logger.warn.mock.calls.filter((call) => pattern.test(String(call[0]))).length; } // Change notifications and asynchronous starts complete on later turns of the event loop. -const settle = () => - new Promise((resolve) => { - setTimeout(resolve, 10); - }); +const settle = () => sleep(10); describe('given an uninitialized client with an override source', () => { let source: TestOverrideSource; let client: LDClientImpl; - let logger: ReturnType; + let logger: MockLogger; beforeEach(() => { - logger = makeLogger(); + logger = makeMockLogger(); source = new TestOverrideSource({ flags: [singleValueFlag('overridden-flag', true)] }); client = makeFDv2Client(makeFDv2Platform(), { logger, dataSystem: { overrides: source } }); }); @@ -483,7 +477,7 @@ describe('given override source lifecycle and configuration', () => { }); it('logs and continues the shutdown when the source fails to close', () => { - const logger = makeLogger(); + const logger = makeMockLogger(); const throwing = { start: () => {}, close: () => { @@ -534,7 +528,7 @@ describe('given override source lifecycle and configuration', () => { }); it('warns and ignores an override option of the wrong type', async () => { - const logger = makeLogger(); + const logger = makeMockLogger(); client = makeFDv2Client(makeFDv2Platform(), { logger, dataSystem: { overrides: 'not an override source' as any }, @@ -546,7 +540,7 @@ describe('given override source lifecycle and configuration', () => { }); it('logs and continues when the initial load fails', async () => { - const logger = makeLogger(); + const logger = makeMockLogger(); const failing = { start: () => Promise.reject(new Error('load failed')), close: () => {}, @@ -559,7 +553,7 @@ describe('given override source lifecycle and configuration', () => { }); it('logs and continues when start throws', async () => { - const logger = makeLogger(); + const logger = makeMockLogger(); const throwing = { start: (_sink: LDOverrideSink) => { throw new Error('start failed'); diff --git a/packages/shared/sdk-server/__tests__/mockLogger.ts b/packages/shared/sdk-server/__tests__/mockLogger.ts new file mode 100644 index 0000000000..28c0b6799c --- /dev/null +++ b/packages/shared/sdk-server/__tests__/mockLogger.ts @@ -0,0 +1,15 @@ +import { LDLogger } from '@launchdarkly/js-sdk-common'; + +/** + * A logger whose four methods are jest mocks, for tests that assert on what was logged. + */ +export type MockLogger = LDLogger & { + error: jest.Mock; + warn: jest.Mock; + info: jest.Mock; + debug: jest.Mock; +}; + +export default function makeMockLogger(): MockLogger { + return { error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() }; +} diff --git a/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts b/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts index 85c9d867d8..96bec5d41b 100644 --- a/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts @@ -1,3 +1,5 @@ +import { sleep } from '@launchdarkly/js-sdk-common'; + import { LDFeatureStoreDataStorage } from '../../src/api/subsystems'; import { OverrideLayer, OverrideSink } from '../../src/overrides'; import AsyncStoreFacade from '../../src/store/AsyncStoreFacade'; @@ -48,10 +50,7 @@ function withReversedProperties(value: any): any { } // Notifications follow the layer replacement asynchronously. Let them run. -const settle = () => - new Promise((resolve) => { - setTimeout(resolve, 0); - }); +const settle = () => sleep(0); describe('given a sink over an initialized base store', () => { let base: InMemoryFeatureStore; diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 4bdcd8910b..ba1d118fcc 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -59,6 +59,7 @@ import TransactionalDataSourceUpdates from './data_sources/TransactionalDataSour import createDiagnosticsInitConfig from './diagnostics/createDiagnosticsInitConfig'; import { allAsync } from './evaluation/collection'; import { Flag } from './evaluation/data/Flag'; +import { markOverrideAffected } from './evaluation/data/overrideMarker'; import { Segment } from './evaluation/data/Segment'; import EvalResult from './evaluation/EvalResult'; import Evaluator from './evaluation/Evaluator'; @@ -1164,9 +1165,13 @@ export default class LDClientImpl implements LDClient { this._onError(error); // The type mismatch replaces the reason. The evaluation read the same definitions, so // the new reason keeps the override-affected marking. - const reason: LDEvaluationReason = detail.reason.overrideAffected - ? { kind: 'ERROR', errorKind: ErrorKinds.WrongType, overrideAffected: true } - : { kind: 'ERROR', errorKind: ErrorKinds.WrongType }; + const errorReason: LDEvaluationReason = { + kind: 'ERROR', + errorKind: ErrorKinds.WrongType, + }; + const reason = detail.reason.overrideAffected + ? markOverrideAffected(errorReason) + : errorReason; resolve({ detail: { value: defaultValue, @@ -1487,7 +1492,7 @@ export default class LDClientImpl implements LDClient { if (evalRes.overrideAffected) { // The type mismatch replaces the reason. The evaluation read the same definitions, // so the new result keeps the override-affected marking. - errorRes.detail.reason = { ...errorRes.detail.reason, overrideAffected: true }; + errorRes.detail.reason = markOverrideAffected(errorRes.detail.reason); errorRes.overrideAffected = true; } this._sendEvalEvent(errorRes, eventFactory, flag, evalContext, defaultValue); diff --git a/packages/shared/sdk-server/src/evaluation/Evaluator.ts b/packages/shared/sdk-server/src/evaluation/Evaluator.ts index a2c1445997..5eee4e5e33 100644 --- a/packages/shared/sdk-server/src/evaluation/Evaluator.ts +++ b/packages/shared/sdk-server/src/evaluation/Evaluator.ts @@ -7,7 +7,7 @@ import { allSeriesAsync, firstSeriesAsync } from './collection'; import { Clause } from './data/Clause'; import { Flag } from './data/Flag'; import { FlagRule } from './data/FlagRule'; -import { isOverrideEntry } from './data/overrideMarker'; +import { isOverrideEntry, markOverrideAffected } from './data/overrideMarker'; import { Segment } from './data/Segment'; import { SegmentRule } from './data/SegmentRule'; import { VariationOrRollout } from './data/VariationOrRollout'; @@ -82,13 +82,6 @@ interface EvalState { overrideAffected: boolean; } -/** - * Returns a copy of the reason with the override indicator set. - */ -function markOverrideAffected(reason: LDEvaluationReason): LDEvaluationReason { - return { ...reason, overrideAffected: true }; -} - interface Match { error: false; isMatch: boolean; diff --git a/packages/shared/sdk-server/src/evaluation/data/overrideMarker.ts b/packages/shared/sdk-server/src/evaluation/data/overrideMarker.ts index 1b5e8a900d..428b65a896 100644 --- a/packages/shared/sdk-server/src/evaluation/data/overrideMarker.ts +++ b/packages/shared/sdk-server/src/evaluation/data/overrideMarker.ts @@ -1,4 +1,5 @@ /* eslint-disable no-underscore-dangle */ +import { LDEvaluationReason } from '@launchdarkly/js-sdk-common'; /** * The override marker is carried on flag and segment definitions under a reserved, namespaced @@ -91,3 +92,10 @@ export function stripOverrideMarker(item: OverrideMarkable | undefined | null): delete item._sdk_override; } } + +/** + * Returns a copy of an evaluation reason with the override indicator set. + */ +export function markOverrideAffected(reason: LDEvaluationReason): LDEvaluationReason { + return { ...reason, overrideAffected: true }; +} diff --git a/packages/shared/sdk-server/src/overrides/OverrideSink.ts b/packages/shared/sdk-server/src/overrides/OverrideSink.ts index 7bcc6fbcc4..5ad40410db 100644 --- a/packages/shared/sdk-server/src/overrides/OverrideSink.ts +++ b/packages/shared/sdk-server/src/overrides/OverrideSink.ts @@ -1,6 +1,11 @@ import { fastDeepEqual, LDLogger } from '@launchdarkly/js-sdk-common'; -import { LDFeatureStoreKindData, LDKeyedFeatureStoreItem, LDOverrideSink } from '../api/subsystems'; +import { + LDFeatureStoreDataStorage, + LDFeatureStoreKindData, + LDKeyedFeatureStoreItem, + LDOverrideSink, +} from '../api/subsystems'; import { computeDependencies } from '../data_sources/DataSourceUpdates'; import DependencyTracker from '../data_sources/DependencyTracker'; import NamespacedDataSet from '../data_sources/NamespacedDataSet'; @@ -16,7 +21,7 @@ const diffNamespaces = [ /** * The data visible at the store read boundary: base data with override entries overlaid. */ -type MergedView = Record; +type MergedView = LDFeatureStoreDataStorage; /** * Returns the keys whose override entry differs between two layer snapshots. An added or removed diff --git a/packages/shared/sdk-server/src/overrides/validateDefinition.ts b/packages/shared/sdk-server/src/overrides/validateDefinition.ts index 7d969503b6..9a1ea46811 100644 --- a/packages/shared/sdk-server/src/overrides/validateDefinition.ts +++ b/packages/shared/sdk-server/src/overrides/validateDefinition.ts @@ -18,12 +18,19 @@ * a malformed-flag result for it. */ +import { isNullish, TypeValidators } from '@launchdarkly/js-sdk-common'; + type DefinitionKind = 'flag' | 'segment'; type Definition = Record; -function isObject(value: unknown): value is Definition { - return typeof value === 'object' && value !== null && !Array.isArray(value); +/** + * Reports whether a value is an object that is not an array and not null. + * + * @internal + */ +export function isPlainObject(value: unknown): value is Definition { + return !isNullish(value) && TypeValidators.Object.is(value); } /** @@ -54,7 +61,7 @@ class Location { */ array(parent: Definition, field: string, required: boolean = false): any[] | undefined { const value = parent[field]; - if (value === undefined || value === null) { + if (isNullish(value)) { if (required) { this.fail(`"${field}" must be an array`); } @@ -73,10 +80,10 @@ class Location { */ object(parent: Definition, field: string): Definition | undefined { const value = parent[field]; - if (value === undefined || value === null) { + if (isNullish(value)) { return undefined; } - if (!isObject(value)) { + if (!isPlainObject(value)) { this.fail(`"${field}" must be an object`); } return value; @@ -87,7 +94,7 @@ class Location { */ string(parent: Definition, field: string): void { const value = parent[field]; - if (value !== undefined && value !== null && typeof value !== 'string') { + if (!isNullish(value) && typeof value !== 'string') { this.fail(`"${field}" must be a string`); } } @@ -104,7 +111,7 @@ class Location { check: (entry: Definition, at: Location) => void = () => {}, ): void { this.array(parent, field)?.forEach((entry, index) => { - if (!isObject(entry)) { + if (!isPlainObject(entry)) { this.fail(`${label} ${index} must be an object`); } check(entry, this.at(`${label} ${index}`)); From 27aa6a8aa6b26de52aac6220c072061505a2401d Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 9 Oct 2026 23:13:37 +0000 Subject: [PATCH 9/9] fix: Read the override layer before the store on an uninitialized client, and harden the layer against odd keys and fields Before initialization, an evaluation asked the data store whether it was initialized before it looked in the override layer. With a persistent store that check is I/O, so an override could wait on a store that was slow or unreachable, in the incident the overrides exist for. The layer is now consulted first, and the source is created before the store so an invalid override configuration fails construction before a store connects. The start handle that waitForInitialization returns now settles after the override source's initial load whether initialization succeeded or failed, including when it had already failed, so an application that handles a failed start and evaluates sees the overrides present at startup. The layer's maps and the merged views no longer have a prototype, so a key such as "constructor" or "__proto__" is an ordinary entry and a key the layer does not hold is not found through Object.prototype. The sink treats an entry whose comparison throws, for a definition with a property named like a method of Object, as changed instead of dropping the snapshot's notifications. The shape check now requires a string key, requires the on switch to be a boolean when present, and rejects the fields the SDK sets when it prepares a definition (the generated target sets and the compiled bucket-by reference), which evaluation would otherwise read with the wrong type. --- .../__tests__/LDClientImpl.overrides.test.ts | 130 +++++++++++++++++- .../__tests__/overrides/OverrideLayer.test.ts | 85 ++++++++++++ .../__tests__/overrides/OverrideSink.test.ts | 24 +++- .../shared/sdk-server/__tests__/waitFor.ts | 21 +++ .../shared/sdk-server/src/LDClientImpl.ts | 48 ++++--- .../sdk-server/src/overrides/OverrideLayer.ts | 16 ++- .../sdk-server/src/overrides/OverrideSink.ts | 17 ++- .../src/overrides/ReadStoreOverlay.ts | 3 +- .../src/overrides/validateDefinition.ts | 61 ++++++-- 9 files changed, 371 insertions(+), 34 deletions(-) create mode 100644 packages/shared/sdk-server/__tests__/waitFor.ts diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts index 14d2d880c6..54a0248a59 100644 --- a/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.overrides.test.ts @@ -1,9 +1,10 @@ import { LDClientContext, sleep } from '@launchdarkly/js-sdk-common'; import { LDMigrationStage } from '../src/api/data/LDMigrationStage'; -import { LDOverrideSink } from '../src/api/subsystems'; +import { LDFeatureStore, LDOverrideSink } from '../src/api/subsystems'; import LDClientImpl from '../src/LDClientImpl'; import InMemoryFeatureStore from '../src/store/InMemoryFeatureStore'; +import { createBasicPlatform } from './createBasicPlatform'; import { TestHook } from './hooks/TestHook'; import makeMockLogger, { MockLogger } from './mockLogger'; import { @@ -14,6 +15,7 @@ import { singleValueFlag, } from './overrides/overridesTestSupport'; import TestOverrideSource from './overrides/TestOverrideSource'; +import waitFor from './waitFor'; const user = { key: 'user-key' }; @@ -340,6 +342,132 @@ describe('given a client with flag change listeners', () => { }); }); +// A persistent store whose initialization check never answers, as a store that cannot be reached +// while its first connection attempt is pending. Reads report nothing. +function unreachableStore(): LDFeatureStore { + return { + get: (_kind, _key, callback) => callback(null), + all: (_kind, callback) => callback({}), + init: (_data, callback) => callback(), + delete: (_kind, _key, _version, callback) => callback(), + upsert: (_kind, _data, callback) => callback(), + initialized: () => {}, + close: () => {}, + getDescription: () => 'unreachable', + } as LDFeatureStore; +} + +// A platform whose every request is rejected with 401, so that initialization fails. +function unauthorizedPlatform() { + const platform = createBasicPlatform(); + platform.requests.fetch = jest.fn(() => + Promise.resolve({ status: 401, headers: new Headers(), text: async () => '' }), + ) as any; + return platform; +} + +describe('given an uninitialized client over a persistent store that does not answer', () => { + let client: LDClientImpl; + + beforeEach(() => { + const source = new TestOverrideSource({ flags: [singleValueFlag('overridden-flag', true)] }); + client = makeFDv2Client(makeFDv2Platform(), { + logger: makeMockLogger(), + dataSystem: { overrides: source, persistentStore: unreachableStore() }, + }); + }); + + afterEach(() => { + client.close(); + }); + + it('serves an override without asking the store whether it is initialized', async () => { + // With a persistent store the initialization check is I/O. An override is read before it, + // so a store that never answers does not delay an overridden flag. + const outcome = await Promise.race([ + client.boolVariationDetail('overridden-flag', user, false), + sleep(200).then(() => 'still waiting after 200 ms'), + ]); + + expect(outcome).toEqual( + expect.objectContaining({ + value: true, + reason: { kind: 'FALLTHROUGH', overrideAffected: true }, + }), + ); + }); +}); + +describe('given a client whose initialization fails and an override source with an asynchronous initial load', () => { + let source: TestOverrideSource; + let client: LDClientImpl; + let callbacks: ReturnType; + + beforeEach(() => { + source = new TestOverrideSource(undefined, true); + callbacks = makeCallbacks(); + client = makeFDv2Client( + unauthorizedPlatform() as any, + { logger: makeMockLogger(), dataSystem: { overrides: source } }, + callbacks, + ); + }); + + afterEach(() => { + client.close(); + }); + + const completeLoad = () => { + source.setOverrides([singleValueFlag('overridden-flag', true)]); + source.completeStart(); + }; + + it('reports the failed start only after the initial load', async () => { + // A failed start is still the completion of the start operation, so it waits for the load. + // An application that handles the failure and evaluates anyway sees the overrides. + let outcome: string | undefined; + const pending = client.waitForInitialization({ timeout: 5 }).then( + () => { + outcome = 'resolved'; + }, + () => { + outcome = 'rejected'; + }, + ); + await waitFor(() => (callbacks.onFailed as jest.Mock).mock.calls.length > 0); + await sleep(10); + expect(outcome).toBeUndefined(); + + completeLoad(); + await pending; + + expect(outcome).toEqual('rejected'); + expect(await client.boolVariation('overridden-flag', user, false)).toBe(true); + }); + + it('waits for the initial load when the start had already failed', async () => { + await waitFor(() => (callbacks.onFailed as jest.Mock).mock.calls.length > 0); + + let outcome: string | undefined; + const pending = client.waitForInitialization({ timeout: 5 }).then( + () => { + outcome = 'resolved'; + }, + () => { + outcome = 'rejected'; + }, + ); + await sleep(10); + expect(outcome).toBeUndefined(); + + completeLoad(); + await pending; + + expect(outcome).toEqual('rejected'); + expect(await client.boolVariation('overridden-flag', user, false)).toBe(true); + }); +}); + describe('given an override source with an asynchronous initial load', () => { let source: TestOverrideSource; let client: LDClientImpl; diff --git a/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts b/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts index 955cbea13e..093de3e116 100644 --- a/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts @@ -327,6 +327,17 @@ describe('given a definition that does not have the shape evaluation requires', flagWithRule({ variation: undefined, rollout: { variations: [null] } }), 'rule 0, rollout: variation 0 must be an object', ], + ['an on switch that is not a boolean', flagWith({ on: 'off' }), '"on" must be a boolean'], + [ + 'a rollout that carries the bucket-by reference the SDK compiles', + flagWith({ fallthrough: { rollout: { variations: [], bucketByAttributeReference: {} } } }), + 'fallthrough, rollout: "bucketByAttributeReference" is set by the SDK and cannot be supplied', + ], + [ + 'a field the SDK generates', + flagWith({ generated_something: {} }), + '"generated_something" is set by the SDK and cannot be supplied', + ], ]; it.each(malformedFlags)('rejects %s', (_description, flag, message) => { @@ -373,6 +384,31 @@ describe('given a definition that does not have the shape evaluation requires', segmentWithRule({ bucketBy: ['name'], clauses: [] }), 'rule 0: "bucketBy" must be a string', ], + [ + 'the included set the SDK generates for a large target list', + segmentWith({ generated_includedSet: ['user-a'] }), + '"generated_includedSet" is set by the SDK and cannot be supplied', + ], + [ + 'a context target that carries the values set the SDK generates', + segmentWith({ + includedContexts: [{ contextKind: 'org', values: ['org-a'], generated_valuesSet: {} }], + }), + 'included context 0: "generated_valuesSet" is set by the SDK and cannot be supplied', + ], + [ + 'a rule that carries the bucket-by reference the SDK compiles', + segmentWith({ + rules: [ + { + id: 'rule', + clauses: [{ attribute: 'name', op: 'in', values: ['x'] }], + bucketByAttributeReference: {}, + }, + ], + }), + 'rule 0: "bucketByAttributeReference" is set by the SDK and cannot be supplied', + ], ]; it.each(malformedSegments)('rejects a segment with %s', (_description, segment, message) => { @@ -382,6 +418,29 @@ describe('given a definition that does not have the shape evaluation requires', expect(layer.isEmpty()).toBe(true); }); + it.each([ + ['no key', { ...rawFlag('flag1'), key: undefined }, 'flag "undefined": "key" must be a string'], + ['a null key', { ...rawFlag('flag1'), key: null }, 'flag "null": "key" must be a string'], + [ + 'a key that is not a string', + { ...rawFlag('flag1'), key: 7 }, + 'flag "7": "key" must be a string', + ], + ])('rejects a flag with %s', (_description, flag, message) => { + const layer = new OverrideLayer(); + + expect(() => layer.setAll([flag], [])).toThrow(message); + expect(layer.isEmpty()).toBe(true); + }); + + it('rejects a segment without a key', () => { + const layer = new OverrideLayer(); + + expect(() => layer.setAll([], [{ ...rawSegment('segment1'), key: undefined }])).toThrow( + 'segment "undefined": "key" must be a string', + ); + }); + it('rejects the whole snapshot and keeps the previous layer', () => { const layer = new OverrideLayer(); layer.setAll([rawFlag('flag1')], [rawSegment('segment1')]); @@ -400,3 +459,29 @@ describe('given a definition that does not have the shape evaluation requires', expect(layer.isEmpty()).toBe(false); }); }); + +describe('given keys that are the names of properties of Object', () => { + // The layer's maps have no prototype, so such keys are ordinary entries and a lookup of one + // the layer does not hold finds nothing, rather than a method inherited from Object. + const features = VersionedDataKinds.Features; + + it('stores and returns entries with such keys', () => { + const layer = new OverrideLayer(); + layer.setAll([rawFlag('constructor'), rawFlag('__proto__'), rawFlag('flag1')], []); + + expect((layer.get(features, 'constructor') as Flag).key).toEqual('constructor'); + expect((layer.get(features, '__proto__') as Flag).key).toEqual('__proto__'); + expect(Object.keys(layer.all(features)).sort()).toEqual(['__proto__', 'constructor', 'flag1']); + }); + + it('finds nothing for such a key that the layer does not hold', () => { + const layer = new OverrideLayer(); + layer.setAll([rawFlag('flag1')], []); + + expect(layer.get(features, 'constructor')).toBeUndefined(); + expect(layer.get(features, 'toString')).toBeUndefined(); + expect(layer.get(features, 'hasOwnProperty')).toBeUndefined(); + expect(layer.get(features, '__proto__')).toBeUndefined(); + expect((layer.get(features, 'flag1') as Flag).on).toBe(true); + }); +}); diff --git a/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts b/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts index 96bec5d41b..743168a20d 100644 --- a/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/OverrideSink.test.ts @@ -5,7 +5,7 @@ import { OverrideLayer, OverrideSink } from '../../src/overrides'; import AsyncStoreFacade from '../../src/store/AsyncStoreFacade'; import InMemoryFeatureStore from '../../src/store/InMemoryFeatureStore'; import VersionedDataKinds from '../../src/store/VersionedDataKinds'; -import TestLogger from '../Logger'; +import TestLogger, { LogLevel } from '../Logger'; import { singleValueFlag } from './overridesTestSupport'; function flagWithSegmentRule(key: string, segmentKey: string, version: number = 1): any { @@ -124,6 +124,28 @@ describe('given a sink over an initialized base store', () => { expect(takeNotified()).toEqual([]); }); + it('still notifies the changed flags when a definition has a property named like a method of Object', async () => { + // Comparing such a definition for changes can throw. That entry counts as changed, and the + // notifications of the rest of the snapshot are not lost. + const odd = singleValueFlag('odd-flag', { valueOf: 'x', toString: 'y' }, 1); + sink.setOverrides([odd, singleValueFlag('flag1', 'a', 1)], []); + await settle(); + takeNotified(); + + sink.setOverrides([odd, singleValueFlag('flag1', 'b', 1)], []); + await settle(); + + expect(takeNotified()).toContain('flag1'); + expect(logger.getCount(LogLevel.Error)).toEqual(0); + }); + + it('notifies an override whose key is the name of a property of Object', async () => { + sink.setOverrides([singleValueFlag('constructor', 'a', 1)], []); + await settle(); + + expect(takeNotified()).toEqual(['constructor']); + }); + it('makes the new layer contents visible before the notifications run', () => { sink.setOverrides([singleValueFlag('flag1', 'a')], []); expect(layer.get(VersionedDataKinds.Features, 'flag1')).toBeDefined(); diff --git a/packages/shared/sdk-server/__tests__/waitFor.ts b/packages/shared/sdk-server/__tests__/waitFor.ts new file mode 100644 index 0000000000..386ca31295 --- /dev/null +++ b/packages/shared/sdk-server/__tests__/waitFor.ts @@ -0,0 +1,21 @@ +/** + * 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 new Promise((resolve) => { + setTimeout(resolve, intervalMs); + }); + } + throw new Error('timed out waiting for the condition'); +} diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index ba1d118fcc..fa6a36e1c3 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -333,6 +333,12 @@ function constructFDv2( const clientContext = new ClientContext(sdkKey, config, platform); const dataSystem = config.dataSystem!; // dataSystem must be defined to get into this construct function + // The override source is created before the store, so that an invalid override configuration + // fails client construction before a persistent store has opened a connection. + const overrideSource = + dataSystem.overrides !== undefined && !config.offline + ? createOverrideSource(dataSystem.overrides, clientContext) + : undefined; const featureStore = dataSystem.featureStoreFactory(clientContext); const dataSourceUpdates = new TransactionalDataSourceUpdates( @@ -343,17 +349,16 @@ function constructFDv2( // The override layer sits at the store read boundary. Evaluation, prerequisite and segment // resolution, and the all-flags read go through the overlay, so an override entry wins over - // LaunchDarkly data for its key. The data system never sees the layer. The source is created - // here so that an invalid configuration fails client construction. + // LaunchDarkly data for its key. The data system never sees the layer. let readStore: ReadStore = featureStore; let overrides: OverrideComponents | undefined; - if (dataSystem.overrides !== undefined && !config.offline) { + if (overrideSource) { const layer = new OverrideLayer(); readStore = new ReadStoreOverlay(featureStore, layer); overrides = { layer, sink: new OverrideSink(layer, featureStore, onUpdate, hasEventListeners, logger), - source: createOverrideSource(dataSystem.overrides, clientContext), + source: overrideSource, }; } @@ -885,9 +890,14 @@ export default class LDClientImpl implements LDClient { // Initialization failed before waitForInitialization was called, so we have completed // and there was no promise. So we make a rejected promise and return it. if (this._initState === InitState.Failed) { - // Already failed, no need to timeout. this._initializedPromise = Promise.reject(this._rejectionReason); - return this._initializedPromise; + // Already failed, so only the override source's initial load, if it is still in progress, + // is left to wait for before the failure is reported. + return this._clientWithTimeout( + this._withOverridesLoaded(this._initializedPromise), + options?.timeout, + this._logger, + ); } if (!this._initializedPromise) { @@ -906,15 +916,20 @@ export default class LDClientImpl implements LDClient { /** * Extends the initialization promise with the override source's initial load, when that load * is still in progress. The initial load is part of starting the client, so the start handle - * that waitForInitialization returns resolves after it. The load never rejects this promise: - * a failed load is logged and the client runs without overrides. + * that waitForInitialization returns settles after it, whether initialization succeeded or + * failed: an application that handles a failed start and evaluates anyway sees the overrides + * that were present at startup. The load never rejects this promise: a failed load is logged + * and the client runs without overrides. */ private _withOverridesLoaded(initialized: Promise): Promise { const overridesReady = this._overridesReady; if (!overridesReady) { return initialized; } - return Promise.all([initialized, overridesReady]).then(() => this); + // A rejection that arrives while the load is still in progress is handled here, so that it + // is not reported as unhandled before the returned promise adopts it. + initialized.catch(() => {}); + return overridesReady.then(() => initialized); } variation( @@ -1533,6 +1548,14 @@ export default class LDClientImpl implements LDClient { typeChecker?: (value: any) => [boolean, string], ): void { if (!this.initialized()) { + if (this._overrideLayer?.get(VersionedDataKinds.Features, flagKey)) { + // The override layer holds this flag, so it is served before the not-initialized + // short-circuit and without asking the data store anything: with a persistent store the + // initialization check below is I/O, and an override must not wait on it. A flag the + // layer does not hold still gets the not-ready default when the store has no data. + this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); + return; + } this._featureStore.initialized((storeInitialized) => { if (storeInitialized) { if (!this._lastKnownValuesWarningLogged) { @@ -1546,13 +1569,6 @@ export default class LDClientImpl implements LDClient { this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); return; } - if (this._overrideLayer?.get(VersionedDataKinds.Features, flagKey)) { - // No LaunchDarkly data is available, but the override layer holds this flag. The - // override is served before the not-initialized short-circuit. A flag the layer does - // not hold still gets the not-ready default. - this._variationInternal(flagKey, context, defaultValue, eventFactory, cb, typeChecker); - return; - } this._logger?.warn( 'Variation called before LaunchDarkly client initialization completed (did you wait for the' + "'ready' event?) - using default value", diff --git a/packages/shared/sdk-server/src/overrides/OverrideLayer.ts b/packages/shared/sdk-server/src/overrides/OverrideLayer.ts index 85936aa9be..74ac9e590b 100644 --- a/packages/shared/sdk-server/src/overrides/OverrideLayer.ts +++ b/packages/shared/sdk-server/src/overrides/OverrideLayer.ts @@ -52,10 +52,19 @@ function prepareSegment(item: LDKeyedFeatureStoreItem): LDFeatureStoreItem { return segment; } +/** + * The maps of a layer have no prototype, so a lookup of a key that the layer does not hold is + * undefined even for a key such as "constructor" or "toString", and a key of "__proto__" is an + * ordinary entry. + */ +function emptyKindContents(): LayerKindContents { + return Object.create(null); +} + function emptyContents(): LayerContents { return { - [VersionedDataKinds.Features.namespace]: {}, - [VersionedDataKinds.Segments.namespace]: {}, + [VersionedDataKinds.Features.namespace]: emptyKindContents(), + [VersionedDataKinds.Segments.namespace]: emptyKindContents(), }; } @@ -113,7 +122,8 @@ export default class OverrideLayer { * Returns the entries of the given kind, keyed by item key. */ all(kind: DataKind): LDFeatureStoreKindData { - return { ...this._contents[kind.namespace] }; + // Copied into a map without a prototype, so that an entry keyed "__proto__" stays an entry. + return Object.assign(Object.create(null), this._contents[kind.namespace]); } /** diff --git a/packages/shared/sdk-server/src/overrides/OverrideSink.ts b/packages/shared/sdk-server/src/overrides/OverrideSink.ts index 5ad40410db..7f2f59c92a 100644 --- a/packages/shared/sdk-server/src/overrides/OverrideSink.ts +++ b/packages/shared/sdk-server/src/overrides/OverrideSink.ts @@ -2,6 +2,7 @@ import { fastDeepEqual, LDLogger } from '@launchdarkly/js-sdk-common'; import { LDFeatureStoreDataStorage, + LDFeatureStoreItem, LDFeatureStoreKindData, LDKeyedFeatureStoreItem, LDOverrideSink, @@ -30,6 +31,17 @@ type MergedView = LDFeatureStoreDataStorage; * by deep equality of the prepared definitions, because the layer is rebuilt on every snapshot, * so a reference comparison would report every retained entry as changed. */ +function sameEntry(a: LDFeatureStoreItem, b: LDFeatureStoreItem): boolean { + try { + return fastDeepEqual(a, b); + } catch { + // The comparison can throw for a definition whose data has an own property named like a + // method of Object, for example a variation value with a "valueOf" key. Treat such an entry + // as changed rather than lose the notifications of the whole snapshot. + return false; + } +} + function diffContents( previous: LayerContents, current: LayerContents, @@ -41,7 +53,7 @@ function diffContents( const newItems = current[namespace] ?? {}; Object.entries(oldItems).forEach(([key, oldEntry]) => { const newEntry = newItems[key]; - if (!newEntry || !fastDeepEqual(oldEntry, newEntry)) { + if (!newEntry || !sameEntry(oldEntry, newEntry)) { seeds.set(namespace, key, true); count += 1; } @@ -59,7 +71,8 @@ function diffContents( function mergedView(base: MergedView, contents: LayerContents): MergedView { const view: MergedView = {}; diffNamespaces.forEach((namespace) => { - const items: LDFeatureStoreKindData = { ...base[namespace] }; + // A map without a prototype, so that an entry keyed "__proto__" is an ordinary entry. + const items: LDFeatureStoreKindData = Object.assign(Object.create(null), base[namespace]); Object.entries(contents[namespace] ?? {}).forEach(([key, entry]) => { items[key] = entry; }); diff --git a/packages/shared/sdk-server/src/overrides/ReadStoreOverlay.ts b/packages/shared/sdk-server/src/overrides/ReadStoreOverlay.ts index f6d3723e11..ff755e2b86 100644 --- a/packages/shared/sdk-server/src/overrides/ReadStoreOverlay.ts +++ b/packages/shared/sdk-server/src/overrides/ReadStoreOverlay.ts @@ -47,7 +47,8 @@ export default class ReadStoreOverlay implements ReadStore { return; } this._base.all(kind, (baseItems) => { - callback({ ...baseItems, ...this._layer.all(kind) }); + // A map without a prototype, so that an item keyed "__proto__" is an ordinary item. + callback(Object.assign(Object.create(null), baseItems, this._layer.all(kind))); }); } diff --git a/packages/shared/sdk-server/src/overrides/validateDefinition.ts b/packages/shared/sdk-server/src/overrides/validateDefinition.ts index 9a1ea46811..e183ee41a8 100644 --- a/packages/shared/sdk-server/src/overrides/validateDefinition.ts +++ b/packages/shared/sdk-server/src/overrides/validateDefinition.ts @@ -9,13 +9,15 @@ * the shape here, before a definition is prepared and stored. * * The checks cover the fields that evaluation and the preparation of a definition read with an - * assumed type: a list is an array, an entry of a list is an object, and an attribute name that - * becomes an attribute reference is a string. A field is required only where evaluation reads it - * without a guard: the variations of a flag and the values of a target or clause. Every other - * field may be absent, as it may be in LaunchDarkly data, and a null field counts as absent - * because the preparation removes null fields. The checks go no further. The content of a field, - * such as a variation index or an operator name, is evaluation's concern, and evaluation reports - * a malformed-flag result for it. + * assumed type: a key is a string, the on switch of a flag is a boolean, a list is an array, an + * entry of a list is an object, and an attribute name that becomes an attribute reference is a + * string. A field is required only where the layer or evaluation reads it without a guard: the + * key, the variations of a flag, and the values of a target or clause. Every other field may be + * absent, as it may be in LaunchDarkly data, and a null field counts as absent because the + * preparation removes null fields. The fields that the preparation itself sets, such as the sets + * it generates for large target lists, cannot be supplied. The checks go no further. The content + * of a field, such as a variation index or an operator name, is evaluation's concern, and + * evaluation reports a malformed-flag result for it. */ import { isNullish, TypeValidators } from '@launchdarkly/js-sdk-common'; @@ -90,15 +92,45 @@ class Location { } /** - * Checks that an optional field is a string when it is present. + * Checks that a field is a string. An optional field may be absent. */ - string(parent: Definition, field: string): void { + string(parent: Definition, field: string, required: boolean = false): void { const value = parent[field]; - if (!isNullish(value) && typeof value !== 'string') { + if (isNullish(value)) { + if (required) { + this.fail(`"${field}" must be a string`); + } + return; + } + if (typeof value !== 'string') { this.fail(`"${field}" must be a string`); } } + /** + * Checks that an optional field is a boolean when it is present. + */ + boolean(parent: Definition, field: string): void { + const value = parent[field]; + if (!isNullish(value) && typeof value !== 'boolean') { + this.fail(`"${field}" must be a boolean`); + } + } + + /** + * Rejects fields that the SDK itself sets when it prepares a definition for evaluation: the + * sets it generates for large target lists and the attribute references it compiles. A + * supplied value for one of them is not what evaluation expects, and the preparation does not + * always replace it, so a definition that carries one is rejected. + */ + noInternalFields(parent: Definition, ...names: string[]): void { + Object.keys(parent).forEach((field) => { + if (field.startsWith('generated_') || names.includes(field)) { + this.fail(`"${field}" is set by the SDK and cannot be supplied`); + } + }); + } + /** * Checks that an optional field is an array of objects when it is present, and runs a check on * each entry at the entry's location. The label names an entry in a message, for example @@ -124,6 +156,7 @@ class Location { */ function checkTargets(parent: Definition, at: Location, field: string, label: string): void { at.entries(parent, field, label, (target, targetAt) => { + targetAt.noInternalFields(target); targetAt.array(target, 'values', true); }); } @@ -134,6 +167,7 @@ function checkTargets(parent: Definition, at: Location, field: string, label: st */ function checkClauses(rule: Definition, at: Location): void { at.entries(rule, 'clauses', 'clause', (clause, clauseAt) => { + clauseAt.noInternalFields(clause); clauseAt.array(clause, 'values', true); clauseAt.string(clause, 'attribute'); }); @@ -147,6 +181,7 @@ function checkRollout(parent: Definition, at: Location): void { const rollout = at.object(parent, 'rollout'); if (rollout) { const rolloutAt = at.at('rollout'); + rolloutAt.noInternalFields(rollout, 'bucketByAttributeReference'); rolloutAt.string(rollout, 'bucketBy'); rolloutAt.entries(rollout, 'variations', 'variation'); } @@ -160,6 +195,9 @@ function checkRollout(parent: Definition, at: Location): void { */ export function validateFlag(flag: Definition): void { const at = new Location('flag', `${flag.key}`); + at.string(flag, 'key', true); + at.noInternalFields(flag); + at.boolean(flag, 'on'); at.array(flag, 'variations', true); at.entries(flag, 'prerequisites', 'prerequisite'); checkTargets(flag, at, 'targets', 'target'); @@ -182,11 +220,14 @@ export function validateFlag(flag: Definition): void { */ export function validateSegment(segment: Definition): void { const at = new Location('segment', `${segment.key}`); + at.string(segment, 'key', true); + at.noInternalFields(segment); at.array(segment, 'included'); at.array(segment, 'excluded'); checkTargets(segment, at, 'includedContexts', 'included context'); checkTargets(segment, at, 'excludedContexts', 'excluded context'); at.entries(segment, 'rules', 'rule', (rule, ruleAt) => { + ruleAt.noInternalFields(rule, 'bucketByAttributeReference'); checkClauses(rule, ruleAt); ruleAt.string(rule, 'bucketBy'); });