Fix up OpenAPI schema handling and support concurrent requests (#58024)…

… (#58096) * Fix JsonUnmappedMemberHandling attribute handling to close #57981 * Fix enum handling for MVC actions to close #57979 * Fix self-referential schema handling to close #58006 * Fix concurrent request handling for OpenAPI documents (#57972) * fix: Allow concurrent requests * test: Update test * test: Use Parallel.ForEachAsync * feat: Use valueFactory overload * feat: Pass valueFactory directly * Harden self-referencing schema ID check --------- Co-authored-by: Justin Lampe <[email protected]>
dotnet · Sep 26, 2024 · e4ca1fd · e4ca1fd
1 parent 745868d
commit e4ca1fd
Show file tree

Hide file tree

Showing 7 changed files with 216 additions and 1 deletion.
diff --git a/src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs b/src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs
@@ -280,6 +280,11 @@ public static void ReadProperty(ref Utf8JsonReader reader, string propertyName,
                 break;
             case OpenApiSchemaKeywords.AdditionalPropertiesKeyword:
                 reader.Read();
+                if (reader.TokenType == JsonTokenType.False)
+                {
+                    schema.AdditionalPropertiesAllowed = false;
+                    break;
+                }
                 var additionalPropsConverter = (JsonConverter<OpenApiJsonSchema>)options.GetTypeInfo(typeof(OpenApiJsonSchema)).Converter;
                 schema.AdditionalProperties = additionalPropsConverter.Read(ref reader, typeof(OpenApiJsonSchema), options)?.Schema;
                 break;

diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs
@@ -403,6 +403,12 @@ private async Task<OpenApiResponse> GetResponseAsync(
                 continue;
             }
 
+            // MVC's ModelMetadata layer will set ApiParameterDescription.Type to string when the parameter
+            // is a parsable or convertible type. In this case, we want to use the actual model type
+            // to generate the schema instead of the string type.
+            var targetType = parameter.Type == typeof(string) && parameter.ModelMetadata.ModelType != parameter.Type
+                ? parameter.ModelMetadata.ModelType
+                : parameter.Type;
             var openApiParameter = new OpenApiParameter
             {
                 Name = parameter.Name,
@@ -414,7 +420,7 @@ private async Task<OpenApiResponse> GetResponseAsync(
                     _ => throw new InvalidOperationException($"Unsupported parameter source: {parameter.Source.Id}")
                 },
                 Required = IsRequired(parameter),
-                Schema = await _componentService.GetOrCreateSchemaAsync(parameter.Type, scopedServiceProvider, schemaTransformers, parameter, cancellationToken: cancellationToken),
+                Schema = await _componentService.GetOrCreateSchemaAsync(targetType, scopedServiceProvider, schemaTransformers, parameter, cancellationToken: cancellationToken),
                 Description = GetParameterDescriptionFromAttribute(parameter)
             };
 

diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs
@@ -154,6 +154,14 @@ private void AddOrUpdateAnyOfSubSchemaByReference(OpenApiSchema schema)
     private void AddOrUpdateSchemaByReference(OpenApiSchema schema, string? baseTypeSchemaId = null, bool captureSchemaByRef = false)
     {
         var targetReferenceId = baseTypeSchemaId is not null ? $"{baseTypeSchemaId}{GetSchemaReferenceId(schema)}" : GetSchemaReferenceId(schema);
+        // Schemas that already have a reference provided by JsonSchemaExporter are skipped here
+        // and handled by the OpenApiSchemaReferenceTransformer instead. This case typically kicks
+        // in for self-referencing schemas where JsonSchemaExporter inlines references to avoid
+        // infinite recursion.
+        if (schema.Reference is not null)
+        {
+            return;
+        }
         if (SchemasByReference.TryGetValue(schema, out var referenceId) || captureSchemaByRef)
         {
             // If we've already used this reference ID else where in the document, increment a counter value to the reference

diff --git a/src/OpenApi/src/Transformers/Implementations/OpenApiSchemaReferenceTransformer.cs b/src/OpenApi/src/Transformers/Implementations/OpenApiSchemaReferenceTransformer.cs
@@ -102,6 +102,16 @@ public Task TransformAsync(OpenApiDocument document, OpenApiDocumentTransformerC
             return new OpenApiSchema { Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = referenceId } };
         }
 
+        // Handle schemas where the references have been inline by the JsonSchemaExporter. In this case,
+        // the `#` ID is generated by the exporter since it has no base document to baseline against. In this
+        // case we we want to replace the reference ID with the schema ID that was generated by the
+        // `CreateSchemaReferenceId` method in the OpenApiSchemaService.
+        if (!isTopLevel && schema.Reference is { Type: ReferenceType.Schema, Id: "#" }
+            && schema.Annotations.TryGetValue(OpenApiConstants.SchemaId, out var schemaId))
+        {
+            return new OpenApiSchema { Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = schemaId?.ToString() } };
+        }
+
         if (schema.AllOf is not null)
         {
             for (var i = 0; i < schema.AllOf.Count; i++)

diff --git a/...Core.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs b/...Core.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs
@@ -537,4 +537,59 @@ await VerifyOpenApiDocument(builder, document =>
             Assert.Null(operation.RequestBody.Content["application/json"].Schema.Type);
         });
     }
+
+    [Theory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public async Task SupportsParameterWithEnumType(bool useAction)
+    {
+        // Arrange
+        if (!useAction)
+        {
+            var builder = CreateBuilder();
+            builder.MapGet("/api/with-enum", (ItemStatus status) => status);
+        }
+        else
+        {
+            var action = CreateActionDescriptor(nameof(GetItemStatus));
+            await VerifyOpenApiDocument(action, AssertOpenApiDocument);
+        }
+
+        static void AssertOpenApiDocument(OpenApiDocument document)
+        {
+            var operation = document.Paths["/api/with-enum"].Operations[OperationType.Get];
+            var parameter = Assert.Single(operation.Parameters);
+            var response = Assert.Single(operation.Responses).Value.Content["application/json"].Schema;
+            Assert.NotNull(parameter.Schema.Reference);
+            Assert.Equal(parameter.Schema.Reference.Id, response.Reference.Id);
+            var schema = parameter.Schema.GetEffective(document);
+            Assert.Collection(schema.Enum,
+            value =>
+            {
+                var openApiString = Assert.IsType<OpenApiString>(value);
+                Assert.Equal("Pending", openApiString.Value);
+            },
+            value =>
+            {
+                var openApiString = Assert.IsType<OpenApiString>(value);
+                Assert.Equal("Approved", openApiString.Value);
+            },
+            value =>
+            {
+                var openApiString = Assert.IsType<OpenApiString>(value);
+                Assert.Equal("Rejected", openApiString.Value);
+            });
+        }
+    }
+
+    [Route("/api/with-enum")]
+    private ItemStatus GetItemStatus([FromQuery] ItemStatus status) => status;
+
+    [JsonConverter(typeof(JsonStringEnumConverter<ItemStatus>))]
+    internal enum ItemStatus
+    {
+        Pending = 0,
+        Approved = 1,
+        Rejected = 2,
+    }
 }
diff --git a/...re.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs b/...re.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs
@@ -4,6 +4,7 @@
 using System.ComponentModel;
 using System.ComponentModel.DataAnnotations;
 using System.IO.Pipelines;
+using System.Text.Json.Serialization;
 using System.Text.Json.Serialization.Metadata;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Mvc;
@@ -594,4 +595,107 @@ await VerifyOpenApiDocument(builder, document =>
                 });
         });
     }
