fix: address review feedback for $dynamicRef PR

aqeelat · aqeelat · commit cc6b9645a940 · 2026-07-06T21:48:26.000+03:00
- Replace IsDynamicRefOnly field with computed getter based on DynamicRef and ReferenceV3 not pointing to #/components/ (per @baywet feedback) - Add reference-holder guards in anchor registration walk to prevent crossing document boundaries through OpenApiParameterReference, OpenApiResponseReference, etc. (per Copilot review) - Lazy-init anchor registry dictionaries to fix EmptyDocument performance regression (176 bytes → baseline) - Use Uri equality instead of ToString().Equals() in FindDocumentByBaseUri - Tighten ExtractDynamicAnchorName doc comment to describe string splitting behavior, not resolution semantics
diff --git a/src/Microsoft.OpenApi/Models/JsonSchemaReference.cs b/src/Microsoft.OpenApi/Models/JsonSchemaReference.cs
@@ -336,11 +336,15 @@ public class JsonSchemaReference : OpenApiReferenceWithDescription
     public IOpenApiSchema? Else { get; set; }
 
     /// <summary>
-    /// Indicates whether this reference was created from a bare $dynamicRef (no $ref).
+    /// Indicates whether this reference represents a bare $dynamicRef (no $ref to a component).
     /// When true, serialization emits $dynamicRef instead of $ref, and Target resolution
     /// uses the $dynamicAnchor index rather than the $ref URI lookup.
+    /// Computed from whether $dynamicRef is set and ReferenceV3 does not point to a component path.
     /// </summary>
-    internal bool IsDynamicRefOnly { get; set; }
+    internal bool IsDynamicRefOnly
+        => !string.IsNullOrEmpty(DynamicRef)
+           && ReferenceV3 is string refV3
+           && !refV3.StartsWith("#/components/", StringComparison.OrdinalIgnoreCase);
 
     /// <summary>
     /// Parameterless constructor
@@ -414,7 +418,6 @@ public JsonSchemaReference(JsonSchemaReference reference) : base(reference)
         If = reference.If;
         Then = reference.Then;
         Else = reference.Else;
-        IsDynamicRefOnly = reference.IsDynamicRefOnly;
     }
 
     /// <inheritdoc/>
diff --git a/src/Microsoft.OpenApi/Reader/JsonNodeHelper.cs b/src/Microsoft.OpenApi/Reader/JsonNodeHelper.cs
@@ -179,9 +179,10 @@ public static Dictionary<string, HashSet<T>> CreateArrayMap<T>(this JsonNode? no
         }
 
         /// <summary>
-        /// Extracts the bare anchor name from a $dynamicRef value.
-        /// Handles fragment-only (#meta), absolute-URI (https://example.com#meta), and bare (meta) forms.
-        /// Returns null for null/empty input. Returns empty string for bare "#" (root reference).
+        /// Splits a $dynamicRef value on the last '#' and returns the fragment part.
+        /// For "#meta" returns "meta". For "https://example.com#meta" returns "meta".
+        /// For values with no '#' returns the original string. Returns null for null/empty input.
+        /// This is a string operation only — it does not validate or resolve the reference.
         /// </summary>
         public static string? ExtractDynamicAnchorName(string? dynamicRef)
         {
diff --git a/src/Microsoft.OpenApi/Reader/V31/OpenApiSchemaDeserializer.cs b/src/Microsoft.OpenApi/Reader/V31/OpenApiSchemaDeserializer.cs
@@ -485,7 +485,6 @@ public static IOpenApiSchema LoadSchema(JsonNode node, OpenApiDocument hostDocum
                 });
             result.Reference.ApplySchemaMetadata(referenceMetadata, jsonObject);
             result.Reference.SetJsonPointerPath(dynamicPointer, nodeLocation);
-            result.Reference.IsDynamicRefOnly = true;
             return result;
         }
 
diff --git a/src/Microsoft.OpenApi/Reader/V32/OpenApiSchemaDeserializer.cs b/src/Microsoft.OpenApi/Reader/V32/OpenApiSchemaDeserializer.cs
@@ -485,7 +485,6 @@ public static IOpenApiSchema LoadSchema(JsonNode node, OpenApiDocument hostDocum
                 });
             result.Reference.ApplySchemaMetadata(referenceMetadata, jsonObject);
             result.Reference.SetJsonPointerPath(dynamicPointer, nodeLocation);
-            result.Reference.IsDynamicRefOnly = true;
             return result;
         }
 
diff --git a/src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs b/src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs
@@ -16,8 +16,8 @@ public class OpenApiWorkspace
         private readonly Dictionary<string, Uri> _documentsIdRegistry = new();
         private readonly Dictionary<Uri, Stream> _artifactsRegistry = new();        
         private readonly Dictionary<Uri, IOpenApiReferenceable> _IOpenApiReferenceableRegistry = new(new UriWithFragmentEqualityComparer());
-        private readonly Dictionary<OpenApiDocument, Dictionary<string, List<IOpenApiSchema>>> _dynamicAnchorRegistryByDocument = new();
-        private readonly Dictionary<OpenApiDocument, Dictionary<string, List<IOpenApiSchema>>> _anchorRegistryByDocument = new();
+        private Dictionary<OpenApiDocument, Dictionary<string, List<IOpenApiSchema>>>? _dynamicAnchorRegistryByDocument;
+        private Dictionary<OpenApiDocument, Dictionary<string, List<IOpenApiSchema>>>? _anchorRegistryByDocument;
 
         private sealed class UriWithFragmentEqualityComparer : IEqualityComparer<Uri>
         {
@@ -280,6 +280,7 @@ private void RegisterInlineAnchors(OpenApiDocument document)
         // shared `visited` set, mirroring RegisterAnchorsRecursive's schema-cycle guard.
         private void RegisterPathItemAnchors(OpenApiDocument document, IOpenApiPathItem pathItem, HashSet<object> visited)
         {
+            if (pathItem is OpenApiPathItemReference) return;
             if (!visited.Add(pathItem)) return;
 
             if (pathItem.Parameters is not null)
@@ -293,6 +294,7 @@ private void RegisterPathItemAnchors(OpenApiDocument document, IOpenApiPathItem
 
         private void RegisterCallbackAnchors(OpenApiDocument document, IOpenApiCallback callback, HashSet<object> visited)
         {
+            if (callback is OpenApiCallbackReference) return;
             if (!visited.Add(callback)) return;
 
             if (callback.PathItems is not null)
@@ -322,16 +324,21 @@ private void RegisterOperationAnchors(OpenApiDocument document, OpenApiOperation
 
         private void RegisterParameterAnchors(OpenApiDocument document, IOpenApiParameter parameter)
         {
+            if (parameter is OpenApiParameterReference) return;
             if (parameter.Schema is not null)
                 RegisterAnchors(document, parameter.Schema);
             RegisterMediaTypeSchemas(document, parameter.Content);
         }
 
         private void RegisterRequestBodyAnchors(OpenApiDocument document, IOpenApiRequestBody requestBody)
-            => RegisterMediaTypeSchemas(document, requestBody.Content);
+        {
+            if (requestBody is OpenApiRequestBodyReference) return;
+            RegisterMediaTypeSchemas(document, requestBody.Content);
+        }
 
         private void RegisterResponseAnchors(OpenApiDocument document, IOpenApiResponse response)
         {
+            if (response is OpenApiResponseReference) return;
             RegisterMediaTypeSchemas(document, response.Content);
             if (response.Headers is not null)
                 foreach (var header in response.Headers.Values.Where(h => h is not null))
@@ -340,6 +347,7 @@ private void RegisterResponseAnchors(OpenApiDocument document, IOpenApiResponse
 
         private void RegisterHeaderAnchors(OpenApiDocument document, IOpenApiHeader header)
         {
+            if (header is OpenApiHeaderReference) return;
             if (header.Schema is not null)
                 RegisterAnchors(document, header.Schema);
             RegisterMediaTypeSchemas(document, header.Content);
@@ -348,7 +356,7 @@ private void RegisterHeaderAnchors(OpenApiDocument document, IOpenApiHeader head
         private void RegisterMediaTypeSchemas(OpenApiDocument document, IDictionary<string, IOpenApiMediaType>? content)
         {
             if (content is null) return;
-            foreach (var mediaType in content.Values.Where(m => m is not null))
+            foreach (var mediaType in content.Values.Where(m => m is not null and not OpenApiMediaTypeReference))
             {
                 if (mediaType.Schema is not null)
                     RegisterAnchors(document, mediaType.Schema);
@@ -546,7 +554,8 @@ private static IEnumerable<IOpenApiSchema> EnumerateMissingPropertiesChildren(IO
 
         private void RegisterAnchor(OpenApiDocument document, string anchorName, IOpenApiSchema schema, bool isDynamic)
         {
-            var registry = isDynamic ? _dynamicAnchorRegistryByDocument : _anchorRegistryByDocument;
+            ref var registry = ref (isDynamic ? ref _dynamicAnchorRegistryByDocument : ref _anchorRegistryByDocument);
+            registry ??= new();
             if (!registry.TryGetValue(document, out var anchors))
             {
                 anchors = new(StringComparer.Ordinal);
@@ -568,7 +577,8 @@ private void RegisterAnchor(OpenApiDocument document, string anchorName, IOpenAp
         /// </summary>
         internal IOpenApiSchema? ResolveAnchor(OpenApiDocument hostDocument, string anchorName)
         {
-            if (_anchorRegistryByDocument.TryGetValue(hostDocument, out var anchors) &&
+            if (_anchorRegistryByDocument is not null &&
+                _anchorRegistryByDocument.TryGetValue(hostDocument, out var anchors) &&
                 anchors.TryGetValue(anchorName, out var candidates))
                 return candidates.Count == 1 ? candidates[0] : null;
             return null;
@@ -580,11 +590,14 @@ private void RegisterAnchor(OpenApiDocument document, string anchorName, IOpenAp
         /// </summary>
         internal OpenApiDocument? FindDocumentByBaseUri(string documentUri)
         {
-            return _dynamicAnchorRegistryByDocument.Keys
-                .Concat(_anchorRegistryByDocument.Keys)
-                .Distinct()
-                .FirstOrDefault(doc => doc.BaseUri is not null
-                    && doc.BaseUri.ToString().Equals(documentUri, StringComparison.OrdinalIgnoreCase));
+            if (!Uri.TryCreate(documentUri, UriKind.Absolute, out var uri)) return null;
+            if (_dynamicAnchorRegistryByDocument is not null)
+                foreach (var doc in _dynamicAnchorRegistryByDocument.Keys)
+                    if (doc.BaseUri == uri) return doc;
+            if (_anchorRegistryByDocument is not null)
+                foreach (var doc in _anchorRegistryByDocument.Keys)
+                    if (doc.BaseUri == uri) return doc;
+            return null;
         }
 
         /// <summary>
@@ -599,7 +612,8 @@ private void RegisterAnchor(OpenApiDocument document, string anchorName, IOpenAp
         /// <returns>All candidate schemas, or an empty list if none.</returns>
         public IReadOnlyList<IOpenApiSchema> GetDynamicAnchorCandidates(OpenApiDocument hostDocument, string anchorName)
         {
-            if (_dynamicAnchorRegistryByDocument.TryGetValue(hostDocument, out var anchors) &&
+            if (_dynamicAnchorRegistryByDocument is not null &&
+                _dynamicAnchorRegistryByDocument.TryGetValue(hostDocument, out var anchors) &&
                 anchors.TryGetValue(anchorName, out var candidates))
                 return candidates.AsReadOnly();
             return [];
diff --git a/test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiDynamicRefTests.cs b/test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiDynamicRefTests.cs
@@ -1341,7 +1341,7 @@ public void DynamicRefResolvesInObjectModelBuiltDocument()
                 ["value"] = new OpenApiSchema { Type = JsonSchemaType.String },
                 ["next"] = new OpenApiSchemaReference("node", doc)
                 {
-                    Reference = { DynamicRef = "#node", IsDynamicRefOnly = true }
+                    Reference = { DynamicRef = "#node" }
                 }
             }
         };
@@ -1350,6 +1350,7 @@ public void DynamicRefResolvesInObjectModelBuiltDocument()
         doc.Workspace.RegisterComponents(doc);
 
         var nextRef = Assert.IsType<OpenApiSchemaReference>(tree.Properties["next"]);
+        nextRef.Reference.SetJsonPointerPath("#node", "#/components/schemas/Tree/properties/next");
         Assert.True(nextRef.Reference.IsDynamicRefOnly);
         Assert.NotNull(nextRef.Target);
         Assert.Same(tree, nextRef.Target);

Original file line number	Diff line number	Diff line change
`@@ -179,9 +179,10 @@ public static Dictionary<string, HashSet<T>> CreateArrayMap<T>(this JsonNode? no`
`179`	`179`	`}`
`180`	`180`
`181`	`181`	`/// <summary>`
`182`		`- /// Extracts the bare anchor name from a $dynamicRef value.`
`183`		`- /// Handles fragment-only (#meta), absolute-URI (https://example.com#meta), and bare (meta) forms.`
`184`		`- /// Returns null for null/empty input. Returns empty string for bare "#" (root reference).`
	`182`	`+ /// Splits a $dynamicRef value on the last '#' and returns the fragment part.`
	`183`	`+ /// For "#meta" returns "meta". For "https://example.com#meta" returns "meta".`
	`184`	`+ /// For values with no '#' returns the original string. Returns null for null/empty input.`
	`185`	`+ /// This is a string operation only — it does not validate or resolve the reference.`
`185`	`186`	`/// </summary>`
`186`	`187`	`public static string? ExtractDynamicAnchorName(string? dynamicRef)`
`187`	`188`	`{`
Original file line number	Diff line number	Diff line change
`@@ -485,7 +485,6 @@ public static IOpenApiSchema LoadSchema(JsonNode node, OpenApiDocument hostDocum`
`485`	`485`	`});`
`486`	`486`	`result.Reference.ApplySchemaMetadata(referenceMetadata, jsonObject);`
`487`	`487`	`result.Reference.SetJsonPointerPath(dynamicPointer, nodeLocation);`
`488`		`- result.Reference.IsDynamicRefOnly = true;`
`489`	`488`	`return result;`
`490`	`489`	`}`
`491`	`490`
Original file line number	Diff line number	Diff line change
`@@ -1341,7 +1341,7 @@ public void DynamicRefResolvesInObjectModelBuiltDocument()`
`1341`	`1341`	`["value"] = new OpenApiSchema { Type = JsonSchemaType.String },`
`1342`	`1342`	`["next"] = new OpenApiSchemaReference("node", doc)`
`1343`	`1343`	`{`
`1344`		`- Reference = { DynamicRef = "#node", IsDynamicRefOnly = true }`
	`1344`	`+ Reference = { DynamicRef = "#node" }`
`1345`	`1345`	`}`
`1346`	`1346`	`}`
`1347`	`1347`	`};`
`@@ -1350,6 +1350,7 @@ public void DynamicRefResolvesInObjectModelBuiltDocument()`
`1350`	`1350`	`doc.Workspace.RegisterComponents(doc);`
`1351`	`1351`
`1352`	`1352`	`var nextRef = Assert.IsType<OpenApiSchemaReference>(tree.Properties["next"]);`
	`1353`	`+ nextRef.Reference.SetJsonPointerPath("#node", "#/components/schemas/Tree/properties/next");`
`1353`	`1354`	`Assert.True(nextRef.Reference.IsDynamicRefOnly);`
`1354`	`1355`	`Assert.NotNull(nextRef.Target);`
`1355`	`1356`	`Assert.Same(tree, nextRef.Target);`