diff --git a/samples/MvcSubAreaSample.Web/MvcSubAreaSample.Web.xproj b/samples/MvcSubAreaSample.Web/MvcSubAreaSample.Web.xproj index cf431ebb9f..8861859670 100644 --- a/samples/MvcSubAreaSample.Web/MvcSubAreaSample.Web.xproj +++ b/samples/MvcSubAreaSample.Web/MvcSubAreaSample.Web.xproj @@ -1,16 +1,30 @@  +<<<<<<< HEAD 14.0.24720 +======= + + + 14.0.24711 +>>>>>>> CR feedback $(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion) +<<<<<<< HEAD 45F6B3B6-D114-4D77-84D6-561B3957F341 +======= + e26979a8-56fb-41f7-8da5-a49a570acd39 +>>>>>>> CR feedback MvcSubAreaSample.Web ..\..\artifacts\obj\$(MSBuildProjectName) ..\..\artifacts\bin\$(MSBuildProjectName)\ +<<<<<<< HEAD +======= + +>>>>>>> CR feedback 2.0 diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/Filters/FilterItem.cs b/src/Microsoft.AspNet.Mvc.Abstractions/Filters/FilterItem.cs index d42965805d..7294d108c9 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/Filters/FilterItem.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/Filters/FilterItem.cs @@ -6,10 +6,19 @@ using System.Diagnostics; namespace Microsoft.AspNet.Mvc.Filters { - // Used to flow filters back from the FilterProviderContext + /// + /// Used to associate executable filters with instances + /// as part of . An should + /// inspect and set and + /// as appropriate. + /// [DebuggerDisplay("FilterItem: {Filter}")] public class FilterItem { + /// + /// Creates a new . + /// + /// The . public FilterItem(FilterDescriptor descriptor) { if (descriptor == null) @@ -20,6 +29,11 @@ namespace Microsoft.AspNet.Mvc.Filters Descriptor = descriptor; } + /// + /// Creates a new . + /// + /// The . + /// public FilterItem(FilterDescriptor descriptor, IFilterMetadata filter) : this(descriptor) { @@ -31,8 +45,19 @@ namespace Microsoft.AspNet.Mvc.Filters Filter = filter; } - public FilterDescriptor Descriptor { get; set; } + /// + /// Gets the containing the filter metadata. + /// + public FilterDescriptor Descriptor { get; } + /// + /// Gets or sets the executable associated with . + /// public IFilterMetadata Filter { get; set; } + + /// + /// Gets or sets a value indicating whether or not can be reused across requests. + /// + public bool IsReusable { get; set; } } } diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/Filters/IFilterFactory.cs b/src/Microsoft.AspNet.Mvc.Abstractions/Filters/IFilterFactory.cs index 1e65313afe..3865152a1e 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/Filters/IFilterFactory.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/Filters/IFilterFactory.cs @@ -5,8 +5,22 @@ using System; namespace Microsoft.AspNet.Mvc.Filters { + /// + /// An interface for for filter metadata which can create an instance of an executable filter. + /// public interface IFilterFactory : IFilterMetadata { + /// + /// Gets a value that indicates if the result of + /// can be reused across requests. + /// + bool IsReusable { get; } + + /// + /// Creates an instance of the executable filter. + /// + /// The request . + /// An instance of the executable filter. IFilterMetadata CreateInstance(IServiceProvider serviceProvider); } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Controllers/ControllerActionInvoker.cs b/src/Microsoft.AspNet.Mvc.Core/Controllers/ControllerActionInvoker.cs index 3b30cde785..cab7bfed93 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controllers/ControllerActionInvoker.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controllers/ControllerActionInvoker.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Mvc.Filters; using Microsoft.AspNet.Mvc.Formatters; +using Microsoft.AspNet.Mvc.Internal; using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.AspNet.Mvc.ModelBinding.Validation; @@ -25,11 +26,11 @@ namespace Microsoft.AspNet.Mvc.Controllers public ControllerActionInvoker( ActionContext actionContext, - IReadOnlyList filterProviders, + FilterCache filterCache, IControllerFactory controllerFactory, ControllerActionDescriptor descriptor, IReadOnlyList inputFormatters, - IControllerActionArgumentBinder controllerActionArgumentBinder, + IControllerActionArgumentBinder argumentBinder, IReadOnlyList modelBinders, IReadOnlyList modelValidatorProviders, IReadOnlyList valueProviderFactories, @@ -38,7 +39,7 @@ namespace Microsoft.AspNet.Mvc.Controllers int maxModelValidationErrors) : base( actionContext, - filterProviders, + filterCache, inputFormatters, modelBinders, modelValidatorProviders, @@ -47,16 +48,6 @@ namespace Microsoft.AspNet.Mvc.Controllers diagnosticSource, maxModelValidationErrors) { - if (actionContext == null) - { - throw new ArgumentNullException(nameof(actionContext)); - } - - if (filterProviders == null) - { - throw new ArgumentNullException(nameof(filterProviders)); - } - if (controllerFactory == null) { throw new ArgumentNullException(nameof(controllerFactory)); @@ -67,51 +58,22 @@ namespace Microsoft.AspNet.Mvc.Controllers throw new ArgumentNullException(nameof(descriptor)); } - if (inputFormatters == null) + if (argumentBinder == null) { - throw new ArgumentNullException(nameof(inputFormatters)); + throw new ArgumentNullException(nameof(argumentBinder)); } - if (controllerActionArgumentBinder == null) - { - throw new ArgumentNullException(nameof(controllerActionArgumentBinder)); - } - - if (modelBinders == null) - { - throw new ArgumentNullException(nameof(modelBinders)); - } - - if (modelValidatorProviders == null) - { - throw new ArgumentNullException(nameof(modelValidatorProviders)); - } - - if (valueProviderFactories == null) - { - throw new ArgumentNullException(nameof(valueProviderFactories)); - } - - if (logger == null) - { - throw new ArgumentNullException(nameof(logger)); - } - - if (diagnosticSource == null) - { - throw new ArgumentNullException(nameof(diagnosticSource)); - } - - _descriptor = descriptor; _controllerFactory = controllerFactory; - _argumentBinder = controllerActionArgumentBinder; + _descriptor = descriptor; + _argumentBinder = argumentBinder; if (descriptor.MethodInfo == null) { throw new ArgumentException( - Resources.FormatPropertyOfTypeCannotBeNull("MethodInfo", - typeof(ControllerActionDescriptor)), - "descriptor"); + Resources.FormatPropertyOfTypeCannotBeNull( + nameof(descriptor.MethodInfo), + typeof(ControllerActionDescriptor)), + nameof(descriptor)); } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Controllers/ControllerActionInvokerProvider.cs b/src/Microsoft.AspNet.Mvc.Core/Controllers/ControllerActionInvokerProvider.cs index a88c9b0292..a24bcb3e9e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controllers/ControllerActionInvokerProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controllers/ControllerActionInvokerProvider.cs @@ -6,8 +6,8 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; using Microsoft.AspNet.Mvc.Abstractions; -using Microsoft.AspNet.Mvc.Filters; using Microsoft.AspNet.Mvc.Formatters; +using Microsoft.AspNet.Mvc.Internal; using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.AspNet.Mvc.ModelBinding.Validation; using Microsoft.Extensions.Logging; @@ -19,7 +19,7 @@ namespace Microsoft.AspNet.Mvc.Controllers { private readonly IControllerActionArgumentBinder _argumentBinder; private readonly IControllerFactory _controllerFactory; - private readonly IFilterProvider[] _filterProviders; + private readonly FilterCache _filterCache; private readonly IReadOnlyList _inputFormatters; private readonly IReadOnlyList _modelBinders; private readonly IReadOnlyList _modelValidatorProviders; @@ -30,14 +30,14 @@ namespace Microsoft.AspNet.Mvc.Controllers public ControllerActionInvokerProvider( IControllerFactory controllerFactory, - IEnumerable filterProviders, + FilterCache filterCache, IControllerActionArgumentBinder argumentBinder, IOptions optionsAccessor, ILoggerFactory loggerFactory, DiagnosticSource diagnosticSource) { _controllerFactory = controllerFactory; - _filterProviders = filterProviders.OrderBy(item => item.Order).ToArray(); + _filterCache = filterCache; _argumentBinder = argumentBinder; _inputFormatters = optionsAccessor.Value.InputFormatters.ToArray(); _modelBinders = optionsAccessor.Value.ModelBinders.ToArray(); @@ -67,7 +67,7 @@ namespace Microsoft.AspNet.Mvc.Controllers { context.Result = new ControllerActionInvoker( context.ActionContext, - _filterProviders, + _filterCache, _controllerFactory, actionDescriptor, _inputFormatters, diff --git a/src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs b/src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs index a26d914b9a..cc62b9e1ec 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs @@ -21,9 +21,7 @@ namespace Microsoft.AspNet.Mvc.Controllers { public abstract class FilterActionInvoker : IActionInvoker { - private static readonly IFilterMetadata[] EmptyFilterArray = new IFilterMetadata[0]; - - private readonly IReadOnlyList _filterProviders; + private readonly FilterCache _filterCache; private readonly IReadOnlyList _inputFormatters; private readonly IReadOnlyList _modelBinders; private readonly IReadOnlyList _modelValidatorProviders; @@ -49,7 +47,7 @@ namespace Microsoft.AspNet.Mvc.Controllers public FilterActionInvoker( ActionContext actionContext, - IReadOnlyList filterProviders, + FilterCache filterCache, IReadOnlyList inputFormatters, IReadOnlyList modelBinders, IReadOnlyList modelValidatorProviders, @@ -63,9 +61,9 @@ namespace Microsoft.AspNet.Mvc.Controllers throw new ArgumentNullException(nameof(actionContext)); } - if (filterProviders == null) + if (filterCache == null) { - throw new ArgumentNullException(nameof(filterProviders)); + throw new ArgumentNullException(nameof(filterCache)); } if (inputFormatters == null) @@ -100,7 +98,7 @@ namespace Microsoft.AspNet.Mvc.Controllers Context = new ControllerContext(actionContext); - _filterProviders = filterProviders; + _filterCache = filterCache; _inputFormatters = inputFormatters; _modelBinders = modelBinders; _modelValidatorProviders = modelValidatorProviders; @@ -184,51 +182,7 @@ namespace Microsoft.AspNet.Mvc.Controllers private IFilterMetadata[] GetFilters() { - var filterDescriptors = Context.ActionDescriptor.FilterDescriptors; - var items = new List(filterDescriptors.Count); - for (var i = 0; i < filterDescriptors.Count; i++) - { - items.Add(new FilterItem(filterDescriptors[i])); - } - - var context = new FilterProviderContext(Context, items); - for (var i = 0; i < _filterProviders.Count; i++) - { - _filterProviders[i].OnProvidersExecuting(context); - } - - for (var i = _filterProviders.Count - 1; i >= 0; i--) - { - _filterProviders[i].OnProvidersExecuted(context); - } - - var count = 0; - for (var i = 0; i < items.Count; i++) - { - if (items[i].Filter != null) - { - count++; - } - } - - if (count == 0) - { - return EmptyFilterArray; - } - else - { - var filters = new IFilterMetadata[count]; - for (int i = 0, j = 0; i < items.Count; i++) - { - var filter = items[i].Filter; - if (filter != null) - { - filters[j++] = filter; - } - } - - return filters; - } + return _filterCache.GetFilters(Context); } private Task InvokeAllAuthorizationFiltersAsync() diff --git a/src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index 110f87b70c..c118c64970 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -119,6 +119,7 @@ namespace Microsoft.Extensions.DependencyInjection // These are stateless services.TryAddSingleton(); + services.TryAddSingleton(); services.TryAddEnumerable( ServiceDescriptor.Singleton()); diff --git a/src/Microsoft.AspNet.Mvc.Core/Filters/DefaultFilterProvider.cs b/src/Microsoft.AspNet.Mvc.Core/Filters/DefaultFilterProvider.cs index 09aada7670..75f523045e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Filters/DefaultFilterProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Filters/DefaultFilterProvider.cs @@ -50,11 +50,13 @@ namespace Microsoft.AspNet.Mvc.Filters if (filterFactory == null) { filterItem.Filter = filter; + filterItem.IsReusable = true; } else { var services = context.ActionContext.HttpContext.RequestServices; filterItem.Filter = filterFactory.CreateInstance(services); + filterItem.IsReusable = filterFactory.IsReusable; if (filterItem.Filter == null) { diff --git a/src/Microsoft.AspNet.Mvc.Core/FormatFilterAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/FormatFilterAttribute.cs index c10b6d6a69..1b98dd224b 100644 --- a/src/Microsoft.AspNet.Mvc.Core/FormatFilterAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/FormatFilterAttribute.cs @@ -15,6 +15,9 @@ namespace Microsoft.AspNet.Mvc [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] public class FormatFilterAttribute : Attribute, IFilterFactory { + /// + public bool IsReusable => true; + /// /// Creates an instance of . /// diff --git a/src/Microsoft.AspNet.Mvc.Core/Internal/FilterCache.cs b/src/Microsoft.AspNet.Mvc.Core/Internal/FilterCache.cs new file mode 100644 index 0000000000..e3076c99af --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Internal/FilterCache.cs @@ -0,0 +1,213 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Threading; +using Microsoft.AspNet.Mvc.Abstractions; +using Microsoft.AspNet.Mvc.Filters; +using Microsoft.AspNet.Mvc.Infrastructure; + +namespace Microsoft.AspNet.Mvc.Internal +{ + public class FilterCache + { + private readonly IFilterMetadata[] EmptyFilterArray = new IFilterMetadata[0]; + + private readonly IActionDescriptorCollectionProvider _collectionProvider; + private readonly IFilterProvider[] _filterProviders; + + private volatile InnerCache _currentCache; + + public FilterCache( + IActionDescriptorCollectionProvider collectionProvider, + IEnumerable filterProviders) + { + _collectionProvider = collectionProvider; + _filterProviders = filterProviders.OrderBy(item => item.Order).ToArray(); + } + + private InnerCache CurrentCache + { + get + { + var current = _currentCache; + var actionDescriptors = _collectionProvider.ActionDescriptors; + + if (current == null || current.Version != actionDescriptors.Version) + { + current = new InnerCache(actionDescriptors.Version); + _currentCache = current; + } + + return current; + } + } + + public IFilterMetadata[] GetFilters(ActionContext actionContext) + { + var cache = CurrentCache; + var actionDescriptor = actionContext.ActionDescriptor; + + CacheEntry entry; + if (cache.Entries.TryGetValue(actionDescriptor, out entry)) + { + return GetFiltersFromEntry(entry, actionContext); + } + + var items = new List(actionDescriptor.FilterDescriptors.Count); + for (var i = 0; i < actionDescriptor.FilterDescriptors.Count; i++) + { + items.Add(new FilterItem(actionDescriptor.FilterDescriptors[i])); + } + + ExecuteProviders(actionContext, items); + + var filters = ExtractFilters(items); + + var allFiltersCached = true; + for (var i = 0; i < items.Count; i++) + { + var item = items[i]; + if (!item.IsReusable) + { + item.Filter = null; + allFiltersCached = false; + } + } + + if (allFiltersCached) + { + entry = new CacheEntry(filters); + } + else + { + entry = new CacheEntry(items); + } + + cache.Entries.TryAdd(actionDescriptor, entry); + return filters; + } + + private IFilterMetadata[] GetFiltersFromEntry(CacheEntry entry, ActionContext actionContext) + { + Debug.Assert(entry.Filters != null || entry.Items != null); + + if (entry.Filters != null) + { + return entry.Filters; + } + + var items = new List(entry.Items.Length); + for (var i = 0; i < entry.Items.Length; i++) + { + var item = entry.Items[i]; + if (item.IsReusable) + { + items.Add(item); + } + else + { + items.Add(new FilterItem(item.Descriptor)); + } + } + + ExecuteProviders(actionContext, items); + + return ExtractFilters(items); + } + + private void ExecuteProviders(ActionContext actionContext, List items) + { + var context = new FilterProviderContext(actionContext, items); + + 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); + } + } + + private IFilterMetadata[] ExtractFilters(List items) + { + var count = 0; + for (var i = 0; i < items.Count; i++) + { + if (items[i].Filter != null) + { + count++; + } + } + + if (count == 0) + { + return EmptyFilterArray; + } + else + { + var filters = new IFilterMetadata[count]; + for (int i = 0, j = 0; i < items.Count; i++) + { + var filter = items[i].Filter; + if (filter != null) + { + filters[j++] = filter; + } + } + + return filters; + } + } + + private class InnerCache + { + public InnerCache(int version) + { + Version = version; + } + + public ConcurrentDictionary Entries { get; } = + new ConcurrentDictionary(); + + public int Version { get; } + } + + private struct CacheEntry + { + public CacheEntry(IFilterMetadata[] filters) + { + Filters = filters; + Items = null; + } + + public CacheEntry(List items) + { + Items = new FilterItem[items.Count]; + for (var i = 0; i < Items.Length; i++) + { + var item = items[i]; + if (item.IsReusable) + { + Items[i] = item; + } + else + { + Items[i] = new FilterItem(item.Descriptor); + } + } + + Filters = null; + } + + public IFilterMetadata[] Filters { get; } + + public FilterItem[] Items { get; } + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/ResponseCacheAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/ResponseCacheAttribute.cs index 9071b64d83..559275c9bc 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ResponseCacheAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ResponseCacheAttribute.cs @@ -81,11 +81,12 @@ namespace Microsoft.AspNet.Mvc /// public string CacheProfileName { get; set; } - /// - /// The order of the filter. - /// + /// public int Order { get; set; } + /// + public bool IsReusable => true; + public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) { if (serviceProvider == null) diff --git a/src/Microsoft.AspNet.Mvc.Core/ServiceFilterAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/ServiceFilterAttribute.cs index 030a5054be..8531c9ebfd 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ServiceFilterAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ServiceFilterAttribute.cs @@ -23,9 +23,13 @@ namespace Microsoft.AspNet.Mvc ServiceType = type; } + /// + public int Order { get; set; } + public Type ServiceType { get; private set; } - public int Order { get; set; } + /// + public bool IsReusable { get; set; } public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) { diff --git a/src/Microsoft.AspNet.Mvc.Core/TypeFilterAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/TypeFilterAttribute.cs index 286907b402..5d553e8339 100644 --- a/src/Microsoft.AspNet.Mvc.Core/TypeFilterAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/TypeFilterAttribute.cs @@ -29,8 +29,12 @@ namespace Microsoft.AspNet.Mvc public Type ImplementationType { get; private set; } + /// public int Order { get; set; } + /// + public bool IsReusable { get; set; } + public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) { if (serviceProvider == null) diff --git a/src/Microsoft.AspNet.Mvc.Cors/CorsAuthorizationFilterFactory.cs b/src/Microsoft.AspNet.Mvc.Cors/CorsAuthorizationFilterFactory.cs index f0f8c35f07..ba4f291687 100644 --- a/src/Microsoft.AspNet.Mvc.Cors/CorsAuthorizationFilterFactory.cs +++ b/src/Microsoft.AspNet.Mvc.Cors/CorsAuthorizationFilterFactory.cs @@ -34,6 +34,9 @@ namespace Microsoft.AspNet.Mvc.Cors } } + /// + public bool IsReusable => true; + public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) { if (serviceProvider == null) diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/AutoValidateAntiforgeryTokenAttribute.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/AutoValidateAntiforgeryTokenAttribute.cs index 1370d8de60..a6827c0f44 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/AutoValidateAntiforgeryTokenAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/AutoValidateAntiforgeryTokenAttribute.cs @@ -24,6 +24,9 @@ namespace Microsoft.AspNet.Mvc /// public int Order { get; set; } + /// + public bool IsReusable => true; + /// public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) { diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ValidateAntiForgeryTokenAttribute.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ValidateAntiForgeryTokenAttribute.cs index 6ac43a6aef..9479620d5f 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ValidateAntiForgeryTokenAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ValidateAntiForgeryTokenAttribute.cs @@ -19,8 +19,12 @@ namespace Microsoft.AspNet.Mvc [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] public class ValidateAntiForgeryTokenAttribute : Attribute, IFilterFactory, IOrderedFilter { + /// public int Order { get; set; } + /// + public bool IsReusable => true; + public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) { return serviceProvider.GetRequiredService(); diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SaveTempDataAttribute.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SaveTempDataAttribute.cs index abf85a715e..6786b9202b 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SaveTempDataAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/SaveTempDataAttribute.cs @@ -11,8 +11,14 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures /// Adds a filter which will save the for a request. /// [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] - public class SaveTempDataAttribute : Attribute, IFilterFactory + public class SaveTempDataAttribute : Attribute, IFilterFactory, IOrderedFilter { + /// + public int Order { get; set; } + + /// + public bool IsReusable => true; + /// public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Controllers/ControllerActionInvokerTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Controllers/ControllerActionInvokerTest.cs index 9990420a17..5d03b4a56a 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Controllers/ControllerActionInvokerTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Controllers/ControllerActionInvokerTest.cs @@ -14,6 +14,7 @@ using Microsoft.AspNet.Mvc.Abstractions; using Microsoft.AspNet.Mvc.Filters; using Microsoft.AspNet.Mvc.Formatters; using Microsoft.AspNet.Mvc.Infrastructure; +using Microsoft.AspNet.Mvc.Internal; using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.AspNet.Mvc.ModelBinding.Validation; using Microsoft.AspNet.Routing; @@ -22,7 +23,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; using Microsoft.Extensions.Options; -using Microsoft.Net.Http.Headers; using Moq; using Xunit; @@ -2027,10 +2027,10 @@ namespace Microsoft.AspNet.Mvc.Controllers { foreach (var filterMetadata in filters) { - var filter = new FilterItem( - new FilterDescriptor(filterMetadata, FilterScope.Action), - filterMetadata); - context.Results.Add(filter); + context.Results.Add(new FilterItem(new FilterDescriptor(filterMetadata, FilterScope.Action)) + { + Filter = filterMetadata, + }); } }); @@ -2106,7 +2106,7 @@ namespace Microsoft.AspNet.Mvc.Controllers var invoker = new ControllerActionInvoker( actionContext, - new List(), + CreateFilterCache(), controllerFactory.Object, actionDescriptor, new IInputFormatter[0], @@ -2222,11 +2222,18 @@ namespace Microsoft.AspNet.Mvc.Controllers } } + private static FilterCache CreateFilterCache(IFilterProvider[] filterProviders = null) + { + var services = new ServiceCollection().BuildServiceProvider(); + var descriptorProvider = new DefaultActionDescriptorCollectionProvider(services); + return new FilterCache(descriptorProvider, filterProviders.AsEnumerable() ?? new List()); + } + private class TestControllerActionInvoker : ControllerActionInvoker { public TestControllerActionInvoker( ActionContext actionContext, - IFilterProvider[] filterProvider, + IFilterProvider[] filterProviders, MockControllerFactory controllerFactory, ControllerActionDescriptor descriptor, IReadOnlyList inputFormatters, @@ -2239,7 +2246,7 @@ namespace Microsoft.AspNet.Mvc.Controllers int maxAllowedErrorsInModelState) : base( actionContext, - filterProvider, + CreateFilterCache(filterProviders), controllerFactory, descriptor, inputFormatters, diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Internal/FilterCacheTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Internal/FilterCacheTest.cs new file mode 100644 index 0000000000..74c206fa4a --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Internal/FilterCacheTest.cs @@ -0,0 +1,157 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNet.Http.Internal; +using Microsoft.AspNet.Mvc.Controllers; +using Microsoft.AspNet.Mvc.Filters; +using Microsoft.AspNet.Mvc.Infrastructure; +using Microsoft.AspNet.Routing; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Microsoft.AspNet.Mvc.Internal +{ + public class FilterCacheTest + { + [Fact] + public void GetFilters_CachesAllFilters() + { + // Arrange + var services = CreateServices(); + var cache = CreateCache(new DefaultFilterProvider()); + + var action = new ControllerActionDescriptor() + { + FilterDescriptors = new[] + { + new FilterDescriptor(new TestFilter(), FilterScope.Action), + new FilterDescriptor(new TestFilter(), FilterScope.Action), + }, + }; + + var context = new ActionContext(new DefaultHttpContext(), new RouteData(), action); + + // Act - 1 + var filters1 = cache.GetFilters(context); + + // Assert - 1 + Assert.Collection( + filters1, + f => Assert.Same(action.FilterDescriptors[0].Filter, f), // Copied by provider + f => Assert.Same(action.FilterDescriptors[1].Filter, f)); // Copied by provider + + // Act - 2 + var filters2 = cache.GetFilters(context); + + Assert.Same(filters1, filters2); + + Assert.Collection( + filters2, + f => Assert.Same(action.FilterDescriptors[0].Filter, f), // Cached + f => Assert.Same(action.FilterDescriptors[1].Filter, f)); // Cached + } + + [Fact] + public void GetFilters_CachesFilterFromFactory() + { + // Arrange + var services = CreateServices(); + var cache = CreateCache(new DefaultFilterProvider()); + + var action = new ControllerActionDescriptor() + { + FilterDescriptors = new[] + { + new FilterDescriptor(new TestFilterFactory() { IsReusable = true }, FilterScope.Action), + new FilterDescriptor(new TestFilter(), FilterScope.Action), + }, + }; + + var context = new ActionContext(new DefaultHttpContext(), new RouteData(), action); + + // Act - 1 + var filters1 = cache.GetFilters(context); + + // Assert - 1 + Assert.Collection( + filters1, + f => Assert.NotSame(action.FilterDescriptors[0].Filter, f), // Created by factory + f => Assert.Same(action.FilterDescriptors[1].Filter, f)); // Copied by provider + + // Act - 2 + var filters2 = cache.GetFilters(context); + + Assert.Same(filters1, filters2); + + Assert.Collection( + filters2, + f => Assert.Same(filters1[0], f), // Cached + f => Assert.Same(filters1[1], f)); // Cached + } + + [Fact] + public void GetFilters_DoesNotCacheFiltersWithIsReusableFalse() + { + // Arrange + var services = CreateServices(); + var cache = CreateCache(new DefaultFilterProvider()); + + var action = new ControllerActionDescriptor() + { + FilterDescriptors = new[] + { + new FilterDescriptor(new TestFilterFactory() { IsReusable = false }, FilterScope.Action), + new FilterDescriptor(new TestFilter(), FilterScope.Action), + }, + }; + + var context = new ActionContext(new DefaultHttpContext(), new RouteData(), action); + + // Act - 1 + var filters1 = cache.GetFilters(context); + + // Assert - 1 + Assert.Collection( + filters1, + f => Assert.NotSame(action.FilterDescriptors[0].Filter, f), // Created by factory + f => Assert.Same(action.FilterDescriptors[1].Filter, f)); // Copied by provider + + // Act - 2 + var filters2 = cache.GetFilters(context); + + Assert.NotSame(filters1, filters2); + + Assert.Collection( + filters2, + f => Assert.NotSame(filters1[0], f), // Created by factory (again) + f => Assert.Same(filters1[1], f)); // Cached + } + + private class TestFilter : IFilterMetadata + { + } + + private class TestFilterFactory : IFilterFactory + { + public bool IsReusable { get; set; } + + public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) + { + return new TestFilter(); + } + } + + private static IServiceProvider CreateServices() + { + return new ServiceCollection().BuildServiceProvider(); + } + + private static FilterCache CreateCache(params IFilterProvider[] providers) + { + var services = CreateServices(); + var descriptorProvider = new DefaultActionDescriptorCollectionProvider(services); + return new FilterCache(descriptorProvider, providers); + } + } +}