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

Support polymorphic serialization through new option #29937

Closed
Tracked by #45189
steveharter opened this issue Jun 18, 2019 · 63 comments
Closed
Tracked by #45189

Support polymorphic serialization through new option #29937

steveharter opened this issue Jun 18, 2019 · 63 comments
Assignees
Labels
api-needs-work API needs work before it is approved, 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

@steveharter
Copy link
Member

steveharter commented Jun 18, 2019

EDIT see #29937 (comment) for the finalized API proposal.

Original proposal by @steveharter (click to view)

Current behavior

Consider a simple model:

abstract class Person {...}
class Customer : Person {...}

The current behavior, by design, is to use the static type and not serialize polymorphically.

// This will serialize all properties on `Person` but not `Customer`:
Person person = new Customer();
string str = JsonSerializer.ToString<Person>(person);

// This does the same thing through generic type inference:
string str = JsonSerializer.ToString(person);

However, if the type is declared to be System.Object then all properties will be serialized:

object person = new Customer();
// This will serialize all properties on `Customer`
string str = JsonSerializer.ToString(person);

In addition, the Type parameter can be specified:

Person person = new Customer();
// This will serialize all properties on `Customer`
string str = JsonSerializer.ToString(person, person.GetType());

All of these semantics are current as will remain as-is.

Proposed API

Add an opt-in setting that will serialize all types polymorphically, instead of just System.Object:

namespace System.Text.Json
{
    public class JsonSerializerOptions // existing class
    {
. . .
        bool SerializePolymorphically { get; set; }    
. . .
    }
}

Example of new behavior:

var options = new JsonSerializerOptions();
options.SerializePolymorphically = true;

// This will serialize all properties on `Customer`:
Person person = new Customer();
string str = JsonSerializer.ToString(person, options);

This new behavior applies both to the root object being serialized (as shown above) plus any properties on POCOs including collections, such as List<Person>.

Note: this issue does not enable any additional support for polymorphic deserialize. Currently, a property or collection of type System.Object will result in a JsonElement being created. Any additional polymorphic deserialize support must be done with a custom converter.

@steveharter steveharter self-assigned this Jun 18, 2019
@steveharter
Copy link
Member Author

Moving to future for the following reasons:

  • 3.0 schedule is feature-complete in a few days.
  • This may not be the best programming model long-term. We should really add feature at the same time we add support for polymorphic deserialization since we may want to share common options or an enum.
  • Since we don't support true polymorphic deserialization, polymorphic serialization is not a useful for any round-tripping scenarios.
  • The new custom converter feature can be used to handle polymorphic serialization of properties, as well as polymorphic deserialization of properties or types.

@steveharter steveharter removed their assignment Jun 20, 2019
@david-driscoll
Copy link

I just ran into this scenario when doing some testing with latest preview. Is this something that is documented elsewhere? If not this should be highlighted in the migration guide as a new "feature" for new applications switching to use only the new JSON serializer. Otherwise someone might not notice the subtle change in their applications for a while.

@Symbai
Copy link

Symbai commented Jul 8, 2019

@steveharter The new custom converter feature cannot be used to handle polymorphic (de)serialization in some cases as, unless I'm missing something, because all properties have to be deserialized by hand / code. Which is not an option for maintainability when you have classes with more than two properties. See https://github.com/dotnet/corefx/issues/39031

The issue is that in the custom converter you only have a Utf8JsonReader which can read but not deserialize. Means you have to read one-by-one and set properties one-by-one.

If there however would be a way to deserialize an object, but only the object not the whole json, within the custom converter all at once, it becomes a feature that can be used.

@steveharter
Copy link
Member Author

steveharter commented Aug 8, 2019

@Symbai yes you are correct for deserialization -- there is not an easy way to support without manually authoring.

Here's a test\sample for using a type discriminator to do that:

Also here's a test\sample for polymorphic serialization. Currently the test\sample assumes the base class is abstract.

I'll work on adding real doc and samples shortly.

@Symbai
Copy link

Symbai commented Aug 9, 2019

@steveharter Thanks but the samples cannot be used in real world scenarios because classes mostly don't have only 2 properties as shown in the sample, but ten to fifty. And now imagine we serialize something that itself has a class as property and this class also has another class and so on. So in the end, we have over hundreds of properties we, currently, have to write for EACH of them this here:

 case "Name of property":
                                ... bla= reader.GetXY();
                                ((MyClass)value).Bla= bla;
                                break;

But if you have to write this more than 10 times already the code gets ugly and hardly maintainable.

