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 Results.Typed factory methods for creating results whose types properly reflect the response type & shape #41009

Closed
DamianEdwards opened this issue Apr 1, 2022 · 22 comments · Fixed by #41161
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 Docs This issue tracks updating documentation feature-minimal-actions Controller-like actions for endpoint routing
Milestone

Comments

@DamianEdwards
Copy link
Member

Background

The existing Results.xxx() static factory methods all return IResult meaning the explicit type information is lost. Even though the in-box types themselves are now public, their constructors are not, meaning having a route handler delegate return an explicit result type requires the result be cast to its actual type, e.g.:

app.MapGet("/todos/{id}", async (int id, TodoDb db) =>
    (OkObjectHttpResult)Results.Ok(await db.FindAsync(id));

The introduction of the Results<TResult1, TResultN> union types presents a compelling reason to allow the creation of the in-box IResult implementing types in a way that preserves their explicit types. Having to explicitly cast each call to a Results factory method is somewhat unsightly:

app.MapGet("/todos/{id}", async Task<Results<OkObjectHttpResult, NotFoundObjectHttpResult>> (int id, TodoDb db) =>
    await db.FindAsync(id) is Todo todo
        ? (OkObjectHttpResult)Results.Ok(todo)
        : (NotFoundObjectHttpResult)Results.NotFound()

Additionally, the in-box result types today that allow setting an object to be serialized to the response body do not preserve the type information of those objects, including OkObjectHttpResult and others, e.g.

public static IResult Ok(object? value = null) => new OkObjectHttpResult(value);

This means that even if the result type itself were preserved, the type of the underlying response body is not, and as such an OpenAPI schema cannot be inferred automatically, requiring the developer to manually annotate the endpoint with type information:

app.MapGet("/todos/{id}", async (int id, TodoDb db) =>
    (OkObjectHttpResult)Results.Ok(await db.FindAsync(id))
    .Produces<Todo>();

In order to enable result types to properly describe the HTTP result they represent, the type must encapsulate the status code, content type, and response body shape statically by the type shape (i.e. without any runtime knowledge).

Proposal

New results types

Introduce new result types that represent all the supported response status codes (within reason) and preserve the type details of the response body via generics. As these result types all serialize their body object to JSON (and no other format is currently supported by the in-box result types) the content type need not be represented in the type shape. An example of the result types being proposed can be found in the MinimalApis.Extensions library here.

Example of what a new generic result representing an OK response might look like:

namespace Microsoft.AspNetCore.Http;

public class OkHttpResult<TValue> : IResult
{
    internal OkHttpResult(TValue value)
    {
        Value = value;
    }

    public TValue Value { get; init; }

    public int StatusCode => StatusCodes.Status200OK;

    public async Task ExecuteAsync(HttpContext httpContext)
    {
        httpContext.Response.StatusCode = StatusCode;
        if (Value is not null)
        {
            await httpContext.Response.WriteAsJsonAsync(Value);
        }
    }
}

❓ New result type names

As the type names for these new types will actually appear in code (rather than being inferred by the compiler) some extra thought should be given to their names. The existing result type names are somewhat unwieldly, e.g. OkObjectHttpResult, NotFoundObjectHttpResult, and as such don't lend themselves well to the "minimal" approach. In the MinimalApis.Extensions library, the types are named with the assumption that they will be used in conjunction with the Results<TResult1, TResultN> union types, and as such the names are very minimal, just using the response type itself, e.g. Ok, NotFound, etc.

This allows for fairly terse signatures like Task<Result<Ok<Todo>, NotFound>> (int id) but this might be a bit too short to accept in the framework. Some other ideas we should consider:

  • OkHttpResult<T>, NotFoundHttpResult, etc.
  • Putting the new types in their own namespace, Microsoft.AspNetCore.Http.TypedResults and having shorter names, e.g.
    • Microsoft.AspNetCore.Http.TypedResults.Ok<TValue>, Microsoft.AspNetCore.Http.TypedResults.NotFound
    • This would allow the types to be used either like TypedResults.Ok<Todo>, TypedResults.NotFound or by importing the namespace explicitly like Ok<Todo>, NotFound, etc.

Results.Typed factory methods

To preserve the existing pattern of creating result types via the static Results class, we will introduce a new member on the Microsoft.AspNetCore.Http.Results class called Typed, upon which are factory methods that use the new result types and preserve the concrete type of the returned results:

namespace Microsoft.AspNetCore.Http;

public static class Results
{
+ public static ITypedResultExtensions Typed { get; } = new TypedResultExtensions();
}

+ internal class TypedResultExtensions : ITypedResultExtensions { }

+ public static class TypedResultExtensionsMethods
+ {
+      public static OkHttpResult<TResult> Ok<TResult>(this ITypedResultExtensions typedResults, TResult? value)
+      {
+          return new OkHttpResult<TResult>(value);
+      }
+ 
+     // Additional factory result methods
+ }

Example use

app.MapGet("/todos/{id}", async Task<Results<OkHttpResult<Todo>, NotFoundHttpResult>> (int id, TodoDb db) =>
    await db.FindAsync(id) is Todo todo
        ? Results.Typed.Ok(todo)
        : Results.Typed.NotFound());

Making the new results self-describe via metadata

These new result types would be updated once #40646 is implemented, such that the result types can self-describe their operation via metadata into ApiExplorer and through to OpenAPI documents and Swagger UI, resulting in much more of an APIs details being described from just the method type information.

The following two API implementations represent the resulting two different approaches to describing the details of an API for OpenAPI/Swagger, the first using just type information from the route handler delegate, the second requiring explicitly adding type information via metadata. The first means there's compile-time checking that the responses returned are actually declared in the method signature, whereas the second requires the developer to manually ensure the metadata added matches the actual method implementation:

// API parameters, responses, and response body shape described just by method type information
app.MapGet("/todos/{id}", async Task<Results<OkHttpResult<Todo>, NotFoundHttpResult>> (int id, TodoDb db) =>
    await db.FindAsync(id) is Todo todo
        ? Results.Typed.Ok(todo)
        : Results.Typed.NotFound());

// API parameters from method type information, but response details requiring manual metadata specification
app.MapGet("/todos/{id}", async (int id, TodoDb db) =>
    await db.FindAsync(id) is Todo todo
        ? Results.Ok(todo)
        : Results.NotFound())
    .Produces<Todo>()
    .Produces(StatusCodes.Status404NotFound);
@DamianEdwards DamianEdwards added feature-minimal-actions Controller-like actions for endpoint routing area-web-frameworks labels Apr 1, 2022
@DamianEdwards
Copy link
Member Author

Another possibility regarding the naming of the new typed results and the factory methods to create them:

  • Create the new result types in a namespace Microsoft.AspNetCore.Http.TypedResults
  • Give the new result types short names based on their response type, e.g. Ok<TValue>, NotFound, etc.
  • Implement the factory methods on a new nested static class Microsoft.AspNetCore.Http.Results.Typed

Implementation of new result types in the Microsoft.AspNetCore.Http.TypedResults namespace:

namespace Microsoft.AspNetCore.Http.TypedResults;

public class Ok<TValue> : IResult
{
    internal Ok(TValue value) => { Value = value; }
    public TValue Value { get; init; }
    public int StatusCode => StatusCodes.Status200OK;
    public async Task ExecuteAsync(HttpContext httpContext)
    {
        httpContext.Response.StatusCode = StatusCode;
        if (Value is not null)
        {
            await httpContext.Response.WriteAsJsonAsync(Value);
        }
    }
}

public class NotFound : IResult
{
    public int StatusCode => StatusCodes.Status404NotFound;
    public Task ExecuteAsync(HttpContext httpContext)
    {
        httpContext.Response.StatusCode = StatusCode;
        return Task.CompletedTask;
    }
}

// Additional result types

Implementation of Microsoft.AspNetCore.Http.Results.Typed:

namespace Microsoft.AspNetCore.Http;

public static class Results
{
+    public static class Typed
+    {
+        public static TypedResults.Ok<TValue>(TValue value)
+        {
+            return new TypedResults.Ok<TResult>(value);
+        }
+        // Additional static factory result methods
+    }
}

This would enable multiple consumption patterns including:

  • Referring to the new result types for method return type signatures when the default implicit namespaces are imported like TypedResults.Ok<Todo>, TypedResults.NotFound, etc.
  • Ability to explicitly import the new namespace so that the the new result types are in scope like Ok<Todo>, NotFound, etc.
  • Calling new static factory methods on the existing Results type via the new nested Typed class like Results.Typed.Ok(todo), Results.Typed.NotFound(), etc.
  • Ability to statically import the the new nested Typed class via using static Microsoft.AspNetCore.Http.Results.Typed such that all the static methods on it become in scope
  • If we wished to support extension methods on the Results.Typed class we'd need to add an Extensions property to it using another extensions-only interface, e.g. ITypedResultExtensions

Example consumption models enabled by this approach:

Existing IResult-based approach

app.MapGet("/todo/{id}", async (int id, TodoDb db) =>
    await db.Todos.FindAsync(id) is Todo todo
        ? Results.Ok(todo)
        : Results.NotFound();

Typed results with default implicit namespace imports

app.MapGet("/todo/{id}", async Task<Results<TypedResults.Ok<Todo>, TypedResults.NotFound>> (int id, TodoDb db) =>
    await db.Todos.FindAsync(id) is Todo todo
        ? Results.Typed.Ok(todo)
        : Results.Typed.NotFound();

Typed results with explicit namespace import

using Microsoft.AspNetCore.Http.TypedResults;

app.MapGet("/todo/{id}", async Task<Results<Ok<Todo>, NotFound>> (int id, TodoDb db) =>
    await db.Todos.FindAsync(id) is Todo todo
        ? Results.Typed.Ok(todo)
        : Results.Typed.NotFound();

Typed results with explicit namespace & static factory class import

using Microsoft.AspNetCore.Http.TypedResults;
using static Microsoft.AspNetCore.Http.Results.Typed;

app.MapGet("/todo/{id}", async Task<Results<Ok<Todo>, NotFound>> (int id, TodoDb db) =>
    await db.Todos.FindAsync(id) is Todo todo
        ? Ok(todo)
        : NotFound();

@brunolins16
Copy link
Member

  • Create the new result types in a namespace Microsoft.AspNetCore.Http.TypedResults

Is not better have all result types under the same namespace instead of duplicating them under a new namespace? Maybe we could consider move the current types to under something else. That was one of the alternative designs proposed in #40656 or we can decide something completely different.

I am thinking something like this:

namespace Microsoft.AspNetCore.Http.HttpResults;

- public sealed class OkObjectResult : IResult
+ public sealed class Ok : IResult
{
-    internal OkObjectResult(object? value) {}
+   internal Ok() {}

-   public object? Value { get; internal init; }

    public int StatusCode => StatusCodes.Status200OK;

    public Task ExecuteAsync(HttpContext httpContext){}
}

+ public sealed class Ok<TValue> : IResult
+ {
+    internal Ok(TValue? value) {}
+    public int StatusCode => StatusCodes.Status200OK;

+    public TValue? Value { get; }

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

@brunolins16
Copy link
Member

  • Give the new result types short names based on their response type, e.g. Ok<TValue>, NotFound, etc.

I love the idea of have short names but will not it get confusing with other framework types (from different namespaces) that probably will be used?

Eg.: If we rename the FileStreamHttpResult to FileStream that will probably conflict with System.IO.FileStream that usually will be imported since the dev is work with a FileStream.

@DamianEdwards
Copy link
Member Author

We could potentially mitigate that by only using short names for the result types that actually implement IEndpointMetadataProvider and thus describe the API result types. Something like FileStreamHttpResult cant' do that accurately because it accepts a content type at runtime. This was also one of the motivations behind putting these results in their own namespace to help denote that they're different from the "regular" result types.

@brunolins16
Copy link
Member

brunolins16 commented Apr 7, 2022

After a quick look, might missed something, in addition to the example I mentioned we will only have conflict with:

VirtualFileHttpResult -> https://docs.microsoft.com/en-us/dotnet/api/system.web.hosting.virtualfile?view=netframework-4.8 Not in .NET Core
JsonHttpResult -> https://docs.microsoft.com/en-us/dotnet/api/system.web.helpers.json?view=aspnet-webpages-3.2
Don't know if is possible to use it

So, maybe if we decide a better name to FileStreamHttpResult other than FileStream we might be able to have the short names without problems.

@brunolins16
Copy link
Member

This was also one of the motivations behind putting these results in their own namespace to help denote that they're different from the "regular" result types.

Got your point but my concern is the TypedResults part of the namespace. All results are "typed" why we will have a namespace with results that are "more typed" than others. I don't have a better option 😂, but that is the reason I feel all result types should be under the same namespace.

Maybe the *.TypedResults could be the namespace where all results are.

@DamianEdwards
Copy link
Member Author

Microsoft.AspNetCore.Http.HttpResults would be an fine namespace, assuming we also add Results.Typed.* methods to create the results while preserving the concrete types. Then one would simply need to add the using or qualify the type names when using them in the signature, e.g.

app.MapGet("/todo/{id}", async Task<Results<HttpResults.Ok<Todo>, HttpResults.NotFound>> (int id, TodoDb db) =>
    await db.Todos.FindAsync(id) is Todo todo
        ? Results.Typed.Ok(todo)
        : Results.Typed.NotFound();

or

using Microsoft.AspNetCore.Http.HttpResults;

app.MapGet("/todo/{id}", async Task<Results<Ok<Todo>, NotFound>> (int id, TodoDb db) =>
    await db.Todos.FindAsync(id) is Todo todo
        ? Results.Typed.Ok(todo)
        : Results.Typed.NotFound();

@brunolins16
Copy link
Member

Do you think we should keep results with the current object Value or change them to not have a value at all?

Eg.:

public sealed class Ok : IResult
{
    internal Ok(object?  value)

    public int StatusCode => StatusCodes.Status200OK;
    public object? Value { get; }
}

become

public sealed class Ok : IResult
{
    internal Ok()

    public int StatusCode => StatusCodes.Status200OK;
}

I prefer to change them but that probably mean the current Results.* methods will need to be changed to return Result or Result<object> based on the input value. Something like this:

public static IResult Ok(object? value = null)
        => value is null ? new Ok() : new Ok<object>(value);

or

public static IResult Ok()
        => new Ok();
public static IResult Ok(object? value)
        =>  new Ok<object>(value);

I hope my question is clear? 😂

@DamianEdwards
Copy link
Member Author

Yep change them I think, and update the IResult returning methods like you say.

@brunolins16
Copy link
Member

Here is my proposal:

Typed Results static methods

They are almost the same available at https://github.com/dotnet/aspnetcore/blob/main/src/Http/Http.Results/src/Results.cs

namespace Microsoft.AspNetCore.Http;

public static partial class Results
{
+    public static class Typed
+    {
+        public static Challenge Challenge(
+            AuthenticationProperties? properties = null,
+            IList<string>? authenticationSchemes = null) {}
       
+        public static Forbid Forbid(
+            AuthenticationProperties? properties = null, 
+            IList<string>? authenticationSchemes = null) {}
       
+        public static SignIn SignIn(
+            ClaimsPrincipal principal,
+            AuthenticationProperties? properties = null,
+            string? authenticationScheme = null) {}
       
+        public static SignOut SignOut(
+            AuthenticationProperties? properties = null, 
+            IList<string>? authenticationSchemes = null) {}
       
+        public static Content Content(
+            string content, 
+            string? contentType = null, 
+            Encoding? contentEncoding = null) {}
       
+        public static Content Text(
+            string content, 
+            string? contentType = null, 
+            Encoding? contentEncoding = null) {}
       
+        public static Content Content(
+            string content, 
+            MediaTypeHeaderValue contentType) {}
       
+        public static Json<TValue> Json<TValue>(
+            TValue? data, 
+            JsonSerializerOptions? options = null, 
+            string? contentType = null, 
+            int? statusCode = null)  {}
       
+        public static FileContent File(
+            byte[] fileContents,
+            string? contentType = null,
+            string? fileDownloadName = null,
+            bool enableRangeProcessing = false,
+            DateTimeOffset? lastModified = null,
+            EntityTagHeaderValue? entityTag = null)  {}
       
+        public static FileContent Bytes(
+            byte[] contents,
+            string? contentType = null,
+            string? fileDownloadName = null,
+            bool enableRangeProcessing = false,
+            DateTimeOffset? lastModified = null,
+            EntityTagHeaderValue? entityTag = null)
+            => new(contents, contentType)  {}
       
+        public static FileContent Bytes(
+            ReadOnlyMemory<byte> contents,
+            string? contentType = null,
+            string? fileDownloadName = null,
+            bool enableRangeProcessing = false,
+            DateTimeOffset? lastModified = null,
+            EntityTagHeaderValue? entityTag = null) {}
       
+        public static HttpFileStream File(
+            Stream fileStream,
+            string? contentType = null,
+            string? fileDownloadName = null,
+            DateTimeOffset? lastModified = null,
+            EntityTagHeaderValue? entityTag = null,
+            bool enableRangeProcessing = false)  {}
       
+        public static HttpFileStream Stream(
+            Stream stream,
+            string? contentType = null,
+            string? fileDownloadName = null,
+            DateTimeOffset? lastModified = null,
+            EntityTagHeaderValue? entityTag = null,
+            bool enableRangeProcessing = false) {}
       
+        public static HttpFileStream Stream(
+            PipeReader pipeReader,
+            string? contentType = null,
+            string? fileDownloadName = null,
+            DateTimeOffset? lastModified = null,
+            EntityTagHeaderValue? entityTag = null,
+            bool enableRangeProcessing = false) {}
       
+        public static PushStream Stream(
+            Func<Stream, Task> streamWriterCallback,
+            string? contentType = null,
+            string? fileDownloadName = null,
+            DateTimeOffset? lastModified = null,
+            EntityTagHeaderValue? entityTag = null)  {}
       
+        public static PhysicalFile PhysicalFile(
+            string path,
+            string? contentType = null,
+            string? fileDownloadName = null,
+            DateTimeOffset? lastModified = null,
+            EntityTagHeaderValue? entityTag = null,
+            bool enableRangeProcessing = false)  {}
       
+        public static VirtualFile VirtualFile(
+            string path,
+            string? contentType = null,
+            string? fileDownloadName = null,
+            DateTimeOffset? lastModified = null,
+            EntityTagHeaderValue? entityTag = null,
+            bool enableRangeProcessing = false) {}
       
+        public static Redirect Redirect(
+            string url, 
+            bool permanent = false, 
+            bool preserveMethod = false) {}
       
+        public static Redirect LocalRedirect(
+            string localUrl, 
+            bool permanent = false, 
+            bool preserveMethod = false) {}
       
+        public static RedirectToRoute RedirectToRoute(
+            string? routeName = null, 
+            object? routeValues = null, 
+            bool permanent = false, 
+            bool preserveMethod = false, 
+            string? fragment = null) {}
       
+        public static Status StatusCode(int statusCode) {}
       
+        public static NotFound NotFound() {}
       
+        public static NotFound<TValue> NotFound<TValue>(TValue? value) {}
       
+        public static Unauthorized Unauthorized() {}
       
+        public static BadRequest BadRequest() {}
       
+        public static BadRequest<TValue> BadRequest<TValue>(TValue? error) {}
       
+        public static Conflict Conflict() {}
       
+        public static Conflict<TValue> Conflict<TValue>(TValue? error) {}
       
+        public static NoContent NoContent() {}
       
+        public static Ok Ok() {}
       
+        public static Ok<TValue> Ok<TValue>(TValue? value) {}
       
+        public static UnprocessableEntity UnprocessableEntity() {}
       
+        public static UnprocessableEntity<TValue> UnprocessableEntity<TValue>(TValue? error) {}
       
+        public static Problem Problem(
+            string? detail = null,
+            string? instance = null,
+            int? statusCode = null,
+            string? title = null,
+            string? type = null,
+            IDictionary<string, object?>? extensions = null) {}
       
+        public static Problem Problem(ProblemDetails problemDetails) {}
       
+        public static Problem ValidationProblem(
+            IDictionary<string, string[]> errors,
+            string? detail = null,
+            string? instance = null,
+            int? statusCode = null,
+            string? title = null,
+            string? type = null,
+            IDictionary<string, object?>? extensions = null) {}
              
+        public static Created Created(string uri) {}
       
+        public static Created<TValue> Created<TValue>(
+            string uri, 
+            TValue? value) {}       

+        public static Created Created(Uri uri) {}
       
+        public static Created<TValue> Created<TValue>(
+            Uri uri, 
+            TValue? value) {}
       
+        public static CreatedAtRoute CreatedAtRoute(
+            string? routeName = null, 
+            object? routeValues = null) {}
       
+        public static CreatedAtRoute<TValue> CreatedAtRoute<TValue>(
+            TValue? value, 
+            string? routeName = null, 
+            object? routeValues = null) {}
       
+        public static Accepted Accepted(string uri) {}
       
+        public static Accepted<TValue> Accepted<TValue>(
+            tring uri, 
+            TValue? value) {}
       
+        public static Accepted Accepted(Uri uri) {}
       
+        public static Accepted<TValue> Accepted<TValue>(
+            Uri uri, 
+            TValue? value) {}
       
+        public static AcceptedAtRoute AcceptedAtRoute(
+            string? routeName = null, 
+            object? routeValues = null) {}
       
+        public static AcceptedAtRoute<TValue> AcceptedAtRoute<TValue>(
+            TValue? value, 
+            string? routeName = null, 
+            object? routeValues = null) {}       
+    }
}

New IResult types with type parameter

+namespace Microsoft.AspNetCore.Http.HttpResults;

+public sealed class AcceptedAtRoute<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public string? RouteName { get; }
+    public RouteValueDictionary RouteValues { get; }
+    public int StatusCode => StatusCodes.Status202Accepted;

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

+public sealed class Accepted<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public int StatusCode => StatusCodes.Status202Accepted;
+    public string? Location { get; }

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

+public sealed class BadRequest<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public int StatusCode => StatusCodes.Status400BadRequest;
+
+    public Task ExecuteAsync(HttpContext httpContext){}
+}

+public sealed class Conflict<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public int StatusCode => StatusCodes.Status409Conflict;
+
+    public Task ExecuteAsync(HttpContext httpContext){}
+}

+public sealed class CreatedAtRoute<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public string? RouteName { get; }
+    public RouteValueDictionary RouteValues { get; }
+    public int StatusCode => StatusCodes.Status201Created;

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

+public sealed class Created<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public int StatusCode => StatusCodes.Status201Created;
+    public string? Location { get; }

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

-public sealed partial class JsonHttpResult : IResult
+public sealed partial class Json<TValue> : IResult
{
-    public object Value { get; }
+    public TValue? Value { get; }
}

+public sealed class Ok<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public int StatusCode => StatusCodes.Status200OK;
+
+    public Task ExecuteAsync(HttpContext httpContext){}
+}

+public sealed class UnprocessableEntity<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public int StatusCode => StatusCodes.Status422UnprocessableEntity;
+
+    public Task ExecuteAsync(HttpContext httpContext){}
+}

Also, the proposal is to change the current types to have a shorter name, move to a new namespace and remove the object Value properties:

-namespace Microsoft.AspNetCore.Http;
+namespace Microsoft.AspNetCore.Http.HttpResults;

-public sealed class AcceptedHttpResult : IResult
+public sealed class Accepted : IResult
{
-    public object? Value { get; }    
}

-public sealed class AcceptedAtRouteHttpResult : IResult
+public sealed class AcceptedAtRoute : IResult
{
-    public object? Value { get; }    
}

-public sealed class BadRequestObjectHttpResult : IResult
+public sealed class BadRequest : IResult
{
-    public object? Value { get; }    
}

-public sealed class ChallengeHttpResult : IResult
+public sealed class Challenge : IResult {   }

-public sealed class ConflictObjectHttpResult : IResult
+public sealed class Conflict : IResult
{
-    public object? Value { get; }    
}

-public sealed class ContentHttpResult : IResult
+public sealed class Content : IResult 
{   
-    public string? Content { get; }   
+    public string? ResponseContent { get; }       
}

-public sealed class CreatedHttpResult : IResult
+public sealed class Created : IResult
{
-    public object? Value { get; }    
}

-public sealed class CreatedAtRouteHttpResult : IResult
+public sealed class CreatedAtRoute : IResult
{
-    public object? Value { get; }    
}

-public sealed class EmptyHttpResult : IResult
+public sealed class Empty : IResult {   }

-public sealed class ForbidHttpResult : IResult
+public sealed class Forbid : IResult {   }

-public sealed class FileStreamHttpResult : IResult
+public sealed class HttpFileStream : IResult {   }

-public sealed class NoContentHttpResult : IResult
+public sealed class NoContent : IResult {   }

-public sealed class NotFoundObjectHttpResult : IResult
+public sealed class NotFound : IResult
{
-    public object? Value { get; }    
}

-public sealed class OkObjectHttpResult : IResult
+public sealed class Ok : IResult
{
-    public object? Value { get; }    
}

-public sealed class PhysicalFileHttpResult : IResult
+public sealed class PhysicalFile : IResult {   }

-public sealed class ProblemHttpResult : IResult
+public sealed class Problem : IResult {   }

-public sealed class PushStreamHttpResult : IResult
+public sealed class PushStream : IResult {   }

-public sealed class RedirectHttpResult : IResult
+public sealed class Redirect : IResult {   }

-public sealed class RedirectToRouteHttpResult : IResult
+public sealed class RedirectToRoute : IResult {   }

-public sealed class SignInHttpResult : IResult
+public sealed class SignIn : IResult {   }

-public sealed class SignOutHttpResult : IResult
+public sealed class SignOut : IResult {   }

-public sealed class StatusCodeHttpResult : IResult
+public sealed class Status : IResult {   }

-public sealed class UnauthorizedHttpResult : IResult
+public sealed class Unauthorized : IResult {   }

-public sealed class UnprocessableEntityObjectHttpResult : IResult
+public sealed class UnprocessableEntity : IResult
{
-    public object? Value { get; }    
}

-public sealed class VirtualFileHttpResult : IResult
+public sealed class VirtualFile : IResult {   }

@brunolins16 brunolins16 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 Apr 8, 2022
@ghost
Copy link

ghost commented Apr 9, 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

❌ DO NOT use public nested types as a logical grouping construct; use namespaces for this.

https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/nested-types

I think this case might be the exception to the rule, but I'm not sure if there's any precedent for this which has me a little bit worried.

An alternative might be a static property, but I'm not convinced that's really better. It seems like it would just increase the surface area of the API by requiring us to add a new property in addition to a new type.

@DamianEdwards
Copy link
Member Author

Yeah I originally wrote it as a new type and a static property like you said, but it just adds more type surface. The other alternative would be to just introduce a new static class TypedResults that's the typed-equivalent of Results and have people access methods on that directly, e.g. return TypedResults.Ok()

@halter73
Copy link
Member

halter73 commented Apr 13, 2022

API review notes:

  • What about linker friendliness?
    • This will be addressed separately along with the existing Results methods.
  • Do we prefer Results.Typed or TypedResults?
    • We prefer TypedResults in the Microsoft.AspNetCore.Http namespace alongside Results. This avoids a nested type or static property.
  • We should use longer names for noun results.
    • Ok is okay
    • Problem is not. Should be ProblemResult or something.

@halter73 halter73 added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Apr 13, 2022
@brunolins16
Copy link
Member

We should use longer names for noun results.

  • Ok is okay
  • Problem is not. Should be ProblemResult or something.

I was not in the review meeting, so, I am sorry if I am commenting without the discussed context but in my opinion we need to be consistent with the types naming, and, if what was discussed is to have the suffix for some types I feel we should keep the suffix for all of them.

@DamianEdwards
Copy link
Member Author

DamianEdwards commented Apr 13, 2022

@brunolins16 that was where it landed in that meeting. I understand your concern and myself have wondered whether we'd be better separating the types into their own namespaces, using the guidelines outlined in the review, which effectively come down to "does the type implement IEndpointMetadataProvider" in most cases.

Taking a step back for a moment, the last outcome I want is for all the types to have the HttpResult suffix as it makes using them in method signatures with Results<TResult1, TResultN> much more cumbersome, e.g.:

// Longer type names
app.MapGet("/todo/{id}", async Task<Results<OkHttpResult<Todo>, NotFoundHttpResult>> (int id, TodoDb db) =>
    await db.Todos.FindAsync(id) is Todo todo
        ? TypedResults.Ok(todo)
        : TypedResults.NotFound();

// Type names optimized for use with Results<T1, TN>
app.MapGet("/todo/{id}", async Task<Results<Ok<Todo>, NotFound>> (int id, TodoDb db) =>
    await db.Todos.FindAsync(id) is Todo todo
        ? TypedResults.Ok(todo)
        : TypedResults.NotFound();

We should consider all options that help us avoid that outcome IMO.

@DamianEdwards DamianEdwards added api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews and removed api-needs-work API needs work before it is approved, it is NOT ready for implementation labels Apr 18, 2022
@ghost
Copy link

ghost commented Apr 18, 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.

@DamianEdwards
Copy link
Member Author

DamianEdwards commented Apr 18, 2022

Latest API proposal that's in PR:

Typed Results static methods

They are almost the same as in existing Results class but they return the concrete type:

namespace Microsoft.AspNetCore.Http;

+public static class TypedResults
+{
+    public static ChallengeHttpResult Challenge(
+        AuthenticationProperties? properties = null,
+        IList<string>? authenticationSchemes = null) {}
       
+    public static ForbidHttpResult Forbid(
+        AuthenticationProperties? properties = null, 
+        IList<string>? authenticationSchemes = null) {}
       
+    public static SignInHttpResult SignIn(
+        ClaimsPrincipal principal,
+        AuthenticationProperties? properties = null,
+        string? authenticationScheme = null) {}
       
+    public static SignOutHttpResult SignOut(
+        AuthenticationProperties? properties = null, 
+        IList<string>? authenticationSchemes = null) {}
       
+    public static ContentHttpResult Content(
+        string content, 
+        string? contentType = null, 
+        Encoding? contentEncoding = null) {}
       
+    public static ContentHttpResult Text(
+        string content, 
+        string? contentType = null, 
+        Encoding? contentEncoding = null) {}
       
+    public static ContentHttpResult Content(
+        string content, 
+        MediaTypeHeaderValue contentType) {}
       
+    public static JsonHttpResult<TValue> Json<TValue>(
+        TValue? data, 
+        JsonSerializerOptions? options = null, 
+        string? contentType = null, 
+        int? statusCode = null)  {}
       
+    public static FileContentHttpResult File(
+        byte[] fileContents,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        bool enableRangeProcessing = false,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null)  {}
       
+    public static FileContentHttpResult Bytes(
+        byte[] contents,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        bool enableRangeProcessing = false,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null)
+        => new(contents, contentType)  {}
       
+    public static FileContentHttpResult Bytes(
+        ReadOnlyMemory<byte> contents,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        bool enableRangeProcessing = false,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null) {}
       
+    public static FileStreamHttpResult File(
+        Stream fileStream,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null,
+        bool enableRangeProcessing = false)  {}
       
+    public static FileStreamHttpResult Stream(
+        Stream stream,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null,
+        bool enableRangeProcessing = false) {}
       
+    public static FileStreamHttpResult Stream(
+        PipeReader pipeReader,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null,
+        bool enableRangeProcessing = false) {}
       
+    public static PushStreamHttpResult Stream(
+        Func<Stream, Task> streamWriterCallback,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null)  {}
       
+    public static PhysicalFileHttpResult PhysicalFile(
+        string path,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null,
+        bool enableRangeProcessing = false)  {}
       
+    public static VirtualFileHttpResult VirtualFile(
+        string path,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null,
+        bool enableRangeProcessing = false) {}
       
+    public static RedirectHttpResult Redirect(
+        string url, 
+        bool permanent = false, 
+        bool preserveMethod = false) {}
       
+    public static RedirectHttpResult LocalRedirect(
+        string localUrl, 
+        bool permanent = false, 
+        bool preserveMethod = false) {}
       
+    public static RedirectToRouteHttpResult RedirectToRoute(
+        string? routeName = null, 
+        object? routeValues = null, 
+        bool permanent = false, 
+        bool preserveMethod = false, 
+        string? fragment = null) {}
       
+    public static StatusHttpResult StatusCode(int statusCode) {}
       
+    public static NotFound NotFound() {}
       
+    public static NotFound<TValue> NotFound<TValue>(TValue? value) {}
       
+    public static UnauthorizedHttpResult Unauthorized() {}
       
+    public static BadRequest BadRequest() {}
       
+    public static BadRequest<TValue> BadRequest<TValue>(TValue? error) {}
       
+    public static Conflict Conflict() {}
       
+    public static Conflict<TValue> Conflict<TValue>(TValue? error) {}
       
+    public static NoContent NoContent() {}
       
+    public static Ok Ok() {}
       
+    public static Ok<TValue> Ok<TValue>(TValue? value) {}
       
+    public static UnprocessableEntity UnprocessableEntity() {}
       
+    public static UnprocessableEntity<TValue> UnprocessableEntity<TValue>(TValue? error) {}
       
+    public static ProblemHttpResult Problem(
+        string? detail = null,
+        string? instance = null,
+        int? statusCode = null,
+        string? title = null,
+        string? type = null,
+        IDictionary<string, object?>? extensions = null) {}
       
+    public static ProblemHttpResult Problem(ProblemDetails problemDetails) {}
       
+    public static ValidationProblem ValidationProblem(
+        IDictionary<string, string[]> errors,
+        string? detail = null,
+        string? instance = null,
+        int? statusCode = null,
+        string? title = null,
+        string? type = null,
+        IDictionary<string, object?>? extensions = null) {}
              
+    public static Created Created(string uri) {}
       
+    public static Created<TValue> Created<TValue>(
+        string uri, 
+        TValue? value) {}       

+    public static Created Created(Uri uri) {}
       
+    public static Created<TValue> Created<TValue>(
+        Uri uri, 
+        TValue? value) {}
       
+    public static CreatedAtRoute CreatedAtRoute(
+        string? routeName = null, 
+        object? routeValues = null) {}
       
+    public static CreatedAtRoute<TValue> CreatedAtRoute<TValue>(
+        TValue? value, 
+        string? routeName = null, 
+        object? routeValues = null) {}
       
+    public static Accepted Accepted(string uri) {}
       
+    public static Accepted<TValue> Accepted<TValue>(
+        tring uri, 
+        TValue? value) {}
       
+    public static Accepted Accepted(Uri uri) {}
       
+    public static Accepted<TValue> Accepted<TValue>(
+        Uri uri, 
+        TValue? value) {}
       
+    public static AcceptedAtRoute AcceptedAtRoute(
+        string? routeName = null, 
+        object? routeValues = null) {}
       
+    public static AcceptedAtRoute<TValue> AcceptedAtRoute<TValue>(
+        TValue? value, 
+        string? routeName = null, 
+        object? routeValues = null) {}       
+
+    public static EmptyHttpResult Empty { get; }
+}

Move result types to Microsoft.AspNetCore.Http.HttpResults namespace

-namespace Microsoft.AspNetCore.Http;
+namespace Microsoft.AspNetCore.Http.HttpResults;

// All result types

Change object IResult types to generic types

+namespace Microsoft.AspNetCore.Http.HttpResults;

+public sealed class Accepted : IResult, IEndpointMetadataProvider
+{
+    public int StatusCode => StatusCodes.Status202Accepted;
+    public string Location { get; }

+    public Task ExecuteAsync(HttpContext context)
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
+}

+public sealed class AcceptedAtRoute : IResult, IEndpointMetadataProvider
+{
+    public string? RouteName { get; }
+    public RouteValueDictionary RouteValues { get; }
+    public int StatusCode => StatusCodes.Status202Accepted;

+    public Task ExecuteAsync(HttpContext context)
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
+}

-public sealed class AcceptedAtRouteHttpResult : IResult
+public sealed class AcceptedAtRoute<TValue> : IResult
+{
-     public object? Value { get; }
+    public TValue? Value { get; }
+}

-public sealed class AcceptedHttpResult : IResult
+public sealed class Accepted<TValue> : IResult
+{
-    public object? Value { get; }
+    public TValue? Value { get; }
+}

+public sealed class BadRequest : IResult, IEndpointMetadataProvider
+{
+    public int StatusCode => StatusCodes.Status400BadRequest

+    public Task ExecuteAsync(HttpContext context)
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
+}

-public sealed class BadRequestObjectHttpResult : IResult
+public sealed class BadRequest<TValue> : IResult
+{
-    public object? Value { get; }
+    public TValue? Value { get; }
+}

+public sealed class Conflict : IResult, IEndpointMetadataProvider
+{
+    public int StatusCode => StatusCodes.Status409Conflict;

+    public Task ExecuteAsync(HttpContext context)
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
+}

-public sealed class ConflictObjectHttpResult : IResult
+public sealed class Conflict<TValue> : IResult
+{
-    public object? Value { get; }
+    public TValue? Value { get; }
+}

+public sealed class Created : IResult, IEndpointMetadataProvider
+{
+    public int StatusCode => StatusCodes.Status201Created;

+    public Task ExecuteAsync(HttpContext context)
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
+}

+public sealed class CreatedAtRoute : IResult, IEndpointMetadataProvider
+{
+    public string? RouteName { get; }
+    public RouteValueDictionary RouteValues { get; }
+    public int StatusCode => StatusCodes.Status201Created;

+    public Task ExecuteAsync(HttpContext context)
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
+}

-public sealed class CreatedAtRouteHttpResult : IResult
+public sealed class CreatedAtRoute<TValue> : IResult
+{
-    public object? Value { get; }
+    public TValue? Value { get; }
+}

-public sealed class CreatedHttpResult : IResult
+public sealed class Created<TValue> : IResult
+{
-    public object? Value { get; }
+    public TValue? Value { get; }
+}

-public sealed partial class JsonHttpResult : IResult
+public sealed partial class JsonHttpResult<TValue> : IResult
{
-    public object? Value { get; }
+    public TValue? Value { get; }
}

+public sealed class Ok : IResult, IEndpointMetadataProvider
+{
+    public int StatusCode => StatusCodes.Status200OK;

+    public Task ExecuteAsync(HttpContext context)
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
+}

-public sealed class OkObjectHttpResult : IResult
+public sealed class Ok<TValue> : IResult
+{
-    public object? Value { get; }
+    public TValue? Value { get; }
+}

+public sealed class UnprocessableEntity : IResult, IEndpointMetadataProvider
+{
+    public int StatusCode => StatusCodes.Status422UnprocessableEntity;

+    public Task ExecuteAsync(HttpContext context)
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
+}

-public sealed class UnprocessableEntityObjectHttpResult : IResult
+public sealed class UnprocessableEntity<TValue> : IResult
+{
-    public object? Value { get; }
+    public TValue? Value { get; }
+}

New ValidationProblem IResult type that represents response type for validation errors

namespace Microsoft.AspNetCore.Http.HttpResults;

+public sealed class ValidationProblem : IResult, IEndpointMetadataProvider
+{
+    public HttpValidationProblemDetails ProblemDetails { get; }
+    public string ContentType => "application/problem+json";
+    public int StatusCode => StatusCodes.Status400BadRequest;

+    public Task ExecuteAsync(HttpContext httpContext){}
+    public static void PopulateMetadata(EndpointMetadataContext context)
+}

Implement IEndpointMetadataProvider on appropriate result types and shorten their names

-namespace Microsoft.AspNetCore.Http;
+namespace Microsoft.AspNetCore.Http.HttpResults;

-public sealed class AcceptedHttpResult : IResult
+public sealed class Accepted : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class AcceptedAtRouteHttpResult : IResult
+public sealed class AcceptedAtRoute : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class BadRequestObjectHttpResult : IResult
+public sealed class BadRequest : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class ConflictObjectHttpResult : IResult
+public sealed class Conflict : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class CreatedHttpResult : IResult
+public sealed class Created : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class CreatedAtRouteHttpResult : IResult
+public sealed class CreatedAtRoute : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class NoContentHttpResult : IResult
+public sealed class NoContent : IResult, IEndpointMetadataProvider {   }

-public sealed class NotFoundObjectHttpResult : IResult
+public sealed class NotFound : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class OkObjectHttpResult : IResult
+public sealed class Ok : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class UnprocessableEntityObjectHttpResult : IResult
+public sealed class UnprocessableEntity : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

@brunolins16
Copy link
Member

@DamianEdwards based on the PR we are also moving all the other IResult concrete types, that not have their names shortened, to the Microsoft.AspNetCore.Http.HttpResults namespace, right?

@DamianEdwards
Copy link
Member Author

Ah yes, thanks I'll update.

@halter73
Copy link
Member

API Review:

  • It was nice using verbs vs nouns to determine if we should use the HttpResult suffix for the type, but using whether or not the type implements IEndpointMetadataProvider is easier.
  • Like the new HttpResults subnamespace for all IResult implementations.
  • Looks good as proposed. Will await community feedback to decide if any of the type names are too generic.
namespace Microsoft.AspNetCore.Http;

+public static class TypedResults
+{
+    public static ChallengeHttpResult Challenge(
+        AuthenticationProperties? properties = null,
+        IList<string>? authenticationSchemes = null) {}
       
+    public static ForbidHttpResult Forbid(
+        AuthenticationProperties? properties = null, 
+        IList<string>? authenticationSchemes = null) {}
       
+    public static SignInHttpResult SignIn(
+        ClaimsPrincipal principal,
+        AuthenticationProperties? properties = null,
+        string? authenticationScheme = null) {}
       
+    public static SignOutHttpResult SignOut(
+        AuthenticationProperties? properties = null, 
+        IList<string>? authenticationSchemes = null) {}
       
+    public static ContentHttpResult Content(
+        string content, 
+        string? contentType = null, 
+        Encoding? contentEncoding = null) {}
       
+    public static ContentHttpResult Text(
+        string content, 
+        string? contentType = null, 
+        Encoding? contentEncoding = null) {}
       
+    public static ContentHttpResult Content(
+        string content, 
+        MediaTypeHeaderValue contentType) {}
       
+    public static JsonHttpResult<TValue> Json<TValue>(
+        TValue? data, 
+        JsonSerializerOptions? options = null, 
+        string? contentType = null, 
+        int? statusCode = null)  {}
       
+    public static FileContentHttpResult File(
+        byte[] fileContents,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        bool enableRangeProcessing = false,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null)  {}
       
+    public static FileContentHttpResult Bytes(
+        byte[] contents,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        bool enableRangeProcessing = false,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null)
+        => new(contents, contentType)  {}
       
+    public static FileContentHttpResult Bytes(
+        ReadOnlyMemory<byte> contents,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        bool enableRangeProcessing = false,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null) {}
       
+    public static FileStreamHttpResult File(
+        Stream fileStream,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null,
+        bool enableRangeProcessing = false)  {}
       
+    public static FileStreamHttpResult Stream(
+        Stream stream,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null,
+        bool enableRangeProcessing = false) {}
       
+    public static FileStreamHttpResult Stream(
+        PipeReader pipeReader,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null,
+        bool enableRangeProcessing = false) {}
       
+    public static PushStreamHttpResult Stream(
+        Func<Stream, Task> streamWriterCallback,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null)  {}
       
+    public static PhysicalFileHttpResult PhysicalFile(
+        string path,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null,
+        bool enableRangeProcessing = false)  {}
       
+    public static VirtualFileHttpResult VirtualFile(
+        string path,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null,
+        bool enableRangeProcessing = false) {}
       
+    public static RedirectHttpResult Redirect(
+        string url, 
+        bool permanent = false, 
+        bool preserveMethod = false) {}
       
+    public static RedirectHttpResult LocalRedirect(
+        string localUrl, 
+        bool permanent = false, 
+        bool preserveMethod = false) {}
       
+    public static RedirectToRouteHttpResult RedirectToRoute(
+        string? routeName = null, 
+        object? routeValues = null, 
+        bool permanent = false, 
+        bool preserveMethod = false, 
+        string? fragment = null) {}
       
+    public static StatusHttpResult StatusCode(int statusCode) {}
       
+    public static NotFound NotFound() {}
       
+    public static NotFound<TValue> NotFound<TValue>(TValue? value) {}
       
+    public static UnauthorizedHttpResult Unauthorized() {}
       
+    public static BadRequest BadRequest() {}
       
+    public static BadRequest<TValue> BadRequest<TValue>(TValue? error) {}
       
+    public static Conflict Conflict() {}
       
+    public static Conflict<TValue> Conflict<TValue>(TValue? error) {}
       
+    public static NoContent NoContent() {}
       
+    public static Ok Ok() {}
       
+    public static Ok<TValue> Ok<TValue>(TValue? value) {}
       
+    public static UnprocessableEntity UnprocessableEntity() {}
       
+    public static UnprocessableEntity<TValue> UnprocessableEntity<TValue>(TValue? error) {}
       
+    public static ProblemHttpResult Problem(
+        string? detail = null,
+        string? instance = null,
+        int? statusCode = null,
+        string? title = null,
+        string? type = null,
+        IDictionary<string, object?>? extensions = null) {}
       
+    public static ProblemHttpResult Problem(ProblemDetails problemDetails) {}
       
+    public static ValidationProblem ValidationProblem(
+        IDictionary<string, string[]> errors,
+        string? detail = null,
+        string? instance = null,
+        int? statusCode = null,
+        string? title = null,
+        string? type = null,
+        IDictionary<string, object?>? extensions = null) {}
              
+    public static Created Created(string uri) {}
       
+    public static Created<TValue> Created<TValue>(
+        string uri, 
+        TValue? value) {}       

+    public static Created Created(Uri uri) {}
       
+    public static Created<TValue> Created<TValue>(
+        Uri uri, 
+        TValue? value) {}
       
+    public static CreatedAtRoute CreatedAtRoute(
+        string? routeName = null, 
+        object? routeValues = null) {}
       
+    public static CreatedAtRoute<TValue> CreatedAtRoute<TValue>(
+        TValue? value, 
+        string? routeName = null, 
+        object? routeValues = null) {}
       
+    public static Accepted Accepted(string uri) {}
       
+    public static Accepted<TValue> Accepted<TValue>(
+        tring uri, 
+        TValue? value) {}
       
+    public static Accepted Accepted(Uri uri) {}
       
+    public static Accepted<TValue> Accepted<TValue>(
+        Uri uri, 
+        TValue? value) {}
       
+    public static AcceptedAtRoute AcceptedAtRoute(
+        string? routeName = null, 
+        object? routeValues = null) {}
       
+    public static AcceptedAtRoute<TValue> AcceptedAtRoute<TValue>(
+        TValue? value, 
+        string? routeName = null, 
+        object? routeValues = null) {}       
+
+    public static EmptyHttpResult Empty { get; }
+}

+namespace Microsoft.AspNetCore.Http.HttpResults;

-public sealed class AcceptedHttpResult : IResult
+public sealed class Accepted : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class AcceptedAtRouteHttpResult : IResult
+public sealed class AcceptedAtRoute : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class BadRequestObjectHttpResult : IResult
+public sealed class BadRequest : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class ConflictObjectHttpResult : IResult
+public sealed class Conflict : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class CreatedHttpResult : IResult
+public sealed class Created : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class CreatedAtRouteHttpResult : IResult
+public sealed class CreatedAtRoute : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class NoContentHttpResult : IResult
+public sealed class NoContent : IResult, IEndpointMetadataProvider {   }

-public sealed class NotFoundObjectHttpResult : IResult
+public sealed class NotFound : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class OkObjectHttpResult : IResult
+public sealed class Ok : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

-public sealed class UnprocessableEntityObjectHttpResult : IResult
+public sealed class UnprocessableEntity : IResult, IEndpointMetadataProvider
{
-    public object? Value { get; }
+    static void IEndpointMetadataProvider.PopulateMetadata(EndpointMetadataContext context)
}

+public sealed class ValidationProblem : IResult, IEndpointMetadataProvider
+{
+    public HttpValidationProblemDetails ProblemDetails { get; }
+    public string ContentType => "application/problem+json";
+    public int StatusCode => StatusCodes.Status400BadRequest;

+    public Task ExecuteAsync(HttpContext httpContext){}
+    public static void PopulateMetadata(EndpointMetadataContext context)
+}

+public sealed class AcceptedAtRoute<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public string? RouteName { get; }
+    public RouteValueDictionary RouteValues { get; }
+    public int StatusCode => StatusCodes.Status202Accepted;

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

+public sealed class Accepted<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public int StatusCode => StatusCodes.Status202Accepted;
+    public string? Location { get; }

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

+public sealed class BadRequest<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public int StatusCode => StatusCodes.Status400BadRequest;
+
+    public Task ExecuteAsync(HttpContext httpContext){}
+}

+public sealed class Conflict<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public int StatusCode => StatusCodes.Status409Conflict;
+
+    public Task ExecuteAsync(HttpContext httpContext){}
+}

+public sealed class CreatedAtRoute<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public string? RouteName { get; }
+    public RouteValueDictionary RouteValues { get; }
+    public int StatusCode => StatusCodes.Status201Created;

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

+public sealed class Created<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public int StatusCode => StatusCodes.Status201Created;
+    public string? Location { get; }

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

-public sealed partial class JsonHttpResult : IResult
+public sealed partial class Json<TValue> : IResult
{
-    public object Value { get; }
+    public TValue? Value { get; }
}

+public sealed class Ok<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public int StatusCode => StatusCodes.Status200OK;
+
+    public Task ExecuteAsync(HttpContext httpContext){}
+}

+public sealed class UnprocessableEntity<TValue> : IResult
+{
+    public TValue? Value { get; }
+    public int StatusCode => StatusCodes.Status422UnprocessableEntity;
+
+    public Task ExecuteAsync(HttpContext httpContext){}
+}

@halter73 halter73 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 Apr 18, 2022
@DamianEdwards
Copy link
Member Author

DamianEdwards commented Apr 18, 2022

If we end up circling back on this WRT the result type names, a thought I just had was to prefix them with Http instead. That seems to keep the type names fairly reasonable in the method signature, e.g.:

app.MapGet("/todo/{id}", async Task<Results<HttpOk<Todo>, HttpNotFound>> (int id, TodoDb db) =>
    await db.Todos.FindAsync(id) is Todo todo
        ? TypedResults.Ok(todo)
        : TypedResults.NotFound();

DamianEdwards added a commit that referenced this issue Apr 18, 2022
…cts (#41161)

- Use the `HttpResult` suffix on `IResult` types except for where the short name has recognized value, e.g. those types that implement `IEndpointMetadataProvider` and will be used in conjunction with `Results<T1, TN>`, e.g. `Results<Ok<Todo>, NotFound>`
- Add `TypedResults.xxx` factory class
- Added `HttpResults.ValidationProblem` type that represents 400 responses with validation errors (equiv to `BadRequest<HttpValidationProblemDetails>`
- Moved `Results<TResult1, TResultN>` types into the `Microsoft.AspNetCore.Http.HttpResults` namespace to match where the other results types are
- Changed `Results.xxx` methods to call through to `TypedResults.xxx` methods
- Explicitly implemented `IEndpointMetadataProvider` on following `IResult` types:
  - `Accepted`
  - `Accepted<TValue>`
  - `AcceptedAtRoute`
  - `AcceptedAtRoute<TValue>` 
  - `BadRequest`
  - `BadRequest<TValue>`
  - `Conflict`
  - `Conflict<TValue>`
  - `Created`
  - `Created<TValue>`
  - `CreatedAtRoute`
  - `CreatedAtRoute<TValue>`
  - `NoContent`
  - `NotFound`
  - `NotFound<TValue>`
  - `Ok`
  - `Ok<TValue>`
  - `UnprocessableEntity`
  - `UnprocessableEntity<TValue>`
- Order `using` statements before `namespace` declarations
- Added tests for `Microsoft.AspNetCore.Http.Results` and `Microsoft.AspNetCore.Http.TypedResults`

Fixes #41009

Co-authored-by: Bruno Lins de Oliveira <brolivei@microsoft.com>
@dotnet dotnet locked as resolved and limited conversation to collaborators May 19, 2022
@rafikiassumani-msft rafikiassumani-msft added the Docs This issue tracks updating documentation label Aug 4, 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 Docs This issue tracks updating documentation feature-minimal-actions Controller-like actions for endpoint routing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants