Support oneOf directive on input objects

[asteinwedel]

Issue: https://github.com/sangria-graphql/sangria/issues/1068

WIP.

The main thing I'm missing is when checking that there's "exactly one non-null field" on a query (I have the ExactlyOneOfFieldGiven rule), I'm struggling with validating against ast.VariableValue (since I need the unmarshalled input for that). (and considering default values for VariableDefinition -- though that seems easier than getting the actual input)

was wondering if it needs to go in the execution part for checking variables, but that seems a bit messier.

Trying to figure it out...but feel free to contribute if you have a way to go about that

[yanns]

first of all, thanks for your contribution! I see it's already going well, with good test coverage.

For the missing validation, do you already have failing tests for it? It would save me time.

[asteinwedel]

first of all, thanks for your contribution! I see it's already going well, with good test coverage.

For the missing validation, do you already have failing tests for it? It would save me time.

I do not, as the tests I have setup aren't setup for variables.

Testing something like this would be a basic case (and then also default values, if you get something setup I can add extra test cases)

val schema = gql"""
   type Query {
      oneOfQuery(input: OneOfInput!)
  }

  input OneOfInput @oneOf {
    foo: String
    bar: Int
  }
"""

val query = """
  query Test($foo: String) {
    oneOfQuery(input: { foo: $foo, bar: 123 })
  }
"""

val variables = """{ "foo": "testing" }"""

[asteinwedel]

@yanns I got something working (well passes the tests I added), though it causes some breaking changes with QueryValidator needing variable values now, and had to move things around for that

working on making sure build / tests passes

but that change is in its own commit

[asteinwedel]

hmm moving validation after variable coercion (since we need that) needs a bit more tuning to make sure validation errors get thrown before variable coercion errors

[asteinwedel]

alright...seems passing now

[asteinwedel]

let me know if think of a better way to check the variable values

[yanns]

Just taking notes:

• we need to validate: "making a OneOf Input Object into a regular Input Object is a non-breaking change (the reverse is a breaking change)"
• would it be better to follow all test cases documented in the spec?
• about the validation of variables with the QueryValidator: it's indeed quite a big change. But I don't see how we can do without this. We could split the validation in 2: one without and one with the variables. But I found it's quite nice to have all validations inside ExactlyOneOfFieldGiven. Maybe I could release a release candidate version, and we could check if we're breaking much.

[yanns]

I almost forgot: we need to test for breaking changes (https://github.com/sangria-graphql/sangria/blob/5f775794a74037c19b27cb03125be5f8f2196818/modules/core/src/main/scala/sangria/schema/SchemaComparator.scala#L603)

[asteinwedel]

ah thanks for the spec link!

looks like the official spec doesn't like null values either which is the main difference (counts as >1 still), updated tests and made sure those cases were all covered

(from spec: I didn't do type checking since that's another area of sangria validation)

will work on the breaking change one now

edit: wait saw you did some commits already

[yanns]

yes sorry, it was easier for me to push some changes directly. I've also pushed the changes for the breaking changes.

[asteinwedel]

yes sorry, it was easier for me to push some changes directly. I've also pushed the changes for the breaking changes.

got it thanks! I pushed the null check change + tests now

[yanns]

oh miss, I force pushed some changes as the last commit from me was not complete (was missing the exceptions for the mina checks). Now I'm over. Can you push your changes again? Edit: i see you did, thanks!

[asteinwedel]

Is there anything left you'd like to add / change?

nope think this covers it all! thanks for the help!