Skip to content

Best practice for a singleton route handler filter with Minimal APIs? #41259

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

Open
1 task done
martincostello opened this issue Apr 19, 2022 · 6 comments
Open
1 task done
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc
Milestone

Comments

@martincostello
Copy link
Member

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

The new support for route handler filters for Minimal APIs currently supports these use cases:

  1. Use an IRouteHandlerFilter instance specified when the route is mapped.
  2. Create a new instance of type T that implements IRouteHandlerFilter per HTTP request and invoke it. The filter is created with access to the IServiceProvider so can use types registered with IServiceCollection, but the filter itself is not resolved from the DI container.
  3. Two variants of invoking an anonymous filter that is the filter with RouteHandlerContext and RouteHandlerFilterDelegate.

In the case that a filter can logically be re-used between multiple endpoints as a singleton but cannot be specified directly when mapping the request delegates (item 1), what is the recommended best practice here with Minimal APIs in ASP.NET Core 7?

An application could do this, but it seems a bit verbose to re-use the same filter instance compared to option 2, except that creates a new instance of the filter for each invocation.

var builder = WebApplication.CreateBuilder(args);

// Register a filter and its dependency as services
builder.Services.AddSingleton<MySingleton>();
builder.Services.AddSingleton<MyFilter>();

var app = builder.Build();

// Filter method that resolves the singleton filter from DI and invokes it
ValueTask<object?> MyFilter(RouteHandlerInvocationContext context, RouteHandlerFilterDelegate next)
{
    var filter = context.HttpContext.RequestServices.GetRequiredService<MyFilter>();
    return filter.InvokeAsync(context, next);
}

// Multiple arbitrary endpoints that use the same filter
app.MapGet("/200", () => Results.Ok())
   .AddFilter(MyFilter);

app.MapGet("/500", () => Results.Problem())
   .AddFilter(MyFilter);

app.MapGet("/404", () => Results.NotFound())
   .AddFilter(MyFilter);

app.Run();

// Arbitrary filter implementation that uses a singleton to achieve something
// and can be a singleton in of itself as it has no per-request state so can be re-used
public class MyFilter : IRouteHandlerFilter
{
    public MyFilter(MySingleton singleton)
    {
        Singleton = singleton;
    }

    private MySingleton Singleton { get; }

    public async ValueTask<object?> InvokeAsync(RouteHandlerInvocationContext context, RouteHandlerFilterDelegate next)
    {
        await Singleton.DoSomething();
        return await next(context);
    }
}

// Arbitrary singleton service that achieves some aim
public class MySingleton
{
    public Task DoSomething() => Task.CompletedTask;
}

Describe the solution you'd like

What's the recommendation on how to author the application for this use-case where minimising the creation of IRouteHandlerFilter instances is desired but it cannot be created up-front when registering the routes?

  • Is there a missing extension method to register a filter that's DI aware, rather than being activated per-request?
  • Is this a use case that will be less verbose to achieve using route groups (Improve minimal api routing with grouping #36007) in later previews?

I thought I'd raise the question about this, as it seems to be a bit of an outlier compared to the rest of Minimal APIs where trying to use a service will get it from DI, otherwise try and parse/bind it or deserialize it from the request body. The AddFilter<T>() method seems to be halfway in between, being able to leverage DI without being in DI itself.

Additional context

No response

@davidfowl davidfowl added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Apr 19, 2022
@rafikiassumani-msft rafikiassumani-msft added this to the .NET 7 Planning milestone Apr 19, 2022
@ghost
Copy link

ghost commented Apr 19, 2022

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@captainsafia
Copy link
Member

Some of what you're describing here feels like the "global filters" concept that we discussed during the design of this feature. At the time, we concluded that we didn't want to build a design that explicitly favored global filters and relegated route groups as the many approach for solving "apply the same filter to multiple routes." After all, it would stand to reason that if you wanted to apply the same filter to multiple routes then they can be conceptually placed in a group.

@davidfowl
Copy link
Member

Is there a missing extension method to register a filter that's DI aware, rather than being activated per-request?

We could make a tweak to AddFilter<T> to detect filters added as services and avoiding the creation of them via ActivatorUtilities. The problem is these filters wont be able to access the RouteHandlerContext.

Is this a use case that will be less verbose to achieve using route groups (#36007) in later previews?

Yes

@captainsafia
Copy link
Member

@martincostello Have you had the chance to use groups + filters in the latest preview7 release? Do you think that's a good solution to the problem you're having?

@martincostello
Copy link
Member Author

Yes, I had a quick play around with them.

In this sample app I added route groups to apply the common metadata for all the endpoints, including applying the previously repeated filter: martincostello/polly-rate-limiting@cdc74e4

I think for what I've done with it, it solves the duplication, thought I still feel it's a bit of an outlier compared to the rest of Minimal APIs that it's not resolved from DI as a service if registered as one.

That's not necessarily an issue with the route groups as it ultimately solves the use case, it just feels a bit different/surprising compared to the overall Developer Experience for using/consuming services with 6.0.

@ghost
Copy link

ghost commented Oct 11, 2022

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@captainsafia captainsafia added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Jun 14, 2023
@captainsafia captainsafia modified the milestones: .NET 8 Planning, Backlog Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc
Projects
No open projects
Status: No status
Development

No branches or pull requests

4 participants