React to IEndpointSelectorPolicy changes

build/dependencies.props

@@ -47,8 +47,8 @@
     <MicrosoftAspNetCoreRazorTagHelpersTestingSourcesPackageVersion>2.2.0-preview3-35359</MicrosoftAspNetCoreRazorTagHelpersTestingSourcesPackageVersion>
     <MicrosoftAspNetCoreResponseCachingAbstractionsPackageVersion>2.2.0-preview3-35359</MicrosoftAspNetCoreResponseCachingAbstractionsPackageVersion>
     <MicrosoftAspNetCoreResponseCachingPackageVersion>2.2.0-preview3-35359</MicrosoftAspNetCoreResponseCachingPackageVersion>
-    <MicrosoftAspNetCoreRoutingAbstractionsPackageVersion>2.2.0-a-preview3-endpoint-selector-17041</MicrosoftAspNetCoreRoutingAbstractionsPackageVersion>
+    <MicrosoftAspNetCoreRoutingAbstractionsPackageVersion>2.2.0-a-preview3-matcher-policy-17051</MicrosoftAspNetCoreRoutingAbstractionsPackageVersion>
-    <MicrosoftAspNetCoreRoutingPackageVersion>2.2.0-a-preview3-endpoint-selector-17041</MicrosoftAspNetCoreRoutingPackageVersion>
+    <MicrosoftAspNetCoreRoutingPackageVersion>2.2.0-a-preview3-matcher-policy-17051</MicrosoftAspNetCoreRoutingPackageVersion>
     <MicrosoftAspNetCoreServerIISIntegrationPackageVersion>2.2.0-preview3-35359</MicrosoftAspNetCoreServerIISIntegrationPackageVersion>
     <MicrosoftAspNetCoreServerKestrelPackageVersion>2.2.0-preview3-35359</MicrosoftAspNetCoreServerKestrelPackageVersion>
     <MicrosoftAspNetCoreSessionPackageVersion>2.2.0-preview3-35359</MicrosoftAspNetCoreSessionPackageVersion>

src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionConstraintCache.cs

@@ -150,10 +150,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             {
                 return null;
             }
-            
+
             var actionConstraints = new IActionConstraint[count];
             var actionConstraintIndex = 0;
