tests and some restructuring/fixes

Author: tanderson-ld

packages/shared/sdk-server/__tests__/options/Configuration.test.ts

@@ -1,5 +1,14 @@
-import { LDOptions } from '../../src';
+import {
+  ClientContext,
+  DataSourceOptions,
+  isStandardOptions,
+  LDFeatureStore,
+  LDOptions,
+  PollingDataSourceOptions,
+  StandardDataSourceOptions,
+} from '../../src';
 import Configuration from '../../src/options/Configuration';
+import InMemoryFeatureStore from '../../src/store/InMemoryFeatureStore';
 import TestLogger, { LogLevel } from '../Logger';
 
 function withLogger(options: LDOptions): LDOptions {
@@ -25,15 +34,16 @@ describe.each([undefined, null, 'potat0', 17, [], {}])('constructed without opti
     expect(config.flushInterval).toBe(5);
     expect(config.logger).toBeUndefined();
     expect(config.offline).toBe(false);
-    expect(config.pollInterval).toBe(30);
+    expect((config.dataSystem.dataSource as StandardDataSourceOptions).pollInterval).toEqual(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.dataSystem.dataSource as StandardDataSourceOptions).streamInitialReconnectDelay,
+    ).toEqual(1);
     expect(config.tags.value).toBeUndefined();
     expect(config.timeout).toEqual(5);
     expect(config.tlsParams).toBeUndefined();
@@ -179,7 +189,9 @@ describe('when setting different options', () => {
   ])('allow setting and validates pollInterval', (value, expected, warnings) => {
     // @ts-ignore
     const config = new Configuration(withLogger({ pollInterval: value }));
-    expect(config.pollInterval).toEqual(expected);
+    expect((config.dataSystem.dataSource as StandardDataSourceOptions).pollInterval).toEqual(
+      expected,
+    );
     expect(logger(config).getCount()).toEqual(warnings);
   });
 
@@ -207,7 +219,7 @@ describe('when setting different options', () => {
   ])('allows setting stream and validates stream', (value, expected, warnings) => {
     // @ts-ignore
     const config = new Configuration(withLogger({ stream: value }));
-    expect(config.stream).toEqual(expected);
+    expect(isStandardOptions(config.dataSystem.dataSource)).toEqual(expected);
     expect(logger(config).getCount()).toEqual(warnings);
   });
 
@@ -409,7 +421,99 @@ describe('when setting different options', () => {
     ]);
   });
 
-  it('drop', () => {
+  it('drops invalid dataystem data source options and replaces with defaults', () => {
+    const config = new Configuration(
+      withLogger({
+        dataSystem: { dataSource: { bogus: 'myBogusOptions' } as unknown as DataSourceOptions },
+      }),
+    );
+    expect(isStandardOptions(config.dataSystem.dataSource)).toEqual(true);
+    logger(config).expectMessages([
+      {
+        level: LogLevel.Warn,
+        matches: /Config option "dataSource" should be of type DataSourceOptions/,
+      },
+    ]);
+  });
 
-  }
+  it('validates the datasystem persitent store is a factory or object', () => {
+    const config1 = new Configuration(
+      withLogger({
+        dataSystem: {
+          persistentStore: () => new InMemoryFeatureStore(),
+        },
+      }),
+    );
+    expect(isStandardOptions(config1.dataSystem.dataSource)).toEqual(true);
+    expect(logger(config1).getCount()).toEqual(0);
+
+    const config2 = new Configuration(
+      withLogger({
+        dataSystem: {
+          persistentStore: 'bogus type' as unknown as LDFeatureStore,
+        },
+      }),
+    );
+    expect(isStandardOptions(config2.dataSystem.dataSource)).toEqual(true);
+    logger(config2).expectMessages([
+      {
+        level: LogLevel.Warn,
+        matches: /Config option "persistentStore" should be of type LDFeatureStore/,
+      },
+    ]);
+  });
+
+  it('it provides reasonable defaults when datasystem is provided, but some options are missing', () => {
+    const config = new Configuration(
+      withLogger({
+        dataSystem: {},
+      }),
+    );
+    expect(isStandardOptions(config.dataSystem.dataSource)).toEqual(true);
+    expect(logger(config).getCount()).toEqual(0);
+  });
+
+  it('it provides reasonable defaults within the dataSystem.dataSource options when they are missing', () => {
+    const config = new Configuration(
+      withLogger({
+        dataSystem: { dataSource: { type: 'standard' } },
+      }),
+    );
+    expect(isStandardOptions(config.dataSystem.dataSource)).toEqual(true);
+    expect(logger(config).getCount()).toEqual(0);
+  });
+
+  it('ignores deprecated top level options when dataSystem.dataSource options are provided', () => {
+    const config = new Configuration(
+      withLogger({
+        pollInterval: 501, // should be ignored
+        streamInitialReconnectDelay: 502, // should be ignored
+        dataSystem: {
+          dataSource: { type: 'standard', pollInterval: 100, streamInitialReconnectDelay: 200 }, // should be used
+        },
+      }),
+    );
+    expect(isStandardOptions(config.dataSystem.dataSource)).toEqual(true);
+    expect((config.dataSystem.dataSource as StandardDataSourceOptions).pollInterval).toEqual(100);
+    expect(
+      (config.dataSystem.dataSource as StandardDataSourceOptions).streamInitialReconnectDelay,
+    ).toEqual(200);
+    expect(logger(config).getCount()).toEqual(0);
+  });
+
+  it('ignores top level featureStore in favor of the datasystem persitent store', () => {
+    const shouldNotBeUsed = new InMemoryFeatureStore();
+    const shouldBeUsed = new InMemoryFeatureStore();
+    const config = new Configuration(
+      withLogger({
+        featureStore: shouldNotBeUsed,
+        dataSystem: {
+          persistentStore: shouldBeUsed,
+        },
+      }),
+    );
+    // @ts-ignore
+    const result = config.dataSystem.featureStoreFactory(null);
+    expect(result).toEqual(shouldBeUsed);
+  });
 });

packages/shared/sdk-server/src/LDClientImpl.ts

@@ -171,7 +171,7 @@ export default class LDClientImpl implements LDClient {
     const baseHeaders = defaultHeaders(_sdkKey, _platform.info, config.tags);
 
     const clientContext = new ClientContext(_sdkKey, config, _platform);
-    const featureStore = config.featureStoreFactory(clientContext);
+    const featureStore = config.dataSystem.featureStoreFactory(clientContext);
 
     const dataSourceUpdates = new DataSourceUpdates(featureStore, hasEventListeners, onUpdate);
 
@@ -253,9 +253,9 @@ export default class LDClientImpl implements LDClient {
       );
     };
 
