Fix issue when using a type condition on an inaccessible type in @req…

…uire (#1873)

The `@require(fields:)` argument can use some type conditions to require
data specific to a subtype. And those condition can even be on an
`@inaccessible` type since "inaccessibility is about the federated API
schema, not about subgraphs.

However, this wasn't properly working because when the execution code
was extracting the "requirements" data to pass it to the subgraph having
the `@require`, the (federated) API schema we used, and consequently
when the code tried to get the definition for the type condition in the
`@require`, it didn't find any definition (since the type in question is
`@inaccessible` and so not in said API schema) and as a result the
corresponding fields were not included in the subgraph query.

This commit fixes this issue, ensuring we pass the full supergraph
(which _has_ the `@inaccessible` elements) to the execution code so
that supergraph can be used when extracting required data.

There was also another place within query planning (in `graphPath.ts`
more precisely) where the code was looking up the possible
implementation types, in the supergraph, of the required `@inaccessible`
type, and was using the supergraph API to do so, which was incorrect
for the same reason. This commit fix that problem as well, passing
the full supergraph instead.

Fixes #1866, #1863

composition-js/src/__tests__/compose.test.ts

@@ -3030,4 +3030,108 @@ describe('composition', () => {
     const result = composeServices([subgraphA]);
     assertCompositionSuccess(result);
   });
+
+  it('handles fragments in @requires using @inaccessible types', () => {
+    const subgraphA = {
+      typeDefs: gql`
+        type Query @shareable {
+          dummy: Entity
+        }
+
+        type Entity @key(fields: "id") {
+          id: ID!
+          data: Foo
+        }
+
+        interface Foo {
+          foo: String!
+        }
+
+        interface Bar implements Foo {
+          foo: String!
+          bar: String!
+        }
+
+        type Baz implements Foo & Bar @shareable {
+          foo: String!
+          bar: String!
+          baz: String!
+        }
+
+        type Qux implements Foo & Bar @shareable {
+          foo: String!
+          bar: String!
+          qux: String!
+        }
+      `,
+      name: 'subgraphA',
+    };
+
+    const subgraphB = {
+      typeDefs: gql`
+        type Query @shareable {
+          dummy: Entity
+        }
+
+        type Entity @key(fields: "id") {
+          id: ID!
+          data: Foo @external
+          requirer: String! @requires(fields: "data { foo ... on Bar { bar ... on Baz { baz } ... on Qux { qux } } }")
+        }
+
+        interface Foo {
+          foo: String!
+        }
+
+        interface Bar implements Foo {
+          foo: String!
+          bar: String!
+        }
+
+        type Baz implements Foo & Bar @shareable @inaccessible {
+          foo: String!
+          bar: String!
+          baz: String!
+        }
+
+        type Qux implements Foo & Bar @shareable {
+          foo: String!
+          bar: String!
+          qux: String!
+        }
+      `,
+      name: 'subgraphB',
+    };
+
+    const result = composeAsFed2Subgraphs([subgraphA, subgraphB]);
+    assertCompositionSuccess(result);
+
+    const [_, api] = schemas(result);
+    expect(printSchema(api)).toMatchString(`
+      interface Bar implements Foo {
+        foo: String!
+        bar: String!
+      }
+
+      type Entity {
+        id: ID!
+        data: Foo
+        requirer: String!
+      }
+
+      interface Foo {
+        foo: String!
+      }
+
+      type Query {
+        dummy: Entity
+      }
+
+      type Qux implements Foo & Bar {
+        foo: String!
+        bar: String!
+        qux: String!
+      }
+    `);
+  });
 });

composition-js/src/compose.ts

