Commonize code from ControllerActionInvokerCache and PageFilterFactoryProvider

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

@@ -44,110 +44,30 @@ namespace Microsoft.AspNetCore.Mvc.Internal
 
         public ControllerActionInvokerState GetState(ControllerContext controllerContext)
         {
-            // Filter instances from statically defined filter descriptors + from filter providers
-            IFilterMetadata[] filters;
-
             var cache = CurrentCache;
             var actionDescriptor = controllerContext.ActionDescriptor;
 
+            IFilterMetadata[] filters;
             Entry cacheEntry;
-            if (cache.Entries.TryGetValue(actionDescriptor, out cacheEntry))
+            if (!cache.Entries.TryGetValue(actionDescriptor, out cacheEntry))
             {
-                // Deep copy the cached filter items as filter providers could modify them
+                var filterFactoryResult = FilterFactory.GetAllFilters(_filterProviders, controllerContext);
-                var filterItems = new List<FilterItem>(cacheEntry.FilterItems.Count);
+                filters = filterFactoryResult.Filters;
-                for (var i = 0; i < cacheEntry.FilterItems.Count; i++)
-                {
-                    var filterItem = cacheEntry.FilterItems[i];
-                    filterItems.Add(
-                        new FilterItem(filterItem.Descriptor)
-                        {
-                            Filter = filterItem.Filter,
-                            IsReusable = filterItem.IsReusable
-                        });
-                }
 
-                filters = GetFilters(controllerContext, filterItems);
+                var executor = ObjectMethodExecutor.Create(
+                    actionDescriptor.MethodInfo,
+                    actionDescriptor.ControllerTypeInfo);
 
-                return new ControllerActionInvokerState(filters, cacheEntry.ActionMethodExecutor);
+                cacheEntry = new Entry(filterFactoryResult.CacheableFilters, executor);
-            }
+                cacheEntry = cache.Entries.GetOrAdd(actionDescriptor, cacheEntry);
-
-            var executor = ObjectMethodExecutor.Create(
-                actionDescriptor.MethodInfo,
-                actionDescriptor.ControllerTypeInfo);
-
-            var staticFilterItems = new List<FilterItem>(actionDescriptor.FilterDescriptors.Count);
-            for (var i = 0; i < actionDescriptor.FilterDescriptors.Count; i++)
-            {
-                staticFilterItems.Add(new FilterItem(actionDescriptor.FilterDescriptors[i]));
-            }
-
-            // Create a separate collection as we want to hold onto the statically defined filter items
-            // in order to cache them
-            var allFilterItems = new List<FilterItem>(staticFilterItems);
-
-            filters = GetFilters(controllerContext, allFilterItems);
-
-            // Cache the filter items based on the following criteria
-            // 1. Are created statically (ex: via filter attributes, added to global filter list etc.)
-            // 2. Are re-usable
-            for (var i = 0; i < staticFilterItems.Count; i++)
-            {
-                var item = staticFilterItems[i];
-                if (!item.IsReusable)
-                {
-                    item.Filter = null;
-                }
-            }
-            cacheEntry = new Entry(staticFilterItems, executor);
-            cache.Entries.TryAdd(actionDescriptor, cacheEntry);
-
-            return new ControllerActionInvokerState(filters, cacheEntry.ActionMethodExecutor);
-        }
-
-        private IFilterMetadata[] GetFilters(ActionContext actionContext, List<FilterItem> filterItems)
-        {
-            // Execute providers
-            var context = new FilterProviderContext(actionContext, filterItems);
-
-            for (var i = 0; i < _filterProviders.Length; i++)
-            {
-                _filterProviders[i].OnProvidersExecuting(context);
-            }
-
-            for (var i = _filterProviders.Length - 1; i >= 0; i--)
-            {
-                _filterProviders[i].OnProvidersExecuted(context);
-            }
-
-            // Extract filter instances from statically defined filters and filter providers
-            var count = 0;
-            for (var i = 0; i < filterItems.Count; i++)
-            {
-                if (filterItems[i].Filter != null)
-                {
-                    count++;
-                }
-            }
-
-            if (count == 0)
-            {
-                return EmptyArray<IFilterMetadata>.Instance;
             }
             else
             {
-                var filters = new IFilterMetadata[count];
+                // Filter instances from statically defined filter descriptors + from filter providers
-                var filterIndex = 0;
+                filters = FilterFactory.CreateUncachedFilters(_filterProviders, controllerContext, cacheEntry.FilterItems);
-                for (int i = 0; i < filterItems.Count; i++)
-                {
-                    var filter = filterItems[i].Filter;
-                    if (filter != null)
-                    {
-                        filters[filterIndex++] = filter;
-                    }
-                }
-
-                return filters;
             }
+
+            return new ControllerActionInvokerState(filters, cacheEntry.ActionMethodExecutor);
         }
 
         private class InnerCache
@@ -165,13 +85,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal
 
         private struct Entry
         {
-            public Entry(List<FilterItem> items, ObjectMethodExecutor executor)
+            public Entry(FilterItem[] items, ObjectMethodExecutor executor)
             {
                 FilterItems = items;
                 ActionMethodExecutor = executor;
             }
 
-            public List<FilterItem> FilterItems { get; }
+            public FilterItem[] FilterItems { get; }
 
             public ObjectMethodExecutor ActionMethodExecutor { get; }
         }

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

@@ -0,0 +1,139 @@
+// 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 Microsoft.AspNetCore.Mvc.Filters;
+
+namespace Microsoft.AspNetCore.Mvc.Internal
+{
+    public static class FilterFactory
+    {
+        public static FilterFactoryResult GetAllFilters(
+            IFilterProvider[] filterProviders,
+            ActionContext actionContext)
+        {
+            if (filterProviders == null)
+            {
+                throw new ArgumentNullException(nameof(filterProviders));
+            }
+
+            if (actionContext == null)
+            {
+                throw new ArgumentNullException(nameof(actionContext));
+            }
+
+            var actionDescriptor = actionContext.ActionDescriptor;
+
+            var staticFilterItems = new FilterItem[actionDescriptor.FilterDescriptors.Count];
+            for (var i = 0; i < actionDescriptor.FilterDescriptors.Count; i++)
+            {
+                staticFilterItems[i] = new FilterItem(actionDescriptor.FilterDescriptors[i]);
+            }
+
+            var allFilterItems = new List<FilterItem>(staticFilterItems);
+
+            // Execute the filter factory to determine which static filters can be cached.
+            var filters = CreateUncachedFiltersCore(filterProviders, actionContext, allFilterItems);
+
+            // Cache the filter items based on the following criteria
+            // 1. Are created statically (ex: via filter attributes, added to global filter list etc.)
+            // 2. Are re-usable
+            for (var i = 0; i < staticFilterItems.Length; i++)
+            {
+                var item = staticFilterItems[i];
+                if (!item.IsReusable)
+                {
+                    item.Filter = null;
+                }
+            }
+
+            return new FilterFactoryResult(staticFilterItems, filters);
+        }
+
+        public static IFilterMetadata[] CreateUncachedFilters(
+            IFilterProvider[] filterProviders,
+            ActionContext actionContext,
+            FilterItem[] cachedFilterItems)
+        {
+            if (filterProviders == null)
+            {
+                throw new ArgumentNullException(nameof(filterProviders));
+            }
+
+            if (actionContext == null)
+            {
+                throw new ArgumentNullException(nameof(actionContext));
+            }
+
+            if (cachedFilterItems == null)
+            {
+                throw new ArgumentNullException(nameof(cachedFilterItems));
+            }
+
+            // Deep copy the cached filter items as filter providers could modify them
+            var filterItems = new List<FilterItem>(cachedFilterItems.Length);
+            for (var i = 0; i < cachedFilterItems.Length; i++)
+            {
+                var filterItem = cachedFilterItems[i];
+                filterItems.Add(
+                    new FilterItem(filterItem.Descriptor)
+                    {
+                        Filter = filterItem.Filter,
+                        IsReusable = filterItem.IsReusable
+                    });
+            }
+
+            return CreateUncachedFiltersCore(filterProviders, actionContext, filterItems);
+        }
+
+        private static IFilterMetadata[] CreateUncachedFiltersCore(
+            IFilterProvider[] filterProviders,
+            ActionContext actionContext,
+            List<FilterItem> filterItems)
+        {
+            // Execute providers
+            var context = new FilterProviderContext(actionContext, filterItems);
+
+            for (var i = 0; i < filterProviders.Length; i++)
+            {
+                filterProviders[i].OnProvidersExecuting(context);
+            }
+
+            for (var i = filterProviders.Length - 1; i >= 0; i--)
+            {
+                filterProviders[i].OnProvidersExecuted(context);
+            }
+
+            // Extract filter instances from statically defined filters and filter providers
+            var count = 0;
+            for (var i = 0; i < filterItems.Count; i++)
+            {
+                if (filterItems[i].Filter != null)
+                {
+                    count++;
+                }
+            }
+
+            if (count == 0)
+            {
+                return EmptyArray<IFilterMetadata>.Instance;
+            }
+            else
+            {
+                var filters = new IFilterMetadata[count];
+                var filterIndex = 0;
+                for (int i = 0; i < filterItems.Count; i++)
+                {
+                    var filter = filterItems[i].Filter;
+                    if (filter != null)
+                    {
+                        filters[filterIndex++] = filter;
+                    }
+                }
+
+                return filters;
+            }
+        }
+    }
+}

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

@@ -0,0 +1,22 @@
+// 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 Microsoft.AspNetCore.Mvc.Filters;
+
+namespace Microsoft.AspNetCore.Mvc.Internal
+{
+    public struct FilterFactoryResult
+    {
+        public FilterFactoryResult(
+            FilterItem[] cacheableFilters,
+            IFilterMetadata[] filters)
+        {
+            CacheableFilters = cacheableFilters;
+            Filters = filters;
+        }
+
+        public FilterItem[] CacheableFilters { get; }
+
+        public IFilterMetadata[] Filters { get; }
+    }
+}

src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs

@@ -3,23 +3,28 @@
 
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Mvc.Abstractions;
+using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Mvc.Internal;
 
 namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
 {
     public class PageActionInvoker : IActionInvoker
     {
-        private readonly PageActionInvokerCacheEntry _cacheEntry;
         private readonly ActionContext _actionContext;
+        private readonly IFilterMetadata[] _filters;
 
         public PageActionInvoker(
             PageActionInvokerCacheEntry cacheEntry,
-            ActionContext actionContext)
+            ActionContext actionContext,
+            IFilterMetadata[] filters)
         {
-            _cacheEntry = cacheEntry;
+            CacheEntry = cacheEntry;
             _actionContext = actionContext;
+            _filters = filters;
         }
 
+        public PageActionInvokerCacheEntry CacheEntry { get; }
+
         public Task InvokeAsync()
         {
             return TaskCache.CompletedTask;

src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerCacheEntry.cs

@@ -12,12 +12,12 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
             CompiledPageActionDescriptor actionDescriptor,
             Func<PageContext, object> pageFactory,
             Action<PageContext, object> releasePage,
-            Func<PageContext, IFilterMetadata[]> filterProvider)
+            FilterItem[] cacheableFilters)
         {
             ActionDescriptor = actionDescriptor;
             PageFactory = pageFactory;
             ReleasePage = releasePage;
-            FilterProvider = filterProvider;
+            CacheableFilters = cacheableFilters;
         }
 
         public CompiledPageActionDescriptor ActionDescriptor { get; }
