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.

Sign up for GitHub

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

Jump to bottom

Add new "MapAction" overloads #30556

Merged
4 commits merged into from
Mar 2, 2021
Merged

Add new "MapAction" overloads #30556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e16e9f
Add new "MapAction" overloads
halter73 Mar 2, 2021
be55d9c
Create RouteEndpointBuilder directly
halter73 Mar 2, 2021
90b86bf
fix typo: my -> by
halter73 Mar 2, 2021
91d6042
is { } -> is not null
halter73 Mar 2, 2021
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
5 changes: 5 additions & 0 deletions src/Http/Routing/src/Builder/MapActionEndpointConventionBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ public sealed class MapActionEndpointConventionBuilder : IEndpointConventionBuil
{
private readonly List<IEndpointConventionBuilder> _endpointConventionBuilders;

internal MapActionEndpointConventionBuilder(IEndpointConventionBuilder endpointConventionBuilder)
{
_endpointConventionBuilders = new List<IEndpointConventionBuilder>() { endpointConventionBuilder };
}

internal MapActionEndpointConventionBuilder(List<IEndpointConventionBuilder> endpointConventionBuilders)
{
_endpointConventionBuilders = endpointConventionBuilders;
Expand Down
205 changes: 205 additions & 0 deletions src/Http/Routing/src/Builder/MapActionEndpointRouteBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Reflection;
using Microsoft.AspNetCore.Routing;
using Microsoft.AspNetCore.Routing.Internal;
using Microsoft.AspNetCore.Routing.Patterns;

namespace Microsoft.AspNetCore.Builder
{
Expand All @@ -15,6 +16,12 @@ namespace Microsoft.AspNetCore.Builder
/// </summary>
public static class MapActionEndpointRouteBuilderExtensions
{
// Avoid creating a new array every call
private static readonly string[] GetVerb = new[] { "GET" };
JamesNK marked this conversation as resolved.
Show resolved Hide resolved
private static readonly string[] PostVerb = new[] { "POST" };
private static readonly string[] PutVerb = new[] { "PUT" };
private static readonly string[] DeleteVerb = new[] { "DELETE" };

/// <summary>
/// Adds a <see cref="RouteEndpoint"/> to the <see cref="IEndpointRouteBuilder"/> that matches the pattern specified via attributes.
/// </summary>
Expand Down Expand Up @@ -76,5 +83,203 @@ public static MapActionEndpointConventionBuilder MapAction(

return new MapActionEndpointConventionBuilder(conventionBuilders);
}

/// <summary>
/// Adds a <see cref="RouteEndpoint"/> to the <see cref="IEndpointRouteBuilder"/> that matches HTTP GET requests
/// for the specified pattern.
/// </summary>
/// <param name="endpoints">The <see cref="IEndpointRouteBuilder"/> to add the route to.</param>
/// <param name="pattern">The route pattern.</param>
/// <param name="action">The delegate executed when the endpoint is matched.</param>
/// <returns>A <see cref="IEndpointConventionBuilder"/> that can be used to further customize the endpoint.</returns>
public static MapActionEndpointConventionBuilder MapGet(
Copy link
Member

Choose a reason for hiding this comment

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

Aren't there already Map* extension methods on route builder? Do these not colide because of Delegate parameter?

Copy link
Member

Choose a reason for hiding this comment

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

Nope.

Copy link
Member Author

Choose a reason for hiding this comment

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

These are overloads, but they should only be used for delegates that aren't RequestDelegates. This was also mentioned in dotnet/csharplang#4451 (comment).

this IEndpointRouteBuilder endpoints,
string pattern,
Delegate action)
{
return MapMethods(endpoints, pattern, GetVerb, action);
}

/// <summary>
/// Adds a <see cref="RouteEndpoint"/> to the <see cref="IEndpointRouteBuilder"/> that matches HTTP POST requests
/// for the specified pattern.
/// </summary>
/// <param name="endpoints">The <see cref="IEndpointRouteBuilder"/> to add the route to.</param>
/// <param name="pattern">The route pattern.</param>
/// <param name="action">The delegate executed when the endpoint is matched.</param>
/// <returns>A <see cref="IEndpointConventionBuilder"/> that can be used to further customize the endpoint.</returns>
public static MapActionEndpointConventionBuilder MapPost(
this IEndpointRouteBuilder endpoints,
string pattern,
Delegate action)
{
return MapMethods(endpoints, pattern, PostVerb, action);
}

/// <summary>
/// Adds a <see cref="RouteEndpoint"/> to the <see cref="IEndpointRouteBuilder"/> that matches HTTP PUT requests
/// for the specified pattern.
/// </summary>
/// <param name="endpoints">The <see cref="IEndpointRouteBuilder"/> to add the route to.</param>
/// <param name="pattern">The route pattern.</param>
/// <param name="action">The delegate executed when the endpoint is matched.</param>
/// <returns>A <see cref="IEndpointConventionBuilder"/> that canaction be used to further customize the endpoint.</returns>
public static MapActionEndpointConventionBuilder MapPut(
this IEndpointRouteBuilder endpoints,
string pattern,
Delegate action)
{
return MapMethods(endpoints, pattern, PutVerb, action);
}

/// <summary>
/// Adds a <see cref="RouteEndpoint"/> to the <see cref="IEndpointRouteBuilder"/> that matches HTTP DELETE requests
/// for the specified pattern.
/// </summary>
/// <param name="endpoints">The <see cref="IEndpointRouteBuilder"/> to add the route to.</param>
/// <param name="pattern">The route pattern.</param>
/// <param name="action">The delegate executed when the endpoint is matched.</param>
/// <returns>A <see cref="IEndpointConventionBuilder"/> that can be used to further customize the endpoint.</returns>
public static MapActionEndpointConventionBuilder MapDelete(
this IEndpointRouteBuilder endpoints,
string pattern,
Delegate action)
{
return MapMethods(endpoints, pattern, DeleteVerb, action);
}

/// <summary>
/// Adds a <see cref="RouteEndpoint"/> to the <see cref="IEndpointRouteBuilder"/> that matches HTTP requests
/// for the specified HTTP methods and pattern.
/// </summary>
/// <param name="endpoints">The <see cref="IEndpointRouteBuilder"/> to add the route to.</param>
/// <param name="pattern">The route pattern.</param>
/// <param name="action">The delegate executed when the endpoint is matched.</param>
/// <param name="httpMethods">HTTP methods that the endpoint will match.</param>
/// <returns>A <see cref="IEndpointConventionBuilder"/> that can be used to further customize the endpoint.</returns>
public static MapActionEndpointConventionBuilder MapMethods(
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this? As a user, there are alternate ways to re-use the delegate if we need to. I'm also similarly against allowing more than one HTTP attributes per MapAction delegate.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that the flexibility of delegates means that we shouldn't need to make routing metadata overly complicated. That's why I removed IRotePatternMetadata and IRouteOrderMetadata in #30563. I also stopped implementing IHttpMethodMetadata in attributes which caused multiple instances to show up on endpoints that didn't have multiple instances before. #29878 (comment)

All that said, this is an overload that already exists for RequestDelegate and any argument about reusing the Delegate, you could make for reusing the RequestDelegate. I'm just keeping it consistent by adding a Delegate overload for every RequestDelegate overload.

this IEndpointRouteBuilder endpoints,
string pattern,
IEnumerable<string> httpMethods,
Delegate action)
{
if (httpMethods is null)
{
throw new ArgumentNullException(nameof(httpMethods));
}

var displayName = $"{pattern} HTTP: {string.Join(", ", httpMethods)}";
var builder = endpoints.Map(RoutePatternFactory.Parse(pattern), action, displayName);
builder.WithMetadata(new HttpMethodMetadata(httpMethods));
return builder;
}

/// <summary>
/// Adds a <see cref="RouteEndpoint"/> to the <see cref="IEndpointRouteBuilder"/> that matches HTTP requests
/// for the specified pattern.
/// </summary>
/// <param name="endpoints">The <see cref="IEndpointRouteBuilder"/> to add the route to.</param>
/// <param name="pattern">The route pattern.</param>
/// <param name="action">The delegate executed when the endpoint is matched.</param>
/// <returns>A <see cref="IEndpointConventionBuilder"/> that can be used to further customize the endpoint.</returns>
public static MapActionEndpointConventionBuilder Map(
this IEndpointRouteBuilder endpoints,
string pattern,
Delegate action)
{
return Map(endpoints, RoutePatternFactory.Parse(pattern), action);
}

/// <summary>
/// Adds a <see cref="RouteEndpoint"/> to the <see cref="IEndpointRouteBuilder"/> that matches HTTP requests
/// for the specified pattern.
/// </summary>
/// <param name="endpoints">The <see cref="IEndpointRouteBuilder"/> to add the route to.</param>
/// <param name="pattern">The route pattern.</param>
/// <param name="action">The delegate executed when the endpoint is matched.</param>
/// <returns>A <see cref="IEndpointConventionBuilder"/> that can be used to further customize the endpoint.</returns>
public static MapActionEndpointConventionBuilder Map(
this IEndpointRouteBuilder endpoints,
RoutePattern pattern,
Delegate action)
{
return Map(endpoints, pattern, action, displayName: null);
}

private static MapActionEndpointConventionBuilder Map(
this IEndpointRouteBuilder endpoints,
RoutePattern pattern,
Delegate action,
string? displayName)
{
if (endpoints is null)
{
throw new ArgumentNullException(nameof(endpoints));
}

if (pattern is null)
{
throw new ArgumentNullException(nameof(pattern));
}

if (action is null)
{
throw new ArgumentNullException(nameof(action));
}

const int defaultOrder = 0;

var builder = new RouteEndpointBuilder(
MapActionExpressionTreeBuilder.BuildRequestDelegate(action),
pattern,
defaultOrder)
{
DisplayName = pattern.RawText ?? pattern.DebuggerToString(),
};

// Add delegate attributes as metadata
var attributes = action.Method.GetCustomAttributes();
string? routeName = null;
int? routeOrder = null;

// This can be null if the delegate is a dynamic method or compiled from an expression tree
if (attributes is not null)
{
foreach (var attribute in attributes)
{
if (attribute is IRoutePatternMetadata patternMetadata && patternMetadata.RoutePattern is not null)
{
throw new InvalidOperationException($"'{attribute.GetType()}' implements {nameof(IRoutePatternMetadata)} which is not supported by this method.");
}
if (attribute is IHttpMethodMetadata methodMetadata && methodMetadata.HttpMethods.Any())
{
throw new InvalidOperationException($"'{attribute.GetType()}' implements {nameof(IHttpMethodMetadata)} which is not supported by this method.");
}

if (attribute is IRouteNameMetadata nameMetadata && nameMetadata.RouteName is string name)
{
routeName = name;
}
if (attribute is IRouteOrderMetadata orderMetadata && orderMetadata.RouteOrder is int order)
{
routeOrder = order;
}

builder.Metadata.Add(attribute);
}
}

builder.DisplayName = routeName ?? displayName ?? builder.DisplayName;
builder.Order = routeOrder ?? defaultOrder;

var dataSource = endpoints.DataSources.OfType<ModelEndpointDataSource>().FirstOrDefault();
if (dataSource is null)
{
dataSource = new ModelEndpointDataSource();
endpoints.DataSources.Add(dataSource);
}

return new MapActionEndpointConventionBuilder(dataSource.AddEndpointBuilder(builder));
}
}
}
7 changes: 7 additions & 0 deletions src/Http/Routing/src/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,11 @@ Microsoft.AspNetCore.Routing.IRoutePatternMetadata
Microsoft.AspNetCore.Routing.IRoutePatternMetadata.RoutePattern.get -> string?
Microsoft.AspNetCore.Routing.RouteNameMetadata.RouteName.get -> string?
Microsoft.AspNetCore.Routing.RouteNameMetadata.RouteNameMetadata(string? routeName) -> void
static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.Map(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, Microsoft.AspNetCore.Routing.Patterns.RoutePattern! pattern, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder!
static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.Map(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder!
static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.MapAction(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder!
static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.MapDelete(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder!
static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.MapGet(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder!
static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.MapMethods(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Collections.Generic.IEnumerable<string!>! httpMethods, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder!
static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.MapPost(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder!
static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.MapPut(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder!
41 changes: 41 additions & 0 deletions src/Http/Routing/test/FunctionalTests/MapActionTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,47 @@ public async Task MapAction_FromBodyWorksWithJsonPayload()
Assert.Equal(42, echoedTodo?.Id);
}

[Fact]
public async Task MapPost_FromBodyWorksWithJsonPayload()
{
Todo EchoTodo([FromRoute] int id, [FromBody] Todo todo) => todo with { Id = id };

using var host = new HostBuilder()
.ConfigureWebHost(webHostBuilder =>
{
webHostBuilder
.Configure(app =>
{
app.UseRouting();
app.UseEndpoints(b => b.MapPost("/EchoTodo/{id}", (Func<int, Todo, Todo>)EchoTodo));
})
.UseTestServer();
})
.ConfigureServices(services =>
{
services.AddRouting();
})
.Build();

using var server = host.GetTestServer();
await host.StartAsync();
var client = server.CreateClient();

var todo = new Todo
{
Name = "Write tests!"
};

var response = await client.PostAsJsonAsync("/EchoTodo/42", todo);
response.EnsureSuccessStatusCode();

var echoedTodo = await response.Content.ReadFromJsonAsync<Todo>();

Assert.NotNull(echoedTodo);
Assert.Equal(todo.Name, echoedTodo?.Name);
Assert.Equal(42, echoedTodo?.Id);
}

private record Todo
{
public int Id { get; set; }
Expand Down
Loading