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

More extensible object and collection converters #36785

Closed
steveharter opened this issue May 20, 2020 · 7 comments
Closed

More extensible object and collection converters #36785

steveharter opened this issue May 20, 2020 · 7 comments
Assignees
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Text.Json
Milestone

Comments

@steveharter
Copy link
Member

steveharter commented May 20, 2020

This is the new issue to add a public object\POCO converter. The previous issue was #1562 which was re-purposed for a refactoring effort and subsequently closed.

API proposal to come soon. It is expected to address several issues to make authoring converters for POCOs much easier and also be leveraged by a code-gen (AOT) effort that is in progress.

See 5.0 serializer goals for high-level information on the code-gen effort.

Work

@steveharter steveharter added api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Text.Json labels May 20, 2020
@steveharter steveharter added this to the 5.0 milestone May 20, 2020
@steveharter steveharter self-assigned this May 20, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label May 20, 2020
@steveharter steveharter removed the untriaged New issue has not been triaged by the area owner label May 20, 2020
@steveharter steveharter changed the title Support for a public Object converter Add support for a more extensible object and collection converters Aug 4, 2020
@steveharter steveharter modified the milestones: 5.0.0, Future Aug 4, 2020
@steveharter
Copy link
Member Author

Moving to future.

There are three aspects to this issue that overlap, so we want a single, consistent model to cover these:

  • Extensible model for object and collections, as an alternative to the existing "value-type" converter model:
  • Expose metadata (Open up metadata infrastructure of System.Text.Json  #34456).
    • Ability to inspect existing metadata (useful for external tooling such as JSON validation).
    • Ability to change or specify metadata.
    • Ability to specify delegate for setting and getting a property.
    • Metadata includes:
      • For an "object" includes its properties, serialization attributes, converter, and constructor delegate.
      • Metadata for an object "property" includes serialization attributes, converter and delegates to the setter\getter.
      • Metadata for a "collection" includes serialization attributes, converter, element converter and constructor delegate.
  • Code-gen effort for performance (startup time, memory) Developers apps using JSON serialization start up and run faster #1568
    • Internally this uses the two items above.

@weifenluo
Copy link

Please at least support writing custom converter for System.Data.DataTable and System.Data.DataRow. Current JsonConverter<T> is intended primarily for data types, not full\complex objects.

@steveharter
Copy link
Member Author

Moving to 7.0 for consideration of remaining features.

@NinoFloris
Copy link
Contributor

For

Allow custom converters to feed back to the POCO converter not to write the property name Allow custom converters to feed back to the POCO converter not to write the property name #50294

see the proposal by @eiriktsarpalis in #55781 (comment)

@scottdorman
Copy link

In #31257, it's hinted that the OnSerialzing methods from #39896 would be helpful in ignoring properties at run time, but I'm not seeing how this behavior would work.

Specifically, I need to exclude a collection during serialization only when it's empty.

I have a custom collection class

[JsonConverter(typeof(LinkCollectionConverter))]
public sealed class LinkCollection : IEnumerable<KeyValuePair<string, Link>>, IEnumerable
{
   public static readonly LinkCollection Empty = new(new Dictionary<string, Link>());
   // omitted for brevity
}

and the associated converter

public class LinkCollectionConverter : JsonConverter<LinkCollection>
{
    public override bool HandleNull => false;

    public override LinkCollection? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        Dictionary<string, Link>? value = JsonSerializer.Deserialize<Dictionary<string, Link>>(ref reader, options);
        return new LinkCollection(value);
    }

    public override void Write(Utf8JsonWriter writer, LinkCollection value, JsonSerializerOptions options)
    {
        if (value is not null && value.Count > 0)
        {
            writer.WriteStartObject();

            foreach (KeyValuePair<string, Link> pair in value)
            {
                writer.WritePropertyName(pair.Key);
                JsonSerializer.Serialize(writer, pair.Value, options);
            }

            writer.WriteEndObject();
        }
    }
}

The POCO using this collection looks like

public class LicenseName
{
    [JsonPropertyName("name")]
    [JsonInclude]
    [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
    public string? Name { get; set; }

    [JsonPropertyName("_links")]
    [JsonInclude]
    [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
    public LinkCollection? Links { get; init; } = null;
}

Although this works, what I really want is for the property in the POCO to be this

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
public LinkCollection Links { get; init; } = LinkCollection.Empty();

A serialized instance should look like

{"name":"test"}

The problem is that when I define my property as I want (with it initialized to an empty collection rather than null), I end up with JSON that looks like

{"name":"test", "_links": }

which isn't valid JSON. I really need a way to prevent the property from being written at all when it's empty. Is this possible with the state of STJ in .NET 6? I've tried implementing IJsonOnSerializing, but the callback never seems to be called.

When I deserialize into an instance, it should populate the Links property or leave it empty, depending on what the source JSON looks like (which is the result of making an HttpClient call to a web API).

What would be really nice is if there were an additional JsonIgnoreCondition value like JsonIgnoreCondition.WhenEmpty that would cause the serialization to behave as expected. It would essentially behave in a way similar to JsonIgnoreCondition.WhenWritingNull or JsonIgnoreCondition.WhenWritingDefault, but check that if the type being serialized is a collection type and it's empty, then it skips writing the property.

@DavidErben
Copy link

DavidErben commented Dec 10, 2021

I might have found a solution for this, but it is a bit ugly since you need to implement the IJsonOnSerializing interface in every single class you want to serialize without empty collections. You basically set all empty collections to null before the serializer kicks in. This way the ignore condition is honored and the property is not serialized.

    public class Product : IJsonOnSerializing
    {
        public string Name { get; set; }
        public ICollection<string> Tags { get; set; }

        public Product()
        {
            Tags = new HashSet<string>();
        }

        public void OnSerializing()
        {
            if (Tags is not null && Tags.Count == 0)
            {
                Tags = null;
            }
        }
    }

    static async Task Main(string[] args)
    {
        var options = new JsonSerializerOptions() { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull };
        Console.WriteLine(JsonSerializer.Serialize(new Product() { Name = "Test" }, options));
    }

Produces the following output:

{"Name":"Test"}

In my case, all classes are reverse engineered from a database schema, so I could easily generate the code with custom handlebar templates. So far the JSON response of my API was often quite big just because of the empty collections, since most classes had several navigation properties. If I load 50 entities of one of my main tables, the JSON response is now only 23kB instead of 111kB which is just 1/5th. So for me this seems to work, but if you cannot generate the classes automatically, it is a pain to edit everything manually.

It is also improving performance and (logically) allocations! Serializing Product class with/without IJsonOnSerializing interface:

Method Mean Error StdDev Median Ratio RatioSD Gen 0 Allocated
SerializeEmptyLists 354.9 ns 7.15 ns 17.95 ns 360.6 ns 1.00 0.00 0.1121 704 B
DontSerializeEmptyLists 233.1 ns 4.72 ns 12.10 ns 233.6 ns 0.66 0.05 0.0484 304 B

Serializing with source generator context:

[JsonSourceGenerationOptions(DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull)]
[JsonSerializable(typeof(Product))]
[JsonSerializable(typeof(ProductCustom))]
internal partial class MyJsonContext : JsonSerializerContext
{
}
Method Mean Error StdDev Ratio Gen 0 Allocated
SerializeEmptyLists 199.5 ns 2.12 ns 1.77 ns 1.00 0.0572 360 B
DontSerializeEmptyLists 157.4 ns 1.18 ns 1.04 ns 0.79 0.0484 304 B

Serializing Product class with three instead of one property of type ICollection:

Method Mean Error StdDev Ratio Gen 0 Allocated
SerializeEmptyLists 607.7 ns 3.69 ns 3.08 ns 1.00 0.1554 976 B
DontSerializeEmptyLists 492.4 ns 5.48 ns 4.85 ns 0.81 0.1450 912 B

Serializing Product class with three instead of one property of type ICollection and source generator context:

Method Mean Error StdDev Ratio RatioSD Gen 0 Allocated
SerializeEmptyLists 304.9 ns 5.87 ns 6.99 ns 1.00 0.00 0.1006 632 B
DontSerializeEmptyLists 259.4 ns 1.30 ns 1.02 ns 0.85 0.02 0.0901 568 B

@eiriktsarpalis
Copy link
Member

Closing in favor of #63686, with a few additional features tracked by #63762.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Text.Json
Projects
None yet
Development

No branches or pull requests

8 participants