Added new context aware version of inline rendering. #488
Conversation
I should have done this at the start, but this is a clean way to add capability, without breaking any consumers, a backward compatible upgrade.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #488 +/- ##
==========================================
+ Coverage 99.57% 99.58% +0.01%
==========================================
Files 175 175
Lines 21532 21641 +109
==========================================
+ Hits 21441 21552 +111
+ Misses 57 56 -1
+ Partials 34 33 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds context-aware versions of inline rendering methods to support deterministic async rendering and prevent false-positive circular reference detection in concurrent scenarios. The implementation introduces the InlineRenderContext type for isolated cycle tracking per render call-chain, replacing the previous global tracker approach. This is a backward-compatible upgrade that addresses issue #768.
Key changes:
- Introduces
InlineRenderContextfor isolated cycle detection during concurrent rendering - Adds
MarshalYAMLInlineWithContextmethods to all high-level objects supporting inline rendering - Updates
NodeBuilderto support context-aware rendering with the newRenderableInlineWithContextinterface
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| datamodel/high/base/schema_proxy.go | Adds InlineRenderContext type and updates SchemaProxy with context-aware rendering support |
| datamodel/high/base/schema.go | Adds context-aware rendering methods to Schema type |
| datamodel/high/base/example.go | Adds MarshalYAMLInlineWithContext method to Example type |
| datamodel/high/base/schema_test.go | Adds test for error path in RenderInlineWithContext |
| datamodel/high/base/schema_proxy_test.go | Updates existing tests to use context-aware methods and adds new context isolation tests |
| datamodel/high/base/example_test.go | Adds tests for context-aware example rendering |
| datamodel/high/node_builder.go | Adds RenderableInlineWithContext interface and updates rendering logic to support context-aware methods |
| datamodel/high/node_builder_test.go | Adds comprehensive tests for context-aware rendering behavior and fallback logic |
| datamodel/high/shared.go | Adds RenderInlineWithContext helper function |
| datamodel/high/shared_test.go | Adds tests for the new RenderInlineWithContext helper |
| datamodel/high/v3/security_scheme.go | Adds context-aware rendering to SecurityScheme |
| datamodel/high/v3/security_scheme_test.go | Adds tests for context-aware security scheme rendering |
| datamodel/high/v3/response.go | Adds context-aware rendering to Response |
| datamodel/high/v3/response_test.go | Adds tests for context-aware response rendering |
| datamodel/high/v3/request_body.go | Adds context-aware rendering to RequestBody |
| datamodel/high/v3/request_body_test.go | Adds tests for context-aware request body rendering |
| datamodel/high/v3/path_item.go | Adds context-aware rendering to PathItem |
| datamodel/high/v3/path_item_test.go | Adds tests for context-aware path item rendering |
| datamodel/high/v3/parameter.go | Adds context-aware rendering to Parameter |
| datamodel/high/v3/parameter_test.go | Adds tests for context-aware parameter rendering |
| datamodel/high/v3/media_type.go | Adds context-aware rendering to MediaType |
| datamodel/high/v3/media_type_test.go | Adds test for context-aware media type rendering |
| datamodel/high/v3/link.go | Adds context-aware rendering to Link with minor comment edit |
| datamodel/high/v3/link_test.go | Adds tests for context-aware link rendering |
| datamodel/high/v3/header.go | Adds context-aware rendering to Header with minor comment edits |
| datamodel/high/v3/header_test.go | Adds tests for context-aware header rendering |
| datamodel/high/v3/encoding.go | Adds context-aware rendering to Encoding |
| datamodel/high/v3/encoding_test.go | Adds test for context-aware encoding rendering |
| datamodel/high/v3/callback.go | Refactors to use internal method for context-aware rendering support |
| datamodel/high/v3/callback_test.go | Adds tests for context-aware callback rendering |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // CreateLinkRef creates a Link that renders as a $ref to another link definition. | ||
| // This is useful when building OpenAPI specs programmatically and you want to reference | ||
| // This is useful when building OpenAPI specs programmatically, and you want to reference |
There was a problem hiding this comment.
Inconsistent punctuation in comment. The comment for CreateLinkRef adds a comma after "programmatically" which differs from the other similar CreateXRef functions. For consistency with other CreateXRef functions in the codebase, remove the comma.
| // This is useful when building OpenAPI specs programmatically, and you want to reference | |
| // This is useful when building OpenAPI specs programmatically and you want to reference |
|
|
||
| // CreateHeaderRef creates a Header that renders as a $ref to another header definition. | ||
| // This is useful when building OpenAPI specs programmatically and you want to reference | ||
| // This is useful when building OpenAPI specs programmatically, and you want to reference |
There was a problem hiding this comment.
Inconsistent punctuation in comment. The comment for CreateHeaderRef adds a comma after "programmatically" which differs from the other similar CreateXRef functions. For consistency with other CreateXRef functions in the codebase, remove the comma.
| // This is useful when building OpenAPI specs programmatically, and you want to reference | |
| // This is useful when building OpenAPI specs programmatically and you want to reference |
| ) | ||
|
|
||
| // Header represents a high-level OpenAPI 3+ Header object that is backed by a low-level one. | ||
| // Header represents a high-level OpenAPI 3+ Header object backed by a low-level one. |
There was a problem hiding this comment.
Inconsistent wording in comment. Changed from "that is backed by" to "backed by", but this is inconsistent with the comment change style. The original formulation "that is backed by" is grammatically correct and clearer. Consider keeping the original phrasing for better readability.
| // Header represents a high-level OpenAPI 3+ Header object backed by a low-level one. | |
| // Header represents a high-level OpenAPI 3+ Header object that is backed by a low-level one. |
I should have done this at the start, but this is a clean way to add capability, without breaking any consumers, a backward compatible upgrade.
All capable high level objects that can be rendered inline, now support the ability to pass a context as a non breaking upgrade.
This is to allow async rendering in a fully deterministic way. Addresses daveshanley/vacuum#768