Fix aspnet/Routing#782

Currently MVC is still running the IActionConstraint implementations for
features that we've already moved into the routing layer. This has a
significant perf cost associated with, and so we want to skip it because
it's redundant. However if anyone has implemented their own
`IActionConstraint`-based features, they still need to just work.

This change takes the approach of skipping the action constraint phase
at runtime unless we see something 'unknown'. This is an all or nothing
choice, and will run action constraints if **any** action constraint we
don't special case exists. This is the most compatible behavior (running
redundant constraints) when the application is using constraints that
the developer implemented.

Another approach I considered was to eliminate these constraints as part
of the process of building ADs. I don't think that's ideal because
people have written code that introspects action constraints. We should
consider something like this in 3.0.

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

@@ -192,14 +192,43 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                         for (var i = 0; i < _actions.Items.Count; i++)
                         {
                             var action = _actions.Items[i];
-                            if (action.ActionConstraints?.Count > 0)
+                            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;

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

@@ -35,6 +35,9 @@ namespace Microsoft.AspNetCore.Mvc.Routing
         // Run really late.
         public override int Order => 100000;
 
+        // Internal for testing
+        internal bool ShouldRunActionConstraints => _actionConstraintCache.CurrentCache.HasActionConstraints;
+
         public Task ApplyAsync(HttpContext httpContext, EndpointFeature endpointFeature, CandidateSet candidateSet)
         {
             // PERF: we can skip over action constraints if there aren't any app-wide.
@@ -42,8 +45,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             // 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.
-            var actions = _actionConstraintCache.CurrentCache;
+            if (ShouldRunActionConstraints)
-            if (actions.HasActionConstraints)
             {
                 ApplyActionConstraints(httpContext, candidateSet);
             }

src/Microsoft.AspNetCore.Mvc.Cors/Internal/CorsHttpMethodActionConstraint.cs

@@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Mvc.Internal;
 
 namespace Microsoft.AspNetCore.Mvc.Cors.Internal
 {
+    // Don't casually change the name of this. We reference the full type name in ActionConstraintCache.
     public class CorsHttpMethodActionConstraint : HttpMethodActionConstraint
     {
         private readonly string OriginHeader = "Origin";

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

@@ -7,16 +7,17 @@ using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Mvc.ActionConstraints;
+using Microsoft.AspNetCore.Mvc.Cors.Internal;
 using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.AspNetCore.Mvc.Internal;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.AspNetCore.Routing.Matching;
-using Microsoft.AspNetCore.Routing.Patterns;
 using Moq;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Mvc.Routing
 {
+    // These tests are intentionally in Mvc.Test so we can also test the CORS action constraint.
     public class ActionConstraintMatcherPolicyTest
     {
         [Fact]
@@ -49,7 +50,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             {
                 ActionConstraints = new List<IActionConstraintMetadata>()
                 {
-                    new HttpMethodActionConstraint(new string[] { "POST" }),
+                    new BooleanConstraint() { Pass = true, },
                 },
                 Parameters = new List<ParameterDescriptor>(),
             };
@@ -336,6 +337,76 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             Assert.False(candidateSet[2].IsValidCandidate);
         }
 
+        [Fact]
+        public void ShouldRunActionConstraints_IgnoresIgnorableConstraints()
+        {
+            // Arrange
+            var actions = new ActionDescriptor[]
+            {
+                new ActionDescriptor()
+                {
+
+                },
+                new ActionDescriptor()
+                {
+                    ActionConstraints = new List<IActionConstraintMetadata>()
+                    {
+                        new HttpMethodActionConstraint(new[]{ "GET", }),
+                    },
+                },
+                new ActionDescriptor()
+                {
+                    ActionConstraints = new List<IActionConstraintMetadata>()
+                    {
+                        new ConsumesAttribute("text/json"),
+                    },
+                },
+                new ActionDescriptor()
+                {
+                    ActionConstraints = new List<IActionConstraintMetadata>()
+                    {
+                        new CorsHttpMethodActionConstraint(new HttpMethodActionConstraint(new[]{ "GET", })),
+                    },
+                },
+            };
+
+            var selector = CreateSelector(actions);
+
+            // Act
+            var result = selector.ShouldRunActionConstraints;
+
+            // Assert
+            Assert.False(result);
+        }
+
+        [Fact]
+        public void ShouldRunActionConstraints_RunsForArbitraryActionConstraint()
+        {
+            // Arrange
+            var actions = new ActionDescriptor[]
+            {
+                new ActionDescriptor()
+                {
+
+                },
+                new ActionDescriptor()
+                {
+                    ActionConstraints = new List<IActionConstraintMetadata>()
+                    {
+                        new BooleanConstraint(),
+                    },
+                },
+            };
+
+            var selector = CreateSelector(actions);
+
+            // Act
+            var result = selector.ShouldRunActionConstraints;
+
+            // Assert
+            Assert.True(result);
+        }
+
         private ActionConstraintMatcherPolicy CreateSelector(ActionDescriptor[] actions)
         {
             // We need to actually provide some actions with some action constraints metadata