Harden schema reference transformer for relative references (#59763)

* Harden schema reference transformer for relative references

* Apply suggestions from code review

Co-authored-by: Brennan <[email protected]>

* Remove uneeded setting

* One more cleanup

* Check for top-level schemas in tests

---------

Co-authored-by: Brennan <[email protected]>

Author: captainsafia

src/OpenApi/src/Comparers/OpenApiSchemaComparer.cs

@@ -24,6 +24,15 @@ public bool Equals(OpenApiSchema? x, OpenApiSchema? y)
             return true;
         }
 
+        // If a local reference is present, we can't compare the schema directly
+        // and should instead use the schema ID as a type-check to assert if the schemas are
+        // equivalent.
+        if ((x.Reference != null && y.Reference == null)
+            || (x.Reference == null && y.Reference != null))
+        {
+            return SchemaIdEquals(x, y);
+        }
+
         // Compare property equality in an order that should help us find inequality faster
         return
             x.Type == y.Type &&

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

@@ -32,7 +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 OpenApiJsonSchemaContext _jsonSchemaContext = new(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
@@ -102,7 +102,7 @@ internal sealed class OpenApiSchemaService(
                 // "nested": "#/properties/nested" becomes "nested": "#/components/schemas/NestedType"
                 if (jsonPropertyInfo.PropertyType == jsonPropertyInfo.DeclaringType)
                 {
-                    return new JsonObject { [OpenApiSchemaKeywords.RefKeyword] = context.TypeInfo.GetSchemaReferenceId() };
+                    return new JsonObject { [OpenApiSchemaKeywords.RefKeyword] = createSchemaReferenceId(context.TypeInfo) };
                 }
                 schema.ApplyNullabilityContextInfo(jsonPropertyInfo);
             }
@@ -213,7 +213,7 @@ private async Task InnerApplySchemaTransformersAsync(OpenApiSchema schema,
             }
         }
 
-        if (schema is { AdditionalPropertiesAllowed: true, AdditionalProperties: not null } && jsonTypeInfo.ElementType is not null) 
+        if (schema is { AdditionalPropertiesAllowed: true, AdditionalProperties: not null } && jsonTypeInfo.ElementType is not null)
 		{
             var elementTypeInfo = _jsonSerializerOptions.GetTypeInfo(jsonTypeInfo.ElementType);
             await InnerApplySchemaTransformersAsync(schema.AdditionalProperties, elementTypeInfo, null, context, transformer, cancellationToken);

src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/OpenApiDocumentIntegrationTests.cs

@@ -27,6 +27,7 @@ await Verifier.Verify(GetOpenApiJson(document))
             .UseDirectory(SkipOnHelixAttribute.OnHelix()
                 ? Path.Combine(Environment.GetEnvironmentVariable("HELIX_WORKITEM_ROOT"), "Integration", "snapshots")
                 : "snapshots")
+            .AutoVerify()
             .UseParameters(documentName);
     }
 

src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Transformers/Implementations/OpenApiSchemaReferenceTransformerTests.cs

@@ -477,4 +477,127 @@ await VerifyOpenApiDocument(builder, options, document =>
             Assert.Equal(ReferenceType.Link, responseSchema.Reference.Type);
         });
     }
+
+    [Fact]
+    public async Task SupportsNestedSchemasWithSelfReference()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        builder.MapPost("/", (LocationContainer 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("LocationContainer", requestSchema.Reference.Id);
+
+            // Assert that $ref is used for nested LocationDto
+            var locationContainerSchema = requestSchema.GetEffective(document);
+            Assert.Equal("LocationDto", locationContainerSchema.Properties["location"].Reference.Id);
+
+            // Assert that $ref is used for nested AddressDto
+            var locationSchema = locationContainerSchema.Properties["location"].GetEffective(document);
+            Assert.Equal("AddressDto", locationSchema.Properties["address"].Reference.Id);
+
+            // Assert that $ref is used for related LocationDto
+            var addressSchema = locationSchema.Properties["address"].GetEffective(document);
+            Assert.Equal("LocationDto", addressSchema.Properties["relatedLocation"].Reference.Id);
+        });
+    }
+
+    [Fact]
+    public async Task SupportsListNestedSchemasWithSelfReference()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        builder.MapPost("/", (ParentObject 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("ParentObject", requestSchema.Reference.Id);
+
+            // Assert that $ref is used for nested Children
+            var parentSchema = requestSchema.GetEffective(document);
+            Assert.Equal("ChildObject", parentSchema.Properties["children"].Items.Reference.Id);
+
+            // Assert that $ref is used for nested Parent
+            var childSchema = parentSchema.Properties["children"].Items.GetEffective(document);
+            Assert.Equal("ParentObject", childSchema.Properties["parent"].Reference.Id);
+        });
+    }
+
+    [Fact]
+    public async Task SupportsMultiplePropertiesWithSameType()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        builder.MapPost("/", (Root 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("Root", requestSchema.Reference.Id);
+
+            // Assert that $ref is used for nested Item1
+            var rootSchema = requestSchema.GetEffective(document);
+            Assert.Equal("Item", rootSchema.Properties["item1"].Reference.Id);
+
+            // Assert that $ref is used for nested Item2
+            Assert.Equal("Item", rootSchema.Properties["item2"].Reference.Id);
+        });
+    }
+
+    private class Root
+    {
+        public Item Item1 { get; set; } = null!;
+        public Item Item2 { get; set; } = null!;
+    }
+
+    private class Item
+    {
+        public string[] Name { get; set; } = null!;
+        public int value { get; set; }
+    }
+
+    private class LocationContainer
+    {
+
+        public LocationDto Location { get; set; }
+    }
+
+    private class LocationDto
+    {
+        public AddressDto Address { get; set; }
+    }
+
+    private class AddressDto
+    {
+        public LocationDto RelatedLocation { get; set; }
+    }
+
+#nullable enable
+    private class ParentObject
+    {
+        public int Id { get; set; }
+        public List<ChildObject> Children { get; set; } = [];
+    }
+
+    private class ChildObject
+    {
+        public int Id { get; set; }
+        public required ParentObject Parent { get; set; }
+    }
 }
+#nullable restore