aspnet-api-versioning icon indicating copy to clipboard operation
aspnet-api-versioning copied to clipboard

ODataQueryOptions and Swagger UI

Open Liero opened this issue 3 years ago • 15 comments

Related to #599

If I use ODataQueryOptions as an action parameter, the swagger doc gets very verbose:

image

[EnableQuery]
[HttpGet]
public IQueryable<Model> Get(ODataQueryOptions<Model> options) {}
app.UseEndpoints(endpoints =>
{
    endpoints.MapControllers().RequireAuthorization();
    endpoints.EnableDependencyInjection();
    endpoints.Select().Filter().OrderBy().MaxTop(100).Count();
});
<PackageReference Include="Microsoft.AspNetCore.OData" Version="7.5.7" />
<PackageReference Include="Swashbuckle.AspNetCore" Version="6.1.2" />

Liero avatar Apr 20 '21 08:04 Liero

Same here. I've been trying to solve this for weeks.

Also, all OData properties appear in the "options" parameter, slowing the page rendering: image

[HttpGet]
public async IQueryable<Page> Get(ODataQueryOptions<Page> options) {}
app.UseEndpoints(endpoints =>
{
    endpoints.MapControllers().RequireAuthorization();
    endpoints.EnableDependencyInjection();
    endpoints.Select().OrderBy().Filter();
});
<PackageReference Include="Microsoft.AspNetCore.OData" Version="7.5.7" />
<PackageReference Include="Swashbuckle.AspNetCore.Annotations" Version="6.0.7" />
<PackageReference Include="Swashbuckle.AspNetCore.Swagger" Version="6.0.7" />
<PackageReference Include="Swashbuckle.AspNetCore.SwaggerGen" Version="6.0.7" />
<PackageReference Include="Swashbuckle.AspNetCore.SwaggerUI" Version="6.0.7" />

davidebriscese avatar Apr 20 '21 08:04 davidebriscese

I made a minimal repro of the problem: ODataIssue559. Same problem as 559 currently closed.

davidebriscese avatar Apr 20 '21 09:04 davidebriscese

@davidebriscese Thanks for the repro; it really helps.

It took less time that I thought; I missed something obvious. I noticed that both of you are trying to use part of OData. I see you're using ODataOptions without actually using an ODataController or - more specifically - ODataRouting. In the current state of the universe, I'll just come out and say - that's not supported. That might be something that could be supported in the future. I feel like at least one other person has asked for this in the past.

There's a bunch of assumptions and searching for things related to OData. I expect that most, if not all, of that will fall down if things are only partially wired up. The OData libraries offer virtually nothing in the API Explorer space so getting anything to work is a lot of work. You might, just might, get what you want though the query options conventions builder as is or, perhaps, with a few modifications. It's uncharted water.

There was a myriad of issues in the repo that prevented it from working (in no particular order):

  • No ODataController or [ODataRouting]
  • Entities did have a key (ex: Id)
  • Response didn't include entities
  • You need services.AddODataApiExplorer not just services.AddVersionedApiExplorer (which is where query option support comes from)
  • You need to configure one or more entity models
  • You need to map an OData route

I'm aware that it is possible (and sometimes arguably preferable) to use vanilla routing with parts of OData using only query options with query composition. That should even work with API Versioning. Making that particular combination work with the API Explorer too - not so much; at least, not yet.

For your convenience, here's the working example echoed back with the required changes: 559-739-Working.zip

I hope that helps.

commonsensesoftware avatar Jun 24 '21 07:06 commonsensesoftware

@davidebriscese Thanks for the repro; it really helps.

It took less time that I thought; I missed something obvious. I noticed that both of you are trying to use part of OData. I see you're using ODataOptions without actually using an ODataController or - more specifically - ODataRouting. In the current state of the universe, I'll just come out and say - that's not supported. That might be something that could be supported in the future. I feel like at least one other person has asked for this in the past.

There's a bunch of assumptions and searching for things related to OData. I expect that most, if not all, of that will fall down if things are only partially wired up. The OData libraries offer virtually nothing in the API Explorer space so getting anything to work is a lot of work. You might, just might, get what you want though the query options conventions builder as is or, perhaps, with a few modifications. It's uncharted water.

