Fix assertion error due to not ignoring some unsatisfiable branches (#…

…2486)

Code was added in #2449 which "optimize" selections by removing
unecessary inline fragments and branches that can't be satisfied (when
we cross type conditions so that the intersection of types leading here
is empty).

Unfortunately that code misses some cases of when a branch is
impossible, which leads to trying to build a selection set that is
invalid and this in turn triggers an assertion error.

.changeset/lovely-walls-sneeze.md

@@ -0,0 +1,7 @@
+---
+"@apollo/federation-internals": patch
+---
+
+Fix assertion error during query planning in some cases where queries has some unsatisfiable branches (a part of the
+query goes through type conditions that no runtime types satisfies).
+

internals-js/src/__tests__/operations.test.ts

@@ -477,7 +477,6 @@ describe('basic operations', () => {
   })
 });
 
-
 describe('MutableSelectionSet', () => {
   test('memoizer', () => {
     const schema = parseSchema(`
@@ -546,3 +545,60 @@ describe('MutableSelectionSet', () => {
     expect(sets).toStrictEqual(['{}', '{ t { v1 } }', '{ t { v1 v3 } }', '{ t { v1 v3 v2 } }', '{ t { v1 v3 v4 } }']);
   });
 });
+
+describe('unsatisfiable branches removal', () => {
+  const schema = parseSchema(`
+    type Query {
+      i: I
+      j: J
+    }
+
+    interface I {
+      a: Int
+      b: Int
+    }
+
+    interface J {
+      b: Int
+    }
+
+    type T1 implements I & J {
+      a: Int
+      b: Int
+      c: Int
+    }
+
+    type T2 implements I {
+      a: Int
+      b: Int
+      d: Int
+    }
+
+    type T3 implements J {
+      a: Int
+      b: Int
+      d: Int
+    }
+  `);
+
+  const withoutUnsatisfiableBranches = (op: string) => {
+    return parseOperation(schema, op).trimUnsatisfiableBranches().toString(false, false)
+  };
+
+
+  it.each([
+    '{ i { a } }',
+    '{ i { ... on T1 { a b c } } }',
+  ])('is identity if there is no unsatisfiable branches', (op) => {
+    expect(withoutUnsatisfiableBranches(op)).toBe(op);
+  });
+
+  it.each([
+    { input: '{ i { ... on I { a } } }', output: '{ i { a } }' },
+    { input: '{ i { ... on T1 { ... on I { a b } } } }', output: '{ i { ... on T1 { a b } } }' },
+    { input: '{ i { ... on I { a ... on T2 { d } } } }', output: '{ i { a ... on T2 { d } } }' },
+    { input: '{ i { ... on T2 { ... on I { a ... on J { b } } } } }', output: '{ i { ... on T2 { a } } }' },
+  ])('removes unsatisfiable branches', ({input, output}) => {
+    expect(withoutUnsatisfiableBranches(input)).toBe(output);
+  });
+});

internals-js/src/operations.ts

@@ -2272,10 +2272,12 @@ class InlineFragmentSelection extends FragmentSelection {
       // If the current type is an object, then we never need to keep the current fragment because:
       // - either the fragment is also an object, but we've eliminated the case where the 2 types are the same,
       //   so this is just an unsatisfiable branch.
-      // - or it's not an object, but then the current type is more precise and no poitn in "casting" to a
-      //   less precise interface/union.
+      // - or it's not an object, but then the current type is more precise and no point in "casting" to a
+      //   less precise interface/union. And if the current type is not even a valid runtime of said interface/union,
+      //   then we should completely ignore the branch (or, since we're eliminating `thisCondition`, we would be
+      //   building an invalid selection). 
       if (isObjectType(currentType)) {
-        if (isObjectType(thisCondition)) {
+        if (isObjectType(thisCondition) || !possibleRuntimeTypes(thisCondition).includes(currentType)) {
           return undefined;
         } else {
           const trimmed = this.selectionSet.trimUnsatisfiableBranches(currentType);
@@ -2340,7 +2342,6 @@ class InlineFragmentSelection extends FragmentSelection {
     return this.selectionSet === trimmedSelectionSet ? this : this.withUpdatedSelectionSet(trimmedSelectionSet);
   }
 
-
   expandAllFragments(): FragmentSelection {
     return this.mapToSelectionSet((s) => s.expandAllFragments());
   }