From 890ddb385f5ba57a4fc23700369b74c5461fbd3c Mon Sep 17 00:00:00 2001 From: bbaker Date: Tue, 12 Aug 2025 13:29:13 +1000 Subject: [PATCH 1/3] Fixed deferred support to have proper Instrumentation --- .../graphql/execution/ExecutionStrategy.java | 8 +- .../incremental/DeferredExecutionSupport.java | 62 +++++++++----- .../ChainedInstrumentation.java | 5 +- .../instrumentation/Instrumentation.java | 7 +- src/test/groovy/graphql/TestUtil.groovy | 21 +++++ .../InstrumentationTest.groovy | 81 +++++++++++++++++++ .../ModernTestingInstrumentation.groovy | 6 ++ 7 files changed, 162 insertions(+), 28 deletions(-) diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 255dff4ce..c1272df44 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -292,7 +292,8 @@ DeferredExecutionSupport createDeferredExecutionSupport(ExecutionContext executi fields, parameters, executionContext, - (ec, esp) -> Async.toCompletableFuture(resolveFieldWithInfo(ec, esp)) + (ec, esp) -> Async.toCompletableFuture(resolveFieldWithInfo(ec, esp)), + this::createExecutionStepInfo ) : DeferredExecutionSupport.NOOP; } @@ -1096,6 +1097,11 @@ protected ExecutionStepInfo createExecutionStepInfo(ExecutionContext executionCo fieldContainer); } + private Supplier createExecutionStepInfo(ExecutionContext executionContext, ExecutionStrategyParameters parameters) { + GraphQLFieldDefinition fieldDef = getFieldDef(executionContext, parameters, parameters.getField().getSingleField()); + return FpKit.intraThreadMemoize(() -> createExecutionStepInfo(executionContext, parameters, fieldDef, null)); + } + // Errors that result from the execution of deferred fields are kept in the deferred context only. private static void addErrorToRightContext(GraphQLError error, ExecutionStrategyParameters parameters, ExecutionContext executionContext) { if (parameters.getDeferredCallContext() != null) { diff --git a/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java b/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java index 9a46020d8..b4b5ee39a 100644 --- a/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java +++ b/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java @@ -7,14 +7,18 @@ import graphql.ExecutionResultImpl; import graphql.Internal; import graphql.execution.ExecutionContext; +import graphql.execution.ExecutionStepInfo; import graphql.execution.ExecutionStrategyParameters; import graphql.execution.FieldValueInfo; import graphql.execution.MergedField; import graphql.execution.MergedSelectionSet; import graphql.execution.ResultPath; import graphql.execution.instrumentation.Instrumentation; +import graphql.execution.instrumentation.InstrumentationContext; +import graphql.execution.instrumentation.parameters.InstrumentationFieldParameters; import graphql.incremental.IncrementalPayload; import graphql.util.FpKit; +import org.jetbrains.annotations.NotNull; import java.util.Collections; import java.util.HashMap; @@ -27,6 +31,8 @@ import java.util.function.BiFunction; import java.util.function.Supplier; +import static graphql.execution.instrumentation.SimpleInstrumentationContext.nonNullCtx; + /** * The purpose of this class hierarchy is to encapsulate most of the logic for deferring field execution, thus * keeping the main execution strategy code clean and focused on the main execution logic. @@ -59,16 +65,19 @@ class DeferredExecutionSupportImpl implements DeferredExecutionSupport { private final ExecutionStrategyParameters parameters; private final ExecutionContext executionContext; private final BiFunction> resolveFieldWithInfoFn; + private final BiFunction> executionStepInfoFn; private final Map>> dfCache = new HashMap<>(); public DeferredExecutionSupportImpl( MergedSelectionSet mergedSelectionSet, ExecutionStrategyParameters parameters, ExecutionContext executionContext, - BiFunction> resolveFieldWithInfoFn + BiFunction> resolveFieldWithInfoFn, + BiFunction> executionStepInfoFn ) { this.executionContext = executionContext; this.resolveFieldWithInfoFn = resolveFieldWithInfoFn; + this.executionStepInfoFn = executionStepInfoFn; ImmutableListMultimap.Builder deferredExecutionToFieldsBuilder = ImmutableListMultimap.builder(); ImmutableSet.Builder deferredFieldsBuilder = ImmutableSet.builder(); ImmutableList.Builder nonDeferredFieldNamesBuilder = ImmutableList.builder(); @@ -154,36 +163,47 @@ private Supplier FpKit.interThreadMemoize(() -> { - CompletableFuture fieldValueResult = resolveFieldWithInfoFn.apply(executionContext, executionStrategyParameters); + key -> FpKit.interThreadMemoize(resolveDeferredFieldValue(currentField, executionContext, executionStrategyParameters) + ) + ); + } - fieldValueResult.whenComplete((fieldValueInfo, throwable) -> { - executionContext.getDataLoaderDispatcherStrategy().deferredOnFieldValue(currentField.getResultKey(), fieldValueInfo, throwable, executionStrategyParameters); - }); + @NotNull + private Supplier> resolveDeferredFieldValue(MergedField currentField, ExecutionContext executionContext, ExecutionStrategyParameters executionStrategyParameters) { + return () -> { + Instrumentation instrumentation = executionContext.getInstrumentation(); + Supplier executionStepInfo = executionStepInfoFn.apply(executionContext, executionStrategyParameters); + InstrumentationFieldParameters fieldParameters = new InstrumentationFieldParameters(executionContext, executionStepInfo); + InstrumentationContext deferredFieldCtx = nonNullCtx(instrumentation.beginDeferredField(fieldParameters, executionContext.getInstrumentationState())); - CompletableFuture executionResultCF = fieldValueResult - .thenCompose(fvi -> fvi - .getFieldValueFuture() - .thenApply(fv -> ExecutionResultImpl.newExecutionResult().data(fv).build()) - ); + CompletableFuture fieldValueResult = resolveFieldWithInfoFn.apply(this.executionContext, executionStrategyParameters); - return executionResultCF - .thenApply(executionResult -> - new DeferredFragmentCall.FieldWithExecutionResult(currentField.getResultKey(), executionResult) - ); - } - ) - ); + deferredFieldCtx.onDispatched(); + + fieldValueResult.whenComplete((fieldValueInfo, throwable) -> { + this.executionContext.getDataLoaderDispatcherStrategy().deferredOnFieldValue(currentField.getResultKey(), fieldValueInfo, throwable, executionStrategyParameters); + deferredFieldCtx.onCompleted(fieldValueInfo, throwable); + }); + + + CompletableFuture executionResultCF = fieldValueResult + .thenCompose(fvi -> fvi + .getFieldValueFuture() + .thenApply(fv -> ExecutionResultImpl.newExecutionResult().data(fv).build()) + ); + + return executionResultCF + .thenApply(executionResult -> + new DeferredFragmentCall.FieldWithExecutionResult(currentField.getResultKey(), executionResult) + ); + }; } } diff --git a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java index daca12293..e48e20fa6 100644 --- a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java @@ -166,11 +166,10 @@ public ExecutionStrategyInstrumentationContext beginExecutionStrategy(Instrument @ExperimentalApi @Override - public InstrumentationContext beginDeferredField(InstrumentationState instrumentationState) { - return new ChainedDeferredExecutionStrategyInstrumentationContext(chainedMapAndDropNulls(instrumentationState, Instrumentation::beginDeferredField)); + public InstrumentationContext beginDeferredField(InstrumentationFieldParameters parameters, InstrumentationState state) { + return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginDeferredField(parameters, specificState)); } - @Override public InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { return chainedCtx(state, (instrumentation, specificState) -> instrumentation.beginSubscribedFieldEvent(parameters, specificState)); diff --git a/src/main/java/graphql/execution/instrumentation/Instrumentation.java b/src/main/java/graphql/execution/instrumentation/Instrumentation.java index 714f96fe3..1ca6c268e 100644 --- a/src/main/java/graphql/execution/instrumentation/Instrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/Instrumentation.java @@ -153,12 +153,13 @@ default ExecuteObjectInstrumentationContext beginExecuteObject(InstrumentationEx *

