ghostdogpr / caliban

Functional GraphQL library for Scala
https://ghostdogpr.github.io/caliban/
Apache License 2.0
947 stars 248 forks source link

Incorrect behaviour of `ArgBuilder` containing only nullable fields when input is null #2407

Closed kyri-petrou closed 2 months ago

kyri-petrou commented 2 months ago

Given the following case class:

case class Foo(value1: Option[String], value2: Option[String]) derives ArgBuilder

I expect that the derived ArgBuilder#build should work in the following ways:

  1. Input: {}, output: Right(Foo(None, None))
  2. Input: null, output Left(ExecutionError("bla"))

However, currently both cases succeed with Foo(None, None). Can be reproduced with the following test that should pass but fails since the first condition is not Left

    suite("derived build")(
      test("should fail when null is provided for case class with optional fields") {
        case class Foo(value: Option[String])
        val ab = ArgBuilder.gen[Foo]
        assertTrue(
          ab.build(NullValue).isLeft,
          ab.build(ObjectValue(Map())).isRight
        )
      }
    )

PS: This is not a problem when validation is enabled as an error will be raised during the validation phase

kyri-petrou commented 2 months ago

@ghostdogpr are you aware of anything that relies on the ArgBuilder succeeding when null is provided instead of {}?

ghostdogpr commented 2 months ago

Hmm yeah that looks like a bug.