.NET 6.0 - AmbiguousMatchException - Correctly Implemented (apparently)

[aicukltd]

Hey!

So I am working on a .NET 6 WebAPI project where I am introducing API versioning. As an example, I have separated out the new (v2) controllers into a new namespace and they inherit from our new base API class, the original controllers (v1) inherit from the old base class.

V1 Base Class

[ApiController]
[Route("api/v{version:apiVersion}/[controller]")]
[ApiVersion("1.0")]

V2 Base Class

[ApiController]
[Route("api/v{version:apiVersion}/[controller]")]
[ApiVersion("2.0")]

I am registering in the startup.cs:

services.AddApiVersioning(options =>
            {
                options.AssumeDefaultVersionWhenUnspecified = true;
                options.DefaultApiVersion = new ApiVersion(1, 0);
                options.ReportApiVersions = true;
                options.ApiVersionReader = ApiVersionReader.Combine(
                    new QueryStringApiVersionReader("api-version"),
                    new HeaderApiVersionReader("X-Version"),
                    new MediaTypeApiVersionReader("ver"), 
                    new UrlSegmentApiVersionReader());
            });
            services.AddVersionedApiExplorer(options =>
            {
                options.GroupNameFormat = "'v'VVV";
                options.SubstituteApiVersionInUrl = true;
            });

Each method / action / endpoint has either:
[MapToApiVersion("1.0")] or [MapToApiVersion("2.0")]

But I am still getting:

AmbiguousMatchException: The request matched multiple endpoints. Matches:

App.Web.Controllers.v2.LocationsController.Get (App.Web)
App.Web.Controllers.v1.LocationsController.GetLocations (App.Web)

I am using the following packages:

    <PackageReference Include="Microsoft.AspNetCore.Mvc.Versioning" Version="5.0.0" />
    <PackageReference Include="Microsoft.AspNetCore.Mvc.Versioning.ApiExplorer" Version="5.0.0" />

[commonsensesoftware]

From your description, it sounds like the issue is inheritance. API Versions are not inherited out-of-the-box. This is by design. You cannot uninherit an API version, but it is likely you'll sunset an API at some point. The main reason this is not supported, however, is that if ControllerA declares some API versions and routes, and then ControllerB inherits all of that, then you have 2 implementations with the same versions and routes, which result in AmbiguousMatchException (a developer mistake). An ancillary goal is to try and prevent developers from driving of a proverbial cliff.

There are ways to use inheritance, but it needs to be done with care. Personally, I think inheritance is the wrong approach for APIs. HTTP is the API and it intrinsically doesn't support inheritance. However, there is value using inheritance for common behaviors that you can't otherwise achieve (say through extension methods). The logic in controllers should purely be about what's happening over the wire. Moving other logic to separate components will reduce the level of duplication. A key driving factor is your versioning policy. Most services define a policy such as N-2 versions. Maintaining 10 different versions of a service can get messy. Maintaining 1-3, not so much. Copying and pasting vCurrent into a new vNext folder/namespace can be an easy and effective way to light up or shut down versions.

You mentioned new controllers go into new namespaces. Since you're organizing your code that way, you might find it easier to let the containing namespace itself define the API version rather than use attributes. This can be achieved by adding the built-in convention that versions by namespace: options.Conventions.Add( new VersionByNamespaceConvention() );. For more information refer to the Version By Namespace wiki section. Applying the API version metadata this way is not mutually exclusive (e.g. you can still use attributes, etc). Avoid version interleaving if choose this path or it can get really confusing to track down the code for an API version. The Version By Namespace Example project shows everything in action.

Some additional side notes based on the information you shared:

• While you can version by very possible method, it doesn't really make sense. I recommend choosing 1 or perhaps 2 (for backward compatibility?) and sticking with that
• It appears that you have set up your routes to version by URL segment. This is least RESTful method, but so be it. This method of versioning doesn't really make sense to be combine with any other because you always have to include the API version in the path. You could or may have other routes that do not version that way. I can't tell from the limited information I've seen. For example, api/v2/locations/42?api-version=2.0 is valid, but seems strange, unnecessary, and unlikely to be used/called by clients
• AssumeDefaultVersionWhenUnspecified will not do what you think it will do when you version by URL segment. You cannot have optional route parameter values in the middle of the template. It's no different than if you had api/locations/{id}/personnel. There is no way to make this request without specifying {id}. The same is true for an API version. The only way you can make that work is if you use double-route registration (e.g. add [Route("api/[controller]")]) as described in the wiki. All other methods of versioning do not have this issue or limitation.
• AssumeDefaultVersionWhenUnspecified is intended for backward compatibility. Review and consider if that's what it's being used for. The whole point of API versioning is to prevent clients from breaking when things change. If a client doesn't have to specify an API version, the carpet is just waiting to be pulled out from under them. There is no such thing is backward compatible over the wire. As a server you cannot guarantee that, even if your changes are additive. The one and only exception to this rule is for APIs that did not formally define/name their original/initial/first API release. Forcing clients bound this this version would break them. They, and they alone, can be configured to not specify an explicit API version and you'll make it to whatever logically makes sense on the server. This mapping and behavior should never change aside from sunsetting the API version altogether, at which point AssumeDefaultVersionWhenUnspecified should be set back to false.

