Merge pull request #864 from konalegi/main

Do not send field to the service if it cannot process it
apollographql · Jan 10, 2022 · 345d5f0 · 345d5f0
2 parents 096ef8d + 613afb1
commit 345d5f0
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 11 deletions.
diff --git a/query-planner-js/src/__tests__/features/basic/build-query-plan.feature b/query-planner-js/src/__tests__/features/basic/build-query-plan.feature
@@ -666,6 +666,28 @@ Scenario: when requesting a relationship field with extension subfields from a d
     }
     """
 
+Scenario: for abstract types, it shouldn't add not owning types on interface unpacking
+  Given query
+    """
+    query {
+      vehicle(id: "xc60") {
+        ...on Vehicle { id }
+      }
+    }
+    """
+  Then query plan
+    """
+    {
+      "kind": "QueryPlan",
+      "node": {
+        "kind": "Fetch",
+        "serviceName": "product",
+        "variableUsages": [],
+        "operation": "{vehicle(id:\"xc60\"){__typename ...on Car{id}...on Van{id}...on Bicycle{id}}}"
+      }
+    }
+    """
+
 Scenario: for abstract types, it should add __typename when fetching objects of an interface type from a service
   Given query
     """

diff --git a/query-planner-js/src/__tests__/features/basic/supergraphSdl.graphql b/query-planner-js/src/__tests__/features/basic/supergraphSdl.graphql
@@ -120,6 +120,7 @@ enum join__Graph {
     INVENTORY @join__graph(name: "inventory" url: "undefined")
     PRODUCT @join__graph(name: "product" url: "undefined")
     REVIEWS @join__graph(name: "reviews" url: "undefined")
+    DOCK @join__graph(name: "dock" url: "undefined")
 }
 
 type KeyValue {
@@ -277,6 +278,22 @@ type Van implements Vehicle
     retailPrice: String @join__field(graph: REVIEWS, requires: "price")
 }
 
+type Bicycle implements Vehicle {
+    id: String!
+    description: String
+    price: String
+    retailPrice: String
+}
+
+type Yacht implements Vehicle
+    @join__owner(graph: DOCK)
+{
+    id: String! @join__field(graph: DOCK)
+    description: String @join__field(graph: DOCK)
+    price: String @join__field(graph: DOCK)
+    retailPrice: String @join__field(graph: REVIEWS, requires: "price")
+}
+
 interface Vehicle {
     id: String!
     description: String

diff --git a/query-planner-js/src/buildQueryPlan.ts b/query-planner-js/src/buildQueryPlan.ts
@@ -828,7 +828,6 @@ function splitFields(
             debug.group(`For runtime parent type ${runtimeParentType}:`);
 
             const fieldDef = context.getFieldDef(
-
               runtimeParentType,
               field.fieldNode,
             );
@@ -838,16 +837,20 @@ function splitFields(
               fieldDef,
             }));
 
-            group.fields.push(
-              completeField(
-                context,
-                scope.refine(runtimeParentType),
-                group,
-                path,
-                fieldsWithRuntimeParentType,
-              ),
-            );
-            debug.groupEnd(() => `Updated fetch group: ${debugPrintGroup(group)}`);
+            if (shouldIncludeToFetchGroup(group, fieldsWithRuntimeParentType, context, runtimeParentType)) {
+              group.fields.push(
+                completeField(
+                  context,
+                  scope.refine(runtimeParentType),
+                  group,
+                  path,
+                  fieldsWithRuntimeParentType,
+                ),
+              );
+              debug.groupEnd(() => `Updated fetch group: ${debugPrintGroup(group)}`);
+            } else {
+              debug.groupEnd(() => `Group has been skipped: ${debugPrintGroup(group)}`);
+            }
           }
           debug.groupEnd();
         }
@@ -859,6 +862,28 @@ function splitFields(
   }
 }
 
+function shouldIncludeToFetchGroup(
+  group: FetchGroup,
+  fields: FieldSet,
+  context: QueryPlanningContext,
+  runtimeParentType: GraphQLObjectType
+  ): Boolean {
+  const { fieldDef } = fields[0];
+  const returnType = getNamedType(fieldDef.type);
+
+  if (isCompositeType(returnType)) {
+    return true;
+  }
+
+  const owningService = context.getOwningService(runtimeParentType, fieldDef);
+
+  if (owningService === undefined) {
+    return true
+  }
+
+  return owningService === group.serviceName || group.providedFields.some(f => f.fieldDef === fields[0].fieldDef)
+}
+
 function completeField(
   context: QueryPlanningContext,
   scope: Scope,