Skip to content

Commit 38760a9

Browse files
[RFC]SDL validation (#1438)
Discussed in #1389
1 parent d40e418 commit 38760a9

13 files changed

+373
-137
lines changed

src/type/__tests__/validation-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ describe('Type System: Input Objects must have fields', () => {
713713
schema = extendSchema(
714714
schema,
715715
parse(`
716-
directive @test on ENUM
716+
directive @test on INPUT_OBJECT
717717
718718
extend input SomeInputObject @test
719719
`),

src/utilities/__tests__/buildASTSchema-test.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,25 @@ describe('Schema Builder', () => {
787787
const errors = validateSchema(schema);
788788
expect(errors.length).to.equal(0);
789789
});
790+
791+
it('Rejects invalid SDL', () => {
792+
const doc = parse(`
793+
type Query {
794+
foo: String @unknown
795+
}
796+
`);
797+
expect(() => buildASTSchema(doc)).to.throw('Unknown directive "unknown".');
798+
});
799+
800+
it('Allows to disable SDL validation', () => {
801+
const body = `
802+
type Query {
803+
foo: String @unknown
804+
}
805+
`;
806+
buildSchema(body, { assumeValid: true });
807+
buildSchema(body, { assumeValidSDL: true });
808+
});
790809
});
791810

792811
describe('Failures', () => {

src/utilities/__tests__/extendSchema-test.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,23 @@ describe('extendSchema', () => {
10181018
expect(isScalarType(arg1.type)).to.equal(true);
10191019
});
10201020

1021+
it('Rejects invalid SDL', () => {
1022+
const sdl = `
1023+
extend schema @unknown
1024+
`;
1025+
expect(() => extendTestSchema(sdl)).to.throw(
1026+
'Unknown directive "unknown".',
1027+
);
1028+
});
1029+
1030+
it('Allows to disable SDL validation', () => {
1031+
const sdl = `
1032+
extend schema @unknown
1033+
`;
1034+
extendTestSchema(sdl, { assumeValid: true });
1035+
extendTestSchema(sdl, { assumeValidSDL: true });
1036+
});
1037+
10211038
it('does not allow replacing a default directive', () => {
10221039
const sdl = `
10231040
directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD

src/utilities/buildASTSchema.js

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import keyMap from '../jsutils/keyMap';
1111
import keyValMap from '../jsutils/keyValMap';
1212
import type { ObjMap } from '../jsutils/ObjMap';
1313
import { valueFromAST } from './valueFromAST';
14+
import { assertValidSDL } from '../validation/validate';
1415
import blockStringValue from '../language/blockStringValue';
1516
import { TokenKind } from '../language/lexer';
1617
import { parse } from '../language/parser';
@@ -86,6 +87,13 @@ export type BuildSchemaOptions = {
8687
* Default: false
8788
*/
8889
commentDescriptions?: boolean,
90+
91+
/**
92+
* Set to true to assume the SDL is valid.
93+
*
94+
* Default: false
95+
*/
96+
assumeValidSDL?: boolean,
8997
};
9098

9199
/**
@@ -112,6 +120,10 @@ export function buildASTSchema(
112120
throw new Error('Must provide a document ast.');
113121
}
114122

123+
if (!options || !(options.assumeValid || options.assumeValidSDL)) {
124+
assertValidSDL(ast);
125+
}
126+
115127
let schemaDef: ?SchemaDefinitionNode;
116128

117129
const typeDefs: Array<TypeDefinitionNode> = [];
@@ -121,9 +133,6 @@ export function buildASTSchema(
121133
const d = ast.definitions[i];
122134
switch (d.kind) {
123135
case Kind.SCHEMA_DEFINITION:
124-
if (schemaDef) {
125-
throw new Error('Must provide only one schema definition.');
126-
}
127136
schemaDef = d;
128137
break;
129138
case Kind.SCALAR_TYPE_DEFINITION:

src/utilities/extendSchema.js

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import keyMap from '../jsutils/keyMap';
1212
import keyValMap from '../jsutils/keyValMap';
1313
import objectValues from '../jsutils/objectValues';
1414
import { ASTDefinitionBuilder } from './buildASTSchema';
15+
import { assertValidSDLExtension } from '../validation/validate';
1516
import { GraphQLError } from '../error/GraphQLError';
1617
import { isSchema, GraphQLSchema } from '../type/schema';
1718
import { isIntrospectionType } from '../type/introspection';
@@ -67,6 +68,13 @@ type Options = {|
6768
* Default: false
6869
*/
6970
commentDescriptions?: boolean,
71+
72+
/**
73+
* Set to true to assume the SDL is valid.
74+
*
75+
* Default: false
76+
*/
77+
assumeValidSDL?: boolean,
7078
|};
7179

7280
/**
@@ -99,6 +107,10 @@ export function extendSchema(
99107
'Must provide valid Document AST',
100108
);
101109

110+
if (!options || !(options.assumeValid || options.assumeValidSDL)) {
111+
assertValidSDLExtension(documentAST, schema);
112+
}
113+
102114
// Collect the type definitions and extensions found in the document.
103115
const typeDefinitionMap = Object.create(null);
104116
const typeExtensionsMap = Object.create(null);
@@ -115,18 +127,6 @@ export function extendSchema(
115127
const def = documentAST.definitions[i];
116128
switch (def.kind) {
117129
case Kind.SCHEMA_DEFINITION:
118-
// Sanity check that a schema extension is not overriding the schema
119-
if (
120-
schema.astNode ||
121-
schema.getQueryType() ||
122-
schema.getMutationType() ||
123-
schema.getSubscriptionType()
124-
) {
125-
throw new GraphQLError(
126-
'Cannot define a new schema within a schema extension.',
127-
[def],
128-
);
129-
}
130130
schemaDef = def;
131131
break;
132132
case Kind.SCHEMA_EXTENSION:

src/validation/ValidationContext.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import type {
1919
FragmentSpreadNode,
2020
FragmentDefinitionNode,
2121
} from '../language/ast';
22+
import type { ASTVisitor } from '../language/visitor';
2223
import type { GraphQLSchema } from '../type/schema';
2324
import type {
2425
GraphQLInputType,
@@ -64,6 +65,21 @@ export class ASTValidationContext {
6465
}
6566
}
6667

68+
export class SDLValidationContext extends ASTValidationContext {
69+
_schema: ?GraphQLSchema;
70+
71+
constructor(ast: DocumentNode, schema?: ?GraphQLSchema): void {
72+
super(ast);
73+
this._schema = schema;
74+
}
75+
76+
getSchema(): ?GraphQLSchema {
77+
return this._schema;
78+
}
79+
}
80+
81+
export type SDLValidationRule = SDLValidationContext => ASTVisitor;
82+
6783
export class ValidationContext extends ASTValidationContext {
6884
_schema: GraphQLSchema;
6985
_typeInfo: TypeInfo;
@@ -234,3 +250,5 @@ export class ValidationContext extends ASTValidationContext {
234250
return this._typeInfo.getArgument();
235251
}
236252
}
253+
254+
export type ValidationRule = ValidationContext => ASTVisitor;

src/validation/__tests__/KnownDirectives-test.js

Lines changed: 92 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,24 @@
66
*/
77

88
import { describe, it } from 'mocha';
9-
import { expectPassesRule, expectFailsRule } from './harness';
9+
import { buildSchema } from '../../utilities';
10+
import {
11+
expectPassesRule,
12+
expectFailsRule,
13+
expectSDLErrorsFromRule,
14+
} from './harness';
15+
1016
import {
1117
KnownDirectives,
1218
unknownDirectiveMessage,
1319
misplacedDirectiveMessage,
1420
} from '../rules/KnownDirectives';
1521

22+
const expectSDLErrors = expectSDLErrorsFromRule.bind(
23+
undefined,
24+
KnownDirectives,
25+
);
26+
1627
function unknownDirective(directiveName, line, column) {
1728
return {
1829
message: unknownDirectiveMessage(directiveName),
@@ -27,6 +38,20 @@ function misplacedDirective(directiveName, placement, line, column) {
2738
};
2839
}
2940

41+
const schemaWithSDLDirectives = buildSchema(`
42+
directive @onSchema on SCHEMA
43+
directive @onScalar on SCALAR
44+
directive @onObject on OBJECT
45+
directive @onFieldDefinition on FIELD_DEFINITION
46+
directive @onArgumentDefinition on ARGUMENT_DEFINITION
47+
directive @onInterface on INTERFACE
48+
directive @onUnion on UNION
49+
directive @onEnum on ENUM
50+
directive @onEnumValue on ENUM_VALUE
51+
directive @onInputObject on INPUT_OBJECT
52+
directive @onInputFieldDefinition on INPUT_FIELD_DEFINITION
53+
`);
54+
3055
describe('Validate: Known directives', () => {
3156
it('with no directives', () => {
3257
expectPassesRule(
@@ -138,10 +163,36 @@ describe('Validate: Known directives', () => {
138163
);
139164
});
140165

141-
describe('within schema language', () => {
166+
describe('within SDL', () => {
167+
it('with directive defined inside SDL', () => {
168+
expectSDLErrors(`
169+
type Query {
170+
foo: String @test
171+
}
172+
173+
directive @test on FIELD_DEFINITION
174+
`).to.deep.equal([]);
175+
});
176+
177+
it('with standard directive', () => {
178+
expectSDLErrors(`
179+
type Query {
180+
foo: String @deprecated
181+
}
182+
`).to.deep.equal([]);
183+
});
184+
185+
it('with overrided standard directive', () => {
186+
expectSDLErrors(`
187+
schema @deprecated {
188+
query: Query
189+
}
190+
directive @deprecated on SCHEMA
191+
`).to.deep.equal([]);
192+
});
193+
142194
it('with well placed directives', () => {
143-
expectPassesRule(
144-
KnownDirectives,
195+
expectSDLErrors(
145196
`
146197
type MyObj implements MyInterface @onObject {
147198
myField(myArg: Int @onArgumentDefinition): String @onFieldDefinition
@@ -180,13 +231,13 @@ describe('Validate: Known directives', () => {
180231
}
181232
182233
extend schema @onSchema
183-
`,
184-
);
234+
`,
235+
schemaWithSDLDirectives,
236+
).to.deep.equal([]);
185237
});
186238

187239
it('with misplaced directives', () => {
188-
expectFailsRule(
189-
KnownDirectives,
240+
expectSDLErrors(
190241
`
191242
type MyObj implements MyInterface @onInterface {
192243
myField(myArg: Int @onInputFieldDefinition): String @onInputFieldDefinition
@@ -213,49 +264,39 @@ describe('Validate: Known directives', () => {
213264
}
214265
215266
extend schema @onObject
216-
`,
217-
[
218-
misplacedDirective('onInterface', 'OBJECT', 2, 43),
219-
misplacedDirective(
220-
'onInputFieldDefinition',
221-
'ARGUMENT_DEFINITION',
222-
3,
223-
30,
224-
),
225-
misplacedDirective(
226-
'onInputFieldDefinition',
227-
'FIELD_DEFINITION',
228-
3,
229-
63,
230-
),
231-
misplacedDirective('onEnum', 'SCALAR', 6, 25),
232-
misplacedDirective('onObject', 'INTERFACE', 8, 31),
233-
misplacedDirective(
234-
'onInputFieldDefinition',
235-
'ARGUMENT_DEFINITION',
236-
9,
237-
30,
238-
),
239-
misplacedDirective(
240-
'onInputFieldDefinition',
241-
'FIELD_DEFINITION',
242-
9,
243-
63,
244-
),
245-
misplacedDirective('onEnumValue', 'UNION', 12, 23),
246-
misplacedDirective('onScalar', 'ENUM', 14, 21),
247-
misplacedDirective('onUnion', 'ENUM_VALUE', 15, 20),
248-
misplacedDirective('onEnum', 'INPUT_OBJECT', 18, 23),
249-
misplacedDirective(
250-
'onArgumentDefinition',
251-
'INPUT_FIELD_DEFINITION',
252-
19,
253-
24,
254-
),
255-
misplacedDirective('onObject', 'SCHEMA', 22, 16),
256-
misplacedDirective('onObject', 'SCHEMA', 26, 23),
257-
],
258-
);
267+
`,
268+
schemaWithSDLDirectives,
269+
).to.deep.equal([
270+
misplacedDirective('onInterface', 'OBJECT', 2, 43),
271+
misplacedDirective(
272+
'onInputFieldDefinition',
273+
'ARGUMENT_DEFINITION',
274+
3,
275+
30,
276+
),
277+
misplacedDirective('onInputFieldDefinition', 'FIELD_DEFINITION', 3, 63),
278+
misplacedDirective('onEnum', 'SCALAR', 6, 25),
279+
misplacedDirective('onObject', 'INTERFACE', 8, 31),
280+
misplacedDirective(
281+
'onInputFieldDefinition',
282+
'ARGUMENT_DEFINITION',
283+
9,
284+
30,
285+
),
286+
misplacedDirective('onInputFieldDefinition', 'FIELD_DEFINITION', 9, 63),
287+
misplacedDirective('onEnumValue', 'UNION', 12, 23),
288+
misplacedDirective('onScalar', 'ENUM', 14, 21),
289+
misplacedDirective('onUnion', 'ENUM_VALUE', 15, 20),
290+
misplacedDirective('onEnum', 'INPUT_OBJECT', 18, 23),
291+
misplacedDirective(
292+
'onArgumentDefinition',
293+
'INPUT_FIELD_DEFINITION',
294+
19,
295+
24,
296+
),
297+
misplacedDirective('onObject', 'SCHEMA', 22, 16),
298+
misplacedDirective('onObject', 'SCHEMA', 26, 23),
299+
]);
259300
});
260301
});
261302
});

0 commit comments

Comments
 (0)