Skip to content
Permalink
Browse files

fix(service-worker): cache opaque responses in data groups with `fres…

…hness` strategy (#30977)

Previously, (presummably due to a typo) the `okToCacheOpaque` argument
of `DataGroup#cacheResponse()` was essentially never taken into account
(since opaque responses have a non-200 status code and thus `res.ok` is
always false).

This commit fixes the typo, which allows opaque responses to be cached
when `okToCacheOpaque` is true (i.e. in data groups using the
`freshness` strategy).

Fixes #30968

PR Close #30977
  • Loading branch information...
gkalpak authored and alxhub committed Jun 24, 2019
1 parent 26efc68 commit b0c345324a8c11e034f98ded1b868f52c3a54e1c
Showing with 32 additions and 5 deletions.
  1. +3 −3 packages/service-worker/worker/src/data.ts
  2. +29 −2 packages/service-worker/worker/test/data_spec.ts
@@ -481,10 +481,10 @@ export class DataGroup {
* If the request times out on the server, an error will be returned but the real network
* request will still be running in the background, to be cached when it completes.
*/
private async cacheResponse(
req: Request, res: Response, lru: LruList, okToCacheOpaque: boolean = false): Promise<void> {
private async cacheResponse(req: Request, res: Response, lru: LruList, okToCacheOpaque = false):
Promise<void> {
// Only cache successful responses.
if (!res.ok || (okToCacheOpaque && res.type === 'opaque')) {
if (!(res.ok || (okToCacheOpaque && res.type === 'opaque'))) {
return;
}

@@ -150,6 +150,14 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope';
server.assertNoOtherRequests();
});

it('does not cache opaque responses', async() => {
expect(await makeNoCorsRequest(scope, '/api/test')).toBe('');
server.assertSawRequestFor('/api/test');

expect(await makeNoCorsRequest(scope, '/api/test')).toBe('');
server.assertSawRequestFor('/api/test');
});

it('refreshes after awhile', async() => {
expect(await makeRequest(scope, '/api/test')).toEqual('version 1');
server.clearRequests();
@@ -199,6 +207,16 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope';
serverUpdate.assertNoOtherRequests();
});

it('caches opaque responses', async() => {
expect(await makeNoCorsRequest(scope, '/fresh/data')).toBe('');
server.assertSawRequestFor('/fresh/data');

server.online = false;

expect(await makeRequest(scope, '/fresh/data')).toBe('');
server.assertNoOtherRequests();
});

it('falls back on the cache when server times out', async() => {
expect(await makeRequest(scope, '/fresh/data')).toEqual('this is fresh data');
server.assertSawRequestFor('/fresh/data');
@@ -250,9 +268,18 @@ function makeRequest(scope: SwTestHarness, url: string, clientId?: string): Prom
return done.then(() => resTextPromise);
}

function makeNoCorsRequest(
scope: SwTestHarness, url: string, clientId?: string): Promise<String|null> {
const req = new MockRequest(url, {mode: 'no-cors'});
const [resTextPromise, done] = makePendingRequest(scope, req, clientId);
return done.then(() => resTextPromise);
}

function makePendingRequest(
scope: SwTestHarness, url: string, clientId?: string): [Promise<string|null>, Promise<void>] {
const [resPromise, done] = scope.handleFetch(new MockRequest(url), clientId || 'default');
scope: SwTestHarness, urlOrReq: string | MockRequest,
clientId?: string): [Promise<string|null>, Promise<void>] {
const req = (typeof urlOrReq === 'string') ? new MockRequest(urlOrReq) : urlOrReq;
const [resPromise, done] = scope.handleFetch(req, clientId || 'default');
return [
resPromise.then<string|null>(res => res ? res.text() : null),
done,

0 comments on commit b0c3453

Please sign in to comment.
You can’t perform that action at this time.