feat(schema): resolve bare $dynamicRef via $dynamicAnchor index#2913
feat(schema): resolve bare $dynamicRef via $dynamicAnchor index#2913aqeelat wants to merge 1 commit into
Conversation
e4c3bd2 to
96e2bcc
Compare
96e2bcc to
573f8bf
Compare
|
@baywet sorry for the mega PR. Most of the changes are tests and mechanical changes. However, if you want, I can split it into multiple PRs if that will make it easier for you to review. |
|
Thanks for the contribution! No worries, I'll take the time to review during the week. Looking back at the reference mechanisms that are implemented throughout the library, I realized we had a specification/documentation page that never got published on release. Took the time to clean it up a little and it's now available here. https://learn.microsoft.com/en-us/openapi/openapi.net/references-openapi Let us know what you think! |
| /// </summary> | ||
| public static string? ExtractDynamicAnchorName(string? dynamicRef) | ||
| { | ||
| if (string.IsNullOrEmpty(dynamicRef) || dynamicRef is null) return null; |
There was a problem hiding this comment.
Fixed. Removed the redundant || dynamicRef is null and used dynamicRef! on the subsequent dereference. The redundant check was a workaround for netstandard2.0 nullable flow analysis; the null-forgiving operator achieves the same without the dead conditional.
| foreach (var parameter in components.Parameters.Values) | ||
| if (parameter is not null) | ||
| RegisterParameterAnchors(document, parameter); |
There was a problem hiding this comment.
Acknowledged. These foreach + if (x is not null) patterns match the existing RegisterComponents code style throughout the file. Converting to .Where() would add LINQ iterator allocations in a one-time registration path for no runtime benefit. Leaving as-is for consistency with the existing codebase.
| foreach (var response in components.Responses.Values) | ||
| if (response is not null) | ||
| RegisterResponseAnchors(document, response); |
| foreach (var requestBody in components.RequestBodies.Values) | ||
| if (requestBody is not null) | ||
| RegisterRequestBodyAnchors(document, requestBody); |
| foreach (var header in components.Headers.Values) | ||
| if (header is not null) | ||
| RegisterHeaderAnchors(document, header); |
| foreach (var response in op.Responses.Values) | ||
| if (response is not null) | ||
| RegisterResponseAnchors(document, response); |
| foreach (var callback in op.Callbacks.Values) | ||
| if (callback is not null) | ||
| RegisterCallbackAnchors(document, callback, visited); |
| foreach (var header in response.Headers.Values) | ||
| if (header is not null) | ||
| RegisterHeaderAnchors(document, header); |
| foreach (var mediaType in content.Values) | ||
| { | ||
| if (mediaType is null) continue; | ||
| if (mediaType.Schema is not null) | ||
| RegisterAnchors(document, mediaType.Schema); | ||
| if (mediaType.ItemSchema is not null) | ||
| RegisterAnchors(document, mediaType.ItemSchema); | ||
| } |
| foreach (var def in contextSchema.Definitions.Values) | ||
| if (def.DynamicAnchor is string da && da.Equals(anchorName, StringComparison.Ordinal)) | ||
| return def; |
There was a problem hiding this comment.
Pull request overview
This PR adds document-scoped $dynamicRef resolution for OpenAPI 3.1/3.2 JSON Schema 2020-12 by indexing $dynamicAnchor/$anchor declarations per OpenApiDocument in OpenApiWorkspace, and by deserializing bare $dynamicRef schemas as OpenApiSchemaReference so they participate in the existing reference-resolution pipeline.
Changes:
- Index
$dynamicAnchorand$anchordeclarations per document inOpenApiWorkspaceand expose APIs for candidate lookup + context-aware resolution. - Update V31/V32 schema deserializers to create an
OpenApiSchemaReferencefor bare$dynamicRef(no$ref) and preserve siblings viaApplySchemaMetadata. - Add extensive V31/V32 unit tests covering registration, resolution precedence, ambiguity behavior, serialization round-trips, and tricky schema-bearing locations (responses/headers/callback cycles/etc.).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.OpenApi.Readers.Tests/V32Tests/OpenApiDynamicRefTests.cs | Adds OpenAPI 3.2 test coverage for bare $dynamicRef deserialization, resolution, anchor registration across OAS locations, and serialization behavior. |
| test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiDynamicRefTests.cs | Adds OpenAPI 3.1 test coverage for anchor indexing across subschema locations, ambiguity behavior, context-aware resolution, and $ref regression scenarios. |
| src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs | Implements per-document $dynamicAnchor/$anchor registries and recursive anchor discovery across components + inline paths/webhooks/callback graphs. |
| src/Microsoft.OpenApi/Reader/V32/OpenApiSchemaDeserializer.cs | Deserializes bare $dynamicRef schemas into OpenApiSchemaReference and applies schema metadata so siblings are preserved. |
| src/Microsoft.OpenApi/Reader/V31/OpenApiSchemaDeserializer.cs | Same as V32, for OpenAPI 3.1 schema deserialization. |
| src/Microsoft.OpenApi/Reader/JsonNodeHelper.cs | Adds $dynamicRef pointer detection and helper methods for extracting anchor names + determining fragment-only refs. |
| src/Microsoft.OpenApi/PublicAPI.Unshipped.txt | Records new/changed public API surface for serialization overrides, Target override, and new workspace APIs. |
| src/Microsoft.OpenApi/Models/References/OpenApiSchemaReference.cs | Overrides Target to resolve bare $dynamicRef via workspace $dynamicAnchor index with $anchor fallback when appropriate. |
| src/Microsoft.OpenApi/Models/JsonSchemaReference.cs | Adds IsDynamicRefOnly and custom serialization behavior to emit $dynamicRef (without $ref) for dynamic-ref-only references, plus interface updates for anchor walking. |
| if (dynamicPointer != null) | ||
| { | ||
| var anchorName = JsonNodeHelper.ExtractDynamicAnchorName(dynamicPointer); | ||
| var result = new OpenApiSchemaReference(!string.IsNullOrEmpty(anchorName) ? anchorName! : dynamicPointer, hostDocument); | ||
| var referenceMetadata = new OpenApiSchema(); | ||
| jsonObject.ParseMap(referenceMetadata, _openApiSchemaFixedFields, _openApiSchemaPatternFields, hostDocument, context, | ||
| static (schema, name, value) => | ||
| { | ||
| if (!string.Equals(name, OpenApiConstants.DynamicRef, StringComparison.Ordinal)) | ||
| { | ||
| schema.UnrecognizedKeywords ??= new Dictionary<string, JsonNode>(StringComparer.Ordinal); | ||
| schema.UnrecognizedKeywords[name] = value; | ||
| } | ||
| }); | ||
| result.Reference.ApplySchemaMetadata(referenceMetadata, jsonObject); | ||
| result.Reference.IsDynamicRefOnly = true; | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Fixed. Added result.Reference.SetJsonPointerPath(dynamicPointer, nodeLocation) before returning, mirroring the $ref branch.
| if (dynamicPointer != null) | ||
| { | ||
| var anchorName = JsonNodeHelper.ExtractDynamicAnchorName(dynamicPointer); | ||
| var result = new OpenApiSchemaReference(!string.IsNullOrEmpty(anchorName) ? anchorName! : dynamicPointer, hostDocument); | ||
| var referenceMetadata = new OpenApiSchema(); | ||
| jsonObject.ParseMap(referenceMetadata, _openApiSchemaFixedFields, _openApiSchemaPatternFields, hostDocument, context, | ||
| static (schema, name, value) => | ||
| { | ||
| if (!string.Equals(name, OpenApiConstants.DynamicRef, StringComparison.Ordinal)) | ||
| { | ||
| schema.UnrecognizedKeywords ??= new Dictionary<string, JsonNode>(StringComparer.Ordinal); | ||
| schema.UnrecognizedKeywords[name] = value; | ||
| } | ||
| }); | ||
| result.Reference.ApplySchemaMetadata(referenceMetadata, jsonObject); | ||
| result.Reference.IsDynamicRefOnly = true; | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Fixed. Same change applied to V32.
| public IReadOnlyList<IOpenApiSchema> GetDynamicAnchorCandidates(OpenApiDocument hostDocument, string anchorName) | ||
| { | ||
| if (_dynamicAnchorRegistryByDocument.TryGetValue(hostDocument, out var anchors) && | ||
| anchors.TryGetValue(anchorName, out var candidates)) | ||
| return candidates; | ||
| return []; | ||
| } |
There was a problem hiding this comment.
Fixed. Now returns candidates.AsReadOnly().
| // A $dynamicRef alongside structural schema keywords must not drop the siblings. The object | ||
| // is parsed as a normal OpenApiSchema (preserving maxProperties and properties) rather than | ||
| // being reduced to a bare reference that loses them. |
There was a problem hiding this comment.
Fixed. Updated comment to reflect that the object is parsed as an OpenApiSchemaReference with siblings preserved via ApplySchemaMetadata.
baywet
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
In addition to the comments I've left, I think we have good coverage of the deserialization scenarios, and some coverage of the serialization scenarios (at least for basic properties).
In think that one key aspect that is missing from the tests is: people using the object model to build documents. For "regular refs" components registration is required. I'd like to see more tests demonstrating that a document built from OM can resolve dynamic refs without requiring a re-parsing.
| /// This class extends OpenApiReference to provide schema-specific metadata override capabilities. | ||
| /// </summary> | ||
| public class JsonSchemaReference : OpenApiReferenceWithDescription | ||
| public class JsonSchemaReference : OpenApiReferenceWithDescription, IOpenApiSchemaMissingProperties |
There was a problem hiding this comment.
what was the motivation for adding this interface?
I think this is going to add confusion, essentially, only OpenApiSchemaReference and OpenApiSchema should provide that service to the consumers. The only reason why this class is public and not internal is because it's a generic type argument for another public class.
There was a problem hiding this comment.
Reverted. I added it to share an EnumerateMissingPropertiesChildren helper between the two EnumerateChildren overloads in OpenApiWorkspace, but you're right that it adds confusion. Restored the separate overload that reads JsonSchemaReference properties directly.
| Assert.True(reference.Reference.IsDynamicRefOnly); | ||
| // External target is not loaded in this workspace, so resolution returns null rather than | ||
| // falling back to the local Tree anchor. | ||
| Assert.Null(reference.Target); |
There was a problem hiding this comment.
could we update this unit test to provide a second document and ensure the resolution of externals also works?
There was a problem hiding this comment.
Good point. External $dynamicRef resolution (cross-document via URI) requires finding the target document by URI and looking up its anchor registry. The current Target override only resolves fragment-only refs (#node) against the local document. I can add cross-document support in this PR if you want it now, or track it as a follow-up. Let me know which you prefer.
…chemaReference Implements document-scoped $dynamicRef resolution per JSON Schema 2020-12 §8.2.3.2. Bare $dynamicRef schemas (no $ref) now deserialize as OpenApiSchemaReference whose Target resolves via per-document $dynamicAnchor and $anchor registries in OpenApiWorkspace. Resolution order in Target: 1. $dynamicAnchor index (single candidate → resolved automatically) 2. $anchor fallback when zero $dynamicAnchor candidates exist (per §8.2.3.2) 3. null when ambiguous (multiple candidates need dynamic-scope tracking) Anchor registries are populated by recursively walking the entire document tree: component schemas, reusable component definitions (parameters, responses, request bodies, headers, callbacks, path items, media types), inline schemas (paths, operations, webhooks), and all nested subschema locations ($defs, properties, items, allOf, if/then/else, etc.). Public APIs for consumers tracking dynamic scope: - GetDynamicAnchorCandidates(doc, anchorName): returns all candidate schemas - ResolveDynamicAnchorInContext(contextSchema, anchorName): resolves against a specific schema's $defs for context-dependent resolution Other changes: - Deserializer (V31/V32): detect bare $dynamicRef, create OpenApiSchemaReference with IsDynamicRefOnly, parse siblings via ApplySchemaMetadata - JsonSchemaReference: add IsDynamicRefOnly flag, implement IOpenApiSchemaMissingProperties, override SerializeAsV31/V32 for dynamic-only refs - JsonNodeHelper: GetDynamicReferencePointer, ExtractDynamicAnchorName, IsFragmentOnlyDynamicRef - Siblings preserved via ApplySchemaMetadata and surfaced through existing Reference-first property getters Fixes microsoft#2911.
573f8bf to
06d3b50
Compare
Pull Request
Description
Implements document-scoped
$dynamicRefresolution per JSON Schema 2020-12 §8.2.3.2. Bare$dynamicRefschemas (no$ref) now deserialize asOpenApiSchemaReferencewhoseTargetresolves via per-document$dynamicAnchorand$anchorregistries inOpenApiWorkspace.Resolution order in
Target$dynamicAnchorindex — single candidate resolves automatically$anchorfallback — when zero$dynamicAnchorcandidates exist (per §8.2.3.2)null— when ambiguous (multiple candidates need dynamic-scope tracking)Type of Change
Public APIs for consumers tracking dynamic scope
GetDynamicAnchorCandidates(doc, anchorName)— returns all candidate schemasResolveDynamicAnchorInContext(contextSchema, anchorName)— resolves against a specific schemas$defsBehavior change:
$refsiblings now take precedenceThe
$refdeserializer path now usesApplySchemaMetadata(which populatesJsonSchemaReferenceproperties) instead ofSetMetadataFromJsonObject. Structural siblings on$refschemas (type,maxProperties,pattern,allOf, etc.) are now captured and take precedence over target values via the existingReference.X ?? Target?.Xproperty getters. Previously these siblings were stored but not surfaced. This affects all$refschemas with siblings in 3.1+ documents, not just$dynamicRef.Anchor index coverage
Registries are populated by recursively walking the entire document tree: component schemas, reusable component definitions (parameters, responses, request bodies, headers, callbacks, path items, media types), inline schemas (paths, operations, webhooks), and all nested subschema locations (
$defs,properties,items,allOf,if/then/else, etc.).Open question
When multiple schemas declare the same
$dynamicAnchor,Targetreturnsnull(ambiguous). The spec requires the outermost candidate in the dynamic evaluation scope. Automatic resolution would require threading evaluation context throughTarget(e.g.,AsyncLocal<Stack>). Currently consumers handle this viaGetDynamicAnchorCandidates+ResolveDynamicAnchorInContext. Feedback welcome on whether the library should handle this automatically or leave it to consumers.Testing
Serialize_DoesNotMutateDomfailures on clean main)46 dynamic-ref tests across V31 and V32 covering: bare
$dynamicRefdeserialization, resolution to anchors in all schema locations, ambiguity handling,$anchorfallback,$dynamicAnchorprecedence, round-trip serialization, sibling preservation, context-aware resolution, and$refregression.Microsoft.OpenApi.Tests: 1149 passed.Microsoft.OpenApi.Readers.Tests: 535 passed, 7 pre-existing failures.Checklist
Versions applicability
Related
$refschemas)