diff --git a/packages/shared/sdk-server/__tests__/LDClientImpl.overrideDependents.test.ts b/packages/shared/sdk-server/__tests__/LDClientImpl.overrideDependents.test.ts new file mode 100644 index 0000000000..5304ebf042 --- /dev/null +++ b/packages/shared/sdk-server/__tests__/LDClientImpl.overrideDependents.test.ts @@ -0,0 +1,134 @@ +import { AsyncQueue } from 'launchdarkly-js-test-helpers'; + +import { sleep } from '@launchdarkly/js-sdk-common'; + +import LDClientImpl from '../src/LDClientImpl'; +import { createBasicPlatform } from './createBasicPlatform'; +import { makeCallbacks, makeFDv2Client } from './overrides/overridesTestSupport'; +import TestOverrideSource from './overrides/TestOverrideSource'; + +const user = { key: 'user-key' }; + +// A full FDv2 payload with one LaunchDarkly flag and one segment. The version parameter moves the +// payload state forward so that a later poll is a new data set rather than a repeat. +function payload(version: number, segmentVersion: number): string { + const events: any[] = [ + { + event: 'server-intent', + data: { payloads: [{ intentCode: 'xfer-full', id: 'test-payload', target: version }] }, + }, + { + event: 'put-object', + data: { + kind: 'flag', + key: 'ld-flag', + version: 1, + object: { + key: 'ld-flag', + version: 1, + on: true, + fallthrough: { variation: 0 }, + variations: [true, false], + }, + }, + }, + { + event: 'put-object', + data: { + kind: 'segment', + key: 'segment1', + version: segmentVersion, + object: { key: 'segment1', version: segmentVersion, included: [user.key] }, + }, + }, + { event: 'payload-transferred', data: { state: `test-state-${version}`, version } }, + ]; + return JSON.stringify({ events }); +} + +// An override flag whose only rule matches a LaunchDarkly segment. +function overriddenFlagMatching(segmentKey: string): any { + return { + key: 'overridden', + version: 1, + on: true, + fallthrough: { variation: 1 }, + variations: [true, false], + rules: [ + { + id: 'rule', + variation: 0, + clauses: [{ attribute: '', op: 'segmentMatch', values: [segmentKey], negate: false }], + }, + ], + }; +} + +describe('given an initialized FDv2 client with an override that references a LaunchDarkly segment', () => { + let body: string; + let client: LDClientImpl; + let queue: AsyncQueue; + + beforeEach(async () => { + body = payload(1, 1); + const platform = createBasicPlatform(); + platform.requests.fetch = jest.fn(() => + Promise.resolve({ status: 200, headers: new Headers(), text: async () => body }), + ) as any; + const source = new TestOverrideSource({ flags: [overriddenFlagMatching('segment1')] }); + queue = new AsyncQueue(); + client = makeFDv2Client( + platform, + { + dataSystem: { + overrides: source, + // The poll interval is in seconds; the test waits for the second poll. + dataSource: { + dataSourceOptionsType: 'custom', + initializers: [], + synchronizers: [{ type: 'polling', pollInterval: 1 }], + }, + }, + }, + makeCallbacks((key) => queue.add(key), true), + ); + await client.waitForInitialization({ timeout: 10 }); + }); + + afterEach(() => { + client.close(); + }); + + // Takes every notification that has arrived, waiting briefly for stragglers, so that a later + // assertion sees only notifications caused by what the test did next. + async function drain(): Promise { + const keys: string[] = []; + for (;;) { + while (!queue.isEmpty()) { + // eslint-disable-next-line no-await-in-loop + keys.push(await queue.take()); + } + // eslint-disable-next-line no-await-in-loop + await sleep(100); + if (queue.isEmpty()) { + return keys; + } + } + } + + it('notifies the overridden flag when the segment changes on the LaunchDarkly side', async () => { + // Startup notifies the override (from the override source) and the LaunchDarkly flag, and the + // segment in the initial data set reaches the override as well. + const initial = await drain(); + expect(initial).toContain('ld-flag'); + expect(initial).toContain('overridden'); + expect(await client.boolVariation('overridden', user, false)).toBe(true); + + // The next poll delivers a new data set in which only the segment changed. The LaunchDarkly + // flag is unchanged, so the override is the only flag notified. + body = payload(2, 2); + expect(await queue.take()).toEqual('overridden'); + await sleep(100); + expect(queue.isEmpty()).toBeTruthy(); + }); +}); diff --git a/packages/shared/sdk-server/__tests__/data_sources/DataSourceUpdates.test.ts b/packages/shared/sdk-server/__tests__/data_sources/DataSourceUpdates.test.ts index 414d339448..adc36c63fb 100644 --- a/packages/shared/sdk-server/__tests__/data_sources/DataSourceUpdates.test.ts +++ b/packages/shared/sdk-server/__tests__/data_sources/DataSourceUpdates.test.ts @@ -5,6 +5,7 @@ import { internal } from '@launchdarkly/js-sdk-common'; import { LDTransactionalFeatureStore } from '../../src/api/subsystems'; import promisify from '../../src/async/promisify'; import DataSourceUpdates from '../../src/data_sources/TransactionalDataSourceUpdates'; +import { OverrideLayer } from '../../src/overrides'; import InMemoryFeatureStore from '../../src/store/InMemoryFeatureStore'; import VersionedDataKinds from '../../src/store/VersionedDataKinds'; @@ -213,3 +214,120 @@ describe.each([true, false])( }); }, ); + +describe('given a DataSourceUpdates whose external dependencies come from an override layer', () => { + let store: LDTransactionalFeatureStore; + let layer: OverrideLayer; + let updates: DataSourceUpdates; + let queue: AsyncQueue; + + function overriddenFlagMatching(key: string, segmentKey: string): any { + return { + key, + version: 1, + on: true, + fallthrough: { variation: 0 }, + variations: [true, false], + rules: [ + { + id: 'rule', + variation: 0, + clauses: [{ attribute: '', op: 'segmentMatch', values: [segmentKey], negate: false }], + }, + ], + }; + } + + async function takeSorted(count: number): Promise { + const keys: string[] = []; + for (let i = 0; i < count; i += 1) { + // eslint-disable-next-line no-await-in-loop + keys.push(await queue.take()); + } + return keys.sort(); + } + + beforeEach(() => { + queue = new AsyncQueue(); + store = new InMemoryFeatureStore(); + layer = new OverrideLayer(); + updates = new DataSourceUpdates( + store, + () => true, + (key) => queue.add(key), + layer, + ); + }); + + it('notifies an overridden flag when the LaunchDarkly segment it references changes', async () => { + layer.setAll([overriddenFlagMatching('overridden', 'segment1')], []); + await promisify((cb) => { + updates.init( + { + features: { ld: { key: 'ld', version: 1 } }, + segments: { segment1: { key: 'segment1', version: 1 } }, + }, + () => cb(undefined), + ); + }); + // The initial data set counts every item as modified; the segment reaches the override. + expect(await takeSorted(2)).toEqual(['ld', 'overridden']); + + await promisify((cb) => { + updates.upsert(VersionedDataKinds.Segments, { key: 'segment1', version: 2 }, () => + cb(undefined), + ); + }); + expect(await queue.take()).toEqual('overridden'); + expect(queue.isEmpty()).toBeTruthy(); + }); + + it('follows a chain that crosses from LaunchDarkly data into the override layer and back', async () => { + // parent is a LaunchDarkly flag whose prerequisite is overridden; overridden references the + // LaunchDarkly segment. A change to the segment reaches parent through the override entry. + layer.setAll([overriddenFlagMatching('overridden', 'segment1')], []); + await promisify((cb) => { + updates.init( + { + features: { + parent: { + key: 'parent', + version: 1, + prerequisites: [{ key: 'overridden', variation: 0 }], + }, + }, + segments: { segment1: { key: 'segment1', version: 1 } }, + }, + () => cb(undefined), + ); + }); + expect(await takeSorted(2)).toEqual(['overridden', 'parent']); + + await promisify((cb) => { + updates.upsert(VersionedDataKinds.Segments, { key: 'segment1', version: 2 }, () => + cb(undefined), + ); + }); + expect(await takeSorted(2)).toEqual(['overridden', 'parent']); + expect(queue.isEmpty()).toBeTruthy(); + }); + + it('does not notify an overridden flag for an item it does not reference', async () => { + layer.setAll([overriddenFlagMatching('overridden', 'segment1')], []); + await promisify((cb) => { + updates.init({ features: {}, segments: { segment2: { key: 'segment2', version: 1 } } }, () => + cb(undefined), + ); + }); + await promisify((cb) => { + updates.upsert(VersionedDataKinds.Segments, { key: 'segment2', version: 2 }, () => + cb(undefined), + ); + }); + await promisify((cb) => { + updates.upsert(VersionedDataKinds.Features, { key: 'ld', version: 1 }, () => cb(undefined)); + }); + expect(await queue.take()).toEqual('ld'); + expect(queue.isEmpty()).toBeTruthy(); + }); +}); diff --git a/packages/shared/sdk-server/__tests__/data_sources/NamespacedDataSet.test.ts b/packages/shared/sdk-server/__tests__/data_sources/NamespacedDataSet.test.ts new file mode 100644 index 0000000000..82cf89c834 --- /dev/null +++ b/packages/shared/sdk-server/__tests__/data_sources/NamespacedDataSet.test.ts @@ -0,0 +1,38 @@ +import DependencyTracker from '../../src/data_sources/DependencyTracker'; +import NamespacedDataSet from '../../src/data_sources/NamespacedDataSet'; + +describe('given keys that are the names of properties of Object', () => { + const keys = ['constructor', 'toString', 'hasOwnProperty', '__proto__', 'valueOf']; + + it('stores, finds, enumerates, and removes such entries like any other', () => { + const set = new NamespacedDataSet(); + keys.forEach((key, index) => set.set('features', key, index)); + keys.forEach((key, index) => expect(set.get('features', key)).toBe(index)); + expect(set.get('features', 'missing')).toBeUndefined(); + expect(set.get('constructor', 'constructor')).toBeUndefined(); + + const seen: string[] = []; + set.enumerate((_namespace, key) => seen.push(key)); + expect(seen.sort()).toEqual([...keys].sort()); + + set.remove('features', '__proto__'); + expect(set.get('features', '__proto__')).toBeUndefined(); + expect(set.get('features', 'constructor')).toBe(0); + }); + + it('tracks dependencies between items with such keys', () => { + // Before the maps lost their prototype, looking up "constructor" returned Object's + // constructor function and the tracker threw while updating. + const tracker = new DependencyTracker(); + const deps = new NamespacedDataSet(); + deps.set('segments', '__proto__', true); + tracker.updateDependenciesFrom('features', 'constructor', deps); + tracker.updateDependenciesFrom('features', 'constructor', deps); + + const affected = new NamespacedDataSet(); + tracker.updateModifiedItems(affected, 'segments', '__proto__'); + const items: string[] = []; + affected.enumerate((namespace, key) => items.push(`${namespace}/${key}`)); + expect(items.sort()).toEqual(['features/constructor', 'segments/__proto__']); + }); +}); diff --git a/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts b/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts index 093de3e116..262c8519ff 100644 --- a/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts +++ b/packages/shared/sdk-server/__tests__/overrides/OverrideLayer.test.ts @@ -2,6 +2,7 @@ import { AttributeReference } from '@launchdarkly/js-sdk-common'; import { makeFlagWithValue } from '../../src/data_sources/FileDataSource'; +import NamespacedDataSet from '../../src/data_sources/NamespacedDataSet'; import { Flag } from '../../src/evaluation/data/Flag'; import { Segment } from '../../src/evaluation/data/Segment'; import TestData from '../../src/integrations/test_data/TestData'; @@ -485,3 +486,57 @@ describe('given keys that are the names of properties of Object', () => { expect((layer.get(features, 'flag1') as Flag).on).toBe(true); }); }); + +describe('given a layer whose entries reference other definitions', () => { + const features = VersionedDataKinds.Features.namespace; + const segments = VersionedDataKinds.Segments.namespace; + + function flagReferencing(key: string, segmentKey: string, prerequisites: string[] = []): any { + return { + key, + version: 1, + on: true, + fallthrough: { variation: 0 }, + variations: [true, false], + prerequisites: prerequisites.map((prerequisite) => ({ key: prerequisite, variation: 0 })), + rules: [ + { + id: 'rule', + variation: 0, + clauses: [{ attribute: '', op: 'segmentMatch', values: [segmentKey], negate: false }], + }, + ], + }; + } + + function reachedFrom(layer: OverrideLayer, namespace: string, key: string): string[] { + const set = new NamespacedDataSet(); + layer.updateModifiedItems(set, namespace, key); + const items: string[] = []; + set.enumerate((itemNamespace, itemKey) => items.push(`${itemNamespace}/${itemKey}`)); + return items.sort(); + } + + it('reports the entries that depend on a segment or a prerequisite, transitively', () => { + const layer = new OverrideLayer(); + layer.setAll([flagReferencing('a', 'seg'), flagReferencing('b', 'other', ['a'])], []); + + // b does not reference seg itself; it depends on a, which does. + expect(reachedFrom(layer, segments, 'seg')).toEqual([ + 'features/a', + 'features/b', + 'segments/seg', + ]); + expect(reachedFrom(layer, features, 'a')).toEqual(['features/a', 'features/b']); + expect(reachedFrom(layer, segments, 'unreferenced')).toEqual(['segments/unreferenced']); + }); + + it('forgets the edges of the entries that a replacement removed', () => { + const layer = new OverrideLayer(); + layer.setAll([flagReferencing('a', 'seg')], []); + layer.setAll([flagReferencing('c', 'seg2')], []); + + expect(reachedFrom(layer, segments, 'seg')).toEqual(['segments/seg']); + expect(reachedFrom(layer, segments, 'seg2')).toEqual(['features/c', 'segments/seg2']); + }); +}); diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 961813aa5d..6f773ce53d 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -341,23 +341,27 @@ function constructFDv2( : undefined; const featureStore = dataSystem.featureStoreFactory(clientContext); + // 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's entries. It only asks + // the layer which entries depend on a LaunchDarkly item that changed, so that those flags are + // notified like the LaunchDarkly flags that depend on it. + const overrideLayer = overrideSource ? new OverrideLayer() : undefined; + const dataSourceUpdates = new TransactionalDataSourceUpdates( featureStore, hasEventListeners, onUpdate, + overrideLayer, ); - // 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. let readStore: ReadStore = featureStore; let overrides: OverrideComponents | undefined; - if (overrideSource) { - const layer = new OverrideLayer(); - readStore = new ReadStoreOverlay(featureStore, layer); + if (overrideSource && overrideLayer) { + readStore = new ReadStoreOverlay(featureStore, overrideLayer); overrides = { - layer, - sink: new OverrideSink(layer, featureStore, onUpdate, hasEventListeners, logger), + layer: overrideLayer, + sink: new OverrideSink(overrideLayer, featureStore, onUpdate, hasEventListeners, logger), source: overrideSource, }; } diff --git a/packages/shared/sdk-server/src/data_sources/NamespacedDataSet.ts b/packages/shared/sdk-server/src/data_sources/NamespacedDataSet.ts index 1510579b41..118eff5255 100644 --- a/packages/shared/sdk-server/src/data_sources/NamespacedDataSet.ts +++ b/packages/shared/sdk-server/src/data_sources/NamespacedDataSet.ts @@ -2,7 +2,9 @@ * @internal */ export default class NamespacedDataSet { - private _itemsByNamespace: Record> = {}; + // The maps have no prototype, so a namespace or key named like a property of Object (for + // example a flag key of "constructor" or "__proto__") is an ordinary entry. + private _itemsByNamespace: Record> = Object.create(null); get(namespace: string, key: string): T | undefined { return this._itemsByNamespace[namespace]?.[key]; @@ -10,7 +12,7 @@ export default class NamespacedDataSet { set(namespace: string, key: string, value: T) { if (!(namespace in this._itemsByNamespace)) { - this._itemsByNamespace[namespace] = {}; + this._itemsByNamespace[namespace] = Object.create(null); } this._itemsByNamespace[namespace][key] = value; } @@ -23,7 +25,7 @@ export default class NamespacedDataSet { } removeAll() { - this._itemsByNamespace = {}; + this._itemsByNamespace = Object.create(null); } enumerate(callback: (namespace: string, key: string, value: T) => void) { diff --git a/packages/shared/sdk-server/src/data_sources/TransactionalDataSourceUpdates.ts b/packages/shared/sdk-server/src/data_sources/TransactionalDataSourceUpdates.ts index 6f2d85d44c..07a86b710a 100644 --- a/packages/shared/sdk-server/src/data_sources/TransactionalDataSourceUpdates.ts +++ b/packages/shared/sdk-server/src/data_sources/TransactionalDataSourceUpdates.ts @@ -13,16 +13,39 @@ import { computeDependencies } from './DataSourceUpdates'; import DependencyTracker from './DependencyTracker'; import NamespacedDataSet from './NamespacedDataSet'; +/** + * Dependency edges that live outside the LaunchDarkly data, such as those of the override layer's + * entries. Given an item, adds it and everything that depends on it through those edges. + * + * @internal + */ +export interface ExternalDependencies { + updateModifiedItems( + inDependencySet: NamespacedDataSet, + namespace: string, + key: string, + ): void; +} + /** * @internal */ export default class TransactionalDataSourceUpdates implements LDTransactionalDataSourceUpdates { private readonly _dependencyTracker = new DependencyTracker(); + /** + * @param _featureStore The store the data is written to. + * @param _hasEventListeners Reports whether anything listens for flag changes. + * @param _onChange Receives the key of each flag whose evaluation may have changed. + * @param _externalDependencies Dependencies of definitions the data system does not hold. The + * override layer supplies its entries' dependencies, so that a change to a LaunchDarkly item + * also notifies the overridden flags that reference it. + */ constructor( private readonly _featureStore: LDTransactionalFeatureStore, private readonly _hasEventListeners: () => boolean, private readonly _onChange: (key: string) => void, + private readonly _externalDependencies?: ExternalDependencies, ) {} init( @@ -135,7 +158,39 @@ export default class TransactionalDataSourceUpdates implements LDTransactionalDa if (newValue && oldValue && newValue.version <= oldValue.version) { return; } - this._dependencyTracker.updateModifiedItems(toDataSet, namespace, key); + this._addAffectedItems(toDataSet, namespace, key); + } + + /** + * Adds the modified item and everything that depends on it to the set. The LaunchDarkly data + * and the external dependencies each know their own edges, and a chain can cross between them + * in either direction: an overridden flag can reference a LaunchDarkly segment, and a + * LaunchDarkly flag can have an overridden prerequisite. Each newly reached item is therefore + * expanded through both until nothing new is found. + */ + private _addAffectedItems(toDataSet: NamespacedDataSet, namespace: string, key: string) { + if (!this._externalDependencies) { + this._dependencyTracker.updateModifiedItems(toDataSet, namespace, key); + return; + } + const pending: [string, string][] = [[namespace, key]]; + while (pending.length > 0) { + const [pendingNamespace, pendingKey] = pending.pop()!; + // Each tracker gets its own set: a tracker skips an item that is already in the set it is + // given, so sharing one set would stop the second tracker at the starting item. + const viaData = new NamespacedDataSet(); + this._dependencyTracker.updateModifiedItems(viaData, pendingNamespace, pendingKey); + const viaExternal = new NamespacedDataSet(); + this._externalDependencies.updateModifiedItems(viaExternal, pendingNamespace, pendingKey); + [viaData, viaExternal].forEach((reached) => { + reached.enumerate((reachedNamespace, reachedKey) => { + if (!toDataSet.get(reachedNamespace, reachedKey)) { + toDataSet.set(reachedNamespace, reachedKey, true); + pending.push([reachedNamespace, reachedKey]); + } + }); + }); + } } sendChangeEvents(dataSet: NamespacedDataSet) { diff --git a/packages/shared/sdk-server/src/overrides/OverrideLayer.ts b/packages/shared/sdk-server/src/overrides/OverrideLayer.ts index 74ac9e590b..8cab3a0be2 100644 --- a/packages/shared/sdk-server/src/overrides/OverrideLayer.ts +++ b/packages/shared/sdk-server/src/overrides/OverrideLayer.ts @@ -6,6 +6,9 @@ import { LDFeatureStoreKindData, LDKeyedFeatureStoreItem, } from '../api/subsystems'; +import { computeDependencies } from '../data_sources/DataSourceUpdates'; +import DependencyTracker from '../data_sources/DependencyTracker'; +import NamespacedDataSet from '../data_sources/NamespacedDataSet'; import { Flag } from '../evaluation/data/Flag'; import { markOverrideEntry } from '../evaluation/data/overrideMarker'; import { Segment } from '../evaluation/data/Segment'; @@ -78,6 +81,10 @@ function emptyContents(): LayerContents { export default class OverrideLayer { private _contents: LayerContents = emptyContents(); + // The dependency edges of the layer's own entries: which entries reference which flags and + // segments. Rebuilt with the contents on every replacement. + private _dependencies = new DependencyTracker(); + private _empty = true; /** @@ -102,12 +109,33 @@ export default class OverrideLayer { segments.forEach((segment) => { current[VersionedDataKinds.Segments.namespace][segment.key] = prepareSegment(segment); }); + const dependencies = new DependencyTracker(); + Object.entries(current).forEach(([namespace, items]) => { + Object.entries(items).forEach(([key, item]) => { + dependencies.updateDependenciesFrom(namespace, key, computeDependencies(namespace, item)); + }); + }); const previous = this._contents; this._contents = current; + this._dependencies = dependencies; this._empty = flags.length === 0 && segments.length === 0; return { previous, current }; } + /** + * Adds the given item to the set together with every item whose override entry depends on + * it, directly or through other override entries. A LaunchDarkly update uses this so that a + * change to a LaunchDarkly segment or prerequisite notifies the overridden flags that reference + * it, which the data system's own dependency tracking cannot see. + */ + updateModifiedItems( + inDependencySet: NamespacedDataSet, + namespace: string, + key: string, + ): void { + this._dependencies.updateModifiedItems(inDependencySet, namespace, key); + } + /** * Returns the override entry for a key, if any. */