From e61c8c383a120e41205488040bd490af9cc7505d Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Wed, 30 Nov 2022 14:02:05 +0000 Subject: [PATCH 1/3] verify that deferred fragment acts as a boundary for nullability rules fixes: #2169 Add a new test to confirm this is the case. --- .../src/services/supergraph_service.rs | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index 4018f25698..da3c5c9840 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -609,6 +609,88 @@ mod tests { insta::assert_json_snapshot!(stream.next_response().await.unwrap()); } + #[tokio::test] + async fn deferred_fragment_bounds_nullability() { + let subgraphs = MockedSubgraphs([ + ("user", MockSubgraph::builder().with_json( + serde_json::json!{{"query":"{currentUser{activeOrganization{__typename id}}}"}}, + serde_json::json!{{"data": {"currentUser": { "activeOrganization": { "__typename": "Organization", "id": "0" } }}}} + ).build()), + ("orga", MockSubgraph::builder().with_json( + serde_json::json!{{ + "query":"query($representations:[_Any!]!){_entities(representations:$representations){...on Organization{suborga{__typename id}}}}", + "variables": { + "representations":[{"__typename": "Organization", "id":"0"}] + } + }}, + serde_json::json!{{ + "data": { + "_entities": [{ "suborga": [ + { "__typename": "Organization", "id": "1"}, + { "__typename": "Organization", "id": "2"}, + { "__typename": "Organization", "id": "3"}, + ] }] + }, + }} + ) + .with_json( + serde_json::json!{{ + "query":"query($representations:[_Any!]!){_entities(representations:$representations){...on Organization{name}}}", + "variables": { + "representations":[ + {"__typename": "Organization", "id":"1"}, + {"__typename": "Organization", "id":"2"}, + {"__typename": "Organization", "id":"3"} + + ] + } + }}, + serde_json::json!{{ + "data": { + "_entities": [ + { "__typename": "Organization", "id": "1"}, + { "__typename": "Organization", "id": "2", "name": "A"}, + { "__typename": "Organization", "id": "3"}, + ] + }, + "errors": [ + { + "message": "error orga 1", + "path": ["_entities", 0], + }, + { + "message": "error orga 3", + "path": ["_entities", 2], + } + ] + }} + ).build()) + ].into_iter().collect()); + + let service = TestHarness::builder() + .configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true } })) + .unwrap() + .schema(SCHEMA) + .extra_plugin(subgraphs) + .build() + .await + .unwrap(); + + let request = supergraph::Request::fake_builder() + .header("Accept", "multipart/mixed; deferSpec=20220824") + .query( + "query { currentUser { activeOrganization { id suborga { id ...@defer { nonNullId } } } } }", + ) + .build() + .unwrap(); + + let mut stream = service.oneshot(request).await.unwrap(); + + insta::assert_json_snapshot!(stream.next_response().await.unwrap()); + + insta::assert_json_snapshot!(stream.next_response().await.unwrap()); + } + #[tokio::test] async fn errors_on_incremental_responses() { let subgraphs = MockedSubgraphs([ From a630989fbaba2058b35b4281e32970c7a7912a9e Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Wed, 30 Nov 2022 14:04:41 +0000 Subject: [PATCH 2/3] add new snapshots to PR sigh... --- ...eferred_fragment_bounds_nullability-2.snap | 129 ++++++++++++++++++ ..._deferred_fragment_bounds_nullability.snap | 25 ++++ 2 files changed, 154 insertions(+) create mode 100644 apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__deferred_fragment_bounds_nullability-2.snap create mode 100644 apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__deferred_fragment_bounds_nullability.snap diff --git a/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__deferred_fragment_bounds_nullability-2.snap b/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__deferred_fragment_bounds_nullability-2.snap new file mode 100644 index 0000000000..acd2170b0b --- /dev/null +++ b/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__deferred_fragment_bounds_nullability-2.snap @@ -0,0 +1,129 @@ +--- +source: apollo-router/src/services/supergraph_service.rs +expression: stream.next_response().await.unwrap() +--- +{ + "hasNext": false, + "incremental": [ + { + "data": null, + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 0 + ], + "extensions": { + "valueCompletion": [ + { + "message": "Cannot return null for non-nullable field Organization.nonNullId", + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 0 + ] + }, + { + "message": "Cannot return null for non-nullable field Organization.nonNullId", + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 1 + ] + }, + { + "message": "Cannot return null for non-nullable field Organization.nonNullId", + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 2 + ] + } + ] + } + }, + { + "data": null, + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 1 + ], + "extensions": { + "valueCompletion": [ + { + "message": "Cannot return null for non-nullable field Organization.nonNullId", + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 0 + ] + }, + { + "message": "Cannot return null for non-nullable field Organization.nonNullId", + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 1 + ] + }, + { + "message": "Cannot return null for non-nullable field Organization.nonNullId", + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 2 + ] + } + ] + } + }, + { + "data": null, + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 2 + ], + "extensions": { + "valueCompletion": [ + { + "message": "Cannot return null for non-nullable field Organization.nonNullId", + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 0 + ] + }, + { + "message": "Cannot return null for non-nullable field Organization.nonNullId", + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 1 + ] + }, + { + "message": "Cannot return null for non-nullable field Organization.nonNullId", + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 2 + ] + } + ] + } + } + ] +} diff --git a/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__deferred_fragment_bounds_nullability.snap b/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__deferred_fragment_bounds_nullability.snap new file mode 100644 index 0000000000..7f3a7ac9be --- /dev/null +++ b/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__deferred_fragment_bounds_nullability.snap @@ -0,0 +1,25 @@ +--- +source: apollo-router/src/services/supergraph_service.rs +expression: stream.next_response().await.unwrap() +--- +{ + "data": { + "currentUser": { + "activeOrganization": { + "id": "0", + "suborga": [ + { + "id": "1" + }, + { + "id": "2" + }, + { + "id": "3" + } + ] + } + } + }, + "hasNext": true +} From 9a031dd94d9c4e3a5535f9774f5d33917faaefd1 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Wed, 30 Nov 2022 14:09:27 +0000 Subject: [PATCH 3/3] add a changelog --- NEXT_CHANGELOG.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 7494558799..42cd36ad59 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -26,7 +26,7 @@ By [@USERNAME](https://github.com/USERNAME) in https://github.com/apollographql/ # [x.x.x] (unreleased) - 2022-mm-dd ## ❗ BREAKING ❗ -### Router debug Docker images now run under the control of heaptrack ([Issue #2135](https://github.com/apollographql/router/pull/2142)) +### Router debug Docker images now run under the control of heaptrack ([Issue #2135](https://github.com/apollographql/router/issues/2135)) From the next release, our debug Docker image will invoke the router under the control of heaptrack. We are making this change to make it simple for users to investigate potential memory issues with the router. @@ -115,7 +115,7 @@ telemetry: headers: true ``` -### Provide multi-arch (amd64/arm64) Docker images for the Router ([Issue #1932](https://github.com/apollographql/router/pull/2138)) +### Provide multi-arch (amd64/arm64) Docker images for the Router ([Issue #1932](https://github.com/apollographql/router/issues/1932)) From the next release, our Docker images will be multi-arch. @@ -261,6 +261,12 @@ By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/p ## 🛠 Maintenance +### Verify that deferred fragment acts as a boundary for nullability rules ([Issue #2169](https://github.com/apollographql/router/issues/2169)) + +Add a test to ensure that deferred fragments act as a boundary for nullability rules. + +By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/2183 + ### Refactor APQ ([PR #2129](https://github.com/apollographql/router/pull/2129)) Remove duplicated code.