modelina icon indicating copy to clipboard operation
modelina copied to clipboard

Exception occurs when using System.Text.Json deserializer to deserialize enum.

Open RowlandBanks opened this issue 1 year ago • 5 comments

The existing runtime-csharp unit-tests do not currently test deserialization. When adding such a test, a bug is exposed.

Steps to reproduce

Add the following unit-test in the runtime-csharp test solution under \test\models\json_serializer.

    [Test]
    public void TestRoundTrip()
    {
        const string expectedJsonString = "{\"house_number\":1,\"marriage\":true,\"members\":2,\"array_type\":[1,\"test\"],\"nestedObject\":{\"test\":\"test\"},\"enumTest\":\"test\",\"houseType\":\"flat\",\"roofType\":\"straw\",\"test_not_used\":2}";
        Address address = JsonSerializer.Deserialize<Address>(expectedJsonString) ?? throw new Exception("Failed to deserialize address.");
        string actualJsonString = JsonSerializer.Serialize(address);
        Assert.That(actualJsonString, Is.EqualTo(expectedJsonString));
    }

Run the test.

Expected outcome

The JSON is correctly deserialized to the relevant model and the test passes.

Actual outcome

An exception occurs:

Microsoft.CSharp.RuntimeBinder.RuntimeBinderException : The best overloaded method match for 'com.mycompany.app.json_serializer.EnumTestExtensions.ToEnumTest(string)' has some invalid arguments

Technical detail

This is caused by the following generated code in the generated JsonConverter<T>:

if (propertyName == "houseType")
{
    var value = HousingTypeExtensions.ToHousingType(JsonSerializer.Deserialize<dynamic>(ref reader, options));
    instance.HouseType = value;
    continue;
}

Note the call to Deserialize<dynamic> - this should be Deserialize<string>.

RowlandBanks avatar Feb 26 '24 14:02 RowlandBanks

I've fixed the enum bug, but the unit-test has identified a second bug. The generator generates the following code:

if (instance.AdditionalProperties == null)
{
    instance.AdditionalProperties = new Dictionary<string, dynamic>();
}

var deserializedValue = JsonSerializer.Deserialize<Dictionary<string, dynamic>?>(ref reader, options);
instance.AdditionalProperties.Add(propertyName, deserializedValue);
continue;

Which fails on the call to Deserialize with the exception:

System.Text.Json.JsonException : The JSON value could not be converted to System.Collections.Generic.Dictionary`2[System.String,System.Object]. Path: $ | LineNumber: 0 | BytePositionInLine: 1.

However, the generated code looks very wrong - I don't think it should be deserializing to a dictionary, but to a simple dynamic value that gets added to the AdditionalProperties dictionary.

RowlandBanks avatar Feb 26 '24 15:02 RowlandBanks

A third bug is that oneOf types are deserialized to a JsonElement, not to the actual type when being deserialized.

Therefore, when attempting to serialize a deserialized model, one gets the following error:

Microsoft.CSharp.RuntimeBinder.RuntimeBinderException : Operator '!=' cannot be applied to operands of type 'System.Text.Json.JsonElement' and '<null>'

For example,

"members": {
  "oneOf": [
    {
      "type": "string"
    },
    {
      "type": "number"
    },
    {
      "type": "boolean"
    }
  ]
},

will cause the error on this serailization code:

if (value.Members != null)
{
    // write property name and let the serializer serialize the value itself
    writer.WritePropertyName("members");
    JsonSerializer.Serialize(writer, value.Members, options);
}

RowlandBanks avatar Feb 27 '24 09:02 RowlandBanks

I've fixed the bugs I've mentioned above, but I have a concern that deserializing to JsonElement will be confusing for consumers.

Perhaps this can be looked into in a future issue? However, without adding type-annotations to the output JSON, it's likely that there is no sensible way around this, so 🤷 .

RowlandBanks avatar Feb 27 '24 10:02 RowlandBanks

/rtm

RowlandBanks avatar Feb 27 '24 10:02 RowlandBanks