Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer execution 2024 #3421

Merged
merged 54 commits into from Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
7694618
WIP: Created execution result classes for incremental delivery
felipe-gdr-atlassian Dec 20, 2023
ae52efd
Merge branch 'defer-support-in-enf' into defer-support-execution
felipe-gdr-atlassian Dec 20, 2023
acb00a5
Merge branch 'defer-support-in-enf' into defer-support-execution
felipe-gdr-atlassian Dec 20, 2023
7aa8c23
WIP
felipe-gdr-atlassian Jan 2, 2024
10b0499
Merge branch 'defer-support-in-enf' into defer-support-execution
felipe-gdr-atlassian Jan 8, 2024
0025495
WIP
felipe-gdr-atlassian Jan 8, 2024
b02efd8
Revert "WIP"
felipe-gdr-atlassian Jan 9, 2024
691d138
Merge branch 'defer-support-in-enf' into defer-support-execution
felipe-gdr-atlassian Jan 9, 2024
cbd9b63
Fixes on IncrementalItem hierarchy
felipe-gdr-atlassian Jan 10, 2024
40b7dfa
Merge branch 'master' into defer-support-execution
felipe-gdr-atlassian Jan 11, 2024
ceac689
Adjustments and plenty of Javadoc
felipe-gdr-atlassian Jan 11, 2024
a8f7906
Add unit test for sanity check
felipe-gdr-atlassian Jan 11, 2024
6b52b02
Fix Javadoc
felipe-gdr-atlassian Jan 11, 2024
d13b509
Minor adjustment in test class
felipe-gdr-atlassian Jan 11, 2024
291cfbf
Reverts both merge commits that deleted the old defer stuff.
felipe-gdr-atlassian Jan 12, 2024
c7088b1
Fix compilation errors
felipe-gdr-atlassian Jan 15, 2024
e86286e
WIP: hacky defer is working, some new tests for basic stuff, but a lo…
felipe-gdr-atlassian Jan 18, 2024
36d0330
Fixed a race condition and added more tests
felipe-gdr-atlassian Jan 19, 2024
348515d
Memoize field value resolution
felipe-gdr-atlassian Jan 19, 2024
ea9726a
Remove code
felipe-gdr-atlassian Jan 19, 2024
d03c8db
Remove unnecessary code and cover more edge cases
felipe-gdr-atlassian Jan 23, 2024
ca51dcb
Add more tests for DeferContext
felipe-gdr-atlassian Jan 23, 2024
85ac363
Create IncrementalCall interface and add type abstraction
felipe-gdr-atlassian Jan 23, 2024
1c60c37
Implement error handling in the defer execution code
felipe-gdr-atlassian Jan 25, 2024
276c872
Add one more test case
felipe-gdr-atlassian Jan 25, 2024
a5aa0fd
Add test for defer in list items
felipe-gdr-atlassian Jan 29, 2024
f43849f
Remove unnecessary test code
felipe-gdr-atlassian Jan 29, 2024
98554dc
Javadoc
felipe-gdr-atlassian Jan 29, 2024
ccb7e70
Refactor IncrementalUtils to remove duplicated code
felipe-gdr-atlassian Jan 29, 2024
e0540d9
WIP
felipe-gdr-atlassian Feb 1, 2024
f912440
Fix all unit tests and start cleaning up
felipe-gdr-atlassian Feb 2, 2024
28d98d4
Simplify instrumentation code for deferred field execution
felipe-gdr-atlassian Feb 2, 2024
8cc3d13
Merge branch 'master' into defer-execution-2024
felipe-gdr-atlassian Feb 2, 2024
07bd477
Rename some things defer -> incremental
felipe-gdr-atlassian Feb 2, 2024
934f88f
Remove code duplication
felipe-gdr-atlassian Feb 2, 2024
52de911
Various small adjustments in preparation to send the PR for review
felipe-gdr-atlassian Feb 2, 2024
f1e03a6
Remove unused import
felipe-gdr-atlassian Feb 2, 2024
ef671d0
PR feedback
felipe-gdr-atlassian Feb 5, 2024
32811eb
Merge branch 'master' into defer-execution-2024
felipe-gdr-atlassian Feb 6, 2024
c6d830c
Undo renaming to NormalizedDeferExecution: will be done in another PR
felipe-gdr-atlassian Feb 7, 2024
fe710a9
Undo refactoring around IncrementalNodes: will be done on future PR
felipe-gdr-atlassian Feb 7, 2024
61ca8d1
Refactoring method to receive individual parameters instead of object
felipe-gdr-atlassian Feb 7, 2024
864bf2e
Replace synchronized with runBlocked
felipe-gdr-atlassian Feb 7, 2024
4db2122
Fix groovy syntax
felipe-gdr-atlassian Feb 9, 2024
89e741d
Move gate mechanism from ExecutionInput to GraphQLContext
felipe-gdr-atlassian Feb 9, 2024
5900e9c
Rename DelayedIncrementalExecutionResult -> DelayedIncrementalPartial…
felipe-gdr-atlassian Feb 9, 2024
6fb8e74
(temporarily) Remove support for using Data loader to fetch deferred …
felipe-gdr-atlassian Feb 9, 2024
d87a6df
Rename DeferredCall -> DeferredFragmentCall
felipe-gdr-atlassian Feb 9, 2024
bfafd99
Move DeferredExecutionSupport into its own class file
felipe-gdr-atlassian Feb 9, 2024
e96eec5
Merge branch 'master' into defer-execution-2024
felipe-gdr-atlassian Feb 9, 2024
00a1862
Adjustments after merging
felipe-gdr-atlassian Feb 9, 2024
a801d20
Move ENABLE_INCREMENTAL_SUPPORT to @ExperimentalApi
felipe-gdr-atlassian Feb 12, 2024
99c5d0f
Remove unnecessary isEmpty check
felipe-gdr-atlassian Feb 12, 2024
b126baf
Add test with label having 'null' value
felipe-gdr-atlassian Feb 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/graphql/ExecutionInput.java
Expand Up @@ -383,4 +383,4 @@ public ExecutionInput build() {
return new ExecutionInput(this);
}
}
}
}
38 changes: 21 additions & 17 deletions src/main/java/graphql/ExecutionResultImpl.java
Expand Up @@ -40,6 +40,10 @@ public ExecutionResultImpl(ExecutionResultImpl other) {
this(other.dataPresent, other.data, other.errors, other.extensions);
}

