Skip to content

Commit f796ab7

Browse files
andimarekandimarek
committed
fix tests, make dataloader strategy per execution
1 parent 2017326 commit f796ab7

8 files changed

Lines changed: 40 additions & 39 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public AsyncExecutionStrategy(DataFetcherExceptionHandler exceptionHandler) {
3636
@Override
3737
@SuppressWarnings("FutureReturnValueIgnored")
3838
public CompletableFuture<ExecutionResult> execute(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException {
39-
dataLoaderDispatcherStrategy.executionStrategy(executionContext, parameters);
39+
executionContext.getDataLoaderDispatcherStrategy().executionStrategy(executionContext, parameters);
4040

4141
Instrumentation instrumentation = executionContext.getInstrumentation();
4242
InstrumentationExecutionStrategyParameters instrumentationParameters = new InstrumentationExecutionStrategyParameters(executionContext, parameters);
@@ -59,14 +59,14 @@ public CompletableFuture<ExecutionResult> execute(ExecutionContext executionCont
5959
for (FieldValueInfo completeValueInfo : completeValueInfos) {
6060
fieldValuesFutures.add(completeValueInfo.getFieldValueFuture());
6161
}
62-
dataLoaderDispatcherStrategy.executionStrategy_onFieldValuesInfo(completeValueInfos, parameters);
62+
executionContext.getDataLoaderDispatcherStrategy().executionStrategy_onFieldValuesInfo(completeValueInfos, parameters);
6363
executionStrategyCtx.onFieldValuesInfo(completeValueInfos);
6464
fieldValuesFutures.await().whenComplete(handleResultsConsumer);
6565
}).exceptionally((ex) -> {
6666
// if there are any issues with combining/handling the field results,
6767
// complete the future at all costs and bubble up any thrown exception so
6868
// the execution does not hang.
69-
dataLoaderDispatcherStrategy.executeObject_onFieldValuesException(ex, parameters);
69+
executionContext.getDataLoaderDispatcherStrategy().executionStrategy_onFieldValuesException(ex, parameters);
7070
executionStrategyCtx.onFieldValuesException();
7171
overallResult.completeExceptionally(ex);
7272
return null;

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

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

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

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,7 @@ private CompletableFuture<ExecutionResult> executeOperation(ExecutionContext exe
163163
try {
164164
ExecutionStrategy executionStrategy = executionContext.getStrategy(operation);
165165
DataLoaderDispatchStrategy dataLoaderDispatchStrategy = createDataLoaderDispatchStrategy(executionContext, executionStrategy);
166-
executionContext.getQueryStrategy().setDataLoaderDispatcherStrategy(dataLoaderDispatchStrategy);
167-
executionContext.getMutationStrategy().setDataLoaderDispatcherStrategy(dataLoaderDispatchStrategy);
168-
executionContext.getSubscriptionStrategy().setDataLoaderDispatcherStrategy(dataLoaderDispatchStrategy);
169-
166+
executionContext.setDataLoaderDispatcherStrategy(dataLoaderDispatchStrategy);
170167
result = executionStrategy.execute(executionContext, parameters);
171168
} catch (NonNullableFieldWasNullException e) {
172169
// this means it was non-null types all the way from an offending non-null type

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

Lines changed: 13 additions & 0 deletions
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.instrumentation.Instrumentation;
@@ -57,6 +58,8 @@ public class ExecutionContext {
5758
private final ExecutionInput executionInput;
5859
private final Supplier<ExecutableNormalizedOperation> queryTree;
5960

61+
private final AtomicReference<DataLoaderDispatchStrategy> dataLoaderDispatcherStrategy = new AtomicReference<>(DataLoaderDispatchStrategy.NO_OP);
62+
6063
ExecutionContext(ExecutionContextBuilder builder) {
6164
this.graphQLSchema = builder.graphQLSchema;
6265
this.executionId = builder.executionId;
@@ -269,6 +272,16 @@ public Supplier<ExecutableNormalizedOperation> getNormalizedQueryTree() {
269272
return queryTree;
270273
}
271274

275+
@Internal
276+
public void setDataLoaderDispatcherStrategy(DataLoaderDispatchStrategy dataLoaderDispatcherStrategy) {
277+
this.dataLoaderDispatcherStrategy.set(dataLoaderDispatcherStrategy);
278+
}
279+
280+
@Internal
281+
public DataLoaderDispatchStrategy getDataLoaderDispatcherStrategy() {
282+
return dataLoaderDispatcherStrategy.get();
283+
}
284+
272285
/**
273286
* This helps you transform the current ExecutionContext object into another one by starting a builder with all
274287
* the current values and allows you to transform it how you want.

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

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@ public abstract class ExecutionStrategy {
133133
protected final DataFetcherExceptionHandler dataFetcherExceptionHandler;
134134
private final ResolveType resolvedType = new ResolveType();
135135

136-
protected DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP;
137136

138137
/**
139138
* The default execution strategy constructor uses the {@link SimpleDataFetcherExceptionHandler}
@@ -155,12 +154,6 @@ protected ExecutionStrategy(DataFetcherExceptionHandler dataFetcherExceptionHand
155154
}
156155

157156

158-
@Internal
159-
void setDataLoaderDispatcherStrategy(DataLoaderDispatchStrategy dataLoaderDispatcherStrategy) {
160-
this.dataLoaderDispatcherStrategy = dataLoaderDispatcherStrategy;
161-
}
162-
163-
164157

165158
@Internal
166159
public static String mkNameForPath(Field currentField) {
@@ -197,7 +190,7 @@ public static String mkNameForPath(List<Field> currentField) {
197190
* @throws NonNullableFieldWasNullException in the future if a non-null field resolves to a null value
198191
*/
199192
protected CompletableFuture<Map<String, Object>> executeObject(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException {
200-
dataLoaderDispatcherStrategy.executeObject(executionContext, parameters);
193+
executionContext.getDataLoaderDispatcherStrategy().executeObject(executionContext, parameters);
201194
Instrumentation instrumentation = executionContext.getInstrumentation();
202195
InstrumentationExecutionStrategyParameters instrumentationParameters = new InstrumentationExecutionStrategyParameters(executionContext, parameters);
203196

@@ -221,14 +214,14 @@ protected CompletableFuture<Map<String, Object>> executeObject(ExecutionContext
221214
for (FieldValueInfo completeValueInfo : completeValueInfos) {
222215
resultFutures.add(completeValueInfo.getFieldValueFuture());
223216
}
224-
dataLoaderDispatcherStrategy.executeObject_onFieldValuesInfo(completeValueInfos, parameters);
217+
executionContext.getDataLoaderDispatcherStrategy().executeObject_onFieldValuesInfo(completeValueInfos, parameters);
225218
resolveObjectCtx.onFieldValuesInfo(completeValueInfos);
226219
resultFutures.await().whenComplete(handleResultsConsumer);
227220
}).exceptionally((ex) -> {
228221
// if there are any issues with combining/handling the field results,
229222
// complete the future at all costs and bubble up any thrown exception so
230223
// the execution does not hang.
231-
dataLoaderDispatcherStrategy.executeObject_onFieldValuesException(ex, parameters);
224+
executionContext.getDataLoaderDispatcherStrategy().executeObject_onFieldValuesException(ex, parameters);
232225
resolveObjectCtx.onFieldValuesException();
233226
overallResult.completeExceptionally(ex);
234227
return null;
@@ -383,10 +376,10 @@ protected CompletableFuture<FetchedValue> fetchField(ExecutionContext executionC
383376
executionContext.getInstrumentationState())
384377
);
385378

386-
dataFetcher = dataLoaderDispatcherStrategy.modifyDataFetcher(dataFetcher);
379+
dataFetcher = executionContext.getDataLoaderDispatcherStrategy().modifyDataFetcher(dataFetcher);
387380
dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, instrumentationFieldFetchParams, executionContext.getInstrumentationState());
388381
CompletableFuture<Object> fetchedValue = invokeDataFetcher(executionContext, parameters, fieldDef, dataFetchingEnvironment, dataFetcher);
389-
dataLoaderDispatcherStrategy.fieldFetched(executionContext, parameters, dataFetcher, fetchedValue);
382+
executionContext.getDataLoaderDispatcherStrategy().fieldFetched(executionContext, parameters, dataFetcher, fetchedValue);
390383
fetchCtx.onDispatched(fetchedValue);
391384
return fetchedValue
392385
.handle((result, exception) -> {

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -102,38 +102,38 @@ public void executionStrategy(ExecutionContext executionContext, ExecutionStrate
102102
@Override
103103
public void executionStrategy_onFieldValuesInfo(List<FieldValueInfo> fieldValueInfoList, ExecutionStrategyParameters parameters) {
104104
int curLevel = parameters.getPath().getLevel() + 1;
105-
onFieldValuesInfoDispatchIfNeeded(fieldValueInfoList, curLevel);
105+
onFieldValuesInfoDispatchIfNeeded(fieldValueInfoList, curLevel, parameters);
106106
}
107107

108+
public void executionStrategy_onFieldValuesException(Throwable t, ExecutionStrategyParameters executionStrategyParameters) {
109+
int curLevel = executionStrategyParameters.getPath().getLevel() + 1;
110+
synchronized (callStack) {
111+
callStack.increaseHappenedOnFieldValueCalls(curLevel);
112+
}
113+
}
114+
115+
108116
@Override
109117
public void executeObject(ExecutionContext executionContext, ExecutionStrategyParameters parameters) {
110118
int curLevel = parameters.getExecutionStepInfo().getPath().getLevel() + 1;
111119
increaseCallCounts(curLevel, parameters);
112120
}
113121

114122
@Override
115-
public void executeObject_onFieldValuesException(Throwable t, ExecutionStrategyParameters parameters) {
123+
public void executeObject_onFieldValuesInfo(List<FieldValueInfo> fieldValueInfoList, ExecutionStrategyParameters parameters) {
116124
int curLevel = parameters.getPath().getLevel() + 1;
117-
synchronized (callStack) {
118-
callStack.increaseHappenedOnFieldValueCalls(curLevel);
119-
}
125+
onFieldValuesInfoDispatchIfNeeded(fieldValueInfoList, curLevel, parameters);
120126
}
121127

122128

123129
@Override
124-
public void executionStrategy_onFieldValuesException(Throwable t, ExecutionStrategyParameters executionStrategyParameters) {
125-
int curLevel = executionStrategyParameters.getPath().getLevel() + 1;
130+
public void executeObject_onFieldValuesException(Throwable t, ExecutionStrategyParameters parameters) {
131+
int curLevel = parameters.getPath().getLevel() + 1;
126132
synchronized (callStack) {
127133
callStack.increaseHappenedOnFieldValueCalls(curLevel);
128134
}
129-
130135
}
131136

132-
@Override
133-
public void executeObject_onFieldValuesInfo(List<FieldValueInfo> fieldValueInfoList, ExecutionStrategyParameters parameters) {
134-
int curLevel = parameters.getPath().getLevel() + 1;
135-
onFieldValuesInfoDispatchIfNeeded(fieldValueInfoList, curLevel);
136-
}
137137

138138
@SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter")
139139
private void increaseCallCounts(int curLevel, ExecutionStrategyParameters executionStrategyParameters) {
@@ -145,7 +145,7 @@ private void increaseCallCounts(int curLevel, ExecutionStrategyParameters execut
145145
}
146146

147147
@SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter")
148-
private void onFieldValuesInfoDispatchIfNeeded(List<FieldValueInfo> fieldValueInfoList, int curLevel) {
148+
private void onFieldValuesInfoDispatchIfNeeded(List<FieldValueInfo> fieldValueInfoList, int curLevel, ExecutionStrategyParameters parameters) {
149149
boolean dispatchNeeded;
150150
synchronized (callStack) {
151151
dispatchNeeded = handleOnFieldValuesInfo(fieldValueInfoList, curLevel);

‎src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceTest.groovy‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ class DataLoaderPerformanceTest extends Specification {
2020
batchCompareDataFetchers = new BatchCompareDataFetchers()
2121
DataLoaderPerformanceData dataLoaderPerformanceData = new DataLoaderPerformanceData(batchCompareDataFetchers)
2222
dataLoaderRegistry = dataLoaderPerformanceData.setupDataLoaderRegistry()
23-
// Instrumentation instrumentation = new DataLoaderDispatcherInstrumentation()
24-
graphQL = dataLoaderPerformanceData.setupGraphQL(instrumentation)
23+
graphQL = dataLoaderPerformanceData.setupGraphQL()
2524
}
2625

2726
def "760 ensure data loader is performant for lists"() {

‎src/test/groovy/graphql/execution/instrumentation/fieldvalidation/FieldValidationTest.groovy‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import graphql.execution.ExecutionId
1212
import graphql.execution.ResultPath
1313
import graphql.execution.ValueUnboxer
1414
import graphql.execution.instrumentation.ChainedInstrumentation
15-
import graphql.execution.instrumentation.Instrumentation
1615
import graphql.execution.instrumentation.SimplePerformantInstrumentation
1716
import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters
1817
import spock.lang.Specification
@@ -308,7 +307,7 @@ class FieldValidationTest extends Specification {
308307
def document = TestUtil.parseQuery(query)
309308
def strategy = new AsyncExecutionStrategy()
310309
def instrumentation = new FieldValidationInstrumentation(validation)
311-
def execution = new Execution(strategy, strategy, strategy, instrumentation, ValueUnboxer.DEFAULT)
310+
def execution = new Execution(strategy, strategy, strategy, instrumentation, ValueUnboxer.DEFAULT, false)
312311

313312
def executionInput = ExecutionInput.newExecutionInput().query(query).variables(variables).build()
314313
execution.execute(document, schema, ExecutionId.generate(), executionInput, SimplePerformantInstrumentation.INSTANCE.createState(new InstrumentationCreateStateParameters(schema, executionInput)))
@@ -322,7 +321,7 @@ class FieldValidationTest extends Specification {
322321
}
323322
}
324323
def instrumentations = [new FieldValidationInstrumentation
325-
(fieldValidation)]
324+
(fieldValidation)]
326325
def chainedInstrumentation = new ChainedInstrumentation(instrumentations);
327326
def graphql = GraphQL
328327
.newGraphQL(schema)

0 commit comments

Comments
 (0)