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 committed Mar 30, 2023
1 parent 0ba5ce6 commit 450b957
Show file tree
Hide file tree
Showing 10 changed files with 524 additions and 198 deletions.
11 changes: 11 additions & 0 deletions .changeset/hungry-berries-destroy.md
@@ -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
Expand Up @@ -4600,6 +4600,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.4", 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.4", 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.4", 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.4", 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.4", 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 450b957

Please sign in to comment.