Fix QP bug querying some fields with @interfaceObject (#2362)

When an interface field is requested only for some implementation, and
the query "transit" through a subgraph using `@interfaceObject`, then
the query planning code was not recognizing early enough that the field
was queried on an implementation type and not the interface itself, and
this lead to a later assertion error.

This commit adds a reproduction test, and fix the issue by simply making
sure, when we build the query plan "paths", to not use an interface field
when it's an implementation field that is queried.

.changeset/tidy-cherries-fly.md

@@ -0,0 +1,7 @@
+---
+"@apollo/query-graphs": patch
+"@apollo/query-planner": patch
+---
+
+Fix assertion errors thrown by the query planner when querying fields for a specific interface implementation in some cases where `@interfaceObject` is involved
+

query-graphs-js/src/graphPath.ts

@@ -2130,6 +2130,12 @@ function advanceWithOperation<V extends Vertex>(
         );
         return { options: pathAsOptions(fieldPath) };
       case 'InterfaceType':
+        // Due to @interfaceObject, we could be in a case where the field asked is not on the interface but
+        // rather on one of it's implementation. This can happen if we just entered the subgraph on an interface @key
+        // and coming from an @interfaceObject. In that case, we'll skip checking for an interface direct edge and
+        // simply cast into that implementation below.
+        const fieldIsOfAnImplementation = field.parent.name !== currentType.name;
+
         // First, we check if there is a direct edge from the interface (which only happens if we're in a subgraph that knows all of the
         // implementations of that interface globally and all of them resolve the field).
         // If there is one, then we have 2 options:
@@ -2141,7 +2147,7 @@ function advanceWithOperation<V extends Vertex>(
         // - either type-exploding cannot work unless taking the interface edge also do (the `anImplementationIsEntityWithFieldShareable`)
         // - or that type-exploding cannot be more efficient than the direct path (when no @provides are involved; if a provide is involved
         //   in one of the implementation, then type-exploding may lead to a shorter overall plan thanks to that @provides)
-        const itfEdge = nextEdgeForField(path, operation);
+        const itfEdge = fieldIsOfAnImplementation ? undefined : nextEdgeForField(path, operation);
         let itfPath: OpGraphPath<V> | undefined = undefined;
         let directPathOverrideTypeExplosion = false;
         if (itfEdge) {
@@ -2174,12 +2180,11 @@ function advanceWithOperation<V extends Vertex>(
         // - the most common is that it's a field of the interface that is queried, and
         //   so we should type-explode because either didn't had a direct edge, or @provides
         //   makes it potentially worthwile to check with type explosion.
-        // - but we could also have the case where the field queried is actually of one
-        //   of the implementation of the interface: this happens if we just entered the
-        //   subgraph on an interface @key. In that case, we only want to consider that one
+        // - but, as mentionned earlier, we could be in the case where the field queried is actually of one
+        //   of the implementation of the interface. In that case, we only want to consider that one
         //   implementation.
         let implementations: readonly ObjectType[];
-        if (field.parent.name !== currentType.name) {
+        if (fieldIsOfAnImplementation) {
           assert(
             isObjectType(field.parent) && path.tailPossibleRuntimeTypes().some((t) => t.name === field.parent.name),
             () => `${field.coordinate} requested on ${currentType}, but ${field.parent} is not an implementation`

query-planner-js/src/__tests__/buildPlan.interfaceObject.test.ts

@@ -312,6 +312,52 @@ describe('basic @key on interface/@interfaceObject handling', () => {
     expect(rewrite.path).toEqual(['... on A', '__typename']);
     expect(rewrite.setValueTo).toBe('I');
   });
+
+  test('handles query of an interface field (that is not on the `@interfaceObject`) for a specific implementation when query starts on the @interfaceObject', () => {
+    // Here, we start on S2, but `x` is only in S1. Further, while `x` is on the `I` interface, we only query it for `A`.
+    const operation = operationFromDocument(api, gql`
+      {
+        iFromS2 {
+          ... on A {
+            x
+          }
+        }
+      }
+    `);
+
+    const plan = queryPlanner.buildQueryPlan(operation);
+    expect(plan).toMatchInlineSnapshot(`
+      QueryPlan {
+        Sequence {
+          Fetch(service: "S2") {
+            {
+              iFromS2 {
+                __typename
+                id
+              }
+            }
+          },
+          Flatten(path: "iFromS2") {
+            Fetch(service: "S1") {
+              {
+                ... on I {
+                  __typename
+                  id
+                }
+              } =>
+              {
+                ... on I {
+                  ... on A {
+                    x
+                  }
+                }
+              }
+            },
+          },
+        },
+      }
+    `);
+  });
 });
 
 it('avoids buffering @interfaceObject results that may have to filtered with lists', () => {