graphql-platform icon indicating copy to clipboard operation
graphql-platform copied to clipboard

Inconsistent behavior within InputParser, when parse nullable scalar value in HC 13 in compare with 12

Open dkogithub opened this issue 2 years ago • 7 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Product

Hot Chocolate

Describe the bug

If there is a input type which has 2 fields like

{
documentId: String
revision: Int
}

If the query with this input type argument run like:

query myquery {
                       tests{
                         test(documentVersion: {documentId: "myid", revision: null}) {
                           revision
                         }
                       }
                     }

InputParser parse "revision" as null - which is expected, but in case the query run without "revision" parameter InputParser parse it as default Int32 value - 0.

HC 12 code retuned null in both cases.

It happens here, which I think is incorrect behavior: https://github.com/ChilliCream/graphql-platform/blob/96b9a9c4198d7f13a15b10f617f536c1ca15d3c6/src/HotChocolate/Core/src/Types/Types/InputParser.cs#L662

the code comment says: // if the type is nullable but the runtime type is a non-nullable value // we will create a default instance and assign that instead. which makes no sense to me, if the field nullable why it's need to forcedly set to default not nullable value?

Steps to reproduce

  1. Build/Run attached repro.zip project
  2. Open "serverPath/graphql/ui" Banana Cake url
  3. Run query:
query  myquery{
    testData (documentVersion: {documentId: "mydocId", revision: null}){
      id
    }
}
  1. Note that revision field is null within resolver method, which is expected. 2023-07-18 10_40_56-WebApplication1 – TestDataResolver cs  Debugger
  2. Run another query, without revision parameter specifying:
query  myquery{
    testData (documentVersion: {documentId: "mydocId"}){
      id
    }
} 
  1. Note that this time revision fields is equals 0, which is not expected for nullable field. 2023-07-18 10_40_34-WebApplication1 – TestDataResolver cs  Debugger

Relevant log output

No response

Additional Context?

No response

Version

13.3.3

dkogithub avatar Jul 18 '23 08:07 dkogithub

13 is correct as the GraphQL spec in the case that you do not provide a value demands that the default value shall be provided.

https://spec.graphql.org/draft/#CoerceArgumentValues()

image

This was probably a bug in version 12 that we fixed with 13.

michaelstaib avatar Jul 18 '23 15:07 michaelstaib

I might have misread here. So in your case you do not have a default value?

michaelstaib avatar Jul 18 '23 15:07 michaelstaib

I just see that you are not using typed objects ... that is causing this issue. I will write a test for this. I think the runtime type in this case is incorrectly inferred.

In any case I would not use the arguments in this way as it will result in a much larger memory footprint. Just an advice. In any case, I will have a look at it.

michaelstaib avatar Jul 18 '23 15:07 michaelstaib

Also being affected by this issue in HC 13.9.0. In case anyone else runs into this, like the OP mentioned, the workaround for now is to explicitly set any scalar field to null if it is a value type (i.e. int, bool, enum, etc).

jeffrimko avatar Apr 04 '24 21:04 jeffrimko