Skip to content

Commit 9087fd8

Browse files
fix(router): allow null value for nullable scalar types while validating variables (#483)
Ref ROUTER-155 When `validate_runtime_value` takes `null` for a non-nullable scalar type, it was throwing like; `Failed to collect GraphQL variables: Expected a string for type 'String', got Null` --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 7caf00c commit 9087fd8

File tree

1 file changed

+57
-4
lines changed
  • lib/executor/src/variables

1 file changed

+57
-4
lines changed

lib/executor/src/variables/mod.rs

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,13 @@ fn validate_runtime_value(
6767
type_node: &TypeNode,
6868
schema_metadata: &SchemaMetadata,
6969
) -> Result<(), String> {
70+
if let ValueRef::Null = value {
71+
return if type_node.is_non_null() {
72+
Err("Value cannot be null for non-nullable type".to_string())
73+
} else {
74+
Ok(())
75+
};
76+
}
7077
match type_node {
7178
TypeNode::Named(name) => {
7279
if let Some(enum_values) = schema_metadata.enum_values.get(name) {
@@ -173,9 +180,7 @@ fn validate_runtime_value(
173180
}
174181
}
175182
TypeNode::NonNull(inner_type) => {
176-
if let ValueRef::Null = value {
177-
return Err("Value cannot be null for non-nullable type".to_string());
178-
}
183+
// The null check is now handled above, so we can just recurse.
179184
validate_runtime_value(value, inner_type, schema_metadata)?;
180185
}
181186
TypeNode::List(inner_type) => {
@@ -184,9 +189,57 @@ fn validate_runtime_value(
184189
validate_runtime_value(item.as_ref(), inner_type, schema_metadata)?;
185190
}
186191
} else {
187-
return Err(format!("Expected an array for list type, got {:?}", value));
192+
validate_runtime_value(value, inner_type, schema_metadata)?;
188193
}
189194
}
190195
}
191196
Ok(())
192197
}
198+
199+
#[cfg(test)]
200+
mod tests {
201+
#[test]
202+
fn allow_null_values_for_nullable_scalar_types() {
203+
let schema_metadata = crate::introspection::schema::SchemaMetadata::default();
204+
205+
let scalars = vec!["String", "Int", "Float", "Boolean", "ID"];
206+
for scalar in scalars {
207+
let type_node = crate::variables::TypeNode::Named(scalar.to_string());
208+
209+
let value = sonic_rs::ValueRef::Null;
210+
211+
let result = super::validate_runtime_value(value, &type_node, &schema_metadata);
212+
assert_eq!(result, Ok(()));
213+
}
214+
}
215+
#[test]
216+
fn allow_null_values_for_nullable_list_types() {
217+
let schema_metadata = crate::introspection::schema::SchemaMetadata::default();
218+
let type_node = crate::variables::TypeNode::List(Box::new(
219+
crate::variables::TypeNode::Named("String".to_string()),
220+
));
221+
let value = sonic_rs::ValueRef::Null;
222+
let result = super::validate_runtime_value(value, &type_node, &schema_metadata);
223+
assert_eq!(result, Ok(()));
224+
}
225+
#[test]
226+
fn allow_matching_non_list_values_for_list_types() {
227+
let schema_metadata = crate::introspection::schema::SchemaMetadata::default();
228+
let type_node = crate::variables::TypeNode::List(Box::new(
229+
crate::variables::TypeNode::Named("String".to_string()),
230+
));
231+
let value = sonic_rs::ValueRef::String("not a list");
232+
let result = super::validate_runtime_value(value, &type_node, &schema_metadata);
233+
assert_eq!(result, Ok(()));
234+
}
235+
#[test]
236+
fn disallow_non_matching_non_list_values_for_list_types() {
237+
let schema_metadata = crate::introspection::schema::SchemaMetadata::default();
238+
let type_node = crate::variables::TypeNode::List(Box::new(
239+
crate::variables::TypeNode::Named("String".to_string()),
240+
));
241+
let value = sonic_rs::ValueRef::Number(sonic_rs::Number::from(123));
242+
let result = super::validate_runtime_value(value, &type_node, &schema_metadata);
243+
assert!(result.is_err());
244+
}
245+
}

0 commit comments

Comments
 (0)