-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Resolve relative JSON schema references in root schema #63256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes issues with relative JSON schema references in the OpenAPI schema generation. It resolves problems where generic types or types appearing multiple times in hierarchies generated invalid references by fully resolving relative references to inlined schemas.
Key Changes
- Added reference resolution logic to expand relative
$ref
paths to their actual schema content - Updated test expectations to verify proper schema references instead of invalid relative paths
- Added comprehensive test coverage for generic types, reused types across hierarchies, and nullable reference types
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
OpenApiSchemaService.cs |
Implements reference resolution with recursive traversal and cycle detection |
OpenApiConstants.cs |
Adds constants for $ref keyword and fragment prefix |
MapSchemasEndpoints.cs |
Adds sample endpoints and model classes for testing reference resolution scenarios |
OpenApiSchemaReferenceTransformerTests.cs |
Adds new test methods and updates existing assertions for resolved references |
Snapshot files (3 files) | Updates test output expectations to reflect resolved schemas instead of invalid references |
If you pull #63095 into this branch, do the two failing tests now pass? |
return ResolveReferencesRecursive(node, rootSchema, []); | ||
} | ||
|
||
private static JsonNode ResolveReferencesRecursive(JsonNode node, JsonNode rootSchema, HashSet<string> visitedRefs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need something to stop an infinite loop happening? IIRC, there's other bits of the code that do recursion that do something like "this is the 64th time I'm calling this, so that's too much - stop".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For handling recursion, we rely on things being caught by JsonSchemaExporter before the schema is returned to us. Tests like ThrowsForOverlyNestedSchemas
and SupportsDeeplyNestedSchemaWithConfiguredMaxDepth
cover this. I don't think there's any new recursion issues that get introduced here and the logic for tracking visited refs is probably sufficient for this scenario?
OpenApiDocumentReferencesAreValid passes. OpenApiDocumentIsValid fails because of the path issue that you're fixing in that PR (thanks!) |
Closes #61194.
This PR fixes a host of issues with generic types or types that appear multiple times in the type hierarchy by fully resolving the relative references in schemas that are generated by JsonSchemaExporter to inlined schemas to avoid generating invalid refs.
Before (Unresolved References)
After (Resolved References)