fix(federation/stitch): avoid recursively type-merging in case of mul…

…tiple keys (#6573)

* fix(federation/stitch): avoid recursively type-merging in case of multiple keys

* Setup chrome

* Try fixing the test

.changeset/eleven-schools-build.md

@@ -0,0 +1,70 @@
+---
+'@graphql-tools/federation': patch
+'@graphql-tools/delegate': patch
+'@graphql-tools/stitch': patch
+---
+
+When there are two services like below then the following query senty, the gateway tries to fetch `id` as an extra field because it considers `id` might be needed while this is not correct.
+This patch avoids any extra calls, and forwards the query as is to the 2nd service.
+
+```graphql
+query {
+  viewer {
+    booksContainer(input: $input) {
+      edges {
+        cursor
+        node {
+          source {
+            # Book(upc=)
+            upc
+          }
+        }
+      }
+      pageInfo {
+        endCursor
+      }
+    }
+  }
+}
+```
+
+```graphql
+type Book @key(fields: "id") @key(fields: "upc") {
+  id: ID!
+  upc: ID!
+}
+```
+
+```graphql
+type BookContainer { # the type that is used in a collection
+  id: ID!
+  # ... other stuff here
+  source: Book!
+}
+
+type Book @key(fields: "upc") {
+  upc: ID!
+}
+
+type Query {
+  viewer: Viewer
+}
+
+type Viewer {
+  booksContainer: BooksContainerResult
+}
+
+type BooksContainerResult {
+  edges: [BooksContainerEdge!]!
+  pageInfo: PageInfo!
+}
+
+type BooksContainerEdge {
+  node: BookContainer!
+  cursor: String!
+}
+
+type PageInfo {
+  endCursor: String
+}
+```

.github/workflows/tests.yml

@@ -148,7 +148,9 @@ jobs:
       - name: Setup env
         uses: the-guild-org/shared-config/setup@main
         with:
-          nodeVersion: 18
+          nodeVersion: 22
+      - name: Setup Chrome
+        uses: browser-actions/setup-chrome@v1
       - name: Build Packages
         run: yarn build
       - name: Test

packages/delegate/src/extractUnavailableFields.ts

@@ -112,6 +112,12 @@ export const extractUnavailableFieldsFromSelectionSet = memoize4(
               },
             });
           }
+        } else if (isObjectType(subFieldType) || isInterfaceType(subFieldType)) {
+          for (const subSelection of selection.selectionSet.selections) {
+            if (shouldAdd(subFieldType, subSelection as FieldNode)) {
+              unavailableSelections.push(subSelection);
+            }
+          }
         }
       }
     }

packages/federation/test/optimizations.test.ts

@@ -1,4 +1,6 @@
-import { GraphQLSchema, parse, versionInfo } from 'graphql';
+import { GraphQLSchema, parse, print, versionInfo } from 'graphql';
+import { kebabCase } from 'lodash';
+import { buildSubgraphSchema } from '@apollo/subgraph';
 import { createDefaultExecutor } from '@graphql-tools/delegate';
 import { normalizedExecutor } from '@graphql-tools/executor';
 import { ExecutionRequest, Executor } from '@graphql-tools/utils';
@@ -243,3 +245,197 @@ describe('awareness-of-other-fields', () => {
     });
   });
 });
+
+it('prevents recursively depending fields in case of multiple keys', async () => {
+  const books = [
+    {
+      __typename: 'Book',
+      id: '1',
+      upc: '1_upc',
+    },
+    {
+      __typename: 'Book',
+      id: '2',
+      upc: '2_upc',
+    },
+  ];
+  const booksSchema = buildSubgraphSchema({
+    typeDefs: parse(/* GraphQL */ `
+      type Book @key(fields: "id") @key(fields: "upc") {
+        id: ID!
+        upc: ID!
+      }
+    `),
+    resolvers: {
+      Book: {
+        __resolveReference(reference: { id?: string; upc?: string }) {
+          if (reference.id) {
+            return books.find(book => book.id === reference.id);
+          }
+          if (reference.upc) {
+            return books.find(book => book.upc === reference.upc);
+          }
+          return null;
+        },
+      },
+    },
+  });
+  const multiLocationMgmt = buildSubgraphSchema({
+    typeDefs: parse(/* GraphQL */ `
+      type BookContainer { # the type that is used in a collection
+        id: ID!
+        # ... other stuff here
+        source: Book!
+      }
+
+      type Book @key(fields: "upc") {
+        upc: ID!
+      }
+
+      type Query {
+        viewer: Viewer
+      }
+
+      type Viewer {
+        booksContainer: BooksContainerResult
+      }
+
+      type BooksContainerResult {
+        edges: [BooksContainerEdge!]!
+        pageInfo: PageInfo!
+      }
+
+      type BooksContainerEdge {
+        node: BookContainer!
+        cursor: String!
+      }
+
+      type PageInfo {
+        endCursor: String
+      }
+    `),
+    resolvers: {
+      Query: {
+        viewer() {
+          return {
+            booksContainer: {
+              edges: [
+                {
+                  node: {
+                    id: '1',
+                    source: {
+                      upc: '2_upc',
+                    },
+                  },
+                  cursor: '1',
+                },
+                {
+                  node: {
+                    id: '2',
+                    source: {
+                      upc: '2_upc',
+                    },
+                  },
+                  cursor: '2',
+                },
+              ],
+              pageInfo: {
+                endCursor: '2',
+              },
+            },
+          };
+        },
+      },
+    },
+  });
+
+  const { IntrospectAndCompose, LocalGraphQLDataSource } = await import('@apollo/gateway');
+  const introspectAndCompose = await new IntrospectAndCompose({
+    subgraphs: [
+      { name: 'books', url: 'books' },
+      { name: 'other-service', url: 'other-service' },
+    ],
+  }).initialize({
+    healthCheck() {
+      return Promise.resolve();
+    },
+    update() {},
+    getDataSource({ name }) {
+      switch (kebabCase(name)) {
+        case 'books':
+          return new LocalGraphQLDataSource(booksSchema);
+        case 'other-service':
+          return new LocalGraphQLDataSource(multiLocationMgmt);
+      }
+      throw new Error(`Unknown subgraph ${name}`);
+    },
+  });
+  const supergraphSdl = introspectAndCompose.supergraphSdl;
+  await introspectAndCompose.cleanup();
+  const subgraphCallsMap = {};
+  function createTracedExecutor(subgraphName: string, schema: GraphQLSchema): Executor {
+    const executor = createDefaultExecutor(schema);
+    return function tracedExecutor(execReq) {
+      subgraphCallsMap[subgraphName] ||= [];
+      subgraphCallsMap[subgraphName].push({
+        query: print(execReq.document),
+        variables: execReq.variables,
+      });
+      return executor(execReq);
+    };
+  }
+  const gwSchema = getStitchedSchemaFromSupergraphSdl({
+    supergraphSdl,
+    onSubschemaConfig(subschemaConfig) {
+      const subgraphName = kebabCase(subschemaConfig.name);
+      switch (subgraphName) {
+        case 'books':
+          subschemaConfig.executor = createTracedExecutor(subgraphName, booksSchema);
+          break;
+        case 'other-service':
+          subschemaConfig.executor = createTracedExecutor(subgraphName, multiLocationMgmt);
+          break;
+        default:
+          throw new Error(`Unknown subgraph ${subgraphName}`);
+      }
+    },
+  });
+  const result = await normalizedExecutor({
+    schema: gwSchema,
+    document: parse(/* GraphQL */ `
+      query {
+        viewer {
+          booksContainer(input: $input) {
+            edges {
+              cursor
+              node {
+                source {
+                  # Book(upc=)
+                  upc
+                }
+              }
+            }
+            pageInfo {
+              endCursor
+            }
+          }
+        }
+      }
+    `),
+  });
+  expect(result).toEqual({
+    data: {
+      viewer: {
+        booksContainer: {
+          edges: [
+            { cursor: '1', node: { source: { upc: '2_upc' } } },
+            { cursor: '2', node: { source: { upc: '2_upc' } } },
+          ],
+          pageInfo: { endCursor: '2' },
+        },
+      },
+    },
+  });
+  expect(Object.keys(subgraphCallsMap)).toEqual(['other-service']);
+  expect(subgraphCallsMap['other-service'].length).toBe(1);
+});

