Added support for type based parameter binding (#35496) #35535
Conversation
* 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.
davidfowl
requested review from
BrennanConroy,
javiercn,
Pilchie,
pranavkm and
Tratcher
as code owners
| var isOptional = parameter.HasDefaultValue || nullability.ReadState == NullabilityState.Nullable; | ||
|
|
||
| // Get the BindAsync method | ||
| var body = TryParseMethodCache.FindBindAsyncMethod(parameter.ParameterType)!; |
There was a problem hiding this comment.
Super small nit, but the name is no longer an accurate reflection of what's being cached. BinderMethodCache or something like that?
There was a problem hiding this comment.
In .NET 7. This shared source is painful.
| @@ -499,6 +499,53 @@ public static bool TryParse(string? value, out MyTryParsableRecord? result) | |||
| } | |||
| } | |||
|
|
|||
| private class MyBindAsyncTypeThatThrows | |||
| { | |||
| public static ValueTask<object?> BindAsync(HttpContext context) | |||
There was a problem hiding this comment.
What's the behavior like if the signature does not match? e.g. ValueTask BindAsync(HttpContext context)?
There was a problem hiding this comment.
It won't find it sorta like TryParse today and will fall through to the other things.
There was a problem hiding this comment.
I could fail if it found the method but had the wrong return type?
There was a problem hiding this comment.
Toss it in the Analyzer pile?
There was a problem hiding this comment.
Can we just look for ValueTask<{type}> in FindBindAsyncMethod? What's the benefit of allowing any object to be returned? I get that we're going to box anyway for structs, but might as well lean on the type-safety checks of the compiler.
We could even throw at startup if a parameter type has a static BindAsync(HttpContext) method with the wrong return type.
davidfowl
added
feature-minimal-actions
|
cc @wtgodbe |
ghost
added this to the 
amcasey
added
the
area-minimal
Customer Impact
We've had several requests to enable MVC style model binding for minimal APIs, for both framework authors and customers who have written basic model binders for various types. We're enabling typed based modeling binding to unlock these scenarios so we can get feedback for .NET 7.0 for the overall design
Testing
Unit testing, lots of it.
Risk
Low