Improve tests for catch-all + defaults/nulls

src/Microsoft.AspNetCore.Routing/Template/TemplateMatcher.cs

@@ -232,11 +232,11 @@ namespace Microsoft.AspNetCore.Routing.Template
                 // It's ok for a catch-all to produce a null value
                 if (_hasDefaultValue[i] || part.IsCatchAll)
                 {
-                    // Don't trounce an existing value with a null.
+                    // Don't replace an existing value with a null.
                     var defaultValue = _defaultValues[i];
                     if (defaultValue != null || !values.ContainsKey(part.Name))
                     {
-                        values[part.Name] = _defaultValues[i];
+                        values[part.Name] = defaultValue;
                     }
                 }
             }

test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateMatcherTests.cs

@@ -697,7 +697,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests
         }
 
         [Fact]
-        public void TryMatch_RouteWithCatchAllClauseCapturesManySlashes()
+        public void TryMatch_RouteWithCatchAll_MatchesMultiplePathSegments()
         {
             // Arrange
             var matcher = CreateMatcher("{p1}/{*p2}");
@@ -715,7 +715,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests
         }
 
         [Fact]
-        public void TryMatch_RouteWithCatchAllClauseCapturesTrailingSlash()
+        public void TryMatch_RouteWithCatchAll_MatchesTrailingSlash()
         {
             // Arrange
             var matcher = CreateMatcher("{p1}/{*p2}");
@@ -733,7 +733,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests
         }
 
         [Fact]
-        public void TryMatch_RouteWithCatchAllClauseCapturesEmptyContent()
+        public void TryMatch_RouteWithCatchAll_MatchesEmptyContent()
         {
             // Arrange
             var matcher = CreateMatcher("{p1}/{*p2}");
@@ -751,12 +751,30 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests
         }
 
         [Fact]
-        public void TryMatch_RouteWithCatchAllClauseUsesDefaultValueForEmptyContent()
+        public void TryMatch_RouteWithCatchAll_MatchesEmptyContent_DoesNotReplaceExistingRouteValue()
+        {
+            // Arrange
+            var matcher = CreateMatcher("{p1}/{*p2}");
+
+            var values = new RouteValueDictionary(new { p2 = "hello" });
+
+            // Act
+            var match = matcher.TryMatch("/v1", values);
+
+            // Assert
+            Assert.True(match);
+            Assert.Equal<int>(2, values.Count);
+            Assert.Equal("v1", values["p1"]);
+            Assert.Equal("hello", values["p2"]);
+        }
+
+        [Fact]
+        public void TryMatch_RouteWithCatchAll_UsesDefaultValueForEmptyContent()
         {
             // Arrange
             var matcher = CreateMatcher("{p1}/{*p2}", new { p2 = "catchall" });
 
-            var values = new RouteValueDictionary();
+            var values = new RouteValueDictionary(new { p2 = "overridden" });
 
             // Act
             var match = matcher.TryMatch("/v1", values);
@@ -769,12 +787,12 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests
         }
 
         [Fact]
-        public void TryMatch_RouteWithCatchAllClauseIgnoresDefaultValueForNonEmptyContent()
+        public void TryMatch_RouteWithCatchAll_IgnoresDefaultValueForNonEmptyContent()
         {
             // Arrange
             var matcher = CreateMatcher("{p1}/{*p2}", new { p2 = "catchall" });
 
-            var values = new RouteValueDictionary();
+            var values = new RouteValueDictionary(new { p2 = "overridden" });
 
             // Act
             var match = matcher.TryMatch("/v1/hello/whatever", values);

test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs

@@ -152,6 +152,97 @@ namespace Microsoft.AspNetCore.Routing.Tree
             Assert.Equal(expectedResult, context.RouteData.Values["path"]);
         }
 
+        [Theory]
+        [InlineData("a/{*path}", "/a")]
+        [InlineData("a/{*path}", "/a/")]
+        public async Task TreeRouter_RouteAsync_MatchesCatchAll_NullValue(
+            string template,
+            string requestPath)
+        {
+            // Arrange
+            var next = new Mock<IRouter>();
+            next
+                .Setup(r => r.RouteAsync(It.IsAny<RouteContext>()))
+                .Callback<RouteContext>(c => c.Handler = NullHandler)
+                .Returns(Task.FromResult(true))
+                .Verifiable();
+
+            var firstRoute = CreateMatchingEntry(next.Object, template, order: 0);
+            var matchingRoutes = new[] { firstRoute };
+            var linkGenerationEntries = Enumerable.Empty<TreeRouteLinkGenerationEntry>();
+            var attributeRoute = CreateAttributeRoute(next.Object, matchingRoutes, linkGenerationEntries);
+            var context = CreateRouteContext(requestPath);
+
+            // Act
+            await attributeRoute.RouteAsync(context);
+
+            // Assert
+            Assert.NotNull(context.Handler);
+            Assert.Null(context.RouteData.Values["path"]);
+        }
+
+        [Theory]
+        [InlineData("a/{*path}", "/a")]
+        [InlineData("a/{*path}", "/a/")]
+        public async Task TreeRouter_RouteAsync_MatchesCatchAll_NullValue_DoesNotReplaceExistingValue(
+            string template,
+            string requestPath)
+        {
+            // Arrange
+            var next = new Mock<IRouter>();
+            next
+                .Setup(r => r.RouteAsync(It.IsAny<RouteContext>()))
+                .Callback<RouteContext>(c => c.Handler = NullHandler)
+                .Returns(Task.FromResult(true))
+                .Verifiable();
+
+            var firstRoute = CreateMatchingEntry(next.Object, template, order: 0);
+            var matchingRoutes = new[] { firstRoute };
+            var linkGenerationEntries = Enumerable.Empty<TreeRouteLinkGenerationEntry>();
+            var attributeRoute = CreateAttributeRoute(next.Object, matchingRoutes, linkGenerationEntries);
+
+            var context = CreateRouteContext(requestPath);
+            context.RouteData.Values["path"] = "existing-value";
+
+            // Act
+            await attributeRoute.RouteAsync(context);
+
+            // Assert
+            Assert.NotNull(context.Handler);
+            Assert.Equal("existing-value", context.RouteData.Values["path"]);
+        }
+
+        [Theory]
+        [InlineData("a/{*path=default}", "/a")]
+        [InlineData("a/{*path=default}", "/a/")]
+        public async Task TreeRouter_RouteAsync_MatchesCatchAll_UsesDefaultValue(
+            string template,
+            string requestPath)
+        {
+            // Arrange
+            var next = new Mock<IRouter>();
+            next
+                .Setup(r => r.RouteAsync(It.IsAny<RouteContext>()))
+                .Callback<RouteContext>(c => c.Handler = NullHandler)
+                .Returns(Task.FromResult(true))
+                .Verifiable();
+
+            var firstRoute = CreateMatchingEntry(next.Object, template, order: 0);
+            var matchingRoutes = new[] { firstRoute };
+            var linkGenerationEntries = Enumerable.Empty<TreeRouteLinkGenerationEntry>();
+            var attributeRoute = CreateAttributeRoute(next.Object, matchingRoutes, linkGenerationEntries);
+
+            var context = CreateRouteContext(requestPath);
+            context.RouteData.Values["path"] = "existing-value";
+
+            // Act
+            await attributeRoute.RouteAsync(context);
+
+            // Assert
+            Assert.NotNull(context.Handler);
+            Assert.Equal("default", context.RouteData.Values["path"]);
+        }
+
         [Theory]
         [InlineData("template/5")]
         [InlineData("template/{parameter:int}")]
@@ -1581,9 +1672,19 @@ namespace Microsoft.AspNetCore.Routing.Tree
             entry.Target = router;
             entry.RouteTemplate = TemplateParser.Parse(template);
             var parsedRouteTemplate = TemplateParser.Parse(template);
-            entry.TemplateMatcher = new TemplateMatcher(
+
-                parsedRouteTemplate,
+            var defaults = new RouteValueDictionary();
-                new RouteValueDictionary(new { test_route_group = routeGroup }));
+            foreach (var parameter in parsedRouteTemplate.Parameters)
+            {
+                if (parameter.DefaultValue != null)
+                {
+                    defaults.Add(parameter.Name, parameter.DefaultValue);
+                }
+            }
+
+            defaults["test_route_group"] = routeGroup;
+
+            entry.TemplateMatcher = new TemplateMatcher(parsedRouteTemplate, defaults);
             entry.Precedence = RoutePrecedence.ComputeMatched(parsedRouteTemplate);
             entry.Order = order;
             entry.Constraints = GetRouteConstriants(CreateConstraintResolver(), template, parsedRouteTemplate);