Remove undefined entities when executing Fetch of query plan (#612)

michael-watson · martijnwalraven · abernix · web-flow · commit d5688a4b72d5 · 2021-03-31T14:07:22.000+03:00
Improved query plan execution by pruning entities which are `undefined`, `null` or didn't match specified type conditions after a prior `FetchNode` execution. After pruning, only the remaining entities will be fetched in the dependent `FetchNode` execution. If NO entities remain, the request will not be made.

Co-authored-by: Michael Watson &lt;watson@apollographql.com&gt;
Co-authored-by: Martijn Walraven &lt;martijn@martijnwalraven.com&gt;
Co-authored-by: Jesse Rosenberger &lt;git@jro.cc&gt;
diff --git a/gateway-js/CHANGELOG.md b/gateway-js/CHANGELOG.md
@@ -4,7 +4,8 @@
 
 > The changes noted within this `vNEXT` section have not been released yet.  New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development.  When a release is being prepared, a new header will be (manually) created below and the appropriate changes within that release will be moved into the new section.
 
-- _Nothing yet! Stay tuned!_
+- Improved query plan execution by pruning entities which are `undefined` or didn't match specified type conditions after a prior `FetchNode` execution.  After pruning, only the remaining entities will be fetched in the dependent `FetchNode` execution.  If NO entities remain, the request will not be made.  [PR #612](https://github.com/apollographql/federation/pull/612)
+
 ## v0.25.0
 
 - Add support for an upcoming mechanism for fetching a graph's schema and configuration when using managed federation. In this release, the new mechanism is opt-in, and users shouldn't use enable it unless instructed by Apollo to do so. [PR #458](https://github.com/apollographql/federation/pull/458) [PR #585](https://github.com/apollographql/federation/pull/585)
diff --git a/gateway-js/src/__tests__/executeQueryPlan.test.ts b/gateway-js/src/__tests__/executeQueryPlan.test.ts
@@ -25,6 +25,13 @@ describe('executeQueryPlan', () => {
     addResolversToSchema(serviceMap[serviceName].schema, resolvers);
   }
 
+  function spyOnEntitiesResolverInService(serviceName: string) {
+    const entitiesField = serviceMap[serviceName].schema
+      .getQueryType()!
+      .getFields()['_entities'];
+    return jest.spyOn(entitiesField, 'resolve');
+  }
+
   let schema: GraphQLSchema;
   let queryPlannerPointer: QueryPlannerPointer;
 
@@ -142,6 +149,320 @@ describe('executeQueryPlan', () => {
       expect(response).toHaveProperty('errors.0.extensions.variables', {});
     });
 
+    it(`should not send request to downstream services when all entities are undefined`, async () => {
+      const accountsEntitiesResolverSpy = spyOnEntitiesResolverInService(
+        'accounts',
+      );
+
+      const operationString = `#graphql
+        query {
+          # The first 3 products are all Furniture
+          topProducts(first: 3) {
+            reviews {
+              body
+            }
+            ... on Book {
+              reviews {
+                author {
+                  name {
+                    first
+                    last
+                  }
+                }
+              }
+            }
+          }
+        }
+      `;
+
+      const operationDocument = gql(operationString);
+
+      const operationContext = buildOperationContext({
+        schema,
+        operationDocument,
+        operationString,
+        queryPlannerPointer,
+      });
+
+      const queryPlan = buildQueryPlan(operationContext);
+
+      const response = await executeQueryPlan(
+        queryPlan,
+        serviceMap,
+        buildRequestContext(),
+        operationContext,
+      );
+
+      expect(accountsEntitiesResolverSpy).not.toHaveBeenCalled();
+
+      expect(response).toMatchInlineSnapshot(`
+        Object {
+          "data": Object {
+            "topProducts": Array [
+              Object {
+                "reviews": Array [
+                  Object {
+                    "body": "Love it!",
+                  },
+                  Object {
+                    "body": "Prefer something else.",
+                  },
+                ],
+              },
+              Object {
+                "reviews": Array [
+                  Object {
+                    "body": "Too expensive.",
+                  },
+                ],
+              },
+              Object {
+                "reviews": Array [
+                  Object {
+                    "body": "Could be better.",
+                  },
+                ],
+              },
+            ],
+          },
+        }
+      `);
+    });
+
+    it(`should send a request to downstream services for the remaining entities when some entities are undefined`, async () => {
+      const accountsEntitiesResolverSpy = spyOnEntitiesResolverInService(
+        'accounts',
+      );
+
+      const operationString = `#graphql
+        query {
+          # The first 3 products are all Furniture, but the next 2 are Books
+          topProducts(first: 5) {
+            reviews {
+              body
+            }
+            ... on Book {
+              reviews {
+                author {
+                  name {
+                    first
+                    last
+                  }
+                }
+              }
+            }
+          }
+        }
+      `;
+
+      const operationDocument = gql(operationString);
+
+      const operationContext = buildOperationContext({
+        schema,
+        operationDocument,
+        operationString,
+        queryPlannerPointer,
+      });
+
+      const queryPlan = buildQueryPlan(operationContext);
+
+      const response = await executeQueryPlan(
+        queryPlan,
+        serviceMap,
+        buildRequestContext(),
+        operationContext,
+      );
+
+      expect(accountsEntitiesResolverSpy).toHaveBeenCalledTimes(1);
+      expect(accountsEntitiesResolverSpy.mock.calls[0][1]).toEqual({
+        representations: [
+          { __typename: 'User', id: '2' },
+          { __typename: 'User', id: '2' },
+        ],
+      });
+
+      expect(response).toMatchInlineSnapshot(`
+        Object {
+          "data": Object {
+            "topProducts": Array [
+              Object {
+                "reviews": Array [
+                  Object {
+                    "body": "Love it!",
+                  },
+                  Object {
+                    "body": "Prefer something else.",
+                  },
+                ],
+              },
+              Object {
+                "reviews": Array [
+                  Object {
+                    "body": "Too expensive.",
+                  },
+                ],
+              },
+              Object {
+                "reviews": Array [
+                  Object {
+                    "body": "Could be better.",
+                  },
+                ],
+              },
+              Object {
+                "reviews": Array [
+                  Object {
+                    "author": Object {
+                      "name": Object {
+                        "first": "Alan",
+                        "last": "Turing",
+                      },
+                    },
+                    "body": "Wish I had read this before.",
+                  },
+                ],
+              },
+              Object {
+                "reviews": Array [
+                  Object {
+                    "author": Object {
+                      "name": Object {
+                        "first": "Alan",
+                        "last": "Turing",
+                      },
+                    },
+                    "body": "A bit outdated.",
+                  },
+                ],
+              },
+            ],
+          },
+        }
+      `);
+    });
+
+    it(`should not send request to downstream service when entities don't match type conditions`, async () => {
+      const reviewsEntitiesResolverSpy = spyOnEntitiesResolverInService(
+        'reviews',
+      );
+
+      const operationString = `#graphql
+        query {
+          # The first 3 products are all Furniture
+          topProducts(first: 3) {
+            ... on Book {
+              reviews {
+                body
+              }
+            }
+          }
+        }
+      `;
+
+      const operationDocument = gql(operationString);
+
+      const operationContext = buildOperationContext({
+        schema,
+        operationDocument,
+        operationString,
+        queryPlannerPointer,
+      });
+
+      const queryPlan = buildQueryPlan(operationContext);
+
+      const response = await executeQueryPlan(
+        queryPlan,
+        serviceMap,
+        buildRequestContext(),
+        operationContext,
+      );
+
+      expect(reviewsEntitiesResolverSpy).not.toHaveBeenCalled();
+
+      expect(response).toMatchInlineSnapshot(`
+        Object {
+          "data": Object {
+            "topProducts": Array [
+              Object {},
+              Object {},
+              Object {},
+            ],
+          },
+        }
+      `);
+    });
+
+    it(`should send a request to downstream services for the remaining entities when some entities don't match type conditions`, async () => {
+      const reviewsEntitiesResolverSpy = spyOnEntitiesResolverInService(
+        'reviews',
+      );
+
+      const operationString = `#graphql
+        query {
+          # The first 3 products are all Furniture, but the next 2 are Books
+          topProducts(first: 5) {
+            ... on Book {
+              reviews {
+                body
+              }
+            }
+          }
+        }
+      `;
+
+      const operationDocument = gql(operationString);
+
+      const operationContext = buildOperationContext({
+        schema,
+        operationDocument,
+        operationString,
+        queryPlannerPointer,
+      });
+
+      const queryPlan = buildQueryPlan(operationContext);
+
+      const response = await executeQueryPlan(
+        queryPlan,
+        serviceMap,
+        buildRequestContext(),
+        operationContext,
+      );
+
+      expect(reviewsEntitiesResolverSpy).toHaveBeenCalledTimes(1);
+      expect(reviewsEntitiesResolverSpy.mock.calls[0][1]).toEqual({
+        representations: [
+          { __typename: 'Book', isbn: '0262510871' },
+          { __typename: 'Book', isbn: '0136291554' },
+        ],
+      });
+
+      expect(response).toMatchInlineSnapshot(`
+        Object {
+          "data": Object {
+            "topProducts": Array [
+              Object {},
+              Object {},
+              Object {},
+              Object {
+                "reviews": Array [
+                  Object {
+                    "body": "Wish I had read this before.",
+                  },
+                ],
+              },
+              Object {
+                "reviews": Array [
+                  Object {
+                    "body": "A bit outdated.",
+                  },
+                ],
+              },
+            ],
+          },
+        }
+      `);
+    });
+
     it(`should still include other root-level results if one root-level field errors out`, async () => {
       overrideResolversInService('accounts', {
         RootQuery: {
diff --git a/gateway-js/src/executeQueryPlan.ts b/gateway-js/src/executeQueryPlan.ts
@@ -25,6 +25,7 @@ import {
   getResponseName
 } from '@apollo/query-planner';
 import { deepMerge } from './utilities/deepMerge';
+import { isNotNullOrUndefined } from './utilities/array';
 
 export type ServiceMap = {
   [serviceName: string]: GraphQLDataSource;
@@ -192,7 +193,7 @@ async function executeNode<TContext>(
 async function executeFetch<TContext>(
   context: ExecutionContext<TContext>,
   fetch: FetchNode,
-  results: ResultMap | ResultMap[],
+  results: ResultMap | (ResultMap | null | undefined)[],
   _path: ResponsePath,
   traceNode: Trace.QueryPlanNode.FetchNode | null,
 ): Promise<void> {
@@ -202,7 +203,14 @@ async function executeFetch<TContext>(
     throw new Error(`Couldn't find service with name "${fetch.serviceName}"`);
   }
 
-  const entities = Array.isArray(results) ? results : [results];
+  let entities: ResultMap[];
+  if (Array.isArray(results)) {
+    // Remove null or undefined entities from the list
+    entities = results.filter(isNotNullOrUndefined);
+  } else {
+    entities = [results];
+  }
+
   if (entities.length < 1) return;
 
   let variables = Object.create(null);
@@ -242,6 +250,10 @@ async function executeFetch<TContext>(
       }
     });
 
+    // If there are no representations, that means the type conditions in
+    // the requires don't match any entities.
+    if (representations.length < 1) return;
+
     if ('representations' in variables) {
       throw new Error(`Variables cannot contain key "representations"`);
     }
diff --git a/gateway-js/src/utilities/array.ts b/gateway-js/src/utilities/array.ts
@@ -1,4 +1,4 @@
-function isNotNullOrUndefined<T>(
+export function isNotNullOrUndefined<T>(
   value: T | null | undefined,
 ): value is T {
   return value !== null && typeof value !== 'undefined';

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-function isNotNullOrUndefined<T>(`
	`1`	`+export function isNotNullOrUndefined<T>(`
`2`	`2`	`value: T \| null \| undefined,`
`3`	`3`	`): value is T {`
`4`	`4`	`return value !== null && typeof value !== 'undefined';`