Support BindAsync Surrogates in Minimal APIs

[commonsensesoftware]

Summary

Support parameter binding using a surrogate type.

Motivation and goals

The existing parameter binding mechanisms are insufficient for all types of model binding. This is particularly painful for extension authors that previously relied on using IModelBinderProvider and IModelBinder.

As a developer, I might want:

var builder = WebApplication.CreateBuilder( args );

builder.Services.AddApiVersioning();

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

v1.MapGet("/", (ApiVersion version) => Results.Ok());

This will not work out-of-the-box. There is no way to resolve ApiVersion.

All of the current, supported strategies are insufficient for binding:

• The ApiVersion type cannot have a BindAsync method
  • There is no direct dependency on ASP.NET Core; it's just metadata
• TryParse:
  • Is delegated to IApiVersionParser so developers can change the parsing implementation
  • The value can come from multiple places and even simultaneously (ex: query string and header)
    • ASP.NET Core will never understand this behavior
• The value is resolved through a feature which is not supported
• Defining and using a surrogate type in a lambda is confusing to developers that are accustomed to the behavior from IModelBinder
  • If a developer changes the parsing behavior, they would have to provide their own, alternate surrogate as well

There are 3 possible workarounds today:

1. Use HttpContext as a parmaeter and retrieve the value via HttpContext.GetRequestedApiVersion()
   a. This isn't very minimal
2. Use an explicit stand-in type; this a lot of tedious ceremony and is unnatural for consuming developers
3. Use DI in an obtuse way
   a. services.AddTransient(sp => sp.GetRequiredService<IHttpContextAccessor>().HttpContext?.GetRequestedApiVersion()!
   b. This is currently supported by explicitly opting into services.EnableApiVersionBinding()

Proposal

The proposal would be to add a service that can accept a surrogate type that can perform the binding. This would allow library authors to provide a similar experience to IModelBinder without a developer having to do anything special. Surrogates would be registered through DI, which would enable anyone to add, remove, or replace surrogate types. Just as there is a 1-to-1 pairing of type to BindAsync or IBindableFromHttpContext<T>, so too, there can be exactly one corresponding surrogate type. A surrogate type will be considered as the last option before choosing IServiceProvider.GetService.

The proposed API would be (final names TDB):

public interface IBindFromFromHttpContextService
{
    bool TryGetSurrogate(Type targetType, [NotNullWhen(true)] out Type surrogateType);
}

public abstract class BindFromHttpContextSurrogate
{
    protected BindFromHttpContextSurrogate(Type targetType) => TargetType = targetType;
    public Type TargetType { get; }
}

The default implementation of the service would be:

internal sealed class DefaultBindFromFromHttpContextService : IBindFromFromHttpContextService
{
    private readonly Dictionary<Type, Type> _surrogates;

    public DefaultBindFromFromHttpContextService(
        IEnumerable<BindFromHttpContextSurrogate> surrogates) =>
        _surrogates => surrogates.ToDictionary(s => s.TargetType, s => s.GetType());

    public bool TryGetSurrogate(Type targetType, [NotNullWhen(true)] out Type surrogateType) =>
        _surrogates.TryGetValue(targetType, out surrogateType);
}

A surrogate type falls under the same rules for BindAsync custom binding albeit on an alternate type.

An example implementation would be:

public sealed class ApiVersionBinder : BindFromHttpContextSurrogate
{
    public ApiVersionBinder() : base(typeof(ApiVersion)) { }

    public static ValueTask<ApiVersion> BindAsync(HttpContext context, ParameterInfo parameter)
    {
        var feature = context.Features.GetRequiredFeature<IApiVersioningFeature>();
        return ValueTask.FromResult(feature.RequestedApiVersion!);
    }
}

The surrogate can be registered in DI with:

services.TryAddEnumerable(ServiceDescriptor.Transient<BindFromHttpContextSurrogate, ApiVersionBinder>());

This is a generic approach can used to delegate any implementation of BindAsync to an alternate type. Other frameworks, such as OData, could support Minimal APIs using this same mechanism.

Risks / unknowns

None that are immediately evident. The only thing changing in runtime behavior is which type to look for BindAsync on.

[commonsensesoftware]

cc: @halter73, @davidfowl

[davidfowl]

This is basically a dupe of #35489. We're currently exploring writing a source generator for minimal APIs (see https://github.com/davidfowl/uController), so anything we design we'd want to make sure keeps working for that as well.

[commonsensesoftware]

@davidfowl Doh! GitHub failed me. I knew I remembered this conversation. I spent 15-20 mins trying to find it, but I couldn't find the thread.

Would this proposal still apply? Is there a working API - yet? The short, short version of this proposal is being able to provide a way to say ApiVersionBinder.BindAsync as a surrogate for ApiVersion.BindAsync. This will have the exact same mechanics as BindAsync today, but on a different type. I would expect that to be fully inline with the forthcoming code generation. If we're already marching toward a better way, I'm happy to close this out. The correlation between issues is there for historical purposes.

[commonsensesoftware]

Based on the example source generator example, I see this fitting in at around line 463 with:

private bool HasBindAsync(Type type, out MethodInfo mi, out int parameterCount)
{
    // TODO: Validate return type

    mi = GetStaticMethodFromHierarchy(type, "BindAsync", new[] { _wellKnownTypes.HttpContextType }, m => true);

    mi ??= GetStaticMethodFromHierarchy(type, "BindAsync", new[] { _wellKnownTypes.HttpContextType, _wellKnownTypes.ParamterInfoType }, m => true);

+    if (mi == null && bindFromHttpContextService.TryGetSurrogate(type, out var surrogateType))
+    {
+        return HasBindAsync(surrogateType, out mi, out parameterCount);
+    }

    parameterCount = mi?.GetParameters().Length ?? 0;

    return mi != null;
}

[davidfowl]

I don't think it's that trivial. How do we resolve the bindFromHttpContextService in a source generator? That's why I said its doable but not as trivial as what you have above. The reason BindAsync works is because you can statically determine the BindAsync-ness from the parameter type. The same isn't true here. The reason I don't like this feature all up (and why we haven't done it yet) is that it makes it impossible to statically determine if a parameter has a surrogate. That makes the static code generation less efficient as it needs to generate code that runs at runtime to determine what the parameter is.

