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

JsonSerializer polymorphic serialization and deserialization support #30083

Closed
Symbai opened this issue Jun 28, 2019 · 64 comments
Closed

JsonSerializer polymorphic serialization and deserialization support #30083

Symbai opened this issue Jun 28, 2019 · 64 comments
Assignees
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions json-functionality-doc Missing JSON specific functionality that needs documenting
Milestone

Comments

@Symbai
Copy link

Symbai commented Jun 28, 2019

EDIT see #30083 (comment) for a finalized proposal.

Original proposal by @Symbai (click to view)
public class FooBase
public class FooA : FooBase
public class FooB : FooBase
List<FooBase> SerializationObject = new List<FooBase> { new FooA(), new FooB() }

Serialized with Newtonsoft.JSON

JsonSerializerSettings Settings = new JsonSerializerSettings
{
    SerializationBinder = new KnownTypesBinder
    {
        KnownTypes = new List<Type> { typeof(FooA), typeof(FooB) }
    },
    TypeNameHandling = TypeNameHandling.Objects
};
List<FooBase> FooBases = new List<FooBase>() { new FooA(), new FooB() };
var json = JsonConvert.SerializeObject(FooBases, Settings);

will be:
[{"$type":"FooA","NameA":"FooA","Name":"FooBase"},{"$type":"FooB","NameB":"FooB","Name":"FooBase"}]

When using System.Text.Json.JsonSerializer to deserialize the json, FooA and FooB are both type of FooBase. Is there a way that JsonSerializer supports inheirited classes? How can I make sure the type will be the same and not the base class it inherits from?

@Symbai
Copy link
Author

Symbai commented Jun 28, 2019

Figured it's somehow possible using JsonConverter, which is for some reason completely undocumented https://docs.microsoft.com/en-us/dotnet/api/system.text.json?view=netcore-3.0 but I have no idea how to deserialize without copying each property by hand like it's shown in this example https://github.com/dotnet/corefx/issues/36639. Copying properties by hand seems extremely odd as classes may have lots of them and maintainability isn't given in such a case anymore. Is there a way I can simply deserialize into a specific type in the converter?

Or am I missing something and there is a simple solution like in Newtonsoft.Json ?

public class FooAConverter : System.Text.Json.Serialization.JsonConverter<FooBase>
{
    public override bool CanConvert(Type type)
    {
        return typeof(FooBase).IsAssignableFrom(type);
    }
    public override FooBase Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        reader.Read();
        var typeProperty = reader.GetString();
        if (typeProperty != "$type")
            throw new NotSupportedException();
        reader.Read();
        var typeValue = reader.GetString();
        FooBase value;
        switch (typeValue)
        {
            case "FooA":
                value = new FooA(); //<-- return deserialized FooA?
                break;
            case "FooB":
                value = new FooB(); //<-- return deserialized FooB?
                break;
            default:
                throw new System.Text.Json.JsonException();
        }
        while (reader.Read())
        {
            //How to deserialize without copying every property at this point?
            if (reader.TokenType == JsonTokenType.EndObject)
            {
                return value;
            }
        }
        throw new NotSupportedException();
    }

    public override void Write(Utf8JsonWriter writer, FooBase value, JsonSerializerOptions options)    {    }
}

@ahsonkhan
Copy link
Member

cc @steveharter

@Timovzl
Copy link

Timovzl commented Sep 23, 2019

It's pretty awkward that JsonConverter doesn't give you a way to recurse back into the "regular" flow (i.e. "not handled by this particular converter") for deserializing subcomponents.

This seems to make the whole design only useful for very simple cases, with few properties and no nested custom types.

@steveharter
Copy link
Member

Figured it's somehow possible using JsonConverter, which is for some reason completely undocumented https://docs.microsoft.com/en-us/dotnet/api/system.text.json?view=netcore-3.0

It's in the sub-namespace Serialization: https://docs.microsoft.com/en-us/dotnet/api/system.text.json.serialization?view=netcore-3.0

@steveharter
Copy link
Member

I have no idea how to deserialize without copying each property by hand like it's shown in this example

and

t's pretty awkward that JsonConverter doesn't give you a way to recurse back into the "regular" flow (i.e. "not handled by this particular converter") for deserializing subcomponents

Yes that is known - the existing converters are bare-bones converters that let you do anything you want, but also require you do essentially do everything for a given type (but you can recurse to handle other types). They were primarily intended for data-type converters, not for objects and collections.

We are working on making this better in 5.0 timeframe by providing object- and collection- based specific converters that would make this much easier, as well as improve performance in certain cases.

@Timovzl
Copy link

Timovzl commented Sep 25, 2019

but you can recurse to handle other types

