diff --git a/Cargo.lock b/Cargo.lock index f6da7eebff..bbddb2208e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -59,7 +59,7 @@ checksum = "ee10e43ae4a853c0a3591d4e2ada1719e553be18199d9da9d4a83f5927c2f5c7" [[package]] name = "apollo-parser" version = "0.1.0" -source = "git+https://github.com/apollographql/apollo-rs.git?rev=0e845624d8f93f9c2f228a456941f07007b60dfc#0e845624d8f93f9c2f228a456941f07007b60dfc" +source = "git+https://github.com/apollographql/apollo-rs.git?rev=14bb84337a8bacd5cd27d7d7df429936f104b63b#14bb84337a8bacd5cd27d7d7df429936f104b63b" dependencies = [ "rowan", ] diff --git a/crates/apollo-router-core/Cargo.toml b/crates/apollo-router-core/Cargo.toml index fdc7696e43..7639807608 100644 --- a/crates/apollo-router-core/Cargo.toml +++ b/crates/apollo-router-core/Cargo.toml @@ -12,7 +12,7 @@ license-file = "./LICENSE" failfast = [] [dependencies] -apollo-parser = { git = "https://github.com/apollographql/apollo-rs.git", rev = "0e845624d8f93f9c2f228a456941f07007b60dfc" } +apollo-parser = { git = "https://github.com/apollographql/apollo-rs.git", rev = "14bb84337a8bacd5cd27d7d7df429936f104b63b" } async-trait = "0.1.51" derivative = "2.2.0" displaydoc = "0.2" diff --git a/crates/apollo-router-core/src/error.rs b/crates/apollo-router-core/src/error.rs index f5d701c171..307a687d7b 100644 --- a/crates/apollo-router-core/src/error.rs +++ b/crates/apollo-router-core/src/error.rs @@ -179,10 +179,12 @@ impl From for QueryPlannerError { QueryPlannerError::JoinError(Arc::new(err)) } } -#[derive(Debug, Error)] + +/// Error in the schema. +#[derive(Debug, Error, Display)] pub enum SchemaError { - #[error(transparent)] + /// IO error: {0} IoError(#[from] std::io::Error), - #[error("Parsing error(s)")] + /// Parsing error(s). ParseErrors(Vec), } diff --git a/crates/apollo-router-core/src/federated.rs b/crates/apollo-router-core/src/federated.rs index 83f19ac164..d39d7cd3ab 100644 --- a/crates/apollo-router-core/src/federated.rs +++ b/crates/apollo-router-core/src/federated.rs @@ -110,7 +110,7 @@ impl Fetcher for FederatedGraph { let plan = { match query_planner .get( - request.query.to_owned(), + request.query.as_str().to_owned(), request.operation_name.to_owned(), Default::default(), ) @@ -165,7 +165,7 @@ impl Fetcher for FederatedGraph { Arc::clone(&response), &root, &plan, - request, + request.clone(), Arc::clone(&service_registry), Arc::clone(&schema), ) @@ -173,9 +173,14 @@ impl Fetcher for FederatedGraph { .await; // TODO: this is not great but there is no other way - Arc::try_unwrap(response) + let mut response = Arc::try_unwrap(response) .expect("todo: how to prove?") - .into_inner() + .into_inner(); + + tracing::debug_span!("format_response") + .in_scope(|| request.query.format_response(&mut response)); + + response } .with_current_subscriber(), ) @@ -243,11 +248,8 @@ fn execute<'a>( serde_json::to_string_pretty(&response.lock().await.data).unwrap(); tracing::trace!("New data:\n{}", received,); } - #[cfg_attr(feature = "failfast", allow(unreachable_code))] Err(err) => { - #[cfg(feature = "failfast")] - panic!("failfast: {}", err); - tracing::error!("Fetch error: {}", err); + failfast_error!("Fetch error: {}", err); response .lock() .await diff --git a/crates/apollo-router-core/src/json_ext.rs b/crates/apollo-router-core/src/json_ext.rs index 7c447fafba..0f72b39ad6 100644 --- a/crates/apollo-router-core/src/json_ext.rs +++ b/crates/apollo-router-core/src/json_ext.rs @@ -26,6 +26,11 @@ pub trait ValueExt { #[track_caller] fn deep_merge(&mut self, other: &Self); + /// Returns `true` if the values are equal and the objects are ordered the same. + /// + /// **Note:** this is recursive. + fn eq_and_ordered(&self, other: &Self) -> bool; + /// Returns `true` if the set is a subset of another, i.e., `other` contains at least all the /// values in `self`. #[track_caller] @@ -145,6 +150,46 @@ impl ValueExt for Value { } } + fn eq_and_ordered(&self, other: &Self) -> bool { + match (self, other) { + (Value::Object(a), Value::Object(b)) => { + let mut it_a = a.iter(); + let mut it_b = b.iter(); + + loop { + match (it_a.next(), it_b.next()) { + (Some(_), None) | (None, Some(_)) => break false, + (None, None) => break true, + (Some((field_a, value_a)), Some((field_b, value_b))) + if field_a == field_b && ValueExt::eq_and_ordered(value_a, value_b) => + { + continue + } + (Some(_), Some(_)) => break false, + } + } + } + (Value::Array(a), Value::Array(b)) => { + let mut it_a = a.iter(); + let mut it_b = b.iter(); + + loop { + match (it_a.next(), it_b.next()) { + (Some(_), None) | (None, Some(_)) => break false, + (None, None) => break true, + (Some(value_a), Some(value_b)) + if ValueExt::eq_and_ordered(value_a, value_b) => + { + continue + } + (Some(_), Some(_)) => break false, + } + } + } + (a, b) => a == b, + } + } + fn is_subset(&self, superset: &Value) -> bool { match (self, superset) { (Value::Object(subset), Value::Object(superset)) => { @@ -452,4 +497,23 @@ mod tests { json!({"obj":{"arr":[{"prop1":1},{"prop4":4}]}}), ); } + + #[test] + fn eq_and_ordered() { + // test not objects + assert!(json!([1, 2, 3]).eq_and_ordered(&json!([1, 2, 3]))); + assert!(!json!([1, 3, 2]).eq_and_ordered(&json!([1, 2, 3]))); + + // test objects not nested + assert!(json!({"foo":1,"bar":2}).eq_and_ordered(&json!({"foo":1,"bar":2}))); + assert!(!json!({"foo":1,"bar":2}).eq_and_ordered(&json!({"foo":1,"bar":3}))); + assert!(!json!({"foo":1,"bar":2}).eq_and_ordered(&json!({"foo":1,"bar":2,"baz":3}))); + assert!(!json!({"foo":1,"bar":2,"baz":3}).eq_and_ordered(&json!({"foo":1,"bar":2}))); + assert!(!json!({"bar":2,"foo":1}).eq_and_ordered(&json!({"foo":1,"bar":2}))); + + // test objects nested + assert!(json!({"baz":{"foo":1,"bar":2}}).eq_and_ordered(&json!({"baz":{"foo":1,"bar":2}}))); + assert!(!json!({"baz":{"bar":2,"foo":1}}).eq_and_ordered(&json!({"baz":{"foo":1,"bar":2}}))); + assert!(!json!([1,{"bar":2,"foo":1},2]).eq_and_ordered(&json!([1,{"foo":1,"bar":2},2]))); + } } diff --git a/crates/apollo-router-core/src/lib.rs b/crates/apollo-router-core/src/lib.rs index ec95a0a1fd..1794e3bcf7 100644 --- a/crates/apollo-router-core/src/lib.rs +++ b/crates/apollo-router-core/src/lib.rs @@ -1,3 +1,27 @@ +#![cfg_attr(feature = "failfast", allow(unreachable_code))] + +macro_rules! failfast_debug { + ($($tokens:tt)+) => {{ + tracing::debug!($($tokens)+); + #[cfg(feature = "failfast")] + panic!( + "failfast triggered. \ + Please remove the feature failfast if you don't want to see these panics" + ); + }}; +} + +macro_rules! failfast_error { + ($($tokens:tt)+) => {{ + tracing::error!($($tokens)+); + #[cfg(feature = "failfast")] + panic!( + "failfast triggered. \ + Please remove the feature failfast if you don't want to see these panics" + ); + }}; +} + mod error; mod federated; mod json_ext; diff --git a/crates/apollo-router-core/src/naive_introspection.rs b/crates/apollo-router-core/src/naive_introspection.rs index 2700e6a73f..233e731098 100644 --- a/crates/apollo-router-core/src/naive_introspection.rs +++ b/crates/apollo-router-core/src/naive_introspection.rs @@ -27,12 +27,12 @@ static KNOWN_INTROSPECTION_QUERIES: Lazy> = Lazy::new(|| { /// A cache containing our well known introspection queries. #[derive(Debug)] pub struct NaiveIntrospection { - cache: HashMap, + cache: HashMap, } impl NaiveIntrospection { #[cfg(test)] - pub fn from_cache(cache: HashMap) -> Self { + pub fn from_cache(cache: HashMap) -> Self { Self { cache } } @@ -53,7 +53,7 @@ impl NaiveIntrospection { .iter() .zip(responses) .filter_map(|(cache_key, response)| match response.into_result() { - Ok(introspection_value) => Some((cache_key.clone(), introspection_value)), + Ok(introspection_value) => Some((cache_key.into(), introspection_value)), Err(e) => { let errors = e .iter() @@ -84,10 +84,10 @@ mod naive_introspection_tests { #[test] fn test_plan() { - let query_to_test = "this is a test query".to_string(); + let query_to_test = "this is a test query"; let expected_data = serde_json::Value::Number(42.into()); - let cache = [(query_to_test.clone(), expected_data.clone())] + let cache = [(query_to_test.into(), expected_data.clone())] .iter() .cloned() .collect(); diff --git a/crates/apollo-router-core/src/request.rs b/crates/apollo-router-core/src/request.rs index 41d7203f83..ed2144535a 100644 --- a/crates/apollo-router-core/src/request.rs +++ b/crates/apollo-router-core/src/request.rs @@ -1,6 +1,8 @@ use crate::prelude::graphql::*; +use apollo_parser::ast; use derivative::Derivative; use serde::{Deserialize, Serialize}; +use std::collections::HashMap; use std::sync::Arc; use typed_builder::TypedBuilder; @@ -12,7 +14,7 @@ use typed_builder::TypedBuilder; #[derivative(Debug, PartialEq)] pub struct Request { /// The graphql query. - pub query: String, + pub query: Query, /// The optional graphql operation. #[serde(skip_serializing_if = "Option::is_none", default)] @@ -30,11 +32,197 @@ pub struct Request { pub extensions: Object, } +#[derive(Debug, Clone, Hash, PartialEq, Eq, Deserialize, Serialize)] +#[serde(transparent)] +pub struct Query { + string: String, +} + +impl Query { + /// Returns a reference to the underlying query string. + pub fn as_str(&self) -> &str { + self.string.as_str() + } + + /// Re-format the response value to match this query. + /// + /// This will discard unrequested fields and re-order the output to match the order of the + /// query. + pub fn format_response(&self, response: &mut Response) { + fn apply_selection_set( + selection_set: &ast::SelectionSet, + input: &mut Object, + output: &mut Object, + fragments: &HashMap, + ) { + for selection in selection_set.selections() { + match selection { + // Spec: https://spec.graphql.org/draft/#Field + ast::Selection::Field(field) => { + let name = field + .name() + .expect("the node Name is not optional in the spec; qed") + .text() + .to_string(); + let alias = field.alias().map(|x| x.name().unwrap().text().to_string()); + let name = alias.unwrap_or(name); + + if let Some(input_value) = input.remove(&name) { + if let Some(selection_set) = field.selection_set() { + match input_value { + Value::Object(mut input_object) => { + let mut output_object = Object::default(); + apply_selection_set( + &selection_set, + &mut input_object, + &mut output_object, + fragments, + ); + output.insert(name, output_object.into()); + } + Value::Array(input_array) => { + let output_array = input_array + .into_iter() + .enumerate() + .map(|(i, mut element)| { + if let Some(input_object) = element.as_object_mut() + { + let mut output_object = Object::default(); + apply_selection_set( + &selection_set, + input_object, + &mut output_object, + fragments, + ); + output_object.into() + } else { + failfast_debug!( + "Array element is not an object: {}[{}]", + name, + i, + ); + element + } + }) + .collect::(); + output.insert(name, output_array); + } + _ => { + output.insert(name.clone(), input_value); + failfast_debug!( + "Field is not an object nor an array of object: {}", + name, + ); + } + } + } else { + output.insert(name, input_value); + } + } else { + failfast_debug!("Missing field: {}", name); + } + } + // Spec: https://spec.graphql.org/draft/#InlineFragment + ast::Selection::InlineFragment(inline_fragment) => { + let selection_set = inline_fragment + .selection_set() + .expect("the node SelectionSet is not optional in the spec; qed"); + + apply_selection_set(&selection_set, input, output, fragments); + } + // Spec: https://spec.graphql.org/draft/#FragmentSpread + ast::Selection::FragmentSpread(fragment_spread) => { + let name = fragment_spread + .fragment_name() + .expect("the node FragmentName is not optional in the spec; qed") + .name() + .unwrap() + .text() + .to_string(); + + if let Some(selection_set) = fragments.get(&name) { + apply_selection_set(selection_set, input, output, fragments); + } else { + failfast_debug!("Missing fragment named: {}", name); + } + } + } + } + } + + fn fragments(document: &ast::Document) -> HashMap { + document + .definitions() + .filter_map(|definition| match definition { + // Spec: https://spec.graphql.org/draft/#FragmentDefinition + ast::Definition::FragmentDefinition(fragment_definition) => { + let name = fragment_definition + .fragment_name() + .expect("the node FragmentName is not optional in the spec; qed") + .name() + .unwrap() + .text() + .to_string(); + let selection_set = fragment_definition + .selection_set() + .expect("the node SelectionSet is not optional in the spec; qed"); + + Some((name, selection_set)) + } + _ => None, + }) + .collect() + } + + let parser = apollo_parser::Parser::new(self.as_str()); + let tree = parser.parse(); + + if !tree.errors().is_empty() { + let errors = tree + .errors() + .iter() + .map(|err| format!("{:?}", err)) + .collect::>(); + failfast_debug!("Parsing error(s): {}", errors.join(", ")); + return; + } + + let document = tree.document(); + let fragments = fragments(&document); + + for definition in document.definitions() { + // Spec: https://spec.graphql.org/draft/#sec-Language.Operations + if let ast::Definition::OperationDefinition(operation) = definition { + let selection_set = operation + .selection_set() + .expect("the node SelectionSet is not optional in the spec; qed"); + if let Some(data) = response.data.as_object_mut() { + let mut output = Object::default(); + apply_selection_set(&selection_set, data, &mut output, &fragments); + response.data = output.into(); + return; + } else { + failfast_debug!("Invalid type for data in response."); + } + } + } + + failfast_debug!("No suitable definition found. This is a bug."); + } +} + +impl> From for Query { + fn from(string: T) -> Self { + Query { + string: string.into(), + } + } +} + #[cfg(test)] mod tests { use super::*; use serde_json::json; - use std::sync::Arc; use test_env_log::test; #[test] @@ -109,4 +297,134 @@ mod tests { .build() ); } + + macro_rules! assert_eq_and_ordered { + ($a:expr, $b:expr $(,)?) => { + assert_eq!($a, $b,); + assert!( + $a.eq_and_ordered(&$b), + "assertion failed: objects are not ordered the same:\ + \n left: `{:?}`\n right: `{:?}`", + $a, + $b, + ); + }; + } + + #[test] + fn reformat_response_data_field() { + let query = Query::from( + r#"{ + foo + stuff{bar} + array{bar} + baz + alias:baz + alias_obj:baz_obj{bar} + alias_array:baz_array{bar} + }"#, + ); + let mut response = Response::builder() + .data(json! {{ + "foo": "1", + "stuff": {"bar": "2"}, + "array": [{"bar": "3", "baz": "4"}, {"bar": "5", "baz": "6"}], + "baz": "7", + "alias": "7", + "alias_obj": {"bar": "8"}, + "alias_array": [{"bar": "9", "baz": "10"}, {"bar": "11", "baz": "12"}], + "other": "13", + }}) + .build(); + query.format_response(&mut response); + assert_eq_and_ordered!( + response.data, + json! {{ + "foo": "1", + "stuff": { + "bar": "2", + }, + "array": [ + {"bar": "3"}, + {"bar": "5"}, + ], + "baz": "7", + "alias": "7", + "alias_obj": { + "bar": "8", + }, + "alias_array": [ + {"bar": "9"}, + {"bar": "11"}, + ], + }}, + ); + } + + #[test] + fn reformat_response_data_inline_fragment() { + let query = Query::from(r#"{... on Stuff { stuff{bar}}}"#); + let mut response = Response::builder() + .data(json! {{"stuff": {"bar": "2"}}}) + .build(); + query.format_response(&mut response); + assert_eq_and_ordered!( + response.data, + json! {{ + "stuff": { + "bar": "2", + }, + }}, + ); + } + + #[test] + fn reformat_response_data_fragment_spread() { + let query = + Query::from(r#"{...foo ...bar} fragment foo on Foo {foo} fragment bar on Bar {bar}"#); + let mut response = Response::builder() + .data(json! {{"foo": "1", "bar": "2"}}) + .build(); + query.format_response(&mut response); + assert_eq_and_ordered!( + response.data, + json! {{ + "foo": "1", + "bar": "2", + }}, + ); + } + + #[test] + fn reformat_response_data_best_effort() { + let query = Query::from(r#"{foo stuff{bar baz} ...fragment array{bar baz} other{bar}}"#); + let mut response = Response::builder() + .data(json! {{ + "foo": "1", + "stuff": {"baz": "2"}, + "array": [ + {"baz": "3"}, + "4", + {"bar": "5"}, + ], + "other": "6", + }}) + .build(); + query.format_response(&mut response); + assert_eq_and_ordered!( + response.data, + json! {{ + "foo": "1", + "stuff": { + "baz": "2", + }, + "array": [ + {"baz": "3"}, + "4", + {"bar": "5"}, + ], + "other": "6", + }}, + ); + } } diff --git a/crates/apollo-router/tests/integration_tests.rs b/crates/apollo-router/tests/integration_tests.rs index dc7c7574a8..fe726a64e5 100644 --- a/crates/apollo-router/tests/integration_tests.rs +++ b/crates/apollo-router/tests/integration_tests.rs @@ -26,15 +26,20 @@ macro_rules! assert_federated_response { let (mut actual, registry) = query_rust(request.clone()).await; let mut expected = query_node(request.clone()).await; - let actual = actual.next().await.unwrap(); + tracing::debug!("query:\n{}\n", request.query.as_str()); + let expected = expected.next().await.unwrap(); - eprintln!("actual: {}", to_string_pretty(&actual).unwrap()); - eprintln!("expected: {}", to_string_pretty(&expected).unwrap()); - // The current implementation does not cull extra properties that should not make is to the - // output yet, so we check that the nodejs implementation returns a subset of the - // output of the rust output. - assert!(expected.data.is_subset(&actual.data)); + assert!( + expected.data.is_object(), + "no response's data: please check that the subgraphs are running", + ); + + let actual = actual.next().await.unwrap(); + tracing::debug!("expected: {}", to_string_pretty(&expected).unwrap()); + tracing::debug!("actual: {}", to_string_pretty(&actual).unwrap()); + + assert!(expected.data.eq_and_ordered(&actual.data)); assert_eq!(registry.totals(), $service_requests); }; } @@ -42,7 +47,7 @@ macro_rules! assert_federated_response { #[test(tokio::test)] async fn basic_request() { assert_federated_response!( - r#"{ topProducts { name } }"#, + r#"{ topProducts { name name2:name } }"#, hashmap! { "products".to_string()=>1, },