Single version-neutral controller and DefaultApiVersion set to neutral causes AmbiguousMatchException

[siewers]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

To start, this is a bit obscure and likely not something a lot of people will ever encounter.

If I set DefaultApiVersion to ApiVersion.Neutral and only have a single controller annotated with [ApiVersionNeutral] I'm getting an AmbiguousMatchException.

I've tracked it down to how the ApiVersionMatcherPolicy handles neutral endpoints, and it seems like the endpoint is added twice in this specific case here.
The issue might also be in the ApiVersionCollator, I'm not sure.

Expected Behavior

I would expect the endpoint to be accessible as normal, and did not expect an exception to happen, although I understand the configuration is unusual.

Steps To Reproduce

using Asp.Versioning;
using Asp.Versioning.Conventions;
using Microsoft.AspNetCore.Mvc;

var builder = WebApplication.CreateBuilder(args);

builder.Services
    .AddApiVersioning(options =>
    {
        options.DefaultApiVersion = ApiVersion.Neutral;
    })
    .AddMvc(options =>
    {
        options.Conventions.Add(new VersionByNamespaceConvention());
    });

var app = builder.Build();

app.UseRouting()
    .UseEndpoints(endpoints => endpoints.MapControllers());

app.Run();

[Route("[controller]")]
[ApiController]
[ApiVersionNeutral]
public class NeutralController : ControllerBase
{
    [HttpGet]
    public IActionResult Get()
    {
        return Ok("Neutral yay!");
    }
}

#region Remove this region and it breaks

namespace V1
{
    [Route("v{version:apiVersion}/[controller]")]
    [ApiController]
    public class VersionedController : ControllerBase
    {
        [HttpGet]
        public IActionResult Get()
        {
            return Ok("Versioned yay!");
        }
    }    
}

#endregion

Exceptions (if any)

By removing or commenting out the region in the repro, you will get the following error:

An unhandled exception occurred while processing the request.
AmbiguousMatchException: The request matched multiple endpoints. Matches:

NeutralController.Get (ControllerFilterTests)
NeutralController.Get (ControllerFilterTests)
Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.ReportAmbiguity(CandidateState[] candidateState)

.NET Version

7.0.306

Anything else?

My team spent quite some time getting to the bottom of this, and in our case, the solution was to simply remove the [ApiVersionNeutral] attribute.
However, I don't think this is expected behavior and it was very difficult to track down.

[commonsensesoftware]

You had me ... for a second. The problem is that ApiVersion.Neutral is not the thing that makes something API version-neutral contrary to the name. ApiVersion.Neutral is effectively a faux value meant to serve the Null Object pattern. API version-neutral means that an API can accept any API version, including none at all. This cannot be represented as a single value. The reason ApiVersion.Neutral exists is to serve as an arbitrary, non-null value that version-neutral APIs can be grouped on when needed. I had considered keeping this property and value internal, but there are a number of places across the various libraries where it can be useful. I elected to keep it public so that others can replicate that behavior. A similar pattern is used for ApiVersionModel.Neutral and ApiVersionMetadata.Neutral. This approach also helped cut down on unnecessary allocations.

The thing that determines if something is API version-neutral is IApiVersionNeutral, which is a marker interface. The ApiVersionNeutralAttribute implements this interface, but it can also be used for conventions. When this interface is collated, no ApiVersion values are collected. The way you can determine if an API is version-neutral is via ApiVersionMetadata.IsApiVersionNeutral, which is an aggregation of ApiVersionModel.IsApiVersionNeutral.

The reason you are seeing this error happen is due to how things are grouped for routing. First and foremost, you will likely encounter problems if the only thing you have are version-neutral APIs; it's as if you didn't version anything. In previous implementations, version-neutral endpoints would respond to any valid API version, which is arguably wrong. Now, a version-neutral endpoint will respond to any defined API version in your application. If you don't have any versioned endpoints, version-neutral can only match the unspecified use case. This can similarly problematic for OpenAPI documentation. This is a rare, nonsensical scenario IMHO. If you have all version-neutral APIs, then you shouldn't need API Versioning.

