Fix unit tests for async support

Author: oliemansm

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

@@ -44,45 +44,49 @@ public void handle(HttpServletRequest request, HttpServletResponse response) thr
       response.setStatus(STATUS_BAD_REQUEST);
       log.info("Bad request: cannot handle http request", e);
       throw e;
-    } catch (Throwable t) {
+    } catch (Exception t) {
       response.setStatus(500);
       log.error("Cannot handle http request", t);
       throw t;
     }
   }
 
-  private void execute(GraphQLInvocationInput invocationInput, HttpServletRequest request,
-      HttpServletResponse response) {
+  protected void execute(GraphQLInvocationInput invocationInput, HttpServletRequest request,
+      HttpServletResponse response) throws IOException {
     if (request.isAsyncSupported()) {
       AsyncContext asyncContext = request.isAsyncStarted()
           ? request.getAsyncContext()
           : request.startAsync(request, response);
       asyncContext.setTimeout(configuration.getAsyncTimeout());
       invoke(invocationInput, request, response)
-          .thenAccept(result -> writeResultResponse(result, request, response))
+          .thenAccept(result -> writeResultResponse(invocationInput, result, request, response))
           .exceptionally(t -> writeErrorResponse(t, response))
           .thenAccept(aVoid -> asyncContext.complete());
     } else {
       try {
         GraphQLQueryResult result = invoke(invocationInput, request, response).join();
-        writeResultResponse(result, request, response);
-      } catch (Throwable t) {
+        writeResultResponse(invocationInput, result, request, response);
+      } catch (Exception t) {
         writeErrorResponse(t, response);
       }
     }
   }
 
-  private void writeResultResponse(GraphQLQueryResult queryResult, HttpServletRequest request,
+  private void writeResultResponse(GraphQLInvocationInput invocationInput, GraphQLQueryResult queryResult, HttpServletRequest request,
       HttpServletResponse response) {
-    QueryResponseWriter queryResponseWriter = QueryResponseWriter.createWriter(queryResult, configuration.getObjectMapper(),
-        configuration.getSubscriptionTimeout());
+    QueryResponseWriter queryResponseWriter = createWriter(invocationInput, queryResult);
     try {
       queryResponseWriter.write(request, response);
     } catch (IOException e) {
       throw new UncheckedIOException(e);
     }
   }
 
+  protected QueryResponseWriter createWriter(GraphQLInvocationInput invocationInput, GraphQLQueryResult queryResult) {
+    return QueryResponseWriter.createWriter(queryResult, configuration.getObjectMapper(),
+        configuration.getSubscriptionTimeout());
+  }
+
   private Void writeErrorResponse(Throwable t, HttpServletResponse response) {
     response.setStatus(STATUS_BAD_REQUEST);
     log.info("Bad GET request: path was not \"/schema.json\" or no query variable named \"query\" given", t);

graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java

@@ -44,6 +44,7 @@ protected void execute(GraphQLInvocationInput invocationInput, HttpServletReques
     }
   }
 
+  @Override
   protected QueryResponseWriter createWriter(GraphQLInvocationInput invocationInput,
       GraphQLQueryResult queryResult) {
     return CachingQueryResponseWriter.createCacheWriter(

graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/AbstractGraphQLHttpServletSpec.groovy

@@ -127,7 +127,7 @@ class AbstractGraphQLHttpServletSpec extends Specification {
         getResponseContent().data.echo == "special char á"
     }
 
-    def "async query over HTTP GET starts async request"() {
+    def "disabling async support on request over HTTP GET does not start async request"() {
         setup:
         servlet = TestUtils.createDefaultServlet({ env -> env.arguments.arg }, { env -> env.arguments.arg }, { env ->
             AtomicReference<SingleSubscriberPublisher<String>> publisherRef = new AtomicReference<>();
@@ -138,12 +138,13 @@ class AbstractGraphQLHttpServletSpec extends Specification {
             return publisherRef.get()
         }, true)
         request.addParameter('query', 'query { echo(arg:"test") }')
+        request.setAsyncSupported(false)
 
         when:
         servlet.doGet(request, response)
 
         then:
-        request.asyncStarted == true
+        request.asyncContext == null
     }
 
     def "query over HTTP GET with variables returns data"() {
@@ -442,7 +443,7 @@ class AbstractGraphQLHttpServletSpec extends Specification {
         getResponseContent().data.echo == "test"
     }
 
-    def "async query over HTTP POST starts async request"() {
+    def "disabling async support on request over HTTP POST does not start async request"() {
         setup:
         servlet = TestUtils.createDefaultServlet({ env -> env.arguments.arg }, { env -> env.arguments.arg }, { env ->
             AtomicReference<SingleSubscriberPublisher<String>> publisherRef = new AtomicReference<>();
@@ -455,12 +456,13 @@ class AbstractGraphQLHttpServletSpec extends Specification {
         request.setContent(mapper.writeValueAsBytes([
                 query: 'query { echo(arg:"test") }'
         ]))
+        request.setAsyncSupported(false)
 
         when:
         servlet.doPost(request, response)
 
         then:
-        request.asyncStarted == true
+        request.asyncContext == null
     }
 
     def "query over HTTP POST body with graphql contentType returns data"() {

graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/TestUtils.groovy

@@ -5,6 +5,9 @@ import graphql.Directives
 import graphql.Scalars
 import graphql.execution.reactive.SingleSubscriberPublisher
 import graphql.kickstart.execution.context.ContextSetting
+import graphql.kickstart.servlet.apollo.ApolloScalars
+import graphql.kickstart.servlet.context.GraphQLServletContextBuilder
+import graphql.kickstart.servlet.input.BatchInputPreProcessor
 import graphql.schema.*
 import graphql.schema.idl.RuntimeWiring
 import graphql.schema.idl.SchemaGenerator
@@ -46,7 +49,7 @@ class TestUtils {
                                         DataFetcher fieldDataFetcher = { env -> env.arguments.arg },
                                         DataFetcher otherDataFetcher,
                                         boolean asyncServletModeEnabled = false, ContextSetting contextSetting,
-                                        graphql.kickstart.servlet.context.GraphQLServletContextBuilder contextBuilder) {
+                                        GraphQLServletContextBuilder contextBuilder) {
         GraphQLSchema schema = createGraphQlSchemaWithTwoLevels(queryDataFetcher, fieldDataFetcher, otherDataFetcher)
         GraphQLHttpServlet servlet = GraphQLHttpServlet.with(GraphQLConfiguration
                 .with(schema)
@@ -68,15 +71,15 @@ class TestUtils {
                                          }))
                                          return publisherRef.get()
                                      }, boolean asyncServletModeEnabled = false,
-                                     graphql.kickstart.servlet.input.BatchInputPreProcessor batchHandler) {
+                                     BatchInputPreProcessor batchHandler) {
         GraphQLHttpServlet servlet = GraphQLHttpServlet.with(
                 graphQLConfiguration(createGraphQlSchema(queryDataFetcher, mutationDataFetcher, subscriptionDataFetcher),
                         batchHandler, asyncServletModeEnabled))
         servlet.init(null)
         return servlet
     }
 
-    static def graphQLConfiguration(GraphQLSchema schema, graphql.kickstart.servlet.input.BatchInputPreProcessor batchInputPreProcessor,
+    static def graphQLConfiguration(GraphQLSchema schema, BatchInputPreProcessor batchInputPreProcessor,
                                     boolean asyncServletModeEnabled) {
         def configBuilder = GraphQLConfiguration.with(schema).with(asyncServletModeEnabled)
         if (batchInputPreProcessor != null) {
@@ -159,7 +162,7 @@ class TestUtils {
                     field.type(Scalars.GraphQLString)
                     field.argument { argument ->
                         argument.name("file")
-                        argument.type(graphql.kickstart.servlet.apollo.ApolloScalars.Upload)
+                        argument.type(ApolloScalars.Upload)
                     }
                     field.dataFetcher({ env -> new String(ByteStreams.toByteArray(env.arguments.file.getInputStream())) })
                 }
@@ -168,7 +171,7 @@ class TestUtils {
                     field.type(GraphQLList.list(Scalars.GraphQLString))
                     field.argument { argument ->
                         argument.name("files")
-                        argument.type(GraphQLList.list(GraphQLNonNull.nonNull(graphql.kickstart.servlet.apollo.ApolloScalars.Upload)))
+                        argument.type(GraphQLList.list(GraphQLNonNull.nonNull(ApolloScalars.Upload)))
                     }
                     field.dataFetcher({ env ->
                         env.arguments.files.collect {
@@ -196,7 +199,7 @@ class TestUtils {
                 .query(query)
                 .mutation(mutation)
                 .subscription(subscription)
-                .additionalType(graphql.kickstart.servlet.apollo.ApolloScalars.Upload)
+                .additionalType(ApolloScalars.Upload)
                 .additionalDirective(Directives.DeferDirective)
                 .build()
     }