-            for (int i = 0; i < items.Count; i++)
+            for (var i = 0; i < items.Count; i++)
             {
                 var actionConstraint = items[i].Constraint;
                 if (actionConstraint != null)
@@ -168,7 +168,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
         internal class InnerCache
         {
             private readonly ActionDescriptorCollection _actions;
-            private bool? _hasActionConstraints;
 
             public InnerCache(ActionDescriptorCollection actions)
             {
@@ -179,61 +178,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                 new ConcurrentDictionary<ActionDescriptor, CacheEntry>();
 
             public int Version => _actions.Version;
-
-            public bool HasActionConstraints
-            {
-                get
-                {
-                    // This is a safe race-condition, since it always transitions from null to non-null.
-                    // All writers will always get the same result.
-                    if (_hasActionConstraints == null)
-                    {
-                        var found = false;
-                        for (var i = 0; i < _actions.Items.Count; i++)
-                        {
-                            var action = _actions.Items[i];
-                            if (action.ActionConstraints?.Count > 0 && HasSignificantActionConstraint(action))
-                            {
-                                // We need to check for some specific action constraint implementations.
-                                // We've implemented consumes, and HTTP method support inside endpoint routing, so 
-                                // we don't need to run an 'action constraint phase' if those are the only constraints.
-                                found = true;
-                                break;
-                            }
-                        }
-
-                        _hasActionConstraints = found;
-
-                        bool HasSignificantActionConstraint(ActionDescriptor action)
-                        {
-                            for (var i = 0; i < action.ActionConstraints.Count; i++)
-                            {
-                                var actionConstraint = action.ActionConstraints[i];
-                                if (actionConstraint.GetType() == typeof(HttpMethodActionConstraint))
-                                {
-                                    // This one is OK, we implement this in endpoint routing.
-                                }
-                                else if (actionConstraint.GetType().FullName == "Microsoft.AspNetCore.Mvc.Cors.Internal.CorsHttpMethodActionConstraint")
-                                {
-                                    // This one is OK, we implement this in endpoint routing.
-                                }
-                                else if (actionConstraint.GetType() == typeof(ConsumesAttribute))
-                                {
-                                    // This one is OK, we implement this in endpoint routing.
-                                }
-                                else
-                                {
-                                    return true;
-                                }
-                            }
-
-                            return false;
-                        }
-                    }
-
-                    return _hasActionConstraints.Value;
-                }
-            }
         }
 
         internal readonly struct CacheEntry

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

@@ -11,7 +11,6 @@ using Microsoft.AspNetCore.Mvc.ActionConstraints;
 using Microsoft.AspNetCore.Mvc.Internal;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.AspNetCore.Routing.Matching;
-using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Mvc.Routing
 {
@@ -24,7 +23,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
         // We need to be able to run IActionConstraints on Endpoints that aren't associated
         // with an action. This is a sentinel value we use when the endpoint isn't from MVC.
         internal static readonly ActionDescriptor NonAction = new ActionDescriptor();
-        
+
         private readonly ActionConstraintCache _actionConstraintCache;
 
         public ActionConstraintMatcherPolicy(ActionConstraintCache actionConstraintCache)
@@ -35,27 +34,59 @@ namespace Microsoft.AspNetCore.Mvc.Routing
         // Run really late.
         public override int Order => 100000;
 
-        // Internal for testing
+        public bool AppliesToEndpoints(IReadOnlyList<Endpoint> endpoints)
-        internal bool ShouldRunActionConstraints => _actionConstraintCache.CurrentCache.HasActionConstraints;
-
-        public Task ApplyAsync(HttpContext httpContext, EndpointSelectorContext context, CandidateSet candidateSet)
         {
-            // PERF: we can skip over action constraints if there aren't any app-wide.
+            if (endpoints == null)
-            //
-            // Running action constraints (or just checking for them) in a candidate set
-            // is somewhat expensive compared to other routing operations. This should only
-            // happen if user-code adds action constraints.
-            if (ShouldRunActionConstraints)
             {
-                ApplyActionConstraints(httpContext, candidateSet);
+                throw new ArgumentNullException(nameof(endpoints));
             }
 
-            return Task.CompletedTask;
+            // We can skip over action constraints when they aren't any for this set
+            // of endpoints. This happens once on startup so it removes this component
+            // from the code path in most scenarios.
+            for (var i = 0; i < endpoints.Count; i++)
+            {
+                var endpoint = endpoints[i];
+                var action = endpoint.Metadata.GetMetadata<ActionDescriptor>();
+                if (action?.ActionConstraints?.Count > 0 && HasSignificantActionConstraint(action))
+                {
+                    // We need to check for some specific action constraint implementations.
+                    // We've implemented consumes, and HTTP method support inside endpoint routing, so 
+                    // we don't need to run an 'action constraint phase' if those are the only constraints.
+                    return true;
+                }
+            }
+
+            return false;
+
+            bool HasSignificantActionConstraint(ActionDescriptor a)
+            {
+                for (var i = 0; i < a.ActionConstraints.Count; i++)
+                {
+                    var actionConstraint = a.ActionConstraints[i];
+                    if (actionConstraint.GetType() == typeof(HttpMethodActionConstraint))
+                    {
+                        // This one is OK, we implement this in endpoint routing.
+                    }
+                    else if (actionConstraint.GetType().FullName == "Microsoft.AspNetCore.Mvc.Cors.Internal.CorsHttpMethodActionConstraint")
+                    {
+                        // This one is OK, we implement this in endpoint routing.
+                    }
+                    else if (actionConstraint.GetType() == typeof(ConsumesAttribute))
+                    {
+                        // This one is OK, we implement this in endpoint routing.
+                    }
+                    else
+                    {
+                        return true;
+                    }
+                }
+
+                return false;
+            }
         }
-        
+
-        private void ApplyActionConstraints(
+        public Task ApplyAsync(HttpContext httpContext, EndpointSelectorContext context, CandidateSet candidateSet)
-            HttpContext httpContext,
-            CandidateSet candidateSet)
         {
             var finalMatches = EvaluateActionConstraints(httpContext, candidateSet);
 
@@ -74,6 +105,8 @@ namespace Microsoft.AspNetCore.Mvc.Routing
                     candidateSet.SetValidity(finalMatches[i].index, true);
                 }
             }
+
+            return Task.CompletedTask;
         }
 
         // This is almost the same as the code in ActionSelector, but we can't really share the logic
@@ -171,8 +204,10 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             var endpointsWithConstraint = new List<(int index, ActionSelectorCandidate candidate)>();
             var endpointsWithoutConstraint = new List<(int index, ActionSelectorCandidate candidate)>();
 
-            var constraintContext = new ActionConstraintContext();
+            var constraintContext = new ActionConstraintContext
-            constraintContext.Candidates = items.Select(i => i.candidate).ToArray();
+            {
+                Candidates = items.Select(i => i.candidate).ToArray()
+            };
 
             // Perf: Avoid allocations
             for (var i = 0; i < items.Count; i++)

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

@@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
 
         public IComparer<Endpoint> Comparer { get; } = new ConsumesMetadataEndpointComparer();
 
-        public bool AppliesToNode(IReadOnlyList<Endpoint> endpoints)
+        public bool AppliesToEndpoints(IReadOnlyList<Endpoint> endpoints)
         {
             if (endpoints == null)
             {
@@ -173,7 +173,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             return new ConsumesPolicyJumpTable(exitDestination, ordered);
         }
 
-        private int GetScore(MediaType mediaType)
+        private int GetScore(in MediaType mediaType)
         {
             // Higher score == lower priority - see comments on MediaType.
             if (mediaType.MatchesAllTypes)

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

@@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
     public class ConsumesMatcherPolicyTest
     {
         [Fact]
-        public void AppliesToNode_EndpointWithoutMetadata_ReturnsFalse()
+        public void AppliesToEndpoints_EndpointWithoutMetadata_ReturnsFalse()
         {
             // Arrange
             var endpoints = new[] { CreateEndpoint("/", null), };
@@ -24,14 +24,14 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             var policy = CreatePolicy();
 
             // Act
-            var result = policy.AppliesToNode(endpoints);
+            var result = policy.AppliesToEndpoints(endpoints);
 
             // Assert
             Assert.False(result);
         }
 
         [Fact]
-        public void AppliesToNode_EndpointWithoutContentTypes_ReturnsFalse()
+        public void AppliesToEndpoints_EndpointWithoutContentTypes_ReturnsFalse()
         {
             // Arrange
             var endpoints = new[]
@@ -42,14 +42,14 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             var policy = CreatePolicy();
 
             // Act
-            var result = policy.AppliesToNode(endpoints);
+            var result = policy.AppliesToEndpoints(endpoints);
 
             // Assert
             Assert.False(result);
         }
 
         [Fact]
-        public void AppliesToNode_EndpointHasContentTypes_ReturnsTrue()
+        public void AppliesToEndpoints_EndpointHasContentTypes_ReturnsTrue()
         {
             // Arrange
             var endpoints = new[]
@@ -61,7 +61,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             var policy = CreatePolicy();
 
             // Act
-            var result = policy.AppliesToNode(endpoints);
+            var result = policy.AppliesToEndpoints(endpoints);
 
             // Assert
             Assert.True(result);

test/Microsoft.AspNetCore.Mvc.Test/Routing/ActionConstraintMatcherPolicyTest.cs

@@ -338,7 +338,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
         }
 
         [Fact]
-        public void ShouldRunActionConstraints_IgnoresIgnorableConstraints()
+        public void AppliesToEndpoints_IgnoresIgnorableConstraints()
         {
             // Arrange
             var actions = new ActionDescriptor[]
@@ -369,11 +369,12 @@ namespace Microsoft.AspNetCore.Mvc.Routing
                     },
                 },
             };
+            var endpoints = actions.Select(CreateEndpoint).ToArray();
 
             var selector = CreateSelector(actions);
 
             // Act
-            var result = selector.ShouldRunActionConstraints;
+            var result = selector.AppliesToEndpoints(endpoints);
 
             // Assert
             Assert.False(result);
@@ -397,11 +398,12 @@ namespace Microsoft.AspNetCore.Mvc.Routing
                     },
                 },
             };
+            var endpoints = actions.Select(CreateEndpoint).ToArray();
 
             var selector = CreateSelector(actions);
 
             // Act
-            var result = selector.ShouldRunActionConstraints;
+            var result = selector.AppliesToEndpoints(endpoints);
 
             // Assert
             Assert.True(result);