graphql-java-kickstart / graphql-java-tools

A schema-first tool for graphql-java inspired by graphql-tools for JS
https://www.graphql-java-kickstart.com/tools/
MIT License
812 stars 174 forks source link

How to do batching? "Source type is not expected type (java.lang.Iterable)!" #151

Closed karl-run closed 4 years ago

karl-run commented 6 years ago

Using graphql-spring-boot-starter, graphql-java-servlet and graphql-java-tools.

I have this resolver:

@Component
public class SomethingGroupResolver implements GraphQLResolver<SomethingGroup> {

  @Autowired
  private SomethingStructureService somethingStructureService;

  @Allow
  public List<Something> getSomethings(SomethingGroup somethingGroup) {
    return somethingStructureService.getAllSomethings(somethingGroup);
  }
}

In my case resolving all somethings on a something group creates a lot of individual database queries so I would like to batch these requests.

My understanding of @Batched is to use it as following:

  @Component
  public class SomethingGroupResolver implements GraphQLResolver<SomethingGroup> {

    @Autowired
    private SomethingStructureService somethingStructureService;

    @Allow
+   @Batched
-   public List<Something> getSomethings(SomethingGroup somethingGroup) {
+   public List<List<Something>> getSomethings(List<SomethingGroup> somethingGroups) {
      return somethingStructureService.getAllSomethingsForGroups(somethingGroups);
    }
  }

When running this the schema and resolvers validate fine, however at runtime I get this error:

com.coxautodev.graphql.tools.ResolverError: Source type (run.karl.example.SomethingGroup) is not expected type (java.lang.Iterable)!
    at com.coxautodev.graphql.tools.MethodFieldResolver$createDataFetcher$1.invoke(MethodFieldResolver.kt:52) ~[graphql-java-tools-5.2.0.jar:na]
    at com.coxautodev.graphql.tools.MethodFieldResolver$createDataFetcher$1.invoke(MethodFieldResolver.kt:20) ~[graphql-java-tools-5.2.0.jar:na]
    at com.coxautodev.graphql.tools.MethodFieldResolverDataFetcher.get(MethodFieldResolver.kt:146) ~[graphql-java-tools-5.2.0.jar:na]
    at com.coxautodev.graphql.tools.BatchedMethodFieldResolverDataFetcher.get(MethodFieldResolver.kt:166) ~[graphql-java-tools-5.2.0.jar:na]

Debugging a bit shows that SomethingGroupResolver.getSomethings that when batched expects List<SomethingGroup> somethingGroups, only receives a single SomethingGroup.

Is there something about batching and @Batched I'm not understanding? :smile_cat:

jkwatson commented 6 years ago

I have exactly this same question! Any ideas yet?

jkwatson commented 6 years ago

This seems very related: https://github.com/graphql-java/graphql-java-tools/issues/123

jkwatson commented 6 years ago

Aha. @karl-run Add one of these to your Spring config:

    @Bean
    public ExecutionStrategy executionStrategy() {
        return new BatchedExecutionStrategy();
    }

Although, this does beg the question about deprecation, since BatchedExecutionStrategy is marked as @deprecated. Is there a better way to do it, I wonder?

karl-run commented 6 years ago

The javadoc states that it has been deprecated in favour of AsyncExecutionStrategy.

BatchedExecutionStrategy has been deprecated in favour of graphql.execution.AsyncExecutionStrategy and graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation. BatchedExecutionStrategy does not properly implement the graphql runtime specification. Specifically it does not correctly handle non null fields and how they are to cascade up their parent fields. It has proven an intractable problem to make this code handle these cases. See http://facebook.github.io/graphql/October2016/#sec-Errors-and-Non-Nullability We will remove it once we are sure the alternative is as least good as the BatchedExecutionStrategy.

I'll see if AsyncExecutionStrategy can be provided with like BatchedExecutionStrategy, and if that fixes the issue.

karl-run commented 6 years ago

Just straight out replacing BatchedExecutionStrategy with AsyncExecutionStrategy does not work. :man_shrugging:

jkwatson commented 6 years ago

Yes, those docs call out that you have to use it in conjunction with DataLoaders. Unfortunately, it doesn't look like the Spring/tools integration with the DataLoader system is very simple or elegant right now. I'm hoping that will happen before the BatchedExecutionStrategy gets removed!

jkwatson commented 6 years ago

This issue has plenty of discussion on using DataLoaders: https://github.com/graphql-java/graphql-java-tools/issues/58

vojtapol commented 4 years ago

@Batched and BatchedExecutionStrategy is deprecated. Please use https://github.com/graphql-java/java-dataloader instead.