Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for endpoint filters #37853

Closed
DamianEdwards opened this issue Oct 26, 2021 · 4 comments · Fixed by #40491
Closed

Add support for endpoint filters #37853

DamianEdwards opened this issue Oct 26, 2021 · 4 comments · Fixed by #40491
Assignees
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-minimal-actions Controller-like actions for endpoint routing Priority:1 Work that is critical for the release, but we could probably ship without triage-focus Add this label to flag the issue for focus at triage
Milestone

Comments

@DamianEdwards
Copy link
Member

DamianEdwards commented Oct 26, 2021

Filters in MVC allow for code to be executed immediately before and after the invocation of action methods, allowing for inspection and modification of input parameters before execution, or even completely short-circuiting the action invocation altogether, along with inspection and modification of action method results, or even completely replacing the result, before they are executed.

There is no equivalent of this today when composing request handlers using raw endpoints however, e.g. via app.MapGet, etc. This issue is proposing adding support for filters as a new intrinsic in the request pipeline. We could consider having MVC support running filters registered via this new mechanism in addition to its own filters feature.

There are two types of filter scenarios to consider:

  1. A "generic" filter that does not target an endpoint or route handler delegate of a particular type. This scenario would require access to the target parameters, etc. after they've been populated by whatever the default logic is before the endpoint/handler is executed, and access to the returned value or exception after the endpoint/hander is executed.
  2. A "specific" filter that explicitly targets an endpoint or route handler of a specific delegate type, e.g. Func<int, string, IResult>. This scenario would require access to the

Both scenarios involve each filter invocation getting passed the delegate representing the next filter in the pipeline, such that they can decide whether or not to invoke the next filter or not.

Generic filter example

var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

app.AddEndpointFilter(async (ctx, next) =>
{
    // Log out the parameters
    var logger = ctx.HttpContext.RequestServices.GetRequiredService<ILoggerFactory>()
        .CreateLogger("DemoFilter");

    foreach (var p in ctx.Parameters)
    {
        logger.LogDebug("Parameter name {ParameterName}", p.Name);
    }

    await next();

    // Log the return type
    logger.LogDebug("Instance of {ResultType} returned", ctx.Result?.GetType());
});

app.MapGet("/", () => "Hello World!");
app.MapGet("/{name}", (string name) => $"Hello {name}!");

app.Run();

WIP, more to come

Scratch

//  Defined on IEndpointRouteBuilder?
var filters = app.CreateFilterBuilder()
                 .AddFilter(async (FilterContext ctx, Func<Task<IResult>> next) =>
                 {
                    // Dynamic filter
                    foreach (RouteHandlerParameter p in ctx.Parameters)
                    {
                        if (!MiniValidation.TryValidate(p.Value, out var errors))
                        {
                            return Results.Problem(errors);
                        }
                    }
                    var result = await next();
                    // Do horribly slow thing with result

                    return result;
                 })
                 .Add((Func<IResult> next) =>
                 {
                    var result = next();
                    if (result is OkResult<string> ok)
                    {
                        return ok.Value + ": Hello from a filter!";
                    }
                    return result;
                 })
                 .Add((AppDb db, Func<string> next) =>
                 // This delegate accepts the same params as the its target
                 // *PLUS* the handler itself
                 {
                    var result = next();
                    return Results.Ok(result + ": Hello from a filter!");
                 });

var validationFilters = app.CreateFilterBuilder()
    .Add((AppDb db, HttpContext context, string (AppDb) next) => next() + ": Hello from a filter!")
    // This is an extension method added by a library that adds filters to this pipeline, in this case filters that 
    // perform validation. This filter pipeline can then be passed in to a RouteGroup or directly into any Map* call
    .Validate();

app.MapGet("/root", filters, () => "Hello from root");

// RouteGroup with no name or path, just used for grouping endpoints & applying middleware/filters/metadata to them
app.RouteGroup()
   .UseResponseCompression()
   .AddFilter((HttpContext context, Func<string> next) => next() + ": Filtered!")
   .MapGet("/root", () => "Hello from root");

// RouteGroup with a filter pipeline passed in
var rootGroup1 = app.RouteGroup(filters);
group.UseWhatever();
group.MapGet("/hello", () => "Hello");
group.MapGet("/bye", () => "Bye");

// RouteGroup with a filter pipeline and metadata that will apply to all endpoints
var rootGroup2 = app.RouteGroup(filters)
    .WithTags("group2");
group.MapGet("/hello2", () => "Hello");
group.MapGet("/bye2", () => "Bye");

