Remove RequestDelegateFactory call from RouteEndpointBuilder (#42827)

Author: halter73

src/Http/Routing/src/Builder/EndpointRouteBuilderExtensions.cs

@@ -2,8 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics.CodeAnalysis;
-using System.Linq;
-using System.Reflection;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.AspNetCore.Routing.Patterns;
@@ -152,10 +150,7 @@ public static IEndpointConventionBuilder MapMethods(
     {
         ArgumentNullException.ThrowIfNull(httpMethods);
 
-        var builder = endpoints.Map(RoutePatternFactory.Parse(pattern), requestDelegate);
-        builder.WithDisplayName($"{pattern} HTTP: {string.Join(", ", httpMethods)}");
-        builder.WithMetadata(new HttpMethodMetadata(httpMethods));
-        return builder;
+        return endpoints.Map(RoutePatternFactory.Parse(pattern), requestDelegate, httpMethods);
     }
 
     /// <summary>
@@ -186,41 +181,21 @@ public static IEndpointConventionBuilder Map(
         this IEndpointRouteBuilder endpoints,
         RoutePattern pattern,
         RequestDelegate requestDelegate)
+    {
+        return Map(endpoints, pattern, requestDelegate, httpMethods: null);
+    }
+
+    private static IEndpointConventionBuilder Map(
+        this IEndpointRouteBuilder endpoints,
+        RoutePattern pattern,
+        RequestDelegate requestDelegate,
+        IEnumerable<string>? httpMethods)
     {
         ArgumentNullException.ThrowIfNull(endpoints);
         ArgumentNullException.ThrowIfNull(pattern);
         ArgumentNullException.ThrowIfNull(requestDelegate);
 
-        const int defaultOrder = 0;
-
-        var builder = new RouteEndpointBuilder(
-            requestDelegate,
-            pattern,
-            defaultOrder)
-        {
-            DisplayName = pattern.RawText ?? pattern.DebuggerToString(),
-        };
-
-        // Add delegate attributes as metadata
-        var attributes = requestDelegate.Method.GetCustomAttributes();
-
-        // This can be null if the delegate is a dynamic method or compiled from an expression tree
-        if (attributes != null)
-        {
-            foreach (var attribute in attributes)
-            {
-                builder.Metadata.Add(attribute);
-            }
-        }
-
-        var dataSource = endpoints.DataSources.OfType<ModelEndpointDataSource>().FirstOrDefault();
-        if (dataSource == null)
-        {
-            dataSource = new ModelEndpointDataSource();
-            endpoints.DataSources.Add(dataSource);
-        }
-
-        return dataSource.AddEndpointBuilder(builder);
+        return endpoints.GetOrAddRouteEndpointDataSource().AddRequestDelegate(pattern, requestDelegate, httpMethods);
     }
 
     /// <summary>
@@ -429,18 +404,38 @@ private static RouteHandlerBuilder Map(
         ArgumentNullException.ThrowIfNull(pattern);
         ArgumentNullException.ThrowIfNull(handler);
 
-        var dataSource = endpoints.DataSources.OfType<RouteEndpointDataSource>().FirstOrDefault();
-        if (dataSource is null)
+        return endpoints.GetOrAddRouteEndpointDataSource().AddRouteHandler(pattern, handler, httpMethods, isFallback);
+    }
+
+    private static RouteEndpointDataSource GetOrAddRouteEndpointDataSource(this IEndpointRouteBuilder endpoints)
+    {
+        RouteEndpointDataSource? routeEndpointDataSource = null;
+
+        foreach (var dataSource in endpoints.DataSources)
         {
-            var routeHandlerOptions = endpoints.ServiceProvider.GetService<IOptions<RouteHandlerOptions>>();
+            if (dataSource is RouteEndpointDataSource foundDataSource)
+            {
+                routeEndpointDataSource = foundDataSource;
+                break;
+            }
+        }
+
+        if (routeEndpointDataSource is null)
+        {
+            // ServiceProvider isn't nullable, but it is being called by methods that historically did not access this property, so we null check anyway.
+            var routeHandlerOptions = endpoints.ServiceProvider?.GetService<IOptions<RouteHandlerOptions>>();
             var throwOnBadRequest = routeHandlerOptions?.Value.ThrowOnBadRequest ?? false;
 
-            dataSource = new RouteEndpointDataSource(endpoints.ServiceProvider, throwOnBadRequest);
-            endpoints.DataSources.Add(dataSource);
+            routeEndpointDataSource = new RouteEndpointDataSource(endpoints.ServiceProvider ?? EmptyServiceProvider.Instance, throwOnBadRequest);
+            endpoints.DataSources.Add(routeEndpointDataSource);
         }
 
-        var conventions = dataSource.AddEndpoint(pattern, handler, httpMethods, isFallback);
+        return routeEndpointDataSource;
+    }
 
-        return new RouteHandlerBuilder(conventions);
+    private sealed class EmptyServiceProvider : IServiceProvider
+    {
+        public static EmptyServiceProvider Instance { get; } = new EmptyServiceProvider();
+        public object? GetService(Type serviceType) => null;
     }
 }

src/Http/Routing/src/Builder/RouteHandlerBuilder.cs

@@ -15,7 +15,7 @@ public sealed class RouteHandlerBuilder : IEndpointConventionBuilder
 
     /// <summary>
     /// Instantiates a new <see cref="RouteHandlerBuilder" /> given a ThrowOnAddAfterEndpointBuiltConventionCollection from
-    /// <see cref="RouteEndpointDataSource.AddEndpoint(Routing.Patterns.RoutePattern, Delegate, IEnumerable{string}?, bool)"/>.
+    /// <see cref="RouteEndpointDataSource.AddRouteHandler(Routing.Patterns.RoutePattern, Delegate, IEnumerable{string}?, bool)"/>.
     /// </summary>
     /// <param name="conventions">The convention list returned from <see cref="RouteEndpointDataSource"/>.</param>
     internal RouteHandlerBuilder(ICollection<Action<EndpointBuilder>> conventions)

src/Http/Routing/src/ConfigureRouteHandlerOptions.cs

@@ -8,16 +8,16 @@ namespace Microsoft.AspNetCore.Routing;
 
 internal sealed class ConfigureRouteHandlerOptions : IConfigureOptions<RouteHandlerOptions>
 {
-    private readonly IHostEnvironment _environment;
+    private readonly IHostEnvironment? _environment;
 
-    public ConfigureRouteHandlerOptions(IHostEnvironment environment)
+    public ConfigureRouteHandlerOptions(IHostEnvironment? environment = null)
     {
         _environment = environment;
     }
 
     public void Configure(RouteHandlerOptions options)
     {
-        if (_environment.IsDevelopment())
+        if (_environment?.IsDevelopment() ?? false)
         {
             options.ThrowOnBadRequest = true;
         }

src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj

@@ -27,7 +27,6 @@
   <ItemGroup>
     <Compile Include="$(SharedSourceRoot)PropertyHelper\*.cs" />
     <Compile Include="$(SharedSourceRoot)RoslynUtils\TypeHelper.cs" />
-    <Compile Include="$(SharedSourceRoot)RoslynUtils\GeneratedNameParser.cs" />
     <Compile Include="$(SharedSourceRoot)MediaType\ReadOnlyMediaTypeHeaderValue.cs" LinkBase="Shared" />
     <Compile Include="$(SharedSourceRoot)MediaType\HttpTokenParsingRule.cs" LinkBase="Shared" />
     <Compile Include="$(SharedSourceRoot)ApiExplorerTypes\*.cs" LinkBase="Shared" />

src/Http/Routing/src/ModelEndpointDataSource.cs

@@ -1,7 +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 System.Diagnostics.CodeAnalysis;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Routing.Patterns;
@@ -44,44 +43,18 @@ public RouteEndpointBuilder(
     }
 
     /// <inheritdoc />
-    [UnconditionalSuppressMessage("Trimmer", "IL2026",
-        Justification = "We surface a RequireUnreferencedCode in AddEndpointFilter which is required to call unreferenced code here. The trimmer is unable to infer this.")]
     public override Endpoint Build()
     {
         if (RequestDelegate is null)
         {
             throw new InvalidOperationException($"{nameof(RequestDelegate)} must be specified to construct a {nameof(RouteEndpoint)}.");
         }
 
-        var requestDelegate = RequestDelegate;
-
-        // Only replace the RequestDelegate if filters have been applied to this builder and they were not already handled by RouteEndpointDataSource.
-        // This affects other data sources like DefaultEndpointDataSource (this is people manually newing up a data source with a list of Endpoints),
-        // ModelEndpointDataSource (Map(RoutePattern, RequestDelegate) and by extension MapHub, MapHealthChecks, etc...),
-        // ActionEndpointDataSourceBase (MapControllers, MapRazorPages, etc...) and people with custom data sources or otherwise manually building endpoints
-        // using this type. At the moment this class is sealed, so at the moment we do not need to concern ourselves with what derived types may be doing.
-        if (EndpointFilterFactories is { Count: > 0 })
-        {
-            // Even with filters applied, RDF.Create() will return back the exact same RequestDelegate instance we pass in if filters decide not to modify the
-            // invocation pipeline. We're just passing in a RequestDelegate so none of the fancy options pertaining to how the Delegate parameters are handled
-            // do not matter.
-            RequestDelegateFactoryOptions rdfOptions = new()
-            {
-                EndpointFilterFactories = EndpointFilterFactories,
-                EndpointMetadata = Metadata,
-            };
-
-            // We ignore the returned EndpointMetadata has been already populated since we passed in non-null EndpointMetadata.
-            requestDelegate = RequestDelegateFactory.Create(requestDelegate, rdfOptions).RequestDelegate;
-        }
-
-        var routeEndpoint = new RouteEndpoint(
-            requestDelegate,
+        return new RouteEndpoint(
+            RequestDelegate,
             RoutePattern,
             Order,
             new EndpointMetadataCollection(Metadata),
             DisplayName);
-
-        return routeEndpoint;
     }
 }