diff --git a/.changesets/fix_duckki_fed_678.md b/.changesets/fix_duckki_fed_678.md new file mode 100644 index 0000000000..6723b7cfd5 --- /dev/null +++ b/.changesets/fix_duckki_fed_678.md @@ -0,0 +1,5 @@ +### (Federation) Removed `RebaseError::InterfaceObjectTypename` variant ([PR #8109](https://github.com/apollographql/router/pull/8109)) + +Fixed an uncommon query planning error, "Cannot add selection of field `X` to selection set of parent type `Y` that is potentially an interface object type at runtime". Although fetching `__typename` selections from interface object types are unnecessary, it is difficult to avoid them in all cases and the effect of having those selections in query plans is benign. Thus, the error variant and the check for the error have been removed. + +By [@duckki](https://github.com/duckki) in https://github.com/apollographql/router/pull/8109 diff --git a/apollo-federation/src/operation/rebase.rs b/apollo-federation/src/operation/rebase.rs index a3f840c424..4e6ca427ef 100644 --- a/apollo-federation/src/operation/rebase.rs +++ b/apollo-federation/src/operation/rebase.rs @@ -3,8 +3,6 @@ //! Often, the change is between equivalent types from different schemas, but selections can also //! be rebased from one type to another in the same schema. -use itertools::Itertools; - use super::Field; use super::FieldSelection; use super::InlineFragment; @@ -80,13 +78,6 @@ pub(crate) enum RebaseError { field_position: crate::schema::position::FieldDefinitionPosition, parent_type: CompositeTypeDefinitionPosition, }, - #[error( - "Cannot add selection of field `{field_position}` to selection set of parent type `{parent_type}` that is potentially an interface object type at runtime" - )] - InterfaceObjectTypename { - field_position: crate::schema::position::FieldDefinitionPosition, - parent_type: CompositeTypeDefinitionPosition, - }, #[error("Cannot rebase composite field selection because its subselection is empty")] EmptySelectionSet, #[error( @@ -125,24 +116,10 @@ impl Field { } if self.name() == &TYPENAME_FIELD { - // TODO interface object info should be precomputed in QP constructor - return if schema - .possible_runtime_types(parent_type.clone())? - .iter() - .map(|t| schema.is_interface_object_type(t.clone().into())) - .process_results(|mut iter| iter.any(|b| b))? - { - Err(RebaseError::InterfaceObjectTypename { - field_position: self.field_position.clone(), - parent_type: parent_type.clone(), - } - .into()) - } else { - let mut updated_field = self.clone(); - updated_field.schema = schema.clone(); - updated_field.field_position = parent_type.introspection_typename_field(); - Ok(updated_field) - }; + let mut updated_field = self.clone(); + updated_field.schema = schema.clone(); + updated_field.field_position = parent_type.introspection_typename_field(); + return Ok(updated_field); } let field_from_parent = parent_type.field(self.name().clone())?; diff --git a/apollo-federation/tests/query_plan/build_query_plan_tests/requires.rs b/apollo-federation/tests/query_plan/build_query_plan_tests/requires.rs index f80bfb1523..a5bd26ec12 100644 --- a/apollo-federation/tests/query_plan/build_query_plan_tests/requires.rs +++ b/apollo-federation/tests/query_plan/build_query_plan_tests/requires.rs @@ -1839,3 +1839,93 @@ fn handles_requires_from_supergraph() { "### ); } + +#[test] +fn allows_post_requires_input_with_typename_on_interface_object_type() { + // This used to panic with an `InternalRebaseError(InterfaceObjectTypename)` error. + let planner = planner!( + A: r#" + type I @key(fields: "id") @interfaceObject { + id: ID! + } + + extend type Query { + start: I! + } + "#, + B: r#" + interface I @key(fields: "id") { + id: ID! + required: String + } + + type P implements I @key(fields: "id") { + id: ID! + required: String + } + "#, + C: r#" + type I @key(fields: "id") @interfaceObject { + id: ID! + required: String @external + data: String! @requires(fields: "required") + } + "#, + ); + assert_plan!( + &planner, + r#" + { + start { + data + } + } + "#, + @r###" + QueryPlan { + Sequence { + Fetch(service: "A") { + { + start { + __typename + id + } + } + }, + Flatten(path: "start") { + Fetch(service: "B") { + { + ... on I { + __typename + id + } + } => + { + ... on I { + __typename + required + } + } + }, + }, + Flatten(path: "start") { + Fetch(service: "C") { + { + ... on I { + __typename + required + id + } + } => + { + ... on I { + data + } + } + }, + }, + }, + } + "### + ); +} diff --git a/apollo-federation/tests/query_plan/supergraphs/allows_post_requires_input_with_typename_on_interface_object_type.graphql b/apollo-federation/tests/query_plan/supergraphs/allows_post_requires_input_with_typename_on_interface_object_type.graphql new file mode 100644 index 0000000000..7a82707af5 --- /dev/null +++ b/apollo-federation/tests/query_plan/supergraphs/allows_post_requires_input_with_typename_on_interface_object_type.graphql @@ -0,0 +1,83 @@ +# Composed from subgraphs with hash: 6a52228f357eb403fba553216a31a338bab93088 +schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.5", for: EXECUTION) +{ + query: Query +} + +directive @join__directive(graphs: [join__Graph!], name: String!, args: join__DirectiveArguments) repeatable on SCHEMA | OBJECT | INTERFACE | FIELD_DEFINITION + +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, overrideLabel: String, contextArguments: [join__ContextArgument!]) 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 + +interface I + @join__type(graph: A, key: "id", isInterfaceObject: true) + @join__type(graph: B, key: "id") + @join__type(graph: C, key: "id", isInterfaceObject: true) +{ + id: ID! + required: String @join__field(graph: B) @join__field(graph: C, external: true) + data: String! @join__field(graph: C, requires: "required") +} + +input join__ContextArgument { + name: String! + type: String! + context: String! + selection: join__FieldValue! +} + +scalar join__DirectiveArguments + +scalar join__FieldSet + +scalar join__FieldValue + +enum join__Graph { + A @join__graph(name: "A", url: "none") + B @join__graph(name: "B", url: "none") + C @join__graph(name: "C", url: "none") +} + +scalar link__Import + +enum link__Purpose { + """ + `SECURITY` features provide metadata necessary to securely resolve fields. + """ + SECURITY + + """ + `EXECUTION` features provide metadata necessary for operation execution. + """ + EXECUTION +} + +type P implements I + @join__implements(graph: B, interface: "I") + @join__type(graph: B, key: "id") +{ + id: ID! + required: String + data: String! @join__field +} + +type Query + @join__type(graph: A) + @join__type(graph: B) + @join__type(graph: C) +{ + start: I! @join__field(graph: A) +}