now passing emitter to DataSourceStatusManager and fixing related tests

Author: tanderson-ld

packages/shared/sdk-client/__tests__/datasource/DataSourceStatusManager.test.ts

@@ -2,22 +2,23 @@ import { DataSourceErrorKind } from '@launchdarkly/js-sdk-common/dist/internal';
 
 import { DataSourceState } from '../../src/datasource/DataSourceStatus';
 import DataSourceStatusManager from '../../src/datasource/DataSourceStatusManager';
+import LDEmitter from '../../src/LDEmitter';
 
 describe('DataSourceStatusManager', () => {
   test('its first state is closed', async () => {
-    const underTest = new DataSourceStatusManager();
+    const underTest = new DataSourceStatusManager(new LDEmitter());
     expect(underTest.status.state).toEqual(DataSourceState.Closed);
   });
 
   test('it stays at initializing if receives recoverable error', async () => {
-    const underTest = new DataSourceStatusManager();
+    const underTest = new DataSourceStatusManager(new LDEmitter());
     underTest.requestStateUpdate(DataSourceState.Initializing);
     underTest.reportError(DataSourceErrorKind.ErrorResponse, 'womp', 404, true);
     expect(underTest.status.state).toEqual(DataSourceState.Initializing);
   });
 
   test('it moves to closed if receives unrecoverable error', async () => {
-    const underTest = new DataSourceStatusManager();
+    const underTest = new DataSourceStatusManager(new LDEmitter());
     underTest.requestStateUpdate(DataSourceState.Initializing);
     underTest.reportError(DataSourceErrorKind.ErrorResponse, 'womp', 404, false);
     expect(underTest.status.state).toEqual(DataSourceState.Closed);
@@ -26,7 +27,7 @@ describe('DataSourceStatusManager', () => {
   test('it updates last error time with each error, but not stateSince', async () => {
     let time = 0;
     const stamper: () => number = () => time;
-    const underTest = new DataSourceStatusManager(stamper);
+    const underTest = new DataSourceStatusManager(new LDEmitter(), stamper);
     underTest.reportError(DataSourceErrorKind.ErrorResponse, 'womp', 404, true);
     expect(underTest.status.stateSince).toEqual(0);
     expect(underTest.status.lastError?.time).toEqual(0);
@@ -46,7 +47,7 @@ describe('DataSourceStatusManager', () => {
     let time = 0;
     const stamper: () => number = () => time;
 
-    const underTest = new DataSourceStatusManager(stamper);
+    const underTest = new DataSourceStatusManager(new LDEmitter(), stamper);
     expect(underTest.status.state).toEqual(DataSourceState.Closed);
     expect(underTest.status.stateSince).toEqual(0);
 
@@ -62,9 +63,9 @@ describe('DataSourceStatusManager', () => {
   test('it notifies listeners when state changes', async () => {
     let time = 0;
     const stamper: () => number = () => time;
-    const underTest = new DataSourceStatusManager(stamper);
-    const spyListener = jest.fn();
-    underTest.on(spyListener);
+    const emitter = new LDEmitter();
+    const spy = jest.spyOn(emitter, 'emit');
+    const underTest = new DataSourceStatusManager(emitter, stamper);
 
     underTest.requestStateUpdate(DataSourceState.SetOffline);
     time += 1;
@@ -73,33 +74,37 @@ describe('DataSourceStatusManager', () => {
     underTest.reportError(DataSourceErrorKind.ErrorResponse, 'womp', 400, true);
     time += 1;
     underTest.requestStateUpdate(DataSourceState.Closed);
-    expect(spyListener).toHaveBeenCalledTimes(4);
-    expect(spyListener).toHaveBeenNthCalledWith(
+    expect(spy).toHaveBeenCalledTimes(4);
+    expect(spy).toHaveBeenNthCalledWith(
       1,
+      'dataSourceStatus',
       expect.objectContaining({
         state: DataSourceState.SetOffline,
         stateSince: 0,
         lastError: undefined,
       }),
     );
-    expect(spyListener).toHaveBeenNthCalledWith(
+    expect(spy).toHaveBeenNthCalledWith(
       2,
+      'dataSourceStatus',
       expect.objectContaining({
         state: DataSourceState.Interrupted,
         stateSince: 1,
         lastError: expect.anything(),
       }),
     );
-    expect(spyListener).toHaveBeenNthCalledWith(
+    expect(spy).toHaveBeenNthCalledWith(
       3,
+      'dataSourceStatus',
       expect.objectContaining({
         state: DataSourceState.Interrupted,
         stateSince: 1, // still in state interrupted
         lastError: expect.anything(),
       }),
     );
-    expect(spyListener).toHaveBeenNthCalledWith(
+    expect(spy).toHaveBeenNthCalledWith(
       4,
+      'dataSourceStatus',
       expect.objectContaining({
         state: DataSourceState.Closed,
         stateSince: 3,

packages/shared/sdk-client/src/DataManager.ts

@@ -78,7 +78,7 @@ export abstract class BaseDataManager implements DataManager {
     protected readonly diagnosticsManager?: internal.DiagnosticsManager,
   ) {
     this.logger = config.logger;
-    this.dataSourceStatusManager = new DataSourceStatusManager();
+    this.dataSourceStatusManager = new DataSourceStatusManager(emitter);
     this.dataSourceEventHandler = new DataSourceEventHandler(
       flagManager,
       this.dataSourceStatusManager,

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

@@ -115,11 +115,6 @@ export default class LDClientImpl implements LDClient {
       this.emitter.emit('change', ldContext, flagKeys);
     });
 
-    // TODO: update this to get events from DataManager
-    // this.dataSourceStatusManager.on((status: DataSourceStatus) => {
-    //   this.emitter.emit('dataSourceStatus', status);
-    // });
-
     this.dataManager = dataManagerFactory(
       this.flagManager,
       this.config,

packages/shared/sdk-client/src/datasource/DataSourceStatusManager.ts

@@ -17,14 +17,9 @@ export default class DataSourceStatusManager {
   private errorInfo?: DataSourceStatusErrorInfo;
   private timeStamper: () => number;
 
-  // TODO: at the moment the LDEmitter requires an event name internally, would be nice to not need to provide an event name,
-  // but perhaps also not worth refactoring/supporting another style
-  private emitter: LDEmitter;
-
-  constructor(timeStamper: () => number = () => Date.now()) {
+  constructor(private readonly emitter: LDEmitter, timeStamper: () => number = () => Date.now()) {
     this.state = DataSourceState.Closed;
     this.stateSinceMillis = timeStamper();
-    this.emitter = new LDEmitter();
     this.timeStamper = timeStamper;
   }
 
@@ -59,20 +54,6 @@ export default class DataSourceStatusManager {
     }
   }
 
-  /**
-   * @param listener that will be registered to receive updates
-   */
-  on(listener: DataSourceStatusCallback) {
-    this.emitter.on('dataSourceStatus', listener);
-  }
-
-  /**
-   * @param listener that will be unregisted and will no longer receive updates
-   */
-  off(listener: DataSourceStatusCallback) {
-    this.emitter.off('dataSourceStatus', listener);
-  }
-
   /**
    * Requests the manager move to the provided state.  This request may be ignored
    * if the current state cannot transition to the requested state.