cr feedback

src/Microsoft.AspNet.Mvc.Core/ActionDescriptor.cs

@@ -1,12 +1,18 @@
 // 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 System;
 using System.Collections.Generic;
 
 namespace Microsoft.AspNet.Mvc
 {
     public class ActionDescriptor
     {
+        public ActionDescriptor()
+        {
+            RouteValueDefaults = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
+        }
+
         public virtual string Name { get; set; }
 
         public List<RouteDataActionConstraint> RouteConstraints { get; set; }
@@ -14,12 +20,12 @@ namespace Microsoft.AspNet.Mvc
         /// <summary>
         /// The set of route values that are added when this action is selected.
         /// </summary>
-        public Dictionary<string, object> RouteValues { get; set; }
+        public Dictionary<string, object> RouteValueDefaults { get; private set; }
 
         /// <summary>
-        /// The route template. May be null if the action has no attribute routes.
+        /// The attribute route template. May be null if the action has no attribute routes.
         /// </summary>
-        public string RouteTemplate { get; set; }
+        public string AttributeRouteTemplate { get; set; }
 
         public List<HttpMethodConstraint> MethodConstraints { get; set; }
 

src/Microsoft.AspNet.Mvc.Core/Microsoft.AspNet.Mvc.Core.kproj

@@ -217,7 +217,7 @@
     <Compile Include="KnownRouteValueConstraint.cs" />
     <Compile Include="RouteKeyHandling.cs" />
     <Compile Include="Routing\AttributeRoute.cs" />
-    <Compile Include="Routing\AttributeRouteGenerationEntry.cs" />
+    <Compile Include="Routing\AttributeRouteLinkGenerationEntry.cs" />
     <Compile Include="Routing\AttributeRouteMatchingEntry.cs" />
     <Compile Include="Routing\AttributeRoutePrecedence.cs" />
     <Compile Include="Routing\AttributeRouteTemplate.cs" />

src/Microsoft.AspNet.Mvc.Core/MvcRouteHandler.cs

@@ -38,9 +38,9 @@ namespace Microsoft.AspNet.Mvc
                 return;
             }
 
-            if (actionDescriptor.RouteValues != null)
+            if (actionDescriptor.RouteValueDefaults != null)
             {
-                foreach (var kvp in actionDescriptor.RouteValues)
+                foreach (var kvp in actionDescriptor.RouteValueDefaults)
                 {
                     if (!context.RouteData.Values.ContainsKey(kvp.Key))
                     {

src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs

@@ -213,19 +213,17 @@ namespace Microsoft.AspNet.Mvc
                         {
                             // An attribute routed action will ignore conventional routed constraints. We still
                             // want to provide these values as ambient values.
-                            var ambientValues = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
                             foreach (var constraint in actionDescriptor.RouteConstraints)
                             {
-                                ambientValues.Add(constraint.RouteKey, constraint.RouteValue);
+                                actionDescriptor.RouteValueDefaults.Add(constraint.RouteKey, constraint.RouteValue);
                             }
 
-                            actionDescriptor.RouteValues = ambientValues;
+                            // 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 
-                            // TODO #738 - this currently has parity with what we did in MVC5 when a template uses parameters
+                            // part of #738.
-                            // like 'area', 'controller', and 'action. This needs to be reconsidered as part of #738.
                             //
-                            // For instance, consider actions mapped with api/Blog/{action}. The value of {action} needs to 
+                            // For instance, consider actions mapped with api/Blog/{action}. The value of {action} 
-                            // passed to action selection to choose the right action.
+                            // needs to passed to action selection to choose the right action.
                             var template = TemplateParser.Parse(templateText, _constraintResolver);
 
                             var routeConstraints = new List<RouteDataActionConstraint>();
@@ -246,7 +244,7 @@ namespace Microsoft.AspNet.Mvc
 
                             actionDescriptor.RouteConstraints = routeConstraints;
 
-                            actionDescriptor.RouteTemplate = templateText;
+                            actionDescriptor.AttributeRouteTemplate = templateText;
                         }
                     }
 
@@ -265,7 +263,7 @@ namespace Microsoft.AspNet.Mvc
             {
                 foreach (var key in removalConstraints)
                 {
-                    if (actionDescriptor.RouteTemplate == null)
+                    if (actionDescriptor.AttributeRouteTemplate == null)
                     {
                         if (!HasConstraint(actionDescriptor.RouteConstraints, key))
                         {
@@ -274,13 +272,6 @@ namespace Microsoft.AspNet.Mvc
                                 RouteKeyHandling.DenyKey));
                         }
                     }
-                    else
-                    {
-                        if (!actionDescriptor.RouteValues.ContainsKey(key))
-                        {
-                            actionDescriptor.RouteValues.Add(key, null);
-                        }
-                    }
                 }
             }
 

src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs

@@ -17,7 +17,7 @@ namespace Microsoft.AspNet.Mvc.Routing
     {
         private readonly IRouter _next;
         private readonly TemplateRoute[] _matchingRoutes;
-        private readonly AttributeRouteGenerationEntry[] _generationEntries;
+        private readonly AttributeRouteLinkGenerationEntry[] _linkGenerationEntries;
 
         /// <summary>
         /// Creates a new <see cref="AttributeRoute"/>.
@@ -27,7 +27,7 @@ namespace Microsoft.AspNet.Mvc.Routing
         public AttributeRoute(
             [NotNull] IRouter next, 
             [NotNull] IEnumerable<AttributeRouteMatchingEntry> matchingEntries,
-            [NotNull] IEnumerable<AttributeRouteGenerationEntry> generationEntries)
+            [NotNull] IEnumerable<AttributeRouteLinkGenerationEntry> linkGenerationEntries)
         {
             _next = next;
 
@@ -35,9 +35,9 @@ namespace Microsoft.AspNet.Mvc.Routing
             // a good data-structure here. See #740
             _matchingRoutes = matchingEntries.OrderBy(e => e.Precedence).Select(e => e.Route).ToArray();
 
-            // FOR RIGHT NOW - this is just an array of binders. We'll follow up by implementing
+            // FOR RIGHT NOW - this is just an array of entries. We'll follow up by implementing
             // a good data-structure here. See #741
-            _generationEntries = generationEntries.OrderBy(e => e.Precedence).ToArray();
+            _linkGenerationEntries = linkGenerationEntries.OrderBy(e => e.Precedence).ToArray();
         }
 
         /// <inheritdoc />
@@ -61,7 +61,7 @@ namespace Microsoft.AspNet.Mvc.Routing
             // and controller.
             //
             // Building a proper data structure to optimize this is tracked by #741
-            foreach (var entry in _generationEntries)
+            foreach (var entry in _linkGenerationEntries)
             {
                 var isMatch = true;
                 foreach (var requiredLinkValue in entry.RequiredLinkValues)
@@ -73,23 +73,21 @@ namespace Microsoft.AspNet.Mvc.Routing
                     }
                 }
                 
-                if (!isMatch)
+                if (isMatch)
                 {
-                    continue;
+                    var path = GenerateLink(context, entry);
-                }
+                    if (path != null)
-
+                    {
-                var path = GenerateLink(context, entry);
+                        context.IsBound = true;
-                if (path != null)
+                        return path;
-                {
+                    }
-                    context.IsBound = true;
-                    return path;
                 }
             }
 
             return null;
         }
 
-        private string GenerateLink(VirtualPathContext context, AttributeRouteGenerationEntry entry)
+        private string GenerateLink(VirtualPathContext context, AttributeRouteLinkGenerationEntry entry)
         {
             // In attribute the context includes the values that are used to select this entry - typically
             // these will be the standard 'action', 'controller' and maybe 'area' tokens. However, we don't
@@ -135,7 +133,7 @@ namespace Microsoft.AspNet.Mvc.Routing
                 RouteDirection.UrlGeneration);
             if (!matched)
             {
-                // A constrant rejected this link.
+                // A constraint rejected this link.
                 return null;
             }
 

src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteLinkGenerationEntry.cs

@@ -11,7 +11,7 @@ namespace Microsoft.AspNet.Mvc.Routing
     /// Used to build an <see cref="AttributeRoute"/>. Represents an individual URL-generating route that will be
     /// aggregated into the <see cref="AttributeRoute"/>.
     /// </summary>
-    public class AttributeRouteGenerationEntry
+    public class AttributeRouteLinkGenerationEntry
     {
         /// <summary>
         /// The <see cref="TemplateBinder"/>.

src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs

@@ -31,7 +31,7 @@ namespace Microsoft.AspNet.Mvc.Routing
 
             // 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<AttributeRouteGenerationEntry>();
+            var generationEntries = new List<AttributeRouteLinkGenerationEntry>();
             foreach (var routeInfo in routeInfos)
             {
                 var defaults = routeInfo.ParsedTemplate.Parameters
@@ -42,13 +42,13 @@ namespace Microsoft.AspNet.Mvc.Routing
                     .Where(p => p.InlineConstraint != null)
                     .ToDictionary(p => p.Name, p => p.InlineConstraint, StringComparer.OrdinalIgnoreCase);
 
-                generationEntries.Add(new AttributeRouteGenerationEntry()
+                generationEntries.Add(new AttributeRouteLinkGenerationEntry()
                 {
                     Binder = new TemplateBinder(routeInfo.ParsedTemplate, defaults),
                     Defaults = defaults,
                     Constraints = constraints,
                     Precedence = routeInfo.Precedence,
-                    RequiredLinkValues = routeInfo.ActionDescriptor.RouteValues,
+                    RequiredLinkValues = routeInfo.ActionDescriptor.RouteValueDefaults,
                     RouteGroup = routeInfo.RouteGroup,
                     Template = routeInfo.ParsedTemplate,
                 });
@@ -108,7 +108,7 @@ namespace Microsoft.AspNet.Mvc.Routing
         {
             var routeInfos = new List<RouteInfo>();
 
-            foreach (var action in actions.Where(a => a.RouteTemplate != null))
+            foreach (var action in actions.Where(a => a.AttributeRouteTemplate != null))
             {
                 var constraint = action.RouteConstraints
                     .Where(c => c.RouteKey == AttributeRouting.RouteGroupKey)
@@ -121,14 +121,14 @@ namespace Microsoft.AspNet.Mvc.Routing
                     continue;
                 }
 
-                var parsedTemplate = TemplateParser.Parse(action.RouteTemplate, constraintResolver);
+                var parsedTemplate = TemplateParser.Parse(action.AttributeRouteTemplate, constraintResolver);
                 routeInfos.Add(new RouteInfo()
                 {
                     ActionDescriptor = action,
                     ParsedTemplate = parsedTemplate,
                     Precedence = AttributeRoutePrecedence.Compute(parsedTemplate),
                     RouteGroup = constraint.RouteValue,
-                    RouteTemplate = action.RouteTemplate,
+                    RouteTemplate = action.AttributeRouteTemplate,
                 });
             }
 

test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTests.cs

@@ -306,11 +306,11 @@ namespace Microsoft.AspNet.Mvc.Routing
                 new RouteValueDictionary(values));
         }
 
-        private static AttributeRouteGenerationEntry CreateGenerationEntry(string template, object requiredValues)
+        private static AttributeRouteLinkGenerationEntry CreateGenerationEntry(string template, object requiredValues)
         {
             var constraintResolver = CreateConstraintResolver();
 
-            var entry = new AttributeRouteGenerationEntry();
+            var entry = new AttributeRouteLinkGenerationEntry();
             entry.Template = TemplateParser.Parse(template, constraintResolver);
 
             var defaults = entry.Template.Parameters
@@ -342,22 +342,22 @@ namespace Microsoft.AspNet.Mvc.Routing
             return new DefaultInlineConstraintResolver(services, optionsMock.Object);
         }
 
-        private static AttributeRoute CreateAttributeRoute(AttributeRouteGenerationEntry entry)
+        private static AttributeRoute CreateAttributeRoute(AttributeRouteLinkGenerationEntry entry)
         {
             return CreateAttributeRoute(new StubRouter(), entry);
         }
 
-        private static AttributeRoute CreateAttributeRoute(IRouter next, AttributeRouteGenerationEntry entry)
+        private static AttributeRoute CreateAttributeRoute(IRouter next, AttributeRouteLinkGenerationEntry entry)
         {
             return CreateAttributeRoute(next, new[] { entry });
         }
 
-        private static AttributeRoute CreateAttributeRoute(params AttributeRouteGenerationEntry[] entries)
+        private static AttributeRoute CreateAttributeRoute(params AttributeRouteLinkGenerationEntry[] entries)
         {
             return CreateAttributeRoute(new StubRouter(), entries);
         }
 
-        private static AttributeRoute CreateAttributeRoute(IRouter next, params AttributeRouteGenerationEntry[] entries)
+        private static AttributeRoute CreateAttributeRoute(IRouter next, params AttributeRouteLinkGenerationEntry[] entries)
         {
             return new AttributeRoute(
                 next,