From 7d2997c0ba9809f688b88ee28bf9ec4e721fdf56 Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@davidglasser.net>
Date: Tue, 22 Jan 2019 11:12:15 -0500
Subject: [PATCH] apollo-cache-control: consider hintless root fields to be
 uncached

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    | 10 ++++++
 .../__tests__/cacheControlDirective.test.ts   | 19 ++++++++++
 packages/apollo-cache-control/src/index.ts    | 35 ++++++++++---------
 3 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/packages/apollo-cache-control/CHANGELOG.md b/packages/apollo-cache-control/CHANGELOG.md
index 26db288ce83..2926b944b83 100644
--- a/packages/apollo-cache-control/CHANGELOG.md
+++ b/packages/apollo-cache-control/CHANGELOG.md
@@ -1,5 +1,15 @@
 # Changelog
 
+### vNEXT
+
+* Fix cache hints of `maxAge: 0` to mean "uncachable". (#2197)
+
+* Apply `defaultMaxAge` to scalar fields on the root object. (#???)
+
+
+(Missing release notes for 0.2.0, 0.2.1, 0.2.2, 0.2.3, 0.2.4, 0.2.5, 0.3.0,
+0.3.1, 0.3.2, 0.3.3, 0.3.4, and 0.4.0!)
+
 ### 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<TContext = any>
       }
     }
 
-    // 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<TContext = any>
       }
     }
 
+    // If maxAge is 0, then we consider it uncacheable so it doesn't matter what
+    // the scope was.
     return lowestMaxAge
       ? {
           maxAge: lowestMaxAge,