From 4fbaea2463cf2d15a0cb5a87a24b69af3d397ca9 Mon Sep 17 00:00:00 2001 From: Ajay Bhargav Baaskaran Date: Tue, 25 Aug 2015 12:32:28 -0700 Subject: [PATCH] [Fixes #2931] AttributeRoute does not replace existing route values with null --- .../Routing/InnerAttributeRoute.cs | 11 ++-- .../Routing/InnerAttributeRouteTest.cs | 56 +++++++++++++++++++ .../RoutingTests.cs | 51 +++++++++++++++-- .../Controllers/HomeController.cs | 5 ++ .../Controllers/TeamController.cs | 6 ++ test/WebSites/RoutingWebSite/Startup.cs | 4 ++ 6 files changed, 123 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/InnerAttributeRoute.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/InnerAttributeRoute.cs index 8122509cbc..3e642871cf 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/InnerAttributeRoute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/InnerAttributeRoute.cs @@ -294,10 +294,13 @@ namespace Microsoft.AspNet.Mvc.Routing { 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 index 7ae4ffe7c0..9fa12d9a17 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Routing/InnerAttributeRouteTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Routing/InnerAttributeRouteTest.cs @@ -1463,6 +1463,62 @@ namespace Microsoft.AspNet.Mvc.Routing Assert.Equal(next.Object.GetType(), context.RouteData.Routers[0].GetType()); } + [Fact] + public async Task AttributeRoute_ReplacesExistingRouteValues_IfNotNull() + { + // Arrange + var router = new Mock(); + router + .Setup(r => r.RouteAsync(It.IsAny())) + .Callback((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(); + router + .Setup(r => r.RouteAsync(It.IsAny())) + .Callback((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 index b60473df30..8773e89ded 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests private readonly Action _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 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests } [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 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests } [Fact] - public async Task ConventionRoutedController_NonActionIsNotReachable() + public async Task ConventionalRoutedController_NonActionIsNotReachable() { // Arrange var server = TestHelper.CreateServer(_app, SiteName, _configureServices); @@ -92,7 +92,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests } [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 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests } [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 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests 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(body); + + Assert.Single(result.ExpectedUrls, expected); + } + [Fact] public async Task AttributeRoutedAction_IsReachable() { @@ -338,7 +359,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests // 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 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests 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(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 index d82a85080f..dcce436b63 100644 --- a/test/WebSites/RoutingWebSite/Controllers/HomeController.cs +++ b/test/WebSites/RoutingWebSite/Controllers/HomeController.cs @@ -30,5 +30,10 @@ namespace RoutingWebSite { return _generator.Generate("/Home/Contact"); } + + public IActionResult OptionalPath(string path = "default") + { + return _generator.Generate("/Home/OptionalPath/" + path); + } } } \ No newline at end of file diff --git a/test/WebSites/RoutingWebSite/Controllers/TeamController.cs b/test/WebSites/RoutingWebSite/Controllers/TeamController.cs index 42345cb868..264dd2b457 100644 --- a/test/WebSites/RoutingWebSite/Controllers/TeamController.cs +++ b/test/WebSites/RoutingWebSite/Controllers/TeamController.cs @@ -62,5 +62,11 @@ namespace RoutingWebSite { return Content(Url.Action(), "text/plain"); } + + [HttpGet("/TeamName/{*Name}/")] + public ActionResult GetTeam(string name = "DefaultName") + { + return _generator.Generate("/TeamName/" + name); + } } } \ No newline at end of file diff --git a/test/WebSites/RoutingWebSite/Startup.cs b/test/WebSites/RoutingWebSite/Startup.cs index 328b4dcc87..f8bf4d8baa 100644 --- a/test/WebSites/RoutingWebSite/Startup.cs +++ b/test/WebSites/RoutingWebSite/Startup.cs @@ -44,6 +44,10 @@ namespace RoutingWebSite "DuplicateRoute", "conventional/Duplicate", defaults: new { controller = "Duplicate", action = "Duplicate" }); + + routes.MapRoute( + "RouteWithOptionalSegment", + "{controller}/{action}/{path?}"); }); } }