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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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 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 23/26] 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 24/26] 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 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 25/26] 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 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 26/26] 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.