From 6b0282fa84c546cf1d1b1e7bf2c707e076b4326f Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 29 Dec 2016 16:43:34 -0800 Subject: [PATCH] Commonize code from ControllerActionInvokerCache and PageFilterFactoryProvider --- .../Internal/ControllerActionInvokerCache.cs | 110 ++-------- .../Internal/FilterFactory.cs | 139 +++++++++++++ .../Internal/FilterFactoryResult.cs | 22 ++ .../Internal/PageActionInvoker.cs | 11 +- .../Internal/PageActionInvokerCacheEntry.cs | 6 +- .../Internal/PageActionInvokerProvider.cs | 41 ++-- .../Internal/PageFilterFactoryProvider.cs | 135 ------------ .../ControllerActionInvokerCacheTest.cs | 194 +----------------- .../Internal/FilterFactoryTest.cs} | 172 ++++++++-------- .../Internal/PageActionInvokerProviderTest.cs | 46 +++-- 10 files changed, 332 insertions(+), 544 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterFactory.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterFactoryResult.cs delete mode 100644 src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageFilterFactoryProvider.cs rename test/{Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageFilterFactoryProviderTest.cs => Microsoft.AspNetCore.Mvc.Core.Test/Internal/FilterFactoryTest.cs} (55%) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerCache.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerCache.cs index 70595cd7a5..b910d38e00 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerCache.cs +++ b/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 filterItems = new List(cacheEntry.FilterItems.Count); - 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 - }); - } + var filterFactoryResult = FilterFactory.GetAllFilters(_filterProviders, controllerContext); + filters = filterFactoryResult.Filters; - filters = GetFilters(controllerContext, filterItems); + var executor = ObjectMethodExecutor.Create( + actionDescriptor.MethodInfo, + actionDescriptor.ControllerTypeInfo); - return new ControllerActionInvokerState(filters, cacheEntry.ActionMethodExecutor); - } - - var executor = ObjectMethodExecutor.Create( - actionDescriptor.MethodInfo, - actionDescriptor.ControllerTypeInfo); - - var staticFilterItems = new List(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(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 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.Instance; + cacheEntry = new Entry(filterFactoryResult.CacheableFilters, executor); + cacheEntry = cache.Entries.GetOrAdd(actionDescriptor, cacheEntry); } 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; + // Filter instances from statically defined filter descriptors + from filter providers + filters = FilterFactory.CreateUncachedFilters(_filterProviders, controllerContext, cacheEntry.FilterItems); } + + return new ControllerActionInvokerState(filters, cacheEntry.ActionMethodExecutor); } private class InnerCache @@ -165,13 +85,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal private struct Entry { - public Entry(List items, ObjectMethodExecutor executor) + public Entry(FilterItem[] items, ObjectMethodExecutor executor) { FilterItems = items; ActionMethodExecutor = executor; } - public List FilterItems { get; } + public FilterItem[] FilterItems { get; } public ObjectMethodExecutor ActionMethodExecutor { get; } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterFactory.cs new file mode 100644 index 0000000000..7a7666dde7 --- /dev/null +++ b/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(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(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 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.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; + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterFactoryResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/FilterFactoryResult.cs new file mode 100644 index 0000000000..e532a08427 --- /dev/null +++ b/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; } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs index 75ba5121be..0c99fd1dca 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs +++ b/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; diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerCacheEntry.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerCacheEntry.cs index c6b356922c..b9a2c09fa0 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerCacheEntry.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerCacheEntry.cs @@ -12,12 +12,12 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal CompiledPageActionDescriptor actionDescriptor, Func pageFactory, Action releasePage, - Func 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 /// public Action ReleasePage { get; } - Func FilterProvider { get; } + public FilterItem[] CacheableFilters { get; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs index 004c536012..db4e5f89ce 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs +++ b/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); - context.Result = new PageActionInvoker(cacheEntry, context.ActionContext); + var cache = CurrentCache; + 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 - 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) + private PageActionInvokerCacheEntry CreateCacheEntry(ActionInvokerProviderContext context, FilterItem[] filters) { 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 diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageFilterFactoryProvider.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageFilterFactoryProvider.cs deleted file mode 100644 index 486911e9f1..0000000000 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageFilterFactoryProvider.cs +++ /dev/null @@ -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 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(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(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, 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.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; - } - }; - } - } -} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs index 791122870b..9c9f99fd78 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs +++ b/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 staticFilter2 = new TestFilter(); + var filter = new TestFilter(); var controllerContext = CreateControllerContext(new[] { - new FilterDescriptor(staticFilter1, FilterScope.Action), - new FilterDescriptor(staticFilter2, FilterScope.Action), + new FilterDescriptor(filter, FilterScope.Action) }); var controllerActionInvokerCache = CreateControllerActionInvokerCache( controllerContext, new[] { new DefaultFilterProvider() }); - // Act - 1 - var request1Filters = controllerActionInvokerCache.GetState(controllerContext).Filters; + // Act + var cacheEntry1 = controllerActionInvokerCache.GetState(controllerContext); + var cacheEntry2 = controllerActionInvokerCache.GetState(controllerContext); - // Assert - 1 - Assert.Collection( - 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(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; - } + // Assert + Assert.Equal(cacheEntry1.Filters, cacheEntry2.Filters); } [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(filters[0]); // Created by factory - Assert.Same(staticFilter, filters[1]); // Cached and the same statically created filter instance - var request1Filter3 = Assert.IsType(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(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(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 _providerExecuting; - private readonly Action _providerExecuted; - - public TestFilterProvider( - Action providerExecuting, - Action 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() diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageFilterFactoryProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/FilterFactoryTest.cs similarity index 55% rename from test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageFilterFactoryProviderTest.cs rename to test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/FilterFactoryTest.cs index 5fa888e1e6..36f5b8673e 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageFilterFactoryProviderTest.cs +++ b/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 FilterFactory_ReturnsNoFilters_IfNoFiltersAreSpecified() + public void GetAllFilters_ReturnsNoFilters_IfNoFiltersAreSpecified() { // Arrange var filterProviders = new IFilterProvider[0]; - var actionInvokerProviderContext = GetInvokerContext(); + var actionContext = CreateActionContext(new FilterDescriptor[0]); // Act - var filterFactory = PageFilterFactoryProvider.GetFilterFactory( - filterProviders, - actionInvokerProviderContext); - var filters1 = filterFactory(actionInvokerProviderContext.ActionContext); - var filters2 = filterFactory(actionInvokerProviderContext.ActionContext); + var filterResult = FilterFactory.GetAllFilters(filterProviders, actionContext); // Assert - Assert.Empty(filters1); - Assert.Empty(filters2); + Assert.Empty(filterResult.CacheableFilters); + 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); - var filters1 = filterFactory(actionInvokerProviderContext.ActionContext); - var filters2 = filterFactory(actionInvokerProviderContext.ActionContext); + actionContext); // Assert - Assert.Empty(filters1); - Assert.Empty(filters2); + Assert.Collection(filterResult.CacheableFilters, + 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[] - { - new FilterDescriptor(staticFilter1, FilterScope.Action), - new FilterDescriptor(staticFilter2, FilterScope.Action), - }); + var actionContext = CreateActionContext(new[] + { + new FilterDescriptor(staticFilter1, 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[] - { - new FilterDescriptor(new TestFilterFactory() { IsReusable = true }, FilterScope.Action), - new FilterDescriptor(staticFilter, FilterScope.Action), - }); + var actionContext = CreateActionContext(new[] + { + new FilterDescriptor(new TestFilterFactory() { IsReusable = true }, FilterScope.Action), + new FilterDescriptor(staticFilter, FilterScope.Action), + }); var filterProviders = new[] { new DefaultFilterProvider() }; + var filterDescriptors = actionContext.ActionDescriptor.FilterDescriptors; // Act & Assert - var filterFactory = PageFilterFactoryProvider.GetFilterFactory( - filterProviders, - actionInvokerProviderContext); + var filterResult = FilterFactory.GetAllFilters(filterProviders, actionContext); - var filters = filterFactory(actionInvokerProviderContext.ActionContext); + var filters = filterResult.Filters; Assert.Equal(2, filters.Length); var cachedFactoryCreatedFilter = Assert.IsType(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[] - { - new FilterDescriptor(new TestFilterFactory() { IsReusable = false }, FilterScope.Action), - new FilterDescriptor(staticFilter, FilterScope.Action), - }); + var actionContext = CreateActionContext(new[] + { + new FilterDescriptor(new TestFilterFactory() { IsReusable = false }, FilterScope.Action), + new FilterDescriptor(staticFilter, FilterScope.Action), + }); var filterProviders = new[] { new DefaultFilterProvider() }; + var filterDescriptors = actionContext.ActionDescriptor.FilterDescriptors; // Act & Assert - var filterFactory = PageFilterFactoryProvider.GetFilterFactory( - filterProviders, - actionInvokerProviderContext); + var filterResult = FilterFactory.GetAllFilters(filterProviders, actionContext); + var filters = filterResult.Filters; 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) => - { - var filter = new TestFilter(providerContext.ActionContext.HttpContext.Items["name"] as string); - providerContext.Results.Add( - new FilterItem(new FilterDescriptor(filter, FilterScope.Global), filter) - { - IsReusable = reusable - }); - }); + 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 actionInvokerProviderContext = GetInvokerContext(new[] - { - new FilterDescriptor(new TestFilterFactory() { IsReusable = false }, FilterScope.Action), - new FilterDescriptor(staticFilter, FilterScope.Action), - }); - var actionContext = actionInvokerProviderContext.ActionContext; + var actionContext = CreateActionContext(new[] + { + new FilterDescriptor(new TestFilterFactory() { IsReusable = false }, FilterScope.Action), + new FilterDescriptor(staticFilter, FilterScope.Action), + }); var filterProviders = new IFilterProvider[] { new DefaultFilterProvider(), customFilterProvider }; - + var filterDescriptors = actionContext.ActionDescriptor.FilterDescriptors; // Act - 1 actionContext.HttpContext.Items["name"] = "foo"; - var filterFactory = PageFilterFactoryProvider.GetFilterFactory( - filterProviders, - actionInvokerProviderContext); - var filters = filterFactory(actionContext); + var filterResult = FilterFactory.GetAllFilters(filterProviders, actionContext); + var filters = filterResult.Filters; // 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 providerExecuting, - Action providerExecuted = null, + Action 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(filters ?? Enumerable.Empty()) + FilterDescriptors = filterDescriptors, }; - var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), actionDescriptor); - return new ActionInvokerProviderContext(actionContext); + + return new ActionContext(new DefaultHttpContext(), new RouteData(), actionDescriptor); } } } diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs index 35895b9707..27cd2c1e36 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs +++ b/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(context.Result); + var entry = actionInvoker.CacheEntry; var compiledPageActionDescriptor = Assert.IsType(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 - var entry1 = invokerProvider.GetOrAddCacheEntry(context, descriptor); - var entry2 = invokerProvider.GetOrAddCacheEntry(context, descriptor); + // Act - 1 + invokerProvider.OnProvidersExecuting(context); - // Assert + // Assert - 1 + Assert.NotNull(context.Result); + var actionInvoker = Assert.IsType(context.Result); + var entry1 = actionInvoker.CacheEntry; + + // Act - 2 + invokerProvider.OnProvidersExecuting(context); + + // Assert - 2 + Assert.NotNull(context.Result); + actionInvoker = Assert.IsType(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 - var entry1 = invokerProvider.GetOrAddCacheEntry(context, descriptor); - var entry2 = invokerProvider.GetOrAddCacheEntry(context, descriptor); + // Act - 1 + invokerProvider.OnProvidersExecuting(context); + + // Assert - 1 + Assert.NotNull(context.Result); + var actionInvoker = Assert.IsType(context.Result); + var entry1 = actionInvoker.CacheEntry; + + // Act - 2 + invokerProvider.OnProvidersExecuting(context); // Assert + Assert.NotNull(context.Result); + actionInvoker = Assert.IsType(context.Result); + var entry2 = actionInvoker.CacheEntry; Assert.NotSame(entry1, entry2); } }