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: Allow out-of-order reading of metadata properties. #72604

Closed
Hawxy opened this issue Jul 21, 2022 · 59 comments · Fixed by #97474
Closed

JsonSerializer: Allow out-of-order reading of metadata properties. #72604

Hawxy opened this issue Jul 21, 2022 · 59 comments · Fixed by #97474
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@Hawxy
Copy link

Hawxy commented Jul 21, 2022

EDIT See #72604 (comment) for an API proposal.

Description

Attempting to deserialize a polymorphic structure with System.Text.Json that doesn't feature $type at start of the object results in an exception.

For my scenario, this currently prevents me from using the built-in polymorphism support with jsonb columns in Postgres, as object properties have no guaranteed order.

Developer comment on this issue: #63747 (comment)

Reproduction Steps

var databaseState = @"{
  ""BaseDictionary"": {
    ""6ed6e524-2ca4-4fea-8e21-7245ccb61863"": {
      ""Id"": ""6ed6e524-2ca4-4fea-8e21-7245ccb61863"",
      ""Name"": ""Something"",
      ""$type"": ""Derived"",
      ""OtherGuid"": ""d471c77d-5412-4e7a-a98d-8304e87792ed""
    }
  }
}";

JsonSerializer.Deserialize<WrapperType>(databaseState);

public record WrapperType(Dictionary<Guid, WrapperType.Base> BaseDictionary)
{
    [JsonDerivedType(typeof(Derived), nameof(Derived))]
    [JsonDerivedType(typeof(AlsoDerived), nameof(AlsoDerived))]
    public abstract record Base(Guid Id, string Name);
    public record Derived(Guid Id, string Name, Guid OtherGuid): Base(Id, Name);
    public record AlsoDerived(Guid Id, string Name) : Base(Id, Name);
}

Expected behavior

Deserialization should work.

Actual behavior

System.NotSupportedException: 'Deserialization of types without a parameterless constructor, a singular parameterized constructor, or a parameterized constructor annotated with 'JsonConstructorAttribute' is not supported. 

Regression?

Limitation of initial implementation

Known Workarounds

I currently use PolyJson as an alternative, as the implementation reads ahead to find the discriminator.

Configuration

Impacts any version of STJ 7.0

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 21, 2022
@ghost
Copy link

ghost commented Jul 21, 2022

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

Attempting to deserialize a polymorphic structure with System.Text.Json that doesn't feature $type at start of the object results in an exception.

For my scenario, this currently prevents me from using the built-in polymorphism support with jsonb columns in Postgres, as object properties have no guaranteed order.

Developer comment on this issue: #63747 (comment)

Reproduction Steps

var databaseState = @"{
  ""BaseDictionary"": {
    ""6ed6e524-2ca4-4fea-8e21-7245ccb61863"": {
      ""Id"": ""6ed6e524-2ca4-4fea-8e21-7245ccb61863"",
      ""Name"": ""Something"",
      ""$type"": ""Derived"",
      ""OtherGuid"": ""d471c77d-5412-4e7a-a98d-8304e87792ed""
    }
  }
}";

JsonSerializer.Deserialize<WrapperType>(databaseState);

public record WrapperType(Dictionary<Guid, WrapperType.Base> BaseDictionary)
{
    [JsonDerivedType(typeof(Derived), nameof(Derived))]
    [JsonDerivedType(typeof(AlsoDerived), nameof(AlsoDerived))]
    public abstract record Base(Guid Id, string Name);
    public record Derived(Guid Id, string Name, Guid OtherGuid): Base(Id, Name);
    public record AlsoDerived(Guid Id, string Name) : Base(Id, Name);
}

Expected behavior

Deserialization should work.

Actual behavior

System.NotSupportedException: 'Deserialization of types without a parameterless constructor, a singular parameterized constructor, or a parameterized constructor annotated with 'JsonConstructorAttribute' is not supported. 

Regression?

No response

Known Workarounds

I currently use PolyJson as an alternative, as the implementation reads ahead to find the discriminator.

Configuration

Impacts any version of STJ 7.0

Other information

No response

Author: Hawxy
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

@eiriktsarpalis eiriktsarpalis added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Jul 21, 2022
@eiriktsarpalis eiriktsarpalis added this to the Future milestone Jul 21, 2022
@eiriktsarpalis
Copy link
Member

Per #63747 (comment) this is a known limitation of the metadata reader. In the future we might consider adding read-ahead support specifically for type discriminators (I don't believe reference preservation to have such requirements), but this would be at the expense of potentially buffering excessive data in the case of async serialization.

@ilya-scale
Copy link

ilya-scale commented Oct 3, 2022

Is there any known workaround for this? I want to deserialize an object that does not come from System.Text.Json but has a discriminator that can be used that is of course not guaranteed to be first. The only thing I managed so far is to just deserialized first that one field and then specify type explicitly (i.e. not using the new polymorphic deserialization at all).

Maybe there is something else that can be done, like e.g. explicit reordering or something like that?

@eiriktsarpalis
Copy link
Member

No workaround that doesn't involve writing a custom converter, unfortunately. In the future we might consider exposing a flag that reads ahead for type discriminators, but obviously that would come at the expense of performance so it would be turned off by default.

@ilya-scale
Copy link

Thanks for the answer! I assume the "future" will not be in .Net 7, so not coming anytime soon, right?

@eiriktsarpalis
Copy link
Member

Correct, .NET 7 is currently in RC so feature development has been concluded.

@JobaDiniz
Copy link

JobaDiniz commented Oct 15, 2022

Why can't I use my own property for the type discriminator?

My object has a string Type property already. According to this documentation
it says:

Avoid a JsonPolymorphicAttribute.TypeDiscriminatorPropertyName if it conflicts with a property in your type hierarchy.

I would like to tell System.Text.Json that there is a property (the first one) named Type, just use it.

[JsonPolymorphic(TypeDiscriminatorPropertyName = "type")]
    [JsonDerivedType(typeof(ChatbotSettings), typeDiscriminator: ChatbotSettings.TypeName)]
    [JsonDerivedType(typeof(UnattendedSettings), typeDiscriminator: UnattendedSettings.TypeName)]
    [JsonDerivedType(typeof(AttendedSettings), typeDiscriminator: AttendedSettings.TypeName)]
    internal abstract class RobotSettings
    {
        /// <summary>
        /// Do not use. This is for Json serialization.
        /// </summary>
        internal RobotSettings() { }

        internal RobotSettings(string type) => Type = type;

        internal required string Type { get; init; }
    }

@eiriktsarpalis
Copy link
Member

Why can't I use my own property for the type discriminator?

The value of a string property can be arbitrary and might not necessarily reflect the runtime type of the object you are trying to serialize. In your example I would simply annotate the Type property with JsonIgnoreAttribute.

@kOchirasu
Copy link

kOchirasu commented Nov 8, 2022

I have this issue as well. When serializing JSON to MySQL using ef core, it seems that the ordering is not preserved so the data cannot be deserialized anymore.

There is a bit of a hack to work around this since MySQL seems to do deterministic ordering:

[JsonPolymorphic(TypeDiscriminatorPropertyName = "!")]

You might be able to get away with " " (space) as well, but basically a very low value ascii character that's valid will cause it to be sorted first. Definitely would not recommend doing this in production, but I am using this for myself.

@SebastianStehle
Copy link

SebastianStehle commented Nov 9, 2022

With .NET 7 I have a solution that should be at least fast.

I have a custom converter (in this case it is abstract because I have different ways to resolve type and discriminator) to handle writing and reading:

public abstract class InheritanceConverterBase<T> : JsonConverter<T>, IInheritanceConverter where T : notnull
{
    private readonly JsonEncodedText discriminatorProperty;

    public string DiscriminatorName { get; }

    protected InheritanceConverterBase(string discriminatorName)
    {
        discriminatorProperty = JsonEncodedText.Encode(discriminatorName);
        DiscriminatorName = discriminatorName;
    }

