From dab05aa4eb4c57a428ef00627867ce7761e07a32 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Tue, 13 May 2025 18:42:10 +1000 Subject: [PATCH 1/4] Add DataLoader 5.0.0 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 5eeb73d10a..ff59f0096a 100644 --- a/build.gradle +++ b/build.gradle @@ -105,7 +105,7 @@ tasks.withType(GroovyCompile) { } dependencies { implementation 'org.antlr:antlr4-runtime:' + antlrVersion - api 'com.graphql-java:java-dataloader:4.0.0' + api 'com.graphql-java:java-dataloader:5.0.0' api 'org.reactivestreams:reactive-streams:' + reactiveStreamsVersion api "org.jspecify:jspecify:1.0.0" antlr 'org.antlr:antlr4:' + antlrVersion From 0a513fcdf5bed2368b9a2282f2b7d0c3db5333d2 Mon Sep 17 00:00:00 2001 From: bbaker Date: Wed, 14 May 2025 22:15:04 +1000 Subject: [PATCH 2/4] Fixed up simple compile stuff for DL --- .../instrumentation/DataLoaderCacheCanBeAsyncTest.groovy | 2 +- .../dataloader/DataLoaderCompanyProductBackend.java | 6 ++++-- .../dataloader/DataLoaderHangingTest.groovy | 2 +- .../instrumentation/dataloader/DataLoaderNodeTest.groovy | 7 +++++-- .../dataloader/StarWarsDataLoaderWiring.groovy | 2 +- src/test/groovy/readme/DataLoaderBatchingExamples.java | 4 ++-- 6 files changed, 14 insertions(+), 9 deletions(-) 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/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/DataLoaderHangingTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderHangingTest.groovy index 2d98da377f..580555d07e 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 diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderNodeTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderNodeTest.groovy index dd4be355f7..70642de4f1 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 @@ -85,7 +86,8 @@ class DataLoaderNodeTest extends Specification { List> nodeLoads = [] - DataLoader> loader = new DataLoader<>({ keys -> + + def closure = { keys -> nodeLoads.add(keys) List> childNodes = new ArrayList<>() for (Node key : keys) { @@ -93,7 +95,8 @@ class DataLoaderNodeTest extends Specification { } System.out.println("BatchLoader called for " + keys + " -> got " + childNodes) return CompletableFuture.completedFuture(childNodes) - }) + } + DataLoader> loader = DataLoaderFactory.newDataLoader(closure) DataFetcher nodeDataFetcher = new NodeDataFetcher(loader) diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/StarWarsDataLoaderWiring.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/StarWarsDataLoaderWiring.groovy index 3bc4848e98..8b45dfe857 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/StarWarsDataLoaderWiring.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/StarWarsDataLoaderWiring.groovy @@ -60,7 +60,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/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 From 54ec195afe804023923520857a584caa6aab401a Mon Sep 17 00:00:00 2001 From: bbaker Date: Thu, 15 May 2025 09:45:59 +1000 Subject: [PATCH 3/4] Stop referencing the DL outside the registry --- ...ataLoaderCompanyProductMutationTest.groovy | 2 +- .../dataloader/DataLoaderHangingTest.groovy | 2 +- .../dataloader/DataLoaderNodeTest.groovy | 19 +++++++++---------- .../StarWarsDataLoaderWiring.groovy | 1 + 4 files changed, 12 insertions(+), 12 deletions(-) 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 580555d07e..00dd49a098 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderHangingTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderHangingTest.groovy @@ -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 70642de4f1..16d00be727 100644 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderNodeTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderNodeTest.groovy @@ -70,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()) } } @@ -87,7 +87,7 @@ class DataLoaderNodeTest extends Specification { List> nodeLoads = [] - def closure = { keys -> + def batchLoadFunction = { keys -> nodeLoads.add(keys) List> childNodes = new ArrayList<>() for (Node key : keys) { @@ -96,15 +96,16 @@ class DataLoaderNodeTest extends Specification { System.out.println("BatchLoader called for " + keys + " -> got " + childNodes) return CompletableFuture.completedFuture(childNodes) } - DataLoader> loader = DataLoaderFactory.newDataLoader(closure) - - 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) @@ -135,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/StarWarsDataLoaderWiring.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/StarWarsDataLoaderWiring.groovy index 8b45dfe857..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 From 8b7de8932f603db09a688d97a6076adfc27ada59 Mon Sep 17 00:00:00 2001 From: bbaker Date: Thu, 15 May 2025 10:14:22 +1000 Subject: [PATCH 4/4] Stop referencing the DL outside the registry and other tweaks --- .../dataloader/BatchCompareDataFetchers.java | 8 ++++---- .../DataLoaderTypeMismatchTest.groovy | 5 +++-- .../Issue1178DataLoaderDispatchTest.groovy | 17 +++++++++-------- ...pleCompaniesAndProductsDataLoaderTest.groovy | 5 +++-- .../DataFetchingEnvironmentImplTest.groovy | 4 ++-- 5 files changed, 21 insertions(+), 18 deletions(-) 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/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/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() }