System.Text.Json.JsonSerializer doesn't deserialize into existing sub-objects

[johncrim]

If a parent object doesn't contain setters for all sub-object properties, the sub-objects are not deserialized. This is inconsistent with other JSON serializers, and seems like a major oversight - current behavior requires a large amount of mutability in the deserialized object model.

It's also a normal pattern to provide sub-objects with default values before deserialization.

For example, in this class, the SubObject1 property won't be deserialized, and no exception is thrown.

    public class Container1
    {
        // Not deserialized due to missing setter. No exceptions thrown.
        public SubObject1 SubObject1 { get; } = new SubObject1();
    }

Full test project:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <Description>Unit tests demonstrating JsonSerializer bugs</Description>
    <TargetFramework>netcoreapp3.0</TargetFramework>
    <Platform>x64</Platform>
    <Platforms>x64</Platforms>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.4.0" />
    <PackageReference Include="xunit" Version="2.4.1" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.1" PrivateAssets="All"/>
  </ItemGroup>

</Project>

using System.Text.Json;
using Xunit;

namespace JsonSerializer_SubObjects
{
    public class JsonSerializerTest
    {
        [Fact] // Fails
        public void DeserializesSubObjectsWithoutSetters()
        {
            const string jsonFooValue = "str1";
            var json = "{ \"SubObject1\": { \"Foo\": \"" + jsonFooValue + "\" }}";

            var c1 = JsonSerializer.Deserialize<Container1>(json);

            // BUG dotnet/corefx#1: Value isn't deserialized (this assertion fails)
            Assert.Equal(jsonFooValue, c1.SubObject1.Foo);
            // BUG dotnet/corefx#2: If value can't be deserialized, exception should be thrown
        }

        [Fact] // Passes
        public void DeserializesSubObjectsWithSetters()
        {
            const string jsonFooValue = "str1";
            var json = "{ \"SubObject1\": { \"Foo\": \"" + jsonFooValue + "\" }}";

            var c2 = JsonSerializer.Deserialize<Container2>(json);

            Assert.Equal(jsonFooValue, c2.SubObject1.Foo);
        }

    }

    public class Container1
    {
        // Not deserialized due to no setter. No exceptions thrown.
        public SubObject1 SubObject1 { get; } = new SubObject1();
    }

    public class Container2
    {
        public SubObject1 SubObject1 { get; set; } = new SubObject1();
    }

    public class SubObject1
    {
        public string Foo { get; set; } = "foo";
    }
}

[Symbai]

Looks like a duplicate of #29743 ?

[layomia]

Looks like a duplicate of #29743 ?

Closing as duplicate of #29743. This will be considered as part of support for internal/private setters.

[johncrim]

@Symbai and @layomia - I want to point out that this is not the same as internal/private setters. The desired behavior is that the existing sub-object be deserialized into (the sub-object is obtained from the getter), instead of requiring that the deserializer create a new sub-object, deserialize into it, and then pass it into the parent object setter (whether private or internal, non-existent should also work).

Doing it this way allows developers to set default values before deserializing, to handle values that aren't present in the JSON. It also allows developers to create less mutable objects, and use non-nullable reference types for properties that should never be null.

[layomia]

@johncrim I see. The serializer's behavior is to replace object instances. We could add an option such as Json.NET's object creation handling feature: https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_ObjectCreationHandling.htm.

[layomia]

Closing as dup of #30258 which considers similar semantics for collections.

[johncrim]

@layomia - sub-objects are not the same as objects in collections. For consistency, both should be implemented the same way and at the same time. In other words, it would be inconsistent to support using an existing collection (and not replacing it), but not using an existing sub-object (eg it gets replaced).

(IMO) this issue should be re-opened, and both #30258 and #31457 should be handled in the same fix. Separate tests should be added for both use-cases.

[digitalpacman]

I agree with @johncrim

I also ran into this issue as I want it to use existing sub objects. I think the object creation handling of json.net would solve my issue. As long as it allows me to bypass the inability to deserialize interfaces. I have constructors setting the internal objects concrete type and read-only interfaces for subobjects. This technique was necessary for me to handle closed source codegen problems.