-    if (!(config.offline || config.useLdd)) {
+    if (!(config.offline || config.dataSystem.useLdd)) {
       this._updateProcessor =
-        config.updateProcessorFactory?.(
+        config.dataSystem.updateProcessorFactory?.(
           clientContext,
           dataSourceUpdates,
           () => this._initSuccess(),

packages/shared/sdk-server/src/api/options/LDDataSystemOptions.ts

@@ -1,6 +1,7 @@
 import { LDClientContext, subsystem } from '@launchdarkly/js-sdk-common';
 
 import { LDDataSourceUpdates, LDFeatureStore } from '../subsystems';
+import { PersistentDataStore } from '../interfaces';
 
 /**
  * Configuration options for the Data System that the SDK uses to get and maintain flags and other
@@ -42,6 +43,29 @@ export interface LDDataSystemOptions {
    */
   dataSource?: DataSourceOptions;
 
+  /**
+   * Before data has arrived from LaunchDarkly, the SDK is able to evaluate flags using
+   * data from the persistent store. Once fresh data has arrived from LaunchDarkly, the
+   * SDK will no longer read from the persistent store, although it will keep it up-to-date
+   * for future startups.
+   * 
+   * Some implementations provide the store implementation object itself, while others
+   * provide a factory function that creates the store implementation based on the SDK
+   * configuration; this property accepts either.
+   * 
+   * @param clientContext whose properties may be used to influence creation of the persistent store.
+   */
+  persistentStore?: LDFeatureStore | ((clientContext: LDClientContext) => LDFeatureStore);
+
+  /**
+   * Whether you are using the LaunchDarkly relay proxy in daemon mode.
+   *
+   * In this configuration, the client will not connect to LaunchDarkly to get feature flags,
+   * but will instead get feature state from a database (Redis or another supported feature
+   * store integration) that is populated by the relay. By default, this is false.
+   */
+  useLdd?: boolean;
+
   /**
    * A component that obtains feature flag data and puts it in the feature store. Setting
    * this supercedes {@link LDDataSystemOptions#dataSource}.
@@ -55,12 +79,6 @@ export interface LDDataSystemOptions {
         errorHandler?: (e: Error) => void,
       ) => subsystem.LDStreamProcessor);
 
-  /**
-   * Before data has arrived from LaunchDarkly, the SDK is able to evaluate flags using
-   * data from the persistent store. Once fresh data is available, the SDK will no longer
-   * read from the persistent store, although it will keep it up-to-date for future startups.
-   */
-  persistentStore?: LDFeatureStore | ((clientContext: LDClientContext) => LDFeatureStore);
 }
 
 export type DataSourceOptions =
@@ -125,13 +143,13 @@ export interface PollingDataSourceOptions {
 }
 
 export function isStandardOptions(u: any): u is StandardDataSourceOptions {
-  return u.kind === 'standard';
+  return u.type === 'standard';
 }
 
 export function isStreamingOptions(u: any): u is StreamingDataSourceOptions {
-  return u.kind === 'streaming';
+  return u.type === 'streaming';
 }
 
 export function isPollingOptions(u: any): u is PollingDataSourceOptions {
-  return u.kind === 'polling';
+  return u.type === 'polling';
 }

packages/shared/sdk-server/src/api/options/LDOptions.ts

@@ -61,6 +61,8 @@ export interface LDOptions {
   /**
    * A component that stores feature flags and related data received from LaunchDarkly.
    *
+   * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored.
+   *
    * By default, this is an in-memory data structure. Database integrations are also
    * available, as described in the
    * [SDK features guide](https://docs.launchdarkly.com/sdk/concepts/data-stores).
@@ -69,8 +71,6 @@ export interface LDOptions {
    * provide a factory function that creates the store implementation based on the SDK
    * configuration; this property accepts either.
    *
-   * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored.
-   *
    * @deprecated This is now superceded by {@link LDOptions#dataSystem} using property
    * {@link LDDataSystemOptions#persistentStore}. The scope of the {@link LDFeatureStore}
    * that you provide is being reduced in the next major version to only being responsible
@@ -169,11 +169,11 @@ export interface LDOptions {
   /**
    * Whether streaming mode should be used to receive flag updates.
    *
+   * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored.
+   *
    * This is true by default. If you set it to false, the client will use polling.
    * Streaming should only be disabled on the advice of LaunchDarkly support.
    *
-   * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored.
-   *
    * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem}
    * by specifying {@link LDDataSystemOptions#dataSource} options. To opt out of
    * streaming, you can use the {@link PollingDataSourceOptions}.  Streaming should
@@ -184,13 +184,14 @@ export interface LDOptions {
   /**
    * Sets the initial reconnect delay for the streaming connection, in seconds.
    *
+   * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored.
+   *
    * The streaming service uses a backoff algorithm (with jitter) every time the connection needs
    * to be reestablished. The delay for the first reconnection will start near this value, and then
    * increase exponentially for any subsequent connection failures.
    *
    * The default value is 1.
    *
-   * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored.
    *
    * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem}
    * by specifying {@link LDDataSystemOptions#dataSource} options. Specifying the reconnect
@@ -202,9 +203,14 @@ export interface LDOptions {
   /**
    * Whether you are using the LaunchDarkly relay proxy in daemon mode.
    *
+   * If you specify the {@link LDOptions#dataSystem}, this setting will be ignored.
+   *
    * In this configuration, the client will not connect to LaunchDarkly to get feature flags,
    * but will instead get feature state from a database (Redis or another supported feature
    * store integration) that is populated by the relay. By default, this is false.
+   *
+   * @deprecated This functionality is now controlled by {@link LDOptions#dataSystem}
+   * by specifying {@link LDDataSystemOptions#useLdd}.
    */
   useLdd?: boolean;