Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0ab4de9
StatuCodeResult as base class
brunolins16 Mar 9, 2022
813786e
StatuCodeResult as base class
brunolins16 Mar 9, 2022
811c604
Making types public and renaming to HTTPResult
brunolins16 Mar 10, 2022
5d66c24
Update comments
brunolins16 Mar 11, 2022
acf0a3b
Make setters internal
brunolins16 Mar 11, 2022
9ddf586
Updating public API file
brunolins16 Mar 11, 2022
9a79b02
Removing base classes
brunolins16 Mar 14, 2022
a1fef75
Changing method name to WriteResultAsStatusCode
brunolins16 Mar 14, 2022
1ee3138
Updating API
brunolins16 Mar 14, 2022
2c029a8
code cleanup
brunolins16 Mar 14, 2022
4103a0f
fixing public api
brunolins16 Mar 15, 2022
5fdebc4
Simplyfing interfaces
brunolins16 Mar 15, 2022
6146e4a
Removing interfaces + Adding ctors
brunolins16 Mar 18, 2022
7d89ce3
Fixing xml docs
brunolins16 Mar 18, 2022
6171f15
Making EmptyResult public
brunolins16 Mar 18, 2022
6a807cf
Updating JsonHttpResult
brunolins16 Mar 18, 2022
5fa233d
Adding missing ctor
brunolins16 Mar 18, 2022
dcf745b
Adding missing ctor
brunolins16 Mar 18, 2022
3d01bb6
Adding missing ctor
brunolins16 Mar 18, 2022
e05b04f
Adding misssed xml comment
brunolins16 Mar 18, 2022
9c628aa
Removing configureResponseHeader
brunolins16 Mar 18, 2022
eada9b4
PR Feedback
brunolins16 Mar 18, 2022
ff3cb76
Moving ctors to internal
brunolins16 Mar 18, 2022
00877b5
Create logger using string category
brunolins16 Mar 18, 2022
825f4c0
Adding more unittests
brunolins16 Mar 18, 2022
73800b9
Update src/Http/Http.Results/src/EmptyHttpResult.cs
brunolins16 Mar 18, 2022
8fb15dd
Update ContentHttpResult.cs
brunolins16 Mar 18, 2022
dac8711
avoid closure allocation
brunolins16 Mar 18, 2022
4a50104
Merge branch 'brunolins16/iresult-apisuggestion' of https://github.co…
brunolins16 Mar 18, 2022
1d76b6e
unit loggerfactory
brunolins16 Mar 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,56 +1,67 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.Http;

using System.Threading.Tasks;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;

namespace Microsoft.AspNetCore.Http.Result;

internal sealed class AcceptedAtRouteResult : ObjectResult
/// <summary>
/// An <see cref="IResult"/> that on execution will write an object to the response
/// with status code Accepted (202) and Location header.
/// Targets a registered route.
/// </summary>
public sealed class AcceptedAtRouteHttpResult : IResult
{
/// <summary>
/// Initializes a new instance of the <see cref="AcceptedAtRouteResult"/> class with the values
/// Initializes a new instance of the <see cref="AcceptedAtRouteHttpResult"/> class with the values
/// provided.
/// </summary>
/// <param name="routeValues">The route data to use for generating the URL.</param>
/// <param name="value">The value to format in the entity body.</param>
public AcceptedAtRouteResult(object? routeValues, object? value)
public AcceptedAtRouteHttpResult(object? routeValues, object? value)
: this(routeName: null, routeValues: routeValues, value: value)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="AcceptedAtRouteResult"/> class with the values
/// Initializes a new instance of the <see cref="AcceptedAtRouteHttpResult"/> class with the values
/// provided.
/// </summary>
/// <param name="routeName">The name of the route to use for generating the URL.</param>
/// <param name="routeValues">The route data to use for generating the URL.</param>
/// <param name="value">The value to format in the entity body.</param>
public AcceptedAtRouteResult(
public AcceptedAtRouteHttpResult(
string? routeName,
object? routeValues,
object? value)
: base(value, StatusCodes.Status202Accepted)
{
Value = value;
RouteName = routeName;
RouteValues = new RouteValueDictionary(routeValues);
}

/// <summary>
/// Gets the name of the route to use for generating the URL.
/// </summary>
/// <inheritdoc/>
public object? Value { get; }

/// <inheritdoc/>
public string? RouteName { get; }

/// <summary>
/// Gets the route data to use for generating the URL.
/// </summary>
public RouteValueDictionary RouteValues { get; }
/// <inheritdoc/>
public RouteValueDictionary? RouteValues { get; }

/// <inheritdoc/>
public int StatusCode => StatusCodes.Status202Accepted;

/// <inheritdoc/>
public Task ExecuteAsync(HttpContext httpContext)
=> HttpResultsWriter.WriteResultAsJsonAsync(httpContext, Value, StatusCode, configureResponseHeader: ConfigureResponseHeaders);

/// <inheritdoc />
protected override void ConfigureResponseHeaders(HttpContext context)
private void ConfigureResponseHeaders(HttpContext context)
{
var linkGenerator = context.RequestServices.GetRequiredService<LinkGenerator>();
var url = linkGenerator.GetUriByAddress(
var url = linkGenerator.GetUriByRouteValues(
context,
RouteName,
RouteValues,
Expand Down
76 changes: 76 additions & 0 deletions src/Http/Http.Results/src/AcceptedHttpResult.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.Http;

using System.Threading.Tasks;

/// <summary>
/// An <see cref="IResult"/> that on execution will write an object to the response
/// with status code Accepted (202) and Location header.
/// Targets a registered route.
/// </summary>
public sealed class AcceptedHttpResult : IResult
{
/// <summary>
/// Initializes a new instance of the <see cref="AcceptedHttpResult"/> class with the values
/// provided.
/// </summary>
/// <param name="location">The location at which the status of requested content can be monitored.</param>
/// <param name="value">The value to format in the entity body.</param>
public AcceptedHttpResult(string? location, object? value)
{
Value = value;
Location = location;
}

/// <summary>
/// Initializes a new instance of the <see cref="AcceptedHttpResult"/> class with the values
/// provided.
/// </summary>
/// <param name="locationUri">The location at which the status of requested content can be monitored.</param>
/// <param name="value">The value to format in the entity body.</param>
public AcceptedHttpResult(Uri locationUri, object? value)
{
Value = value;

if (locationUri == null)
{
throw new ArgumentNullException(nameof(locationUri));
}

if (locationUri.IsAbsoluteUri)
{
Location = locationUri.AbsoluteUri;
}
else
{
Location = locationUri.GetComponents(UriComponents.SerializationInfoString, UriFormat.UriEscaped);
}
}

/// <inheritdoc/>
public object? Value { get; }

/// <inheritdoc/>
public int StatusCode => StatusCodes.Status202Accepted;

/// <inheritdoc/>
public string? Location { get; }

/// <inheritdoc/>
public Task ExecuteAsync(HttpContext httpContext)
{
return HttpResultsWriter.WriteResultAsJsonAsync(
httpContext,
Value,
StatusCode,
configureResponseHeader: (context) =>
{
if (!string.IsNullOrEmpty(Location))
{
context.Response.Headers.Location = Location;
}
});
}
}
66 changes: 0 additions & 66 deletions src/Http/Http.Results/src/AcceptedResult.cs

This file was deleted.

31 changes: 31 additions & 0 deletions src/Http/Http.Results/src/BadRequestObjectHttpResult.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.Http;

/// <summary>
/// An <see cref="IResult"/> that on execution will write an object to the response
/// with Bad Request (400) status code.
/// </summary>
public sealed class BadRequestObjectHttpResult : IResult
{
/// <summary>
/// Initializes a new instance of the <see cref="BadRequestObjectHttpResult"/> class with the values
/// provided.
/// </summary>
/// <param name="error">The error content to format in the entity body.</param>
public BadRequestObjectHttpResult(object? error)
{
Value = error;
}

/// <inheritdoc/>
public object? Value { get; internal init; }

/// <inheritdoc/>
public int StatusCode => StatusCodes.Status400BadRequest;

/// <inheritdoc/>
public Task ExecuteAsync(HttpContext httpContext)
=> HttpResultsWriter.WriteResultAsJsonAsync(httpContext, Value, StatusCode);
}
12 changes: 0 additions & 12 deletions src/Http/Http.Results/src/BadRequestObjectResult.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,84 +6,91 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

namespace Microsoft.AspNetCore.Http.Result;
namespace Microsoft.AspNetCore.Http;

/// <summary>
/// An <see cref="IResult"/> that on execution invokes <see cref="M:HttpContext.ChallengeAsync"/>.
/// </summary>
internal sealed partial class ChallengeResult : IResult
public sealed partial class ChallengeHttpResult : IResult
{
/// <summary>
/// Initializes a new instance of <see cref="ChallengeResult"/>.
/// Initializes a new instance of <see cref="ChallengeHttpResult"/>.
/// </summary>
public ChallengeResult()
public ChallengeHttpResult()
: this(Array.Empty<string>())
{
}

/// <summary>
/// Initializes a new instance of <see cref="ChallengeResult"/> with the
/// Initializes a new instance of <see cref="ChallengeHttpResult"/> with the
/// specified authentication scheme.
/// </summary>
/// <param name="authenticationScheme">The authentication scheme to challenge.</param>
public ChallengeResult(string authenticationScheme)
public ChallengeHttpResult(string authenticationScheme)
: this(new[] { authenticationScheme })
{
}

/// <summary>
/// Initializes a new instance of <see cref="ChallengeResult"/> with the
/// Initializes a new instance of <see cref="ChallengeHttpResult"/> with the
/// specified authentication schemes.
/// </summary>
/// <param name="authenticationSchemes">The authentication schemes to challenge.</param>
public ChallengeResult(IList<string> authenticationSchemes)
public ChallengeHttpResult(IList<string> authenticationSchemes)
: this(authenticationSchemes, properties: null)
{
}

/// <summary>
/// Initializes a new instance of <see cref="ChallengeResult"/> with the
/// Initializes a new instance of <see cref="ChallengeHttpResult"/> with the
/// specified <paramref name="properties"/>.
/// </summary>
/// <param name="properties"><see cref="AuthenticationProperties"/> used to perform the authentication
/// challenge.</param>
public ChallengeResult(AuthenticationProperties? properties)
public ChallengeHttpResult(AuthenticationProperties? properties)
: this(Array.Empty<string>(), properties)
{
}

/// <summary>
/// Initializes a new instance of <see cref="ChallengeResult"/> with the
/// Initializes a new instance of <see cref="ChallengeHttpResult"/> with the
/// specified authentication scheme and <paramref name="properties"/>.
/// </summary>
/// <param name="authenticationScheme">The authentication schemes to challenge.</param>
/// <param name="properties"><see cref="AuthenticationProperties"/> used to perform the authentication
/// challenge.</param>
public ChallengeResult(string authenticationScheme, AuthenticationProperties? properties)
public ChallengeHttpResult(string authenticationScheme, AuthenticationProperties? properties)
: this(new[] { authenticationScheme }, properties)
{
}

/// <summary>
/// Initializes a new instance of <see cref="ChallengeResult"/> with the
/// Initializes a new instance of <see cref="ChallengeHttpResult"/> with the
/// specified authentication schemes and <paramref name="properties"/>.
/// </summary>
/// <param name="authenticationSchemes">The authentication scheme to challenge.</param>
/// <param name="properties"><see cref="AuthenticationProperties"/> used to perform the authentication
/// challenge.</param>
public ChallengeResult(IList<string> authenticationSchemes, AuthenticationProperties? properties)
public ChallengeHttpResult(IList<string> authenticationSchemes, AuthenticationProperties? properties)
{
AuthenticationSchemes = authenticationSchemes;
AuthenticationSchemes = authenticationSchemes.AsReadOnly();
Properties = properties;
}

public IList<string> AuthenticationSchemes { get; init; } = Array.Empty<string>();
/// <summary>
/// Gets or sets the authentication schemes that are challenged.
/// </summary>
public IReadOnlyList<string> AuthenticationSchemes { get; internal init; } = Array.Empty<string>();

public AuthenticationProperties? Properties { get; init; }
/// <summary>
/// Gets or sets the <see cref="AuthenticationProperties"/> used to perform the sign-out operation.
/// </summary>
public AuthenticationProperties? Properties { get; internal init; }

/// <inheritdoc/>
public async Task ExecuteAsync(HttpContext httpContext)
{
var logger = httpContext.RequestServices.GetRequiredService<ILogger<ChallengeResult>>();
var logger = httpContext.RequestServices.GetRequiredService<ILogger<ChallengeHttpResult>>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we care about changing log categories? I could see that being considered a breaking change. I don't see a huge benefit of changing it other than slightly cleaner code. @BrennanConroy @davidfowl

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is super important for preview3, but I'd like to clarify our stance on changing log categories in general.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing categories is considered a breaking change, example

// We create the logger with a string to preserve the logging namespace after the server side transport renames.
_logger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Http.Connections.Internal.Transports.WebSocketsTransport");

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, i think we should not change the categories. I have pushed a commit following @BrennanConroy example.

@halter73 / @BrennanConroy can you do a quick review? 00877b5


Log.ChallengeResultExecuting(logger, AuthenticationSchemes);

Expand All @@ -102,7 +109,7 @@ public async Task ExecuteAsync(HttpContext httpContext)

private static partial class Log
{
public static void ChallengeResultExecuting(ILogger logger, IList<string> authenticationSchemes)
public static void ChallengeResultExecuting(ILogger logger, IReadOnlyList<string> authenticationSchemes)
{
if (logger.IsEnabled(LogLevel.Information))
{
Expand Down
Loading