Added caching for ActionConstraints

src/Microsoft.AspNet.Mvc.Abstractions/ActionConstraints/ActionConstraintItem.cs

@@ -34,5 +34,10 @@ namespace Microsoft.AspNet.Mvc.ActionConstraints
         /// The <see cref="IActionConstraintMetadata"/> instance.
         /// </summary>
         public IActionConstraintMetadata Metadata { get; private set; }
+
+        /// <summary>
+        /// Gets or sets a value indicating whether or not <see cref="Constraint"/> can be reused across requests.
+        /// </summary>
+        public bool IsReusable { get; set; }
     }
 }

src/Microsoft.AspNet.Mvc.Abstractions/ActionConstraints/IActionConstraintFactory.cs

@@ -17,6 +17,12 @@ namespace Microsoft.AspNet.Mvc.ActionConstraints
     /// </remarks>
     public interface IActionConstraintFactory : IActionConstraintMetadata
     {
+        /// <summary>
+        /// Gets a value that indicates if the result of <see cref="CreateInstance(IServiceProvider)"/>
+        /// can be reused across requests.
+        /// </summary>
+        bool IsReusable { get; }
+
         /// <summary>
         /// Creates a new <see cref="IActionConstraint"/>.
         /// </summary>

src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs

@@ -98,6 +98,7 @@ namespace Microsoft.Extensions.DependencyInjection
             // Action Selection
             //
             services.TryAddSingleton<IActionSelector, ActionSelector>();
+            services.TryAddSingleton<ActionConstraintCache>();
 
             // Performs caching
             services.TryAddSingleton<IActionSelectorDecisionTreeProvider, ActionSelectorDecisionTreeProvider>();

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

@@ -0,0 +1,200 @@
+// 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.Collections.Concurrent;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+using Microsoft.AspNet.Http;
+using Microsoft.AspNet.Mvc.Abstractions;
+using Microsoft.AspNet.Mvc.ActionConstraints;
+using Microsoft.AspNet.Mvc.Infrastructure;
+
+namespace Microsoft.AspNet.Mvc.Internal
+{
+    public class ActionConstraintCache
+    {
+        private readonly IActionDescriptorCollectionProvider _collectionProvider;
+        private readonly IActionConstraintProvider[] _actionConstraintProviders;
+
+        private volatile InnerCache _currentCache;
+
+        public ActionConstraintCache(
+            IActionDescriptorCollectionProvider collectionProvider,
+            IEnumerable<IActionConstraintProvider> actionConstraintProviders)
+        {
+            _collectionProvider = collectionProvider;
+            _actionConstraintProviders = actionConstraintProviders.OrderBy(item => item.Order).ToArray();
+        }
+
+        private InnerCache CurrentCache
+        {
+            get
+            {
+                var current = _currentCache;
+                var actionDescriptors = _collectionProvider.ActionDescriptors;
+
+                if (current == null || current.Version != actionDescriptors.Version)
+                {
+                    current = new InnerCache(actionDescriptors.Version);
+                    _currentCache = current;
+                }
+
+                return current;
+            }
+        }
+
+        public IReadOnlyList<IActionConstraint> GetActionConstraints(HttpContext httpContext, ActionDescriptor action)
+        {
+            var cache = CurrentCache;
+
+            CacheEntry entry;
+            if (cache.Entries.TryGetValue(action, out entry))
+            {
+                return GetActionConstraintsFromEntry(entry, httpContext, action);
+            }
+
+            if (action.ActionConstraints == null || action.ActionConstraints.Count == 0)
+            {
+                return null;
+            }
+
+            var items = new List<ActionConstraintItem>(action.ActionConstraints.Count);
+            for (var i = 0; i < action.ActionConstraints.Count; i++)
+            {
+                items.Add(new ActionConstraintItem(action.ActionConstraints[i]));
+            }
+
+            ExecuteProviders(httpContext, action, items);
+
+            var actionConstraints = ExtractActionConstraints(items);
+
+            var allActionConstraintsCached = true;
+            for (var i = 0; i < items.Count; i++)
+            {
+                var item = items[i];
+                if (!item.IsReusable)
+                {
+                    item.Constraint = null;
+                    allActionConstraintsCached = false;
+                }
+            }
+
+            if (allActionConstraintsCached)
+            {
+                entry = new CacheEntry(actionConstraints);
+            }
+            else
+            {
+                entry = new CacheEntry(items);
+            }
+
+            cache.Entries.TryAdd(action, entry);
+            return actionConstraints;
+        }
+
+        private IReadOnlyList<IActionConstraint> GetActionConstraintsFromEntry(CacheEntry entry, HttpContext httpContext, ActionDescriptor action)
+        {
+            Debug.Assert(entry.ActionConstraints != null || entry.Items != null);
+
+            if (entry.ActionConstraints != null)
+            {
+                return entry.ActionConstraints;
+            }
+
+            var items = new List<ActionConstraintItem>(entry.Items.Count);
+            for (var i = 0; i < entry.Items.Count; i++)
+            {
+                var item = entry.Items[i];
+                if (item.IsReusable)
+                {
+                    items.Add(item);
+                }
+                else
+                {
+                    items.Add(new ActionConstraintItem(item.Metadata));
+                }
+            }
+
+            ExecuteProviders(httpContext, action, items);
+
+            return ExtractActionConstraints(items);
+        }
+
+        private void ExecuteProviders(HttpContext httpContext, ActionDescriptor action, List<ActionConstraintItem> items)
+        {
+            var context = new ActionConstraintProviderContext(httpContext, action, items);
+
+            for (var i = 0; i < _actionConstraintProviders.Length; i++)
+            {
+                _actionConstraintProviders[i].OnProvidersExecuting(context);
+            }
+
+            for (var i = _actionConstraintProviders.Length - 1; i >= 0; i--)
+            {
+                _actionConstraintProviders[i].OnProvidersExecuted(context);
+            }
+        }
+
+        private IReadOnlyList<IActionConstraint> ExtractActionConstraints(List<ActionConstraintItem> items)
+        {
+            var count = 0;
+            for (var i = 0; i < items.Count; i++)
+            {
+                if (items[i].Constraint != null)
+                {
+                    count++;
+                }
+            }
+
+            if (count == 0)
+            {
+                return null;
+            }
+            
+            var actionConstraints = new IActionConstraint[count];
+            for (int i = 0, j = 0; i < items.Count; i++)
+            {
+                var actionConstraint = items[i].Constraint;
+                if (actionConstraint != null)
+                {
+                    actionConstraints[j++] = actionConstraint;
+                }
+            }
+
+            return actionConstraints;
+        }
+
+        private class InnerCache
+        {
+            public InnerCache(int version)
+            {
+                Version = version;
+            }
+
+            public ConcurrentDictionary<ActionDescriptor, CacheEntry> Entries { get; } =
+                new ConcurrentDictionary<ActionDescriptor, CacheEntry>();
+
+            public int Version { get; }
+        }
+
+        private struct CacheEntry
+        {
+            public CacheEntry(IReadOnlyList<IActionConstraint> actionConstraints)
+            {
+                ActionConstraints = actionConstraints;
+                Items = null;
+            }
+
+            public CacheEntry(List<ActionConstraintItem> items)
+            {
+                Items = items;
+                ActionConstraints = null;
+            }
+
+            public IReadOnlyList<IActionConstraint> ActionConstraints { get; }
+
+            public List<ActionConstraintItem> Items { get; }
+        }
+    }
+}

src/Microsoft.AspNet.Mvc.Core/Internal/ActionSelector.cs

@@ -4,13 +4,11 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using Microsoft.AspNet.Http;
 using Microsoft.AspNet.Mvc.Abstractions;
 using Microsoft.AspNet.Mvc.ActionConstraints;
 using Microsoft.AspNet.Mvc.Core;
 using Microsoft.AspNet.Mvc.Infrastructure;
 using Microsoft.AspNet.Mvc.Logging;
-using Microsoft.AspNet.Mvc.Routing;
 using Microsoft.AspNet.Routing;
 using Microsoft.Extensions.Logging;
 
@@ -22,7 +20,7 @@ namespace Microsoft.AspNet.Mvc.Internal
     public class ActionSelector : IActionSelector
     {
         private readonly IActionSelectorDecisionTreeProvider _decisionTreeProvider;
-        private readonly IActionConstraintProvider[] _actionConstraintProviders;
+        private readonly ActionConstraintCache _actionConstraintCache;
         private ILogger _logger;
 
         /// <summary>
@@ -33,12 +31,12 @@ namespace Microsoft.AspNet.Mvc.Internal
         /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
         public ActionSelector(
             IActionSelectorDecisionTreeProvider decisionTreeProvider,
-            IEnumerable<IActionConstraintProvider> actionConstraintProviders,
+            ActionConstraintCache actionConstraintCache,
             ILoggerFactory loggerFactory)
         {
             _decisionTreeProvider = decisionTreeProvider;
-            _actionConstraintProviders = actionConstraintProviders.OrderBy(item => item.Order).ToArray();
             _logger = loggerFactory.CreateLogger<ActionSelector>();
+            _actionConstraintCache = actionConstraintCache;
         }
 
         /// <inheritdoc />
@@ -58,7 +56,7 @@ namespace Microsoft.AspNet.Mvc.Internal
             for (var i = 0; i < matchingRouteConstraints.Count; i++)
             {
                 var action = matchingRouteConstraints[i];
-                var constraints = GetConstraints(context.HttpContext, action);
+                var constraints = _actionConstraintCache.GetActionConstraints(context.HttpContext, action);
                 candidates.Add(new ActionSelectorCandidate(action, constraints));
             }
 
@@ -217,56 +215,5 @@ namespace Microsoft.AspNet.Mvc.Internal
                 return EvaluateActionConstraints(context, actionsWithoutConstraint, order);
             }
         }
-
-        private IReadOnlyList<IActionConstraint> GetConstraints(HttpContext httpContext, ActionDescriptor action)
-        {
-            if (action.ActionConstraints == null || action.ActionConstraints.Count == 0)
-            {
-                return null;
-            }
-
-            var items = new List<ActionConstraintItem>(action.ActionConstraints.Count);
-            for (var i = 0; i < action.ActionConstraints.Count; i++)
-            {
-                items.Add(new ActionConstraintItem(action.ActionConstraints[i]));
-            }
-
-            var context = new ActionConstraintProviderContext(httpContext, action, items);
-            for (var i = 0; i < _actionConstraintProviders.Length; i++)
-            {
-                _actionConstraintProviders[i].OnProvidersExecuting(context);
-            }
-
-            for (var i = _actionConstraintProviders.Length - 1; i >= 0; i--)
-            {
-                _actionConstraintProviders[i].OnProvidersExecuted(context);
-            }
-
-            var count = 0;
-            for (var i = 0; i < context.Results.Count; i++)
-            {
-                if (context.Results[i].Constraint != null)
-                {
-                    count++;
-                }
-            }
-
-            if (count == 0)
-            {
-                return null;
-            }
-
-            var results = new IActionConstraint[count];
-            for (int i = 0, j = 0; i < context.Results.Count; i++)
-            {
-                var constraint = context.Results[i].Constraint;
-                if (constraint != null)
-                {
-                    results[j++] = constraint;
-                }
-            }
-
-            return results;
-        }
     }
 }

src/Microsoft.AspNet.Mvc.Core/Internal/DefaultActionConstraintProvider.cs

@@ -53,6 +53,7 @@ namespace Microsoft.AspNet.Mvc.Internal
             if (constraint != null)
             {
                 item.Constraint = constraint;
+                item.IsReusable = true;
                 return;
             }
 
@@ -60,6 +61,7 @@ namespace Microsoft.AspNet.Mvc.Internal
             if (factory != null)
             {
                 item.Constraint = factory.CreateInstance(services);
+                item.IsReusable = factory.IsReusable;
                 return;
             }
         }

src/Microsoft.AspNet.Mvc.Core/Internal/FilterCache.cs

@@ -5,7 +5,6 @@ using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
-using System.Threading;
 using Microsoft.AspNet.Mvc.Abstractions;
 using Microsoft.AspNet.Mvc.Filters;
 using Microsoft.AspNet.Mvc.Infrastructure;
@@ -100,8 +99,8 @@ namespace Microsoft.AspNet.Mvc.Internal
                 return entry.Filters;
             }
 
-            var items = new List<FilterItem>(entry.Items.Length);
+            var items = new List<FilterItem>(entry.Items.Count);
-            for (var i = 0; i < entry.Items.Length; i++)
+            for (var i = 0; i < entry.Items.Count; i++)
             {
                 var item = entry.Items[i];
                 if (item.IsReusable)
@@ -188,26 +187,13 @@ namespace Microsoft.AspNet.Mvc.Internal
 
             public CacheEntry(List<FilterItem> items)
             {
-                Items = new FilterItem[items.Count];
+                Items = items;
-                for (var i = 0; i < Items.Length; i++)
-                {
-                    var item = items[i];
-                    if (item.IsReusable)
-                    {
-                        Items[i] = item;
-                    }
-                    else
-                    {
-                        Items[i] = new FilterItem(item.Descriptor);
-                    }
-                }
-
                 Filters = null;
             }
 
             public IFilterMetadata[] Filters { get; }
 
-            public FilterItem[] Items { get; }
+            public List<FilterItem> Items { get; }
         }
     }
 }