@@ -29,6 +29,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
         /// </summary>
         public Action<PageContext, object> ReleasePage { get; }
 
-        Func<PageContext, IFilterMetadata[]> FilterProvider { get; }
+        public FilterItem[] CacheableFilters { get; }
     }
 }

src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs

@@ -9,6 +9,7 @@ using System.Reflection;
 using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Mvc.Infrastructure;
+using Microsoft.AspNetCore.Mvc.Internal;
 using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure;
 
 namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
@@ -49,8 +50,23 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
                 return;
             }
 
-            var cacheEntry = GetOrAddCacheEntry(context, actionDescriptor);
+            var cache = CurrentCache;
-            context.Result = new PageActionInvoker(cacheEntry, context.ActionContext);
+            PageActionInvokerCacheEntry cacheEntry;
+
+            IFilterMetadata[] filters;
+            if (!cache.Entries.TryGetValue(actionDescriptor, out cacheEntry))
+            {
+                var filterFactoryResult = FilterFactory.GetAllFilters(_filterProviders, context.ActionContext);
+                filters = filterFactoryResult.Filters;
+                cacheEntry = CreateCacheEntry(context, filterFactoryResult.CacheableFilters);
+                cacheEntry = cache.Entries.GetOrAdd(actionDescriptor, cacheEntry);
+            }
+            else
+            {
+                filters = FilterFactory.CreateUncachedFilters(_filterProviders, context.ActionContext, cacheEntry.CacheableFilters);
+            }
+
+            context.Result = new PageActionInvoker(cacheEntry, context.ActionContext, filters);
         }
 
         public void OnProvidersExecuted(ActionInvokerProviderContext context)
@@ -75,24 +91,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
             }
         }
 
-        // Internal for unit testing
+        private PageActionInvokerCacheEntry CreateCacheEntry(ActionInvokerProviderContext context, FilterItem[] filters)
-        internal PageActionInvokerCacheEntry GetOrAddCacheEntry(
-            ActionInvokerProviderContext context,
-            PageActionDescriptor actionDescriptor)
-        {
-            var cache = CurrentCache;
-
-            PageActionInvokerCacheEntry cacheEntry;
-            if (!cache.Entries.TryGetValue(actionDescriptor, out cacheEntry))
-            {
-                cacheEntry = CreateCacheEntry(context);
-                cacheEntry = cache.Entries.GetOrAdd(actionDescriptor, cacheEntry);
-            }
-
-            return cacheEntry;
-        }
-
-        private PageActionInvokerCacheEntry CreateCacheEntry(ActionInvokerProviderContext context)
         {
             var actionDescriptor = (PageActionDescriptor)context.ActionContext.ActionDescriptor;
             var compiledType = _loader.Load(actionDescriptor).GetTypeInfo();
@@ -108,7 +107,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
                 compiledActionDescriptor,
                 _pageFactoryProvider.CreatePageFactory(compiledActionDescriptor),
                 _pageFactoryProvider.CreatePageDisposer(compiledActionDescriptor),
-                PageFilterFactoryProvider.GetFilterFactory(_filterProviders, context));
+                filters);
         }
 
         private class InnerCache

src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageFilterFactoryProvider.cs

