diff --git a/lib/graphql/pagination/relation_connection.rb b/lib/graphql/pagination/relation_connection.rb index 0e6b091474..785e89d5c4 100644 --- a/lib/graphql/pagination/relation_connection.rb +++ b/lib/graphql/pagination/relation_connection.rb @@ -221,7 +221,16 @@ def limited_nodes # returns an array of nodes def load_nodes # Return an array so we can consistently use `.index(node)` on it - @nodes ||= limited_nodes.to_a + return @nodes if @nodes + # `AsyncDataloader` may resolve sibling fields (eg, `edges` and `pageInfo`) + # in separate Fibers, so several callers can get here before `@nodes` is set. + # The lock makes the later ones wait and reuse the result instead of loading + # the same relation again -- with ActiveRecord, that second load can raise + # `UnmodifiableRelation`. `Mutex` is Fiber-aware: waiting yields to the + # scheduler rather than blocking the thread. + (@load_lock ||= Mutex.new).synchronize do + @nodes ||= limited_nodes.to_a + end end end end diff --git a/spec/graphql/pagination/active_record_relation_connection_spec.rb b/spec/graphql/pagination/active_record_relation_connection_spec.rb index f8573c3814..9f7bbc918d 100644 --- a/spec/graphql/pagination/active_record_relation_connection_spec.rb +++ b/spec/graphql/pagination/active_record_relation_connection_spec.rb @@ -253,5 +253,53 @@ def total_count include ConnectionAssertions end + + if RUBY_VERSION >= "3.2.0" + require "async" + + describe "when Fibers share a connection" do + # `sleep` yields to the scheduler like a slow query would, so the other + # Fibers reach `#load_nodes` before this load finishes. + class SlowLoadingRelation + attr_reader :load_count + + def initialize(relation) + @relation = relation + @load_count = 0 + end + + def to_a + @load_count += 1 + sleep(0.1) + @relation.to_a + end + end + + class SlowLoadingRelationConnection < GraphQL::Pagination::ActiveRecordRelationConnection + def load_count + @slow_nodes ? @slow_nodes.load_count : 0 + end + + private + + def limited_nodes + @slow_nodes ||= SlowLoadingRelation.new(super) + end + end + + it "loads the page only once when several Fibers resolve it" do + connection = SlowLoadingRelationConnection.new(Food.all, first: 2, max_page_size: 10) + results = [] + + Sync do + 3.times.map { Async { results << connection.nodes } }.each(&:wait) + end + + assert_equal 1, connection.load_count, "It runs the query once" + assert_equal 3, results.size + results.each { |nodes| assert_equal 2, nodes.size } + end + end + end end end