Skip to content

Commit 2e29b78

Browse files
leebyronleebyron
committed
Assert proper Enum configuration.
This asserts that Enums have properly configured values. This also asserts that isTypeOf and resolveType are functions, if provided. Finally, this changes the signature of EnumType.getValues() to return an array instead of an Object map, since every single use case was calling Object.keys. This fixes #122
1 parent fec9862 commit 2e29b78

File tree

6 files changed

+226
-65
lines changed

6 files changed

+226
-65
lines changed

‎src/type/__tests__/definition.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ var ObjectType = new GraphQLObjectType({
8787
});
8888
var InterfaceType = new GraphQLInterfaceType({ name: 'Interface' });
8989
var UnionType = new GraphQLUnionType({ name: 'Union', types: [ ObjectType ] });
90-
var EnumType = new GraphQLEnumType({ name: 'Enum' });
90+
var EnumType = new GraphQLEnumType({ name: 'Enum', values: { foo: {} } });
9191
var InputObjectType = new GraphQLInputObjectType({ name: 'InputObject' });
9292

9393
describe('Type System: Example', () => {

‎src/type/__tests__/validation.js

+142
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,33 @@ describe('Type System: Input Objects must have fields', () => {
618618
});
619619

620620

621+
describe('Type System: Object types must be assertable', () => {
622+
623+
it('accepts an Object type with an isTypeOf function', () => {
624+
expect(() => {
625+
schemaWithFieldType(new GraphQLObjectType({
626+
name: 'AnotherObject',
627+
isTypeOf: () => true,
628+
fields: { f: { type: GraphQLString } }
629+
}));
630+
}).not.to.throw();
631+
});
632+
633+
it('rejects an Object type with an incorrect type for isTypeOf', () => {
634+
expect(() => {
635+
schemaWithFieldType(new GraphQLObjectType({
636+
name: 'AnotherObject',
637+
isTypeOf: {},
638+
fields: { f: { type: GraphQLString } }
639+
}));
640+
}).to.throw(
641+
'AnotherObject must provide "isTypeOf" as a function.'
642+
);
643+
});
644+
645+
});
646+
647+
621648
describe('Type System: Interface types must be resolvable', () => {
622649

623650
it('accepts an Interface type defining resolveType', () => {
@@ -669,6 +696,18 @@ describe('Type System: Interface types must be resolvable', () => {
669696
}).not.to.throw();
670697
});
671698

699+
it('rejects an Interface type with an incorrect type for resolveType', () => {
700+
expect(() =>
701+
new GraphQLInterfaceType({
702+
name: 'AnotherInterface',
703+
resolveType: {},
704+
fields: { f: { type: GraphQLString } }
705+
})
706+
).to.throw(
707+
'AnotherInterface must provide "resolveType" as a function.'
708+
);
709+
});
710+
672711
it('rejects an Interface type not defining resolveType with implementing type not defining isTypeOf', () => {
673712
expect(() => {
674713
var InterfaceTypeWithoutResolveType = new GraphQLInterfaceType({
@@ -723,6 +762,18 @@ describe('Type System: Union types must be resolvable', () => {
723762
).not.to.throw();
724763
});
725764

765+
it('rejects an Interface type with an incorrect type for resolveType', () => {
766+
expect(() =>
767+
schemaWithFieldType(new GraphQLUnionType({
768+
name: 'SomeUnion',
769+
resolveType: {},
770+
types: [ ObjectWithIsTypeOf ],
771+
}))
772+
).to.throw(
773+
'SomeUnion must provide "resolveType" as a function.'
774+
);
775+
});
776+
726777
it('rejects a Union type not defining resolveType of Object types not defining isTypeOf', () => {
727778
expect(() =>
728779
schemaWithFieldType(new GraphQLUnionType({
@@ -826,6 +877,97 @@ describe('Type System: Scalar types must be serializable', () => {
826877
});
827878

828879

880+
describe('Type System: Enum types must be well defined', () => {
881+
882+
it('accepts a well defined Enum type with empty value definition', () => {
883+
expect(() =>
884+
new GraphQLEnumType({
885+
name: 'SomeEnum',
886+
values: {
887+
FOO: {},
888+
BAR: {},
889+
}
890+
})
891+
).not.to.throw();
892+
});
893+
894+
it('accepts a well defined Enum type with internal value definition', () => {
895+
expect(() =>
896+
new GraphQLEnumType({
897+
name: 'SomeEnum',
898+
values: {
899+
FOO: { value: 10 },
900+
BAR: { value: 20 },
901+
}
902+
})
903+
).not.to.throw();
904+
});
905+
906+
it('rejects an Enum type without values', () => {
907+
expect(() =>
908+
new GraphQLEnumType({
909+
name: 'SomeEnum',
910+
})
911+
).to.throw(
912+
'SomeEnum values must be an object with value names as keys.'
913+
);
914+
});
915+
916+
it('rejects an Enum type with empty values', () => {
917+
expect(() =>
918+
new GraphQLEnumType({
919+
name: 'SomeEnum',
920+
values: {}
921+
})
922+
).to.throw(
923+
'SomeEnum values must be an object with value names as keys.'
924+
);
925+
});
926+
927+
it('rejects an Enum type with incorrectly typed values', () => {
928+
expect(() =>
929+
new GraphQLEnumType({
930+
name: 'SomeEnum',
931+
values: [
932+
{ FOO: 10 }
933+
]
934+
})
935+
).to.throw(
936+
'SomeEnum values must be an object with value names as keys.'
937+
);
938+
});
939+
940+
it('rejects an Enum type with missing value definition', () => {
941+
expect(() =>
942+
new GraphQLEnumType({
943+
name: 'SomeEnum',
944+
values: {
945+
FOO: null
946+
}
947+
})
948+
).to.throw(
949+
'SomeEnum.FOO must refer to an object with a "value" key representing ' +
950+
'an internal value but got: null.'
951+
);
952+
});
953+
954+
it('rejects an Enum type with incorrectly typed value definition', () => {
955+
expect(() =>
956+
new GraphQLEnumType({
957+
name: 'SomeEnum',
958+
values: {
959+
FOO: 10
960+
}
961+
})
962+
).to.throw(
963+
'SomeEnum.FOO must refer to an object with a "value" key representing ' +
964+
'an internal value but got: 10.'
965+
);
966+
});
967+
968+
});
969+
970+
829971
describe('Type System: Object fields must have output types', () => {
830972

831973
function schemaWithObjectFieldOfType(fieldType) {

‎src/type/definition.js

+66-34
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
*/
1010

1111
import invariant from '../jsutils/invariant';
12+
import isNullish from '../jsutils/isNullish';
1213
import { ENUM } from '../language/kinds';
1314
import type {
1415
OperationDefinition,
@@ -296,6 +297,12 @@ export class GraphQLObjectType {
296297
invariant(config.name, 'Type must be named.');
297298
this.name = config.name;
298299
this.description = config.description;
300+
if (config.isTypeOf) {
301+
invariant(
302+
typeof config.isTypeOf === 'function',
303+
`${this} must provide "isTypeOf" as a function.`
304+
);
305+
}
299306
this.isTypeOf = config.isTypeOf;
300307
this._typeConfig = config;
301308
addImplementationToInterfaces(this);
@@ -359,7 +366,7 @@ function defineFieldMap(
359366
): GraphQLFieldDefinitionMap {
360367
var fieldMap: any = resolveMaybeThunk(fields);
361368
invariant(
362-
typeof fieldMap === 'object' && !Array.isArray(fieldMap),
369+
isPlainObj(fieldMap),
363370
`${type} fields must be an object with field names as keys or a ` +
364371
`function which returns such an object.`
365372
);
@@ -381,7 +388,7 @@ function defineFieldMap(
381388
field.args = [];
382389
} else {
383390
invariant(
384-
typeof field.args === 'object' && !Array.isArray(field.args),
391+
isPlainObj(field.args),
385392
`${type}.${fieldName} args must be an object with argument names ` +
386393
`as keys.`
387394
);
@@ -404,6 +411,10 @@ function defineFieldMap(
404411
return fieldMap;
405412
}
406413

414+
function isPlainObj(obj) {
415+
return obj && typeof obj === 'object' && !Array.isArray(obj);
416+
}
417+
407418
/**
408419
* Update the interfaces to know about this implementation.
409420
* This is an rare and unfortunate use of mutation in the type definition
@@ -521,6 +532,12 @@ export class GraphQLInterfaceType {
521532
invariant(config.name, 'Type must be named.');
522533
this.name = config.name;
523534
this.description = config.description;
535+
if (config.resolveType) {
536+
invariant(
537+
typeof config.resolveType === 'function',
538+
`${this} must provide "resolveType" as a function.`
539+
);
540+
}
524541
this.resolveType = config.resolveType;
525542
this._typeConfig = config;
526543
this._implementations = [];
@@ -621,6 +638,12 @@ export class GraphQLUnionType {
621638
invariant(config.name, 'Type must be named.');
622639
this.name = config.name;
623640
this.description = config.description;
641+
if (config.resolveType) {
642+
invariant(
643+
typeof config.resolveType === 'function',
644+
`${this} must provide "resolveType" as a function.`
645+
);
646+
}
624647
this.resolveType = config.resolveType;
625648
invariant(
626649
Array.isArray(config.types) && config.types.length > 0,
@@ -711,18 +734,19 @@ export class GraphQLEnumType/* <T> */ {
711734
description: ?string;
712735

713736
_enumConfig: GraphQLEnumTypeConfig/* <T> */;
714-
_values: GraphQLEnumValueDefinitionMap/* <T> */;
737+
_values: Array<GraphQLEnumValueDefinition/* <T> */>;
715738
_valueLookup: Map<any/* T */, GraphQLEnumValueDefinition>;
716-
_nameLookup: Map<string, GraphQLEnumValueDefinition>;
739+
_nameLookup: { [valueName: string]: GraphQLEnumValueDefinition };
717740

718741
constructor(config: GraphQLEnumTypeConfig/* <T> */) {
719742
this.name = config.name;
720743
this.description = config.description;
744+
this._values = defineEnumValues(this, config.values);
721745
this._enumConfig = config;
722746
}
723747

724-
getValues(): GraphQLEnumValueDefinitionMap/* <T> */ {
725-
return this._values || (this._values = this._defineValueMap());
748+
getValues(): Array<GraphQLEnumValueDefinition/* <T> */> {
749+
return this._values;
726750
}
727751

728752
serialize(value: any/* T */): ?string {
@@ -731,53 +755,37 @@ export class GraphQLEnumType/* <T> */ {
731755
}
732756

733757
parseValue(value: any): ?any/* T */ {
734-
var enumValue = this._getNameLookup().get(value);
758+
var enumValue = this._getNameLookup()[value];
735759
if (enumValue) {
736760
return enumValue.value;
737761
}
738762
}
739763

740764
parseLiteral(valueAST: Value): ?any/* T */ {
741765
if (valueAST.kind === ENUM) {
742-
var enumValue = this._getNameLookup().get(valueAST.value);
766+
var enumValue = this._getNameLookup()[valueAST.value];
743767
if (enumValue) {
744768
return enumValue.value;
745769
}
746770
}
747771
}
748772

749-
_defineValueMap(): GraphQLEnumValueDefinitionMap/* <T> */ {
750-
var valueMap = (this._enumConfig.values: any);
751-
Object.keys(valueMap).forEach(valueName => {
752-
var value = valueMap[valueName];
753-
value.name = valueName;
754-
if (!value.hasOwnProperty('value')) {
755-
value.value = valueName;
756-
}
757-
});
758-
return valueMap;
759-
}
760-
761773
_getValueLookup(): Map<any/* T */, GraphQLEnumValueDefinition> {
762774
if (!this._valueLookup) {
763775
var lookup = new Map();
764-
var values = this.getValues();
765-
Object.keys(values).forEach(valueName => {
766-
var value = values[valueName];
776+
this.getValues().forEach(value => {
767777
lookup.set(value.value, value);
768778
});
769779
this._valueLookup = lookup;
770780
}
771781
return this._valueLookup;
772782
}
773783

774-
_getNameLookup(): Map<string, GraphQLEnumValueDefinition> {
784+
_getNameLookup(): { [valueName: string]: GraphQLEnumValueDefinition } {
775785
if (!this._nameLookup) {
776-
var lookup = new Map();
777-
var values = this.getValues();
778-
Object.keys(values).forEach(valueName => {
779-
var value = values[valueName];
780-
lookup.set(value.name, value);
786+
var lookup = Object.create(null);
787+
this.getValues().forEach(value => {
788+
lookup[value.name] = value;
781789
});
782790
this._nameLookup = lookup;
783791
}
@@ -789,6 +797,34 @@ export class GraphQLEnumType/* <T> */ {
789797
}
790798
}
791799

800+
function defineEnumValues(
801+
type: GraphQLEnumType,
802+
valueMap: GraphQLEnumValueConfigMap/* <T> */
803+
): Array<GraphQLEnumValueDefinition/* <T> */> {
804+
invariant(
805+
isPlainObj(valueMap),
806+
`${type} values must be an object with value names as keys.`
807+
);
808+
var valueNames = Object.keys(valueMap);
809+
invariant(
810+
valueNames.length > 0,
811+
`${type} values must be an object with value names as keys.`
812+
);
813+
return valueNames.map(valueName => {
814+
var value = valueMap[valueName];
815+
invariant(
816+
isPlainObj(value),
817+
`${type}.${valueName} must refer to an object with a "value" key ` +
818+
`representing an internal value but got: ${value}.`
819+
);
820+
value.name = valueName;
821+
if (isNullish(value.value)) {
822+
value.value = valueName;
823+
}
824+
return value;
825+
});
826+
}
827+
792828
export type GraphQLEnumTypeConfig/* <T> */ = {
793829
name: string;
794830
values: GraphQLEnumValueConfigMap/* <T> */;
@@ -805,10 +841,6 @@ export type GraphQLEnumValueConfig/* <T> */ = {
805841
description?: ?string;
806842
}
807843

808-
export type GraphQLEnumValueDefinitionMap/* <T> */ = {
809-
[valueName: string]: GraphQLEnumValueDefinition/* <T> */;
810-
};
811-
812844
export type GraphQLEnumValueDefinition/* <T> */ = {
813845
name: string;
814846
value?: any/* T */;
@@ -859,7 +891,7 @@ export class GraphQLInputObjectType {
859891
_defineFieldMap(): InputObjectFieldMap {
860892
var fieldMap: any = resolveMaybeThunk(this._typeConfig.fields);
861893
invariant(
862-
typeof fieldMap === 'object' && !Array.isArray(fieldMap),
894+
isPlainObj(fieldMap),
863895
`${this} fields must be an object with field names as keys or a ` +
864896
`function which returns such an object.`
865897
);

0 commit comments

Comments
 (0)