There was a myriad of issues in the repo that prevented it from working (in no particular order):

  • No ODataController or [ODataRouting]
  • Entities did have a key (ex: Id)
  • Response didn't include entities
  • You need services.AddODataApiExplorer not just services.AddVersionedApiExplorer (which is where query option support comes from)
  • You need to configure one or more entity models
  • You need to map an OData route

I'm aware that it is possible (and sometimes arguably preferable) to use vanilla routing with parts of OData using only query options with query composition. That should even work with API Versioning. Making that particular combination work with the API Explorer too - not so much; at least, not yet.

For your convenience, here's the working example echoed back with the required changes: 559-739-Working.zip

I hope that helps.

Any ideas how to get this working with version 8.0.x of Microsoft.AspNetCore.OData? That "upgrade" has been impossible for me to reconcile.

HuntJason avatar Aug 10 '21 14:08 HuntJason

@HuntJason it simply not possible - currently. OData 8.0+ is not yet supported. There's a much longer discussion about this in #677. Unfortunately, the OData team has pulled the rug out from under me again and it's effectively a pretty significant rewrite of a number of pieces - unfortunately. They have a team, but I'm just one guy. Despite my efforts to coordinate more smoothly with them, there seems to be no formal effort or desire to align. I am getting close to burning down the backlog of open issues. Once that is done, I'll be able to focus on lighting up support for 8.0. Thanks for your patience.

commonsensesoftware avatar Aug 10 '21 17:08 commonsensesoftware

Its disappointing that the OData team does not take into consideration the Versioning and Swashbuckle (Swagger/OpenAPI) teams. It would seem they are linked technologies and deserve coordinated releases, samples, and documentation for us plebs to follow along. Versioning, OData, and OpenAPI spec would seem to me to be fairly standard things to be considered all together or, at the least, coordinated.

I'm not given much hope that this relationship will change either. The "The Future of OData" section at the bottom of Hassan Habib's article Up & Running w/ OData in ASP.NET 6 leads me to believe this is just a train on the tracks without consideration for these other, reliant technologies.

If there's anything I could do to help, please let me know.

HuntJason avatar Aug 10 '21 17:08 HuntJason

@HuntJason indeed, it's unfortunate. I know some of the contacts, but discussions and feedback seem to go in one ear and out the other. I guess some communication channel is better than none. As soon as there is something feasible for 8.0, there will be an announcement. Thanks again for your patience.

commonsensesoftware avatar Aug 10 '21 17:08 commonsensesoftware

First off, thank you so much for the 559-739.zip sample. That cleared up a bunch of things for me!!

I can only seem to get the Select, Filter, OrderBy, and Count to show up in Swagger in the sample. What I need, in addition, is the Top and Skip (for pagination). Any ideas how to get that showing?

HuntJason avatar Aug 13 '21 15:08 HuntJason

First off, thank you so much for the 559-739.zip sample. That cleared up a bunch of things for me!!

I can only seem to get the Select, Filter, OrderBy, and Count to show up in Swagger in the sample. What I need, in addition, is the Top and Skip (for pagination). Any ideas how to get that showing?

Figured this from OData Query Options documentation.

HuntJason avatar Aug 13 '21 16:08 HuntJason

Has anyone been able to figure out a workaround? This generates a crazily large schema file that takes so long for me to import into tools like Insomina, etc. I tried manually removing a lot of schemas based on the class name containing "Odata", but I mean it pulls in HttpContext and everything else.

sjd2021 avatar Nov 16 '21 17:11 sjd2021

@sjd2021 as has now be flushed out (I think), I suspect you are using only part of OData. While it is possible for that to work from a pure API perspective, it doesn't [currently] work for the API Explorer, which is already not intrinsically supported by OData. This happens because OData adds many services (ex: model binders, etc) and other options that the API Explorer extensions use to explore your API. Much of that is highly dependent (as should be expected) on OData's routing, conventions, etc. While it is possible to create an API that deviates from that, the API Explorer extensions for API Versioning simply doesn't understand that. Unfortunately, the only feasible alternative is to use all of OData or build up the documentation yourself.

If you have a repo, I can probably help confirm the behavior if you're unsure.

commonsensesoftware avatar Nov 16 '21 18:11 commonsensesoftware

public async Task<ActionResult<PagedResult<Things>>> GetThings(ODataQueryOptions<Things> options)

That's accurate. Above is the signature of my list-based endpoints for which odata works amazingly, and this is the only place I reference it aside from applying the filter. I only use the filter, order, skip, and top parameters so I'm able to add those as query string parameters in a single OperationFilter class so everything works seamlessly in all of the many tools we use which leverage the generated Open API doc. I keep experimenting with schema filters, etc. but can't get the right combination to rip out everything that this one class seems to be adding. It's not your problem to solve though. This makes sense to me.

sjd2021 avatar Nov 16 '21 18:11 sjd2021

@sjd2021 you got it. Essentially, ODataQueryOptions is no longer seen as special and the type is treated as a normal model. The default behavior is to treat such models as coming from the query string. A complex object can be used in the query string, but it can be quite ugly; especially for something like this. Some users have indicated that it causes an explosion of 1600+ parameters. This is the built-in API Explorer expressing the entire ODataQueryOptions object graph. 😬

I don't have a simple solution, but this is a common enough scenario that I'd like to be able to address as a supported use case.

commonsensesoftware avatar Nov 16 '21 19:11 commonsensesoftware

Any update on this?

Criperum avatar Mar 03 '22 14:03 Criperum

@Criperum which update are you looking for? If you're using part of the OData stack and trying combine it with the API Explorer, it will not work - today. I'm putting together a roadmap for the next major release and supporting OData query options in this manner will be added as an enhancement. Admittedly, this will be one of the lower priority items and I'm not even entirely sure how feasible it is to implement - yet. Several people have asked for this feature, so it's something I want to explore.

If you're using the full OData stack with the API Explorer and are still encountering this issue, then a repro will be helpful. I've been unable to recreate the scenario under those conditions.

commonsensesoftware avatar Mar 03 '22 15:03 commonsensesoftware

This thread has gone dark. Thank you for all the feedback and discussion. 6.0 has been officially released and contains support for using only some of OData. The some OData OpenAPI example demonstrates everything working together with Swashbuckle.

If something else has been missed, I'm more than willing to continue the discussion or reopen the issue.

commonsensesoftware avatar Aug 24 '22 01:08 commonsensesoftware

Literally WHY isn't the OData team working to ensure that their product works with OpenAPI?

https://github.com/OData/AspNetCoreOData/issues/757

HuntJason avatar Nov 30 '22 19:11 HuntJason

@HuntJason I wish I could give you an answer. I've asked the same question many times. There are several (unofficial) reasons I can speculate on:

  1. It's not a priority (given team size, capacity, and workload)
  2. EDMX and the $metadata endpoint has been the way of documenting and generating clients years before Swagger/OpenAPI
  3. There is a built-in EDM-to-Swagger converter, but it only scratches at what could/should be there
  4. There is a new, experimental OData library that attempts to more closely pair with OpenAPI a. I haven't used it so I don't know all the gaps b. I do know it expects and requires an EDM to produce anything (which is a dead stick for those using partial OData).

If there is a specific scenario related to OData + API Versioning + OpenAPI, I'm happy to help or provide some guidance. The whole reason I created this functionality is because it doesn't exist in OData. I've even publicly stated in their repo that I would consider taking on and driving an effort to extract the generic, non-versioned aspects of the support I have into something for all of OData. I won't even move an inch on that until there is a commitment from the team. So far it's been nothing but 🦗 🦗 🦗 on that topic.

commonsensesoftware avatar Nov 30 '22 19:11 commonsensesoftware

@commonsensesoftware Thank you!

The thread I had before contains the example replicates the issue I am having:

public IActionResult Get(ODataQueryOptions<Account> queryOptions)

When you run their ODataRoutingSample and open the swagger page, you cannot execute the Account's get method from the SwaggerUI.

HuntJason avatar Nov 30 '22 20:11 HuntJason

I can't speak to what the OData team's examples do or don't support. If you want to see it working with API Versioning and OData query options in OpenAPI, then you can follow one of the provided E2E examples:

  • OData OpenAPI - full OData stack with OpenAPI
  • Partial OData OpenAPI - vanilla API with partial OData query support and OpenAPI

If you want a solution without API Versioning, I don't have a good answer for you I'm afraid.

commonsensesoftware avatar Nov 30 '22 20:11 commonsensesoftware

Just use [FromServices]

[HttpGet]
public async Task<IActionResult> GetAllItems([FromServices] ODataQueryOptions options, CancellationToken cancellationToken)
{
   ...
}

Schinwinkwinsky avatar Nov 29 '23 11:11 Schinwinkwinsky