Skip to content

Commit

Permalink
Move validation of names into GraphQL* constructors (#3288)
Browse files Browse the repository at this point in the history
  • Loading branch information
@IvanGoncharov
IvanGoncharov committed Oct 5, 2021
1 parent 22b9504 commit 5774173
Show file tree
Hide file tree
Showing 12 changed files with 257 additions and 162 deletions.
3 changes: 3 additions & 0 deletions src/index.ts
View file
Expand Up @@ -126,6 +126,9 @@ export {
/** Validate GraphQL schema. */
validateSchema,
assertValidSchema,
/** Upholds the spec rules about naming. */
assertName,
assertEnumValueName,
} from './type/index';

export type {
Expand Down
71 changes: 71 additions & 0 deletions src/type/__tests__/assertName-test.ts
View file
@@ -0,0 +1,71 @@
import { expect } from 'chai';
import { describe, it } from 'mocha';

import { assertName, assertEnumValueName } from '../assertName';
import { assertValidName } from '../../utilities/assertValidName';

describe('assertValidName()', () => {
assertValidName('test');
it('passthrough valid name', () => {
expect(assertName('_ValidName123')).to.equal('_ValidName123');
});

it('throws for non-strings', () => {
// @ts-expect-error
expect(() => assertName({})).to.throw('Expected name to be a string.');
});

it('throws on empty strings', () => {
expect(() => assertName('')).to.throw(
'Expected name to be a non-empty string.',
);
});

it('throws for names with invalid characters', () => {
expect(() => assertName('>--()-->')).to.throw(
'Names must only contain [_a-zA-Z0-9] but ">--()-->" does not.',
);
});

it('throws for names starting with invalid characters', () => {
expect(() => assertName('42MeaningsOfLife')).to.throw(
'Names must start with [_a-zA-Z] but "42MeaningsOfLife" does not.',
);
});
});

describe('assertEnumValueName()', () => {
it('passthrough valid name', () => {
expect(assertEnumValueName('_ValidName123')).to.equal('_ValidName123');
});

it('throws on empty strings', () => {
expect(() => assertEnumValueName('')).to.throw(
'Expected name to be a non-empty string.',
);
});

it('throws for names with invalid characters', () => {
expect(() => assertEnumValueName('>--()-->')).to.throw(
'Names must only contain [_a-zA-Z0-9] but ">--()-->" does not.',
);
});

it('throws for names starting with invalid characters', () => {
expect(() => assertEnumValueName('42MeaningsOfLife')).to.throw(
'Names must start with [_a-zA-Z] but "42MeaningsOfLife" does not.',
);
});

it('throws for restricted names', () => {
expect(() => assertEnumValueName('true')).to.throw(
'Enum values cannot be named: true',
);
expect(() => assertEnumValueName('false')).to.throw(
'Enum values cannot be named: false',
);
expect(() => assertEnumValueName('null')).to.throw(
'Enum values cannot be named: null',
);
});
});
92 changes: 74 additions & 18 deletions src/type/__tests__/definition-test.ts
View file
Expand Up @@ -324,9 +324,10 @@ describe('Type System: Objects', () => {
expect(() => objType.getFields()).to.not.throw();
});

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

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

it('rejects an Object type with incorrectly named fields', () => {
const objType = new GraphQLObjectType({
name: 'SomeObject',
fields: {
'bad-name': { type: ScalarType },
},
});
expect(() => objType.getFields()).to.throw(
'Names must only contain [_a-zA-Z0-9] but "bad-name" does not.',
);
});

it('rejects an Object type with a field function that returns incorrect type', () => {
const objType = new GraphQLObjectType({
name: 'SomeObject',
Expand Down Expand Up @@ -380,6 +393,23 @@ describe('Type System: Objects', () => {
);
});

it('rejects an Object type with incorrectly named field args', () => {
const objType = new GraphQLObjectType({
name: 'SomeObject',
fields: {
badField: {
type: ScalarType,
args: {
'bad-name': { type: ScalarType },
},
},
},
});
expect(() => objType.getFields()).to.throw(
'Names must only contain [_a-zA-Z0-9] but "bad-name" does not.',
);
});

it('rejects an Object type with incorrectly typed interfaces', () => {
const objType = new GraphQLObjectType({
name: 'SomeObject',
Expand Down Expand Up @@ -478,9 +508,10 @@ describe('Type System: Interfaces', () => {
expect(implementing.getInterfaces()).to.deep.equal([InterfaceType]);
});

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

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

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

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

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

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

it('rejects an Enum type with incorrectly named values', () => {
expect(
() =>
new GraphQLEnumType({
name: 'SomeEnum',
values: {
'bad-name': {},
},
}),
).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.');
});

it('rejects an Enum type with missing value definition', () => {
expect(
() =>
Expand Down Expand Up @@ -761,10 +804,11 @@ describe('Type System: Input Objects', () => {
});
});

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

Expand All @@ -789,6 +833,18 @@ describe('Type System: Input Objects', () => {
'SomeInputObject fields must be an object with field names as keys or a function which returns such an object.',
);
});

it('rejects an Input Object type with incorrectly named fields', () => {
const inputObjType = new GraphQLInputObjectType({
name: 'SomeInputObject',
fields: {
'bad-name': { type: ScalarType },
},
});
expect(() => inputObjType.getFields()).to.throw(
'Names must only contain [_a-zA-Z0-9] but "bad-name" does not.',
);
});
});

describe('Input Object fields must not have resolvers', () => {
Expand Down
22 changes: 17 additions & 5 deletions src/type/__tests__/directive-test.ts
View file
Expand Up @@ -84,11 +84,10 @@ describe('Type System: Directive', () => {
);
});

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

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

it('rejects a directive with incorrectly named arg', () => {
expect(
() =>
new GraphQLDirective({
name: 'Foo',
locations: ['QUERY'],
args: {
'bad-name': { type: GraphQLString },
},
}),
).to.throw('Names must only contain [_a-zA-Z0-9] but "bad-name" does not.');
});

it('rejects a directive with undefined locations', () => {
// @ts-expect-error
expect(() => new GraphQLDirective({ name: 'Foo' })).to.throw(
Expand Down
61 changes: 16 additions & 45 deletions src/type/__tests__/validation-test.ts
View file
Expand Up @@ -18,7 +18,6 @@ import type {
GraphQLFieldConfig,
GraphQLArgumentConfig,
GraphQLInputFieldConfig,
GraphQLEnumValueConfigMap,
} from '../definition';
import { GraphQLSchema } from '../schema';
import { GraphQLString } from '../scalars';
Expand Down Expand Up @@ -487,13 +486,15 @@ describe('Type System: Objects must have fields', () => {
const schema = schemaWithFieldType(
new GraphQLObjectType({
name: 'SomeObject',
fields: { 'bad-name-with-dashes': { type: GraphQLString } },
fields: {
__badName: { type: GraphQLString },
},
}),
);
expectJSON(validateSchema(schema)).to.deep.equal([
{
message:
'Names must only contain [_a-zA-Z0-9] but "bad-name-with-dashes" does not.',
'Name "__badName" must not begin with "__", which is reserved by GraphQL introspection.',
},
]);
});
Expand Down Expand Up @@ -525,7 +526,7 @@ describe('Type System: Fields args must be properly named', () => {
badField: {
type: GraphQLString,
args: {
'bad-name-with-dashes': { type: GraphQLString },
__badName: { type: GraphQLString },
},
},
},
Expand All @@ -535,7 +536,7 @@ describe('Type System: Fields args must be properly named', () => {
expectJSON(validateSchema(schema)).to.deep.equal([
{
message:
'Names must only contain [_a-zA-Z0-9] but "bad-name-with-dashes" does not.',
'Name "__badName" must not begin with "__", which is reserved by GraphQL introspection.',
},
]);
});
Expand Down Expand Up @@ -956,51 +957,21 @@ describe('Type System: Enum types must be well defined', () => {
});

it('rejects an Enum type with incorrectly named values', () => {
function schemaWithEnum(values: GraphQLEnumValueConfigMap): GraphQLSchema {
return schemaWithFieldType(
new GraphQLEnumType({
name: 'SomeEnum',
values,
}),
);
}

const schema1 = schemaWithEnum({ '#value': {} });
expectJSON(validateSchema(schema1)).to.deep.equal([
{
message: 'Names must start with [_a-zA-Z] but "#value" does not.',
},
]);

const schema2 = schemaWithEnum({ '1value': {} });
expectJSON(validateSchema(schema2)).to.deep.equal([
{
message: 'Names must start with [_a-zA-Z] but "1value" does not.',
},
]);
const schema = schemaWithFieldType(
new GraphQLEnumType({
name: 'SomeEnum',
values: {
__badName: {},
},
}),
);

const schema3 = schemaWithEnum({ 'KEBAB-CASE': {} });
expectJSON(validateSchema(schema3)).to.deep.equal([
expectJSON(validateSchema(schema)).to.deep.equal([
{
message:
'Names must only contain [_a-zA-Z0-9] but "KEBAB-CASE" does not.',
'Name "__badName" must not begin with "__", which is reserved by GraphQL introspection.',
},
]);

const schema4 = schemaWithEnum({ true: {} });
expectJSON(validateSchema(schema4)).to.deep.equal([
{ message: 'Enum type SomeEnum cannot include value: true.' },
]);

const schema5 = schemaWithEnum({ false: {} });
expectJSON(validateSchema(schema5)).to.deep.equal([
{ message: 'Enum type SomeEnum cannot include value: false.' },
]);

const schema6 = schemaWithEnum({ null: {} });
expectJSON(validateSchema(schema6)).to.deep.equal([
{ message: 'Enum type SomeEnum cannot include value: null.' },
]);
});
});

Expand Down

0 comments on commit 5774173

Please sign in to comment.