Fix relative path logic when eliding subgraph jump for @fromContext (#3005)

This PR makes some changes to #2988, specifically:
- It fixes a bug where the relative path is computed incorrectly when a
subgraph jump is skipped/elided in the case of `@fromContext`.
- It changes the selection-set-to-renamer conversion logic to create
multiple inline fragments instead of introducing a new syntax, in the
case of a type condition on an abstract type. (This saves us from having
to update router to understand it.)
- It tweaks the iteration over the `GraphPath` in
`canSatisfyConditions()` to be cleaner/easier to understand.
- When I looked at that code again, I noticed it was strange that we
initialized `levelsInDataPath` at `1` even though we aren't guaranteed
that the last element in `GraphPath` is a field. The gist is that at the
first match, we know it's a field, so that ends up being okay. But I
think it's easier to understand code-wise if both `levelsInQueryPath`
and `levelsInDataPath` start off at 0, and every time we get a new
element (at the start of the `for` block) we increment the counts
accordingly.

Author: sachindshinde

.changeset/rare-grapes-dream.md

@@ -0,0 +1,5 @@
+---
+"@apollo/query-planner": patch
+---
+
+Fix relative path logic when eliding subgraph jumps for `@fromContext`

query-graphs-js/src/graphPath.ts

@@ -1909,10 +1909,14 @@ function canSatisfyConditions<TTrigger, V extends Vertex, TNullEdge extends null
     // if one of the conditions fails to satisfy, it's ok to bail
     let someSelectionUnsatisfied = false;
     for (const cxt of requiredContexts) {
-      let levelsInQueryPath = 1;
-      let levelsInDataPath = 1;
+      let levelsInQueryPath = 0;
+      let levelsInDataPath = 0;
       for (const [e, trigger] of [...path].reverse()) {
         const parentType = getFieldParentType(trigger);
+        levelsInQueryPath += 1;
+        if (parentType) {
+          levelsInDataPath += 1;
+        }
         if (e !== null && !contextMap.has(cxt.namedParameter) && !someSelectionUnsatisfied) {
           const matches = Array.from(cxt.typesWithContextSet).some(t => {
             if (parentType) {
@@ -1965,10 +1969,6 @@ function canSatisfyConditions<TTrigger, V extends Vertex, TNullEdge extends null
             }
           }
         }
-        levelsInQueryPath += 1;
-        if (parentType) {
-          levelsInDataPath += 1;
-        }
       }
     }
 

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

@@ -9478,7 +9478,12 @@ describe('@fromContext impacts on query planning', () => {
     expect((plan as any).node.nodes[1].node.contextRewrites).toEqual([
       {
         kind: 'KeyRenamer',
-        path: ['..', '... on A,B', 'prop'],
+        path: ['..', '... on A', 'prop'],
+        renameKeyTo: 'contextualArgument_1_0',
+      },
+      {
+        kind: 'KeyRenamer',
+        path: ['..', '... on B', 'prop'],
         renameKeyTo: 'contextualArgument_1_0',
       },
     ]);

query-planner-js/src/buildPlan.ts

@@ -1678,13 +1678,11 @@ function selectionSetAsKeyRenamers(selectionSet: SelectionSet, relPath: string[]
     if (selection.kind === 'FieldSelection') {
       // We always have at least one '..' in the relative path.
       if (relPath[relPath.length - 1] === '..') {
-        const runtimeTypes = 
-          possibleRuntimeTypes(selectionSet.parentType).map((t) => t.name).join(",");
-        return [{
+        return possibleRuntimeTypes(selectionSet.parentType).map((t) => ({
           kind: 'KeyRenamer',
-          path: [...relPath, `... on ${runtimeTypes}`, selection.element.name],
+          path: [...relPath, `... on ${t.name}`, selection.element.name],
           renameKeyTo: alias,
-        }];
+        }));
       } else {
         return [{
           kind: 'KeyRenamer',
@@ -4445,9 +4443,11 @@ function computeGroupsForTree(
           } else {
             // in this case we can just continue with the current group, but we need to add the context rewrites
             if (parameterToContext) {
+              const numFields = updated.path.inGroup().filter((e) => e.kind === 'Field').length;
               for (const [_, { selectionSet, relativePath, contextId, subgraphArgType }] of parameterToContext) {
+                const newRelativePath = relativePath.slice(0, relativePath.length - numFields);
                 updated.group.addInputContext(contextId, subgraphArgType);
-                const keyRenamers = selectionSetAsKeyRenamers(selectionSet, relativePath, contextId);
+                const keyRenamers = selectionSetAsKeyRenamers(selectionSet, newRelativePath, contextId);
                 for (const keyRenamer of keyRenamers) {
                   updated.group.addContextRenamer(keyRenamer);
                 }