From 821578775d3ca62ac33a76a8d3d8f51f412cdd80 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Fri, 25 Jan 2019 08:06:25 -0800 Subject: [PATCH] apollo-cache-control: consider hintless root fields to be uncached (#2210) This is consistent with the old engineproxy interpretation of cache hints. We special-case scalar fields to inherit their parent field's hints for simplicity (so you don't have to hint every scalar field in a hinted object), but when the parent field is non-root that inherited hint gets defaultMaxAge applied to it. When the parent field is the root, that inherited hint doesn't get defaultMaxAge applied because we don't run willResolveField for the root query. Includes a CHANGELOG update for #2197. --- packages/apollo-cache-control/CHANGELOG.md | 19 ++++++++++ .../__tests__/cacheControlDirective.test.ts | 19 ++++++++++ packages/apollo-cache-control/src/index.ts | 35 ++++++++++--------- 3 files changed, 57 insertions(+), 16 deletions(-) diff --git a/packages/apollo-cache-control/CHANGELOG.md b/packages/apollo-cache-control/CHANGELOG.md index 26db288ce83..9a6cefa3886 100644 --- a/packages/apollo-cache-control/CHANGELOG.md +++ b/packages/apollo-cache-control/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +### vNEXT + +* Fix cache hints of `maxAge: 0` to mean "uncachable". (#2197) + +* Apply `defaultMaxAge` to scalar fields on the root object. (#2210) + +### v0.3.0 + +* Support calculating Cache-Control HTTP headers when used by `apollo-server@2.0.0`. + +(There are a number of other 0.3.x releases as well as 0.4.0 with no code +changes due to how the `apollo-server` release process works.) + +### v0.2.0 + +Moved to the `apollo-server` git repository. No code changes. (There are a +number of other 0.2.x releases with no code changes due to how the +`apollo-server` release process works.) + ### v0.1.1 * Fix `defaultMaxAge` feature (introduced in 0.1.0) so that `maxAge: 0` overrides the default, as previously documented. diff --git a/packages/apollo-cache-control/src/__tests__/cacheControlDirective.test.ts b/packages/apollo-cache-control/src/__tests__/cacheControlDirective.test.ts index 5327c3c5e08..a10ded07543 100644 --- a/packages/apollo-cache-control/src/__tests__/cacheControlDirective.test.ts +++ b/packages/apollo-cache-control/src/__tests__/cacheControlDirective.test.ts @@ -30,6 +30,25 @@ describe('@cacheControl directives', () => { expect(hints).toContainEqual({ path: ['droid'], maxAge: 0 }); }); + it('should set maxAge: 0 and no scope for a top-level scalar field without cache hints', async () => { + const schema = buildSchemaWithCacheControlSupport(` + type Query { + name: String + } + `); + + const hints = await collectCacheControlHints( + schema, + ` + query { + name + } + `, + ); + + expect(hints).toContainEqual({ path: ['name'], maxAge: 0 }); + }); + it('should set maxAge to the default and no scope for a field without cache hints', async () => { const schema = buildSchemaWithCacheControlSupport(` type Query { diff --git a/packages/apollo-cache-control/src/index.ts b/packages/apollo-cache-control/src/index.ts index 80d8cd23913..0165628f106 100644 --- a/packages/apollo-cache-control/src/index.ts +++ b/packages/apollo-cache-control/src/index.ts @@ -75,26 +75,27 @@ export class CacheControlExtension } } - // If this field is a field on an object, look for hints on the field - // itself, taking precedence over previously calculated hints. - const parentType = info.parentType; - if (parentType instanceof GraphQLObjectType) { - const fieldDef = parentType.getFields()[info.fieldName]; - if (fieldDef.astNode) { - hint = mergeHints( - hint, - cacheHintFromDirectives(fieldDef.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 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.) + // 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) && + targetType instanceof GraphQLInterfaceType || + !info.path.prev) && hint.maxAge === undefined ) { hint.maxAge = this.defaultMaxAge; @@ -167,6 +168,8 @@ export class CacheControlExtension } } + // If maxAge is 0, then we consider it uncacheable so it doesn't matter what + // the scope was. return lowestMaxAge ? { maxAge: lowestMaxAge,