From 288359c3d6253f66cb5c9422c9ad6cea793f07be Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 9 Aug 2024 14:58:05 -0400 Subject: [PATCH] Fix dataloader fiber switching in Mutations --- lib/graphql/execution/interpreter/runtime.rb | 32 +++++++++++++------- spec/graphql/schema/mutation_spec.rb | 26 ++++++++++++++++ 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index 0fda49bb5d..7c6818e6f4 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -207,22 +207,32 @@ def evaluate_selections(gathered_selections, selections_result, target_result, r finished_jobs = 0 enqueued_jobs = gathered_selections.size gathered_selections.each do |result_name, field_ast_nodes_or_ast_node| - @dataloader.append_job { - evaluate_selection( - result_name, field_ast_nodes_or_ast_node, selections_result - ) - finished_jobs += 1 - if target_result && finished_jobs == enqueued_jobs - selections_result.merge_into(target_result) - end - } + # Field resolution may pause the fiber, # so it wouldn't get to the `Resolve` call that happens below. # So instead trigger a run from this outer context. if selections_result.graphql_is_eager @dataloader.clear_cache - @dataloader.run - @dataloader.clear_cache + @dataloader.run_isolated { + evaluate_selection( + result_name, field_ast_nodes_or_ast_node, selections_result + ) + finished_jobs += 1 + if target_result && finished_jobs == enqueued_jobs + selections_result.merge_into(target_result) + end + @dataloader.clear_cache + } + else + @dataloader.append_job { + evaluate_selection( + result_name, field_ast_nodes_or_ast_node, selections_result + ) + finished_jobs += 1 + if target_result && finished_jobs == enqueued_jobs + selections_result.merge_into(target_result) + end + } end end selections_result diff --git a/spec/graphql/schema/mutation_spec.rb b/spec/graphql/schema/mutation_spec.rb index 6cd792120a..9cafdf6ca2 100644 --- a/spec/graphql/schema/mutation_spec.rb +++ b/spec/graphql/schema/mutation_spec.rb @@ -303,8 +303,24 @@ def resolve(counter:) end end + class ReadyCounter < GraphQL::Schema::Mutation + field :id, ID + argument :counter_id, ID + + def ready?(counter_id:) + # Just fill the cache: + dataloader.with(CounterSource).load(counter_id) + true + end + + def resolve(counter_id:) + { id: counter_id } + end + end + class Mutation < GraphQL::Schema::Object field :increment, mutation: Increment + field :ready_counter, mutation: ReadyCounter end mutation(Mutation) @@ -328,5 +344,15 @@ def self.resolve_type(abs_type, obj, ctx) res2 = MutationDataloaderCacheSchema.execute("mutation { increment(counterId: \"4\") { counter { value } } }") assert_equal 2, res2["data"]["increment"]["counter"]["value"] end + + it "uses a fresh cache for `ready?` calls" do + multiplex = [ + { query: "mutation { r1: readyCounter(counterId: 1) { id } }" }, + { query: "mutation { r2: readyCounter(counterId: 1) { id } }" }, + { query: "mutation { r3: readyCounter(counterId: 1) { id } }" }, + ] + result = MutationDataloaderCacheSchema.multiplex(multiplex) + assert_equal ["1", "1", "1"], result.map { |r| r["data"].first.last["id"] } + end end end