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

Add Results.Empty #40168

Closed
davidfowl opened this issue Feb 12, 2022 · 5 comments
Closed

Add Results.Empty #40168

davidfowl opened this issue Feb 12, 2022 · 5 comments
Labels
api-approved API was approved in API review, it can be implemented area-web-frameworks
Milestone

Comments

@davidfowl
Copy link
Member

davidfowl commented Feb 12, 2022

Background and Motivation

Sometimes you want to return a result that does not mutate the response in anyway. This happens when you have multiple branches in a route handle and one of them should be a noop. You are forced to return an IResult and we don't have a built in result type that fills this role. Mvc has EmptyResult for this purpose.

Proposed API

namespace Microsoft.AspNetCore.Http
{
    public static class Results
    {
        public static IResult Empty(); // This could also be a property
    }
}

Usage Examples

app.MapGet("/{id}", (HttpContext context, string id) =>
{
    var httpProcessor = GetTargetStream(id);
    if (httpProcessor is null) return Results.NotFound();
 
    await httpProcessor.ProcessAsync(context);
     // Don't change what the httpProcessor did
    return Results.Empty();
});

Alternative Designs

Force people to write their own.

Risks

None

@davidfowl davidfowl added api-suggestion Early API idea and discussion, it is NOT ready for implementation api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Feb 12, 2022
@ghost
Copy link

ghost commented Feb 12, 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.

@ghost
Copy link

ghost commented Feb 14, 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.

@pranavkm
Copy link
Contributor

API review:

We'll use properties instead. Here's the approved API:

public static class Results
{
+   public static IResult Empty { get; } = new EmptyResult();
}

public class ControllerBase
{
+    public static EmptyResult Empty { get; } = new ();
}

@pranavkm pranavkm added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Feb 28, 2022
@ShreyasJejurkar
Copy link
Contributor

Hi @davidfowl @pranavkm, do we need to address this for MVC as well? I mean in property in ControllerBase??

@davidfowl
Copy link
Member Author

Yep

pranavkm pushed a commit that referenced this issue Mar 4, 2022
* feat (MVC) : Add Empty property to `ControllerBase`

Contributes to #40168
@dotnet dotnet locked as resolved and limited conversation to collaborators Apr 4, 2022
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-web-frameworks
Projects
None yet
Development

No branches or pull requests

3 participants