Refactor response caching

Author: oliemansm

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

@@ -7,7 +7,6 @@
 import graphql.kickstart.execution.GraphQLQueryInvoker;
 import graphql.kickstart.execution.GraphQLRequest;
 import graphql.kickstart.execution.input.GraphQLSingleInvocationInput;
-import graphql.kickstart.servlet.cache.CachingHttpRequestHandlerImpl;
 import graphql.kickstart.servlet.core.GraphQLMBean;
 import graphql.kickstart.servlet.core.GraphQLServletListener;
 import graphql.kickstart.servlet.input.GraphQLInvocationInputFactory;
@@ -65,17 +64,10 @@ public AbstractGraphQLHttpServlet(List<GraphQLServletListener> listeners) {
   @Deprecated
   protected abstract GraphQLObjectMapper getGraphQLObjectMapper();
 
-  /**
-   * @deprecated override {@link #getConfiguration()} instead
-   */
-  @Deprecated
-  protected abstract boolean isAsyncServletMode();
-
   protected GraphQLConfiguration getConfiguration() {
     return GraphQLConfiguration.with(getInvocationInputFactory())
         .with(getQueryInvoker())
         .with(getGraphQLObjectMapper())
-        .with(isAsyncServletMode())
         .with(listeners)
         .build();
   }
@@ -84,11 +76,7 @@ protected GraphQLConfiguration getConfiguration() {
   public void init() {
     if (configuration == null) {
       this.configuration = getConfiguration();
-      if (configuration.getResponseCacheManager() != null) {
-        this.requestHandler = new CachingHttpRequestHandlerImpl(configuration);
-      } else {
-        this.requestHandler = HttpRequestHandlerFactory.create(configuration);
-      }
+      this.requestHandler = configuration.createHttpRequestHandler();
     }
   }
 

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

@@ -21,9 +21,4 @@ protected GraphQLObjectMapper getGraphQLObjectMapper() {
     return GraphQLObjectMapper.newBuilder().build();
   }
 
-  @Override
-  protected boolean isAsyncServletMode() {
-    return false;
-  }
-
 }

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

@@ -4,21 +4,19 @@
 import graphql.kickstart.execution.GraphQLObjectMapper;
 import graphql.kickstart.execution.GraphQLQueryInvoker;
 import graphql.kickstart.execution.context.ContextSetting;
+import graphql.kickstart.servlet.cache.CachingHttpRequestInvoker;
 import graphql.kickstart.servlet.cache.GraphQLResponseCacheManager;
 import graphql.kickstart.servlet.config.DefaultGraphQLSchemaServletProvider;
 import graphql.kickstart.servlet.config.GraphQLSchemaServletProvider;
 import graphql.kickstart.servlet.context.GraphQLServletContextBuilder;
 import graphql.kickstart.servlet.core.GraphQLServletListener;
 import graphql.kickstart.servlet.core.GraphQLServletRootObjectBuilder;
-import graphql.kickstart.servlet.core.internal.GraphQLThreadFactory;
 import graphql.kickstart.servlet.input.BatchInputPreProcessor;
 import graphql.kickstart.servlet.input.GraphQLInvocationInputFactory;
 import graphql.kickstart.servlet.input.NoOpBatchInputPreProcessor;
 import graphql.schema.GraphQLSchema;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.Executor;
-import java.util.concurrent.Executors;
 import java.util.function.Supplier;
 import lombok.Getter;
 
@@ -30,19 +28,6 @@ public class GraphQLConfiguration {
   private final GraphQLInvoker graphQLInvoker;
   private final GraphQLObjectMapper objectMapper;
   private final List<GraphQLServletListener> listeners;
-  /**
-   * For removal
-   * @since 10.1.0
-   */
-  @Deprecated
-  private final boolean asyncServletModeEnabled;
-  /**
-   * For removal
-   * @since 10.1.0
-   */
-  @Deprecated
-  private final Executor asyncExecutor;
-
   private final long subscriptionTimeout;
   @Getter
   private final long asyncTimeout;
@@ -52,8 +37,7 @@ public class GraphQLConfiguration {
   private GraphQLConfiguration(GraphQLInvocationInputFactory invocationInputFactory,
       GraphQLQueryInvoker queryInvoker,
       GraphQLObjectMapper objectMapper, List<GraphQLServletListener> listeners,
-      boolean asyncServletModeEnabled,
-      Executor asyncExecutor, long subscriptionTimeout, long asyncTimeout,
+      long subscriptionTimeout, long asyncTimeout,
       ContextSetting contextSetting,
       Supplier<BatchInputPreProcessor> batchInputPreProcessor,
       GraphQLResponseCacheManager responseCacheManager) {
@@ -62,8 +46,6 @@ private GraphQLConfiguration(GraphQLInvocationInputFactory invocationInputFactor
     this.graphQLInvoker = queryInvoker.toGraphQLInvoker();
     this.objectMapper = objectMapper;
     this.listeners = listeners;
-    this.asyncServletModeEnabled = asyncServletModeEnabled;
-    this.asyncExecutor = asyncExecutor;
     this.subscriptionTimeout = subscriptionTimeout;
     this.asyncTimeout = asyncTimeout;
     this.contextSetting = contextSetting;
@@ -104,14 +86,6 @@ public List<GraphQLServletListener> getListeners() {
     return new ArrayList<>(listeners);
   }
 
-  public boolean isAsyncServletModeEnabled() {
-    return asyncServletModeEnabled;
-  }
-
-  public Executor getAsyncExecutor() {
-    return asyncExecutor;
-  }
-
   public void add(GraphQLServletListener listener) {
     listeners.add(listener);
   }
@@ -136,15 +110,21 @@ public GraphQLResponseCacheManager getResponseCacheManager() {
     return responseCacheManager;
   }
 
+  public HttpRequestHandler createHttpRequestHandler() {
+    HttpRequestHandler requestHandler = HttpRequestHandlerFactory.create(this);
+    if (responseCacheManager == null) {
+      return requestHandler;
+    }
+    return new HttpRequestHandlerImpl(this, new CachingHttpRequestInvoker(this));
+  }
+
   public static class Builder {
 
     private GraphQLInvocationInputFactory.Builder invocationInputFactoryBuilder;
     private GraphQLInvocationInputFactory invocationInputFactory;
     private GraphQLQueryInvoker queryInvoker = GraphQLQueryInvoker.newBuilder().build();
     private GraphQLObjectMapper objectMapper = GraphQLObjectMapper.newBuilder().build();
     private List<GraphQLServletListener> listeners = new ArrayList<>();
-    private boolean asyncServletModeEnabled = false;
-    private Executor asyncExecutor = Executors.newCachedThreadPool(new GraphQLThreadFactory());
     private long subscriptionTimeout = 0;
     private long asyncTimeout = 30000;
     private ContextSetting contextSetting = ContextSetting.PER_QUERY_WITH_INSTRUMENTATION;
@@ -180,18 +160,6 @@ public Builder with(List<GraphQLServletListener> listeners) {
       return this;
     }
 
-    public Builder with(boolean asyncServletModeEnabled) {
-      this.asyncServletModeEnabled = asyncServletModeEnabled;
-      return this;
-    }
-
-    public Builder with(Executor asyncExecutor) {
-      if (asyncExecutor != null) {
-        this.asyncExecutor = asyncExecutor;
-      }
-      return this;
-    }
-
     public Builder with(GraphQLServletContextBuilder contextBuilder) {
       this.invocationInputFactoryBuilder.withGraphQLContextBuilder(contextBuilder);
       return this;
@@ -245,8 +213,6 @@ public GraphQLConfiguration build() {
           queryInvoker,
           objectMapper,
           listeners,
-          asyncServletModeEnabled,
-          asyncExecutor,
           subscriptionTimeout,
           asyncTimeout,
           contextSetting,

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

@@ -36,9 +36,4 @@ protected GraphQLObjectMapper getGraphQLObjectMapper() {
     throw new UnsupportedOperationException();
   }
 
-  @Override
-  protected boolean isAsyncServletMode() {
-    throw new UnsupportedOperationException();
-  }
-
 }

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

@@ -10,6 +10,7 @@ public interface HttpRequestHandler {
 
   int STATUS_OK = 200;
   int STATUS_BAD_REQUEST = 400;
+  int STATUS_INTERNAL_SERVER_ERROR = 500;
 
   void handle(HttpServletRequest request, HttpServletResponse response) throws Exception;
 

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

@@ -2,30 +2,27 @@
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import graphql.GraphQLException;
-import graphql.kickstart.execution.GraphQLInvoker;
-import graphql.kickstart.execution.GraphQLQueryResult;
-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.concurrent.CompletableFuture;
-import javax.servlet.AsyncContext;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import lombok.extern.slf4j.Slf4j;
 
 @Slf4j
-public class HttpRequestHandlerImpl implements HttpRequestHandler {
+class HttpRequestHandlerImpl implements HttpRequestHandler {
 
   private final GraphQLConfiguration configuration;
-  private final GraphQLInvoker graphQLInvoker;
+  private final HttpRequestInvoker requestInvoker;
 
   public HttpRequestHandlerImpl(GraphQLConfiguration configuration) {
+    this(configuration, new HttpRequestInvokerImpl(configuration, configuration.getGraphQLInvoker(),
+        new QueryResponseWriterFactoryImpl()));
+  }
+
+  public HttpRequestHandlerImpl(GraphQLConfiguration configuration,
+      HttpRequestInvoker requestInvoker) {
     this.configuration = configuration;
-    graphQLInvoker = configuration.getGraphQLInvoker();
+    this.requestInvoker = requestInvoker;
   }
 
   @Override
@@ -39,79 +36,16 @@ public void handle(HttpServletRequest request, HttpServletResponse response) thr
       );
       GraphQLInvocationInput invocationInput = invocationInputParser
           .getGraphQLInvocationInput(request, response);
-      execute(invocationInput, request, response);
-    } catch (GraphQLException| JsonProcessingException e) {
+      requestInvoker.execute(invocationInput, request, response);
+    } catch (GraphQLException | JsonProcessingException e) {
       response.setStatus(STATUS_BAD_REQUEST);
       log.info("Bad request: cannot handle http request", e);
       throw e;
     } catch (Exception t) {
-      response.setStatus(500);
+      response.setStatus(STATUS_INTERNAL_SERVER_ERROR);
       log.error("Cannot handle http request", t);
       throw t;
     }
   }
 
-  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(invocationInput, result, request, response))
-          .exceptionally(t -> writeErrorResponse(t, 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);
-      }
-    }
-  }
-
-  private void writeResultResponse(GraphQLInvocationInput invocationInput, GraphQLQueryResult queryResult, HttpServletRequest request,
-      HttpServletResponse response) {
-    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);
-    return null;
-  }
-
-  private CompletableFuture<GraphQLQueryResult> invoke(GraphQLInvocationInput invocationInput, HttpServletRequest request,
-      HttpServletResponse response) {
-    if (invocationInput instanceof GraphQLSingleInvocationInput) {
-      return graphQLInvoker.queryAsync(invocationInput);
-    }
-    return invokeBatched((GraphQLBatchedInvocationInput) invocationInput, request, response);
-  }
-
-  private CompletableFuture<GraphQLQueryResult> 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 CompletableFuture.completedFuture(GraphQLQueryResult.createError(result.getStatusCode(), result.getStatusMessage()));
-  }
-
 }

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

@@ -0,0 +1,12 @@
+package graphql.kickstart.servlet;
+
+import graphql.kickstart.execution.input.GraphQLInvocationInput;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+public interface HttpRequestInvoker {
+
+  void execute(GraphQLInvocationInput invocationInput, HttpServletRequest request,
+      HttpServletResponse response);
+
+}