OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
453 stars 160 forks source link

Incorrect output formatter selected to serialize response #462

Open engenb opened 2 years ago

engenb commented 2 years ago

Hello,

I'm working through a POC with the new 8.x framework in order to update a bunch of microservices I maintain. My goal is to get all of these technologies (among others) working together:

Microsoft.AspNetCore.OData AutoMapper.AspNetCore.OData.EFCore Microsoft.AspNetCore.Identity.EntityFrameworkCore Microsoft.AspNetCore.Mvc.Versioning.ApiExplorer

It looks like you've made things a lot simpler working more with the new AspNetCore routing framework instead of alongside it, so thanks for that!

I'm having a problem with how my responses are serialized. I can see in the log that the wrong output formatter is chosen to handle my response:

[17:06:30 DBG] Selected output formatter 'Microsoft.AspNetCore.Mvc.Formatters.SystemTextJsonOutputFormatter' and content type 'application/json' to write the response.

Per my understanding, this should be using the Microsoft.AspNetCore.OData.Formatter.ODataOutputFormatter and the content type should be application/json;odata.metadata=minimal;odata.streaming=true

So far, I've verified that the OdataOutputFormatter is definitely getting registered. I can see this in my log as well:

[17:06:30 DBG] List of registered output formatters, in the following order: ["Microsoft.AspNetCore.OData.Formatter.ODataOutputFormatter", "Microsoft.AspNetCore.OData.Formatter.ODataOutputFormatter", "Microsoft.AspNetCore.OData.Formatter.ODataOutputFormatter", "Microsoft.AspNetCore.Mvc.Formatters.HttpNoContentOutputFormatter", "Microsoft.AspNetCore.Mvc.Formatters.StringOutputFormatter", "Microsoft.AspNetCore.Mvc.Formatters.StreamOutputFormatter", "Microsoft.AspNetCore.Mvc.Formatters.SystemTextJsonOutputFormatter"]

I'm assuming that I'm doing something incorrect or unexpected. Here's what my controller looks like:

[ApiController]
[Route("sample/v{version:apiVersion}")]
public class UsersController : ControllerBase
{
    private ODataDbContext DbContext { get; }
    private IMapper Mapper { get; }

    public UsersController(ODataDbContext dbContext, IMapper mapper)
    {
        DbContext = dbContext;
        Mapper = mapper;
    }

    [ApiVersion("1")]
    [HttpGet("Users")]
    [ODataRouteComponent("v1")]
    [ProducesResponseType(typeof(IEnumerable<Model.V1.User>), Status200OK)]
    public async Task<IActionResult> GetV1(ODataQueryOptions<Model.V1.User> queryOptions) =>
        Ok(await DbContext.Users
            .AsNoTracking()
            .GetAsync(Mapper, queryOptions, (QuerySettings)null));

    [ApiVersion("2")]
    [HttpGet("Users")]
    [ODataRouteComponent("v2")]
    [ProducesResponseType(typeof(IEnumerable<Model.V2.User>), Status200OK)]
    public async Task<IActionResult> GetV2(ODataQueryOptions<Model.V2.User> queryOptions) =>
        Ok(await DbContext.Users
            .AsNoTracking()
            .GetAsync(Mapper, queryOptions, (QuerySettings)null));
}

Everything more or less works, but my resulting json is just an array of data instead of the standard object with @odata.{stuff} and value properties. I've been comparing my approach to those in the samples and the various blog posts, but not sure what I'm missing. If more source code or context is needed, it's no problem to publish a quick github repo for this. Any help would be appreciated, thanks!

engenb commented 2 years ago

Quick update - I just added app.UseODataRouteDebug(); and can confirm that my endpoints aren't being mapped as odata endpoints. I'm experimenting with different options, but I'd be grateful for any insight as to why this is happening - thanks!

engenb commented 2 years ago

To follow up with where I've landed:

It seems that the odata framework is only mapping my controller endpoints if they follow odata routing conventions. I thought this wasn't necessary and I could use normal attribute routing, but it only seems to work if I rename my controller method to .Get(...) this means I have to separate my v1 and v2 into separate controllers which is more in line what what the examples have shown.

yet another versioning use case - if I added a V3 to this project but the version bump was due to some change elsewhere in the api, meaning there was no change to my UsersController(s), I'd want to indicate that my V2.UsersController.Get(...) endpoint was compatible with both versions 2 and 3. I know the api versioning framework can do this with normal aspnetcore routing, but I'm not sure how to go about this with odata.

I effectively tried this:

[ApiVersion("2")]
[ApiVersion("3")]
[HttpGet("Users")]
[ODataRouteComponent("sample/v3")]
[ProducesResponseType(typeof(IEnumerable<Model.V3.User>), Status200OK)]
public async Task<IActionResult> Get(ODataQueryOptions<Model.V3.User> queryOptions) =>
    Ok(await DbContext.Users
        .AsNoTracking()
        .GetAsync(Mapper, queryOptions, (QuerySettings)null));

this results in only sample/v3/Users mapped to V2.UsersController.Get (no sample/v2). If I remove the ODataRouteComponent attribute, all versions get mapped. if I add two ODataRouteComponent attributes to this method, one for sample/v2 and one for sample/v3 only one of the two routes will be registered, depending on which attribute is first.

Reading this blog article, I'm curious to learn more about the mentioned config for versioning:

services.AddControllers()
    .AddOData(opt => opt.AddRouteComponents("v{version}", edmModel));

it's unclear to me how this would work if I have a different edm model for different versions as my api changes over time.

Thanks again!

julealgon commented 2 years ago

Can you change your controller base class from ControllerBase to ODataController and see how that affects the route definitions?

engenb commented 2 years ago

thanks for the suggestion, but using ODataController doesn't result in any different result.

the "issue" is right here, I think.

on that line of code, my routeTemplate is "sample/v{version:apiVersion}/Users" and prefix is "sample/v1" or "sample/v2" as it loops through the foreach so it fails the condition and therefore determines the endpoints are non-odata.

After finding this, I'm expecting I need to make my own IODataControllerActionConvention that works with the API versioning framework, similar to what's outlined in this blog post. I need something to resolve the effective route, taking into account the ApiVersion attributes. All the info needed is there in the ActionModel so I don't expect it to be too bad.

I don't know if the team that maintains Microsoft.AspNetCore.Odata.Versioning is planning a release for odata 8 - none of the release branches (or main) have seen any activity for a while now and I can't wait if I'm going to move forward with odata 8. my APIs already have requirements to support some of the API versioning scenarios I described above. I don't want to have to copy/paste all of my controllers and then maintain those copies going forward just to support versioning with odata 8, but I absolutely need odata 8 because it resolves some other critical performance issues we're experiencing with odata 7 and it's counterpart version of AutoMapper.AspNetCore.OData.EFCore

engenb commented 2 years ago

if anyone is wanting this same capability, I've put together a project that will hopefully be helpful to others. pretty simple adaptation of the AttributeRoutingConvention to expand out the action into its multiple supported versioned routes before testing if any of them are supported by a registered EDM route component. I wouldn't call this production-ready yet, but hopefully, this can give others a head-start - enjoy!

https://github.com/engenb/AspNetCoreOData.ApiVersioning

engenb commented 2 years ago

I've updated my repo (linked above) to a "working state" with API versioning and a brute-force example for ApiExplorer/swagger support that I may improve later if I have time.

ways my solution is ideal:

are there any plans to provide "official" solutions for these features with odata 8? curious if there are any plans to update the api versioning project for odata 8. my bain-aid can get me by for now, but I think a more robust solution built by people with a deeper understanding of the framework would be ideal.

mlafleur commented 2 years ago

@engenb , thank you for sharing this! It will save me a lot of investigation time pulling together a solution.

I completely agree with you on the private and non-virtual methods. More generous use of protected/virtual methods across the library would save a lot of unnecessary code duplication.