diff --git a/lib/graphql/execution.rb b/lib/graphql/execution.rb index b9eddc263a..298232a4e7 100644 --- a/lib/graphql/execution.rb +++ b/lib/graphql/execution.rb @@ -14,5 +14,12 @@ class Skip < GraphQL::Error; end # Just a singleton for implementing {Query::Context#skip} # @api private SKIP = Skip.new + + # @api private + class SkipFromParentList < GraphQL::Error; end + + # Just a singleton for implementing {Query::Context#skip} + # @api private + SKIP_FROM_PARENT_LIST = SkipFromParentList.new end end diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index 19af3e1412..2f4cde6e86 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -22,7 +22,12 @@ def initialize end module GraphQLResult - def initialize(result_name, parent_result, is_non_null_in_parent) + # FIXME: storing `represented_value` here holds onto a lot of objects, keeping them from GCing and increasing peak memory. + # ... but the value is only needed for lists with skippable items. + # Maybe we can set to `nil` unless we know this object is a skippable list item + # (i.e. a direct child of a list with `skip_items_on_raise: true`) + # FIXME: likewise for `field`. + def initialize(result_name, parent_result, is_non_null_in_parent, represented_value, field) @graphql_parent = parent_result if parent_result && parent_result.graphql_dead @graphql_dead = true @@ -31,6 +36,8 @@ def initialize(result_name, parent_result, is_non_null_in_parent) @graphql_is_non_null_in_parent = is_non_null_in_parent # Jump through some hoops to avoid creating this duplicate storage if at all possible. @graphql_metadata = nil + @represented_value = represented_value + @field = field end def path @@ -43,14 +50,38 @@ def build_path(path_array) end attr_accessor :graphql_dead - attr_reader :graphql_parent, :graphql_result_name, :graphql_is_non_null_in_parent + attr_reader :graphql_parent, :graphql_result_name, :graphql_is_non_null_in_parent, :represented_value, :field + + # True when this result is a List that was marked `skip_items_on_raise: true`. + # Descendants of this result will have `can_be_skipped?` be true. + # @return [nil, true] + attr_accessor :graphql_skip_list_items_that_raise + + # A result can be skipped when it's an ancestor of an item in a List marked `skip_items_on_raise: true`. + # @return [Boolean] + def can_be_skipped? + !nearest_skippable_parent_result.nil? + end + + # Crawls up the tree to find the first ancestor that can be skipped, that is, the first item that's a child + # of a List marked `skip_items_on_raise: true`. + # @return [GraphQLResult] + def nearest_skippable_parent_result + return @nearest_skippable_parent_result if defined?(@nearest_skippable_parent_result) + + @nearest_skippable_parent_result = if graphql_parent && graphql_parent.graphql_skip_list_items_that_raise + self + else + graphql_parent && graphql_parent.nearest_skippable_parent_result + end + end # @return [Hash] Plain-Ruby result data (`@graphql_metadata` contains Result wrapper objects) attr_accessor :graphql_result_data end class GraphQLResultHash - def initialize(_result_name, _parent_result, _is_non_null_in_parent) + def initialize(_result_name, _parent_result, _is_non_null_in_parent, _represented_value, _field) super @graphql_result_data = {} end @@ -138,7 +169,7 @@ def merge_into(into_result) class GraphQLResultArray include GraphQLResult - def initialize(_result_name, _parent_result, _is_non_null_in_parent) + def initialize(_result_name, _parent_result, _is_non_null_in_parent, _represented_value, _field) super @graphql_result_data = [] end @@ -198,7 +229,7 @@ def initialize(query:, lazies_at_depth:) @lazies_at_depth = lazies_at_depth @schema = query.schema @context = query.context - @response = GraphQLResultHash.new(nil, nil, false) + @response = GraphQLResultHash.new(nil, nil, false, query.root_value, nil) # Identify runtime directives by checking which of this schema's directives have overridden `def self.resolve` @runtime_directive_names = [] noop_resolve_owner = GraphQL::Schema::Directive.singleton_class @@ -265,7 +296,7 @@ def run_eager # directly evaluated and the results can be written right into the main response hash. tap_or_each(gathered_selections) do |selections, is_selection_array| if is_selection_array - selection_response = GraphQLResultHash.new(nil, nil, false) + selection_response = GraphQLResultHash.new(nil, nil, false, query.root_value, nil) final_response = @response else selection_response = @response @@ -547,7 +578,20 @@ def evaluate_selection_with_resolved_keyword_args(kwarg_arguments, resolved_argu err rescue StandardError => err begin - query.handle_or_reraise(err) + nearest_skippable_parent_result = selection_result.nearest_skippable_parent_result + + if nearest_skippable_parent_result && (on_raise_callback = nearest_skippable_parent_result.field.on_raise_callback) + begin + nearest_skippable_list_item = nearest_skippable_parent_result.represented_value.object + on_raise_callback.call(err, object, kwarg_arguments, context, nearest_skippable_parent_result.field, nearest_skippable_list_item) + rescue GraphQL::ExecutionError => err # TODO: DRY me + err + rescue StandardError => err + query.handle_or_reraise(err) + end + else + query.handle_or_reraise(err) + end rescue GraphQL::ExecutionError => ex_err ex_err end @@ -598,6 +642,33 @@ def set_result(selection_result, result_name, value, is_child_result, is_non_nul end elsif is_child_result selection_result.set_child_result(result_name, value) + elsif value == GraphQL::Execution::SKIP_FROM_PARENT_LIST + if selection_result.graphql_skip_list_items_that_raise # This is the first list that this item can be skipped from. + unless selection_result.is_a?(GraphQLResultArray) + raise "Invariant: unexpected result class #{selection_result.class} (#{selection_result.inspect})" + end + + selection_result.graphql_skip_at(result_name) + else # Propograte up to find the first list this item can be skipped from. + parent = selection_result.graphql_parent + name_in_parent = selection_result.graphql_result_name + + set_result(parent, name_in_parent, GraphQL::Execution::SKIP_FROM_PARENT_LIST, false, is_non_null_in_parent) + set_graphql_dead(selection_result) + end + elsif value == GraphQL::Execution::SKIP_FROM_PARENT_LIST + if selection_result.graphql_skip_list_items_that_raise # This is the first list that this item can be skipped from. + unless selection_result.is_a?(GraphQLResultArray) + raise "Invariant: unexpected result class #{selection_result.class} (#{selection_result.inspect})" + end + + selection_result.graphql_skip_at(result_name) + else # Propograte up to find the first list this item can be skipped from. + parent = selection_result.graphql_parent + name_in_parent = selection_result.graphql_result_name + set_result(parent, name_in_parent, GraphQL::Execution::SKIP_FROM_PARENT_LIST) + set_graphql_dead(selection_result) + end else selection_result.set_leaf(result_name, value) end @@ -690,6 +761,12 @@ def continue_value(value, parent_type, field, is_non_null, ast_node, result_name raise "Invariant: unexpected result class #{selection_result.class} (#{selection_result.inspect})" end HALT + elsif GraphQL::Execution::SKIP_FROM_PARENT_LIST == value + unless selection_result.can_be_skipped? + raise "Cannot skip list items from lists not marked `skip_items_on_raise: true`" + end + + set_result(selection_result, result_name, GraphQL::Execution::SKIP_FROM_PARENT_LIST, false, is_non_null) else # What could this actually _be_? Anyhow, # preserve the default behavior of doing nothing with it. @@ -781,7 +858,7 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select after_lazy(object_proxy, ast_node: ast_node, field: field, owner_object: owner_object, arguments: arguments, trace: false, result_name: result_name, result: selection_result) do |inner_object| continue_value = continue_value(inner_object, owner_type, field, is_non_null, ast_node, result_name, selection_result) if HALT != continue_value - response_hash = GraphQLResultHash.new(result_name, selection_result, is_non_null) + response_hash = GraphQLResultHash.new(result_name, selection_result, is_non_null, continue_value, field) set_result(selection_result, result_name, response_hash, true, is_non_null) gathered_selections = gather_selections(continue_value, current_type, next_selections) # There are two possibilities for `gathered_selections`: @@ -794,7 +871,7 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select # (Technically, it's possible that one of those entries _doesn't_ require isolation.) tap_or_each(gathered_selections) do |selections, is_selection_array| if is_selection_array - this_result = GraphQLResultHash.new(result_name, selection_result, is_non_null) + this_result = GraphQLResultHash.new(result_name, selection_result, is_non_null, continue_value, nil) final_result = response_hash else this_result = response_hash @@ -831,7 +908,8 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select # This is true for objects, unions, and interfaces use_dataloader_job = !inner_type.unwrap.kind.input? inner_type_non_null = inner_type.non_null? - response_list = GraphQLResultArray.new(result_name, selection_result, is_non_null) + response_list = GraphQLResultArray.new(result_name, selection_result, is_non_null, value, field) + response_list.graphql_skip_list_items_that_raise = current_type.skip_nodes_on_raise? set_result(selection_result, result_name, response_list, true, is_non_null) idx = nil list_value = begin diff --git a/lib/graphql/query/context.rb b/lib/graphql/query/context.rb index a1029158b6..16691839bf 100644 --- a/lib/graphql/query/context.rb +++ b/lib/graphql/query/context.rb @@ -11,6 +11,15 @@ def skip GraphQL::Execution::SKIP end + # Return this value to tell the runtime + # to exclude this whole object from parent list. + # + # The runtime will find the find the nearest parent list marked `skip_items_on_raise: true`, + # and exclude the entire list item (including this object). + def skip_from_parent_list + GraphQL::Execution::SKIP_FROM_PARENT_LIST + end + # Add error at query-level. # @param error [GraphQL::ExecutionError] an execution error # @return [void] diff --git a/lib/graphql/schema/build_from_definition.rb b/lib/graphql/schema/build_from_definition.rb index c670dff2b5..a546916d3b 100644 --- a/lib/graphql/schema/build_from_definition.rb +++ b/lib/graphql/schema/build_from_definition.rb @@ -478,7 +478,7 @@ def build_resolve_type(lookup_hash, directives, missing_type_handler) when GraphQL::Language::Nodes::NonNullType resolve_type_proc.call(ast_node.of_type).to_non_null_type when GraphQL::Language::Nodes::ListType - resolve_type_proc.call(ast_node.of_type).to_list_type + resolve_type_proc.call(ast_node.of_type).to_list_type(skip_nodes_on_raise: false) when String directives[ast_node] else diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 3be059d7da..f9bb7d5bed 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -83,6 +83,8 @@ def subscription_scope end attr_writer :subscription_scope + attr_reader :on_raise_callback + # Create a field instance from a list of arguments, keyword arguments, and a block. # # This method implements prioritization between the `resolver` or `mutation` defaults @@ -218,8 +220,10 @@ def method_conflict_warning? # @param ast_node [Language::Nodes::FieldDefinition, nil] If this schema was parsed from definition, this AST node defined the field # @param method_conflict_warning [Boolean] If false, skip the warning if this field's method conflicts with a built-in method # @param validates [Array] Configurations for validating this field - # @fallback_value [Object] A fallback value if the method is not defined - def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CONFIGURED, deprecation_reason: nil, method: nil, hash_key: nil, dig: nil, resolver_method: nil, connection: nil, max_page_size: NOT_CONFIGURED, default_page_size: NOT_CONFIGURED, scope: nil, introspection: false, camelize: true, trace: nil, complexity: nil, ast_node: nil, extras: EMPTY_ARRAY, extensions: EMPTY_ARRAY, connection_extension: self.class.connection_extension, resolver_class: nil, subscription_scope: nil, relay_node_field: false, relay_nodes_field: false, method_conflict_warning: true, broadcastable: NOT_CONFIGURED, arguments: EMPTY_HASH, directives: EMPTY_HASH, validates: EMPTY_ARRAY, fallback_value: NOT_CONFIGURED, &definition_block) + # @param fallback_value [Object] A fallback value if the method is not defined + # @param skip_nodes_on_raise [Boolean] true if this field's list items should be skipped, if resolving them led to an error being raised. Only applicable to List types. + # @param on_raise [Proc] A callback that's invoked when an error is raised while resolving this field's list items. Only applicable to List types. + def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CONFIGURED, deprecation_reason: nil, method: nil, hash_key: nil, dig: nil, resolver_method: nil, connection: nil, max_page_size: NOT_CONFIGURED, default_page_size: NOT_CONFIGURED, scope: nil, introspection: false, camelize: true, trace: nil, complexity: nil, ast_node: nil, extras: EMPTY_ARRAY, extensions: EMPTY_ARRAY, connection_extension: self.class.connection_extension, resolver_class: nil, subscription_scope: nil, relay_node_field: false, relay_nodes_field: false, method_conflict_warning: true, broadcastable: NOT_CONFIGURED, arguments: EMPTY_HASH, directives: EMPTY_HASH, validates: EMPTY_ARRAY, fallback_value: NOT_CONFIGURED, skip_nodes_on_raise: false, on_raise: nil, &definition_block) if name.nil? raise ArgumentError, "missing first `name` argument or keyword `name:`" end @@ -275,6 +279,8 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CON else true end + @skip_nodes_on_raise = skip_nodes_on_raise + @on_raise_callback = on_raise @connection = connection @max_page_size = max_page_size @default_page_size = default_page_size @@ -349,6 +355,17 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CON self.extensions.each(&:after_define_apply) @call_after_define = true + + # TODO: should this check be centralized in `GraphQL::Schema::Member::BuildType#parse_type` instead? + if skip_nodes_on_raise && !self.type.list? + raise ArgumentError, "The `skip_nodes_on_raise` option is only applicable to lists." + end + + # TODO: Rename "on_raise". + # This callback is specific to list _items_, but the "on_raise" naming doesn't make that clear. + if on_raise && !self.type.list? + raise ArgumentError, "The `on_raise` option is only applicable to lists." + end end # If true, subscription updates with this field can be shared between viewers @@ -584,9 +601,10 @@ def type raise MissingReturnTypeError, "Can't determine the return type for #{self.path} (it has `resolver: #{@resolver_class}`, perhaps that class is missing a `type ...` declaration, or perhaps its type causes a cyclical loading issue)" end nullable = @return_type_null.nil? ? @resolver_class.null : @return_type_null - Member::BuildType.parse_type(return_type, null: nullable) + skip_nodes_on_raise = @skip_nodes_on_raise.nil? ? @resolver_class.skip_nodes_on_raise : @skip_nodes_on_raise + Member::BuildType.parse_type(return_type, null: nullable, skip_nodes_on_raise: skip_nodes_on_raise) else - @type ||= Member::BuildType.parse_type(@return_type_expr, null: @return_type_null) + @type ||= Member::BuildType.parse_type(@return_type_expr, null: @return_type_null, skip_nodes_on_raise: @skip_nodes_on_raise) end rescue GraphQL::Schema::InvalidDocumentError, MissingReturnTypeError => err # Let this propagate up diff --git a/lib/graphql/schema/late_bound_type.rb b/lib/graphql/schema/late_bound_type.rb index b91532b83c..fc5c459f91 100644 --- a/lib/graphql/schema/late_bound_type.rb +++ b/lib/graphql/schema/late_bound_type.rb @@ -21,8 +21,13 @@ def to_non_null_type @to_non_null_type ||= GraphQL::Schema::NonNull.new(self) end - def to_list_type - @to_list_type ||= GraphQL::Schema::List.new(self) + # can we just inherit this from graphql/schema/member/type_system_helpers.rb? + def to_list_type(skip_nodes_on_raise: false) + if skip_nodes_on_raise + @to_skipping_list_type ||= GraphQL::Schema::List.new(self, skip_nodes_on_raise: true) + else + @to_list_type ||= GraphQL::Schema::List.new(self) + end end def inspect diff --git a/lib/graphql/schema/list.rb b/lib/graphql/schema/list.rb index 3ad0f313a8..aabf783ecb 100644 --- a/lib/graphql/schema/list.rb +++ b/lib/graphql/schema/list.rb @@ -8,6 +8,11 @@ class Schema class List < GraphQL::Schema::Wrapper include Schema::Member::ValidatesInput + def initialize(of_type, skip_nodes_on_raise: false) + super(of_type) + @skip_nodes_on_raise = skip_nodes_on_raise + end + # @return [GraphQL::TypeKinds::LIST] def kind GraphQL::TypeKinds::LIST @@ -18,6 +23,10 @@ def list? true end + def skip_nodes_on_raise? + @skip_nodes_on_raise + end + def to_type_signature "[#{@of_type.to_type_signature}]" end diff --git a/lib/graphql/schema/member/build_type.rb b/lib/graphql/schema/member/build_type.rb index a9e07e09c4..8a600a9cba 100644 --- a/lib/graphql/schema/member/build_type.rb +++ b/lib/graphql/schema/member/build_type.rb @@ -9,7 +9,7 @@ module BuildType module_function # @param type_expr [String, Class, GraphQL::BaseType] # @return [GraphQL::BaseType] - def parse_type(type_expr, null:) + def parse_type(type_expr, null:, skip_nodes_on_raise: false) list_type = false return_type = case type_expr @@ -85,7 +85,7 @@ def parse_type(type_expr, null:) # Apply list_type first, that way the # .to_non_null_type applies to the list type, not the inner type if list_type - return_type = return_type.to_list_type + return_type = return_type.to_list_type(skip_nodes_on_raise: skip_nodes_on_raise) end if !null diff --git a/lib/graphql/schema/member/type_system_helpers.rb b/lib/graphql/schema/member/type_system_helpers.rb index a2c9d9bd49..c9bbca07df 100644 --- a/lib/graphql/schema/member/type_system_helpers.rb +++ b/lib/graphql/schema/member/type_system_helpers.rb @@ -17,8 +17,12 @@ def to_non_null_type end # @return [Schema::List] Make a list-type representation of this type - def to_list_type - @to_list_type ||= GraphQL::Schema::List.new(self) + def to_list_type(skip_nodes_on_raise: false) + if skip_nodes_on_raise + @to_skipping_list_type ||= GraphQL::Schema::List.new(self, skip_nodes_on_raise: true) + else + @to_list_type ||= GraphQL::Schema::List.new(self) + end end # @return [Boolean] true if this is a non-nullable type. A nullable list of non-nullables is considered nullable. @@ -26,6 +30,11 @@ def non_null? false end + # @return [Boolean] true if this field's nodes should be skipped, if resolving them led to an error being raised. + def skip_nodes_on_raise? + false + end + # @return [Boolean] true if this is a list type. A non-nullable list is considered a list. def list? false diff --git a/lib/graphql/schema/non_null.rb b/lib/graphql/schema/non_null.rb index 455a065ae4..69ac3dcaa3 100644 --- a/lib/graphql/schema/non_null.rb +++ b/lib/graphql/schema/non_null.rb @@ -18,6 +18,11 @@ def non_null? true end + # @return [Boolean] true if this field's nodes should be skipped, if resolving them led to an error being raised. + def skip_nodes_on_raise? + @of_type.skip_nodes_on_raise? + end + # @return [Boolean] True if this type wraps a list type def list? @of_type.list? diff --git a/lib/graphql/schema/resolver.rb b/lib/graphql/schema/resolver.rb index 843016fdf6..586b832036 100644 --- a/lib/graphql/schema/resolver.rb +++ b/lib/graphql/schema/resolver.rb @@ -252,6 +252,9 @@ def null(allow_null = nil) @null.nil? ? (superclass.respond_to?(:null) ? superclass.null : true) : @null end + # @return [Boolean] + attr_reader :skip_nodes_on_raise + def resolver_method(new_method_name = nil) if new_method_name @resolver_method = new_method_name @@ -266,17 +269,19 @@ def resolver_method(new_method_name = nil) # TODO unify with {#null} # @param new_type [Class, Array, nil] If a type definition class is provided, it will be used as the return type of the field # @param null [true, false] Whether or not the field may return `nil` + # @param null [true, false] Whether or not the field may skip list items that raise errors. Only applicable to lists. # @return [Class] The type which this field returns. - def type(new_type = nil, null: nil) + def type(new_type = nil, null: nil, skip_nodes_on_raise: false) if new_type if null.nil? raise ArgumentError, "required argument `null:` is missing" end @type_expr = new_type @null = null + @skip_nodes_on_raise = skip_nodes_on_raise else if type_expr - GraphQL::Schema::Member::BuildType.parse_type(type_expr, null: self.null) + GraphQL::Schema::Member::BuildType.parse_type(type_expr, null: self.null, skip_nodes_on_raise: self.skip_nodes_on_raise) elsif superclass.respond_to?(:type) superclass.type else diff --git a/lib/graphql/types/relay/connection_behaviors.rb b/lib/graphql/types/relay/connection_behaviors.rb index a70eac4d4f..aab54b8dfa 100644 --- a/lib/graphql/types/relay/connection_behaviors.rb +++ b/lib/graphql/types/relay/connection_behaviors.rb @@ -11,6 +11,7 @@ def self.included(child_class) child_class.extend(ClassMethods) child_class.has_nodes_field(true) child_class.node_nullable(true) + child_class.skip_nodes_on_raise(false) child_class.edges_nullable(true) child_class.edge_nullable(true) child_class.module_eval { @@ -54,7 +55,7 @@ def default_relay? # class name to set defaults. You can call it again in the class definition # to override the default (or provide a value, if the default lookup failed). # @param field_options [Hash] Any extra keyword arguments to pass to the `field :edges, ...` and `field :nodes, ...` configurations - def edge_type(edge_type_class, edge_class: GraphQL::Pagination::Connection::Edge, node_type: edge_type_class.node_type, nodes_field: self.has_nodes_field, node_nullable: self.node_nullable, edges_nullable: self.edges_nullable, edge_nullable: self.edge_nullable, field_options: nil) + def edge_type(edge_type_class, edge_class: GraphQL::Pagination::Connection::Edge, node_type: edge_type_class.node_type, nodes_field: self.has_nodes_field, node_nullable: self.node_nullable, skip_nodes_on_raise: self.skip_nodes_on_raise, edges_nullable: self.edges_nullable, edge_nullable: self.edge_nullable, field_options: nil) # Set this connection's graphql name node_type_name = node_type.graphql_name @@ -78,7 +79,7 @@ def edge_type(edge_type_class, edge_class: GraphQL::Pagination::Connection::Edge field(**base_field_options) - define_nodes_field(node_nullable, field_options: field_options) if nodes_field + define_nodes_field(node_nullable, skip_nodes_on_raise, field_options: field_options) if nodes_field description("The connection type for #{node_type_name}.") end @@ -89,8 +90,8 @@ def scope_items(items, context) end # Add the shortcut `nodes` field to this connection and its subclasses - def nodes_field(node_nullable: self.node_nullable, field_options: nil) - define_nodes_field(node_nullable, field_options: field_options) + def nodes_field(node_nullable: self.node_nullable, skip_nodes_on_raise: self.skip_nodes_on_raise, field_options: nil) + define_nodes_field(node_nullable, skip_nodes_on_raise, field_options: field_options) end def authorized?(obj, ctx) @@ -112,6 +113,14 @@ def node_nullable(new_value = nil) end end + def skip_nodes_on_raise(new_value = nil) + if new_value.nil? + defined?(@skip_nodes_on_raise) ? @skip_nodes_on_raise : superclass.skip_nodes_on_raise + else + @skip_nodes_on_raise = new_value + end + end + # Set the default `edges_nullable` for this class and its child classes. (Defaults to `true`.) # Use `edges_nullable(false)` in your base class to make non-null `edges` fields. def edges_nullable(new_value = nil) @@ -148,7 +157,7 @@ def has_nodes_field(new_value = nil) private - def define_nodes_field(nullable, field_options: nil) + def define_nodes_field(nullable, skip_nodes_on_raise, field_options: nil) base_field_options = { name: :nodes, type: [@node_type, null: nullable], @@ -157,6 +166,7 @@ def define_nodes_field(nullable, field_options: nil) connection: false, # Assume that the connection was scoped before this step: scope: false, + skip_nodes_on_raise: skip_nodes_on_raise, } if field_options base_field_options.merge!(field_options) diff --git a/spec/graphql/execution/interpreter/list_item_skipping_spec.rb b/spec/graphql/execution/interpreter/list_item_skipping_spec.rb new file mode 100644 index 0000000000..3c4a8490f5 --- /dev/null +++ b/spec/graphql/execution/interpreter/list_item_skipping_spec.rb @@ -0,0 +1,264 @@ +# frozen_string_literal: true +require "spec_helper" + +describe GraphQL::Execution::Interpreter do + describe "skipping errored items in Lists" do + describe "when the List's items aren't skippable" do + it "raises the error if the list's items are required and not skippable" do + assert_raises(ListItemSkippingTest::Boom) do + query_list(list_field: "arrayOfRequiredItems") + end + end + + it "raises the error if the list's items are nullable and not skippable" do + assert_raises(ListItemSkippingTest::Boom) do + query_list(list_field: "arrayOfNullableItems") + end + end + end + + describe "when the List's items are skippable" do + it "skips errored items if the list's items are required and skippable" do + items = query_list(list_field: "arrayOfRequiredSkippableItems") + + assert_equal ["Item 1", "Item 3"], items.fetch("data").fetch("arrayOfRequiredSkippableItems").map { |h| h["title"] } + end + + it "skips errored items if the list's items are nullable and skippable" do + items = query_list(list_field: "arrayOfNullableSkippableItems") + + assert_equal ["Item 1", "Item 3"], items.fetch("data").fetch("arrayOfNullableSkippableItems").map { |h| h["title"] } + end + end + end + + describe "skipping errored items in Connections" do + # TODO: test edges + # How should edges be skipped? Does skipping a node mean its edge should also be skipped? + + describe "when the Connection's items aren't skippable" do + it "raises the error if the connection's items are required and not skippable" do + skip "on_raise API not implemented for Connections yet" + + assert_raises(ListItemSkippingTest::Boom) do + query_connection(connection_field: "connectionOfRequiredItems") + end + end + + it "raises the error if the connection's items are nullable and not skippable" do + skip "on_raise API not implemented for Connections yet" + + assert_raises(ListItemSkippingTest::Boom) do + query_connection(connection_field: "connectionOfNullableItems") + end + end + end + + describe "when the Connection's items are skippable" do + it "skips errored items if the connection's items are required and skippable" do + skip "on_raise API not implemented for Connections yet" + items = query_connection(connection_field: "connectionOfRequiredSkippableItems") + + assert_equal ["Item 1", "Item 3"], items.dig("data", "connectionOfRequiredSkippableItems", "nodes").map { |h| h["title"] } + end + + it "skips errored items if the connection's items are nullable and skippable" do + skip "on_raise API not implemented for Connections yet" + items = query_connection(connection_field: "connectionOfNullableSkippableItems") + + assert_equal ["Item 1", "Item 3"], items.dig("data", "connectionOfNullableSkippableItems", "nodes").map { |h| h["title"] } + end + end + end + + describe "other" do + it "dump schema" do # just to help during development + puts ListItemSkippingTest::Schema.to_definition + end + + it "raises ArgumentError when setting skip_nodes_on_raise on a field that isn't a list or connection" do + c = Class.new(GraphQL::Schema::Object) + + e = assert_raises(ArgumentError) do + c.field(:not_a_list_or_connection, String, skip_nodes_on_raise: true) + end + + assert_match /The `skip_nodes_on_raise` option is only applicable to lists./, e.message + end + end + + private + + def query_list(list_field:) + ListItemSkippingTest::Schema.execute(<<~QUERY).to_h + query { + #{list_field} { + title + subfield + } + } + QUERY + end + + def query_connection(connection_field:) + ListItemSkippingTest::Schema.execute(<<~QUERY).to_h + query { + #{connection_field} { + nodes { + title + subfield + } + } + } + QUERY + end +end + +module ListItemSkippingTest + class Boom < RuntimeError + def initialize = super("boom") + end + + class Item < Struct.new(:title, :subfield_should_raise, keyword_init: true) + def subfield + if subfield_should_raise + raise Boom + end + + "subfield for #{title}" + end + + def to_s = "#" + end +end + +module ListItemSkippingTest + class ItemType < GraphQL::Schema::Object + # connection_type_class(SkippableItemConnection) + + field :title, String, null: false + field :should_skip, Boolean, null: false + field :subfield, String, null: false + + def to_s = "#" + + # def self.scope_items(items, _context) = items + end + + class ItemEdge < GraphQL::Types::Relay::BaseEdge + node_type ItemType + end + + class RequiredItemConnection < GraphQL::Types::Relay::BaseConnection + node_nullable(false) + edges_nullable(false) + edge_nullable(false) + has_nodes_field(true) + edge_type ItemEdge + + # def self.inspect = to_s + # def self.to_s = "#" # This crashes the debugger?? + end + + class NullableItemConnection < GraphQL::Types::Relay::BaseConnection + node_nullable(true) + edges_nullable(true) + edge_nullable(true) + has_nodes_field(true) + edge_type ItemEdge + end + + class RequiredSkippableItemConnection < RequiredItemConnection + skip_nodes_on_raise(true) # Come back to this, should this property be defined here on the connection, or on the field decl where this connection type is used + # Node type is not heritable, so we need to set it here again, otherwise you get: + # NoMethodError: undefined method `scope_items' for nil:NilClass + # /Users/alex/src/github.com/amomchilov/graphql-ruby/lib/graphql/types/relay/connection_behaviors.rb:71:in `scope_items' + # /Users/alex/src/github.com/amomchilov/graphql-ruby/lib/graphql/schema/field/scope_extension.rb:13:in `after_resolve' + # /Users/alex/src/github.com/amomchilov/graphql-ruby/lib/graphql/schema/field.rb:830:in `block (2 levels) in + # ... + edge_type ItemEdge + end + + class NullableSkippableItemConnection < NullableItemConnection + skip_nodes_on_raise(true) + edge_type ItemEdge # Node type is not heritable, so we need to set it here again. See above. + end + + class QueryType < GraphQL::Schema::Object + field :array_of_required_items, + [ItemType, null: false], + method: :items_list, + null: false + + field :array_of_nullable_items, + [ItemType, null: true], + method: :items_list, + null: false + + field :array_of_required_skippable_items, + [ItemType, null: false], + method: :items_list, + null: false, + skip_nodes_on_raise: true, + on_raise: -> (err, current_object, args, context, current_field, nearest_skippable_list_item) { + puts(:array_of_required_skippable_items) + context.skip_from_parent_list + } + + field :array_of_nullable_skippable_items, + [ItemType, null: true], + method: :items_list, + null: false, + skip_nodes_on_raise: true, + on_raise: -> (err, current_object, args, context, current_field, nearest_skippable_list_item) { + puts(:array_of_required_skippable_items) + context.skip_from_parent_list + } + + field :connection_of_required_items, + RequiredItemConnection, + method: :items_connection, + null: false + + field :connection_of_nullable_items, + NullableItemConnection, + method: :items_connection, + null: false + + field :connection_of_required_skippable_items, + RequiredSkippableItemConnection, + method: :items_connection, + null: false + + field :connection_of_nullable_skippable_items, + NullableSkippableItemConnection, + method: :items_connection, + null: false + + def items_list + [ + Item.new(title: "Item 1", subfield_should_raise: false), + Item.new(title: "Item 2 💣", subfield_should_raise: true), + Item.new(title: "Item 3", subfield_should_raise: false), + ] + end + + def items_connection + items_list + end + + # TODO: why are these necessary? Shouldn't `method: :items_list` be enough? + alias_method :array_of_required_items, :items_list + alias_method :array_of_nullable_items, :items_list + alias_method :array_of_required_skippable_items, :items_list + alias_method :array_of_nullable_skippable_items, :items_list + alias_method :connection_of_required_items, :items_connection + alias_method :connection_of_nullable_items, :items_connection + alias_method :connection_of_required_skippable_items, :items_connection + alias_method :connection_of_nullable_skippable_items, :items_connection + end + + class Schema < GraphQL::Schema + query QueryType + end +end