@steveharter That seems like it could help. Can you give an example of how to do this?

@ANahr
Copy link
Contributor

ANahr commented Oct 21, 2019

We are using custom converters like the following for inherited class support. I'm pretty sure deserialization is abysmal from a perf point of view, but it works and doesn't need much code and serialization works as it should. So as a Workaround you can use somthing like this:

public class ParameterTransferObjectConverter : JsonConverter<ParameterTransferObject>
{
    public override ParameterTransferObject Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        using (var doc = JsonDocument.ParseValue(ref reader))
        {
            ParamType typeDiscriminator = (ParamType)doc.RootElement.GetProperty(@"parameterType").GetInt32();
            var type = ParameterManager.GetTransferObjectType(typeDiscriminator);
            // Enhance: doc.RootElement.GetRawText() has likely bad perf characteristics
            return (ParameterTransferObject)JsonSerializer.Deserialize(doc.RootElement.GetRawText(), type, options);
        }
    }

    public override void Write(Utf8JsonWriter writer, ParameterTransferObject value, JsonSerializerOptions options)
    {
        var type = ParameterManager.GetTransferObjectType(value.ParameterType);
        JsonSerializer.Serialize(writer, value, type, options);
    }
}

@lfr
Copy link

lfr commented Oct 24, 2019

Thanks @ANahr, indeed performance must be abysmal in comparison, but this allowed me to at least get deserialization to work till System.Text.Json matures, in my opinion it's a mistake for Microsoft to encourage people to use it, it should still be flagged as preview

@layomia
Copy link
Contributor

layomia commented Nov 23, 2019

Marking this as the issue to track polymorphic deserialization for 5.0.
Serialization is tracked in https://github.com/dotnet/corefx/issues/38650.

@layomia layomia changed the title JsonSerializer support for inheirited classes? JsonSerializer polymorphic deserialization support Nov 23, 2019
@layomia
Copy link
Contributor

layomia commented Nov 23, 2019

From @Milkitic in dotnet/corefx#41305

For some reasons that Json for configuration use, I did a test on how the System.Text.Json handles the interface deserialization. I read the API and can't find anything about this function. First I tried IEnumerable<T>. Here is the code:

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

namespace dotnet30test
{
    class Program2
    {
        static void Main(string[] args)
        {
            var obj = new TestSerializationObj2
            {
                Names = new HashSet<string> { "1", "2" }
            };
            var sysContent = JsonSerializer.Serialize(obj, new JsonSerializerOptions { WriteIndented = true, });
            var sysNewObj = JsonSerializer.Deserialize<TestSerializationObj>(sysContent);
        }
    }

    public class TestSerializationObj2
    {
        public IEnumerable<string> Names { get; set; }
    }
}

The Names property is forced to convert from Hashset<string> to List<string>, but it works successfully indeedly.
And then I try the custom interface, it throws JsonException. What I think that needed is something like Json.NET's auto type name handling. Here is the code:

using System;
using System.Collections;
using System.Collections.Generic;
using System.IO;
using NewtonJson = Newtonsoft.Json;
using SysJson = System.Text.Json;

namespace dotnet30test
{
    class Program
    {
        static void Main(string[] args)
        {
            var obj = new TestSerializationObj
            {
                CustomObj = new DefaultCustom()
            };

            var newtonContent = NewtonJson.JsonConvert.SerializeObject(obj,
                new NewtonJson.JsonSerializerSettings
                {
                    TypeNameHandling = NewtonJson.TypeNameHandling.Auto,
                    Formatting = NewtonJson.Formatting.Indented
                });
            var newtonNewObj = NewtonJson.JsonConvert.DeserializeObject<TestSerializationObj>(newtonContent,
                new NewtonJson.JsonSerializerSettings { TypeNameHandling = NewtonJson.TypeNameHandling.Auto }); // works

            var sysContent = SysJson.JsonSerializer.Serialize(obj, new SysJson.JsonSerializerOptions { WriteIndented = true, });
            var sysNewObj = SysJson.JsonSerializer.Deserialize<TestSerializationObj>(sysContent); // throws exception
        }
    }


    public sealed class TestSerializationObj
    {
        public Dictionary<string, int> Dictionary { get; set; } = new Dictionary<string, int>() { ["a"] = 0, ["b"] = 1 };
        public ICustom CustomObj { get; set; }
    }

    public class DefaultCustom : ICustom
    {
        public string Name { get; set; } = "Default Implementation";
        public int Id { get; set; } = 300;
    }

    public interface ICustom
    {
        int Id { get; set; }
    }
}

Thanks!

@mcatanzariti
Copy link

Please try this library I wrote as an extension to System.Text.Json to offer polymorphism: https://github.com/dahomey-technologies/Dahomey.Json

@Symbai
Copy link
Author

Symbai commented Jul 22, 2020

Why has this been removed from 5.0 roadmap?! We can currently not deserialize any POCO class, as well as any class which has an interface. This is definitely not a minor issue as both is widely used. Its a blocking issue for anyone currently using Newtonsoft. And its a known issue since about a year, so there was plenty of time. I'm sorry to say but I'm disappointed if this is something we need to wait another whole year when it ships with .NET 6 instead of .NET 5. And the bad taste also comes from the fact that serialization of these classes is supported already. So we have JSON support which only works into one direction only, it does not feel finished and ready to be shipped with .NET 5.

@lwardzala
Copy link

I've just discovered this thread and I decided to publish a solution I've been using in my projects.
It's an implementation of abstraction converter factory very easy to introduce.
It might be helpful till the issue will be closed :)
https://github.com/lwardzala/Json.Abstraction

@LarsKemmann
Copy link

@eiriktsarpalis I'm not familiar enough with the details of the serialization/type converter stack... will the proposed design support nested polymorphic types? E.g.:

[JsonKnownType(typeof(Derived1),"derived1")]
[JsonKnownType(typeof(Derived2),"derived2")]
public record Base();
public record Derived1(int X, NestedBase NestedValue) : Base();
public record Derived2(string Y) : Base();

[JsonKnownType(typeof(NestedDerived1),"nestedderived1")]
[JsonKnownType(typeof(NestedDerived2),"nestedderived2")]
public record NestedBase(string Foo);
public record NestedDerived1(string Foo, double Z) : NestedBase(Foo);
public record NestedDerived2(string Foo) : NestedBase(Foo);

Could I round-trip a Derived1 with a NestedValue of type NestedDerived1, and will the value of Z round-trip as well?

@eiriktsarpalis
Copy link
Member

Hi @LarsKemmann, yes that particular scenario would certainly be supported.

@dferretti
Copy link

@eiriktsarpalis with this proposed TypeDiscriminatorConfiguration it looks like we have a way of building up a dictionary of Type->string and its inverse, and the json serializer would use that type map when reading/writing the discriminator property. I have 2 questions for you:

  • What about letting the user provide their own Func<string, Type> and Func<Type, string> functions? I'm picturing a scenario where I might interpret the discriminator my-generic-type:some-known-concrete-type with something like
class MyTypeDiscriminatorConfiguration : ITypeDiscriminatorConfiguration // hypothethical interface
{
    Type GetType(string discriminator)
    {
        var parts = discriminator.Split(':');
        if (parts.Length == 2)
        {
            var genericType = _myKnownSafeGenericTypes[parts[0]];
            var type = _myKnownSafeTypes[parts[1]];
            return genericType.MakeGenericType(type);
        }

        return _myKnownSafeTypes[parts[0]];
    }
}

That way I don't have to register all combinations of the generic types and their type args into the type map ahead of time, and allow for other custom sorts of type lookups.

  • What about letting the discriminator be an arbitrary json object? Instead of seeing the $type key and assuming the value is a string to pass to the Type GetType(string discriminator) lookup we passed in the Utf8JsonReader like Type GetType(Utf8JsonReader discriminatorReader). Then my example above I could use a json object to represent both parts of the generic type instead of some other string concatenation method.

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Aug 26, 2021

What about letting the user provide their own Func<string, Type> and Func<Type, string> functions?

The design intentionally avoids this type of functionality, primarily since admitting arbitrary Func<string, Type> conversions would invariably expose STJ to the same class of security issues that are impacting BinaryFormatter.

@tthiery
Copy link

tthiery commented Dec 4, 2021

Please make sure to support configuring the type discriminator property name. "$type" is very common but not universal.

Thanks for picking this up. I wrote my own one with a bad performance pretty much like this one but with the caveat of the lacking infrastructure.

@IFYates
Copy link

IFYates commented Dec 15, 2021

While this change looks great for a round-trip full polymorphic solution, does it mean that there's no chance for a simple option on the serializer so that serialized output resolves polymorphic values when there's no need for additional type information?

@tthiery
Copy link

tthiery commented Dec 15, 2021

I want to nit pick on "roundtrip": polymorphic deserialization is about receiving input which has a type attribute. Roundtrip or .net to .net is a tiny percentage of the demand.

@Ilchert
Copy link

Ilchert commented Dec 30, 2021

@IFYates you can write your own converter factory to handle polymorphic serialization. It is not good enough for default implementation but works.

[AttributeUsage(AttributeTargets.Class, AllowMultiple = true)]
public class JsonKnownTypeAttribute : Attribute
{
    public Type NestedType { get; }
    public string Discriminator { get; }

    public JsonKnownTypeAttribute(Type nestedType, string discriminator)
    {
        NestedType = nestedType;
        Discriminator = discriminator;
    }
}

public class PolyJsonConverter : JsonConverterFactory
{
    private static readonly Type converterType = typeof(PolyJsonConverter<>);

    public override bool CanConvert(Type typeToConvert)
    {
        var attr = typeToConvert.GetCustomAttributes<JsonKnownTypeAttribute>(false);
        return attr.Any();
    }

    public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
    {
        var attr = typeToConvert.GetCustomAttributes<JsonKnownTypeAttribute>();
        var concreteConverterType = converterType.MakeGenericType(typeToConvert);
        return (JsonConverter)Activator.CreateInstance(concreteConverterType, attr);
    }
}

internal class PolyJsonConverter<T> : JsonConverter<T>
{
    private readonly Dictionary<string, Type> _discriminatorCache;
    private readonly Dictionary<Type, string> _typeCache;

    private static readonly JsonEncodedText TypeProperty = JsonEncodedText.Encode("$type");

    public PolyJsonConverter(IEnumerable<JsonKnownTypeAttribute> resolvers)
    {
        _discriminatorCache = resolvers.ToDictionary(p => p.Discriminator, p => p.NestedType);
        _typeCache = resolvers.ToDictionary(p => p.NestedType, p => p.Discriminator);
    }

    public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        using var doc = JsonDocument.ParseValue(ref reader);
        if (!doc.RootElement.TryGetProperty(TypeProperty.EncodedUtf8Bytes, out var typeElement))
            throw new JsonException();

        var discriminator = typeElement.GetString();
        if (discriminator is null || !_discriminatorCache.TryGetValue(discriminator, out var type))
            throw new JsonException();


        return (T)doc.Deserialize(type, options);
    }

    public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
    {
        var type = value.GetType();
        if (!_typeCache.TryGetValue(type, out var discriminator))
            throw new JsonException();

        writer.WriteStartObject();
        writer.WritePropertyName(TypeProperty.EncodedUtf8Bytes);
        writer.WriteStringValue(discriminator);
        using var doc = JsonSerializer.SerializeToDocument(value, type, options);
        foreach (var prop in doc.RootElement.EnumerateObject())
            prop.WriteTo(writer);

        writer.WriteEndObject();
    }
}

Example of usage

var data = new A[] { new B { PropA = "A-B", PropB = "B-B" }, new C { PropA = "A-C", PropC = "C-C" } };
var options = new JsonSerializerOptions
{
    Converters = { new PolyJsonConverter() }
};
var json = JsonSerializer.Serialize(data, options);
// 
var l = JsonSerializer.Deserialize<A[]>(json, options);

[JsonKnownType(typeof(B), "TypeB")]
[JsonKnownType(typeof(C), "TypeC")]
abstract class A
{
    public string PropA { get; set; }
}

class B : A
{
    public string PropB { get; set; }
}

class C : A
{
    public string PropC { get; set; }
}

This code produces following json:

[
  {
    "$type": "TypeB",
    "PropB": "B-B",
    "PropA": "A-B"
  },
  {
    "$type": "TypeC",
    "PropC": "C-C",
    "PropA": "A-C"
  }
]

@Symbai
Copy link
Author

Symbai commented Dec 30, 2021

I would prefer an official solution rather than custom "workaround" as this might break in the future. If you are in control over both serialization and deserialization this could work. If you don't you risk that whatever you released into the world, will break your application sooner or later. I have the feeling many developers might agree with me. This "feature request" has been delayed over and over again. With 76 upvotes I really hope the .NET team finally gives it the attention it deserves.

@tthiery
Copy link

tthiery commented Dec 30, 2021

Indeed. There are enough samples on the internet. We need a canon solution. Polymorphic deserialization is a basic feature.

Another topic: Using attribute based metadata is hopefully not the only way. Not everyone is in control of the target types. I understand the security implications of providing callback lookups or list based inputs which can be misused by a stack overflow sample but attributes only a not a way (see EF Core and friends)

@eiriktsarpalis
Copy link
Member

Given that the scope of this feature has changed substantially since the issue was originally opened, I've created a new user story which should hopefully serve to better establish expectations wrt the brand of polymorphism we are planning to deliver. I'm going to close this one, feel free to continue the conversation in the new story if you have any feedback. Thanks!

@eiriktsarpalis eiriktsarpalis removed User Story A single user-facing feature. Can be grouped under an epic. Team:Libraries labels Jan 13, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Feb 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions json-functionality-doc Missing JSON specific functionality that needs documenting
Projects
None yet
Development

No branches or pull requests