[JsonSerializer] Relax restrictions on ctor param type to immutable property type matching where reasonable

[JSkimming]

Description

I'm trying to deserialise an object from a json string. In this case the constructor allows a nullable value type as the parameter, setting a non-null property (defaulting if null).

I expect (since it is the behaviour with Newtonsoft.Json) the serializer to handle compatible constructor and bound value type properties (or fields) where the only difference is one is nullable.

The following demonstrates the issue:

public class Example
{
    public Example(Guid? aGuid) => AGuid = aGuid ?? Guid.Empty;
    public Guid AGuid { get; }
}

Example original = new Example(Guid.NewGuid());

// Works with Newtonsoft.Json.
string withJsonNet = JsonConvert.SerializeObject(original);
JsonConvert.DeserializeObject<Example>(withJsonNet);

// Fails with System.Text.Json.
string withSystemTextJson = System.Text.Json.JsonSerializer.Serialize(original);
System.Text.Json.JsonSerializer.Deserialize<Example>(withSystemTextJson);

An InvalidOperationException is thrown from the method System.Text.Json.JsonSerializer.Deserialize :

System.InvalidOperationException: Each parameter in constructor 'Void .ctor(System.Nullable`1[System.Guid])' on type 'Example' must bind to an object property or field on deserialization. Each parameter name must match with a property or field on the object. The match can be case-insensitive.

Configuration

I'm building an ASP.NET Core app targeting netcoreapp3.1, and using version 5.0.0-rc.2.20475.5 of System.Text.Json. I'm also using version 12.0.3 of Newtonsoft.Json.

Other information

Stack Trace:

System.InvalidOperationException: Each parameter in constructor 'Void .ctor(System.Nullable`1[System.Guid])' on type 'Example' must bind to an object property or field on deserialization. Each parameter name must match with a property or field on the object. The match can be case-insensitive.
   at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_ConstructorParameterIncompleteBinding(ConstructorInfo constructorInfo, Type parentType)
   at System.Text.Json.Serialization.Converters.ObjectWithParameterizedConstructorConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadCore[TValue](JsonConverter jsonConverter, Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadCore[TValue](Utf8JsonReader& reader, Type returnType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, Type returnType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)
   at <custom code>

[layomia]

Why make the constructor parameter nullable? The AGuid property is non-nullable which means that null will never be serialized. Are you expecting null JSON tokens to bind with the parameter on deserialization? Where would this JSON come from?

[JSkimming]

Thanks for the reply @layomia

To present a minimal use-case the example is intentionally trivial.

My situation is rather more complex. We have several scenarios where having the parameter as nullable was desirable (and worked well using Newtonsoft.Json), some of which we can, and have, resolved by making the parameter non-nullable.

But there are other scenarios where a nullable parameter with a non-nullable property is still preferred. One being where we have an unknown number of legacy objects that are serialised to a store with null values. The use-cases have since been updated where new instances are not serialised with null, but we still want to support the de-serialization of the legacy objects.

If we had been using System.Text.Json at the time, we probably would have implemented the solution differently, but as I highlighted above Newtonsoft.Json worked.

Hope the context helps.

Ultimately though, this is a difference in behaviour with Newtonsoft.Json. It looks like you are trying to resolve edge cases in to make System.Text.Json "the standard JSON stack for .NET." (See Issue #43620). And this is one such edge case.

[layomia]

From @https://github.com/NN--- in #46480:

Description

    public class Q
    {
        [JsonConstructor]
        public Q(int? x)
        {
            if (x is null) throw new Exception();
            X = x.Value;
        }
        
        public int X { get; }
    }

Given string:

var str = JsonSerializer.Deserialize<Q>("{\"X\":123}" ");

It is deserialized fine with Newtonsoft, but not with System.Text.
The reason is that constructor parameter type is int? while the property has type of int.
Newtonsoft doesn't validate it, giving an option to do anything in the constructor.

There are possible workarounds but they are not as simple as the original code:

Make property private and add a public non nullable:

    public class Q
    {
        [JsonConstructor]
        public Q(int? x)
        {
            if (x is null) throw new Exception();
            X = x.Value;
        }

        public int? X { get; }

        [JsonIgnore]
        public int NonNullableX => X!.Value;
    }

Annotate nullable as non nullable, however it requires explicit call to Value.:

    public class Q
    {
        [JsonConstructor]
        public Q(int? x)
        {
            if (x is null) throw new Exception();
            X = x.Value;
        }

        [DisallowNull][NotNull]
        public int? X { get; }
    }

Configuration

Regression?

Not a regression.

Other information

[layomia]

We could look into relaxing the matching algorithm here if it proves to be unwieldy. The workaround here is to refactor the POCO code slightly to make the property type and the constructor parameter type the same. This issue will not be treated as high priority until a blocked and non-trivial scenario is provided.

[GabeDeBacker]

Deserializing JSON into an object that can be bound to WPF\XAML is likely very common place and converting an incoming IEnumerable into a observable collection that XAML can use is also likely common.

Not supporting this limits System.Text.Json's use with an XAML\WPF\UWP apps.

[JSkimming]

This issue will not be treated as high priority until a blocked and non-trivial scenario is provided.

@layomia As I explained, the example is trivial to present a minimal use-case. I also went on to explain the non-trivial scenario:

we have an unknown number of legacy objects that are serialised to a store with null values. The use-cases have since been updated where new instances are not serialised with null, but we still want to support the de-serialization of the legacy objects.

We are also blocked. Our workaround is to stick with Newtonsoft.Json.

[layomia]

Thanks for the responses and expanding on the importance here. There are definitely various scenarios where loosening the matching restrictions is helpful. We can address this in the .NET 6.0 release.