dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.46k stars 10.03k forks source link

Make IResult concrete types public #40656

Closed brunolins16 closed 2 years ago

brunolins16 commented 2 years ago

Background and Motivation

Today, all Http Results are internal sealed types and the only public parts are:

  1. Results static class that has utility methods for creation of all Result Types
  2. IResult interface that only expose the ExecuteAsync method.

Because it is extremely minimalist, it became far more complicated to create a simple test like this:

    [Fact]
    public void GetByIdTest()
    {
        //Arrange
        var id = 1;

        //Act
        var result = TodoEndpoints.GetTodoById(id);

        //Assert
        var okResult = Assert.IsAssignableFrom<OkObjectResult>(result);
        var foundTodo = Assert.IsAssignableFrom<Models.Todo>(okResult.Value);
        Assert.Equal(id, foundTodo.Id);
    }

The issue #37502 reported the problem and describe a current working complext Unit test sample, that basically executes the result and checks the Mock<HttpContext> information.

Proposed API

This proposal is following these general ideas:

  1. Scoped to make endpoints more testable, however, making all types public will open opportunities for different usages
  2. Named all public Result types with the HttpResult suffix
  3. All constructors will be internal and Result creation must be from Results static class
  4. Introducing interfaces to allow easier naming/typing consistency across types but they are an optional in the proposal.

Interfaces

+namespace Microsoft.AspNetCore.Http;

+/// <summary>
+/// Defines a contract that represents the result of an HTTP endpoint
+/// that contains a <see cref="StatusCode"/>.
+/// </summary>
+public interface IStatusCodeHttpResult : IResult
+{
+    int? StatusCode { get; }
+}

+/// <summary>
+/// Defines a contract that represents the result of an HTTP endpoint
+/// that contains an object <see cref="Value"/> and <see cref="StatusCode"/>.
+/// </summary>
+public interface IObjectHttpResult : IResult, IStatusCodeHttpResult
+{
+    object? Value { get; }
+}

+/// <summary>
+/// Defines a contract that represents the result of an HTTP result endpoint
+/// that constains an object route.
+/// </summary>
+public interface IAtRouteHttpResult : IResult
+{
+    string? RouteName { get; }
+    RouteValueDictionary? RouteValues { get; }
+}

+/// <summary>
+/// Defines a contract that represents the result of an HTTP result endpoint
+/// that constains an <see cref="Location"/>.
+/// </summary>
+public interface IAtLocationHttpResult : IResult
+{
+    string? Location { get; }
+}

+/// <summary>
+/// Defines a contract that represents the HTTP Redirect result of an HTTP result endpoint.
+/// </summary>
+public interface IRedirectHttpResult : IResult
+{
+    bool Permanent { get; }
+    bool PreserveMethod { get; }
+    string? Url { get; }
+    bool AcceptLocalUrlOnly { get; }
+}

+/// <summary>
+/// Defines a contract that represents the file result of an HTTP result endpoint.
+/// </summary>
+public interface IFileHttpResult : IResult
+{
+    string ContentType { get; }
+    string? FileDownloadName { get; }
+    DateTimeOffset? LastModified { get; }
+    EntityTagHeaderValue? EntityTag { get; }
+    bool EnableRangeProcessing { get; }
+    long? FileLength { get; }
+}

IResult that when executed will do nothing

+namespace Microsoft.AspNetCore.Http;

+public sealed class EmptyHttpResult : IResult
+{
+}

An IResult that when executed will produce a response with content

+namespace Microsoft.AspNetCore.Http;

+public sealed partial class ContentHttpResult : IResult, IStatusCodeHttpResult
+{
+   public string? Content { get; }
+   public int? StatusCode { get; }
+   public string? ContentType { get; }
+}

An IResult that when executed will produce a JSON response

+namespace Microsoft.AspNetCore.Http;

+public sealed class JsonHttpResult : IResult, IObjectHttpResult, IStatusCodeHttpResult
+{
+   public string? Content { get; }
+   public int? StatusCode { get; }
+   public string? ContentType { get; }
+   public JsonSerializerOptions? JsonSerializerOptions { get; }
+}

An IResult that on execution will write an object to the response with the Ok (200) status code

+namespace Microsoft.AspNetCore.Http;

+public sealed class OkObjectHttpResult : IResult, IObjectHttpResult, IStatusCodeHttpResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

An IResult that on execution will write an object to the response with Created (201) status code and Location header

+namespace Microsoft.AspNetCore.Http;

+public sealed class CreatedHttpResult : IResult, IObjectHttpResult, IStatusCodeHttpResult, IAtLocationHttpResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public string? Location { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

An IResult that on execution will write an object to the response with Created (201) status code and Location header to a registered route location

+namespace Microsoft.AspNetCore.Http;

+public sealed class CreatedAtRouteHttpResult : IResult, IObjectHttpResult, IAtRouteHttpResult, IStatusCodeHttpResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public string? RouteName { get; }
+   public RouteValueDictionary? RouteValues { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

An IResult that on execution will write an object to the response with Accepted (202) status code and Location header

+namespace Microsoft.AspNetCore.Http;

+public sealed class AcceptedHttpResult : IResult, IObjectHttpResult, IStatusCodeHttpResult, IAtLocationHttpResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public string? Location { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

An IResult that on execution will write an object to the response with Accepted (202) status code and Location header to a registered route location

+namespace Microsoft.AspNetCore.Http;

+public sealed class AcceptedHttpResult : IResult, IObjectHttpResult, IStatusCodeHttpResult, IAtLocationHttpResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public string? RouteName { get; }
+   public RouteValueDictionary? RouteValues { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

An IResult that on execution will produces response with the No Content (204) status code

+namespace Microsoft.AspNetCore.Http;

+public class NoContentHttpResult : IResult, IStatusCodeHttpResult
+{
+   public int? StatusCode { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

An IResult that returns a Found (302), Moved Permanently (301), Temporary Redirect (307) or Permanent Redirect (308) response with a Location header to a registered route location

+namespace Microsoft.AspNetCore.Http;

+public sealed partial class RedirectToRouteHttpResult : IResult, IRedirectHttpResult, IAtRouteHttpResult
+{
+   public string? Fragment { get; }
+   public string? RouteName { get; }
+   public RouteValueDictionary? RouteValues { get; }
+   public bool Permanent { get; }
+   public bool PreserveMethod { get; }
+   public bool AcceptLocalUrlOnly { get; }
+   public string? Url { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

An IResult that returns a Found (302), Moved Permanently (301), Temporary Redirect (307) or Permanent Redirect (308) response with a Location header to the supplied URL

+namespace Microsoft.AspNetCore.Http;

+public sealed partial class RedirectHttpResult : IResult, IRedirectHttpResult
+{
+   public bool AcceptLocalUrlOnly { get; }
+   public bool Permanent { get; }
+   public bool PreserveMethod { get; }
+   public string? Url { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

An IResult that on execution will write an object to the response with the Bad Request (400) status code

+namespace Microsoft.AspNetCore.Http;

+public sealed class BadRequestObjectHttpResult : IResult, IObjectHttpResult, IStatusCodeHttpResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

An IResult that on execution will produces response with the Unauthorized (401) status code

+namespace Microsoft.AspNetCore.Http;

+public sealed class UnauthorizedHttpResult : IResult, IStatusCodeHttpResult
+{
+   public int? StatusCode { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

An IResult that on execution will write an object to the response with the Not Found (404) status code

+namespace Microsoft.AspNetCore.Http;

+public sealed class NotFoundObjectHttpResult : IResult, IObjectHttpResult, IStatusCodeHttpResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

An IResult that on execution will write an object to the response with the Conflict (409) status code

+namespace Microsoft.AspNetCore.Http;

+public sealed class ConflictObjectHttpResult : IResult, IObjectHttpResult, IStatusCodeHttpResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

An IResult that on execution will write an object to the response with the Unprocessable Entity (422) status code

+namespace Microsoft.AspNetCore.Http;

+public sealed class UnprocessableEntityObjectHttpResult : IResult, IObjectHttpResult, IStatusCodeHttpResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

An IResult that on execution will write Problem Details HTTP API responses based on https://tools.ietf.org/html/rfc7807

+namespace Microsoft.AspNetCore.Http;

+public sealed class ProblemHttpResult : IResult, IObjectHttpResult
+{
+   public ProblemDetails ProblemDetails { get; }
+   public string ContentType { get ;}
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

An IResult that on execution invokes HttpContext.ChallengeAsync

+namespace Microsoft.AspNetCore.Http;

+public sealed partial class ChallengeHttpResult : IResult
+{
+   public IReadOnlyList<string> AuthenticationSchemes { get; }
+   public AuthenticationProperties? Properties { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

An IResult that on execution invokes HttpContext.SignOutAsync

+namespace Microsoft.AspNetCore.Http;

+public sealed partial class SignOutHttpResult : IResult
+{
+   public IReadOnlyList<string> AuthenticationSchemes { get; }
+   public AuthenticationProperties? Properties { get;  }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

An IResult that on execution invokes HttpContext.SignInAsync

+namespace Microsoft.AspNetCore.Http;

+public sealed partial class SignInHttpResult : IResult
+{
+   public string? AuthenticationScheme { get; }
+   public AuthenticationProperties? Properties { get;  }
+   public ClaimsPrincipal Principal { get;  }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

An IResult that on execution invokes HttpContext.ForbidAsync

+namespace Microsoft.AspNetCore.Http;

+public sealed partial class ForbidHttpResult : IResult
+{
+   public IReadOnlyList<string> AuthenticationSchemes { get; }
+   public AuthenticationProperties? Properties { get;  }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

An IResult that on execution writes the file specified using a virtual path to the response

+namespace Microsoft.AspNetCore.Http;

+public sealed class VirtualFileHttpResult : IResult, IFileHttpResult
+{
+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public string FileName { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

An IResult that on execution will write a file from disk to the response

+namespace Microsoft.AspNetCore.Http;

+public sealed class PhysicalFileHttpResult : IResult, IFileHttpResult
+{
+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public string FileName { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

An IResult that when executed will write a file from a stream to the response

+namespace Microsoft.AspNetCore.Http;

+public sealed class FileStreamHttpResult : IResult, IFileHttpResult
+{
+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public Stream FileStream { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

An IResult that when executed will write a file from the writer callback to the response

+namespace Microsoft.AspNetCore.Http;

+public sealed class PushStreamHttpResult : IResult, IFileHttpResult
+{
+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

An IResult that when executed will write a file from the content to the response to the response

+namespace Microsoft.AspNetCore.Http;

+public sealed class FileContentHttpResult : IResult, IFileHttpResult
+{
+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public ReadOnlyMemory<byte> FileContents { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

Usage Examples

The new types could be used for unit testing Minimal API endpoints. Eg.:

    [Fact]
    public void GetByIdTest()
    {
        //Arrange
        var id = 1;

        //Act
        var result = (OkObjectHttpResult)TodoEndpoints.GetTodoById(id);

        //Assert
        var foundTodo = Assert.IsAssignableFrom<Models.Todo>(result.Value);
        Assert.Equal(200, result.StatusCode);
        Assert.Equal(id, foundTodo.Id);
    }

    [Fact]
    public void GetByIdTest_NotFound_UsingInterface()
    {
        //Arrange
        var id = 1;

        //Act
        var result = (IStatusCodeHttpResult)TodoEndpoints.GetTodoById(id);

        //Assert
        Assert.Equal(404, result.StatusCode);
    }

    [Fact]
    public void CreateTodo_WithValidationProblems()
    {
        //Arrange
        var newTodo = default(Todo);

        //Act
        var result = TodoEndpoints.CreateTodo(newTodo);

        //Assert        
        var problemResult = Assert.IsAssignableFrom<ProblemHttpResult>(result);
        Assert.NotNull(problemResult.ProblemDetails);
        Assert.Equal(400, problemResult.StatusCode);
    }

Alternative Designs

Since Microsoft.AspNetCore.Http is an auto-imported global namespace for all apps targeting the WebSDK, we could think about using a different namespace.

Here are few options:

Risks

The proposal is to expose the minimal as possible of the implementation, however, once the types became public we will not have the flexibility to change them anymore.

pranavkm commented 2 years ago
- public List<string> AuthenticationSchemes { get; init; }
+ public IReadOnlyList<string> AuthenticationSchemes { get; }
brunolins16 commented 2 years ago

@pranavkm, thanks for taking a look at this :)

  • Do you anticipate users wanting to derive from these newly exposed result types? If not, you could consider uniformly sealing them. If you really want to be super strict about it, you could make their ctors non-public and declare Results.XXX is the only factory to create these types. This leaves the door open to explore object pooling as a way to avoid allocating these.

The idea is to make the ctors non-public, the only api suggestion is the listed in the issue, nothing else, like default parameterless ctors. Unfortunately, i cannot make all of them sealed because few of them, eg. ObjectHttpResult and StatusCodeHttpResult, are part base class of the internal Result types.

  • Continuing on this - if the intent is to be test-focused, the setters / init-ers could be made non-public and the date types hardened to be read-only e.g.
- public List<string> AuthenticationSchemes { get; init; }
+ public IReadOnlyList<string> AuthenticationSchemes { get; }

That is true, i missed to make the setter/ init-ers non-public in the proposal but I have them in my branch, I will update the proposal. I am sure not if change the data type will be possible, but I can take a look.

  • Microsoft.AspNetCore.Http is an auto-imported global namespace for all apps targeting the WebSDK. It would be worthwhile considering a sub-namespace to host these types.

Good point and I will let it to be discussed during the review meeting.

pranavkm commented 2 years ago

Unfortunately, i cannot make all of them sealed because few of them, eg. ObjectHttpResult and StatusCodeHttpResult, are part base class of the internal Result types.

Unless you'd like to preserve this hierarchy for some reason (result filters are a possible reason), you could refactor this to not have a hierarchy. The hierarchy exists as a way to implementation.

brunolins16 commented 2 years ago

Unfortunately, i cannot make all of them sealed because few of them, eg. ObjectHttpResult and StatusCodeHttpResult, are part base class of the internal Result types.

Unless you'd like to preserve this hierarchy for some reason (result filters are a possible reason), you could refactor this to not have a hierarchy. The hierarchy exists as a way to implementation.

I kind of like the hierarchy but I don't see a reason to keep, other than code reuse that we can achieve with a different implementation. Also, I was reviewing this #40672, and I changed a little bit my proposal and removed all the hierarchy.

ghost commented 2 years ago

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:

halter73 commented 2 years ago

API Review

+namespace Microsoft.AspNetCore.Http;

+public sealed class EmptyHttpResult : IResult
+{
+}

+public sealed partial class ContentHttpResult : IResult
+{
+   public string? Content { get; }
+   public int? StatusCode { get; }
+   public string? ContentType { get; }
+}

+public sealed class JsonHttpResult : IResult
+{
+   public string? Content { get; }
+   public int? StatusCode { get; }
+   public string? ContentType { get; }
+   public JsonSerializerOptions? JsonSerializerOptions { get; }
+}

+public sealed class OkObjectHttpResult : IResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class CreatedHttpResult : IResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public string? Location { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class CreatedAtRouteHttpResult : IResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public string? RouteName { get; }
+   public RouteValueDictionary? RouteValues { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class AcceptedHttpResult : IResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public string? Location { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class AcceptedHttpResult : IResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public string? RouteName { get; }
+   public RouteValueDictionary? RouteValues { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public class NoContentHttpResult : IResult
+{
+   public int? StatusCode { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed partial class RedirectToRouteHttpResult : IResult
+{
+   public string? Fragment { get; }
+   public string? RouteName { get; }
+   public RouteValueDictionary? RouteValues { get; }
+   public bool Permanent { get; }
+   public bool PreserveMethod { get; }
+   public bool AcceptLocalUrlOnly { get; }
+   public string? Url { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed partial class RedirectHttpResult : IResult
+{
+   public bool AcceptLocalUrlOnly { get; }
+   public bool Permanent { get; }
+   public bool PreserveMethod { get; }
+   public string? Url { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed class BadRequestObjectHttpResult : IResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class UnauthorizedHttpResult : IResult
+{
+   public int? StatusCode { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class NotFoundObjectHttpResult : IResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class ConflictObjectHttpResult : IResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class UnprocessableEntityObjectHttpResult : IResult
+{
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class ProblemHttpResult : IResult
+{
+   public ProblemDetails ProblemDetails { get; }
+   public string ContentType { get ;}
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed partial class ChallengeHttpResult : IResult
+{
+   public IReadOnlyList<string> AuthenticationSchemes { get; }
+   public AuthenticationProperties? Properties { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed partial class SignOutHttpResult : IResult
+{
+   public IReadOnlyList<string> AuthenticationSchemes { get; }
+   public AuthenticationProperties? Properties { get;  }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed partial class SignInHttpResult : IResult
+{
+   public string? AuthenticationScheme { get; }
+   public AuthenticationProperties? Properties { get;  }
+   public ClaimsPrincipal Principal { get;  }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed partial class ForbidHttpResult : IResult
+{
+   public IReadOnlyList<string> AuthenticationSchemes { get; }
+   public AuthenticationProperties? Properties { get;  }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed class VirtualFileHttpResult : IResult
+{
+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public string FileName { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed class PhysicalFileHttpResult : IResult
+{
+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public string FileName { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed class FileStreamHttpResult : IResult
+{
+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public Stream FileStream { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed class PushStreamHttpResult : IResult
+{
+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed class FileContentHttpResult : IResult
+{
+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public ReadOnlyMemory<byte> FileContents { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

The exact signatures of the constructors still need to be determined.

brunolins16 commented 2 years ago

@halter73 Here is the API updated with the proposal ctors and removing the interfaces:

+namespace Microsoft.AspNetCore.Http;

+public sealed class EmptyHttpResult : IResult
+{
+   public static readonly EmptyHttpResult Instance;
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed partial class ContentHttpResult : IResult
+{
+   public ContentHttpResult(string? content, string? contentType) {}
+   public ContentHttpResult(string? content, string? contentType, int? statusCode) {}

+   public string? Content { get; }
+   public int? StatusCode { get; }
+   public string? ContentType { get; }

+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class JsonHttpResult : IResult
+{
+   public JsonHttpResult(object? value, JsonSerializerOptions? jsonSerializerOptions) {}
+   public JsonHttpResult(object? value, int? statusCode, JsonSerializerOptions? jsonSerializerOptions) {}
+   public JsonHttpResult(object? value, string? contentType, JsonSerializerOptions? jsonSerializerOptions) {}
+   public JsonHttpResult(object? value, int? statusCode, string? contentType, JsonSerializerOptions? jsonSerializerOptions) {}

+   public string? Content { get; }
+   public int? StatusCode { get; }
+   public string? ContentType { get; }
+   public JsonSerializerOptions? JsonSerializerOptions { get; }

+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class OkObjectHttpResult : IResult
+{
+   public OkObjectHttpResult(object? value) {}

+   public int StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class CreatedHttpResult : IResult
+{
+   public CreatedHttpResult(string location, object? value) {}
+   public CreatedHttpResult(Uri locationUri, object? value) {}

+   public int StatusCode { get; }
+   public object? Value { get; }
+   public string? Location { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class CreatedAtRouteHttpResult : IResult
+{
+   public CreatedAtRouteHttpResult(object? routeValues, object? value) {}
+   public CreatedAtRouteHttpResult(string? routeName, object? routeValues, object? value) {}

+   public int StatusCode { get; }
+   public object? Value { get; }
+   public string? RouteName { get; }
+   public RouteValueDictionary? RouteValues { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class AcceptedHttpResult : IResult
+{
+   public AcceptedHttpResult(string location, object? value) {}
+   public AcceptedHttpResult(Uri locationUri, object? value) {}

+   public int StatusCode { get; }
+   public object? Value { get; }
+   public string? Location { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class AcceptedHttpResult : IResult
+{
+   public AcceptedHttpResult(object? routeValues, object? value) {}
+   public AcceptedHttpResult(string? routeName, object? routeValues, object? value) {}

+   public int StatusCode { get; }
+   public object? Value { get; }
+   public string? RouteName { get; }
+   public RouteValueDictionary? RouteValues { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public class NoContentHttpResult : IResult
+{
+   public NoContentHttpResult() {}

+   public intStatusCode { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed partial class RedirectToRouteHttpResult : IResult
+{
+  public RedirectToRouteHttpResult(object? routeValues) {}
+  public RedirectToRouteHttpResult(string? routeName, object? routeValues) {}
+  public RedirectToRouteHttpResult(string? routeName, object? routeValues, bool permanent) {}
+  public RedirectToRouteHttpResult(string? routeName, object? routeValues, bool permanent, bool preserveMethod) {}
+  public RedirectToRouteHttpResult(string? routeName, object? routeValues, string? fragment) {}
+  public RedirectToRouteHttpResult(string? routeName, object? routeValues, bool permanent, string? fragment) {}
+  public RedirectToRouteHttpResult(string? routeName, object? routeValues, bool permanent, bool preserveMethod, string? fragment) {}

+   public string? Fragment { get; }
+   public string? RouteName { get; }
+   public RouteValueDictionary? RouteValues { get; }
+   public bool Permanent { get; }
+   public bool PreserveMethod { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed partial class RedirectHttpResult : IResult
+{
+  public RedirectHttpResult(string url) {}
+  public RedirectHttpResult(string url, bool permanent) {}
+  public RedirectHttpResult(string url, bool permanent, bool preserveMethod) {}
+  public RedirectHttpResult(string url, bool acceptLocalUrlOnly, bool permanent, bool preserveMethod) {}

+   public bool AcceptLocalUrlOnly { get; }
+   public bool Permanent { get; }
+   public bool PreserveMethod { get; }
+   public string? Url { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed class BadRequestObjectHttpResult : IResult
+{
+   public BadRequestObjectHttpResult(object? error){}

+   public int StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class UnauthorizedHttpResult : IResult
+{
+   public UnauthorizedHttpResult() {}

+   public int StatusCode { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class NotFoundObjectHttpResult : IResult
+{
+   public NotFoundObjectHttpResult(object? value) {}

+   public int StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class ConflictObjectHttpResult : IResult
+{
+   public ConflictObjectHttpResult(object? error) {}

+   public int StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class UnprocessableEntityObjectHttpResult : IResult
+{
+   public UnprocessableEntityObjectHttpResult(object? error) {}

+   public int StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed class ProblemHttpResult : IResult
+{
+   public ProblemHttpResult(ProblemDetails problemDetails) {}

+   public ProblemDetails ProblemDetails { get; }
+   public string ContentType { get ;}
+   public int? StatusCode { get; }
+   public object? Value { get; }
+   public Task ExecuteAsync(HttpContext httpContext)  {}
+}

+public sealed partial class ChallengeHttpResult : IResult
+{
+   public ChallengeHttpResult() {}
+   public ChallengeHttpResult(string authenticationScheme) {}
+   public ChallengeHttpResult(IList<string> authenticationSchemes) {}
+   public ChallengeHttpResult(AuthenticationProperties? properties) {}
+   public ChallengeHttpResult(string authenticationScheme, AuthenticationProperties? properties) {}
+   public ChallengeHttpResult(IList<string> authenticationSchemes, AuthenticationProperties? properties) {}

+   public IReadOnlyList<string> AuthenticationSchemes { get; }
+   public AuthenticationProperties? Properties { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed partial class SignOutHttpResult : IResult
+{
+   public SignOutHttpResult() {}
+   public SignOutHttpResult(string authenticationScheme) {}
+   public SignOutHttpResult(IList<string> authenticationSchemes) {}
+   public SignOutHttpResult(AuthenticationProperties? properties) {}
+   public SignOutHttpResult(string authenticationScheme, AuthenticationProperties? properties) {}
+   public SignOutHttpResult(IList<string> authenticationSchemes, AuthenticationProperties? properties) {}

+   public IReadOnlyList<string> AuthenticationSchemes { get; }
+   public AuthenticationProperties? Properties { get;  }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed partial class SignInHttpResult : IResult
+{
+   public SignInHttpResult(ClaimsPrincipal principal) {}
+   public SignInHttpResult(ClaimsPrincipal principal, string? authenticationScheme, AuthenticationProperties? properties) {}

+   public string? AuthenticationScheme { get; }
+   public AuthenticationProperties? Properties { get;  }
+   public ClaimsPrincipal Principal { get;  }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed partial class ForbidHttpResult : IResult
+{
+   public ForbidHttpResult() {}
+   public ForbidHttpResult(string authenticationScheme) {}
+   public ForbidHttpResult(IList<string> authenticationSchemes) {}
+   public ForbidHttpResult(AuthenticationProperties? properties) {}
+   public ForbidHttpResult(string authenticationScheme, AuthenticationProperties? properties) {}
+   public ForbidHttpResult(IList<string> authenticationSchemes, AuthenticationProperties? properties) {}

+   public IReadOnlyList<string> AuthenticationSchemes { get; }
+   public AuthenticationProperties? Properties { get;  }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed class VirtualFileHttpResult : IResult
+{
+   public VirtualFileHttpResult(string fileName, string? contentType) {}
+   public VirtualFileHttpResult(string fileName, string? contentType, string? fileDownloadName) {}
+   public VirtualFileHttpResult(string fileName, string? contentType, string? fileDownloadName, bool enableRangeProcessing, DateTimeOffset? lastModified = null, EntityTagHeaderValue? entityTag = null) {}

+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public string FileName { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed class PhysicalFileHttpResult : IResult
+{
+   public PhysicalFileHttpResult(string fileName, string? contentType) {}
+   public PhysicalFileHttpResult(string fileName, string? contentType, string? fileDownloadName) {}
+   public PhysicalFileHttpResult(string fileName, string? contentType, string? fileDownloadName, bool enableRangeProcessing, DateTimeOffset? lastModified = null, EntityTagHeaderValue? entityTag = null) {}

+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public string FileName { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed class FileStreamHttpResult : IResult
+{
+   public FileStreamHttpResult(Stream fileStream, string? contentType) {}
+   public FileStreamHttpResult(Stream fileStream, string? contentType, string? fileDownloadName) {}
+   public FileStreamHttpResult(Stream fileStream, string? contentType, string? fileDownloadName, bool enableRangeProcessing, DateTimeOffset? lastModified = null, EntityTagHeaderValue? entityTag = null) {}

+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public Stream FileStream { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed class PushStreamHttpResult : IResult
+{
+   public PushStreamHttpResult(Func<Stream, Task> streamWriterCallback, string? contentType) {}
+   public PushStreamHttpResult(Func<Stream, Task> streamWriterCallback, string? contentType, string? fileDownloadName) {}
+   public PushStreamHttpResult(Func<Stream, Task> streamWriterCallback, string? contentType, string? fileDownloadName, bool enableRangeProcessing, DateTimeOffset? lastModified = null, EntityTagHeaderValue? entityTag = null) {}

+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

+public sealed class FileContentHttpResult : IResult
+{
+   public FileContentHttpResult(ReadOnlyMemory<byte> fileContents, string? contentType) {}
+   public FileContentHttpResult(ReadOnlyMemory<byte> fileContents, string? contentType, string? fileDownloadName) {}
+   public FileContentHttpResult(ReadOnlyMemory<byte> fileContents, string? contentType, string? fileDownloadName, bool enableRangeProcessing, DateTimeOffset? lastModified = null, EntityTagHeaderValue? entityTag = null) {}

+   public string ContentType { get; }
+   public string FileDownloadName { get; }
+   public DateTimeOffset? LastModified { get; }
+   public EntityTagHeaderValue? EntityTag { get; }
+   public bool EnableRangeProcessing { get; }
+   public long? FileLength { get;  }
+   public ReadOnlyMemory<byte> FileContents { get; }
+   public Task ExecuteAsync(HttpContext httpContext);
+}

Also, this type that was not listed before:

+namespace Microsoft.AspNetCore.Http;

+public sealed partial class StatusCodeHttpResult : IResult
+{
+   public StatusCodeHttpResult(int statusCode) {}

+   public int StatusCode { get; }

+   public Task ExecuteAsync(HttpContext httpContext);
+}
halter73 commented 2 years ago

Let's hold of on the public constructors and make them internal for preview3 (even the implicit default ones if any) so we have more time to consider them in API review preview4.

I'm fine with keeping EmptyHttpResult.Instance, but it should be a property:

public static EmptyHttpResult Instance { get; }
brunolins16 commented 2 years ago

Let's hold of on the public constructors and make them internal for preview3 (even the implicit default ones if any) so we have more time to consider them in API review preview4.

I'm fine with keeping EmptyHttpResult.Instance, but it should be a property:

public static EmptyHttpResult Instance { get; }

I have created a follow up issue for this #40802 .