Skip to content
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

Fix up OpenAPI schema handling and support concurrent requests #58024

Merged
merged 5 commits into from
Sep 25, 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
5 changes: 5 additions & 0 deletions src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,11 @@ public static void ReadProperty(ref Utf8JsonReader reader, string propertyName,
break;
case OpenApiSchemaKeywords.AdditionalPropertiesKeyword:
reader.Read();
if (reader.TokenType == JsonTokenType.False)
{
schema.AdditionalPropertiesAllowed = false;
break;
}
var additionalPropsConverter = (JsonConverter<OpenApiJsonSchema>)options.GetTypeInfo(typeof(OpenApiJsonSchema)).Converter;
schema.AdditionalProperties = additionalPropsConverter.Read(ref reader, typeof(OpenApiJsonSchema), options)?.Schema;
break;
Expand Down
11 changes: 9 additions & 2 deletions src/OpenApi/src/Services/OpenApiDocumentService.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Concurrent;
using System.Collections.Frozen;
using System.ComponentModel;
using System.ComponentModel.DataAnnotations;
Expand Down Expand Up @@ -46,7 +47,7 @@ internal sealed class OpenApiDocumentService(
/// are unique within the lifetime of an application and serve as helpful associators between
/// operations, API descriptions, and their respective transformer contexts.
/// </summary>
private readonly Dictionary<string, OpenApiOperationTransformerContext> _operationTransformerContextCache = new();
private readonly ConcurrentDictionary<string, OpenApiOperationTransformerContext> _operationTransformerContextCache = new();
private static readonly ApiResponseType _defaultApiResponseType = new() { StatusCode = StatusCodes.Status200OK };

private static readonly FrozenSet<string> _disallowedHeaderParameters = new[] { HeaderNames.Accept, HeaderNames.Authorization, HeaderNames.ContentType }.ToFrozenSet(StringComparer.OrdinalIgnoreCase);
Expand Down Expand Up @@ -402,6 +403,12 @@ private async Task<OpenApiResponse> GetResponseAsync(
continue;
}

// MVC's ModelMetadata layer will set ApiParameterDescription.Type to string when the parameter
// is a parsable or convertible type. In this case, we want to use the actual model type
// to generate the schema instead of the string type.
var targetType = parameter.Type == typeof(string) && parameter.ModelMetadata.ModelType != parameter.Type
? parameter.ModelMetadata.ModelType
: parameter.Type;
var openApiParameter = new OpenApiParameter
{
Name = parameter.Name,
Expand All @@ -413,7 +420,7 @@ private async Task<OpenApiResponse> GetResponseAsync(
_ => throw new InvalidOperationException($"Unsupported parameter source: {parameter.Source.Id}")
},
Required = IsRequired(parameter),
Schema = await _componentService.GetOrCreateSchemaAsync(parameter.Type, scopedServiceProvider, schemaTransformers, parameter, cancellationToken: cancellationToken),
Schema = await _componentService.GetOrCreateSchemaAsync(targetType, scopedServiceProvider, schemaTransformers, parameter, cancellationToken: cancellationToken),
Description = GetParameterDescriptionFromAttribute(parameter)
};

Expand Down
23 changes: 13 additions & 10 deletions src/OpenApi/src/Services/Schemas/OpenApiSchemaStore.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Concurrent;
using System.IO.Pipelines;
using System.Text.Json.Nodes;
using Microsoft.AspNetCore.Http;
Expand All @@ -14,7 +15,7 @@ namespace Microsoft.AspNetCore.OpenApi;
/// </summary>
internal sealed class OpenApiSchemaStore
{
private readonly Dictionary<OpenApiSchemaKey, JsonNode> _schemas = new()
private readonly ConcurrentDictionary<OpenApiSchemaKey, JsonNode> _schemas = new()
{
// Pre-populate OpenAPI schemas for well-defined types in ASP.NET Core.
[new OpenApiSchemaKey(typeof(IFormFile), null)] = new JsonObject
Expand Down Expand Up @@ -48,8 +49,8 @@ internal sealed class OpenApiSchemaStore
},
};

public readonly Dictionary<OpenApiSchema, string?> SchemasByReference = new(OpenApiSchemaComparer.Instance);
private readonly Dictionary<string, int> _referenceIdCounter = new();
public readonly ConcurrentDictionary<OpenApiSchema, string?> SchemasByReference = new(OpenApiSchemaComparer.Instance);
private readonly ConcurrentDictionary<string, int> _referenceIdCounter = new();

/// <summary>
/// Resolves the JSON schema for the given type and parameter description.
Expand All @@ -59,13 +60,7 @@ internal sealed class OpenApiSchemaStore
/// <returns>A <see cref="JsonObject" /> representing the JSON schema associated with the key.</returns>
public JsonNode GetOrAdd(OpenApiSchemaKey key, Func<OpenApiSchemaKey, JsonNode> valueFactory)
{
if (_schemas.TryGetValue(key, out var schema))
{
return schema;
}
var targetSchema = valueFactory(key);
_schemas.Add(key, targetSchema);
return targetSchema;
return _schemas.GetOrAdd(key, valueFactory);
}

/// <summary>
Expand Down Expand Up @@ -159,6 +154,14 @@ private void AddOrUpdateAnyOfSubSchemaByReference(OpenApiSchema schema)
private void AddOrUpdateSchemaByReference(OpenApiSchema schema, string? baseTypeSchemaId = null, bool captureSchemaByRef = false)
{
var targetReferenceId = baseTypeSchemaId is not null ? $"{baseTypeSchemaId}{GetSchemaReferenceId(schema)}" : GetSchemaReferenceId(schema);
// Schemas that already have a reference provided by JsonSchemaExporter are skipped here
// and handled by the OpenApiSchemaReferenceTransformer instead. This case typically kicks
// in for self-referencing schemas where JsonSchemaExporter inlines references to avoid
// infinite recursion.
if (schema.Reference is not null)
{
return;
}
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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Concurrent;
using System.Linq;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.OpenApi.Models;
Expand Down Expand Up @@ -85,7 +86,7 @@ public Task TransformAsync(OpenApiDocument document, OpenApiDocumentTransformerC
/// <param name="schema">The inline schema to replace with a reference.</param>
/// <param name="schemasByReference">A cache of schemas and their associated reference IDs.</param>
/// <param name="isTopLevel">When <see langword="true" />, will skip resolving references for the top-most schema provided.</param>
internal static OpenApiSchema? ResolveReferenceForSchema(OpenApiSchema? schema, Dictionary<OpenApiSchema, string?> schemasByReference, bool isTopLevel = false)
internal static OpenApiSchema? ResolveReferenceForSchema(OpenApiSchema? schema, ConcurrentDictionary<OpenApiSchema, string?> schemasByReference, bool isTopLevel = false)
{
if (schema is null)
{
Expand All @@ -101,6 +102,16 @@ public Task TransformAsync(OpenApiDocument document, OpenApiDocumentTransformerC
return new OpenApiSchema { Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = referenceId } };
}

// Handle schemas where the references have been inline by the JsonSchemaExporter. In this case,
// the `#` ID is generated by the exporter since it has no base document to baseline against. In this
// case we we want to replace the reference ID with the schema ID that was generated by the
// `CreateSchemaReferenceId` method in the OpenApiSchemaService.
if (!isTopLevel && schema.Reference is { Type: ReferenceType.Schema, Id: "#" }
&& schema.Annotations.TryGetValue(OpenApiConstants.SchemaId, out var schemaId))
{
return new OpenApiSchema { Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = schemaId?.ToString() } };
}

if (schema.AllOf is not null)
{
for (var i = 0; i < schema.AllOf.Count; i++)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Net;
using System.Net.Http;

namespace Microsoft.AspNetCore.OpenApi.Tests.Integration;

public class OpenApiDocumentConcurrentRequestTests(SampleAppFixture fixture) : IClassFixture<SampleAppFixture>
{
[Fact]
public async Task MapOpenApi_HandlesConcurrentRequests()
{
// Arrange
var client = fixture.CreateClient();

// Act
await Parallel.ForAsync(0, 150, async (_, ctx) =>
{
var response = await client.GetAsync("/openapi/v1.json", ctx);

// Assert
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -537,4 +537,59 @@ await VerifyOpenApiDocument(builder, document =>
Assert.Null(operation.RequestBody.Content["application/json"].Schema.Type);
});
}

[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task SupportsParameterWithEnumType(bool useAction)
{
// Arrange
if (!useAction)
{
var builder = CreateBuilder();
builder.MapGet("/api/with-enum", (ItemStatus status) => status);
}
else
{
var action = CreateActionDescriptor(nameof(GetItemStatus));
await VerifyOpenApiDocument(action, AssertOpenApiDocument);
}

static void AssertOpenApiDocument(OpenApiDocument document)
{
var operation = document.Paths["/api/with-enum"].Operations[OperationType.Get];
var parameter = Assert.Single(operation.Parameters);
var response = Assert.Single(operation.Responses).Value.Content["application/json"].Schema;
Assert.NotNull(parameter.Schema.Reference);
Assert.Equal(parameter.Schema.Reference.Id, response.Reference.Id);
var schema = parameter.Schema.GetEffective(document);
Assert.Collection(schema.Enum,
value =>
{
var openApiString = Assert.IsType<OpenApiString>(value);
Assert.Equal("Pending", openApiString.Value);
},
value =>
{
var openApiString = Assert.IsType<OpenApiString>(value);
Assert.Equal("Approved", openApiString.Value);
},
value =>
{
var openApiString = Assert.IsType<OpenApiString>(value);
Assert.Equal("Rejected", openApiString.Value);
});
}
}

[Route("/api/with-enum")]
private ItemStatus GetItemStatus([FromQuery] ItemStatus status) => status;

[JsonConverter(typeof(JsonStringEnumConverter<ItemStatus>))]
internal enum ItemStatus
{
Pending = 0,
Approved = 1,
Rejected = 2,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.ComponentModel;
using System.ComponentModel.DataAnnotations;
using System.IO.Pipelines;
using System.Text.Json.Serialization;
using System.Text.Json.Serialization.Metadata;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Mvc;
Expand Down Expand Up @@ -594,4 +595,107 @@ await VerifyOpenApiDocument(builder, document =>
});
});
}

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

// Act
builder.MapPost("/api", (ExampleWithDisallowedUnmappedMembers type) => { });

// Assert
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.GetEffective(document);
Assert.Collection(schema.Properties,
property =>
{
Assert.Equal("number", property.Key);
Assert.Equal("integer", property.Value.Type);
});
Assert.False(schema.AdditionalPropertiesAllowed);
});
}

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

// Act
builder.MapPost("/api", (ExampleWithSkippedUnmappedMembers type) => { });

// Assert
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.GetEffective(document);
Assert.Collection(schema.Properties,
property =>
{
Assert.Equal("number", property.Key);
Assert.Equal("integer", property.Value.Type);
});
Assert.True(schema.AdditionalPropertiesAllowed);
});
}

