Skip to content

Commit 51bed5b

Browse files
fix(composition): Handle output type covariance properly in satisfiability (#3277)
This PR fixes two bugs in the satisfiability code when the subgraph type differs from the supergraph type due to output type covariance: 1. It now handles no-op inline fragments when advancing a subgraph path with a graph transition. - Previously, the code assumed this was impossible, but it can happen in the case of output type covariance where the subgraph type matches the inline fragment type, which doesn't match the supergraph type. Since we don't store no-op edges in the query graph, this led to no edge being found, making satisfiability believe the transition was mistakenly unsatisfiable. 2. The data of previous vertex visits now additionally includes the tail's type name for a given subgraph path. - Previously, the code assumed the tail's type name always matched the type name of the vertex in the key of the previous vertex visit state store. This is not the case when there's output type covariance, which led to satisfiability mistakenly believing a previous vertex visit was matching the current one (i.e. a loop's been detected) when it wasn't.
1 parent 7799ad1 commit 51bed5b

File tree

3 files changed

+17
-1
lines changed

3 files changed

+17
-1
lines changed

.changeset/ten-gorillas-boil.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@apollo/composition": patch
3+
"@apollo/query-graphs": patch
4+
---
5+
6+
Fix bug in composition where, when a field's type in a subgraph is a subtype of the field's type in the supergraph, the satisfiability validation spuriously succeeds/errors.

composition-js/src/validate.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,7 @@ export class ValidationState {
641641
for (const pathInfo of this.subgraphPathInfos) {
642642
const tailSubgraphName = pathInfo.path.path.tail.source;
643643
const tailSubgraphEnumValue = subgraphNameToGraphEnumValue.get(tailSubgraphName);
644+
const tailTypeName = pathInfo.path.path.tail.type.name;
644645
const entryKeys = [];
645646
const contexts = Array.from(pathInfo.contexts.entries());
646647
contexts.sort((a, b) => a[0].localeCompare(b[0]));
@@ -649,7 +650,7 @@ export class ValidationState {
649650
entryKeys.push(`${context}=${subgraphEnumValue}.${typeName}`);
650651
}
651652
subgraphContextKeys.add(
652-
`${tailSubgraphEnumValue}[${entryKeys.join(',')}]`
653+
`${tailSubgraphEnumValue}.${tailTypeName}[${entryKeys.join(',')}]`
653654
);
654655
}
655656
return subgraphContextKeys;

query-graphs-js/src/graphPath.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,6 +1735,15 @@ function advancePathWithDirectTransition<V extends Vertex>(
17351735
// We can now continue on dealing with the actual field.
17361736
}
17371737

1738+
if (
1739+
transition.kind === 'DownCast'
1740+
&& transition.castedType.name === path.tail.type.name
1741+
) {
1742+
// Due to output type covariance, a downcast supergraph transition may be a no-op on the
1743+
// subgraph path. In these cases, we effectively ignore the type condition.
1744+
return [path];
1745+
}
1746+
17381747
const options: GraphPath<Transition, V>[] = [];
17391748
const deadEndClosures: UnadvanceableClosure[] = [];
17401749

0 commit comments

Comments
 (0)