Skip to content

Commit

Permalink
Fixes issue where there can be naming collisions in contextual parame…
Browse files Browse the repository at this point in the history
…ter names (#3155)

Subgraph number set for a contextual paramter was colliding if used in
multiple subgraphs
  • Loading branch information
clenfest authored Sep 25, 2024
1 parent 8520c7d commit e1e2605
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 13 deletions.
6 changes: 6 additions & 0 deletions .changeset/tame-paws-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@apollo/query-planner": patch
"@apollo/query-graphs": patch
---

Fixes issue where contextual parameters can have naming collisions if used in multiple subgraphs
27 changes: 16 additions & 11 deletions query-graphs-js/src/querygraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -934,17 +934,6 @@ function federateSubgraphs(supergraph: Schema, subgraphs: QueryGraph[]): QueryGr
}
}

for (const [subgraphName, args] of subgraphToArgs) {
args.sort();
const argToIndex = new Map();
for (let idx=0; idx < args.length; idx++) {
argToIndex.set(args[idx], `contextualArgument_${i}_${idx}`);
}
subgraphToArgIndices.set(subgraphName, argToIndex);
}

builder.setContextMaps(subgraphToArgs, subgraphToArgIndices);

simpleTraversal(
subgraph,
_v => { return undefined; },
Expand All @@ -965,6 +954,22 @@ function federateSubgraphs(supergraph: Schema, subgraphs: QueryGraph[]): QueryGr

}

// add contextual argument maps to builder
for (const [i, subgraph] of subgraphs.entries()) {
const subgraphName = subgraph.name;
const args = subgraphToArgs.get(subgraph.name);
if (args) {
args.sort();
const argToIndex = new Map();
for (let idx=0; idx < args.length; idx++) {
argToIndex.set(args[idx], `contextualArgument_${i+1}_${idx}`);
}
subgraphToArgIndices.set(subgraphName, argToIndex);
}
}

builder.setContextMaps(subgraphToArgs, subgraphToArgIndices);

// Now we handle @provides
let provideId = 0;
for (const [i, subgraph] of subgraphs.entries()) {
Expand Down
4 changes: 2 additions & 2 deletions query-planner-js/src/__tests__/buildPlan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9164,7 +9164,7 @@ describe('@fromContext impacts on query planning', () => {
} =>
{
... on U {
field(a: $contextualArgument_1_0)
field(a: $contextualArgument_2_0)
}
}
},
Expand All @@ -9176,7 +9176,7 @@ describe('@fromContext impacts on query planning', () => {
{
kind: 'KeyRenamer',
path: ['..', '... on T', 'prop'],
renameKeyTo: 'contextualArgument_1_0',
renameKeyTo: 'contextualArgument_2_0',
},
]);
});
Expand Down

0 comments on commit e1e2605

Please sign in to comment.