[release/10.0] Fix circular reference resolution for schemas containing arrays (#64720)

* OpenAPI schemas: Add failing test for circular references with an array

* Fix array circular references.

The fix is done by registering components before going deep into the recursion tree (where the leaves would be registered first).

Fixing this revealed an issue for default values for "local" attributes. Local attributes/parameter info should not apply to componetized schemas.

* Fix build warnings

* Fix missing negation

* Cleanup some funky logic

* Fix cases where the schemas will not be componetized because the x-schema-id is null or empty

* Stop ResolveReferenceForSchema when the schema has already been resolved

* Remove unused method `CreateReference`

* Remove OpenApiSchemaKey as only the Type property was used

* Remove unnecessary using

* Fix punctuation in comment.

* Refactor null check in AddOpenApiSchemaByReference

* Fix schemaId to be null instead of string.Empty when a schema is not componetized

* Remove x-ref-id metadata as it isn't used anymore (and gave undesired results)

---------

Co-authored-by: Sjoerd van der Meer <sjoerd.vandermeer@xebia.com>

Author: github-actions[bot]

src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs

@@ -3,6 +3,7 @@
 
 using System.ComponentModel;
 using System.ComponentModel.DataAnnotations;
+using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Linq;
 using System.Reflection;
@@ -175,13 +176,17 @@ internal static void ApplyDefaultValue(this JsonNode schema, object? defaultValu
             return;
         }
 
+        var schemaAttribute = schema.WillBeComponentized()
+            ? OpenApiConstants.RefDefaultAnnotation
+            : OpenApiSchemaKeywords.DefaultKeyword;
+
         if (defaultValue is null)
         {
-            schema[OpenApiSchemaKeywords.DefaultKeyword] = null;
+            schema[schemaAttribute] = null;
         }
         else
         {
-            schema[OpenApiSchemaKeywords.DefaultKeyword] = JsonSerializer.SerializeToNode(defaultValue, jsonTypeInfo);
+            schema[schemaAttribute] = JsonSerializer.SerializeToNode(defaultValue, jsonTypeInfo);
         }
     }
 
@@ -429,6 +434,36 @@ internal static void ApplySchemaReferenceId(this JsonNode schema, JsonSchemaExpo
         }
     }
 
+    /// <summary>
+    /// Determines whether the specified JSON schema will be moved into the components section.
+    /// </summary>
+    /// <param name="schema">The <see cref="JsonNode"/> produced by the underlying schema generator.</param>
+    /// <returns><see langword="true"/> if the schema will be componentized; otherwise, <see langword="false"/>.</returns>
+    internal static bool WillBeComponentized(this JsonNode schema)
+        => schema.WillBeComponentized(out _);
+
+    /// <summary>
+    /// Determines whether the specified JSON schema node contains a componentized schema identifier.
+    /// </summary>
+    /// <param name="schema">The JSON schema node to inspect for a componentized schema identifier.</param>
+    /// <param name="schemaId">When this method returns <see langword="true"/>, contains the schema identifier found in the node; otherwise,
+    /// <see langword="null"/>.</param>
+    /// <returns><see langword="true"/> if the schema will be componentized; otherwise, <see langword="false"/>.</returns>
+    internal static bool WillBeComponentized(this JsonNode schema, [NotNullWhen(true)] out string? schemaId)
+    {
+        if (schema[OpenApiConstants.SchemaId] is JsonNode schemaIdNode
+            && schemaIdNode.GetValueKind() == JsonValueKind.String)
+        {
+            schemaId = schemaIdNode.GetValue<string>();
+            if (!string.IsNullOrEmpty(schemaId))
+            {
+                return true;
+            }
+        }
+        schemaId = null;
+        return false;
+    }
+
     /// <summary>
     /// Returns <langword ref="true" /> if the current type is a non-abstract base class that is not defined as its
     /// own derived type.
@@ -458,7 +493,7 @@ internal static void ApplyNullabilityContextInfo(this JsonNode schema, JsonPrope
                 schema[OpenApiSchemaKeywords.TypeKeyword] = (schemaTypes | JsonSchemaType.Null).ToString();
             }
         }
