JsonPolymorphicAttribute.IgnoreUnrecognizedTypeDiscriminators doesn't handle "no discriminator" use case

[boylec]

JsonPolymorphicAttribute.UnknownDerivedTypeHandling should handle the case where when deserializing an object there is no discriminator present in the JSON at all (as opposed to simply an unrecognized one).

I would've expected this to be the case, and it would be useful for various reasons. For me it would allow a much quicker/painless path to start using STJ in projects that have not previously had discriminators on polymorphic serialized objects.

[eiriktsarpalis]

Could you share a reproduction? This works as expected for me:

using System.Text.Json;
using System.Text.Json.Serialization;

string json = JsonSerializer.Serialize<Base>(new Derived2(1,2,3));
Console.WriteLine(json); // {"y":2,"x":1}

[JsonPolymorphic(UnknownDerivedTypeHandling = JsonUnknownDerivedTypeHandling.FallBackToNearestAncestor)]
[JsonDerivedType(typeof(Derived))]
record Base(int x);
record Derived(int x, int y) : Base(x);
record Derived2(int x, int y, int z) : Derived(x, y);

If your question concerns deserialization support (i.e. being able to roundtrip a polymorphic value), this is not possible if no discriminator is specified in the payload.

[elgonzo]

FYI, there was just recently a similar question in the discussion area also asking about some mechanism to deal with absent type discriminators in json data for polymorphic deserialization scenarios: https://github.com/dotnet/runtime/discussions/104089

[boylec]

Could you share a reproduction? This works as expected for me:

using System.Text.Json;
using System.Text.Json.Serialization;

string json = JsonSerializer.Serialize<Base>(new Derived2(1,2,3));
Console.WriteLine(json); // {"y":2,"x":1}

[JsonPolymorphic(UnknownDerivedTypeHandling = JsonUnknownDerivedTypeHandling.FallBackToNearestAncestor)]
[JsonDerivedType(typeof(Derived))]
record Base(int x);
record Derived(int x, int y) : Base(x);
record Derived2(int x, int y, int z) : Derived(x, y);

If your question concerns deserialization support (i.e. being able to roundtrip a polymorphic value), this is not possible if no discriminator is specified in the payload.

My apologies Eirik, I think I actually mistook the purpose of this property. I was probably looking more at something along the lines of the IgnoreUnrecognizedTypeDiscriminators property. As per the discussion @elgonzo mentioned.

Will close this and move to the discussion.

[boylec]

Ok I did look at that discussion but it wasn't precisely pertinent to what I was getting at with this issue in my opinion so I'm reopening.

I'm basically looking to customize what happens if JSON has no discriminator defined. It occurs to me that this would likely be useful to many consumers of STJ.

Something along the lines of replacing

public bool IgnoreUnrecognizedTypeDiscriminators { get; set; }

with something like

public Type? UnrecognizedDiscriminatorFallbackType { get; set; }

on JsonPolymorphicAttribute and have this control the behavior if there is either an un-matching discriminator or no discriminator at all on the JSON being deserialized.

• If UnrecognizedDiscriminatorFallbackType were null it would imply the equivalent behavior of how IgnoreUnrecognizedTypeDiscriminators being set to false behaves today.
  • Throw an exception when the JSON has a non-matching type discriminator or no type discriminator at all.
• If UnrecognizedDiscriminatorFallbackType were not null, it would imply what Type should be deserialized to as a fallback.
  • Deserialize to the specified type when un-matching or no type discriminator at all.

In my view this would present more granular control for developers than just forcing deserialization to the base type when there is no matching discriminator found. Useful when introducing STJ polymorphism to a project with existing JSON structures that have no discriminators already defined (such as mine).

For the time being I've implemented a custom converter to accomplish similar, but this does seem like this (or something similar) could be a useful addition to the built in polymorphism feature of STJ.

Thanks for your time.

[eiriktsarpalis]

An "unrecognized discriminator" in this context refers to a present discriminator property in the payload that simply does not map to any of the explicitly configured derived types. It does not have any effect on payloads that do not specify a discriminator. I'm not sure I understand the utility of this proposal: if your model explicitly defines types without a discriminator but then you want to have every payload map back to a type other than the base type, it seems to me that it's defeating the purpose of using polymorphism in the first place.

