From 8abda4074f2009ccaa3e00541e30fb81409c62fe Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Thu, 5 Mar 2026 14:43:47 -0500 Subject: [PATCH 1/9] Move support for resolvers and loads into main module --- lib/graphql/execution/batching.rb | 1 + .../execution/batching/field_compatibility.rb | 74 ----------- .../execution/batching/field_resolve_step.rb | 122 +++++++++++++----- .../execution/batching/resolvers_step.rb | 25 ++++ lib/graphql/schema/field.rb | 16 ++- lib/graphql/schema/resolver.rb | 28 ++++ spec/graphql/dataloader_spec.rb | 7 +- spec/graphql/schema/resolver_spec.rb | 2 +- 8 files changed, 163 insertions(+), 112 deletions(-) create mode 100644 lib/graphql/execution/batching/resolvers_step.rb diff --git a/lib/graphql/execution/batching.rb b/lib/graphql/execution/batching.rb index ca116e480d..7e87522a73 100644 --- a/lib/graphql/execution/batching.rb +++ b/lib/graphql/execution/batching.rb @@ -2,6 +2,7 @@ require "graphql/execution/batching/prepare_object_step" require "graphql/execution/batching/field_compatibility" require "graphql/execution/batching/field_resolve_step" +require "graphql/execution/batching/resolvers_step" require "graphql/execution/batching/runner" require "graphql/execution/batching/selections_step" module GraphQL diff --git a/lib/graphql/execution/batching/field_compatibility.rb b/lib/graphql/execution/batching/field_compatibility.rb index ddded9e3a6..b649496411 100644 --- a/lib/graphql/execution/batching/field_compatibility.rb +++ b/lib/graphql/execution/batching/field_compatibility.rb @@ -3,53 +3,11 @@ module GraphQL module Execution module Batching module FieldCompatibility - def resolve_all_load_arguments(frs, object_from_id_receiver, arguments, argument_owner, context) - arg_defns = context.types.arguments(argument_owner) - arg_defns.each do |arg_defn| - if arg_defn.loads - if arguments.key?(arg_defn.keyword) - id = arguments.delete(arg_defn.keyword) - if !id.nil? - value = if arg_defn.type.list? - id.map { |inner_id| - object_from_id_receiver.load_and_authorize_application_object(arg_defn, inner_id, context) - } - else - object_from_id_receiver.load_and_authorize_application_object(arg_defn, id, context) - end - - if frs.runner.resolves_lazies - value = frs.sync(value) - end - if value.is_a?(GraphQL::Error) - value.path = frs.path - return value - end - else - value = nil - end - arguments[arg_defn.keyword] = value - end - elsif (input_type = arg_defn.type.unwrap).kind.input_object? && - (value = arguments[arg_defn.keyword]) # TODO lists - resolve_all_load_arguments(frs, object_from_id_receiver, value, input_type, context) - end - end - nil - end - def resolve_batch(frs, objects, context, kwargs) if @batch_mode && !:direct_send.equal?(@batch_mode) return super end - if !@resolver_class - maybe_err = resolve_all_load_arguments(frs, self, kwargs, self, context) - if maybe_err - return maybe_err - end - end - if @owner.method_defined?(@resolver_method) results = [] frs.selections_step.graphql_objects.each_with_index do |obj_inst, idx| @@ -65,38 +23,6 @@ def resolve_batch(frs, objects, context, kwargs) end end results - elsif @resolver_class - objects.map do |o| - resolver_inst_kwargs = kwargs.dup - resolver_inst = @resolver_class.new(object: o, context: context, field: self) - maybe_err = resolve_all_load_arguments(frs, resolver_inst, resolver_inst_kwargs, self, context) - if maybe_err - next maybe_err - end - ruby_kwargs = if @resolver_class < Schema::HasSingleInputArgument - resolver_inst_kwargs[:input] - else - resolver_inst_kwargs - end - resolver_inst.prepared_arguments = ruby_kwargs - is_authed, new_return_value = resolver_inst.authorized?(**ruby_kwargs) - if frs.runner.resolves_lazies && frs.runner.schema.lazy?(is_authed) - is_authed, new_return_value = frs.runner.schema.sync_lazy(is_authed) - end - if is_authed - resolver_inst.call_resolve(ruby_kwargs) - else - new_return_value - end - rescue RuntimeError => err - err - rescue StandardError => stderr - begin - context.query.handle_or_reraise(stderr) - rescue GraphQL::ExecutionError => ex_err - ex_err - end - end elsif objects.first.is_a?(Hash) objects.map { |o| o[method_sym] || o[graphql_name] } elsif objects.first.is_a?(Interpreter::RawValue) diff --git a/lib/graphql/execution/batching/field_resolve_step.rb b/lib/graphql/execution/batching/field_resolve_step.rb index d1ebe7adae..66d45af4fa 100644 --- a/lib/graphql/execution/batching/field_resolve_step.rb +++ b/lib/graphql/execution/batching/field_resolve_step.rb @@ -58,54 +58,65 @@ def coerce_arguments(argument_owner, ast_arguments_or_hash) arg_defn = arg_defns.each_value.find { |a| a.keyword == key || a.graphql_name == (key_s ||= String(key)) } - arg_value = coerce_argument_value(arg_defn.type, value) - args_hash[arg_defn.keyword] = arg_value + maybe_err = coerce_argument_value(args_hash, arg_defn, value) + if maybe_err + return maybe_err + end end else ast_arguments_or_hash.each { |arg_node| arg_defn = arg_defns[arg_node.name] - arg_value = coerce_argument_value(arg_defn.type, arg_node.value) - arg_key = arg_defn.keyword - args_hash[arg_key] = arg_value + maybe_err = coerce_argument_value(args_hash, arg_defn, arg_node.value) + if maybe_err + return maybe_err + end } end - + # TODO refactor the loop above into this one arg_defns.each do |arg_graphql_name, arg_defn| if arg_defn.default_value? && !args_hash.key?(arg_defn.keyword) - args_hash[arg_defn.keyword] = arg_defn.default_value + maybe_err = coerce_argument_value(args_hash, arg_defn, arg_defn.default_value) + if maybe_err + return maybe_err + end end end args_hash end - def coerce_argument_value(arg_t, arg_value) + def coerce_argument_value(arguments, arg_defn, arg_value, target_keyword: arg_defn.keyword, as_type: nil) + arg_t = as_type || arg_defn.type if arg_t.non_null? arg_t = arg_t.of_type end - if arg_value.is_a?(Language::Nodes::VariableIdentifier) + arg_value = if arg_value.is_a?(Language::Nodes::VariableIdentifier) vars = @selections_step.query.variables - arg_value = if vars.key?(arg_value.name) + if vars.key?(arg_value.name) vars[arg_value.name] elsif vars.key?(arg_value.name.to_sym) vars[arg_value.name.to_sym] end elsif arg_value.is_a?(Language::Nodes::NullValue) - arg_value = nil + nil elsif arg_value.is_a?(Language::Nodes::Enum) - arg_value = arg_value.name + arg_value.name elsif arg_value.is_a?(Language::Nodes::InputObject) - arg_value = arg_value.arguments # rubocop:disable Development/ContextIsPassedCop + arg_value.arguments # rubocop:disable Development/ContextIsPassedCop + else + arg_value end - if arg_t.list? + arg_value = if arg_t.list? if arg_value.nil? arg_value else arg_value = Array(arg_value) inner_t = arg_t.of_type - arg_value.map { |v| coerce_argument_value(inner_t, v) } + result = Array.new(arg_value.size) + arg_value.each_with_index { |v, i| coerce_argument_value(result, arg_defn, v, target_keyword: i, as_type: inner_t) } + result end elsif arg_t.kind.leaf? begin @@ -123,6 +134,46 @@ def coerce_argument_value(arg_t, arg_value) else raise "Unsupported argument value: #{arg_t.to_type_signature} / #{arg_value.class} (#{arg_value.inspect})" end + + if arg_defn.loads && as_type.nil? && !arg_value.nil? + context = @selections_step.query.context + # This is for legacy compat: + object_from_id_receiver = if (r = @field_definition.resolver) + r.new(field: @field_definition, context: context, object: nil) + else + @field_definition + end + begin + arg_value = if arg_t.list? + arg_value.map { |inner_id| + object_from_id_receiver.load_and_authorize_application_object(arg_defn, inner_id, context) + } + else + object_from_id_receiver.load_and_authorize_application_object(arg_defn, arg_value, context) + end + + # TODO spin up steps for this + if runner.resolves_lazies + arg_value = sync(arg_value) + end + + rescue GraphQL::RuntimeError => err + arg_value = err + rescue StandardError => stderr + arg_value = begin + context.query.handle_or_reraise(stderr) + rescue GraphQL::ExecutionError => ex_err + ex_err + end + end + + if arg_value.is_a?(GraphQL::Error) + arg_value.path = path + return arg_value + end + end + arguments[target_keyword] = arg_value + nil end # Implement that Lazy API @@ -197,7 +248,30 @@ def execute_field objects = @selections_step.graphql_objects end - @arguments = coerce_arguments(@field_definition, @ast_node.arguments) # rubocop:disable Development/ContextIsPassedCop + ctx = query.context + + if @runner.authorization && @runner.authorizes?(@field_definition, ctx) + authorized_objects = [] + @object_is_authorized = objects.map { |o| + is_authed = @field_definition.authorized?(o, @arguments, ctx) + if is_authed + authorized_objects << o + end + is_authed + } + else + authorized_objects = objects + @object_is_authorized = AlwaysAuthorized + end + + arguments_or_error = coerce_arguments(@field_definition, @ast_node.arguments) # rubocop:disable Development/ContextIsPassedCop + if arguments_or_error.is_a?(GraphQL::Error) + @field_results = Array.new(authorized_objects.size, arguments_or_error) + build_results + return + else + @arguments = arguments_or_error + end @field_definition.extras.each do |extra| case extra when :lookahead @@ -219,22 +293,6 @@ def execute_field end end - ctx = query.context - - if @runner.authorization && @runner.authorizes?(@field_definition, ctx) - authorized_objects = [] - @object_is_authorized = objects.map { |o| - is_authed = @field_definition.authorized?(o, @arguments, ctx) - if is_authed - authorized_objects << o - end - is_authed - } - else - authorized_objects = objects - @object_is_authorized = AlwaysAuthorized - end - if @parent_type.default_relay? && authorized_objects.all? { |o| o.respond_to?(:was_authorized_by_scope_items?) && o.was_authorized_by_scope_items? } @was_scoped = true end diff --git a/lib/graphql/execution/batching/resolvers_step.rb b/lib/graphql/execution/batching/resolvers_step.rb new file mode 100644 index 0000000000..b5be65d3d2 --- /dev/null +++ b/lib/graphql/execution/batching/resolvers_step.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true +module GraphQL + module Execution + module Batching + class ResolversStep + def initialize(field_resolve_step:, resolvers:) + @field_resolve_step = field_resolve_step + @resolvers = resolvers + @finished_resolvers = nil + end + + def call + if @finished_resolvers.nil? + @resolvers.each do |r| + r.resolvers_step = self + @field_resolve_step.runner.add_step(r) + end + end + end + + attr_reader :field_resolve_step + end + end + end +end diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 3d4f67ded0..288c44cc8b 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -282,6 +282,9 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CON elsif dig @batch_mode = :dig @batch_mode_key = dig + elsif resolver_class + @batch_mode = :resolver_class + @batch_mode_key = resolver_class else @batch_mode = :direct_send @batch_mode_key = @method_sym @@ -396,8 +399,19 @@ def resolve_batch(field_resolve_step, objects, context, args_hash) end when :dig objects.map { |o| o.dig(*@batch_mode_key) } + when :resolver_class + results = Array.new(objects.size, nil) + resolvers = objects.map.with_index do |o, idx| + resolver_inst = @resolver_class.new(object: o, context: context, field: self) + resolver_inst.prepared_arguments = args_hash + resolver_inst.exec_result = results + resolver_inst.exec_index = idx + resolver_inst + end + field_resolve_step.runner.add_step(Execution::Batching::ResolversStep.new(field_resolve_step: field_resolve_step, resolvers: resolvers)) + results else - raise "Batching execution for #{path} not implemented; provide `resolve_static:`, `resolve_batch:`, `hash_key:`, `method:`, or use a compatibility plug-in" + raise "Batching execution for #{path} not implemented (batch_mode: #{@batch_mode.inspect}); provide `resolve_static:`, `resolve_batch:`, `hash_key:`, `method:`, or use a compatibility plug-in" end end diff --git a/lib/graphql/schema/resolver.rb b/lib/graphql/schema/resolver.rb index 5f01183f0f..bf1d55d651 100644 --- a/lib/graphql/schema/resolver.rb +++ b/lib/graphql/schema/resolver.rb @@ -46,6 +46,8 @@ def initialize(object:, context:, field:) @prepared_arguments = nil end + attr_accessor :exec_result, :exec_index, :resolvers_step + # @return [Object] The application object this field is being resolved on attr_accessor :object @@ -57,6 +59,32 @@ def initialize(object:, context:, field:) attr_writer :prepared_arguments + def call + if self.class < Schema::HasSingleInputArgument + @prepared_arguments = @prepared_arguments[:input] + end + is_authed, new_return_value = authorized?(**@prepared_arguments) + if (runner = @resolvers_step.field_resolve_step.runner).resolves_lazies && runner.schema.lazy?(is_authed) + is_authed, new_return_value = runner.schema.sync_lazy(is_authed) + end + + result = if is_authed + call_resolve(@prepared_arguments) + else + new_return_value + end + + exec_result[exec_index] = result + rescue RuntimeError => err + exec_result[exec_index] = err + rescue StandardError => stderr + exec_result[exec_index] = begin + context.query.handle_or_reraise(stderr) + rescue GraphQL::ExecutionError => ex_err + ex_err + end + end + def arguments @prepared_arguments || raise("Arguments have not been prepared yet, still waiting for #load_arguments to resolve. (Call `.arguments` later in the code.)") end diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb index 25b731dd23..4b629971c4 100644 --- a/spec/graphql/dataloader_spec.rb +++ b/spec/graphql/dataloader_spec.rb @@ -287,7 +287,7 @@ def recipes_by_id_using_load_all(ids:) end def self.recipes_by_id(context, recipes:) - context.dataloader.with(DataObject).load_all(recipes) + recipes end def recipes_by_id(recipes:) @@ -338,8 +338,6 @@ def common_ingredients(recipe_1_id:, recipe_2_id:) end def self.common_ingredients_with_load(objects, context, recipe_1:, recipe_2:) - recipe_1, recipe_2 = context.dataloader.with(DataObject).load_all([recipe_1, recipe_2]) - common_ids = recipe_1[:ingredient_ids] & recipe_2[:ingredient_ids] results = context.dataloader.with(DataObject).load_all(common_ids) Array.new(objects.size, results) @@ -367,7 +365,8 @@ def common_ingredients_from_input_object(input:) end def self.common_ingredients_from_input_object(objects, context, input:) - recipe_1, recipe_2 = context.dataloader.with(DataObject).load_all([input[:recipe_1], input[:recipe_2]]) + recipe_1 = input[:recipe_1] + recipe_2 = input[:recipe_2] common_ids = recipe_1[:ingredient_ids] & recipe_2[:ingredient_ids] results = context.dataloader.with(DataObject).load_all(common_ids) Array.new(objects.size, results) diff --git a/spec/graphql/schema/resolver_spec.rb b/spec/graphql/schema/resolver_spec.rb index c4b73b9d45..5cfbd5ac77 100644 --- a/spec/graphql/schema/resolver_spec.rb +++ b/spec/graphql/schema/resolver_spec.rb @@ -446,7 +446,7 @@ def resolve(**inputs) class MutationWithRequiredLoadsArgument < GraphQL::Schema::Mutation argument :label_id, ID, loads: HasValue - field :inputs, String, null: false + field :inputs, String, null: false, hash_key: :inputs def resolve(**inputs) { From a550a02beb4e40432e31f7f55460611b50f223e8 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 6 Mar 2026 08:15:21 -0500 Subject: [PATCH 2/9] Build arguments before field authorization because arguments are an input to #authorized? --- .../execution/batching/field_resolve_step.rb | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/graphql/execution/batching/field_resolve_step.rb b/lib/graphql/execution/batching/field_resolve_step.rb index 66d45af4fa..e0b87aff1f 100644 --- a/lib/graphql/execution/batching/field_resolve_step.rb +++ b/lib/graphql/execution/batching/field_resolve_step.rb @@ -250,20 +250,6 @@ def execute_field ctx = query.context - if @runner.authorization && @runner.authorizes?(@field_definition, ctx) - authorized_objects = [] - @object_is_authorized = objects.map { |o| - is_authed = @field_definition.authorized?(o, @arguments, ctx) - if is_authed - authorized_objects << o - end - is_authed - } - else - authorized_objects = objects - @object_is_authorized = AlwaysAuthorized - end - arguments_or_error = coerce_arguments(@field_definition, @ast_node.arguments) # rubocop:disable Development/ContextIsPassedCop if arguments_or_error.is_a?(GraphQL::Error) @field_results = Array.new(authorized_objects.size, arguments_or_error) @@ -293,6 +279,20 @@ def execute_field end end + if @runner.authorization && @runner.authorizes?(@field_definition, ctx) + authorized_objects = [] + @object_is_authorized = objects.map { |o| + is_authed = @field_definition.authorized?(o, @arguments, ctx) + if is_authed + authorized_objects << o + end + is_authed + } + else + authorized_objects = objects + @object_is_authorized = AlwaysAuthorized + end + if @parent_type.default_relay? && authorized_objects.all? { |o| o.respond_to?(:was_authorized_by_scope_items?) && o.was_authorized_by_scope_items? } @was_scoped = true end From 9d07a70ab1753832051e804a20fd9775d1c5ce25 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 6 Mar 2026 08:29:05 -0500 Subject: [PATCH 3/9] Fix objects usage on arg errors --- lib/graphql/execution/batching/field_resolve_step.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/graphql/execution/batching/field_resolve_step.rb b/lib/graphql/execution/batching/field_resolve_step.rb index e0b87aff1f..ae1739b274 100644 --- a/lib/graphql/execution/batching/field_resolve_step.rb +++ b/lib/graphql/execution/batching/field_resolve_step.rb @@ -252,7 +252,8 @@ def execute_field arguments_or_error = coerce_arguments(@field_definition, @ast_node.arguments) # rubocop:disable Development/ContextIsPassedCop if arguments_or_error.is_a?(GraphQL::Error) - @field_results = Array.new(authorized_objects.size, arguments_or_error) + @field_results = Array.new(objects.size, arguments_or_error) + @object_is_authorized = AlwaysAuthorized build_results return else From 6b2dda070b487e2bbb6e5c485adcef920ab93184 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 6 Mar 2026 08:44:15 -0500 Subject: [PATCH 4/9] Implement resolver fanning and waiting --- lib/graphql/execution/batching.rb | 1 - .../execution/batching/field_resolve_step.rb | 19 +++++++++----- .../execution/batching/resolvers_step.rb | 25 ------------------- lib/graphql/schema/field.rb | 5 ++-- lib/graphql/schema/resolver.rb | 16 ++++++++++-- spec/graphql/dataloader_spec.rb | 2 +- 6 files changed, 31 insertions(+), 37 deletions(-) delete mode 100644 lib/graphql/execution/batching/resolvers_step.rb diff --git a/lib/graphql/execution/batching.rb b/lib/graphql/execution/batching.rb index 7e87522a73..ca116e480d 100644 --- a/lib/graphql/execution/batching.rb +++ b/lib/graphql/execution/batching.rb @@ -2,7 +2,6 @@ require "graphql/execution/batching/prepare_object_step" require "graphql/execution/batching/field_compatibility" require "graphql/execution/batching/field_resolve_step" -require "graphql/execution/batching/resolvers_step" require "graphql/execution/batching/runner" require "graphql/execution/batching/selections_step" module GraphQL diff --git a/lib/graphql/execution/batching/field_resolve_step.rb b/lib/graphql/execution/batching/field_resolve_step.rb index ae1739b274..bf7596c058 100644 --- a/lib/graphql/execution/batching/field_resolve_step.rb +++ b/lib/graphql/execution/batching/field_resolve_step.rb @@ -22,11 +22,14 @@ def initialize(parent_type:, runner:, key:, selections_step:) @object_is_authorized = nil @finish_extension_idx = nil @was_scoped = nil + @resolver_instances = nil end attr_reader :ast_node, :key, :parent_type, :selections_step, :runner, :field_definition, :object_is_authorized, :arguments, :was_scoped + attr_accessor :resolver_instances + def path @path ||= [*@selections_step.path, @key].freeze end @@ -178,11 +181,15 @@ def coerce_argument_value(arguments, arg_defn, arg_value, target_keyword: arg_de # Implement that Lazy API def value - query = @selections_step.query - query.current_trace.begin_execute_field(@field_definition, @arguments, @field_results, query) - @field_results = sync(@field_results) - query.current_trace.end_execute_field(@field_definition, @arguments, @field_results, query, @field_results) - @runner.add_step(self) + if @resolver_instances + @runner.dataloader.lazy_at_depth(path.size, self) + else + query = @selections_step.query + query.current_trace.begin_execute_field(@field_definition, @arguments, @field_results, query) + @field_results = sync(@field_results) + query.current_trace.end_execute_field(@field_definition, @arguments, @field_results, query, @field_results) + @runner.add_step(self) + end true end @@ -316,7 +323,7 @@ def execute_field query.current_trace.end_execute_field(@field_definition, @arguments, authorized_objects, query, @field_results) - if any_lazy_results? + if any_lazy_results? || @resolver_instances @runner.dataloader.lazy_at_depth(path.size, self) elsif has_extensions finish_extensions diff --git a/lib/graphql/execution/batching/resolvers_step.rb b/lib/graphql/execution/batching/resolvers_step.rb deleted file mode 100644 index b5be65d3d2..0000000000 --- a/lib/graphql/execution/batching/resolvers_step.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true -module GraphQL - module Execution - module Batching - class ResolversStep - def initialize(field_resolve_step:, resolvers:) - @field_resolve_step = field_resolve_step - @resolvers = resolvers - @finished_resolvers = nil - end - - def call - if @finished_resolvers.nil? - @resolvers.each do |r| - r.resolvers_step = self - @field_resolve_step.runner.add_step(r) - end - end - end - - attr_reader :field_resolve_step - end - end - end -end diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 288c44cc8b..0ae5ce1229 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -401,14 +401,15 @@ def resolve_batch(field_resolve_step, objects, context, args_hash) objects.map { |o| o.dig(*@batch_mode_key) } when :resolver_class results = Array.new(objects.size, nil) - resolvers = objects.map.with_index do |o, idx| + field_resolve_step.resolver_instances = objects.map.with_index do |o, idx| resolver_inst = @resolver_class.new(object: o, context: context, field: self) + resolver_inst.field_resolve_step = field_resolve_step resolver_inst.prepared_arguments = args_hash resolver_inst.exec_result = results resolver_inst.exec_index = idx + field_resolve_step.runner.add_step(resolver_inst) resolver_inst end - field_resolve_step.runner.add_step(Execution::Batching::ResolversStep.new(field_resolve_step: field_resolve_step, resolvers: resolvers)) results else raise "Batching execution for #{path} not implemented (batch_mode: #{@batch_mode.inspect}); provide `resolve_static:`, `resolve_batch:`, `hash_key:`, `method:`, or use a compatibility plug-in" diff --git a/lib/graphql/schema/resolver.rb b/lib/graphql/schema/resolver.rb index bf1d55d651..f1d56e01b7 100644 --- a/lib/graphql/schema/resolver.rb +++ b/lib/graphql/schema/resolver.rb @@ -46,7 +46,7 @@ def initialize(object:, context:, field:) @prepared_arguments = nil end - attr_accessor :exec_result, :exec_index, :resolvers_step + attr_accessor :exec_result, :exec_index, :field_resolve_step # @return [Object] The application object this field is being resolved on attr_accessor :object @@ -63,8 +63,12 @@ def call if self.class < Schema::HasSingleInputArgument @prepared_arguments = @prepared_arguments[:input] end + q = context.query + trace_objs = [object] + q.current_trace.begin_execute_field(field, @prepared_arguments, trace_objs, q) is_authed, new_return_value = authorized?(**@prepared_arguments) - if (runner = @resolvers_step.field_resolve_step.runner).resolves_lazies && runner.schema.lazy?(is_authed) + + if (runner = @field_resolve_step.runner).resolves_lazies && runner.schema.lazy?(is_authed) is_authed, new_return_value = runner.schema.sync_lazy(is_authed) end @@ -73,6 +77,8 @@ def call else new_return_value end + q = context.query + q.current_trace.end_execute_field(field, @prepared_arguments, trace_objs, q, [result]) exec_result[exec_index] = result rescue RuntimeError => err @@ -83,6 +89,12 @@ def call rescue GraphQL::ExecutionError => ex_err ex_err end + ensure + ri = field_resolve_step.resolver_instances + ri.delete(self) + if ri.size == 0 + field_resolve_step.resolver_instances = nil + end end def arguments diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb index 4b629971c4..e043b254c0 100644 --- a/spec/graphql/dataloader_spec.rb +++ b/spec/graphql/dataloader_spec.rb @@ -1258,7 +1258,7 @@ def assert_last_max_fiber_count(expected_last_max_fiber_count, message = nil) res = exec_query(query_str, context: { dataloader: fiber_counting_dataloader_class.new(fiber_limit: 4) }) assert_equal 4, res.context.dataloader.fiber_limit - assert_equal((TESTING_BATCHING ? 10 : 12), FiberCounting.last_spawn_fiber_count) + assert_equal((TESTING_BATCHING ? 11 : 12), FiberCounting.last_spawn_fiber_count) assert_last_max_fiber_count(4, "Limit of 4 works as expected") res = exec_query(query_str, context: { dataloader: fiber_counting_dataloader_class.new(fiber_limit: 6) }) From 2bac77635db2036aca99df494197149ab15689f0 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 6 Mar 2026 09:09:05 -0500 Subject: [PATCH 5/9] Split argument loading into new step --- lib/graphql/execution/batching.rb | 1 + .../execution/batching/field_resolve_step.rb | 80 +++++++------------ .../execution/batching/load_argument_step.rb | 75 +++++++++++++++++ lib/graphql/schema/field.rb | 2 +- lib/graphql/schema/resolver.rb | 8 +- 5 files changed, 111 insertions(+), 55 deletions(-) create mode 100644 lib/graphql/execution/batching/load_argument_step.rb diff --git a/lib/graphql/execution/batching.rb b/lib/graphql/execution/batching.rb index ca116e480d..c2dfecec4a 100644 --- a/lib/graphql/execution/batching.rb +++ b/lib/graphql/execution/batching.rb @@ -2,6 +2,7 @@ require "graphql/execution/batching/prepare_object_step" require "graphql/execution/batching/field_compatibility" require "graphql/execution/batching/field_resolve_step" +require "graphql/execution/batching/load_argument_step" require "graphql/execution/batching/runner" require "graphql/execution/batching/selections_step" module GraphQL diff --git a/lib/graphql/execution/batching/field_resolve_step.rb b/lib/graphql/execution/batching/field_resolve_step.rb index bf7596c058..60b7225b36 100644 --- a/lib/graphql/execution/batching/field_resolve_step.rb +++ b/lib/graphql/execution/batching/field_resolve_step.rb @@ -22,13 +22,13 @@ def initialize(parent_type:, runner:, key:, selections_step:) @object_is_authorized = nil @finish_extension_idx = nil @was_scoped = nil - @resolver_instances = nil + @pending_steps = nil end attr_reader :ast_node, :key, :parent_type, :selections_step, :runner, :field_definition, :object_is_authorized, :arguments, :was_scoped - attr_accessor :resolver_instances + attr_accessor :pending_steps def path @path ||= [*@selections_step.path, @key].freeze @@ -139,49 +139,19 @@ def coerce_argument_value(arguments, arg_defn, arg_value, target_keyword: arg_de end if arg_defn.loads && as_type.nil? && !arg_value.nil? - context = @selections_step.query.context - # This is for legacy compat: - object_from_id_receiver = if (r = @field_definition.resolver) - r.new(field: @field_definition, context: context, object: nil) - else - @field_definition - end - begin - arg_value = if arg_t.list? - arg_value.map { |inner_id| - object_from_id_receiver.load_and_authorize_application_object(arg_defn, inner_id, context) - } - else - object_from_id_receiver.load_and_authorize_application_object(arg_defn, arg_value, context) - end - - # TODO spin up steps for this - if runner.resolves_lazies - arg_value = sync(arg_value) - end - - rescue GraphQL::RuntimeError => err - arg_value = err - rescue StandardError => stderr - arg_value = begin - context.query.handle_or_reraise(stderr) - rescue GraphQL::ExecutionError => ex_err - ex_err - end - end - - if arg_value.is_a?(GraphQL::Error) - arg_value.path = path - return arg_value - end + loads_step = LoadArgumentStep.new(field_resolve_step: self, argument_definition: arg_defn, arguments: arguments, argument_type: arg_t) + @runner.add_step(loads_step) + ps = @pending_steps ||= [] + ps.push(loads_step) + else + arguments[target_keyword] = arg_value end - arguments[target_keyword] = arg_value nil end # Implement that Lazy API def value - if @resolver_instances + if @pending_steps @runner.dataloader.lazy_at_depth(path.size, self) else query = @selections_step.query @@ -212,8 +182,10 @@ def call finish_extensions elsif @field_results build_results - else + elsif @arguments execute_field + else + build_arguments end rescue StandardError => err if @field_definition && !err.message.start_with?("Resolving ") @@ -237,26 +209,21 @@ def self.[](_key) end end - def execute_field + def build_arguments query = @selections_step.query field_name = @ast_node.name @field_definition = query.get_field(@parent_type, field_name) || raise("Invariant: no field found for #{@parent_type.to_type_signature}.#{ast_node.name}") - objects = @selections_step.objects if field_name == "__typename" # TODO handle custom introspection - @field_results = Array.new(objects.size, @parent_type.graphql_name) + @field_results = Array.new(@selections_step.objects.size, @parent_type.graphql_name) @object_is_authorized = AlwaysAuthorized build_results return end - if @field_definition.dynamic_introspection - # TODO break this backwards compat somehow? - objects = @selections_step.graphql_objects - end - - ctx = query.context - + # TODO Split somewhere here so that this step can wait for arguments + # Then resume later -- this will require some instance state, eg `@arguments` + # to be accumulated during those other steps arguments_or_error = coerce_arguments(@field_definition, @ast_node.arguments) # rubocop:disable Development/ContextIsPassedCop if arguments_or_error.is_a?(GraphQL::Error) @field_results = Array.new(objects.size, arguments_or_error) @@ -265,7 +232,14 @@ def execute_field return else @arguments = arguments_or_error + execute_field end + end + + def execute_field + query = @selections_step.query + objects = @selections_step.objects + @field_definition.extras.each do |extra| case extra when :lookahead @@ -287,6 +261,12 @@ def execute_field end end + if @field_definition.dynamic_introspection + # TODO break this backwards compat somehow? + objects = @selections_step.graphql_objects + end + + ctx = query.context if @runner.authorization && @runner.authorizes?(@field_definition, ctx) authorized_objects = [] @object_is_authorized = objects.map { |o| diff --git a/lib/graphql/execution/batching/load_argument_step.rb b/lib/graphql/execution/batching/load_argument_step.rb new file mode 100644 index 0000000000..b3670cba53 --- /dev/null +++ b/lib/graphql/execution/batching/load_argument_step.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true +module GraphQL + module Execution + module Batching + class LoadArgumentStep + def initialize(field_resolve_step:, arguments:, argument_type:, argument_definition:) + @field_resolve_step = field_resolve_step + @arguments = arguments + @argument_definition = argument_definition + @argument_type = argument_type + @loaded_value = nil + end + + def value + @loaded_value = @field_resolve_step.sync(@loaded_value) + assign_value + end + + def call + context = @field_resolve_step.selections_step.query.context + field_definition = @field_resolve_step.field_definition + # This is for legacy compat: + object_from_id_receiver = if (r = field_definition.resolver) + r.new(field: field_definition, context: context, object: nil) + else + field_definition + end + begin + @loaded_value = if @argument_type.list? + arg_value.map { |inner_id| + object_from_id_receiver.load_and_authorize_application_object(@argument_definition, inner_id, context) + } + else + object_from_id_receiver.load_and_authorize_application_object(@argument_definition, arg_value, context) + end + + # TODO enqueue as lazy instead + if (runner = @field_resolve_step.runner).resolves_lazies && runner.schema.lazy?(@loaded_value) + runner.dataloader.lazy_at_depth(@field_resolve_step.path.size, self) + else + assign_value + end + + rescue GraphQL::RuntimeError => err + @loaded_value = err + rescue StandardError => stderr + @loaded_value = begin + context.query.handle_or_reraise(stderr) + rescue GraphQL::ExecutionError => ex_err + ex_err + end + end + end + + private + + def assign_value + # TODO signal this error somehow?? + if @loaded_value.is_a?(GraphQL::Error) + @loaded_value.path = path + return @loaded_value + end + + @arguments[@argument_definition.keyword] = @loaded_value + ensure + field_pending_steps = @field_resolve_step.pending_steps + field_pending_steps.delete(self) + if field_pending_steps.size == 0 + field_resolve_step.field_pending_steps = nil + end + end + end + end + end +end diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 0ae5ce1229..a3d96669f9 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -401,7 +401,7 @@ def resolve_batch(field_resolve_step, objects, context, args_hash) objects.map { |o| o.dig(*@batch_mode_key) } when :resolver_class results = Array.new(objects.size, nil) - field_resolve_step.resolver_instances = objects.map.with_index do |o, idx| + field_resolve_step.pending_steps = objects.map.with_index do |o, idx| resolver_inst = @resolver_class.new(object: o, context: context, field: self) resolver_inst.field_resolve_step = field_resolve_step resolver_inst.prepared_arguments = args_hash diff --git a/lib/graphql/schema/resolver.rb b/lib/graphql/schema/resolver.rb index f1d56e01b7..00d9d2306b 100644 --- a/lib/graphql/schema/resolver.rb +++ b/lib/graphql/schema/resolver.rb @@ -90,10 +90,10 @@ def call ex_err end ensure - ri = field_resolve_step.resolver_instances - ri.delete(self) - if ri.size == 0 - field_resolve_step.resolver_instances = nil + field_pending_steps = field_resolve_step.pending_steps + field_pending_steps.delete(self) + if field_pending_steps.size == 0 + field_resolve_step.field_pending_steps = nil end end From 9874c921d840c353ddb44d77d7850eb353afa4fe Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 6 Mar 2026 10:16:08 -0500 Subject: [PATCH 6/9] Implement waiting for loads, including list arguments --- .../execution/batching/field_resolve_step.rb | 75 ++++++++++++++----- .../execution/batching/load_argument_step.rb | 59 ++++++--------- lib/graphql/schema/field.rb | 4 +- lib/graphql/schema/resolver.rb | 2 +- spec/graphql/dataloader_spec.rb | 6 +- spec/graphql/schema/resolver_spec.rb | 1 - 6 files changed, 86 insertions(+), 61 deletions(-) diff --git a/lib/graphql/execution/batching/field_resolve_step.rb b/lib/graphql/execution/batching/field_resolve_step.rb index 60b7225b36..11b490c774 100644 --- a/lib/graphql/execution/batching/field_resolve_step.rb +++ b/lib/graphql/execution/batching/field_resolve_step.rb @@ -26,9 +26,9 @@ def initialize(parent_type:, runner:, key:, selections_step:) end attr_reader :ast_node, :key, :parent_type, :selections_step, :runner, - :field_definition, :object_is_authorized, :arguments, :was_scoped + :field_definition, :object_is_authorized, :was_scoped - attr_accessor :pending_steps + attr_accessor :pending_steps, :arguments def path @path ||= [*@selections_step.path, @key].freeze @@ -139,10 +139,40 @@ def coerce_argument_value(arguments, arg_defn, arg_value, target_keyword: arg_de end if arg_defn.loads && as_type.nil? && !arg_value.nil? - loads_step = LoadArgumentStep.new(field_resolve_step: self, argument_definition: arg_defn, arguments: arguments, argument_type: arg_t) - @runner.add_step(loads_step) - ps = @pending_steps ||= [] - ps.push(loads_step) + # This is for legacy compat: + load_receiver = if (r = @field_definition.resolver) + r.new(field: @field_definition, context: @selections_step.query.context, object: nil) + else + @field_definition + end + @pending_steps ||= [] + if arg_t.list? + results = Array.new(arg_value.size, nil) + arguments[arg_defn.keyword] = results + arg_value.each_with_index do |inner_v, idx| + loads_step = LoadArgumentStep.new( + field_resolve_step: self, + load_receiver: load_receiver, + argument_value: inner_v, + argument_definition: arg_defn, + arguments: results, + argument_key: idx, + ) + @pending_steps.push(loads_step) + @runner.add_step(loads_step) + end + else + loads_step = LoadArgumentStep.new( + field_resolve_step: self, + load_receiver: load_receiver, + argument_value: arg_value, + argument_definition: arg_defn, + arguments: arguments, + argument_key: arg_defn.keyword, + ) + @pending_steps.push(loads_step) + @runner.add_step(loads_step) + end else arguments[target_keyword] = arg_value end @@ -173,6 +203,12 @@ def sync(lazy) @runner.schema.unauthorized_object(auth_err) rescue GraphQL::ExecutionError => err err + rescue StandardError => stderr + begin + @selections_step.query.handle_or_reraise(stderr) + rescue GraphQL::ExecutionError => ex_err + ex_err + end end def call @@ -221,24 +257,27 @@ def build_arguments return end - # TODO Split somewhere here so that this step can wait for arguments - # Then resume later -- this will require some instance state, eg `@arguments` - # to be accumulated during those other steps - arguments_or_error = coerce_arguments(@field_definition, @ast_node.arguments) # rubocop:disable Development/ContextIsPassedCop - if arguments_or_error.is_a?(GraphQL::Error) - @field_results = Array.new(objects.size, arguments_or_error) - @object_is_authorized = AlwaysAuthorized - build_results - return + arguments = coerce_arguments(@field_definition, @ast_node.arguments) # rubocop:disable Development/ContextIsPassedCop + @arguments ||= arguments # may have already been set to an error + + if @pending_steps + @runner.dataloader.lazy_at_depth(path.size, self) else - @arguments = arguments_or_error execute_field end end def execute_field - query = @selections_step.query objects = @selections_step.objects + # TODO not as good because only one error? + if @arguments.is_a?(GraphQL::Error) + @field_results = Array.new(objects.size, @arguments) + @object_is_authorized = AlwaysAuthorized + build_results + return + end + + query = @selections_step.query @field_definition.extras.each do |extra| case extra @@ -303,7 +342,7 @@ def execute_field query.current_trace.end_execute_field(@field_definition, @arguments, authorized_objects, query, @field_results) - if any_lazy_results? || @resolver_instances + if any_lazy_results? || @pending_steps @runner.dataloader.lazy_at_depth(path.size, self) elsif has_extensions finish_extensions diff --git a/lib/graphql/execution/batching/load_argument_step.rb b/lib/graphql/execution/batching/load_argument_step.rb index b3670cba53..20232298ab 100644 --- a/lib/graphql/execution/batching/load_argument_step.rb +++ b/lib/graphql/execution/batching/load_argument_step.rb @@ -3,11 +3,13 @@ module GraphQL module Execution module Batching class LoadArgumentStep - def initialize(field_resolve_step:, arguments:, argument_type:, argument_definition:) + def initialize(field_resolve_step:, arguments:, load_receiver:, argument_value:, argument_definition:, argument_key:) @field_resolve_step = field_resolve_step + @load_receiver = load_receiver @arguments = arguments + @argument_value = argument_value @argument_definition = argument_definition - @argument_type = argument_type + @argument_key = argument_key @loaded_value = nil end @@ -18,55 +20,38 @@ def value def call context = @field_resolve_step.selections_step.query.context - field_definition = @field_resolve_step.field_definition - # This is for legacy compat: - object_from_id_receiver = if (r = field_definition.resolver) - r.new(field: field_definition, context: context, object: nil) + @loaded_value = @load_receiver.load_and_authorize_application_object(@argument_definition, @argument_value, context) + if (runner = @field_resolve_step.runner).resolves_lazies && runner.schema.lazy?(@loaded_value) + runner.dataloader.lazy_at_depth(@field_resolve_step.path.size, self) else - field_definition + assign_value end - begin - @loaded_value = if @argument_type.list? - arg_value.map { |inner_id| - object_from_id_receiver.load_and_authorize_application_object(@argument_definition, inner_id, context) - } - else - object_from_id_receiver.load_and_authorize_application_object(@argument_definition, arg_value, context) - end - - # TODO enqueue as lazy instead - if (runner = @field_resolve_step.runner).resolves_lazies && runner.schema.lazy?(@loaded_value) - runner.dataloader.lazy_at_depth(@field_resolve_step.path.size, self) - else - assign_value - end - - rescue GraphQL::RuntimeError => err - @loaded_value = err - rescue StandardError => stderr - @loaded_value = begin - context.query.handle_or_reraise(stderr) - rescue GraphQL::ExecutionError => ex_err - ex_err - end + rescue GraphQL::RuntimeError => err + @loaded_value = err + assign_value + rescue StandardError => stderr + @loaded_value = begin + context.query.handle_or_reraise(stderr) + rescue GraphQL::ExecutionError => ex_err + ex_err end + assign_value end private def assign_value - # TODO signal this error somehow?? if @loaded_value.is_a?(GraphQL::Error) - @loaded_value.path = path - return @loaded_value + @loaded_value.path = @field_resolve_step.path + @field_resolve_step.arguments = @loaded_value + else + @arguments[@argument_key] = @loaded_value end - @arguments[@argument_definition.keyword] = @loaded_value - ensure field_pending_steps = @field_resolve_step.pending_steps field_pending_steps.delete(self) if field_pending_steps.size == 0 - field_resolve_step.field_pending_steps = nil + @field_resolve_step.pending_steps = nil end end end diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index a3d96669f9..53ad6de712 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -401,8 +401,10 @@ def resolve_batch(field_resolve_step, objects, context, args_hash) objects.map { |o| o.dig(*@batch_mode_key) } when :resolver_class results = Array.new(objects.size, nil) - field_resolve_step.pending_steps = objects.map.with_index do |o, idx| + ps = field_resolve_step.pending_steps ||= [] + objects.map.with_index do |o, idx| resolver_inst = @resolver_class.new(object: o, context: context, field: self) + ps << resolver_inst resolver_inst.field_resolve_step = field_resolve_step resolver_inst.prepared_arguments = args_hash resolver_inst.exec_result = results diff --git a/lib/graphql/schema/resolver.rb b/lib/graphql/schema/resolver.rb index 00d9d2306b..fba89f54fb 100644 --- a/lib/graphql/schema/resolver.rb +++ b/lib/graphql/schema/resolver.rb @@ -93,7 +93,7 @@ def call field_pending_steps = field_resolve_step.pending_steps field_pending_steps.delete(self) if field_pending_steps.size == 0 - field_resolve_step.field_pending_steps = nil + field_resolve_step.pending_steps = nil end end diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb index e043b254c0..5502b2607c 100644 --- a/spec/graphql/dataloader_spec.rb +++ b/spec/graphql/dataloader_spec.rb @@ -1253,8 +1253,8 @@ def assert_last_max_fiber_count(expected_last_max_fiber_count, message = nil) res = exec_query(query_str, context: { dataloader: fiber_counting_dataloader_class.new }) assert_nil res.context.dataloader.fiber_limit - assert_equal((TESTING_BATCHING ? 9 : 10), FiberCounting.last_spawn_fiber_count) - assert_last_max_fiber_count((TESTING_BATCHING ? 8 : 9), "No limit works as expected") + assert_equal((TESTING_BATCHING ? 12 : 10), FiberCounting.last_spawn_fiber_count) + assert_last_max_fiber_count((TESTING_BATCHING ? 9 : 9), "No limit works as expected") res = exec_query(query_str, context: { dataloader: fiber_counting_dataloader_class.new(fiber_limit: 4) }) assert_equal 4, res.context.dataloader.fiber_limit @@ -1263,7 +1263,7 @@ def assert_last_max_fiber_count(expected_last_max_fiber_count, message = nil) res = exec_query(query_str, context: { dataloader: fiber_counting_dataloader_class.new(fiber_limit: 6) }) assert_equal 6, res.context.dataloader.fiber_limit - assert_equal(8, FiberCounting.last_spawn_fiber_count) + assert_equal(schema.dataloader_class <= GraphQL::Dataloader::AsyncDataloader ? 11 : 10, FiberCounting.last_spawn_fiber_count) assert_last_max_fiber_count(6, "Limit of 6 works as expected") end diff --git a/spec/graphql/schema/resolver_spec.rb b/spec/graphql/schema/resolver_spec.rb index 5cfbd5ac77..0ac5b7c470 100644 --- a/spec/graphql/schema/resolver_spec.rb +++ b/spec/graphql/schema/resolver_spec.rb @@ -1028,7 +1028,6 @@ def load_input(input); end it "returns an error when nullable argument is provided an invalid value" do res = exec_query('mutation { mutationWithNullableLoadsArgument(labelId: "invalid") { inputs } }') - assert res["errors"] assert_equal 'No object found for `labelId: "invalid"`', res["errors"][0]["message"] end From b6c5c1c2f98c95ee94dcb364792a8d1077a5192f Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 6 Mar 2026 10:49:03 -0500 Subject: [PATCH 7/9] Fix when all objects fail authorization --- lib/graphql/execution/batching/field_resolve_step.rb | 6 ++++++ lib/graphql/schema/field.rb | 2 +- .../tracing/active_support_notifications_trace_spec.rb | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/graphql/execution/batching/field_resolve_step.rb b/lib/graphql/execution/batching/field_resolve_step.rb index 11b490c774..a7c55fe4a3 100644 --- a/lib/graphql/execution/batching/field_resolve_step.rb +++ b/lib/graphql/execution/batching/field_resolve_step.rb @@ -182,6 +182,9 @@ def coerce_argument_value(arguments, arg_defn, arg_value, target_keyword: arg_de # Implement that Lazy API def value if @pending_steps + if @pending_steps.size == 0 + raise "Invariant: Waiting on empty list of pending steps. This is a bug in GraphQL-Ruby, please report this error along with query details on GitHub" + end @runner.dataloader.lazy_at_depth(path.size, self) else query = @selections_step.query @@ -315,6 +318,9 @@ def execute_field end is_authed } + if authorized_objects.size == 0 + return + end else authorized_objects = objects @object_is_authorized = AlwaysAuthorized diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 53ad6de712..8f3b4f850c 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -402,7 +402,7 @@ def resolve_batch(field_resolve_step, objects, context, args_hash) when :resolver_class results = Array.new(objects.size, nil) ps = field_resolve_step.pending_steps ||= [] - objects.map.with_index do |o, idx| + objects.each_with_index do |o, idx| resolver_inst = @resolver_class.new(object: o, context: context, field: self) ps << resolver_inst resolver_inst.field_resolve_step = field_resolve_step diff --git a/spec/graphql/tracing/active_support_notifications_trace_spec.rb b/spec/graphql/tracing/active_support_notifications_trace_spec.rb index b4614908fd..b520b616e7 100644 --- a/spec/graphql/tracing/active_support_notifications_trace_spec.rb +++ b/spec/graphql/tracing/active_support_notifications_trace_spec.rb @@ -76,9 +76,9 @@ def self.resolve_type(_abs, _obj, _ctx) "validate.graphql", "analyze.graphql", "authorized.graphql", - (TESTING_BATCHING ? "execute_field.graphql" : nil), # `loads:` happens during field execution in this case "dataloader_source.graphql", "execute_field.graphql", + (TESTING_BATCHING ? "execute_field.graphql" : nil), # `loads:` happens during field execution in this case (TESTING_BATCHING ? "resolve_type.graphql" : nil), # `loads:`-related? "resolve_type.graphql", "authorized.graphql", From f6f847ba4ff36997c3e70a3e8063b2cb786c1774 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 6 Mar 2026 10:58:59 -0500 Subject: [PATCH 8/9] Fix fiber count assertion, document directive incompatibility --- guides/execution/batching.md | 4 ++++ spec/graphql/dataloader_spec.rb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/guides/execution/batching.md b/guides/execution/batching.md index 784f7e5b6a..2f4cc84d67 100644 --- a/guides/execution/batching.md +++ b/guides/execution/batching.md @@ -281,6 +281,10 @@ This depends on `current_path` so isn't possible yet. Actually this probably works but I haven't tested it. +## Custom Directives + +Not supported yet. This will need some new kind of integration. + ### Argument `as:` ✅ `as:` is applied: arguments are passed into Ruby methods by their `as:` names instead of their GraphQL names. diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb index 5502b2607c..6ad332f828 100644 --- a/spec/graphql/dataloader_spec.rb +++ b/spec/graphql/dataloader_spec.rb @@ -1263,7 +1263,7 @@ def assert_last_max_fiber_count(expected_last_max_fiber_count, message = nil) res = exec_query(query_str, context: { dataloader: fiber_counting_dataloader_class.new(fiber_limit: 6) }) assert_equal 6, res.context.dataloader.fiber_limit - assert_equal(schema.dataloader_class <= GraphQL::Dataloader::AsyncDataloader ? 11 : 10, FiberCounting.last_spawn_fiber_count) + assert_equal(TESTING_BATCHING ? (schema.dataloader_class <= GraphQL::Dataloader::AsyncDataloader ? 11 : 10) : 8, FiberCounting.last_spawn_fiber_count) assert_last_max_fiber_count(6, "Limit of 6 works as expected") end From 9cc5e16887935b8da4d3e170bc123c1a8d51100e Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 6 Mar 2026 11:51:35 -0500 Subject: [PATCH 9/9] Merge pending step counting --- .../execution/batching/field_resolve_step.rb | 44 ++++++++----------- .../execution/batching/load_argument_step.rb | 4 +- .../execution/batching/prepare_object_step.rb | 2 +- lib/graphql/schema/resolver.rb | 4 +- spec/graphql/dataloader_spec.rb | 4 +- ...active_support_notifications_trace_spec.rb | 1 - 6 files changed, 25 insertions(+), 34 deletions(-) diff --git a/lib/graphql/execution/batching/field_resolve_step.rb b/lib/graphql/execution/batching/field_resolve_step.rb index a7c55fe4a3..f544bcf6f2 100644 --- a/lib/graphql/execution/batching/field_resolve_step.rb +++ b/lib/graphql/execution/batching/field_resolve_step.rb @@ -14,7 +14,6 @@ def initialize(parent_type:, runner:, key:, selections_step:) @field_results = nil @path = nil @enqueued_authorization = false - @pending_authorize_steps_count = 0 @all_next_objects = nil @all_next_results = nil @static_type = nil @@ -26,7 +25,7 @@ def initialize(parent_type:, runner:, key:, selections_step:) end attr_reader :ast_node, :key, :parent_type, :selections_step, :runner, - :field_definition, :object_is_authorized, :was_scoped + :field_definition, :object_is_authorized, :was_scoped, :field_results attr_accessor :pending_steps, :arguments @@ -181,18 +180,11 @@ def coerce_argument_value(arguments, arg_defn, arg_value, target_keyword: arg_de # Implement that Lazy API def value - if @pending_steps - if @pending_steps.size == 0 - raise "Invariant: Waiting on empty list of pending steps. This is a bug in GraphQL-Ruby, please report this error along with query details on GitHub" - end - @runner.dataloader.lazy_at_depth(path.size, self) - else - query = @selections_step.query - query.current_trace.begin_execute_field(@field_definition, @arguments, @field_results, query) - @field_results = sync(@field_results) - query.current_trace.end_execute_field(@field_definition, @arguments, @field_results, query, @field_results) - @runner.add_step(self) - end + query = @selections_step.query + query.current_trace.begin_execute_field(@field_definition, @arguments, @field_results, query) + @field_results = sync(@field_results) + query.current_trace.end_execute_field(@field_definition, @arguments, @field_results, query, @field_results) + @runner.add_step(self) true end @@ -215,7 +207,7 @@ def sync(lazy) end def call - if @enqueued_authorization && @pending_authorize_steps_count == 0 + if @enqueued_authorization enqueue_next_steps elsif @finish_extension_idx finish_extensions @@ -263,9 +255,7 @@ def build_arguments arguments = coerce_arguments(@field_definition, @ast_node.arguments) # rubocop:disable Development/ContextIsPassedCop @arguments ||= arguments # may have already been set to an error - if @pending_steps - @runner.dataloader.lazy_at_depth(path.size, self) - else + if @pending_steps.nil? || @pending_steps.size == 0 execute_field end end @@ -348,7 +338,7 @@ def execute_field query.current_trace.end_execute_field(@field_definition, @arguments, authorized_objects, query, @field_results) - if any_lazy_results? || @pending_steps + if any_lazy_results? @runner.dataloader.lazy_at_depth(path.size, self) elsif has_extensions finish_extensions @@ -458,7 +448,7 @@ def build_results end @enqueued_authorization = true - if @pending_authorize_steps_count == 0 + if @pending_steps.nil? || @pending_steps.size == 0 enqueue_next_steps else # Do nothing -- it will enqueue itself later @@ -539,9 +529,9 @@ def enqueue_next_steps end end - def authorized_finished - remaining = @pending_authorize_steps_count -= 1 - if @enqueued_authorization && remaining == 0 + def authorized_finished(step) + @pending_steps.delete(step) + if @enqueued_authorization && @pending_steps.size == 0 @runner.add_step(self) end end @@ -581,8 +571,7 @@ def build_graphql_result(graphql_result, key, field_result, return_type, is_nn, (runtime_type = (@runner.runtime_type_at[graphql_result] = @runner.resolve_type(@static_type, field_result, @selections_step.query)) ) && @runner.authorizes?(runtime_type, @selections_step.query.context) ))) - @pending_authorize_steps_count += 1 - @runner.add_step(Batching::PrepareObjectStep.new( + obj_step = Batching::PrepareObjectStep.new( static_type: @static_type, object: field_result, runner: @runner, @@ -593,7 +582,10 @@ def build_graphql_result(graphql_result, key, field_result, return_type, is_nn, is_non_null: is_nn, key: key, is_from_array: is_from_array, - )) + ) + ps = @pending_steps ||= [] + ps << obj_step + @runner.add_step(obj_step) else next_result_h = {} @all_next_results << next_result_h diff --git a/lib/graphql/execution/batching/load_argument_step.rb b/lib/graphql/execution/batching/load_argument_step.rb index 20232298ab..e9156ec541 100644 --- a/lib/graphql/execution/batching/load_argument_step.rb +++ b/lib/graphql/execution/batching/load_argument_step.rb @@ -50,8 +50,8 @@ def assign_value field_pending_steps = @field_resolve_step.pending_steps field_pending_steps.delete(self) - if field_pending_steps.size == 0 - @field_resolve_step.pending_steps = nil + if @field_resolve_step.arguments && field_pending_steps.size == 0 # rubocop:disable Development/ContextIsPassedCop + @field_resolve_step.runner.add_step(@field_resolve_step) end end end diff --git a/lib/graphql/execution/batching/prepare_object_step.rb b/lib/graphql/execution/batching/prepare_object_step.rb index 66aa26a62c..5694fe407b 100644 --- a/lib/graphql/execution/batching/prepare_object_step.rb +++ b/lib/graphql/execution/batching/prepare_object_step.rb @@ -121,7 +121,7 @@ def create_result @runner.static_type_at[next_result_h] = @static_type end - @field_resolve_step.authorized_finished + @field_resolve_step.authorized_finished(self) end end end diff --git a/lib/graphql/schema/resolver.rb b/lib/graphql/schema/resolver.rb index fba89f54fb..5fd82b16a4 100644 --- a/lib/graphql/schema/resolver.rb +++ b/lib/graphql/schema/resolver.rb @@ -92,8 +92,8 @@ def call ensure field_pending_steps = field_resolve_step.pending_steps field_pending_steps.delete(self) - if field_pending_steps.size == 0 - field_resolve_step.pending_steps = nil + if field_pending_steps.size == 0 && field_resolve_step.field_results + field_resolve_step.runner.add_step(field_resolve_step) end end diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb index 6ad332f828..5843dc54e6 100644 --- a/spec/graphql/dataloader_spec.rb +++ b/spec/graphql/dataloader_spec.rb @@ -1253,7 +1253,7 @@ def assert_last_max_fiber_count(expected_last_max_fiber_count, message = nil) res = exec_query(query_str, context: { dataloader: fiber_counting_dataloader_class.new }) assert_nil res.context.dataloader.fiber_limit - assert_equal((TESTING_BATCHING ? 12 : 10), FiberCounting.last_spawn_fiber_count) + assert_equal(10, FiberCounting.last_spawn_fiber_count) assert_last_max_fiber_count((TESTING_BATCHING ? 9 : 9), "No limit works as expected") res = exec_query(query_str, context: { dataloader: fiber_counting_dataloader_class.new(fiber_limit: 4) }) @@ -1263,7 +1263,7 @@ def assert_last_max_fiber_count(expected_last_max_fiber_count, message = nil) res = exec_query(query_str, context: { dataloader: fiber_counting_dataloader_class.new(fiber_limit: 6) }) assert_equal 6, res.context.dataloader.fiber_limit - assert_equal(TESTING_BATCHING ? (schema.dataloader_class <= GraphQL::Dataloader::AsyncDataloader ? 11 : 10) : 8, FiberCounting.last_spawn_fiber_count) + assert_equal(8, FiberCounting.last_spawn_fiber_count) assert_last_max_fiber_count(6, "Limit of 6 works as expected") end diff --git a/spec/graphql/tracing/active_support_notifications_trace_spec.rb b/spec/graphql/tracing/active_support_notifications_trace_spec.rb index b520b616e7..5a1d491180 100644 --- a/spec/graphql/tracing/active_support_notifications_trace_spec.rb +++ b/spec/graphql/tracing/active_support_notifications_trace_spec.rb @@ -78,7 +78,6 @@ def self.resolve_type(_abs, _obj, _ctx) "authorized.graphql", "dataloader_source.graphql", "execute_field.graphql", - (TESTING_BATCHING ? "execute_field.graphql" : nil), # `loads:` happens during field execution in this case (TESTING_BATCHING ? "resolve_type.graphql" : nil), # `loads:`-related? "resolve_type.graphql", "authorized.graphql",