Skip to content

Commit b44bc85

Browse files
oliemansmoliemansm
authored
Merge pull request #154 from jmccaull/refactor_batch_handling
Moved injection of Batch handler from the servlet into query invoker
2 parents f18a083 + 15f5fb4 commit b44bc85

14 files changed

+107
-184
lines changed

‎src/main/java/graphql/servlet/AbstractGraphQLHttpServlet.java

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,6 @@ public abstract class AbstractGraphQLHttpServlet extends HttpServlet implements
7373
@Deprecated
7474
protected abstract GraphQLObjectMapper getGraphQLObjectMapper();
7575

76-
/**
77-
* @deprecated override {@link #getConfiguration()} instead
78-
*/
79-
@Deprecated
80-
protected abstract GraphQLBatchExecutionHandlerFactory getBatchExecutionHandlerFactory();
81-
8276
/**
8377
* @deprecated override {@link #getConfiguration()} instead
8478
*/
@@ -91,7 +85,6 @@ protected GraphQLConfiguration getConfiguration() {
9185
.with(getGraphQLObjectMapper())
9286
.with(isAsyncServletMode())
9387
.with(listeners)
94-
.with(getBatchExecutionHandlerFactory())
9588
.build();
9689
}
9790

@@ -120,7 +113,6 @@ public void init(ServletConfig servletConfig) {
120113
GraphQLInvocationInputFactory invocationInputFactory = configuration.getInvocationInputFactory();
121114
GraphQLObjectMapper graphQLObjectMapper = configuration.getObjectMapper();
122115
GraphQLQueryInvoker queryInvoker = configuration.getQueryInvoker();
123-
GraphQLBatchExecutionHandlerFactory batchExecutionHandlerFactory = configuration.getBatchExecutionHandlerFactory();
124116

125117
String path = request.getPathInfo();
126118
if (path == null) {
@@ -133,7 +125,7 @@ public void init(ServletConfig servletConfig) {
133125
if (query != null) {
134126

135127
if (isBatchedQuery(query)) {
136-
queryBatched(batchExecutionHandlerFactory, queryInvoker, graphQLObjectMapper, invocationInputFactory.createReadOnly(graphQLObjectMapper.readBatchedGraphQLRequest(query), request, response), response);
128+
queryBatched(queryInvoker, graphQLObjectMapper, invocationInputFactory.createReadOnly(graphQLObjectMapper.readBatchedGraphQLRequest(query), request, response), response);
137129
} else {
138130
final Map<String, Object> variables = new HashMap<>();
139131
if (request.getParameter("variables") != null) {
@@ -155,7 +147,6 @@ public void init(ServletConfig servletConfig) {
155147
GraphQLInvocationInputFactory invocationInputFactory = configuration.getInvocationInputFactory();
156148
GraphQLObjectMapper graphQLObjectMapper = configuration.getObjectMapper();
157149
GraphQLQueryInvoker queryInvoker = configuration.getQueryInvoker();
158-
GraphQLBatchExecutionHandlerFactory batchExecutionHandlerFactory = configuration.getBatchExecutionHandlerFactory();
159150

160151
try {
161152
if (APPLICATION_GRAPHQL.equals(request.getContentType())) {
@@ -190,7 +181,7 @@ public void init(ServletConfig servletConfig) {
190181
GraphQLBatchedInvocationInput invocationInput =
191182
invocationInputFactory.create(graphQLRequests, request, response);
192183
invocationInput.getContext().setParts(fileItems);
193-
queryBatched(batchExecutionHandlerFactory, queryInvoker, graphQLObjectMapper, invocationInput, response);
184+
queryBatched(queryInvoker, graphQLObjectMapper, invocationInput, response);
194185
return;
195186
} else {
196187
GraphQLRequest graphQLRequest;
@@ -216,7 +207,7 @@ public void init(ServletConfig servletConfig) {
216207
InputStream inputStream = asMarkableInputStream(request.getInputStream());
217208

218209
if (isBatchedQuery(inputStream)) {
219-
queryBatched(batchExecutionHandlerFactory, queryInvoker, graphQLObjectMapper, invocationInputFactory.create(graphQLObjectMapper.readBatchedGraphQLRequest(inputStream), request, response), response);
210+
queryBatched(queryInvoker, graphQLObjectMapper, invocationInputFactory.create(graphQLObjectMapper.readBatchedGraphQLRequest(inputStream), request, response), response);
220211
} else {
221212
query(queryInvoker, graphQLObjectMapper, invocationInputFactory.create(graphQLObjectMapper.readGraphQLRequest(inputStream), request, response), response);
222213
}
@@ -373,13 +364,13 @@ private void query(GraphQLQueryInvoker queryInvoker, GraphQLObjectMapper graphQL
373364
}
374365
}
375366

376-
private void queryBatched(GraphQLBatchExecutionHandlerFactory batchInputHandlerFactory, GraphQLQueryInvoker queryInvoker, GraphQLObjectMapper graphQLObjectMapper, GraphQLBatchedInvocationInput invocationInput, HttpServletResponse resp) throws Exception {
367+
private void queryBatched(GraphQLQueryInvoker queryInvoker, GraphQLObjectMapper graphQLObjectMapper, GraphQLBatchedInvocationInput invocationInput, HttpServletResponse resp) throws Exception {
377368
resp.setContentType(APPLICATION_JSON_UTF8);
378369
resp.setStatus(STATUS_OK);
379370

380371
Writer respWriter = resp.getWriter();
381372

382-
queryInvoker.query(invocationInput, batchInputHandlerFactory.getBatchHandler(respWriter, graphQLObjectMapper));
373+
queryInvoker.query(invocationInput, respWriter, graphQLObjectMapper);
383374
}
384375

385376
private <R> List<R> runListeners(Function<? super GraphQLServletListener, R> action) {

‎src/main/java/graphql/servlet/BatchExecutionHandler.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,22 @@
33
import graphql.ExecutionInput;
44
import graphql.ExecutionResult;
55

6+
import java.io.Writer;
67
import java.util.function.BiFunction;
78

89
/**
910
* @author Andrew Potter
1011
*/
1112
public interface BatchExecutionHandler {
13+
1214
/**
1315
* Allows separating the logic of handling batch queries from how each individual query is resolved.
1416
* @param batchedInvocationInput the batch query input
1517
* @param queryFunction Function to produce query results.
18+
* @param graphQLObjectMapper object mapper used to serialize results
19+
* @param writer request writer to ouput results.
1620
*/
17-
void handleBatch(GraphQLBatchedInvocationInput batchedInvocationInput, BiFunction<GraphQLInvocationInput, ExecutionInput, ExecutionResult> queryFunction);
21+
void handleBatch(GraphQLBatchedInvocationInput batchedInvocationInput, Writer writer, GraphQLObjectMapper graphQLObjectMapper,
22+
BiFunction<GraphQLInvocationInput, ExecutionInput, ExecutionResult> queryFunction);
1823
}
1924

‎src/main/java/graphql/servlet/DeafultBatchExecutionHandler.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package graphql.servlet;
2+
3+
import graphql.ExecutionInput;
4+
import graphql.ExecutionResult;
5+
6+
import java.io.IOException;
7+
import java.io.Writer;
8+
import java.util.Iterator;
9+
import java.util.function.BiFunction;
10+
11+
public class DeafultBatchExecutionHandler implements BatchExecutionHandler {
12+
13+
@Override
14+
public void handleBatch(GraphQLBatchedInvocationInput batchedInvocationInput, Writer writer, GraphQLObjectMapper graphQLObjectMapper,
15+
BiFunction<GraphQLInvocationInput, ExecutionInput, ExecutionResult> queryFunction) {
16+
Iterator<ExecutionInput> executionInputIterator = batchedInvocationInput.getExecutionInputs().iterator();
17+
try {
18+
writer.write("[");
19+
while (executionInputIterator.hasNext()) {
20+
ExecutionResult result = queryFunction.apply(batchedInvocationInput, executionInputIterator.next());
21+
writer.write(graphQLObjectMapper.serializeResultAsJson(result));
22+
if (executionInputIterator.hasNext()) {
23+
writer.write(",");
24+
}
25+
}
26+
writer.write("]");
27+
} catch (IOException e) {
28+
throw new RuntimeException(e);
29+
}
30+
}
31+
}
32+

‎src/main/java/graphql/servlet/DefaultGraphQLBatchExecutionHandlerFactory.java

Lines changed: 0 additions & 47 deletions
This file was deleted.

‎src/main/java/graphql/servlet/DefaultGraphQLServlet.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,6 @@ protected GraphQLObjectMapper getGraphQLObjectMapper() {
2525
return GraphQLObjectMapper.newBuilder().build();
2626
}
2727

28-
@Override
29-
protected GraphQLBatchExecutionHandlerFactory getBatchExecutionHandlerFactory() {
30-
return null;
31-
}
32-
3328
@Override
3429
protected boolean isAsyncServletMode() {
3530
return false;

‎src/main/java/graphql/servlet/GraphQLBatchExecutionHandlerFactory.java

Lines changed: 0 additions & 16 deletions
This file was deleted.

‎src/main/java/graphql/servlet/GraphQLConfiguration.java

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ public class GraphQLConfiguration {
1313
private GraphQLInvocationInputFactory invocationInputFactory;
1414
private GraphQLQueryInvoker queryInvoker;
1515
private GraphQLObjectMapper objectMapper;
16-
private GraphQLBatchExecutionHandlerFactory batchExecutionHandlerFactory;
1716
private List<GraphQLServletListener> listeners;
1817
private boolean asyncServletModeEnabled;
1918
private Executor asyncExecutor;
@@ -31,11 +30,10 @@ public static GraphQLConfiguration.Builder with(GraphQLInvocationInputFactory in
3130
return new Builder(invocationInputFactory);
3231
}
3332

34-
private GraphQLConfiguration(GraphQLInvocationInputFactory invocationInputFactory, GraphQLQueryInvoker queryInvoker, GraphQLObjectMapper objectMapper, GraphQLBatchExecutionHandlerFactory batchExecutionHandlerFactory, List<GraphQLServletListener> listeners, boolean asyncServletModeEnabled, Executor asyncExecutor, long subscriptionTimeout) {
33+
private GraphQLConfiguration(GraphQLInvocationInputFactory invocationInputFactory, GraphQLQueryInvoker queryInvoker, GraphQLObjectMapper objectMapper, List<GraphQLServletListener> listeners, boolean asyncServletModeEnabled, Executor asyncExecutor, long subscriptionTimeout) {
3534
this.invocationInputFactory = invocationInputFactory;
3635
this.queryInvoker = queryInvoker;
3736
this.objectMapper = objectMapper;
38-
this.batchExecutionHandlerFactory = batchExecutionHandlerFactory;
3937
this.listeners = listeners;
4038
this.asyncServletModeEnabled = asyncServletModeEnabled;
4139
this.asyncExecutor = asyncExecutor;
@@ -54,10 +52,6 @@ public GraphQLObjectMapper getObjectMapper() {
5452
return objectMapper;
5553
}
5654

57-
public GraphQLBatchExecutionHandlerFactory getBatchExecutionHandlerFactory() {
58-
return batchExecutionHandlerFactory;
59-
}
60-
6155
public List<GraphQLServletListener> getListeners() {
6256
return new ArrayList<>(listeners);
6357
}
@@ -88,7 +82,6 @@ public static class Builder {
8882
private GraphQLInvocationInputFactory invocationInputFactory;
8983
private GraphQLQueryInvoker queryInvoker = GraphQLQueryInvoker.newBuilder().build();
9084
private GraphQLObjectMapper objectMapper = GraphQLObjectMapper.newBuilder().build();
91-
private GraphQLBatchExecutionHandlerFactory graphQLBatchExecutionHandlerFactory = new DefaultGraphQLBatchExecutionHandlerFactory();
9285
private List<GraphQLServletListener> listeners = new ArrayList<>();
9386
private boolean asyncServletModeEnabled = false;
9487
private Executor asyncExecutor = Executors.newCachedThreadPool(new GraphQLThreadFactory());
@@ -116,13 +109,6 @@ public Builder with(GraphQLObjectMapper objectMapper) {
116109
return this;
117110
}
118111

119-
public Builder with(GraphQLBatchExecutionHandlerFactory batchExecutionHandlerFactory) {
120-
if (batchExecutionHandlerFactory != null) {
121-
this.graphQLBatchExecutionHandlerFactory = batchExecutionHandlerFactory;
122-
}
123-
return this;
124-
}
125-
126112
public Builder with(List<GraphQLServletListener> listeners) {
127113
if (listeners != null) {
128114
this.listeners = listeners;
@@ -162,7 +148,6 @@ public GraphQLConfiguration build() {
162148
this.invocationInputFactory != null ? this.invocationInputFactory : invocationInputFactoryBuilder.build(),
163149
queryInvoker,
164150
objectMapper,
165-
graphQLBatchExecutionHandlerFactory,
166151
listeners,
167152
asyncServletModeEnabled,
168153
asyncExecutor,

‎src/main/java/graphql/servlet/GraphQLHttpServlet.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,6 @@ protected GraphQLObjectMapper getGraphQLObjectMapper() {
3333
throw new UnsupportedOperationException();
3434
}
3535

36-
@Override
37-
protected GraphQLBatchExecutionHandlerFactory getBatchExecutionHandlerFactory() {
38-
throw new UnsupportedOperationException();
39-
}
40-
4136
@Override
4237
protected boolean isAsyncServletMode() {
4338
throw new UnsupportedOperationException();

‎src/main/java/graphql/servlet/GraphQLQueryInvoker.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import graphql.schema.GraphQLSchema;
1414

1515
import javax.security.auth.Subject;
16+
import java.io.Writer;
1617
import java.security.AccessController;
1718
import java.security.PrivilegedAction;
1819
import java.util.ArrayList;
@@ -28,20 +29,22 @@ public class GraphQLQueryInvoker {
2829
private final Supplier<Instrumentation> getInstrumentation;
2930
private final Supplier<PreparsedDocumentProvider> getPreparsedDocumentProvider;
3031
private final Supplier<DataLoaderDispatcherInstrumentationOptions> dataLoaderDispatcherInstrumentationOptionsSupplier;
32+
private final BatchExecutionHandler batchExecutionHandler;
3133

32-
protected GraphQLQueryInvoker(Supplier<ExecutionStrategyProvider> getExecutionStrategyProvider, Supplier<Instrumentation> getInstrumentation, Supplier<PreparsedDocumentProvider> getPreparsedDocumentProvider, Supplier<DataLoaderDispatcherInstrumentationOptions> optionsSupplier) {
34+
protected GraphQLQueryInvoker(Supplier<ExecutionStrategyProvider> getExecutionStrategyProvider, Supplier<Instrumentation> getInstrumentation, Supplier<PreparsedDocumentProvider> getPreparsedDocumentProvider, Supplier<DataLoaderDispatcherInstrumentationOptions> optionsSupplier, BatchExecutionHandler batchExecutionHandler) {
3335
this.getExecutionStrategyProvider = getExecutionStrategyProvider;
3436
this.getInstrumentation = getInstrumentation;
3537
this.getPreparsedDocumentProvider = getPreparsedDocumentProvider;
3638
this.dataLoaderDispatcherInstrumentationOptionsSupplier = optionsSupplier;
39+
this.batchExecutionHandler = batchExecutionHandler;
3740
}
3841

3942
public ExecutionResult query(GraphQLSingleInvocationInput singleInvocationInput) {
4043
return query(singleInvocationInput, singleInvocationInput.getExecutionInput());
4144
}
4245

43-
public void query(GraphQLBatchedInvocationInput batchedInvocationInput, BatchExecutionHandler batchExecutionHandler) {
44-
batchExecutionHandler.handleBatch(batchedInvocationInput, this::query);
46+
public void query(GraphQLBatchedInvocationInput batchedInvocationInput, Writer writer, GraphQLObjectMapper graphQLObjectMapper) {
47+
batchExecutionHandler.handleBatch(batchedInvocationInput, writer, graphQLObjectMapper, this::query);
4548
}
4649

4750
private GraphQL newGraphQL(GraphQLSchema schema, Object context) {
@@ -97,6 +100,7 @@ public static class Builder {
97100
private Supplier<Instrumentation> getInstrumentation = () -> SimpleInstrumentation.INSTANCE;
98101
private Supplier<PreparsedDocumentProvider> getPreparsedDocumentProvider = () -> NoOpPreparsedDocumentProvider.INSTANCE;
99102
private Supplier<DataLoaderDispatcherInstrumentationOptions> dataLoaderDispatcherInstrumentationOptionsSupplier = DataLoaderDispatcherInstrumentationOptions::newOptions;
103+
private BatchExecutionHandler batchExecutionHandler = new DeafultBatchExecutionHandler();
100104

101105
public Builder withExecutionStrategyProvider(ExecutionStrategyProvider provider) {
102106
return withExecutionStrategyProvider(() -> provider);
@@ -128,6 +132,13 @@ public Builder with(List<Instrumentation> instrumentations) {
128132
return this;
129133
}
130134

135+
public Builder withBatchExeuctionHandler(BatchExecutionHandler batchExeuctionHandler) {
136+
if (batchExeuctionHandler != null) {
137+
this.batchExecutionHandler = batchExeuctionHandler;
138+
}
139+
return this;
140+
}
141+
131142
public Builder withPreparsedDocumentProvider(PreparsedDocumentProvider provider) {
132143
return withPreparsedDocumentProvider(() -> provider);
133144
}
@@ -147,7 +158,7 @@ public Builder withDataLoaderDispatcherInstrumentationOptions(Supplier<DataLoade
147158
}
148159

149160
public GraphQLQueryInvoker build() {
150-
return new GraphQLQueryInvoker(getExecutionStrategyProvider, getInstrumentation, getPreparsedDocumentProvider, dataLoaderDispatcherInstrumentationOptionsSupplier);
161+
return new GraphQLQueryInvoker(getExecutionStrategyProvider, getInstrumentation, getPreparsedDocumentProvider, dataLoaderDispatcherInstrumentationOptionsSupplier, batchExecutionHandler);
151162
}
152163
}
153164
}

‎src/main/java/graphql/servlet/OsgiGraphQLHttpServlet.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ public class OsgiGraphQLHttpServlet extends AbstractGraphQLHttpServlet {
4646
private InstrumentationProvider instrumentationProvider = new NoOpInstrumentationProvider();
4747
private GraphQLErrorHandler errorHandler = new DefaultGraphQLErrorHandler();
4848
private PreparsedDocumentProvider preparsedDocumentProvider = NoOpPreparsedDocumentProvider.INSTANCE;
49-
private GraphQLBatchExecutionHandlerFactory executionResultHandlerFactory = new DefaultGraphQLBatchExecutionHandlerFactory();
5049

5150
private GraphQLSchemaProvider schemaProvider;
5251

@@ -85,11 +84,6 @@ protected GraphQLObjectMapper getGraphQLObjectMapper() {
8584
return graphQLObjectMapper;
8685
}
8786

88-
@Override
89-
protected GraphQLBatchExecutionHandlerFactory getBatchExecutionHandlerFactory() {
90-
return executionResultHandlerFactory;
91-
}
92-
9387
@Override
9488
protected boolean isAsyncServletMode() {
9589
return false;

0 commit comments

Comments
 (0)