remove all DataLoaderDispatcherInstrumentation related classes and refactor instrumentation

Author: hiddewie

graphql-java-kickstart/src/main/java/graphql/kickstart/execution/BatchedDataLoaderGraphQLBuilder.java

@@ -1,48 +1,20 @@
 package graphql.kickstart.execution;
 
-import graphql.ExecutionInput;
 import graphql.GraphQL;
-import graphql.execution.instrumentation.Instrumentation;
-import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentationOptions;
 import graphql.kickstart.execution.config.GraphQLBuilder;
 import graphql.kickstart.execution.input.GraphQLBatchedInvocationInput;
 import graphql.kickstart.execution.input.GraphQLSingleInvocationInput;
-import java.util.List;
-import java.util.function.Supplier;
 
 public class BatchedDataLoaderGraphQLBuilder {
 
-  private final Supplier<DataLoaderDispatcherInstrumentationOptions> optionsSupplier;
-
-  public BatchedDataLoaderGraphQLBuilder(
-      Supplier<DataLoaderDispatcherInstrumentationOptions> optionsSupplier) {
-    if (optionsSupplier != null) {
-      this.optionsSupplier = optionsSupplier;
-    } else {
-      this.optionsSupplier = DataLoaderDispatcherInstrumentationOptions::newOptions;
-    }
-  }
-
   GraphQL newGraphQL(GraphQLBatchedInvocationInput invocationInput, GraphQLBuilder graphQLBuilder) {
-    Supplier<Instrumentation> supplier =
-        augment(invocationInput, graphQLBuilder.getInstrumentationSupplier());
     return invocationInput.getInvocationInputs().stream()
         .findFirst()
         .map(GraphQLSingleInvocationInput::getSchema)
-        .map(schema -> graphQLBuilder.build(schema, supplier))
+        .map(schema -> graphQLBuilder.build(schema, graphQLBuilder.getInstrumentationSupplier()))
         .orElseThrow(
             () ->
                 new IllegalArgumentException(
                     "Batched invocation input must contain at least one query"));
   }
-
-  private Supplier<Instrumentation> augment(
-      GraphQLBatchedInvocationInput batchedInvocationInput,
-      Supplier<Instrumentation> instrumentationSupplier) {
-    List<ExecutionInput> executionInputs = batchedInvocationInput.getExecutionInputs();
-    return batchedInvocationInput
-        .getContextSetting()
-        .configureInstrumentationForContext(
-            instrumentationSupplier, executionInputs, optionsSupplier.get());
-  }
 }

graphql-java-kickstart/src/main/java/graphql/kickstart/execution/GraphQLQueryInvoker.java

@@ -3,7 +3,6 @@
 import graphql.execution.instrumentation.ChainedInstrumentation;
 import graphql.execution.instrumentation.Instrumentation;
 import graphql.execution.instrumentation.SimplePerformantInstrumentation;
-import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentationOptions;
 import graphql.execution.preparsed.NoOpPreparsedDocumentProvider;
 import graphql.execution.preparsed.PreparsedDocumentProvider;
 import graphql.kickstart.execution.config.DefaultExecutionStrategyProvider;
@@ -20,17 +19,14 @@ public class GraphQLQueryInvoker {
   private final Supplier<ExecutionStrategyProvider> getExecutionStrategyProvider;
   private final Supplier<Instrumentation> getInstrumentation;
   private final Supplier<PreparsedDocumentProvider> getPreparsedDocumentProvider;
-  private final Supplier<DataLoaderDispatcherInstrumentationOptions> optionsSupplier;
 
   protected GraphQLQueryInvoker(
       Supplier<ExecutionStrategyProvider> getExecutionStrategyProvider,
       Supplier<Instrumentation> getInstrumentation,
-      Supplier<PreparsedDocumentProvider> getPreparsedDocumentProvider,
-      Supplier<DataLoaderDispatcherInstrumentationOptions> optionsSupplier) {
+      Supplier<PreparsedDocumentProvider> getPreparsedDocumentProvider) {
     this.getExecutionStrategyProvider = getExecutionStrategyProvider;
     this.getInstrumentation = getInstrumentation;
     this.getPreparsedDocumentProvider = getPreparsedDocumentProvider;
-    this.optionsSupplier = optionsSupplier;
   }
 
   public static Builder newBuilder() {
@@ -43,7 +39,7 @@ public GraphQLInvoker toGraphQLInvoker() {
             .executionStrategyProvider(getExecutionStrategyProvider)
             .instrumentation(getInstrumentation)
             .preparsedDocumentProvider(getPreparsedDocumentProvider);
-    return new GraphQLInvoker(graphQLBuilder, new BatchedDataLoaderGraphQLBuilder(optionsSupplier));
+    return new GraphQLInvoker(graphQLBuilder, new BatchedDataLoaderGraphQLBuilder());
   }
 
   public static class Builder {
@@ -54,9 +50,6 @@ public static class Builder {
         () -> SimplePerformantInstrumentation.INSTANCE;
     private Supplier<PreparsedDocumentProvider> getPreparsedDocumentProvider =
         () -> NoOpPreparsedDocumentProvider.INSTANCE;
-    private Supplier<DataLoaderDispatcherInstrumentationOptions>
-        dataLoaderDispatcherInstrumentationOptionsSupplier =
-            DataLoaderDispatcherInstrumentationOptions::newOptions;
 
     public Builder withExecutionStrategyProvider(ExecutionStrategyProvider provider) {
       return withExecutionStrategyProvider(() -> provider);
@@ -97,23 +90,11 @@ public Builder withPreparsedDocumentProvider(Supplier<PreparsedDocumentProvider>
       return this;
     }
 
-    public Builder withDataLoaderDispatcherInstrumentationOptions(
-        DataLoaderDispatcherInstrumentationOptions options) {
-      return withDataLoaderDispatcherInstrumentationOptions(() -> options);
-    }
-
-    public Builder withDataLoaderDispatcherInstrumentationOptions(
-        Supplier<DataLoaderDispatcherInstrumentationOptions> supplier) {
-      this.dataLoaderDispatcherInstrumentationOptionsSupplier = supplier;
-      return this;
-    }
-
     public GraphQLQueryInvoker build() {
       return new GraphQLQueryInvoker(
           getExecutionStrategyProvider,
           getInstrumentation,
-          getPreparsedDocumentProvider,
-          dataLoaderDispatcherInstrumentationOptionsSupplier);
+          getPreparsedDocumentProvider);
     }
   }
 }

graphql-java-kickstart/src/main/java/graphql/kickstart/execution/config/GraphQLBuilder.java

@@ -2,10 +2,8 @@
 
 import graphql.GraphQL;
 import graphql.execution.ExecutionStrategy;
-import graphql.execution.instrumentation.ChainedInstrumentation;
 import graphql.execution.instrumentation.Instrumentation;
 import graphql.execution.instrumentation.SimplePerformantInstrumentation;
-import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation;
 import graphql.execution.preparsed.NoOpPreparsedDocumentProvider;
 import graphql.execution.preparsed.PreparsedDocumentProvider;
 import graphql.schema.GraphQLSchema;
@@ -89,18 +87,7 @@ public GraphQL build(
 
     Instrumentation instrumentation = configuredInstrumentationSupplier.get();
     builder.instrumentation(instrumentation);
-    if (containsDispatchInstrumentation(instrumentation)) {
-      builder.doNotAutomaticallyDispatchDataLoader();
-    }
     graphQLBuilderConfigurerSupplier.get().configure(builder);
     return builder.build();
   }
-
-  private boolean containsDispatchInstrumentation(Instrumentation instrumentation) {
-    if (instrumentation instanceof ChainedInstrumentation) {
-      return ((ChainedInstrumentation) instrumentation)
-          .getInstrumentations().stream().anyMatch(this::containsDispatchInstrumentation);
-    }
-    return instrumentation instanceof DataLoaderDispatcherInstrumentation;
-  }
 }

graphql-java-kickstart/src/main/java/graphql/kickstart/execution/context/ContextSetting.java

@@ -4,12 +4,10 @@
 import graphql.execution.ExecutionId;
 import graphql.execution.instrumentation.ChainedInstrumentation;
 import graphql.execution.instrumentation.Instrumentation;
-import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentationOptions;
 import graphql.kickstart.execution.GraphQLRequest;
 import graphql.kickstart.execution.input.GraphQLBatchedInvocationInput;
 import graphql.kickstart.execution.input.PerQueryBatchedInvocationInput;
 import graphql.kickstart.execution.input.PerRequestBatchedInvocationInput;
-import graphql.kickstart.execution.instrumentation.ConfigurableDispatchInstrumentation;
 import graphql.kickstart.execution.instrumentation.FieldLevelTrackingApproach;
 import graphql.kickstart.execution.instrumentation.RequestLevelTrackingApproach;
 import graphql.schema.GraphQLSchema;
@@ -28,11 +26,9 @@ public enum ContextSetting {
    * A context object, and therefor dataloader registry and subject, should be shared between all
    * GraphQL executions in a http request.
    */
-  PER_REQUEST_WITH_INSTRUMENTATION,
-  PER_REQUEST_WITHOUT_INSTRUMENTATION,
+  PER_REQUEST,
   /** Each GraphQL execution should always have its own context. */
-  PER_QUERY_WITH_INSTRUMENTATION,
-  PER_QUERY_WITHOUT_INSTRUMENTATION;
+  PER_QUERY;
 
   /**
    * Creates a set of inputs with the correct context based on the setting.
@@ -50,13 +46,9 @@ public GraphQLBatchedInvocationInput getBatch(
       Supplier<GraphQLKickstartContext> contextSupplier,
       Object root) {
     switch (this) {
-      case PER_QUERY_WITH_INSTRUMENTATION:
-        // Intentional fallthrough
-      case PER_QUERY_WITHOUT_INSTRUMENTATION:
+      case PER_QUERY:
         return new PerQueryBatchedInvocationInput(requests, schema, contextSupplier, root, this);
-      case PER_REQUEST_WITHOUT_INSTRUMENTATION:
-        // Intentional fallthrough
-      case PER_REQUEST_WITH_INSTRUMENTATION:
+      case PER_REQUEST:
         return new PerRequestBatchedInvocationInput(requests, schema, contextSupplier, root, this);
       default:
         throw new ContextSettingNotConfiguredException();
@@ -69,43 +61,12 @@ public GraphQLBatchedInvocationInput getBatch(
    *
    * @param instrumentation the instrumentation supplier to augment
    * @param executionInputs the inputs that will be dispatched by the instrumentation
-   * @param options the DataLoader dispatching instrumentation options that will be used.
+//   * @param options the DataLoader dispatching instrumentation options that will be used.
    * @return augmented instrumentation supplier.
    */
   public Supplier<Instrumentation> configureInstrumentationForContext(
       Supplier<Instrumentation> instrumentation,
-      List<ExecutionInput> executionInputs,
-      DataLoaderDispatcherInstrumentationOptions options) {
-    ConfigurableDispatchInstrumentation dispatchInstrumentation;
-    switch (this) {
-      case PER_REQUEST_WITH_INSTRUMENTATION:
-        DataLoaderRegistry registry =
-            executionInputs.stream()
-                .findFirst()
-                .map(ExecutionInput::getDataLoaderRegistry)
-                .orElseThrow(IllegalArgumentException::new);
-        List<ExecutionId> executionIds =
-            executionInputs.stream()
-                .map(ExecutionInput::getExecutionId)
-                .collect(Collectors.toList());
-        RequestLevelTrackingApproach requestTrackingApproach =
-            new RequestLevelTrackingApproach(executionIds, registry);
-        dispatchInstrumentation =
-            new ConfigurableDispatchInstrumentation(
-                options, (dataLoaderRegistry -> requestTrackingApproach));
-        break;
-      case PER_QUERY_WITH_INSTRUMENTATION:
-        dispatchInstrumentation =
-            new ConfigurableDispatchInstrumentation(options, FieldLevelTrackingApproach::new);
-        break;
-      case PER_REQUEST_WITHOUT_INSTRUMENTATION:
-        // Intentional fallthrough
-      case PER_QUERY_WITHOUT_INSTRUMENTATION:
-        return instrumentation;
-      default:
-        throw new ContextSettingNotConfiguredException();
-    }
-    return () ->
-        new ChainedInstrumentation(Arrays.asList(dispatchInstrumentation, instrumentation.get()));
+      List<ExecutionInput> executionInputs) {
+    return instrumentation;
   }
 }

graphql-java-kickstart/src/main/java/graphql/kickstart/execution/instrumentation/AbstractTrackingApproach.java

@@ -45,7 +45,7 @@ public ExecutionStrategyInstrumentationContext beginExecutionStrategy(
 
     return new ExecutionStrategyInstrumentationContext() {
       @Override
-      public void onDispatched(CompletableFuture<ExecutionResult> result) {
+      public void onDispatched() {
         // default empty implementation
       }
 
@@ -110,7 +110,7 @@ public InstrumentationContext<Object> beginFieldFetch(
     return new InstrumentationContext<Object>() {
 
       @Override
-      public void onDispatched(CompletableFuture result) {
+      public void onDispatched() {
         synchronized (stack) {
           stack.increaseFetchCount(executionId, level);
           stack.setStatus(executionId, dispatchIfNeeded(stack, executionId, level));