In the interest of keeping things simple, I would be inclined to say that we wouldn't want to support such a feature. If you want to be able to round trip values, use type discriminators. If you want to support a nonstandard scenario, write a custom converter.

[boylec]

Let me see if I can describe what I think is an example of the value add.

1. I have a data store with a "Cart" collection. At first over the first couple of years of my apps existence this is a non-polymorphic type just a simple shopping cart object. The table builds up data over time.

// no polymorphic JSON attributes
public record Cart {
  public string CartId { get; init; }
  // ... other cart stuff
}

2. A new use case presents itself a couple years down the road where it would be advantageous to start delineating between guest and member shopping cart objects. The team elects to go with a polymorphic Cart object instead of a "monomorphic" Cart. They update their model(s).

[JsonDerivedType(typeof(GuestCart), "guestCart")]
[JsonDerivedType(typeof(MemberCart), "memberCart")]
[JsonPolymorphic(TypeDiscriminatorPropertyName="cartType")]
// this is abstract because moving forward there is no such thing as a cart which is not a member or guest cart
public abstract record Cart {
  public string CartId { get; init; }
  // ... other cart stuff
}

public record MemberCart : Cart {
  // ... member-cart-specific stuff
}

public record GuestCart : Cart {
  // ... guest-cart-specific stuff
}

Now we run in to a problem. Yes, new cart objects will serialize and deserialize correctly, but there is no direct way to enable our "old" (non polymorphic) cart JSON to be deserialized correctly by our runtime. It is going to throw a JsonException due to no type discriminator being present. We have to either migrate all of our data and add type discriminators (a risky proposition in production) or write a custom converter any time we want to go from monomorphic to polymorphic.

Evolving types are a common/expected occurrence no?

Of course a custom converter can do the trick - but it seems like a common enough occurrence that maybe this ought to be a built in functionality.

[eiriktsarpalis]

That scenario would still work if you didn't mark the base Cart type as abstract, right?

[boylec]

I think that is correct; making Cart a non-abstract type in this scenario would enable the object to deserialize to Cart, but that would not be a correct state in this particular domain. Obviously this is a contrived example, but it seems suboptimal for lack of a better word to force people to make things non abstract due to limitations of serialization framework - that would otherwise be abstract.

[eiriktsarpalis]

but that would not be a correct state in this particular domain.

It better reflects the shape of the data being stored in your database though. If this creates problems with your domain modelling I would suggest extracting that concern to a separate DTO layer.

due to limitations of serialization framework

What can be said of serialization libraries in general is that there exists a threshold of customizability after which maintainability and performance begin to suffer. We avoid adding new knobs unless there is demonstrable impact to a large number of users. The built-in polymorphism implementation is an opinionated implementation (to an extent, this is influenced by security concerns ever present in polymorphic serialization) that caters to the 80% of use cases. We fully expect that this won't satisfy every use case, and JsonConverter provides an escape hatch for everything else.

[boylec]

but that would not be a correct state in this particular domain.

It better reflects the shape of the data being stored in your database though. If this creates problems with your domain modelling I would suggest extracting that concern to a separate DTO layer.

due to limitations of serialization framework

What can be said of serialization libraries in general is that there exists a threshold of customizability after which maintainability and performance begin to suffer. We avoid adding new knobs unless there is demonstrable impact to a large number of users. The built-in polymorphism implementation is an opinionated implementation (to an extent, this is influenced by security concerns ever present in polymorphic serialization) that caters to the 80% of use cases. We fully expect that this won't satisfy every use case, and JsonConverter provides an escape hatch for everything else.

Everything you said makes sense and I appreciate the thoughtful response. 🙏

At the end of the day, as you mentioned, I/we can indeed accomplish what I'm mentioning here with some extra code.

My subjective view is that there is a good chance that > 80% of projects that use System.Text.Json are going to have models that evolve from a concrete monomorphic type to an abstract (polymorphic) type over time. I take your point that there is a delicate line drawn in the sand about how much to build in to the lib and how much to leave to the JsonConverter escape hatch.

That being said: I am an avid user of serialization libs, and I do wish this were a built in feature! :)

Thank you for hearing my two cents. You guys are doing great. Feel free to close.

[eiriktsarpalis]

We can leave this in the back burner for now. Assuming it receives added traction (responses, upvotes, etc.) we could try taking a look at this in the future.