Skip to content

Commit 9a21dc9

Browse files
amomchilovamomchilov
committed
Implement ability to skip list items on raise
1 parent 71bc874 commit 9a21dc9

File tree

13 files changed

+391
-19
lines changed

13 files changed

+391
-19
lines changed

‎lib/graphql/execution.rb‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,12 @@ class Skip < GraphQL::Error; end
1414
# Just a singleton for implementing {Query::Context#skip}
1515
# @api private
1616
SKIP = Skip.new
17+
18+
# @api private
19+
class SkipFromParentList < GraphQL::Error; end
20+
21+
# Just a singleton for implementing {Query::Context#skip}
22+
# @api private
23+
SKIP_FROM_PARENT_LIST = SkipFromParentList.new
1724
end
1825
end

‎lib/graphql/execution/interpreter/runtime.rb‎

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,19 @@ def build_path(path_array)
4545
attr_accessor :graphql_dead
4646
attr_reader :graphql_parent, :graphql_result_name, :graphql_is_non_null_in_parent
4747

48+
# True when this result is a List that was marked `skip_items_on_raise: true`.
49+
# Descendants of this result will have `can_be_skipped?` be true.
50+
# @return [nil, true]
51+
attr_accessor :graphql_skip_list_items_that_raise
52+
53+
# A result can be skipped when it's an ancestor of an item in a List marked `skip_items_on_raise: true`.
54+
# @return [Boolean]
55+
def can_be_skipped?
56+
return @can_be_skipped if defined?(@can_be_skipped)
57+
58+
@can_be_skipped = graphql_skip_list_items_that_raise || !!(graphql_parent && graphql_parent.can_be_skipped?)
59+
end
60+
4861
# @return [Hash] Plain-Ruby result data (`@graphql_metadata` contains Result wrapper objects)
4962
attr_accessor :graphql_result_data
5063
end
@@ -547,7 +560,15 @@ def evaluate_selection_with_resolved_keyword_args(kwarg_arguments, resolved_argu
547560
err
548561
rescue StandardError => err
549562
begin
550-
query.handle_or_reraise(err)
563+
if selection_result.can_be_skipped?
564+
context.skip_from_parent_list # Skip silently without informing the user's `rescue_from` block.
565+
else
566+
begin
567+
query.handle_or_reraise(err)
568+
rescue GraphQL::ExecutionError => ex_err
569+
ex_err
570+
end
571+
end
551572
rescue GraphQL::ExecutionError => ex_err
552573
ex_err
553574
end
@@ -598,6 +619,33 @@ def set_result(selection_result, result_name, value, is_child_result, is_non_nul
598619
end
599620
elsif is_child_result
600621
selection_result.set_child_result(result_name, value)
622+
elsif value == GraphQL::Execution::SKIP_FROM_PARENT_LIST
623+
if selection_result.graphql_skip_list_items_that_raise # This is the first list that this item can be skipped from.
624+
unless selection_result.is_a?(GraphQLResultArray)
625+
raise "Invariant: unexpected result class #{selection_result.class} (#{selection_result.inspect})"
626+
end
627+
628+
selection_result.graphql_skip_at(result_name)
629+
else # Propograte up to find the first list this item can be skipped from.
630+
parent = selection_result.graphql_parent
631+
name_in_parent = selection_result.graphql_result_name
632+
633+
set_result(parent, name_in_parent, GraphQL::Execution::SKIP_FROM_PARENT_LIST, false, is_non_null_in_parent)
634+
set_graphql_dead(selection_result)
635+
end
636+
elsif value == GraphQL::Execution::SKIP_FROM_PARENT_LIST
637+
if selection_result.graphql_skip_list_items_that_raise # This is the first list that this item can be skipped from.
638+
unless selection_result.is_a?(GraphQLResultArray)
639+
raise "Invariant: unexpected result class #{selection_result.class} (#{selection_result.inspect})"
640+
end
641+
642+
selection_result.graphql_skip_at(result_name)
643+
else # Propograte up to find the first list this item can be skipped from.
644+
parent = selection_result.graphql_parent
645+
name_in_parent = selection_result.graphql_result_name
646+
set_result(parent, name_in_parent, GraphQL::Execution::SKIP_FROM_PARENT_LIST)
647+
set_graphql_dead(selection_result)
648+
end
601649
else
602650
selection_result.set_leaf(result_name, value)
603651
end
@@ -690,6 +738,12 @@ def continue_value(value, parent_type, field, is_non_null, ast_node, result_name
690738
raise "Invariant: unexpected result class #{selection_result.class} (#{selection_result.inspect})"
691739
end
692740
HALT
741+
elsif GraphQL::Execution::SKIP_FROM_PARENT_LIST == value
742+
unless selection_result.can_be_skipped?
743+
raise "Cannot skip list items from lists not marked `skip_items_on_raise: true`"
744+
end
745+
746+
set_result(selection_result, result_name, GraphQL::Execution::SKIP_FROM_PARENT_LIST, false, is_non_null)
693747
else
694748
# What could this actually _be_? Anyhow,
695749
# preserve the default behavior of doing nothing with it.
@@ -832,6 +886,7 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
832886
use_dataloader_job = !inner_type.unwrap.kind.input?
833887
inner_type_non_null = inner_type.non_null?
834888
response_list = GraphQLResultArray.new(result_name, selection_result, is_non_null)
889+
response_list.graphql_skip_list_items_that_raise = current_type.skip_nodes_on_raise?
835890
set_result(selection_result, result_name, response_list, true, is_non_null)
836891
idx = nil
837892
list_value = begin

‎lib/graphql/query/context.rb‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,15 @@ def skip
1111
GraphQL::Execution::SKIP
1212
end
1313

14+
# Return this value to tell the runtime
15+
# to exclude this whole object from parent list.
16+
#
17+
# The runtime will find the find the nearest parent list marked `skip_items_on_raise: true`,
18+
# and exclude the entire list item (including this object).
19+
def skip_from_parent_list
20+
GraphQL::Execution::SKIP_FROM_PARENT_LIST
21+
end
22+
1423
# Add error at query-level.
1524
# @param error [GraphQL::ExecutionError] an execution error
1625
# @return [void]

‎lib/graphql/schema/build_from_definition.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ def build_resolve_type(lookup_hash, directives, missing_type_handler)
478478
when GraphQL::Language::Nodes::NonNullType
479479
resolve_type_proc.call(ast_node.of_type).to_non_null_type
480480
when GraphQL::Language::Nodes::ListType
481-
resolve_type_proc.call(ast_node.of_type).to_list_type
481+
resolve_type_proc.call(ast_node.of_type).to_list_type(skip_nodes_on_raise: false)
482482
when String
483483
directives[ast_node]
484484
else

‎lib/graphql/schema/field.rb‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,9 @@ def method_conflict_warning?
218218
# @param ast_node [Language::Nodes::FieldDefinition, nil] If this schema was parsed from definition, this AST node defined the field
219219
# @param method_conflict_warning [Boolean] If false, skip the warning if this field's method conflicts with a built-in method
220220
# @param validates [Array<Hash>] Configurations for validating this field
221-
# @fallback_value [Object] A fallback value if the method is not defined
222-
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)
221+
# @param fallback_value [Object] A fallback value if the method is not defined
222+
# @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.
223+
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, &definition_block)
223224
if name.nil?
224225
raise ArgumentError, "missing first `name` argument or keyword `name:`"
225226
end
@@ -275,6 +276,7 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CON
275276
else
276277
true
277278
end
279+
@skip_nodes_on_raise = skip_nodes_on_raise
278280
@connection = connection
279281
@max_page_size = max_page_size
280282
@default_page_size = default_page_size
@@ -349,6 +351,11 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CON
349351

350352
self.extensions.each(&:after_define_apply)
351353
@call_after_define = true
354+
355+
# TODO: should this check be centralized in `GraphQL::Schema::Member::BuildType#parse_type` instead?
356+
if skip_nodes_on_raise && !self.type.list?
357+
raise ArgumentError, "The `skip_nodes_on_raise` option is only applicable to lists."
358+
end
352359
end
353360

354361
# If true, subscription updates with this field can be shared between viewers
@@ -584,9 +591,10 @@ def type
584591
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)"
585592
end
586593
nullable = @return_type_null.nil? ? @resolver_class.null : @return_type_null
587-
Member::BuildType.parse_type(return_type, null: nullable)
594+
skip_nodes_on_raise = @skip_nodes_on_raise.nil? ? @resolver_class.skip_nodes_on_raise : @skip_nodes_on_raise
595+
Member::BuildType.parse_type(return_type, null: nullable, skip_nodes_on_raise: skip_nodes_on_raise)
588596
else
589-
@type ||= Member::BuildType.parse_type(@return_type_expr, null: @return_type_null)
597+
@type ||= Member::BuildType.parse_type(@return_type_expr, null: @return_type_null, skip_nodes_on_raise: @skip_nodes_on_raise)
590598
end
591599
rescue GraphQL::Schema::InvalidDocumentError, MissingReturnTypeError => err
592600
# Let this propagate up

‎lib/graphql/schema/late_bound_type.rb‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,13 @@ def to_non_null_type
2121
@to_non_null_type ||= GraphQL::Schema::NonNull.new(self)
2222
end
2323

24-
def to_list_type
25-
@to_list_type ||= GraphQL::Schema::List.new(self)
24+
# can we just inherit this from graphql/schema/member/type_system_helpers.rb?
25+
def to_list_type(skip_nodes_on_raise: false)
26+
if skip_nodes_on_raise
27+
@to_skipping_list_type ||= GraphQL::Schema::List.new(self, skip_nodes_on_raise: true)
28+
else
29+
@to_list_type ||= GraphQL::Schema::List.new(self)
30+
end
2631
end
2732

2833
def inspect

‎lib/graphql/schema/list.rb‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ class Schema
88
class List < GraphQL::Schema::Wrapper
99
include Schema::Member::ValidatesInput
1010

11+
def initialize(of_type, skip_nodes_on_raise: false)
12+
super(of_type)
13+
@skip_nodes_on_raise = skip_nodes_on_raise
14+
end
15+
1116
# @return [GraphQL::TypeKinds::LIST]
1217
def kind
1318
GraphQL::TypeKinds::LIST
@@ -18,6 +23,10 @@ def list?
1823
true
1924
end
2025

