Skip to content

How to access @BatchMapping filter arguments? #417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
aboger opened this issue Jun 17, 2022 · 4 comments
Closed

How to access @BatchMapping filter arguments? #417

aboger opened this issue Jun 17, 2022 · 4 comments

Comments

@aboger
Copy link

aboger commented Jun 17, 2022

Thanks for Spring GraphQL. Integrating it is a joy, everything works just fine out-of-the-box. I just found this little spot:

During integration of Spring GraphQL into an existing application, I implemented @BatchMapping in order to solve the N+1 problem when loading related types. I then figured that methods annotated with @BatchMapping do not provide access to input objects using the @Argument annotation. Intuitively, I hoped for something like this to work:

@BatchMapping
public Map<Store, List<Product>> products(List<Store> stores, @Argument("filter") Filter filter) {
    return storeService.loadProducts(stores, filter);
}

I read though the documentation, and support for @Argument with @BatchMapping seems absent.
I briefly debugged the code and couldn't spot support in org.springframework.graphql.data.method.annotation.support.BatchLoaderHandlerMethod#resolveArgument.

Apparently, I can currently only implement filtering suffering from N+1 using @SchemaMapping like this:

@SchemaMapping
public List<Product> products(Store store, @Argument("filter") Filter filter) {
    return storeService.loadProducts(store, filter);
}

Did I misunderstand something? If so, kindly asking for support on this :-)
If not, is a work-around possible to access the filter object, e.g. using an interceptor?
Should support for this already be planned, could you already estimate when it would be released?

Issue 411 could be related.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 17, 2022
@rstoyanchev
Copy link
Contributor

This is a duplicate of #232. Please, see the discussion there. At present, there is no easy way to do this and ensure it works in all cases. We can add something to the documentation on that, but that's about it.

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Jun 17, 2022
@aboger
Copy link
Author

aboger commented Jun 18, 2022

By following #232 and digging deeper into the topic I understood that I can build something like:

@SchemaMapping
public CompletableFuture<List<Product>> products(
        Store store, @Argument Filter filter, DataLoader<BatchLoad, List<Product>> loader) {
    return loader.load(BatchLoad.of(store, filter, "products", Store::getProducts));
}

and

batchLoaderRegistry.forTypePair(BatchLoad.class, List.class)
            .registerMappedBatchLoader((batchLoads, env) -> Mono.just(load(batchLoads)))

Based on this, I meanhwile implemented a minimal working solution for filtered batch loading, which does not suffer from the N+1 problem. Thanks @rstoyanchev.

@aboger aboger closed this as completed Jun 18, 2022
@rstoyanchev rstoyanchev removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Jul 8, 2022
@nothing-personal-gh
Copy link

nothing-personal-gh commented Jun 28, 2023

I think this issue is worth reconsidering. It would be elegant to pull in arguments that are sent in the current field regardless of batch mapping or schema mapping. It didn't occur immediately to me why @Argument wasn't working only for Batch mapping. I had to dig through the code and Google search and ended up here. Consider this use-case:

query siteSearch {
  sites(filter: {name: "texas"}) {
    things(filter: {name: "chairs"}) {
      id
      name
    }
  }
}

Compare this code:

    @BatchMapping
    public Map<SiteModel, List<ThingModel>> things(
        List<SiteModel> sites,
        @Argument ThingFilterInput filter
    ) {

with the workaround above. Workaround is much longer and more boilerplate code (about 20 extra lines in my implementation now).

Edit - Is there an example that returns a list of objects in the batch? I tried using @aboger's workaround but stuck on this type issue with List.

        registry.forTypePair(BatchLoadSiteThings.class, List.class)
            .registerMappedBatchLoader((batches, env) -> {
                return Mono.just(getThingsInSites(batches));
            });

getThingsInSites returns Map<BatchLoadSiteThings, List<ThingGraphQLModel>>. Error is

Required type: Mono<Map<BatchLoadSiteThings, List>>
Provided: Mono<Map<BatchLoadSiteThings, List<ThingGraphQLModel>>>

If I try registry.forTypePair(BatchLoadSiteThings.class, List<ThingGraphQLModel>.class), the error is Cannot access class object of parameterized type.


I was able to resolve this type issue using https://techdozo.dev/spring-for-graphql-how-to-solve-the-n1-problem/

Basically we'll lose the type with this approach:

        registry.forTypePair(BatchLoadSiteThings.class, List.class)
            .registerMappedBatchLoader((batches, env) -> {
                Map thingsInSites = getThingsInSites(batches);
                return Mono.just(thingsInSites);
            });

@nothing-personal-gh
Copy link

@aboger what does load function do in your code?

batchLoaderRegistry.forTypePair(BatchLoad.class, List.class)
            .registerMappedBatchLoader((batchLoads, env) -> Mono.just(load(batchLoads)))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants