diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/FilterContext.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/FilterContext.cs index f74e7c7404..1a88cb721e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/FilterContext.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/FilterContext.cs @@ -33,5 +33,67 @@ namespace Microsoft.AspNetCore.Mvc.Filters /// Gets all applicable implementations. /// public virtual IList Filters { get; } + + /// + /// Returns a value indicating whether the provided is the most effective + /// policy (most specific) applied to the action associated with the . + /// + /// The type of the filter policy. + /// The filter policy instance. + /// + /// true if the provided is the most effective policy, otherwise false. + /// + /// + /// + /// The method is used to implement a common convention + /// for filters that define an overriding behavior. When multiple filters may apply to the same + /// cross-cutting concern, define a common interface for the filters () and + /// implement the filters such that all of the implementations call this method to determine if they should + /// take action. + /// + /// + /// For instance, a global filter might be overridden by placing a filter attribute on an action method. + /// The policy applied directly to the action method could be considered more specific. + /// + /// + /// This mechanism for overriding relies on the rules of order and scope that the filter system + /// provides to control ordering of filters. It is up to the implementor of filters to implement this + /// protocol cooperatively. The filter system has no innate notion of overrides, this is a recommended + /// convention. + /// + /// + public bool IsEffectivePolicy(TMetadata policy) where TMetadata : IFilterMetadata + { + if (policy == null) + { + throw new ArgumentNullException(nameof(policy)); + } + + var effective = FindEffectivePolicy(); + return ReferenceEquals(policy, effective); + } + + /// + /// Returns the most effective (most specific) policy of type applied to + /// the action assocatied with the . + /// + /// The type of the filter policy. + /// The implementation of applied to the action assocatied with + /// the + /// + public TMetadata FindEffectivePolicy() where TMetadata : IFilterMetadata + { + // The most specific policy is the one closest to the action (nearest the end of the list). + for (var i = Filters.Count - 1; i >= 0; i--) + { + var filter = Filters[i]; + if (filter is TMetadata match) + { + return match; + } + } + + return default(TMetadata); + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DisableRequestSizeLimitFilter.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DisableRequestSizeLimitFilter.cs index e34679a335..09a68a3b8b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DisableRequestSizeLimitFilter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DisableRequestSizeLimitFilter.cs @@ -40,7 +40,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal throw new ArgumentNullException(nameof(context)); } - if (IsClosestRequestSizePolicy(context.Filters)) + if (context.IsEffectivePolicy(this)) { var maxRequestBodySizeFeature = context.HttpContext.Features.Get(); @@ -59,21 +59,5 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } } - - private bool IsClosestRequestSizePolicy(IList filters) - { - // Determine if this instance is the 'effective' request size policy. - for (var i = filters.Count - 1; i >= 0; i--) - { - var filter = filters[i]; - if (filter is IRequestSizePolicy) - { - return ReferenceEquals(this, filter); - } - } - - Debug.Fail("The current instance should be in the list of filters."); - return false; - } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/RequestSizeLimitFilter.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/RequestSizeLimitFilter.cs index 92d8cc40e6..3b63566e91 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/RequestSizeLimitFilter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/RequestSizeLimitFilter.cs @@ -41,7 +41,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal throw new ArgumentNullException(nameof(context)); } - if (IsClosestRequestSizePolicy(context.Filters)) + if (context.IsEffectivePolicy(this)) { var maxRequestBodySizeFeature = context.HttpContext.Features.Get(); @@ -60,21 +60,5 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } } - - private bool IsClosestRequestSizePolicy(IList filters) - { - // Determine if this instance is the 'effective' request size policy. - for (var i = filters.Count - 1; i >= 0; i--) - { - var filter = filters[i]; - if (filter is IRequestSizePolicy) - { - return ReferenceEquals(this, filter); - } - } - - Debug.Fail("The current instance should be in the list of filters."); - return false; - } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilter.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilter.cs index 5138927c89..6ca409f601 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilter.cs @@ -86,7 +86,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // If there are more filters which can override the values written by this filter, // then skip execution of this filter. - if (ResponseCacheFilterExecutor.IsOverridden(this, context)) + if (!context.IsEffectivePolicy(this)) { return; } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilterExecutor.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilterExecutor.cs index ee86c5d860..02100b838e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilterExecutor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResponseCacheFilterExecutor.cs @@ -130,24 +130,5 @@ namespace Microsoft.AspNetCore.Mvc.Internal headers[HeaderNames.CacheControl] = cacheControlValue; } } - - public static bool IsOverridden(IResponseCacheFilter executingFilter, FilterContext context) - { - Debug.Assert(context != null); - - // Return true if there are any filters which are after the current filter. In which case the current - // filter should be skipped. - for (var i = context.Filters.Count - 1; i >= 0; i--) - { - var filter = context.Filters[i]; - if (filter is IResponseCacheFilter) - { - return !object.ReferenceEquals(executingFilter, filter); - } - } - - Debug.Fail("The executing filter must be part of the filter context."); - return false; - } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Cors/CorsAuthorizationFilter.cs b/src/Microsoft.AspNetCore.Mvc.Cors/CorsAuthorizationFilter.cs index 184d9560be..87f23963d1 100644 --- a/src/Microsoft.AspNetCore.Mvc.Cors/CorsAuthorizationFilter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Cors/CorsAuthorizationFilter.cs @@ -51,7 +51,7 @@ namespace Microsoft.AspNetCore.Mvc.Cors } // If this filter is not closest to the action, it is not applicable. - if (!IsClosestToAction(context.Filters)) + if (!context.IsEffectivePolicy(this)) { return; } @@ -87,14 +87,5 @@ namespace Microsoft.AspNetCore.Mvc.Cors // Continue with other filters and action. } } - - private bool IsClosestToAction(IEnumerable filters) - { - // If there are multiple ICorsAuthorizationFilter that are defined at the class and - // at the action level, the one closest to the action overrides the others. - // Since filterdescriptor collection is ordered (the last filter is the one closest to the action), - // we apply this constraint only if there is no ICorsAuthorizationFilter after this. - return filters.Last(filter => filter is ICorsAuthorizationFilter) == this; - } } } diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/ResponseCacheFilter.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/ResponseCacheFilter.cs index 353493550a..dbf177051e 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/ResponseCacheFilter.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/ResponseCacheFilter.cs @@ -88,7 +88,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal throw new ArgumentNullException(nameof(context)); } - if (ResponseCacheFilterExecutor.IsOverridden(this, context)) + if (!context.IsEffectivePolicy(this)) { return; } diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ValidateAntiforgeryTokenAuthorizationFilter.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ValidateAntiforgeryTokenAuthorizationFilter.cs index 43dd1c3920..aa34b96012 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ValidateAntiforgeryTokenAuthorizationFilter.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ValidateAntiforgeryTokenAuthorizationFilter.cs @@ -34,7 +34,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal throw new ArgumentNullException(nameof(context)); } - if (IsClosestAntiforgeryPolicy(context.Filters) && ShouldValidate(context)) + if (context.IsEffectivePolicy(this) && ShouldValidate(context)) { try { @@ -57,21 +57,5 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal return true; } - - private bool IsClosestAntiforgeryPolicy(IList filters) - { - // Determine if this instance is the 'effective' antiforgery policy. - for (var i = filters.Count - 1; i >= 0; i--) - { - var filter = filters[i]; - if (filter is IAntiforgeryPolicy) - { - return object.ReferenceEquals(this, filter); - } - } - - Debug.Fail("The current instance should be in the list of filters."); - return false; - } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/Filters/FilterContextTest.cs b/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/Filters/FilterContextTest.cs new file mode 100644 index 0000000000..0902e7f179 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/Filters/FilterContextTest.cs @@ -0,0 +1,131 @@ +// 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.Generic; +using System.Linq; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Routing; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Filters +{ + public class FilterContextTest + { + [Fact] + public void IsEffectivePolicy_FindsAnotherFilter_ReturnsFalse() + { + // Arrange + var filters = new IFilterMetadata[] + { + Mock.Of(), + Mock.Of(), + Mock.Of(), + }; + + var context = new TestFilterContext(filters); + + // Act + var result = context.IsEffectivePolicy((ITestFilterPolicy)filters.First()); + + // Assert + Assert.False(result); + } + + [Fact] + public void IsEffectivePolicy_FindsFilterOfInterest_ReturnsTrue() + { + // Arrange + var filters = new IFilterMetadata[] + { + Mock.Of(), + Mock.Of(), + Mock.Of(), + }; + + var context = new TestFilterContext(filters); + + // Act + var result = context.IsEffectivePolicy((ITestFilterPolicy)filters.Last()); + + // Assert + Assert.True(result); + } + + [Fact] + public void IsEffectivePolicy_NoMatch_ReturnsFalse() + { + // Arrange + var filters = new IFilterMetadata[] + { + Mock.Of(), + Mock.Of(), + }; + + var context = new TestFilterContext(filters); + + // Act + var result = context.IsEffectivePolicy(Mock.Of()); + + // Assert + Assert.False(result); + } + + + [Fact] + public void FindEffectivePolicy_FindsLastFilter_ReturnsIt() + { + // Arrange + var filters = new IFilterMetadata[] + { + Mock.Of(), + Mock.Of(), + Mock.Of(), + }; + + var context = new TestFilterContext(filters); + + // Act + var result = context.FindEffectivePolicy(); + + // Assert + Assert.Same(filters.Last(), result); + } + + [Fact] + public void FindEffectivePolicy_NoMatch_ReturnsNull() + { + // Arrange + var filters = new IFilterMetadata[] + { + Mock.Of(), + Mock.Of(), + }; + + var context = new TestFilterContext(filters); + + // Act + var result = context.FindEffectivePolicy(); + + // Assert + Assert.Null(result); + } + + internal class ITestFilterPolicy : IFilterMetadata + { + } + + internal class IAnotherTestFilterPolicy : IFilterMetadata + { + } + + private class TestFilterContext : FilterContext + { + public TestFilterContext(IList filters) + : base(new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()), filters) + { + } + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/Properties/AssemblyInfo.cs b/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/Properties/AssemblyInfo.cs new file mode 100644 index 0000000000..3337ebeac2 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/Properties/AssemblyInfo.cs @@ -0,0 +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.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7")] \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ResponseCacheFilterExecutorTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ResponseCacheFilterExecutorTest.cs index 23d71ae5f3..93969159db 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ResponseCacheFilterExecutorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ResponseCacheFilterExecutorTest.cs @@ -447,44 +447,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal("no-cache", context.HttpContext.Response.Headers["Pragma"]); } - [Fact] - public void IsOverridden_ReturnsTrueForAllButLastFilter() - { - // Arrange - var filter1 = new ResponseCacheFilter(new CacheProfile()); - var filter2 = new ResponseCacheFilter(new CacheProfile()); - var filters = new List - { - filter1, - Mock.Of(), - filter2, - Mock.Of(), - }; - var context = GetActionExecutingContext(filters); - - // Act & Assert - Assert.True(ResponseCacheFilterExecutor.IsOverridden(filter1, context)); - Assert.False(ResponseCacheFilterExecutor.IsOverridden(filter2, context)); - } - - [Fact] - public void IsOverridden_ReturnsTrueIfInstanceIsTheOnlyResponseCacheFilter() - { - // Arrange - var filter = new ResponseCacheFilter(new CacheProfile()); - var filters = new List - { - Mock.Of(), - filter, - Mock.Of(), - Mock.Of(), - }; - var context = GetActionExecutingContext(filters); - - // Act & Assert - Assert.False(ResponseCacheFilterExecutor.IsOverridden(filter, context)); - } - [Fact] public void FilterDurationProperty_OverridesCachePolicySetting() {