Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move validation of names into GraphQL* constructors #3288

Merged
merged 1 commit into from Oct 5, 2021
Merged

Move validation of names into GraphQL* constructors #3288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/index.ts
View file
Open in desktop
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
Open in desktop
@@ -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
Open in desktop
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
Open in desktop
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
Open in desktop
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