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 ignore only when null API enhancement #30687

Closed
CodeBlanch opened this issue Aug 27, 2019 · 15 comments · Fixed by #34049
Closed

System.Text.Json ignore only when null API enhancement #30687

CodeBlanch opened this issue Aug 27, 2019 · 15 comments · Fixed by #34049
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json
Milestone

Comments

@CodeBlanch
Copy link
Contributor

CodeBlanch commented Aug 27, 2019

Rationale

Right now null handling is an all or nothing thing. We have an option to ignore all null values (JsonSerializerOptions.IgnoreNullValues) and we have an option to ignore properties completely (JsonIgnoreAttribute), but you can't specify per-property handling. Per-property handling is useful when you want to hide something from JSON unless it is populated, perhaps because it was not requested or is only applicable to certain audiences.

This gives us parity with something like Json.NET's NullValueHandling.

Proposed API

Via expansion of JsonIgnoreAttribute...

namespace System.Text.Json.Serialization
{
  /// <summary>
  /// Controls how the <see cref="JsonIgnoreAttribute"/> will decide to ignore properties.
  /// </summary>
  public enum JsonIgnoreCondition
  {
      /// <summary>
      /// Property will always be ignored.
      /// </summary>
      Always,
      /// <summary>
      /// Property will only be ignored if it is null.
      /// </summary>
      WhenNull,
      /// <summary>
      /// Property will always be serialized/deserialized regardless of <see cref="JsonSerializerOptions"/> configuration.
      /// </summary>
      Never
  }

  /// <summary>
  /// Prevents a property from being serialized or deserialized.
  /// </summary>
  [AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
  public sealed class JsonIgnoreAttribute : JsonAttribute
  {
      /// <summary>
      /// Specifies the condition that must be met before a property will be ignored. Default value: <see cref="JsonIgnoreCondition.Always"/>.
      /// </summary>
      public JsonIgnoreCondition Condition { get; set; } = JsonIgnoreCondition.Always;

      /// <summary>
      /// Initializes a new instance of <see cref="JsonIgnoreAttribute"/>.
      /// </summary>
      public JsonIgnoreAttribute() { }
  }
}

Usage

Always

This is equivalent to the current [JsonIgnore] semantics where properties that have this value are always ignored.

WhenNull

public class ClassUsingIgnoreWhenNullAttribute
{
    [JsonIgnore(Condition = JsonIgnoreCondition.WhenNull)]
    public SimpleTestClass Class { get; set; }

    [JsonIgnore(Condition = JsonIgnoreCondition.WhenNull)]
    public Dictionary<string, string> Dictionary { get; set; } = new Dictionary<string, string> { ["Key"] = "Value" };
}

[Fact]
public static void SerializerSupportsClassUsingIgnoreWhenNullAttribute()
{
    string json = @"{""Class"":{""MyInt16"":18}, ""Dictionary"":null}";

    ClassUsingIgnoreWhenNullAttribute obj = JsonSerializer.Deserialize<ClassUsingIgnoreWhenNullAttribute>(json);

    // Class is deserialized because it is not null in json...
    Assert.NotNull(obj.Class);
    Assert.Equal(18, obj.Class.MyInt16);

    // Dictionary is left alone because it is null in json...
    Assert.NotNull(obj.Dictionary);
    Assert.Equal(1, obj.Dictionary.Count);
    Assert.Equal("Value", obj.Dictionary["Key"]);

    obj = new ClassUsingIgnoreWhenNullAttribute();

    json = JsonSerializer.Serialize(obj);

    // Class is not included in json because it was null, Dictionary is included because it is not null...
    Assert.Equal(@"{""Dictionary"":{""Key"":""Value""}}", json);
}

Never

public class ClassUsingIgnoreWhenNullAttribute
{
    public SimpleTestClass Class { get; set; } new MySimpleTestClass { MyInt16 = 18 };

