diff --git a/.github/workflows/invoke_test_runner.yml b/.github/workflows/invoke_test_runner.yml index 61be8d20da..09f5cdb243 100644 --- a/.github/workflows/invoke_test_runner.yml +++ b/.github/workflows/invoke_test_runner.yml @@ -50,7 +50,7 @@ jobs: - id: 'auth' name: 'Authenticate to Google Cloud' - uses: google-github-actions/auth@v2.1.3 + uses: google-github-actions/auth@v2.1.5 with: credentials_json: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }} diff --git a/agent-test/build.gradle b/agent-test/build.gradle index d447dc42fe..3a5911b824 100644 --- a/agent-test/build.gradle +++ b/agent-test/build.gradle @@ -4,9 +4,9 @@ plugins { dependencies { implementation(rootProject) - implementation("net.bytebuddy:byte-buddy-agent:1.14.18") + implementation("net.bytebuddy:byte-buddy-agent:1.15.1") - testImplementation 'org.junit.jupiter:junit-jupiter:5.10.3' + testImplementation 'org.junit.jupiter:junit-jupiter:5.11.0' testRuntimeOnly 'org.junit.platform:junit-platform-launcher' testImplementation("org.assertj:assertj-core:3.26.3") diff --git a/agent/build.gradle b/agent/build.gradle index 1dfeba3f27..7d220cfcdd 100644 --- a/agent/build.gradle +++ b/agent/build.gradle @@ -6,7 +6,7 @@ plugins { } dependencies { - implementation("net.bytebuddy:byte-buddy:1.14.18") + implementation("net.bytebuddy:byte-buddy:1.15.1") // graphql-java itself implementation(rootProject) } diff --git a/build.gradle b/build.gradle index 07a6456b03..07509bb943 100644 --- a/build.gradle +++ b/build.gradle @@ -117,7 +117,7 @@ dependencies { testImplementation 'org.reactivestreams:reactive-streams-tck:' + reactiveStreamsVersion testImplementation "io.reactivex.rxjava2:rxjava:2.2.21" - testImplementation "io.projectreactor:reactor-core:3.6.8" + testImplementation "io.projectreactor:reactor-core:3.6.9" testImplementation 'org.testng:testng:7.10.2' // use for reactive streams test inheritance diff --git a/src/main/java/graphql/execution/DataLoaderDispatchStrategy.java b/src/main/java/graphql/execution/DataLoaderDispatchStrategy.java index ee3c0bd97c..9e21b94bfa 100644 --- a/src/main/java/graphql/execution/DataLoaderDispatchStrategy.java +++ b/src/main/java/graphql/execution/DataLoaderDispatchStrategy.java @@ -4,7 +4,6 @@ import graphql.schema.DataFetcher; import java.util.List; -import java.util.concurrent.CompletableFuture; @Internal public interface DataLoaderDispatchStrategy { @@ -50,7 +49,7 @@ default DataFetcher modifyDataFetcher(DataFetcher dataFetcher) { return dataFetcher; } - default void deferredField(ExecutionContext executionContext, MergedField currentField) { + default void executeDeferredOnFieldValueInfo(FieldValueInfo fieldValueInfo, ExecutionStrategyParameters executionStrategyParameters) { } } diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index a3fff5a8e8..cf805a84fb 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -14,6 +14,7 @@ import graphql.execution.instrumentation.InstrumentationState; import graphql.execution.instrumentation.dataloader.FallbackDataLoaderDispatchStrategy; import graphql.execution.instrumentation.dataloader.PerLevelDataLoaderDispatchStrategy; +import graphql.execution.instrumentation.dataloader.PerLevelDataLoaderDispatchStrategyWithDeferAlwaysDispatch; import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters; import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters; import graphql.extensions.ExtensionsBuilder; @@ -228,7 +229,14 @@ private DataLoaderDispatchStrategy createDataLoaderDispatchStrategy(ExecutionCon return DataLoaderDispatchStrategy.NO_OP; } if (executionStrategy instanceof AsyncExecutionStrategy) { - return new PerLevelDataLoaderDispatchStrategy(executionContext); + boolean deferEnabled = Optional.ofNullable(executionContext.getGraphQLContext()) + .map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_INCREMENTAL_SUPPORT)) + .orElse(false); + + // Dedicated strategy for defer support, for safety purposes. + return deferEnabled ? + new PerLevelDataLoaderDispatchStrategyWithDeferAlwaysDispatch(executionContext) : + new PerLevelDataLoaderDispatchStrategy(executionContext); } else { return new FallbackDataLoaderDispatchStrategy(executionContext); } diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 36c2e78956..f2cccd4072 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -318,7 +318,7 @@ Async.CombinedBuilder getAsyncFieldValueInfo( ) { MergedSelectionSet fields = parameters.getFields(); - executionContext.getIncrementalCallState().enqueue(deferredExecutionSupport.createCalls()); + executionContext.getIncrementalCallState().enqueue(deferredExecutionSupport.createCalls(parameters)); // Only non-deferred fields should be considered for calculating the expected size of futures. Async.CombinedBuilder futures = Async diff --git a/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java b/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java index 034138d110..3b8e7efe8a 100644 --- a/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java +++ b/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java @@ -44,7 +44,7 @@ public interface DeferredExecutionSupport { List getNonDeferredFieldNames(List allFieldNames); - Set> createCalls(); + Set> createCalls(ExecutionStrategyParameters executionStrategyParameters); DeferredExecutionSupport NOOP = new DeferredExecutionSupport.NoOp(); @@ -105,19 +105,19 @@ public List getNonDeferredFieldNames(List allFieldNames) { } @Override - public Set> createCalls() { + public Set> createCalls(ExecutionStrategyParameters executionStrategyParameters) { return deferredExecutionToFields.keySet().stream() - .map(this::createDeferredFragmentCall) + .map(deferredExecution -> this.createDeferredFragmentCall(deferredExecution, executionStrategyParameters)) .collect(Collectors.toSet()); } - private DeferredFragmentCall createDeferredFragmentCall(DeferredExecution deferredExecution) { + private DeferredFragmentCall createDeferredFragmentCall(DeferredExecution deferredExecution, ExecutionStrategyParameters executionStrategyParameters) { DeferredCallContext deferredCallContext = new DeferredCallContext(); List mergedFields = deferredExecutionToFields.get(deferredExecution); List>> calls = mergedFields.stream() - .map(currentField -> this.createResultSupplier(currentField, deferredCallContext)) + .map(currentField -> this.createResultSupplier(currentField, deferredCallContext, executionStrategyParameters)) .collect(Collectors.toList()); return new DeferredFragmentCall( @@ -130,7 +130,8 @@ private DeferredFragmentCall createDeferredFragmentCall(DeferredExecution deferr private Supplier> createResultSupplier( MergedField currentField, - DeferredCallContext deferredCallContext + DeferredCallContext deferredCallContext, + ExecutionStrategyParameters executionStrategyParameters ) { Map fields = new LinkedHashMap<>(); fields.put(currentField.getResultKey(), currentField); @@ -149,7 +150,6 @@ private Supplier fieldValueResult = resolveFieldWithInfoFn .apply(executionContext, callParameters); - // Create a reference to the CompletableFuture that resolves an ExecutionResult - // so we can pass it to the Instrumentation "onDispatched" callback. CompletableFuture executionResultCF = fieldValueResult - .thenCompose(fvi -> fvi - .getFieldValueFuture() - .thenApply(fv -> ExecutionResultImpl.newExecutionResult().data(fv).build()) + .thenCompose(fvi -> { + executionContext.getDataLoaderDispatcherStrategy().executeDeferredOnFieldValueInfo(fvi, executionStrategyParameters); + + return fvi + .getFieldValueFuture() + .thenApply(fv -> ExecutionResultImpl.newExecutionResult().data(fv).build()); + } ); return executionResultCF @@ -199,7 +201,7 @@ public List getNonDeferredFieldNames(List allFieldNames) { } @Override - public Set> createCalls() { + public Set> createCalls(ExecutionStrategyParameters executionStrategyParameters) { return Collections.emptySet(); } } diff --git a/src/main/java/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentation.java b/src/main/java/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentation.java deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java b/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java index a407346954..60bd36e6f9 100644 --- a/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java +++ b/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java @@ -98,7 +98,7 @@ public PerLevelDataLoaderDispatchStrategy(ExecutionContext executionContext) { } @Override - public void deferredField(ExecutionContext executionContext, MergedField currentField) { + public void executeDeferredOnFieldValueInfo(FieldValueInfo fieldValueInfo, ExecutionStrategyParameters executionStrategyParameters) { throw new UnsupportedOperationException("Data Loaders cannot be used to resolve deferred fields"); } diff --git a/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategyWithDeferAlwaysDispatch.java b/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategyWithDeferAlwaysDispatch.java new file mode 100644 index 0000000000..e81ef3948c --- /dev/null +++ b/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategyWithDeferAlwaysDispatch.java @@ -0,0 +1,274 @@ +package graphql.execution.instrumentation.dataloader; + +import graphql.Assert; +import graphql.Internal; +import graphql.execution.DataLoaderDispatchStrategy; +import graphql.execution.ExecutionContext; +import graphql.execution.ExecutionStrategyParameters; +import graphql.execution.FieldValueInfo; +import graphql.schema.DataFetcher; +import graphql.util.LockKit; +import org.dataloader.DataLoaderRegistry; + +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * The execution of a query can be divided into 2 phases: first, the non-deferred fields are executed and only once + * they are completely resolved, we start to execute the deferred fields. + * The behavior of this Data Loader strategy is quite different during those 2 phases. During the execution of the + * deferred fields the Data Loader will not attempt to dispatch in a optimal way. It will essentially dispatch for + * every field fetched, which is quite ineffective. + * This is the first iteration of the Data Loader strategy with support for @defer, and it will be improved in the + * future. + */ +@Internal +public class PerLevelDataLoaderDispatchStrategyWithDeferAlwaysDispatch implements DataLoaderDispatchStrategy { + + private final CallStack callStack; + private final ExecutionContext executionContext; + + /** + * This flag is used to determine if we have started the deferred execution. + * The value of this flag is set to true as soon as we identified that a deferred field is being executed, and then + * the flag stays on that state for the remainder of the execution. + */ + private final AtomicBoolean startedDeferredExecution = new AtomicBoolean(false); + + + private static class CallStack { + + private final LockKit.ReentrantLock lock = new LockKit.ReentrantLock(); + private final LevelMap expectedFetchCountPerLevel = new LevelMap(); + private final LevelMap fetchCountPerLevel = new LevelMap(); + private final LevelMap expectedStrategyCallsPerLevel = new LevelMap(); + private final LevelMap happenedStrategyCallsPerLevel = new LevelMap(); + private final LevelMap happenedOnFieldValueCallsPerLevel = new LevelMap(); + + private final Set dispatchedLevels = new LinkedHashSet<>(); + + public CallStack() { + expectedStrategyCallsPerLevel.set(1, 1); + } + + void increaseExpectedFetchCount(int level, int count) { + expectedFetchCountPerLevel.increment(level, count); + } + + void increaseFetchCount(int level) { + fetchCountPerLevel.increment(level, 1); + } + + void increaseExpectedStrategyCalls(int level, int count) { + expectedStrategyCallsPerLevel.increment(level, count); + } + + void increaseHappenedStrategyCalls(int level) { + happenedStrategyCallsPerLevel.increment(level, 1); + } + + void increaseHappenedOnFieldValueCalls(int level) { + happenedOnFieldValueCallsPerLevel.increment(level, 1); + } + + boolean allStrategyCallsHappened(int level) { + return happenedStrategyCallsPerLevel.get(level) == expectedStrategyCallsPerLevel.get(level); + } + + boolean allOnFieldCallsHappened(int level) { + return happenedOnFieldValueCallsPerLevel.get(level) == expectedStrategyCallsPerLevel.get(level); + } + + boolean allFetchesHappened(int level) { + return fetchCountPerLevel.get(level) == expectedFetchCountPerLevel.get(level); + } + + @Override + public String toString() { + return "CallStack{" + + "expectedFetchCountPerLevel=" + expectedFetchCountPerLevel + + ", fetchCountPerLevel=" + fetchCountPerLevel + + ", expectedStrategyCallsPerLevel=" + expectedStrategyCallsPerLevel + + ", happenedStrategyCallsPerLevel=" + happenedStrategyCallsPerLevel + + ", happenedOnFieldValueCallsPerLevel=" + happenedOnFieldValueCallsPerLevel + + ", dispatchedLevels" + dispatchedLevels + + '}'; + } + + + public boolean dispatchIfNotDispatchedBefore(int level) { + if (dispatchedLevels.contains(level)) { + Assert.assertShouldNeverHappen("level " + level + " already dispatched"); + return false; + } + dispatchedLevels.add(level); + return true; + } + } + + public PerLevelDataLoaderDispatchStrategyWithDeferAlwaysDispatch(ExecutionContext executionContext) { + this.callStack = new CallStack(); + this.executionContext = executionContext; + } + + @Override + public void executeDeferredOnFieldValueInfo(FieldValueInfo fieldValueInfo, ExecutionStrategyParameters executionStrategyParameters) { + this.startedDeferredExecution.set(true); + } + + @Override + public void executionStrategy(ExecutionContext executionContext, ExecutionStrategyParameters parameters) { + if (this.startedDeferredExecution.get()) { + return; + } + int curLevel = parameters.getExecutionStepInfo().getPath().getLevel() + 1; + increaseCallCounts(curLevel, parameters); + } + + @Override + public void executeObject(ExecutionContext executionContext, ExecutionStrategyParameters parameters) { + if (this.startedDeferredExecution.get()) { + return; + } + int curLevel = parameters.getExecutionStepInfo().getPath().getLevel() + 1; + increaseCallCounts(curLevel, parameters); + } + + @Override + public void executionStrategyOnFieldValuesInfo(List fieldValueInfoList, ExecutionStrategyParameters parameters) { + if (this.startedDeferredExecution.get()) { + this.dispatch(); + } + int curLevel = parameters.getPath().getLevel() + 1; + onFieldValuesInfoDispatchIfNeeded(fieldValueInfoList, curLevel, parameters); + } + + @Override + public void executionStrategyOnFieldValuesException(Throwable t, ExecutionStrategyParameters executionStrategyParameters) { + int curLevel = executionStrategyParameters.getPath().getLevel() + 1; + callStack.lock.runLocked(() -> + callStack.increaseHappenedOnFieldValueCalls(curLevel) + ); + } + + @Override + public void executeObjectOnFieldValuesInfo(List fieldValueInfoList, ExecutionStrategyParameters parameters) { + if (this.startedDeferredExecution.get()) { + this.dispatch(); + } + int curLevel = parameters.getPath().getLevel() + 1; + onFieldValuesInfoDispatchIfNeeded(fieldValueInfoList, curLevel, parameters); + } + + + @Override + public void executeObjectOnFieldValuesException(Throwable t, ExecutionStrategyParameters parameters) { + int curLevel = parameters.getPath().getLevel() + 1; + callStack.lock.runLocked(() -> + callStack.increaseHappenedOnFieldValueCalls(curLevel) + ); + } + + @Override + public void fieldFetched(ExecutionContext executionContext, + ExecutionStrategyParameters parameters, + DataFetcher dataFetcher, + Object fetchedValue) { + + final boolean dispatchNeeded; + + if (parameters.getField().isDeferred() || this.startedDeferredExecution.get()) { + this.startedDeferredExecution.set(true); + dispatchNeeded = true; + } else { + int level = parameters.getPath().getLevel(); + dispatchNeeded = callStack.lock.callLocked(() -> { + callStack.increaseFetchCount(level); + return dispatchIfNeeded(level); + }); + } + + if (dispatchNeeded) { + dispatch(); + } + + } + + private void increaseCallCounts(int curLevel, ExecutionStrategyParameters parameters) { + int nonDeferredFieldCount = (int) parameters.getFields().getSubFieldsList().stream() + .filter(field -> !field.isDeferred()) + .count(); + + callStack.lock.runLocked(() -> { + callStack.increaseExpectedFetchCount(curLevel, nonDeferredFieldCount); + callStack.increaseHappenedStrategyCalls(curLevel); + }); + } + + private void onFieldValuesInfoDispatchIfNeeded(List fieldValueInfoList, int curLevel, ExecutionStrategyParameters parameters) { + boolean dispatchNeeded = callStack.lock.callLocked(() -> + handleOnFieldValuesInfo(fieldValueInfoList, curLevel) + ); + if (dispatchNeeded) { + dispatch(); + } + } + + // + // thread safety: called with callStack.lock + // + private boolean handleOnFieldValuesInfo(List fieldValueInfos, int curLevel) { + callStack.increaseHappenedOnFieldValueCalls(curLevel); + int expectedStrategyCalls = getCountForList(fieldValueInfos); + callStack.increaseExpectedStrategyCalls(curLevel + 1, expectedStrategyCalls); + return dispatchIfNeeded(curLevel + 1); + } + + private int getCountForList(List fieldValueInfos) { + int result = 0; + for (FieldValueInfo fieldValueInfo : fieldValueInfos) { + if (fieldValueInfo.getCompleteValueType() == FieldValueInfo.CompleteValueType.OBJECT) { + result += 1; + } else if (fieldValueInfo.getCompleteValueType() == FieldValueInfo.CompleteValueType.LIST) { + result += getCountForList(fieldValueInfo.getFieldValueInfos()); + } + } + return result; + } + + // + // thread safety : called with callStack.lock + // + private boolean dispatchIfNeeded(int level) { + boolean ready = levelReady(level); + if (ready) { + return callStack.dispatchIfNotDispatchedBefore(level); + } + return false; + } + + // + // thread safety: called with callStack.lock + // + private boolean levelReady(int level) { + if (level == 1) { + // level 1 is special: there is only one strategy call and that's it + return callStack.allFetchesHappened(1); + } + if (levelReady(level - 1) && callStack.allOnFieldCallsHappened(level - 1) + && callStack.allStrategyCallsHappened(level) && callStack.allFetchesHappened(level)) { + + return true; + } + return false; + } + + void dispatch() { + DataLoaderRegistry dataLoaderRegistry = executionContext.getDataLoaderRegistry(); + dataLoaderRegistry.dispatchAll(); + } + +} + diff --git a/src/main/java/graphql/incremental/IncrementalExecutionResultImpl.java b/src/main/java/graphql/incremental/IncrementalExecutionResultImpl.java index 77b05c6fe5..704d1b67cc 100644 --- a/src/main/java/graphql/incremental/IncrementalExecutionResultImpl.java +++ b/src/main/java/graphql/incremental/IncrementalExecutionResultImpl.java @@ -10,6 +10,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.function.Consumer; import java.util.stream.Collectors; @ExperimentalApi @@ -52,6 +53,13 @@ public static Builder fromExecutionResult(ExecutionResult executionResult) { return new Builder().from(executionResult); } + @Override + public IncrementalExecutionResult transform(Consumer> builderConsumer) { + var builder = fromExecutionResult(this); + builderConsumer.accept(builder); + return builder.build(); + } + @Override public Map toSpecification() { Map map = new LinkedHashMap<>(super.toSpecification()); diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/BatchCompareDataFetchers.java b/src/test/groovy/graphql/execution/instrumentation/dataloader/BatchCompareDataFetchers.java index 7f20938b3b..d0dacd5964 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/BatchCompareDataFetchers.java +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/BatchCompareDataFetchers.java @@ -10,6 +10,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -36,7 +37,7 @@ public void useAsyncBatchLoading(boolean flag) { useAsyncBatchLoading.set(flag); } - // Shops + private static final Map shops = new LinkedHashMap<>(); private static final Map expensiveShops = new LinkedHashMap<>(); diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceData.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceData.groovy index 6246f883d8..7366ded562 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceData.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceData.groovy @@ -1,5 +1,6 @@ package graphql.execution.instrumentation.dataloader +import com.fasterxml.jackson.databind.ObjectMapper import graphql.Directives import graphql.GraphQL import graphql.execution.instrumentation.Instrumentation @@ -60,20 +61,29 @@ class DataLoaderPerformanceData { [id: "department-9", name: "Department 9", products: [[id: "product-9", name: "Product 9"]]]] ]] ] + static String getQuery() { + return getQuery(false, false) + } - static def query = """ + static String getQuery(boolean deferDepartments, boolean deferProducts) { + return """ query { shops { id name - departments { - id name - products { + ... @defer(if: $deferDepartments) { + departments { id name + ... @defer(if: $deferProducts) { + products { + id name + } + } } - } + } } } """ + } static def expectedExpensiveData = [ shops : [[name : "Shop 1", @@ -162,7 +172,8 @@ class DataLoaderPerformanceData { assert incrementalResultsItems.any { it == [path: [], data: [expensiveShops: [[id: "exshop-1", name: "ExShop 1"], [id: "exshop-2", name: "ExShop 2"], [id: "exshop-3", name: "ExShop 3"]]]] } } - static def expensiveQuery = """ + static String getExpensiveQuery(boolean deferredEnabled) { + return """ query { shops { name @@ -171,19 +182,25 @@ class DataLoaderPerformanceData { products { name } - expensiveProducts { - name - } + ... @defer(if: $deferredEnabled) { + expensiveProducts { + name + } + } } - expensiveDepartments { - name - products { - name - } - expensiveProducts { + ... @defer(if: $deferredEnabled) { + expensiveDepartments { name + products { + name + } + ... @defer(if: $deferredEnabled) { + expensiveProducts { + name + } + } } - } + } } expensiveShops { name @@ -192,159 +209,30 @@ class DataLoaderPerformanceData { products { name } - expensiveProducts { - name - } + ... @defer(if: $deferredEnabled) { + expensiveProducts { + name + } + } } - expensiveDepartments { - name - products { - name - } - expensiveProducts { + ... @defer(if: $deferredEnabled) { + expensiveDepartments { name - } - } - } - } - """ - - static def expectedInitialDeferredData = [ - data : [ - shops: [ - [id: "shop-1", name: "Shop 1"], - [id: "shop-2", name: "Shop 2"], - [id: "shop-3", name: "Shop 3"], - ] - ], - hasNext: true - ] - - static def expectedListOfDeferredData = [ - [ - hasNext : true, - incremental: [[ - path: ["shops", 0], - data: [ - departments: [ - [id: "department-1", name: "Department 1", products: [[id: "product-1", name: "Product 1"]]], - [id: "department-2", name: "Department 2", products: [[id: "product-2", name: "Product 2"]]], - [id: "department-3", name: "Department 3", products: [[id: "product-3", name: "Product 3"]]] - ] - ] - ]], - ], - [ - hasNext : true, - incremental: [[ - path: ["shops", 1], - data: [ - departments: [ - [id: "department-4", name: "Department 4", products: [[id: "product-4", name: "Product 4"]]], - [id: "department-5", name: "Department 5", products: [[id: "product-5", name: "Product 5"]]], - [id: "department-6", name: "Department 6", products: [[id: "product-6", name: "Product 6"]]] - ] - ], - ]], - ], - [ - hasNext : false, - incremental: [[ - path: ["shops", 2], - data: [ - departments: [ - [id: "department-7", name: "Department 7", products: [[id: "product-7", name: "Product 7"]]], - [id: "department-8", name: "Department 8", products: [[id: "product-8", name: "Product 8"]]], - [id: "department-9", name: "Department 9", products: [[id: "product-9", name: "Product 9"]]] - ] - ] - ]], - ] - ] - - - static def deferredQuery = """ - query { - shops { - id name - ... @defer { - departments { - id name products { - id name + name } - } - } - } - } - """ - - static def expensiveDeferredQuery = """ - query { - shops { - id name - ... @defer { - departments { - name - ... @defer { - products { - name - } - } - ... @defer { + ... @defer(if: $deferredEnabled) { expensiveProducts { name } } } } - ... @defer { - expensiveDepartments { - name - products { - name - } - expensiveProducts { - name - } - } - } } - ... @defer { - expensiveShops { - id name - } - } } - """ +""" - static def expectedExpensiveDeferredData = [ - [[id: "exshop-1", name: "ExShop 1"], [id: "exshop-2", name: "ExShop 2"], [id: "exshop-3", name: "ExShop 3"]], - [[name: "Department 1",products:null, expensiveProducts:null], [name: "Department 2",products:null, expensiveProducts:null], [name: "Department 3",products:null, expensiveProducts:null]], - [[name: "Department 1", products: [[name: "Product 1"]], expensiveProducts: [[name: "Product 1"]]], [name: "Department 2", products: [[name: "Product 2"]], expensiveProducts: [[name: "Product 2"]]], [name: "Department 3", products: [[name: "Product 3"]], expensiveProducts: [[name: "Product 3"]]]], - [[name: "Department 4",products:null, expensiveProducts:null], [name: "Department 5",products:null, expensiveProducts:null], [name: "Department 6",products:null, expensiveProducts:null]], - [[name: "Department 4", products: [[name: "Product 4"]], expensiveProducts: [[name: "Product 4"]]], [name: "Department 5", products: [[name: "Product 5"]], expensiveProducts: [[name: "Product 5"]]], [name: "Department 6", products: [[name: "Product 6"]], expensiveProducts: [[name: "Product 6"]]]], - [[name: "Department 7",products:null, expensiveProducts:null], [name: "Department 8",products:null, expensiveProducts:null], [name: "Department 9",products:null, expensiveProducts:null]], - [[name: "Department 7", products: [[name: "Product 7"]], expensiveProducts: [[name: "Product 7"]]], [name: "Department 8", products: [[name: "Product 8"]], expensiveProducts: [[name: "Product 8"]]], [name: "Department 9", products: [[name: "Product 9"]], expensiveProducts: [[name: "Product 9"]]]], - [[name: "Product 1"]], - [[name: "Product 1"]], - [[name: "Product 2"]], - [[name: "Product 2"]], - [[name: "Product 3"]], - [[name: "Product 3"]], - [[name: "Product 4"]], - [[name: "Product 4"]], - [[name: "Product 5"]], - [[name: "Product 5"]], - [[name: "Product 6"]], - [[name: "Product 6"]], - [[name: "Product 7"]], - [[name: "Product 7"]], - [[name: "Product 8"]], - [[name: "Product 8"]], - [[name: "Product 9"]], - [[name: "Product 9"]], - ] + } static List> getIncrementalResults(IncrementalExecutionResult initialResult) { Publisher deferredResultStream = initialResult.incrementalItemPublisher @@ -353,7 +241,63 @@ class DataLoaderPerformanceData { deferredResultStream.subscribe(subscriber) Awaitility.await().untilTrue(subscriber.isDone()) + if(subscriber.getThrowable() != null) { + throw subscriber.getThrowable() + } + return subscriber.getEvents() .collect { it.toSpecification() } } + + /** + * Combines the initial result with the incremental results into a single result that has the same shape as a + * "normal" execution result. + * + * @param initialResult the data from the initial execution + * @param incrementalResults the data from the incremental executions + * @return a single result that combines the initial and incremental results + */ + static Map combineExecutionResults(Map initialResult, List> incrementalResults) { + Map combinedResult = deepClone(initialResult, Map.class) + + incrementalResults + // groovy's flatMap + .collectMany { (List) it.incremental } + .each { result -> + def parent = findByPath((Map) combinedResult.data, (List) result.path) + if (parent instanceof Map) { + parent.putAll((Map) result.data) + } else if (parent instanceof List) { + parent.addAll(result.data) + } else { + throw new RuntimeException("Unexpected parent type: ${parent.getClass()}") + } + + if(combinedResult.errors != null && !result.errors.isEmpty()) { + if(combinedResult.errors == null) { + combinedResult.errors = [] + } + + combinedResult.errors.addAll(result.errors) + } + } + + // Remove the "hasNext" to make the result look like a normal execution result + combinedResult.remove("hasNext") + + combinedResult + } + + private static ObjectMapper objectMapper = new ObjectMapper() + private static T deepClone(Object obj, Class clazz) { + return objectMapper.readValue(objectMapper.writeValueAsString(obj), clazz) + } + + private static Object findByPath(Map data, List path) { + def current = data + path.each { key -> + current = current[key] + } + current + } } diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceTest.groovy index 61fba228bf..c4239243ca 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceTest.groovy @@ -2,21 +2,13 @@ package graphql.execution.instrumentation.dataloader import graphql.ExecutionInput import graphql.GraphQL -import graphql.incremental.IncrementalExecutionResult import org.dataloader.DataLoaderRegistry -import spock.lang.Ignore import spock.lang.Specification import static graphql.ExperimentalApi.ENABLE_INCREMENTAL_SUPPORT -import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.assertIncrementalExpensiveData import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.expectedExpensiveData -import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.expectedInitialDeferredData -import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getDeferredQuery import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getExpectedData -import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getExpectedListOfDeferredData -import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getExpensiveDeferredQuery import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getExpensiveQuery -import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getIncrementalResults import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getQuery class DataLoaderPerformanceTest extends Specification { @@ -35,7 +27,7 @@ class DataLoaderPerformanceTest extends Specification { def "760 ensure data loader is performant for lists"() { when: ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(query) + .query(getQuery()) .dataLoaderRegistry(dataLoaderRegistry) .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): incrementalSupport]) .build() @@ -57,7 +49,7 @@ class DataLoaderPerformanceTest extends Specification { when: ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(expensiveQuery) + .query(getExpensiveQuery(false)) .dataLoaderRegistry(dataLoaderRegistry) .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): incrementalSupport]) .build() @@ -80,7 +72,7 @@ class DataLoaderPerformanceTest extends Specification { batchCompareDataFetchers.useAsyncBatchLoading(true) ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(query) + .query(getQuery()) .dataLoaderRegistry(dataLoaderRegistry) .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): incrementalSupport]) .build() @@ -105,7 +97,7 @@ class DataLoaderPerformanceTest extends Specification { batchCompareDataFetchers.useAsyncBatchLoading(true) ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(expensiveQuery) + .query(getExpensiveQuery(false)) .dataLoaderRegistry(dataLoaderRegistry) .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): incrementalSupport]) .build() @@ -121,75 +113,4 @@ class DataLoaderPerformanceTest extends Specification { where: incrementalSupport << [true, false] } - - def "data loader will not work with deferred queries"() { - when: - ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(deferredQuery) - .dataLoaderRegistry(dataLoaderRegistry) - .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): true]) - .build() - - def result = graphQL.execute(executionInput) - println(result); - - then: - def exception = thrown(UnsupportedOperationException) - exception.message == "Data Loaders cannot be used to resolve deferred fields" - } - - @Ignore("Resolution of deferred fields via Data loaders is not yet supported") - def "data loader will work with deferred queries"() { - - when: - - ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(deferredQuery) - .dataLoaderRegistry(dataLoaderRegistry) - .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): true]) - .build() - - IncrementalExecutionResult result = graphQL.execute(executionInput) - - then: - result.toSpecification() == expectedInitialDeferredData - - when: - def incrementalResults = getIncrementalResults(result) - - then: - incrementalResults == expectedListOfDeferredData - - // With deferred results, we don't achieve the same efficiency. - batchCompareDataFetchers.departmentsForShopsBatchLoaderCounter.get() == 3 - batchCompareDataFetchers.productsForDepartmentsBatchLoaderCounter.get() == 3 - } - - @Ignore("Resolution of deferred fields via Data loaders is not yet supported") - def "data loader will work with deferred queries on multiple levels deep"() { - - when: - - ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(expensiveDeferredQuery) - .dataLoaderRegistry(dataLoaderRegistry) - .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): true]) - .build() - - IncrementalExecutionResult result = graphQL.execute(executionInput) - - then: - result.toSpecification() == expectedInitialDeferredData - - when: - def incrementalResults = getIncrementalResults(result) - - then: - assertIncrementalExpensiveData(incrementalResults) - - // With deferred results, we don't achieve the same efficiency. - // The final number of loader calls is non-deterministic, so we can't assert an exact number. - batchCompareDataFetchers.departmentsForShopsBatchLoaderCounter.get() >= 3 - batchCompareDataFetchers.productsForDepartmentsBatchLoaderCounter.get() >= 3 - } } diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceWithChainedInstrumentationTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceWithChainedInstrumentationTest.groovy index e5d1089bab..5467c87220 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceWithChainedInstrumentationTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderPerformanceWithChainedInstrumentationTest.groovy @@ -2,21 +2,14 @@ package graphql.execution.instrumentation.dataloader import graphql.ExecutionInput import graphql.GraphQL -import graphql.incremental.IncrementalExecutionResult import org.dataloader.DataLoaderRegistry import spock.lang.Ignore import spock.lang.Specification import static graphql.ExperimentalApi.ENABLE_INCREMENTAL_SUPPORT -import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.assertIncrementalExpensiveData import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.expectedExpensiveData -import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.expectedInitialDeferredData -import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.expectedListOfDeferredData -import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getDeferredQuery import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getExpectedData -import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getExpensiveDeferredQuery import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getExpensiveQuery -import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getIncrementalResults import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getQuery class DataLoaderPerformanceWithChainedInstrumentationTest extends Specification { @@ -38,7 +31,7 @@ class DataLoaderPerformanceWithChainedInstrumentationTest extends Specification when: ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(query) + .query(getQuery()) .dataLoaderRegistry(dataLoaderRegistry) .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): incrementalSupport]) .build() @@ -62,7 +55,7 @@ class DataLoaderPerformanceWithChainedInstrumentationTest extends Specification when: ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(expensiveQuery) + .query(getExpensiveQuery(false)) .dataLoaderRegistry(dataLoaderRegistry) .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): incrementalSupport]) .build() @@ -110,7 +103,7 @@ class DataLoaderPerformanceWithChainedInstrumentationTest extends Specification batchCompareDataFetchers.useAsyncBatchLoading(true) ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(expensiveQuery) + .query(getExpensiveQuery(false)) .dataLoaderRegistry(dataLoaderRegistry) .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): incrementalSupport]) .build() @@ -126,73 +119,4 @@ class DataLoaderPerformanceWithChainedInstrumentationTest extends Specification incrementalSupport << [true, false] } - def "chainedInstrumentation: data loader will not work with deferred queries"() { - when: - ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(deferredQuery) - .dataLoaderRegistry(dataLoaderRegistry) - .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): true]) - .build() - - graphQL.execute(executionInput) - - then: - def exception = thrown(UnsupportedOperationException) - exception.message == "Data Loaders cannot be used to resolve deferred fields" - } - - @Ignore("Resolution of deferred fields via Data loaders is not yet supported") - def "chainedInstrumentation: data loader will work with deferred queries"() { - - when: - - ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .query(deferredQuery) - .dataLoaderRegistry(dataLoaderRegistry) - .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): true]) - .build() - - IncrementalExecutionResult result = graphQL.execute(executionInput) - - then: - result.toSpecification() == expectedInitialDeferredData - - when: - def incrementalResults = getIncrementalResults(result) - - then: - incrementalResults == expectedListOfDeferredData - - // With deferred results, we don't achieve the same efficiency. - batchCompareDataFetchers.departmentsForShopsBatchLoaderCounter.get() == 3 - batchCompareDataFetchers.productsForDepartmentsBatchLoaderCounter.get() == 3 - } - - - @Ignore("Resolution of deferred fields via Data loaders is not yet supported") - def "chainedInstrumentation: data loader will work with deferred queries on multiple levels deep"() { - when: - ExecutionInput executionInput = ExecutionInput.newExecutionInput() - .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): true]) - .query(expensiveDeferredQuery) - .dataLoaderRegistry(dataLoaderRegistry) - .build() - - IncrementalExecutionResult result = graphQL.execute(executionInput) - - then: - result.toSpecification() == expectedInitialDeferredData - - when: - def incrementalResults = getIncrementalResults(result) - - - then: - assertIncrementalExpensiveData(incrementalResults) - - // With deferred results, we don't achieve the same efficiency. - // The final number of loader calls is non-deterministic, so we can't assert an exact number. - batchCompareDataFetchers.departmentsForShopsBatchLoaderCounter.get() >= 3 - batchCompareDataFetchers.productsForDepartmentsBatchLoaderCounter.get() >= 3 - } } diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/DeferWithDataLoaderTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/DeferWithDataLoaderTest.groovy new file mode 100644 index 0000000000..6da9489c76 --- /dev/null +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/DeferWithDataLoaderTest.groovy @@ -0,0 +1,340 @@ +package graphql.execution.instrumentation.dataloader + +import graphql.ExecutionInput +import graphql.ExecutionResult +import graphql.GraphQL +import graphql.incremental.IncrementalExecutionResult +import org.dataloader.DataLoaderRegistry +import spock.lang.Specification + +import java.util.stream.Collectors + +import static graphql.ExperimentalApi.ENABLE_INCREMENTAL_SUPPORT +import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.combineExecutionResults +import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.expectedData +import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.expectedExpensiveData +import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getExpensiveQuery +import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getIncrementalResults +import static graphql.execution.instrumentation.dataloader.DataLoaderPerformanceData.getQuery + +class DeferWithDataLoaderTest extends Specification { + + GraphQL graphQL + DataLoaderRegistry dataLoaderRegistry + BatchCompareDataFetchers batchCompareDataFetchers + + + void setup() { + batchCompareDataFetchers = new BatchCompareDataFetchers() + DataLoaderPerformanceData dataLoaderPerformanceData = new DataLoaderPerformanceData(batchCompareDataFetchers) + + dataLoaderRegistry = dataLoaderPerformanceData.setupDataLoaderRegistry() + graphQL = dataLoaderPerformanceData.setupGraphQL() + } + + /** + * @param results a list of the incremental results from the execution + * @param expectedPaths a list of the expected paths in the incremental results. The order of the elements in the list is not important. + */ + private static void assertIncrementalResults(List> results, List> expectedPaths) { + assert results.size() == expectedPaths.size(), "Expected ${expectedPaths.size()} results, got ${results.size()}" + + assert results.dropRight(1).every { it.hasNext == true }, "Expected all but the last result to have hasNext=true" + assert results.last().hasNext == false, "Expected last result to have hasNext=false" + + assert results.every { it.incremental.size() == 1 }, "Expected every result to have exactly one incremental item" + + expectedPaths.each { path -> + assert results.any { it.incremental[0].path == path }, "Expected path $path not found in $results" + } + } + + def "query with single deferred field"() { + given: + def query = getQuery(true, false) + + def expectedInitialData = [ + data : [ + shops: [ + [id: "shop-1", name: "Shop 1"], + [id: "shop-2", name: "Shop 2"], + [id: "shop-3", name: "Shop 3"], + ] + ], + hasNext: true + ] + + when: + ExecutionInput executionInput = ExecutionInput.newExecutionInput() + .query(query) + .dataLoaderRegistry(dataLoaderRegistry) + .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): true]) + .build() + + IncrementalExecutionResult result = graphQL.execute(executionInput) + + then: + result.toSpecification() == expectedInitialData + + when: + def incrementalResults = getIncrementalResults(result) + + then: + + assertIncrementalResults(incrementalResults, [["shops", 0], ["shops", 1], ["shops", 2]]) + + when: + def combined = combineExecutionResults(result.toSpecification(), incrementalResults) + then: + combined.errors == null + combined.data == expectedData + + batchCompareDataFetchers.departmentsForShopsBatchLoaderCounter.get() == 3 + batchCompareDataFetchers.productsForDepartmentsBatchLoaderCounter.get() == 9 + } + + def "multiple fields on same defer block"() { + given: + def query = """ + query { + shops { + id + ... @defer { + name + departments { + name + } + } + } + } + + """ + + def expectedInitialData = [ + data : [ + shops: [ + [id: "shop-1"], + [id: "shop-2"], + [id: "shop-3"], + ] + ], + hasNext: true + ] + + when: + ExecutionInput executionInput = ExecutionInput.newExecutionInput() + .query(query) + .dataLoaderRegistry(dataLoaderRegistry) + .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): true]) + .build() + + IncrementalExecutionResult result = graphQL.execute(executionInput) + + then: + result.toSpecification() == expectedInitialData + + when: + def incrementalResults = getIncrementalResults(result) + + then: + + assertIncrementalResults(incrementalResults, [["shops", 0], ["shops", 1], ["shops", 2]]) + + when: + def combined = combineExecutionResults(result.toSpecification(), incrementalResults) + then: + combined.errors == null + combined.data == [ + shops: [ + [id : "shop-1", name: "Shop 1", + departments: [[name: "Department 1"], + [name: "Department 2"], + [name: "Department 3"] + ]], + [id : "shop-2", name: "Shop 2", + departments: [[name: "Department 4"], + [name: "Department 5"], + [name: "Department 6"] + ]], + [id : "shop-3", name: "Shop 3", + departments: [[name: "Department 7"], + [name: "Department 8"], + [name: "Department 9"]] + ]] + ] + batchCompareDataFetchers.departmentsForShopsBatchLoaderCounter.get() == 3 + batchCompareDataFetchers.productsForDepartmentsBatchLoaderCounter.get() == 0 + } + + def "query with nested deferred fields"() { + given: + def query = getQuery(true, true) + + def expectedInitialData = [ + data : [ + shops: [ + [id: "shop-1", name: "Shop 1"], + [id: "shop-2", name: "Shop 2"], + [id: "shop-3", name: "Shop 3"], + ] + ], + hasNext: true + ] + + when: + ExecutionInput executionInput = ExecutionInput.newExecutionInput() + .query(query) + .dataLoaderRegistry(dataLoaderRegistry) + .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): true]) + .build() + + ExecutionResult result = graphQL.execute(executionInput) + + then: + result.toSpecification() == expectedInitialData + + when: + def incrementalResults = getIncrementalResults(result) + + then: + + assertIncrementalResults(incrementalResults, + [ + ["shops", 0], ["shops", 1], ["shops", 2], + ["shops", 0, "departments", 0], ["shops", 1, "departments", 0], ["shops", 2, "departments", 0], + ["shops", 0, "departments", 1], ["shops", 1, "departments", 1], ["shops", 2, "departments", 1], + ["shops", 0, "departments", 2], ["shops", 1, "departments", 2], ["shops", 2, "departments", 2], + ] + ) + + when: + def combined = combineExecutionResults(result.toSpecification(), incrementalResults) + then: + combined.errors == null + combined.data == expectedData + + batchCompareDataFetchers.departmentsForShopsBatchLoaderCounter.get() == 3 + batchCompareDataFetchers.productsForDepartmentsBatchLoaderCounter.get() == 9 + } + + def "query with top-level deferred field"() { + given: + def query = """ + query { + shops { + departments { + name + } + } + ... @defer { + expensiveShops { + name + } + } + } +""" + + def expectedInitialData = [ + data : [ + shops: [[departments: [[name: "Department 1"], [name: "Department 2"], [name: "Department 3"]]], + [departments: [[name: "Department 4"], [name: "Department 5"], [name: "Department 6"]]], + [departments: [[name: "Department 7"], [name: "Department 8"], [name: "Department 9"]]], + ] + ], + hasNext: true + ] + + when: + ExecutionInput executionInput = ExecutionInput.newExecutionInput() + .query(query) + .dataLoaderRegistry(dataLoaderRegistry) + .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): true]) + .build() + + IncrementalExecutionResult result = graphQL.execute(executionInput) + + then: + result.toSpecification() == expectedInitialData + + when: + def incrementalResults = getIncrementalResults(result) + + then: + + assertIncrementalResults(incrementalResults, + [ + [] + ] + ) + + when: + def combined = combineExecutionResults(result.toSpecification(), incrementalResults) + then: + combined.errors == null + combined.data == [shops : [[departments: [[name: "Department 1"], [name: "Department 2"], [name: "Department 3"]]], + [departments: [[name: "Department 4"], [name: "Department 5"], [name: "Department 6"]]], + [departments: [[name: "Department 7"], [name: "Department 8"], [name: "Department 9"]]]], + expensiveShops: [[name: "ExShop 1"], [name: "ExShop 2"], [name: "ExShop 3"]]] + + batchCompareDataFetchers.departmentsForShopsBatchLoaderCounter.get() == 1 + batchCompareDataFetchers.productsForDepartmentsBatchLoaderCounter.get() == 0 + } + + def "query with multiple deferred fields"() { + given: + def query = getExpensiveQuery(true) + + def expectedInitialData = + [data : [shops : [[name : "Shop 1", + departments: [[name: "Department 1", products: [[name: "Product 1"]]], [name: "Department 2", products: [[name: "Product 2"]]], [name: "Department 3", products: [[name: "Product 3"]]]]], + [name : "Shop 2", + departments: [[name: "Department 4", products: [[name: "Product 4"]]], [name: "Department 5", products: [[name: "Product 5"]]], [name: "Department 6", products: [[name: "Product 6"]]]]], + [name : "Shop 3", + departments: [[name: "Department 7", products: [[name: "Product 7"]]], [name: "Department 8", products: [[name: "Product 8"]]], [name: "Department 9", products: [[name: "Product 9"]]]]]], + expensiveShops: [[name : "ExShop 1", + departments: [[name: "Department 1", products: [[name: "Product 1"]]], [name: "Department 2", products: [[name: "Product 2"]]], [name: "Department 3", products: [[name: "Product 3"]]]]], + [name : "ExShop 2", + departments: [[name: "Department 4", products: [[name: "Product 4"]]], [name: "Department 5", products: [[name: "Product 5"]]], [name: "Department 6", products: [[name: "Product 6"]]]]], + [name : "ExShop 3", + departments: [[name: "Department 7", products: [[name: "Product 7"]]], [name: "Department 8", products: [[name: "Product 8"]]], [name: "Department 9", products: [[name: "Product 9"]]]]]]], + hasNext: true] + + when: + ExecutionInput executionInput = ExecutionInput.newExecutionInput() + .query(query) + .dataLoaderRegistry(dataLoaderRegistry) + .graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): true]) + .build() + + IncrementalExecutionResult result = graphQL.execute(executionInput) + + then: + result.toSpecification() == expectedInitialData + + when: + def incrementalResults = getIncrementalResults(result) + + then: + + assertIncrementalResults(incrementalResults, + [ + ["expensiveShops", 0], ["expensiveShops", 1], ["expensiveShops", 2], + ["shops", 0], ["shops", 1], ["shops", 2], + ["shops", 0, "departments", 0], ["shops", 0, "departments", 1],["shops", 0, "departments", 2], ["shops", 1, "departments", 0],["shops", 1, "departments", 1], ["shops", 1, "departments", 2], ["shops", 2, "departments", 0],["shops", 2, "departments", 1],["shops", 2, "departments", 2], + ["shops", 0, "expensiveDepartments", 0], ["shops", 0, "expensiveDepartments", 1], ["shops", 0, "expensiveDepartments", 2], ["shops", 1, "expensiveDepartments", 0], ["shops", 1, "expensiveDepartments", 1], ["shops", 1, "expensiveDepartments", 2], ["shops", 2, "expensiveDepartments", 0], ["shops", 2, "expensiveDepartments", 1],["shops", 2, "expensiveDepartments", 2], + ["expensiveShops", 0, "expensiveDepartments", 0], ["expensiveShops", 0, "expensiveDepartments", 1], ["expensiveShops", 0, "expensiveDepartments", 2], ["expensiveShops", 1, "expensiveDepartments", 0], ["expensiveShops", 1, "expensiveDepartments", 1], ["expensiveShops", 1, "expensiveDepartments", 2], ["expensiveShops", 2, "expensiveDepartments", 0], ["expensiveShops", 2, "expensiveDepartments", 1],["expensiveShops", 2, "expensiveDepartments", 2], + ["expensiveShops", 0, "departments", 0], ["expensiveShops", 0, "departments", 1], ["expensiveShops", 0, "departments", 2], ["expensiveShops", 1, "departments", 0], ["expensiveShops", 1, "departments", 1], ["expensiveShops", 1, "departments", 2], ["expensiveShops", 2, "departments", 0], ["expensiveShops", 2, "departments", 1],["expensiveShops", 2, "departments", 2]] + ) + + when: + def combined = combineExecutionResults(result.toSpecification(), incrementalResults) + then: + combined.errors == null + combined.data == expectedExpensiveData + + // TODO: Why the load counters are only 1? + batchCompareDataFetchers.departmentsForShopsBatchLoaderCounter.get() == 1 + batchCompareDataFetchers.productsForDepartmentsBatchLoaderCounter.get() == 1 + } + +} diff --git a/src/test/groovy/graphql/incremental/IncrementalExecutionResultTest.groovy b/src/test/groovy/graphql/incremental/IncrementalExecutionResultTest.groovy index 9bf954cb63..b40bfd8712 100644 --- a/src/test/groovy/graphql/incremental/IncrementalExecutionResultTest.groovy +++ b/src/test/groovy/graphql/incremental/IncrementalExecutionResultTest.groovy @@ -1,9 +1,7 @@ package graphql.incremental import graphql.execution.ResultPath -import groovy.json.JsonOutput import io.reactivex.Flowable -import org.reactivestreams.Publisher import spock.lang.Specification import static graphql.incremental.DeferPayload.newDeferredItem @@ -120,4 +118,18 @@ class IncrementalExecutionResultTest extends Specification { newIncrementalExecutionResult.hasNext() == incrementalExecutionResult.hasNext() newIncrementalExecutionResult.toSpecification() == incrementalExecutionResult.toSpecification() } + + def "transform returns IncrementalExecutionResult"() { + when: + def initial = newIncrementalExecutionResult().hasNext(true).build() + + then: + def transformed = initial.transform { b -> + b.addExtension("ext-key", "ext-value") + b.hasNext(false) + } + transformed instanceof IncrementalExecutionResult + transformed.extensions == ["ext-key": "ext-value"] + transformed.hasNext == false + } }