AmbiguousActionException after upgrade from 1.0-beta 2 to 1.0-rc1 (and all later versions) (ASP.Net Core)

[Eneuman]

Hi!

I think 1.0 rc1 introduced a bug when using route attributes in web api with Asp.Net Core 1.1.2
The following code works fine in mvc.versioning beta2.

[ApiVersion("1.0")]
[Route("[controller]")]
[Produces("application/json")]
public class ErsattningsunderlagController : Controller
......

[HttpGet("{id}"), MapToApiVersion("1.0")]
public async Task<IActionResult> HämtaErsättningsunderlagAsyncV1([FromRoute]int id)
{
....
}

[HttpGet("Pagaende"), MapToApiVersion("1.0")]
public async Task<IActionResult> HämtaPågåendeErsättningsunderlagFörGridAsyncV1()
{
.....
}

But the exact same code fails to run after upgrade (I only added app.UseApiVersioning() to startup);

The error is this:

Microsoft.AspNetCore.Mvc.Internal.AmbiguousActionException: Multiple actions matched. The following actions matched route data and had all constraints satisfied:

Ekonomi.Forsaljning.Service.Controllers.ErsattningsunderlagController.HämtaPågåendeErsättningsunderlagFörGridAsyncV1 (Ekonomi.Forsaljning.Service)
Ekonomi.Forsaljning.Service.Controllers.ErsattningsunderlagController.HämtaErsättningsunderlagAsyncV1 (Ekonomi.Forsaljning.Service)
   vid Microsoft.AspNetCore.Mvc.Routing.DefaultApiVersionRoutePolicy.OnMultipleMatches(RouteContext context, ActionSelectionResult selectionResult)
   vid Microsoft.AspNetCore.Mvc.Routing.DefaultApiVersionRoutePolicy.RouteAsync(RouteContext context)
   vid Microsoft.AspNetCore.Routing.RouteCollection.<RouteAsync>d__9.MoveNext()
--- Slut på stackspårningen från föregående plats där ett undantag utlöstes ---
   vid System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   vid System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   vid Microsoft.AspNetCore.Builder.RouterMiddleware.<Invoke>d__4.MoveNext()
--- Slut på stackspårningen från föregående plats där ett undantag utlöstes ---
   vid System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   vid System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   vid Microsoft.AspNetCore.Builder.RouterMiddleware.<Invoke>d__4.MoveNext()
--- Slut på stackspårningen från föregående plats där ett undantag utlöstes ---
   vid System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   vid System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   vid Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.<Invoke>d__7.MoveNext()

It seems like something is breaking routing.

[commonsensesoftware]

Your setup should have at least this:

public void ConfigureServices( IServiceCollection services )
{
  services.AddMvc();
  services.AddApiVersioning();
}

public void Configure( IApplicationBuilder app )
{
  app.UseMvc();
  app.UseApiVersioning();
}

Can you confirm that is in place? Let's start with that before we consider other factors and a possible repro. Thanks.

[Eneuman]

Yupp, thats in place:

  services.AddMvcCore().AddVersionedApiExplorer();
  services.AddMvc();

  // Lägg till versionshantering för apierna
  services.AddApiVersioning(o => o.ReportApiVersions = true);

[Eneuman]

and also:

      app.UseMvc();
      app.UseApiVersioning();

[commonsensesoftware]

Hmmm.. ok .. Can we boil things down to the signature the offending controllers? I assuming this is happening across controllers or are you saying it happens for actions on the same controller?

[Eneuman]

Yeah, its happening in the same controller.
Somehow it thinks that
[HttpGet("Pagaende"), MapToApiVersion("1.0")]
is the same signature as
[HttpGet("{id}"), MapToApiVersion("1.0")]

It's like it doesn't understand the "{id}"

The url I'm calling looks something like this:
http://localhost/Ekonomi/Forsaljning/Ersattningsunderlag/Pagaende?api-version=1.0

[Eneuman]

I think I just found a workaround.

Using this signature will work:
[HttpGet("{id:int}"), MapToApiVersion("1.0")]
but this will not:
[HttpGet("{id}"), MapToApiVersion("1.0")]

[commonsensesoftware]

Hmmm... there was an issue in the RC that I fixed. Basically, the issue was that {id} and {Pagaende} would be ambiguous because the string Pagaende can be substituted for {id}. That should fixed in the final release and I thought I added a test case for that. I'll double-check and add additional coverage as necessary. It seems that you're unblocked for now.

In terms of precedence, the routes should be evaluated as:

• /Ekonomi/Forsaljning/Ersattningsunderlag/Pagaende
• /Ekonomi/Forsaljning/Ersattningsunderlag/{some id}

Just FYI, not sure if you just wanted to be very explicit, but you don't need to use MapToApiVersion unless you're interleaving your API versions (which is supported, but generally not recommended). The API version of each action will implicitly match to the API version defined on the controller.

[Eneuman]

I upgraded to version 1.0 and the problem still occurse if I remove the explicit type declaration :(
I could add the type declaration to all our controllers but then I would need to changing several hundreds of files so Im gonna hold off until you can confirm if it's a bug that will be fixed or by design ;)

We are creating a Service Fabric application that consist of several applications. During deployment our application needs to support multiple versions of the API. After the deployment is complete, we remove the obsolete versions from code, thats why we use MapApiVersion in the code :)

[commonsensesoftware]

Hooray for using Service Fabric!

I'm not sure I follow the issue with the attribute mapping though. Those should be compile-time constants with the way you are using them. The MapToApiVersion attribute just provides a way to disambiguate actions when you interleave API versions on a single controller (e.g. service). For example, if your controller had a method called Get for V1 of your service and a method called GetV2 that implements V2, they would have the same route. The action selection process needs to know which method to dispatch to. If no attribution is provided, the action selector will look at the controller attribution. Any candidate action that provides explicit API version mapping using MapToApiVersion will take precedence over the implicit matching process using the parent controller API version metadata.

Is it possible to get a simple repro for your scenario? Adding a constraint to a route parameter should not be the thing that enables this to work. I'm either not seeing or understanding something about your setup or there is possibly a more systemic issue.

Thanks

[Eneuman]

:)

What we want to accomplishe is this:
If we need to change a api in a way that will break backwards compability, we create a copy of that method and bump the API version for that method (say up to V2). We will also mark the V1 method as obsolete.
During the upgrade of the cluster, we will have old services calling the old api version and new (just upgraded) services calling the new api version. When the deployment has finished, all services will call the new API version. We will then delete the code for the V1 service. So in the new deployment, V1 will no longer exist.

What I was thinking was declaring the controller like this:

  [ApiVersion("1.0", true)]
  [ApiVersion("2.0")]
  [Route("[controller]")]
  public class LeverantorerController : Controller
{
    [Obsolete]
    [HttpGet("{id}"), MapToApiVersion("1.0")]
    public async Task<IActionResult> GetListV1Async([FromRoute]int id)
    {
      return Ok();
    }

    [HttpGet("{id}"), MapToApiVersion("2.0")]
    public async Task<IActionResult> GetListV2Async([FromRoute]int id)
    {
      return Ok();
    }

}

Or is there a better way to do this ?