Skip to content
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

VersionedMetadataController is not in Swagger configuration #893

Closed
1 task done
Angelinsky7 opened this issue Oct 24, 2022 · 6 comments · Fixed by #901
Closed
1 task done

VersionedMetadataController is not in Swagger configuration #893

Angelinsky7 opened this issue Oct 24, 2022 · 6 comments · Fixed by #901
Assignees

Comments

@Angelinsky7
Copy link

Angelinsky7 commented Oct 24, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

I would expect that VersionedMetadataController would appear naturally into the swagger's configuration. (like MetadataController does if using Microsoft.Aspnetcore.Mvc.Versioning.
If you try both of the example app with swagger, the /api/ and /api/$metadata don't appear in the documentation.
It would seem to me that it would be "normal" to have it automatically, don't you think ?
We could of course add a IDocumentFilter to add it manually (and it's working correctly like that) but i feel that most dev would want this ?
Thanks for you amazing support !

Expected Behavior

No response

Steps To Reproduce

run examples/AspNetCore/OData/ODataOpenApiExample and go to swagger and see that the $metadata api endpoint is not in the documentation but is working correctly.

Exceptions (if any)

No response

.NET Version

6.0.402

Anything else?

ASP.NET Core version: 6.0
Asp.Versioning.OData.ApiExplorer: 6.1.0
Microsoft.AspNetCore.OData: 8.0.10, 8.0.11
Swashbuckle.AspNetCore: 6.3.0, 6.4.0

@commonsensesoftware
Copy link
Collaborator

The $metadata endpoint has never been documented via the API Explorer extensions. MetadataController is not used in any flavor of API Versioning (including Microsoft.AspNetCore.Mvc.Versioning for < 6.0).

Philosophically, using $metadata with OpenAPI is strange - they serve the same purpose. Querying $metadata would provide the XML the EDM to code generate a client. VersionedMetadataController extends this so that $metadata?api-version=1.0 and so on return a versioned EDM to generate a particular client. There are addition extensions that tooling could send, such as OPTIONS $metadata to determine the versions available.

It is certainly possible to build out documentation for $metadata that can be exposed to OpenAPI, I just don't understand why someone would want to do that. The biggest challenge to supporting this capability is generating a model for the EDM. Most of the OpenAPI generators, such as Swashbuckle or NSwag, rely on Reflection for a Type in the documentation. OData doesn't exactly provide that. Microsoft.OData.Edm provides the model, but serialization is all based on EDMX. I'm not sure it can easily be serialized or consumed in a useful way for OpenAPI. If it can, I suspect it's non-trivial and I'm skeptical of the benefits.

Can you elaborate on the scenario where you'd need or want both OpenAPI and $metadata? The OData team has been working on an EDM-to-OpenAPI library. That might be a better solution once complete.

@Angelinsky7
Copy link
Author

Thanks @commonsensesoftware for you quick and complete answer !

The truth is that i'm new to building rest api with odata and before using this package i was simply using Microsoft.AspNetCore.OData without any versioning and the two $metadata endpoint are displayed in swagger's documentation.
It's true that they are not properly documented but they exist and are usable.

I was surprise to discover with this package that they have disappeared and was naively thinking that it was not normal. (i still thing it is, because in my point of view, every endpoints of the api should be "documented" and be known for the dev that would want to use them)

Thanks again for your support and amazing answer, i will certainly add the two endpoints with a IDocumentFilter by hand.

@commonsensesoftware
Copy link
Collaborator

🤔 In years gone by there was no support or integration for the API Explorer, and hence OpenAPI, for OData; I had to create it all. There are still gaps between what OData offers out of the box and the documentation features provided for versioned OData routes; for example, documenting query options. Hopefully, the OData will support all of the necessary documentation needs and supplant the need for me to provide it.

I guess if people want it, then they should have it; especially if it is there by default. OData technically has 2 special built-in routes:

  • ~/ - Service document endpoint
  • ~/$metadata - Metadata endpoint

There should be a little more to the documentation than just the endpoint URL. If OData provides something by default, then it shouldn't be taken away. It seems there needs to be a configuration option for people to decide if they want one, both, or neither of the endpoints. No one has asked for this in years, so the default would continue to be None.

Adding to list of enhancements.

@Angelinsky7
Copy link
Author

Thanks for your answer. And like always i often ask for features than nobody seems to be interested in so don't worry about it :-)

@commonsensesoftware
Copy link
Collaborator

FYI, 6.2 is queuing up right now. I've added a new feature to support this. The new setup will look like:

services.AddControllers().AddOData();

services.AddApiVersioning()
        .AddOData( options => options.AddRouteComponents() )
        .AddODataApiExplorer( options.MetadataOptions = ODataMetadataOptions.All );

The ODataMetadataOptions are:

  • None
  • Service Document (/)
  • Metadata (/$metadata)
  • All

These will be described as an API called OData and will appear in every API version. The default value will continue to be None.

@Angelinsky7
Copy link
Author

@commonsensesoftware wahou ! thanks you very much !!! You're the best.

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

Successfully merging a pull request may close this issue.

2 participants