Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Added a flag to JsonIgnoreAttribute so we can toggle ignoring always or only when null. #40522

Closed
wants to merge 1 commit into from

Conversation

CodeBlanch
Copy link
Contributor

A new feature for consideration by the team. 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.

To illustrate how it works here's the test snippet from the PR:

        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);
        }

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

@ahsonkhan
Copy link
Member

@CodeBlanch - thanks for the PR and contribution. Since this is adding new public surface area, it should go through an API review. I have not yet reviewed this PR. Can you file the appropriate issue for the new API to be reviewed with the rationale and sample usage, similar to what you have done in the description here, and we can move forward from there. Thanks!

https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/api-review-process.md

@ahsonkhan ahsonkhan added this to the 5.0 milestone Aug 26, 2019
@ahsonkhan ahsonkhan added the enhancement Product code improvement that does NOT require public API changes/additions label Aug 26, 2019
@CodeBlanch
Copy link
Contributor Author

@ahsonkhan I created https://github.com/dotnet/corefx/issues/40600 for the API review.

@maryamariyan
Copy link
Member

maryamariyan commented Nov 6, 2019

Thank you for your contribution. As announced in dotnet/coreclr#27549 this repository will be moving to dotnet/runtime on November 13. If you would like to continue working on this PR after this date, the easiest way to move the change to dotnet/runtime is:

  1. In your corefx repository clone, create patch by running git format-patch origin
  2. In your runtime repository clone, apply the patch by running git apply --directory src/libraries <path to the patch created in step 1>

@ericstj
Copy link
Member

ericstj commented Nov 12, 2019

As @ahsonkhan mentioned we need to first follow the API review process. Once that's done we can open a PR (likely against dotnet/runtime) with this change. Thank you for your interest and contribution!

@ericstj ericstj closed this Nov 12, 2019
@lfr
Copy link

lfr commented Nov 13, 2019

This suggestion does not give us parity with Json.NET's NullValueHandling as suggested by @CodeBlanch. Json.NET allows overriding the default behavior at the property level both when the default is ignoring nulls as well as when the behavior is to include nulls.

Please look at https://github.com/dotnet/corefx/issues/42034 for more context. This suggestion is about ignoring nulls for a specific property, but when the default behavior is ignoring nulls, we may want to force including them for a specific property.

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

Successfully merging this pull request may close these issues.

6 participants