From 5a4bf946e3874cc6e9a9c8e884c5ee3cb6a1cf58 Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 14 Sep 2021 10:37:08 -0700 Subject: [PATCH 1/4] Add support for BindAsync without ParameterInfo --- .../src/RequestDelegateFactory.cs | 5 +- .../test/ParameterBindingMethodCacheTests.cs | 81 +++++++++- .../test/RequestDelegateFactoryTests.cs | 144 +++++++++++++++++- src/Shared/ParameterBindingMethodCache.cs | 55 +++++-- 4 files changed, 262 insertions(+), 23 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 627c38d7e64d..dbaeb5d5fcdd 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -840,7 +840,7 @@ private static Expression BindParameterFromBindAsync(ParameterInfo parameter, Fa var isOptional = IsOptionalParameter(parameter, factoryContext); // Get the BindAsync method for the type. - var bindAsyncExpression = ParameterBindingMethodCache.FindBindAsyncMethod(parameter); + var (bindAsyncExpression, paramCount) = ParameterBindingMethodCache.FindBindAsyncMethod(parameter); // We know BindAsync exists because there's no way to opt-in without defining the method on the type. Debug.Assert(bindAsyncExpression is not null); @@ -854,6 +854,7 @@ private static Expression BindParameterFromBindAsync(ParameterInfo parameter, Fa if (!isOptional) { var typeName = TypeNameHelper.GetTypeDisplayName(parameter.ParameterType, fullName: false); + var message = paramCount == 2 ? $"{typeName}.BindAsync(HttpContext, ParameterInfo)" : $"{typeName}.BindAsync(HttpContext)"; var checkRequiredBodyBlock = Expression.Block( Expression.IfThen( Expression.Equal(boundValueExpr, Expression.Constant(null)), @@ -863,7 +864,7 @@ private static Expression BindParameterFromBindAsync(ParameterInfo parameter, Fa HttpContextExpr, Expression.Constant(typeName), Expression.Constant(parameter.Name), - Expression.Constant($"{typeName}.BindAsync(HttpContext, ParameterInfo)"), + Expression.Constant(message), Expression.Constant(factoryContext.ThrowOnBadRequest)) ) ) diff --git a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs index a51e5c4d5304..717ea63f7c01 100644 --- a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs +++ b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs @@ -173,12 +173,13 @@ public async Task FindBindAsyncMethod_FindsCorrectMethodOnClass() var parameter = new MockParameterInfo(type, "bindAsyncRecord"); var methodFound = cache.FindBindAsyncMethod(parameter); - Assert.NotNull(methodFound); + Assert.NotNull(methodFound.Item1); + Assert.Equal(2, methodFound.Item2); var parsedValue = Expression.Variable(type, "parsedValue"); var parseHttpContext = Expression.Lambda>>( - Expression.Block(new[] { parsedValue }, methodFound!), + Expression.Block(new[] { parsedValue }, methodFound.Item1!), ParameterBindingMethodCache.HttpContextExpr).Compile(); var httpContext = new DefaultHttpContext @@ -195,6 +196,37 @@ public async Task FindBindAsyncMethod_FindsCorrectMethodOnClass() Assert.Equal(new BindAsyncRecord(42), await parseHttpContext(httpContext)); } + [Fact] + public async Task FindBindAsyncMethod_FindsSingleArgBindAsync() + { + var type = typeof(BindAsyncSingleArgStruct); + var cache = new ParameterBindingMethodCache(); + var parameter = new MockParameterInfo(type, "bindAsyncSingleArgStruct"); + var methodFound = cache.FindBindAsyncMethod(parameter); + + Assert.NotNull(methodFound.Item1); + Assert.Equal(1, methodFound.Item2); + + var parsedValue = Expression.Variable(type, "parsedValue"); + + var parseHttpContext = Expression.Lambda>>( + Expression.Block(new[] { parsedValue }, methodFound.Item1!), + ParameterBindingMethodCache.HttpContextExpr).Compile(); + + var httpContext = new DefaultHttpContext + { + Request = + { + Headers = + { + ["ETag"] = "42", + }, + }, + }; + + Assert.Equal(new BindAsyncSingleArgStruct(42), await parseHttpContext(httpContext)); + } + public static IEnumerable BindAsyncParameterInfoData { get @@ -209,6 +241,14 @@ public static IEnumerable BindAsyncParameterInfoData { GetFirstParameter((BindAsyncStruct arg) => BindAsyncStructMethod(arg)), }, + new[] + { + GetFirstParameter((BindAsyncSingleArgRecord arg) => BindAsyncSingleArgRecordMethod(arg)), + }, + new[] + { + GetFirstParameter((BindAsyncSingleArgStruct arg) => BindAsyncSingleArgStructMethod(arg)), + } }; } } @@ -250,6 +290,11 @@ private static void BindAsyncStructMethod(BindAsyncStruct arg) { } private static void BindAsyncNullableStructMethod(BindAsyncStruct? arg) { } private static void NullableReturningBindAsyncStructMethod(NullableReturningBindAsyncStruct arg) { } + private static void BindAsyncSingleArgRecordMethod(BindAsyncSingleArgRecord arg) { } + private static void BindAsyncSingleArgStructMethod(BindAsyncSingleArgStruct arg) { } + private static void BindAsyncNullableSingleArgStructMethod(BindAsyncSingleArgStruct? arg) { } + private static void NullableReturningBindAsyncSingleArgStructMethod(NullableReturningBindAsyncSingleArgStruct arg) { } + private static ParameterInfo GetFirstParameter(Expression> expr) { var mc = (MethodCallExpression)expr.Body; @@ -324,6 +369,38 @@ private record struct NullableReturningBindAsyncStruct(int Value) throw new NotImplementedException(); } + private record BindAsyncSingleArgRecord(int Value) + { + public static ValueTask BindAsync(HttpContext context) + { + if (!int.TryParse(context.Request.Headers.ETag, out var val)) + { + return new(result: null); + } + + return new(result: new(val)); + } + } + + private record struct BindAsyncSingleArgStruct(int Value) + { + public static ValueTask BindAsync(HttpContext context) + { + if (!int.TryParse(context.Request.Headers.ETag, out var val)) + { + throw new BadHttpRequestException("The request is missing the required ETag header."); + } + + return new(result: new(val)); + } + } + + private record struct NullableReturningBindAsyncSingleArgStruct(int Value) + { + public static ValueTask BindAsync(HttpContext context, ParameterInfo parameter) => + throw new NotImplementedException(); + } + private class MockParameterInfo : ParameterInfo { public MockParameterInfo(Type type, string name) diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 16787a4c98fe..c71c5d53a77b 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -612,6 +612,54 @@ public static async ValueTask BindAsync(HttpContext co } } + private record struct MyBothBindAsyncStruct(Uri Uri) + { + public static ValueTask BindAsync(HttpContext context, ParameterInfo parameter) + { + Assert.True(parameter.ParameterType == typeof(MyBothBindAsyncStruct) || parameter.ParameterType == typeof(MyBothBindAsyncStruct?)); + Assert.Equal("myBothBindAsyncStruct", parameter.Name); + + if (!Uri.TryCreate(context.Request.Headers.Referer, UriKind.Absolute, out var uri)) + { + throw new BadHttpRequestException("The request is missing the required Referer header."); + } + + return new(result: new(uri)); + } + + // BindAsync with ParameterInfo is preferred + public static ValueTask BindAsync(HttpContext context) + { + throw new NotImplementedException(); + } + } + + private record struct MySimpleBindAsyncStruct(Uri Uri) + { + public static ValueTask BindAsync(HttpContext context) + { + if (!Uri.TryCreate(context.Request.Headers.Referer, UriKind.Absolute, out var uri)) + { + throw new BadHttpRequestException("The request is missing the required Referer header."); + } + + return new(result: new(uri)); + } + } + + private record MySimpleBindAsyncRecord(Uri Uri) + { + public static ValueTask BindAsync(HttpContext context) + { + if (!Uri.TryCreate(context.Request.Headers.Referer, UriKind.Absolute, out var uri)) + { + return new(result: null); + } + + return new(result: new(uri)); + } + } + [Theory] [MemberData(nameof(TryParsableParameters))] public async Task RequestDelegatePopulatesUnattributedTryParsableParametersFromRouteValue(Delegate action, string? routeValue, object? expectedParameterValue) @@ -724,6 +772,24 @@ public async Task RequestDelegateUsesBindAsyncOverTryParseGivenNullableStruct() Assert.Equal(new MyBindAsyncStruct(new Uri("https://example.org")), httpContext.Items["myBindAsyncStruct"]); } + [Fact] + public async Task RequestDelegateUsesParameterInfoBindAsyncOverOtherBindAsync() + { + var httpContext = CreateHttpContext(); + + httpContext.Request.Headers.Referer = "https://example.org"; + + var resultFactory = RequestDelegateFactory.Create((HttpContext httpContext, MyBothBindAsyncStruct? myBothBindAsyncStruct) => + { + httpContext.Items["myBothBindAsyncStruct"] = myBothBindAsyncStruct; + }); + + var requestDelegate = resultFactory.RequestDelegate; + await requestDelegate(httpContext); + + Assert.Equal(new MyBothBindAsyncStruct(new Uri("https://example.org")), httpContext.Items["myBothBindAsyncStruct"]); + } + [Fact] public async Task RequestDelegateUsesTryParseOverBindAsyncGivenExplicitAttribute() { @@ -873,7 +939,7 @@ void TestAction([FromRoute] int tryParsable, [FromRoute] int tryParsable2) [Fact] public async Task RequestDelegateLogsBindAsyncFailuresAndSets400Response() { - // Not supplying any headers will cause the HttpContext TryParse overload to fail. + // Not supplying any headers will cause the HttpContext BindAsync overload to return null. var httpContext = CreateHttpContext(); var invoked = false; @@ -905,7 +971,7 @@ public async Task RequestDelegateLogsBindAsyncFailuresAndSets400Response() [Fact] public async Task RequestDelegateLogsBindAsyncFailuresAndThrowsIfThrowOnBadRequest() { - // Not supplying any headers will cause the HttpContext TryParse overload to fail. + // Not supplying any headers will cause the HttpContext BindAsync overload to return null. var httpContext = CreateHttpContext(); var invoked = false; @@ -931,10 +997,72 @@ public async Task RequestDelegateLogsBindAsyncFailuresAndThrowsIfThrowOnBadReque Assert.Equal(400, badHttpRequestException.StatusCode); } + [Fact] + public async Task RequestDelegateLogsSingleArgBindAsyncFailuresAndSets400Response() + { + // Not supplying any headers will cause the HttpContext BindAsync overload to return null. + var httpContext = CreateHttpContext(); + var invoked = false; + + var factoryResult = RequestDelegateFactory.Create((MySimpleBindAsyncRecord mySimpleBindAsyncRecord1, + MySimpleBindAsyncRecord mySimpleBindAsyncRecord2) => + { + invoked = true; + }); + + var requestDelegate = factoryResult.RequestDelegate; + await requestDelegate(httpContext); + + Assert.False(invoked); + Assert.False(httpContext.RequestAborted.IsCancellationRequested); + Assert.Equal(400, httpContext.Response.StatusCode); + + var logs = TestSink.Writes.ToArray(); + + Assert.Equal(2, logs.Length); + + Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), logs[0].EventId); + Assert.Equal(LogLevel.Debug, logs[0].LogLevel); + Assert.Equal(@"Required parameter ""MySimpleBindAsyncRecord mySimpleBindAsyncRecord1"" was not provided from MySimpleBindAsyncRecord.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 ""MySimpleBindAsyncRecord mySimpleBindAsyncRecord2"" was not provided from MySimpleBindAsyncRecord.BindAsync(HttpContext).", logs[1].Message); + } + + [Fact] + public async Task RequestDelegateLogsSingleArgBindAsyncFailuresAndThrowsIfThrowOnBadRequest() + { + // Not supplying any headers will cause the HttpContext BindAsync overload to return null. + var httpContext = CreateHttpContext(); + var invoked = false; + + var factoryResult = RequestDelegateFactory.Create((MySimpleBindAsyncRecord mySimpleBindAsyncRecord1, + MySimpleBindAsyncRecord mySimpleBindAsyncRecord2) => + { + invoked = true; + }, new() { ThrowOnBadRequest = true }); + + var requestDelegate = factoryResult.RequestDelegate; + var badHttpRequestException = await Assert.ThrowsAsync(() => requestDelegate(httpContext)); + + Assert.False(invoked); + + // The httpContext should be untouched. + Assert.False(httpContext.RequestAborted.IsCancellationRequested); + Assert.Equal(200, httpContext.Response.StatusCode); + Assert.False(httpContext.Response.HasStarted); + + // We don't log bad requests when we throw. + Assert.Empty(TestSink.Writes); + + Assert.Equal(@"Required parameter ""MySimpleBindAsyncRecord mySimpleBindAsyncRecord1"" was not provided from MySimpleBindAsyncRecord.BindAsync(HttpContext).", badHttpRequestException.Message); + Assert.Equal(400, badHttpRequestException.StatusCode); + } + [Fact] public async Task BindAsyncExceptionsAreUncaught() { - // Not supplying any headers will cause the HttpContext BindAsync overload to fail. var httpContext = CreateHttpContext(); var factoryResult = RequestDelegateFactory.Create((MyBindAsyncTypeThatThrows arg1) => { }); @@ -2239,6 +2367,10 @@ void nullableReferenceType(HttpContext context, MyBindAsyncRecord? myBindAsyncRe { context.Items["uri"] = myBindAsyncRecord?.Uri; } + void requiredReferenceTypeSimple(HttpContext context, MySimpleBindAsyncRecord mySimpleBindAsyncRecord) + { + context.Items["uri"] = mySimpleBindAsyncRecord.Uri; + } void requiredValueType(HttpContext context, MyNullableBindAsyncStruct myNullableBindAsyncStruct) @@ -2253,11 +2385,16 @@ void nullableValueType(HttpContext context, MyNullableBindAsyncStruct? myNullabl { context.Items["uri"] = myNullableBindAsyncStruct?.Uri; } + void requiredValueTypeSimple(HttpContext context, MySimpleBindAsyncStruct mySimpleBindAsyncStruct) + { + context.Items["uri"] = mySimpleBindAsyncStruct.Uri; + } return new object?[][] { new object?[] { (Action)requiredReferenceType, false, true, false }, new object?[] { (Action)requiredReferenceType, true, false, false, }, + new object?[] { (Action)requiredReferenceTypeSimple, true, false, false }, new object?[] { (Action)defaultReferenceType, false, false, false, }, new object?[] { (Action)defaultReferenceType, true, false, false }, @@ -2267,6 +2404,7 @@ void nullableValueType(HttpContext context, MyNullableBindAsyncStruct? myNullabl new object?[] { (Action)requiredValueType, false, true, true }, new object?[] { (Action)requiredValueType, true, false, true }, + new object?[] { (Action)requiredValueTypeSimple, true, false, true }, new object?[] { (Action)defaultValueType, false, false, true }, new object?[] { (Action)defaultValueType, true, false, true }, diff --git a/src/Shared/ParameterBindingMethodCache.cs b/src/Shared/ParameterBindingMethodCache.cs index f546d0054fb0..fb8fc88164fe 100644 --- a/src/Shared/ParameterBindingMethodCache.cs +++ b/src/Shared/ParameterBindingMethodCache.cs @@ -26,7 +26,7 @@ internal sealed class ParameterBindingMethodCache // Since this is shared source, the cache won't be shared between RequestDelegateFactory and the ApiDescriptionProvider sadly :( private readonly ConcurrentDictionary?> _stringMethodCallCache = new(); - private readonly ConcurrentDictionary?> _bindAsyncMethodCallCache = new(); + private readonly ConcurrentDictionary?, int)> _bindAsyncMethodCallCache = new(); // If IsDynamicCodeSupported is false, we can't use the static Enum.TryParse since there's no easy way for // this code to generate the specific instantiation for any enums used @@ -47,7 +47,7 @@ public bool HasTryParseMethod(ParameterInfo parameter) } public bool HasBindAsyncMethod(ParameterInfo parameter) => - FindBindAsyncMethod(parameter) is not null; + FindBindAsyncMethod(parameter).Item1 is not null; public Func? FindTryParseMethod(Type type) { @@ -128,12 +128,18 @@ public bool HasBindAsyncMethod(ParameterInfo parameter) => return _stringMethodCallCache.GetOrAdd(type, Finder); } - public Expression? FindBindAsyncMethod(ParameterInfo parameter) + public (Expression?, int) FindBindAsyncMethod(ParameterInfo parameter) { - static Func? Finder(Type nonNullableParameterType) + static (Func?, int) Finder(Type nonNullableParameterType) { + var hasParameterInfo = true; // There should only be one BindAsync method with these parameters since C# does not allow overloading on return type. var methodInfo = nonNullableParameterType.GetMethod("BindAsync", BindingFlags.Public | BindingFlags.Static, new[] { typeof(HttpContext), typeof(ParameterInfo) }); + if (methodInfo is null) + { + hasParameterInfo = false; + methodInfo = nonNullableParameterType.GetMethod("BindAsync", BindingFlags.Public | BindingFlags.Static, new[] { typeof(HttpContext) }); + } // We're looking for a method with the following signatures: // public static ValueTask<{type}> BindAsync(HttpContext context, ParameterInfo parameter) @@ -147,34 +153,51 @@ public bool HasBindAsyncMethod(ParameterInfo parameter) => // ValueTask<{type}>? if (valueTaskResultType == nonNullableParameterType) { - return (parameter) => + return ((parameter) => { - // parameter is being intentionally shadowed. We never want to use the outer ParameterInfo inside - // this Func because the ParameterInfo varies after it's been cached for a given parameter type. - var typedCall = Expression.Call(methodInfo, HttpContextExpr, Expression.Constant(parameter)); + MethodCallExpression typedCall; + if (hasParameterInfo) + { + // parameter is being intentionally shadowed. We never want to use the outer ParameterInfo inside + // this Func because the ParameterInfo varies after it's been cached for a given parameter type. + typedCall = Expression.Call(methodInfo, HttpContextExpr, Expression.Constant(parameter)); + } + else + { + typedCall = Expression.Call(methodInfo, HttpContextExpr); + } return Expression.Call(ConvertValueTaskMethod.MakeGenericMethod(nonNullableParameterType), typedCall); - }; + }, hasParameterInfo ? 2 : 1); } // ValueTask>? else if (valueTaskResultType.IsGenericType && valueTaskResultType.GetGenericTypeDefinition() == typeof(Nullable<>) && valueTaskResultType.GetGenericArguments()[0] == nonNullableParameterType) { - return (parameter) => + return ((parameter) => { - // parameter is being intentionally shadowed. We never want to use the outer ParameterInfo inside - // this Func because the ParameterInfo varies after it's been cached for a given parameter type. - var typedCall = Expression.Call(methodInfo, HttpContextExpr, Expression.Constant(parameter)); + MethodCallExpression typedCall; + if (hasParameterInfo) + { + // parameter is being intentionally shadowed. We never want to use the outer ParameterInfo inside + // this Func because the ParameterInfo varies after it's been cached for a given parameter type. + typedCall = Expression.Call(methodInfo, HttpContextExpr, Expression.Constant(parameter)); + } + else + { + typedCall = Expression.Call(methodInfo, HttpContextExpr); + } return Expression.Call(ConvertValueTaskOfNullableResultMethod.MakeGenericMethod(nonNullableParameterType), typedCall); - }; + }, hasParameterInfo ? 2 : 1); } } - return null; + return (null, 0); } var nonNullableParameterType = Nullable.GetUnderlyingType(parameter.ParameterType) ?? parameter.ParameterType; - return _bindAsyncMethodCallCache.GetOrAdd(nonNullableParameterType, Finder)?.Invoke(parameter); + var (method, paramCount) = _bindAsyncMethodCallCache.GetOrAdd(nonNullableParameterType, Finder); + return (method?.Invoke(parameter), paramCount); } private static MethodInfo GetEnumTryParseMethod(bool preferNonGenericEnumParseOverload) From 557a2341439cdb332a36d7b3548b8f68f7b74cbb Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 14 Sep 2021 19:56:55 -0700 Subject: [PATCH 2/4] Update src/Shared/ParameterBindingMethodCache.cs Co-authored-by: David Fowler --- src/Shared/ParameterBindingMethodCache.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Shared/ParameterBindingMethodCache.cs b/src/Shared/ParameterBindingMethodCache.cs index fb8fc88164fe..2b266092f8db 100644 --- a/src/Shared/ParameterBindingMethodCache.cs +++ b/src/Shared/ParameterBindingMethodCache.cs @@ -47,7 +47,8 @@ public bool HasTryParseMethod(ParameterInfo parameter) } public bool HasBindAsyncMethod(ParameterInfo parameter) => - FindBindAsyncMethod(parameter).Item1 is not null; + var (expression, _) = FindBindAsyncMethod(parameter); + return expression is not null; public Func? FindTryParseMethod(Type type) { From 6ffd3c0751cc8a15a9fc66f934e58e4fed1dab61 Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 14 Sep 2021 20:04:17 -0700 Subject: [PATCH 3/4] Apply suggestions from code review --- src/Shared/ParameterBindingMethodCache.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Shared/ParameterBindingMethodCache.cs b/src/Shared/ParameterBindingMethodCache.cs index 2b266092f8db..c9dbc6e1bcad 100644 --- a/src/Shared/ParameterBindingMethodCache.cs +++ b/src/Shared/ParameterBindingMethodCache.cs @@ -46,9 +46,11 @@ public bool HasTryParseMethod(ParameterInfo parameter) return FindTryParseMethod(nonNullableParameterType) is not null; } - public bool HasBindAsyncMethod(ParameterInfo parameter) => + public bool HasBindAsyncMethod(ParameterInfo parameter) + { var (expression, _) = FindBindAsyncMethod(parameter); return expression is not null; + } public Func? FindTryParseMethod(Type type) { From eb2695298847fba298b003c3e212008e268c4acd Mon Sep 17 00:00:00 2001 From: Brennan Date: Wed, 15 Sep 2021 15:12:30 -0700 Subject: [PATCH 4/4] named tuple --- .../Http.Extensions/src/RequestDelegateFactory.cs | 8 ++++---- .../test/ParameterBindingMethodCacheTests.cs | 12 ++++++------ src/Shared/ParameterBindingMethodCache.cs | 9 +++------ 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index dbaeb5d5fcdd..9355e5f0da4c 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -840,12 +840,12 @@ private static Expression BindParameterFromBindAsync(ParameterInfo parameter, Fa var isOptional = IsOptionalParameter(parameter, factoryContext); // Get the BindAsync method for the type. - var (bindAsyncExpression, paramCount) = ParameterBindingMethodCache.FindBindAsyncMethod(parameter); + var bindAsyncMethod = ParameterBindingMethodCache.FindBindAsyncMethod(parameter); // We know BindAsync exists because there's no way to opt-in without defining the method on the type. - Debug.Assert(bindAsyncExpression is not null); + Debug.Assert(bindAsyncMethod.Expression is not null); // Compile the delegate to the BindAsync method for this parameter index - var bindAsyncDelegate = Expression.Lambda>>(bindAsyncExpression, HttpContextExpr).Compile(); + var bindAsyncDelegate = Expression.Lambda>>(bindAsyncMethod.Expression, HttpContextExpr).Compile(); factoryContext.ParameterBinders.Add(bindAsyncDelegate); // boundValues[index] @@ -854,7 +854,7 @@ private static Expression BindParameterFromBindAsync(ParameterInfo parameter, Fa if (!isOptional) { var typeName = TypeNameHelper.GetTypeDisplayName(parameter.ParameterType, fullName: false); - var message = paramCount == 2 ? $"{typeName}.BindAsync(HttpContext, ParameterInfo)" : $"{typeName}.BindAsync(HttpContext)"; + var message = bindAsyncMethod.ParamCount == 2 ? $"{typeName}.BindAsync(HttpContext, ParameterInfo)" : $"{typeName}.BindAsync(HttpContext)"; var checkRequiredBodyBlock = Expression.Block( Expression.IfThen( Expression.Equal(boundValueExpr, Expression.Constant(null)), diff --git a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs index 717ea63f7c01..80edc7b2a3fd 100644 --- a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs +++ b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs @@ -173,13 +173,13 @@ public async Task FindBindAsyncMethod_FindsCorrectMethodOnClass() var parameter = new MockParameterInfo(type, "bindAsyncRecord"); var methodFound = cache.FindBindAsyncMethod(parameter); - Assert.NotNull(methodFound.Item1); - Assert.Equal(2, methodFound.Item2); + Assert.NotNull(methodFound.Expression); + Assert.Equal(2, methodFound.ParamCount); var parsedValue = Expression.Variable(type, "parsedValue"); var parseHttpContext = Expression.Lambda>>( - Expression.Block(new[] { parsedValue }, methodFound.Item1!), + Expression.Block(new[] { parsedValue }, methodFound.Expression!), ParameterBindingMethodCache.HttpContextExpr).Compile(); var httpContext = new DefaultHttpContext @@ -204,13 +204,13 @@ public async Task FindBindAsyncMethod_FindsSingleArgBindAsync() var parameter = new MockParameterInfo(type, "bindAsyncSingleArgStruct"); var methodFound = cache.FindBindAsyncMethod(parameter); - Assert.NotNull(methodFound.Item1); - Assert.Equal(1, methodFound.Item2); + Assert.NotNull(methodFound.Expression); + Assert.Equal(1, methodFound.ParamCount); var parsedValue = Expression.Variable(type, "parsedValue"); var parseHttpContext = Expression.Lambda>>( - Expression.Block(new[] { parsedValue }, methodFound.Item1!), + Expression.Block(new[] { parsedValue }, methodFound.Expression!), ParameterBindingMethodCache.HttpContextExpr).Compile(); var httpContext = new DefaultHttpContext diff --git a/src/Shared/ParameterBindingMethodCache.cs b/src/Shared/ParameterBindingMethodCache.cs index c9dbc6e1bcad..1d605195c74a 100644 --- a/src/Shared/ParameterBindingMethodCache.cs +++ b/src/Shared/ParameterBindingMethodCache.cs @@ -46,11 +46,8 @@ public bool HasTryParseMethod(ParameterInfo parameter) return FindTryParseMethod(nonNullableParameterType) is not null; } - public bool HasBindAsyncMethod(ParameterInfo parameter) - { - var (expression, _) = FindBindAsyncMethod(parameter); - return expression is not null; - } + public bool HasBindAsyncMethod(ParameterInfo parameter) => + FindBindAsyncMethod(parameter).Expression is not null; public Func? FindTryParseMethod(Type type) { @@ -131,7 +128,7 @@ public bool HasBindAsyncMethod(ParameterInfo parameter) return _stringMethodCallCache.GetOrAdd(type, Finder); } - public (Expression?, int) FindBindAsyncMethod(ParameterInfo parameter) + public (Expression? Expression, int ParamCount) FindBindAsyncMethod(ParameterInfo parameter) { static (Func?, int) Finder(Type nonNullableParameterType) {