Skip to content

Fix mapping for nested schemas and [Produces] attributes in OpenAPI implementation #57852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/OpenApi/src/Services/OpenApiDocumentService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ private async Task<OpenApiResponse> GetResponseAsync(
.SelectMany(attr => attr.ContentTypes);
foreach (var contentType in explicitContentTypes)
{
response.Content[contentType] = new OpenApiMediaType();
response.Content.TryAdd(contentType, new OpenApiMediaType());
}

return response;
Expand Down
5 changes: 4 additions & 1 deletion src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ internal sealed class OpenApiSchemaService(
IOptionsMonitor<OpenApiOptions> optionsMonitor)
{
private readonly OpenApiSchemaStore _schemaStore = serviceProvider.GetRequiredKeyedService<OpenApiSchemaStore>(documentName);
private readonly OpenApiJsonSchemaContext _jsonSchemaContext = new OpenApiJsonSchemaContext(new(jsonOptions.Value.SerializerOptions));
private readonly JsonSerializerOptions _jsonSerializerOptions = new(jsonOptions.Value.SerializerOptions)
{
// In order to properly handle the `RequiredAttribute` on type properties, add a modifier to support
Expand Down Expand Up @@ -135,7 +136,9 @@ internal async Task<OpenApiSchema> GetOrCreateSchemaAsync(Type type, IServicePro
{
schemaAsJsonObject.ApplyParameterInfo(parameterDescription, _jsonSerializerOptions.GetTypeInfo(type));
}
var deserializedSchema = JsonSerializer.Deserialize(schemaAsJsonObject, OpenApiJsonSchemaContext.Default.OpenApiJsonSchema);
// Use _jsonSchemaContext constructed from _jsonSerializerOptions to respect shared config set by end-user,
// particularly in the case of maxDepth.
var deserializedSchema = JsonSerializer.Deserialize(schemaAsJsonObject, _jsonSchemaContext.OpenApiJsonSchema);
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, scopedServiceProvider, schemaTransformers, parameterDescription, cancellationToken);
Expand Down
16 changes: 10 additions & 6 deletions src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,34 +77,38 @@ public JsonNode GetOrAdd(OpenApiSchemaKey key, Func<OpenApiSchemaKey, JsonNode>
/// been encountered more than once in the document to avoid unnecessarily capturing unique
/// schemas into the top-level document.
/// </summary>
/// <remarks>
/// We don't do a depth check in the recursion call here since we assume that
/// System.Text.Json has already validate the depth of the schema based on
/// the configured JsonSerializerOptions.MaxDepth value.
/// </remarks>
/// <param name="schema">The <see cref="OpenApiSchema"/> to add to the schemas-with-references cache.</param>
/// <param name="captureSchemaByRef"><see langword="true"/> if schema should always be referenced instead of inlined.</param>
public void PopulateSchemaIntoReferenceCache(OpenApiSchema schema, bool captureSchemaByRef)
{
// Only capture top-level schemas by ref. Nested schemas will follow the
// reference by duplicate rules.
AddOrUpdateSchemaByReference(schema, captureSchemaByRef: captureSchemaByRef);
AddOrUpdateAnyOfSubSchemaByReference(schema);

if (schema.AdditionalProperties is not null)
{
AddOrUpdateSchemaByReference(schema.AdditionalProperties);
PopulateSchemaIntoReferenceCache(schema.AdditionalProperties, captureSchemaByRef);
}
if (schema.Items is not null)
{
AddOrUpdateSchemaByReference(schema.Items);
PopulateSchemaIntoReferenceCache(schema.Items, captureSchemaByRef);
}
if (schema.AllOf is not null)
{
foreach (var allOfSchema in schema.AllOf)
{
AddOrUpdateSchemaByReference(allOfSchema);
PopulateSchemaIntoReferenceCache(allOfSchema, captureSchemaByRef);
}
}
if (schema.Properties is not null)
{
foreach (var property in schema.Properties.Values)
{
AddOrUpdateSchemaByReference(property);
PopulateSchemaIntoReferenceCache(property, captureSchemaByRef);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public async Task GeneratesSchemaForPoco_WithSchemaReferenceIdCustomization()

// Act
builder.MapPost("/", (Todo todo) => { });
var options = new OpenApiOptions { CreateSchemaReferenceId = (type) => $"{type.Type.Name}Schema" };
var options = new OpenApiOptions { CreateSchemaReferenceId = (type) => type.Type.Name == "Todo" ? $"{type.Type.Name}Schema" : OpenApiOptions.CreateDefaultSchemaReferenceId(type) };

// Assert
await VerifyOpenApiDocument(builder, options, document =>
Expand Down Expand Up @@ -114,7 +114,7 @@ public async Task GeneratesInlineSchemaForPoco_WithCustomNullId()

// Act
builder.MapPost("/", (Todo todo) => { });
var options = new OpenApiOptions { CreateSchemaReferenceId = (type) => type.Type.Name == "Todo" ? null : $"{type.Type.Name}Schema" };
var options = new OpenApiOptions { CreateSchemaReferenceId = (type) => type.Type.Name == "Todo" ? null : OpenApiOptions.CreateDefaultSchemaReferenceId(type) };

// Assert
await VerifyOpenApiDocument(builder, options, document =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Reflection;
using System.Runtime.InteropServices;
using System.Text;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting.Server;
using Microsoft.AspNetCore.Hosting.Server.Features;
Expand All @@ -12,15 +12,18 @@
using Microsoft.AspNetCore.Mvc.ActionConstraints;
using Microsoft.AspNetCore.Mvc.ApiExplorer;
using Microsoft.AspNetCore.Mvc.Controllers;
using Microsoft.AspNetCore.Mvc.Formatters;
using Microsoft.AspNetCore.Mvc.Infrastructure;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.AspNetCore.OpenApi;
using Microsoft.AspNetCore.Routing;
using Microsoft.AspNetCore.Routing.Constraints;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
using Microsoft.Net.Http.Headers;
using Microsoft.OpenApi.Models;
using Moq;
using Xunit.Sdk;
using static Microsoft.AspNetCore.OpenApi.Tests.OpenApiOperationGeneratorTests;

public abstract class OpenApiDocumentServiceTestBase
Expand Down Expand Up @@ -53,7 +56,10 @@ internal static OpenApiDocumentService CreateDocumentService(IEndpointRouteBuild
ApplicationName = nameof(OpenApiDocumentServiceTests)
};

var options = new MvcOptions();
var options = new MvcOptions
{
OutputFormatters = { new TestJsonOutputFormatter() }
};
var optionsAccessor = Options.Create(options);

var constraintResolver = new Mock<IInlineConstraintResolver>();
Expand Down Expand Up @@ -87,6 +93,22 @@ internal static OpenApiDocumentService CreateDocumentService(IEndpointRouteBuild
return documentService;
}

private class TestJsonOutputFormatter : TextOutputFormatter
{
public TestJsonOutputFormatter()
{
SupportedMediaTypes.Add(new MediaTypeHeaderValue("application/json"));
SupportedMediaTypes.Add(new MediaTypeHeaderValue("text/json"));

SupportedEncodings.Add(Encoding.UTF8);
}

public override Task WriteResponseBodyAsync(OutputFormatterWriteContext context, Encoding selectedEncoding)
{
return Task.FromResult(0);
}
}

internal static OpenApiDocumentService CreateDocumentService(IEndpointRouteBuilder builder, OpenApiOptions openApiOptions)
{
var context = new ApiDescriptionProviderContext([]);
Expand Down Expand Up @@ -203,6 +225,14 @@ public ControllerActionDescriptor CreateActionDescriptor(string methodName = nul
action.RouteValues.Add("controller", "Test");
action.RouteValues.Add("action", action.MethodInfo.Name);
action.ActionConstraints = [new HttpMethodActionConstraint(["GET"])];
action.EndpointMetadata = [..action.MethodInfo.GetCustomAttributes()];
if (controllerType is not null)
{
foreach (var attribute in controllerType.GetCustomAttributes())
{
action.EndpointMetadata.Add(attribute);
}
}

action.Parameters = [];
foreach (var parameter in action.MethodInfo.GetParameters())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.ComponentModel;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.OpenApi.Any;
using Microsoft.OpenApi.Models;

Expand Down Expand Up @@ -292,8 +293,9 @@ await VerifyOpenApiDocument(builder, document =>
property =>
{
Assert.Equal("value", property.Key);
Assert.Equal("object", property.Value.Type);
Assert.Collection(property.Value.Properties,
var propertyValue = property.Value.GetEffective(document);
Assert.Equal("object", propertyValue.Type);
Assert.Collection(propertyValue.Properties,
property =>
{
Assert.Equal("id", property.Key);
Expand All @@ -317,8 +319,9 @@ await VerifyOpenApiDocument(builder, document =>
property =>
{
Assert.Equal("error", property.Key);
Assert.Equal("object", property.Value.Type);
Assert.Collection(property.Value.Properties, property =>
var propertyValue = property.Value.GetEffective(document);
Assert.Equal("object", propertyValue.Type);
Assert.Collection(propertyValue.Properties, property =>
{
Assert.Equal("code", property.Key);
Assert.Equal("integer", property.Value.Type);
Expand Down Expand Up @@ -404,8 +407,10 @@ await VerifyOpenApiDocument(builder, document =>
property =>
{
Assert.Equal("todo", property.Key);
Assert.Equal("object", property.Value.Type);
Assert.Collection(property.Value.Properties,
Assert.NotNull(property.Value.Reference);
var propertyValue = property.Value.GetEffective(document);
Assert.Equal("object", propertyValue.Type);
Assert.Collection(propertyValue.Properties,
property =>
{
Assert.Equal("id", property.Key);
Expand Down Expand Up @@ -529,8 +534,10 @@ await VerifyOpenApiDocument(builder, document =>
Assert.Equal("items", property.Key);
Assert.Equal("array", property.Value.Type);
Assert.NotNull(property.Value.Items);
Assert.Equal("object", property.Value.Items.Type);
Assert.Collection(property.Value.Items.Properties,
Assert.NotNull(property.Value.Items.Reference);
Assert.Equal("object", property.Value.Items.GetEffective(document).Type);
var itemsValue = property.Value.Items.GetEffective(document);
Assert.Collection(itemsValue.Properties,
property =>
{
Assert.Equal("id", property.Key);
Expand Down Expand Up @@ -653,6 +660,53 @@ await VerifyOpenApiDocument(builder, document =>
});
}

[Fact]
public async Task GetOpenApiResponse_SupportsProducesWithProducesResponseTypeOnController()
{
var actionDescriptor = CreateActionDescriptor(nameof(TestController.Get), typeof(TestController));

await VerifyOpenApiDocument(actionDescriptor, document =>
{
var operation = document.Paths["/"].Operations[OperationType.Get];
var responses = Assert.Single(operation.Responses);
var response = responses.Value;
Assert.True(response.Content.TryGetValue("application/json", out var mediaType));
var schema = mediaType.Schema.GetEffective(document);
Assert.Equal("object", schema.Type);
Assert.Collection(schema.Properties,
property =>
{
Assert.Equal("id", property.Key);
Assert.Equal("integer", property.Value.Type);
},
property =>
{
Assert.Equal("title", property.Key);
Assert.Equal("string", property.Value.Type);
},
property =>
{
Assert.Equal("completed", property.Key);
Assert.Equal("boolean", property.Value.Type);
},
property =>
{
Assert.Equal("createdAt", property.Key);
Assert.Equal("string", property.Value.Type);
Assert.Equal("date-time", property.Value.Format);
});
});
}

[ApiController]
[Produces("application/json")]
public class TestController
{
[Route("/")]
[ProducesResponseType(typeof(Todo), StatusCodes.Status200OK)]
internal Todo Get() => new(1, "Write test", false, DateTime.Now);
}

private class ClassWithObjectProperty
{
public object Object { get; set; }
Expand Down
Loading
Loading