aPureBase / KGraphQL

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

Access request properties #94

Closed AlexeySoshin closed 4 years ago

AlexeySoshin commented 4 years ago

Tackles PR #40 Currently I'm putting those on the context, to have an easy access to them.

Need to understand what to do with nested objects. Probably returning a map of maps or some dot notation, eg actor.name, would make more sense.

AlexeySoshin commented 4 years ago

@jeggy WDYT?

jeggy commented 4 years ago

@jeggy WDYT?

I think maybe instead of us providing a simple string array, we should provide the full object. Not sure if that should be the DocumentNode or ExecutionPlan. Then we could provide some easy to use extension functions to get a string array like what you have done here on top of that.

But I think how exactly this will be used could be up to the developers using this library.

If we allowed requesting the Execution.Node object like this:

resolver { myArg: Int, ctx: Context, gqlNode: Execution.Node ->
  // `getFields` would be an extension function that returns all scalar type fields requested on this type.
  val fields: List<String> = gqlNode.getFields()
  println("SELECT ${fields.joinToString()} FROM MY_TABLE WHERE USER_ID = ${ctx.get<User>().id}")
  ...
}

This would also give anyone that needs it access to the whole query tree, both up and down by giving them access to the SelectionNode.

AlexeySoshin commented 4 years ago

I'll take a look into that.

AlexeySoshin commented 4 years ago

@jeggy Now I'm putting execution nodes on the context. Not sure if this could be done, though:

resolver { myArg: Int, ctx: Context, gqlNode: Execution.Node ->

And I think we're getting a list of them, and not the root one.

jeggy commented 4 years ago

I tried to implement this without changing anything regarding the Context.

Please take a look here: https://github.com/aPureBase/KGraphQL/pull/99/files#diff-355fb3d2890c86fe5ee3371d7c32ef48R284-R325

AlexeySoshin commented 4 years ago

Yep, that's smarted way of doing this :)

I think I'll close this PR then.