Add a size method to the Queue class

[philipwalton]

R: @jeffposnick

Fixes #2011

This PR adds a method to count the number of entries in the queue store for the given queue name.

---

UPDATE: Given #2011 (comment) and the fact that the initial changes in the PR didn't consider that a simple IndexedDB count operation wouldn't account for entries that have expired, I decided to just add a single getAll() method that returns an array of unexpired entries. Users who need a count can access that array's length.

[jeffposnick]

LGTM.

Quick check: is there any benefit of testing a case where you add the same StorableRequest to the queue multiple times, and seeing how .size() deals with that scenario? I'm not sure what the expected behavior is in that scenario, so if it's not an "interesting" test case, feel free to ignore.

[philipwalton]

There's not currently any deduping, so it would report the total number of entries regardless of whether the underlying requests were the same (or not).

[philipwalton]

@jeffposnick I force-pushed the changes we talked about in chat. Let me know if you have any comments on them.

[bligny]

I understand that adding a single getAll() method also allows to get the queue size by taking the array length, but it's a bit resource-consuming just to get the size. It would be much more efficient to have - in addition to getAll() - a separate and dedicated size() method using the count() method of the IDBObjectStore.
My two cents
BTW: a small typo in your commit ("unexpirtedEntries")

[philipwalton]

@bligny thanks for taking a look. Your suggestion is actually how I originally implemented it, but then I realized that a size() method using the count() IDB call would return potentially expired entries. It would be weird/confusing if count return 10, but then you iterated through the queue and only got 8 entries, no?

We could potentially add a size() method that uses a cursor in IDB to count without returning the entire list, but that would still involved reading the data into memory.

The best solution (in my opinion) is unfortunately not available to us at the moment, and that's to use a multi-key index on our IDB object store, so we can have the entries sorted by queueName and timestamp. That would allow us to easily count all entries above a certain point without having to iterate through each. Unfortunately this isn't supported in Edge.

So, given all this context, what do you think is the best option to support your use case?

[workbox-pr-bot]

PR-Bot Size Plugin

Changed File Sizes

File	Before	After	Change	GZipped
packages/workbox-background-sync/build/workbox-background-sync.prod.js	3.46 KB	3.81 KB	+10%	1.58 KB	☠️

New Files

No new files have been added.

All File Sizes

View Table

File	Before	After	Change	GZipped
packages/workbox-background-sync/build/workbox-background-sync.prod.js	3.46 KB	3.81 KB	+10%	1.58 KB	☠️
packages/workbox-broadcast-update/build/workbox-broadcast-update.prod.js	1.89 KB	1.89 KB	0%	944 B
packages/workbox-build/build/_types.js	41 B	41 B	0%	61 B
packages/workbox-build/build/index.js	3.64 KB	3.64 KB	0%	1.36 KB
packages/workbox-cacheable-response/build/workbox-cacheable-response.prod.js	579 B	579 B	0%	344 B
packages/workbox-cli/build/app.js	5.58 KB	5.58 KB	0%	1.98 KB
packages/workbox-cli/build/bin.js	1.16 KB	1.16 KB	0%	580 B
packages/workbox-core/build/workbox-core.prod.js	5.75 KB	5.75 KB	0%	2.41 KB
packages/workbox-expiration/build/workbox-expiration.prod.js	2.89 KB	2.89 KB	0%	1.25 KB
packages/workbox-google-analytics/build/workbox-offline-ga.prod.js	1.89 KB	1.89 KB	0%	898 B
packages/workbox-navigation-preload/build/workbox-navigation-preload.prod.js	652 B	652 B	0%	317 B
packages/workbox-precaching/build/workbox-precaching.prod.js	4.24 KB	4.24 KB	0%	1.70 KB
packages/workbox-range-requests/build/workbox-range-requests.prod.js	1.51 KB	1.51 KB	0%	758 B
packages/workbox-routing/build/workbox-routing.prod.js	3.40 KB	3.40 KB	0%	1.48 KB
packages/workbox-strategies/build/workbox-strategies.prod.js	4.86 KB	4.86 KB	0%	1.19 KB
packages/workbox-streams/build/workbox-streams.prod.js	1.38 KB	1.38 KB	0%	678 B
packages/workbox-sw/build/workbox-sw.js	1.33 KB	1.33 KB	0%	741 B
packages/workbox-webpack-plugin/build/generate-sw.js	5.29 KB	5.29 KB	0%	1.84 KB
packages/workbox-webpack-plugin/build/index.js	349 B	349 B	0%	255 B
packages/workbox-webpack-plugin/build/inject-manifest.js	7.22 KB	7.22 KB	0%	2.48 KB
packages/workbox-window/build/workbox-window.dev.umd.js	30.42 KB	30.42 KB	0%	8.07 KB
packages/workbox-window/build/workbox-window.prod.umd.js	4.54 KB	4.54 KB	0%	1.81 KB

Workbox Aggregate Size Plugin

8.9KB gzip'ed (59% of limit)
22.5KB uncompressed

[philipwalton]

Also cc: @Fitzchev

[bligny]

@philipwalton thanks for your quick reply. I do agree that the size() and getAll() responses should be consistent. There might yet be some room for an intermediate solution, e.g:

async getAll(includeExpiredEntries = true) {
 ...
}

The next question is what would be the best (most logical) default value for the optional includeExpiredEntries parameter. On the one hand, true is nice because consistent with size(), but on the other hand false is probably much more interesting.

I let you take the final decision, I'm already happy with the existing improvement :-)
Thanks again for the follow-up.

PS: Personal opinion: I'm a bit sceptical about the automatic purge (deletion of expired entries) which is done in your getAll(). I find dangerous to perform write operations inside a read-only (eg getX()) method.

[Fitzchev]

@philipwalton thanks a lot, it definitively covers my use case.

Reading @bligny comment and looking at the "automatic purge" of expired items, it makes me think that it could be interesting to know that there are expired items left in the queue. What about having a global parameter automatic_purge = yes/no - if yes, keep the same behavior, if no rise an event workbox_expired_items so that the user can decide what to do with them (rise an alert, purge anyway...)

[philipwalton]

Thanks everyone for the comments. I think for the moment I'm going to keep this PR as is.

Once we can drop Edge 18 support, we can rework the data model to use multi-key indexes and add a standalone size() method that can take advantage of the underlying IndexedDB count method.

Also, once we get to the next major release we can re-evaluate whether or not it makes sense to have entries silently expire (or maybe change the default, but that would be a breaking change). At the moment, if you don't like the silent expire behavior, you can alway set the maxRetentionTime configuration option to Infinity and expire entries manually (i.e. just don't re-fetch them after removing them from the queue).