Fix dynamic routes with no route values

Fixes: #12915

This was just missing a null check.

Also added unit tests that were missing for these types.

src/Mvc/Mvc.Core/src/Infrastructure/ActionSelectionTable.cs

@@ -66,10 +66,11 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
                 // For action selection, ignore attribute routed actions
                 items: actions.Items.Where(a => a.AttributeRouteInfo == null),
 
-                getRouteKeys: a => a.RouteValues.Keys,
+                getRouteKeys: a => a.RouteValues?.Keys,
                 getRouteValue: (a, key) =>
                 {
-                    a.RouteValues.TryGetValue(key, out var value);
+                    string value = null;
+                    a.RouteValues?.TryGetValue(key, out value);
                     return value ?? string.Empty;
                 });
         }
@@ -87,10 +88,11 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
                     return e.GetType() == typeof(Endpoint);
                 }),
 
-                getRouteKeys: e => e.Metadata.GetMetadata<ActionDescriptor>().RouteValues.Keys,
+                getRouteKeys: e => e.Metadata.GetMetadata<ActionDescriptor>()?.RouteValues?.Keys,
                 getRouteValue: (e, key) =>
                 {
-                    e.Metadata.GetMetadata<ActionDescriptor>().RouteValues.TryGetValue(key, out var value);
+                    string value = null;
+                    e.Metadata.GetMetadata<ActionDescriptor>()?.RouteValues?.TryGetValue(key, out value);
                     return Convert.ToString(value, CultureInfo.InvariantCulture) ?? string.Empty;
                 });
         }
@@ -112,9 +114,13 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
 
             foreach (var item in items)
             {
-                foreach (var key in getRouteKeys(item))
+                var keys = getRouteKeys(item);
+                if (keys != null)
                 {
-                    routeKeys.Add(key);
+                    foreach (var key in keys)
+                    {
+                        routeKeys.Add(key);
+                    }
                 }
             }
 

src/Mvc/Mvc.Core/src/Routing/DynamicControllerEndpointMatcherPolicy.cs

@@ -138,9 +138,12 @@ namespace Microsoft.AspNetCore.Mvc.Routing
                 var values = new RouteValueDictionary(dynamicValues);
 
                 // Include values that were matched by the fallback route.
-                foreach (var kvp in originalValues)
+                if (originalValues != null)
                 {
-                    values.TryAdd(kvp.Key, kvp.Value);
+                    foreach (var kvp in originalValues)
+                    {
+                        values.TryAdd(kvp.Key, kvp.Value);
+                    }
                 }
 
                 // Update the route values

src/Mvc/Mvc.Core/src/Routing/DynamicControllerEndpointSelector.cs

