feat: add flag to CacheControlInstrumentation to optionally allow max…

…-age of zero (#392) This change adds an optional flag to `CacheControlInstrumentation`, `allowZeroMaxAge`, to allow consumers to configure the behavior such that resolved header can validly resolve a header value of `max-age=0`.
apollographql · May 15, 2024 · 18cf2a4 · 18cf2a4
1 parent 0f9eca8
commit 18cf2a4
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 14 deletions.
diff --git a/...in/java/com/apollographql/federation/graphqljava/caching/CacheControlInstrumentation.java b/...in/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<ExecutionResult> 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<String> 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/...ava/com/apollographql/federation/graphqljava/caching/CacheControlInstrumentationTest.java b/...ava/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<String, Object> 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<String, Object> 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));
   }
 }