fix: address PR review feedback

- Replace OpenApiSpecVersion branching with Action<IOpenApiWriter,
  IOpenApiSerializable> callback for future-proof serialization
- Add $defs context segment + try/catch error handling in V31/V32
  $defs parsing (mirrors ParseField error pattern)
- Convert all FluentAssertions calls to Assert.* per repo convention

Ref: #2896

Author: aqeelat

src/Microsoft.OpenApi/Models/JsonSchemaReference.cs

@@ -130,14 +130,14 @@ public JsonSchemaReference(JsonSchemaReference reference) : base(reference)
     /// <inheritdoc/>
     protected override void SerializeAdditionalV31Properties(IOpenApiWriter writer)
     {
-        SerializeAdditionalV3XProperties(writer, OpenApiSpecVersion.OpenApi3_1, base.SerializeAdditionalV31Properties);
+        SerializeAdditionalV3XProperties(writer, (w, e) => e.SerializeAsV31(w), base.SerializeAdditionalV31Properties);
     }
     /// <inheritdoc/>
     protected override void SerializeAdditionalV32Properties(IOpenApiWriter writer)
     {
-        SerializeAdditionalV3XProperties(writer, OpenApiSpecVersion.OpenApi3_2, base.SerializeAdditionalV32Properties);
+        SerializeAdditionalV3XProperties(writer, (w, e) => e.SerializeAsV32(w), base.SerializeAdditionalV32Properties);
     }
