Enable trimming for OpenAPI package (#55465)

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
dotnet · May 3, 2024 · d28d0c5 · d28d0c5
1 parent 3fdb60a
commit d28d0c5
Show file tree

Hide file tree

Showing 17 changed files with 133 additions and 24 deletions.
diff --git a/eng/TrimmableProjects.props b/eng/TrimmableProjects.props
@@ -107,5 +107,6 @@
     <TrimmableProject Include="Microsoft.Extensions.Diagnostics.HealthChecks.Abstractions" />
     <TrimmableProject Include="Microsoft.Extensions.Diagnostics.HealthChecks" />
     <TrimmableProject Include="Microsoft.Extensions.Features" />
+    <TrimmableProject Include="Microsoft.AspNetCore.OpenApi" />
   </ItemGroup>
 </Project>
diff --git a/src/Http/Http.Extensions/gen/StaticRouteHandlerModel/EndpointParameter.cs b/src/Http/Http.Extensions/gen/StaticRouteHandlerModel/EndpointParameter.cs
@@ -107,6 +107,16 @@ private void ProcessEndpointParameterSource(Endpoint endpoint, ISymbol symbol, I
             {
                 AssigningCode = "httpContext.Request.Form";
             }
+            // Complex form binding is only supported in RDF because it uses shared source with Blazor that requires dynamic analysis
+            // and codegen. Emit a diagnostic when these are encountered to avoid producing buggy code.
+            else if (!(SymbolEqualityComparer.Default.Equals(Type, wellKnownTypes.Get(WellKnownType.Microsoft_Extensions_Primitives_StringValues))
+                    || Type.SpecialType == SpecialType.System_String
+                    || TryGetParsability(Type, wellKnownTypes, out var _)
+                    || (IsArray && TryGetParsability(ElementType, wellKnownTypes, out var _))))
+            {
+                var location = endpoint.Operation.Syntax.GetLocation();
+                endpoint.Diagnostics.Add(Diagnostic.Create(DiagnosticDescriptors.UnableToResolveParameterDescriptor, location, symbol.Name));
+            }
             else
             {
                 AssigningCode = !IsArray

diff --git a/src/Http/Http.Extensions/test/RequestDelegateGenerator/CompileTimeCreationTests.cs b/src/Http/Http.Extensions/test/RequestDelegateGenerator/CompileTimeCreationTests.cs
@@ -737,4 +737,24 @@ public class ConcreteService : IGenericService<SomeInput, string?>
         var diagnostics = updatedCompilation.GetDiagnostics();
         Assert.Empty(diagnostics.Where(d => d.Severity >= DiagnosticSeverity.Warning));
     }
+
+    [Fact]
+    public async Task RequestDelegateGenerator_SkipsComplexFormParameter()
+    {
+        var source = """
+app.MapPost("/", ([FromForm] Todo todo) => { });
+app.MapPost("/", ([FromForm] Todo todo, IFormFile formFile) => { });
+app.MapPost("/", ([FromForm] Todo todo, [FromForm] int[] ids) => { });
+""";
+        var (generatorRunResult, _) = await RunGeneratorAsync(source);
+
+        // Emits diagnostics but no generated sources
+        var result = Assert.IsType<GeneratorRunResult>(generatorRunResult);
+        Assert.Empty(result.GeneratedSources);
+        Assert.All(result.Diagnostics, diagnostic =>
+        {
+            Assert.Equal(DiagnosticDescriptors.UnableToResolveParameterDescriptor.Id, diagnostic.Id);
+            Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
+        });
+    }
 }
diff --git a/src/OpenApi/sample/Controllers/TestController.cs b/src/OpenApi/sample/Controllers/TestController.cs
@@ -1,4 +1,5 @@
 using System.ComponentModel.DataAnnotations;
+using System.Diagnostics.CodeAnalysis;
 using Microsoft.AspNetCore.Mvc;
 
 [ApiController]
@@ -26,6 +27,7 @@ public class RouteParamsContainer
 
         [FromRoute]
         [MinLength(5)]
+        [UnconditionalSuppressMessage("Trimming", "IL2026:RequiresUnreferencedCode", Justification = "MinLengthAttribute works without reflection on string properties.")]
         public string? Name { get; set; }
     }
 

diff --git a/src/OpenApi/sample/Program.cs b/src/OpenApi/sample/Program.cs
@@ -7,7 +7,9 @@
 
 var builder = WebApplication.CreateBuilder(args);
 
