diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bb572c6e73..1bb286d10cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - `apollo-server`: Support `onHealthCheck` in the `ApolloServer` constructor in the same way as `cors` is supported. This contrasts with the `-express`, `-hapi`, etc. variations which accept this parameter via their `applyMiddleware` methods and will remain as-is. [PR #2672](https://github.com/apollographql/apollo-server/pull/2672) - core: Expose SHA-256 hex hash digest of the Engine API key to plugins, when available, as `engine.apiKeyHash`. [PR# 2685](https://github.com/apollographql/apollo-server/pull/2685) - `apollo-datasource-rest`: If another `Content-type` is already set on the response, don't overwrite it with `application/json`, allowing the user's initial `Content-type` to prevail. [PR #2520](https://github.com/apollographql/apollo-server/issues/2035) +- `apollo-cache-control`: Do not respond with `Cache-control` headers if the HTTP response contains `errors`. [PR #2715](https://github.com/apollographql/apollo-server/pull/2715) ### v2.5.0 diff --git a/packages/apollo-cache-control/src/__tests__/cacheControlExtension.test.ts b/packages/apollo-cache-control/src/__tests__/cacheControlExtension.test.ts index 127071f55e9..29a6e0d195d 100644 --- a/packages/apollo-cache-control/src/__tests__/cacheControlExtension.test.ts +++ b/packages/apollo-cache-control/src/__tests__/cacheControlExtension.test.ts @@ -1,4 +1,6 @@ -import { ResponsePath } from 'graphql'; +import { ResponsePath, GraphQLError } from 'graphql'; +import { GraphQLResponse } from 'graphql-extensions'; +import { Headers } from 'apollo-server-env'; import { CacheControlExtension, CacheScope } from '../'; describe('CacheControlExtension', () => { @@ -8,6 +10,53 @@ describe('CacheControlExtension', () => { cacheControlExtension = new CacheControlExtension(); }); + describe('willSendResponse', () => { + let graphqlResponse: GraphQLResponse; + + beforeEach(() => { + cacheControlExtension.options.calculateHttpHeaders = true; + cacheControlExtension.computeOverallCachePolicy = () => ({ + maxAge: 300, + scope: CacheScope.Public, + }); + graphqlResponse = { + http: { + headers: new Headers(), + }, + data: { test: 'test' }, + }; + }); + + it('sets cache-control header', () => { + cacheControlExtension.willSendResponse && + cacheControlExtension.willSendResponse({ graphqlResponse }); + expect(graphqlResponse.http!.headers.get('Cache-Control')).toBe( + 'max-age=300, public', + ); + }); + + const shouldNotSetCacheControlHeader = () => { + cacheControlExtension.willSendResponse && + cacheControlExtension.willSendResponse({ graphqlResponse }); + expect(graphqlResponse.http!.headers.get('Cache-Control')).toBeNull(); + }; + + it('does not set cache-control header if calculateHttpHeaders is set to false', () => { + cacheControlExtension.options.calculateHttpHeaders = false; + shouldNotSetCacheControlHeader(); + }); + + it('does not set cache-control header if graphqlResponse has errors', () => { + graphqlResponse.errors = [new GraphQLError('Test Error')]; + shouldNotSetCacheControlHeader(); + }); + + it('does not set cache-control header if there is no overall cache policy', () => { + cacheControlExtension.computeOverallCachePolicy = () => undefined; + shouldNotSetCacheControlHeader(); + }); + }); + describe('computeOverallCachePolicy', () => { const responsePath: ResponsePath = { key: 'test', diff --git a/packages/apollo-cache-control/src/index.ts b/packages/apollo-cache-control/src/index.ts index ff719f75d96..bf818adf2aa 100644 --- a/packages/apollo-cache-control/src/index.ts +++ b/packages/apollo-cache-control/src/index.ts @@ -153,17 +153,23 @@ export class CacheControlExtension } public willSendResponse?(o: { graphqlResponse: GraphQLResponse }) { - if (this.options.calculateHttpHeaders && o.graphqlResponse.http) { - const overallCachePolicy = this.computeOverallCachePolicy(); - - if (overallCachePolicy) { - o.graphqlResponse.http.headers.set( - 'Cache-Control', - `max-age=${ - overallCachePolicy.maxAge - }, ${overallCachePolicy.scope.toLowerCase()}`, - ); - } + if ( + !this.options.calculateHttpHeaders || + !o.graphqlResponse.http || + o.graphqlResponse.errors + ) { + return; + } + + const overallCachePolicy = this.computeOverallCachePolicy(); + + if (overallCachePolicy) { + o.graphqlResponse.http.headers.set( + 'Cache-Control', + `max-age=${ + overallCachePolicy.maxAge + }, ${overallCachePolicy.scope.toLowerCase()}`, + ); } }