From 34e1971102416734cefcc577455ce6b32d96ec7a Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 18 Mar 2024 15:57:46 -0700 Subject: [PATCH 01/10] feat: Implement support for hooks. --- contract-tests/TestHook.js | 51 ++ contract-tests/index.js | 3 +- contract-tests/sdkClientEntity.js | 6 + .../__tests__/LDClient.hooks.test.ts | 603 ++++++++++++++++++ .../__tests__/options/Configuration.test.ts | 15 + .../shared/sdk-server/src/LDClientImpl.ts | 434 +++++++++---- .../shared/sdk-server/src/api/LDClient.ts | 11 + packages/shared/sdk-server/src/api/index.ts | 3 + .../sdk-server/src/api/integrations/Hook.ts | 80 +++ .../sdk-server/src/api/integrations/index.ts | 1 + .../sdk-server/src/api/options/LDOptions.ts | 20 + .../sdk-server/src/integrations/index.ts | 3 + .../sdk-server/src/options/Configuration.ts | 6 + .../src/options/ValidatedOptions.ts | 2 + 14 files changed, 1122 insertions(+), 116 deletions(-) create mode 100644 contract-tests/TestHook.js create mode 100644 packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts create mode 100644 packages/shared/sdk-server/src/api/integrations/Hook.ts diff --git a/contract-tests/TestHook.js b/contract-tests/TestHook.js new file mode 100644 index 0000000000..d0f147ca55 --- /dev/null +++ b/contract-tests/TestHook.js @@ -0,0 +1,51 @@ +import got from 'got'; + +export default class TestHook { + constructor(name, endpoint, data) { + this._name = name; + this._endpoint = endpoint; + this._data = data; + } + + async _safePost(body) { + try { + await got.post(this._endpoint, { json: body }); + } catch { + // The test could move on before the post, so we are ignoring + // failed posts. + } + } + + getMetadata() { + return { + name: 'LaunchDarkly Tracing Hook', + }; + } + + beforeEvaluation( + hookContext, + data, + ) { + this._safePost({ + evaluationHookContext: hookContext, + evaluationHookData: data, + stage: 'beforeEvaluation', + }); + return {...data, ...(this._data?.['beforeEvaluation'] || {})}; + } + + afterEvaluation( + hookContext, + data, + detail, + ) { + this._safePost({ + evaluationHookContext: hookContext, + evaluationHookData: data, + stage: 'afterEvaluation', + evaluationDetail: detail, + }); + + return {...data, ...(this._data?.['afterEvaluation'] || {})}; + } +} diff --git a/contract-tests/index.js b/contract-tests/index.js index 4891ff3934..6422e6b1c6 100644 --- a/contract-tests/index.js +++ b/contract-tests/index.js @@ -32,8 +32,7 @@ app.get('/', (req, res) => { 'event-sampling', 'strongly-typed', 'polling-gzip', - 'inline-context', - 'anonymous-redaction', + 'evaluation-hooks' ], }); }); diff --git a/contract-tests/sdkClientEntity.js b/contract-tests/sdkClientEntity.js index c9a1556f31..d888d8f1b0 100644 --- a/contract-tests/sdkClientEntity.js +++ b/contract-tests/sdkClientEntity.js @@ -10,6 +10,7 @@ import ld, { import BigSegmentTestStore from './BigSegmentTestStore.js'; import { Log, sdkLogger } from './log.js'; +import TestHook from './TestHook.js'; const badCommandError = new Error('unsupported command'); export { badCommandError }; @@ -19,6 +20,7 @@ export function makeSdkConfig(options, tag) { logger: sdkLogger(tag), diagnosticOptOut: true, }; + const maybeTime = (seconds) => seconds === undefined || seconds === null ? undefined : seconds / 1000; if (options.streaming) { @@ -60,6 +62,10 @@ export function makeSdkConfig(options, tag) { : undefined, }; } + if (options.hooks) { + cf.hooks = options.hooks.hooks + .map(hook => new TestHook(hook.name, hook.callbackUri, hook.data)); + } return cf; } diff --git a/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts b/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts new file mode 100644 index 0000000000..5c86e2ff8e --- /dev/null +++ b/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts @@ -0,0 +1,603 @@ +import { basicPlatform } from '@launchdarkly/private-js-mocks'; + +import { integrations, LDClientImpl, LDEvaluationDetail, LDMigrationStage } from '../src'; +import Reasons from '../src/evaluation/Reasons'; +import TestData from '../src/integrations/test_data/TestData'; +import TestLogger, { LogLevel } from './Logger'; +import makeCallbacks from './makeCallbacks'; + +const defaultUser = { kind: 'user', key: 'user-key' }; + +type EvalCapture = { + method: string; + hookContext: integrations.EvaluationHookContext; + hookData: integrations.EvaluationHookData; + detail?: LDEvaluationDetail; +}; + +class TestHook implements integrations.Hook { + captureBefore: EvalCapture[] = []; + captureAfter: EvalCapture[] = []; + + getMetadataImpl: () => integrations.HookMetadata = () => ({ name: 'LaunchDarkly Test Hook' }); + + getMetadata(): integrations.HookMetadata { + return this.getMetadataImpl(); + } + + verifyBefore( + hookContext: integrations.EvaluationHookContext, + data: integrations.EvaluationHookData, + ) { + expect(this.captureBefore).toHaveLength(1); + expect(this.captureBefore[0].hookContext).toEqual(hookContext); + expect(this.captureBefore[0].hookData).toEqual(data); + } + + verifyAfter( + hookContext: integrations.EvaluationHookContext, + data: integrations.EvaluationHookData, + detail: LDEvaluationDetail, + ) { + expect(this.captureAfter).toHaveLength(1); + expect(this.captureAfter[0].hookContext).toEqual(hookContext); + expect(this.captureAfter[0].hookData).toEqual(data); + expect(this.captureAfter[0].detail).toEqual(detail); + } + + beforeEvalImpl: ( + hookContext: integrations.EvaluationHookContext, + data: integrations.EvaluationHookData, + ) => integrations.EvaluationHookData = (_hookContext, data) => data; + + afterEvalImpl: ( + hookContext: integrations.EvaluationHookContext, + data: integrations.EvaluationHookData, + detail: LDEvaluationDetail, + ) => integrations.EvaluationHookData = (_hookContext, data, _detail) => data; + + beforeEvaluation?( + hookContext: integrations.EvaluationHookContext, + data: integrations.EvaluationHookData, + ): integrations.EvaluationHookData { + this.captureBefore.push({ method: 'beforeEvaluation', hookContext, hookData: data }); + return this.beforeEvalImpl(hookContext, data); + } + afterEvaluation?( + hookContext: integrations.EvaluationHookContext, + data: integrations.EvaluationHookData, + detail: LDEvaluationDetail, + ): integrations.EvaluationHookData { + this.captureAfter.push({ method: 'afterEvaluation', hookContext, hookData: data, detail }); + return this.afterEvalImpl(hookContext, data, detail); + } +} + +describe('given an LDClient with test data', () => { + let client: LDClientImpl; + let td: TestData; + let testHook: TestHook; + let logger: TestLogger; + + beforeEach(async () => { + logger = new TestLogger(); + testHook = new TestHook(); + td = new TestData(); + client = new LDClientImpl( + 'sdk-key', + basicPlatform, + { + updateProcessor: td.getFactory(), + sendEvents: false, + hooks: [testHook], + logger, + }, + makeCallbacks(true), + ); + + await client.waitForInitialization(); + }); + + afterEach(() => { + client.close(); + }); + + it('variation triggers before/after evaluation hooks', async () => { + td.update(td.flag('flagKey').booleanFlag().on(true)); + await client.variation('flagKey', defaultUser, false); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.variation', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.variation', + }, + {}, + { + value: true, + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); + }); + + it('variation detail triggers before/after evaluation hooks', async () => { + td.update(td.flag('flagKey').booleanFlag().on(true)); + await client.variationDetail('flagKey', defaultUser, false); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.variationDetail', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.variationDetail', + }, + {}, + { + value: true, + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); + }); + + it('boolean variation triggers before/after evaluation hooks', async () => { + td.update(td.flag('flagKey').booleanFlag().on(true)); + await client.boolVariation('flagKey', defaultUser, false); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.boolVariation', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.boolVariation', + }, + {}, + { + value: true, + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); + }); + + it('boolean variation detail triggers before/after evaluation hooks', async () => { + td.update(td.flag('flagKey').booleanFlag().on(true)); + await client.boolVariationDetail('flagKey', defaultUser, false); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.boolVariationDetail', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.boolVariationDetail', + }, + {}, + { + value: true, + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); + }); + + it('number variation triggers before/after evaluation hooks', async () => { + td.update(td.flag('flagKey').valueForAll(42).on(true)); + await client.numberVariation('flagKey', defaultUser, 21); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: 21, + method: 'LDClient.numberVariation', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: 21, + method: 'LDClient.numberVariation', + }, + {}, + { + value: 42, + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); + }); + + it('number variation detail triggers before/after evaluation hooks', async () => { + td.update(td.flag('flagKey').valueForAll(42).on(true)); + await client.numberVariationDetail('flagKey', defaultUser, 21); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: 21, + method: 'LDClient.numberVariationDetail', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: 21, + method: 'LDClient.numberVariationDetail', + }, + {}, + { + value: 42, + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); + }); + + it('string variation triggers before/after evaluation hooks', async () => { + td.update(td.flag('flagKey').valueForAll('strValue').on(true)); + await client.stringVariation('flagKey', defaultUser, 'default'); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: 'default', + method: 'LDClient.stringVariation', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: 'default', + method: 'LDClient.stringVariation', + }, + {}, + { + value: 'strValue', + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); + }); + + it('string variation detail triggers before/after evaluation hooks', async () => { + td.update(td.flag('flagKey').valueForAll('strValue').on(true)); + await client.stringVariationDetail('flagKey', defaultUser, 'default'); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: 'default', + method: 'LDClient.stringVariationDetail', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: 'default', + method: 'LDClient.stringVariationDetail', + }, + {}, + { + value: 'strValue', + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); + }); + + it('json variation triggers before/after evaluation hooks', async () => { + td.update(td.flag('flagKey').valueForAll({ the: 'value' }).on(true)); + await client.jsonVariation('flagKey', defaultUser, { default: 'value' }); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: { default: 'value' }, + method: 'LDClient.jsonVariation', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: { default: 'value' }, + method: 'LDClient.jsonVariation', + }, + {}, + { + value: { the: 'value' }, + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); + }); + + it('json variation detail triggers before/after evaluation hooks', async () => { + td.update(td.flag('flagKey').valueForAll({ the: 'value' }).on(true)); + await client.jsonVariationDetail('flagKey', defaultUser, { default: 'value' }); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: { default: 'value' }, + method: 'LDClient.jsonVariationDetail', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: { default: 'value' }, + method: 'LDClient.jsonVariationDetail', + }, + {}, + { + value: { the: 'value' }, + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); + }); + + it('migration variation triggers before/after evaluation hooks', async () => { + td.update(td.flag('flagKey').valueForAll('live')); + await client.migrationVariation('flagKey', defaultUser, LDMigrationStage.Off); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: 'off', + method: 'LDClient.migrationVariation', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: 'off', + method: 'LDClient.migrationVariation', + }, + {}, + { + value: 'live', + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); + }); + + it('propagates data between stages', async () => { + testHook.beforeEvalImpl = ( + _hookContext: integrations.EvaluationHookContext, + data: integrations.EvaluationHookData, + ) => ({ + ...data, + added: 'added data', + }); + await client.variation('flagKey', defaultUser, false); + + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.variation', + }, + { added: 'added data' }, + { + value: false, + reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' }, + variationIndex: null, + }, + ); + }); + + it('handles an exception thrown in beforeEvaluation', async () => { + testHook.beforeEvalImpl = ( + _hookContext: integrations.EvaluationHookContext, + _data: integrations.EvaluationHookData, + ) => { + throw new Error('bad hook'); + }; + await client.variation('flagKey', defaultUser, false); + logger.expectMessages([ + { + level: LogLevel.Error, + matches: + /An error was encountered in "beforeEvaluation" of the "LaunchDarkly Test Hook" hook: Error: bad hook/, + }, + ]); + }); + + it('handles an exception thrown in afterEvaluation', async () => { + testHook.afterEvalImpl = () => { + throw new Error('bad hook'); + }; + await client.variation('flagKey', defaultUser, false); + logger.expectMessages([ + { + level: LogLevel.Error, + matches: + /An error was encountered in "afterEvaluation" of the "LaunchDarkly Test Hook" hook: Error: bad hook/, + }, + ]); + }); + + it('handles exception getting the hook metadata', async () => { + testHook.getMetadataImpl = () => { + throw new Error('bad hook'); + }; + await client.variation('flagKey', defaultUser, false); + + logger.expectMessages([ + { + level: LogLevel.Error, + matches: /Exception thrown getting metadata for hook. Unable to get hook name./, + }, + ]); + }); + + it('uses unknown name when the name cannot be accessed', async () => { + testHook.beforeEvalImpl = ( + _hookContext: integrations.EvaluationHookContext, + _data: integrations.EvaluationHookData, + ) => { + throw new Error('bad hook'); + }; + testHook.getMetadataImpl = () => { + throw new Error('bad hook'); + }; + testHook.afterEvalImpl = () => { + throw new Error('bad hook'); + }; + await client.variation('flagKey', defaultUser, false); + logger.expectMessages([ + { + level: LogLevel.Error, + matches: + /An error was encountered in "afterEvaluation" of the "unknown hook" hook: Error: bad hook/, + }, + { + level: LogLevel.Error, + matches: + /An error was encountered in "beforeEvaluation" of the "unknown hook" hook: Error: bad hook/, + }, + ]); + }); +}); + +it('can add a hook after initialization', async () => { + const logger = new TestLogger(); + const td = new TestData(); + const client = new LDClientImpl( + 'sdk-key', + basicPlatform, + { + updateProcessor: td.getFactory(), + sendEvents: false, + logger, + }, + makeCallbacks(true), + ); + + await client.waitForInitialization(); + + td.update(td.flag('flagKey').booleanFlag().on(true)); + const testHook = new TestHook(); + client.addHook(testHook); + + await client.variation('flagKey', defaultUser, false); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.variation', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.variation', + }, + {}, + { + value: true, + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); +}); + +it('executes hook stages in the specified order', async () => { + const beforeCalledOrder: string[] = []; + const afterCalledOrder: string[] = []; + + const hookA = new TestHook(); + hookA.beforeEvalImpl = (_context, data) => { + beforeCalledOrder.push('a'); + return data; + }; + + hookA.afterEvalImpl = (_context, data, _detail) => { + afterCalledOrder.push('a'); + return data; + }; + + const hookB = new TestHook(); + hookB.beforeEvalImpl = (_context, data) => { + beforeCalledOrder.push('b'); + return data; + }; + hookB.afterEvalImpl = (_context, data, _detail) => { + afterCalledOrder.push('b'); + return data; + }; + + const logger = new TestLogger(); + const td = new TestData(); + const client = new LDClientImpl( + 'sdk-key', + basicPlatform, + { + updateProcessor: td.getFactory(), + sendEvents: false, + logger, + hooks: [hookA, hookB], + }, + makeCallbacks(true), + ); + + await client.waitForInitialization(); + await client.variation('flagKey', defaultUser, false); + + expect(beforeCalledOrder).toEqual(['a', 'b']); + expect(afterCalledOrder).toEqual(['b', 'a']); +}); diff --git a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts index 9f1561538b..03b6fa27dd 100644 --- a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts +++ b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts @@ -40,6 +40,7 @@ describe.each([undefined, null, 'potat0', 17, [], {}])('constructed without opti expect(config.useLdd).toBe(false); expect(config.wrapperName).toBeUndefined(); expect(config.wrapperVersion).toBeUndefined(); + expect(config.hooks).toBeUndefined(); }); }); @@ -364,4 +365,18 @@ describe('when setting different options', () => { const config = new Configuration(withLogger({ ...values })); expect(logger(config).getCount()).toEqual(warnings); }); + + // Valid usage is covered in LDClient.hooks.test.ts + test('non-array hooks should use default', () => { + // @ts-ignore + const config = new Configuration(withLogger({ hooks: 'hook!' })); + expect(config.hooks).toBeUndefined(); + logger(config).expectMessages([ + { + level: LogLevel.Warn, + matches: + /Config option "hooks" should be of type Hook\[\], got string, using default value/, + }, + ]); + }); }); diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index c0dbc6b077..ea3d28a9bb 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -24,6 +24,7 @@ import { LDMigrationVariation, LDOptions, } from './api'; +import { EvaluationHookContext, EvaluationHookData, Hook } from './api/integrations/Hook'; import { BigSegmentStoreMembership } from './api/interfaces'; import BigSegmentsManager from './BigSegmentsManager'; import BigSegmentStoreStatusProvider from './BigSegmentStatusProviderImpl'; @@ -45,7 +46,6 @@ import FlagsStateBuilder from './FlagsStateBuilder'; import MigrationOpEventToInputEvent from './MigrationOpEventConversion'; import MigrationOpTracker from './MigrationOpTracker'; import Configuration from './options/Configuration'; -import AsyncStoreFacade from './store/AsyncStoreFacade'; import VersionedDataKinds from './store/VersionedDataKinds'; const { ClientMessages, ErrorKinds, NullEventProcessor } = internal; @@ -66,6 +66,23 @@ export interface LDClientCallbacks { hasEventListeners: () => boolean; } +const BOOL_VARIATION_METHOD_NAME = 'LDClient.boolVariation'; +const NUMBER_VARIATION_METHOD_NAME = 'LDClient.numberVariation'; +const STRING_VARIATION_METHOD_NAME = 'LDClient.stringVariation'; +const JSON_VARIATION_METHOD_NAME = 'LDClient.jsonVariation'; +const VARIATION_METHOD_NAME = 'LDClient.variation'; +const MIGRATION_VARIATION_METHOD_NAME = 'LDClient.migrationVariation'; + +const BOOL_VARIATION_DETAIL_METHOD_NAME = 'LDClient.boolVariationDetail'; +const NUMBER_VARIATION_DETAIL_METHOD_NAME = 'LDClient.numberVariationDetail'; +const STRING_VARIATION_DETAIL_METHOD_NAME = 'LDClient.stringVariationDetail'; +const JSON_VARIATION_DETAIL_METHOD_NAME = 'LDClient.jsonVariationDetail'; +const VARIATION_METHOD_DETAIL_NAME = 'LDClient.variationDetail'; + +const BEFORE_EVALUATION_STAGE_NAME = 'beforeEvaluation'; +const AFTER_EVALUATION_STAGE_NAME = 'afterEvaluation'; +const UNKNOWN_HOOK_NAME = 'unknown hook'; + /** * @ignore */ @@ -74,8 +91,6 @@ export default class LDClientImpl implements LDClient { private featureStore: LDFeatureStore; - private asyncFeatureStore: AsyncStoreFacade; - private updateProcessor?: subsystem.LDStreamProcessor; private eventFactoryDefault = new EventFactory(false); @@ -108,6 +123,8 @@ export default class LDClientImpl implements LDClient { private diagnosticsManager?: internal.DiagnosticsManager; + private hooks: Hook[]; + /** * Intended for use by platform specific client implementations. * @@ -131,6 +148,8 @@ export default class LDClientImpl implements LDClient { const { onUpdate, hasEventListeners } = callbacks; const config = new Configuration(options, internalOptions); + this.hooks = config.hooks || []; + if (!sdkKey && !config.offline) { throw new Error('You must configure the client with an SDK key'); } @@ -139,7 +158,7 @@ export default class LDClientImpl implements LDClient { const clientContext = new ClientContext(sdkKey, config, platform); const featureStore = config.featureStoreFactory(clientContext); - this.asyncFeatureStore = new AsyncStoreFacade(featureStore); + const dataSourceUpdates = new DataSourceUpdates(featureStore, hasEventListeners, onUpdate); if (config.sendEvents && !config.offline && !config.diagnosticOptOut) { @@ -272,11 +291,20 @@ export default class LDClientImpl implements LDClient { defaultValue: any, callback?: (err: any, res: any) => void, ): Promise { - return new Promise((resolve) => { - this.evaluateIfPossible(key, context, defaultValue, this.eventFactoryDefault, (res) => { - resolve(res.detail.value); - callback?.(null, res.detail.value); - }); + return this.withHooks( + key, + context, + defaultValue, + VARIATION_METHOD_NAME, + () => + new Promise((resolve) => { + this.evaluateIfPossible(key, context, defaultValue, this.eventFactoryDefault, (res) => { + resolve(res.detail); + }); + }), + ).then((detail) => { + callback?.(null, detail.value); + return detail.value; }); } @@ -286,12 +314,25 @@ export default class LDClientImpl implements LDClient { defaultValue: any, callback?: (err: any, res: LDEvaluationDetail) => void, ): Promise { - return new Promise((resolve) => { - this.evaluateIfPossible(key, context, defaultValue, this.eventFactoryWithReasons, (res) => { - resolve(res.detail); - callback?.(null, res.detail); - }); - }); + return this.withHooks( + key, + context, + defaultValue, + VARIATION_METHOD_DETAIL_NAME, + () => + new Promise((resolve) => { + this.evaluateIfPossible( + key, + context, + defaultValue, + this.eventFactoryWithReasons, + (res) => { + resolve(res.detail); + callback?.(null, res.detail); + }, + ); + }), + ); } private typedEval( @@ -299,56 +340,87 @@ export default class LDClientImpl implements LDClient { context: LDContext, defaultValue: TResult, eventFactory: EventFactory, + methodName: string, typeChecker: (value: unknown) => [boolean, string], ): Promise { - return new Promise>((resolve) => { - this.evaluateIfPossible( - key, - context, - defaultValue, - eventFactory, - (res) => { - const typedRes: LDEvaluationDetailTyped = { - value: res.detail.value as TResult, - reason: res.detail.reason, - variationIndex: res.detail.variationIndex, - }; - resolve(typedRes); - }, - typeChecker, - ); - }); + return this.withHooks( + key, + context, + defaultValue, + methodName, + () => + new Promise>((resolve) => { + this.evaluateIfPossible( + key, + context, + defaultValue, + eventFactory, + (res) => { + const typedRes: LDEvaluationDetailTyped = { + value: res.detail.value as TResult, + reason: res.detail.reason, + variationIndex: res.detail.variationIndex, + }; + resolve(typedRes); + }, + typeChecker, + ); + }), + ); } async boolVariation(key: string, context: LDContext, defaultValue: boolean): Promise { return ( - await this.typedEval(key, context, defaultValue, this.eventFactoryDefault, (value) => [ - TypeValidators.Boolean.is(value), - TypeValidators.Boolean.getType(), - ]) + await this.typedEval( + key, + context, + defaultValue, + this.eventFactoryDefault, + BOOL_VARIATION_METHOD_NAME, + (value) => [TypeValidators.Boolean.is(value), TypeValidators.Boolean.getType()], + ) ).value; } async numberVariation(key: string, context: LDContext, defaultValue: number): Promise { return ( - await this.typedEval(key, context, defaultValue, this.eventFactoryDefault, (value) => [ - TypeValidators.Number.is(value), - TypeValidators.Number.getType(), - ]) + await this.typedEval( + key, + context, + defaultValue, + this.eventFactoryDefault, + NUMBER_VARIATION_METHOD_NAME, + (value) => [TypeValidators.Number.is(value), TypeValidators.Number.getType()], + ) ).value; } async stringVariation(key: string, context: LDContext, defaultValue: string): Promise { return ( - await this.typedEval(key, context, defaultValue, this.eventFactoryDefault, (value) => [ - TypeValidators.String.is(value), - TypeValidators.String.getType(), - ]) + await this.typedEval( + key, + context, + defaultValue, + this.eventFactoryDefault, + STRING_VARIATION_METHOD_NAME, + (value) => [TypeValidators.String.is(value), TypeValidators.String.getType()], + ) ).value; } jsonVariation(key: string, context: LDContext, defaultValue: unknown): Promise { - return this.variation(key, context, defaultValue); + return this.withHooks( + key, + context, + defaultValue, + JSON_VARIATION_METHOD_NAME, + () => + new Promise((resolve) => { + this.evaluateIfPossible(key, context, defaultValue, this.eventFactoryDefault, (res) => { + resolve(res.detail); + }); + }), + ).then((detail) => detail.value); } boolVariationDetail( @@ -356,10 +428,14 @@ export default class LDClientImpl implements LDClient { context: LDContext, defaultValue: boolean, ): Promise> { - return this.typedEval(key, context, defaultValue, this.eventFactoryWithReasons, (value) => [ - TypeValidators.Boolean.is(value), - TypeValidators.Boolean.getType(), - ]); + return this.typedEval( + key, + context, + defaultValue, + this.eventFactoryWithReasons, + BOOL_VARIATION_DETAIL_METHOD_NAME, + (value) => [TypeValidators.Boolean.is(value), TypeValidators.Boolean.getType()], + ); } numberVariationDetail( @@ -367,10 +443,14 @@ export default class LDClientImpl implements LDClient { context: LDContext, defaultValue: number, ): Promise> { - return this.typedEval(key, context, defaultValue, this.eventFactoryWithReasons, (value) => [ - TypeValidators.Number.is(value), - TypeValidators.Number.getType(), - ]); + return this.typedEval( + key, + context, + defaultValue, + this.eventFactoryWithReasons, + NUMBER_VARIATION_DETAIL_METHOD_NAME, + (value) => [TypeValidators.Number.is(value), TypeValidators.Number.getType()], + ); } stringVariationDetail( @@ -378,10 +458,14 @@ export default class LDClientImpl implements LDClient { context: LDContext, defaultValue: string, ): Promise> { - return this.typedEval(key, context, defaultValue, this.eventFactoryWithReasons, (value) => [ - TypeValidators.String.is(value), - TypeValidators.String.getType(), - ]); + return this.typedEval( + key, + context, + defaultValue, + this.eventFactoryWithReasons, + STRING_VARIATION_DETAIL_METHOD_NAME, + (value) => [TypeValidators.String.is(value), TypeValidators.String.getType()], + ); } jsonVariationDetail( @@ -389,71 +473,112 @@ export default class LDClientImpl implements LDClient { context: LDContext, defaultValue: unknown, ): Promise> { - return this.variationDetail(key, context, defaultValue); + return this.withHooks( + key, + context, + defaultValue, + JSON_VARIATION_DETAIL_METHOD_NAME, + () => + new Promise((resolve) => { + this.evaluateIfPossible( + key, + context, + defaultValue, + this.eventFactoryWithReasons, + (res) => { + resolve(res.detail); + }, + ); + }), + ); } - async migrationVariation( + private async migrationVariationInternal( key: string, context: LDContext, defaultValue: LDMigrationStage, - ): Promise { + ): Promise<{ detail: LDEvaluationDetail; migration: LDMigrationVariation }> { const convertedContext = Context.fromLDContext(context); - return new Promise((resolve) => { - this.evaluateIfPossible( - key, - context, - defaultValue, - this.eventFactoryWithReasons, - ({ detail }, flag) => { - const contextKeys = convertedContext.valid ? convertedContext.kindsAndKeys : {}; - const checkRatio = flag?.migration?.checkRatio; - const samplingRatio = flag?.samplingRatio; - - if (!IsMigrationStage(detail.value)) { - const error = new Error( - `Unrecognized MigrationState for "${key}"; returning default value.`, - ); - this.onError(error); - const reason = { - kind: 'ERROR', - errorKind: ErrorKinds.WrongType, - }; + return new Promise<{ detail: LDEvaluationDetail; migration: LDMigrationVariation }>( + (resolve) => { + this.evaluateIfPossible( + key, + context, + defaultValue, + this.eventFactoryWithReasons, + ({ detail }, flag) => { + const contextKeys = convertedContext.valid ? convertedContext.kindsAndKeys : {}; + const checkRatio = flag?.migration?.checkRatio; + const samplingRatio = flag?.samplingRatio; + + if (!IsMigrationStage(detail.value)) { + const error = new Error( + `Unrecognized MigrationState for "${key}"; returning default value.`, + ); + this.onError(error); + const reason = { + kind: 'ERROR', + errorKind: ErrorKinds.WrongType, + }; + resolve({ + detail: { + value: defaultValue, + reason, + }, + migration: { + value: defaultValue, + tracker: new MigrationOpTracker( + key, + contextKeys, + defaultValue, + defaultValue, + reason, + checkRatio, + undefined, + flag?.version, + samplingRatio, + this.logger, + ), + }, + }); + return; + } resolve({ - value: defaultValue, - tracker: new MigrationOpTracker( - key, - contextKeys, - defaultValue, - defaultValue, - reason, - checkRatio, - undefined, - flag?.version, - samplingRatio, - this.logger, - ), + detail, + migration: { + value: detail.value as LDMigrationStage, + tracker: new MigrationOpTracker( + key, + contextKeys, + defaultValue, + detail.value, + detail.reason, + checkRatio, + // Can be null for compatibility reasons. + detail.variationIndex === null ? undefined : detail.variationIndex, + flag?.version, + samplingRatio, + this.logger, + ), + }, }); - return; - } - resolve({ - value: detail.value as LDMigrationStage, - tracker: new MigrationOpTracker( - key, - contextKeys, - defaultValue, - detail.value, - detail.reason, - checkRatio, - // Can be null for compatibility reasons. - detail.variationIndex === null ? undefined : detail.variationIndex, - flag?.version, - samplingRatio, - this.logger, - ), - }); - }, - ); - }); + }, + ); + }, + ); + } + + async migrationVariation( + key: string, + context: LDContext, + defaultValue: LDMigrationStage, + ): Promise { + const { hooks, hookContext }: { hooks: Hook[]; hookContext: EvaluationHookContext } = + this.prepareHooks(key, context, defaultValue, MIGRATION_VARIATION_METHOD_NAME); + const hookData = this.executeBeforeEvaluation(hooks, hookContext); + const res = await this.migrationVariationInternal(key, context, defaultValue); + this.executeAfterEvaluation(hooks, hookContext, hookData, res.detail); + return res.migration; } allFlagsState( @@ -601,6 +726,10 @@ export default class LDClientImpl implements LDClient { callback?.(null, true); } + addHook(hook: Hook): void { + this.hooks.push(hook); + } + private variationInternal( flagKey: string, context: LDContext, @@ -738,4 +867,81 @@ export default class LDClientImpl implements LDClient { this.onReady(); } } + + private async withHooks( + key: string, + context: LDContext, + defaultValue: unknown, + methodName: string, + method: () => Promise, + ): Promise { + if (this.hooks.length === 0) { + return method(); + } + const { hooks, hookContext }: { hooks: Hook[]; hookContext: EvaluationHookContext } = + this.prepareHooks(key, context, defaultValue, methodName); + const hookData = this.executeBeforeEvaluation(hooks, hookContext); + const result = await method(); + this.executeAfterEvaluation(hooks, hookContext, hookData, result); + return result; + } + + private tryExecuteStage(method: string, hookName: string, stage: () => T) { + try { + return stage(); + } catch (err) { + this.logger?.error( + `An error was encountered in "${method}" of the "${hookName}" hook: ${err}`, + ); + return {}; + } + } + + private hookName(hook?: Hook) { + try { + return hook?.getMetadata().name ?? UNKNOWN_HOOK_NAME; + } catch { + this.logger?.error(`Exception thrown getting metadata for hook. Unable to get hook name.`); + return UNKNOWN_HOOK_NAME; + } + } + + private executeAfterEvaluation( + hooks: Hook[], + hookContext: EvaluationHookContext, + updatedData: (EvaluationHookData | undefined)[], + result: LDEvaluationDetail, + ) { + // This iterates in reverse, versus reversing a shallow copy of the hooks, + // for efficiency. + for (let hookIndex = hooks.length - 1; hookIndex >= 0; hookIndex -= 1) { + const hook = hooks[hookIndex]; + const data = updatedData[hookIndex] ?? {}; + this.tryExecuteStage(AFTER_EVALUATION_STAGE_NAME, this.hookName(hook), () => + hook?.afterEvaluation?.(hookContext, data, result), + ); + } + } + + private executeBeforeEvaluation(hooks: Hook[], hookContext: EvaluationHookContext) { + return hooks.map((hook) => + this.tryExecuteStage(BEFORE_EVALUATION_STAGE_NAME, this.hookName(hook), () => + hook?.beforeEvaluation?.(hookContext, {}), + ), + ); + } + + private prepareHooks(key: string, context: LDContext, defaultValue: unknown, methodName: string) { + // Copy the hooks to use a consistent set during evaluation. Hooks could be added and we want + // to ensure all correct stages for any give hook execute. Not for instance the afterEvaluation + // stage without beforeEvaluation having been called on that hook. + const hooks: Hook[] = [...this.hooks]; + const hookContext: EvaluationHookContext = { + flagKey: key, + context, + defaultValue, + method: methodName, + }; + return { hooks, hookContext }; + } } diff --git a/packages/shared/sdk-server/src/api/LDClient.ts b/packages/shared/sdk-server/src/api/LDClient.ts index 177727c44d..aee36f0921 100644 --- a/packages/shared/sdk-server/src/api/LDClient.ts +++ b/packages/shared/sdk-server/src/api/LDClient.ts @@ -9,6 +9,7 @@ import { LDMigrationOpEvent, LDMigrationVariation } from './data'; import { LDFlagsState } from './data/LDFlagsState'; import { LDFlagsStateOptions } from './data/LDFlagsStateOptions'; import { LDMigrationStage } from './data/LDMigrationStage'; +import { Hook } from './integrations/Hook'; /** * The LaunchDarkly SDK client object. @@ -441,4 +442,14 @@ export interface LDClient { * fails, so be sure to attach a rejection handler to it. */ flush(callback?: (err: Error | null, res: boolean) => void): Promise; + + /** + * Add a hook to the client. In order to register a hook before the client + * starts, please use the `hooks` property of {@link LDOptions}. + * + * Hooks provide entrypoints which allow for observation of SDK functions. + * + * @param Hook The hook to add. + */ + addHook(hook: Hook): void; } diff --git a/packages/shared/sdk-server/src/api/index.ts b/packages/shared/sdk-server/src/api/index.ts index 41e35f8bb3..8a99cc362e 100644 --- a/packages/shared/sdk-server/src/api/index.ts +++ b/packages/shared/sdk-server/src/api/index.ts @@ -7,6 +7,9 @@ export * from './subsystems/LDFeatureStore'; // These are items that should be less frequently used, and therefore they // are namespaced to reduce clutter amongst the top level exports. + +// Integrations was overwritten by the exports of index.ts. On a major version +// we should consider removing this and exporting integrations differently. export * as integrations from './integrations'; export * as interfaces from './interfaces'; export * as subsystems from './subsystems'; diff --git a/packages/shared/sdk-server/src/api/integrations/Hook.ts b/packages/shared/sdk-server/src/api/integrations/Hook.ts new file mode 100644 index 0000000000..0322ce5fbe --- /dev/null +++ b/packages/shared/sdk-server/src/api/integrations/Hook.ts @@ -0,0 +1,80 @@ +import { LDContext, LDEvaluationDetail } from '@launchdarkly/js-sdk-common'; + +/** + * Contextual information provided to evaluation stages. + */ +export interface EvaluationHookContext { + readonly flagKey: string; + readonly context: LDContext; + readonly defaultValue: unknown; + readonly method: string; +} + +/** + * Implementation specific hook data for evaluation stages. + * + * Hook implementations can use this to store data needed between stages. + */ +export interface EvaluationHookData { + [index: string]: unknown; +} + +/** + * Meta-data about a hook implementation. + */ +export interface HookMetadata { + readonly name: string; +} + +/** + * Interface extending SDK functionality via hooks. + */ +export interface Hook { + /** + * Get metadata about the hook implementation. + */ + getMetadata(): HookMetadata; + + /** + * The before method is called during the execution of a variation method + * before the flag value has been determined. The method is executed synchronously. + * + * @param hookContext Contains information about the evaluation being performed. This is not + * mutable. + * @param data A record associated with each stage of hook invocations. Each stage is called with + * the data of the previous stage for a series. The input record should not be modified. + * @returns Data to use when executing the next state of the hook in the evaluation series. It is + * recommended to expand the previous input into the return. This helps ensure your stage remains + * compatible moving forward as more stages are added. + * ```js + * return {...data, "my-new-field": /*my data/*} + * ``` + */ + beforeEvaluation?( + hookContext: EvaluationHookContext, + data: EvaluationHookData, + ): EvaluationHookData; + + /** + * The after method is called during the execution of the variation method + * after the flag value has been determined. The method is executed synchronously. + * + * @param hookContext Contains read-only information about the evaluation + * being performed. + * @param data A record associated with each stage of hook invocations. Each + * stage is called with the data of the previous stage for a series. + * @param detail The result of the evaluation. This value should not be + * modified. + * @returns Data to use when executing the next state of the hook in the evaluation series. It is + * recommended to expand the previous input into the return. This helps ensure your stage remains + * compatible moving forward as more stages are added. + * ```js + * return {...data, "my-new-field": /*my data/*} + * ``` + */ + afterEvaluation?( + hookContext: EvaluationHookContext, + data: EvaluationHookData, + detail: LDEvaluationDetail, + ): EvaluationHookData; +} diff --git a/packages/shared/sdk-server/src/api/integrations/index.ts b/packages/shared/sdk-server/src/api/integrations/index.ts index fdf3c3574d..19c0a75224 100644 --- a/packages/shared/sdk-server/src/api/integrations/index.ts +++ b/packages/shared/sdk-server/src/api/integrations/index.ts @@ -1 +1,2 @@ export * from './FileDataSourceOptions'; +export * from './Hook'; diff --git a/packages/shared/sdk-server/src/api/options/LDOptions.ts b/packages/shared/sdk-server/src/api/options/LDOptions.ts index 3954563c7b..a0d38b0fab 100644 --- a/packages/shared/sdk-server/src/api/options/LDOptions.ts +++ b/packages/shared/sdk-server/src/api/options/LDOptions.ts @@ -1,5 +1,6 @@ import { LDClientContext, LDLogger, subsystem, VoidFunction } from '@launchdarkly/js-sdk-common'; +import { Hook } from '../integrations/Hook'; import { LDDataSourceUpdates, LDFeatureStore } from '../subsystems'; import { LDBigSegmentsOptions } from './LDBigSegmentsOptions'; import { LDProxyOptions } from './LDProxyOptions'; @@ -268,4 +269,23 @@ export interface LDOptions { */ versionName?: string; }; + + /** + * Initial set of hooks for the client. + * + * Hooks provide entrypoints which allow for observation of SDK functions. + * + * LaunchDarkly provides integration packages, and most applications will not + * need to implement their own hooks. Refer to the `@launchdarkly/node-server-sdk-otel` + * for instrumentation for the `@launchdarkly/node-server-sdk`. + * + * Example: + * ```typescript + * import { init } from '@launchdarkly/node-server-sdk'; + * import { TracingHook } from '@launchdarkly/node-server-sdk-otel'; + * + * const client = init('my-sdk-key', { hooks: [new TracingHook()] }); + * ``` + */ + hooks?: Hook[]; } diff --git a/packages/shared/sdk-server/src/integrations/index.ts b/packages/shared/sdk-server/src/integrations/index.ts index 2c162e8220..0bbbcdf0ac 100644 --- a/packages/shared/sdk-server/src/integrations/index.ts +++ b/packages/shared/sdk-server/src/integrations/index.ts @@ -1,4 +1,7 @@ import FileDataSourceFactory from './FileDataSourceFactory'; export * from './test_data'; +// Api exported integrations, but it was overwritten by the more specific +// integrations from index.ts. +export * from '../api/integrations'; export { FileDataSourceFactory }; diff --git a/packages/shared/sdk-server/src/options/Configuration.ts b/packages/shared/sdk-server/src/options/Configuration.ts index 709216d189..9e68a4f202 100644 --- a/packages/shared/sdk-server/src/options/Configuration.ts +++ b/packages/shared/sdk-server/src/options/Configuration.ts @@ -13,6 +13,7 @@ import { } from '@launchdarkly/js-sdk-common'; import { LDBigSegmentsOptions, LDOptions, LDProxyOptions, LDTLSOptions } from '../api'; +import { Hook } from '../api/integrations'; import { LDDataSourceUpdates, LDFeatureStore } from '../api/subsystems'; import InMemoryFeatureStore from '../store/InMemoryFeatureStore'; import { ValidatedOptions } from './ValidatedOptions'; @@ -54,6 +55,7 @@ const validations: Record = { wrapperName: TypeValidators.String, wrapperVersion: TypeValidators.String, application: TypeValidators.Object, + hooks: TypeValidators.createTypeArray('Hook[]', {}), }; /** @@ -208,6 +210,8 @@ export default class Configuration { public readonly bigSegments?: LDBigSegmentsOptions; + public readonly hooks?: Hook[]; + constructor(options: LDOptions = {}, internalOptions: internal.LDInternalOptions = {}) { // The default will handle undefined, but not null. // Because we can be called from JS we need to be extra defensive. @@ -272,5 +276,7 @@ export default class Configuration { // @ts-ignore this.featureStoreFactory = () => validatedOptions.featureStore; } + + this.hooks = validatedOptions.hooks; } } diff --git a/packages/shared/sdk-server/src/options/ValidatedOptions.ts b/packages/shared/sdk-server/src/options/ValidatedOptions.ts index 45484e8e1d..ce9a58de9b 100644 --- a/packages/shared/sdk-server/src/options/ValidatedOptions.ts +++ b/packages/shared/sdk-server/src/options/ValidatedOptions.ts @@ -1,6 +1,7 @@ import { LDLogger, subsystem } from '@launchdarkly/js-sdk-common'; import { LDBigSegmentsOptions, LDOptions, LDProxyOptions, LDTLSOptions } from '../api'; +import { Hook } from '../api/integrations'; import { LDFeatureStore } from '../api/subsystems'; /** @@ -39,4 +40,5 @@ export interface ValidatedOptions { // Allow indexing this by a string for the validation step. [index: string]: any; bigSegments?: LDBigSegmentsOptions; + hooks?: Hook[]; } From 55df5fb19633fcdadea91061aa50658a15fd924e Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 19 Mar 2024 08:31:37 -0700 Subject: [PATCH 02/10] Linting --- contract-tests/TestHook.js | 17 +++++------------ contract-tests/index.js | 2 +- contract-tests/sdkClientEntity.js | 5 +++-- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/contract-tests/TestHook.js b/contract-tests/TestHook.js index d0f147ca55..1364f6291f 100644 --- a/contract-tests/TestHook.js +++ b/contract-tests/TestHook.js @@ -22,30 +22,23 @@ export default class TestHook { }; } - beforeEvaluation( - hookContext, - data, - ) { + beforeEvaluation(hookContext, data) { this._safePost({ evaluationHookContext: hookContext, evaluationHookData: data, stage: 'beforeEvaluation', }); - return {...data, ...(this._data?.['beforeEvaluation'] || {})}; + return { ...data, ...(this._data?.['beforeEvaluation'] || {}) }; } - afterEvaluation( - hookContext, - data, - detail, - ) { + afterEvaluation(hookContext, data, detail) { this._safePost({ evaluationHookContext: hookContext, evaluationHookData: data, stage: 'afterEvaluation', evaluationDetail: detail, }); - - return {...data, ...(this._data?.['afterEvaluation'] || {})}; + + return { ...data, ...(this._data?.['afterEvaluation'] || {}) }; } } diff --git a/contract-tests/index.js b/contract-tests/index.js index 6422e6b1c6..7aacab173e 100644 --- a/contract-tests/index.js +++ b/contract-tests/index.js @@ -32,7 +32,7 @@ app.get('/', (req, res) => { 'event-sampling', 'strongly-typed', 'polling-gzip', - 'evaluation-hooks' + 'evaluation-hooks', ], }); }); diff --git a/contract-tests/sdkClientEntity.js b/contract-tests/sdkClientEntity.js index d888d8f1b0..c3891fad56 100644 --- a/contract-tests/sdkClientEntity.js +++ b/contract-tests/sdkClientEntity.js @@ -63,8 +63,9 @@ export function makeSdkConfig(options, tag) { }; } if (options.hooks) { - cf.hooks = options.hooks.hooks - .map(hook => new TestHook(hook.name, hook.callbackUri, hook.data)); + cf.hooks = options.hooks.hooks.map( + (hook) => new TestHook(hook.name, hook.callbackUri, hook.data), + ); } return cf; } From 1ea16d271710f497ddbd4c1381fd966c625d4a54 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 19 Mar 2024 09:20:37 -0700 Subject: [PATCH 03/10] Fix capabilities. --- contract-tests/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contract-tests/index.js b/contract-tests/index.js index 7aacab173e..5df1596190 100644 --- a/contract-tests/index.js +++ b/contract-tests/index.js @@ -32,6 +32,8 @@ app.get('/', (req, res) => { 'event-sampling', 'strongly-typed', 'polling-gzip', + 'inline-context', + 'anonymous-redaction', 'evaluation-hooks', ], }); From 0167d37c83f6477a8c1f4e4d3f420ea0a59d8634 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 19 Mar 2024 10:44:00 -0700 Subject: [PATCH 04/10] More explicit type safety. --- .../shared/sdk-server/src/LDClientImpl.ts | 35 ++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index ea3d28a9bb..3859ab879a 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -886,7 +886,11 @@ export default class LDClientImpl implements LDClient { return result; } - private tryExecuteStage(method: string, hookName: string, stage: () => T) { + private tryExecuteStage( + method: string, + hookName: string, + stage: () => EvaluationHookData, + ): EvaluationHookData { try { return stage(); } catch (err) { @@ -897,7 +901,7 @@ export default class LDClientImpl implements LDClient { } } - private hookName(hook?: Hook) { + private hookName(hook?: Hook): string { try { return hook?.getMetadata().name ?? UNKNOWN_HOOK_NAME; } catch { @@ -917,21 +921,36 @@ export default class LDClientImpl implements LDClient { for (let hookIndex = hooks.length - 1; hookIndex >= 0; hookIndex -= 1) { const hook = hooks[hookIndex]; const data = updatedData[hookIndex] ?? {}; - this.tryExecuteStage(AFTER_EVALUATION_STAGE_NAME, this.hookName(hook), () => - hook?.afterEvaluation?.(hookContext, data, result), + this.tryExecuteStage( + AFTER_EVALUATION_STAGE_NAME, + this.hookName(hook), + () => hook?.afterEvaluation?.(hookContext, data, result) ?? {}, ); } } - private executeBeforeEvaluation(hooks: Hook[], hookContext: EvaluationHookContext) { + private executeBeforeEvaluation( + hooks: Hook[], + hookContext: EvaluationHookContext, + ): EvaluationHookData[] { return hooks.map((hook) => - this.tryExecuteStage(BEFORE_EVALUATION_STAGE_NAME, this.hookName(hook), () => - hook?.beforeEvaluation?.(hookContext, {}), + this.tryExecuteStage( + BEFORE_EVALUATION_STAGE_NAME, + this.hookName(hook), + () => hook?.beforeEvaluation?.(hookContext, {}) ?? {}, ), ); } - private prepareHooks(key: string, context: LDContext, defaultValue: unknown, methodName: string) { + private prepareHooks( + key: string, + context: LDContext, + defaultValue: unknown, + methodName: string, + ): { + hooks: Hook[]; + hookContext: EvaluationHookContext; + } { // Copy the hooks to use a consistent set during evaluation. Hooks could be added and we want // to ensure all correct stages for any give hook execute. Not for instance the afterEvaluation // stage without beforeEvaluation having been called on that hook. From 0ecb67bdf6190dc1062e1897c09c849584c84c08 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 19 Mar 2024 14:53:38 -0700 Subject: [PATCH 05/10] Rename series data. --- .../shared/sdk-server/src/LDClientImpl.ts | 22 +++++++++---------- .../sdk-server/src/api/integrations/Hook.ts | 18 +++++++-------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 3859ab879a..024e792370 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -24,7 +24,7 @@ import { LDMigrationVariation, LDOptions, } from './api'; -import { EvaluationHookContext, EvaluationHookData, Hook } from './api/integrations/Hook'; +import { EvaluationSeriesContext, EvaluationSeriesData, Hook } from './api/integrations/Hook'; import { BigSegmentStoreMembership } from './api/interfaces'; import BigSegmentsManager from './BigSegmentsManager'; import BigSegmentStoreStatusProvider from './BigSegmentStatusProviderImpl'; @@ -573,7 +573,7 @@ export default class LDClientImpl implements LDClient { context: LDContext, defaultValue: LDMigrationStage, ): Promise { - const { hooks, hookContext }: { hooks: Hook[]; hookContext: EvaluationHookContext } = + const { hooks, hookContext }: { hooks: Hook[]; hookContext: EvaluationSeriesContext } = this.prepareHooks(key, context, defaultValue, MIGRATION_VARIATION_METHOD_NAME); const hookData = this.executeBeforeEvaluation(hooks, hookContext); const res = await this.migrationVariationInternal(key, context, defaultValue); @@ -878,7 +878,7 @@ export default class LDClientImpl implements LDClient { if (this.hooks.length === 0) { return method(); } - const { hooks, hookContext }: { hooks: Hook[]; hookContext: EvaluationHookContext } = + const { hooks, hookContext }: { hooks: Hook[]; hookContext: EvaluationSeriesContext } = this.prepareHooks(key, context, defaultValue, methodName); const hookData = this.executeBeforeEvaluation(hooks, hookContext); const result = await method(); @@ -889,8 +889,8 @@ export default class LDClientImpl implements LDClient { private tryExecuteStage( method: string, hookName: string, - stage: () => EvaluationHookData, - ): EvaluationHookData { + stage: () => EvaluationSeriesData, + ): EvaluationSeriesData { try { return stage(); } catch (err) { @@ -912,8 +912,8 @@ export default class LDClientImpl implements LDClient { private executeAfterEvaluation( hooks: Hook[], - hookContext: EvaluationHookContext, - updatedData: (EvaluationHookData | undefined)[], + hookContext: EvaluationSeriesContext, + updatedData: (EvaluationSeriesData | undefined)[], result: LDEvaluationDetail, ) { // This iterates in reverse, versus reversing a shallow copy of the hooks, @@ -931,8 +931,8 @@ export default class LDClientImpl implements LDClient { private executeBeforeEvaluation( hooks: Hook[], - hookContext: EvaluationHookContext, - ): EvaluationHookData[] { + hookContext: EvaluationSeriesContext, + ): EvaluationSeriesData[] { return hooks.map((hook) => this.tryExecuteStage( BEFORE_EVALUATION_STAGE_NAME, @@ -949,13 +949,13 @@ export default class LDClientImpl implements LDClient { methodName: string, ): { hooks: Hook[]; - hookContext: EvaluationHookContext; + hookContext: EvaluationSeriesContext; } { // Copy the hooks to use a consistent set during evaluation. Hooks could be added and we want // to ensure all correct stages for any give hook execute. Not for instance the afterEvaluation // stage without beforeEvaluation having been called on that hook. const hooks: Hook[] = [...this.hooks]; - const hookContext: EvaluationHookContext = { + const hookContext: EvaluationSeriesContext = { flagKey: key, context, defaultValue, diff --git a/packages/shared/sdk-server/src/api/integrations/Hook.ts b/packages/shared/sdk-server/src/api/integrations/Hook.ts index 0322ce5fbe..5bfec51cf7 100644 --- a/packages/shared/sdk-server/src/api/integrations/Hook.ts +++ b/packages/shared/sdk-server/src/api/integrations/Hook.ts @@ -3,7 +3,7 @@ import { LDContext, LDEvaluationDetail } from '@launchdarkly/js-sdk-common'; /** * Contextual information provided to evaluation stages. */ -export interface EvaluationHookContext { +export interface EvaluationSeriesContext { readonly flagKey: string; readonly context: LDContext; readonly defaultValue: unknown; @@ -15,8 +15,8 @@ export interface EvaluationHookContext { * * Hook implementations can use this to store data needed between stages. */ -export interface EvaluationHookData { - [index: string]: unknown; +export interface EvaluationSeriesData { + readonly [index: string]: unknown; } /** @@ -51,9 +51,9 @@ export interface Hook { * ``` */ beforeEvaluation?( - hookContext: EvaluationHookContext, - data: EvaluationHookData, - ): EvaluationHookData; + hookContext: EvaluationSeriesContext, + data: EvaluationSeriesData, + ): EvaluationSeriesData; /** * The after method is called during the execution of the variation method @@ -73,8 +73,8 @@ export interface Hook { * ``` */ afterEvaluation?( - hookContext: EvaluationHookContext, - data: EvaluationHookData, + hookContext: EvaluationSeriesContext, + data: EvaluationSeriesData, detail: LDEvaluationDetail, - ): EvaluationHookData; + ): EvaluationSeriesData; } From 59d83cf3e9177fef4040bbd1642ec37d67ec313d Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 19 Mar 2024 14:56:51 -0700 Subject: [PATCH 06/10] More name updates. --- contract-tests/TestHook.js | 8 ++-- .../__tests__/LDClient.hooks.test.ts | 48 +++++++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/contract-tests/TestHook.js b/contract-tests/TestHook.js index 1364f6291f..9cb2274f3b 100644 --- a/contract-tests/TestHook.js +++ b/contract-tests/TestHook.js @@ -24,8 +24,8 @@ export default class TestHook { beforeEvaluation(hookContext, data) { this._safePost({ - evaluationHookContext: hookContext, - evaluationHookData: data, + evaluationSeriesContext: hookContext, + evaluationSeriesData: data, stage: 'beforeEvaluation', }); return { ...data, ...(this._data?.['beforeEvaluation'] || {}) }; @@ -33,8 +33,8 @@ export default class TestHook { afterEvaluation(hookContext, data, detail) { this._safePost({ - evaluationHookContext: hookContext, - evaluationHookData: data, + evaluationSeriesContext: hookContext, + evaluationSeriesData: data, stage: 'afterEvaluation', evaluationDetail: detail, }); diff --git a/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts b/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts index 5c86e2ff8e..49c63afb0f 100644 --- a/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts @@ -10,8 +10,8 @@ const defaultUser = { kind: 'user', key: 'user-key' }; type EvalCapture = { method: string; - hookContext: integrations.EvaluationHookContext; - hookData: integrations.EvaluationHookData; + hookContext: integrations.EvaluationSeriesContext; + hookData: integrations.EvaluationSeriesData; detail?: LDEvaluationDetail; }; @@ -26,8 +26,8 @@ class TestHook implements integrations.Hook { } verifyBefore( - hookContext: integrations.EvaluationHookContext, - data: integrations.EvaluationHookData, + hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, ) { expect(this.captureBefore).toHaveLength(1); expect(this.captureBefore[0].hookContext).toEqual(hookContext); @@ -35,8 +35,8 @@ class TestHook implements integrations.Hook { } verifyAfter( - hookContext: integrations.EvaluationHookContext, - data: integrations.EvaluationHookData, + hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, detail: LDEvaluationDetail, ) { expect(this.captureAfter).toHaveLength(1); @@ -46,28 +46,28 @@ class TestHook implements integrations.Hook { } beforeEvalImpl: ( - hookContext: integrations.EvaluationHookContext, - data: integrations.EvaluationHookData, - ) => integrations.EvaluationHookData = (_hookContext, data) => data; + hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, + ) => integrations.EvaluationSeriesData = (_hookContext, data) => data; afterEvalImpl: ( - hookContext: integrations.EvaluationHookContext, - data: integrations.EvaluationHookData, + hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, detail: LDEvaluationDetail, - ) => integrations.EvaluationHookData = (_hookContext, data, _detail) => data; + ) => integrations.EvaluationSeriesData = (_hookContext, data, _detail) => data; beforeEvaluation?( - hookContext: integrations.EvaluationHookContext, - data: integrations.EvaluationHookData, - ): integrations.EvaluationHookData { + hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, + ): integrations.EvaluationSeriesData { this.captureBefore.push({ method: 'beforeEvaluation', hookContext, hookData: data }); return this.beforeEvalImpl(hookContext, data); } afterEvaluation?( - hookContext: integrations.EvaluationHookContext, - data: integrations.EvaluationHookData, + hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, detail: LDEvaluationDetail, - ): integrations.EvaluationHookData { + ): integrations.EvaluationSeriesData { this.captureAfter.push({ method: 'afterEvaluation', hookContext, hookData: data, detail }); return this.afterEvalImpl(hookContext, data, detail); } @@ -412,8 +412,8 @@ describe('given an LDClient with test data', () => { it('propagates data between stages', async () => { testHook.beforeEvalImpl = ( - _hookContext: integrations.EvaluationHookContext, - data: integrations.EvaluationHookData, + _hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, ) => ({ ...data, added: 'added data', @@ -438,8 +438,8 @@ describe('given an LDClient with test data', () => { it('handles an exception thrown in beforeEvaluation', async () => { testHook.beforeEvalImpl = ( - _hookContext: integrations.EvaluationHookContext, - _data: integrations.EvaluationHookData, + _hookContext: integrations.EvaluationSeriesContext, + _data: integrations.EvaluationSeriesData, ) => { throw new Error('bad hook'); }; @@ -483,8 +483,8 @@ describe('given an LDClient with test data', () => { it('uses unknown name when the name cannot be accessed', async () => { testHook.beforeEvalImpl = ( - _hookContext: integrations.EvaluationHookContext, - _data: integrations.EvaluationHookData, + _hookContext: integrations.EvaluationSeriesContext, + _data: integrations.EvaluationSeriesData, ) => { throw new Error('bad hook'); }; From 931fc5b25ff13e3a8ebafd1def981ebf8693d0ee Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 20 Mar 2024 09:36:26 -0700 Subject: [PATCH 07/10] Fix sentence. --- packages/shared/sdk-server/src/api/integrations/Hook.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/sdk-server/src/api/integrations/Hook.ts b/packages/shared/sdk-server/src/api/integrations/Hook.ts index 5bfec51cf7..52e7639866 100644 --- a/packages/shared/sdk-server/src/api/integrations/Hook.ts +++ b/packages/shared/sdk-server/src/api/integrations/Hook.ts @@ -27,7 +27,7 @@ export interface HookMetadata { } /** - * Interface extending SDK functionality via hooks. + * Interface for extending SDK functionality via hooks. */ export interface Hook { /** From 1c4d071c813349672225a56bf4c05db52abfe80a Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 28 Mar 2024 10:55:44 -0700 Subject: [PATCH 08/10] Make addHook optional in the interface. --- packages/shared/sdk-server/src/api/LDClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/sdk-server/src/api/LDClient.ts b/packages/shared/sdk-server/src/api/LDClient.ts index aee36f0921..2b15d51f74 100644 --- a/packages/shared/sdk-server/src/api/LDClient.ts +++ b/packages/shared/sdk-server/src/api/LDClient.ts @@ -451,5 +451,5 @@ export interface LDClient { * * @param Hook The hook to add. */ - addHook(hook: Hook): void; + addHook?(hook: Hook): void; } From 746be2a00d5800e2746d40acf04118241cf14ada Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 1 Apr 2024 09:36:11 -0700 Subject: [PATCH 09/10] Add runtime registered hook. --- .../sdk-server/__tests__/LDClient.hooks.test.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts b/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts index 49c63afb0f..89557553b5 100644 --- a/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts @@ -581,6 +581,17 @@ it('executes hook stages in the specified order', async () => { return data; }; + const hookC = new TestHook(); + hookC.beforeEvalImpl = (_context, data) => { + beforeCalledOrder.push('c'); + return data; + }; + + hookC.afterEvalImpl = (_context, data, _detail) => { + afterCalledOrder.push('c'); + return data; + }; + const logger = new TestLogger(); const td = new TestData(); const client = new LDClientImpl( @@ -596,8 +607,9 @@ it('executes hook stages in the specified order', async () => { ); await client.waitForInitialization(); + client.addHook(hookC) await client.variation('flagKey', defaultUser, false); - expect(beforeCalledOrder).toEqual(['a', 'b']); - expect(afterCalledOrder).toEqual(['b', 'a']); + expect(beforeCalledOrder).toEqual(['a', 'b', 'c']); + expect(afterCalledOrder).toEqual(['c', 'b', 'a']); }); From 5e13b58fa5c6302f28e818ece2e864f6a1e50213 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 1 Apr 2024 10:08:29 -0700 Subject: [PATCH 10/10] semicolon --- packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts b/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts index 89557553b5..b69de7083f 100644 --- a/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts @@ -607,7 +607,7 @@ it('executes hook stages in the specified order', async () => { ); await client.waitForInitialization(); - client.addHook(hookC) + client.addHook(hookC); await client.variation('flagKey', defaultUser, false); expect(beforeCalledOrder).toEqual(['a', 'b', 'c']);