From f8f6a3ed9f7034e039b226859c0904c7e12d962a Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 24 Sep 2021 10:31:17 +0100 Subject: [PATCH] Register batch load function by name This is an alternative to providing the key and value types, which are only used to form the DataLoader name. See gh-130 --- .../execution/BatchLoaderRegistry.java | 34 ++++++++++++++++--- .../execution/DefaultBatchLoaderRegistry.java | 32 +++++++++++++++-- .../DataLoaderArgumentResolverTests.java | 14 +++----- .../DefaultBatchLoaderRegistryTests.java | 4 +-- 4 files changed, 63 insertions(+), 21 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/BatchLoaderRegistry.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/BatchLoaderRegistry.java index f33265c7..7187ab46 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/BatchLoaderRegistry.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/BatchLoaderRegistry.java @@ -44,16 +44,39 @@ import reactor.core.publisher.Mono; public interface BatchLoaderRegistry { /** - * Start the registration of a new function for batch loading data values by - * specifying the key and value types. - * @param keyType the type of the key that identifies the value - * @param valueType the type of the data value + * Begin the registration of a new batch load function by specifying the + * types of the keys and values that will be used as input and output. + * + *

When this method is used, the name for the + * {@link org.dataloader.DataLoader} is automatically set as defined in + * {@link RegistrationSpec#withName(String)}, and likewise, + * {@code @SchemaMapping} handler methods can transparenly locate and + * inject a {@code DataLoader} argument based on the generic type + * {@code }. + * + * @param keyType the type of keys that will be used as input + * @param valueType the type of value that will be returned as output * @param the key type * @param the value type * @return a spec to complete the registration */ RegistrationSpec forTypePair(Class keyType, Class valueType); + /** + * Begin the registration of a new batch load function by specifying the + * name for the {@link org.dataloader.DataLoader}. + * + *

Note: when this method is used, the parameter name + * of a {@code DataLoader} argument in a {@code @SchemaMapping} handler + * method needs to match the name given here. + * + * @param name the name to use to register a {@code DataLoader} + * @param the type of keys that will be used as input + * @param the type of values that will be used as output + * @return a spec to complete the registration + */ + RegistrationSpec forName(String name); + /** * Spec to complete the registration of a batch loading function. @@ -66,7 +89,8 @@ public interface BatchLoaderRegistry { /** * Customize the name under which the {@link org.dataloader.DataLoader} * is registered and can be accessed in the data layer. - *

By default, this is the full class name of the value type. + *

By default, this is the full class name of the value type, if the + * value type is specified via {@link #forTypePair(Class, Class)}. * @param name the name to use * @return a spec to complete the registration */ 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 05633e32..aa155446 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 @@ -33,6 +33,10 @@ import org.dataloader.MappedBatchLoaderWithContext; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import org.springframework.lang.Nullable; +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 @@ -50,7 +54,12 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry, DataLoad @Override public RegistrationSpec forTypePair(Class keyType, Class valueType) { - return new DefaultRegistrationSpec<>(valueType.getName()); + return new DefaultRegistrationSpec<>(valueType); + } + + @Override + public RegistrationSpec forName(String name) { + return new DefaultRegistrationSpec<>(name); } @Override @@ -75,12 +84,21 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry, DataLoad private class DefaultRegistrationSpec implements RegistrationSpec { + @Nullable + private final Class valueType; + + @Nullable private String name; private DataLoaderOptions options = DataLoaderOptions.newOptions(); + public DefaultRegistrationSpec(Class valueType) { + this.valueType = valueType; + } + public DefaultRegistrationSpec(String name) { this.name = name; + this.valueType = null; } @Override @@ -104,13 +122,21 @@ public class DefaultBatchLoaderRegistry implements BatchLoaderRegistry, DataLoad @Override public void registerBatchLoader(BiFunction, BatchLoaderEnvironment, Flux> loader) { DefaultBatchLoaderRegistry.this.loaders.add( - new ReactorBatchLoader<>(this.name, loader, this.options)); + new ReactorBatchLoader<>(initName(), loader, this.options)); } @Override public void registerMappedBatchLoader(BiFunction, BatchLoaderEnvironment, Mono>> loader) { DefaultBatchLoaderRegistry.this.mappedLoaders.add( - new ReactorMappedBatchLoader<>(this.name, loader, this.options)); + new ReactorMappedBatchLoader<>(initName(), loader, this.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()); } } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/DataLoaderArgumentResolverTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/DataLoaderArgumentResolverTests.java index 3d3be35a..4f91fd83 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/DataLoaderArgumentResolverTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/DataLoaderArgumentResolverTests.java @@ -28,7 +28,6 @@ import reactor.core.publisher.Flux; import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.MethodParameter; import org.springframework.graphql.Author; -import org.springframework.graphql.Book; import org.springframework.graphql.execution.BatchLoaderRegistry; import org.springframework.graphql.execution.DefaultBatchLoaderRegistry; import org.springframework.util.ClassUtils; @@ -57,6 +56,7 @@ public class DataLoaderArgumentResolverTests { @Test void resolveArgument() { + DataFetchingEnvironment environment = initEnvironment(registry -> registry.forTypePair(Long.class, Author.class).registerBatchLoader((ids, env) -> Flux.empty())); @@ -67,9 +67,7 @@ public class DataLoaderArgumentResolverTests { @Test void resolveArgumentViaParameterName() { DataFetchingEnvironment environment = initEnvironment(registry -> - registry.forTypePair(Long.class, Author.class) - .withName("namedDataLoader") - .registerBatchLoader((ids, env) -> Flux.empty())); + registry.forName("namedDataLoader").registerBatchLoader((ids, env) -> Flux.empty())); MethodParameter parameter = initParameter(1); parameter.initParameterNameDiscovery(new DefaultParameterNameDiscoverer()); @@ -90,9 +88,7 @@ public class DataLoaderArgumentResolverTests { @Test void resolveArgumentFailureWithoutParameterName() { DataFetchingEnvironment environment = initEnvironment(registry -> - registry.forTypePair(Long.class, Author.class) - .withName("namedDataLoader") - .registerBatchLoader((ids, env) -> Flux.empty())); + registry.forName("namedDataLoader").registerBatchLoader((ids, env) -> Flux.empty())); MethodParameter parameter = initParameter(1); // Skip ParameterNameDiscovery @@ -104,9 +100,7 @@ public class DataLoaderArgumentResolverTests { @Test void resolveArgumentFailureNoMatch() { DataFetchingEnvironment environment = initEnvironment(registry -> - registry.forTypePair(Long.class, Book.class) - .withName("bookDataLoader") - .registerBatchLoader((ids, env) -> Flux.empty())); + registry.forName("bookDataLoader").registerBatchLoader((ids, env) -> Flux.empty())); MethodParameter parameter = initParameter(0); parameter.initParameterNameDiscovery(new DefaultParameterNameDiscoverer()); 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 c4962d00..8fa44667 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 @@ -65,9 +65,7 @@ public class DefaultBatchLoaderRegistryTests { String name = "myLoader"; StatisticsCollector collector = new NoOpStatisticsCollector(); - this.batchLoaderRegistry - .forTypePair(String.class, Book.class) - .withName(name) + this.batchLoaderRegistry.forName(name) .withOptions(options -> options.setStatisticsCollector(() -> collector)) .registerBatchLoader((keys, environment) -> Flux.empty());