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.

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

Discussion on query validation #191

Open
dwwoelfel opened this issue Apr 24, 2020 · 3 comments
Open

Discussion on query validation #191

dwwoelfel opened this issue Apr 24, 2020 · 3 comments

Comments

@dwwoelfel
Copy link
Contributor

We'd like to perform query validation against the schema before we execute queries. For reference, the spec includes a handy section on doing validation before execution: http://spec.graphql.org/draft/#sec-Validation

Looking at graphql-js, their validation is handled through a visitor pattern that applies some default validation rules: https://github.com/graphql/graphql-js/blob/master/src/validation/validate.js.

I've found graphql visitors to be very handy in general. For example, we use visitors in graphiql-explorer to perform some query transformations. The graphql-js repo has a good docstring on how they're implemented: https://github.com/graphql/graphql-js/blob/master/src/language/visitor.js#L140

Any thoughts on the best way to implement visitors with ocaml-graphql-server? Or maybe there is some better way to implement query validation in ocaml?

cc @sgrove

@andreas
Copy link
Owner

andreas commented Apr 24, 2020

Validation is definitely a sore point right now. I've previously started on some half-hearted attempts at implementing validation using visitors, e.g. using visitors, but didn't find it to be a great fit.

I've been very inspired by graphql-erlang approach to validation using bidirectional type checking. I have a fairly comprehensive proof-of-concept using that approach, though it still needs more work -- I can try whip it into shape. One interesting challenge I've found is how to capture in the type system that the query has been validated, such that executing the query does not validate the query again.

@anmonteiro
Copy link
Contributor

@andreas did you ever push your WIP branch for this? I'd love to take a look.

@andreas
Copy link
Owner

andreas commented Jul 20, 2022

@andreas did you ever push your WIP branch for this? I'd love to take a look.

I'm afraid I never pushed the branch, and I've lost the code in the transition to another computer. I thought I had a backup, but apparently not 😢

My vague recollection is that the implementation was a transiteration from graphql-erlang (primarily this file). I remember the theoretical aspects of bidirectional type checking carrying little importance in my implementation: it's probably what I would have come up with when implementing the validation logic in OCaml anyway. Performing the validation is generally quite straightforward, but is a bit tedious and requires being meticulous.

The hard part, as I also alluded to in my previous comment, is how to capture the fact that the query has been validated. Executing a validated query with suitable variables should only fail with `Resolve_error, not `No_operation_found, etc. I think coming up with a representation for a validated query is key in this regard, though there's also the cheap option out: keep validation in the execution step as in the current implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants