Error when using RouteAttribute and HttpPost together on same action

[DamianEdwards]

Microsoft.AspNet.Mvc.Routing (1.0.0-beta1-10401)

Repro Steps

• Create a new ASP.NET vNext MVC project using the project template
• Add the following action method declaration to the HomeController:

[Route("foo")]
[HttpPost]
public IActionResult Foo()
{
    return View();
}

• Launch the application from VS

Expected Results

The site runs

Actual Results

A Helios error is displayed with the following details:

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.InvalidOperationException: The following errors occurred with attribute routing information:

Error 1:
A method 'MusicStore.Apis.AlbumsApiController.CreateAlbum' must not define attribute routed actions and non attribute routed actions at the same time:
Action: 'MusicStore.Apis.AlbumsApiController.CreateAlbum' - Template: 'api/albums'
Action: 'MusicStore.Apis.AlbumsApiController.CreateAlbum' - Template: '(none)'
 at Microsoft.AspNet.Mvc.ReflectedActionDescriptorProvider.Build(ReflectedApplicationModel model)
 at Microsoft.AspNet.Mvc.ReflectedActionDescriptorProvider.GetDescriptors()
 at Microsoft.AspNet.Mvc.ReflectedActionDescriptorProvider.Invoke(ActionDescriptorProviderContext context, Action callNext)
 at Microsoft.Framework.DependencyInjection.NestedProviders.NestedProviderManager`1.CallNext.CallNextProvider()
 at Microsoft.Framework.DependencyInjection.NestedProviders.NestedProviderManager`1.Invoke(T context)
 at Microsoft.AspNet.Mvc.DefaultActionDescriptorsCollectionProvider.GetCollection()
 at Microsoft.AspNet.Mvc.DefaultActionDescriptorsCollectionProvider.get_ActionDescriptors()
 at Microsoft.AspNet.Mvc.Routing.AttributeRouting.GetActionDescriptors(IServiceProvider services)
 at Microsoft.AspNet.Mvc.Routing.AttributeRouting.CreateAttributeMegaRoute(IRouter target, IServiceProvider services)
 at Microsoft.AspNet.Builder.BuilderExtensions.UseMvc(IApplicationBuilder app, Action`1 configureRoutes)
 at Microsoft.AspNet.Builder.BuilderExtensions.UseMvc(IApplicationBuilder app)
 at MusicStore.Spa.Startup.Configure(IApplicationBuilder app) in C:\src\GitHub\AspNet\MusicStore\src\MusicStore.Spa\Startup.cs:line 66
 --- End of inner exception stack trace ---
 at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
 at System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments)
 at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
 at Microsoft.AspNet.Hosting.Startup.StartupLoader.<>c__DisplayClass0.<LoadStartup>b__3(IApplicationBuilder builder)
 at Microsoft.AspNet.Loader.IIS.KlrHttpApplication.HeliosStartupLoaderProvider.WrappingStartupLoader.<>c__DisplayClass0.<LoadStartup>b__1(IApplicationBuilder builder)
 at Microsoft.AspNet.Hosting.HostingEngine.EnsureApplicationDelegate(HostingContext context)
 at Microsoft.AspNet.Hosting.HostingEngine.Start(HostingContext context)
 at Microsoft.AspNet.Loader.IIS.KlrHttpApplication.ApplicationStart(IHttpApplication application)
 at Microsoft.AspNet.Loader.IIS.HttpApplicationBase.InvokeApplicationStart(IHttpApplication application)

[danroth27]

We should definitely improve the error message and also see if we can come up with a behavior that is more compatible with the MVC 5 behavior.

[javiercn]

The current behavior is by design, [Route] defines an attribute routed action without a constraint. [HttpGet] without a valid template defines an IHttpConstraint. In the new behavior a method can define attributed actions or non attributed actions, but not both at the same time.

The same behavior can be achieved by just doing

[HttpPost("foo")]

We can discuss changing this when @rynowak is around.

[DamianEdwards]

@javiercn was it really intended that HttpPost without a template argument defines a new route? You say that without a template it defines an IHttpConstraint. That does not appear to be what's happening right now. If that were the case, I'd expect the example above to result in a single route (foo) with a constraint of POST added.

[javiercn]

@DamianEdwards Sorry, I didn't explain myself properly. Let me try to reword it in clearer terms. An action like Foo can define one or more actions that can either be conventionally routed or attribute routed, but not both at the same time.

If a method defines a valid attribute routed action (like the one produced by route) we consider that all the actions defined by that method have to be attribute routed (we don't allow mixing conventionally routed and attribute routed actions on the same method, that's the error message).

That said, If during action discovery we see that you don't define a valid route template (by providing a template using [Route] on the controller, or [Http_] or [Route] on the action) we just treat [Http_] as plain IHttpConstraints so that you can still have conventionally routed actions that use [HttpGet], [AcceptVerbs], etc.

The recomendation is not to do

[Route("template")]
[Http*]
public void MyAction(){}

but to just do

[Http*("template")]
public void MyAction(){}

or

[AcceptVerbs("POST, GET", Route = "template")]
public void MyAction(){}

if you wan to support multiple HTTP methods.

If you want to enable this scenario

[Route("template")]
[HttpPost]
public void MyAction(){}

you can easily do so by writing your own IActionDiscoveryConvention or by writing an IReflectedApplicationModelConvention that tweaks the orginal model we produce.

The reason why we chose this approach is that you can achieve the scenario above more succinctly by just using [Http*("template")] and with the other option, scenarios like the following

[HttpPut("v1/template/{id}")]
[HttpPatch("v2/template/{id}")]
public void MyAction(int id, ...){}

won't work as expected out of the box.

I hope this makes things clearer, but I completely agree with you that the error message could be improved.