Introduce ability to pre-parse Request's query

[DoumanAsh]

This would allow user to avoid parsing query himself in order to determine its semantics

@sunli829 This is a bit specialized use case that I found myself in need.
Right now it is difficult to determine type of operation to finalize query context (in our case it is selection of Database: whether to use replica or primary)
So I made this split which would allow to parse, check operation type and then finalize execution.

It does require our database connection to be customizable without mutable reference to Data which is inconvenient but can be worked around with almost zero overhead (which is better than we parse query twice 😄 )

[sunli829]

LGTM👏🏻, but there seems to be an issue that changes the execution order of the extension callbacks.

thread 'test_extension_call_order' panicked at 'assertion failed: `(left == right)`
[496](https://github.com/async-graphql/async-graphql/runs/6033591760?check_suite_focus=true#step:6:496)
  left: `["prepare_request_start", "prepare_request_end", "parse_query_start", "parse_query_end", "validation_start", "validation_end", "request_start", "execute_start", "resolve_start", "resolve_end", "resolve_start", "resolve_end", "execute_end", "request_end"]`,
[497](https://github.com/async-graphql/async-graphql/runs/6033591760?check_suite_focus=true#step:6:497)
 right: `["request_start", "prepare_request_start", "prepare_request_end", "parse_query_start", "parse_query_end", "validation_start", "validation_end", "execute_start", "resolve_start", "resolve_end", "resolve_start", "resolve_end", "execute_end", "request_end"]`', tests/extension.rs:270:9

[DoumanAsh]

Huh, strange, maybe I changed something after refactoring...
I'll check it out

[DoumanAsh]

@sunli829 Ah it makes sense, I wanted to avoid boxing future so I just split it into parts but now the lifecycle of extension changes 😞
I need to think on how to retain order

[DoumanAsh]

@sunli829 Hm looking back, it seems my idea is difficult to be implemented with current lifetime of extensions.
Would it be acceptable to change lifetime for two-time way to execute request, while retaining it the same for Schema::execute?

[sunli829]

Another way to allow create async_graphql::Request with a parsed query, what do you think?

[DoumanAsh]

@sunli829 Since query is public method I cannot just replace it with enum that would have parsed variant.
Do you suggest to add parsed_query: Option<ExecutableDocument> 🤔

[sunli829]

@sunli829 Since query is public method I cannot just replace it with enum that would have parsed variant.
Do you suggest to add parsed_query: Option<ExecutableDocument> 🤔

Yes, adding the parsed_query:Option<ExecutableDocument> field avoids breaking changes. 😁

[DoumanAsh]

@sunli829 I reworked my solution as per your suggestion.
Note that unfortunately it makes Request struct quite big so clippy started to complain about RequestBatch enum, and I had to silence it (cannot really break API for the sake of clippy, so maybe in major version bump)

[sunli829]

@sunli829 I reworked my solution as per your suggestion.
Note that unfortunately it makes Request struct quite big so clippy started to complain about RequestBatch enum, and I had to silence it (cannot really break API for the sake of clippy, so maybe in major version bump)

I don't think it's a big problem, usually there won't be a lot of Request objects, so it's not a waste. 😂

[sunli829]

I removed ParseQueryFut. 😁

[DoumanAsh]

Oh ok, no problem.
It is just habit of mine to use Future manually

[cynecx]

This change is actually breaking semver. Since code that would've had directly constructed this public struct would not compile anymore.

This struct should've been #[non_exhaustive] imo. 🤔

@sunli829 thoughts?

[sunli829]

Thanks a lot, I just realized this, so I created the async-graphql-v4 branch. 😁

[DoumanAsh]

oh... that's really unfortunate
I don't think we should allow Request to be constructed from fields in future.
It is just pain in ass to worry about compatibility