refactor to return ordered filter list from filter provider

Arrange filters to a pipeline in the action invoker
Allow providing the original filter definition to a type and service filter

src/Microsoft.AspNet.Mvc.Core/ActionResults/EmptyResult.cs

@@ -13,6 +13,7 @@ namespace Microsoft.AspNet.Mvc
 
         public async Task ExecuteResultAsync(ActionContext context)
         {
+            context.HttpContext.Response.StatusCode = 204;
         }
     }
 }

src/Microsoft.AspNet.Mvc.Core/Filters/ActionFilterAttribute.cs

@@ -5,7 +5,7 @@ namespace Microsoft.AspNet.Mvc
 {
     // TODO: Consider making this user a before and after pattern instead of just Invoke, same for all other filter attributes.
     [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true, Inherited = true)]
-    public abstract class ActionFilterAttribute : Attribute, IActionFilter, IFilter
+    public abstract class ActionFilterAttribute : Attribute, IActionFilter, IOrderedFilter
     {
         public abstract Task Invoke(ActionFilterContext context, Func<Task> next);
 

src/Microsoft.AspNet.Mvc.Core/Filters/ActionResultFilterAttribute.cs

@@ -4,7 +4,7 @@ using System.Threading.Tasks;
 namespace Microsoft.AspNet.Mvc
 {
     [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true, Inherited = true)]
-    public abstract class ActionResultFilterAttribute : Attribute, IActionResultFilter, IFilter
+    public abstract class ActionResultFilterAttribute : Attribute, IActionResultFilter, IOrderedFilter
     {
         public abstract Task Invoke(ActionResultFilterContext context, Func<Task> next);
 

src/Microsoft.AspNet.Mvc.Core/Filters/AuthorizationFilterAttribute.cs

@@ -4,7 +4,7 @@ using System.Threading.Tasks;
 namespace Microsoft.AspNet.Mvc
 {
     [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true, Inherited = true)]
-    public abstract class AuthorizationFilterAttribute : Attribute, IFilter, IAuthorizationFilter
+    public abstract class AuthorizationFilterAttribute : Attribute, IAuthorizationFilter, IOrderedFilter
     {
         public abstract Task Invoke(AuthorizationFilterContext context, Func<Task> next);
 

src/Microsoft.AspNet.Mvc.Core/Filters/DefaultFilterProvider.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.Contracts;
 using Microsoft.AspNet.DependencyInjection;
 
 namespace Microsoft.AspNet.Mvc.Filters
@@ -11,23 +12,23 @@ namespace Microsoft.AspNet.Mvc.Filters
             ServiceProvider = serviceProvider;
         }
 
+        public int Order
+        {
+            get { return 0; }
+        }
+
+        protected IServiceProvider ServiceProvider { get; private set; }
+
         public virtual void Invoke(FilterProviderContext context, Action callNext)
         {
-            FilterDescriptor[] filterDescriptors;
-
             if (context.ActionDescriptor.FilterDescriptors != null)
             {
                 // make a copy of the list, TODO: Make the actiondescriptor immutable
-                filterDescriptors = context.ActionDescriptor.FilterDescriptors.ToArray();
+                var filterDescriptors = context.ActionDescriptor.FilterDescriptors.ToArray();
 
-                //AddGlobalFilters_moveToAdPipeline(filters);
+                for (int i = 0; i < filterDescriptors.Length; i++)
-
-                if (filterDescriptors.Length > 0)
                 {
-                    for (int i = 0; i < filterDescriptors.Length; i++)
+                    ProvideFilter(context, filterDescriptors[i].Filter);
-                    {
-                        GetFilter(context, filterDescriptors[i].Filter);
-                    }
                 }
             }
 
@@ -37,96 +38,69 @@ namespace Microsoft.AspNet.Mvc.Filters
             }
         }
 
-        public virtual void GetFilter(FilterProviderContext context, IFilter filter)
+        public virtual void ProvideFilter(FilterProviderContext context, IFilter filter)
         {
-            bool failIfNotFilter = true;
-
             var serviceFilterSignature = filter as IServiceFilter;
             if (serviceFilterSignature != null)
             {
-                // TODO: How do we pass extra parameters
+                var serviceFilter = ServiceProvider.GetService(serviceFilterSignature.ServiceType) as IFilter;
-                var serviceFilter = ServiceProvider.GetService(serviceFilterSignature.ServiceType);
 
-                AddFilters(context, serviceFilter, true);
+                if (serviceFilter == null)
-                failIfNotFilter = false;
+                {
+                    throw new InvalidOperationException("Service filter must be of type IFilter");
+                }
+
+                ApplyFilterToContainer(serviceFilter, filter);
+                AddFilters(context, serviceFilter);
             }
-
+            else
-            var typeFilterSignature = filter as ITypeFilter;
-            if (typeFilterSignature != null)
             {
-                // TODO: How do we pass extra parameters
+                var typeFilterSignature = filter as ITypeFilter;
-                var typeFilter = ActivatorUtilities.CreateInstance(ServiceProvider, typeFilterSignature.ImplementationType);
+                if (typeFilterSignature != null)
+                {
+                    if (typeFilterSignature.ImplementationType == null)
+                    {
+                        throw new InvalidOperationException("Type filter must provide a type to instantiate");
+                    }
 
-                AddFilters(context, typeFilter, true);
+                    if (!typeof (IFilter).IsAssignableFrom(typeFilterSignature.ImplementationType))
-                failIfNotFilter = false;
+                    {
+                        throw new InvalidOperationException("Type filter must implement IFilter");
+                    }
+
+                    // TODO: Move activatorUtilities to come from the service provider.
+                    var typeFilter = ActivatorUtilities.CreateInstance(ServiceProvider, typeFilterSignature.ImplementationType) as IFilter;
+
+                    ApplyFilterToContainer(typeFilter, filter);
+                    AddFilters(context, typeFilter);
+                }
+                else
+                {
+                    AddFilters(context, filter);
+                }
             }
-
-            AddFilters(context, filter, failIfNotFilter);
         }
 
-        protected IServiceProvider ServiceProvider { get; private set; }
+        private void AddFilters(FilterProviderContext context, IFilter filter)
-
-        public int Order
         {
-            get { return 0; }
+            if (context.OrderedFilterList == null)
+            {
+                context.OrderedFilterList = new List<IFilter>();
+            }
+
+            context.OrderedFilterList.Add(filter);
         }
 
-        private void AddFilters(FilterProviderContext context, object filter, bool throwIfNotFilter)
+        private void ApplyFilterToContainer(object actualFilter, IFilter filterMetadata)
         {
-            bool shouldThrow = throwIfNotFilter;
+            Contract.Assert(actualFilter != null, "actualFilter should not be null");
+            Contract.Assert(filterMetadata != null, "filterMetadata should not be null");
 
-            var authFilter = filter as IAuthorizationFilter;
+            var container = actualFilter as IFilterContainer;
-            var actionFilter = filter as IActionFilter;
-            var actionResultFilter = filter as IActionResultFilter;
-            var exceptionFilter = filter as IExceptionFilter;
 
-            if (authFilter != null)
+            if (container != null)
             {
-                if (context.AuthorizationFilters == null)
+                container.FilterDefinition = filterMetadata;
-                {
-                    context.AuthorizationFilters = new List<IAuthorizationFilter>();
-                }
-
-                context.AuthorizationFilters.Add(authFilter);
-                shouldThrow = false;
-            }
-
-            if (actionFilter != null)
-            {
-                if (context.ActionFilters == null)
-                {
-                    context.ActionFilters = new List<IActionFilter>();
-                }
-
-                context.ActionFilters.Add(actionFilter);
-                shouldThrow = false;
-            }
-
-            if (actionResultFilter != null)
-            {
-                if (context.ActionResultFilters == null)
-                {
-                    context.ActionResultFilters = new List<IActionResultFilter>();
-                }
-
-                context.ActionResultFilters.Add(actionResultFilter);
-                shouldThrow = false;
-            }
-
-            if (exceptionFilter != null)
-            {
-                if (context.ExceptionFilters == null)
-                {
-                    context.ExceptionFilters = new List<IExceptionFilter>();
-                }
-
-                context.ExceptionFilters.Add(exceptionFilter);
-                shouldThrow = false;
-            }
-
-            if (shouldThrow)
-            {
-                throw new InvalidOperationException("Filter has to be IActionResultFilter, IActionFilter, IExceptionFilter or IAuthorizationFilter.");
             }
         }
     }