// RouteGroup with a route prefix, group name, and filter pipeline, plus some metadata that's applied to all endpoints in the group
var fooGroup = app.RouteGroup(routePatternPrefix: "/foo", groupName: "FooGroup", filters)
    .WithTags("foo")
    .RequiresAuthentication();
group.MapGet("/hello", () => "Hello");
group.MapGet("/bye", () => "Bye");

// How can we weave in route matcher policy semantics to enable things like specifying
// a middleware that only runs for a specific Razor page or controller
@DamianEdwards DamianEdwards added feature-minimal-actions Controller-like actions for endpoint routing area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Oct 26, 2021
@DamianEdwards DamianEdwards added this to the .NET 7 Planning milestone Oct 26, 2021
@DamianEdwards DamianEdwards changed the title Consider adding support for endpoint filters (WIP) WIP: Consider adding support for endpoint filters Oct 27, 2021
@rafikiassumani-msft rafikiassumani-msft added triage-focus Add this label to flag the issue for focus at triage Priority:1 Work that is critical for the release, but we could probably ship without Needs: Design This issue requires design work before implementating. Cost:XL labels Jan 11, 2022
@captainsafia captainsafia removed the Needs: Design This issue requires design work before implementating. label Jan 27, 2022
@captainsafia captainsafia changed the title WIP: Consider adding support for endpoint filters Add support for endpoint filters Jan 27, 2022
@captainsafia
Copy link
Member

captainsafia commented Jan 31, 2022

We met last week to review a design for this last week. The text of the design is below. There's some open questions that we need to follow-up on but for now we are moving forward with implementation primarily in two phases:

  • Update code generation in RequestDelegateFactory to support invoking filters
  • Add new APIs for registering endpoints onto a filter

Endpoint Filters Design

Date: January 27, 2022

Summary

Filters are MVC construct that allow developers to. Unlike middlewares, filters run after the routing middleware and have access to additional data that middlewares don’t. While middlewares operate on the HttpContext, filters have access MVC-specific constructs (such as HttpContext, RotueData, ActionDescriptor, and ModelState) that they can operate on.

Filters, and particularly ActionFilters , are heavily tied to action and controllers concepts in MVC and there are no parallels that exist for endpoints. This design doc outlines an approach for adding support for filters on endpoints that are registered as route handlers. Some general notes about the approach outlined here:

  • It more closely aligns with the filter implementation in SignalR than the one in MVC for simplicity’s sake.
  • It attempts to outline the “minimum viable product for most valuable feedback” or the simplest implementation we can ship that will allow us to gather feedback and review the implementation.
  • The document outlines new APIs that need to be added. The API shape is there for illustrative purposes and not meant to be taken at face value.

Goals

  • Provide support for filters that give access to request and response data
  • Providing an “flexible” design that allows us to go in different directions in response to user feedback
  • Endpoints without filters registered should not encounter any negative side effects related to perf/etc

Non-goals

  • Implementing a default set of filters in the framework as part of this work
  • Providing first-class support for handler-specific filters

Risks/Unknowns

  • How will this API intersect with features like route groups?
  • What are the performance ramifications for this implementation?

Proposed Design Walkthrough

  1. The user implements UnsupportedContentTypeFilter by extending the IEndpointFilter interface.
public class UnsupportedContentTypeFilter : IEndpointFilter
{
	public override ValueTask<object> RunAsync(IEndpointFilterContext context, Func<IEndpointFilterContext, ValueTask<object?>> next)
	{
			// Assume last argument is some Boolean flag
			if (context.Parameters.Last())
			{
				var result = await next(context);
				if (result.ContentType != "application/json")
				{
					return new UnsupportedMediaTypeResult();
				}
				return result;
			}
	} 
}
  • Framework Code

    To support this feature, we’ll be adding two new interfaces for implementing a filter and accessing it’s context.

    public class EndpointFilterContext
    {
    	public HttpContext HttpContext { get; }
    	public IList<object?> Parameters { get; } // Not read-only to premit modifying of parameters by filters
    }
    
    public interface IEndpointFilter
    {
    	public abstract ValueTask<object?> RunAsync(IEndpointFilterContext context, Func<IEndpointFilterContext, ValueTask<object?>> next)
    
    	public ValueTask OnBeforeAsync()
    
    	public ValueTask OnAfterAsync()
    }

  1. A user register a filter on an endpoint by invoking their preferred AddFilter method on the RouteHandlerBuilder for the endpoint they want to target.
// Option 1
app.MapGet("/todos/{id}", (int id) => new Todo(id, $"Task {id}"))
	.AddFilter<UnsupportedContentTypeFilter>();

// Option 2
app
	.AddFilter<UnsupportedContentTypeFilter>()
	.MapGet("/todos/{id}", (int id) => new Todo(id, $"Task {id}"));

// Option 2.5
app.MapGet("/root", () => "Hello from root").AddFilter<UnsupportedContentTypeFilter>()
var rootGroup1 = app.RouteGroup().AddFilter<UnsupportedContentTypeFilter>;

// Option 3
app
	.MapGet(new UnsupportedContentTypeFilter(), "/todos/{id}", (int id) => new Todo(id, $"Task {id}"));
  • Framework Code

    We expose a collection of extension methods for registering an filter on a particular endpoint.

    public static class EndpointFilterExtensions
    {
    	public static RouteHandlerBuilder AddFilter<TFilterType>();
    	public static RouteHandlerBuilder AddFilter(Type filterType);
    	public static RouteHandlerBuilder AddFilter(IEndpointFilter filter);
    	public static RouteHandlerBuilder AddFilter(Func<IEndpointFilterContext, Func<IEndpointFilterContext, Task<object>>> filter);
    }

    Under the hood, the AddFilter methods will populate a new Filters property on the RouteHandlerBuilder.

    public sealed class RouteHandlerBuilder
    {
    	internal IEnumerable<IEndpointFilter> Filters { get; set; }
    }

    The RouteHandlerBuilder.Filters property is passed to the RequestDelegateFactory.Create call in the [RouteHandlerBuilder.Map](http://RouteHandlerBuilder.Map) method. To support adding the filter invocations before/after the handler invocation, we need to restructure the RequestDelegateFactory in the following ways:

    • Add a new RequestDelegateFactory.Create overload that takes a reference to a list of filters and returns an Expression.
    public static class RequestDelegateFactory
    {
    	internal static Expression Create(
    		Delegate handler,
    		RequestDelegateFactoryOptions options = null,
    		IEnumerable<IEndpointFilter> filters = new List<IEndpointFilter>());
    }
    • Add a new RequestDelegateFactory.Compile overload that has the same method signature but produces a compiled delegate.
    public static class RequestDelegateFactory
    {
    	internal static Func<object?, HttpContext, Task> Compile(Expression requestDelegateExpression);
    }
    • Add a new RequestDelegateExpression property to the RouteEndpointBuilder class which will be used to delay the compilation of the request delegate until Build is called.
    public sealed class RouteEndpointBuilder
    {
    	public Expression RequestDelegateExpression { get; set; }
    
    	public RouteEndpointBuilder(Expression requestDelegateExpression, RoutePattern routePattern, int order);
    }

    With these overloads in place, we will “delay” the compilation of the request delegate until the Endpoint is being constructed so that we have access to the filters added after the invocation of the MapAction methods.

    public static class EndpointRouteBuilderExtensions
    {
    	public static RouteHanlderBuilder Map(this IEndpointRouteBuilder endpoints)
    	{
    		var requestDelegateExpression = RequestDelegateFactory.Create(handler, options, endpoints.Filters);
    		var requestDelegate = RequestDelegateFactory.Create(handler, options, endpoints.Filters);
    	
    	  var builder = new RouteEndpointBuilder(
    	      requestDelegateExpression,
    	      pattern,
    	      defaultOrder)
    	  {
    	      DisplayName = pattern.RawText ?? pattern.DebuggerToString(),
    	  };
    
    		endpoints.Filters.Clear();
    	}
    }
    public sealed class RouteEndpointBuilder : EndpointBuilder
    {
    	public override Endpoint Build()
      {
          if (RequestDelegate is null && RequestExpression is null)
          {
              throw new InvalidOperationException($"{nameof(RequestDelegate)} or {nameof(RequestExpression)} must be specified to construct a {nameof(RouteEndpoint)}.");
          }
    
    			if (RequestExpression is not null && RequestDelegate is null)
    			{
    				var compiledDelegate = RequestDelegateFactory.Compile(RequestExpression);
    				RequestDelegate = compiledDelegate.RequestDelegate;
    			}
    
          var routeEndpoint = new RouteEndpoint(
              RequestDelegate,
              RoutePattern,
              Order,
              new EndpointMetadataCollection(Metadata),
              DisplayName);
    
          return routeEndpoint;
      }
    }

  1. The user sends a request to the endpoint with a filter registered.
$ http POST http://localhost:5000/todos/1
  • Framework Code

    When the user starts up the application, the RequestDelegate associated with their endpoint is not compiled. Instead, this happens lazily when they visit the route and the routing logic invokes the [EndpointBuilder.Build](http://EndpointBuilder.Build) logic which compiles the request delegate and produces an endpoint from it.

    The request delegate that is invoked consists of a “wrapped” delegate that invokes the registered filters in FILO order before processing the user’s handler.


RequestDelegateFactory Changes

The most consequential changes in this design will happen in the RequestDelegateFactory which will need to be modified to support:

  • Delaying the compilation of the complete expression until RequestDelegateFactory.Compile is invoked in [RouteEndpointBuilder.Build](http://RouteEndpointBuilder.Build) is invoked
  • Supporting generating an expression tree for invoking the filters in the appropriate order
  • Support for processing the arguments passed to the endpoint to both the filters and the core handler

Comments from the Call

  • Is it possible to add the Result to the HttpContext so that filters can function more like middleware?
  • Should the IEndpointFilterContext provide access to endpoint info like metadata, name, route template, etc?
    • As an alternative users can call GetEndpoint()
  • Should it be called IEndpointFilter.RunAsync or something else?
  • Should IEndpointFilter just be a delegate?
  • EndpointDecorators
  • How do we make sure that Validate scenarios work?

@davidfowl
Copy link
Member

davidfowl commented Jan 31, 2022

Some feedback

  • I think we should stick with the routehandler* naming instead of the end point naming. These filters don't run outside of route handler specific code.
  • I like InvokeAsync over RunAsync because its a filter around the method invocation.
  • OnBefore and OnAfter need parameters

@rynowak
Copy link
Member

rynowak commented Feb 19, 2022

A few thoughts...

What's the purpose of defining all of RunAsync, OnBeforeAsync, OnAfterAsync on the filter interface? It feels more like this should be a pure decorator (just RunAsync). If someone really likes the coding style of OnBeforeAsync/OnAfterAsync you can provide that via a base-class.

Defining OnAfterAsync in this way is really tricky because you have to consider the exception case. Is OnAfterAsync called when an exception is thrown from the innermost function? How do users handle exceptions here? Lots of people will use filters to define/handle their own kinds of exceptions.

Another thing that feels important based on MVC's scenarios is the ability to introspect the method being called and its parameter declarations. This comes up a lot when someone writes a generic reusable filter and distributes it or applies it globally. It's a good way to "build your own DSL" with attributes that can go on methods/parameters.

The ability to modify/decorate the result appears to be missing from this. Many results in MVC are mutable for this reason, not sure if that's true of minimal APIs.

I'm glad that this design only defines one kind of filter, and it's the most useful kind. Legacy MVC had too many kinds of specialized filters because it was designed for the pre-middleware world. We had to keep these in MVC core for compat reasons and that's the root cause of much of MVC's perf overhead. One of the valid cases that might come up is the "I want to run a filter before model binding occurs". This would require another kind of filter to implement, so when that ask comes up you should say no 😆. Middleware can already do this.

Also last minor nitpick: context.Parameters is a misleading name since it contains the function arguments. The parameters would be the declarations on the function being routed-to. In MVC we defined this as a dictionary in an effort to try and make it easier to use. I'm not sure that it was actually easy to use, but it definitely resulted in more runtime overhead and allocations that we were obligated to keep on the hot path.

@captainsafia
Copy link
Member

What's the purpose of defining all of RunAsync, OnBeforeAsync, OnAfterAsync on the filter interface? It feels more like this should be a pure decorator (just RunAsync). If someone really likes the coding style of OnBeforeAsync/OnAfterAsync you can provide that via a base-class.

We actually decided on just a RunAsync method for the interface for this reason. The notes admittedly don't do a good job of capturing this... :/

Another thing that feels important based on MVC's scenarios is the ability to introspect the method being called and its parameter declarations. This comes up a lot when someone writes a generic reusable filter and distributes it or applies it globally. It's a good way to "build your own DSL" with attributes that can go on methods/parameters.

Yeah, this proposal explicitly doesn't try to solve the global filters problem. For now, filters can be applied on routes (and in the future on route groups) but there is no scenario for global ones at the moment.

The ability to modify/decorate the result appears to be missing from this. Many results in MVC are mutable for this reason, not sure if that's true of minimal APIs.

We anticipate that users will be able to modify both the existing arguments (e.g. uppercase all the string parameters to a method) and modify the response (e.g. change the error message to address the type mentioned above). I suspect that for most global scenarios middlewares would be sufficient...

Also last minor nitpick: context.Parameters is a misleading name since it contains the function arguments.

Fair point. We can definitely adjust this in response to user feedback. I opted for "parameters" instead of "arguments" since that list semantically represents things like query parameters, route parameters, and body parameters to the route as opposed to the literal function arguments.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 3, 2022
@danroth27 danroth27 added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Apr 6, 2022
@amcasey amcasey added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-minimal-actions Controller-like actions for endpoint routing Priority:1 Work that is critical for the release, but we could probably ship without triage-focus Add this label to flag the issue for focus at triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants