From 752ff7d508f6f414d7e1747b425578047a0527da Mon Sep 17 00:00:00 2001 From: Rafiki Assumani Date: Mon, 9 Aug 2021 08:54:26 -0700 Subject: [PATCH 01/10] Improve error messages for implicit and explicit FromBody attribute in Minimal API --- .../src/RequestDelegateFactory.cs | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index e658ab7caa8d..397eb6e0c1f5 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -8,6 +8,7 @@ using System.Linq.Expressions; using System.Reflection; using System.Security.Claims; +using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http.Features; @@ -194,6 +195,20 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory args[i] = CreateArgument(parameters[i], factoryContext); } + if(factoryContext.HasAnotherBodyParameter) + { + var errorMessage = new StringBuilder(); + errorMessage.Append($"Failure to infer one or more parameters. Below is the list of parameters we found: \n\n"); + errorMessage.Append(String.Format("{0,6} {1,15}\n\n", "Parameter", "Source")); + + foreach (KeyValuePair kv in factoryContext.TrackedParameters) + { + errorMessage.Append(String.Format("{0,6} {1,15:N0}\n", kv.Key, kv.Value)); + } + //Could also throw a custom exception message if needed. + throw new InvalidOperationException(errorMessage.ToString()); + } + return args; } @@ -208,6 +223,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) { + factoryContext.TrackedParameters.Add(parameter.Name, "Route Attribute"); if (factoryContext.RouteParameters is { } routeParams && !routeParams.Contains(parameter.Name)) { throw new InvalidOperationException($"{parameter.Name} is not a route paramter."); @@ -217,18 +233,22 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } queryAttribute) { + factoryContext.TrackedParameters.Add(parameter.Name, "Query Attribute"); return BindParameterFromProperty(parameter, QueryExpr, queryAttribute.Name ?? parameter.Name, factoryContext); } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } headerAttribute) { + factoryContext.TrackedParameters.Add(parameter.Name, "Header Attribute"); return BindParameterFromProperty(parameter, HeadersExpr, headerAttribute.Name ?? parameter.Name, factoryContext); } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } bodyAttribute) { + factoryContext.TrackedParameters.Add(parameter.Name, "Body Attribute"); return BindParameterFromBody(parameter, bodyAttribute.AllowEmpty, factoryContext); } else if (parameter.CustomAttributes.Any(a => typeof(IFromServiceMetadata).IsAssignableFrom(a.AttributeType))) { + factoryContext.TrackedParameters.Add(parameter.Name, "From Services"); return BindParameterFromService(parameter); } else if (parameter.ParameterType == typeof(HttpContext)) @@ -257,9 +277,11 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext // to query string in this case if (factoryContext.RouteParameters is { } routeParams && routeParams.Contains(parameter.Name)) { + factoryContext.TrackedParameters.Add(parameter.Name, "Route Parameter"); return BindParameterFromProperty(parameter, RouteValuesExpr, parameter.Name, factoryContext); } + factoryContext.TrackedParameters.Add(parameter.Name, "Query String Parameter"); return BindParameterFromRouteValueOrQueryString(parameter, parameter.Name, factoryContext); } else @@ -268,6 +290,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext { if (serviceProviderIsService.IsService(parameter.ParameterType)) { + factoryContext.TrackedParameters.Add(parameter.Name, "Service Parameter"); return Expression.Call(GetRequiredServiceMethod.MakeGenericMethod(parameter.ParameterType), RequestServicesExpr); } } @@ -489,9 +512,6 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, { object? bodyValue = defaultBodyValue; - var feature = httpContext.Features.Get(); - if (feature?.CanHaveBody == true) - { try { bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType); @@ -503,11 +523,11 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, } catch (InvalidDataException ex) { + Log.RequestBodyInvalidDataException(httpContext, ex); httpContext.Response.StatusCode = 400; return; } - } await invoker(target, httpContext, bodyValue); }; @@ -711,7 +731,10 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al { if (factoryContext.JsonRequestBodyType is not null) { - throw new InvalidOperationException("Action cannot have more than one FromBody attribute."); + factoryContext.HasAnotherBodyParameter = true; +#pragma warning disable CS8604 // Possible null reference argument. + factoryContext.TrackedParameters.Add(parameter.Name, "We failed to infer this parameter. Did you forget to inject it as a Service?"); +#pragma warning restore CS8604 // Possible null reference argument. } var nullability = NullabilityContext.Create(parameter); @@ -954,6 +977,10 @@ private class FactoryContext public bool UsingTempSourceString { get; set; } public List<(ParameterExpression, Expression)> CheckParams { get; } = new(); + + public Dictionary TrackedParameters { get; } = new(); + + public bool HasAnotherBodyParameter { get; set; } } private static partial class Log From ecdb0fb22064e3179ffe46bb1730faec7c9d55cb Mon Sep 17 00:00:00 2001 From: Rafiki Assumani Date: Mon, 9 Aug 2021 10:30:22 -0700 Subject: [PATCH 02/10] Add error message for mapget when user forget to register service --- .../src/HttpRequestJsonExtensions.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs b/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs index e2adc83d7128..83cc1686e3e7 100644 --- a/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs +++ b/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs @@ -4,10 +4,12 @@ using System; using System.Diagnostics.CodeAnalysis; using System.IO; +using System.Net.Http; using System.Text; using System.Text.Json; using System.Threading; using System.Threading.Tasks; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Http.Json; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; @@ -59,7 +61,7 @@ public static class HttpRequestJsonExtensions if (!request.HasJsonContentType(out var charset)) { - throw CreateContentTypeError(request); + throw CreateContentTypeError(request, null); } options ??= ResolveSerializerOptions(request.HttpContext); @@ -124,7 +126,7 @@ public static class HttpRequestJsonExtensions if (!request.HasJsonContentType(out var charset)) { - throw CreateContentTypeError(request); + throw CreateContentTypeError(request, type); } options ??= ResolveSerializerOptions(request.HttpContext); @@ -135,7 +137,7 @@ public static class HttpRequestJsonExtensions try { return await JsonSerializer.DeserializeAsync(inputStream, type, options, cancellationToken); - } + } finally { if (usesTranscodingStream) @@ -192,8 +194,15 @@ private static JsonSerializerOptions ResolveSerializerOptions(HttpContext httpCo return httpContext.RequestServices?.GetService>()?.Value?.SerializerOptions ?? JsonOptions.DefaultSerializerOptions; } - private static InvalidOperationException CreateContentTypeError(HttpRequest request) - { + private static InvalidOperationException CreateContentTypeError(HttpRequest request, Type? type) + { + var feature = request.HttpContext.Features.Get(); + + //if there is no content-type and the content-length is zero, it might be safe to assume that the parameter is a service that the user forgot to register with the DI + if (feature?.CanHaveBody == false) + { + return new InvalidOperationException($"We failed to infer or bind '{type?.FullName}' parameter. Was this a Service parameter that you forgot to register ?"); + } return new InvalidOperationException($"Unable to read the request as JSON because the request content type '{request.ContentType}' is not a known JSON content type."); } From 47a1e90af3caa3ff748d0fdae382c156258f5fb8 Mon Sep 17 00:00:00 2001 From: Rafiki Assumani Date: Mon, 9 Aug 2021 12:20:03 -0700 Subject: [PATCH 03/10] add suggested changes --- src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs b/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs index 83cc1686e3e7..b8db40d48a13 100644 --- a/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs +++ b/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs @@ -61,7 +61,7 @@ public static class HttpRequestJsonExtensions if (!request.HasJsonContentType(out var charset)) { - throw CreateContentTypeError(request, null); + throw CreateContentTypeError(request, typeof(TValue)); } options ??= ResolveSerializerOptions(request.HttpContext); From f834444449a66a5f20e2958c7277a897d0e78e9d Mon Sep 17 00:00:00 2001 From: Rafiki Assumani Date: Mon, 9 Aug 2021 13:46:14 -0700 Subject: [PATCH 04/10] replace null with typeof(Tvalue) --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index dd353a3a17dd..a101ae895269 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -732,9 +732,7 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al if (factoryContext.JsonRequestBodyType is not null) { factoryContext.HasAnotherBodyParameter = true; -#pragma warning disable CS8604 // Possible null reference argument. - factoryContext.TrackedParameters.Add(parameter.Name, "We failed to infer this parameter. Did you forget to inject it as a Service?"); -#pragma warning restore CS8604 // Possible null reference argument. + factoryContext.TrackedParameters.Add(parameter.Name!, "We failed to infer this parameter. Did you forget to inject it as a Service?"); } var nullability = NullabilityContext.Create(parameter); From 45eeb28a0e3ee9565ec8378a046c934f58c4ddbe Mon Sep 17 00:00:00 2001 From: Rafiki Assumani Date: Mon, 16 Aug 2021 11:25:28 -0700 Subject: [PATCH 05/10] multiple frombody --- .../src/HttpRequestJsonExtensions.cs | 13 ++--- .../src/RequestDelegateFactory.cs | 47 ++++++++++++++----- 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs b/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs index b8db40d48a13..8645444c8d38 100644 --- a/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs +++ b/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs @@ -61,7 +61,7 @@ public static class HttpRequestJsonExtensions if (!request.HasJsonContentType(out var charset)) { - throw CreateContentTypeError(request, typeof(TValue)); + throw CreateContentTypeError(request); } options ??= ResolveSerializerOptions(request.HttpContext); @@ -126,7 +126,7 @@ public static class HttpRequestJsonExtensions if (!request.HasJsonContentType(out var charset)) { - throw CreateContentTypeError(request, type); + throw CreateContentTypeError(request); } options ??= ResolveSerializerOptions(request.HttpContext); @@ -194,15 +194,8 @@ private static JsonSerializerOptions ResolveSerializerOptions(HttpContext httpCo return httpContext.RequestServices?.GetService>()?.Value?.SerializerOptions ?? JsonOptions.DefaultSerializerOptions; } - private static InvalidOperationException CreateContentTypeError(HttpRequest request, Type? type) + private static InvalidOperationException CreateContentTypeError(HttpRequest request) { - var feature = request.HttpContext.Features.Get(); - - //if there is no content-type and the content-length is zero, it might be safe to assume that the parameter is a service that the user forgot to register with the DI - if (feature?.CanHaveBody == false) - { - return new InvalidOperationException($"We failed to infer or bind '{type?.FullName}' parameter. Was this a Service parameter that you forgot to register ?"); - } return new InvalidOperationException($"Unable to read the request as JSON because the request content type '{request.ContentType}' is not a known JSON content type."); } diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index a101ae895269..02d520756002 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -201,11 +201,10 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory errorMessage.Append($"Failure to infer one or more parameters. Below is the list of parameters we found: \n\n"); errorMessage.Append(string.Format("{0,6} {1,15}\n\n", "Parameter", "Source")); - foreach (KeyValuePair kv in factoryContext.TrackedParameters) + foreach (var kv in factoryContext.TrackedParameters) { errorMessage.Append(string.Format("{0,6} {1,15:N0}\n", kv.Key, kv.Value)); } - //Could also throw a custom exception message if needed. throw new InvalidOperationException(errorMessage.ToString()); } @@ -223,7 +222,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) { - factoryContext.TrackedParameters.Add(parameter.Name, "Route Attribute"); + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteAttribue); if (factoryContext.RouteParameters is { } routeParams && !routeParams.Contains(parameter.Name, StringComparer.OrdinalIgnoreCase)) { throw new InvalidOperationException($"{parameter.Name} is not a route paramter."); @@ -233,22 +232,22 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } queryAttribute) { - factoryContext.TrackedParameters.Add(parameter.Name, "Query Attribute"); + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.QueryAttribue); return BindParameterFromProperty(parameter, QueryExpr, queryAttribute.Name ?? parameter.Name, factoryContext); } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } headerAttribute) { - factoryContext.TrackedParameters.Add(parameter.Name, "Header Attribute"); + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.HeaderAttribue); return BindParameterFromProperty(parameter, HeadersExpr, headerAttribute.Name ?? parameter.Name, factoryContext); } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } bodyAttribute) { - factoryContext.TrackedParameters.Add(parameter.Name, "Body Attribute"); + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.BodyAttribue); return BindParameterFromBody(parameter, bodyAttribute.AllowEmpty, factoryContext); } else if (parameter.CustomAttributes.Any(a => typeof(IFromServiceMetadata).IsAssignableFrom(a.AttributeType))) { - factoryContext.TrackedParameters.Add(parameter.Name, "From Services"); + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.ServiceAttribue); return BindParameterFromService(parameter); } else if (parameter.ParameterType == typeof(HttpContext)) @@ -277,11 +276,11 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext // to query string in this case if (factoryContext.RouteParameters is { } routeParams && routeParams.Contains(parameter.Name, StringComparer.OrdinalIgnoreCase)) { - factoryContext.TrackedParameters.Add(parameter.Name, "Route Parameter"); + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteParameter); return BindParameterFromProperty(parameter, RouteValuesExpr, parameter.Name, factoryContext); } - factoryContext.TrackedParameters.Add(parameter.Name, "Query String Parameter"); + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.QueryStringParameter); return BindParameterFromRouteValueOrQueryString(parameter, parameter.Name, factoryContext); } else @@ -290,11 +289,12 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext { if (serviceProviderIsService.IsService(parameter.ParameterType)) { - factoryContext.TrackedParameters.Add(parameter.Name, "Service Parameter"); + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.ServiceParameter); return Expression.Call(GetRequiredServiceMethod.MakeGenericMethod(parameter.ParameterType), RequestServicesExpr); } } + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.BodyParameter); return BindParameterFromBody(parameter, allowEmpty: false, factoryContext); } } @@ -511,7 +511,9 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, return async (target, httpContext) => { object? bodyValue = defaultBodyValue; - + var feature = httpContext.Features.Get(); + if (feature?.CanHaveBody == true) + { try { bodyValue = await httpContext.Request.ReadFromJsonAsync(bodyType); @@ -528,7 +530,7 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, httpContext.Response.StatusCode = 400; return; } - + } await invoker(target, httpContext, bodyValue); }; } @@ -732,7 +734,12 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al if (factoryContext.JsonRequestBodyType is not null) { factoryContext.HasAnotherBodyParameter = true; - factoryContext.TrackedParameters.Add(parameter.Name!, "We failed to infer this parameter. Did you forget to inject it as a Service?"); + var parameterName = parameter.Name; + if (parameterName is not null && factoryContext.TrackedParameters.ContainsKey(parameterName)) + { + factoryContext.TrackedParameters.Remove(parameterName); + factoryContext.TrackedParameters.Add(parameter.Name!, "We failed to infer this parameter. Did you forget to inject it as a Service in the DI container?"); + } } var nullability = NullabilityContext.Create(parameter); @@ -981,6 +988,20 @@ private class FactoryContext public bool HasAnotherBodyParameter { get; set; } } + private static class RequestDelegateFactoryConstants + { + public const string RouteAttribue = "Route Attribute"; + public const string QueryAttribue = "Query Attribute"; + public const string HeaderAttribue = "Header Attribute"; + public const string BodyAttribue = "Body Attribute"; + public const string ServiceAttribue = "Service Attribute"; + public const string RouteParameter = "Route Parameter"; + public const string QueryStringParameter = "Query String Parameter"; + public const string ServiceParameter = "Service Parameter"; + public const string BodyParameter = "Body Parameter"; + + } + private static partial class Log { public static void RequestBodyIOException(HttpContext httpContext, IOException exception) From 98217cbb5d92bf2283c82b449586276f56947c36 Mon Sep 17 00:00:00 2001 From: Rafiki Assumani Date: Mon, 16 Aug 2021 12:04:23 -0700 Subject: [PATCH 06/10] resolve more merge issues --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 9b116532354f..83af2152d7ff 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -5,6 +5,7 @@ using System.Linq.Expressions; using System.Reflection; using System.Security.Claims; +using System.Text; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Http.Metadata; using Microsoft.Extensions.DependencyInjection; @@ -988,7 +989,8 @@ private class FactoryContext public List? RouteParameters { get; set; } public bool UsingTempSourceString { get; set; } - public List<(ParameterExpression, Expression)> CheckParams { get; } = new(); + public List ExtraLocals { get; } = new(); + public List ParamCheckExpressions { get; } = new(); public Dictionary TrackedParameters { get; } = new(); From 776b041052d5b4b7651f680edd34f4ec16dc0b54 Mon Sep 17 00:00:00 2001 From: Rafiki Assumani Date: Mon, 16 Aug 2021 19:50:07 -0700 Subject: [PATCH 07/10] address pr suggestions --- .../src/HttpRequestJsonExtensions.cs | 2 + .../src/RequestDelegateFactory.cs | 48 ++++++++++++------- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs b/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs index 8645444c8d38..9e09c5a0f21d 100644 --- a/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs +++ b/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs @@ -137,6 +137,7 @@ public static class HttpRequestJsonExtensions try { return await JsonSerializer.DeserializeAsync(inputStream, type, options, cancellationToken); + } finally { @@ -197,6 +198,7 @@ private static JsonSerializerOptions ResolveSerializerOptions(HttpContext httpCo private static InvalidOperationException CreateContentTypeError(HttpRequest request) { return new InvalidOperationException($"Unable to read the request as JSON because the request content type '{request.ContentType}' is not a known JSON content type."); + } private static (Stream inputStream, bool usesTranscodingStream) GetInputStream(HttpContext httpContext, Encoding? encoding) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 83af2152d7ff..242e6fca1de7 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -192,15 +192,9 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory if(factoryContext.HasAnotherBodyParameter) { - var errorMessage = new StringBuilder(); - errorMessage.Append($"Failure to infer one or more parameters. Below is the list of parameters we found: \n\n"); - errorMessage.Append(string.Format("{0,6} {1,15}\n\n", "Parameter", "Source")); + var errorMessage = BuildErrorMessageForMultipleBodyParameters(factoryContext); + throw new InvalidOperationException(errorMessage); - foreach (var kv in factoryContext.TrackedParameters) - { - errorMessage.Append(string.Format("{0,6} {1,15:N0}\n", kv.Key, kv.Value)); - } - throw new InvalidOperationException(errorMessage.ToString()); } return args; @@ -752,7 +746,8 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al if (parameterName is not null && factoryContext.TrackedParameters.ContainsKey(parameterName)) { factoryContext.TrackedParameters.Remove(parameterName); - factoryContext.TrackedParameters.Add(parameter.Name!, "We failed to infer this parameter. Did you forget to inject it as a Service in the DI container?"); + factoryContext.TrackedParameters.Add(parameterName, "UNKNOWN"); + } } @@ -999,15 +994,15 @@ private class FactoryContext private static class RequestDelegateFactoryConstants { - public const string RouteAttribue = "Route Attribute"; - public const string QueryAttribue = "Query Attribute"; - public const string HeaderAttribue = "Header Attribute"; - public const string BodyAttribue = "Body Attribute"; - public const string ServiceAttribue = "Service Attribute"; - public const string RouteParameter = "Route Parameter"; - public const string QueryStringParameter = "Query String Parameter"; - public const string ServiceParameter = "Service Parameter"; - public const string BodyParameter = "Body Parameter"; + public const string RouteAttribue = "Route (Attribute)"; + public const string QueryAttribue = "Query (Attribute)"; + public const string HeaderAttribue = "Header (Attribute)"; + public const string BodyAttribue = "Body (Attribute)"; + public const string ServiceAttribue = "Service (Attribute)"; + public const string RouteParameter = "Route (Inferred)"; + public const string QueryStringParameter = "Query String (Inferred)"; + public const string ServiceParameter = "Services (Inferred)"; + public const string BodyParameter = "Body (Inferred)"; } @@ -1078,5 +1073,22 @@ private static void SetPlaintextContentType(HttpContext httpContext) { httpContext.Response.ContentType ??= "text/plain; charset=utf-8"; } + + private static string BuildErrorMessageForMultipleBodyParameters(FactoryContext factoryContext) + { + var errorMessage = new StringBuilder(); + errorMessage.Append($"Failure to infer one or more parameters.\n"); + errorMessage.Append("Below is the list of parameters that we found: \n\n"); + errorMessage.Append($"{"Parameter",-20}|{"Source",-30} \n"); + errorMessage.Append("---------------------------------------------------------------------------------\n"); + + foreach (var kv in factoryContext.TrackedParameters) + { + errorMessage.Append($"{kv.Key,-19} | {kv.Value, -15}\n"); + } + errorMessage.Append("\n\n"); + errorMessage.Append("Did you forget to inject the \"UNKNOWN\" parameters as a Service?\n\n"); + return errorMessage.ToString(); + } } } From 8e3d540e2618e673d5fcd803994600cceed078e7 Mon Sep 17 00:00:00 2001 From: Rafiki Assumani Date: Mon, 16 Aug 2021 20:02:17 -0700 Subject: [PATCH 08/10] address more PR comments --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 242e6fca1de7..add895e9a3b6 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -190,7 +190,7 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory args[i] = CreateArgument(parameters[i], factoryContext); } - if(factoryContext.HasAnotherBodyParameter) + if(factoryContext.HasMultipleBodyParameters) { var errorMessage = BuildErrorMessageForMultipleBodyParameters(factoryContext); throw new InvalidOperationException(errorMessage); @@ -741,7 +741,7 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al { if (factoryContext.JsonRequestBodyType is not null) { - factoryContext.HasAnotherBodyParameter = true; + factoryContext.HasMultipleBodyParameters = true; var parameterName = parameter.Name; if (parameterName is not null && factoryContext.TrackedParameters.ContainsKey(parameterName)) { @@ -989,7 +989,7 @@ private class FactoryContext public Dictionary TrackedParameters { get; } = new(); - public bool HasAnotherBodyParameter { get; set; } + public bool HasMultipleBodyParameters { get; set; } } private static class RequestDelegateFactoryConstants @@ -1087,7 +1087,7 @@ private static string BuildErrorMessageForMultipleBodyParameters(FactoryContext errorMessage.Append($"{kv.Key,-19} | {kv.Value, -15}\n"); } errorMessage.Append("\n\n"); - errorMessage.Append("Did you forget to inject the \"UNKNOWN\" parameters as a Service?\n\n"); + errorMessage.Append("Did you mean to register the \"UNKNOWN\" parameters as a Service?\n\n"); return errorMessage.ToString(); } } From cdceb0544c0694f9c71ad5cccf5174d5ee87041f Mon Sep 17 00:00:00 2001 From: Rafiki Assumani Date: Tue, 17 Aug 2021 12:47:59 -0700 Subject: [PATCH 09/10] clean up --- .../Http.Extensions/src/HttpRequestJsonExtensions.cs | 5 ++--- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 9 ++++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs b/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs index 9e09c5a0f21d..9598066739e6 100644 --- a/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs +++ b/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs @@ -4,7 +4,6 @@ using System; using System.Diagnostics.CodeAnalysis; using System.IO; -using System.Net.Http; using System.Text; using System.Text.Json; using System.Threading; @@ -138,7 +137,7 @@ public static class HttpRequestJsonExtensions { return await JsonSerializer.DeserializeAsync(inputStream, type, options, cancellationToken); - } + } finally { if (usesTranscodingStream) @@ -196,7 +195,7 @@ private static JsonSerializerOptions ResolveSerializerOptions(HttpContext httpCo } private static InvalidOperationException CreateContentTypeError(HttpRequest request) - { + { return new InvalidOperationException($"Unable to read the request as JSON because the request content type '{request.ContentType}' is not a known JSON content type."); } diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index add895e9a3b6..f6d3127cfc2f 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -190,7 +190,7 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory args[i] = CreateArgument(parameters[i], factoryContext); } - if(factoryContext.HasMultipleBodyParameters) + if (factoryContext.HasMultipleBodyParameters) { var errorMessage = BuildErrorMessageForMultipleBodyParameters(factoryContext); throw new InvalidOperationException(errorMessage); @@ -748,7 +748,7 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al factoryContext.TrackedParameters.Remove(parameterName); factoryContext.TrackedParameters.Add(parameterName, "UNKNOWN"); - } + } } var nullability = NullabilityContext.Create(parameter); @@ -988,8 +988,7 @@ private class FactoryContext public List ParamCheckExpressions { get; } = new(); public Dictionary TrackedParameters { get; } = new(); - - public bool HasMultipleBodyParameters { get; set; } + public bool HasMultipleBodyParameters { get; set; } } private static class RequestDelegateFactoryConstants @@ -1084,7 +1083,7 @@ private static string BuildErrorMessageForMultipleBodyParameters(FactoryContext foreach (var kv in factoryContext.TrackedParameters) { - errorMessage.Append($"{kv.Key,-19} | {kv.Value, -15}\n"); + errorMessage.Append($"{kv.Key,-19} | {kv.Value,-15}\n"); } errorMessage.Append("\n\n"); errorMessage.Append("Did you mean to register the \"UNKNOWN\" parameters as a Service?\n\n"); From d8253c650251dc8dd77121ec4770390be18ae758 Mon Sep 17 00:00:00 2001 From: Rafiki Assumani Date: Tue, 17 Aug 2021 13:24:43 -0700 Subject: [PATCH 10/10] reset HttpRequestJsonExtensions.cs --- src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs | 3 --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 7 +++++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs b/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs index 9598066739e6..e2adc83d7128 100644 --- a/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs +++ b/src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs @@ -8,7 +8,6 @@ using System.Text.Json; using System.Threading; using System.Threading.Tasks; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Http.Json; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; @@ -136,7 +135,6 @@ public static class HttpRequestJsonExtensions try { return await JsonSerializer.DeserializeAsync(inputStream, type, options, cancellationToken); - } finally { @@ -197,7 +195,6 @@ private static JsonSerializerOptions ResolveSerializerOptions(HttpContext httpCo private static InvalidOperationException CreateContentTypeError(HttpRequest request) { return new InvalidOperationException($"Unable to read the request as JSON because the request content type '{request.ContentType}' is not a known JSON content type."); - } private static (Stream inputStream, bool usesTranscodingStream) GetInputStream(HttpContext httpContext, Encoding? encoding) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index f6d3127cfc2f..d6d80df8bae2 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -267,20 +267,22 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext // when RDF.Create is manually invoked. if (factoryContext.RouteParameters is { } routeParams) { - factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteParameter); + 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 // to query string in this case + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteParameter); return BindParameterFromProperty(parameter, RouteValuesExpr, parameter.Name, factoryContext); } else { + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.QueryStringParameter); return BindParameterFromProperty(parameter, QueryExpr, parameter.Name, factoryContext); } } - factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.QueryStringParameter); + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteOrQueryStringParameter); return BindParameterFromRouteValueOrQueryString(parameter, parameter.Name, factoryContext); } else @@ -1002,6 +1004,7 @@ private static class RequestDelegateFactoryConstants public const string QueryStringParameter = "Query String (Inferred)"; public const string ServiceParameter = "Services (Inferred)"; public const string BodyParameter = "Body (Inferred)"; + public const string RouteOrQueryStringParameter = "Route or Query String (Inferred)"; }