Add access to services for route handler filter factories to RouteHandlerContext

[DamianEdwards]

When adding a route handler filter via a filter factory there are cases where the filter factory (as opposed to the filter itself) needs access to the application services, e.g. to get an ILogger instance. We should add a property to RouteHandlerContext to enable this.

namespace Microsoft.AspNetCore.Http;

public sealed class RouteHandlerContext
{
+    public IServiceProvider ApplicationServices { get; }
}

Note that the property is nullable due to where it's instantiated in RequestDelegateFactory and matches the Services properties on EndpointMetadataContext and EndpointParameterMetadataContext.

Example Usage

public static RouteHandlerBuilder WithParameterValidation(this RouteHandlerBuilder builder, int statusCode = StatusCodes.Status400BadRequest)
{
    builder.Add(eb =>
    {
        var methodInfo = eb.Metadata.OfType<MethodInfo>().SingleOrDefault();
        if (methodInfo is not null && IsValidatable(methodInfo))
        {
            eb.Metadata.Add(new ProducesResponseTypeAttribute(typeof(HttpValidationProblemDetails), statusCode, "application/problem+json"));
        }
    });
    builder.AddFilter((RouteHandlerContext context, RouteHandlerFilterDelegate next) =>
    {
        // *************************************
        // ** We use ApplicationServices here **
        // *************************************
        var loggerFactory = context.ApplicationServices.GetRequiredService<ILoggerFactory>();
        var logger = loggerFactory.Create("MinimalApis.Extensions.Filters.ValidationRouteHandlerFilterFactory");
 
        if (!IsValidatable(context.MethodInfo))
        {
            return next;
        }
 
        return rhic =>
        {
            foreach (var parameter in rhic.Parameters)
            {
                if (parameter is not null && !MiniValidator.TryValidate(parameter, out var errors))
                {
                    return new(Results.ValidationProblem(errors));
                }
            }
 
            return next(rhic);
        };
    });

    return builder;
}

Risks

Should be very low. This is following the pattern used in other context classes including those where the call site originates in RequestDelegateFactory, e.g. EndpointMetadataContext.

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

In IApplicationBuilder, IConnectionBuilder and KestrelServerOptions we call this ApplicationServices to make it extra clear it's different from HttpContext.RequestServices. Should we do the same here?

[davidfowl]

Yep.

[halter73]

From our email thread,

This is following the pattern used in other context classes including those where the call site originates in RequestDelegateFactory, e.g. EndpointMetadataContext

EndpointMetadataContext.Services hasn't shipped in a major release yet. Should we make it EndpointMetadataContext.ApplicationServices?

namespace Microsoft.AspNetCore.Http;

public sealed class RouteHandlerContext
{
+    public IServiceProvider? ApplicationServices { get; }
}

namespace Microsoft.AspNetCore.Http.Metadata;

public sealed class EndpointMetadataContext
{
-    public IServiceProvider? Services { get; }
+    public IServiceProvider? ApplicationServices { get; }
}

[DamianEdwards]

EndpointMetadataContext.Services hasn't shipped in a major release yet. Should we make it EndpointMetadataContext.ApplicationServices?

Sounds fair to me.

[halter73]

From @davidfowl's email:

I don't think the property should be nullable.

That's a good point. At first, I thought you were suggesting we lie about the nullability, but it occurs to me that we can just provide an "empty" IServiceProvider if it would otherwise be null. We should do the same for the EndpointMetadataContext.

[DamianEdwards]

Agreed. Fixed in the PR.

[halter73]

Updated API proposal:

namespace Microsoft.AspNetCore.Http;

public sealed class RouteHandlerContext
{
-     public RouteHandlerContext(MethodInfo methodInfo, EndpointMetadataCollection endpointMetadata);
+     public RouteHandlerContext(MethodInfo methodInfo, EndpointMetadataCollection endpointMetadata, IServiceProvider applicationServices);

+     public IServiceProvider ApplicationServices { get; }
}

namespace Microsoft.AspNetCore.Http.Metadata;

public sealed class EndpointMetadataContext
{
-     public EndpointMetadataContext(MethodInfo method, IList<object> endpointMetadata, IServiceProvider? services);
+     public EndpointMetadataContext(MethodInfo method, IList<object> endpointMetadata, IServiceProvider applicationServices);

-     public IServiceProvider? Services { get; }
+     public IServiceProvider ApplicationServices { get; }
}

public sealed class EndpointParameterMetadataContext
{
-     public EndpointParameterMetadataContext(ParameterInfo parameter, IList<object> endpointMetadata, IServiceProvider? services);
+     public EndpointParameterMetadataContext(ParameterInfo parameter, IList<object> endpointMetadata, IServiceProvider applicationServices);

-     public IServiceProvider? Services { get; }
+     public IServiceProvider ApplicationServices { get; }
}

[halter73]

API Review Note:

• No further feedback. Approved!