@@ -11,10 +11,17 @@ namespace Microsoft.AspNetCore.Mvc.Routing
 {
     internal class DynamicControllerEndpointSelector : IDisposable
     {
-        private readonly ControllerActionEndpointDataSource _dataSource;
+        private readonly EndpointDataSource _dataSource;
         private readonly DataSourceDependentCache<ActionSelectionTable<Endpoint>> _cache;
 
         public DynamicControllerEndpointSelector(ControllerActionEndpointDataSource dataSource)
+            : this((EndpointDataSource)dataSource)
+        {
+        }
+
+        // Exposed for tests. We need to accept a more specific type in the constructor for DI
+        // to work.
+        protected DynamicControllerEndpointSelector(EndpointDataSource dataSource)
         {
             if (dataSource == null)
             {

src/Mvc/Mvc.Core/test/Routing/DynamicControllerEndpointMatcherPolicyTest.cs

@@ -0,0 +1,272 @@
+// Copyright (c) .NET Foundation. 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;
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc.Abstractions;
+using Microsoft.AspNetCore.Mvc.Controllers;
+using Microsoft.AspNetCore.Routing;
+using Microsoft.AspNetCore.Routing.Matching;
+using Microsoft.Extensions.DependencyInjection;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Mvc.Routing
+{
+    public class DynamicControllerEndpointMatcherPolicyTest
+    {
+        public DynamicControllerEndpointMatcherPolicyTest()
+        {
+            var actions = new ActionDescriptor[]
+            {
+                new ControllerActionDescriptor()
+                {
+                    RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+                    {
+                        ["action"] = "Index",
+                        ["controller"] = "Home",
+                    },
+                },
+                new ControllerActionDescriptor()
+                {
+                    RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+                    {
+                        ["action"] = "About",
+                        ["controller"] = "Home",
+                    },
+                },
+                new ControllerActionDescriptor()
+                {
+                    RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+                    {
+                        ["action"] = "Index",
+                        ["controller"] = "Blog",
+                    },
+                }
+            };
+
+            ControllerEndpoints = new[]
+            {
+                new Endpoint(_ => Task.CompletedTask, new EndpointMetadataCollection(actions[0]), "Test1"),
+                new Endpoint(_ => Task.CompletedTask, new EndpointMetadataCollection(actions[1]), "Test2"),
+                new Endpoint(_ => Task.CompletedTask, new EndpointMetadataCollection(actions[2]), "Test3"),
+            };
+
+            DynamicEndpoint = new Endpoint(
+                _ => Task.CompletedTask,
+                new EndpointMetadataCollection(new object[]
+                {
+                    new DynamicControllerRouteValueTransformerMetadata(typeof(CustomTransformer)),
+                }),
+                "dynamic");
+
+            DataSource = new DefaultEndpointDataSource(ControllerEndpoints);
+
+            Selector = new TestDynamicControllerEndpointSelector(DataSource);
+
+            var services = new ServiceCollection();
+            services.AddRouting();
+            services.AddScoped<CustomTransformer>(s =>
+            {
+                var transformer = new CustomTransformer();
+                transformer.Transform = (c, values) => Transform(c, values);
+                return transformer;
+            });
+            Services = services.BuildServiceProvider();
+
+            Comparer = Services.GetRequiredService<EndpointMetadataComparer>();
+        }
+
+        private EndpointMetadataComparer Comparer { get; }
+
+        private DefaultEndpointDataSource DataSource { get; }
+
+        private Endpoint[] ControllerEndpoints { get; }
+
+        private Endpoint DynamicEndpoint { get; }
+
+        private DynamicControllerEndpointSelector Selector { get; }
+
+        private IServiceProvider Services { get; }
+
+        private Func<HttpContext, RouteValueDictionary, ValueTask<RouteValueDictionary>> Transform { get; set; }
+
+        [Fact]
+        public async Task ApplyAsync_NoMatch()
+        {
+            // Arrange
+            var policy = new DynamicControllerEndpointMatcherPolicy(Selector, Comparer);
+
+            var endpoints = new[] { DynamicEndpoint, };
+            var values = new RouteValueDictionary[] { null, };
+            var scores = new[] { 0, };
+
+            var candidates = new CandidateSet(endpoints, values, scores);
+            candidates.SetValidity(0, false);
+
+            Transform = (c, values) =>
+            {
+                throw new InvalidOperationException();
+            };
+
+            var httpContext = new DefaultHttpContext()
+            {
+                RequestServices = Services,
+            };
+
+            // Act
+            await policy.ApplyAsync(httpContext, candidates);
+
+            // Assert
+            Assert.False(candidates.IsValidCandidate(0));
+        }
+
+        [Fact]
+        public async Task ApplyAsync_HasMatchNoEndpointFound()
+        {
+            // Arrange
+            var policy = new DynamicControllerEndpointMatcherPolicy(Selector, Comparer);
+
+            var endpoints = new[] { DynamicEndpoint, };
+            var values = new RouteValueDictionary[] { null, };
+            var scores = new[] { 0, };
+
+            var candidates = new CandidateSet(endpoints, values, scores);
+
+            Transform = (c, values) =>
+            {
+                return new ValueTask<RouteValueDictionary>(new RouteValueDictionary());
+            };
+
+            var httpContext = new DefaultHttpContext()
+            {
+                RequestServices = Services,
+            };
+
+            // Act
+            await policy.ApplyAsync(httpContext, candidates);
+
+            // Assert
+            Assert.Null(candidates[0].Endpoint);
+            Assert.Null(candidates[0].Values);
+            Assert.False(candidates.IsValidCandidate(0));
+        }
+
+        [Fact]
+        public async Task ApplyAsync_HasMatchFindsEndpoint_WithoutRouteValues()
+        {
+            // Arrange
+            var policy = new DynamicControllerEndpointMatcherPolicy(Selector, Comparer);
+
+            var endpoints = new[] { DynamicEndpoint, };
+            var values = new RouteValueDictionary[] { null, };
+            var scores = new[] { 0, };
+
+            var candidates = new CandidateSet(endpoints, values, scores);
+
+            Transform = (c, values) =>
+            {
+                return new ValueTask<RouteValueDictionary>(new RouteValueDictionary(new
+                {
+                    controller = "Home",
+                    action = "Index",
+                }));
+            };
+
+            var httpContext = new DefaultHttpContext()
+            {
+                RequestServices = Services,
+            };
+
+            // Act
+            await policy.ApplyAsync(httpContext, candidates);
+
+            // Assert
+            Assert.Same(ControllerEndpoints[0], candidates[0].Endpoint);
+            Assert.Collection(
+                candidates[0].Values.OrderBy(kvp => kvp.Key),
+                kvp =>
+                {
+                    Assert.Equal("action", kvp.Key);
+                    Assert.Equal("Index", kvp.Value);
+                },
+                kvp =>
+                {
+                    Assert.Equal("controller", kvp.Key);
+                    Assert.Equal("Home", kvp.Value);
+                });
+            Assert.True(candidates.IsValidCandidate(0));
+        }
+
+        [Fact]
+        public async Task ApplyAsync_HasMatchFindsEndpoint_WithRouteValues()
+        {
+            // Arrange
+            var policy = new DynamicControllerEndpointMatcherPolicy(Selector, Comparer);
+
+            var endpoints = new[] { DynamicEndpoint, };
+            var values = new RouteValueDictionary[] { new RouteValueDictionary(new { slug = "test", }), };
+            var scores = new[] { 0, };
+
+            var candidates = new CandidateSet(endpoints, values, scores);
+
+            Transform = (c, values) =>
+            {
+                return new ValueTask<RouteValueDictionary>(new RouteValueDictionary(new
+                {
+                    controller = "Home",
+                    action = "Index",
+                }));
+            };
+
+            var httpContext = new DefaultHttpContext()
+            {
+                RequestServices = Services,
+            };
+
+            // Act
+            await policy.ApplyAsync(httpContext, candidates);
+
+            // Assert
+            Assert.Same(ControllerEndpoints[0], candidates[0].Endpoint);
+            Assert.Collection(
+                candidates[0].Values.OrderBy(kvp => kvp.Key),
+                kvp =>
+                {
+                    Assert.Equal("action", kvp.Key);
+                    Assert.Equal("Index", kvp.Value);
+                },
+                kvp =>
+                {
+                    Assert.Equal("controller", kvp.Key);
+                    Assert.Equal("Home", kvp.Value);
+                }, 
+                kvp =>
+                {
+                    Assert.Equal("slug", kvp.Key);
+                    Assert.Equal("test", kvp.Value);
+                });
+            Assert.True(candidates.IsValidCandidate(0));
+        }
+
+        private class TestDynamicControllerEndpointSelector : DynamicControllerEndpointSelector
+        {
+            public TestDynamicControllerEndpointSelector(EndpointDataSource dataSource)
+                : base(dataSource)
+            {
+            }
+        }
+
+        private class CustomTransformer : DynamicRouteValueTransformer
+        {
+            public Func<HttpContext, RouteValueDictionary, ValueTask<RouteValueDictionary>> Transform { get; set; }
+
+            public override ValueTask<RouteValueDictionary> TransformAsync(HttpContext httpContext, RouteValueDictionary values)
+            {
+                return Transform(httpContext, values);
+            }
+        }
+    }
+}

src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointMatcherPolicy.cs

@@ -146,9 +146,12 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
                 var values = new RouteValueDictionary(dynamicValues);
 
                 // Include values that were matched by the fallback route.
-                foreach (var kvp in originalValues)
+                if (originalValues != null)
                 {
-                    values.TryAdd(kvp.Key, kvp.Value);
+                    foreach (var kvp in originalValues)
+                    {
+                        values.TryAdd(kvp.Key, kvp.Value);
+                    }
                 }
 
                 // Update the route values

src/Mvc/Mvc.RazorPages/src/Infrastructure/DynamicPageEndpointSelector.cs

@@ -11,10 +11,17 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
 {
     internal class DynamicPageEndpointSelector : IDisposable
     {
-        private readonly PageActionEndpointDataSource _dataSource;
+        private readonly EndpointDataSource _dataSource;
         private readonly DataSourceDependentCache<ActionSelectionTable<Endpoint>> _cache;
 
         public DynamicPageEndpointSelector(PageActionEndpointDataSource dataSource)
+            : this((EndpointDataSource)dataSource)
+        {
+        }
+
+        // Exposed for tests. We need to accept a more specific type in the constructor for DI
+        // to work.
+        protected DynamicPageEndpointSelector(EndpointDataSource dataSource)
         {
             if (dataSource == null)
             {

src/Mvc/Mvc.RazorPages/test/Infrastructure/DynamicPageEndpointMatcherPolicyTest.cs

@@ -0,0 +1,265 @@
+// Copyright (c) .NET Foundation. 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;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc.Abstractions;
+using Microsoft.AspNetCore.Mvc.Controllers;
+using Microsoft.AspNetCore.Mvc.Routing;
+using Microsoft.AspNetCore.Routing;
+using Microsoft.AspNetCore.Routing.Matching;
+using Microsoft.Extensions.DependencyInjection;
+using Moq;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
+{
+    public class DynamicPageEndpointMatcherPolicyTest
+    {
+        public DynamicPageEndpointMatcherPolicyTest()
+        {
+            var actions = new ActionDescriptor[]
+            {
+                new PageActionDescriptor()
+                {
+                    RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+                    {
+                        ["page"] = "/Index",
+                    },
+                },
+                new PageActionDescriptor()
+                {
+                    RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+                    {
+                        ["page"] = "/About",
+                    },
+                },
+            };
+
+            PageEndpoints = new[]
+            {
+                new Endpoint(_ => Task.CompletedTask, new EndpointMetadataCollection(actions[0]), "Test1"),
+                new Endpoint(_ => Task.CompletedTask, new EndpointMetadataCollection(actions[1]), "Test2"),
+            };
+
+            DynamicEndpoint = new Endpoint(
+                _ => Task.CompletedTask,
+                new EndpointMetadataCollection(new object[]
+                {
+                    new DynamicPageRouteValueTransformerMetadata(typeof(CustomTransformer)),
+                }),
+                "dynamic");
+
+            DataSource = new DefaultEndpointDataSource(PageEndpoints);
+
+            Selector = new TestDynamicPageEndpointSelector(DataSource);
+
+            var services = new ServiceCollection();
+            services.AddRouting();
+            services.AddScoped<CustomTransformer>(s =>
+            {
+                var transformer = new CustomTransformer();
+                transformer.Transform = (c, values) => Transform(c, values);
+                return transformer;
+            });
+            Services = services.BuildServiceProvider();
+
+            Comparer = Services.GetRequiredService<EndpointMetadataComparer>();
+
+            LoadedEndpoint = new Endpoint(_ => Task.CompletedTask, EndpointMetadataCollection.Empty, "Loaded");
+
+            var loader = new Mock<PageLoader>();
+            loader
+                .Setup(l => l.LoadAsync(It.IsAny<PageActionDescriptor>()))
+                .Returns(Task.FromResult(new CompiledPageActionDescriptor() { Endpoint = LoadedEndpoint, }));
+            Loader = loader.Object;
+            
+        }
+
+        private EndpointMetadataComparer Comparer { get; }
+
+        private DefaultEndpointDataSource DataSource { get; }
+
+        private Endpoint[] PageEndpoints { get; }
+
+        private Endpoint DynamicEndpoint { get; }
+
+        private Endpoint LoadedEndpoint { get; }
+
+        private PageLoader Loader { get; }
+
+        private DynamicPageEndpointSelector Selector { get; }
+
+        private IServiceProvider Services { get; }
+
+        private Func<HttpContext, RouteValueDictionary, ValueTask<RouteValueDictionary>> Transform { get; set; }
+
+        [Fact]
+        public async Task ApplyAsync_NoMatch()
+        {
+            // Arrange
+            var policy = new DynamicPageEndpointMatcherPolicy(Selector, Loader, Comparer);
+
+            var endpoints = new[] { DynamicEndpoint, };
+            var values = new RouteValueDictionary[] { null, };
+            var scores = new[] { 0, };
+
+            var candidates = new CandidateSet(endpoints, values, scores);
+            candidates.SetValidity(0, false);
+
+            Transform = (c, values) =>
+            {
+                throw new InvalidOperationException();
+            };
+
+            var httpContext = new DefaultHttpContext()
+            {
+                RequestServices = Services,
+            };
+
+            // Act
+            await policy.ApplyAsync(httpContext, candidates);
+
+            // Assert
+            Assert.False(candidates.IsValidCandidate(0));
+        }
+
+        [Fact]
+        public async Task ApplyAsync_HasMatchNoEndpointFound()
+        {
+            // Arrange
+            var policy = new DynamicPageEndpointMatcherPolicy(Selector, Loader, Comparer);
+
+            var endpoints = new[] { DynamicEndpoint, };
+            var values = new RouteValueDictionary[] { null, };
+            var scores = new[] { 0, };
+
+            var candidates = new CandidateSet(endpoints, values, scores);
+
+            Transform = (c, values) =>
+            {
+                return new ValueTask<RouteValueDictionary>(new RouteValueDictionary());
+            };
+
+            var httpContext = new DefaultHttpContext()
+            {
+                RequestServices = Services,
+            };
+
+            // Act
+            await policy.ApplyAsync(httpContext, candidates);
+
+            // Assert
+            Assert.Null(candidates[0].Endpoint);
+            Assert.Null(candidates[0].Values);
+            Assert.False(candidates.IsValidCandidate(0));
+        }
+
+        [Fact]
+        public async Task ApplyAsync_HasMatchFindsEndpoint_WithoutRouteValues()
+        {
+            // Arrange
+            var policy = new DynamicPageEndpointMatcherPolicy(Selector, Loader, Comparer);
+
+            var endpoints = new[] { DynamicEndpoint, };
+            var values = new RouteValueDictionary[] { null, };
+            var scores = new[] { 0, };
+
+            var candidates = new CandidateSet(endpoints, values, scores);
+
+            Transform = (c, values) =>
+            {
+                return new ValueTask<RouteValueDictionary>(new RouteValueDictionary(new
+                {
+                    page = "/Index",
+                }));
+            };
+
+            var httpContext = new DefaultHttpContext()
+            {
+                RequestServices = Services,
+            };
+
+            // Act
+            await policy.ApplyAsync(httpContext, candidates);
+
+            // Assert
+            Assert.Same(LoadedEndpoint, candidates[0].Endpoint);
+            Assert.Collection(
+                candidates[0].Values.OrderBy(kvp => kvp.Key),
+                kvp =>
+                {
+                    Assert.Equal("page", kvp.Key);
+                    Assert.Equal("/Index", kvp.Value);
+                });
+            Assert.True(candidates.IsValidCandidate(0));
+        }
+
+        [Fact]
+        public async Task ApplyAsync_HasMatchFindsEndpoint_WithRouteValues()
+        {
+            // Arrange
+            var policy = new DynamicPageEndpointMatcherPolicy(Selector, Loader, Comparer);
+
+            var endpoints = new[] { DynamicEndpoint, };
+            var values = new RouteValueDictionary[] { new RouteValueDictionary(new { slug = "test", }), };
+            var scores = new[] { 0, };
+
+            var candidates = new CandidateSet(endpoints, values, scores);
+
+            Transform = (c, values) =>
+            {
+                return new ValueTask<RouteValueDictionary>(new RouteValueDictionary(new
+                {
+                    page = "/Index",
+                }));
+            };
+
+            var httpContext = new DefaultHttpContext()
+            {
+                RequestServices = Services,
+            };
+
+            // Act
+            await policy.ApplyAsync(httpContext, candidates);
+
+            // Assert
+            Assert.Same(LoadedEndpoint, candidates[0].Endpoint);
+            Assert.Collection(
+                candidates[0].Values.OrderBy(kvp => kvp.Key),
+                kvp =>
+                {
+                    Assert.Equal("page", kvp.Key);
+                    Assert.Equal("/Index", kvp.Value);
+                },
+                kvp =>
+                {
+                    Assert.Equal("slug", kvp.Key);
+                    Assert.Equal("test", kvp.Value);
+                });
+            Assert.True(candidates.IsValidCandidate(0));
+        }
+
+        private class TestDynamicPageEndpointSelector : DynamicPageEndpointSelector
+        {
+            public TestDynamicPageEndpointSelector(EndpointDataSource dataSource)
+                : base(dataSource)
+            {
+            }
+        }
+
+        private class CustomTransformer : DynamicRouteValueTransformer
+        {
+            public Func<HttpContext, RouteValueDictionary, ValueTask<RouteValueDictionary>> Transform { get; set; }
+
+            public override ValueTask<RouteValueDictionary> TransformAsync(HttpContext httpContext, RouteValueDictionary values)
+            {
+                return Transform(httpContext, values);
+            }
+        }
+    }
+}