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

Native DataLoader dispatch strategy without Instrumentation #3447

Merged
merged 21 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
4 changes: 2 additions & 2 deletions src/main/java/graphql/ExecutionInput.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import graphql.collect.ImmutableKit;
import graphql.execution.ExecutionId;
import graphql.execution.RawVariables;
import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentationState;
import org.dataloader.DataLoaderRegistry;

import java.util.Locale;
Expand All @@ -12,6 +11,7 @@
import java.util.function.UnaryOperator;

import static graphql.Assert.assertNotNull;
import static graphql.execution.instrumentation.dataloader.EmptyDataLoaderRegistryInstance.EMPTY_DATALOADER_REGISTRY;

/**
* This represents the series of values that can be input on a graphql query execution
Expand Down Expand Up @@ -213,7 +213,7 @@ public static class Builder {
// this is important - it allows code to later known if we never really set a dataloader and hence it can optimize
// dataloader field tracking away.
//
private DataLoaderRegistry dataLoaderRegistry = DataLoaderDispatcherInstrumentationState.EMPTY_DATALOADER_REGISTRY;
private DataLoaderRegistry dataLoaderRegistry = EMPTY_DATALOADER_REGISTRY;
private Locale locale = Locale.getDefault();
private ExecutionId executionId;

Expand Down
91 changes: 40 additions & 51 deletions src/main/java/graphql/GraphQL.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@
import graphql.execution.SimpleDataFetcherExceptionHandler;
import graphql.execution.SubscriptionExecutionStrategy;
import graphql.execution.ValueUnboxer;
import graphql.execution.instrumentation.ChainedInstrumentation;
import graphql.execution.instrumentation.DocumentAndVariables;
import graphql.execution.instrumentation.Instrumentation;
import graphql.execution.instrumentation.InstrumentationContext;
import graphql.execution.instrumentation.InstrumentationState;
import graphql.execution.instrumentation.NoContextChainedInstrumentation;
import graphql.execution.instrumentation.SimplePerformantInstrumentation;
import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation;
import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters;
import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters;
import graphql.execution.instrumentation.parameters.InstrumentationValidationParameters;
Expand All @@ -30,7 +28,6 @@
import graphql.schema.GraphQLSchema;
import graphql.validation.ValidationError;

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
Expand Down Expand Up @@ -96,6 +93,7 @@ public class GraphQL {
private final Instrumentation instrumentation;
private final PreparsedDocumentProvider preparsedDocumentProvider;
private final ValueUnboxer valueUnboxer;
private final boolean doNotAutomaticallyDispatchDataLoader;


private GraphQL(Builder builder) {
Expand All @@ -107,6 +105,7 @@ private GraphQL(Builder builder) {
this.instrumentation = assertNotNull(builder.instrumentation, () -> "instrumentation must not be null");
this.preparsedDocumentProvider = assertNotNull(builder.preparsedDocumentProvider, () -> "preparsedDocumentProvider must be non null");
this.valueUnboxer = assertNotNull(builder.valueUnboxer, () -> "valueUnboxer must not be null");
this.doNotAutomaticallyDispatchDataLoader = builder.doNotAutomaticallyDispatchDataLoader;
}

/**
Expand Down Expand Up @@ -151,6 +150,10 @@ public Instrumentation getInstrumentation() {
return instrumentation;
}

public boolean isDoNotAutomaticallyDispatchDataLoader() {
return doNotAutomaticallyDispatchDataLoader;
}

/**
* @return the PreparsedDocumentProvider for this {@link GraphQL} instance
*/
Expand All @@ -169,7 +172,6 @@ public ValueUnboxer getValueUnboxer() {
* Helps you build a GraphQL object ready to execute queries
*
* @param graphQLSchema the schema to use
*
* @return a builder of GraphQL objects
*/
public static Builder newGraphQL(GraphQLSchema graphQLSchema) {
Expand All @@ -181,18 +183,17 @@ public static Builder newGraphQL(GraphQLSchema graphQLSchema) {
* the current values and allows you to transform it how you want.
*
* @param builderConsumer the consumer code that will be given a builder to transform
*
* @return a new GraphQL object based on calling build on that builder
*/
public GraphQL transform(Consumer<GraphQL.Builder> builderConsumer) {
Builder builder = new Builder(this.graphQLSchema);
builder
.queryExecutionStrategy(this.queryStrategy)
.mutationExecutionStrategy(this.mutationStrategy)
.subscriptionExecutionStrategy(this.subscriptionStrategy)
.executionIdProvider(Optional.ofNullable(this.idProvider).orElse(builder.idProvider))
.instrumentation(Optional.ofNullable(this.instrumentation).orElse(builder.instrumentation))
.preparsedDocumentProvider(Optional.ofNullable(this.preparsedDocumentProvider).orElse(builder.preparsedDocumentProvider));
.queryExecutionStrategy(this.queryStrategy)
.mutationExecutionStrategy(this.mutationStrategy)
.subscriptionExecutionStrategy(this.subscriptionStrategy)
.executionIdProvider(Optional.ofNullable(this.idProvider).orElse(builder.idProvider))
.instrumentation(Optional.ofNullable(this.instrumentation).orElse(builder.instrumentation))
.preparsedDocumentProvider(Optional.ofNullable(this.preparsedDocumentProvider).orElse(builder.preparsedDocumentProvider));

builderConsumer.accept(builder);

Expand All @@ -210,6 +211,7 @@ public static class Builder {
private Instrumentation instrumentation = null; // deliberate default here
private PreparsedDocumentProvider preparsedDocumentProvider = NoOpPreparsedDocumentProvider.INSTANCE;
private boolean doNotAddDefaultInstrumentations = false;
private boolean doNotAutomaticallyDispatchDataLoader = false;
private ValueUnboxer valueUnboxer = ValueUnboxer.DEFAULT;


Expand Down Expand Up @@ -242,7 +244,6 @@ public Builder subscriptionExecutionStrategy(ExecutionStrategy executionStrategy
* in {@link graphql.schema.DataFetcher} invocations.
*
* @param dataFetcherExceptionHandler the default handler for data fetching exception
*
* @return this builder
*/
public Builder defaultDataFetcherExceptionHandler(DataFetcherExceptionHandler dataFetcherExceptionHandler) {
Expand All @@ -266,19 +267,28 @@ public Builder executionIdProvider(ExecutionIdProvider executionIdProvider) {
}

/**
* For performance reasons you can opt into situation where the default instrumentations (such
* as {@link graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation} will not be
* automatically added into the graphql instance.
* <p>
* For most situations this is not needed unless you are really pushing the boundaries of performance
* As of GraphQL Java 22 there are no default instrumentations. Before that a DataLoaderDispatcherInstrumentation
* was added by default.
* <p>
* By default a certain graphql instrumentations will be added to the mix to more easily enable certain functionality. This
* allows you to stop this behavior
* For GraphQL Java 22 calling this method is equal to calling {@link #doNotAutomaticallyDispatchDataLoader()}
*
* @return this builder
*/
public Builder doNotAddDefaultInstrumentations() {
andimarek marked this conversation as resolved.
Show resolved Hide resolved
this.doNotAddDefaultInstrumentations = true;
// legacy reasons: calling this method is equal to calling doNotAutomaticallyDispatchDataLoader
this.doNotAutomaticallyDispatchDataLoader = true;
return this;
}

/**
* Deactivates the automatic dispatching of DataLoaders.
* If deactivated the user is responsible for dispatching the DataLoaders manually.
*
* @return this builder
*/
public Builder doNotAutomaticallyDispatchDataLoader() {
this.doNotAutomaticallyDispatchDataLoader = true;
return this;
}

Expand Down Expand Up @@ -308,13 +318,12 @@ public GraphQL build() {
* Executes the specified graphql query/mutation/subscription
*
* @param query the query/mutation/subscription
*
* @return an {@link ExecutionResult} which can include errors
*/
public ExecutionResult execute(String query) {
ExecutionInput executionInput = ExecutionInput.newExecutionInput()
.query(query)
.build();
.query(query)
.build();
return execute(executionInput);
}

Expand All @@ -323,7 +332,6 @@ public ExecutionResult execute(String query) {
* Executes the graphql query using the provided input object builder
*
* @param executionInputBuilder {@link ExecutionInput.Builder}
*
* @return an {@link ExecutionResult} which can include errors
*/
public ExecutionResult execute(ExecutionInput.Builder executionInputBuilder) {
Expand All @@ -341,7 +349,6 @@ public ExecutionResult execute(ExecutionInput.Builder executionInputBuilder) {
* </pre>
*
* @param builderFunction a function that is given a {@link ExecutionInput.Builder}
*
* @return an {@link ExecutionResult} which can include errors
*/
public ExecutionResult execute(UnaryOperator<ExecutionInput.Builder> builderFunction) {
Expand All @@ -352,7 +359,6 @@ public ExecutionResult execute(UnaryOperator<ExecutionInput.Builder> builderFunc
* Executes the graphql query using the provided input object
*
* @param executionInput {@link ExecutionInput}
*
* @return an {@link ExecutionResult} which can include errors
*/
public ExecutionResult execute(ExecutionInput executionInput) {
Expand All @@ -374,7 +380,6 @@ public ExecutionResult execute(ExecutionInput executionInput) {
* which is the result of executing the provided query.
*
* @param executionInputBuilder {@link ExecutionInput.Builder}
*
* @return a promise to an {@link ExecutionResult} which can include errors
*/
public CompletableFuture<ExecutionResult> executeAsync(ExecutionInput.Builder executionInputBuilder) {
Expand All @@ -395,7 +400,6 @@ public CompletableFuture<ExecutionResult> executeAsync(ExecutionInput.Builder ex
* </pre>
*
* @param builderFunction a function that is given a {@link ExecutionInput.Builder}
*
* @return a promise to an {@link ExecutionResult} which can include errors
*/
public CompletableFuture<ExecutionResult> executeAsync(UnaryOperator<ExecutionInput.Builder> builderFunction) {
Expand All @@ -409,7 +413,6 @@ public CompletableFuture<ExecutionResult> executeAsync(UnaryOperator<ExecutionIn
* which is the result of executing the provided query.
*
* @param executionInput {@link ExecutionInput}
*
* @return a promise to an {@link ExecutionResult} which can include errors
*/
public CompletableFuture<ExecutionResult> executeAsync(ExecutionInput executionInput) {
Expand Down Expand Up @@ -522,7 +525,7 @@ private ParseAndValidateResult parse(ExecutionInput executionInput, GraphQLSchem
DocumentAndVariables documentAndVariables = parseResult.getDocumentAndVariables();
documentAndVariables = instrumentation.instrumentDocumentAndVariables(documentAndVariables, parameters, instrumentationState);
return ParseAndValidateResult.newResult()
.document(documentAndVariables.getDocument()).variables(documentAndVariables.getVariables()).build();
.document(documentAndVariables.getDocument()).variables(documentAndVariables.getVariables()).build();
}
}

Expand All @@ -540,9 +543,13 @@ private List<ValidationError> validate(ExecutionInput executionInput, Document d
return validationErrors;
}

private CompletableFuture<ExecutionResult> execute(ExecutionInput executionInput, Document document, GraphQLSchema graphQLSchema, InstrumentationState instrumentationState) {
private CompletableFuture<ExecutionResult> execute(ExecutionInput executionInput,
Document document,
GraphQLSchema graphQLSchema,
InstrumentationState instrumentationState
) {

Execution execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation, valueUnboxer);
Execution execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation, valueUnboxer, doNotAutomaticallyDispatchDataLoader);
ExecutionId executionId = executionInput.getExecutionId();

return execution.execute(document, graphQLSchema, executionId, executionInput, instrumentationState);
Expand All @@ -552,30 +559,12 @@ private static Instrumentation checkInstrumentationDefaultState(Instrumentation
if (doNotAddDefaultInstrumentations) {
return instrumentation == null ? SimplePerformantInstrumentation.INSTANCE : instrumentation;
}
if (instrumentation instanceof DataLoaderDispatcherInstrumentation) {
return instrumentation;
}
if (instrumentation instanceof NoContextChainedInstrumentation) {
return instrumentation;
}
if (instrumentation == null) {
return new DataLoaderDispatcherInstrumentation();
return SimplePerformantInstrumentation.INSTANCE;
}

//
// if we don't have a DataLoaderDispatcherInstrumentation in play, we add one. We want DataLoader to be 1st class in graphql without requiring
// people to remember to wire it in. Later we may decide to have more default instrumentations but for now it's just the one
//
List<Instrumentation> instrumentationList = new ArrayList<>();
if (instrumentation instanceof ChainedInstrumentation) {
instrumentationList.addAll(((ChainedInstrumentation) instrumentation).getInstrumentations());
} else {
instrumentationList.add(instrumentation);
}
boolean containsDLInstrumentation = instrumentationList.stream().anyMatch(instr -> instr instanceof DataLoaderDispatcherInstrumentation);
if (!containsDLInstrumentation) {
instrumentationList.add(new DataLoaderDispatcherInstrumentation());
}
return new ChainedInstrumentation(instrumentationList);
return instrumentation;
}
}
3 changes: 3 additions & 0 deletions src/main/java/graphql/execution/AsyncExecutionStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public AsyncExecutionStrategy(DataFetcherExceptionHandler exceptionHandler) {
@Override
@SuppressWarnings("FutureReturnValueIgnored")
public CompletableFuture<ExecutionResult> execute(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException {
executionContext.getDataLoaderDispatcherStrategy().executionStrategy(executionContext, parameters);
Instrumentation instrumentation = executionContext.getInstrumentation();
andimarek marked this conversation as resolved.
Show resolved Hide resolved
InstrumentationExecutionStrategyParameters instrumentationParameters = new InstrumentationExecutionStrategyParameters(executionContext, parameters);

Expand Down Expand Up @@ -63,12 +64,14 @@ public CompletableFuture<ExecutionResult> execute(ExecutionContext executionCont
for (FieldValueInfo completeValueInfo : completeValueInfos) {
fieldValuesFutures.add(completeValueInfo.getFieldValueFuture());
}
executionContext.getDataLoaderDispatcherStrategy().executionStrategy_onFieldValuesInfo(completeValueInfos, parameters);
andimarek marked this conversation as resolved.
Show resolved Hide resolved
executionStrategyCtx.onFieldValuesInfo(completeValueInfos);
fieldValuesFutures.await().whenComplete(handleResultsConsumer);
}).exceptionally((ex) -> {
// if there are any issues with combining/handling the field results,
// complete the future at all costs and bubble up any thrown exception so
// the execution does not hang.
executionContext.getDataLoaderDispatcherStrategy().executionStrategy_onFieldValuesException(ex, parameters);
executionStrategyCtx.onFieldValuesException();
overallResult.completeExceptionally(ex);
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ public AsyncSerialExecutionStrategy(DataFetcherExceptionHandler exceptionHandler
@Override
@SuppressWarnings({"TypeParameterUnusedInFormals", "FutureReturnValueIgnored"})
public CompletableFuture<ExecutionResult> execute(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException {
executionContext.getDataLoaderDispatcherStrategy().executionStrategy(executionContext, parameters);

Instrumentation instrumentation = executionContext.getInstrumentation();
InstrumentationExecutionStrategyParameters instrumentationParameters = new InstrumentationExecutionStrategyParameters(executionContext, parameters);
InstrumentationContext<ExecutionResult> executionStrategyCtx = nonNullCtx(instrumentation.beginExecutionStrategy(instrumentationParameters,
executionContext.getInstrumentationState())
executionContext.getInstrumentationState())
);
MergedSelectionSet fields = parameters.getFields();
ImmutableList<String> fieldNames = ImmutableList.copyOf(fields.keySet());
Expand All @@ -43,7 +44,7 @@ public CompletableFuture<ExecutionResult> execute(ExecutionContext executionCont
MergedField currentField = fields.getSubField(fieldName);
ResultPath fieldPath = parameters.getPath().segment(mkNameForPath(currentField));
ExecutionStrategyParameters newParameters = parameters
.transform(builder -> builder.field(currentField).path(fieldPath));
.transform(builder -> builder.field(currentField).path(fieldPath));
return resolveField(executionContext, newParameters);
});

Expand Down
56 changes: 56 additions & 0 deletions src/main/java/graphql/execution/DataLoaderDispatchStrategy.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package graphql.execution;

import graphql.Internal;
import graphql.schema.DataFetcher;

import java.util.List;
import java.util.concurrent.CompletableFuture;

@Internal
public interface DataLoaderDispatchStrategy {

DataLoaderDispatchStrategy NO_OP = new DataLoaderDispatchStrategy() {
};


default void executionStrategy(ExecutionContext executionContext, ExecutionStrategyParameters parameters) {

}

default void executionStrategy_onFieldValuesInfo(List<FieldValueInfo> fieldValueInfoList, ExecutionStrategyParameters parameters) {
andimarek marked this conversation as resolved.
Show resolved Hide resolved

}

default void executionStrategy_onFieldValuesException(Throwable t, ExecutionStrategyParameters parameters) {

}


default void executeObject(ExecutionContext executionContext, ExecutionStrategyParameters executionStrategyParameters) {

}

default void executeObject_onFieldValuesInfo(List<FieldValueInfo> fieldValueInfoList, ExecutionStrategyParameters parameters) {
andimarek marked this conversation as resolved.
Show resolved Hide resolved

}

default void executeObject_onFieldValuesException(Throwable t, ExecutionStrategyParameters parameters) {

}

default void fieldFetched(ExecutionContext executionContext,
ExecutionStrategyParameters executionStrategyParameters,
DataFetcher<?> dataFetcher,
CompletableFuture<Object> fetchedValue) {

}


default DataFetcher<?> modifyDataFetcher(DataFetcher<?> dataFetcher) {
return dataFetcher;
}

default void deferredField(ExecutionContext executionContext, MergedField currentField) {

}
}