From db097cbbc4d67cd7905590ba4c69c54633b02ff6 Mon Sep 17 00:00:00 2001
From: dariuszkuc <9501705+dariuszkuc@users.noreply.github.com>
Date: Thu, 22 Aug 2024 00:27:12 -0500
Subject: [PATCH] fix(federation): remove redundant typename for inline
 fragments

When adding `__typename` field for abstract types, we need to check for inline fragment type condition and not the parent selection set. If we have an inline fragment without type condition and it is pointing to and abstract type, then the required `__typename` field should have already been added to its parent field.
---
 apollo-federation/src/operation/mod.rs        | 14 ++++-
 .../query_plan/build_query_plan_tests.rs      | 40 ++++++++++++++
 ...e_fragments_without_type_condition.graphql | 53 +++++++++++++++++++
 3 files changed, 106 insertions(+), 1 deletion(-)
 create mode 100644 apollo-federation/tests/query_plan/supergraphs/redundant_typename_for_inline_fragments_without_type_condition.graphql

diff --git a/apollo-federation/src/operation/mod.rs b/apollo-federation/src/operation/mod.rs
index 71b4aece8b..dca1d7522d 100644
--- a/apollo-federation/src/operation/mod.rs
+++ b/apollo-federation/src/operation/mod.rs
@@ -895,7 +895,19 @@ impl Selection {
     }
 
     fn sub_selection_type_position(&self) -> Option<CompositeTypeDefinitionPosition> {
-        Some(self.selection_set()?.type_position.clone())
+        match self {
+            Selection::Field(field_selection) => field_selection
+                .selection_set
+                .as_ref()
+                .map(|s| s.type_position.clone()),
+            Selection::FragmentSpread(fragment_selection) => {
+                Some(fragment_selection.spread.type_condition_position.clone())
+            }
+            Selection::InlineFragment(inline_fragment_selection) => inline_fragment_selection
+                .inline_fragment
+                .type_condition_position
+                .clone(),
+        }
     }
 
     pub(crate) fn conditions(&self) -> Result<Conditions, FederationError> {
diff --git a/apollo-federation/tests/query_plan/build_query_plan_tests.rs b/apollo-federation/tests/query_plan/build_query_plan_tests.rs
index a5a8769c2a..4062311f5b 100644
--- a/apollo-federation/tests/query_plan/build_query_plan_tests.rs
+++ b/apollo-federation/tests/query_plan/build_query_plan_tests.rs
@@ -914,3 +914,43 @@ fn test_merging_fetches_do_not_create_cycle_in_fetch_dependency_graph() {
     "###
     );
 }
+
+#[test]
+fn redundant_typename_for_inline_fragments_without_type_condition() {
+    let planner = planner!(
+        Subgraph1: r#"
+          type Query {
+            products: [Product]
+          }
+          interface Product {
+            name: String
+          }
+        "#,
+    );
+    assert_plan!(
+        &planner,
+        r#"
+          {
+            products {
+              ... @skip(if: false) {
+                name
+              }
+            }
+          }
+        "#,
+        @r###"
+        QueryPlan {
+          Fetch(service: "Subgraph1") {
+            {
+              products {
+                __typename
+                ... @skip(if: false) {
+                  name
+                }
+              }
+            }
+          },
+        }
+        "###
+    );
+}
diff --git a/apollo-federation/tests/query_plan/supergraphs/redundant_typename_for_inline_fragments_without_type_condition.graphql b/apollo-federation/tests/query_plan/supergraphs/redundant_typename_for_inline_fragments_without_type_condition.graphql
new file mode 100644
index 0000000000..0b9050d640
--- /dev/null
+++ b/apollo-federation/tests/query_plan/supergraphs/redundant_typename_for_inline_fragments_without_type_condition.graphql
@@ -0,0 +1,53 @@
+# Composed from subgraphs with hash: d267b3f18f7736e1a3e39a951696c318d1f46a34
+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
+
+scalar join__FieldSet
+
+enum join__Graph {
+  SUBGRAPH1 @join__graph(name: "Subgraph1", 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
+}
+
+interface Product
+  @join__type(graph: SUBGRAPH1)
+{
+  name: String
+}
+
+type Query
+  @join__type(graph: SUBGRAPH1)
+{
+  products: [Product]
+}