From 39dce2222b45572ca60382fad2e84dd6058c0785 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 12 Aug 2022 14:21:23 -0700 Subject: [PATCH 1/4] Expose entire EndpointBuilder to filter factories --- .../src/EndpointFilterFactoryContext.cs | 33 +--- .../src/PublicAPI.Unshipped.txt | 7 +- .../src/PublicAPI.Unshipped.txt | 6 +- .../src/RequestDelegateFactory.cs | 124 ++++++++------ .../src/RequestDelegateFactoryOptions.cs | 23 +-- .../test/RequestDelegateFactoryTests.cs | 159 ++++++++++++------ src/Http/Routing/src/PublicAPI.Unshipped.txt | 2 + src/Http/Routing/src/RouteEndpointBuilder.cs | 4 +- .../Routing/src/RouteEndpointDataSource.cs | 22 ++- ...egateEndpointRouteBuilderExtensionsTest.cs | 4 +- ...ndlerEndpointRouteBuilderExtensionsTest.cs | 41 ++--- .../UnitTests/RouteEndpointBuilderTest.cs | 14 ++ src/Http/samples/MinimalSample/Program.cs | 2 +- .../src/Routing/ActionEndpointFactory.cs | 8 +- 14 files changed, 255 insertions(+), 194 deletions(-) diff --git a/src/Http/Http.Abstractions/src/EndpointFilterFactoryContext.cs b/src/Http/Http.Abstractions/src/EndpointFilterFactoryContext.cs index 86ab54683508..6b7c2118bc41 100644 --- a/src/Http/Http.Abstractions/src/EndpointFilterFactoryContext.cs +++ b/src/Http/Http.Abstractions/src/EndpointFilterFactoryContext.cs @@ -13,34 +13,15 @@ namespace Microsoft.AspNetCore.Http; public sealed class EndpointFilterFactoryContext { /// - /// Creates a new instance of the . + /// The associated with the current route handler, or MVC action. /// - /// The associated with the route handler of the current request. - /// The associated with the endpoint the filter is targeting. - /// The instance used to access the application services. - public EndpointFilterFactoryContext(MethodInfo methodInfo, IList endpointMetadata, IServiceProvider applicationServices) - { - ArgumentNullException.ThrowIfNull(methodInfo); - ArgumentNullException.ThrowIfNull(endpointMetadata); - ArgumentNullException.ThrowIfNull(applicationServices); - - MethodInfo = methodInfo; - EndpointMetadata = endpointMetadata; - ApplicationServices = applicationServices; - } - - /// - /// The associated with the current route handler. - /// - public MethodInfo MethodInfo { get; } - - /// - /// The associated with the current endpoint. - /// - public IList EndpointMetadata { get; } + /// + /// In the future this could support more endpoint types. + /// + public required MethodInfo MethodInfo { get; init; } /// - /// Gets the instance used to access application services. + /// The associated with the current endpoint being filtered. /// - public IServiceProvider ApplicationServices { get; } + public required EndpointBuilder EndpointBuilder { get; init; } } diff --git a/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt b/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt index cadc3cab2da6..ac59bca7b5cd 100644 --- a/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt +++ b/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt @@ -14,10 +14,11 @@ Microsoft.AspNetCore.Http.DefaultEndpointFilterInvocationContext Microsoft.AspNetCore.Http.DefaultEndpointFilterInvocationContext.DefaultEndpointFilterInvocationContext(Microsoft.AspNetCore.Http.HttpContext! httpContext, params object![]! arguments) -> void Microsoft.AspNetCore.Http.EndpointFilterDelegate Microsoft.AspNetCore.Http.EndpointFilterFactoryContext -Microsoft.AspNetCore.Http.EndpointFilterFactoryContext.ApplicationServices.get -> System.IServiceProvider! -Microsoft.AspNetCore.Http.EndpointFilterFactoryContext.EndpointFilterFactoryContext(System.Reflection.MethodInfo! methodInfo, System.Collections.Generic.IList! endpointMetadata, System.IServiceProvider! applicationServices) -> void -Microsoft.AspNetCore.Http.EndpointFilterFactoryContext.EndpointMetadata.get -> System.Collections.Generic.IList! +Microsoft.AspNetCore.Http.EndpointFilterFactoryContext.EndpointBuilder.get -> Microsoft.AspNetCore.Builder.EndpointBuilder! +Microsoft.AspNetCore.Http.EndpointFilterFactoryContext.EndpointBuilder.init -> void +Microsoft.AspNetCore.Http.EndpointFilterFactoryContext.EndpointFilterFactoryContext() -> void Microsoft.AspNetCore.Http.EndpointFilterFactoryContext.MethodInfo.get -> System.Reflection.MethodInfo! +Microsoft.AspNetCore.Http.EndpointFilterFactoryContext.MethodInfo.init -> void Microsoft.AspNetCore.Http.EndpointFilterInvocationContext Microsoft.AspNetCore.Http.EndpointFilterInvocationContext.EndpointFilterInvocationContext() -> void Microsoft.AspNetCore.Http.EndpointMetadataCollection.Enumerator.Current.get -> object! diff --git a/src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt b/src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt index 4b8eccfa6afb..82365c8ec649 100644 --- a/src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt +++ b/src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt @@ -38,6 +38,8 @@ Microsoft.AspNetCore.Http.ProblemDetailsOptions.ProblemDetailsOptions() -> void *REMOVED*Microsoft.AspNetCore.Mvc.ProblemDetails.Title.set -> void *REMOVED*Microsoft.AspNetCore.Mvc.ProblemDetails.Type.get -> string? *REMOVED*Microsoft.AspNetCore.Mvc.ProblemDetails.Type.set -> void +Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.EndpointBuilder.get -> Microsoft.AspNetCore.Builder.EndpointBuilder? +Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.EndpointBuilder.init -> void Microsoft.AspNetCore.Mvc.ProblemDetails (forwarded, contained in Microsoft.AspNetCore.Http.Abstractions) Microsoft.AspNetCore.Mvc.ProblemDetails.Detail.get -> string? (forwarded, contained in Microsoft.AspNetCore.Http.Abstractions) Microsoft.AspNetCore.Mvc.ProblemDetails.Detail.set -> void (forwarded, contained in Microsoft.AspNetCore.Http.Abstractions) @@ -52,10 +54,6 @@ Microsoft.AspNetCore.Mvc.ProblemDetails.Title.set -> void (forwarded, contained Microsoft.AspNetCore.Mvc.ProblemDetails.Type.get -> string? (forwarded, contained in Microsoft.AspNetCore.Http.Abstractions) Microsoft.AspNetCore.Mvc.ProblemDetails.Type.set -> void (forwarded, contained in Microsoft.AspNetCore.Http.Abstractions) Microsoft.Extensions.DependencyInjection.ProblemDetailsServiceCollectionExtensions -Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.EndpointFilterFactories.get -> System.Collections.Generic.IReadOnlyList!>? -Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.EndpointFilterFactories.init -> void -Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.EndpointMetadata.get -> System.Collections.Generic.IList? -Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.EndpointMetadata.init -> void Microsoft.Extensions.DependencyInjection.HttpJsonServiceExtensions static Microsoft.AspNetCore.Http.HttpRequestJsonExtensions.ReadFromJsonAsync(this Microsoft.AspNetCore.Http.HttpRequest! request, System.Type! type, System.Text.Json.Serialization.JsonSerializerContext! context, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.ValueTask static Microsoft.AspNetCore.Http.HttpRequestJsonExtensions.ReadFromJsonAsync(this Microsoft.AspNetCore.Http.HttpRequest! request, System.Text.Json.Serialization.Metadata.JsonTypeInfo! jsonTypeInfo, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.ValueTask diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 810d902964c1..52db3f41baf3 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -13,6 +13,7 @@ using System.Security.Claims; using System.Text; using System.Text.Json; +using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Routing; @@ -138,17 +139,26 @@ public static RequestDelegateResult Create(Delegate handler, RequestDelegateFact var factoryContext = CreateFactoryContext(options, handler); Expression> targetFactory = (httpContext) => handler.Target; - var targetableRequestDelegate = CreateTargetableRequestDelegate(handler.Method, targetExpression, factoryContext, targetFactory); - if (targetableRequestDelegate is null) + RequestDelegate finalRequestDelegate = targetableRequestDelegate switch { // handler is a RequestDelegate that has not been modified by a filter. Short-circuit and return the original RequestDelegate back. // It's possible a filter factory has still modified the endpoint metadata though. - return new RequestDelegateResult((RequestDelegate)handler, AsReadOnlyList(factoryContext.Metadata)); + null => (RequestDelegate)handler, + _ => httpContext => targetableRequestDelegate(handler.Target, httpContext), + }; + + if (targetableRequestDelegate is null) + { + finalRequestDelegate = (RequestDelegate)handler; + } + else + { + finalRequestDelegate = httpContext => targetableRequestDelegate(handler.Target, httpContext); } - return new RequestDelegateResult(httpContext => targetableRequestDelegate(handler.Target, httpContext), AsReadOnlyList(factoryContext.Metadata)); + return CreateRequestDelegateResult(finalRequestDelegate, factoryContext.EndpointBuilder); } /// @@ -173,46 +183,61 @@ public static RequestDelegateResult Create(MethodInfo methodInfo, Func untargetableRequestDelegate(null, httpContext), AsReadOnlyList(factoryContext.Metadata)); - } + // CreateTargetableRequestDelegate can only return null given a RequestDelegate passed into the other RDF.Create() overload. + Debug.Assert(untargetableRequestDelegate is not null); - targetFactory = context => Activator.CreateInstance(methodInfo.DeclaringType)!; + finalRequestDelegate = httpContext => untargetableRequestDelegate(null, httpContext); } + else + { + targetFactory ??= context => Activator.CreateInstance(methodInfo.DeclaringType)!; - var targetExpression = Expression.Convert(TargetExpr, methodInfo.DeclaringType); - var targetableRequestDelegate = CreateTargetableRequestDelegate(methodInfo, targetExpression, factoryContext, context => targetFactory(context)); + var targetExpression = Expression.Convert(TargetExpr, methodInfo.DeclaringType); + var targetableRequestDelegate = CreateTargetableRequestDelegate(methodInfo, targetExpression, factoryContext, context => targetFactory(context)); - // CreateTargetableRequestDelegate can only return null given a RequestDelegate passed into the other RDF.Create() overload. - Debug.Assert(targetableRequestDelegate is not null); + // CreateTargetableRequestDelegate can only return null given a RequestDelegate passed into the other RDF.Create() overload. + Debug.Assert(targetableRequestDelegate is not null); + + finalRequestDelegate = httpContext => targetableRequestDelegate(targetFactory(httpContext), httpContext); + } - return new RequestDelegateResult(httpContext => targetableRequestDelegate(targetFactory(httpContext), httpContext), AsReadOnlyList(factoryContext.Metadata)); + return CreateRequestDelegateResult(finalRequestDelegate, factoryContext.EndpointBuilder); } private static FactoryContext CreateFactoryContext(RequestDelegateFactoryOptions? options, Delegate? handler = null) { + if (options?.EndpointBuilder?.RequestDelegate is not null) + { + throw new ArgumentException($"{nameof(RequestDelegateFactoryOptions)}.{nameof(RequestDelegateFactoryOptions.EndpointBuilder)} must be null.", nameof(options)); + } + + var endointBuilder = options?.EndpointBuilder ?? new RDFEndpointBuilder(); + var serviceProvider = options?.ServiceProvider ?? endointBuilder.ApplicationServices; + return new FactoryContext { Handler = handler, - ServiceProvider = options?.ServiceProvider, - ServiceProviderIsService = options?.ServiceProvider?.GetService(), + ServiceProvider = serviceProvider, + ServiceProviderIsService = serviceProvider.GetService(), RouteParameters = options?.RouteParameterNames?.ToList(), ThrowOnBadRequest = options?.ThrowOnBadRequest ?? false, DisableInferredFromBody = options?.DisableInferBodyFromParameters ?? false, - FilterFactories = options?.EndpointFilterFactories?.ToList(), - Metadata = options?.EndpointMetadata ?? new List(), + EndpointBuilder = endointBuilder, }; } + private static RequestDelegateResult CreateRequestDelegateResult(RequestDelegate finalRequestDelegate, EndpointBuilder endpointBuilder) + { + endpointBuilder.RequestDelegate = finalRequestDelegate; + return new RequestDelegateResult(finalRequestDelegate, AsReadOnlyList(endpointBuilder.Metadata)); + } + private static IReadOnlyList AsReadOnlyList(IList metadata) { if (metadata is IReadOnlyList readOnlyList) @@ -248,7 +273,7 @@ private static IReadOnlyList AsReadOnlyList(IList metadata) // Add metadata provided by the delegate return type and parameter types next, this will be more specific than inferred metadata from above AddTypeProvidedMetadata(methodInfo, - factoryContext.Metadata, + factoryContext.EndpointBuilder.Metadata, factoryContext.ServiceProvider, CollectionsMarshal.AsSpan(factoryContext.Parameters)); @@ -256,7 +281,7 @@ private static IReadOnlyList AsReadOnlyList(IList 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.FilterFactories is { Count: > 0 }) + if (factoryContext.EndpointBuilder.FilterFactories.Count > 0 ) { filterPipeline = CreateFilterPipeline(methodInfo, targetExpression, factoryContext, targetFactory); @@ -279,11 +304,7 @@ private static IReadOnlyList AsReadOnlyList(IList metadata) // return null for plain RequestDelegates that have not been modified by filters so we can just pass back the original RequestDelegate. if (filterPipeline is null && factoryContext.Handler is RequestDelegate) { - // Make sure we're still not handling a return value. - if (!returnType.IsGenericType || returnType.GetGenericTypeDefinition() != typeof(Task<>)) - { - return null; - } + return null; } var responseWritingMethodCall = factoryContext.ParamCheckExpressions.Count > 0 ? @@ -300,7 +321,7 @@ private static IReadOnlyList AsReadOnlyList(IList metadata) private static EndpointFilterDelegate? CreateFilterPipeline(MethodInfo methodInfo, Expression? targetExpression, FactoryContext factoryContext, Expression>? targetFactory) { - Debug.Assert(factoryContext.FilterFactories is not null); + Debug.Assert(factoryContext.EndpointBuilder.FilterFactories.Count > 0); // httpContext.Response.StatusCode >= 400 // ? Task.CompletedTask // : { @@ -339,16 +360,17 @@ targetExpression is null CompletedValueTaskExpr, handlerInvocation), FilterContextExpr).Compile(); - var routeHandlerContext = new EndpointFilterFactoryContext( - methodInfo, - factoryContext.Metadata, - factoryContext.ServiceProvider ?? EmptyServiceProvider.Instance); + var routeHandlerContext = new EndpointFilterFactoryContext + { + MethodInfo = methodInfo, + EndpointBuilder = factoryContext.EndpointBuilder, + }; var initialFilteredInvocation = filteredInvocation; - for (var i = factoryContext.FilterFactories.Count - 1; i >= 0; i--) + for (var i = factoryContext.EndpointBuilder.FilterFactories.Count - 1; i >= 0; i--) { - var currentFilterFactory = factoryContext.FilterFactories[i]; + var currentFilterFactory = factoryContext.EndpointBuilder.FilterFactories[i]; filteredInvocation = currentFilterFactory(routeHandlerContext, filteredInvocation); } @@ -483,7 +505,7 @@ private static Expression CreateEndpointFilterInvocationContextBase(FactoryConte return fallbackConstruction; } - private static void AddTypeProvidedMetadata(MethodInfo methodInfo, IList metadata, IServiceProvider? services, ReadOnlySpan parameters) + private static void AddTypeProvidedMetadata(MethodInfo methodInfo, IList metadata, IServiceProvider services, ReadOnlySpan parameters) { object?[]? invokeArgs = null; @@ -493,7 +515,7 @@ private static void AddTypeProvidedMetadata(MethodInfo methodInfo, IList if (typeof(IEndpointParameterMetadataProvider).IsAssignableFrom(parameter.ParameterType)) { // Parameter type implements IEndpointParameterMetadataProvider - var parameterContext = new EndpointParameterMetadataContext(parameter, metadata, services ?? EmptyServiceProvider.Instance); + var parameterContext = new EndpointParameterMetadataContext(parameter, metadata, services); invokeArgs ??= new object[1]; invokeArgs[0] = parameterContext; PopulateMetadataForParameterMethod.MakeGenericMethod(parameter.ParameterType).Invoke(null, invokeArgs); @@ -502,7 +524,7 @@ private static void AddTypeProvidedMetadata(MethodInfo methodInfo, IList if (typeof(IEndpointMetadataProvider).IsAssignableFrom(parameter.ParameterType)) { // Parameter type implements IEndpointMetadataProvider - var context = new EndpointMetadataContext(methodInfo, metadata, services ?? EmptyServiceProvider.Instance); + var context = new EndpointMetadataContext(methodInfo, metadata, services); invokeArgs ??= new object[1]; invokeArgs[0] = context; PopulateMetadataForEndpointMethod.MakeGenericMethod(parameter.ParameterType).Invoke(null, invokeArgs); @@ -519,7 +541,7 @@ private static void AddTypeProvidedMetadata(MethodInfo methodInfo, IList if (returnType is not null && typeof(IEndpointMetadataProvider).IsAssignableFrom(returnType)) { // Return type implements IEndpointMetadataProvider - var context = new EndpointMetadataContext(methodInfo, metadata, services ?? EmptyServiceProvider.Instance); + var context = new EndpointMetadataContext(methodInfo, metadata, services); invokeArgs ??= new object[1]; invokeArgs[0] = context; PopulateMetadataForEndpointMethod.MakeGenericMethod(returnType).Invoke(null, invokeArgs); @@ -552,7 +574,7 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory factoryContext.BoxedArgs = new Expression[parameters.Length]; factoryContext.Parameters = new List(parameters); - var hasFilters = factoryContext.FilterFactories is { Count: > 0 }; + var hasFilters = factoryContext.EndpointBuilder.FilterFactories.Count > 0; for (var i = 0; i < parameters.Length; i++) { @@ -842,7 +864,7 @@ private static Expression CreateParamCheckingResponseWritingMethodCall(Type retu // If filters have been registered, we set the `wasParamCheckFailure` property // but do not return from the invocation to allow the filters to run. - if (factoryContext.FilterFactories is { Count: > 0 }) + if (factoryContext.EndpointBuilder.FilterFactories.Count > 0 ) { // if (wasParamCheckFailure) // { @@ -1722,7 +1744,7 @@ private static void InsertInferredAcceptsMetadata(FactoryContext factoryContext, // Insert the automatically-inferred AcceptsMetadata at the beginning of the list 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 out of an abundance of caution. // I suspect most filters and metadata providers will just add their metadata to the end of the list. - factoryContext.Metadata.Insert(0, new AcceptsMetadata(type, factoryContext.AllowEmptyRequestBody, contentTypes)); + factoryContext.EndpointBuilder.Metadata.Insert(0, new AcceptsMetadata(type, factoryContext.AllowEmptyRequestBody, contentTypes)); } private static Expression BindParameterFromFormFiles( @@ -2103,7 +2125,7 @@ private sealed class FactoryContext // Handler could be null if the MethodInfo overload of RDF.Create is used, but that doesn't matter because this is // only referenced to optimize certain cases where a RequestDelegate is the handler and filters don't modify it. public Delegate? Handler { get; init; } - public IServiceProvider? ServiceProvider { get; init; } + public required IServiceProvider ServiceProvider { get; init; } public IServiceProviderIsService? ServiceProviderIsService { get; init; } public List? RouteParameters { get; init; } public bool ThrowOnBadRequest { get; init; } @@ -2122,7 +2144,7 @@ private sealed class FactoryContext public bool HasMultipleBodyParameters { get; set; } public bool HasInferredBody { get; set; } - public IList Metadata { get; init; } = default!; + public required EndpointBuilder EndpointBuilder { get; init; } public NullabilityInfoContext NullabilityContext { get; } = new(); @@ -2134,7 +2156,6 @@ private sealed class FactoryContext public Type[] ArgumentTypes { get; set; } = Array.Empty(); public Expression[] ArgumentExpressions { get; set; } = Array.Empty(); public Expression[] BoxedArgs { get; set; } = Array.Empty(); - public List>? FilterFactories { get; init; } public bool FilterFactoriesHaveRunWithoutModifyingPerRequestBehavior { get; set; } public List Parameters { get; set; } = new(); @@ -2398,10 +2419,11 @@ public Task ExecuteAsync(HttpContext httpContext) } } - private sealed class EmptyServiceProvider : IServiceProvider + private sealed class RDFEndpointBuilder : EndpointBuilder { - public static IServiceProvider Instance { get; } = new EmptyServiceProvider(); - - public object? GetService(Type serviceType) => null; + public override Endpoint Build() + { + throw new NotSupportedException(); + } } } diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactoryOptions.cs b/src/Http/Http.Extensions/src/RequestDelegateFactoryOptions.cs index a3f05d9db8a3..a11cba424330 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactoryOptions.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactoryOptions.cs @@ -34,25 +34,18 @@ public sealed class RequestDelegateFactoryOptions public bool DisableInferBodyFromParameters { get; init; } /// - /// The list of filters that must run in the pipeline for a given route handler. - /// - public IReadOnlyList>? EndpointFilterFactories { get; init; } - - /// - /// The mutable initial endpoint metadata to add as part of the creation of the . In most cases, - /// this should come from . + /// The mutable used to assist in the creation of the . + /// This is primarily used to run and populate inferred . + /// The must be . After the call to , + /// will be the same as . /// /// - /// This metadata will be included in before most metadata inferred during creation of the - /// and before any metadata provided by types in the delegate signature that implement - /// or . The exception to this general rule is the + /// Any metadata already in will be included in before + /// most metadata inferred during creation of the and before any metadata provided by types in + /// the delegate signature that implement or . The exception to this general rule is the /// that 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 /// insert their metadata at the start to give lower precedence, but this is unusual. /// - public IList? EndpointMetadata { get; init; } - - // TODO: Add a RouteEndpointBuilder property and remove the EndpointMetadata property. Then do the same in RouteHandlerContext, EndpointMetadataContext - // and EndpointParameterMetadataContext. This will allow seeing the entire route pattern if the caller chooses to allow it. - // We'll probably want to add the RouteEndpointBuilder constructor without a RequestDelegate back and make it public too. + public EndpointBuilder? EndpointBuilder { get; init; } } diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 3517fe81b4f8..d43608f5f862 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -18,10 +18,12 @@ using System.Text; using System.Text.Json; using System.Text.Json.Serialization; +using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Http.Json; using Microsoft.AspNetCore.Http.Metadata; +using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Internal; @@ -5164,14 +5166,14 @@ string HelloName(string name) // Act var factoryResult = RequestDelegateFactory.Create(HelloName, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { context.Arguments[0] = context.Arguments[0] != null ? $"{((string)context.Arguments[0]!)}Prefix" : "NULL"; return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5196,13 +5198,13 @@ public async Task RequestDelegateFactory_InvokesFilters_OnDelegateWithTarget() // Act var factoryResult = RequestDelegateFactory.Create((string name) => $"Hello, {name}!", new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5238,13 +5240,13 @@ public async Task RequestDelegateFactory_InvokesFilters_OnMethodInfoWithNullTarg // Act var factoryResult = RequestDelegateFactory.Create(methodInfo!, null, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5281,13 +5283,13 @@ public async Task RequestDelegateFactory_InvokesFilters_OnMethodInfoWithProvided }; var factoryResult = RequestDelegateFactory.Create(methodInfo!, targetFactory, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5318,7 +5320,7 @@ string HelloName(string name) // Act var factoryResult = RequestDelegateFactory.Create(HelloName, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() { + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { if (context.HttpContext.Response.StatusCode == 400) @@ -5327,7 +5329,7 @@ string HelloName(string name) } return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5364,7 +5366,7 @@ string HelloName(string name, int age) // Act var factoryResult = RequestDelegateFactory.Create(HelloName, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { @@ -5379,7 +5381,7 @@ string HelloName(string name, int age) } return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5412,7 +5414,7 @@ string HelloName(string name) // Act var factoryResult = RequestDelegateFactory.Create(HelloName, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => { @@ -5428,7 +5430,7 @@ string HelloName(string name) return "Is not an int."; }; }, - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5467,11 +5469,11 @@ string HelloName(IFormFileCollection formFiles) // Act var factoryResult = RequestDelegateFactory.Create(HelloName, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => { - var acceptsMetadata = routeHandlerContext.EndpointMetadata.OfType(); + var acceptsMetadata = routeHandlerContext.EndpointBuilder.Metadata.OfType(); var contentType = acceptsMetadata.SingleOrDefault()?.ContentTypes.SingleOrDefault(); return async (context) => @@ -5483,7 +5485,7 @@ string HelloName(IFormFileCollection formFiles) return await next(context); }; }, - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5518,7 +5520,7 @@ string PrintTodo(Todo todo) // Act var factoryResult = RequestDelegateFactory.Create(PrintTodo, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { @@ -5527,7 +5529,7 @@ string PrintTodo(Todo todo) context.Arguments[0] = originalTodo; return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5559,7 +5561,7 @@ string HelloName(string name) // Act var factoryResult = RequestDelegateFactory.Create(HelloName, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { @@ -5570,7 +5572,7 @@ string HelloName(string name) } return previousResult; } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5602,7 +5604,7 @@ string HelloName(string name) // Act var factoryResult = RequestDelegateFactory.Create(HelloName, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { @@ -5619,7 +5621,7 @@ string HelloName(string name) context.Arguments[0] = newValue; return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5671,13 +5673,13 @@ public async Task CanInvokeFilter_OnTaskOfTReturningHandler(Delegate @delegate) // Act var factoryResult = RequestDelegateFactory.Create(@delegate, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5729,13 +5731,13 @@ public async Task CanInvokeFilter_OnValueTaskOfTReturningHandler(Delegate @deleg // Act var factoryResult = RequestDelegateFactory.Create(@delegate, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5794,13 +5796,13 @@ public async Task CanInvokeFilter_OnVoidReturningHandler(Delegate @delegate) // Act var factoryResult = RequestDelegateFactory.Create(@delegate, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5828,13 +5830,13 @@ async Task HandlerWithTaskAwait(HttpContext c) // Act var factoryResult = RequestDelegateFactory.Create(HandlerWithTaskAwait, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; @@ -5895,13 +5897,13 @@ public async Task CanInvokeFilter_OnHandlerReturningTasksOfStruct(Delegate @dele // Act var factoryResult = RequestDelegateFactory.Create(@delegate, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5931,7 +5933,7 @@ string HelloName(int? one, string? two, int? three, string? four, int? five, boo // Act var factoryResult = RequestDelegateFactory.Create(HelloName, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { @@ -5939,7 +5941,7 @@ string HelloName(int? one, string? two, int? three, string? four, int? five, boo Assert.Equal(11, context.Arguments.Count); return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5962,7 +5964,7 @@ string HelloName() // Act var factoryResult = RequestDelegateFactory.Create(HelloName, new RequestDelegateFactoryOptions() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => async (context) => { @@ -5970,7 +5972,7 @@ string HelloName() Assert.Equal(0, context.Arguments.Count); return await next(context); } - } + }), }); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); @@ -5996,7 +5998,7 @@ public void Create_DoesNotAddAnythingBefore_ThePassedInEndpointMetadata() // Arrange var @delegate = (AddsCustomParameterMetadataBindable param1) => { }; var customMetadata = new CustomEndpointMetadata(); - var options = new RequestDelegateFactoryOptions { EndpointMetadata = new List { customMetadata } }; + var options = new RequestDelegateFactoryOptions { EndpointBuilder = CreateEndpointBuilderFromMetadata(new List { customMetadata }) }; // Act var result = RequestDelegateFactory.Create(@delegate, options); @@ -6097,10 +6099,10 @@ public void Create_CombinesDefaultMetadata_AndMetadataFromReturnTypesImplementin var @delegate = () => new CountsDefaultEndpointMetadataResult(); var options = new RequestDelegateFactoryOptions { - EndpointMetadata = new List + EndpointBuilder = CreateEndpointBuilderFromMetadata(new List { new CustomEndpointMetadata { Source = MetadataSource.Caller } - } + }), }; // Act @@ -6119,10 +6121,10 @@ public void Create_CombinesDefaultMetadata_AndMetadataFromTaskWrappedReturnTypes var @delegate = () => Task.FromResult(new CountsDefaultEndpointMetadataResult()); var options = new RequestDelegateFactoryOptions { - EndpointMetadata = new List + EndpointBuilder = CreateEndpointBuilderFromMetadata(new List { new CustomEndpointMetadata { Source = MetadataSource.Caller } - } + }), }; // Act @@ -6141,10 +6143,10 @@ public void Create_CombinesDefaultMetadata_AndMetadataFromValueTaskWrappedReturn var @delegate = () => ValueTask.FromResult(new CountsDefaultEndpointMetadataResult()); var options = new RequestDelegateFactoryOptions { - EndpointMetadata = new List + EndpointBuilder = CreateEndpointBuilderFromMetadata(new List { new CustomEndpointMetadata { Source = MetadataSource.Caller } - } + }), }; // Act @@ -6163,10 +6165,10 @@ public void Create_CombinesDefaultMetadata_AndMetadataFromParameterTypesImplemen var @delegate = (AddsCustomParameterMetadata param1) => "Hello"; var options = new RequestDelegateFactoryOptions { - EndpointMetadata = new List + EndpointBuilder = CreateEndpointBuilderFromMetadata(new List { new CustomEndpointMetadata { Source = MetadataSource.Caller } - } + }), }; // Act @@ -6184,10 +6186,10 @@ public void Create_CombinesDefaultMetadata_AndMetadataFromParameterTypesImplemen var @delegate = (AddsCustomParameterMetadata param1) => "Hello"; var options = new RequestDelegateFactoryOptions { - EndpointMetadata = new List + EndpointBuilder = CreateEndpointBuilderFromMetadata(new List { new CustomEndpointMetadata { Source = MetadataSource.Caller } - } + }), }; // Act @@ -6205,10 +6207,10 @@ public void Create_CombinesPropertiesAsParameterMetadata_AndTopLevelParameter() var @delegate = ([AsParameters] AddsCustomParameterMetadata param1) => new CountsDefaultEndpointMetadataResult(); var options = new RequestDelegateFactoryOptions { - EndpointMetadata = new List + EndpointBuilder = CreateEndpointBuilderFromMetadata(new List { new CustomEndpointMetadata { Source = MetadataSource.Caller } - } + }), }; // Act @@ -6228,10 +6230,10 @@ public void Create_CombinesAllMetadata_InCorrectOrder() var @delegate = [Attribute1, Attribute2] (AddsCustomParameterMetadata param1) => new CountsDefaultEndpointMetadataResult(); var options = new RequestDelegateFactoryOptions { - EndpointMetadata = new List + EndpointBuilder = CreateEndpointBuilderFromMetadata(new List { new CustomEndpointMetadata { Source = MetadataSource.Caller } - } + }), }; // Act @@ -6356,19 +6358,19 @@ public void Create_ReturnsSameRequestDelegatePassedIn_IfNotModifiedByFilters() RequestDelegateFactoryOptions options = new() { - EndpointFilterFactories = new List>() + EndpointBuilder = CreateEndpointBuilderFromFilterFactories(new List>() { (routeHandlerContext, next) => { - routeHandlerContext.EndpointMetadata.Add(filter1Tag); + routeHandlerContext.EndpointBuilder.Metadata.Add(filter1Tag); return next; }, (routeHandlerContext, next) => { - routeHandlerContext.EndpointMetadata.Add(filter2Tag); + routeHandlerContext.EndpointBuilder.Metadata.Add(filter2Tag); return next; }, - } + }), }; var result = RequestDelegateFactory.Create(initialRequestDelegate, options); @@ -6378,6 +6380,37 @@ public void Create_ReturnsSameRequestDelegatePassedIn_IfNotModifiedByFilters() m => Assert.Same(filter1Tag, m)); } + [Fact] + public void Create_Rejects_EndpointBuilderWithNonNullRequestDelegate() + { + RequestDelegate requestDelegate = static context => Task.CompletedTask; + + RequestDelegateFactoryOptions options = new() + { + EndpointBuilder = new RouteEndpointBuilder(requestDelegate, RoutePatternFactory.Parse("/"), order: 0), + }; + + var ex = Assert.Throws(() => RequestDelegateFactory.Create(requestDelegate, options)); + + Assert.Equal("options", ex.ParamName); + } + + [Fact] + public void Create_Populates_EndpointBuilderWithRequestDelegateAndMetadata() + { + RequestDelegate requestDelegate = static context => Task.CompletedTask; + + RequestDelegateFactoryOptions options = new() + { + EndpointBuilder = new RouteEndpointBuilder(null, RoutePatternFactory.Parse("/"), order: 0), + }; + + var result = RequestDelegateFactory.Create(requestDelegate, options); + + Assert.Same(options.EndpointBuilder.RequestDelegate, result.RequestDelegate); + Assert.Same(options.EndpointBuilder.Metadata, result.EndpointMetadata); + } + private DefaultHttpContext CreateHttpContext() { var responseFeature = new TestHttpResponseFeature(); @@ -6394,6 +6427,20 @@ private DefaultHttpContext CreateHttpContext() }; } + private EndpointBuilder CreateEndpointBuilderFromMetadata(IEnumerable metadata) + { + var builder = new RouteEndpointBuilder(null, RoutePatternFactory.Parse("/"), 0); + ((List)builder.Metadata).AddRange(metadata); + return builder; + } + + private EndpointBuilder CreateEndpointBuilderFromFilterFactories(IEnumerable> filterFactories) + { + var builder = new RouteEndpointBuilder(null, RoutePatternFactory.Parse("/"), 0); + ((List>)builder.FilterFactories).AddRange(filterFactories); + return builder; + } + private record MetadataService; private class AccessesServicesMetadataResult : IResult, IEndpointMetadataProvider diff --git a/src/Http/Routing/src/PublicAPI.Unshipped.txt b/src/Http/Routing/src/PublicAPI.Unshipped.txt index fc232ca4290e..d76cc7d7b97a 100644 --- a/src/Http/Routing/src/PublicAPI.Unshipped.txt +++ b/src/Http/Routing/src/PublicAPI.Unshipped.txt @@ -1,8 +1,10 @@ #nullable enable +*REMOVED*Microsoft.AspNetCore.Routing.RouteEndpointBuilder.RouteEndpointBuilder(Microsoft.AspNetCore.Http.RequestDelegate! requestDelegate, Microsoft.AspNetCore.Routing.Patterns.RoutePattern! routePattern, int order) -> void Microsoft.AspNetCore.Http.EndpointFilterExtensions Microsoft.AspNetCore.Routing.CompositeEndpointDataSource.Dispose() -> void Microsoft.AspNetCore.Routing.HttpMethodMetadata.AcceptCorsPreflight.set -> void Microsoft.AspNetCore.Routing.IHttpMethodMetadata.AcceptCorsPreflight.set -> void +Microsoft.AspNetCore.Routing.RouteEndpointBuilder.RouteEndpointBuilder(Microsoft.AspNetCore.Http.RequestDelegate? requestDelegate, Microsoft.AspNetCore.Routing.Patterns.RoutePattern! routePattern, int order) -> void Microsoft.AspNetCore.Routing.RouteGroupBuilder Microsoft.AspNetCore.Routing.RouteGroupContext Microsoft.AspNetCore.Routing.RouteGroupContext.ApplicationServices.get -> System.IServiceProvider! diff --git a/src/Http/Routing/src/RouteEndpointBuilder.cs b/src/Http/Routing/src/RouteEndpointBuilder.cs index 2498426d06e7..8c37f1047991 100644 --- a/src/Http/Routing/src/RouteEndpointBuilder.cs +++ b/src/Http/Routing/src/RouteEndpointBuilder.cs @@ -30,10 +30,12 @@ public sealed class RouteEndpointBuilder : EndpointBuilder /// The to use in URL matching. /// The order assigned to the endpoint. public RouteEndpointBuilder( - RequestDelegate requestDelegate, + RequestDelegate? requestDelegate, RoutePattern routePattern, int order) { + ArgumentNullException.ThrowIfNull(routePattern); + RequestDelegate = requestDelegate; RoutePattern = routePattern; Order = order; diff --git a/src/Http/Routing/src/RouteEndpointDataSource.cs b/src/Http/Routing/src/RouteEndpointDataSource.cs index 64c288819716..005edbddc02e 100644 --- a/src/Http/Routing/src/RouteEndpointDataSource.cs +++ b/src/Http/Routing/src/RouteEndpointDataSource.cs @@ -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 System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Runtime.CompilerServices; @@ -196,6 +197,12 @@ private RouteEndpointBuilder CreateRouteEndpointBuilder( entrySpecificConvention(builder); } + // If no convention has modified builder.RequestDelegate, we can use the RequestDelegate returned by the RequestDelegateFactory directly. + var conventionOverriddenRequestDelegate = ReferenceEquals(builder.RequestDelegate, redirectRequestDelegate) ? null : builder.RequestDelegate; + + // Now that we're done running conventions, we no longer need the redirect RequestDelegate. The RequestDelegateFactory prevents it being set ahead of time. + builder.RequestDelegate = null; + if (isRouteHandler || builder.FilterFactories.Count > 0) { var routeParamNames = new List(pattern.Parameters.Count); @@ -210,20 +217,19 @@ private RouteEndpointBuilder CreateRouteEndpointBuilder( RouteParameterNames = routeParamNames, ThrowOnBadRequest = _throwOnBadRequest, DisableInferBodyFromParameters = ShouldDisableInferredBodyParameters(entry.HttpMethods), - EndpointMetadata = builder.Metadata, - EndpointFilterFactories = builder.FilterFactories.AsReadOnly(), + EndpointBuilder = builder, }; // We ignore the returned EndpointMetadata has been already populated since we passed in non-null EndpointMetadata. + // We always set factoryRequestDelegate in case something is still referencing the redirected version of the RequestDelegate. factoryCreatedRequestDelegate = RequestDelegateFactory.Create(entry.RouteHandler, factoryOptions).RequestDelegate; } - if (ReferenceEquals(builder.RequestDelegate, redirectRequestDelegate)) - { - // No convention has changed builder.RequestDelegate, so we can just replace it with the final version as an optimization. - // We still set factoryRequestDelegate in case something is still referencing the redirected version of the RequestDelegate. - builder.RequestDelegate = factoryCreatedRequestDelegate; - } + Debug.Assert(factoryCreatedRequestDelegate is not null); + + // Use the overridden RequestDelegate if it exists. If the overridden RequestDelegate is merely wrapping the final RequestDelegate, + // it will still work because of the redirectRequestDelegate. + builder.RequestDelegate = conventionOverriddenRequestDelegate ?? factoryCreatedRequestDelegate; return builder; } diff --git a/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs index d1e24f276702..e101cc56892a 100644 --- a/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs @@ -115,7 +115,7 @@ public async Task MapEndpoint_CanBeFiltered_ByEndpointFilters(Func { - routeHandlerContext.EndpointMetadata.Add(filterTag); + routeHandlerContext.EndpointBuilder.Metadata.Add(filterTag); return async invocationContext => { Assert.IsAssignableFrom(Assert.Single(invocationContext.Arguments)); @@ -151,7 +151,7 @@ public void MapEndpoint_UsesOriginalRequestDelegateInstance_IfFilterDoesNotChang var endpointBuilder = map(builder, "/", initialRequestDelegate).AddEndpointFilterFactory((routeHandlerContext, next) => { - routeHandlerContext.EndpointMetadata.Add(filterTag); + routeHandlerContext.EndpointBuilder.Metadata.Add(filterTag); return next; }); diff --git a/src/Http/Routing/test/UnitTests/Builder/RouteHandlerEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/RouteHandlerEndpointRouteBuilderExtensionsTest.cs index 4c3822e0a530..d202b97f705f 100644 --- a/src/Http/Routing/test/UnitTests/Builder/RouteHandlerEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/RouteHandlerEndpointRouteBuilderExtensionsTest.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -871,7 +872,7 @@ void WithFilterFactory(IEndpointConventionBuilder builder) => { Assert.NotNull(routeHandlerContext.MethodInfo); Assert.NotNull(routeHandlerContext.MethodInfo.DeclaringType); - Assert.NotNull(routeHandlerContext.ApplicationServices); + Assert.NotNull(routeHandlerContext.EndpointBuilder.ApplicationServices); Assert.Equal("RouteHandlerEndpointRouteBuilderExtensionsTest", routeHandlerContext.MethodInfo.DeclaringType?.Name); context.Arguments[0] = context.GetArgument(0) + 1; return await next(context); @@ -981,7 +982,7 @@ public async Task RequestDelegateFactory_CanInvokeEndpointFilter_ThatAccessesSer } [Fact] - public void RequestDelegateFactory_ProvidesAppServiceProvider_ToFilterFactory() + public void RequestDelegateFactory_ProvidesRouteEndpointBuilder_ToFilterFactory() { var appServiceCollection = new ServiceCollection(); var appService = new MyService(); @@ -991,41 +992,31 @@ public void RequestDelegateFactory_ProvidesAppServiceProvider_ToFilterFactory() string? PrintLogger(HttpContext context) => $"loggerErrorIsEnabled: {context.Items["loggerErrorIsEnabled"]}, parentName: {context.Items["parentName"]}"; var routeHandlerBuilder = builder.Map("/", PrintLogger); + + var customDisplayName = "MyFilterFactory"; + var customRoutePattern = RoutePatternFactory.Parse("/MyFilteredPattern"); + routeHandlerBuilder.AddEndpointFilterFactory((rhc, next) => { - Assert.NotNull(rhc.ApplicationServices); - var myService = rhc.ApplicationServices.GetRequiredService(); + Assert.NotNull(rhc.EndpointBuilder.ApplicationServices); + var myService = rhc.EndpointBuilder.ApplicationServices.GetRequiredService(); Assert.Equal(appService, myService); + + rhc.EndpointBuilder.DisplayName = customDisplayName; + ((RouteEndpointBuilder)rhc.EndpointBuilder).RoutePattern = customRoutePattern; + filterFactoryRan = true; return next; }); var dataSource = GetBuilderEndpointDataSource(builder); // Trigger Endpoint build by calling getter. - Assert.Single(dataSource.Endpoints); + var routeEndpoint = Assert.Single(dataSource.Endpoints); + Assert.Equal(customDisplayName, routeEndpoint.DisplayName); + Assert.Equal(customRoutePattern, routeEndpoint.RoutePattern); Assert.True(filterFactoryRan); } - [Fact] - public void RouteHandlerContext_ThrowsArgumentNullException_ForMethodInfo() - { - Assert.Throws("methodInfo", () => new EndpointFilterFactoryContext(null!, new List(), new ServiceCollection().BuildServiceProvider())); - } - - [Fact] - public void RouteHandlerContext_ThrowsArgumentNullException_ForEndpointMetadata() - { - var handler = () => { }; - Assert.Throws("endpointMetadata", () => new EndpointFilterFactoryContext(handler.Method, null!, new ServiceCollection().BuildServiceProvider())); - } - - [Fact] - public void RouteHandlerContext_ThrowsArgumentNullException_ForApplicationServices() - { - var handler = () => { }; - Assert.Throws("applicationServices", () => new EndpointFilterFactoryContext(handler.Method, new List(), null!)); - } - class MyService { } class ServiceAccessingEndpointFilter : IEndpointFilter diff --git a/src/Http/Routing/test/UnitTests/RouteEndpointBuilderTest.cs b/src/Http/Routing/test/UnitTests/RouteEndpointBuilderTest.cs index 7539757125c1..669962719644 100644 --- a/src/Http/Routing/test/UnitTests/RouteEndpointBuilderTest.cs +++ b/src/Http/Routing/test/UnitTests/RouteEndpointBuilderTest.cs @@ -9,6 +9,20 @@ namespace Microsoft.AspNetCore.Routing; public class RouteEndpointBuilderTest { + [Fact] + public void Constructor_AllowsNullRequestDelegate() + { + var builder = new RouteEndpointBuilder(requestDelegate: null, RoutePatternFactory.Parse("/"), order: 0); + Assert.Null(builder.RequestDelegate); + } + + [Fact] + public void Constructor_DoesNotAllowNullRoutePattern() + { + var ex = Assert.Throws(() => new RouteEndpointBuilder(context => Task.CompletedTask, routePattern: null, order: 0)); + Assert.Equal("routePattern", ex.ParamName); + } + [Fact] public void Build_AllValuesSet_EndpointCreated() { diff --git a/src/Http/samples/MinimalSample/Program.cs b/src/Http/samples/MinimalSample/Program.cs index c0705588ae37..cb899ae7dadf 100644 --- a/src/Http/samples/MinimalSample/Program.cs +++ b/src/Http/samples/MinimalSample/Program.cs @@ -23,7 +23,7 @@ inner.AddEndpointFilterFactory((routeContext, next) => { - var tags = routeContext.EndpointMetadata.OfType().FirstOrDefault(); + var tags = routeContext.EndpointBuilder.Metadata.OfType().FirstOrDefault(); return async invocationContext => { diff --git a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs index 50d2650f5d1d..5323d4521585 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs @@ -324,7 +324,7 @@ private static (RoutePattern resolvedRoutePattern, IDictionary return (attributeRoutePattern, resolvedRequiredValues ?? action.RouteValues); } - private void AddActionDataToBuilder( + private static void AddActionDataToBuilder( EndpointBuilder builder, HashSet routeNames, ActionDescriptor action, @@ -451,7 +451,11 @@ private void AddActionDataToBuilder( return controllerInvocationContext.ActionDescriptor.CacheEntry!.InnerActionMethodExecutor.Execute(controllerInvocationContext); }; - var context = new EndpointFilterFactoryContext(cad.MethodInfo, builder.Metadata, _serviceProvider); + var context = new EndpointFilterFactoryContext + { + MethodInfo = cad.MethodInfo, + EndpointBuilder = builder, + }; var initialFilteredInvocation = del; From 551037ff2030b3e4b6a984c0bd364691406983cc Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Sun, 14 Aug 2022 12:30:19 -0700 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Brennan --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 52db3f41baf3..fb526313532d 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -217,8 +217,8 @@ private static FactoryContext CreateFactoryContext(RequestDelegateFactoryOptions throw new ArgumentException($"{nameof(RequestDelegateFactoryOptions)}.{nameof(RequestDelegateFactoryOptions.EndpointBuilder)} must be null.", nameof(options)); } - var endointBuilder = options?.EndpointBuilder ?? new RDFEndpointBuilder(); - var serviceProvider = options?.ServiceProvider ?? endointBuilder.ApplicationServices; + var endpointBuilder = options?.EndpointBuilder ?? new RDFEndpointBuilder(); + var serviceProvider = options?.ServiceProvider ?? endpointBuilder.ApplicationServices; return new FactoryContext { @@ -228,7 +228,7 @@ private static FactoryContext CreateFactoryContext(RequestDelegateFactoryOptions RouteParameters = options?.RouteParameterNames?.ToList(), ThrowOnBadRequest = options?.ThrowOnBadRequest ?? false, DisableInferredFromBody = options?.DisableInferBodyFromParameters ?? false, - EndpointBuilder = endointBuilder, + EndpointBuilder = endpointBuilder, }; } @@ -281,7 +281,7 @@ private static IReadOnlyList AsReadOnlyList(IList 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.EndpointBuilder.FilterFactories.Count > 0 ) + if (factoryContext.EndpointBuilder.FilterFactories.Count > 0) { filterPipeline = CreateFilterPipeline(methodInfo, targetExpression, factoryContext, targetFactory); @@ -864,7 +864,7 @@ private static Expression CreateParamCheckingResponseWritingMethodCall(Type retu // If filters have been registered, we set the `wasParamCheckFailure` property // but do not return from the invocation to allow the filters to run. - if (factoryContext.EndpointBuilder.FilterFactories.Count > 0 ) + if (factoryContext.EndpointBuilder.FilterFactories.Count > 0) { // if (wasParamCheckFailure) // { From 635a189c7ffd87d4e8a54c66a8f819bc456e6d94 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Sun, 14 Aug 2022 12:31:44 -0700 Subject: [PATCH 3/4] Remove redundant if/else --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index fb526313532d..ccdaf1105721 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -149,15 +149,6 @@ public static RequestDelegateResult Create(Delegate handler, RequestDelegateFact _ => httpContext => targetableRequestDelegate(handler.Target, httpContext), }; - if (targetableRequestDelegate is null) - { - finalRequestDelegate = (RequestDelegate)handler; - } - else - { - finalRequestDelegate = httpContext => targetableRequestDelegate(handler.Target, httpContext); - } - return CreateRequestDelegateResult(finalRequestDelegate, factoryContext.EndpointBuilder); } From fc587726248e008371f24d8378ae0ddd8d264b44 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 15 Aug 2022 07:55:08 -0700 Subject: [PATCH 4/4] Fix ArgumentException --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index ccdaf1105721..a2a71af7b224 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -205,7 +205,7 @@ private static FactoryContext CreateFactoryContext(RequestDelegateFactoryOptions { if (options?.EndpointBuilder?.RequestDelegate is not null) { - throw new ArgumentException($"{nameof(RequestDelegateFactoryOptions)}.{nameof(RequestDelegateFactoryOptions.EndpointBuilder)} must be null.", nameof(options)); + throw new ArgumentException($"{nameof(RequestDelegateFactoryOptions)}.{nameof(RequestDelegateFactoryOptions.EndpointBuilder)}.{nameof(EndpointBuilder.RequestDelegate)} must be null.", nameof(options)); } var endpointBuilder = options?.EndpointBuilder ?? new RDFEndpointBuilder();