Bugfix/469 apply async timeout

.github/workflows/pull-request.yml

@@ -11,33 +11,6 @@ jobs:
       - uses: actions/checkout@v2
       - uses: gradle/wrapper-validation-action@v1
 
-  verify-google-java-format:
-    name: Google Java Format Verification
-    runs-on: ubuntu-latest
-    needs: validation
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v2
-      - name: Setup Java
-        uses: actions/setup-java@v2
-        with:
-          distribution: 'zulu'
-          java-version: 15
-      - name: Cache Gradle
-        uses: actions/cache@v2
-        env:
-          java-version: 15
-        with:
-          path: |
-            ~/.gradle/caches
-            ~/.gradle/wrapper
-          key: ${{ runner.os }}-${{ env.java-version }}-gradle-${{ hashFiles('**/*.gradle*') }}
-          restore-keys: ${{ runner.os }}-${{ env.java-version }}-gradle-
-      - name: Make gradlew executable
-        run: chmod +x ./gradlew
-      - name: Gradle Check
-        run: ./gradlew --info build -x test
-
   test:
     name: Test run
     strategy:
@@ -70,11 +43,11 @@ jobs:
         run: chmod +x ./gradlew
       - name: Gradle Check (non-Windows)
         if: matrix.os != 'windows-latest'
-        run: ./gradlew --info check -x verifyGoogleJavaFormat
+        run: ./gradlew --info check
       - name: Gradle Check (Windows)
         if: matrix.os == 'windows-latest'
         shell: cmd
-        run: gradlew --info check -x verifyGoogleJavaFormat
+        run: gradlew --info check
 
   build:
     name: Sonar analysis
@@ -112,4 +85,4 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}  # Needed to get PR information, if any
           SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
-        run: ./gradlew build jacocoTestReport sonarqube --info -x verifyGoogleJavaFormat
+        run: ./gradlew build jacocoTestReport sonarqube --info

.github/workflows/release.yml

@@ -35,7 +35,7 @@ jobs:
       - name: Make gradlew executable
         run: chmod +x ./gradlew
       - name: Gradle Check
-        run: ./gradlew --info check -x verifyGoogleJavaFormat
+        run: ./gradlew --info check
 
   build:
     name: Publish release

.github/workflows/snapshot.yml

@@ -12,33 +12,6 @@ jobs:
       - uses: actions/checkout@v2
       - uses: gradle/wrapper-validation-action@v1
 
-  verify-google-java-format:
-    name: Google Java Format Verification
-    runs-on: ubuntu-latest
-    needs: validation
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v2
-      - name: Setup Java
-        uses: actions/setup-java@v2
-        with:
-          distribution: 'zulu'
-          java-version: 15
-      - name: Cache Gradle
-        uses: actions/cache@v2
-        env:
-          java-version: 15
-        with:
-          path: |
-            ~/.gradle/caches
-            ~/.gradle/wrapper
-          key: ${{ runner.os }}-${{ env.java-version }}-gradle-${{ hashFiles('**/*.gradle*') }}
-          restore-keys: ${{ runner.os }}-${{ env.java-version }}-gradle-
-      - name: Make gradlew executable
-        run: chmod +x ./gradlew
-      - name: Gradle Check
-        run: ./gradlew --info build -x test
-
   test:
     name: Test run
     needs: validation
@@ -65,7 +38,7 @@ jobs:
       - name: Make gradlew executable
         run: chmod +x ./gradlew
       - name: Gradle Check
-        run: ./gradlew --info check -x verifyGoogleJavaFormat
+        run: ./gradlew --info check
 
   build:
     name: Publish snapshot
@@ -97,7 +70,7 @@ jobs:
         env:
           OSS_USER_TOKEN_KEY: ${{ secrets.OSS_USER_TOKEN_KEY }}
           OSS_USER_TOKEN_PASS: ${{ secrets.OSS_USER_TOKEN_PASS }}
-        run: ./gradlew clean build publish -x test -x verifyGoogleJavaFormat
+        run: ./gradlew clean build publish -x test
 
   sonar:
     name: Sonar analysis
@@ -129,4 +102,4 @@ jobs:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}  # Needed to get PR information, if any
           SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
         if: env.SONAR_TOKEN != null
