Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
21 changes: 14 additions & 7 deletions src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ namespace Microsoft.AspNetCore.OpenApi;
/// </summary>
internal sealed class OpenApiSchemaStore
{
// Matches default MaxValidationDepth and MaxModelBindingRecursionDepth used by MVC.
private readonly int MaxSchemaReferenceRecursionDepth = 32;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MaxValidationDepth and MaxModelBindingRecursionDepth are both configurable via MvcOptions. Do we think this will need to be configurable too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a future release, yes, but we will have to live with it being un-configurable for .NET 9 for now. :/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per off-line discussion, we're relying on STJ's recursion depth check which do have a configurable option via JsonSerializerOptions.MaxDepth.

private readonly Dictionary<OpenApiSchemaKey, JsonNode> _schemas = new()
{
// Pre-populate OpenAPI schemas for well-defined types in ASP.NET Core.
Expand Down Expand Up @@ -79,32 +81,37 @@ public JsonNode GetOrAdd(OpenApiSchemaKey key, Func<OpenApiSchemaKey, JsonNode>
/// </summary>
/// <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)
/// <param name="currentDepth">The current recursion depth for the reference schema.</param>
public void PopulateSchemaIntoReferenceCache(OpenApiSchema schema, bool captureSchemaByRef, int currentDepth = 0)
{
// Only capture top-level schemas by ref. Nested schemas will follow the
// reference by duplicate rules.
if (schema == null || currentDepth > MaxSchemaReferenceRecursionDepth)
{
return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can/should we log?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we should be able to inject a logger into this codepath.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm...actually I think we should throw here...

}

AddOrUpdateSchemaByReference(schema, captureSchemaByRef: captureSchemaByRef);
AddOrUpdateAnyOfSubSchemaByReference(schema);

if (schema.AdditionalProperties is not null)
{
AddOrUpdateSchemaByReference(schema.AdditionalProperties);
PopulateSchemaIntoReferenceCache(schema.AdditionalProperties, captureSchemaByRef, currentDepth++);
}
if (schema.Items is not null)
{
AddOrUpdateSchemaByReference(schema.Items);
PopulateSchemaIntoReferenceCache(schema.Items, captureSchemaByRef, currentDepth++);
}
if (schema.AllOf is not null)
{
foreach (var allOfSchema in schema.AllOf)
{
AddOrUpdateSchemaByReference(allOfSchema);
PopulateSchemaIntoReferenceCache(allOfSchema, captureSchemaByRef, currentDepth++);
}
}
if (schema.Properties is not null)
{
foreach (var property in schema.Properties.Values)
{
AddOrUpdateSchemaByReference(property);
PopulateSchemaIntoReferenceCache(property, captureSchemaByRef, currentDepth++);
}
}
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -359,4 +359,49 @@ private class TodoListContainer
{
public ICollection<Todo> Todos { get; set; } = [];
}

[Fact]
public async Task SupportsRefMappingInDeeplyNestedTypes()
{
// Arrange
var builder = CreateBuilder();

builder.MapPost("/", (Level1 item) => { });

await VerifyOpenApiDocument(builder, document =>
{
var operation = document.Paths["/"].Operations[OperationType.Post];
var requestSchema = operation.RequestBody.Content["application/json"].Schema;

// Assert $ref used for top-level
Assert.Equal("Level1", requestSchema.Reference.Id);

// Assert that $ref is used for Level1.Item2
var level1Schema = requestSchema.GetEffective(document);
Assert.Equal("Level2", level1Schema.Properties["item2"].Reference.Id);

// Assert that $ref is used for Level2.Item3
var level2Schema = level1Schema.Properties["item2"].GetEffective(document);
Assert.Equal("Level3", level2Schema.Properties["item3"].Reference.Id);

// Assert that no $ref is used for string property
var level3Schema = level2Schema.Properties["item3"].GetEffective(document);
Assert.Null(level3Schema.Properties["terminate"].Reference);
});
}

private class Level1
{
public Level2 Item2 { get; set; }
}

private class Level2
{
public Level3 Item3 { get; set; }
}

private class Level3
{
public string Terminate { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -531,14 +531,14 @@ await VerifyOpenApiDocument(builder, options, document =>
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;
var itemSchema = responseSchema.GetEffective(document).Items.GetEffective(document);
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;
var otherItemSchema = otherResponseSchema.GetEffective(document).Items.GetEffective(document);
Assert.Equal("modified-number-format", otherItemSchema.Format);
});
}
Expand Down Expand Up @@ -611,7 +611,7 @@ await VerifyOpenApiDocument(builder, options, document =>
var path = document.Paths["/list"];
var getOperation = path.Operations[OperationType.Get];
var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema;
var someShapeSchema = responseSchema.GetEffective(document).Properties["someShape"];
var someShapeSchema = responseSchema.GetEffective(document).Properties["someShape"].GetEffective(document);
var triangleSubschema = Assert.Single(someShapeSchema.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));
Expand Down Expand Up @@ -652,7 +652,7 @@ await VerifyOpenApiDocument(builder, options, document =>
var path = document.Paths["/list"];
var getOperation = path.Operations[OperationType.Get];
var responseSchema = getOperation.Responses["200"].Content["application/json"].Schema;
var someShapeSchema = responseSchema.GetEffective(document).Items.GetEffective(document).Properties["someShape"];
var someShapeSchema = responseSchema.GetEffective(document).Items.GetEffective(document).Properties["someShape"].GetEffective(document);
var triangleSubschema = Assert.Single(someShapeSchema.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));
Expand Down