Example
public override Person Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
	if (reader.TokenType != JsonTokenType.StartObject)
	{
		throw new JsonException();
	}

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

	string propertyName = reader.GetString();
	if (propertyName != "TypeDiscriminator")
	{
		throw new JsonException();
	}

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

	Person value;
	TypeDiscriminator typeDiscriminator = (TypeDiscriminator)reader.GetInt32();
	switch (typeDiscriminator)
	{
		case TypeDiscriminator.Customer:
			value = new Customer();
			break;

		case TypeDiscriminator.Employee:
			value = new Employee();
			break;

		default:
			throw new JsonException();
	}

	while (reader.Read())
	{
		if (reader.TokenType == JsonTokenType.EndObject)
		{
			return value;
		}

		if (reader.TokenType == JsonTokenType.PropertyName)
		{
			propertyName = reader.GetString();
			reader.Read();
			switch (propertyName)
			{
				case "0":
					decimal zero = reader.GetDecimal();
					((Class1)value).ZERO = zero;
					break;
				case "1":
					decimal one = reader.GetDecimal();
					((Class1)value).ONE = one;
					break;
				case "2":
					decimal two = reader.GetDecimal();
					((Class1)value).TWO = two;
					break;
				case "3":
					decimal three = reader.GetDecimal();
					((Class1)value).THREE = three;
					break;
				case "4":
					decimal four = reader.GetDecimal();
					((Class1)value).FOUR = four;
					break;
				case "5":
					decimal five = reader.GetDecimal();
					((Class1)value).FIVE = five;
					break;
				case "6":
					decimal six = reader.GetDecimal();
					((Class1)value).SIX = six;
					break;
				case "7":
					decimal seven = reader.GetDecimal();
					((Class1)value).SEVEN = seven;
					break;
				case "8":
					decimal eight = reader.GetDecimal();
					((Class1)value).EIGHT = eight;
					break;
				case "9":
					decimal nine = reader.GetDecimal();
					((Class1)value).NINE = nine;
					break;
				case "10":
					decimal ten = reader.GetDecimal();
					((Class1)value).TEN = ten;
					break;
				case "11":
					decimal eleven = reader.GetDecimal();
					((Class2)value).ELEVEN = eleven;
					break;
				case "12":
					decimal twelve = reader.GetDecimal();
					((Class2)value).TWELVE = twelve;
					break;
				case "13":
					decimal thirteen = reader.GetDecimal();
					((Class2)value).THIRTEEN = thirteen;
					break;
				case "14":
					decimal fourteen = reader.GetDecimal();
					((Class2)value).FOURTEEN = fourteen;
					break;
				case "15":
					decimal fifteen = reader.GetDecimal();
					((Class2)value).FIFTEEN = fifteen;
					break;
				case "16":
					decimal sixteen = reader.GetDecimal();
					((Class2)value).SIXTEEN = sixteen;
					break;
				case "17":
					decimal seventeen = reader.GetDecimal();
					((Class2)value).SEVENTEEN = seventeen;
					break;
				case "18":
					decimal eighteen = reader.GetDecimal();
					((Class2)value).EIGHTEEN = eighteen;
					break;
				case "19":
					decimal nineteen = reader.GetDecimal();
					((Class2)value).NINETEEN = nineteen;
					break;
				case "20":
					decimal twenty = reader.GetDecimal();
					((Class2)value).TWENTY = twenty;
					break;
				case "21":
					decimal twenty-one = reader.GetDecimal();
					((Class3)value).TWENTY-ONE = twenty-one;
					break;
				case "22":
					decimal twenty-two = reader.GetDecimal();
					((Class3)value).TWENTY-TWO = twenty-two;
					break;
				case "23":
					decimal twenty-three = reader.GetDecimal();
					((Class3)value).TWENTY-THREE = twenty-three;
					break;
				case "24":
					decimal twenty-four = reader.GetDecimal();
					((Class3)value).TWENTY-FOUR = twenty-four;
					break;
				case "25":
					decimal twenty-five = reader.GetDecimal();
					((Class3)value).TWENTY-FIVE = twenty-five;
					break;
				case "26":
					decimal twenty-six = reader.GetDecimal();
					((Class3)value).TWENTY-SIX = twenty-six;
					break;
				case "27":
					decimal twenty-seven = reader.GetDecimal();
					((Class3)value).TWENTY-SEVEN = twenty-seven;
					break;
				case "28":
					decimal twenty-eight = reader.GetDecimal();
					((Class3)value).TWENTY-EIGHT = twenty-eight;
					break;
				case "29":
					decimal twenty-nine = reader.GetDecimal();
					((Class3)value).TWENTY-NINE = twenty-nine;
					break;
				case "30":
					decimal thirty = reader.GetDecimal();
					((Class3)value).THIRTY = thirty;
					break;
				case "31":
					decimal thirty-one = reader.GetDecimal();
					((Class4)value).THIRTY-ONE = thirty-one;
					break;
				case "32":
					decimal thirty-two = reader.GetDecimal();
					((Class4)value).THIRTY-TWO = thirty-two;
					break;
				case "33":
					decimal thirty-three = reader.GetDecimal();
					((Class4)value).THIRTY-THREE = thirty-three;
					break;
				case "34":
					decimal thirty-four = reader.GetDecimal();
					((Class4)value).THIRTY-FOUR = thirty-four;
					break;
				case "35":
					decimal thirty-five = reader.GetDecimal();
					((Class4)value).THIRTY-FIVE = thirty-five;
					break;
				case "36":
					decimal thirty-six = reader.GetDecimal();
					((Class4)value).THIRTY-SIX = thirty-six;
					break;
				case "37":
					decimal thirty-seven = reader.GetDecimal();
					((Class4)value).THIRTY-SEVEN = thirty-seven;
					break;
				case "38":
					decimal thirty-eight = reader.GetDecimal();
					((Class4)value).THIRTY-EIGHT = thirty-eight;
					break;
				case "39":
					decimal thirty-nine = reader.GetDecimal();
					((Class4)value).THIRTY-NINE = thirty-nine;
					break;
				case "40":
					decimal forty = reader.GetDecimal();
					((Class4)value).FORTY = forty;
					break;
				case "41":
					decimal forty-one = reader.GetDecimal();
					((Class5)value).FORTY-ONE = forty-one;
					break;
				case "42":
					decimal forty-two = reader.GetDecimal();
					((Class5)value).FORTY-TWO = forty-two;
					break;
				case "43":
					decimal forty-three = reader.GetDecimal();
					((Class5)value).FORTY-THREE = forty-three;
					break;
				case "44":
					decimal forty-four = reader.GetDecimal();
					((Class5)value).FORTY-FOUR = forty-four;
					break;
				case "45":
					decimal forty-five = reader.GetDecimal();
					((Class5)value).FORTY-FIVE = forty-five;
					break;
				case "46":
					decimal forty-six = reader.GetDecimal();
					((Class5)value).FORTY-SIX = forty-six;
					break;
				case "47":
					decimal forty-seven = reader.GetDecimal();
					((Class5)value).FORTY-SEVEN = forty-seven;
					break;
				case "48":
					decimal forty-eight = reader.GetDecimal();
					((Class5)value).FORTY-EIGHT = forty-eight;
					break;
				case "49":
					decimal forty-nine = reader.GetDecimal();
					((Class5)value).FORTY-NINE = forty-nine;
					break;
				case "50":
					decimal fifty = reader.GetDecimal();
					((Class5)value).FIFTY = fifty;
					break;
				case "51":
					decimal fifty-one = reader.GetDecimal();
					((Class6)value).FIFTY-ONE = fifty-one;
					break;
				case "52":
					decimal fifty-two = reader.GetDecimal();
					((Class6)value).FIFTY-TWO = fifty-two;
					break;
				case "53":
					decimal fifty-three = reader.GetDecimal();
					((Class6)value).FIFTY-THREE = fifty-three;
					break;
				case "54":
					decimal fifty-four = reader.GetDecimal();
					((Class6)value).FIFTY-FOUR = fifty-four;
					break;
				case "55":
					decimal fifty-five = reader.GetDecimal();
					((Class6)value).FIFTY-FIVE = fifty-five;
					break;
				case "56":
					decimal fifty-six = reader.GetDecimal();
					((Class6)value).FIFTY-SIX = fifty-six;
					break;
				case "57":
					decimal fifty-seven = reader.GetDecimal();
					((Class6)value).FIFTY-SEVEN = fifty-seven;
					break;
				case "58":
					decimal fifty-eight = reader.GetDecimal();
					((Class6)value).FIFTY-EIGHT = fifty-eight;
					break;
				case "59":
					decimal fifty-nine = reader.GetDecimal();
					((Class6)value).FIFTY-NINE = fifty-nine;
					break;
				case "60":
					decimal sixty = reader.GetDecimal();
					((Class6)value).SIXTY = sixty;
					break;
				case "61":
					decimal sixty-one = reader.GetDecimal();
					((Class7)value).SIXTY-ONE = sixty-one;
					break;
				case "62":
					decimal sixty-two = reader.GetDecimal();
					((Class7)value).SIXTY-TWO = sixty-two;
					break;
				case "63":
					decimal sixty-three = reader.GetDecimal();
					((Class7)value).SIXTY-THREE = sixty-three;
					break;
				case "64":
					decimal sixty-four = reader.GetDecimal();
					((Class7)value).SIXTY-FOUR = sixty-four;
					break;
				case "65":
					decimal sixty-five = reader.GetDecimal();
					((Class7)value).SIXTY-FIVE = sixty-five;
					break;
				case "66":
					decimal sixty-six = reader.GetDecimal();
					((Class7)value).SIXTY-SIX = sixty-six;
					break;
				case "67":
					decimal sixty-seven = reader.GetDecimal();
					((Class7)value).SIXTY-SEVEN = sixty-seven;
					break;
				case "68":
					decimal sixty-eight = reader.GetDecimal();
					((Class7)value).SIXTY-EIGHT = sixty-eight;
					break;
				case "69":
					decimal sixty-nine = reader.GetDecimal();
					((Class7)value).SIXTY-NINE = sixty-nine;
					break;
				case "70":
					decimal seventy = reader.GetDecimal();
					((Class7)value).SEVENTY = seventy;
					break;
				case "71":
					decimal seventy-one = reader.GetDecimal();
					((Class8)value).SEVENTY-ONE = seventy-one;
					break;
				case "72":
					decimal seventy-two = reader.GetDecimal();
					((Class8)value).SEVENTY-TWO = seventy-two;
					break;
				case "73":
					decimal seventy-three = reader.GetDecimal();
					((Class8)value).SEVENTY-THREE = seventy-three;
					break;
				case "74":
					decimal seventy-four = reader.GetDecimal();
					((Class8)value).SEVENTY-FOUR = seventy-four;
					break;
				case "75":
					decimal seventy-five = reader.GetDecimal();
					((Class8)value).SEVENTY-FIVE = seventy-five;
					break;
				case "76":
					decimal seventy-six = reader.GetDecimal();
					((Class8)value).SEVENTY-SIX = seventy-six;
					break;
				case "77":
					decimal seventy-seven = reader.GetDecimal();
					((Class8)value).SEVENTY-SEVEN = seventy-seven;
					break;
				case "78":
					decimal seventy-eight = reader.GetDecimal();
					((Class8)value).SEVENTY-EIGHT = seventy-eight;
					break;
				case "79":
					decimal seventy-nine = reader.GetDecimal();
					((Class8)value).SEVENTY-NINE = seventy-nine;
					break;
				case "80":
					decimal eighty = reader.GetDecimal();
					((Class8)value).EIGHTY = eighty;
					break;
				case "81":
					decimal eighty-one = reader.GetDecimal();
					((Class9)value).EIGHTY-ONE = eighty-one;
					break;
				case "82":
					decimal eighty-two = reader.GetDecimal();
					((Class9)value).EIGHTY-TWO = eighty-two;
					break;
				case "83":
					decimal eighty-three = reader.GetDecimal();
					((Class9)value).EIGHTY-THREE = eighty-three;
					break;
				case "84":
					decimal eighty-four = reader.GetDecimal();
					((Class9)value).EIGHTY-FOUR = eighty-four;
					break;
				case "85":
					decimal eighty-five = reader.GetDecimal();
					((Class9)value).EIGHTY-FIVE = eighty-five;
					break;
				case "86":
					decimal eighty-six = reader.GetDecimal();
					((Class9)value).EIGHTY-SIX = eighty-six;
					break;
				case "87":
					decimal eighty-seven = reader.GetDecimal();
					((Class9)value).EIGHTY-SEVEN = eighty-seven;
					break;
				case "88":
					decimal eighty-eight = reader.GetDecimal();
					((Class9)value).EIGHTY-EIGHT = eighty-eight;
					break;
				case "89":
					decimal eighty-nine = reader.GetDecimal();
					((Class9)value).EIGHTY-NINE = eighty-nine;
					break;
				case "90":
					decimal ninety = reader.GetDecimal();
					((Class9)value).NINETY = ninety;
					break;
				case "91":
					decimal ninety-one = reader.GetDecimal();
					((Class10)value).NINETY-ONE = ninety-one;
					break;
				case "92":
					decimal ninety-two = reader.GetDecimal();
					((Class10)value).NINETY-TWO = ninety-two;
					break;
				case "93":
					decimal ninety-three = reader.GetDecimal();
					((Class10)value).NINETY-THREE = ninety-three;
					break;
				case "94":
					decimal ninety-four = reader.GetDecimal();
					((Class10)value).NINETY-FOUR = ninety-four;
					break;
				case "95":
					decimal ninety-five = reader.GetDecimal();
					((Class10)value).NINETY-FIVE = ninety-five;
					break;
				case "96":
					decimal ninety-six = reader.GetDecimal();
					((Class10)value).NINETY-SIX = ninety-six;
					break;
				case "97":
					decimal ninety-seven = reader.GetDecimal();
					((Class10)value).NINETY-SEVEN = ninety-seven;
					break;
				case "98":
					decimal ninety-eight = reader.GetDecimal();
					((Class10)value).NINETY-EIGHT = ninety-eight;
					break;
				case "99":
					decimal ninety-nine = reader.GetDecimal();
					((Class10)value).NINETY-NINE = ninety-nine;
					break;

			}
		}
	}

	throw new JsonException();
}

@AlexanderJohnston
Copy link

This seriously needs to be documented somewhere, like on the System.Text.Json namespace, in highlighted text. I spent hours trying to track down why it was serializing empty strings while working in a Blazor project.

@knom
Copy link

knom commented Oct 10, 2019

This is bad.. just ran into this as well.. after hours of debugging :-(
Is there a workaround using Newtonsoft.JSON?

@steentottrup
Copy link

My own findings:

https://stackoverflow.com/questions/58305928/the-new-asp-net-core-3-0-json-serializer-is-leaving-out-data/58316304#58316304

And an article on how to "go back" to Newtonsoft.Json:

https://devblogs.microsoft.com/dotnet/try-the-new-system-text-json-apis/

@Symbai
Copy link

Symbai commented Oct 10, 2019

Is there a workaround using Newtonsoft.JSON?

On Newtonsoft it's super easy. For example just tell Newtonsoft all known Types and Newtonsoft does all the magic itself for you. No converter, no worries:

public static readonly JsonSerializerSettings Settings = new JsonSerializerSettings {
	TypeNameHandling = TypeNameHandling.Objects,
	SerializationBinder = new KnownTypesBinder {
		KnownTypes = new List<Type> {
			typeof(TypeA),
			typeof(TypeB),
			typeof(TypeC),
			typeof(TypeD),
			typeof(TypeE)
		}
}};

I really wish this would get more attention, at least set to 5.0 milestone instead of future. Currently it's just unusable for polymorphic situations and there is NO workaround (I consider the converter as no workaround in real world scenarios for the reason I posted earlier), so switching is not possible.

@steveharter
Copy link
Member Author

Moving to 5.0.

However this should also be tied to polymorphic deserialization support as well which is also a requested feature but much larger in scope. There are several requests for that and the implementation must be secure and opt-in through known types; we need to combine\triage these polymorphic issues -- here's one request: https://github.com/dotnet/corefx/issues/41347

@bklooste
Copy link

Known types / magic polymorphism on the wire are bad patterns and I have seen people waste a ton of time on weird issues and bugs around this - the benefit aint worth it. Neither Json or XML are polymorphic by design and making them do it is a hack at best.

That said we should document explicit type creation better and possibly have some patterns around this eg

var typeName = json.GetProperty("TypeName").GetString(); var type = Type.GetType(typeName); return (T) JsonSerializer.Deserialize(utfJson , type);

@Symbai
Copy link

Symbai commented Oct 21, 2019

@bklooste newtonsoft Json supports it, without weird issues and bugs around. Since the .NET Team is working together with the author of Newtonsoft Json, it's even more easier.

And regarding XML, most of its code was written for more than a decade ago and remains untouched. Obviously it lacks of features and therefore is a bad comparison.

@liesahead
Copy link

liesahead commented Oct 31, 2019

As I needed only one-side serializer for specific base class (to make API return derived classes properties), I came up with current solution

public class CustomConverter : JsonConverter<BaseClass>
{
    private readonly JsonSerializerOptions _serializerOptions;

    public CustomConverter()
    {
        _serializerOptions = new JsonSerializerOptions
        {
            PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
            IgnoreNullValues = true,
        };
    }

    public override bool CanConvert(Type objectType)
    {
        return (objectType == typeof(BaseClass));
    }

    public override BaseClass Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        throw new NotImplementedException();
    }

    public override void Write(Utf8JsonWriter writer, BaseClass value, JsonSerializerOptions options)
    {
        writer.WriteStringValue(JsonSerializer.SerializeToUtf8Bytes(value, value.GetType(), _serializerOptions));
    }
}

@sfadeev
Copy link

sfadeev commented Oct 31, 2019

@DenisSemionov, it seems like your sample escapes classes and writes data as string. To serialize normally json converter should be something like this (similar to @steveharter sample above):

public class PolymorphicWriteOnlyJsonConverter<T> : JsonConverter<T>
{
	public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
	{
		throw new NotImplementedException();
	}

	public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
	{
		JsonSerializer.Serialize(writer, value, value.GetType(), options);
	}
}

... and then registered in JsonOptions:

options.JsonSerializerOptions.Converters.Add(new PolymorphicWriteOnlyJsonConverter<BaseClass>());

@liesahead
Copy link

@sfadeev , I agree with you, noticed my mistake too late. Can't find any good examples or documentation for polymorphic serialization in .net core 3.0, need to support 9 subclasses in one web API get method. I will share my solution if I come up with a smart one.

@sfadeev
Copy link

sfadeev commented Oct 31, 2019

need to support 9 subclasses in one web API get method. I will share my solution if I come up with a smart one.

@DenisSemionov, you can use sample from my comment above.

@liesahead
Copy link

need to support 9 subclasses in one web API get method. I will share my solution if I come up with a smart one.

@DenisSemionov, you can use sample from my comment above.

@sfadeev, This works exactly as needed, much appreciate for provided example!

