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

[API Proposal]: ignore type discriminator ("$type" properties) in JSON model properties #82012

Closed
Sergio0694 opened this issue Feb 12, 2023 · 2 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json

Comments

@Sergio0694
Copy link
Contributor

Sergio0694 commented Feb 12, 2023

Background and motivation

As part of migrating the Microsoft Store to System.Text.Json, we have several cases where we cannot change the exact JSON schema being used, as it would cause older versions of the client to no longer work correctly. This means we have to structure some of our System.Text.Json-based models to account for some Newtonsoft.Json-specific quirks.

As an example, consider this JSON response:

{
    "A": {
        "$type": "System.Collections.Generic.Dictionary`2[[System.String, mscorlib],[System.String, mscorlib]], mscorlib",
        "Foo": "Bar",
        "Baz": "Blah"
    },
    "B": {
        "$type": "System.Collections.Generic.Dictionary`2[[System.String, mscorlib],[System.Collections.Generic.IList`1[[System.String, mscorlib]], mscorlib]], mscorlib",
        "Foo": [ "Bar", "Baz" ],
        "Blah": [ "Hello" ]
    }
}

Note those two $type properties. This is needed because older clients are deserializing this response with an unstructured approach, and those type discriminators are needed to ensure the deserialized objects have the correct type. If we move this to a System.Text.Json model though, we'd instead end up with something like this:

public sealed class SomeResponse
{
    public Dictionary<string, string>? A { get; set; }

    public Dictionary<string, List<string>>? B { get; set; }
}

This has several issues though:

  1. In the "A" case, the dictionary will end up having an additional (incorrect) "$type" key.
  2. In the "B" case, that will outright fail to deserialize, because the value of "$type" can't be converted to a List<string>.

One approach could be to manually write a custom converter for each of these properties, but that's rather verbose, error prone, and it just overall adds a lot of complexity for something that would be nice to just work "out of the box".

I don't have an exact API shape in mind to address this, so opening this more of a discussion, but would it be possible to consider adding some way (eg. some [JsonIgnoreTypeDiscriminator] attribute on the property) to instruct the serializer to ignore the "$type" property, if present, and have it not affect the deserialization of the actual property?

In case this is already possible and I just haven't figured out why, even better 😄

cc. @eiriktsarpalis

@Sergio0694 Sergio0694 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 12, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 12, 2023
@ghost
Copy link

ghost commented Feb 12, 2023

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

Background and motivation

As part of migrating the Microsoft Store to System.Text.Json, we have several cases where we cannot change the exact JSON schema being used, as it would cause older versions of the client to no longer work correctly. This means we have to structure some of our System.Text.Json-based models to account for some Newtonsoft.Json-specific quirks.

As an example, consider this JSON response:

{
    "A": {
        "$type": "System.Collections.Generic.Dictionary`2[[System.String, mscorlib],[System.String, mscorlib]], mscorlib",
        "Foo": "Bar",
        "Baz": "Blah"
    },
    "B": {
        "$type": "System.Collections.Generic.Dictionary`2[[System.String, mscorlib],[System.Collections.Generic.IList`1[[System.String, mscorlib]], mscorlib]], mscorlib",
        "Foo": [ "Bar", "Baz" ],
        "Blah": [ "Hello" ]
    }
}

Note those two $type properties. This is needed because older clients are deserializing this response with an unstructured approach, and those type discriminators are needed to ensure the deserialized objects have the correct type. If we move this to a System.Text.Json model though, we'd instead end up with something like this:

public sealed class SomeResponse
{
    public Dictionary<string, string>? A { get; set; }

    public Dictionary<string, List<string>>? B { get; set; }
}

This has several issues though:

  1. In the "A" case, the dictionary will end up having an additional (incorrect) "$type" key.
  2. In the "B" case, that will outright fail to deserialize, because the value of "$type" can't be converted to a List<string>.

One approach could be to manually write a custom converter for each of these properties, but that's rather verbose, error prone, and it just overall adds a lot of complexity for something that would be nice to just work "out of the box".

I don't have an exact API shape in mind to address this, so opening this more of a discussion, but would it be possible to consider adding some way (eg. some [JsonIgnoreTypeDiscriminator] attribute on the property) to instruct the serializer to ignore the "$type" property, if present, and have it not affect the deserialization of the actual property?

In case this is already possible and I just haven't figured out why, even better 😄

cc. @eiriktsarpalis

API Proposal

n/a

API Usage

n/a

Alternative Designs

No response

Risks

No response

Author: Sergio0694
Assignees: -
Labels:

api-suggestion, area-System.Text.Json

Milestone: -

@eiriktsarpalis
Copy link
Member

This appears to be a duplicate of #79059, I think it might be solvable using something like this #79059 (comment)

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Feb 14, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json
Projects
None yet
Development

No branches or pull requests

2 participants