From ec49ecf446c115ad61646a9bc8a5d377126cf202 Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Mon, 8 Apr 2019 11:58:13 -0700 Subject: [PATCH 1/2] Add a getAll() method to the Queue class --- packages/workbox-background-sync/Queue.mjs | 56 ++++++- .../lib/QueueStore.mjs | 36 +++- packages/workbox-expiration/Plugin.mjs | 1 - .../sw/lib/test-QueueStore.mjs | 158 ++++++++++++++++-- .../workbox-background-sync/sw/test-Queue.mjs | 57 +++++++ 5 files changed, 284 insertions(+), 24 deletions(-) diff --git a/packages/workbox-background-sync/Queue.mjs b/packages/workbox-background-sync/Queue.mjs index 92d328863..9278ea0ec 100644 --- a/packages/workbox-background-sync/Queue.mjs +++ b/packages/workbox-background-sync/Queue.mjs @@ -162,6 +162,32 @@ class Queue { return this._removeRequest('shift'); } + /** + * Returns all the entries that have not expired (per `maxRetentionTime`). + * Any expired entries are removed from the queue. + * + * @return {Promise>} + */ + async getAll() { + const allEntries = await this._queueStore.getAll(); + const now = Date.now(); + + const unexpirtedEntries = []; + for (const entry of allEntries) { + // Ignore requests older than maxRetentionTime. Call this function + // recursively until an unexpired request is found. + const maxRetentionTimeInMs = this._maxRetentionTime * 60 * 1000; + if (now - entry.timestamp > maxRetentionTimeInMs) { + await this._queueStore.deleteEntry(entry.id); + } else { + unexpirtedEntries.push(convertEntry(entry)); + } + } + + return unexpirtedEntries; + } + + /** * Adds the entry to the QueueStore and registers for a sync event. * @@ -204,7 +230,7 @@ class Queue { /** * Removes and returns the first or last (depending on `operation`) entry - * form the QueueStore that's not older than the `maxRetentionTime`. + * from the QueueStore that's not older than the `maxRetentionTime`. * * @param {string} operation ('pop' or 'shift') * @return {Object|undefined} @@ -214,7 +240,7 @@ class Queue { const now = Date.now(); const entry = await this._queueStore[`${operation}Entry`](); - if (entry ) { + if (entry) { // Ignore requests older than maxRetentionTime. Call this function // recursively until an unexpired request is found. const maxRetentionTimeInMs = this._maxRetentionTime * 60 * 1000; @@ -222,10 +248,7 @@ class Queue { return this._removeRequest(operation); } - entry.request = new StorableRequest(entry.requestData).toRequest(); - delete entry.requestData; - - return entry; + return convertEntry(entry); } } @@ -347,4 +370,25 @@ class Queue { } } + +/** + * Converts a QueueStore entry into the format exposed by Queue. This entails + * converting the request data into a real request and omitting the `id` and + * `queueName` properties. + * + * @param {Object} queueStoreEntry + * @return {Object} + * @private + */ +const convertEntry = (queueStoreEntry) => { + const queueEntry = { + request: new StorableRequest(queueStoreEntry.requestData).toRequest(), + timestamp: queueStoreEntry.timestamp, + }; + if (queueStoreEntry.metadata) { + queueEntry.metadata = queueStoreEntry.metadata; + } + return queueEntry; +}; + export {Queue}; diff --git a/packages/workbox-background-sync/lib/QueueStore.mjs b/packages/workbox-background-sync/lib/QueueStore.mjs index dc3ca09f8..0c56f48a1 100644 --- a/packages/workbox-background-sync/lib/QueueStore.mjs +++ b/packages/workbox-background-sync/lib/QueueStore.mjs @@ -102,6 +102,7 @@ export class QueueStore { // Pick an ID one less than the lowest ID in the object store. entry.id = firstEntry.id - 1; } else { + // Otherwise let the auto-incrementor assign the ID. delete entry.id; } entry.queueName = this._queueName; @@ -129,6 +130,35 @@ export class QueueStore { return this._removeEntry({direction: 'next'}); } + /** + * Returns all entries in the store matching the `queueName`. + * + * @param {Object} options See workbox.backgroundSync.Queue~getAll} + * @return {Promise>} + * @private + */ + async getAll() { + return await this._db.getAllMatching(OBJECT_STORE_NAME, { + index: INDEXED_PROP, + query: IDBKeyRange.only(this._queueName), + }); + } + + /** + * Deletes the entry for the given ID. + * + * WARNING: this method does not ensure the deleted enry belongs to this + * queue (i.e. matches the `queueName`). But this limitation is acceptable + * as this class is not publicly exposed. An additional check would make + * this method slower than it needs to be. + * + * @private + * @param {number} id + */ + async deleteEntry(id) { + await this._db.delete(OBJECT_STORE_NAME, id); + } + /** * Removes and returns the first or last entry in the queue (based on the * `direction` argument) matching the `queueName`. @@ -145,11 +175,7 @@ export class QueueStore { }); if (entry) { - await this._db.delete(OBJECT_STORE_NAME, entry.id); - - // Dont' expose the ID or queueName; - delete entry.id; - delete entry.queueName; + await this.deleteEntry(entry.id); return entry; } } diff --git a/packages/workbox-expiration/Plugin.mjs b/packages/workbox-expiration/Plugin.mjs index 3e7f8ae3b..3a2407137 100644 --- a/packages/workbox-expiration/Plugin.mjs +++ b/packages/workbox-expiration/Plugin.mjs @@ -31,7 +31,6 @@ import './_version.mjs'; * used immediately. * * When using `maxEntries`, the entry least-recently requested will be removed from the cache first. - * * * @memberof workbox.expiration */ diff --git a/test/workbox-background-sync/sw/lib/test-QueueStore.mjs b/test/workbox-background-sync/sw/lib/test-QueueStore.mjs index ce973afbb..ef1130c3b 100644 --- a/test/workbox-background-sync/sw/lib/test-QueueStore.mjs +++ b/test/workbox-background-sync/sw/lib/test-QueueStore.mjs @@ -419,9 +419,8 @@ describe(`QueueStore`, function() { expect(sr2a.requestData).to.deep.equal(sr2.toObject()); expect(sr2a.timestamp).to.equal(2000); expect(sr2a.metadata).to.deep.equal({name: 'meta2'}); - // It should not return the ID or queue name. - expect(sr2a.id).to.be.undefined; - expect(sr2a.queueName).to.be.undefined; + expect(sr2a.id).to.equal(firstId + 1); + expect(sr2a.queueName).to.equal('b'); entries = await db.getAll('requests'); expect(entries).to.have.lengthOf(4); @@ -434,9 +433,8 @@ describe(`QueueStore`, function() { expect(sr1a.requestData).to.deep.equal(sr1.toObject()); expect(sr1a.timestamp).to.equal(1000); expect(sr1a.metadata).to.deep.equal({name: 'meta1'}); - // It should not return the ID or queue name. - expect(sr1a.id).to.be.undefined; - expect(sr1a.queueName).to.be.undefined; + expect(sr1a.id).to.equal(firstId); + expect(sr1a.queueName).to.equal('a'); entries = await db.getAll('requests'); expect(entries).to.have.lengthOf(3); @@ -494,9 +492,8 @@ describe(`QueueStore`, function() { expect(sr4a.requestData).to.deep.equal(sr4.toObject()); expect(sr4a.timestamp).to.equal(4000); expect(sr4a.metadata).to.deep.equal({name: 'meta4'}); - // It should not return the ID or queue name. - expect(sr4a.id).to.be.undefined; - expect(sr4a.queueName).to.be.undefined; + expect(sr4a.id).to.equal(firstId + 3); + expect(sr4a.queueName).to.equal('b'); entries = await db.getAll('requests'); expect(entries).to.have.lengthOf(4); @@ -517,9 +514,8 @@ describe(`QueueStore`, function() { expect(sr5a.requestData).to.deep.equal(sr5.toObject()); expect(sr5a.timestamp).to.equal(5000); expect(sr5a.metadata).to.deep.equal({name: 'meta5'}); - // It should not return the ID or queue name. - expect(sr5a.id).to.be.undefined; - expect(sr5a.queueName).to.be.undefined; + expect(sr5a.id).to.equal(firstId + 4); + expect(sr5a.queueName).to.equal('a'); entries = await db.getAll('requests'); expect(entries).to.have.lengthOf(3); @@ -528,4 +524,142 @@ describe(`QueueStore`, function() { expect(entries[2].id).to.equal(firstId + 2); }); }); + + describe(`getAll`, function() { + it(`should return all entries in IDB with the right queue name`, async function() { + const queueStore1 = new QueueStore('a'); + const queueStore2 = new QueueStore('b'); + + const sr1 = await StorableRequest.fromRequest(new Request('/one')); + const sr2 = await StorableRequest.fromRequest(new Request('/two')); + const sr3 = await StorableRequest.fromRequest(new Request('/three')); + const sr4 = await StorableRequest.fromRequest(new Request('/four')); + const sr5 = await StorableRequest.fromRequest(new Request('/five')); + + await queueStore1.pushEntry({ + requestData: sr1.toObject(), + timestamp: 1000, + metadata: {name: 'meta1'}, + }); + + let entries = await db.getAll('requests'); + const firstId = entries[0].id; + + await queueStore2.pushEntry({ + requestData: sr2.toObject(), + timestamp: 2000, + metadata: {name: 'meta2'}, + }); + await queueStore2.pushEntry({ + requestData: sr3.toObject(), + timestamp: 3000, + metadata: {name: 'meta3'}, + }); + await queueStore2.pushEntry({ + requestData: sr4.toObject(), + timestamp: 4000, + metadata: {name: 'meta4'}, + }); + await queueStore1.pushEntry({ + requestData: sr5.toObject(), + timestamp: 5000, + metadata: {name: 'meta5'}, + }); + + expect(await queueStore1.getAll()).to.deep.equal([ + { + id: firstId, + queueName: 'a', + requestData: sr1.toObject(), + timestamp: 1000, + metadata: {name: 'meta1'}, + }, + { + id: firstId + 4, + queueName: 'a', + requestData: sr5.toObject(), + timestamp: 5000, + metadata: {name: 'meta5'}, + }, + ]); + + expect(await queueStore2.getAll()).to.deep.equal([ + { + id: firstId + 1, + queueName: 'b', + requestData: sr2.toObject(), + timestamp: 2000, + metadata: {name: 'meta2'}, + }, + { + id: firstId + 2, + queueName: 'b', + requestData: sr3.toObject(), + timestamp: 3000, + metadata: {name: 'meta3'}, + }, + { + id: firstId + 3, + queueName: 'b', + requestData: sr4.toObject(), + timestamp: 4000, + metadata: {name: 'meta4'}, + }, + ]); + + await db.clear('requests'); + + expect(await queueStore1.getAll()).to.deep.equal([]); + expect(await queueStore2.getAll()).to.deep.equal([]); + }); + }); + + describe(`delete`, function() { + it(`should delete an entry for the given ID`, async function() { + const queueStore = new QueueStore('a'); + + const sr1 = await StorableRequest.fromRequest(new Request('/one')); + const sr2 = await StorableRequest.fromRequest(new Request('/two')); + const sr3 = await StorableRequest.fromRequest(new Request('/three')); + + await queueStore.pushEntry({ + requestData: sr1.toObject(), + timestamp: 1000, + metadata: {name: 'meta1'}, + }); + + let entries = await db.getAll('requests'); + const firstId = entries[0].id; + + await queueStore.pushEntry({ + requestData: sr2.toObject(), + timestamp: 2000, + metadata: {name: 'meta2'}, + }); + await queueStore.pushEntry({ + requestData: sr3.toObject(), + timestamp: 3000, + metadata: {name: 'meta3'}, + }); + + await queueStore.deleteEntry(firstId + 1); + + expect(await db.getAll('requests')).to.deep.equal([ + { + id: firstId, + queueName: 'a', + requestData: sr1.toObject(), + timestamp: 1000, + metadata: {name: 'meta1'}, + }, + { + id: firstId + 2, + queueName: 'a', + requestData: sr3.toObject(), + timestamp: 3000, + metadata: {name: 'meta3'}, + }, + ]); + }); + }); }); diff --git a/test/workbox-background-sync/sw/test-Queue.mjs b/test/workbox-background-sync/sw/test-Queue.mjs index 7c09e8ba8..c1d29cf2b 100644 --- a/test/workbox-background-sync/sw/test-Queue.mjs +++ b/test/workbox-background-sync/sw/test-Queue.mjs @@ -700,4 +700,61 @@ describe(`Queue`, function() { await queue.registerSync(); }); }); + + describe(`getAll()`, function() { + it(`returns all requests in the QueueStore instance`, async function() { + const queue = new Queue('a'); + + const request1 = new Request('/one', {method: 'POST', body: '...'}); + const request2 = new Request('/two', {method: 'POST', body: '...'}); + const request3 = new Request('/three', {method: 'POST', body: '...'}); + + await queue.pushRequest({request: request1}); + await queue.pushRequest({request: request2}); + await queue.pushRequest({ + request: request3, + metadata: {meta: 'data'}, + }); + + const entries = await queue.getAll(); + expect(entries.length).to.equal(3); + expect(entries[0].request).to.deep.equal(request1); + expect(entries[0].metadata).to.equal(undefined); + expect(entries[1].request).to.deep.equal(request2); + expect(entries[1].metadata).to.equal(undefined); + expect(entries[2].request).to.deep.equal(request3); + expect(entries[2].metadata).to.deep.equal({meta: 'data'}); + + // Ensure the entries aren't deleted. + expect(await db.getAll('requests')).to.have.lengthOf(3); + }); + + it(`doesn't return expired entries (and it deletes them)`, async function() { + const queue = new Queue('a'); + + const request1 = new Request('/one', {method: 'POST', body: '...'}); + const request2 = new Request('/two', {method: 'POST', body: '...'}); + const request3 = new Request('/three', {method: 'POST', body: '...'}); + + await queue.pushRequest({request: request1}); + await queue.pushRequest({ + request: request2, + timestamp: Date.now() - 1e12, + }); + await queue.pushRequest({ + request: request3, + metadata: {meta: 'data'}, + }); + + const entries = await queue.getAll(); + expect(entries.length).to.equal(2); + expect(entries[0].request).to.deep.equal(request1); + expect(entries[0].metadata).to.equal(undefined); + expect(entries[1].request).to.deep.equal(request3); + expect(entries[1].metadata).to.deep.equal({meta: 'data'}); + + // Ensure the expired entry was deleted. + expect(await db.getAll('requests')).to.have.lengthOf(2); + }); + }); }); From c625fe5bc39f059928b64b5ab006ce038d6a6f8b Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Tue, 9 Apr 2019 09:35:44 -0700 Subject: [PATCH 2/2] Fix typo --- packages/workbox-background-sync/Queue.mjs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/workbox-background-sync/Queue.mjs b/packages/workbox-background-sync/Queue.mjs index 9278ea0ec..849155396 100644 --- a/packages/workbox-background-sync/Queue.mjs +++ b/packages/workbox-background-sync/Queue.mjs @@ -172,7 +172,7 @@ class Queue { const allEntries = await this._queueStore.getAll(); const now = Date.now(); - const unexpirtedEntries = []; + const unexpiredEntries = []; for (const entry of allEntries) { // Ignore requests older than maxRetentionTime. Call this function // recursively until an unexpired request is found. @@ -180,11 +180,11 @@ class Queue { if (now - entry.timestamp > maxRetentionTimeInMs) { await this._queueStore.deleteEntry(entry.id); } else { - unexpirtedEntries.push(convertEntry(entry)); + unexpiredEntries.push(convertEntry(entry)); } } - return unexpirtedEntries; + return unexpiredEntries; }