diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index cb2bc5d44cc1..475c03fbde22 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -40,8 +40,8 @@ public static partial class RequestDelegateFactory private static readonly MethodInfo LogParameterBindingFailedMethod = GetMethodInfo>((httpContext, parameterType, parameterName, sourceValue) => Log.ParameterBindingFailed(httpContext, parameterType, parameterName, sourceValue)); - private static readonly MethodInfo LogRequiredParameterNotProvidedMethod = GetMethodInfo>((httpContext, parameterType, parameterName) => - Log.RequiredParameterNotProvided(httpContext, parameterType, parameterName)); + private static readonly MethodInfo LogRequiredParameterNotProvidedMethod = GetMethodInfo>((httpContext, parameterType, parameterName, source) => + Log.RequiredParameterNotProvided(httpContext, parameterType, parameterName, source)); private static readonly ParameterExpression TargetExpr = Expression.Parameter(typeof(object), "target"); private static readonly ParameterExpression BodyValueExpr = Expression.Parameter(typeof(object), "bodyValue"); @@ -219,17 +219,17 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext throw new InvalidOperationException($"{parameter.Name} is not a route paramter."); } - return BindParameterFromProperty(parameter, RouteValuesExpr, routeAttribute.Name ?? parameter.Name, factoryContext); + return BindParameterFromProperty(parameter, RouteValuesExpr, routeAttribute.Name ?? parameter.Name, factoryContext, "route"); } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } queryAttribute) { factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.QueryAttribue); - return BindParameterFromProperty(parameter, QueryExpr, queryAttribute.Name ?? parameter.Name, factoryContext); + return BindParameterFromProperty(parameter, QueryExpr, queryAttribute.Name ?? parameter.Name, factoryContext, "query string"); } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } headerAttribute) { factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.HeaderAttribue); - return BindParameterFromProperty(parameter, HeadersExpr, headerAttribute.Name ?? parameter.Name, factoryContext); + return BindParameterFromProperty(parameter, HeadersExpr, headerAttribute.Name ?? parameter.Name, factoryContext, "header"); } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } bodyAttribute) { @@ -278,12 +278,12 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext // We're in the fallback case and we have a parameter and route parameter match so don't fallback // to query string in this case factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteParameter); - return BindParameterFromProperty(parameter, RouteValuesExpr, parameter.Name, factoryContext); + return BindParameterFromProperty(parameter, RouteValuesExpr, parameter.Name, factoryContext, "route"); } else { factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.QueryStringParameter); - return BindParameterFromProperty(parameter, QueryExpr, parameter.Name, factoryContext); + return BindParameterFromProperty(parameter, QueryExpr, parameter.Name, factoryContext, "query string"); } } @@ -634,12 +634,16 @@ private static Expression BindParameterFromService(ParameterInfo parameter) : Expression.Call(GetRequiredServiceMethod.MakeGenericMethod(parameter.ParameterType), RequestServicesExpr); } - private static Expression BindParameterFromValue(ParameterInfo parameter, Expression valueExpression, FactoryContext factoryContext) + private static Expression BindParameterFromValue(ParameterInfo parameter, Expression valueExpression, FactoryContext factoryContext, string source) { var isOptional = IsOptionalParameter(parameter); var argument = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); + var parameterTypeNameConstant = Expression.Constant(parameter.ParameterType.Name); + var parameterNameConstant = Expression.Constant(parameter.Name); + var sourceConstant = Expression.Constant(source); + if (parameter.ParameterType == typeof(string)) { if (!isOptional) @@ -658,7 +662,7 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres Expression.Block( Expression.Assign(WasParamCheckFailureExpr, Expression.Constant(true)), Expression.Call(LogRequiredParameterNotProvidedMethod, - HttpContextExpr, Expression.Constant(parameter.ParameterType.Name), Expression.Constant(parameter.Name)) + HttpContextExpr, parameterTypeNameConstant, parameterNameConstant, sourceConstant) ) ) ); @@ -740,9 +744,6 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres // If the parameter is nullable, create a "parsedValue" local to TryParse into since we cannot use the parameter directly. var parsedValue = isNotNullable ? argument : Expression.Variable(nonNullableParameterType, "parsedValue"); - var parameterTypeNameConstant = Expression.Constant(parameter.ParameterType.Name); - var parameterNameConstant = Expression.Constant(parameter.Name); - var failBlock = Expression.Block( Expression.Assign(WasParamCheckFailureExpr, Expression.Constant(true)), Expression.Call(LogParameterBindingFailedMethod, @@ -763,7 +764,7 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres Expression.Block( Expression.Assign(WasParamCheckFailureExpr, Expression.Constant(true)), Expression.Call(LogRequiredParameterNotProvidedMethod, - HttpContextExpr, parameterTypeNameConstant, parameterNameConstant) + HttpContextExpr, parameterTypeNameConstant, parameterNameConstant, sourceConstant) ) ) ); @@ -802,14 +803,14 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres return argument; } - private static Expression BindParameterFromProperty(ParameterInfo parameter, MemberExpression property, string key, FactoryContext factoryContext) => - BindParameterFromValue(parameter, GetValueFromProperty(property, key), factoryContext); + private static Expression BindParameterFromProperty(ParameterInfo parameter, MemberExpression property, string key, FactoryContext factoryContext, string source) => + BindParameterFromValue(parameter, GetValueFromProperty(property, key), factoryContext, source); private static Expression BindParameterFromRouteValueOrQueryString(ParameterInfo parameter, string key, FactoryContext factoryContext) { var routeValue = GetValueFromProperty(RouteValuesExpr, key); var queryValue = GetValueFromProperty(QueryExpr, key); - return BindParameterFromValue(parameter, Expression.Coalesce(routeValue, queryValue), factoryContext); + return BindParameterFromValue(parameter, Expression.Coalesce(routeValue, queryValue), factoryContext, "route or query string"); } private static Expression BindParameterFromBindAsync(ParameterInfo parameter, FactoryContext factoryContext) @@ -836,7 +837,10 @@ private static Expression BindParameterFromBindAsync(ParameterInfo parameter, Fa Expression.Block( Expression.Assign(WasParamCheckFailureExpr, Expression.Constant(true)), Expression.Call(LogRequiredParameterNotProvidedMethod, - HttpContextExpr, Expression.Constant(parameter.ParameterType.Name), Expression.Constant(parameter.Name)) + HttpContextExpr, + Expression.Constant(parameter.ParameterType.Name), + Expression.Constant(parameter.Name), + Expression.Constant($"{parameter.ParameterType.Name}.BindAsync(HttpContext)")) ) ) ); @@ -883,7 +887,10 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al Expression.Block( Expression.Assign(WasParamCheckFailureExpr, Expression.Constant(true)), Expression.Call(LogRequiredParameterNotProvidedMethod, - HttpContextExpr, Expression.Constant(parameter.ParameterType.Name), Expression.Constant(parameter.Name)) + HttpContextExpr, + Expression.Constant(parameter.ParameterType.Name), + Expression.Constant(parameter.Name), + Expression.Constant("body")) ) ) ); @@ -1153,13 +1160,13 @@ public static void ParameterBindingFailed(HttpContext httpContext, string parame EventName = "ParamaterBindingFailed")] private static partial void ParameterBindingFailed(ILogger logger, string parameterType, string parameterName, string sourceValue); - public static void RequiredParameterNotProvided(HttpContext httpContext, string parameterTypeName, string parameterName) - => RequiredParameterNotProvided(GetLogger(httpContext), parameterTypeName, parameterName); + public static void RequiredParameterNotProvided(HttpContext httpContext, string parameterTypeName, string parameterName, string source) + => RequiredParameterNotProvided(GetLogger(httpContext), parameterTypeName, parameterName, source); [LoggerMessage(4, LogLevel.Debug, - @"Required parameter ""{ParameterType} {ParameterName}"" was not provided.", + @"Required parameter ""{ParameterType} {ParameterName}"" was not provided from {Source}.", EventName = "RequiredParameterNotProvided")] - private static partial void RequiredParameterNotProvided(ILogger logger, string parameterType, string parameterName); + private static partial void RequiredParameterNotProvided(ILogger logger, string parameterType, string parameterName, string source); public static void ParameterBindingFromHttpContextFailed(HttpContext httpContext, string parameterTypeName, string parameterName) => ParameterBindingFromHttpContextFailed(GetLogger(httpContext), parameterTypeName, parameterName); diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 631e6c0b886f..9b21224e2223 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -819,11 +819,11 @@ public async Task RequestDelegateLogsBindAsyncFailuresAndSets400Response() Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), logs[0].EventId); Assert.Equal(LogLevel.Debug, logs[0].LogLevel); - Assert.Equal(@"Required parameter ""MyBindAsyncRecord arg1"" was not provided.", logs[0].Message); + Assert.Equal(@"Required parameter ""MyBindAsyncRecord arg1"" was not provided from MyBindAsyncRecord.BindAsync(HttpContext).", logs[0].Message); Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), logs[1].EventId); Assert.Equal(LogLevel.Debug, logs[1].LogLevel); - Assert.Equal(@"Required parameter ""MyBindAsyncRecord arg2"" was not provided.", logs[1].Message); + Assert.Equal(@"Required parameter ""MyBindAsyncRecord arg2"" was not provided from MyBindAsyncRecord.BindAsync(HttpContext).", logs[1].Message); } [Fact] @@ -1859,7 +1859,7 @@ public async Task RequestDelegateHandlesQueryParamOptionality(Delegate @delegate Assert.Equal(LogLevel.Debug, log.LogLevel); Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), log.EventId); var expectedType = paramName == "age" ? "Int32 age" : "String name"; - Assert.Equal($@"Required parameter ""{expectedType}"" was not provided.", log.Message); + Assert.Equal($@"Required parameter ""{expectedType}"" was not provided from route or query string.", log.Message); } else { @@ -1935,7 +1935,7 @@ public async Task RequestDelegateHandlesRouteParamOptionality(Delegate @delegate Assert.Equal(LogLevel.Debug, log.LogLevel); Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), log.EventId); var expectedType = paramName == "age" ? "Int32 age" : "String name"; - Assert.Equal($@"Required parameter ""{expectedType}"" was not provided.", log.Message); + Assert.Equal($@"Required parameter ""{expectedType}"" was not provided from query string.", log.Message); } else { @@ -2006,7 +2006,7 @@ public async Task RequestDelegateHandlesBodyParamOptionality(Delegate @delegate, var log = Assert.Single(logs); Assert.Equal(LogLevel.Debug, log.LogLevel); Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), log.EventId); - Assert.Equal(@"Required parameter ""Todo todo"" was not provided.", log.Message); + Assert.Equal(@"Required parameter ""Todo todo"" was not provided from body.", log.Message); } else { @@ -2126,7 +2126,6 @@ public async Task AllowEmptyOverridesOptionality(Delegate @delegate, bool allows var factoryResult = RequestDelegateFactory.Create(@delegate); var requestDelegate = factoryResult.RequestDelegate; - await requestDelegate(httpContext); var logs = TestSink.Writes.ToArray(); @@ -2137,7 +2136,7 @@ public async Task AllowEmptyOverridesOptionality(Delegate @delegate, bool allows var log = Assert.Single(logs); Assert.Equal(LogLevel.Debug, log.LogLevel); Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), log.EventId); - Assert.Equal(@"Required parameter ""Todo todo"" was not provided.", log.Message); + Assert.Equal(@"Required parameter ""Todo todo"" was not provided from body.", log.Message); } else {