Gateway over-merging fields of unioned types

packages/apollo-gateway/src/FieldSet.ts

@@ -64,8 +64,25 @@ function groupBy<T, U>(keyFunction: (element: T) => U) {
   };
 }
 
-export const groupByResponseName = groupBy<Field, string>(field =>
-  getResponseName(field.fieldNode),
+// The response name isn't sufficient for determining uniqueness. In the case of
+// unions, for example, we can see a response name collision where the parent type
+// is different. In this case, these should not be merged (media)!
+// query {
+//   content {
+//     ... on Audio {
+//       media {
+//         url
+//       }
+//     }
+//     ... on Video {
+//       media {
+//         aspectRatio
+//       }
+//     }
+//   }
+// }
+export const groupByParentTypeAndResponseName = groupBy<Field, string>(field =>
+  `${field.scope.parentType}:${getResponseName(field.fieldNode)}`,
 );
 
 export const groupByParentType = groupBy<Field, GraphQLCompositeType>(
@@ -81,7 +98,7 @@ export function selectionSetFromFieldSet(
     selections: Array.from(groupByParentType(fields)).flatMap(
       ([typeCondition, fieldsByParentType]: [GraphQLCompositeType, FieldSet]) =>
         wrapInInlineFragmentIfNeeded(
-          Array.from(groupByResponseName(fieldsByParentType).values()).map(
+          Array.from(groupByParentTypeAndResponseName(fieldsByParentType).values()).map(
             fieldsByResponseName => {
               return combineFields(fieldsByResponseName)
                 .fieldNode;

packages/apollo-gateway/src/__tests__/integration/unions.test.ts

@@ -0,0 +1,79 @@
+import gql from 'graphql-tag';
+import { astSerializer, queryPlanSerializer } from '../../snapshotSerializers';
+import { execute } from '../execution-utils';
+
+expect.addSnapshotSerializer(astSerializer);
+expect.addSnapshotSerializer(queryPlanSerializer);
+
+it('handles multiple union type conditions that share a response name (media)', async () => {
+  const query = gql`
+    query {
+      content {
+        ...Audio
+        ... on Video {
+          media {
+            aspectRatio
+          }
+        }
+      }
+    }
+    fragment Audio on Audio {
+      media {
+        url
+      }
+    }
+  `;
+
+  const { queryPlan, errors } = await execute(
+    [
+      {
+        name: 'contentService',
+        typeDefs: gql`
+          extend type Query {
+            content: Content
+          }
+          union Content = Audio | Video
+          type Audio {
+            media: AudioURL
+          }
+          type AudioURL {
+            url: String
+          }
+          type Video {
+            media: VideoAspectRatio
+          }
+          type VideoAspectRatio {
+            aspectRatio: String
+          }
+        `,
+        resolvers: {
+          Query: {},
+        },
+      },
+    ],
+    { query },
+  );
+
+  expect(errors).toBeUndefined();
+  expect(queryPlan).toMatchInlineSnapshot(`
+    QueryPlan {
+      Fetch(service: "contentService") {
+        {
+          content {
+            __typename
+            ... on Audio {
+              media {
+                url
+              }
+            }
+            ... on Video {
+              media {
+                aspectRatio
+              }
+            }
+          }
+        }
+      },
+    }
+  `);
+});

packages/apollo-gateway/src/buildQueryPlan.ts

@@ -31,7 +31,7 @@ import {
   Field,
   FieldSet,
   groupByParentType,
-  groupByResponseName,
+  groupByParentTypeAndResponseName,
   matchesField,
   selectionSetFromFieldSet,
   Scope,
@@ -373,7 +373,7 @@ function splitFields(
   fields: FieldSet,
   groupForField: (field: Field<GraphQLObjectType>) => FetchGroup,
 ) {
-  for (const fieldsForResponseName of groupByResponseName(fields).values()) {
+  for (const fieldsForResponseName of groupByParentTypeAndResponseName(fields).values()) {
     for (const [parentType, fieldsForParentType] of groupByParentType(
       fieldsForResponseName,
     )) {