diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 97addb1bea..3f0c3375cd 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -120,6 +120,31 @@ in a previous payload. By [@Geal](https://github.com/geal) in https://github.com/apollographql/router/pull/2184 +### Return root `__typename` when parts of a query with deferred fragment ([Issue #1677](https://github.com/apollographql/router/issues/1677)) + +With this query: + +```graphql +{ + __typename + fast + ...deferedFragment @defer +} + +fragment deferedFragment on Query { + slow +} +``` + +You will received first response chunk: + +```json +{"data":{"__typename": "Query", "fast":0},"hasNext":true} +``` + +By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/2188 + + ### wait for opentelemetry tracer provider to shutdown ([PR #2191](https://github.com/apollographql/router/pull/2191)) When we drop Telemetry we spawn a thread to perform the global opentelemetry trace provider shutdown. The documentation of this function indicates that "This will invoke the shutdown method on all span processors. span processors should export remaining spans before return". We should give that process some time to complete (5 seconds currently) before returning from the `drop`. This will provide more opportunity for spans to be exported. diff --git a/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__root_typename_with_defer-2.snap b/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__root_typename_with_defer-2.snap new file mode 100644 index 0000000000..5862863f46 --- /dev/null +++ b/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__root_typename_with_defer-2.snap @@ -0,0 +1,42 @@ +--- +source: apollo-router/src/services/supergraph_service.rs +expression: stream.next_response().await.unwrap() +--- +{ + "hasNext": false, + "incremental": [ + { + "data": { + "name": null + }, + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 0 + ] + }, + { + "data": { + "name": "A" + }, + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 1 + ] + }, + { + "data": { + "name": null + }, + "path": [ + "currentUser", + "activeOrganization", + "suborga", + 2 + ] + } + ] +} diff --git a/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__root_typename_with_defer.snap b/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__root_typename_with_defer.snap new file mode 100644 index 0000000000..3640cf4d26 --- /dev/null +++ b/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__root_typename_with_defer.snap @@ -0,0 +1,26 @@ +--- +source: apollo-router/src/services/supergraph_service.rs +expression: res +--- +{ + "data": { + "__typename": "Query", + "currentUser": { + "activeOrganization": { + "id": "0", + "suborga": [ + { + "id": "1" + }, + { + "id": "2" + }, + { + "id": "3" + } + ] + } + } + }, + "hasNext": true +} diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index d73512d654..9c816ed9cd 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -885,6 +885,140 @@ mod tests { insta::assert_json_snapshot!(stream.next_response().await.unwrap()); } + #[tokio::test] + async fn root_typename_with_defer() { + 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"}, + ] + } + }} + ).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 { __typename currentUser { activeOrganization { id suborga { id ...@defer { name } } } } }", + ) + .build() + .unwrap(); + + let mut stream = service.oneshot(request).await.unwrap(); + let res = stream.next_response().await.unwrap(); + assert_eq!( + res.data.as_ref().unwrap().get("__typename"), + Some(&serde_json_bytes::Value::String("Query".into())) + ); + insta::assert_json_snapshot!(res); + + insta::assert_json_snapshot!(stream.next_response().await.unwrap()); + } + + #[tokio::test] + async fn root_typename_with_defer_in_defer() { + 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 name}}}}", + "variables": { + "representations":[{"__typename": "Organization", "id":"0"}] + } + }}, + serde_json::json!{{ + "data": { + "_entities": [{ "suborga": [ + { "__typename": "Organization", "id": "1"}, + { "__typename": "Organization", "id": "2", "name": "A"}, + { "__typename": "Organization", "id": "3"}, + ] }] + }, + }} + ).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 { ...@defer { __typename currentUser { activeOrganization { id suborga { id name } } } } }", + ) + .build() + .unwrap(); + + let mut stream = service.oneshot(request).await.unwrap(); + let _res = stream.next_response().await.unwrap(); + let res = stream.next_response().await.unwrap(); + assert_eq!( + res.incremental + .get(0) + .unwrap() + .data + .as_ref() + .unwrap() + .get("__typename"), + Some(&serde_json_bytes::Value::String("Query".into())) + ); + } + #[tokio::test] async fn query_reconstruction() { let schema = r#"schema diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index f1004a981d..3d08e23ec0 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -106,7 +106,7 @@ impl Query { ) -> Vec { let data = std::mem::take(&mut response.data); if let Some(Value::Object(mut input)) = data { - let operation = self.operation(operation_name); + let original_operation = self.operation(operation_name); if is_deferred { if let Some(subselection) = &response.subselection { // Get subselection from hashmap @@ -123,6 +123,19 @@ impl Query { errors: Vec::new(), nullified: Vec::new(), }; + // Detect if root __typename is asked in the original query (the qp doesn't put root __typename in subselections) + // cf https://github.com/apollographql/router/issues/1677 + let operation_kind_if_root_typename = + original_operation.and_then(|op| { + op.selection_set + .iter() + .any(|f| f.is_typename_field()) + .then(|| *op.kind()) + }); + if let Some(operation_kind) = operation_kind_if_root_typename { + output.insert(TYPENAME, operation_kind.as_str().into()); + } + response.data = Some( match self.apply_root_selection_set( operation, @@ -151,7 +164,7 @@ impl Query { response.data = Some(Value::Object(Object::default())); return vec![]; } - } else if let Some(operation) = operation { + } else if let Some(operation) = original_operation { let mut output = Object::default(); let all_variables = if operation.variables.is_empty() { @@ -211,9 +224,8 @@ impl Query { schema: &Schema, configuration: &Configuration, ) -> Result { - let string = query.into(); - - let parser = apollo_parser::Parser::new(string.as_str()) + let query = query.into(); + let parser = apollo_parser::Parser::new(query.as_str()) .recursion_limit(configuration.server.experimental_parser_recursion_limit); let tree = parser.parse(); @@ -248,7 +260,7 @@ impl Query { .collect::, SpecError>>()?; Ok(Query { - string, + string: query, fragments, operations, subselections: HashMap::new(), @@ -1099,7 +1111,7 @@ impl Operation { && self .selection_set .get(0) - .map(|s| matches!(s, Selection::Field {name, ..} if name.as_str() == TYPENAME)) + .map(|s| s.is_typename_field()) .unwrap_or_default() } diff --git a/apollo-router/src/spec/selection.rs b/apollo-router/src/spec/selection.rs index 07282d853d..ddc57ceba0 100644 --- a/apollo-router/src/spec/selection.rs +++ b/apollo-router/src/spec/selection.rs @@ -317,6 +317,10 @@ impl Selection { Ok(selection) } + pub(crate) fn is_typename_field(&self) -> bool { + matches!(self, Selection::Field {name, ..} if name.as_str() == TYPENAME) + } + pub(crate) fn contains_error_path(&self, path: &[PathElement], fragments: &Fragments) -> bool { match (path.get(0), self) { (None, _) => true,