diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerCache.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerCache.cs index 402149b61a..a232dd09ff 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerCache.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerCache.cs @@ -56,7 +56,20 @@ namespace Microsoft.AspNetCore.Mvc.Internal Entry cacheEntry; if (cache.Entries.TryGetValue(actionDescriptor, out cacheEntry)) { - filters = GetFilters(controllerContext, cacheEntry.FilterItems); + // 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 + }); + } + + filters = GetFilters(controllerContext, filterItems); return new ControllerActionInvokerState(filters, cacheEntry.ActionMethodExecutor); } @@ -71,7 +84,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal staticFilterItems.Add(new FilterItem(actionDescriptor.FilterDescriptors[i])); } - filters = GetFilters(controllerContext, staticFilterItems); + // 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.) @@ -90,12 +107,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal return new ControllerActionInvokerState(filters, cacheEntry.ActionMethodExecutor); } - private IFilterMetadata[] GetFilters(ActionContext actionContext, List staticFilterItems) + private IFilterMetadata[] GetFilters(ActionContext actionContext, List filterItems) { - // 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); - // Execute providers var context = new FilterProviderContext(actionContext, filterItems); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs index 329d56e101..791122870b 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs @@ -63,22 +63,20 @@ namespace Microsoft.AspNetCore.Mvc.Internal new[] { new DefaultFilterProvider() }); var filterDescriptors = controllerContext.ActionDescriptor.FilterDescriptors; - // Act - 1 + // Act & Assert var filters = controllerActionInvokerCache.GetState(controllerContext).Filters; - - // Assert - 1 Assert.Equal(2, filters.Length); - var request1Filter1 = Assert.IsType(filters[0]); // Created by factory + var cachedFactoryCreatedFilter = Assert.IsType(filters[0]); // Created by factory Assert.Same(staticFilter, filters[1]); // Cached and the same statically created filter instance - // Act - 2 - filters = controllerActionInvokerCache.GetState(controllerContext).Filters; + for (var i = 0; i < 5; i++) + { + filters = controllerActionInvokerCache.GetState(controllerContext).Filters; - // Assert - 2 - Assert.Collection( - filters, - f => Assert.Same(request1Filter1, f), // Cached - f => Assert.Same(staticFilter, f)); // Cached and the same statically created filter instance + var currentFactoryCreatedFilter = filters[0]; + Assert.Same(currentFactoryCreatedFilter, cachedFactoryCreatedFilter); // Cached + Assert.Same(staticFilter, filters[1]); // Cached + } } [Fact] @@ -96,22 +94,18 @@ namespace Microsoft.AspNetCore.Mvc.Internal new[] { new DefaultFilterProvider() }); var filterDescriptors = controllerContext.ActionDescriptor.FilterDescriptors; - // Act - 1 - var filters = controllerActionInvokerCache.GetState(controllerContext).Filters; + // Act & Assert + IFilterMetadata previousFactoryCreatedFilter = null; + for (var i = 0; i < 5; i++) + { + var filters = controllerActionInvokerCache.GetState(controllerContext).Filters; - // Assert - 1 - Assert.Equal(2, 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 currentFactoryCreatedFilter = filters[0]; + Assert.NotSame(currentFactoryCreatedFilter, previousFactoryCreatedFilter); // Never Cached + Assert.Same(staticFilter, filters[1]); // Cached - // Act - 2 - filters = controllerActionInvokerCache.GetState(controllerContext).Filters; - - // Assert - 2 - Assert.Collection( - filters, - f => Assert.NotSame(request1Filter1, f), // Created by factory again - f => Assert.Same(staticFilter, f)); // Cached and the same statically created filter instance + previousFactoryCreatedFilter = currentFactoryCreatedFilter; + } } [Fact]