GraphQL v1.8.7 not compatible with Batch Loader about batch-loader

[cabello]

I can't reproduce it, my Gemfile.lock contains:

graphql (1.8.7)
batch-loader (1.2.1)

While running a query that would potentially cause a N+1 I got only two queries as expected:

D, [2018-08-15T09:31:48.706667 #67559] DEBUG -- :   FundsTransfer Load (8.0ms)  SELECT  "funds_transfers".* FROM "funds_transfers" ORDER BY "funds_transfers"."created_at" DESC LIMIT $1 OFFSET $2  [["LIMIT", 50], ["OFFSET", 0]]
D, [2018-08-15T09:31:48.821072 #67559] DEBUG -- :   Account Load (4.8ms)  SELECT "accounts".* FROM "accounts" WHERE "accounts"."id" IN (13, 12, 9, 8, 7, 6)

Am I missing something? Can you post the code on how you are using batch loader?

[JanStevens]

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"
  gem "graphql", '1.8.7'
  gem "batch-loader"
  gem 'pg'
  gem 'rails', '5.2.1'
end

require 'pg'
require 'active_record'
require 'active_support'
require 'logger'

ActiveRecord::Base.establish_connection(adapter: "postgresql", database: "hacky", host: 'localhost')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :posts, force: true do |t|
    t.string :title
    t.timestamps(null: false)
  end

  create_table :comments, force: true do |t|
    t.string :title
    t.string :comment
    t.integer :post_id
    t.timestamps(null: false)
  end
end

class Post < ActiveRecord::Base
  has_many :comments
end

class Comment < ActiveRecord::Base
  belongs_to :post
end

# Data setup
post = Post.create!(title: 'AAAA1')
post.comments << Comment.create!(title: 'AAAA1')
post.comments << Comment.create!(title: 'AAAA2')
post.comments << Comment.create!(title: 'AAAA3')

post = Post.create!(title: 'BBBB1')
post.comments << Comment.create!(title: 'BBBB1')
post.comments << Comment.create!(title: 'BBBB2')
post.comments << Comment.create!(title: 'BBBB3')


class CommentType < GraphQL::Schema::Object
  field :id, ID, null: false
  field :title, String, null: false
end

class PostType < GraphQL::Schema::Object
  field :id, ID, null: false
  field :title, String, null: false
  field :comments, [CommentType], null: true

  def comments
    BatchLoader.for(object.id).batch(default_value: []) do |post_ids, loader|
      Comment.where(post_id: post_ids).each do |comment|
        loader.call(comment.post_id) { |memo| memo << comment }
      end
    end
  end
end

class QueryType < GraphQL::Schema::Object
  field :posts, [PostType], null: false

  def posts
    Post.all
  end
end

class Schema < GraphQL::Schema
  query QueryType
  use BatchLoader::GraphQL
end

query_string = <<~GRAPHQL
  query posts {
    posts {
      comments {
        id
        title
      }
    }
  }
GRAPHQL

context = {}
variables = {}

puts Schema.execute(query_string, context: context, variables: variables).to_json

You can change the graphql version from 1.8.6 (working) to 1.8.7 (not working).

Output for 1.8.6

D, [2018-08-15T15:44:30.794020 #54453] DEBUG -- :   Post Load (0.5ms)  SELECT "posts".* FROM "posts"
D, [2018-08-15T15:44:30.795289 #54453] DEBUG -- :   Comment Load (0.4ms)  SELECT "comments".* FROM "comments" WHERE "comments"."post_id" IN ($1, $2)  [["post_id", 1], ["post_id", 2]]
{"data":{"posts":[{"comments":[{"id":"1","title":"AAAA1"},{"id":"2","title":"AAAA2"},{"id":"3","title":"AAAA3"}]},{"comments":[{"id":"4","title":"BBBB1"},{"id":"5","title":"BBBB2"},{"id":"6","title":"BBBB3"}]}]}}

Output for 1.8.7

D, [2018-08-15T15:44:52.600755 #54659] DEBUG -- :   Post Load (0.4ms)  SELECT "posts".* FROM "posts"
D, [2018-08-15T15:44:52.601887 #54659] DEBUG -- :   Comment Load (0.2ms)  SELECT "comments".* FROM "comments" WHERE "comments"."post_id" = $1  [["post_id", 1]]
D, [2018-08-15T15:44:52.603110 #54659] DEBUG -- :   Comment Load (0.3ms)  SELECT "comments".* FROM "comments" WHERE "comments"."post_id" = $1  [["post_id", 2]]
{"data":{"posts":[{"comments":[{"id":"1","title":"AAAA1"},{"id":"2","title":"AAAA2"},{"id":"3","title":"AAAA3"}]},{"comments":[{"id":"4","title":"BBBB1"},{"id":"5","title":"BBBB2"},{"id":"6","title":"BBBB3"}]}]}}

[JanStevens]

It seems that the API changed of GraphQL Ruby and basically the gist is rmosolgo/graphql-ruby#1778 (comment)

Any reason why class method is undefined?

[exAspArk]

@JanStevens I was able to reproduce it, thanks for the code example!

That's sad that graphql-ruby introduced a breaking change and started wrapping the values with it's own lazy objects.

Any reason why class method is undefined?

It's not undefined, it's being proxied to the resolved value. For example:

id = 1

result = BatchLoader.for(id).batch { |ids, loader| loader.call(ids.first, 1) }
result.class # => Fixnum

result = BatchLoader.for(id).batch { |ids, loader| loader.call(ids.first, nil) }
result.class # => NilClass

[exAspArk]

There are some ideas on how to make graphql-ruby more flexible in terms of this "lazy" functionality. Maybe they'll be implemented one day.

In a meantime, here is a quick fix:

def comments
  batch_loader = BatchLoader.for(object.id).batch(default_value: []) do |post_ids, loader|
    Comment.where(post_id: post_ids).each do |comment|
      loader.call(comment.post_id) { |memo| memo << comment }
    end
  end

  BatchLoader::GraphQL::Wrapper.new(batch_loader) # it is done automatically with graphql-ruby 1.8.6
end

Just wrap BatchLoader instance into BatchLoader::GraphQL::Wrapper.

That way the new graphql-ruby version will be able to understand that it's a lazy object, before wrapping it into another lazy GraphQL::Execution::Lazy object.

[exAspArk]

@rmosolgo is so crazy fast! He already started working on rmosolgo/graphql-ruby#1784 🚀

That potentially will allow DRYing the BatchLoader::GraphQL::Wrapper logic instead of repeating it everywhere. Or something similar to detect lazy BatchLoader instances in the GraphQL world. Right now it's done by using .class.

[Batou99]

Has this been fixed on 1.8.8?

[exAspArk]

I added tests and set up a build matrix to test the latest GraphQL 1.8 and 1.7 versions in this PR #28.

batch-loader/spec/graphql_spec.rb

Lines 4 to 28 in a726994

	it 'resolves BatchLoader fields lazily' do
	user1 = User.save(id: "1")
	user2 = User.save(id: "2")
	Post.save(user_id: user1.id)
	Post.save(user_id: user2.id)
	query = <<~QUERY
	{
	posts {
	user { id }
	userId
	}
	}
	QUERY
	expect(User).to receive(:where).with(id: ["1", "2"]).once.and_call_original
	result = GraphqlSchema.execute(query)
	expect(result['data']).to eq({
	'posts' => [
	{'user' => {'id' => "1"}, "userId" => 1},
	{'user' => {'id' => "2"}, "userId" => 2}
	]
	})
	end

It seems like it works fine with graphql version 1.8.11 now? Going to close the issue. Feel free to reopen if it still doesn't work.