Refactor KnownRouteValueConstraint to not require HttpContext (#8352)

build/dependencies.props

@@ -48,8 +48,8 @@
     <MicrosoftAspNetCoreRazorTagHelpersTestingSourcesPackageVersion>2.2.0-preview2-35090</MicrosoftAspNetCoreRazorTagHelpersTestingSourcesPackageVersion>
     <MicrosoftAspNetCoreResponseCachingAbstractionsPackageVersion>2.2.0-preview2-35090</MicrosoftAspNetCoreResponseCachingAbstractionsPackageVersion>
     <MicrosoftAspNetCoreResponseCachingPackageVersion>2.2.0-preview2-35090</MicrosoftAspNetCoreResponseCachingPackageVersion>
-    <MicrosoftAspNetCoreRoutingAbstractionsPackageVersion>2.2.0-preview2-35090</MicrosoftAspNetCoreRoutingAbstractionsPackageVersion>
+    <MicrosoftAspNetCoreRoutingAbstractionsPackageVersion>2.2.0-a-preview2-routeconstraint-httpcontext-16912</MicrosoftAspNetCoreRoutingAbstractionsPackageVersion>
-    <MicrosoftAspNetCoreRoutingPackageVersion>2.2.0-preview2-35090</MicrosoftAspNetCoreRoutingPackageVersion>
+    <MicrosoftAspNetCoreRoutingPackageVersion>2.2.0-a-preview2-routeconstraint-httpcontext-16912</MicrosoftAspNetCoreRoutingPackageVersion>
     <MicrosoftAspNetCoreServerIISIntegrationPackageVersion>2.2.0-preview2-35090</MicrosoftAspNetCoreServerIISIntegrationPackageVersion>
     <MicrosoftAspNetCoreServerKestrelPackageVersion>2.2.0-preview2-35090</MicrosoftAspNetCoreServerKestrelPackageVersion>
     <MicrosoftAspNetCoreSessionPackageVersion>2.2.0-preview2-35090</MicrosoftAspNetCoreSessionPackageVersion>

src/Microsoft.AspNetCore.Mvc.Core/Routing/KnownRouteValueConstraint.cs

@@ -14,8 +14,26 @@ namespace Microsoft.AspNetCore.Mvc.Routing
 {
     public class KnownRouteValueConstraint : IRouteConstraint
     {
+        private readonly IActionDescriptorCollectionProvider _actionDescriptorCollectionProvider;
         private RouteValuesCollection _cachedValuesCollection;
 
+        [Obsolete("This constructor is obsolete. Use KnownRouteValueConstraint.ctor(IActionDescriptorCollectionProvider) instead.")]
+        public KnownRouteValueConstraint()
+        {
+            // Empty constructor for backwards compatibility
+            // Services will need to be resolved from HttpContext when this ctor is used
+        }
+
+        public KnownRouteValueConstraint(IActionDescriptorCollectionProvider actionDescriptorCollectionProvider)
+        {
+            if (actionDescriptorCollectionProvider == null)
+            {
+                throw new ArgumentNullException(nameof(actionDescriptorCollectionProvider));
+            }
+
+            _actionDescriptorCollectionProvider = actionDescriptorCollectionProvider;
+        }
+
         public bool Match(
             HttpContext httpContext,
             IRouter route,
@@ -23,16 +41,6 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             RouteValueDictionary values,
             RouteDirection routeDirection)
         {
-            if (httpContext == null)
-            {
-                throw new ArgumentNullException(nameof(httpContext));
-            }
-
-            if (route == null)
-            {
-                throw new ArgumentNullException(nameof(route));
-            }
-
             if (routeKey == null)
             {
                 throw new ArgumentNullException(nameof(routeKey));
@@ -49,7 +57,9 @@ namespace Microsoft.AspNetCore.Mvc.Routing
                 var value = obj as string;
                 if (value != null)
                 {
-                    var allValues = GetAndCacheAllMatchingValues(routeKey, httpContext);
+                    var actionDescriptors = GetAndValidateActionDescriptors(httpContext);
+
+                    var allValues = GetAndCacheAllMatchingValues(routeKey, actionDescriptors);
                     foreach (var existingValue in allValues)
                     {
                         if (string.Equals(value, existingValue, StringComparison.OrdinalIgnoreCase))
@@ -63,9 +73,36 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             return false;
         }
 
-        private string[] GetAndCacheAllMatchingValues(string routeKey, HttpContext httpContext)
+        private ActionDescriptorCollection GetAndValidateActionDescriptors(HttpContext httpContext)
+        {
+            var actionDescriptorsProvider = _actionDescriptorCollectionProvider;
+
+            if (actionDescriptorsProvider == null)
+            {
+                // Only validate that HttpContext was passed to constraint if it is needed
+                if (httpContext == null)
+                {
+                    throw new ArgumentNullException(nameof(httpContext));
+                }
+
+                var services = httpContext.RequestServices;
+                actionDescriptorsProvider = services.GetRequiredService<IActionDescriptorCollectionProvider>();
+            }
+
+            var actionDescriptors = actionDescriptorsProvider.ActionDescriptors;
+            if (actionDescriptors == null)
+            {
+                throw new InvalidOperationException(
+                    Resources.FormatPropertyOfTypeCannotBeNull(
+                        nameof(IActionDescriptorCollectionProvider.ActionDescriptors),
+                        actionDescriptorsProvider.GetType()));
+            }
+
+            return actionDescriptors;
+        }
+
+        private string[] GetAndCacheAllMatchingValues(string routeKey, ActionDescriptorCollection actionDescriptors)
         {
-            var actionDescriptors = GetAndValidateActionDescriptorCollection(httpContext);
             var version = actionDescriptors.Version;
             var valuesCollection = _cachedValuesCollection;
 
@@ -77,8 +114,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
                 {
                     var action = actionDescriptors.Items[i];
 
-                    string value;
+                    if (action.RouteValues.TryGetValue(routeKey, out var value) &&
-                    if (action.RouteValues.TryGetValue(routeKey, out value) &&
                         !string.IsNullOrEmpty(value))
                     {
                         values.Add(value);
@@ -92,22 +128,6 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             return _cachedValuesCollection.Items;
         }
 
-        private static ActionDescriptorCollection GetAndValidateActionDescriptorCollection(HttpContext httpContext)
-        {
-            var services = httpContext.RequestServices;
-            var provider = services.GetRequiredService<IActionDescriptorCollectionProvider>();
-            var descriptors = provider.ActionDescriptors;
-
-            if (descriptors == null)
-            {
-                throw new InvalidOperationException(
-                    Resources.FormatPropertyOfTypeCannotBeNull("ActionDescriptors",
-                                                               provider.GetType()));
-            }
-
-            return descriptors;
-        }
-
         private class RouteValuesCollection
         {
             public RouteValuesCollection(int version, string[] items)

test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs

@@ -180,7 +180,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             var actionDescriptorCollection = GetActionDescriptorCollection(
                 new { controller = "TestController", action = "TestAction", area = "TestArea" });
             var dataSource = CreateMvcEndpointDataSource(actionDescriptorCollection);
-            dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(string.Empty, endpointInfoRoute));
+
+            var services = new ServiceCollection();
+            services.AddRouting();
+            services.AddSingleton(actionDescriptorCollection);
+
+            var routeOptionsSetup = new MvcCoreRouteOptionsSetup();
+            services.Configure<RouteOptions>(routeOptionsSetup.Configure);
+
+            dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(string.Empty, endpointInfoRoute, serviceProvider: services.BuildServiceProvider()));
 
             // Act
             var endpoints = dataSource.Endpoints;
@@ -719,13 +727,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                     Array.Empty<IActionDescriptorChangeProvider>());
             }
 
-            var serviceProviderMock = new Mock<IServiceProvider>();
+            var services = new ServiceCollection();
-            serviceProviderMock.Setup(m => m.GetService(typeof(IActionDescriptorCollectionProvider))).Returns(actionDescriptorCollectionProvider);
+            services.AddSingleton(actionDescriptorCollectionProvider);
 
             var dataSource = new MvcEndpointDataSource(
                 actionDescriptorCollectionProvider,
                 mvcEndpointInvokerFactory ?? new MvcEndpointInvokerFactory(new ActionInvokerFactory(Array.Empty<IActionInvokerProvider>())),
-                serviceProviderMock.Object);
+                services.BuildServiceProvider());
 
             return dataSource;
         }
@@ -735,15 +743,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             string template,
             RouteValueDictionary defaults = null,
             IDictionary<string, object> constraints = null,
-            RouteValueDictionary dataTokens = null)
+            RouteValueDictionary dataTokens = null,
+            IServiceProvider serviceProvider = null)
         {
-            var serviceCollection = new ServiceCollection();
+            if (serviceProvider == null)
-            serviceCollection.AddRouting();
+            {
+                var serviceCollection = new ServiceCollection();
+                serviceCollection.AddRouting();
 
-            var routeOptionsSetup = new MvcCoreRouteOptionsSetup();
+                var routeOptionsSetup = new MvcCoreRouteOptionsSetup();
-            serviceCollection.Configure<RouteOptions>(routeOptionsSetup.Configure);
+                serviceCollection.Configure<RouteOptions>(routeOptionsSetup.Configure);
 
-            var serviceProvider = serviceCollection.BuildServiceProvider();
+                serviceProvider = serviceCollection.BuildServiceProvider();
+            }
 
             var parameterPolicyFactory = serviceProvider.GetRequiredService<ParameterPolicyFactory>();
             return new MvcEndpointInfo(name, template, defaults, constraints, dataTokens, parameterPolicyFactory);

test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/KnownRouteValueConstraintTests.cs

@@ -2,7 +2,6 @@
 // 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 Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc.Abstractions;
@@ -10,6 +9,7 @@ using Microsoft.AspNetCore.Mvc.Controllers;
 using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.AspNetCore.Mvc.Internal;
 using Microsoft.AspNetCore.Routing;
+using Microsoft.Extensions.DependencyInjection;
 using Moq;
 using Xunit;
 
@@ -17,7 +17,47 @@ namespace Microsoft.AspNetCore.Mvc.Routing
 {
     public class KnownRouteValueConstraintTests
     {
+#pragma warning disable CS0618 // Type or member is obsolete
         private readonly IRouteConstraint _constraint = new KnownRouteValueConstraint();
+#pragma warning restore CS0618 // Type or member is obsolete
+
+        [Fact]
+        public void ResolveFromServices_InjectsServiceProvider_HttpContextNotNeeded()
+        {
+            // Arrange
+            var actionDescriptor = CreateActionDescriptor("testArea",
+                "testController",
+                "testAction");
+            actionDescriptor.RouteValues.Add("randomKey", "testRandom");
+            var descriptorCollectionProvider = CreateActionDesciprtorCollectionProvider(actionDescriptor);
+
+            var services = new ServiceCollection();
+            services.AddRouting();
+            services.AddSingleton(descriptorCollectionProvider);
+
+            var routeOptionsSetup = new MvcCoreRouteOptionsSetup();
+            services.Configure<RouteOptions>(routeOptionsSetup.Configure);
+
+            var serviceProvider = services.BuildServiceProvider();
+
+            var inlineConstraintResolver = serviceProvider.GetRequiredService<IInlineConstraintResolver>();
+            var constraint = inlineConstraintResolver.ResolveConstraint("exists");
+
+            var values = new RouteValueDictionary()
+            {
+                { "area", "testArea" },
+                { "controller", "testController" },
+                { "action", "testAction" },
+                { "randomKey", "testRandom" }
+            };
+
+            // Act
+            var knownRouteValueConstraint = Assert.IsType<KnownRouteValueConstraint>(constraint);
+            var match = knownRouteValueConstraint.Match(httpContext: null, route: null, "area", values, RouteDirection.IncomingRequest);
+
+            // Assert
+            Assert.True(match);
+        }
 
         [Theory]
         [InlineData("area", RouteDirection.IncomingRequest)]
@@ -55,8 +95,8 @@ namespace Microsoft.AspNetCore.Mvc.Routing
         {
             // Arrange
             var actionDescriptor = CreateActionDescriptor("testArea",
-                                                          "testController",
+                "testController",
-                                                          "testAction");
+                "testAction");
             actionDescriptor.RouteValues.Add("randomKey", "testRandom");
             var httpContext = GetHttpContext(actionDescriptor);
             var route = Mock.Of<IRouter>();
@@ -115,8 +155,8 @@ namespace Microsoft.AspNetCore.Mvc.Routing
         public void RouteValue_IsNotAString_MatchFails(RouteDirection direction)
         {
             var actionDescriptor = CreateActionDescriptor("testArea",
-                                                          controller: null,
+                controller: null,
-                                                          action: null);
+                action: null);
             var httpContext = GetHttpContext(actionDescriptor);
             var route = Mock.Of<IRouter>();
             var values = new RouteValueDictionary()
@@ -157,7 +197,57 @@ namespace Microsoft.AspNetCore.Mvc.Routing
                 ex.Message);
         }
 
-        private static HttpContext GetHttpContext(ActionDescriptor actionDescriptor)
+        [Theory]
+        [InlineData("area", RouteDirection.IncomingRequest)]
+        [InlineData("controller", RouteDirection.IncomingRequest)]
+        [InlineData("action", RouteDirection.IncomingRequest)]
+        [InlineData("randomKey", RouteDirection.IncomingRequest)]
+        [InlineData("area", RouteDirection.UrlGeneration)]
+        [InlineData("controller", RouteDirection.UrlGeneration)]
+        [InlineData("action", RouteDirection.UrlGeneration)]
+        [InlineData("randomKey", RouteDirection.UrlGeneration)]
+        public void ServiceInjected_RouteKey_Exists_MatchSucceeds(string keyName, RouteDirection direction)
+        {
+            // Arrange
+            var actionDescriptor = CreateActionDescriptor("testArea",
+                "testController",
+                "testAction");
+            actionDescriptor.RouteValues.Add("randomKey", "testRandom");
+
+            var provider = CreateActionDesciprtorCollectionProvider(actionDescriptor);
+
+            var constraint = new KnownRouteValueConstraint(provider);
+
+            var values = new RouteValueDictionary()
+            {
+                { "area", "testArea" },
+                { "controller", "testController" },
+                { "action", "testAction" },
+                { "randomKey", "testRandom" }
+            };
+
+            // Act
+            var match = constraint.Match(httpContext: null, route: null, keyName, values, direction);
+
+            // Assert
+            Assert.True(match);
+        }
+
+        private static HttpContext GetHttpContext(ActionDescriptor actionDescriptor, bool setupRequestServices = true)
+        {
+            var descriptorCollectionProvider = CreateActionDesciprtorCollectionProvider(actionDescriptor);
+
+            var context = new Mock<HttpContext>();
+            if (setupRequestServices)
+            {
+                context.Setup(o => o.RequestServices
+                    .GetService(typeof(IActionDescriptorCollectionProvider)))
+                    .Returns(descriptorCollectionProvider);
+            }
+            return context.Object;
+        }
+
+        private static IActionDescriptorCollectionProvider CreateActionDesciprtorCollectionProvider(ActionDescriptor actionDescriptor)
         {
             var actionProvider = new Mock<IActionDescriptorProvider>(MockBehavior.Strict);
 
@@ -176,12 +266,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             var descriptorCollectionProvider = new DefaultActionDescriptorCollectionProvider(
                 new[] { actionProvider.Object },
                 Enumerable.Empty<IActionDescriptorChangeProvider>());
-
+            return descriptorCollectionProvider;
-            var context = new Mock<HttpContext>();
-            context.Setup(o => o.RequestServices
-                .GetService(typeof(IActionDescriptorCollectionProvider)))
-                .Returns(descriptorCollectionProvider);
-            return context.Object;
         }
 
         private static ActionDescriptor CreateActionDescriptor(string area, string controller, string action)