Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce interface with static abstract BindAsync method for custom bound parameters of route handler delegates #40927

Closed
DamianEdwards opened this issue Mar 28, 2022 · 4 comments · Fixed by #41100
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels blocked The work on this issue is blocked due to some dependency feature-minimal-actions Controller-like actions for endpoint routing
Milestone

Comments

@DamianEdwards
Copy link
Member

DamianEdwards commented Mar 28, 2022

C# 11 introduces support for static abstract interface members. We should update the support for custom parameter binding via a static BindAsync method in Minimal APIs to discover and invoke the BindAsync method via a newly defined interface (see below). We would continue to support the existing discovery and invocation via reflection approach we introduced in .NET 6 to maintain backwards compatibility.

public interface IBindableFromHttpContext<TSelf>
    where TSelf : class, IBindableFromHttpContext<TSelf>
{
    static abstract ValueTask<TSelf?> BindAsync(HttpContext context, ParameterInfo parameter);
}

I welcome additional interface name suggestions 😄

@DamianEdwards DamianEdwards added feature-minimal-actions Controller-like actions for endpoint routing area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Mar 28, 2022
@DamianEdwards DamianEdwards added this to the .NET 7 Planning milestone Mar 28, 2022
@ghost
Copy link

ghost commented Mar 28, 2022

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@DamianEdwards DamianEdwards removed this from the .NET 7 Planning milestone Mar 29, 2022
@rafikiassumani-msft rafikiassumani-msft added this to the .NET 7 Planning milestone Mar 29, 2022
@ghost
Copy link

ghost commented Mar 29, 2022

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@DamianEdwards DamianEdwards added the blocked The work on this issue is blocked due to some dependency label Apr 7, 2022
@DamianEdwards DamianEdwards linked a pull request Apr 8, 2022 that will close this issue
@DamianEdwards DamianEdwards self-assigned this Apr 8, 2022
@DamianEdwards DamianEdwards added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Apr 8, 2022
@ghost
Copy link

ghost commented Apr 8, 2022

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@halter73
Copy link
Member

API Review Notes:

  • Let's use the Http.Abstractions namespace
  • Add class constraint from PR
namespaces Microsoft.AspNetCore.Http.Abstractions;

public interface IBindableFromHttpContext<TSelf>
    where TSelf : class, IBindableFromHttpContext<TSelf>
{
    static abstract ValueTask<TSelf?> BindAsync(HttpContext context, ParameterInfo parameter);
}

@halter73 halter73 added the api-approved API was approved in API review, it can be implemented label Apr 18, 2022
@DamianEdwards DamianEdwards removed the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Apr 18, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 11, 2022
@amcasey amcasey added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels blocked The work on this issue is blocked due to some dependency feature-minimal-actions Controller-like actions for endpoint routing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants