BindAsync Surrogate Method in Minimal APIs

[commonsensesoftware]

Background and Motivation

Minimal APIs currently support a number of mechanisms for parameter binding, but are limited to a static TryParse or BindAsync method for custom binding.

Some types of parameter binding, such as features, cannot be achieved without resorting to IHttpContextAccessor. This is both clunky and a forceful hand by platform extenders for consuming developers. TryParse is insufficient because the process can be more involved than simple parsing and BindAsync is impractical for types that do not have affinity to HTTP and cannot implement IBindableFromHttpContext<TSelf>.

Consider the following scenario for a Minimal API in ASP.NET API Versioning:

var app = builder.Build();
var orders = app.NewVersionedApi();
var v1 = orders.MapGroup("/orders").HasApiVersion(1.0);

var.MapGet("/{id:int}", (int id, ApiVersion version) => new Order() { Id = id, Version = version.ToString() });

For the ApiVersion parameter to be bound (currently), the following DI workaround is required:

builder.Services.AddHttpContextAccessor();
builder.Services.AddTransient(sp => sp.GetRequiredService<IHttpContextAccessor>().HttpContext?.GetRequestedApiVersion()!);

While this is just one scenario, there are many others like it. This is particularly true for edge cases that continue to crop up in the ever expanding items outlined in the internal EndpointParameterSource enumeration.

Proposed API

The proposed API would expand upon the capabilities of the BindAsync conventions and rules by allowing a surrogate method. These surrogates would be defined and consumed through Options. A surrogate BindAsync method would have the following rules:

• Must be a static method (to avoid closure issues)
• Must use the same supported signatures as the BindAsync convention
• Must use the same supported IBindableFromHttpContext<TSelf>.BindAsync signature
• May allow an alternate to an existing BindAsync implementation

The options might be defined as follows:

public sealed class BindAsyncOptions : IReadOnlyDictionary<Type, MethodInfo>
{
    private Dictionary<Type, MethodInfo>? map;

    public void Add<T>(Func<HttpContext, ValueTask<T?>> bindAsync) =>
        Add(typeof(T), bindAsync.Method);

    public void Add<T>(Func<HttpContext, ParameterInfo, ValueTask<T?>> bindAsync) =>
        Add(typeof(T), bindAsync.Method);

    private void Add(Type type, MethodInfo method)
    {
        if (!method.IsStatic)
        {
            throw new ArgumentException("The specified method must be static.");
        }

        map ??= new();
        map.Add(type, method);
    }

    MethodInfo IReadOnlyDictionary<Type, MethodInfo>.this[Type key] =>
        map is null ? throw new KeyNotFoundException() : map[key];

    IEnumerable<Type> IReadOnlyDictionary<Type, MethodInfo>.Keys =>
        map is null ? Enumerable.Empty<Type>() : map.Keys;

    IEnumerable<MethodInfo> IReadOnlyDictionary<Type, MethodInfo>.Values =>
        map is null ? Enumerable.Empty<MethodInfo>() : map.Values;

    public int Count => map is null ? 0 : map.Count;

    public bool ContainsKey(Type key) => map is not null && map.ContainsKey(key);

    IEnumerator<KeyValuePair<Type, MethodInfo>> IEnumerable<KeyValuePair<Type, MethodInfo>>.GetEnumerator() =>
        map is null ? Enumerable.Empty<KeyValuePair<Type, MethodInfo>>().GetEnumerator() : map.GetEnumerator();

    public bool TryGetValue(Type key, [MaybeNullWhen(false)] out MethodInfo value)
    {
        if (map is null)
        {
            value = default;
            return false;
        }

        return map.TryGetValue(key, out value);
    }

    IEnumerator System.Collections.IEnumerable.GetEnumerator() =>
        map is null ? Enumerable.Empty<KeyValuePair<Type, MethodInfo>>().GetEnumerator() : map.GetEnumerator();
}

To leverage the configuration, the internal ParameterBindingMethodCache would have to be changed. This class is source-shared across multiple libraries so the change should only use types known to all implementations (which may otherwise look strange).

HasBindAsyncMethod needs a new overload that can resolve the surrogate mapping dictionary:

+    [RequiresUnreferencedCode("Performs reflection on type hierarchy. This cannot be statically analyzed.")]
+    [RequiresDynamicCode("Performs reflection on type hierarchy. This cannot be statically analyzed.")]
+    public bool HasBindAsyncMethod(
+        ParameterInfo parameter,
+        IServiceProvider serviceProvider,
+        Func<IServiceProvider, IReadOnlyDictionary<Type, MethodInfo>?> resolve) =>
+        FindBindAsyncMethod(parameter, serviceProvider, resolve).Expression is not null;

A new GetIBindableSurrogate method would be added to allow resolving BindAsync mapped to a specific type.

+    private static MethodInfo? GetIBindableSurrogate(
+        Type type,
+        IServiceProvider? serviceProvider,
+        Func<IServiceProvider, IReadOnlyDictionary<Type, MethodInfo>?>? resolve)
+    {
+        if (serviceProvider is not null &&
+            resolve is not null &&
+            resolve(serviceProvider) is { } map &&
+            map.TryGetValue(type, out var method))
+        {
+            return method;
+        }
+
+        return null;
+    }

The FindBindAsyncMethod

https://github.com/dotnet/aspnetcore/blob/28b2bfd3ac67f07a5985550f1bec2e659af02aea/src/Shared/ParameterBindingMethodCache.cs#L215

would be updated to allow:

+    var methodInfo = GetIBindableSurrogate(nonNullableParameterType, serviceProvider, resolve) ??
+                     GetIBindableFromHttpContextMethod(nonNullableParameterType);

The remaining rules and processing for BindAsync would remain unchanged and just work.

RequestDelegateFactory would subsequently be updated as follows:

+    private static IReadOnlyDictionary<Type, MethodInfo> ResolveBindAsyncSurrogates(IServiceProvider serviceProvider) =>
+        serviceProvider.GetService<IOptions<BindAsyncOptions>>()?.Value);

and then:

https://github.com/dotnet/aspnetcore/blob/28b2bfd3ac67f07a5985550f1bec2e659af02aea/src/Http/Http.Extensions/src/RequestDelegateFactory.cs#L831

becomes:

else if (ParameterBindingMethodCache.HasBindAsyncMethod(
         parameter,
+        factoryContext.ServiceProvider,
+        ResolveBindAsyncSurrogates))

and:

https://github.com/dotnet/aspnetcore/blob/28b2bfd3ac67f07a5985550f1bec2e659af02aea/src/Http/Http.Extensions/src/RequestDelegateFactory.cs#L1916

becomes:

    var bindAsyncMethod = ParameterBindingMethodCache.FindBindAsyncMethod(
            parameter,
+           factoryContext.ServiceProvider,
+           ResolveBindAsyncSurrogates);

Usage Examples

This will now allow developers and platform extenders to define arbitrary BindAsync methods mapped to a specific type.

In the original example, instead of using IHttpContextAccessor, API Versioning could now register:

private static ValueTask<ApiVersion?> BindApiVersionAsync(HttpContext context) =>
    context.ApiVersioningFeature().RequestedApiVersion;

services.Configure((BindAsyncOptions options) => options.Add(BindApiVersionAsync));

This approach would work for any other feature, type, and so on that doesn't have TryParse (or is unsuitable) and cannot implement IBindableFromHttpContext<TSelf>. Consumers of such mappings are none the wiser and do not implicitly have to take a dependency on IHttpContextAccessor (which should be avoided).

Alternative Designs

• BindAsyncOptions does not have to be IReadOnlyDictionary<Type, MethodInfo>; however,
  • most of the capabilities are dictionary-like
  • the instance would likely to need to expose or project into another dictionary instance
• BindAsyncOptions does not have to supersede existing implementations
• The surrogate method must be static, which means it must be a static function or local function
  • static lambdas are not guaranteed to be static functions by the compiler
  • It might be worth changing or loosening the rules to allow something like static (context) => ValueTask.FromResult(context.ApiVersioningFeature().RequestedApiVersion)

Open Questions

• Should BindAsyncOptions.Add replace an existing registration or are multiple registrations an error?
• Is source generation still possible for surrogate methods?
• It is not entirely clear how IServiceProvider is resolved from RequestDelegateFactoryOptions

Assuming it ties to the root container, resolving the options just works and the following unit test passes:

[Fact]
public async Task CanExecuteRequestDelegateWithBindAsyncSurrogate()
{
    // Arrange
    IResult actionWithBindAsyncSurrogate(FeatureValue value) => Results.Extensions.TestResult(value.IsTest.ToString());

    var httpContext = CreateHttpContext();
    var responseBodyStream = new MemoryStream();

    httpContext.Response.Body = responseBodyStream;
    httpContext.Features.Set(new TestFeature(new() { IsTest = true }));

    var services = new ServiceCollection();

    services.Configure((BindAsyncOptions options) => options.Add(BindAsync));

    var factoryResult = RequestDelegateFactory.Create(
        actionWithBindAsyncSurrogate,
        new RequestDelegateFactoryOptions() { ServiceProvider = services.BuildServiceProvider() });

    var requestDelegate = factoryResult.RequestDelegate;

    // Act
    await requestDelegate(httpContext);

    // Assert
    Assert.Equal(200, httpContext.Response.StatusCode);
    Assert.Equal(@"""Hello True. This is from an extension method.""", Encoding.UTF8.GetString(responseBodyStream.ToArray()));

    static ValueTask<FeatureValue?> BindAsync(HttpContext h, ParameterInfo c) =>
        ValueTask.FromResult(h.Features.Get<TestFeature>()?.Value);
}

Risks

• BindAsync implementations might collide (but there can still only be one)
• BindAsync processing might be unclear

[commonsensesoftware]

This is duplicate request of:

• 45525

• 35489

A key difference is this implementation variant could potentially address the Source Generation limitation.