Skip to content

Commit d81252f

Browse files
andimarekandimarek
authored
Merge pull request #4449 from graphql-java/codex/backport-4384-to-26.x
Backport 26.1: Fix subscription alternative call context cleanup
2 parents a82f21b + 46a265c commit d81252f

13 files changed

Lines changed: 143 additions & 48 deletions

‎src/main/java/graphql/execution/DataLoaderDispatchStrategy.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ default void subscriptionEventCompletionDone(AlternativeCallContext alternativeC
6565

6666
}
6767

68+
default void subscriptionEventExecutionDone(AlternativeCallContext alternativeCallContext) {
69+
70+
}
71+
6872
default void finishedFetching(ExecutionContext executionContext, ExecutionStrategyParameters newParameters) {
6973

7074
}

‎src/main/java/graphql/execution/ExecutionStrategy.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import graphql.UnresolvedTypeError;
1515
import graphql.execution.directives.QueryDirectives;
1616
import graphql.execution.directives.QueryDirectivesImpl;
17+
import graphql.execution.incremental.AlternativeCallContext;
1718
import graphql.execution.incremental.DeferredExecutionSupport;
1819
import graphql.execution.incremental.IncrementalExecutionContextKeys;
1920
import graphql.execution.instrumentation.ExecuteObjectInstrumentationContext;
@@ -457,7 +458,7 @@ private Object fetchField(GraphQLFieldDefinition fieldDef, ExecutionContext exec
457458
.parentType(parentType)
458459
.selectionSet(fieldCollector)
459460
.queryDirectives(queryDirectives)
460-
.deferredCallContext(parameters.getDeferredCallContext())
461+
.alternativeCallContext(parameters.getAlternativeCallContext())
461462
.level(parameters.getPath().getLevel())
462463
.build();
463464
});
@@ -1122,18 +1123,20 @@ private Supplier<ExecutionStepInfo> createExecutionStepInfo(ExecutionContext exe
11221123
return FpKit.intraThreadMemoize(() -> createExecutionStepInfo(executionContext, parameters, fieldDef, null));
11231124
}
11241125