    public abstract Type GetDiscriminatorType(string name, Type typeToConvert);

    public abstract string GetDiscriminatorValue(Type type);

    public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        // Creating a copy of the reader (The derived deserialisation has to be done from the start)
        Utf8JsonReader typeReader = reader;

        if (typeReader.TokenType != JsonTokenType.StartObject)
        {
            throw new JsonException();
        }

        while (typeReader.Read())
        {
            if (typeReader.TokenType == JsonTokenType.PropertyName &&
                typeReader.ValueTextEquals(discriminatorProperty.EncodedUtf8Bytes))
            {
                // Advance the reader to the property value
                typeReader.Read();

                if (typeReader.TokenType != JsonTokenType.String)
                {
                    ThrowHelper.JsonException($"Expected string discriminator value, got '{reader.TokenType}'");
                    return default!;
                }

                // Resolve the type from the discriminator value.
                var type = GetDiscriminatorType(typeReader.GetString()!, typeToConvert);

                // Perform the actual deserialization with the original reader
                return (T)JsonSerializer.Deserialize(ref reader, type, options)!;
            }
            else if (typeReader.TokenType == JsonTokenType.StartObject || typeReader.TokenType == JsonTokenType.StartArray)
            {
                if (!typeReader.TrySkip())
                {
                    typeReader.Skip();
                }
            }
        }

        ThrowHelper.JsonException($"Object has no discriminator '{DiscriminatorName}.");
        return default!;
    }

    public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
    {
        JsonSerializer.Serialize<object>(writer, value!, options);
    }
}

public interface IInheritanceConverter
{
    string DiscriminatorName { get; }

    Type GetDiscriminatorType(string name, Type typeToConvert);

    string GetDiscriminatorValue(Type type);
}

The problem was always the writing part, because you have to buffer the resulting object:

public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
{
	var name = GetDiscriminatorValue(value.GetType());

	writer.WriteStartObject();
	writer.WriteString(discriminatorProperty, name);

	using (var document = JsonSerializer.SerializeToDocument(value, value.GetType(), options))
	{
		foreach (var property in document.RootElement.EnumerateObject())
		{
			property.WriteTo(writer);
		}
	}

	writer.WriteEndObject();
}

So you effectively write the object, then you deserialize it and then you serialize it again. So my idea with .NET 7 was to add a custom property to the contract with a custom resolver. You cannot use modifiers, because you need the type info of the base class.

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

		var baseType = type.BaseType;

		if (baseType == null || baseType == typeof(object))
		{
			return typeInfo;
		}

		var baseInfo = GetTypeInfo(baseType, options);

		if (baseInfo.Converter is IInheritanceConverter inheritanceConverter)
		{
			var discriminatorField = typeInfo.CreateJsonPropertyInfo(typeof(string), inheritanceConverter.DiscriminatorName);
			var discriminatorValue = inheritanceConverter.GetDiscriminatorValue(type);

			discriminatorField.Get = x =>
			{
				return discriminatorValue;
			};

			typeInfo.Properties.Insert(0, discriminatorField);
		}

		return typeInfo;
	}
}

I don't like that you need the type resolver and the converter right now, but I do not see another option.

@tkvalvik
Copy link

tkvalvik commented Nov 22, 2022

Currently running into this issue. We are required to consume JSON from a predefined contract from an Open API-spec provided by a third party. They are using type discriminators liberally, and we cannot consume their requests because of this without making a workaround.

Type discrimination is part of Open API specs, but ordering of elements in JSON is by definition not a thing. I would prefer correctness by default on this issue, and rather have an optional spec-violating performance mode when you van control both the consumer and producer.

@snebjorn
Copy link

snebjorn commented Dec 6, 2022

There also seem to be an issue with the error messages.

If the Base type is abstract and the payload doesn't have the typeDiscriminator as the first property then this error is shown:

