From 5b9c2161353cc91fa8de62446d3e938bc6765666 Mon Sep 17 00:00:00 2001 From: Jeremy Lempereur Date: Mon, 11 Sep 2023 14:13:39 +0200 Subject: [PATCH] Fix: deal with interface inheritance when retrieving selectionset (#3793) Followup to #3718, this changeset makes sure we're able to generate the most concrete selection set for a given operation. This means finding the most concrete type we can when we're dealing with interfaces: - If InterfaceA implements InterfaceB, use InterfaceA as current_type to generate an inline fragment's selection set Given the following invariants: ```graphql interface OperationItemStuff implements OperationItem ``` For ```graphql fragment OperationItemFragment on OperationItem { ... on OperationItemStuff { stuff } } ``` The most concrete interface to generate fields for `OperationItemStuff` is not `OperationItem`, so we narrow down the selection to `OperationItemStuff`. The fixes for #3718 still apply, IE: Given the following invariants: ```graphql type Dog implements Animal ``` For ```graphql ...on Animal { id ...on Dog { name } } ``` The most concrete type to generate a selection set for `Dog` is not `Animal`, so we narrow down the selection to `Dog`. --- .../src/services/supergraph_service.rs | 203 ++++++++++++++++++ apollo-router/src/spec/schema.rs | 38 ++++ apollo-router/src/spec/selection.rs | 9 +- 3 files changed, 248 insertions(+), 2 deletions(-) diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index 9f14a693a0..198b29b8be 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -3103,4 +3103,207 @@ mod tests { ); insta::assert_json_snapshot!(with_reversed_fragments); } + + #[tokio::test] + async fn multiple_interface_types() { + let schema = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) { + query: Query + } + + directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE + + 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 + isInterfaceObject: Boolean! = false + ) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + + directive @join__unionMember( + graph: join__Graph! + member: String! + ) repeatable on UNION + + 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 | SCHEMA + + enum link__Purpose { + EXECUTION + SECURITY + } + + scalar join__FieldSet + scalar link__Import + + enum join__Graph { + GRAPH1 @join__graph(name: "graph1", url: "http://localhost:8080/graph1") + } + + type Query @join__type(graph: GRAPH1) { + root(id: ID!): Root @join__field(graph: GRAPH1) + } + + type Root @join__type(graph: GRAPH1, key: "id") { + id: ID! + operation(a: Int, b: Int): OperationResult! + } + + union OperationResult + @join__type(graph: GRAPH1) + @join__unionMember(graph: GRAPH1, member: "Operation") = + Operation + + type Operation @join__type(graph: GRAPH1) { + id: ID! + item: [OperationItem!]! + } + + interface OperationItem @join__type(graph: GRAPH1) { + type: OperationType! + } + + enum OperationType @join__type(graph: GRAPH1) { + ADD_ARGUMENT @join__enumValue(graph: GRAPH1) + } + + interface OperationItemRootType implements OperationItem + @join__implements(graph: GRAPH1, interface: "OperationItem") + @join__type(graph: GRAPH1) { + rootType: String! + type: OperationType! + } + + interface OperationItemStuff implements OperationItem + @join__implements(graph: GRAPH1, interface: "OperationItem") + @join__type(graph: GRAPH1) { + stuff: String! + type: OperationType! + } + + type OperationAddArgument implements OperationItem & OperationItemStuff & OperationItemValue + @join__implements(graph: GRAPH1, interface: "OperationItem") + @join__implements(graph: GRAPH1, interface: "OperationItemStuff") + @join__implements(graph: GRAPH1, interface: "OperationItemValue") + @join__type(graph: GRAPH1) { + stuff: String! + type: OperationType! + value: String! + } + + interface OperationItemValue implements OperationItem + @join__implements(graph: GRAPH1, interface: "OperationItem") + @join__type(graph: GRAPH1) { + type: OperationType! + value: String! + } + + type OperationRemoveSchemaRootOperation implements OperationItem & OperationItemRootType + @join__implements(graph: GRAPH1, interface: "OperationItem") + @join__implements(graph: GRAPH1, interface: "OperationItemRootType") + @join__type(graph: GRAPH1) { + rootType: String! + type: OperationType! + } + "#; + + let query = r#"fragment OperationItemFragment on OperationItem { + __typename + ... on OperationItemStuff { + __typename + stuff + } + ... on OperationItemRootType { + __typename + rootType + } + } + query MyQuery($id: ID!, $a: Int, $b: Int) { + root(id: $id) { + __typename + operation(a: $a, b: $b) { + __typename + ... on Operation { + __typename + item { + __typename + ...OperationItemFragment + ... on OperationItemStuff { + __typename + stuff + } + ... on OperationItemValue { + __typename + value + } + } + id + } + } + id + } + }"#; + + let subgraphs = MockedSubgraphs([ + // The response isn't interesting to us, + // we just need to make sure the query makes it through parsing and validation + ("graph1", MockSubgraph::builder().with_json( + serde_json::json!{{"query":"query MyQuery__graph1__0($id:ID!$a:Int$b:Int){root(id:$id){__typename operation(a:$a b:$b){__typename ...on Operation{__typename item{__typename ...on OperationItemStuff{__typename stuff}...on OperationItemRootType{__typename rootType}...on OperationItemValue{__typename value}}id}}id}}", "operationName": "MyQuery__graph1__0", "variables":{"id":"1234","a":1,"b":2}}}, + serde_json::json!{{"data": null }} + ).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_supergraph() + .await + .unwrap(); + + let request = supergraph::Request::fake_builder() + .context(defer_context()) + .query(query) + .variables( + serde_json_bytes::json! {{ "id": "1234", "a": 1, "b": 2}} + .as_object() + .unwrap() + .clone(), + ) + .build() + .unwrap(); + + let mut stream = service.clone().oneshot(request).await.unwrap(); + let response = stream.next_response().await.unwrap(); + assert_eq!(serde_json_bytes::Value::Null, response.data.unwrap()); + } } diff --git a/apollo-router/src/spec/schema.rs b/apollo-router/src/spec/schema.rs index 32e955b786..744e3d2cee 100644 --- a/apollo-router/src/spec/schema.rs +++ b/apollo-router/src/spec/schema.rs @@ -13,6 +13,7 @@ use http::Uri; use sha2::Digest; use sha2::Sha256; +use super::FieldType; use crate::configuration::GraphQLValidationMode; use crate::error::ParseErrors; use crate::error::SchemaError; @@ -159,6 +160,19 @@ impl Schema { .unwrap_or(false) } + pub(crate) fn is_implementation(&self, interface: &str, implementor: &str) -> bool { + self.type_system + .definitions + .interfaces + .get(interface) + .map(|interface| { + interface + .implements_interfaces() + .any(|i| i.interface() == implementor) + }) + .unwrap_or(false) + } + pub(crate) fn is_interface(&self, abstract_type: &str) -> bool { self.type_system .definitions @@ -166,6 +180,30 @@ impl Schema { .contains_key(abstract_type) } + // given two field, returns the one that implements the other, if applicable + pub(crate) fn most_precise<'f>( + &self, + a: &'f FieldType, + b: &'f FieldType, + ) -> Option<&'f FieldType> { + let typename_a = a.inner_type_name().unwrap_or_default(); + let typename_b = b.inner_type_name().unwrap_or_default(); + if typename_a == typename_b { + return Some(a); + } + if self.is_subtype(typename_a, typename_b) || self.is_implementation(typename_a, typename_b) + { + Some(b) + } else if self.is_subtype(typename_b, typename_a) + || self.is_implementation(typename_b, typename_a) + { + Some(a) + } else { + // No relationship between a and b + None + } + } + /// Return an iterator over subgraphs that yields the subgraph name and its URL. pub(crate) fn subgraphs(&self) -> impl Iterator { self.subgraphs.iter() diff --git a/apollo-router/src/spec/selection.rs b/apollo-router/src/spec/selection.rs index 5252a7f5d9..e1d6ee7386 100644 --- a/apollo-router/src/spec/selection.rs +++ b/apollo-router/src/spec/selection.rs @@ -164,12 +164,17 @@ impl Selection { schema.is_subtype( type_condition.as_str(), current_type.inner_type_name().unwrap_or("") - ) || + ) || schema.is_implementation( + type_condition.as_str(), + current_type.inner_type_name().unwrap_or("")) + || // if the current type and the type condition are both the same interface, it is still valid type_condition.as_str() == current_type.inner_type_name().unwrap_or("") ); - current_type + let relevant_type = schema.most_precise(current_type, &fragment_type); + debug_assert!(relevant_type.is_some()); + relevant_type.unwrap_or(&fragment_type) } else { &fragment_type };