@philpowers
Copy link

I also ran into this issue, which seemed very counter-intuitive to me. I understand the issues with deserializing polymorphic types, but serialization should be trivial because the types are always known at runtime. Because of that, I somewhat disagree that this should be tied to deserialization support (which I expected to write custom code for anyway).

Anyway, I have dealt with this by making a more generic version of the solution given by @sfadeev :

    public class AbstractWriteOnlyJsonConverter : JsonConverter<object>
    {
        public override bool CanConvert(Type typeToConvert)
        {
            return typeToConvert.IsAbstract;
        }

        public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        {
            throw new NotImplementedException();
        }

        public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
        {
            JsonSerializer.Serialize(writer, value, value.GetType(), options);
        }
    }

This allows the serializer to serialize any object derived from an abstract class:

public abstract class DtoDetails
{
}

public class ConcreteDetails : DtoDetails
{
    public string ConcreteField1 { get; set; }
    // ...
}

public class RequestDto
{
    public DtoDetails Details { get; set; }   
}

Details property of RequestDto can now be serialized properly:

options.Converters.Add(new AbstractWriteOnlyJsonConverter());

// ...
var requestDto = new RequestDto { /* ... */ }
var serialized = JsonSerializer.Serialize(requestDto, options);

This has so far worked for me.

@GarrettClyde
Copy link

We're going back to Newtonsoft for now. Here's a pretty unsophisticated workaround that might help someone else out.

    public static class RealJsonSerializer
    {
        public static string Serialize<TValue>(TValue value, JsonSerializerOptions options = null)
        {
            object objectValue = value;
            return JsonSerializer.Serialize(objectValue, options);
        }  
    }

@y-lobau
Copy link

y-lobau commented Dec 3, 2019

I've also lost half a day on that debugging asp.net sources. As Newtonsoft's out-of-the-box behavior covers inheritence, i consider this as a real breaking change which will definitely lead to regression after migration to 3.0. I guess the 1st best is to make a red alert in corresponding section of the article Migrate from ASP.NET Core 2.2 to 3.0.

@lwansbrough
Copy link

lwansbrough commented Dec 31, 2019

+1 for a day lost to this. It's not just polymorphic class abstractions, it happens to interfaces too.

If you make an interface IFoo with property IBar, which is implemented by Foo : IFoo, and then populate IBar with Baz : IBar, when you serialize Foo, all Baz properties are dropped except for those of the IBar interface. That's totally unpractical and I can't imagine why you'd find it appropriate to recommend switching from Newtonsoft with such a massive breaking change left undocumented.

I would go as far as saying making polymorphic serialization optional, and then not making it the default and then also not including it in 3.0 should be classified as a bug.

@jan-johansson-mr
Copy link

jan-johansson-mr commented Jan 18, 2020

I followed your example, and I got the exception: System.NotSupportedException: 'Deserialization of interface types is not supported' from System.Text.Json. Is this how you set up the types?

    public interface IFoo
    {
        string Label { get; set; }
        IBar Bar { get; set; }
    }

    public interface IBar
    {
        int Age { get; set; }
        string Name { get; set; }
    }
    public class Foo : IFoo
    {
        public string Label { get; set; }
        public IBar Bar { get; set; }
    }
    public class Bar : IBar
    {
        public int Age { get; set; }
        public string Name { get; set; }
    }

As an example...

One observation I've made is that only properties are handled by System.Text.Json, e.g. if you have a type with fields, then the fields will not be handled at all.

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

From @mauricio-bv in #31742

I am trying to use the System.Text.Json serialization libray, but I can't get it is serialize it as I used to with Newtonson. Properties in derived classes or Interfaces are not serializing. This issue has also been asked and explained in the link below.

Link

Is there any serialization option I should set for proper serialization?

cc @Arash-Sabet, @bailei1987, @bstordrup

@Symbai
Copy link

Symbai commented May 14, 2021

for us system.text.json works way better than newtonsoft json. the performance is so much better and we don't miss the magic.

Then go ahead and move on. Stop commenting an OPT-IN feature request by only saying that you don't need it. Not only that you ping everyone who subscribes to it, which is really annoying, it's also completely redundant and helps no one. If you have the desire to tell the world about things you dislike, please use Twitter. If you have some constructive / technical feedback to share, you're welcome. FYI: This is not just you but also other people here, I just picked you because you are the last. Thanks in advance for yours and everyone else understanding and have a nice weekend.

@ColinM9991
Copy link

Any updates on whether this is being picked up in this lifetime? There are many issues like this that just seem to sit in limbo as updates to .NET Core and the surrounding ecosystem are pushed out.

@IanKemp
Copy link

IanKemp commented May 14, 2021

Any updates on whether this is being picked up in this lifetime? There are many issues like this that just seem to sit in limbo as updates to .NET Core and the surrounding ecosystem are pushed out.

The answer is invariably "no", since Microsoft's figured out that hype-driven development is the next big thing. As in they announce and ship a cool new feature that works for 80% of scenarios, then pretend it's complete and thus never finish the outstanding 20%. I was hoping that with James Newton-King joining Microsoft, STJ would gain parity with Newtonsoft.Json, but it seems that James has been tasked to work on other new features that will inevitably also be delivered incomplete.

@bklooste
Copy link

bklooste commented May 14, 2021

@bklooste so keep dto”s plain and simple. But now you have to solve the polymorphic mapping in the dto mappers.

I think tagged unions on dtos have been around for decades (used in Kernighan and Richie C ) and the performance difference between CreateType(type) and new T() is massive.

switch ( dto.type) new mytype and setting some variables is a GOOD thing and the new c# features make this pretty, its simple code someone whose picked the language up for a few weeks can do it that cant go wrong vs signficant complexity burried in a lib which goes wrong in complicate ways..

KISS, remove the magic.

I can write 10 simple converters in an hour and test them ( basically the time in this forum) .. it takes me days ( or weeks in some cases) to find and chase bugs or look at performance issues due to complex serialization paths hitting things like the reflection type cache limit.

@saithis
Copy link

saithis commented May 14, 2021

for us system.text.json works way better than newtonsoft json. the performance is so much better and we don't miss the magic.

Then go ahead and move on. Stop commenting an OPT-IN feature request by only saying that you don't need it. Not only that you ping everyone who subscribes to it, which is really annoying, it's also completely redundant and helps no one. If you have the desire to tell the world about things you dislike, please use Twitter. If you have some constructive / technical feedback to share, you're welcome. FYI: This is not just you but also other people here, I just picked you because you are the last. Thanks in advance for yours and everyone else understanding and have a nice weekend.

What I wanted to say with my comment is, that I don't want this feature if it means worse performance. At the moment STJ and Newtonsoft.Json complement eachother nicely. STJ has top performance and Newtonsoft.Json has all the bells and whistles. Pick whats more important to you.

If STJ gets feature parity, then the performance will most likely suffer immensely and we have essentially another Newtonsoft.Json.

