Skip to content

Commit

Permalink
Remove more visible? calls to unused arguments
Browse files Browse the repository at this point in the history
Replaces more usages of `warden.arguments` during static validation
(since they trigger `visible?` calls for all arguments on a type
regardless if they are specified or not) with more selective `warden.get_argument`.

rmosolgo#2985 which fixed some of
these occurrences, but not all of them.

This also required additional backwards compatible work to port `get_argument` to the legacy types.
  • Loading branch information
swalkinshaw committed Jun 17, 2020
1 parent 0684cc3 commit 9d74972
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 13 deletions.
4 changes: 4 additions & 0 deletions lib/graphql/directive.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ def inspect
def type_class
metadata[:type_class]
end

def get_argument(argument_name)
arguments[argument_name]
end
end
end

Expand Down
4 changes: 4 additions & 0 deletions lib/graphql/field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ def type_class
metadata[:type_class]
end

def get_argument(argument_name)
arguments[argument_name]
end

private

def build_default_resolver
Expand Down
4 changes: 4 additions & 0 deletions lib/graphql/input_object_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ def coerce_result(value, ctx = nil)
result
end

def get_argument(argument_name)
arguments[argument_name]
end

private

def coerce_non_null_input(value, ctx)
Expand Down
4 changes: 3 additions & 1 deletion lib/graphql/schema/member/has_arguments.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ def arguments

# @return [GraphQL::Schema::Argument, nil] Argument defined on this thing, fetched by name.
def get_argument(argument_name)
if (a = own_arguments[argument_name])
a = own_arguments[argument_name]

if a || !self.is_a?(Class)
a
else
for ancestor in ancestors
Expand Down
14 changes: 7 additions & 7 deletions lib/graphql/static_validation/literal_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,30 +95,30 @@ def constant_scalar?(ast_value)
def required_input_fields_are_present(type, ast_node)
# TODO - would be nice to use these to create an error message so the caller knows
# that required fields are missing
required_field_names = @warden.arguments(type)
.select { |f| f.type.kind.non_null? }
required_field_names = type.arguments.each_value
.select { |argument| argument.type.kind.non_null? && @warden.get_argument(type, argument.name) }
.map(&:name)

present_field_names = ast_node.arguments.map(&:name)
missing_required_field_names = required_field_names - present_field_names
if @context.schema.error_bubbling
missing_required_field_names.empty? ? @valid_response : @invalid_response
else
results = missing_required_field_names.map do |name|
arg_type = @warden.arguments(type).find { |f| f.name == name }.type
arg_type = @warden.get_argument(type, name).type
recursively_validate(GraphQL::Language::Nodes::NullValue.new(name: name), arg_type)
end
merge_results(results)
end
end

def present_input_field_values_are_valid(type, ast_node)
field_map = @warden.arguments(type).reduce({}) { |m, f| m[f.name] = f; m}
results = ast_node.arguments.map do |value|
field = field_map[value.name]
field = @warden.get_argument(type, value.name)
# we want to call validate on an argument even if it's an invalid one
# so that our raise exception is on it instead of the entire InputObject
type = field && field.type
recursively_validate(value.value, type)
field_type = field && field.type
recursively_validate(value.value, field_type)
end
merge_results(results)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module ArgumentsAreDefined
def on_argument(node, parent)
parent_defn = parent_definition(parent)

if parent_defn && context.warden.arguments(parent_defn).any? { |arg| arg.name == node.name }
if parent_defn && context.warden.get_argument(parent_defn, node.name)
super
elsif parent_defn
kind_of_node = node_type(parent)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ def on_directive(node, _parent)

def assert_required_args(ast_node, defn)
present_argument_names = ast_node.arguments.map(&:name)
required_argument_names = context.warden.arguments(defn)
.select { |a| a.type.kind.non_null? && !a.default_value? }
required_argument_names = defn.arguments.each_value
.select { |a| a.type.kind.non_null? && !a.default_value? && context.warden.get_argument(defn, a.name) }
.map(&:name)

missing_names = required_argument_names - present_argument_names
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ def get_parent_type(context, parent)
context.field_definition
end

parent_type = context.warden.arguments(defn)
.find{|f| f.name == parent_name(parent, defn) }
parent_type = context.warden.get_argument(defn, parent_name(parent, defn))
parent_type ? parent_type.type.unwrap : nil
end

Expand Down

0 comments on commit 9d74972

Please sign in to comment.