Fix #4914 patternize filter overrides

Adding some convenience methods to the context class and updating our
code to use them.

src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/FilterContext.cs

@@ -33,5 +33,67 @@ public FilterContext(
         /// Gets all applicable <see cref="IFilterMetadata"/> implementations.
         /// </summary>
         public virtual IList<IFilterMetadata> Filters { get; }
+
+        /// <summary>
+        /// Returns a value indicating whether the provided <see cref="IFilterMetadata"/> is the most effective
+        /// policy (most specific) applied to the action associated with the <see cref="FilterContext"/>.
+        /// </summary>
+        /// <typeparam name="TMetadata">The type of the filter policy.</typeparam>
+        /// <param name="policy">The filter policy instance.</param>
+        /// <returns>
+        /// <c>true</c> if the provided <see cref="IFilterMetadata"/> is the most effective policy, otherwise <c>false</c>.
+        /// </returns>
+        /// <remarks>
+        /// <para>
+        /// The <see cref="IsEffectivePolicy{TMetadata}(TMetadata)"/> 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 (<typeparamref name="TMetadata"/>) and 
+        /// implement the filters such that all of the implementations call this method to determine if they should
+        /// take action.
+        /// </para>
+        /// <para>
+        /// 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.
+        /// </para>
+        /// <para>
+        /// 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.
+        /// </para>
+        /// </remarks>
+        public bool IsEffectivePolicy<TMetadata>(TMetadata policy) where TMetadata : IFilterMetadata
+        {
+            if (policy == null)
+            {
+                throw new ArgumentNullException(nameof(policy));
+            }
+
+            var effective = FindEffectivePolicy<TMetadata>();
+            return ReferenceEquals(policy, effective);
+        }
+
+        /// <summary>
+        /// Returns the most effective (most specific) policy of type <typeparamref name="TMetadata"/> applied to 
+        /// the action assocatied with the <see cref="FilterContext"/>.
+        /// </summary>
+        /// <typeparam name="TMetadata">The type of the filter policy.</typeparam>
+        /// <returns>The implementation of <typeparamref name="TMetadata"/> applied to the action assocatied with
+        /// the <see cref="FilterContext"/>
+        /// </returns>
+        public TMetadata FindEffectivePolicy<TMetadata>() 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);
+        }
     }
 }

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

@@ -40,7 +40,7 @@ public void OnAuthorization(AuthorizationFilterContext context)
                 throw new ArgumentNullException(nameof(context));
             }
 
-            if (IsClosestRequestSizePolicy(context.Filters))
+            if (context.IsEffectivePolicy<IRequestSizePolicy>(this))
             {
                 var maxRequestBodySizeFeature = context.HttpContext.Features.Get<IHttpMaxRequestBodySizeFeature>();
 
@@ -59,21 +59,5 @@ public void OnAuthorization(AuthorizationFilterContext context)
                 }
             }
         }
-
-        private bool IsClosestRequestSizePolicy(IList<IFilterMetadata> 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;
-        }
     }
 }

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

@@ -41,7 +41,7 @@ public void OnAuthorization(AuthorizationFilterContext context)
                 throw new ArgumentNullException(nameof(context));
             }
 
-            if (IsClosestRequestSizePolicy(context.Filters))
+            if (context.IsEffectivePolicy<IRequestSizePolicy>(this))
             {
                 var maxRequestBodySizeFeature = context.HttpContext.Features.Get<IHttpMaxRequestBodySizeFeature>();
 
@@ -60,21 +60,5 @@ public void OnAuthorization(AuthorizationFilterContext context)
                 }
             }
         }
-
-        private bool IsClosestRequestSizePolicy(IList<IFilterMetadata> 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;
-        }
     }
 }

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

@@ -86,7 +86,7 @@ public void OnActionExecuting(ActionExecutingContext context)
 
             // 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<IResponseCacheFilter>(this))
             {
                 return;
             }

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

@@ -130,24 +130,5 @@ public void Execute(FilterContext context)
                 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;
-        }
     }
 }

src/Microsoft.AspNetCore.Mvc.Cors/CorsAuthorizationFilter.cs

@@ -51,7 +51,7 @@ public async Task OnAuthorizationAsync(Filters.AuthorizationFilterContext contex
             }
 
             // If this filter is not closest to the action, it is not applicable.
-            if (!IsClosestToAction(context.Filters))
+            if (!context.IsEffectivePolicy<ICorsAuthorizationFilter>(this))
             {
                 return;
             }
@@ -87,14 +87,5 @@ public async Task OnAuthorizationAsync(Filters.AuthorizationFilterContext contex
                 // Continue with other filters and action.
             }
         }
-
-        private bool IsClosestToAction(IEnumerable<IFilterMetadata> 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;
-        }
     }
 }

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

@@ -88,7 +88,7 @@ public void OnPageHandlerExecuting(PageHandlerExecutingContext context)
                 throw new ArgumentNullException(nameof(context));
             }
 
-            if (ResponseCacheFilterExecutor.IsOverridden(this, context))
+            if (!context.IsEffectivePolicy<IResponseCacheFilter>(this))
             {
                 return;
             }

src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ValidateAntiforgeryTokenAuthorizationFilter.cs

@@ -34,7 +34,7 @@ public async Task OnAuthorizationAsync(AuthorizationFilterContext context)
                 throw new ArgumentNullException(nameof(context));
             }
 
-            if (IsClosestAntiforgeryPolicy(context.Filters) && ShouldValidate(context))
+            if (context.IsEffectivePolicy<IAntiforgeryPolicy>(this) && ShouldValidate(context))
             {
                 try
                 {
@@ -57,21 +57,5 @@ protected virtual bool ShouldValidate(AuthorizationFilterContext context)
 
             return true;
         }
-
-        private bool IsClosestAntiforgeryPolicy(IList<IFilterMetadata> 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;
-        }
     }
 }

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<ITestFilterPolicy>(),
+                Mock.Of<IAnotherTestFilterPolicy>(),
+                Mock.Of<ITestFilterPolicy>(),
+            };
+
+            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<ITestFilterPolicy>(),
+                Mock.Of<IAnotherTestFilterPolicy>(),
+                Mock.Of<ITestFilterPolicy>(),
+            };
+
+            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<ITestFilterPolicy>(),
+                Mock.Of<ITestFilterPolicy>(),
+            };
+
+            var context = new TestFilterContext(filters);
+
+            // Act
+            var result = context.IsEffectivePolicy(Mock.Of<IAnotherTestFilterPolicy>());
+
+            // Assert
+            Assert.False(result);
+        }
+
+
+        [Fact]
+        public void FindEffectivePolicy_FindsLastFilter_ReturnsIt()
+        {
+            // Arrange
+            var filters = new IFilterMetadata[]
+            {
+                Mock.Of<ITestFilterPolicy>(),
+                Mock.Of<IAnotherTestFilterPolicy>(),
+                Mock.Of<ITestFilterPolicy>(),
+            };
+
+            var context = new TestFilterContext(filters);
+
+            // Act
+            var result = context.FindEffectivePolicy<ITestFilterPolicy>();
+
+            // Assert
+            Assert.Same(filters.Last(), result);
+        }
+
+        [Fact]
+        public void FindEffectivePolicy_NoMatch_ReturnsNull()
+        {
+            // Arrange
+            var filters = new IFilterMetadata[]
+            {
+                Mock.Of<ITestFilterPolicy>(),
+                Mock.Of<ITestFilterPolicy>(),
+            };
+
+            var context = new TestFilterContext(filters);
+
+            // Act
+            var result = context.FindEffectivePolicy<IAnotherTestFilterPolicy>();
+
+            // Assert
+            Assert.Null(result);
+        }
+
+        internal class ITestFilterPolicy : IFilterMetadata
+        {
+        }
+
+        internal class IAnotherTestFilterPolicy : IFilterMetadata
+        {
+        }
+
+        private class TestFilterContext : FilterContext
+        {
+            public TestFilterContext(IList<IFilterMetadata> filters) 
+                : base(new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()), filters)
+            {
+            }
+        }
+    }
+}

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")]