From ccc20a38c15ae4a366b8d56796c829d2630a324b Mon Sep 17 00:00:00 2001 From: jacalvar Date: Thu, 21 Aug 2014 12:49:38 -0700 Subject: [PATCH] [Fixes #734] Attribute Routing: Implement Name 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. --- .../HttpMethodAttribute.cs | 3 + .../Properties/Resources.Designer.cs | 68 +++- .../ReflectedActionDescriptorProvider.cs | 94 +++++- .../ReflectedAttributeRouteModel.cs | 26 +- src/Microsoft.AspNet.Mvc.Core/Resources.resx | 15 + .../RouteAttribute.cs | 3 + .../Routing/AttributeRoute.cs | 65 +++- .../Routing/AttributeRouteInfo.cs | 9 +- .../AttributeRouteLinkGenerationEntry.cs | 5 + .../Routing/AttributeRouting.cs | 7 +- .../Routing/IRouteTemplateProvider.cs | 6 + .../Properties/Resources.Designer.cs | 4 +- .../ReflectedActionDescriptorProviderTests.cs | 119 +++++++ .../ReflectedAttributeRouteModelTests.cs | 80 ++++- .../Routing/AttributeRouteTests.cs | 299 +++++++++++++++++- .../RoutingTests.cs | 122 ++++++- .../Controllers/CompanyController.cs | 63 ++++ .../Controllers/DuplicateController.cs | 41 +++ .../RoutingWebSite/HttpMergeAttribute.cs | 3 + test/WebSites/RoutingWebSite/Startup.cs | 9 + .../RoutingWebSite/TestResponseGenerator.cs | 3 + 21 files changed, 1020 insertions(+), 24 deletions(-) create mode 100644 test/WebSites/RoutingWebSite/Controllers/CompanyController.cs create mode 100644 test/WebSites/RoutingWebSite/Controllers/DuplicateController.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/HttpMethodAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/HttpMethodAttribute.cs index c7d96f1074..9fbdbc05fd 100644 --- a/src/Microsoft.AspNet.Mvc.Core/HttpMethodAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/HttpMethodAttribute.cs @@ -70,5 +70,8 @@ namespace Microsoft.AspNet.Mvc return _order; } } + + /// + public string Name { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs index 90327b5bbd..e444499ce1 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs @@ -1275,7 +1275,7 @@ namespace Microsoft.AspNet.Mvc.Core } /// - /// Unable to find the required services. Please add all the required services by calling '{0}' inside the call to '{1}' or before calling '{2}' in the application startup code. + /// Unable to find the required services. Please add all the required services by calling '{0}' inside the call to '{1}' or '{2}' in the application startup code. /// internal static string UnableToFindServices { @@ -1283,13 +1283,77 @@ namespace Microsoft.AspNet.Mvc.Core } /// - /// Unable to find the required services. Please add all the required services by calling '{0}' inside the call to '{1}' or before calling '{2}' in the application startup code. + /// Unable to find the required services. Please add all the required services by calling '{0}' inside the call to '{1}' or '{2}' in the application startup code. /// internal static string FormatUnableToFindServices(object p0, object p1, object p2) { return string.Format(CultureInfo.CurrentCulture, GetString("UnableToFindServices"), p0, p1, p2); } + /// + /// Two or more routes named '{0}' have different templates. + /// + internal static string AttributeRoute_DifferentLinkGenerationEntries_SameName + { + get { return GetString("AttributeRoute_DifferentLinkGenerationEntries_SameName"); } + } + + /// + /// Two or more routes named '{0}' have different templates. + /// + internal static string FormatAttributeRoute_DifferentLinkGenerationEntries_SameName(object p0) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_DifferentLinkGenerationEntries_SameName"), p0); + } + + /// + /// Action: '{0}' - Template: '{1}' + /// + internal static string AttributeRoute_DuplicateNames_Item + { + get { return GetString("AttributeRoute_DuplicateNames_Item"); } + } + + /// + /// Action: '{0}' - Template: '{1}' + /// + internal static string FormatAttributeRoute_DuplicateNames_Item(object p0, object p1) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_DuplicateNames_Item"), p0, p1); + } + + /// + /// Attribute routes with the same name '{0}' must have the same template:{1}{2} + /// + internal static string AttributeRoute_DuplicateNames + { + get { return GetString("AttributeRoute_DuplicateNames"); } + } + + /// + /// Attribute routes with the same name '{0}' must have the same template:{1}{2} + /// + internal static string FormatAttributeRoute_DuplicateNames(object p0, object p1, object p2) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_DuplicateNames"), p0, p1, p2); + } + + /// + /// Error {0}:{1}{2} + /// + internal static string AttributeRoute_AggregateErrorMessage_ErrorNumber + { + get { return GetString("AttributeRoute_AggregateErrorMessage_ErrorNumber"); } + } + + /// + /// Error {0}:{1}{2} + /// + internal static string FormatAttributeRoute_AggregateErrorMessage_ErrorNumber(object p0, object p1, object p2) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_AggregateErrorMessage_ErrorNumber"), p0, p1, p2); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs index 526d2ad2a2..0c4d37b32f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs @@ -161,7 +161,8 @@ namespace Microsoft.AspNet.Mvc var attributeRouteInfo = combinedRoute == null ? null : new AttributeRouteInfo() { Template = combinedRoute.Template, - Order = combinedRoute.Order ?? DefaultAttributeRouteOrder + Order = combinedRoute.Order ?? DefaultAttributeRouteOrder, + Name = combinedRoute.Name, }; var actionDescriptor = new ReflectedActionDescriptor() @@ -291,6 +292,9 @@ namespace Microsoft.AspNet.Mvc } } + var actionsByRouteName = new Dictionary>( + StringComparer.OrdinalIgnoreCase); + foreach (var actionDescriptor in actions) { if (actionDescriptor.AttributeRouteInfo == null || @@ -317,6 +321,25 @@ namespace Microsoft.AspNet.Mvc } else { + var attributeRouteInfo = actionDescriptor.AttributeRouteInfo; + if (attributeRouteInfo.Name != null) + { + // Build a map of attribute route name to action descriptors to ensure that all + // attribute routes with a given name have the same template. + IList namedActionGroup; + + if (actionsByRouteName.TryGetValue(attributeRouteInfo.Name, out namedActionGroup)) + { + namedActionGroup.Add(actionDescriptor); + } + else + { + namedActionGroup = new List(); + namedActionGroup.Add(actionDescriptor); + actionsByRouteName.Add(attributeRouteInfo.Name, namedActionGroup); + } + } + // We still want to add a 'null' for any constraint with DenyKey so that link generation // works properly. // @@ -332,17 +355,86 @@ namespace Microsoft.AspNet.Mvc } } + var namedRoutedErrors = ValidateNamedAttributeRoutedActions(actionsByRouteName); + if (namedRoutedErrors.Any()) + { + namedRoutedErrors = AddErrorNumbers(namedRoutedErrors); + + var message = Resources.FormatAttributeRoute_AggregateErrorMessage( + Environment.NewLine, + string.Join(Environment.NewLine + Environment.NewLine, namedRoutedErrors)); + + throw new InvalidOperationException(message); + } + if (routeTemplateErrors.Any()) { var message = Resources.FormatAttributeRoute_AggregateErrorMessage( Environment.NewLine, string.Join(Environment.NewLine + Environment.NewLine, routeTemplateErrors)); + throw new InvalidOperationException(message); } return actions; } + private static IList AddErrorNumbers(IList namedRoutedErrors) + { + return namedRoutedErrors + .Select((nre, i) => + Resources.FormatAttributeRoute_AggregateErrorMessage_ErrorNumber( + i + 1, + Environment.NewLine, + nre)) + .ToList(); + } + + private static IList ValidateNamedAttributeRoutedActions( + IDictionary> actionsGroupedByRouteName) + { + var namedRouteErrors = new List(); + + foreach (var kvp in actionsGroupedByRouteName) + { + // We are looking for attribute routed actions that have the same name but + // different route templates. We pick the first template of the group and + // we compare it against the rest of the templates that have that same name + // associated. + // The moment we find one that is different we report the whole group to the + // user in the error message so that he can see the different actions and the + // different templates for a given named attribute route. + var firstActionDescriptor = kvp.Value[0]; + var firstTemplate = firstActionDescriptor.AttributeRouteInfo.Template; + + for (var i = 1; i < kvp.Value.Count; i++) + { + var otherActionDescriptor = kvp.Value[i]; + var otherActionTemplate = otherActionDescriptor.AttributeRouteInfo.Template; + + if (!firstTemplate.Equals(otherActionTemplate, StringComparison.OrdinalIgnoreCase)) + { + var descriptions = kvp.Value.Select(ad => + Resources.FormatAttributeRoute_DuplicateNames_Item( + ad.DisplayName, + ad.AttributeRouteInfo.Template)); + + var errorDescription = string.Join(Environment.NewLine, descriptions); + var message = Resources.FormatAttributeRoute_DuplicateNames( + kvp.Key, + Environment.NewLine, + errorDescription); + + namedRouteErrors.Add(message); + break; + } + } + } + + return namedRouteErrors; + } + private static string GetRouteGroupValue(int order, string template) { var group = string.Format("{0}-{1}", order, template); diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedAttributeRouteModel.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedAttributeRouteModel.cs index a58f46e490..a0dc996058 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedAttributeRouteModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedAttributeRouteModel.cs @@ -21,12 +21,15 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder { Template = templateProvider.Template; Order = templateProvider.Order; + Name = templateProvider.Name; } public string Template { get; set; } public int? Order { get; set; } + public string Name { get; set; } + /// /// Combines two instances and returns /// a new instance with the result. @@ -60,10 +63,25 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder return new ReflectedAttributeRouteModel() { Template = combinedTemplate, - Order = right.Order ?? left.Order + Order = right.Order ?? left.Order, + Name = ChooseName(left, right), }; } + private static string ChooseName( + ReflectedAttributeRouteModel left, + ReflectedAttributeRouteModel right) + { + if (right.Name == null && string.IsNullOrEmpty(right.Template)) + { + return left.Name; + } + else + { + return right.Name; + } + } + internal static string CombineTemplates(string left, string right) { var result = CombineCore(left, right); @@ -252,7 +270,7 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder { // This is an unclosed replacement token var message = Resources.FormatAttributeRoute_TokenReplacement_InvalidSyntax( - template, + template, Resources.AttributeRoute_TokenReplacement_UnclosedToken); throw new InvalidOperationException(message); } @@ -272,7 +290,7 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder { // Unescaped left-bracket is not allowed inside a token. var message = Resources.FormatAttributeRoute_TokenReplacement_InvalidSyntax( - template, + template, Resources.AttributeRoute_TokenReplacement_UnescapedBraceInToken); throw new InvalidOperationException(message); } @@ -303,7 +321,7 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder } builder.Append(value); - + if (c == '[') { state = TemplateParserState.SeenLeft; diff --git a/src/Microsoft.AspNet.Mvc.Core/Resources.resx b/src/Microsoft.AspNet.Mvc.Core/Resources.resx index 11507e0ba0..d08914297f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Core/Resources.resx @@ -360,4 +360,19 @@ Unable to find the required services. Please add all the required services by calling '{0}' inside the call to '{1}' or '{2}' in the application startup code. + + Two or more routes named '{0}' have different templates. + + + Action: '{0}' - Template: '{1}' + Formats an action descriptor display name and it's associated template. + + + Attribute routes with the same name '{0}' must have the same template:{1}{2} + {0} is the name of the attribute route, {1} is the newline, {2} is the list of errors formatted using ActionDescriptor_WithNamedAttributeRouteAndDifferentTemplate + + + Error {0}:{1}{2} + {0} is the error number, {1} is Environment.NewLine {2} is the error message + \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/RouteAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/RouteAttribute.cs index 138215762f..217e2afdf6 100644 --- a/src/Microsoft.AspNet.Mvc.Core/RouteAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/RouteAttribute.cs @@ -46,5 +46,8 @@ namespace Microsoft.AspNet.Mvc return _order; } } + + /// + public string Name { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs index 7e53483a04..6c31c13858 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Mvc.Internal.Routing; using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Routing; @@ -20,6 +21,8 @@ namespace Microsoft.AspNet.Mvc.Routing { private readonly IRouter _next; private readonly TemplateRoute[] _matchingRoutes; + private readonly IDictionary _namedEntries; + private ILogger _logger; private ILogger _constraintLogger; private readonly LinkGenerationDecisionTree _linkGenerationTree; @@ -49,6 +52,36 @@ namespace Microsoft.AspNet.Mvc.Routing .Select(e => e.Route) .ToArray(); + var namedEntries = new Dictionary( + StringComparer.OrdinalIgnoreCase); + + foreach (var entry in linkGenerationEntries) + { + // Skip unnamed entries + if (entry.Name == null) + { + continue; + } + + // We only need to keep one AttributeRouteLinkGenerationEntry per route template + // so in case two entries have the same name and the same template we only keep + // the first entry. + AttributeRouteLinkGenerationEntry namedEntry = null; + if (namedEntries.TryGetValue(entry.Name, out namedEntry) && + !namedEntry.TemplateText.Equals(entry.TemplateText, StringComparison.OrdinalIgnoreCase)) + { + throw new ArgumentException( + Resources.FormatAttributeRoute_DifferentLinkGenerationEntries_SameName(entry.Name), + "linkGenerationEntries"); + } + else if (namedEntry == null) + { + namedEntries.Add(entry.Name, entry); + } + } + + _namedEntries = namedEntries; + // The decision tree will take care of ordering for these entries. _linkGenerationTree = new LinkGenerationDecisionTree(linkGenerationEntries.ToArray()); @@ -61,12 +94,12 @@ namespace Microsoft.AspNet.Mvc.Routing { using (_logger.BeginScope("AttributeRoute.RouteAsync")) { - foreach (var route in _matchingRoutes) - { - await route.RouteAsync(context); - - if (context.IsHandled) + foreach (var route in _matchingRoutes) { + await route.RouteAsync(context); + + if (context.IsHandled) + { break; } } @@ -85,6 +118,13 @@ namespace Microsoft.AspNet.Mvc.Routing /// public string GetVirtualPath([NotNull] VirtualPathContext context) { + // If it's a named route we will try to generate a link directly and + // if we can't, we will not try to generate it using an unnamed route. + if (context.RouteName != null) + { + return GetVirtualPathForNamedRoute(context); + } + // The decision tree will give us back all entries that match the provided route data in the correct // order. We just need to iterate them and use the first one that can generate a link. var matches = _linkGenerationTree.GetMatches(context); @@ -102,6 +142,21 @@ namespace Microsoft.AspNet.Mvc.Routing return null; } + private string GetVirtualPathForNamedRoute(VirtualPathContext context) + { + AttributeRouteLinkGenerationEntry entry; + if (_namedEntries.TryGetValue(context.RouteName, out entry)) + { + var path = GenerateLink(context, entry); + if (path != null) + { + context.IsBound = true; + return path; + } + } + return null; + } + private string GenerateLink(VirtualPathContext context, AttributeRouteLinkGenerationEntry entry) { // In attribute the context includes the values that are used to select this entry - typically diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteInfo.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteInfo.cs index 463de8c510..22544bcb86 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteInfo.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteInfo.cs @@ -14,10 +14,17 @@ namespace Microsoft.AspNet.Mvc.Routing public string Template { get; set; } /// - /// Gets the order of the route associated with this . This property determines + /// Gets the order of the route associated with a given action. This property determines /// the order in which routes get executed. Routes with a lower order value are tried first. In case a route /// doesn't specify a value, it gets a default order of 0. /// public int Order { get; set; } + + /// + /// Gets the name of the route associated with a given action. This property can be used + /// to generate a link by referring to the route by name instead of attempting to match a + /// route by provided route data. + /// + public string Name { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteLinkGenerationEntry.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteLinkGenerationEntry.cs index f088991513..8510f8e310 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteLinkGenerationEntry.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteLinkGenerationEntry.cs @@ -38,6 +38,11 @@ namespace Microsoft.AspNet.Mvc.Routing /// public decimal Precedence { get; set; } + /// + /// The name of the route. + /// + public string Name { get; set; } + /// /// The route group. /// diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs index a9922c846f..f85f46e05a 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs @@ -46,7 +46,8 @@ namespace Microsoft.AspNet.Mvc.Routing RequiredLinkValues = routeInfo.ActionDescriptor.RouteValueDefaults, RouteGroup = routeInfo.RouteGroup, Template = routeInfo.ParsedTemplate, - TemplateText = routeInfo.RouteTemplate + TemplateText = routeInfo.RouteTemplate, + Name = routeInfo.Name, }); } @@ -215,6 +216,8 @@ namespace Microsoft.AspNet.Mvc.Routing routeInfo.Precedence = AttributeRoutePrecedence.Compute(routeInfo.ParsedTemplate); + routeInfo.Name = action.AttributeRouteInfo.Name; + routeInfo.Constraints = routeInfo.ParsedTemplate.Parameters .Where(p => p.InlineConstraint != null) .ToDictionary(p => p.Name, p => p.InlineConstraint, StringComparer.OrdinalIgnoreCase); @@ -245,6 +248,8 @@ namespace Microsoft.AspNet.Mvc.Routing public string RouteGroup { get; set; } public string RouteTemplate { get; set; } + + public string Name { get; set; } } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/IRouteTemplateProvider.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/IRouteTemplateProvider.cs index 4cbd7ff3d1..bb4415b8bf 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/IRouteTemplateProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/IRouteTemplateProvider.cs @@ -20,5 +20,11 @@ namespace Microsoft.AspNet.Mvc.Routing /// route. /// int? Order { get; } + + /// + /// Gets the route name. The route name can be used to generate a link using a specific route, instead + /// of relying on selection of a route based on the given set of route values. + /// + string Name { get; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Razor.Host/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Razor.Host/Properties/Resources.Designer.cs index b1aa28a735..a8eda4a1ee 100644 --- a/src/Microsoft.AspNet.Mvc.Razor.Host/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Razor.Host/Properties/Resources.Designer.cs @@ -27,7 +27,7 @@ namespace Microsoft.AspNet.Mvc.Razor.Host } /// - /// Argument must be an instance of type '{0}'. + /// Argument must be an instance of '{0}'. /// internal static string ArgumentMustBeOfType { @@ -35,7 +35,7 @@ namespace Microsoft.AspNet.Mvc.Razor.Host } /// - /// Argument must be an instance of type '{0}'. + /// Argument must be an instance of '{0}'. /// internal static string FormatArgumentMustBeOfType(object p0) { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs index e4bc637118..e9ecffbbdb 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs @@ -265,6 +265,76 @@ namespace Microsoft.AspNet.Mvc.Test Assert.Equal(expectedMessage, ex.Message); } + [Fact] + public void AttributeRouting_Name_ThrowsIfMultipleActions_WithDifferentTemplatesHaveTheSameName() + { + // Arrange + var provider = GetProvider(typeof(SameNameDifferentTemplatesController).GetTypeInfo()); + + var expectedMessage = + "The following errors occurred with attribute routing information:" + + Environment.NewLine + Environment.NewLine + + "Error 1:" + Environment.NewLine + + "Attribute routes with the same name 'Products' must have the same template:" + + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "SameNameDifferentTemplatesController.Get' - Template: 'Products'" + + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "SameNameDifferentTemplatesController.Get' - Template: 'Products/{id}'" + + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "SameNameDifferentTemplatesController.Put' - Template: 'Products/{id}'" + + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "SameNameDifferentTemplatesController.Post' - Template: 'Products'" + + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "SameNameDifferentTemplatesController.Delete' - Template: 'Products/{id}'" + + Environment.NewLine + Environment.NewLine + + "Error 2:" + Environment.NewLine + + "Attribute routes with the same name 'Items' must have the same template:" + + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "SameNameDifferentTemplatesController.GetItems' - Template: 'Items/{id}'" + + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "SameNameDifferentTemplatesController.PostItems' - Template: 'Items'" + + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "SameNameDifferentTemplatesController.PutItems' - Template: 'Items/{id}'" + + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "SameNameDifferentTemplatesController.DeleteItems' - Template: 'Items/{id}'" + + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "SameNameDifferentTemplatesController.PatchItems' - Template: 'Items'"; + + // Act + var ex = Assert.Throws(() => { provider.GetDescriptors(); }); + + // Assert + Assert.Equal(expectedMessage, ex.Message); + } + + [Fact] + public void AttributeRouting_Name_AllowsMultipleAttributeRoutesInDifferentActions_WithTheSameNameAndTemplate() + { + // Arrange + var provider = GetProvider(typeof(DifferentCasingsAttributeRouteNamesController).GetTypeInfo()); + + // Act + var descriptors = provider.GetDescriptors(); + + // Assert + foreach (var descriptor in descriptors) + { + Assert.NotNull(descriptor.AttributeRouteInfo); + Assert.Equal("{id}", descriptor.AttributeRouteInfo.Template, StringComparer.OrdinalIgnoreCase); + Assert.Equal("Products", descriptor.AttributeRouteInfo.Name, StringComparer.OrdinalIgnoreCase); + } + } + [Fact] public void AttributeRouting_RouteGroupConstraint_IsAddedOnceForNonAttributeRoutes() { @@ -564,6 +634,55 @@ namespace Microsoft.AspNet.Mvc.Test public void AnotherNonAttributedAction() { } } + [Route("Products", Name = "Products")] + public class SameNameDifferentTemplatesController + { + [HttpGet] + public void Get() { } + + [HttpGet("{id}", Name = "Products")] + public void Get(int id) { } + + [HttpPut("{id}", Name = "Products")] + public void Put(int id) { } + + [HttpPost] + public void Post() { } + + [HttpDelete("{id}", Name = "Products")] + public void Delete(int id) { } + + [HttpGet("/Items/{id}", Name = "Items")] + public void GetItems(int id) { } + + [HttpPost("/Items", Name = "Items")] + public void PostItems() { } + + [HttpPut("/Items/{id}", Name = "Items")] + public void PutItems(int id) { } + + [HttpDelete("/Items/{id}", Name = "Items")] + public void DeleteItems(int id) { } + + [HttpPatch("/Items", Name = "Items")] + public void PatchItems() { } + } + + public class DifferentCasingsAttributeRouteNamesController + { + [HttpGet("{id}", Name = "Products")] + public void Get() { } + + [HttpGet("{ID}", Name = "Products")] + public void Get(int id) { } + + [HttpPut("{id}", Name = "PRODUCTS")] + public void Put(int id) { } + + [HttpDelete("{ID}", Order = 1, Name = "PRODUCTS")] + public void Delete(int id) { } + } + [MyRouteConstraint(blockNonAttributedActions: true)] [MySecondRouteConstraint(blockNonAttributedActions: true)] private class ConstrainedController diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedAttributeRouteModelTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedAttributeRouteModelTests.cs index 304b6a9a90..2ad2d8952d 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedAttributeRouteModelTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedAttributeRouteModelTests.cs @@ -229,10 +229,79 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder Assert.Equal(combined.Order, right.Order); } + [Theory] + [MemberData("CombineNamesTestData")] + public void Combine_Names( + ReflectedAttributeRouteModel left, + ReflectedAttributeRouteModel right, + string expectedName) + { + // Arrange & Act + var combined = ReflectedAttributeRouteModel.CombineReflectedAttributeRouteModel(left, right); + + // Assert + Assert.NotNull(combined); + Assert.Equal(expectedName, combined.Name); + } + + public static IEnumerable CombineNamesTestData + { + get + { + // AttributeRoute on the controller, attribute route on the action, expected name. + var data = new TheoryData(); + + // Combined name is null if no name is provided. + data.Add(Create(template: "/", order: null, name: null), null, null); + data.Add(Create(template: "~/", order: null, name: null), null, null); + data.Add(Create(template: "", order: null, name: null), null, null); + data.Add(Create(template: "home", order: null, name: null), null, null); + data.Add(Create(template: "/", order: 1, name: null), null, null); + data.Add(Create(template: "~/", order: 1, name: null), null, null); + data.Add(Create(template: "", order: 1, name: null), null, null); + data.Add(Create(template: "home", order: 1, name: null), null, null); + + // Combined name is inherited if no right name is provided and the template is empty. + data.Add(Create(template: "/", order: null, name: "Named"), null, "Named"); + data.Add(Create(template: "~/", order: null, name: "Named"), null, "Named"); + data.Add(Create(template: "", order: null, name: "Named"), null, "Named"); + data.Add(Create(template: "home", order: null, name: "Named"), null, "Named"); + data.Add(Create(template: "home", order: null, name: "Named"), Create(null, null, null), "Named"); + data.Add(Create(template: "", order: null, name: "Named"), Create("", null, null), "Named"); + + // Order doesn't matter for combining the name. + data.Add(Create(template: "", order: null, name: "Named"), Create("", 1, null), "Named"); + data.Add(Create(template: "", order: 1, name: "Named"), Create("", 1, null), "Named"); + data.Add(Create(template: "", order: 2, name: "Named"), Create("", 1, null), "Named"); + data.Add(Create(template: "", order: null, name: "Named"), Create("index", 1, null), null); + data.Add(Create(template: "", order: 1, name: "Named"), Create("index", 1, null), null); + data.Add(Create(template: "", order: 2, name: "Named"), Create("index", 1, null), null); + data.Add(Create(template: "", order: null, name: "Named"), Create("", 1, "right"), "right"); + data.Add(Create(template: "", order: 1, name: "Named"), Create("", 1, "right"), "right"); + data.Add(Create(template: "", order: 2, name: "Named"), Create("", 1, "right"), "right"); + + // Combined name is not inherited if right name is provided or the template is not empty. + data.Add(Create(template: "/", order: null, name: "Named"), Create(null, null, "right"), "right"); + data.Add(Create(template: "~/", order: null, name: "Named"), Create(null, null, "right"), "right"); + data.Add(Create(template: "", order: null, name: "Named"), Create(null, null, "right"), "right"); + data.Add(Create(template: "home", order: null, name: "Named"), Create(null, null, "right"), "right"); + data.Add(Create(template: "home", order: null, name: "Named"), Create("index", null, null), null); + data.Add(Create(template: "home", order: null, name: "Named"), Create("/", null, null), null); + data.Add(Create(template: "home", order: null, name: "Named"), Create("~/", null, null), null); + data.Add(Create(template: "home", order: null, name: "Named"), Create("index", null, "right"), "right"); + data.Add(Create(template: "home", order: null, name: "Named"), Create("/", null, "right"), "right"); + data.Add(Create(template: "home", order: null, name: "Named"), Create("~/", null, "right"), "right"); + data.Add(Create(template: "home", order: null, name: "Named"), Create("index", null, ""), ""); + + return data; + } + } + public static IEnumerable CombineOrdersTestData { get { + // AttributeRoute on the controller, attribute route on the action, expected order. var data = new TheoryData(); data.Add(Create("", order: 1), Create("", order: 2), 2); @@ -261,6 +330,7 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder { get { + // AttributeRoute on the controller, attribute route on the action. var data = new TheoryData(); var leftModel = Create("Home", order: 3); @@ -279,7 +349,9 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder { get { + // AttributeRoute on the controller, attribute route on the action. var data = new TheoryData(); + data.Add(null, null); data.Add(null, Create(null)); data.Add(Create(null), null); @@ -293,9 +365,10 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder { get { + // AttributeRoute on the controller, attribute route on the action, expected combined attribute route. var data = new TheoryData(); data.Add(null, Create("Index"), Create("Index")); - data.Add(Create(null), Create("Index"), Create("Index"));; + data.Add(Create(null), Create("Index"), Create("Index")); data.Add(Create("Home"), null, Create("Home")); data.Add(Create("Home"), Create(null), Create("Home")); data.Add(Create("Home"), Create("Index"), Create("Home/Index")); @@ -451,12 +524,13 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder } } - private static ReflectedAttributeRouteModel Create(string template, int? order = null) + private static ReflectedAttributeRouteModel Create(string template, int? order = null, string name = null) { return new ReflectedAttributeRouteModel { Template = template, - Order = order + Order = order, + Name = name }; } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTests.cs index a23054ff22..4582759f87 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTests.cs @@ -379,6 +379,289 @@ namespace Microsoft.AspNet.Mvc.Routing Assert.Equal(expectedGroup, selectedGroup); } + public static IEnumerable NamedEntriesWithDifferentTemplates + { + get + { + var data = new TheoryData>(); + data.Add(new[] + { + CreateGenerationEntry("template", null, 0, "NamedEntry"), + CreateGenerationEntry("otherTemplate", null, 0, "NamedEntry"), + CreateGenerationEntry("anotherTemplate", null, 0, "NamedEntry") + }); + + // Default values for parameters are taken into account by comparing the templates. + data.Add(new[] + { + CreateGenerationEntry("template/{parameter=0}", null, 0, "NamedEntry"), + CreateGenerationEntry("template/{parameter=1}", null, 0, "NamedEntry"), + CreateGenerationEntry("template/{parameter=2}", null, 0, "NamedEntry") + }); + + // Names for entries are compared ignoring casing. + data.Add(new[] + { + CreateGenerationEntry("template/{*parameter:int=0}", null, 0, "NamedEntry"), + CreateGenerationEntry("template/{*parameter:int=1}", null, 0, "NAMEDENTRY"), + CreateGenerationEntry("template/{*parameter:int=2}", null, 0, "namedentry") + }); + return data; + } + } + + [Theory] + [MemberData(nameof(AttributeRouteTest.NamedEntriesWithDifferentTemplates))] + public void AttributeRoute_CreateAttributeRoute_ThrowsIfDifferentEntriesHaveTheSameName( + IEnumerable namedEntries) + { + // Arrange + string expectedExceptionMessage = "Two or more routes named 'NamedEntry' have different templates." + + Environment.NewLine + + "Parameter name: linkGenerationEntries"; + + var next = new Mock().Object; + + var matchingEntries = Enumerable.Empty(); + + // Act + var exception = Assert.Throws( + "linkGenerationEntries", + () => new AttributeRoute( + next, + matchingEntries, + namedEntries, + NullLoggerFactory.Instance)); + + Assert.Equal(expectedExceptionMessage, exception.Message, StringComparer.OrdinalIgnoreCase); + } + + public static IEnumerable NamedEntriesWithTheSameTemplate + { + get + { + var data = new TheoryData>(); + + data.Add(new[] + { + CreateGenerationEntry("template", null, 0, "NamedEntry"), + CreateGenerationEntry("template", null, 1, "NamedEntry"), + CreateGenerationEntry("template", null, 2, "NamedEntry") + }); + + // Templates are compared ignoring casing. + data.Add(new[] + { + CreateGenerationEntry("template", null, 0, "NamedEntry"), + CreateGenerationEntry("Template", null, 1, "NamedEntry"), + CreateGenerationEntry("TEMPLATE", null, 2, "NamedEntry") + }); + + data.Add(new[] + { + CreateGenerationEntry("template/{parameter=0}", null, 0, "NamedEntry"), + CreateGenerationEntry("template/{parameter=0}", null, 1, "NamedEntry"), + CreateGenerationEntry("template/{parameter=0}", null, 2, "NamedEntry") + }); + + return data; + } + } + + [Theory] + [MemberData(nameof(AttributeRouteTest.NamedEntriesWithTheSameTemplate))] + public void AttributeRoute_GeneratesLink_ForMultipleNamedEntriesWithTheSameTemplate( + IEnumerable namedEntries) + { + // Arrange + var expectedLink = namedEntries.First().Template.Parameters.Any() ? "template/5" : "template"; + + var expectedGroup = "0&" + namedEntries.First().TemplateText; + string selectedGroup = null; + var next = new Mock(); + next.Setup(s => s.GetVirtualPath(It.IsAny())) + .Callback(vpc => + { + vpc.IsBound = true; + selectedGroup = (string)vpc.ProvidedValues[AttributeRouting.RouteGroupKey]; + }); + + var matchingEntries = Enumerable.Empty(); + + var route = new AttributeRoute( + next.Object, + matchingEntries, + namedEntries, + NullLoggerFactory.Instance); + + var ambientValues = namedEntries.First().Template.Parameters.Any() ? new { parameter = 5 } : null; + + var context = CreateVirtualPathContext(values: null, ambientValues: ambientValues, name: "NamedEntry"); + + // Act + var result = route.GetVirtualPath(context); + + // Assert + Assert.NotNull(result); + Assert.Equal(expectedGroup, selectedGroup); + Assert.Equal(expectedLink, result); + } + + [Fact] + public void AttributeRoute_GenerateLink_WithName() + { + // Arrange + string selectedGroup = null; + var next = new Mock(); + next.Setup(s => s.GetVirtualPath(It.IsAny())) + .Callback(vpc => + { + vpc.IsBound = true; + selectedGroup = (string)vpc.ProvidedValues[AttributeRouting.RouteGroupKey]; + }); + + var namedEntry = CreateGenerationEntry("named", requiredValues: null, order: 1, name: "NamedRoute"); + var unnamedEntry = CreateGenerationEntry("unnamed", requiredValues: null, order: 0); + + // The named route has a lower order which will ensure that we aren't trying the route as + // if it were an unnamed route. + var linkGenerationEntries = new[] { namedEntry, unnamedEntry }; + + var matchingEntries = Enumerable.Empty(); + + var route = new AttributeRoute(next.Object, matchingEntries, linkGenerationEntries, NullLoggerFactory.Instance); + + var context = CreateVirtualPathContext(values: null, ambientValues: null, name: "NamedRoute"); + + // Act + var result = route.GetVirtualPath(context); + + // Assert + Assert.NotNull(result); + Assert.Equal("1&named", selectedGroup); + Assert.Equal("named", result); + } + + [Fact] + public void AttributeRoute_DoesNotGenerateLink_IfThereIsNoRouteForAGivenName() + { + // Arrange + string selectedGroup = null; + var next = new Mock(); + next.Setup(s => s.GetVirtualPath(It.IsAny())) + .Callback(vpc => + { + vpc.IsBound = true; + selectedGroup = (string)vpc.ProvidedValues[AttributeRouting.RouteGroupKey]; + }); + + var namedEntry = CreateGenerationEntry("named", requiredValues: null, order: 1, name: "NamedRoute"); + + // Add an unnamed entry to ensure we don't fall back to generating a link for an unnamed route. + var unnamedEntry = CreateGenerationEntry("unnamed", requiredValues: null, order: 0); + + // The named route has a lower order which will ensure that we aren't trying the route as + // if it were an unnamed route. + var linkGenerationEntries = new[] { namedEntry, unnamedEntry }; + + var matchingEntries = Enumerable.Empty(); + + var route = new AttributeRoute(next.Object, matchingEntries, linkGenerationEntries, NullLoggerFactory.Instance); + + var context = CreateVirtualPathContext(values: null, ambientValues: null, name: "NonExistingNamedRoute"); + + // Act + var result = route.GetVirtualPath(context); + + // Assert + Assert.Null(result); + } + + [Theory] + [InlineData("template/{parameter:int}", null)] + [InlineData("template/{parameter:int}", "NaN")] + [InlineData("template/{parameter}", null)] + [InlineData("template/{*parameter:int}", null)] + [InlineData("template/{*parameter:int}", "NaN")] + public void AttributeRoute_DoesNotGenerateLink_IfValuesDoNotMatchNamedEntry(string template, string value) + { + // Arrange + string selectedGroup = null; + var next = new Mock(); + next.Setup(s => s.GetVirtualPath(It.IsAny())) + .Callback(vpc => + { + vpc.IsBound = true; + selectedGroup = (string)vpc.ProvidedValues[AttributeRouting.RouteGroupKey]; + }); + + var namedEntry = CreateGenerationEntry(template, requiredValues: null, order: 1, name: "NamedRoute"); + + // Add an unnamed entry to ensure we don't fall back to generating a link for an unnamed route. + var unnamedEntry = CreateGenerationEntry("unnamed", requiredValues: null, order: 0); + + // The named route has a lower order which will ensure that we aren't trying the route as + // if it were an unnamed route. + var linkGenerationEntries = new[] { namedEntry, unnamedEntry }; + + var matchingEntries = Enumerable.Empty(); + + var route = new AttributeRoute(next.Object, matchingEntries, linkGenerationEntries, NullLoggerFactory.Instance); + + var ambientValues = value == null ? null : new { parameter = value }; + + var context = CreateVirtualPathContext(values: null, ambientValues: ambientValues, name: "NamedRoute"); + + // Act + var result = route.GetVirtualPath(context); + + // Assert + Assert.Null(result); + } + + [Theory] + [InlineData("template/{parameter:int}", "5")] + [InlineData("template/{parameter}", "5")] + [InlineData("template/{*parameter:int}", "5")] + [InlineData("template/{*parameter}", "5")] + public void AttributeRoute_GeneratesLink_IfValuesMatchNamedEntry(string template, string value) + { + // Arrange + string selectedGroup = null; + var next = new Mock(); + next.Setup(s => s.GetVirtualPath(It.IsAny())) + .Callback(vpc => + { + vpc.IsBound = true; + selectedGroup = (string)vpc.ProvidedValues[AttributeRouting.RouteGroupKey]; + }); + + var namedEntry = CreateGenerationEntry(template, requiredValues: null, order: 1, name: "NamedRoute"); + + // Add an unnamed entry to ensure we don't fall back to generating a link for an unnamed route. + var unnamedEntry = CreateGenerationEntry("unnamed", requiredValues: null, order: 0); + + // The named route has a lower order which will ensure that we aren't trying the route as + // if it were an unnamed route. + var linkGenerationEntries = new[] { namedEntry, unnamedEntry }; + + var matchingEntries = Enumerable.Empty(); + + var route = new AttributeRoute(next.Object, matchingEntries, linkGenerationEntries, NullLoggerFactory.Instance); + + var ambientValues = value == null ? null : new { parameter = value }; + + var context = CreateVirtualPathContext(values: null, ambientValues: ambientValues, name: "NamedRoute"); + + // Act + var result = route.GetVirtualPath(context); + + // Assert + Assert.NotNull(result); + Assert.Equal(string.Format("1&{0}", template), selectedGroup); + Assert.Equal("template/5", result); + } + [Fact] public async void AttributeRoute_RouteAsyncHandled_LogsCorrectValues() { @@ -843,7 +1126,10 @@ namespace Microsoft.AspNet.Mvc.Routing return new RouteContext(context.Object); } - private static VirtualPathContext CreateVirtualPathContext(object values, object ambientValues = null) + private static VirtualPathContext CreateVirtualPathContext( + object values, + object ambientValues = null, + string name = null) { var mockHttpContext = new Mock(); mockHttpContext.Setup(h => h.RequestServices.GetService(typeof(ILoggerFactory))) @@ -852,7 +1138,8 @@ namespace Microsoft.AspNet.Mvc.Routing return new VirtualPathContext( mockHttpContext.Object, new RouteValueDictionary(ambientValues), - new RouteValueDictionary(values)); + new RouteValueDictionary(values), + name); } private static AttributeRouteMatchingEntry CreateMatchingEntry(IRouter router, string template, int order) @@ -872,7 +1159,11 @@ namespace Microsoft.AspNet.Mvc.Routing return entry; } - private static AttributeRouteLinkGenerationEntry CreateGenerationEntry(string template, object requiredValues, int order = 0) + private static AttributeRouteLinkGenerationEntry CreateGenerationEntry( + string template, + object requiredValues, + int order = 0, + string name = null) { var constraintResolver = CreateConstraintResolver(); @@ -895,7 +1186,7 @@ namespace Microsoft.AspNet.Mvc.Routing entry.Precedence = AttributeRoutePrecedence.Compute(entry.Template); entry.RequiredLinkValues = new RouteValueDictionary(requiredValues); entry.RouteGroup = CreateRouteGroup(order, template); - + entry.Name = name; return entry; } diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs index df02fc80e7..08ab08e9e4 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs @@ -529,6 +529,124 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.Equal("/", result.Link); } + [Theory] + [InlineData("GET", "Get")] + [InlineData("PUT", "Put")] + public async Task AttributeRoutedAction_LinkWithName_WithNameInheritedFromControllerRoute(string method, string actionName) + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.Handler; + + // Act + var response = await client.SendAsync(method, "http://localhost/api/Company/5"); + Assert.Equal(200, response.StatusCode); + + // Assert + var body = await response.ReadBodyAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + // Assert + Assert.Equal("Company", result.Controller); + Assert.Equal(actionName, result.Action); + + Assert.Equal("/api/Company/5", result.ExpectedUrls.Single()); + Assert.Equal("Company", result.RouteName); + } + + [Fact] + public async Task AttributeRoutedAction_LinkWithName_WithNameOverrridenFromController() + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.Handler; + + // Act + var response = await client.SendAsync("DELETE", "http://localhost/api/Company/5"); + Assert.Equal(200, response.StatusCode); + + // Assert + var body = await response.ReadBodyAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + // Assert + Assert.Equal("Company", result.Controller); + Assert.Equal("Delete", result.Action); + + Assert.Equal("/api/Company/5", result.ExpectedUrls.Single()); + Assert.Equal("RemoveCompany", result.RouteName); + } + + [Fact] + public async Task AttributeRoutedAction_Link_WithNonEmptyActionRouteTemplateAndNoActionRouteName() + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.Handler; + + var url = LinkFrom("http://localhost") + .To(new { id = 5 }); + + // Act + var response = await client.SendAsync("GET", "http://localhost/api/Company/5/Employees"); + Assert.Equal(200, response.StatusCode); + + // Assert + var body = await response.ReadBodyAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + // Assert + Assert.Equal("Company", result.Controller); + Assert.Equal("GetEmployees", result.Action); + + Assert.Equal("/api/Company/5/Employees", result.ExpectedUrls.Single()); + Assert.Equal(null, result.RouteName); + } + + [Fact] + public async Task AttributeRoutedAction_LinkWithName_WithNonEmptyActionRouteTemplateAndActionRouteName() + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.Handler; + + // Act + var response = await client.SendAsync("GET", "http://localhost/api/Company/5/Departments"); + Assert.Equal(200, response.StatusCode); + + // Assert + var body = await response.ReadBodyAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + // Assert + Assert.Equal("Company", result.Controller); + Assert.Equal("GetDepartments", result.Action); + + Assert.Equal("/api/Company/5/Departments", result.ExpectedUrls.Single()); + Assert.Equal("Departments", result.RouteName); + } + + [Theory] + [InlineData("http://localhost/Duplicate/Index")] + [InlineData("http://localhost/api/Duplicate/IndexAttribute")] + [InlineData("http://localhost/api/Duplicate")] + [InlineData("http://localhost/conventional/Duplicate")] + public async Task AttributeRoutedAction_ThowsIfConventionalRouteWithTheSameName(string url) + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.Handler; + + var expectedMessage = "The supplied route name 'DuplicateRoute' is ambiguous and matched more than one route."; + + // Act + var ex = await Assert.ThrowsAsync(async () => + await client.SendAsync("GET", url)); + + // Assert + Assert.Equal(expectedMessage, ex.Message); + } + [Fact] public async Task ConventionalRoutedAction_LinkToArea() { @@ -869,7 +987,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var client = server.Handler; // Act - var url = + var url = LinkFrom("http://localhost/") .To(new { action = "GetProducts", controller = "Products", country = "US" }); var response = await client.GetAsync(url); @@ -935,6 +1053,8 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests public Dictionary RouteValues { get; set; } + public string RouteName { get; set; } + public string Action { get; set; } public string Controller { get; set; } diff --git a/test/WebSites/RoutingWebSite/Controllers/CompanyController.cs b/test/WebSites/RoutingWebSite/Controllers/CompanyController.cs new file mode 100644 index 0000000000..85d8f1a214 --- /dev/null +++ b/test/WebSites/RoutingWebSite/Controllers/CompanyController.cs @@ -0,0 +1,63 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNet.Mvc; + +namespace RoutingWebSite +{ + // A controller can define a route for all of the actions + // in it and give it a name for link generation purposes. + [Route("api/Company/{id}", Name = "Company")] + public class CompanyController : Controller + { + private readonly TestResponseGenerator _generator; + + public CompanyController(TestResponseGenerator generator) + { + _generator = generator; + } + + // An action with the same template will inherit the name + // from the controller. + [HttpGet] + public ActionResult Get(int id) + { + return _generator.Generate(Url.RouteUrl("Company", new { id = id })); + } + + // Multiple actions can have the same named route as long + // as for a given Name, all the actions have the same template. + // That is, there can't be two link generation entries with same + // name and different templates. + [HttpPut] + public ActionResult Put(int id) + { + return _generator.Generate(Url.RouteUrl("Company", new { id = id })); + } + + // Two actions can have the same template and each of them can have + // a different route name. That is, a given template can have multiple + // names associated with it. + [HttpDelete(Name = "RemoveCompany")] + public ActionResult Delete(int id) + { + return _generator.Generate(Url.RouteUrl("RemoveCompany", new { id = id })); + } + + // An action that defines a non empty template doesn't inherit the name + // from the route on the controller . + [HttpGet("Employees")] + public ActionResult GetEmployees(int id) + { + return _generator.Generate(Url.RouteUrl(new { id = id })); + } + + // An action that defines a non empty template doesn't inherit the name + // from the controller but can perfectly define its own name. + [HttpGet("Departments", Name = "Departments")] + public ActionResult GetDepartments(int id) + { + return _generator.Generate(Url.RouteUrl("Departments", new { id = id })); + } + } +} \ No newline at end of file diff --git a/test/WebSites/RoutingWebSite/Controllers/DuplicateController.cs b/test/WebSites/RoutingWebSite/Controllers/DuplicateController.cs new file mode 100644 index 0000000000..d0e5ded0e2 --- /dev/null +++ b/test/WebSites/RoutingWebSite/Controllers/DuplicateController.cs @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNet.Mvc; +using System; + +namespace RoutingWebSite +{ + public class DuplicateController : Controller + { + private readonly TestResponseGenerator _generator; + + public DuplicateController(TestResponseGenerator generator) + { + _generator = generator; + } + + [HttpGet("api/Duplicate/", Name = "DuplicateRoute")] + public ActionResult DuplicateAttribute() + { + return _generator.Generate(Url.RouteUrl("DuplicateRoute")); + } + + [HttpGet("api/Duplicate/IndexAttribute")] + public ActionResult IndexAttribute() + { + return _generator.Generate(Url.RouteUrl("DuplicateRoute")); + } + + [HttpGet] + public ActionResult Duplicate() + { + return _generator.Generate(Url.RouteUrl("DuplicateRoute")); + } + + public ActionResult Index() + { + return _generator.Generate(Url.RouteUrl("DuplicateRoute")); + } + } +} \ No newline at end of file diff --git a/test/WebSites/RoutingWebSite/HttpMergeAttribute.cs b/test/WebSites/RoutingWebSite/HttpMergeAttribute.cs index d497aea828..d538dd4898 100644 --- a/test/WebSites/RoutingWebSite/HttpMergeAttribute.cs +++ b/test/WebSites/RoutingWebSite/HttpMergeAttribute.cs @@ -29,5 +29,8 @@ namespace RoutingWebSite /// public int? Order { get; set; } + + /// + public string Name { get; set; } } } \ No newline at end of file diff --git a/test/WebSites/RoutingWebSite/Startup.cs b/test/WebSites/RoutingWebSite/Startup.cs index 65aee1009a..011532f8a7 100644 --- a/test/WebSites/RoutingWebSite/Startup.cs +++ b/test/WebSites/RoutingWebSite/Startup.cs @@ -33,6 +33,15 @@ namespace RoutingWebSite "products", "api/Products/{country}/{action}", defaults: new { controller = "Products" }); + + // Added this route to validate that we throw an exception when a conventional + // route matches a link generated by a named attribute route. + // The conventional route will match first, but when the attribute route generates + // a valid route an exception will be thrown. + routes.MapRoute( + "DuplicateRoute", + "conventional/Duplicate", + defaults: new { controller = "Duplicate", action = "Duplicate" }); }); } } diff --git a/test/WebSites/RoutingWebSite/TestResponseGenerator.cs b/test/WebSites/RoutingWebSite/TestResponseGenerator.cs index 8ab5b8ed5e..b5acea56e9 100644 --- a/test/WebSites/RoutingWebSite/TestResponseGenerator.cs +++ b/test/WebSites/RoutingWebSite/TestResponseGenerator.cs @@ -37,10 +37,13 @@ namespace RoutingWebSite link = urlHelper.Action(query["link_action"], query["link_controller"], values); } + var attributeRoutingInfo = _actionContext.ActionDescriptor.AttributeRouteInfo; + return new JsonResult(new { expectedUrls = expectedUrls, actualUrl = _actionContext.HttpContext.Request.Path.Value, + routeName = attributeRoutingInfo == null ? null : attributeRoutingInfo.Name, routeValues = new Dictionary(_actionContext.RouteData.Values), action = _actionContext.ActionDescriptor.Name,