Added ExecutionInput cancellation by bbakerman · Pull Request #3880 · graphql-java/graphql-java

bbakerman · 2025-04-01T10:24:07Z

Addressing #3879

This allows the graphql operation to be co-operatively cancelled. It use the "engine run state" tracking to know when its a good time to cancel and abort

github-actions · 2025-04-01T10:26:40Z

Test Results

313 files ±0 313 suites ±0 48s ⏱️ -1s
3 590 tests +2 3 584 ✅ +2 6 💤 ±0 0 ❌ ±0
3 679 runs +2 3 673 ✅ +2 6 💤 ±0 0 ❌ ±0

Results for commit bd4a098. ± Comparison against base commit 5a03541.

This pull request removes 150 and adds 130 tests. Note that renamed tests count towards both.

	?
                __schema { types { fields { args { type { name fields { name }}}}}}
                __schema { types { fields { type { name fields { name }}}}}
                __schema { types { inputFields { type { inputFields { name }}}}}
                __schema { types { interfaces { fields { type { interfaces { name } } } } } }
                __schema { types { name} }
                __type(name : "t") { name }
                a1: __schema { types { name} }
                a1: __type(name : "t") { name }
                a2 :  __type(name : "t1") { name }
…

graphql.ExecutionInputTest ‑ can cancel the execution
graphql.ScalarsBooleanTest ‑ parseValue throws exception for invalid input <java.lang.Object@759cd0b8>
graphql.ScalarsBooleanTest ‑ serialize throws exception for invalid input <java.lang.Object@6bfe5fd3>
graphql.ScalarsIDTest ‑ parseValue allows any object via String.valueOf <java.lang.Object@192de918>
graphql.ScalarsIDTest ‑ serialize allows any object via String.valueOf <java.lang.Object@243b43ed>
graphql.ScalarsIntTest ‑ parseValue throws exception for invalid input <java.lang.Object@615a7c2b>
graphql.ScalarsIntTest ‑ serialize throws exception for invalid input <java.lang.Object@7371b42b>
graphql.execution.SubscriptionExecutionStrategyTest ‑ we can cancel the operation and the upstream publisher is told
graphql.execution.instrumentation.TracingInstrumentationTest ‑ do not trace introspection information [testExecutionStrategy: <graphql.execution.AsyncExecutionStrategy@7c97eb79 fieldCollector=graphql.execution.FieldCollector@55b58dba executionStepInfoFactory=graphql.execution.ExecutionStepInfoFactory@20de1fed dataFetcherExceptionHandler=graphql.execution.SimpleDataFetcherExceptionHandler@3bd2182f resolvedType=graphql.execution.ResolveType@78ddf00f>, #0]
graphql.execution.instrumentation.TracingInstrumentationTest ‑ do not trace introspection information [testExecutionStrategy: <graphql.execution.AsyncSerialExecutionStrategy@510d3c56 fieldCollector=graphql.execution.FieldCollector@7ab55538 executionStepInfoFactory=graphql.execution.ExecutionStepInfoFactory@6dff8fe9 dataFetcherExceptionHandler=graphql.execution.SimpleDataFetcherExceptionHandler@fb6db57 resolvedType=graphql.execution.ResolveType@37e1a97b>, #1]
…

♻️ This comment has been updated with latest results.

andimarek · 2025-04-01T11:14:12Z

This should be combined with #3876

…uest

…ncellation-of-request # Conflicts: # src/main/java/graphql/execution/AsyncExecutionStrategy.java # src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java # src/main/java/graphql/execution/ExecutionStrategy.java # src/main/java/graphql/execution/SubscriptionExecutionStrategy.java

…uest # Conflicts: # src/main/java/graphql/execution/ExecutionContext.java # src/main/java/graphql/execution/ExecutionStrategy.java

bbakerman · 2025-04-02T23:02:07Z

src/main/java/graphql/ExecutionInput.java


        // hidden on purpose
-        private Builder transfer(GraphQLContext graphQLContext) {
+        private Builder internalTransferContext(GraphQLContext graphQLContext) {


I can rename because its private method. This is a transferring of the context to a new instance

bbakerman · 2025-04-02T23:02:34Z

src/main/java/graphql/GraphQL.java

    private PreparsedDocumentEntry parseAndValidate(AtomicReference<ExecutionInput> executionInputRef, GraphQLSchema graphQLSchema, InstrumentationState instrumentationState) {

        ExecutionInput executionInput = executionInputRef.get();
-        String query = executionInput.getQuery();


never used. Grey code. took the opportunity to remove it

bbakerman · 2025-04-02T23:03:55Z

src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java


    protected BiConsumer<List<Object>, Throwable> handleResults(ExecutionContext executionContext, List<String> fieldNames, CompletableFuture<ExecutionResult> overallResult) {
-        return (List<Object> results, Throwable exception) -> executionContext.run(() -> {
+        return (List<Object> results, Throwable exception) -> executionContext.run(exception, () -> {


whenever we call run here we need to respect if an exception was thrown on the inside when we check if we are cancelled.

There is not point throwing a "cancelled" exception if we are already in exception

bbakerman · 2025-04-02T23:04:22Z

src/main/java/graphql/execution/EngineRunningObserver.java

+     * @param executionId    the id of the current execution
+     * @param graphQLContext the graphql context
+     */
+    void runningStateChanged(ExecutionId executionId, GraphQLContext graphQLContext, RunningState runningState);


made i an enum versus a boolean

bbakerman · 2025-04-02T23:04:45Z

src/main/java/graphql/execution/ExecutionContext.java


-    public void incrementRunning() {
-        if (isRunning.incrementAndGet() == 1 && engineRunningObserver != null) {
-            engineRunningObserver.runningStateChanged(executionId, graphQLContext, true);


never needed to be public. Never used publically

bbakerman · 2025-04-02T23:05:12Z

src/main/java/graphql/execution/ExecutionContext.java

+        checkIsCancelled(throwable);
+
+        if (isRunning.incrementAndGet() == 1) {
+            changeOfState(RUNNING);


changeOfState encapsulates the null check

bbakerman · 2025-04-02T23:05:39Z

src/main/java/graphql/execution/ExecutionContext.java

-        if (isRunning.incrementAndGet() == 1 && engineRunningObserver != null) {
-            engineRunningObserver.runningStateChanged(executionId, graphQLContext, true);
+    private void incrementRunning(Throwable throwable) {
+        checkIsCancelled(throwable);


we now check if we are cancelled in a common place

bbakerman · 2025-04-02T23:07:01Z

src/main/java/graphql/execution/SubscriptionExecutionStrategy.java

-                    return executionResult;
-                });
-            });
+                    }));


looks like formatting

andimarek · 2025-04-02T23:14:56Z

src/main/java/graphql/execution/EngineRunningObserver.java

 @NullMarked
 public interface EngineRunningObserver {

+    enum RunningState {


Can we add started and finished here to make it complete?

Maybe not .., we have ways to detect that

… asked for

Added ExecutionInput cancellation

5f3e632

bbakerman added this to the 23.0 breaking changes milestone Apr 1, 2025

bbakerman mentioned this pull request Apr 1, 2025

Co-operative way to cancel request execution #3879

Closed

bbakerman added 7 commits April 2, 2025 12:36

Added ExecutionInput cancellation - added tests and tweak the code more

9c12ab3

Added ExecutionInput cancellation - fixed tests with null EI

9a1fe99

Merge remote-tracking branch 'origin/master' into cancellation-of-req…

d68efe9

…uest

Fixed dual cancelled checks

61ae7b0

Merge remote-tracking branch 'origin/master' into cancellation-of-req…

136301e

…uest # Conflicts: # src/main/java/graphql/execution/ExecutionContext.java # src/main/java/graphql/execution/ExecutionStrategy.java

Merged in master to get cancellation PR working again with new states

fd302f0

bbakerman commented Apr 2, 2025

View reviewed changes

andimarek requested changes Apr 2, 2025

View reviewed changes

andimarek approved these changes Apr 2, 2025

View reviewed changes

bbakerman added 2 commits April 3, 2025 12:24

Added test for subscriptions

033ba93

Added test for subscriptions - checking publisher and how many is got…

bd4a098

… asked for

bbakerman merged commit aa5afea into master Apr 3, 2025
2 checks passed

Comments

Conversation

bbakerman commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

andimarek commented Apr 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bbakerman commented Apr 1, 2025 •

edited

Loading

github-actions bot commented Apr 1, 2025 •

edited

Loading