Skip to content

Commit 25e98ba

Browse files
committed
Add tempSourceString to avoid reevaluation
1 parent 1aabf29 commit 25e98ba

File tree

1 file changed

+71
-57
lines changed

1 file changed

+71
-57
lines changed

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

Lines changed: 71 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public static class RequestDelegateFactory
4040
private static readonly ParameterExpression HttpContextExpr = Expression.Parameter(typeof(HttpContext), "httpContext");
4141
private static readonly ParameterExpression BodyValueExpr = Expression.Parameter(typeof(object), "bodyValue");
4242
private static readonly ParameterExpression WasTryParseFailureExpr = Expression.Variable(typeof(bool), "wasTryParseFailure");
43+
private static readonly ParameterExpression TempSourceStringExpr = Expression.Variable(typeof(string), "tempSourceString");
4344

4445
private static readonly MemberExpression RequestServicesExpr = Expression.Property(HttpContextExpr, nameof(HttpContext.RequestServices));
4546
private static readonly MemberExpression HttpRequestExpr = Expression.Property(HttpContextExpr, nameof(HttpContext.Request));
@@ -277,23 +278,30 @@ private static Expression CreateResponseWritingMethodCall(MethodInfo methodInfo,
277278
// If we're calling TryParse and the WasTryParseFailureVariable indicates it failed, set a 400 StatusCode instead of calling the method.
278279
private static Expression CreateTryParseCheckingResponseWritingMethodCall(MethodInfo methodInfo, Expression? target, Expression[] arguments)
279280
{
281+
280282
// {
281-
// bool wasTryParseFailure = false;
283+
// bool wasTryParseFailureVariable = false;
284+
// string tempSourceString;
282285
//
283286
// // Assume "[FromRoute] int id" is the first parameter.
284-
// int param1 = httpContext.Request.RouteValue["id"] == null ? default :
287+
//
288+
// tempSourceString = httpContext.RequestValue["id"];
289+
//
290+
// int param1 = tempSourceString == null ? default :
285291
// {
286-
// var sourceValue = httpContext.Request.RouteValue["id"];
287292
// int parsedValue = default;
288293
//
289-
// if (!int.TryParse(sourceValue, out parsedValue))
294+
// if (!int.TryParse(tempSourceString, out parsedValue))
290295
// {
291-
// wasTryParseFailure = true;
296+
// wasTryParseFailureVariable = true;
292297
// Log.ParameterBindingFailed(httpContext, "Int32", "id", sourceValue)
293298
// }
294299
//
295300
// return parsedValue;
296301
// };
302+
//
303+
// tempSourceString = httpContext.RequestValue["param2"];
304+
// int param2 = tempSourceString == null ? default :
297305
// // ...
298306
//
299307
// return wasTryParseFailure ?
@@ -302,20 +310,21 @@ private static Expression CreateTryParseCheckingResponseWritingMethodCall(Method
302310
// return Task.CompletedTask;
303311
// } :
304312
// {
305-
// // Logic generated by AddResponseWritingToMethodCall() that calls action(param1, ...)
313+
// // Logic generated by AddResponseWritingToMethodCall() that calls action(param1, parm2, ...)
306314
// };
307315
// }
308316

309317
var parameters = methodInfo.GetParameters();
310318
var storedArguments = new ParameterExpression[parameters.Length];
311-
var localVariables = new ParameterExpression[parameters.Length + 1];
319+
var localVariables = new ParameterExpression[parameters.Length + 2];
312320

313321
for (var i = 0; i < parameters.Length; i++)
314322
{
315323
storedArguments[i] = localVariables[i] = Expression.Parameter(parameters[i].ParameterType);
316324
}
317325

318326
localVariables[parameters.Length] = WasTryParseFailureExpr;
327+
localVariables[parameters.Length + 1] = TempSourceStringExpr;
319328

320329
var assignAndCall = new Expression[parameters.Length + 1];
321330
for (var i = 0; i < parameters.Length; i++)
@@ -592,74 +601,79 @@ private static Expression GetValueFromProperty(Expression sourceExpression, stri
592601

593602
private static Expression BindParameterFromValue(ParameterInfo parameter, Expression valueExpression, FactoryContext factoryContext)
594603
{
595-
Expression argumentExpression;
596-
597604
if (parameter.ParameterType == typeof(string))
598605
{
599-
argumentExpression = valueExpression;
606+
return valueExpression;
600607
}
601-
else
602-
{
603-
var nonNullableParameterType = Nullable.GetUnderlyingType(parameter.ParameterType) ?? parameter.ParameterType;
604-
var tryParseMethod = FindTryParseMethod(nonNullableParameterType);
605608

606-
if (tryParseMethod is null)
607-
{
608-
throw new InvalidOperationException($"No public static bool {parameter.ParameterType.Name}.TryParse(string, out {parameter.ParameterType.Name}) method found for {parameter.Name}.");
609-
}
609+
var underlyingNullableType = Nullable.GetUnderlyingType(parameter.ParameterType);
610+
var nonNullableParameterType = underlyingNullableType ?? parameter.ParameterType;
611+
var tryParseMethod = FindTryParseMethod(nonNullableParameterType);
612+
613+
if (tryParseMethod is null)
614+
{
615+
throw new InvalidOperationException($"No public static bool {parameter.ParameterType.Name}.TryParse(string, out {parameter.ParameterType.Name}) method found for {parameter.Name}.");
616+
}
610617

611-
// bool wasTryParseFailureVariable = false;
612-
//
613-
// // Assume "[FromRoute] int id" is the first parameter.
614-
// int param1 = httpContext.Request.RouteValue["id"] == null ? default :
615-
// {
616-
// var sourceValue = httpContext.Request.RouteValue["id"];
617-
// int parsedValue = default;
618-
//
619-
// if (!int.TryParse(sourceValue, out parsedValue))
620-
// {
621-
// wasTryParseFailureVariable = true;
622-
// Log.ParameterBindingFailed(httpContext, "Int32", "id", sourceValue)
623-
// }
624-
//
625-
// return parsedValue;
626-
// };
618+
// bool wasTryParseFailureVariable = false;
619+
// string tempSourceString;
620+
//
621+
// // Assume "[FromRoute] int id" is the first parameter.
622+
//
623+
// tempSourceString = httpContext.RequestValue["id"];
624+
//
625+
// int param1 = tempSourceString == null ? default :
626+
// {
627+
// int parsedValue = default;
628+
//
629+
// if (!int.TryParse(tempSourceString, out parsedValue))
630+
// {
631+
// wasTryParseFailureVariable = true;
632+
// Log.ParameterBindingFailed(httpContext, "Int32", "id", sourceValue)
633+
// }
634+
//
635+
// return parsedValue;
636+
// };
627637

628-
factoryContext.CheckForTryParseFailure = true;
638+
factoryContext.CheckForTryParseFailure = true;
629639

630-
var parsedValue = Expression.Variable(nonNullableParameterType);
640+
var parsedValue = Expression.Variable(nonNullableParameterType);
631641

632-
var tryParseCall = Expression.Call(tryParseMethod, valueExpression, parsedValue);
642+
var parameterTypeNameConstant = Expression.Constant(parameter.ParameterType.Name);
643+
var parameterNameConstant = Expression.Constant(parameter.Name);
633644

634-
var parameterTypeConstant = Expression.Constant(nonNullableParameterType.Name);
635-
var parameterNameConstant = Expression.Constant(parameter.Name);
636-
var failBlock = Expression.Block(
637-
Expression.Assign(WasTryParseFailureExpr, Expression.Constant(true)),
638-
Expression.Call(LogParameterBindingFailureMethod,
639-
HttpContextExpr, parameterTypeConstant, parameterNameConstant, valueExpression));
645+
var failBlock = Expression.Block(
646+
Expression.Assign(WasTryParseFailureExpr, Expression.Constant(true)),
647+
Expression.Call(LogParameterBindingFailureMethod,
648+
HttpContextExpr, parameterTypeNameConstant, parameterNameConstant, TempSourceStringExpr));
640649

641-
var ifFailExpression = Expression.IfThen(Expression.Not(tryParseCall), failBlock);
650+
var tryParseCall = Expression.Call(tryParseMethod, TempSourceStringExpr, parsedValue);
651+
var ifFailExpression = Expression.IfThen(Expression.Not(tryParseCall), failBlock);
642652

643-
argumentExpression = Expression.Block(new[] { parsedValue },
644-
ifFailExpression,
645-
parsedValue);
646-
}
653+
Expression parsedValueExpression = Expression.Block(new[] { parsedValue },
654+
ifFailExpression,
655+
parsedValue);
647656

648-
// Convert to nullable if necessary.
649-
if (argumentExpression.Type != parameter.ParameterType)
657+
// Convert back to nullable if necessary.
658+
if (underlyingNullableType is not null)
650659
{
651-
argumentExpression = Expression.Convert(argumentExpression, parameter.ParameterType);
660+
parsedValueExpression = Expression.Convert(parsedValueExpression, parameter.ParameterType);
652661
}
653662

654663
Expression defaultExpression = parameter.HasDefaultValue ?
655664
Expression.Constant(parameter.DefaultValue) :
656665
Expression.Default(parameter.ParameterType);
657666

658-
// int param1 = httpContext.Request.RouteValue["id"] == null ? default : ...
659-
return Expression.Condition(
660-
Expression.Equal(valueExpression, Expression.Constant(null)),
667+
// tempSourceString = httpContext.RequestValue["id];
668+
var storeValueToTemp = Expression.Assign(TempSourceStringExpr, valueExpression);
669+
670+
// int param1 = tempSourcString == null ? default : ...
671+
var ternary = Expression.Condition(
672+
Expression.Equal(TempSourceStringExpr, Expression.Constant(null)),
661673
defaultExpression,
662-
argumentExpression);
674+
parsedValueExpression);
675+
676+
return Expression.Block(storeValueToTemp, ternary);
663677
}
664678

665679
private static Expression BindParameterFromProperty(ParameterInfo parameter, MemberExpression property, string key, FactoryContext factoryContext) =>
@@ -828,9 +842,9 @@ public static void RequestBodyInvalidDataException(HttpContext httpContext, Inva
828842
_requestBodyInvalidDataException(GetLogger(httpContext), exception);
829843
}
830844

831-
public static void ParameterBindingFailed(HttpContext httpContext, string parameterType, string parameterName, string sourceValue)
845+
public static void ParameterBindingFailed(HttpContext httpContext, string parameterTypeName, string parameterName, string sourceValue)
832846
{
833-
_parameterBindingFailed(GetLogger(httpContext), parameterType, parameterName, sourceValue, null);
847+
_parameterBindingFailed(GetLogger(httpContext), parameterTypeName, parameterName, sourceValue, null);
834848
}
835849

836850
private static ILogger GetLogger(HttpContext httpContext)

0 commit comments

Comments
 (0)