Skip to content

Commit 204c63b

Browse files
aqeelataqeelat
committed
fix: address review feedback for $dynamicRef PR
- 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
1 parent 00c6a27 commit 204c63b

9 files changed

Lines changed: 70 additions & 25 deletions

File tree

‎src/Microsoft.OpenApi/Models/JsonSchemaReference.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,11 +336,15 @@ public class JsonSchemaReference : OpenApiReferenceWithDescription
336336
public IOpenApiSchema? Else { get; set; }
337337

338338
/// <summary>
339-
/// Indicates whether this reference was created from a bare $dynamicRef (no $ref).
339+
/// Indicates whether this reference represents a bare $dynamicRef (no $ref to a component).
340340
/// When true, serialization emits $dynamicRef instead of $ref, and Target resolution
341341
/// uses the $dynamicAnchor index rather than the $ref URI lookup.
342+
/// Computed from whether $dynamicRef is set and ReferenceV3 does not point to a component path.
342343
/// </summary>
343-
internal bool IsDynamicRefOnly { get; set; }
344+
internal bool IsDynamicRefOnly
345+
=> !string.IsNullOrEmpty(DynamicRef)
346+
&& ReferenceV3 is string refV3
347+
&& !refV3.StartsWith("#/components/", StringComparison.OrdinalIgnoreCase);
344348

345349
/// <summary>
346350
/// Parameterless constructor
@@ -414,7 +418,6 @@ public JsonSchemaReference(JsonSchemaReference reference) : base(reference)
414418
If = reference.If;
415419
Then = reference.Then;
416420
Else = reference.Else;
417-
IsDynamicRefOnly = reference.IsDynamicRefOnly;
418421
}
419422

420423
/// <inheritdoc/>

‎src/Microsoft.OpenApi/Reader/JsonNodeHelper.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,10 @@ public static Dictionary<string, HashSet<T>> CreateArrayMap<T>(this JsonNode? no
179179
}
180180

181181
/// <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.
185186
/// </summary>
186187
public static string? ExtractDynamicAnchorName(string? dynamicRef)
187188
{

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,6 @@ public static IOpenApiSchema LoadSchema(JsonNode node, OpenApiDocument hostDocum
485485
});
486486
result.Reference.ApplySchemaMetadata(referenceMetadata, jsonObject);
487487
result.Reference.SetJsonPointerPath(dynamicPointer, nodeLocation);
488-
result.Reference.IsDynamicRefOnly = true;
489488
return result;
490489
}
491490

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,6 @@ public static IOpenApiSchema LoadSchema(JsonNode node, OpenApiDocument hostDocum
485485
});
486486
result.Reference.ApplySchemaMetadata(referenceMetadata, jsonObject);
487487
result.Reference.SetJsonPointerPath(dynamicPointer, nodeLocation);
488-
result.Reference.IsDynamicRefOnly = true;
489488
return result;
490489
}
491490

‎src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ public class OpenApiWorkspace
1616
private readonly Dictionary<string, Uri> _documentsIdRegistry = new();
1717
private readonly Dictionary<Uri, Stream> _artifactsRegistry = new();
1818
private readonly Dictionary<Uri, IOpenApiReferenceable> _IOpenApiReferenceableRegistry = new(new UriWithFragmentEqualityComparer());
19-
private readonly Dictionary<OpenApiDocument, Dictionary<string, List<IOpenApiSchema>>> _dynamicAnchorRegistryByDocument = new();
20-
private readonly Dictionary<OpenApiDocument, Dictionary<string, List<IOpenApiSchema>>> _anchorRegistryByDocument = new();
19+
private Dictionary<OpenApiDocument, Dictionary<string, List<IOpenApiSchema>>>? _dynamicAnchorRegistryByDocument;
20+
private Dictionary<OpenApiDocument, Dictionary<string, List<IOpenApiSchema>>>? _anchorRegistryByDocument;
2121

2222
private sealed class UriWithFragmentEqualityComparer : IEqualityComparer<Uri>
2323
{
@@ -277,6 +277,7 @@ private void RegisterInlineAnchors(OpenApiDocument document)
277277
// shared `visited` set, mirroring RegisterAnchorsRecursive's schema-cycle guard.
278278
private void RegisterPathItemAnchors(OpenApiDocument document, IOpenApiPathItem pathItem, HashSet<object> visited)
279279
{
280+
if (pathItem is OpenApiPathItemReference) return;
280281
if (!visited.Add(pathItem)) return;
281282

282283
if (pathItem.Parameters is not null)
@@ -290,6 +291,7 @@ private void RegisterPathItemAnchors(OpenApiDocument document, IOpenApiPathItem
290291

291292
private void RegisterCallbackAnchors(OpenApiDocument document, IOpenApiCallback callback, HashSet<object> visited)
292293
{
294+
if (callback is OpenApiCallbackReference) return;
293295
if (!visited.Add(callback)) return;
294296

295297
if (callback.PathItems is not null)
@@ -319,16 +321,21 @@ private void RegisterOperationAnchors(OpenApiDocument document, OpenApiOperation
319321

320322
private void RegisterParameterAnchors(OpenApiDocument document, IOpenApiParameter parameter)
321323
{
324+
if (parameter is OpenApiParameterReference) return;
322325
if (parameter.Schema is not null)
323326
RegisterAnchors(document, parameter.Schema);
324327
RegisterMediaTypeSchemas(document, parameter.Content);
325328
}
326329

327330
private void RegisterRequestBodyAnchors(OpenApiDocument document, IOpenApiRequestBody requestBody)
328-
=> RegisterMediaTypeSchemas(document, requestBody.Content);
331+
{
332+
if (requestBody is OpenApiRequestBodyReference) return;
333+
RegisterMediaTypeSchemas(document, requestBody.Content);
334+
}
329335

330336
private void RegisterResponseAnchors(OpenApiDocument document, IOpenApiResponse response)
331337
{
338+
if (response is OpenApiResponseReference) return;
332339
RegisterMediaTypeSchemas(document, response.Content);
333340
if (response.Headers is not null)
334341
foreach (var header in response.Headers.Values.Where(h => h is not null))
@@ -337,6 +344,7 @@ private void RegisterResponseAnchors(OpenApiDocument document, IOpenApiResponse
337344

338345
private void RegisterHeaderAnchors(OpenApiDocument document, IOpenApiHeader header)
339346
{
347+
if (header is OpenApiHeaderReference) return;
340348
if (header.Schema is not null)
341349
RegisterAnchors(document, header.Schema);
342350
RegisterMediaTypeSchemas(document, header.Content);
@@ -345,7 +353,7 @@ private void RegisterHeaderAnchors(OpenApiDocument document, IOpenApiHeader head
345353
private void RegisterMediaTypeSchemas(OpenApiDocument document, IDictionary<string, IOpenApiMediaType>? content)
346354
{
347355
if (content is null) return;
348-
foreach (var mediaType in content.Values.Where(m => m is not null))
356+
foreach (var mediaType in content.Values.Where(m => m is not null and not OpenApiMediaTypeReference))
349357
{
350358
if (mediaType.Schema is not null)
351359
RegisterAnchors(document, mediaType.Schema);
@@ -617,7 +625,8 @@ private static IEnumerable<IOpenApiSchema> EnumerateMissingPropertiesChildren(IO
617625

618626
private void RegisterAnchor(OpenApiDocument document, string anchorName, IOpenApiSchema schema, bool isDynamic)
619627
{
620-
var registry = isDynamic ? _dynamicAnchorRegistryByDocument : _anchorRegistryByDocument;
628+
ref var registry = ref (isDynamic ? ref _dynamicAnchorRegistryByDocument : ref _anchorRegistryByDocument);
629+
registry ??= new();
621630
if (!registry.TryGetValue(document, out var anchors))
622631
{
623632
anchors = new(StringComparer.Ordinal);
@@ -639,7 +648,8 @@ private void RegisterAnchor(OpenApiDocument document, string anchorName, IOpenAp
639648
/// </summary>
640649
internal IOpenApiSchema? ResolveAnchor(OpenApiDocument hostDocument, string anchorName)
641650
{
642-
if (_anchorRegistryByDocument.TryGetValue(hostDocument, out var anchors) &&
651+
if (_anchorRegistryByDocument is not null &&
652+
_anchorRegistryByDocument.TryGetValue(hostDocument, out var anchors) &&
643653
anchors.TryGetValue(anchorName, out var candidates))
644654
return candidates.Count == 1 ? candidates[0] : null;
645655
return null;
@@ -651,11 +661,14 @@ private void RegisterAnchor(OpenApiDocument document, string anchorName, IOpenAp
651661
/// </summary>
652662
internal OpenApiDocument? FindDocumentByBaseUri(string documentUri)
653663
{
654-
return _dynamicAnchorRegistryByDocument.Keys
655-
.Concat(_anchorRegistryByDocument.Keys)
656-
.Distinct()
657-
.FirstOrDefault(doc => doc.BaseUri is not null
658-
&& doc.BaseUri.ToString().Equals(documentUri, StringComparison.OrdinalIgnoreCase));
664+
if (!Uri.TryCreate(documentUri, UriKind.Absolute, out var uri)) return null;
665+
if (_dynamicAnchorRegistryByDocument is not null)
666+
foreach (var doc in _dynamicAnchorRegistryByDocument.Keys)
667+
if (doc.BaseUri == uri) return doc;
668+
if (_anchorRegistryByDocument is not null)
669+
foreach (var doc in _anchorRegistryByDocument.Keys)
670+
if (doc.BaseUri == uri) return doc;
671+
return null;
659672
}
660673

661674
/// <summary>
@@ -670,7 +683,8 @@ private void RegisterAnchor(OpenApiDocument document, string anchorName, IOpenAp
670683
/// <returns>All candidate schemas, or an empty list if none.</returns>
671684
public IReadOnlyList<IOpenApiSchema> GetDynamicAnchorCandidates(OpenApiDocument hostDocument, string anchorName)
672685
{
673-
if (_dynamicAnchorRegistryByDocument.TryGetValue(hostDocument, out var anchors) &&
686+
if (_dynamicAnchorRegistryByDocument is not null &&
687+
_dynamicAnchorRegistryByDocument.TryGetValue(hostDocument, out var anchors) &&
674688
anchors.TryGetValue(anchorName, out var candidates))
675689
return candidates.AsReadOnly();
676690
return [];

‎test/Microsoft.OpenApi.Readers.Tests/OpenApiSchemaInterfaceJsonConverter.cs‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT license.
3+
4+
using System;
5+
using System.IO;
6+
using System.Text.Json;
7+
using System.Text.Json.Serialization;
8+
9+
namespace Microsoft.OpenApi.Readers.Tests
10+
{
11+
/// <summary>
12+
/// Serializes <see cref="IOpenApiSchema"/> values via the native OpenAPI writer,
13+
/// avoiding reflection on computed getters that create cycles for recursive $dynamicRef schemas.
14+
/// </summary>
15+
internal sealed class OpenApiSchemaInterfaceJsonConverter(Action<IOpenApiWriter, IOpenApiSchema> serialize) : JsonConverter<IOpenApiSchema>
16+
{
17+
public override IOpenApiSchema Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
18+
=> throw new NotSupportedException();
19+
20+
public override void Write(Utf8JsonWriter writer, IOpenApiSchema value, JsonSerializerOptions options)
21+
{
22+
using var tw = new StringWriter();
23+
var ow = new OpenApiJsonWriter(tw);
24+
serialize(ow, value);
25+
writer.WriteRawValue(tw.ToString(), skipInputValidation: true);
26+
}
27+
}
28+
}

‎test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiDocumentSerializationTests .cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ public async Task Serialize_DoesNotMutateDom(OpenApiSpecVersion version)
2727
// Act: Serialize using System.Text.Json
2828
var options = new JsonSerializerOptions
2929
{
30-
ReferenceHandler = System.Text.Json.Serialization.ReferenceHandler.Preserve,
3130
Converters =
3231
{
33-
new HttpMethodOperationDictionaryConverter()
32+
new HttpMethodOperationDictionaryConverter(),
33+
new OpenApiSchemaInterfaceJsonConverter(static (w, s) => s.SerializeAsV31(w))
3434
},
3535
};
3636
var originalSerialized = JsonSerializer.Serialize(doc, options);

‎test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiDynamicRefTests.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1341,7 +1341,7 @@ public void DynamicRefResolvesInObjectModelBuiltDocument()
13411341
["value"] = new OpenApiSchema { Type = JsonSchemaType.String },
13421342
["next"] = new OpenApiSchemaReference("node", doc)
13431343
{
1344-
Reference = { DynamicRef = "#node", IsDynamicRefOnly = true }
1344+
Reference = { DynamicRef = "#node" }
13451345
}
13461346
}
13471347
};
@@ -1350,6 +1350,7 @@ public void DynamicRefResolvesInObjectModelBuiltDocument()
13501350
doc.Workspace.RegisterComponents(doc);
13511351

13521352
var nextRef = Assert.IsType<OpenApiSchemaReference>(tree.Properties["next"]);
1353+
nextRef.Reference.SetJsonPointerPath("#node", "#/components/schemas/Tree/properties/next");
13531354
Assert.True(nextRef.Reference.IsDynamicRefOnly);
13541355
Assert.NotNull(nextRef.Target);
13551356
Assert.Same(tree, nextRef.Target);

‎test/Microsoft.OpenApi.Readers.Tests/V32Tests/OpenApiDocumentSerializationTests .cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ public async Task Serialize_DoesNotMutateDom(OpenApiSpecVersion version)
2828
// Act: Serialize using System.Text.Json
2929
var options = new JsonSerializerOptions
3030
{
31-
ReferenceHandler = System.Text.Json.Serialization.ReferenceHandler.Preserve,
3231
Converters =
3332
{
34-
new HttpMethodOperationDictionaryConverter()
33+
new HttpMethodOperationDictionaryConverter(),
34+
new OpenApiSchemaInterfaceJsonConverter(static (w, s) => s.SerializeAsV32(w))
3535
},
3636
};
3737
var originalSerialized = JsonSerializer.Serialize(doc, options);

0 commit comments

Comments
 (0)