src/Microsoft.AspNet.Mvc.Core/Filters/ExceptionFilterAttribute.cs

@@ -4,7 +4,7 @@ using System.Threading.Tasks;
 namespace Microsoft.AspNet.Mvc
 {
     [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true, Inherited = true)]
-    public abstract class ExceptionFilterAttribute : Attribute, IExceptionFilter, IFilter
+    public abstract class ExceptionFilterAttribute : Attribute, IExceptionFilter, IOrderedFilter
     {
         public abstract Task Invoke(ExceptionFilterContext context, Func<Task> next);
 

src/Microsoft.AspNet.Mvc.Core/Filters/FilterDescriptor.cs

@@ -1,6 +1,4 @@
-using System;
+namespace Microsoft.AspNet.Mvc
-
-namespace Microsoft.AspNet.Mvc
 {
     public class FilterDescriptor
     {
@@ -8,9 +6,19 @@ namespace Microsoft.AspNet.Mvc
         {
             Filter = filter;
             Scope = filterScope;
+
+            var orderedFilter = Filter as IOrderedFilter;
+
+            if (orderedFilter != null)
+            {
+                Order = orderedFilter.Order;
+            }
         }
 
         public IFilter Filter { get; private set; }
+
+        public int Order { get; private set; }
+
         public int Scope { get; private set; }
     }
 }

src/Microsoft.AspNet.Mvc.Core/Filters/FilterDescriptorOrderComparer.cs

@@ -10,13 +10,13 @@ namespace Microsoft.AspNet.Mvc
 
         public int Compare([NotNull]FilterDescriptor x, [NotNull]FilterDescriptor y)
         {
-            if (x.Filter.Order == y.Filter.Order)
+            if (x.Order == y.Order)
             {
                 return x.Scope.CompareTo(y.Scope);
             }
             else
             {
-                return x.Filter.Order.CompareTo(y.Filter.Order);
+                return x.Order.CompareTo(y.Order);
             }
         }
     }

src/Microsoft.AspNet.Mvc.Core/Filters/FilterProviderContext.cs

@@ -12,13 +12,7 @@ namespace Microsoft.AspNet.Mvc
         // Input
         public ActionDescriptor ActionDescriptor { get; set; }
 
-        // Results
+        // Result
-        public List<IAuthorizationFilter> AuthorizationFilters { get; set; }
+        public List<IFilter> OrderedFilterList { get; set; }
-
-        public List<IActionFilter> ActionFilters { get; set; }
-
-        public List<IActionResultFilter> ActionResultFilters { get; set; }
-
-        public List<IExceptionFilter> ExceptionFilters { get; set; }
     }
 }

src/Microsoft.AspNet.Mvc.Core/Filters/IFilter.cs

@@ -2,6 +2,5 @@ namespace Microsoft.AspNet.Mvc
 {
     public interface IFilter
     {
-        int Order { get; }
     }
 }

src/Microsoft.AspNet.Mvc.Core/Filters/IFilterContainer.cs

@@ -0,0 +1,7 @@
+namespace Microsoft.AspNet.Mvc.Filters
+{
+    public interface IFilterContainer
+    {
+        IFilter FilterDefinition { get; set; }
+    }
+}

src/Microsoft.AspNet.Mvc.Core/Filters/IOrderedFilter.cs

@@ -0,0 +1,7 @@
+namespace Microsoft.AspNet.Mvc
+{
+    public interface IOrderedFilter : IFilter
+    {
+        int Order { get; }
+    }
+}

src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvoker.cs

@@ -13,18 +13,20 @@ namespace Microsoft.AspNet.Mvc
         private readonly ActionContext _actionContext;
         private readonly ReflectedActionDescriptor _descriptor;
         private readonly IActionResultFactory _actionResultFactory;
-        private readonly IServiceProvider _serviceProvider;
         private readonly IControllerFactory _controllerFactory;
         private readonly IActionBindingContextProvider _bindingProvider;
         private readonly INestedProviderManager<FilterProviderContext> _filterProvider;
 
