From 4d4020d5cfc85ba0486432b5ef72dc07317704f0 Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Thu, 23 Apr 2020 19:17:56 +0800 Subject: [PATCH 01/13] Added ability to cache the whole graphql response. --- .../servlet/BatchedQueryResponseWriter.java | 25 +++++-- .../servlet/ErrorQueryResponseWriter.java | 17 ++++- .../servlet/GraphQLConfiguration.java | 18 ++++- .../kickstart/servlet/HttpRequestHandler.java | 2 +- .../servlet/HttpRequestHandlerImpl.java | 37 ++++++++-- .../servlet/QueryResponseWriter.java | 16 +++-- ...SingleAsynchronousQueryResponseWriter.java | 12 +++- .../servlet/SingleQueryResponseWriter.java | 18 ++++- .../servlet/cache/CacheResponseWriter.java | 25 +++++++ .../servlet/cache/CachedResponse.java | 72 +++++++++++++++++++ .../servlet/cache/GraphQLResponseCache.java | 25 +++++++ 11 files changed, 244 insertions(+), 23 deletions(-) create mode 100644 graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheResponseWriter.java create mode 100644 graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachedResponse.java create mode 100644 graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java index 0cfcb733..f8dedef3 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java @@ -2,22 +2,29 @@ import graphql.ExecutionResult; import graphql.kickstart.execution.GraphQLObjectMapper; +import graphql.kickstart.execution.input.GraphQLInvocationInput; +import graphql.kickstart.servlet.cache.CachedResponse; +import graphql.kickstart.servlet.cache.GraphQLResponseCache; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Iterator; import java.util.List; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import lombok.RequiredArgsConstructor; +@Slf4j @RequiredArgsConstructor class BatchedQueryResponseWriter implements QueryResponseWriter { private final List results; private final GraphQLObjectMapper graphQLObjectMapper; + private final GraphQLInvocationInput invocationInput; @Override - public void write(HttpServletRequest request, HttpServletResponse response) throws IOException { + public void write(HttpServletRequest request, HttpServletResponse response, GraphQLResponseCache responseCache) throws IOException { response.setContentType(HttpRequestHandler.APPLICATION_JSON_UTF8); response.setStatus(HttpRequestHandler.STATUS_OK); @@ -34,6 +41,16 @@ public void write(HttpServletRequest request, HttpServletResponse response) thro String responseContent = responseBuilder.toString(); byte[] contentBytes = responseContent.getBytes(StandardCharsets.UTF_8); + + if (responseCache != null) { + try { + responseCache.cacheResponse(invocationInput, CachedResponse.ofContent(contentBytes)); + } catch (Throwable t) { + log.warn(t.getMessage(), t); + log.warn("Ignore read from cache, unexpected error happened"); + } + } + response.setContentLength(contentBytes.length); response.getOutputStream().write(contentBytes); } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java index 4a765e4e..96867b7c 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java @@ -3,16 +3,31 @@ import java.io.IOException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; + +import graphql.kickstart.execution.input.GraphQLInvocationInput; +import graphql.kickstart.servlet.cache.CachedResponse; +import graphql.kickstart.servlet.cache.GraphQLResponseCache; import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +@Slf4j @RequiredArgsConstructor class ErrorQueryResponseWriter implements QueryResponseWriter { private final int statusCode; private final String message; + private final GraphQLInvocationInput invocationInput; @Override - public void write(HttpServletRequest request, HttpServletResponse response) throws IOException { + public void write(HttpServletRequest request, HttpServletResponse response, GraphQLResponseCache responseCache) throws IOException { + if (responseCache != null) { + try { + responseCache.cacheResponse(invocationInput, CachedResponse.ofError(statusCode, message)); + } catch (Throwable t) { + log.warn(t.getMessage(), t); + log.warn("Ignore read from cache, unexpected error happened"); + } + } response.sendError(statusCode, message); } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java index 8790160b..61e307ba 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java @@ -4,6 +4,7 @@ import graphql.kickstart.execution.GraphQLObjectMapper; import graphql.kickstart.execution.GraphQLQueryInvoker; import graphql.kickstart.execution.context.ContextSetting; +import graphql.kickstart.servlet.cache.GraphQLResponseCache; import graphql.kickstart.servlet.config.DefaultGraphQLSchemaServletProvider; import graphql.kickstart.servlet.config.GraphQLSchemaServletProvider; import graphql.kickstart.servlet.context.GraphQLServletContextBuilder; @@ -33,12 +34,13 @@ public class GraphQLConfiguration { private final Executor asyncExecutor; private final long subscriptionTimeout; private final ContextSetting contextSetting; + private final GraphQLResponseCache responseCache; private GraphQLConfiguration(GraphQLInvocationInputFactory invocationInputFactory, GraphQLQueryInvoker queryInvoker, GraphQLObjectMapper objectMapper, List listeners, boolean asyncServletModeEnabled, Executor asyncExecutor, long subscriptionTimeout, ContextSetting contextSetting, - Supplier batchInputPreProcessor) { + Supplier batchInputPreProcessor, GraphQLResponseCache responseCache) { this.invocationInputFactory = invocationInputFactory; this.queryInvoker = queryInvoker; this.graphQLInvoker = queryInvoker.toGraphQLInvoker(); @@ -49,6 +51,7 @@ private GraphQLConfiguration(GraphQLInvocationInputFactory invocationInputFactor this.subscriptionTimeout = subscriptionTimeout; this.contextSetting = contextSetting; this.batchInputPreProcessor = batchInputPreProcessor; + this.responseCache = responseCache; } public static GraphQLConfiguration.Builder with(GraphQLSchema schema) { @@ -109,6 +112,10 @@ public BatchInputPreProcessor getBatchInputPreProcessor() { return batchInputPreProcessor.get(); } + public GraphQLResponseCache getResponseCache() { + return responseCache; + } + public static class Builder { private GraphQLInvocationInputFactory.Builder invocationInputFactoryBuilder; @@ -121,6 +128,7 @@ public static class Builder { private long subscriptionTimeout = 0; private ContextSetting contextSetting = ContextSetting.PER_QUERY_WITH_INSTRUMENTATION; private Supplier batchInputPreProcessorSupplier = () -> new NoOpBatchInputPreProcessor(); + private GraphQLResponseCache responseCache; private Builder(GraphQLInvocationInputFactory.Builder invocationInputFactoryBuilder) { this.invocationInputFactoryBuilder = invocationInputFactoryBuilder; @@ -199,6 +207,11 @@ public Builder with(Supplier batchInputPreProcessor) { return this; } + public Builder with(GraphQLResponseCache responseCache) { + this.responseCache = responseCache; + return this; + } + public GraphQLConfiguration build() { return new GraphQLConfiguration( this.invocationInputFactory != null ? this.invocationInputFactory : invocationInputFactoryBuilder.build(), @@ -209,7 +222,8 @@ public GraphQLConfiguration build() { asyncExecutor, subscriptionTimeout, contextSetting, - batchInputPreProcessorSupplier + batchInputPreProcessorSupplier, + responseCache ); } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandler.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandler.java index 758d2f8e..5c7a2dca 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandler.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandler.java @@ -3,7 +3,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -interface HttpRequestHandler { +public interface HttpRequestHandler { String APPLICATION_JSON_UTF8 = "application/json;charset=UTF-8"; String APPLICATION_EVENT_STREAM_UTF8 = "text/event-stream;charset=UTF-8"; diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java index 83b34da6..59719a0b 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java @@ -3,15 +3,18 @@ import graphql.GraphQLException; import graphql.kickstart.execution.GraphQLInvoker; import graphql.kickstart.execution.GraphQLQueryResult; -import graphql.kickstart.servlet.input.BatchInputPreProcessResult; -import graphql.kickstart.servlet.input.BatchInputPreProcessor; import graphql.kickstart.execution.input.GraphQLBatchedInvocationInput; import graphql.kickstart.execution.input.GraphQLInvocationInput; import graphql.kickstart.execution.input.GraphQLSingleInvocationInput; -import java.io.IOException; +import graphql.kickstart.servlet.cache.CacheResponseWriter; +import graphql.kickstart.servlet.cache.CachedResponse; +import graphql.kickstart.servlet.input.BatchInputPreProcessResult; +import graphql.kickstart.servlet.input.BatchInputPreProcessor; +import lombok.extern.slf4j.Slf4j; + import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import lombok.extern.slf4j.Slf4j; +import java.io.IOException; @Slf4j class HttpRequestHandlerImpl implements HttpRequestHandler { @@ -49,11 +52,31 @@ public void handle(HttpServletRequest request, HttpServletResponse response) thr private void execute(GraphQLInvocationInput invocationInput, HttpServletRequest request, HttpServletResponse response) { try { + if (configuration.getResponseCache() != null) { + CachedResponse cachedResponse = null; + try { + cachedResponse = configuration.getResponseCache().getCachedResponse(invocationInput); + } catch (Throwable t) { + log.warn(t.getMessage(), t); + log.warn("Ignore read from cache, unexpected error happened"); + } + + if (cachedResponse != null) { + CacheResponseWriter cacheResponseWriter = new CacheResponseWriter(); + cacheResponseWriter.write(request, response, cachedResponse); + return; + } + } + GraphQLQueryResult queryResult = invoke(invocationInput, request, response); - QueryResponseWriter queryResponseWriter = QueryResponseWriter.createWriter(queryResult, configuration.getObjectMapper(), - configuration.getSubscriptionTimeout()); - queryResponseWriter.write(request, response); + QueryResponseWriter queryResponseWriter = QueryResponseWriter.createWriter( + queryResult, + configuration.getObjectMapper(), + configuration.getSubscriptionTimeout(), + invocationInput + ); + queryResponseWriter.write(request, response, configuration.getResponseCache()); } catch (Throwable t) { response.setStatus(STATUS_BAD_REQUEST); log.info("Bad GET request: path was not \"/schema.json\" or no query variable named \"query\" given"); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java index 7139b2fa..6861aac6 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java @@ -2,6 +2,9 @@ import graphql.kickstart.execution.GraphQLQueryResult; import graphql.kickstart.execution.GraphQLObjectMapper; +import graphql.kickstart.execution.input.GraphQLInvocationInput; +import graphql.kickstart.servlet.cache.GraphQLResponseCache; + import java.io.IOException; import java.util.Objects; import javax.servlet.http.HttpServletRequest; @@ -12,20 +15,21 @@ interface QueryResponseWriter { static QueryResponseWriter createWriter( GraphQLQueryResult result, GraphQLObjectMapper graphQLObjectMapper, - long subscriptionTimeout + long subscriptionTimeout, + GraphQLInvocationInput invocationInput ) { Objects.requireNonNull(result, "GraphQL query result cannot be null"); if (result.isBatched()) { - return new BatchedQueryResponseWriter(result.getResults(), graphQLObjectMapper); + return new BatchedQueryResponseWriter(result.getResults(), graphQLObjectMapper, invocationInput); } else if (result.isAsynchronous()) { - return new SingleAsynchronousQueryResponseWriter(result.getResult(), graphQLObjectMapper, subscriptionTimeout); + return new SingleAsynchronousQueryResponseWriter(result.getResult(), graphQLObjectMapper, subscriptionTimeout, invocationInput); } else if (result.isError()) { - return new ErrorQueryResponseWriter(result.getStatusCode(), result.getMessage()); + return new ErrorQueryResponseWriter(result.getStatusCode(), result.getMessage(), invocationInput); } - return new SingleQueryResponseWriter(result.getResult(), graphQLObjectMapper); + return new SingleQueryResponseWriter(result.getResult(), graphQLObjectMapper, invocationInput); } - void write(HttpServletRequest request, HttpServletResponse response) throws IOException; + void write(HttpServletRequest request, HttpServletResponse response, GraphQLResponseCache responseCache) throws IOException; } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleAsynchronousQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleAsynchronousQueryResponseWriter.java index 620c38f9..66fe5d3d 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleAsynchronousQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleAsynchronousQueryResponseWriter.java @@ -10,11 +10,16 @@ import javax.servlet.AsyncContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; + +import graphql.kickstart.execution.input.GraphQLInvocationInput; +import graphql.kickstart.servlet.cache.GraphQLResponseCache; import lombok.Getter; import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; import org.reactivestreams.Publisher; import org.reactivestreams.Subscription; +@Slf4j @RequiredArgsConstructor class SingleAsynchronousQueryResponseWriter implements QueryResponseWriter { @@ -22,9 +27,14 @@ class SingleAsynchronousQueryResponseWriter implements QueryResponseWriter { private final ExecutionResult result; private final GraphQLObjectMapper graphQLObjectMapper; private final long subscriptionTimeout; + private final GraphQLInvocationInput invocationInput; @Override - public void write(HttpServletRequest request, HttpServletResponse response) { + public void write(HttpServletRequest request, HttpServletResponse response, GraphQLResponseCache responseCache) { + if (responseCache != null) { + log.warn("Response cache for asynchronous query are not implemented yet"); + } + Objects.requireNonNull(request, "Http servlet request cannot be null"); response.setContentType(HttpRequestHandler.APPLICATION_EVENT_STREAM_UTF8); response.setStatus(HttpRequestHandler.STATUS_OK); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java index fa166d3e..4fddbdbb 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java @@ -2,26 +2,42 @@ import graphql.ExecutionResult; import graphql.kickstart.execution.GraphQLObjectMapper; +import graphql.kickstart.execution.input.GraphQLInvocationInput; +import graphql.kickstart.servlet.cache.CachedResponse; +import graphql.kickstart.servlet.cache.GraphQLResponseCache; import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; import java.io.IOException; import java.nio.charset.StandardCharsets; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +@Slf4j @RequiredArgsConstructor class SingleQueryResponseWriter implements QueryResponseWriter { private final ExecutionResult result; private final GraphQLObjectMapper graphQLObjectMapper; + private final GraphQLInvocationInput invocationInput; @Override - public void write(HttpServletRequest request, HttpServletResponse response) throws IOException { + public void write(HttpServletRequest request, HttpServletResponse response, GraphQLResponseCache responseCache) throws IOException { response.setContentType(HttpRequestHandler.APPLICATION_JSON_UTF8); response.setStatus(HttpRequestHandler.STATUS_OK); response.setCharacterEncoding(StandardCharsets.UTF_8.name()); String responseContent = graphQLObjectMapper.serializeResultAsJson(result); byte[] contentBytes = responseContent.getBytes(StandardCharsets.UTF_8); + + if (responseCache != null) { + try { + responseCache.cacheResponse(invocationInput, CachedResponse.ofContent(contentBytes)); + } catch (Throwable t) { + log.warn(t.getMessage(), t); + log.warn("Ignore read from cache, unexpected error happened"); + } + } + response.setContentLength(contentBytes.length); response.getOutputStream().write(contentBytes); } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheResponseWriter.java new file mode 100644 index 00000000..dc6909a3 --- /dev/null +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheResponseWriter.java @@ -0,0 +1,25 @@ +package graphql.kickstart.servlet.cache; + +import graphql.kickstart.servlet.HttpRequestHandler; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +public class CacheResponseWriter { + + public void write(HttpServletRequest request, HttpServletResponse response, CachedResponse cachedResponse) + throws IOException { + if (cachedResponse.isError()) { + response.sendError(cachedResponse.getErrorStatusCode(), cachedResponse.getErrorMessage()); + } else { + response.setContentType(HttpRequestHandler.APPLICATION_JSON_UTF8); + response.setStatus(HttpRequestHandler.STATUS_OK); + response.setCharacterEncoding(StandardCharsets.UTF_8.name()); + response.setContentLength(cachedResponse.getContentBytes().length); + response.getOutputStream().write(cachedResponse.getContentBytes()); + } + } + +} diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachedResponse.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachedResponse.java new file mode 100644 index 00000000..1d1b288b --- /dev/null +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachedResponse.java @@ -0,0 +1,72 @@ +package graphql.kickstart.servlet.cache; + +import java.io.Serializable; +import java.util.Objects; + +public class CachedResponse implements Serializable { + + private static final long serialVersionUID = 5894555791705575139L; + + private final byte[] contentBytes; + + private final boolean error; + private final Integer errorStatusCode; + private final String errorMessage; + + private CachedResponse(byte[] contentBytes, boolean error, Integer errorStatusCode, String errorMessage) { + this.contentBytes = contentBytes; + this.error = error; + this.errorStatusCode = errorStatusCode; + this.errorMessage = errorMessage; + } + + /** + * Constructor for success response + * + * @param contentBytes bytes array of graphql json response + */ + public static CachedResponse ofContent(byte[] contentBytes) { + Objects.requireNonNull(contentBytes, "contentBytes can not be null"); + + return new CachedResponse(contentBytes, false, null, null); + } + + /** + * Constructor for error response + * + * @param errorStatusCode the status code for the error response + * @param errorMessage the error message for the error response + */ + public static CachedResponse ofError(int errorStatusCode, String errorMessage) { + return new CachedResponse(null, true, errorStatusCode, errorMessage); + } + + /** + * @return {@literal true} when this request was failed + */ + public boolean isError() { + return error; + } + + /** + * @return the response body for success requests, {@literal null} when {@link #isError()} is {@literal true} + */ + public byte[] getContentBytes() { + return contentBytes; + } + + /** + * @return the response error code + */ + public Integer getErrorStatusCode() { + return errorStatusCode; + } + + /** + * @return the response error message + */ + public String getErrorMessage() { + return errorMessage; + } + +} diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java new file mode 100644 index 00000000..f5157a55 --- /dev/null +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java @@ -0,0 +1,25 @@ +package graphql.kickstart.servlet.cache; + +import graphql.kickstart.execution.input.GraphQLInvocationInput; + +import java.util.Optional; + +public interface GraphQLResponseCache { + + /** + * Retrieve the cache by input data. If this query was not cached before, will return empty {@link Optional}. + * + * @param invocationInput input data + * @return cached response if something available in cache or {@literal null} if nothing cached + */ + CachedResponse getCachedResponse(GraphQLInvocationInput invocationInput); + + /** + * Decide to cache or not this response. It depends on the implementation. + * + * @param invocationInput input data + * @param cachedResponse response to cache + */ + void cacheResponse(GraphQLInvocationInput invocationInput, CachedResponse cachedResponse); + +} From b66190ea4d892d2465499ea4215e887faa1d0f8a Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Thu, 23 Apr 2020 19:32:25 +0800 Subject: [PATCH 02/13] Fixed SingleQueryResponseWriterTest. --- .../kickstart/servlet/SingleQueryResponseWriterTest.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/SingleQueryResponseWriterTest.groovy b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/SingleQueryResponseWriterTest.groovy index ae834c19..668a52b6 100644 --- a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/SingleQueryResponseWriterTest.groovy +++ b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/SingleQueryResponseWriterTest.groovy @@ -29,8 +29,8 @@ class SingleQueryResponseWriterTest extends Specification { 1 * responseMock.getOutputStream().write(expectedResponseContent.getBytes(StandardCharsets.UTF_8)) expect: - def writer = new SingleQueryResponseWriter(new ExecutionResultImpl(result, []), graphQLObjectMapperMock) - writer.write(requestMock, responseMock) + def writer = new SingleQueryResponseWriter(new ExecutionResultImpl(result, []), graphQLObjectMapperMock, null) + writer.write(requestMock, responseMock, null) where: result || expectedContentLenght | expectedResponseContent From 8836fabfc68f863bdf0cfb9f31e6939dae8e30cb Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Thu, 23 Apr 2020 21:57:09 +0800 Subject: [PATCH 03/13] Pass HttpServletRequest to GraphQLResponseCache methods. --- .../kickstart/servlet/BatchedQueryResponseWriter.java | 2 +- .../graphql/kickstart/servlet/HttpRequestHandlerImpl.java | 2 +- .../kickstart/servlet/cache/GraphQLResponseCache.java | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java index f8dedef3..70c72dce 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java @@ -44,7 +44,7 @@ public void write(HttpServletRequest request, HttpServletResponse response, Grap if (responseCache != null) { try { - responseCache.cacheResponse(invocationInput, CachedResponse.ofContent(contentBytes)); + responseCache.cacheResponse(request, invocationInput, CachedResponse.ofContent(contentBytes)); } catch (Throwable t) { log.warn(t.getMessage(), t); log.warn("Ignore read from cache, unexpected error happened"); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java index 59719a0b..6567f79e 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java @@ -55,7 +55,7 @@ private void execute(GraphQLInvocationInput invocationInput, HttpServletRequest if (configuration.getResponseCache() != null) { CachedResponse cachedResponse = null; try { - cachedResponse = configuration.getResponseCache().getCachedResponse(invocationInput); + cachedResponse = configuration.getResponseCache().getCachedResponse(request, invocationInput); } catch (Throwable t) { log.warn(t.getMessage(), t); log.warn("Ignore read from cache, unexpected error happened"); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java index f5157a55..2116f98d 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java @@ -2,6 +2,7 @@ import graphql.kickstart.execution.input.GraphQLInvocationInput; +import javax.servlet.http.HttpServletRequest; import java.util.Optional; public interface GraphQLResponseCache { @@ -12,7 +13,7 @@ public interface GraphQLResponseCache { * @param invocationInput input data * @return cached response if something available in cache or {@literal null} if nothing cached */ - CachedResponse getCachedResponse(GraphQLInvocationInput invocationInput); + CachedResponse getCachedResponse(HttpServletRequest request, GraphQLInvocationInput invocationInput); /** * Decide to cache or not this response. It depends on the implementation. @@ -20,6 +21,6 @@ public interface GraphQLResponseCache { * @param invocationInput input data * @param cachedResponse response to cache */ - void cacheResponse(GraphQLInvocationInput invocationInput, CachedResponse cachedResponse); + void cacheResponse(HttpServletRequest request, GraphQLInvocationInput invocationInput, CachedResponse cachedResponse); } From c52e45b52d9d0c035d91521cab942ad7dd781e1c Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Thu, 23 Apr 2020 22:01:21 +0800 Subject: [PATCH 04/13] Added comments. --- .../graphql/kickstart/servlet/cache/GraphQLResponseCache.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java index 2116f98d..d9c1da8b 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java @@ -10,6 +10,7 @@ public interface GraphQLResponseCache { /** * Retrieve the cache by input data. If this query was not cached before, will return empty {@link Optional}. * + * @param request the http request * @param invocationInput input data * @return cached response if something available in cache or {@literal null} if nothing cached */ @@ -18,6 +19,7 @@ public interface GraphQLResponseCache { /** * Decide to cache or not this response. It depends on the implementation. * + * @param request the http request * @param invocationInput input data * @param cachedResponse response to cache */ From 2322c2588dd1efd551c60c2952e5ade0e9304233 Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Thu, 23 Apr 2020 22:52:31 +0800 Subject: [PATCH 05/13] Pass request to cacheResponse method. --- .../graphql/kickstart/servlet/ErrorQueryResponseWriter.java | 2 +- .../graphql/kickstart/servlet/SingleQueryResponseWriter.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java index 96867b7c..b195d790 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java @@ -22,7 +22,7 @@ class ErrorQueryResponseWriter implements QueryResponseWriter { public void write(HttpServletRequest request, HttpServletResponse response, GraphQLResponseCache responseCache) throws IOException { if (responseCache != null) { try { - responseCache.cacheResponse(invocationInput, CachedResponse.ofError(statusCode, message)); + responseCache.cacheResponse(request, invocationInput, CachedResponse.ofError(statusCode, message)); } catch (Throwable t) { log.warn(t.getMessage(), t); log.warn("Ignore read from cache, unexpected error happened"); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java index 4fddbdbb..88221c1c 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java @@ -31,7 +31,7 @@ public void write(HttpServletRequest request, HttpServletResponse response, Grap if (responseCache != null) { try { - responseCache.cacheResponse(invocationInput, CachedResponse.ofContent(contentBytes)); + responseCache.cacheResponse(request, invocationInput, CachedResponse.ofContent(contentBytes)); } catch (Throwable t) { log.warn(t.getMessage(), t); log.warn("Ignore read from cache, unexpected error happened"); From 9153aa5810e24356a01f5774cdbeb38a7f074512 Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Fri, 1 May 2020 22:24:25 +0800 Subject: [PATCH 06/13] Added method which check is this request must be cached. --- .../servlet/BatchedQueryResponseWriter.java | 4 ++-- .../kickstart/servlet/ErrorQueryResponseWriter.java | 4 ++-- .../kickstart/servlet/SingleQueryResponseWriter.java | 4 ++-- .../servlet/cache/GraphQLResponseCache.java | 12 ++++++++++-- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java index 70c72dce..9ffa2ac0 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java @@ -42,9 +42,9 @@ public void write(HttpServletRequest request, HttpServletResponse response, Grap String responseContent = responseBuilder.toString(); byte[] contentBytes = responseContent.getBytes(StandardCharsets.UTF_8); - if (responseCache != null) { + if (responseCache != null && responseCache.isCacheable(request, invocationInput)) { try { - responseCache.cacheResponse(request, invocationInput, CachedResponse.ofContent(contentBytes)); + responseCache.put(request, invocationInput, CachedResponse.ofContent(contentBytes)); } catch (Throwable t) { log.warn(t.getMessage(), t); log.warn("Ignore read from cache, unexpected error happened"); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java index b195d790..a51ac9dd 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java @@ -20,9 +20,9 @@ class ErrorQueryResponseWriter implements QueryResponseWriter { @Override public void write(HttpServletRequest request, HttpServletResponse response, GraphQLResponseCache responseCache) throws IOException { - if (responseCache != null) { + if (responseCache != null && responseCache.isCacheable(request, invocationInput)) { try { - responseCache.cacheResponse(request, invocationInput, CachedResponse.ofError(statusCode, message)); + responseCache.put(request, invocationInput, CachedResponse.ofError(statusCode, message)); } catch (Throwable t) { log.warn(t.getMessage(), t); log.warn("Ignore read from cache, unexpected error happened"); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java index 88221c1c..8aeaf7f8 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java @@ -29,9 +29,9 @@ public void write(HttpServletRequest request, HttpServletResponse response, Grap String responseContent = graphQLObjectMapper.serializeResultAsJson(result); byte[] contentBytes = responseContent.getBytes(StandardCharsets.UTF_8); - if (responseCache != null) { + if (responseCache != null && responseCache.isCacheable(request, invocationInput)) { try { - responseCache.cacheResponse(request, invocationInput, CachedResponse.ofContent(contentBytes)); + responseCache.put(request, invocationInput, CachedResponse.ofContent(contentBytes)); } catch (Throwable t) { log.warn(t.getMessage(), t); log.warn("Ignore read from cache, unexpected error happened"); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java index d9c1da8b..dc8cbbb1 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java @@ -14,15 +14,23 @@ public interface GraphQLResponseCache { * @param invocationInput input data * @return cached response if something available in cache or {@literal null} if nothing cached */ - CachedResponse getCachedResponse(HttpServletRequest request, GraphQLInvocationInput invocationInput); + CachedResponse get(HttpServletRequest request, GraphQLInvocationInput invocationInput); /** * Decide to cache or not this response. It depends on the implementation. * * @param request the http request * @param invocationInput input data + */ + boolean isCacheable(HttpServletRequest request, GraphQLInvocationInput invocationInput); + + /** + * Cache this response. It depends on the implementation. + * + * @param request the http request + * @param invocationInput input data * @param cachedResponse response to cache */ - void cacheResponse(HttpServletRequest request, GraphQLInvocationInput invocationInput, CachedResponse cachedResponse); + void put(HttpServletRequest request, GraphQLInvocationInput invocationInput, CachedResponse cachedResponse); } From 86cc378ec96e50564d3b9e78d331ea3be614cb41 Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Fri, 1 May 2020 22:27:52 +0800 Subject: [PATCH 07/13] Fixed HttpRequestHandlerImpl. --- .../java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java index 6567f79e..158f91de 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java @@ -55,7 +55,7 @@ private void execute(GraphQLInvocationInput invocationInput, HttpServletRequest if (configuration.getResponseCache() != null) { CachedResponse cachedResponse = null; try { - cachedResponse = configuration.getResponseCache().getCachedResponse(request, invocationInput); + cachedResponse = configuration.getResponseCache().get(request, invocationInput); } catch (Throwable t) { log.warn(t.getMessage(), t); log.warn("Ignore read from cache, unexpected error happened"); From 4b4edfe3b05792d0112a79d53544e8aae7736dee Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Mon, 25 May 2020 02:31:39 +0800 Subject: [PATCH 08/13] Code review changes. --- .../servlet/BatchedQueryResponseWriter.java | 15 +- .../servlet/ErrorQueryResponseWriter.java | 17 +-- .../servlet/GraphQLConfiguration.java | 20 +-- .../servlet/HttpRequestHandlerImpl.java | 38 ++--- .../servlet/QueryResponseWriter.java | 26 +++- ...SingleAsynchronousQueryResponseWriter.java | 12 +- .../servlet/SingleQueryResponseWriter.java | 22 +-- .../cache/BufferedHttpServletResponse.java | 142 ++++++++++++++++++ .../kickstart/servlet/cache/CacheReader.java | 57 +++++++ .../servlet/cache/CacheResponseWriter.java | 25 --- .../cache/CachingQueryResponseWriter.java | 56 +++++++ ....java => GraphQLResponseCacheManager.java} | 2 +- 12 files changed, 303 insertions(+), 129 deletions(-) create mode 100644 graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/BufferedHttpServletResponse.java create mode 100644 graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheReader.java delete mode 100644 graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheResponseWriter.java create mode 100644 graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingQueryResponseWriter.java rename graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/{GraphQLResponseCache.java => GraphQLResponseCacheManager.java} (96%) diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java index 9ffa2ac0..39811a1f 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/BatchedQueryResponseWriter.java @@ -2,9 +2,6 @@ import graphql.ExecutionResult; import graphql.kickstart.execution.GraphQLObjectMapper; -import graphql.kickstart.execution.input.GraphQLInvocationInput; -import graphql.kickstart.servlet.cache.CachedResponse; -import graphql.kickstart.servlet.cache.GraphQLResponseCache; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -21,10 +18,9 @@ class BatchedQueryResponseWriter implements QueryResponseWriter { private final List results; private final GraphQLObjectMapper graphQLObjectMapper; - private final GraphQLInvocationInput invocationInput; @Override - public void write(HttpServletRequest request, HttpServletResponse response, GraphQLResponseCache responseCache) throws IOException { + public void write(HttpServletRequest request, HttpServletResponse response) throws IOException { response.setContentType(HttpRequestHandler.APPLICATION_JSON_UTF8); response.setStatus(HttpRequestHandler.STATUS_OK); @@ -42,15 +38,6 @@ public void write(HttpServletRequest request, HttpServletResponse response, Grap String responseContent = responseBuilder.toString(); byte[] contentBytes = responseContent.getBytes(StandardCharsets.UTF_8); - if (responseCache != null && responseCache.isCacheable(request, invocationInput)) { - try { - responseCache.put(request, invocationInput, CachedResponse.ofContent(contentBytes)); - } catch (Throwable t) { - log.warn(t.getMessage(), t); - log.warn("Ignore read from cache, unexpected error happened"); - } - } - response.setContentLength(contentBytes.length); response.getOutputStream().write(contentBytes); } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java index a51ac9dd..4a765e4e 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/ErrorQueryResponseWriter.java @@ -3,31 +3,16 @@ import java.io.IOException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - -import graphql.kickstart.execution.input.GraphQLInvocationInput; -import graphql.kickstart.servlet.cache.CachedResponse; -import graphql.kickstart.servlet.cache.GraphQLResponseCache; import lombok.RequiredArgsConstructor; -import lombok.extern.slf4j.Slf4j; -@Slf4j @RequiredArgsConstructor class ErrorQueryResponseWriter implements QueryResponseWriter { private final int statusCode; private final String message; - private final GraphQLInvocationInput invocationInput; @Override - public void write(HttpServletRequest request, HttpServletResponse response, GraphQLResponseCache responseCache) throws IOException { - if (responseCache != null && responseCache.isCacheable(request, invocationInput)) { - try { - responseCache.put(request, invocationInput, CachedResponse.ofError(statusCode, message)); - } catch (Throwable t) { - log.warn(t.getMessage(), t); - log.warn("Ignore read from cache, unexpected error happened"); - } - } + public void write(HttpServletRequest request, HttpServletResponse response) throws IOException { response.sendError(statusCode, message); } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java index e2a3aa4a..12311dfd 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java @@ -4,7 +4,7 @@ import graphql.kickstart.execution.GraphQLObjectMapper; import graphql.kickstart.execution.GraphQLQueryInvoker; import graphql.kickstart.execution.context.ContextSetting; -import graphql.kickstart.servlet.cache.GraphQLResponseCache; +import graphql.kickstart.servlet.cache.GraphQLResponseCacheManager; import graphql.kickstart.servlet.config.DefaultGraphQLSchemaServletProvider; import graphql.kickstart.servlet.config.GraphQLSchemaServletProvider; import graphql.kickstart.servlet.context.GraphQLServletContextBuilder; @@ -37,13 +37,13 @@ public class GraphQLConfiguration { @Getter private final long asyncTimeout; private final ContextSetting contextSetting; - private final GraphQLResponseCache responseCache; + private final GraphQLResponseCacheManager responseCacheManager; private GraphQLConfiguration(GraphQLInvocationInputFactory invocationInputFactory, GraphQLQueryInvoker queryInvoker, GraphQLObjectMapper objectMapper, List listeners, boolean asyncServletModeEnabled, Executor asyncExecutor, long subscriptionTimeout, long asyncTimeout, ContextSetting contextSetting, - Supplier batchInputPreProcessor, GraphQLResponseCache responseCache) { + Supplier batchInputPreProcessor, GraphQLResponseCacheManager responseCacheManager) { this.invocationInputFactory = invocationInputFactory; this.queryInvoker = queryInvoker; this.graphQLInvoker = queryInvoker.toGraphQLInvoker(); @@ -55,7 +55,7 @@ private GraphQLConfiguration(GraphQLInvocationInputFactory invocationInputFactor this.asyncTimeout = asyncTimeout; this.contextSetting = contextSetting; this.batchInputPreProcessor = batchInputPreProcessor; - this.responseCache = responseCache; + this.responseCacheManager = responseCacheManager; } public static GraphQLConfiguration.Builder with(GraphQLSchema schema) { @@ -116,8 +116,8 @@ public BatchInputPreProcessor getBatchInputPreProcessor() { return batchInputPreProcessor.get(); } - public GraphQLResponseCache getResponseCache() { - return responseCache; + public GraphQLResponseCacheManager getResponseCacheManager() { + return responseCacheManager; } public static class Builder { @@ -133,7 +133,7 @@ public static class Builder { private long asyncTimeout = 30; private ContextSetting contextSetting = ContextSetting.PER_QUERY_WITH_INSTRUMENTATION; private Supplier batchInputPreProcessorSupplier = NoOpBatchInputPreProcessor::new; - private GraphQLResponseCache responseCache; + private GraphQLResponseCacheManager responseCacheManager; private Builder(GraphQLInvocationInputFactory.Builder invocationInputFactoryBuilder) { this.invocationInputFactoryBuilder = invocationInputFactoryBuilder; @@ -217,8 +217,8 @@ public Builder with(Supplier batchInputPreProcessor) { return this; } - public Builder with(GraphQLResponseCache responseCache) { - this.responseCache = responseCache; + public Builder with(GraphQLResponseCacheManager responseCache) { + this.responseCacheManager = responseCache; return this; } @@ -234,7 +234,7 @@ public GraphQLConfiguration build() { asyncTimeout, contextSetting, batchInputPreProcessorSupplier, - responseCache + responseCacheManager ); } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java index 158f91de..17423c97 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java @@ -6,8 +6,7 @@ import graphql.kickstart.execution.input.GraphQLBatchedInvocationInput; import graphql.kickstart.execution.input.GraphQLInvocationInput; import graphql.kickstart.execution.input.GraphQLSingleInvocationInput; -import graphql.kickstart.servlet.cache.CacheResponseWriter; -import graphql.kickstart.servlet.cache.CachedResponse; +import graphql.kickstart.servlet.cache.CacheReader; import graphql.kickstart.servlet.input.BatchInputPreProcessResult; import graphql.kickstart.servlet.input.BatchInputPreProcessor; import lombok.extern.slf4j.Slf4j; @@ -52,31 +51,20 @@ public void handle(HttpServletRequest request, HttpServletResponse response) thr private void execute(GraphQLInvocationInput invocationInput, HttpServletRequest request, HttpServletResponse response) { try { - if (configuration.getResponseCache() != null) { - CachedResponse cachedResponse = null; - try { - cachedResponse = configuration.getResponseCache().get(request, invocationInput); - } catch (Throwable t) { - log.warn(t.getMessage(), t); - log.warn("Ignore read from cache, unexpected error happened"); - } + // try to return value from cache if cache manager was set, otherwise processed the query + if (configuration.getResponseCacheManager() != null && + !CacheReader.responseFromCache(invocationInput, request, response, configuration.getResponseCacheManager())) { + GraphQLQueryResult queryResult = invoke(invocationInput, request, response); - if (cachedResponse != null) { - CacheResponseWriter cacheResponseWriter = new CacheResponseWriter(); - cacheResponseWriter.write(request, response, cachedResponse); - return; - } + QueryResponseWriter queryResponseWriter = QueryResponseWriter.createWriter( + queryResult, + configuration.getObjectMapper(), + configuration.getSubscriptionTimeout(), + invocationInput, + configuration.getResponseCacheManager() + ); + queryResponseWriter.write(request, response); } - - GraphQLQueryResult queryResult = invoke(invocationInput, request, response); - - QueryResponseWriter queryResponseWriter = QueryResponseWriter.createWriter( - queryResult, - configuration.getObjectMapper(), - configuration.getSubscriptionTimeout(), - invocationInput - ); - queryResponseWriter.write(request, response, configuration.getResponseCache()); } catch (Throwable t) { response.setStatus(STATUS_BAD_REQUEST); log.info("Bad GET request: path was not \"/schema.json\" or no query variable named \"query\" given"); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java index 6861aac6..d58724a1 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java @@ -3,33 +3,43 @@ import graphql.kickstart.execution.GraphQLQueryResult; import graphql.kickstart.execution.GraphQLObjectMapper; import graphql.kickstart.execution.input.GraphQLInvocationInput; -import graphql.kickstart.servlet.cache.GraphQLResponseCache; +import graphql.kickstart.servlet.cache.CachingQueryResponseWriter; +import graphql.kickstart.servlet.cache.GraphQLResponseCacheManager; import java.io.IOException; import java.util.Objects; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -interface QueryResponseWriter { +public interface QueryResponseWriter { static QueryResponseWriter createWriter( GraphQLQueryResult result, GraphQLObjectMapper graphQLObjectMapper, long subscriptionTimeout, - GraphQLInvocationInput invocationInput + GraphQLInvocationInput invocationInput, + GraphQLResponseCacheManager responseCache ) { Objects.requireNonNull(result, "GraphQL query result cannot be null"); + QueryResponseWriter writer; + if (result.isBatched()) { - return new BatchedQueryResponseWriter(result.getResults(), graphQLObjectMapper, invocationInput); + writer = new BatchedQueryResponseWriter(result.getResults(), graphQLObjectMapper); } else if (result.isAsynchronous()) { - return new SingleAsynchronousQueryResponseWriter(result.getResult(), graphQLObjectMapper, subscriptionTimeout, invocationInput); + writer = new SingleAsynchronousQueryResponseWriter(result.getResult(), graphQLObjectMapper, subscriptionTimeout); } else if (result.isError()) { - return new ErrorQueryResponseWriter(result.getStatusCode(), result.getMessage(), invocationInput); + writer = new ErrorQueryResponseWriter(result.getStatusCode(), result.getMessage()); + } else { + writer = new SingleQueryResponseWriter(result.getResult(), graphQLObjectMapper); + } + + if (responseCache != null) { + writer = new CachingQueryResponseWriter(writer, responseCache, invocationInput, result.isError()); } - return new SingleQueryResponseWriter(result.getResult(), graphQLObjectMapper, invocationInput); + return writer; } - void write(HttpServletRequest request, HttpServletResponse response, GraphQLResponseCache responseCache) throws IOException; + void write(HttpServletRequest request, HttpServletResponse response) throws IOException; } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleAsynchronousQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleAsynchronousQueryResponseWriter.java index 66fe5d3d..620c38f9 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleAsynchronousQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleAsynchronousQueryResponseWriter.java @@ -10,16 +10,11 @@ import javax.servlet.AsyncContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - -import graphql.kickstart.execution.input.GraphQLInvocationInput; -import graphql.kickstart.servlet.cache.GraphQLResponseCache; import lombok.Getter; import lombok.RequiredArgsConstructor; -import lombok.extern.slf4j.Slf4j; import org.reactivestreams.Publisher; import org.reactivestreams.Subscription; -@Slf4j @RequiredArgsConstructor class SingleAsynchronousQueryResponseWriter implements QueryResponseWriter { @@ -27,14 +22,9 @@ class SingleAsynchronousQueryResponseWriter implements QueryResponseWriter { private final ExecutionResult result; private final GraphQLObjectMapper graphQLObjectMapper; private final long subscriptionTimeout; - private final GraphQLInvocationInput invocationInput; @Override - public void write(HttpServletRequest request, HttpServletResponse response, GraphQLResponseCache responseCache) { - if (responseCache != null) { - log.warn("Response cache for asynchronous query are not implemented yet"); - } - + public void write(HttpServletRequest request, HttpServletResponse response) { Objects.requireNonNull(request, "Http servlet request cannot be null"); response.setContentType(HttpRequestHandler.APPLICATION_EVENT_STREAM_UTF8); response.setStatus(HttpRequestHandler.STATUS_OK); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java index 8aeaf7f8..4245c19d 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java @@ -2,42 +2,26 @@ import graphql.ExecutionResult; import graphql.kickstart.execution.GraphQLObjectMapper; -import graphql.kickstart.execution.input.GraphQLInvocationInput; -import graphql.kickstart.servlet.cache.CachedResponse; -import graphql.kickstart.servlet.cache.GraphQLResponseCache; import lombok.RequiredArgsConstructor; -import lombok.extern.slf4j.Slf4j; -import java.io.IOException; -import java.nio.charset.StandardCharsets; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.nio.charset.StandardCharsets; -@Slf4j @RequiredArgsConstructor class SingleQueryResponseWriter implements QueryResponseWriter { private final ExecutionResult result; private final GraphQLObjectMapper graphQLObjectMapper; - private final GraphQLInvocationInput invocationInput; @Override - public void write(HttpServletRequest request, HttpServletResponse response, GraphQLResponseCache responseCache) throws IOException { + public void write(HttpServletRequest request, HttpServletResponse response) throws IOException { response.setContentType(HttpRequestHandler.APPLICATION_JSON_UTF8); response.setStatus(HttpRequestHandler.STATUS_OK); response.setCharacterEncoding(StandardCharsets.UTF_8.name()); String responseContent = graphQLObjectMapper.serializeResultAsJson(result); byte[] contentBytes = responseContent.getBytes(StandardCharsets.UTF_8); - - if (responseCache != null && responseCache.isCacheable(request, invocationInput)) { - try { - responseCache.put(request, invocationInput, CachedResponse.ofContent(contentBytes)); - } catch (Throwable t) { - log.warn(t.getMessage(), t); - log.warn("Ignore read from cache, unexpected error happened"); - } - } - response.setContentLength(contentBytes.length); response.getOutputStream().write(contentBytes); } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/BufferedHttpServletResponse.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/BufferedHttpServletResponse.java new file mode 100644 index 00000000..d34dbb1d --- /dev/null +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/BufferedHttpServletResponse.java @@ -0,0 +1,142 @@ +package graphql.kickstart.servlet.cache; + +import lombok.extern.slf4j.Slf4j; + +import javax.servlet.ServletOutputStream; +import javax.servlet.WriteListener; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletResponseWrapper; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.io.PrintWriter; + +@Slf4j +public class BufferedHttpServletResponse extends HttpServletResponseWrapper { + + private static final class BufferedOutputStream extends ServletOutputStream { + + private final OutputStream delegate; + private final ByteArrayOutputStream buf = new ByteArrayOutputStream(); + + public BufferedOutputStream(OutputStream delegate) { + this.delegate = delegate; + } + + public void write(int b) throws IOException { + buf.write(b); + delegate.write(b); + } + + @Override + public void flush() throws IOException { + buf.flush(); + delegate.flush(); + } + + @Override + public void close() throws IOException { + buf.close(); + delegate.close(); + } + + @Override + public boolean isReady() { + return true; + } + + @Override + public void setWriteListener(WriteListener writeListener) { + } + + public byte[] toByteArray() { + return buf.toByteArray(); + } + + } + + private BufferedOutputStream copier; + + private ServletOutputStream outputStream; + private PrintWriter writer; + private String errorMessage; + + public BufferedHttpServletResponse(HttpServletResponse response) { + super(response); + } + + @Override + public void sendError(int sc, String msg) throws IOException { + errorMessage = msg; + super.sendError(sc, msg); + } + + @Override + public void sendError(int sc) throws IOException { + sendError(sc, null); + } + + @Override + public ServletOutputStream getOutputStream() throws IOException { + if (writer != null) { + throw new IllegalStateException("getWriter() has already been called on this response."); + } + + if (outputStream == null) { + outputStream = getResponse().getOutputStream(); + copier = new BufferedOutputStream(outputStream); + } + + return copier; + } + + @Override + public PrintWriter getWriter() throws IOException { + if (outputStream != null) { + throw new IllegalStateException("getOutputStream() has already been called on this response."); + } + + if (writer == null) { + copier = new BufferedOutputStream(getResponse().getOutputStream()); + writer = new PrintWriter(new OutputStreamWriter(copier, getResponse().getCharacterEncoding()), true); + } + + return writer; + } + + @Override + public void flushBuffer() throws IOException { + if (writer != null) { + writer.flush(); + } else if (copier != null) { + copier.flush(); + } + } + + @Override + public boolean isCommitted() { + return false; + } + + public void close() throws IOException { + if (writer != null) { + writer.close(); + } else if (copier != null) { + copier.close(); + } + } + + public String getErrorMessage() { + return errorMessage; + } + + public byte[] getContentAsByteArray() { + if (copier != null) { + return copier.toByteArray(); + } else { + return new byte[0]; + } + } + +} diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheReader.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheReader.java new file mode 100644 index 00000000..c8297c1c --- /dev/null +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheReader.java @@ -0,0 +1,57 @@ +package graphql.kickstart.servlet.cache; + +import graphql.kickstart.execution.input.GraphQLInvocationInput; +import graphql.kickstart.servlet.HttpRequestHandler; +import lombok.extern.slf4j.Slf4j; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +@Slf4j +public final class CacheReader { + + private CacheReader() { + } + + /** + * Response from cache if possible, if nothing in cache will not produce any response + * + * @return {@literal true} if response was fulfilled from cache, {@literal false} is cache not found or an error + * occurred while reading value from cache + * @throws IOException if can not read value from the cache + */ + public static boolean responseFromCache(GraphQLInvocationInput invocationInput, + HttpServletRequest request, + HttpServletResponse response, + GraphQLResponseCacheManager cacheManager) throws IOException { + CachedResponse cachedResponse = null; + try { + cachedResponse = cacheManager.get(request, invocationInput); + } catch (Throwable t) { + log.warn("Ignore read from cache, unexpected error happened", t); + } + + if (cachedResponse != null) { + write(response, cachedResponse); + return true; + } + + return false; + } + + private static void write(HttpServletResponse response, CachedResponse cachedResponse) + throws IOException { + if (cachedResponse.isError()) { + response.sendError(cachedResponse.getErrorStatusCode(), cachedResponse.getErrorMessage()); + } else { + response.setContentType(HttpRequestHandler.APPLICATION_JSON_UTF8); + response.setStatus(HttpRequestHandler.STATUS_OK); + response.setCharacterEncoding(StandardCharsets.UTF_8.name()); + response.setContentLength(cachedResponse.getContentBytes().length); + response.getOutputStream().write(cachedResponse.getContentBytes()); + } + } + +} diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheResponseWriter.java deleted file mode 100644 index dc6909a3..00000000 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheResponseWriter.java +++ /dev/null @@ -1,25 +0,0 @@ -package graphql.kickstart.servlet.cache; - -import graphql.kickstart.servlet.HttpRequestHandler; - -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.nio.charset.StandardCharsets; - -public class CacheResponseWriter { - - public void write(HttpServletRequest request, HttpServletResponse response, CachedResponse cachedResponse) - throws IOException { - if (cachedResponse.isError()) { - response.sendError(cachedResponse.getErrorStatusCode(), cachedResponse.getErrorMessage()); - } else { - response.setContentType(HttpRequestHandler.APPLICATION_JSON_UTF8); - response.setStatus(HttpRequestHandler.STATUS_OK); - response.setCharacterEncoding(StandardCharsets.UTF_8.name()); - response.setContentLength(cachedResponse.getContentBytes().length); - response.getOutputStream().write(cachedResponse.getContentBytes()); - } - } - -} diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingQueryResponseWriter.java new file mode 100644 index 00000000..0cd9c703 --- /dev/null +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingQueryResponseWriter.java @@ -0,0 +1,56 @@ +package graphql.kickstart.servlet.cache; + +import graphql.kickstart.execution.input.GraphQLInvocationInput; +import graphql.kickstart.servlet.QueryResponseWriter; +import lombok.extern.slf4j.Slf4j; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; + +@Slf4j +public class CachingQueryResponseWriter implements QueryResponseWriter { + + private final QueryResponseWriter delegate; + private final GraphQLResponseCacheManager responseCache; + private final GraphQLInvocationInput invocationInput; + private final boolean error; + + public CachingQueryResponseWriter(QueryResponseWriter delegate, GraphQLResponseCacheManager responseCache, + GraphQLInvocationInput invocationInput, boolean error) { + this.delegate = delegate; + this.responseCache = responseCache; + this.invocationInput = invocationInput; + this.error = error; + } + + @Override + public void write(HttpServletRequest request, HttpServletResponse response) throws IOException { + if (responseCache.isCacheable(request, invocationInput)) { + BufferedHttpServletResponse cachingResponseWrapper = new BufferedHttpServletResponse(response); + + delegate.write(request, cachingResponseWrapper); + + try { + if (error) { + int errorStatusCode = cachingResponseWrapper.getStatus(); + String errorMessage = cachingResponseWrapper.getErrorMessage(); + + responseCache.put(request, invocationInput, CachedResponse.ofError(errorStatusCode, errorMessage)); + } else { + byte[] contentBytes = cachingResponseWrapper.getContentAsByteArray(); + + responseCache.put(request, invocationInput, CachedResponse.ofContent(contentBytes)); + } + } catch (Throwable t) { + log.warn("Ignore read from cache, unexpected error happened", t); + } + + cachingResponseWrapper.flushBuffer(); + cachingResponseWrapper.close(); + } else { + delegate.write(request, response); + } + } + +} diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCacheManager.java similarity index 96% rename from graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java rename to graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCacheManager.java index dc8cbbb1..1e71a801 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCache.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/GraphQLResponseCacheManager.java @@ -5,7 +5,7 @@ import javax.servlet.http.HttpServletRequest; import java.util.Optional; -public interface GraphQLResponseCache { +public interface GraphQLResponseCacheManager { /** * Retrieve the cache by input data. If this query was not cached before, will return empty {@link Optional}. From aa936f995c2375d28c2360e36bef04f22ed9e40d Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Mon, 25 May 2020 22:22:57 +0800 Subject: [PATCH 09/13] Fixed tests. --- .../graphql/kickstart/servlet/HttpRequestHandlerImpl.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java index 17423c97..20e35769 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java @@ -52,8 +52,10 @@ private void execute(GraphQLInvocationInput invocationInput, HttpServletRequest HttpServletResponse response) { try { // try to return value from cache if cache manager was set, otherwise processed the query - if (configuration.getResponseCacheManager() != null && - !CacheReader.responseFromCache(invocationInput, request, response, configuration.getResponseCacheManager())) { + boolean returnedFromCache = configuration.getResponseCacheManager() != null && + !CacheReader.responseFromCache(invocationInput, request, response, configuration.getResponseCacheManager()); + + if (!returnedFromCache) { GraphQLQueryResult queryResult = invoke(invocationInput, request, response); QueryResponseWriter queryResponseWriter = QueryResponseWriter.createWriter( From 4d25d64737a503794cf4eacde4910c6414e4f994 Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Mon, 25 May 2020 22:59:01 +0800 Subject: [PATCH 10/13] Fixed tests. --- .../kickstart/servlet/SingleQueryResponseWriterTest.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/SingleQueryResponseWriterTest.groovy b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/SingleQueryResponseWriterTest.groovy index 668a52b6..ae834c19 100644 --- a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/SingleQueryResponseWriterTest.groovy +++ b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/SingleQueryResponseWriterTest.groovy @@ -29,8 +29,8 @@ class SingleQueryResponseWriterTest extends Specification { 1 * responseMock.getOutputStream().write(expectedResponseContent.getBytes(StandardCharsets.UTF_8)) expect: - def writer = new SingleQueryResponseWriter(new ExecutionResultImpl(result, []), graphQLObjectMapperMock, null) - writer.write(requestMock, responseMock, null) + def writer = new SingleQueryResponseWriter(new ExecutionResultImpl(result, []), graphQLObjectMapperMock) + writer.write(requestMock, responseMock) where: result || expectedContentLenght | expectedResponseContent From 2316944a4c70266ab15af091ec4c830d7513fc9a Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Mon, 1 Jun 2020 18:21:12 +0800 Subject: [PATCH 11/13] Code review fixes. --- .../servlet/AbstractGraphQLHttpServlet.java | 7 ++- .../servlet/HttpRequestHandlerImpl.java | 39 +++++-------- .../servlet/QueryResponseWriter.java | 32 ++++++----- .../kickstart/servlet/cache/CacheReader.java | 8 +-- .../cache/CachingHttpRequestHandlerImpl.java | 56 +++++++++++++++++++ 5 files changed, 99 insertions(+), 43 deletions(-) create mode 100644 graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/AbstractGraphQLHttpServlet.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/AbstractGraphQLHttpServlet.java index 1a955f9e..d4b5add0 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/AbstractGraphQLHttpServlet.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/AbstractGraphQLHttpServlet.java @@ -7,6 +7,7 @@ import graphql.kickstart.execution.GraphQLQueryInvoker; import graphql.kickstart.execution.GraphQLRequest; import graphql.kickstart.execution.input.GraphQLSingleInvocationInput; +import graphql.kickstart.servlet.cache.CachingHttpRequestHandlerImpl; import graphql.schema.GraphQLFieldDefinition; import graphql.kickstart.servlet.core.GraphQLMBean; import graphql.kickstart.servlet.core.GraphQLServletListener; @@ -83,7 +84,11 @@ protected GraphQLConfiguration getConfiguration() { public void init() { if (configuration == null) { this.configuration = getConfiguration(); - this.requestHandler = new HttpRequestHandlerImpl(configuration); + if (configuration.getResponseCacheManager() != null) { + this.requestHandler = new CachingHttpRequestHandlerImpl(configuration); + } else { + this.requestHandler = new HttpRequestHandlerImpl(configuration); + } } } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java index 20e35769..25e9847b 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandlerImpl.java @@ -3,25 +3,23 @@ import graphql.GraphQLException; import graphql.kickstart.execution.GraphQLInvoker; import graphql.kickstart.execution.GraphQLQueryResult; +import graphql.kickstart.servlet.input.BatchInputPreProcessResult; +import graphql.kickstart.servlet.input.BatchInputPreProcessor; import graphql.kickstart.execution.input.GraphQLBatchedInvocationInput; import graphql.kickstart.execution.input.GraphQLInvocationInput; import graphql.kickstart.execution.input.GraphQLSingleInvocationInput; -import graphql.kickstart.servlet.cache.CacheReader; -import graphql.kickstart.servlet.input.BatchInputPreProcessResult; -import graphql.kickstart.servlet.input.BatchInputPreProcessor; -import lombok.extern.slf4j.Slf4j; - +import java.io.IOException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import java.io.IOException; +import lombok.extern.slf4j.Slf4j; @Slf4j -class HttpRequestHandlerImpl implements HttpRequestHandler { +public class HttpRequestHandlerImpl implements HttpRequestHandler { private final GraphQLConfiguration configuration; private final GraphQLInvoker graphQLInvoker; - HttpRequestHandlerImpl(GraphQLConfiguration configuration) { + public HttpRequestHandlerImpl(GraphQLConfiguration configuration) { this.configuration = configuration; graphQLInvoker = configuration.getGraphQLInvoker(); } @@ -48,25 +46,13 @@ public void handle(HttpServletRequest request, HttpServletResponse response) thr } } - private void execute(GraphQLInvocationInput invocationInput, HttpServletRequest request, + protected void execute(GraphQLInvocationInput invocationInput, HttpServletRequest request, HttpServletResponse response) { try { - // try to return value from cache if cache manager was set, otherwise processed the query - boolean returnedFromCache = configuration.getResponseCacheManager() != null && - !CacheReader.responseFromCache(invocationInput, request, response, configuration.getResponseCacheManager()); + GraphQLQueryResult queryResult = invoke(invocationInput, request, response); - if (!returnedFromCache) { - GraphQLQueryResult queryResult = invoke(invocationInput, request, response); - - QueryResponseWriter queryResponseWriter = QueryResponseWriter.createWriter( - queryResult, - configuration.getObjectMapper(), - configuration.getSubscriptionTimeout(), - invocationInput, - configuration.getResponseCacheManager() - ); - queryResponseWriter.write(request, response); - } + QueryResponseWriter queryResponseWriter = createWriter(invocationInput, queryResult); + queryResponseWriter.write(request, response); } catch (Throwable t) { response.setStatus(STATUS_BAD_REQUEST); log.info("Bad GET request: path was not \"/schema.json\" or no query variable named \"query\" given"); @@ -74,6 +60,11 @@ private void execute(GraphQLInvocationInput invocationInput, HttpServletRequest } } + protected QueryResponseWriter createWriter(GraphQLInvocationInput invocationInput, GraphQLQueryResult queryResult) { + return QueryResponseWriter.createWriter(queryResult, configuration.getObjectMapper(), + configuration.getSubscriptionTimeout()); + } + private GraphQLQueryResult invoke(GraphQLInvocationInput invocationInput, HttpServletRequest request, HttpServletResponse response) { if (invocationInput instanceof GraphQLSingleInvocationInput) { diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java index d58724a1..9334ff53 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java @@ -1,41 +1,45 @@ package graphql.kickstart.servlet; -import graphql.kickstart.execution.GraphQLQueryResult; import graphql.kickstart.execution.GraphQLObjectMapper; +import graphql.kickstart.execution.GraphQLQueryResult; import graphql.kickstart.execution.input.GraphQLInvocationInput; import graphql.kickstart.servlet.cache.CachingQueryResponseWriter; import graphql.kickstart.servlet.cache.GraphQLResponseCacheManager; -import java.io.IOException; -import java.util.Objects; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.Objects; public interface QueryResponseWriter { static QueryResponseWriter createWriter( GraphQLQueryResult result, GraphQLObjectMapper graphQLObjectMapper, - long subscriptionTimeout, - GraphQLInvocationInput invocationInput, - GraphQLResponseCacheManager responseCache + long subscriptionTimeout ) { Objects.requireNonNull(result, "GraphQL query result cannot be null"); - QueryResponseWriter writer; - if (result.isBatched()) { - writer = new BatchedQueryResponseWriter(result.getResults(), graphQLObjectMapper); + return new BatchedQueryResponseWriter(result.getResults(), graphQLObjectMapper); } else if (result.isAsynchronous()) { - writer = new SingleAsynchronousQueryResponseWriter(result.getResult(), graphQLObjectMapper, subscriptionTimeout); + return new SingleAsynchronousQueryResponseWriter(result.getResult(), graphQLObjectMapper, subscriptionTimeout); } else if (result.isError()) { - writer = new ErrorQueryResponseWriter(result.getStatusCode(), result.getMessage()); - } else { - writer = new SingleQueryResponseWriter(result.getResult(), graphQLObjectMapper); + return new ErrorQueryResponseWriter(result.getStatusCode(), result.getMessage()); } + return new SingleQueryResponseWriter(result.getResult(), graphQLObjectMapper); + } + static QueryResponseWriter createCacheWriter( + GraphQLQueryResult result, + GraphQLObjectMapper graphQLObjectMapper, + long subscriptionTimeout, + GraphQLInvocationInput invocationInput, + GraphQLResponseCacheManager responseCache + ) { + QueryResponseWriter writer = createWriter(result, graphQLObjectMapper, subscriptionTimeout); if (responseCache != null) { - writer = new CachingQueryResponseWriter(writer, responseCache, invocationInput, result.isError()); + return new CachingQueryResponseWriter(writer, responseCache, invocationInput, result.isError()); } return writer; } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheReader.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheReader.java index c8297c1c..52ffaae5 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheReader.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheReader.java @@ -23,9 +23,9 @@ private CacheReader() { * @throws IOException if can not read value from the cache */ public static boolean responseFromCache(GraphQLInvocationInput invocationInput, - HttpServletRequest request, - HttpServletResponse response, - GraphQLResponseCacheManager cacheManager) throws IOException { + HttpServletRequest request, + HttpServletResponse response, + GraphQLResponseCacheManager cacheManager) throws IOException { CachedResponse cachedResponse = null; try { cachedResponse = cacheManager.get(request, invocationInput); @@ -42,7 +42,7 @@ public static boolean responseFromCache(GraphQLInvocationInput invocationInput, } private static void write(HttpServletResponse response, CachedResponse cachedResponse) - throws IOException { + throws IOException { if (cachedResponse.isError()) { response.sendError(cachedResponse.getErrorStatusCode(), cachedResponse.getErrorMessage()); } else { diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java new file mode 100644 index 00000000..b0e7a20d --- /dev/null +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java @@ -0,0 +1,56 @@ +package graphql.kickstart.servlet.cache; + +import graphql.kickstart.execution.GraphQLQueryResult; +import graphql.kickstart.execution.input.GraphQLInvocationInput; +import graphql.kickstart.servlet.GraphQLConfiguration; +import graphql.kickstart.servlet.HttpRequestHandlerImpl; +import graphql.kickstart.servlet.QueryResponseWriter; +import lombok.extern.slf4j.Slf4j; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.Objects; + +@Slf4j +public class CachingHttpRequestHandlerImpl extends HttpRequestHandlerImpl { + + private final GraphQLConfiguration configuration; + + public CachingHttpRequestHandlerImpl(GraphQLConfiguration configuration) { + super(configuration); + Objects.requireNonNull(configuration.getResponseCacheManager(), "Response Cache Manager cannot be null"); + this.configuration = configuration; + } + + protected void execute(GraphQLInvocationInput invocationInput, HttpServletRequest request, + HttpServletResponse response) { + // try to return value from cache if cache exists, otherwise processed the query + boolean returnedFromCache; + + try { + returnedFromCache = !CacheReader.responseFromCache( + invocationInput, request, response, configuration.getResponseCacheManager() + ); + } catch (IOException e) { + response.setStatus(STATUS_BAD_REQUEST); + log.warn("unexpected error happened during response from cache", e); + return; + } + + if (!returnedFromCache) { + super.execute(invocationInput, request, response); + } + } + + protected QueryResponseWriter createWriter(GraphQLInvocationInput invocationInput, GraphQLQueryResult queryResult) { + return QueryResponseWriter.createCacheWriter( + queryResult, + configuration.getObjectMapper(), + configuration.getSubscriptionTimeout(), + invocationInput, + configuration.getResponseCacheManager() + ); + } + +} From ac6fc42bca7a066812abad60d6395e8ff36c3b88 Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Mon, 1 Jun 2020 19:10:00 +0800 Subject: [PATCH 12/13] Moved createCacheWriter from QueryResponseWriter to CachingQueryResponseWriter. --- .../kickstart/servlet/QueryResponseWriter.java | 17 ----------------- .../cache/CachingHttpRequestHandlerImpl.java | 2 +- .../cache/CachingQueryResponseWriter.java | 16 ++++++++++++++++ 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java index 9334ff53..41550a95 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/QueryResponseWriter.java @@ -2,9 +2,6 @@ import graphql.kickstart.execution.GraphQLObjectMapper; import graphql.kickstart.execution.GraphQLQueryResult; -import graphql.kickstart.execution.input.GraphQLInvocationInput; -import graphql.kickstart.servlet.cache.CachingQueryResponseWriter; -import graphql.kickstart.servlet.cache.GraphQLResponseCacheManager; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -30,20 +27,6 @@ static QueryResponseWriter createWriter( return new SingleQueryResponseWriter(result.getResult(), graphQLObjectMapper); } - static QueryResponseWriter createCacheWriter( - GraphQLQueryResult result, - GraphQLObjectMapper graphQLObjectMapper, - long subscriptionTimeout, - GraphQLInvocationInput invocationInput, - GraphQLResponseCacheManager responseCache - ) { - QueryResponseWriter writer = createWriter(result, graphQLObjectMapper, subscriptionTimeout); - if (responseCache != null) { - return new CachingQueryResponseWriter(writer, responseCache, invocationInput, result.isError()); - } - return writer; - } - void write(HttpServletRequest request, HttpServletResponse response) throws IOException; } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java index b0e7a20d..854e51a5 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java @@ -44,7 +44,7 @@ protected void execute(GraphQLInvocationInput invocationInput, HttpServletReques } protected QueryResponseWriter createWriter(GraphQLInvocationInput invocationInput, GraphQLQueryResult queryResult) { - return QueryResponseWriter.createCacheWriter( + return CachingQueryResponseWriter.createCacheWriter( queryResult, configuration.getObjectMapper(), configuration.getSubscriptionTimeout(), diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingQueryResponseWriter.java index 0cd9c703..0b5513ce 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingQueryResponseWriter.java @@ -1,5 +1,7 @@ package graphql.kickstart.servlet.cache; +import graphql.kickstart.execution.GraphQLObjectMapper; +import graphql.kickstart.execution.GraphQLQueryResult; import graphql.kickstart.execution.input.GraphQLInvocationInput; import graphql.kickstart.servlet.QueryResponseWriter; import lombok.extern.slf4j.Slf4j; @@ -11,6 +13,20 @@ @Slf4j public class CachingQueryResponseWriter implements QueryResponseWriter { + static QueryResponseWriter createCacheWriter( + GraphQLQueryResult result, + GraphQLObjectMapper graphQLObjectMapper, + long subscriptionTimeout, + GraphQLInvocationInput invocationInput, + GraphQLResponseCacheManager responseCache + ) { + QueryResponseWriter writer = QueryResponseWriter.createWriter(result, graphQLObjectMapper, subscriptionTimeout); + if (responseCache != null) { + return new CachingQueryResponseWriter(writer, responseCache, invocationInput, result.isError()); + } + return writer; + } + private final QueryResponseWriter delegate; private final GraphQLResponseCacheManager responseCache; private final GraphQLInvocationInput invocationInput; From 315b77faa6dd82c613f68e6e6cf9781395c5e7dd Mon Sep 17 00:00:00 2001 From: Alexey Zhokhov Date: Mon, 1 Jun 2020 20:23:41 +0800 Subject: [PATCH 13/13] Added Override annotation. --- .../kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java index 854e51a5..43dd7e2c 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestHandlerImpl.java @@ -23,6 +23,7 @@ public CachingHttpRequestHandlerImpl(GraphQLConfiguration configuration) { this.configuration = configuration; } + @Override protected void execute(GraphQLInvocationInput invocationInput, HttpServletRequest request, HttpServletResponse response) { // try to return value from cache if cache exists, otherwise processed the query