Skip to content

[Route Groups] Support AddFilter, WithOpenApi and other additive conventions #42195

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

Merged
merged 23 commits into from
Jun 21, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
79803dd
Add EndpointDataSource.GetGroupedEndpoints
halter73 Jun 10, 2022
d3f0bfd
TBuilderify AddFilter and WithOpenApi
halter73 Jun 15, 2022
e1c8b91
Fix missed ServiceProvider -> ApplicationServices
halter73 Jun 15, 2022
97a5ada
Fix some tests and doc comments
halter73 Jun 16, 2022
888f3c9
Fix CompositeEndpointDataSource.HandleChange ordering
halter73 Jun 16, 2022
59c9daf
Address PR review feedback
halter73 Jun 17, 2022
b986444
Address more PR feedback
halter73 Jun 17, 2022
1c5feef
Move MapMethods logic into RouteEndpointDataSource
halter73 Jun 17, 2022
dde086f
Merge remote-tracking branch 'origin/main' into halter73/41427
halter73 Jun 17, 2022
539097d
Fix internal doc comment cref
halter73 Jun 17, 2022
3da2136
Move MapFallback logic to RouteEndpointDataSource too
halter73 Jun 17, 2022
4e88138
GetGroupedEndpoints -> GetEndpointGroup
halter73 Jun 17, 2022
8afc04e
Fix build
halter73 Jun 17, 2022
30c3cf7
I thought I already fixed this lol
halter73 Jun 17, 2022
77a6a2c
cleanup
halter73 Jun 18, 2022
1bb70c8
polish
halter73 Jun 18, 2022
a0ff534
Clean up doc comments
halter73 Jun 18, 2022
b0c8b1b
Address straightforward PR feedback
halter73 Jun 19, 2022
1aa70bb
Add filters to RouteEndpointBuilder
halter73 Jun 20, 2022
e9f9ee5
Merge remote-tracking branch 'origin/main' into halter73/41427
halter73 Jun 20, 2022
9760dcb
Remove redundant RequestDelegate returnType check
halter73 Jun 20, 2022
f094c48
Fix merge conflict with RDF AOT changes
halter73 Jun 20, 2022
8e4ece6
Add missed MapHealthChecks suppression
halter73 Jun 20, 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
Expand Up @@ -1572,12 +1572,13 @@ public async Task BranchingPipelineHasOwnRoutes()
app.Start();

var ds = app.Services.GetRequiredService<EndpointDataSource>();
Assert.Equal(5, ds.Endpoints.Count);
Assert.Equal("One", ds.Endpoints[0].DisplayName);
Assert.Equal("Two", ds.Endpoints[1].DisplayName);
Assert.Equal("Three", ds.Endpoints[2].DisplayName);
Assert.Equal("Four", ds.Endpoints[3].DisplayName);
Assert.Equal("Five", ds.Endpoints[4].DisplayName);
var displayNames = ds.Endpoints.Select(e => e.DisplayName).ToArray();
Assert.Equal(5, displayNames.Length);
Assert.Contains("One", displayNames);
Assert.Contains("Two", displayNames);
Assert.Contains("Three", displayNames);
Assert.Contains("Four", displayNames);
Assert.Contains("Five", displayNames);

var client = app.GetTestClient();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,17 @@ public abstract class EndpointBuilder
/// <summary>
/// Gets the <see cref="IServiceProvider"/> associated with the endpoint.
/// </summary>
public IServiceProvider? ServiceProvider { get; set; }
public IServiceProvider ApplicationServices { get; set; } = EmptyServiceProvicer.Instance;

/// <summary>
/// Creates an instance of <see cref="Endpoint"/> from the <see cref="EndpointBuilder"/>.
/// </summary>
/// <returns>The created <see cref="Endpoint"/>.</returns>
public abstract Endpoint Build();

private sealed class EmptyServiceProvicer : IServiceProvider
{
public static EmptyServiceProvicer Instance { get; } = new EmptyServiceProvicer();
public object? GetService(Type serviceType) => null;
}
}
8 changes: 4 additions & 4 deletions src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#nullable enable
*REMOVED*abstract Microsoft.AspNetCore.Http.HttpResponse.ContentType.get -> string!
*REMOVED*Microsoft.AspNetCore.Http.EndpointMetadataCollection.Enumerator.Current.get -> object?
Microsoft.AspNetCore.Builder.EndpointBuilder.ServiceProvider.get -> System.IServiceProvider?
Microsoft.AspNetCore.Builder.EndpointBuilder.ServiceProvider.set -> void
Microsoft.AspNetCore.Builder.EndpointBuilder.ApplicationServices.get -> System.IServiceProvider!
Microsoft.AspNetCore.Builder.EndpointBuilder.ApplicationServices.set -> void
Microsoft.AspNetCore.Http.AsParametersAttribute
Microsoft.AspNetCore.Http.AsParametersAttribute.AsParametersAttribute() -> void
Microsoft.AspNetCore.Http.DefaultRouteHandlerInvocationContext
Expand All @@ -18,9 +18,9 @@ Microsoft.AspNetCore.Http.Metadata.IRequestSizeLimitMetadata
Microsoft.AspNetCore.Http.Metadata.IRequestSizeLimitMetadata.MaxRequestBodySize.get -> long?
Microsoft.AspNetCore.Http.RouteHandlerContext
Microsoft.AspNetCore.Http.RouteHandlerContext.ApplicationServices.get -> System.IServiceProvider!
Microsoft.AspNetCore.Http.RouteHandlerContext.EndpointMetadata.get -> Microsoft.AspNetCore.Http.EndpointMetadataCollection!
Microsoft.AspNetCore.Http.RouteHandlerContext.EndpointMetadata.get -> System.Collections.Generic.IList<object!>!
Microsoft.AspNetCore.Http.RouteHandlerContext.MethodInfo.get -> System.Reflection.MethodInfo!
Microsoft.AspNetCore.Http.RouteHandlerContext.RouteHandlerContext(System.Reflection.MethodInfo! methodInfo, Microsoft.AspNetCore.Http.EndpointMetadataCollection! endpointMetadata, System.IServiceProvider! applicationServices) -> void
Microsoft.AspNetCore.Http.RouteHandlerContext.RouteHandlerContext(System.Reflection.MethodInfo! methodInfo, System.Collections.Generic.IList<object!>! endpointMetadata, System.IServiceProvider! applicationServices) -> void
Microsoft.AspNetCore.Http.RouteHandlerFilterDelegate
Microsoft.AspNetCore.Http.RouteHandlerInvocationContext
Microsoft.AspNetCore.Http.RouteHandlerInvocationContext.RouteHandlerInvocationContext() -> void
Expand Down
3 changes: 2 additions & 1 deletion src/Http/Http.Abstractions/src/RequestDelegateResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ public RequestDelegateResult(RequestDelegate requestDelegate, IReadOnlyList<obje
public RequestDelegate RequestDelegate { get; }

/// <summary>
/// Gets endpoint metadata inferred from creating the <see cref="RequestDelegate" />
/// Gets endpoint metadata inferred from creating the <see cref="RequestDelegate" />. If a non-null
/// RequestDelegateFactoryOptions.EndpointMetadata list was passed in, this will be the same instance.
/// </summary>
public IReadOnlyList<object> EndpointMetadata { get; }
}
7 changes: 4 additions & 3 deletions src/Http/Http.Abstractions/src/RouteHandlerContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Reflection;
using Microsoft.AspNetCore.Builder;

namespace Microsoft.AspNetCore.Http;

