Contract model example is incorrect - Field DerivedTypes cannot be set

[kezenator]

Type of issue

Code doesn't work

Description

The example in the "Configure polymorphism with the contract model" section is incorrect.
It suggest that you need to set the "DerivedTypes" property initialization - but property JsonPolymorphismOptions.DerivedTypes is get only.

Instead of:

jsonTypeInfo.PolymorphismOptions = new JsonPolymorphismOptions
{
    TypeDiscriminatorPropertyName = "$point-type",
    IgnoreUnrecognizedTypeDiscriminators = true,
    UnknownDerivedTypeHandling = JsonUnknownDerivedTypeHandling.FailSerialization,
    DerivedTypes =
    {
        new JsonDerivedType(typeof(ThreeDimensionalPoint), "3d"),
        new JsonDerivedType(typeof(FourDimensionalPoint), "4d")
    }
};

It should be:

jsonTypeInfo.PolymorphismOptions = new JsonPolymorphismOptions
{
    TypeDiscriminatorPropertyName = "$point-type",
    IgnoreUnrecognizedTypeDiscriminators = true,
    UnknownDerivedTypeHandling = JsonUnknownDerivedTypeHandling.FailSerialization
};
jsonTypeInfo.PolymorphismOptions.DerivedTypes.Add(new JsonDerivedType(typeof(ThreeDimensionalPoint), "3d"));
jsonTypeInfo.PolymorphismOptions.DerivedTypes.Add(new JsonDerivedType(typeof(FourDimensionalPoint), "4d"));

Page URL

https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/polymorphism

Content source URL

https://github.com/dotnet/docs/blob/main/docs/standard/serialization/system-text-json/polymorphism.md

Document Version Independent Id

17511810-6572-ad88-6eaf-4f9bded720b7

Platform Id

c3675880-a4e2-ce92-63f1-ddab72bb3c4c

Article author

@gewarren

Metadata

• ID: 179cc49a-a37d-155e-5ecf-deca4f219a5a
• PlatformId: c3675880-a4e2-ce92-63f1-ddab72bb3c4c
• Service: dotnet-fundamentals

Related Issues

[kezenator]

NOTE - looks like the VB example is correct - it's the C# example that's incorrect.

[gewarren]

@kezenator The example is actually correct and compiles without warning. Even though the collection initializer syntax makes it look like the property is being set, it's not actually being set. The DerivedTypes property is an IList. The collection initializer calls the get accessor to retrieve the (already instantiated) IList then calls Add on that interface. cc @BillWagner

[kezenator]

Yes. I'm sure you are correct.
However I would still argue the example is misleading and the VB sample is much more useful for the use-case you describe.

The example you have shown (with a fixed list of DerivedType instances) doesn't require the use of the contract model - you could do this by using the appropriate JsonDerivedType/JsonPolymorphic properties as per the previous sections in the page "Serialize properties of derived classes" / "Polymorphic type discriminators" / "Customize the type discriminator name".

Surely one of the key reasons to use the contract method is when the list of derived types is not known at compile time - e.g. as you actually say:

For use cases where attribute annotations are impractical or impossible (such as large domain models, cross-assembly hierarchies, or hierarchies in third-party dependencies), to configure polymorphism use the contract model.

In this case - you cannot use the list initialization syntax, and need to use the Add method as per the VB example.

Again - just to repeat - your example code IS NOT APPROPRIATE for the use case you explicitly mention in that section.

[gewarren]

The use case talks about the ability to add attribute annotations, and whether you call Add() or use a collection initializer, you still need to know the type names at compile time, so I'm not sure I follow your logic. @IEvangelist or @eiriktsarpalis do you want to weigh in here?

[kezenator]

Hi Genevieve,
Thanks for participating. Last chance. If I can't convince you this time - I'll give up.
Thanks again for you time - and you work (for it looks like years!) on what is generally really good documentation.

