Skip to content

System.Text.Json: Allow derived types to specify base type for polymorphic serialization #111295

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

Closed
TheTrueColonel opened this issue Jan 11, 2025 · 6 comments

Comments

@TheTrueColonel
Copy link

TheTrueColonel commented Jan 11, 2025

Starting with .NET 7, according to this article, the only ways to do polymorphic de/serialization is to use JsonDerivedType and/or JsonPolymorphic on the base type or to create a custom JsonTypeInfoResolver. While this should be perfectly reasonable for many projects, it's very unreasonable for a project I work on.

I would like to see an attribute similar to JsonDerivedType but kinda working in reverse, possibly called JsonBaseType, that could be used like so:

[JsonPolymorphic(/*options...*/)]
public abstract class Member {
    public string? Name { get; set; }
    public DateTime BirthDate { get; set; }
}

[JsonBaseType(baseType: typeof(Member), typeDiscriminator: nameof(Student))]
public class Student : Member {
    public int RegistrationYear { get; set; }
    public List<string> Courses { get; set; } = [];
}

[JsonBaseType(baseType: typeof(Member), typeDiscriminator: nameof(Professor))]
public class Professor : Member {
    public string? Rank { get; set; }
    public bool IsTenured { get; set; }
}

My use case, and why this would be much more useful for me, is at my job we handle, and need slightly custom method logic, for hundreds of PDF forms. So every time a new form is processed and added to our system, it's going to have its own class generated with its metadata and an employee will add the custom logic as needed. Here is a very simplified example of how this looks in practice:

// FormBase.cs
public class FormBase {
    public virtual string? Name { get; set; }
    public virtual DateTime RevisionDate { get; set; }
    
    public virtual void FormRules() {
        // Some generic, default logic
    }
}

// Form1.cs
public class Form1 : FormBase {
    public override string Name => "Form1";
    public override DateTime RevisionDate => new(2020, 1, 1);
    
    public override void FormRules() {
        // Some custom logic
    }
}

// Form2.cs
public class Form2 : FormBase {
    public override string Name => "Form2";
    public override DateTime RevisionDate => new(2023, 1, 1);
    
    public override void FormRules() {
        // Some more custom logic
    }
}

With the scale of this pattern, and how our form classes are being generated, it would be much easier to just add an attribute to every new derived class referencing its base than needing to add hundreds of attributes to the base class or having hundreds of entries in a custom JsonTypeInfoResolver. An option for this to be handled in a way similar to Newtonsoft.JSON would be a very painless solution, but I believe that's been shot down before.

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 11, 2025
Copy link
Contributor

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

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Jan 11, 2025

Fundamentally the reason why this scenario is not supported is that we cannot reliably implement it in the context of the source generator/trimmed apps/Native AOT. Searching the type hierarchy at runtime for potential derived types requires reflection that only works in full CoreCLR. This is why we recommend using contract customization and custom attributes to get support for this scenario.

While this should be perfectly reasonable for many projects, it's very unreasonable for a project I work on.

Why is it unreasonable for your project?

@TheTrueColonel
Copy link
Author

Why is it unreasonable for your project?

Simply just because of the unique position the project is in. Legacy project where this style has been used since well before I joined, making it a tough sell on making changes (whether it be source generators or something else) on that base class.

This is why we recommend using contract customization and custom attributes to get support for this scenario.

Do you mind sharing a resource for this? This recommendation you're describing sounds like it may be able to work like I'm wanting.

we cannot reliably implement it in the context of the source generator/trimmed apps/Native AOT

And just out of curiosity, why wouldn't something like I'm suggesting work in those scenarios? I'm not too knowledgeable about how the compiler handle base/derived classes, but at least from where I'm at, if specifying every derived class from the base would work, I'd think specifying the base from why derived class could work too. Maybe some order of operations thing I'm just missing, so I'm just curious with this point.

@eiriktsarpalis
Copy link
Member

Do you mind sharing a resource for this? This recommendation you're describing sounds like it may be able to work like I'm wanting.

Here's how you can implement an attribute like the one you describe:

[AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)]
public sealed class JsonBaseTypeAttribute(Type baseType, string typeDiscriminator) : Attribute
{
    public Type BaseType { get; } = baseType;
    public string TypeDiscriminator { get; } = typeDiscriminator;
}

public static class JsonBaseTypeExtensions
{
    public static JsonSerializerOptions WithBaseTypeAttribute(this JsonSerializerOptions options)
    {
        IJsonTypeInfoResolver resolver = options.TypeInfoResolver ?? new DefaultJsonTypeInfoResolver();
        options.TypeInfoResolver = resolver.WithAddedModifier(static typeInfo =>
        {
            if (typeInfo.Type.IsValueType || typeInfo.Type.IsSealed)
            {
                return;
            }

            var derivedTypes = System.AppDomain.CurrentDomain.GetAssemblies()
                .SelectMany(a => a.GetTypes())
                .Where(t => typeInfo.Type.IsAssignableFrom(t));

            foreach (Type derivedType in derivedTypes)
            {
                if (derivedType.GetCustomAttribute<JsonBaseTypeAttribute>() is { } attr && 
                    attr.BaseType == typeInfo.Type)
                {
                    typeInfo.PolymorphismOptions ??= new();
                    typeInfo.PolymorphismOptions.DerivedTypes.Add(new(derivedType, attr.TypeDiscriminator));
                }
            }
        });

        return options;
    }
}

You can then use this in your application as follows:

var options = new JsonSerializerOptions().WithBaseTypeAttribute();
Base value = new Derived(1, 2);
JsonSerializer.Serialize(value, options); // {"$type":"derived","y":2,"x":1}

And just out of curiosity, why wouldn't something like I'm suggesting work in those scenarios?

Ιf you look at the implementation above you will see that it needs to query the full AppDomain for all possible types deriving from your base. This creates a number of problems in trimmed and AOT applications because the derived type might have been trimmed or because the source generator is not able to see and generate a contract for the derived type at compile time. This is why attributes must be specified at the base type if you're using the built-in attributes.

@TheTrueColonel
Copy link
Author

That snippet looks to work perfectly, thanks! That explanation also makes sense.

@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Jan 14, 2025
@jilles-sg
Copy link

One small nit: System.AppDomain.CurrentDomain.GetAssemblies().SelectMany(a => a.GetTypes()) leaves the application vulnerable to bugs like dotnet/SqlClient#1930 . I suggest restricting it to assemblies under your control and/or restricting it to exported types and/or catching ReflectionTypeLoadException suitably.

@github-actions github-actions bot locked and limited conversation to collaborators Feb 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants