Add ability to add raw batch loader instances to BatchLoaderRegistry alongside registration methods

[bsara]

There are times when the registration methods for batch loaders are too limited or when using them creates code that is not in a preferred style for a project. Also, it is not possible to register a batch loader that returns a typed collection which itself is accurately typed when placed in the registry because we can't specify a value type with generics.

[bsara]

I've started implementing something that hopefully will be a good starting point for discussion around what I've proposed. I've tested it all locally and everything is working in this branch: https://github.com/spring-projects/spring-graphql/compare/main...bsara:spring-graphql:batch-loader-registry-updates. So, anyone should be able to pull it down and try it out. I don't have automated tests in place yet. I was hoping to get feedback and find out if these updates would even be considered before I went that far 😄.

Here's what I've done:

1. BatchLoaderRegistry can now register loader classes directly. Something like this, for example:

   @Component
   @RequiredArgsConstructor
   public class PersonBatchLoader implements MappedBatchLoader<String, Person> {
   
     private final MockData mockData;
   
     @Override
     public CompletionStage<Map<String, Person>> load(Set<String> personIds) {
       return supplyAsync(() -> {
         var people = mockData.getPeople();
         return personIds.stream().collect(toMap(identity(), people::get))
       });
     }
   }
   

   ...would be registered in BatchLoaderRegistry using a Spring Boot config like this:

   @Bean
   public BatchLoaderRegistry batchLoaderRegistry(
     @Autowired(required = false) DataLoaderOptions defaultDataLoaderOptions,
     Collection<BatchLoader<?, ?>> batchLoaders,
     Collection<BatchLoaderWithContext<?, ?>> batchLoadersWithContext,
     Collection<MappedBatchLoader<?, ?>> mappedBatchLoaders,
     Collection<MappedBatchLoaderWithContext<?, ?>> mappedBatchLoadersWithContext,
     Collection<SpringBatchLoaderWithOptions> batchLoadersWithOptions
   ) {
     var registry = (
       defaultDataLoaderOptions == null
         ? new DefaultBatchLoaderRegistry()
         : new DefaultBatchLoaderRegistry(() -> defaultDataLoaderOptions)
     );
   
     batchLoaders.forEach(registry::register);
     batchLoadersWithContext.forEach(registry::register);
     mappedBatchLoaders.forEach(registry::register);
     mappedBatchLoadersWithContext.forEach(registry::register);
     batchLoadersWithOptions.forEach(registry::register);
   
     return registry;
   }
   

2. By default, the names of the generated DataLoader for a batch loader class is the camelcase version of the batch loader class name. If the batch loader class name ends in BatchLoader, then it is replaced with DataLoader in the data loader name.
   • Example: The DataLoader for PersonBatchLoader would, by default, be named personDataLoader.
3. *WithOptions interfaces were added to allow one to create a component and provide the desired DataLoaderOptions that would be connected with the correlating request DataLoader. These also allow for specifying an explicit name for the DataLoader that is created if the default naming convention is not wanted.
   • All of the *WithOptions interfaces inherit a single SpringBatchLoaderWithOptions interface for ease of referencing them all. Inheritance of SpringBatchLoaderWithOptions is sealed to only the interfaces that were created.
   • Example:

     @Component
     @RequiredArgsConstructor
     public class PersonBatchLoader implements MappedBatchLoaderWithOptions<String, Person> {
     
       private final MockData mockData;
     
       @Override
       public CompletionStage<Map<String, Person>> load(Set<String> personIds) {
         return supplyAsync(() -> {
           var people = mockData.getPeople();
           return personIds.stream().collect(toMap(identity(), people::get));
         });
       }
     
       /** NOT required to override this method */
       @Override
       public String getName() {
         return "peopleDataLoader";
       }
     
       @Override
       public DataLoaderOptions getDataLoaderOptions() {
         return DataLoaderOptions.newOptions().setMaxBatchSize(42);
       }
     }
     

4. BatchLoaderRegistry can now register mapped and unmapped Reactive batch loaders which return collections without issue.
   • I noticed that there are issues with typing and resolution of data loaders because of the fact that we can't provide a collection with a generic when calling forTypePair and similar typing issues occur when using forName to register a batch loader.
   • register methods were added that allow passing a name, DataLoaderOptions (optionally), and a BiFunction instance. This allows typing issues to be resolved without the need to provide explicit classes when registering a batch loader.
   • Example:

     registry.registerMapped("parentsLoader", (Set<String> childrenIds, BatchLoaderEnvironment env) -> {
       Mono<Map<String, Collection<Person>>> childrenMap = personService.getParents(childrenIds);
       return childrenMap;
     });
     

     There is a register method as well used for the Flux return values
5. Added FunctionBatchLoaderSpec to allow the option to create batch loaders as beans that can then be registered in BatchLoaderRegistry.
   • Example:

     @Bean
     public FunctionBatchLoaderSpec<String, Person> personBatchLoaderSpec(MockData mockData) {
       return FunctionBatchLoaderSpec.createMapped("personLoader", (Set<String> personIds, BatchLoaderEnvironment env) -> {
         var people = getPeople();
         return Mono.just(personIds.stream().collect(toMap(identity(), people::get)));
       });
     }
     
     @Bean
     public BatchLoaderRegistry batchLoaderRegistry(
       @Autowired(required = false) DataLoaderOptions defaultDataLoaderOptions,
       Collection<FunctionBatchLoaderSpec> functionBatchLoaderSpecs,
       Collection<BatchLoader<?, ?>> batchLoaders,
       Collection<BatchLoaderWithContext<?, ?>> batchLoadersWithContext,
       Collection<MappedBatchLoader<?, ?>> mappedBatchLoaders,
       Collection<MappedBatchLoaderWithContext<?, ?>> mappedBatchLoadersWithContext,
       Collection<SpringBatchLoaderWithOptions> batchLoadersWithOptions
     ) {
       var registry = (
         defaultDataLoaderOptions == null
           ? new DefaultBatchLoaderRegistry()
           : new DefaultBatchLoaderRegistry(() -> defaultDataLoaderOptions)
       );
     
       functionBatchLoaderSpecs.forEach(registry::register);
       batchLoaders.forEach(registry::register);
       batchLoadersWithContext.forEach(registry::register);
       mappedBatchLoaders.forEach(registry::register);
       mappedBatchLoadersWithContext.forEach(registry::register);
       batchLoadersWithOptions.forEach(registry::register);
     
       return registry;
     }
     

[rstoyanchev]

@bsara thanks for raising this discussion and happy to provide feedback, but could you please take a step back and better explain the issues you are facing in a new comment below? I see you have a detailed draft in a branch, but we can't look at a solution without understanding the problems it is trying to solve.

[bsara]

...could you please take a step back and better explain the issues you are facing in a new comment below?

There really are two issues here:

1. The only ways in which one can add batch loaders to BatchLoaderRegistry are via the @BatchMapping annotation, or manually adding a batch loader by creating a BiFunction and using BatchLoaderRegistry#forTypePair or BatchLoaderRegistry#forName. While the option of a functional approach is much appreciated, there is no way to add batch loader objects directly to BatchLoaderRegistry (which would be instances of any of the available interfaces provided by java-dataloader like BatchLoader or MappedBatchLoaderWithContext). It would be desirable for reasons of easy reuse, lack of boilerplate, and the preferred coding style of some to allow batch loader component classes to be auto-registered with BatchLoaderRegistry (using Spring Boot auto-configuration).
2. With the current API, one cannot create a batch loader function where the value would be a collection and have the generic of the value collection come through when creating the loader. (Yes, one could just inline the BiFunction, but there should be a way to use non-inlined instances as well that are accurately typed with generics). For example:

The first point is the main problem I'm attempting to solve, the second is something that I think goes along with providing alternate means to register batch loaders in general.

[bsara]

Bump