From 3308b897d4d36a06e8822c76db9b31052e9c8efa Mon Sep 17 00:00:00 2001 From: Martijn Walraven Date: Tue, 17 Jul 2018 21:11:59 -0700 Subject: [PATCH] Make overridden TTL trump all headers --- .../apollo-datasource-rest/src/HTTPCache.ts | 37 +++--- .../src/__tests__/HTTPCache.test.ts | 109 ++++++++++++------ .../src/types/http-cache-semantics/index.d.ts | 1 + 3 files changed, 93 insertions(+), 54 deletions(-) diff --git a/packages/apollo-datasource-rest/src/HTTPCache.ts b/packages/apollo-datasource-rest/src/HTTPCache.ts index 0d57c1a6..c091e6b5 100644 --- a/packages/apollo-datasource-rest/src/HTTPCache.ts +++ b/packages/apollo-datasource-rest/src/HTTPCache.ts @@ -37,13 +37,16 @@ export class HTTPCache { ); } - const { policy: policyRaw, body } = JSON.parse(entry); + const { policy: policyRaw, ttlOverride, body } = JSON.parse(entry); const policy = CachePolicy.fromObject(policyRaw); // Remove url from the policy, because otherwise it would never match a request with a custom cache key policy._url = undefined; - if (policy.satisfiesWithoutRevalidation(policyRequestFrom(request))) { + if ( + (ttlOverride && policy.age() < ttlOverride) || + policy.satisfiesWithoutRevalidation(policyRequestFrom(request)) + ) { const headers = policy.responseHeaders(); return new Response(body, { url: policy._url, @@ -65,13 +68,11 @@ export class HTTPCache { ); return this.storeResponseAndReturnClone( - modified - ? revalidationResponse - : new Response(body, { - url: revalidatedPolicy._url, - status: revalidatedPolicy._status, - headers: revalidatedPolicy.responseHeaders(), - }), + new Response(modified ? await revalidationResponse.text() : body, { + url: revalidatedPolicy._url, + status: revalidatedPolicy._status, + headers: revalidatedPolicy.responseHeaders(), + }), request, revalidatedPolicy, cacheKey, @@ -93,17 +94,12 @@ export class HTTPCache { cacheOptions = cacheOptions(response, request); } - let ttl = cacheOptions && cacheOptions.ttl; + let ttlOverride = cacheOptions && cacheOptions.ttl; - if (ttl) { - policy._rescc = { 'max-age': ttl }; - } + if (!ttlOverride && !policy.storable()) return response; - if (!policy.storable()) return response; - - if (!ttl) { - ttl = Math.round(policy.timeToLive() / 1000); - } + let ttl = ttlOverride || Math.round(policy.timeToLive() / 1000); + if (ttl <= 0) return response; // If a response can be revalidated, we don't want to remove it from the cache right after it expires. // We may be able to use better heuristics here, but for now we'll take the max-age times 2. @@ -111,11 +107,10 @@ export class HTTPCache { ttl *= 2; } - if (ttl <= 0) return response; - const body = await response.text(); const entry = JSON.stringify({ policy: policy.toObject(), + ttlOverride, body, }); @@ -131,7 +126,7 @@ export class HTTPCache { url: response.url, status: response.status, statusText: response.statusText, - headers: policy.responseHeaders(), + headers: response.headers, }); } } diff --git a/packages/apollo-datasource-rest/src/__tests__/HTTPCache.test.ts b/packages/apollo-datasource-rest/src/__tests__/HTTPCache.test.ts index 57c54bca..48ed67c1 100644 --- a/packages/apollo-datasource-rest/src/__tests__/HTTPCache.test.ts +++ b/packages/apollo-datasource-rest/src/__tests__/HTTPCache.test.ts @@ -86,50 +86,93 @@ describe('HTTPCache', () => { expect(response.headers.get('Age')).toEqual('0'); }); - it('allows overriding the TTL', async () => { - fetch.mockJSONResponseOnce( - { name: 'Ada Lovelace' }, - { 'Cache-Control': 'private, no-cache' }, - ); + describe('overriding TTL', () => { + it('returns a cached response when the overridden TTL is not expired', async () => { + fetch.mockJSONResponseOnce( + { name: 'Ada Lovelace' }, + { + 'Cache-Control': 'private, no-cache', + 'Set-Cookie': 'foo', + }, + ); - await httpCache.fetch(new Request('https://api.example.com/people/1'), { - cacheOptions: { - ttl: 30, - }, + await httpCache.fetch(new Request('https://api.example.com/people/1'), { + cacheOptions: { + ttl: 30, + }, + }); + + advanceTimeBy(10000); + + const response = await httpCache.fetch( + new Request('https://api.example.com/people/1'), + ); + + expect(fetch.mock.calls.length).toEqual(1); + expect(await response.json()).toEqual({ name: 'Ada Lovelace' }); + expect(response.headers.get('Age')).toEqual('10'); }); - advanceTimeBy(10000); + it('fetches a fresh response from the origin when the overridden TTL expired', async () => { + fetch.mockJSONResponseOnce( + { name: 'Ada Lovelace' }, + { + 'Cache-Control': 'private, no-cache', + 'Set-Cookie': 'foo', + }, + ); - const response = await httpCache.fetch( - new Request('https://api.example.com/people/1'), - ); + await httpCache.fetch(new Request('https://api.example.com/people/1'), { + cacheOptions: { + ttl: 30, + }, + }); - expect(fetch.mock.calls.length).toEqual(1); - expect(await response.json()).toEqual({ name: 'Ada Lovelace' }); - expect(response.headers.get('Age')).toEqual('10'); - }); + advanceTimeBy(30000); - it('allows overriding the TTL dynamically', async () => { - fetch.mockJSONResponseOnce( - { name: 'Ada Lovelace' }, - { 'Cache-Control': 'private, no-cache' }, - ); + fetch.mockJSONResponseOnce( + { name: 'Alan Turing' }, + { + 'Cache-Control': 'private, no-cache', + 'Set-Cookie': 'foo', + }, + ); - await httpCache.fetch(new Request('https://api.example.com/people/1'), { - cacheOptions: (response: Response, request: Request) => ({ - ttl: 30, - }), + const response = await httpCache.fetch( + new Request('https://api.example.com/people/1'), + ); + + expect(fetch.mock.calls.length).toEqual(2); + + expect(await response.json()).toEqual({ name: 'Alan Turing' }); + expect(response.headers.get('Age')).toEqual('0'); }); - advanceTimeBy(10000); + it('allows overriding the TTL dynamically', async () => { + fetch.mockJSONResponseOnce( + { name: 'Ada Lovelace' }, + { + 'Cache-Control': 'private, no-cache', + 'Set-Cookie': 'foo', + }, + ); - const response = await httpCache.fetch( - new Request('https://api.example.com/people/1'), - ); + await httpCache.fetch(new Request('https://api.example.com/people/1'), { + cacheOptions: (response: Response, request: Request) => ({ + ttl: 30, + }), + }); - expect(fetch.mock.calls.length).toEqual(1); - expect(await response.json()).toEqual({ name: 'Ada Lovelace' }); - expect(response.headers.get('Age')).toEqual('10'); + advanceTimeBy(10000); + + const response = await httpCache.fetch( + new Request('https://api.example.com/people/1'), + ); + + expect(fetch.mock.calls.length).toEqual(1); + expect(await response.json()).toEqual({ name: 'Ada Lovelace' }); + expect(response.headers.get('Age')).toEqual('10'); + }); }); it('allows specifying a custom cache key', async () => { diff --git a/packages/apollo-datasource-rest/src/types/http-cache-semantics/index.d.ts b/packages/apollo-datasource-rest/src/types/http-cache-semantics/index.d.ts index 38883b5e..5e06a6d8 100644 --- a/packages/apollo-datasource-rest/src/types/http-cache-semantics/index.d.ts +++ b/packages/apollo-datasource-rest/src/types/http-cache-semantics/index.d.ts @@ -20,6 +20,7 @@ declare module 'http-cache-semantics' { satisfiesWithoutRevalidation(request: Request): boolean; responseHeaders(): Headers; + age(): number; timeToLive(): number; revalidationHeaders(request: Request): Headers;