AspNetCoreOData icon indicating copy to clipboard operation
AspNetCoreOData copied to clipboard

Incorrect output formatter selected to serialize response

Open engenb opened this issue 2 years ago • 7 comments

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 avatar Jan 28 '22 23:01 engenb

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 avatar Jan 29 '22 00:01 engenb

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!

engenb avatar Jan 29 '22 00:01 engenb

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

julealgon avatar Jan 29 '22 23:01 julealgon

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 avatar Jan 30 '22 00:01 engenb

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 avatar Feb 01 '22 01:02 engenb

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:

  • it works with the way I use OData (your milage may vary)

ways my solution is NOT ideal:

  • full replacement of the default AttributeRoutingConvention with only minor differences. it'd be nice if the methods of this class were virtual so only the necessary parts could be overridden
  • it breaks easily. either need to clear all route conventions from ODataOptions or name methods so as not to match any registered conventions
    • food for thought: if I opt out of conventions in favor of attribute routing, all I'd need is a way to flag an action to be handled by the OData output formatter AND which EDM model the result is for. As a dev (and fan of OData) I wouldn't mind having to explicitly indicate which EDM model rather than OData having to implicitly figure it out. I'm aware of [ODataRouteComponent(...)] but I think this has its own limitations when tools like ApiVersioning are taken into account i.e. which EDM model to use based on the version. It'd be ugly, but I'd be perfectly happy with something like return Ok(results, "edm route component key") or if you don't want to require a controller method overload, return Ok(new SomeODataResultWrapper(result, "model key")) for the output formatter to pick up. this would give me, the developer, a lot of flexibility and control to inform OData of my intentions. Or, maybe there's something I'm missing entirely with API versioning strategies with OData 8, but I can't be the only one out there who has requirements to version their API surface.
  • ApiExplorer ODataQueryOptions<T> parameter expand/replace implementation "works" but it's pretty naive and doesn't make any attempt to inspect what query options are configured for the action's respective entity set

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.

engenb avatar Feb 03 '22 19:02 engenb

@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.

mlafleur avatar Mar 14 '22 01:03 mlafleur