SDammann.WebApi.Versioning icon indicating copy to clipboard operation
SDammann.WebApi.Versioning copied to clipboard

Ambiguous routes reported incorrectly

Open richardp-au opened this issue 9 years ago • 6 comments

I have the following controller in my project:

namespace MyProject.Api.Version1
{
    [RoutePrefix("api/something")]
    public class SomethingController : ApiController
    {
        [Route("")]
        [HttpGet]
        public IHttpActionResult GetSomething()
        {
            return Ok();
        }

        [Route("else")]
        [HttpGet]
        public IHttpActionResult GetSomethingElse()
        {
            return Ok();
        }
    }
}

When I issue a GET /api/something or GET /api/something/else, I get an InvalidOperationException (details below). The exception message is:

Multiple actions were found that match the request: 
    Something on type MyProject.Api.Version1.SomethingController
    SomethingElse on type MyProject.Api.Version1.SomethingController

Despite the issue with the routes not being detected properly, I would've thought that if there was an error with multiple routes being detected, that it would throw an AmbigiousApiReqestException (using 3.0.0-beta3). Also note that Ambiguous is incorrectly spelt in that exception name.

I've also tried moving the GetSomethingElse() method into a separate SomethingElseController but that doesn't make a difference.

System.InvalidOperationException was unhandled by user code
  HResult=-2146233079
  Message=Multiple actions were found that match the request: 
Something on type MyProject.Api.Version1.SomethingController
SomethingElse on type MyProject.Api.Version1.SomethingController
  Source=MyProject
  StackTrace:
       at MyProject.Api.GlobalExceptionHandler.Handle(ExceptionHandlerContext context) in [...]\MyProject\Api\GlobalExceptionHandler.cs:line 19
       at System.Web.Http.ExceptionHandling.ExceptionHandler.HandleAsync(ExceptionHandlerContext context, CancellationToken cancellationToken)
       at System.Web.Http.ExceptionHandling.ExceptionHandler.System.Web.Http.ExceptionHandling.IExceptionHandler.HandleAsync(ExceptionHandlerContext context, CancellationToken cancellationToken)
       at System.Web.Http.ExceptionHandling.LastChanceExceptionHandler.HandleAsync(ExceptionHandlerContext context, CancellationToken cancellationToken)
       at System.Web.Http.ExceptionHandling.ExceptionHandlerExtensions.<HandleAsyncCore>d__0.MoveNext()
  InnerException: 

If I look at the actual exception inside the global exception handler's ExceptionHandlerContext.Exception, the stack trace starts with:

at System.Web.Http.Controllers.ApiControllerActionSelector.ActionSelectorCacheItem.SelectAction(HttpControllerContext controllerContext)
at System.Web.Http.Controllers.ApiControllerActionSelector.SelectAction(HttpControllerContext controllerContext)
at System.Web.Http.ApiController.ExecuteAsync(HttpControllerContext controllerContext, CancellationToken cancellationToken)
[...]

richardp-au avatar Mar 04 '15 00:03 richardp-au

I looked into this further and discovered that attribute routes aren't being handled properly in the DefaultRequestControllerNameDetector. It only looks at non-attribute routes.

I was able to get it working by creating a new IRequestControllerNameDetector to handle attribute routing correctly.

public class AttributeRouteRequestControllerNameDetector : IRequestControllerNameDetector
{
    public string GetControllerName(HttpRequestMessage requestMessage)
    {
        if (requestMessage == null)
        {
            throw new ArgumentNullException("requestMessage");
        }

        var routeData = requestMessage.GetRouteData();
        if (routeData == null)
        {
            return default(string);
        }

        var attributeSubRoutes = routeData.GetSubRoutes();
        if (attributeSubRoutes == null)
        {
            return default(string);
        }

        return GetControllerNameFromAttributeSubRoutes(attributeSubRoutes);
    }

    protected virtual string GetControllerNameFromAttributeSubRoutes(IEnumerable<IHttpRouteData> attributeSubRoutes)
    {
        return attributeSubRoutes.Select(a => ((HttpActionDescriptor[]) a.Route.DataTokens["actions"]).First().ControllerDescriptor.ControllerName).FirstOrDefault();
    }
}

I'm not sure if there's more to it than just this but this got it working with attribute routes for me. The other option would be to combine both attribute and non-attribute routing into the one class but that's your decision.

richardp-au avatar Mar 04 '15 07:03 richardp-au

Yes, attribute routing was not in any sense tested and thus supported.

With your permission, I'd like to merge your code in the library.


Van: Richard Pickettmailto:[email protected] Verzonden: ‎4-‎3-‎2015 08:32 Aan: Sebazzz/SDammann.WebApi.Versioningmailto:[email protected] Onderwerp: Re: [SDammann.WebApi.Versioning] Ambiguous routes reported incorrectly (#45)

I looked into this further and discovered that attribute routes aren't being handled properly in the DefaultRequestControllerNameDetector. It only looks at non-attribute routes.

I was able to get it working by creating a new IRequestControllerNameDetector to handle attribute routing correctly.

public class AttributeRouteRequestControllerNameDetector : IRequestControllerNameDetector
{
    public string GetControllerName(HttpRequestMessage requestMessage)
    {
        if (requestMessage == null)
        {
            throw new ArgumentNullException("requestMessage");
        }

        var routeData = requestMessage.GetRouteData();
        if (routeData == null)
        {
            return default(string);
        }

        var attributeSubRoutes = routeData.GetSubRoutes();
        if (attributeSubRoutes == null)
        {
            return default(string);
        }

        return GetControllerNameFromAttributeSubRoutes(attributeSubRoutes);
    }

    protected virtual string GetControllerNameFromAttributeSubRoutes(IEnumerable<IHttpRouteData> attributeSubRoutes)
    {
        return attributeSubRoutes.Select(a => ((HttpActionDescriptor[]) a.Route.DataTokens["actions"]).First().ControllerDescriptor.ControllerName).FirstOrDefault();
    }
}

I'm not sure if there's more to it than just this but this got it working with attribute routes for me. The other option would be to combine both attribute and non-attribute routing into the one class but that's your decision.


Reply to this email directly or view it on GitHub: https://github.com/Sebazzz/SDammann.WebApi.Versioning/issues/45#issuecomment-77109117

Sebazzz avatar Mar 04 '15 15:03 Sebazzz

You're welcome to include that code. I actually grabbed the attribute sub route code from this StackOverflow question which was written up in this blog.

I wonder also if it might be better to provide implementations for AttributeRouteRequestControllerNameDetector (as per my code above) and ControllerRouteKeyRequestControllerNameDetector (rename of the current DefaultRequestControllerNameDetector class). And then make a new DefaultRequestControllerNameDetector that combines both attribute and non-attribute routes. You'd basically just do

var attributeSubRoutes = routeData.GetSubRoutes();

return attributeSubRoutes == null
    ? GetControllerNameFromRouteData(routeData)
    : GetControllerNameFromAttributeSubRoutes(attributeSubRoutes);

in the GetControllerName method. By making the default work for both attribute and non-attribute routing, it would simply work by default which I think is a good default.

richardp-au avatar Mar 05 '15 03:03 richardp-au

I am using 2.7 release in my project. Is this issue fixed in there? I am facing same issue in my project.

yalin78 avatar Oct 22 '15 20:10 yalin78

Hello, Was this issue being handled ... if not, any plans in the future ?

jalchr avatar Dec 30 '15 11:12 jalchr

Currently not working on it, sorry.

Sebazzz avatar Jan 02 '16 15:01 Sebazzz