Handling Variables via JSON?

[russmatney]

Thanks much for all the work getting this GraphQL implementation together - it's no small task.

How would you recommend using this library with variables parsed from a JSON Request? It looks like #128 would accomplish that - just want to make sure this was actually not yet supported, as the path I'm on seems to require parsing the JSON (via Aeson) prior to interpreting the query.

Would love to see that PR merged soon!

[teh]

@russmatney I think for now parsing the JSON request yourself is the way to go. I haven't yet managed to convince myself that the FromJSON instance in #128 is what we want. My feeling is that if we have all the type information then we can do better than parsing Enum as a string but I may be wrong, too :)

[theobat]

I think we should add an example of this in the docs as people using graphql are really used to querying their API using a separate JSON object for the variables. Can I try to make a PR for this ?

Also, I'm not sure if handling a JSON object of variables is just "delayed" until we can figure out a better implementation than #128 or whether there is no decent implementation possible given the way things are right now ? (if that makes sense)

[teh]

@theobat PRs are very welcome, especially doc PRs!

JSON handling is delayed until someone has time to pick it up. My feeling is that the parsing should by type directed (i.e. same instance pattern matching we use for resolution) but I don't have the time in the foreseeable future to do this. It's a bit frustrating that it's stalled, but I prefer stalled over a solution I don't really believe in, especially because people can roll their own solution today (so they're not hard-blocked).

Does that make sense?

[theobat]

thanks for the quick answer, it does make sense.

[theobat]

So I'm playing around with this and here's what I'd like to do in another PR (But planning everything ahead seems like a safer bet):

• First of all I believe the right way to "parse" the variables passed as JSON from a query into graphQL types is by making use of the parsed text query, that is the AST.QueryDocument which is created in parseQuery, because it contains a list of VariableDefinitions which must consist in a name and a type.
• Now there are two steps to interpret the query, first of all building the AST and then, validating that it makes sense given the user's GraphQL schema (syntaxic and then semantic parsing). These operations respectively being parseQuery and validate
• So I believe what could be interesting would be to use the (syntaxic) parsing step to generate a list of VariableDefinition in the AST by parsing the JSON whenever we find a variable name matching a key in the JSON object provided as an argument. An even simpler (but probably less optimal) way would be to (sort of) inline as Text the JSON's value corresponding to the variable name. I am not sure yet how this:

  query plantList($plantFilter: PlantFilter) {
    plantList (plantFilter: $plantFilter) {

is represented in the AST exactly. I believe the argument part should be something like:

// first "node", the query name
Node "plantList" [VariableDefinition Variable "plantFilter" GType "PlantFilter"] ...etc
// second "node" the args "value"
Node "plantList" then and I'm not sure, is (plantFilter: $plantFilter) a variable definition ?, it should rather look like a value since it could be replaced by something like plantFilter: {"test": "ok"}

So please let me know if anyone has any suggestion on this, I'll continue my investigation anyway (and try to get the exact structure of the AST in this case).
As a side note all of my findings will probably go into a doc PR at some point especially regarding the AST (at least I hope to put together more examples with variables and input types).

[teh]

Points 1 and 2 sound like a good idea! I had forgotten that we're storing the GType in VariableDefinitions. Need a bit more time to understand part 3.

[jml]

It's very likely I'm missing something, so take all of this as lightly held, sleep-deprived opinion.

I don't know why we'd go to the AST to answer questions about variable definitions, rather than on the validated query.

The goal is to go from an Aeson.Value to VariableValues (an alias for Map Variable GraphQL.Value). Parsing from one to the other effectively requires knowing the variables defined in the query, which are available from QueryDocument. There's no QueryDocument -> VariableDefinitions function, but it's trivial to write.

The major downside of this approach is that have some type information that we can't get up to the type level, partly due to VariableValues being a Map and not some kind of awesome record thing. VariableDefinitions knows exactly which type is needed (at the value level), the parser will have to use that to parse the variable, but it will then stash it in a GraphQL.Value, which is pretty generic.

[theobat]

@jml yeah actually you're totally right, I came to the same conclusion myself just a few minutes ago by getting a better understanding of the true role of the AST while adding a test case on it.

So this brings only two functions to the table:

extractVariableDefinitions :: QueryDocument -> VariableDefinitions
parseJSONVariableValues :: Aeson.Value -> VariableDefinitions -> GraphQL.Value

[jml]

Yup, although parseJSONVariableValues will need a failure mode of some sort, and I'd swap the order of arguments for better currying opportunities. So, something like:

parseJSONVariableValues :: VariableDefinitions -> Aeson.Value -> Either VariableError GraphQL.Value

[theobat]

I'm having a very hard time transforming VariableDefinitions down to a bunch of Schema.InputTypeDefinition. The reason I critically need this is that a VariableDefinition is defined in Validation.hs as:

-- | Defines a variable within the context of an operation.
--
-- See <https://facebook.github.io/graphql/#sec-Language.Variables>
data VariableDefinition
  = VariableDefinition
    { variable :: Variable -- ^ The name of the variable
    , variableType :: AST.GType -- ^ The type of the variable
    , defaultValue :: Maybe Value -- ^ An optional default value for the variable
    } deriving (Eq, Ord, Show)

type VariableDefinitions = Map Variable VariableDefinition

And the AST.GType itself (which I thought was carrying enough information) is just the representation of what can be written in a query such as:
($myVar: MyVarType, $myListVar: [MyVarType!]!)
mirrored by :

data GType = TypeNamed NamedType
           | TypeList ListType
           | TypeNonNull NonNullType
           deriving (Eq, Ord, Show)

newtype NamedType = NamedType Name deriving (Eq, Ord, Show)

So, just a parsed version of the GraphQL input (which is completely fine, it's an AST after all).

Now, here's my problem, I need the entire schema information about MyVarType in my example to build a proper VariableValues object. Which means I need something along the lines of:

newtype Schema = Schema (Map Name TypeDefinition) deriving (Eq, Ord, Show)

Now we begin to see there's something bad for my use case in here: The schema does not directly expose InputTypes ... One could probably say it is normal, since the graphql Operations are represented as Type definitions, not InputType definitions, and their (optional) arguments are part of their definition. But it seems like traversing the entire schema every time we parse a JSON object of variables seems like a lot of work at runtime for no valid reason.
So either I write a function doing this traversal or, I change the schema API a little bit to expose a way to query input types (which by the way have the same namespace as regular types in graphql-js)

What do you think ?

[jml]

I’ll try to take a look on the weekend.

…

On Wed, 13 Jun 2018 at 22:07, theobat ***@***.***> wrote: I'm having a very hard time passing VariableDefinitions down to Schema.InputTypeDefinition. The reason I critically need this is that a VariableDefinition is defined in Validation as: -- | Defines a variable within the context of an operation. -- -- See <https://facebook.github.io/graphql/#sec-Language.Variables> data VariableDefinition = VariableDefinition { variable :: Variable -- ^ The name of the variable , variableType :: AST.GType -- ^ The type of the variable , defaultValue :: Maybe Value -- ^ An optional default value for the variable } deriving (Eq, Ord, Show) type VariableDefinitions = Map Variable VariableDefinition And the AST.GType itself (which I thought was carrying enough information) is just the representation of what can be written in a query such as: ($myVar: MyVarType, $myListVar: [MyVarType!]!) mirrored by : data GType = TypeNamed NamedType | TypeList ListType | TypeNonNull NonNullType deriving (Eq, Ord, Show) newtype NamedType = NamedType Name deriving (Eq, Ord, Show) So, just a parsed version of the GraphQL input (which is completely fine, it's an *AST* after all). Now, here's my problem, I need the entire schema information about MyVarType in my example to build a proper VariableValues object. Which means I need something along the lines of: newtype Schema = Schema (Map Name TypeDefinition) deriving (Eq, Ord, Show) Now we begin to see there's something bad for my use case in here: The schema does not directly expose InputTypes ... One could probably say it is normal, since the graphql *Operations* are represented as Type definitions, not InputType definitions, and their (optional) arguments are part of their definition. But it seems like traversing the entire schema every time we parse a JSON object of variables seems like a lot of work at runtime for no valid reason. So either I write a function doing this traversal or, I change the schema API a little bit to expose a way to query input types (which by the way have the same namespace as regular types in graphql-js) What do you think ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#145 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHq6vLJYzvjLaVq8kn4F2iNt6kUBfB3ks5t8X8UgaJpZM4Rh2LZ> .

[jml]

I mostly follow, but then I don't. Not sure whether it's because I'm not deep enough into the details of the project or because you're using terminology loosely. Anyway, this is where I lose you:

Now, here's my problem, I need the entire schema information about MyVarType in my example to build a proper VariableValues object.

What's "the entire schema information" here?

Which means I need something along the lines of:

newtype Schema = Schema (Map Name TypeDefinition) deriving (Eq, Ord, Show)

I don't know what you mean by Schema. That word gets used a lot already. I guess you mean a schema specifically for input variables?

If so, that would be Schema.InputTypeDefinition (or possible Schema.InputType, I'm not certain).

But it seems like traversing the entire schema every time we parse a JSON object of variables seems like a lot of work at runtime for no valid reason.

OK, sure, but I'm not sure exactly what proposal you're rejecting.

I change the schema API a little bit to expose a way to query input types (which by the way have the same namespace as regular types in graphql-js)

I don't understand what you mean here, and I don't get why namespaces are relevant.

Here's my take:

• returning AST.GType in VariableDefinition is under-baked, we really should be returning an InputTypeDefinition (or InputType) at that point
• once that's done, all of this devolves into the same problem as argument coercion, with the same solutions

Does this help?

[theobat]

because you're using terminology loosely

Yes it's probably a lack of accuracy on my end, plus mixing things together, sorry about that. The main idea I was trying to convey (which I think you essentially understood given your answer) is that:

• A variable defined in a query document has a type, its type is defined in the query document by a reference -through the type name- to an existing type in the graphQL schema. This is a partial/referential schema information and it's mirrored by AST.GType.
• To resolve my variables from JSON, I need the type definition not just its name, this is what I meant by the entire schema information.

Now, I agree with your first point, but going from AST.GType to InputTypeDefinition/TypeDefinition is merely finding the correct (if any...) InputTypeDefinition/TypeDefinition in the schema given its name. This is what brought me to say:

But it seems like traversing the entire schema every time we parse a JSON object of variables seems like a lot of work at runtime for no valid reason.

I think we should have a simple Map between graphql type names and graphql type definitions for all the types of a schema. It would transfer some work to GraphQL.makeSchema and save some work in query treatment.
Edit: this is already how the Schema type behave, for some reason I thought the InputTypes were not reachable like the others but I was wrong.

So what I propose is:

• change VariableDefinition to embed an Schema.AnnotatedType Schema.InputTypeDefinition instead of AST.GType (just like you said)
• change validateVariableDefinition to take the Schema into account (verify the type defined in the query document actually exist, and pass it to VariableDefinition constructor.
• add a way to find any type defined in a graphql schema given its name in constant time (this is optional but important, we could traverse the entire Schema every time we look for an InputTypeDefinition)

Does this help?

Yes !!

[jml]

So what I propose is:

• change VariableDefinition to embed an Schema.AnnotatedType Schema.InputTypeDefinition instead of AST.GType (just like you said)
• change validateVariableDefinition to take the Schema into account (verify the type defined in the query document actually exist, and pass it to VariableDefinition constructor.

Sounds great!