Skip to content

Commit

Permalink
generateQueryFragments error with nested selection set (#3043)
Browse files Browse the repository at this point in the history
Fixes #3042 

generateQueryFragments does't handle repeated (... on B { ... on B ...
on B } } }) fragments well. Given the query planner can actually
generate these query structures, this causes this option to be
potentially dangerous, generating invalid subgraph queries.

---------

Co-authored-by: Marc-Andre Giroux <[email protected]>
  • Loading branch information
clenfest and Marc-Andre Giroux committed Jun 20, 2024
1 parent b258406 commit b2e5ab6
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/calm-candles-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/federation-internals": patch
---

generateQueryFragments() could generate fragments with naming collisions when nested
60 changes: 60 additions & 0 deletions internals-js/src/__tests__/operations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,66 @@ function astSSet(...selections: SelectionNode[]): SelectionSetNode {
};
}

describe('generate query fragments', () => {
test('generateQueryFragments handles repeated fragment spreads', () => {
const schema = parseSchema(`
type Query {
entities: [Entity]
}
union Entity = A | B
type A {
a: Int
}
type B {
b: Int
}
`);

const operation = parseOperation(schema, `
query {
entities {
... on B {
... on B {
... on B {
... on B {
b
}
}
}
}
}
}
`);

const withGeneratedFragments = operation.generateQueryFragments();
console.log(withGeneratedFragments.toString());
expect(withGeneratedFragments.toString()).toMatchString(`
fragment _generated_onB1_0 on B {
... on B {
b
}
}
fragment _generated_onB1_1 on B {
..._generated_onB1_0
}
fragment _generated_onB1_2 on B {
..._generated_onB1_1
}
{
entities {
..._generated_onB1_2
}
}
`);
});
});

describe('fragments optimization', () => {
// Takes a query with fragments as inputs, expand all those fragments, and ensures that all the
// fragments gets optimized back, and that we get back the exact same query.
Expand Down
7 changes: 4 additions & 3 deletions internals-js/src/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1594,16 +1594,17 @@ export class SelectionSet {
// No match, so we need to create a new fragment. First, we minimize the
// selection set before creating the fragment with it.
const [minimizedSelectionSet] = selection.selectionSet.minimizeSelectionSet(namedFragments, seenSelections);
const updatedEquivalentSelectionSetCandidates = seenSelections.get(mockHashCode); // may have changed after previous statement
const fragmentDefinition = new NamedFragmentDefinition(
this.parentType.schema(),
`_generated_${mockHashCode}_${equivalentSelectionSetCandidates?.length ?? 0}`,
`_generated_${mockHashCode}_${updatedEquivalentSelectionSetCandidates?.length ?? 0}`,
selection.element.typeCondition
).setSelectionSet(minimizedSelectionSet);
namedFragments.add(fragmentDefinition);

// Create a new "hash code" bucket or add to the existing one.
if (equivalentSelectionSetCandidates) {
equivalentSelectionSetCandidates.push([selection.selectionSet, fragmentDefinition]);
if (updatedEquivalentSelectionSetCandidates) {
updatedEquivalentSelectionSetCandidates.push([selection.selectionSet, fragmentDefinition]);
} else {
seenSelections.set(mockHashCode, [[selection.selectionSet, fragmentDefinition]]);
}
Expand Down

0 comments on commit b2e5ab6

Please sign in to comment.