Skip to content

Commit 93b222e

Browse files
Mayank1791989Mayank1791989
committed
fix(query validation): fix sub field error coming even for scalar fields.
upgrading to graphql v0.9.3 broke the patched ScalarLeaf rule. As there are no test cases for query validation CI build doesnt failed. changes - change patched rules to wrap original rule and change only part required. This will reduce syncing rules with original. - add test cases for query validation.
1 parent 0c0030d commit 93b222e

File tree

4 files changed

+185
-76
lines changed

4 files changed

+185
-76
lines changed

‎src/query/validation/__tests__/__snapshots__/validate.test.js.snap‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`Rule: ArgumentsOfCorrectType report error for wrong type 1`] = `
4+
Array [
5+
Object {
6+
"locations": Array [
7+
Object {
8+
"column": 23,
9+
"line": 4,
10+
"path": "GraphQL",
11+
},
12+
],
13+
"message": "Argument \\"size\\" has invalid value
14+
Expected type \\"Int\\", found \\"some_string_value\\". (ArgumentsOfCorrectType)",
15+
"severity": "error",
16+
},
17+
]
18+
`;
19+
20+
exports[`Rule: ProvidedNonNullArguments allow mutation field without arguments 1`] = `Array []`;
21+
22+
exports[`Rule: ProvidedNonNullArguments report error if field arguments missing 1`] = `
23+
Array [
24+
Object {
25+
"locations": Array [
26+
Object {
27+
"column": 11,
28+
"line": 4,
29+
"path": "GraphQL",
30+
},
31+
],
32+
"message": "Field \\"image\\" argument \\"size\\" of type \\"Int!\\" is required but not provided. (ProvidedNonNullArguments)",
33+
"severity": "error",
34+
},
35+
]
36+
`;
37+
38+
exports[`Rule: ScalarLeafs allow field without subselection in fragments if @relay(pattern: true) is present 1`] = `Array []`;
39+
40+
exports[`Rule: ScalarLeafs allow mutation without subselection 1`] = `Array []`;
41+
42+
exports[`Rule: ScalarLeafs allow query without subselection 1`] = `Array []`;
43+
44+
exports[`Rule: ScalarLeafs report error for missing subselection in fragments 1`] = `
45+
Array [
46+
Object {
47+
"locations": Array [
48+
Object {
49+
"column": 11,
50+
"line": 4,
51+
"path": "GraphQL",
52+
},
53+
],
54+
"message": "Field \\"me\\" of type \\"Player!\\" must have a selection of subfields. Did you mean \\"me { ... }\\"? (ScalarLeafs)",
55+
"severity": "error",
56+
},
57+
]
58+
`;
59+
60+
exports[`should not report errors for valid query 1`] = `Array []`;

‎src/query/validation/__tests__/validate.test.js‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/* @flow */
2+
import { getSchema } from '../../../__test-data__/utils';
3+
import parseQuery from '../../_shared/parseQuery';
4+
import { Source } from 'graphql/language/source';
5+
import validate from '../validate';
6+
import invariant from 'invariant';
7+
8+
function validateSource(source: string) {
9+
const schema = getSchema();
10+
const { ast } = parseQuery(new Source(source), {
11+
isRelay: true,
12+
parser: ['EmbeddedQueryParser', { startTag: 'Relay\\.QL`', endTag: '`' }],
13+
});
14+
invariant(ast, '[unexpected error] ast should be defined here');
15+
return validate(schema, ast, { isRelay: true });
16+
}
17+
18+
describe('Rule: ScalarLeafs', () => {
19+
it('report error for missing subselection in fragments', () => {
20+
const errors = validateSource(`
21+
Relay.QL\`
22+
fragment on Viewer {
23+
me
24+
}
25+
\`
26+
`);
27+
expect(errors).toMatchSnapshot();
28+
});
29+
30+
it('allow field without subselection in fragments if @relay(pattern: true) is present', () => {
31+
const errors = validateSource(`
32+
Relay.QL\`
33+
fragment on PlayerCreatePayload @relay(pattern: true) {
34+
player
35+
}
36+
\`
37+
`);
38+
expect(errors).toMatchSnapshot();
39+
});
40+
41+
it('allow query without subselection', () => {
42+
const errors = validateSource(`
43+
Relay.QL\`
44+
query { viewer }
45+
\`
46+
`);
47+
expect(errors).toMatchSnapshot();
48+
});
49+
50+
it('allow mutation without subselection', () => {
51+
const errors = validateSource(`
52+
Relay.QL\`
53+
mutation { PlayerCreate }
54+
\`
55+
`);
56+
expect(errors).toMatchSnapshot();
57+
});
58+
});
59+
60+
describe('Rule: ArgumentsOfCorrectType', () => {
61+
it('report error for wrong type', () => {
62+
const errors = validateSource(`
63+
Relay.QL\`
64+
fragment on Player {
65+
image(size: "some_string_value")
66+
}
67+
\`
68+
`);
69+
expect(errors).toMatchSnapshot();
70+
});
71+
});
72+
73+
describe('Rule: ProvidedNonNullArguments', () => {
74+
it('report error if field arguments missing', () => {
75+
const errors = validateSource(`
76+
Relay.QL\`
77+
fragment on Player {
78+
image
79+
}
80+
\`
81+
`);
82+
expect(errors).toMatchSnapshot();
83+
});
84+
85+
it('allow mutation field without arguments', () => {
86+
const errors = validateSource(`
87+
Relay.QL\`
88+
mutation { PlayerCreate }
89+
\`
90+
`);
91+
expect(errors).toMatchSnapshot();
92+
});
93+
});
94+
95+
test('should not report errors for valid query', () => {
96+
const errors = validateSource(`
97+
Relay.QL\`
98+
fragment on Player {
99+
name
100+
id
101+
}
102+
\`
103+
`);
104+
expect(errors).toMatchSnapshot();
105+
});