Expand All @@ -15,9 +16,9 @@ public sealed class RouteHandlerContext
/// Creates a new instance of the <see cref="RouteHandlerContext"/>.
/// </summary>
/// <param name="methodInfo">The <see cref="MethodInfo"/> associated with the route handler of the current request.</param>
/// <param name="endpointMetadata">The <see cref="EndpointMetadataCollection"/> associated with the endpoint the filter is targeting.</param>
/// <param name="endpointMetadata">The <see cref="EndpointBuilder.Metadata"/> associated with the endpoint the filter is targeting.</param>
/// <param name="applicationServices">The <see cref="IServiceProvider"/> instance used to access the application services.</param>
public RouteHandlerContext(MethodInfo methodInfo, EndpointMetadataCollection endpointMetadata, IServiceProvider applicationServices)
public RouteHandlerContext(MethodInfo methodInfo, IList<object> endpointMetadata, IServiceProvider applicationServices)
{
ArgumentNullException.ThrowIfNull(methodInfo);
ArgumentNullException.ThrowIfNull(endpointMetadata);
Expand All @@ -36,7 +37,7 @@ public RouteHandlerContext(MethodInfo methodInfo, EndpointMetadataCollection end
/// <summary>
/// The <see cref="EndpointMetadataCollection"/> associated with the current endpoint.
/// </summary>
public EndpointMetadataCollection EndpointMetadata { get; }
public IList<object> EndpointMetadata { get; }

/// <summary>
/// Gets the <see cref="IServiceProvider"/> instance used to access application services.
Expand Down
4 changes: 2 additions & 2 deletions src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ Microsoft.AspNetCore.Http.Metadata.IEndpointMetadataProvider
Microsoft.AspNetCore.Http.Metadata.IEndpointMetadataProvider.PopulateMetadata(Microsoft.AspNetCore.Http.Metadata.EndpointMetadataContext! context) -> void
Microsoft.AspNetCore.Http.Metadata.IEndpointParameterMetadataProvider
Microsoft.AspNetCore.Http.Metadata.IEndpointParameterMetadataProvider.PopulateMetadata(Microsoft.AspNetCore.Http.Metadata.EndpointParameterMetadataContext! parameterContext) -> void
Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.InitialEndpointMetadata.get -> System.Collections.Generic.IEnumerable<object!>?
Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.InitialEndpointMetadata.init -> void
Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.EndpointMetadata.get -> System.Collections.Generic.IList<object!>?
Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.EndpointMetadata.init -> void
Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.RouteHandlerFilterFactories.get -> System.Collections.Generic.IReadOnlyList<System.Func<Microsoft.AspNetCore.Http.RouteHandlerContext!, Microsoft.AspNetCore.Http.RouteHandlerFilterDelegate!, Microsoft.AspNetCore.Http.RouteHandlerFilterDelegate!>!>?
Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.RouteHandlerFilterFactories.init -> void
Microsoft.Extensions.DependencyInjection.RouteHandlerJsonServiceExtensions
Expand Down
57 changes: 26 additions & 31 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public static RequestDelegateResult Create(Delegate handler, RequestDelegateFact

var targetableRequestDelegate = CreateTargetableRequestDelegate(handler.Method, targetExpression, factoryContext, targetFactory);

return new RequestDelegateResult(httpContext => targetableRequestDelegate(handler.Target, httpContext), factoryContext.Metadata);
return new RequestDelegateResult(httpContext => targetableRequestDelegate(handler.Target, httpContext), AsReadOnlyList(factoryContext.Metadata));
}

/// <summary>
Expand Down Expand Up @@ -165,7 +165,7 @@ public static RequestDelegateResult Create(MethodInfo methodInfo, Func<HttpConte
{
var untargetableRequestDelegate = CreateTargetableRequestDelegate(methodInfo, targetExpression: null, factoryContext);

return new RequestDelegateResult(httpContext => untargetableRequestDelegate(null, httpContext), factoryContext.Metadata);
return new RequestDelegateResult(httpContext => untargetableRequestDelegate(null, httpContext), AsReadOnlyList(factoryContext.Metadata));
}

targetFactory = context => Activator.CreateInstance(methodInfo.DeclaringType)!;
Expand All @@ -174,27 +174,31 @@ public static RequestDelegateResult Create(MethodInfo methodInfo, Func<HttpConte
var targetExpression = Expression.Convert(TargetExpr, methodInfo.DeclaringType);
var targetableRequestDelegate = CreateTargetableRequestDelegate(methodInfo, targetExpression, factoryContext, context => targetFactory(context));

return new RequestDelegateResult(httpContext => targetableRequestDelegate(targetFactory(httpContext), httpContext), factoryContext.Metadata);
return new RequestDelegateResult(httpContext => targetableRequestDelegate(targetFactory(httpContext), httpContext), AsReadOnlyList(factoryContext.Metadata));
}

private static FactoryContext CreateFactoryContext(RequestDelegateFactoryOptions? options)
{
var context = new FactoryContext
return new FactoryContext
{
ServiceProvider = options?.ServiceProvider,
ServiceProviderIsService = options?.ServiceProvider?.GetService<IServiceProviderIsService>(),
RouteParameters = options?.RouteParameterNames?.ToList(),
ThrowOnBadRequest = options?.ThrowOnBadRequest ?? false,
DisableInferredFromBody = options?.DisableInferBodyFromParameters ?? false,
Filters = options?.RouteHandlerFilterFactories?.ToList()
Filters = options?.RouteHandlerFilterFactories?.ToList(),
Metadata = options?.EndpointMetadata ?? new List<object>(),
};
}

if (options?.InitialEndpointMetadata is not null)
private static IReadOnlyList<object> AsReadOnlyList(IList<object> metadata)
{
if (metadata is IReadOnlyList<object> readOnlyList)
{
context.Metadata.AddRange(options.InitialEndpointMetadata);
return readOnlyList;
}

return context;
return new List<object>(metadata);
}

private static Func<object?, HttpContext, Task> CreateTargetableRequestDelegate(MethodInfo methodInfo, Expression? targetExpression, FactoryContext factoryContext, Expression<Func<HttpContext, object?>>? targetFactory = null)
Expand All @@ -215,9 +219,6 @@ private static FactoryContext CreateFactoryContext(RequestDelegateFactoryOptions
// return default;
// }

// Add MethodInfo as first metadata item
factoryContext.Metadata.Insert(0, methodInfo);

// CreateArguments will add metadata inferred from parameter details
var arguments = CreateArguments(methodInfo.GetParameters(), factoryContext);
var returnType = methodInfo.ReturnType;
Expand All @@ -229,9 +230,6 @@ private static FactoryContext CreateFactoryContext(RequestDelegateFactoryOptions
factoryContext.ServiceProvider,
CollectionsMarshal.AsSpan(factoryContext.Parameters));

// Add method attributes as metadata *after* any inferred metadata so that the attributes hava a higher specificity
AddMethodAttributesAsMetadata(methodInfo, factoryContext.Metadata);

// If there are filters registered on the route handler, then we update the method call and
// return type associated with the request to allow for the filter invocation pipeline.
if (factoryContext.Filters is { Count: > 0 })
Expand Down Expand Up @@ -305,7 +303,7 @@ targetExpression is null
FilterContextExpr).Compile();
var routeHandlerContext = new RouteHandlerContext(
methodInfo,
new EndpointMetadataCollection(factoryContext.Metadata),
factoryContext.Metadata,
factoryContext.ServiceProvider ?? EmptyServiceProvider.Instance);

for (var i = factoryContext.Filters.Count - 1; i >= 0; i--)
Expand Down Expand Up @@ -433,7 +431,7 @@ private static Expression CreateRouteHandlerInvocationContextBase(FactoryContext
return fallbackConstruction;
}

private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List<object> metadata, IServiceProvider? services, ReadOnlySpan<ParameterInfo> parameters)
private static void AddTypeProvidedMetadata(MethodInfo methodInfo, IList<object> metadata, IServiceProvider? services, ReadOnlySpan<ParameterInfo> parameters)
{
object?[]? invokeArgs = null;

Expand Down Expand Up @@ -488,17 +486,6 @@ private static void PopulateMetadataForEndpoint<T>(EndpointMetadataContext conte
T.PopulateMetadata(context);
}

private static void AddMethodAttributesAsMetadata(MethodInfo methodInfo, List<object> metadata)
{
var attributes = methodInfo.GetCustomAttributes();

// This can be null if the delegate is a dynamic method or compiled from an expression tree
if (attributes is not null)
{
metadata.AddRange(attributes);
}
}

private static Expression[] CreateArguments(ParameterInfo[]? parameters, FactoryContext factoryContext)
{
if (parameters is null || parameters.Length == 0)
Expand Down Expand Up @@ -1659,6 +1646,14 @@ private static Expression BindParameterFromBindAsync(ParameterInfo parameter, Fa
return Expression.Convert(boundValueExpr, parameter.ParameterType);
}

private static void InsertInferredAcceptsMetadata(FactoryContext factoryContext, Type type, string[] contentTypes)
{
// Unlike most metadata that will probably to be added by filters or metadata providers, we insert the automatically-inferred AcceptsMetadata
// to the beginning of the metadata to give it the lowest precedence. It really doesn't makes sense for this metadata to be overridden but
// we're preserving the old behavior here out of an abundance of caution.
factoryContext.Metadata.Insert(0, new AcceptsMetadata(type, factoryContext.AllowEmptyRequestBody, contentTypes));
}

private static Expression BindParameterFromFormFiles(
ParameterInfo parameter,
FactoryContext factoryContext)
Expand All @@ -1673,7 +1668,7 @@ private static Expression BindParameterFromFormFiles(
// Do not duplicate the metadata if there are multiple form parameters
if (!factoryContext.ReadForm)
{
factoryContext.Metadata.Add(new AcceptsMetadata(parameter.ParameterType, factoryContext.AllowEmptyRequestBody, FormFileContentType));
InsertInferredAcceptsMetadata(factoryContext, parameter.ParameterType, FormFileContentType);
}

factoryContext.ReadForm = true;
Expand All @@ -1697,7 +1692,7 @@ private static Expression BindParameterFromFormFile(
// Do not duplicate the metadata if there are multiple form parameters
if (!factoryContext.ReadForm)
{
factoryContext.Metadata.Add(new AcceptsMetadata(parameter.ParameterType, factoryContext.AllowEmptyRequestBody, FormFileContentType));
InsertInferredAcceptsMetadata(factoryContext, parameter.ParameterType, FormFileContentType);
}

factoryContext.ReadForm = true;
Expand Down Expand Up @@ -1727,7 +1722,7 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al

factoryContext.JsonRequestBodyParameter = parameter;
factoryContext.AllowEmptyRequestBody = allowEmpty || isOptional;
factoryContext.Metadata.Add(new AcceptsMetadata(parameter.ParameterType, factoryContext.AllowEmptyRequestBody, DefaultAcceptsContentType));
InsertInferredAcceptsMetadata(factoryContext, parameter.ParameterType, DefaultAcceptsContentType);

if (!factoryContext.AllowEmptyRequestBody)
{
Expand Down Expand Up @@ -2077,7 +2072,7 @@ private sealed class FactoryContext
public bool HasMultipleBodyParameters { get; set; }
public bool HasInferredBody { get; set; }

public List<object> Metadata { get; internal set; } = new();
public IList<object> Metadata { get; init; } = default!;

public NullabilityInfoContext NullabilityContext { get; } = new();

Expand Down
14 changes: 9 additions & 5 deletions src/Http/Http.Extensions/src/RequestDelegateFactoryOptions.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http.Metadata;
using Microsoft.Extensions.Logging;

Expand Down Expand Up @@ -38,13 +39,16 @@ public sealed class RequestDelegateFactoryOptions
public IReadOnlyList<Func<RouteHandlerContext, RouteHandlerFilterDelegate, RouteHandlerFilterDelegate>>? RouteHandlerFilterFactories { get; init; }

/// <summary>
/// The initial endpoint metadata to add as part of the creation of the <see cref="RequestDelegateResult.RequestDelegate"/>.
/// The mutable initial endpoint metadata to add as part of the creation of the <see cref="RequestDelegateResult.RequestDelegate"/>. In most cases,
/// this should come from <see cref="EndpointBuilder.Metadata"/>.
/// </summary>
/// <remarks>
/// This metadata will be included in <see cref="RequestDelegateResult.EndpointMetadata" /> <b>before</b> any metadata inferred during creation of the
/// This metadata will be included in <see cref="RequestDelegateResult.EndpointMetadata" /> <b>before</b> most metadata inferred during creation of the
/// <see cref="RequestDelegateResult.RequestDelegate"/> and <b>before</b> any metadata provided by types in the delegate signature that implement
/// <see cref="IEndpointMetadataProvider" /> or <see cref="IEndpointParameterMetadataProvider" />, i.e. this metadata will be less specific than any
/// inferred by the call to <see cref="RequestDelegateFactory.Create(Delegate, RequestDelegateFactoryOptions?)"/>.
/// <see cref="IEndpointMetadataProvider" /> or <see cref="IEndpointParameterMetadataProvider" />. The exception to this general rule is the
/// <see cref="IAcceptsMetadata"/> that <see cref="RequestDelegateFactory.Create(Delegate, RequestDelegateFactoryOptions?)"/> infers automatically
/// without any custom metadata providers which instead is inserted at the start to give it lower precedence. Custom metadata providers can choose to do
/// insert their metadata at the start to give lower precedence, but this is unusual.
/// </remarks>
public IEnumerable<object>? InitialEndpointMetadata { get; init; }
public IList<object>? EndpointMetadata { get; init; }
}
Loading