Skip to content

Commit f85c093

Browse files
shYkiStoardatan
andauthored
fix(utils): prevent race conditions when validating documents (#5795)
* fix(utils): prevent race conditions when validating documents * Changeset --------- Co-authored-by: Arda TANRIKULU <ardatanrikulu@gmail.com>
1 parent 8770be9 commit f85c093

File tree

3 files changed

+67
-5
lines changed

3 files changed

+67
-5
lines changed

.changeset/twenty-walls-sleep.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@graphql-tools/utils': patch
3+
---
4+
5+
prevent race conditions when validating documents

packages/utils/src/validate-documents.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,20 @@ export function validateGraphQlDocuments(
1717
documents: DocumentNode[],
1818
rules: ValidationRule[] = createDefaultRules(),
1919
) {
20-
const definitionMap = new Map<string, DefinitionNode>();
20+
const definitions = new Set<DefinitionNode>();
21+
const fragmentsDefinitionsMap = new Map<string, DefinitionNode>();
2122
for (const document of documents) {
2223
for (const docDefinition of document.definitions) {
23-
if ('name' in docDefinition && docDefinition.name) {
24-
definitionMap.set(`${docDefinition.kind}_${docDefinition.name.value}`, docDefinition);
24+
if (docDefinition.kind === Kind.FRAGMENT_DEFINITION) {
25+
fragmentsDefinitionsMap.set(docDefinition.name.value, docDefinition);
2526
} else {
26-
definitionMap.set(Date.now().toString(), docDefinition);
27+
definitions.add(docDefinition);
2728
}
2829
}
2930
}
3031
const fullAST: DocumentNode = {
3132
kind: Kind.DOCUMENT,
32-
definitions: Array.from(definitionMap.values()),
33+
definitions: Array.from([...definitions, ...fragmentsDefinitionsMap.values()]),
3334
};
3435
const errors = validate(schema, fullAST, rules);
3536
for (const error of errors) {

packages/utils/tests/validate-documents.spec.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,62 @@ describe('validateGraphQlDocuments', () => {
5959
at packages/client/src/pages/search/searchPage.query.graphql:6:15`);
6060
});
6161

62+
it('Should throw a validation error when multiple anonymous queries are present', async () => {
63+
const schema = buildSchema(/* GraphQL */ `
64+
type OtherStuff {
65+
foo: String
66+
}
67+
68+
type Pizzeria {
69+
id: Int
70+
name: String
71+
location: String
72+
}
73+
74+
type Query {
75+
otherStuff: OtherStuff
76+
pizzeria: Pizzeria
77+
}
78+
`);
79+
80+
const result = validateGraphQlDocuments(schema, [
81+
parse(
82+
new Source(
83+
/* GraphQL */ `
84+
query {
85+
pizzeria {
86+
id
87+
name
88+
}
89+
}
90+
`,
91+
'packages/client/src/pages/products/productPage.query.graphql',
92+
),
93+
),
94+
parse(
95+
new Source(
96+
/* GraphQL */ `
97+
query {
98+
otherStuff {
99+
foo
100+
}
101+
}
102+
`,
103+
'packages/client/src/pages/search/searchPage.query.graphql',
104+
),
105+
),
106+
]);
107+
108+
expect(result).toHaveLength(2);
109+
expect(result[0].source?.name).toBe(
110+
'packages/client/src/pages/products/productPage.query.graphql',
111+
);
112+
expect(result[0] instanceof GraphQLError).toBeTruthy();
113+
expect(result[0].message).toBe('This anonymous operation must be the only defined operation.');
114+
expect(result[0].stack).toBe(`This anonymous operation must be the only defined operation.
115+
at packages/client/src/pages/products/productPage.query.graphql:2:13`);
116+
});
117+
62118
it('Should not swallow fragments on operation/fragment name conflict', async () => {
63119
const schema = buildSchema(/* GraphQL */ `
64120
type Query {

0 commit comments

Comments
 (0)