‎src/query/validation/rules/gql-rules-query-relay/ProvidedNonNullArguments.js‎

Lines changed: 8 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,15 @@
11
/* @flow */
2-
32
/**
43
* NOTE: This is patched version of graphql ProvideNonNullArguments rule
54
* disable check for mutation as relay manages input variables
65
* using differenty syntax (getVariables function)
76
* in relay `mutation { someMutation }` is valid
87
* but in graphql `mutation { someMutation(input: $input) { } }` is valid
98
*/
10-
11-
import { GraphQLError } from 'graphql/error';
12-
import keyMap from 'graphql/jsutils/keyMap';
13-
import { GraphQLNonNull } from 'graphql/type/definition';
14-
159
import {
1610
missingFieldArgMessage,
1711
missingDirectiveArgMessage,
12+
ProvidedNonNullArguments as GraphQLProvidedNonNullArguments,
1813
} from 'graphql/validation/rules/ProvidedNonNullArguments';
1914

2015
export {
@@ -29,63 +24,20 @@ export {
2924
* have been provided.
3025
*/
3126
export function ProvidedNonNullArguments(context: any): any {
27+
const origProvidedNonNullArguments = GraphQLProvidedNonNullArguments(context);
28+
3229
return {
30+
...origProvidedNonNullArguments,
31+
3332
Field: {
3433
// Validate on leave to allow for deeper errors to appear first.
35-
leave(fieldAST) { // eslint-disable-line
34+
leave(node) { // eslint-disable-line
3635
const parentType = context.getParentType();
37-
// Patch
38-
// for mutation ignore check
36+
// Patch: ignore check for mutations
3937
if (parentType && parentType.name === 'Mutation') {
4038
return false;
4139
}
42-
43-
const fieldDef = context.getFieldDef();
44-
if (!fieldDef) {
45-
return false;
46-
}
47-
const argASTs = fieldAST.arguments || [];
48-
49-
const argASTMap = keyMap(argASTs, arg => arg.name.value);
50-
fieldDef.args.forEach((argDef) => {
51-
const argAST = argASTMap[argDef.name];
52-
if (!argAST && argDef.type instanceof GraphQLNonNull) {
53-
context.reportError(new GraphQLError(
54-
missingFieldArgMessage(
55-
fieldAST.name.value,
56-
argDef.name,
57-
argDef.type,
58-
),
59-
[fieldAST],
60-
));
61-
}
62-
});
63-
},
64-
},
65-
66-
Directive: {
67-
// Validate on leave to allow for deeper errors to appear first.
68-
leave(directiveAST) { // eslint-disable-line
69-
const directiveDef = context.getDirective();
70-
if (!directiveDef) {
71-
return false;
72-
}
73-
const argASTs = directiveAST.arguments || [];
74-
75-
const argASTMap = keyMap(argASTs, arg => arg.name.value);
76-
directiveDef.args.forEach((argDef) => {
77-
const argAST = argASTMap[argDef.name];
78-
if (!argAST && argDef.type instanceof GraphQLNonNull) {
79-
context.reportError(new GraphQLError(
80-
missingDirectiveArgMessage(
81-
directiveAST.name.value,
82-
argDef.name,
83-
argDef.type,
84-
),
85-
[directiveAST],
86-
));
87-
}
88-
});
40+
origProvidedNonNullArguments.Field.leave(node);
8941
},
9042
},
9143
};

‎src/query/validation/rules/gql-rules-query-relay/ScalarLeafs.js‎

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,9 @@
11
/* @flow */
22
// patched ScalarLeaf
3-
import {
4-
isLeafType,
5-
GraphQLError,
6-
} from 'graphql';
7-
83
import {
94
noSubselectionAllowedMessage,
105
requiredSubselectionMessage,
6+
ScalarLeafs as GraphqlScalarLeafs,
117
} from 'graphql/validation/rules/ScalarLeafs';
128

139
import { ValidationContext } from 'graphql/validation';
@@ -45,27 +41,23 @@ function ignore(node, ancestors) {
4541
(_node.kind === 'FragmentDefinition' && isFragmentPattern(_node))
4642
));
4743
return Boolean(found);
48-
// return false;
4944
}
5045

5146
export function ScalarLeafs(context: ValidationContext): any {
47+
const origScalarLeafs = GraphqlScalarLeafs(context);
5248
return {
53-
Field(node, key, parent, path, ancestors) {
49+
...origScalarLeafs,
50+
Field(...args) {
51+
const [
52+
node,
53+
key, // eslint-disable-line
54+
parent, // eslint-disable-line
55+
path, // eslint-disable-line
56+
ancestors,
57+
] = args;
5458
const type = context.getType();
5559
if (type && !ignore(node, ancestors)) {
56-
if (isLeafType(type)) {
57-
if (node.selectionSet) {
58-
context.reportError(new GraphQLError(
59-
noSubselectionAllowedMessage(node.name.value, type),
60-
[node.selectionSet],
61-
));
62-
}
63-
} else if (!node.selectionSet) {
64-
context.reportError(new GraphQLError(
65-
requiredSubselectionMessage(node.name.value, type),
66-
[node],
67-
));
68-
}
60+
origScalarLeafs.Field(...args);
6961
}
7062
},
7163
};

0 commit comments

Comments
 (0)