andreas / ocaml-graphql-server

GraphQL servers in OCaml
MIT License
622 stars 60 forks source link

Distinguish between null and missing in args #179

Open dwwoelfel opened 5 years ago

dwwoelfel commented 5 years ago

Is there some way to distinguish between the user passing in null and the user passing in nothing for an arg?

I have a use case for an update mutation that requires me to distinguish between missing and null. If the value was provided as null, then the underlying field in storage needs to be nulled out, but if the value is missing, then the underlying field should remain unchanged.

Example:

type Person {
  id: Int!
  firstName: String
  lastName: String
}

input PersonInput {
  id: Int!
  firstName: String
  lastName: String
}

type Mutation {
  updatePerson(person: PersonInput): Person
}
mutation {
  updatePerson(person: {id: 1, firstName: "Daniel", lastName: "Woelfel"}) {
    id
    firstName
    lastName
  }
}

# results in {id: 1, firstName: "Daniel", lastName: "Woelfel"}
mutation {
  updatePerson(person: {id: 1, firstName: "Dan"}) {
    id
    firstName
    lastName
  }
}

# should result in {id: 1, firstName: "Dan", lastName: "Woelfel"},
# not {id: 1, firstName: "Dan", lastName: null}
andreas commented 5 years ago

Sorry for the slow response. I have two ideas for how to solve this:

The second approach is a bit of a hack to implement outside of the library though. I haven't thought it through, but maybe it should be a native feature of OGS.

ozanmakes commented 5 years ago

I'm not very familiar with the GraphQL spec, but this discussion could be relevant: https://github.com/baransu/graphql_ppx_re/issues/26#issuecomment-542182296

If this is correct, the current behavior of raising an error when nullable variables are missing from the variables dictionary does not align with the spec.

andreas commented 5 years ago

I'm not very familiar with the GraphQL spec, but this discussion could be relevant: baransu/graphql_ppx_re#26 (comment)

If this is correct, the current behavior of raising an error when nullable variables are missing from the variables dictionary does not align with the spec.

Can you elaborate on what you mean by this? And feel free to open a separate issue -- at first glance it does not appear related to this issue (though I could be mistaken) 😄

ozanmakes commented 5 years ago

@andreas It's more of a question I guess, I don't know what behavior is correct and I'm curious whether a possible OGS solution to this issue would be relevant to my issue.

query getUsers($first: Int, $after: Cursor) {
  users(first: $first, after: $after) {
    nodes {
      id
    }
  }
}

If I call this query with { "first": 10 } I get this error back:

{
  "errors": [
    {
      "message": "Missing variable `after`"
    }
  ],
  "data": null
}

while { "first": 10, "after": null } works.

In fact, this was the way graphql_ppx worked. It would force me to declare every variable and construct something like { "first": Some(10), "after": None }, and it would encode None with null.

After switching to graphql_ppx_re because of newer BuckleScript support I noticed my null (None) values started to get dropped, which ocaml-graphql-server did not like.

During the subsequent discussion in the issue the author said he made this change to work around an issue with Sangria, but also that this change is compliant with the spec.

Now I realize that my issue is about variables passed to the query, which is not this issue is about. But I'm still curious about your opinion on this, is this something I should attempt to solve on the client side, or at ocaml-graphql-server side?