Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation - more data in errors #1970

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
342 changes: 342 additions & 0 deletions javascript_client/yarn.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion lib/graphql/query/validation_pipeline.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def valid?
@valid
end

# @return [Array<GraphQL::StaticValidation::Message>] Static validation errors for the query string
# @return [Array<GraphQL::StaticValidation::Error >] Static validation errors for the query string
def validation_errors
ensure_has_validated
@validation_errors
Expand Down
2 changes: 1 addition & 1 deletion lib/graphql/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ def using_ast_analysis?

# Validate a query string according to this schema.
# @param string_or_document [String, GraphQL::Language::Nodes::Document]
# @return [Array<GraphQL::StaticValidation::Message>]
# @return [Array<GraphQL::StaticValidation::Error >]
def validate(string_or_document, rules: nil)
doc = if string_or_document.is_a?(String)
GraphQL.parse(string_or_document)
Expand Down
2 changes: 1 addition & 1 deletion lib/graphql/static_validation.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true
require "graphql/static_validation/message"
require "graphql/static_validation/error"
require "graphql/static_validation/definition_dependencies"
require "graphql/static_validation/type_stack"
require "graphql/static_validation/validator"
Expand Down
10 changes: 4 additions & 6 deletions lib/graphql/static_validation/base_visitor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,13 +172,11 @@ def on_fragment_with_type(node)

private

# Error `message` is located at `node`
def add_error(message, nodes, path: nil)
path ||= @path.dup
nodes = Array(nodes)
m = GraphQL::StaticValidation::Message.new(message, nodes: nodes, path: path)
context.errors << m
def add_error(error)
error.path ||= @path.dup
context.errors << error
end

end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,32 @@
module GraphQL
module StaticValidation
# Generates GraphQL-compliant validation message.
class Message
class Error
# Convenience for validators
module MessageHelper
# Error `message` is located at `node`
def message(message, nodes, context: nil, path: nil)
module ErrorHelper
# Error `error_message` is located at `node`
def error(error_message, nodes, context: nil, path: nil, extensions: {})
path ||= context.path
nodes = Array(nodes)
GraphQL::StaticValidation::Message.new(message, nodes: nodes, path: path)
GraphQL::StaticValidation::Error.new(error_message, nodes: nodes, path: path)
end
end

attr_reader :message, :path
attr_reader :message
attr_accessor :path

def initialize(message, path: [], nodes: [])
def initialize(message, path: nil, nodes: [])
@message = message
@nodes = nodes
@nodes = Array(nodes)
@path = path
end

# A hash representation of this Message
def to_h
{
"message" => message,
"locations" => locations,
"fields" => path,
}
"locations" => locations
}.tap { |h| h["path"] = path unless path.nil? }
end

private
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,22 @@ def on_argument(node, parent)
begin
valid = context.valid_literal?(node.value, arg_defn.type)
rescue GraphQL::CoercionError => err
error_message = err.message
error = GraphQL::StaticValidation::ArgumentLiteralsAreCompatibleError.new(err.message, nodes: parent, type: "CoercionError")
end

if !valid
error_message ||= begin
error ||= begin
kind_of_node = node_type(parent)
error_arg_name = parent_name(parent, parent_defn)
"Argument '#{node.name}' on #{kind_of_node} '#{error_arg_name}' has an invalid value. Expected type '#{arg_defn.type}'."
end

add_error(error_message, parent)
GraphQL::StaticValidation::ArgumentLiteralsAreCompatibleError.new(
"Argument '#{node.name}' on #{kind_of_node} '#{error_arg_name}' has an invalid value. Expected type '#{arg_defn.type}'.",
nodes: parent,
type: kind_of_node,
argument: node.name
)
end
add_error(error)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true
module GraphQL
module StaticValidation
class ArgumentLiteralsAreCompatibleError < StaticValidation::Error
attr_reader :type_name
attr_reader :argument_name

def initialize(message, path: nil, nodes: [], type:, argument: nil)
super(message, path: path, nodes: nodes)
@type_name = type
@argument_name = argument
end

# A hash representation of this Message
def to_h
extensions = {
"code" => code,
"typeName" => type_name
}.tap { |h| h["argumentName"] = argument_name unless argument_name.nil? }

super.merge({
"extensions" => extensions
})
end

private
def code
"argumentLiteralsIncompatible"
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
module GraphQL
module StaticValidation
module ArgumentNamesAreUnique
include GraphQL::StaticValidation::Message::MessageHelper
include GraphQL::StaticValidation::Error::ErrorHelper

def on_field(node, parent)
validate_arguments(node)
Expand All @@ -21,7 +21,7 @@ def validate_arguments(node)
argument_defns.each { |a| args_by_name[a.name] << a }
args_by_name.each do |name, defns|
if defns.size > 1
add_error("There can be only one argument named \"#{name}\"", defns)
add_error(GraphQL::StaticValidation::ArgumentNamesAreUniqueError.new("There can be only one argument named \"#{name}\"", nodes: defns, name: name))
end
end
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# frozen_string_literal: true
module GraphQL
module StaticValidation
class ArgumentNamesAreUniqueError < StaticValidation::Error
attr_reader :name

