refactor: update core gql libs

Author: aaron-steinfeld

hypertrace-core-graphql-context/src/main/java/org/hypertrace/core/graphql/context/DefaultGraphQlRequestContextBuilder.java

@@ -1,9 +1,8 @@
 package org.hypertrace.core.graphql.context;
 
 import com.google.common.collect.Streams;
-import graphql.kickstart.servlet.context.DefaultGraphQLServletContext;
+import graphql.kickstart.execution.context.DefaultGraphQLContext;
 import graphql.kickstart.servlet.context.DefaultGraphQLServletContextBuilder;
-import graphql.kickstart.servlet.context.GraphQLServletContext;
 import graphql.schema.DataFetcher;
 import java.util.Arrays;
 import java.util.Map;
@@ -14,10 +13,8 @@
 import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
 import javax.inject.Inject;
-import javax.security.auth.Subject;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-import org.dataloader.DataLoaderRegistry;
 import org.hypertrace.core.graphql.spi.config.GraphQlServiceConfig;
 
 class DefaultGraphQlRequestContextBuilder extends DefaultGraphQLServletContextBuilder
@@ -40,32 +37,22 @@ class DefaultGraphQlRequestContextBuilder extends DefaultGraphQLServletContextBu
 
   @Override
   public GraphQlRequestContext build(HttpServletRequest request, HttpServletResponse response) {
-    return new DefaultGraphQlRequestContext(request, response);
+    return new DefaultGraphQlRequestContext(request);
   }
 
-  private final class DefaultGraphQlRequestContext implements GraphQlRequestContext {
-    private final GraphQLServletContext servletContext;
+  private final class DefaultGraphQlRequestContext extends DefaultGraphQLContext
+      implements GraphQlRequestContext {
     private final ContextualCachingKey cachingKey;
 
     private final String requestId = UUID.randomUUID().toString();
+    private final HttpServletRequest request;
 
-    private DefaultGraphQlRequestContext(HttpServletRequest request, HttpServletResponse response) {
-      this.servletContext =
-          DefaultGraphQLServletContext.createServletContext().with(request).with(response).build();
+    private DefaultGraphQlRequestContext(HttpServletRequest request) {
+      this.request = request;
       this.cachingKey =
           new DefaultContextualCacheKey(this, this.getTenantId().orElse(DEFAULT_CONTEXT_ID));
     }
 
-    @Override
-    public Optional<Subject> getSubject() {
-      return this.servletContext.getSubject();
-    }
-
-    @Override
-    public Optional<DataLoaderRegistry> getDataLoaderRegistry() {
-      return this.servletContext.getDataLoaderRegistry();
-    }
-
     @Override
     public <T> DataFetcher<CompletableFuture<T>> constructDataFetcher(
         Class<? extends DataFetcher<CompletableFuture<T>>> dataFetcherClass) {
@@ -75,29 +62,24 @@ public <T> DataFetcher<CompletableFuture<T>> constructDataFetcher(
 
     @Override
     public Optional<String> getAuthorizationHeader() {
-      HttpServletRequest request = this.servletContext.getHttpServletRequest();
       return Optional.ofNullable(request.getHeader(AUTHORIZATION_HEADER_KEY))
           .or(() -> Optional.ofNullable(request.getHeader(AUTHORIZATION_HEADER_KEY.toLowerCase())));
     }
 
     @Override
     public Optional<String> getTenantId() {
-      HttpServletRequest request = this.servletContext.getHttpServletRequest();
       return Optional.ofNullable(request.getHeader(TENANT_ID_HEADER_KEY))
           .or(DefaultGraphQlRequestContextBuilder.this.serviceConfig::getDefaultTenantId);
     }
 
     @Override
     public Map<String, String> getTracingContextHeaders() {
-      return Streams.stream(
-              this.servletContext.getHttpServletRequest().getHeaderNames().asIterator())
+      return Streams.stream(request.getHeaderNames().asIterator())
           .filter(
               header ->
                   TRACING_CONTEXT_HEADER_KEY_PREFIXES.stream()
                       .anyMatch(prefix -> header.toLowerCase().startsWith(prefix.toLowerCase())))
-          .collect(
-              Collectors.toUnmodifiableMap(
-                  String::toLowerCase, this.servletContext.getHttpServletRequest()::getHeader));
+          .collect(Collectors.toUnmodifiableMap(String::toLowerCase, request::getHeader));
     }
 
     @Nonnull

hypertrace-core-graphql-context/src/main/java/org/hypertrace/core/graphql/context/GraphQlRequestContext.java

@@ -1,13 +1,13 @@
 package org.hypertrace.core.graphql.context;
 
-import graphql.kickstart.execution.context.GraphQLContext;
+import graphql.kickstart.execution.context.GraphQLKickstartContext;
 import graphql.schema.DataFetcher;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import javax.annotation.Nonnull;
 
-public interface GraphQlRequestContext extends GraphQLContext {
+public interface GraphQlRequestContext extends GraphQLKickstartContext {
 
   /**
    * A tool to create data fetchers via injection container due to limitations in the framework. For

hypertrace-core-graphql-context/src/test/java/org/hypertrace/core/graphql/context/DefaultGraphQlRequestContextBuilderTest.java

@@ -7,7 +7,6 @@
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNotSame;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.reset;
@@ -67,7 +66,7 @@ void returnsEmptyOptionalIfNoAuthorizationHeaderPresent() {
 
   @Test
   void delegatesDataLoaderRegistry() {
-    assertTrue(this.requestContext.getDataLoaderRegistry().isPresent());
+    assertNotNull(this.requestContext.getDataLoaderRegistry());
   }
 
   @Test

hypertrace-core-graphql-platform/build.gradle.kts

@@ -18,8 +18,8 @@ dependencies {
     api("org.hypertrace.core.attribute.service:caching-attribute-service-client:0.14.8")
 
     api("com.google.inject:guice:5.1.0")
-    api("com.graphql-java:graphql-java:15.0")
-    api("io.github.graphql-java:graphql-java-annotations:8.3")
+    api("com.graphql-java:graphql-java:19.2")
+    api("io.github.graphql-java:graphql-java-annotations:9.1")
     api("org.slf4j:slf4j-api:1.7.36")
     api("io.reactivex.rxjava3:rxjava:3.1.5")
     api("com.google.protobuf:protobuf-java-util:3.21.1")
@@ -28,7 +28,7 @@ dependencies {
     api("com.google.code.findbugs:jsr305:3.0.2")
     api("com.typesafe:config:1.4.2")
     api("com.google.guava:guava:31.1-jre")
-    api("com.graphql-java-kickstart:graphql-java-servlet:10.1.0")
+    api("com.graphql-java-kickstart:graphql-java-servlet:14.0.0")
 
     api("com.fasterxml.jackson.core:jackson-databind:2.13.4")
     api("com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.13.4")

hypertrace-core-graphql-schema-registry/src/main/java/org/hypertrace/core/graphql/schema/registry/DefaultGraphQlSchemaRegistry.java

@@ -22,7 +22,7 @@ public Set<GraphQlSchemaFragment> getRegisteredFragments() {
   }
 
   @Override
-  public GraphQlSchemaFragment getRootFragment() {
+  public DefaultSchema getRootFragment() {
     return this.defaultSchemaFragment;
   }
 }

hypertrace-core-graphql-schema-registry/src/main/java/org/hypertrace/core/graphql/schema/registry/DefaultSchema.java

@@ -1,9 +1,17 @@
 package org.hypertrace.core.graphql.schema.registry;
 
+import graphql.annotations.annotationTypes.GraphQLDescription;
+import graphql.annotations.annotationTypes.GraphQLField;
 import graphql.annotations.annotationTypes.GraphQLName;
 import org.hypertrace.core.graphql.spi.schema.GraphQlSchemaFragment;
 
 class DefaultSchema implements GraphQlSchemaFragment {
+  // Placeholder description is used to identify and remove placeholder fields before building the
+  // schema. Placeholders are used while the schema is under construction so it is always valid
+  // (i.e. has at least one value)
+  static final String PLACEHOLDER_DESCRIPTION = "::placeholder::";
+  static final String ROOT_QUERY_NAME = "Query";
+  static final String ROOT_MUTATION_NAME = "Mutation";
 
   @Override
   public String fragmentName() {
@@ -20,9 +28,17 @@ public Class<?> annotatedMutationClass() {
     return MutationSchema.class;
   }
 
-  @GraphQLName("Query")
-  private interface QuerySchema {}
+  @GraphQLName(ROOT_QUERY_NAME)
+  private interface QuerySchema {
+    @GraphQLField
+    @GraphQLDescription(PLACEHOLDER_DESCRIPTION)
+    String placeholder();
+  }
 
-  @GraphQLName("Mutation")
-  private interface MutationSchema {}
+  @GraphQLName(ROOT_MUTATION_NAME)
+  private interface MutationSchema {
+    @GraphQLField
+    @GraphQLDescription(PLACEHOLDER_DESCRIPTION)
+    String placeholder();
+  }
 }

hypertrace-core-graphql-schema-registry/src/main/java/org/hypertrace/core/graphql/schema/registry/GraphQlAnnotatedSchemaMerger.java

@@ -12,6 +12,7 @@
 import java.util.Collection;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
@@ -39,7 +40,8 @@ public GraphQLSchema get() {
 
     return fragments.stream()
         .map(fragment -> this.fragmentToSchema(fragment, annotationProcessor))
-        .reduce(this.fragmentToSchema(rootFragment, annotationProcessor), this::merge);
+        .reduce(this.fragmentToSchema(rootFragment, annotationProcessor), this::merge)
+        .transform(this::removeRootPlaceholders);
   }
 
   private void registerAllTypeFunctions(
@@ -134,4 +136,36 @@ private Map<GraphQLObjectType, GraphQLObjectType> buildMapWithoutNulls(
         .filter(entry -> nonNull(entry) && nonNull(entry.getKey()) && nonNull(entry.getValue()))
         .collect(Collectors.toUnmodifiableMap(Entry::getKey, Entry::getValue));
   }
+
+  private void removeRootPlaceholders(GraphQLSchema.Builder mergedSchema) {
+    GraphQLSchema mergedWithPlaceholders = mergedSchema.build();
+    // Queries must be defined, mutation is optional which is represented by a null
+    mergedSchema.query(
+        this.rebuildTypeWithoutPlaceholders(mergedWithPlaceholders.getQueryType()).orElseThrow());
+    mergedSchema.mutation(
+        this.rebuildTypeWithoutPlaceholders(mergedWithPlaceholders.getMutationType()).orElse(null));
+  }
+
+  private Optional<GraphQLObjectType> rebuildTypeWithoutPlaceholders(GraphQLObjectType objectType) {
+    // Schema validation now requires all objects to have at least one field. To merge partial
+    // fragments, we start with placeholders, identified by a constant, then remove them at the end.
+    GraphQLObjectType newType =
+        GraphQLObjectType.newObject(objectType)
+            .replaceFields(
+                objectType.getFields().stream()
+                    .filter(
+                        field ->
+                            Optional.ofNullable(field.getDescription())
+                                .map(
+                                    description ->
+                                        !description.equals(DefaultSchema.PLACEHOLDER_DESCRIPTION))
+                                .orElse(true))
+                    .collect(Collectors.toList()))
+            .build();
+
+    if (newType.getFields().isEmpty()) {
+      return Optional.empty();
+    }
+    return Optional.of(newType);
+  }
 }

hypertrace-core-graphql-schema-registry/src/test/java/org/hypertrace/core/graphql/schema/registry/GraphQlAnnotatedSchemaMergerTest.java

@@ -1,12 +1,14 @@
 package org.hypertrace.core.graphql.schema.registry;
 
+import static org.hypertrace.core.graphql.schema.registry.DefaultSchema.ROOT_MUTATION_NAME;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.when;
 
+import graphql.Scalars;
 import graphql.annotations.annotationTypes.GraphQLDataFetcher;
 import graphql.annotations.annotationTypes.GraphQLField;
-import graphql.annotations.annotationTypes.GraphQLName;
 import graphql.annotations.processor.ProcessingElementsContainer;
 import graphql.annotations.processor.typeFunctions.TypeFunction;
 import graphql.schema.DataFetcher;
@@ -30,20 +32,11 @@
 @ExtendWith(MockitoExtension.class)
 public class GraphQlAnnotatedSchemaMergerTest {
 
-  private static final String ROOT_QUERY_NAME = "root";
-  private static final String ROOT_MUTATION_NAME = "rootMutation";
-
   @Mock private GraphQlSchemaRegistry mockRegistry;
   @Mock private DataFetchingEnvironment mockDataFetchingEnvironment;
 
   private GraphQlAnnotatedSchemaMerger merger;
 
-  @GraphQLName(ROOT_QUERY_NAME)
-  interface RootQuerySchema {}
-
-  @GraphQLName(ROOT_MUTATION_NAME)
-  interface RootMutationSchema {}
-
   interface FirstQuerySchema {
     @GraphQLField
     String first();
@@ -91,8 +84,7 @@ interface SharedType {
   public void beforeEach() {
     this.merger = new GraphQlAnnotatedSchemaMerger(mockRegistry);
 
-    when(mockRegistry.getRootFragment())
-        .thenReturn(this.createSchemaFragment(RootQuerySchema.class, RootMutationSchema.class));
+    when(mockRegistry.getRootFragment()).thenReturn(new DefaultSchema());
   }
 
   @Test
@@ -148,10 +140,13 @@ void mergesMutationSchemas() {
   @Test
   void supportsMutationOnlyFragment() {
     when(mockRegistry.getRegisteredFragments())
-        .thenReturn(Set.of(this.createSchemaFragment(null, FirstMutationSchema.class)));
+        .thenReturn(
+            Set.of(
+                this.createSchemaFragment(FirstQuerySchema.class),
+                this.createSchemaFragment(null, FirstMutationSchema.class)));
 
     this.verifySchemaWithMutationFields(this.merger.get(), Set.of("mutateOne"));
-    this.verifySchemaWithQueryFields(this.merger.get(), Set.of());
+    this.verifySchemaWithQueryFields(this.merger.get(), Set.of("first", "second"));
   }
 
   @Test
@@ -169,7 +164,13 @@ public GraphQLType buildType(
               Class<?> aClass,
               AnnotatedType annotatedType,
               ProcessingElementsContainer container) {
-            return GraphQLObjectType.newObject().name("typeFunctionType").build();
+            return GraphQLObjectType.newObject()
+                .name("typeFunctionType")
+                .field(
+                    GraphQLFieldDefinition.newFieldDefinition()
+                        .name("typeFunctionTypeField")
+                        .type(Scalars.GraphQLString))
+                .build();
           }
         };
 
@@ -220,7 +221,7 @@ public List<TypeFunction> typeFunctions() {
   }
 
   private void verifySchemaWithQueryFields(GraphQLSchema schema, Set<String> fields) {
-    assertEquals(ROOT_QUERY_NAME, schema.getQueryType().getName());
+    assertEquals(DefaultSchema.ROOT_QUERY_NAME, schema.getQueryType().getName());
     List<GraphQLFieldDefinition> fieldDefinitions = schema.getQueryType().getFieldDefinitions();
     assertEquals(fields.size(), fieldDefinitions.size());
     assertTrue(
@@ -231,13 +232,19 @@ private void verifySchemaWithQueryFields(GraphQLSchema schema, Set<String> field
   }
 
   private void verifySchemaWithMutationFields(GraphQLSchema schema, Set<String> fields) {
-    assertEquals(ROOT_MUTATION_NAME, schema.getMutationType().getName());
-    List<GraphQLFieldDefinition> fieldDefinitions = schema.getMutationType().getFieldDefinitions();
-    assertEquals(fields.size(), fieldDefinitions.size());
-    assertTrue(
-        fields.containsAll(
-            fieldDefinitions.stream()
-                .map(GraphQLFieldDefinition::getName)
-                .collect(Collectors.toUnmodifiableSet())));
+    if (fields.isEmpty()) {
+      // A type must have fields, so if no fields expected, no type expected
+      assertNull(schema.getMutationType());
+    } else {
+      assertEquals(ROOT_MUTATION_NAME, schema.getMutationType().getName());
+      List<GraphQLFieldDefinition> fieldDefinitions =
+          schema.getMutationType().getFieldDefinitions();
+      assertEquals(fields.size(), fieldDefinitions.size());
+      assertTrue(
+          fields.containsAll(
+              fieldDefinitions.stream()
+                  .map(GraphQLFieldDefinition::getName)
+                  .collect(Collectors.toUnmodifiableSet())));
+    }
   }
 }