System.Text.Json contract customization: not possible to enable deserialization in properties marked JsonIgnoreCondition.Always

[eiriktsarpalis]

Consider the following test:

[Fact]
public static void CanEnableDeserializationOnAlwaysIgnoreProperty()
{
    var options = new JsonSerializerOptions
    {
        TypeInfoResolver = new DefaultJsonTypeInfoResolver
        {
            Modifiers =
            {
                jsonTypeInfo =>
                {
                    if (jsonTypeInfo.Type != typeof(ClassWithJsonIgnoreAlwaysProperty))
                    {
                        return;
                    }

                    JsonPropertyInfo jsonPropertyInfo = jsonTypeInfo.Properties[0];
                    jsonPropertyInfo.Set = (obj, value) => ((ClassWithJsonIgnoreAlwaysProperty)obj).Value = (int)value;
                }
            }
        }
    };

    ClassWithJsonIgnoreAlwaysProperty value = JsonSerializer.Deserialize<ClassWithJsonIgnoreAlwaysProperty>("""{"Value":42}""", options);
    Assert.Equal(42, value.Value);
}

public class ClassWithJsonIgnoreAlwaysProperty
{
    [JsonIgnore]
    public int Value { get; set; }
}

This happens because the DetermineSerializationCapabilities method will mark the property as non-serializable/deserializable based on the initial value of JsonIgnoreCondition. JsonIgnoreCondition is not user-settable so no user modification on the contract model can prevent this.

I would recommend we change this so that all logic in DetermineSerializationCapabilities is done at initialization time and can be modified by users. Additionally, I propose we introduce a ShouldDeserialize method to JsonPropertyInfo:

public partial class JsonPropertyInfo
{
    public Func<object, object?, bool> ShouldSerialize { get; set; }
+    public Func<object, object?, bool> ShouldDeserialize { get; set; }
}

The two delegates would allow mapping all possible JsonIgnoreCondition configurations to these two delegates and not require nulling out the Get and Set delegates as we currently do for JsonIgnoreCondition.Always. Contract customization in Json.NET also includes a ShouldDeserialize delegate complementing ShouldSerialize so we should be following a similar approach in System.Text.Json.

cc @krwq @jeffhandley

[ghost]

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

---

Consider the following test:

[Fact]
public static void CanEnableDeserializationOnAlwaysIgnoreProperty()
{
    var options = new JsonSerializerOptions
    {
        TypeInfoResolver = new DefaultJsonTypeInfoResolver
        {
            Modifiers =
            {
                jsonTypeInfo =>
                {
                    if (jsonTypeInfo.Type != typeof(ClassWithJsonIgnoreAlwaysProperty))
                    {
                        return;
                    }

                    JsonPropertyInfo jsonPropertyInfo = jsonTypeInfo.Properties[0];
                    jsonPropertyInfo.Set = (obj, value) => ((ClassWithJsonIgnoreAlwaysProperty)obj).Value = (int)value;
                }
            }
        }
    };

    ClassWithJsonIgnoreAlwaysProperty value = JsonSerializer.Deserialize<ClassWithJsonIgnoreAlwaysProperty>("""{"Value":42}""", options);
    Assert.Equal(42, value.Value);
}

public class ClassWithJsonIgnoreAlwaysProperty
{
    [JsonIgnore]
    public int Value { get; set; }
}

This happens because the DetermineSerializationCapabilities method will mark the property as non-serializable/deserializable based on the initial value of JsonIgnoreCondition. JsonIgnoreCondition is not user-settable so no user modification on the contract model can prevent this.

I would recommend we change this so that all logic in DetermineSerializationCapabilities is done at initialization time and can be modified by users. Additionally, I propose we introduce a ShouldDeserialize method to JsonPropertyInfo:

public partial class JsonPropertyInfo
{
    public Func<object, object?, bool> ShouldSerialize { get; set; }
+    public Func<object, object?, bool> ShouldDeserialize { get; set; }
}

The two delegates would allow mapping all possible JsonIgnoreCondition configurations to these two delegates and not require nulling out the Get and Set delegates as we currently do for JsonIgnoreCondition.Always. Contract customization in Json.NET also includes a ShouldDeserialize delegate complementing ShouldSerialize so we should be following a similar approach in System.Text.Json.

cc @krwq @jeffhandley

Author:	eiriktsarpalis
Assignees:	eiriktsarpalis
Labels:	bug, api-suggestion, area-System.Text.Json
Milestone:	7.0.0

[eiriktsarpalis]

The following internal properties are also of interest:

• IgnoreDefaultValuesOnWrite
• IgnoreDefaultValuesOnRead
• CanSerialize
• CanDeserialize

These all map from JsonIgnoreCondition but are not user modifiable. Their behavior should be mapped to ShouldSerialize/ShouldDeserialize delegates.

[krwq]

I think when Set is set we should simply replace value of ShouldSerialize to delegate (I think it's already set) and clear _ignoreCondition and that should already be enough for that to work correctly.

[eiriktsarpalis]

I think when Set is set we should simply replace value of ShouldSerialize to delegate

Why should making changes to Set have any bearing on the value of ShouldSerialize? One concerns serialization configuration and the other concerns deserialization. While they are related by virtue of the fact that JsonIgnoreCondition feeds into their initial population, from a user's perspective they should be independent.

[krwq]

it won't. if you request ShouldSerialize value it will be the same regardless of touching Set - the only thing which will change is perf since we can't use internal enum which allows us to not having to touch delegate