Polymorphic deserialization with System.Text.Json requires discriminator property to have the shortest JSON object key length

[schnerring]

The following sample only works due to overriding the default discriminator property named $type (key length = 5) to $t (key length = 2). Otherwise, the deserialization throws an exception.

[JsonPolymorphic(TypeDiscriminatorPropertyName = "$t")]
[JsonDerivedType(typeof(WeatherForecastBase), typeDiscriminator: "base")]
[JsonDerivedType(typeof(WeatherForecastWithCity), typeDiscriminator: "withCity")]
public class WeatherForecastBase
{
    public DateTimeOffset Date { get; set; } // key length = 4
    public int TemperatureCelsius { get; set; }
    public string? Summary { get; set; }
}

public class WeatherForecastWithCity : WeatherForecastBase
{
    public string? City { get; set; } // key length = 4
}

The System.Text.Json (STJ) docs state:

The type discriminator must be placed at the start of the JSON object, grouped together with other metadata properties like $id and $ref [and $values].

It's a pretty annoying .NET runtime issue that has been raised upstream. Not sure why STJ is opinionated about that, maybe for backwards compatibility with legacy systems? 🤔

This conflicts with jsonb columns (re-)ordering JSON object keys by length:

By contrast, jsonb does not preserve white space, does not preserve the order of object keys, and does not keep duplicate object keys. If duplicate keys are specified in the input, only the last value is kept.
[...]
In general, most applications should prefer to store JSON data as jsonb, unless there are quite specialized needs, such as legacy assumptions about ordering of object keys.
[...]
Note that object keys are compared in their storage order; in particular, since shorter keys are stored before longer keys

Should we mention this quirk in the docs under Serialization with System.Text.Json?

Another idea is to let Marten align STJ and jsonb by configuring the STJ serializer via contract model.

Similar to the JsonNetContractResolver via Json.NET serializer options, we could add the STJ analog that does something like this:

public class SystemTextJsonContractResolver : DefaultJsonTypeInfoResolver
{
    public override JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options)
    {
        var jsonTypeInfo = base.GetTypeInfo(type, options);

        var isPolymorphic = type
            .GetCustomAttributes(typeof(JsonDerivedTypeAttribute))
            .Any();

        if (isPolymorphic)
        {
            jsonTypeInfo.PolymorphismOptions = new JsonPolymorphismOptions
            {
                TypeDiscriminatorPropertyName = "$t"
                // that works too and is even shorter
                // TypeDiscriminatorPropertyName = "!"
            };
        }

        return jsonTypeInfo;
    }
}

[schnerring]

Not sure why STJ is opinionated about that, maybe for backwards compatibility with legacy systems? 🤔

See dotnet/runtime#63747 (comment)

[Hawxy]

This is like a self-referential issue, I opened the original issue in the runtime repo due to discovering the problem whilst testing marten with STJ polymorphism and now it's come full circle and ended up back here. 😆

Use of a shorter type discriminator is an interesting solution, but I personally see it as a "hack" rather than "fix" as such a short discriminator is not the norm within the ecosystem and would make migrating from newtonsoft polymorphism & other STJ polymorphic packages more difficult. Thus I wouldn't expect marten to patch this out of the box with a contract resolver. Adding something to docs mentioning this issue and some potential workarounds seems reasonable.

I'm still fine with using the PolyJson package until STJ resolves the particular issue.

[schnerring]

Thus I wouldn't expect marten to patch this out of the box with a contract resolver. Adding something to docs mentioning this issue and some potential workarounds seems reasonable.

Seems reasonable.

Another option could be allowing users to override the default Marten contract resolver via Marten store options. Without having looked at the code, I could imagine that this could potentially lead to more issues down the road.

[oskardudycz]

My personal take on this is that the STJ is terrible in that regard; some of the reasons you already mentioned. I also don’t see much benefit of having polymorphism in DTOs.

You can override the STJ settings, AFAIK we’re not blocking anything here. If we do, then happy to enable that.

I’m sorry, but I don’t see happening changing the way we serialise things just to enable this specific feature as it’s too high risk for too small a reward.

I’ll turn this into the discussion for now.