* This is an EXPERIMENTAL instrumentation callback. The method signature will definitely change. * - * @param state the state created during the call to {@link #createStateAsync(InstrumentationCreateStateParameters)} + * @param parameters the parameters to this step + * @param state the state created during the call to {@link #createStateAsync(InstrumentationCreateStateParameters)} * - * @return a nullable {@link ExecutionStrategyInstrumentationContext} object that will be called back when the step ends (assuming it's not null) + * @return a nullable {@link InstrumentationContext} object that will be called back when the step ends (assuming it's not null) */ @ExperimentalApi - default InstrumentationContext beginDeferredField(InstrumentationState state) { + default InstrumentationContext beginDeferredField(InstrumentationFieldParameters parameters, InstrumentationState state) { return noOp(); } diff --git a/src/test/groovy/graphql/TestUtil.groovy b/src/test/groovy/graphql/TestUtil.groovy index dea2c2c7c..48a1ad11a 100644 --- a/src/test/groovy/graphql/TestUtil.groovy +++ b/src/test/groovy/graphql/TestUtil.groovy @@ -2,6 +2,9 @@ package graphql import graphql.execution.MergedField import graphql.execution.MergedSelectionSet +import graphql.execution.pubsub.CapturingSubscriber +import graphql.incremental.DelayedIncrementalPartialResult +import graphql.incremental.IncrementalExecutionResult import graphql.introspection.Introspection.DirectiveLocation import graphql.language.Document import graphql.language.Field @@ -31,6 +34,8 @@ import graphql.schema.idl.TypeRuntimeWiring import graphql.schema.idl.WiringFactory import graphql.schema.idl.errors.SchemaProblem import groovy.json.JsonOutput +import org.awaitility.Awaitility +import org.reactivestreams.Publisher import java.util.function.Supplier import java.util.stream.Collectors @@ -323,4 +328,20 @@ class TestUtil { return rn.nextInt(max - min + 1) + min } + + static List> getIncrementalResults(IncrementalExecutionResult initialResult) { + Publisher deferredResultStream = initialResult.incrementalItemPublisher + + def subscriber = new CapturingSubscriber() + + deferredResultStream.subscribe(subscriber) + + Awaitility.await().untilTrue(subscriber.isDone()) + if (subscriber.throwable != null) { + throw new RuntimeException(subscriber.throwable) + } + return subscriber.getEvents() + .collect { it.toSpecification() } + } + } diff --git a/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy b/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy index 6580d5990..ffe8f5a62 100644 --- a/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy @@ -4,11 +4,13 @@ import graphql.ExecutionInput import graphql.ExecutionResult import graphql.GraphQL import graphql.StarWarsSchema +import graphql.TestUtil import graphql.execution.AsyncExecutionStrategy import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters +import graphql.incremental.IncrementalExecutionResult import graphql.language.AstPrinter import graphql.parser.Parser import graphql.schema.DataFetcher @@ -496,4 +498,83 @@ class InstrumentationTest extends Specification { then: er.extensions == [i1: "I1"] } + + def "can instrumented deferred fields"() { + + given: + + def query = """ + { + hero { + ... @defer(label: "id") { + id + } + ... @defer(label: "name") { + name + } + } + } + """ + + + when: + + def instrumentation = new ModernTestingInstrumentation() + + def graphQL = GraphQL + .newGraphQL(StarWarsSchema.starWarsSchema) + .queryExecutionStrategy(new AsyncExecutionStrategy()) + .instrumentation(instrumentation) + .build() + + def ei = ExecutionInput.newExecutionInput(query).graphQLContext { it -> + GraphQL.unusualConfiguration(it).incrementalSupport().enableIncrementalSupport(true) + }.build() + + IncrementalExecutionResult incrementalER = graphQL.execute(ei) as IncrementalExecutionResult + // + // cause the defer Publish to be finished + def results = TestUtil.getIncrementalResults(incrementalER) + + + then: + + instrumentation.executionList == ["start:execution", + "start:parse", + "end:parse", + "start:validation", + "end:validation", + "start:execute-operation", + "start:execution-strategy", + "start:field-hero", + "start:fetch-hero", + "end:fetch-hero", + "start:complete-hero", + "start:execute-object", + "end:execute-object", + "end:complete-hero", + "end:field-hero", + "end:execution-strategy", + "end:execute-operation", + "end:execution", + // + // the deferred field resolving now happens after the operation has come back + "start:deferred-field-id", + "start:field-id", + "start:fetch-id", + "end:fetch-id", + "start:complete-id", + "end:complete-id", + "end:field-id", + "end:deferred-field-id", + "start:deferred-field-name", + "start:field-name", + "start:fetch-name", + "end:fetch-name", + "start:complete-name", + "end:complete-name", + "end:field-name", + "end:deferred-field-name", + ] + } } diff --git a/src/test/groovy/graphql/execution/instrumentation/ModernTestingInstrumentation.groovy b/src/test/groovy/graphql/execution/instrumentation/ModernTestingInstrumentation.groovy index 822d08f5a..6cbb23941 100644 --- a/src/test/groovy/graphql/execution/instrumentation/ModernTestingInstrumentation.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/ModernTestingInstrumentation.groovy @@ -103,6 +103,12 @@ class ModernTestingInstrumentation implements Instrumentation { return new TestingInstrumentContext("complete-list-$parameters.field.name", executionList, throwableList, useOnDispatch) } + @Override + InstrumentationContext beginDeferredField(InstrumentationFieldParameters parameters, InstrumentationState state) { + assert state == instrumentationState + return new TestingInstrumentContext("deferred-field-$parameters.field.name", executionList, throwableList, useOnDispatch) + } + @Override ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters, InstrumentationState state) { assert state == instrumentationState From b831e2148d008906025bad14e4ec75baa986c4e3 Mon Sep 17 00:00:00 2001 From: bbaker Date: Tue, 12 Aug 2025 15:31:42 +1000 Subject: [PATCH 2/3] Fixed deferred support to have proper Instrumentation - fixed bugs --- .../incremental/DeferredExecutionSupport.java | 6 ++---- .../InstrumentationTest.groovy | 19 ++++++++----------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java b/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java index b4b5ee39a..6e428041a 100644 --- a/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java +++ b/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java @@ -18,7 +18,7 @@ import graphql.execution.instrumentation.parameters.InstrumentationFieldParameters; import graphql.incremental.IncrementalPayload; import graphql.util.FpKit; -import org.jetbrains.annotations.NotNull; +import org.jspecify.annotations.NonNull; import java.util.Collections; import java.util.HashMap; @@ -162,8 +162,6 @@ private Supplier> resolveDeferredFieldValue(MergedField currentField, ExecutionContext executionContext, ExecutionStrategyParameters executionStrategyParameters) { return () -> { diff --git a/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy b/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy index ffe8f5a62..08ea4850c 100644 --- a/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy @@ -506,9 +506,7 @@ class InstrumentationTest extends Specification { def query = """ { hero { - ... @defer(label: "id") { - id - } + id ... @defer(label: "name") { name } @@ -551,6 +549,13 @@ class InstrumentationTest extends Specification { "end:fetch-hero", "start:complete-hero", "start:execute-object", + "start:field-id", + "start:fetch-id", + "end:fetch-id", + "start:complete-id", + "end:complete-id", + "end:field-id", + "end:execute-object", "end:complete-hero", "end:field-hero", @@ -559,14 +564,6 @@ class InstrumentationTest extends Specification { "end:execution", // // the deferred field resolving now happens after the operation has come back - "start:deferred-field-id", - "start:field-id", - "start:fetch-id", - "end:fetch-id", - "start:complete-id", - "end:complete-id", - "end:field-id", - "end:deferred-field-id", "start:deferred-field-name", "start:field-name", "start:fetch-name", From 215be0b95eb1bfab5417c1e47ad956eda2f0cda8 Mon Sep 17 00:00:00 2001 From: bbaker Date: Tue, 12 Aug 2025 23:58:52 +1000 Subject: [PATCH 3/3] Added NoContextChainedInstrumentation.java support --- .../instrumentation/NoContextChainedInstrumentation.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java index 89e91b6e5..5ae45dc9d 100644 --- a/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/NoContextChainedInstrumentation.java @@ -83,6 +83,11 @@ public ExecutionStrategyInstrumentationContext beginExecutionStrategy(Instrument return runAll(state, (instrumentation, specificState) -> instrumentation.beginExecuteObject(parameters, specificState)); } + @Override + public InstrumentationContext beginDeferredField(InstrumentationFieldParameters parameters, InstrumentationState state) { + return runAll(state, (instrumentation, specificState) -> instrumentation.beginDeferredField(parameters, specificState)); + } + @Override public InstrumentationContext beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) { return runAll(state, (instrumentation, specificState) -> instrumentation.beginSubscribedFieldEvent(parameters, specificState));