Only invoke 'IsService' check once in resolution (#47097)

captainsafia · web-flow · commit 2a7dc900bc0a · 2023-03-16T18:18:15.000-07:00
* Only invoke 'IsService' check once in resolution

* Update method name

* Add baseline test
diff --git a/src/Http/Http.Extensions/gen/RequestDelegateGenerator.cs b/src/Http/Http.Extensions/gen/RequestDelegateGenerator.cs
@@ -86,7 +86,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
             codeWriter.WriteLine($"var handler = ({endpoint!.EmitHandlerDelegateCast()})del;");
             codeWriter.WriteLine("EndpointFilterDelegate? filteredInvocation = null;");
             endpoint!.EmitRouteOrQueryResolver(codeWriter);
-            endpoint!.EmitJsonBodyOrServicePreparation(codeWriter);
+            endpoint!.EmitJsonBodyOrServiceResolver(codeWriter);
             endpoint!.Response?.EmitJsonPreparation(codeWriter);
             if (endpoint.NeedsParameterArray)
             {
@@ -190,7 +190,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
 
                 if (hasJsonBodyOrService)
                 {
-                    codeWriter.WriteLine(RequestDelegateGeneratorSources.TryResolveJsonBodyOrServiceAsyncMethod);
+                    codeWriter.WriteLine(RequestDelegateGeneratorSources.ResolveJsonBodyOrServiceMethod);
                 }
 
                 if (hasParsable)
diff --git a/src/Http/Http.Extensions/gen/RequestDelegateGeneratorSources.cs b/src/Http/Http.Extensions/gen/RequestDelegateGeneratorSources.cs
@@ -65,20 +65,6 @@ private static bool TryParseExplicit<T>(string? s, IFormatProvider? provider, [M
             => T.TryParse(s, provider, out result);
 """;
 
-    public static string TryResolveJsonBodyOrServiceAsyncMethod => """
-        private static ValueTask<(bool, T?)> TryResolveJsonBodyOrServiceAsync<T>(HttpContext httpContext, bool isOptional, IServiceProviderIsService? serviceProviderIsService = null)
-        {
-            if (serviceProviderIsService is not null)
-            {
-                if (serviceProviderIsService.IsService(typeof(T)))
-                {
-                    return new ValueTask<(bool, T?)>((true, httpContext.RequestServices.GetService<T>()));
-                }
-            }
-            return TryResolveBodyAsync<T>(httpContext, isOptional);
-        }
-""";
-
     public static string BindAsyncMethod => """
         private static ValueTask<T?> BindAsync<T>(HttpContext context, ParameterInfo parameter)
             where T : class, IBindableFromHttpContext<T>
@@ -109,6 +95,20 @@ private static Task WriteToResponseAsync<T>(T? value, HttpContext httpContext, J
         }
 """;
 
+    public static string ResolveJsonBodyOrServiceMethod => """
+        private static Func<HttpContext, bool, ValueTask<(bool, T?)>> ResolveJsonBodyOrService<T>(IServiceProviderIsService? serviceProviderIsService = null)
+        {
+            if (serviceProviderIsService is not null)
+            {
+                if (serviceProviderIsService.IsService(typeof(T)))
+                {
+                    return static (httpContext, isOptional) => new ValueTask<(bool, T?)>((true, httpContext.RequestServices.GetService<T>()));
+                }
+            }
+            return static (httpContext, isOptional) => TryResolveBodyAsync<T>(httpContext, isOptional);
+        }
+""";
+
     public static string GetGeneratedRouteBuilderExtensionsSource(string genericThunks, string thunks, string endpoints, string helperMethods) => $$"""
 {{SourceHeader}}
 
diff --git a/src/Http/Http.Extensions/gen/StaticRouteHandlerModel/Emitters/EndpointEmitter.cs b/src/Http/Http.Extensions/gen/StaticRouteHandlerModel/Emitters/EndpointEmitter.cs
@@ -61,14 +61,20 @@ public static void EmitRouteOrQueryResolver(this Endpoint endpoint, CodeWriter c
         }
     }
 
-    public static void EmitJsonBodyOrServicePreparation(this Endpoint endpoint, CodeWriter codeWriter)
+    public static void EmitJsonBodyOrServiceResolver(this Endpoint endpoint, CodeWriter codeWriter)
     {
+        var serviceProviderEmitted = false;
         foreach (var parameter in endpoint.Parameters)
         {
             if (parameter.Source == EndpointParameterSource.JsonBodyOrService)
             {
-                codeWriter.WriteLine("var serviceProviderIsService = options?.ServiceProvider?.GetService<IServiceProviderIsService>();");
-                return;
+                if (!serviceProviderEmitted)
+                {
+                    codeWriter.WriteLine("var serviceProviderIsService = options?.ServiceProvider?.GetService<IServiceProviderIsService>();");
+                    serviceProviderEmitted = true;
+                }
+                codeWriter.Write($@"var {parameter.Name}_JsonBodyOrServiceResolver = ");
+                codeWriter.WriteLine($"ResolveJsonBodyOrService<{parameter.Type.ToDisplayString(EmitterConstants.DisplayFormat)}>(serviceProviderIsService);");
             }
         }
     }
diff --git a/src/Http/Http.Extensions/gen/StaticRouteHandlerModel/Emitters/EndpointParameterEmitter.cs b/src/Http/Http.Extensions/gen/StaticRouteHandlerModel/Emitters/EndpointParameterEmitter.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Reflection.Metadata;
 using Microsoft.AspNetCore.Analyzers.Infrastructure;
 using Microsoft.AspNetCore.Analyzers.RouteEmbeddedLanguage.Infrastructure;
 using Microsoft.CodeAnalysis;
@@ -166,15 +167,15 @@ internal static void EmitJsonBodyOrServiceParameterPreparationString(this Endpoi
         // Preamble for diagnostics purposes.
         codeWriter.WriteLine(endpointParameter.EmitParameterDiagnosticComment());
 
-        // Invoke TryResolveJsonBodyOrService method to resolve the
+        // Invoke ResolveJsonBodyOrService method to resolve the
         // type from DI if it exists. Otherwise, resolve the parameter
         // as a body parameter.
-        var assigningCode = $"await GeneratedRouteBuilderExtensionsCore.TryResolveJsonBodyOrServiceAsync<{endpointParameter.Type.ToDisplayString(EmitterConstants.DisplayFormat)}>(httpContext, {(endpointParameter.IsOptional ? "true" : "false")}, serviceProviderIsService)";
+        var assigningCode = $"await {endpointParameter.Name}_JsonBodyOrServiceResolver(httpContext, {(endpointParameter.IsOptional ? "true" : "false")})";
         var resolveJsonBodyOrServiceResult = $"{endpointParameter.Name}_resolveJsonBodyOrServiceResult";
         codeWriter.WriteLine($"var {resolveJsonBodyOrServiceResult} = {assigningCode};");
         codeWriter.WriteLine($"var {endpointParameter.EmitHandlerArgument()} = {resolveJsonBodyOrServiceResult}.Item2;");
 
-        // If binding from the JSON body fails, TryResolveJsonBodyOrService
+        // If binding from the JSON body fails, ResolveJsonBodyOrService
         // will return `false` and we will need to exit early.
         codeWriter.WriteLine($"if (!{resolveJsonBodyOrServiceResult}.Item1)");
         codeWriter.StartBlock();
diff --git a/src/Http/Http.Extensions/test/RequestDelegateGenerator/Baselines/MapAction_JsonBodyOrService_HandlesBothJsonAndService.generated.txt b/src/Http/Http.Extensions/test/RequestDelegateGenerator/Baselines/MapAction_JsonBodyOrService_HandlesBothJsonAndService.generated.txt
@@ -0,0 +1,279 @@
+//------------------------------------------------------------------------------
+// <auto-generated>
+//     This code was generated by a tool.
+//
+//     Changes to this file may cause incorrect behavior and will be lost if
+//     the code is regenerated.
+// </auto-generated>
+//------------------------------------------------------------------------------
+#nullable enable
+
+namespace Microsoft.AspNetCore.Builder
+{
+    %GENERATEDCODEATTRIBUTE%
+    internal class SourceKey
+    {
+        public string Path { get; init; }
+        public int Line { get; init; }
+
+        public SourceKey(string path, int line)
+        {
+            Path = path;
+            Line = line;
+        }
+    }
+
+    // This class needs to be internal so that the compiled application
+    // has access to the strongly-typed endpoint definitions that are
+    // generated by the compiler so that they will be favored by
+    // overload resolution and opt the runtime in to the code generated
+    // implementation produced here.
+    %GENERATEDCODEATTRIBUTE%
+    internal static class GenerateRouteBuilderEndpoints
+    {
+        private static readonly string[] GetVerb = new[] { global::Microsoft.AspNetCore.Http.HttpMethods.Get };
+        private static readonly string[] PostVerb = new[] { global::Microsoft.AspNetCore.Http.HttpMethods.Post };
+        private static readonly string[] PutVerb = new[]  { global::Microsoft.AspNetCore.Http.HttpMethods.Put };
+        private static readonly string[] DeleteVerb = new[] { global::Microsoft.AspNetCore.Http.HttpMethods.Delete };
+        private static readonly string[] PatchVerb = new[] { global::Microsoft.AspNetCore.Http.HttpMethods.Patch };
+
+        internal static global::Microsoft.AspNetCore.Builder.RouteHandlerBuilder MapPost(
+            this global::Microsoft.AspNetCore.Routing.IEndpointRouteBuilder endpoints,
+            [global::System.Diagnostics.CodeAnalysis.StringSyntax("Route")] string pattern,
+            global::System.Func<global::Microsoft.AspNetCore.Http.Generators.Tests.Todo, global::Microsoft.AspNetCore.Http.Generators.Tests.TestService, global::System.String> handler,
+            [global::System.Runtime.CompilerServices.CallerFilePath] string filePath = "",
+            [global::System.Runtime.CompilerServices.CallerLineNumber]int lineNumber = 0)
+        {
+            return global::Microsoft.AspNetCore.Http.Generated.GeneratedRouteBuilderExtensionsCore.MapCore(
+                endpoints,
+                pattern,
+                handler,
+                PostVerb,
+                filePath,
+                lineNumber);
+        }
+
+    }
+}
+
+namespace Microsoft.AspNetCore.Http.Generated
+{
+    using System;
+    using System.Collections;
+    using System.Collections.Generic;
+    using System.Collections.ObjectModel;
+    using System.Diagnostics;
+    using System.Diagnostics.CodeAnalysis;
+    using System.Globalization;
+    using System.Linq;
+    using System.Reflection;
+    using System.Text.Json;
+    using System.Text.Json.Serialization.Metadata;
+    using System.Threading.Tasks;
+    using System.IO;
+    using Microsoft.AspNetCore.Routing;
+    using Microsoft.AspNetCore.Routing.Patterns;
+    using Microsoft.AspNetCore.Builder;
+    using Microsoft.AspNetCore.Http;
+    using Microsoft.AspNetCore.Http.Json;
+    using Microsoft.AspNetCore.Http.Metadata;
+    using Microsoft.Extensions.DependencyInjection;
+    using Microsoft.Extensions.FileProviders;
+    using Microsoft.Extensions.Primitives;
+    using Microsoft.Extensions.Options;
+
+    using MetadataPopulator = System.Func<System.Reflection.MethodInfo, Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions?, Microsoft.AspNetCore.Http.RequestDelegateMetadataResult>;
+    using RequestDelegateFactoryFunc = System.Func<System.Delegate, Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions, Microsoft.AspNetCore.Http.RequestDelegateMetadataResult?, Microsoft.AspNetCore.Http.RequestDelegateResult>;
+
+    file static class GeneratedRouteBuilderExtensionsCore
+    {
+
+        private static readonly Dictionary<(string, int), (MetadataPopulator, RequestDelegateFactoryFunc)> map = new()
+        {
+            [(@"TestMapActions.cs", 24)] = (
+                (methodInfo, options) =>
+                {
+                    Debug.Assert(options?.EndpointBuilder != null, "EndpointBuilder not found.");
+                    options.EndpointBuilder.Metadata.Add(new SourceKey(@"TestMapActions.cs", 24));
+                    return new RequestDelegateMetadataResult { EndpointMetadata = options.EndpointBuilder.Metadata.AsReadOnly() };
+                },
+                (del, options, inferredMetadataResult) =>
+                {
+                    var handler = (Func<global::Microsoft.AspNetCore.Http.Generators.Tests.Todo, global::Microsoft.AspNetCore.Http.Generators.Tests.TestService, global::System.String>)del;
+                    EndpointFilterDelegate? filteredInvocation = null;
+                    var serviceProviderIsService = options?.ServiceProvider?.GetService<IServiceProviderIsService>();
+                    var todo_JsonBodyOrServiceResolver = ResolveJsonBodyOrService<global::Microsoft.AspNetCore.Http.Generators.Tests.Todo>(serviceProviderIsService);
+                    var svc_JsonBodyOrServiceResolver = ResolveJsonBodyOrService<global::Microsoft.AspNetCore.Http.Generators.Tests.TestService>(serviceProviderIsService);
+
+                    if (options?.EndpointBuilder?.FilterFactories.Count > 0)
+                    {
+                        filteredInvocation = GeneratedRouteBuilderExtensionsCore.BuildFilterDelegate(ic =>
+                        {
+                            if (ic.HttpContext.Response.StatusCode == 400)
+                            {
+                                return ValueTask.FromResult<object?>(Results.Empty);
+                            }
+                            return ValueTask.FromResult<object?>(handler(ic.GetArgument<global::Microsoft.AspNetCore.Http.Generators.Tests.Todo>(0), ic.GetArgument<global::Microsoft.AspNetCore.Http.Generators.Tests.TestService>(1)));
+                        },
+                        options.EndpointBuilder,
+                        handler.Method);
+                    }
+
+                    async Task RequestHandler(HttpContext httpContext)
+                    {
+                        var wasParamCheckFailure = false;
+                        // Endpoint Parameter: todo (Type = Microsoft.AspNetCore.Http.Generators.Tests.Todo, IsOptional = False, IsParsable = False, Source = JsonBodyOrService)
+                        var todo_resolveJsonBodyOrServiceResult = await todo_JsonBodyOrServiceResolver(httpContext, false);
+                        var todo_local = todo_resolveJsonBodyOrServiceResult.Item2;
+                        if (!todo_resolveJsonBodyOrServiceResult.Item1)
+                        {
+                            return;
+                        }
+                        // Endpoint Parameter: svc (Type = Microsoft.AspNetCore.Http.Generators.Tests.TestService, IsOptional = False, IsParsable = False, Source = JsonBodyOrService)
+                        var svc_resolveJsonBodyOrServiceResult = await svc_JsonBodyOrServiceResolver(httpContext, false);
+                        var svc_local = svc_resolveJsonBodyOrServiceResult.Item2;
+                        if (!svc_resolveJsonBodyOrServiceResult.Item1)
+                        {
+                            return;
+                        }
+
+                        if (wasParamCheckFailure)
+                        {
+                            httpContext.Response.StatusCode = 400;
+                            return;
+                        }
+                        httpContext.Response.ContentType ??= "text/plain";
+                        var result = handler(todo_local!, svc_local!);
+                        await httpContext.Response.WriteAsync(result);
+                    }
+
+                    async Task RequestHandlerFiltered(HttpContext httpContext)
+                    {
+                        var wasParamCheckFailure = false;
+                        // Endpoint Parameter: todo (Type = Microsoft.AspNetCore.Http.Generators.Tests.Todo, IsOptional = False, IsParsable = False, Source = JsonBodyOrService)
+                        var todo_resolveJsonBodyOrServiceResult = await todo_JsonBodyOrServiceResolver(httpContext, false);
+                        var todo_local = todo_resolveJsonBodyOrServiceResult.Item2;
+                        if (!todo_resolveJsonBodyOrServiceResult.Item1)
+                        {
+                            return;
+                        }
+                        // Endpoint Parameter: svc (Type = Microsoft.AspNetCore.Http.Generators.Tests.TestService, IsOptional = False, IsParsable = False, Source = JsonBodyOrService)
+                        var svc_resolveJsonBodyOrServiceResult = await svc_JsonBodyOrServiceResolver(httpContext, false);
+                        var svc_local = svc_resolveJsonBodyOrServiceResult.Item2;
+                        if (!svc_resolveJsonBodyOrServiceResult.Item1)
+                        {
+                            return;
+                        }
+
+                        if (wasParamCheckFailure)
+                        {
+                            httpContext.Response.StatusCode = 400;
+                        }
+                        var result = await filteredInvocation(EndpointFilterInvocationContext.Create<global::Microsoft.AspNetCore.Http.Generators.Tests.Todo, global::Microsoft.AspNetCore.Http.Generators.Tests.TestService>(httpContext, todo_local!, svc_local!));
+                        await GeneratedRouteBuilderExtensionsCore.ExecuteObjectResult(result, httpContext);
+                    }
+
+                    RequestDelegate targetDelegate = filteredInvocation is null ? RequestHandler : RequestHandlerFiltered;
+                    var metadata = inferredMetadataResult?.EndpointMetadata ?? ReadOnlyCollection<object>.Empty;
+                    return new RequestDelegateResult(targetDelegate, metadata);
+                }),
+
+        };
+
+        internal static RouteHandlerBuilder MapCore(
+            this IEndpointRouteBuilder routes,
+            string pattern,
+            Delegate handler,
+            IEnumerable<string> httpMethods,
+            string filePath,
+            int lineNumber)
+        {
+            var (populateMetadata, createRequestDelegate) = map[(filePath, lineNumber)];
+            return RouteHandlerServices.Map(routes, pattern, handler, httpMethods, populateMetadata, createRequestDelegate);
+        }
+
+        private static EndpointFilterDelegate BuildFilterDelegate(EndpointFilterDelegate filteredInvocation, EndpointBuilder builder, MethodInfo mi)
+        {
+            var routeHandlerFilters =  builder.FilterFactories;
+            var context0 = new EndpointFilterFactoryContext
+            {
+                MethodInfo = mi,
+                ApplicationServices = builder.ApplicationServices,
+            };
+            var initialFilteredInvocation = filteredInvocation;
+            for (var i = routeHandlerFilters.Count - 1; i >= 0; i--)
+            {
+                var filterFactory = routeHandlerFilters[i];
+                filteredInvocation = filterFactory(context0, filteredInvocation);
+            }
+            return filteredInvocation;
+        }
+
+        private static Task ExecuteObjectResult(object? obj, HttpContext httpContext)
+        {
+            if (obj is IResult r)
+            {
+                return r.ExecuteAsync(httpContext);
+            }
+            else if (obj is string s)
+            {
+                return httpContext.Response.WriteAsync(s);
+            }
+            else
+            {
+                return httpContext.Response.WriteAsJsonAsync(obj);
+            }
+        }
+
+        private static async ValueTask<(bool, T?)> TryResolveBodyAsync<T>(HttpContext httpContext, bool allowEmpty)
+        {
+            var feature = httpContext.Features.Get<Microsoft.AspNetCore.Http.Features.IHttpRequestBodyDetectionFeature>();
+
+            if (feature?.CanHaveBody == true)
+            {
+                if (!httpContext.Request.HasJsonContentType())
+                {
+                    httpContext.Response.StatusCode = StatusCodes.Status415UnsupportedMediaType;
+                    return (false, default);
+                }
+                try
+                {
+                    var bodyValue = await httpContext.Request.ReadFromJsonAsync<T>();
+                    if (!allowEmpty && bodyValue == null)
+                    {
+                        httpContext.Response.StatusCode = StatusCodes.Status400BadRequest;
+                        return (false, bodyValue);
+                    }
+                    return (true, bodyValue);
+                }
+                catch (IOException)
+                {
+                    return (false, default);
+                }
+                catch (System.Text.Json.JsonException)
+                {
+                    httpContext.Response.StatusCode = StatusCodes.Status400BadRequest;
+                    return (false, default);
+                }
+            }
+            else if (!allowEmpty)
+            {
+                httpContext.Response.StatusCode = StatusCodes.Status400BadRequest;
+            }
+
+            return (false, default);
+        }
+        private static Func<HttpContext, bool, ValueTask<(bool, T?)>> ResolveJsonBodyOrService<T>(IServiceProviderIsService? serviceProviderIsService = null)
+        {
+            if (serviceProviderIsService is not null)
+            {
+                if (serviceProviderIsService.IsService(typeof(T)))
+                {
+                    return static (httpContext, isOptional) => new ValueTask<(bool, T?)>((true, httpContext.RequestServices.GetService<T>()));
+                }
+            }
+            return static (httpContext, isOptional) => TryResolveBodyAsync<T>(httpContext, isOptional);
+        }
+
+    }
+}
diff --git a/src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTests.JsonBodyOrService.cs b/src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTests.JsonBodyOrService.cs

Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)`
`86`	`86`	`codeWriter.WriteLine($"var handler = ({endpoint!.EmitHandlerDelegateCast()})del;");`
`87`	`87`	`codeWriter.WriteLine("EndpointFilterDelegate? filteredInvocation = null;");`
`88`	`88`	`endpoint!.EmitRouteOrQueryResolver(codeWriter);`
`89`		`- endpoint!.EmitJsonBodyOrServicePreparation(codeWriter);`
	`89`	`+ endpoint!.EmitJsonBodyOrServiceResolver(codeWriter);`
`90`	`90`	`endpoint!.Response?.EmitJsonPreparation(codeWriter);`
`91`	`91`	`if (endpoint.NeedsParameterArray)`
`92`	`92`	`{`
`@@ -190,7 +190,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)`
`190`	`190`
`191`	`191`	`if (hasJsonBodyOrService)`
`192`	`192`	`{`
`193`		`- codeWriter.WriteLine(RequestDelegateGeneratorSources.TryResolveJsonBodyOrServiceAsyncMethod);`
	`193`	`+ codeWriter.WriteLine(RequestDelegateGeneratorSources.ResolveJsonBodyOrServiceMethod);`
`194`	`194`	`}`
`195`	`195`
`196`	`196`	`if (hasParsable)`
Original file line number	Diff line number	Diff line change
`@@ -61,14 +61,20 @@ public static void EmitRouteOrQueryResolver(this Endpoint endpoint, CodeWriter c`
`61`	`61`	`}`
`62`	`62`	`}`
`63`	`63`
`64`		`- public static void EmitJsonBodyOrServicePreparation(this Endpoint endpoint, CodeWriter codeWriter)`
	`64`	`+ public static void EmitJsonBodyOrServiceResolver(this Endpoint endpoint, CodeWriter codeWriter)`
`65`	`65`	`{`
	`66`	`+ var serviceProviderEmitted = false;`
`66`	`67`	`foreach (var parameter in endpoint.Parameters)`
`67`	`68`	`{`
`68`	`69`	`if (parameter.Source == EndpointParameterSource.JsonBodyOrService)`
`69`	`70`	`{`
`70`		`- codeWriter.WriteLine("var serviceProviderIsService = options?.ServiceProvider?.GetService<IServiceProviderIsService>();");`
`71`		`- return;`
	`71`	`+ if (!serviceProviderEmitted)`
	`72`	`+ {`
	`73`	`+ codeWriter.WriteLine("var serviceProviderIsService = options?.ServiceProvider?.GetService<IServiceProviderIsService>();");`
	`74`	`+ serviceProviderEmitted = true;`
	`75`	`+ }`
	`76`	`+ codeWriter.Write($@"var {parameter.Name}_JsonBodyOrServiceResolver = ");`
	`77`	`+ codeWriter.WriteLine($"ResolveJsonBodyOrService<{parameter.Type.ToDisplayString(EmitterConstants.DisplayFormat)}>(serviceProviderIsService);");`
`72`	`78`	`}`
`73`	`79`	`}`
`74`	`80`	`}`