Skip to content

Commit 9577fd1

Browse files
committed
Add runtime check for resolve() related params.
According to type assuming between GraphQL v0.6.0 and Relay.
1 parent 4e0a86e commit 9577fd1

File tree

6 files changed

+119
-21
lines changed

6 files changed

+119
-21
lines changed

src/mutation/__tests__/mutation.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ var simpleRootValueMutation = mutationWithClientMutationId({
6565
type: GraphQLInt
6666
}
6767
},
68+
// $FlowFixMe: check if Relay narrowed GraphQLResolveInfo.rootValue to Object
6869
mutateAndGetPayload: (params, context, {rootValue}) => (rootValue)
6970
});
7071

@@ -384,5 +385,13 @@ describe('mutationWithClientMutationId()', () => {
384385

385386
return expect(graphql(schema, query)).to.become({data: expected});
386387
});
388+
389+
it('must get a args.input:Object from GraphQL', () => {
390+
const args = {input: 'wrongType'};
391+
return expect(()=>{
392+
(simpleMutationWithThunkFields:any).resolve({},args,{},{});
393+
}).to.throw('args.input must be string,' +
394+
' but its wrongType(type: string)');
395+
});
387396
});
388397
});

src/mutation/mutation.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import type {
2323
} from 'graphql';
2424

2525
type mutationFn = (object: Object, ctx: mixed, info: GraphQLResolveInfo) =>
26-
( Object | Promise<Object> );
26+
( Object | Promise<Object> );
2727

