diff --git a/src/main/java/graphql/execution/DataFetcherResult.java b/src/main/java/graphql/execution/DataFetcherResult.java index cbf0a03639..cfe6337d0c 100644 --- a/src/main/java/graphql/execution/DataFetcherResult.java +++ b/src/main/java/graphql/execution/DataFetcherResult.java @@ -10,6 +10,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.function.Consumer; import java.util.function.Function; @@ -24,10 +25,19 @@ * This also allows you to pass down new local context objects between parent and child fields. If you return a * {@link #getLocalContext()} value then it will be passed down into any child fields via * {@link graphql.schema.DataFetchingEnvironment#getLocalContext()} - * + *

* You can also have {@link DataFetcher}s contribute to the {@link ExecutionResult#getExtensions()} by returning * extensions maps that will be merged together via the {@link graphql.extensions.ExtensionsBuilder} and its {@link graphql.extensions.ExtensionsMerger} * in place. + *

+ * This provides {@link #hashCode()} and {@link #equals(Object)} methods that afford comparison with other {@link DataFetcherResult} object.s + * However, to function correctly, this relies on the values provided in the following fields in turn also implementing {@link #hashCode()}} and {@link #equals(Object)} as appropriate: + *

* * @param The type of the data fetched */ @@ -123,6 +133,35 @@ public DataFetcherResult map(Function transformation) { .build(); } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) { + return false; + } + + DataFetcherResult that = (DataFetcherResult) o; + return Objects.equals(data, that.data) + && errors.equals(that.errors) + && Objects.equals(localContext, that.localContext) + && Objects.equals(extensions, that.extensions); + } + + @Override + public int hashCode() { + return Objects.hash(data, errors, localContext, extensions); + } + + @Override + public String toString() { + return "DataFetcherResult{" + + "data=" + data + + ", errors=" + errors + + ", localContext=" + localContext + + ", extensions=" + extensions + + '}'; + } + /** * Creates a new data fetcher result builder * diff --git a/src/test/groovy/graphql/execution/DataFetcherResultTest.groovy b/src/test/groovy/graphql/execution/DataFetcherResultTest.groovy index 35fbfe2f1d..07318afa75 100644 --- a/src/test/groovy/graphql/execution/DataFetcherResultTest.groovy +++ b/src/test/groovy/graphql/execution/DataFetcherResultTest.groovy @@ -1,5 +1,6 @@ package graphql.execution +import graphql.GraphQLError import graphql.InvalidSyntaxError import graphql.validation.ValidationError import graphql.validation.ValidationErrorType @@ -107,4 +108,77 @@ class DataFetcherResultTest extends Specification { result.getExtensions() == [a : "b"] result.getErrors() == [error1, error2] } + + def "implements equals/hashCode for matching results"() { + when: + def firstResult = toDataFetcherResult(first) + def secondResult = toDataFetcherResult(second) + + then: + firstResult == secondResult + firstResult.hashCode() == secondResult.hashCode() + + where: + first | second + [data: "A string"] | [data: "A string"] + [data: 5] | [data: 5] + [data: ["a", "b"]] | [data: ["a", "b"]] + [errors: [error("An error")]] | [errors: [error("An error")]] + [data: "A value", errors: [error("An error")]] | [data: "A value", errors: [error("An error")]] + [data: "A value", localContext: 5] | [data: "A value", localContext: 5] + [data: "A value", errors: [error("An error")], localContext: 5] | [data: "A value", errors: [error("An error")], localContext: 5] + [data: "A value", extensions: ["key": "value"]] | [data: "A value", extensions: ["key": "value"]] + [data: "A value", errors: [error("An error")], localContext: 5, extensions: ["key": "value"]] | [data: "A value", errors: [error("An error")], localContext: 5, extensions: ["key": "value"]] + } + + def "implements equals/hashCode for different results"() { + when: + def firstResult = toDataFetcherResult(first) + def secondResult = toDataFetcherResult(second) + + then: + firstResult != secondResult + firstResult.hashCode() != secondResult.hashCode() + + where: + first | second + [data: "A string"] | [data: "A different string"] + [data: 5] | [data: "not 5"] + [data: ["a", "b"]] | [data: ["a", "c"]] + [errors: [error("An error")]] | [errors: [error("A different error")]] + [data: "A value", errors: [error("An error")]] | [data: "A different value", errors: [error("An error")]] + [data: "A value", localContext: 5] | [data: "A value", localContext: 1] + [data: "A value", errors: [error("An error")], localContext: 5] | [data: "A value", errors: [error("A different error")], localContext: 5] + [data: "A value", extensions: ["key": "value"]] | [data: "A value", extensions: ["key", "different value"]] + [data: "A value", errors: [error("An error")], localContext: 5, extensions: ["key": "value"]] | [data: "A value", errors: [error("An error")], localContext: 5, extensions: ["key": "different value"]] + } + + private static DataFetcherResult toDataFetcherResult(Map resultFields) { + def resultBuilder = DataFetcherResult.newResult(); + resultFields.forEach { key, value -> + if (value != null) { + switch (key) { + case "data": + resultBuilder.data(value) + break; + case "errors": + resultBuilder.errors(value as List); + break; + case "localContext": + resultBuilder.localContext(value); + break; + case "extensions": + resultBuilder.extensions(value as Map); + break; + } + } + } + return resultBuilder.build(); + } + + private static GraphQLError error(String message) { + return GraphQLError.newError() + .message(message) + .build(); + } } diff --git a/src/test/groovy/graphql/execution/instrumentation/DataLoaderCacheCanBeAsyncTest.groovy b/src/test/groovy/graphql/execution/instrumentation/DataLoaderCacheCanBeAsyncTest.groovy index 3b57bf9780..9aebce3640 100644 --- a/src/test/groovy/graphql/execution/instrumentation/DataLoaderCacheCanBeAsyncTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/DataLoaderCacheCanBeAsyncTest.groovy @@ -87,7 +87,7 @@ class DataLoaderCacheCanBeAsyncTest extends Specification { def valueCache = new CustomValueCache() valueCache.store.put("a", [id: "cachedA", name: "cachedAName"]) - DataLoaderOptions options = DataLoaderOptions.newOptions().setValueCache(valueCache).setCachingEnabled(true) + DataLoaderOptions options = DataLoaderOptions.newOptions().setValueCache(valueCache).setCachingEnabled(true).build() DataLoader userDataLoader = DataLoaderFactory.newDataLoader(userBatchLoader, options) registry = DataLoaderRegistry.newRegistry() diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/BatchCompareDataFetchers.java b/src/test/groovy/graphql/execution/instrumentation/dataloader/BatchCompareDataFetchers.java index d0dacd5964..3c43b94b5b 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/BatchCompareDataFetchers.java +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/BatchCompareDataFetchers.java @@ -100,9 +100,9 @@ private static List> getDepartmentsForShops(List shops) { public DataLoader> departmentsForShopDataLoader = DataLoaderFactory.newDataLoader(departmentsForShopsBatchLoader); - public DataFetcher>> departmentsForShopDataLoaderDataFetcher = environment -> { + public DataFetcher departmentsForShopDataLoaderDataFetcher = environment -> { Shop shop = environment.getSource(); - return departmentsForShopDataLoader.load(shop.getId()); + return environment.getDataLoader("departments").load(shop.getId()); }; // Products @@ -136,9 +136,9 @@ private static List> getProductsForDepartments(List de public DataLoader> productsForDepartmentDataLoader = DataLoaderFactory.newDataLoader(productsForDepartmentsBatchLoader); - public DataFetcher>> productsForDepartmentDataLoaderDataFetcher = environment -> { + public DataFetcher productsForDepartmentDataLoaderDataFetcher = environment -> { Department department = environment.getSource(); - return productsForDepartmentDataLoader.load(department.getId()); + return environment.getDataLoader("products").load(department.getId()); }; private CompletableFuture maybeAsyncWithSleep(Supplier> supplier) { diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderCompanyProductBackend.java b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderCompanyProductBackend.java index 51d2353bf7..14d2f425c8 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderCompanyProductBackend.java +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderCompanyProductBackend.java @@ -2,6 +2,7 @@ import com.google.common.collect.ImmutableList; +import org.dataloader.BatchLoader; import org.dataloader.DataLoader; import org.dataloader.DataLoaderFactory; @@ -26,12 +27,13 @@ public DataLoaderCompanyProductBackend(int companyCount, int projectCount) { mkCompany(projectCount); } - projectsLoader = DataLoaderFactory.newDataLoader(keys -> getProjectsForCompanies(keys).thenApply(projects -> keys + BatchLoader> uuidListBatchLoader = keys -> getProjectsForCompanies(keys).thenApply(projects -> keys .stream() .map(companyId -> projects.stream() .filter(project -> project.getCompanyId().equals(companyId)) .collect(Collectors.toList())) - .collect(Collectors.toList()))); + .collect(Collectors.toList())); + projectsLoader = DataLoaderFactory.newDataLoader(uuidListBatchLoader); } diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderCompanyProductMutationTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderCompanyProductMutationTest.groovy index 649da5e0d4..33cbb86d1f 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderCompanyProductMutationTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderCompanyProductMutationTest.groovy @@ -48,7 +48,7 @@ class DataLoaderCompanyProductMutationTest extends Specification { newTypeWiring("Company").dataFetcher("projects", { environment -> DataLoaderCompanyProductBackend.Company source = environment.getSource() - return backend.getProjectsLoader().load(source.getId()) + return environment.getDataLoader("projects-dl").load(source.getId()) })) .type( newTypeWiring("Query").dataFetcher("companies", { diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderHangingTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderHangingTest.groovy index 2d98da377f..00dd49a098 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderHangingTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderHangingTest.groovy @@ -192,7 +192,7 @@ class DataLoaderHangingTest extends Specification { }) }, executor) } - }, DataLoaderOptions.newOptions().setMaxBatchSize(5)) + }, DataLoaderOptions.newOptions().setMaxBatchSize(5).build()) def dataLoaderSongs = DataLoaderFactory.newDataLoader(new BatchLoader>() { @Override @@ -209,7 +209,7 @@ class DataLoaderHangingTest extends Specification { }) }, executor) } - }, DataLoaderOptions.newOptions().setMaxBatchSize(5)) + }, DataLoaderOptions.newOptions().setMaxBatchSize(5).build()) def dataLoaderRegistry = new DataLoaderRegistry() dataLoaderRegistry.register("artist.albums", dataLoaderAlbums) diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderNodeTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderNodeTest.groovy index dd4be355f7..16d00be727 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderNodeTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderNodeTest.groovy @@ -11,6 +11,7 @@ import graphql.schema.GraphQLObjectType import graphql.schema.GraphQLSchema import graphql.schema.StaticDataFetcher import org.dataloader.DataLoader +import org.dataloader.DataLoaderFactory import org.dataloader.DataLoaderRegistry import spock.lang.Specification @@ -69,15 +70,15 @@ class DataLoaderNodeTest extends Specification { } class NodeDataFetcher implements DataFetcher { - DataLoader loader + String name - NodeDataFetcher(DataLoader loader) { - this.loader = loader + NodeDataFetcher(String name) { + this.name = name } @Override Object get(DataFetchingEnvironment environment) throws Exception { - return loader.load(environment.getSource()) + return environment.getDataLoader(name).load(environment.getSource()) } } @@ -85,7 +86,8 @@ class DataLoaderNodeTest extends Specification { List> nodeLoads = [] - DataLoader> loader = new DataLoader<>({ keys -> + + def batchLoadFunction = { keys -> nodeLoads.add(keys) List> childNodes = new ArrayList<>() for (Node key : keys) { @@ -93,15 +95,17 @@ class DataLoaderNodeTest extends Specification { } System.out.println("BatchLoader called for " + keys + " -> got " + childNodes) return CompletableFuture.completedFuture(childNodes) - }) - - DataFetcher nodeDataFetcher = new NodeDataFetcher(loader) + } + DataLoader> loader = DataLoaderFactory.newDataLoader(batchLoadFunction) def nodeTypeName = "Node" def childNodesFieldName = "childNodes" def queryTypeName = "Query" def rootFieldName = "root" + DataFetcher nodeDataFetcher = new NodeDataFetcher(childNodesFieldName) + DataLoaderRegistry registry = new DataLoaderRegistry().register(childNodesFieldName, loader) + GraphQLObjectType nodeType = GraphQLObjectType .newObject() .name(nodeTypeName) @@ -132,8 +136,6 @@ class DataLoaderNodeTest extends Specification { .build()) .build() - DataLoaderRegistry registry = new DataLoaderRegistry().register(childNodesFieldName, loader) - ExecutionResult result = GraphQL.newGraphQL(schema) // .instrumentation(new DataLoaderDispatcherInstrumentation()) .build() diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderTypeMismatchTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderTypeMismatchTest.groovy index 03b60e4e39..5b6f1cfb2f 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderTypeMismatchTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderTypeMismatchTest.groovy @@ -9,6 +9,7 @@ import graphql.schema.idl.SchemaGenerator import graphql.schema.idl.SchemaParser import org.dataloader.BatchLoader import org.dataloader.DataLoader +import org.dataloader.DataLoaderFactory import org.dataloader.DataLoaderRegistry import spock.lang.Specification @@ -36,7 +37,7 @@ class DataLoaderTypeMismatchTest extends Specification { def typeDefinitionRegistry = new SchemaParser().parse(sdl) - def dataLoader = new DataLoader(new BatchLoader() { + def dataLoader = DataLoaderFactory.newDataLoader(new BatchLoader() { @Override CompletionStage> load(List keys) { return CompletableFuture.completedFuture([ @@ -50,7 +51,7 @@ class DataLoaderTypeMismatchTest extends Specification { def todosDef = new DataFetcher>() { @Override CompletableFuture get(DataFetchingEnvironment environment) { - return dataLoader.load(environment) + return environment.getDataLoader("getTodos").load(environment) } } diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/Issue1178DataLoaderDispatchTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/Issue1178DataLoaderDispatchTest.groovy index b816602cde..135b52ba8d 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/Issue1178DataLoaderDispatchTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/Issue1178DataLoaderDispatchTest.groovy @@ -8,6 +8,7 @@ import graphql.schema.StaticDataFetcher import graphql.schema.idl.RuntimeWiring import org.dataloader.BatchLoader import org.dataloader.DataLoader +import org.dataloader.DataLoaderFactory import org.dataloader.DataLoaderRegistry import spock.lang.Specification @@ -40,7 +41,7 @@ class Issue1178DataLoaderDispatchTest extends Specification { def executor = Executors.newFixedThreadPool(5) - def dataLoader = new DataLoader(new BatchLoader() { + def dataLoader = DataLoaderFactory.newDataLoader(new BatchLoader() { @Override CompletionStage> load(List keys) { return CompletableFuture.supplyAsync({ @@ -48,7 +49,7 @@ class Issue1178DataLoaderDispatchTest extends Specification { }, executor) } }) - def dataLoader2 = new DataLoader(new BatchLoader() { + def dataLoader2 = DataLoaderFactory.newDataLoader(new BatchLoader() { @Override CompletionStage> load(List keys) { return CompletableFuture.supplyAsync({ @@ -61,8 +62,8 @@ class Issue1178DataLoaderDispatchTest extends Specification { dataLoaderRegistry.register("todo.related", dataLoader) dataLoaderRegistry.register("todo.related2", dataLoader2) - def relatedDf = new MyDataFetcher(dataLoader) - def relatedDf2 = new MyDataFetcher(dataLoader2) + def relatedDf = new MyDataFetcher("todo.related") + def relatedDf2 = new MyDataFetcher("todo.related2") def wiring = RuntimeWiring.newRuntimeWiring() .type(newTypeWiring("Query") @@ -119,16 +120,16 @@ class Issue1178DataLoaderDispatchTest extends Specification { static class MyDataFetcher implements DataFetcher> { - private final DataLoader dataLoader + private final String name - MyDataFetcher(DataLoader dataLoader) { - this.dataLoader = dataLoader + MyDataFetcher(String name) { + this.name = name } @Override CompletableFuture get(DataFetchingEnvironment environment) { def todo = environment.source as Map - return dataLoader.load(todo['id']) + return environment.getDataLoader(name).load(todo['id']) } } } diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/PeopleCompaniesAndProductsDataLoaderTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/PeopleCompaniesAndProductsDataLoaderTest.groovy index 70bad946b0..0cc9a73c40 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/PeopleCompaniesAndProductsDataLoaderTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/PeopleCompaniesAndProductsDataLoaderTest.groovy @@ -12,6 +12,7 @@ import graphql.schema.DataFetchingEnvironment import graphql.schema.idl.RuntimeWiring import org.dataloader.BatchLoader import org.dataloader.DataLoader +import org.dataloader.DataLoaderFactory import org.dataloader.DataLoaderRegistry import spock.lang.Specification @@ -168,8 +169,8 @@ class PeopleCompaniesAndProductsDataLoaderTest extends Specification { private DataLoaderRegistry buildRegistry() { - DataLoader personDataLoader = new DataLoader<>(personBatchLoader) - DataLoader companyDataLoader = new DataLoader<>(companyBatchLoader) + DataLoader personDataLoader = DataLoaderFactory.newDataLoader(personBatchLoader) + DataLoader companyDataLoader = DataLoaderFactory.newDataLoader(companyBatchLoader) DataLoaderRegistry registry = new DataLoaderRegistry() registry.register("person", personDataLoader) diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/StarWarsDataLoaderWiring.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/StarWarsDataLoaderWiring.groovy index 3bc4848e98..757d5564a0 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/StarWarsDataLoaderWiring.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/StarWarsDataLoaderWiring.groovy @@ -10,6 +10,7 @@ import graphql.schema.idl.MapEnumValuesProvider import graphql.schema.idl.RuntimeWiring import org.dataloader.BatchLoader import org.dataloader.DataLoader +import org.dataloader.DataLoaderFactory import org.dataloader.DataLoaderRegistry import java.util.concurrent.CompletableFuture @@ -60,7 +61,7 @@ class StarWarsDataLoaderWiring { def newDataLoaderRegistry() { // a data loader for characters that points to the character batch loader - def characterDataLoader = new DataLoader(characterBatchLoader) + def characterDataLoader = DataLoaderFactory.newDataLoader(characterBatchLoader) new DataLoaderRegistry().register("character", characterDataLoader) } diff --git a/src/test/groovy/graphql/schema/DataFetchingEnvironmentImplTest.groovy b/src/test/groovy/graphql/schema/DataFetchingEnvironmentImplTest.groovy index 2830419999..36aa87eb54 100644 --- a/src/test/groovy/graphql/schema/DataFetchingEnvironmentImplTest.groovy +++ b/src/test/groovy/graphql/schema/DataFetchingEnvironmentImplTest.groovy @@ -73,7 +73,7 @@ class DataFetchingEnvironmentImplTest extends Specification { dfe.getVariables() == variables dfe.getOperationDefinition() == operationDefinition dfe.getExecutionId() == executionId - dfe.getDataLoader("dataLoader") == dataLoader + dfe.getDataLoader("dataLoader") != null } def "create environment from existing one will copy everything to new instance"() { @@ -118,7 +118,7 @@ class DataFetchingEnvironmentImplTest extends Specification { dfe.getDocument() == dfeCopy.getDocument() dfe.getOperationDefinition() == dfeCopy.getOperationDefinition() dfe.getVariables() == dfeCopy.getVariables() - dfe.getDataLoader("dataLoader") == dataLoader + dfe.getDataLoader("dataLoader") != null dfe.getLocale() == dfeCopy.getLocale() dfe.getLocalContext() == dfeCopy.getLocalContext() } diff --git a/src/test/groovy/readme/DataLoaderBatchingExamples.java b/src/test/groovy/readme/DataLoaderBatchingExamples.java index 287d4c5650..1bf55e0452 100644 --- a/src/test/groovy/readme/DataLoaderBatchingExamples.java +++ b/src/test/groovy/readme/DataLoaderBatchingExamples.java @@ -171,7 +171,7 @@ public CompletableFuture clear() { } }; - DataLoaderOptions options = DataLoaderOptions.newOptions().setValueCache(crossRequestValueCache); + DataLoaderOptions options = DataLoaderOptions.newOptions().setValueCache(crossRequestValueCache).build(); DataLoader dataLoader = DataLoaderFactory.newDataLoader(batchLoader, options); } @@ -260,7 +260,7 @@ public Object getContext() { // // this creates an overall context for the dataloader // - DataLoaderOptions loaderOptions = DataLoaderOptions.newOptions().setBatchLoaderContextProvider(contextProvider); + DataLoaderOptions loaderOptions = DataLoaderOptions.newOptions().setBatchLoaderContextProvider(contextProvider).build(); DataLoader characterDataLoader = DataLoaderFactory.newDataLoader(batchLoaderWithCtx, loaderOptions); // .... later in your data fetcher