Skip to content

Commit 81cfc0e

Browse files
bbakermanbbakerman
committed
Merge remote-tracking branch 'origin/master' into git-rid-of-er-wrapping
# Conflicts: # src/main/java/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentation.java # src/test/groovy/graphql/GraphQLTest.groovy
2 parents b0b3b6d + 4cfbb35 commit 81cfc0e

35 files changed

+490
-843
lines changed

‎src/main/java/graphql/ExecutionInput.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import graphql.collect.ImmutableKit;
44
import graphql.execution.ExecutionId;
55
import graphql.execution.RawVariables;
6-
import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentationState;
76
import org.dataloader.DataLoaderRegistry;
87

98
import java.util.Locale;
@@ -12,6 +11,7 @@
1211
import java.util.function.UnaryOperator;
1312

1413
import static graphql.Assert.assertNotNull;
14+
import static graphql.execution.instrumentation.dataloader.EmptyDataLoaderRegistryInstance.EMPTY_DATALOADER_REGISTRY;
1515

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

‎src/main/java/graphql/GraphQL.java‎

Lines changed: 21 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,11 @@
1212
import graphql.execution.SimpleDataFetcherExceptionHandler;
1313
import graphql.execution.SubscriptionExecutionStrategy;
1414
import graphql.execution.ValueUnboxer;
15-
import graphql.execution.instrumentation.ChainedInstrumentation;
1615
import graphql.execution.instrumentation.DocumentAndVariables;
1716
import graphql.execution.instrumentation.Instrumentation;
1817
import graphql.execution.instrumentation.InstrumentationContext;
1918
import graphql.execution.instrumentation.InstrumentationState;
20-
import graphql.execution.instrumentation.NoContextChainedInstrumentation;
2119
import graphql.execution.instrumentation.SimplePerformantInstrumentation;
22-
import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation;
2320
import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters;
2421
import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters;
2522
import graphql.execution.instrumentation.parameters.InstrumentationValidationParameters;
@@ -30,7 +27,6 @@
3027
import graphql.schema.GraphQLSchema;
3128
import graphql.validation.ValidationError;
3229

