Gusto / apollo-federation-ruby

A Ruby implementation of Apollo Federation
MIT License
216 stars 72 forks source link

Making `Query._entities` work with GraphQL Dataloader #149

Closed daemonsy closed 2 years ago

daemonsy commented 2 years ago

Background

GraphQL Ruby has its own native batch loading abstraction that uses Ruby's fibers.

Right now, we are using it to resolve all N+1 queries in our app, except for incoming top level subgraph queries which are resolved by the Query._entities field.

The problem

Given a query:

query($representations:[_Any!]!){
  _entities(representations:$representations){
    ...on BlogPost{ 
      id
      title
    }
  }
}

Where we have variables representing two objects to be resolved:

{
  "variables": {
    "representations": [
      {
        "__typename": "BlogPost",
        "id": "1"
      },
      {
        "__typename": "BlogPost",
        "id": "2"
      }
    ]
  }
}

One might write a simple BlogPostType.resolve_reference method that looks like this:

def self.resolve_reference(reference, context)

  post = context.dataloader.with(DataSources::ActiveRecord, BlogPost).load(reference[:id])

  return nil unless post

  BlogPostService.new(blog_post: post)
end

However, this doesn't work with Dataloader. With debugging statements around it, The moment #load is called, we also end up instantiating theDataSource and call #fetch on it. This ends up generating the N+1s as before.

Expected behavior

In Apollo's own implementation, the typical recommendation is to do batch loading in __resolveReference. We can differ in implementation on Ruby, but if we're sticking to the same paradigm, I would hope that there is a way for #resolve_reference to work with the native batch loader.

Possible causes and fixes

Still investigating, will update this section of the issue once I have more information.

As a starting point, this is the resolver for Query._entities. It uses a map of calls to each type's resolve_reference, which probably does not work with the Dataloader.

https://github.com/Gusto/apollo-federation-ruby/blob/1d3baf4f8efcd02e7bf5bc7e3fee5b4fb963cd25/lib/apollo-federation/entities_field.rb#L29-L55

daemonsy commented 2 years ago

In the end, I think the right way to do it is to return dataloader.with(...).load as the result of resolve_reference. If we need to do anything further like wrapping a service class over the entity, this can be done in the loader.

col commented 2 years ago

@daemonsy I've run into this problem today. We are returning dataloader.with(...).load as the result of resolve_reference but it's calling fetch on the data source for each individual reference. I've been scratching my head on this one for hours and I think it's a legitimate issue. Do you have an example of this working correctly with a data loader?