Make Result types constructors public

[brunolins16]

Background and Motivation

Result types were changed to be public but was decided to keep ctors internal to have a better design discussion.

The original proposal is described in #40656

Proposed API

namespace Microsoft.AspNetCore.Http;

public class EmptyHttpResult : IResult
{
-    public static readonly EmptyHttpResult Instance { get; } = new();
+    public EmptyHttpResult() {} 
}

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

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

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

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

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

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

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

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

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

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

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

public sealed class UnprocessableEntityObjectHttpResult : IResult
{
+    public UnprocessableEntityObjectHttpResult(object? value = null) {}
}

public sealed class ProblemHttpResult : IResult
{
+    public ProblemHttpResult(ProblemDetails problemDetails) {}
+    public ProblemHttpResult(
+        string? detail = null,
+        string? instance = null,
+        int? statusCode = null,
+        string? title = null,
+        string? type = null,
+        IDictionary<string, object?>? extensions = null) {}
}

public sealed class JsonHttpResult : IResult
{
+    public JsonHttpResult(object? value = null, JsonSerializerOptions? jsonSerializerOptions = null) {}

-    public string ContentType { get; }
+    public string ContentType { get; init; }

-    public int? StatusCode { get; }
+    public int? StatusCode { get; init; }
}

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

-    public int? StatusCode { get; }
+    public int? StatusCode { get; init; }
}

public sealed partial class FileContentHttpResult : IResult
{
+    public FileContentHttpResult(ReadOnlyMemory<byte> fileContents, string? contentType = null) {}

-    public bool EnableRangeProcessing { get; }
+    public bool EnableRangeProcessing { get; init; }

-    public EntityTagHeaderValue? EntityTag { get; }
+    public EntityTagHeaderValue? EntityTag { get; init; }

-    public string? FileDownloadName { get; }
+    public string? FileDownloadName { get; init; }

-    public DateTimeOffset? LastModified { get; }
+    public DateTimeOffset? LastModified { get; init; }
}

public sealed class FileStreamHttpResult : IResult
{
+    public FileStreamHttpResult(Stream fileStream, string? contentType = null) {}

-    public bool EnableRangeProcessing { get; }
+    public bool EnableRangeProcessing { get; init; }

-    public EntityTagHeaderValue? EntityTag { get; }
+    public EntityTagHeaderValue? EntityTag { get; init; }

-    public string? FileDownloadName { get; }
+    public string? FileDownloadName { get; init; }

-    public DateTimeOffset? LastModified { get; }
+    public DateTimeOffset? LastModified { get; init; }
}

public sealed partial class PhysicalFileHttpResult : IResult
{
+    public PhysicalFileHttpResult(string fileName, string? contentType = null) {}

-    public bool EnableRangeProcessing { get; }
+    public bool EnableRangeProcessing { get; init; }

-    public EntityTagHeaderValue? EntityTag { get; }
+    public EntityTagHeaderValue? EntityTag { get; init; }

-    public string? FileDownloadName { get; }
+    public string? FileDownloadName { get; init; }

-    public DateTimeOffset? LastModified { get; }
+    public DateTimeOffset? LastModified { get; init; }
}

public sealed class PushStreamHttpResult : IResult
{
+    public PushStreamHttpResult(Func<Stream, Task> streamWriterCallback, string? contentType = null) {}

-    public bool EnableRangeProcessing { get; }
+    public bool EnableRangeProcessing { get; init; }

-    public EntityTagHeaderValue? EntityTag { get; }
+    public EntityTagHeaderValue? EntityTag { get; init; }

-    public string? FileDownloadName { get; }
+    public string? FileDownloadName { get; init; }

-    public DateTimeOffset? LastModified { get; }
+    public DateTimeOffset? LastModified { get; init; }
}

public sealed partial class VirtualFileHttpResult : IResult
{
+    public VirtualFileHttpResult(string fileName, string? contentType = null) {}

-    public bool EnableRangeProcessing { get; }
+    public bool EnableRangeProcessing { get; init; }

-    public EntityTagHeaderValue? EntityTag { get; }
+    public EntityTagHeaderValue? EntityTag { get; init; }

-    public string? FileDownloadName { get; }
+    public string? FileDownloadName { get; init; }

-    public DateTimeOffset? LastModified { get; }
+    public DateTimeOffset? LastModified { get; init; }
}

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

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

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

public sealed partial class SignInHttpResult : IResult
{
+    public SignInHttpResult(ClaimsPrincipal principal) {}

-    public string? AuthenticationScheme { get; }
+    public string? AuthenticationScheme { get; init; }

-    public AuthenticationProperties? Properties { get; }
+    public AuthenticationProperties? Properties { get; init; }
}

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

-    public bool Permanent { get; }
+    public bool Permanent { get; init; }

-    public bool PreserveMethod { get; }
+    public bool PreserveMethod { get; init; }
}

public sealed partial class RedirectToRouteHttpResult : IResult
{
+    public RedirectToRouteHttpResult(string? routeName = null, object? routeValues = null) {}

-    public bool Permanent { get; }
+    public bool Permanent { get; init; }

-    public bool PreserveMethod { get; }
+    public bool PreserveMethod { get; init; }

-    public string Fragment { get; }
+    public string Fragment { get; init; }
}

[ghost]

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]

API Review:

1. We're unsure about Instance vs. Default vs. Empty. Are we going to need this at all if Results.Typed.... returns a strongly typed shared instance? We think this should wait.
2. We want to constructors, but we want to be more consistent with default values vs. overloads vs. init-only properties.

[brunolins16]

@halter73 for those two types:

public class NoContentHttpResult : IResult
{
+    public static NoContentHttpResult Instance { get; } = new();
}

public class UnauthorizedHttpResult : IResult
{
+    public static UnauthorizedHttpResult Instance { get; } = new();
}

Should they have the Instance property or the default ctor?

[halter73]

If we decide to add constructors to any of these types, I think every one of them should have at least one constructor even if there's an equivalent Instance property that's more efficient. Most people aren't going to care about one extra allocation a request, so I don't think that's an excuse to make the API less consistent.

My thinking is we should get rid of the public Instance properties altogether and just return the shared instance from Results. and Results.Typed. methods.

[davidfowl]

My thinking is we should get rid of the public Instance properties altogether and just return the shared instance from Results. and Results.Typed. methods.

Also the non typed ones. I guess I wasn't clear but I expected us to change the existing impl to cache.

[halter73]

API Review:

On second thought, we think the Results.Typed methods described in #41009 will be sufficient. We can always add the constructors later.