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

enum coercion in mutation argument from string does not work... #122

Closed
johanatan opened this issue Aug 12, 2015 · 17 comments
Closed

enum coercion in mutation argument from string does not work... #122

johanatan opened this issue Aug 12, 2015 · 17 comments

Comments

@johanatan
Copy link
Contributor

... however coercion from int does (arguably coercion from string is more useful than coercion from int).

Schema:

enum Color { blue, orange, green } // mapped to 1, 2, 3

type Favorite {
  id: ID!
  color: Color
}

Client:

curl -XPOST -H 'Content-Type:application/graphql' -d '{"query":"mutation createFavorite($color: Color!) { createFavorite(color:$color) { color } }", "params":{"color":2}}' http://localhost:3000/graphql    
{
  "data": {
    "createFavorite": null  // Server is returning null at the moment but server logs are pasted below.
  }
}

Server:

Received insert request: uid: c4e1dd90-9b89-4fcf-8b6b-c32e9f63dccb, entity: Favorite, params: {"color" "{\"orange\" {
Returning: null

Bad request:

curl -XPOST -H 'Content-Type:application/graphql' -d '{"query":"mutation createFavorite($color: Color!) { createFavorite(color:$color) { color } }", "params":{"color":"orange"}}' http://localhost:3000/graphql    
{
  "errors": [
    {
      "message": "Variable $color expected value of type Color! but got: \"orange\".",
      "stack": "Variable $color expected value of type Color! but got: \"orange\".",
      "locations": [
        {
          "line": 1,
          "column": 25
        }
      ]
    }
  ]
}
@johanatan johanatan changed the title enum coercion from string equivalent does not work... enum coercion in mutation argument from string does not work... Aug 12, 2015
@leebyron
Copy link
Contributor

As of 2cf2f4c this should now be working as specified.

@johanatan
Copy link
Contributor Author

[deleted] Sorry for the false alarm-- was still pulling in the old version.

@johanatan
Copy link
Contributor Author

Actually, I am still seeing this issue with the latest code:

curl -XPOST -H 'Content-Type:application/graphql' -d '{"query":"mutation createFavorite($color:Color!) { createFavorite(color:$color) { color } }", "params":{"color":"blue" } }' http://localhost:3000/graphql    
{
  "errors": [
    {
      "message": "Variable \"$color\" expected value of type \"Color!\" but got: \"blue\"."
    }
  ]
}

And a new one (invalid JSON because 'blue' isn't quoted or in scope):

curl -XPOST -H 'Content-Type:application/graphql' -d '{"query":"mutation createFavorite($color:Color!) { createFavorite(color:$color) { color } }", "params":{"color":blue } }' http://localhost:3000/graphql    
{
  "errors": [
    {}
  ]
}

@leebyron
Copy link
Contributor

Your test here looks the same as 2cf2f4c#diff-a333d8645ed867ad7e82181baa05ad14R135 which is currently passing.

Could you draft a test case that includes your definition files? That would help me understand if there is a bug that I'm not seeing here, or I can help you understand if there is an issue with your code that I could improve error messaging for.

@johanatan
Copy link
Contributor Author

Oh, I see the problem. In my query declaration, I'm specifying the actual GraphQLEnumType ('ColorType' in your case) for the type field of the pertinent arg.

Attempting to coerce the supplied value into the expected value was what I expected. Otherwise, there would be a lot of unnecessary, repeated boilerplate for each query wanting to take advantage of this coercion.

Here's my mutation definition (please pardon the ClojureScript syntax) ["Color" is a GraphQLEnumType]:

Mutation descriptor for typename: "Favorite": {"createFavorite"
 {:type #<Favorite>,
  :args {"color" {:type #<Color>}}, 
  :resolve #<function (a, b) { [omitted] } }

@johanatan
Copy link
Contributor Author

P.S. from my reading of the code yesterday, the only problem seemed to be that coerceValue wasn't being called during the validation phase-- the functionality is already there that will do the coercion properly-- it just needs to be invoked before (or very early on) in validation.

@leebyron
Copy link
Contributor

I'm curious what the definition for Color is - perhaps it is ill-defined if you're getting the error message "Variable "$color" expected value of type "Color!" but got: "blue"."? and "blue" is in fact one of the names of your Color enum values?

@leebyron
Copy link
Contributor

P.S. from my reading of the code yesterday, the only problem seemed to be that coerceValue wasn't being called during the validation phase-- the functionality is already there that will do the coercion properly-- it just needs to be invoked before (or very early on) in validation.

I'm not sure I follow this. Validation is a static-time pass that happens before a query is executed. Variable values ("blue" in this case) are only introduced at Execution time.

@johanatan
Copy link
Contributor Author

Oh, sorry. I was confusing with the other bug-- i.e., allowing coercion in arguments as well as parameters. I just figured the two were related.

@leebyron
Copy link
Contributor

Gotcha. That one should be covered. parseLiteral is called on literal argument values during validation, which catches this kind of issue very early.

@johanatan
Copy link
Contributor Author

Here's the descriptor for my Color (once again in Clojure syntax) [it looks like there is bug in my generation logic].

Created enum type: Color: descriptor: 
{:name "Color", 
:values {
   {"blue" {:value 1}} {:value 1}, 
   {"orange" {:value 2}} {:value 2},
   {"red" {:value 3}} {:value 3}}}

values is a hash of hashes to hashes, lol [probably not what the engine was expecting].

Sorry for the collossal waste of your time! And thanks for your help!

@leebyron
Copy link
Contributor

No problem, this is actually really valuable information. I will add assertion messages that enums are properly constructed. That could have saved you a lot of time, I'm sure!

@johanatan
Copy link
Contributor Author

I'm sorry to report that I'm still getting the error.

My 'Color' enum is now described by:

Created enum type: Color: descriptor: {:name "Color", :values {:blue {:value 1}, :orange {:value 2}, :red {:value 3}}}

And this is the request:

curl -XPOST -H 'Content-Type:application/graphql' -d '{"query":"mutation createFavorite($color:Color!) { createFavorite(color:$color) { color } }", "params":{"color":"blue" } }' http://localhost:3000/graphql   

[EDIT: oops, sorry. Please ignore my analysis of your code previously pasted here].

Since you do seem to have the cases to cover this, I will keep searching in my codes for the bug.

@johanatan
Copy link
Contributor Author

Actually, there is a difference between our test cases here: mine involves a mutation and yours is a plain query. Here's the descriptor for my mutation:

Creating GraphQLObjectType: {:name "RootMutation",
 :fields
 {"createFavorite"
  {:type #<Favorite>,
   :args {"color" {:type #<Color>}}, 
   :resolve #<function (a, b) {
              return frontend.datasvc.create.call(null, g.data_resolver, m, cljs.core.js__GT_clj.call(null, b));
            }>}, ...

i.e., this object is placed on the mutation field of the GraphQLSchema rather than the query field as in your example.

@leebyron
Copy link
Contributor

That shouldn't change the test case - the code for checking variable values happens before determining the type of operation.

@johanatan
Copy link
Contributor Author

Ahh, cool. Good to know.

leebyron added a commit that referenced this issue Aug 14, 2015
This asserts that Enums have properly configured values.

This also asserts that isTypeOf and resolveType are functions, if provided.

Finally, this changes the signature of EnumType.getValues() to return an array instead of an Object map, since every single use case was calling Object.keys.

This fixes #122
@jdrelick-va
Copy link

jdrelick-va commented Nov 27, 2018

Also relevant: #1324

So this is the most relevant issue I could find... I am having a similar issue where using the actual Enum Constant value (not a string) in GraphiQL coerces my Enum key to it's specified value correctly. When sending JSON instead, however, I don't get a GraphQL schema error (although in GraphiQL I do) but the input value is not coerced:

export const StatusType = new GraphQLEnumType({
    name: "enumStatusType",
    values: {
        "ACTIVE": {
            value: "Active"
        },
        "PAUSED": {
            value: "Paused"
        },
        "STOPPED": {
            value: "Stopped"
        },
        "PENDING": {
            value: "Pending"
        },
        "DRAFT": {
            value: "Draft"
        }
    }
});

This will coerce ACTIVE -> "Active" (in GraphiQL):

mutation {
  bulkEdit(
    mutations: [
      {
        _kind_: "user"
        _id_: "123"
        _operation_: "edit"
	status: ACTIVE
      }
    ]
  )
}

This will not:

{
  "query": "...",
  "variables": {
     "mutations": {
       ...
       "status": "ACTIVE"
     }
  }
}

The main difference is one uses variables over JSON and therefore MUST use a string ("ACTIVE" in this case), while the other does not. The JSON version will not coerce "ACTIVE" to "Active" for usage in the backend, though it does for reads ("Active" -> "ACTIVE").

I hope this makes sense and thank you.

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

No branches or pull requests

3 participants