From c39272006251037fbc2ae9a7eda36bfbe4e104d6 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt <lukas.kalbertodt@gmail.com> Date: Wed, 9 Dec 2020 13:43:36 +0100 Subject: [PATCH 1/4] Apply type condition in inline fragments This fixes a panic when using inline fragments with interfaces (#815) --- juniper/src/types/async_await.rs | 42 +++++++++++++++++--------------- juniper/src/types/base.rs | 28 ++++++++++++--------- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/juniper/src/types/async_await.rs b/juniper/src/types/async_await.rs index 395faf5ef..6979e15ab 100644 --- a/juniper/src/types/async_await.rs +++ b/juniper/src/types/async_await.rs @@ -323,26 +323,30 @@ where ); if let Some(ref type_condition) = fragment.type_condition { - let sub_result = instance - .resolve_into_type_async( - info, - type_condition.item, - Some(&fragment.selection_set[..]), - &sub_exec, - ) - .await; - - if let Ok(Value::Object(obj)) = sub_result { - for (k, v) in obj { - async_values.push(AsyncValueFuture::InlineFragment1(async move { - AsyncValue::Field(AsyncField { - name: k, - value: Some(v), - }) - })); + // Check whether the type matches the type condition. + let concrete_type_name = instance.concrete_type_name(sub_exec.context(), info); + if type_condition.item == concrete_type_name { + let sub_result = instance + .resolve_into_type_async( + info, + type_condition.item, + Some(&fragment.selection_set[..]), + &sub_exec, + ) + .await; + + if let Ok(Value::Object(obj)) = sub_result { + for (k, v) in obj { + async_values.push(AsyncValueFuture::InlineFragment1(async move { + AsyncValue::Field(AsyncField { + name: k, + value: Some(v), + }) + })); + } + } else if let Err(e) = sub_result { + sub_exec.push_error_at(e, *start_pos); } - } else if let Err(e) = sub_result { - sub_exec.push_error_at(e, *start_pos); } } else { async_values.push(AsyncValueFuture::InlineFragment2(async move { diff --git a/juniper/src/types/base.rs b/juniper/src/types/base.rs index 1b395d440..d25f7441d 100644 --- a/juniper/src/types/base.rs +++ b/juniper/src/types/base.rs @@ -532,19 +532,23 @@ where ); if let Some(ref type_condition) = fragment.type_condition { - let sub_result = instance.resolve_into_type( - info, - type_condition.item, - Some(&fragment.selection_set[..]), - &sub_exec, - ); - - if let Ok(Value::Object(object)) = sub_result { - for (k, v) in object { - merge_key_into(result, &k, v); + // Check whether the type matches the type condition. + let concrete_type_name = instance.concrete_type_name(sub_exec.context(), info); + if type_condition.item == concrete_type_name { + let sub_result = instance.resolve_into_type( + info, + type_condition.item, + Some(&fragment.selection_set[..]), + &sub_exec, + ); + + if let Ok(Value::Object(object)) = sub_result { + for (k, v) in object { + merge_key_into(result, &k, v); + } + } else if let Err(e) = sub_result { + sub_exec.push_error_at(e, *start_pos); } - } else if let Err(e) = sub_result { - sub_exec.push_error_at(e, *start_pos); } } else if !resolve_selection_set_into( instance, From 1fcea248ef602a6b1f43b2d3377ddd252eb385bd Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt <lukas.kalbertodt@gmail.com> Date: Wed, 9 Dec 2020 16:30:25 +0100 Subject: [PATCH 2/4] Add `GraphQLValue::concrete_type_name` implementation to `RootNode` `type_name` was already forwarded to `QueryT`, so I guess `concrete_type_name` should be forwarded as well. `RootNode` is an "object" anyway and those should implement the method. --- juniper/src/schema/schema.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/juniper/src/schema/schema.rs b/juniper/src/schema/schema.rs index 62f03cc51..261da6422 100644 --- a/juniper/src/schema/schema.rs +++ b/juniper/src/schema/schema.rs @@ -51,6 +51,10 @@ where QueryT::name(info) } + fn concrete_type_name(&self, context: &Self::Context, info: &Self::TypeInfo) -> String { + self.query_type.concrete_type_name(context, info) + } + fn resolve_field( &self, info: &Self::TypeInfo, From 4c9071c036611a32011b86c3180d55d1d107a7a6 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt <lukas.kalbertodt@gmail.com> Date: Wed, 9 Dec 2020 16:31:14 +0100 Subject: [PATCH 3/4] Add test regarding inline fragments with type condition on interfaces --- juniper/src/tests/query_tests.rs | 66 ++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/juniper/src/tests/query_tests.rs b/juniper/src/tests/query_tests.rs index d671c5bf6..f5b7757dd 100644 --- a/juniper/src/tests/query_tests.rs +++ b/juniper/src/tests/query_tests.rs @@ -731,3 +731,69 @@ async fn test_object_typename() { )) ); } + +#[tokio::test] +async fn interface_inline_fragment_friends() { + let doc = r#"{ + human(id: "1002") { + friends { + name + ... on Human { homePlanet } + ... on Droid { primaryFunction } + } + } + }"#; + let database = Database::new(); + let schema = RootNode::new( + Query, + EmptyMutation::<Database>::new(), + EmptySubscription::<Database>::new(), + ); + + assert_eq!( + crate::execute(doc, None, &schema, &Variables::new(), &database).await, + Ok(( + Value::object( + vec![( + "human", + Value::object( + vec![( + "friends", + Value::list(vec![ + Value::object( + vec![ + ("name", Value::scalar("Luke Skywalker")), + ("homePlanet", Value::scalar("Tatooine")), + ] + .into_iter() + .collect(), + ), + Value::object( + vec![ + ("name", Value::scalar("Leia Organa")), + ("homePlanet", Value::scalar("Alderaan")), + ] + .into_iter() + .collect(), + ), + Value::object( + vec![ + ("name", Value::scalar("R2-D2")), + ("primaryFunction", Value::scalar("Astromech")), + ] + .into_iter() + .collect(), + ), + ]), + )] + .into_iter() + .collect(), + ), + )] + .into_iter() + .collect() + ), + vec![] + )) + ); +} From 937903a1176dd47e19394feb0950fc269c8e7548 Mon Sep 17 00:00:00 2001 From: tyranron <tyranron@gmail.com> Date: Wed, 9 Dec 2020 19:53:00 +0200 Subject: [PATCH 4/4] Simplify value definition in test --- juniper/src/tests/query_tests.rs | 50 ++++++-------------------------- 1 file changed, 9 insertions(+), 41 deletions(-) diff --git a/juniper/src/tests/query_tests.rs b/juniper/src/tests/query_tests.rs index f5b7757dd..32d79c1ca 100644 --- a/juniper/src/tests/query_tests.rs +++ b/juniper/src/tests/query_tests.rs @@ -1,6 +1,7 @@ use crate::{ ast::InputValue, executor::Variables, + graphql_value, schema::model::RootNode, tests::fixtures::starwars::schema::{Database, Query}, types::scalars::{EmptyMutation, EmptySubscription}, @@ -753,47 +754,14 @@ async fn interface_inline_fragment_friends() { assert_eq!( crate::execute(doc, None, &schema, &Variables::new(), &database).await, Ok(( - Value::object( - vec![( - "human", - Value::object( - vec![( - "friends", - Value::list(vec![ - Value::object( - vec![ - ("name", Value::scalar("Luke Skywalker")), - ("homePlanet", Value::scalar("Tatooine")), - ] - .into_iter() - .collect(), - ), - Value::object( - vec![ - ("name", Value::scalar("Leia Organa")), - ("homePlanet", Value::scalar("Alderaan")), - ] - .into_iter() - .collect(), - ), - Value::object( - vec![ - ("name", Value::scalar("R2-D2")), - ("primaryFunction", Value::scalar("Astromech")), - ] - .into_iter() - .collect(), - ), - ]), - )] - .into_iter() - .collect(), - ), - )] - .into_iter() - .collect() - ), - vec![] + graphql_value!({"human": { + "friends": [ + {"name": "Luke Skywalker", "homePlanet": "Tatooine"}, + {"name": "Leia Organa", "homePlanet": "Alderaan"}, + {"name": "R2-D2", "primaryFunction": "Astromech"}, + ], + }}), + vec![], )) ); }