BindAsync Surrogate Method in Minimal APIs
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 290
Description
## Background and Motivation
_Minimal APIs_ currently support a number of mechanisms for [parameter binding](https://learn.microsoft.com/en-us/aspnet/core/fundamentals/minimal-apis/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`.
Consider the following scenario for a _Minimal API_ in ASP.NET API Versioning:
```c#
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:
```c#
builder.Services.AddHttpContextAccessor();
builder.Services.AddTransient(sp => sp.GetRequiredService().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](../blob/main/src/Http/Http.Extensions/gen/StaticRouteHandlerModel/EndpointParameterSource.cs) 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.BindAsync` signature
- May allow an alternate to an existing `BindAsync` implementation
The options might be defined as follows:
```c#
public sealed class BindAsyncOptions : IReadOnlyDictionary
{
private Dictionary? map;
public void Add(Func> bindAsync) =>
Add(typeof(T), bindAsync.Method);
public void Add(Func> 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.this[Type key] =>
map is null ? throw new KeyNotFoundException() : map[key];
IEnumerable IReadOnlyDictionary.Keys =>
map is null ? Enumerable.Empty() : map.Keys;
IEnumerable IReadOnlyDictionary.Values =>
map is null ? Enumerable.Empty() : map.Values;
public int Count => map is null ? 0 : map.Count;
public bool ContainsKey(Type key) => map is not null && map.ContainsKey(key);
IEnumerator> IEnumerable>.GetEnumerator() =>
map is null ? Enumerable.Empty>().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>().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:
```diff
+ [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?> 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.
```diff
+ private static MethodInfo? GetIBindableSurrogate(
+ Type type,
+ IServiceProvider? serviceProvider,
+ Func?>? 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:
```diff
+ 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:
```diff
+ private static IReadOnlyDictionary ResolveBindAsyncSurrogates(IServiceProvider serviceProvider) =>
+ serviceProvider.GetService>()?.Value);
```
and then:
https://github.com/dotnet/aspnetcore/blob/28b2bfd3ac67f07a5985550f1bec2e659af02aea/src/Http/Http.Extensions/src/RequestDelegateFactory.cs#L831
becomes:
```diff
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:
```diff
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:
```c#
private static ValueTask 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`. 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`; 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:
```c#
[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 BindAsync(HttpContext h, ParameterInfo c) =>
ValueTask.FromResult(h.Features.Get()?.Value);
}
```
## Risks
- `BindAsync` implementations might collide (but there can still only be one)
- `BindAsync` processing might be unclear
Contributor guide
Assessment
This issue has not been assessed yet.