Skip to content

Commit

Permalink
apollo-cache-control: consider hintless root fields to be uncached
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
glasser committed Jan 24, 2019
1 parent 06e4907 commit ce287da
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 16 deletions.
19 changes: 19 additions & 0 deletions packages/apollo-cache-control/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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 `[email protected]`.

(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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
35 changes: 19 additions & 16 deletions packages/apollo-cache-control/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit ce287da

Please sign in to comment.