-    private void SerializeAdditionalV3XProperties(IOpenApiWriter writer, OpenApiSpecVersion version, Action<IOpenApiWriter> baseSerializer)
+    private void SerializeAdditionalV3XProperties(IOpenApiWriter writer, Action<IOpenApiWriter, IOpenApiSerializable> serializeCallback, Action<IOpenApiWriter> baseSerializer)
     {
         if (Type != ReferenceType.Schema) throw new InvalidOperationException(
             $"JsonSchemaReference can only be serialized for ReferenceType.Schema, but was {Type}.");
@@ -149,14 +149,7 @@ private void SerializeAdditionalV3XProperties(IOpenApiWriter writer, OpenApiSpec
         writer.WriteProperty(OpenApiConstants.DollarSchema, Schema?.ToString());
         writer.WriteProperty(OpenApiConstants.Comment, Comment);
         writer.WriteOptionalMap(OpenApiConstants.Vocabulary, Vocabulary, (w, s) => w.WriteValue(s));
-        if (version == OpenApiSpecVersion.OpenApi3_1)
-        {
-            writer.WriteOptionalMap(OpenApiConstants.Defs, Definitions, (w, s) => s.SerializeAsV31(w));
-        }
-        else
-        {
-            writer.WriteOptionalMap(OpenApiConstants.Defs, Definitions, (w, s) => s.SerializeAsV32(w));
-        }
+        writer.WriteOptionalMap(OpenApiConstants.Defs, Definitions, serializeCallback);
         writer.WriteProperty(OpenApiConstants.Anchor, Anchor);
         writer.WriteProperty(OpenApiConstants.DynamicRef, DynamicRef);
         writer.WriteProperty(OpenApiConstants.DynamicAnchor, DynamicAnchor);

src/Microsoft.OpenApi/Reader/V31/OpenApiSchemaDeserializer.cs

@@ -458,19 +458,36 @@ public static IOpenApiSchema LoadSchema(JsonNode node, OpenApiDocument hostDocum
             if (jsonObject.TryGetPropertyValue(OpenApiConstants.Defs, out var defsNode) && defsNode is JsonObject defsObj)
             {
                 var defs = new Dictionary<string, IOpenApiSchema>(StringComparer.Ordinal);
-                foreach (var kvp in defsObj)
+                context.StartObject(OpenApiConstants.Defs);
+                try
                 {
-                    if (kvp.Value is null) continue;
-                    context.StartObject(kvp.Key);
-                    try
+                    foreach (var kvp in defsObj)
                     {
-                        defs[kvp.Key] = LoadSchema(kvp.Value, hostDocument, context);
-                    }
-                    finally
-                    {
-                        context.EndObject();
+                        if (kvp.Value is null) continue;
+                        try
+                        {
+                            context.StartObject(kvp.Key);
+                            defs[kvp.Key] = LoadSchema(kvp.Value, hostDocument, context);
+                        }
+                        catch (OpenApiReaderException ex)
+                        {
+                            context.Diagnostic.Errors.Add(new(ex));
+                        }
+                        catch (OpenApiException ex)
+                        {
+                            ex.Pointer = context.GetLocation();
+                            context.Diagnostic.Errors.Add(new(ex));
+                        }
+                        finally
+                        {
+                            context.EndObject();
+                        }
                     }
                 }
+                finally
+                {
+                    context.EndObject();
+                }
                 if (defs.Count > 0)
                 {
                     result.Reference.Definitions = defs;

src/Microsoft.OpenApi/Reader/V32/OpenApiSchemaDeserializer.cs

@@ -458,19 +458,36 @@ public static IOpenApiSchema LoadSchema(JsonNode node, OpenApiDocument hostDocum
             if (jsonObject.TryGetPropertyValue(OpenApiConstants.Defs, out var defsNode) && defsNode is JsonObject defsObj)
             {
                 var defs = new Dictionary<string, IOpenApiSchema>(StringComparer.Ordinal);
-                foreach (var kvp in defsObj)
+                context.StartObject(OpenApiConstants.Defs);
+                try
                 {
-                    if (kvp.Value is null) continue;
-                    context.StartObject(kvp.Key);
-                    try
+                    foreach (var kvp in defsObj)
                     {
-                        defs[kvp.Key] = LoadSchema(kvp.Value, hostDocument, context);
-                    }
-                    finally
-                    {
-                        context.EndObject();
+                        if (kvp.Value is null) continue;
+                        try
+                        {
+                            context.StartObject(kvp.Key);
+                            defs[kvp.Key] = LoadSchema(kvp.Value, hostDocument, context);
+                        }
+                        catch (OpenApiReaderException ex)
+                        {
+                            context.Diagnostic.Errors.Add(new(ex));
+                        }
+                        catch (OpenApiException ex)
+                        {
+                            ex.Pointer = context.GetLocation();
+                            context.Diagnostic.Errors.Add(new(ex));
+                        }
+                        finally
+                        {
+                            context.EndObject();
+                        }
                     }
                 }
+                finally
+                {
+                    context.EndObject();
+                }
                 if (defs.Count > 0)
                 {
                     result.Reference.Definitions = defs;

test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiSchemaTests.cs

@@ -969,12 +969,12 @@ public async Task ParseSchemaReferencePreservesJsonSchema2020KeywordSiblings()
             var referencing = result.Document.Components!.Schemas["Referencing"];
 
             // Assert — siblings are preserved on the OpenApiSchemaReference
-            referencing.Should().BeOfType<OpenApiSchemaReference>();
-            referencing.Description.Should().Be("Sibling description");
-            referencing.DynamicAnchor.Should().Be("anchor");
-            referencing.Definitions.Should().NotBeNull();
-            referencing.Definitions!.Should().ContainKey("sibling");
-            referencing.Definitions["sibling"].DynamicAnchor.Should().Be("inner");
+            Assert.IsType<OpenApiSchemaReference>(referencing);
+            Assert.Equal("Sibling description", referencing.Description);
+            Assert.Equal("anchor", referencing.DynamicAnchor);
+            Assert.NotNull(referencing.Definitions);
+            Assert.True(referencing.Definitions!.ContainsKey("sibling"));
+            Assert.Equal("inner", referencing.Definitions["sibling"].DynamicAnchor);
         }
 
         [Fact]
@@ -1015,11 +1015,11 @@ public async Task SerializeSchemaReferencePreservesJsonSchema2020KeywordSiblings
             var roundTripResult = await OpenApiDocument.LoadAsync(roundTripStream, "yaml", SettingsFixture.ReaderSettings);
             var referencing = roundTripResult.Document.Components!.Schemas["Referencing"];
 
-            referencing.Should().BeOfType<OpenApiSchemaReference>();
-            referencing.DynamicAnchor.Should().Be("anchor");
-            referencing.Definitions.Should().NotBeNull();
-            referencing.Definitions!.Should().ContainKey("itemType");
-            referencing.Definitions["itemType"].DynamicAnchor.Should().Be("itemType");
+            Assert.IsType<OpenApiSchemaReference>(referencing);
+            Assert.Equal("anchor", referencing.DynamicAnchor);
+            Assert.NotNull(referencing.Definitions);
+            Assert.True(referencing.Definitions!.ContainsKey("itemType"));
+            Assert.Equal("itemType", referencing.Definitions["itemType"].DynamicAnchor);
         }
 
         [Fact]
@@ -1051,12 +1051,12 @@ public async Task ParseSchemaReferencePreservesScalarKeywordSiblings()
             var referencing = result.Document.Components!.Schemas["Referencing"];
 
             // Assert
-            referencing.Should().BeOfType<OpenApiSchemaReference>();
-            referencing.Id.Should().Be("https://example.com/referencing.json");
-            referencing.Schema.Should().Be(new Uri("https://json-schema.org/draft/2020-12/schema"));
-            referencing.Comment.Should().Be("A comment sibling");
-            ((IOpenApiSchemaMissingProperties)referencing).Anchor.Should().Be("myAnchor");
-            referencing.DynamicRef.Should().Be("#myAnchor");
+            Assert.IsType<OpenApiSchemaReference>(referencing);
+            Assert.Equal("https://example.com/referencing.json", referencing.Id);
+            Assert.Equal(new Uri("https://json-schema.org/draft/2020-12/schema"), referencing.Schema);
+            Assert.Equal("A comment sibling", referencing.Comment);
+            Assert.Equal("myAnchor", ((IOpenApiSchemaMissingProperties)referencing).Anchor);
+            Assert.Equal("#myAnchor", referencing.DynamicRef);
         }
 
         [Fact]
@@ -1094,12 +1094,12 @@ public async Task SerializeSchemaReferencePreservesScalarKeywordSiblings()
             var roundTripResult = await OpenApiDocument.LoadAsync(roundTripStream, "yaml", SettingsFixture.ReaderSettings);
             var referencing = roundTripResult.Document.Components!.Schemas["Referencing"];
 
-            referencing.Should().BeOfType<OpenApiSchemaReference>();
-            referencing.Id.Should().Be("https://example.com/referencing.json");
-            referencing.Schema.Should().Be(new Uri("https://json-schema.org/draft/2020-12/schema"));
-            referencing.Comment.Should().Be("A comment sibling");
-            ((IOpenApiSchemaMissingProperties)referencing).Anchor.Should().Be("myAnchor");
-            referencing.DynamicRef.Should().Be("#myAnchor");
+            Assert.IsType<OpenApiSchemaReference>(referencing);
+            Assert.Equal("https://example.com/referencing.json", referencing.Id);
+            Assert.Equal(new Uri("https://json-schema.org/draft/2020-12/schema"), referencing.Schema);
+            Assert.Equal("A comment sibling", referencing.Comment);
+            Assert.Equal("myAnchor", ((IOpenApiSchemaMissingProperties)referencing).Anchor);
+            Assert.Equal("#myAnchor", referencing.DynamicRef);
         }
 
         [Fact]
@@ -1129,11 +1129,11 @@ public async Task ParseSchemaReferencePreservesVocabularySibling()
             var referencing = result.Document.Components!.Schemas["Referencing"];
 
             // Assert
-            referencing.Should().BeOfType<OpenApiSchemaReference>();
-            referencing.Vocabulary.Should().NotBeNull();
-            referencing.Vocabulary!.Should().HaveCount(2);
-            referencing.Vocabulary["https://json-schema.org/draft/2020-12/vocab/core"].Should().BeTrue();
-            referencing.Vocabulary["https://json-schema.org/draft/2020-12/vocab/applicator"].Should().BeFalse();
+            Assert.IsType<OpenApiSchemaReference>(referencing);
+            Assert.NotNull(referencing.Vocabulary);
+            Assert.Equal(2, referencing.Vocabulary!.Count);
+            Assert.True(referencing.Vocabulary["https://json-schema.org/draft/2020-12/vocab/core"]);
+            Assert.False(referencing.Vocabulary["https://json-schema.org/draft/2020-12/vocab/applicator"]);
         }
 
         [Fact]
@@ -1179,14 +1179,14 @@ public async Task ParseSchemaReferencePreservesDynamicAnchorInsideDefsInAllOf()
             // allOf[0] is a regular OpenApiSchema (no $ref at top level), so $defs is parsed normally.
             // The nested contentType schema is an OpenApiSchemaReference ($ref: Asset),
             // and its $dynamicAnchor sibling must be preserved.
-            assetPaged.AllOf.Should().NotBeNull();
-            assetPaged.AllOf!.Count.Should().Be(2);
+            Assert.NotNull(assetPaged.AllOf);
+            Assert.Equal(2, assetPaged.AllOf!.Count);
             var defsHolder = assetPaged.AllOf[0];
-            defsHolder.Definitions.Should().NotBeNull();
-            defsHolder.Definitions!.Should().ContainKey("contentType");
+            Assert.NotNull(defsHolder.Definitions);
+            Assert.True(defsHolder.Definitions!.ContainsKey("contentType"));
             var contentType = defsHolder.Definitions["contentType"];
-            contentType.Should().BeOfType<OpenApiSchemaReference>();
-            contentType.DynamicAnchor.Should().Be("contentType");
+            Assert.IsType<OpenApiSchemaReference>(contentType);
+            Assert.Equal("contentType", contentType.DynamicAnchor);
         }
 
         [Fact]
@@ -1221,11 +1221,11 @@ public async Task EmptySiblingCollectionsFallThroughToTarget()
             var referencing = result.Document.Components!.Schemas["Referencing"];
 
             // Assert — empty siblings fall through to Target's values
-            referencing.Should().BeOfType<OpenApiSchemaReference>();
-            referencing.Definitions.Should().NotBeNull();
-            referencing.Definitions!.Should().ContainKey("targetDef");
-            referencing.Vocabulary.Should().NotBeNull();
-            referencing.Vocabulary!.Should().ContainKey("https://json-schema.org/draft/2020-12/vocab/core");
+            Assert.IsType<OpenApiSchemaReference>(referencing);
+            Assert.NotNull(referencing.Definitions);
+            Assert.True(referencing.Definitions!.ContainsKey("targetDef"));
+            Assert.NotNull(referencing.Vocabulary);
+            Assert.True(referencing.Vocabulary!.ContainsKey("https://json-schema.org/draft/2020-12/vocab/core"));
         }
 
         [Fact]
@@ -1261,9 +1261,9 @@ public async Task SiblingsOnRefAreDroppedForOpenApi30()
             var referencing = result.Document.Components!.Schemas["Referencing"];
 
             // Assert — siblings are dropped for 3.0 (per spec: $ref siblings MUST be ignored)
-            referencing.Should().BeOfType<OpenApiSchemaReference>();
-            referencing.DynamicAnchor.Should().BeNull();
-            referencing.Definitions?.Should().BeNull();
+            Assert.IsType<OpenApiSchemaReference>(referencing);
+            Assert.Null(referencing.DynamicAnchor);
+            Assert.Null(referencing.Definitions);
         }
 
         [Fact]
