Rework listeners to allow context between before/after.

Author: apottere

README.md

@@ -56,42 +56,52 @@ GraphQLServlet servlet = new SimpleGraphQLServlet(schema, executionStrategy);
 GraphQLServlet servlet = new SimpleGraphQLServlet(schema, executionStrategy, operationListeners, servletListeners);
 ```
 
-## Operation and Servlet Listeners
+## Servlet Listeners
 
-You can also add [operation listeners](https://github.com/graphql-java/graphql-java-servlet/blob/master/src/main/java/graphql/servlet/GraphQLOperationListener.java) and [servlet listeners](https://github.com/graphql-java/graphql-java-servlet/blob/master/src/main/java/graphql/servlet/GraphQLServletListener.java) to an existing servlet.
-These listeners provide hooks into query execution (before, on success, and on failure) and servlet execution (before, on error, and finally):
+You can also add [servlet listeners](https://github.com/graphql-java/graphql-java-servlet/blob/master/src/main/java/graphql/servlet/GraphQLServletListener.java) to an existing servlet.
+These listeners provide hooks into query execution (before, success, failure, and finally) and servlet execution (before, success, error, and finally):
 ```java
-servlet.addOperationListener(new GraphQLOperationListener() {
+servlet.addListener(new GraphQLServletListener() {
     @Override
-    void beforeGraphQLOperation(GraphQLContext context, String operationName, String query, Map<String, Object> variables) {
+    GraphQLServletListener.RequestCallback onRequest(HttpServletRequest request, HttpServletResponse response) {
 
-    }
+        return new GraphQLServletListener.RequestCallback() {
+            @Override
+            void onSuccess(HttpServletRequest request, HttpServletResponse response) {
 
-    @Override
-    void onSuccessfulGraphQLOperation(GraphQLContext context, String operationName, String query, Map<String, Object> variables, Object data) {
+            }
 
-    }
+            @Override
+            void onError(HttpServletRequest request, HttpServletResponse response, Throwable throwable) {
 
-    @Override
-    void onFailedGraphQLOperation(GraphQLContext context, String operationName, String query, Map<String, Object> variables, Object data, List<GraphQLError> errors) {
+            }
+
+            @Override
+            void onFinally(HttpServletRequest request, HttpServletResponse response) {
 
+            }
+        }
     }
-})
 
-servlet.addServletListener(new GraphQLServletListener() {
     @Override
-    void onStart(HttpServletRequest request, HttpServletResponse response) {
+    GraphQLServletListener.OperationCallback onOperation(GraphQLContext context, String operationName, String query, Map<String, Object> variables) {
 
-    }
+        return new GraphQLServletListener.OperationCallback() {
+            @Override
+            void onSuccess(GraphQLContext context, String operationName, String query, Map<String, Object> variables, Object data) {
 
-    @Override
-    void onError(HttpServletRequest request, HttpServletResponse response, Throwable throwable) {
+            }
 
-    }
+            @Override
+            void onError(GraphQLContext context, String operationName, String query, Map<String, Object> variables, Object data, List<GraphQLError> errors) {
 
-    @Override
-    void onFinally(HttpServletRequest request, HttpServletResponse response) {
+            }
+
+            @Override
+            void onFinally(GraphQLContext context, String operationName, String query, Map<String, Object> variables, Object data) {
 
+            }
+        }
     }
 })
 ```

src/main/java/graphql/servlet/GraphQLOperationListener.java

@@ -49,12 +49,15 @@
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 /**
@@ -77,20 +80,18 @@ public abstract class GraphQLServlet extends HttpServlet implements Servlet, Gra
     protected abstract Instrumentation getInstrumentation();
     protected abstract Map<String, Object> transformVariables(GraphQLSchema schema, String query, Map<String, Object> variables);
 
-    private final List<GraphQLOperationListener> operationListeners;
-    private final List<GraphQLServletListener> servletListeners;
+    private final List<GraphQLServletListener> listeners;
     private final ServletFileUpload fileUpload;
 
     private final RequestHandler getHandler;
     private final RequestHandler postHandler;
 
     public GraphQLServlet() {
-        this(null, null, null);
+        this(null, null);
     }
 
-    public GraphQLServlet(List<GraphQLOperationListener> operationListeners, List<GraphQLServletListener> servletListeners, FileItemFactory fileItemFactory) {
-        this.operationListeners = operationListeners != null ? new ArrayList<>(operationListeners) : new ArrayList<>();
-        this.servletListeners = servletListeners != null ? new ArrayList<>(servletListeners) : new ArrayList<>();
+    public GraphQLServlet(List<GraphQLServletListener> listeners, FileItemFactory fileItemFactory) {
+        this.listeners = listeners != null ? new ArrayList<>(listeners) : new ArrayList<>();
         this.fileUpload = new ServletFileUpload(fileItemFactory != null ? fileItemFactory : new DiskFileItemFactory());
 
         this.getHandler = (request, response) -> {
@@ -188,20 +189,12 @@ public GraphQLServlet(List<GraphQLOperationListener> operationListeners, List<Gr
         };
     }
 
-    public void addOperationListener(GraphQLOperationListener operationListener) {
-        operationListeners.add(operationListener);
+    public void addListener(GraphQLServletListener servletListener) {
+        listeners.add(servletListener);
     }
 
-    public void removeOperationListener(GraphQLOperationListener operationListener) {
-        operationListeners.remove(operationListener);
-    }
-
-    public void addServletListener(GraphQLServletListener servletListener) {
-        servletListeners.add(servletListener);
-    }
-
-    public void removeServletListener(GraphQLServletListener servletListener) {
-        servletListeners.remove(servletListener);
+    public void removeListener(GraphQLServletListener servletListener) {
+        listeners.remove(servletListener);
     }
 
     @Override
@@ -225,16 +218,18 @@ public String executeQuery(String query) {
     }
 
     private void doRequest(HttpServletRequest request, HttpServletResponse response, RequestHandler handler) {
+
+        List<GraphQLServletListener.RequestCallback> requestCallbacks = runListeners(l -> l.onRequest(request, response));
+
         try {
-            runListeners(servletListeners, l -> l.onStart(request, response));
             handler.handle(request, response);
-
+            runCallbacks(requestCallbacks, c -> c.onSuccess(request, response));
         } catch (Throwable t) {
             response.setStatus(500);
             log.error("Error executing GraphQL request!", t);
-            runListeners(servletListeners, l -> l.onError(request, response, t));
+            runCallbacks(requestCallbacks, c -> c.onError(request, response, t));
         } finally {
-            runListeners(servletListeners, l -> l.onFinally(request, response));
+            runCallbacks(requestCallbacks, c -> c.onFinally(request, response));
         }
     }
 
@@ -276,7 +271,7 @@ private void query(String query, String operationName, Map<String, Object> varia
                 return null;
             });
         } else {
-            runListeners(operationListeners, l -> runListener(l, it -> it.beforeGraphQLOperation(context, operationName, query, variables)));
+            List<GraphQLServletListener.OperationCallback> operationCallbacks = runListeners(l -> l.onOperation(context, operationName, query, variables));
 
             final ExecutionResult executionResult = newGraphQL(schema).execute(query, operationName, context, transformVariables(schema, query, variables));
             final List<GraphQLError> errors = executionResult.getErrors();
@@ -289,10 +284,12 @@ private void query(String query, String operationName, Map<String, Object> varia
             resp.getWriter().write(response);
 
             if(errorsPresent(errors)) {
-                runListeners(operationListeners, l -> l.onFailedGraphQLOperation(context, operationName, query, variables, data, errors));
+                runCallbacks(operationCallbacks, c -> c.onError(context, operationName, query, variables, data, errors));
             } else {
-                runListeners(operationListeners, l -> l.onSuccessfulGraphQLOperation(context, operationName, query, variables, data));
+                runCallbacks(operationCallbacks, c -> c.onSuccess(context, operationName, query, variables, data));
             }
+
+            runCallbacks(operationCallbacks, c -> c.onFinally(context, operationName, query, variables, data));
         }
     }
 
@@ -333,21 +330,38 @@ protected boolean isClientError(GraphQLError error) {
         return error instanceof InvalidSyntaxError || error instanceof ValidationError;
     }
 
-    private <T> void runListeners(List<T> listeners, Consumer<? super T> action) {
-        if (listeners != null) {
-            listeners.forEach(l -> runListener(l, action));
+    private <R> List<R> runListeners(Function<? super GraphQLServletListener, R> action) {
+        if (listeners == null) {
+            return Collections.emptyList();
         }
+
+        return listeners.stream()
+            .map(listener -> {
+                try {
+                    return action.apply(listener);
+                } catch (Throwable 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 (Throwable t) {
+                log.error("Error running callback: {}", callback, t);
+            }
+        });
     }
 
     /**
      * Don't let listener errors escape to the client.
      */
-    private <T> void runListener(T listener, Consumer<? super T> action) {
-        try {
-            action.accept(listener);
-        } catch (Throwable t) {
-            log.error("Error running listener: {}", listener.getClass().getName(), t);
-        }
+    private <T, R> void runListener(T listener, Function<? super T, ? super R> action) {
     }
 
     protected static class VariablesDeserializer extends JsonDeserializer<Map<String, Object>> {

src/main/java/graphql/servlet/GraphQLServlet.java

@@ -14,14 +14,33 @@
  */
 package graphql.servlet;
 
+import graphql.GraphQLError;
+
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import java.util.List;
+import java.util.Map;
 
 /**
  * @author Andrew Potter
  */
 public interface GraphQLServletListener {
-    default void onStart(HttpServletRequest request, HttpServletResponse response) {}
-    default void onError(HttpServletRequest request, HttpServletResponse response, Throwable throwable) {}
-    default void onFinally(HttpServletRequest request, HttpServletResponse response) {}
+    default RequestCallback onRequest(HttpServletRequest request, HttpServletResponse response) {
+        return null;
+    }
+    default OperationCallback onOperation(GraphQLContext context, String operationName, String query, Map<String, Object> variables) {
+        return null;
+    }
+
+    interface RequestCallback {
+        default void onSuccess(HttpServletRequest request, HttpServletResponse response) {}
+        default void onError(HttpServletRequest request, HttpServletResponse response, Throwable throwable) {}
+        default void onFinally(HttpServletRequest request, HttpServletResponse response) {}
+    }
+
+    interface OperationCallback {
+        default void onSuccess(GraphQLContext context, String operationName, String query, Map<String, Object> variables, Object data) {}
+        default void onError(GraphQLContext context, String operationName, String query, Map<String, Object> variables, Object data, List<GraphQLError> errors) {}
+        default void onFinally(GraphQLContext context, String operationName, String query, Map<String, Object> variables, Object data) {}
+    }
 }

src/main/java/graphql/servlet/GraphQLServletListener.java

@@ -170,13 +170,4 @@ protected Instrumentation getInstrumentation() {
     protected Map<String, Object> transformVariables(GraphQLSchema schema, String query, Map<String, Object> variables) {
         return new GraphQLVariables(schema, query, variables);
     }
-
-    @Reference(cardinality = ReferenceCardinality.MULTIPLE, policyOption = ReferencePolicyOption.GREEDY)
-    public void bindOperationListener(GraphQLOperationListener listener) {
-        addOperationListener(listener);
-    }
-
-    public void unbindOperationListener(GraphQLOperationListener listener) {
-        removeOperationListener(listener);
-    }
 }

src/main/java/graphql/servlet/OsgiGraphQLServlet.java

@@ -15,6 +15,7 @@
 package graphql.servlet
 
 import com.fasterxml.jackson.databind.ObjectMapper
+import graphql.GraphQLError
 import graphql.Scalars
 import graphql.execution.SimpleExecutionStrategy
 import graphql.schema.DataFetcher
@@ -25,6 +26,9 @@ import org.springframework.mock.web.MockHttpServletResponse
 import spock.lang.Shared
 import spock.lang.Specification
 
+import javax.servlet.http.HttpServletRequest
+import javax.servlet.http.HttpServletResponse
+
 /**
  * @author Andrew Potter
  */