@@ -1,135 +0,0 @@
-// 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.Threading;
-using Microsoft.AspNetCore.Mvc.Abstractions;
-using Microsoft.AspNetCore.Mvc.Filters;
-using Microsoft.AspNetCore.Mvc.Internal;
-
-namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
-{
-    public static class PageFilterFactoryProvider
-    {
-        public static Func<ActionContext, IFilterMetadata[]> GetFilterFactory(
-            IFilterProvider[] filterProviders,
-            ActionInvokerProviderContext actionInvokerProviderContext)
-        {
-            if (filterProviders == null)
-            {
-                throw new ArgumentNullException(nameof(filterProviders));
-            }
-
-            if (actionInvokerProviderContext == null)
-            {
-                throw new ArgumentNullException(nameof(actionInvokerProviderContext));
-            }
-
-            var actionDescriptor = actionInvokerProviderContext.ActionContext.ActionDescriptor;
-
-            // staticFilterItems is captured as part of the closure.We evaluate it once to determine
-            // which of the staticFilters are reusable.
-            var staticFilterItems = new FilterItem[actionDescriptor.FilterDescriptors.Count];
-            for (var i = 0; i < actionDescriptor.FilterDescriptors.Count; i++)
-            {
-                staticFilterItems[i] = new FilterItem(actionDescriptor.FilterDescriptors[i]);
-            }
-
-            var internalFilterFactory = GetFilterFactory(filterProviders);
-            var allFilterItems = new List<FilterItem>(staticFilterItems);
-
-            // Execute the filter factory to determine which static filters can be cached.
-            var filters = internalFilterFactory(allFilterItems, actionInvokerProviderContext.ActionContext);
-
-            // Cache the filter items based on the following criteria
-            // 1. Are created statically (ex: via filter attributes, added to global filter list etc.)
-            // 2. Are re-usable
-            for (var i = 0; i < staticFilterItems.Length; i++)
-            {
-                var item = staticFilterItems[i];
-                if (!item.IsReusable)
-                {
-                    item.Filter = null;
-                }
-            }
-
-            return (actionContext) =>
-            {
-                // Reuse the filters cached outside the closure for the very first run. This avoids re-running
-                // filters twice the first time we cache for a page.
-                var cachedFilters = Interlocked.Exchange(ref filters, null);
-                if (cachedFilters != null)
-                {
-                    return cachedFilters;
-                }
-
-                // Create a separate collection as we want to hold onto the statically defined filter items
-                // in order to cache them
-                var filterItems = new List<FilterItem>(staticFilterItems.Length);
-                for (var i = 0; i < staticFilterItems.Length; i++)
-                {
-                    // Deep copy the cached filter items as filter providers could modify them
-                    var filterItem = staticFilterItems[i];
-                    filterItems.Add(new FilterItem(filterItem.Descriptor)
-                    {
-                        Filter = filterItem.Filter,
-                        IsReusable = filterItem.IsReusable
-                    });
-                }
-
-                return internalFilterFactory(filterItems, actionContext);
-            };
-        }
-
-        private static Func<IList<FilterItem>, ActionContext, IFilterMetadata[]> GetFilterFactory(
-            IFilterProvider[] filterProviders)
-        {
-            return (filterItems, actionContext) =>
-            {
-                // Execute providers
-                var filterContext = new FilterProviderContext(actionContext, filterItems);
-
-                for (var i = 0; i < filterProviders.Length; i++)
-                {
-                    filterProviders[i].OnProvidersExecuting(filterContext);
-                }
-
-                for (var i = filterProviders.Length - 1; i >= 0; i--)
-                {
-                    filterProviders[i].OnProvidersExecuted(filterContext);
-                }
-
-                // Extract filter instances from statically defined filters and filter providers
-                var count = 0;
-                for (var i = 0; i < filterItems.Count; i++)
-                {
-                    if (filterItems[i].Filter != null)
-                    {
-                        count++;
-                    }
-                }
-
-                if (count == 0)
-                {
-                    return EmptyArray<IFilterMetadata>.Instance;
-                }
-                else
-                {
-                    var filters = new IFilterMetadata[count];
-                    var filterIndex = 0;
-                    for (int i = 0; i < filterItems.Count; i++)
-                    {
-                        var filter = filterItems[i].Filter;
-                        if (filter != null)
-                        {
-                            filters[filterIndex++] = filter;
-                        }
-                    }
-
-                    return filters;
-                }
-            };
-        }
-    }
-}

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

@@ -1,7 +1,6 @@
 // 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.Reflection;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc.Controllers;
@@ -15,97 +14,24 @@ namespace Microsoft.AspNetCore.Mvc.Internal
     public class ControllerActionInvokerCacheTest
     {
         [Fact]
-        public void GetFilters_CachesAllFilters()
+        public void GetControllerActionMethodExecutor_CachesFilters()
         {
             // Arrange
-            var staticFilter1 = new TestFilter();
+            var filter = new TestFilter();
-            var staticFilter2 = new TestFilter();
             var controllerContext = CreateControllerContext(new[]
                 {
-                    new FilterDescriptor(staticFilter1, FilterScope.Action),
+                    new FilterDescriptor(filter, FilterScope.Action)
-                    new FilterDescriptor(staticFilter2, FilterScope.Action),
                 });
             var controllerActionInvokerCache = CreateControllerActionInvokerCache(
                 controllerContext,
                 new[] { new DefaultFilterProvider() });
 
-            // Act - 1
+            // Act
-            var request1Filters = controllerActionInvokerCache.GetState(controllerContext).Filters;
+            var cacheEntry1 = controllerActionInvokerCache.GetState(controllerContext);
+            var cacheEntry2 = controllerActionInvokerCache.GetState(controllerContext);
 
-            // Assert - 1
+            // Assert
-            Assert.Collection(
+            Assert.Equal(cacheEntry1.Filters, cacheEntry2.Filters);
-                request1Filters,
-                f => Assert.Same(staticFilter1, f),
-                f => Assert.Same(staticFilter2, f));
-
-            // Act - 2
-            var request2Filters = controllerActionInvokerCache.GetState(controllerContext).Filters;
-
-            // Assert - 2
-            Assert.Collection(
-                request2Filters,
-                f => Assert.Same(staticFilter1, f),
-                f => Assert.Same(staticFilter2, f));
-        }
-
-        [Fact]
-        public void GetFilters_CachesFilterFromFactory()
-        {
-            // Arrange
-            var staticFilter = new TestFilter();
-            var controllerContext = CreateControllerContext(new[]
-                {
-                    new FilterDescriptor(new TestFilterFactory() { IsReusable = true }, FilterScope.Action),
-                    new FilterDescriptor(staticFilter, FilterScope.Action),
-                });
-            var controllerActionInvokerCache = CreateControllerActionInvokerCache(
-                controllerContext,
-                new[] { new DefaultFilterProvider() });
-            var filterDescriptors = controllerContext.ActionDescriptor.FilterDescriptors;
-
-            // Act & Assert
-            var filters = controllerActionInvokerCache.GetState(controllerContext).Filters;
-            Assert.Equal(2, filters.Length);
-            var cachedFactoryCreatedFilter = Assert.IsType<TestFilter>(filters[0]); // Created by factory
-            Assert.Same(staticFilter, filters[1]); // Cached and the same statically created filter instance
-
-            for (var i = 0; i < 5; i++)
-            {
-                filters = controllerActionInvokerCache.GetState(controllerContext).Filters;
-
-                var currentFactoryCreatedFilter = filters[0];
-                Assert.Same(currentFactoryCreatedFilter, cachedFactoryCreatedFilter); // Cached
-                Assert.Same(staticFilter, filters[1]); // Cached
-            }
-        }
-
-        [Fact]
-        public void GetFilters_DoesNotCacheFiltersWithIsReusableFalse()
-        {
-            // Arrange
-            var staticFilter = new TestFilter();
-            var controllerContext = CreateControllerContext(new[]
-                {
-                    new FilterDescriptor(new TestFilterFactory() { IsReusable = false }, FilterScope.Action),
-                    new FilterDescriptor(staticFilter, FilterScope.Action),
-                });
-            var controllerActionInvokerCache = CreateControllerActionInvokerCache(
-                controllerContext,
-                new[] { new DefaultFilterProvider() });
-            var filterDescriptors = controllerContext.ActionDescriptor.FilterDescriptors;
-
-            // Act & Assert
-            IFilterMetadata previousFactoryCreatedFilter = null;
-            for (var i = 0; i < 5; i++)
-            {
-                var filters = controllerActionInvokerCache.GetState(controllerContext).Filters;
-
-                var currentFactoryCreatedFilter = filters[0];
-                Assert.NotSame(currentFactoryCreatedFilter, previousFactoryCreatedFilter); // Never Cached
-                Assert.Same(staticFilter, filters[1]); // Cached
-
-                previousFactoryCreatedFilter = currentFactoryCreatedFilter;
-            }
         }
 
         [Fact]
@@ -129,59 +55,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             Assert.Same(cacheEntry1.ActionMethodExecutor, cacheEntry2.ActionMethodExecutor);
         }
 
