[Fixes #2931] AttributeRoute does not replace existing route values w…

…ith null
aspnet · Aug 28, 2015 · 4fbaea2 · 4fbaea2
1 parent c0d4981
commit 4fbaea2
Show file tree

Hide file tree

Showing 6 changed files with 123 additions and 10 deletions.
diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/InnerAttributeRoute.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/InnerAttributeRoute.cs
@@ -294,10 +294,13 @@ private static void MergeValues(
         {
             foreach (var kvp in values)
             {
-                // This will replace the original value for the specified key.
-                // Values from the matched route will take preference over previous
-                // data in the route context.
-                destination[kvp.Key] = kvp.Value;
+                if (kvp.Value != null)
+                {
+                    // This will replace the original value for the specified key.
+                    // Values from the matched route will take preference over previous
+                    // data in the route context.
+                    destination[kvp.Key] = kvp.Value;
+                }
             }
         }
 

diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Routing/InnerAttributeRouteTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Routing/InnerAttributeRouteTest.cs
@@ -1463,6 +1463,62 @@ public async Task AttributeRoute_CreatesNewRouteData()
             Assert.Equal(next.Object.GetType(), context.RouteData.Routers[0].GetType());
         }
 
+        [Fact]
+        public async Task AttributeRoute_ReplacesExistingRouteValues_IfNotNull()
+        {
+            // Arrange
+            var router = new Mock<IRouter>();
+            router
+                .Setup(r => r.RouteAsync(It.IsAny<RouteContext>()))
+                .Callback<RouteContext>((c) =>
+                {
+                    c.IsHandled = true;
+                })
+                .Returns(Task.FromResult(true));
+
+            var entry = CreateMatchingEntry(router.Object, "Foo/{*path}", order: 0);
+            var route = CreateAttributeRoute(router.Object, entry);
+
+            var context = CreateRouteContext("/Foo/Bar");
+
+            var originalRouteData = context.RouteData;
+            originalRouteData.Values.Add("path", "default");
+
+            // Act
+            await route.RouteAsync(context);
+
+            // Assert
+            Assert.Equal("Bar", context.RouteData.Values["path"]);
+        }
+
+        [Fact]
+        public async Task AttributeRoute_DoesNotReplaceExistingRouteValues_IfNull()
+        {
+            // Arrange
+            var router = new Mock<IRouter>();
+            router
+                .Setup(r => r.RouteAsync(It.IsAny<RouteContext>()))
+                .Callback<RouteContext>((c) =>
+                {
+                    c.IsHandled = true;
+                })
+                .Returns(Task.FromResult(true));
+
+            var entry = CreateMatchingEntry(router.Object, "Foo/{*path}", order: 0);
+            var route = CreateAttributeRoute(router.Object, entry);
+
+            var context = CreateRouteContext("/Foo/");
+
+            var originalRouteData = context.RouteData;
+            originalRouteData.Values.Add("path", "default");
+
+            // Act
+            await route.RouteAsync(context);
+
+            // Assert
+            Assert.Equal("default", context.RouteData.Values["path"]);
+        }
+
         [Fact]
         public async Task AttributeRoute_CreatesNewRouteData_ResetsWhenNotMatched()
         {

diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs
@@ -22,7 +22,7 @@ public class RoutingTests
         private readonly Action<IServiceCollection> _configureServices = new RoutingWebSite.Startup().ConfigureServices;
 
         [Fact]
-        public async Task ConventionRoutedController_ActionIsReachable()
+        public async Task ConventionalRoutedController_ActionIsReachable()
         {
             // Arrange
             var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
@@ -50,7 +50,7 @@ public async Task ConventionRoutedController_ActionIsReachable()
         }
 
         [Fact]
-        public async Task ConventionRoutedController_ActionIsReachable_WithDefaults()
+        public async Task ConventionalRoutedController_ActionIsReachable_WithDefaults()
         {
             // Arrange
             var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
@@ -78,7 +78,7 @@ public async Task ConventionRoutedController_ActionIsReachable_WithDefaults()
         }
 
         [Fact]
-        public async Task ConventionRoutedController_NonActionIsNotReachable()
+        public async Task ConventionalRoutedController_NonActionIsNotReachable()
         {
             // Arrange
             var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
@@ -92,7 +92,7 @@ public async Task ConventionRoutedController_NonActionIsNotReachable()
         }
 
         [Fact]
-        public async Task ConventionRoutedController_InArea_ActionIsReachable()
+        public async Task ConventionalRoutedController_InArea_ActionIsReachable()
         {
             // Arrange
             var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
@@ -121,7 +121,7 @@ public async Task ConventionRoutedController_InArea_ActionIsReachable()
         }
 
         [Fact]
-        public async Task ConventionRoutedController_InArea_ActionBlockedByHttpMethod()
+        public async Task ConventionalRoutedController_InArea_ActionBlockedByHttpMethod()
         {
             // Arrange
             var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
@@ -134,6 +134,27 @@ public async Task ConventionRoutedController_InArea_ActionBlockedByHttpMethod()
             Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
         }
 
+        [Theory]
+        [InlineData("", "/Home/OptionalPath/default")]
+        [InlineData("CustomPath", "/Home/OptionalPath/CustomPath")]
+        public async Task ConventionalRoutedController_WithOptionalSegment(string optionalSegment, string expected)
+        {
+            // Arrange
+            var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
+            var client = server.CreateClient();
+
+            // Act
+            var response = await client.GetAsync("http://localhost/Home/OptionalPath/" + optionalSegment);
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+
+            var body = await response.Content.ReadAsStringAsync();
+            var result = JsonConvert.DeserializeObject<RoutingResult>(body);
+
+            Assert.Single(result.ExpectedUrls, expected);
+        }
+
         [Fact]
         public async Task AttributeRoutedAction_IsReachable()
         {
@@ -338,7 +359,7 @@ public async Task AttributeRoutedAction_IsNotReachableWithTraditionalRoute()
         // There's two actions at this URL - but attribute routes go in the route table
         // first.
         [Fact]
-        public async Task AttributeRoutedAction_TriedBeforeConventionRouting()
+        public async Task AttributeRoutedAction_TriedBeforeConventionalRouting()
         {
             // Arrange
             var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
@@ -721,6 +742,24 @@ public async Task AttributeRoutedAction_LinkGeneration_OrderOnActionOverridesOrd
             Assert.Equal("/Teams/AllOrganizations", response);
         }
 
+        [Theory]
+        [InlineData("", "/TeamName/DefaultName")]
+        [InlineData("CustomName", "/TeamName/CustomName")]
+        public async Task AttributeRoutedAction_PreservesDefaultValue_IfRouteValueIsNull(string teamName, string expected)
+        {
+            // Arrange
+            var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
+            var client = server.CreateClient();
+
+            // Act
+            var body = await client.GetStringAsync("http://localhost/TeamName/" + teamName);
+
+            // Assert
+            Assert.NotNull(body);
+            var result = JsonConvert.DeserializeObject<RoutingResult>(body);
+            Assert.Single(result.ExpectedUrls, expected);
+        }
+
         [Fact]
         public async Task AttributeRoutedAction_LinkToSelf()
         {

diff --git a/test/WebSites/RoutingWebSite/Controllers/HomeController.cs b/test/WebSites/RoutingWebSite/Controllers/HomeController.cs
@@ -30,5 +30,10 @@ public IActionResult Contact()
         {
             return _generator.Generate("/Home/Contact");
         }
+
+        public IActionResult OptionalPath(string path = "default")
+        {
+            return _generator.Generate("/Home/OptionalPath/" + path);
+        }
     }
 }
diff --git a/test/WebSites/RoutingWebSite/Controllers/TeamController.cs b/test/WebSites/RoutingWebSite/Controllers/TeamController.cs
@@ -62,5 +62,11 @@ public ActionResult GetAllTeams(int notRelevant)
         {
             return Content(Url.Action(), "text/plain");
         }
+
+        [HttpGet("/TeamName/{*Name}/")]
+        public ActionResult GetTeam(string name = "DefaultName")
+        {
+            return _generator.Generate("/TeamName/" + name);
+        }
     }
 }
diff --git a/test/WebSites/RoutingWebSite/Startup.cs b/test/WebSites/RoutingWebSite/Startup.cs
@@ -44,6 +44,10 @@ public void Configure(IApplicationBuilder app)
                     "DuplicateRoute",
                     "conventional/Duplicate",
                     defaults: new { controller = "Duplicate", action = "Duplicate" });
+
+                routes.MapRoute(
+                    "RouteWithOptionalSegment",
+                    "{controller}/{action}/{path?}");
             });
         }
     }