filters rewrite #2398

tdykstra · 2016-12-15T22:16:25Z

Staged at https://review.docs.microsoft.com/aspnet/core/mvc/controllers/filters?branch=filters

Rick-Anderson · 2016-12-16T19:22:27Z

aspnetcore/mvc/controllers/filters.md

@@ -5,7 +5,7 @@ description:
 keywords: ASP.NET Core,
 ms.author: riande


Rick-Anderson · 2016-12-16T19:23:55Z

aspnetcore/mvc/controllers/filters.md

@@ -5,7 +5,7 @@ description:
 keywords: ASP.NET Core,
 ms.author: riande
 manager: wpickett


needs meta data
description:
keywords: ASP.NET Core,

Rick-Anderson · 2016-12-16T19:25:29Z

aspnetcore/mvc/controllers/filters.md

@@ -14,68 +14,64 @@ uid: mvc/controllers/filters
 ---
 # Filters

-By [Steve Smith](http://ardalis.com)
+By [Steve Smith](http://ardalis.com) and [Tom Dykstra](https://github.com/tdykstra/) 


By Tom and Steve - change order

Rick-Anderson · 2016-12-16T19:35:48Z

aspnetcore/mvc/controllers/filters/sample/src/FiltersSample/Startup.cs

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddMvc(options =>
            {
+                options.Filters.Add(new AddHeaderAttribute("GlobalAddHeader", "Result filter added to MvcOptions.Filters"));


newline on "Result filter added to MvcOptions.Filters")); to get rid of scroll bar

Rick-Anderson · 2016-12-16T19:37:12Z

aspnetcore/mvc/controllers/filters.md


-In the following code, both the `ShortCircuitingResourceFilter` and the `AddHeader` filter target the `SomeResource` action method. However, because the `ShortCircuitingResourceFilter` runs first and short-circuits the rest of the pipeline, the `AddHeader` filter never runs for the `SomeResource` action. This behavior would be the same if both filters were applied at the action method level, provided the `ShortCircuitingResourceFilter` ran first (see [Ordering](xref:mvc/controllers/filters#ordering)).
+Filters can be added by type or by instance. If you add an instance, that instance will be used for every request. If you add a type, it will be type-activated, meaning an instance will be created for each request and any constructor dependencies will be populated by DI. Adding a filter by type is equivalent to `filters.Add(new TypeFilterAttribute(typeof(MyFilter)))`.


populated by DI.

Either spell out dependency injection make a link to the article.

Rick-Anderson · 2016-12-17T01:20:17Z

aspnetcore/mvc/controllers/filters.md


-You can override the built-in `ResultFilterAttribute` to create result filters. The [AddHeaderAttribute](xref:mvc/controllers/filters#add-header-attribute) class shown above is an example of a result filter.
+You can override the built-in `ResultFilterAttribute` to create result filters. The [AddHeaderAttribute](#add-header-attribute) class shown earlier is an example of a result filter.

 >[!TIP]
 > If you need to add headers to the response, do so before the action result executes. Otherwise, the response may have been sent to the client, and it will be too late to modify it. For a result filter, this means adding the header in `OnResultExecuting` rather than `OnResultExecuted`.


Not sure you need this as you state it above. If you keep it.
~~For a result filter, this means adding the header in OnResultExecuting rather than OnResultExecuted.~~

It's useful information, worth keeping

Rick-Anderson · 2016-12-17T01:20:52Z

aspnetcore/mvc/controllers/filters.md

-## Filters vs. Middleware
+## Using Middleware as Filters
+
+Filters work like [middleware](../../fundamentals/middleware.md), but the filter pipeline is part of MVC, which means that filters have access to MVC context and constructs. For instance, a filter can easily detect whether model validation on a request has generated errors. 


~~easily~~ banned word

Rick-Anderson · 2016-12-17T01:22:49Z

aspnetcore/mvc/controllers/filters.md

-## Filters vs. Middleware
+## Using Middleware as Filters
+
+Filters work like [middleware](../../fundamentals/middleware.md), but the filter pipeline is part of MVC, which means that filters have access to MVC context and constructs. For instance, a filter can easily detect whether model validation on a request has generated errors. 


too long - maybe split as
Filters work like middleware. The filter pipeline is part of MVC, which means that filters have access to MVC context and constructs.

Rick-Anderson · 2016-12-17T01:26:35Z

aspnetcore/mvc/controllers/filters/sample/src/FiltersSample/Filters/LocalizationPipeline.cs

+                SupportedCultures = supportedCultures,
+                SupportedUICultures = supportedCultures
+            };
+            options.RequestCultureProviders = new[] { new RouteDataRequestCultureProvider() { Options = options } };


2 lines to get rid of horiz scroll bar

Rick-Anderson · 2016-12-17T01:27:35Z

aspnetcore/mvc/controllers/filters/sample/src/FiltersSample/Controllers/HomeController.cs

+        public IActionResult CultureFromRouteData()
+        {
+
+            return Content($"CurrentCulture:{CultureInfo.CurrentCulture.Name},CurrentUICulture:{CultureInfo.CurrentUICulture.Name}");


two lines to get rid of horiz scrool bar

rynowak · 2016-12-27T16:20:19Z

aspnetcore/mvc/controllers/filters.md


-*Filters* in ASP.NET MVC allow you to run code before or after a particular stage in the execution pipeline. Filters can be configured globally, per-controller, or per-action.
+*Filters* in ASP.NET Core MVC allow you to run code before or after certain stages in the request processing pipeline. Built-in filters handle tasks such as authorization (preventing access to resources a user isn't authorized for), ensuring that all requests use HTTPS, and response caching (short-circuiting the request pipeline to return a cached response). You can create custom filters for work specific to your application, such as error handling.



We should mention cross-cutting-concerns here instead of just saying 'work'. Filters are the solution for anything that you don't want to duplicate across actions.

rynowak · 2016-12-27T16:21:43Z

aspnetcore/mvc/controllers/filters.md


-### Selecting a Filter
+* [Resource filters](#resource-filters) are the first to handle a request after authorization.  They can run code before the rest of the filter pipeline, and after the rest of the pipeline has completed. They're useful to implement caching or otherwise short-circuit the filter pipeline for performance reasons.


We should mention here that resource filters also run before model binding. So anything you wanted to do to influence MB before it happens should occur here.

rynowak · 2016-12-27T16:25:24Z

aspnetcore/mvc/controllers/filters.md


-[Resource filters](xref:mvc/controllers/filters#resource-filters) are the first filter to handle a request after authorization, and the last one to touch the request as it is leaving the filter pipeline. They're especially useful to implement caching or otherwise short-circuit the filter pipeline for performance reasons.
+* [Exception filters](#exception-filters) come after Action filters in the pipeline and are used to apply global policies to unhandled exceptions that have occurred up to that point. 


come after Action filters in the pipeline - this is a little tricky and why I didn't try to include this information in the summary when I wrote this.

Exception filters surround action filters and model binding. and will run in general when an exception occurs before writing to the response body - which is why they don't surround execution of an action result.

@rynowak

Exception filters surround action filters and model binding.

What do you mean by "surround" (it doesn't look like that in the diagram)?

Also, you don't mention Resource filters, but the diagram implies that an Exception filter would run for an exception thrown by OnResourceExecuting -- is that not true?

Given the complexity, would this be close enough for the summary, leaving details for the Exception Filters section later in the article:

Exception filters are used to apply global policies to unhandled exceptions that occur before anything has been written to the response body.

rynowak · 2016-12-27T16:27:00Z

aspnetcore/mvc/controllers/filters.md


 ### Implementation

-All filters support both synchronous and asynchronous implementations through different interface definitions. Choose the sync or async variant depending on the kind of task you need to perform. They are interchangeable from the framework's perspective.


Choose the sync or async variant depending on the kind of task you need to perform. They are interchangeable from the framework's perspective.

I'd really prefer to keep something like this in the doc and even make it more prominent. We continue to get questions about this from users who are convinced that they must implement both.

@rynowak

I'd really prefer to keep something like this in the doc and even make it more prominent.

I added this sentence to the intro paragraph, but the warning is already there, formatted as a Note following the examples.

rynowak · 2016-12-27T16:29:36Z

aspnetcore/mvc/controllers/filters.md


-<a name=short-circuiting-resource-filter></a>
+A filter can be added to the pipeline at one of three *scopes*. You can add a filter to a particular action method or to a controller class by using an attribute. Or you can register a filter globally by adding it to the `MvcOptions.Filters` collection in the `ConfigureServices` method in the `Startup` class:


It's not really suitable to give this information without explaining how scopes determine order of execution.

rynowak · 2016-12-27T16:40:22Z

aspnetcore/mvc/controllers/filters.md


-8. The Controller `OnActionExecuted`
+In this example, `OnResourceExecuting` checks if the result is already in the static dictionary cache. If so, it sets the `Result`, which short-circuits the rest of the pipeline. In the `OnResourceExecuted` method, if the current request's key isn't already in use, the current `Result` is stored in the cache.


I'm not a huge fan of this example given that we now provide a cache filter and middleware. It's not just that the example isn't production-worthy, this is not something a user should ever write.

And sorry I don't have a more constructive example we can use here, we've implemented all of the useful things. If you're not averse to linking to another sample (or including it) https://github.com/aspnet/Entropy/blob/dev/samples/Mvc.FileUpload/Filters/DisableFormValueModelBindingAttribute.cs

This is a filter that will prevent model binding from accessing the form data. It's useful for cases where you know that you're going to receive large file uploads and want to prevent the form from being read into memory.

rynowak · 2016-12-27T16:46:58Z

aspnetcore/mvc/controllers/filters.md


->[!TIP]
-> One example where you might need a different form of error handling for different actions would be in an app that exposes both API endpoints and actions that return views/HTML. The API endpoints could return error information as JSON, while the view-based actions could return an error page as HTML.
+Result filters are only executed for successful results - when the action or action filters produce an action result. Result filters are not executed when exception filters handle an exception, unless the exception filter sets `Exception = null`.


Result filters are not executed when exception filters handle an exception, unless the exception filter sets Exception = null.

This is incorrect. Result filters are never executed when an exception filter runs.

The special case is that an action filter can turn an exception into a 'success'. An exception filter cannot.

@rynowak

Given that

... an action filter can turn an exception into a 'success'. An exception filter cannot.

And considering the confusion around the unresolved issue aspnet/Mvc#5594 -- what is the intended usage/purpose of the ExceptionHandled flag in ExceptionContext?

rynowak · 2016-12-27T16:48:41Z

aspnetcore/mvc/controllers/filters.md


-Result filters are ideal for any logic that needs to directly surround view execution or formatter execution. Result filters can replace or modify the action result that's responsible for producing the response.
+Filters work like [middleware](../../fundamentals/middleware.md), but the filter pipeline is part of MVC, which means that filters have access to MVC context and constructs. For instance, a filter can detect whether model validation on a request has generated errors. 


Filters work like [middleware]

This is really only true of resource filters. The other stages are not a good conceptual match. Resource filters surround execution of everything that comes later in the pipeline, which is why they are isomorphic with middleware.

For instance, a filter can detect whether model validation on a request has generated errors.

This is great example of something that users frequently do with action filters, but it's not related to middleware

rynowak · 2016-12-27T16:50:24Z

aspnetcore/mvc/controllers/filters.md


-## Filters vs. Middleware
+Middleware filters run at the same stage of the filter pipeline as Resource filters, before and after model binding and the rest of the pipeline.


before and after model binding and the rest of the pipeline.

before model binding and after the rest of the pipeline.

rynowak · 2016-12-27T16:51:30Z

aspnetcore/mvc/controllers/filters/sample/src/FiltersSample/Filters/LoggingAddHeaderFilter.cs

-            _logger.LogInformation($"Header added: {headerName}");
-
+            // Can't add to headers because response has already begun.
+            _logger.LogInformation("OnResultExecuted");


Please don't show code like this, the framework already logs this kind of information.

dnfclas added the cla-already-signed label Dec 15, 2016

tdykstra requested review from Rick-Anderson, rynowak and danroth27 December 16, 2016 18:02

Rick-Anderson requested changes Dec 17, 2016

View reviewed changes

tdykstra force-pushed the filters branch from 77b952a to 63a8cce Compare December 19, 2016 17:03

tdykstra closed this Dec 20, 2016

tdykstra reopened this Dec 20, 2016

dnfclas added the cla-already-signed label Dec 20, 2016

filters rewrite

d11e7a2

tdykstra force-pushed the filters branch from 909f5be to d11e7a2 Compare December 20, 2016 01:51

Merge branch 'master' into filters

d2c8573

rynowak requested changes Dec 27, 2016

View reviewed changes

tdykstra added 2 commits December 28, 2016 13:22

@rynowak feedback

c328c26

@rynowak feedback b

083fdd6

Rick-Anderson approved these changes Jan 3, 2017

View reviewed changes

exception filter text updates

65c9f4a

tdykstra merged commit 62a1da5 into master Jan 5, 2017

tdykstra deleted the filters branch January 5, 2017 03:34

tdykstra mentioned this pull request Jan 10, 2017

1.1 feature: Middleware as MVC filters #2305

Closed

tdykstra mentioned this pull request Mar 10, 2017

mvc/controllers/filters.html "Filters" #1267

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

filters rewrite #2398

filters rewrite #2398

tdykstra commented Dec 15, 2016 •

edited

Loading

Rick-Anderson Dec 16, 2016

Rick-Anderson Dec 16, 2016

Rick-Anderson Dec 16, 2016

Rick-Anderson Dec 16, 2016

Rick-Anderson Dec 16, 2016

Rick-Anderson Dec 17, 2016

tdykstra Dec 19, 2016

Rick-Anderson Dec 17, 2016

Rick-Anderson Dec 17, 2016

Rick-Anderson Dec 17, 2016

Rick-Anderson Dec 17, 2016

rynowak Dec 27, 2016

rynowak Dec 27, 2016

rynowak Dec 27, 2016

tdykstra Dec 27, 2016 •

edited

Loading

rynowak Dec 27, 2016

tdykstra Dec 27, 2016

rynowak Dec 27, 2016

rynowak Dec 27, 2016

rynowak Dec 27, 2016

rynowak Dec 27, 2016

tdykstra Dec 30, 2016

rynowak Dec 27, 2016

rynowak Dec 27, 2016

rynowak Dec 27, 2016

rynowak Dec 27, 2016

		@@ -5,7 +5,7 @@ description:
		keywords: ASP.NET Core,
		ms.author: riande


		In the following code, both the `ShortCircuitingResourceFilter` and the `AddHeader` filter target the `SomeResource` action method. However, because the `ShortCircuitingResourceFilter` runs first and short-circuits the rest of the pipeline, the `AddHeader` filter never runs for the `SomeResource` action. This behavior would be the same if both filters were applied at the action method level, provided the `ShortCircuitingResourceFilter` ran first (see [Ordering](xref:mvc/controllers/filters#ordering)).
		Filters can be added by type or by instance. If you add an instance, that instance will be used for every request. If you add a type, it will be type-activated, meaning an instance will be created for each request and any constructor dependencies will be populated by DI. Adding a filter by type is equivalent to `filters.Add(new TypeFilterAttribute(typeof(MyFilter)))`.


		Filters in ASP.NET MVC allow you to run code before or after a particular stage in the execution pipeline. Filters can be configured globally, per-controller, or per-action.
		Filters in ASP.NET Core MVC allow you to run code before or after certain stages in the request processing pipeline. Built-in filters handle tasks such as authorization (preventing access to resources a user isn't authorized for), ensuring that all requests use HTTPS, and response caching (short-circuiting the request pipeline to return a cached response). You can create custom filters for work specific to your application, such as error handling.


		### Selecting a Filter
		* [Resource filters](#resource-filters) are the first to handle a request after authorization. They can run code before the rest of the filter pipeline, and after the rest of the pipeline has completed. They're useful to implement caching or otherwise short-circuit the filter pipeline for performance reasons.


		[Resource filters](xref:mvc/controllers/filters#resource-filters) are the first filter to handle a request after authorization, and the last one to touch the request as it is leaving the filter pipeline. They're especially useful to implement caching or otherwise short-circuit the filter pipeline for performance reasons.
		* [Exception filters](#exception-filters) come after Action filters in the pipeline and are used to apply global policies to unhandled exceptions that have occurred up to that point.


		### Implementation

		All filters support both synchronous and asynchronous implementations through different interface definitions. Choose the sync or async variant depending on the kind of task you need to perform. They are interchangeable from the framework's perspective.


		<a name=short-circuiting-resource-filter></a>
		A filter can be added to the pipeline at one of three scopes. You can add a filter to a particular action method or to a controller class by using an attribute. Or you can register a filter globally by adding it to the `MvcOptions.Filters` collection in the `ConfigureServices` method in the `Startup` class:


		8. The Controller `OnActionExecuted`
		In this example, `OnResourceExecuting` checks if the result is already in the static dictionary cache. If so, it sets the `Result`, which short-circuits the rest of the pipeline. In the `OnResourceExecuted` method, if the current request's key isn't already in use, the current `Result` is stored in the cache.


		Result filters are ideal for any logic that needs to directly surround view execution or formatter execution. Result filters can replace or modify the action result that's responsible for producing the response.
		Filters work like [middleware](../../fundamentals/middleware.md), but the filter pipeline is part of MVC, which means that filters have access to MVC context and constructs. For instance, a filter can detect whether model validation on a request has generated errors.


		## Filters vs. Middleware
		Middleware filters run at the same stage of the filter pipeline as Resource filters, before and after model binding and the rest of the pipeline.

filters rewrite #2398

filters rewrite #2398

Conversation

tdykstra commented Dec 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tdykstra Dec 27, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tdykstra commented Dec 15, 2016 •

edited

Loading

tdykstra Dec 27, 2016 •

edited

Loading