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

Provide option for the serializer to treat IEnumerables like objects with members #1808

Closed
vbelinschi opened this issue Jan 16, 2020 · 12 comments
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@vbelinschi
Copy link

If I have a similar class:

public class MyEnumerable : IEnumerable<string>
    {
        public int Doing { get; set; }
        public IList<string> Stuff { get; set; }

        public IEnumerator<string> GetEnumerator()
        {
            return Stuff.GetEnumerator();
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return GetEnumerator();
        }
    }

The result of the serialization will be just the values of Stuff.
Fx.:

var my = new MyEnumerable();
my.Stuff = new List<string> { "one", "two" };
my.Doing = 3;
var json = System.Text.Json.JsonSerializer.Serialize(my);

the result is: ["one","two"].
The Doing member is ingored. So the expected result would be: {"Doing":3,"Stuff":["one","two"]}.

In Newtonsoft.JSON I can use [JsonObject] attribute, to enforce the serialization of all the members.

Is there something similar in System.Text.Json? And if not, is there plan to implement it?

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jan 16, 2020
@vbelinschi vbelinschi changed the title Serializing a class that implements IEnumerable<T> with Serializing a class that implements IEnumerable<T> with System.Text.Json Jan 16, 2020
@colathro
Copy link

I don't think so at this time. I believe if the object we are serializing at the top level is an enumerable, we ignore other top level attributes all the time.

Adding an optional parameter for this may be nice; serializing a List would probably include Count and Capacity. It also brings the question if the parameter applies to enumerable serialization at all levels or just top level (For all serialized Lists we'd get the Count and Capacity also).

The expected output you put is what I would imagine this situation done through composition.

public class MyObject
{
    public int Doing { get; set; }
    public IEnumerable<string> Stuff { get; set; }
}

@layomia layomia added this to the Future milestone Jan 17, 2020
@ahsonkhan ahsonkhan removed the untriaged New issue has not been triaged by the area owner label Jan 25, 2020
@ahsonkhan
Copy link
Member

@vitalie-ucommerce, is there any reason you couldn't wrap the IEnumerable<T> into a containing class that has the properties you want serialized as @colathro mentioned?

@vbelinschi
Copy link
Author

vbelinschi commented Jan 27, 2020

@ahsonkhan that would be easiest, of course. But it's not what the requirements are, we need like a IEnumerable with some meta information.
So we can use it to something like this:

var myObject = new MyObject();
foreach(var item in myObject){
...
}

but also like this:

if(myObject.Doing>1){}

@layomia
Copy link
Contributor

layomia commented Feb 1, 2020

From @HughPH in #30855:

My class is designed to get the items from an Amazon DynamoDB query and convert them to T through the magic of the Enumerator:

    public class DynamoQueryResult<T> : IEnumerable<T> where T : class, new()
    {
        public int Count;
        public Dictionary<string, DynamoValue>[] Items;
        public IEnumerator<T> GetEnumerator()
        {
            return new DynamoQueryResultEnumerator<T>(this);
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return new DynamoQueryResultEnumerator<T>(this);
        }
    }

Json Deserializers detect IEnumerable and expect the JSON at that level to therefore be a set (where this class represents a complex object containing both set metadata and set members). I can override that behaviour with Newtonsoft by adding the [JsonObject] attribute to the object and it does the job perfectly - and Newtonsoft's exception message gives a load of help which leads to this. I can't find a way to do the same for System.Text.Json. Also, really descriptive exception messages that guide the developer are one of my favourite things to create and see.

(Changing to an IReadOnlyCollection doesn't help, but ReadOnly IEnumerables could be a way to 'hint' that they're not meant to be deserialized into?)

  1. I know there is a big library of junk for dealing with Amazon stuff. That is not the answer I'm looking for - it's likely to just add delay and potentially more questions. This is a working example of where this falls over.
  2. In the immediate term, I'm likely to just create a customised collection to replace the array of dictionaries, but it would be cool if I could just convince the deserializer to fill the object.

@layomia
Copy link
Contributor

layomia commented Feb 1, 2020

From myself in #30855:

ReadOnly IEnumerables could be a way to 'hint' that they're not meant to be deserialized into

I don't think this is enough information for the serializer to change the normal behavior.

It seems that the way to move forward here is if there were an explicit way to tell the serializer to treat an IEnumerable like an object with properties, similar to Newtonsoft's [JsonObject] attribute. I'll rename this issue to reflect this.

I'll rename this issue as well.

@layomia layomia changed the title Serializing a class that implements IEnumerable<T> with System.Text.Json Provide option for the serializer to treat IEnumerables like objects with members Feb 1, 2020
@vborovikov
Copy link

Is there a workaround for this since it's not yet supported out of the box?

@arjunsol
Copy link

Have recently bumped up against this limitation had to remove the interfaces to get our custom collections to work. Luckily foreach only requires the presence of the enumerator method.

From the looks of it, we can't disable this behaviour or even replace the enumerable converter. Further comment would be welcome.

@layomia
Copy link
Contributor

layomia commented Oct 21, 2020

The workaround here is to write a custom converter for the type. Here's an example using the MyEnumerable type from the issue description. This example assumes default options are used. The code can be run here.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Text.Json;
using System.Text.Json.Serialization;

public class Program
{
    public static void Main()
    {
        var my = new MyEnumerable();
        my.Stuff = new List<string> { "one", "two" };
        my.Doing = 3;

        var options = new JsonSerializerOptions { Converters = { new MyEnumerableConverter() } };

        string json = JsonSerializer.Serialize(my, options);
        Console.WriteLine(json); // {"Doing":3,"Stuff":["one","two"]}

        my = JsonSerializer.Deserialize<MyEnumerable>(json, options);
        Console.WriteLine(my.Doing); // 3
        Console.WriteLine(my.Stuff[0]);  // one
        Console.WriteLine(my.Stuff[1]);  // two
    }

    public class MyEnumerable : IEnumerable<string>
    {
        public int Doing { get; set; }
        public IList<string> Stuff { get; set; }

        public IEnumerator<string> GetEnumerator()
        {
            return Stuff.GetEnumerator();
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return GetEnumerator();
        }
    }

    public class MyEnumerableConverter : JsonConverter<MyEnumerable>
    {
        public override MyEnumerable Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        {
            // By default, serializer handles null deserialization for reference types.
            Debug.Assert(reader.TokenType != JsonTokenType.Null);

            if (reader.TokenType != JsonTokenType.StartObject)
            {
                throw new JsonException();
            }

            var obj = new MyEnumerable();

            do
            {
                reader.Read();

                if (reader.TokenType == JsonTokenType.PropertyName)
                {
                    string propName = reader.GetString();
                    reader.Read();

                    if (propName == "Doing")
                    {
                        obj.Doing = reader.GetInt32();
                    }
                    else if (propName == "Stuff")
                    {
                        obj.Stuff = JsonSerializer.Deserialize<IList<string>>(ref reader, options);
                    }
                    else
                    {
                        reader.Skip();
                    }
                }
                else if (reader.TokenType == JsonTokenType.EndObject)
                {
                    break;
                }
            } while (true);

            return obj;
        }

        public override void Write(Utf8JsonWriter writer, MyEnumerable value, JsonSerializerOptions options)
        {
            // By default, serializer handles null serialization.
            Debug.Assert(value != null);

            writer.WriteStartObject();
            writer.WriteNumber("Doing", value.Doing);
            writer.WritePropertyName("Stuff");
            JsonSerializer.Serialize(writer, value.Stuff, options);
            writer.WriteEndObject();
        }
    }
}

@rvarna
Copy link

rvarna commented Jan 21, 2021

@layomia , is the workaround temporary or permanent (as in, there are no plans to support it out of the box)? This is blocking our migration from newtonsoft as well

@layomia
Copy link
Contributor

layomia commented Jan 21, 2021

@rvarna - we could support this out of the box in the future. This is not planned for .NET 6.0 since there is a workaround and the community demand for this is low compared to other features.

@eiriktsarpalis
Copy link
Member

One possible workaround is to simply expose ObjectConverterFactory. Users would then be able to to simply opt in to object semantics by applying the JsonConverter attribute. @layomia thoughts?

@eiriktsarpalis
Copy link
Member

Closing in favor of #63791.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 13, 2022
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

No branches or pull requests

10 participants