test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs

@@ -530,7 +530,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
 
             var defaultActionSelector = new ActionSelector(
                 decisionTreeProvider,
-                actionConstraintProviders,
+                GetActionConstraintCache(actionConstraintProviders),
                 NullLoggerFactory.Instance);
 
             return defaultActionSelector.Select(context);
@@ -614,7 +614,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
 
             return new ActionSelector(
                 decisionTreeProvider,
-                actionConstraintProviders,
+                GetActionConstraintCache(actionConstraintProviders),
                 loggerFactory);
         }
 
@@ -680,6 +680,13 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
             return actionDescriptor;
         }
 
+        private static ActionConstraintCache GetActionConstraintCache(IActionConstraintProvider[] actionConstraintProviders = null)
+        {
+            var services = new ServiceCollection().BuildServiceProvider();
+            var descriptorProvider = new ActionDescriptorCollectionProvider(services);
+            return new ActionConstraintCache(descriptorProvider, actionConstraintProviders.AsEnumerable() ?? new List<IActionConstraintProvider>());
+        }
+
         private class BooleanConstraint : IActionConstraint
         {
             public bool Pass { get; set; }
@@ -696,6 +703,8 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
         {
             public IActionConstraint Constraint { get; set; }
 
+            public bool IsReusable => true;
+
             public IActionConstraint CreateInstance(IServiceProvider services)
             {
                 return Constraint;

test/Microsoft.AspNet.Mvc.Core.Test/Internal/ActionConstraintCacheTest.cs

@@ -0,0 +1,165 @@
+// 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 Microsoft.AspNet.Http.Internal;
+using Microsoft.AspNet.Mvc.ActionConstraints;
+using Microsoft.AspNet.Mvc.Controllers;
+using Microsoft.AspNet.Mvc.Infrastructure;
+using Microsoft.Extensions.DependencyInjection;
+using Xunit;
+
+namespace Microsoft.AspNet.Mvc.Internal
+{
+    public class ActionConstraintCacheTest
+    {
+        [Fact]
+        public void GetActionConstraints_CachesAllActionConstraints()
+        {
+            // Arrange
+            var services = CreateServices();
+            var cache = CreateCache(new DefaultActionConstraintProvider());
+
+            var action = new ControllerActionDescriptor()
+            {
+                ActionConstraints = new[]
+                {
+                    new TestActionConstraint(),
+                    new TestActionConstraint()
+                },
+            };
+            var context = new DefaultHttpContext();
+
+            // Act - 1
+            var actionConstraints1 = cache.GetActionConstraints(context, action);
+
+            // Assert - 1
+            Assert.Collection(
+                actionConstraints1,
+                a => Assert.Same(action.ActionConstraints[0], a), // Copied by provider
+                a => Assert.Same(action.ActionConstraints[1], a)); // Copied by provider
+
+            // Act - 2
+            var actionConstraints2 = cache.GetActionConstraints(context, action);
+
+            Assert.Same(actionConstraints1, actionConstraints2);
+
+            Assert.Collection(
+                actionConstraints2,
+                a => Assert.Same(actionConstraints1[0], a), // Cached
+                a => Assert.Same(actionConstraints1[1], a)); // Cached
+        }
+
+        [Fact]
+        public void GetActionConstraints_CachesActionConstraintFromFactory()
+        {
+            // Arrange
+            var services = CreateServices();
+            var cache = CreateCache(new DefaultActionConstraintProvider());
+
+            var action = new ControllerActionDescriptor()
+            {
+                ActionConstraints = new[]
+                {
+                    new TestActionConstraintFactory() { IsReusable = true },
+                    new TestActionConstraint() as IActionConstraintMetadata
+                },
+            };
+            var context = new DefaultHttpContext();
+
+            // Act - 1
+            var actionConstraints1 = cache.GetActionConstraints(context, action);
+
+            // Assert - 1
+            Assert.Collection(
+                actionConstraints1,
+                a => Assert.NotSame(action.ActionConstraints[0], a), // Created by factory
+                a => Assert.Same(action.ActionConstraints[1], a)); // Copied by provider
+
+            // Act - 2
+            var actionConstraints2 = cache.GetActionConstraints(context, action);
+
+            Assert.Same(actionConstraints1, actionConstraints2);
+
+            Assert.Collection(
+                actionConstraints2,
+                a => Assert.Same(actionConstraints1[0], a), // Cached
+                a => Assert.Same(actionConstraints1[1], a)); // Cached
+        }
+
+        [Fact]
+        public void GetActionConstraints_DoesNotCacheActionConstraintsWithIsReusableFalse()
+        {
+            // Arrange
+            var services = CreateServices();
+            var cache = CreateCache(new DefaultActionConstraintProvider());
+
+            var action = new ControllerActionDescriptor()
+            {
+                ActionConstraints = new[]
+                {
+                    new TestActionConstraintFactory() { IsReusable = false },
+                    new TestActionConstraint() as IActionConstraintMetadata
+                },
+            };
+            var context = new DefaultHttpContext();
+
+            // Act - 1
+            var actionConstraints1 = cache.GetActionConstraints(context, action);
+
+            // Assert - 1
+            Assert.Collection(
+                actionConstraints1,
+                a => Assert.NotSame(action.ActionConstraints[0], a), // Created by factory
+                a => Assert.Same(action.ActionConstraints[1], a)); // Copied by provider
+
+            // Act - 2
+            var actionConstraints2 = cache.GetActionConstraints(context, action);
+
+            Assert.NotSame(actionConstraints1, actionConstraints2);
+
+            Assert.Collection(
+                actionConstraints2,
+                a => Assert.NotSame(actionConstraints1[0], a), // Created by factory (again)
+                a => Assert.Same(actionConstraints1[1], a)); // Cached
+        }
+
+        private class TestActionConstraint : IActionConstraint
+        {
+            public int Order
+            {
+                get
+                {
+                    throw new NotImplementedException();
+                }
+            }
+
+            public bool Accept(ActionConstraintContext context)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
+        private class TestActionConstraintFactory : IActionConstraintFactory
+        {
+            public bool IsReusable { get; set; }
+
+            public IActionConstraint CreateInstance(IServiceProvider serviceProvider)
+            {
+                return new TestActionConstraint();
+            }
+        }
+
+        private static IServiceProvider CreateServices()
+        {
+            return new ServiceCollection().BuildServiceProvider();
+        }
+
+        private static ActionConstraintCache CreateCache(params IActionConstraintProvider[] providers)
+        {
+            var services = CreateServices();
+            var descriptorProvider = new ActionDescriptorCollectionProvider(services);
+            return new ActionConstraintCache(descriptorProvider, providers);
+        }
+    }
+}

test/WebSites/BasicWebSite/RequestScopedActionConstraint.cs

@@ -17,6 +17,8 @@ namespace BasicWebSite
         private readonly ConcurrentDictionary<Type, ObjectFactory> _constraintCache =
                new ConcurrentDictionary<Type, ObjectFactory>();
 
+        public bool IsReusable => false;
+
         public RequestScopedActionConstraintAttribute(string requestId)
         {
             _requestId = requestId;

test/WebSites/VersioningWebSite/VersionAttribute.cs

@@ -30,6 +30,8 @@ namespace VersioningWebSite
             set { _order = value; }
         }
 
+        public bool IsReusable => true;
+
         public IActionConstraint CreateInstance(IServiceProvider services)
         {
             return new VersionRangeValidator(_minVersion, _maxVersion) { Order = _order ?? 0 };

test/WebSites/VersioningWebSite/VersionRoute.cs

@@ -26,6 +26,8 @@ namespace VersioningWebSite
         // We filter out (5), (5], [5) manually after we do the parsing.
         private static readonly Regex _versionParser = new Regex(@"^(?<lb>[\(\[])?(?<range>\d+(-\d+)?)(?<hb>[\)\]])?$");
 
+        public bool IsReusable => true;
+
         public VersionRoute(string template)
             : base(template)
         {