-        [Theory]
-        [InlineData(true)]
-        [InlineData(false)]
-        public void GetFilters_FiltersAddedByFilterProviders_AreNeverCached(bool reusable)
-        {
-            // Arrange
-            var customFilterProvider = new TestFilterProvider(
-                    providerExecuting: (providerContext) =>
-                    {
-                        var filter = new TestFilter(providerContext.ActionContext.HttpContext.Items["name"] as string);
-                        providerContext.Results.Add(
-                            new FilterItem(new FilterDescriptor(filter, FilterScope.Global), filter)
-                            {
-                                IsReusable = reusable
-                            });
-                    },
-                    providerExecuted: null);
-            var staticFilter = new TestFilter();
-            var controllerContext = CreateControllerContext(new[]
-                {
-                    new FilterDescriptor(new TestFilterFactory() { IsReusable = false }, FilterScope.Action),
-                    new FilterDescriptor(staticFilter, FilterScope.Action),
-                });
-            var controllerActionInvokerCache = CreateControllerActionInvokerCache(
-                controllerContext,
-                new IFilterProvider[] { new DefaultFilterProvider(), customFilterProvider });
-            var filterDescriptors = controllerContext.ActionDescriptor.FilterDescriptors;
-
-            // Act - 1
-            controllerContext.HttpContext.Items["name"] = "foo";
-            var filters = controllerActionInvokerCache.GetState(controllerContext).Filters;
-
-            // Assert - 1
-            Assert.Equal(3, filters.Length);
-            var request1Filter1 = Assert.IsType<TestFilter>(filters[0]); // Created by factory
-            Assert.Same(staticFilter, filters[1]); // Cached and the same statically created filter instance
-            var request1Filter3 = Assert.IsType<TestFilter>(filters[2]); // Created by custom filter provider
-            Assert.Equal("foo", request1Filter3.Data);
-
-            // Act - 2
-            controllerContext.HttpContext.Items["name"] = "bar";
-            filters = controllerActionInvokerCache.GetState(controllerContext).Filters;
-
-            // Assert -2
-            Assert.Equal(3, filters.Length);
-            var request2Filter1 = Assert.IsType<TestFilter>(filters[0]);
-            Assert.NotSame(request1Filter1, request2Filter1); // Created by factory
-            Assert.Same(staticFilter, filters[1]);   // Cached and the same statically created filter instance
-            var request2Filter3 = Assert.IsType<TestFilter>(filters[2]);
-            Assert.NotSame(request1Filter3, request2Filter3); // Created by custom filter provider again
-            Assert.Equal("bar", request2Filter3.Data);
-        }
-
         private class TestFilter : IFilterMetadata
         {
             public TestFilter()
@@ -196,57 +69,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             public string Data { get; }
         }
 
