From a0901700ff04c0a26b526e3210b05ecf10d24736 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 19 May 2022 16:26:36 -0700 Subject: [PATCH 01/39] Work on initial scaffold. --- .../src/BigSegmentsStoreStatusProviderNode.ts | 26 +++++ platform-node/src/LDClientNode.ts | 96 +++++++++++++++ platform-node/src/api/LDClient.ts | 22 ++++ platform-node/src/api/index.ts | 2 + .../BigSegmentStoreStatusProvider.ts | 8 +- platform-node/src/api/interfaces/index.ts | 1 + platform-node/src/index.ts | 25 +--- .../src/{ => platform}/NodeCrypto.ts | 0 .../src/{ => platform}/NodeFilesystem.ts | 0 platform-node/src/{ => platform}/NodeInfo.ts | 2 +- .../src/{ => platform}/NodePlatform.ts | 0 .../src/{ => platform}/NodeRequests.ts | 0 .../src/{ => platform}/NodeResponse.ts | 0 .../src/BigSegmentStatusProvider.ts | 36 ++++++ server-sdk-common/src/LDClientImpl.ts | 109 ++++++++++++++++++ server-sdk-common/src/api/LDClient.ts | 45 +------- server-sdk-common/src/api/interfaces/index.ts | 1 - server-sdk-common/src/index.ts | 4 + 18 files changed, 304 insertions(+), 73 deletions(-) create mode 100644 platform-node/src/BigSegmentsStoreStatusProviderNode.ts create mode 100644 platform-node/src/LDClientNode.ts create mode 100644 platform-node/src/api/LDClient.ts create mode 100644 platform-node/src/api/index.ts rename {server-sdk-common => platform-node}/src/api/interfaces/BigSegmentStoreStatusProvider.ts (86%) create mode 100644 platform-node/src/api/interfaces/index.ts rename platform-node/src/{ => platform}/NodeCrypto.ts (100%) rename platform-node/src/{ => platform}/NodeFilesystem.ts (100%) rename platform-node/src/{ => platform}/NodeInfo.ts (94%) rename platform-node/src/{ => platform}/NodePlatform.ts (100%) rename platform-node/src/{ => platform}/NodeRequests.ts (100%) rename platform-node/src/{ => platform}/NodeResponse.ts (100%) create mode 100644 server-sdk-common/src/BigSegmentStatusProvider.ts create mode 100644 server-sdk-common/src/LDClientImpl.ts diff --git a/platform-node/src/BigSegmentsStoreStatusProviderNode.ts b/platform-node/src/BigSegmentsStoreStatusProviderNode.ts new file mode 100644 index 0000000000..0a4b03cd5c --- /dev/null +++ b/platform-node/src/BigSegmentsStoreStatusProviderNode.ts @@ -0,0 +1,26 @@ +import { LDClientImpl } from '@launchdarkly/js-server-sdk-common'; +import { BigSegmentStoreStatus } from '@launchdarkly/js-server-sdk-common/dist/api/interfaces'; +import EventEmitter from 'events'; +import { BigSegmentStoreStatusProvider } from './api/interfaces/BigSegmentStoreStatusProvider'; + +export default class BigSegmentStoreStatusProviderNode + extends EventEmitter implements BigSegmentStoreStatusProvider { + clientBase: LDClientImpl; + + constructor(clientBase: LDClientImpl) { + super(); + this.clientBase = clientBase; + + clientBase.bigSegmentStoreStatusProvider.setStatusHandler((status) => { + this.emit('change', status); + }); + } + + getStatus(): BigSegmentStoreStatus | undefined { + return this.clientBase.bigSegmentStoreStatusProvider.getStatus(); + } + + requireStatus(): Promise { + return this.clientBase.bigSegmentStoreStatusProvider.requireStatus(); + } +} diff --git a/platform-node/src/LDClientNode.ts b/platform-node/src/LDClientNode.ts new file mode 100644 index 0000000000..b34d16660f --- /dev/null +++ b/platform-node/src/LDClientNode.ts @@ -0,0 +1,96 @@ +/* eslint-disable @typescript-eslint/no-unused-vars */ +/* eslint-disable class-methods-use-this */ +import { + LDClientImpl, LDContext, LDEvaluationDetail, LDFlagsState, LDFlagsStateOptions, LDOptions, +} from '@launchdarkly/js-server-sdk-common'; + +import EventEmitter from 'events'; +import { LDClient } from './api/LDClient'; +import BigSegmentStoreStatusProviderNode from './BigSegmentsStoreStatusProviderNode'; +import { BigSegmentStoreStatusProvider } from './api/interfaces/BigSegmentStoreStatusProvider'; +import NodePlatform from './platform/NodePlatform'; + +export default class LDClientNode extends EventEmitter implements LDClient { + /** + * The LDClientImpl is included through composition, instead of inheritance, + * for a couple of reasons. + * 1. This allows the node client to implement EventEmitter, where otherwise + * it would be unable to from single inheritance. EventEmitter is not included + * in common because it is node specific. + * + * 2. This allows conversions of some types compared to the base implementation. + * Such as returning a node `LDClient` instead of the base `LDClient` interface. + * This interface extends EventEmitter to support #1. + */ + private client: LDClientImpl; + + constructor(options: LDOptions) { + super(); + // TODO: Remember to parse the options before providing them to the platform. + this.client = new LDClientImpl(new NodePlatform(options)); + // Extend the BigSegmentStoreStatusProvider from the common client to allow + // for use of the event emitter. + this.bigSegmentStoreStatusProvider = new BigSegmentStoreStatusProviderNode(this.client); + } + + bigSegmentStoreStatusProvider: BigSegmentStoreStatusProvider; + + initialized(): boolean { + return this.client.initialized(); + } + + async waitForInitialization(): Promise { + await this.client.waitForInitialization(); + return this; + } + + variation( + key: string, + context: LDContext, + defaultValue: any, + callback?: (err: any, res: any) => void, + ): Promise { + return this.client.variation(key, context, defaultValue, callback); + } + + variationDetail( + key: string, + context: LDContext, + defaultValue: any, + callback?: (err: any, res: LDEvaluationDetail) => void, + ): Promise { + return this.client.variationDetail(key, context, defaultValue, callback); + } + + allFlagsState( + context: LDContext, + options?: LDFlagsStateOptions, + callback?: (err: Error, res: LDFlagsState) => void, + ): Promise { + return this.client.allFlagsState(context, options, callback); + } + + secureModeHash(context: LDContext): string { + return this.client.secureModeHash(context); + } + + close(): void { + this.client.close(); + } + + isOffline(): boolean { + return this.client.isOffline(); + } + + track(key: string, context: LDContext, data?: any, metricValue?: number): void { + return this.client.track(key, context, data, metricValue); + } + + identify(context: LDContext): void { + return this.client.identify(context); + } + + flush(callback?: (err: Error, res: boolean) => void): Promise { + return this.client.flush(callback); + } +} diff --git a/platform-node/src/api/LDClient.ts b/platform-node/src/api/LDClient.ts new file mode 100644 index 0000000000..e36c8d94c7 --- /dev/null +++ b/platform-node/src/api/LDClient.ts @@ -0,0 +1,22 @@ +import { LDClient as LDClientCommon } from '@launchdarkly/js-server-sdk-common'; +import EventEmitter from 'events'; +import { BigSegmentStoreStatusProvider } from './interfaces/BigSegmentStoreStatusProvider'; + +/** + * The LaunchDarkly SDK client object. + * + * Create this object with [[init]]. Applications should configure the client at startup time and + * continue to use it throughout the lifetime of the application, rather than creating instances on + * the fly. + * + */ +export interface LDClient extends LDClientCommon, EventEmitter { + /** + * A mechanism for tracking the status of a Big Segment store. + * + * This object has methods for checking whether the Big Segment store is (as far as the SDK + * knows) currently operational and tracking changes in this status. See + * {@link interfaces.BigSegmentStoreStatusProvider} for more about this functionality. + */ + readonly bigSegmentStoreStatusProvider: BigSegmentStoreStatusProvider; +} diff --git a/platform-node/src/api/index.ts b/platform-node/src/api/index.ts new file mode 100644 index 0000000000..5ace2345c0 --- /dev/null +++ b/platform-node/src/api/index.ts @@ -0,0 +1,2 @@ +export * from './LDClient'; +export * from './interfaces'; diff --git a/server-sdk-common/src/api/interfaces/BigSegmentStoreStatusProvider.ts b/platform-node/src/api/interfaces/BigSegmentStoreStatusProvider.ts similarity index 86% rename from server-sdk-common/src/api/interfaces/BigSegmentStoreStatusProvider.ts rename to platform-node/src/api/interfaces/BigSegmentStoreStatusProvider.ts index 45a512db30..0ae8846d8a 100644 --- a/server-sdk-common/src/api/interfaces/BigSegmentStoreStatusProvider.ts +++ b/platform-node/src/api/interfaces/BigSegmentStoreStatusProvider.ts @@ -1,5 +1,5 @@ -import { EventEmitter } from 'events'; -import { BigSegmentStoreStatus } from './BigSegmentStoreStatus'; +import EventEmitter from 'events'; +import { interfaces } from '@launchdarkly/js-server-sdk-common'; /** * An interface for querying the status of a Big Segment store. @@ -27,12 +27,12 @@ export interface BigSegmentStoreStatusProvider extends EventEmitter { * @returns a {@link BigSegmentStoreStatus}, or `undefined` if the SDK has not yet queried the * Big Segment store status */ - getStatus(): BigSegmentStoreStatus | undefined; + getStatus(): interfaces.BigSegmentStoreStatus | undefined; /** * Gets the current status of the store, querying it if the status has not already been queried. * * @returns a Promise for the status of the store */ - requireStatus(): Promise; + requireStatus(): Promise; } diff --git a/platform-node/src/api/interfaces/index.ts b/platform-node/src/api/interfaces/index.ts new file mode 100644 index 0000000000..aaf6c352a2 --- /dev/null +++ b/platform-node/src/api/interfaces/index.ts @@ -0,0 +1 @@ +export * from './BigSegmentStoreStatusProvider'; \ No newline at end of file diff --git a/platform-node/src/index.ts b/platform-node/src/index.ts index 897f20d7d7..0567f419eb 100644 --- a/platform-node/src/index.ts +++ b/platform-node/src/index.ts @@ -1,24 +1,3 @@ -/* eslint-disable no-console */ -// This file contains temporary code for testing. -import NodePlatform from './NodePlatform'; +export * from '@launchdarkly/js-server-sdk-common'; -const platform = new NodePlatform({ - tlsParams: { - checkServerIdentity: (name) => { - console.log('GOT A IDENTITY CHECK', name); - return undefined; - }, - }, -}); - -async function runTest() { - console.log('Making request'); - const res = await platform.requests.fetch('https://www.google.com'); - console.log('Got res', res); - const body = await res.text(); - console.log('Body', body); -} - -runTest(); - -console.log(platform.info.sdkData()); +export { LDClient } from './api'; diff --git a/platform-node/src/NodeCrypto.ts b/platform-node/src/platform/NodeCrypto.ts similarity index 100% rename from platform-node/src/NodeCrypto.ts rename to platform-node/src/platform/NodeCrypto.ts diff --git a/platform-node/src/NodeFilesystem.ts b/platform-node/src/platform/NodeFilesystem.ts similarity index 100% rename from platform-node/src/NodeFilesystem.ts rename to platform-node/src/platform/NodeFilesystem.ts diff --git a/platform-node/src/NodeInfo.ts b/platform-node/src/platform/NodeInfo.ts similarity index 94% rename from platform-node/src/NodeInfo.ts rename to platform-node/src/platform/NodeInfo.ts index 72ee63aeab..03d9e198ca 100644 --- a/platform-node/src/NodeInfo.ts +++ b/platform-node/src/platform/NodeInfo.ts @@ -4,7 +4,7 @@ import { PlatformData, SdkData } from '@launchdarkly/js-server-sdk-common/dist/p import * as os from 'os'; -import * as packageJson from '../package.json'; +import * as packageJson from '../../package.json'; function processPlatformName(name: string): string { switch (name) { diff --git a/platform-node/src/NodePlatform.ts b/platform-node/src/platform/NodePlatform.ts similarity index 100% rename from platform-node/src/NodePlatform.ts rename to platform-node/src/platform/NodePlatform.ts diff --git a/platform-node/src/NodeRequests.ts b/platform-node/src/platform/NodeRequests.ts similarity index 100% rename from platform-node/src/NodeRequests.ts rename to platform-node/src/platform/NodeRequests.ts diff --git a/platform-node/src/NodeResponse.ts b/platform-node/src/platform/NodeResponse.ts similarity index 100% rename from platform-node/src/NodeResponse.ts rename to platform-node/src/platform/NodeResponse.ts diff --git a/server-sdk-common/src/BigSegmentStatusProvider.ts b/server-sdk-common/src/BigSegmentStatusProvider.ts new file mode 100644 index 0000000000..3bf42038ae --- /dev/null +++ b/server-sdk-common/src/BigSegmentStatusProvider.ts @@ -0,0 +1,36 @@ +/* eslint-disable class-methods-use-this */ +import { BigSegmentStoreStatus } from './api/interfaces'; + +type StatusHandler = (status: BigSegmentStoreStatus) => void; + +export default class BigSegmentStoreStatusProvider { + private onStatus?: StatusHandler; + + /** + * Gets the current status of the store, if known. + * + * @returns a {@link BigSegmentStoreStatus}, or `undefined` if the SDK has not yet queried the + * Big Segment store status + */ + getStatus(): BigSegmentStoreStatus | undefined { + return undefined; + } + + /** + * Gets the current status of the store, querying it if the status has not already been queried. + * + * @returns a Promise for the status of the store + */ + async requireStatus(): Promise { + return Promise.reject(); + } + + /** + * Set the status handler. Only one handler can be registered and this will replace the existing + * handler. + * @param handler + */ + setStatusHandler(handler: StatusHandler) { + this.onStatus = handler; + } +} diff --git a/server-sdk-common/src/LDClientImpl.ts b/server-sdk-common/src/LDClientImpl.ts new file mode 100644 index 0000000000..27d1b92050 --- /dev/null +++ b/server-sdk-common/src/LDClientImpl.ts @@ -0,0 +1,109 @@ +/* eslint-disable @typescript-eslint/no-unused-vars */ +/* eslint-disable class-methods-use-this */ +import { + LDClient, LDContext, LDEvaluationDetail, LDFlagsState, LDFlagsStateOptions, +} from './api'; +import BigSegmentStoreStatusProvider from './BigSegmentStatusProvider'; +import { Platform } from './platform'; + +enum EventTypes { + /** + * Emitted on errors other than a failure to initialize. + */ + Error = 'error', + /** + * Emitted when the SDK fails to initialize. + */ + Failed = 'failed', + /** + * Emitted when the SDK is ready to use. + */ + Ready = 'ready', +} + +enum InitState { + Initializing, + Initialized, + Failed, +} + +export default class LDClientImpl implements LDClient { + private platform: Platform; + + private initState: InitState = InitState.Initializing; + + /** + * Intended for use by platform specific client implementations. + * + * It is not included in the main interface because it requires the use of + * a platform event system. For node this would be an EventEmitter, for other + * platforms it would likely be an EventTarget. + */ + public bigSegmentStoreStatusProvider = new BigSegmentStoreStatusProvider(); + + constructor(targetPlatform: Platform) { + this.platform = targetPlatform; + } + + initialized(): boolean { + return this.initState === InitState.Initialized; + } + + waitForInitialization(): Promise { + throw new Error('Method not implemented.'); + } + + variation( + key: string, + context: LDContext, + defaultValue: any, + callback?: (err: any, res: any) => void, + ): Promise { + throw new Error('Method not implemented.'); + } + + variationDetail( + key: string, + context: LDContext, + defaultValue: any, + callback?: (err: any, res: LDEvaluationDetail) => void, + ): Promise { + throw new Error('Method not implemented.'); + } + + allFlagsState( + context: LDContext, + options?: LDFlagsStateOptions, + callback?: (err: Error, res: LDFlagsState) => void, + ): Promise { + throw new Error('Method not implemented.'); + } + + secureModeHash(context: LDContext): string { + throw new Error('Method not implemented.'); + } + + close(): void { + throw new Error('Method not implemented.'); + } + + isOffline(): boolean { + throw new Error('Method not implemented.'); + } + + track(key: string, context: LDContext, data?: any, metricValue?: number): void { + throw new Error('Method not implemented.'); + } + + identify(context: LDContext): void { + throw new Error('Method not implemented.'); + } + + flush(callback?: (err: Error, res: boolean) => void): Promise { + throw new Error('Method not implemented.'); + } + + on(event: string | symbol, listener: (...args: any[]) => void): this { + throw new Error('Method not implemented.'); + } +} diff --git a/server-sdk-common/src/api/LDClient.ts b/server-sdk-common/src/api/LDClient.ts index 6a53d11fa4..ada8704a04 100644 --- a/server-sdk-common/src/api/LDClient.ts +++ b/server-sdk-common/src/api/LDClient.ts @@ -1,6 +1,4 @@ -import { EventEmitter } from 'events'; import { LDContext } from './LDContext'; -import { BigSegmentStoreStatusProvider } from './interfaces/BigSegmentStoreStatusProvider'; import { LDEvaluationDetail } from './data/LDEvaluationDetail'; import { LDFlagsState } from './data/LDFlagsState'; import { LDFlagsStateOptions } from './data/LDFlagsStateOptions'; @@ -13,15 +11,8 @@ import { LDFlagValue } from './data/LDFlagValue'; * continue to use it throughout the lifetime of the application, rather than creating instances on * the fly. * - * Note that `LDClient` inherits from `EventEmitter`, so you can use the standard `on()`, `once()`, - * and `off()` methods to receive events. The standard `EventEmitter` methods are not documented - * here; see the - * {@link https://nodejs.org/api/events.html#events_class_eventemitter|Node API documentation}. For - * a description of events you can listen for, see [[on]]. - * - * @see {@link https://docs.launchdarkly.com/sdk/server-side/node-js|SDK Reference Guide} */ -export interface LDClient extends EventEmitter { +export interface LDClient { /** * Tests whether the client has completed initialization. * @@ -241,23 +232,9 @@ export interface LDClient extends EventEmitter { */ flush(callback?: (err: Error, res: boolean) => void): Promise; - /** - * A mechanism for tracking the status of a Big Segment store. - * - * This object has methods for checking whether the Big Segment store is (as far as the SDK - * knows) currently operational and tracking changes in this status. See - * {@link interfaces.BigSegmentStoreStatusProvider} for more about this functionality. - */ - readonly bigSegmentStoreStatusProvider: BigSegmentStoreStatusProvider; - /** * Registers an event listener that will be called when the client triggers some type of event. * - * This is the standard `on` method inherited from Node's `EventEmitter`; see the - * {@link https://nodejs.org/api/events.html#events_class_eventemitter|Node API docs} for more - * details on how to manage event listeners. Here is a description of the event types defined by - * `LDClient`. - * * - `"ready"`: Sent only once, when the client has successfully connected to LaunchDarkly. * Alternately, you can detect this with [[waitForInitialization]]. * - `"failed"`: Sent only once, if the client has permanently failed to connect to LaunchDarkly. @@ -275,24 +252,4 @@ export interface LDClient extends EventEmitter { * @param listener the function to call when the event happens */ on(event: string | symbol, listener: (...args: any[]) => void): this; - - // The following are symbols that LDClient inherits from EventEmitter, which we are declaring - // again here only so that we can use @ignore to exclude them from the generated docs. - // Unfortunately it does not seem possible to exclude these inherited methods en masse without - // using a Typedoc plugin. - /** @ignore */ addListener(event: string | symbol, listener: (...args: any[]) => void): this; - /** @ignore */ emit(event: string | symbol, ...args: any[]): boolean; - /** @ignore */ eventNames(): Array; - /** @ignore */ getMaxListeners(): number; - /** @ignore */ listenerCount(type: string | symbol): number; - /** @ignore */ listeners(event: string | symbol): Function[]; - /** @ignore */ prependListener(event: string | symbol, listener: (...args: any[]) => void): this; - /** @ignore */ prependOnceListener(event: string | symbol, listener: - (...args: any[]) => void): this; - /** @ignore */ rawListeners(event: string | symbol): Function[]; - /** @ignore */ removeAllListeners(event?: string | symbol): this; - /** @ignore */ removeListener(event: string | symbol, listener: (...args: any[]) => void): this; - /** @ignore */ setMaxListeners(n: number): this; - /** @ignore */ once(event: string | symbol, listener: (...args: any[]) => void): this; - /** @ignore */ off(event: string | symbol, listener: (...args: any[]) => void): this; } diff --git a/server-sdk-common/src/api/interfaces/index.ts b/server-sdk-common/src/api/interfaces/index.ts index 827cee6e7b..1a4a141cde 100644 --- a/server-sdk-common/src/api/interfaces/index.ts +++ b/server-sdk-common/src/api/interfaces/index.ts @@ -2,7 +2,6 @@ export * from './BigSegmentStore'; export * from './BigSegmentStoreMembership'; export * from './BigSegmentStoreMetadata'; export * from './BigSegmentStoreStatus'; -export * from './BigSegmentStoreStatusProvider'; export * from './DataCollection'; export * from './DataKind'; export * from './FullDataSet'; diff --git a/server-sdk-common/src/index.ts b/server-sdk-common/src/index.ts index e349fcd5f6..2a3603259f 100644 --- a/server-sdk-common/src/index.ts +++ b/server-sdk-common/src/index.ts @@ -1,2 +1,6 @@ +import LDClientImpl from './LDClientImpl'; + export * as platform from './platform'; export * from './api'; + +export { LDClientImpl }; From 0b470eab6d55b529f666bbd953458bb632c0c35e Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 19 May 2022 16:31:17 -0700 Subject: [PATCH 02/39] Update index exports. --- platform-node/src/index.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/platform-node/src/index.ts b/platform-node/src/index.ts index 0567f419eb..c1fecbeeab 100644 --- a/platform-node/src/index.ts +++ b/platform-node/src/index.ts @@ -1,3 +1,9 @@ +import LDClientImpl from './LDClientNode'; + export * from '@launchdarkly/js-server-sdk-common'; -export { LDClient } from './api'; +// To replace the exports from `export *` we need to name them. +// So the below exports replace them with the Node specific variants. + +export { LDClient, BigSegmentStoreStatusProvider } from './api'; +export { LDClientImpl }; From 93fbac63cf6625d2e03a719c4023aa0a2458ec2c Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 19 May 2022 16:39:43 -0700 Subject: [PATCH 03/39] Add an extra blank line. --- platform-node/src/api/interfaces/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform-node/src/api/interfaces/index.ts b/platform-node/src/api/interfaces/index.ts index aaf6c352a2..153474b58c 100644 --- a/platform-node/src/api/interfaces/index.ts +++ b/platform-node/src/api/interfaces/index.ts @@ -1 +1 @@ -export * from './BigSegmentStoreStatusProvider'; \ No newline at end of file +export * from './BigSegmentStoreStatusProvider'; From 8be08daaa960319bae0285db3ed629b4b3ab1267 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 23 May 2022 10:20:03 -0700 Subject: [PATCH 04/39] Attempt using mixin for event emitter. --- platform-node/__tests__/NodeInfo_test.ts | 2 +- platform-node/__tests__/NodeRequests_test.ts | 2 +- .../src/BigSegmentsStoreStatusProviderNode.ts | 29 ++---- platform-node/src/Emits.ts | 74 ++++++++++++++ platform-node/src/LDClientNode.ts | 97 +++---------------- platform-node/src/api/LDClient.ts | 6 +- ...rovider.ts => BigSegmentStatusProvider.ts} | 2 +- platform-node/src/api/interfaces/index.ts | 2 +- platform-node/src/index.ts | 5 +- ...der.ts => BigSegmentStatusProviderImpl.ts} | 6 +- server-sdk-common/src/LDClientImpl.ts | 4 +- .../BigSegmentStoreStatusProvider.ts | 30 ++++++ server-sdk-common/src/api/interfaces/index.ts | 1 + server-sdk-common/src/index.ts | 2 + 14 files changed, 141 insertions(+), 121 deletions(-) create mode 100644 platform-node/src/Emits.ts rename platform-node/src/api/interfaces/{BigSegmentStoreStatusProvider.ts => BigSegmentStatusProvider.ts} (95%) rename server-sdk-common/src/{BigSegmentStatusProvider.ts => BigSegmentStatusProviderImpl.ts} (78%) create mode 100644 server-sdk-common/src/api/interfaces/BigSegmentStoreStatusProvider.ts diff --git a/platform-node/__tests__/NodeInfo_test.ts b/platform-node/__tests__/NodeInfo_test.ts index 18c527e4ba..0567413c76 100644 --- a/platform-node/__tests__/NodeInfo_test.ts +++ b/platform-node/__tests__/NodeInfo_test.ts @@ -1,5 +1,5 @@ import * as os from 'os'; -import NodeInfo from '../src/NodeInfo'; +import NodeInfo from '../src/platform/NodeInfo'; describe('given an information instance', () => { const info = new NodeInfo(); diff --git a/platform-node/__tests__/NodeRequests_test.ts b/platform-node/__tests__/NodeRequests_test.ts index bf21e2358e..4cdb820811 100644 --- a/platform-node/__tests__/NodeRequests_test.ts +++ b/platform-node/__tests__/NodeRequests_test.ts @@ -1,6 +1,6 @@ import * as http from 'http'; -import NodeRequests from '../src/NodeRequests'; +import NodeRequests from '../src/platform/NodeRequests'; const PORT = '3000'; const TEXT_RESPONSE = 'Test Text'; diff --git a/platform-node/src/BigSegmentsStoreStatusProviderNode.ts b/platform-node/src/BigSegmentsStoreStatusProviderNode.ts index 0a4b03cd5c..24ccdc61fa 100644 --- a/platform-node/src/BigSegmentsStoreStatusProviderNode.ts +++ b/platform-node/src/BigSegmentsStoreStatusProviderNode.ts @@ -1,26 +1,9 @@ -import { LDClientImpl } from '@launchdarkly/js-server-sdk-common'; -import { BigSegmentStoreStatus } from '@launchdarkly/js-server-sdk-common/dist/api/interfaces'; +import { BigSegmentStoreStatusProviderImpl } from '@launchdarkly/js-server-sdk-common'; import EventEmitter from 'events'; -import { BigSegmentStoreStatusProvider } from './api/interfaces/BigSegmentStoreStatusProvider'; +import { Emits } from './Emits'; -export default class BigSegmentStoreStatusProviderNode - extends EventEmitter implements BigSegmentStoreStatusProvider { - clientBase: LDClientImpl; - - constructor(clientBase: LDClientImpl) { - super(); - this.clientBase = clientBase; - - clientBase.bigSegmentStoreStatusProvider.setStatusHandler((status) => { - this.emit('change', status); - }); - } - - getStatus(): BigSegmentStoreStatus | undefined { - return this.clientBase.bigSegmentStoreStatusProvider.getStatus(); - } - - requireStatus(): Promise { - return this.clientBase.bigSegmentStoreStatusProvider.requireStatus(); - } +class BigSegmentStoreStatusProviderNode extends BigSegmentStoreStatusProviderImpl { + emitter: EventEmitter = new EventEmitter(); } + +export default Emits(BigSegmentStoreStatusProviderNode); diff --git a/platform-node/src/Emits.ts b/platform-node/src/Emits.ts new file mode 100644 index 0000000000..57addc03f9 --- /dev/null +++ b/platform-node/src/Emits.ts @@ -0,0 +1,74 @@ +import EventEmitter from 'events'; + +export type EventableConstructor = new (...args: any[]) => T; +export type Eventable = EventableConstructor<{ emitter: EventEmitter }>; + +export function Emits(Base: TBase) { + return class WithEvents extends Base { + addListener(eventName: string | symbol, listener: (...args: any[]) => void): this { + this.emitter.addListener(eventName, listener); + return this; + } + + once(eventName: string | symbol, listener: (...args: any[]) => void): this { + this.emitter.once(eventName, listener); + return this; + } + + removeListener(eventName: string | symbol, listener: ( + ...args: any[]) => void): this { + this.emitter.removeListener(eventName, listener); + return this; + } + + off(eventName: string | symbol, listener: (...args: any) => void): this { + this.emitter.off(eventName, listener); + return this; + } + + removeAllListeners(event?: string | symbol): this { + this.emitter.removeAllListeners(event); + return this; + } + + setMaxListeners(n: number): this { + this.emitter.setMaxListeners(n); + return this; + } + + getMaxListeners(): number { + return this.emitter.getMaxListeners(); + } + + listeners(eventName: string | symbol): Function[] { + return this.emitter.listeners(eventName); + } + + rawListeners(eventName: string | symbol): Function[] { + return this.emitter.rawListeners(eventName); + } + + emit(eventName: string | symbol, ...args: any[]): boolean { + return this.emitter.emit(eventName, args); + } + + listenerCount(eventName: string | symbol): number { + return this.emitter.listenerCount(eventName); + } + + prependListener(eventName: string | symbol, listener: ( + ...args: any[]) => void): this { + this.emitter.prependListener(eventName, listener); + return this; + } + + prependOnceListener(eventName: string | symbol, listener: (...args: any[]) => void): this { + this.emitter.prependOnceListener(eventName, listener); + return this; + } + + eventNames(): (string | symbol)[] { + return this.emitter.eventNames(); + } + }; +} \ No newline at end of file diff --git a/platform-node/src/LDClientNode.ts b/platform-node/src/LDClientNode.ts index b34d16660f..6204e6cfb0 100644 --- a/platform-node/src/LDClientNode.ts +++ b/platform-node/src/LDClientNode.ts @@ -1,96 +1,25 @@ /* eslint-disable @typescript-eslint/no-unused-vars */ /* eslint-disable class-methods-use-this */ +// eslint-disable-next-line max-classes-per-file import { - LDClientImpl, LDContext, LDEvaluationDetail, LDFlagsState, LDFlagsStateOptions, LDOptions, + LDClientImpl, LDOptions, } from '@launchdarkly/js-server-sdk-common'; import EventEmitter from 'events'; -import { LDClient } from './api/LDClient'; -import BigSegmentStoreStatusProviderNode from './BigSegmentsStoreStatusProviderNode'; -import { BigSegmentStoreStatusProvider } from './api/interfaces/BigSegmentStoreStatusProvider'; import NodePlatform from './platform/NodePlatform'; +import { Emits } from './Emits'; +import BigSegmentStoreStatusProviderNode from './BigSegmentsStoreStatusProviderNode'; -export default class LDClientNode extends EventEmitter implements LDClient { - /** - * The LDClientImpl is included through composition, instead of inheritance, - * for a couple of reasons. - * 1. This allows the node client to implement EventEmitter, where otherwise - * it would be unable to from single inheritance. EventEmitter is not included - * in common because it is node specific. - * - * 2. This allows conversions of some types compared to the base implementation. - * Such as returning a node `LDClient` instead of the base `LDClient` interface. - * This interface extends EventEmitter to support #1. - */ - private client: LDClientImpl; - - constructor(options: LDOptions) { - super(); - // TODO: Remember to parse the options before providing them to the platform. - this.client = new LDClientImpl(new NodePlatform(options)); - // Extend the BigSegmentStoreStatusProvider from the common client to allow - // for use of the event emitter. - this.bigSegmentStoreStatusProvider = new BigSegmentStoreStatusProviderNode(this.client); - } - - bigSegmentStoreStatusProvider: BigSegmentStoreStatusProvider; - - initialized(): boolean { - return this.client.initialized(); - } - - async waitForInitialization(): Promise { - await this.client.waitForInitialization(); - return this; - } - - variation( - key: string, - context: LDContext, - defaultValue: any, - callback?: (err: any, res: any) => void, - ): Promise { - return this.client.variation(key, context, defaultValue, callback); - } - - variationDetail( - key: string, - context: LDContext, - defaultValue: any, - callback?: (err: any, res: LDEvaluationDetail) => void, - ): Promise { - return this.client.variationDetail(key, context, defaultValue, callback); - } - - allFlagsState( - context: LDContext, - options?: LDFlagsStateOptions, - callback?: (err: Error, res: LDFlagsState) => void, - ): Promise { - return this.client.allFlagsState(context, options, callback); - } - - secureModeHash(context: LDContext): string { - return this.client.secureModeHash(context); - } - - close(): void { - this.client.close(); - } - - isOffline(): boolean { - return this.client.isOffline(); - } - - track(key: string, context: LDContext, data?: any, metricValue?: number): void { - return this.client.track(key, context, data, metricValue); - } +class LDClientNode extends LDClientImpl { + emitter: EventEmitter = new EventEmitter(); - identify(context: LDContext): void { - return this.client.identify(context); - } + override bigSegmentStoreStatusProvider: + InstanceType; - flush(callback?: (err: Error, res: boolean) => void): Promise { - return this.client.flush(callback); + constructor(options: LDOptions) { + super(new NodePlatform(options)); + this.bigSegmentStoreStatusProvider = new BigSegmentStoreStatusProviderNode(); } } + +export default Emits(LDClientNode); diff --git a/platform-node/src/api/LDClient.ts b/platform-node/src/api/LDClient.ts index e36c8d94c7..8e2488dc68 100644 --- a/platform-node/src/api/LDClient.ts +++ b/platform-node/src/api/LDClient.ts @@ -1,6 +1,6 @@ import { LDClient as LDClientCommon } from '@launchdarkly/js-server-sdk-common'; -import EventEmitter from 'events'; -import { BigSegmentStoreStatusProvider } from './interfaces/BigSegmentStoreStatusProvider'; +import { BigSegmentStoreStatusProvider } from './interfaces'; + /** * The LaunchDarkly SDK client object. @@ -10,7 +10,7 @@ import { BigSegmentStoreStatusProvider } from './interfaces/BigSegmentStoreStatu * the fly. * */ -export interface LDClient extends LDClientCommon, EventEmitter { +export interface LDClient extends LDClientCommon { /** * A mechanism for tracking the status of a Big Segment store. * diff --git a/platform-node/src/api/interfaces/BigSegmentStoreStatusProvider.ts b/platform-node/src/api/interfaces/BigSegmentStatusProvider.ts similarity index 95% rename from platform-node/src/api/interfaces/BigSegmentStoreStatusProvider.ts rename to platform-node/src/api/interfaces/BigSegmentStatusProvider.ts index 0ae8846d8a..13678396a4 100644 --- a/platform-node/src/api/interfaces/BigSegmentStoreStatusProvider.ts +++ b/platform-node/src/api/interfaces/BigSegmentStatusProvider.ts @@ -20,7 +20,7 @@ import { interfaces } from '@launchdarkly/js-server-sdk-common'; * type of the status change event is `"change"`, and its value is the same value that would be * returned by {@link getStatus}. */ -export interface BigSegmentStoreStatusProvider extends EventEmitter { +export interface BigSegmentStoreStatusProvider { /** * Gets the current status of the store, if known. * diff --git a/platform-node/src/api/interfaces/index.ts b/platform-node/src/api/interfaces/index.ts index 153474b58c..1ad24aaa45 100644 --- a/platform-node/src/api/interfaces/index.ts +++ b/platform-node/src/api/interfaces/index.ts @@ -1 +1 @@ -export * from './BigSegmentStoreStatusProvider'; +export * from '@launchdarkly/js-server-sdk-common/src/api/interfaces/BigSegmentStoreStatusProvider'; diff --git a/platform-node/src/index.ts b/platform-node/src/index.ts index c1fecbeeab..a51e695836 100644 --- a/platform-node/src/index.ts +++ b/platform-node/src/index.ts @@ -1,9 +1,10 @@ import LDClientImpl from './LDClientNode'; +import BigSegmentStoreStatusProviderNode from './BigSegmentsStoreStatusProviderNode'; export * from '@launchdarkly/js-server-sdk-common'; // To replace the exports from `export *` we need to name them. // So the below exports replace them with the Node specific variants. -export { LDClient, BigSegmentStoreStatusProvider } from './api'; -export { LDClientImpl }; +export { LDClient } from './api'; +export { LDClientImpl, BigSegmentStoreStatusProviderNode }; diff --git a/server-sdk-common/src/BigSegmentStatusProvider.ts b/server-sdk-common/src/BigSegmentStatusProviderImpl.ts similarity index 78% rename from server-sdk-common/src/BigSegmentStatusProvider.ts rename to server-sdk-common/src/BigSegmentStatusProviderImpl.ts index 3bf42038ae..ddc9e184d4 100644 --- a/server-sdk-common/src/BigSegmentStatusProvider.ts +++ b/server-sdk-common/src/BigSegmentStatusProviderImpl.ts @@ -1,9 +1,9 @@ /* eslint-disable class-methods-use-this */ -import { BigSegmentStoreStatus } from './api/interfaces'; +import { BigSegmentStoreStatusProvider, BigSegmentStoreStatus } from './api/interfaces'; type StatusHandler = (status: BigSegmentStoreStatus) => void; -export default class BigSegmentStoreStatusProvider { +export default class BigSegmentStoreStatusProviderImpl implements BigSegmentStoreStatusProvider { private onStatus?: StatusHandler; /** @@ -30,7 +30,7 @@ export default class BigSegmentStoreStatusProvider { * handler. * @param handler */ - setStatusHandler(handler: StatusHandler) { + protected setStatusHandler(handler: StatusHandler) { this.onStatus = handler; } } diff --git a/server-sdk-common/src/LDClientImpl.ts b/server-sdk-common/src/LDClientImpl.ts index 27d1b92050..99a6f0dcb1 100644 --- a/server-sdk-common/src/LDClientImpl.ts +++ b/server-sdk-common/src/LDClientImpl.ts @@ -3,7 +3,7 @@ import { LDClient, LDContext, LDEvaluationDetail, LDFlagsState, LDFlagsStateOptions, } from './api'; -import BigSegmentStoreStatusProvider from './BigSegmentStatusProvider'; +import BigSegmentStoreStatusProvider from './BigSegmentStatusProviderImpl'; import { Platform } from './platform'; enum EventTypes { @@ -39,7 +39,7 @@ export default class LDClientImpl implements LDClient { * a platform event system. For node this would be an EventEmitter, for other * platforms it would likely be an EventTarget. */ - public bigSegmentStoreStatusProvider = new BigSegmentStoreStatusProvider(); + bigSegmentStoreStatusProvider = new BigSegmentStoreStatusProvider(); constructor(targetPlatform: Platform) { this.platform = targetPlatform; diff --git a/server-sdk-common/src/api/interfaces/BigSegmentStoreStatusProvider.ts b/server-sdk-common/src/api/interfaces/BigSegmentStoreStatusProvider.ts new file mode 100644 index 0000000000..e181ecfc47 --- /dev/null +++ b/server-sdk-common/src/api/interfaces/BigSegmentStoreStatusProvider.ts @@ -0,0 +1,30 @@ +import { BigSegmentStoreStatus } from './BigSegmentStoreStatus'; + +/** + * An interface for querying the status of a Big Segment store. + * + * The Big Segment store is the component that receives information about Big Segments, normally + * from a database populated by the LaunchDarkly Relay Proxy. Big Segments are a specific type of + * user segments. For more information, read the LaunchDarkly documentation: + * https://docs.launchdarkly.com/home/users/big-segments + * + * An implementation of this interface is returned by + * {@link LDClient.bigSegmentStoreStatusProvider}. Application code never needs to implement this + * interface. + */ +export interface BigSegmentStoreStatusProvider { + /** + * Gets the current status of the store, if known. + * + * @returns a {@link BigSegmentStoreStatus}, or `undefined` if the SDK has not yet queried the + * Big Segment store status + */ + getStatus(): BigSegmentStoreStatus | undefined; + + /** + * Gets the current status of the store, querying it if the status has not already been queried. + * + * @returns a Promise for the status of the store + */ + requireStatus(): Promise; +} diff --git a/server-sdk-common/src/api/interfaces/index.ts b/server-sdk-common/src/api/interfaces/index.ts index 1a4a141cde..73c35c13b5 100644 --- a/server-sdk-common/src/api/interfaces/index.ts +++ b/server-sdk-common/src/api/interfaces/index.ts @@ -10,3 +10,4 @@ export * from './PersistentDataStore'; export * from './PersistentDataStoreBase'; export * from './PersistentDataStoreNonAtomic'; export * from './VersionedData'; +export * from './BigSegmentStoreStatusProvider'; diff --git a/server-sdk-common/src/index.ts b/server-sdk-common/src/index.ts index 2a3603259f..432a9d7a4c 100644 --- a/server-sdk-common/src/index.ts +++ b/server-sdk-common/src/index.ts @@ -1,6 +1,8 @@ import LDClientImpl from './LDClientImpl'; +import BigSegmentStoreStatusProviderImpl from './BigSegmentStatusProviderImpl'; export * as platform from './platform'; export * from './api'; export { LDClientImpl }; +export { BigSegmentStoreStatusProviderImpl }; From 5302683ee1c73a584c3455bf3aaa96d584cf8bbe Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 23 May 2022 10:24:11 -0700 Subject: [PATCH 05/39] Add implementation for emitting events for big segment status. --- .../src/BigSegmentsStoreStatusProviderNode.ts | 6 +++++- .../src/BigSegmentStatusProviderImpl.ts | 15 +++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/platform-node/src/BigSegmentsStoreStatusProviderNode.ts b/platform-node/src/BigSegmentsStoreStatusProviderNode.ts index 24ccdc61fa..ebd4e1005c 100644 --- a/platform-node/src/BigSegmentsStoreStatusProviderNode.ts +++ b/platform-node/src/BigSegmentsStoreStatusProviderNode.ts @@ -1,9 +1,13 @@ -import { BigSegmentStoreStatusProviderImpl } from '@launchdarkly/js-server-sdk-common'; +import { BigSegmentStoreStatusProviderImpl, interfaces } from '@launchdarkly/js-server-sdk-common'; import EventEmitter from 'events'; import { Emits } from './Emits'; class BigSegmentStoreStatusProviderNode extends BigSegmentStoreStatusProviderImpl { emitter: EventEmitter = new EventEmitter(); + + override dispatch(eventType: string, status: interfaces.BigSegmentStoreStatus) { + this.emitter.emit(eventType, status); + } } export default Emits(BigSegmentStoreStatusProviderNode); diff --git a/server-sdk-common/src/BigSegmentStatusProviderImpl.ts b/server-sdk-common/src/BigSegmentStatusProviderImpl.ts index ddc9e184d4..fa457409e8 100644 --- a/server-sdk-common/src/BigSegmentStatusProviderImpl.ts +++ b/server-sdk-common/src/BigSegmentStatusProviderImpl.ts @@ -1,11 +1,7 @@ /* eslint-disable class-methods-use-this */ import { BigSegmentStoreStatusProvider, BigSegmentStoreStatus } from './api/interfaces'; -type StatusHandler = (status: BigSegmentStoreStatus) => void; - export default class BigSegmentStoreStatusProviderImpl implements BigSegmentStoreStatusProvider { - private onStatus?: StatusHandler; - /** * Gets the current status of the store, if known. * @@ -26,11 +22,10 @@ export default class BigSegmentStoreStatusProviderImpl implements BigSegmentStor } /** - * Set the status handler. Only one handler can be registered and this will replace the existing - * handler. - * @param handler + * This should be overridden by derived implementations. + * @param eventType + * @param status */ - protected setStatusHandler(handler: StatusHandler) { - this.onStatus = handler; - } + // eslint-disable-next-line @typescript-eslint/no-unused-vars + dispatch(eventType: string, status: BigSegmentStoreStatus) {} } From 263fccdefcd92326224d5274d8cb84c8936369b0 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 23 May 2022 10:25:45 -0700 Subject: [PATCH 06/39] Add documentation. --- platform-node/src/Emits.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/platform-node/src/Emits.ts b/platform-node/src/Emits.ts index 57addc03f9..7cbbf919f6 100644 --- a/platform-node/src/Emits.ts +++ b/platform-node/src/Emits.ts @@ -3,6 +3,12 @@ import EventEmitter from 'events'; export type EventableConstructor = new (...args: any[]) => T; export type Eventable = EventableConstructor<{ emitter: EventEmitter }>; +/** + * Adds the implementation of an event emitter to something that contains + * a field of `emitter` with type `EventEmitter`. + * @param Base The class to derive the mixin from. + * @returns A class extending the base with an event emitter. + */ export function Emits(Base: TBase) { return class WithEvents extends Base { addListener(eventName: string | symbol, listener: (...args: any[]) => void): this { @@ -71,4 +77,4 @@ export function Emits(Base: TBase) { return this.emitter.eventNames(); } }; -} \ No newline at end of file +} From 831b964933bddfa9da2fb3c8ff81a551ff1706a7 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 23 May 2022 10:34:32 -0700 Subject: [PATCH 07/39] Fix lint and interface hierarchy. --- platform-node/src/LDClientNode.ts | 3 --- platform-node/src/api/LDClient.ts | 4 ++-- platform-node/src/api/interfaces/BigSegmentStatusProvider.ts | 2 +- platform-node/src/api/interfaces/index.ts | 2 +- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/platform-node/src/LDClientNode.ts b/platform-node/src/LDClientNode.ts index 6204e6cfb0..8079063093 100644 --- a/platform-node/src/LDClientNode.ts +++ b/platform-node/src/LDClientNode.ts @@ -1,6 +1,3 @@ -/* eslint-disable @typescript-eslint/no-unused-vars */ -/* eslint-disable class-methods-use-this */ -// eslint-disable-next-line max-classes-per-file import { LDClientImpl, LDOptions, } from '@launchdarkly/js-server-sdk-common'; diff --git a/platform-node/src/api/LDClient.ts b/platform-node/src/api/LDClient.ts index 8e2488dc68..5c7f331556 100644 --- a/platform-node/src/api/LDClient.ts +++ b/platform-node/src/api/LDClient.ts @@ -1,7 +1,7 @@ import { LDClient as LDClientCommon } from '@launchdarkly/js-server-sdk-common'; +import EventEmitter from 'events'; import { BigSegmentStoreStatusProvider } from './interfaces'; - /** * The LaunchDarkly SDK client object. * @@ -10,7 +10,7 @@ import { BigSegmentStoreStatusProvider } from './interfaces'; * the fly. * */ -export interface LDClient extends LDClientCommon { +export interface LDClient extends LDClientCommon, EventEmitter { /** * A mechanism for tracking the status of a Big Segment store. * diff --git a/platform-node/src/api/interfaces/BigSegmentStatusProvider.ts b/platform-node/src/api/interfaces/BigSegmentStatusProvider.ts index 13678396a4..0ae8846d8a 100644 --- a/platform-node/src/api/interfaces/BigSegmentStatusProvider.ts +++ b/platform-node/src/api/interfaces/BigSegmentStatusProvider.ts @@ -20,7 +20,7 @@ import { interfaces } from '@launchdarkly/js-server-sdk-common'; * type of the status change event is `"change"`, and its value is the same value that would be * returned by {@link getStatus}. */ -export interface BigSegmentStoreStatusProvider { +export interface BigSegmentStoreStatusProvider extends EventEmitter { /** * Gets the current status of the store, if known. * diff --git a/platform-node/src/api/interfaces/index.ts b/platform-node/src/api/interfaces/index.ts index 1ad24aaa45..19659eab65 100644 --- a/platform-node/src/api/interfaces/index.ts +++ b/platform-node/src/api/interfaces/index.ts @@ -1 +1 @@ -export * from '@launchdarkly/js-server-sdk-common/src/api/interfaces/BigSegmentStoreStatusProvider'; +export * from './BigSegmentStatusProvider'; From 3b1ece5f00d1c3bda2b0f9ed7b1fb56b4805e5e8 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 23 May 2022 11:13:59 -0700 Subject: [PATCH 08/39] Start implementation. --- server-sdk-common/src/options/LDOptionsImpl.ts | 6 ++++++ server-sdk-common/src/options/ServiceEndpoints.ts | 6 ++++++ 2 files changed, 12 insertions(+) create mode 100644 server-sdk-common/src/options/LDOptionsImpl.ts create mode 100644 server-sdk-common/src/options/ServiceEndpoints.ts diff --git a/server-sdk-common/src/options/LDOptionsImpl.ts b/server-sdk-common/src/options/LDOptionsImpl.ts new file mode 100644 index 0000000000..b65a165238 --- /dev/null +++ b/server-sdk-common/src/options/LDOptionsImpl.ts @@ -0,0 +1,6 @@ +import { LDOptions } from '../api'; + +export default class LDOptionsImpl { + constructor(options: LDOptions) { + } +} diff --git a/server-sdk-common/src/options/ServiceEndpoints.ts b/server-sdk-common/src/options/ServiceEndpoints.ts new file mode 100644 index 0000000000..2ac8ec7ebd --- /dev/null +++ b/server-sdk-common/src/options/ServiceEndpoints.ts @@ -0,0 +1,6 @@ +class ServiceEndpoints { + + static FromOptions(LDOptions options) { + + } +} \ No newline at end of file From cec9858e26233b7cb11f3fdd3d65391fef6f77d6 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 23 May 2022 14:54:43 -0700 Subject: [PATCH 09/39] Progress on option handling. --- platform-node/tsconfig.json | 1 + sdk-common/tsconfig.json | 1 + .../src/options/Configuration.ts | 142 ++++++++++++++++++ .../src/options/LDOptionsImpl.ts | 6 - .../src/options/OptionMessages.ts | 21 +++ .../src/options/ServiceEndpoints.ts | 57 ++++++- server-sdk-common/src/options/validators.ts | 50 ++++++ .../src/options/valueOrDefault.ts | 6 + server-sdk-common/tsconfig.json | 1 + 9 files changed, 276 insertions(+), 9 deletions(-) create mode 100644 server-sdk-common/src/options/Configuration.ts delete mode 100644 server-sdk-common/src/options/LDOptionsImpl.ts create mode 100644 server-sdk-common/src/options/OptionMessages.ts create mode 100644 server-sdk-common/src/options/validators.ts create mode 100644 server-sdk-common/src/options/valueOrDefault.ts diff --git a/platform-node/tsconfig.json b/platform-node/tsconfig.json index bddab03ff1..171b58e8a8 100644 --- a/platform-node/tsconfig.json +++ b/platform-node/tsconfig.json @@ -15,5 +15,6 @@ "declaration": true, "declarationMap": true, // enables importers to jump to source "resolveJsonModule": true, + "stripInternal": true }, } diff --git a/sdk-common/tsconfig.json b/sdk-common/tsconfig.json index 0c10db0c9d..4b913cbe13 100644 --- a/sdk-common/tsconfig.json +++ b/sdk-common/tsconfig.json @@ -14,5 +14,6 @@ "sourceMap": true, "declaration": true, "declarationMap": true, // enables importers to jump to source + "stripInternal": true } } diff --git a/server-sdk-common/src/options/Configuration.ts b/server-sdk-common/src/options/Configuration.ts new file mode 100644 index 0000000000..244b8f875e --- /dev/null +++ b/server-sdk-common/src/options/Configuration.ts @@ -0,0 +1,142 @@ +// eslint-disable-next-line max-classes-per-file +import { LDLogger, LDOptions } from '../api'; +import OptionMessages from './OptionMessages'; +import ServiceEndpoints from './ServiceEndpoints'; +import TypeValidators, { TypeValidator } from './validators'; + +// Once things are internal to the implementation of the SDK we can depend on +// types. Calls to the SDK could contain anything without any regard to typing. +// So, data we take from external sources must be normalized into something +// that can be trusted. + +/** + * These perform cursory validations. Complex objects are implemented with classes + * and these should allow for conditional construction. + */ +const validations: Record = { + baseUri: TypeValidators.String, + streamUri: TypeValidators.String, + eventsUri: TypeValidators.String, + timeout: TypeValidators.Number, + capacity: TypeValidators.Number, + logger: TypeValidators.Object, + featureStore: TypeValidators.Object, + bigSegments: TypeValidators.Object, + updateProcessor: TypeValidators.ObjectOrFactory, + flushInterval: TypeValidators.Number, + pollInterval: TypeValidators.Number, + proxyOptions: TypeValidators.Object, + offline: TypeValidators.Boolean, + stream: TypeValidators.Boolean, + streamInitialReconnectDelay: TypeValidators.Number, + useLdd: TypeValidators.Boolean, + sendEvents: TypeValidators.Boolean, + allAttributesPrivate: TypeValidators.Boolean, + privateAttributes: TypeValidators.StringArray, + contextKeysCapacity: TypeValidators.Number, + contextKeysFlushInterval: TypeValidators.Number, + tlsParams: TypeValidators.Object, + diagnosticOptOut: TypeValidators.Boolean, + diagnosticRecordingInterval: TypeValidators.Number, + wrapperName: TypeValidators.String, + wrapperVersion: TypeValidators.String, + application: TypeValidators.Object, +}; + +const defaultValues: Record = { + baseUri: 'https://app.launchdarkly.com', + streamUri: 'https://stream.launchdarkly.com', + eventsUri: 'https://events.launchdarkly.com', + stream: true, + streamInitialReconnectDelay: 1, + sendEvents: true, + timeout: 5, + capacity: 10000, + flushInterval: 5, + pollInterval: 30, + offline: false, + useLdd: false, + allAttributesPrivate: false, + privateAttributes: [], + contextKeysCapacity: 1000, + contextKeysFlushInterval: 300, + diagnosticOptOut: false, + diagnosticRecordingInterval: 900, + // TODO: Implement once available. + // featureStore: InMemoryFeatureStore(), +}; + +function validateTypesAndNames(options: LDOptions): { + errors: string[], validatedOptions: LDOptions +} { + const errors: string[] = []; + const validatedOptions: Record = {}; + Object.keys(options).forEach((optionName) => { + // We need to tell typescript it doesn't actually know what options are. + // If we don't then it complains we are doing crazy things with it. + const optionValue = (options as unknown as any)[optionName]; + const validator = validations[optionName]; + if (validator) { + if (!validator.is(optionValue)) { + if (validator.getType() === 'boolean') { + errors.push(OptionMessages.wrongOptionTypeBoolean( + optionName, + typeof optionValue, + )); + validatedOptions[optionName] = !!optionValue; + } else { + errors.push(OptionMessages.wrongOptionType( + optionName, + validator.getType(), + typeof optionValue, + )); + validatedOptions[optionName] = defaultValues[optionName]; + } + } else { + validatedOptions[optionName] = optionValue; + } + } + }); + return { errors, validatedOptions }; +} + +/** + * Configuration options for the LDClient. + * + * @internal + */ +export default class Configuration { + public readonly serviceEndpoints: ServiceEndpoints; + + public readonly eventsCapacity: number; + + public readonly timeout: number; + + public readonly logger?: LDLogger; + + public readonly flushInterval: number; + + public readonly pollInterval: number; + + constructor(options: LDOptions) { + // If there isn't a valid logger from the platform, then logs would go nowhere. + this.logger = options.logger; + + const { errors, validatedOptions } = validateTypesAndNames(options); + errors.forEach((error) => { + this.logger?.warn(error); + }); + + this.serviceEndpoints = ServiceEndpoints.FromOptions(validatedOptions); + // We know these options are valid now, so we cast away the uncertainty. + this.eventsCapacity = validatedOptions.capacity as number; + this.timeout = validatedOptions.timeout as number; + // TODO: featureStore + // TODO: bigSegments + // TODO: updateProcessor + this.flushInterval = validatedOptions.flushInterval as number; + this.pollInterval = validatedOptions.pollInterval as number; + + // this.serviceEndpoints = ServiceEndpoints.FromOptions(options); + } +} diff --git a/server-sdk-common/src/options/LDOptionsImpl.ts b/server-sdk-common/src/options/LDOptionsImpl.ts deleted file mode 100644 index b65a165238..0000000000 --- a/server-sdk-common/src/options/LDOptionsImpl.ts +++ /dev/null @@ -1,6 +0,0 @@ -import { LDOptions } from '../api'; - -export default class LDOptionsImpl { - constructor(options: LDOptions) { - } -} diff --git a/server-sdk-common/src/options/OptionMessages.ts b/server-sdk-common/src/options/OptionMessages.ts new file mode 100644 index 0000000000..ee1f7dcb2f --- /dev/null +++ b/server-sdk-common/src/options/OptionMessages.ts @@ -0,0 +1,21 @@ +export default class OptionMessages { + static deprecated(oldName: string, newName: string): string { return `"${oldName}" is deprecated, please use "${newName}"`; } + + static optionBelowMinimum(name: string, value: number, min: number): string { + return `Config option "${name}" had invalid value of ${value}, using minimum of ${min} instead`; + } + + static unknownOption(name: string): string { return `Ignoring unknown config option "${name}"`; } + + static wrongOptionType(name: string, expectedType: string, actualType: string): string { + return `Config option "${name}" should be of type ${expectedType}, got ${actualType}, using default value`; + } + + static wrongOptionTypeBoolean(name: string, actualType: string): string { + return `Config option "${name}" should be a boolean, got ${actualType}, converting to boolean`; + } + + static invalidTagValue(name: string): string { + return `Config option "${name}" must only contain letters, numbers, ., _ or -.`; + } +} diff --git a/server-sdk-common/src/options/ServiceEndpoints.ts b/server-sdk-common/src/options/ServiceEndpoints.ts index 2ac8ec7ebd..d2edbc8156 100644 --- a/server-sdk-common/src/options/ServiceEndpoints.ts +++ b/server-sdk-common/src/options/ServiceEndpoints.ts @@ -1,6 +1,57 @@ -class ServiceEndpoints { +import { LDOptions } from '../api'; +import TypeValidators from './validators'; +import valueOrDefault from './valueOrDefault'; - static FromOptions(LDOptions options) { +/** + * DefaultStreamingBaseURI is the default base URI of the streaming service. +*/ +const DefaultStreamingBaseURI = 'https://stream.launchdarkly.com/'; +/** + * DefaultPollingBaseURI is the default base URI of the polling service. + */ +const DefaultPollingBaseURI = 'https://sdk.launchdarkly.com/'; + +/** + * DefaultEventsBaseURI is the default base URI of the events service. + */ +const DefaultEventsBaseURI = 'https://events.launchdarkly.com/'; + +function canonicalizeUri(uri: string): string { + return uri.replace(/\/+$/, ''); +} + +/** + * Specifies the base service URIs used by SDK components. + * + * @internal + */ +export default class ServiceEndpoints { + private streaming: string; + + private polling: string; + + private events: string; + + public get streamUri() { return this.streaming; } + + public get pollingUri() { return this.polling; } + + public get eventsUri() { return this.events; } + + private constructor(streaming: string, polling: string, events: string) { + this.streaming = canonicalizeUri(streaming); + this.polling = canonicalizeUri(polling); + this.events = canonicalizeUri(events); + } + + public static FromOptions(options: LDOptions): ServiceEndpoints { + const { baseUri, streamUri, eventsUri } = options; + + return new ServiceEndpoints( + valueOrDefault(streamUri, DefaultStreamingBaseURI), + valueOrDefault(baseUri, DefaultPollingBaseURI), + valueOrDefault(eventsUri, DefaultEventsBaseURI), + ); } -} \ No newline at end of file +} diff --git a/server-sdk-common/src/options/validators.ts b/server-sdk-common/src/options/validators.ts new file mode 100644 index 0000000000..08732c10a7 --- /dev/null +++ b/server-sdk-common/src/options/validators.ts @@ -0,0 +1,50 @@ +/* eslint-disable class-methods-use-this */ +/* eslint-disable max-classes-per-file */ + +export interface TypeValidator { + is(u:unknown): boolean; + getType(): string; +} + +class FactoryOrInstance implements TypeValidator { + is(factoryOrInstance: unknown) { + const anyFactory = factoryOrInstance as any; + const typeOfFactory = typeof anyFactory; + return typeOfFactory === 'function' || typeOfFactory === 'object'; + } + + getType(): string { + return 'factory method or object'; + } +} + +class Type implements TypeValidator { + private typeName: string; + + constructor(typeName: string) { + this.typeName = typeName; + } + + // eslint-disable-next-line class-methods-use-this + is(u: unknown): u is T { + return true; + } + + getType(): string { + return this.typeName; + } +} + +export default class TypeValidators { + static readonly String = new Type('string'); + + static readonly Number = new Type('number'); + + static readonly ObjectOrFactory = new FactoryOrInstance(); + + static readonly Object = new Type('object'); + + static readonly StringArray = new Type>('string[]'); + + static readonly Boolean = new Type('boolean'); +} diff --git a/server-sdk-common/src/options/valueOrDefault.ts b/server-sdk-common/src/options/valueOrDefault.ts new file mode 100644 index 0000000000..499bd1c03c --- /dev/null +++ b/server-sdk-common/src/options/valueOrDefault.ts @@ -0,0 +1,6 @@ +export default function valueOrDefault(value: T | undefined | null, defValue: T): T { + if (value === undefined || value === null) { + return defValue; + } + return value; +} diff --git a/server-sdk-common/tsconfig.json b/server-sdk-common/tsconfig.json index 0c10db0c9d..4b913cbe13 100644 --- a/server-sdk-common/tsconfig.json +++ b/server-sdk-common/tsconfig.json @@ -14,5 +14,6 @@ "sourceMap": true, "declaration": true, "declarationMap": true, // enables importers to jump to source + "stripInternal": true } } From 954a13595dd51301a7810f3294c8ff85655d98a3 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 23 May 2022 16:12:09 -0700 Subject: [PATCH 10/39] Add application tags support. --- .../src/options/ApplicationTags.ts | 48 ++++++++++ .../src/options/Configuration.ts | 94 +++++++++++++++---- .../src/options/OptionMessages.ts | 5 + .../src/options/ServiceEndpoints.ts | 31 +----- .../src/options/ValidatedOptions.ts | 34 +++++++ server-sdk-common/src/options/validators.ts | 68 +++++++++++++- .../src/options/valueOrDefault.ts | 6 -- 7 files changed, 231 insertions(+), 55 deletions(-) create mode 100644 server-sdk-common/src/options/ApplicationTags.ts create mode 100644 server-sdk-common/src/options/ValidatedOptions.ts delete mode 100644 server-sdk-common/src/options/valueOrDefault.ts diff --git a/server-sdk-common/src/options/ApplicationTags.ts b/server-sdk-common/src/options/ApplicationTags.ts new file mode 100644 index 0000000000..6cc44409eb --- /dev/null +++ b/server-sdk-common/src/options/ApplicationTags.ts @@ -0,0 +1,48 @@ +import OptionMessages from './OptionMessages'; +import { ValidatedOptions } from './ValidatedOptions'; +import TypeValidators from './validators'; + +/** +* Expression to validate characters that are allowed in tag keys and values. +*/ +const allowedTagCharacters = /^(\w|\.|-)+$/; + +const tagValidator = TypeValidators.StringMatchingRegex(allowedTagCharacters); + +/** + * Class for managing tags. + * + * @internal + */ +export default class ApplicationTags { + public readonly value?: string; + + constructor(options: ValidatedOptions) { + const tags: Record = {}; + const { application } = options; + if (application) { + if (application.id) { + if (tagValidator.is(application.id)) { + tags['application-id'] = [application.id]; + } else { + options.logger?.warn(OptionMessages.invalidTagValue('application.id')); + } + } + if (application.version) { + if (tagValidator.is(application.version)) { + tags['application-version'] = [application.version]; + } else { + options.logger?.warn(OptionMessages.invalidTagValue('application.version')); + } + } + } + + const tagKeys = Object.keys(tags); + if (tagKeys.length) { + this.value = tagKeys + .sort() + .flatMap((key) => tags[key].sort().map((value) => `${key}/${value}`)) + .join(' '); + } + } +} diff --git a/server-sdk-common/src/options/Configuration.ts b/server-sdk-common/src/options/Configuration.ts index 244b8f875e..2b31fe3839 100644 --- a/server-sdk-common/src/options/Configuration.ts +++ b/server-sdk-common/src/options/Configuration.ts @@ -1,8 +1,10 @@ // eslint-disable-next-line max-classes-per-file -import { LDLogger, LDOptions } from '../api'; +import { LDLogger, LDOptions, LDProxyOptions, LDTLSOptions } from '../api'; +import ApplicationTags from './ApplicationTags'; import OptionMessages from './OptionMessages'; import ServiceEndpoints from './ServiceEndpoints'; -import TypeValidators, { TypeValidator } from './validators'; +import { ValidatedOptions } from './ValidatedOptions'; +import TypeValidators, { NumberWithMinimum, Type, TypeValidator } from './validators'; // Once things are internal to the implementation of the SDK we can depend on // types. Calls to the SDK could contain anything without any regard to typing. @@ -24,7 +26,7 @@ const validations: Record = { bigSegments: TypeValidators.Object, updateProcessor: TypeValidators.ObjectOrFactory, flushInterval: TypeValidators.Number, - pollInterval: TypeValidators.Number, + pollInterval: TypeValidators.NumberWithMin(30), proxyOptions: TypeValidators.Object, offline: TypeValidators.Boolean, stream: TypeValidators.Boolean, @@ -37,13 +39,13 @@ const validations: Record = { contextKeysFlushInterval: TypeValidators.Number, tlsParams: TypeValidators.Object, diagnosticOptOut: TypeValidators.Boolean, - diagnosticRecordingInterval: TypeValidators.Number, + diagnosticRecordingInterval: TypeValidators.NumberWithMin(60), wrapperName: TypeValidators.String, wrapperVersion: TypeValidators.String, application: TypeValidators.Object, }; -const defaultValues: Record = { +const defaultValues: ValidatedOptions = { baseUri: 'https://app.launchdarkly.com', streamUri: 'https://stream.launchdarkly.com', eventsUri: 'https://events.launchdarkly.com', @@ -67,10 +69,10 @@ const defaultValues: Record = { }; function validateTypesAndNames(options: LDOptions): { - errors: string[], validatedOptions: LDOptions + errors: string[], validatedOptions: ValidatedOptions } { const errors: string[] = []; - const validatedOptions: Record = {}; + const validatedOptions: ValidatedOptions = { ...defaultValues }; Object.keys(options).forEach((optionName) => { // We need to tell typescript it doesn't actually know what options are. // If we don't then it complains we are doing crazy things with it. @@ -84,7 +86,11 @@ function validateTypesAndNames(options: LDOptions): { typeof optionValue, )); validatedOptions[optionName] = !!optionValue; - } else { + } else if (validator instanceof NumberWithMinimum) { + const min = (validator as NumberWithMinimum).min; + errors.push(OptionMessages.optionBelowMinimum(optionName, optionValue, min)); + } + else { errors.push(OptionMessages.wrongOptionType( optionName, validator.getType(), @@ -92,14 +98,20 @@ function validateTypesAndNames(options: LDOptions): { )); validatedOptions[optionName] = defaultValues[optionName]; } - } else { - validatedOptions[optionName] = optionValue; } } }); return { errors, validatedOptions }; } +// function enforceMinimum(configIn, name, min) { +// const config = configIn; +// if (config[name] < min) { +// config.logger.warn(messages.optionBelowMinimum(name, config[name], min)); +// config[name] = min; +// } +// } + /** * Configuration options for the LDClient. * @@ -118,6 +130,37 @@ export default class Configuration { public readonly pollInterval: number; + public readonly proxyOptions?: LDProxyOptions; + + public readonly offline: boolean; + + public readonly stream: boolean; + + public readonly streamInitialReconnectDelay: number; + + public readonly useLdd: boolean; + + public readonly sendEvents: boolean; + + public readonly allAttributesPrivate: boolean; + + // TODO: Change to attribute references once available. + public readonly privateAttributes: string[]; + + public readonly contextKeysCapacity: number; + + public readonly contextKeysFlushInterval: number; + + public readonly tlsParams?: LDTLSOptions; + + public readonly diagnosticOptOut: boolean; + + public readonly wrapperName?: string; + + public readonly wrapperVersion?: string; + + public readonly tags: ApplicationTags; + constructor(options: LDOptions) { // If there isn't a valid logger from the platform, then logs would go nowhere. this.logger = options.logger; @@ -127,16 +170,33 @@ export default class Configuration { this.logger?.warn(error); }); - this.serviceEndpoints = ServiceEndpoints.FromOptions(validatedOptions); - // We know these options are valid now, so we cast away the uncertainty. - this.eventsCapacity = validatedOptions.capacity as number; - this.timeout = validatedOptions.timeout as number; + this.serviceEndpoints = new ServiceEndpoints( + validatedOptions.streamUri, + validatedOptions.baseUri, + validatedOptions.eventsUri, + ); + this.eventsCapacity = validatedOptions.capacity; + this.timeout = validatedOptions.timeout; // TODO: featureStore // TODO: bigSegments // TODO: updateProcessor - this.flushInterval = validatedOptions.flushInterval as number; - this.pollInterval = validatedOptions.pollInterval as number; + this.flushInterval = validatedOptions.flushInterval; + this.pollInterval = validatedOptions.pollInterval; + this.proxyOptions = validatedOptions.proxyOptions; - // this.serviceEndpoints = ServiceEndpoints.FromOptions(options); + this.offline = validatedOptions.offline; + this.stream = validatedOptions.stream; + this.streamInitialReconnectDelay = validatedOptions.streamInitialReconnectDelay; + this.useLdd = validatedOptions.useLdd; + this.sendEvents = validatedOptions.sendEvents; + this.allAttributesPrivate = validatedOptions.allAttributesPrivate; + this.privateAttributes = validatedOptions.privateAttributes; + this.contextKeysCapacity = validatedOptions.contextKeysCapacity; + this.contextKeysFlushInterval = validatedOptions.contextKeysFlushInterval; + this.tlsParams = validatedOptions.tlsParams; + this.diagnosticOptOut = validatedOptions.diagnosticOptOut; + this.wrapperName = validatedOptions.wrapperName; + this.wrapperVersion = validatedOptions.wrapperVersion; + this.tags = new ApplicationTags(validatedOptions); } } diff --git a/server-sdk-common/src/options/OptionMessages.ts b/server-sdk-common/src/options/OptionMessages.ts index ee1f7dcb2f..c55a071a18 100644 --- a/server-sdk-common/src/options/OptionMessages.ts +++ b/server-sdk-common/src/options/OptionMessages.ts @@ -1,3 +1,8 @@ +/** + * Messages for issues which can be encountered from processing the configuration options. + * + * @internal + */ export default class OptionMessages { static deprecated(oldName: string, newName: string): string { return `"${oldName}" is deprecated, please use "${newName}"`; } diff --git a/server-sdk-common/src/options/ServiceEndpoints.ts b/server-sdk-common/src/options/ServiceEndpoints.ts index d2edbc8156..bad5fef371 100644 --- a/server-sdk-common/src/options/ServiceEndpoints.ts +++ b/server-sdk-common/src/options/ServiceEndpoints.ts @@ -1,22 +1,3 @@ -import { LDOptions } from '../api'; -import TypeValidators from './validators'; -import valueOrDefault from './valueOrDefault'; - -/** - * DefaultStreamingBaseURI is the default base URI of the streaming service. -*/ -const DefaultStreamingBaseURI = 'https://stream.launchdarkly.com/'; - -/** - * DefaultPollingBaseURI is the default base URI of the polling service. - */ -const DefaultPollingBaseURI = 'https://sdk.launchdarkly.com/'; - -/** - * DefaultEventsBaseURI is the default base URI of the events service. - */ -const DefaultEventsBaseURI = 'https://events.launchdarkly.com/'; - function canonicalizeUri(uri: string): string { return uri.replace(/\/+$/, ''); } @@ -39,19 +20,9 @@ export default class ServiceEndpoints { public get eventsUri() { return this.events; } - private constructor(streaming: string, polling: string, events: string) { + public constructor(streaming: string, polling: string, events: string) { this.streaming = canonicalizeUri(streaming); this.polling = canonicalizeUri(polling); this.events = canonicalizeUri(events); } - - public static FromOptions(options: LDOptions): ServiceEndpoints { - const { baseUri, streamUri, eventsUri } = options; - - return new ServiceEndpoints( - valueOrDefault(streamUri, DefaultStreamingBaseURI), - valueOrDefault(baseUri, DefaultPollingBaseURI), - valueOrDefault(eventsUri, DefaultEventsBaseURI), - ); - } } diff --git a/server-sdk-common/src/options/ValidatedOptions.ts b/server-sdk-common/src/options/ValidatedOptions.ts new file mode 100644 index 0000000000..db65cb880f --- /dev/null +++ b/server-sdk-common/src/options/ValidatedOptions.ts @@ -0,0 +1,34 @@ +import { LDLogger, LDProxyOptions, LDTLSOptions } from '../api'; + +/** + * This interface applies to the options after they have been validated and defaults + * have been applied. + */ +export interface ValidatedOptions { + baseUri: string; + streamUri: string; + eventsUri: string; + stream: boolean; + streamInitialReconnectDelay: number; + sendEvents: boolean; + timeout: number; + capacity: number; + flushInterval: number; + pollInterval: number; + offline: boolean; + useLdd: boolean; + allAttributesPrivate: false; + privateAttributes: string[]; + contextKeysCapacity: number; + contextKeysFlushInterval: number; + diagnosticOptOut: boolean; + diagnosticRecordingInterval: number; + tlsParams?: LDTLSOptions; + wrapperName?: string; + wrapperVersion?: string; + application?: { id?: string; version?: string; }; + proxyOptions?: LDProxyOptions; + logger?: LDLogger; + // Allow indexing this by a string for the validation step. + [index: string]: any; +} diff --git a/server-sdk-common/src/options/validators.ts b/server-sdk-common/src/options/validators.ts index 08732c10a7..5b9c6dfd2e 100644 --- a/server-sdk-common/src/options/validators.ts +++ b/server-sdk-common/src/options/validators.ts @@ -1,12 +1,22 @@ /* eslint-disable class-methods-use-this */ /* eslint-disable max-classes-per-file */ +/** + * Interface for type validation. + * + * @internal + */ export interface TypeValidator { is(u:unknown): boolean; getType(): string; } -class FactoryOrInstance implements TypeValidator { +/** + * Validate a factory or instance. + * + * @internal + */ +export class FactoryOrInstance implements TypeValidator { is(factoryOrInstance: unknown) { const anyFactory = factoryOrInstance as any; const typeOfFactory = typeof anyFactory; @@ -18,7 +28,12 @@ class FactoryOrInstance implements TypeValidator { } } -class Type implements TypeValidator { +/** + * Validate a basic type. + * + * @internal + */ +export class Type implements TypeValidator { private typeName: string; constructor(typeName: string) { @@ -35,6 +50,47 @@ class Type implements TypeValidator { } } +/** + * Validate a value is a number and is greater or eval than a minimum. + * + * @internal + */ +export class NumberWithMinimum extends Type { + readonly min: number; + + constructor(min: number) { + super(`number with minimum value of ${min}`); + this.min = min; + } + + override is(u: unknown): u is number { + return (u as number) >= this.min; + } +} + +/** + * Validate a value is a string and it matches the given expression. + * + * @internal + */ +export class StringMatchingRegex extends Type { + readonly expression: RegExp; + + constructor(expression: RegExp) { + super(`string matching ${expression}`); + this.expression = expression; + } + + override is(u: unknown): u is string { + return !!(u as string).match(this.expression); + } +} + +/** + * A set of standard type validator. + * + * @internal + */ export default class TypeValidators { static readonly String = new Type('string'); @@ -47,4 +103,12 @@ export default class TypeValidators { static readonly StringArray = new Type>('string[]'); static readonly Boolean = new Type('boolean'); + + static NumberWithMin(min: number): NumberWithMinimum { + return new NumberWithMinimum(min); + } + + static StringMatchingRegex(expression: RegExp): StringMatchingRegex { + return new StringMatchingRegex(expression); + } } diff --git a/server-sdk-common/src/options/valueOrDefault.ts b/server-sdk-common/src/options/valueOrDefault.ts deleted file mode 100644 index 499bd1c03c..0000000000 --- a/server-sdk-common/src/options/valueOrDefault.ts +++ /dev/null @@ -1,6 +0,0 @@ -export default function valueOrDefault(value: T | undefined | null, defValue: T): T { - if (value === undefined || value === null) { - return defValue; - } - return value; -} From 1a0dffc8857315911d9ab8f7451e26852a2538c3 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 23 May 2022 16:41:09 -0700 Subject: [PATCH 11/39] Un-generalize tags. --- .../src/options/ApplicationTags.ts | 31 +++++++++---------- .../src/options/Configuration.ts | 12 ++----- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/server-sdk-common/src/options/ApplicationTags.ts b/server-sdk-common/src/options/ApplicationTags.ts index 6cc44409eb..c8707f4e45 100644 --- a/server-sdk-common/src/options/ApplicationTags.ts +++ b/server-sdk-common/src/options/ApplicationTags.ts @@ -1,6 +1,6 @@ import OptionMessages from './OptionMessages'; import { ValidatedOptions } from './ValidatedOptions'; -import TypeValidators from './validators'; +import TypeValidators, { TypeValidator } from './validators'; /** * Expression to validate characters that are allowed in tag keys and values. @@ -9,8 +9,7 @@ const allowedTagCharacters = /^(\w|\.|-)+$/; const tagValidator = TypeValidators.StringMatchingRegex(allowedTagCharacters); -/** - * Class for managing tags. +/** Class for managing tags. * * @internal */ @@ -20,20 +19,20 @@ export default class ApplicationTags { constructor(options: ValidatedOptions) { const tags: Record = {}; const { application } = options; - if (application) { - if (application.id) { - if (tagValidator.is(application.id)) { - tags['application-id'] = [application.id]; - } else { - options.logger?.warn(OptionMessages.invalidTagValue('application.id')); - } + + if (application?.id !== null && application?.id !== undefined) { + if (tagValidator.is(application.id)) { + tags['application-id'] = [application.id]; + } else { + options.logger?.warn(OptionMessages.invalidTagValue('application.id')); } - if (application.version) { - if (tagValidator.is(application.version)) { - tags['application-version'] = [application.version]; - } else { - options.logger?.warn(OptionMessages.invalidTagValue('application.version')); - } + } + + if (application?.version !== null && application?.version !== undefined) { + if (tagValidator.is(application.version)) { + tags['application-version'] = [application.version]; + } else { + options.logger?.warn(OptionMessages.invalidTagValue('application.version')); } } diff --git a/server-sdk-common/src/options/Configuration.ts b/server-sdk-common/src/options/Configuration.ts index 2b31fe3839..7e4c9dddbb 100644 --- a/server-sdk-common/src/options/Configuration.ts +++ b/server-sdk-common/src/options/Configuration.ts @@ -4,7 +4,7 @@ import ApplicationTags from './ApplicationTags'; import OptionMessages from './OptionMessages'; import ServiceEndpoints from './ServiceEndpoints'; import { ValidatedOptions } from './ValidatedOptions'; -import TypeValidators, { NumberWithMinimum, Type, TypeValidator } from './validators'; +import TypeValidators, { NumberWithMinimum, TypeValidator } from './validators'; // Once things are internal to the implementation of the SDK we can depend on // types. Calls to the SDK could contain anything without any regard to typing. @@ -87,7 +87,7 @@ function validateTypesAndNames(options: LDOptions): { )); validatedOptions[optionName] = !!optionValue; } else if (validator instanceof NumberWithMinimum) { - const min = (validator as NumberWithMinimum).min; + const { min } = validator as NumberWithMinimum; errors.push(OptionMessages.optionBelowMinimum(optionName, optionValue, min)); } else { @@ -104,14 +104,6 @@ function validateTypesAndNames(options: LDOptions): { return { errors, validatedOptions }; } -// function enforceMinimum(configIn, name, min) { -// const config = configIn; -// if (config[name] < min) { -// config.logger.warn(messages.optionBelowMinimum(name, config[name], min)); -// config[name] = min; -// } -// } - /** * Configuration options for the LDClient. * From a141aeb58710ab452fb046970d0296e1dce17a3d Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 23 May 2022 16:41:28 -0700 Subject: [PATCH 12/39] Fix tag formatting. --- server-sdk-common/src/options/ApplicationTags.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server-sdk-common/src/options/ApplicationTags.ts b/server-sdk-common/src/options/ApplicationTags.ts index c8707f4e45..588d2222d4 100644 --- a/server-sdk-common/src/options/ApplicationTags.ts +++ b/server-sdk-common/src/options/ApplicationTags.ts @@ -9,7 +9,8 @@ const allowedTagCharacters = /^(\w|\.|-)+$/; const tagValidator = TypeValidators.StringMatchingRegex(allowedTagCharacters); -/** Class for managing tags. +/** + * Class for managing tags. * * @internal */ From 40d27e33f82ef68e703b0a2d64770d5693ec6762 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 23 May 2022 16:44:19 -0700 Subject: [PATCH 13/39] Comments. --- server-sdk-common/src/options/ApplicationTags.ts | 2 +- server-sdk-common/src/options/Configuration.ts | 8 ++++---- server-sdk-common/src/options/validators.ts | 9 ++++++++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/server-sdk-common/src/options/ApplicationTags.ts b/server-sdk-common/src/options/ApplicationTags.ts index 588d2222d4..53037795b2 100644 --- a/server-sdk-common/src/options/ApplicationTags.ts +++ b/server-sdk-common/src/options/ApplicationTags.ts @@ -1,6 +1,6 @@ import OptionMessages from './OptionMessages'; import { ValidatedOptions } from './ValidatedOptions'; -import TypeValidators, { TypeValidator } from './validators'; +import TypeValidators from './validators'; /** * Expression to validate characters that are allowed in tag keys and values. diff --git a/server-sdk-common/src/options/Configuration.ts b/server-sdk-common/src/options/Configuration.ts index 7e4c9dddbb..0b8547a41b 100644 --- a/server-sdk-common/src/options/Configuration.ts +++ b/server-sdk-common/src/options/Configuration.ts @@ -1,5 +1,6 @@ -// eslint-disable-next-line max-classes-per-file -import { LDLogger, LDOptions, LDProxyOptions, LDTLSOptions } from '../api'; +import { + LDLogger, LDOptions, LDProxyOptions, LDTLSOptions, +} from '../api'; import ApplicationTags from './ApplicationTags'; import OptionMessages from './OptionMessages'; import ServiceEndpoints from './ServiceEndpoints'; @@ -89,8 +90,7 @@ function validateTypesAndNames(options: LDOptions): { } else if (validator instanceof NumberWithMinimum) { const { min } = validator as NumberWithMinimum; errors.push(OptionMessages.optionBelowMinimum(optionName, optionValue, min)); - } - else { + } else { errors.push(OptionMessages.wrongOptionType( optionName, validator.getType(), diff --git a/server-sdk-common/src/options/validators.ts b/server-sdk-common/src/options/validators.ts index 5b9c6dfd2e..b63fc761b8 100644 --- a/server-sdk-common/src/options/validators.ts +++ b/server-sdk-common/src/options/validators.ts @@ -1,6 +1,13 @@ /* eslint-disable class-methods-use-this */ /* eslint-disable max-classes-per-file */ +// The classes here are static, but needs to be instantiated to +// support the generic functionality. Which is why we do not care about using +// `this` + +// These validators are also of trivial complexity, so we are allowing more than +// one per file. + /** * Interface for type validation. * @@ -87,7 +94,7 @@ export class StringMatchingRegex extends Type { } /** - * A set of standard type validator. + * A set of standard type validators. * * @internal */ From f4295714488df4eb685882478681b628630f68cb Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 23 May 2022 16:48:49 -0700 Subject: [PATCH 14/39] Add internal to validated options. --- server-sdk-common/src/options/ValidatedOptions.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server-sdk-common/src/options/ValidatedOptions.ts b/server-sdk-common/src/options/ValidatedOptions.ts index db65cb880f..d953657d30 100644 --- a/server-sdk-common/src/options/ValidatedOptions.ts +++ b/server-sdk-common/src/options/ValidatedOptions.ts @@ -3,6 +3,8 @@ import { LDLogger, LDProxyOptions, LDTLSOptions } from '../api'; /** * This interface applies to the options after they have been validated and defaults * have been applied. + * + * @internal */ export interface ValidatedOptions { baseUri: string; From 579212252f408758afd5325eadd6909e8a2916f1 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 24 May 2022 09:34:21 -0700 Subject: [PATCH 15/39] Add tests for type checks. Finish implementing type checks. --- .../{NodeInfo_test.ts => NodeInfo.test.ts} | 0 ...eRequests_test.ts => NodeRequests.test.ts} | 0 .../__tests__/options/validators.test.ts | 57 +++++++++++++++++++ server-sdk-common/src/options/validators.ts | 55 ++++++++++++++---- 4 files changed, 101 insertions(+), 11 deletions(-) rename platform-node/__tests__/{NodeInfo_test.ts => NodeInfo.test.ts} (100%) rename platform-node/__tests__/{NodeRequests_test.ts => NodeRequests.test.ts} (100%) create mode 100644 server-sdk-common/__tests__/options/validators.test.ts diff --git a/platform-node/__tests__/NodeInfo_test.ts b/platform-node/__tests__/NodeInfo.test.ts similarity index 100% rename from platform-node/__tests__/NodeInfo_test.ts rename to platform-node/__tests__/NodeInfo.test.ts diff --git a/platform-node/__tests__/NodeRequests_test.ts b/platform-node/__tests__/NodeRequests.test.ts similarity index 100% rename from platform-node/__tests__/NodeRequests_test.ts rename to platform-node/__tests__/NodeRequests.test.ts diff --git a/server-sdk-common/__tests__/options/validators.test.ts b/server-sdk-common/__tests__/options/validators.test.ts new file mode 100644 index 0000000000..f0caea8bcf --- /dev/null +++ b/server-sdk-common/__tests__/options/validators.test.ts @@ -0,0 +1,57 @@ +import TypeValidators from '../../src/options/validators'; + +const stringValue = 'this is a string'; +const numberValue = 3.14159; +const objectValue = { yes: 'no' }; +const functionValue = () => ({}); +const stringArrayValue = ['these', 'are', 'strings']; +const booleanValue = true; + +const allValues = [ + stringValue, + numberValue, + objectValue, + functionValue, + stringArrayValue, + booleanValue, +]; + +function without(array: Array, value: any): Array { + const returnArray = [...array]; + const index = returnArray.indexOf(value); + if (index > -1) { + returnArray.splice(index, 1); + } + return returnArray; +} + +const invalidForString = without(allValues, stringValue); +const invalidForNumber = without(allValues, numberValue); +const invalidForObject = without(allValues, objectValue); +const invalidForFactory = without(invalidForObject, functionValue); +const invalidForStringArray = without(allValues, stringArrayValue); +const invalidForBoolean = without(allValues, booleanValue); + +describe.each([ + [TypeValidators.String, [stringValue], invalidForString], + [TypeValidators.Number, [numberValue], invalidForNumber], + [TypeValidators.ObjectOrFactory, [objectValue, functionValue], invalidForFactory], + [TypeValidators.Object, [objectValue], invalidForObject], + [TypeValidators.StringArray, [stringArrayValue], invalidForStringArray], + [TypeValidators.Boolean, [booleanValue], invalidForBoolean], +])( + 'Given a validator, valid values, and invalid values', + (validator, validValues, invalidValues) => { + it('validates the correct type', () => { + validValues.forEach((validValue) => { + expect(validator.is(validValue)).toBeTruthy(); + }); + }); + + it(`does not validate incorrect types ${validator.getType()}: ${invalidValues}`, () => { + invalidValues.forEach((invalidValue) => { + expect(validator.is(invalidValue)).toBeFalsy(); + }); + }); + }, +); diff --git a/server-sdk-common/src/options/validators.ts b/server-sdk-common/src/options/validators.ts index b63fc761b8..64c3ca5999 100644 --- a/server-sdk-common/src/options/validators.ts +++ b/server-sdk-common/src/options/validators.ts @@ -25,6 +25,9 @@ export interface TypeValidator { */ export class FactoryOrInstance implements TypeValidator { is(factoryOrInstance: unknown) { + if (Array.isArray(factoryOrInstance)) { + return false; + } const anyFactory = factoryOrInstance as any; const typeOfFactory = typeof anyFactory; return typeOfFactory === 'function' || typeOfFactory === 'object'; @@ -43,13 +46,43 @@ export class FactoryOrInstance implements TypeValidator { export class Type implements TypeValidator { private typeName: string; - constructor(typeName: string) { + protected typeOf: string; + + constructor(typeName: string, example: T) { + this.typeName = typeName; + this.typeOf = typeof example; + } + + is(u: unknown): u is T { + if (Array.isArray(u)) { + return false; + } + return typeof u === this.typeOf; + } + + getType(): string { + return this.typeName; + } +} + +export class TypeArray implements TypeValidator { + private typeName: string; + + protected typeOf: string; + + constructor(typeName: string, example: T) { this.typeName = typeName; + this.typeOf = typeof example; } - // eslint-disable-next-line class-methods-use-this is(u: unknown): u is T { - return true; + if (Array.isArray(u)) { + if (u.length > 0) { + return u.every((val) => typeof val === this.typeOf); + } + return true; + } + return false; } getType(): string { @@ -66,12 +99,12 @@ export class NumberWithMinimum extends Type { readonly min: number; constructor(min: number) { - super(`number with minimum value of ${min}`); + super(`number with minimum value of ${min}`, 0); this.min = min; } override is(u: unknown): u is number { - return (u as number) >= this.min; + return typeof u === this.typeOf && (u as number) >= this.min; } } @@ -84,7 +117,7 @@ export class StringMatchingRegex extends Type { readonly expression: RegExp; constructor(expression: RegExp) { - super(`string matching ${expression}`); + super(`string matching ${expression}`, ''); this.expression = expression; } @@ -99,17 +132,17 @@ export class StringMatchingRegex extends Type { * @internal */ export default class TypeValidators { - static readonly String = new Type('string'); + static readonly String = new Type('string', ''); - static readonly Number = new Type('number'); + static readonly Number = new Type('number', 0); static readonly ObjectOrFactory = new FactoryOrInstance(); - static readonly Object = new Type('object'); + static readonly Object = new Type('object', {}); - static readonly StringArray = new Type>('string[]'); + static readonly StringArray = new TypeArray('string[]', ''); - static readonly Boolean = new Type('boolean'); + static readonly Boolean = new Type('boolean', true); static NumberWithMin(min: number): NumberWithMinimum { return new NumberWithMinimum(min); From b93ed50344676b487644430cc286636196706c7b Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 24 May 2022 09:54:05 -0700 Subject: [PATCH 16/39] Add remaining validator tests. --- .../__tests__/options/validators.test.ts | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/server-sdk-common/__tests__/options/validators.test.ts b/server-sdk-common/__tests__/options/validators.test.ts index f0caea8bcf..e36d7eae9b 100644 --- a/server-sdk-common/__tests__/options/validators.test.ts +++ b/server-sdk-common/__tests__/options/validators.test.ts @@ -1,4 +1,4 @@ -import TypeValidators from '../../src/options/validators'; +import TypeValidators, { TypeArray } from '../../src/options/validators'; const stringValue = 'this is a string'; const numberValue = 3.14159; @@ -42,7 +42,7 @@ describe.each([ ])( 'Given a validator, valid values, and invalid values', (validator, validValues, invalidValues) => { - it('validates the correct type', () => { + it(`validates the correct type ${validator.getType()}: ${validValues}`, () => { validValues.forEach((validValue) => { expect(validator.is(validValue)).toBeTruthy(); }); @@ -55,3 +55,47 @@ describe.each([ }); }, ); + +describe.each([ + [TypeValidators.StringArray, [['a', 'b', 'c', 'd'], []], [[0, 'potato'], [{}]]], + [new TypeArray('number[]', 0), [[0, 1, 2, 3], []], [[0, 'potato'], [{}]]], + [new TypeArray('object[]', {}), [[{}, { yes: 'no' }], []], [[0, 'potato'], [{}, 17]]], +])('given an array validator, valid arrays, and invalid arrays', (validator, validValues, invalidValues) => { + it(`validates the correct type ${validator.getType()}: ${validValues}`, () => { + validValues.forEach((validValue) => { + expect(validator.is(validValue)).toBeTruthy(); + }); + }); + + it(`does not validate incorrect types ${validator.getType()}: ${invalidValues}`, () => { + invalidValues.forEach((invalidValue) => { + expect(validator.is(invalidValue)).toBeFalsy(); + }); + }); +}); + +describe('given a regex validator', () => { + const validator = TypeValidators.StringMatchingRegex(/^(\w|\.|-)+$/); + it('matches valid instances', () => { + expect(validator.is('valid-version._')).toBeTruthy(); + }); + + it('does not match invalid instances', () => { + expect(validator.is('invalid-version!@#$%^&*()')).toBeFalsy(); + }); +}); + +describe.each([ + [TypeValidators.NumberWithMin(0), 0], + [TypeValidators.NumberWithMin(10), 10], + [TypeValidators.NumberWithMin(1000), 1000], +])('given minumum number validators', (validator, min) => { + it('validates numbers equal or above the minimum', () => { + expect(validator.is(min)).toBeTruthy(); + expect(validator.is(min + 1)).toBeTruthy(); + }); + + it('does not validate numbers less than the minimum', () => { + expect(validator.is(min - 1)).toBeFalsy(); + }); +}); From 82599e2130d60f5ebe35f641560ef34d3144a5d4 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 24 May 2022 10:08:16 -0700 Subject: [PATCH 17/39] Improve coverage collection. Start implementing application tags tests. --- jest.config.js | 7 ++++++- .../__tests__/options/ApplicationTags.test.ts | 16 ++++++++++++++++ server-sdk-common/src/options/ApplicationTags.ts | 2 +- 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 server-sdk-common/__tests__/options/ApplicationTags.test.ts diff --git a/jest.config.js b/jest.config.js index 5b0cd4cfa9..f87a9a767c 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,5 +1,10 @@ module.exports = { transform: {'^.+\\.ts?$': 'ts-jest'}, testEnvironment: 'node', - moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'] + moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'], + collectCoverageFrom: [ + "platform-node/src/**/*.ts", + "sdk-common/src/**/*.ts", + "server-sdk-common/src/**/*.ts" + ] }; diff --git a/server-sdk-common/__tests__/options/ApplicationTags.test.ts b/server-sdk-common/__tests__/options/ApplicationTags.test.ts new file mode 100644 index 0000000000..2363cb9d8e --- /dev/null +++ b/server-sdk-common/__tests__/options/ApplicationTags.test.ts @@ -0,0 +1,16 @@ +import ApplicationTags from '../../src/options/ApplicationTags'; +import { ValidatedOptions } from '../../src/options/ValidatedOptions'; + +describe.each([ + [{application: {id: 'is-valid', version: 'also-valid'}}, "application-id/is-valid application-version/also-valid"], + [{application: {id: 'is-valid'}}, "application-id/is-valid"], + [{application: {version: 'also-valid'}}, "application-version/also-valid"], + [{application: {}}, undefined], + [{}, undefined], + [undefined, undefined] +])('given application tags configurations', (config, result) => { + it('produces the correct tag values', () => { + const tags = new ApplicationTags(config as unknown as ValidatedOptions); + expect(tags.value).toEqual(result); + }); +}); \ No newline at end of file diff --git a/server-sdk-common/src/options/ApplicationTags.ts b/server-sdk-common/src/options/ApplicationTags.ts index 53037795b2..00f9305016 100644 --- a/server-sdk-common/src/options/ApplicationTags.ts +++ b/server-sdk-common/src/options/ApplicationTags.ts @@ -19,7 +19,7 @@ export default class ApplicationTags { constructor(options: ValidatedOptions) { const tags: Record = {}; - const { application } = options; + const application = options?.application; if (application?.id !== null && application?.id !== undefined) { if (tagValidator.is(application.id)) { From ef6711584867c88c11bbb7936f3b6cca9553778b Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 24 May 2022 10:37:26 -0700 Subject: [PATCH 18/39] Add test logger and finish tag tests. --- jest.config.js | 1 + server-sdk-common/__tests__/Logger.ts | 62 +++++++++++++++++++ .../__tests__/options/ApplicationTags.test.ts | 43 ++++++++++--- .../__tests__/options/validators.test.ts | 2 +- 4 files changed, 99 insertions(+), 9 deletions(-) create mode 100644 server-sdk-common/__tests__/Logger.ts diff --git a/jest.config.js b/jest.config.js index f87a9a767c..0abe6622af 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,5 +1,6 @@ module.exports = { transform: {'^.+\\.ts?$': 'ts-jest'}, + testMatch: ["**/__tests__/**/*test.ts?(x)"], testEnvironment: 'node', moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'], collectCoverageFrom: [ diff --git a/server-sdk-common/__tests__/Logger.ts b/server-sdk-common/__tests__/Logger.ts new file mode 100644 index 0000000000..a926ff43e2 --- /dev/null +++ b/server-sdk-common/__tests__/Logger.ts @@ -0,0 +1,62 @@ +import { LDLogger } from '../src'; + +export default class TestLogger implements LDLogger { + public readonly errorMessages: string[] = []; + + public readonly warningMessages: string[] = []; + + public readonly infoMessages: string[] = []; + + public readonly debugMessages: string[] = []; + + private callCount = 0; + + private waiters: Array<() => void> = []; + + timeout(timeoutMs: number): Promise { + return new Promise((resolve) => { + setTimeout(() => resolve(this.callCount), timeoutMs); + }); + } + + async waitForMessages(count: number, timeoutMs: number = 1000): Promise { + return Promise.race([ + new Promise((resolve) => { + const waiter = () => { + if (this.callCount >= count) { + resolve(this.callCount); + } + }; + waiter(); + this.waiters.push(waiter); + }), this.timeout(timeoutMs)]); + } + + getCount() { + return this.callCount; + } + + checkResolves() { + this.waiters.forEach((waiter) => waiter()); + } + + error(...args: any[]): void { + this.errorMessages.push(args.join(' ')); + this.callCount += 1; + } + + warn(...args: any[]): void { + this.warningMessages.push(args.join(' ')); + this.callCount += 1; + } + + info(...args: any[]): void { + this.infoMessages.push(args.join(' ')); + this.callCount += 1; + } + + debug(...args: any[]): void { + this.debugMessages.push(args.join(' ')); + this.callCount += 1; + } +} diff --git a/server-sdk-common/__tests__/options/ApplicationTags.test.ts b/server-sdk-common/__tests__/options/ApplicationTags.test.ts index 2363cb9d8e..c94c8b59fa 100644 --- a/server-sdk-common/__tests__/options/ApplicationTags.test.ts +++ b/server-sdk-common/__tests__/options/ApplicationTags.test.ts @@ -1,16 +1,43 @@ import ApplicationTags from '../../src/options/ApplicationTags'; import { ValidatedOptions } from '../../src/options/ValidatedOptions'; +import TestLogger from '../Logger'; describe.each([ - [{application: {id: 'is-valid', version: 'also-valid'}}, "application-id/is-valid application-version/also-valid"], - [{application: {id: 'is-valid'}}, "application-id/is-valid"], - [{application: {version: 'also-valid'}}, "application-version/also-valid"], - [{application: {}}, undefined], - [{}, undefined], - [undefined, undefined] -])('given application tags configurations', (config, result) => { + [ + { application: { id: 'is-valid', version: 'also-valid' }, logger: new TestLogger() }, + 'application-id/is-valid application-version/also-valid', 0, + ], + [{ application: { id: 'is-valid' }, logger: new TestLogger() }, 'application-id/is-valid', 0], + [{ application: { version: 'also-valid' }, logger: new TestLogger() }, 'application-version/also-valid', 0], + [{ application: {}, logger: new TestLogger() }, undefined, 0], + [{ logger: new TestLogger() }, undefined, 0], + [undefined, undefined, undefined], + + // Above ones are 'valid' cases. Below are invalid. + [ + { application: { id: 'bad tag' }, logger: new TestLogger() }, + undefined, 1, + ], + [ + { application: { id: 'bad tag', version: 'good-tag' }, logger: new TestLogger() }, + 'application-version/good-tag', 1, + ], + [ + { application: { id: 'bad tag', version: 'also bad' }, logger: new TestLogger() }, + undefined, 2, + ], + // Bad tags and no logger. + [ + { application: { id: 'bad tag', version: 'also bad' }, logger: undefined }, + undefined, undefined, + ], +])('given application tags configurations', (config, result, logCount) => { it('produces the correct tag values', () => { const tags = new ApplicationTags(config as unknown as ValidatedOptions); expect(tags.value).toEqual(result); }); -}); \ No newline at end of file + + it('logs issues it encounters', () => { + expect(config?.logger?.warningMessages.length).toEqual(logCount); + }); +}); diff --git a/server-sdk-common/__tests__/options/validators.test.ts b/server-sdk-common/__tests__/options/validators.test.ts index e36d7eae9b..3eb73b57ec 100644 --- a/server-sdk-common/__tests__/options/validators.test.ts +++ b/server-sdk-common/__tests__/options/validators.test.ts @@ -87,7 +87,7 @@ describe('given a regex validator', () => { describe.each([ [TypeValidators.NumberWithMin(0), 0], - [TypeValidators.NumberWithMin(10), 10], + [TypeValidators.NumberWithMin(10), 10], [TypeValidators.NumberWithMin(1000), 1000], ])('given minumum number validators', (validator, min) => { it('validates numbers equal or above the minimum', () => { From cd8357aa6a82ecff867b7491e2d9459472f927e6 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 24 May 2022 11:09:49 -0700 Subject: [PATCH 19/39] Start testing the Configuration class. --- .../__tests__/options/Configuration.test.ts | 37 +++++++++++++++++++ .../options/ServiceEndpoints.test.ts | 19 ++++++++++ .../src/options/Configuration.ts | 8 +++- .../src/options/ServiceEndpoints.ts | 12 ++---- 4 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 server-sdk-common/__tests__/options/Configuration.test.ts create mode 100644 server-sdk-common/__tests__/options/ServiceEndpoints.test.ts diff --git a/server-sdk-common/__tests__/options/Configuration.test.ts b/server-sdk-common/__tests__/options/Configuration.test.ts new file mode 100644 index 0000000000..52403bd2ce --- /dev/null +++ b/server-sdk-common/__tests__/options/Configuration.test.ts @@ -0,0 +1,37 @@ +import Configuration from '../../src/options/Configuration'; + +describe.each([ + undefined, null, "potat0", 17, [], {} +])('constructed without options', (input) => { + it('should have default options', () => { + // JavaScript is not going to stop you from calling this with whatever + // you want. So we need to tell TS to ingore our bad behavior. + // @ts-ignore + const config = new Configuration(input); + + expect(config.allAttributesPrivate).toBe(false); + expect(config.contextKeysCapacity).toBe(1000); + expect(config.contextKeysFlushInterval).toBe(300); + expect(config.diagnosticOptOut).toBe(false); + expect(config.eventsCapacity).toBe(10000); + expect(config.flushInterval).toBe(5); + expect(config.logger).toBeUndefined(); + expect(config.offline).toBe(false); + expect(config.pollInterval).toBe(30); + expect(config.privateAttributes).toStrictEqual([]); + expect(config.proxyOptions).toBeUndefined(); + expect(config.sendEvents).toBe(true); + expect(config.serviceEndpoints.streaming).toEqual('https://stream.launchdarkly.com'); + expect(config.serviceEndpoints.polling).toEqual('https://sdk.launchdarkly.com'); + expect(config.serviceEndpoints.events).toEqual('https://events.launchdarkly.com'); + expect(config.stream).toBe(true); + expect(config.streamInitialReconnectDelay).toEqual(1); + expect(config.tags.value).toBeUndefined(); + expect(config.timeout).toEqual(5); + expect(config.tlsParams).toBeUndefined(); + expect(config.useLdd).toBe(false); + expect(config.wrapperName).toBeUndefined(); + expect(config.wrapperVersion).toBeUndefined(); + + }); +}); \ No newline at end of file diff --git a/server-sdk-common/__tests__/options/ServiceEndpoints.test.ts b/server-sdk-common/__tests__/options/ServiceEndpoints.test.ts new file mode 100644 index 0000000000..f12bdb2c70 --- /dev/null +++ b/server-sdk-common/__tests__/options/ServiceEndpoints.test.ts @@ -0,0 +1,19 @@ +import ServiceEndpoints from '../../src/options/ServiceEndpoints'; + +describe.each([ + [ + {baseUri: "https://sdk.launchdarkly.com", streamingUri: "https://stream.launchdarkly.com", eventsUri: "https://events.launchdarkly.com"}, + {baseUri: "https://sdk.launchdarkly.com", streamingUri: "https://stream.launchdarkly.com", eventsUri: "https://events.launchdarkly.com"} + ], + [ + {baseUri: "https://sdk.launchdarkly.com/", streamingUri: "https://stream.launchdarkly.com/", eventsUri: "https://events.launchdarkly.com/"}, + {baseUri: "https://sdk.launchdarkly.com", streamingUri: "https://stream.launchdarkly.com", eventsUri: "https://events.launchdarkly.com"} + ] +])('given endpoint urls', (input, expected) => { + it('has canonical urls', () => { + const endpoints = new ServiceEndpoints(input.streamingUri, input.baseUri, input.eventsUri); + expect(endpoints.streaming).toEqual(expected.streamingUri); + expect(endpoints.polling).toEqual(expected.baseUri); + expect(endpoints.events).toEqual(expected.eventsUri); + }); +}); \ No newline at end of file diff --git a/server-sdk-common/src/options/Configuration.ts b/server-sdk-common/src/options/Configuration.ts index 0b8547a41b..073d08543c 100644 --- a/server-sdk-common/src/options/Configuration.ts +++ b/server-sdk-common/src/options/Configuration.ts @@ -47,7 +47,7 @@ const validations: Record = { }; const defaultValues: ValidatedOptions = { - baseUri: 'https://app.launchdarkly.com', + baseUri: 'https://sdk.launchdarkly.com', streamUri: 'https://stream.launchdarkly.com', eventsUri: 'https://events.launchdarkly.com', stream: true, @@ -153,7 +153,11 @@ export default class Configuration { public readonly tags: ApplicationTags; - constructor(options: LDOptions) { + constructor(options: LDOptions = {}) { + // The default will handle undefined, but not null. + // Because we can be called from JS we need to be extra defensive. + // eslint-disable-next-line no-param-reassign + options = options || {}; // If there isn't a valid logger from the platform, then logs would go nowhere. this.logger = options.logger; diff --git a/server-sdk-common/src/options/ServiceEndpoints.ts b/server-sdk-common/src/options/ServiceEndpoints.ts index bad5fef371..567df6c81c 100644 --- a/server-sdk-common/src/options/ServiceEndpoints.ts +++ b/server-sdk-common/src/options/ServiceEndpoints.ts @@ -8,17 +8,11 @@ function canonicalizeUri(uri: string): string { * @internal */ export default class ServiceEndpoints { - private streaming: string; + public readonly streaming: string; - private polling: string; + public readonly polling: string; - private events: string; - - public get streamUri() { return this.streaming; } - - public get pollingUri() { return this.polling; } - - public get eventsUri() { return this.events; } + public readonly events: string; public constructor(streaming: string, polling: string, events: string) { this.streaming = canonicalizeUri(streaming); From 7ea76b5a5331fc46ca19505ce0fd79c299aaeb21 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 24 May 2022 11:20:09 -0700 Subject: [PATCH 20/39] Fix validation. Start on individual option tests. --- .../__tests__/options/Configuration.test.ts | 19 +++++++++++++++++++ .../src/options/Configuration.ts | 4 ++++ 2 files changed, 23 insertions(+) diff --git a/server-sdk-common/__tests__/options/Configuration.test.ts b/server-sdk-common/__tests__/options/Configuration.test.ts index 52403bd2ce..54793d54d3 100644 --- a/server-sdk-common/__tests__/options/Configuration.test.ts +++ b/server-sdk-common/__tests__/options/Configuration.test.ts @@ -1,4 +1,14 @@ +import { LDOptions } from '../../src'; import Configuration from '../../src/options/Configuration'; +import TestLogger from '../Logger'; + +function withLogger(options: LDOptions): LDOptions { + return {...options, logger: new TestLogger()}; +} + +function logger(options: LDOptions): TestLogger { + return options.logger as TestLogger; +} describe.each([ undefined, null, "potat0", 17, [], {} @@ -32,6 +42,15 @@ describe.each([ expect(config.useLdd).toBe(false); expect(config.wrapperName).toBeUndefined(); expect(config.wrapperVersion).toBeUndefined(); + }); +}); +describe('when setting different options', () => { + it.each([ + ["http://cats.launchdarkly.com", "http://cats.launchdarkly.com", 0] + ])('allows setting the baseUri and validates the baseUri', (uri, expected, warnings) => { + const config = new Configuration(withLogger({baseUri: uri})); + expect(config.serviceEndpoints.polling).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); }); }); \ No newline at end of file diff --git a/server-sdk-common/src/options/Configuration.ts b/server-sdk-common/src/options/Configuration.ts index 073d08543c..1862787775 100644 --- a/server-sdk-common/src/options/Configuration.ts +++ b/server-sdk-common/src/options/Configuration.ts @@ -98,7 +98,11 @@ function validateTypesAndNames(options: LDOptions): { )); validatedOptions[optionName] = defaultValues[optionName]; } + } else { + validatedOptions[optionName] = optionValue; } + } else { + options.logger?.warn(OptionMessages.unknownOption(optionName)); } }); return { errors, validatedOptions }; From 82788fd6b0b5b5d287b0d8fce678d1fd0381acdd Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 24 May 2022 12:31:52 -0700 Subject: [PATCH 21/39] Add remaining config tests. --- .../__tests__/options/Configuration.test.ts | 244 +++++++++++++++++- .../options/ServiceEndpoints.test.ts | 12 +- .../src/options/Configuration.ts | 7 +- 3 files changed, 251 insertions(+), 12 deletions(-) diff --git a/server-sdk-common/__tests__/options/Configuration.test.ts b/server-sdk-common/__tests__/options/Configuration.test.ts index 54793d54d3..a84b6e09ea 100644 --- a/server-sdk-common/__tests__/options/Configuration.test.ts +++ b/server-sdk-common/__tests__/options/Configuration.test.ts @@ -1,9 +1,10 @@ import { LDOptions } from '../../src'; import Configuration from '../../src/options/Configuration'; +import OptionMessages from '../../src/options/OptionMessages'; import TestLogger from '../Logger'; function withLogger(options: LDOptions): LDOptions { - return {...options, logger: new TestLogger()}; + return { ...options, logger: new TestLogger() }; } function logger(options: LDOptions): TestLogger { @@ -11,7 +12,7 @@ function logger(options: LDOptions): TestLogger { } describe.each([ - undefined, null, "potat0", 17, [], {} + undefined, null, 'potat0', 17, [], {}, ])('constructed without options', (input) => { it('should have default options', () => { // JavaScript is not going to stop you from calling this with whatever @@ -47,10 +48,243 @@ describe.each([ describe('when setting different options', () => { it.each([ - ["http://cats.launchdarkly.com", "http://cats.launchdarkly.com", 0] + ['http://cats.launchdarkly.com', 'http://cats.launchdarkly.com', 0], + ['http://cats.launchdarkly.com/', 'http://cats.launchdarkly.com', 0], + [0, 'https://sdk.launchdarkly.com', 1], ])('allows setting the baseUri and validates the baseUri', (uri, expected, warnings) => { - const config = new Configuration(withLogger({baseUri: uri})); + // @ts-ignore + const config = new Configuration(withLogger({ baseUri: uri })); expect(config.serviceEndpoints.polling).toEqual(expected); expect(logger(config).warningMessages.length).toEqual(warnings); + if (warnings) { + expect(logger(config).warningMessages[0]).toEqual( + OptionMessages.wrongOptionType('baseUri', 'string', typeof uri), + ); + } + }); + + it.each([ + ['http://cats.launchdarkly.com', 'http://cats.launchdarkly.com', 0], + ['http://cats.launchdarkly.com/', 'http://cats.launchdarkly.com', 0], + [0, 'https://stream.launchdarkly.com', 1], + ])('allows setting the streamUri and validates the streamUri', (uri, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ streamUri: uri })); + expect(config.serviceEndpoints.streaming).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); }); -}); \ No newline at end of file + + it.each([ + ['http://cats.launchdarkly.com', 'http://cats.launchdarkly.com', 0], + ['http://cats.launchdarkly.com/', 'http://cats.launchdarkly.com', 0], + [0, 'https://events.launchdarkly.com', 1], + ])('allows setting the eventsUri and validates the eventsUri', (uri, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ eventsUri: uri })); + expect(config.serviceEndpoints.events).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [0, 0, 0], + [6, 6, 0], + ['potato', 5, 1], + ])('allow setting timeout and validates timeout', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ timeout: value })); + expect(config.timeout).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [0, 0, 0], + [6, 6, 0], + ['potato', 10000, 1], + ])('allow setting and validates capacity', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ capacity: value })); + expect(config.eventsCapacity).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [0, 0, 0], + [6, 6, 0], + ['potato', 5, 1], + ])('allow setting and validates flushInterval', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ flushInterval: value })); + expect(config.flushInterval).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [0, 30, 1], + [500, 500, 0], + ['potato', 30, 1], + ])('allow setting and validates pollInterval', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ pollInterval: value })); + expect(config.pollInterval).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [false, false, 0], + [true, true, 0], + ['', false, 1], + ['true', true, 1], + [0, false, 1], + [1, true, 1], + ])('allows setting stream and validates offline', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ offline: value })); + expect(config.offline).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [false, false, 0], + [true, true, 0], + ['', false, 1], + ['true', true, 1], + [0, false, 1], + [1, true, 1], + ])('allows setting stream and validates stream', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ stream: value })); + expect(config.stream).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [false, false, 0], + [true, true, 0], + ['', false, 1], + ['true', true, 1], + [0, false, 1], + [1, true, 1], + ])('allows setting stream and validates useLdd', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ useLdd: value })); + expect(config.useLdd).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [false, false, 0], + [true, true, 0], + ['', false, 1], + ['true', true, 1], + [0, false, 1], + [1, true, 1], + ])('allows setting stream and validates sendEvents', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ sendEvents: value })); + expect(config.sendEvents).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [false, false, 0], + [true, true, 0], + ['', false, 1], + ['true', true, 1], + [0, false, 1], + [1, true, 1], + ])('allows setting stream and validates allAttributesPrivate', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ allAttributesPrivate: value })); + expect(config.allAttributesPrivate).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [['a', 'b', 'c'], ['a', 'b', 'c'], 0], + [[], [], 0], + [[0], [], 1], + ['potato', [], 1], + ])('allows setting and validates privateAttributes', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ privateAttributes: value })); + expect(config.privateAttributes).toStrictEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [0, 0, 0], + [500, 500, 0], + ['potato', 1000, 1], + ])('allow setting and validates contextKeysCapacity', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ contextKeysCapacity: value })); + expect(config.contextKeysCapacity).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [0, 0, 0], + [500, 500, 0], + ['potato', 300, 1], + ])('allow setting and validates contextKeysFlushInterval', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ contextKeysFlushInterval: value })); + expect(config.contextKeysFlushInterval).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [false, false, 0], + [true, true, 0], + ['', false, 1], + ['true', true, 1], + [0, false, 1], + [1, true, 1], + ])('allows setting stream and validates diagnosticOptOut', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ diagnosticOptOut: value })); + expect(config.diagnosticOptOut).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); + + it.each([ + [0, 60, 1], + [500, 500, 0], + ['potato', 900, 1], + ])('allow setting and validates diagnosticRecordingInterval', (value, expected, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ diagnosticRecordingInterval: value })); + expect(config.diagnosticRecordingInterval).toEqual(expected); + expect(logger(config).warningMessages.length).toEqual(warnings); + if (warnings) { + expect(logger(config).warningMessages[0]).toEqual( + value < 60 + ? OptionMessages.optionBelowMinimum('diagnosticRecordingInterval', value as number, 60) + : OptionMessages.wrongOptionType('diagnosticRecordingInterval', 'number with minimum value of 60', typeof value), + ); + } + }); + + it('discards unrecognized options with a warning', () => { + // @ts-ignore + const config = new Configuration(withLogger({ yes: 'no', cat: 'yes' })); + expect(logger(config).warningMessages.length).toEqual(2); + + expect(logger(config).warningMessages[0]).toEqual( + OptionMessages.unknownOption('yes'), + ); + expect(logger(config).warningMessages[1]).toEqual( + OptionMessages.unknownOption('cat'), + ); + }); + + // This is more thoroughly tested in the application tags test. + it.each([ + [{application: {id: 'valid-id', version: 'valid-version'}}, 0], + [{application: "tomato"}, 1] + ])('handles application tag settings', (values, warnings) => { + // @ts-ignore + const config = new Configuration(withLogger({ ...values })); + expect(logger(config).warningMessages.length).toEqual(warnings); + }); +}); diff --git a/server-sdk-common/__tests__/options/ServiceEndpoints.test.ts b/server-sdk-common/__tests__/options/ServiceEndpoints.test.ts index f12bdb2c70..4877adbd84 100644 --- a/server-sdk-common/__tests__/options/ServiceEndpoints.test.ts +++ b/server-sdk-common/__tests__/options/ServiceEndpoints.test.ts @@ -2,13 +2,13 @@ import ServiceEndpoints from '../../src/options/ServiceEndpoints'; describe.each([ [ - {baseUri: "https://sdk.launchdarkly.com", streamingUri: "https://stream.launchdarkly.com", eventsUri: "https://events.launchdarkly.com"}, - {baseUri: "https://sdk.launchdarkly.com", streamingUri: "https://stream.launchdarkly.com", eventsUri: "https://events.launchdarkly.com"} + { baseUri: 'https://sdk.launchdarkly.com', streamingUri: 'https://stream.launchdarkly.com', eventsUri: 'https://events.launchdarkly.com' }, + { baseUri: 'https://sdk.launchdarkly.com', streamingUri: 'https://stream.launchdarkly.com', eventsUri: 'https://events.launchdarkly.com' }, ], [ - {baseUri: "https://sdk.launchdarkly.com/", streamingUri: "https://stream.launchdarkly.com/", eventsUri: "https://events.launchdarkly.com/"}, - {baseUri: "https://sdk.launchdarkly.com", streamingUri: "https://stream.launchdarkly.com", eventsUri: "https://events.launchdarkly.com"} - ] + { baseUri: 'https://sdk.launchdarkly.com/', streamingUri: 'https://stream.launchdarkly.com/', eventsUri: 'https://events.launchdarkly.com/' }, + { baseUri: 'https://sdk.launchdarkly.com', streamingUri: 'https://stream.launchdarkly.com', eventsUri: 'https://events.launchdarkly.com' }, + ], ])('given endpoint urls', (input, expected) => { it('has canonical urls', () => { const endpoints = new ServiceEndpoints(input.streamingUri, input.baseUri, input.eventsUri); @@ -16,4 +16,4 @@ describe.each([ expect(endpoints.polling).toEqual(expected.baseUri); expect(endpoints.events).toEqual(expected.eventsUri); }); -}); \ No newline at end of file +}); diff --git a/server-sdk-common/src/options/Configuration.ts b/server-sdk-common/src/options/Configuration.ts index 1862787775..9010a1cedc 100644 --- a/server-sdk-common/src/options/Configuration.ts +++ b/server-sdk-common/src/options/Configuration.ts @@ -87,9 +87,11 @@ function validateTypesAndNames(options: LDOptions): { typeof optionValue, )); validatedOptions[optionName] = !!optionValue; - } else if (validator instanceof NumberWithMinimum) { + } else if (validator instanceof NumberWithMinimum + && TypeValidators.Number.is(optionValue)) { const { min } = validator as NumberWithMinimum; errors.push(OptionMessages.optionBelowMinimum(optionName, optionValue, min)); + validatedOptions[optionName] = min; } else { errors.push(OptionMessages.wrongOptionType( optionName, @@ -157,6 +159,8 @@ export default class Configuration { public readonly tags: ApplicationTags; + public readonly diagnosticRecordingInterval: number; + constructor(options: LDOptions = {}) { // The default will handle undefined, but not null. // Because we can be called from JS we need to be extra defensive. @@ -198,5 +202,6 @@ export default class Configuration { this.wrapperName = validatedOptions.wrapperName; this.wrapperVersion = validatedOptions.wrapperVersion; this.tags = new ApplicationTags(validatedOptions); + this.diagnosticRecordingInterval = validatedOptions.diagnosticRecordingInterval; } } From 97405841575f28549fc316e2f5b4aaefd47b69d6 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 24 May 2022 12:36:25 -0700 Subject: [PATCH 22/39] Make sure to check resolves in the logger. --- server-sdk-common/__tests__/Logger.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server-sdk-common/__tests__/Logger.ts b/server-sdk-common/__tests__/Logger.ts index a926ff43e2..8c5928b358 100644 --- a/server-sdk-common/__tests__/Logger.ts +++ b/server-sdk-common/__tests__/Logger.ts @@ -36,27 +36,31 @@ export default class TestLogger implements LDLogger { return this.callCount; } - checkResolves() { + private checkResolves() { this.waiters.forEach((waiter) => waiter()); } error(...args: any[]): void { this.errorMessages.push(args.join(' ')); this.callCount += 1; + this.checkResolves(); } warn(...args: any[]): void { this.warningMessages.push(args.join(' ')); this.callCount += 1; + this.checkResolves(); } info(...args: any[]): void { this.infoMessages.push(args.join(' ')); this.callCount += 1; + this.checkResolves(); } debug(...args: any[]): void { this.debugMessages.push(args.join(' ')); this.callCount += 1; + this.checkResolves(); } } From 2517279744be944a4b3e5f7c6ef9fec55069ca25 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 24 May 2022 14:54:25 -0700 Subject: [PATCH 23/39] Start implementation. --- sdk-common/src/AttributeReference.ts | 46 ++++++++++++++++++++++++++++ sdk-common/src/Context.ts | 0 2 files changed, 46 insertions(+) create mode 100644 sdk-common/src/AttributeReference.ts create mode 100644 sdk-common/src/Context.ts diff --git a/sdk-common/src/AttributeReference.ts b/sdk-common/src/AttributeReference.ts new file mode 100644 index 0000000000..58af40e70f --- /dev/null +++ b/sdk-common/src/AttributeReference.ts @@ -0,0 +1,46 @@ +function processEscapeCharacters(value: string): string { + return value.replace(/~/g, '~0').replace(/\//g, '~1'); +} + +function getComponents(reference: string): string[] { + const referenceWithoutPrefix = reference.startsWith('/') ? reference.substring(1) : reference; + return referenceWithoutPrefix + .split('/') + .map(component => (component.indexOf('~') >= 0 ? processEscapeCharacters(component) : component)); +} + +function isLiteral(reference: string) { + return !reference.startsWith('/'); +} + +function isValid(reference: string) { + return !reference.match(/\/\/|(^\/.*~[^0|^1])|~$/); +} + +export default class AttributeReference { + public readonly isValid; + public readonly original; + + private readonly components?: string[]; + + constructor(reference: string) { + this.original = reference; + if(reference === "" || reference === "/" || !isValid(reference)) { + this.isValid = false; + } else if(isLiteral(reference)) { + this.components = [reference]; + } else if(reference.indexOf('/', 1) < 0) { + this.components = [reference.slice(1)] + } else { + this.components = getComponents(reference); + } + } + + public depth(): number | undefined { + return this.components?.length; + } + + public getComponent(index: number): string | undefined { + return this.components?.[index]; + } +} \ No newline at end of file diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts new file mode 100644 index 0000000000..e69de29bb2 From b5731517eed61f8f5a12c301eae1e2abb55669ad Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 25 May 2022 10:22:06 -0700 Subject: [PATCH 24/39] Implement check for service endpoints consistency. Improve testing methodology for logs. --- sdk-common/src/Context.ts | 19 ++ server-sdk-common/__tests__/Logger.ts | 75 ++++++-- .../__tests__/options/ApplicationTags.test.ts | 32 ++-- .../__tests__/options/Configuration.test.ts | 166 +++++++++++------- .../src/options/Configuration.ts | 27 +++ .../src/options/OptionMessages.ts | 4 + 6 files changed, 228 insertions(+), 95 deletions(-) create mode 100644 sdk-common/src/Context.ts diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts new file mode 100644 index 0000000000..653630b8b3 --- /dev/null +++ b/sdk-common/src/Context.ts @@ -0,0 +1,19 @@ +type MultiKind = { + kind: 'multi', + isMulti: true, +}; + +type SingleKind = { + kind: string + isMulti: false +}; + +type Kind = MultiKind | SingleKind; + +export default class Context { + public readonly kind: Kind; + + private contexts?: Context[]; + + private attributes?: Record; +} diff --git a/server-sdk-common/__tests__/Logger.ts b/server-sdk-common/__tests__/Logger.ts index 8c5928b358..631af53efa 100644 --- a/server-sdk-common/__tests__/Logger.ts +++ b/server-sdk-common/__tests__/Logger.ts @@ -1,13 +1,20 @@ import { LDLogger } from '../src'; -export default class TestLogger implements LDLogger { - public readonly errorMessages: string[] = []; - - public readonly warningMessages: string[] = []; - - public readonly infoMessages: string[] = []; +// TODO: Move this to sdk-common when implementing logging. +export enum LogLevel { + Debug, + Info, + Warn, + Error, +} - public readonly debugMessages: string[] = []; +export default class TestLogger implements LDLogger { + private readonly messages: Record = { + [LogLevel.Debug]: [], + [LogLevel.Info]: [], + [LogLevel.Warn]: [], + [LogLevel.Error]: [], + }; private callCount = 0; @@ -32,6 +39,40 @@ export default class TestLogger implements LDLogger { }), this.timeout(timeoutMs)]); } + /** + * Check received messages for expected messages. + * + * @param expectedMessages List of expected messages. If a message is expected + * more than once, then it should be included multiple times. + * @returns A list of messages that were not received. + */ + verifyMessages( + expectedMessages: { level: LogLevel, matches: RegExp }[], + ): string[] { + const missing: string[] = []; + const matched: Record = { + [LogLevel.Debug]: [], + [LogLevel.Info]: [], + [LogLevel.Warn]: [], + [LogLevel.Error]: [], + }; + + expectedMessages.forEach((expectedMessage) => { + const received = this.messages[expectedMessage.level]; + const index = received.findIndex( + (receivedMessage) => receivedMessage.match(expectedMessage.matches), + ); + if (index < 0) { + missing.push(expectedMessage.matches.toString()); + } else if (matched[expectedMessage.level].indexOf(index) >= 0) { + missing.push(`did not get expected message ${expectedMessage.matches}`); + } else { + matched[expectedMessage.level].push(index); + } + }); + return missing; + } + getCount() { return this.callCount; } @@ -40,27 +81,25 @@ export default class TestLogger implements LDLogger { this.waiters.forEach((waiter) => waiter()); } - error(...args: any[]): void { - this.errorMessages.push(args.join(' ')); + private log(level: LogLevel, ...args: any[]) { + this.messages[level].push(args.join(' ')); this.callCount += 1; this.checkResolves(); } + error(...args: any[]): void { + this.log(LogLevel.Error, args); + } + warn(...args: any[]): void { - this.warningMessages.push(args.join(' ')); - this.callCount += 1; - this.checkResolves(); + this.log(LogLevel.Warn, args); } info(...args: any[]): void { - this.infoMessages.push(args.join(' ')); - this.callCount += 1; - this.checkResolves(); + this.log(LogLevel.Info, args); } debug(...args: any[]): void { - this.debugMessages.push(args.join(' ')); - this.callCount += 1; - this.checkResolves(); + this.log(LogLevel.Debug, args); } } diff --git a/server-sdk-common/__tests__/options/ApplicationTags.test.ts b/server-sdk-common/__tests__/options/ApplicationTags.test.ts index c94c8b59fa..ee0b3929f2 100644 --- a/server-sdk-common/__tests__/options/ApplicationTags.test.ts +++ b/server-sdk-common/__tests__/options/ApplicationTags.test.ts @@ -1,43 +1,53 @@ import ApplicationTags from '../../src/options/ApplicationTags'; import { ValidatedOptions } from '../../src/options/ValidatedOptions'; -import TestLogger from '../Logger'; +import TestLogger, { LogLevel } from '../Logger'; describe.each([ [ { application: { id: 'is-valid', version: 'also-valid' }, logger: new TestLogger() }, - 'application-id/is-valid application-version/also-valid', 0, + 'application-id/is-valid application-version/also-valid', [], ], - [{ application: { id: 'is-valid' }, logger: new TestLogger() }, 'application-id/is-valid', 0], - [{ application: { version: 'also-valid' }, logger: new TestLogger() }, 'application-version/also-valid', 0], - [{ application: {}, logger: new TestLogger() }, undefined, 0], - [{ logger: new TestLogger() }, undefined, 0], + [{ application: { id: 'is-valid' }, logger: new TestLogger() }, 'application-id/is-valid', []], + [{ application: { version: 'also-valid' }, logger: new TestLogger() }, 'application-version/also-valid', []], + [{ application: {}, logger: new TestLogger() }, undefined, []], + [{ logger: new TestLogger() }, undefined, []], [undefined, undefined, undefined], // Above ones are 'valid' cases. Below are invalid. [ { application: { id: 'bad tag' }, logger: new TestLogger() }, - undefined, 1, + undefined, [ + { level: LogLevel.Warn, matches: /Config option "application.id" must/ }, + ], ], [ { application: { id: 'bad tag', version: 'good-tag' }, logger: new TestLogger() }, - 'application-version/good-tag', 1, + 'application-version/good-tag', [ + { level: LogLevel.Warn, matches: /Config option "application.id" must/ }, + ], ], [ { application: { id: 'bad tag', version: 'also bad' }, logger: new TestLogger() }, - undefined, 2, + undefined, [ + { level: LogLevel.Warn, matches: /Config option "application.id" must/ }, + { level: LogLevel.Warn, matches: /Config option "application.version" must/ }, + ], ], // Bad tags and no logger. [ { application: { id: 'bad tag', version: 'also bad' }, logger: undefined }, undefined, undefined, ], -])('given application tags configurations', (config, result, logCount) => { +])('given application tags configurations', (config, result, logs) => { it('produces the correct tag values', () => { const tags = new ApplicationTags(config as unknown as ValidatedOptions); expect(tags.value).toEqual(result); }); it('logs issues it encounters', () => { - expect(config?.logger?.warningMessages.length).toEqual(logCount); + expect(config?.logger?.getCount()).toEqual(logs?.length); + if (logs) { + config?.logger?.verifyMessages(logs).forEach((message) => expect(message).toBeUndefined()); + } }); }); diff --git a/server-sdk-common/__tests__/options/Configuration.test.ts b/server-sdk-common/__tests__/options/Configuration.test.ts index a84b6e09ea..ff4d44dcfc 100644 --- a/server-sdk-common/__tests__/options/Configuration.test.ts +++ b/server-sdk-common/__tests__/options/Configuration.test.ts @@ -1,7 +1,6 @@ import { LDOptions } from '../../src'; import Configuration from '../../src/options/Configuration'; -import OptionMessages from '../../src/options/OptionMessages'; -import TestLogger from '../Logger'; +import TestLogger, { LogLevel } from '../Logger'; function withLogger(options: LDOptions): LDOptions { return { ...options, logger: new TestLogger() }; @@ -48,63 +47,100 @@ describe.each([ describe('when setting different options', () => { it.each([ - ['http://cats.launchdarkly.com', 'http://cats.launchdarkly.com', 0], - ['http://cats.launchdarkly.com/', 'http://cats.launchdarkly.com', 0], - [0, 'https://sdk.launchdarkly.com', 1], - ])('allows setting the baseUri and validates the baseUri', (uri, expected, warnings) => { + ['http://cats.launchdarkly.com', 'http://cats.launchdarkly.com', [ + { level: LogLevel.Warn, matches: /You have set custom uris without.* streamUri/ }, + { level: LogLevel.Warn, matches: /You have set custom uris without.* eventsUri/ }, + ]], + ['http://cats.launchdarkly.com/', 'http://cats.launchdarkly.com', [ + { level: LogLevel.Warn, matches: /You have set custom uris without.* streamUri/ }, + { level: LogLevel.Warn, matches: /You have set custom uris without.* eventsUri/ }, + ]], + [0, 'https://sdk.launchdarkly.com', [ + { level: LogLevel.Warn, matches: /Config option "baseUri" should be of type/ }, + { level: LogLevel.Warn, matches: /You have set custom uris without.* streamUri/ }, + { level: LogLevel.Warn, matches: /You have set custom uris without.* eventsUri/ }, + ]], + ])('allows setting the baseUri and validates the baseUri', (uri, expected, logs) => { // @ts-ignore const config = new Configuration(withLogger({ baseUri: uri })); expect(config.serviceEndpoints.polling).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); - if (warnings) { - expect(logger(config).warningMessages[0]).toEqual( - OptionMessages.wrongOptionType('baseUri', 'string', typeof uri), - ); - } + expect(logger(config).getCount()).toEqual(logs.length); + // There should not be any messages, so checking them for undefined is a workaround + // for a lack of pure assert. + logger(config).verifyMessages(logs).forEach((message) => expect(message).toBeUndefined()); }); it.each([ - ['http://cats.launchdarkly.com', 'http://cats.launchdarkly.com', 0], - ['http://cats.launchdarkly.com/', 'http://cats.launchdarkly.com', 0], - [0, 'https://stream.launchdarkly.com', 1], + ['http://cats.launchdarkly.com', 'http://cats.launchdarkly.com', 2], + ['http://cats.launchdarkly.com/', 'http://cats.launchdarkly.com', 2], + [0, 'https://stream.launchdarkly.com', 3], ])('allows setting the streamUri and validates the streamUri', (uri, expected, warnings) => { // @ts-ignore const config = new Configuration(withLogger({ streamUri: uri })); expect(config.serviceEndpoints.streaming).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); }); it.each([ - ['http://cats.launchdarkly.com', 'http://cats.launchdarkly.com', 0], - ['http://cats.launchdarkly.com/', 'http://cats.launchdarkly.com', 0], - [0, 'https://events.launchdarkly.com', 1], + ['http://cats.launchdarkly.com', 'http://cats.launchdarkly.com', 2], + ['http://cats.launchdarkly.com/', 'http://cats.launchdarkly.com', 2], + [0, 'https://events.launchdarkly.com', 3], ])('allows setting the eventsUri and validates the eventsUri', (uri, expected, warnings) => { // @ts-ignore const config = new Configuration(withLogger({ eventsUri: uri })); expect(config.serviceEndpoints.events).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); + }); + + it('produces no logs when setting all URLs.', () => { + // @ts-ignore + const config = new Configuration(withLogger({ eventsUri: 'cats', baseUri: 'cats', streamUri: 'cats' })); + expect(config.serviceEndpoints.events).toEqual('cats'); + expect(config.serviceEndpoints.streaming).toEqual('cats'); + expect(config.serviceEndpoints.polling).toEqual('cats'); + expect(logger(config).getCount()).toEqual(0); + }); + + it('Does not log a warning for the events URI if sendEvents is false..', () => { + // @ts-ignore + const config = new Configuration(withLogger({ sendEvents: false, baseUri: 'cats', streamUri: 'cats' })); + expect(config.serviceEndpoints.streaming).toEqual('cats'); + expect(config.serviceEndpoints.polling).toEqual('cats'); + expect(logger(config).getCount()).toEqual(0); + }); + + it('Does log a warning for the events URI if sendEvents is true..', () => { + // @ts-ignore + const config = new Configuration(withLogger({ sendEvents: true, baseUri: 'cats', streamUri: 'cats' })); + expect(config.serviceEndpoints.streaming).toEqual('cats'); + expect(config.serviceEndpoints.polling).toEqual('cats'); + expect(logger(config).getCount()).toEqual(1); }); it.each([ - [0, 0, 0], - [6, 6, 0], - ['potato', 5, 1], - ])('allow setting timeout and validates timeout', (value, expected, warnings) => { + [0, 0, []], + [6, 6, []], + ['potato', 5, [ + { level: LogLevel.Warn, matches: /Config option "timeout" should be of type/ }, + ]], + ])('allow setting timeout and validates timeout', (value, expected, logs) => { // @ts-ignore const config = new Configuration(withLogger({ timeout: value })); expect(config.timeout).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + logger(config).verifyMessages(logs).forEach((message) => expect(message).toBeUndefined()); }); it.each([ - [0, 0, 0], - [6, 6, 0], - ['potato', 10000, 1], - ])('allow setting and validates capacity', (value, expected, warnings) => { + [0, 0, []], + [6, 6, []], + ['potato', 10000, [ + { level: LogLevel.Warn, matches: /Config option "capacity" should be of type/ }, + ]], + ])('allow setting and validates capacity', (value, expected, logs) => { // @ts-ignore const config = new Configuration(withLogger({ capacity: value })); expect(config.eventsCapacity).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + logger(config).verifyMessages(logs).forEach((message) => expect(message).toBeUndefined()); }); it.each([ @@ -115,7 +151,7 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ flushInterval: value })); expect(config.flushInterval).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); }); it.each([ @@ -126,7 +162,7 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ pollInterval: value })); expect(config.pollInterval).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); }); it.each([ @@ -140,7 +176,7 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ offline: value })); expect(config.offline).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); }); it.each([ @@ -154,7 +190,7 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ stream: value })); expect(config.stream).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); }); it.each([ @@ -168,7 +204,7 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ useLdd: value })); expect(config.useLdd).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); }); it.each([ @@ -182,7 +218,7 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ sendEvents: value })); expect(config.sendEvents).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); }); it.each([ @@ -196,7 +232,7 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ allAttributesPrivate: value })); expect(config.allAttributesPrivate).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); }); it.each([ @@ -208,7 +244,7 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ privateAttributes: value })); expect(config.privateAttributes).toStrictEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); }); it.each([ @@ -219,7 +255,7 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ contextKeysCapacity: value })); expect(config.contextKeysCapacity).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); }); it.each([ @@ -230,7 +266,7 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ contextKeysFlushInterval: value })); expect(config.contextKeysFlushInterval).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); }); it.each([ @@ -244,47 +280,45 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ diagnosticOptOut: value })); expect(config.diagnosticOptOut).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); + expect(logger(config).getCount()).toEqual(warnings); }); it.each([ - [0, 60, 1], - [500, 500, 0], - ['potato', 900, 1], - ])('allow setting and validates diagnosticRecordingInterval', (value, expected, warnings) => { + [0, 60, [ + { level: LogLevel.Warn, matches: /Config option "diagnosticRecordingInterval" had invalid/ }, + ]], + [500, 500, []], + ['potato', 900, [ + { level: LogLevel.Warn, matches: /Config option "diagnosticRecordingInterval" should be of type/ }, + ]], + ])('allow setting and validates diagnosticRecordingInterval', (value, expected, logs) => { // @ts-ignore const config = new Configuration(withLogger({ diagnosticRecordingInterval: value })); expect(config.diagnosticRecordingInterval).toEqual(expected); - expect(logger(config).warningMessages.length).toEqual(warnings); - if (warnings) { - expect(logger(config).warningMessages[0]).toEqual( - value < 60 - ? OptionMessages.optionBelowMinimum('diagnosticRecordingInterval', value as number, 60) - : OptionMessages.wrongOptionType('diagnosticRecordingInterval', 'number with minimum value of 60', typeof value), - ); - } + logger(config).verifyMessages(logs).forEach((message) => expect(message).toBeUndefined()); }); it('discards unrecognized options with a warning', () => { // @ts-ignore const config = new Configuration(withLogger({ yes: 'no', cat: 'yes' })); - expect(logger(config).warningMessages.length).toEqual(2); - - expect(logger(config).warningMessages[0]).toEqual( - OptionMessages.unknownOption('yes'), - ); - expect(logger(config).warningMessages[1]).toEqual( - OptionMessages.unknownOption('cat'), - ); + expect(logger(config).getCount()).toEqual(2); + logger(config).verifyMessages([ + { + level: LogLevel.Warn, matches: /Ignoring unknown config option "yes"/, + }, + { + level: LogLevel.Warn, matches: /Ignoring unknown config option "cat"/, + }, + ]).forEach((message) => expect(message).toBeUndefined()); }); // This is more thoroughly tested in the application tags test. it.each([ - [{application: {id: 'valid-id', version: 'valid-version'}}, 0], - [{application: "tomato"}, 1] + [{ application: { id: 'valid-id', version: 'valid-version' } }, 0], + [{ application: 'tomato' }, 1], ])('handles application tag settings', (values, warnings) => { - // @ts-ignore - const config = new Configuration(withLogger({ ...values })); - expect(logger(config).warningMessages.length).toEqual(warnings); + // @ts-ignore + const config = new Configuration(withLogger({ ...values })); + expect(logger(config).getCount()).toEqual(warnings); }); }); diff --git a/server-sdk-common/src/options/Configuration.ts b/server-sdk-common/src/options/Configuration.ts index 9010a1cedc..8a6fb4d15e 100644 --- a/server-sdk-common/src/options/Configuration.ts +++ b/server-sdk-common/src/options/Configuration.ts @@ -110,6 +110,31 @@ function validateTypesAndNames(options: LDOptions): { return { errors, validatedOptions }; } +function validateEndpoints(options: LDOptions, validatedOptions: ValidatedOptions) { + const { baseUri, streamUri, eventsUri } = options; + const streamingEndpointSpecified = streamUri !== undefined && streamUri !== null; + const pollingEndpointSpecified = baseUri !== undefined && baseUri !== null; + const eventEndpointSpecified = eventsUri !== undefined && eventsUri !== null; + + if ((streamingEndpointSpecified === pollingEndpointSpecified) + && (streamingEndpointSpecified === eventEndpointSpecified)) { + // Either everything is default, or everything is set. + return; + } + + if (!streamingEndpointSpecified) { + validatedOptions.logger?.warn(OptionMessages.partialEndpoint('streamUri')); + } + + if (!pollingEndpointSpecified) { + validatedOptions.logger?.warn(OptionMessages.partialEndpoint('baseUri')); + } + + if (!eventEndpointSpecified && validatedOptions.sendEvents) { + validatedOptions.logger?.warn(OptionMessages.partialEndpoint('eventsUri')); + } +} + /** * Configuration options for the LDClient. * @@ -174,6 +199,8 @@ export default class Configuration { this.logger?.warn(error); }); + validateEndpoints(options, validatedOptions); + this.serviceEndpoints = new ServiceEndpoints( validatedOptions.streamUri, validatedOptions.baseUri, diff --git a/server-sdk-common/src/options/OptionMessages.ts b/server-sdk-common/src/options/OptionMessages.ts index c55a071a18..129afce7ba 100644 --- a/server-sdk-common/src/options/OptionMessages.ts +++ b/server-sdk-common/src/options/OptionMessages.ts @@ -23,4 +23,8 @@ export default class OptionMessages { static invalidTagValue(name: string): string { return `Config option "${name}" must only contain letters, numbers, ., _ or -.`; } + + static partialEndpoint(name: string): string { + return `You have set custom uris without specifying the ${name} URI; connections may not work properly`; + } } From 9080329431665cc18062d4dea7db14746538f38b Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 25 May 2022 10:35:37 -0700 Subject: [PATCH 25/39] Remove unintended file. --- sdk-common/src/Context.ts | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 sdk-common/src/Context.ts diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts deleted file mode 100644 index 653630b8b3..0000000000 --- a/sdk-common/src/Context.ts +++ /dev/null @@ -1,19 +0,0 @@ -type MultiKind = { - kind: 'multi', - isMulti: true, -}; - -type SingleKind = { - kind: string - isMulti: false -}; - -type Kind = MultiKind | SingleKind; - -export default class Context { - public readonly kind: Kind; - - private contexts?: Context[]; - - private attributes?: Record; -} From af34183d8a2bdf8a19eaf3a358be894ae99026b6 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 25 May 2022 10:54:07 -0700 Subject: [PATCH 26/39] Empty context. --- sdk-common/src/Context.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts index e69de29bb2..653630b8b3 100644 --- a/sdk-common/src/Context.ts +++ b/sdk-common/src/Context.ts @@ -0,0 +1,19 @@ +type MultiKind = { + kind: 'multi', + isMulti: true, +}; + +type SingleKind = { + kind: string + isMulti: false +}; + +type Kind = MultiKind | SingleKind; + +export default class Context { + public readonly kind: Kind; + + private contexts?: Context[]; + + private attributes?: Record; +} From 5e0d9b1265adf47d1825e804b97c9461d11ce6cc Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 25 May 2022 11:04:18 -0700 Subject: [PATCH 27/39] Update server-sdk-common/src/options/Configuration.ts Co-authored-by: Alex Engelberg --- server-sdk-common/src/options/Configuration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server-sdk-common/src/options/Configuration.ts b/server-sdk-common/src/options/Configuration.ts index 8a6fb4d15e..8fe8fe44f4 100644 --- a/server-sdk-common/src/options/Configuration.ts +++ b/server-sdk-common/src/options/Configuration.ts @@ -122,7 +122,7 @@ function validateEndpoints(options: LDOptions, validatedOptions: ValidatedOption return; } - if (!streamingEndpointSpecified) { + if (!streamingEndpointSpecified && validatedOptions.stream) { validatedOptions.logger?.warn(OptionMessages.partialEndpoint('streamUri')); } From 3b207fd3b7bcc643a848a3bebfad79eb3f2a44d3 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 25 May 2022 11:11:23 -0700 Subject: [PATCH 28/39] Progress --- sdk-common/src/Context.ts | 8 ++++++++ sdk-common/src/api/LDContext.ts | 8 ++++++++ .../src/api/context/LDContextCommon.ts | 0 .../src/api/context/LDContextMeta.ts | 0 .../src/api/context/LDMultiKindContext.ts | 0 .../src/api/context/LDSingleKindContext.ts | 0 .../src/api => sdk-common/src/api/context}/LDUser.ts | 0 .../src/api/context/index.ts | 1 + sdk-common/src/api/index.ts | 1 + sdk-common/src/index.ts | 1 + server-sdk-common/src/api/LDContext.ts | 8 -------- server-sdk-common/src/api/index.ts | 2 -- 12 files changed, 19 insertions(+), 10 deletions(-) create mode 100644 sdk-common/src/api/LDContext.ts rename {server-sdk-common => sdk-common}/src/api/context/LDContextCommon.ts (100%) rename {server-sdk-common => sdk-common}/src/api/context/LDContextMeta.ts (100%) rename {server-sdk-common => sdk-common}/src/api/context/LDMultiKindContext.ts (100%) rename {server-sdk-common => sdk-common}/src/api/context/LDSingleKindContext.ts (100%) rename {server-sdk-common/src/api => sdk-common/src/api/context}/LDUser.ts (100%) rename {server-sdk-common => sdk-common}/src/api/context/index.ts (84%) create mode 100644 sdk-common/src/api/index.ts delete mode 100644 server-sdk-common/src/api/LDContext.ts diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts index 653630b8b3..b9c2dd6759 100644 --- a/sdk-common/src/Context.ts +++ b/sdk-common/src/Context.ts @@ -1,3 +1,4 @@ + type MultiKind = { kind: 'multi', isMulti: true, @@ -16,4 +17,11 @@ export default class Context { private contexts?: Context[]; private attributes?: Record; + + constructor(context: LDContext) { + this.kind = { + kind: context.kind, + isMulti: context.kind === 'multi', + }; + } } diff --git a/sdk-common/src/api/LDContext.ts b/sdk-common/src/api/LDContext.ts new file mode 100644 index 0000000000..79a7be1e77 --- /dev/null +++ b/sdk-common/src/api/LDContext.ts @@ -0,0 +1,8 @@ +import { LDMultiKindContext } from '../../../sdk-common/src/api/context/LDMultiKindContext'; +import { LDSingleKindContext } from '../../../sdk-common/src/api/context/LDSingleKindContext'; +import { LDUser } from './LDUser'; + +/** + * A LaunchDarkly context object. + */ +export type LDContext = LDUser | LDSingleKindContext | LDMultiKindContext; diff --git a/server-sdk-common/src/api/context/LDContextCommon.ts b/sdk-common/src/api/context/LDContextCommon.ts similarity index 100% rename from server-sdk-common/src/api/context/LDContextCommon.ts rename to sdk-common/src/api/context/LDContextCommon.ts diff --git a/server-sdk-common/src/api/context/LDContextMeta.ts b/sdk-common/src/api/context/LDContextMeta.ts similarity index 100% rename from server-sdk-common/src/api/context/LDContextMeta.ts rename to sdk-common/src/api/context/LDContextMeta.ts diff --git a/server-sdk-common/src/api/context/LDMultiKindContext.ts b/sdk-common/src/api/context/LDMultiKindContext.ts similarity index 100% rename from server-sdk-common/src/api/context/LDMultiKindContext.ts rename to sdk-common/src/api/context/LDMultiKindContext.ts diff --git a/server-sdk-common/src/api/context/LDSingleKindContext.ts b/sdk-common/src/api/context/LDSingleKindContext.ts similarity index 100% rename from server-sdk-common/src/api/context/LDSingleKindContext.ts rename to sdk-common/src/api/context/LDSingleKindContext.ts diff --git a/server-sdk-common/src/api/LDUser.ts b/sdk-common/src/api/context/LDUser.ts similarity index 100% rename from server-sdk-common/src/api/LDUser.ts rename to sdk-common/src/api/context/LDUser.ts diff --git a/server-sdk-common/src/api/context/index.ts b/sdk-common/src/api/context/index.ts similarity index 84% rename from server-sdk-common/src/api/context/index.ts rename to sdk-common/src/api/context/index.ts index 7babf9615c..c7ff48d3ee 100644 --- a/server-sdk-common/src/api/context/index.ts +++ b/sdk-common/src/api/context/index.ts @@ -2,3 +2,4 @@ export * from './LDContextCommon'; export * from './LDContextMeta'; export * from './LDMultiKindContext'; export * from './LDSingleKindContext'; +export * from './LDUser'; diff --git a/sdk-common/src/api/index.ts b/sdk-common/src/api/index.ts new file mode 100644 index 0000000000..c38e8e8215 --- /dev/null +++ b/sdk-common/src/api/index.ts @@ -0,0 +1 @@ +export * from './context'; diff --git a/sdk-common/src/index.ts b/sdk-common/src/index.ts index e69de29bb2..308f5ae158 100644 --- a/sdk-common/src/index.ts +++ b/sdk-common/src/index.ts @@ -0,0 +1 @@ +export * from './api'; \ No newline at end of file diff --git a/server-sdk-common/src/api/LDContext.ts b/server-sdk-common/src/api/LDContext.ts deleted file mode 100644 index 5c9f6cb9b2..0000000000 --- a/server-sdk-common/src/api/LDContext.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { LDMultiKindContext } from './context/LDMultiKindContext'; -import { LDSingleKindContext } from './context/LDSingleKindContext'; -import { LDUser } from './LDUser'; - -/** - * A LaunchDarkly context object. - */ -export type LDContext = LDUser | LDSingleKindContext | LDMultiKindContext; diff --git a/server-sdk-common/src/api/index.ts b/server-sdk-common/src/api/index.ts index bcdb25da43..d9f2b12c44 100644 --- a/server-sdk-common/src/api/index.ts +++ b/server-sdk-common/src/api/index.ts @@ -1,11 +1,9 @@ -export * from './context'; export * from './data'; export * from './options'; export * from './LDClient'; export * from './LDLogger'; export * from './LDLogLevel'; export * from './subsystems/LDStreamProcessor'; -export * from './LDContext'; export * from './LDUser'; // These are items that should be less frequently used, and therefore they From 79dd5d637b1d58040b91bf4466f37f16ef15eadc Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 25 May 2022 11:20:38 -0700 Subject: [PATCH 29/39] Refactor expectMessages. --- server-sdk-common/__tests__/Logger.ts | 14 +++++++------- .../__tests__/options/ApplicationTags.test.ts | 2 +- .../__tests__/options/Configuration.test.ts | 12 ++++++------ 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/server-sdk-common/__tests__/Logger.ts b/server-sdk-common/__tests__/Logger.ts index 631af53efa..0082fa2cee 100644 --- a/server-sdk-common/__tests__/Logger.ts +++ b/server-sdk-common/__tests__/Logger.ts @@ -8,6 +8,8 @@ export enum LogLevel { Error, } +type ExpectedMessage = { level: LogLevel, matches: RegExp }; + export default class TestLogger implements LDLogger { private readonly messages: Record = { [LogLevel.Debug]: [], @@ -46,10 +48,9 @@ export default class TestLogger implements LDLogger { * more than once, then it should be included multiple times. * @returns A list of messages that were not received. */ - verifyMessages( - expectedMessages: { level: LogLevel, matches: RegExp }[], - ): string[] { - const missing: string[] = []; + expectMessages( + expectedMessages: ExpectedMessage[], + ): void { const matched: Record = { [LogLevel.Debug]: [], [LogLevel.Info]: [], @@ -63,14 +64,13 @@ export default class TestLogger implements LDLogger { (receivedMessage) => receivedMessage.match(expectedMessage.matches), ); if (index < 0) { - missing.push(expectedMessage.matches.toString()); + throw new Error(`Did not find expected message: ${expectedMessage}`); } else if (matched[expectedMessage.level].indexOf(index) >= 0) { - missing.push(`did not get expected message ${expectedMessage.matches}`); + throw new Error(`Did not find expected message: ${expectedMessage}`); } else { matched[expectedMessage.level].push(index); } }); - return missing; } getCount() { diff --git a/server-sdk-common/__tests__/options/ApplicationTags.test.ts b/server-sdk-common/__tests__/options/ApplicationTags.test.ts index ee0b3929f2..6aae7e69e8 100644 --- a/server-sdk-common/__tests__/options/ApplicationTags.test.ts +++ b/server-sdk-common/__tests__/options/ApplicationTags.test.ts @@ -47,7 +47,7 @@ describe.each([ it('logs issues it encounters', () => { expect(config?.logger?.getCount()).toEqual(logs?.length); if (logs) { - config?.logger?.verifyMessages(logs).forEach((message) => expect(message).toBeUndefined()); + config?.logger?.expectMessages(logs); } }); }); diff --git a/server-sdk-common/__tests__/options/Configuration.test.ts b/server-sdk-common/__tests__/options/Configuration.test.ts index ff4d44dcfc..8e0ba8cdb8 100644 --- a/server-sdk-common/__tests__/options/Configuration.test.ts +++ b/server-sdk-common/__tests__/options/Configuration.test.ts @@ -67,7 +67,7 @@ describe('when setting different options', () => { expect(logger(config).getCount()).toEqual(logs.length); // There should not be any messages, so checking them for undefined is a workaround // for a lack of pure assert. - logger(config).verifyMessages(logs).forEach((message) => expect(message).toBeUndefined()); + logger(config).expectMessages(logs); }); it.each([ @@ -127,7 +127,7 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ timeout: value })); expect(config.timeout).toEqual(expected); - logger(config).verifyMessages(logs).forEach((message) => expect(message).toBeUndefined()); + logger(config).expectMessages(logs); }); it.each([ @@ -140,7 +140,7 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ capacity: value })); expect(config.eventsCapacity).toEqual(expected); - logger(config).verifyMessages(logs).forEach((message) => expect(message).toBeUndefined()); + logger(config).expectMessages(logs); }); it.each([ @@ -295,21 +295,21 @@ describe('when setting different options', () => { // @ts-ignore const config = new Configuration(withLogger({ diagnosticRecordingInterval: value })); expect(config.diagnosticRecordingInterval).toEqual(expected); - logger(config).verifyMessages(logs).forEach((message) => expect(message).toBeUndefined()); + logger(config).expectMessages(logs); }); it('discards unrecognized options with a warning', () => { // @ts-ignore const config = new Configuration(withLogger({ yes: 'no', cat: 'yes' })); expect(logger(config).getCount()).toEqual(2); - logger(config).verifyMessages([ + logger(config).expectMessages([ { level: LogLevel.Warn, matches: /Ignoring unknown config option "yes"/, }, { level: LogLevel.Warn, matches: /Ignoring unknown config option "cat"/, }, - ]).forEach((message) => expect(message).toBeUndefined()); + ]); }); // This is more thoroughly tested in the application tags test. From 2b38d4035fe4f55dbb7818af1801f956de8984fc Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 25 May 2022 11:23:44 -0700 Subject: [PATCH 30/39] Show what was received. --- server-sdk-common/__tests__/Logger.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server-sdk-common/__tests__/Logger.ts b/server-sdk-common/__tests__/Logger.ts index 0082fa2cee..980cf362a8 100644 --- a/server-sdk-common/__tests__/Logger.ts +++ b/server-sdk-common/__tests__/Logger.ts @@ -64,9 +64,9 @@ export default class TestLogger implements LDLogger { (receivedMessage) => receivedMessage.match(expectedMessage.matches), ); if (index < 0) { - throw new Error(`Did not find expected message: ${expectedMessage}`); + throw new Error(`Did not find expected message: ${expectedMessage} received: ${this.messages}`); } else if (matched[expectedMessage.level].indexOf(index) >= 0) { - throw new Error(`Did not find expected message: ${expectedMessage}`); + throw new Error(`Did not find expected message: ${expectedMessage} received: ${this.messages}`); } else { matched[expectedMessage.level].push(index); } From 59ed4c84fa8f122df1dd5935be25e88e9f4d9d56 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 25 May 2022 11:39:54 -0700 Subject: [PATCH 31/39] Finish moving things. --- sdk-common/src/AttributeReference.ts | 13 +++++++------ sdk-common/src/Context.ts | 19 ++++++++++--------- sdk-common/src/api/LDContext.ts | 6 +++--- sdk-common/src/api/index.ts | 1 + sdk-common/src/index.ts | 2 +- server-sdk-common/package.json | 3 +++ server-sdk-common/src/LDClientImpl.ts | 3 ++- server-sdk-common/src/api/LDClient.ts | 2 +- server-sdk-common/src/api/index.ts | 1 - 9 files changed, 28 insertions(+), 22 deletions(-) diff --git a/sdk-common/src/AttributeReference.ts b/sdk-common/src/AttributeReference.ts index 58af40e70f..701593b8a6 100644 --- a/sdk-common/src/AttributeReference.ts +++ b/sdk-common/src/AttributeReference.ts @@ -6,7 +6,7 @@ function getComponents(reference: string): string[] { const referenceWithoutPrefix = reference.startsWith('/') ? reference.substring(1) : reference; return referenceWithoutPrefix .split('/') - .map(component => (component.indexOf('~') >= 0 ? processEscapeCharacters(component) : component)); + .map((component) => (component.indexOf('~') >= 0 ? processEscapeCharacters(component) : component)); } function isLiteral(reference: string) { @@ -19,18 +19,19 @@ function isValid(reference: string) { export default class AttributeReference { public readonly isValid; + public readonly original; private readonly components?: string[]; constructor(reference: string) { this.original = reference; - if(reference === "" || reference === "/" || !isValid(reference)) { + if (reference === '' || reference === '/' || !isValid(reference)) { this.isValid = false; - } else if(isLiteral(reference)) { + } else if (isLiteral(reference)) { this.components = [reference]; - } else if(reference.indexOf('/', 1) < 0) { - this.components = [reference.slice(1)] + } else if (reference.indexOf('/', 1) < 0) { + this.components = [reference.slice(1)]; } else { this.components = getComponents(reference); } @@ -43,4 +44,4 @@ export default class AttributeReference { public getComponent(index: number): string | undefined { return this.components?.[index]; } -} \ No newline at end of file +} diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts index b9c2dd6759..ea79f12c30 100644 --- a/sdk-common/src/Context.ts +++ b/sdk-common/src/Context.ts @@ -1,3 +1,4 @@ +import { LDContext } from './api/LDContext'; type MultiKind = { kind: 'multi', @@ -12,16 +13,16 @@ type SingleKind = { type Kind = MultiKind | SingleKind; export default class Context { - public readonly kind: Kind; + // public readonly kind: Kind; - private contexts?: Context[]; + // private contexts?: Context[]; - private attributes?: Record; + // private attributes?: Record; - constructor(context: LDContext) { - this.kind = { - kind: context.kind, - isMulti: context.kind === 'multi', - }; - } + // constructor(context: LDContext) { + // this.kind = { + // kind: context.kind, + // isMulti: context.kind === 'multi', + // }; + // } } diff --git a/sdk-common/src/api/LDContext.ts b/sdk-common/src/api/LDContext.ts index 79a7be1e77..6d608228ba 100644 --- a/sdk-common/src/api/LDContext.ts +++ b/sdk-common/src/api/LDContext.ts @@ -1,6 +1,6 @@ -import { LDMultiKindContext } from '../../../sdk-common/src/api/context/LDMultiKindContext'; -import { LDSingleKindContext } from '../../../sdk-common/src/api/context/LDSingleKindContext'; -import { LDUser } from './LDUser'; +import { LDMultiKindContext } from './context/LDMultiKindContext'; +import { LDSingleKindContext } from './context/LDSingleKindContext'; +import { LDUser } from './context/LDUser'; /** * A LaunchDarkly context object. diff --git a/sdk-common/src/api/index.ts b/sdk-common/src/api/index.ts index c38e8e8215..371c16e0e1 100644 --- a/sdk-common/src/api/index.ts +++ b/sdk-common/src/api/index.ts @@ -1 +1,2 @@ export * from './context'; +export * from './LDContext'; diff --git a/sdk-common/src/index.ts b/sdk-common/src/index.ts index 308f5ae158..b1c13e7340 100644 --- a/sdk-common/src/index.ts +++ b/sdk-common/src/index.ts @@ -1 +1 @@ -export * from './api'; \ No newline at end of file +export * from './api'; diff --git a/server-sdk-common/package.json b/server-sdk-common/package.json index 97edcb76bd..1d57466323 100644 --- a/server-sdk-common/package.json +++ b/server-sdk-common/package.json @@ -17,6 +17,9 @@ "build": "tsc" }, "license": "Apache-2.0", + "dependencies": { + "@launchdarkly/js-sdk-common": "@launchdarkly/js-sdk-common" + }, "devDependencies": { "@types/jest": "^27.4.1", "jest": "^27.5.1", diff --git a/server-sdk-common/src/LDClientImpl.ts b/server-sdk-common/src/LDClientImpl.ts index 99a6f0dcb1..d5cda587f3 100644 --- a/server-sdk-common/src/LDClientImpl.ts +++ b/server-sdk-common/src/LDClientImpl.ts @@ -1,7 +1,8 @@ /* eslint-disable @typescript-eslint/no-unused-vars */ /* eslint-disable class-methods-use-this */ +import { LDContext } from '@launchdarkly/js-sdk-common'; import { - LDClient, LDContext, LDEvaluationDetail, LDFlagsState, LDFlagsStateOptions, + LDClient, LDEvaluationDetail, LDFlagsState, LDFlagsStateOptions, } from './api'; import BigSegmentStoreStatusProvider from './BigSegmentStatusProviderImpl'; import { Platform } from './platform'; diff --git a/server-sdk-common/src/api/LDClient.ts b/server-sdk-common/src/api/LDClient.ts index ada8704a04..222c3a05c7 100644 --- a/server-sdk-common/src/api/LDClient.ts +++ b/server-sdk-common/src/api/LDClient.ts @@ -1,4 +1,4 @@ -import { LDContext } from './LDContext'; +import { LDContext } from '@launchdarkly/js-sdk-common'; import { LDEvaluationDetail } from './data/LDEvaluationDetail'; import { LDFlagsState } from './data/LDFlagsState'; import { LDFlagsStateOptions } from './data/LDFlagsStateOptions'; diff --git a/server-sdk-common/src/api/index.ts b/server-sdk-common/src/api/index.ts index d9f2b12c44..59edda462f 100644 --- a/server-sdk-common/src/api/index.ts +++ b/server-sdk-common/src/api/index.ts @@ -4,7 +4,6 @@ export * from './LDClient'; export * from './LDLogger'; export * from './LDLogLevel'; export * from './subsystems/LDStreamProcessor'; -export * from './LDUser'; // These are items that should be less frequently used, and therefore they // are namespaced to reduce clutter amongst the top level exports. From a9773537e1e0103a2430250410f8b7bf71c0cfa3 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 25 May 2022 13:49:41 -0700 Subject: [PATCH 32/39] Move validation. Implement more of Context. --- .../__tests__}/validators.test.ts | 2 +- sdk-common/src/Context.ts | 181 ++++++++++++++++-- sdk-common/src/index.ts | 1 + .../options => sdk-common/src}/validators.ts | 17 +- .../src/options/ApplicationTags.ts | 2 +- .../src/options/Configuration.ts | 2 +- 6 files changed, 172 insertions(+), 33 deletions(-) rename {server-sdk-common/__tests__/options => sdk-common/__tests__}/validators.test.ts (97%) rename {server-sdk-common/src/options => sdk-common/src}/validators.ts (96%) diff --git a/server-sdk-common/__tests__/options/validators.test.ts b/sdk-common/__tests__/validators.test.ts similarity index 97% rename from server-sdk-common/__tests__/options/validators.test.ts rename to sdk-common/__tests__/validators.test.ts index 3eb73b57ec..c18b72223b 100644 --- a/server-sdk-common/__tests__/options/validators.test.ts +++ b/sdk-common/__tests__/validators.test.ts @@ -1,4 +1,4 @@ -import TypeValidators, { TypeArray } from '../../src/options/validators'; +import { TypeArray, TypeValidators } from '../src'; const stringValue = 'this is a string'; const numberValue = 3.14159; diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts index ea79f12c30..cc5a01a07a 100644 --- a/sdk-common/src/Context.ts +++ b/sdk-common/src/Context.ts @@ -1,28 +1,175 @@ +// eslint-disable-next-line max-classes-per-file +import { + LDSingleKindContext, LDMultiKindContext, LDUser, LDContextCommon, +} from './api/context'; import { LDContext } from './api/LDContext'; +import { TypeValidators } from './validators'; -type MultiKind = { - kind: 'multi', - isMulti: true, -}; +// Validates a kind excluding check that it isn't "kind". +const KindValidator = TypeValidators.StringMatchingRegex(/^(\w|\.|-)+$/); -type SingleKind = { - kind: string - isMulti: false -}; +// The API allows for calling with an `LDContext` which is +// `LDUser | LDSingleKindContext | LDMultiKindContext`. When ingesting a context +// first the type must be determined to allow us to put it into a consistent type. -type Kind = MultiKind | SingleKind; +/** + * Check if a context is a single kind context. + * @param context + * @returns frue if the context is a single kind context. + */ +function isSingleKind(context: LDContext): context is LDSingleKindContext { + if ('kind' in context) { + return TypeValidators.String.is(context.kind) && context.kind !== 'multi'; + } + return false; +} + +/** + * Check if a context is a multi-kind context. + * @param context + * @returns true if it is a multi-kind context. + */ +function isMultiKind(context: LDContext): context is LDMultiKindContext { + if ('kind' in context) { + return TypeValidators.String.is(context.kind) && context.kind === 'multi'; + } + return false; +} + +/** + * Check if a context is a legacy user context. + * @param context + * @returns true if it is a legacy user context. + */ +function isLegacyUser(context: LDContext): context is LDUser { + return !('kind' in context) || context.kind === null || context.kind === undefined; +} + +/** + * Check if the given value is a LDContextCommon. + * @param kindOrContext + * @returns true if it is an LDContextCommon + * + * Due to a limitation in the expressiveness of these highly polymorphic types any field + * in a multi-kind context can either be a context or 'kind'. So we need to re-assure + * the compiler that is isn't the word multi. + * + * Because we do not allow top level values in a multi-kind context we can validate + * that as well. + */ +function isContextCommon(kindOrContext: 'multi' | LDContextCommon): kindOrContext is LDContextCommon { + return TypeValidators.Object.is(kindOrContext); +} + +/** + * Validate a context kind. + * @param kind + * @returns true if the kind is valid. + */ +function validKind(kind: string) { + return KindValidator.is(kind) && kind !== 'kind'; +} + +/** + * Validate a context key. + * @param key + * @returns true if the key is valid. + */ +function validKey(key: string) { + return TypeValidators.String.is(key) && key !== ''; +} export default class Context { - // public readonly kind: Kind; + private context?: LDSingleKindContext | LDUser; + + private isMulti: boolean = false; + + private isUser: boolean = false; + + private contexts: Record = {}; + + public readonly kind: string; + + private constructor(kind: string) { + this.kind = kind; + } + + private static FromMultiKindContext(context: LDMultiKindContext): Context | undefined { + const kinds = Object.keys(context).filter((key) => key !== 'kind'); + const kindsValid = kinds.every(validKind); + + if (!kindsValid) { + return undefined; + } + + const contexts = kinds.map((kind) => context[kind]); + const contextsAreObjects = contexts.every(isContextCommon); + + if (!contextsAreObjects) { + return undefined; + } + + if (!contexts.every((part) => validKey(part.key))) { + return undefined; + } + + const created = new Context(context.kind); + created.isMulti = true; + return created; + } + + private static FromSingleKindContext(context: LDSingleKindContext): Context | undefined { + const { key, kind } = context; + const kindValid = validKind(kind); + const keyValid = validKey(key); + + if (keyValid && kindValid) { + const created = new Context(kind); + created.isUser = kind === 'user'; + created.context = context; + return created; + } + return undefined; + } + + private static FromLegacyUser(context: LDUser): Context | undefined { + // TODO: Check user key. + const created = new Context('user'); + created.isUser = true; + created.context = context; + return created; + } + + static FromLDContext(context: LDContext): Context | undefined { + if (isSingleKind(context)) { + return this.FromSingleKindContext(context); + } if (isMultiKind(context)) { + return this.FromMultiKindContext(context); + } if (isLegacyUser(context)) { + return this.FromLegacyUser(context); + } + return undefined; + } + + public get canonicalKey(): string { + if (this.isUser) { + return this.context!.key; + } + if (this.isMulti) { + return Object.keys(this.contexts).map((key) => `${key}:${encodeURIComponent(this.contexts[key].key)}`) + .join(':'); + } + return `${this.kind}:${encodeURIComponent(this.context!.key)}`; + } - // private contexts?: Context[]; + public get kinds(): string[] { + if (this.isMulti) { + return Object.keys(this.contexts); + } + return [this.kind]; + } - // private attributes?: Record; + // public getValueForKind(kind: string, reference: AttributeReference) { - // constructor(context: LDContext) { - // this.kind = { - // kind: context.kind, - // isMulti: context.kind === 'multi', - // }; // } } diff --git a/sdk-common/src/index.ts b/sdk-common/src/index.ts index b1c13e7340..d06405c841 100644 --- a/sdk-common/src/index.ts +++ b/sdk-common/src/index.ts @@ -1 +1,2 @@ export * from './api'; +export * from './validators'; diff --git a/server-sdk-common/src/options/validators.ts b/sdk-common/src/validators.ts similarity index 96% rename from server-sdk-common/src/options/validators.ts rename to sdk-common/src/validators.ts index 64c3ca5999..bdebc3f7fc 100644 --- a/server-sdk-common/src/options/validators.ts +++ b/sdk-common/src/validators.ts @@ -10,8 +10,6 @@ /** * Interface for type validation. - * - * @internal */ export interface TypeValidator { is(u:unknown): boolean; @@ -20,8 +18,6 @@ export interface TypeValidator { /** * Validate a factory or instance. - * - * @internal */ export class FactoryOrInstance implements TypeValidator { is(factoryOrInstance: unknown) { @@ -40,8 +36,6 @@ export class FactoryOrInstance implements TypeValidator { /** * Validate a basic type. - * - * @internal */ export class Type implements TypeValidator { private typeName: string; @@ -65,6 +59,9 @@ export class Type implements TypeValidator { } } +/** + * Validate an array of the specified type. + */ export class TypeArray implements TypeValidator { private typeName: string; @@ -92,8 +89,6 @@ export class TypeArray implements TypeValidator { /** * Validate a value is a number and is greater or eval than a minimum. - * - * @internal */ export class NumberWithMinimum extends Type { readonly min: number; @@ -110,8 +105,6 @@ export class NumberWithMinimum extends Type { /** * Validate a value is a string and it matches the given expression. - * - * @internal */ export class StringMatchingRegex extends Type { readonly expression: RegExp; @@ -128,10 +121,8 @@ export class StringMatchingRegex extends Type { /** * A set of standard type validators. - * - * @internal */ -export default class TypeValidators { +export class TypeValidators { static readonly String = new Type('string', ''); static readonly Number = new Type('number', 0); diff --git a/server-sdk-common/src/options/ApplicationTags.ts b/server-sdk-common/src/options/ApplicationTags.ts index 00f9305016..43b4aea5e6 100644 --- a/server-sdk-common/src/options/ApplicationTags.ts +++ b/server-sdk-common/src/options/ApplicationTags.ts @@ -1,6 +1,6 @@ +import { TypeValidators } from '@launchdarkly/js-sdk-common'; import OptionMessages from './OptionMessages'; import { ValidatedOptions } from './ValidatedOptions'; -import TypeValidators from './validators'; /** * Expression to validate characters that are allowed in tag keys and values. diff --git a/server-sdk-common/src/options/Configuration.ts b/server-sdk-common/src/options/Configuration.ts index 8fe8fe44f4..a58394f28c 100644 --- a/server-sdk-common/src/options/Configuration.ts +++ b/server-sdk-common/src/options/Configuration.ts @@ -1,3 +1,4 @@ +import { NumberWithMinimum, TypeValidator, TypeValidators } from '@launchdarkly/js-sdk-common'; import { LDLogger, LDOptions, LDProxyOptions, LDTLSOptions, } from '../api'; @@ -5,7 +6,6 @@ import ApplicationTags from './ApplicationTags'; import OptionMessages from './OptionMessages'; import ServiceEndpoints from './ServiceEndpoints'; import { ValidatedOptions } from './ValidatedOptions'; -import TypeValidators, { NumberWithMinimum, TypeValidator } from './validators'; // Once things are internal to the implementation of the SDK we can depend on // types. Calls to the SDK could contain anything without any regard to typing. From a935f1abb3094f5ae09be467bfdf3e432a4454ce Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 25 May 2022 14:26:09 -0700 Subject: [PATCH 33/39] Basic context and attribute reference structure. --- sdk-common/src/AttributeReference.ts | 42 +++++++++++++++++----- sdk-common/src/Context.ts | 53 ++++++++++++++++++++++++---- 2 files changed, 80 insertions(+), 15 deletions(-) diff --git a/sdk-common/src/AttributeReference.ts b/sdk-common/src/AttributeReference.ts index 701593b8a6..33164dcf8b 100644 --- a/sdk-common/src/AttributeReference.ts +++ b/sdk-common/src/AttributeReference.ts @@ -1,3 +1,5 @@ +import { LDContextCommon } from './api'; + function processEscapeCharacters(value: string): string { return value.replace(/~/g, '~0').replace(/\//g, '~1'); } @@ -9,11 +11,11 @@ function getComponents(reference: string): string[] { .map((component) => (component.indexOf('~') >= 0 ? processEscapeCharacters(component) : component)); } -function isLiteral(reference: string) { +function isLiteral(reference: string): boolean { return !reference.startsWith('/'); } -function isValid(reference: string) { +function validate(reference: string): boolean { return !reference.match(/\/\/|(^\/.*~[^0|^1])|~$/); } @@ -22,12 +24,13 @@ export default class AttributeReference { public readonly original; - private readonly components?: string[]; + private readonly components: string[]; constructor(reference: string) { this.original = reference; - if (reference === '' || reference === '/' || !isValid(reference)) { + if (reference === '' || reference === '/' || !validate(reference)) { this.isValid = false; + this.components = []; } else if (isLiteral(reference)) { this.components = [reference]; } else if (reference.indexOf('/', 1) < 0) { @@ -37,11 +40,32 @@ export default class AttributeReference { } } - public depth(): number | undefined { - return this.components?.length; - } + public get(target: LDContextCommon) { + const { components, isValid } = this; + if (!isValid) { + return undefined; + } - public getComponent(index: number): string | undefined { - return this.components?.[index]; + let current = target; + + // This doesn't use a range based for loops, because those use generators. + // See `no-restricted-syntax`. + // It also doesn't use a collection method because this logic is more + // straightforward with a loop. + for (let index = 0; index < components.length; index += 1) { + const component = components[index]; + if ( + current !== null + && current !== undefined + // See https://eslint.org/docs/rules/no-prototype-builtins + && Object.prototype.hasOwnProperty.call(current, component) + && typeof current === 'object' + ) { + current = current[component]; + } else { + return undefined; + } + } + return current; } } diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts index cc5a01a07a..74dfbc0b50 100644 --- a/sdk-common/src/Context.ts +++ b/sdk-common/src/Context.ts @@ -3,6 +3,7 @@ import { LDSingleKindContext, LDMultiKindContext, LDUser, LDContextCommon, } from './api/context'; import { LDContext } from './api/LDContext'; +import AttributeReference from './AttributeReference'; import { TypeValidators } from './validators'; // Validates a kind excluding check that it isn't "kind". @@ -79,6 +80,11 @@ function validKey(key: string) { return TypeValidators.String.is(key) && key !== ''; } +/** + * Container for a context/contexts. Because contexts come from external code + * they must be thoroughly validated and then formed to comply with + * the type system. + */ export default class Context { private context?: LDSingleKindContext | LDUser; @@ -90,6 +96,10 @@ export default class Context { public readonly kind: string; + /** + * Contexts should be created using the static factory method {@link Context.FromLDContext}. + * @param kind The kind of the context. + */ private constructor(kind: string) { this.kind = kind; } @@ -133,24 +143,37 @@ export default class Context { } private static FromLegacyUser(context: LDUser): Context | undefined { - // TODO: Check user key. + const keyValid = context.key !== undefined && context.key !== null; + // For legacy users we allow empty keys. + if (!keyValid) { + return undefined; + } const created = new Context('user'); created.isUser = true; created.context = context; return created; } + /** + * Attempt to create a {@link Context} from an {@link LDContext}. + * @param context The input context to create a Context from. + * @returns a {@link Context} or `undefined` if one could not be created. + */ static FromLDContext(context: LDContext): Context | undefined { if (isSingleKind(context)) { - return this.FromSingleKindContext(context); + return Context.FromSingleKindContext(context); } if (isMultiKind(context)) { - return this.FromMultiKindContext(context); + return Context.FromMultiKindContext(context); } if (isLegacyUser(context)) { - return this.FromLegacyUser(context); + return Context.FromLegacyUser(context); } return undefined; } + public get isMultiKind(): boolean { + return this.isMulti; + } + public get canonicalKey(): string { if (this.isUser) { return this.context!.key; @@ -169,7 +192,25 @@ export default class Context { return [this.kind]; } - // public getValueForKind(kind: string, reference: AttributeReference) { + private static getValueFromContext( + reference: AttributeReference, + context?: LDContextCommon, + ): any { + if (!context || !reference.isValid) { + return undefined; + } - // } + return reference.get(context); + } + + /** + * Attempt to get a value for the given context kind using the given reference. + * @param kind The kind of the context to get the value for. + * @param reference The reference to the value to get. + * @returns a value or `undefined` if one is not found. + */ + public getValueForKind(kind: string, reference: AttributeReference): any | undefined { + const context = this.isMulti ? this.contexts[kind] : this.context; + return Context.getValueFromContext(reference, context); + } } From ba71331cba8137cbd7fd2e6947da5582e29f3d50 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 25 May 2022 16:22:19 -0700 Subject: [PATCH 34/39] Add attribute reference tests. --- .../__tests__/AttributeReference.test.ts | 74 +++++++++++++++++ sdk-common/src/AttributeReference.ts | 80 +++++++++++++++---- sdk-common/src/Context.ts | 67 ++++++++++++++-- sdk-common/src/api/context/LDContextCommon.ts | 5 ++ sdk-common/src/api/context/LDContextMeta.ts | 5 -- 5 files changed, 207 insertions(+), 24 deletions(-) create mode 100644 sdk-common/__tests__/AttributeReference.test.ts diff --git a/sdk-common/__tests__/AttributeReference.test.ts b/sdk-common/__tests__/AttributeReference.test.ts new file mode 100644 index 0000000000..c1936dd068 --- /dev/null +++ b/sdk-common/__tests__/AttributeReference.test.ts @@ -0,0 +1,74 @@ +import { LDContextCommon } from '../src'; +import AttributeReference from '../src/AttributeReference'; + +function AsContextCommon(values: Record): LDContextCommon { + return { + ...values, + key: 'potato', + }; +} + +describe.each([ + new AttributeReference('/'), + new AttributeReference(''), + new AttributeReference('//'), + new AttributeReference(''), + new AttributeReference('', true), + new AttributeReference('_meta'), + new AttributeReference('/_meta'), + new AttributeReference('/_meta/secondary'), +])('when given invalid attribute references', (reference) => { + it('should not be valid', () => { + expect(reference.isValid).toBeFalsy(); + }); + + it('should not be able to get a value', () => { + expect(reference.get(AsContextCommon({ + '/': true, + '//': true, + '/~3': true, + '': true, + _meta: { secondary: true }, + }))).toBeUndefined(); + }); +}); + +describe.each([ + [new AttributeReference('/', true), { '/': true }, true], + [new AttributeReference('//', true), { '//': 17 }, 17], + [new AttributeReference('/~0', true), { '/~0': 'string' }, 'string'], + [new AttributeReference('a~b', true), { 'a~b': 'another' }, 'another'], + [new AttributeReference('a~b'), { 'a~b': 'another' }, 'another'], + [new AttributeReference('a/b', true), { 'a/b': true }, true], + [new AttributeReference('a/b'), { 'a/b': true }, true], + [new AttributeReference('/a~1~0b'), { 'a/~b': true }, true], + [new AttributeReference('/a~0b'), { 'a~b': true }, true], + [new AttributeReference(' /a/b', true), { ' /a/b': true }, true], + [new AttributeReference(' /a/b', false), { ' /a/b': true }, true], + [new AttributeReference('/a/b'), { a: { b: 'c' } }, 'c'], + [new AttributeReference('/a/1'), { a: ['b', 'c'] }, 'c'], +])('when given valid attribute references', (reference, object, expected) => { + it('should be valid', () => { + expect(reference.isValid).toBeTruthy(); + }); + + it('should be able to get a value', () => { + expect(reference.get(AsContextCommon(object))).toEqual(expected); + }); +}); + +describe.each([ + [new AttributeReference('name'), { }], + [new AttributeReference('/a/b'), { a: {} }], + [new AttributeReference('/a/0'), { a: 'test' }], + [new AttributeReference('/a/b'), { a: null }], + [new AttributeReference('/a/7'), { a: [0, 1] }], +])('should gracefully handle values that do not exist', (reference, object) => { + it('should be valid', () => { + expect(reference.isValid).toBeTruthy(); + }); + + it('should not be able to get a value', () => { + expect(reference.get(AsContextCommon(object))).toBeUndefined(); + }); +}); diff --git a/sdk-common/src/AttributeReference.ts b/sdk-common/src/AttributeReference.ts index 33164dcf8b..bf835adf04 100644 --- a/sdk-common/src/AttributeReference.ts +++ b/sdk-common/src/AttributeReference.ts @@ -1,14 +1,28 @@ import { LDContextCommon } from './api'; -function processEscapeCharacters(value: string): string { - return value.replace(/~/g, '~0').replace(/\//g, '~1'); +/** + * Converts a literal to a ref string. + * @param value + * @returns An escaped literal which can be used as a ref. + */ +function toRefString(value: string): string { + return `/${value.replace(/~/g, '~0').replace(/\//g, '~1')}`; +} + +/** + * Produce a literal from a ref component. + * @param ref + * @returns A literal version of the ref. + */ +function unescape(ref: string): string { + return ref.indexOf('~') ? ref.replace(/~1/g, '/').replace(/~0/g, '~') : ref; } function getComponents(reference: string): string[] { const referenceWithoutPrefix = reference.startsWith('/') ? reference.substring(1) : reference; return referenceWithoutPrefix .split('/') - .map((component) => (component.indexOf('~') >= 0 ? processEscapeCharacters(component) : component)); + .map((component) => (unescape(component))); } function isLiteral(reference: string): boolean { @@ -22,21 +36,59 @@ function validate(reference: string): boolean { export default class AttributeReference { public readonly isValid; - public readonly original; + /** + * When redacting attributes this name can be directly added to the list of + * redactions. + */ + public readonly redactionName; private readonly components: string[]; - constructor(reference: string) { - this.original = reference; - if (reference === '' || reference === '/' || !validate(reference)) { - this.isValid = false; - this.components = []; - } else if (isLiteral(reference)) { - this.components = [reference]; - } else if (reference.indexOf('/', 1) < 0) { - this.components = [reference.slice(1)]; + /** + * Take an attribute reference string, or literal string, and produce + * an attribute reference. + * + * Legacy user objects would have been created with names not + * references. So, in that case, we need to use them as a component + * without escaping them. + * + * e.g. A user could contain a custom attribute of `/a` which would + * become the literal `a` if treated as a reference. Which would cause + * it to no longer be redacted. + * @param refOrLiteral The attribute reference string or literal string. + * @param literal it true the value should be treated as a literal. + */ + public constructor(refOrLiteral: string, literal: boolean = false) { + if (!literal) { + this.redactionName = refOrLiteral; + if (refOrLiteral === '' || refOrLiteral === '/' || !validate(refOrLiteral)) { + this.isValid = false; + this.components = []; + return; + } + + if (isLiteral(refOrLiteral)) { + this.components = [refOrLiteral]; + } else if (refOrLiteral.indexOf('/', 1) < 0) { + this.components = [unescape(refOrLiteral.slice(1))]; + } else { + this.components = getComponents(refOrLiteral); + } + // The items inside of '_meta' are not intended to be addressable. + // Excluding it as a valid reference means that we can make it non-addressable + // without having to copy all the attributes out of the context object + // provided by the user. + if (this.components[0] === '_meta') { + this.isValid = false; + } else { + this.isValid = true; + } } else { - this.components = getComponents(reference); + const literalVal = refOrLiteral; + this.components = [literalVal]; + this.isValid = literalVal !== ''; + // Literals which start with '/' need escaped to prevent ambiguity. + this.redactionName = literalVal.startsWith('/') ? toRefString(literalVal) : literalVal; } } diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts index 74dfbc0b50..c0b49acd98 100644 --- a/sdk-common/src/Context.ts +++ b/sdk-common/src/Context.ts @@ -80,13 +80,45 @@ function validKey(key: string) { return TypeValidators.String.is(key) && key !== ''; } +function processPrivateAttributes( + privateAttributes?: string[], + literals: boolean = false, +): AttributeReference[] { + if (privateAttributes) { + return privateAttributes.map( + (privateAttribute) => new AttributeReference(privateAttribute, literals), + ); + } + return []; +} + +function legacyToSingleKind(user: LDUser): LDSingleKindContext { + const singleKindContext: LDSingleKindContext = { + ...(user.custom || []), + kind: 'user', + key: user.key, + }; + + // For legacy users we never established a difference between null + // and undefined for inputs. Because transient can be used in evaluations + // we would want it to not possibly match true/false unless defined. + // Which is different than treating implicit transient as `false`. + if (user.anonymous !== undefined && user.anonymous !== null) { + const transient = !!singleKindContext.anonymous; + delete singleKindContext.anonymous; + singleKindContext.transient = transient; + } + + return singleKindContext; +} + /** * Container for a context/contexts. Because contexts come from external code * they must be thoroughly validated and then formed to comply with * the type system. */ export default class Context { - private context?: LDSingleKindContext | LDUser; + private context?: LDSingleKindContext; private isMulti: boolean = false; @@ -94,11 +126,16 @@ export default class Context { private contexts: Record = {}; + private privateAttributes?: Record; + public readonly kind: string; /** * Contexts should be created using the static factory method {@link Context.FromLDContext}. * @param kind The kind of the context. + * + * The factory methods are static functions within the class because they access private + * implementation details, so they cannot be free functions. */ private constructor(kind: string) { this.kind = kind; @@ -112,18 +149,29 @@ export default class Context { return undefined; } - const contexts = kinds.map((kind) => context[kind]); - const contextsAreObjects = contexts.every(isContextCommon); + let contextsAreObjects = true; + const contexts = kinds.reduce((acc: Record, kind) => { + const singleContext = context[kind]; + if (isContextCommon(singleContext)) { + acc[kind] = singleContext; + } else { + // No early break isn't the most efficient, but it is an error condition. + contextsAreObjects = false; + } + return acc; + }, {}); if (!contextsAreObjects) { return undefined; } - if (!contexts.every((part) => validKey(part.key))) { + if (!Object.values(contexts).every((part) => validKey(part.key))) { return undefined; } const created = new Context(context.kind); + created.contexts = contexts; + created.isMulti = true; return created; } @@ -134,9 +182,15 @@ export default class Context { const keyValid = validKey(key); if (keyValid && kindValid) { + // The JSON interfaces uses dandling _. + // eslint-disable-next-line no-underscore-dangle + const privateAttributeReferences = processPrivateAttributes(context._meta?.privateAttributes); const created = new Context(kind); created.isUser = kind === 'user'; created.context = context; + created.privateAttributes = { + [kind]: privateAttributeReferences, + }; return created; } return undefined; @@ -150,7 +204,10 @@ export default class Context { } const created = new Context('user'); created.isUser = true; - created.context = context; + created.context = legacyToSingleKind(context); + created.privateAttributes = { + user: processPrivateAttributes(context.privateAttributeNames, true), + }; return created; } diff --git a/sdk-common/src/api/context/LDContextCommon.ts b/sdk-common/src/api/context/LDContextCommon.ts index 67941172c0..8c45b01cc5 100644 --- a/sdk-common/src/api/context/LDContextCommon.ts +++ b/sdk-common/src/api/context/LDContextCommon.ts @@ -22,6 +22,11 @@ export interface LDContextCommon { */ _meta?: LDContextMeta; + /** + * If true, the context will _not_ appear on the Contexts page in the LaunchDarkly dashboard. + */ + transient?: boolean; + /** * Any additional attributes associated with the context. */ diff --git a/sdk-common/src/api/context/LDContextMeta.ts b/sdk-common/src/api/context/LDContextMeta.ts index 9115450f02..b49efcc0cd 100644 --- a/sdk-common/src/api/context/LDContextMeta.ts +++ b/sdk-common/src/api/context/LDContextMeta.ts @@ -6,11 +6,6 @@ * cannot be addressed in targetting rules. */ export interface LDContextMeta { - /** - * If true, the context will _not_ appear on the Contexts page in the LaunchDarkly dashboard. - */ - transient?: boolean; - /** * An optional secondary key for a context. * From a7a04eae3899c7ca0a9d5ad046ff8adf84d6a4de Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 25 May 2022 16:24:56 -0700 Subject: [PATCH 35/39] Add some comments. --- sdk-common/src/Context.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts index c0b49acd98..0d6aa2edc7 100644 --- a/sdk-common/src/Context.ts +++ b/sdk-common/src/Context.ts @@ -6,6 +6,14 @@ import { LDContext } from './api/LDContext'; import AttributeReference from './AttributeReference'; import { TypeValidators } from './validators'; +// The general strategy for the context is to tranform the passed in context +// as little as possible. We do convert the legacy users to a single kind +// context, but we do not translate all passed contexts into a rigid structure. +// The context will have to be copied for events, but we want to avoid any +// copying that we can. +// So we validate that the information we are given is correct, and then we +// just proxy calls with a nicely typed interface. + // Validates a kind excluding check that it isn't "kind". const KindValidator = TypeValidators.StringMatchingRegex(/^(\w|\.|-)+$/); @@ -92,6 +100,11 @@ function processPrivateAttributes( return []; } +/** + * Convert a legacy user to a single kind context. + * @param user + * @returns A single kind context. + */ function legacyToSingleKind(user: LDUser): LDSingleKindContext { const singleKindContext: LDSingleKindContext = { ...(user.custom || []), From db8e51d369d902607a2c5346ed721ab09ed88e33 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 26 May 2022 09:14:10 -0700 Subject: [PATCH 36/39] Flesh out public interface for contexts. Add more documentation. --- sdk-common/src/Context.ts | 99 ++++++++++++++++++++++++++++++--------- 1 file changed, 78 insertions(+), 21 deletions(-) diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts index 0d6aa2edc7..15385ba57f 100644 --- a/sdk-common/src/Context.ts +++ b/sdk-common/src/Context.ts @@ -1,3 +1,4 @@ +/* eslint-disable no-underscore-dangle */ // eslint-disable-next-line max-classes-per-file import { LDSingleKindContext, LDMultiKindContext, LDUser, LDContextCommon, @@ -13,6 +14,8 @@ import { TypeValidators } from './validators'; // copying that we can. // So we validate that the information we are given is correct, and then we // just proxy calls with a nicely typed interface. +// This is to reduce work on the hot-path. Later, for event processing, deeper +// cloning of the context will be done. // Validates a kind excluding check that it isn't "kind". const KindValidator = TypeValidators.StringMatchingRegex(/^(\w|\.|-)+$/); @@ -100,6 +103,10 @@ function processPrivateAttributes( return []; } +function defined(value: any) { + return value !== null && value !== undefined; +} + /** * Convert a legacy user to a single kind context. * @param user @@ -115,13 +122,23 @@ function legacyToSingleKind(user: LDUser): LDSingleKindContext { // For legacy users we never established a difference between null // and undefined for inputs. Because transient can be used in evaluations // we would want it to not possibly match true/false unless defined. - // Which is different than treating implicit transient as `false`. - if (user.anonymous !== undefined && user.anonymous !== null) { + // Which is different than coercing a null/undefined transient as `false`. + if (defined(user.anonymous)) { const transient = !!singleKindContext.anonymous; delete singleKindContext.anonymous; singleKindContext.transient = transient; } + if (defined(user.secondary)) { + singleKindContext._meta = {}; + const { secondary } = singleKindContext; + delete singleKindContext.secondary; + singleKindContext._meta.secondary = secondary; + } + + // We are not pulling private attributes over because we will serialize + // those from attribute references for events. + return singleKindContext; } @@ -154,6 +171,21 @@ export default class Context { this.kind = kind; } + private static getValueFromContext( + reference: AttributeReference, + context?: LDContextCommon, + ): any { + if (!context || !reference.isValid) { + return undefined; + } + + return reference.get(context); + } + + private contextForKind(kind: string): LDContextCommon | undefined { + return this.isMulti ? this.contexts[kind] : this.context; + } + private static FromMultiKindContext(context: LDMultiKindContext): Context | undefined { const kinds = Object.keys(context).filter((key) => key !== 'kind'); const kindsValid = kinds.every(validKind); @@ -229,7 +261,7 @@ export default class Context { * @param context The input context to create a Context from. * @returns a {@link Context} or `undefined` if one could not be created. */ - static FromLDContext(context: LDContext): Context | undefined { + public static FromLDContext(context: LDContext): Context | undefined { if (isSingleKind(context)) { return Context.FromSingleKindContext(context); } if (isMultiKind(context)) { @@ -240,10 +272,40 @@ export default class Context { return undefined; } + /** + * Attempt to get a value for the given context kind using the given reference. + * @param kind The kind of the context to get the value for. + * @param reference The reference to the value to get. + * @returns a value or `undefined` if one is not found. + */ + public valueForKind(kind: string, reference: AttributeReference): any | undefined { + return Context.getValueFromContext(reference, this.contextForKind(kind)); + } + + /** + * Attempt to get a secondary key from a context. + * @param kind The kind of the context to get the secondary key for. + * @returns the secondary key, or undefined if not present or not a string. + */ + public secondary(kind: string): string | undefined { + const context = this.contextForKind(kind); + if (defined(context?._meta?.secondary) + && TypeValidators.String.is(context?._meta?.secondary)) { + return context?._meta?.secondary; + } + return undefined; + } + + /** + * True if this is a multi-kind context. + */ public get isMultiKind(): boolean { return this.isMulti; } + /** + * Get the canonical key for this context. + */ public get canonicalKey(): string { if (this.isUser) { return this.context!.key; @@ -255,6 +317,9 @@ export default class Context { return `${this.kind}:${encodeURIComponent(this.context!.key)}`; } + /** + * Get the kinds of this context. + */ public get kinds(): string[] { if (this.isMulti) { return Object.keys(this.contexts); @@ -262,25 +327,17 @@ export default class Context { return [this.kind]; } - private static getValueFromContext( - reference: AttributeReference, - context?: LDContextCommon, - ): any { - if (!context || !reference.isValid) { - return undefined; - } - - return reference.get(context); - } - /** - * Attempt to get a value for the given context kind using the given reference. - * @param kind The kind of the context to get the value for. - * @param reference The reference to the value to get. - * @returns a value or `undefined` if one is not found. + * Get the kinds, and their keys, for this context. */ - public getValueForKind(kind: string, reference: AttributeReference): any | undefined { - const context = this.isMulti ? this.contexts[kind] : this.context; - return Context.getValueFromContext(reference, context); + public get kindsAndKeys(): Record { + if (this.isMulti) { + return Object.entries(this.contexts) + .reduce((acc: Record, [kind, context]) => { + acc[kind] = context.key; + return acc; + }, {}); + } + return { [this.kind]: this.context!.key }; } } From e23954badcdd8861ceaaf5b903f7bf6f54ab166e Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 26 May 2022 11:39:01 -0700 Subject: [PATCH 37/39] Finish adding context tests. --- sdk-common/__tests__/Context.test.ts | 221 +++++++++++++++++++++++++++ sdk-common/src/Context.ts | 62 +++++++- 2 files changed, 275 insertions(+), 8 deletions(-) create mode 100644 sdk-common/__tests__/Context.test.ts diff --git a/sdk-common/__tests__/Context.test.ts b/sdk-common/__tests__/Context.test.ts new file mode 100644 index 0000000000..2942b9f578 --- /dev/null +++ b/sdk-common/__tests__/Context.test.ts @@ -0,0 +1,221 @@ +import AttributeReference from '../src/AttributeReference'; +import Context from '../src/Context'; + +// A sample of invalid characters. +const invalidSampleChars = [...`#$%&'()*+,/:;<=>?@[\\]^\`{|}~ ¡¢£¤¥¦§¨©ª«¬­®¯°±² +³´µ¶·¸¹º»¼½¾¿À汉字`]; +const badKinds = invalidSampleChars.map((char) => ({ kind: char, key: 'test' })); + +describe.each([ + {}, + { kind: 'kind', key: 'kind' }, + { kind: {}, key: 'key' }, + { kind: 17, key: 'key' }, + { kind: 'multi', key: 'key' }, + { kind: 'multi', bad: 'value' }, + { kind: 'multi', 'p@rty': 'value' }, + { + kind: 'multi', + bad: { + key: 17, + }, + }, + ...badKinds, +])('given invalid LDContext', (ldConext) => { + it('should not create a context', () => { + // Force TS to accept our bad contexts. + // @ts-ignore + expect(Context.FromLDContext(ldConext)).toBeUndefined(); + }); +}); + +describe.each([ + { + key: 'test', + name: 'context name', + custom: { cat: 'calico', '/dog~~//': 'lab' }, + anonymous: true, + secondary: 'secondary', + privateAttributeNames: ['/dog~~//'], + }, + { + kind: 'user', + key: 'test', + name: 'context name', + cat: 'calico', + transient: true, + _meta: { secondary: 'secondary', privateAttributes: ['/~1dog~0~0~1~1'] }, + }, + { + kind: 'multi', + user: { + key: 'test', + cat: 'calico', + transient: true, + name: 'context name', + _meta: { secondary: 'secondary', privateAttributes: ['/~1dog~0~0~1~1'] }, + }, + }, +])('given a series of equivalent good user contexts', (ldConext) => { + // Here we are providing good contexts, but the types derived from + // the parameterization are causing some problems. + // @ts-ignore + const context = Context.FromLDContext(ldConext); + + it('should create a context', () => { + expect(context).toBeDefined(); + }); + + it('should get the same values', () => { + expect(context?.valueForKind('user', new AttributeReference('cat'))).toEqual('calico'); + expect(context?.valueForKind('user', new AttributeReference('name'))).toEqual('context name'); + expect(context?.kinds).toStrictEqual(['user']); + expect(context?.kindsAndKeys).toStrictEqual({ user: 'test' }); + // Canonical keys for 'user' contexts are just the key. + expect(context?.canonicalKey).toEqual('test'); + expect(context?.valueForKind('user', new AttributeReference('transient'))).toBeTruthy(); + expect(context?.secondary('user')).toEqual('secondary'); + expect(context?.isMultiKind).toBeFalsy(); + expect(context?.privateAttributes('user')?.[0].redactionName) + .toEqual(new AttributeReference('/~1dog~0~0~1~1').redactionName); + }); + + it('should not get values for a context kind that does not exist', () => { + expect(context?.valueForKind('org', new AttributeReference('cat'))).toBeUndefined(); + }); + + it('should have the correct kinds', () => { + expect(context?.kinds).toEqual(['user']); + }); + + it('should have the correct kinds and keys', () => { + expect(context?.kindsAndKeys).toEqual({ user: 'test' }); + }); +}); + +describe('given a valid legacy user without custom attributes', () => { + const context = Context.FromLDContext({ + key: 'test', + name: 'context name', + custom: { cat: 'calico', '/dog~~//': 'lab' }, + anonymous: true, + secondary: 'secondary', + privateAttributeNames: ['/dog~~//'], + }); + + it('should create a context', () => { + expect(context).toBeDefined(); + }); + + it('should get expected values', () => { + expect(context?.valueForKind('user', new AttributeReference('name'))).toEqual('context name'); + expect(context?.kinds).toStrictEqual(['user']); + expect(context?.kindsAndKeys).toStrictEqual({ user: 'test' }); + // Canonical keys for 'user' contexts are just the key. + expect(context?.canonicalKey).toEqual('test'); + expect(context?.valueForKind('user', new AttributeReference('transient'))).toBeTruthy(); + expect(context?.secondary('user')).toEqual('secondary'); + expect(context?.isMultiKind).toBeFalsy(); + expect(context?.privateAttributes('user')?.[0].redactionName) + .toEqual(new AttributeReference('/~1dog~0~0~1~1').redactionName); + }); +}); + +describe('given a non-user single kind context', () => { + const context = Context.FromLDContext({ + kind: 'org', + // Key will be URL encoded. + key: 'Org/Key', + value: 'OrgValue', + }); + it('should have the correct canonical key', () => { + expect(context?.canonicalKey).toEqual('org:Org%2FKey'); + }); + + it('secondary should not be defined when not present', () => { + expect(context?.secondary('org')).toBeUndefined(); + }); + + it('should have the correct kinds', () => { + expect(context?.kinds).toEqual(['org']); + }); + + it('should have the correct kinds and keys', () => { + expect(context?.kindsAndKeys).toEqual({ org: 'Org/Key' }); + }); +}); + +it('secondary should be defined when present', () => { + const context = Context.FromLDContext({ + kind: 'org', + // Key will be URL encoded. + key: 'Org/Key', + value: 'OrgValue', + _meta: { secondary: 'secondary' }, + }); + expect(context?.secondary('org')).toEqual('secondary'); +}); + +it('secondary should be undefined when meta is present, but secondary is not', () => { + const context = Context.FromLDContext({ + kind: 'org', + // Key will be URL encoded. + key: 'Org/Key', + value: 'OrgValue', + _meta: {}, + }); + expect(context?.secondary('org')).toBeUndefined(); +}); + +it('secondary key should be undefined when not a string', () => { + const context = Context.FromLDContext({ + // @ts-ignore + kind: 'org', + // Key will be URL encoded. + key: 'Org/Key', + // @ts-ignore + value: 'OrgValue', + // @ts-ignore + _meta: { secondary: 17 }, // This really displeases typescript. + }); + expect(context?.secondary('org')).toBeUndefined(); +}); + +describe('given a multi-kind context', () => { + const context = Context.FromLDContext({ + kind: 'multi', + + org: { + key: 'OrgKey', + value: 'OrgValue', + _meta: { + secondary: 'value', + }, + }, + user: { + key: 'User /Key', + // Key will be URL encoded. + value: 'UserValue', + }, + }); + + it('should have the correct canonical key', () => { + expect(context?.canonicalKey).toEqual('org:OrgKey:user:User%20%2FKey'); + }); + + it('should get values from the correct context', () => { + expect(context?.valueForKind('org', new AttributeReference('value'))).toEqual('OrgValue'); + expect(context?.valueForKind('user', new AttributeReference('value'))).toEqual('UserValue'); + + expect(context?.secondary('org')).toEqual('value'); + expect(context?.secondary('user')).toBeUndefined(); + }); + + it('should have the correct kinds', () => { + expect(context?.kinds).toEqual(['org', 'user']); + }); + + it('should have the correct kinds and keys', () => { + expect(context?.kindsAndKeys).toEqual({ org: 'OrgKey', user: 'User /Key' }); + }); +}); diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts index 15385ba57f..8edeaebdb6 100644 --- a/sdk-common/src/Context.ts +++ b/sdk-common/src/Context.ts @@ -124,18 +124,28 @@ function legacyToSingleKind(user: LDUser): LDSingleKindContext { // we would want it to not possibly match true/false unless defined. // Which is different than coercing a null/undefined transient as `false`. if (defined(user.anonymous)) { - const transient = !!singleKindContext.anonymous; + const transient = !!user.anonymous; delete singleKindContext.anonymous; singleKindContext.transient = transient; } if (defined(user.secondary)) { singleKindContext._meta = {}; - const { secondary } = singleKindContext; + const { secondary } = user; delete singleKindContext.secondary; singleKindContext._meta.secondary = secondary; } + // TODO: Determine if we want to enforce typing. Previously we would have + // stringified these at event type. + singleKindContext.name = user.name; + singleKindContext.ip = user.ip; + singleKindContext.firstName = user.firstName; + singleKindContext.lastName = user.lastName; + singleKindContext.email = user.email; + singleKindContext.avatar = user.avatar; + singleKindContext.country = user.country; + // We are not pulling private attributes over because we will serialize // those from attribute references for events. @@ -148,7 +158,7 @@ function legacyToSingleKind(user: LDUser): LDSingleKindContext { * the type system. */ export default class Context { - private context?: LDSingleKindContext; + private context?: LDContextCommon; private isMulti: boolean = false; @@ -156,7 +166,7 @@ export default class Context { private contexts: Record = {}; - private privateAttributes?: Record; + private privateAttributeReferences?: Record; public readonly kind: string; @@ -183,22 +193,30 @@ export default class Context { } private contextForKind(kind: string): LDContextCommon | undefined { - return this.isMulti ? this.contexts[kind] : this.context; + if (this.isMulti) { + return this.contexts[kind]; + } + if (this.kind === kind) { + return this.context; + } + return undefined; } private static FromMultiKindContext(context: LDMultiKindContext): Context | undefined { const kinds = Object.keys(context).filter((key) => key !== 'kind'); - const kindsValid = kinds.every(validKind); + const kindsValid = kinds.every(validKind) && kinds.length; if (!kindsValid) { return undefined; } + const privateAttributes: Record = {}; let contextsAreObjects = true; const contexts = kinds.reduce((acc: Record, kind) => { const singleContext = context[kind]; if (isContextCommon(singleContext)) { acc[kind] = singleContext; + privateAttributes[kind] = processPrivateAttributes(singleContext._meta?.privateAttributes); } else { // No early break isn't the most efficient, but it is an error condition. contextsAreObjects = false; @@ -214,8 +232,22 @@ export default class Context { return undefined; } + // There was only a single kind in the multi-kind context. + // So we can just translate this to a single-kind context. + // TODO: Node was not doing this. So we should determine if we want to do this. + // it would make it consistent with strongly typed SDKs. + if (kinds.length === 1) { + const kind = kinds[0]; + const created = new Context(kind); + created.context = contexts[kind]; + created.privateAttributeReferences = privateAttributes; + created.isUser = kind === 'user'; + return created; + } + const created = new Context(context.kind); created.contexts = contexts; + created.privateAttributeReferences = privateAttributes; created.isMulti = true; return created; @@ -233,7 +265,7 @@ export default class Context { const created = new Context(kind); created.isUser = kind === 'user'; created.context = context; - created.privateAttributes = { + created.privateAttributeReferences = { [kind]: privateAttributeReferences, }; return created; @@ -250,7 +282,7 @@ export default class Context { const created = new Context('user'); created.isUser = true; created.context = legacyToSingleKind(context); - created.privateAttributes = { + created.privateAttributeReferences = { user: processPrivateAttributes(context.privateAttributeNames, true), }; return created; @@ -340,4 +372,18 @@ export default class Context { } return { [this.kind]: this.context!.key }; } + + /** + * Get the attribute references. + * + * For now this is for testing and therefore is flagged internal. + * It will not be accessible outside this package. + * + * @internal + * + * @param kind + */ + public privateAttributes(kind: string): AttributeReference[] { + return this.privateAttributeReferences?.[kind] || []; + } } From 552d49e8ea47d9427dab6ff2947ccbac6dbd966c Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 1 Jun 2022 09:24:14 -0700 Subject: [PATCH 38/39] Update sdk-common/src/Context.ts Co-authored-by: Matthew M. Keeler --- sdk-common/src/Context.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts index 8edeaebdb6..f6aa361afb 100644 --- a/sdk-common/src/Context.ts +++ b/sdk-common/src/Context.ts @@ -259,7 +259,7 @@ export default class Context { const keyValid = validKey(key); if (keyValid && kindValid) { - // The JSON interfaces uses dandling _. + // The JSON interfaces uses dangling _. // eslint-disable-next-line no-underscore-dangle const privateAttributeReferences = processPrivateAttributes(context._meta?.privateAttributes); const created = new Context(kind); From 347be72aae668c889bedabc8ee06734bd1f73a25 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 1 Jun 2022 09:24:44 -0700 Subject: [PATCH 39/39] Update sdk-common/src/Context.ts Co-authored-by: Matthew M. Keeler --- sdk-common/src/Context.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk-common/src/Context.ts b/sdk-common/src/Context.ts index f6aa361afb..65f889b705 100644 --- a/sdk-common/src/Context.ts +++ b/sdk-common/src/Context.ts @@ -64,7 +64,7 @@ function isLegacyUser(context: LDContext): context is LDUser { * * Due to a limitation in the expressiveness of these highly polymorphic types any field * in a multi-kind context can either be a context or 'kind'. So we need to re-assure - * the compiler that is isn't the word multi. + * the compiler that it isn't the word multi. * * Because we do not allow top level values in a multi-kind context we can validate * that as well.