The problem with using the JsonDerivedType Attribute is that it needs to be placed on the base class and list all of the derived classes - as per this example I've copied from the top of section "Serialize properties of derived classes"

[JsonDerivedType(typeof(WeatherForecastWithCity))]
public class WeatherForecastBase
{
    public DateTimeOffset Date { get; set; }
    public int TemperatureCelsius { get; set; }
    public string? Summary { get; set; }
}
public class WeatherForecastWithCity : WeatherForecastBase
{
    public string? City { get; set; }
}

As per the quote I included above - this is not practical in a number of cases - including when you are writing a library and you want a third-party to be able to register an additional type - a third-party user "can't" change the attributes on your base class.

In the above example - imagine that class WeatherForecastWithCity is in another assembly, or provided by a third-party as an extension. This means class WeatherForecastBase can't know to include the JsonDerivedType attribute.

Extending the example above - here is a rough outline of the code I ended up with in my project.
As I said - I think the VB example is a good example.

private readonly List<JsonDerivedType> _derivedTypes;

public void RegisterExtensionType<T>(string discriminator)
        where T: WetherForecastBase
{
    _derivedTypes.Add(new JsonDerivedType(typeof(T), discriminator));
}

public JsonSerializerOptions CreateJsonSerializerOptions()
{
    ...
        new JsonPolymorphismOptions {
            TypeDiscriminatorPropertyName = "$type",
            IgnoreUnrecognizedTypeDiscriminators = false,
            UnknownDerivedTypeHandling = JsonUnknownDerivedTypeHandling.FailSerialization,
        };

        foreach (var derivedType in _derivedTypes) {
            typeInfo.PolymorphismOptions.DerivedTypes.Add(derivedType);
        }
    ...
}

Changing to use a collection initializer spread element might also be a good option.

...
new JsonPolymorphismOptions {
    TypeDiscriminatorPropertyName = "$type",
    IgnoreUnrecognizedTypeDiscriminators = false,
    UnknownDerivedTypeHandling = JsonUnknownDerivedTypeHandling.FailSerialization,
    DerivedTypes = [.. _derivedTypes]
};
...

[IEvangelist]

Hi @kezenator — I follow what you're saying, and I understand the limitation you're calling attention to with the attributes in the base class not possibly knowing about external assemblies that aim to extend the base. That's what the contract model is for though, and that code example is correct. I know that the syntax can be misleading as it resembles a property assignment although it's a collection initializer. The VB example is identical in terms of functionality. Use the spread operator as you suggest, results in a compiler error as you cannot assign to a read-only { get; } property.

If I understand your ask here, you'd prefer to see this code—is that right?

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

        Type basePointType = typeof(BasePoint);
        if (jsonTypeInfo.Type == basePointType)
        {
            jsonTypeInfo.PolymorphismOptions = new JsonPolymorphismOptions
            {
                TypeDiscriminatorPropertyName = "$point-type",
                IgnoreUnrecognizedTypeDiscriminators = true,
                UnknownDerivedTypeHandling = JsonUnknownDerivedTypeHandling.FailSerialization
-                DerivedTypes =
-                {
-                    new JsonDerivedType(typeof(ThreeDimensionalPoint), "3d"),
-                    new JsonDerivedType(typeof(FourDimensionalPoint), "4d")
-                }
            };

+           jsonTypeInfo.PolymorphismOptions.DerivedTypes.Add(
+               new JsonDerivedType(typeof(ThreeDimensionalPoint), "3d"));
+           jsonTypeInfo.PolymorphismOptions.DerivedTypes.Add(
+               new JsonDerivedType(typeof(FourDimensionalPoint), "4d"));
        }

        return jsonTypeInfo;
    }
}

I respectfully disagree, as this suggestion is more verbose (and not necessary a major violation of LoD, but certainly leaning in that direction). I'm not sure if this is helpful, but I'm happy to continue to help where possible. Thank you for posting this issue, much appreciated.