Skip to content
6 changes: 4 additions & 2 deletions crates/apollo-compiler/src/resolvers/input_coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,10 @@ fn coerce_variable_value(
}
}
"Float" => {
// https://spec.graphql.org/October2021/#sec-Float.Input-Coercion
if value.is_f64() {
// https://spec.graphql.org/September2025/#sec-Float.Input-Coercion
// Accept any JSON number (`int` or `float`) that coerces to a finite f64,
// rejecting special values (NaN, +∞, -∞) as required by the GraphQL spec.
if value.as_f64().is_some_and(f64::is_finite) {
return Ok(value.clone());
}
}
Expand Down
50 changes: 50 additions & 0 deletions crates/apollo-compiler/tests/input_coercion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@

#[test]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test won't run as is. It needs to be added to apollo-compiler/tests/validation/main.rs. Once added, you will notice rust compiler won't compile this file, and you will need to declare Schema, ExecutableDocument and coerce_variable_values.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve moved input_coercion.rs to apollo-compiler/tests/validation directory and added its declaration to apollo-compiler/tests/validation/mod.rs with c5f4bac. I believe this is what you were referring to?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, my bad, I typed out the path wrong. Where you had it was good - input coercion is not a validation rule. It needed to be added to apolo-compiler/tests/main.rs as mod input_coercion (and I wrote apolo-compiler/tests/validation/main.rs, leading to confusion).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for clarification @lrlna 🙌. You are right on this. I moved it to apollo-compiler/tests/ and correctly adjusted apollo-compiler/tests/main.rs in a4c357e like you wrote.

fn test_graphql_float_variable_coercion() {
// Small schema with a Float in the input object
let sdl = r#"
type Car { id: ID! kilometers: Float! }
input CarInput { kilometers: Float! }
type Query { getCarById(id: ID!): Car }
type Mutation { insertACar(car: CarInput!): Car!
}
"#;

let parsed_schema = Schema::parse_and_validate(sdl, "sdl").unwrap();

let executable_mutation = ExecutableDocument::parse_and_validate(
&parsed_schema,
"mutation MyCarInsertMutation($car: CarInput!){ insertACar(car:$car) { id kilometers } }",
"MyCarInsertMutation",
)
.unwrap();

let operation = executable_mutation
.operations
.get(Some("MyCarInsertMutation"))
.unwrap();

let kilometers_value = 3000;

// Provide an integer for a Float field
let input_variables = serde_json_bytes::json!({ "car": { "kilometers": kilometers_value } });
let map = match input_variables {
serde_json_bytes::Value::Object(m) => m,
_ => unreachable!(),
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could I ask you to add another test to this file that shows how coercion would fail?

Since we have changed the coercion code to now be value.as_f64().is_some_and(f64::is_finite), there is a chance for a None to be returned from as_f64. You might have to have kilometers_value be defined as a string, otherwise the compiler won't let this compile. So potentially two variations of the test like this:

// Infinity; should fail coercion
let kilometers_value = "1e1000";

and another one:

// i128 MAX; should also fail coercion, since it's > i64 MAX
let kilometers_value = "170141183460469231731687303715884105727";

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch, thank you! It seems that the coercion changes I made in resolvers/input_coercion.rs didn’t fully cover the case involving values beyond i128::MAX. The extended unit tests helped me spotting this edge case.

I’ve fixed the issue in 23d67b8 and also refactored tests/input_coercion.rs to reduce code duplication and improve readability.


// Coerce and validate.
let coerced = coerce_variable_values(&parsed_schema, operation, &map).unwrap();
let vars_for_exec = coerced.into_inner();

// ---- Assertions ----
let car = vars_for_exec
.get("car")
.and_then(|value| value.as_object())
.expect("coerced `car` object");
assert_eq!(
car.get("kilometers").unwrap(),
kilometers_value,
"kilometers should be present and a valid amount."
);
}