CR Feedback

src/Microsoft.AspNet.Routing/Properties/Resources.Designer.cs

@@ -10,6 +10,38 @@ namespace Microsoft.AspNet.Routing
         private static readonly ResourceManager _resourceManager
             = new ResourceManager("Microsoft.AspNet.Routing.Resources", typeof(Resources).GetTypeInfo().Assembly);
 
+        /// <summary>
+        /// A default handler must be set on the RouteCollection.
+        /// </summary>
+        internal static string DefaultHandler_MustBeSet
+        {
+            get { return GetString("DefaultHandler_MustBeSet"); }
+        }
+
+        /// <summary>
+        /// A default handler must be set on the RouteCollection.
+        /// </summary>
+        internal static string FormatDefaultHandler_MustBeSet()
+        {
+            return GetString("DefaultHandler_MustBeSet");
+        }
+
+        /// <summary>
+        /// The constraint entry '{0}' must have a string value or be of a type which implements '{1}'.
+        /// </summary>
+        internal static string GeneralConstraints_ValidationMustBeStringOrCustomConstraint
+        {
+            get { return GetString("GeneralConstraints_ValidationMustBeStringOrCustomConstraint"); }
+        }
+
+        /// <summary>
+        /// The constraint entry '{0}' must have a string value or be of a type which implements '{1}'.
+        /// </summary>
+        internal static string FormatGeneralConstraints_ValidationMustBeStringOrCustomConstraint(object p0, object p1)
+        {
+            return string.Format(CultureInfo.CurrentCulture, GetString("GeneralConstraints_ValidationMustBeStringOrCustomConstraint"), p0, p1);
+        }
+
         /// <summary>
         /// A path segment that contains more than one section, such as a literal section or a parameter, cannot contain a catch-all parameter.
         /// </summary>
@@ -18,6 +50,14 @@ namespace Microsoft.AspNet.Routing
             get { return GetString("TemplateRoute_CannotHaveCatchAllInMultiSegment"); }
         }
 
+        /// <summary>
+        /// A path segment that contains more than one section, such as a literal section or a parameter, cannot contain a catch-all parameter.
+        /// </summary>
+        internal static string FormatTemplateRoute_CannotHaveCatchAllInMultiSegment()
+        {
+            return GetString("TemplateRoute_CannotHaveCatchAllInMultiSegment");
+        }
+
         /// <summary>
         /// A path segment cannot contain two consecutive parameters. They must be separated by a '/' or by a literal string.
         /// </summary>
@@ -26,6 +66,14 @@ namespace Microsoft.AspNet.Routing
             get { return GetString("TemplateRoute_CannotHaveConsecutiveParameters"); }
         }
 
+        /// <summary>
+        /// A path segment cannot contain two consecutive parameters. They must be separated by a '/' or by a literal string.
+        /// </summary>
+        internal static string FormatTemplateRoute_CannotHaveConsecutiveParameters()
+        {
+            return GetString("TemplateRoute_CannotHaveConsecutiveParameters");
+        }
+
         /// <summary>
         /// The route template separator character '/' cannot appear consecutively. It must be separated by either a parameter or a literal value.
         /// </summary>
@@ -34,6 +82,14 @@ namespace Microsoft.AspNet.Routing
             get { return GetString("TemplateRoute_CannotHaveConsecutiveSeparators"); }
         }
 
+        /// <summary>
+        /// The route template separator character '/' cannot appear consecutively. It must be separated by either a parameter or a literal value.
+        /// </summary>
+        internal static string FormatTemplateRoute_CannotHaveConsecutiveSeparators()
+        {
+            return GetString("TemplateRoute_CannotHaveConsecutiveSeparators");
+        }
+
         /// <summary>
         /// A path segment that contains more than one section, such as a literal section or a parameter, cannot contain an optional parameter.
         /// </summary>
@@ -42,6 +98,14 @@ namespace Microsoft.AspNet.Routing
             get { return GetString("TemplateRoute_CannotHaveOptionalParameterInMultiSegment"); }
         }
 
+        /// <summary>
+        /// A path segment that contains more than one section, such as a literal section or a parameter, cannot contain an optional parameter.
+        /// </summary>
+        internal static string FormatTemplateRoute_CannotHaveOptionalParameterInMultiSegment()
+        {
+            return GetString("TemplateRoute_CannotHaveOptionalParameterInMultiSegment");
+        }
+
         /// <summary>
         /// A catch-all parameter cannot be marked optional.
         /// </summary>
@@ -50,6 +114,14 @@ namespace Microsoft.AspNet.Routing
             get { return GetString("TemplateRoute_CatchAllCannotBeOptional"); }
         }
 
+        /// <summary>
+        /// A catch-all parameter cannot be marked optional.
+        /// </summary>
+        internal static string FormatTemplateRoute_CatchAllCannotBeOptional()
+        {
+            return GetString("TemplateRoute_CatchAllCannotBeOptional");
+        }
+
         /// <summary>
         /// A catch-all parameter can only appear as the last segment of the route template.
         /// </summary>
@@ -58,6 +130,14 @@ namespace Microsoft.AspNet.Routing
             get { return GetString("TemplateRoute_CatchAllMustBeLast"); }
         }
 
+        /// <summary>
+        /// A catch-all parameter can only appear as the last segment of the route template.
+        /// </summary>
+        internal static string FormatTemplateRoute_CatchAllMustBeLast()
+        {
+            return GetString("TemplateRoute_CatchAllMustBeLast");
+        }
+
         /// <summary>
         /// The literal section '{0}' is invalid. Literal sections cannot contain the '?' character.
         /// </summary>
@@ -66,6 +146,14 @@ namespace Microsoft.AspNet.Routing
             get { return GetString("TemplateRoute_InvalidLiteral"); }
         }
 
+        /// <summary>
+        /// The literal section '{0}' is invalid. Literal sections cannot contain the '?' character.
+        /// </summary>
+        internal static string FormatTemplateRoute_InvalidLiteral(object p0)
+        {
+            return string.Format(CultureInfo.CurrentCulture, GetString("TemplateRoute_InvalidLiteral"), p0);
+        }
+
         /// <summary>
         /// The route parameter name '{0}' is invalid. Route parameter names must be non-empty and cannot contain these characters: '{{', '}}', '/'. The '?' character marks a parameter as optional, and can only occur at the end of the parameter.
         /// </summary>
@@ -74,6 +162,14 @@ namespace Microsoft.AspNet.Routing
             get { return GetString("TemplateRoute_InvalidParameterName"); }
         }
 
+        /// <summary>
+        /// The route parameter name '{0}' is invalid. Route parameter names must be non-empty and cannot contain these characters: '{{', '}}', '/'. The '?' character marks a parameter as optional, and can only occur at the end of the parameter.
+        /// </summary>
+        internal static string FormatTemplateRoute_InvalidParameterName(object p0)
+        {
+            return string.Format(CultureInfo.CurrentCulture, GetString("TemplateRoute_InvalidParameterName"), p0);
+        }
+
         /// <summary>
         /// The route template cannot start with a '/' or '~' character.
         /// </summary>
@@ -82,6 +178,14 @@ namespace Microsoft.AspNet.Routing
             get { return GetString("TemplateRoute_InvalidRouteTemplate"); }
         }
 
+        /// <summary>
+        /// The route template cannot start with a '/' or '~' character.
+        /// </summary>
+        internal static string FormatTemplateRoute_InvalidRouteTemplate()
+        {
+            return GetString("TemplateRoute_InvalidRouteTemplate");
+        }
+
         /// <summary>
         /// There is an incomplete parameter in the route template. Check that each '{' character has a matching '}' character.
         /// </summary>
@@ -90,6 +194,14 @@ namespace Microsoft.AspNet.Routing
             get { return GetString("TemplateRoute_MismatchedParameter"); }
         }
 
+        /// <summary>
+        /// There is an incomplete parameter in the route template. Check that each '{' character has a matching '}' character.
+        /// </summary>
+        internal static string FormatTemplateRoute_MismatchedParameter()
+        {
+            return GetString("TemplateRoute_MismatchedParameter");
+        }
+
         /// <summary>
         /// The route parameter name '{0}' appears more than one time in the route template.
         /// </summary>
@@ -98,6 +210,14 @@ namespace Microsoft.AspNet.Routing
             get { return GetString("TemplateRoute_RepeatedParameter"); }
         }
 