+        private readonly List<IAuthorizationFilter> _authorizationFilters = new List<IAuthorizationFilter>();
+        private readonly List<IActionFilter> _actionFilters = new List<IActionFilter>();
+        private readonly List<IActionResultFilter> _actionResultFilters = new List<IActionResultFilter>();
+
         public ReflectedActionInvoker(ActionContext actionContext,
                                       ReflectedActionDescriptor descriptor,
                                       IActionResultFactory actionResultFactory,
                                       IControllerFactory controllerFactory,
                                       IActionBindingContextProvider bindingContextProvider,
-                                      INestedProviderManager<FilterProviderContext> filterProvider,
+                                      INestedProviderManager<FilterProviderContext> filterProvider)
-                                      IServiceProvider serviceProvider)
         {
             _actionContext = actionContext;
             _descriptor = descriptor;
@@ -32,15 +34,15 @@ namespace Microsoft.AspNet.Mvc
             _controllerFactory = controllerFactory;
             _bindingProvider = bindingContextProvider;
             _filterProvider = filterProvider;
-
-            _serviceProvider = serviceProvider;
         }
 
         public async Task InvokeActionAsync()
         {
             IActionResult actionResult;
-            var context = new FilterProviderContext(_descriptor);
+            var filterProviderContext = new FilterProviderContext(_descriptor);
-            _filterProvider.Invoke(context);
+            _filterProvider.Invoke(filterProviderContext);
+
+            PreArrangeFiltersInPipeline(filterProviderContext);
 
             var modelState = new ModelStateDictionary();
             object controller = _controllerFactory.CreateController(_actionContext, modelState);
@@ -61,15 +63,13 @@ namespace Microsoft.AspNet.Mvc
                 {
                     var parameterValues = await GetParameterValues(modelState);
 
-                    var authZFilters = context.AuthorizationFilters;
+                    if (_authorizationFilters.Count > 0)
-
-                    if (authZFilters != null && authZFilters.Count > 0)
                     {
                         var authZEndPoint = new AuthorizationFilterEndPoint();
-                        authZFilters.Add(authZEndPoint);
+                        _authorizationFilters.Add(authZEndPoint);
 
                         var authZContext = new AuthorizationFilterContext(_actionContext);
-                        var authZPipeline = new FilterPipelineBuilder<AuthorizationFilterContext>(authZFilters, authZContext);
+                        var authZPipeline = new FilterPipelineBuilder<AuthorizationFilterContext>(_authorizationFilters, authZContext);
 
                         await authZPipeline.InvokeAsync();
 
@@ -92,7 +92,6 @@ namespace Microsoft.AspNet.Mvc
 
                     if (actionResult == null)
                     {
-                        var actionFilters = context.ActionFilters ?? new List<IActionFilter>();
                         var actionFilterContext = new ActionFilterContext(_actionContext,
                                                                           parameterValues,
                                                                           method.ReturnType);
@@ -101,9 +100,9 @@ namespace Microsoft.AspNet.Mvc
                         var actionEndPoint = new ReflectedActionFilterEndPoint(async (inArray) => method.Invoke(controller, inArray),
                                                                                 _actionResultFactory);
 
-                        actionFilters.Add(actionEndPoint);
+                        _actionFilters.Add(actionEndPoint);
 
-                        var actionFilterPipeline = new FilterPipelineBuilder<ActionFilterContext>(actionFilters,
+                        var actionFilterPipeline = new FilterPipelineBuilder<ActionFilterContext>(_actionFilters,
                             actionFilterContext);
 
                         await actionFilterPipeline.InvokeAsync();
@@ -113,12 +112,11 @@ namespace Microsoft.AspNet.Mvc
                 }
             }
 
-            var actionResultFilters = context.ActionResultFilters ?? new List<IActionResultFilter>();
             var actionResultFilterContext = new ActionResultFilterContext(_actionContext, actionResult);
             var actionResultFilterEndPoint = new ActionResultFilterEndPoint();
-            actionResultFilters.Add(actionResultFilterEndPoint);
+            _actionResultFilters.Add(actionResultFilterEndPoint);
 
-            var actionResultPipeline = new FilterPipelineBuilder<ActionResultFilterContext>(actionResultFilters, actionResultFilterContext);
+            var actionResultPipeline = new FilterPipelineBuilder<ActionResultFilterContext>(_actionResultFilters, actionResultFilterContext);
 
             await actionResultPipeline.InvokeAsync();
         }
@@ -153,25 +151,41 @@ namespace Microsoft.AspNet.Mvc
             return parameterValues;
         }
 
-        private object[] GetArgumentValues(IDictionary<string, object> parameterValues)
+        private void PreArrangeFiltersInPipeline(FilterProviderContext context)
         {
-            var parameters = _descriptor.MethodInfo.GetParameters();
+            if (context.OrderedFilterList == null || context.OrderedFilterList.Count == 0)
-            var arguments = new object[parameters.Length];
-
-            for (int i = 0; i < arguments.Length; i++)
             {
-                object value;
+                return;
-                if (parameterValues.TryGetValue(parameters[i].Name, out value))
-                {
-                    arguments[i] = value;
-                }
-                else
-                {
-                    arguments[i] = parameters[i].DefaultValue;
-                }
             }
 
-            return arguments;
+            foreach (var filter in context.OrderedFilterList)
+            {
+                PlaceFilter(filter);
+            }
+        }
+
+        private void PlaceFilter(object filter)
+        {
+            var authFilter = filter as IAuthorizationFilter;
+            var actionFilter = filter as IActionFilter;
+            var actionResultFilter = filter as IActionResultFilter;
+
+            // TODO: Exception filters
+
+            if (authFilter != null)
+            {
+                _authorizationFilters.Add(authFilter);
+            }
+
+            if (actionFilter != null)
+            {
+                _actionFilters.Add(actionFilter);
+            }
+
+            if (actionResultFilter != null)
+            {
+                _actionResultFilters.Add(actionResultFilter);
+            }
         }
     }
 }

src/Microsoft.AspNet.Mvc.Core/ReflectedActionInvokerProvider.cs

@@ -41,8 +41,7 @@ namespace Microsoft.AspNet.Mvc
                                     _actionResultFactory,
                                     _controllerFactory,
                                     _bindingProvider,
-                                    _filterProvider,
+                                    _filterProvider);
-                                    _serviceProvider);
             }
 
             callNext();