System.NotSupportedException: 'Deserialization of types without a parameterless constructor, a singular parameterized constructor, or a parameterized constructor annotated with 'JsonConstructorAttribute' is not supported.

However if the Base type is not abstract and the payload doesn't have the typeDiscriminator as the first property then this error is shown:

The metadata property is either not supported by the type or is not the first property in the deserialized JSON object.

It should say the latter in both cases.

@onionhammer
Copy link

onionhammer commented Jan 3, 2023

This would also break things like cosmos' patching, where you dont control the order that properties are stored in.

Honestly, I would say this feature is not ready for prime-time at all without this issue resolved.

@TimPurdum
Copy link

This issue makes using the new JsonDerivedType attribute with EF Core impossible. Seems to me that two core .NET libraries should work together, right?

@dragorosson
Copy link

dragorosson commented Feb 22, 2023

I wrote a custom converter I'd like to share. I'm not sure how performant it is or if the error handling is up to snuff, but the code is straightforward. I think it works in .Net 6 too. Any feedback would be appreciated.

EDIT/UPDATE: I updated the code because serialization would only work if the type parameter was the base type (like JsonSerializer.Serialize<BaseType>(subtypeInstance, options)).
New .Net fiddle to see it in action. Here's the old fiddle

Some features:

  • Type discriminator property doesn't have to be in C# classes
  • Annotations like [JsonIgnore] and [JsonPropertyName] are respected
  • [JsonPolymorphicAttribute]/[JsonDerivedTypeAttribute] isn't necessary
  • Serializer options are passed to property (de)serialization
  • Deserialization doesn't need type discriminator to be first
  • Serialization puts type discriminator first

Basically it's:

  • Deserialize
    • Parse JSON to a JsonDocument (JsonDocument.TryParseValue(...))
    • Get the discriminator field (doc.RootElement.GetProperty(discriminatorPropName))
    • Use the JsonTypeInfo for the corresponding type to create an instance and deserialize/set the properties
  • Serialize
    • Serialize the value's specific type to a JsonObject (JsonSerializer.SerializeToElement, JsonObject.Create)
    • Insert the type discriminator (jObj[discriminatorPropName] = ...)
    • Use the JsonTypeInfo for the corresponding type to get/serialize/write the properties

The code:

#nullable enable
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Text.Json.Serialization.Metadata;

sealed public class PolymorphicJsonConverter<T> : JsonConverter<T>
{
    private readonly string discriminatorPropName;
    private readonly Func<Type, string> getDiscriminator;
    private readonly IReadOnlyDictionary<string, Type> discriminatorToSubtype;

    public PolymorphicJsonConverter(
        string typeDiscriminatorPropertyName,
        Func<Type, string> getDiscriminatorForSubtype,
        IEnumerable<Type> subtypes)
    {
        discriminatorPropName = typeDiscriminatorPropertyName;
        getDiscriminator = getDiscriminatorForSubtype;
        discriminatorToSubtype = subtypes.ToDictionary(getDiscriminator, t => t);
    }

    public override bool CanConvert(Type typeToConvert)
        => typeof(T).IsAssignableFrom(typeToConvert);
    
    // When a custom converter is defined for a JsonSerializerOptions instance,
    // you can't use the options to get the JsonTypeInfo for any types the
    // converter can convert, so unfortunately we have to create a copy with
    // the converters removed.
    JsonSerializerOptions? originalOptions = null;
    JsonSerializerOptions? optionsWithoutConverters = null;
    JsonTypeInfo getTypeInfo(Type t, JsonSerializerOptions givenOpts)
    {
        if (optionsWithoutConverters is null)
        {
            originalOptions = givenOpts;
            optionsWithoutConverters = new(givenOpts);
            optionsWithoutConverters.Converters.Clear();
        }

        if (originalOptions != givenOpts)
        {
            throw new Exception(
                $"A {typeof(PolymorphicJsonConverter<>).Name} instance cannot " +
                $"be used in multiple {nameof(JsonSerializerOptions)} instances!");
        }

        return optionsWithoutConverters.GetTypeInfo(t);
    }

    public override T Read(
        ref Utf8JsonReader reader, Type objectType, JsonSerializerOptions options)
    {
        using var doc = JsonDocument.ParseValue(ref reader);

        JsonElement root = doc.RootElement;
        JsonElement typeField = root.GetProperty(discriminatorPropName);

        if (typeField.GetString() is not string typeName)
        {
            throw new JsonException(
                $"Could not find string property {discriminatorPropName} " +
                $"when trying to deserialize {typeof(T).Name}");
        }

        if (!discriminatorToSubtype.TryGetValue(typeName, out Type? type))
        {
            throw new JsonException($"Unknown type: {typeName}");
        }

        JsonTypeInfo info = getTypeInfo(type, options);

        T instance = (T)info.CreateObject!();

        foreach (var p in info.Properties)
        {
            if (p.Set is null) continue;

            if (!root.TryGetProperty(p.Name, out JsonElement propValue))
            {
                if (p.IsRequired)
                {
                    throw new JsonException($"Required property {p.Name} was not found.");
                }
                else
                {
                    continue;
                }
            }

            p.Set(instance, propValue.Deserialize(p.PropertyType, options));
        }

        return instance;
    }

    public override void Write(
        Utf8JsonWriter writer, T? value, JsonSerializerOptions options)
    {
        Type type = value!.GetType();

        if (type == typeof(T))
        {
            throw new NotSupportedException(
                $"Cannot serialize an instance of type {typeof(T)}, only its subtypes.");
        }

        writer.WriteStartObject();
        
        writer.WriteString(discriminatorPropName, getDiscriminator(type));

        JsonTypeInfo info = getTypeInfo(type, options);

        foreach (var p in info.Properties)
        {
            if (p.Get is null) continue;

            writer.WritePropertyName(p.Name);
            object? pVal = p.Get(value);
            JsonSerializer.Serialize(writer, pVal, options);
        }

        writer.WriteEndObject();
    }
}

@onionhammer
Copy link

@dragorosson you might want to check out https://github.com/wivuu/Wivuu.JsonPolymorphism/

it does basically what you're doing above but with a source generator

@jaliyaudagedara
Copy link
Contributor

This is a huge issue. Azure Logic Apps adds the discriminator as the last property. So if we want to Deserialize a workflow that is already created using Azure Portal, we can't deserialize just because of this. it's a bummer.

We shouldn't rely on the order of properties.

I can understand the performance impact, but my thinking is primary functionality isn't working is a bigger issue than that.

@schnerring
Copy link

schnerring commented May 11, 2023

I'm using Marten which under the hood uses Postgres its jsonb columns.

For my scenario, this currently prevents me from using the built-in polymorphism support with jsonb columns in Postgres, as object properties have no guaranteed order.

jsonb columns (re-)order 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

So a workaround for the case of Postgres is choosing a discriminator that's shorter than any other object key (i.e. property name) like [JsonPolymorphic(TypeDiscriminatorPropertyName = "$t")]

@ilmax
Copy link

ilmax commented Dec 7, 2023

I'm also unfortunately blocked on migrating to STJ due to this.
It would be nice to have a temporary package that allows the reader to find the discriminator in whichever position it is. We could use a package till the limitation is lifted

@trejjam
Copy link

trejjam commented Dec 7, 2023

That's why https://github.com/aviationexam/json-converter-source-generator/ happen :) Sure it's not a MS package, but it does the job

@arknu
Copy link

arknu commented Jan 4, 2024

Great job. You implement support for polymorphism in System.Text.Json, only to immediately make it unusable by implementing an unrealistic ordering requirement that is rarely met in the wild. So you can basically only use it with JSON you control the creation of yourself. Why bother at all, then? Per the spec, JSON has no ordering, so why require it in System.Text.JSON? Performance is always welcome, but not at the expense of functionality.

@miminno
Copy link

miminno commented Jan 4, 2024

At least some opt-in workaround should've been offered for those that can sacrifice some performance for compatibility. How the heck can you guarantee that discriminator will come first if JSON has no ordering?? That's some poor engineering decision. And I thought STJ was ready for prime time, finally, after many years of development.

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Jan 22, 2024

API proposal

namespace System.Text.Json;

public partial class JsonSerializerOptions
{
    public bool AllowOutOfOrderMetadataProperties { get; set; } = false;
}

namespace System.Text.Json.Serialization;

public partial JsonSourceGenerationOptionsAttribute
{
     public bool AllowOutOfOrderMetadataProperties { get; set; } = false;
}

API Usage

Example 1: Polymorphism

var options = new JsonSerializerOptions { AllowOutOfOrderMetadataProperties = true };

JsonSerializer.Deserialize<MyBase>("""{ "Value1" : 1, "Value2" : 2, "$type" : "derived" }""", options); // succeeds

[JsonDerived(typeof(MyDerived), "derived")]
public record MyBase(int Value1);
public record MyDerived(int Value1, int Value2) : MyBase(Value1);

Example 2: Reference preservation

var options = new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.Preserve, AllowOutOfOrderMetadataProperties = true };

var result = JsonSerializer.Deserialize<List<int>[]>("""[{ "$values" : [1, 2, 3], "$id" : "1" }, { "$ref" : "1" }]""", options);
Console.WriteLine(result[0] == result[1]); // True

@eiriktsarpalis eiriktsarpalis self-assigned this Jan 22, 2024
@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 Jan 22, 2024
@onionhammer
Copy link

I would suggest the opposite in order to comply with JSON spec by default, then add a property that requires strict ordering for those that want that optimization.. less opportunity for footguns

@eiriktsarpalis
Copy link
Member

The concern there is that would regress users that rely on the streaming serializer not over-buffering by default. Folks using STJ to roundtrip data in a schema that they own should not need this -- for every other use case we'll make sure that the flag is adequately documented so that people know to turn it on.

@onionhammer
Copy link

If the type does happen to be at the start does it still need to continue buffering the whole thing?

@eiriktsarpalis
Copy link
Member

Yes, the metadata reader checks for a number of properties that need to be validated (invalid combinations, duplicates, etc.) so it can't just stop on the first occurrence of $type.

@eiriktsarpalis eiriktsarpalis changed the title Polymorphic Deserialization throws if $type metadata isn't present at the start of the object JsonSerializer: Allow out-of-order reading of metadata properties. Jan 22, 2024
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 24, 2024
@eiriktsarpalis
Copy link
Member

Candidate implementation: #97474

@bartonjs
Copy link
Member

bartonjs commented Jan 30, 2024

Video

Looks good as proposed. The default may need to change from false to true in the future; but we'll let that be based on data/feedback.

namespace System.Text.Json
{
    public partial class JsonSerializerOptions
    {
        public bool AllowOutOfOrderMetadataProperties { get; set; } = false;
    }
}

namespace System.Text.Json.Serialization
{
    public partial class JsonSourceGenerationOptionsAttribute
    {
        public bool AllowOutOfOrderMetadataProperties { get; set; } = false;
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation in-pr There is an active PR which will close this issue when it is merged labels Jan 30, 2024
@eiriktsarpalis eiriktsarpalis added the in-pr There is an active PR which will close this issue when it is merged label Feb 1, 2024
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 2, 2024
@eiriktsarpalis
Copy link
Member

The new flag should become available with .NET 9 Preview 2.

@tyler-boyd
Copy link

tyler-boyd commented Feb 7, 2024

I know there were a few workarounds posted already, but none of them supported my use case. I don't want to use source generation, and I need it to support nested polymorphic objects as well as always including the discriminator when serializing subclasses (even if we're not casting them to the base class). So, this is what I came up with (by modifying things I saw in this thread and others), hopefully it helps anyone else who needs this behaviour before .NET 9!

Custom attribute and converter

/// <summary>
/// Same as <see cref="JsonDerivedTypeAttribute"/> but used for the hack below. Necessary because using the built-in
/// attribute will lead to NotSupportedExceptions.
/// </summary>
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Interface, AllowMultiple = true, Inherited = false)]
public class HackyJsonDerivedAttribute(Type subtype, string discriminator) : Attribute
{
    public Type Subtype { get; set; } = subtype;
    public string Discriminator { get; set; } = discriminator;
}

public sealed class PolymorphicJsonConverterFactory : JsonConverterFactory
{
    public override bool CanConvert(Type typeToConvert)
    {
        return typeToConvert.IsAbstract && typeToConvert.GetCustomAttributes<HackyJsonDerivedAttribute>().Any();
    }

    public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options)
    {
        return (JsonConverter?) Activator.CreateInstance(
            typeof(PolymorphicJsonConverter<>).MakeGenericType(typeToConvert), options);
    }
}

/// <summary>
/// A temporary hack to support deserializing JSON payloads that use polymorphism but don't specify $type as the first field.
/// Modified from https://github.com/dotnet/runtime/issues/72604#issuecomment-1440708052.
/// </summary>
public sealed class PolymorphicJsonConverter<T> : JsonConverter<T>
{
    private readonly string _discriminatorPropName;
    private readonly Dictionary<string, Type> _discriminatorToSubtype = [];

    public PolymorphicJsonConverter(JsonSerializerOptions options)
    {
        _discriminatorPropName = options.PropertyNamingPolicy?.ConvertName("$type") ?? "$type";
        foreach (var subtype in typeof(T).GetCustomAttributes<HackyJsonDerivedAttribute>())
        {
            _discriminatorToSubtype.Add(subtype.Discriminator, subtype.Subtype);
        }
    }

    public override bool CanConvert(Type typeToConvert) => typeof(T) == typeToConvert;

    public override T Read(
        ref Utf8JsonReader reader, Type objectType, JsonSerializerOptions options)
    {
        var reader2 = reader;
        using var doc = JsonDocument.ParseValue(ref reader2);

        var root = doc.RootElement;
        var typeField = root.GetProperty(_discriminatorPropName);

        if (typeField.GetString() is not { } typeName)
        {
            throw new JsonException(
                $"Could not find string property {_discriminatorPropName} " +
                $"when trying to deserialize {typeof(T).Name}");
        }

        if (!_discriminatorToSubtype.TryGetValue(typeName, out var type))
        {
            throw new JsonException($"Unknown type: {typeName}");
        }

        return (T) JsonSerializer.Deserialize(ref reader, type, options)!;
    }

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

JsonTypeInfo modifier to always include discriminator when writing JSON

options.TypeInfoResolver = new DefaultJsonTypeInfoResolver
{
    Modifiers =
    {
        static typeInfo =>
        {
            var propertyNamingPolicy = typeInfo.Options.PropertyNamingPolicy;

            // Temporary hack to ensure subclasses of abstract classes will always include the $type field
            if (typeInfo.Type.BaseType is {IsAbstract: true} &&
                typeInfo.Type.BaseType.GetCustomAttributes<HackyJsonDerivedAttribute>().Any())
            {
                var discriminatorPropertyName = propertyNamingPolicy?.ConvertName("$type") ?? "$type";
                if (!typeInfo.Properties.Any(p => p.Name == discriminatorPropertyName))
                {
                    var discriminatorValue = typeInfo.Type.BaseType
                        .GetCustomAttributes<HackyJsonDerivedAttribute>()
                        .First(attr => attr.Subtype == typeInfo.Type).Discriminator;
                    var propInfo = typeInfo.CreateJsonPropertyInfo(typeof(string), discriminatorPropertyName);
                    propInfo.Get = _ => discriminatorValue;
                    typeInfo.Properties.Insert(0, propInfo);
                }
            }
        },
    },
};

@github-actions github-actions bot locked and limited conversation to collaborators Mar 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

Successfully merging a pull request may close this issue.