[JsonUnmappedMemberHandling(JsonUnmappedMemberHandling.Disallow)]
private class ExampleWithDisallowedUnmappedMembers
{
public int Number { get; init; }
}

[JsonUnmappedMemberHandling(JsonUnmappedMemberHandling.Skip)]
private class ExampleWithSkippedUnmappedMembers
{
public int Number { get; init; }
}

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

// Act
builder.MapPost("/api", (Parent parent) => { });

// Assert
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.GetEffective(document);
Assert.Collection(schema.Properties,
property =>
{
Assert.Equal("selfReferenceList", property.Key);
Assert.Equal("array", property.Value.Type);
Assert.Equal("Parent", property.Value.Items.Reference.Id);
},
property =>
{
Assert.Equal("selfReferenceDictionary", property.Key);
Assert.Equal("object", property.Value.Type);
Assert.Equal("Parent", property.Value.AdditionalProperties.Reference.Id);
});
});
}

public class Parent
{
public IEnumerable<Parent> SelfReferenceList { get; set; } = [ ];
public IDictionary<string, Parent> SelfReferenceDictionary { get; set; } = new Dictionary<string, Parent>();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -450,4 +450,31 @@ await VerifyOpenApiDocument(builder, document =>
}
});
}

[Fact]
public async Task SelfReferenceMapperOnlyOperatesOnSchemaReferenceTypes()
{
var builder = CreateBuilder();

builder.MapGet("/todo", () => new Todo(1, "Item1", false, DateTime.Now));

var options = new OpenApiOptions();
options.AddSchemaTransformer((schema, context, cancellationToken) =>
{
if (context.JsonTypeInfo.Type == typeof(Todo))
{
schema.Reference = new OpenApiReference { Id = "#", Type = ReferenceType.Link };
}
return Task.CompletedTask;
});

await VerifyOpenApiDocument(builder, options, document =>
{
var operation = document.Paths["/todo"].Operations[OperationType.Get];
var response = operation.Responses["200"].Content["application/json"];
var responseSchema = response.Schema;
Assert.Equal("#", responseSchema.Reference.Id);
Assert.Equal(ReferenceType.Link, responseSchema.Reference.Type);
});
}
}
Loading