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

System.Text.Json does not fill JsonExtensionData when a property with the same name already exists or has the JsonIgnoreAttribute #68895

Open
softlion opened this issue May 5, 2022 · 5 comments
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@softlion
Copy link

softlion commented May 5, 2022

Description

The JsonExtensionData property does not contain the properties from the deserialized Json that have matching read only or JsonIgnore properties on the target object.

Reproduction Steps

When deserializing that json:

var json = @"{ ""value"": { ""content"": ""test"" } }";

into that object

    public class TestDefault
    {
        [JsonIgnore]
        public JsonElement Value => Values["value"];
        
        [JsonExtensionData]
        public Dictionary<string, JsonElement> Values { get; set; }
    }

with this code:

    [TestClass]
    public class TestNotionHtml
    {
        [TestMethod]
        public void TestJsonExtensionData_Inherited()
        {
            var json = @"{ ""value"": { ""content"": ""test"" } }";
            var r = JsonSerializer.Deserialize<TestDefault>(json, new(JsonSerializerDefaults.Web));
            Assert.IsNotNull(r);
            Assert.IsNotNull(r.Values);
            Assert.AreEqual(1, r.Values.Count);
            Assert.IsTrue(r.Values.ContainsKey("value"));
            Assert.IsNotNull(r.Value);
        }
  }

the Values dictionary is empty.

Expected behavior

Values should contains all properties that are not deserialized

Actual behavior

Values is empty

Regression?

No

Known Workarounds

None

Configuration

.net6

system.text.json 6.0.3

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 5, 2022
@ghost
Copy link

ghost commented May 5, 2022

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

The JsonExtensionData property does not contain the properties from the deserialized Json that have matching read only or JsonIgnore properties on the target object.

Reproduction Steps

When deserializing that json:

var json = @"{ ""value"": { ""content"": ""test"" } }";

into that object

    public class TestDefault
    {
        [JsonIgnore]
        public JsonElement Value => Values["value"];
        
        [JsonExtensionData]
        public Dictionary<string, JsonElement> Values { get; set; }
    }

with this code:

    [TestClass]
    public class TestNotionHtml
    {
        [TestMethod]
        public void TestJsonExtensionData_Inherited()
        {
            var json = @"{ ""value"": { ""content"": ""test"" } }";
            var r = JsonSerializer.Deserialize<TestDefault>(json, new(JsonSerializerDefaults.Web));
            Assert.IsNotNull(r);
            Assert.IsNotNull(r.Values);
            Assert.AreEqual(1, r.Values.Count);
            Assert.IsTrue(r.Values.ContainsKey("value"));
            Assert.IsNotNull(r.Value);
        }
  }

the Values dictionary is empty.

Expected behavior

Values should contains all properties that are not deserialized

Actual behavior

Values is empty

Regression?

No

Known Workarounds

None

Configuration

.net6

system.text.json 6.0.3

Other information

No response

Author: softlion
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@eiriktsarpalis
Copy link
Member

I can reproduce, here's a minimal reproduction:

var json = @"{ ""Value"": true }";
var r = JsonSerializer.Deserialize<TestDefault>(json);
Console.WriteLine(r.ExtensionData?.Count ?? 0); // prints 0

public class TestDefault
{
    public bool Value { get; }

    [JsonExtensionData]
    public Dictionary<string, object> ExtensionData { get; set; }
}

I wouldn't necessarily consider this to be a bug, although I can see why the expected behavior would be nice to have. We might want to consider it in a future release. cc @layomia @steveharter

@eiriktsarpalis eiriktsarpalis added the enhancement Product code improvement that does NOT require public API changes/additions label May 9, 2022
@eiriktsarpalis eiriktsarpalis added this to the Future milestone May 9, 2022
@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label May 9, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label May 9, 2022
@svick
Copy link
Contributor

svick commented Nov 27, 2022

This shouldn't have been closed, right?

@Mizuwokiru
Copy link

Mizuwokiru commented Feb 10, 2023

If someone still needs to use this approach, the issue can be avoided by adding JsonPropertyName attribute that doesn't match to any property of your object.
For example:

public class TestDefault
{
    [JsonPropertyName($"${nameof(Value)}"), JsonIgnore]
    public JsonElement Value => Values["value"];

    [JsonExtensionData]
    public Dictionary<string, JsonElement> Values { get; set; }
}

@krwq
Copy link
Member

krwq commented Feb 10, 2023

Better workaround would be:

using System.Text.Json;
using System.Text.Json.Serialization;
using System.Text.Json.Serialization.Metadata;

JsonSerializerOptions options = new()
{
    TypeInfoResolver = new DefaultJsonTypeInfoResolver()
    {
        Modifiers =
        {
            ti =>
            {
                if (ti.Kind == JsonTypeInfoKind.Object)
                {
                    JsonPropertyInfo[] props = ti.Properties
                        .Where(prop => prop.AttributeProvider == null || prop.AttributeProvider.GetCustomAttributes(typeof(JsonIgnoreAttribute), false).Length == 0)
                        .ToArray();

                    if (props.Length != ti.Properties.Count)
                    {
                        ti.Properties.Clear();
                        foreach (var prop in props)
                        {
                            ti.Properties.Add(prop);
                        }
                    }
                }
            }
        }
    }
};

var json = @"{ ""Value"": { ""content"": ""test"" } }";
TestDefault obj = JsonSerializer.Deserialize<TestDefault>(json, options);
if (obj.Values != null)
{
    foreach (var (k, v) in obj.Values)
    {
        Console.WriteLine($"{k} => {v}"); // Value => { "content": "test" }
    }
}

public class TestDefault
{
    [JsonIgnore]
    public JsonElement Value => Values["value"];

    [JsonExtensionData]
    public Dictionary<string, JsonElement>? Values { get; set; }
}

Note 1: ignored properties will also not work with parametrized constructors in that case (assuming that was desired in the first place).

Note 2: It needs testing with inheritance if needed for production

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

5 participants