Issue with multiple routes on a controller method

[orryverducci]

I've hit an issue with ASP.NET API Versioning and a controller which has multiple routes to a single method.

Basically I'm working on an application which needs to implement an external API specification. That specification has multiple API versions which can be implemented, indicated to the application in the URL path, so I'm using ASP.NET API Versioning to achieve this.

In one controller I have multiple URL paths, which the specification requires me to implement, but in my application I don't have anything to return on those paths. So I've put multiple routes, via HttpGet attributes, on to a single method which returns the same response (an empty array) to all those routes.

However when I do so the API version part of the URL path only works as expected on the first route defined on that method. The additional routes will return a response on any API version, not just the version the controller specify it supports. In addition for those routes Swagger shows the API part of path as being "v{version}" instead of the expected API version, as you'll see below:

Here's the code for the controller

[ApiController]
[ApiVersion(1.3)]
[Tags("Node API")]
[Route("x-nmos/[controller]/v{version:apiVersion}")]
public class NodeController : ControllerBase
{
    [HttpGet("sources")]
    [HttpGet("flows")]
    [HttpGet("devices")]
    [HttpGet("senders")]
    [HttpGet("receivers")]
    public string[] ReturnResources() => Array.Empty<string>();
}

It's easily worked around by giving each route their own method, but as they're identical it doesn't feel like it's the cleanest solution. Not sure if I'm missing something here, or if I've hit a limitation of the library, so any guidance would be appreciated!

[commonsensesoftware]

Are you using the API Explorer extensions? It will be required. By setting, options.SubstituteApiVersionInUrl = true in AddApiExplorer, the API version value will be substituted into the URL and the {version} (or whatever name you used as the parameter name) will be removed. This will be repeated for each API version.

Without seeing more of the setup, I don't have any other immediate ideas. This looks like it should work. If you have the world's simplest repo, that will go a long way in helping troubleshoot the problem.

As an aside, you might consider returning 501 (Not Implemented) unless an empty array is truly a valid response.

[orryverducci]

Yes I'm using the API Explorer extensions. This is what I have in my Program class, which I've essentially copy and pasted from the examples:

builder.Services.AddApiVersioning()
            .AddMvc()
            .AddApiExplorer(
                    options =>
                    {
                        options.GroupNameFormat = "'v'VVV";
                        options.SubstituteApiVersionInUrl = true;
                    });

I don't have a public repo to hand right now, but I'll throw together a simple example and get it up shortly.

As for your suggestion, an empty array is a valid response and required by the specification I'm working to, so that's why I'm doing it this way.

[orryverducci]

I've put together a simple repo which exibits this issue: https://github.com/orryverducci/API-Versioning-Issue-Demo

[commonsensesoftware]

The repo was very useful. I actually misread the initial issue. I see that {version} is properly substituted for the first route template. After a little digging, I can confirm that this is a 🐞. I'm actually surprised this hasn't happened before. The long and short of it is that when you apply multiple partial route templates as you have, ASP.NET Core creates multiple SelectorModel instances for the ActionModel, which each have their own endpoint metadata. This is where the ApiVersionMetadata is added. Currently, this information was only being added to the first selector, when it should be added to all of them.

Fortunately, the fix is simple and it seems to work with regression. I'll queue the fix up with some other pending fixes and hopefully get it published this week. The simplest short-term workaround is to fan out the implementation into separate action methods. Inconvenient, but that should unblock you until the fix is available.

[orryverducci]

Thanks for checking it and confirming it's a bug and not something that I was missing. Glad to hear it's a simple fix and that a release with it is on the way soon ☺️