Skip to content

[AOT] Add expression free request filter pipeline for RequestDelegate #46020

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 9 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -23,6 +23,7 @@
<Compile Include="$(SharedSourceRoot)ProblemDetails\ProblemDetailsDefaults.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)ValueStringBuilder\**\*.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)Json\JsonSerializerExtensions.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)RouteHandlers\ExecuteHandlerHelper.cs" LinkBase="Shared"/>
</ItemGroup>

<ItemGroup>
Expand Down
52 changes: 10 additions & 42 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Http.Json;
using Microsoft.AspNetCore.Http.Metadata;
using Microsoft.AspNetCore.Internal;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Internal;
Expand Down Expand Up @@ -260,12 +261,12 @@ public static RequestDelegateResult Create(MethodInfo methodInfo, Func<HttpConte

private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegateFactoryOptions? options, RequestDelegateMetadataResult? metadataResult = null, Delegate? handler = null)
{
if (metadataResult?.CachedFactoryContext is not null)
if (metadataResult?.CachedFactoryContext is RequestDelegateFactoryContext cachedFactoryContext)
{
metadataResult.CachedFactoryContext.MetadataAlreadyInferred = true;
cachedFactoryContext.MetadataAlreadyInferred = true;
// The handler was not passed in to the InferMetadata call that originally created this context.
metadataResult.CachedFactoryContext.Handler = handler;
return metadataResult.CachedFactoryContext;
cachedFactoryContext.Handler = handler;
return cachedFactoryContext;
}

var serviceProvider = options?.ServiceProvider ?? options?.EndpointBuilder?.ApplicationServices ?? EmptyServiceProvider.Instance;
Expand Down Expand Up @@ -2135,21 +2136,7 @@ static async Task ExecuteAwaited(Task<object> task, HttpContext httpContext, Jso

private static Task ExecuteAwaitedReturn(object obj, HttpContext httpContext, JsonSerializerOptions options, JsonTypeInfo<object> jsonTypeInfo)
{
// Terminal built ins
if (obj is IResult result)
{
return ExecuteResultWriteResponse(result, httpContext);
}
else if (obj is string stringValue)
{
SetPlaintextContentType(httpContext);
return httpContext.Response.WriteAsync(stringValue);
}
else
{
// Otherwise, we JSON serialize when we reach the terminal state
return WriteJsonResponse(httpContext.Response, obj, options, jsonTypeInfo);
}
return ExecuteHandlerHelper.ExecuteReturnAsync(obj, httpContext, options, jsonTypeInfo);
}

private static Task ExecuteTaskOfTFast<T>(Task<T> task, HttpContext httpContext, JsonTypeInfo<T> jsonTypeInfo)
Expand Down Expand Up @@ -2188,7 +2175,7 @@ static async Task ExecuteAwaited(Task<T> task, HttpContext httpContext, JsonSeri

private static Task ExecuteTaskOfString(Task<string?> task, HttpContext httpContext)
{
SetPlaintextContentType(httpContext);
ExecuteHandlerHelper.SetPlaintextContentType(httpContext);
EnsureRequestTaskNotNull(task);

static async Task ExecuteAwaited(Task<string> task, HttpContext httpContext)
Expand All @@ -2206,7 +2193,7 @@ static async Task ExecuteAwaited(Task<string> task, HttpContext httpContext)

private static Task ExecuteWriteStringResponseAsync(HttpContext httpContext, string text)
{
SetPlaintextContentType(httpContext);
ExecuteHandlerHelper.SetPlaintextContentType(httpContext);
return httpContext.Response.WriteAsync(text);
}

Expand Down Expand Up @@ -2293,7 +2280,7 @@ static async Task ExecuteAwaited(ValueTask<T> task, HttpContext httpContext, Jso

private static Task ExecuteValueTaskOfString(ValueTask<string?> task, HttpContext httpContext)
{
SetPlaintextContentType(httpContext);
ExecuteHandlerHelper.SetPlaintextContentType(httpContext);

static async Task ExecuteAwaited(ValueTask<string> task, HttpContext httpContext)
{
Expand Down Expand Up @@ -2342,21 +2329,7 @@ private static Task WriteJsonResponseFast<T>(HttpResponse response, T value, Jso

private static Task WriteJsonResponse<T>(HttpResponse response, T? value, JsonSerializerOptions options, JsonTypeInfo<T> jsonTypeInfo)
{
var runtimeType = value?.GetType();

if (runtimeType is null || jsonTypeInfo.Type == runtimeType || jsonTypeInfo.IsPolymorphicSafe())
{
// In this case the polymorphism is not
// relevant for us and will be handled by STJ, if needed.
return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, jsonTypeInfo, default);
}

// Call WriteAsJsonAsync() with the runtime type to serialize the runtime type rather than the declared type
// and avoid source generators issues.
// https://github.com/dotnet/aspnetcore/issues/43894
// https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-polymorphism
var runtimeTypeInfo = options.GetTypeInfo(runtimeType);
return HttpResponseJsonExtensions.WriteAsJsonAsync(response, value!, runtimeTypeInfo, default);
return ExecuteHandlerHelper.WriteJsonResponseAsync(response, value, options, jsonTypeInfo);
}

private static NotSupportedException GetUnsupportedReturnTypeException(Type returnType)
Expand Down Expand Up @@ -2545,11 +2518,6 @@ private static IResult EnsureRequestResultNotNull(IResult? result)
return result;
}

private static void SetPlaintextContentType(HttpContext httpContext)
{
httpContext.Response.ContentType ??= "text/plain; charset=utf-8";
}

private static string BuildErrorMessageForMultipleBodyParameters(RequestDelegateFactoryContext factoryContext)
{
var errorMessage = new StringBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ public sealed class RequestDelegateMetadataResult

// This internal cached context avoids redoing unnecessary reflection in Create that was already done in InferMetadata.
// InferMetadata currently does more work than it needs to building up expression trees, but the expectation is that InferMetadata will usually be followed by Create.
internal RequestDelegateFactoryContext? CachedFactoryContext { get; set; }
// The property is typed as object to avoid having a dependency System.Linq.Expressions. The value is RequestDelegateFactoryContext.
internal object? CachedFactoryContext { get; set; }
}
24 changes: 22 additions & 2 deletions src/Http/Routing/src/Builder/EndpointRouteBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,25 @@ private static IEndpointConventionBuilder Map(
ArgumentNullException.ThrowIfNull(pattern);
ArgumentNullException.ThrowIfNull(requestDelegate);

return endpoints.GetOrAddRouteEndpointDataSource().AddRequestDelegate(pattern, requestDelegate, httpMethods);
return endpoints
.GetOrAddRouteEndpointDataSource()
.AddRequestDelegate(pattern, requestDelegate, httpMethods, CreateHandlerRequestDelegate);

static RequestDelegateResult CreateHandlerRequestDelegate(Delegate handler, RequestDelegateFactoryOptions options, RequestDelegateMetadataResult? metadataResult)
{
var requestDelegate = (RequestDelegate)handler;

// Create request delegate that calls filter pipeline.
if (options.EndpointBuilder?.FilterFactories.Count > 0)
{
requestDelegate = RequestDelegateFilterPipelineBuilder.Create(requestDelegate, options);
}

IReadOnlyList<object> metadata = options.EndpointBuilder?.Metadata is not null ?
new List<object>(options.EndpointBuilder.Metadata) :
Array.Empty<object>();
return new RequestDelegateResult(requestDelegate, metadata);
}
}

/// <summary>
Expand Down Expand Up @@ -416,7 +434,9 @@ private static RouteHandlerBuilder Map(
ArgumentNullException.ThrowIfNull(pattern);
ArgumentNullException.ThrowIfNull(handler);

return endpoints.GetOrAddRouteEndpointDataSource().AddRouteHandler(pattern, handler, httpMethods, isFallback);
return endpoints
.GetOrAddRouteEndpointDataSource()
.AddRouteHandler(pattern, handler, httpMethods, isFallback, RequestDelegateFactory.InferMetadata, RequestDelegateFactory.Create);
}

private static RouteEndpointDataSource GetOrAddRouteEndpointDataSource(this IEndpointRouteBuilder endpoints)
Expand Down
8 changes: 0 additions & 8 deletions src/Http/Routing/src/Builder/RouteHandlerBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// 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.Routing;

namespace Microsoft.AspNetCore.Builder;

/// <summary>
Expand All @@ -14,12 +12,6 @@ public sealed class RouteHandlerBuilder : IEndpointConventionBuilder
private readonly ICollection<Action<EndpointBuilder>>? _conventions;
private readonly ICollection<Action<EndpointBuilder>>? _finallyConventions;

/// <summary>
/// Instantiates a new <see cref="RouteHandlerBuilder" /> given a ThrowOnAddAfterEndpointBuiltConventionCollection from
/// <see cref="RouteEndpointDataSource.AddRouteHandler(Routing.Patterns.RoutePattern, Delegate, IEnumerable{string}?, bool)"/>.
/// </summary>
/// <param name="conventions">The convention list returned from <see cref="RouteEndpointDataSource"/>.</param>
/// <param name="finallyConventions">The final convention list returned from <see cref="RouteEndpointDataSource"/>.</param>
internal RouteHandlerBuilder(ICollection<Action<EndpointBuilder>> conventions, ICollection<Action<EndpointBuilder>> finallyConventions)
{
_conventions = conventions;
Expand Down
2 changes: 2 additions & 0 deletions src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
<Compile Include="$(SharedSourceRoot)MediaType\HttpTokenParsingRule.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)ApiExplorerTypes\*.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)RoutingMetadata\AcceptsMetadata.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)Json\JsonSerializerExtensions.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)RouteHandlers\ExecuteHandlerHelper.cs" LinkBase="Shared"/>
</ItemGroup>

<ItemGroup>
Expand Down
68 changes: 68 additions & 0 deletions src/Http/Routing/src/RequestDelegateFilterPipelineBuilder.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// 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.Text.Json.Serialization.Metadata;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Json;
using Microsoft.AspNetCore.Internal;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;

namespace Microsoft.AspNetCore.Routing;

internal static class RequestDelegateFilterPipelineBuilder
{
// Due to https://github.com/dotnet/aspnetcore/issues/41330 we cannot reference the EmptyHttpResult type
// but users still need to assert on it as in https://github.com/dotnet/aspnetcore/issues/45063
// so we temporarily work around this here by using reflection to get the actual type.
private static readonly object? EmptyHttpResultInstance = Type.GetType("Microsoft.AspNetCore.Http.HttpResults.EmptyHttpResult, Microsoft.AspNetCore.Http.Results")?.GetProperty("Instance")?.GetValue(null, null);

public static RequestDelegate Create(RequestDelegate requestDelegate, RequestDelegateFactoryOptions options)
{
Debug.Assert(options.EndpointBuilder != null);

var serviceProvider = options.ServiceProvider ?? options.EndpointBuilder.ApplicationServices;
var jsonOptions = serviceProvider?.GetService<IOptions<JsonOptions>>()?.Value ?? new JsonOptions();
var jsonSerializerOptions = jsonOptions.SerializerOptions;

var factoryContext = new EndpointFilterFactoryContext
{
MethodInfo = requestDelegate.Method,
ApplicationServices = options.EndpointBuilder.ApplicationServices
};
var jsonTypeInfo = (JsonTypeInfo<object>)jsonSerializerOptions.GetReadOnlyTypeInfo(typeof(object));
Copy link
Member

Choose a reason for hiding this comment

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

How does this work with source generation, if we always get the JsonTypeInfo for object?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

This is weird, but OK I guess.

Copy link
Member

Choose a reason for hiding this comment

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

Very weird.


EndpointFilterDelegate filteredInvocation = async (EndpointFilterInvocationContext context) =>
{
Debug.Assert(EmptyHttpResultInstance != null, "Unable to get EmptyHttpResult instance via reflection.");
if (context.HttpContext.Response.StatusCode < 400)
{
await requestDelegate(context.HttpContext);
}
return EmptyHttpResultInstance;
};

var initialFilteredInvocation = filteredInvocation;
for (var i = options.EndpointBuilder.FilterFactories.Count - 1; i >= 0; i--)
{
var currentFilterFactory = options.EndpointBuilder.FilterFactories[i];
filteredInvocation = currentFilterFactory(factoryContext, filteredInvocation);
}

// The filter factories have run without modifying per-request behavior, we can skip running the pipeline.
if (ReferenceEquals(initialFilteredInvocation, filteredInvocation))
{
return requestDelegate;
}

return async (HttpContext httpContext) =>
{
var obj = await filteredInvocation(new DefaultEndpointFilterInvocationContext(httpContext, new object[] { httpContext }));
if (obj is not null)
{
await ExecuteHandlerHelper.ExecuteReturnAsync(obj, httpContext, jsonSerializerOptions, jsonTypeInfo);
}
};
}
}
Loading