-        if (schema[OpenApiConstants.SchemaId] is not null &&
+        if (schema.WillBeComponentized() &&
             propertyInfo.PropertyType != typeof(object) && propertyInfo.ShouldApplyNullablePropertySchema())
         {
             schema[OpenApiConstants.NullableProperty] = true;
@@ -472,7 +507,7 @@ internal static void ApplyNullabilityContextInfo(this JsonNode schema, JsonPrope
     /// <param name="schema">The <see cref="JsonNode"/> produced by the underlying schema generator.</param>
     internal static void PruneNullTypeForComponentizedTypes(this JsonNode schema)
     {
-        if (schema[OpenApiConstants.SchemaId] is not null &&
+        if (schema.WillBeComponentized() &&
                 schema[OpenApiSchemaKeywords.TypeKeyword] is JsonArray typeArray)
         {
             for (var i = typeArray.Count - 1; i >= 0; i--)

src/OpenApi/src/Extensions/OpenApiDocumentExtensions.cs

@@ -14,28 +14,33 @@ internal static class OpenApiDocumentExtensions
     /// <param name="document">The <see cref="OpenApiDocument"/> to register the schema onto.</param>
     /// <param name="schemaId">The ID that serves as the key for the schema in the schema store.</param>
     /// <param name="schema">The <see cref="IOpenApiSchema" /> to register into the document.</param>
-    /// <returns>An <see cref="IOpenApiSchema"/> with a reference to the stored schema.</returns>
-    public static IOpenApiSchema AddOpenApiSchemaByReference(this OpenApiDocument document, string schemaId, IOpenApiSchema schema)
+    /// <param name="schemaReference">An <see cref="IOpenApiSchema"/> with a reference to the stored schema.</param>
+    /// <returns>Whether the schema was added or already existed.</returns>
+    public static bool AddOpenApiSchemaByReference(this OpenApiDocument document, string schemaId, IOpenApiSchema schema, out OpenApiSchemaReference schemaReference)
     {
-        document.Components ??= new();
-        document.Components.Schemas ??= new Dictionary<string, IOpenApiSchema>();
-        document.Components.Schemas[schemaId] = schema;
+        // Make sure the document has a workspace,
+        // AddComponent will add it to the workspace when adding the component.
         document.Workspace ??= new();
-        var location = document.BaseUri + "/components/schemas/" + schemaId;
-        document.Workspace.RegisterComponentForDocument(document, schema, location);
+        // AddComponent will only add the schema if it doesn't already exist.
+        var schemaAdded = document.AddComponent(schemaId, schema);
 
         object? description = null;
         object? example = null;
-        if (schema is OpenApiSchema actualSchema)
+        object? defaultAnnotation = null;
+        if (schema is OpenApiSchema { Metadata: not null } actualSchema)
         {
-            actualSchema.Metadata?.TryGetValue(OpenApiConstants.RefDescriptionAnnotation, out description);
-            actualSchema.Metadata?.TryGetValue(OpenApiConstants.RefExampleAnnotation, out example);
+            actualSchema.Metadata.TryGetValue(OpenApiConstants.RefDescriptionAnnotation, out description);
+            actualSchema.Metadata.TryGetValue(OpenApiConstants.RefExampleAnnotation, out example);
+            actualSchema.Metadata.TryGetValue(OpenApiConstants.RefDefaultAnnotation, out defaultAnnotation);
         }
 
-        return new OpenApiSchemaReference(schemaId, document)
+        schemaReference = new OpenApiSchemaReference(schemaId, document)
         {
             Description = description as string,
             Examples = example is JsonNode exampleJson ? [exampleJson] : null,
+            Default = defaultAnnotation as JsonNode,
         };
+
+        return schemaAdded;
     }
 }

src/OpenApi/src/Extensions/OpenApiSchemaExtensions.cs

@@ -1,6 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics.CodeAnalysis;
+
 namespace Microsoft.AspNetCore.OpenApi;
 
 internal static class OpenApiSchemaExtensions
@@ -18,4 +20,21 @@ public static IOpenApiSchema CreateOneOfNullableWrapper(this IOpenApiSchema orig
             ]
         };
     }
+
+    public static bool IsComponentizedSchema(this OpenApiSchema schema)
+        => schema.IsComponentizedSchema(out _);
+
+    public static bool IsComponentizedSchema(this OpenApiSchema schema, [NotNullWhen(true)] out string? schemaId)
+    {
+        if(schema.Metadata is not null
+            && schema.Metadata.TryGetValue(OpenApiConstants.SchemaId, out var schemaIdAsObject)
+            && schemaIdAsObject is string schemaIdString
+            && !string.IsNullOrEmpty(schemaIdString))
+        {
+            schemaId = schemaIdString;
+            return true;
+        }
+        schemaId = null;
+        return false;
+    }
 }

src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs

@@ -345,17 +345,16 @@ public static void ReadProperty(ref Utf8JsonReader reader, string propertyName,
                 schema.Enum = ReadJsonNode(ref reader, out var constType) is { } jsonNode ? [jsonNode] : [];
                 schema.Type = constType;
                 break;
-            case OpenApiSchemaKeywords.RefKeyword:
+            case OpenApiConstants.RefDescriptionAnnotation:
                 reader.Read();
                 schema.Metadata ??= new Dictionary<string, object>();
-                schema.Metadata[OpenApiConstants.RefId] = reader.GetString() ?? string.Empty;
+                schema.Metadata[OpenApiConstants.RefDescriptionAnnotation] = reader.GetString() ?? string.Empty;
                 break;
-            case OpenApiConstants.RefDescriptionAnnotation:
+            case OpenApiConstants.RefDefaultAnnotation:
                 reader.Read();
                 schema.Metadata ??= new Dictionary<string, object>();
-                schema.Metadata[OpenApiConstants.RefDescriptionAnnotation] = reader.GetString() ?? string.Empty;
+                schema.Metadata[OpenApiConstants.RefDefaultAnnotation] = ReadJsonNode(ref reader)!;
                 break;
-
             default:
                 reader.Skip();
                 break;

src/OpenApi/src/Services/OpenApiConstants.cs

@@ -12,7 +12,7 @@ internal static class OpenApiConstants
     internal const string DefaultOpenApiRoute = "/openapi/{documentName}.json";
     internal const string DescriptionId = "x-aspnetcore-id";
     internal const string SchemaId = "x-schema-id";
-    internal const string RefId = "x-ref-id";
+    internal const string RefDefaultAnnotation = "x-ref-default";
     internal const string RefDescriptionAnnotation = "x-ref-description";
     internal const string RefExampleAnnotation = "x-ref-example";
     internal const string RefKeyword = "$ref";

src/OpenApi/src/Services/Schemas/OpenApiSchemaKey.cs

@@ -14,7 +14,6 @@
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Json;
 using Microsoft.AspNetCore.Mvc.ApiExplorer;
-using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Options;
 
@@ -116,7 +115,7 @@ internal sealed class OpenApiSchemaService(
                 {
                     schema.ApplyDefaultValue(defaultValueAttribute.Value, context.TypeInfo);
                 }
-                var isInlinedSchema = schema[OpenApiConstants.SchemaId] is null;
+                var isInlinedSchema = !schema.WillBeComponentized();
                 if (isInlinedSchema)
                 {
                     if (propertyAttributes.OfType<DescriptionAttribute>().LastOrDefault() is { } descriptionAttribute)
@@ -229,10 +228,7 @@ static JsonArray JsonArray(ReadOnlySpan<JsonNode> values)
 
     internal async Task<OpenApiSchema> GetOrCreateUnresolvedSchemaAsync(OpenApiDocument? document, Type type, IServiceProvider scopedServiceProvider, IOpenApiSchemaTransformer[] schemaTransformers, ApiParameterDescription? parameterDescription = null, CancellationToken cancellationToken = default)
     {
-        var key = parameterDescription?.ParameterDescriptor is IParameterInfoParameterDescriptor parameterInfoDescription
-            && parameterDescription.ModelMetadata.PropertyName is null
-            ? new OpenApiSchemaKey(type, parameterInfoDescription.ParameterInfo) : new OpenApiSchemaKey(type, null);
-        var schemaAsJsonObject = CreateSchema(key);
+        var schemaAsJsonObject = CreateSchema(type);
         if (parameterDescription is not null)
         {
             schemaAsJsonObject.ApplyParameterInfo(parameterDescription, _jsonSerializerOptions.GetTypeInfo(type));
@@ -265,18 +261,33 @@ internal static IOpenApiSchema ResolveReferenceForSchema(OpenApiDocument documen
     {
         var schema = UnwrapOpenApiSchema(inputSchema);
 
-        if (schema.Metadata is not null &&
-            schema.Metadata.TryGetValue(OpenApiConstants.SchemaId, out var resolvedBaseSchemaId))
+        var isComponentizedSchema = schema.IsComponentizedSchema(out var schemaId);
+
+        // When we register it, this will be the resulting reference
+        OpenApiSchemaReference? resultSchemaReference = null;
+        if (inputSchema is OpenApiSchema && isComponentizedSchema)
         {
-            if (schema.AnyOf is { Count: > 0 })
+            var targetReferenceId = baseSchemaId is not null
+                ? $"{baseSchemaId}{schemaId}"
+                : schemaId;
+            if (!string.IsNullOrEmpty(targetReferenceId))
             {
-                for (var i = 0; i < schema.AnyOf.Count; i++)
+                if (!document.AddOpenApiSchemaByReference(targetReferenceId, schema, out resultSchemaReference))
                 {
-                    schema.AnyOf[i] = ResolveReferenceForSchema(document, schema.AnyOf[i], rootSchemaId, resolvedBaseSchemaId?.ToString());
+                    // We already added this schema, so it has already been resolved.
+                    return resultSchemaReference;
                 }
             }
         }
 
+        if (schema.AnyOf is { Count: > 0 })
+        {
+            for (var i = 0; i < schema.AnyOf.Count; i++)
+            {
+                schema.AnyOf[i] = ResolveReferenceForSchema(document, schema.AnyOf[i], rootSchemaId, schemaId);
+            }
+        }
+
         if (schema.Properties is not null)
         {
             foreach (var property in schema.Properties)
@@ -326,39 +337,9 @@ internal static IOpenApiSchema ResolveReferenceForSchema(OpenApiDocument documen
             schema.Not = ResolveReferenceForSchema(document, schema.Not, rootSchemaId);
         }
 
-        // Handle schemas where the references have been inlined 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 (schema.Metadata is not null &&
-            schema.Metadata.TryGetValue(OpenApiConstants.RefId, out var refId) &&
-            refId is string refIdString)
+        if (resultSchemaReference is not null)
         {
-            if (schema.Metadata.TryGetValue(OpenApiConstants.SchemaId, out var schemaId) &&
-                schemaId is string schemaIdString)
-            {
-                return new OpenApiSchemaReference(schemaIdString, document);
-            }
-            var relativeSchemaId = $"#/components/schemas/{rootSchemaId}{refIdString.Replace("#", string.Empty)}";
-            return new OpenApiSchemaReference(relativeSchemaId, document);
-        }
-
-        // If we're resolving schemas for a top-level schema being referenced in the `components.schema` property
-        // we don't want to replace the top-level inline schema with a reference to itself. We want to replace
-        // inline schemas to reference schemas for all schemas referenced in the top-level schema though (such as
-        // `allOf`, `oneOf`, `anyOf`, `items`, `properties`, etc.) which is why `isTopLevel` is only set once.
-        if (schema is OpenApiSchema && schema.Metadata is not null &&
-            !schema.Metadata.ContainsKey(OpenApiConstants.RefId) &&
-            schema.Metadata.TryGetValue(OpenApiConstants.SchemaId, out var referenceId) &&
-            referenceId is string referenceIdString)
-        {
-            var targetReferenceId = baseSchemaId is not null
-                ? $"{baseSchemaId}{referenceIdString}"
-                : referenceIdString;
-            if (!string.IsNullOrEmpty(targetReferenceId))
-            {
-                return document.AddOpenApiSchemaByReference(targetReferenceId, schema);
-            }
+            return resultSchemaReference;
         }
 
         return schema;
@@ -466,9 +447,9 @@ private async Task InnerApplySchemaTransformersAsync(IOpenApiSchema inputSchema,
         }
     }
 
-    private JsonNode CreateSchema(OpenApiSchemaKey key)
+    private JsonNode CreateSchema(Type type)
     {
-        var schema = JsonSchemaExporter.GetJsonSchemaAsNode(_jsonSerializerOptions, key.Type, _configuration);
+        var schema = JsonSchemaExporter.GetJsonSchemaAsNode(_jsonSerializerOptions, type, _configuration);
         return ResolveReferences(schema, schema);
     }
 

src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs

@@ -984,8 +984,9 @@ private void VerifyOptionalEnum(OpenApiDocument document)
         var property = properties["status"];
 
         Assert.NotNull(property);
-        Assert.Equal(3, property.Enum.Count);
-        Assert.Equal("Approved", property.Default.GetValue<string>());
+        var statusReference = Assert.IsType<OpenApiSchemaReference>(property);
+        Assert.Equal(3, statusReference.RecursiveTarget.Enum.Count);
+        Assert.Equal("Approved", statusReference.Default.GetValue<string>());
     }
 
     [ApiController]

src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.RequestBodySchemas.cs

@@ -1122,6 +1122,38 @@ await VerifyOpenApiDocument(builder, document =>
         });
     }
 
+    // Test for: https://github.com/dotnet/aspnetcore/issues/64048
+    public static object[][] CircularReferencesWithArraysHandlers =>
+    [
+        [(CircularReferenceWithArrayRootOrderArrayFirst dto) => { }],
+        [(CircularReferenceWithArrayRootOrderArrayLast dto) => { }],
+    ];
+
+    [Theory]
+    [MemberData(nameof(CircularReferencesWithArraysHandlers))]
+    public async Task HandlesCircularReferencesWithArraysRegardlessOfPropertyOrder(Delegate requestHandler)
+    {
+        var builder = CreateBuilder();
+        builder.MapPost("/", requestHandler);
+
+        await VerifyOpenApiDocument(builder, (OpenApiDocument document) =>
+        {
+            Assert.NotNull(document.Components?.Schemas);
+            var schema = document.Components.Schemas["CircularReferenceWithArrayModel"];
+            Assert.Equal(JsonSchemaType.Object, schema.Type);
+            Assert.NotNull(schema.Properties);
+            Assert.Collection(schema.Properties,
+                property =>
+                {
+                    Assert.Equal("selfArray", property.Key);
+                    var arraySchema = Assert.IsType<OpenApiSchema>(property.Value);
+                    Assert.Equal(JsonSchemaType.Array, arraySchema.Type);
+                    var itemReference = Assert.IsType<OpenApiSchemaReference>(arraySchema.Items);
+                    Assert.Equal("#/components/schemas/CircularReferenceWithArrayModel", itemReference.Reference.ReferenceV3);
+                });
+        });
+    }
+
     // Test models for issue 61194
     private class Config
     {
@@ -1203,5 +1235,23 @@ private class ReferencedModel
     {
         public int Id { get; set; }
     }
+
+    // Test models for issue 64048
+    public class CircularReferenceWithArrayRootOrderArrayLast
+    {
+        public CircularReferenceWithArrayModel Item { get; set; } = null!;
+        public ICollection<CircularReferenceWithArrayModel> ItemArray { get; set; } = [];
+    }
+
+    public class CircularReferenceWithArrayRootOrderArrayFirst
+    {
+        public ICollection<CircularReferenceWithArrayModel> ItemArray { get; set; } = [];
+        public CircularReferenceWithArrayModel Item { get; set; } = null!;
+    }
+
+    public class CircularReferenceWithArrayModel
+    {
+        public ICollection<CircularReferenceWithArrayModel> SelfArray { get; set; } = [];
+    }
 }
 #nullable restore