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

Author: rstoyanchev

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}"

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<Long, Author>
 		});
 	}
 
@@ -561,7 +561,7 @@ boilerplate that can be avoided with a `@BatchMapping` method. For example:
 	public class BookController {
 
 		@BatchMapping
-		public Flux<Author> author(List<Book> books) {
+		public Mono<Map<Book, Author>> author(List<Book> books) {
 			// ...
 		}
 	}
@@ -582,23 +582,23 @@ 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"]
 ----
 	@Controller
 	public class BookController {
 
 		@BatchMapping
-		public Mono<Map<Book, Author>> author(List<Book> books) {
+		public Flux<Author> author(List<Book> books) {
 			// ...
 		}
 	}
 ----
 
-It is possible to use imperative method signatures too, i.e. returning `List<V>` or
-`Map<K, V>`, 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<K, V>` or
+`List<V>`, 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<Long, Author>
 		});
 	}
 

spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/BatchMappingInvocationTests.java

@@ -25,9 +25,11 @@
 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.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<Arguments> controllerClasses() {
+		return Stream.of(
+				arguments(named("Returning Mono<Map<K,V>>", BatchMonoMapController.class)),
+				arguments(named("Returning Map<K,V>", BatchMapController.class)),
+				arguments(named("Returning Flux<V>", BatchFluxController.class)),
+				arguments(named("Returning List<V>", BatchListController.class))
+		);
 	}
 
 	@ParameterizedTest
@@ -199,60 +203,60 @@ public Collection<Course> courses() {
 	}
 
 	@Controller
-	private static class BatchFluxController extends CourseController {
+	private static class BatchMonoMapController extends CourseController {
 
 		@BatchMapping
-		public Flux<Person> instructor(List<Course> courses) {
-			return Flux.fromIterable(courses).map(Course::instructor);
+		public Mono<Map<Course, Person>> instructor(List<Course> courses) {
+			return Flux.fromIterable(Course.allCourses())
+					.collect(Collectors.toMap(Function.identity(), Course::instructor));
 		}
 
 		@BatchMapping
-		public Flux<List<Person>> students(List<Course> courses) {
-			return Flux.fromIterable(courses).map(Course::students);
+		public Mono<Map<Course, List<Person>>> students(Set<Course> courses) {
+			return Flux.fromIterable(courses).collect(Collectors.toMap(Function.identity(), Course::students));
 		}
 	}
 
 	@Controller
-	private static class BatchListController extends CourseController {
+	private static class BatchMapController extends CourseController {
 
 		@BatchMapping
-		public List<Person> instructor(List<Course> courses) {
-			return courses.stream().map(Course::instructor).collect(Collectors.toList());
+		public Map<Course, Person> instructor(List<Course> courses) {
+			return Course.allCourses().stream().collect(
+					Collectors.toMap(Function.identity(), Course::instructor));
 		}
 
 		@BatchMapping
-		public List<List<Person>> students(List<Course> courses) {
-			return courses.stream().map(Course::students).collect(Collectors.toList());
+		public Map<Course, List<Person>> students(List<Course> courses) {
+			return courses.stream().collect(Collectors.toMap(Function.identity(), Course::students));
 		}
 	}
 
 	@Controller
-	private static class BatchMonoMapController extends CourseController {
+	private static class BatchFluxController extends CourseController {
 
 		@BatchMapping
-		public Mono<Map<Course, Person>> instructor(List<Course> courses) {
-			return Flux.fromIterable(Course.allCourses())
-					.collect(Collectors.toMap(Function.identity(), Course::instructor));
+		public Flux<Person> instructor(List<Course> courses) {
+			return Flux.fromIterable(courses).map(Course::instructor);
 		}
 
 		@BatchMapping
-		public Mono<Map<Course, List<Person>>> students(Set<Course> courses) {
-			return Flux.fromIterable(courses).collect(Collectors.toMap(Function.identity(), Course::students));
+		public Flux<List<Person>> students(List<Course> courses) {
+			return Flux.fromIterable(courses).map(Course::students);
 		}
 	}
 
 	@Controller
-	private static class BatchMapController extends CourseController {
+	private static class BatchListController extends CourseController {
 
 		@BatchMapping
-		public Map<Course, Person> instructor(List<Course> courses) {
-			return Course.allCourses().stream().collect(
-					Collectors.toMap(Function.identity(), Course::instructor));
+		public List<Person> instructor(List<Course> courses) {
+			return courses.stream().map(Course::instructor).collect(Collectors.toList());
 		}
 
 		@BatchMapping
-		public Map<Course, List<Person>> students(List<Course> courses) {
-			return courses.stream().collect(Collectors.toMap(Function.identity(), Course::students));
+		public List<List<Person>> students(List<Course> courses) {
+			return courses.stream().map(Course::students).collect(Collectors.toList());
 		}
 	}