@apollo/query-planner - npm Package Compare versions

Comparing version 2.0.2 to 2.0.3

dist/buildPlan.js

@@ -443,14 +443,8 @@ "use strict";
         (0, federation_internals_1.assert)(!toMerge.isTopLevel, () => `Shouldn't merge top level group ${toMerge} into ${this}`);
+        const mergePathConditionalDirectives = (0, federation_internals_1.conditionalDirectivesInOperationPath)(mergePath);
         const selectionSet = (0, federation_internals_1.selectionSetOfPath)(mergePath, (endOfPathSet) => {
             (0, federation_internals_1.assert)(endOfPathSet, () => `Merge path ${mergePath} ends on a non-selectable type`);
-            for (const typeCastSel of toMerge.selection.selections()) {
-                (0, federation_internals_1.assert)(typeCastSel instanceof federation_internals_1.FragmentSelection, () => `Unexpected field selection ${typeCastSel} at top-level of ${toMerge} selection.`);
-                const entityType = typeCastSel.element().typeCondition;
-                (0, federation_internals_1.assert)(entityType, () => `Unexpected fragment _without_ condition at start of ${toMerge}`);
-                if ((0, federation_internals_1.sameType)(endOfPathSet.parentType, entityType)) {
-                    endOfPathSet.mergeIn(typeCastSel.selectionSet);
-                }
-                else {
-                    endOfPathSet.add(typeCastSel);
-                }
+            for (const selection of toMerge.selection.selections()) {
+                const withoutUneededFragments = removeRedundantFragments(selection, endOfPathSet.parentType, mergePathConditionalDirectives);
+                addSelectionOrSelectionSet(endOfPathSet, withoutUneededFragments);
             }
@@ -496,2 +490,54 @@ });
 }