2828
function resolveMaybeThunk<T>(thingOrThunk: T | () => T): T {
2929
return typeof thingOrThunk === 'function' ? thingOrThunk() : thingOrThunk;
@@ -86,7 +86,14 @@ export function mutationWithClientMutationId(
8686
args: {
8787
input: {type: new GraphQLNonNull(inputType)}
8888
},
89-
resolve: (_, {input}, context, info) => {
89+
resolve: (_, args, context, info) => {
90+
// ToDo: Should make a static flow check later
91+
// runtime check GraphQL(mixed) -> Relay(Object)
92+
const input:Object = ( args.input instanceof Object) ? args.input
93+
: (()=>{
94+
throw new Error('args.input must be string,' +
95+
` but its ${args.input}(type: ${typeof args.input})`);
96+
})();
9097
return Promise.resolve(mutateAndGetPayload(input, context, info))
9198
.then(payload => {
9299
payload.clientMutationId = input.clientMutationId;

src/node/__tests__/node.js

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ import {
2222
} from 'graphql';
2323

2424
import {
25-
nodeDefinitions
25+
nodeDefinitions,
26+
globalIdField
2627
} from '../node';
2728

2829
var userData = {
@@ -239,6 +240,39 @@ describe('Node interface and fields', () => {
239240
});
240241
});
241242

243+
describe('reliability (#runtime check)', () => {
244+
it('nodeField.resolve() will throw a error, ' +
245+
'when receieve a non-string args.id from GraphQL', () => {
246+
const id = 4;
247+
return expect(()=>{
248+
(nodeField:any).resolve({},{id: 4},{},{});
249+
}).to.throw(`args.id must be string,but its ${id}` +
250+
`(type: ${typeof id})` );
251+
});
252+
253+
it('globalIdField() will throw a error, ' +
254+
'when receieve a !(string|number) id from upstream', () => {
255+
const gIdFielfConfig = globalIdField('test');
256+
const id = {test: 4};
257+
return expect(()=>{
258+
(gIdFielfConfig:any).resolve({id: id},{},{},{});
259+
}).to.throw('id field must be string|number, ' +
260+
'but it is [object Object](type: object)');
261+
});
262+
263+
// most of this case must be caused by user's wrong typo.
264+
it('globalIdField() will throw a error, when the user custom idFetcher ' +
265+
'return a !(string|number) id', () => {
266+
const source = {customId: '2'};
267+
// do a wrong typo customId vs customid
268+
const gIdFielfConfig = globalIdField('test',src => src.customid);
269+
return expect(()=>{
270+
(gIdFielfConfig:any).resolve(source,{},{},{});
271+
}).to.throw('globalIdField.idFetcher must return a string|number, ' +
272+
'check your code.');
273+
});
274+
});
275+
242276
describe('introspection', () => {
243277
it('has correct node interface', () => {
244278
var query = `{

src/node/__tests__/plural.js

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,22 @@ var userType = new GraphQLObjectType({
4040
}),
4141
});
4242

43+
var pluralRootField = pluralIdentifyingRootField({
44+
argName: 'usernames',
45+
description: 'Map from a username to the user',
46+
inputType: GraphQLString,
47+
outputType: userType,
48+
// $FlowFixMe : rootValue Graphql(mixed) -> relay(object)
49+
resolveSingleInput: (username, context, {rootValue: {lang}}) => ({
50+
username: username,
51+
url: 'www.facebook.com/' + username + '?lang=' + lang
52+
}),
53+
});
54+
4355
var queryType = new GraphQLObjectType({
4456
name: 'Query',
4557
fields: () => ({
46-
usernames: pluralIdentifyingRootField({
47-
argName: 'usernames',
48-
description: 'Map from a username to the user',
49-
inputType: GraphQLString,
50-
outputType: userType,
51-
// $FlowFixMe : rootValue Graphql(mixed) -> relay(object)
52-
resolveSingleInput: (username, context, {rootValue: {lang}}) => ({
53-
username: username,
54-
url: 'www.facebook.com/' + username + '?lang=' + lang
55-
})
56-
})
58+
usernames: pluralRootField
5759
})
5860
});
5961

@@ -163,6 +165,13 @@ describe('pluralIdentifyingRootField()', () => {
163165

164166
return expect(graphql(schema, query)).to.become({data: expected});
165167
});
168+
169+
it('only accept Array<> args from GraphQL', () => {
170+
return expect(()=>{
171+
(pluralRootField:any).resolve({},{usernames: 'test'},{},{});
172+
}).to.throw('plural\'s inputArgs must be Array,' +
173+
' but its test(type: string)');
174+
});
166175
});
167176

168177
describe('nonNull()', () => {

src/node/node.js

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,17 @@ export function nodeDefinitions(
6868
description: 'The ID of an object'
6969
}
7070
},
71-
resolve: (obj, {id}, context, info) => idFetcher(id, context, info),
71+
resolve: (obj, args, context, info) => {
72+
// ToDo : should make static flow check later
73+
// graphql should pass a args.id(GraphQLID) in.
74+
// which always serialize to a string in graphql v0.6
75+
const id:string = (typeof args.id === 'string') ? args.id
76+
: (()=>{
77+
throw new Error(`args.id must be string,but its ${args.id}` +
78+
`(type: ${typeof args.id})`);
79+
})();
80+
return idFetcher(id, context, info);
81+
},
7282
};
7383

7484
return {nodeInterface, nodeField};
@@ -108,15 +118,37 @@ export function fromGlobalId(globalId: string): ResolvedGlobalId {
108118
*/
109119
export function globalIdField(
110120
typeName?: ?string,
111-
idFetcher?: (object: any, context: any, info: GraphQLResolveInfo) => string
121+
idFetcher?: (object: Object, context: mixed,
122+
info: GraphQLResolveInfo) => string
112123
): GraphQLFieldConfig {
113124
return {
114125
name: 'id',
115126
description: 'The ID of an object',
116127
type: new GraphQLNonNull(GraphQLID),
117-
resolve: (obj, args, context, info) => toGlobalId(
118-
typeName || info.parentType.name,
119-
idFetcher ? idFetcher(obj, context, info) : obj.id
120-
)
128+
resolve: (source, args, context, info) => {
129+
// ToDo : should make static flow check later
130+
// runtime check for GraphQLNonNull(GraphQLID) : string|number
131+
if ( idFetcher ) {
132+
let id = check( idFetcher((source:any), context, info),
133+
'globalIdField.idFetcher must return a string|number, ' +
134+
'check your code.');
135+
return toGlobalId(typeName || info.parentType.name, id);
136+
} else {
137+
let id = check( (source:any).id, 'id field must be string|number, ' +
138+
`but it is ${(source:any).id}(type: ${typeof (source:any).id})`);
139+
return toGlobalId(typeName || info.parentType.name, id);
140+
}
141+
142+
function check(unId:any,eMsg:string):string {
143+
switch (typeof unId) {
144+
case 'string':
145+
return unId;
146+
case 'number':
147+
return unId.toString();
148+
default:
149+
throw new Error(eMsg);
150+
}
151+
}
152+
}
121153
};
122154
}

src/node/plural.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,14 @@ export function pluralIdentifyingRootField(
4848
type: new GraphQLList(config.outputType),
4949
args: inputArgs,
5050
resolve: (obj, args, context, info) => {
51-
var inputs = args[config.argName];
51+
const uncheckedInputs = args[config.argName];
52+
// ToDo : should make static flow check later
53+
// runtime check for args[config.argName] : Array<mixed>
54+
const inputs = Array.isArray(uncheckedInputs) ? uncheckedInputs
55+
: (()=>{
56+
throw new Error('plural\'s inputArgs must be Array, but its' +
57+
` ${uncheckedInputs}(type: ${typeof uncheckedInputs})`);
58+
})();
5259
return Promise.all(inputs.map(
5360
input => Promise.resolve(
5461
config.resolveSingleInput(input, context, info)

0 commit comments

Comments
 (0)