public <T extends Builder<T>> ExecutionResultImpl(Builder<T> builder) {
this(builder.dataPresent, builder.data, builder.errors, builder.extensions);
}

private ExecutionResultImpl(boolean dataPresent, Object data, List<? extends GraphQLError> errors, Map<Object, Object> extensions) {
this.dataPresent = dataPresent;
this.data = data;
Expand Down Expand Up @@ -103,61 +107,61 @@ public String toString() {
'}';
}

public static Builder newExecutionResult() {
return new Builder();
public static <T extends Builder<T>> Builder<T> newExecutionResult() {
return new Builder<>();
}

public static class Builder implements ExecutionResult.Builder<Builder> {
public static class Builder<T extends Builder<T>> implements ExecutionResult.Builder<T> {
private boolean dataPresent;
private Object data;
private List<GraphQLError> errors = new ArrayList<>();
private Map<Object, Object> extensions;

@Override
public Builder from(ExecutionResult executionResult) {
public T from(ExecutionResult executionResult) {
dataPresent = executionResult.isDataPresent();
data = executionResult.getData();
errors = new ArrayList<>(executionResult.getErrors());
extensions = executionResult.getExtensions();
return this;
return (T) this;
}

@Override
public Builder data(Object data) {
public T data(Object data) {
dataPresent = true;
this.data = data;
return this;
return (T) this;
}

@Override
public Builder errors(List<GraphQLError> errors) {
public T errors(List<GraphQLError> errors) {
this.errors = errors;
return this;
return (T) this;
}

@Override
public Builder addErrors(List<GraphQLError> errors) {
public T addErrors(List<GraphQLError> errors) {
this.errors.addAll(errors);
return this;
return (T) this;
}

@Override
public Builder addError(GraphQLError error) {
public T addError(GraphQLError error) {
this.errors.add(error);
return this;
return (T) this;
}

@Override
public Builder extensions(Map<Object, Object> extensions) {
public T extensions(Map<Object, Object> extensions) {
this.extensions = extensions;
return this;
return (T) this;
}

@Override
public Builder addExtension(String key, Object value) {
public T addExtension(String key, Object value) {
this.extensions = (this.extensions == null ? new LinkedHashMap<>() : this.extensions);
this.extensions.put(key, value);
return this;
return (T) this;
}

@Override
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I had more time again I would make all Builder builders of T

Expand Down
4 changes: 4 additions & 0 deletions src/main/java/graphql/ExperimentalApi.java
Expand Up @@ -20,4 +20,8 @@
@Target(value = {CONSTRUCTOR, METHOD, TYPE, FIELD})
@Documented
public @interface ExperimentalApi {
/**
* The key that should be associated with a boolean value which indicates whether @defer and @stream behaviour is enabled for this execution.
*/
String ENABLE_INCREMENTAL_SUPPORT = "ENABLE_INCREMENTAL_SUPPORT";
}
15 changes: 10 additions & 5 deletions src/main/java/graphql/execution/AsyncExecutionStrategy.java
Expand Up @@ -2,6 +2,7 @@

import graphql.ExecutionResult;
import graphql.PublicApi;
import graphql.execution.incremental.DeferredExecutionSupport;
import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext;
import graphql.execution.instrumentation.Instrumentation;
import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters;
Expand All @@ -10,7 +11,6 @@
import java.util.concurrent.CompletableFuture;
import java.util.function.BiConsumer;


/**
* The standard graphql execution strategy that runs fields asynchronously non-blocking.
*/
Expand All @@ -36,19 +36,24 @@ public AsyncExecutionStrategy(DataFetcherExceptionHandler exceptionHandler) {
@Override
@SuppressWarnings("FutureReturnValueIgnored")
public CompletableFuture<ExecutionResult> execute(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException {

Instrumentation instrumentation = executionContext.getInstrumentation();
InstrumentationExecutionStrategyParameters instrumentationParameters = new InstrumentationExecutionStrategyParameters(executionContext, parameters);

ExecutionStrategyInstrumentationContext executionStrategyCtx = ExecutionStrategyInstrumentationContext.nonNullCtx(instrumentation.beginExecutionStrategy(instrumentationParameters, executionContext.getInstrumentationState()));

List<String> fieldNames = parameters.getFields().getKeys();
Async.CombinedBuilder<FieldValueInfo> futures = getAsyncFieldValueInfo(executionContext, parameters);
MergedSelectionSet fields = parameters.getFields();
List<String> fieldNames = fields.getKeys();

DeferredExecutionSupport deferredExecutionSupport = createDeferredExecutionSupport(executionContext, parameters);
Async.CombinedBuilder<FieldValueInfo> futures = getAsyncFieldValueInfo(executionContext, parameters, deferredExecutionSupport);

CompletableFuture<ExecutionResult> overallResult = new CompletableFuture<>();
executionStrategyCtx.onDispatched(overallResult);

futures.await().whenComplete((completeValueInfos, throwable) -> {
BiConsumer<List<Object>, Throwable> handleResultsConsumer = handleResults(executionContext, fieldNames, overallResult);
List<String> fieldsExecutedOnInitialResult = deferredExecutionSupport.getNonDeferredFieldNames(fieldNames);

BiConsumer<List<Object>, Throwable> handleResultsConsumer = handleResults(executionContext, fieldsExecutedOnInitialResult, overallResult);
if (throwable != null) {
handleResultsConsumer.accept(null, throwable.getCause());
return;
Expand Down
40 changes: 38 additions & 2 deletions src/main/java/graphql/execution/Execution.java
Expand Up @@ -4,15 +4,19 @@
import graphql.ExecutionInput;
import graphql.ExecutionResult;
import graphql.ExecutionResultImpl;
import graphql.ExperimentalApi;
import graphql.GraphQLContext;
import graphql.GraphQLError;
import graphql.Internal;
import graphql.execution.incremental.IncrementalCallState;
import graphql.execution.instrumentation.Instrumentation;
import graphql.execution.instrumentation.InstrumentationContext;
import graphql.execution.instrumentation.InstrumentationState;
import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters;
import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters;
import graphql.extensions.ExtensionsBuilder;
import graphql.incremental.DelayedIncrementalPartialResult;
import graphql.incremental.IncrementalExecutionResultImpl;
import graphql.language.Document;
import graphql.language.FragmentDefinition;
import graphql.language.NodeUtil;
Expand All @@ -21,10 +25,12 @@
import graphql.schema.GraphQLObjectType;
import graphql.schema.GraphQLSchema;
import graphql.schema.impl.SchemaUtil;
import org.reactivestreams.Publisher;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;

import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder;
Expand Down Expand Up @@ -133,7 +139,13 @@ private CompletableFuture<ExecutionResult> executeOperation(ExecutionContext exe
.graphQLContext(graphQLContext)
.build();

MergedSelectionSet fields = fieldCollector.collectFields(collectorParameters, operationDefinition.getSelectionSet());
MergedSelectionSet fields = fieldCollector.collectFields(
collectorParameters,
operationDefinition.getSelectionSet(),
Optional.ofNullable(executionContext.getGraphQLContext())
.map(graphqlContext -> (Boolean) graphqlContext.get(ExperimentalApi.ENABLE_INCREMENTAL_SUPPORT))
.orElse(false)
);

ResultPath path = ResultPath.rootPath();
ExecutionStepInfo executionStepInfo = newExecutionStepInfo().type(operationRootType).path(path).build();
Expand Down Expand Up @@ -170,7 +182,31 @@ private CompletableFuture<ExecutionResult> executeOperation(ExecutionContext exe
result = result.thenApply(er -> mergeExtensionsBuilderIfPresent(er, graphQLContext));

result = result.whenComplete(executeOperationCtx::onCompleted);
return result;

return incrementalSupport(executionContext, result);
}

/*
* Adds the deferred publisher if it's needed at the end of the query. This is also a good time for the deferred code to start running
*/
private CompletableFuture<ExecutionResult> incrementalSupport(ExecutionContext executionContext, CompletableFuture<ExecutionResult> result) {
return result.thenApply(er -> {
IncrementalCallState incrementalCallState = executionContext.getIncrementalCallState();
if (incrementalCallState.getIncrementalCallsDetected()) {
// we start the rest of the query now to maximize throughput. We have the initial important results,
// and now we can start the rest of the calls as early as possible (even before someone subscribes)
Publisher<DelayedIncrementalPartialResult> publisher = incrementalCallState.startDeferredCalls();

return IncrementalExecutionResultImpl.fromExecutionResult(er)
// "hasNext" can, in theory, be "false" when all the incremental items are delivered in the
// first response payload. However, the current implementation will never result in this.
// The behaviour might change if we decide to make optimizations in the future.
.hasNext(true)
.incrementalItemPublisher(publisher)
.build();
}
return er;
});
}

private void addExtensionsBuilderNotPresent(GraphQLContext graphQLContext) {
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/graphql/execution/ExecutionContext.java
Expand Up @@ -8,6 +8,7 @@
import graphql.GraphQLError;
import graphql.PublicApi;
import graphql.collect.ImmutableKit;
import graphql.execution.incremental.IncrementalCallState;
import graphql.execution.instrumentation.Instrumentation;
import graphql.execution.instrumentation.InstrumentationState;
import graphql.language.Document;
Expand Down Expand Up @@ -53,6 +54,7 @@ public class ExecutionContext {
private final Set<ResultPath> errorPaths = new HashSet<>();
private final DataLoaderRegistry dataLoaderRegistry;
private final Locale locale;
private final IncrementalCallState incrementalCallState = new IncrementalCallState();
private final ValueUnboxer valueUnboxer;
private final ExecutionInput executionInput;
private final Supplier<ExecutableNormalizedOperation> queryTree;
Expand Down Expand Up @@ -255,6 +257,10 @@ public ExecutionStrategy getSubscriptionStrategy() {
return subscriptionStrategy;
}

public IncrementalCallState getIncrementalCallState() {
return incrementalCallState;
}

public ExecutionStrategy getStrategy(OperationDefinition.Operation operation) {
if (operation == OperationDefinition.Operation.MUTATION) {
return getMutationStrategy();
Expand Down