From a2d372326f5d3d69e5083996d7ab8ad6ddbde4b8 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Tue, 7 May 2019 16:50:39 -0700 Subject: [PATCH 1/4] Fix datafile manager scheduling to be like Java --- .../httpPollingDatafileManager.spec.ts | 33 ++++++++++++++++--- .../src/httpPollingDatafileManager.ts | 25 +++++++++++--- 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts b/packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts index 9b4e6f386..34e02081f 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,34 @@ describe('httpPollingDatafileManager', () => { expect(manager.get()).toEqual({ foo3: 'bar3' }) }) + it('waits until the request is complete before making the next request when the update interval time fires before the request is complete', 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..9a5e70743 100644 --- a/packages/datafile-manager/src/httpPollingDatafileManager.ts +++ b/packages/datafile-manager/src/httpPollingDatafileManager.ts @@ -75,6 +75,12 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana 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(), @@ -118,6 +124,7 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana this.updateInterval = DEFAULT_UPDATE_INTERVAL } this.backoffController = new BackoffController() + this.syncOnCurrentRequestComplete = false } get(): object | null { @@ -211,13 +218,15 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana this.currentRequest = undefined - 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 +250,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 +271,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) } From 216f6778b867eac9c7d735e001c95fb6b087aa36 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Wed, 8 May 2019 10:59:37 -0700 Subject: [PATCH 2/4] Split up the test description using a describe block to make it more readable --- .../httpPollingDatafileManager.spec.ts | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts b/packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts index 34e02081f..00dc8d468 100644 --- a/packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts +++ b/packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts @@ -275,32 +275,34 @@ describe('httpPollingDatafileManager', () => { expect(manager.get()).toEqual({ foo3: 'bar3' }) }) - it('waits until the request is complete before making the next request when the update interval time fires before the request is complete', 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') + 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) + manager.start() + expect(setTimeoutSpy).toBeCalledTimes(1) + expect(makeGetRequestSpy).toBeCalledTimes(1) - testTimeoutFactory.timeoutFns[0]() - expect(makeGetRequestSpy).toBeCalledTimes(1) + testTimeoutFactory.timeoutFns[0]() + expect(makeGetRequestSpy).toBeCalledTimes(1) - resolveResponsePromise!({ - statusCode: 200, - body: '{"foo": "bar"}', - headers: {}, + resolveResponsePromise!({ + statusCode: 200, + body: '{"foo": "bar"}', + headers: {}, + }) + await responsePromise + expect(makeGetRequestSpy).toBeCalledTimes(2) + expect(setTimeoutSpy).toBeCalledTimes(2) }) - await responsePromise - expect(makeGetRequestSpy).toBeCalledTimes(2) - expect(setTimeoutSpy).toBeCalledTimes(2) }) it('cancels a pending timeout when stop is called', async () => { From a3164859e7c4539751f2793498905455cc77ac80 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Wed, 8 May 2019 11:04:58 -0700 Subject: [PATCH 3/4] Use null instead of undefined when assigning to clear out references --- .../src/httpPollingDatafileManager.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/datafile-manager/src/httpPollingDatafileManager.ts b/packages/datafile-manager/src/httpPollingDatafileManager.ts index 9a5e70743..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,7 +71,7 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana private timeoutFactory: TimeoutFactory - private currentRequest?: AbortableRequest + private currentRequest: AbortableRequest | null private backoffController: BackoffController @@ -123,6 +123,8 @@ 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 } @@ -145,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() @@ -216,7 +218,7 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana return } - this.currentRequest = undefined + this.currentRequest = null if (!this.isReadyPromiseSettled && !this.autoUpdate) { // We will never resolve ready, so reject it From 0c8cb4815bba32ccd90868c90dbf4eaf3211555d Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Wed, 8 May 2019 11:33:57 -0700 Subject: [PATCH 4/4] Update changelog --- packages/datafile-manager/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) 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