fix(federation): fixed a bug where cached node costs were not reset a…

…fter modification (#5870)

apollo-federation/src/query_plan/fetch_dependency_graph.rs

@@ -2051,7 +2051,7 @@ impl FetchDependencyGraph {
 
         if path.is_empty() {
             mutable_node
-                .selection_set
+                .selection_set_mut()
                 .add_selections(&merged.selection_set.selection_set)?;
         } else {
             // The merged nodes might have some @include/@skip at top-level that are already part of the path. If so,
@@ -2061,7 +2061,7 @@ impl FetchDependencyGraph {
                 &path.conditional_directives(),
             )?;
             mutable_node
-                .selection_set
+                .selection_set_mut()
                 .add_at_path(path, Some(&Arc::new(merged_selection_set)))?;
         }
 
@@ -2361,9 +2361,9 @@ impl FetchDependencyGraphNode {
     }
 
     fn remove_inputs_from_selection(&mut self) -> Result<(), FederationError> {
-        let fetch_selection_set = &mut self.selection_set;
         if let Some(inputs) = &mut self.inputs {
             self.cached_cost = None;
+            let fetch_selection_set = &mut self.selection_set;
             for (_, selection) in &inputs.selection_sets_per_parent_type {
                 fetch_selection_set.selection_set =
                     Arc::new(fetch_selection_set.selection_set.minus(selection)?);

apollo-federation/tests/query_plan/build_query_plan_tests.rs

@@ -954,3 +954,128 @@ fn redundant_typename_for_inline_fragments_without_type_condition() {
         "###
     );
 }
+
+#[test]
+fn test_merging_fetches_reset_cached_costs() {
+    // This is a test for ROUTER-553.
+    let planner = planner!(
+      A: r#"
+            type Query {
+                start: S @shareable
+            }
+
+            type S @key(fields: "id") {
+                id: ID!
+                u: U @shareable
+            }
+
+            type U @key(fields: "id") {
+                id: ID!
+            }
+        "#,
+      B: r#"
+            type Query {
+                start: S @shareable
+            }
+
+            type S @key(fields: "id") {
+                id: ID!
+            }
+        "#,
+      C: r#"
+            type S @key(fields: "id") {
+                id: ID!
+                x: X
+                a: String!
+            }
+
+            type X {
+                t: T
+            }
+
+            type T {
+                u: U @shareable
+            }
+
+            type U @key(fields: "id") {
+                id: ID!
+                b: String
+            }
+        "#,
+    );
+    assert_plan!(
+        &planner,
+        r#"{
+            start {
+                u {
+                    b
+                }
+                a
+                x {
+                    t {
+                        u {
+                        id
+                        }
+                    }
+                }
+            }
+        }"#,
+        @r###"
+    QueryPlan {
+      Sequence {
+        Fetch(service: "A") {
+          {
+            start {
+              __typename
+              u {
+                __typename
+                id
+              }
+              id
+            }
+          }
+        },
+        Parallel {
+          Flatten(path: "start") {
+            Fetch(service: "C") {
+              {
+                ... on S {
+                  __typename
+                  id
+                }
+              } =>
+              {
+                ... on S {
+                  a
+                  x {
+                    t {
+                      u {
+                        id
+                      }
+                    }
+                  }
+                }
+              }
+            },
+          },
+          Flatten(path: "start.u") {
+            Fetch(service: "C") {
+              {
+                ... on U {
+                  __typename
+                  id
+                }
+              } =>
+              {
+                ... on U {
+                  b
+                }
+              }
+            },
+          },
+        },
+      },
+    }
+    "###
+    );
+}

apollo-federation/tests/query_plan/supergraphs/test_merging_fetches_reset_cached_costs.graphql

@@ -0,0 +1,82 @@
+# Composed from subgraphs with hash: 30529f3ff76bc917d12a637ede6e78ce39e7bca8
+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 {
+  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 Query
+  @join__type(graph: A)
+  @join__type(graph: B)
+  @join__type(graph: C)
+{
+  start: S @join__field(graph: A) @join__field(graph: B)
+}
+
+type S
+  @join__type(graph: A, key: "id")
+  @join__type(graph: B, key: "id")
+  @join__type(graph: C, key: "id")
+{
+  id: ID!
+  u: U @join__field(graph: A)
+  x: X @join__field(graph: C)
+  a: String! @join__field(graph: C)
+}
+
+type T
+  @join__type(graph: C)
+{
+  u: U
+}
+
+type U
+  @join__type(graph: A, key: "id")
+  @join__type(graph: C, key: "id")
+{
+  id: ID!
+  b: String @join__field(graph: C)
+}
+
+type X
+  @join__type(graph: C)
+{
+  t: T
+}