packages/loaders/url/tests/url-loader-browser.spec.ts

@@ -151,6 +151,7 @@ describe('[url-loader] webpack bundle compat', () => {
       });
       browser = await puppeteer.launch({
         // headless: false,
+        args: ['--no-sandbox', '--disable-setuid-sandbox'],
       });
       page = await browser.newPage();
       await page.goto(httpAddress);

packages/stitch/src/createDelegationPlanBuilder.ts

@@ -251,6 +251,7 @@ export function createDelegationPlanBuilder(mergedTypeInfo: MergedTypeInfo): Del
       fieldNodes,
       fragments,
       variableValues,
+      sourceSubschema,
     );
 
     if (!fieldsNotInSubschema.length) {

packages/stitch/src/getFieldsNotInSubschema.ts

@@ -2,13 +2,12 @@ import {
   FieldNode,
   FragmentDefinitionNode,
   GraphQLField,
-  GraphQLNamedOutputType,
   GraphQLObjectType,
   GraphQLSchema,
   isAbstractType,
   Kind,
 } from 'graphql';
-import { extractUnavailableFields, StitchingInfo } from '@graphql-tools/delegate';
+import { extractUnavailableFields, StitchingInfo, Subschema } from '@graphql-tools/delegate';
 import { collectSubFields } from '@graphql-tools/utils';
 
 export function getFieldsNotInSubschema(
@@ -19,6 +18,7 @@ export function getFieldsNotInSubschema(
   fieldNodes: FieldNode[],
   fragments: Record<string, FragmentDefinitionNode>,
   variableValues: Record<string, any>,
+  subschema: Subschema,
 ): Array<FieldNode> {
   let { fields: subFieldNodesByResponseKey, patches } = collectSubFields(
     schema,
@@ -99,23 +99,25 @@ export function getFieldsNotInSubschema(
 
   // TODO: Verify whether it is safe that extensions always exists.
   const fieldNodesByField = stitchingInfo?.fieldNodesByField;
-  const shouldAdd = (fieldType: GraphQLNamedOutputType, selection: FieldNode) =>
-    !fieldNodesByField?.[fieldType.name]?.[selection.name.value];
 
   const fields = subschemaType.getFields();
 
+  const fieldNodesByFieldForType = fieldNodesByField?.[gatewayType.name];
+
   for (const [, subFieldNodes] of subFieldNodesByResponseKey) {
+    let fieldNotInSchema = false;
     const fieldName = subFieldNodes[0].name.value;
-
     if (!fields[fieldName]) {
+      fieldNotInSchema = true;
       for (const subFieldNode of subFieldNodes) {
         fieldsNotInSchema.add(subFieldNode);
       }
     } else {
       const field = fields[fieldName];
       for (const subFieldNode of subFieldNodes) {
-        const unavailableFields = extractUnavailableFields(schema, field, subFieldNode, shouldAdd);
+        const unavailableFields = extractUnavailableFields(schema, field, subFieldNode, () => true);
         if (unavailableFields.length) {
+          fieldNotInSchema = true;
           fieldsNotInSchema.add({
             ...subFieldNode,
             selectionSet: {
@@ -126,10 +128,10 @@ export function getFieldsNotInSubschema(
         }
       }
     }
+    const isComputedField = subschema.merge?.[gatewayType.name]?.fields?.[fieldName]?.computed;
     let addedSubFieldNodes = false;
-    const fieldNodesByFieldForType = fieldNodesByField?.[gatewayType.name];
-    const visitedFieldNames = new Set<string>();
-    if (fieldNodesByFieldForType) {
+    if ((isComputedField || fieldNotInSchema) && fieldNodesByFieldForType) {
+      const visitedFieldNames = new Set<string>();
       addMissingRequiredFields({
         fieldName,
         fields,