andreas / ocaml-graphql-server

GraphQL servers in OCaml
MIT License
624 stars 60 forks source link

Discussion on query validation #191

Open dwwoelfel opened 4 years ago

dwwoelfel commented 4 years ago

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 commented 4 years ago

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 commented 2 years ago

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

andreas commented 2 years ago

@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.