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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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/46] 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 7fc75e14060896e1330cddc7a516c0dce0f759d6 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 27 May 2022 09:34:11 -0700 Subject: [PATCH 38/46] Move logging settings/interfaces to sdk-common, implement loggers, implement tests. --- .../__tests__/logging/BasicLogger.test.ts | 72 +++++++++ .../__tests__/logging/SafeLogger.test.ts | 33 ++++ sdk-common/__tests__/logging/format.test.ts | 39 +++++ sdk-common/src/api/index.ts | 1 + .../src/api/logging}/BasicLoggerOptions.ts | 12 +- .../src/api/logging}/LDLogLevel.ts | 0 .../src/api/logging}/LDLogger.ts | 0 sdk-common/src/api/logging/index.ts | 3 + sdk-common/src/index.ts | 1 + sdk-common/src/logging/BasicLogger.ts | 93 +++++++++++ sdk-common/src/logging/SafeLogger.ts | 70 ++++++++ sdk-common/src/logging/format.ts | 151 ++++++++++++++++++ sdk-common/src/logging/index.ts | 5 + sdk-common/src/validators.ts | 19 +++ server-sdk-common/__tests__/Logger.ts | 44 ++--- server-sdk-common/src/api/index.ts | 2 - .../api/integrations/FileDataSourceOptions.ts | 2 +- .../src/api/options/LDOptions.ts | 2 +- server-sdk-common/src/api/options/index.ts | 1 - .../src/options/Configuration.ts | 4 +- .../src/options/ValidatedOptions.ts | 3 +- 21 files changed, 527 insertions(+), 30 deletions(-) create mode 100644 sdk-common/__tests__/logging/BasicLogger.test.ts create mode 100644 sdk-common/__tests__/logging/SafeLogger.test.ts create mode 100644 sdk-common/__tests__/logging/format.test.ts rename {server-sdk-common/src/api/options => sdk-common/src/api/logging}/BasicLoggerOptions.ts (70%) rename {server-sdk-common/src/api => sdk-common/src/api/logging}/LDLogLevel.ts (100%) rename {server-sdk-common/src/api => sdk-common/src/api/logging}/LDLogger.ts (100%) create mode 100644 sdk-common/src/api/logging/index.ts create mode 100644 sdk-common/src/logging/BasicLogger.ts create mode 100644 sdk-common/src/logging/SafeLogger.ts create mode 100644 sdk-common/src/logging/format.ts create mode 100644 sdk-common/src/logging/index.ts diff --git a/sdk-common/__tests__/logging/BasicLogger.test.ts b/sdk-common/__tests__/logging/BasicLogger.test.ts new file mode 100644 index 0000000000..f8e82a8e9a --- /dev/null +++ b/sdk-common/__tests__/logging/BasicLogger.test.ts @@ -0,0 +1,72 @@ +import { BasicLogger, LDLogLevel } from '../../src'; + +const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); + +describe.each<[LDLogLevel, string[]]>([ + ['debug', ['a', 'b', 'c', 'd']], + ['info', ['b', 'c', 'd']], + ['warn', ['c', 'd']], + ['error', ['d']], + ['none', []], +])('given a logger with a log level', (level: LDLogLevel, expected: string[]) => { + const strings: string[] = []; + const logger = new BasicLogger({level, destination: (...args: any) => { + strings.push(args.join(' ')) + }}) + it('it only logs messages of that level', () => { + logger.debug('a'); + logger.info('b'); + logger.warn('c'); + logger.error('d'); + expect(strings).toEqual(expected); + }); +}); + +describe('given a default logger', () => { + const logger = new BasicLogger({}); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('logs to the console', () => { + logger.warn('potato', 'bacon'); + expect(spy).toHaveBeenCalledWith('potato', 'bacon'); + }); +}); + +describe('given a logger with a destination that throws', () => { + const logger = new BasicLogger({destination: () => { + throw new Error('BAD LOGGER'); + }}) + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('logs to the console instead of throwing', () => { + logger.error('a'); + expect(spy).toHaveBeenCalledWith('a'); + }); +}); + +describe('given a logger with a formatter that throws', () => { + const strings: string[] = []; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + const logger = new BasicLogger({ + destination: (...args: any) => { + strings.push(args.join(' ')) + }, + formatter: () => { + throw new Error('BAD LOGGER'); + }}) + + it('logs with default formatting instead of throwing', () => { + logger.error('a'); + expect(spy).toHaveBeenCalledTimes(0); + }); +}); \ No newline at end of file diff --git a/sdk-common/__tests__/logging/SafeLogger.test.ts b/sdk-common/__tests__/logging/SafeLogger.test.ts new file mode 100644 index 0000000000..0abd9b64d2 --- /dev/null +++ b/sdk-common/__tests__/logging/SafeLogger.test.ts @@ -0,0 +1,33 @@ +import { BasicLogger, LDLogger } from '../../src'; +import { SafeLogger } from '../../src/logging/SafeLogger'; + +it('throws when constructed with an invalid logger', () => { + expect( + () => { + new SafeLogger({} as LDLogger, new BasicLogger({})); + } + ).toThrow(); +}); + +describe('given a logger that throws in logs', () => { + const strings: string[] = []; + const logger = new SafeLogger({ + info: () => { throw new Error('info') }, + debug: () => { throw new Error('info') }, + warn: () => { throw new Error('info') }, + error: () => { throw new Error('info') }, + }, new BasicLogger({ + level: 'debug', + destination: (...args: any) => { + strings.push(args.join(' ')); + } + })); + + it("uses the fallback logger", () => { + logger.debug('a'); + logger.info('b'); + logger.warn('c'); + logger.error('d'); + expect(strings).toEqual(['a', 'b', 'c', 'd']); + }); +}); \ No newline at end of file diff --git a/sdk-common/__tests__/logging/format.test.ts b/sdk-common/__tests__/logging/format.test.ts new file mode 100644 index 0000000000..6a4e1f2931 --- /dev/null +++ b/sdk-common/__tests__/logging/format.test.ts @@ -0,0 +1,39 @@ +import format from '../../src/logging/format'; + +// For circular reference test. +const circular: any = {}; +circular.circular = circular; + +describe.each([ + ['', [], ''], + ['the', ['end'], 'the end'], + ['%s', [], '%s'], + ['%s', [1], '1'], + ['The best %s', [{ apple: 'pie' }], 'The best {\"apple\":\"pie\"}'], + ['The best %o', [{ apple: 'pie' }], 'The best {\"apple\":\"pie\"}'], + ['The best %O', [{ apple: 'pie' }], 'The best {\"apple\":\"pie\"}'], + ['%o', [17], '17'], + ['%O', [17], '17'], + ['', [{ apple: 'pie' }, 7, 12], '{\"apple\":\"pie\"} 7 12'], + ['%s', [BigInt(1)], "1n"], + ['%d', [BigInt(1)], "1n"], + ['%i', [BigInt(1)], "1n"], + ['%f', [BigInt(1)], "1"], + ['%i', [3.14159], "3"], + ['%i %d', [3.14159], "3 %d"], + ['', [1, 2, 3, 4], "1 2 3 4"], + ['%s %d %f', [1, 2, 3, 4], "1 2 3 4"], + ['%s %d %f ', [1, 2, 3, 4], "1 2 3 4"], + ['%s %j', [circular, circular], "[Circular] [Circular]"], + ['%d', [Symbol('foo')], "NaN"], + ['%i', [Symbol('foo')], "NaN"], + ['%f', [Symbol('foo')], "NaN"], + ['%%', [], "%"], + ['', [Symbol('foo'), circular, BigInt(7), { apple: 'pie' }, global, undefined, null], + " [Circular] 7n {\"apple\":\"pie\"} [object Object] undefined null"] +])('given node style format strings', (formatStr, args, result) => { + + it('produces the expected string', () => { + expect(format(formatStr, ...args)).toEqual(result); + }); +}); \ No newline at end of file diff --git a/sdk-common/src/api/index.ts b/sdk-common/src/api/index.ts index 371c16e0e1..11ed751c7d 100644 --- a/sdk-common/src/api/index.ts +++ b/sdk-common/src/api/index.ts @@ -1,2 +1,3 @@ export * from './context'; export * from './LDContext'; +export * from './logging'; diff --git a/server-sdk-common/src/api/options/BasicLoggerOptions.ts b/sdk-common/src/api/logging/BasicLoggerOptions.ts similarity index 70% rename from server-sdk-common/src/api/options/BasicLoggerOptions.ts rename to sdk-common/src/api/logging/BasicLoggerOptions.ts index 2cb5a98120..a707b4e9a1 100644 --- a/server-sdk-common/src/api/options/BasicLoggerOptions.ts +++ b/sdk-common/src/api/logging/BasicLoggerOptions.ts @@ -1,4 +1,4 @@ -import { LDLogLevel } from '../LDLogLevel'; +import { LDLogLevel } from './LDLogLevel'; /** * Configuration for [[basicLogger]]. @@ -28,4 +28,14 @@ export interface BasicLoggerOptions { * initialization to fail. */ destination?: (line: string) => void; + + /** + * An optional formatter to use. The formatter should be compatible + * with node-style format strings like those used with `util.format`. + * + * If not specified, then a default implementation will be used. + * But using a node-specific implementation, for instance, would + * have performance and quality benefits. + */ + formatter?: (...args: any[]) => string; } diff --git a/server-sdk-common/src/api/LDLogLevel.ts b/sdk-common/src/api/logging/LDLogLevel.ts similarity index 100% rename from server-sdk-common/src/api/LDLogLevel.ts rename to sdk-common/src/api/logging/LDLogLevel.ts diff --git a/server-sdk-common/src/api/LDLogger.ts b/sdk-common/src/api/logging/LDLogger.ts similarity index 100% rename from server-sdk-common/src/api/LDLogger.ts rename to sdk-common/src/api/logging/LDLogger.ts diff --git a/sdk-common/src/api/logging/index.ts b/sdk-common/src/api/logging/index.ts new file mode 100644 index 0000000000..41087dc5ce --- /dev/null +++ b/sdk-common/src/api/logging/index.ts @@ -0,0 +1,3 @@ +export * from './BasicLoggerOptions'; +export * from './LDLogger'; +export * from './LDLogLevel'; diff --git a/sdk-common/src/index.ts b/sdk-common/src/index.ts index d06405c841..62a1fe8400 100644 --- a/sdk-common/src/index.ts +++ b/sdk-common/src/index.ts @@ -1,2 +1,3 @@ export * from './api'; export * from './validators'; +export * from './logging'; \ No newline at end of file diff --git a/sdk-common/src/logging/BasicLogger.ts b/sdk-common/src/logging/BasicLogger.ts new file mode 100644 index 0000000000..858cec2de7 --- /dev/null +++ b/sdk-common/src/logging/BasicLogger.ts @@ -0,0 +1,93 @@ +import { BasicLoggerOptions, LDLogger } from '../api'; +import format from './format'; + +const LogPriority = { + debug: 0, + info: 1, + warn: 2, + error: 3, + none: 4, +}; + +/** + * A basic logger which handles filtering by level. + * + * With the default options it will write to `console.error` + * and it will use the formatting provided by `console.error`. + * If the destination is overwritten, then it will use an included + * formatter similar to `util.format`. + * + * If a formatter is available, then that should be overridden + * as well for performance. + */ +export default class BasicLogger implements LDLogger { + private logLevel: number; + + private destination?: (line: string) => void; + + private formatter?: (...args: any[]) => string; + + constructor(options: BasicLoggerOptions) { + this.logLevel = LogPriority[options.level ?? 'info'] ?? LogPriority.info; + // eslint-disable-next-line no-console + this.destination = options.destination; + this.formatter = options.formatter; + } + + private tryFormat(...args: any[]): string { + try { + if (this.formatter) { + // In case the provided formatter fails. + return this.formatter?.(...args); + } + return format(...args); + } catch { + return format(...args); + } + } + + private tryWrite(msg: string) { + try { + this.destination!(msg); + } catch { + // eslint-disable-next-line no-console + console.error(msg); + } + } + + private log(level: number, args: any[]) { + // Default to info. + if (level >= this.logLevel) { + try { + if (this.destination) { + this.tryWrite(this.tryFormat(...args)); + } else { + // `console.error` has its own formatter. + // So we don't need to do anything. + // eslint-disable-next-line no-console + console.error(...args); + } + } catch { + // If all else fails do not break. + // eslint-disable-next-line no-console + console.error(...args); + } + } + } + + error(...args: any[]): void { + this.log(LogPriority.error, args); + } + + warn(...args: any[]): void { + this.log(LogPriority.warn, args); + } + + info(...args: any[]): void { + this.log(LogPriority.info, args); + } + + debug(...args: any[]): void { + this.log(LogPriority.debug, args); + } +} diff --git a/sdk-common/src/logging/SafeLogger.ts b/sdk-common/src/logging/SafeLogger.ts new file mode 100644 index 0000000000..dbac8409b7 --- /dev/null +++ b/sdk-common/src/logging/SafeLogger.ts @@ -0,0 +1,70 @@ +import { LDLogger, LDLogLevel } from '../api'; +import { TypeValidators } from '../validators'; + +const loggerRequirements = { + error: TypeValidators.Function, + warn: TypeValidators.Function, + info: TypeValidators.Function, + debug: TypeValidators.Function +} + +/** + * The safeLogger logic exists because we allow the application to pass in a custom logger, but + * there is no guarantee that the logger works correctly and if it ever throws exceptions there + * could be serious consequences (e.g. an uncaught exception within an error event handler, due + * to the SDK trying to log the error, can terminate the application). An exception could result + * from faulty logic in the logger implementation, or it could be that this is not a logger at + * all but some other kind of object; the former is handled by a catch block that logs an error + * message to the SDK's default logger, and we can at least partly guard against the latter by + * checking for the presence of required methods at configuration time. + */ +export class SafeLogger implements LDLogger { + private logger: LDLogger; + + private fallback: LDLogger; + + /** + * Construct a safe logger with the specified logger. + * @param logger The logger to use. + * @param fallback A fallback logger to use in case an issue is encountered using + * the provided logger. + */ + constructor(logger: LDLogger, fallback: LDLogger) { + Object.entries(loggerRequirements).forEach(([level, validator]) => { + if(!validator.is((logger as any)[level])) { + throw new Error('Provided logger instance must support logger.' + level + '(...) method'); + // Note that the SDK normally does not throw exceptions to the application, but that rule + // does not apply to LDClient.init() which will throw an exception if the parameters are so + // invalid that we cannot proceed with creating the client. An invalid logger meets those + // criteria since the SDK calls the logger during nearly all of its operations. + } + }); + this.logger = logger; + this.fallback = fallback; + } + + private log(level: 'error' | 'warn' | 'info' | 'debug', args: any[]) { + try { + this.logger[level](...args); + } catch { + // If all else fails do not break. + this.fallback[level](...args); + } + } + + error(...args: any[]): void { + this.log('error', args); + } + + warn(...args: any[]): void { + this.log('warn', args); + } + + info(...args: any[]): void { + this.log('info', args); + } + + debug(...args: any[]): void { + this.log('debug', args); + } +} \ No newline at end of file diff --git a/sdk-common/src/logging/format.ts b/sdk-common/src/logging/format.ts new file mode 100644 index 0000000000..55a93eeacb --- /dev/null +++ b/sdk-common/src/logging/format.ts @@ -0,0 +1,151 @@ +import { TypeValidators } from '../validators'; + +/** + * Attempt to produce a string representation of a value. + * The format should be roughly comparable to `util.format` + * aside from object which will be JSON versus the `util.inspect` + * format. + * @param val + * @returns A string representation of the value if possible. + */ +function tryStringify(val: any) { + if (typeof val === 'string') { + return val; + } + if (val === undefined) { + return 'undefined'; + } + if (val === null) { + return 'null'; + } + if (Object.prototype.hasOwnProperty.call(val, 'toString')) { + try { + return val.toString(); + } catch { /* Keep going */ } + } + + if (typeof val === 'bigint') { + return `${val}n`; + } + try { + return JSON.stringify(val); + } catch (error) { + if (error instanceof TypeError && error.message.indexOf('circular') >= 0) { + return '[Circular]'; + } + return '[Not Stringifiable]'; + } +} + +/** + * Attempt to produce a numeric representation. + * BigInts have an `n` suffix. + * @param val + * @returns The numeric representation or 'NaN' if not numeric. + */ +function toNumber(val: any): string { + // Symbol has to be treated special because it will + // throw an exception if an attempt is made to convert it. + if (typeof val === 'symbol') { + return 'NaN'; + } + if (typeof val === 'bigint') { + return `${val}n`; + } + return String(Number(val)); +} + +/** + * Attempt to produce an integer representation. + * BigInts have an `n` suffix. + * @param val + * @returns The integer representation or 'NaN' if not numeric. + */ +function toInt(val: any): string { + if (typeof val === 'symbol') { + return 'NaN'; + } + if (typeof val === 'bigint') { + return `${val}n`; + } + return String(parseInt(val, 10)); +} + +/** + * Attempt to produce a float representation. + * BigInts have an `n` suffix. + * @param val + * @returns The integer representation or 'NaN' if not numeric. + */ +function toFloat(val: any): string { + if (typeof val === 'symbol') { + return 'NaN'; + } + return String(parseFloat(val)); +} + +// Based on: +// https://nodejs.org/api/util.html#utilformatformat-args +// The result will not match node exactly, but it should get the +// right information through. +const escapes: Record string> = { + s: (val: any) => tryStringify(val), + d: (val: any) => toNumber(val), + i: (val: any) => toInt(val), + f: (val: any) => toFloat(val), + j: (val: any) => tryStringify(val), + o: (val: any) => tryStringify(val), + O: (val: any) => tryStringify(val), + c: () => '', +}; + +/** + * A basic formatted for use where `util.format` is not available. + * This will not be as performant, but it will produce formatted + * messages. + * + * @internal + * + * @param args + * @returns Formatted string. + */ +export default function format(...args: any[]): string { + const formatString = args.shift(); + if (TypeValidators.String.is(formatString)) { + let out = ''; + let i = 0; + while (i < formatString.length) { + const char = formatString.charAt(i); + if (char === '%') { + const nextIndex = i + 1; + if (nextIndex < formatString.length) { + const nextChar = formatString.charAt(i + 1); + if (nextChar in escapes && args.length) { + const value = args.shift(); + // This rule is for math. + // eslint-disable-next-line no-unsafe-optional-chaining + out += escapes[nextChar]?.(value); + } else if (nextChar === '%') { + out += '%'; + } else { + out += `%${nextChar}`; + } + i += 2; + } + } else { + out += char; + i += 1; + } + } + // If there are any args left after we exhaust the format string + // then just stick those on the end. + if (args.length) { + if (out.length) { + out += ' '; + } + out += args.map(tryStringify).join(' '); + } + return out; + } + return args.map(tryStringify).join(' '); +} diff --git a/sdk-common/src/logging/index.ts b/sdk-common/src/logging/index.ts new file mode 100644 index 0000000000..d9f3976229 --- /dev/null +++ b/sdk-common/src/logging/index.ts @@ -0,0 +1,5 @@ +import BasicLogger from './BasicLogger'; + +export { + BasicLogger +}; diff --git a/sdk-common/src/validators.ts b/sdk-common/src/validators.ts index bdebc3f7fc..f3e58b4890 100644 --- a/sdk-common/src/validators.ts +++ b/sdk-common/src/validators.ts @@ -119,6 +119,23 @@ export class StringMatchingRegex extends Type { } } +/** + * Validate a value is a function. + */ +export class Function implements TypeValidator { + is(u: unknown): u is (...args: any[]) => void { + // We cannot inspect the parameters and there isn't really + // a generic function type we can instantiate. + // So the type guard is here just to make TS comfortable + // caling something after using this guard. + return typeof u === 'function'; + } + + getType(): string { + return 'function'; + } +} + /** * A set of standard type validators. */ @@ -135,6 +152,8 @@ export class TypeValidators { static readonly Boolean = new Type('boolean', true); + static readonly Function = new Function(); + static NumberWithMin(min: number): NumberWithMinimum { return new NumberWithMinimum(min); } diff --git a/server-sdk-common/__tests__/Logger.ts b/server-sdk-common/__tests__/Logger.ts index 980cf362a8..87e3aa5fc7 100644 --- a/server-sdk-common/__tests__/Logger.ts +++ b/server-sdk-common/__tests__/Logger.ts @@ -1,21 +1,22 @@ -import { LDLogger } from '../src'; +import { LDLogger, LDLogLevel } from '@launchdarkly/js-sdk-common'; -// TODO: Move this to sdk-common when implementing logging. export enum LogLevel { - Debug, - Info, - Warn, - Error, + Debug = 'debug', + Info = 'info', + Warn = 'warn', + Error = 'error', } type ExpectedMessage = { level: LogLevel, matches: RegExp }; export default class TestLogger implements LDLogger { - private readonly messages: Record = { - [LogLevel.Debug]: [], - [LogLevel.Info]: [], - [LogLevel.Warn]: [], - [LogLevel.Error]: [], + private readonly messages: Record = { + debug: [], + info: [], + warn: [], + error: [], + // Should not be used to log. + none: [], }; private callCount = 0; @@ -51,11 +52,12 @@ export default class TestLogger implements LDLogger { expectMessages( expectedMessages: ExpectedMessage[], ): void { - const matched: Record = { - [LogLevel.Debug]: [], - [LogLevel.Info]: [], - [LogLevel.Warn]: [], - [LogLevel.Error]: [], + const matched: Record = { + debug: [], + info: [], + warn: [], + error: [], + none: [], }; expectedMessages.forEach((expectedMessage) => { @@ -81,25 +83,25 @@ export default class TestLogger implements LDLogger { this.waiters.forEach((waiter) => waiter()); } - private log(level: LogLevel, ...args: any[]) { + private log(level: LDLogLevel, ...args: any[]) { this.messages[level].push(args.join(' ')); this.callCount += 1; this.checkResolves(); } error(...args: any[]): void { - this.log(LogLevel.Error, args); + this.log('error', args); } warn(...args: any[]): void { - this.log(LogLevel.Warn, args); + this.log('warn', args); } info(...args: any[]): void { - this.log(LogLevel.Info, args); + this.log('info', args); } debug(...args: any[]): void { - this.log(LogLevel.Debug, args); + this.log('debug', args); } } diff --git a/server-sdk-common/src/api/index.ts b/server-sdk-common/src/api/index.ts index 59edda462f..978cea6c6e 100644 --- a/server-sdk-common/src/api/index.ts +++ b/server-sdk-common/src/api/index.ts @@ -1,8 +1,6 @@ export * from './data'; export * from './options'; export * from './LDClient'; -export * from './LDLogger'; -export * from './LDLogLevel'; export * from './subsystems/LDStreamProcessor'; // These are items that should be less frequently used, and therefore they diff --git a/server-sdk-common/src/api/integrations/FileDataSourceOptions.ts b/server-sdk-common/src/api/integrations/FileDataSourceOptions.ts index a0482d65c4..36cdf83fd5 100644 --- a/server-sdk-common/src/api/integrations/FileDataSourceOptions.ts +++ b/server-sdk-common/src/api/integrations/FileDataSourceOptions.ts @@ -1,4 +1,4 @@ -import { LDLogger } from '../LDLogger'; +import { LDLogger } from '@launchdarkly/js-sdk-common'; /** * Configuration for [[FileDataSource]]. diff --git a/server-sdk-common/src/api/options/LDOptions.ts b/server-sdk-common/src/api/options/LDOptions.ts index 1f8e587ee5..3bb94fad5b 100644 --- a/server-sdk-common/src/api/options/LDOptions.ts +++ b/server-sdk-common/src/api/options/LDOptions.ts @@ -1,5 +1,5 @@ import { LDFeatureStore } from '../subsystems/LDFeatureStore'; -import { LDLogger } from '../LDLogger'; +import { LDLogger } from '@launchdarkly/js-sdk-common'; import { LDBigSegmentsOptions } from './LDBigSegmentsOptions'; import { LDProxyOptions } from './LDProxyOptions'; import { LDTLSOptions } from './LDTLSOptions'; diff --git a/server-sdk-common/src/api/options/index.ts b/server-sdk-common/src/api/options/index.ts index 9531a83aab..a3467b9cc5 100644 --- a/server-sdk-common/src/api/options/index.ts +++ b/server-sdk-common/src/api/options/index.ts @@ -1,4 +1,3 @@ -export * from './BasicLoggerOptions'; export * from './LDBigSegmentsOptions'; export * from './LDOptions'; export * from './LDProxyOptions'; diff --git a/server-sdk-common/src/options/Configuration.ts b/server-sdk-common/src/options/Configuration.ts index a58394f28c..bfe1dbf254 100644 --- a/server-sdk-common/src/options/Configuration.ts +++ b/server-sdk-common/src/options/Configuration.ts @@ -1,6 +1,6 @@ -import { NumberWithMinimum, TypeValidator, TypeValidators } from '@launchdarkly/js-sdk-common'; +import { LDLogger, NumberWithMinimum, TypeValidator, TypeValidators } from '@launchdarkly/js-sdk-common'; import { - LDLogger, LDOptions, LDProxyOptions, LDTLSOptions, + LDOptions, LDProxyOptions, LDTLSOptions, } from '../api'; import ApplicationTags from './ApplicationTags'; import OptionMessages from './OptionMessages'; diff --git a/server-sdk-common/src/options/ValidatedOptions.ts b/server-sdk-common/src/options/ValidatedOptions.ts index d953657d30..35f4379bbc 100644 --- a/server-sdk-common/src/options/ValidatedOptions.ts +++ b/server-sdk-common/src/options/ValidatedOptions.ts @@ -1,4 +1,5 @@ -import { LDLogger, LDProxyOptions, LDTLSOptions } from '../api'; +import { LDLogger } from '@launchdarkly/js-sdk-common'; +import { LDProxyOptions, LDTLSOptions } from '../api'; /** * This interface applies to the options after they have been validated and defaults From bbd02bbe00793b7324d55928ca66aec193f1400d Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 27 May 2022 09:37:24 -0700 Subject: [PATCH 39/46] Lint --- .../__tests__/logging/BasicLogger.test.ts | 26 +++++++----- .../__tests__/logging/SafeLogger.test.ts | 20 ++++----- sdk-common/__tests__/logging/format.test.ts | 41 +++++++++---------- sdk-common/src/index.ts | 2 +- sdk-common/src/logging/SafeLogger.ts | 28 ++++++------- sdk-common/src/logging/index.ts | 4 +- .../src/api/options/LDOptions.ts | 2 +- .../src/options/Configuration.ts | 4 +- 8 files changed, 67 insertions(+), 60 deletions(-) diff --git a/sdk-common/__tests__/logging/BasicLogger.test.ts b/sdk-common/__tests__/logging/BasicLogger.test.ts index f8e82a8e9a..cb0be3db9c 100644 --- a/sdk-common/__tests__/logging/BasicLogger.test.ts +++ b/sdk-common/__tests__/logging/BasicLogger.test.ts @@ -10,9 +10,12 @@ describe.each<[LDLogLevel, string[]]>([ ['none', []], ])('given a logger with a log level', (level: LDLogLevel, expected: string[]) => { const strings: string[] = []; - const logger = new BasicLogger({level, destination: (...args: any) => { - strings.push(args.join(' ')) - }}) + const logger = new BasicLogger({ + level, + destination: (...args: any) => { + strings.push(args.join(' ')); + }, + }); it('it only logs messages of that level', () => { logger.debug('a'); logger.info('b'); @@ -36,9 +39,11 @@ describe('given a default logger', () => { }); describe('given a logger with a destination that throws', () => { - const logger = new BasicLogger({destination: () => { - throw new Error('BAD LOGGER'); - }}) + const logger = new BasicLogger({ + destination: () => { + throw new Error('BAD LOGGER'); + }, + }); beforeEach(() => { jest.clearAllMocks(); @@ -59,14 +64,15 @@ describe('given a logger with a formatter that throws', () => { const logger = new BasicLogger({ destination: (...args: any) => { - strings.push(args.join(' ')) + strings.push(args.join(' ')); }, formatter: () => { - throw new Error('BAD LOGGER'); - }}) + throw new Error('BAD LOGGER'); + }, + }); it('logs with default formatting instead of throwing', () => { logger.error('a'); expect(spy).toHaveBeenCalledTimes(0); }); -}); \ No newline at end of file +}); diff --git a/sdk-common/__tests__/logging/SafeLogger.test.ts b/sdk-common/__tests__/logging/SafeLogger.test.ts index 0abd9b64d2..fd8be8dd23 100644 --- a/sdk-common/__tests__/logging/SafeLogger.test.ts +++ b/sdk-common/__tests__/logging/SafeLogger.test.ts @@ -1,33 +1,31 @@ import { BasicLogger, LDLogger } from '../../src'; -import { SafeLogger } from '../../src/logging/SafeLogger'; +import SafeLogger from '../../src/logging/SafeLogger'; it('throws when constructed with an invalid logger', () => { expect( - () => { - new SafeLogger({} as LDLogger, new BasicLogger({})); - } + () => new SafeLogger({} as LDLogger, new BasicLogger({})), ).toThrow(); }); describe('given a logger that throws in logs', () => { const strings: string[] = []; const logger = new SafeLogger({ - info: () => { throw new Error('info') }, - debug: () => { throw new Error('info') }, - warn: () => { throw new Error('info') }, - error: () => { throw new Error('info') }, + info: () => { throw new Error('info'); }, + debug: () => { throw new Error('info'); }, + warn: () => { throw new Error('info'); }, + error: () => { throw new Error('info'); }, }, new BasicLogger({ level: 'debug', destination: (...args: any) => { strings.push(args.join(' ')); - } + }, })); - it("uses the fallback logger", () => { + it('uses the fallback logger', () => { logger.debug('a'); logger.info('b'); logger.warn('c'); logger.error('d'); expect(strings).toEqual(['a', 'b', 'c', 'd']); }); -}); \ No newline at end of file +}); diff --git a/sdk-common/__tests__/logging/format.test.ts b/sdk-common/__tests__/logging/format.test.ts index 6a4e1f2931..961f22959b 100644 --- a/sdk-common/__tests__/logging/format.test.ts +++ b/sdk-common/__tests__/logging/format.test.ts @@ -9,31 +9,30 @@ describe.each([ ['the', ['end'], 'the end'], ['%s', [], '%s'], ['%s', [1], '1'], - ['The best %s', [{ apple: 'pie' }], 'The best {\"apple\":\"pie\"}'], - ['The best %o', [{ apple: 'pie' }], 'The best {\"apple\":\"pie\"}'], - ['The best %O', [{ apple: 'pie' }], 'The best {\"apple\":\"pie\"}'], + ['The best %s', [{ apple: 'pie' }], 'The best {"apple":"pie"}'], + ['The best %o', [{ apple: 'pie' }], 'The best {"apple":"pie"}'], + ['The best %O', [{ apple: 'pie' }], 'The best {"apple":"pie"}'], ['%o', [17], '17'], ['%O', [17], '17'], - ['', [{ apple: 'pie' }, 7, 12], '{\"apple\":\"pie\"} 7 12'], - ['%s', [BigInt(1)], "1n"], - ['%d', [BigInt(1)], "1n"], - ['%i', [BigInt(1)], "1n"], - ['%f', [BigInt(1)], "1"], - ['%i', [3.14159], "3"], - ['%i %d', [3.14159], "3 %d"], - ['', [1, 2, 3, 4], "1 2 3 4"], - ['%s %d %f', [1, 2, 3, 4], "1 2 3 4"], - ['%s %d %f ', [1, 2, 3, 4], "1 2 3 4"], - ['%s %j', [circular, circular], "[Circular] [Circular]"], - ['%d', [Symbol('foo')], "NaN"], - ['%i', [Symbol('foo')], "NaN"], - ['%f', [Symbol('foo')], "NaN"], - ['%%', [], "%"], + ['', [{ apple: 'pie' }, 7, 12], '{"apple":"pie"} 7 12'], + ['%s', [BigInt(1)], '1n'], + ['%d', [BigInt(1)], '1n'], + ['%i', [BigInt(1)], '1n'], + ['%f', [BigInt(1)], '1'], + ['%i', [3.14159], '3'], + ['%i %d', [3.14159], '3 %d'], + ['', [1, 2, 3, 4], '1 2 3 4'], + ['%s %d %f', [1, 2, 3, 4], '1 2 3 4'], + ['%s %d %f ', [1, 2, 3, 4], '1 2 3 4'], + ['%s %j', [circular, circular], '[Circular] [Circular]'], + ['%d', [Symbol('foo')], 'NaN'], + ['%i', [Symbol('foo')], 'NaN'], + ['%f', [Symbol('foo')], 'NaN'], + ['%%', [], '%'], ['', [Symbol('foo'), circular, BigInt(7), { apple: 'pie' }, global, undefined, null], - " [Circular] 7n {\"apple\":\"pie\"} [object Object] undefined null"] + ' [Circular] 7n {"apple":"pie"} [object Object] undefined null'], ])('given node style format strings', (formatStr, args, result) => { - it('produces the expected string', () => { expect(format(formatStr, ...args)).toEqual(result); }); -}); \ No newline at end of file +}); diff --git a/sdk-common/src/index.ts b/sdk-common/src/index.ts index 62a1fe8400..1fcd673c45 100644 --- a/sdk-common/src/index.ts +++ b/sdk-common/src/index.ts @@ -1,3 +1,3 @@ export * from './api'; export * from './validators'; -export * from './logging'; \ No newline at end of file +export * from './logging'; diff --git a/sdk-common/src/logging/SafeLogger.ts b/sdk-common/src/logging/SafeLogger.ts index dbac8409b7..ec67a43e8c 100644 --- a/sdk-common/src/logging/SafeLogger.ts +++ b/sdk-common/src/logging/SafeLogger.ts @@ -1,12 +1,12 @@ -import { LDLogger, LDLogLevel } from '../api'; +import { LDLogger } from '../api'; import { TypeValidators } from '../validators'; const loggerRequirements = { error: TypeValidators.Function, warn: TypeValidators.Function, info: TypeValidators.Function, - debug: TypeValidators.Function -} + debug: TypeValidators.Function, +}; /** * The safeLogger logic exists because we allow the application to pass in a custom logger, but @@ -18,10 +18,10 @@ const loggerRequirements = { * message to the SDK's default logger, and we can at least partly guard against the latter by * checking for the presence of required methods at configuration time. */ -export class SafeLogger implements LDLogger { +export default class SafeLogger implements LDLogger { private logger: LDLogger; - private fallback: LDLogger; + private fallback: LDLogger; /** * Construct a safe logger with the specified logger. @@ -31,8 +31,8 @@ export class SafeLogger implements LDLogger { */ constructor(logger: LDLogger, fallback: LDLogger) { Object.entries(loggerRequirements).forEach(([level, validator]) => { - if(!validator.is((logger as any)[level])) { - throw new Error('Provided logger instance must support logger.' + level + '(...) method'); + if (!validator.is((logger as any)[level])) { + throw new Error(`Provided logger instance must support logger.${level}(...) method`); // Note that the SDK normally does not throw exceptions to the application, but that rule // does not apply to LDClient.init() which will throw an exception if the parameters are so // invalid that we cannot proceed with creating the client. An invalid logger meets those @@ -44,12 +44,12 @@ export class SafeLogger implements LDLogger { } private log(level: 'error' | 'warn' | 'info' | 'debug', args: any[]) { - try { - this.logger[level](...args); - } catch { - // If all else fails do not break. - this.fallback[level](...args); - } + try { + this.logger[level](...args); + } catch { + // If all else fails do not break. + this.fallback[level](...args); + } } error(...args: any[]): void { @@ -67,4 +67,4 @@ export class SafeLogger implements LDLogger { debug(...args: any[]): void { this.log('debug', args); } -} \ No newline at end of file +} diff --git a/sdk-common/src/logging/index.ts b/sdk-common/src/logging/index.ts index d9f3976229..b1aa673fd9 100644 --- a/sdk-common/src/logging/index.ts +++ b/sdk-common/src/logging/index.ts @@ -1,5 +1,7 @@ import BasicLogger from './BasicLogger'; +import SafeLogger from './SafeLogger'; export { - BasicLogger + BasicLogger, + SafeLogger, }; diff --git a/server-sdk-common/src/api/options/LDOptions.ts b/server-sdk-common/src/api/options/LDOptions.ts index 3bb94fad5b..24a0db5aff 100644 --- a/server-sdk-common/src/api/options/LDOptions.ts +++ b/server-sdk-common/src/api/options/LDOptions.ts @@ -1,5 +1,5 @@ -import { LDFeatureStore } from '../subsystems/LDFeatureStore'; import { LDLogger } from '@launchdarkly/js-sdk-common'; +import { LDFeatureStore } from '../subsystems/LDFeatureStore'; import { LDBigSegmentsOptions } from './LDBigSegmentsOptions'; import { LDProxyOptions } from './LDProxyOptions'; import { LDTLSOptions } from './LDTLSOptions'; diff --git a/server-sdk-common/src/options/Configuration.ts b/server-sdk-common/src/options/Configuration.ts index bfe1dbf254..8a15a01f8c 100644 --- a/server-sdk-common/src/options/Configuration.ts +++ b/server-sdk-common/src/options/Configuration.ts @@ -1,4 +1,6 @@ -import { LDLogger, NumberWithMinimum, TypeValidator, TypeValidators } from '@launchdarkly/js-sdk-common'; +import { + LDLogger, NumberWithMinimum, TypeValidator, TypeValidators, +} from '@launchdarkly/js-sdk-common'; import { LDOptions, LDProxyOptions, LDTLSOptions, } from '../api'; From 8d9d3fa58d3d9f1ed3812e35e083d75bfb2a6e36 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 27 May 2022 09:46:50 -0700 Subject: [PATCH 40/46] Remove incorrect comment. --- sdk-common/src/logging/BasicLogger.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk-common/src/logging/BasicLogger.ts b/sdk-common/src/logging/BasicLogger.ts index 858cec2de7..2c8d6d0453 100644 --- a/sdk-common/src/logging/BasicLogger.ts +++ b/sdk-common/src/logging/BasicLogger.ts @@ -56,7 +56,6 @@ export default class BasicLogger implements LDLogger { } private log(level: number, args: any[]) { - // Default to info. if (level >= this.logLevel) { try { if (this.destination) { From de3a5e482749bac58e3a32c9387166c5830e011f Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 27 May 2022 11:23:46 -0700 Subject: [PATCH 41/46] Add in memory data store. --- server-sdk-common/src/InMemoryFeatureStore.ts | 83 +++++++++++++++++++ .../src/api/subsystems/LDFeatureStore.ts | 39 +++++++-- 2 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 server-sdk-common/src/InMemoryFeatureStore.ts diff --git a/server-sdk-common/src/InMemoryFeatureStore.ts b/server-sdk-common/src/InMemoryFeatureStore.ts new file mode 100644 index 0000000000..9ae61277b2 --- /dev/null +++ b/server-sdk-common/src/InMemoryFeatureStore.ts @@ -0,0 +1,83 @@ +import { DataKind } from './api/interfaces'; +import { LDFeatureStoreDataStorage, LDKeyedFeatureStoreItem, LDFeatureStore, LDFeatureStoreKindData, LDFeatureStoreItem } from './api/subsystems'; + +/** + * Clone an object using JSON. This will not preserve + * non-JSON types (like functions). + * @param obj + * @returns A clone of the object. + */ +function clone(obj: any): any { + return JSON.parse(JSON.stringify(obj)); +} + +export default class InMemoryFeatureStore implements LDFeatureStore { + allData: LDFeatureStoreDataStorage = {}; + initCalled = false; + + private addItem(kind: DataKind, key: string, item: LDFeatureStoreItem) { + let items = this.allData[kind.namespace]; + if (!items) { + items = {}; + this.allData[kind.namespace] = items; + } + if (Object.hasOwnProperty.call(items, key)) { + const old = items[key]; + if (!old || old.version < item.version) { + items[key] = item; + } + } else { + items[key] = item; + } + } + + get(kind: DataKind, key: string, callback: (res: LDFeatureStoreItem | null) => void): void { + const items = this.allData[kind.namespace]; + if (items) { + if (Object.prototype.hasOwnProperty.call(items, key)) { + const item = items[key]; + if (item && !item.deleted) { + return callback?.(item); + } + } + } + return callback?.(null); + } + + all(kind: DataKind, callback: (res: LDFeatureStoreKindData) => void): void { + const result: LDFeatureStoreKindData = {}; + const items = this.allData[kind.namespace] ?? {}; + Object.entries(items).forEach(([key, item]) => { + if (item && !item.deleted) { + result[key] = item; + } + }); + callback?.(result); + } + + init(allData: object, callback: () => void): void { + this.initCalled = true; + this.allData = allData as LDFeatureStoreDataStorage; + callback?.(); + } + + delete(kind: DataKind, key: string, version: number, callback: () => void): void { + const deletedItem = { version, deleted: true }; + this.addItem(kind, key, deletedItem); + callback?.(); + } + + upsert(kind: DataKind, data: LDKeyedFeatureStoreItem, callback: () => void): void { + const item = clone(data); + this.addItem(kind, item.key, item); + callback?.(); + } + + initialized(callback: (isInitialized: boolean) => void): void { + return callback?.(this.initCalled); + } + + close(): void { + // For the memory store this is a no-op. + } +} \ No newline at end of file diff --git a/server-sdk-common/src/api/subsystems/LDFeatureStore.ts b/server-sdk-common/src/api/subsystems/LDFeatureStore.ts index 7981e332f2..acb69852b5 100644 --- a/server-sdk-common/src/api/subsystems/LDFeatureStore.ts +++ b/server-sdk-common/src/api/subsystems/LDFeatureStore.ts @@ -1,3 +1,32 @@ +import { DataKind } from '../interfaces'; + +/** + * Represents an item which can be stored in the feature store. + */ +export interface LDFeatureStoreItem { + deleted?: boolean; + version: number; + // The actual data associated with the item. + [attribute: string]: any; +} + +/** + * When upserting an item it must contain a key. + */ +export interface LDKeyedFeatureStoreItem extends LDFeatureStoreItem { + key: string; +} + +/** + * Represents the storage for a single kind of data. e.g. 'flag' or 'segment'. + */ +export interface LDFeatureStoreKindData {[key: string]: LDFeatureStoreItem } + +/** + * Represents the storage for the full data store. + */ +export interface LDFeatureStoreDataStorage {[namespace: string]: LDFeatureStoreKindData } + /** * Interface for a feature store component. * @@ -30,7 +59,7 @@ export interface LDFeatureStore { * Will be called with the retrieved entity, or null if not found. The actual type of the result * value is [[interfaces.VersionedData]]. */ - get(kind: object, key: string, callback: (res: object) => void): void; + get(kind: DataKind, key: string, callback: (res: LDFeatureStoreItem | null) => void): void; /** * Get all entities from a collection. @@ -46,7 +75,7 @@ export interface LDFeatureStore { * Will be called with the resulting map. The actual type of the result value is * `interfaces.KeyedItems`. */ - all(kind: object, callback: (res: object) => void): void; + all(kind: DataKind, callback: (res: LDFeatureStoreKindData) => void): void; /** * Initialize the store, overwriting any existing data. @@ -59,7 +88,7 @@ export interface LDFeatureStore { * @param callback * Will be called when the store has been initialized. */ - init(allData: object, callback: () => void): void; + init(allData: DataKind, callback: () => void): void; /** * Delete an entity from the store. @@ -83,7 +112,7 @@ export interface LDFeatureStore { * @param callback * Will be called when the delete operation is complete. */ - delete(kind: object, key: string, version: string, callback: () => void): void; + delete(kind: DataKind, key: string, version: number, callback: () => void): void; /** * Add an entity or update an existing entity. @@ -101,7 +130,7 @@ export interface LDFeatureStore { * @param callback * Will be called after the upsert operation is complete. */ - upsert(kind: object, data: object, callback: () => void): void; + upsert(kind: DataKind, data: LDKeyedFeatureStoreItem, callback: () => void): void; /** * Tests whether the store is initialized. From fe0801ae23f89e877853b3e71ce851d64fca29e5 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 27 May 2022 11:37:29 -0700 Subject: [PATCH 42/46] Add an async facade for using stores. --- .../store/InMemoryFeatureStore.test.ts | 0 .../src/store/AsyncStoreFacade.ts | 71 +++++++++++++++++++ .../src/{ => store}/InMemoryFeatureStore.ts | 14 +++- 3 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 server-sdk-common/__tests__/store/InMemoryFeatureStore.test.ts create mode 100644 server-sdk-common/src/store/AsyncStoreFacade.ts rename server-sdk-common/src/{ => store}/InMemoryFeatureStore.ts (89%) diff --git a/server-sdk-common/__tests__/store/InMemoryFeatureStore.test.ts b/server-sdk-common/__tests__/store/InMemoryFeatureStore.test.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/server-sdk-common/src/store/AsyncStoreFacade.ts b/server-sdk-common/src/store/AsyncStoreFacade.ts new file mode 100644 index 0000000000..0d7ea0181e --- /dev/null +++ b/server-sdk-common/src/store/AsyncStoreFacade.ts @@ -0,0 +1,71 @@ +import { DataKind } from '../api/interfaces'; +import { + LDFeatureStore, LDFeatureStoreItem, LDFeatureStoreKindData, LDKeyedFeatureStoreItem, +} from '../api/subsystems'; + +/** + * A basic wrapper to make async methods with callbacks into promises. + * + * @param method + * @returns A promisified version of the method. + */ +function promisify(method: (callback: (val: T) => void)=> void): Promise { + return new Promise((resolve) => { + method((val: T) => { resolve(val); }); + }); +} + +/** + * Provides an async interface to a feature store. + * + * This allows for using a store using async/awat instead of callbacks. + * + * @internal + */ +export default class AsyncStoreFacade { + private store: LDFeatureStore; + + constructor(store: LDFeatureStore) { + this.store = store; + } + + async get(kind: DataKind, key: string): Promise { + return promisify((cb) => { + this.store.get(kind, key, cb); + }); + } + + async all(kind: DataKind): Promise { + return promisify((cb) => { + this.store.all(kind, cb); + }); + } + + async init(allData: DataKind): Promise { + return promisify((cb) => { + this.store.init(allData, cb); + }); + } + + async delete(kind: DataKind, key: string, version: number): Promise { + return promisify((cb) => { + this.store.delete(kind, key, version, cb); + }); + } + + async upsert(kind: DataKind, data: LDKeyedFeatureStoreItem): Promise { + return promisify((cb) => { + this.store.upsert(kind, data, cb); + }); + } + + async initialized(): Promise { + return promisify((cb) => { + this.store.initialized(cb); + }); + } + + close(): void { + this.store.close(); + } +} diff --git a/server-sdk-common/src/InMemoryFeatureStore.ts b/server-sdk-common/src/store/InMemoryFeatureStore.ts similarity index 89% rename from server-sdk-common/src/InMemoryFeatureStore.ts rename to server-sdk-common/src/store/InMemoryFeatureStore.ts index 9ae61277b2..92ec6c89dc 100644 --- a/server-sdk-common/src/InMemoryFeatureStore.ts +++ b/server-sdk-common/src/store/InMemoryFeatureStore.ts @@ -1,5 +1,11 @@ -import { DataKind } from './api/interfaces'; -import { LDFeatureStoreDataStorage, LDKeyedFeatureStoreItem, LDFeatureStore, LDFeatureStoreKindData, LDFeatureStoreItem } from './api/subsystems'; +import { DataKind } from '../api/interfaces'; +import { + LDFeatureStoreDataStorage, + LDKeyedFeatureStoreItem, + LDFeatureStore, + LDFeatureStoreKindData, + LDFeatureStoreItem, +} from '../api/subsystems'; /** * Clone an object using JSON. This will not preserve @@ -13,6 +19,7 @@ function clone(obj: any): any { export default class InMemoryFeatureStore implements LDFeatureStore { allData: LDFeatureStoreDataStorage = {}; + initCalled = false; private addItem(kind: DataKind, key: string, item: LDFeatureStoreItem) { @@ -77,7 +84,8 @@ export default class InMemoryFeatureStore implements LDFeatureStore { return callback?.(this.initCalled); } + /* eslint-disable class-methods-use-this */ close(): void { // For the memory store this is a no-op. } -} \ No newline at end of file +} From 77658567914c56d68acb78fdd0bf7da275450b0d Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 27 May 2022 12:54:14 -0700 Subject: [PATCH 43/46] Add unit tests. --- .../store/InMemoryFeatureStore.test.ts | 150 ++++++++++++++++++ .../src/api/subsystems/LDFeatureStore.ts | 2 +- .../src/store/AsyncStoreFacade.ts | 10 +- .../src/store/InMemoryFeatureStore.ts | 2 +- .../src/store/VersionedDataKinds.ts | 24 +++ 5 files changed, 183 insertions(+), 5 deletions(-) create mode 100644 server-sdk-common/src/store/VersionedDataKinds.ts diff --git a/server-sdk-common/__tests__/store/InMemoryFeatureStore.test.ts b/server-sdk-common/__tests__/store/InMemoryFeatureStore.test.ts index e69de29bb2..648916013f 100644 --- a/server-sdk-common/__tests__/store/InMemoryFeatureStore.test.ts +++ b/server-sdk-common/__tests__/store/InMemoryFeatureStore.test.ts @@ -0,0 +1,150 @@ +import AsyncStoreFacade from '../../src/store/AsyncStoreFacade'; +import InMemoryFeatureStore from '../../src/store/InMemoryFeatureStore'; +import VersionedDataKinds from '../../src/store/VersionedDataKinds'; + +// To simplify testing the memory store will be wrapped with the async facade. +// Writing the tests with callbacks would make them much more difficult to follow. + +describe('given an empty feature store and async facade', () => { + const store = new AsyncStoreFacade(new InMemoryFeatureStore()); + + it('can be initialized', async () => { + await store.init({}); + const initialized = await store.initialized(); + expect(initialized).toBeTruthy(); + }); +}); + +describe('given an initialized feature store', () => { + let featureStore: AsyncStoreFacade; + const feature1 = { key: 'foo', version: 10 }; + const feature2 = { key: 'bar', version: 10 }; + + beforeEach(async () => { + featureStore = new AsyncStoreFacade(new InMemoryFeatureStore()); + return featureStore.init({ + [VersionedDataKinds.Features.namespace]: { + foo: feature1, + bar: feature2, + }, + [VersionedDataKinds.Segments.namespace]: {}, + }); + }); + + it('gets an existing feature', async () => { + const feature = await featureStore.get(VersionedDataKinds.Features, 'foo'); + expect(feature).toStrictEqual(feature1); + }); + + it('gets null for a feature that does not exist', async () => { + const feature = await featureStore.get(VersionedDataKinds.Features, 'unknown'); + expect(feature).toBeNull(); + }); + + it('gets all features', async () => { + const features = await featureStore.all(VersionedDataKinds.Features); + expect(features).toStrictEqual({ + foo: feature1, + bar: feature2, + }); + }); + + it('does not upsert an older version', async () => { + await featureStore.upsert(VersionedDataKinds.Features, { + ...feature1, + version: feature1.version - 1, + }); + const feature = await featureStore.get(VersionedDataKinds.Features, 'foo'); + expect(feature).toEqual(feature1); + }); + + it('does upsert a newer version', async () => { + const updatedFeature = { + ...feature1, + version: feature1.version + 1, + }; + await featureStore.upsert(VersionedDataKinds.Features, updatedFeature); + const feature = await featureStore.get(VersionedDataKinds.Features, 'foo'); + expect(feature).toEqual(updatedFeature); + }); + + it('does upsert a new feature', async () => { + const newFeature = { + key: 'new-feature', + version: feature1.version + 1, + }; + await featureStore.upsert(VersionedDataKinds.Features, newFeature); + const feature = await featureStore.get(VersionedDataKinds.Features, newFeature.key); + expect(feature).toEqual(newFeature); + }); + + it('handles race conditions in upserts', async () => { + const ver1 = { key: feature1.key, version: feature1.version + 1 }; + const ver2 = { key: feature1.key, version: feature1.version + 2 }; + + // Intentionally not awaiting these. + const p1 = featureStore.upsert(VersionedDataKinds.Features, ver1); + const p2 = featureStore.upsert(VersionedDataKinds.Features, ver2); + + // Let them both finish. + await Promise.all([p2, p1]); + + const feature = await featureStore.get(VersionedDataKinds.Features, feature1.key); + expect(feature).toEqual(ver2); + }); + + it('deletes with newer version', async () => { + featureStore.delete(VersionedDataKinds.Features, feature1.key, feature1.version + 1); + const feature = await featureStore.get(VersionedDataKinds.Features, feature1.key); + expect(feature).toBeNull(); + }); + + it('does not delete with older version', async () => { + featureStore.delete(VersionedDataKinds.Features, feature1.key, feature1.version - 1); + const feature = await featureStore.get(VersionedDataKinds.Features, feature1.key); + expect(feature).toStrictEqual(feature1); + }); + + it('allows deleting an unknown feature', async () => { + featureStore.delete(VersionedDataKinds.Features, 'unknown', 10); + const feature = await featureStore.get(VersionedDataKinds.Features, 'unknown'); + expect(feature).toBeNull(); + }); + + it('does not upsert older version after delete', async () => { + const key = 'featureKey'; + featureStore.delete(VersionedDataKinds.Features, key, 10); + + featureStore.upsert(VersionedDataKinds.Features, { + key, + version: 9, + }); + const feature = await featureStore.get(VersionedDataKinds.Features, key); + expect(feature).toBeNull(); + }); + + it('does upsert newer version after delete', async () => { + const key = 'featureKey'; + featureStore.delete(VersionedDataKinds.Features, key, 10); + + featureStore.upsert(VersionedDataKinds.Features, { + key, + version: 11, + }); + const feature = await featureStore.get(VersionedDataKinds.Features, key); + expect(feature).toStrictEqual({ + key, + version: 11, + }); + }); + + it('does upsert a new item of unknown kind', async () => { + const newPotato = { + key: 'new-feature', + version: 1, + }; + await featureStore.upsert({namespace: 'potato'}, newPotato); + const feature = await featureStore.get({namespace: 'potato'}, newPotato.key); + expect(feature).toEqual(newPotato); + }); +}); diff --git a/server-sdk-common/src/api/subsystems/LDFeatureStore.ts b/server-sdk-common/src/api/subsystems/LDFeatureStore.ts index acb69852b5..657230fd15 100644 --- a/server-sdk-common/src/api/subsystems/LDFeatureStore.ts +++ b/server-sdk-common/src/api/subsystems/LDFeatureStore.ts @@ -88,7 +88,7 @@ export interface LDFeatureStore { * @param callback * Will be called when the store has been initialized. */ - init(allData: DataKind, callback: () => void): void; + init(allData: LDFeatureStoreDataStorage, callback: () => void): void; /** * Delete an entity from the store. diff --git a/server-sdk-common/src/store/AsyncStoreFacade.ts b/server-sdk-common/src/store/AsyncStoreFacade.ts index 0d7ea0181e..e99652de8c 100644 --- a/server-sdk-common/src/store/AsyncStoreFacade.ts +++ b/server-sdk-common/src/store/AsyncStoreFacade.ts @@ -1,6 +1,10 @@ import { DataKind } from '../api/interfaces'; import { - LDFeatureStore, LDFeatureStoreItem, LDFeatureStoreKindData, LDKeyedFeatureStoreItem, + LDFeatureStore, + LDFeatureStoreDataStorage, + LDFeatureStoreItem, + LDFeatureStoreKindData, + LDKeyedFeatureStoreItem, } from '../api/subsystems'; /** @@ -9,7 +13,7 @@ import { * @param method * @returns A promisified version of the method. */ -function promisify(method: (callback: (val: T) => void)=> void): Promise { +function promisify(method: (callback: (val: T) => void) => void): Promise { return new Promise((resolve) => { method((val: T) => { resolve(val); }); }); @@ -41,7 +45,7 @@ export default class AsyncStoreFacade { }); } - async init(allData: DataKind): Promise { + async init(allData: LDFeatureStoreDataStorage): Promise { return promisify((cb) => { this.store.init(allData, cb); }); diff --git a/server-sdk-common/src/store/InMemoryFeatureStore.ts b/server-sdk-common/src/store/InMemoryFeatureStore.ts index 92ec6c89dc..98de8d278d 100644 --- a/server-sdk-common/src/store/InMemoryFeatureStore.ts +++ b/server-sdk-common/src/store/InMemoryFeatureStore.ts @@ -62,7 +62,7 @@ export default class InMemoryFeatureStore implements LDFeatureStore { callback?.(result); } - init(allData: object, callback: () => void): void { + init(allData: LDFeatureStoreDataStorage, callback: () => void): void { this.initCalled = true; this.allData = allData as LDFeatureStoreDataStorage; callback?.(); diff --git a/server-sdk-common/src/store/VersionedDataKinds.ts b/server-sdk-common/src/store/VersionedDataKinds.ts new file mode 100644 index 0000000000..7015a36756 --- /dev/null +++ b/server-sdk-common/src/store/VersionedDataKinds.ts @@ -0,0 +1,24 @@ +import { DataKind } from '../api/interfaces'; + +export interface VersionedDataKind extends DataKind { + namespace: string, + streamApiPath: string, + requestPath: string, + priority: number, +} + +export default class VersionedDataKinds { + static readonly Features: VersionedDataKind = { + namespace: 'features', + streamApiPath: '/flags/', + requestPath: '/sdk/latest-flags/', + priority: 1, + }; + + static readonly Segments: VersionedDataKind = { + namespace: 'segments', + streamApiPath: '/segments/', + requestPath: '/sdk/latest-segments/', + priority: 0, + }; +} From 2612b6da8b16e9ba45858f971b08e730c27cd07d Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 27 May 2022 12:58:35 -0700 Subject: [PATCH 44/46] Make memory store members private. --- server-sdk-common/src/store/InMemoryFeatureStore.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server-sdk-common/src/store/InMemoryFeatureStore.ts b/server-sdk-common/src/store/InMemoryFeatureStore.ts index 98de8d278d..8cd65118b4 100644 --- a/server-sdk-common/src/store/InMemoryFeatureStore.ts +++ b/server-sdk-common/src/store/InMemoryFeatureStore.ts @@ -18,9 +18,9 @@ function clone(obj: any): any { } export default class InMemoryFeatureStore implements LDFeatureStore { - allData: LDFeatureStoreDataStorage = {}; + private allData: LDFeatureStoreDataStorage = {}; - initCalled = false; + private initCalled = false; private addItem(kind: DataKind, key: string, item: LDFeatureStoreItem) { let items = this.allData[kind.namespace]; From 37a03cdca0b240b081734efec82ff0c5a41d1da2 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 1 Jun 2022 09:29:48 -0700 Subject: [PATCH 45/46] Update server-sdk-common/src/store/AsyncStoreFacade.ts Co-authored-by: Matthew M. Keeler --- server-sdk-common/src/store/AsyncStoreFacade.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server-sdk-common/src/store/AsyncStoreFacade.ts b/server-sdk-common/src/store/AsyncStoreFacade.ts index e99652de8c..5e5359859d 100644 --- a/server-sdk-common/src/store/AsyncStoreFacade.ts +++ b/server-sdk-common/src/store/AsyncStoreFacade.ts @@ -20,7 +20,7 @@ function promisify(method: (callback: (val: T) => void) => void): Promise } /** - * Provides an async interface to a feature store. + * Provides an async interface to a feature store. * * This allows for using a store using async/awat instead of callbacks. * From 48282d25e0a140edc08f0ad38c0206302a07df53 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 1 Jun 2022 09:29:54 -0700 Subject: [PATCH 46/46] Update server-sdk-common/src/store/AsyncStoreFacade.ts Co-authored-by: Matthew M. Keeler --- server-sdk-common/src/store/AsyncStoreFacade.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server-sdk-common/src/store/AsyncStoreFacade.ts b/server-sdk-common/src/store/AsyncStoreFacade.ts index 5e5359859d..9982d7d1b0 100644 --- a/server-sdk-common/src/store/AsyncStoreFacade.ts +++ b/server-sdk-common/src/store/AsyncStoreFacade.ts @@ -22,7 +22,7 @@ function promisify(method: (callback: (val: T) => void) => void): Promise /** * Provides an async interface to a feature store. * - * This allows for using a store using async/awat instead of callbacks. + * This allows for using a store using async/await instead of callbacks. * * @internal */