Skip to content

Commit 73339fd

Browse files
committed
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.
1 parent 064497e commit 73339fd

6 files changed

+616
-35
lines changed

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

Lines changed: 134 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ public static partial class RequestDelegateFactory
4545
private static readonly ParameterExpression HttpContextExpr = Expression.Parameter(typeof(HttpContext), "httpContext");
4646
private static readonly ParameterExpression BodyValueExpr = Expression.Parameter(typeof(object), "bodyValue");
4747
private static readonly ParameterExpression WasParamCheckFailureExpr = Expression.Variable(typeof(bool), "wasParamCheckFailure");
48+
private static readonly ParameterExpression BoundValuesArrayExpr = Expression.Parameter(typeof(object[]), "boundValues");
4849

4950
private static readonly MemberExpression RequestServicesExpr = Expression.Property(HttpContextExpr, typeof(HttpContext).GetProperty(nameof(HttpContext.RequestServices))!);
5051
private static readonly MemberExpression HttpRequestExpr = Expression.Property(HttpContextExpr, typeof(HttpContext).GetProperty(nameof(HttpContext.Request))!);
@@ -194,7 +195,6 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory
194195
{
195196
var errorMessage = BuildErrorMessageForMultipleBodyParameters(factoryContext);
196197
throw new InvalidOperationException(errorMessage);
197-
198198
}
199199

200200
return args;
@@ -259,15 +259,18 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext
259259
{
260260
return RequestAbortedExpr;
261261
}
262-
else if (parameter.ParameterType == typeof(string) || TryParseMethodCache.HasTryParseMethod(parameter))
262+
else if (TryParseMethodCache.HasBindAsyncMethod(parameter))
263+
{
264+
return BindParameterFromBindAsync(parameter, factoryContext);
265+
}
266+
else if (parameter.ParameterType == typeof(string) || TryParseMethodCache.HasTryParseStringMethod(parameter))
263267
{
264268
// 1. We bind from route values only, if route parameters are non-null and the parameter name is in that set.
265269
// 2. We bind from query only, if route parameters are non-null and the parameter name is NOT in that set.
266270
// 3. Otherwise, we fallback to route or query if route parameters is null (it means we don't know what route parameters are defined). This case only happens
267271
// when RDF.Create is manually invoked.
268272
if (factoryContext.RouteParameters is { } routeParams)
269273
{
270-
271274
if (routeParams.Contains(parameter.Name, StringComparer.OrdinalIgnoreCase))
272275
{
273276
// We're in the fallback case and we have a parameter and route parameter match so don't fallback
@@ -353,7 +356,6 @@ private static Expression CreateParamCheckingResponseWritingMethodCall(
353356
var localVariables = new ParameterExpression[factoryContext.ExtraLocals.Count + 1];
354357
var checkParamAndCallMethod = new Expression[factoryContext.ParamCheckExpressions.Count + 1];
355358

356-
357359
for (var i = 0; i < factoryContext.ExtraLocals.Count; i++)
358360
{
359361
localVariables[i] = factoryContext.ExtraLocals[i];
@@ -500,14 +502,33 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
500502
{
501503
if (factoryContext.JsonRequestBodyType is null)
502504
{
505+
if (factoryContext.ParameterBinders.Count > 0)
506+
{
507+
// We need to generate the code for reading from the custom binders calling into the delegate
508+
var continuation = Expression.Lambda<Func<object?, HttpContext, object?[], Task>>(
509+
responseWritingMethodCall, TargetExpr, HttpContextExpr, BoundValuesArrayExpr).Compile();
510+
511+
// Looping over arrays is faster
512+
var binders = factoryContext.ParameterBinders.ToArray();
513+
var count = binders.Length;
514+
515+
return async (target, httpContext) =>
516+
{
517+
var boundValues = new object?[count];
518+
519+
for (var i = 0; i < count; i++)
520+
{
521+
boundValues[i] = await binders[i](httpContext);
522+
}
523+
524+
await continuation(target, httpContext, boundValues);
525+
};
526+
}
527+
503528
return Expression.Lambda<Func<object?, HttpContext, Task>>(
504529
responseWritingMethodCall, TargetExpr, HttpContextExpr).Compile();
505530
}
506531

507-
// We need to generate the code for reading from the body before calling into the delegate
508-
var invoker = Expression.Lambda<Func<object?, HttpContext, object?, Task>>(
509-
responseWritingMethodCall, TargetExpr, HttpContextExpr, BodyValueExpr).Compile();
510-
511532
var bodyType = factoryContext.JsonRequestBodyType;
512533
object? defaultBodyValue = null;
513534

@@ -516,31 +537,82 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
516537
defaultBodyValue = Activator.CreateInstance(bodyType);
517538
}
518539

519-
return async (target, httpContext) =>
540+
if (factoryContext.ParameterBinders.Count > 0)
520541
{
521-
object? bodyValue = defaultBodyValue;
522-
var feature = httpContext.Features.Get<IHttpRequestBodyDetectionFeature>();
523-
if (feature?.CanHaveBody == true)
542+
// We need to generate the code for reading from the body before calling into the delegate
543+
var continuation = Expression.Lambda<Func<object?, HttpContext, object?, object?[], Task>>(
544+
responseWritingMethodCall, TargetExpr, HttpContextExpr, BodyValueExpr, BoundValuesArrayExpr).Compile();
545+
546+
// Looping over arrays is faster
547+
var binders = factoryContext.ParameterBinders.ToArray();
548+
var count = binders.Length;
549+
550+
return async (target, httpContext) =>
524551
{
525-
try
552+
// Run these first so that they can potentially read and rewind the body
553+
var boundValues = new object?[count];
554+
555+
for (var i = 0; i < count; i++)
526556
{
527-
bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType);
557+
boundValues[i] = await binders[i](httpContext);
528558
}
529-
catch (IOException ex)
559+
560+
var bodyValue = defaultBodyValue;
561+
var feature = httpContext.Features.Get<IHttpRequestBodyDetectionFeature>();
562+
if (feature?.CanHaveBody == true)
530563
{
531-
Log.RequestBodyIOException(httpContext, ex);
532-
return;
564+
try
565+
{
566+
bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType);
567+
}
568+
catch (IOException ex)
569+
{
570+
Log.RequestBodyIOException(httpContext, ex);
571+
return;
572+
}
573+
catch (InvalidDataException ex)
574+
{
575+
Log.RequestBodyInvalidDataException(httpContext, ex);
576+
httpContext.Response.StatusCode = 400;
577+
return;
578+
}
533579
}
534-
catch (InvalidDataException ex)
535-
{
536580

537-
Log.RequestBodyInvalidDataException(httpContext, ex);
538-
httpContext.Response.StatusCode = 400;
539-
return;
581+
await continuation(target, httpContext, bodyValue, boundValues);
582+
};
583+
}
584+
else
585+
{
586+
// We need to generate the code for reading from the body before calling into the delegate
587+
var continuation = Expression.Lambda<Func<object?, HttpContext, object?, Task>>(
588+
responseWritingMethodCall, TargetExpr, HttpContextExpr, BodyValueExpr).Compile();
589+
590+
return async (target, httpContext) =>
591+
{
592+
var bodyValue = defaultBodyValue;
593+
var feature = httpContext.Features.Get<IHttpRequestBodyDetectionFeature>();
594+
if (feature?.CanHaveBody == true)
595+
{
596+
try
597+
{
598+
bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType);
599+
}
600+
catch (IOException ex)
601+
{
602+
Log.RequestBodyIOException(httpContext, ex);
603+
return;
604+
}
605+
catch (InvalidDataException ex)
606+
{
607+
608+
Log.RequestBodyInvalidDataException(httpContext, ex);
609+
httpContext.Response.StatusCode = 400;
610+
return;
611+
}
540612
}
541-
}
542-
await invoker(target, httpContext, bodyValue);
543-
};
613+
await continuation(target, httpContext, bodyValue);
614+
};
615+
}
544616
}
545617

