diff --git a/samples/MvcSample.Web/SimpleRest.cs b/samples/MvcSample.Web/SimpleRest.cs index 3f84c8cdd6..e98e1ef4cd 100644 --- a/samples/MvcSample.Web/SimpleRest.cs +++ b/samples/MvcSample.Web/SimpleRest.cs @@ -11,10 +11,11 @@ namespace MvcSample.Web return "Get method"; } - [HttpGet("OtherThing")] + [HttpGet("[action]")] public string GetOtherThing() { - return "Get other thing"; + // Will be GetOtherThing + return (string)ActionContext.RouteData.Values["action"]; } [HttpGet("Link")] diff --git a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs index 864be77022..e20173e1d3 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs @@ -1066,6 +1066,149 @@ namespace Microsoft.AspNet.Mvc.Core return string.Format(CultureInfo.CurrentCulture, GetString("OutputFormatterNoEncoding"), p0); } + /// The following errors occurred with attribute routing information:{0}{0}{1} + /// + internal static string AttributeRoute_AggregateErrorMessage + { + get { return GetString("AttributeRoute_AggregateErrorMessage"); } + } + + /// + /// The following errors occurred with attribute routing information:{0}{0}{1} + /// + internal static string FormatAttributeRoute_AggregateErrorMessage(object p0, object p1) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_AggregateErrorMessage"), p0, p1); + } + + /// + /// The attribute route '{0}' cannot contain a parameter named '{{{1}}}'. Use '[{1}]' in the route template to insert the value '{2}'. + /// + internal static string AttributeRoute_CannotContainParameter + { + get { return GetString("AttributeRoute_CannotContainParameter"); } + } + + /// + /// The attribute route '{0}' cannot contain a parameter named '{{{1}}}'. Use '[{1}]' in the route template to insert the value '{2}'. + /// + internal static string FormatAttributeRoute_CannotContainParameter(object p0, object p1, object p2) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_CannotContainParameter"), p0, p1, p2); + } + + /// + /// For action: '{0}'{1}Error: {2} + /// + internal static string AttributeRoute_IndividualErrorMessage + { + get { return GetString("AttributeRoute_IndividualErrorMessage"); } + } + + /// + /// For action: '{0}'{1}Error: {2} + /// + internal static string FormatAttributeRoute_IndividualErrorMessage(object p0, object p1, object p2) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_IndividualErrorMessage"), p0, p1, p2); + } + + /// + /// An empty replacement token ('[]') is not allowed. + /// + internal static string AttributeRoute_TokenReplacement_EmptyTokenNotAllowed + { + get { return GetString("AttributeRoute_TokenReplacement_EmptyTokenNotAllowed"); } + } + + /// + /// An empty replacement token ('[]') is not allowed. + /// + internal static string FormatAttributeRoute_TokenReplacement_EmptyTokenNotAllowed() + { + return GetString("AttributeRoute_TokenReplacement_EmptyTokenNotAllowed"); + } + + /// + /// Token delimiters ('[', ']') are imbalanced. + /// + internal static string AttributeRoute_TokenReplacement_ImbalancedSquareBrackets + { + get { return GetString("AttributeRoute_TokenReplacement_ImbalancedSquareBrackets"); } + } + + /// + /// Token delimiters ('[', ']') are imbalanced. + /// + internal static string FormatAttributeRoute_TokenReplacement_ImbalancedSquareBrackets() + { + return GetString("AttributeRoute_TokenReplacement_ImbalancedSquareBrackets"); + } + + /// + /// The route template '{0}' has invalid syntax. {1} + /// + internal static string AttributeRoute_TokenReplacement_InvalidSyntax + { + get { return GetString("AttributeRoute_TokenReplacement_InvalidSyntax"); } + } + + /// + /// The route template '{0}' has invalid syntax. {1} + /// + internal static string FormatAttributeRoute_TokenReplacement_InvalidSyntax(object p0, object p1) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_TokenReplacement_InvalidSyntax"), p0, p1); + } + + /// + /// While processing template '{0}', a replacement value for the token '{1}' could not be found. Available tokens: '{2}'. + /// + internal static string AttributeRoute_TokenReplacement_ReplacementValueNotFound + { + get { return GetString("AttributeRoute_TokenReplacement_ReplacementValueNotFound"); } + } + + /// + /// While processing template '{0}', a replacement value for the token '{1}' could not be found. Available tokens: '{2}'. + /// + internal static string FormatAttributeRoute_TokenReplacement_ReplacementValueNotFound(object p0, object p1, object p2) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_TokenReplacement_ReplacementValueNotFound"), p0, p1, p2); + } + + /// + /// A replacement token is not closed. + /// + internal static string AttributeRoute_TokenReplacement_UnclosedToken + { + get { return GetString("AttributeRoute_TokenReplacement_UnclosedToken"); } + } + + /// + /// A replacement token is not closed. + /// + internal static string FormatAttributeRoute_TokenReplacement_UnclosedToken() + { + return GetString("AttributeRoute_TokenReplacement_UnclosedToken"); + } + + /// + /// An unescaped '[' token is not allowed inside of a replacement token. Use '[[' to escape. + /// + internal static string AttributeRoute_TokenReplacement_UnescapedBraceInToken + { + get { return GetString("AttributeRoute_TokenReplacement_UnescapedBraceInToken"); } + } + + /// + /// An unescaped '[' token is not allowed inside of a replacement token. Use '[[' to escape. + /// + internal static string FormatAttributeRoute_TokenReplacement_UnescapedBraceInToken() + { + return GetString("AttributeRoute_TokenReplacement_UnescapedBraceInToken"); + } + 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 d5e668f70e..b0abaf3b0d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs @@ -7,10 +7,10 @@ using System.Linq; #if K10 using System.Reflection; #endif +using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Mvc.ReflectedModelBuilder; using Microsoft.AspNet.Mvc.Routing; using Microsoft.AspNet.Routing; -using Microsoft.AspNet.Routing.Template; using Microsoft.Framework.OptionsModel; namespace Microsoft.AspNet.Mvc @@ -112,12 +112,13 @@ namespace Microsoft.AspNet.Mvc public List Build(ReflectedApplicationModel model) { - var routeGroupsByTemplate = GetRouteGroupsByTemplate(model); - var actions = new List(); + var routeGroupsByTemplate = new Dictionary(StringComparer.OrdinalIgnoreCase); var removalConstraints = new HashSet(StringComparer.OrdinalIgnoreCase); + var routeTemplateErrors = new List(); + foreach (var controller in model.Controllers) { var controllerDescriptor = new ControllerDescriptor(controller.ControllerType); @@ -136,7 +137,7 @@ namespace Microsoft.AspNet.Mvc ParameterBindingInfo = isFromBody ? null : new ParameterBindingInfo( - parameter.ParameterName, + parameter.ParameterName, parameter.ParameterInfo.ParameterType), BodyParameterInfo = isFromBody @@ -201,56 +202,54 @@ namespace Microsoft.AspNet.Mvc } } - if (routeGroupsByTemplate.Any()) + var templateText = AttributeRouteTemplate.Combine( + controller.RouteTemplate, + action.RouteTemplate); + + if (templateText != null) { - var templateText = AttributeRouteTemplate.Combine( - controller.RouteTemplate, - action.RouteTemplate); - - if (templateText == null) + // An attribute routed action will ignore conventional routed constraints. We still + // want to provide these values as ambient values. + foreach (var constraint in actionDescriptor.RouteConstraints) { - // A conventional routed action can't match any route group. - actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint( - AttributeRouting.RouteGroupKey, - RouteKeyHandling.DenyKey)); + actionDescriptor.RouteValueDefaults.Add(constraint.RouteKey, constraint.RouteValue); } - else + + // Replaces tokens like [controller]/[action] in the route template with the actual values + // for this action. + try { - // An attribute routed action will ignore conventional routed constraints. We still - // want to provide these values as ambient values. - foreach (var constraint in actionDescriptor.RouteConstraints) - { - actionDescriptor.RouteValueDefaults.Add(constraint.RouteKey, constraint.RouteValue); - } - - // TODO #738 - this currently has parity with what we did in MVC5 when a template uses - // parameters like 'area', 'controller', and 'action. This needs to be changed as - // part of #738. - // - // For instance, consider actions mapped with api/Blog/{action}. The value of {action} - // needs to passed to action selection to choose the right action. - var template = TemplateParser.Parse(templateText, _constraintResolver); - - var routeConstraints = new List(); - foreach (var constraint in actionDescriptor.RouteConstraints) - { - if (template.Parameters.Any( - p => p.IsParameter && - string.Equals(p.Name, constraint.RouteKey, StringComparison.OrdinalIgnoreCase))) - { - routeConstraints.Add(constraint); - } - } - - var routeGroup = routeGroupsByTemplate[templateText]; - routeConstraints.Add(new RouteDataActionConstraint( - AttributeRouting.RouteGroupKey, - routeGroup)); - - actionDescriptor.RouteConstraints = routeConstraints; - - actionDescriptor.AttributeRouteTemplate = templateText; + templateText = AttributeRouteTemplate.ReplaceTokens( + templateText, + actionDescriptor.RouteValueDefaults); } + catch (InvalidOperationException ex) + { + var message = Resources.FormatAttributeRoute_IndividualErrorMessage( + actionDescriptor.DisplayName, + Environment.NewLine, + ex.Message); + + routeTemplateErrors.Add(message); + } + + actionDescriptor.AttributeRouteTemplate = templateText; + + // An attribute routed action is matched by its 'route group' which identifies all equivalent + // actions. + string routeGroup; + if (!routeGroupsByTemplate.TryGetValue(templateText, out routeGroup)) + { + routeGroup = GetRouteGroup(templateText); + routeGroupsByTemplate.Add(templateText, routeGroup); + } + + var routeConstraints = new List(); + routeConstraints.Add(new RouteDataActionConstraint( + AttributeRouting.RouteGroupKey, + routeGroup)); + + actionDescriptor.RouteConstraints = routeConstraints; } actionDescriptor.FilterDescriptors = @@ -270,6 +269,15 @@ namespace Microsoft.AspNet.Mvc { if (actionDescriptor.AttributeRouteTemplate == null) { + // Any any attribute routes are in use, then non-attribute-routed ADs can't be selected + // when a route group returned by the route. + if (routeGroupsByTemplate.Any()) + { + actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint( + AttributeRouting.RouteGroupKey, + RouteKeyHandling.DenyKey)); + } + if (!HasConstraint(actionDescriptor.RouteConstraints, key)) { actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint( @@ -292,27 +300,21 @@ namespace Microsoft.AspNet.Mvc } } + if (routeTemplateErrors.Any()) + { + var message = Resources.FormatAttributeRoute_AggregateErrorMessage( + Environment.NewLine, + string.Join(Environment.NewLine + Environment.NewLine, routeTemplateErrors)); + throw new InvalidOperationException(message); + } + return actions; } - // Groups the set of all attribute routing templates and returns mapping of [template -> group]. - private static Dictionary GetRouteGroupsByTemplate(ReflectedApplicationModel model) + // Returns a unique, stable key per-route-template (OrdinalIgnoreCase) + private static string GetRouteGroup(string template) { - var groupsByTemplate = new Dictionary(StringComparer.OrdinalIgnoreCase); - - foreach (var controller in model.Controllers) - { - foreach (var action in controller.Actions) - { - var template = AttributeRouteTemplate.Combine(controller.RouteTemplate, action.RouteTemplate); - if (template != null && !groupsByTemplate.ContainsKey(template)) - { - groupsByTemplate.Add(template, "__route__" + template); - } - } - } - - return groupsByTemplate; + return ("__route__" + template).ToUpperInvariant(); } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Resources.resx b/src/Microsoft.AspNet.Mvc.Core/Resources.resx index c73c9886e1..bd5145d266 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Core/Resources.resx @@ -315,4 +315,34 @@ No encoding found for output formatter '{0}'. There must be at least one supported encoding registered in order for the output formatter to write content. + + The following errors occurred with attribute routing information:{0}{0}{1} + {0} is the newline. {1} is the formatted list of errors using AttributeRoute_IndividualErrorMessage + + + The attribute route '{0}' cannot contain a parameter named '{{{1}}}'. Use '[{1}]' in the route template to insert the value '{2}'. + + + For action: '{0}'{1}Error: {2} + {1} is the newline. + + + An empty replacement token ('[]') is not allowed. + + + Token delimiters ('[', ']') are imbalanced. + + + The route template '{0}' has invalid syntax. {1} + {1} is the specific error message. + + + While processing template '{0}', a replacement value for the token '{1}' could not be found. Available tokens: '{2}'. + + + A replacement token is not closed. + + + An unescaped '[' token is not allowed inside of a replacement token. Use '[[' to escape. + \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteTemplate.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteTemplate.cs index 4c60deb075..720e26ef4c 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteTemplate.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteTemplate.cs @@ -2,6 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; +using System.Text; +using Microsoft.AspNet.Mvc.Core; namespace Microsoft.AspNet.Mvc.Routing { @@ -93,5 +96,207 @@ namespace Microsoft.AspNet.Mvc.Routing return result.Substring(startIndex, subStringLength); } + + public static string ReplaceTokens(string template, IDictionary values) + { + var builder = new StringBuilder(); + var state = TemplateParserState.Plaintext; + + int? tokenStart = null; + + // We'll run the loop one extra time with 'null' to detect the end of the string. + for (var i = 0; i <= template.Length; i++) + { + var c = i < template.Length ? (char?)template[i] : null; + switch (state) + { + case TemplateParserState.Plaintext: + if (c == '[') + { + state = TemplateParserState.SeenLeft; + break; + } + else if (c == ']') + { + state = TemplateParserState.SeenRight; + break; + } + else if (c == null) + { + // We're at the end of the string, nothing left to do. + break; + } + else + { + builder.Append(c); + break; + } + case TemplateParserState.SeenLeft: + if (c == '[') + { + // This is an escaped left-bracket + builder.Append(c); + state = TemplateParserState.Plaintext; + break; + } + else if (c == ']') + { + // This is zero-width parameter - not allowed. + var message = Resources.FormatAttributeRoute_TokenReplacement_InvalidSyntax( + template, + Resources.AttributeRoute_TokenReplacement_EmptyTokenNotAllowed); + throw new InvalidOperationException(message); + } + else if (c == null) + { + // This is a left-bracket at the end of the string. + var message = Resources.FormatAttributeRoute_TokenReplacement_InvalidSyntax( + template, + Resources.AttributeRoute_TokenReplacement_UnclosedToken); + throw new InvalidOperationException(message); + } + else + { + tokenStart = i; + state = TemplateParserState.InsideToken; + break; + } + case TemplateParserState.SeenRight: + if (c == ']') + { + // This is an escaped right-bracket + builder.Append(c); + state = TemplateParserState.Plaintext; + break; + } + else if (c == null) + { + // This is an imbalanced right-bracket at the end of the string. + var message = Resources.FormatAttributeRoute_TokenReplacement_InvalidSyntax( + template, + Resources.AttributeRoute_TokenReplacement_ImbalancedSquareBrackets); + throw new InvalidOperationException(message); + } + else + { + // This is an imbalanced right-bracket. + var message = Resources.FormatAttributeRoute_TokenReplacement_InvalidSyntax( + template, + Resources.AttributeRoute_TokenReplacement_ImbalancedSquareBrackets); + throw new InvalidOperationException(message); + } + case TemplateParserState.InsideToken: + if (c == '[') + { + state = TemplateParserState.InsideToken | TemplateParserState.SeenLeft; + break; + } + else if (c == ']') + { + state = TemplateParserState.InsideToken | TemplateParserState.SeenRight; + break; + } + else if (c == null) + { + // This is an unclosed replacement token + var message = Resources.FormatAttributeRoute_TokenReplacement_InvalidSyntax( + template, + Resources.AttributeRoute_TokenReplacement_UnclosedToken); + throw new InvalidOperationException(message); + } + else + { + // This is a just part of the parameter + break; + } + case TemplateParserState.InsideToken | TemplateParserState.SeenLeft: + if (c == '[') + { + // This is an escaped left-bracket + state = TemplateParserState.InsideToken; + break; + } + else + { + // Unescaped left-bracket is not allowed inside a token. + var message = Resources.FormatAttributeRoute_TokenReplacement_InvalidSyntax( + template, + Resources.AttributeRoute_TokenReplacement_UnescapedBraceInToken); + throw new InvalidOperationException(message); + } + case TemplateParserState.InsideToken | TemplateParserState.SeenRight: + if (c == ']') + { + // This is an escaped right-bracket + state = TemplateParserState.InsideToken; + break; + } + else + { + // This is the end of a replacement token. + var token = template + .Substring(tokenStart.Value, i - tokenStart.Value - 1) + .Replace("[[", "[") + .Replace("]]", "]"); + + object value; + if (!values.TryGetValue(token, out value)) + { + // Value not found + var message = Resources.FormatAttributeRoute_TokenReplacement_ReplacementValueNotFound( + template, + token, + string.Join(", ", values.Keys)); + throw new InvalidOperationException(message); + } + + builder.Append(value); + + if (c == '[') + { + state = TemplateParserState.SeenLeft; + } + else if (c == ']') + { + state = TemplateParserState.SeenRight; + } + else if (c == null) + { + state = TemplateParserState.Plaintext; + } + else + { + builder.Append(c); + state = TemplateParserState.Plaintext; + } + + tokenStart = null; + break; + } + } + } + + return builder.ToString(); + } + + [Flags] + private enum TemplateParserState : uint + { + // default state - allow non-special characters to pass through to the + // buffer. + Plaintext = 0, + + // We're inside a replacement token, may be combined with other states to detect + // a possible escaped bracket inside the token. + InsideToken = 1, + + // We've seen a left brace, need to see the next character to find out if it's escaped + // or not. + SeenLeft = 2, + + // We've seen a right brace, need to see the next character to find out if it's escaped + // or not. + SeenRight = 4, + } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs index ebfd14e84d..7107e9646f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Routing; using Microsoft.AspNet.Routing.Template; using Microsoft.Framework.DependencyInjection; @@ -27,26 +28,18 @@ namespace Microsoft.AspNet.Mvc.Routing var actions = GetActionDescriptors(services); var inlineConstraintResolver = services.GetService(); - var routeInfos = GetRouteInfos(actions, inlineConstraintResolver); + var routeInfos = GetRouteInfos(inlineConstraintResolver, actions); // We're creating one AttributeRouteGenerationEntry per action. This allows us to match the intended // action by expected route values, and then use the TemplateBinder to generate the link. var generationEntries = new List(); foreach (var routeInfo in routeInfos) { - var defaults = routeInfo.ParsedTemplate.Parameters - .Where(p => p.DefaultValue != null) - .ToDictionary(p => p.Name, p => p.DefaultValue, StringComparer.OrdinalIgnoreCase); - - var constraints = routeInfo.ParsedTemplate.Parameters - .Where(p => p.InlineConstraint != null) - .ToDictionary(p => p.Name, p => p.InlineConstraint, StringComparer.OrdinalIgnoreCase); - generationEntries.Add(new AttributeRouteLinkGenerationEntry() { - Binder = new TemplateBinder(routeInfo.ParsedTemplate, defaults), - Defaults = defaults, - Constraints = constraints, + Binder = new TemplateBinder(routeInfo.ParsedTemplate, routeInfo.Defaults), + Defaults = routeInfo.Defaults, + Constraints = routeInfo.Constraints, Precedence = routeInfo.Precedence, RequiredLinkValues = routeInfo.ActionDescriptor.RouteValueDefaults, RouteGroup = routeInfo.RouteGroup, @@ -103,42 +96,133 @@ namespace Microsoft.AspNet.Mvc.Routing } private static List GetRouteInfos( - IReadOnlyList actions, - IInlineConstraintResolver constraintResolver) + IInlineConstraintResolver constraintResolver, + IReadOnlyList actions) { var routeInfos = new List(); + var errors = new List(); + + // This keeps a cache of 'Template' objects. It's a fairly common case that multiple actions + // will use the same route template string; thus, the `Template` object can be shared. + // + // For a relatively simple route template, the `Template` object will hold about 500 bytes + // of memory, so sharing is worthwhile. + var templateCache = new Dictionary(StringComparer.OrdinalIgnoreCase); foreach (var action in actions.Where(a => a.AttributeRouteTemplate != null)) { - var constraint = action.RouteConstraints - .Where(c => c.RouteKey == AttributeRouting.RouteGroupKey) - .FirstOrDefault(); - if (constraint == null || - constraint.KeyHandling != RouteKeyHandling.RequireKey || - constraint.RouteValue == null) + var routeInfo = GetRouteInfo(constraintResolver, templateCache, action); + if (routeInfo.ErrorMessage == null) { - // This is unlikely to happen by default, but could happen through extensibility. Just ignore it. - continue; + routeInfos.Add(routeInfo); } - - var parsedTemplate = TemplateParser.Parse(action.AttributeRouteTemplate, constraintResolver); - routeInfos.Add(new RouteInfo() + else { - ActionDescriptor = action, - ParsedTemplate = parsedTemplate, - Precedence = AttributeRoutePrecedence.Compute(parsedTemplate), - RouteGroup = constraint.RouteValue, - RouteTemplate = action.AttributeRouteTemplate, - }); + errors.Add(routeInfo); + } + } + + if (errors.Count > 0) + { + var allErrors = string.Join( + Environment.NewLine + Environment.NewLine, + errors.Select( + e => Resources.FormatAttributeRoute_IndividualErrorMessage( + e.ActionDescriptor.DisplayName, + Environment.NewLine, + e.ErrorMessage))); + + var message = Resources.FormatAttributeRoute_AggregateErrorMessage(Environment.NewLine, allErrors); + throw new InvalidOperationException(message); } return routeInfos; } + private static RouteInfo GetRouteInfo( + IInlineConstraintResolver constraintResolver, + Dictionary templateCache, + ActionDescriptor action) + { + var constraint = action.RouteConstraints + .Where(c => c.RouteKey == AttributeRouting.RouteGroupKey) + .FirstOrDefault(); + if (constraint == null || + constraint.KeyHandling != RouteKeyHandling.RequireKey || + constraint.RouteValue == null) + { + // This can happen if an ActionDescriptor has a route template, but doesn't have one of our + // special route group constraints. This is a good indication that the user is using a 3rd party + // routing system, or has customized their ADs in a way that we can no longer understand them. + // + // We just treat this case as an 'opt-out' of our attribute routing system. + return null; + } + + var routeInfo = new RouteInfo() + { + ActionDescriptor = action, + RouteGroup = constraint.RouteValue, + RouteTemplate = action.AttributeRouteTemplate, + }; + + try + { + Template parsedTemplate; + if (!templateCache.TryGetValue(action.AttributeRouteTemplate, out parsedTemplate)) + { + // Parsing with throw if the template is invalid. + parsedTemplate = TemplateParser.Parse(action.AttributeRouteTemplate, constraintResolver); + templateCache.Add(action.AttributeRouteTemplate, parsedTemplate); + } + + routeInfo.ParsedTemplate = parsedTemplate; + } + catch (Exception ex) + { + routeInfo.ErrorMessage = ex.Message; + return routeInfo; + } + + foreach (var kvp in action.RouteValueDefaults) + { + foreach (var parameter in routeInfo.ParsedTemplate.Parameters) + { + if (string.Equals(kvp.Key, parameter.Name, StringComparison.OrdinalIgnoreCase)) + { + routeInfo.ErrorMessage = Resources.FormatAttributeRoute_CannotContainParameter( + routeInfo.RouteTemplate, + kvp.Key, + kvp.Value); + + return routeInfo; + } + } + } + + routeInfo.Precedence = AttributeRoutePrecedence.Compute(routeInfo.ParsedTemplate); + + routeInfo.Constraints = routeInfo.ParsedTemplate.Parameters + .Where(p => p.InlineConstraint != null) + .ToDictionary(p => p.Name, p => p.InlineConstraint, StringComparer.OrdinalIgnoreCase); + + routeInfo.Defaults = routeInfo.ParsedTemplate.Parameters + .Where(p => p.DefaultValue != null) + .ToDictionary(p => p.Name, p => p.DefaultValue, StringComparer.OrdinalIgnoreCase); + + return routeInfo; + } + private class RouteInfo { public ActionDescriptor ActionDescriptor { get; set; } + public IDictionary Constraints { get; set; } + + public IDictionary Defaults { get; set; } + + public string ErrorMessage { get; set; } + public Template ParsedTemplate { get; set; } public decimal Precedence { get; set; } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj b/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj index 8b4ab08e7f..84c210b9d8 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj @@ -89,6 +89,7 @@ + diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs index 41bb1fe3af..6becd5361b 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; using Microsoft.AspNet.Routing; +using Microsoft.AspNet.Mvc.Routing; using Moq; using Xunit; @@ -209,6 +210,113 @@ namespace Microsoft.AspNet.Mvc.Test displayNames); } + public void AttributeRouting_TokenReplacement_IsAfterReflectedModel() + { + // Arrange + var provider = GetProvider(typeof(TokenReplacementController).GetTypeInfo()); + + // Act + var model = provider.BuildModel(); + + // Assert + var controller = Assert.Single(model.Controllers); + Assert.Equal("api/Token/[key]/[controller]", controller.RouteTemplate); + + var action = Assert.Single(controller.Actions); + Assert.Equal("stub/[action]", action.RouteTemplate); + } + + [Fact] + public void AttributeRouting_TokenReplacement_InActionDescriptor() + { + // Arrange + var provider = GetProvider(typeof(TokenReplacementController).GetTypeInfo()); + + // Act + var actions = provider.GetDescriptors(); + + // Assert + var action = Assert.Single(actions); + Assert.Equal("api/Token/value/TokenReplacement/stub/ThisIsAnAction", action.AttributeRouteTemplate); + } + + [Fact] + public void AttributeRouting_TokenReplacement_ThrowsWithMultipleMessages() + { + // Arrange + var provider = GetProvider(typeof(MultipleErrorsController).GetTypeInfo()); + + var expectedMessage = + "The following errors occurred with attribute routing information:" + Environment.NewLine + + Environment.NewLine + + "For action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "MultipleErrorsController.Unknown'" + Environment.NewLine + + "Error: While processing template 'stub/[action]/[unknown]', a replacement value for the token 'unknown' " + + "could not be found. Available tokens: 'controller, action'." + Environment.NewLine + + Environment.NewLine + + "For action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "MultipleErrorsController.Invalid'" + Environment.NewLine + + "Error: The route template '[invalid/syntax' has invalid syntax. A replacement token is not closed."; + + // Act + var ex = Assert.Throws(() => { provider.GetDescriptors(); }); + + // Assert + Assert.Equal(expectedMessage, ex.Message); + } + + [Fact] + public void AttributeRouting_TokenReplacement_CaseInsensitive() + { + // Arrange + var provider = GetProvider(typeof(CaseInsensitiveController).GetTypeInfo()); + + // Act + var actions = provider.GetDescriptors(); + + // Assert + var action = Assert.Single(actions); + Assert.Equal("stub/ThisIsAnAction", action.AttributeRouteTemplate); + } + + // Token replacement happens before we 'group' routes. So two route templates + // that are equivalent after token replacement go to the same 'group'. + [Fact] + public void AttributeRouting_TokenReplacement_BeforeGroupId() + { + // Arrange + var provider = GetProvider(typeof(SameGroupIdController).GetTypeInfo()); + + // Act + var actions = provider.GetDescriptors().ToArray(); + + var groupIds = actions.Select( + a => a.RouteConstraints + .Where(rc => rc.RouteKey == AttributeRouting.RouteGroupKey) + .Select(rc => rc.RouteValue) + .Single()) + .ToArray(); + + // Assert + Assert.Equal(2, groupIds.Length); + Assert.Equal(groupIds[0], groupIds[1]); + } + + // Parameters are validated later. This action uses the forbidden {action} and {controller} + [Fact] + public void AttributeRouting_DoesNotValidateParameters() + { + // Arrange + var provider = GetProvider(typeof(InvalidParametersController).GetTypeInfo()); + + // Act + var actions = provider.GetDescriptors(); + + // Assert + var action = Assert.Single(actions); + Assert.Equal("stub/{controller}/{action}", action.AttributeRouteTemplate); + } + private ReflectedActionDescriptorProvider GetProvider( TypeInfo controllerTypeInfo, IEnumerable filters = null) @@ -312,5 +420,43 @@ namespace Microsoft.AspNet.Mvc.Test { } } + + [Route("api/Token/[key]/[controller]")] + [MyRouteConstraint(false)] + private class TokenReplacementController + { + [HttpGet("stub/[action]")] + public void ThisIsAnAction() { } + } + + private class CaseInsensitiveController + { + [HttpGet("stub/[ActIon]")] + public void ThisIsAnAction() { } + } + + private class MultipleErrorsController + { + [HttpGet("stub/[action]/[unknown]")] + public void Unknown() { } + + [HttpGet("[invalid/syntax")] + public void Invalid() { } + } + + private class InvalidParametersController + { + [HttpGet("stub/{controller}/{action}")] + public void Action1() { } + } + + private class SameGroupIdController + { + [HttpGet("stub/[action]")] + public void Action1() { } + + [HttpGet("stub/Action1")] + public void Action2() { } + } } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTemplateTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTemplateTests.cs index f8d70aff18..aaaf4a0b8a 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTemplateTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTemplateTests.cs @@ -1,7 +1,9 @@ // 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.Routing; using System; +using System.Collections.Generic; using Xunit; namespace Microsoft.AspNet.Mvc.Routing @@ -98,5 +100,218 @@ namespace Microsoft.AspNet.Mvc.Routing // Assert Assert.Equal(expected, combined); } + + public static IEnumerable ReplaceTokens_ValueValuesData + { + get + { + yield return new object[] + { + "[controller]/[action]", + new { controller = "Home", action = "Index" }, + "Home/Index" + }; + + yield return new object[] + { + "[controller]", + new { controller = "Home", action = "Index" }, + "Home" + }; + + yield return new object[] + { + "[controller][[", + new { controller = "Home", action = "Index" }, + "Home[" + }; + + yield return new object[] + { + "[coNTroller]", + new { contrOLler = "Home", action = "Index" }, + "Home" + }; + + yield return new object[] + { + "thisisSomeText[action]", + new { controller = "Home", action = "Index" }, + "thisisSomeTextIndex" + }; + + yield return new object[] + { + "[[-]][[/[[controller]]", + new { controller = "Home", action = "Index" }, + "[-][/[controller]" + }; + + yield return new object[] + { + "[contr[[oller]/[act]]ion]", + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "contr[oller", "Home" }, + { "act]ion", "Index" } + }, + "Home/Index" + }; + + yield return new object[] + { + "[controller][action]", + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "Index" } + }, + "HomeIndex" + }; + + yield return new object[] + { + "[contr}oller]/[act{ion]/{id}", + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "contr}oller", "Home" }, + { "act{ion", "Index" } + }, + "Home/Index/{id}" + }; + } + } + + [Theory] + [MemberData("ReplaceTokens_ValueValuesData")] + public void ReplaceTokens_ValidValues(string template, object values, string expected) + { + // Arrange + var valuesDictionary = values as IDictionary; + if (valuesDictionary == null) + { + valuesDictionary = new RouteValueDictionary(values); + } + + // Act + var result = AttributeRouteTemplate.ReplaceTokens(template, valuesDictionary); + + // Assert + Assert.Equal(expected, result); + } + + public static IEnumerable ReplaceTokens_InvalidFormatValuesData + { + get + { + yield return new object[] + { + "[", + new { }, + "A replacement token is not closed." + }; + + yield return new object[] + { + "text]", + new { }, + "Token delimiters ('[', ']') are imbalanced.", + }; + + yield return new object[] + { + "text]morecooltext", + new { }, + "Token delimiters ('[', ']') are imbalanced.", + }; + + yield return new object[] + { + "[action", + new { }, + "A replacement token is not closed.", + }; + + yield return new object[] + { + "[action]]][", + new RouteValueDictionary() + { + { "action]", "Index" } + }, + "A replacement token is not closed.", + }; + + yield return new object[] + { + "[action]]", + new { }, + "A replacement token is not closed." + }; + + yield return new object[] + { + "[ac[tion]", + new { }, + "An unescaped '[' token is not allowed inside of a replacement token. Use '[[' to escape." + }; + + yield return new object[] + { + "[]", + new { }, + "An empty replacement token ('[]') is not allowed.", + }; + } + } + + [Theory] + [MemberData("ReplaceTokens_InvalidFormatValuesData")] + public void ReplaceTokens_InvalidFormat(string template, object values, string reason) + { + // Arrange + var valuesDictionary = values as IDictionary; + if (valuesDictionary == null) + { + valuesDictionary = new RouteValueDictionary(values); + } + + var expected = string.Format( + "The route template '{0}' has invalid syntax. {1}", + template, + reason); + + // Act + var ex = Assert.Throws( + () => { AttributeRouteTemplate.ReplaceTokens(template, valuesDictionary); }); + + // Assert + Assert.Equal(expected, ex.Message); + } + + [Fact] + public void ReplaceTokens_UnknownValue() + { + // Arrange + var template = "[area]/[controller]/[action2]"; + var values = new RouteValueDictionary() + { + { "area", "Help" }, + { "controller", "Admin" }, + { "action", "SeeUsers" }, + }; + + var expected = + "While processing template '[area]/[controller]/[action2]', " + + "a replacement value for the token 'action2' could not be found. " + + "Available tokens: 'area, controller, action'."; + + // Act + var ex = Assert.Throws( + () => { AttributeRouteTemplate.ReplaceTokens(template, values); }); + + // Assert + Assert.Equal(expected, ex.Message); + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRoutingTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRoutingTest.cs new file mode 100644 index 0000000000..85541b4502 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRoutingTest.cs @@ -0,0 +1,184 @@ +// 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. + +#if NET45 +using Microsoft.AspNet.Routing; +using Microsoft.Framework.OptionsModel; +using Moq; +using System; +using System.Collections.Generic; +using Xunit; + +namespace Microsoft.AspNet.Mvc.Routing +{ + public class AttributeRoutingTest + { + [Fact] + public void AttributeRouting_SyntaxErrorInTemplate() + { + // Arrange + var action = CreateAction("InvalidTemplate", "{a/dkfk}"); + + var expectedMessage = + "The following errors occurred with attribute routing information:" + Environment.NewLine + + Environment.NewLine + + "For action: 'InvalidTemplate'" + Environment.NewLine + + "Error: There is an incomplete parameter in the route template. " + + "Check that each '{' character has a matching '}' character." + Environment.NewLine + + "Parameter name: routeTemplate"; + + var router = CreateRouter(); + var services = CreateServices(action); + + // Act & Assert + var ex = Assert.Throws( + () => { AttributeRouting.CreateAttributeMegaRoute(router, services); }); + + Assert.Equal(expectedMessage, ex.Message); + } + + [Fact] + public void AttributeRouting_DisallowedParameter() + { + // Arrange + var action = CreateAction("DisallowedParameter", "{foo}/{action}"); + action.RouteValueDefaults.Add("foo", "bleh"); + + var expectedMessage = + "The following errors occurred with attribute routing information:" + Environment.NewLine + + Environment.NewLine + + "For action: 'DisallowedParameter'" + Environment.NewLine + + "Error: The attribute route '{foo}/{action}' cannot contain a parameter named '{foo}'. " + + "Use '[foo]' in the route template to insert the value 'bleh'."; + + var router = CreateRouter(); + var services = CreateServices(action); + + // Act & Assert + var ex = Assert.Throws( + () => { AttributeRouting.CreateAttributeMegaRoute(router, services); }); + + Assert.Equal(expectedMessage, ex.Message); + } + + [Fact] + public void AttributeRouting_MultipleErrors() + { + // Arrange + var action1 = CreateAction("DisallowedParameter1", "{foo}/{action}"); + action1.RouteValueDefaults.Add("foo", "bleh"); + + var action2 = CreateAction("DisallowedParameter2", "cool/{action}"); + action2.RouteValueDefaults.Add("action", "hey"); + + var expectedMessage = + "The following errors occurred with attribute routing information:" + Environment.NewLine + + Environment.NewLine + + "For action: 'DisallowedParameter1'" + Environment.NewLine + + "Error: The attribute route '{foo}/{action}' cannot contain a parameter named '{foo}'. " + + "Use '[foo]' in the route template to insert the value 'bleh'." + Environment.NewLine + + Environment.NewLine + + "For action: 'DisallowedParameter2'" + Environment.NewLine + + "Error: The attribute route 'cool/{action}' cannot contain a parameter named '{action}'. " + + "Use '[action]' in the route template to insert the value 'hey'."; + + var router = CreateRouter(); + var services = CreateServices(action1, action2); + + // Act & Assert + var ex = Assert.Throws( + () => { AttributeRouting.CreateAttributeMegaRoute(router, services); }); + + Assert.Equal(expectedMessage, ex.Message); + } + + [Fact] + public void AttributeRouting_WithReflectedActionDescriptor() + { + // Arrange + var controllerType = typeof(HomeController); + var actionMethod = controllerType.GetMethod("Index"); + + var action = new ReflectedActionDescriptor(); + action.DisplayName = "Microsoft.AspNet.Mvc.Routing.AttributeRoutingTest+HomeController.Index"; + action.MethodInfo = actionMethod; + action.RouteConstraints = new List() + { + new RouteDataActionConstraint(AttributeRouting.RouteGroupKey, "group"), + }; + action.AttributeRouteTemplate = "{controller}/{action}"; + action.RouteValueDefaults.Add("controller", "Home"); + action.RouteValueDefaults.Add("action", "Index"); + + var expectedMessage = + "The following errors occurred with attribute routing information:" + Environment.NewLine + + Environment.NewLine + + "For action: 'Microsoft.AspNet.Mvc.Routing.AttributeRoutingTest+HomeController.Index'" + Environment.NewLine + + "Error: The attribute route '{controller}/{action}' cannot contain a parameter named '{controller}'. " + + "Use '[controller]' in the route template to insert the value 'Home'."; + + var router = CreateRouter(); + var services = CreateServices(action); + + // Act & Assert + var ex = Assert.Throws( + () => { AttributeRouting.CreateAttributeMegaRoute(router, services); }); + + Assert.Equal(expectedMessage, ex.Message); + } + + private static ActionDescriptor CreateAction(string displayName, string template) + { + return new DisplayNameActionDescriptor() + { + DisplayName = displayName, + RouteConstraints = new List() + { + new RouteDataActionConstraint(AttributeRouting.RouteGroupKey, "whatever"), + }, + AttributeRouteTemplate = template, + }; + } + + private static IRouter CreateRouter() + { + return Mock.Of(); + } + + private static IServiceProvider CreateServices(params ActionDescriptor[] actions) + { + var collection = new ActionDescriptorsCollection(actions, version: 0); + + var actionDescriptorProvider = new Mock(); + actionDescriptorProvider + .Setup(a => a.ActionDescriptors) + .Returns(collection); + + var services = new Mock(); + services + .Setup(s => s.GetService(typeof(IActionDescriptorsCollectionProvider))) + .Returns(actionDescriptorProvider.Object); + + var routeOptions = new Mock>(); + routeOptions + .SetupGet(o => o.Options) + .Returns(new RouteOptions()); + + services + .Setup(s => s.GetService(typeof(IInlineConstraintResolver))) + .Returns(new DefaultInlineConstraintResolver(services.Object, routeOptions.Object)); + + return services.Object; + } + + private class DisplayNameActionDescriptor : ActionDescriptor + { + } + + private class HomeController + { + public void Index() { } + } + } +} +#endif \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs index 2bbe1e20a3..bec2033ccf 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs @@ -358,7 +358,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests } [Fact] - public async Task AttributeRoutedAction_LinkToAttribueRoutedController() + public async Task AttributeRoutedAction_LinkToAttributeRoutedController() { // Arrange var server = TestServer.Create(_services, _app); @@ -378,7 +378,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.Equal("Employee", result.Controller); Assert.Equal("List", result.Action); - Assert.Equal("/Blog", result.Link); + Assert.Equal("/Blog/ShowPosts", result.Link); } [Fact] diff --git a/test/WebSites/RoutingWebSite/Areas/Admin/UserManagementController.cs b/test/WebSites/RoutingWebSite/Areas/Admin/UserManagementController.cs index b1feae4d56..9b23f99743 100644 --- a/test/WebSites/RoutingWebSite/Areas/Admin/UserManagementController.cs +++ b/test/WebSites/RoutingWebSite/Areas/Admin/UserManagementController.cs @@ -6,7 +6,7 @@ using Microsoft.AspNet.Mvc; namespace RoutingWebSite.Admin { [Area("Admin")] - [Route("{area}/Users")] + [Route("[area]/Users")] public class UserManagementController : Controller { private readonly TestResponseGenerator _generator; diff --git a/test/WebSites/RoutingWebSite/Controllers/BlogController.cs b/test/WebSites/RoutingWebSite/Controllers/BlogController.cs index b702aab21e..40968c108d 100644 --- a/test/WebSites/RoutingWebSite/Controllers/BlogController.cs +++ b/test/WebSites/RoutingWebSite/Controllers/BlogController.cs @@ -6,7 +6,7 @@ using Microsoft.AspNet.Mvc; namespace RoutingWebSite { // This controller contains actions mapped with a single controller-level route. - [Route("Blog/{action=ShowPosts}/{postId?}")] + [Route("Blog/[action]/{postId?}")] public class BlogController { private readonly TestResponseGenerator _generator; @@ -18,7 +18,7 @@ namespace RoutingWebSite public IActionResult ShowPosts() { - return _generator.Generate("/Blog", "/Blog/ShowPosts"); + return _generator.Generate("/Blog/ShowPosts"); } public IActionResult Edit(int postId)