Cannot inject DataLoader<> directly from auto configured @BatchMapping

[xcq1]

I'm in the process of porting an existing project to spring-graphql 1.3.1 and want to use all of its convenience features for more readable code. From the docs about annotated controllers I gathered that defining and reusing DataLoaders should be quite easy. I have a setup similar to this:

@Controller
class EntityConnectionsBatchController {

    @BatchMapping
    fun entityConnections(keys: Set<EntityFilter>): Mono<Map<EntityBatchFilter, EntityConnectionGraphQL>> {
        return Mono.just(
            // complex filtering logic...
            .toMap()
        )
    }
}

@Controller
class EntityQueryController {

    @QueryMapping
    fun entities(
        @Argument first: Int,
        @Argument after: CursorGraphQL?,
        @Argument offset: Int?,
        @Argument filter: EntityFilterInputGraphQL?,
        dataLoader: DataLoader<EntityBatchFilter, EntityConnectionGraphQL>
    ): CompletableFuture<EntityConnectionGraphQL> =
        dataLoader.load(
            EntityFilter(
                // add all the arguments into the filter
            )
        )
}

But when I try to query this, I run into an exception:

Cannot resolve DataLoader for parameter 'dataLoader' in method public java.util.concurrent.CompletableFuture<org.example.controller.graphql.model.EntityConnectionGraphQL> org.example.controller.graphql.resolvers.queries.EntityQueryController.entities(int,java.lang.String,java.lang.Integer,org.example.controller.graphql.model.EntityFilterInputGraphQL,org.dataloader.DataLoader<org.example.controller.graphql.resolvers.queries.EntityBatchFilter, org.example.controller.graphql.model.EntityConnectionGraphQL>). Neither the name of the declared value type 'class org.example.controller.graphql.model.EntityConnectionGraphQL' nor the method parameter name 'dataLoader' match to any DataLoader. The DataLoaderRegistry contains: [EntityGroupBatchFilter.entityGroupConnections, EntityBatchFilter.entityConnections, /* ... */]

I was trying to change some internal names to get it to work out of the box, but upon debugging a bit I am confused how this is supposed to work:

• The AnnotatedControllerConfigurer reads the methods and creates FieldCoordinates of each consisting of the pattern <typeName>.<fieldName>. Both are overridable from the annotation, typeName is by default Class<>.getSimpleName():
  https://github.com/spring-projects/spring-graphql/blob/v1.3.1/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java#L324
• The DataLoaderMethodArgumentResolver however is looking up by Class<>.getName() (fully-qualified) OR alternatively <parameterName> a few lines after:
  https://github.com/spring-projects/spring-graphql/blob/v1.3.1/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataLoaderMethodArgumentResolver.java#L57

Not only will the class names never match by default, because one is simpleName and one is package-qualified, but it is impossible to select an existing data loader since a.) I cannot configure the @BatchMapping annotation to not include a dot and b.) I cannot request injection of a data loader with dot in the name since dots are (thankfully) illegal in parameter names.

Am I missing something and there is a reason for this? To me it sounds as if the DataLoaderMethodArgumentResolver should actually request the same kind of pattern of <simpleClassName>.<parameterName> by default.

