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,