diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistry.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistry.java index 891c7edc..8a52466e 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistry.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistry.java @@ -43,9 +43,9 @@ import org.springframework.util.Assert; import org.springframework.util.StringUtils; /** - * A default implementation of {@link BatchLoaderRegistry} that accepts - * registrations, and also an implementation of {@link DataLoaderRegistrar} to - * apply those registrations to a {@link DataLoaderRegistry}. + * Default implementation of {@link BatchLoaderRegistry} that stores batch loader + * registrations. Also, an implementation of {@link DataLoaderRegistrar} that + * registers the batch loaders as {@link DataLoader}s in {@link DataLoaderRegistry}. * * @author Rossen Stoyanchev * @since 1.0.0 @@ -60,7 +60,7 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry { /** - * Default constructor + * Default constructor. */ public DefaultBatchLoaderRegistry() { this(DataLoaderOptions::newOptions); @@ -68,10 +68,11 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry { /** * Constructor with a default {@link DataLoaderOptions} supplier to use as - * a starting point for all registrations. - * @since 1.1 + * a starting point for batch loader registrations. + * @since 1.1.0 */ public DefaultBatchLoaderRegistry(Supplier defaultOptionsSupplier) { + Assert.notNull(defaultOptionsSupplier, "'defaultOptionsSupplier' is required"); this.defaultOptionsSupplier = defaultOptionsSupplier; } @@ -89,16 +90,15 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry { @Override public void registerDataLoaders(DataLoaderRegistry registry, GraphQLContext context) { BatchLoaderContextProvider contextProvider = () -> context; - DataLoaderOptions defaultOptions = this.defaultOptionsSupplier.get(); for (ReactorBatchLoader loader : this.loaders) { DataLoaderOptions options = loader.getOptions(); - options = (options != null ? options : defaultOptions).setBatchLoaderContextProvider(contextProvider); + options = options.setBatchLoaderContextProvider(contextProvider); DataLoader dataLoader = DataLoaderFactory.newDataLoader(loader, options); registerDataLoader(loader.getName(), dataLoader, registry); } for (ReactorMappedBatchLoader loader : this.mappedLoaders) { DataLoaderOptions options = loader.getOptions(); - options = (options != null ? options : defaultOptions).setBatchLoaderContextProvider(contextProvider); + options = options.setBatchLoaderContextProvider(contextProvider); DataLoader dataLoader = DataLoaderFactory.newMappedDataLoader(loader, options); registerDataLoader(loader.getName(), dataLoader, registry); } @@ -166,14 +166,19 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry { new ReactorMappedBatchLoader<>(initName(), loader, initOptionsSupplier())); } - @Nullable - private Supplier initOptionsSupplier() { - if (this.options == null && this.optionsConsumer == null) { - return null; + private String initName() { + if (StringUtils.hasText(this.name)) { + return this.name; } + Assert.notNull(this.valueType, "Value type not available to select a default DataLoader name."); + return (StringUtils.hasText(this.name) ? this.name : this.valueType.getName()); + } - Supplier optionsSupplier = - (this.options != null ? () -> this.options : defaultOptionsSupplier); + private Supplier initOptionsSupplier() { + + Supplier optionsSupplier = () -> + new DataLoaderOptions(this.options != null ? + this.options : DefaultBatchLoaderRegistry.this.defaultOptionsSupplier.get()); if (this.optionsConsumer == null) { return optionsSupplier; @@ -185,14 +190,6 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry { return options; }; } - - private String initName() { - if (StringUtils.hasText(this.name)) { - return this.name; - } - Assert.notNull(this.valueType, "Value type not available to select a default DataLoader name."); - return (StringUtils.hasText(this.name) ? this.name : this.valueType.getName()); - } } @@ -206,12 +203,11 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry { private final BiFunction, BatchLoaderEnvironment, Flux> loader; - @Nullable private final Supplier optionsSupplier; private ReactorBatchLoader(String name, BiFunction, BatchLoaderEnvironment, Flux> loader, - @Nullable Supplier optionsSupplier) { + Supplier optionsSupplier) { this.name = name; this.loader = loader; @@ -222,9 +218,8 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry { return this.name; } - @Nullable public DataLoaderOptions getOptions() { - return (this.optionsSupplier != null ? this.optionsSupplier.get() : null); + return this.optionsSupplier.get(); } @Override @@ -257,12 +252,11 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry { private final BiFunction, BatchLoaderEnvironment, Mono>> loader; - @Nullable private final Supplier optionsSupplier; private ReactorMappedBatchLoader(String name, BiFunction, BatchLoaderEnvironment, Mono>> loader, - @Nullable Supplier optionsSupplier) { + Supplier optionsSupplier) { this.name = name; this.loader = loader; @@ -273,9 +267,8 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry { return this.name; } - @Nullable public DataLoaderOptions getOptions() { - return (this.optionsSupplier != null ? this.optionsSupplier.get() : null); + return this.optionsSupplier.get(); } @Override diff --git a/spring-graphql/src/test/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistryTests.java b/spring-graphql/src/test/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistryTests.java index 2bdf379a..08e2d414 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistryTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/execution/DefaultBatchLoaderRegistryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.springframework.graphql.execution; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; @@ -101,7 +102,41 @@ public class DefaultBatchLoaderRegistryTests { } @Test - void batchLoaderWithCustomNameAndOptions() { + void dataLoaderOptions() throws Exception { + + DataLoaderOptions defaultOptions = DataLoaderOptions.newOptions().setBatchingEnabled(false); + DefaultBatchLoaderRegistry batchLoaderRegistry = new DefaultBatchLoaderRegistry(() -> defaultOptions); + + AtomicInteger counter = new AtomicInteger(1); + + batchLoaderRegistry.forName("loader1") + .withOptions(options -> options.setCachingEnabled(false)) + .registerBatchLoader((keys, environment) -> Flux.just(counter.getAndIncrement())); + + batchLoaderRegistry.forName("loader2") + .withOptions(options -> options.setCachingEnabled(true)) + .registerBatchLoader((keys, environment) -> Flux.just(counter.getAndIncrement())); + + GraphQLContext graphQLContext = GraphQLContext.newContext().build(); + batchLoaderRegistry.registerDataLoaders(this.dataLoaderRegistry, graphQLContext); + + DataLoader loader1 = + (DataLoader) this.dataLoaderRegistry.getDataLoadersMap().get("loader1"); + + assertThat(loader1.load(1L).get()).isEqualTo(1); + assertThat(loader1.load(1L).get()).isEqualTo(2); + assertThat(loader1.load(1L).get()).isEqualTo(3); + + DataLoader loader2 = + (DataLoader) this.dataLoaderRegistry.getDataLoadersMap().get("loader2"); + + assertThat(loader2.load(1L).get()).isEqualTo(4); + assertThat(loader2.load(1L).get()).isEqualTo(4); + assertThat(loader2.load(1L).get()).isEqualTo(4); + } + + @Test + void batchLoaderOptionsConsumer() { String name = "myLoader"; StatisticsCollector collector = new NoOpStatisticsCollector();