Skip to content

Commit 5774173

Browse files
IvanGoncharovIvanGoncharov
authored
Move validation of names into GraphQL* constructors (#3288)
1 parent 22b9504 commit 5774173

File tree

12 files changed

+257
-162
lines changed

12 files changed

+257
-162
lines changed

‎src/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@ export {
126126
/** Validate GraphQL schema. */
127127
validateSchema,
128128
assertValidSchema,
129+
/** Upholds the spec rules about naming. */
130+
assertName,
131+
assertEnumValueName,
129132
} from './type/index';
130133

131134
export type {

‎src/type/__tests__/assertName-test.ts‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { expect } from 'chai';
2+
import { describe, it } from 'mocha';
3+
4+
import { assertName, assertEnumValueName } from '../assertName';
5+
import { assertValidName } from '../../utilities/assertValidName';
6+
7+
describe('assertValidName()', () => {
8+
assertValidName('test');
9+
it('passthrough valid name', () => {
10+
expect(assertName('_ValidName123')).to.equal('_ValidName123');
11+
});
12+
13+
it('throws for non-strings', () => {
14+
// @ts-expect-error
15+
expect(() => assertName({})).to.throw('Expected name to be a string.');
16+
});
17+
18+
it('throws on empty strings', () => {
19+
expect(() => assertName('')).to.throw(
20+
'Expected name to be a non-empty string.',
21+
);
22+
});
23+
24+
it('throws for names with invalid characters', () => {
25+
expect(() => assertName('>--()-->')).to.throw(
26+
'Names must only contain [_a-zA-Z0-9] but ">--()-->" does not.',
27+
);
28+
});
29+
30+
it('throws for names starting with invalid characters', () => {
31+
expect(() => assertName('42MeaningsOfLife')).to.throw(
32+
'Names must start with [_a-zA-Z] but "42MeaningsOfLife" does not.',
33+
);
34+
});
35+
});
36+
37+
describe('assertEnumValueName()', () => {
38+
it('passthrough valid name', () => {
39+
expect(assertEnumValueName('_ValidName123')).to.equal('_ValidName123');
40+
});
41+
42+
it('throws on empty strings', () => {
43+
expect(() => assertEnumValueName('')).to.throw(
44+
'Expected name to be a non-empty string.',
45+
);
46+
});
47+
48+
it('throws for names with invalid characters', () => {
49+
expect(() => assertEnumValueName('>--()-->')).to.throw(
50+
'Names must only contain [_a-zA-Z0-9] but ">--()-->" does not.',
51+
);
52+
});
53+
54+
it('throws for names starting with invalid characters', () => {
55+
expect(() => assertEnumValueName('42MeaningsOfLife')).to.throw(
56+
'Names must start with [_a-zA-Z] but "42MeaningsOfLife" does not.',
57+
);
58+
});
59+
60+
it('throws for restricted names', () => {
61+
expect(() => assertEnumValueName('true')).to.throw(
62+
'Enum values cannot be named: true',
63+
);
64+
expect(() => assertEnumValueName('false')).to.throw(
65+
'Enum values cannot be named: false',
66+
);
67+
expect(() => assertEnumValueName('null')).to.throw(
68+
'Enum values cannot be named: null',
69+
);
70+
});
71+
});

‎src/type/__tests__/definition-test.ts‎

Lines changed: 74 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -324,9 +324,10 @@ describe('Type System: Objects', () => {
324324
expect(() => objType.getFields()).to.not.throw();
325325
});
326326

327-
it('rejects an Object type without name', () => {
328-
// @ts-expect-error
329-
expect(() => new GraphQLObjectType({})).to.throw('Must provide name.');
327+
it('rejects an Object type with invalid name', () => {
328+
expect(
329+
() => new GraphQLObjectType({ name: 'bad-name', fields: {} }),
330+
).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.');
330331
});
331332

332333
it('rejects an Object type field with undefined config', () => {
@@ -353,6 +354,18 @@ describe('Type System: Objects', () => {
353354
);
354355
});
355356

357+
it('rejects an Object type with incorrectly named fields', () => {
358+
const objType = new GraphQLObjectType({
359+
name: 'SomeObject',
360+
fields: {
361+
'bad-name': { type: ScalarType },
362+
},
363+
});
364+
expect(() => objType.getFields()).to.throw(
365+
'Names must only contain [_a-zA-Z0-9] but "bad-name" does not.',
366+
);
367+
});
368+
356369
it('rejects an Object type with a field function that returns incorrect type', () => {
357370
const objType = new GraphQLObjectType({
358371
name: 'SomeObject',
@@ -380,6 +393,23 @@ describe('Type System: Objects', () => {
380393
);
381394
});
382395

396+
it('rejects an Object type with incorrectly named field args', () => {
397+
const objType = new GraphQLObjectType({
398+
name: 'SomeObject',
399+
fields: {
400+
badField: {
401+
type: ScalarType,
402+
args: {
403+
'bad-name': { type: ScalarType },
404+
},
405+
},
406+
},
407+
});
408+
expect(() => objType.getFields()).to.throw(
409+
'Names must only contain [_a-zA-Z0-9] but "bad-name" does not.',
410+
);
411+
});
412+
383413
it('rejects an Object type with incorrectly typed interfaces', () => {
384414
const objType = new GraphQLObjectType({
385415
name: 'SomeObject',
@@ -478,9 +508,10 @@ describe('Type System: Interfaces', () => {
478508
expect(implementing.getInterfaces()).to.deep.equal([InterfaceType]);
479509
});
480510

481-
it('rejects an Interface type without name', () => {
482-
// @ts-expect-error
483-
expect(() => new GraphQLInterfaceType({})).to.throw('Must provide name.');
511+
it('rejects an Interface type with invalid name', () => {
512+
expect(
513+
() => new GraphQLInterfaceType({ name: 'bad-name', fields: {} }),
514+
).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.');
484515
});
485516

486517
it('rejects an Interface type with incorrectly typed interfaces', () => {
@@ -559,9 +590,10 @@ describe('Type System: Unions', () => {
559590
expect(unionType.getTypes()).to.deep.equal([]);
560591
});
561592

562-
it('rejects an Union type without name', () => {
563-
// @ts-expect-error
564-
expect(() => new GraphQLUnionType({})).to.throw('Must provide name.');
593+
it('rejects an Union type with invalid name', () => {
594+
expect(
595+
() => new GraphQLUnionType({ name: 'bad-name', types: [] }),
596+
).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.');
565597
});
566598

567599
it('rejects an Union type with an incorrect type for resolveType', () => {
@@ -674,11 +706,10 @@ describe('Type System: Enums', () => {
674706
expect(enumType.getValue('BAR')).has.property('value', 20);
675707
});
676708

677-
it('rejects an Enum type without name', () => {
678-
// @ts-expect-error
679-
expect(() => new GraphQLEnumType({ values: {} })).to.throw(
680-
'Must provide name.',
681-
);
709+
it('rejects an Enum type with invalid name', () => {
710+
expect(
711+
() => new GraphQLEnumType({ name: 'bad-name', values: {} }),
712+
).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.');
682713
});
683714

684715
it('rejects an Enum type with incorrectly typed values', () => {
@@ -692,6 +723,18 @@ describe('Type System: Enums', () => {
692723
).to.throw('SomeEnum values must be an object with value names as keys.');
693724
});
694725

726+
it('rejects an Enum type with incorrectly named values', () => {
727+
expect(
728+
() =>
729+
new GraphQLEnumType({
730+
name: 'SomeEnum',
731+
values: {
732+
'bad-name': {},
733+
},
734+
}),
735+
).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.');
736+
});
737+
695738
it('rejects an Enum type with missing value definition', () => {
696739
expect(
697740
() =>
@@ -761,10 +804,11 @@ describe('Type System: Input Objects', () => {
761804
});
762805
});
763806

764-
it('rejects an Input Object type without name', () => {
765-
// @ts-expect-error
766-
expect(() => new GraphQLInputObjectType({})).to.throw(
767-
'Must provide name.',
807+
it('rejects an Input Object type with invalid name', () => {
808+
expect(
809+
() => new GraphQLInputObjectType({ name: 'bad-name', fields: {} }),
810+
).to.throw(
811+
'Names must only contain [_a-zA-Z0-9] but "bad-name" does not.',
768812
);
769813
});
770814

@@ -789,6 +833,18 @@ describe('Type System: Input Objects', () => {
789833
'SomeInputObject fields must be an object with field names as keys or a function which returns such an object.',
790834
);
791835
});
836+
837+
it('rejects an Input Object type with incorrectly named fields', () => {
838+
const inputObjType = new GraphQLInputObjectType({
839+
name: 'SomeInputObject',
840+
fields: {
841+
'bad-name': { type: ScalarType },
842+
},
843+
});
844+
expect(() => inputObjType.getFields()).to.throw(
845+
'Names must only contain [_a-zA-Z0-9] but "bad-name" does not.',
846+
);
847+
});
792848
});
793849

794850
describe('Input Object fields must not have resolvers', () => {

‎src/type/__tests__/directive-test.ts‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,10 @@ describe('Type System: Directive', () => {
8484
);
8585
});
8686

87-
it('rejects an unnamed directive', () => {
88-
// @ts-expect-error
89-
expect(() => new GraphQLDirective({ locations: ['QUERY'] })).to.throw(
90-
'Directive must be named.',
91-
);
87+
it('rejects a directive with invalid name', () => {
88+
expect(
89+
() => new GraphQLDirective({ name: 'bad-name', locations: ['QUERY'] }),
90+
).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.');
9291
});
9392

9493
it('rejects a directive with incorrectly typed args', () => {
@@ -103,6 +102,19 @@ describe('Type System: Directive', () => {
103102
).to.throw('@Foo args must be an object with argument names as keys.');
104103
});
105104

105+
it('rejects a directive with incorrectly named arg', () => {
106+
expect(
107+
() =>
108+
new GraphQLDirective({
109+
name: 'Foo',
110+
locations: ['QUERY'],
111+
args: {
112+
'bad-name': { type: GraphQLString },
113+
},
114+
}),
115+
).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.');
116+
});
117+
106118
it('rejects a directive with undefined locations', () => {
107119
// @ts-expect-error
108120
expect(() => new GraphQLDirective({ name: 'Foo' })).to.throw(

‎src/type/__tests__/validation-test.ts‎

Lines changed: 16 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import type {
1818
GraphQLFieldConfig,
1919
GraphQLArgumentConfig,
2020
GraphQLInputFieldConfig,
21-
GraphQLEnumValueConfigMap,
2221
} from '../definition';
2322
import { GraphQLSchema } from '../schema';
2423
import { GraphQLString } from '../scalars';
@@ -487,13 +486,15 @@ describe('Type System: Objects must have fields', () => {
487486
const schema = schemaWithFieldType(
488487
new GraphQLObjectType({
489488
name: 'SomeObject',
490-
fields: { 'bad-name-with-dashes': { type: GraphQLString } },
489+
fields: {
490+
__badName: { type: GraphQLString },
491+
},
491492
}),
492493
);
493494
expectJSON(validateSchema(schema)).to.deep.equal([
494495
{
495496
message:
496-
'Names must only contain [_a-zA-Z0-9] but "bad-name-with-dashes" does not.',
497+
'Name "__badName" must not begin with "__", which is reserved by GraphQL introspection.',
497498
},
498499
]);
499500
});
@@ -525,7 +526,7 @@ describe('Type System: Fields args must be properly named', () => {
525526
badField: {
526527
type: GraphQLString,
527528
args: {
528-
'bad-name-with-dashes': { type: GraphQLString },
529+
__badName: { type: GraphQLString },
529530
},
530531
},
531532
},
@@ -535,7 +536,7 @@ describe('Type System: Fields args must be properly named', () => {
535536
expectJSON(validateSchema(schema)).to.deep.equal([
536537
{
537538
message:
538-
'Names must only contain [_a-zA-Z0-9] but "bad-name-with-dashes" does not.',
539+
'Name "__badName" must not begin with "__", which is reserved by GraphQL introspection.',
539540
},
540541
]);
541542
});
@@ -956,51 +957,21 @@ describe('Type System: Enum types must be well defined', () => {
956957
});
957958

958959
it('rejects an Enum type with incorrectly named values', () => {
959-
function schemaWithEnum(values: GraphQLEnumValueConfigMap): GraphQLSchema {
960-
return schemaWithFieldType(
961-
new GraphQLEnumType({
962-
name: 'SomeEnum',
963-
values,
964-
}),
965-
);
966-
}
967-
968-
const schema1 = schemaWithEnum({ '#value': {} });
969-
expectJSON(validateSchema(schema1)).to.deep.equal([
970-
{
971-
message: 'Names must start with [_a-zA-Z] but "#value" does not.',
972-
},
973-
]);
974-
975-
const schema2 = schemaWithEnum({ '1value': {} });
976-
expectJSON(validateSchema(schema2)).to.deep.equal([
977-
{
978-
message: 'Names must start with [_a-zA-Z] but "1value" does not.',
979-
},
980-
]);
960+
const schema = schemaWithFieldType(
961+
new GraphQLEnumType({
962+
name: 'SomeEnum',
963+
values: {
964+
__badName: {},
965+
},
966+
}),
967+
);
981968

982-
const schema3 = schemaWithEnum({ 'KEBAB-CASE': {} });
983-
expectJSON(validateSchema(schema3)).to.deep.equal([
969+
expectJSON(validateSchema(schema)).to.deep.equal([
984970
{
985971
message:
986-
'Names must only contain [_a-zA-Z0-9] but "KEBAB-CASE" does not.',
972+
'Name "__badName" must not begin with "__", which is reserved by GraphQL introspection.',
987973
},
988974
]);
989-
990-
const schema4 = schemaWithEnum({ true: {} });
991-
expectJSON(validateSchema(schema4)).to.deep.equal([
992-
{ message: 'Enum type SomeEnum cannot include value: true.' },
993-
]);
994-
995-
const schema5 = schemaWithEnum({ false: {} });
996-
expectJSON(validateSchema(schema5)).to.deep.equal([
997-
{ message: 'Enum type SomeEnum cannot include value: false.' },
998-
]);
999-
1000-
const schema6 = schemaWithEnum({ null: {} });
1001-
expectJSON(validateSchema(schema6)).to.deep.equal([
1002-
{ message: 'Enum type SomeEnum cannot include value: null.' },
1003-
]);
1004975
});
1005976
});
1006977

0 commit comments

Comments
 (0)