1125-
// Errors that result from the execution of deferred fields are kept in the deferred context only.
1126+
// Errors in alternative execution paths are kept in the alternative call context.
11261127
private static void addErrorToRightContext(GraphQLError error, ExecutionStrategyParameters parameters, ExecutionContext executionContext) {
1127-
if (parameters.getDeferredCallContext() != null) {
1128-
parameters.getDeferredCallContext().addError(error);
1128+
AlternativeCallContext alternativeCallContext = parameters.getAlternativeCallContext();
1129+
if (alternativeCallContext != null) {
1130+
alternativeCallContext.addError(error);
11291131
} else {
11301132
executionContext.addError(error);
11311133
}
11321134
}
11331135

11341136
private static void addErrorsToRightContext(List<GraphQLError> errors, ExecutionStrategyParameters parameters, ExecutionContext executionContext) {
1135-
if (parameters.getDeferredCallContext() != null) {
1136-
parameters.getDeferredCallContext().addErrors(errors);
1137+
AlternativeCallContext alternativeCallContext = parameters.getAlternativeCallContext();
1138+
if (alternativeCallContext != null) {
1139+
alternativeCallContext.addErrors(errors);
11371140
} else {
11381141
executionContext.addErrors(errors);
11391142
}

‎src/main/java/graphql/execution/ExecutionStrategyParameters.java‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -76,30 +76,26 @@ public ResultPath getPath() {
7676
return parent;
7777
}
7878

79+
/**
80+
* Returns the alternative call context if this execution is scoped to an alternative execution path.
81+
* This is used for deferred fragment execution and subscription event execution.
82+
* @return the alternative call context or null if execution is not scoped to an alternative execution path
83+
*/
84+
@Nullable
85+
@Internal
86+
public AlternativeCallContext getAlternativeCallContext() {
87+
return alternativeCallContext;
88+
}
89+
7990
/**
8091
* Returns the deferred call context if we're in the scope of a deferred call.
81-
* A new DeferredCallContext is created for each @defer block, and is passed down to all fields within the deferred call.
82-
*
83-
* <pre>
84-
* query {
85-
* ... @defer {
86-
* field1 { # new DeferredCallContext created here
87-
* field1a # DeferredCallContext passed down to this field
88-
* }
89-
* }
90-
*
91-
* ... @defer {
92-
* field2 # new DeferredCallContext created here
93-
* }
94-
* }
95-
* </pre>
9692
*
9793
* @return the deferred call context or null if we're not in the scope of a deferred call
9894
*/
9995
@Nullable
10096
@Internal
10197
public AlternativeCallContext getDeferredCallContext() {
102-
return alternativeCallContext;
98+
return getAlternativeCallContext();
10399
}
104100

105101
/**
@@ -293,11 +289,15 @@ public Builder parent(ExecutionStrategyParameters parent) {
293289
return this;
294290
}
295291

296-
public Builder deferredCallContext(AlternativeCallContext alternativeCallContext) {
292+
public Builder alternativeCallContext(AlternativeCallContext alternativeCallContext) {
297293
this.alternativeCallContext = alternativeCallContext;
298294
return this;
299295
}
300296

297+
public Builder deferredCallContext(AlternativeCallContext alternativeCallContext) {
298+
return alternativeCallContext(alternativeCallContext);
299+
}
300+
301301
public ExecutionStrategyParameters build() {
302302
return new ExecutionStrategyParameters(executionStepInfo, source, localContext, fields, nonNullableFieldValidator, path, currentField, parent, alternativeCallContext);
303303
}

‎src/main/java/graphql/execution/NonNullableFieldValidator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ public <T> T validate(ExecutionStrategyParameters parameters, T result) throws N
5050

5151
NonNullableFieldWasNullException nonNullException = new NonNullableFieldWasNullException(executionStepInfo, path);
5252
final GraphQLError error = new NonNullableFieldWasNullError(nonNullException);
53-
if(parameters.getDeferredCallContext() != null) {
54-
parameters.getDeferredCallContext().addError(error);
53+
if(parameters.getAlternativeCallContext() != null) {
54+
parameters.getAlternativeCallContext().addError(error);
5555
} else {
5656
executionContext.addError(error, path);
5757
}

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -180,14 +180,20 @@ private CompletableFuture<ExecutionResult> executeSubscriptionEvent(ExecutionCon
180180
));
181181

182182

183-
executionContext.getDataLoaderDispatcherStrategy().newSubscriptionExecution(newParameters.getDeferredCallContext());
183+
AlternativeCallContext alternativeCallContext = assertNotNull(
184+
newParameters.getAlternativeCallContext(),
185+
"alternativeCallContext must not be null");
186+
executionContext.getDataLoaderDispatcherStrategy().newSubscriptionExecution(alternativeCallContext);
184187
Object fetchedValue = unboxPossibleDataFetcherResult(newExecutionContext, newParameters, eventPayload);
185188
FieldValueInfo fieldValueInfo = completeField(newExecutionContext, newParameters, fetchedValue);
186-
executionContext.getDataLoaderDispatcherStrategy().subscriptionEventCompletionDone(newParameters.getDeferredCallContext());
189+
executionContext.getDataLoaderDispatcherStrategy().subscriptionEventCompletionDone(alternativeCallContext);
187190
CompletableFuture<ExecutionResult> overallResult = fieldValueInfo
188191
.getFieldValueFuture()
189-
.thenApply(val -> new ExecutionResultImpl(val, assertNotNull(newParameters.getDeferredCallContext(), "deferredCallContext must not be null").getErrors()))
190-
.thenApply(executionResult -> wrapWithRootFieldName(newParameters, executionResult));
192+
.thenApply(val -> new ExecutionResultImpl(val, alternativeCallContext.getErrors()))
193+
.thenApply(executionResult -> wrapWithRootFieldName(newParameters, executionResult))
194+
.whenComplete((executionResult, throwable) -> {
195+
executionContext.getDataLoaderDispatcherStrategy().subscriptionEventExecutionDone(alternativeCallContext);
196+
});
191197

192198
// dispatch instrumentation so they can know about each subscription event
193199
subscribedFieldCtx.onDispatched();
@@ -230,7 +236,7 @@ private ExecutionStrategyParameters firstFieldOfSubscriptionSelection(ExecutionC
230236
.path(fieldPath)
231237
.nonNullFieldValidator(nonNullableFieldValidator);
232238
if (newCallContext) {
233-
builder.deferredCallContext(new AlternativeCallContext(1, 1));
239+
builder.alternativeCallContext(new AlternativeCallContext(1, 1));
234240
}
235241
});
236242

‎src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ private Supplier<CompletableFuture<DeferredFragmentCall.FieldWithExecutionResult
155155
{
156156
MergedSelectionSet mergedSelectionSet = MergedSelectionSet.newMergedSelectionSet().subFields(fields).build();
157157
ResultPath path = parameters.getPath().segment(currentField.getResultKey());
158-
builder.deferredCallContext(alternativeCallContext)
158+
builder.alternativeCallContext(alternativeCallContext)
159159
.field(currentField)
160160
.fields(mergedSelectionSet)
161161
.path(path)

‎src/main/java/graphql/execution/instrumentation/dataloader/ExhaustedDataLoaderDispatchStrategy.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,12 +173,17 @@ public void subscriptionEventCompletionDone(AlternativeCallContext alternativeCa
173173
decrementObjectRunningAndMaybeDispatch(callStack);
174174
}
175175

176+
@Override
177+
public void subscriptionEventExecutionDone(AlternativeCallContext alternativeCallContext) {
178+
alternativeCallContextMap.remove(alternativeCallContext);
179+
}
180+
176181
@Override
177182
public void deferFieldFetched(ExecutionStrategyParameters parameters) {
178183
CallStack callStack = getCallStack(parameters);
179184
int deferredFragmentRootFieldsCompleted = callStack.deferredFragmentRootFieldsCompleted.incrementAndGet();
180-
Assert.assertNotNull(parameters.getDeferredCallContext());
181-
if (deferredFragmentRootFieldsCompleted == parameters.getDeferredCallContext().getFields()) {
185+
Assert.assertNotNull(parameters.getAlternativeCallContext());
186+
if (deferredFragmentRootFieldsCompleted == parameters.getAlternativeCallContext().getFields()) {
182187
decrementObjectRunningAndMaybeDispatch(callStack);
183188
}
184189
}
@@ -195,7 +200,7 @@ public void stopComplete(ExecutionStrategyParameters parameters) {
195200
}
196201

197202
private CallStack getCallStack(ExecutionStrategyParameters parameters) {
198-
return getCallStack(parameters.getDeferredCallContext());
203+
return getCallStack(parameters.getAlternativeCallContext());
199204
}
200205

201206
private CallStack getCallStack(@Nullable AlternativeCallContext alternativeCallContext) {
@@ -281,4 +286,3 @@ public void newDataLoaderInvocation(@Nullable AlternativeCallContext alternative
281286

282287

283288
}
284-

‎src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -362,8 +362,8 @@ public void fieldFetched(ExecutionContext executionContext,
362362
Supplier<DataFetchingEnvironment> dataFetchingEnvironment) {
363363
CallStack callStack = getCallStack(executionStrategyParameters);
364364
int level = executionStrategyParameters.getPath().getLevel();
365-
AlternativeCallContext deferredCallContext = executionStrategyParameters.getDeferredCallContext();
366-
if (level == 1 || (deferredCallContext != null && level == deferredCallContext.getStartLevel())) {
365+
AlternativeCallContext alternativeCallContext = executionStrategyParameters.getAlternativeCallContext();
366+
if (level == 1 || (alternativeCallContext != null && level == alternativeCallContext.getStartLevel())) {
367367
int happenedFirstLevelFetchCount = callStack.happenedFirstLevelFetchCount.incrementAndGet();
368368
if (happenedFirstLevelFetchCount == callStack.expectedFirstLevelFetchCount) {
369369
callStack.dispatchedLevels.add(level);
@@ -395,20 +395,25 @@ public void subscriptionEventCompletionDone(AlternativeCallContext alternativeCa
395395
onCompletionFinished(0, callStack);
396396
}
397397

398+
@Override
399+
public void subscriptionEventExecutionDone(AlternativeCallContext alternativeCallContext) {
400+
alternativeCallContextMap.remove(alternativeCallContext);
401+
}
402+
398403
@Override
399404
public void deferredOnFieldValue(String resultKey, FieldValueInfo fieldValueInfo, Throwable throwable, ExecutionStrategyParameters parameters) {
400405
CallStack callStack = getCallStack(parameters);
401406
int deferredFragmentRootFieldsCompleted = callStack.deferredFragmentRootFieldsCompleted.incrementAndGet();
402-
Assert.assertNotNull(parameters.getDeferredCallContext());
403-
if (deferredFragmentRootFieldsCompleted == parameters.getDeferredCallContext().getFields()) {
404-
onCompletionFinished(parameters.getDeferredCallContext().getStartLevel() - 1, callStack);
407+
Assert.assertNotNull(parameters.getAlternativeCallContext());
408+
if (deferredFragmentRootFieldsCompleted == parameters.getAlternativeCallContext().getFields()) {
409+
onCompletionFinished(parameters.getAlternativeCallContext().getStartLevel() - 1, callStack);
405410
}
406411

407412
}
408413

409414

410415
private CallStack getCallStack(ExecutionStrategyParameters parameters) {
411-
return getCallStack(parameters.getDeferredCallContext());
416+
return getCallStack(parameters.getAlternativeCallContext());
412417
}
413418

414419
private CallStack getCallStack(@Nullable AlternativeCallContext alternativeCallContext) {
@@ -520,4 +525,3 @@ public void newDataLoaderInvocation(int level,
520525

521526

522527
}
523-

‎src/main/java/graphql/schema/DataFetchingEnvironmentImpl.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,11 +458,15 @@ public Builder queryDirectives(QueryDirectives queryDirectives) {
458458
return this;
459459
}
460460

461-
public Builder deferredCallContext(AlternativeCallContext alternativeCallContext) {
461+
public Builder alternativeCallContext(AlternativeCallContext alternativeCallContext) {
462462
this.alternativeCallContext = alternativeCallContext;
463463
return this;
464464
}
465465

466+
public Builder deferredCallContext(AlternativeCallContext alternativeCallContext) {
467+
return alternativeCallContext(alternativeCallContext);
468+
}
469+
466470
public DataFetchingEnvironment build() {
467471
return new DataFetchingEnvironmentImpl(this);
468472
}
@@ -499,10 +503,14 @@ public DataLoaderDispatchStrategy getDataLoaderDispatchStrategy() {
499503
return dataLoaderDispatchStrategy;
500504
}
501505

502-
public AlternativeCallContext getDeferredCallContext() {
506+
public AlternativeCallContext getAlternativeCallContext() {
503507
return alternativeCallContext;
504508
}
505509

510+
public AlternativeCallContext getDeferredCallContext() {
511+
return getAlternativeCallContext();
512+
}
513+
506514
public Profiler getProfiler() {
507515
return profiler;
508516
}

‎src/main/java/graphql/schema/DataLoaderWithContext.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,11 @@ private void newDataLoaderInvocation() {
6868
DataFetchingEnvironmentImpl dfeImpl = (DataFetchingEnvironmentImpl) dfe;
6969
DataFetchingEnvironmentImpl.DFEInternalState dfeInternalState = (DataFetchingEnvironmentImpl.DFEInternalState) dfeImpl.toInternal();
7070
if (dfeInternalState.getDataLoaderDispatchStrategy() instanceof PerLevelDataLoaderDispatchStrategy) {
71-
AlternativeCallContext alternativeCallContext = dfeInternalState.getDeferredCallContext();
71+
AlternativeCallContext alternativeCallContext = dfeInternalState.getAlternativeCallContext();
7272
int level = dfeImpl.getLevel();
7373
((PerLevelDataLoaderDispatchStrategy) dfeInternalState.dataLoaderDispatchStrategy).newDataLoaderInvocation(level, delegate, alternativeCallContext);
7474
} else if (dfeInternalState.getDataLoaderDispatchStrategy() instanceof ExhaustedDataLoaderDispatchStrategy) {
75-
AlternativeCallContext alternativeCallContext = dfeInternalState.getDeferredCallContext();
75+
AlternativeCallContext alternativeCallContext = dfeInternalState.getAlternativeCallContext();
7676
((ExhaustedDataLoaderDispatchStrategy) dfeInternalState.dataLoaderDispatchStrategy).newDataLoaderInvocation(alternativeCallContext);
7777
}
7878
}

0 commit comments

Comments
 (0)