So if you need all the features, why not just use Newtonsoft.Json and be happy with it. If the performance isn't a problem for you, then it's a great choice.

I would like Microsoft to only implement new features for STJ, that don't sacrifice performance. We had some cases where Newtonsoft.Json took multiple seconds to serialize something and with STJ it is now almost instant. I don't want this problem back. Thanks.

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Jun 15, 2021

Proposed Design

Implementation PR

Consider the following type hierarchy:

public class Foo
{
    public int A { get; set; }
}

public class Bar : Foo
{
    public int B { get; set; }
}

public class Baz : Bar
{
    public int C { get; set; }
}

Currently, when serializing a Bar instance as type Foo the serializer will apply the JSON schema derived from the type Foo:

Foo foo = new Foo { A = 1 };
Bar bar = new Bar { A = 1, B = 2 };
Baz baz = new Baz { A = 1, B = 2, C = 3 };

JsonSerializer.Serialize<Foo>(foo); // { "A" : 1 }
JsonSerializer.Serialize<Foo>(bar); // { "A" : 1 }
JsonSerializer.Serialize<Foo>(baz); // { "A" : 1 }

Under the new proposal we can change this behaviour by annotating the base class (or interface) with the JsonPolymorphicType attribute:

[JsonPolymorphicType]
public class Foo
{
    public int A { get; set; }
}

which will result in the above values now being serialized as follows:

JsonSerializer.Serialize<Foo>(foo); // { "A" : 1 }
JsonSerializer.Serialize<Foo>(bar); // { "A" : 1, "B" : 2 }
JsonSerializer.Serialize<Foo>(baz); // { "A" : 1, "B" : 2, "C" : 3 }

Polymorphism applies to nested values as well, for example:

public class MyPoco
{
    public Foo Value { get; set; }
}

JsonSerializer.Serialize(new MyPoco { Value = foo }); // { "Value" : { "A" : 1 } }
JsonSerializer.Serialize(new MyPoco { Value = bar }); // { "Value" : { "A" : 1, "B" : 2 } }
JsonSerializer.Serialize(new MyPoco { Value = baz }); // { "Value" : { "A" : 1, "B" : 2, "C" : 3 } }

Note that the JsonPolymorphicType attribute is not inherited by derived types. In the above example Bar inherits from Foo yet is not polymorphic in its own right:

JsonSerializer.Serialize<Bar>(baz); // { "A" : 1, "B" : 2 }

If annotating the base class with an attribute is not possible, polymorphism can alternatively be opted in for a type using the new JsonSerializerOptions.SupportedPolymorphicTypes predicate:

public class JsonSerializerOptions
{
  public Func<Type, bool> SupportedPolymorphicTypes { get; set; }
}

Applied to the example above:

var options = new JsonSerializerOptions { SupportedPolymorphicTypes = type => type == typeof(Foo) };
JsonSerializer.Serialize<Foo>(foo1, options); // { "A" : 1, "B" : 2 }
JsonSerializer.Serialize<Foo>(foo2, options); // { "A" : 1, "B" : 2, "C" : 3 }

It is always possible to use this setting to enable polymorphism for every serialized type:

var options = new JsonSerializerOptions { SupportedPolymorphicTypes = _ => true };

// `options` treats both `Foo` and `Bar` members as polymorphic
Baz baz = new Baz { A = 1, B = 2, C = 3 };
JsonSerializer.Serialize<Foo>(baz, options); // { "A" : 1, "B" : 2, "C" : 3 }
JsonSerializer.Serialize<Bar>(baz, options); // { "A" : 1, "B" : 2, "C" : 3 }

As mentioned previously, this feature provides no provision for deserialization. If deserialization is a requirement, users would need to opt for the polymorphic serialization with type discriminators feature.

Proposed APIs

namespace System.Text.Json
{
+    [AttributeUsageAttribute(AttributeTargets.Class | AttributeTargets.Interface, AllowMultiple = false, Inherited = false)]
+    public sealed partial class JsonPolymorphicTypeAttribute : JsonAttribute
+    {
+        public JsonPolymorphicTypeAttribute() { }
+    }

+    public class JsonSerializerOptions
+    {
+        public Func<Type, bool> SupportedPolymorphicTypes { get; set; }
+    }
}

Open Questions

Naming Ambiguity

This feature is orthogonal to #30083, which implements a different brand of polymorphism using type discriminators. Calling both features "polymorphism" is likely to confuse users who might expect the two API sets to be the same or compatible. They might reasonably expect that APIs serialized using this feature to be deserializable using APIs from #30083. We need to be careful about the names we pick so that they clearly illustrate that they are independent. Suggested names might include "Simple Polymorphism" or "Write-Only Polymorphism".

JsonSerializerOptions.SupportedPolymorphicTypes uses a predicate instead of a class

The prototype uses a Func<Type, bool> predicate for simplicity, however that might prove constraining in the future if we need to expose more knobs. Suggested alternative implementation using an interface:

public interface IPolymorphicTypeResolver
{
    bool CanBePolymorphic(Type type);
}

@eiriktsarpalis eiriktsarpalis added api-ready-for-review API is ready for review, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work labels Jun 15, 2021
@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 6.0.0 Jun 15, 2021
@thomaslevesque
Copy link
Member

