From 139c3b2d2cb73ea304b15a3c860664bb77538a32 Mon Sep 17 00:00:00 2001 From: Nick Armstrong Date: Tue, 14 May 2024 16:39:25 -0700 Subject: [PATCH] feat: add flag to CacheControlInstrumentation to optionally allow max-age=0 This change updates the CacheControlInsturmentation class to include an optional flag for allowZeroMaxAge in order to provide a minimally invasive way to for consumers to configure their cache control to allow for a max-age of 0. --- .../caching/CacheControlInstrumentation.java | 29 ++++++-- .../CacheControlInstrumentationTest.java | 68 +++++++++++++++++-- 2 files changed, 83 insertions(+), 14 deletions(-) diff --git a/graphql-java-support/src/main/java/com/apollographql/federation/graphqljava/caching/CacheControlInstrumentation.java b/graphql-java-support/src/main/java/com/apollographql/federation/graphqljava/caching/CacheControlInstrumentation.java index da70e1b9..036cbb16 100644 --- a/graphql-java-support/src/main/java/com/apollographql/federation/graphqljava/caching/CacheControlInstrumentation.java +++ b/graphql-java-support/src/main/java/com/apollographql/federation/graphqljava/caching/CacheControlInstrumentation.java @@ -29,6 +29,7 @@ */ public class CacheControlInstrumentation extends SimplePerformantInstrumentation { private final int defaultMaxAge; + private final boolean allowZeroMaxAge; private static final Object CONTEXT_KEY = new Object(); private static final String DIRECTIVE_NAME = "cacheControl"; @@ -37,11 +38,16 @@ public class CacheControlInstrumentation extends SimplePerformantInstrumentation private static final String INHERIT_MAX_AGE = "inheritMaxAge"; public CacheControlInstrumentation() { - this(0); + this(0, false); } public CacheControlInstrumentation(int defaultMaxAge) { + this(defaultMaxAge, false); + } + + public CacheControlInstrumentation(int defaultMaxAge, boolean allowZeroMaxAge) { this.defaultMaxAge = defaultMaxAge; + this.allowZeroMaxAge = allowZeroMaxAge; } @Nullable @@ -51,7 +57,7 @@ public static String cacheControlHeaderFromGraphQLContext(GraphQLContext context @Override public InstrumentationState createState(InstrumentationCreateStateParameters parameters) { - return new CacheControlState(); + return new CacheControlState(allowZeroMaxAge); } @Override @@ -78,7 +84,7 @@ public void onCompleted(ExecutionResult executionResult, Throwable throwable) { public InstrumentationContext beginField( InstrumentationFieldParameters parameters, InstrumentationState state) { CacheControlState cacheControlState = (CacheControlState) state; - CacheControlPolicy fieldPolicy = new CacheControlPolicy(); + CacheControlPolicy fieldPolicy = new CacheControlPolicy(allowZeroMaxAge); boolean inheritMaxAge = false; GraphQLUnmodifiedType unwrappedFieldType = @@ -172,12 +178,21 @@ enum CacheControlScope { } private static class CacheControlState implements InstrumentationState { - public final CacheControlPolicy overallPolicy = new CacheControlPolicy(); + public final CacheControlPolicy overallPolicy; + + public CacheControlState(boolean allowZeroMaxAge) { + this.overallPolicy = new CacheControlPolicy(allowZeroMaxAge); + } } private static class CacheControlPolicy { @Nullable private Integer maxAge; @Nullable private CacheControlScope scope = CacheControlScope.PUBLIC; + private final boolean allowZeroMaxAge; + + public CacheControlPolicy(boolean allowZeroMaxAge) { + this.allowZeroMaxAge = allowZeroMaxAge; + } void restrict(CacheControlPolicy policy) { if (policy.maxAge != null && (maxAge == null || policy.maxAge < maxAge)) { @@ -222,14 +237,14 @@ void replace(@Nullable CacheControlScope scope) { } public Optional maybeAsString() { - Integer maxAgeValue = maxAge == null ? 0 : maxAge; - if (maxAgeValue.equals(0)) { + if (maxAge == null || (!allowZeroMaxAge && maxAge.equals(0))) { return Optional.empty(); } CacheControlScope scopeValue = scope == null ? CacheControlScope.PUBLIC : scope; + return Optional.of( - String.format("max-age=%d, %s", maxAgeValue, scopeValue.toString().toLowerCase())); + String.format("max-age=%d, %s", maxAge, scopeValue.toString().toLowerCase())); } public boolean hasMaxAge() { diff --git a/graphql-java-support/src/test/java/com/apollographql/federation/graphqljava/caching/CacheControlInstrumentationTest.java b/graphql-java-support/src/test/java/com/apollographql/federation/graphqljava/caching/CacheControlInstrumentationTest.java index f95692c0..d20d207a 100644 --- a/graphql-java-support/src/test/java/com/apollographql/federation/graphqljava/caching/CacheControlInstrumentationTest.java +++ b/graphql-java-support/src/test/java/com/apollographql/federation/graphqljava/caching/CacheControlInstrumentationTest.java @@ -29,7 +29,7 @@ public class CacheControlInstrumentationTest { "enum CacheControlScope { PUBLIC PRIVATE }\n" + "directive @cacheControl(maxAge: Int scope: CacheControlScope inheritMaxAge: Boolean) on FIELD_DEFINITION | OBJECT | INTERFACE | UNION\n"; - static GraphQL makeExecutor(String sdl, int defaultMaxAge) { + static GraphQL makeExecutor(String sdl, int defaultMaxAge, boolean allowZeroMaxAge) { TypeDefinitionRegistry typeDefs = new SchemaParser().parse(DIRECTIVE_DEF + sdl); RuntimeWiring resolvers = @@ -42,21 +42,30 @@ static GraphQL makeExecutor(String sdl, int defaultMaxAge) { .build(); return GraphQL.newGraphQL(schema) - .instrumentation(new CacheControlInstrumentation(defaultMaxAge)) + .instrumentation(new CacheControlInstrumentation(defaultMaxAge, allowZeroMaxAge)) .build(); } static @Nullable String execute(String schema, String query) { - return execute(schema, query, 0, new HashMap<>()); + return execute(schema, query, 0, false, new HashMap<>()); } static @Nullable String execute(String schema, String query, int defaultMaxAge) { - return execute(schema, query, defaultMaxAge, new HashMap<>()); + return execute(schema, query, defaultMaxAge, false, new HashMap<>()); } static @Nullable String execute( - String schema, String query, int defaultMaxAge, Map variables) { - GraphQL graphql = makeExecutor(schema, defaultMaxAge); + String schema, String query, int defaultMaxAge, boolean allowZeroMaxAge) { + return execute(schema, query, defaultMaxAge, allowZeroMaxAge, new HashMap<>()); + } + + static @Nullable String execute( + String schema, + String query, + int defaultMaxAge, + boolean allowZeroMaxAge, + Map variables) { + GraphQL graphql = makeExecutor(schema, defaultMaxAge, allowZeroMaxAge); ExecutionInput input = ExecutionInput.newExecutionInput().query(query).variables(variables).build(); @@ -204,6 +213,51 @@ void overrideDefaultMaxAge() { assertNull(execute(schema, query, 10)); } + @Test + void overrideDefaultMaxAgeAllowZero() { + String schema = + "type Query {" + + " droid(id: ID!): Droid" + + "}" + + "" + + "type Droid @cacheControl(maxAge: 0) {" + + " id: ID!" + + " name: String" + + "}"; + String query = "{ droid(id: 2001) { name } }"; + assertEquals("max-age=0, public", execute(schema, query, 10, true)); + } + + @Test + void defaultMaxAgeAllowZero() { + String schema = + "type Query {" + + " droid(id: ID!): Droid" + + "}" + + "" + + "type Droid {" + + " id: ID!" + + " name: String" + + "}"; + String query = "{ droid(id: 2001) { name } }"; + assertEquals("max-age=0, public", execute(schema, query, 0, true)); + } + + @Test + void noMaxAgeWithDefaultMaxAge() { + String schema = + "type Query {" + + " droid(id: ID!): Droid" + + "}" + + "" + + "type Droid {" + + " id: ID!" + + " name: String" + + "}"; + String query = "{ droid(id: 2001) { name } }"; + assertEquals("max-age=10, public", execute(schema, query, 10, false)); + } + @Test void hintOnFieldOverrideMaxAgeHintOnReturnType() { String schema = @@ -380,6 +434,6 @@ void entities() { rs.add(user); variables.put("rs", rs); - assertEquals("max-age=30, public", execute(schema, query, 0, variables)); + assertEquals("max-age=30, public", execute(schema, query, 0, false, variables)); } }