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

6.0.0 - operationId not formatted properly for OData routes

Open spaasis opened this issue 3 years ago • 4 comments
trafficstars

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the bug

Hi,

Version 6 works like a dream and I've migrated most of our projects to it. Huge thanks to you again :)

I have configured my OData routes as such:

    services.AddApiVersioning(o => o.ReportApiVersions = true;)
        .AddMvc(o => o.Conventions.Add(new VersionByNamespaceConvention()))
        .AddOData(o => o.AddRouteComponents("api/v{version:apiVersion}/odata"))

And everything works fine on the swagger UI and the endpoints are routed as expected. However, one of our frontend developers noticed that the swagger.json operationIds for OData endpoints configured this way are not formatted properly, causing generated client code to be a bit ugly:

"operationId": "api/v{version:apiVersion}/odata/Things",

Expected Behavior

Operation id should have the version information: "operationId": "api/v1/odata/Things",

Anything else?

A perfectly fine workaround is to toggle operation id generation off completely. This might not work for everyone: services.AddSwaggerGen(c => c.CustomOperationIds(c => null))

spaasis avatar Sep 13 '22 11:09 spaasis

Interesting. I would have expected it be generated without the route constraint (e.g. apiVersion) as:

"operationId": "api/v{version}/odata/Things",

Based on the configuration you've shared, it doesn't appear that you are using the OData-specific API Explorer extensions. You might be able to get the results you want without them, but you likely want them. A common API Explorer extension is supporting this very use case, including non-OData routes.

The full setup would then look like:

services.AddApiVersioning(o => o.ReportApiVersions = true)
        .AddMvc(o => o.Conventions.Add(new VersionByNamespaceConvention()))
        .AddOData(o => o.AddRouteComponents("api/v{version:apiVersion}/odata"))
        .AddODataApiExplorer(o => o.SubstituteApiVersionInUrl = true);

This will replace the {version} token (or whatever name you define) and replace it with the corresponding, formatted API version. If the API version were 1.0, then it is formatted as:

"operationId": "api/v1/odata/Things",

When the API version parameter is in the path (🤮) and is substituted, the corresponding parameter is also removed. You can influence the format of substituted value via ApiExplorerOptions.SubstitutionFormat according to the supported formatting options. The default format is VVV, which is typically how APIs that version in the path prefer it to be seen.

commonsensesoftware avatar Sep 13 '22 15:09 commonsensesoftware

Pardon, I really need to stop leaving stuff out for "brevity"...

Here's my config (you can maybe sense where the inspiration came from :) ):

    services.AddApiVersioning(
            options => {
                // reporting api versions will return the headers
                // "api-supported-versions" and "api-deprecated-versions"
                options.ReportApiVersions = true;
            })
        .AddMvc(o => o.Conventions.Add(new VersionByNamespaceConvention()))
        .AddOData(o => o.AddRouteComponents("api/v{version:apiVersion}/odata"))
        .AddODataApiExplorer(options => {
            // add the versioned api explorer, which also adds IApiVersionDescriptionProvider service
            // note: the specified format code will format the version as "'v'major[.minor][-status]"
            options.GroupNameFormat = "'v'VVV";
            options.SubstituteApiVersionInUrl = true;
        });

So it's a match.

Actually this can be easily repro'd by setting the same route component in the sample https://github.com/dotnet/aspnet-api-versioning/blob/main/examples/AspNetCore/OData/ODataOpenApiExample/Program.cs

This produces a swagger.json like:

    "/api/v0.9/odata/Orders/{key}": {
      "get": {
        "tags": [
          "Orders"
        ],
        "summary": "Gets a single order.",
        "operationId": "api/v{version:apiVersion}/odata/Orders/{key}",

spaasis avatar Sep 13 '22 19:09 spaasis

Ah ... I see. I believe this is a behavior of however the OpenAPI generator creates the operationId for the document. In this case, I believe it's what Swashbuckle does. It is interesting that the 2 values are different though. I suspect there might be some type of metadata added that includes the full route template elsewhere. I'm sure I can get the value, but it's unclear if it can be changed; probably. I'm not sure where that is coming from, but it's definitely not from the ApiDescription. The other clue is that the route constraint is still present.

commonsensesoftware avatar Sep 13 '22 22:09 commonsensesoftware

Yeah, overriding the Swashbuckle CustomOperationIds would work around this, either by disabling operation ids completely (OP) or manually replacing the version information.

But realIy, this is a minor inconvenience which can be easily worked around if necessary. The only reason I've found where you'd "need" url path versioning is with Excel's OData functionality (did not support custom query parameters since last I inspected), but that is such a rare use case I don't think it's worth investing much effort to fix.

I'm fine with closing this issue as is, since now the thing is "documented" 👍

spaasis avatar Sep 14 '22 06:09 spaasis

I wasn't able to track down how this behavior is changed. DefaultOperationIdSelector in SwaggerGeneratorOptions appears to use either the route name and falls back to the endpoint name. The route table is not version-aware so using names often requires something like GetProductV1 or some other convention. As I recall, endpoint names are auto-generated and almost always a GUID, which likely makes them useless for this purpose.

The selector is just a Func<ApiDescription,string>. If you want the operationId to be the path with the version substituted, then api => api.RelativePath should do the trick. It's not clear to me how or where this template is coming from.

It appears that Swashbuckle also has a way to annotate the value using attributes a la [SwaggerOperation(OperationId = "GetOrder")]. That should generate "operationId": "GetOrder". Since the API only appears once per OpenAPI document, this will be the name used in all documents; no prefix or suffix is required.

Hopefully that gives you some 💡💡💡. Since you're satisfied with the resolution, I'm going to close out the issue. Thanks.

commonsensesoftware avatar Sep 29 '22 15:09 commonsensesoftware