Make IResult concrete types public

[brunolins16]

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:

• Microsoft.AspNetCore.Http.Endpoints
• Microsoft.AspNetCore.Http.Endpoints.Results
• Microsoft.AspNetCore.Http.RouteHandling
• Microsoft.AspNetCore.Http.RouteHandling.Results

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]

• 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.

• 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; }

• 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.

[brunolins16]

@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]

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]

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.

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.