ChilliCream / graphql-platform

Welcome to the home of the Hot Chocolate GraphQL server for .NET, the Strawberry Shake GraphQL client for .NET and Banana Cake Pop the awesome Monaco based GraphQL IDE.
https://chillicream.com
MIT License
5.28k stars 748 forks source link

Null values in arrays are not deserialized to "null" #6040

Closed pixlcrashr closed 1 year ago

pixlcrashr commented 1 year ago

Is there an existing issue for this?

Product

Strawberry Shake

Describe the bug

Consider the following query:

query User {
    id: Int!
}

query Query {
    users(ids: [String!]!): [User]!
}

---

query ListUsers($ids: [String!]!) {
    users(ids: $ids) {
        __typename
        id
    }
}

Now, if users returns the following (valid!) result

{
    "data": {
        "users": [
            { ... },
            null,
            ...
        ]
    }
}

Strawberry Shake throws an error that it can not find the __typename on the null object - which is logically true but does not make any sense. The following .NET exception is thrown:

System.InvalidOperationException: 'The requested operation requires an element of type 'Object', but the target element has type 'Null'.'

The following is the generated deserialization method for above query:

private global::Test.GraphQl.State.UserData? Deserialize_IListUsers_Users(global::System.Text.Json.JsonElement? obj)
{
    if (!obj.HasValue)
    {
        return null;
    }

    var typename = obj.Value.GetProperty("__typename").GetString();
    if (typename?.Equals("User", global::System.StringComparison.Ordinal) ?? false)
    {
        return new global::Test.GraphQl.State.UserData(typename, id: Deserialize_NonNullableString(global::StrawberryShake.Json.JsonElementExtensions.GetPropertyOrNull(obj, "id")));
    }

    throw new global::System.NotSupportedException();
}

The line var typename = obj.Value.GetProperty("__typename").GetString(); is the one that fails, because the input obj is of JsonElement value kind JsonValueKind.Null and is not generally a nullable type (JsonElement?).

Generally, this should be easily solvable by modifying the template that generates above function to:

private global::Test.GraphQl.State.UserData? Deserialize_IListUsers_Users(global::System.Text.Json.JsonElement? obj)
{
    if (!obj.HasValue)
    {
        return null;
    }

        if (obj.Value.ValueKind == JsonValueKind.Null) {
                return null;
        }

    var typename = obj.Value.GetProperty("__typename").GetString();
    if (typename?.Equals("User", global::System.StringComparison.Ordinal) ?? false)
    {
        return new global::Test.GraphQl.State.UserData(typename, id: Deserialize_NonNullableString(global::StrawberryShake.Json.JsonElementExtensions.GetPropertyOrNull(obj, "id")));
    }

    throw new global::System.NotSupportedException();
}

Of course, above example does not handle non-nullable types.

I would appreciate it if you quickly do a hotfix on that.

Steps to reproduce

See above.

Relevant log output

System.InvalidOperationException: 'The requested operation requires an element of type 'Object', but the target element has type 'Null'.'

Additional Context?

No response

Version

13.0.5

Tasks ❤️

pixlcrashr commented 1 year ago

@michaelstaib any news on this?

nightroman commented 1 year ago

@michaelstaib Our clients report this issue in the latest official SS 13.7.0 and tell that it seems to be fixed in v14 previews. If v14 is not expected "soon", is there any chance to backport the fix (if this is the case) to some 13.x?

michaelstaib commented 1 year ago

Most of the bug fixes will be back ported. There is already a preview for 13

nightroman commented 1 year ago

Thank you @michaelstaib , we look forward to the update of v13.

s0bh commented 9 months ago

Hello, Just wanted to check if this fix for deserialization issue when we should expect it to be released because I do have the same situation and upgraded to 13.8.1 and still not fixed on it, so any timeline for it?