-        run: ./gradlew build jacocoTestReport sonarqube --info -x verifyGoogleJavaFormat
+        run: ./gradlew build jacocoTestReport sonarqube --info

build.gradle

@@ -31,7 +31,6 @@ plugins {
     id "org.sonarqube" version "3.2.0"
     id "jacoco"
     id 'io.codearte.nexus-staging' version '0.30.0'
-    id 'com.github.sherter.google-java-format' version '0.9' apply false
 }
 
 sonarqube {
@@ -49,7 +48,6 @@ subprojects {
     apply plugin: 'java'
     apply plugin: 'maven-publish'
     apply plugin: 'signing'
-    apply plugin: 'com.github.sherter.google-java-format'
 
     repositories {
         mavenLocal()
@@ -80,8 +78,6 @@ subprojects {
 
     compileJava.dependsOn(processResources)
 
-    compileJava.mustRunAfter verifyGoogleJavaFormat
-
     test {
         useJUnitPlatform()
 

github-build.sh

@@ -45,7 +45,7 @@ git config --global user.name "GitHub Actions"
 echo "Deploying release to Maven Central"
 removeSnapshots
 
-./gradlew clean build publish closeAndReleaseRepository -x verifyGoogleJavaFormat
+./gradlew clean build publish closeAndReleaseRepository
 
 commitRelease
 bumpVersion

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

@@ -0,0 +1,26 @@
+package graphql.kickstart.execution;
+
+import graphql.ExecutionResult;
+import graphql.kickstart.execution.input.GraphQLInvocationInput;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+@RequiredArgsConstructor
+class FutureBatchedExecutionResult implements FutureExecutionResult {
+
+  @Getter
+  private final GraphQLInvocationInput invocationInput;
+  private final CompletableFuture<List<ExecutionResult>> batched;
+
+  @Override
+  public CompletableFuture<GraphQLQueryResult> thenApplyQueryResult() {
+    return batched.thenApply(GraphQLQueryResult::create);
+  }
+
+  @Override
+  public void cancel() {
+    batched.cancel(true);
+  }
+}

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

@@ -0,0 +1,26 @@
+package graphql.kickstart.execution;
+
+import graphql.kickstart.execution.input.GraphQLInvocationInput;
+import java.util.concurrent.CompletableFuture;
+import lombok.RequiredArgsConstructor;
+
+@RequiredArgsConstructor
+class FutureErrorExecutionResult implements FutureExecutionResult {
+
+  private final GraphQLErrorQueryResult errorQueryResult;
+
+  @Override
+  public CompletableFuture<GraphQLQueryResult> thenApplyQueryResult() {
+    return CompletableFuture.completedFuture(errorQueryResult);
+  }
+
+  @Override
+  public GraphQLInvocationInput getInvocationInput() {
+    return null;
+  }
+
+  @Override
+  public void cancel() {
+    // nothing to do
+  }
+}

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

@@ -0,0 +1,29 @@
+package graphql.kickstart.execution;
+
+import graphql.ExecutionResult;
+import graphql.kickstart.execution.input.GraphQLInvocationInput;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+
+public interface FutureExecutionResult {
+
+  static FutureExecutionResult single(
+      GraphQLInvocationInput invocationInput, CompletableFuture<ExecutionResult> single) {
+    return new FutureSingleExecutionResult(invocationInput, single);
+  }
+
+  static FutureExecutionResult batched(
+      GraphQLInvocationInput invocationInput, CompletableFuture<List<ExecutionResult>> batched) {
+    return new FutureBatchedExecutionResult(invocationInput, batched);
+  }
+
+  static FutureExecutionResult error(GraphQLErrorQueryResult result) {
+    return new FutureErrorExecutionResult(result);
+  }
+
+  CompletableFuture<GraphQLQueryResult> thenApplyQueryResult();
+
+  GraphQLInvocationInput getInvocationInput();
+
+  void cancel();
+}

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

@@ -0,0 +1,25 @@
+package graphql.kickstart.execution;
+
+import graphql.ExecutionResult;
+import graphql.kickstart.execution.input.GraphQLInvocationInput;
+import java.util.concurrent.CompletableFuture;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+@RequiredArgsConstructor
+class FutureSingleExecutionResult implements FutureExecutionResult {
+
+  @Getter
+  private final GraphQLInvocationInput invocationInput;
+  private final CompletableFuture<ExecutionResult> single;
+
+  @Override
+  public CompletableFuture<GraphQLQueryResult> thenApplyQueryResult() {
+    return single.thenApply(GraphQLQueryResult::create);
+  }
+
+  @Override
+  public void cancel() {
+    single.cancel(true);
+  }
+}

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

@@ -12,14 +12,25 @@
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
 
+@Slf4j
 @RequiredArgsConstructor
 public class GraphQLInvoker {
 
   private final GraphQLBuilder graphQLBuilder;
   private final BatchedDataLoaderGraphQLBuilder batchedDataLoaderGraphQLBuilder;
   private final GraphQLInvokerProxy proxy = GraphQL::executeAsync;
 
+  public FutureExecutionResult execute(GraphQLInvocationInput invocationInput) {
+    if (invocationInput instanceof GraphQLSingleInvocationInput) {
+      return FutureExecutionResult.single(
+          invocationInput, executeAsync((GraphQLSingleInvocationInput) invocationInput));
+    }
+    return FutureExecutionResult.batched(
+        invocationInput, executeAsync((GraphQLBatchedInvocationInput) invocationInput));
+  }
+
   public CompletableFuture<ExecutionResult> executeAsync(
       GraphQLSingleInvocationInput invocationInput) {
     GraphQL graphQL = graphQLBuilder.build(invocationInput.getSchema());

graphql-java-servlet/src/main/java/graphql/kickstart/servlet/AsyncTimeoutListener.java

@@ -0,0 +1,14 @@
+package graphql.kickstart.servlet;
+
+import java.io.IOException;
+import javax.servlet.AsyncEvent;
+import javax.servlet.AsyncListener;
+
+interface AsyncTimeoutListener extends AsyncListener {
+
+  default void onComplete(AsyncEvent event) throws IOException {}
+
+  default void onError(AsyncEvent event) throws IOException {}
+
+  default void onStartAsync(AsyncEvent event) throws IOException {}
+}

graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestInvokerImpl.java

@@ -1,17 +1,23 @@
 package graphql.kickstart.servlet;
 
-import static graphql.kickstart.servlet.HttpRequestHandler.STATUS_BAD_REQUEST;
-
+import graphql.ExecutionResult;
+import graphql.ExecutionResultImpl;
+import graphql.kickstart.execution.FutureExecutionResult;
 import graphql.kickstart.execution.GraphQLInvoker;
 import graphql.kickstart.execution.GraphQLQueryResult;
+import graphql.kickstart.execution.error.GenericGraphQLError;
 import graphql.kickstart.execution.input.GraphQLBatchedInvocationInput;
 import graphql.kickstart.execution.input.GraphQLInvocationInput;
 import graphql.kickstart.execution.input.GraphQLSingleInvocationInput;
 import graphql.kickstart.servlet.input.BatchInputPreProcessResult;
 import graphql.kickstart.servlet.input.BatchInputPreProcessor;
 import java.io.IOException;
 import java.io.UncheckedIOException;
+import java.util.Optional;
+import java.util.concurrent.CancellationException;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
+import java.util.concurrent.atomic.AtomicReference;
 import javax.servlet.AsyncContext;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -31,84 +37,129 @@ public void execute(
       GraphQLInvocationInput invocationInput,
       HttpServletRequest request,
       HttpServletResponse response) {
+    ListenerHandler listenerHandler =
+        ListenerHandler.start(request, response, configuration.getListeners());
     if (request.isAsyncSupported()) {
-      AsyncContext asyncContext =
-          request.isAsyncStarted()
-              ? request.getAsyncContext()
-              : request.startAsync(request, response);
-      asyncContext.setTimeout(configuration.getAsyncTimeout());
-      invokeAndHandle(invocationInput, request, response)
-          .thenAccept(aVoid -> asyncContext.complete());
+      invokeAndHandleAsync(invocationInput, request, response, listenerHandler);
     } else {
-      invokeAndHandle(invocationInput, request, response).join();
+      handle(invoke(invocationInput, request, response), request, response, listenerHandler).join();
     }
   }
 
-  private CompletableFuture<Void> invokeAndHandle(
+  private void invokeAndHandleAsync(
       GraphQLInvocationInput invocationInput,
       HttpServletRequest request,
-      HttpServletResponse response) {
-    ListenerHandler listenerHandler =
-        ListenerHandler.start(request, response, configuration.getListeners());
-    return invoke(invocationInput, request, response)
+      HttpServletResponse response,
+      ListenerHandler listenerHandler) {
+    AsyncContext asyncContext =
+        request.isAsyncStarted()
+            ? request.getAsyncContext()
+            : request.startAsync(request, response);
+    asyncContext.setTimeout(configuration.getAsyncTimeout());
+    AtomicReference<FutureExecutionResult> futureHolder = new AtomicReference<>();
+    AsyncTimeoutListener timeoutListener =
+        event -> Optional.ofNullable(futureHolder.get()).ifPresent(FutureExecutionResult::cancel);
+    asyncContext.addListener(timeoutListener);
+    asyncContext.start(
+        () -> {
+          FutureExecutionResult futureResult = invoke(invocationInput, request, response);
+          futureHolder.set(futureResult);
+          handle(futureResult, request, response, listenerHandler);
+        });
+  }
+
+  private CompletableFuture<Void> handle(
+      FutureExecutionResult futureResult,
+      HttpServletRequest request,
+      HttpServletResponse response,
+      ListenerHandler listenerHandler) {
+    return futureResult
+        .thenApplyQueryResult()
         .thenAccept(
-            result ->
-                writeResultResponse(invocationInput, result, request, response, listenerHandler))
-        .exceptionally(t -> writeErrorResponse(t, response, listenerHandler))
-        .thenAccept(aVoid -> listenerHandler.onFinally());
+            it -> writeResultResponse(futureResult.getInvocationInput(), it, request, response))
+        .thenAccept(it -> listenerHandler.onSuccess())
+        .exceptionally(
+            t ->
+                writeErrorResponse(
+                    futureResult.getInvocationInput(), request, response, listenerHandler, t))
+        .thenAccept(it -> listenerHandler.onFinally());
   }
 
   private void writeResultResponse(
       GraphQLInvocationInput invocationInput,
       GraphQLQueryResult queryResult,
       HttpServletRequest request,
-      HttpServletResponse response,
-      ListenerHandler listenerHandler) {
+      HttpServletResponse response) {
     QueryResponseWriter queryResponseWriter = createWriter(invocationInput, queryResult);
     try {
       queryResponseWriter.write(request, response);
-      listenerHandler.onSuccess();
     } catch (IOException e) {
       throw new UncheckedIOException(e);
     }
   }
 
+  private Void writeErrorResponse(
+      GraphQLInvocationInput invocationInput,
+      HttpServletRequest request,
+      HttpServletResponse response,
+      ListenerHandler listenerHandler,
+      Throwable t) {
+    Throwable cause = getCause(t);
+    if (!response.isCommitted()) {
+      writeResultResponse(
+          invocationInput, GraphQLQueryResult.create(toErrorResult(cause)), request, response);
+      listenerHandler.onError(cause);
+    } else {
+      log.warn(
+          "Cannot write GraphQL response, because the HTTP response is already committed. It most likely timed out.");
+    }
+    return null;
+  }
+
+  private Throwable getCause(Throwable t) {
+    return t instanceof CompletionException && t.getCause() != null ? t.getCause() : t;
+  }
+
+  private ExecutionResult toErrorResult(Throwable t) {
+    String message =
+        t instanceof CancellationException
+            ? "Execution canceled because timeout of "
+                + configuration.getAsyncTimeout()
+                + " millis was reached"
+            : t.getMessage();
+    if (message == null) {
+      message = "Unexpected error occurred";
+    }
+    return new ExecutionResultImpl(new GenericGraphQLError(message));
+  }
+
   protected QueryResponseWriter createWriter(
       GraphQLInvocationInput invocationInput, GraphQLQueryResult queryResult) {
     return queryResponseWriterFactory.createWriter(invocationInput, queryResult, configuration);
   }
 
-  private Void writeErrorResponse(
-      Throwable t, HttpServletResponse response, ListenerHandler listenerHandler) {
-    response.setStatus(STATUS_BAD_REQUEST);
-    log.info(
-        "Bad request: path was not \"/schema.json\" or no query variable named \"query\" given", t);
-    listenerHandler.onError(t);
-    return null;
-  }
-
-  private CompletableFuture<GraphQLQueryResult> invoke(
+  private FutureExecutionResult invoke(
       GraphQLInvocationInput invocationInput,
       HttpServletRequest request,
       HttpServletResponse response) {
     if (invocationInput instanceof GraphQLSingleInvocationInput) {
-      return graphQLInvoker.queryAsync(invocationInput);
+      return graphQLInvoker.execute(invocationInput);
     }
     return invokeBatched((GraphQLBatchedInvocationInput) invocationInput, request, response);
   }
 
-  private CompletableFuture<GraphQLQueryResult> invokeBatched(
+  private FutureExecutionResult invokeBatched(
       GraphQLBatchedInvocationInput batchedInvocationInput,
       HttpServletRequest request,
       HttpServletResponse response) {
     BatchInputPreProcessor preprocessor = configuration.getBatchInputPreProcessor();
     BatchInputPreProcessResult result =
         preprocessor.preProcessBatch(batchedInvocationInput, request, response);
     if (result.isExecutable()) {
-      return graphQLInvoker.queryAsync(result.getBatchedInvocationInput());
+      return graphQLInvoker.execute(result.getBatchedInvocationInput());
     }
 
-    return CompletableFuture.completedFuture(
+    return FutureExecutionResult.error(
         GraphQLQueryResult.createError(result.getStatusCode(), result.getStatusMessage()));
   }
 }

graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java

@@ -23,5 +23,6 @@ public void write(HttpServletRequest request, HttpServletResponse response) thro
     byte[] contentBytes = graphQLObjectMapper.serializeResultAsBytes(result);
     response.setContentLength(contentBytes.length);
     response.getOutputStream().write(contentBytes);
+    response.getOutputStream().flush();
   }
 }

graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CachingHttpRequestInvokerTest.groovy

@@ -1,12 +1,16 @@
 package graphql.kickstart.servlet.cache
 
+import graphql.ExecutionResult
+import graphql.kickstart.execution.FutureExecutionResult
 import graphql.kickstart.execution.GraphQLInvoker
+import graphql.kickstart.execution.GraphQLObjectMapper
 import graphql.kickstart.execution.GraphQLQueryResult
 import graphql.kickstart.execution.input.GraphQLSingleInvocationInput
 import graphql.kickstart.servlet.GraphQLConfiguration
 import graphql.kickstart.servlet.HttpRequestInvoker
 import spock.lang.Specification
 
+import javax.servlet.ServletOutputStream
 import javax.servlet.http.HttpServletRequest
 import javax.servlet.http.HttpServletResponse
 import java.util.concurrent.CompletableFuture
@@ -22,6 +26,8 @@ class CachingHttpRequestInvokerTest extends Specification {
   def httpRequestInvokerMock
   def graphqlInvoker
   def configuration
+  def graphqlObjectMapper
+  def outputStreamMock
 
   def setup() {
     cacheReaderMock = Mock(CacheReader)
@@ -32,11 +38,18 @@ class CachingHttpRequestInvokerTest extends Specification {
     configuration = Mock(GraphQLConfiguration)
     httpRequestInvokerMock = Mock(HttpRequestInvoker)
     graphqlInvoker = Mock(GraphQLInvoker)
+    graphqlObjectMapper = Mock(GraphQLObjectMapper)
+    outputStreamMock = Mock(ServletOutputStream)
+    graphqlInvoker.execute(invocationInputMock) >> FutureExecutionResult.single(invocationInputMock, CompletableFuture.completedFuture(Mock(GraphQLQueryResult)))
     cachingInvoker = new CachingHttpRequestInvoker(configuration, httpRequestInvokerMock, cacheReaderMock)
 
     configuration.getResponseCacheManager() >> responseCacheManagerMock
     configuration.getGraphQLInvoker() >> graphqlInvoker
+    configuration.getObjectMapper() >> graphqlObjectMapper
+    graphqlObjectMapper.serializeResultAsBytes(_ as ExecutionResult) >> new byte[0]
     graphqlInvoker.queryAsync(invocationInputMock) >> CompletableFuture.completedFuture(Mock(GraphQLQueryResult))
+
+    responseMock.getOutputStream() >> outputStreamMock
   }
 
   def "should execute regular invoker if cache not exists"() {