From f471d24df1abe98dcd18857e4c5a610b5864d423 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Sun, 22 Sep 2024 13:19:13 -0700 Subject: [PATCH 1/5] Fix JsonUnmappedMemberHandling attribute handling to close #57981 --- .../src/Schemas/OpenApiJsonSchema.Helpers.cs | 5 ++ ...OpenApiSchemaService.RequestBodySchemas.cs | 65 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs b/src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs index c9d90f5d9e5a..830b0375d396 100644 --- 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)options.GetTypeInfo(typeof(OpenApiJsonSchema)).Converter; schema.AdditionalProperties = additionalPropsConverter.Read(ref reader, typeof(OpenApiJsonSchema), options)?.Schema; break; diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs index e7d0ea13af19..2d14ef927961 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs +++ b/src/OpenApi/test/Microsoft.AspNetCore.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,68 @@ 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; } + } } From 01bda267f8873fc11b548309e81dfefa0dde0d36 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Sun, 22 Sep 2024 15:38:35 -0700 Subject: [PATCH 2/5] Fix enum handling for MVC actions to close #57979 --- .../src/Services/OpenApiDocumentService.cs | 8 ++- .../OpenApiSchemaService.ParameterSchemas.cs | 55 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs index 2745d64770a7..62443c5033e0 100644 --- a/src/OpenApi/src/Services/OpenApiDocumentService.cs +++ b/src/OpenApi/src/Services/OpenApiDocumentService.cs @@ -402,6 +402,12 @@ private async Task 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, @@ -413,7 +419,7 @@ private async Task 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/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs index af9b7e5663d7..4842e189ade4 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs +++ b/src/OpenApi/test/Microsoft.AspNetCore.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(value); + Assert.Equal("Pending", openApiString.Value); + }, + value => + { + var openApiString = Assert.IsType(value); + Assert.Equal("Approved", openApiString.Value); + }, + value => + { + var openApiString = Assert.IsType(value); + Assert.Equal("Rejected", openApiString.Value); + }); + } + } + + [Route("/api/with-enum")] + private ItemStatus GetItemStatus([FromQuery] ItemStatus status) => status; + + [JsonConverter(typeof(JsonStringEnumConverter))] + internal enum ItemStatus + { + Pending = 0, + Approved = 1, + Rejected = 2, + } } From 72c21319cc3ced5673e57a37c3eaf79e25fe5fa6 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Sun, 22 Sep 2024 19:16:16 -0700 Subject: [PATCH 3/5] Fix self-referential schema handling to close #58006 --- .../Services/Schemas/OpenApiSchemaStore.cs | 8 ++++ .../OpenApiSchemaReferenceTransformer.cs | 10 +++++ ...OpenApiSchemaService.RequestBodySchemas.cs | 39 +++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs index 88f1dd4633af..8115e779b123 100644 --- a/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs +++ b/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs @@ -159,6 +159,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 index 07c76fe22974..caa469a38598 100644 --- a/src/OpenApi/src/Transformers/Implementations/OpenApiSchemaReferenceTransformer.cs +++ b/src/OpenApi/src/Transformers/Implementations/OpenApiSchemaReferenceTransformer.cs @@ -101,6 +101,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 { 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/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs index 2d14ef927961..3e95257e874b 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs @@ -659,4 +659,43 @@ 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 SelfReferenceList { get; set; } = [ ]; + public IDictionary SelfReferenceDictionary { get; set; } = new Dictionary(); + } + } From a1c2acc4dea2fae98c6ea1834c78c9a6ffa60987 Mon Sep 17 00:00:00 2001 From: Justin Lampe Date: Sun, 22 Sep 2024 22:22:07 +0200 Subject: [PATCH 4/5] 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 --- .../src/Services/OpenApiDocumentService.cs | 3 ++- .../Services/Schemas/OpenApiSchemaStore.cs | 15 ++++------- .../OpenApiSchemaReferenceTransformer.cs | 3 ++- .../OpenApiDocumentConcurrentRequestTests.cs | 26 +++++++++++++++++++ 4 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/OpenApiDocumentConcurrentRequestTests.cs diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs index 62443c5033e0..c594ebb7ac08 100644 --- a/src/OpenApi/src/Services/OpenApiDocumentService.cs +++ b/src/OpenApi/src/Services/OpenApiDocumentService.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.Collections.Concurrent; using System.Collections.Frozen; using System.ComponentModel; using System.ComponentModel.DataAnnotations; @@ -46,7 +47,7 @@ internal sealed class OpenApiDocumentService( /// are unique within the lifetime of an application and serve as helpful associators between /// operations, API descriptions, and their respective transformer contexts. /// - private readonly Dictionary _operationTransformerContextCache = new(); + private readonly ConcurrentDictionary _operationTransformerContextCache = new(); private static readonly ApiResponseType _defaultApiResponseType = new() { StatusCode = StatusCodes.Status200OK }; private static readonly FrozenSet _disallowedHeaderParameters = new[] { HeaderNames.Accept, HeaderNames.Authorization, HeaderNames.ContentType }.ToFrozenSet(StringComparer.OrdinalIgnoreCase); diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs index 8115e779b123..304aacdfa98d 100644 --- a/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs +++ b/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.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.Collections.Concurrent; using System.IO.Pipelines; using System.Text.Json.Nodes; using Microsoft.AspNetCore.Http; @@ -14,7 +15,7 @@ namespace Microsoft.AspNetCore.OpenApi; /// internal sealed class OpenApiSchemaStore { - private readonly Dictionary _schemas = new() + private readonly ConcurrentDictionary _schemas = new() { // Pre-populate OpenAPI schemas for well-defined types in ASP.NET Core. [new OpenApiSchemaKey(typeof(IFormFile), null)] = new JsonObject @@ -48,8 +49,8 @@ internal sealed class OpenApiSchemaStore }, }; - public readonly Dictionary SchemasByReference = new(OpenApiSchemaComparer.Instance); - private readonly Dictionary _referenceIdCounter = new(); + public readonly ConcurrentDictionary SchemasByReference = new(OpenApiSchemaComparer.Instance); + private readonly ConcurrentDictionary _referenceIdCounter = new(); /// /// Resolves the JSON schema for the given type and parameter description. @@ -59,13 +60,7 @@ internal sealed class OpenApiSchemaStore /// A representing the JSON schema associated with the key. public JsonNode GetOrAdd(OpenApiSchemaKey key, Func valueFactory) { - if (_schemas.TryGetValue(key, out var schema)) - { - return schema; - } - var targetSchema = valueFactory(key); - _schemas.Add(key, targetSchema); - return targetSchema; + return _schemas.GetOrAdd(key, valueFactory); } /// diff --git a/src/OpenApi/src/Transformers/Implementations/OpenApiSchemaReferenceTransformer.cs b/src/OpenApi/src/Transformers/Implementations/OpenApiSchemaReferenceTransformer.cs index caa469a38598..ce95db0f9db4 100644 --- a/src/OpenApi/src/Transformers/Implementations/OpenApiSchemaReferenceTransformer.cs +++ b/src/OpenApi/src/Transformers/Implementations/OpenApiSchemaReferenceTransformer.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.Collections.Concurrent; using System.Linq; using Microsoft.Extensions.DependencyInjection; using Microsoft.OpenApi.Models; @@ -85,7 +86,7 @@ public Task TransformAsync(OpenApiDocument document, OpenApiDocumentTransformerC /// The inline schema to replace with a reference. /// A cache of schemas and their associated reference IDs. /// When , will skip resolving references for the top-most schema provided. - internal static OpenApiSchema? ResolveReferenceForSchema(OpenApiSchema? schema, Dictionary schemasByReference, bool isTopLevel = false) + internal static OpenApiSchema? ResolveReferenceForSchema(OpenApiSchema? schema, ConcurrentDictionary schemasByReference, bool isTopLevel = false) { if (schema is null) { diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/OpenApiDocumentConcurrentRequestTests.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/OpenApiDocumentConcurrentRequestTests.cs new file mode 100644 index 000000000000..3f2ce1177aa3 --- /dev/null +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/OpenApiDocumentConcurrentRequestTests.cs @@ -0,0 +1,26 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Net; +using System.Net.Http; + +namespace Microsoft.AspNetCore.OpenApi.Tests.Integration; + +public class OpenApiDocumentConcurrentRequestTests(SampleAppFixture fixture) : IClassFixture +{ + [Fact] + public async Task MapOpenApi_HandlesConcurrentRequests() + { + // Arrange + var client = fixture.CreateClient(); + + // Act + await Parallel.ForAsync(0, 150, async (_, ctx) => + { + var response = await client.GetAsync("/openapi/v1.json", ctx); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + }); + } +} From f6cac7ea9e8bfadd7265d6f385d23511730bcb9a Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Wed, 25 Sep 2024 10:23:31 -0700 Subject: [PATCH 5/5] Harden self-referencing schema ID check --- .../OpenApiSchemaReferenceTransformer.cs | 2 +- .../OpenApiSchemaReferenceTransformerTests.cs | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/OpenApi/src/Transformers/Implementations/OpenApiSchemaReferenceTransformer.cs b/src/OpenApi/src/Transformers/Implementations/OpenApiSchemaReferenceTransformer.cs index ce95db0f9db4..ee7e166daab7 100644 --- a/src/OpenApi/src/Transformers/Implementations/OpenApiSchemaReferenceTransformer.cs +++ b/src/OpenApi/src/Transformers/Implementations/OpenApiSchemaReferenceTransformer.cs @@ -106,7 +106,7 @@ public Task TransformAsync(OpenApiDocument document, OpenApiDocumentTransformerC // 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 { Id: "#" } + 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() } }; diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs index eecb520c1bb0..4d16ff51d4e7 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs +++ b/src/OpenApi/test/Microsoft.AspNetCore.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); + }); + } }