33-
import java.util.ArrayList;
3430
import java.util.List;
3531
import java.util.Locale;
3632
import java.util.Optional;
@@ -96,6 +92,7 @@ public class GraphQL {
9692
private final Instrumentation instrumentation;
9793
private final PreparsedDocumentProvider preparsedDocumentProvider;
9894
private final ValueUnboxer valueUnboxer;
95+
private final boolean doNotAutomaticallyDispatchDataLoader;
9996

10097

10198
private GraphQL(Builder builder) {
@@ -107,6 +104,7 @@ private GraphQL(Builder builder) {
107104
this.instrumentation = assertNotNull(builder.instrumentation, () -> "instrumentation must not be null");
108105
this.preparsedDocumentProvider = assertNotNull(builder.preparsedDocumentProvider, () -> "preparsedDocumentProvider must be non null");
109106
this.valueUnboxer = assertNotNull(builder.valueUnboxer, () -> "valueUnboxer must not be null");
107+
this.doNotAutomaticallyDispatchDataLoader = builder.doNotAutomaticallyDispatchDataLoader;
110108
}
111109

112110
/**
@@ -151,6 +149,10 @@ public Instrumentation getInstrumentation() {
151149
return instrumentation;
152150
}
153151

152+
public boolean isDoNotAutomaticallyDispatchDataLoader() {
153+
return doNotAutomaticallyDispatchDataLoader;
154+
}
155+
154156
/**
155157
* @return the PreparsedDocumentProvider for this {@link GraphQL} instance
156158
*/
@@ -209,7 +211,7 @@ public static class Builder {
209211
private ExecutionIdProvider idProvider = DEFAULT_EXECUTION_ID_PROVIDER;
210212
private Instrumentation instrumentation = null; // deliberate default here
211213
private PreparsedDocumentProvider preparsedDocumentProvider = NoOpPreparsedDocumentProvider.INSTANCE;
212-
private boolean doNotAddDefaultInstrumentations = false;
214+
private boolean doNotAutomaticallyDispatchDataLoader = false;
213215
private ValueUnboxer valueUnboxer = ValueUnboxer.DEFAULT;
214216

215217

@@ -265,20 +267,15 @@ public Builder executionIdProvider(ExecutionIdProvider executionIdProvider) {
265267
return this;
266268
}
267269

270+
268271
/**
269-
* For performance reasons you can opt into situation where the default instrumentations (such
270-
* as {@link graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation} will not be
271-
* automatically added into the graphql instance.
272-
* <p>
273-
* For most situations this is not needed unless you are really pushing the boundaries of performance
274-
* <p>
275-
* By default a certain graphql instrumentations will be added to the mix to more easily enable certain functionality. This
276-
* allows you to stop this behavior
272+
* Deactivates the automatic dispatching of DataLoaders.
273+
* If deactivated the user is responsible for dispatching the DataLoaders manually.
277274
*
278275
* @return this builder
279276
*/
280-
public Builder doNotAddDefaultInstrumentations() {
281-
this.doNotAddDefaultInstrumentations = true;
277+
public Builder doNotAutomaticallyDispatchDataLoader() {
278+
this.doNotAutomaticallyDispatchDataLoader = true;
282279
return this;
283280
}
284281

@@ -299,7 +296,9 @@ public GraphQL build() {
299296
this.subscriptionExecutionStrategy = new SubscriptionExecutionStrategy(this.defaultExceptionHandler);
300297
}
301298

302-
this.instrumentation = checkInstrumentationDefaultState(this.instrumentation, this.doNotAddDefaultInstrumentations);
299+
if (instrumentation == null) {
300+
this.instrumentation = SimplePerformantInstrumentation.INSTANCE;
301+
}
303302
return new GraphQL(this);
304303
}
305304
}
@@ -540,42 +539,16 @@ private List<ValidationError> validate(ExecutionInput executionInput, Document d
540539
return validationErrors;
541540
}
542541

543-
private CompletableFuture<ExecutionResult> execute(ExecutionInput executionInput, Document document, GraphQLSchema graphQLSchema, InstrumentationState instrumentationState) {
542+
private CompletableFuture<ExecutionResult> execute(ExecutionInput executionInput,
543+
Document document,
544+
GraphQLSchema graphQLSchema,
545+
InstrumentationState instrumentationState
546+
) {
544547

545-
Execution execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation, valueUnboxer);
548+
Execution execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation, valueUnboxer, doNotAutomaticallyDispatchDataLoader);
546549
ExecutionId executionId = executionInput.getExecutionId();
547550

548551
return execution.execute(document, graphQLSchema, executionId, executionInput, instrumentationState);
549552
}
550553

551-
private static Instrumentation checkInstrumentationDefaultState(Instrumentation instrumentation, boolean doNotAddDefaultInstrumentations) {
552-
if (doNotAddDefaultInstrumentations) {
553-
return instrumentation == null ? SimplePerformantInstrumentation.INSTANCE : instrumentation;
554-
}
555-
if (instrumentation instanceof DataLoaderDispatcherInstrumentation) {
556-
return instrumentation;
557-
}
558-
if (instrumentation instanceof NoContextChainedInstrumentation) {
559-
return instrumentation;
560-
}
561-
if (instrumentation == null) {
562-
return new DataLoaderDispatcherInstrumentation();
563-
}
564-
565-
//
566-
// if we don't have a DataLoaderDispatcherInstrumentation in play, we add one. We want DataLoader to be 1st class in graphql without requiring
567-
// people to remember to wire it in. Later we may decide to have more default instrumentations but for now it's just the one
568-
//
569-
List<Instrumentation> instrumentationList = new ArrayList<>();
570-
if (instrumentation instanceof ChainedInstrumentation) {
571-
instrumentationList.addAll(((ChainedInstrumentation) instrumentation).getInstrumentations());
572-
} else {
573-
instrumentationList.add(instrumentation);
574-
}
575-
boolean containsDLInstrumentation = instrumentationList.stream().anyMatch(instr -> instr instanceof DataLoaderDispatcherInstrumentation);
576-
if (!containsDLInstrumentation) {
577-
instrumentationList.add(new DataLoaderDispatcherInstrumentation());
578-
}
579-
return new ChainedInstrumentation(instrumentationList);
580-
}
581554
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ public AsyncExecutionStrategy(DataFetcherExceptionHandler exceptionHandler) {
3636
@Override
3737
@SuppressWarnings("FutureReturnValueIgnored")
3838
public CompletableFuture<ExecutionResult> execute(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException {
39+
DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = executionContext.getDataLoaderDispatcherStrategy();
40+
dataLoaderDispatcherStrategy.executionStrategy(executionContext, parameters);
3941
Instrumentation instrumentation = executionContext.getInstrumentation();
4042
InstrumentationExecutionStrategyParameters instrumentationParameters = new InstrumentationExecutionStrategyParameters(executionContext, parameters);
4143

@@ -63,12 +65,14 @@ public CompletableFuture<ExecutionResult> execute(ExecutionContext executionCont
6365
for (FieldValueInfo completeValueInfo : completeValueInfos) {
6466
fieldValuesFutures.add(completeValueInfo.getFieldValueFuture());
6567
}
68+
dataLoaderDispatcherStrategy.executionStrategyOnFieldValuesInfo(completeValueInfos, parameters);
6669
executionStrategyCtx.onFieldValuesInfo(completeValueInfos);
6770
fieldValuesFutures.await().whenComplete(handleResultsConsumer);
6871
}).exceptionally((ex) -> {
6972
// if there are any issues with combining/handling the field results,
7073
// complete the future at all costs and bubble up any thrown exception so
7174
// the execution does not hang.
75+
dataLoaderDispatcherStrategy.executionStrategyOnFieldValuesException(ex, parameters);
7276
executionStrategyCtx.onFieldValuesException();
7377
overallResult.completeExceptionally(ex);
7478
return null;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ public AsyncSerialExecutionStrategy(DataFetcherExceptionHandler exceptionHandler
3030
@Override
3131
@SuppressWarnings({"TypeParameterUnusedInFormals", "FutureReturnValueIgnored"})
3232
public CompletableFuture<ExecutionResult> execute(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException {
33+
executionContext.getDataLoaderDispatcherStrategy().executionStrategy(executionContext, parameters);
3334

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

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package graphql.execution;
2+
3+
import graphql.Internal;
4+
import graphql.schema.DataFetcher;
5+
6+
import java.util.List;
7+
import java.util.concurrent.CompletableFuture;
8+
9+
@Internal
10+
public interface DataLoaderDispatchStrategy {
11+
12+
DataLoaderDispatchStrategy NO_OP = new DataLoaderDispatchStrategy() {
13+
};
14+
15+
16+
default void executionStrategy(ExecutionContext executionContext, ExecutionStrategyParameters parameters) {
17+
18+
}
19+
20+
default void executionStrategyOnFieldValuesInfo(List<FieldValueInfo> fieldValueInfoList, ExecutionStrategyParameters parameters) {
21+
22+
}
23+
24+
default void executionStrategyOnFieldValuesException(Throwable t, ExecutionStrategyParameters parameters) {
25+
26+
}
27+
28+
29+
default void executeObject(ExecutionContext executionContext, ExecutionStrategyParameters executionStrategyParameters) {
30+
31+
}
32+
33+
default void executeObjectOnFieldValuesInfo(List<FieldValueInfo> fieldValueInfoList, ExecutionStrategyParameters parameters) {
34+
35+
}
36+
37+
default void executeObjectOnFieldValuesException(Throwable t, ExecutionStrategyParameters parameters) {
38+
39+
}
40+
41+
default void fieldFetched(ExecutionContext executionContext,
42+
ExecutionStrategyParameters executionStrategyParameters,
43+
DataFetcher<?> dataFetcher,
44+
CompletableFuture<Object> fetchedValue) {
45+
46+
}
47+
48+
49+
default DataFetcher<?> modifyDataFetcher(DataFetcher<?> dataFetcher) {
50+
return dataFetcher;
51+
}
52+
53+
default void deferredField(ExecutionContext executionContext, MergedField currentField) {
54+
55+
}
56+
}

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

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
import graphql.execution.instrumentation.Instrumentation;
1313
import graphql.execution.instrumentation.InstrumentationContext;
1414
import graphql.execution.instrumentation.InstrumentationState;
15+
import graphql.execution.instrumentation.dataloader.FallbackDataLoaderDispatchStrategy;
16+
import graphql.execution.instrumentation.dataloader.PerLevelDataLoaderDispatchStrategy;
1517
import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters;
1618
import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters;
1719
import graphql.extensions.ExtensionsBuilder;
@@ -37,6 +39,7 @@
3739
import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo;
3840
import static graphql.execution.ExecutionStrategyParameters.newParameters;
3941
import static graphql.execution.instrumentation.SimpleInstrumentationContext.nonNullCtx;
42+
import static graphql.execution.instrumentation.dataloader.EmptyDataLoaderRegistryInstance.EMPTY_DATALOADER_REGISTRY;
4043
import static java.util.concurrent.CompletableFuture.completedFuture;
4144

4245
@Internal
@@ -47,13 +50,20 @@ public class Execution {
4750
private final ExecutionStrategy subscriptionStrategy;
4851
private final Instrumentation instrumentation;
4952
private final ValueUnboxer valueUnboxer;
50-
51-
public Execution(ExecutionStrategy queryStrategy, ExecutionStrategy mutationStrategy, ExecutionStrategy subscriptionStrategy, Instrumentation instrumentation, ValueUnboxer valueUnboxer) {
53+
private final boolean doNotAutomaticallyDispatchDataLoader;
54+
55+
public Execution(ExecutionStrategy queryStrategy,
56+
ExecutionStrategy mutationStrategy,
57+
ExecutionStrategy subscriptionStrategy,
58+
Instrumentation instrumentation,
59+
ValueUnboxer valueUnboxer,
60+
boolean doNotAutomaticallyDispatchDataLoader) {
5261
this.queryStrategy = queryStrategy != null ? queryStrategy : new AsyncExecutionStrategy();
5362
this.mutationStrategy = mutationStrategy != null ? mutationStrategy : new AsyncSerialExecutionStrategy();
5463
this.subscriptionStrategy = subscriptionStrategy != null ? subscriptionStrategy : new AsyncExecutionStrategy();
5564
this.instrumentation = instrumentation;
5665
this.valueUnboxer = valueUnboxer;
66+
this.doNotAutomaticallyDispatchDataLoader = doNotAutomaticallyDispatchDataLoader;
5767
}
5868

5969
public CompletableFuture<ExecutionResult> execute(Document document, GraphQLSchema graphQLSchema, ExecutionId executionId, ExecutionInput executionInput, InstrumentationState instrumentationState) {
@@ -160,9 +170,12 @@ private CompletableFuture<ExecutionResult> executeOperation(ExecutionContext exe
160170
.path(path)
161171
.build();
162172

173+
163174
CompletableFuture<ExecutionResult> result;
164175
try {
165176
ExecutionStrategy executionStrategy = executionContext.getStrategy(operation);
177+
DataLoaderDispatchStrategy dataLoaderDispatchStrategy = createDataLoaderDispatchStrategy(executionContext, executionStrategy);
178+
executionContext.setDataLoaderDispatcherStrategy(dataLoaderDispatchStrategy);
166179
result = executionStrategy.execute(executionContext, parameters);
167180
} catch (NonNullableFieldWasNullException e) {
168181
// this means it was non-null types all the way from an offending non-null type
@@ -209,6 +222,18 @@ private CompletableFuture<ExecutionResult> incrementalSupport(ExecutionContext e
209222
});
210223
}
211224

225+
private DataLoaderDispatchStrategy createDataLoaderDispatchStrategy(ExecutionContext executionContext, ExecutionStrategy executionStrategy) {
226+
if (executionContext.getDataLoaderRegistry() == EMPTY_DATALOADER_REGISTRY || doNotAutomaticallyDispatchDataLoader) {
227+
return DataLoaderDispatchStrategy.NO_OP;
228+
}
229+
if (executionStrategy instanceof AsyncExecutionStrategy) {
230+
return new PerLevelDataLoaderDispatchStrategy(executionContext);
231+
} else {
232+
return new FallbackDataLoaderDispatchStrategy(executionContext);
233+
}
234+
}
235+
236+
212237
private void addExtensionsBuilderNotPresent(GraphQLContext graphQLContext) {
213238
Object builder = graphQLContext.get(ExtensionsBuilder.class);
214239
if (builder == null) {

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import graphql.ExecutionInput;
77
import graphql.GraphQLContext;
88
import graphql.GraphQLError;
9+
import graphql.Internal;
910
import graphql.PublicApi;
1011
import graphql.collect.ImmutableKit;
1112
import graphql.execution.incremental.IncrementalCallState;
@@ -59,6 +60,9 @@ public class ExecutionContext {
5960
private final ExecutionInput executionInput;
6061
private final Supplier<ExecutableNormalizedOperation> queryTree;
6162

63+
// this is modified after creation so it needs to be volatile to ensure visibility across Threads
64+
private volatile DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP;
65+
6266
ExecutionContext(ExecutionContextBuilder builder) {
6367
this.graphQLSchema = builder.graphQLSchema;
6468
this.executionId = builder.executionId;
@@ -247,7 +251,9 @@ public List<GraphQLError> getErrors() {
247251
return errors.get();
248252
}
249253

250-
public ExecutionStrategy getQueryStrategy() { return queryStrategy; }
254+
public ExecutionStrategy getQueryStrategy() {
255+
return queryStrategy;
256+
}
251257

252258
public ExecutionStrategy getMutationStrategy() {
253259
return mutationStrategy;
@@ -275,6 +281,16 @@ public Supplier<ExecutableNormalizedOperation> getNormalizedQueryTree() {
275281
return queryTree;
276282
}
277283

284+
@Internal
285+
public void setDataLoaderDispatcherStrategy(DataLoaderDispatchStrategy dataLoaderDispatcherStrategy) {
286+
this.dataLoaderDispatcherStrategy = dataLoaderDispatcherStrategy;
287+
}
288+
289+
@Internal
290+
public DataLoaderDispatchStrategy getDataLoaderDispatcherStrategy() {
291+
return dataLoaderDispatcherStrategy;
292+
}
293+
278294
/**
279295
* This helps you transform the current ExecutionContext object into another one by starting a builder with all
280296
* the current values and allows you to transform it how you want.

0 commit comments

Comments
 (0)