Skip to content

ODataQueryOptions and Swagger UI #739

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

Closed
Liero opened this issue Apr 20, 2021 · 27 comments
Closed

ODataQueryOptions and Swagger UI #739

Liero opened this issue Apr 20, 2021 · 27 comments

Comments

@Liero
Copy link

Liero commented Apr 20, 2021

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" />
@davidebriscese
Copy link

davidebriscese commented Apr 20, 2021

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
Copy link

davidebriscese commented Apr 20, 2021

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

@commonsensesoftware
Copy link
Collaborator

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

@HuntJason
Copy link

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

@commonsensesoftware
Copy link
Collaborator

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

@HuntJason
Copy link

HuntJason commented Aug 10, 2021

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.

@commonsensesoftware
Copy link
Collaborator

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

@HuntJason
Copy link

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
Copy link

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.

@sjd2021
Copy link

sjd2021 commented Nov 16, 2021

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.

@commonsensesoftware
Copy link
Collaborator

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

@sjd2021
Copy link

sjd2021 commented Nov 16, 2021

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.

@commonsensesoftware
Copy link
Collaborator

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

@Criperum
Copy link

Criperum commented Mar 3, 2022

Any update on this?

@commonsensesoftware
Copy link
Collaborator

@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
Copy link
Collaborator

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.

@HuntJason
Copy link

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

OData/AspNetCoreOData#757

@commonsensesoftware
Copy link
Collaborator

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

@HuntJason
Copy link

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

@commonsensesoftware
Copy link
Collaborator

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:

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

@Schinwinkwinsky
Copy link

Just use [FromServices]

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

@reponemec
Copy link

@Schinwinkwinsky [FromServices] helped, thanks.

@kuldeepGDI
Copy link

Just use [FromServices]

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

this still seems to not remove the schemas from the swagger UI though !

@commonsensesoftware
Copy link
Collaborator

@kuldeepGDI can you clarify what you mean by the schemas. Schemas are the very thing that OpenAPI shows. I suspect you mean that ODataQueryOptions explodes with a bunch of things that you don't want and aren't relevant. Should this be the case, there's a 99% chance you have not followed the Partial OData OpenAPI example. In order for API Versioning to work its magic, it needs some hints that at least some of OData is involved. The example shows what you need to do and a working example.

@kuldeepGDI
Copy link

@kuldeepGDI can you clarify what you mean by the schemas. Schemas are the very thing that OpenAPI shows. I suspect you mean that ODataQueryOptions explodes with a bunch of things that you don't want and aren't relevant. Should this be the case, there's a 99% chance you have not followed the Partial OData OpenAPI example. In order for API Versioning to work its magic, it needs some hints that at least some of OData is involved. The example shows what you need to do and a working example.

Yes that is what i meant, Yes i cloned the repo today, and will have a look at it tomorrow. I thought that only [FromServices] is enough :) but i get it now. Thanks for quick reply :)

@kuldeepGDI
Copy link

kuldeepGDI commented Nov 21, 2024

@kuldeepGDI can you clarify what you mean by the schemas. Schemas are the very thing that OpenAPI shows. I suspect you mean that ODataQueryOptions explodes with a bunch of things that you don't want and aren't relevant. Should this be the case, there's a 99% chance you have not followed the Partial OData OpenAPI example. In order for API Versioning to work its magic, it needs some hints that at least some of OData is involved. The example shows what you need to do and a working example.

Yes that is what i meant, Yes i cloned the repo today, and will have a look at it tomorrow. I thought that only [FromServices] is enough :) but i get it now. Thanks for quick reply :)

So what worked for me is to use combination of AddApiVersioning() and [FromServices], However, the example that we have in partial OData repo, uses additionally AddODataApiExplorer, where I can configure controllers in callbacks, however, we do not have ID fields in atleast some of the entities as they are DTOs, and swagger UI can not load as it complains of that. I suspect this is expected via OData standard. So in conclusion, I was managed to get rid of unnecessary models in schema ON swagger UI and unnecessary queryOptions as inputs on endpoints. However, the whole wiring logic still requires some understanding, and I would need to dig into the code and get better understanding with OData specific functionalities !

@commonsensesoftware
Copy link
Collaborator

@kuldeepGDI some level of understanding of OData's particulars are, unfortunately, required. Yes, you need AddODataApiExplorer to do anything with OData and the API Explorer, even partial OData. Most of the information needed and derived for the API Explorer comes from an EDM. When you aren't using OData, there is no EDM to derive from, but an ephemeral EDM is generated just for the API Explorer. Similarly, there is no way to know or understand what partial OData features are you are supporting in your controller since all of that logic is imperative. There's not really a way to configure this other than via conventions. You can, however, create custom conventions if you have a standard convention to apply to all or some of your controllers. Even though you aren't using OData, IModelConfiguration is still supported for the purposes of generating an EDM and the models used by the API Explorer. If you have a model that does not have an identity (e.g. no Id property), this is called a Complex Object (vs an Entity) in OData parlance. If you configure things this way, it should bridge the gap. I wish I had better answers about how to marry these two things together. I'm always open to new ideas about how that might be achieved. Hopefully, that clarifies things a bit and puts you down the path of success.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants