Repository navigation
[API Proposal]: Expose JsonConverter.CanHaveMetadata #118900
Description
Activity
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.Note that the
CanHaveMetadataproperty is simply an internal marker meant for validation; changing it value has no bearing on whether the underlying converter actually supports polymorphism.Today, user-defined converters do not support polymorphism because the publicly available
WriteandReadmethods do not accept serialization state parameters. Sharing serialization state is important in the context of polymorphism because you have two converters writing or reading from the same JSON object, so coordination becomes essential. Built-in converters work because they implement internal APIs to achieve this, however this capability is lost when these converters are wrapped inside a custom converter. There's an issue tracking support for making state-passing converter methods accessible to users, however we had the chance to prioritise this work so far.Our initial approach for this was to define a RampUpJsonConverter : JsonConverter that used a RNG to try strict serialization/deserialization options at a rate that wouldn't impact our SLA while emitting telemetry on failures.
Why not create two separate
JsonSerializerOptionsinstances with separate configurations and use an RNG to pick one of those? It sounds like it might achieve the same effect without faffing about with custom converters.I think the issue that @jamescourtney wants to avoid is having to change all the places where they currently cache a
JsonSerializerOptionsto store some otherJsonSerializerOptionsFactory(which may not even be effective if someone then tries to cache the option they get from it instead of getting a randomized option every time).Is it possible for us to just remove the polymorphism restriction during configuration time and make it throw when it's used in an unsupported way? In the mentioned use case, since you're just forwarding to an internal converter at the top level, the wrapping converter won't ever need to support the internal stack based calls. But if someone does try to use it to serialize a nested polymorphic type then we can throw at that point.
I think the issue that @jamescourtney wants to avoid is having to change all the places where they currently cache a
JsonSerializerOptionsto store some otherJsonSerializerOptionsFactory(which may not even be effective if someone then tries to cache the option they get from it instead of getting a randomized option every time).I still think that it's a less risky solution compared to injecting custom converters when previously built-in converters were being used. Because of how STJ works, changing from a built-in converter to a custom converter that wraps said built-in converter changes serialization behavior in subtle and multi-faceted ways. The
CanHaveMetadataissue being cited here is just the tip of the iceberg.
Background and motivation
My project is maintained by ~1500 developers with over 200 C# projects. We rely heavily on:
We have recently become aware of
RespectNullableAnnotationsandRespectRequiredConstructorParametersin STJ. Our codebase predates .NET 9. Since we do not have these flags turned on today, our nullable analysis is suspect since we cannot trust the annotations on types we deserialize with STJ.We would like to have a gradual migration to enable
RespectNullableAnnotationsandRespectRequiredConstructorParameters. Simply enabling them globally is not a good option as this is a production service. Further complicating this is that some of our dependencies such as MVC and CosmosDb simply want a singleJsonSerializerOptionswithout the ability to toggle per-request or per-JSON operation.Our initial approach for this was to define a
RampUpJsonConverter<T> : JsonConverter<T>that used a RNG to try strict serialization/deserialization options at a rate that wouldn't impact our SLA while emitting telemetry on failures. This is appealing because:However, it seems we are completely blocked by STJ with polymorphic types because
JsonConverter.CanHaveMetadatais an internal property that gives us no way to override. The exception in STJ is thrown from here.The current stack also does not permit any workarounds or other hacks. The current implementation of
CanHaveMetadatais just:internal virtual bool CanHaveMetadata => false;. This does not permit any reflection twiddling of a backing field or other workaround to unblock ourselves.API Proposal
The simplest change is to change
CanHaveMetadatafrominternaltopublic.API Usage
Alternative Designs
Alternatively, STJ could expose a new property on
JsonConverterFactorythat disables caching of the returned converter instances.This would allow a custom factory to implement a delegation pattern while owning caching itself internally. This doesn't work today because the output from JsonConverterFactory is always cached, which forces the RNG and strict/loose decision to be made in a custom
JsonConverter, which then hits the blocker aroundCanHaveMetadata.Risks
No response