Skip to content

Commit

Permalink
Add missing query graph edge for interfaces implementing interfaces (#…
Browse files Browse the repository at this point in the history
…3122)

During `addAdditionalAbstractTypeEdges()`, we add edges between abstract
types, but we specifically were skipping edges between interfaces
implementing interfaces because that function expected it to be added
already by `addAbstractTypeEdges()`. However, the latter function only
handles edges between concrete (object) and abstract types. This PR
accordingly updates `addAdditionalAbstractTypeEdges()` to stop skipping
interfaces-implementing-interfaces edges, and updates tests accordingly.
  • Loading branch information
sachindshinde authored Aug 21, 2024
1 parent b91e81a commit acfe319
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 16 deletions.
6 changes: 6 additions & 0 deletions .changeset/cuddly-badgers-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@apollo/gateway": patch
"@apollo/query-graphs": patch
---

Avoid type explosion for inline fragments where the type condition is an interface that implements the parent type.
12 changes: 2 additions & 10 deletions gateway-js/src/__tests__/executeQueryPlan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2300,22 +2300,14 @@ describe('executeQueryPlan', () => {

queryPlan = buildPlan(operation, queryPlanner);

// TODO: we're actually type-exploding in this case because currently, as soon as we need to type-explode, we do
// so into all the runtime types, while here it could make sense to only type-explode into the direct sub-types=
// (the sub-interfaces). We should fix this (but it's only sub-optimal, not incorrect).
expect(queryPlan).toMatchInlineSnapshot(`
QueryPlan {
Fetch(service: "S1") {
{
allValues {
__typename
... on T1 {
a
}
... on T2 {
a
}
... on T4 {
... on SubInterface1 {
__typename
a
}
}
Expand Down
6 changes: 0 additions & 6 deletions query-graphs-js/src/querygraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1611,12 +1611,6 @@ class GraphBuilderFromSchema extends GraphBuilder {
for (let j = i; j < abstractTypesWithTheirRuntimeTypes.length; j++) {
const t2 = abstractTypesWithTheirRuntimeTypes[j];

// We ignore the pair if both are interfaces and one implements the other. We'll already have appropriate
// edges if that's the case.
if (isInterfaceType(t1.type) && isInterfaceType(t2.type) && (t1.type.implementsInterface(t2.type) || t2.type.implementsInterface(t1.type))) {
continue;
}

let addT1ToT2 = false;
let addT2ToT1 = false;
if (t1.type === t2.type) {
Expand Down

0 comments on commit acfe319

Please sign in to comment.