Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support nullable structs in JsonSerializer #30843

Closed
layomia opened this issue Sep 12, 2019 · 5 comments · Fixed by #33819
Closed

Support nullable structs in JsonSerializer #30843

layomia opened this issue Sep 12, 2019 · 5 comments · Fixed by #33819

Comments

@layomia
Copy link
Contributor

layomia commented Sep 12, 2019

Given

public class ClassWithNullablePerson
{
    public Person? Person { get; set; } = new Person();
}

public struct Person
{
    public string FirstName { get; set; }
    public int? Age { get; set; }
    public DateTime? Birthday { get; set; }
}

Serialization

string serialized = JsonSerializer.Serialize(new ClassWithNullablePerson());
Console.WriteLine(serialized)
// {"Person":{"HasValue":true,"Value":{"FirstName":null,"Age":null,"Birthday":null}}} (Actual)
// {"Person":{"FirstName":null,"Age":null,"Birthday":null}} (Expected)

Deserialization (Debug)

var person = JsonSerializer.Deserialize<ClassWithNullablePerson>(@"{""Person"":{""FirstName"":""Layo"",""Age"":22,""Birthday"":""1995-04-16""}}");
Process terminated. Assertion failed.
     at System.Text.Json.JsonSerializer.HandlePropertyName(JsonSerializerOptions options, Utf8JsonReader& reader, ReadStack& state) in D:\repos\corefx\src\System.Text.Json\src\System\Text\Json
  \Serialization\JsonSerializer.Read.HandlePropertyName.cs:line 23
     at System.Text.Json.JsonSerializer.ReadCore(JsonSerializerOptions options, Utf8JsonReader& reader, ReadStack& readStack) in D:\repos\corefx\src\System.Text.Json\src\System\Text\Json\Seria
  lization\JsonSerializer.Read.cs:line 54
     at System.Text.Json.JsonSerializer.ReadCore(Type returnType, JsonSerializerOptions options, Utf8JsonReader& reader) in D:\repos\corefx\src\System.Text.Json\src\System\Text\Json\Serializat
  ion\JsonSerializer.Read.Helpers.cs:line 17

In Release, deserialization returns an instance with null members for the nested Person struct.

@ahsonkhan
Copy link
Member

From @andrewjsaid here (https://github.com/dotnet/corefx/issues/41310):

Example below, where we see that my object was serialized unconventionally, then could not be deserialized.

First Problem

public class MyClass
{
    public MyDate? Date { get; set; }
}

public struct MyDate
{
    public int Year { get; set; }
}

// Code
var original = new MyClass { Date = new MyDate { Year = 2 } };

var json = System.Text.Json.JsonSerializer.Serialize(original);
Console.WriteLine($"JSON: {json}");

var after = System.Text.Json.JsonSerializer.Deserialize<MyClass>(json);
Console.WriteLine($"Original: {original.Date == null}");
Console.WriteLine($"After: {after.Date == null}");

Output of the program:

JSON: {"Date":{"HasValue":true,"Value":{"Year":2}}}
Original: False
After: True

Found after migrating similar code to ASP.NET Core 3 when my API was not deserializing my nullable structs.

Second Problem
Furthermore, I believe the following is a related bug:

var json2 = "{\"Date\":{\"Year\":2}}";
var after2 = System.Text.Json.JsonSerializer.Deserialize<MyClass>(json2);
Console.WriteLine($"After2: {after.Date == null}"); // Outputs True

From this example we see that System.Text.Json doesn't even expect the "conventional" way of serializing nullables. Does this mean that it just does not support nullable custom structs at all?

Expected
What I expect and what I call the "conventional" way is to output null for any Nullable which is null, and otherwise just write the JSON of X directly (ignoring HasValue and Value). This is the approach of Newtonsoft JSON.NET, as well as what browsers seem to do.

@esolCrusador
Copy link

Currently we are using this workaround.

public class NullableStructSerializerFactory : JsonConverterFactory
{
    public override bool CanConvert(Type typeToConvert)
    {
        if (!typeToConvert.IsGenericType)
            return false;

        if (!typeToConvert.IsGenericType || typeToConvert.GetGenericTypeDefinition() != typeof(Nullable<>))
            return false;

        var structType = typeToConvert.GenericTypeArguments[0];
        if (structType.IsPrimitive || structType.Namespace != null && structType.Namespace.StartsWith(nameof(System)) || structType.IsEnum)
            return false;

        return true;
    }

    public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
    {
        var structType = typeToConvert.GenericTypeArguments[0];

        return (JsonConverter)Activator.CreateInstance(typeof(NullableStructSerializer<>).MakeGenericType(structType));
    }
}

public class NullableStructSerializer<TStruct> : JsonConverter<TStruct?>
    where TStruct : struct
{
    public override TStruct? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        if (reader.TokenType == JsonTokenType.Null)
            return null;
            
        return JsonSerializer.Deserialize<TStruct>(ref reader, options);
    }

    public override void Write(Utf8JsonWriter writer, TStruct? value, JsonSerializerOptions options)
    {
        if (value == null)
            writer.WriteNullValue();
        else
            JsonSerializer.Serialize(writer, value.Value, options);
    }
}

@layomia

This comment has been minimized.

@layomia layomia self-assigned this Dec 4, 2019
@layomia layomia removed their assignment Dec 19, 2019
@ylibrach
Copy link

ylibrach commented Jan 7, 2020

I ran into another problem which may be related to this issue, specifically when dealing with a Nullable<DateTime> (or presumably any string-serialized nullable type). Trying to deserialize an empty string into this type causes a JsonException to get thrown:

JsonSerializer.Deserialize<Nullable<DateTime>>("\"\"")
// System.Text.Json.JsonException: 'The JSON value could not be converted to 
// System.Nullable`1[System.DateTime]. Path: $ | LineNumber: 0 | BytePositionInLine: 2.

The same statement using Newtonsoft.Json deserializes into a null value.

If this is already covered by the scope of this issue, then no worries. If you prefer that I open a separate issue for this, let me know.

@ahsonkhan
Copy link
Member

The issue you brought up is about trying to coerce an empty string into a null value (in this case the type just happens to be Nullable<DateTime>. This issue is unrelated and is covered by #434 (comment)

I don't think we should allow an empty string to mean null for DateTime by default, because the empty string is not null. Also, other users might depend on the empty string being invalid for these types.

The workaround would be to special-case/handle that in your own converter.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants