From 8b8c8b9618e27656662ae96545f02664d3bffca8 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 15 Nov 2022 18:47:25 +0100 Subject: [PATCH 01/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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 3286b82dd4bc0f5d0c5ee4ecd9a7309f728af2e8 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 12 Dec 2022 12:00:46 +0100 Subject: [PATCH 17/17] changelog --- NEXT_CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index e1d6bfab1b..5b167be786 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -118,6 +118,12 @@ When we drop Telemetry we spawn a thread to perform the global opentelemetry tra By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/2191 +### Reconstruct deferred queries with knowledge about fragments ([Issue #2105](https://github.com/apollographql/router/issues/2105)) + +When we are using `@defer`, response formatting must apply on a subset of the query (primary or deferred), that is reconstructed from information provided by the query planner: a path into the response and a subselection. Previously, that path did not include information on fragment application, which resulted in query reconstruction issues if `@defer` was used under a fragment application on an interface. + +By [@Geal](https://github.com/geal) in https://github.com/apollographql/router/pull/2109 + ## 🛠 Maintenance ### improve plugin registration predictability ([PR #2181](https://github.com/apollographql/router/pull/2181))