Skip to content

Commit

Permalink
Fix incorrectly rejected composition and subgraph issues with `@inter…
Browse files Browse the repository at this point in the history
…faceObject` (#2494)

There is 2 main issues with the handling of `@interfaceObject` that this
patch fixes:
1. given some `@interfaceObject` type `I` in some subgraph `S1`, if
   another subgraph `S2` was declaring an _implementation_ type `T` of
   `I` but _without_ declaring `I`, then the code missed creating an
   "edge" between `T` in `S2` and `I` in `S1`, which resulted in
   composition rejecting such cases, even though it should work.
2. there were an additional issue in the handling of `@requires` so
   that the "input rewrites" generated were not generic enough, leading
   to sometimes sending incorrect queries to subgraph (more precisely,
   we were sometimes not rewriting the typename when querying an
   `@interfaceObject` subgraph, leading that subgraph to complain that
   it is asked for type it doesn't know).

Note that fixing those 2 issues surfaced the fact that the handling
of "rewrites" in the gateway was not working exactly as it should have.
More precisely, hen a path had a fragment `... on I`, if `I` was an
interface, then we would no select objects that implements `I`
correctly. As it happens, the router implementation of those rewrites
was both cleaner and didn't had that issue, so this patch also updates
the handling of "rewrites" to mimick what the router implementation
does (fixing the issue, and overall cleaning the code).

Fixes #2485
  • Loading branch information
Sylvain Lebresne authored and Sylvain Lebresne committed May 16, 2023
1 parent ce0459a commit 11f2d7c
Show file tree
Hide file tree
Showing 10 changed files with 525 additions and 198 deletions.
11 changes: 11 additions & 0 deletions .changeset/hungry-berries-destroy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@apollo/query-planner": patch
"@apollo/query-graphs": patch
"@apollo/federation-internals": patch
"@apollo/gateway": patch
---

Fix issues (incorrectly rejected composition and/or subgraph errors) with `@interfaceObject`. Those issues may occur
either due to some use of `@requires` in an `@interfaceObject` type, or when some subgraph `S` defines a type that is an
implementation of an interface `I` in the supergraph, and there is an `@interfaceObject` for `I` in another subgraph,
but `S` does not itself defines `I`.
263 changes: 263 additions & 0 deletions gateway-js/src/__tests__/executeQueryPlan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4587,6 +4587,269 @@ describe('executeQueryPlan', () => {
}
`);
});

test('handles querying @interfaceObject from a specific implementation', async () => {
const s1 = {
name: 's1',
typeDefs: gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key"])
type Query {
ts: [T!]!
}
interface I {
id: ID!
}
type T implements I @key(fields: "id") {
id: ID!
}
`,
resolvers: {
Query: {
ts: () => [ { id: '2' }, { id: '4' }, { id: '1' } ]
},
}
}

const s2 = {
name: 's2',
typeDefs: gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key", "@interfaceObject", "@external", "@requires"])
type I @key(fields: "id") @interfaceObject {
id: ID!
v: String
}
`,
resolvers: {
I: {
__resolveReference(ref: any) {
return {
...ref,
v: `id=${ref.id}`
};
},
}
}
}

const { serviceMap, schema, queryPlanner} = getFederatedTestingSchema([ s1, s2 ]);

let operation = parseOp(`
{
ts {
v
}
}
`, schema);

let queryPlan = buildPlan(operation, queryPlanner);
const expectedPlan = `
QueryPlan {
Sequence {
Fetch(service: "s1") {
{
ts {
__typename
id
}
}
},
Flatten(path: "ts.@") {
Fetch(service: "s2") {
{
... on T {
__typename
id
}
} =>
{
... on I {
v
}
}
},
},
},
}
`;
expect(queryPlan).toMatchInlineSnapshot(expectedPlan);

let response = await executePlan(queryPlan, operation, undefined, schema, serviceMap);
expect(response.data).toMatchInlineSnapshot(`
Object {
"ts": Array [
Object {
"v": "id=2",
},
Object {
"v": "id=4",
},
Object {
"v": "id=1",
},
],
}
`);
});

test('handles querying @interfaceObject from a specific implementation (even when the subgraph does not have the corresponding interface)', async () => {
const s1 = {
name: 's1',
typeDefs: gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key"])
type Query {
ts: [T!]!
}
type T @key(fields: "id", resolvable: false) {
id: ID!
}
`,
resolvers: {
Query: {
ts: () => [ { id: '2' }, { id: '4' }, { id: '1' } ]
},
}
}

const s2 = {
name: 's2',
typeDefs: gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key", "@interfaceObject"])
interface I @key(fields: "id") {
id: ID!
required: String
}
type T implements I @key(fields: "id") {
id: ID!
required: String
}
`,
resolvers: {
I: {
__resolveReference(ref: any) {
return [
{ id: '1', __typename: "T", required: "r1" },
{ id: '2', __typename: "T", required: "r2" },
{ id: '3', __typename: "T", required: "r3" },
{ id: '4', __typename: "T", required: "r4" },
].find(({id}) => id === ref.id);
},
},
}
}

const s3 = {
name: 's3',
typeDefs: gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key", "@interfaceObject", "@external", "@requires"])
type I @key(fields: "id") @interfaceObject {
id: ID!
required: String @external
v: String @requires(fields: "required")
}
`,
resolvers: {
I: {
__resolveReference(ref: any) {
return {
...ref,
v: `req=${ref.required}`
};
},
}
}
}

const { serviceMap, schema, queryPlanner} = getFederatedTestingSchema([ s1, s2, s3 ]);

let operation = parseOp(`
{
ts {
v
}
}
`, schema);

let queryPlan = buildPlan(operation, queryPlanner);
const expectedPlan = `
QueryPlan {
Sequence {
Fetch(service: "s1") {
{
ts {
__typename
id
}
}
},
Flatten(path: "ts.@") {
Fetch(service: "s2") {
{
... on I {
__typename
id
}
} =>
{
... on I {
__typename
required
}
}
},
},
Flatten(path: "ts.@") {
Fetch(service: "s3") {
{
... on I {
__typename
required
id
}
} =>
{
... on I {
v
}
}
},
},
},
}
`;
expect(queryPlan).toMatchInlineSnapshot(expectedPlan);

let response = await executePlan(queryPlan, operation, undefined, schema, serviceMap);
expect(response.data).toMatchInlineSnapshot(`
Object {
"ts": Array [
Object {
"v": "req=r2",
},
Object {
"v": "req=r4",
},
Object {
"v": "req=r1",
},
],
}
`);
});
});

describe('fields with conflicting types needing aliasing', () => {
Expand Down

0 comments on commit 11f2d7c

Please sign in to comment.