-        private class TestFilterFactory : IFilterFactory
-        {
-            private TestFilter testFilter;
-
-            public bool IsReusable { get; set; }
-
-            public IFilterMetadata CreateInstance(IServiceProvider serviceProvider)
-            {
-                if (IsReusable)
-                {
-                    if (testFilter == null)
-                    {
-                        testFilter = new TestFilter();
-                    }
-                    return testFilter;
-                }
-                else
-                {
-                    return new TestFilter();
-                }
-            }
-        }
-
-        private class TestFilterProvider : IFilterProvider
-        {
-            private readonly Action<FilterProviderContext> _providerExecuting;
-            private readonly Action<FilterProviderContext> _providerExecuted;
-
-            public TestFilterProvider(
-                Action<FilterProviderContext> providerExecuting,
-                Action<FilterProviderContext> providerExecuted,
-                int order = 0)
-            {
-                _providerExecuting = providerExecuting;
-                _providerExecuted = providerExecuted;
-                Order = order;
-            }
-
-            public int Order { get; }
-
-            public void OnProvidersExecuting(FilterProviderContext context)
-            {
-                _providerExecuting?.Invoke(context);
-            }
-
-            public void OnProvidersExecuted(FilterProviderContext context)
-            {
-                _providerExecuted?.Invoke(context);
-            }
-        }
-
         private class TestController
         {
             public void Index()

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

@@ -2,78 +2,72 @@
 // 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;
 using Microsoft.AspNetCore.Mvc.Filters;
-using Microsoft.AspNetCore.Mvc.Internal;
 using Microsoft.AspNetCore.Routing;
 using Xunit;
 
-namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
+namespace Microsoft.AspNetCore.Mvc.Internal
 {
-    public class PageFilterFactoryProviderTest
+    public class FilterFactoryTest
     {
-        [Fact]
+        public void GetAllFilters_ReturnsNoFilters_IfNoFiltersAreSpecified()
-        public void FilterFactory_ReturnsNoFilters_IfNoFiltersAreSpecified()
         {
             // Arrange
             var filterProviders = new IFilterProvider[0];
-            var actionInvokerProviderContext = GetInvokerContext();
+            var actionContext = CreateActionContext(new FilterDescriptor[0]);
 
             // Act
-            var filterFactory = PageFilterFactoryProvider.GetFilterFactory(
+            var filterResult = FilterFactory.GetAllFilters(filterProviders, actionContext);
-                filterProviders,
-                actionInvokerProviderContext);
-            var filters1 = filterFactory(actionInvokerProviderContext.ActionContext);
-            var filters2 = filterFactory(actionInvokerProviderContext.ActionContext);
 
             // Assert
-            Assert.Empty(filters1);
+            Assert.Empty(filterResult.CacheableFilters);
-            Assert.Empty(filters2);
+            Assert.Empty(filterResult.Filters);
         }
 
         [Fact]
-        public void FilterFactory_ReturnsNoFilters_IfAllFiltersAreRemoved()
+        public void GetAllFilters_ReturnsNoFilters_IfAllFiltersAreRemoved()
         {
             // Arrange
             var filterProvider = new TestFilterProvider(
-                context => context.Results.Clear());
+                context => context.Results.Clear(),
+                content => { });
             var filter = new FilterDescriptor(new TypeFilterAttribute(typeof(object)), FilterScope.Global);
-            var actionInvokerProviderContext = GetInvokerContext(filter);
+            var actionContext = CreateActionContext(new[] { filter });
 
             // Act
-            var filterFactory = PageFilterFactoryProvider.GetFilterFactory(
+            var filterResult = FilterFactory.GetAllFilters(
                 new[] { filterProvider },
-                actionInvokerProviderContext);
+                actionContext);
-            var filters1 = filterFactory(actionInvokerProviderContext.ActionContext);
-            var filters2 = filterFactory(actionInvokerProviderContext.ActionContext);
 
             // Assert
-            Assert.Empty(filters1);
+            Assert.Collection(filterResult.CacheableFilters,
-            Assert.Empty(filters2);
+                f =>
+                {
+                    Assert.Null(f.Filter);
+                    Assert.False(f.IsReusable);
+                });
+            Assert.Empty(filterResult.Filters);
         }
 
         [Fact]
-        public void FilterFactory_CachesAllFilters()
+        public void GetAllFilters_CachesAllFilters()
         {
             // Arrange
             var staticFilter1 = new TestFilter();
             var staticFilter2 = new TestFilter();
-            var actionInvokerProviderContext = GetInvokerContext(new[]
+            var actionContext = CreateActionContext(new[]
-            {
+                {
-                new FilterDescriptor(staticFilter1, FilterScope.Action),
+                    new FilterDescriptor(staticFilter1, FilterScope.Action),
-                new FilterDescriptor(staticFilter2, FilterScope.Action),
+                    new FilterDescriptor(staticFilter2, FilterScope.Action),
-            });
+                });
             var filterProviders = new[] { new DefaultFilterProvider() };
 
-            // Act - 1
-            var filterFactory = PageFilterFactoryProvider.GetFilterFactory(
-                filterProviders,
-                actionInvokerProviderContext);
 
-            var request1Filters = filterFactory(actionInvokerProviderContext.ActionContext);
+            // Act - 1
+            var filterResult = FilterFactory.GetAllFilters(filterProviders, actionContext);
+            var request1Filters = filterResult.Filters;
 
             // Assert - 1
             Assert.Collection(
@@ -82,7 +76,10 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
                 f => Assert.Same(staticFilter2, f));
 
             // Act - 2
-            var request2Filters = filterFactory(actionInvokerProviderContext.ActionContext);
+            var request2Filters = FilterFactory.CreateUncachedFilters(
+                filterProviders,
+                actionContext,
+                filterResult.CacheableFilters);
 
             // Assert - 2
             Assert.Collection(
@@ -92,30 +89,29 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
         }
 
         [Fact]
-        public void FilterFactory_CachesFilterFromFactory()
+        public void GetAllFilters_CachesFilterFromFactory()
         {
             // Arrange
             var staticFilter = new TestFilter();
-            var actionInvokerProviderContext = GetInvokerContext(new[]
+            var actionContext = CreateActionContext(new[]
-            {
+                {
-                new FilterDescriptor(new TestFilterFactory() { IsReusable = true }, FilterScope.Action),
+                    new FilterDescriptor(new TestFilterFactory() { IsReusable = true }, FilterScope.Action),
-                new FilterDescriptor(staticFilter, FilterScope.Action),
+                    new FilterDescriptor(staticFilter, FilterScope.Action),
-            });
+                });
             var filterProviders = new[] { new DefaultFilterProvider() };
+            var filterDescriptors = actionContext.ActionDescriptor.FilterDescriptors;
 
             // Act & Assert
-            var filterFactory = PageFilterFactoryProvider.GetFilterFactory(
+            var filterResult = FilterFactory.GetAllFilters(filterProviders, actionContext);
-                filterProviders,
-                actionInvokerProviderContext);
 
-            var filters = filterFactory(actionInvokerProviderContext.ActionContext);
+            var filters = filterResult.Filters;
             Assert.Equal(2, filters.Length);
             var cachedFactoryCreatedFilter = Assert.IsType<TestFilter>(filters[0]); // Created by factory
             Assert.Same(staticFilter, filters[1]); // Cached and the same statically created filter instance
 
             for (var i = 0; i < 5; i++)
             {
-                filters = filterFactory(actionInvokerProviderContext.ActionContext);
+                filters = FilterFactory.CreateUncachedFilters(filterProviders, actionContext, filterResult.CacheableFilters);
 
                 var currentFactoryCreatedFilter = filters[0];
                 Assert.Same(currentFactoryCreatedFilter, cachedFactoryCreatedFilter); // Cached
@@ -124,25 +120,25 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
         }
 
         [Fact]
-        public void FilterFactory_DoesNotCacheFiltersWithIsReusableFalse()
+        public void GetAllFilters_DoesNotCacheFiltersWithIsReusableFalse()
         {
             // Arrange
             var staticFilter = new TestFilter();
-            var actionInvokerProviderContext = GetInvokerContext(new[]
+            var actionContext = CreateActionContext(new[]
-            {
+                {
-                new FilterDescriptor(new TestFilterFactory() { IsReusable = false }, FilterScope.Action),
+                    new FilterDescriptor(new TestFilterFactory() { IsReusable = false }, FilterScope.Action),
-                new FilterDescriptor(staticFilter, FilterScope.Action),
+                    new FilterDescriptor(staticFilter, FilterScope.Action),
-            });
+                });
             var filterProviders = new[] { new DefaultFilterProvider() };
+            var filterDescriptors = actionContext.ActionDescriptor.FilterDescriptors;
 
             // Act & Assert
-            var filterFactory = PageFilterFactoryProvider.GetFilterFactory(
+            var filterResult = FilterFactory.GetAllFilters(filterProviders, actionContext);
-                filterProviders,
+            var filters = filterResult.Filters;
-                actionInvokerProviderContext);
             IFilterMetadata previousFactoryCreatedFilter = null;
             for (var i = 0; i < 5; i++)
             {
-                var filters = filterFactory(actionInvokerProviderContext.ActionContext);
+                filters = FilterFactory.CreateUncachedFilters(filterProviders, actionContext, filterResult.CacheableFilters);
 
                 var currentFactoryCreatedFilter = filters[0];
                 Assert.NotSame(currentFactoryCreatedFilter, previousFactoryCreatedFilter); // Never Cached
@@ -155,35 +151,33 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
         [Theory]
         [InlineData(true)]
         [InlineData(false)]
-        public void FilterFactory_FiltersAddedByFilterProviders_AreNeverCached(bool reusable)
+        public void GetAllFilters_FiltersAddedByFilterProviders_AreNeverCached(bool reusable)
         {
             // Arrange
             var customFilterProvider = new TestFilterProvider(
-                providerExecuting: (providerContext) =>
+                    providerExecuting: (providerContext) =>
-                {
+                    {
-                    var filter = new TestFilter(providerContext.ActionContext.HttpContext.Items["name"] as string);
+                        var filter = new TestFilter(providerContext.ActionContext.HttpContext.Items["name"] as string);
-                    providerContext.Results.Add(
+                        providerContext.Results.Add(
-                        new FilterItem(new FilterDescriptor(filter, FilterScope.Global), filter)
+                            new FilterItem(new FilterDescriptor(filter, FilterScope.Global), filter)
-                        {
+                            {
-                            IsReusable = reusable
+                                IsReusable = reusable
-                        });
+                            });
-                });
+                    },
+                    providerExecuted: null);
             var staticFilter = new TestFilter();
-            var actionInvokerProviderContext = GetInvokerContext(new[]
+            var actionContext = CreateActionContext(new[]
-            {
+                {
-                new FilterDescriptor(new TestFilterFactory() { IsReusable = false }, FilterScope.Action),
+                    new FilterDescriptor(new TestFilterFactory() { IsReusable = false }, FilterScope.Action),
-                new FilterDescriptor(staticFilter, FilterScope.Action),
+                    new FilterDescriptor(staticFilter, FilterScope.Action),
-            });
+                });
-            var actionContext = actionInvokerProviderContext.ActionContext;
             var filterProviders = new IFilterProvider[] { new DefaultFilterProvider(), customFilterProvider };
-
+            var filterDescriptors = actionContext.ActionDescriptor.FilterDescriptors;
 
             // Act - 1
             actionContext.HttpContext.Items["name"] = "foo";
-            var filterFactory = PageFilterFactoryProvider.GetFilterFactory(
+            var filterResult = FilterFactory.GetAllFilters(filterProviders, actionContext);
-                filterProviders,
+            var filters = filterResult.Filters;
-                actionInvokerProviderContext);
-            var filters = filterFactory(actionContext);
 
             // Assert - 1
             Assert.Equal(3, filters.Length);
@@ -194,7 +188,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
 
             // Act - 2
             actionContext.HttpContext.Items["name"] = "bar";
-            filters = filterFactory(actionContext);
+            filters = FilterFactory.CreateUncachedFilters(filterProviders, actionContext, filterResult.CacheableFilters);
 
             // Assert -2
             Assert.Equal(3, filters.Length);
@@ -222,7 +216,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
 
         private class TestFilterFactory : IFilterFactory
         {
-            private TestFilter _testFilter;
+            private TestFilter testFilter;
 
             public bool IsReusable { get; set; }
 
@@ -230,11 +224,11 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
             {
                 if (IsReusable)
                 {
-                    if (_testFilter == null)
+                    if (testFilter == null)
                     {
-                        _testFilter = new TestFilter();
+                        testFilter = new TestFilter();
                     }
-                    return _testFilter;
+                    return testFilter;
                 }
                 else
                 {
@@ -250,7 +244,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
 
             public TestFilterProvider(
                 Action<FilterProviderContext> providerExecuting,
-                Action<FilterProviderContext> providerExecuted = null,
+                Action<FilterProviderContext> providerExecuted,
                 int order = 0)
             {
                 _providerExecuting = providerExecuting;
@@ -271,14 +265,14 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
             }
         }
 
-        private static ActionInvokerProviderContext GetInvokerContext(params FilterDescriptor[] filters)
+        private static ActionContext CreateActionContext(FilterDescriptor[] filterDescriptors)
         {
-            var actionDescriptor = new PageActionDescriptor
+            var actionDescriptor = new ActionDescriptor
             {
-                FilterDescriptors = new List<FilterDescriptor>(filters ?? Enumerable.Empty<FilterDescriptor>())
+                FilterDescriptors = filterDescriptors,
             };
-            var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), actionDescriptor);
+
-            return new ActionInvokerProviderContext(actionContext);
+            return new ActionContext(new DefaultHttpContext(), new RouteData(), actionDescriptor);
         }
     }
 }

test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs

@@ -14,7 +14,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
     public class PageInvokerProviderTest
     {
         [Fact]
-        public void GetOrAddCacheEntry_PopulatesCacheEntry()
+        public void OnProvidersExecuting_PopulatesCacheEntry()
         {
             // Arrange
             var descriptor = new PageActionDescriptor
@@ -47,10 +47,12 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
             });
 
             // Act
-            var entry = invokerProvider.GetOrAddCacheEntry(context, descriptor);
+            invokerProvider.OnProvidersExecuting(context);
 
             // Assert
-            Assert.NotNull(entry);
+            Assert.NotNull(context.Result);
+            var actionInvoker = Assert.IsType<PageActionInvoker>(context.Result);
+            var entry = actionInvoker.CacheEntry;
             var compiledPageActionDescriptor = Assert.IsType<CompiledPageActionDescriptor>(entry.ActionDescriptor);
             Assert.Equal(descriptor.RelativePath, compiledPageActionDescriptor.RelativePath);
             Assert.Same(factory, entry.PageFactory);
@@ -58,7 +60,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
         }
 
         [Fact]
-        public void GetOrAddCacheEntry_CachesEntries()
+        public void OnProvidersExecuting_CachesEntries()
         {
             // Arrange
             var descriptor = new PageActionDescriptor
@@ -83,16 +85,26 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
                 ActionDescriptor = descriptor,
             });
 
-            // Act
+            // Act - 1
-            var entry1 = invokerProvider.GetOrAddCacheEntry(context, descriptor);
+            invokerProvider.OnProvidersExecuting(context);
-            var entry2 = invokerProvider.GetOrAddCacheEntry(context, descriptor);
 
-            // Assert
+            // Assert - 1
+            Assert.NotNull(context.Result);
+            var actionInvoker = Assert.IsType<PageActionInvoker>(context.Result);
+            var entry1 = actionInvoker.CacheEntry;
+
+            // Act - 2
+            invokerProvider.OnProvidersExecuting(context);
+
+            // Assert - 2
+            Assert.NotNull(context.Result);
+            actionInvoker = Assert.IsType<PageActionInvoker>(context.Result);
+            var entry2 = actionInvoker.CacheEntry;
             Assert.Same(entry1, entry2);
         }
 
         [Fact]
-        public void GetOrAddCacheEntry_UpdatesEntriesWhenActionDescriptorProviderCollectionIsUpdated()
+        public void OnProvidersExecuting_UpdatesEntriesWhenActionDescriptorProviderCollectionIsUpdated()
         {
             // Arrange
             var descriptor = new PageActionDescriptor
@@ -120,11 +132,21 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
                 ActionDescriptor = descriptor,
             });
 
-            // Act
+            // Act - 1
-            var entry1 = invokerProvider.GetOrAddCacheEntry(context, descriptor);
+            invokerProvider.OnProvidersExecuting(context);
-            var entry2 = invokerProvider.GetOrAddCacheEntry(context, descriptor);
+
+            // Assert - 1
+            Assert.NotNull(context.Result);
+            var actionInvoker = Assert.IsType<PageActionInvoker>(context.Result);
+            var entry1 = actionInvoker.CacheEntry;
+
+            // Act - 2
+            invokerProvider.OnProvidersExecuting(context);
 
             // Assert
+            Assert.NotNull(context.Result);
+            actionInvoker = Assert.IsType<PageActionInvoker>(context.Result);
+            var entry2 = actionInvoker.CacheEntry;
             Assert.NotSame(entry1, entry2);
         }
     }