Merge branch 'master' into graphql-java-16

Author: oliemansm

.github/ISSUE_TEMPLATE/config.yml

@@ -1,5 +1,5 @@
 blank_issues_enabled: false
 contact_links:
   - name: Question
-    url: https://spectrum.chat/graphql-java-kick
-    about: Anything you are not sure about? Ask the community on Spectrum!
+    url: https://github.com/graphql-java-kickstart/graphql-java-servlet/discussions
+    about: Anything you are not sure about? Ask the community in Discussions!

.github/workflows/pull-request.yml

@@ -4,7 +4,7 @@ on:
     branches-ignore:
       - master
   pull_request:
-
+    types: [opened, synchronize, reopened]
 jobs:
   validation:
     name: Gradle Wrapper Validation
@@ -49,3 +49,39 @@ jobs:
         if: matrix.os == 'windows-latest'
         shell: cmd
         run: gradlew --info check
+  build:
+    name: Sonar analysis
+    needs: validation
+    runs-on: ubuntu-latest
+    env:
+      SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+    steps:
+      - uses: actions/checkout@v2
+        if: env.SONAR_TOKEN != null
+        with:
+          fetch-depth: 0  # Shallow clones should be disabled for a better relevancy of analysis
+      - name: Set up JDK 11
+        if: env.SONAR_TOKEN != null
+        uses: actions/setup-java@v1
+        with:
+          java-version: 11
+      - name: Cache SonarCloud packages
+        if: env.SONAR_TOKEN != null
+        uses: actions/cache@v1
+        with:
+          path: ~/.sonar/cache
+          key: ${{ runner.os }}-sonar
+          restore-keys: ${{ runner.os }}-sonar
+      - name: Cache Gradle packages
+        if: env.SONAR_TOKEN != null
+        uses: actions/cache@v1
+        with:
+          path: ~/.gradle/caches
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle
+      - name: Build and analyze
+        if: env.SONAR_TOKEN != null
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}  # Needed to get PR information, if any
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+        run: ./gradlew build jacocoTestReport sonarqube --info

.github/workflows/snapshot.yml

@@ -66,3 +66,32 @@ jobs:
           BINTRAY_USER: ${{ secrets.BINTRAY_USER }}
           BINTRAY_PASS: ${{ secrets.BINTRAY_PASSWORD }}
         run: ./gradlew artifactoryPublish -Dsnapshot=true -Dbuild.number=${{ env.GITHUB_RUN_NUMBER }}
+  sonar:
+    name: Sonar analysis
+    needs: validation
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+        with:
+          fetch-depth: 0  # Shallow clones should be disabled for a better relevancy of analysis
+      - name: Set up JDK 11
+        uses: actions/setup-java@v1
+        with:
+          java-version: 11
+      - name: Cache SonarCloud packages
+        uses: actions/cache@v1
+        with:
+          path: ~/.sonar/cache
+          key: ${{ runner.os }}-sonar
+          restore-keys: ${{ runner.os }}-sonar
+      - name: Cache Gradle packages
+        uses: actions/cache@v1
+        with:
+          path: ~/.gradle/caches
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle
+      - name: Build and analyze
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}  # Needed to get PR information, if any
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+        run: ./gradlew build jacocoTestReport sonarqube --info

build.gradle

