Skip to content

Commit c0a9126

Browse files
committed
fix(#469): allow graphql-java future to be canceled
1 parent 4399530 commit c0a9126

File tree

7 files changed

+155
-21
lines changed

7 files changed

+155
-21
lines changed
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package graphql.kickstart.execution;
2+
3+
import graphql.ExecutionResult;
4+
import graphql.kickstart.execution.input.GraphQLInvocationInput;
5+
import java.util.List;
6+
import java.util.concurrent.CompletableFuture;
7+
import lombok.Getter;
8+
import lombok.RequiredArgsConstructor;
9+
10+
@RequiredArgsConstructor
11+
class FutureBatchedExecutionResult implements FutureExecutionResult {
12+
13+
@Getter
14+
private final GraphQLInvocationInput invocationInput;
15+
private final CompletableFuture<List<ExecutionResult>> batched;
16+
17+
@Override
18+
public CompletableFuture<GraphQLQueryResult> thenApplyQueryResult() {
19+
return batched.thenApply(GraphQLQueryResult::create);
20+
}
21+
22+
@Override
23+
public void cancel() {
24+
batched.cancel(true);
25+
}
26+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package graphql.kickstart.execution;
2+
3+
import graphql.kickstart.execution.input.GraphQLInvocationInput;
4+
import java.util.concurrent.CompletableFuture;
5+
import lombok.RequiredArgsConstructor;
6+
7+
@RequiredArgsConstructor
8+
class FutureErrorExecutionResult implements FutureExecutionResult {
9+
10+
private final GraphQLErrorQueryResult errorQueryResult;
11+
12+
@Override
13+
public CompletableFuture<GraphQLQueryResult> thenApplyQueryResult() {
14+
return CompletableFuture.completedFuture(errorQueryResult);
15+
}
16+
17+
@Override
18+
public GraphQLInvocationInput getInvocationInput() {
19+
return null;
20+
}
21+
22+
@Override
23+
public void cancel() {
24+
// nothing to do
25+
}
26+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package graphql.kickstart.execution;
2+
3+
import graphql.ExecutionResult;
4+
import graphql.kickstart.execution.input.GraphQLInvocationInput;
5+
import java.util.List;
6+
import java.util.concurrent.CompletableFuture;
7+
8+
public interface FutureExecutionResult {
9+
10+
static FutureExecutionResult single(
11+
GraphQLInvocationInput invocationInput, CompletableFuture<ExecutionResult> single) {
12+
return new FutureSingleExecutionResult(invocationInput, single);
13+
}
14+
15+
static FutureExecutionResult batched(
16+
GraphQLInvocationInput invocationInput, CompletableFuture<List<ExecutionResult>> batched) {
17+
return new FutureBatchedExecutionResult(invocationInput, batched);
18+
}
19+
20+
static FutureExecutionResult error(GraphQLErrorQueryResult result) {
21+
return new FutureErrorExecutionResult(result);
22+
}
23+
24+
CompletableFuture<GraphQLQueryResult> thenApplyQueryResult();
25+
26+
GraphQLInvocationInput getInvocationInput();
27+
28+
void cancel();
29+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package graphql.kickstart.execution;
2+
3+
import graphql.ExecutionResult;
4+
import graphql.kickstart.execution.input.GraphQLInvocationInput;
5+
import java.util.concurrent.CompletableFuture;
6+
import lombok.Getter;
7+
import lombok.RequiredArgsConstructor;
8+
9+
@RequiredArgsConstructor
10+
class FutureSingleExecutionResult implements FutureExecutionResult {
11+
12+
@Getter
13+
private final GraphQLInvocationInput invocationInput;
14+
private final CompletableFuture<ExecutionResult> single;
15+
16+
@Override
17+
public CompletableFuture<GraphQLQueryResult> thenApplyQueryResult() {
18+
return single.thenApply(GraphQLQueryResult::create);
19+
}
20+
21+
@Override
22+
public void cancel() {
23+
single.cancel(true);
24+
}
25+
}

graphql-java-kickstart/src/main/java/graphql/kickstart/execution/GraphQLInvoker.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ public class GraphQLInvoker {
2222
private final BatchedDataLoaderGraphQLBuilder batchedDataLoaderGraphQLBuilder;
2323
private final GraphQLInvokerProxy proxy = GraphQL::executeAsync;
2424

25+
public FutureExecutionResult execute(GraphQLInvocationInput invocationInput) {
26+
if (invocationInput instanceof GraphQLSingleInvocationInput) {
27+
return FutureExecutionResult.single(
28+
invocationInput, executeAsync((GraphQLSingleInvocationInput) invocationInput));
29+
}
30+
return FutureExecutionResult.batched(
31+
invocationInput, executeAsync((GraphQLBatchedInvocationInput) invocationInput));
32+
}
33+
2534
public CompletableFuture<ExecutionResult> executeAsync(
2635
GraphQLSingleInvocationInput invocationInput) {
2736
GraphQL graphQL = graphQLBuilder.build(invocationInput.getSchema());

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

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import graphql.ExecutionResult;
44
import graphql.ExecutionResultImpl;
5+
import graphql.kickstart.execution.FutureExecutionResult;
56
import graphql.kickstart.execution.GraphQLInvoker;
67
import graphql.kickstart.execution.GraphQLQueryResult;
78
import graphql.kickstart.execution.error.GenericGraphQLError;
@@ -13,6 +14,7 @@
1314
import java.io.IOException;
1415
import java.io.UncheckedIOException;
1516
import java.util.Optional;
17+
import java.util.concurrent.CancellationException;
1618
import java.util.concurrent.CompletableFuture;
1719
import java.util.concurrent.CompletionException;
1820
import java.util.concurrent.atomic.AtomicReference;
@@ -40,7 +42,7 @@ public void execute(
4042
if (request.isAsyncSupported()) {
4143
invokeAndHandleAsync(invocationInput, request, response, listenerHandler);
4244
} else {
43-
invokeAndHandle(invocationInput, request, response, listenerHandler).join();
45+
handle(invoke(invocationInput, request, response), request, response, listenerHandler).join();
4446
}
4547
}
4648

@@ -54,10 +56,10 @@ private void invokeAndHandleAsync(
5456
? request.getAsyncContext()
5557
: request.startAsync(request, response);
5658
asyncContext.setTimeout(configuration.getAsyncTimeout());
57-
AtomicReference<CompletableFuture<Void>> futureHolder = new AtomicReference<>();
59+
AtomicReference<FutureExecutionResult> futureHolder = new AtomicReference<>();
5860
AsyncTimeoutListener timeoutListener =
5961
event -> {
60-
Optional.ofNullable(futureHolder.get()).ifPresent(it -> it.cancel(true));
62+
Optional.ofNullable(futureHolder.get()).ifPresent(FutureExecutionResult::cancel);
6163
writeResultResponse(
6264
invocationInput,
6365
GraphQLQueryResult.create(
@@ -68,22 +70,27 @@ private void invokeAndHandleAsync(
6870
};
6971
asyncContext.addListener(timeoutListener);
7072
asyncContext.start(
71-
() ->
72-
futureHolder.set(
73-
invokeAndHandle(invocationInput, request, response, listenerHandler)
74-
.thenAccept(aVoid -> asyncContext.complete())));
73+
() -> {
74+
FutureExecutionResult futureResult = invoke(invocationInput, request, response);
75+
futureHolder.set(futureResult);
76+
handle(futureResult, request, response, listenerHandler);
77+
});
7578
}
7679

77-
private CompletableFuture<Void> invokeAndHandle(
78-
GraphQLInvocationInput invocationInput,
80+
private CompletableFuture<Void> handle(
81+
FutureExecutionResult futureResult,
7982
HttpServletRequest request,
8083
HttpServletResponse response,
8184
ListenerHandler listenerHandler) {
82-
return invoke(invocationInput, request, response)
83-
.thenAccept(it -> writeResultResponse(invocationInput, it, request, response))
85+
return futureResult
86+
.thenApplyQueryResult()
87+
.thenAccept(
88+
it -> writeResultResponse(futureResult.getInvocationInput(), it, request, response))
8489
.thenAccept(it -> listenerHandler.onSuccess())
8590
.exceptionally(
86-
t -> writeErrorResponse(invocationInput, request, response, listenerHandler, t))
91+
t ->
92+
writeErrorResponse(
93+
futureResult.getInvocationInput(), request, response, listenerHandler, t))
8794
.thenAccept(it -> listenerHandler.onFinally());
8895
}
8996

@@ -106,22 +113,32 @@ private Void writeErrorResponse(
106113
HttpServletResponse response,
107114
ListenerHandler listenerHandler,
108115
Throwable t) {
116+
Throwable cause = getCause(t);
109117
if (!response.isCommitted()) {
110118
writeResultResponse(
111-
invocationInput, GraphQLQueryResult.create(toErrorResult(t)), request, response);
112-
listenerHandler.onError(t);
119+
invocationInput, GraphQLQueryResult.create(toErrorResult(cause)), request, response);
120+
listenerHandler.onError(cause);
113121
} else {
114122
log.warn(
115123
"Cannot write GraphQL response, because the HTTP response is already committed. It most likely timed out.");
116124
}
117125
return null;
118126
}
119127

128+
private Throwable getCause(Throwable t) {
129+
return t instanceof CompletionException && t.getCause() != null ? t.getCause() : t;
130+
}
131+
120132
private ExecutionResult toErrorResult(Throwable t) {
121133
String message =
122-
t instanceof CompletionException && t.getCause() != null
123-
? t.getCause().getMessage()
134+
t instanceof CancellationException
135+
? "Execution canceled because timeout of "
136+
+ configuration.getAsyncTimeout()
137+
+ " millis was reached"
124138
: t.getMessage();
139+
if (message == null) {
140+
message = "Unexpected error occurred";
141+
}
125142
return new ExecutionResultImpl(new GenericGraphQLError(message));
126143
}
127144

@@ -130,28 +147,28 @@ protected QueryResponseWriter createWriter(
130147
return queryResponseWriterFactory.createWriter(invocationInput, queryResult, configuration);
131148
}
132149

133-
private CompletableFuture<GraphQLQueryResult> invoke(
150+
private FutureExecutionResult invoke(
134151
GraphQLInvocationInput invocationInput,
135152
HttpServletRequest request,
136153
HttpServletResponse response) {
137154
if (invocationInput instanceof GraphQLSingleInvocationInput) {
138-
return graphQLInvoker.queryAsync(invocationInput);
155+
return graphQLInvoker.execute(invocationInput);
139156
}
140157
return invokeBatched((GraphQLBatchedInvocationInput) invocationInput, request, response);
141158
}
142159

143-
private CompletableFuture<GraphQLQueryResult> invokeBatched(
160+
private FutureExecutionResult invokeBatched(
144161
GraphQLBatchedInvocationInput batchedInvocationInput,
145162
HttpServletRequest request,
146163
HttpServletResponse response) {
147164
BatchInputPreProcessor preprocessor = configuration.getBatchInputPreProcessor();
148165
BatchInputPreProcessResult result =
149166
preprocessor.preProcessBatch(batchedInvocationInput, request, response);
150167
if (result.isExecutable()) {
151-
return graphQLInvoker.queryAsync(result.getBatchedInvocationInput());
168+
return graphQLInvoker.execute(result.getBatchedInvocationInput());
152169
}
153170

154-
return CompletableFuture.completedFuture(
171+
return FutureExecutionResult.error(
155172
GraphQLQueryResult.createError(result.getStatusCode(), result.getStatusMessage()));
156173
}
157174
}

graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CachingHttpRequestInvokerTest.groovy

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package graphql.kickstart.servlet.cache
22

33
import graphql.ExecutionResult
4+
import graphql.kickstart.execution.FutureExecutionResult
45
import graphql.kickstart.execution.GraphQLInvoker
56
import graphql.kickstart.execution.GraphQLObjectMapper
67
import graphql.kickstart.execution.GraphQLQueryResult
@@ -39,6 +40,7 @@ class CachingHttpRequestInvokerTest extends Specification {
3940
graphqlInvoker = Mock(GraphQLInvoker)
4041
graphqlObjectMapper = Mock(GraphQLObjectMapper)
4142
outputStreamMock = Mock(ServletOutputStream)
43+
graphqlInvoker.execute(invocationInputMock) >> FutureExecutionResult.single(invocationInputMock, CompletableFuture.completedFuture(Mock(GraphQLQueryResult)))
4244
cachingInvoker = new CachingHttpRequestInvoker(configuration, httpRequestInvokerMock, cacheReaderMock)
4345

4446
configuration.getResponseCacheManager() >> responseCacheManagerMock

0 commit comments

Comments
 (0)