From 8696a81135be28facb59d9186c4bbd8f97048ed0 Mon Sep 17 00:00:00 2001 From: jnutting Date: Mon, 11 Feb 2019 17:49:09 -0600 Subject: [PATCH 1/2] Use thread pool executor for async queries --- .../servlet/AbstractGraphQLHttpServlet.java | 4 +-- .../graphql/servlet/GraphQLConfiguration.java | 23 +++++++++++++-- .../internal/GraphQLThreadFactory.java | 26 +++++++++++++++++ .../AbstractGraphQLHttpServletSpec.groovy | 28 +++++++++++++++++++ .../groovy/graphql/servlet/TestUtils.groovy | 7 +++-- 5 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 src/main/java/graphql/servlet/internal/GraphQLThreadFactory.java diff --git a/src/main/java/graphql/servlet/AbstractGraphQLHttpServlet.java b/src/main/java/graphql/servlet/AbstractGraphQLHttpServlet.java index 4f080881..d624655a 100644 --- a/src/main/java/graphql/servlet/AbstractGraphQLHttpServlet.java +++ b/src/main/java/graphql/servlet/AbstractGraphQLHttpServlet.java @@ -51,7 +51,7 @@ public abstract class AbstractGraphQLHttpServlet extends HttpServlet implements private static final String[] MULTIPART_KEYS = new String[]{"operations", "graphql", "query"}; private GraphQLConfiguration configuration; - + /** * @deprecated override {@link #getConfiguration()} instead */ @@ -292,7 +292,7 @@ private void doRequestAsync(HttpServletRequest request, HttpServletResponse resp AsyncContext asyncContext = request.startAsync(); HttpServletRequest asyncRequest = (HttpServletRequest) asyncContext.getRequest(); HttpServletResponse asyncResponse = (HttpServletResponse) asyncContext.getResponse(); - new Thread(() -> doRequest(asyncRequest, asyncResponse, handler, asyncContext)).start(); + configuration.getAsyncExecutor().execute(() -> doRequest(asyncRequest, asyncResponse, handler, asyncContext)); } else { doRequest(request, response, handler, null); } diff --git a/src/main/java/graphql/servlet/GraphQLConfiguration.java b/src/main/java/graphql/servlet/GraphQLConfiguration.java index facdd33e..ef8bc2e2 100644 --- a/src/main/java/graphql/servlet/GraphQLConfiguration.java +++ b/src/main/java/graphql/servlet/GraphQLConfiguration.java @@ -1,10 +1,12 @@ package graphql.servlet; import graphql.schema.GraphQLSchema; +import graphql.servlet.internal.GraphQLThreadFactory; import java.util.ArrayList; import java.util.List; -import java.util.Objects; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; public class GraphQLConfiguration { @@ -13,6 +15,7 @@ public class GraphQLConfiguration { private GraphQLObjectMapper objectMapper; private List listeners; private boolean asyncServletModeEnabled; + private Executor asyncExecutor; public static GraphQLConfiguration.Builder with(GraphQLSchema schema) { return with(new DefaultGraphQLSchemaProvider(schema)); @@ -26,12 +29,13 @@ public static GraphQLConfiguration.Builder with(GraphQLInvocationInputFactory in return new Builder(invocationInputFactory); } - private GraphQLConfiguration(GraphQLInvocationInputFactory invocationInputFactory, GraphQLQueryInvoker queryInvoker, GraphQLObjectMapper objectMapper, List listeners, boolean asyncServletModeEnabled) { + private GraphQLConfiguration(GraphQLInvocationInputFactory invocationInputFactory, GraphQLQueryInvoker queryInvoker, GraphQLObjectMapper objectMapper, List listeners, boolean asyncServletModeEnabled, Executor asyncExecutor) { this.invocationInputFactory = invocationInputFactory; this.queryInvoker = queryInvoker; this.objectMapper = objectMapper; this.listeners = listeners; this.asyncServletModeEnabled = asyncServletModeEnabled; + this.asyncExecutor = asyncExecutor; } public GraphQLInvocationInputFactory getInvocationInputFactory() { @@ -54,6 +58,10 @@ public boolean isAsyncServletModeEnabled() { return asyncServletModeEnabled; } + public Executor getAsyncExecutor() { + return asyncExecutor; + } + public void add(GraphQLServletListener listener) { listeners.add(listener); } @@ -70,6 +78,7 @@ public static class Builder { private GraphQLObjectMapper objectMapper = GraphQLObjectMapper.newBuilder().build(); private List listeners = new ArrayList<>(); private boolean asyncServletModeEnabled = false; + private Executor asyncExecutor = Executors.newCachedThreadPool(new GraphQLThreadFactory()); private Builder(GraphQLInvocationInputFactory.Builder invocationInputFactoryBuilder) { this.invocationInputFactoryBuilder = invocationInputFactoryBuilder; @@ -105,6 +114,13 @@ public Builder with(boolean asyncServletModeEnabled) { return this; } + public Builder with(Executor asyncExecutor) { + if (asyncExecutor != null) { + this.asyncExecutor = asyncExecutor; + } + return this; + } + public Builder with(GraphQLContextBuilder contextBuilder) { this.invocationInputFactoryBuilder.withGraphQLContextBuilder(contextBuilder); return this; @@ -121,7 +137,8 @@ public GraphQLConfiguration build() { queryInvoker, objectMapper, listeners, - asyncServletModeEnabled + asyncServletModeEnabled, + asyncExecutor ); } diff --git a/src/main/java/graphql/servlet/internal/GraphQLThreadFactory.java b/src/main/java/graphql/servlet/internal/GraphQLThreadFactory.java new file mode 100644 index 00000000..34fdb709 --- /dev/null +++ b/src/main/java/graphql/servlet/internal/GraphQLThreadFactory.java @@ -0,0 +1,26 @@ +package graphql.servlet.internal; + +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicInteger; + +import graphql.servlet.AbstractGraphQLHttpServlet; + +/** + * {@link ThreadFactory} implementation for {@link AbstractGraphQLHttpServlet} async operations + * + * @author John Nutting + */ +public class GraphQLThreadFactory implements ThreadFactory { + + final static String NAME_PREFIX = "GraphQLServlet-"; + final AtomicInteger threadNumber = new AtomicInteger(1); + + @Override + public Thread newThread(final Runnable r) { + Thread t = new Thread(r, NAME_PREFIX + threadNumber.getAndIncrement()); + t.setDaemon(false); + t.setPriority(Thread.NORM_PRIORITY); + return t; + } + +} \ No newline at end of file diff --git a/src/test/groovy/graphql/servlet/AbstractGraphQLHttpServletSpec.groovy b/src/test/groovy/graphql/servlet/AbstractGraphQLHttpServletSpec.groovy index 86534f72..c50e5739 100644 --- a/src/test/groovy/graphql/servlet/AbstractGraphQLHttpServletSpec.groovy +++ b/src/test/groovy/graphql/servlet/AbstractGraphQLHttpServletSpec.groovy @@ -7,6 +7,7 @@ import graphql.execution.ExecutionStepInfo import graphql.execution.instrumentation.ChainedInstrumentation import graphql.execution.instrumentation.Instrumentation +import graphql.schema.DataFetcher import graphql.schema.GraphQLNonNull import org.dataloader.DataLoaderRegistry import org.springframework.mock.web.MockHttpServletRequest @@ -38,6 +39,7 @@ class AbstractGraphQLHttpServletSpec extends Specification { def setup() { servlet = TestUtils.createServlet() request = new MockHttpServletRequest() + request.setAsyncSupported(true) response = new MockHttpServletResponse() } @@ -84,6 +86,18 @@ class AbstractGraphQLHttpServletSpec extends Specification { getResponseContent().data.echo == "test" } + def "async query over HTTP GET starts async request"() { + setup: + servlet = TestUtils.createServlet({ env -> env.arguments.arg },{ env -> env.arguments.arg }, true) + request.addParameter('query', 'query { echo(arg:"test") }') + + when: + servlet.doGet(request, response) + + then: + request.asyncStarted == true + } + def "query over HTTP GET with variables returns data"() { setup: request.addParameter('query', 'query Echo($arg: String) { echo(arg:$arg) }') @@ -286,6 +300,20 @@ class AbstractGraphQLHttpServletSpec extends Specification { getResponseContent().data.echo == "test" } + def "async query over HTTP POST starts async request"() { + setup: + servlet = TestUtils.createServlet({ env -> env.arguments.arg },{ env -> env.arguments.arg }, true) + request.setContent(mapper.writeValueAsBytes([ + query: 'query { echo(arg:"test") }' + ])) + + when: + servlet.doPost(request, response) + + then: + request.asyncStarted == true + } + def "query over HTTP POST body with graphql contentType returns data"() { setup: request.addHeader("Content-Type", "application/graphql") diff --git a/src/test/groovy/graphql/servlet/TestUtils.groovy b/src/test/groovy/graphql/servlet/TestUtils.groovy index c91e8d06..977c914d 100644 --- a/src/test/groovy/graphql/servlet/TestUtils.groovy +++ b/src/test/groovy/graphql/servlet/TestUtils.groovy @@ -8,10 +8,13 @@ import graphql.schema.* class TestUtils { static def createServlet(DataFetcher queryDataFetcher = { env -> env.arguments.arg }, - DataFetcher mutationDataFetcher = { env -> env.arguments.arg }) { + DataFetcher mutationDataFetcher = { env -> env.arguments.arg }, + boolean asyncServletModeEnabled = false) { GraphQLHttpServlet servlet = GraphQLHttpServlet.with(GraphQLConfiguration .with(createGraphQlSchema(queryDataFetcher, mutationDataFetcher)) - .with(createInstrumentedQueryInvoker()).build()) + .with(createInstrumentedQueryInvoker()) + .with(asyncServletModeEnabled) + .build()) servlet.init(null) return servlet } From 43d00d2969d74b6c8b2b04b33e6dda2b44c4e73d Mon Sep 17 00:00:00 2001 From: Michiel Oliemans Date: Sat, 16 Feb 2019 10:11:28 +0100 Subject: [PATCH 2/2] Fix build errors --- .../graphql/servlet/GraphQLConfiguration.java | 2 +- .../groovy/graphql/servlet/TestUtils.groovy | 41 ++++++++++--------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/main/java/graphql/servlet/GraphQLConfiguration.java b/src/main/java/graphql/servlet/GraphQLConfiguration.java index 25f1ea56..1683a328 100644 --- a/src/main/java/graphql/servlet/GraphQLConfiguration.java +++ b/src/main/java/graphql/servlet/GraphQLConfiguration.java @@ -30,7 +30,7 @@ public static GraphQLConfiguration.Builder with(GraphQLInvocationInputFactory in return new Builder(invocationInputFactory); } - private GraphQLConfiguration(GraphQLInvocationInputFactory invocationInputFactory, GraphQLQueryInvoker queryInvoker, GraphQLObjectMapper objectMapper, List listeners, boolean asyncServletModeEnabled, Executor asyncExecutor, , long subscriptionTimeout) { + private GraphQLConfiguration(GraphQLInvocationInputFactory invocationInputFactory, GraphQLQueryInvoker queryInvoker, GraphQLObjectMapper objectMapper, List listeners, boolean asyncServletModeEnabled, Executor asyncExecutor, long subscriptionTimeout) { this.invocationInputFactory = invocationInputFactory; this.queryInvoker = queryInvoker; this.objectMapper = objectMapper; diff --git a/src/test/groovy/graphql/servlet/TestUtils.groovy b/src/test/groovy/graphql/servlet/TestUtils.groovy index 9d1891b9..01c64716 100644 --- a/src/test/groovy/graphql/servlet/TestUtils.groovy +++ b/src/test/groovy/graphql/servlet/TestUtils.groovy @@ -5,7 +5,6 @@ import graphql.Scalars import graphql.execution.instrumentation.Instrumentation import graphql.execution.reactive.SingleSubscriberPublisher import graphql.schema.* -import org.reactivestreams.Publisher import java.util.concurrent.atomic.AtomicReference @@ -13,15 +12,15 @@ class TestUtils { static def createServlet(DataFetcher queryDataFetcher = { env -> env.arguments.arg }, DataFetcher mutationDataFetcher = { env -> env.arguments.arg }, - boolean asyncServletModeEnabled = false) { - DataFetcher subscriptionDataFetcher = { env -> - AtomicReference> publisherRef = new AtomicReference<>(); - publisherRef.set(new SingleSubscriberPublisher<>({ subscription -> - publisherRef.get().offer(env.arguments.arg) - publisherRef.get().noMoreData() - })) - return publisherRef.get() - }) { + boolean asyncServletModeEnabled = false, + DataFetcher subscriptionDataFetcher = { env -> + AtomicReference> publisherRef = new AtomicReference<>(); + publisherRef.set(new SingleSubscriberPublisher<>({ subscription -> + publisherRef.get().offer(env.arguments.arg) + publisherRef.get().noMoreData() + })) + return publisherRef.get() + }) { GraphQLHttpServlet servlet = GraphQLHttpServlet.with(GraphQLConfiguration .with(createGraphQlSchema(queryDataFetcher, mutationDataFetcher, subscriptionDataFetcher)) .with(createInstrumentedQueryInvoker()) @@ -75,23 +74,27 @@ class TestUtils { } field.dataFetcher(mutationDataFetcher) } - .field { field -> + .field { field -> field.name("echoFile") field.type(Scalars.GraphQLString) field.argument { argument -> argument.name("file") argument.type(ApolloScalars.Upload) } - field.dataFetcher( { env -> new String(ByteStreams.toByteArray(env.arguments.file.getInputStream())) } ) + field.dataFetcher({ env -> new String(ByteStreams.toByteArray(env.arguments.file.getInputStream())) }) } - .field { field -> + .field { field -> field.name("echoFiles") field.type(GraphQLList.list(Scalars.GraphQLString)) field.argument { argument -> argument.name("files") argument.type(GraphQLList.list(GraphQLNonNull.nonNull(ApolloScalars.Upload))) } - field.dataFetcher( { env -> env.arguments.files.collect { new String(ByteStreams.toByteArray(it.getInputStream())) } } ) + field.dataFetcher({ env -> + env.arguments.files.collect { + new String(ByteStreams.toByteArray(it.getInputStream())) + } + }) } .build() @@ -110,11 +113,11 @@ class TestUtils { return GraphQLSchema.newSchema() - .query(query) - .mutation(mutation) - .subscription(subscription) - .additionalType(ApolloScalars.Upload) - .build() + .query(query) + .mutation(mutation) + .subscription(subscription) + .additionalType(ApolloScalars.Upload) + .build() } }