From 429f6f44bf0b7e41c01d73e9b9d7612351ed53da Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Mon, 4 Oct 2021 09:38:24 +0100 Subject: [PATCH] Polishing Show map-based batch loading first which is easier to implement without having to order results. Use JUnit named arguments in BatchMappingInvocationTests. See gh-130 --- build.gradle | 2 +- .../src/docs/asciidoc/index.adoc | 20 ++--- .../support/BatchMappingInvocationTests.java | 74 ++++++++++--------- 3 files changed, 50 insertions(+), 46 deletions(-) diff --git a/build.gradle b/build.gradle index c00486a0..fba83701 100644 --- a/build.gradle +++ b/build.gradle @@ -63,7 +63,7 @@ configure(moduleProjects) { mavenBom "org.springframework.security:spring-security-bom:5.5.2" mavenBom "org.jetbrains.kotlin:kotlin-bom:1.5.31" mavenBom "org.jetbrains.kotlinx:kotlinx-coroutines-bom:1.5.2" - mavenBom "org.junit:junit-bom:5.7.2" + mavenBom "org.junit:junit-bom:5.8.1" } dependencies { dependency "com.graphql-java:graphql-java:${graphQlJavaVersion}" diff --git a/spring-graphql-docs/src/docs/asciidoc/index.adoc b/spring-graphql-docs/src/docs/asciidoc/index.adoc index ddc5dcf1..63deae5d 100644 --- a/spring-graphql-docs/src/docs/asciidoc/index.adoc +++ b/spring-graphql-docs/src/docs/asciidoc/index.adoc @@ -538,8 +538,8 @@ can be loaded together. For example: public class BookController { public BookController(BatchLoaderRegistry registry) { - registry.forTypePair(Long.class, Author.class).registerBatchLoader((authorIds, environment) -> { - // how to load authors for the given author id's... + registry.forTypePair(Long.class, Author.class).registerMappedBatchLoader((authorIds, env) -> { + // return Map }); } @@ -561,7 +561,7 @@ boilerplate that can be avoided with a `@BatchMapping` method. For example: public class BookController { @BatchMapping - public Flux author(List books) { + public Mono> author(List books) { // ... } } @@ -582,8 +582,8 @@ the simple class name of the input `List` element type. Both can be customized t annotation attributes. The type name can also be inherited from a class level `@SchemaMapping`. -A `@BatchMapping` method can be a -{javadoc}/org/springframework/graphql/execution/BatchLoaderRegistry.RegistrationSpec.html#registerMappedBatchLoader-java.util.function.BiFunction-[mapped batch loading] function: +A `@BatchMapping` method can also return a sequence of instances, and that needs to match +the order of the source/parent objects: [source,java,indent=0,subs="verbatim,quotes"] ---- @@ -591,14 +591,14 @@ A `@BatchMapping` method can be a public class BookController { @BatchMapping - public Mono> author(List books) { + public Flux author(List books) { // ... } } ---- -It is possible to use imperative method signatures too, i.e. returning `List` or -`Map`, which can be useful when there are no remote calls to make. +It is possible to use imperative method signatures too, i.e. returning `Map` or +`List`, which can be useful when there are no remote calls to make. `BatchMapping` methods support two types of arguments: @@ -741,8 +741,8 @@ method argument of type `DataLoader` and use it to load the entity: public class BookController { public BookController(BatchLoaderRegistry registry) { - registry.forTypePair(Long.class, Author.class).registerBatchLoader((authorIds, env) -> { - // load authors + registry.forTypePair(Long.class, Author.class).registerMappedBatchLoader((authorIds, env) -> { + // return Map }); } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/BatchMappingInvocationTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/BatchMappingInvocationTests.java index 3595e6f2..4949f23a 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/BatchMappingInvocationTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/BatchMappingInvocationTests.java @@ -25,9 +25,11 @@ import java.util.Map; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; +import java.util.stream.Stream; import graphql.ExecutionResult; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -47,6 +49,8 @@ import org.springframework.lang.Nullable; import org.springframework.stereotype.Controller; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Named.named; +import static org.junit.jupiter.params.provider.Arguments.arguments; /** * Test GraphQL requests handled through {@code @BatchMapping} methods. @@ -91,13 +95,13 @@ public class BatchMappingInvocationTests { "}"; - private static Class[] controllerClasses() { - return new Class[] { - BatchFluxController.class, - BatchListController.class, - BatchMonoMapController.class, - BatchMapController.class - }; + private static Stream controllerClasses() { + return Stream.of( + arguments(named("Returning Mono>", BatchMonoMapController.class)), + arguments(named("Returning Map", BatchMapController.class)), + arguments(named("Returning Flux", BatchFluxController.class)), + arguments(named("Returning List", BatchListController.class)) + ); } @ParameterizedTest @@ -198,34 +202,6 @@ public class BatchMappingInvocationTests { } } - @Controller - private static class BatchFluxController extends CourseController { - - @BatchMapping - public Flux instructor(List courses) { - return Flux.fromIterable(courses).map(Course::instructor); - } - - @BatchMapping - public Flux> students(List courses) { - return Flux.fromIterable(courses).map(Course::students); - } - } - - @Controller - private static class BatchListController extends CourseController { - - @BatchMapping - public List instructor(List courses) { - return courses.stream().map(Course::instructor).collect(Collectors.toList()); - } - - @BatchMapping - public List> students(List courses) { - return courses.stream().map(Course::students).collect(Collectors.toList()); - } - } - @Controller private static class BatchMonoMapController extends CourseController { @@ -256,6 +232,34 @@ public class BatchMappingInvocationTests { } } + @Controller + private static class BatchFluxController extends CourseController { + + @BatchMapping + public Flux instructor(List courses) { + return Flux.fromIterable(courses).map(Course::instructor); + } + + @BatchMapping + public Flux> students(List courses) { + return Flux.fromIterable(courses).map(Course::students); + } + } + + @Controller + private static class BatchListController extends CourseController { + + @BatchMapping + public List instructor(List courses) { + return courses.stream().map(Course::instructor).collect(Collectors.toList()); + } + + @BatchMapping + public List> students(List courses) { + return courses.stream().map(Course::students).collect(Collectors.toList()); + } + } + private static class CourseConfig {