dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.48k stars 10.04k forks source link

Type disciminator is missing with OkResult #58832

Open vanillajonathan opened 1 week ago

vanillajonathan commented 1 week ago

Is there an existing issue for this?

Describe the bug

I am using polymorphic JSON which works when returning the class but not when returning Ok(data) because then the $type type discriminator is missing.

Works:

[HttpGet(Name = "Pet")]
public ActionResult<BaseModel> Get()
{
    return new Cat() { CatName = "Mizzy" };
}

Does not work:

[HttpGet(Name = "Pet")]
public ActionResult<BaseModel> Get()
{
    return Ok(new Cat() { CatName = "Mizzy" });
}

Also does not work:

[HttpGet(Name = "Pet")]
public ActionResult<BaseModel> Get()
{
    return Ok(new Cat() { CatName = "Mizzy" } as BaseModel);
}

Expected Behavior

When using the Ok method to return a OkObjectResult it should output a type discriminator.

Steps To Reproduce

using Microsoft.AspNetCore.Mvc;
using System.Text.Json.Serialization;

namespace WebApplication1.Controllers
{
    [ApiController]
    [Route("[controller]")]
    public class ExampleController : ControllerBase
    {
        [HttpGet(Name = "GetPet")]
        public ActionResult<BaseModel> Get()
        {
            return Ok(new Cat() { CatName = "Mizzy" } as BaseModel);
        }
    }

    [JsonPolymorphic]
    [JsonDerivedType(typeof(Cat), "cat")]
    [JsonDerivedType(typeof(Dog), "dog")]
    public class BaseModel
    {
        public string Name { get; set; }
    }

    public class Cat : BaseModel
    {
        public string CatName { get; set; }
    }

    class Dog : BaseModel
    {
        public string DogName { get; set; }
    }
}

Exceptions (if any)

No response

.NET Version

8.0.10

Anything else?

No response

mikekistler commented 5 days ago

Thank you for filing this issue and providing clear and complete information on how to reproduce it. I have reproduced it and done some investigation. I believe that this is happening because when the result value is wrapped in "Ok" the type information is lost / not conveyed to System.Text.Json, which treats is as just a regular object rather than a Cat.

I will continue investigating and post here when I have something concrete.

mikekistler commented 1 day ago

I've spoken with the engineering team and learned a bit more. In particular, there seem to be a couple workarounds for this problem. The first is to explicitly set the DeclaredType on the ObjectResult returned from Ok, like this:

     [HttpGet("fido")]
    public ActionResult<BaseModel> GetFido()
    {
        var result = Ok(new Dog() { DogName = "Fido" });
        result.DeclaredType = typeof(BaseModel);
        return result;
    }

Another workaround is to change the return type of the method to an IResult and use TypedResults to generate the response, like this:

    [HttpGet("snoopy")]
    public Ok<BaseModel> GetSnoopy()
    {
        return TypedResults.Ok(new Dog() { DogName = "Snoopy" } as BaseModel);
    }

Both of these approaches generate a response body with the $type property.

Would one of these approaches work for you?

vanillajonathan commented 1 day ago

None of them are pretty, it would be great if it "just worked" without having to use any workarounds. The workarounds are not intuitive if users have to talk to the engineering team to figure it out.

But until then I could do:

[HttpGet("mizzy")]
public ActionResult<BaseModel> GetMizzy()
{
    var pet = new Cat() { CatName = "Mizzy" };
    return Ok(pet) { DeclaredType = typeof(BaseModel) };
}

Maybe the Ok method could have generic overload so I could do return Ok<Cat>(pet);.

BrennanConroy commented 1 day ago

The root cause is that the T from ActionResult<T> isn't being used when returning an ActionResult (non-T), Ok is an ActionResult.

The following code is where we convert the ActionResult<T> into an IActionResult for processing, and you'll notice that the first thing it does it just return the ActionResult directly if it has one, otherwise it'll create an ObjectResult and use typeof(TValue) for the DeclaredType field. https://github.com/dotnet/aspnetcore/blob/1ccdec2e037477e1ec3ff8bc3e1b9f1b686a9d0a/src/Mvc/Mvc.Core/src/ActionResultOfT.cs#L78

So in the case of returning an ActionResult, the T isn't being used anywhere which means when we write the Json all we see if object.GetType() which will be Cat or Dog so will be written as a non-polymorphic object.