fix (datafile manager): Change onReady meaning to 'a datafile is available' (optimizely#14)

mjc1283 · web-flow · commit 4dcb5c415201 · 2019-04-01T20:37:09.000-07:00
Summary:

This changes the behavior of the `onReady` Promise to align with the Java implementation. The `onReady` Promise is now resolved as soon as the datafile manager has any datafile (whether it was passed in or fetched). An update event is always emitted when the datafile changes after `onReady`.

Test plan:

Updated unit tests
diff --git a/packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts b/packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts
@@ -79,24 +79,88 @@ describe('httpPollingDatafileManager', () => {
     jest.restoreAllMocks()
   })
 
-  describe('when constructed with sdkKey and datafile', () => {
+  describe('when constructed with sdkKey and datafile and autoUpdate: true,', () => {
     beforeEach(() => {
-      manager = createTestManager({ datafile: { foo: 'abcd' }, sdkKey: '123' })
+      manager = createTestManager({ datafile: { foo: 'abcd' }, sdkKey: '123', autoUpdate: true })
     })
 
     it('returns the passed datafile from get', () => {
       expect(manager.get()).toEqual({ foo: 'abcd' })
     })
 
-    it('after being started, fetches the datafile and resolves onReady', async () => {
+    it('resolves onReady immediately', async () => {
+      manager.start()
+      await manager.onReady()
+      expect(manager.get()).toEqual({ foo: 'abcd' })
+    })
+
+    it('after being started, fetches the datafile, updates itself, emits an update event, and updates itself again after a timeout', async () => {
+      manager.queuedResponses.push(
+        {
+          statusCode: 200,
+          body: '{"fooz": "barz"}',
+          headers: {}
+        },
+        {
+          statusCode: 200,
+          body: '{"foo": "bar"}',
+          headers: {}
+        }
+      )
+      const updateFn = jest.fn()
+      manager.on('update', updateFn)
+      manager.start()
+      expect(manager.responsePromises.length).toBe(1)
+      await manager.responsePromises[0]
+      expect(updateFn).toBeCalledTimes(1)
+      expect(updateFn).toBeCalledWith({
+        datafile: { foo: 'bar' }
+      })
+      expect(manager.get()).toEqual({ foo: 'bar' })
+      updateFn.mockReset()
+      testTimeoutFactory.timeoutFns[0]()
+      expect(manager.responsePromises.length).toBe(2)
+      await manager.responsePromises[1]
+      expect(updateFn).toBeCalledTimes(1)
+      expect(updateFn).toBeCalledWith({
+        datafile: { fooz: 'barz' }
+      })
+      expect(manager.get()).toEqual({ fooz: 'barz' })
+    })
+  })
+
+  describe('when constructed with sdkKey and datafile and autoUpdate: false,', () => {
+    beforeEach(() => {
+      manager = createTestManager({ datafile: { foo: 'abcd' }, sdkKey: '123', autoUpdate: false })
+    })
+
+    it('returns the passed datafile from get', () => {
+      expect(manager.get()).toEqual({ foo: 'abcd' })
+    })
+
+    it('after being started, resolves onReady immediately', async () => {
+      manager.start()
+      await manager.onReady()
+      expect(manager.get()).toEqual({ foo: 'abcd' })
+    })
+
+    it('after being started, fetches the datafile, updates itself once, and emits an update event, but does not schedule a future update', async () => {
       manager.queuedResponses.push({
         statusCode: 200,
         body: '{"foo": "bar"}',
         headers: {}
       })
+      const updateFn = jest.fn()
+      manager.on('update', updateFn)
       manager.start()
-      await manager.onReady()
+      expect(manager.responsePromises.length).toBe(1)
+      await manager.responsePromises[0]
+      expect(updateFn).toBeCalledTimes(1)
+      expect(updateFn).toBeCalledWith({
+        datafile: { foo: 'bar' }
+      })
       expect(manager.get()).toEqual({ foo: 'bar' })
+      expect(testTimeoutFactory.timeoutFns.length).toBe(0)
     })
   })
 
diff --git a/packages/datafile-manager/src/httpPollingDatafileManager.ts b/packages/datafile-manager/src/httpPollingDatafileManager.ts
@@ -92,12 +92,6 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana
 
     this.sdkKey = sdkKey
 
-    if (typeof datafile !== 'undefined') {
-      this.currentDatafile = datafile
-    } else {
-      this.currentDatafile = null
-    }
-
     this.isReadyPromiseSettled = false
     this.readyPromiseResolver = () => {}
     this.readyPromiseRejecter = () => {}
@@ -106,6 +100,13 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana
       this.readyPromiseRejecter = reject
     })
 
+    if (typeof datafile !== 'undefined') {
+      this.currentDatafile = datafile
+      this.resolveReadyPromise()
+    } else {
+      this.currentDatafile = null
+    }
+
     this.isStarted = false
 
     this.urlTemplate = urlTemplate
@@ -204,7 +205,7 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana
       this.currentDatafile = datafile
       if (!this.isReadyPromiseSettled) {
         this.resolveReadyPromise()
-      } else if (this.autoUpdate) {
+      } else {
         const datafileUpdate: DatafileUpdate = {
           datafile,
         }