26+
def skip_nodes_on_raise?
27+
@skip_nodes_on_raise
28+
end
29+
2130
def to_type_signature
2231
"[#{@of_type.to_type_signature}]"
2332
end

‎lib/graphql/schema/member/build_type.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ module BuildType
99
module_function
1010
# @param type_expr [String, Class, GraphQL::BaseType]
1111
# @return [GraphQL::BaseType]
12-
def parse_type(type_expr, null:)
12+
def parse_type(type_expr, null:, skip_nodes_on_raise: false)
1313
list_type = false
1414

1515
return_type = case type_expr
@@ -85,7 +85,7 @@ def parse_type(type_expr, null:)
8585
# Apply list_type first, that way the
8686
# .to_non_null_type applies to the list type, not the inner type
8787
if list_type
88-
return_type = return_type.to_list_type
88+
return_type = return_type.to_list_type(skip_nodes_on_raise: skip_nodes_on_raise)
8989
end
9090

9191
if !null

‎lib/graphql/schema/member/type_system_helpers.rb‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,24 @@ def to_non_null_type
1717
end
1818

1919
# @return [Schema::List] Make a list-type representation of this type
20-
def to_list_type
21-
@to_list_type ||= GraphQL::Schema::List.new(self)
20+
def to_list_type(skip_nodes_on_raise: false)
21+
if skip_nodes_on_raise
22+
@to_skipping_list_type ||= GraphQL::Schema::List.new(self, skip_nodes_on_raise: true)
23+
else
24+
@to_list_type ||= GraphQL::Schema::List.new(self)
25+
end
2226
end
2327

2428
# @return [Boolean] true if this is a non-nullable type. A nullable list of non-nullables is considered nullable.
2529
def non_null?
2630
false
2731
end
2832

33+
# @return [Boolean] true if this field's nodes should be skipped, if resolving them led to an error being raised.
34+
def skip_nodes_on_raise?
35+
false
36+
end
37+
2938
# @return [Boolean] true if this is a list type. A non-nullable list is considered a list.
3039
def list?
3140
false

‎lib/graphql/schema/non_null.rb‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ def non_null?
1818
true
1919
end
2020

21+
# @return [Boolean] true if this field's nodes should be skipped, if resolving them led to an error being raised.
22+
def skip_nodes_on_raise?
23+
@of_type.skip_nodes_on_raise?
24+
end
25+
2126
# @return [Boolean] True if this type wraps a list type
2227
def list?
2328
@of_type.list?

0 commit comments

Comments
 (0)