Merge pull request #338 from graphql-java-kickstart/bugfix/293-async-call-listeners

fix(#293): move listener callbacks into new invoker

Author: oliemansm

.idea/codeStyles/Project.xml

@@ -8,11 +8,6 @@
 import graphql.kickstart.servlet.core.GraphQLMBean;
 import graphql.kickstart.servlet.core.GraphQLServletListener;
 import graphql.schema.GraphQLFieldDefinition;
-import java.util.List;
-import java.util.Objects;
-import java.util.function.Consumer;
-import java.util.function.Function;
-import java.util.stream.Collectors;
 import javax.servlet.Servlet;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
@@ -85,43 +80,10 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) {
   }
 
   private void doRequest(HttpServletRequest request, HttpServletResponse response) {
-    List<GraphQLServletListener.RequestCallback> requestCallbacks =
-        runListeners(l -> l.onRequest(request, response));
-
     try {
       getConfiguration().getHttpRequestHandler().handle(request, response);
-      runCallbacks(requestCallbacks, c -> c.onSuccess(request, response));
     } catch (Exception t) {
       log.error("Error executing GraphQL request!", t);
-      runCallbacks(requestCallbacks, c -> c.onError(request, response, t));
-    } finally {
-      runCallbacks(requestCallbacks, c -> c.onFinally(request, response));
     }
   }
-
-  private <R> List<R> runListeners(Function<? super GraphQLServletListener, R> action) {
-    return getConfiguration().getListeners().stream()
-        .map(
-            listener -> {
-              try {
-                return action.apply(listener);
-              } catch (Exception t) {
-                log.error("Error running listener: {}", listener, t);
-                return null;
-              }
-            })
-        .filter(Objects::nonNull)
-        .collect(Collectors.toList());
-  }
-
-  private <T> void runCallbacks(List<T> callbacks, Consumer<T> action) {
-    callbacks.forEach(
-        callback -> {
-          try {
-            action.accept(callback);
-          } catch (Exception t) {
-            log.error("Error running callback: {}", callback, t);
-          }
-        });
-  }
 }

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

@@ -37,28 +37,37 @@ public void execute(
               ? request.getAsyncContext()
               : request.startAsync(request, response);
       asyncContext.setTimeout(configuration.getAsyncTimeout());
-      invoke(invocationInput, request, response)
-          .thenAccept(result -> writeResultResponse(invocationInput, result, request, response))
-          .exceptionally(t -> writeErrorResponse(t, response))
+      invokeAndHandle(invocationInput, request, response)
           .thenAccept(aVoid -> asyncContext.complete());
     } else {
-      try {
-        GraphQLQueryResult result = invoke(invocationInput, request, response).join();
-        writeResultResponse(invocationInput, result, request, response);
-      } catch (Exception t) {
-        writeErrorResponse(t, response);
-      }
+      invokeAndHandle(invocationInput, request, response).join();
     }
   }
 
+  private CompletableFuture<Void> invokeAndHandle(
+      GraphQLInvocationInput invocationInput,
+      HttpServletRequest request,
+      HttpServletResponse response) {
+    ListenerHandler listenerHandler =
+        ListenerHandler.start(request, response, configuration.getListeners());
+    return invoke(invocationInput, request, response)
+        .thenAccept(
+            result ->
+                writeResultResponse(invocationInput, result, request, response, listenerHandler))
+        .exceptionally(t -> writeErrorResponse(t, response, listenerHandler))
+        .thenAccept(aVoid -> listenerHandler.onFinally());
+  }
+
   private void writeResultResponse(
       GraphQLInvocationInput invocationInput,
       GraphQLQueryResult queryResult,
       HttpServletRequest request,
-      HttpServletResponse response) {
+      HttpServletResponse response,
+      ListenerHandler listenerHandler) {
     QueryResponseWriter queryResponseWriter = createWriter(invocationInput, queryResult);
     try {
       queryResponseWriter.write(request, response);
+      listenerHandler.onSuccess();
     } catch (IOException e) {
       throw new UncheckedIOException(e);
     }
@@ -69,10 +78,12 @@ protected QueryResponseWriter createWriter(
     return queryResponseWriterFactory.createWriter(invocationInput, queryResult, configuration);
   }
 
-  private Void writeErrorResponse(Throwable t, HttpServletResponse response) {
+  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;
   }
 

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

@@ -0,0 +1,74 @@
+package graphql.kickstart.servlet;
+
+import static java.util.Collections.emptyList;
+
+import graphql.kickstart.servlet.core.GraphQLServletListener;
+import graphql.kickstart.servlet.core.GraphQLServletListener.RequestCallback;
+import java.util.List;
+import java.util.Objects;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+
+@Slf4j
+@RequiredArgsConstructor
+class ListenerHandler {
+
+  private final List<RequestCallback> callbacks;
+  private final HttpServletRequest request;
+  private final HttpServletResponse response;
+
+  static ListenerHandler start(
+      HttpServletRequest request,
+      HttpServletResponse response,
+      List<GraphQLServletListener> listeners) {
+    if (listeners != null) {
+      return new ListenerHandler(
+          runListeners(listeners, it -> it.onRequest(request, response)), request, response);
+    }
+    return new ListenerHandler(emptyList(), request, response);
+  }
+
+  private static <R> List<R> runListeners(
+      List<GraphQLServletListener> listeners, Function<? super GraphQLServletListener, R> action) {
+    return listeners.stream()
+        .map(
+            listener -> {
+              try {
+                return action.apply(listener);
+              } catch (Exception t) {
+                log.error("Error running listener: {}", listener, t);
+                return null;
+              }
+            })
+        .filter(Objects::nonNull)
+        .collect(Collectors.toList());
+  }
+
+  void runCallbacks(Consumer<RequestCallback> action) {
+    callbacks.forEach(
+        callback -> {
+          try {
+            action.accept(callback);
+          } catch (Exception t) {
+            log.error("Error running callback: {}", callback, t);
+          }
+        });
+  }
+
+  void onSuccess() {
+    runCallbacks(it -> it.onSuccess(request, response));
+  }
+
+  void onError(Throwable throwable) {
+    runCallbacks(it -> it.onError(request, response, throwable));
+  }
+
+  void onFinally() {
+    runCallbacks(it -> it.onFinally(request, response));
+  }
+}