From 6b83a74b3c3df08a7e1360e03462164fa2d71532 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 1 Sep 2022 13:38:29 +0200 Subject: [PATCH] do not nullify the entire query if the root operation is not present (#1674) if the root operation was not returned by the subgraph (example: when there's an error), we should not nullify the entire data objet. Instead, it's the root operation field that should be null (unless it is not nullable) Co-authored-by: Martin Bonnin --- NEXT_CHANGELOG.md | 9 ++++ apollo-router/src/spec/query.rs | 90 +++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 7c3a769f92..7fd7c22e34 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -63,6 +63,15 @@ The 2.1.1 release of the query planner fixes an issue with the `@defer` directiv By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/1650 and https://github.com/apollographql/router/pull/1672 +### Do not nullify the entire query if the root operation is not present ([PR #1674](https://github.com/apollographql/router/issues/1674)) + +if a root field was not returned by the subgraph (example: when +there's an error), we should not nullify the entire data objet. Instead, +it's the root field that should be null (unless it is non +nullable). + +By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/1674 + ## 🛠 Maintenance ### Remove cache layer ([PR #1647](https://github.com/apollographql/router/pull/1647)) diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index f430b4a336..53e87f86a6 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -581,6 +581,8 @@ impl Query { } } else if field_type.is_non_null() { return Err(InvalidValue); + } else { + output.insert(field_name.clone(), Value::Null); } } Selection::InlineFragment { @@ -5574,4 +5576,92 @@ mod tests { }}, ); } + + #[test] + fn query_operation_nullification() { + assert_format_response!( + "type Query { + get: Thing + } + type Thing { + name: String + } + ", + "{ + get { + name + } + }", + json! {{}}, + None, + json! {{ + "get": null, + }}, + ); + + assert_format_response!( + "type Query { + get: Thing + } + type Thing { + name: String + } + ", + "query { + ...F + } + + fragment F on Query { + get { + name + } + } + ", + json! {{}}, + None, + json! {{ + "get": null, + }}, + ); + + assert_format_response!( + "type Query { + get: Thing + } + type Thing { + name: String + } + ", + "query { + ... on Query { + get { + name + } + } + }", + json! {{}}, + None, + json! {{ + "get": null, + }}, + ); + + assert_format_response!( + "type Query { + get: Thing! + } + type Thing { + name: String + } + ", + "{ + get { + name + } + }", + json! {{}}, + None, + Value::Null, + ); + } }