From 90cd2f04ce26bef59831a567a2664cf4595ef8da Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Thu, 6 Jul 2017 22:48:39 -0700 Subject: [PATCH 1/9] Add a module to warn about deprecated methods --- lib/deprecate.js | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 lib/deprecate.js diff --git a/lib/deprecate.js b/lib/deprecate.js new file mode 100644 index 000000000..521496cae --- /dev/null +++ b/lib/deprecate.js @@ -0,0 +1,33 @@ +/* + Copyright 2017 Google Inc. All Rights Reserved. + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + + +/** + * Warns users that an old object method is deprecated in favor of a new method. + * @param {Object} obj The object containing the methods. + * @param {string} base The base project/object to identify the method names. + * @param {string} old The method to deprecate + * @param {string} new The new method replacing the deprecated method. + * @return {Function} A wrapper calling the new method and warning about the + * old method's deprecation. + */ +export default (obj, base, oldMethod, newMethod) => { + console.warn(`In ${base}: ` + + `${oldMethod} is deprecated, use ${newMethod} instead`); + + if (obj[oldMethod] && !obj[newMethod]) { + obj[newMethod] = obj[oldMethod]; + } +}; From 6f05e679c72c0584e8c20a1a14e84a3cd103df07 Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Thu, 6 Jul 2017 22:51:04 -0700 Subject: [PATCH 2/9] Update/add callbacks for background sync queue --- .../src/lib/background-sync-queue-plugin.js | 6 ++- .../src/lib/background-sync-queue.js | 11 ++-- .../src/lib/request-manager.js | 21 +++++--- .../src/lib/request-queue.js | 40 ++++++++++---- .../test/browser/request-manager.js | 49 ++++++++++++----- .../test/browser/request-queue.js | 52 ++++++++++++++----- 6 files changed, 132 insertions(+), 47 deletions(-) diff --git a/packages/workbox-background-sync/src/lib/background-sync-queue-plugin.js b/packages/workbox-background-sync/src/lib/background-sync-queue-plugin.js index 32792c4b2..597eed747 100644 --- a/packages/workbox-background-sync/src/lib/background-sync-queue-plugin.js +++ b/packages/workbox-background-sync/src/lib/background-sync-queue-plugin.js @@ -7,13 +7,15 @@ import Queue from './background-sync-queue'; * failed requests. * let bgQueue = new workbox.backgroundSync.QueuePlugin({ * callbacks: { - * onResponse: async(hash, res) => { + * replayDidSucceed: async(hash, res) => { * self.registration.showNotification('Background sync demo', { * body: 'Product has been purchased.', * icon: '/images/shop-icon-384.png', * }); * }, - * onRetryFailure: (hash) => {}, + * replayDidFail: (hash) => {}, + * requestWillEnqueue: (reqData) => {}, + * requestWillDequeue: (reqData) => {}, * }, * }); * diff --git a/packages/workbox-background-sync/src/lib/background-sync-queue.js b/packages/workbox-background-sync/src/lib/background-sync-queue.js index d19ed3396..e527d3c5f 100644 --- a/packages/workbox-background-sync/src/lib/background-sync-queue.js +++ b/packages/workbox-background-sync/src/lib/background-sync-queue.js @@ -35,8 +35,8 @@ class Queue { * @param {Object} [input] * @param {Number} [input.maxRetentionTime = 5 days] Time for which a queued * request will live in the queue(irespective of failed/success of replay). - * @param {Object} [input.callbacks] Callbacks for successfull/ failed - * replay of a request. + * @param {Object} [input.callbacks] Callbacks for successfull/failed + * replay of a request as well as modifying before enqueue/dequeue-ing. * @param {string} [input.queueName] Queue name inside db in which * requests will be queued. * @param {BroadcastChannel=} [input.broadcastChannel] BroadcastChannel @@ -66,9 +66,12 @@ class Queue { queueName, idbQDb: new IDBHelper(this._dbName, 1, 'QueueStore'), broadcastChannel, + callbacks, + }); + this._requestManager = new RequestManager({ + callbacks, + queue: this._queue }); - this._requestManager = new RequestManager({callbacks, - queue: this._queue}); this.cleanupQueue(); } diff --git a/packages/workbox-background-sync/src/lib/request-manager.js b/packages/workbox-background-sync/src/lib/request-manager.js index 0cbc570c9..11ab33af9 100644 --- a/packages/workbox-background-sync/src/lib/request-manager.js +++ b/packages/workbox-background-sync/src/lib/request-manager.js @@ -1,3 +1,4 @@ +import deprecate from '../../../../lib/deprecate'; import {putResponse} from './response-manager'; import {getFetchableRequest} from './queue-utils'; import {tagNamePrefix, replayAllQueuesTag} from './constants'; @@ -18,8 +19,15 @@ class RequestManager { * * @private */ - constructor({callbacks, queue}) { - this._globalCallbacks = callbacks || {}; + constructor({callbacks, queue} = {}) { + callbacks = callbacks || {}; + + // Rename deprecated callbacks. + const base = 'workbox-background-sync.RequestManager.callbacks'; + deprecate(callbacks, base, 'onResponse', 'retryDidSucceed'); + deprecate(callbacks, base, 'onRetryFailure', 'retryDidFail'); + + this._globalCallbacks = callbacks; this._queue = queue; this.attachSyncHandler(); } @@ -70,8 +78,8 @@ class RequestManager { response: response.clone(), idbQDb: this._queue.idbQDb, }); - if (this._globalCallbacks.onResponse) - this._globalCallbacks.onResponse(hash, response); + if (this._globalCallbacks.retryDidSucceed) + this._globalCallbacks.retryDidSucceed(hash, response); } } catch(err) { return Promise.reject(err); @@ -94,8 +102,9 @@ class RequestManager { try { await this.replayRequest(hash); } catch (err) { - if(this._globalCallbacks.onRetryFailure) - this._globalCallbacks.onRetryFailure(hash, err); + if (this._globalCallbacks.retryDidFail) { + this._globalCallbacks.retryDidFail(hash, err); + } failedItems.push(err); } } diff --git a/packages/workbox-background-sync/src/lib/request-queue.js b/packages/workbox-background-sync/src/lib/request-queue.js index a3bd91935..62c66edc6 100644 --- a/packages/workbox-background-sync/src/lib/request-queue.js +++ b/packages/workbox-background-sync/src/lib/request-queue.js @@ -32,12 +32,14 @@ class RequestQueue { queueName = defaultQueueName + '_' + _queueCounter++, idbQDb, broadcastChannel, + callbacks, }) { this._isQueueNameAddedToAllQueue = false; this._queueName = queueName; this._config = config; this._idbQDb = idbQDb; this._broadcastChannel = broadcastChannel; + this._globalCallbacks = callbacks || {}; this._queue = []; this.initQueue(); } @@ -86,6 +88,7 @@ class RequestQueue { * preferably when network comes back * * @param {Request} request request object to be queued by this + * @return {Promise} * * @memberOf Queue * @private @@ -94,17 +97,23 @@ class RequestQueue { isInstance({request}, Request); const hash = `${request.url}!${Date.now()}!${_requestCounter++}`; - const queuableRequest = - await getQueueableRequest({ - request, - config: this._config, - }); + const reqData = await getQueueableRequest({ + request, + config: this._config, + }); + + // Apply the `requestWillEnqueue` callback so plugins can modify the + // request data before it's stored in IndexedDB. + if (this._globalCallbacks.requestWillEnqueue) { + this._globalCallbacks.requestWillEnqueue(reqData); + } + try{ this._queue.push(hash); // add to queue this.saveQueue(); - this._idbQDb.put(hash, queuableRequest); + this._idbQDb.put(hash, reqData); await this.addQueueNameToAllQueues(); // register sync self.registration && @@ -117,7 +126,9 @@ class RequestQueue { id: hash, url: request.url, }); - } catch(e) { + + return hash; + } catch (err) { // broadcast the failure of request added to the queue broadcastMessage({ broadcastChannel: this._broadcastChannel, @@ -125,6 +136,8 @@ class RequestQueue { id: hash, url: request.url, }); + + return err; } } @@ -132,7 +145,7 @@ class RequestQueue { * get the Request from the queue at a particular index * * @param {string} hash hash of the request at the given index - * @return {Request} request object corresponding to given hash + * @return {Promise} request object corresponding to given hash * @memberOf Queue * @private */ @@ -140,8 +153,15 @@ class RequestQueue { isType({hash}, 'string'); if(this._queue.includes(hash)) { - const req = await this._idbQDb.get(hash); - return req; + const reqData = await this._idbQDb.get(hash); + + // Apply the `requestWillDequeue` callback so plugins can modify the + // stored data before it's converted back into a request to be replayed. + if (this._globalCallbacks.requestWillDequeue) { + this._globalCallbacks.requestWillDequeue(reqData); + } + + return reqData; } } diff --git a/packages/workbox-background-sync/test/browser/request-manager.js b/packages/workbox-background-sync/test/browser/request-manager.js index 6a9f10edc..b0cd4075c 100644 --- a/packages/workbox-background-sync/test/browser/request-manager.js +++ b/packages/workbox-background-sync/test/browser/request-manager.js @@ -12,17 +12,11 @@ */ /* eslint-env mocha, browser */ -/* global chai, workbox */ +/* global chai, sinon, workbox */ 'use strict'; describe('request-manager test', () => { - let responseAchieved = 0; - const callbacks = { - onResponse: function() { - responseAchieved ++; - }, - }; - + const callbacks = {}; let queue; let reqManager; @@ -56,13 +50,42 @@ describe('request-manager test', () => { }); it('check replay', async function() { - const backgroundSyncQueue - = new workbox.backgroundSync.test.BackgroundSyncQueue({ - callbacks, - }); + sinon.spy(self, 'fetch'); + + callbacks.retryDidSucceed = sinon.spy(); + callbacks.retryDidFail = sinon.spy(); + + const backgroundSyncQueue = + new workbox.backgroundSync.test.BackgroundSyncQueue({callbacks}); + await backgroundSyncQueue.pushIntoQueue({request: new Request('/__echo/counter')}); await backgroundSyncQueue.pushIntoQueue({request: new Request('/__echo/counter')}); await backgroundSyncQueue._requestManager.replayRequests(); - chai.assert.equal(responseAchieved, 2); + + // Asset retryDidSucceed callback was called with the correct arguments. + chai.assert.equal(callbacks.retryDidSucceed.callCount, 2); + chai.assert(callbacks.retryDidSucceed.alwaysCalledWith( + sinon.match.string, sinon.match.instanceOf(Response))); + + // Assert fetch was called for each replayed request. + chai.assert(self.fetch.calledTwice); + + await backgroundSyncQueue.pushIntoQueue({request: new Request('/__test/404')}); + try { + await backgroundSyncQueue._requestManager.replayRequests(); + } catch (err) { + // Error is expected due to 404 response. + } + + // Asset retryDidFail callback was called with the correct arguments. + chai.assert.equal(callbacks.retryDidSucceed.callCount, 2); + chai.assert.equal(callbacks.retryDidFail.callCount, 1); + chai.assert(callbacks.retryDidFail.alwaysCalledWith( + sinon.match.string, sinon.match.instanceOf(Response))); + + delete callbacks.retryDidSucceed; + delete callbacks.retryDidFail; + + self.fetch.restore(); }); }); diff --git a/packages/workbox-background-sync/test/browser/request-queue.js b/packages/workbox-background-sync/test/browser/request-queue.js index 70b212744..43c1d16c7 100644 --- a/packages/workbox-background-sync/test/browser/request-queue.js +++ b/packages/workbox-background-sync/test/browser/request-queue.js @@ -19,14 +19,16 @@ describe('request-queue tests', () => { const QUEUE_NAME = 'QUEUE_NAME'; const MAX_AGE = 6; + + const callbacks = {}; const idbHelper = new workbox.backgroundSync.test.IdbHelper( 'bgQueueSyncDB', 1, 'QueueStore'); - let queue = - new workbox.backgroundSync.test.RequestQueue({ - idbQDb: idbHelper, - config: {maxAge: MAX_AGE}, - queueName: QUEUE_NAME, - }); + const queue = new workbox.backgroundSync.test.RequestQueue({ + idbQDb: idbHelper, + config: {maxAge: MAX_AGE}, + queueName: QUEUE_NAME, + callbacks, + }); it('queue object should exist', () => { chai.assert.isObject(queue); @@ -45,12 +47,38 @@ describe('request-queue tests', () => { queue._config.maxAge, workbox.backgroundSync.test.Constants.maxAge); }); - it('pushRequest is working', () => { - let queueLength = queue._queue.length; - return queue.push({request: new Request('http://lipsum.com/generate')}) - .then(() => { - chai.assert.equal(queue._queue.length, queueLength + 1); - }); + it('push is working', async () => { + callbacks.requestWillEnqueue = sinon.spy(); + + const queueLength = queue._queue.length; + const hash = await queue.push({ + request: new Request('http://lipsum.com/generate') + }); + + chai.assert.isString(hash); + chai.assert.equal(queue._queue.length, queueLength + 1); + + chai.assert(callbacks.requestWillEnqueue.calledOnce); + chai.assert(callbacks.requestWillEnqueue.calledWith( + sinon.match.has('request'))); + + delete callbacks.requestWillEnqueue; + }); + + it('getRequestFromQueue is working', async () => { + callbacks.requestWillDequeue = sinon.spy(); + + const hash = await queue.push({ + request: new Request('http://lipsum.com/generate') + }); + + const reqData = await queue.getRequestFromQueue({hash}); + + chai.assert.hasAllKeys(reqData, ['request', 'config', 'metadata']); + chai.assert(callbacks.requestWillDequeue.calledOnce); + chai.assert(callbacks.requestWillDequeue.calledWith(reqData)); + + delete callbacks.requestWillDequeue; }); it('default config is correct', () => { From f190b28d97c56b3e13606d9930aad3b4c670b133 Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Thu, 6 Jul 2017 23:03:44 -0700 Subject: [PATCH 3/9] Fix lint errors --- lib/deprecate.js | 12 ++++++------ .../src/lib/background-sync-queue.js | 2 +- .../test/browser/request-queue.js | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/deprecate.js b/lib/deprecate.js index 521496cae..d4ce14aa1 100644 --- a/lib/deprecate.js +++ b/lib/deprecate.js @@ -13,19 +13,19 @@ limitations under the License. */ - /** - * Warns users that an old object method is deprecated in favor of a new method. + * Warns users that an old object method is deprecated in favor of a new method + * and aliases the old name to the new name. * @param {Object} obj The object containing the methods. * @param {string} base The base project/object to identify the method names. - * @param {string} old The method to deprecate - * @param {string} new The new method replacing the deprecated method. - * @return {Function} A wrapper calling the new method and warning about the - * old method's deprecation. + * @param {string} oldMethod The method to deprecate. + * @param {string} newMethod The new method replacing the deprecated method. */ export default (obj, base, oldMethod, newMethod) => { + /* eslint-disable no-console */ console.warn(`In ${base}: ` + `${oldMethod} is deprecated, use ${newMethod} instead`); + /* eslint-enable no-console */ if (obj[oldMethod] && !obj[newMethod]) { obj[newMethod] = obj[oldMethod]; diff --git a/packages/workbox-background-sync/src/lib/background-sync-queue.js b/packages/workbox-background-sync/src/lib/background-sync-queue.js index e527d3c5f..391136ad2 100644 --- a/packages/workbox-background-sync/src/lib/background-sync-queue.js +++ b/packages/workbox-background-sync/src/lib/background-sync-queue.js @@ -70,7 +70,7 @@ class Queue { }); this._requestManager = new RequestManager({ callbacks, - queue: this._queue + queue: this._queue, }); this.cleanupQueue(); diff --git a/packages/workbox-background-sync/test/browser/request-queue.js b/packages/workbox-background-sync/test/browser/request-queue.js index 43c1d16c7..c48d71311 100644 --- a/packages/workbox-background-sync/test/browser/request-queue.js +++ b/packages/workbox-background-sync/test/browser/request-queue.js @@ -12,7 +12,7 @@ */ /* eslint-env mocha, browser */ -/* global chai, workbox */ +/* global chai, sinon, workbox */ 'use strict'; @@ -52,7 +52,7 @@ describe('request-queue tests', () => { const queueLength = queue._queue.length; const hash = await queue.push({ - request: new Request('http://lipsum.com/generate') + request: new Request('http://lipsum.com/generate'), }); chai.assert.isString(hash); @@ -69,7 +69,7 @@ describe('request-queue tests', () => { callbacks.requestWillDequeue = sinon.spy(); const hash = await queue.push({ - request: new Request('http://lipsum.com/generate') + request: new Request('http://lipsum.com/generate'), }); const reqData = await queue.getRequestFromQueue({hash}); From 97395783e9b580079b3fff7df127ee248d05a0d9 Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Thu, 6 Jul 2017 23:15:08 -0700 Subject: [PATCH 4/9] Fix incorrectly named methods --- .../src/lib/request-manager.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/workbox-background-sync/src/lib/request-manager.js b/packages/workbox-background-sync/src/lib/request-manager.js index 11ab33af9..958f1a64c 100644 --- a/packages/workbox-background-sync/src/lib/request-manager.js +++ b/packages/workbox-background-sync/src/lib/request-manager.js @@ -24,8 +24,8 @@ class RequestManager { // Rename deprecated callbacks. const base = 'workbox-background-sync.RequestManager.callbacks'; - deprecate(callbacks, base, 'onResponse', 'retryDidSucceed'); - deprecate(callbacks, base, 'onRetryFailure', 'retryDidFail'); + deprecate(callbacks, base, 'onResponse', 'replayDidSucceed'); + deprecate(callbacks, base, 'onRetryFailure', 'replayDidFail'); this._globalCallbacks = callbacks; this._queue = queue; @@ -78,8 +78,8 @@ class RequestManager { response: response.clone(), idbQDb: this._queue.idbQDb, }); - if (this._globalCallbacks.retryDidSucceed) - this._globalCallbacks.retryDidSucceed(hash, response); + if (this._globalCallbacks.replayDidSucceed) + this._globalCallbacks.replayDidSucceed(hash, response); } } catch(err) { return Promise.reject(err); @@ -102,8 +102,8 @@ class RequestManager { try { await this.replayRequest(hash); } catch (err) { - if (this._globalCallbacks.retryDidFail) { - this._globalCallbacks.retryDidFail(hash, err); + if (this._globalCallbacks.replayDidFail) { + this._globalCallbacks.replayDidFail(hash, err); } failedItems.push(err); } From ac3d74aa04df8ba192a189a4aef3acf7c4ea24e5 Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Thu, 6 Jul 2017 23:22:33 -0700 Subject: [PATCH 5/9] Fix failing test due to rename --- .../test/browser/request-manager.js | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/workbox-background-sync/test/browser/request-manager.js b/packages/workbox-background-sync/test/browser/request-manager.js index b0cd4075c..2d3d4d0fa 100644 --- a/packages/workbox-background-sync/test/browser/request-manager.js +++ b/packages/workbox-background-sync/test/browser/request-manager.js @@ -52,8 +52,8 @@ describe('request-manager test', () => { it('check replay', async function() { sinon.spy(self, 'fetch'); - callbacks.retryDidSucceed = sinon.spy(); - callbacks.retryDidFail = sinon.spy(); + callbacks.replayDidSucceed = sinon.spy(); + callbacks.replayDidFail = sinon.spy(); const backgroundSyncQueue = new workbox.backgroundSync.test.BackgroundSyncQueue({callbacks}); @@ -62,9 +62,9 @@ describe('request-manager test', () => { await backgroundSyncQueue.pushIntoQueue({request: new Request('/__echo/counter')}); await backgroundSyncQueue._requestManager.replayRequests(); - // Asset retryDidSucceed callback was called with the correct arguments. - chai.assert.equal(callbacks.retryDidSucceed.callCount, 2); - chai.assert(callbacks.retryDidSucceed.alwaysCalledWith( + // Asset replayDidSucceed callback was called with the correct arguments. + chai.assert.equal(callbacks.replayDidSucceed.callCount, 2); + chai.assert(callbacks.replayDidSucceed.alwaysCalledWith( sinon.match.string, sinon.match.instanceOf(Response))); // Assert fetch was called for each replayed request. @@ -77,14 +77,14 @@ describe('request-manager test', () => { // Error is expected due to 404 response. } - // Asset retryDidFail callback was called with the correct arguments. - chai.assert.equal(callbacks.retryDidSucceed.callCount, 2); - chai.assert.equal(callbacks.retryDidFail.callCount, 1); - chai.assert(callbacks.retryDidFail.alwaysCalledWith( + // Asset replayDidFail callback was called with the correct arguments. + chai.assert.equal(callbacks.replayDidSucceed.callCount, 2); + chai.assert.equal(callbacks.replayDidFail.callCount, 1); + chai.assert(callbacks.replayDidFail.alwaysCalledWith( sinon.match.string, sinon.match.instanceOf(Response))); - delete callbacks.retryDidSucceed; - delete callbacks.retryDidFail; + delete callbacks.replayDidSucceed; + delete callbacks.replayDidFail; self.fetch.restore(); }); From 1bf91ec22d666d5ce5b42d505e4cc850ea3222d5 Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Fri, 7 Jul 2017 11:50:46 -0700 Subject: [PATCH 6/9] Update deprecation untility function --- lib/deprecate.js | 22 +++++++++---------- .../src/lib/request-manager.js | 6 ++--- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/deprecate.js b/lib/deprecate.js index d4ce14aa1..a07c4ef65 100644 --- a/lib/deprecate.js +++ b/lib/deprecate.js @@ -14,20 +14,20 @@ */ /** - * Warns users that an old object method is deprecated in favor of a new method + * Warns users that an old property is deprecated in favor of a new property * and aliases the old name to the new name. * @param {Object} obj The object containing the methods. - * @param {string} base The base project/object to identify the method names. - * @param {string} oldMethod The method to deprecate. - * @param {string} newMethod The new method replacing the deprecated method. + * @param {string} oldName The method to deprecate. + * @param {string} newName The new method replacing the deprecated method. + * @param {string} ctx The context project/object to identify the method names. */ -export default (obj, base, oldMethod, newMethod) => { - /* eslint-disable no-console */ - console.warn(`In ${base}: ` + - `${oldMethod} is deprecated, use ${newMethod} instead`); - /* eslint-enable no-console */ +export default (obj, oldName, newName, ctx) => { + if (Object.prototype.hasOwnProperty.call(obj, oldName)) { + /* eslint-disable no-console */ + console.warn(`In ${ctx}: ` + + `${oldName} is deprecated, use ${newName} instead`); + /* eslint-enable no-console */ - if (obj[oldMethod] && !obj[newMethod]) { - obj[newMethod] = obj[oldMethod]; + obj[newName] = obj[oldName]; } }; diff --git a/packages/workbox-background-sync/src/lib/request-manager.js b/packages/workbox-background-sync/src/lib/request-manager.js index 958f1a64c..ee4c4bb07 100644 --- a/packages/workbox-background-sync/src/lib/request-manager.js +++ b/packages/workbox-background-sync/src/lib/request-manager.js @@ -23,9 +23,9 @@ class RequestManager { callbacks = callbacks || {}; // Rename deprecated callbacks. - const base = 'workbox-background-sync.RequestManager.callbacks'; - deprecate(callbacks, base, 'onResponse', 'replayDidSucceed'); - deprecate(callbacks, base, 'onRetryFailure', 'replayDidFail'); + const ctx = 'workbox-background-sync.RequestManager.callbacks'; + deprecate(callbacks, 'onResponse', 'replayDidSucceed', ctx); + deprecate(callbacks, 'onRetryFailure', 'replayDidFail', ctx); this._globalCallbacks = callbacks; this._queue = queue; From 509245d2100bf49a2d7f47acad05c477cba03a4f Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Mon, 10 Jul 2017 12:10:45 -0700 Subject: [PATCH 7/9] Update jsdoc comments per review feedback --- .../src/lib/background-sync-queue.js | 72 +++++++++++-------- .../src/lib/request-queue.js | 2 +- 2 files changed, 45 insertions(+), 29 deletions(-) diff --git a/packages/workbox-background-sync/src/lib/background-sync-queue.js b/packages/workbox-background-sync/src/lib/background-sync-queue.js index 391136ad2..879c3708b 100644 --- a/packages/workbox-background-sync/src/lib/background-sync-queue.js +++ b/packages/workbox-background-sync/src/lib/background-sync-queue.js @@ -34,46 +34,62 @@ class Queue { * * @param {Object} [input] * @param {Number} [input.maxRetentionTime = 5 days] Time for which a queued - * request will live in the queue(irespective of failed/success of replay). + * request will live in the queue(irrespective of failed/success of replay). * @param {Object} [input.callbacks] Callbacks for successfull/failed * replay of a request as well as modifying before enqueue/dequeue-ing. + * @param {Fuction} [input.callbacks.replayDidSucceed] + * Invoked with params (hash:string, response:Response) after a request is + * successfully replayed. + * @param {Fuction} [input.callbacks.replayDidFail] + * Invoked with param (hash:string) after a replay attempt has failed. + * @param {Fuction} [input.callbacks.requestWillEnqueue] + * Invoked with param (reqData:Object) before a failed request is saved to + * the queue. Use this to modify the saved data. + * @param {Fuction} [input.callbacks.requestWillDequeue] + * Invoked with param (reqData:Object) before a failed request is retrieved + * from the queue. Use this to modify the data before the request is replayed. * @param {string} [input.queueName] Queue name inside db in which * requests will be queued. * @param {BroadcastChannel=} [input.broadcastChannel] BroadcastChannel * which will be used to publish messages when the request will be queued. */ - constructor({maxRetentionTime = maxAge, callbacks, queueName, - broadcastChannel, dbName = defaultDBName} = {}) { - if(queueName) { - isType({queueName}, 'string'); - } + constructor({ + broadcastChannel, + callbacks, + queueName, + dbName = defaultDBName, + maxRetentionTime = maxAge, + } = {}) { + if(queueName) { + isType({queueName}, 'string'); + } - if(maxRetentionTime) { - isType({maxRetentionTime}, 'number'); - } + if(maxRetentionTime) { + isType({maxRetentionTime}, 'number'); + } - if(broadcastChannel) { - isInstance({broadcastChannel}, BroadcastChannel); - } + if(broadcastChannel) { + isInstance({broadcastChannel}, BroadcastChannel); + } - isType({dbName}, 'string'); + isType({dbName}, 'string'); - this._dbName = dbName; - this._queue = new RequestQueue({ - config: { - maxAge: maxRetentionTime, - }, - queueName, - idbQDb: new IDBHelper(this._dbName, 1, 'QueueStore'), - broadcastChannel, - callbacks, - }); - this._requestManager = new RequestManager({ - callbacks, - queue: this._queue, - }); + this._dbName = dbName; + this._queue = new RequestQueue({ + config: { + maxAge: maxRetentionTime, + }, + queueName, + idbQDb: new IDBHelper(this._dbName, 1, 'QueueStore'), + broadcastChannel, + callbacks, + }); + this._requestManager = new RequestManager({ + callbacks, + queue: this._queue, + }); - this.cleanupQueue(); + this.cleanupQueue(); } /** diff --git a/packages/workbox-background-sync/src/lib/request-queue.js b/packages/workbox-background-sync/src/lib/request-queue.js index 62c66edc6..419c50dce 100644 --- a/packages/workbox-background-sync/src/lib/request-queue.js +++ b/packages/workbox-background-sync/src/lib/request-queue.js @@ -145,7 +145,7 @@ class RequestQueue { * get the Request from the queue at a particular index * * @param {string} hash hash of the request at the given index - * @return {Promise} request object corresponding to given hash + * @return {Promise} request object corresponding to given hash * @memberOf Queue * @private */ From 8af00af93553d40c69c9461495c0af4ef66a19f9 Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Tue, 11 Jul 2017 09:46:03 -0700 Subject: [PATCH 8/9] Use logHelper instead of console.warn --- lib/deprecate.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/deprecate.js b/lib/deprecate.js index a07c4ef65..20368f41c 100644 --- a/lib/deprecate.js +++ b/lib/deprecate.js @@ -13,6 +13,8 @@ limitations under the License. */ +import logHelper from './log-helper'; + /** * Warns users that an old property is deprecated in favor of a new property * and aliases the old name to the new name. @@ -23,10 +25,8 @@ */ export default (obj, oldName, newName, ctx) => { if (Object.prototype.hasOwnProperty.call(obj, oldName)) { - /* eslint-disable no-console */ - console.warn(`In ${ctx}: ` + - `${oldName} is deprecated, use ${newName} instead`); - /* eslint-enable no-console */ + logHelper.warn( + `${oldName} is deprecated; use ${newName} instead`, {Context: ctx}); obj[newName] = obj[oldName]; } From 21f7ad24727fc5886a9e7cf90eb2a72eb0c35acf Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Tue, 11 Jul 2017 12:57:30 -0700 Subject: [PATCH 9/9] Fix post-merge lint errors --- .../workbox-background-sync/src/lib/request-manager.js | 3 ++- packages/workbox-background-sync/src/lib/request-queue.js | 4 ++-- .../test/browser/background-sync-queue-plugin.js | 8 +++++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/workbox-background-sync/src/lib/request-manager.js b/packages/workbox-background-sync/src/lib/request-manager.js index c9e056f89..923d35a73 100644 --- a/packages/workbox-background-sync/src/lib/request-manager.js +++ b/packages/workbox-background-sync/src/lib/request-manager.js @@ -78,8 +78,9 @@ class RequestManager { response: response.clone(), idbQDb: this._queue.idbQDb, }); - if (this._globalCallbacks.replayDidSucceed) + if (this._globalCallbacks.replayDidSucceed) { this._globalCallbacks.replayDidSucceed(hash, response); + } } } catch (err) { return Promise.reject(err); diff --git a/packages/workbox-background-sync/src/lib/request-queue.js b/packages/workbox-background-sync/src/lib/request-queue.js index 5d19bb4f6..6085a4efa 100644 --- a/packages/workbox-background-sync/src/lib/request-queue.js +++ b/packages/workbox-background-sync/src/lib/request-queue.js @@ -108,7 +108,7 @@ class RequestQueue { this._globalCallbacks.requestWillEnqueue(reqData); } - try{ + try { this._queue.push(hash); // add to queue @@ -152,7 +152,7 @@ class RequestQueue { async getRequestFromQueue({hash}) { isType({hash}, 'string'); - if(this._queue.includes(hash)) { + if (this._queue.includes(hash)) { const reqData = await this._idbQDb.get(hash); // Apply the `requestWillDequeue` callback so plugins can modify the diff --git a/packages/workbox-background-sync/test/browser/background-sync-queue-plugin.js b/packages/workbox-background-sync/test/browser/background-sync-queue-plugin.js index 06dad9f11..582ac0a54 100644 --- a/packages/workbox-background-sync/test/browser/background-sync-queue-plugin.js +++ b/packages/workbox-background-sync/test/browser/background-sync-queue-plugin.js @@ -17,12 +17,14 @@ 'use strict'; describe('background-sync-queue-plugin test', () => { - const backgroundSyncQueue - = new workbox.backgroundSync.test.BackgroundSyncQueuePlugin({}); + const backgroundSyncQueue = + new workbox.backgroundSync.test.BackgroundSyncQueuePlugin({}); it('check fetchDid fail proxy', async () => { const currentLen = backgroundSyncQueue._queue.queue.length; - await backgroundSyncQueue.fetchDidFail({request: new Request('http://lipsum.com')}); + await backgroundSyncQueue.fetchDidFail({ + request: new Request('http://lipsum.com'), + }); chai.assert.equal(backgroundSyncQueue._queue.queue.length, currentLen + 1); }); });