Skip to content

Allow to specify type to serialize in SystemTextJsonOutputFormatter #41399

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

Open
1 task done
ArturDorochowicz opened this issue Apr 27, 2022 · 4 comments
Open
1 task done
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Milestone

Comments

@ArturDorochowicz
Copy link

ArturDorochowicz commented Apr 27, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

I'm trying to specify what type the SystemTextJsonOutputFormatter should serialize, but it's not possible, because it goes out of its way to use the actual type of the value.

Let's consider code like this:

class Parent {}
class Child : Parent {
  public string Prop { get; set; }
}

[ApiController]
class Ctrl : ControllerBase {
  [HttpGet]
  public Parent Get1() {
    return new Child();
  }

  [HttpGet]
  public ActionResult<Parent> Get2()
  {
    return new Child();
  }

  [HttpGet]
  public IActionResult Get3() {
    // getting desperate ...
    var ok = Ok(new Child());
    ok.DeclaredType = typeof(Parent);
    return ok;
  }
}

In all cases above, the value is serialized as Child and not Parent. In fact there seems to be no way to serialize it as Parent.

This is because of code in SystemTextJsonOutputFormatter

// context.ObjectType reflects the declared model type when specified.
// For polymorphic scenarios where the user declares a return type, but returns a derived type,
// we want to serialize all the properties on the derived type. This keeps parity with
// the behavior you get when the user does not declare the return type and with Json.Net at least at the top level.
var objectType = context.Object?.GetType() ?? context.ObjectType ?? typeof(object);

I understand the sentiment. As the comment says, you want to be compatible with NewtonsoftJson at least at the root level. But I think it goes too far and hope that there can be a way added to specify the type for serialization.

I think the comment: "context.ObjectType reflects the declared model type when specified" is inaccurate. In case of simple return type (Parent Get() {}), the ObjectType property is in fact set to the runtime type of the instance due to code in OutputResultExecutor:

var objectType = result.DeclaredType;
if (objectType == null || objectType == typeof(object))
{
objectType = result.Value?.GetType();
}

Describe the solution you'd like

I understand that there's slim chance you'll change the existing behavior, because it will be a breaking change. But anyway, I'd like the cases where ObjectResult specifies the DeclaredType to pass that type to the STJ serializer. That means that in the example code above, second and third cases should serialize the value as Parent and not Child.

Additional context

I stumbled upon this problem when analyzing some code that directly returns EF entities out of MVC actions. The entity used virtual property for lazy loading support. The property was decorated with JsonIgnore attribute, but it didn't work.

The analysis showed that Mvc passes the EF-generated proxy type to STJ and STJ doesn't read inherited attributes (GetCustomAttributes(..., inherit: false). The end result is that lazy loaded property was getting serialized while the intention was to ignore it.

@javiercn javiercn 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 Apr 27, 2022
@captainsafia
Copy link
Member

Thanks for filing this issue!

Triage: At the moment, we don't anticipate doing any work in this area unless more compelling scenarios arise. For this particular scenario,

I stumbled upon this problem when analyzing some code that directly returns EF entities out of MVC actions.

We recommend returning DTO from actions instead of EF models to avoid issues processing the EF-generated types.

We'll leave this issue open to see if more people are interested in a flag to manage this behavior.

@captainsafia captainsafia added this to the Backlog milestone May 3, 2022
@ghost
Copy link

ghost commented May 3, 2022

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@TonyTroeff
Copy link

This is a problem that my team has encountered this week as well. Specifying the DeclaredType makes no difference whatsoever and its existence becomes meaningless in this case.

@u-ashikov
Copy link

Currently I have migrated one of our projects to use the polymorphic type resolving from .NET 7. We are facing also the same issue as the type information from the produced/written model is missing but we actually need it. Digging into the code we realized that the DeclaredType is not used at all and the serializing object's type is used instead.

@captainsafia captainsafia added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

No branches or pull requests

5 participants