Skip to content

Commit

Permalink
Revert default value validation of input fields and arguments
Browse files Browse the repository at this point in the history
This turns out to break enough existing schemas
(Router rejects them as invalid where it previously accepted them)
that we consider it a breaking change.

See #928
  • Loading branch information
SimonSapin committed Nov 25, 2024
1 parent 973254d commit 5923c27
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 36 deletions.
2 changes: 1 addition & 1 deletion crates/apollo-compiler/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## Fixes
- **Validate against reserved names starting with `__` in schemas - [SimonSapin], [pull/923].**
- **Validate the default value of input fields and arguments - [SimonSapin], [pull/925].**
- **Fix duplicate diagnostic for variable with invalid default value - [SimonSapin], [pull/925].**

[SimonSapin]: https://github.com/SimonSapin
[pull/923]: https://github.com/apollographql/apollo-rs/issues/923
Expand Down
12 changes: 7 additions & 5 deletions crates/apollo-compiler/src/validation/input_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::collections::HashMap;
use crate::schema::validation::BuiltInScalars;
use crate::schema::InputObjectType;
use crate::validation::diagnostics::DiagnosticData;
use crate::validation::value::value_of_correct_type;
use crate::validation::CycleError;
use crate::validation::DiagnosticList;
use crate::validation::RecursionGuard;
Expand Down Expand Up @@ -209,10 +208,13 @@ pub(crate) fn validate_input_value_definitions(
},
);
}
if let Some(default) = &input_value.default_value {
let var_defs = &[];
value_of_correct_type(diagnostics, schema, &input_value.ty, default, var_defs);
}
// TODO: Validate default values in apollo-compiler 2.0
// https://github.com/apollographql/apollo-rs/issues/928
//
// if let Some(default) = &input_value.default_value {
// let var_defs = &[];
// value_of_correct_type(diagnostics, schema, &input_value.ty, default, var_defs);
// }
} else if is_built_in {
// `validate_schema()` will insert the missing definition
} else {
Expand Down
41 changes: 11 additions & 30 deletions crates/apollo-compiler/tests/validation/variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,13 +354,6 @@ fn variables_in_const_contexts() {
);
let errors = schema.validate().unwrap_err().errors;
let expected = expect_test::expect![[r#"
Error: variable `$x` is not defined
╭─[input.graphql:3:34]
3 │ arg: InputObj = {x: ["x"]} @dir2(arg: "x")
│ ─┬─
│ ╰─── not found in this scope
───╯
Error: variable `$x` is not defined
╭─[input.graphql:3:51]
Expand Down Expand Up @@ -403,13 +396,6 @@ fn variables_in_const_contexts() {
│ ─┬─
│ ╰─── not found in this scope
────╯
Error: variable `$x` is not defined
╭─[input.graphql:43:39]
43 │ arg2: InputObj = {x: ["x"]} @dir(arg: {x: ["x"]})
│ ─┬─
│ ╰─── not found in this scope
────╯
Error: variable `$x` is not defined
╭─[input.graphql:43:60]
Expand Down Expand Up @@ -438,13 +424,6 @@ fn variables_in_const_contexts() {
│ ─┬─
│ ╰─── not found in this scope
────╯
Error: variable `$x` is not defined
╭─[input.graphql:51:39]
51 │ arg2: InputObj = {x: ["x"]} @dir(arg: {x: ["x"]})
│ ─┬─
│ ╰─── not found in this scope
────╯
Error: variable `$x` is not defined
╭─[input.graphql:51:60]
Expand Down Expand Up @@ -515,13 +494,6 @@ fn variables_in_const_contexts() {
│ ─┬─
│ ╰─── not found in this scope
────╯
Error: variable `$x` is not defined
╭─[input.graphql:66:28]
66 │ x: [String] = ["x"] @dir2(arg: "x")
│ ─┬─
│ ╰─── not found in this scope
────╯
Error: variable `$x` is not defined
╭─[input.graphql:66:44]
Expand All @@ -538,10 +510,19 @@ fn variables_in_const_contexts() {
────╯
"#]];
expected.assert_eq(&errors.to_string());
let expected_schema_errors = 26;
let expected_schema_errors = 22;
assert_eq!(errors.len(), expected_schema_errors);

// Default values not validated yet: https://github.com/apollographql/apollo-rs/issues/928
// * @dir(arg:)
// * Query.field(arg2:)
// * Inter.field(arg2:)
// * InputObj.x
let input_default_values_not_yet_validated = 4;
assert_eq!(
input.matches("\"x\"").count(),
expected_schema_errors + expected_executable_errors
expected_schema_errors
+ expected_executable_errors
+ input_default_values_not_yet_validated
)
}

0 comments on commit 5923c27

Please sign in to comment.