System.Text.Json Polymorphic Type Resolving Issue

[akurone]

Description

hi there,
as discussed in #dotnet/aspnetcore/issues/41399 SystemTextJsonOutputFormatter calls serializer with derived type of the object and this seems reasonable (as per the comment in the code). when called like this, even the attributes introduced in #63747 are present, the serializer does not output type discriminators. in my use case this causes a deserialization problem: without the discriminators, receiving side cannot use the json produced (by the endpoint). i have a workaround but it seems like a misuse and has its own drawback: i need to change my class design for it to work.

Reproduction Steps

using System;
using System.Text.Json;
using System.Text.Json.Serialization;

try
{
	Console.WriteLine($"{nameof(DerivedA)}, showing workaround:");
	Console.WriteLine(JsonSerializer.Serialize(new DerivedA(), typeof(DerivedA)));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedB)}, like STJOutputFormatter:");
	//STJOutputFormatter calls serializer this way https://github.com/dotnet/aspnetcore/issues/41399
	Console.WriteLine(JsonSerializer.Serialize(new DerivedB(), typeof(DerivedB)));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedB)}, with base type generic works as expected:");
	Console.WriteLine(JsonSerializer.Serialize<SomeBaseType>(new DerivedB()));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedB)}, with base type info works as expected:");
	Console.WriteLine(JsonSerializer.Serialize(new DerivedB(), typeof(SomeBaseType)));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedC)}, workaround is crippled:");
	Console.WriteLine(JsonSerializer.Serialize(new DerivedC(), typeof(DerivedC)));
}
catch (Exception ex)
{
	Console.WriteLine(ex.Message);
}
Console.ReadLine();


[JsonPolymorphic/*properties of this attribute are not altering the result in this case*/,
 JsonDerivedType(typeof(DerivedA), nameof(DerivedA)),
 JsonDerivedType(typeof(DerivedB), nameof(DerivedB)),
 JsonDerivedType(typeof(DerivedC), nameof(DerivedC))]
public abstract class SomeBaseType { public int Prop1 { get; set; } = 1; }

[JsonDerivedType(typeof(DerivedA), nameof(DerivedA))] // this extra attribute is **the** workaround
public class DerivedA : SomeBaseType { public int Prop2 { get; set; } = 2; }

//no extra attribute
public class DerivedB : SomeBaseType { public int Prop3 { get; set; } = 3; }

[JsonDerivedType(typeof(DerivedC), nameof(DerivedC))]
//sealing this type will throw: https://github.com/dotnet/runtime/blob/v7.0.0-rc.2.22472.3/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PolymorphicTypeResolver.cs#L174
//but this is a valid use case here
public sealed class DerivedC : SomeBaseType { public int Prop4 { get; set; } = 4; }

Expected behavior

when the base type is decorated with required attributes (or other tools mentioned in #63747 are utilized to provide polymorphic information) the serializer should respect those even the call is made with direct type and add correct discriminator to output.
2nd case of the repro code should print:

DerivedB, like STJOutputFormatter:
{"$type":"DerivedB","Prop3":3,"Prop1":1}

Actual behavior

when called with the derived type serializer is not adding discriminators to output (result of the repro code):

DerivedA, showing workaround:
{"$type":"DerivedA","Prop2":2,"Prop1":1}

DerivedB, like STJOutputFormatter:
{"Prop3":3,"Prop1":1}

DerivedB, with base type generic works as expected:
{"$type":"DerivedB","Prop3":3,"Prop1":1}

DerivedB, with base type info works as expected:
{"$type":"DerivedB","Prop3":3,"Prop1":1}

DerivedC, workaround is crippled:
Specified type 'DerivedC' does not support polymorphism. Polymorphic types cannot be structs, sealed types, generic types or System.Object.

Regression?

no

Known Workarounds

see the repro code.

Configuration

SDK: 7.0.100-rc.2.22477.23

Other information

No response

[ghost]

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

hi there,
as discussed in #dotnet/aspnetcore/issues/41399 SystemTextJsonOutputFormatter calls serializer with derived type of the object and this seems reasonable (as per the comment in the code). when called like this, even the attributes introduced in #63747 are present, the serializer does not output type discriminators. in my use case this causes a deserialization problem: without the discriminators, receiving side cannot use the json produced (by the endpoint). i have a workaround but it seems like a misuse and has its own drawback: i need to change my class design for it to work.

Reproduction Steps

using System;
using System.Text.Json;
using System.Text.Json.Serialization;

try
{
	Console.WriteLine($"{nameof(DerivedA)}, showing workaround:");
	Console.WriteLine(JsonSerializer.Serialize(new DerivedA(), typeof(DerivedA)));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedB)}, like STJOutputFormatter:");
	//STJOutputFormatter calls serializer this way https://github.com/dotnet/aspnetcore/issues/41399
	Console.WriteLine(JsonSerializer.Serialize(new DerivedB(), typeof(DerivedB)));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedB)}, with base type generic works as expected:");
	Console.WriteLine(JsonSerializer.Serialize<SomeBaseType>(new DerivedB()));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedB)}, with base type info works as expected:");
	Console.WriteLine(JsonSerializer.Serialize(new DerivedB(), typeof(SomeBaseType)));
	Console.WriteLine();
	Console.WriteLine($"{nameof(DerivedC)}, workaround is crippled:");
	Console.WriteLine(JsonSerializer.Serialize(new DerivedC(), typeof(DerivedC)));
}
catch (Exception ex)
{
	Console.WriteLine(ex.Message);
}
Console.ReadLine();


