From 68cbc938ff709078efc8a1e726300e61222f6b8b Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Mon, 30 Mar 2020 15:02:44 +0300 Subject: [PATCH 1/3] refactor: Introduce "cache-control" plugin and switch req. pipeline to it. Similar to 6009d8a00278e, which migrated the tracing package to the plugin API, this commit introduces a `plugin` (named) export from the `apollo-cache-control` module. It similarly accomplishes that by duplicating the behavior of the `CacheControlExtension` which leveraged the soon-to-be-deprecated `graphql-extensions`. The functionality, again, is intended to be identical in spirit. Since the delta of the commits was otherwise even _more_ confusing, I've left the `CacheControlExtension` present and exported and will remove it in a subsequent commit. Briefly summarizing what the necessary changes were: 1. We no longer use a class instance to house the extension, which was necessitated by the `graphql-extensions` API. This means that uses of `this` have been replaced with function scoped variables by the same name. 2. The logic which actually does the formatting (previously handled by the `format` method in `graphql-extension`, now takes place within the plugin API's `willSendResponse` method. 3. Rather than leaning on plugin-specific behavior from within the request pipeline, the `apollo-cache-control` plugin now makes a number of assertions throughout its various life-cycle methods to ensure that the `overallCachePolicy` is calculated. 4. Switch tests to use the new `pluginTestHarness` method for testing plugins which was introduced in eec87a6c6dead7fd (#3990). --- package-lock.json | 1 + packages/apollo-cache-control/package.json | 3 +- .../__tests__/cacheControlExtension.test.ts | 162 ++++++++++----- .../src/__tests__/collectCacheControlHints.ts | 33 +-- packages/apollo-cache-control/src/index.ts | 188 ++++++++++++++++++ packages/apollo-cache-control/tsconfig.json | 1 + .../apollo-server-core/src/ApolloServer.ts | 60 +++--- .../apollo-server-core/src/requestPipeline.ts | 25 --- .../apollo-server-core/src/runHttpQuery.ts | 4 - 9 files changed, 353 insertions(+), 124 deletions(-) diff --git a/package-lock.json b/package-lock.json index 20a701000e3..519c1acc2ed 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4277,6 +4277,7 @@ "version": "file:packages/apollo-cache-control", "requires": { "apollo-server-env": "file:packages/apollo-server-env", + "apollo-server-plugin-base": "file:packages/apollo-server-plugin-base", "graphql-extensions": "file:packages/graphql-extensions" } }, diff --git a/packages/apollo-cache-control/package.json b/packages/apollo-cache-control/package.json index a14de3e4b15..23b4e1f583a 100644 --- a/packages/apollo-cache-control/package.json +++ b/packages/apollo-cache-control/package.json @@ -12,7 +12,8 @@ }, "dependencies": { "apollo-server-env": "file:../apollo-server-env", - "graphql-extensions": "file:../graphql-extensions" + "graphql-extensions": "file:../graphql-extensions", + "apollo-server-plugin-base": "file:../apollo-server-plugin-base" }, "peerDependencies": { "graphql": "^0.12.0 || ^0.13.0 || ^14.0.0 || ^15.0.0" diff --git a/packages/apollo-cache-control/src/__tests__/cacheControlExtension.test.ts b/packages/apollo-cache-control/src/__tests__/cacheControlExtension.test.ts index 29a6e0d195d..ff4d1fb75eb 100644 --- a/packages/apollo-cache-control/src/__tests__/cacheControlExtension.test.ts +++ b/packages/apollo-cache-control/src/__tests__/cacheControlExtension.test.ts @@ -1,59 +1,110 @@ import { ResponsePath, GraphQLError } from 'graphql'; import { GraphQLResponse } from 'graphql-extensions'; import { Headers } from 'apollo-server-env'; -import { CacheControlExtension, CacheScope } from '../'; +import { + CacheScope, + CacheControlExtensionOptions, + CacheHint, + __testing__, + plugin, +} from '../'; +const { addHint, computeOverallCachePolicy } = __testing__; +import { + GraphQLRequestContextWillSendResponse, +} from 'apollo-server-plugin-base'; +import pluginTestHarness from 'apollo-server-core/dist/utils/pluginTestHarness'; describe('CacheControlExtension', () => { - let cacheControlExtension: CacheControlExtension; - - beforeEach(() => { - cacheControlExtension = new CacheControlExtension(); - }); - describe('willSendResponse', () => { - let graphqlResponse: GraphQLResponse; + function makePluginWithOptions({ + pluginInitializationOptions, + overallCachePolicy, + errors = false, + }: { + pluginInitializationOptions?: CacheControlExtensionOptions; + overallCachePolicy?: Required; + errors?: boolean; + } = Object.create(null)) { + const pluginInstance = plugin(pluginInitializationOptions); + + return pluginTestHarness({ + pluginInstance, + overallCachePolicy, + graphqlRequest: { query: 'does not matter' }, + executor: () => { + const response: GraphQLResponse = { + http: { + headers: new Headers(), + }, + data: { test: 'test' }, + }; + + if (errors) { + response.errors = [new GraphQLError('Test Error')]; + } + + return response; + }, + }); + } - beforeEach(() => { - cacheControlExtension.options.calculateHttpHeaders = true; - cacheControlExtension.computeOverallCachePolicy = () => ({ + describe('HTTP cache-control header', () => { + const overallCachePolicy: Required = { 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', - ); - }); + it('is set when calculateHttpHeaders is set to true', async () => { + const requestContext = await makePluginWithOptions({ + pluginInitializationOptions: { + calculateHttpHeaders: true, + }, + overallCachePolicy, + }); + expect(requestContext.response.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(); - }; + const shouldNotSetCacheControlHeader = ( + requestContext: GraphQLRequestContextWillSendResponse, + ) => { + expect( + requestContext.response.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('is not set when calculateHttpHeaders is set to false', async () => { + const requestContext = await makePluginWithOptions({ + pluginInitializationOptions: { + calculateHttpHeaders: false, + }, + overallCachePolicy, + }); + shouldNotSetCacheControlHeader(requestContext); + }); - it('does not set cache-control header if graphqlResponse has errors', () => { - graphqlResponse.errors = [new GraphQLError('Test Error')]; - shouldNotSetCacheControlHeader(); - }); + it('is not set if response has errors', async () => { + const requestContext = await makePluginWithOptions({ + pluginInitializationOptions: { + calculateHttpHeaders: false, + }, + overallCachePolicy, + errors: true, + }); + shouldNotSetCacheControlHeader(requestContext); + }); - it('does not set cache-control header if there is no overall cache policy', () => { - cacheControlExtension.computeOverallCachePolicy = () => undefined; - shouldNotSetCacheControlHeader(); + it('does not set cache-control header if there is no overall cache policy', async () => { + const requestContext = await makePluginWithOptions({ + pluginInitializationOptions: { + calculateHttpHeaders: false, + }, + overallCachePolicy: undefined, + errors: true, + }); + shouldNotSetCacheControlHeader(requestContext); + }); }); }); @@ -71,46 +122,49 @@ describe('CacheControlExtension', () => { prev: responseSubPath, }; + const hints = new Map(); + afterEach(() => hints.clear()); + it('returns undefined without cache hints', () => { - const cachePolicy = cacheControlExtension.computeOverallCachePolicy(); + const cachePolicy = computeOverallCachePolicy(hints); expect(cachePolicy).toBeUndefined(); }); it('returns lowest max age value', () => { - cacheControlExtension.addHint(responsePath, { maxAge: 10 }); - cacheControlExtension.addHint(responseSubPath, { maxAge: 20 }); + addHint(hints, responsePath, { maxAge: 10 }); + addHint(hints, responseSubPath, { maxAge: 20 }); - const cachePolicy = cacheControlExtension.computeOverallCachePolicy(); + const cachePolicy = computeOverallCachePolicy(hints); expect(cachePolicy).toHaveProperty('maxAge', 10); }); it('returns undefined if any cache hint has a maxAge of 0', () => { - cacheControlExtension.addHint(responsePath, { maxAge: 120 }); - cacheControlExtension.addHint(responseSubPath, { maxAge: 0 }); - cacheControlExtension.addHint(responseSubSubPath, { maxAge: 20 }); + addHint(hints, responsePath, { maxAge: 120 }); + addHint(hints, responseSubPath, { maxAge: 0 }); + addHint(hints, responseSubSubPath, { maxAge: 20 }); - const cachePolicy = cacheControlExtension.computeOverallCachePolicy(); + const cachePolicy = computeOverallCachePolicy(hints); expect(cachePolicy).toBeUndefined(); }); it('returns PUBLIC scope by default', () => { - cacheControlExtension.addHint(responsePath, { maxAge: 10 }); + addHint(hints, responsePath, { maxAge: 10 }); - const cachePolicy = cacheControlExtension.computeOverallCachePolicy(); + const cachePolicy = computeOverallCachePolicy(hints); expect(cachePolicy).toHaveProperty('scope', CacheScope.Public); }); it('returns PRIVATE scope if any cache hint has PRIVATE scope', () => { - cacheControlExtension.addHint(responsePath, { + addHint(hints, responsePath, { maxAge: 10, scope: CacheScope.Public, }); - cacheControlExtension.addHint(responseSubPath, { + addHint(hints, responseSubPath, { maxAge: 10, scope: CacheScope.Private, }); - const cachePolicy = cacheControlExtension.computeOverallCachePolicy(); + const cachePolicy = computeOverallCachePolicy(hints); expect(cachePolicy).toHaveProperty('scope', CacheScope.Private); }); }); diff --git a/packages/apollo-cache-control/src/__tests__/collectCacheControlHints.ts b/packages/apollo-cache-control/src/__tests__/collectCacheControlHints.ts index e69e8bc448e..fdadf7d7ca4 100644 --- a/packages/apollo-cache-control/src/__tests__/collectCacheControlHints.ts +++ b/packages/apollo-cache-control/src/__tests__/collectCacheControlHints.ts @@ -1,38 +1,41 @@ import { GraphQLSchema, graphql } from 'graphql'; - -import { - enableGraphQLExtensions, - GraphQLExtensionStack, -} from 'graphql-extensions'; import { - CacheControlExtension, CacheHint, CacheControlExtensionOptions, + plugin, } from '../'; +import pluginTestHarness from 'apollo-server-core/dist/utils/pluginTestHarness'; export async function collectCacheControlHints( schema: GraphQLSchema, source: string, options?: CacheControlExtensionOptions, ): Promise { - enableGraphQLExtensions(schema); // Because this test helper looks at the formatted extensions, we always want - // to include them. - const cacheControlExtension = new CacheControlExtension({ + // to include them in the response rather than allow them to be stripped + // out. + const pluginInstance = plugin({ ...options, stripFormattedExtensions: false, }); - const response = await graphql({ + const requestContext = await pluginTestHarness({ + pluginInstance, schema, - source, - contextValue: { - _extensionStack: new GraphQLExtensionStack([cacheControlExtension]), + graphqlRequest: { + query: source, }, + executor: async (requestContext) => { + return await graphql({ + schema, + source: requestContext.request.query, + contextValue: requestContext.context, + }); + } }); - expect(response.errors).toBeUndefined(); + expect(requestContext.response.errors).toBeUndefined(); - return cacheControlExtension.format()[1].hints; + return requestContext.response.extensions!.cacheControl.hints; } diff --git a/packages/apollo-cache-control/src/index.ts b/packages/apollo-cache-control/src/index.ts index fa37370f03e..f1a9a944e42 100644 --- a/packages/apollo-cache-control/src/index.ts +++ b/packages/apollo-cache-control/src/index.ts @@ -7,6 +7,7 @@ import { ResponsePath, responsePathAsArray, } from 'graphql'; +import { ApolloServerPlugin } from "apollo-server-plugin-base"; import { GraphQLExtension, GraphQLResponse } from 'graphql-extensions'; @@ -49,6 +50,151 @@ declare module 'apollo-server-types' { } } +type MapResponsePathHints = Map; + +export const plugin = ( + options: CacheControlExtensionOptions = Object.create(null), +): ApolloServerPlugin => ({ + requestDidStart(requestContext) { + const defaultMaxAge: number = options.defaultMaxAge || 0; + const hints: MapResponsePathHints = new Map(); + + + function setOverallCachePolicyWhenUnset() { + if (!requestContext.overallCachePolicy) { + requestContext.overallCachePolicy = computeOverallCachePolicy(hints); + } + } + + return { + willResolveField(...args) { + const [, , , info] = args; + let hint: CacheHint = {}; + + // If this field's resolver returns an object or interface, look for + // hints on that return type. + const targetType = getNamedType(info.returnType); + if ( + targetType instanceof GraphQLObjectType || + targetType instanceof GraphQLInterfaceType + ) { + if (targetType.astNode) { + hint = mergeHints( + hint, + cacheHintFromDirectives(targetType.astNode.directives), + ); + } + } + + // Look for hints on the field itself (on its parent type), taking + // precedence over previously calculated hints. + const fieldDef = info.parentType.getFields()[info.fieldName]; + if (fieldDef.astNode) { + hint = mergeHints( + hint, + cacheHintFromDirectives(fieldDef.astNode.directives), + ); + } + + // If this resolver returns an object or is a root field and we haven't + // seen an explicit maxAge hint, set the maxAge to 0 (uncached) or the + // default if specified in the constructor. (Non-object fields by + // default are assumed to inherit their cacheability from their parents. + // But on the other hand, while root non-object fields can get explicit + // hints from their definition on the Query/Mutation object, if that + // doesn't exist then there's no parent field that would assign the + // default maxAge, so we do it here.) + if ( + (targetType instanceof GraphQLObjectType || + targetType instanceof GraphQLInterfaceType || + !info.path.prev) && + hint.maxAge === undefined + ) { + hint.maxAge = defaultMaxAge; + } + + if (hint.maxAge !== undefined || hint.scope !== undefined) { + addHint(hints, info.path, hint); + } + + info.cacheControl = { + setCacheHint: (hint: CacheHint) => { + addHint(hints, info.path, hint); + }, + cacheHint: hint, + }; + }, + + responseForOperation() { + // We are not supplying an answer, we are only setting the cache + // policy if it's not set! Therefore, we return null. + setOverallCachePolicyWhenUnset(); + return null; + }, + + executionDidStart() { + return () => setOverallCachePolicyWhenUnset(); + }, + + willSendResponse(requestContext) { + const { + response, + overallCachePolicy: overallCachePolicyOverride, + } = requestContext; + + // If there are any errors, we don't consider this cacheable. + if (response.errors) { + return; + } + + // Use the override by default, but if it's not overridden, set our + // own computation onto the `requestContext` for other plugins to read. + const overallCachePolicy = + overallCachePolicyOverride || + (requestContext.overallCachePolicy = + computeOverallCachePolicy(hints)); + + if ( + overallCachePolicy && + options.calculateHttpHeaders && + response.http + ) { + response.http.headers.set( + 'Cache-Control', + `max-age=${ + overallCachePolicy.maxAge + }, ${overallCachePolicy.scope.toLowerCase()}`, + ); + } + + // We should have to explicitly ask to leave the formatted extension in, + // or pass the old-school `cacheControl: true` (as interpreted by + // apollo-server-core/ApolloServer), in order to include the + // engineproxy-aimed extensions. Specifically, we want users of + // apollo-server-plugin-response-cache to be able to specify + // `cacheControl: {defaultMaxAge: 600}` without accidentally turning on + // the extension formatting. + if (options.stripFormattedExtensions !== false) return; + + const extensions = + response.extensions || (response.extensions = Object.create(null)); + + if (typeof extensions.cacheControl !== 'undefined') { + throw new Error("The cacheControl information already existed."); + } + + extensions.cacheControl = { + version: 1, + hints: Array.from(hints).map(([path, hint]) => ({ + path: [...responsePathAsArray(path)], + ...hint, + })), + }; + } + } + } +}); + export class CacheControlExtension implements GraphQLExtension { private defaultMaxAge: number; @@ -255,3 +401,45 @@ function mergeHints( scope: otherHint.scope || hint.scope, }; } + +function computeOverallCachePolicy( + hints: MapResponsePathHints, +): Required | undefined { + let lowestMaxAge: number | undefined = undefined; + let scope: CacheScope = CacheScope.Public; + + for (const hint of hints.values()) { + if (hint.maxAge !== undefined) { + lowestMaxAge = + lowestMaxAge !== undefined + ? Math.min(lowestMaxAge, hint.maxAge) + : hint.maxAge; + } + if (hint.scope === CacheScope.Private) { + scope = CacheScope.Private; + } + } + + // If maxAge is 0, then we consider it uncacheable so it doesn't matter what + // the scope was. + return lowestMaxAge + ? { + maxAge: lowestMaxAge, + scope, + } + : undefined; +} + +function addHint(hints: MapResponsePathHints, path: ResponsePath, hint: CacheHint) { + const existingCacheHint = hints.get(path); + if (existingCacheHint) { + hints.set(path, mergeHints(existingCacheHint, hint)); + } else { + hints.set(path, hint); + } +} + +export const __testing__ = { + addHint, + computeOverallCachePolicy, +}; diff --git a/packages/apollo-cache-control/tsconfig.json b/packages/apollo-cache-control/tsconfig.json index 0de28001c29..1276353ea04 100644 --- a/packages/apollo-cache-control/tsconfig.json +++ b/packages/apollo-cache-control/tsconfig.json @@ -8,5 +8,6 @@ "exclude": ["**/__tests__", "**/__mocks__"], "references": [ { "path": "../graphql-extensions" }, + { "path": "../apollo-server-plugin-base" }, ] } diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index bea27fea464..8bddf49873a 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -70,6 +70,10 @@ import { import { Headers } from 'apollo-server-env'; import { buildServiceDefinition } from '@apollographql/apollo-tools'; import { Logger } from "apollo-server-types"; +import { + plugin as pluginCacheControl, + CacheControlExtensionOptions, +} from 'apollo-cache-control'; const NoIntrospection = (context: ValidationContext) => ({ Field(node: FieldDefinitionNode) { @@ -190,6 +194,7 @@ export class ApolloServerBase { playground, plugins, gateway, + cacheControl, experimental_approximateDocumentStoreMiB, ...requestOptions } = config; @@ -249,31 +254,6 @@ export class ApolloServerBase { : noIntro; } - if (requestOptions.cacheControl !== false) { - if ( - typeof requestOptions.cacheControl === 'boolean' && - requestOptions.cacheControl === true - ) { - // cacheControl: true means that the user needs the cache-control - // extensions. This means we are running the proxy, so we should not - // strip out the cache control extension and not add cache-control headers - requestOptions.cacheControl = { - stripFormattedExtensions: false, - calculateHttpHeaders: false, - defaultMaxAge: 0, - }; - } else { - // Default behavior is to run default header calculation and return - // no cacheControl extensions - requestOptions.cacheControl = { - stripFormattedExtensions: true, - calculateHttpHeaders: true, - defaultMaxAge: 0, - ...requestOptions.cacheControl, - }; - } - } - if (!requestOptions.cache) { requestOptions.cache = new InMemoryLRUCache(); } @@ -790,7 +770,37 @@ export class ApolloServerBase { // at the end of that list so they take precidence. // A follow-up commit will actually introduce this. + // Enable cache control unless it was explicitly disabled. + if (this.config.cacheControl !== false) { + let cacheControlOptions: CacheControlExtensionOptions = {}; + if ( + typeof this.config.cacheControl === 'boolean' && + this.config.cacheControl === true + ) { + // cacheControl: true means that the user needs the cache-control + // extensions. This means we are running the proxy, so we should not + // strip out the cache control extension and not add cache-control headers + cacheControlOptions = { + stripFormattedExtensions: false, + calculateHttpHeaders: false, + defaultMaxAge: 0, + }; + } else { + // Default behavior is to run default header calculation and return + // no cacheControl extensions + cacheControlOptions = { + stripFormattedExtensions: true, + calculateHttpHeaders: true, + defaultMaxAge: 0, + ...this.config.cacheControl, + }; + } + + pluginsToInit.push(pluginCacheControl(cacheControlOptions)); + } + pluginsToInit.push(...plugins); + this.plugins = pluginsToInit.map(plugin => { if (typeof plugin === 'function') { return plugin(); diff --git a/packages/apollo-server-core/src/requestPipeline.ts b/packages/apollo-server-core/src/requestPipeline.ts index 6393d82c95a..4751f44eb8f 100644 --- a/packages/apollo-server-core/src/requestPipeline.ts +++ b/packages/apollo-server-core/src/requestPipeline.ts @@ -22,10 +22,6 @@ import { symbolRequestListenerDispatcher, enablePluginsForSchemaResolvers, } from '.'; -import { - CacheControlExtension, - CacheControlExtensionOptions, -} from 'apollo-cache-control'; import { TracingExtension } from 'apollo-tracing'; import { ApolloError, @@ -97,7 +93,6 @@ export interface GraphQLRequestPipelineConfig { extensions?: Array<() => GraphQLExtension>; tracing?: boolean; persistedQueries?: PersistedQueryOptions; - cacheControl?: CacheControlExtensionOptions; formatError?: (error: GraphQLError) => GraphQLFormattedError; formatResponse?: ( @@ -126,7 +121,6 @@ export async function processGraphQLRequest( // all of our own machinery will certainly set it now. const logger = requestContext.logger || console; - let cacheControlExtension: CacheControlExtension | undefined; const extensionStack = initializeExtensionStack(); (requestContext.context as any)._extensionStack = extensionStack; @@ -390,20 +384,6 @@ export async function processGraphQLRequest( } } - if (cacheControlExtension) { - if (requestContext.overallCachePolicy) { - // If we read this response from a cache and it already has its own - // policy, teach that to cacheControlExtension so that it'll use the - // saved policy for HTTP headers. (If cacheControlExtension was a - // plugin, it could just read from the requestContext, but it isn't.) - cacheControlExtension.overrideOverallCachePolicy( - requestContext.overallCachePolicy, - ); - } else { - requestContext.overallCachePolicy = cacheControlExtension.computeOverallCachePolicy(); - } - } - const formattedExtensions = extensionStack.format(); if (Object.keys(formattedExtensions).length > 0) { response.extensions = formattedExtensions; @@ -604,11 +584,6 @@ export async function processGraphQLRequest( extensions.push(new TracingExtension()); } - if (config.cacheControl) { - cacheControlExtension = new CacheControlExtension(config.cacheControl); - extensions.push(cacheControlExtension); - } - return new GraphQLExtensionStack(extensions); } diff --git a/packages/apollo-server-core/src/runHttpQuery.ts b/packages/apollo-server-core/src/runHttpQuery.ts index c229fe87cf7..b02ce408fb0 100644 --- a/packages/apollo-server-core/src/runHttpQuery.ts +++ b/packages/apollo-server-core/src/runHttpQuery.ts @@ -17,7 +17,6 @@ import { GraphQLRequestContext, GraphQLResponse, } from './requestPipeline'; -import { CacheControlExtensionOptions } from 'apollo-cache-control'; import { ApolloServerPlugin } from 'apollo-server-plugin-base'; import { WithRequired, GraphQLExecutionResult } from 'apollo-server-types'; @@ -173,9 +172,6 @@ export async function runHttpQuery( // cacheControl defaults will also have been set if a boolean argument is // passed in. cache: options.cache!, - cacheControl: options.cacheControl as - | CacheControlExtensionOptions - | undefined, dataSources: options.dataSources, documentStore: options.documentStore, From 8db08b3b4d8be774be30178542b58a861d4bae3b Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Mon, 6 Apr 2020 20:46:55 +0300 Subject: [PATCH 2/3] eliminate! Remove now-unnecessary `apollo-cache-control` extension. Which has been replaced by the plugin API introduced in 68cbc938ff709078. --- package-lock.json | 3 +- packages/apollo-cache-control/package.json | 1 - ...ion.test.ts => cacheControlPlugin.test.ts} | 4 +- packages/apollo-cache-control/src/index.ts | 162 ------------------ packages/apollo-cache-control/tsconfig.json | 1 - 5 files changed, 3 insertions(+), 168 deletions(-) rename packages/apollo-cache-control/src/__tests__/{cacheControlExtension.test.ts => cacheControlPlugin.test.ts} (98%) diff --git a/package-lock.json b/package-lock.json index 519c1acc2ed..81f495d1916 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4277,8 +4277,7 @@ "version": "file:packages/apollo-cache-control", "requires": { "apollo-server-env": "file:packages/apollo-server-env", - "apollo-server-plugin-base": "file:packages/apollo-server-plugin-base", - "graphql-extensions": "file:packages/graphql-extensions" + "apollo-server-plugin-base": "file:packages/apollo-server-plugin-base" } }, "apollo-datasource": { diff --git a/packages/apollo-cache-control/package.json b/packages/apollo-cache-control/package.json index 23b4e1f583a..e4bb0cf3fbd 100644 --- a/packages/apollo-cache-control/package.json +++ b/packages/apollo-cache-control/package.json @@ -12,7 +12,6 @@ }, "dependencies": { "apollo-server-env": "file:../apollo-server-env", - "graphql-extensions": "file:../graphql-extensions", "apollo-server-plugin-base": "file:../apollo-server-plugin-base" }, "peerDependencies": { diff --git a/packages/apollo-cache-control/src/__tests__/cacheControlExtension.test.ts b/packages/apollo-cache-control/src/__tests__/cacheControlPlugin.test.ts similarity index 98% rename from packages/apollo-cache-control/src/__tests__/cacheControlExtension.test.ts rename to packages/apollo-cache-control/src/__tests__/cacheControlPlugin.test.ts index ff4d1fb75eb..a29cbbf02b1 100644 --- a/packages/apollo-cache-control/src/__tests__/cacheControlExtension.test.ts +++ b/packages/apollo-cache-control/src/__tests__/cacheControlPlugin.test.ts @@ -1,5 +1,4 @@ import { ResponsePath, GraphQLError } from 'graphql'; -import { GraphQLResponse } from 'graphql-extensions'; import { Headers } from 'apollo-server-env'; import { CacheScope, @@ -11,10 +10,11 @@ import { const { addHint, computeOverallCachePolicy } = __testing__; import { GraphQLRequestContextWillSendResponse, + GraphQLResponse, } from 'apollo-server-plugin-base'; import pluginTestHarness from 'apollo-server-core/dist/utils/pluginTestHarness'; -describe('CacheControlExtension', () => { +describe('plugin', () => { describe('willSendResponse', () => { function makePluginWithOptions({ pluginInitializationOptions, diff --git a/packages/apollo-cache-control/src/index.ts b/packages/apollo-cache-control/src/index.ts index f1a9a944e42..de847c31106 100644 --- a/packages/apollo-cache-control/src/index.ts +++ b/packages/apollo-cache-control/src/index.ts @@ -3,14 +3,11 @@ import { getNamedType, GraphQLInterfaceType, GraphQLObjectType, - GraphQLResolveInfo, ResponsePath, responsePathAsArray, } from 'graphql'; import { ApolloServerPlugin } from "apollo-server-plugin-base"; -import { GraphQLExtension, GraphQLResponse } from 'graphql-extensions'; - export interface CacheControlFormat { version: 1; hints: ({ path: (string | number)[] } & CacheHint)[]; @@ -195,165 +192,6 @@ export const plugin = ( } }); -export class CacheControlExtension - implements GraphQLExtension { - private defaultMaxAge: number; - - constructor(public options: CacheControlExtensionOptions = {}) { - this.defaultMaxAge = options.defaultMaxAge || 0; - } - - private hints: Map = new Map(); - private overallCachePolicyOverride?: Required; - - willResolveField( - _source: any, - _args: { [argName: string]: any }, - _context: TContext, - info: GraphQLResolveInfo, - ) { - let hint: CacheHint = {}; - - // If this field's resolver returns an object or interface, look for hints - // on that return type. - const targetType = getNamedType(info.returnType); - if ( - targetType instanceof GraphQLObjectType || - targetType instanceof GraphQLInterfaceType - ) { - if (targetType.astNode) { - hint = mergeHints( - hint, - cacheHintFromDirectives(targetType.astNode.directives), - ); - } - } - - // Look for hints on the field itself (on its parent type), taking - // precedence over previously calculated hints. - const fieldDef = info.parentType.getFields()[info.fieldName]; - if (fieldDef.astNode) { - hint = mergeHints( - hint, - cacheHintFromDirectives(fieldDef.astNode.directives), - ); - } - - // If this resolver returns an object or is a root field and we haven't seen - // an explicit maxAge hint, set the maxAge to 0 (uncached) or the default if - // specified in the constructor. (Non-object fields by default are assumed - // to inherit their cacheability from their parents. But on the other hand, - // while root non-object fields can get explicit hints from their definition - // on the Query/Mutation object, if that doesn't exist then there's no - // parent field that would assign the default maxAge, so we do it here.) - if ( - (targetType instanceof GraphQLObjectType || - targetType instanceof GraphQLInterfaceType || - !info.path.prev) && - hint.maxAge === undefined - ) { - hint.maxAge = this.defaultMaxAge; - } - - if (hint.maxAge !== undefined || hint.scope !== undefined) { - this.addHint(info.path, hint); - } - - info.cacheControl = { - setCacheHint: (hint: CacheHint) => { - this.addHint(info.path, hint); - }, - cacheHint: hint, - }; - } - - addHint(path: ResponsePath, hint: CacheHint) { - const existingCacheHint = this.hints.get(path); - if (existingCacheHint) { - this.hints.set(path, mergeHints(existingCacheHint, hint)); - } else { - this.hints.set(path, hint); - } - } - - format(): [string, CacheControlFormat] | undefined { - // We should have to explicitly ask to leave the formatted extension in, or - // pass the old-school `cacheControl: true` (as interpreted by - // apollo-server-core/ApolloServer), in order to include the - // engineproxy-aimed extensions. Specifically, we want users of - // apollo-server-plugin-response-cache to be able to specify - // `cacheControl: {defaultMaxAge: 600}` without accidentally turning on the - // extension formatting. - if (this.options.stripFormattedExtensions !== false) return; - - return [ - 'cacheControl', - { - version: 1, - hints: Array.from(this.hints).map(([path, hint]) => ({ - path: [...responsePathAsArray(path)], - ...hint, - })), - }, - ]; - } - - public willSendResponse?(o: { graphqlResponse: GraphQLResponse }) { - 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()}`, - ); - } - } - - public overrideOverallCachePolicy(overallCachePolicy: Required) { - this.overallCachePolicyOverride = overallCachePolicy; - } - - computeOverallCachePolicy(): Required | undefined { - if (this.overallCachePolicyOverride) { - return this.overallCachePolicyOverride; - } - - let lowestMaxAge: number | undefined = undefined; - let scope: CacheScope = CacheScope.Public; - - for (const hint of this.hints.values()) { - if (hint.maxAge !== undefined) { - lowestMaxAge = - lowestMaxAge !== undefined - ? Math.min(lowestMaxAge, hint.maxAge) - : hint.maxAge; - } - if (hint.scope === CacheScope.Private) { - scope = CacheScope.Private; - } - } - - // If maxAge is 0, then we consider it uncacheable so it doesn't matter what - // the scope was. - return lowestMaxAge - ? { - maxAge: lowestMaxAge, - scope, - } - : undefined; - } -} - function cacheHintFromDirectives( directives: ReadonlyArray | undefined, ): CacheHint | undefined { diff --git a/packages/apollo-cache-control/tsconfig.json b/packages/apollo-cache-control/tsconfig.json index 1276353ea04..29dff935854 100644 --- a/packages/apollo-cache-control/tsconfig.json +++ b/packages/apollo-cache-control/tsconfig.json @@ -7,7 +7,6 @@ "include": ["src/**/*"], "exclude": ["**/__tests__", "**/__mocks__"], "references": [ - { "path": "../graphql-extensions" }, { "path": "../apollo-server-plugin-base" }, ] } From 6f0dee7871d8cf06fa24b4242bb03f318a0697a8 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Fri, 8 May 2020 18:25:25 +0000 Subject: [PATCH 3/3] Switch to new `willResolveField` object parameter, rather position. Implements pattern gained by a926b7eedbb87abab2ec70fb03d7174398 in #3988. Ref: https://github.com/apollographql/apollo-server/pull/3998 --- packages/apollo-cache-control/src/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/apollo-cache-control/src/index.ts b/packages/apollo-cache-control/src/index.ts index 5e2277c2e7c..53f8e4272fd 100644 --- a/packages/apollo-cache-control/src/index.ts +++ b/packages/apollo-cache-control/src/index.ts @@ -66,8 +66,7 @@ export const plugin = ( return { executionDidStart: () => ({ executionDidEnd: () => setOverallCachePolicyWhenUnset(), - willResolveField(...args) { - const [, , , info] = args; + willResolveField({ info }) { let hint: CacheHint = {}; // If this field's resolver returns an object or interface, look for