Skip to content

Commit

Permalink
Fix issue with @requires using wrong "return key"
Browse files Browse the repository at this point in the history
Some times, when a @requires is just after a key, we can simply collect
the required field before taking the key. Other times, we encounter
a @require in a type T of a subgraph A, and have to jump to subgraph B
to get the required field. In that case, once we've collected the
required fields, we need to "resume" query on T in A, and that means
using a key there. The code dealing with that post-require "return key"
later case was incorrect, essentially using a key on subgraph B instead
of one on subgraph A. As a consequence, subgraphs that shouldn't have
been allowed to compose (because they were missing the needed key on A)
were allowed to composed, and the code later failed during query
planning.

This commit fixes this issue, and adds tests for that case. One of the
test introduced _fails_ as of this commit, but that is due to the
problem describe on apollographql#376 and the fix will be in a followup commit.
  • Loading branch information
Sylvain Lebresne committed Jan 3, 2022
1 parent 3824052 commit 91e1b17
Show file tree
Hide file tree
Showing 6 changed files with 737 additions and 23 deletions.
52 changes: 51 additions & 1 deletion composition-js/src/__tests__/validation_errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ function errorMessages(r: CompositionResult): string[] {
return r.errors?.map(e => e.message) ?? [];
}

describe('composition', () => {
describe('@requires', () => {
it('fails if it cannot satisfy a @requires', () => {
const subgraphA = {
name: 'A',
Expand Down Expand Up @@ -61,4 +61,54 @@ describe('composition', () => {
`
]);
});

it('fails if it no usable post-@requires keys', () => {
const subgraphA = {
name: 'A',
typeDefs: gql`
type T1 @key(fields: "id") {
id: Int!
f1: String
}
`
};

const subgraphB = {
name: 'B',
typeDefs: gql`
type Query {
getT1s: [T1]
}
type T1 {
id: Int!
f1: String @external
f2: T2! @requires(fields: "f1")
}
type T2 {
a: String
}
`
};

const result = composeServices([subgraphA, subgraphB]);
expect(result.errors).toBeDefined();
expect(errorMessages(result)).toMatchStringArray([
`
The follow supergraph API query:
{
getT1s {
f2 {
...
}
}
}
cannot be satisfied by the subgraphs because:
- from subgraph "B": @require condition on field "T1.f2" can be satisfied but missing usable key on "T1" in subgraph "B" to resume query.
- from subgraph "A": cannot find field "T1.f2".
`
]);
});

});
Loading

0 comments on commit 91e1b17

Please sign in to comment.