+
+    [Fact]
+    public async Task SupportsClassWithJsonUnmappedMemberHandlingDisallowed()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        // Act
+        builder.MapPost("/api", (ExampleWithDisallowedUnmappedMembers type) => { });
+
+        // Assert
+        await VerifyOpenApiDocument(builder, document =>
+        {
+            var operation = document.Paths["/api"].Operations[OperationType.Post];
+            var requestBody = operation.RequestBody;
+            var content = Assert.Single(requestBody.Content);
+            var schema = content.Value.Schema.GetEffective(document);
+            Assert.Collection(schema.Properties,
+                property =>
+                {
+                    Assert.Equal("number", property.Key);
+                    Assert.Equal("integer", property.Value.Type);
+                });
+            Assert.False(schema.AdditionalPropertiesAllowed);
+        });
+    }
+
+    [Fact]
+    public async Task SupportsClassWithJsonUnmappedMemberHandlingSkipped()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        // Act
+        builder.MapPost("/api", (ExampleWithSkippedUnmappedMembers type) => { });
+
+        // Assert
+        await VerifyOpenApiDocument(builder, document =>
+        {
+            var operation = document.Paths["/api"].Operations[OperationType.Post];
+            var requestBody = operation.RequestBody;
+            var content = Assert.Single(requestBody.Content);
+            var schema = content.Value.Schema.GetEffective(document);
+            Assert.Collection(schema.Properties,
+                property =>
+                {
+                    Assert.Equal("number", property.Key);
+                    Assert.Equal("integer", property.Value.Type);
+                });
+            Assert.True(schema.AdditionalPropertiesAllowed);
+        });
+    }
+
+    [JsonUnmappedMemberHandling(JsonUnmappedMemberHandling.Disallow)]
+    private class ExampleWithDisallowedUnmappedMembers
+    {
+        public int Number { get; init; }
+    }
+
+    [JsonUnmappedMemberHandling(JsonUnmappedMemberHandling.Skip)]
+    private class ExampleWithSkippedUnmappedMembers
+    {
+        public int Number { get; init; }
+    }
+
+    [Fact]
+    public async Task SupportsTypesWithSelfReferencedProperties()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        // Act
+        builder.MapPost("/api", (Parent parent) => { });
+
+        // Assert
+        await VerifyOpenApiDocument(builder, document =>
+        {
+            var operation = document.Paths["/api"].Operations[OperationType.Post];
+            var requestBody = operation.RequestBody;
+            var content = Assert.Single(requestBody.Content);
+            var schema = content.Value.Schema.GetEffective(document);
+            Assert.Collection(schema.Properties,
+                property =>
+                {
+                    Assert.Equal("selfReferenceList", property.Key);
+                    Assert.Equal("array", property.Value.Type);
+                    Assert.Equal("Parent", property.Value.Items.Reference.Id);
+                },
+                property =>
+                {
+                    Assert.Equal("selfReferenceDictionary", property.Key);
+                    Assert.Equal("object", property.Value.Type);
+                    Assert.Equal("Parent", property.Value.AdditionalProperties.Reference.Id);
+                });
+        });
+    }
+
+    public class Parent
+    {
+        public IEnumerable<Parent> SelfReferenceList { get; set; } = [ ];
+        public IDictionary<string, Parent> SelfReferenceDictionary { get; set; } = new Dictionary<string, Parent>();
+    }
+
 }
diff --git a/...Core.OpenApi.Tests/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs b/...Core.OpenApi.Tests/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs
@@ -450,4 +450,31 @@ await VerifyOpenApiDocument(builder, document =>
             }
         });
     }
+
+    [Fact]
+    public async Task SelfReferenceMapperOnlyOperatesOnSchemaReferenceTypes()
+    {
+        var builder = CreateBuilder();
+
+        builder.MapGet("/todo", () => new Todo(1, "Item1", false, DateTime.Now));
+
+        var options = new OpenApiOptions();
+        options.AddSchemaTransformer((schema, context, cancellationToken) =>
+        {
+            if (context.JsonTypeInfo.Type == typeof(Todo))
+            {
+                schema.Reference = new OpenApiReference { Id = "#", Type = ReferenceType.Link };
+            }
+            return Task.CompletedTask;
+        });
+
+        await VerifyOpenApiDocument(builder, options, document =>
+        {
+            var operation = document.Paths["/todo"].Operations[OperationType.Get];
+            var response = operation.Responses["200"].Content["application/json"];
+            var responseSchema = response.Schema;
+            Assert.Equal("#", responseSchema.Reference.Id);
+            Assert.Equal(ReferenceType.Link, responseSchema.Reference.Type);
+        });
+    }
 }