[JsonPolymorphic/*properties of this attribute are not altering the result in this case*/,
 JsonDerivedType(typeof(DerivedA), nameof(DerivedA)),
 JsonDerivedType(typeof(DerivedB), nameof(DerivedB)),
 JsonDerivedType(typeof(DerivedC), nameof(DerivedC))]
public abstract class SomeBaseType { public int Prop1 { get; set; } = 1; }

[JsonDerivedType(typeof(DerivedA), nameof(DerivedA))] // this extra attribute is **the** workaround
public class DerivedA : SomeBaseType { public int Prop2 { get; set; } = 2; }

//no extra attribute
public class DerivedB : SomeBaseType { public int Prop3 { get; set; } = 3; }

[JsonDerivedType(typeof(DerivedC), nameof(DerivedC))]
//sealing this type will throw: https://github.com/dotnet/runtime/blob/v7.0.0-rc.2.22472.3/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/PolymorphicTypeResolver.cs#L174
//but this is a valid use case here
public sealed class DerivedC : SomeBaseType { public int Prop4 { get; set; } = 4; }

Expected behavior

when the base type is decorated with required attributes (or other tools mentioned in #63747 are utilized to provide polymorphic information) the serializer should respect those even the call is made with direct type and add correct discriminator to output.
2nd case of the repro code should print:

DerivedB, like STJOutputFormatter:
{"$type":"DerivedB","Prop3":3,"Prop1":1}

Actual behavior

when called with the derived type serializer is not adding discriminators to output (result of the repro code):

DerivedA, showing workaround:
{"$type":"DerivedA","Prop2":2,"Prop1":1}

DerivedB, like STJOutputFormatter:
{"Prop3":3,"Prop1":1}

DerivedB, with base type generic works as expected:
{"$type":"DerivedB","Prop3":3,"Prop1":1}

DerivedB, with base type info works as expected:
{"$type":"DerivedB","Prop3":3,"Prop1":1}

DerivedC, workaround is crippled:
Specified type 'DerivedC' does not support polymorphism. Polymorphic types cannot be structs, sealed types, generic types or System.Object.

Regression?

no

Known Workarounds

see the repro code.

Configuration

SDK: 7.0.100-rc.2.22477.23

Other information

No response

Author:	akurone
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[eiriktsarpalis]

This is by design -- polymorphic serialization is type-directed and as such only works if the declared type has been configured as polymorphic and this configuration is not inherited by derived types. This matches the semantics of object polymorphism: it only kicks in if the declared type is of type object but nothing happens if you specify a derived type.

Arguably, the issue lies with the implementation of SystemTextJsonOutputFormatter. Using the contract for the runtime type rather than that of the declared type can result in unintended serialization/disclosure of derived type data. I think it should be changed, if possible. cc @captainsafia.

i have a workaround but it seems like a misuse and has its own drawback: i need to change my class design for it to work.

You might consider using a custom contract resolver that implements inherited polymorphism configuration.

[akurone]

thanks for the quick response Eirik.
i (over-pragmatically) thought changing a relatively new feature could be a bit more easy than something old enough to break a lot (and also has many friends like json object response formatter/producer/what ever that can break a lot on their own) and opened the issue here; also having all the properties of derived type in the json output is a more valid requirement than mine (which might not be possible before STJ supported polymorphic serialization) - so the justification seemed reasonable.

You might consider using a custom contract resolver that implements inherited polymorphism configuration.

i already implemented a custom resolver to solve my problem (repro code is the simplest form i could summarize it), it appends needed info to the derived type:

//if the type is a derived type of a base which defines its own polymorphic type info, 
//containing this type as a derived type then:
jsonTypeInfo.PolymorphismOptions = new() { TypeDiscriminatorPropertyName = TypeProperty };
jsonTypeInfo.PolymorphismOptions.DerivedTypes.Add(new JsonDerivedType(type, type.Name));
return jsonTypeInfo;

this tricks the serializer to think that the type (which is already a derived type of some base) is going to be derived (but in my case this is never going to happen; hence originally was sealed). so using this resolver effectively changes the sample in #63747 below:

[JsonDerivedType(typeof(Base), typeDiscriminatorId: "base")]
[JsonDerivedType(typeof(Derived), typeDiscriminatorId: "derived")]
public class Base
{
    public int X { get; set; }
}

public class Derived : Base
{
    public int Y { get; set; }
}

to:

[JsonDerivedType(typeof(Base), typeDiscriminatorId: "base")]
[JsonDerivedType(typeof(Derived), typeDiscriminatorId: "derived")]
public class Base
{
    public int X { get; set; }
}

//with no actual derived types and can not be sealed now
[JsonDerivedType(typeof(Derived), typeDiscriminatorId: "derived")]
public class Derived : Base
{
    public int Y { get; set; }
}

i agree with your point; both from design perspective and being practical: not having to declare same info twice is "more" ok for me 😊. but if the change (of STJOutputFormatter & friends) you mention is not going happen; having something (attribute is just to demo the idea, i will use resolver if this is implemented) like this will be ok:

[JsonDerivedType(typeof(Base), typeDiscriminatorId: "base")]
[JsonDerivedType(typeof(Derived), typeDiscriminatorId: "derived")]
public class Base
{
    public int X { get; set; }
}

[IAmADerivedTypeAndINeedToOutputTypeDiscriminator(typeof(Derived), typeDiscriminatorId: "derived")]
public sealed class Derived : Base
{
    public int Y { get; set; }
}

[eiriktsarpalis]

Closing as answered.

[akurone]

hello Eirik,
my last comment (which i think you take as answer) just sums up the workaround and its limitations and states my agreement with your proposed solution(: resolver or attribute (i'd rather not -hence the need for something like IAmADerivedTypeAndINeedToOutputTypeDiscriminator- but) i have to change (unseal the derived type) my current domain class to be able to serialize this kind of polymorphic object with STJOutputFormatter; and (as you mentioned) a change in the formatter to properly call STJ serializer would be a better solution).

so i think closing the issue here means this will not be implemented:

...
[IAmADerivedTypeAndINeedToOutputTypeDiscriminator(typeof(Derived), typeDiscriminatorId: "derived")]
public sealed class Derived : Base
...

this issue is closed as answered (and the "answer" is just a summary of current situation) but the problem still exists and no comments from @captainsafia, i am a little confused how to proceed: shall i open an issue in aspnetcore repo or shall i stick with my half way workaround as the solution?

[eiriktsarpalis]

Hi, I closed the issue because, as mentioned, this is by-design behaviour, and we won't be changing it in future releases (it would constitute a breaking change).

-hence the need for something like IAmADerivedTypeAndINeedToOutputTypeDiscriminator-

I don't think a special attribute is necessary, it should be possible to write a custom resolver that enforces the semantics you desire. Obviously unsealing the derived type is necessary for this to work:

var options = new JsonSerializerOptions { TypeInfoResolver = new InheritedPolymorphismResolver() };
JsonSerializer.Serialize(new Derived(), options); // {"$type":"derived","Y":0,"X":0}

[JsonDerivedType(typeof(Base), typeDiscriminator: "base")]
[JsonDerivedType(typeof(Derived), typeDiscriminator: "derived")]
public class Base
{
    public int X { get; set; }
}

public class Derived : Base
{
    public int Y { get; set; }
}

public class InheritedPolymorphismResolver : DefaultJsonTypeInfoResolver
{
    public override JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options)
    {
        JsonTypeInfo typeInfo = base.GetTypeInfo(type, options);

        // Only handles class hierarchies -- interface hierarchies left out intentionally here
        if (!type.IsSealed && typeInfo.PolymorphismOptions is null && type.BaseType != null)
        {
            // recursively resolve metadata for the base type and extract any derived type declarations that overlap with the current type
            JsonPolymorphismOptions? basePolymorphismOptions = GetTypeInfo(type.BaseType, options).PolymorphismOptions;
            if (basePolymorphismOptions != null)
            {
                foreach (JsonDerivedType derivedType in basePolymorphismOptions.DerivedTypes)
                {
                    if (type.IsAssignableFrom(derivedType.DerivedType))
                    {
                        typeInfo.PolymorphismOptions ??= new()
                        {
                            IgnoreUnrecognizedTypeDiscriminators = basePolymorphismOptions.IgnoreUnrecognizedTypeDiscriminators,
                            TypeDiscriminatorPropertyName = basePolymorphismOptions.TypeDiscriminatorPropertyName,
                            UnknownDerivedTypeHandling = basePolymorphismOptions.UnknownDerivedTypeHandling,
                        };

                        typeInfo.PolymorphismOptions.DerivedTypes.Add(derivedType);
                    }
                }
            }
        }

        return typeInfo;
    }
}

[akurone]

thanks a lot Eirik, i submitted the issue to aspnetcore team. for reference to people having the same issue: along with the workaround described here -if you must seal the types and accept to take a little performance hit and can negotiate the return value with the receiving party- you can wrap the polymorphic type as a second workaround, see: #dotnet/aspnetcore/issues/44852

[halter73]

@eiriktsarpalis A problem is that in most ASP.NET Core scenarios we do not know if users want this behavior or not. It seems to us that in most cases, if you've gone through the effort of attributing the base type for this kind of polymorphism, we should default to including the "$type" property. I feels like the developer is basically opting in at that point.

Is ASP.NET expected to add an InheritedPolymorphismResolver to achieve this? It seems like more libraries than just ASP.NET might want to do this. If it's too breaking to change the default behavior, can we add a top-level property to JsonSerializerOptions to control this? And make this a default option given JsonSerializerDefaults.Web?

@brunolins16

[eiriktsarpalis]

if you've gone through the effort of attributing the base type for this kind of polymorphism, we should default to including the "$type" property. I feels like the developer is basically opting in at that point.

This follows existing polymorphism semantics of object, as illustrated in the following example:

public class Base { }

public class Derived : Base
{
    public int X { get; set; }
}

var value = new Derived { X = 42 };
JsonSerializer.Serialize<Base>(value); // Serialized as Base: {}
JsonSerializer.Serialize<object>(value): // Serialized as Derived: { "X" : 42 }

TL;DR polymorphism is a property of the declared type being serialized and is not inherited by derived types. We actually debated this during API review of #63747 and didn't think that derived types should inherit polymorphic configuration, by default. There are use cases where it shouldn't happen, and it becomes hairy to support in the case of more complicated type hierarchies (conflicting attribute declarations/multiple inheritance in interfaces).

If it's too breaking to change the default behavior, can we add a top-level property to JsonSerializerOptions to control this? And make this a default option given JsonSerializerDefaults.Web?

We might consider adding a knob in JsonPolymorphicAttribute that opts it in (thought it would likely just throw in certain cases). It would still be a breaking change to include in JsonSerializerDefaults.Web however.

[alfkonee]

I really hope this is Implemented in in the runtime as it's a major blocker that's preventing our team from moving to this from Newtonsoft

[habex-ch]

I really hope this will be implemented because with JsonPolymorphic a want polymorphic behaviour or else this whole new feature makes no sense with ASP.NET Core.

[eiriktsarpalis]

This has been addressed in the aspnetcore layer, but we still need to add support unspeakable types in System.Text.Json. Tracking issue: #82457

[eiriktsarpalis]

We'd still want to support this via an opt-in flag on the JsonPolymorphicAttribute:

namespace System.Text.Json.Serialization;

public class JsonPolymorphicAttribute
{
+    public bool ApplyToDerivedTypes { get; set; } = false;
}

which can be used as follows

Derived derived = new Derived(1, 2);
// produces same output for both declared types
JsonSerializer.Serialize<Base>(derived); // {"$type":"derived","y":2,"x":1}
JsonSerializer.Serialize<Derived>(derived); // {"$type":"derived","y":2,"x":1}

[JsonPolymorphic(ApplyToDerivedTypes = true)]
[JsonDerivedType(typeof(Derived), "derived")]
public record Base(int x);
public record Derived(int x, int y) : base(x);

Notes on behaviour:

1. Uses nearest ancestor resolution to detect any supertype that is polymorphic.
2. The flag would trigger an exception in case of diamond ambiguity in interface hierarchies.
3. Consequently, derived types specifying JsonDerivedType configuration of their own would override any ancestor polymorphic types.

cc @eerhardt