[aicukltd]

Thank you for your timely response. I appreciate the time spent writing it.

Ill address your message as subheadings, as it just makes it easier.

Context
We are building a V2 API to supersede an existing API, the response will be identical and routes the same, however, the structure of the code behind and services will be different. So the versioning is almost being used so that we can switch over to the new version when ready while deprecating the existing without removing any functionality. This is why we are using the Default version property (which seems to work in terms of allowing the default, non-versioned routes, to resolve to the V1 API).

Inheritence
We are utilising, for the V2 implementation, a common base class that provides the same functionality using generics and routing which works in terms of functionality and expected endpoint accessibility. But it seems you could be correct, but surely this would have been resolved by, in the V1 controller, which we have done, specifying an exact API version per method? Could it be the V2 controller that is in fact not working due to inheritance? Surely it makes more sense to utilise inheritance where possible to save rewriting of code?

Versioning Resolution
We are happy with the URL segment as the client side will only ever see a non-versioned (utilising the default version property) URL it will just point to different controllers. This is the behaviour we wanted as this API needs to be implemented without any changes to the client side.

Namespace Versioning
Ill check this out...

[commonsensesoftware]

No problem. I provide advice and guidance, but the decisions are up to you. 😉

Context

This is fine and how it is meant to work. In this case, you do want the AssumeDefaultVersionWhenUnspecified = true then. I didn't mention it before, but options.DefaultApiVersion == ApiVersion.Default == new ApiVersion(1,0). There's absolutely nothing wrong with explicitly setting it so that it's clear, but I thought you should know.

Inheritance

"Surely it makes more sense to utilise inheritance where possible to save rewriting of code?"

Not really. It was never the intent to force developers to do more work, but inheritance is an implementation detail. From experience, working with many teams over the years, I've come the conclusion that inheritance in this context causes more problems than it solves. Honestly, a controller action shouldn't have more 10 lines of code, if even that, IMHO. Saving those few lines of code doesn't buy much.

In addition, you have to consider the cascading effects, which might require you to undo inheritance as an implementation detail. Imagine a scenario where a bug is detected in 2.0 which you want to fix. The bug also existed in 1.0, but clients figured out how to deal with it without reporting it. 1.0 and 2.0 are coupled through inheritance. Making a change to one or the other could result in a breaking change. This could end up being a lot of work to unwind. In fact, it may not be fixable at all without introducing new version 3.0. Ideally, each version should have an independent implementation so that this never happens. A break changing would be any change to the wire protocol, but it could also be a change to a known, expected behavior. I'm all for less implementation code, but not at the expense of risking cascading changes to different APIs. Only you know this risk, how likely it is, and how you might mitigate it.

Let me illustrate an example that may shed some additional light. Consider that you have:

[assembly: ApiController] // indicate that all controllers in the assembly are API controllers =D

[ApiVersion("1.0")]
[Route("api/[controller]")]
public class LocationsController : ControllerBase
{
    // GET api/locations (back-compat, no ver option).
    // GET api/locations?api-version=1.0
    [HttpGet]
    public virtual IActionResult Get(ApiVersion version) => Ok(new {ver = version.ToString()});
}

If the ApiVersionAttribute were inherited, then you might define 2.0 like this:

[ApiVersion("2.0")]
public class Locations2Controller : LocationsController
{
    // GET api/locations?api-version=2.0
    public override IActionResult Get(ApiVersion version) => Ok(new {version.Major, version.Minor});
}

What you wanted was:

Action	Route	Version
LocationsController.Get	api/locations	1.0
LocationsController.Get	api/locations?api-version=1.0	1.0
Locations2Controller.Get	api/locations?api-version=2.0	2.0

What you actually have is:

Action	Route	Version
LocationsController.Get	api/locations	1.0
LocationsController.Get	api/locations?api-version=1.0	1.0
Locations2Controller.Get	api/locations?api-version=1.0	1.0
Locations2Controller.Get	api/locations?api-version=2.0	2.0

This is how you end up with AmbiguousMatchException. LocationsController and Locations2Controller (via inheritance) both provide a 1.0 API version mapping to api/locations making it ambiguous. This is a confusing situation to be in, which is why it isn't supported out-of-the-box.

There are several ways to get out of this situation:

1. Don't use inheritance; move all the useable stuff to extension methods, results, business components, etc
2. Only inherit code functionality (think protected methods); derived types perform all the actions and map API versions
3. Roll your own attribute that allows inheritance. API Versioning doesn't care, you just need to implement IApiVersionProvider. ApiVersionsBaseAttribute is the place to start. Warning: There be dragons.
4. Use a custom convention. You can define custom conventions, which might work for your scenario. You'd have to define how inherited API versions are handled, but you have full control over it.

Version Resolution

It's fine if you want to version by URL segment. I don't recommend it and it's not what I would do (because it's not RESTful), but it's a popular method nevertheless. If this is your strategy, then you should do the following:

• options.ApiVersionReader = new UrlSegmentApiVersionReader(); no sense in having others if this is your policy
• The 1.0 controllers should define [Route("api/[controller]")] and [Route("api/v{version:apiVersion}/[controller]")]. This will support existing clients and provide a path forward to specify the version. Clients that become version-aware are likely to construct the URL by always providing version they want. Unless you are enforcing symmetrical versions, bare in mind that a client might use a mix of 1.0 and 2.0 at the same time. It's easier for them if they always specify the version consistently, even if there is a path in the original version that doesn't require it.

[aicukltd]

Inheritance

So, the reason for looking at inheritance is this; We have 65 entities that we perform CRUD operations on through a Vue.js client. The original (now V1) API implementation was thicc controllers with all of the business logic in the actions and no SOLID principles or ability to test or scale etc (The business was brought in to redesign). The controllers were pretty much identical, save for about 10 that additional functionality in them. So in our case the V2 controller, will NEVER inherit from the V1 because these will be fully deprecated and removed once V2 works.

The V2 BaseApiController looks like this for a reason.

[ApiController]
[Route("api/v{version:apiVersion}/[controller]")]
[ApiVersion("2.0")]
public class BaseApiController<T, TId> : ControllerBase where T : class, IHasId<TId> where TId : struct
{
        public BaseApiController(ICrudServiceAsync<T, TId> service)
        {
            this.Service = service;
        }

        private ICrudServiceAsync<T,TId> Service { get; set; }

        // GET: api/Ts
        [MapToApiVersion("2.0")]
        [HttpGet]
        public virtual async Task<ActionResult<IEnumerable<T>>> Get()
        {
            return (await this.Service.ReadAsync()).ToList();
        }

        // GET: api/Ts/5
        [MapToApiVersion("2.0")]
        [HttpGet("{id}")]
        public virtual async Task<ActionResult<T>> Get(TId id)
        {
            return await this.Service.ReadAsync(id);
        }

        // PUT: api/Ts/5
        // To protect from overposting attacks, see https://go.microsoft.com/fwlink/?linkid=2123754
        [MapToApiVersion("2.0")]
        [HttpPut("{id}")]
        public virtual async Task<IActionResult> Put(TId id, T model)
        {
            var modifiedId = await this.Service.CreateOrUpdateAsync(model, x => x.Id.Equals(id));

            if (modifiedId.Equals(id)) return this.Ok();

            return this.NoContent();
        }

        // POST: api/Ts
        // To protect from overposting attacks, see https://go.microsoft.com/fwlink/?linkid=2123754
        [MapToApiVersion("2.0")]
        [HttpPost]
        public virtual async Task<ActionResult<T>> Post(T model)
        {
            var createdId = await this.Service.CreateOrUpdateAsync(model);

            return this.CreatedAtAction($"Get{typeof(T).Name}", new { id = createdId }, model);
        }

        // DELETE: api/Ts/5
        [MapToApiVersion("2.0")]
        [HttpDelete("{id}")]
        public virtual async Task<IActionResult> Delete(TId id)
        {
            var deleted = await this.Service.DeleteAsync(id);

            if (deleted) return this.Ok();

            return this.NoContent();
        }

        [MapToApiVersion("2.0")]
        [HttpGet("exists/{id}")]
        public virtual async Task<bool> Exists(TId id)
        {
            return await this.Service.ExistsAsync(id);
        }
}

I have use the generic base API controller pattern before and it has worked, it just seems to be the API versioning that isn't!

I have implemented the [Route("api/[controller]")] in the V1 controllers with no difference in behaviour, I am still receiving the AmbiguousMatchException.

[aicukltd]

I am going to try the route of implementing Inheritance aware Attributes and see if this solves my issue.

[aicukltd]

To confirm, I have resolved this issue by implementing the following. Thank you @commonsensesoftware .

[AttributeUsage(Class | Method, AllowMultiple = true, Inherited = true)]
    public class InheritableApiVersionAttribute : ApiVersionAttribute
    {
        protected InheritableApiVersionAttribute(ApiVersion version) : base(version)
        {
        }
        public InheritableApiVersionAttribute(string version) : base(version) { }
    }

[AttributeUsage(Method, AllowMultiple = true, Inherited = true)]
    public class InheritableMapToApiVersionAttribute : MapToApiVersionAttribute
    {
        protected InheritableMapToApiVersionAttribute(ApiVersion version) : base(version)
        {
        }

        public InheritableMapToApiVersionAttribute(string version) : base(version) { }
    }

[commonsensesoftware]

Awesome! Glad you got it working. For completeness, this also should have been solvable with inheritance like this:

// define base functionality
[ApiController]
[Route("api/v{version:apiVersion}/[controller]")]
public class BaseApiController<T, TId> : ControllerBase
    where T : class, IHasId<TId>
    where TId : struct
{
    protected BaseApiController() { }
    // implementation omitted for brevity
}

// apply base functionality to v2 via inheritance
[ApiVersion("2.0")]
public class V2Controller<T, TId> : BaseApiController<T, TId>
    where T : class, IHasId<TId>
    where TId : struct
{
    // the implementation is completely inherited and implicitly maps all actions to v2
}