+        /// <summary>
+        /// The route parameter name '{0}' appears more than one time in the route template.
+        /// </summary>
+        internal static string FormatTemplateRoute_RepeatedParameter(object p0)
+        {
+            return string.Format(CultureInfo.CurrentCulture, GetString("TemplateRoute_RepeatedParameter"), p0);
+        }
+
         /// <summary>
         /// The constraint entry '{0}' on the route with route template '{1}' must have a string value or be of a type which implements '{2}'.
         /// </summary>
@@ -106,15 +226,26 @@ namespace Microsoft.AspNet.Routing
             get { return GetString("TemplateRoute_ValidationMustBeStringOrCustomConstraint"); }
         }
 
-        private static string GetString(string name, params string[] argumentNames)
+        /// <summary>
+        /// The constraint entry '{0}' on the route with route template '{1}' must have a string value or be of a type which implements '{2}'.
+        /// </summary>
+        internal static string FormatTemplateRoute_ValidationMustBeStringOrCustomConstraint(object p0, object p1, object p2)
+        {
+            return string.Format(CultureInfo.CurrentCulture, GetString("TemplateRoute_ValidationMustBeStringOrCustomConstraint"), p0, p1, p2);
+        }
+
+        private static string GetString(string name, params string[] formatterNames)
         {
             var value = _resourceManager.GetString(name);
 
             System.Diagnostics.Debug.Assert(value != null);
-
+    
-            for (var i = 0; i < argumentNames.Length; i++)
+            if (formatterNames != null)
             {
-                value = value.Replace("{" + argumentNames[i] + "}", "{" + i + "}");
+                for (var i = 0; i < formatterNames.Length; i++)
+                {
+                    value = value.Replace("{" + formatterNames[i] + "}", "{" + i + "}");
+                }
             }
 
             return value;

src/Microsoft.AspNet.Routing/Resources.resx

@@ -117,6 +117,12 @@
   <resheader name="writer">
     <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
   </resheader>
+  <data name="DefaultHandler_MustBeSet" xml:space="preserve">
+    <value>A default handler must be set on the RouteCollection.</value>
+  </data>
+  <data name="GeneralConstraints_ValidationMustBeStringOrCustomConstraint" xml:space="preserve">
+    <value>The constraint entry '{0}' must have a string value or be of a type which implements '{1}'.</value>
+  </data>
   <data name="TemplateRoute_CannotHaveCatchAllInMultiSegment" xml:space="preserve">
     <value>A path segment that contains more than one section, such as a literal section or a parameter, cannot contain a catch-all parameter.</value>
   </data>

src/Microsoft.AspNet.Routing/RouteCollectionExtensions.cs

@@ -10,35 +10,38 @@ namespace Microsoft.AspNet.Routing
     {
         public static IRouteCollection MapRoute(this IRouteCollection routes, string template)
         {
-            MapRoute(routes, template, null);
+            MapRoute(routes, template, defaults: null);
             return routes;
         }
 
-        public static IRouteCollection MapRoute(this IRouteCollection routes, string template, object defaults)
+        public static IRouteCollection MapRoute(this IRouteCollection routes, string template,
+                                                object defaults)
         {
             MapRoute(routes, template, new RouteValueDictionary(defaults));
             return routes;
         }
 
-        public static IRouteCollection MapRoute(this IRouteCollection routes, string template, IDictionary<string, object> defaults)
+        public static IRouteCollection MapRoute(this IRouteCollection routes, string template,
+                                                IDictionary<string, object> defaults)
         {
             if (routes.DefaultHandler == null)
             {
-                throw new ArgumentException("DefaultHandler must be set.");
+                throw new InvalidOperationException(Resources.DefaultHandler_MustBeSet);
             }
 
-            routes.Add(new TemplateRoute(routes.DefaultHandler, template, defaults, null));
+            routes.Add(new TemplateRoute(routes.DefaultHandler, template, defaults, constraints: null));
             return routes;
         }
 
-        public static IRouteCollection MapRoute(this IRouteCollection routes, string template, object defaults, object constraints)
+        public static IRouteCollection MapRoute(this IRouteCollection routes, string template,
+                                                object defaults, object constraints)
         {
             MapRoute(routes, template, new RouteValueDictionary(defaults), new RouteValueDictionary(constraints));
             return routes;
         }
 
-        public static IRouteCollection MapRoute(this IRouteCollection routes, string template, object defaults,
+        public static IRouteCollection MapRoute(this IRouteCollection routes, string template,
-                                                IDictionary<string, object> constraints)
+                                                object defaults, IDictionary<string, object> constraints)
         {
             MapRoute(routes, template, new RouteValueDictionary(defaults), constraints);
             return routes;
@@ -56,7 +59,7 @@ namespace Microsoft.AspNet.Routing
         {
             if (routes.DefaultHandler == null)
             {
-                throw new ArgumentException("DefaultHandler must be set.");
+                throw new InvalidOperationException(Resources.DefaultHandler_MustBeSet);
             }
 
             routes.Add(new TemplateRoute(routes.DefaultHandler, template, defaults, constraints));

src/Microsoft.AspNet.Routing/RouteConstraintBuilder.cs

@@ -5,8 +5,20 @@ namespace Microsoft.AspNet.Routing
 {
     public class RouteConstraintBuilder
     {
-        public static IDictionary<string, IRouteConstraint> 
+        public static IDictionary<string, IRouteConstraint>
             BuildConstraints(IDictionary<string, object> inputConstraints)
+        {
+            return BuildConstraintsCore(inputConstraints, routeTemplate: null);
+        }
+
+        public static IDictionary<string, IRouteConstraint>
+            BuildConstraints(IDictionary<string, object> inputConstraints, [NotNull] string routeTemplate)
+        {
+            return BuildConstraintsCore(inputConstraints, routeTemplate);
+        }
+
+        public static IDictionary<string, IRouteConstraint>
+            BuildConstraintsCore(IDictionary<string, object> inputConstraints, string routeTemplate)
         {
             if (inputConstraints == null || inputConstraints.Count == 0)
             {
@@ -25,7 +37,18 @@ namespace Microsoft.AspNet.Routing
 
                     if (regexPattern == null)
                     {
-                        throw new InvalidOperationException("Constraint can be a valid regex string or an IRouteConstraint");
+                        if (routeTemplate != null)
+                        {
+                            throw new InvalidOperationException(
+                                Resources.FormatTemplateRoute_ValidationMustBeStringOrCustomConstraint(
+                                    kvp.Key, routeTemplate, typeof (IRouteConstraint)));
+                        }
+                        else
+                        {
+                            throw new InvalidOperationException(
+                                Resources.FormatGeneralConstraints_ValidationMustBeStringOrCustomConstraint(
+                                    kvp.Key, typeof(IRouteConstraint)));
+                        }
                     }
 
                     var constraintsRegEx = "^(" + regexPattern + ")$";

src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs

@@ -20,18 +20,13 @@ namespace Microsoft.AspNet.Routing.Template
         {
         }
 
-        public TemplateRoute(IRouter target, string routeTemplate, IDictionary<string, object> defaults,
+        public TemplateRoute([NotNull] IRouter target, string routeTemplate, IDictionary<string, object> defaults,
                              IDictionary<string, object> constraints)
         {
-            if (target == null)
-            {
-                throw new ArgumentNullException("target");
-            }
-
             _target = target;
             _routeTemplate = routeTemplate ?? string.Empty;
             _defaults = defaults ?? new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
-            _constraints = RouteConstraintBuilder.BuildConstraints(constraints);
+            _constraints = RouteConstraintBuilder.BuildConstraints(constraints, _routeTemplate);
 
             // The parser will throw for invalid routes.
             var parsedTemplate = TemplateParser.Parse(RouteTemplate);
@@ -50,13 +45,8 @@ namespace Microsoft.AspNet.Routing.Template
             get { return _routeTemplate; }
         }
 
-        public async virtual Task RouteAsync(RouteContext context)
+        public async virtual Task RouteAsync([NotNull] RouteContext context)
         {
-            if (context == null)
-            {
-                throw new ArgumentNullException("context");
-            }
-
             var requestPath = context.RequestPath;
             if (!string.IsNullOrEmpty(requestPath) && requestPath[0] == '/')
             {