    [JsonIgnore(Condition = JsonIgnoreCondition.Never)]
    public Dictionary<string, string> Dictionary { get; set; } = new Dictionary<string, string> { ["Key"] = "Value" };
}

[Fact]
public static void SerializerSupportsClassUsingIgnoreNeverAttribute()
{
    string json = @"{""Class"":null, ""Dictionary"":null}";
    var options = new JsonSerializerOptions { IgnoreNullValues = true };

    ClassUsingIgnoreWhenNullAttribute obj = JsonSerializer.Deserialize<ClassUsingIgnoreWhenNullAttribute>(json, options);

    // Class is not deserialized because it is null in json...
    Assert.NotNull(obj.Class);
    Assert.Equal(18, obj.Class.MyInt16);

    // Dictionary is deserialized regardless of being null in json...
    Assert.Null(obj.Dictionary);

    obj = new ClassUsingIgnoreWhenNullAttribute();
    obj.Class = null;

    json = JsonSerializer.Serialize(obj, options);

    // Class is not included in json because it was null, Dictionary is included regardless of being null...
    Assert.Equal(@"{""Dictionary"":null}", json);
}

Considerations

Implementation

See dotnet/corefx#40522

@SidShetye
Copy link

Requesting to prioritize as we're blocked on this given the all-or-nothing choice. We have many vendor APIs (including our own) that are fine but have one vendor API (an Azure one interestingly) that requires us to not send out a property but only when it's null. This is also enforced in the certification process, so it's hard to avoid.

@steveharter
Copy link
Member

Moving to 5.0 for consideration as this is blocking adoption.

Consider if the global option JsonSerializerOptions.IgnoreNullValues==true but we don't want to ignore certain properties: then adding a "Never" value to the enum make sense.

Also, since the current options apply to both serialization and deserialization, are there valid cases for controlling serialization and deserialization separately with the proposed enum?

@ericstj
Copy link
Member

ericstj commented Nov 12, 2019

/cc @layomia @ahsonkhan

@SidShetye
Copy link

Also, since the current options apply to both serialization and deserialization, are there valid cases for controlling serialization and deserialization separately with the proposed enum?

For us, we do not need them to be controlled separately.

@pharring
Copy link
Contributor

Value types were mentioned in "Considerations". I would vote to include value types and Enums. Json.Net's equivalent is DefaultValueHandling.Ignore. In my applications I use that on Guid, TimeSpan, DateTime, Boolean and some Enums.

@pharring
Copy link
Contributor

Also, for completeness, there's precedent in System.Runtime.Serialization.DataMemberAttribute's EmitDefaultValue param which works on value types too.

@lfr
Copy link

lfr commented Nov 13, 2019

If implemented as suggested here this enhancement is incomplete. It needs a third option:

    /// <summary>
    /// Controls how the <see cref="JsonIgnoreAttribute"/> will decide to ignore properties.
    /// </summary>
    public enum JsonIgnoreCondition
    {
        /// <summary>
        /// Property will always be ignored.
        /// </summary>
        Always,
        /// <summary>
        /// Property will only be ignored if it is null.
        /// </summary>
        WhenNull,
        /// <summary>
        /// Property will always be serialized/deserialized regardless of Options settings
        /// </summary>
        Never
    }

Please look at https://github.com/dotnet/corefx/issues/42034 for more context, Json.Net does it correctly via NullValueHandling, this suggestion covers the case when we want to for a specific property override the default general options that dictate that null values should be included, but we also need to be able to do the opposite, meaning overriding general options that dictate that null values should be ignored for a specific property.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@layomia
Copy link
Contributor

layomia commented Feb 5, 2020

Also, since the current options apply to both serialization and deserialization, are there valid cases for controlling serialization and deserialization separately with the proposed enum?

If there are such cases, instead of using one enum for both, maybe we can have two properties on JsonIgnoreAttribute:

/// <summary>
/// Prevents a property from being serialized or deserialized.
/// </summary>
[AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
public sealed class JsonIgnoreAttribute : JsonAttribute
{
    /// <summary>
    /// Specifies the condition that must be met before a property will be ignored on deserialization. Default value: <see cref="JsonIgnoreCondition.Always"/>.
    /// </summary>
    public JsonIgnoreCondition ReadCondition { get; set; } = JsonIgnoreCondition.Always;

    /// <summary>
    /// Specifies the condition that must be met before a property will be ignored on serialization. Default value: <see cref="JsonIgnoreCondition.Always"/>.
    /// </summary>
    public JsonIgnoreCondition WriteCondition { get; set; } = JsonIgnoreCondition.Always;

    /// <summary>
    /// Initializes a new instance of <see cref="JsonIgnoreAttribute"/>.
    /// </summary>
    public JsonIgnoreAttribute() { }
}

@CodeBlanch
Copy link
Contributor Author

@steveharter @lfr I added "Never" to the JsonIgnoreCondition definition in the description.

To mitigate #31786 & #682, here's a curveball:

    /// <summary>
    /// Controls how the <see cref="JsonIgnoreAttribute"/> will decide to ignore properties.
    /// </summary>
    public enum JsonIgnoreCondition
    {
        /// <summary>
        /// Property will always be ignored.
        /// </summary>
        Always,
        /// <summary>
        /// Property will only be ignored if it is null.
        /// </summary>
        WhenNull,
        /// <summary>
        /// Property will always be serialized/deserialized regardless of <see cref="JsonSerializerOptions"/> configuration.
        /// </summary>
        Never
    }

    /// <summary>
    /// Controls how the <see cref="JsonIgnoreAttribute"/> will decide to ignore items contained in IEnumerable and IDictionary properties.
    /// </summary>
    public enum JsonChildItemIgnoreCondition
    {
        /// <summary>
        /// Children will always be serialized/deserialized regardless of value.
        /// </summary>
        Never,
        /// <summary>
        /// Children will only be ignored when they are null.
        /// </summary>
        WhenNull
    }

    /// <summary>
    /// Prevents a property from being serialized or deserialized.
    /// </summary>
    [AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
    public sealed class JsonIgnoreAttribute : JsonAttribute
    {
        /// <summary>
        /// Specifies the condition that must be met before a property will be ignored. Default value: <see cref="JsonIgnoreCondition.Always"/>.
        /// </summary>
        public JsonIgnoreCondition PropertyCondition { get; set; } = JsonIgnoreCondition.Always;

        /// <summary>
        /// Specifies the condition that must be met before a child item will be ignored. Default value: <see cref="JsonChildItemIgnoreCondition.Never"/>.
        /// </summary>
        /// <remarks>
        /// Applies to IEnumerable and IDictionary properties only.
        /// </remarks>
        public JsonChildItemIgnoreCondition ChildItemCondition { get; set; } = JsonChildItemIgnoreCondition.Never;

        /// <summary>
        /// Initializes a new instance of <see cref="JsonIgnoreAttribute"/>.
        /// </summary>
        public JsonIgnoreAttribute() { }
    }

Introduces JsonChildItemIgnoreCondition enum which can be applied as "ChildItemCondition" on JsonIgnoreAttribute. "Condition" on JsonIgnoreAttribute renamed "PropertyCondition."

The idea being, JsonChildItemIgnoreCondition.WhenNull will cause null items in IEnumerable or IDictionary props to be ignored. Example:

public class TestClass
{
   [JsonIgnore(PropertyCondition=JsonIgnoreCondition.WhenNull, ChildItemCondition=JsonChildItemIgnoreCondition.WhenNull)]
   [JsonExtensionData]
   public IDictionary<string, object> Fields { get; set; } = new Dictionary<string, object>
   {
      ["Prop1"] = "value",
      ["Prop2"] = null
   }
}

@lfr
Copy link

lfr commented Feb 6, 2020

Thanks @CodeBlanch, looking forward to testing it!

@layomia layomia self-assigned this Mar 20, 2020
@terrajobst terrajobst added the blocking Marks issues that we want to fast track in order to unblock other important work label Mar 20, 2020
@terrajobst
Copy link
Member

@layomia should this be marked as api-ready-for-review?

@layomia
Copy link
Contributor

layomia commented Mar 21, 2020

@terrajobst yes, we'll move forward with the proposal in the description.

Per-property ignore semantics for collection elements will not be included in the API proposal as there are open questions on correctness. #682 tracks this issue. Workarounds for this scenario include filtering out nulls from your collections before serializing, and writing a custom converter.

@layomia layomia added api-ready-for-review and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Mar 21, 2020
@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review blocking Marks issues that we want to fast track in order to unblock other important work labels Mar 24, 2020
@terrajobst
Copy link
Member

terrajobst commented Mar 24, 2020

Video

  • Looks good
  • We should have a follow-up for JsonSerializerOptions.IgnoreDefaultValues to support skipping value types.
namespace System.Text.Json.Serialization
{
    public enum JsonIgnoreCondition
    {
        Always,
        WhenNull,
        Never
    }

    public partial class JsonIgnoreAttribute
    {
        public JsonIgnoreCondition Condition { get; set; };
    }
}

@layomia
Copy link
Contributor

layomia commented Mar 31, 2020

From @ahsonkhan:

Rather than an enum, does it make sense to have users pass in a predicate for the IgnoreCondition? What if I only want to ignore empty strings (not null strings)? So, can I specify my own custom predicate for when to ignore things?

@layomia
Copy link
Contributor

layomia commented Mar 31, 2020

@ahsonkhan

Rather than an enum, does it make sense to have users pass in a predicate for the IgnoreCondition?

I think the enum values JsonIgnoreCondition provide a nice set of common choices "built-in" for convenience.

We could add another "OnPredicate" (or something similar) enum value, and a new property on JsonIgnore called Predicate. If the Condition's value is OnPredicate, we would ignore based on the result of the predicate (or throw if the predicate is not specified).

What if I only want to ignore empty strings (not null strings)?

Aside from using a predicate, a simple solution achievable when we add ignore semantics for default values is to set the empty string as the default value and set the Condition to WhenDefault.

@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.
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants