diff --git a/packages/datafile-manager/CHANGELOG.md b/packages/datafile-manager/CHANGELOG.md index 35830f8de..21c21274c 100644 --- a/packages/datafile-manager/CHANGELOG.md +++ b/packages/datafile-manager/CHANGELOG.md @@ -9,6 +9,9 @@ Changes that have landed but are not yet released. ### Changed - Changed value for node in engines in package.json from >=4.0.0 to >=6.0.0 +- Updated polling behavior: + - Start update interval timer immediately after request + - When update interval timer fires during request, wait until request completes, then immediately start next request ## [0.2.0] - April 9, 2019 diff --git a/packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts b/packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts index 9b4e6f386..00dc8d468 100644 --- a/packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts +++ b/packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts @@ -221,17 +221,14 @@ describe('httpPollingDatafileManager', () => { }) describe('live updates', () => { - it('passes the update interval to its timeoutFactory setTimeout method', async () => { + it('passes the update interval to its timeoutFactory setTimeout method', () => { manager.queuedResponses.push({ statusCode: 200, body: '{"foo3": "bar3"}', headers: {}, }) - const setTimeoutSpy: jest.SpyInstance<() => void, [() => void, number]> = jest.spyOn(testTimeoutFactory, 'setTimeout') - manager.start() - await manager.onReady() expect(setTimeoutSpy).toBeCalledTimes(1) expect(setTimeoutSpy.mock.calls[0][1]).toBe(1000) }) @@ -278,6 +275,36 @@ describe('httpPollingDatafileManager', () => { expect(manager.get()).toEqual({ foo3: 'bar3' }) }) + describe('when the update interval time fires before the request is complete', () => { + it('waits until the request is complete before making the next request', async () => { + let resolveResponsePromise: (resp: Response) => void + const responsePromise: Promise = new Promise(res => { + resolveResponsePromise = res + }) + const makeGetRequestSpy = jest.spyOn(manager, 'makeGetRequest').mockReturnValueOnce({ + abort() {}, + responsePromise, + }) + const setTimeoutSpy = jest.spyOn(testTimeoutFactory, 'setTimeout') + + manager.start() + expect(setTimeoutSpy).toBeCalledTimes(1) + expect(makeGetRequestSpy).toBeCalledTimes(1) + + testTimeoutFactory.timeoutFns[0]() + expect(makeGetRequestSpy).toBeCalledTimes(1) + + resolveResponsePromise!({ + statusCode: 200, + body: '{"foo": "bar"}', + headers: {}, + }) + await responsePromise + expect(makeGetRequestSpy).toBeCalledTimes(2) + expect(setTimeoutSpy).toBeCalledTimes(2) + }) + }) + it('cancels a pending timeout when stop is called', async () => { manager.queuedResponses.push( { diff --git a/packages/datafile-manager/src/httpPollingDatafileManager.ts b/packages/datafile-manager/src/httpPollingDatafileManager.ts index 8ce83b14d..62f4387aa 100644 --- a/packages/datafile-manager/src/httpPollingDatafileManager.ts +++ b/packages/datafile-manager/src/httpPollingDatafileManager.ts @@ -61,7 +61,7 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana private readonly updateInterval: number - private cancelTimeout?: () => void + private cancelTimeout: (() => void) | null private isStarted: boolean @@ -71,10 +71,16 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana private timeoutFactory: TimeoutFactory - private currentRequest?: AbortableRequest + private currentRequest: AbortableRequest | null private backoffController: BackoffController + // When true, this means the update interval timeout fired before the current + // sync completed. In that case, we should sync again immediately upon + // completion of the current request, instead of waiting another update + // interval. + private syncOnCurrentRequestComplete: boolean + constructor(config: DatafileManagerConfig) { const configWithDefaultsApplied: DatafileManagerConfig = { ...this.getConfigDefaults(), @@ -117,7 +123,10 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana logger.warn('Invalid updateInterval %s, defaulting to %s', updateInterval, DEFAULT_UPDATE_INTERVAL) this.updateInterval = DEFAULT_UPDATE_INTERVAL } + this.cancelTimeout = null + this.currentRequest = null this.backoffController = new BackoffController() + this.syncOnCurrentRequestComplete = false } get(): object | null { @@ -138,14 +147,14 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana this.isStarted = false if (this.cancelTimeout) { this.cancelTimeout() - this.cancelTimeout = undefined + this.cancelTimeout = null } this.emitter.removeAllListeners() if (this.currentRequest) { this.currentRequest.abort() - this.currentRequest = undefined + this.currentRequest = null } return Promise.resolve() @@ -209,15 +218,17 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana return } - this.currentRequest = undefined + this.currentRequest = null - if (this.autoUpdate) { - this.scheduleNextUpdate() - } if (!this.isReadyPromiseSettled && !this.autoUpdate) { // We will never resolve ready, so reject it this.rejectReadyPromise(new Error('Failed to become ready')) } + + if (this.autoUpdate && this.syncOnCurrentRequestComplete) { + this.syncDatafile() + } + this.syncOnCurrentRequestComplete = false } private syncDatafile(): void { @@ -241,6 +252,10 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana this.currentRequest.responsePromise .then(onRequestResolved, onRequestRejected) .then(onRequestComplete, onRequestComplete) + + if (this.autoUpdate) { + this.scheduleNextUpdate() + } } private resolveReadyPromise(): void { @@ -258,7 +273,11 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana const nextUpdateDelay = Math.max(currentBackoffDelay, this.updateInterval) logger.debug('Scheduling sync in %s ms', nextUpdateDelay) this.cancelTimeout = this.timeoutFactory.setTimeout(() => { - this.syncDatafile() + if (this.currentRequest) { + this.syncOnCurrentRequestComplete = true + } else { + this.syncDatafile() + } }, nextUpdateDelay) }