Add support for processing RequiredAttribute on OpenApi schemas (#56225)

captainsafia · web-flow · commit 333f1de9981d · 2024-06-14T12:05:41.000-07:00
* Add support for processing RequiredAttribute on OpenApi schemas

* Bring back missing using

* Address feedback

* Fix bad merge
diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Concurrent;
+using System.ComponentModel.DataAnnotations;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
@@ -11,6 +12,7 @@
 using Microsoft.AspNetCore.Http.Metadata;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.ApiExplorer;
+using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.AspNetCore.WebUtilities;
@@ -262,9 +264,7 @@ private async Task<OpenApiResponse> GetResponseAsync(ApiDescription apiDescripti
                     "Path" => ParameterLocation.Path,
                     _ => throw new InvalidOperationException($"Unsupported parameter source: {parameter.Source.Id}")
                 },
-                // Per the OpenAPI specification, parameters that are sourced from the path
-                // are always required, regardless of the requiredness status of the parameter.
-                Required = parameter.Source == BindingSource.Path || parameter.IsRequired,
+                Required = IsRequired(parameter),
                 Schema = await _componentService.GetOrCreateSchemaAsync(parameter.Type, parameter, cancellationToken),
             };
             parameters ??= [];
@@ -273,6 +273,15 @@ private async Task<OpenApiResponse> GetResponseAsync(ApiDescription apiDescripti
         return parameters;
     }
 
+    private static bool IsRequired(ApiParameterDescription parameter)
+    {
+        var hasRequiredAttribute = parameter.ParameterDescriptor is IParameterInfoParameterDescriptor parameterInfoDescriptor &&
+            parameterInfoDescriptor.ParameterInfo.GetCustomAttributes(inherit: true).Any(attr => attr is RequiredAttribute);
+        // Per the OpenAPI specification, parameters that are sourced from the path
+        // are always required, regardless of the requiredness status of the parameter.
+        return parameter.Source == BindingSource.Path || parameter.IsRequired || hasRequiredAttribute;
+    }
+
     private async Task<OpenApiRequestBody?> GetRequestBodyAsync(ApiDescription description, CancellationToken cancellationToken)
     {
         // Only one parameter can be bound from the body in each request.
@@ -301,7 +310,7 @@ private async Task<OpenApiRequestBody> GetFormRequestBody(IList<ApiRequestFormat
 
         var requestBody = new OpenApiRequestBody
         {
-            Required = formParameters.Any(parameter => parameter.IsRequired),
+            Required = formParameters.Any(IsRequired),
             Content = new Dictionary<string, OpenApiMediaType>()
         };
 
@@ -435,7 +444,7 @@ private async Task<OpenApiRequestBody> GetJsonRequestBody(IList<ApiRequestFormat
 
         var requestBody = new OpenApiRequestBody
         {
-            Required = bodyParameter.IsRequired,
+            Required = IsRequired(bodyParameter),
             Content = new Dictionary<string, OpenApiMediaType>()
         };
 
diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs
@@ -8,6 +8,7 @@
 using System.Linq;
 using System.Text.Json;
 using System.Text.Json.Nodes;
+using System.Text.Json.Serialization.Metadata;
 using JsonSchemaMapper;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Json;
@@ -32,7 +33,26 @@ internal sealed class OpenApiSchemaService(
 {
     private readonly OpenApiSchemaStore _schemaStore = serviceProvider.GetRequiredKeyedService<OpenApiSchemaStore>(documentName);
     private readonly OpenApiOptions _openApiOptions = optionsMonitor.Get(documentName);
-    private readonly JsonSerializerOptions _jsonSerializerOptions = jsonOptions.Value.SerializerOptions;
+    private readonly JsonSerializerOptions _jsonSerializerOptions = new(jsonOptions.Value.SerializerOptions)
+    {
+        // In order to properly handle the `RequiredAttribute` on type properties, add a modifier to support
+        // setting `JsonPropertyInfo.IsRequired` based on the presence of the `RequiredAttribute`.
+        TypeInfoResolver = jsonOptions.Value.SerializerOptions.TypeInfoResolver?.WithAddedModifier(jsonTypeInfo =>
+        {
+            if (jsonTypeInfo.Kind != JsonTypeInfoKind.Object)
+            {
+                return;
+            }
+            foreach (var propertyInfo in jsonTypeInfo.Properties)
+            {
+                var hasRequiredAttribute = propertyInfo.AttributeProvider?
+                    .GetCustomAttributes(inherit: false)
+                    .Any(attr => attr is RequiredAttribute);
+                propertyInfo.IsRequired |= hasRequiredAttribute ?? false;
+            }
+        })
+    };
+
     private readonly JsonSchemaMapperConfiguration _configuration = new()
     {
         OnSchemaGenerated = (context, schema) =>
diff --git a/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiComponentService.ParameterSchemas.cs b/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiComponentService.ParameterSchemas.cs
@@ -359,6 +359,31 @@ await VerifyOpenApiDocument(builder, document =>
         });
     }
 
+    [Fact]
+    public async Task GetOpenApiParameters_HandlesParametersWithRequiredAttribute()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        // Act -- route parameters are always required so we test other
+        // parameter sources here.
+        builder.MapGet("/api-1", ([Required] string id) => { });
+        builder.MapGet("/api-2", ([Required] int? age) => { });
+        builder.MapGet("/api-3", ([Required] Guid guid) => { });
+        builder.MapGet("/api-4", ([Required][FromHeader] DateTime date) => { });
+
+        // Assert
+        await VerifyOpenApiDocument(builder, document =>
+        {
+            foreach (var path in document.Paths.Values)
+            {
+                var operation = path.Operations[OperationType.Get];
+                var parameter = Assert.Single(operation.Parameters);
+                Assert.True(parameter.Required);
+            }
+        });
+    }
+
     public static object[][] ArrayBasedQueryParameters =>
     [
         [(int[] id) => { }, "integer", false],
diff --git a/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiComponentService.RequestBodySchemas.cs b/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiComponentService.RequestBodySchemas.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.ComponentModel.DataAnnotations;
 using System.IO.Pipelines;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Mvc;
@@ -102,6 +103,58 @@ await VerifyOpenApiDocument(builder, document =>
         });
     }
 
+    [Fact]
+    public async Task GetOpenApiRequestBody_RespectsRequiredAttributeOnBodyParameter()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        // Act
+        builder.MapPost("/required-poco", ([Required] Todo todo) => { });
+        builder.MapPost("/non-required-poco", (Todo todo) => { });
+        builder.MapPost("/required-form", ([Required][FromForm] Todo todo) => { });
+        builder.MapPost("/non-required-form", ([FromForm] Todo todo) => { });
+        builder.MapPost("/", (ProjectBoard todo) => { });
+
+        // Assert
+        await VerifyOpenApiDocument(builder, document =>
+        {
+            Assert.True(GetRequestBodyForPath(document, "/required-poco").Required);
+            Assert.False(GetRequestBodyForPath(document, "/non-required-poco").Required);
+            Assert.True(GetRequestBodyForPath(document, "/required-form").Required);
+            Assert.False(GetRequestBodyForPath(document, "/non-required-form").Required);
+        });
+
+        static OpenApiRequestBody GetRequestBodyForPath(OpenApiDocument document, string path)
+        {
+            var operation = document.Paths[path].Operations[OperationType.Post];
+            return operation.RequestBody;
+        }
+    }
+
+    [Fact]
+    public async Task GetOpenApiRequestBody_RespectsRequiredAttributeOnBodyProperties()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        // Act
+        builder.MapPost("/required-properties", (RequiredTodo todo) => { });
+
+        // Assert
+        await VerifyOpenApiDocument(builder, document =>
+        {
+            var operation = document.Paths["/required-properties"].Operations[OperationType.Post];
+            var requestBody = operation.RequestBody;
+            var content = Assert.Single(requestBody.Content);
+            var schema = content.Value.Schema;
+            Assert.Collection(schema.Required,
+                property => Assert.Equal("title", property),
+                property => Assert.Equal("completed", property));
+            Assert.DoesNotContain("assignee", schema.Required);
+        });
+    }
+
     [Fact]
     public async Task GetOpenApiRequestBody_GeneratesSchemaForFileTypes()
     {
diff --git a/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiComponentService.ResponseSchemas.cs b/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiComponentService.ResponseSchemas.cs
@@ -182,6 +182,28 @@ await VerifyOpenApiDocument(builder, document =>
         });
     }
 
+    [Fact]
+    public async Task GetOpenApiResponse_RespectsRequiredAttributeOnBodyProperties()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        // Act
+        builder.MapPost("/required-properties", () => new RequiredTodo { Title = "Test Title", Completed = true });
+
+        // Assert
+        await VerifyOpenApiDocument(builder, document =>
+        {
+            var operation = document.Paths["/required-properties"].Operations[OperationType.Post];
+            var response = operation.Responses["200"];
+            var content = Assert.Single(response.Content);
+            var schema = content.Value.Schema;
+            Assert.Collection(schema.Required,
+                property => Assert.Equal("title", property),
+                property => Assert.Equal("completed", property));
+        });
+    }
+
     [Fact]
     public async Task GetOpenApiResponse_HandlesInheritedTypeResponse()
     {
diff --git a/src/OpenApi/test/SharedTypes.cs b/src/OpenApi/test/SharedTypes.cs
@@ -76,6 +76,15 @@ internal class PaginatedItems<T>(int pageIndex, int pageSize, long totalItems, i
     public IEnumerable<T> Items { get; set; } = items;
 }
 
+internal class RequiredTodo
+{
+    [Required]
+    public string Title { get; set; } = string.Empty;
+    [Required]
+    public bool Completed { get; set; }
+    public string Assignee { get; set; } = string.Empty;
+}
+
 #nullable enable
 internal class ProjectBoard
 {
