fix(avro)!: support named type references#32
Merged
Conversation
Adds a new job named Obfuscan to the CI workflow to scan the pull request diff using the ByteBardOrg/obfuscan-action. This job runs only when a pull request is opened, and it uses the head SHA of the pull request to check for potential issues in the code changes before the main build proceeds.
Adds Avro-specific named type handling for schemas that reference previously defined records, enums, or fixed types by name. Introduces `AvroNamedType`, keeps AsyncAPI `$ref` handling unchanged, supports recursive schemas like `LongList`, and widens map `values` to accept any Avro schema instead of only primitive types. Existing construction with `AvroPrimitiveType` is preserved through the existing implicit conversion to `AsyncApiAvroSchema`, and primitive schema values can be converted back with an explicit cast. BREAKING CHANGE: `AvroMap.Values` now uses `AsyncApiAvroSchema` instead of `AvroPrimitiveType`.
Changed how validation errors and warnings are added to the diagnostic collection. Previously, all items from the validation result were iterated over. This change explicitly separates the handling of AsyncApiValidatorError into diagnostic.Errors and AsyncApiValidatorWarning into diagnostic.Warnings, ensuring correct categorization of validation feedback.
Contributor
📝 WalkthroughWalkthroughThis PR implements support for Avro named types (recursive and forward-referenced schemas) in AsyncAPI.NET, resolving issue ChangesAvro Named Types Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/Schemas.md`:
- Line 91: Update the wording to use compound-modifier hyphenation: replace the
phrase “named type references” with “named-type references” in the sentence
describing Avro named schemas so the text reads that Avro named schemas can be
referenced by name using AvroNamedType and that these are named-type references
(serializing as plain strings like "LongList", not AsyncAPI $ref objects).
- Line 11: Fix several documentation typos in the Schemas doc: change
"convertable" to "convertible" in the sentence referencing AsyncApiJsonSchema
and AsyncMultiFormatSchema (look for the phrase "AsyncApiJsonSchema is
implicitly convertable to AsyncMultiFormatSchema"), remove the stray "to" in the
phrase "attaching it via to" (make it "attaching it via"), change "parts that
needs" to "parts that need", correct "propery" to "property", and change "its
looks" to "it looks". Search for those exact typo strings to locate the places
to edit and apply the replacements so spelling and grammar are fixed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3293fe74-0de2-4d93-b679-9b01702b2be2
📒 Files selected for processing (10)
docs/Schemas.mdsrc/ByteBard.AsyncAPI.Readers/AsyncApiJsonDocumentReader.cssrc/ByteBard.AsyncAPI.Readers/Schemas/AsyncApiAvroSchemaDeserializer.cssrc/ByteBard.AsyncAPI/Models/Avro/AsyncApiAvroSchema.cssrc/ByteBard.AsyncAPI/Models/Avro/AvroMap.cssrc/ByteBard.AsyncAPI/Models/Avro/AvroNamedType.cssrc/ByteBard.AsyncAPI/Services/AsyncApiWalker.cssrc/ByteBard.AsyncAPI/Validation/Rules/AsyncApiAvroRules.cstest/ByteBard.AsyncAPI.Tests/Models/AvroSchema_Should.cstest/ByteBard.AsyncAPI.Tests/Validation/ValidationRulesetTests.cs
obfuscan reportStatus: No findings.
No suspicious obfuscation or backdoor patterns were found in the scanned diff. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
About the PR
Adds Avro named type reference support for
AsyncApiAvroSchema.This introduces
AvroNamedTypefor Avro schema references that are represented as plain Avro strings, such as"LongList", instead of AsyncAPI$refobjects. Named types are resolved using Avro name and namespace rules, including recursive references and map value schemas.The PR also updates validation so unresolved Avro named types are reported as warnings, and fixes fragment diagnostics so validator warnings are returned in
diagnostic.Warningsinstead of being treated as errors.Documentation for schema payloads and Avro named types was added in
docs/Schemas.md.Assigning primitives still works, but reading AvroMap.Values as AvroPrimitiveType now needs the explicit cast because the property type was widened.
Changelog
AvroNamedTypefor Avro named schema references.docs/Schemas.mdwith Avro named type documentation.AvroMap.Valuesto accept anyAsyncApiAvroSchema, not only primitive types.ReadFragmentdiagnostics so validation warnings are added todiagnostic.Warnings.Related Issues
BREAKING CHANGE: AvroMap.Values now uses AsyncApiAvroSchema instead of AvroPrimitiveType.
Summary by CodeRabbit
New Features
Bug Fixes
Validation
Documentation