@@ -51,7 +51,7 @@ export function compose(subgraphs: Subgraphs): CompositionResult {
   const supergraphSchema = mergeResult.supergraph;
   const supergraphQueryGraph = buildSupergraphAPIQueryGraph(supergraphSchema);
   const federatedQueryGraph = buildFederatedQueryGraph(supergraphSchema, false);
-  const validationResult = validateGraphComposition(supergraphQueryGraph, federatedQueryGraph);
+  const validationResult = validateGraphComposition(supergraphSchema, supergraphQueryGraph, federatedQueryGraph);
   if (validationResult.errors) {
     return { errors: validationResult.errors.map(e => ERRORS.SATISFIABILITY_ERROR.err(e.message)) };
   }

composition-js/src/validate.ts

@@ -4,7 +4,6 @@ import {
   Field,
   FieldDefinition,
   FieldSelection,
-  firstOf,
   FragmentElement,
   InputType,
   isLeafType,
@@ -216,17 +215,40 @@ function generateWitnessValue(type: InputType): any {
   }
 }
 
-export function validateGraphComposition(supergraph: QueryGraph, subgraphs: QueryGraph): {errors? : ValidationError[]} {
-  const errors = new ValidationTraversal(supergraph, subgraphs).validate();
+/**
+ * Validates that all the queries expressable on the API schema resulting of the composition of the provided subgraphs can be executed
+ * on those subgraphs.
+ *
+ * @param supergraphSchema the schema of the supergraph that composing `subgraphs` generated. Note this *must* be the full supergraph, not
+ *   just it's API schema (because it may be used to find the definition of elements that are marked `@inaccessible`). Note that this _not_
+ *   the same schema that the one reference inside `supergraphAPI` in particular.
+ * @param supergraphAPI the `QueryGraph` corresponding to the `supergraphSchema` API schema.
+ * @param subgraphs the (federated) `QueryGraph` corresponding the subgraphs having been composed to obtain `supergraphSchema`.
+ */
+export function validateGraphComposition(
+  supergraphSchema: Schema,
+  supergraphAPI: QueryGraph,
+  subgraphs: QueryGraph
+): {
+  errors? : ValidationError[]
+} {
+  const errors = new ValidationTraversal(supergraphSchema, supergraphAPI, subgraphs).validate();
   return errors.length > 0 ? {errors} : {};
 }
 
-export function computeSubgraphPaths(supergraphPath: RootPath<Transition>, subgraphs: QueryGraph): {traversal?: ValidationState, isComplete?: boolean, error?: ValidationError} {
+export function computeSubgraphPaths(
+  supergraphSchema: Schema,
+  supergraphPath: RootPath<Transition>,
+  subgraphs: QueryGraph
+): {
+  traversal?: ValidationState,
+  isComplete?: boolean,
+  error?: ValidationError
+} {
   try {
     assert(!supergraphPath.hasAnyEdgeConditions(), () => `A supergraph path should not have edge condition paths (as supergraph edges should not have conditions): ${supergraphPath}`);
-    const supergraphSchema = firstOf(supergraphPath.graph.sources.values())!;
     const conditionResolver = new ConditionValidationResolver(supergraphSchema, subgraphs);
-    const initialState = ValidationState.initial({supergraph: supergraphPath.graph, kind: supergraphPath.root.rootKind, subgraphs, conditionResolver});
+    const initialState = ValidationState.initial({supergraphAPI: supergraphPath.graph, kind: supergraphPath.root.rootKind, subgraphs, conditionResolver});
     let state = initialState;
     let isIncomplete = false;
     for (const [edge] of supergraphPath) {
@@ -269,18 +291,18 @@ export class ValidationState {
   }
 
   static initial({
-    supergraph,
+    supergraphAPI,
     kind,
     subgraphs,
     conditionResolver,
   }: {
-    supergraph: QueryGraph,
+    supergraphAPI: QueryGraph,
     kind: SchemaRootKind,
     subgraphs: QueryGraph,
     conditionResolver: ConditionValidationResolver,
   }) {
     return new ValidationState(
-      GraphPath.fromGraphRoot(supergraph, kind)!,
+      GraphPath.fromGraphRoot(supergraphAPI, kind)!,
       initialSubgraphPaths(kind, subgraphs).map((p) => TransitionPathWithLazyIndirectPaths.initial(p, conditionResolver.resolver)),
     );
   }
@@ -342,7 +364,6 @@ function isSupersetOrEqual(maybeSuperset: string[], other: string[]): boolean {
 }
 
 class ValidationTraversal {
-  private readonly supergraphSchema: Schema;
   private readonly conditionResolver: ConditionValidationResolver;
   // The stack contains all states that aren't terminal.
   private readonly stack: ValidationState[] = [];
@@ -353,16 +374,19 @@ class ValidationTraversal {
 
   private readonly validationErrors: ValidationError[] = [];
 
-  constructor(supergraph: QueryGraph, subgraphs: QueryGraph) {
-    this.supergraphSchema = firstOf(supergraph.sources.values())!;
-    this.conditionResolver = new ConditionValidationResolver(this.supergraphSchema, subgraphs);
-    supergraph.rootKinds().forEach((kind) => this.stack.push(ValidationState.initial({
-      supergraph,
+  constructor(
+    supergraphSchema: Schema,
+    supergraphAPI: QueryGraph,
+    subgraphs: QueryGraph
+  ) {
+    this.conditionResolver = new ConditionValidationResolver(supergraphSchema, subgraphs);
+    supergraphAPI.rootKinds().forEach((kind) => this.stack.push(ValidationState.initial({
+      supergraphAPI,
       kind,
       subgraphs,
       conditionResolver: this.conditionResolver
     })));
-    this.previousVisits = new QueryGraphState(supergraph);
+    this.previousVisits = new QueryGraphState(supergraphAPI);
   }
 
   validate(): ValidationError[] {

gateway-js/CHANGELOG.md

@@ -2,6 +2,11 @@
 
 This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The Federation v0.x equivalent for this package can be found [here](https://github.com/apollographql/federation/blob/version-0.x/gateway-js/CHANGELOG.md) on the `version-0.x` branch of this repo.
 
+- Fix issue when using a type condition on an inaccessible type in `@require` [#1873](https://github.com/apollographql/federation/pull/1873).
+  - __BREAKING__: this fix required passing a new argument to the `executeQueryPlan` method, which is technically
+    exported by the gateway. Most users of the gateway should _not_ call this method directly (which is exported mainly
+    for testing purposes in the first place) and will thus be unaffected, but if you do call this method directly, you
+    will have to pass the new argument when upgrading. See the method documentation for details.
 - Cleanup error related code, adding missing error code to a few errors [PR #1914](https://github.com/apollographql/federation/pull/1914).
 - Fix issue generating plan for a "diamond-shaped" dependency [PR #1900](https://github.com/apollographql/federation/pull/1900).