Extract query validation into its own crate?

[tomhoule]

I am working on improving the graphql-client crate and one place where it could use better ergonomics is query validation. Since it is defined by the spec and already implemented by Juniper, I feel like it would be wasted work, and it would be vastly preferable to share one high quality Rust crate for validation of a GraphQL query against a schema.

Of course this would be using the juniper ASTs for the query and the schema, but I think mappings could be defined without too much work. Juniper is the only crate where performance matters, since graphql-client validates queries at compile-time, so the conversion overhead/parsing twice is acceptable there.

This is not the only use-case for such a crate, it could be useful on its own with a very thin wrapper CLI. One use-case I have at work would also be to warn when a query is using deprecated fields.

Since I am not very familiar with the Juniper codebase, I may be missing difficulties. Do you think this is desirable/possible? I would be happy to make a PR or help in any other way.

[LegNeato]

I've personally never touched the validation code so I can't speak to the feasibility or difficulty, but to me it sounds like a great idea.

[tomhoule]

Cool, I'm in vacations now but I'll try to find time to attempt this when I'm back.

[davidpdrsn]

@tomhoule Did look into this? I have the exact need you mentioned of building a CLI tool to validate queries against a schema. If you didn't spend time on it I might wanna give it a shot 😄 But I make no promises.

[tomhoule]

I haven't had time for open source work recently, sadly, so I haven't done anything on that front. Feel free to go ahead :)

[davidpdrsn]

I have started looking into this but since I'm new to this project I have a few questions:

1. Currently juniper::execute starts off by validating the variables and query against juniper's representation of a schema (RootNode<QueryT, MutationT, S>). So pulling out a function that only does the validation is quite easy, however it requires a juniper::RootNode type. "graphql-client" doesn't have such a type because it uses "graphql-parser". So is the AST mapping that @tomhoule mentioned mapping from graphql_parser::schema::Document to juniper::RootNode?
2. Performing such a mapping will probably require all the things in juniper::ast, but most of that module is currently private. Should it be made public or moved somewhere else? The existing validation code also depends heavily on juniper::ast.
3. Is the end goal to have functions like these

   // For use in juniper, with better name
   fn validate_for_juniper(
     query: &'a str,
     variables: HashMap<String, _>,
     root_node: &'a RootNode<QueryT, MutationT, S>,
   ) -> Result<(), Vec<ValidationError>> {
       // ...
   }
   
   // For use in graphql-client
   fn validate(
     query: &'a str,
     variables: HashMap<String, _>,
     schema: &'a str,
   ) -> Result<(), Vec<ValidationError>> {
       let root_node = schema_to_root_node(schema);
       validate_for_juniper(query, variables, root_node)
   }

[tomhoule]

For your points 1 and 2, that is what I imagined yes. In graphql-client, we could either: 1. parse the query twice (one for the graphql-parser AST, another for the juniper AST to perform validation with), 2. make juniper's query parser public and use that (it would be a bit of work though), 3. define a mapping from graphql_parser's AST to juniper's query AST.

Solution 1 is obviously slow but I think it would be acceptable, since it all happens at compile time.

Whatever the final solution, the query and schema roots will have to be exposed, I think.

[allancalix]

I'm interested in being able to validate separately from execution, I'm happy to work on this if no one is currently working on it

[tyranron]

@allancalix we're not against this, go ahead!

Regarding the solution, I don't see reasons to move the validation into a separate crate. But making a nice public API for using it - quite good to go 👍

[allancalix]

Sorry it's been a while, I ended up going in a different direction because at the time there was no way to initialize a schema from SDL files which I was looking to do. Going to unassign myself