def initialize(message, path: nil, nodes: [], name:)
super(message, path: path, nodes: nodes)
@name = name
end

# A hash representation of this Message
def to_h
extensions = {
"code" => code,
"name" => name
}

super.merge({
"extensions" => extensions
})
end

private
def code
"argumentNotUnique"
end
end
end
end

8 changes: 7 additions & 1 deletion lib/graphql/static_validation/rules/arguments_are_defined.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ def on_argument(node, parent)
elsif parent_defn
kind_of_node = node_type(parent)
error_arg_name = parent_name(parent, parent_defn)
add_error("#{kind_of_node} '#{error_arg_name}' doesn't accept argument '#{node.name}'", node)
add_error(GraphQL::StaticValidation::ArgumentsAreDefinedError.new(
"#{kind_of_node} '#{error_arg_name}' doesn't accept argument '#{node.name}'",
nodes: node,
name: error_arg_name,
type: kind_of_node,
argument: node.name
))
else
# Some other weird error
super
Expand Down
36 changes: 36 additions & 0 deletions lib/graphql/static_validation/rules/arguments_are_defined_error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# frozen_string_literal: true
module GraphQL
module StaticValidation
class ArgumentsAreDefinedError < StaticValidation::Error
attr_reader :name
attr_reader :type_name
attr_reader :argument_name

def initialize(message, path: nil, nodes: [], name:, type:, argument:)
super(message, path: path, nodes: nodes)
@name = name
@type_name = type
@argument_name = argument
end

# A hash representation of this Message
def to_h
extensions = {
"code" => code,
"name" => name,
"typeName" => type_name,
"argumentName" => argument_name
}

super.merge({
"extensions" => extensions
})
end

private
def code
"argumentNotAccepted"
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ def initialize(*)

def on_directive(node, parent)
if !@directive_names.include?(node.name)
add_error("Directive @#{node.name} is not defined", node)
add_error(GraphQL::StaticValidation::DirectivesAreDefinedError.new(
"Directive @#{node.name} is not defined",
nodes: node,
directive: node.name
))
else
super
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# frozen_string_literal: true
module GraphQL
module StaticValidation
class DirectivesAreDefinedError < StaticValidation::Error
attr_reader :directive_name

def initialize(message, path: nil, nodes: [], directive:)
super(message, path: path, nodes: nodes)
@directive_name = directive
end

# A hash representation of this Message
def to_h
extensions = {
"code" => code,
"directiveName" => directive_name
}

super.merge({
"extensions" => extensions
})
end

private
def code
"undefinedDirective"
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,24 @@ def validate_location(ast_directive, ast_parent, directives)
required_location = SIMPLE_LOCATIONS[ast_parent.class]
assert_includes_location(directive_defn, ast_directive, required_location)
else
add_error("Directives can't be applied to #{ast_parent.class.name}s", ast_directive)
add_error(GraphQL::StaticValidation::DirectivesAreInValidLocationsError.new(
"Directives can't be applied to #{ast_parent.class.name}s",
nodes: ast_directive,
target: ast_parent.class.name
))
end
end

def assert_includes_location(directive_defn, directive_ast, required_location)
if !directive_defn.locations.include?(required_location)
location_name = LOCATION_MESSAGE_NAMES[required_location]
allowed_location_names = directive_defn.locations.map { |loc| LOCATION_MESSAGE_NAMES[loc] }
add_error("'@#{directive_defn.name}' can't be applied to #{location_name} (allowed: #{allowed_location_names.join(", ")})", directive_ast)
add_error(GraphQL::StaticValidation::DirectivesAreInValidLocationsError.new(
"'@#{directive_defn.name}' can't be applied to #{location_name} (allowed: #{allowed_location_names.join(", ")})",
nodes: directive_ast,
target: location_name,
name: directive_defn.name
))
end
end
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true
module GraphQL
module StaticValidation
class DirectivesAreInValidLocationsError < StaticValidation::Error
attr_reader :target_name
attr_reader :name

def initialize(message, path: nil, nodes: [], target:, name: nil)
super(message, path: path, nodes: nodes)
@target_name = target
@name = name
end

# A hash representation of this Message
def to_h
extensions = {
"code" => code,
"targetName" => target_name
}.tap { |h| h["name"] = name unless name.nil? }

super.merge({
"extensions" => extensions
})
end

private
def code
"directiveCannotBeApplied"
end
end
end
end
13 changes: 11 additions & 2 deletions lib/graphql/static_validation/rules/fields_are_defined_on_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,18 @@ def on_field(node, parent)

if field.nil?
if parent_type.kind.union?
add_error("Selections can't be made directly on unions (see selections on #{parent_type.name})", parent)
add_error(GraphQL::StaticValidation::FieldsHaveAppropriateSelectionsError.new(
"Selections can't be made directly on unions (see selections on #{parent_type.name})",
nodes: parent,
node_name: parent_type.name
))
else
add_error("Field '#{node.name}' doesn't exist on type '#{parent_type.name}'", node)
add_error(GraphQL::StaticValidation::FieldsAreDefinedOnTypeError.new(
"Field '#{node.name}' doesn't exist on type '#{parent_type.name}'",
nodes: node,
field: node.name,
type: parent_type.name
))
end
else
super
Expand Down
Loading