Executor: allow overriding AsyncExecute

[pkese]

Type Executor<'Root> is said to be "The standard schema executor"
however there's no way to change it, as all functions taking an Executor are accepting only instances of this type and thus execution process can not be overridden.

I needed to provide my own AsyncExecute that intercepts cases where result.Content = GQLExecutionResult.RequestError _ and in some cases defers execution elsewhere.

So I made AsyncExecute abstract in order to be able to override it.

After applying this pull request, I can now:

type CustomExecutor<'Root>(schema,middleware) =
    inherit Executor<'Root>(schema, middleware)
    override _.AsyncExecute(executionPlan: ExecutionPlan, ?data: 'Root, ?variables: ImmutableDictionary<string, JsonElement>): Async<GQLExecutionResult> =
        let result = base.AsyncExecute(executionPlan, ?data=data, ?variables=variables)
        async {
            let! result = result
            match result.Content with
            | (* ... fiddle with result *)
        }

[xperiandri]

Sounds reasonable.
However, could you describe your scenario in a bit more detail?
I want to better understand your challenge.

[pkese]

I was fiddling with directives and hit into a limitation with aliases:

query {
  People {
    name
    friends
    girlFriends: friends @filter(gender: Female)
  }
}

Notice how friends and girlFriends both refer to the same underlying friends field and you currently can't do that with this library.

---

Anyways ...
My architecture is that I'm parsing GraphQL requests and generating SQL queries (as SQL text) for my server, so the way I resolved this is:

• I render SQL query as normal but instead of executing it directly and parsing the responses back to dotnet types, ...
• I instead throw a CustomException with SQL text as a property of that exception class,
• then I intercept that CustomException in CustomExecutor.AsyncExecute, extract SQL text from exception and I call the database from there
• and I instruct my database to return a JSON string with FORMAT JSON
• so I can JsonDocument.Parse(databaseResponse)
• and return GQLExecutionResult.Direct(result.DocumentId, jsonElement, [], result.Metadata)

It works surprisingly well and is almost an order of magnitude faster than before.
Of course I lose type checking of database responses, but I have exact schema of the database so any type checking on the way back is just extra overhead.

[pkese]

An alternative could be to extend the IExecutorMiddleware and introduce another kind of middleware that has the ability process the GQLExecutionResult and return Async<GQLExecutionResult>

Or maybe I could have solved that with OperationExecutionMiddleware in the first place.
I was looking at the source code of this middleware invocation, but I didn't quite understand if this would let me alter the response after the executor is done.

If it is possible to alter the response in OperationExecutionMiddleware, then this Pull request may not be necessary.

[xperiandri]

Notice how friends and girlFriends both refer to the same underlying friends field and you currently can't do that with this library.

Should it be supported by spec?

[xperiandri]

That is an interesting case.
Don't you want to use LINQ generation code that already exists in the library?

[xperiandri]

GraphQL spec requires output items to be coerced if a database gives an invalid value. I thought about the scenario you have but considered following that coercion important

[xperiandri]

I think we need to consider some ability to pass JsonElement to a type as a result and allow the type to parse it.
Or do you want even higher performance with the ability to return the raw JSON?

[xperiandri]

Also for LINQ query generation, I think we need metadata that can map the field to a DB value with a different name or nested complex object field

[pkese]

If it is possible to alter the response in OperationExecutionMiddleware, then this Pull request may not be necessary.

Indeed,
it was possible to inject this functionality into OperationExecutionMiddleware, so this pull request can be closed.

[xperiandri]

Notice how friends and girlFriends both refer to the same underlying friends field and you currently can't do that with this library.

What about this? If it is supported by spec, let's open an issue so we track this functionality absence