+#pragma warning disable IL2026 // MVC isn't trim-friendly yet
 builder.Services.AddControllers();
+#pragma warning restore IL2026
 builder.Services.AddAuthentication().AddJwtBearer();
 
 builder.Services.AddOpenApi("v1", options =>
@@ -44,8 +46,15 @@
 forms.MapPost("/form-file", (IFormFile resume) => Results.Ok(resume.FileName));
 forms.MapPost("/form-files", (IFormFileCollection files) => Results.Ok(files.Count));
 forms.MapPost("/form-file-multiple", (IFormFile resume, IFormFileCollection files) => Results.Ok(files.Count + resume.FileName));
+// Disable warnings because RDG does not support complex form binding yet.
+#pragma warning disable IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code
+#pragma warning disable IL3050 // Calling members annotated with 'RequiresDynamicCodeAttribute' may break functionality when AOT compiling.
+#pragma warning disable RDG003 // Unable to resolve parameter
 forms.MapPost("/form-todo", ([FromForm] Todo todo) => Results.Ok(todo));
 forms.MapPost("/forms-pocos-and-files", ([FromForm] Todo todo, IFormFile file) => Results.Ok(new { Todo = todo, File = file.FileName }));
+#pragma warning restore RDG003 // Unable to resolve parameter
+#pragma warning restore IL3050 // Calling members annotated with 'RequiresDynamicCodeAttribute' may break functionality when AOT compiling.
+#pragma warning restore IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code
 
 var v1 = app.MapGroup("v1")
     .WithGroupName("v1");

diff --git a/src/OpenApi/sample/Sample.csproj b/src/OpenApi/sample/Sample.csproj
@@ -4,6 +4,9 @@
     <TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
     <Nullable>enable</Nullable>
     <ImplicitUsings>enable</ImplicitUsings>
+    <!-- Required to generated trimmable Map-invocations. -->
+    <EnableRequestDelegateGenerator>true</EnableRequestDelegateGenerator>
+    <IsAotCompatible>true</IsAotCompatible>
   </PropertyGroup>
 
   <ItemGroup>
@@ -22,4 +25,9 @@
     <Compile Include="../test/SharedTypes.cs" />
   </ItemGroup>
 
+  <!-- Required to generated trimmable Map-invocations. -->
+  <ItemGroup>
+    <ProjectReference Include="$(RepoRoot)/src/Http/Http.Extensions/gen/Microsoft.AspNetCore.Http.RequestDelegateGenerator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
+  </ItemGroup>
+
 </Project>
diff --git a/src/OpenApi/src/Extensions/JsonObjectSchemaExtensions.cs b/src/OpenApi/src/Extensions/JsonObjectSchemaExtensions.cs
@@ -4,7 +4,6 @@
 using System.ComponentModel.DataAnnotations;
 using System.Globalization;
 using System.Linq;
-using System.Reflection;
 using System.Text.Json.Nodes;
 using JsonSchemaMapper;
 using Microsoft.AspNetCore.Mvc.ApiExplorer;
@@ -248,14 +247,10 @@ internal static void ApplyParameterInfo(this JsonObject schema, ApiParameterDesc
         // based on our model binding heuristics. In that case, to access the validation attributes that the
         // model binder will respect we will need to get the property from the container type and map the
         // attributes on it to the schema.
-        if (parameterDescription.ModelMetadata.PropertyName is { } propertyName)
+        if (parameterDescription.ModelMetadata is { PropertyName: { }, ContainerType: { }, HasValidators: true, ValidatorMetadata: { } validations })
         {
-            var property = parameterDescription.ModelMetadata.ContainerType?.GetProperty(propertyName, BindingFlags.Public | BindingFlags.Instance);
-            if (property is not null)
-            {
-                var attributes = property.GetCustomAttributes(true).OfType<ValidationAttribute>();
-                schema.ApplyValidationAttributes(attributes);
-            }
+            var attributes = validations.OfType<ValidationAttribute>();
+            schema.ApplyValidationAttributes(attributes);
         }
         // Route constraints are only defined on parameters that are sourced from the path. Since
         // they are encoded in the route template, and not in the type information based to the underlying

diff --git a/src/OpenApi/src/Extensions/OpenApiEndpointConventionBuilderExtensions.cs b/src/OpenApi/src/Extensions/OpenApiEndpointConventionBuilderExtensions.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.CodeAnalysis;
 using System.Linq;
 using System.Reflection;
 using Microsoft.AspNetCore.Http;
@@ -17,12 +18,16 @@ namespace Microsoft.AspNetCore.Builder;
 /// </summary>
 public static class OpenApiEndpointConventionBuilderExtensions
 {
+    private const string TrimWarningMessage = "Calls Microsoft.AspNetCore.OpenApi.OpenApiGenerator.GetOpenApiOperation(MethodInfo, EndpointMetadataCollection, RoutePattern) which uses dynamic analysis. Use IServiceCollection.AddOpenApi() to generate OpenAPI metadata at startup for all endpoints,";
+
     /// <summary>
     /// Adds an OpenAPI annotation to <see cref="Endpoint.Metadata" /> associated
     /// with the current endpoint.
     /// </summary>
     /// <param name="builder">The <see cref="IEndpointConventionBuilder"/>.</param>
     /// <returns>A <see cref="IEndpointConventionBuilder"/> that can be used to further customize the endpoint.</returns>
+    [RequiresDynamicCode(TrimWarningMessage)]
+    [RequiresUnreferencedCode(TrimWarningMessage)]
     public static TBuilder WithOpenApi<TBuilder>(this TBuilder builder) where TBuilder : IEndpointConventionBuilder
     {
         builder.Finally(builder => AddAndConfigureOperationForEndpoint(builder));
@@ -36,13 +41,17 @@ public static class OpenApiEndpointConventionBuilderExtensions
     /// <param name="builder">The <see cref="IEndpointConventionBuilder"/>.</param>
     /// <param name="configureOperation">An <see cref="Func{T, TResult}"/> that returns a new OpenAPI annotation given a generated operation.</param>
     /// <returns>A <see cref="IEndpointConventionBuilder"/> that can be used to further customize the endpoint.</returns>
+    [RequiresDynamicCode(TrimWarningMessage)]
+    [RequiresUnreferencedCode(TrimWarningMessage)]
     public static TBuilder WithOpenApi<TBuilder>(this TBuilder builder, Func<OpenApiOperation, OpenApiOperation> configureOperation)
         where TBuilder : IEndpointConventionBuilder
     {
         builder.Finally(endpointBuilder => AddAndConfigureOperationForEndpoint(endpointBuilder, configureOperation));
         return builder;
     }
 
+    [RequiresDynamicCode(TrimWarningMessage)]
+    [RequiresUnreferencedCode(TrimWarningMessage)]
     private static void AddAndConfigureOperationForEndpoint(EndpointBuilder endpointBuilder, Func<OpenApiOperation, OpenApiOperation>? configure = null)
     {
         foreach (var item in endpointBuilder.Metadata)

diff --git a/src/OpenApi/src/Extensions/OpenApiServiceCollectionExtensions.cs b/src/OpenApi/src/Extensions/OpenApiServiceCollectionExtensions.cs
@@ -1,8 +1,11 @@
 // 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.Http.Json;
 using Microsoft.AspNetCore.OpenApi;
 using Microsoft.Extensions.ApiDescriptions;
+using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Extensions.Options;
 
 namespace Microsoft.Extensions.DependencyInjection;
 
@@ -67,6 +70,8 @@ private static IServiceCollection AddOpenApiCore(this IServiceCollection service
         services.AddSingleton<IDocumentProvider, OpenApiDocumentProvider>();
         // Required to resolve document names for build-time generation
         services.AddSingleton(new NamedService<OpenApiDocumentService>(documentName));
+        // Required to support JSON serializations
+        services.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<JsonOptions>, OpenApiSchemaJsonOptions>());
         return services;
     }
 }
diff --git a/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj b/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj
@@ -7,6 +7,10 @@
     <!-- Needed to support compiling Utf8BufferTextWriter implementation shared with SignalR -->
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <Description>Provides APIs for annotating route handler endpoints in ASP.NET Core with OpenAPI annotations.</Description>
+    <IsAotCompatible>true</IsAotCompatible>
+    <!-- Required to generated native AoT friendly `MapOpenApi` endpoint. -->
+    <EnableRequestDelegateGenerator>true</EnableRequestDelegateGenerator>
+    <InterceptorsPreviewNamespaces>$(InterceptorsPreviewNamespaces);Microsoft.AspNetCore.Http.Generated</InterceptorsPreviewNamespaces>
   </PropertyGroup>
 
   <ItemGroup>
@@ -36,4 +40,9 @@
     <Compile Include="$(RepoRoot)/src/SignalR/common/Shared/MemoryBufferWriter.cs" LinkBase="Shared" />
   </ItemGroup>
 
+  <!-- Required to generated native AoT friendly `MapOpenApi` endpoint. -->
+  <ItemGroup>
+    <ProjectReference Include="$(RepoRoot)/src/Http/Http.Extensions/gen/Microsoft.AspNetCore.Http.RequestDelegateGenerator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
+  </ItemGroup>
+
 </Project>
diff --git a/src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs b/src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs
@@ -4,6 +4,7 @@
 using System.Linq;
 using System.Text.Json;
 using System.Text.Json.Serialization;
+using Microsoft.AspNetCore.OpenApi;
 using Microsoft.OpenApi.Any;
 using Microsoft.OpenApi.Models;
 
@@ -14,9 +15,8 @@ internal sealed partial class OpenApiJsonSchema
     /// </summary>
     /// <typeparam name="T">The type of the elements that will populate the list.</typeparam>
     /// <param name="reader">The <see cref="Utf8JsonReader"/> to consume the list from.</param>
-    /// <param name="options">The <see cref="JsonSerializerOptions"/> instance.</param>
     /// <returns>A list parsed from the JSON array.</returns>
-    public static List<T>? ReadList<T>(ref Utf8JsonReader reader, JsonSerializerOptions options)
+    public static List<T>? ReadList<T>(ref Utf8JsonReader reader)
     {
         if (reader.TokenType == JsonTokenType.Null)
         {
@@ -29,7 +29,7 @@ internal sealed partial class OpenApiJsonSchema
             reader.Read();
             while (reader.TokenType != JsonTokenType.EndArray)
             {
-                values.Add(JsonSerializer.Deserialize<T>(ref reader, options)!);
+                values.Add((T)JsonSerializer.Deserialize(ref reader, typeof(T), OpenApiJsonSchemaContext.Default)!);
                 reader.Read();
             }
 
@@ -49,10 +49,9 @@ internal sealed partial class OpenApiJsonSchema
     /// </summary>
     /// <typeparam name="T">The type associated with the values in the dictionary.</typeparam>
     /// <param name="reader">The <see cref="Utf8JsonReader"/> to consume the dictionary from.</param>
-    /// <param name="options">The <see cref="JsonSerializerOptions"/> instance.</param>
     /// <returns>A dictionary parsed from the JSON object.</returns>
     /// <exception cref="JsonException">Thrown if JSON object is not valid.</exception>
-    public static Dictionary<string, T>? ReadDictionary<T>(ref Utf8JsonReader reader, JsonSerializerOptions options)
+    public static Dictionary<string, T>? ReadDictionary<T>(ref Utf8JsonReader reader)
     {
         if (reader.TokenType == JsonTokenType.Null)
         {
@@ -75,7 +74,7 @@ internal sealed partial class OpenApiJsonSchema
 
             var key = reader.GetString()!;
             reader.Read();
-            values[key] = JsonSerializer.Deserialize<T>(ref reader, options)!;
+            values[key] = (T)JsonSerializer.Deserialize(ref reader, typeof(T), OpenApiJsonSchemaContext.Default)!;
             reader.Read();
         }
 
@@ -170,7 +169,7 @@ public static void ReadProperty(ref Utf8JsonReader reader, string propertyName,
                 reader.Read();
                 if (reader.TokenType == JsonTokenType.StartArray)
                 {
-                    var types = ReadList<string>(ref reader, options);
+                    var types = ReadList<string>(ref reader);
                     foreach (var type in types ?? [])
                     {
                         // JSON Schema represents nullable types using an array consisting of
@@ -195,7 +194,7 @@ public static void ReadProperty(ref Utf8JsonReader reader, string propertyName,
                 break;
             case OpenApiSchemaKeywords.EnumKeyword:
                 reader.Read();
-                var enumValues = ReadList<string>(ref reader, options);
+                var enumValues = ReadList<string>(ref reader);
                 if (enumValues is not null)
                 {
                     schema.Enum = enumValues.Select(v => new OpenApiString(v)).ToList<IOpenApiAny>();
@@ -224,7 +223,7 @@ public static void ReadProperty(ref Utf8JsonReader reader, string propertyName,
                 break;
             case OpenApiSchemaKeywords.RequiredKeyword:
                 reader.Read();
-                schema.Required = ReadList<string>(ref reader, options)?.ToHashSet();
+                schema.Required = ReadList<string>(ref reader)?.ToHashSet();
                 break;
             case OpenApiSchemaKeywords.MinLengthKeyword:
                 reader.Read();
@@ -263,13 +262,13 @@ public static void ReadProperty(ref Utf8JsonReader reader, string propertyName,
                 break;
             case OpenApiSchemaKeywords.PropertiesKeyword:
                 reader.Read();
-                var props = ReadDictionary<OpenApiJsonSchema>(ref reader, options);
+                var props = ReadDictionary<OpenApiJsonSchema>(ref reader);
                 schema.Properties = props?.ToDictionary(p => p.Key, p => p.Value.Schema);
                 break;
             case OpenApiSchemaKeywords.AnyOfKeyword:
                 reader.Read();
                 schema.Type = "object";
-                var schemas = ReadList<OpenApiJsonSchema>(ref reader, options);
+                var schemas = ReadList<OpenApiJsonSchema>(ref reader);
                 schema.AnyOf = schemas?.Select(s => s.Schema).ToList();
                 break;
             case OpenApiSchemaKeywords.DiscriminatorKeyword:
@@ -282,7 +281,7 @@ public static void ReadProperty(ref Utf8JsonReader reader, string propertyName,
                 break;
             case OpenApiSchemaKeywords.DiscriminatorMappingKeyword:
                 reader.Read();
-                var mappings = ReadDictionary<string>(ref reader, options);
+                var mappings = ReadDictionary<string>(ref reader);
                 schema.Discriminator.Mapping = mappings;
                 break;
         }

diff --git a/src/OpenApi/src/Schemas/OpenApiJsonSchemaContext.cs b/src/OpenApi/src/Schemas/OpenApiJsonSchemaContext.cs
@@ -0,0 +1,10 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Text.Json.Serialization;
+
+namespace Microsoft.AspNetCore.OpenApi;
+
+[JsonSerializable(typeof(OpenApiJsonSchema))]
+[JsonSerializable(typeof(string))]
+internal sealed partial class OpenApiJsonSchemaContext : JsonSerializerContext { }
diff --git a/src/OpenApi/src/Schemas/OpenApiSchemaJsonOptions.cs b/src/OpenApi/src/Schemas/OpenApiSchemaJsonOptions.cs
@@ -0,0 +1,16 @@
+// 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.Http.Json;
+using Microsoft.Extensions.Options;
+
+namespace Microsoft.AspNetCore.OpenApi;
+
+internal sealed class OpenApiSchemaJsonOptions : IConfigureOptions<JsonOptions>
+{
+    public void Configure(JsonOptions options)
+    {
+        // Put our resolver in front of the reflection-based one. See ProblemDetailsJsonOptionsSetup for more info.
+        options.SerializerOptions.TypeInfoResolverChain.Insert(0, OpenApiJsonSchemaContext.Default);
+    }
+}
diff --git a/src/OpenApi/src/Services/OpenApiComponentService.cs b/src/OpenApi/src/Services/OpenApiComponentService.cs
@@ -80,7 +80,8 @@ internal OpenApiSchema GetOrCreateSchema(Type type, ApiParameterDescription? par
         {
             schemaAsJsonObject.ApplyParameterInfo(parameterDescription);
         }
-        return JsonSerializer.Deserialize<OpenApiJsonSchema>(schemaAsJsonObject)?.Schema ?? new OpenApiSchema();
+        var deserializedSchema = JsonSerializer.Deserialize(schemaAsJsonObject, OpenApiJsonSchemaContext.Default.OpenApiJsonSchema);
+        return deserializedSchema != null ? deserializedSchema.Schema : new OpenApiSchema();
     }
 
     private JsonObject CreateSchema((Type Type, ParameterInfo? ParameterInfo) key)

diff --git a/src/OpenApi/src/Services/OpenApiGenerator.cs b/src/OpenApi/src/Services/OpenApiGenerator.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.CodeAnalysis;
 using System.Globalization;
 using System.Linq;
 using System.Reflection;
@@ -25,6 +26,8 @@ namespace Microsoft.AspNetCore.OpenApi;
 /// <summary>
 /// Defines a set of methods for generating OpenAPI definitions for endpoints.
 /// </summary>
+[RequiresUnreferencedCode("OpenApiGenerator performs reflection to generate OpenAPI descriptors. This cannot be statically analyzed.")]
+[RequiresDynamicCode("OpenApiGenerator performs reflection to generate OpenAPI descriptors. This cannot be statically analyzed.")]
 internal sealed class OpenApiGenerator
 {
     private readonly IHostEnvironment? _environment;