The versioned route tree edges are grouped on an API version. Since a version-neutral API doesn't have any specific API versions, it doesn't have a value to group on. This is a use case for ApiVersion.Neutral. The edges themselves are not distinct. As there are no other defined APIs in the application, ApiVersioningOptions.DefaultApiVersion is used as a fallback; however, that value is ApiVersion.Neutral. Similarly, version-neutral APIs are also grouped under ApiVersion.Neutral. As a result, you get two entries for the same destination - e.g. ambiguous.

On one hand, you could call this a 🐞. It's certainly not expected and arguably violates POLA. However, ApiVersion.Neutral is not the thing that makes something API version-neutral, so if that was you intention, it is a misconfiguration. As I type, I'm thinking about how you might avoid this landmine albeit obscure. ApiVersion.Neutral is not meant to have special meaning in the implementation so I'm hesitant to add explicit checks for it.

It would appear that your goal is to have otherwise unversioned APIs be version-neutral. This is certainly possible, but know that there be 🐉🐉🐉. The most significant concern is that if any version-neutral API ever needs to become versioned, you need to be very careful about changing things. This behavior is all or nothing. To go from version-neutral to versioned, you'd need to split and explicitly support all of the previous versions. If you have a sound versioning policy - say N-2, that might not be too horrible of a thing to fix. The case where a client doesn't specify an API version is the scenario that will haunt you. In that case, the client might break. In order to continue allowing no API version, you'd have to enable ApiVersioningOptions.AssumeDefaultVersionWhenUnspecified and you'd likely to have a custom IApiVersionSelector. That could get really messy if the initial version is inconsistent across APIs. The moral of the story is make sure you really, really intend to have a version-neutral API.

There are some alternate solutions. There isn't a built-in convention for version-neutral APIs, but you could easily create one. For example, like the VersionByNamespaceConvention, you could have a convention that maps version-neutral APIs based on some other convention in the .NET namespace; for example, it contains .Neutral.

Version-neutral APIs are also a sleazy way of opting out of versioning. If that is your intent and you have many controllers in this scenario, there is a better way to handle that. This type of edge case is advanced, but straight forward. The IApiControllerFilter decides whether a controller is included in API versioning. The default implementation just takes a sequence of IApiControllerSpecification instances. One of the out-of-the-box implementations is the ApiControllerSpecification, which matches controllers with [ApiController] applied. You could easily expand upon this. Let's say you had:

[AttributeUsage(AttributeTargets.Class, AllowMultiple = false)]
public sealed class UnversionedAttribute : Attribute { }

[Unversioned]
[ApiController]
[Route("[controller]")]
public sealed class TestController : ControllerBase
{
    [HttpGet]
    public IActionResult Get() => Ok();
}

public sealed class VersionedSpecification : IApiControllerSpecification
{
    public bool IsSatisifiedBy(ControllerModel controller) =>
        !controller.Attributes.OfType<UnversionedAttribute>().Any();
}

builder.Services.TryAddEnumerable(ServiceDescriptor.Transient<IApiControllerSpecification, VersionedSpecification>());

Now anything with [Unversioned] applied will truly opt out. The specification can be whatever you want. It might use a namespace convention rather than an attribute. You could also use NamespaceParser.Parse in the way that the VersionByNamespaceConvention does and if the result is an empty list, then you consider it version-neutral. That's entirely up to you.

I need to think a little bit more as to whether there is a way to protect you from yourself if you configure things this way, but ultimately the reported behavior is expected because it's misconfigured.

[siewers]

Thanks a lot for the thorough explanation. I must admit that our setup is slightly complex as we split three different API types based on area, where one of the APIs are unversioned. The APIs are all hosted within the same application and we’ve split them between a “private” unversioned API, only accessible by the web app in the system, an “internal user”, versioned, API accessible by the front end of other systems and finally an “internal system”, versioned, API accessible by the backend of other systems.
The reasoning behind this split is not important here, but as you might understand, it makes configuring our APIs a bit more difficult than usual.
Your last suggestion would make a lot of sense in our case as we already annotate our controllers with the type of API they are, so adding a controller filter that simply removes the controllers of the unversioned API might make things a bit simpler.
However, we also have unversioned controllers that are shared among all API types. In this case, introducing a new attribute could perhaps also be the solution, or simply just remove the ApiVersionNeutral attribute altogether.