[API Proposal]: TypeDiscriminatorPropertyName Reuse existing property

[rudiv]

Background and motivation

In a storage <-> API flow it seems typical that a polymorphic JSON entity would already have some form of discriminator.

Consider an event base that could be considered as follows:

[JsonDerivedType(typeof(MyTypedEvent), "myevent")]
public class EventBase {
    public string Type { get; set; }
}
public class MyTypedEvent : EventBase {
    public MyTypedEvent() {
        Type = "myevent";
    }
    public bool EventProperty { get; set; }
}

The $discriminator column would be automatically added and contain "myevent", however Type already states that it is of the type "myevent".

At present the docs state:

Avoid using a JsonPolymorphicAttribute.TypeDiscriminatorPropertyName that conflicts with a property in your type hierarchy.

Rightfully so, as the serialisation would output {"Type":"myevent","Type":"myevent", .. }.

Furthermore, deserialisation of this duplicate type results in the following exception:

System.Text.Json.JsonException: The '$id', '$ref' or '$type' metadata properties must be JSON strings. Current token type is 'Null'. Path: $.Type | LineNumber: 0 | BytePositionInLine: 12.`

This proposal covers that it should be an allowable scenario to prevent the $type (which would match the type in this scenario) being serialised twice.

API Proposal

There would be no difference to the TypeDiscriminatorPropertyName, it would simply gain the ability to reuse an existing property if present.

API Usage

[JsonPolymorphic(TypeDiscriminatorPropertyName = "Type")]
[JsonDerivedType(typeof(MyTypedEvent), "myevent")]
public class EventBase {
    public string Type { get; set; }
}

Alternative Designs

Rather than using the same method, it could be added as a new attribute against the property:

[JsonPolymorphicTypeDiscriminator]
public string Type { get; set; }

Risks

The JsonDerivedType attribute allows a string or an int to be provided as the discriminator, however an existing property would only map to a singular type.

There are of course implementation "risks" (ie. incorrect implementations) wherein the value contained within the discriminator placed by the class could be mismatched by the [JsonDerivedType(...)] attribute, however this could be solved in the API by overwriting the value contained, or simply ignored.

---

This may be somewhat related to #79933, insofar as the real "Type" property at present would be null. I can't reproduce as I can't actually deserialise the class with these duplicate properties. However solving either of these would potentially allow property re-use, I think my proposal focuses primarily on the need to not serialise twice in the first place.

Also a related discussion appears to have happened in #72604 which seems to imply a performance aspect to having the discriminator ordered at the start of the document. I'm not sure anyone would see an issue with an explicit (existing) property being ordered to the start of the document if it were relied upon for performant deserialisation (which is why we use STJ in the first instance).

We could emulate this behaviour with a JsonConverter, but it seems unnecessary if the core API was extended to support existing properties.

[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

---

Background and motivation

In a typical storage/API return flow it seems typical that a polymorphic JSON entity would already have some form of discriminator.

Consider an event base that could be considered as follows:

[JsonDerivedType(typeof(MyTypedEvent), "myevent")]
public class EventBase {
    public string Type { get; set; }
}
public class MyTypedEvent : EventBase {
    public MyTypedEvent() {
        Type = "myevent";
    }
    public bool EventProperty { get; set; }
}

The $discriminator column would be automatically added and contain "myevent", however Type already states that it is of the type "myevent".

At present the docs state:

Avoid using a JsonPolymorphicAttribute.TypeDiscriminatorPropertyName that conflicts with a property in your type hierarchy.

Rightfully so, as the serialisation would output {"Type":"myevent","Type":"myevent", .. }.

Furthermore, deserialisation of this duplicate type results in the following exception:

System.Text.Json.JsonException: The '$id', '$ref' or '$type' metadata properties must be JSON strings. Current token type is 'Null'. Path: $.Type | LineNumber: 0 | BytePositionInLine: 12.`

This proposal covers that it should be an allowable scenario to prevent the $discriminator (which would match the type in this scenario) being serialised twice.

API Proposal

There would be no difference to the TypeDiscriminatorPropertyName, it would simply gain the ability to reuse an existing property if present.

API Usage

[JsonPolymorphic(TypeDiscriminatorPropertyName = "Type")]
[JsonDerivedType(typeof(MyTypedEvent), "myevent")]
public class EventBase {
    public string Type { get; set; }
}

Alternative Designs

Rather than using the same method, it could be added as a new attribute against the property:

[JsonPolymorphicTypeDiscriminator]
public string Type { get; set; }

Risks

The JsonDerivedType attribute allows a string or an int to be provided as the discriminator, however an existing property would only map to a singular type.

Author:	rudiv
Assignees:	-
Labels:	api-suggestion, area-System.Text.Json, untriaged
Milestone:	-

[ProTip]

I want to use explicit properties defined on my types for the discriminator so tools like Swashbuckle can pick them up. Tried using explicit static Type on an abstract base overridden with values from an enum but got bitten by the duplicate serialization of the discriminator.

Related discussion in Swashbuckle domaindrivendev/Swashbuckle.AspNetCore#2571 (though it seems to no longer be actively developed).

Edit: On #72604 (these really add up!) PolyJson was mentioned. I gave this a go before writing my own general polymorphic converter. Seems to address both issues so far.