@@ -37,10 +37,22 @@ plugins {
     id 'io.franzbecker.gradle-lombok' version '3.2.0' apply false
     id "com.jfrog.artifactory" version "4.11.0" apply false
     id "biz.aQute.bnd.builder" version "5.1.2" apply false
+    id "org.sonarqube" version "3.0"
+    id "jacoco"
+}
+
+sonarqube {
+    properties {
+        property "sonar.projectKey", "graphql-java-kickstart_graphql-java-servlet"
+        property "sonar.organization", "graphql-java-kickstart"
+        property "sonar.host.url", "https://sonarcloud.io"
+    }
 }
 
 subprojects {
     apply plugin: 'idea'
+    apply plugin: 'jacoco'
+    apply plugin: 'org.sonarqube'
     apply plugin: 'java'
     apply plugin: 'maven-publish'
     apply plugin: "com.jfrog.bintray"
@@ -75,6 +87,13 @@ subprojects {
         sha256 = ""
     }
 
+    jacocoTestReport {
+        reports {
+            xml.enabled = true
+            html.enabled = false
+            csv.enabled = false
+        }
+    }
 
     if (!it.name.startsWith('example')) {
 

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

@@ -8,18 +8,17 @@
 import graphql.kickstart.execution.input.GraphQLBatchedInvocationInput;
 import graphql.kickstart.execution.input.GraphQLInvocationInput;
 import graphql.kickstart.execution.input.GraphQLSingleInvocationInput;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
-import lombok.AllArgsConstructor;
 import lombok.RequiredArgsConstructor;
 
-@AllArgsConstructor
 @RequiredArgsConstructor
 public class GraphQLInvoker {
 
   private final GraphQLBuilder graphQLBuilder;
   private final BatchedDataLoaderGraphQLBuilder batchedDataLoaderGraphQLBuilder;
-  private GraphQLInvokerProxy proxy = GraphQL::executeAsync;
+  private final GraphQLInvokerProxy proxy = GraphQL::executeAsync;
 
   public CompletableFuture<ExecutionResult> executeAsync(
       GraphQLSingleInvocationInput invocationInput) {
@@ -28,26 +27,39 @@ public CompletableFuture<ExecutionResult> executeAsync(
   }
 
   public GraphQLQueryResult query(GraphQLInvocationInput invocationInput) {
+    return queryAsync(invocationInput).join();
+  }
+
+  public CompletableFuture<GraphQLQueryResult> queryAsync(GraphQLInvocationInput invocationInput) {
     if (invocationInput instanceof GraphQLSingleInvocationInput) {
-      return GraphQLQueryResult.create(query((GraphQLSingleInvocationInput) invocationInput));
+      return executeAsync((GraphQLSingleInvocationInput) invocationInput)
+          .thenApply(GraphQLQueryResult::create);
     }
     GraphQLBatchedInvocationInput batchedInvocationInput = (GraphQLBatchedInvocationInput) invocationInput;
-    return GraphQLQueryResult.create(query(batchedInvocationInput));
-  }
-
-  private ExecutionResult query(GraphQLSingleInvocationInput singleInvocationInput) {
-    return executeAsync(singleInvocationInput).join();
+    return executeAsync(batchedInvocationInput).thenApply(GraphQLQueryResult::create);
   }
 
-  private List<ExecutionResult> query(GraphQLBatchedInvocationInput batchedInvocationInput) {
+  private CompletableFuture<List<ExecutionResult>> executeAsync(
+      GraphQLBatchedInvocationInput batchedInvocationInput) {
     GraphQL graphQL = batchedDataLoaderGraphQLBuilder
         .newGraphQL(batchedInvocationInput, graphQLBuilder);
-    return batchedInvocationInput.getExecutionInputs().stream()
-        .map(executionInput -> proxy.executeAsync(graphQL, executionInput))
-        .collect(toList())
-        .stream()
-        .map(CompletableFuture::join)
-        .collect(toList());
+    return sequence(
+        batchedInvocationInput.getExecutionInputs().stream()
+            .map(executionInput -> proxy.executeAsync(graphQL, executionInput))
+            .collect(toList()));
+  }
+
+  @SuppressWarnings({"unchecked", "rawtypes"})
+  private <T> CompletableFuture<List<T>> sequence(List<CompletableFuture<T>> futures) {
+    CompletableFuture[] futuresArray = futures.toArray(new CompletableFuture[0]);
+    return CompletableFuture.allOf(futuresArray).thenApply(aVoid -> {
+      List<T> result = new ArrayList<>(futures.size());
+      for (CompletableFuture future : futuresArray) {
+        assert future.isDone(); // per the API contract of allOf()
+        result.add((T) future.join());
+      }
+      return result;
+    });
   }
 }
 

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

@@ -17,17 +17,16 @@ public CompletableFuture<ExecutionResult> executeAsync(GraphQL graphQL,
     GraphQLContext context = (GraphQLContext) executionInput.getContext();
     if (Subject.getSubject(AccessController.getContext()) == null && context.getSubject()
         .isPresent()) {
-      return Subject
-          .doAs(context.getSubject().get(),
-              (PrivilegedAction<CompletableFuture<ExecutionResult>>) () -> {
-                try {
-                  return graphQL.executeAsync(executionInput);
-                } catch (Exception e) {
-                  throw new RuntimeException(e);
-                }
-              });
+      return context.getSubject()
+          .map(it -> Subject.doAs(it, doAction(graphQL, executionInput)))
+          .orElseGet(() -> graphQL.executeAsync(executionInput));
     }
     return graphQL.executeAsync(executionInput);
   }
 
+  private PrivilegedAction<CompletableFuture<ExecutionResult>> doAction(GraphQL graphQL,
+      ExecutionInput executionInput) {
+    return () -> graphQL.executeAsync(executionInput);
+  }
+
 }

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

@@ -18,6 +18,7 @@ public class GraphQLBuilder {
   private Supplier<PreparsedDocumentProvider> preparsedDocumentProviderSupplier = () -> NoOpPreparsedDocumentProvider.INSTANCE;
   @Getter
   private Supplier<Instrumentation> instrumentationSupplier = () -> SimpleInstrumentation.INSTANCE;
+  private Supplier<GraphQLBuilderConfigurer> graphQLBuilderConfigurerSupplier = () -> builder -> {};
 
   public GraphQLBuilder executionStrategyProvider(Supplier<ExecutionStrategyProvider> supplier) {
     if (supplier != null) {
@@ -40,6 +41,13 @@ public GraphQLBuilder instrumentation(Supplier<Instrumentation> supplier) {
     return this;
   }
 
+  public GraphQLBuilder graphQLBuilderConfigurer(Supplier<GraphQLBuilderConfigurer> supplier) {
+    if (supplier != null) {
+      graphQLBuilderConfigurerSupplier = supplier;
+    }
+    return this;
+  }
+
   public GraphQL build(GraphQLSchemaProvider schemaProvider) {
     return build(schemaProvider.getSchema());
   }
@@ -78,6 +86,7 @@ public GraphQL build(GraphQLSchema schema,
     if (containsDispatchInstrumentation(instrumentation)) {
       builder.doNotAddDefaultInstrumentations();
     }
+    graphQLBuilderConfigurerSupplier.get().configure(builder);
     return builder.build();
   }
 

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

@@ -0,0 +1,9 @@
+package graphql.kickstart.execution.config;
+
+import graphql.GraphQL;
+
+public interface GraphQLBuilderConfigurer {
+
+  void configure(GraphQL.Builder builder);
+
+}

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

@@ -25,7 +25,7 @@ public abstract class AbstractTrackingApproach implements TrackingApproach {
 
   private final RequestStack stack = new RequestStack();
 
-  public AbstractTrackingApproach(DataLoaderRegistry dataLoaderRegistry) {
+  protected AbstractTrackingApproach(DataLoaderRegistry dataLoaderRegistry) {
     this.dataLoaderRegistry = dataLoaderRegistry;
   }
 
@@ -52,12 +52,12 @@ public ExecutionStrategyInstrumentationContext beginExecutionStrategy(
     return new ExecutionStrategyInstrumentationContext() {
       @Override
       public void onDispatched(CompletableFuture<ExecutionResult> result) {
-
+        // default empty implementation
       }
 
       @Override
       public void onCompleted(ExecutionResult result, Throwable t) {
-
+        // default empty implementation
       }
 
       @Override
@@ -126,6 +126,7 @@ public void onDispatched(CompletableFuture result) {
 
       @Override
       public void onCompleted(Object result, Throwable t) {
+        // default empty implementation
       }
     };
   }

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

@@ -23,17 +23,14 @@
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Function;
+import lombok.extern.slf4j.Slf4j;
 import org.dataloader.DataLoader;
 import org.dataloader.DataLoaderRegistry;
 import org.dataloader.stats.Statistics;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
+@Slf4j
 public class ConfigurableDispatchInstrumentation extends DataLoaderDispatcherInstrumentation {
 
-  private static final Logger log = LoggerFactory
-      .getLogger(ConfigurableDispatchInstrumentation.class);
-
   private final DataLoaderDispatcherInstrumentationOptions options;
 
   private final Function<DataLoaderRegistry, TrackingApproach> approachFunction;
@@ -78,26 +75,26 @@ public DataFetcher<?> instrumentDataFetcher(DataFetcher<?> dataFetcher,
     // which allows them to work if used.
     return (DataFetcher<Object>) environment -> {
       Object obj = dataFetcher.get(environment);
-      immediatelyDispatch(state);
+      doImmediatelyDispatch(state);
       return obj;
     };
   }
 
-  private void immediatelyDispatch(DataLoaderDispatcherInstrumentationState state) {
+  private void doImmediatelyDispatch(DataLoaderDispatcherInstrumentationState state) {
     state.getApproach().dispatch();
   }
 
   @Override
   public InstrumentationContext<ExecutionResult> beginExecuteOperation(
       InstrumentationExecuteOperationParameters parameters) {
-    if (!isDataLoaderCompatibleExecution(parameters.getExecutionContext())) {
+    if (!isDataLoaderCompatible(parameters.getExecutionContext())) {
       DataLoaderDispatcherInstrumentationState state = parameters.getInstrumentationState();
       state.setAggressivelyBatching(false);
     }
     return new SimpleInstrumentationContext<>();
   }
 
-  private boolean isDataLoaderCompatibleExecution(ExecutionContext executionContext) {
+  private boolean isDataLoaderCompatible(ExecutionContext executionContext) {
     //
     // currently we only support Query operations and ONLY with AsyncExecutionStrategy as the query ES
     // This may change in the future but this is the fix for now
@@ -121,10 +118,12 @@ public ExecutionStrategyInstrumentationContext beginExecutionStrategy(
       return new ExecutionStrategyInstrumentationContext() {
         @Override
         public void onDispatched(CompletableFuture<ExecutionResult> result) {
+          // default empty implementation
         }
 
         @Override
         public void onCompleted(ExecutionResult result, Throwable t) {
+          // default empty implementation
         }
       };
 
@@ -157,7 +156,7 @@ public CompletableFuture<ExecutionResult> instrumentExecutionResult(
       Map<Object, Object> currentExt = executionResult.getExtensions();
       Map<Object, Object> statsMap = new LinkedHashMap<>(
           currentExt == null ? Collections.emptyMap() : currentExt);
-      Map<Object, Object> dataLoaderStats = buildStatsMap(state);
+      Map<Object, Object> dataLoaderStats = buildStatisticsMap(state);
       statsMap.put("dataloader", dataLoaderStats);
 
       log.debug("Data loader stats : {}", dataLoaderStats);
@@ -169,7 +168,7 @@ public CompletableFuture<ExecutionResult> instrumentExecutionResult(
     }
   }
 
-  private Map<Object, Object> buildStatsMap(DataLoaderDispatcherInstrumentationState state) {
+  private Map<Object, Object> buildStatisticsMap(DataLoaderDispatcherInstrumentationState state) {
     DataLoaderRegistry dataLoaderRegistry = state.getDataLoaderRegistry();
     Statistics allStats = dataLoaderRegistry.getStatistics();
     Map<Object, Object> statsMap = new LinkedHashMap<>();