Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for query variables #154

Closed
iancw opened this issue Jun 1, 2016 · 11 comments · Fixed by #158
Closed

Support for query variables #154

iancw opened this issue Jun 1, 2016 · 11 comments · Fixed by #158

Comments

@iancw
Copy link
Contributor

iancw commented Jun 1, 2016

It looks like express-graphql and graphql-ruby accept queries with query and variables in separate structures. For example, here is a query generated by the express-graphql client library.

{
   "query":"query AppHomeRoute($uuid_0:String!) {
         account(uuid:$uuid_0) { 
               ...
         }
     }",
   "variables":{"uuid_0":"88888888-4444-4444-4444-121212121212"}
}

Here are a couple articles discussing variables:

It doesn't look like the graphql-java server accepts query variables like this currently. It seems like this would be a nice feature to have, so you can use graphql-java servers with the relay-starter-kit.

@danielkwinsor
Copy link
Contributor

The GraphQL object accepts variables aka arguments
public ExecutionResult execute(String requestString, Object context, Map<String, Object> arguments)`
You can set context to null in this case.

@iancw
Copy link
Contributor Author

iancw commented Jun 7, 2016

I've expanded the provided example for a bit more context. When I put a variable named uuid_0 in the arguments map, I still get ValidationError: Validation error of type UndefinedVariable: Undefined variable uuid_0 during validation. I also attempted $uuid_0 with same result.

When I provide the values statically, the framework automatically puts them into the arguments map. For example, this works.

query  {
    account(uuid:"88888888-4444-4444-4444-121212121212") {
        ...
    }
}

And uuid shows up as a key in the arguments map. It looks like the Graphql grammar the parser is based on understands variables, but there is no grammar for specifying the values of said variables. They appear instead to be passed as part of a JSON request alongside the GraphQL query by react / express-graphql and graphql-ruby in a extra-graphql capacity (as described above).

I see no mechanism to set them in graphql-java currently. I could easily write a wrapper in my service to parse JSON response and stick the variable definitions somewhere. There doesn't appear to be a place though. If I'm corrected, I'll happily submit a PR to the README that documents how to do this.

@iancw
Copy link
Contributor Author

iancw commented Jun 7, 2016

So I just found your TODO MVC example, and see that you do in fact use variables as described above (https://github.com/graphql-java/todomvc-relay-java/blob/master/src/main/java/todomvc/GraphQLController.java#L33-L34). And also it seems to be working fine for me now. Maybe I just needed to clean & rebuild or something. Working on a PR to update README as promised.

@iancw
Copy link
Contributor Author

iancw commented Jun 11, 2016

I submitted a PR to README to document how variables are supported. Not sure when / if that will merge, so I'm just closing this issue. I have my graphql-Java server working with relay following advice from @danielkwinsor. Thanks!

@iancw iancw closed this as completed Jun 11, 2016
@ghost
Copy link

ghost commented Aug 30, 2017

Are variables really supported? Maybe I am missing something, but the following code always results in an error "Validation error of type UndefinedVariable: Undefined variable personId"

String query = "query { person(id: $personId) { id, name } }";
Map<String, Object> variables = new HashMap<>();
variables.put("personId", "42");

ExecutionInput input = ExecutionInput.newExecutionInput().
    query(query).
    variables(variables).
    build();
ExecutionResult result = graphQL.execute(input);

The following works without problems:

String query = "query { person(id: \"42\") { id, name } }";
ExecutionInput input = ExecutionInput.newExecutionInput().
    query(query).
    build();
ExecutionResult result = graphQL.execute(input);

I am using graphql-java 4.0

@SebastianWjertzoch
Copy link

I have the exact same problem with version 4.1.

@andimarek
Copy link
Member

andimarek commented Sep 9, 2017

@DevTomK @SebastianWjertzoch variables are fully supported ... your query is wrong.

you have to declare each variable with type ... for example:

query nameOfTheQuery($nameOfTheVariable: Int) { 
  foo(someArgument: $nameOfTheVariable) 
} 

@SebastianWjertzoch
Copy link

SebastianWjertzoch commented Sep 10, 2017

We both use the ExecutionInput. So type declaration is not part of the query.

Like this
String query = "query { person(id: $personId) { id, name } }";
Map<String, Object> variables = new HashMap<>();
variables.put("personId", "42");

ExecutionInput input = ExecutionInput.newExecutionInput().
query(query).
variables(variables).
build();
ExecutionResult result = graphQL.execute(input);

@andimarek
Copy link
Member

@SebastianWjertzoch as mentioned above: you have to declare the variable.

you query is

query { person(id: $personId) { id, name } }

but it is missing the declaration of the variable. It should like this:

query myQuery($personId: Int) { person(id: $personId) { id, name } }

I hope this helps

@SebastianWjertzoch
Copy link

SebastianWjertzoch commented Sep 11, 2017

Ok. So i have to build an special query for every kind of request. I was confused because this:
query { person(id: \"42\") { id, name } }
works fine without special queries.

@andimarek
Copy link
Member

well ... if you wanna use a variable you have to declare the variable.

the query you mention works fine, because you don't use a variable, hence you don't have to declare it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants