Skip to content

Commit

Permalink
Fix bug during query planner construction where interface object type…
Browse files Browse the repository at this point in the history
…s are computed incorrectly
  • Loading branch information
sachindshinde committed Jun 14, 2024
1 parent 028b287 commit cb8b5f4
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 28 deletions.
45 changes: 22 additions & 23 deletions apollo-federation/src/query_plan/query_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,19 @@ use std::cell::Cell;
use std::num::NonZeroU32;
use std::sync::Arc;

use apollo_compiler::schema::ExtendedType;
use apollo_compiler::schema::Name;
use apollo_compiler::validation::Valid;
use apollo_compiler::ExecutableDocument;
use apollo_compiler::NodeStr;
use indexmap::IndexMap;
use indexmap::IndexSet;
use itertools::Itertools;
use petgraph::csr::NodeIndex;
use petgraph::stable_graph::IndexType;

use crate::error::FederationError;
use crate::error::SingleFederationError;
use crate::link::federation_spec_definition::FederationSpecDefinition;
use crate::link::federation_spec_definition::FEDERATION_INTERFACEOBJECT_DIRECTIVE_NAME_IN_SPEC;
use crate::link::spec::Identity;
use crate::operation::normalize_operation;
use crate::operation::NamedFragments;
use crate::operation::RebasedFragments;
Expand Down Expand Up @@ -219,34 +217,35 @@ impl QueryPlanner {
Some(true),
)?;

let metadata = supergraph_schema.metadata().unwrap();

let federation_link = metadata.for_identity(&Identity::federation_identity());
let interface_object_directive =
federation_link.map_or(FEDERATION_INTERFACEOBJECT_DIRECTIVE_NAME_IN_SPEC, |link| {
link.directive_name_in_schema(&FEDERATION_INTERFACEOBJECT_DIRECTIVE_NAME_IN_SPEC)
});

let is_interface_object =
|ty: &ExtendedType| ty.is_object() && ty.directives().has(&interface_object_directive);

let interface_types_with_interface_objects = supergraph
.schema
.get_types()
.filter_map(|position| match position {
TypeDefinitionPosition::Interface(interface_position) => Some(interface_position),
_ => None,
})
.filter(|position| {
query_graph.subgraphs().any(|(_name, schema)| {
schema
.schema()
.types
.get(&position.type_name)
.is_some_and(is_interface_object)
})
.map(|position| {
let is_interface_object = query_graph
.subgraphs()
.map(|(_name, schema)| {
let Some(position) = schema.try_get_type(position.type_name.clone()) else {
return Ok(false);
};
schema.is_interface_object_type(position)
})
.process_results(|mut iter| iter.any(|b| b))?;
Ok::<_, FederationError>((position, is_interface_object))
})
.collect::<IndexSet<_>>();
.process_results(|iter| {
iter.flat_map(|(position, is_interface_object)| {
if is_interface_object {
Some(position)
} else {
None
}
})
.collect::<IndexSet<_>>()
})?;

let is_inconsistent = |position: AbstractTypeDefinitionPosition| {
let mut sources = query_graph.subgraphs().filter_map(|(_name, subgraph)| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,6 @@ fn only_uses_an_interface_object_if_it_can() {
}

#[test]
#[should_panic(
expected = "Cannot add selection of field \"I.__typename\" to selection set of parent type \"I\" that is potentially an interface object type at runtime"
)]
// TODO: investigate this failure
// - Fails to rebase on an interface object type in a subgraph.
fn does_not_rely_on_an_interface_object_directly_for_typename() {
let planner = planner!(
S1: SUBGRAPH1,
Expand Down

0 comments on commit cb8b5f4

Please sign in to comment.