@eiriktsarpalis thanks for the update. I generally like the proposed design. Just one thing: the name SupportedPolymorphicTypes is a bit misleading; it sounds like it's a list of types, when it's actually a predicate (I don't have a better idea right now, but I think it's worth trying to find a better name)

@eiriktsarpalis
Copy link
Member

Thanks @thomaslevesque. I think you're onto something since I've already received the same feedback from someone else. For lack of a better alternative (SupportedPolymorphicTypesPredicate?) I'll let this be decided during API review.

@qsdfplkj
Copy link
Contributor

qsdfplkj commented Jun 15, 2021

How about deserialzing and a tree of foo’s where nodes can be any subtype.

For top level polymorphic behavior as in your foo example you can easily use ‘serialize < object >‘ to get similar behavior?

besides I think you should be able to deserialize whatever you serialize. It would look really broken if my client serialize an object and the server can’t deserialize it.

@eiriktsarpalis
Copy link
Member

@qsdfplkj the particular feature has no provision for deserialization. This is a simple feature intended at extending the simple brand of polymorphism that is currently possible with object values. It should only be used in scenaria where you are only interested in dumping all properties of a type hierarchy on the wire or where the deserializing process is dynamically typed.

Since it is not possible to roundtrip polymorphic values in nominal type systems without emitting some form of metadata on the wire, we are working on a separate feature that allows polymorphic deserialization using type discriminators (cf. #30083)

@qsdfplkj
Copy link
Contributor

qsdfplkj commented Jun 15, 2021

@eiriktsarpalis So far it seems you can do all of it with JsonSerializer.Serialize<object>(...). And for List<Foo> you either need to change it to List<object> or create a JsonConverter<List<T>> that calls JsonSerializer.Serialize(writer, value.Cast<object>().ToList());

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 17, 2021
@eiriktsarpalis
Copy link
Member

This feature is orthogonal to #30083, which implements a different brand of polymorphism using type discriminators. Calling both features "polymorphism" is likely to confuse users who might expect the two API sets to be the same or compatible. They might reasonably expect that APIs serialized using this feature to be deserializable using APIs from #30083. We need to be careful about the names we pick so that they clearly illustrate that they are independent. Suggested names might include "Simple Polymorphism" or "Write-Only Polymorphism".

@qsdfplkj
Copy link
Contributor

I think its not needed because I can write such code already but SerializeRuntimeTypes vs SerializeDeclaredTypes is what this is about.

@bartonjs
Copy link
Member

bartonjs commented Jun 17, 2021

Video

API Review Feedback:

  • We're concerned that the model as proposed is still too open for accidental data exposure, and so we recommend a "closed hierarchy" approach, similar to DataContractSerializer's known-types attribute.
    • The "late-bound" doodle for this was public Dictionary<Type, IEnumerable<Type>> PolymorphicSerialization { get; }
  • Given that the same closed hierarchy would apply to deserialization, we feel that the two should be reviewed together, and that a simplified serialization-only can be designed later... but not first.

@bartonjs bartonjs added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed api-ready-for-review API is ready for review, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work labels Jun 17, 2021
@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Jun 17, 2021

Here is a sketch of a model that might accommodate both designs:

[AttributeUsage(AttributeTargets.Class | AttributeTargets.Interface, AllowMultiple = false, Inherited = false)]
public class JsonPolymorphicAttribute : JsonAttribute
{
    public JsonPolymorphicAttribute();
    public JsonPolymorphicAttribute(string typeDiscriminatorPropertyName);

    public string? TypeDiscriminatorPropertyName { get; }
}

[AttributeUsage(AttributeTargets.Class | AttributeTargets.Interface, AllowMultiple = true, Inherited = false)]
public class JsonKnownTypeAttribute : JsonAttribute
{
    public JsonKnownType(Type derivedType, string typeIdentifier);
    public JsonKnownType(Type derivedType);

    public Type DerivedType { get; }
    public string? TypeIdentifier { get; }
}

Here's how a hierarchy that doesn't emit any metadata can be defined:

[JsonPolymorphic]
[JsonKnownType(typeof(Bat))]
public class Goo { }
public class Bat : Goo 
{ 
    public int A { get; set; }
}

JsonSerializer.Serialize<Goo>(new Bat { A = 42 }); // { "A" : 42 }

And here's one that does:

[JsonPolymorphic("_case")]
[JsonKnownType(typeof(Bat), "bat")]
public class Goo { }
public class Bat : Goo
{ 
    public int A { get; set; } = 42;
}

JsonSerializer.Serialize<Goo>(new Bat { A = 42 }); // { "_case" : "bat", "A" : 42 }

A JsonSerializerOptions knob might have a shape equivalent to

public Dictionary<Type, (string? TypeDiscriminatorPropertyName, IEnumerable<(Type DerivedType, string? TypeIdentifier)>)> PolymorphicSerialization { get; }`

@eiriktsarpalis
Copy link
Member

Given the API review feedback I'm going to close this issue in favor of #30083, which will be revised to address both requirements.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 18, 2021
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 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

Successfully merging a pull request may close this issue.