From 62e16afcfe944e6f714f04a6e99f228e344e8231 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Fri, 28 Jun 2024 09:58:35 -0700 Subject: [PATCH 1/5] Apply schema transformers on properties and other subschemas --- .../Microbenchmarks/TransformersBenchmark.cs | 4 +- src/OpenApi/src/PublicAPI.Unshipped.txt | 6 +- .../Services/Schemas/OpenApiSchemaService.cs | 45 ++- .../OpenApiSchemaTransformerContext.cs | 29 +- ...enApiDocument_documentName=v2.verified.txt | 6 +- .../OpenApiSchemaReferenceTransformerTests.cs | 2 +- .../Transformers/SchemaTransformerTests.cs | 280 +++++++++++++++++- 7 files changed, 348 insertions(+), 24 deletions(-) diff --git a/src/OpenApi/perf/Microbenchmarks/TransformersBenchmark.cs b/src/OpenApi/perf/Microbenchmarks/TransformersBenchmark.cs index 42b64a83fba0..3bb4c73be163 100644 --- a/src/OpenApi/perf/Microbenchmarks/TransformersBenchmark.cs +++ b/src/OpenApi/perf/Microbenchmarks/TransformersBenchmark.cs @@ -95,7 +95,7 @@ public void SchemaTransformer_Setup() { _options.AddSchemaTransformer((schema, context, token) => { - if (context.Type == typeof(Todo) && context.ParameterDescription != null) + if (context.JsonTypeInfo.Type == typeof(Todo) && context.ParameterDescription != null) { schema.Extensions["x-my-extension"] = new OpenApiString(context.ParameterDescription.Name); } @@ -167,7 +167,7 @@ private class SchemaTransformer : IOpenApiSchemaTransformer { public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext context, CancellationToken cancellationToken) { - if (context.Type == typeof(Todo) && context.ParameterDescription != null) + if (context.JsonTypeInfo.Type == typeof(Todo) && context.ParameterDescription != null) { schema.Extensions["x-my-extension"] = new OpenApiString(context.ParameterDescription.Name); } diff --git a/src/OpenApi/src/PublicAPI.Unshipped.txt b/src/OpenApi/src/PublicAPI.Unshipped.txt index 5b9065d333b9..5b3cb33941d0 100644 --- a/src/OpenApi/src/PublicAPI.Unshipped.txt +++ b/src/OpenApi/src/PublicAPI.Unshipped.txt @@ -30,11 +30,13 @@ Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.ApplicationServices Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.ApplicationServices.init -> void Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.DocumentName.get -> string! Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.DocumentName.init -> void +Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.JsonPropertyInfo.get -> System.Text.Json.Serialization.Metadata.JsonPropertyInfo? +Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.JsonPropertyInfo.init -> void +Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.JsonTypeInfo.get -> System.Text.Json.Serialization.Metadata.JsonTypeInfo! +Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.JsonTypeInfo.init -> void Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.OpenApiSchemaTransformerContext() -> void Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.ParameterDescription.get -> Microsoft.AspNetCore.Mvc.ApiExplorer.ApiParameterDescription? Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.ParameterDescription.init -> void -Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.Type.get -> System.Type! -Microsoft.AspNetCore.OpenApi.OpenApiSchemaTransformerContext.Type.init -> void Microsoft.Extensions.DependencyInjection.OpenApiServiceCollectionExtensions static Microsoft.AspNetCore.Builder.OpenApiEndpointRouteBuilderExtensions.MapOpenApi(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern = "/openapi/{documentName}.json") -> Microsoft.AspNetCore.Builder.IEndpointConventionBuilder! static Microsoft.AspNetCore.OpenApi.OpenApiOptions.CreateDefaultSchemaReferenceId(System.Text.Json.Serialization.Metadata.JsonTypeInfo! jsonTypeInfo) -> string? diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs index 60751c4d7e04..7df4c6d36773 100644 --- a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs +++ b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs @@ -144,17 +144,58 @@ internal async Task GetOrCreateSchemaAsync(Type type, ApiParamete internal async Task ApplySchemaTransformersAsync(OpenApiSchema schema, Type type, ApiParameterDescription? parameterDescription = null, CancellationToken cancellationToken = default) { + var jsonTypeInfo = _jsonSerializerOptions.GetTypeInfo(type); var context = new OpenApiSchemaTransformerContext { DocumentName = documentName, - Type = type, + JsonTypeInfo = jsonTypeInfo, + JsonPropertyInfo = null, ParameterDescription = parameterDescription, ApplicationServices = serviceProvider }; for (var i = 0; i < _openApiOptions.SchemaTransformers.Count; i++) { var transformer = _openApiOptions.SchemaTransformers[i]; - await transformer.TransformAsync(schema, context, cancellationToken); + await InnerApplySchemaTransformersAsync(schema, jsonTypeInfo, context, transformer, cancellationToken); + } + } + + private async Task InnerApplySchemaTransformersAsync(OpenApiSchema schema, + JsonTypeInfo jsonTypeInfo, + OpenApiSchemaTransformerContext context, + IOpenApiSchemaTransformer transformer, + CancellationToken cancellationToken = default) + { + await transformer.TransformAsync(schema, context, cancellationToken); + + // Only apply transformers on polymorphic schemas where we can resolve the derived + // types associated with the base type. + if (schema.AnyOf is { Count: > 0 } && jsonTypeInfo.PolymorphismOptions is not null) + { + var anyOfIndex = 0; + foreach (var derivedType in jsonTypeInfo.PolymorphismOptions.DerivedTypes) + { + var derivedJsonTypeInfo = _jsonSerializerOptions.GetTypeInfo(derivedType.DerivedType); + context.UpdateJsonTypeInfo(derivedJsonTypeInfo, null); + await InnerApplySchemaTransformersAsync(schema.AnyOf[anyOfIndex], derivedJsonTypeInfo, context, transformer, cancellationToken); + anyOfIndex++; + } + } + + if (schema.Items is not null) + { + var elementTypeInfo = _jsonSerializerOptions.GetTypeInfo(jsonTypeInfo.ElementType!); + context.UpdateJsonTypeInfo(elementTypeInfo, null); + await InnerApplySchemaTransformersAsync(schema.Items, elementTypeInfo, context, transformer, cancellationToken); + } + + if (schema.Properties is { Count: > 0 }) + { + foreach (var propertyInfo in jsonTypeInfo.Properties) + { + context.UpdateJsonTypeInfo(_jsonSerializerOptions.GetTypeInfo(propertyInfo.PropertyType), propertyInfo); + await InnerApplySchemaTransformersAsync(schema.Properties[propertyInfo.Name], jsonTypeInfo, context, transformer, cancellationToken); + } } } diff --git a/src/OpenApi/src/Transformers/OpenApiSchemaTransformerContext.cs b/src/OpenApi/src/Transformers/OpenApiSchemaTransformerContext.cs index b915978ff35b..94a03e679639 100644 --- a/src/OpenApi/src/Transformers/OpenApiSchemaTransformerContext.cs +++ b/src/OpenApi/src/Transformers/OpenApiSchemaTransformerContext.cs @@ -1,8 +1,8 @@ // 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.Metadata; using Microsoft.AspNetCore.Mvc.ApiExplorer; -using Microsoft.OpenApi.Models; namespace Microsoft.AspNetCore.OpenApi; @@ -11,24 +11,41 @@ namespace Microsoft.AspNetCore.OpenApi; /// public sealed class OpenApiSchemaTransformerContext { + private JsonTypeInfo? _jsonTypeInfo; + private JsonPropertyInfo? _jsonPropertyInfo; + /// /// Gets the name of the associated OpenAPI document. /// public required string DocumentName { get; init; } - /// - /// Gets the associated with the current . - /// - public required Type Type { get; init; } - /// /// Gets the associated with the target schema. /// Null when processing an OpenAPI schema for a response type. /// public required ApiParameterDescription? ParameterDescription { get; init; } + /// + /// Gets the associated with the target schema. + /// + public required JsonTypeInfo JsonTypeInfo { get => _jsonTypeInfo!; init => _jsonTypeInfo = value; } + + /// + /// Gets the associated with the target schema if the + /// target schema is a property of a parent schema. + /// + public required JsonPropertyInfo? JsonPropertyInfo { get => _jsonPropertyInfo; init => _jsonPropertyInfo = value; } + /// /// Gets the application services associated with the current document the target schema is in. /// public required IServiceProvider ApplicationServices { get; init; } + + // Expose internal setters for the properties that only allow initializations to avoid allocating + // new instances of the context for each sub-schema transformation. + internal void UpdateJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonPropertyInfo? jsonPropertyInfo) + { + _jsonTypeInfo = jsonTypeInfo; + _jsonPropertyInfo = jsonPropertyInfo; + } } diff --git a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v2.verified.txt b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v2.verified.txt index c5baf51f8bfe..527dbd0450fa 100644 --- a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v2.verified.txt +++ b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v2.verified.txt @@ -52,7 +52,11 @@ "ArrayOfstring": { "type": "array", "items": { - "type": "string" + "type": "string", + "externalDocs": { + "description": "Documentation for this OpenAPI schema", + "url": "https://example.com/api/docs/schemas/string" + } }, "externalDocs": { "description": "Documentation for this OpenAPI schema", diff --git a/src/OpenApi/test/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs b/src/OpenApi/test/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs index 7552c15a0777..e71338a24b4a 100644 --- a/src/OpenApi/test/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs +++ b/src/OpenApi/test/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs @@ -269,7 +269,7 @@ public async Task TypeModifiedWithSchemaTransformerMapsToDifferentReferenceId() var options = new OpenApiOptions(); options.AddSchemaTransformer((schema, context, cancellationToken) => { - if (context.Type == typeof(Todo) && context.ParameterDescription is not null) + if (context.JsonTypeInfo.Type == typeof(Todo) && context.ParameterDescription is not null) { schema.Extensions["x-my-extension"] = new OpenApiString(context.ParameterDescription.Name); } diff --git a/src/OpenApi/test/Transformers/SchemaTransformerTests.cs b/src/OpenApi/test/Transformers/SchemaTransformerTests.cs index ba9f8692743c..ebac8b224687 100644 --- a/src/OpenApi/test/Transformers/SchemaTransformerTests.cs +++ b/src/OpenApi/test/Transformers/SchemaTransformerTests.cs @@ -3,10 +3,12 @@ using System.Globalization; using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.OpenApi; using Microsoft.Extensions.DependencyInjection; using Microsoft.OpenApi.Any; using Microsoft.OpenApi.Models; +using Xunit.Sdk; public class SchemaTransformerTests : OpenApiDocumentServiceTestBase { @@ -20,8 +22,27 @@ public async Task SchemaTransformer_CanAccessTypeAndParameterDescriptionForParam var options = new OpenApiOptions(); options.AddSchemaTransformer((schema, context, cancellationToken) => { - Assert.Equal(typeof(Todo), context.Type); - Assert.Equal("todo", context.ParameterDescription.Name); + if (context.JsonPropertyInfo == null) + { + Assert.Equal(typeof(Todo), context.JsonTypeInfo.Type); + Assert.Equal("todo", context.ParameterDescription.Name); + } + if (context.JsonPropertyInfo?.Name == "id") + { + Assert.Equal(typeof(int), context.JsonTypeInfo.Type); + } + if (context.JsonPropertyInfo?.Name == "name") + { + Assert.Equal(typeof(string), context.JsonTypeInfo.Type); + } + if (context.JsonPropertyInfo?.Name == "isComplete") + { + Assert.Equal(typeof(bool), context.JsonTypeInfo.Type); + } + if (context.JsonPropertyInfo?.Name == "dueDate") + { + Assert.Equal(typeof(DateTime), context.JsonTypeInfo.Type); + } return Task.CompletedTask; }); @@ -38,7 +59,26 @@ public async Task SchemaTransformer_CanAccessTypeForResponse() var options = new OpenApiOptions(); options.AddSchemaTransformer((schema, context, cancellationToken) => { - Assert.Equal(typeof(Todo), context.Type); + if (context.JsonPropertyInfo == null) + { + Assert.Equal(typeof(Todo), context.JsonTypeInfo.Type); + } + if (context.JsonPropertyInfo?.Name == "id") + { + Assert.Equal(typeof(int), context.JsonTypeInfo.Type); + } + if (context.JsonPropertyInfo?.Name == "name") + { + Assert.Equal(typeof(string), context.JsonTypeInfo.Type); + } + if (context.JsonPropertyInfo?.Name == "isComplete") + { + Assert.Equal(typeof(bool), context.JsonTypeInfo.Type); + } + if (context.JsonPropertyInfo?.Name == "dueDate") + { + Assert.Equal(typeof(DateTime), context.JsonTypeInfo.Type); + } Assert.Null(context.ParameterDescription); return Task.CompletedTask; }); @@ -124,7 +164,7 @@ public async Task SchemaTransformer_OnTypeModifiesBothRequestAndResponse() var options = new OpenApiOptions(); options.AddSchemaTransformer((schema, context, cancellationToken) => { - if (context.Type == typeof(Todo)) + if (context.JsonTypeInfo.Type == typeof(Todo)) { schema.Extensions["x-my-extension"] = new OpenApiString("1"); } @@ -154,7 +194,7 @@ public async Task SchemaTransformer_WithDescriptionOnlyModifiesParameter() var options = new OpenApiOptions(); options.AddSchemaTransformer((schema, context, cancellationToken) => { - if (context.Type == typeof(Todo) && context.ParameterDescription is not null) + if (context.JsonTypeInfo.Type == typeof(Todo) && context.ParameterDescription is not null) { schema.Extensions["x-my-extension"] = new OpenApiString(context.ParameterDescription.Name); } @@ -270,7 +310,9 @@ public async Task SchemaTransformer_SupportsActivatedTransformerWithTransientDep options.AddSchemaTransformer(); // Assert that transient dependency is instantiated once for each - // request to the OpenAPI document for each created schema. + // request to the OpenAPI document for each created schema and its + // sub-schemas. In this case, it's instantiated 4 times for each top-level + // schema, 16 times for each property within each schema. var countBefore = Dependency.InstantiationCount; await VerifyOpenApiDocument(builder, options, document => { @@ -293,7 +335,7 @@ await VerifyOpenApiDocument(builder, options, document => Assert.True(responseSchema.Extensions.ContainsKey("x-my-extension")); }); var countAfter = Dependency.InstantiationCount; - Assert.Equal(countBefore + 4, countAfter); + Assert.Equal(countBefore + 20, countAfter); } [Fact] @@ -318,7 +360,9 @@ await VerifyOpenApiDocument(builder, options, document => var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema.GetEffective(document); Assert.Equal("Schema Description", responseSchema.Description); }); - Assert.Equal(2, DisposableTransformer.DisposeCount); + // Assert that the transformer is disposed for each top-level schema + // and the four properties within that schema. + Assert.Equal(10, DisposableTransformer.DisposeCount); } [Fact] @@ -343,14 +387,230 @@ await VerifyOpenApiDocument(builder, options, document => var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema.GetEffective(document); Assert.Equal("Schema Description", responseSchema.Description); }); - Assert.Equal(2, AsyncDisposableTransformer.DisposeCount); + // Assert that the transformer is disposed after the top-level + // schema and each sub-property within the schema. + Assert.Equal(10, AsyncDisposableTransformer.DisposeCount); + } + + [Fact] + public async Task SchemaTransformer_CanModifyAllTypesInADocument() + { + var builder = CreateBuilder(); + + builder.MapPost("/todo", (Todo todo) => { }); + builder.MapGet("/todo", (int id) => { }); + + var options = new OpenApiOptions(); + options.AddSchemaTransformer((schema, context, cancellationToken) => + { + if (context.JsonTypeInfo.Type == typeof(int)) + { + schema.Format = "modified-number-format"; + } + return Task.CompletedTask; + }); + + await VerifyOpenApiDocument(builder, options, document => + { + // Assert that parameter schema has been update + var path = Assert.Single(document.Paths.Values); + var getOperation = path.Operations[OperationType.Get]; + var responseSchema = getOperation.Parameters[0].Schema; + Assert.Equal("modified-number-format", responseSchema.Format); + + // Assert that property in request body schema has been updated + var postOperation = path.Operations[OperationType.Post]; + var requestSchema = postOperation.RequestBody.Content["application/json"].Schema; + Assert.Equal("modified-number-format", requestSchema.GetEffective(document).Properties["id"].Format); + }); + } + + [Fact] + public async Task SchemaTransformer_CanModifyItemTypesInADocument() + { + var builder = CreateBuilder(); + + builder.MapGet("/list", () => new List { 1, 2, 3, 4 }); + builder.MapGet("/single", () => 1); + + var options = new OpenApiOptions(); + options.AddSchemaTransformer((schema, context, cancellationToken) => + { + if (context.JsonTypeInfo.Type == typeof(int)) + { + schema.Format = "modified-number-format"; + } + schema = new OpenApiSchema { Type = "array", Items = schema }; + return Task.CompletedTask; + }); + + await VerifyOpenApiDocument(builder, options, document => + { + // Assert that the schema represent list elements has been modified + var path = document.Paths["/list"]; + var getOperation = path.Operations[OperationType.Get]; + var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema.GetEffective(document); + Assert.Equal("modified-number-format", responseSchema.Items.Format); + + // Assert that top-level schema associated with the standalone integer has been updated + path = document.Paths["/single"]; + getOperation = path.Operations[OperationType.Get]; + responseSchema = getOperation.Responses["200"].Content["application/json"].Schema.GetEffective(document); + Assert.Equal("modified-number-format", responseSchema.Format); + }); + } + + [Fact] + public async Task SchemaTransformer_CanModifyPolymorphicChildSchemas() + { + var builder = CreateBuilder(); + + builder.MapPost("/shape", (Shape todo) => { }); + builder.MapPost("/triangle", (Triangle todo) => { }); + + var options = new OpenApiOptions(); + options.AddSchemaTransformer((schema, context, cancellationToken) => + { + if (context.JsonTypeInfo.Type == typeof(Triangle)) + { + schema.Extensions["x-my-extension"] = new OpenApiString("this-is-a-triangle"); + } + return Task.CompletedTask; + }); + + await VerifyOpenApiDocument(builder, options, document => + { + // Assert that the polymorphic sub-type `Triangle` has been updated + var path = document.Paths["/shape"]; + var postOperation = path.Operations[OperationType.Post]; + var requestSchema = postOperation.RequestBody.Content["application/json"].Schema.GetEffective(document); + var triangleSubschema = Assert.Single(requestSchema.AnyOf.Where(s => s.Reference.Id == "ShapeTriangle")); + Assert.True(triangleSubschema.GetEffective(document).Extensions.TryGetValue("x-my-extension", out var _)); + + // Assert that the standalone `Triangle` type has been updated + path = document.Paths["/triangle"]; + postOperation = path.Operations[OperationType.Post]; + requestSchema = postOperation.RequestBody.Content["application/json"].Schema.GetEffective(document); + Assert.Equal("this-is-a-triangle", ((OpenApiString)requestSchema.Extensions["x-my-extension"]).Value); + }); + } + + [Fact] + public async Task SchemaTransformer_CanModifyPropertiesInAnItemsType() + { + var builder = CreateBuilder(); + + builder.MapGet("/list-of-todo", () => new List { new Todo(1, "Item1", false, DateTime.Now) }); + builder.MapGet("/list-of-int", () => new List { 1, 2, 3, 4 }); + + var options = new OpenApiOptions(); + options.AddSchemaTransformer((schema, context, cancellationToken) => + { + if (context.JsonTypeInfo.Type == typeof(int)) + { + schema.Format = "modified-number-format"; + } + return Task.CompletedTask; + }); + + await VerifyOpenApiDocument(builder, options, document => + { + // Assert that the `id` property in the `Todo` within the array has been updated + var path = document.Paths["/list-of-todo"]; + var getOperation = path.Operations[OperationType.Get]; + var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema; + var itemSchema = responseSchema.GetEffective(document).Items; + Assert.Equal("modified-number-format", itemSchema.Properties["id"].Format); + + // Assert that the integer type within the list has been updated + var otherPath = document.Paths["/list-of-int"]; + var otherGetOperation = otherPath.Operations[OperationType.Get]; + var otherResponseSchema = otherGetOperation.Responses["200"].Content["application/json"].Schema; + var otherItemSchema = otherResponseSchema.GetEffective(document).Items; + Assert.Equal("modified-number-format", otherItemSchema.Format); + }); + } + + [Fact(Skip = "Depends on https://github.com/dotnet/runtime/issues/104046")] + public async Task SchemaTransformer_CanModifyListOfPolymorphicTypes() + { + var builder = CreateBuilder(); + + builder.MapGet("/list", () => new List { new Triangle { Hypotenuse = 12, Color = "blue", Sides = 3 }, new Square { Area = 24, Color = "red ", Sides = 4 } }); + + var options = new OpenApiOptions(); + options.AddSchemaTransformer((schema, context, cancellationToken) => + { + if (context.JsonTypeInfo.Type == typeof(Triangle)) + { + schema.Extensions["x-my-extension"] = new OpenApiString("this-is-a-triangle"); + } + if (context.JsonTypeInfo.Type == typeof(Square)) + { + schema.Extensions["x-my-extension"] = new OpenApiString("this-is-a-square"); + } + return Task.CompletedTask; + }); + + await VerifyOpenApiDocument(builder, options, document => + { + // Assert that the `Triangle` type within the list has been updated + var path = document.Paths["/list"]; + var getOperation = path.Operations[OperationType.Get]; + var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema; + var itemSchema = responseSchema.GetEffective(document).Items.GetEffective(document); + var triangleSubschema = Assert.Single(itemSchema.AnyOf.Where(s => s.Reference.Id == "ShapeTriangle")); + // Assert that the x-my-extension type is set to this-is-a-triangle + Assert.True(triangleSubschema.GetEffective(document).Extensions.TryGetValue("x-my-extension", out var triangleExtension)); + Assert.Equal("this-is-a-triangle", ((OpenApiString)triangleExtension).Value); + + // Assert that the `Square` type within the polymorphic type list has been updated + var squareSubschema = Assert.Single(itemSchema.AnyOf.Where(s => s.Reference.Id == "ShapeSquare")); + // Assert that the x-my-extension type is set to this-is-a-square + Assert.True(squareSubschema.GetEffective(document).Extensions.TryGetValue("x-my-extension", out var squareExtension)); + Assert.Equal("this-is-a-square", ((OpenApiString)squareExtension).Value); + }); + } + + [Fact] + public async Task SchemaTransformers_CanModifyMultipleFormParameters() + { + var builder = CreateBuilder(); + + builder.MapPost("/todo", ([FromForm] Todo todo, [FromForm] Error error) => { }); + + var options = new OpenApiOptions(); + options.AddSchemaTransformer((schema, context, cancellationToken) => + { + if (context.JsonTypeInfo.Type == typeof(int)) + { + schema.Format = "modified-number-format"; + } + return Task.CompletedTask; + }); + + // We use `allOf` for multiple form parameters to ensure that they should be aggregated + // appropriately in the request body schema. Although we don't handle `AllOf` when we apply + // schema transformers, these modifications still work because the wrapping of these schemas into + // allOf definitions happens after all transformers have been applied. + await VerifyOpenApiDocument(builder, options, document => + { + var path = document.Paths["/todo"]; + var postOperation = path.Operations[OperationType.Post]; + var requestSchema = postOperation.RequestBody.Content["application/x-www-form-urlencoded"].Schema; + Assert.Equal(2, requestSchema.AllOf.Count); + var todoSchema = requestSchema.AllOf[0]; + var errorSchema = requestSchema.AllOf[1]; + Assert.Equal("modified-number-format", todoSchema.Properties["id"].Format); + Assert.Equal("modified-number-format", errorSchema.Properties["code"].Format); + }); } private class ActivatedTransformer : IOpenApiSchemaTransformer { public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext context, CancellationToken cancellationToken) { - if (context.Type == typeof(Todo)) + if (context.JsonTypeInfo.Type == typeof(Todo)) { schema.Extensions["x-my-extension"] = new OpenApiString("1"); } From c525ccd377e7f9d77923d23699ed79e80fc27e31 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Wed, 10 Jul 2024 12:48:27 -0700 Subject: [PATCH 2/5] Add test for tranformers on unsupported subschemas --- .../Transformers/SchemaTransformerTests.cs | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/src/OpenApi/test/Transformers/SchemaTransformerTests.cs b/src/OpenApi/test/Transformers/SchemaTransformerTests.cs index ebac8b224687..935d801555d9 100644 --- a/src/OpenApi/test/Transformers/SchemaTransformerTests.cs +++ b/src/OpenApi/test/Transformers/SchemaTransformerTests.cs @@ -8,7 +8,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.OpenApi.Any; using Microsoft.OpenApi.Models; -using Xunit.Sdk; public class SchemaTransformerTests : OpenApiDocumentServiceTestBase { @@ -606,6 +605,61 @@ await VerifyOpenApiDocument(builder, options, document => }); } + [Fact] + public async Task SchemaTransformers_CanImplementNotSchemaIndependently() + { + var builder = CreateBuilder(); + + builder.MapGet("/todo", () => new Todo(1, "Item1", false, DateTime.Now)); + builder.MapPost("/shape", (Shape shape) => { }); + + var options = new OpenApiOptions(); + options.AddSchemaTransformer((schema, context, cancellationToken) => + { + if (context.JsonTypeInfo.Type == typeof(Todo)) + { + schema.Not = new OpenApiSchema { Type = "string" }; + } + if (context.JsonTypeInfo.Type == typeof(Triangle)) + { + schema.Not = new OpenApiSchema { Type = "string" }; + } + return Task.CompletedTask; + }); + UseNotSchemaTransformer(options, (schema, context, cancellationToken) => + { + schema.Extensions["modified-by-not-schema-transformer"] = new OpenApiBoolean(true); + return Task.CompletedTask; + }); + + // Assert that not schemas have been modified for both `Todo` and `Triangle` types. + await VerifyOpenApiDocument(builder, options, document => + { + var path = document.Paths["/todo"]; + var getOperation = path.Operations[OperationType.Get]; + var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema.GetEffective(document); + Assert.True(((OpenApiBoolean)responseSchema.Not.Extensions["modified-by-not-schema-transformer"]).Value); + + var shapePath = document.Paths["/shape"]; + var shapeOperation = shapePath.Operations[OperationType.Post]; + var shapeRequestSchema = shapeOperation.RequestBody.Content["application/json"].Schema.GetEffective(document); + var triangleSchema = Assert.Single(shapeRequestSchema.AnyOf.Where(s => s.Reference.Id == "ShapeTriangle")).GetEffective(document); + Assert.True(((OpenApiBoolean)triangleSchema.Not.Extensions["modified-by-not-schema-transformer"]).Value); + }); + + static void UseNotSchemaTransformer(OpenApiOptions options, Func func) + { + options.AddSchemaTransformer(async (schema, context, cancellationToken) => + { + if (schema.Not != null) + { + await func(schema.Not, context, cancellationToken); + } + return; + }); + } + } + private class ActivatedTransformer : IOpenApiSchemaTransformer { public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext context, CancellationToken cancellationToken) From 518048752cb71292210a3444108379b93ff0125a Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 16 Jul 2024 13:56:58 -0700 Subject: [PATCH 3/5] Avoid aggressive disposable for type-based schema transformers --- .../Services/Schemas/OpenApiSchemaService.cs | 20 ++++++++++- .../TypeBasedOpenApiSchemaTransformer.cs | 33 +++++++++++-------- .../Transformers/SchemaTransformerTests.cs | 12 +++---- 3 files changed, 43 insertions(+), 22 deletions(-) diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs index 7df4c6d36773..73ab17d5c359 100644 --- a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs +++ b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs @@ -156,7 +156,25 @@ internal async Task ApplySchemaTransformersAsync(OpenApiSchema schema, Type type for (var i = 0; i < _openApiOptions.SchemaTransformers.Count; i++) { var transformer = _openApiOptions.SchemaTransformers[i]; - await InnerApplySchemaTransformersAsync(schema, jsonTypeInfo, context, transformer, cancellationToken); + // If the transformer is a type-based transformer, we need to initialize and finalize it + // once in the context of the top-level assembly and not the child properties we are invoking + // it on. + if (transformer is TypeBasedOpenApiSchemaTransformer typeBasedTransformer) + { + typeBasedTransformer.InitializeTransformer(serviceProvider); + try + { + await InnerApplySchemaTransformersAsync(schema, jsonTypeInfo, context, typeBasedTransformer, cancellationToken); + } + finally + { + await typeBasedTransformer.FinalizeTransformer(); + } + } + else + { + await InnerApplySchemaTransformersAsync(schema, jsonTypeInfo, context, transformer, cancellationToken); + } } } diff --git a/src/OpenApi/src/Transformers/TypeBasedOpenApiSchemaTransformer.cs b/src/OpenApi/src/Transformers/TypeBasedOpenApiSchemaTransformer.cs index 5aaaf93941bf..7253498c3ebe 100644 --- a/src/OpenApi/src/Transformers/TypeBasedOpenApiSchemaTransformer.cs +++ b/src/OpenApi/src/Transformers/TypeBasedOpenApiSchemaTransformer.cs @@ -13,6 +13,7 @@ internal sealed class TypeBasedOpenApiSchemaTransformer : IOpenApiSchemaTransfor [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] private readonly Type _transformerType; private readonly ObjectFactory _transformerFactory; + private IOpenApiSchemaTransformer? _transformer; internal TypeBasedOpenApiSchemaTransformer([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type transformerType) { @@ -20,24 +21,28 @@ internal TypeBasedOpenApiSchemaTransformer([DynamicallyAccessedMembers(Dynamical _transformerFactory = ActivatorUtilities.CreateFactory(_transformerType, []); } - public async Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext context, CancellationToken cancellationToken) + internal void InitializeTransformer(IServiceProvider serviceProvider) + { + _transformer = _transformerFactory.Invoke(serviceProvider, []) as IOpenApiSchemaTransformer; + + } + + internal async Task FinalizeTransformer() { - var transformer = _transformerFactory.Invoke(context.ApplicationServices, []) as IOpenApiSchemaTransformer; - Debug.Assert(transformer != null, $"The type {_transformerType} does not implement {nameof(IOpenApiSchemaTransformer)}."); - try + if (_transformer is IAsyncDisposable asyncDisposable) { - await transformer.TransformAsync(schema, context, cancellationToken); + await asyncDisposable.DisposeAsync(); } - finally + else if (_transformer is IDisposable disposable) { - if (transformer is IAsyncDisposable asyncDisposable) - { - await asyncDisposable.DisposeAsync(); - } - else if (transformer is IDisposable disposable) - { - disposable.Dispose(); - } + disposable.Dispose(); } } + + public async Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext context, CancellationToken cancellationToken) + { + Debug.Assert(_transformer != null, $"The type {_transformerType} does not implement {nameof(IOpenApiSchemaTransformer)}."); + await _transformer.TransformAsync(schema, context, cancellationToken); + + } } diff --git a/src/OpenApi/test/Transformers/SchemaTransformerTests.cs b/src/OpenApi/test/Transformers/SchemaTransformerTests.cs index 935d801555d9..ece10d596de6 100644 --- a/src/OpenApi/test/Transformers/SchemaTransformerTests.cs +++ b/src/OpenApi/test/Transformers/SchemaTransformerTests.cs @@ -334,7 +334,7 @@ await VerifyOpenApiDocument(builder, options, document => Assert.True(responseSchema.Extensions.ContainsKey("x-my-extension")); }); var countAfter = Dependency.InstantiationCount; - Assert.Equal(countBefore + 20, countAfter); + Assert.Equal(countBefore + 4, countAfter); } [Fact] @@ -359,9 +359,8 @@ await VerifyOpenApiDocument(builder, options, document => var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema.GetEffective(document); Assert.Equal("Schema Description", responseSchema.Description); }); - // Assert that the transformer is disposed for each top-level schema - // and the four properties within that schema. - Assert.Equal(10, DisposableTransformer.DisposeCount); + // Assert that the transformer is disposed twice for each top-level schema. + Assert.Equal(2, DisposableTransformer.DisposeCount); } [Fact] @@ -386,9 +385,8 @@ await VerifyOpenApiDocument(builder, options, document => var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema.GetEffective(document); Assert.Equal("Schema Description", responseSchema.Description); }); - // Assert that the transformer is disposed after the top-level - // schema and each sub-property within the schema. - Assert.Equal(10, AsyncDisposableTransformer.DisposeCount); + // Assert that the transformer is disposed twice for each top-level schema. + Assert.Equal(2, AsyncDisposableTransformer.DisposeCount); } [Fact] From ac380b648bdbf73f07afd504dbfda4212b5d44bc Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 16 Jul 2024 14:10:23 -0700 Subject: [PATCH 4/5] Add some guards around recursion into properties and anyOf --- src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs index 73ab17d5c359..52b3bce62f8c 100644 --- a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs +++ b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs @@ -195,6 +195,10 @@ private async Task InnerApplySchemaTransformersAsync(OpenApiSchema schema, { var derivedJsonTypeInfo = _jsonSerializerOptions.GetTypeInfo(derivedType.DerivedType); context.UpdateJsonTypeInfo(derivedJsonTypeInfo, null); + if (schema.AnyOf.Count <= anyOfIndex) + { + break; + } await InnerApplySchemaTransformersAsync(schema.AnyOf[anyOfIndex], derivedJsonTypeInfo, context, transformer, cancellationToken); anyOfIndex++; } @@ -212,7 +216,10 @@ private async Task InnerApplySchemaTransformersAsync(OpenApiSchema schema, foreach (var propertyInfo in jsonTypeInfo.Properties) { context.UpdateJsonTypeInfo(_jsonSerializerOptions.GetTypeInfo(propertyInfo.PropertyType), propertyInfo); - await InnerApplySchemaTransformersAsync(schema.Properties[propertyInfo.Name], jsonTypeInfo, context, transformer, cancellationToken); + if (schema.Properties.TryGetValue(propertyInfo.Name, out var propertySchema)) + { + await InnerApplySchemaTransformersAsync(propertySchema, _jsonSerializerOptions.GetTypeInfo(propertyInfo.PropertyType), context, transformer, cancellationToken); + } } } } From 96531784bba9bef958619467413998267bd16cad Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 16 Jul 2024 16:02:51 -0700 Subject: [PATCH 5/5] Manage activated IOpenApiSchemaTransformer in OpenApiSchemaService --- .../Services/Schemas/OpenApiSchemaService.cs | 6 ++--- .../TypeBasedOpenApiSchemaTransformer.cs | 23 ++++++++----------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs index 52b3bce62f8c..22a6691b3b04 100644 --- a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs +++ b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs @@ -161,14 +161,14 @@ internal async Task ApplySchemaTransformersAsync(OpenApiSchema schema, Type type // it on. if (transformer is TypeBasedOpenApiSchemaTransformer typeBasedTransformer) { - typeBasedTransformer.InitializeTransformer(serviceProvider); + var initializedTransformer = typeBasedTransformer.InitializeTransformer(serviceProvider); try { - await InnerApplySchemaTransformersAsync(schema, jsonTypeInfo, context, typeBasedTransformer, cancellationToken); + await InnerApplySchemaTransformersAsync(schema, jsonTypeInfo, context, initializedTransformer, cancellationToken); } finally { - await typeBasedTransformer.FinalizeTransformer(); + await TypeBasedOpenApiSchemaTransformer.FinalizeTransformer(initializedTransformer); } } else diff --git a/src/OpenApi/src/Transformers/TypeBasedOpenApiSchemaTransformer.cs b/src/OpenApi/src/Transformers/TypeBasedOpenApiSchemaTransformer.cs index 7253498c3ebe..202e7847fa7a 100644 --- a/src/OpenApi/src/Transformers/TypeBasedOpenApiSchemaTransformer.cs +++ b/src/OpenApi/src/Transformers/TypeBasedOpenApiSchemaTransformer.cs @@ -13,7 +13,6 @@ internal sealed class TypeBasedOpenApiSchemaTransformer : IOpenApiSchemaTransfor [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] private readonly Type _transformerType; private readonly ObjectFactory _transformerFactory; - private IOpenApiSchemaTransformer? _transformer; internal TypeBasedOpenApiSchemaTransformer([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type transformerType) { @@ -21,28 +20,26 @@ internal TypeBasedOpenApiSchemaTransformer([DynamicallyAccessedMembers(Dynamical _transformerFactory = ActivatorUtilities.CreateFactory(_transformerType, []); } - internal void InitializeTransformer(IServiceProvider serviceProvider) + internal IOpenApiSchemaTransformer InitializeTransformer(IServiceProvider serviceProvider) { - _transformer = _transformerFactory.Invoke(serviceProvider, []) as IOpenApiSchemaTransformer; - + var transformer = _transformerFactory.Invoke(serviceProvider, []) as IOpenApiSchemaTransformer; + Debug.Assert(transformer != null, $"The type {_transformerType} does not implement {nameof(IOpenApiSchemaTransformer)}."); + return transformer; } - internal async Task FinalizeTransformer() + internal static async Task FinalizeTransformer(IOpenApiSchemaTransformer transformer) { - if (_transformer is IAsyncDisposable asyncDisposable) + if (transformer is IAsyncDisposable asyncDisposable) { await asyncDisposable.DisposeAsync(); } - else if (_transformer is IDisposable disposable) + else if (transformer is IDisposable disposable) { disposable.Dispose(); } } - public async Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext context, CancellationToken cancellationToken) - { - Debug.Assert(_transformer != null, $"The type {_transformerType} does not implement {nameof(IOpenApiSchemaTransformer)}."); - await _transformer.TransformAsync(schema, context, cancellationToken); - - } + // No-op because the activate instance is invoked by the OpenApiSchema service. + public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext context, CancellationToken cancellationToken) + => Task.CompletedTask; }