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

SystemTextJsonOutputFormatter calls the serializer with unexpected Type #44852

Closed
1 task done
akurone opened this issue Nov 2, 2022 · 8 comments · Fixed by #46008
Closed
1 task done

SystemTextJsonOutputFormatter calls the serializer with unexpected Type #44852

akurone opened this issue Nov 2, 2022 · 8 comments · Fixed by #46008
Assignees
Labels
old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels
Milestone

Comments

@akurone
Copy link

akurone commented Nov 2, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

hi there,
after a short talk with lovely people on runtime team (#dotnet/runtime/issues/77532); i think the issue below needs to be resolved here:
i want to utilize the new json polymorphism features:

[JsonPolymorphic]
[JsonDerivedType(typeof(TheDerivedType), nameof(TheDerivedType))]
public class TheBaseType { public int Prop1 { get; set; } = 1; }

public class TheDerivedType: TheBaseType { public int Prop2 { get; set; } = 2; }

//and in the controller:
[HttpGet] public TheBaseType A() => new TheDerivedType();
// call to this endpoint produces:
// {"prop2":2,"prop1":1}

what i expect to get is:

{"$type":"TheDerivedType","prop2":2,"prop1":1}

but as discussed in #41399 SystemTextJsonOutputFormatter calls the serializer in a very specific way. this call practically hides all the new metadata from the serializer and produces json without discriminator info. without discriminator, the receiving party fails to deserialize this json.

Expected Behavior

SystemTextJsonOutputFormatter and other json returning types (like JO result etc.) rendering with STJ should respect the polymorphic serialization metadata provided.

current:

{"prop2":2,"prop1":1}

expected:

{"$type":"TheDerivedType","prop2":2,"prop1":1}

Steps To Reproduce

clone the repo and make the get reqs below:

  • test/a
  • test/b

Exceptions (if any)

No response

.NET Version

7.0.100-rc.2.22477.23

Anything else?

No response

@TanayParikh TanayParikh added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Nov 2, 2022
@rafikiassumani-msft rafikiassumani-msft added this to the .NET 8 Planning milestone Nov 3, 2022
@ghost
Copy link

ghost commented Nov 3, 2022

Thanks for contacting us.
We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@marcelbarner
Copy link

marcelbarner commented Nov 22, 2022

I had the same problem and I created a repo/package to solve it.

Repo: marcelbarner/AspNetCore.Json.SinglePolymorphicResponse

Package: AspNetCore.Json.SinglePolymorphicResponse

@stephenwelsh
Copy link

stephenwelsh commented Dec 7, 2022

This looks incorrect to me, different output depending if I use the Generic form or not:

var test = new TestA{ Id="123", Name="TestA"}
var nonGenericCall = JsonSerializer.Serialize(test, test.GetType());
var genericCall = JsonSerializer.Serialize<TestBase>(test);

[JsonDerivedType(typeof(TestBase), typeDiscriminator: nameof(TestBase))]
[JsonDerivedType(typeof(TestA), typeDiscriminator: nameof(TestA))]
public class TestBase
{
    public string Id { get; set; }
}
public class TestA : TestBase
{
    public string Name { get; set; }
}

I get the following:

nonGenericCall = "{"id"="123","name"="TestA"}"
genericCall = "{$type="TestA", "id"="123", "name"="TestA"}"

*** Update ***

Looks like my bad, specifying base class as type argument works:
var nonGenericCallWithBase = JsonSerializer.Serialize(test, typeof(TestBase));
nonGenericCallWithBase = "{$type="TestA", "id"="123", "name"="TestA"}"

@stephenwelsh
Copy link

stephenwelsh commented Dec 7, 2022

In my scenario (bi directional polymorphic events using SignalR with .Net client) it is not possible to specify a base class and normally a custom JsonConverter is used to manually add the type discriminators.
Looking to use the new Net7 support for polymorphic types exposed this differentiated behaviour when in some SignalR calling directions the base type is not known.

After further research I found that duplicating the JsonDerivedType attribute on the corresponding Derived class enables the non generic call to JsonSerializer.Serialize to add the type discriminator.

This restriction and work around may be worth adding to the Serialize properties of derived classes documentation.

var test = new TestA{ Id="123", Name="TestA"}
var nonGenericCall = JsonSerializer.Serialize(test, test.GetType());
var genericCall = JsonSerializer.Serialize<TestBase>(test);

[JsonDerivedType(typeof(TestBase), typeDiscriminator: nameof(TestBase))]
[JsonDerivedType(typeof(TestA), typeDiscriminator: nameof(TestA))]
public class TestBase
{
    public string Id { get; set; }
}
[JsonDerivedType(typeof(TestA), typeDiscriminator: nameof(TestA))]
public class TestA : TestBase
{
    public string Name { get; set; }
}

I now get the desired output as follows:

nonGenericCall = "{$type="TestA", "id"="123", "name"="TestA"}"
genericCall = "{$type="TestA", "id"="123", "name"="TestA", $type="TestA"}"

@brunolins16
Copy link
Member

This restriction and work around may be worth adding to the Serialize properties of derived classes documentation.

cc @dotnet/area-system-text-json

@eiriktsarpalis
Copy link
Member

After further research I found that duplicating the JsonDerivedType attribute on the corresponding Derived class enables the non generic call to JsonSerializer.Serialize to add the type discriminator.

This restriction and work around may be worth adding to the Serialize properties of derived classes documentation.

cc @dotnet/area-system-text-json

I don't think this will be necessary once #46008 has been merged, no?

@brunolins16
Copy link
Member

I don't think this will be necessary once #46008 has been merged, no?

Exactly, we are adding additional code to fix this behavior in ASP.NET Core. However, I believe the question is about how the Serializer polymorphism work in general, @stephenwelsh is that right?

@eiriktsarpalis
Copy link
Member

I see. @stephenwelsh you might want to check dotnet/runtime#77532 which tracks this issue in the STJ repo, in particular I've included a workaround that lets you achieve the semantics you desire without the need to duplicate any annotations.

@brunolins16 brunolins16 moved this to In Progress in [.NET 8] Web Frameworks Jan 26, 2023
@brunolins16 brunolins16 moved this from In Progress to Done in [.NET 8] Web Frameworks Feb 1, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants