From 5f9c92a05b678e241859b3df8c2f0918c248c7f8 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Thu, 27 Jun 2024 20:40:46 -0700 Subject: [PATCH 1/2] Always ref request body and response schemas and fix nested types --- .../src/Extensions/JsonTypeInfoExtensions.cs | 3 +- .../src/Schemas/OpenApiJsonSchema.Helpers.cs | 4 + .../src/Services/OpenApiDocumentService.cs | 4 +- .../Services/Schemas/OpenApiSchemaService.cs | 12 +- .../Services/Schemas/OpenApiSchemaStore.cs | 11 +- ...cument_documentName=responses.verified.txt | 72 +++++----- ...t_documentName=schemas-by-ref.verified.txt | 131 ++++++++++-------- ...enApiDocument_documentName=v1.verified.txt | 108 ++++++++------- ...enApiDocument_documentName=v2.verified.txt | 16 ++- .../OpenApiDocumentServiceTests.Responses.cs | 7 +- ...OpenApiSchemaService.PolymorphicSchemas.cs | 8 +- ...OpenApiSchemaService.RequestBodySchemas.cs | 76 +++++++--- .../OpenApiSchemaService.ResponseSchemas.cs | 75 +++++----- .../OpenApiSchemaReferenceTransformerTests.cs | 14 +- .../Transformers/SchemaTransformerTests.cs | 6 +- 15 files changed, 320 insertions(+), 227 deletions(-) diff --git a/src/OpenApi/src/Extensions/JsonTypeInfoExtensions.cs b/src/OpenApi/src/Extensions/JsonTypeInfoExtensions.cs index ac1a87c129cc..acb82909000b 100644 --- a/src/OpenApi/src/Extensions/JsonTypeInfoExtensions.cs +++ b/src/OpenApi/src/Extensions/JsonTypeInfoExtensions.cs @@ -53,7 +53,8 @@ internal static class JsonTypeInfoExtensions internal static string? GetSchemaReferenceId(this JsonTypeInfo jsonTypeInfo, bool isTopLevel = true) { var type = jsonTypeInfo.Type; - if (isTopLevel && OpenApiConstants.PrimitiveTypes.Contains(type)) + var underlyingType = Nullable.GetUnderlyingType(type); + if (isTopLevel && OpenApiConstants.PrimitiveTypes.Contains(underlyingType ?? type)) { return null; } diff --git a/src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs b/src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs index 26bb74e2c8cc..d069ce9836c4 100644 --- a/src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs +++ b/src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs @@ -313,6 +313,10 @@ public static void ReadProperty(ref Utf8JsonReader reader, string propertyName, schema.Enum = [ReadOpenApiAny(ref reader, out var constType)]; schema.Type = constType; break; + case OpenApiSchemaKeywords.RefKeyword: + reader.Read(); + schema.Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = reader.GetString() }; + break; default: reader.Skip(); break; diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs index 34987212ab95..0031c5ca2162 100644 --- a/src/OpenApi/src/Services/OpenApiDocumentService.cs +++ b/src/OpenApi/src/Services/OpenApiDocumentService.cs @@ -228,7 +228,7 @@ private async Task GetResponseAsync(ApiDescription apiDescripti .Select(responseFormat => responseFormat.MediaType); foreach (var contentType in apiResponseFormatContentTypes) { - var schema = apiResponseType.Type is { } type ? await _componentService.GetOrCreateSchemaAsync(type, null, cancellationToken) : new OpenApiSchema(); + var schema = apiResponseType.Type is { } type ? await _componentService.GetOrCreateSchemaAsync(type, null, cancellationToken, captureSchemaByRef: true) : new OpenApiSchema(); response.Content[contentType] = new OpenApiMediaType { Schema = schema }; } @@ -465,7 +465,7 @@ private async Task GetJsonRequestBody(IList() is { } validationAttributes) { @@ -112,7 +120,7 @@ internal sealed class OpenApiSchemaService( } }; - internal async Task GetOrCreateSchemaAsync(Type type, ApiParameterDescription? parameterDescription = null, CancellationToken cancellationToken = default) + internal async Task GetOrCreateSchemaAsync(Type type, ApiParameterDescription? parameterDescription = null, CancellationToken cancellationToken = default, bool captureSchemaByRef = false) { var key = parameterDescription?.ParameterDescriptor is IParameterInfoParameterDescriptor parameterInfoDescription && parameterDescription.ModelMetadata.PropertyName is null @@ -126,7 +134,7 @@ internal async Task GetOrCreateSchemaAsync(Type type, ApiParamete Debug.Assert(deserializedSchema != null, "The schema should have been deserialized successfully and materialize a non-null value."); var schema = deserializedSchema.Schema; await ApplySchemaTransformersAsync(schema, type, parameterDescription, cancellationToken); - _schemaStore.PopulateSchemaIntoReferenceCache(schema); + _schemaStore.PopulateSchemaIntoReferenceCache(schema, captureSchemaByRef); return schema; } diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs index 67923622e71d..2937a37ba5c1 100644 --- a/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs +++ b/src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs @@ -79,9 +79,12 @@ public JsonNode GetOrAdd(OpenApiSchemaKey key, Func /// schemas into the top-level document. /// /// The to add to the schemas-with-references cache. - public void PopulateSchemaIntoReferenceCache(OpenApiSchema schema) + /// if schema should always be referenced instead of inlined. + public void PopulateSchemaIntoReferenceCache(OpenApiSchema schema, bool captureSchemaByRef) { - AddOrUpdateSchemaByReference(schema); + // Only capture top-level schemas by ref. Nested schemas will follow the + // reference by duplicate rules. + AddOrUpdateSchemaByReference(schema, captureSchemaByRef: captureSchemaByRef); if (schema.AdditionalProperties is not null) { AddOrUpdateSchemaByReference(schema.AdditionalProperties); @@ -119,10 +122,10 @@ public void PopulateSchemaIntoReferenceCache(OpenApiSchema schema) } } - private void AddOrUpdateSchemaByReference(OpenApiSchema schema, string? baseTypeSchemaId = null) + private void AddOrUpdateSchemaByReference(OpenApiSchema schema, string? baseTypeSchemaId = null, bool captureSchemaByRef = false) { var targetReferenceId = baseTypeSchemaId is not null ? $"{baseTypeSchemaId}{GetSchemaReferenceId(schema)}" : GetSchemaReferenceId(schema); - if (SchemasByReference.TryGetValue(schema, out var referenceId)) + 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 // ID to avoid name collisions. These collisions are most likely to occur when the same .NET type produces a different diff --git a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=responses.verified.txt b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=responses.verified.txt index 65411d66e17e..12fb88cb35e6 100644 --- a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=responses.verified.txt +++ b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=responses.verified.txt @@ -59,20 +59,7 @@ "content": { "application/json": { "schema": { - "type": "object", - "properties": { - "hypotenuse": { - "type": "number", - "format": "double" - }, - "color": { - "type": "string" - }, - "sides": { - "type": "integer", - "format": "int32" - } - } + "$ref": "#/components/schemas/Triangle" } } } @@ -91,25 +78,7 @@ "content": { "application/json": { "schema": { - "required": [ - "$type" - ], - "type": "object", - "anyOf": [ - { - "$ref": "#/components/schemas/ShapeTriangle" - }, - { - "$ref": "#/components/schemas/ShapeSquare" - } - ], - "discriminator": { - "propertyName": "$type", - "mapping": { - "triangle": "#/components/schemas/ShapeTriangle", - "square": "#/components/schemas/ShapeSquare" - } - } + "$ref": "#/components/schemas/Shape" } } } @@ -120,6 +89,27 @@ }, "components": { "schemas": { + "Shape": { + "required": [ + "$type" + ], + "type": "object", + "anyOf": [ + { + "$ref": "#/components/schemas/ShapeTriangle" + }, + { + "$ref": "#/components/schemas/ShapeSquare" + } + ], + "discriminator": { + "propertyName": "$type", + "mapping": { + "triangle": "#/components/schemas/ShapeTriangle", + "square": "#/components/schemas/ShapeSquare" + } + } + }, "ShapeSquare": { "properties": { "$type": { @@ -186,6 +176,22 @@ "format": "date-time" } } + }, + "Triangle": { + "type": "object", + "properties": { + "hypotenuse": { + "type": "number", + "format": "double" + }, + "color": { + "type": "string" + }, + "sides": { + "type": "integer", + "format": "int32" + } + } } } }, diff --git a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=schemas-by-ref.verified.txt b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=schemas-by-ref.verified.txt index 82a7723f7d1a..13d82fc07859 100644 --- a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=schemas-by-ref.verified.txt +++ b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=schemas-by-ref.verified.txt @@ -303,25 +303,7 @@ "content": { "application/json": { "schema": { - "required": [ - "$type" - ], - "type": "object", - "anyOf": [ - { - "$ref": "#/components/schemas/ShapeTriangle" - }, - { - "$ref": "#/components/schemas/ShapeSquare" - } - ], - "discriminator": { - "propertyName": "$type", - "mapping": { - "triangle": "#/components/schemas/ShapeTriangle", - "square": "#/components/schemas/ShapeSquare" - } - } + "$ref": "#/components/schemas/Shape" } } }, @@ -343,29 +325,7 @@ "content": { "application/json": { "schema": { - "required": [ - "$type" - ], - "type": "object", - "anyOf": [ - { - "$ref": "#/components/schemas/WeatherForecastBaseWeatherForecastWithCity" - }, - { - "$ref": "#/components/schemas/WeatherForecastBaseWeatherForecastWithTimeSeries" - }, - { - "$ref": "#/components/schemas/WeatherForecastBaseWeatherForecastWithLocalNews" - } - ], - "discriminator": { - "propertyName": "$type", - "mapping": { - "0": "#/components/schemas/WeatherForecastBaseWeatherForecastWithCity", - "1": "#/components/schemas/WeatherForecastBaseWeatherForecastWithTimeSeries", - "2": "#/components/schemas/WeatherForecastBaseWeatherForecastWithLocalNews" - } - } + "$ref": "#/components/schemas/WeatherForecastBase" } } }, @@ -387,25 +347,7 @@ "content": { "application/json": { "schema": { - "required": [ - "discriminator" - ], - "type": "object", - "anyOf": [ - { - "$ref": "#/components/schemas/PersonStudent" - }, - { - "$ref": "#/components/schemas/PersonTeacher" - } - ], - "discriminator": { - "propertyName": "discriminator", - "mapping": { - "student": "#/components/schemas/PersonStudent", - "teacher": "#/components/schemas/PersonTeacher" - } - } + "$ref": "#/components/schemas/Person" } } }, @@ -447,6 +389,27 @@ "format": "int32" } }, + "Person": { + "required": [ + "discriminator" + ], + "type": "object", + "anyOf": [ + { + "$ref": "#/components/schemas/PersonStudent" + }, + { + "$ref": "#/components/schemas/PersonTeacher" + } + ], + "discriminator": { + "propertyName": "discriminator", + "mapping": { + "student": "#/components/schemas/PersonStudent", + "teacher": "#/components/schemas/PersonTeacher" + } + } + }, "PersonStudent": { "properties": { "discriminator": { @@ -489,6 +452,27 @@ } } }, + "Shape": { + "required": [ + "$type" + ], + "type": "object", + "anyOf": [ + { + "$ref": "#/components/schemas/ShapeTriangle" + }, + { + "$ref": "#/components/schemas/ShapeSquare" + } + ], + "discriminator": { + "propertyName": "$type", + "mapping": { + "triangle": "#/components/schemas/ShapeTriangle", + "square": "#/components/schemas/ShapeSquare" + } + } + }, "ShapeSquare": { "properties": { "$type": { @@ -547,6 +531,31 @@ } } }, + "WeatherForecastBase": { + "required": [ + "$type" + ], + "type": "object", + "anyOf": [ + { + "$ref": "#/components/schemas/WeatherForecastBaseWeatherForecastWithCity" + }, + { + "$ref": "#/components/schemas/WeatherForecastBaseWeatherForecastWithTimeSeries" + }, + { + "$ref": "#/components/schemas/WeatherForecastBaseWeatherForecastWithLocalNews" + } + ], + "discriminator": { + "propertyName": "$type", + "mapping": { + "0": "#/components/schemas/WeatherForecastBaseWeatherForecastWithCity", + "1": "#/components/schemas/WeatherForecastBaseWeatherForecastWithTimeSeries", + "2": "#/components/schemas/WeatherForecastBaseWeatherForecastWithLocalNews" + } + } + }, "WeatherForecastBaseWeatherForecastWithCity": { "required": [ "city" diff --git a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v1.verified.txt b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v1.verified.txt index d8b4654e535a..cb5f0bf47425 100644 --- a/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v1.verified.txt +++ b/src/OpenApi/test/Integration/snapshots/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=v1.verified.txt @@ -62,29 +62,7 @@ "content": { "application/json": { "schema": { - "required": [ - "id", - "title", - "completed", - "createdAt" - ], - "type": "object", - "properties": { - "id": { - "type": "integer", - "format": "int32" - }, - "title": { - "type": "string" - }, - "completed": { - "type": "boolean" - }, - "createdAt": { - "type": "string", - "format": "date-time" - } - } + "$ref": "#/components/schemas/Todo" } } }, @@ -128,34 +106,7 @@ "content": { "application/json": { "schema": { - "required": [ - "dueDate", - "id", - "title", - "completed", - "createdAt" - ], - "type": "object", - "properties": { - "dueDate": { - "type": "string", - "format": "date-time" - }, - "id": { - "type": "integer", - "format": "int32" - }, - "title": { - "type": "string" - }, - "completed": { - "type": "boolean" - }, - "createdAt": { - "type": "string", - "format": "date-time" - } - } + "$ref": "#/components/schemas/TodoWithDueDate" } } } @@ -172,6 +123,61 @@ "type": "string", "format": "uuid" } + }, + "Todo": { + "required": [ + "id", + "title", + "completed", + "createdAt" + ], + "type": "object", + "properties": { + "id": { + "type": "integer", + "format": "int32" + }, + "title": { + "type": "string" + }, + "completed": { + "type": "boolean" + }, + "createdAt": { + "type": "string", + "format": "date-time" + } + } + }, + "TodoWithDueDate": { + "required": [ + "dueDate", + "id", + "title", + "completed", + "createdAt" + ], + "type": "object", + "properties": { + "dueDate": { + "type": "string", + "format": "date-time" + }, + "id": { + "type": "integer", + "format": "int32" + }, + "title": { + "type": "string" + }, + "completed": { + "type": "boolean" + }, + "createdAt": { + "type": "string", + "format": "date-time" + } + } } }, "securitySchemes": { 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 62cb111a0e15..e9d352c92a3d 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 @@ -23,10 +23,7 @@ "content": { "application/json": { "schema": { - "type": "array", - "items": { - "type": "string" - } + "$ref": "#/components/schemas/ArrayOfstring" } } } @@ -45,7 +42,16 @@ } } }, - "components": { }, + "components": { + "schemas": { + "ArrayOfstring": { + "type": "array", + "items": { + "type": "string" + } + } + } + }, "tags": [ { "name": "users" diff --git a/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Responses.cs b/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Responses.cs index af2d994fe972..9c1d5b3c61ab 100644 --- a/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Responses.cs +++ b/src/OpenApi/test/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Responses.cs @@ -231,7 +231,7 @@ await VerifyOpenApiDocument(builder, document => Assert.Empty(response.Value.Description); var mediaTypeEntry = Assert.Single(response.Value.Content); Assert.Equal("application/json", mediaTypeEntry.Key); - var schema = mediaTypeEntry.Value.Schema; + var schema = mediaTypeEntry.Value.Schema.GetEffective(document); Assert.Equal("object", schema.Type); Assert.Collection(schema.Properties, property => { @@ -264,7 +264,8 @@ await VerifyOpenApiDocument(builder, document => Assert.NotNull(defaultResponse); Assert.Empty(defaultResponse.Description); var defaultContent = Assert.Single(defaultResponse.Content.Values); - Assert.Collection(defaultContent.Schema.Properties, + var defaultSchema = defaultContent.Schema.GetEffective(document); + Assert.Collection(defaultSchema.Properties, property => { Assert.Equal("code", property.Key); @@ -281,7 +282,7 @@ await VerifyOpenApiDocument(builder, document => Assert.Equal("OK", okResponse.Description); var okContent = Assert.Single(okResponse.Content); Assert.Equal("application/json", okContent.Key); - var schema = okContent.Value.Schema; + var schema = okContent.Value.Schema.GetEffective(document); Assert.Equal("object", schema.Type); Assert.Collection(schema.Properties, property => { diff --git a/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiSchemaService.PolymorphicSchemas.cs b/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiSchemaService.PolymorphicSchemas.cs index 255134bc86a7..7d84741d7818 100644 --- a/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiSchemaService.PolymorphicSchemas.cs +++ b/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiSchemaService.PolymorphicSchemas.cs @@ -23,7 +23,7 @@ await VerifyOpenApiDocument(builder, document => Assert.NotNull(operation.RequestBody); var requestBody = operation.RequestBody.Content; Assert.True(requestBody.TryGetValue("application/json", out var mediaType)); - var schema = mediaType.Schema; + var schema = mediaType.Schema.GetEffective(document); // Assert discriminator mappings have been configured correctly Assert.Equal("$type", schema.Discriminator.PropertyName); Assert.Contains(schema.Discriminator.PropertyName, schema.Required); @@ -60,7 +60,7 @@ await VerifyOpenApiDocument(builder, document => Assert.NotNull(operation.RequestBody); var requestBody = operation.RequestBody.Content; Assert.True(requestBody.TryGetValue("application/json", out var mediaType)); - var schema = mediaType.Schema; + var schema = mediaType.Schema.GetEffective(document); // Assert discriminator mappings have been configured correctly Assert.Equal("$type", schema.Discriminator.PropertyName); Assert.Contains(schema.Discriminator.PropertyName, schema.Required); @@ -105,7 +105,7 @@ await VerifyOpenApiDocument(builder, document => Assert.NotNull(operation.RequestBody); var requestBody = operation.RequestBody.Content; Assert.True(requestBody.TryGetValue("application/json", out var mediaType)); - var schema = mediaType.Schema; + var schema = mediaType.Schema.GetEffective(document); // Assert discriminator mappings have been configured correctly Assert.Equal("discriminator", schema.Discriminator.PropertyName); Assert.Contains(schema.Discriminator.PropertyName, schema.Required); @@ -144,7 +144,7 @@ await VerifyOpenApiDocument(builder, document => Assert.NotNull(operation.RequestBody); var requestBody = operation.RequestBody.Content; Assert.True(requestBody.TryGetValue("application/json", out var mediaType)); - var schema = mediaType.Schema; + var schema = mediaType.Schema.GetEffective(document); // Assert discriminator mappings have been configured correctly Assert.Equal("$type", schema.Discriminator.PropertyName); Assert.Collection(schema.Discriminator.Mapping, diff --git a/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs b/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs index d5856fed98af..8930d88c0410 100644 --- a/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs +++ b/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs @@ -30,8 +30,9 @@ await VerifyOpenApiDocument(builder, document => var content = Assert.Single(requestBody.Content); Assert.Equal("application/json", content.Key); Assert.NotNull(content.Value.Schema); - Assert.Equal("object", content.Value.Schema.Type); - Assert.Collection(content.Value.Schema.Properties, + var schema = content.Value.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Collection(schema.Properties, property => { Assert.Equal("id", property.Key); @@ -76,8 +77,9 @@ await VerifyOpenApiDocument(builder, document => var content = Assert.Single(requestBody.Content); Assert.Equal("application/json", content.Key); Assert.NotNull(content.Value.Schema); - Assert.Equal("object", content.Value.Schema.Type); - Assert.Collection(content.Value.Schema.Properties, + var effectiveSchema = content.Value.Schema.GetEffective(document); + Assert.Equal("object", effectiveSchema.Type); + Assert.Collection(effectiveSchema.Properties, property => { Assert.Equal("id", property.Key); @@ -148,7 +150,7 @@ 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; + var schema = content.Value.Schema.GetEffective(document); Assert.Collection(schema.Required, property => Assert.Equal("title", property), property => Assert.Equal("completed", property)); @@ -175,7 +177,7 @@ await VerifyOpenApiDocument(builder, document => var operation = document.Paths[$"/{path}"].Operations[OperationType.Post]; var requestBody = operation.RequestBody; - var effectiveSchema = requestBody.Content["application/octet-stream"].Schema; + var effectiveSchema = requestBody.Content["application/octet-stream"].Schema.GetEffective(document); Assert.Equal("string", effectiveSchema.Type); Assert.Equal("binary", effectiveSchema.Format); @@ -198,15 +200,18 @@ await VerifyOpenApiDocument(builder, document => var operation = document.Paths[$"/proposal"].Operations[OperationType.Post]; var requestBody = operation.RequestBody; var schema = requestBody.Content["application/json"].Schema; - Assert.Collection(schema.Properties, + Assert.Equal("Proposal", schema.Reference.Id); + var effectiveSchema = schema.GetEffective(document); + Assert.Collection(effectiveSchema.Properties, property => { Assert.Equal("proposalElement", property.Key); - // Todo: Assert that refs are used correctly. + Assert.Equal("Proposal", property.Value.Reference.Id); }, property => { Assert.Equal("stream", property.Key); - Assert.Equal("string", property.Value.Type); - Assert.Equal("binary", property.Value.Format); + var targetSchema = property.Value.GetEffective(document); + Assert.Equal("string", targetSchema.Type); + Assert.Equal("binary", targetSchema.Format); }); }); } @@ -290,9 +295,10 @@ await VerifyOpenApiDocument(builder, document => Assert.NotNull(operation.RequestBody); var requestBody = operation.RequestBody.Content; Assert.True(requestBody.TryGetValue("application/json", out var mediaType)); - Assert.Equal("object", mediaType.Schema.Type); - Assert.Empty(mediaType.Schema.AnyOf); - Assert.Collection(mediaType.Schema.Properties, + var schema = mediaType.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Empty(schema.AnyOf); + Assert.Collection(schema.Properties, property => { Assert.Equal("length", property.Key); @@ -329,8 +335,9 @@ await VerifyOpenApiDocument(builder, document => Assert.NotNull(operation.RequestBody); var requestBody = operation.RequestBody.Content; Assert.True(requestBody.TryGetValue("application/json", out var mediaType)); - Assert.Equal("object", mediaType.Schema.Type); - Assert.Collection(mediaType.Schema.Properties, + var schema = mediaType.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Collection(schema.Properties, property => { Assert.Equal("id", property.Key); @@ -393,7 +400,7 @@ 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; + var schema = content.Value.Schema.GetEffective(document); Assert.Collection(schema.Properties, property => { @@ -430,6 +437,37 @@ await VerifyOpenApiDocument(builder, document => }); } + [Fact] + public async Task SupportsNestedTypes() + { + // Arrange + var builder = CreateBuilder(); + + // Act + builder.MapPost("/api", (NestedType type) => { }); + + // Assert + await VerifyOpenApiDocument(builder, document => + { + var operation = document.Paths["/api"].Operations[OperationType.Post]; + var requestBody = operation.RequestBody; + var content = Assert.Single(requestBody.Content); + Assert.Equal("NestedType", content.Value.Schema.Reference.Id); + var schema = content.Value.Schema.GetEffective(document); + Assert.Collection(schema.Properties, + property => + { + Assert.Equal("name", property.Key); + Assert.Equal("string", property.Value.Type); + }, + property => + { + Assert.Equal("nested", property.Key); + Assert.Equal("NestedType", property.Value.Reference.Id); + }); + }); + } + private class DescriptionTodo { [Description("The unique identifier for a todo item.")] @@ -455,4 +493,10 @@ private class NullablePropertiesType public Uri? NullableUri { get; set; } } #nullable restore + + private class NestedType + { + public string Name { get; set; } + public NestedType Nested { get; set; } + } } diff --git a/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiSchemaService.ResponseSchemas.cs b/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiSchemaService.ResponseSchemas.cs index c356de229cc9..2850fddb4df0 100644 --- a/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiSchemaService.ResponseSchemas.cs +++ b/src/OpenApi/test/Services/OpenApiSchemaService/OpenApiSchemaService.ResponseSchemas.cs @@ -65,8 +65,9 @@ await VerifyOpenApiDocument(builder, document => var responses = Assert.Single(operation.Responses); var response = responses.Value; Assert.True(response.Content.TryGetValue("application/json", out var mediaType)); - Assert.Equal("object", mediaType.Schema.Type); - Assert.Collection(mediaType.Schema.Properties, + var schema = mediaType.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Collection(schema.Properties, property => { Assert.Equal("id", property.Key); @@ -111,8 +112,9 @@ await VerifyOpenApiDocument(builder, document => var content = Assert.Single(response.Content); Assert.Equal("application/json", content.Key); Assert.NotNull(content.Value.Schema); - Assert.Equal("object", content.Value.Schema.Type); - Assert.Collection(content.Value.Schema.Properties, + var schema = content.Value.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Collection(schema.Properties, property => { Assert.Equal("id", property.Key); @@ -156,8 +158,9 @@ await VerifyOpenApiDocument(builder, document => var responses = Assert.Single(operation.Responses); var response = responses.Value; Assert.True(response.Content.TryGetValue("application/json", out var mediaType)); - Assert.Equal("object", mediaType.Schema.Type); - Assert.Collection(mediaType.Schema.Properties, + var schema = mediaType.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Collection(schema.Properties, property => { Assert.Equal("id", property.Key); @@ -198,7 +201,7 @@ 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; + var schema = content.Value.Schema.GetEffective(document); Assert.Collection(schema.Required, property => Assert.Equal("title", property), property => Assert.Equal("completed", property)); @@ -221,8 +224,9 @@ await VerifyOpenApiDocument(builder, document => var responses = Assert.Single(operation.Responses); var response = responses.Value; Assert.True(response.Content.TryGetValue("application/json", out var mediaType)); - Assert.Equal("object", mediaType.Schema.Type); - Assert.Collection(mediaType.Schema.Properties, + var schema = mediaType.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Collection(schema.Properties, property => { Assert.Equal("dueDate", property.Key); @@ -276,8 +280,9 @@ await VerifyOpenApiDocument(builder, document => var responses = Assert.Single(operation.Responses); var response = responses.Value; Assert.True(response.Content.TryGetValue("application/json", out var mediaType)); - Assert.Equal("object", mediaType.Schema.Type); - Assert.Collection(mediaType.Schema.Properties, + var schema = mediaType.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Collection(schema.Properties, property => { Assert.Equal("isSuccessful", property.Key); @@ -341,9 +346,10 @@ await VerifyOpenApiDocument(builder, document => var responses = Assert.Single(operation.Responses); var response = responses.Value; Assert.True(response.Content.TryGetValue("application/json", out var mediaType)); - Assert.Equal("object", mediaType.Schema.Type); - Assert.Empty(mediaType.Schema.AnyOf); - Assert.Collection(mediaType.Schema.Properties, + var schema = mediaType.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Empty(schema.AnyOf); + Assert.Collection(schema.Properties, property => { Assert.Equal("length", property.Key); @@ -380,8 +386,9 @@ await VerifyOpenApiDocument(builder, document => var responses = Assert.Single(operation.Responses); var response = responses.Value; Assert.True(response.Content.TryGetValue("application/json", out var mediaType)); - Assert.Equal("object", mediaType.Schema.Type); - Assert.Collection(mediaType.Schema.Properties, + var schema = mediaType.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Collection(schema.Properties, property => { Assert.Equal("id", property.Key); @@ -440,10 +447,12 @@ await VerifyOpenApiDocument(builder, document => var responses = Assert.Single(operation.Responses); var response = responses.Value; Assert.True(response.Content.TryGetValue("application/json", out var mediaType)); - Assert.Equal("array", mediaType.Schema.Type); - Assert.NotNull(mediaType.Schema.Items); - Assert.Equal("object", mediaType.Schema.Items.Type); - Assert.Collection(mediaType.Schema.Items.Properties, + var schema = mediaType.Schema.GetEffective(document); + Assert.Equal("array", schema.Type); + Assert.NotNull(schema.Items); + var effectiveItemsSchema = schema.Items.GetEffective(document); + Assert.Equal("object", effectiveItemsSchema.Type); + Assert.Collection(effectiveItemsSchema.Properties, property => { Assert.Equal("id", property.Key); @@ -487,8 +496,9 @@ await VerifyOpenApiDocument(builder, document => var responses = Assert.Single(operation.Responses); var response = responses.Value; Assert.True(response.Content.TryGetValue("application/json", out var mediaType)); - Assert.Equal("object", mediaType.Schema.Type); - Assert.Collection(mediaType.Schema.Properties, + var schema = mediaType.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Collection(schema.Properties, property => { Assert.Equal("pageIndex", property.Key); @@ -565,20 +575,18 @@ await VerifyOpenApiDocument(builder, document => var responses = Assert.Single(operation.Responses); var response = responses.Value; Assert.True(response.Content.TryGetValue("application/problem+json", out var mediaType)); - Assert.Equal("object", mediaType.Schema.Type); - // `string` schemas appear multiple times in this document so they should - // all resolve to reference IDs, hence the use of `GetEffective` to resolve the - // final schema. - Assert.Collection(mediaType.Schema.Properties, + var schema = mediaType.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Collection(schema.Properties, property => { Assert.Equal("type", property.Key); - Assert.Equal("string", property.Value.GetEffective(document).Type); + Assert.Equal("string", property.Value.Type); }, property => { Assert.Equal("title", property.Key); - Assert.Equal("string", property.Value.GetEffective(document).Type); + Assert.Equal("string", property.Value.Type); }, property => { @@ -589,12 +597,12 @@ await VerifyOpenApiDocument(builder, document => property => { Assert.Equal("detail", property.Key); - Assert.Equal("string", property.Value.GetEffective(document).Type); + Assert.Equal("string", property.Value.Type); }, property => { Assert.Equal("instance", property.Key); - Assert.Equal("string", property.Value.GetEffective(document).Type); + Assert.Equal("string", property.Value.Type); }, property => { @@ -625,8 +633,9 @@ await VerifyOpenApiDocument(builder, document => var responses = Assert.Single(operation.Responses); var response = responses.Value; Assert.True(response.Content.TryGetValue("application/json", out var mediaType)); - Assert.Equal("object", mediaType.Schema.Type); - Assert.Collection(mediaType.Schema.Properties, + var schema = mediaType.Schema.GetEffective(document); + Assert.Equal("object", schema.Type); + Assert.Collection(schema.Properties, property => { Assert.Equal("object", property.Key); diff --git a/src/OpenApi/test/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs b/src/OpenApi/test/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs index a2c88e306c38..de63d9ed42dd 100644 --- a/src/OpenApi/test/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs +++ b/src/OpenApi/test/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs @@ -130,11 +130,11 @@ await VerifyOpenApiDocument(builder, document => { var operation = document.Paths["/api"].Operations[OperationType.Post]; var requestBody = operation.RequestBody.Content["application/json"]; - var requestBodySchema = requestBody.Schema; + var requestBodySchema = requestBody.Schema.GetEffective(document); var operation2 = document.Paths["/api-2"].Operations[OperationType.Post]; var requestBody2 = operation2.RequestBody.Content["application/json"]; - var requestBodySchema2 = requestBody2.Schema; + var requestBodySchema2 = requestBody2.Schema.GetEffective(document); // { // "type": "array", @@ -281,16 +281,12 @@ await VerifyOpenApiDocument(builder, options, document => var path = Assert.Single(document.Paths.Values); var postOperation = path.Operations[OperationType.Post]; var requestSchema = postOperation.RequestBody.Content["application/json"].Schema; - // Schemas are distinct because of applied transformer so no reference is used. - Assert.Null(requestSchema.Reference); - Assert.Equal("todo", ((OpenApiString)requestSchema.Extensions["x-my-extension"]).Value); var getOperation = path.Operations[OperationType.Get]; var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema; - Assert.False(responseSchema.Extensions.TryGetValue("x-my-extension", out var _)); // Schemas are distinct because of applied transformer so no reference is used. - Assert.Null(responseSchema.Reference); - // No schemas get componentized here - Assert.Empty(document.Components.Schemas); + Assert.NotEqual(requestSchema.Reference.Id, responseSchema.Reference.Id); + Assert.Equal("todo", ((OpenApiString)requestSchema.GetEffective(document).Extensions["x-my-extension"]).Value); + Assert.False(responseSchema.GetEffective(document).Extensions.TryGetValue("x-my-extension", out var _)); }); } } diff --git a/src/OpenApi/test/Transformers/SchemaTransformerTests.cs b/src/OpenApi/test/Transformers/SchemaTransformerTests.cs index 291a331e4fa8..757fd3eb87fa 100644 --- a/src/OpenApi/test/Transformers/SchemaTransformerTests.cs +++ b/src/OpenApi/test/Transformers/SchemaTransformerTests.cs @@ -107,7 +107,7 @@ public async Task SchemaTransformer_RunsInRegisteredOrder() await VerifyOpenApiDocument(builder, options, document => { var operation = Assert.Single(document.Paths.Values).Operations.Values.Single(); - var schema = operation.RequestBody.Content["application/json"].Schema; + var schema = operation.RequestBody.Content["application/json"].Schema.GetEffective(document); Assert.Equal("2", ((OpenApiString)schema.Extensions["x-my-extension"]).Value); }); } @@ -164,10 +164,10 @@ await VerifyOpenApiDocument(builder, options, document => { var path = Assert.Single(document.Paths.Values); var postOperation = path.Operations[OperationType.Post]; - var requestSchema = postOperation.RequestBody.Content["application/json"].Schema; + var requestSchema = postOperation.RequestBody.Content["application/json"].Schema.GetEffective(document); Assert.Equal("todo", ((OpenApiString)requestSchema.Extensions["x-my-extension"]).Value); var getOperation = path.Operations[OperationType.Get]; - var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema; + var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema.GetEffective(document); Assert.False(responseSchema.Extensions.TryGetValue("x-my-extension", out var _)); }); } From 6f335da0bbaeb439b7b6e9889c6358f2b9fca85b Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Fri, 28 Jun 2024 12:27:18 -0700 Subject: [PATCH 2/2] Address feedback --- src/OpenApi/src/Services/OpenApiDocumentService.cs | 12 ++++++------ .../src/Services/Schemas/OpenApiSchemaService.cs | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs index 0031c5ca2162..a6b9db95a407 100644 --- a/src/OpenApi/src/Services/OpenApiDocumentService.cs +++ b/src/OpenApi/src/Services/OpenApiDocumentService.cs @@ -228,7 +228,7 @@ private async Task GetResponseAsync(ApiDescription apiDescripti .Select(responseFormat => responseFormat.MediaType); foreach (var contentType in apiResponseFormatContentTypes) { - var schema = apiResponseType.Type is { } type ? await _componentService.GetOrCreateSchemaAsync(type, null, cancellationToken, captureSchemaByRef: true) : new OpenApiSchema(); + var schema = apiResponseType.Type is { } type ? await _componentService.GetOrCreateSchemaAsync(type, null, captureSchemaByRef: true, cancellationToken) : new OpenApiSchema(); response.Content[contentType] = new OpenApiMediaType { Schema = schema }; } @@ -269,7 +269,7 @@ private async Task GetResponseAsync(ApiDescription apiDescripti _ => throw new InvalidOperationException($"Unsupported parameter source: {parameter.Source.Id}") }, Required = IsRequired(parameter), - Schema = await _componentService.GetOrCreateSchemaAsync(parameter.Type, parameter, cancellationToken), + Schema = await _componentService.GetOrCreateSchemaAsync(parameter.Type, parameter, cancellationToken: cancellationToken), Description = GetParameterDescriptionFromAttribute(parameter) }; @@ -347,7 +347,7 @@ private async Task GetFormRequestBody(IList parameter.ModelMetadata.ContainerType is null)) { var description = parameter.Single(); - var parameterSchema = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken); + var parameterSchema = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken: cancellationToken); // Form files are keyed by their parameter name so we must capture the parameter name // as a property in the schema. if (description.Type == typeof(IFormFile) || description.Type == typeof(IFormFileCollection)) @@ -410,7 +410,7 @@ private async Task GetFormRequestBody(IList() }; foreach (var description in parameter) { - propertySchema.Properties[description.Name] = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken); + propertySchema.Properties[description.Name] = await _componentService.GetOrCreateSchemaAsync(description.Type, null, cancellationToken: cancellationToken); } schema.AllOf.Add(propertySchema); } @@ -418,7 +418,7 @@ private async Task GetFormRequestBody(IList GetJsonRequestBody(IList GetOrCreateSchemaAsync(Type type, ApiParameterDescription? parameterDescription = null, CancellationToken cancellationToken = default, bool captureSchemaByRef = false) + internal async Task GetOrCreateSchemaAsync(Type type, ApiParameterDescription? parameterDescription = null, bool captureSchemaByRef = false, CancellationToken cancellationToken = default) { var key = parameterDescription?.ParameterDescriptor is IParameterInfoParameterDescriptor parameterInfoDescription && parameterDescription.ModelMetadata.PropertyName is null