From 8b8c8b9618e27656662ae96545f02664d3bffca8 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 15 Nov 2022 18:47:25 +0100 Subject: [PATCH 01/22] split queries between primary and deferred parts --- Cargo.lock | 1 + apollo-router/Cargo.toml | 1 + .../src/services/supergraph_service.rs | 118 ++++++++++ apollo-router/src/spec/query.rs | 215 ++++++++++++++++++ 4 files changed, 335 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 3fc21f3094..9f585fcbf5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -147,6 +147,7 @@ dependencies = [ "access-json", "ansi_term", "anyhow", + "apollo-encoder", "apollo-parser", "askama", "async-compression", diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml index 153e812dc4..99b876bc3a 100644 --- a/apollo-router/Cargo.toml +++ b/apollo-router/Cargo.toml @@ -199,6 +199,7 @@ urlencoding = "2.1.2" uuid = { version = "1.2.1", features = ["serde", "v4"] } yaml-rust = "0.4.5" askama = "0.11.1" +apollo-encoder = "0.3.4" [target.'cfg(macos)'.dependencies] uname = "0.1.1" diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index 0a4ee2e752..d2fac823e7 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -673,4 +673,122 @@ mod tests { insta::assert_json_snapshot!(stream.next_response().await.unwrap()); } + + #[tokio::test] + async fn query_reconstruction2() { + let schema = r#"schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.2", for: EXECUTION) + @link(url: "https://specs.apollo.dev/tag/v0.2") + @link(url: "https://specs.apollo.dev/inaccessible/v0.2") + { + query: Query + } + + directive @inaccessible on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION + directive @join__field(graph: join__Graph!, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + directive @join__graph(name: String!, url: String!) on ENUM_VALUE + directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE + directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + directive @tag(name: String!) repeatable on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION + + scalar join__FieldSet + + enum join__Graph { + USER @join__graph(name: "user", url: "http://localhost:4000/graphql") + } + + scalar link__Import + + enum link__Purpose { + SECURITY + EXECUTION + } + + type Query + @join__type(graph: USER) + { + me: Identity @join__field(graph: USER) + } + + interface Identity + @join__type(graph: USER) + { + id: ID! + name: String! + } + + type User implements Identity + @join__implements(graph: USER, interface: "Identity") + @join__type(graph: USER, key: "id") + { + fullName: String! @join__field(graph: USER) + id: ID! + memberships: [UserMembership!]! @join__field(graph: USER) + name: String! @join__field(graph: USER) + } + + type UserMembership + @join__type(graph: USER) + @tag(name: "platform-api") + { + """The organization that the user belongs to.""" + account: Account! + """The user's permission level within the organization.""" + permission: UserPermission! + } + + enum UserPermission + @join__type(graph: USER) + { + USER + ADMIN + } + + type Account + @join__type(graph: USER, key: "id") + { + id: ID! @join__field(graph: USER) + name: String! @join__field(graph: USER) + } +"#; + + let service = TestHarness::builder() + .configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true } })) + .unwrap() + .schema(schema) + .build() + .await + .unwrap(); + + let request = supergraph::Request::fake_builder() + .header("Accept", "multipart/mixed; deferSpec=20220824") + .query( + r#"query { + me { + ... on User { + id + fullName + memberships { + permission + account { + ... on Account @defer { + name + } + } + } + } + } + }"#, + ) + .build() + .unwrap(); + + let mut stream = service.oneshot(request).await.unwrap(); + + insta::assert_json_snapshot!(stream.next_response().await.unwrap()); + + panic!() + } } diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index d21df6a82f..d4bffd3749 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -8,6 +8,7 @@ use std::collections::HashSet; use apollo_parser::ast; use apollo_parser::ast::AstNode; +use apollo_parser::ast::DirectiveLocation; use derivative::Derivative; use graphql::Error; use serde::de::Visitor; @@ -938,6 +939,7 @@ impl Operation { #[allow(clippy::mutable_key_type)] // Spec: https://spec.graphql.org/draft/#sec-Language.Operations fn from_ast(operation: ast::OperationDefinition, schema: &Schema) -> Result { + extract_subqueries(&operation, schema); let name = operation.name().map(|x| x.text().to_string()); let kind = operation @@ -1043,6 +1045,219 @@ impl Operation { } } +fn extract_subqueries(operation: &ast::OperationDefinition, schema: &Schema) { + let mut v = Vec::new(); + + //let op: apollo_encoder::OperationDefinition = operation.clone().try_into().unwrap(); + extract_operation(&operation, schema, &mut v); + println!("subqueries: {:#?}", v); +} + +fn extract_operation(operation: &ast::OperationDefinition, schema: &Schema, v: &mut Vec) { + let name: Option = operation.name().map(|n| n.text().to_string().into()); + let op_type: apollo_encoder::OperationType = operation + .operation_type() + .and_then(|op| op.try_into().ok()) + .unwrap_or(apollo_encoder::OperationType::Query); + + if let Some(selection_set) = operation.selection_set() { + for selection_set in extract_selection_set(&selection_set, schema) { + let mut document = apollo_encoder::Document::new(); + + let mut operation = apollo_encoder::OperationDefinition::new( + apollo_encoder::OperationType::Query, + selection_set, + ); + operation.name(name.clone()); + //FIXME: variables, directives + + document.operation(operation); + + println!("EXTRACTED: {}", document); + } + } +} + +fn extract_selection_set( + selection_set: &ast::SelectionSet, + schema: &Schema, +) -> impl Iterator { + let mut primary_selection_set = Vec::new(); + let mut deferred_selection_set = Vec::new(); + + for selection in selection_set.selections() { + match selection { + ast::Selection::Field(f) => { + primary_selection_set.push(extract_selection(&ast::Selection::Field(f), schema)) + } + ast::Selection::FragmentSpread(f) => { + if f.directives() + .map(|d| { + d.directives().any(|dir| { + dir.name() + .map(|n| n.text().as_str() == "defer") + .unwrap_or(false) + }) + }) + .unwrap_or(false) + { + deferred_selection_set.push(extract_selection( + &ast::Selection::FragmentSpread(f), + schema, + )); + } else { + primary_selection_set.push(extract_selection( + &ast::Selection::FragmentSpread(f), + schema, + )); + } + } + ast::Selection::InlineFragment(f) => { + if f.directives() + .map(|d| { + d.directives().any(|dir| { + dir.name() + .map(|n| n.text().as_str() == "defer") + .unwrap_or(false) + }) + }) + .unwrap_or(false) + { + deferred_selection_set.push(extract_selection( + &ast::Selection::InlineFragment(f), + schema, + )); + } else { + primary_selection_set.push(extract_selection( + &ast::Selection::InlineFragment(f), + schema, + )); + } + } + } + } + + let mut primary = apollo_encoder::SelectionSet::new(); + + for selection_it in primary_selection_set.iter_mut() { + if let Some(selection) = selection_it.next().and_then(|s| s.try_into().ok()) { + primary.selection(selection); + } + } + + std::iter::once(primary).chain( + primary_selection_set + .into_iter() + .chain(deferred_selection_set.into_iter()) + .flatten() + .filter_map(|s| s.try_into().ok()) + .map(|s| { + let mut set = apollo_encoder::SelectionSet::new(); + set.selection(s); + set + }), + ) +} + +fn extract_selection( + selection: &ast::Selection, + //operation_type: ast::OperationType, + //operation_name: Option<&str>, + schema: &Schema, +) -> impl Iterator { + match selection { + ast::Selection::Field(field) => { + let mut name = field.name().map(|n| n.text().to_string()).unwrap(); + let mut f = apollo_encoder::Field::new(name); + for argument in field + .arguments() + .into_iter() + .map(|arg| arg.arguments()) + .flatten() + .filter_map(|argument| argument.try_into().ok()) + { + f.argument(argument); + } + for directive in field + .directives() + .into_iter() + .map(|dir| dir.directives()) + .flatten() + .filter_map(|directive| directive.try_into().ok()) + { + f.directive(directive); + } + + match field.selection_set().as_ref() { + None => Box::new(std::iter::once(apollo_encoder::Selection::Field(f))) + as Box>, + Some(selection_set) => Box::new(extract_selection_set(selection_set, schema).map( + move |selection_set| { + let mut new_field = f.clone(); + new_field.selection_set(Some(selection_set)); + apollo_encoder::Selection::Field(new_field) + }, + )) + as Box>, + } + } + ast::Selection::FragmentSpread(fragment) => { + let mut name = fragment + .fragment_name() + .and_then(|f| f.name()) + .map(|n| n.text().to_string()) + .unwrap(); + + let mut f = apollo_encoder::FragmentSpread::new(name); + for directive in fragment + .directives() + .into_iter() + .map(|dir| dir.directives()) + .flatten() + .filter_map(|directive| directive.try_into().ok()) + { + f.directive(directive); + } + + Box::new(std::iter::once(apollo_encoder::Selection::FragmentSpread( + f, + ))) as Box> + + /*extract_selection_set(fragment.selection_set().as_ref().unwrap(), schema).map( + move |selection_set| { + let mut new_fragment = f.clone(); + new_fragment.selection_set(Some(selection_set)); + apollo_encoder::Selection::FragmentSpread(new_fragment) + }, + )*/ + } + ast::Selection::InlineFragment(fragment) => { + let type_condition = fragment.type_condition().and_then(|ty| ty.try_into().ok()); + + let directives = fragment + .directives() + .into_iter() + .map(|dir| dir.directives()) + .flatten() + .filter_map(|directive| directive.try_into().ok()) + .collect::>(); + + Box::new( + extract_selection_set(fragment.selection_set().as_ref().unwrap(), schema).map( + move |selection_set| { + let mut f = apollo_encoder::InlineFragment::new(selection_set); + f.type_condition(type_condition.clone()); + for directive in directives.iter() { + f.directive(directive.clone()); + } + apollo_encoder::Selection::InlineFragment(f) + }, + ), + ) as Box> + } + } +} + impl From for OperationKind { // Spec: https://spec.graphql.org/draft/#OperationType fn from(operation_type: ast::OperationType) -> Self { From 994dac2ce3a0ee321172d683adad8041804b541a Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 15 Nov 2022 19:51:56 +0100 Subject: [PATCH 02/22] handle named fragments --- apollo-router/src/spec/query.rs | 151 +++++++++++++++++++++++--------- 1 file changed, 110 insertions(+), 41 deletions(-) diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index d4bffd3749..ec5bfe6268 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -226,6 +226,17 @@ impl Query { let document = tree.document(); let fragments = Fragments::from_ast(&document, schema)?; + let ast_fragments = document + .definitions() + .filter_map(|def| match def { + ast::Definition::FragmentDefinition(f) => f + .fragment_name() + .and_then(|f| f.name()) + .map(|n| (n.text().to_string(), f)), + _ => None, + }) + .collect::>(); + let operations: Vec = document .definitions() .filter_map(|definition| { @@ -235,7 +246,7 @@ impl Query { None } }) - .map(|operation| Operation::from_ast(operation, schema)) + .map(|operation| Operation::from_ast(operation, &ast_fragments, schema)) .collect::, SpecError>>()?; Ok(Query { @@ -938,8 +949,12 @@ impl Operation { // ref: https://rust-lang.github.io/rust-clippy/master/index.html#mutable_key_type #[allow(clippy::mutable_key_type)] // Spec: https://spec.graphql.org/draft/#sec-Language.Operations - fn from_ast(operation: ast::OperationDefinition, schema: &Schema) -> Result { - extract_subqueries(&operation, schema); + fn from_ast( + operation: ast::OperationDefinition, + fragments: &HashMap, + schema: &Schema, + ) -> Result { + extract_subqueries(&operation, fragments, schema); let name = operation.name().map(|x| x.text().to_string()); let kind = operation @@ -1045,15 +1060,24 @@ impl Operation { } } -fn extract_subqueries(operation: &ast::OperationDefinition, schema: &Schema) { +fn extract_subqueries( + operation: &ast::OperationDefinition, + fragments: &HashMap, + schema: &Schema, +) { let mut v = Vec::new(); //let op: apollo_encoder::OperationDefinition = operation.clone().try_into().unwrap(); - extract_operation(&operation, schema, &mut v); + extract_operation(&operation, fragments, schema, &mut v); println!("subqueries: {:#?}", v); } -fn extract_operation(operation: &ast::OperationDefinition, schema: &Schema, v: &mut Vec) { +fn extract_operation( + operation: &ast::OperationDefinition, + fragments: &HashMap, + schema: &Schema, + v: &mut Vec, +) { let name: Option = operation.name().map(|n| n.text().to_string().into()); let op_type: apollo_encoder::OperationType = operation .operation_type() @@ -1061,11 +1085,22 @@ fn extract_operation(operation: &ast::OperationDefinition, schema: &Schema, v: & .unwrap_or(apollo_encoder::OperationType::Query); if let Some(selection_set) = operation.selection_set() { - for selection_set in extract_selection_set(&selection_set, schema) { + for selection_set in extract_selection_set(&selection_set, fragments, schema) { let mut document = apollo_encoder::Document::new(); let mut operation = apollo_encoder::OperationDefinition::new( - apollo_encoder::OperationType::Query, + //apollo_encoder::OperationType is not Clone yet + match &op_type { + apollo_encoder::OperationType::Query => apollo_encoder::OperationType::Query, + apollo_encoder::OperationType::Mutation => { + apollo_encoder::OperationType::Mutation + } + apollo_encoder::OperationType::Subscription => { + apollo_encoder::OperationType::Subscription + } + }, + //op_type.clone(), + //apollo_encoder::OperationType::Query, selection_set, ); operation.name(name.clone()); @@ -1080,6 +1115,7 @@ fn extract_operation(operation: &ast::OperationDefinition, schema: &Schema, v: & fn extract_selection_set( selection_set: &ast::SelectionSet, + fragments: &HashMap, schema: &Schema, ) -> impl Iterator { let mut primary_selection_set = Vec::new(); @@ -1087,9 +1123,11 @@ fn extract_selection_set( for selection in selection_set.selections() { match selection { - ast::Selection::Field(f) => { - primary_selection_set.push(extract_selection(&ast::Selection::Field(f), schema)) - } + ast::Selection::Field(f) => primary_selection_set.push(extract_selection( + &ast::Selection::Field(f), + fragments, + schema, + )), ast::Selection::FragmentSpread(f) => { if f.directives() .map(|d| { @@ -1103,11 +1141,13 @@ fn extract_selection_set( { deferred_selection_set.push(extract_selection( &ast::Selection::FragmentSpread(f), + fragments, schema, )); } else { primary_selection_set.push(extract_selection( &ast::Selection::FragmentSpread(f), + fragments, schema, )); } @@ -1125,11 +1165,13 @@ fn extract_selection_set( { deferred_selection_set.push(extract_selection( &ast::Selection::InlineFragment(f), + fragments, schema, )); } else { primary_selection_set.push(extract_selection( &ast::Selection::InlineFragment(f), + fragments, schema, )); } @@ -1161,8 +1203,7 @@ fn extract_selection_set( fn extract_selection( selection: &ast::Selection, - //operation_type: ast::OperationType, - //operation_name: Option<&str>, + fragments: &HashMap, schema: &Schema, ) -> impl Iterator { match selection { @@ -1191,14 +1232,15 @@ fn extract_selection( match field.selection_set().as_ref() { None => Box::new(std::iter::once(apollo_encoder::Selection::Field(f))) as Box>, - Some(selection_set) => Box::new(extract_selection_set(selection_set, schema).map( - move |selection_set| { - let mut new_field = f.clone(); - new_field.selection_set(Some(selection_set)); - apollo_encoder::Selection::Field(new_field) - }, - )) - as Box>, + Some(selection_set) => { + Box::new(extract_selection_set(selection_set, fragments, schema).map( + move |selection_set| { + let mut new_field = f.clone(); + new_field.selection_set(Some(selection_set)); + apollo_encoder::Selection::Field(new_field) + }, + )) as Box> + } } } ast::Selection::FragmentSpread(fragment) => { @@ -1208,7 +1250,39 @@ fn extract_selection( .map(|n| n.text().to_string()) .unwrap(); - let mut f = apollo_encoder::FragmentSpread::new(name); + let named = fragments.get(&name).unwrap(); + + let type_condition = named.type_condition().and_then(|ty| ty.try_into().ok()); + + let mut directives: Vec = fragment + .directives() + .into_iter() + .map(|dir| dir.directives()) + .flatten() + .filter_map(|directive| directive.try_into().ok()) + .collect::>(); + + directives.extend( + named + .directives() + .into_iter() + .map(|dir| dir.directives()) + .flatten() + .filter_map(|directive| directive.try_into().ok()), + ); + + Box::new( + extract_selection_set(named.selection_set().as_ref().unwrap(), fragments, schema) + .map(move |selection_set| { + let mut f = apollo_encoder::InlineFragment::new(selection_set); + f.type_condition(type_condition.clone()); + for directive in directives.iter() { + f.directive(directive.clone()); + } + apollo_encoder::Selection::InlineFragment(f) + }), + ) as Box> + /*let mut f = apollo_encoder::FragmentSpread::new(name); for directive in fragment .directives() .into_iter() @@ -1221,15 +1295,7 @@ fn extract_selection( Box::new(std::iter::once(apollo_encoder::Selection::FragmentSpread( f, - ))) as Box> - - /*extract_selection_set(fragment.selection_set().as_ref().unwrap(), schema).map( - move |selection_set| { - let mut new_fragment = f.clone(); - new_fragment.selection_set(Some(selection_set)); - apollo_encoder::Selection::FragmentSpread(new_fragment) - }, - )*/ + ))) as Box>*/ } ast::Selection::InlineFragment(fragment) => { let type_condition = fragment.type_condition().and_then(|ty| ty.try_into().ok()); @@ -1243,16 +1309,19 @@ fn extract_selection( .collect::>(); Box::new( - extract_selection_set(fragment.selection_set().as_ref().unwrap(), schema).map( - move |selection_set| { - let mut f = apollo_encoder::InlineFragment::new(selection_set); - f.type_condition(type_condition.clone()); - for directive in directives.iter() { - f.directive(directive.clone()); - } - apollo_encoder::Selection::InlineFragment(f) - }, - ), + extract_selection_set( + fragment.selection_set().as_ref().unwrap(), + fragments, + schema, + ) + .map(move |selection_set| { + let mut f = apollo_encoder::InlineFragment::new(selection_set); + f.type_condition(type_condition.clone()); + for directive in directives.iter() { + f.directive(directive.clone()); + } + apollo_encoder::Selection::InlineFragment(f) + }), ) as Box> } } From 84e9d834cbe195ca6e2523915c2fa26b603f6907 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Wed, 16 Nov 2022 10:04:15 +0100 Subject: [PATCH 03/22] cleanup --- apollo-router/src/spec/query.rs | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index ec5bfe6268..56498794af 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -8,7 +8,6 @@ use std::collections::HashSet; use apollo_parser::ast; use apollo_parser::ast::AstNode; -use apollo_parser::ast::DirectiveLocation; use derivative::Derivative; use graphql::Error; use serde::de::Visitor; @@ -1099,8 +1098,6 @@ fn extract_operation( apollo_encoder::OperationType::Subscription } }, - //op_type.clone(), - //apollo_encoder::OperationType::Query, selection_set, ); operation.name(name.clone()); @@ -1208,7 +1205,7 @@ fn extract_selection( ) -> impl Iterator { match selection { ast::Selection::Field(field) => { - let mut name = field.name().map(|n| n.text().to_string()).unwrap(); + let name = field.name().map(|n| n.text().to_string()).unwrap(); let mut f = apollo_encoder::Field::new(name); for argument in field .arguments() @@ -1244,7 +1241,7 @@ fn extract_selection( } } ast::Selection::FragmentSpread(fragment) => { - let mut name = fragment + let name = fragment .fragment_name() .and_then(|f| f.name()) .map(|n| n.text().to_string()) @@ -1282,20 +1279,6 @@ fn extract_selection( apollo_encoder::Selection::InlineFragment(f) }), ) as Box> - /*let mut f = apollo_encoder::FragmentSpread::new(name); - for directive in fragment - .directives() - .into_iter() - .map(|dir| dir.directives()) - .flatten() - .filter_map(|directive| directive.try_into().ok()) - { - f.directive(directive); - } - - Box::new(std::iter::once(apollo_encoder::Selection::FragmentSpread( - f, - ))) as Box>*/ } ast::Selection::InlineFragment(fragment) => { let type_condition = fragment.type_condition().and_then(|ty| ty.try_into().ok()); From 5f993b924a8a9426bc45aec05896ea50563ace14 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Wed, 16 Nov 2022 10:04:40 +0100 Subject: [PATCH 04/22] do nnot add the defer directive to the generated query --- apollo-router/src/spec/query.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index 56498794af..a38829e886 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -1221,6 +1221,11 @@ fn extract_selection( .into_iter() .map(|dir| dir.directives()) .flatten() + .filter(|dir| { + dir.name() + .map(|n| n.text().as_str() != "defer") + .unwrap_or(true) + }) .filter_map(|directive| directive.try_into().ok()) { f.directive(directive); @@ -1256,6 +1261,11 @@ fn extract_selection( .into_iter() .map(|dir| dir.directives()) .flatten() + .filter(|dir| { + dir.name() + .map(|n| n.text().as_str() != "defer") + .unwrap_or(true) + }) .filter_map(|directive| directive.try_into().ok()) .collect::>(); @@ -1265,6 +1275,11 @@ fn extract_selection( .into_iter() .map(|dir| dir.directives()) .flatten() + .filter(|dir| { + dir.name() + .map(|n| n.text().as_str() != "defer") + .unwrap_or(true) + }) .filter_map(|directive| directive.try_into().ok()), ); @@ -1288,6 +1303,11 @@ fn extract_selection( .into_iter() .map(|dir| dir.directives()) .flatten() + .filter(|dir| { + dir.name() + .map(|n| n.text().as_str() != "defer") + .unwrap_or(true) + }) .filter_map(|directive| directive.try_into().ok()) .collect::>(); From 597fbc87873e811f48c7fdc0ad99125b7f59760c Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Wed, 16 Nov 2022 14:17:33 +0100 Subject: [PATCH 05/22] logs --- apollo-router/src/query_planner/plan.rs | 8 ++++++++ apollo-router/src/spec/selection.rs | 2 ++ 2 files changed, 10 insertions(+) diff --git a/apollo-router/src/query_planner/plan.rs b/apollo-router/src/query_planner/plan.rs index 8db166f31e..41d1c21e74 100644 --- a/apollo-router/src/query_planner/plan.rs +++ b/apollo-router/src/query_planner/plan.rs @@ -194,6 +194,10 @@ impl PlanNode { // re-create full query with the right path // parse the subselection let mut subselections = HashMap::new(); + println!( + "query plan:\n{}", + serde_json::to_string_pretty(&self).unwrap() + ); self.collect_subselections(schema, &Path::default(), &mut subselections)?; Ok(subselections) @@ -224,6 +228,7 @@ impl PlanNode { let primary_path = initial_path.join(&primary.path.clone().unwrap_or_default()); if let Some(primary_subselection) = &primary.subselection { let query = reconstruct_full_query(&primary_path, primary_subselection); + println!("[{}] PARSING reconstructed query {}", line!(), query); // ----------------------- Parse --------------------------------- let sub_selection = Query::parse(&query, schema, &Default::default())?; // ----------------------- END Parse --------------------------------- @@ -238,8 +243,11 @@ impl PlanNode { } deferred.iter().try_fold(subselections, |subs, current| { + //println!("deferred node: {:?}", current); + if let Some(subselection) = ¤t.subselection { let query = reconstruct_full_query(¤t.path, subselection); + println!("[{}] PARSING reconstructed query {}", line!(), query); // ----------------------- Parse --------------------------------- let sub_selection = Query::parse(&query, schema, &Default::default())?; // ----------------------- END Parse --------------------------------- diff --git a/apollo-router/src/spec/selection.rs b/apollo-router/src/spec/selection.rs index 11e67e36db..55a8306648 100644 --- a/apollo-router/src/spec/selection.rs +++ b/apollo-router/src/spec/selection.rs @@ -97,6 +97,7 @@ impl Selection { .text() .to_string(); + println!("trying to get type of {} in {}", field_name, current_type); let field_type = if field_name.as_str() == TYPENAME { FieldType::String } else if field_name == "__schema" { @@ -120,6 +121,7 @@ impl Selection { .get(name) .and_then(|ty| ty.field(&field_name)) }) + //.expect("invalid field") .ok_or_else(|| { SpecError::InvalidField(field_name.clone(), current_type.to_string()) })? From 73c24fbd34f4e3e9c032b7affa9c81fcebda40d9 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 5 Dec 2022 11:42:02 +0100 Subject: [PATCH 06/22] update router-bridge and apollo-encoder --- Cargo.lock | 5 ++--- apollo-router/Cargo.toml | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9c7ddd9f9a..733cbbae9a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -167,7 +167,7 @@ dependencies = [ "access-json", "ansi_term", "anyhow", - "apollo-encoder 0.3.4", + "apollo-encoder 0.4.0", "apollo-parser 0.4.0", "askama", "async-compression", @@ -4091,8 +4091,7 @@ dependencies = [ [[package]] name = "router-bridge" version = "0.1.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "74d2434deb0c39a41b7b68f968c78517c59b1032894c2e013244ba18d9fb49b4" +source = "git+https://github.com/apollographql/federation-rs?branch=geal/test-2.2.2-rc0#8a963a7e26792792b7f162caea9651bb29091da9" dependencies = [ "anyhow", "async-channel", diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml index a4e7ad6779..5bc951dbbe 100644 --- a/apollo-router/Cargo.toml +++ b/apollo-router/Cargo.toml @@ -153,7 +153,7 @@ reqwest = { version = "0.11.13", default-features = false, features = [ "json", "stream", ] } -router-bridge = "0.1.11" +router-bridge = { git = "https://github.com/apollographql/federation-rs", branch = "geal/test-2.2.2-rc0" } rust-embed="6.4.2" schemars = { version = "0.8.11", features = ["url"] } shellexpand = "2.1.2" @@ -194,7 +194,7 @@ urlencoding = "2.1.2" uuid = { version = "1.2.2", features = ["serde", "v4"] } yaml-rust = "0.4.5" askama = "0.11.1" -apollo-encoder = "0.3.4" +apollo-encoder = "0.4.0" [target.'cfg(macos)'.dependencies] uname = "0.1.1" From 405803c24fdd74762631c4710e6ecb3ad1436d0c Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 5 Dec 2022 15:55:43 +0100 Subject: [PATCH 07/22] handle queryPath in deferred nodes the deferred node's path now contains fragments to help in query reconstruction, and the flatten elements are not present anymore --- Cargo.lock | 2 +- apollo-router/src/json_ext.rs | 47 +++++ apollo-router/src/query_planner/execution.rs | 2 +- apollo-router/src/query_planner/plan.rs | 17 +- apollo-router/src/query_planner/tests.rs | 2 +- .../src/services/supergraph_service.rs | 179 +++++++++++------- 6 files changed, 171 insertions(+), 78 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 733cbbae9a..721ee32e31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4091,7 +4091,7 @@ dependencies = [ [[package]] name = "router-bridge" version = "0.1.11" -source = "git+https://github.com/apollographql/federation-rs?branch=geal/test-2.2.2-rc0#8a963a7e26792792b7f162caea9651bb29091da9" +source = "git+https://github.com/apollographql/federation-rs?branch=geal/test-2.2.2-rc0#657a5d4af4633a6aa31d92a599195ecdbc0609a5" dependencies = [ "anyhow", "async-channel", diff --git a/apollo-router/src/json_ext.rs b/apollo-router/src/json_ext.rs index 5e27a48eae..a385f72473 100644 --- a/apollo-router/src/json_ext.rs +++ b/apollo-router/src/json_ext.rs @@ -242,6 +242,7 @@ impl ValueExt for Value { .get_mut(k.as_str()) .expect("the value at that key was just inserted"); } + PathElement::Fragment(_) => {} } } @@ -319,6 +320,7 @@ impl ValueExt for Value { }) } }, + PathElement::Fragment(f) => {} } } @@ -402,8 +404,17 @@ where iterate_path(parent, &path[1..], value, f); parent.pop(); } + } else if let Value::Array(array) = data { + for (i, value) in array.iter().enumerate() { + parent.push(PathElement::Index(i)); + iterate_path(parent, &path, value, f); + parent.pop(); + } } } + Some(PathElement::Fragment(_)) => { + iterate_path(parent, &path[1..], data, f); + } } } @@ -422,6 +433,10 @@ pub enum PathElement { /// An index path element. Index(usize), + /// A fragment application + #[serde(deserialize_with = "deserialize_fragment")] + Fragment(String), + /// A key path element. Key(String), } @@ -464,6 +479,37 @@ where serializer.serialize_str("@") } +fn deserialize_fragment<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + deserializer.deserialize_str(FragmentVisitor) +} + +struct FragmentVisitor; + +impl<'de> serde::de::Visitor<'de> for FragmentVisitor { + type Value = String; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + write!(formatter, "a string that begins with '... '") + } + + fn visit_str(self, s: &str) -> Result + where + E: serde::de::Error, + { + if s.starts_with("... ") { + Ok(s.to_string()) + } else { + Err(serde::de::Error::invalid_value( + serde::de::Unexpected::Str(s), + &self, + )) + } + } +} + /// A path into the result document. /// /// This can be composed of strings and numbers @@ -587,6 +633,7 @@ impl fmt::Display for Path { PathElement::Index(index) => write!(f, "{}", index)?, PathElement::Key(key) => write!(f, "{}", key)?, PathElement::Flatten => write!(f, "@")?, + PathElement::Fragment(fragment) => write!(f, "{fragment}")?, } } Ok(()) diff --git a/apollo-router/src/query_planner/execution.rs b/apollo-router/src/query_planner/execution.rs index 363115b02b..5f898f2566 100644 --- a/apollo-router/src/query_planner/execution.rs +++ b/apollo-router/src/query_planner/execution.rs @@ -376,7 +376,7 @@ impl DeferredNode { let mut stream: stream::FuturesUnordered<_> = deferred_receivers.into_iter().collect(); //FIXME/ is there a solution without cloning the entire node? Maybe it could be moved instead? let deferred_inner = self.node.clone(); - let deferred_path = self.path.clone(); + let deferred_path = self.query_path.clone(); let subselection = self.subselection(); let label = self.label.clone(); let mut tx = sender; diff --git a/apollo-router/src/query_planner/plan.rs b/apollo-router/src/query_planner/plan.rs index 74a3fb1b08..99a9eaa6ab 100644 --- a/apollo-router/src/query_planner/plan.rs +++ b/apollo-router/src/query_planner/plan.rs @@ -245,7 +245,11 @@ impl PlanNode { //println!("deferred node: {:?}", current); if let Some(subselection) = ¤t.subselection { - let query = reconstruct_full_query(¤t.path, kind, subselection); + println!( + "will reconstruct deferred {} at path {}: {}", + kind, current.query_path, subselection + ); + let query = reconstruct_full_query(¤t.query_path, kind, subselection); println!("[{}] PARSING reconstructed query {}", line!(), query); // ----------------------- Parse --------------------------------- @@ -254,7 +258,7 @@ impl PlanNode { subs.insert( SubSelection { - path: current.path.clone(), + path: current.query_path.clone(), subselection: subselection.clone(), }, sub_selection, @@ -263,7 +267,7 @@ impl PlanNode { if let Some(current_node) = ¤t.node { current_node.collect_subselections( schema, - &initial_path.join(¤t.path), + &initial_path.join(¤t.query_path), kind, subs, )?; @@ -349,6 +353,11 @@ fn reconstruct_full_query(path: &Path, kind: &OperationKind, subselection: &str) .expect("writing to a String should not fail because it can reallocate"); len += 1; } + json_ext::PathElement::Fragment(fragment) => { + write!(&mut query, "{{ {fragment}") + .expect("writing to a String should not fail because it can reallocate"); + len += 1; + } } } @@ -399,7 +408,7 @@ pub(crate) struct DeferredNode { /// The optional defer label. pub(crate) label: Option, /// Path to the @defer this correspond to. `subselection` start at that `path`. - pub(crate) path: Path, + pub(crate) query_path: Path, /// The part of the original query that "selects" the data to send /// in that deferred response (once the plan in `node` completes). /// Will be set _unless_ `node` is a `DeferNode` itself. diff --git a/apollo-router/src/query_planner/tests.rs b/apollo-router/src/query_planner/tests.rs index 7d372f9e9e..acbface5f7 100644 --- a/apollo-router/src/query_planner/tests.rs +++ b/apollo-router/src/query_planner/tests.rs @@ -242,7 +242,7 @@ async fn defer() { defer_label: None, }], label: None, - path: Path(vec![PathElement::Key("t".to_string())]), + query_path: Path(vec![PathElement::Key("t".to_string())]), subselection: Some("{ y }".to_string()), node: Some(Arc::new(PlanNode::Flatten(FlattenNode { path: Path(vec![PathElement::Key("t".to_string())]), diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index 7cd73f666d..b68f3621f6 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -841,6 +841,8 @@ mod tests { } "#; + // this test does not need to generate a valid response, it is only here to check + // that the router does not panic when reconstructing the query for the deferred part let service = TestHarness::builder() .configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true } })) .unwrap() @@ -872,81 +874,117 @@ mod tests { } #[tokio::test] - async fn query_reconstruction2() { + async fn reconstruct_deferred_query_under_interface() { let schema = r#"schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.2", for: EXECUTION) - @link(url: "https://specs.apollo.dev/tag/v0.2") - @link(url: "https://specs.apollo.dev/inaccessible/v0.2") - { - query: Query - } - - directive @inaccessible on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION - directive @join__field(graph: join__Graph!, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION - directive @join__graph(name: String!, url: String!) on ENUM_VALUE - directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE - directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA - directive @tag(name: String!) repeatable on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION - - scalar join__FieldSet - enum join__Graph { - USER @join__graph(name: "user", url: "http://localhost:4000/graphql") - } - scalar link__Import - enum link__Purpose { - SECURITY - EXECUTION - } - type Query - @join__type(graph: USER) - { - me: Identity @join__field(graph: USER) - } - interface Identity - @join__type(graph: USER) - { - id: ID! - name: String! - } - - type User implements Identity - @join__implements(graph: USER, interface: "Identity") - @join__type(graph: USER, key: "id") - { - fullName: String! @join__field(graph: USER) - id: ID! - memberships: [UserMembership!]! @join__field(graph: USER) - name: String! @join__field(graph: USER) - } - type UserMembership - @join__type(graph: USER) - @tag(name: "platform-api") - { - """The organization that the user belongs to.""" - account: Account! - """The user's permission level within the organization.""" - permission: UserPermission! - } - enum UserPermission - @join__type(graph: USER) - { - USER - ADMIN - } - type Account - @join__type(graph: USER, key: "id") - { - id: ID! @join__field(graph: USER) - name: String! @join__field(graph: USER) - } -"#; + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.2", for: EXECUTION) + @link(url: "https://specs.apollo.dev/tag/v0.2") + @link(url: "https://specs.apollo.dev/inaccessible/v0.2") + { + query: Query + } + + directive @inaccessible on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION + directive @join__field(graph: join__Graph!, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + directive @join__graph(name: String!, url: String!) on ENUM_VALUE + directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE + directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + directive @tag(name: String!) repeatable on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION + + scalar join__FieldSet + enum join__Graph { + USER @join__graph(name: "user", url: "http://localhost:4000/graphql") + } + scalar link__Import + enum link__Purpose { + SECURITY + EXECUTION + } + + type Query + @join__type(graph: USER) + { + me: Identity @join__field(graph: USER) + } + interface Identity + @join__type(graph: USER) + { + id: ID! + name: String! + } + + type User implements Identity + @join__implements(graph: USER, interface: "Identity") + @join__type(graph: USER, key: "id") + { + fullName: String! @join__field(graph: USER) + id: ID! + memberships: [UserMembership!]! @join__field(graph: USER) + name: String! @join__field(graph: USER) + } + type UserMembership + @join__type(graph: USER) + @tag(name: "platform-api") + { + """The organization that the user belongs to.""" + account: Account! + """The user's permission level within the organization.""" + permission: UserPermission! + } + enum UserPermission + @join__type(graph: USER) + { + USER + ADMIN + } + type Account + @join__type(graph: USER, key: "id") + { + id: ID! @join__field(graph: USER) + name: String! @join__field(graph: USER) + }"#; + + let subgraphs = MockedSubgraphs([ + ("user", MockSubgraph::builder().with_json( + serde_json::json!{{"query":"{me{__typename ...on User{id fullName memberships{permission account{__typename id}}}}}"}}, + serde_json::json!{{"data": {"me": { + "__typename": "User", + "id": 0, + "fullName": "A", + "memberships": [ + { + "permission": "USER", + "account": { + "__typename": "Account", + "id": 1 + } + } + ] + }}}} + ) .with_json( + serde_json::json!{{ + "query":"query($representations:[_Any!]!){_entities(representations:$representations){...on Account{name}}}", + "variables": { + "representations":[ + {"__typename": "Account", "id": 1} + ] + } + }}, + serde_json::json!{{ + "data": { + "_entities": [ + { "__typename": "Account", "id": 1, "name": "B"} + ] + } + }}).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(); @@ -977,7 +1015,6 @@ mod tests { let mut stream = service.oneshot(request).await.unwrap(); insta::assert_json_snapshot!(stream.next_response().await.unwrap()); - - panic!() + insta::assert_json_snapshot!(stream.next_response().await.unwrap()); } } From 02bdb7837414f9bc0a02caf6f5688adab4b3a27d Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 6 Dec 2022 11:39:21 +0100 Subject: [PATCH 08/22] split arrays --- apollo-router/src/axum_factory/tests.rs | 26 +------------------ apollo-router/src/json_ext.rs | 2 +- .../testdata/defer_clause_plan.json | 2 +- .../src/services/execution_service.rs | 13 +++++++++- apollo-router/src/spec/selection.rs | 2 -- 5 files changed, 15 insertions(+), 30 deletions(-) diff --git a/apollo-router/src/axum_factory/tests.rs b/apollo-router/src/axum_factory/tests.rs index 8b41246005..c5b126f99d 100644 --- a/apollo-router/src/axum_factory/tests.rs +++ b/apollo-router/src/axum_factory/tests.rs @@ -1922,31 +1922,7 @@ async fn test_defer_is_not_buffered() { let (parts, counts): (Vec<_>, Vec<_>) = parts.map(|part| (part, counter.get())).unzip().await; let parts = serde_json::Value::Array(parts); - assert_eq!( - parts, - json!([ - { - "data": { - "topProducts": [ - {"upc": "1", "name": "Table", "reviews": null}, - {"upc": "2", "name": "Couch", "reviews": null} - ] - }, - "errors": [ - { - "message": "couldn't find mock for query {\"query\":\"query TopProducts__reviews__1($representations:[_Any!]!){_entities(representations:$representations){...on Product{reviews{__typename id product{__typename upc}}}}}\",\"operationName\":\"TopProducts__reviews__1\",\"variables\":{\"representations\":[{\"__typename\":\"Product\",\"upc\":\"1\"},{\"__typename\":\"Product\",\"upc\":\"2\"}]}}" - }, - { - "message": "Subgraph response from 'reviews' was missing key `_entities`", - "path": [ "topProducts", "@" ] - }], - "hasNext": true, - }, - {"hasNext": false} - ]), - "{}", - serde_json::to_string(&parts).unwrap() - ); + insta::assert_json_snapshot!(parts); // Non-regression test for https://github.com/apollographql/router/issues/1572 // diff --git a/apollo-router/src/json_ext.rs b/apollo-router/src/json_ext.rs index a385f72473..706f6d7e8a 100644 --- a/apollo-router/src/json_ext.rs +++ b/apollo-router/src/json_ext.rs @@ -320,7 +320,7 @@ impl ValueExt for Value { }) } }, - PathElement::Fragment(f) => {} + PathElement::Fragment(_) => {} } } diff --git a/apollo-router/src/query_planner/testdata/defer_clause_plan.json b/apollo-router/src/query_planner/testdata/defer_clause_plan.json index 62de983cdf..f483b0e3b3 100644 --- a/apollo-router/src/query_planner/testdata/defer_clause_plan.json +++ b/apollo-router/src/query_planner/testdata/defer_clause_plan.json @@ -25,7 +25,7 @@ } ], "label": null, - "path": ["me"], + "queryPath": ["me"], "subselection": "{ ... on User { name username } }", "node": { "kind": "Flatten", diff --git a/apollo-router/src/services/execution_service.rs b/apollo-router/src/services/execution_service.rs index 3b3fa67ede..ede382db3e 100644 --- a/apollo-router/src/services/execution_service.rs +++ b/apollo-router/src/services/execution_service.rs @@ -11,6 +11,7 @@ use futures::future::BoxFuture; use futures::stream::once; use futures::SinkExt; use futures::StreamExt; +use serde_json_bytes::Value; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -23,6 +24,7 @@ use super::subgraph_service::SubgraphServiceFactory; use super::Plugins; use crate::graphql::IncrementalResponse; use crate::graphql::Response; +use crate::json_ext::PathElement; use crate::json_ext::ValueExt; use crate::services::execution; use crate::ExecutionRequest; @@ -125,7 +127,16 @@ where (Some(response_path), Some(response_data)) => { let mut sub_responses = Vec::new(); response_data.select_values_and_paths(response_path, |path, value| { - sub_responses.push((path.clone(), value.clone())); + if let Value::Array(array) = value { + let mut parent = path.clone(); + for (i, value) in array.iter().enumerate() { + parent.push(PathElement::Index(i)); + sub_responses.push((parent.clone(), value.clone())); + parent.pop(); + } + } else { + sub_responses.push((path.clone(), value.clone())); + } }); Response::builder() diff --git a/apollo-router/src/spec/selection.rs b/apollo-router/src/spec/selection.rs index 55a8306648..11e67e36db 100644 --- a/apollo-router/src/spec/selection.rs +++ b/apollo-router/src/spec/selection.rs @@ -97,7 +97,6 @@ impl Selection { .text() .to_string(); - println!("trying to get type of {} in {}", field_name, current_type); let field_type = if field_name.as_str() == TYPENAME { FieldType::String } else if field_name == "__schema" { @@ -121,7 +120,6 @@ impl Selection { .get(name) .and_then(|ty| ty.field(&field_name)) }) - //.expect("invalid field") .ok_or_else(|| { SpecError::InvalidField(field_name.clone(), current_type.to_string()) })? From 0563c780979387df0de733445d9e98ca69c6f817 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 6 Dec 2022 13:36:22 +0100 Subject: [PATCH 09/22] missing snapshots --- ...ruct_deferred_query_under_interface-2.snap | 20 +++++++++++++++++++ ...struct_deferred_query_under_interface.snap | 18 +++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__reconstruct_deferred_query_under_interface-2.snap create mode 100644 apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__reconstruct_deferred_query_under_interface.snap diff --git a/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__reconstruct_deferred_query_under_interface-2.snap b/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__reconstruct_deferred_query_under_interface-2.snap new file mode 100644 index 0000000000..b40d41ef1a --- /dev/null +++ b/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__reconstruct_deferred_query_under_interface-2.snap @@ -0,0 +1,20 @@ +--- +source: apollo-router/src/services/supergraph_service.rs +expression: stream.next_response().await.unwrap() +--- +{ + "hasNext": false, + "incremental": [ + { + "data": { + "name": "B" + }, + "path": [ + "me", + "memberships", + 0, + "account" + ] + } + ] +} diff --git a/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__reconstruct_deferred_query_under_interface.snap b/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__reconstruct_deferred_query_under_interface.snap new file mode 100644 index 0000000000..b9fea0b168 --- /dev/null +++ b/apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__reconstruct_deferred_query_under_interface.snap @@ -0,0 +1,18 @@ +--- +source: apollo-router/src/services/supergraph_service.rs +expression: stream.next_response().await.unwrap() +--- +{ + "data": { + "me": { + "id": 0, + "fullName": "A", + "memberships": [ + { + "permission": "USER" + } + ] + } + }, + "hasNext": true +} From f903ee5960063f1644192a0506e9aeabc8c45cd0 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 6 Dec 2022 13:50:33 +0100 Subject: [PATCH 10/22] missing snapshot --- ...factory__tests__defer_is_not_buffered.snap | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 apollo-router/src/axum_factory/snapshots/apollo_router__axum_factory__tests__defer_is_not_buffered.snap diff --git a/apollo-router/src/axum_factory/snapshots/apollo_router__axum_factory__tests__defer_is_not_buffered.snap b/apollo-router/src/axum_factory/snapshots/apollo_router__axum_factory__tests__defer_is_not_buffered.snap new file mode 100644 index 0000000000..166a799aa1 --- /dev/null +++ b/apollo-router/src/axum_factory/snapshots/apollo_router__axum_factory__tests__defer_is_not_buffered.snap @@ -0,0 +1,38 @@ +--- +source: apollo-router/src/axum_factory/tests.rs +expression: "json!([{\n \"data\" :\n {\n \"topProducts\" :\n [{ \"upc\" : \"1\", \"name\" : \"Table\", \"reviews\" : null },\n { \"upc\" : \"2\", \"name\" : \"Couch\", \"reviews\" : null }]\n }, \"errors\" :\n [{\n \"message\" :\n \"couldn't find mock for query {\\\"query\\\":\\\"query TopProducts__reviews__1($representations:[_Any!]!){_entities(representations:$representations){...on Product{reviews{__typename id product{__typename upc}}}}}\\\",\\\"operationName\\\":\\\"TopProducts__reviews__1\\\",\\\"variables\\\":{\\\"representations\\\":[{\\\"__typename\\\":\\\"Product\\\",\\\"upc\\\":\\\"1\\\"},{\\\"__typename\\\":\\\"Product\\\",\\\"upc\\\":\\\"2\\\"}]}}\"\n },\n {\n \"message\" :\n \"Subgraph response from 'reviews' was missing key `_entities`\",\n \"path\" : [\"topProducts\", \"@\"]\n }], \"hasNext\" : true,\n }, { \"hasNext\" : false }])" +--- +[ + { + "data": { + "topProducts": [ + { + "upc": "1", + "name": "Table", + "reviews": null + }, + { + "upc": "2", + "name": "Couch", + "reviews": null + } + ] + }, + "errors": [ + { + "message": "couldn't find mock for query {\"query\":\"query TopProducts__reviews__1($representations:[_Any!]!){_entities(representations:$representations){...on Product{reviews{__typename id product{__typename upc}}}}}\",\"operationName\":\"TopProducts__reviews__1\",\"variables\":{\"representations\":[{\"__typename\":\"Product\",\"upc\":\"1\"},{\"__typename\":\"Product\",\"upc\":\"2\"}]}}" + }, + { + "message": "Subgraph response from 'reviews' was missing key `_entities`", + "path": [ + "topProducts", + "@" + ] + } + ], + "hasNext": true + }, + { + "hasNext": false + } +] From ecb2dcba5dbc934b0b988bce3f33e9cf1dd79e6c Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 6 Dec 2022 14:25:26 +0100 Subject: [PATCH 11/22] lint --- apollo-router/src/json_ext.rs | 2 +- .../src/services/execution_service.rs | 2 - apollo-router/src/spec/query.rs | 93 +++++++++++-------- 3 files changed, 57 insertions(+), 40 deletions(-) diff --git a/apollo-router/src/json_ext.rs b/apollo-router/src/json_ext.rs index 706f6d7e8a..e16d2f2e48 100644 --- a/apollo-router/src/json_ext.rs +++ b/apollo-router/src/json_ext.rs @@ -407,7 +407,7 @@ where } else if let Value::Array(array) = data { for (i, value) in array.iter().enumerate() { parent.push(PathElement::Index(i)); - iterate_path(parent, &path, value, f); + iterate_path(parent, path, value, f); parent.pop(); } } diff --git a/apollo-router/src/services/execution_service.rs b/apollo-router/src/services/execution_service.rs index 13614c2e6a..4f3a6acf7f 100644 --- a/apollo-router/src/services/execution_service.rs +++ b/apollo-router/src/services/execution_service.rs @@ -11,7 +11,6 @@ use futures::future::BoxFuture; use futures::stream::once; use futures::SinkExt; use futures::StreamExt; -use serde_json_bytes::Value; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -25,7 +24,6 @@ use super::Plugins; use crate::graphql::IncrementalResponse; use crate::graphql::Response; use crate::json_ext::Path; -use crate::json_ext::PathElement; use crate::json_ext::ValueExt; use crate::services::execution; use crate::ExecutionRequest; diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index b620f5a473..92bfb3e7bb 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -5,6 +5,7 @@ use std::collections::HashMap; use std::collections::HashSet; +use std::iter::empty; use apollo_parser::ast; use apollo_parser::ast::AstNode; @@ -1115,7 +1116,7 @@ fn extract_subqueries( let mut v = Vec::new(); //let op: apollo_encoder::OperationDefinition = operation.clone().try_into().unwrap(); - extract_operation(&operation, fragments, schema, &mut v); + extract_operation(operation, fragments, schema, &mut v); println!("subqueries: {:#?}", v); } @@ -1123,9 +1124,9 @@ fn extract_operation( operation: &ast::OperationDefinition, fragments: &HashMap, schema: &Schema, - v: &mut Vec, + _v: &mut [String], ) { - let name: Option = operation.name().map(|n| n.text().to_string().into()); + let name: Option = operation.name().map(|n| n.text().to_string()); let op_type: apollo_encoder::OperationType = operation .operation_type() .and_then(|op| op.try_into().ok()) @@ -1227,7 +1228,7 @@ fn extract_selection_set( let mut primary = apollo_encoder::SelectionSet::new(); for selection_it in primary_selection_set.iter_mut() { - if let Some(selection) = selection_it.next().and_then(|s| s.try_into().ok()) { + if let Some(selection) = selection_it.next() { primary.selection(selection); } } @@ -1237,7 +1238,6 @@ fn extract_selection_set( .into_iter() .chain(deferred_selection_set.into_iter()) .flatten() - .filter_map(|s| s.try_into().ok()) .map(|s| { let mut set = apollo_encoder::SelectionSet::new(); set.selection(s); @@ -1253,13 +1253,18 @@ fn extract_selection( ) -> impl Iterator { match selection { ast::Selection::Field(field) => { - let name = field.name().map(|n| n.text().to_string()).unwrap(); + let name = match field.name().map(|n| n.text().to_string()) { + Some(name) => name, + None => { + return Box::new(empty()) as Box> + } + }; + let mut f = apollo_encoder::Field::new(name); for argument in field .arguments() .into_iter() - .map(|arg| arg.arguments()) - .flatten() + .flat_map(|arg| arg.arguments()) .filter_map(|argument| argument.try_into().ok()) { f.argument(argument); @@ -1267,8 +1272,7 @@ fn extract_selection( for directive in field .directives() .into_iter() - .map(|dir| dir.directives()) - .flatten() + .flat_map(|dir| dir.directives()) .filter(|dir| { dir.name() .map(|n| n.text().as_str() != "defer") @@ -1294,21 +1298,28 @@ fn extract_selection( } } ast::Selection::FragmentSpread(fragment) => { - let name = fragment + let name = match fragment .fragment_name() .and_then(|f| f.name()) .map(|n| n.text().to_string()) - .unwrap(); + { + Some(name) => name, + None => { + return Box::new(empty()) as Box> + } + }; - let named = fragments.get(&name).unwrap(); + let named = fragments.get(&name); - let type_condition = named.type_condition().and_then(|ty| ty.try_into().ok()); + let type_condition = named + .as_ref() + .and_then(|n| n.type_condition()) + .and_then(|ty| ty.try_into().ok()); let mut directives: Vec = fragment .directives() .into_iter() - .map(|dir| dir.directives()) - .flatten() + .flat_map(|dir| dir.directives()) .filter(|dir| { dir.name() .map(|n| n.text().as_str() != "defer") @@ -1319,10 +1330,9 @@ fn extract_selection( directives.extend( named - .directives() .into_iter() - .map(|dir| dir.directives()) - .flatten() + .flat_map(|n| n.directives().into_iter()) + .flat_map(|dir| dir.directives()) .filter(|dir| { dir.name() .map(|n| n.text().as_str() != "defer") @@ -1331,16 +1341,23 @@ fn extract_selection( .filter_map(|directive| directive.try_into().ok()), ); + let selection_set = match named.as_ref().and_then(|n| n.selection_set()) { + Some(d) => d, + None => { + return Box::new(empty()) as Box> + } + }; Box::new( - extract_selection_set(named.selection_set().as_ref().unwrap(), fragments, schema) - .map(move |selection_set| { + extract_selection_set(&selection_set, fragments, schema).map( + move |selection_set| { let mut f = apollo_encoder::InlineFragment::new(selection_set); f.type_condition(type_condition.clone()); for directive in directives.iter() { f.directive(directive.clone()); } apollo_encoder::Selection::InlineFragment(f) - }), + }, + ), ) as Box> } ast::Selection::InlineFragment(fragment) => { @@ -1349,8 +1366,7 @@ fn extract_selection( let directives = fragment .directives() .into_iter() - .map(|dir| dir.directives()) - .flatten() + .flat_map(|dir| dir.directives()) .filter(|dir| { dir.name() .map(|n| n.text().as_str() != "defer") @@ -1359,20 +1375,23 @@ fn extract_selection( .filter_map(|directive| directive.try_into().ok()) .collect::>(); + let selection_set = match fragment.selection_set() { + Some(d) => d, + None => { + return Box::new(empty()) as Box> + } + }; Box::new( - extract_selection_set( - fragment.selection_set().as_ref().unwrap(), - fragments, - schema, - ) - .map(move |selection_set| { - let mut f = apollo_encoder::InlineFragment::new(selection_set); - f.type_condition(type_condition.clone()); - for directive in directives.iter() { - f.directive(directive.clone()); - } - apollo_encoder::Selection::InlineFragment(f) - }), + extract_selection_set(&selection_set, fragments, schema).map( + move |selection_set| { + let mut f = apollo_encoder::InlineFragment::new(selection_set); + f.type_condition(type_condition.clone()); + for directive in directives.iter() { + f.directive(directive.clone()); + } + apollo_encoder::Selection::InlineFragment(f) + }, + ), ) as Box> } } From 93a7ec2eac9c28099b4dd494ba8b908a0879b75d Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 6 Dec 2022 14:43:59 +0100 Subject: [PATCH 12/22] handle arrays --- .../src/services/execution_service.rs | 60 +++++++++++-------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/apollo-router/src/services/execution_service.rs b/apollo-router/src/services/execution_service.rs index 4f3a6acf7f..bb499e3982 100644 --- a/apollo-router/src/services/execution_service.rs +++ b/apollo-router/src/services/execution_service.rs @@ -11,6 +11,7 @@ use futures::future::BoxFuture; use futures::stream::once; use futures::SinkExt; use futures::StreamExt; +use serde_json_bytes::Value; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -24,6 +25,7 @@ use super::Plugins; use crate::graphql::IncrementalResponse; use crate::graphql::Response; use crate::json_ext::Path; +use crate::json_ext::PathElement; use crate::json_ext::ValueExt; use crate::services::execution; use crate::ExecutionRequest; @@ -153,34 +155,42 @@ where } }); + let incremental = sub_responses + .into_iter() + .filter_map(move |(path, data)| { + let errors = response + .errors + .iter() + .filter(|error| match &error.path { + None => false, + Some(err_path) => err_path.starts_with(&path), + }) + .cloned() + .collect::>(); + + if !data.is_null() + || !errors.is_empty() + || !response.extensions.is_empty() + { + Some( + IncrementalResponse::builder() + .and_label(response.label.clone()) + .data(data) + .path(path) + .errors(errors) + .extensions(response.extensions.clone()) + .build(), + ) + } else { + None + } + }) + .collect(); + ready(Some( Response::builder() .has_next(has_next) - .incremental( - sub_responses - .into_iter() - .map(move |(path, data)| { - let errors = response - .errors - .iter() - .filter(|error| match &error.path { - None => false, - Some(err_path) => { - err_path.starts_with(&path) - } - }) - .cloned() - .collect::>(); - IncrementalResponse::builder() - .and_label(response.label.clone()) - .data(data) - .path(path) - .errors(errors) - .extensions(response.extensions.clone()) - .build() - }) - .collect(), - ) + .incremental(incremental) .build(), )) } From 9b222bf70fd95d32a58b238fc21b453aa88acacd Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 6 Dec 2022 14:52:32 +0100 Subject: [PATCH 13/22] remove unused code --- apollo-router/src/query_planner/plan.rs | 8 - apollo-router/src/spec/query.rs | 310 +----------------------- 2 files changed, 2 insertions(+), 316 deletions(-) diff --git a/apollo-router/src/query_planner/plan.rs b/apollo-router/src/query_planner/plan.rs index 99a9eaa6ab..699e3a0f58 100644 --- a/apollo-router/src/query_planner/plan.rs +++ b/apollo-router/src/query_planner/plan.rs @@ -226,7 +226,6 @@ impl PlanNode { let primary_path = initial_path.join(&primary.path.clone().unwrap_or_default()); if let Some(primary_subselection) = &primary.subselection { let query = reconstruct_full_query(&primary_path, kind, primary_subselection); - println!("[{}] PARSING reconstructed query {}", line!(), query); // ----------------------- Parse --------------------------------- let sub_selection = Query::parse(&query, schema, &Default::default())?; @@ -242,15 +241,8 @@ impl PlanNode { } deferred.iter().try_fold(subselections, |subs, current| { - //println!("deferred node: {:?}", current); - if let Some(subselection) = ¤t.subselection { - println!( - "will reconstruct deferred {} at path {}: {}", - kind, current.query_path, subselection - ); let query = reconstruct_full_query(¤t.query_path, kind, subselection); - println!("[{}] PARSING reconstructed query {}", line!(), query); // ----------------------- Parse --------------------------------- let sub_selection = Query::parse(&query, schema, &Default::default())?; diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index 92bfb3e7bb..73e08e9e6e 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -5,7 +5,6 @@ use std::collections::HashMap; use std::collections::HashSet; -use std::iter::empty; use apollo_parser::ast; use apollo_parser::ast::AstNode; @@ -236,17 +235,6 @@ impl Query { let document = tree.document(); let fragments = Fragments::from_ast(&document, schema)?; - let ast_fragments = document - .definitions() - .filter_map(|def| match def { - ast::Definition::FragmentDefinition(f) => f - .fragment_name() - .and_then(|f| f.name()) - .map(|n| (n.text().to_string(), f)), - _ => None, - }) - .collect::>(); - let operations: Vec = document .definitions() .filter_map(|definition| { @@ -256,7 +244,7 @@ impl Query { None } }) - .map(|operation| Operation::from_ast(operation, &ast_fragments, schema)) + .map(|operation| Operation::from_ast(operation, schema)) .collect::, SpecError>>()?; Ok(Query { @@ -997,12 +985,7 @@ impl Operation { // ref: https://rust-lang.github.io/rust-clippy/master/index.html#mutable_key_type #[allow(clippy::mutable_key_type)] // Spec: https://spec.graphql.org/draft/#sec-Language.Operations - fn from_ast( - operation: ast::OperationDefinition, - fragments: &HashMap, - schema: &Schema, - ) -> Result { - extract_subqueries(&operation, fragments, schema); + fn from_ast(operation: ast::OperationDefinition, schema: &Schema) -> Result { let name = operation.name().map(|x| x.text().to_string()); let kind = operation @@ -1108,295 +1091,6 @@ impl Operation { } } -fn extract_subqueries( - operation: &ast::OperationDefinition, - fragments: &HashMap, - schema: &Schema, -) { - let mut v = Vec::new(); - - //let op: apollo_encoder::OperationDefinition = operation.clone().try_into().unwrap(); - extract_operation(operation, fragments, schema, &mut v); - println!("subqueries: {:#?}", v); -} - -fn extract_operation( - operation: &ast::OperationDefinition, - fragments: &HashMap, - schema: &Schema, - _v: &mut [String], -) { - let name: Option = operation.name().map(|n| n.text().to_string()); - let op_type: apollo_encoder::OperationType = operation - .operation_type() - .and_then(|op| op.try_into().ok()) - .unwrap_or(apollo_encoder::OperationType::Query); - - if let Some(selection_set) = operation.selection_set() { - for selection_set in extract_selection_set(&selection_set, fragments, schema) { - let mut document = apollo_encoder::Document::new(); - - let mut operation = apollo_encoder::OperationDefinition::new( - //apollo_encoder::OperationType is not Clone yet - match &op_type { - apollo_encoder::OperationType::Query => apollo_encoder::OperationType::Query, - apollo_encoder::OperationType::Mutation => { - apollo_encoder::OperationType::Mutation - } - apollo_encoder::OperationType::Subscription => { - apollo_encoder::OperationType::Subscription - } - }, - selection_set, - ); - operation.name(name.clone()); - //FIXME: variables, directives - - document.operation(operation); - - println!("EXTRACTED: {}", document); - } - } -} - -fn extract_selection_set( - selection_set: &ast::SelectionSet, - fragments: &HashMap, - schema: &Schema, -) -> impl Iterator { - let mut primary_selection_set = Vec::new(); - let mut deferred_selection_set = Vec::new(); - - for selection in selection_set.selections() { - match selection { - ast::Selection::Field(f) => primary_selection_set.push(extract_selection( - &ast::Selection::Field(f), - fragments, - schema, - )), - ast::Selection::FragmentSpread(f) => { - if f.directives() - .map(|d| { - d.directives().any(|dir| { - dir.name() - .map(|n| n.text().as_str() == "defer") - .unwrap_or(false) - }) - }) - .unwrap_or(false) - { - deferred_selection_set.push(extract_selection( - &ast::Selection::FragmentSpread(f), - fragments, - schema, - )); - } else { - primary_selection_set.push(extract_selection( - &ast::Selection::FragmentSpread(f), - fragments, - schema, - )); - } - } - ast::Selection::InlineFragment(f) => { - if f.directives() - .map(|d| { - d.directives().any(|dir| { - dir.name() - .map(|n| n.text().as_str() == "defer") - .unwrap_or(false) - }) - }) - .unwrap_or(false) - { - deferred_selection_set.push(extract_selection( - &ast::Selection::InlineFragment(f), - fragments, - schema, - )); - } else { - primary_selection_set.push(extract_selection( - &ast::Selection::InlineFragment(f), - fragments, - schema, - )); - } - } - } - } - - let mut primary = apollo_encoder::SelectionSet::new(); - - for selection_it in primary_selection_set.iter_mut() { - if let Some(selection) = selection_it.next() { - primary.selection(selection); - } - } - - std::iter::once(primary).chain( - primary_selection_set - .into_iter() - .chain(deferred_selection_set.into_iter()) - .flatten() - .map(|s| { - let mut set = apollo_encoder::SelectionSet::new(); - set.selection(s); - set - }), - ) -} - -fn extract_selection( - selection: &ast::Selection, - fragments: &HashMap, - schema: &Schema, -) -> impl Iterator { - match selection { - ast::Selection::Field(field) => { - let name = match field.name().map(|n| n.text().to_string()) { - Some(name) => name, - None => { - return Box::new(empty()) as Box> - } - }; - - let mut f = apollo_encoder::Field::new(name); - for argument in field - .arguments() - .into_iter() - .flat_map(|arg| arg.arguments()) - .filter_map(|argument| argument.try_into().ok()) - { - f.argument(argument); - } - for directive in field - .directives() - .into_iter() - .flat_map(|dir| dir.directives()) - .filter(|dir| { - dir.name() - .map(|n| n.text().as_str() != "defer") - .unwrap_or(true) - }) - .filter_map(|directive| directive.try_into().ok()) - { - f.directive(directive); - } - - match field.selection_set().as_ref() { - None => Box::new(std::iter::once(apollo_encoder::Selection::Field(f))) - as Box>, - Some(selection_set) => { - Box::new(extract_selection_set(selection_set, fragments, schema).map( - move |selection_set| { - let mut new_field = f.clone(); - new_field.selection_set(Some(selection_set)); - apollo_encoder::Selection::Field(new_field) - }, - )) as Box> - } - } - } - ast::Selection::FragmentSpread(fragment) => { - let name = match fragment - .fragment_name() - .and_then(|f| f.name()) - .map(|n| n.text().to_string()) - { - Some(name) => name, - None => { - return Box::new(empty()) as Box> - } - }; - - let named = fragments.get(&name); - - let type_condition = named - .as_ref() - .and_then(|n| n.type_condition()) - .and_then(|ty| ty.try_into().ok()); - - let mut directives: Vec = fragment - .directives() - .into_iter() - .flat_map(|dir| dir.directives()) - .filter(|dir| { - dir.name() - .map(|n| n.text().as_str() != "defer") - .unwrap_or(true) - }) - .filter_map(|directive| directive.try_into().ok()) - .collect::>(); - - directives.extend( - named - .into_iter() - .flat_map(|n| n.directives().into_iter()) - .flat_map(|dir| dir.directives()) - .filter(|dir| { - dir.name() - .map(|n| n.text().as_str() != "defer") - .unwrap_or(true) - }) - .filter_map(|directive| directive.try_into().ok()), - ); - - let selection_set = match named.as_ref().and_then(|n| n.selection_set()) { - Some(d) => d, - None => { - return Box::new(empty()) as Box> - } - }; - Box::new( - extract_selection_set(&selection_set, fragments, schema).map( - move |selection_set| { - let mut f = apollo_encoder::InlineFragment::new(selection_set); - f.type_condition(type_condition.clone()); - for directive in directives.iter() { - f.directive(directive.clone()); - } - apollo_encoder::Selection::InlineFragment(f) - }, - ), - ) as Box> - } - ast::Selection::InlineFragment(fragment) => { - let type_condition = fragment.type_condition().and_then(|ty| ty.try_into().ok()); - - let directives = fragment - .directives() - .into_iter() - .flat_map(|dir| dir.directives()) - .filter(|dir| { - dir.name() - .map(|n| n.text().as_str() != "defer") - .unwrap_or(true) - }) - .filter_map(|directive| directive.try_into().ok()) - .collect::>(); - - let selection_set = match fragment.selection_set() { - Some(d) => d, - None => { - return Box::new(empty()) as Box> - } - }; - Box::new( - extract_selection_set(&selection_set, fragments, schema).map( - move |selection_set| { - let mut f = apollo_encoder::InlineFragment::new(selection_set); - f.type_condition(type_condition.clone()); - for directive in directives.iter() { - f.directive(directive.clone()); - } - apollo_encoder::Selection::InlineFragment(f) - }, - ), - ) as Box> - } - } -} - impl From for OperationKind { // Spec: https://spec.graphql.org/draft/#OperationType fn from(operation_type: ast::OperationType) -> Self { From 606dcbd06b3bc535579a7ac6e20848e11ac20aea Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 6 Dec 2022 14:59:39 +0100 Subject: [PATCH 14/22] add comments --- apollo-router/src/services/execution_service.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apollo-router/src/services/execution_service.rs b/apollo-router/src/services/execution_service.rs index bb499e3982..b9c24e110a 100644 --- a/apollo-router/src/services/execution_service.rs +++ b/apollo-router/src/services/execution_service.rs @@ -143,6 +143,8 @@ where (Some(response_path), Some(response_data)) => { let mut sub_responses = Vec::new(); response_data.select_values_and_paths(response_path, |path, value| { + // if the deferred path points to an array, split it into multiple subresponses + // because the root must be an object if let Value::Array(array) = value { let mut parent = path.clone(); for (i, value) in array.iter().enumerate() { @@ -158,6 +160,7 @@ where let incremental = sub_responses .into_iter() .filter_map(move |(path, data)| { + // filter errors that match the path of this incremental response let errors = response .errors .iter() @@ -168,6 +171,9 @@ where .cloned() .collect::>(); + // an empty response should not be sent + // still, if there's an error or extension to show, we should + // send it if !data.is_null() || !errors.is_empty() || !response.extensions.is_empty() From 3a1bc19cf2d6af954342a9f104a4f92167fd394d Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 6 Dec 2022 18:11:07 +0100 Subject: [PATCH 15/22] update router-bridge to fix CI builds --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index ddd0764f9d..914cc4bcd3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4079,7 +4079,7 @@ dependencies = [ [[package]] name = "router-bridge" version = "0.1.11" -source = "git+https://github.com/apollographql/federation-rs?branch=geal/test-2.2.2-rc0#657a5d4af4633a6aa31d92a599195ecdbc0609a5" +source = "git+https://github.com/apollographql/federation-rs?branch=geal/test-2.2.2-rc0#5451d7e6b6f9d6adf5f7dcb66cbe81a7af5736fe" dependencies = [ "anyhow", "async-channel", From e794955b4a8d28cca36a699d52e2ac7d6b098bb1 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 12 Dec 2022 11:03:59 +0100 Subject: [PATCH 16/22] update router-bridge --- Cargo.lock | 5 +++-- apollo-router/Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 914cc4bcd3..90a2fd8e2e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4078,8 +4078,9 @@ dependencies = [ [[package]] name = "router-bridge" -version = "0.1.11" -source = "git+https://github.com/apollographql/federation-rs?branch=geal/test-2.2.2-rc0#5451d7e6b6f9d6adf5f7dcb66cbe81a7af5736fe" +version = "0.1.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "855a1971da25bf89dcdf00ce91ce2accbb9abfe247f040fc4064098685960f17" dependencies = [ "anyhow", "async-channel", diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml index a308e93f43..96625a20c9 100644 --- a/apollo-router/Cargo.toml +++ b/apollo-router/Cargo.toml @@ -153,7 +153,7 @@ reqwest = { version = "0.11.13", default-features = false, features = [ "json", "stream", ] } -router-bridge = { git = "https://github.com/apollographql/federation-rs", branch = "geal/test-2.2.2-rc0" } +router-bridge = "0.1.12" rust-embed="6.4.2" schemars = { version = "0.8.11", features = ["url"] } shellexpand = "2.1.2" From b5457751de1345eb3f49784d331406c3b88680cf Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Thu, 1 Dec 2022 11:07:20 +0100 Subject: [PATCH 17/22] fix: Return root when parts of a query with deferred fragment Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- NEXT_CHANGELOG.md | 25 ++++++ ...ce__tests__root_typename_with_defer-2.snap | 42 ++++++++++ ...vice__tests__root_typename_with_defer.snap | 26 +++++++ .../src/services/supergraph_service.rs | 76 +++++++++++++++++++ apollo-router/src/spec/query.rs | 21 +++-- apollo-router/src/spec/selection.rs | 4 + 6 files changed, 189 insertions(+), 5 deletions(-) create mode 100644 apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__root_typename_with_defer-2.snap create mode 100644 apollo-router/src/services/snapshots/apollo_router__services__supergraph_service__tests__root_typename_with_defer.snap diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index e1d6bfab1b..fb79aede9e 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -112,6 +112,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..03056f2420 --- /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: stream.next_response().await.unwrap() +--- +{ + "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 07846fd21a..4f77b10cc5 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -779,6 +779,82 @@ 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 query_reconstruction() { let schema = r#"schema diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index 73e08e9e6e..7cb88b1c6c 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -123,6 +123,18 @@ 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 = + self.operations.get(0).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, @@ -211,9 +223,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 +259,7 @@ impl Query { .collect::, SpecError>>()?; Ok(Query { - string, + string: query, fragments, operations, subselections: HashMap::new(), @@ -1066,7 +1077,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 11e67e36db..65568c91b2 100644 --- a/apollo-router/src/spec/selection.rs +++ b/apollo-router/src/spec/selection.rs @@ -314,6 +314,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 parse_skip(directive: &ast::Directive) -> Option { From 643b547d2a0af0e87786ef3181f577d18c6b4a47 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Thu, 1 Dec 2022 15:45:42 +0100 Subject: [PATCH 18/22] move code in the right method Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- apollo-router/src/spec/query.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index 7cb88b1c6c..027b3cc0ca 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -123,18 +123,6 @@ 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 = - self.operations.get(0).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, @@ -848,6 +836,17 @@ impl Query { } } } + // 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 = self.operations.get(0).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()); + } Ok(()) } From 716051604b64ac75e4dec21ba9c68d663809396f Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Fri, 2 Dec 2022 10:50:33 +0100 Subject: [PATCH 19/22] tests: fix test for root typename with defer Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- ...supergraph_service__tests__root_typename_with_defer.snap | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 index 03056f2420..058305dd59 100644 --- 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 @@ -1,10 +1,9 @@ --- source: apollo-router/src/services/supergraph_service.rs -expression: stream.next_response().await.unwrap() +expression: res --- { "data": { - "__typename": "Query", "currentUser": { "activeOrganization": { "id": "0", @@ -20,7 +19,8 @@ expression: stream.next_response().await.unwrap() } ] } - } + }, + "__typename": "Query" }, "hasNext": true } From ad1d0754cb4e31952b39732c81401823b778ae82 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Fri, 2 Dec 2022 16:11:52 +0100 Subject: [PATCH 20/22] tests: only keep the test Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- apollo-router/src/spec/query.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index 027b3cc0ca..998a3098bd 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -836,17 +836,6 @@ impl Query { } } } - // 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 = self.operations.get(0).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()); - } Ok(()) } From 0d0d38540b1bd02383fed5cd6f75a3622895748f Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Mon, 12 Dec 2022 12:10:07 +0100 Subject: [PATCH 21/22] fix: root typename with defer Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- apollo-router/src/spec/query.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index 998a3098bd..027b3cc0ca 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -836,6 +836,17 @@ impl Query { } } } + // 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 = self.operations.get(0).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()); + } Ok(()) } From 1b679eeb12fb630d515ae0750966a6e144eb1125 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Mon, 12 Dec 2022 15:58:46 +0100 Subject: [PATCH 22/22] tests: add test with typename and defer Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- ...vice__tests__root_typename_with_defer.snap | 4 +- .../src/services/supergraph_service.rs | 58 +++++++++++++++++++ apollo-router/src/spec/query.rs | 28 ++++----- 3 files changed, 75 insertions(+), 15 deletions(-) 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 index 058305dd59..3640cf4d26 100644 --- 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 @@ -4,6 +4,7 @@ expression: res --- { "data": { + "__typename": "Query", "currentUser": { "activeOrganization": { "id": "0", @@ -19,8 +20,7 @@ expression: res } ] } - }, - "__typename": "Query" + } }, "hasNext": true } diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index 4f77b10cc5..a52b766349 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -855,6 +855,64 @@ mod tests { 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 027b3cc0ca..17f4c8ec86 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() { @@ -836,17 +849,6 @@ impl Query { } } } - // 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 = self.operations.get(0).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()); - } Ok(()) }