diff --git a/packages/workbox-core/_private/cacheWrapper.mjs b/packages/workbox-core/_private/cacheWrapper.mjs index 584d0e7e5..5b699012a 100644 --- a/packages/workbox-core/_private/cacheWrapper.mjs +++ b/packages/workbox-core/_private/cacheWrapper.mjs @@ -161,7 +161,7 @@ const matchWrapper = async ({ /** * This method will call cacheWillUpdate on the available plugins (or use - * response.ok) to determine if the Response is safe and valid to cache. + * status === 200) to determine if the Response is safe and valid to cache. * * @param {Object} options * @param {Request} options.request @@ -204,7 +204,7 @@ const _isResponseSafeToCache = async ({request, response, event, plugins}) => { if (!pluginsUsed) { if (process.env.NODE_ENV !== 'production') { - if (!responseToCache.ok) { + if (!responseToCache.status === 200) { if (responseToCache.status === 0) { logger.warn(`The response for '${request.url}' is an opaque ` + `response. The caching strategy that you're using will not ` + @@ -216,7 +216,7 @@ const _isResponseSafeToCache = async ({request, response, event, plugins}) => { } } } - responseToCache = responseToCache.ok ? responseToCache : null; + responseToCache = responseToCache.status === 200 ? responseToCache : null; } return responseToCache ? responseToCache : null; diff --git a/packages/workbox-strategies/plugins/cacheOkAndOpaquePlugin.mjs b/packages/workbox-strategies/plugins/cacheOkAndOpaquePlugin.mjs index 6719e951c..1a8fa9b3c 100644 --- a/packages/workbox-strategies/plugins/cacheOkAndOpaquePlugin.mjs +++ b/packages/workbox-strategies/plugins/cacheOkAndOpaquePlugin.mjs @@ -10,8 +10,8 @@ import '../_version.mjs'; export default { /** - * Return return a response (i.e. allow caching) if the - * response is ok (i.e. 200) or is opaque. + * Returns a valid response (to allow caching) if the status is 200 (OK) or + * 0 (opaque). * * @param {Object} options * @param {Response} options.response @@ -20,7 +20,7 @@ export default { * @private */ cacheWillUpdate: ({response}) => { - if (response.ok || response.status === 0) { + if (response.status === 200 || response.status === 0) { return response; } return null; diff --git a/test/workbox-core/node/_private/test-cacheWrapper.mjs b/test/workbox-core/node/_private/test-cacheWrapper.mjs index 6163b7497..0411c66a5 100644 --- a/test/workbox-core/node/_private/test-cacheWrapper.mjs +++ b/test/workbox-core/node/_private/test-cacheWrapper.mjs @@ -54,28 +54,29 @@ describe(`workbox-core cacheWrapper`, function() { expect(cacheResponse).to.equal(putResponse); }); - it(`should not cache opaque responses by default`, async function() { - const testCache = await caches.open('TEST-CACHE'); - const cacheOpenStub = sandbox.stub(global.caches, 'open'); - const cachePutStub = sandbox.stub(testCache, 'put'); - cacheOpenStub.callsFake(async (cacheName) => { - return testCache; - }); - const putRequest = new Request('/test/string'); - const putResponse = new Response('Response for /test/string', { - // The mock doesn't allow a status of zero due to a bug - // so mock to 1. - status: 1, - }); - await cacheWrapper.put({ - cacheName: 'TODO-CHANGE-ME', - request: putRequest, - response: putResponse, - }); + // This covers opaque responses (0) and partial content responses (206). + for (const status of [0, 206]) { + it(`should not cache response.status of ${status} by default`, async function() { + const cacheName = 'test-cache'; + const testCache = await caches.open(cacheName); + const cacheOpenStub = sandbox.stub(global.caches, 'open').resolves(testCache); + const cachePutSpy = sandbox.spy(testCache, 'put'); + + const putRequest = new Request('/test/string'); + const putResponse = new Response('', { + status, + }); - expect(cacheOpenStub.callCount).to.equal(0); - expect(cachePutStub.callCount).to.equal(0); - }); + await cacheWrapper.put({ + cacheName, + request: putRequest, + response: putResponse, + }); + + expect(cacheOpenStub.callCount).to.equal(0); + expect(cachePutSpy.callCount).to.equal(0); + }); + } devOnly.it(`should not cache POST responses`, async function() { const testCache = await caches.open('TEST-CACHE'); diff --git a/test/workbox-strategies/node/test-cacheOkAndOpaquePlugin.mjs b/test/workbox-strategies/node/test-cacheOkAndOpaquePlugin.mjs index 239aa9fbc..dc28a55d4 100644 --- a/test/workbox-strategies/node/test-cacheOkAndOpaquePlugin.mjs +++ b/test/workbox-strategies/node/test-cacheOkAndOpaquePlugin.mjs @@ -10,12 +10,14 @@ import {expect} from 'chai'; import plugin from '../../../packages/workbox-strategies/plugins/cacheOkAndOpaquePlugin.mjs'; describe(`[workbox-strategies] cacheOkAndOpaquePlugin`, function() { - it(`should return null if status is not ok and status is not opaque`, function() { - const response = new Response('Hello', { - status: 404, + for (const status of [206, 404]) { + it(`should return null when status is ${status}`, function() { + const response = new Response('Hello', { + status, + }); + expect(plugin.cacheWillUpdate({response})).to.equal(null); }); - expect(plugin.cacheWillUpdate({response})).to.equal(null); - }); + } it(`should return Response if status is opaque`, function() { const response = new Response('Hello', {