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

Query parameters don't get converted to appropriate GraphQL types #41

Open
thecodeboss opened this issue Apr 15, 2021 · 6 comments
Open
Labels
enhancement New feature or request

Comments

@thecodeboss
Copy link

Description

When passing arguments in a query string, there is no type conversion step. So strings don't get converted into integers, etc. For example, an HTTP request like this one:

GET https://myapi.com/projects?limit=100

would attempt to pass the limit parameter as a string "100" instead of consulting the GraphQL schema and realizing it is supposed to take an Int type instead.

It's possible to define a middleware that converts these, however I'd like to suggest it as a feature instead. Most GraphQL built-in scalar types I think could be automatically converted, but obviously custom scalars or non-scalars would still require custom middleware to be built.

Another workaround is to use the request body instead of the query string, as JSON allows you to specify scalars without ambiguity.

I would be happy to take a stab at creating a pull request for this if there's no immediate objection to the idea.

Steps to Reproduce

Set up a graphql2rest project with the following GraphQL Schema:

type Project {
  id: ID!
}

type Query {
  projects(limit: Int): [Project!]!
}

this manifest.json:

{
  "endpoints": {
    "/projects": {
      "get": {
        "operation": "projects"
      }
    }
  }
}

and then execute this HTTP request:

GET https://myapi.com/projects?limit=100

You should get a GraphQL error like the following:

Variable "$limit" got invalid value "100"; Expected type Int. Int cannot represent non-integer value: "100"
@roy-mor
Copy link
Collaborator

roy-mor commented Apr 16, 2021

Thanks for this issue @thecodeboss.

Just to clarify - what you specified under "You should get a GraphQL error like the following" is the current behavior, yes? Which is a problem because the API user specified an integer value for the query parameter, but it got interpreted as a string, thereby violating the schema and so the input validation failed, resulting in an API request with valid input failing to execute?

@roy-mor roy-mor added the enhancement New feature or request label Apr 16, 2021
@MatthewHerbst
Copy link

Just to clarify - what you specified under "You should get a GraphQL error like the following" is the current behavior, yes? Which is a problem because the API user specified an integer value for the query parameter, but it got interpreted as a string, thereby violating the schema and so the input validation failed, resulting in an API request with valid input failing to execute?

Yes, that's the current behavior. The steps above are listed under a Steps to Reproduce heading

@thecodeboss
Copy link
Author

Just to clarify - what you specified under "You should get a GraphQL error like the following" is the current behavior, yes?

Yes

@roy-mor
Copy link
Collaborator

roy-mor commented Apr 17, 2021

@thecodeboss OK good. I wanted to make sure. Please go ahead and create a pull request for this. Please make sure to add appropriate tests for your change. Thank you!

@roy-mor
Copy link
Collaborator

roy-mor commented May 25, 2021

@thecodeboss any progress on your suggested PR?

@shuffledex
Copy link

We need that PR ASAP!

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

No branches or pull requests

4 participants