So to get this to work currently I have to inject the environment and find it by knowing the naming convention of AnnotatedControllerConfigurer:

 fun entities(
        // ...
        env: DataFetchingEnvironment
    ): CompletableFuture<EntityConnectionGraphQL> {
        val dataLoader = env.getDataLoader<EntityBatchFilter, EntityConnectionGraphQL>("${EntityBatchFilter::class.simpleName}.entityConnections")
        return dataLoader.load(/* ... */)

[rstoyanchev]

@xcq1 what version are you upgrading from, or is this from a different framework altogether? In other words, is it a specific feature in 1.3 or just generally converting to Spring for GraphQL.

At any rate, we should start by making it clear that use of DataLoader and @BatchMapping in your controller is mutually exclusive. You use one or the other, but not both. In this section of the documentation you'll see two code snippets. The first shows how you would use a DataLoader by injecting it into the controller method in order to manage the Book to Author association. The second example shows how to do the same through a @BatchMapping method instead. Essentially @BatchMapping is a kind of shortcut that aims to simplify code a little by not having to call DataLoader directly.

As for how the injected DataLoader is located, that's done by comparing the generic types for key and value. It should just work but it depends on what the BatchLoaderRegistry registration looks like and you haven't shown that.

Also keep in mind we have pagination support as of version 1.2 that may be relevant to you.

[xcq1]

@rstoyanchev Thanks for the pointers. I'm upgrading from graphql-java-kickstart with a lot of custom code that deals with handling federation, data loaders, etc. Definitely will take another look at the support for pagination, nullability and Querydsl.

So I got further by following your suggestion to create a BatchLoader with a top-level query something like this, and then also the DataLoader can be injected directly:

@Controller
class TriangleController(registry: BatchLoaderRegistry, ...) {
    init {
        registry.forTypePair(TriangleId::class.java, TriangleGraphQL::class.java).registerMappedBatchLoader { keys, _ ->
            Mono.just(
                triangleRepository.getAllFromIds(keys).toMap()
            )
        }
    @QueryMapping
    fun triangles(@Argument id: TriangleId, dataLoader: DataLoader<TriangleId, TriangleGraphQL>): CompletableFuture<TriangleGraphQL> {
        return dataLoader.load(id)
    }
 }

But now oddly enough, when I want to use the same for a federated query the same approach does not work:

    @EntityMapping
    fun triangle(
        @Argument idList: List<String>,
        dataLoader: DataLoader<TriangleId, TriangleGraphQL>
    ) : CompletableFuture<List<TriangleGraphQL>>? {
      // formatArgumentError: Could not resolve parameter [1] in public java.util.concurrent.CompletableFuture<java.util.List<....): No suitable resolver
        return dataLoader.loadMany(idList.map { TriangleId(it) })
    }

This means I need to do this again which seems strange since it requires again knowledge about with which name it got registered in the first place:

val dataLoader = dataFetchingEnvironment.getDataLoader<TriangleId, TriangleGraphQL>(TriangleGraphQL::class.java.name)

Am I misunderstanding something there as well?

I think part of my confusion also stems from the association that all things essentially just map ids to objects, so "retrieving an object by id" should be no different whether I want that object to respond to a query, to a federated entities call or to resolving a field from a different object. Shouldn't it be possible to define it one time and then have some customizable "boiler plate" generated automatically?

[rstoyanchev]

You cannot use the DataLoader mechanism on top level methods like EntityMapping. However, EntityMapping methods themselves support loading all entities of the same type. Please, check the Federation section of the documentation and the example of how to batch load federated entities of the same type.

[xcq1]

Please forgive my ignorance, but is there a specific reason why this should not be done on EntityMapping? If I just inject the DataFetchingEnvironment like mentioned and get the data loader from there, it appears to work just fine, including context and everything.

[rstoyanchev]

The DataLoader mechanism is for nested fields where the N+1 problem arises. There is no need for that at the top where you can return a list. Just a like you return a List for a query that returns multiple items. This was also discussed previously at #922 (comment).

[xcq1]

Thanks for the explanation. I understand your point, but I wanted to give two more inputs here:

1. Data loader caching could still be beneficial: If the graph contains not only a tree but is cyclic, e.g. because it contains both directions of entity grouping (one-to-many) or links between entities (one-to-one). In that case it is possible that in the query response the same top-level object appears again as a nested object and could just be fetched from the data loader cache instead of requesting it from the database again.
   This can actually be useful to use in a query when the recursion depth is limited. To illustrate this point, this blog post explains an example: https://escape.tech/blog/cyclic-queries-and-depth-limit/

2. In all the use cases I'm currently porting over, it would be easier to just reuse the same data loader code instead of duplicating it. So at least having the option to use it would be nice.

[rstoyanchev]

Aside from re-using your existing code, I'm curious, in relation to data loader caching, do you actually have cases where a federated type appears again in nested parts of the graph, and is that even supported in federation?

[rstoyanchev]

@xcq1 coming back to this, as the original description started from a different question, I've created a new issue #1095, and plan to experiment.

It occurs to me that support for DataLoader could be as simple as adding the DataLoaderMethodArgumentResolver to the list of argument resolvers. At the moment it's not very convenient to do that, but it is possible by overriding FederationSchemaFactory#initArgumentResolvers. Let us know, if you do try it.