The codegen runtime code that looks like:

// Fallback case
// DI 
// Surrogate
// Body

if (serviceProviderIsService.IsService(parameter.ParameterType))
{
    // It's a service
}
else if (serviceProviderIsService.IsService(typeof(ParameterBinder<>).MakeGenericType(parameter.ParameterType)))
{
   // There's a parameter binder here
}
else
{
   // Assume frombody
}

I'd much prefer an attribute that points to a type so that this feature would be more pay for play and statically analyzable.

[commonsensesoftware]

True. I wasn't thinking about zero access at design-time.

You almost had me stumped. I definitely had to rethink the problem, but I think the same goal can be achieved by just ditching all the runtime aspects. What if a surrogate could be provided to the source code generator in a statically determined way without any DI or attributes?

Let the sorcery ensue 🧙🏽‍♂️ !

Source code generation is all about design-time. So we need to hook into a design-time process that the source code generator can pick up. What if we add...

<ItemGroup>
    <AdditionalFiles Include="Asp.Versioning.ApiVersion"
                     SurrogateType="Asp.Versioning.Http.ApiVersionBinder"
                     MinimalApi="true" />
</ItemGroup>

Then any library author can register a surrogate at design-time and have it picked up through the standard NuGet build extensions. Developers still have the ability to remove or update the mapping.

This is a bit of contrived way of using AdditionalFiles, but it would pass the necessary metadata without using any actual files. A quick breakdown:

• Include would be qualified type name to be parsed. This must exactly match the ParameterInfo type
• SurrogateType would the qualified type name used for source code generation
  • It must resolve by the generator via the target assembly or its dependencies (which I believe is possible)