@@ -1299,11 +1299,11 @@ public async Task SerializeSchemaReferencePreservesVocabularySibling()
             var roundTripResult = await OpenApiDocument.LoadAsync(roundTripStream, "yaml", SettingsFixture.ReaderSettings);
             var referencing = roundTripResult.Document.Components!.Schemas["Referencing"];
 
-            referencing.Should().BeOfType<OpenApiSchemaReference>();
-            referencing.Vocabulary.Should().NotBeNull();
-            referencing.Vocabulary!.Should().HaveCount(2);
-            referencing.Vocabulary["https://json-schema.org/draft/2020-12/vocab/core"].Should().BeTrue();
-            referencing.Vocabulary["https://json-schema.org/draft/2020-12/vocab/applicator"].Should().BeFalse();
+            Assert.IsType<OpenApiSchemaReference>(referencing);
+            Assert.NotNull(referencing.Vocabulary);
+            Assert.Equal(2, referencing.Vocabulary!.Count);
+            Assert.True(referencing.Vocabulary["https://json-schema.org/draft/2020-12/vocab/core"]);
+            Assert.False(referencing.Vocabulary["https://json-schema.org/draft/2020-12/vocab/applicator"]);
         }
 
         [Fact]
@@ -1336,11 +1336,11 @@ public async Task CreateShallowCopyPreservesKeywordSiblings()
             var copy = referencing.CreateShallowCopy();
 
             // Assert — CreateShallowCopy preserves sibling values via the JsonSchemaReference copy constructor
-            copy.Should().BeOfType<OpenApiSchemaReference>();
-            copy.DynamicAnchor.Should().Be("anchor");
-            copy.Definitions.Should().NotBeNull();
-            copy.Definitions!.Should().ContainKey("sibling");
-            copy.Definitions["sibling"].DynamicAnchor.Should().Be("inner");
+            Assert.IsType<OpenApiSchemaReference>(copy);
+            Assert.Equal("anchor", copy.DynamicAnchor);
+            Assert.NotNull(copy.Definitions);
+            Assert.True(copy.Definitions!.ContainsKey("sibling"));
+            Assert.Equal("inner", copy.Definitions["sibling"].DynamicAnchor);
         }
     }
 }