Skip to content

Commit ace6935

Browse files
zaksabeasttyranron
andauthored
Improve validation errors for input values (#811, #693)
Co-authored-by: Kai Ren <[email protected]>
1 parent 7c03f92 commit ace6935

File tree

8 files changed

+352
-221
lines changed

8 files changed

+352
-221
lines changed

juniper/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ All user visible changes to `juniper` crate will be documented in this file. Thi
7878
- Made `GraphQLRequest` fields public. ([#750])
7979
- Relaxed [object safety] requirement for `GraphQLValue` and `GraphQLValueAsync` traits. ([ba1ed85b])
8080
- Updated [GraphQL Playground] to 1.7.28 version. ([#1190])
81+
- Improve validation errors for input values. ([#811], [#693])
8182

8283
## Fixed
8384

@@ -94,8 +95,10 @@ All user visible changes to `juniper` crate will be documented in this file. Thi
9495
[#456]: /../../issues/456
9596
[#503]: /../../issues/503
9697
[#528]: /../../issues/528
98+
[#693]: /../../issues/693
9799
[#750]: /../../issues/750
98100
[#798]: /../../issues/798
101+
[#811]: /../../pull/811
99102
[#918]: /../../issues/918
100103
[#965]: /../../pull/965
101104
[#966]: /../../pull/966

juniper/src/executor_tests/enums.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ async fn does_not_accept_string_literals() {
100100
assert_eq!(
101101
error,
102102
ValidationError(vec![RuleError::new(
103-
r#"Invalid value for argument "color", expected type "Color!""#,
103+
r#"Invalid value for argument "color", reason: Invalid value ""RED"" for enum "Color""#,
104104
&[SourcePosition::new(18, 0, 18)],
105105
)])
106106
);

juniper/src/executor_tests/variables.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -916,7 +916,8 @@ async fn does_not_allow_missing_required_field() {
916916
assert_eq!(
917917
error,
918918
ValidationError(vec![RuleError::new(
919-
r#"Invalid value for argument "arg", expected type "ExampleInputObject!""#,
919+
"Invalid value for argument \"arg\", \
920+
reason: \"ExampleInputObject\" is missing fields: \"b\"",
920921
&[SourcePosition::new(20, 0, 20)],
921922
)]),
922923
);
@@ -940,7 +941,9 @@ async fn does_not_allow_null_in_required_field() {
940941
assert_eq!(
941942
error,
942943
ValidationError(vec![RuleError::new(
943-
r#"Invalid value for argument "arg", expected type "ExampleInputObject!""#,
944+
"Invalid value for argument \"arg\", \
945+
reason: Error on \"ExampleInputObject\" field \"b\": \
946+
\"null\" specified for not nullable type \"Int!\"",
944947
&[SourcePosition::new(20, 0, 20)],
945948
)]),
946949
);

juniper/src/types/utilities.rs

Lines changed: 121 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,126 @@
1+
use std::{collections::HashSet, iter::Iterator};
2+
13
use crate::{
24
ast::InputValue,
35
schema::{
4-
meta::{EnumMeta, InputObjectMeta, MetaType},
6+
meta::{Argument, EnumMeta, InputObjectMeta, MetaType},
57
model::{SchemaType, TypeType},
68
},
79
value::ScalarValue,
810
};
9-
use std::collections::HashSet;
1011

11-
pub fn is_valid_literal_value<S>(
12+
/// Common error messages used in validation and execution of GraphQL operations
13+
pub(crate) mod error {
14+
use std::fmt::Display;
15+
16+
pub(crate) fn non_null(arg_type: impl Display) -> String {
17+
format!("\"null\" specified for not nullable type \"{arg_type}\"")
18+
}
19+
20+
pub(crate) fn enum_value(arg_value: impl Display, arg_type: impl Display) -> String {
21+
format!("Invalid value \"{arg_value}\" for enum \"{arg_type}\"")
22+
}
23+
24+
pub(crate) fn type_value(arg_value: impl Display, arg_type: impl Display) -> String {
25+
format!("Invalid value \"{arg_value}\" for type \"{arg_type}\"")
26+
}
27+
28+
pub(crate) fn parser(arg_type: impl Display, msg: impl Display) -> String {
29+
format!("Parser error for \"{arg_type}\": {msg}")
30+
}
31+
32+
pub(crate) fn not_input_object(arg_type: impl Display) -> String {
33+
format!("\"{arg_type}\" is not an input object")
34+
}
35+
36+
pub(crate) fn field(
37+
arg_type: impl Display,
38+
field_name: impl Display,
39+
error_message: impl Display,
40+
) -> String {
41+
format!("Error on \"{arg_type}\" field \"{field_name}\": {error_message}")
42+
}
43+
44+
pub(crate) fn missing_fields(arg_type: impl Display, missing_fields: impl Display) -> String {
45+
format!("\"{arg_type}\" is missing fields: {missing_fields}")
46+
}
47+
48+
pub(crate) fn unknown_field(arg_type: impl Display, field_name: impl Display) -> String {
49+
format!("Field \"{field_name}\" does not exist on type \"{arg_type}\"")
50+
}
51+
52+
pub(crate) fn invalid_list_length(
53+
arg_value: impl Display,
54+
actual: usize,
55+
expected: usize,
56+
) -> String {
57+
format!("Expected list of length {expected}, but \"{arg_value}\" has length {actual}")
58+
}
59+
}
60+
61+
/// Validates the specified field of a GraphQL object and returns an error message if the field is
62+
/// invalid.
63+
fn validate_object_field<S>(
64+
schema: &SchemaType<S>,
65+
object_type: &TypeType<S>,
66+
object_fields: &[Argument<S>],
67+
field_value: &InputValue<S>,
68+
field_key: &str,
69+
) -> Option<String>
70+
where
71+
S: ScalarValue,
72+
{
73+
let field_type = object_fields
74+
.iter()
75+
.filter(|f| f.name == field_key)
76+
.map(|f| schema.make_type(&f.arg_type))
77+
.next();
78+
79+
if let Some(field_arg_type) = field_type {
80+
let error_message = validate_literal_value(schema, &field_arg_type, field_value);
81+
82+
error_message.map(|m| error::field(object_type, field_key, m))
83+
} else {
84+
Some(error::unknown_field(object_type, field_key))
85+
}
86+
}
87+
88+
/// Validates the specified GraphQL literal and returns an error message if the it's invalid.
89+
pub fn validate_literal_value<S>(
1290
schema: &SchemaType<S>,
1391
arg_type: &TypeType<S>,
1492
arg_value: &InputValue<S>,
15-
) -> bool
93+
) -> Option<String>
1694
where
1795
S: ScalarValue,
1896
{
1997
match *arg_type {
2098
TypeType::NonNull(ref inner) => {
2199
if arg_value.is_null() {
22-
false
100+
Some(error::non_null(arg_type))
23101
} else {
24-
is_valid_literal_value(schema, inner, arg_value)
102+
validate_literal_value(schema, inner, arg_value)
25103
}
26104
}
27105
TypeType::List(ref inner, expected_size) => match *arg_value {
28-
InputValue::Null | InputValue::Variable(_) => true,
106+
InputValue::Null | InputValue::Variable(_) => None,
29107
InputValue::List(ref items) => {
30108
if let Some(expected) = expected_size {
31109
if items.len() != expected {
32-
return false;
110+
return Some(error::invalid_list_length(arg_value, items.len(), expected));
33111
}
34112
}
35113
items
36114
.iter()
37-
.all(|i| is_valid_literal_value(schema, inner, &i.item))
115+
.find_map(|i| validate_literal_value(schema, inner, &i.item))
38116
}
39117
ref v => {
40118
if let Some(expected) = expected_size {
41119
if expected != 1 {
42-
return false;
120+
return Some(error::invalid_list_length(arg_value, 1, expected));
43121
}
44122
}
45-
is_valid_literal_value(schema, inner, v)
123+
validate_literal_value(schema, inner, v)
46124
}
47125
},
48126
TypeType::Concrete(t) => {
@@ -51,19 +129,23 @@ where
51129
if let (&InputValue::Scalar(_), Some(&MetaType::Enum(EnumMeta { .. }))) =
52130
(arg_value, arg_type.to_concrete())
53131
{
54-
return false;
132+
return Some(error::enum_value(arg_value, arg_type));
55133
}
56134

57135
match *arg_value {
58-
InputValue::Null | InputValue::Variable(_) => true,
136+
InputValue::Null | InputValue::Variable(_) => None,
59137
ref v @ InputValue::Scalar(_) | ref v @ InputValue::Enum(_) => {
60138
if let Some(parse_fn) = t.input_value_parse_fn() {
61-
parse_fn(v).is_ok()
139+
if parse_fn(v).is_ok() {
140+
None
141+
} else {
142+
Some(error::type_value(arg_value, arg_type))
143+
}
62144
} else {
63-
false
145+
Some(error::parser(arg_type, "no parser present"))
64146
}
65147
}
66-
InputValue::List(_) => false,
148+
InputValue::List(_) => Some("Input lists are not literals".to_owned()),
67149
InputValue::Object(ref obj) => {
68150
if let MetaType::InputObject(InputObjectMeta {
69151
ref input_fields, ..
@@ -77,23 +159,33 @@ where
77159
})
78160
.collect::<HashSet<_>>();
79161

80-
let all_types_ok = obj.iter().all(|(key, value)| {
162+
let error_message = obj.iter().find_map(|(key, value)| {
81163
remaining_required_fields.remove(&key.item);
82-
if let Some(ref arg_type) = input_fields
83-
.iter()
84-
.filter(|f| f.name == key.item)
85-
.map(|f| schema.make_type(&f.arg_type))
86-
.next()
87-
{
88-
is_valid_literal_value(schema, arg_type, &value.item)
89-
} else {
90-
false
91-
}
164+
validate_object_field(
165+
schema,
166+
arg_type,
167+
input_fields,
168+
&value.item,
169+
&key.item,
170+
)
92171
});
93172

94-
all_types_ok && remaining_required_fields.is_empty()
173+
if error_message.is_some() {
174+
return error_message;
175+
}
176+
177+
if remaining_required_fields.is_empty() {
178+
None
179+
} else {
180+
let missing_fields = remaining_required_fields
181+
.into_iter()
182+
.map(|s| format!("\"{}\"", &**s))
183+
.collect::<Vec<_>>()
184+
.join(", ");
185+
Some(error::missing_fields(arg_type, missing_fields))
186+
}
95187
} else {
96-
false
188+
Some(error::not_input_object(arg_type))
97189
}
98190
}
99191
}

0 commit comments

Comments
 (0)