Fix __typename inside router response

.changesets/fix_fix_typename.md

@@ -0,0 +1,22 @@
+### Fix variout edge cases for `__typename` field ([PR #6009](https://github.com/apollographql/router/pull/6009))
+
+The router now correctly handles the `__typename` field used on operation root types, even when the subgraph's root type has a name that differs from the supergraph's root type.
+
+For example, in query like this:
+```graphql
+{
+  ...RootFragment
+}
+
+fragment RootFragment on Query {
+  __typename
+  me {
+    name
+  }
+}
+```
+Even if the subgraph's root type returns a `__typename` that differs from `Query`, the router will still use `Query` as the value of the `__typename` field.
+
+This change also includes fixes for other edge cases related to the handling of `__typename` fields. For a detailed technical description of the edge cases that were fixed, please see [this description](https://github.com/apollographql/router/pull/6009#issue-2529717207).
+
+By [@IvanGoncharov](https://github.com/IvanGoncharov) in https://github.com/apollographql/router/pull/6009

apollo-router/src/query_planner/execution.rs

@@ -311,6 +311,8 @@ impl PlanNode {
                             let _ = primary_sender.send((value.clone(), errors.clone()));
                         } else {
                             let _ = primary_sender.send((value.clone(), errors.clone()));
+                            // primary response should be an empty object
+                            value.deep_merge(Value::Object(Default::default()));
                         }
                     }
                     .instrument(tracing::info_span!(

apollo-router/src/services/supergraph/tests.rs

@@ -1154,13 +1154,31 @@ async fn root_typename_with_defer_and_empty_first_response() {
         .await
         .unwrap();
 
+    let query = r#"
+        query {
+            ...OnlyTypename
+            ... @defer {
+                currentUser {
+                    activeOrganization {
+                        id
+                        suborga {
+                            id
+                            name
+                        }
+                    }
+                }
+            }
+        }
+
+        fragment OnlyTypename on Query {
+          __typename
+        }
+    "#;
     let request = supergraph::Request::fake_builder()
-            .context(defer_context())
-            .query(
-                "query { __typename ... @defer { currentUser { activeOrganization { id  suborga { id name } } } } }",
-            )
-            .build()
-            .unwrap();
+        .context(defer_context())
+        .query(query)
+        .build()
+        .unwrap();
 
     let mut stream = service.oneshot(request).await.unwrap();
     let res = stream.next_response().await.unwrap();

apollo-router/src/spec/query.rs

@@ -21,6 +21,7 @@ use self::change::QueryHashVisitor;
 use self::subselections::BooleanValues;
 use self::subselections::SubSelectionKey;
 use self::subselections::SubSelectionValue;
+use super::Fragment;
 use crate::error::FetchError;
 use crate::graphql::Error;
 use crate::graphql::Request;
@@ -146,18 +147,6 @@ impl Query {
                                 errors: Vec::new(),
                                 nullified: Vec::new(),
                             };
-                            // Detect if root __typename is asked in the original query (the qp doesn't put root __typename in subselections)
-                            // cf https://github.com/apollographql/router/issues/1677
-                            let operation_kind_if_root_typename =
-                                original_operation.and_then(|op| {
-                                    op.selection_set
-                                        .iter()
-                                        .any(|f| f.is_typename_field())
-                                        .then(|| *op.kind())
-                                });
-                            if let Some(operation_kind) = operation_kind_if_root_typename {
-                                output.insert(TYPENAME, operation_kind.default_type_name().into());
-                            }
 
                             response.data = Some(
                                 match self.apply_root_selection_set(
@@ -239,31 +228,15 @@ impl Query {
                 }
             }
             Some(Value::Null) => {
-                // Detect if root __typename is asked in the original query (the qp doesn't put root __typename in subselections)
-                // cf https://github.com/apollographql/router/issues/1677
-                let operation_kind_if_root_typename = original_operation.and_then(|op| {
-                    op.selection_set
-                        .iter()
-                        .any(|f| f.is_typename_field())
-                        .then(|| *op.kind())
-                });
-                response.data = match operation_kind_if_root_typename {
-                    Some(operation_kind) => {
-                        let mut output = Object::default();
-                        output.insert(TYPENAME, operation_kind.default_type_name().into());
-                        Some(output.into())
-                    }
-                    None => Some(Value::default()),
-                };
-
+                response.data = Some(Value::Null);
                 return vec![];
             }
             _ => {
                 failfast_debug!("invalid type for data in response. data: {:#?}", data);
             }
         }
 
-        response.data = Some(Value::default());
+        response.data = Some(Value::Null);
 
         vec![]
     }
@@ -571,19 +544,13 @@ impl Query {
                             .and_then(|s| apollo_compiler::ast::NamedType::new(s).ok())
                             .map(apollo_compiler::ast::Type::Named);
 
-                        let current_type = if parameters
-                            .schema
-                            .get_interface(field_type.inner_named_type())
-                            .is_some()
-                            || parameters
-                                .schema
-                                .get_union(field_type.inner_named_type())
-                                .is_some()
-                        {
-                            typename.as_ref().unwrap_or(field_type)
-                        } else {
-                            field_type
-                        };
+                        let current_type =
+                            match parameters.schema.types.get(field_type.inner_named_type()) {
+                                Some(ExtendedType::Interface(..) | ExtendedType::Union(..)) => {
+                                    typename.as_ref().unwrap_or(field_type)
+                                }
+                                _ => field_type,
+                            };
 
                         if self
                             .apply_selection_set(
@@ -640,21 +607,18 @@ impl Query {
                     }
 
                     if name.as_str() == TYPENAME {
-                        let input_value = input
-                            .get(field_name.as_str())
-                            .cloned()
-                            .filter(|v| v.is_string())
-                            .unwrap_or_else(|| {
-                                Value::String(ByteString::from(
-                                    current_type.inner_named_type().as_str().to_owned(),
-                                ))
+                        let object_type = parameters
+                            .schema
+                            .get_object(current_type.inner_named_type())
+                            .or_else(|| {
+                                let input_value = input.get(field_name.as_str())?.as_str()?;
+                                parameters.schema.get_object(input_value)
                             });
-                        if let Some(input_str) = input_value.as_str() {
-                            if parameters.schema.get_object(input_str).is_some() {
-                                output.insert((*field_name).clone(), input_value);
-                            } else {
-                                return Err(InvalidValue);
-                            }
+
+                        if let Some(object_type) = object_type {
+                            output.insert((*field_name).clone(), object_type.name.as_str().into());
+                        } else {
+                            return Err(InvalidValue);
                         }
                         continue;
                     }
@@ -751,11 +715,15 @@ impl Query {
                         continue;
                     }
 
-                    if let Some(fragment) = self.fragments.get(name) {
+                    if let Some(Fragment {
+                        type_condition,
+                        selection_set,
+                    }) = self.fragments.get(name)
+                    {
                         let is_apply = current_type.inner_named_type().as_str()
-                            == fragment.type_condition.as_str()
+                            == type_condition.as_str()
                             || parameters.schema.is_subtype(
-                                &fragment.type_condition,
+                                type_condition,
                                 current_type.inner_named_type().as_str(),
                             );
 
@@ -768,7 +736,7 @@ impl Query {
                             }
 
                             self.apply_selection_set(
-                                &fragment.selection_set,
+                                selection_set,
                                 parameters,
                                 input,
                                 output,
@@ -811,7 +779,12 @@ impl Query {
 
                     let field_name = alias.as_ref().unwrap_or(name);
                     let field_name_str = field_name.as_str();
-                    if let Some(input_value) = input.get_mut(field_name_str) {
+
+                    if name.as_str() == TYPENAME {
+                        if !output.contains_key(field_name_str) {
+                            output.insert(field_name.clone(), Value::String(root_type_name.into()));
+                        }
+                    } else if let Some(input_value) = input.get_mut(field_name_str) {
                         // if there's already a value for that key in the output it means either:
                         // - the value is a scalar and was moved into output using take(), replacing
                         // the input value with Null
@@ -837,10 +810,6 @@ impl Query {
                         );
                         path.pop();
                         res?
-                    } else if name.as_str() == TYPENAME {
-                        if !output.contains_key(field_name_str) {
-                            output.insert(field_name.clone(), Value::String(root_type_name.into()));
-                        }
                     } else if field_type.is_non_null() {
                         parameters.errors.push(Error {
                             message: format!(
@@ -861,25 +830,25 @@ impl Query {
                     include_skip,
                     ..
                 } => {
-                    // top level objects will not provide a __typename field
-                    if type_condition.as_str() != root_type_name {
-                        return Err(InvalidValue);
-                    }
-
                     if include_skip.should_skip(parameters.variables) {
                         continue;
                     }
 
-                    self.apply_selection_set(
-                        selection_set,
-                        parameters,
-                        input,
-                        output,
-                        path,
-                        // FIXME: use `ast::Name` everywhere so fallible conversion isn’t needed
-                        #[allow(clippy::unwrap_used)]
-                        &FieldType::new_named(type_condition.try_into().unwrap()).0,
-                    )?;
+                    // check if the fragment matches the input type directly, and if not, check if the
+                    // input type is a subtype of the fragment's type condition (interface, union)
+                    let is_apply = (root_type_name == type_condition.as_str())
+                        || parameters.schema.is_subtype(type_condition, root_type_name);
+
+                    if is_apply {
+                        self.apply_root_selection_set(
+                            root_type_name,
+                            selection_set,
+                            parameters,
+                            input,
+                            output,
+                            path,
+                        )?;
+                    }
                 }
                 Selection::FragmentSpread {
                     name,
@@ -892,30 +861,26 @@ impl Query {
                         continue;
                     }
 
-                    if let Some(fragment) = self.fragments.get(name) {
-                        let is_apply = {
-                            // check if the fragment matches the input type directly, and if not, check if the
-                            // input type is a subtype of the fragment's type condition (interface, union)
-                            root_type_name == fragment.type_condition.as_str()
-                                || parameters
-                                    .schema
-                                    .is_subtype(&fragment.type_condition, root_type_name)
-                        };
+                    if let Some(Fragment {
+                        type_condition,
+                        selection_set,
+                    }) = self.fragments.get(name)
+                    {
+                        // check if the fragment matches the input type directly, and if not, check if the
+                        // input type is a subtype of the fragment's type condition (interface, union)
+                        let is_apply = (root_type_name == type_condition.as_str())
+                            || parameters.schema.is_subtype(type_condition, root_type_name);
 
-                        if !is_apply {
-                            return Err(InvalidValue);
+                        if is_apply {
+                            self.apply_root_selection_set(
+                                root_type_name,
+                                selection_set,
+                                parameters,
+                                input,
+                                output,
+                                path,
+                            )?;
                         }
-
-                        self.apply_selection_set(
-                            &fragment.selection_set,
-                            parameters,
-                            input,
-                            output,
-                            path,
-                            // FIXME: use `ast::Name` everywhere so fallible conversion isn’t needed
-                            #[allow(clippy::unwrap_used)]
-                            &FieldType::new_named(root_type_name.try_into().unwrap()).0,
-                        )?;
                     } else {
                         // the fragment should have been already checked with the schema
                         failfast_debug!("missing fragment named: {}", name);

apollo-router/src/spec/selection.rs

@@ -11,7 +11,6 @@ use crate::spec::query::DeferStats;
 use crate::spec::FieldType;
 use crate::spec::Schema;
 use crate::spec::SpecError;
-use crate::spec::TYPENAME;
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub(crate) enum Selection {
@@ -190,10 +189,6 @@ impl Selection {
         })
     }
 
-    pub(crate) fn is_typename_field(&self) -> bool {
-        matches!(self, Selection::Field {name, ..} if name.as_str() == TYPENAME)
-    }
-
     pub(crate) fn contains_error_path(&self, path: &[PathElement], fragments: &Fragments) -> bool {
         match (path.first(), self) {
             (None, _) => true,