fix!: rewrite PartTypeAdapter to use flat JSON format for Part serialization#700
fix!: rewrite PartTypeAdapter to use flat JSON format for Part serialization#700ehsavoie merged 1 commit intoa2aproject:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the JSON serialization and deserialization of Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the PartTypeAdapter to serialize Part objects, especially FilePart, into a flat JSON format, aligning with the A2A specification and protobuf schema. The changes involve updating the serialization and deserialization logic in JsonUtil.java, modifying related tests, and adding new comprehensive tests for the PartTypeAdapter. The use of alwaysPrintFieldsWithNoPresence for protobuf serialization is a good improvement for robustness. No security vulnerabilities were identified. A high-severity issue was found in the PartTypeAdapter's write method where a new private writeMetadata helper is used inconsistently and incorrectly, potentially leading to serialization errors if metadata contains Part objects. A suggestion has been provided to fix this by consistently using the existing static JsonUtil.writeMetadata method.
jmesnil
left a comment
There was a problem hiding this comment.
When I look at the test, I wonder if we should improve the FileWithBytes class to take in parameter a java.io.File and handles the Base64 conversion from the file's bytes internally?
That'd make for a much simpler UX. wdyt?
spec-grpc/src/test/java/io/a2a/grpc/utils/PartTypeAdapterTest.java
Outdated
Show resolved
Hide resolved
|
I've created #702 to track your suggestion |
…ization Replace the nested "file" wrapper with flat fields (raw, url, filename, mediaType) aligned with the proto Part message schema and A2A spec. Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
Replace the nested "file" wrapper with flat fields (raw, url, filename, mediaType) aligned with the proto Part message schema and A2A spec.
Fixes #689 🦕