graphql-js
graphql-js copied to clipboard
graphql
Validation in Input types proposal
Validations in GraphQL-js
I would like to open a discussion around validation.
I think could be a great idea permit validation in input elements: GraphQLInputField and GraphQLArgument.
This would not involve any spec change as it relies on the implementation (as the resolve method).
Right now is possible to achieve this by using a custom GraphQLScalar per validation.
However, by doing this we have to create a new type just for validation and is not easy to compose multiple validation rules without creating a type for each composition.
Also, is possible to achieve it by add the validation logic in the resolver method, however things get trickier when we want to validate nested input data.
So, will be much simpler to have this in the implementation.
Handling errors
We could have two ways for handling validation errors:
- Not executing the query
- Executing the query excluding the fields with non-valid input, and add the validation error to the context.
Example usage
const queryType = new GraphQLObjectType({
name: 'Query',
fields: () => ({
getUser: {
type: User,
args: {
email: {
description: 'The email of the user',
validators: [email_validator],
type: String
}
},
resolve: (root, { email }) => {
// We know we received a proper input, so this simplifies the resolver logic
return getUserByEmail(email)
},
},
})
});
Implementation
The implementation of this should be really simple, as it could be achieved by creating a new validation rule.
I like the idea, definitely gives the code more structure.
As for what should happen when validation fails: I strongly believe that the whole query should be rejected, because otherwise the developer has to start reasoning about all possible cases where part of the arguments get accepted, but others not. It's just much easier to deal with clear success/failure and not having to worry about the whole gray area in between.
This is a good idea.
Currently you can emulate it by doing something like:
const queryType = new GraphQLObjectType({
name: 'Query',
fields: () => ({
getUser: {
type: User,
args: {
email: {
description: 'The email of the user',
type: String
}
},
resolve: (root, { email }) => {
email_validator(email);
// We know we received a proper input, so this simplifies the resolver logic
return getUserByEmail(email)
},
},
})
});
Assuming the email_validator is a function which works like an assertion: returning void and throwing an error if invalid.
However this waits until field execution time which doesn't match @helfer's suggestion of rejecting the entire query. I agree that bad input should be considered a "developer error" and therefore block the whole query.
Validation is perhaps the right place to put this (and variable value validation, right before execution) - however we can run into the case where client tools which perform validation may falsely mark a query as valid when the server would then reject it because only the server knows about these validation rules. Maybe this is just okay? I'm not sure.
Thanks for the reply @leebyron :)
we can run into the case where client tools which perform validation may falsely mark a query as valid when the server would then reject it because only the server knows about these validation rules.
That's completely right, but happens the same with scalar types, in which both client and server have to know what this scalar type means and how to deal with it. I think is worth to think about validation in the validate context and not in the executor.
New GraphQL type proposal: GraphQLValidationType
Maybe adding a validation type to the schema would be a good approach. So both server and client have to know how to deal with that.
schema {
query: Query
}
validation EmailValidator
type Query {
name: String! # String[Required] ?
email: String[EmailValidator]
}
Perhaps GraphQLNonNull is just another kind of validation on top of a type 😉
Validation operation proposal
Another thought I had was adding a new operation type asides of query, mutation, subscription; validation. So actually clients could validate queries without actually executing them in server-side.
PS: I'm just throwing some thoughts about how we can approach this problem, with the willing of opening a discussion that could help GraphQL usage in more scenarios
(maybe this is a conversation that could be opened in the spec repo instead of here)
After looking at decorators proposal, I got a feeling that it may also this use-case as well: https://github.com/apollostack/graphql-decorators. We had a discussion about it here https://github.com/apollostack/graphql-decorators/issues/1 - with some modifications to proposal it would be possible to perform the validation before the actual execution and reject the whole query if it is invalid.
@syrusakbary I made a solution (flow-dynamic ) for this too. dynamic check data,and also gives them a corresponding Flow Type. ex:
const resolver = (src:any) => {
// .... doing something
return src;
});
can be checked like this
const wrappedFn = check1(
(v) => ({
mail:pro.isString.isEmail(v.mail) // check src.mail.
}), (v) => { // Now, the input data is validated and have a Flow type {mail:string}
// .... doing something
});
The demonstration is here graphql-relay pr:89 (detail codes)
@OlegIlyenko I think a better validation system is more related to the core than hacking around decorators. @iamchenxin doing the validation in the resolving phase will not stop execution, and for validation I think is somehow essential.
A recent comment in Stackoverflow about it: http://stackoverflow.com/questions/37665539/mixing-of-schema-level-and-app-level-errors-in-graphql
Extra thoughts
Adding a new optional validate token into the language.
It could let us to do validation without any execution.
validate query MyQuery {
# ...
}
or
validate mutation MyMutation {
# ...
}
@leebyron ?
@syrusakbary Error process for resolver is in execute.js#L603. Maybe it should give users more control for error behavior.(by token or by a graphql special exception name).
.or maybe give them both? in addition to token, also can assign an exception name/class which Let us to have a chance to stop current execution(pass though that exception out of graphql).
and maybe there should be some standard exception name in graphql which means STOP ,not only ignore.To stop a execution , exception is one of the standard code pattern.
maybe cause i come from other language,that makes me more like using exception to express an exception.
@syrusakbary I think it depends on what shape decorators will take. I agree that that it would be great to have validators as a core concept in the library, or at least very good integrated. On the other hand all implementations already have a query validation mechanism, which is also present in the reference implementation. I think it can be beneficial to use it, but in order to do so one need a bit more information/meta-information on a field and argument level.
It's similar concept to the one I described on the decorators discussion. One nice property of this approach is that validation mechanism (as implemented in reference implementation) is able to see the whole query and all of the arguments of a field. This makes it easier to implement validation rules that validate several interdependent arguments at once (like, for instance, password and passwordConfirmation).
@OlegIlyenko yes, being able to compare arguments during validation is very important -- but such a thing could also be achieved by simply having a validate function called with all the same arguments as the resolve function before the query is executed.
One of the cases where client/server differences could arise (concerning context-free syntax validation) would be if old clients are connecting to a server with an updated schema. So I think an app must be structured so that the server can send back errors that the client will handle just the same as if they had occured in client-side validation.
Ow yeah... built-in validations - GraphQL would become a perfect tool! We need this... :). I've been using custom scalars for months but I found it not appropriate. Also most of my projects were using forms (form validation) and I needed to send user-friendly validation errors to a user (to a form; a custom validation error code would be enough here).
I think that user-friendly validation errors are important. What I use now in my GraphQL projects is similar to Konstantin Tarkus's proposal - smart.
mutation {
createUser() { // returns UserMutationType
data: {name} // null if invalid
errors: {message, code} // validation errors
}
}
To make my resolvers small I wrote a helper package which handles validation and error handling in a way I need (e.g. MongoDB E11000 error can also be treated as validation error thus I can respond with already taken validation error message instead of the unreadable MongoError warning). Here is how my resolvers currently look like using Approved.js:
import {user} from '../approvals/users';
export async function createUser(root, args, ctx) {
let data, errors = null;
try {
await user.validate(data, ctx);
let res = await ctx.mongo.collection('users').insert(data);
data = res.ops[0];
} catch(err) {
errors = await user.handle(err, ctx);
if (!errors) throw err;
}
return {data, errors};
}
Having this kind of functionality as part of GraphQL would be just amazing!
@syrusakbary I agree with what you're saying here:
Right now is possible to achieve this by using a custom GraphQLScalar per validation. However, by doing this we have to create a new type just for validation and is not easy to compose multiple validation rules without creating a type for each composition.
What kinds of validation do you imagine composing on an email address - for instance making sure it's not likely a botnet or throwaway email address? I definitely agree that these kinds of checks don't belong in custom scalar methods. But I can't imagine any drawbacks to using a custom scalar Email type that merely checks whether a syntactically valid email is provided, and apart from that use a system like you're proposing for contextual validation, like the subset of syntactically valid emails that the server will accept.
I think usernames or passwords would be a better example than Email; I don't think it would be as wise to use a custom scalar for usernames or passwords, since one might decide to change the restrictions down the road.
I don't see what's wrong with adding it to the spec itself. Validation that can be defined per input field at field definition instead of custom scalars could be very useful. I've asked for it here: https://github.com/facebook/graphql/issues/184
@IamCornholio I'm not saying I'm opposed to this proposal here, it's just that this is related to a debate about whether it's appropriate to use custom scalars for validation, and I think it's not necessarily ideal to keep email fields as type String and do all of the validation in a per field validator, as in the OP's example. Plus it seems like the OP is opposed to doing any kind of validation with custom scalars, and I'm debating that.
@jedwards1211 My comments were directed more towards the general conversation and OP's mention that
This would not involve any spec change as it relies on the implementation (as the resolve method).
Apologies for not setting the context appropriately.
I agree though that email could be implemented as a custom scalar rather having to add a validator to each resolve function or add a validation per field. My per field validation recommendation is assuming that the validation requirement is very specific to the field in context (such as a new password mutation with the argument restriction/validation of needing Uppercase, lowercase, numbers, symbols etc)
Also just noticed that are quite a few competing (and perhaps more interesting) proposals out there for this.
Also just noticed that are quite a few competing (and perhaps more interesting) proposals out there for this.
Again, referring to the per field validation proposal. Just the stuff already mentioned on this thread..
https://github.com/apollostack/graphql-decorators see validateRange and the long discussion here https://github.com/apollostack/graphql-decorators/issues/1
OP's proposal applied to passwords instead of email perhaps
schema {
query: Query
}
validation EmailValidator
type Query {
name: String! # String[Required] ?
email: String[EmailValidator]
}
as against my proposal which goes something like this:
type Human {
id: String![0-9a-z]{16} // 16 digit alphanumeric
name: String![a-zA-Z ]{1,30} // 01-30 characters
email: String+@+ // must have @
age: int{1,} // min value 1, max val undefined
money: float{2} // precision 2 decimal places
}
Thanks for bringing up the discussion.
Regex validation is only one use case of multiple kinds of validation. There are other validation cases like:
- Compounded validation. Checking the value of multiple fields at the same time (can't be covered using the scalar approach).
- Local-side field validation (password strength, for example)
- Complex validation rules that might require server side checking (checking if a domain have valid MX records, for email validation for example).
As I read in other issues about validation in this repo, it might be interesting to have this types of validation only for Input types/fields.
The reasoning behind creating or not Validation types in GraphQL is being able to reference them between client and server side and being able to reuse validation rules across different types/fields.
@syrusakbary for compounded validation wouldn't it be simpler to just have a validate function called before any resolve with the same args, instead of having validators in the fields themselves?
As I read in other issues about validation in this repo, it might be interesting to have this types of validation only for Input types/fields.
+1. Things get complicated really fast when you start considering this for output types.
The reasoning behind creating or not Validation types in GraphQL is being able to reference them between client and server side and being able to reuse validation rules across different types/fields.
+1. IMO it'd be nice to have a declared validation type over needing to call a helper/validator function everywhere (which I assume would be available as an option anyway for really complex validations that are not easy to declare). With a Validation type the validation becomes part of the language spec instead of being implementation specific. At some point I hope there are tools that will let you define a GraphQL API in the GraphQL language and then generate basic model/stubs in Java/JS et al.
Would stackable decorators work?
# contrived
input SomeInput {
@Length(min: 6, max: 14, error: "Invalid length etc....")
name: String
@OneOf(items: ["red", "green", "blue"], error: "Invalid colour can only be one of ${items}......")
favouriteColour: String
}
Ideally we're be able to
- apply multiple rules for given field
- summarize them
- introspect them, and
- customize them
@syrusakbary This issue has been open for 1.5 year now. Any updates on that?
The only way to achieve that right now seems to be:
- https://github.com/stephenhandley/graphql-validated-types by @stephenhandley
- https://github.com/xpepermint/graphql-type-factory by @xpepermint
Or did I miss something?
(issue https://github.com/graphql/graphql-js/issues/44 may also be relevant)
Happy to adapt graphql-validated-types as needed if helpful, currently its a fluent api but would be easy to accept validation args via constructor. I meant to more properly document the base class api for adding validators and the validation process.. that's here https://github.com/stephenhandley/graphql-validated-types/blob/master/src/GraphQLValidatedScalar.js#L47-L60
I just realized that this isn't much of a problem for me anymore because I'm now using Sequelize and doing most of my field validations in Sequelize itself.
Which makes me think:
- Many servers will at some point need to operate on their model layer directly, aside from any GraphQL requests.
- So the model layer should perform its own validation, lest any server code write invalid data to the database. If only GraphQL requests are validated, any operations on the database outside of a GraphQL context still run the risk of writing invalid data.
- Therefore, is it really necessary to validate args to GraphQL as well?
- Validation errors from the model layer can be converted to expressive GraphQL errors, eliminating the need for any validation in GraphQL beyond general data type validation.
My earlier thinking about this was skewed because I was using Rethink and didn't have any ORM or handwritten model abstraction enforcing a schema. At the time I thought GraphQL was a fine place to enforce the schema, but I realize now, it was probably not the best place.
Extra verification is not really a problem is it?
Also, in my experience, having the GrapQL schema be auto-generated from the DB paints you in a corner for later DB changes.
The schema your client app uses is very similar to the schema the server app uses, but not the same.
@jedwards1211 I was thinking the same but there might be a problem. I think that tools like GraphiQL rely on the GraphQL types to validate input, including arguments for mutations. That said, I'm trying to find a way/function that I can provide an object representing an input object for a mutation to validate the input before submitting it to the server.
@hisapy even if GraphQL types don't throw any validation errors, your query/mutation resolvers can check whatever they want and if they throw any error it will show up in GraphiQL and any other consumers of your API.
I like to validate objects in my resolvers with flow-runtime, check it out.
I mean that those tools, validate input (not data) as you type, because they know the schema so they don't have to send a request to the server render schema GraphQL errors locally.
I'm using Relay and I'd like to re-use the GraphQL types to validate the input object for a mutation. Currently mutation only shows a warning but submits anyway. I would like to avoid commit if the args don't pass the GraphQL validation.