Skip to content

Commit

Permalink
Fix mapping for nested schemas and [Produces] attributes in OpenAPI i…
Browse files Browse the repository at this point in the history
…mplementation (#57852)

* Fix handling of [Produces] without type on controller

* Recursively map sub-schemas to reference cache

* Rely on MaxDepth behavior in STJ

* Improve nested type test
  • Loading branch information
captainsafia committed Sep 16, 2024
1 parent 4f2a59c commit 9b3ca0b
Show file tree
Hide file tree
Showing 9 changed files with 387 additions and 24 deletions.
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

0 comments on commit 9b3ca0b

Please sign in to comment.