+function addSelectionOrSelectionSet(selectionSet, toAdd) {
+    if (toAdd instanceof federation_internals_1.SelectionSet) {
+        selectionSet.mergeIn(toAdd);
+    }
+    else {
+        selectionSet.add(toAdd);
+    }
+}
+function removeRedundantFragmentsOfSet(selectionSet, type, unneededDirectives) {
+    let newSet = undefined;
+    const selections = selectionSet.selections();
+    for (let i = 0; i < selections.length; i++) {
+        const selection = selections[i];
+        const updated = removeRedundantFragments(selection, type, unneededDirectives);
+        if (newSet) {
+            addSelectionOrSelectionSet(newSet, updated);
+        }
+        else if (selection !== updated) {
+            newSet = new federation_internals_1.SelectionSet(type);
+            for (let j = 0; j < i; j++) {
+                newSet.add(selections[j]);
+            }
+            addSelectionOrSelectionSet(newSet, updated);
+        }
+    }
+    return newSet ? newSet : selectionSet;
+}
+function removeRedundantFragments(selection, type, unneededDirectives) {
+    if (selection.kind !== 'FragmentSelection') {
+        return selection;
+    }
+    const fragment = selection.element();
+    const fragmentType = fragment.typeCondition;
+    if (!fragmentType) {
+        return selection;
+    }
+    let neededDirectives = [];
+    if (fragment.appliedDirectives.length > 0) {
+        neededDirectives = (0, federation_internals_1.directiveApplicationsSubstraction)(fragment.appliedDirectives, unneededDirectives);
+    }
+    if ((0, federation_internals_1.sameType)(type, fragmentType) && neededDirectives.length === 0) {
+        return removeRedundantFragmentsOfSet(selection.selectionSet, type, unneededDirectives);
+    }
+    else if (neededDirectives.length === fragment.appliedDirectives.length) {
+        return selection;
+    }
+    else {
+        const updatedFragement = new federation_internals_1.FragmentElement(type, fragment.typeCondition);
+        neededDirectives.forEach((d) => updatedFragement.applyDirective(d.definition, d.arguments()));
+        return (0, federation_internals_1.selectionSetOfElement)(updatedFragement, selection.selectionSet);
+    }
+}
 function schemaRootKindToOperationKind(operation) {
@@ -941,7 +987,7 @@ switch (operation) {
 }
-function computeGroupsForTree(dependencyGraph, pathTree, startGroup, initialMergeAt = [], initialPath = []) {
-    const stack = [{ tree: pathTree, group: startGroup, mergeAt: initialMergeAt, path: initialPath }];
+function computeGroupsForTree(dependencyGraph, pathTree, startGroup, initialMergeAt = [], initialPath = [], initialContext = query_graphs_1.emptyContext) {
+    const stack = [{ tree: pathTree, group: startGroup, mergeAt: initialMergeAt, path: initialPath, context: initialContext }];
     const createdGroups = [];
     while (stack.length > 0) {
-        const { tree, group, mergeAt, path } = stack.pop();
+        const { tree, group, mergeAt, path, context } = stack.pop();
         if (tree.isLeaf()) {
@@ -953,2 +999,3 @@ group.addSelection(path);
                 if ((0, query_graphs_1.isPathContext)(operation)) {
+                    const newContext = operation;
                     (0, federation_internals_1.assert)(edge !== null, () => `Unexpected 'null' edge with no trigger at ${path}`);
@@ -967,6 +1014,6 @@ (0, federation_internals_1.assert)(edge.head.source !== edge.tail.source, () => `Key/Query edge ${edge} should change the underlying subgraph`);
                         inputSelections.mergeIn(edge.conditions);
-                        const [inputs, newPath] = createNewFetchSelectionContext(type, inputSelections, operation);
+                        const [inputs, newPath] = createNewFetchSelectionContext(type, inputSelections, newContext);
                         newGroup.addInputs(inputs);
                         group.addSelection(path.concat(new federation_internals_1.Field(edge.head.type.typenameField())));
-                        stack.push({ tree: child, group: newGroup, mergeAt, path: newPath });
+                        stack.push({ tree: child, group: newGroup, mergeAt, path: newPath, context: newContext });
                     }
@@ -982,4 +1029,4 @@ else {
                         const newGroup = dependencyGraph.newRootTypeFetchGroup(edge.tail.source, rootKind, type, mergeAt, group, path);
-                        const newPath = createNewFetchSelectionContext(type, undefined, operation)[1];
-                        stack.push({ tree: child, group: newGroup, mergeAt, path: newPath });
+                        const newPath = createNewFetchSelectionContext(type, undefined, newContext)[1];
+                        stack.push({ tree: child, group: newGroup, mergeAt, path: newPath, context: newContext });
                     }
@@ -989,9 +1036,9 @@ }
                     const newPath = operation.appliedDirectives.length === 0 ? path : path.concat(operation);
-                    stack.push({ tree: child, group, mergeAt, path: newPath });
+                    stack.push({ tree: child, group, mergeAt, path: newPath, context });
                 }
                 else {
                     (0, federation_internals_1.assert)(edge.head.source === edge.tail.source, () => `Collecting edge ${edge} for ${operation} should not change the underlying subgraph`);
-                    const updated = { tree: child, group, mergeAt, path };
+                    const updated = { tree: child, group, mergeAt, path, context };
                     if (conditions) {
-                        const requireResult = handleRequires(dependencyGraph, edge, conditions, group, mergeAt, path);
+                        const requireResult = handleRequires(dependencyGraph, edge, conditions, group, mergeAt, path, context);
                         updated.group = requireResult.group;
@@ -1032,5 +1079,8 @@ updated.mergeAt = requireResult.mergeAt;
 }
-function handleRequires(dependencyGraph, edge, requiresConditions, group, mergeAt, path) {
+function pathHasOnlyFragments(path) {
+    return path.every((element) => element.kind === 'FragmentElement');
+}
+function handleRequires(dependencyGraph, edge, requiresConditions, group, mergeAt, path, context) {
     const entityType = edge.head.type;
-    if (!group.isTopLevel && path.length == 1 && path[0].kind === 'FragmentElement') {
+    if (!group.isTopLevel && pathHasOnlyFragments(path)) {
         const originalInputs = group.clonedInputs();
@@ -1088,3 +1138,3 @@ const newGroup = dependencyGraph.newKeyFetchGroup(group.subgraphName, group.mergeAt);
         if (unmergedGroups.length == 0) {
-            group.addInputs(inputsForRequire(dependencyGraph.federatedQueryGraph, entityType, edge, false)[0]);
+            group.addInputs(inputsForRequire(dependencyGraph.federatedQueryGraph, entityType, edge, context, false)[0]);
             return { group, mergeAt, path, createdGroups: [] };
@@ -1094,3 +1144,3 @@ }
         postRequireGroup.addDependencyOn(unmergedGroups);
-        const [inputs, newPath] = inputsForRequire(dependencyGraph.federatedQueryGraph, entityType, edge);
+        const [inputs, newPath] = inputsForRequire(dependencyGraph.federatedQueryGraph, entityType, edge, context);
         postRequireGroup.addInputs(inputs);
@@ -1111,3 +1161,3 @@ return {
         newGroup.addDependencyOn(createdGroups);
-        const [inputs, newPath] = inputsForRequire(dependencyGraph.federatedQueryGraph, entityType, edge);
+        const [inputs, newPath] = inputsForRequire(dependencyGraph.federatedQueryGraph, entityType, edge, context);
         newGroup.addInputs(inputs);
@@ -1117,4 +1167,3 @@ return { group: newGroup, mergeAt, path: newPath, createdGroups };
 }
-function inputsForRequire(graph, entityType, edge, includeKeyInputs = true) {
-    const typeCast = new federation_internals_1.FragmentElement(entityType, entityType.name);
+function inputsForRequire(graph, entityType, edge, context, includeKeyInputs = true) {
     const fullSelectionSet = new federation_internals_1.SelectionSet(entityType);
@@ -1128,3 +1177,3 @@ fullSelectionSet.add(new federation_internals_1.FieldSelection(new federation_internals_1.Field(entityType.typenameField())));
     }
-    return [(0, federation_internals_1.selectionOfElement)(typeCast, fullSelectionSet), [typeCast]];
+    return createNewFetchSelectionContext(entityType, fullSelectionSet, context);
 }
@@ -1131,0 +1180,0 @@ const representationsVariable = new federation_internals_1.Variable('representations');

package.json

 {
   "name": "@apollo/query-planner",
-  "version": "2.0.2",
+  "version": "2.0.3",
   "description": "Apollo Query Planner",
@@ -28,4 +28,4 @@ "author": "Apollo <[e-mail]>",
   "dependencies": {
-    "@apollo/federation-internals": "^2.0.2",
-    "@apollo/query-graphs": "^2.0.2",
+    "@apollo/federation-internals": "^2.0.3",
+    "@apollo/query-graphs": "^2.0.3",
     "chalk": "^4.1.0",
@@ -38,3 +38,3 @@ "deep-equal": "^2.0.5",
   },
-  "gitHead": "02bd93d0ed4f75f9cd59b84ddcb66a30babb1553"
+  "gitHead": "cda3ff6399a820367eb43c2e9a81c77763a57bdd"
 }

src/__tests__/buildPlan.test.ts

@@ -1998,2 +1998,256 @@ import { astSerializer, queryPlanSerializer, QueryPlanner } from '@apollo/query-planner';
   });
+
+  describe('@include and @skip', () => {
+    it('handles a simple @requires triggered within a conditional', () => {
+      const subgraph1 = {
+        name: 'Subgraph1',
+        typeDefs: gql`
+          type Query {
+            t: T
+          }
+
+          type T @key(fields: "id") {
+            id: ID!
+            a: Int
+          }
+        `
+      }
+
+      const subgraph2 = {
+        name: 'Subgraph2',
+        typeDefs: gql`
+          type T @key(fields: "id") {
+            id: ID!
+            a: Int @external
+            b: Int @requires(fields: "a")
+          }
+        `
+      }
+
+      const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2);
+      const operation = operationFromDocument(api, gql`
+        query foo($test: Boolean!){
+          t @include(if: $test) {
+            b
+          }
+        }
+      `);
+
+      const plan = queryPlanner.buildQueryPlan(operation);
+      // Note that during query planning, the inputs for the 2nd fetch also have the `@include(if: $test)` condition
+      // on them, but the final query plan format does not support that at the momment, which is why they don't
+      // appear below (it is a bit of a shame because that means the gateway/router can't use it, and so this
+      // should (imo) be fixed but...).
+      expect(plan).toMatchInlineSnapshot(`
+        QueryPlan {
+          Sequence {
+            Fetch(service: "Subgraph1") {
+              {
+                t @include(if: $test) {
+                  __typename
+                  id
+                  a
+                }
+              }
+            },
+            Flatten(path: "t") {
+              Fetch(service: "Subgraph2") {
+                {
+                  ... on T {
+                    __typename
+                    id
+                    a
+                  }
+                } =>
+                {
+                  ... on T @include(if: $test) {
+                    b
+                  }
+                }
+              },
+            },
+          },
+        }
+      `);
+    });
+
+    it('handles a @requires triggered conditionally', () => {
+      const subgraph1 = {
+        name: 'Subgraph1',
+        typeDefs: gql`
+          type Query {
+            t: T
+          }
+
+          type T @key(fields: "id") {
+            id: ID!
+            a: Int
+          }
+        `
+      }
+
+      const subgraph2 = {
+        name: 'Subgraph2',
+        typeDefs: gql`
+          type T @key(fields: "id") {
+            id: ID!
+            a: Int @external
+            b: Int @requires(fields: "a")
+          }
+        `
+      }
+
+      const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2);
+      const operation = operationFromDocument(api, gql`
+        query foo($test: Boolean!){
+          t {
+            b @include(if: $test)
+          }
+        }
+      `);
+
+      const plan = queryPlanner.buildQueryPlan(operation);
+      expect(plan).toMatchInlineSnapshot(`
+        QueryPlan {
+          Sequence {
+            Fetch(service: "Subgraph1") {
+              {
+                t {
+                  __typename
+                  id
+                  ... on T @include(if: $test) {
+                    a
+                  }
+                }
+              }
+            },
+            Flatten(path: "t") {
+              Fetch(service: "Subgraph2") {
+                {
+                  ... on T {
+                    __typename
+                    id
+                    a
+                  }
+                } =>
+                {
+                  ... on T {
+                    b @include(if: $test)
+                  }
+                }
+              },
+            },
+          },
+        }
+      `);
+    });
+
+    it('handles a @requires where multiple conditional are involved', () => {
+      const subgraph1 = {
+        name: 'Subgraph1',
+        typeDefs: gql`
+          type Query {
+            a: A
+          }
+
+          type A @key(fields: "idA") {
+            idA: ID!
+          }
+        `
+      }
+
+      const subgraph2 = {
+        name: 'Subgraph2',
+        typeDefs: gql`
+          type A @key(fields: "idA") {
+            idA: ID!
+            b: [B]
+          }
+
+          type B @key(fields: "idB") {
+            idB: ID!
+            required: Int
+          }
+        `
+      }
+
+      const subgraph3 = {
+        name: 'Subgraph3',
+        typeDefs: gql`
+          type B @key(fields: "idB") {
+            idB: ID!
+            c: Int @requires(fields: "required")
+            required: Int @external
+          }
+        `
+      }
+
+      const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2, subgraph3);
+      const operation = operationFromDocument(api, gql`
+        query foo($test1: Boolean!, $test2: Boolean!){
+          a @include(if: $test1) {
+            b @include(if: $test2) {
+              c
+            }
+          }
+        }
+      `);
+
+      global.console = require('console');
+      const plan = queryPlanner.buildQueryPlan(operation);
+      expect(plan).toMatchInlineSnapshot(`
+        QueryPlan {
+          Sequence {
+            Fetch(service: "Subgraph1") {
+              {
+                a @include(if: $test1) {
+                  __typename
+                  idA
+                }
+              }
+            },
+            Flatten(path: "a") {
+              Fetch(service: "Subgraph2") {
+                {
+                  ... on A {
+                    __typename
+                    idA
+                  }
+                } =>
+                {
+                  ... on A @include(if: $test1) {
+                    b @include(if: $test2) {
+                      __typename
+                      idB
+                      required
+                    }
+                  }
+                }
+              },
+            },
+            Flatten(path: "a.b.@") {
+              Fetch(service: "Subgraph3") {
+                {
+                  ... on B {
+                    ... on B {
+                      __typename
+                      idB
+                      required
+                    }
+                  }
+                } =>
+                {
+                  ... on B @include(if: $test1) {
+                    ... on B @include(if: $test2) {
+                      c
+                    }
+                  }
+                }
+              },
+            },
+          },
+        }
+      `);
+    });
+  });
 });
@@ -2261,1 +2515,150 @@
 
+describe('Field covariance and type-explosion', () => {
+  // This tests the issue from https://github.com/apollographql/federation/issues/1858.
+  // That issue, which was a bug in the handling of selection sets, was concretely triggered with
+  // a mix of an interface field implemented with some covariance and the query plan using
+  // type-explosion.
+  // We include a test using a federation 1 supergraph as this is how the issue was discovered
+  // and it is the simplest way to reproduce since type-explosion is always triggered when we
+  // have federation 1 supergraph (due to those lacking information on interfaces). The 2nd
+  // test shows that error can be reproduced on a pure fed2 example, it's just a bit more
+  // complex as we need to involve a @provide just to force the query planner to type explode
+  // (more precisely, this force the query planner to _consider_ type explosion; the generated
+  // query plan still ends up not type-exploding in practice since as it's not necessary).
+  test('with federation 1 supergraphs', () => {
+    const supergraphSdl = `
+      schema @core(feature: "https://specs.apollo.dev/core/v0.1") @core(feature: "https://specs.apollo.dev/join/v0.1") {
+        query: Query
+      }
+
+      directive @core(feature: String!) repeatable on SCHEMA
+      directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION
+      directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE
+      directive @join__owner(graph: join__Graph!) on OBJECT | INTERFACE
+      directive @join__graph(name: String!, url: String!) on ENUM_VALUE
+
+      interface Interface {
+        field: Interface
+      }
+
+      scalar join__FieldSet
+
+      enum join__Graph {
+        SUBGRAPH @join__graph(name: "subgraph", url: "http://localhost:4001/")
+      }
+
+      type Object implements Interface {
+        field: Object
+      }
+
+      type Query {
+        dummy: Interface @join__field(graph: SUBGRAPH)
+      }
+    `;
+
+    const supergraph = buildSchema(supergraphSdl);
+    const api = supergraph.toAPISchema();
+    const queryPlanner = new QueryPlanner(supergraph);
+
+    const operation = operationFromDocument(api, gql`
+      {
+        dummy {
+          field {
+            ... on Object {
+              field {
+                __typename
+              }
+            }
+          }
+        }
+      }
+    `);
+    const queryPlan = queryPlanner.buildQueryPlan(operation);
+    expect(queryPlan).toMatchInlineSnapshot(`
+      QueryPlan {
+        Fetch(service: "subgraph") {
+          {
+            dummy {
+              __typename
+              ... on Object {
+                field {
+                  field {
+                    __typename
+                  }
+                }
+              }
+            }
+          }
+        },
+      }
+    `);
+  });
+
+  it('with federation 2 subgraphs', () => {
+    const subgraph1 = {
+      name: 'Subgraph1',
+      typeDefs: gql`
+        type Query {
+          dummy: Interface
+        }
+
+        interface Interface {
+          field: Interface
+        }
+
+        type Object implements Interface @key(fields: "id") {
+          id: ID!
+          field: Object @provides(fields: "x")
+          x: Int @external
+        }
+      `
+    }
+
+    const subgraph2 = {
+      name: 'Subgraph2',
+      typeDefs: gql`
+        type Object @key(fields: "id") {
+          id: ID!
+          x: Int @shareable
+        }
+      `
+    }
+
+    const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2);
+    const operation = operationFromDocument(api, gql`
+      {
+        dummy {
+          field {
+            ... on Object {
+              field {
+                __typename
+              }
+            }
+          }
+        }
+      }
+    `);
+
+    const plan = queryPlanner.buildQueryPlan(operation);
+    expect(plan).toMatchInlineSnapshot(`
+      QueryPlan {
+        Fetch(service: "Subgraph1") {
+          {
+            dummy {
+              __typename
+              field {
+                __typename
+                ... on Object {
+                  field {
+                    __typename
+                  }
+                }
+              }
+            }
+          }
+        },
+      }
+    `);
+  });
+})
+

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

479650

increased by3.83%

Lines of code

6838

increased by7.87%

Dependency changes

• Updated@apollo/federation-internals@^2.0.3

• Updated@apollo/query-graphs@^2.0.3