aPureBase / KGraphQL

Pure Kotlin GraphQL implementation
https://kgraphql.io
MIT License
305 stars 58 forks source link

Make Context and Execution.Node accessible via Receiver parameter #145

Open NathanPB opened 3 years ago

NathanPB commented 3 years ago

Hello, I see that currently, to access the Context or Execution.Node we have to do something like:

resolver { foo: String, ctx: Context -> /* ... */ }

which implies that we need to add rules to the schema parser to ignore those classes when kgraphql is reflecting the FunctionWrapper<*>'s arguments:

https://github.com/aPureBase/KGraphQL/blob/main/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/SchemaCompilation.kt#L162-L182

So, if I'm hypothetically working on #68, and I want to add a new type that should not be introspected (let's say List<Throwable>), I should create a wrapper class (like Context is just a wrapper for Map<Any, Any>), ignore this class in the schema introspection, and inject an object of this class into the FunctionWrapper's arguments when invoking it... Am I Right?

Kotlin supports Receiver Functions (https://kotlinlang.org/docs/lambdas.html#function-types), which, are, for instance, A.(B)->C where in this lambda, A will be the this context of the execution.

So, why instead of injecting Contexts and Execution.Node's in the Resolvers, we create a ExecutionScope class and make this class the receiver of the resolvers?

class ExecutionScope(val context: Context, val node: Execution.Node, val errors: MutableList<Throwable>)

// This, obviously adapted to work with ``FunctionWrapper``
fun <A, B> resolver(ExecutionScope.(A)->B: body) { ... }

resolver { foo: String ->
   /* context, node and errors are accessible here. */
   /* We can do something like errors += Throwable("bondia") */ 
}

This will make it much easier and cleaner to work with contexts and other runtime wrappers.

I have been thinking about this while reading the code, but I did not attempt to do anything yet, do you @jeggy thinks that this issue makes sense? Any advices?

jeggy commented 3 years ago

Hi, I'm not sure if having access to the errors directly in the resolvers makes sense? as you wouldn't care in one resolver that another resolver failed.

I'm a huge fan of your suggestion for the ExecutionScope :+1:

I never got back to #68, but what I was thinking to solve that one is to change the return type of Schema.execute to return a ExecutionResult object instead of only a String. Then the result within this ExecutionResult object would follow the rules as specified in §7.1.2 and then provide the a non mutable list of errors in ExecutionResult also.

Maybe there should be support for adding custom errors in ExecutionScope, but I'm mostly thinking you would just throw an exception within your resolver and then the RequestExecutor will take care of adding it to the ExecutionResult.errors

NathanPB commented 3 years ago

I'm not sure if having access to the errors directly in the resolvers makes sense? as you wouldn't care in one resolver that another resolver failed.

I was thinking of something like throwing multiple errors in one resolver, but it really sounds like it doesn't pay the effort.

My thoughts were like

class ExecutionScope {
  fun throwAbort(t: Throwable) = ...
  fun throwContinue(t: Throwable) = ...
}

but this is all starting not to get related to this issue.

Digging a bit more I see how my suggestion is far from easy imp (as I thought) :(