[Fixes #734] Attribute Routing: Implement Name #1058
Conversation
| ad.AttributeRouteInfo.Template)); | ||
|
|
||
| var errorDescription = string.Join(Environment.NewLine, descriptions); | ||
| var message = Resources.FormatAttributeRoute_ActionsWithDifferentTemplate_AndSameName( |
There was a problem hiding this comment.
This doesn't look like a clean error message
There was a problem hiding this comment.
This is a sample generated error message. It points the name given to the attribute routes, the actions with an attribute route with that name, and the template associated with each attribute route defined for a given action.
I think it's fine, but if you feel there's anything that can be improved I'm open to suggestions:
The following named attribute routed actions have the same name 'Products', but different templates:
'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+InvalidAttributeRouteNamesController.Get': 'Products'
'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+InvalidAttributeRouteNamesController.Get': 'Products/{id}'
'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+InvalidAttributeRouteNamesController.Put': 'Products/{id}'
'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+InvalidAttributeRouteNamesController.Post': 'Products'
'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+InvalidAttributeRouteNamesController.Delete': 'Products/{id}'
Take into account that this is the error message produced for a test class (nested) which is why the action descriptor DisplayName is so long. On a typical scenario the name would be AppNamespace.Controllers.ProductsController.Get for example
|
⌚ |
| var namedRoutedErrors = ValidateNamedAttributeRoutedActions(actionsByRouteName); | ||
| if (namedRoutedErrors != null) | ||
| { | ||
| throw new InvalidOperationException(namedRoutedErrors); |
There was a problem hiding this comment.
It would be nice to see the same pattern as the code below. Rather than return a string of 'errors' return a list of errors and format them into an aggregate.
There was a problem hiding this comment.
This should give all the errors not just the first one. We've made an effort with the rest of attribute routing to return all of the information when there's a failure.
| if (!firstTemplate.Equals(otherActionTemplate, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| var descriptions = kvp.Value.Select(ad => | ||
| Resources.FormatActionDescriptor_WithNamedAttributeRouteAndDifferentTemplate( |
There was a problem hiding this comment.
These two resource names are so different I don't know what they mean
|
⌚ |
|
Addressed most of the feedback, some feedback on the test part of the PR |
| @@ -317,6 +321,25 @@ public List<ReflectedActionDescriptor> Build(ReflectedApplicationModel model) | |||
| } | |||
| else | |||
| { | |||
| IList<ActionDescriptor> namedActionGroup; | |||
|
⌚ |
|
|
1. Added support for Name in attribute routing. Name can be defined using [RouteAttribute] and the different Http*Attributes, for example [HttpGet]. 2. Names defined on actions always override names defined on the controller. 3. Actions with a non empty template don't inherit the name from the controller. The name is only inherited from the controller when the action template is null or empty. 4. Multiple attribute routes with different templates and the same name are not allowed.
and the different Http*Attributes, for example [HttpGet].
is only inherited from the controller when the action template is null or empty.
Attribute Routing: Implement Name #734