fix(federation): updated GraphPath to use a custom MaxHeap, instead o…

…f BinaryHeap (#6132)

apollo-federation/src/query_graph/graph_path.rs

@@ -1,5 +1,4 @@
 use std::cmp::Ordering;
-use std::collections::BinaryHeap;
 use std::fmt::Display;
 use std::fmt::Formatter;
 use std::fmt::Write;
@@ -818,6 +817,39 @@ pub(crate) struct ClosedBranch(pub(crate) Vec<Arc<ClosedPath>>);
 #[derive(Debug, serde::Serialize)]
 pub(crate) struct OpenBranch(pub(crate) Vec<SimultaneousPathsWithLazyIndirectPaths>);
 
+// A drop-in replacement for `BinaryHeap`, but behaves more like JS QP's `popMin` method.
+struct MaxHeap<T>
+where
+    T: Ord,
+{
+    items: Vec<T>,
+}
+
+impl<T> MaxHeap<T>
+where
+    T: Ord,
+{
+    fn new() -> Self {
+        Self { items: Vec::new() }
+    }
+
+    fn push(&mut self, item: T) {
+        self.items.push(item);
+    }
+
+    fn pop(&mut self) -> Option<T> {
+        // PORT_NOTE: JS QP returns the max item, but favors the first inserted one if there are
+        //            multiple maximum items.
+        // Note: `position_max` returns the last of the equally maximum items. Thus, we use
+        //       `position_min_by` by reversing the ordering.
+        let pos = self.items.iter().position_min_by(|a, b| b.cmp(a));
+        let Some(pos) = pos else {
+            return None;
+        };
+        Some(self.items.remove(pos))
+    }
+}
+
 impl<TTrigger, TEdge> GraphPath<TTrigger, TEdge>
 where
     TTrigger: Eq + Hash + std::fmt::Debug,
@@ -1458,7 +1490,7 @@ where
         // that means it's important we try the smallest paths first. That is, if we could in theory
         // have path A -> B and A -> C -> B, and we can do B -> D, then we want to keep A -> B -> D,
         // not A -> C -> B -> D.
-        let mut heap: BinaryHeap<HeapElement<TTrigger, TEdge>> = BinaryHeap::new();
+        let mut heap: MaxHeap<HeapElement<TTrigger, TEdge>> = MaxHeap::new();
         heap.push(HeapElement(self.clone()));
 
         while let Some(HeapElement(to_advance)) = heap.pop() {

apollo-federation/tests/query_plan/build_query_plan_tests/interface_object.rs

@@ -826,3 +826,133 @@ fn it_handles_interface_object_input_rewrites_when_cloning_dependency_graph() {
       "###
     );
 }
+
+#[test]
+fn test_interface_object_advance_with_non_collecting_and_type_preserving_transitions_ordering() {
+    let planner = planner!(
+        S1: r#"
+            type A @key(fields: "id") {
+                id: ID!
+            }
+
+            type Query {
+                test: A
+            }
+        "#,
+        S2: r#"
+            type A @key(fields: "id") {
+                id: ID!
+            }
+        "#,
+        S3: r#"
+            type A @key(fields: "id") {
+                id: ID!
+            }
+        "#,
+        S4: r#"
+            type A @key(fields: "id") {
+                id: ID!
+            }
+        "#,
+        Y1: r#"
+            interface I {
+                id: ID!
+            }
+
+            type A implements I @key(fields: "id") @key(fields: "alt_id { id }") {
+                id: ID!
+                alt_id: AltID!
+            }
+
+            type AltID {
+                id: ID!
+            }
+        "#,
+        Y2: r#"
+            interface I {
+                id: ID!
+            }
+
+            type A implements I @key(fields: "id") @key(fields: "alt_id { id }") {
+                id: ID!
+                alt_id: AltID!
+            }
+
+            type AltID {
+                id: ID!
+            }
+        "#,
+        Z: r#"
+            type I @interfaceObject @key(fields: "alt_id { id }") {
+                alt_id: AltID!
+                data: String!
+            }
+
+            type AltID {
+                id: ID!
+            }
+        "#,
+    );
+    assert_plan!(
+        &planner,
+        r#"
+            {
+                test {
+                    data
+                }
+            }
+        "#,
+
+        // Make sure we fetch S1 -> Y1 -> Z, not S1 -> Y2 -> Z.
+        // That's following JS QP's behavior.
+        @r###"
+    QueryPlan {
+      Sequence {
+        Fetch(service: "S1") {
+          {
+            test {
+              __typename
+              id
+            }
+          }
+        },
+        Flatten(path: "test") {
+          Fetch(service: "Y1") {
+            {
+              ... on A {
+                __typename
+                id
+              }
+            } =>
+            {
+              ... on A {
+                __typename
+                alt_id {
+                  id
+                }
+              }
+            }
+          },
+        },
+        Flatten(path: "test") {
+          Fetch(service: "Z") {
+            {
+              ... on A {
+                __typename
+                alt_id {
+                  id
+                }
+              }
+            } =>
+            {
+              ... on I {
+                data
+              }
+            }
+          },
+        },
+      },
+    }
+    "###
+    );
+}

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

@@ -0,0 +1,98 @@
+# Composed from subgraphs with hash: 415e22ad50245441330e67dc6637cf09714a4831
+schema
+  @link(url: "https://specs.apollo.dev/link/v1.0")
+  @link(url: "https://specs.apollo.dev/join/v0.4", 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) 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
+
+type A implements I
+  @join__implements(graph: Y1, interface: "I")
+  @join__implements(graph: Y2, interface: "I")
+  @join__type(graph: S1, key: "id")
+  @join__type(graph: S2, key: "id")
+  @join__type(graph: S3, key: "id")
+  @join__type(graph: S4, key: "id")
+  @join__type(graph: Y1, key: "id")
+  @join__type(graph: Y1, key: "alt_id { id }")
+  @join__type(graph: Y2, key: "id")
+  @join__type(graph: Y2, key: "alt_id { id }")
+{
+  id: ID!
+  alt_id: AltID! @join__field(graph: Y1) @join__field(graph: Y2)
+  data: String! @join__field
+}
+
+type AltID
+  @join__type(graph: Y1)
+  @join__type(graph: Y2)
+  @join__type(graph: Z)
+{
+  id: ID!
+}
+
+interface I
+  @join__type(graph: Y1)
+  @join__type(graph: Y2)
+  @join__type(graph: Z, key: "alt_id { id }", isInterfaceObject: true)
+{
+  id: ID! @join__field(graph: Y1) @join__field(graph: Y2)
+  alt_id: AltID! @join__field(graph: Z)
+  data: String! @join__field(graph: Z)
+}
+
+scalar join__DirectiveArguments
+
+scalar join__FieldSet
+
+enum join__Graph {
+  S1 @join__graph(name: "S1", url: "none")
+  S2 @join__graph(name: "S2", url: "none")
+  S3 @join__graph(name: "S3", url: "none")
+  S4 @join__graph(name: "S4", url: "none")
+  Y1 @join__graph(name: "Y1", url: "none")
+  Y2 @join__graph(name: "Y2", url: "none")
+  Z @join__graph(name: "Z", 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: S1)
+  @join__type(graph: S2)
+  @join__type(graph: S3)
+  @join__type(graph: S4)
+  @join__type(graph: Y1)
+  @join__type(graph: Y2)
+  @join__type(graph: Z)
+{
+  test: A @join__field(graph: S1)
+}