Add global filters with the right scope

Previously global filters were added to the page application model with Action scope. This would have resulted in incorrect ordering of filters during execution. We'll instead add global filters separately with the right scope. #6579 will be used to express global filters as part of the application model.
aspnet · Jul 21, 2017 · 2ef2648 · 2ef2648
1 parent a5b55ed
commit 2ef2648
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 61 deletions.
diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/CompiledPageActionDescriptorBuilder.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/CompiledPageActionDescriptorBuilder.cs
@@ -18,12 +18,16 @@ public static class CompiledPageActionDescriptorBuilder
         /// Creates a <see cref="CompiledPageActionDescriptor"/> from the specified <paramref name="applicationModel"/>.
         /// </summary>
         /// <param name="applicationModel">The <see cref="PageApplicationModel"/>.</param>
+        /// <param name="globalFilters">Global filters to apply to the page.</param>
         /// <returns>The <see cref="CompiledPageActionDescriptor"/>.</returns>
-        public static CompiledPageActionDescriptor Build(PageApplicationModel applicationModel)
+        public static CompiledPageActionDescriptor Build(
+            PageApplicationModel applicationModel,
+            FilterCollection globalFilters)
         {
             var boundProperties = CreateBoundProperties(applicationModel);
-            var filters = applicationModel.Filters
-                .Select(f => new FilterDescriptor(f, FilterScope.Action))
+            var filters = Enumerable.Concat(
+                    globalFilters.Select(f => new FilterDescriptor(f, FilterScope.Global)),
+                    applicationModel.Filters.Select(f => new FilterDescriptor(f, FilterScope.Action)))
                 .ToArray();
             var handlerMethods = CreateHandlerMethods(applicationModel);
 

diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageApplicationModelProvider.cs
@@ -16,16 +16,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
     public class DefaultPageApplicationModelProvider : IPageApplicationModelProvider
     {
         private const string ModelPropertyName = "Model";
-        private readonly FilterCollection _globalFilters;
-
-        /// <summary>
-        /// Initializes a new instance of <see cref="DefaultPageApplicationModelProvider"/>.
-        /// </summary>
-        /// <param name="mvcOptions"></param>
-        public DefaultPageApplicationModelProvider(IOptions<MvcOptions> mvcOptions)
-        {
-            _globalFilters = mvcOptions.Value.Filters;
-        }
 
         /// <inheritdoc />
         public int Order => -1000;
@@ -146,11 +136,6 @@ internal void PopulateHandlerMethods(PageApplicationModel pageModel)
 
         internal void PopulateFilters(PageApplicationModel pageModel)
         {
-            for (var i = 0; i < _globalFilters.Count; i++)
-            {
-                pageModel.Filters.Add(_globalFilters[i]);
-            }
-
             for (var i = 0; i < pageModel.HandlerTypeAttributes.Count; i++)
             {
                 if (pageModel.HandlerTypeAttributes[i] is IFilterMetadata filter)

diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageLoader.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageLoader.cs
@@ -6,6 +6,7 @@
 using System.Linq;
 using System.Reflection;
 using Microsoft.AspNetCore.Mvc.ApplicationModels;
+using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Mvc.Razor.Compilation;
 using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure;
 using Microsoft.Extensions.Options;
@@ -17,11 +18,13 @@ public class DefaultPageLoader : IPageLoader
         private readonly IPageApplicationModelProvider[] _applicationModelProviders;
         private readonly IViewCompilerProvider _viewCompilerProvider;
         private readonly IPageApplicationModelConvention[] _conventions;
+        private readonly FilterCollection _globalFilters;
 
         public DefaultPageLoader(
             IEnumerable<IPageApplicationModelProvider> applicationModelProviders,
             IViewCompilerProvider viewCompilerProvider,
-            IOptions<RazorPagesOptions> pageOptions)
+            IOptions<RazorPagesOptions> pageOptions,
+            IOptions<MvcOptions> mvcOptions)
         {
             _applicationModelProviders = applicationModelProviders
                 .OrderBy(p => p.Order)
@@ -30,6 +33,7 @@ public DefaultPageLoader(
             _conventions = pageOptions.Value.Conventions
                 .OfType<IPageApplicationModelConvention>()
                 .ToArray();
+            _globalFilters = mvcOptions.Value.Filters;
         }
 
         private IViewCompiler Compiler => _viewCompilerProvider.GetCompiler();
@@ -61,7 +65,7 @@ public CompiledPageActionDescriptor Load(PageActionDescriptor actionDescriptor)
                 _conventions[i].Apply(context.PageApplicationModel);
             }
 
-            return CompiledPageActionDescriptorBuilder.Build(context.PageApplicationModel);
+            return CompiledPageActionDescriptorBuilder.Build(context.PageApplicationModel, _globalFilters);
         }
     }
 }
diff --git a/....AspNetCore.Mvc.RazorPages.Test/Internal/AuthorizationPageApplicationModelProviderTest.cs b/....AspNetCore.Mvc.RazorPages.Test/Internal/AuthorizationPageApplicationModelProviderTest.cs
@@ -151,7 +151,7 @@ public void OnGet() { }
 
         private static PageApplicationModelProviderContext GetApplicationProviderContext(TypeInfo typeInfo)
         {
-            var defaultProvider = new DefaultPageApplicationModelProvider(new TestOptionsManager<MvcOptions>());
+            var defaultProvider = new DefaultPageApplicationModelProvider();
             var context = new PageApplicationModelProviderContext(new PageActionDescriptor(), typeInfo);
             defaultProvider.OnProvidersExecuting(context);
             return context;

diff --git a/...rosoft.AspNetCore.Mvc.RazorPages.Test/Internal/CompiledPageActionDescriptorBuilderTest.cs b/...rosoft.AspNetCore.Mvc.RazorPages.Test/Internal/CompiledPageActionDescriptorBuilderTest.cs
@@ -31,9 +31,10 @@ public void CreateDescriptor_CopiesPropertiesFromPageActionDescriptor()
             };
             var handlerTypeInfo = typeof(object).GetTypeInfo();
             var pageApplicationModel = new PageApplicationModel(actionDescriptor, handlerTypeInfo, new object[0]);
+            var globalFilters = new FilterCollection();
 
             // Act
-            var actual = CompiledPageActionDescriptorBuilder.Build(pageApplicationModel);
+            var actual = CompiledPageActionDescriptorBuilder.Build(pageApplicationModel, globalFilters);
 
             // Assert
             Assert.Same(actionDescriptor.ActionConstraints, actual.ActionConstraints);
@@ -78,9 +79,10 @@ public void CreateDescriptor_CopiesPropertiesFromPageApplicationModel()
                     },
                 }
             };
+            var globalFilters = new FilterCollection();
 
             // Act
-            var actual = CompiledPageActionDescriptorBuilder.Build(pageApplicationModel);
+            var actual = CompiledPageActionDescriptorBuilder.Build(pageApplicationModel, globalFilters);
 
             // Assert
             Assert.Same(pageApplicationModel.PageType, actual.PageTypeInfo);
@@ -92,6 +94,52 @@ public void CreateDescriptor_CopiesPropertiesFromPageApplicationModel()
             Assert.Equal(pageApplicationModel.HandlerProperties.Select(p => p.PropertyName), actual.BoundProperties.Select(p => p.Name));
         }
 
+        [Fact]
+        public void CreateDescriptor_AddsGlobalFiltersWithTheRightScope()
+        {
+            // Arrange
+            var actionDescriptor = new PageActionDescriptor
+            {
+                ActionConstraints = new List<IActionConstraintMetadata>(),
+                AttributeRouteInfo = new AttributeRouteInfo(),
+                FilterDescriptors = new List<FilterDescriptor>(),
+                RelativePath = "/Foo",
+                RouteValues = new Dictionary<string, string>(),
+                ViewEnginePath = "/Pages/Foo",
+            };
+            var handlerTypeInfo = typeof(TestModel).GetTypeInfo();
+            var pageApplicationModel = new PageApplicationModel(actionDescriptor, handlerTypeInfo, new object[0])
+            {
+                PageType = typeof(TestPage).GetTypeInfo(),
+                ModelType = typeof(TestModel).GetTypeInfo(),
+                Filters =
+                {
+                    Mock.Of<IFilterMetadata>(),
+                },
+            };
+            var globalFilters = new FilterCollection
+            {
+                Mock.Of<IFilterMetadata>(),
+            };
+
+            // Act
+            var compiledPageActionDescriptor = CompiledPageActionDescriptorBuilder.Build(pageApplicationModel, globalFilters);
+
+            // Assert
+            Assert.Collection(
+                compiledPageActionDescriptor.FilterDescriptors,
+                filterDescriptor =>
+                {
+                    Assert.Same(globalFilters[0], filterDescriptor.Filter);
+                    Assert.Equal(FilterScope.Global, filterDescriptor.Scope);
+                },
+                filterDescriptor =>
+                {
+                    Assert.Same(pageApplicationModel.Filters[0], filterDescriptor.Filter);
+                    Assert.Equal(FilterScope.Action, filterDescriptor.Scope);
+                });
+        }
+
         private class TestPage
         {
             public TestModel Model { get; } = new TestModel();