Added support for type based parameter binding (#35496)

* Added support for type based parameter binding
- Added a convention that allows custom async binding logic to run for parameters that have a static BindAsync method that takes an HttpContext and return a ValueTask of object. This allows customers to write custom binders based solely on type (it's an extension of the existing TryParse pattern).
- There's allocation overhead per request once there's a parameter binder for a delegate. This is because we need to box all of the arguments since we're not using generated code to compute data from the list of binders.
- Changed TryParse tests to BindAsync tests and added more tests.

Author: davidfowl

src/Http/Http.Extensions/src/RequestDelegateFactory.cs

@@ -41,12 +41,11 @@ public static partial class RequestDelegateFactory
             Log.ParameterBindingFailed(httpContext, parameterType, parameterName, sourceValue));
         private static readonly MethodInfo LogRequiredParameterNotProvidedMethod = GetMethodInfo<Action<HttpContext, string, string>>((httpContext, parameterType, parameterName) =>
             Log.RequiredParameterNotProvided(httpContext, parameterType, parameterName));
-        private static readonly MethodInfo LogParameterBindingFromHttpContextFailedMethod = GetMethodInfo<Action<HttpContext, string, string>>((httpContext, parameterType, parameterName) =>
-            Log.ParameterBindingFromHttpContextFailed(httpContext, parameterType, parameterName));
 
         private static readonly ParameterExpression TargetExpr = Expression.Parameter(typeof(object), "target");
         private static readonly ParameterExpression BodyValueExpr = Expression.Parameter(typeof(object), "bodyValue");
         private static readonly ParameterExpression WasParamCheckFailureExpr = Expression.Variable(typeof(bool), "wasParamCheckFailure");
+        private static readonly ParameterExpression BoundValuesArrayExpr = Expression.Parameter(typeof(object[]), "boundValues");
 
         private static ParameterExpression HttpContextExpr => TryParseMethodCache.HttpContextExpr;
         private static readonly MemberExpression RequestServicesExpr = Expression.Property(HttpContextExpr, typeof(HttpContext).GetProperty(nameof(HttpContext.RequestServices))!);
@@ -198,7 +197,6 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory
             {
                 var errorMessage = BuildErrorMessageForMultipleBodyParameters(factoryContext);
                 throw new InvalidOperationException(errorMessage);
-
             }
 
             return args;
@@ -263,9 +261,9 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext
             {
                 return RequestAbortedExpr;
             }
-            else if (TryParseMethodCache.HasTryParseHttpContextMethod(parameter))
+            else if (TryParseMethodCache.HasBindAsyncMethod(parameter))
             {
-                return BindParameterFromTryParseHttpContext(parameter, factoryContext);
+                return BindParameterFromBindAsync(parameter, factoryContext);
             }
             else if (parameter.ParameterType == typeof(string) || TryParseMethodCache.HasTryParseStringMethod(parameter))
             {
@@ -275,7 +273,6 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext
                 // when RDF.Create is manually invoked.
                 if (factoryContext.RouteParameters is { } routeParams)
                 {
-                   
                     if (routeParams.Contains(parameter.Name, StringComparer.OrdinalIgnoreCase))
                     {
                         // We're in the fallback case and we have a parameter and route parameter match so don't fallback
@@ -361,7 +358,6 @@ private static Expression CreateParamCheckingResponseWritingMethodCall(
             var localVariables = new ParameterExpression[factoryContext.ExtraLocals.Count + 1];
             var checkParamAndCallMethod = new Expression[factoryContext.ParamCheckExpressions.Count + 1];
 
-
             for (var i = 0; i < factoryContext.ExtraLocals.Count; i++)
             {
                 localVariables[i] = factoryContext.ExtraLocals[i];
@@ -508,14 +504,33 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
         {
             if (factoryContext.JsonRequestBodyType is null)
             {
+                if (factoryContext.ParameterBinders.Count > 0)
+                {
+                    // We need to generate the code for reading from the custom binders calling into the delegate
+                    var continuation = Expression.Lambda<Func<object?, HttpContext, object?[], Task>>(
+                        responseWritingMethodCall, TargetExpr, HttpContextExpr, BoundValuesArrayExpr).Compile();
+
+                    // Looping over arrays is faster
+                    var binders = factoryContext.ParameterBinders.ToArray();
+                    var count = binders.Length;
+
+                    return async (target, httpContext) =>
+                    {
+                        var boundValues = new object?[count];
+
+                        for (var i = 0; i < count; i++)
+                        {
+                            boundValues[i] = await binders[i](httpContext);
+                        }
+
+                        await continuation(target, httpContext, boundValues);
+                    };
+                }
+
                 return Expression.Lambda<Func<object?, HttpContext, Task>>(
                     responseWritingMethodCall, TargetExpr, HttpContextExpr).Compile();
             }
 
-            // We need to generate the code for reading from the body before calling into the delegate
-            var invoker = Expression.Lambda<Func<object?, HttpContext, object?, Task>>(
-                responseWritingMethodCall, TargetExpr, HttpContextExpr, BodyValueExpr).Compile();
-
             var bodyType = factoryContext.JsonRequestBodyType;
             object? defaultBodyValue = null;
 
@@ -524,31 +539,82 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
                 defaultBodyValue = Activator.CreateInstance(bodyType);
             }
 
-            return async (target, httpContext) =>
+            if (factoryContext.ParameterBinders.Count > 0)
             {
-                object? bodyValue = defaultBodyValue;
-                var feature = httpContext.Features.Get<IHttpRequestBodyDetectionFeature>();
-                if (feature?.CanHaveBody == true)
+                // We need to generate the code for reading from the body before calling into the delegate
+                var continuation = Expression.Lambda<Func<object?, HttpContext, object?, object?[], Task>>(
+                responseWritingMethodCall, TargetExpr, HttpContextExpr, BodyValueExpr, BoundValuesArrayExpr).Compile();
+
+                // Looping over arrays is faster
+                var binders = factoryContext.ParameterBinders.ToArray();
+                var count = binders.Length;
+
+                return async (target, httpContext) =>
                 {
-                    try
+                    // Run these first so that they can potentially read and rewind the body
+                    var boundValues = new object?[count];
+
+                    for (var i = 0; i < count; i++)
                     {
-                        bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType);
+                        boundValues[i] = await binders[i](httpContext);
                     }
-                    catch (IOException ex)
+
+                    var bodyValue = defaultBodyValue;
+                    var feature = httpContext.Features.Get<IHttpRequestBodyDetectionFeature>();
+                    if (feature?.CanHaveBody == true)
                     {
-                        Log.RequestBodyIOException(httpContext, ex);
-                        return;
+                        try
+                        {
+                            bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType);
+                        }
+                        catch (IOException ex)
+                        {
+                            Log.RequestBodyIOException(httpContext, ex);
+                            return;
+                        }
+                        catch (InvalidDataException ex)
+                        {
+                            Log.RequestBodyInvalidDataException(httpContext, ex);
+                            httpContext.Response.StatusCode = 400;
+                            return;
+                        }
                     }
-                    catch (InvalidDataException ex)
-                    {
 
-                        Log.RequestBodyInvalidDataException(httpContext, ex);
-                        httpContext.Response.StatusCode = 400;
-                        return;
+                    await continuation(target, httpContext, bodyValue, boundValues);
+                };
+            }
+            else
+            {
+                // We need to generate the code for reading from the body before calling into the delegate
+                var continuation = Expression.Lambda<Func<object?, HttpContext, object?, Task>>(
+                responseWritingMethodCall, TargetExpr, HttpContextExpr, BodyValueExpr).Compile();
+
+                return async (target, httpContext) =>
+                {
+                    var bodyValue = defaultBodyValue;
+                    var feature = httpContext.Features.Get<IHttpRequestBodyDetectionFeature>();
+                    if (feature?.CanHaveBody == true)
+                    {
+                        try
+                        {
+                            bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType);
+                        }
+                        catch (IOException ex)
+                        {
+                            Log.RequestBodyIOException(httpContext, ex);
+                            return;
+                        }
+                        catch (InvalidDataException ex)
+                        {
+
+                            Log.RequestBodyInvalidDataException(httpContext, ex);
+                            httpContext.Response.StatusCode = 400;
+                            return;
+                        }
                     }
-                }
-                await invoker(target, httpContext, bodyValue);
-            };
+                    await continuation(target, httpContext, bodyValue);
+                };
+            }
         }
 
         private static Expression GetValueFromProperty(Expression sourceExpression, string key)
@@ -747,40 +813,40 @@ private static Expression BindParameterFromRouteValueOrQueryString(ParameterInfo
             return BindParameterFromValue(parameter, Expression.Coalesce(routeValue, queryValue), factoryContext);
         }
 
-        private static Expression BindParameterFromTryParseHttpContext(ParameterInfo parameter, FactoryContext factoryContext)
+        private static Expression BindParameterFromBindAsync(ParameterInfo parameter, FactoryContext factoryContext)
         {
-            // bool wasParamCheckFailure = false;
-            //
-            // // Assume "Foo param1" is the first parameter and "public static bool TryParse(HttpContext context, out Foo foo)" exists.
-            // Foo param1_local;
-            //
-            // if (!Foo.TryParse(httpContext, out param1_local))
-            // {
-            //     wasParamCheckFailure = true;
-            //     Log.ParameterBindingFromHttpContextFailed(httpContext, "Foo", "foo")
-            // }
-
-            var argument = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local");
-            var tryParseMethodCall = TryParseMethodCache.FindTryParseHttpContextMethod(parameter.ParameterType);
+            // We reference the boundValues array by parameter index here
+            var nullability = NullabilityContext.Create(parameter);
+            var isOptional = parameter.HasDefaultValue || nullability.ReadState == NullabilityState.Nullable;
 
-            // There's no way to opt-in to using a TryParse method on HttpContext other than defining the method, so it's guaranteed to exist here.
-            Debug.Assert(tryParseMethodCall is not null);
+            // Get the BindAsync method
+            var body = TryParseMethodCache.FindBindAsyncMethod(parameter.ParameterType)!;
 
-            var parameterTypeNameConstant = Expression.Constant(parameter.ParameterType.Name);
-            var parameterNameConstant = Expression.Constant(parameter.Name);
+            // Compile the delegate to the BindAsync method for this parameter index
+            var bindAsyncDelegate = Expression.Lambda<Func<HttpContext, ValueTask<object?>>>(body, HttpContextExpr).Compile();
+            factoryContext.ParameterBinders.Add(bindAsyncDelegate);
 
-            var failBlock = Expression.Block(
-                Expression.Assign(WasParamCheckFailureExpr, Expression.Constant(true)),
-                Expression.Call(LogParameterBindingFromHttpContextFailedMethod,
-                    HttpContextExpr, parameterTypeNameConstant, parameterNameConstant));
+            // boundValues[index]
+            var boundValueExpr = Expression.ArrayIndex(BoundValuesArrayExpr, Expression.Constant(factoryContext.ParameterBinders.Count - 1));
 
-            var tryParseCall = tryParseMethodCall(argument);
-            var fullParamCheckBlock = Expression.IfThen(Expression.Not(tryParseCall), failBlock);
+            if (!isOptional)
+            {
+                var checkRequiredBodyBlock = Expression.Block(
+                        Expression.IfThen(
+                        Expression.Equal(boundValueExpr, Expression.Constant(null)),
+                            Expression.Block(
+                                Expression.Assign(WasParamCheckFailureExpr, Expression.Constant(true)),
+                                Expression.Call(LogRequiredParameterNotProvidedMethod,
+                                        HttpContextExpr, Expression.Constant(parameter.ParameterType.Name), Expression.Constant(parameter.Name))
+                            )
+                        )
+                    );
 
-            factoryContext.ExtraLocals.Add(argument);
-            factoryContext.ParamCheckExpressions.Add(fullParamCheckBlock);
+                factoryContext.ParamCheckExpressions.Add(checkRequiredBodyBlock);
+            }
 
-            return argument;
+            // (ParamterType)boundValues[i]
+            return Expression.Convert(boundValueExpr, parameter.ParameterType);
         }
 
         private static Expression BindParameterFromBody(ParameterInfo parameter, bool allowEmpty, FactoryContext factoryContext)
@@ -793,7 +859,6 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al
                 {
                     factoryContext.TrackedParameters.Remove(parameterName);
                     factoryContext.TrackedParameters.Add(parameterName, "UNKNOWN");
-
                 }
             }
 
@@ -925,7 +990,6 @@ static async Task ExecuteAwaited(Task<T> task, HttpContext httpContext)
 
         private static Task ExecuteTaskOfString(Task<string?> task, HttpContext httpContext)
         {
-
             SetPlaintextContentType(httpContext);
             EnsureRequestTaskNotNull(task);
 
@@ -1032,6 +1096,7 @@ private class FactoryContext
             public bool UsingTempSourceString { get; set; }
             public List<ParameterExpression> ExtraLocals { get; } = new();
             public List<Expression> ParamCheckExpressions { get; } = new();
+            public List<Func<HttpContext, ValueTask<object?>>> ParameterBinders { get; } = new();
 
             public Dictionary<string, string> TrackedParameters { get; } = new();
             public bool HasMultipleBodyParameters { get; set; }