• MinimalApi not strictly necessary, it may be worth considering (or some other format) to avoid possible ambiguity

This information can be retrieved and used with something like:

private static Dictionary<Type, Type> MapBindAsyncSurrogateTypes(SourceGeneratorContext context)
{
    const string Prefix = "build_metadata.additionalfiles.";
    const string MinimalApi = Prefix + nameof(MinimalApi);
    const string SurrogateType = Prefix + nameof(SurrogateType);

    var map = default(Dictionary<Type, Type);

    foreach (var file in context.AdditionalFiles)
    {
        var options = context.AnalyzerConfigOptions.GetOptions(file);

        if (!options.TryGetValue(MinimalApi, out var value) ||
            !bool.TryParse(value, out minApi) ||
            !minApi)
        {
            continue;
        }

        if (Type.GetType(file.Path, throwOnError: false) is not { } targetType)
        {
            continue;
        }

        if (options.TryGetValue(SurrogateType, out value) &&
            Type.GetType(value, throwOnError: false) is { } surrogateType)
        {
            map ??= new();
            map[targetType] = surrogateType;
        }



    return map ?? new(capacity: 0);            
}

The previous example will likely initialize this somewhere earlier (just once) and pass it in. Something like:

private bool HasBindAsync(
    Type type,
+    Dictionary<Type, Type> surrogateTypeMap,
    out MethodInfo mi,
    out int parameterCount)
{
    // TODO: Validate return type

    mi = GetStaticMethodFromHierarchy(type, "BindAsync", new[] { _wellKnownTypes.HttpContextType }, m => true);

    mi ??= GetStaticMethodFromHierarchy(type, "BindAsync", new[] { _wellKnownTypes.HttpContextType, _wellKnownTypes.ParamterInfoType }, m => true);

+    if (mi == null && surrogateTypeMap.TryGetValue(type, out var surrogateType))
+    {
+       mi = GetStaticMethodFromHierarchy(surrogateType, "BindAsync", new[] { _wellKnownTypes.HttpContextType }, m => true);
+       mi ??= GetStaticMethodFromHierarchy(surrogateType, "BindAsync", new[] { _wellKnownTypes.HttpContextType, _wellKnownTypes.ParamterInfoType }, m => true);
+    }

    parameterCount = mi?.GetParameters().Length ?? 0;

    return mi != null;
}

Thoughts?

[davidfowl]

Or an attribute 😄. That would work at both runtime and statically. I know it makes the minimal API noisier and you don't want to couple this type to the type you don't own. I can think of a couple of ways to do this association:

Technique	Can be determined statically	Can be determined at runtime
Attribute on the parameter	y	y
Attribute on the type	y	y
Generic wrapper type	y	y
Attribute on the method	y	y
Attribute on the assembly (which assembly?)	y	y
Store the association in the DI Container	n	y
Provide this association by some other non-code means	y	?

The most direct associations are the first 2, put an attribute on the type or put it on the parameter. The big downside of the attribute is that the owner of the library can't make the type available for binding automagically as part of their call to add their services (in this case AddApiVersioning).

I would like the parameter defined to somehow hint to the system that it needs this behavior, or that this parameter is indeed special. When the type has no way to know it's being bound.

Maybe this just adds a little overhead to the pure fallback case so using the DI container is the right thing to do here. That code I wrote initially would look like this in the generated code for the source generator:

if (serviceProviderIsService.IsService(typeof(ApiVersion))
{
    // It's a service
}
else if (serviceProviderIsService.IsService(typeof(IParameterBinder<ApiVersion>)))
{
   // There's a parameter binder here
}
else
{
   // Assume frombody
}

PS: This is the language feature that is trying to formalize these surrogate types (dotnet/csharplang#5496). I'm working with the team closely to see if we can solve scenarios like this generally with this language feature. It still has the "problem" of having to create a new type though.

[davidfowl]

I'm gonna close this as a dupe

[davidfowl]

Let's continue the discussion on that original issue with the DI based idea.