546618
private static Expression GetValueFromProperty(Expression sourceExpression, string key)
@@ -739,6 +811,42 @@ private static Expression BindParameterFromRouteValueOrQueryString(ParameterInfo
739811
return BindParameterFromValue(parameter, Expression.Coalesce(routeValue, queryValue), factoryContext);
740812
}
741813

814+
private static Expression BindParameterFromBindAsync(ParameterInfo parameter, FactoryContext factoryContext)
815+
{
816+
// We reference the boundValues array by parameter index here
817+
var nullability = NullabilityContext.Create(parameter);
818+
var isOptional = parameter.HasDefaultValue || nullability.ReadState == NullabilityState.Nullable;
819+
820+
// Get the BindAsync method
821+
var body = TryParseMethodCache.FindBindAsyncMethod(parameter.ParameterType)!;
822+
823+
// Compile the delegate to the BindAsync method for this parameter index
824+
var bindAsyncDelegate = Expression.Lambda<Func<HttpContext, ValueTask<object?>>>(body, HttpContextExpr).Compile();
825+
factoryContext.ParameterBinders.Add(bindAsyncDelegate);
826+
827+
// boundValues[index]
828+
var boundValueExpr = Expression.ArrayIndex(BoundValuesArrayExpr, Expression.Constant(factoryContext.ParameterBinders.Count - 1));
829+
830+
if (!isOptional)
831+
{
832+
var checkRequiredBodyBlock = Expression.Block(
833+
Expression.IfThen(
834+
Expression.Equal(boundValueExpr, Expression.Constant(null)),
835+
Expression.Block(
836+
Expression.Assign(WasParamCheckFailureExpr, Expression.Constant(true)),
837+
Expression.Call(LogRequiredParameterNotProvidedMethod,
838+
HttpContextExpr, Expression.Constant(parameter.ParameterType.Name), Expression.Constant(parameter.Name))
839+
)
840+
)
841+
);
842+
843+
factoryContext.ParamCheckExpressions.Add(checkRequiredBodyBlock);
844+
}
845+
846+
// (ParamterType)boundValues[i]
847+
return Expression.Convert(boundValueExpr, parameter.ParameterType);
848+
}
849+
742850
private static Expression BindParameterFromBody(ParameterInfo parameter, bool allowEmpty, FactoryContext factoryContext)
743851
{
744852
if (factoryContext.JsonRequestBodyType is not null)
@@ -749,7 +857,6 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al
749857
{
750858
factoryContext.TrackedParameters.Remove(parameterName);
751859
factoryContext.TrackedParameters.Add(parameterName, "UNKNOWN");
752-
753860
}
754861
}
755862

@@ -881,7 +988,6 @@ static async Task ExecuteAwaited(Task<T> task, HttpContext httpContext)
881988

882989
private static Task ExecuteTaskOfString(Task<string?> task, HttpContext httpContext)
883990
{
884-
885991
SetPlaintextContentType(httpContext);
886992
EnsureRequestTaskNotNull(task);
887993

@@ -988,6 +1094,7 @@ private class FactoryContext
9881094
public bool UsingTempSourceString { get; set; }
9891095
public List<ParameterExpression> ExtraLocals { get; } = new();
9901096
public List<Expression> ParamCheckExpressions { get; } = new();
1097+
public List<Func<HttpContext, ValueTask<object?>>> ParameterBinders { get; } = new();
9911098

9921099
public Dictionary<string, string> TrackedParameters { get; } = new();
9931100
public bool HasMultipleBodyParameters { get; set; }

0 commit comments

Comments
 (0)