fix: Fix Security JSON serialization

[jmesnil]

fix: Use GSON serialization for JSON-RPC Agent card endpoint
fix: Fix Security JSON serialization

Fixes #590 & #591

[gemini-code-assist]

Summary of Changes

Hello @jmesnil, I'm Gemini Code Assist¹! 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 introduces significant improvements to JSON serialization within the system, primarily focusing on the SecurityScheme hierarchy and the AgentCard endpoint. By implementing custom Gson TypeAdapter's and standardizing type identification, it resolves issues related to polymorphic serialization and ensures consistent and correct JSON output for security-related objects and the agent configuration. The changes also streamline existing serialization logic for Part and StreamingEventKind objects.

Highlights

• Standardized Security Scheme Type Identification: Replaced specific constants (e.g., API_KEY, OAUTH2) with a generic TYPE constant and introduced a type() method in the SecurityScheme interface and its implementations for consistent identification.
• Polymorphic Security Scheme Serialization: Implemented a custom Gson TypeAdapter (SecuritySchemeTypeAdapter) to correctly serialize and deserialize SecurityScheme and its sealed implementations, using a type discriminator field in the JSON output.
• Improved JSON Serialization for Agent Card Endpoint: Modified the /well-known/agent-card.json endpoint to explicitly use JsonUtil.toJson() for serializing the AgentCard object, ensuring proper JSON output.
• Refactored Part and StreamingEventKind Serialization: Simplified PartTypeAdapter and StreamingEventKindTypeAdapter by using constants for discriminator fields and value.getClass() for delegate serialization, reducing boilerplate and improving maintainability.
• Enhanced API Key Location Serialization: Added APIKeyLocationTypeAdapter to handle the serialization and deserialization of APIKeySecurityScheme.Location enum to its wire format string representation.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[jmesnil]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a custom GSON TypeAdapter for SecurityScheme to correctly handle polymorphic JSON serialization and deserialization, which is a solid approach. The changes also improve maintainability by replacing magic strings with constants. I've identified a few areas for improvement, including an unused import, a missing @SuppressWarnings that could lead to build warnings, and an opportunity to enhance performance and clarity by using a static constant. Additionally, there's a minor naming inconsistency in one of the security scheme types that could be addressed for better code uniformity. Overall, the changes are well-implemented and effectively address the serialization issue.

[gemini-code-assist]

Code Review

This pull request effectively addresses the JSON serialization issues for SecurityScheme by implementing a custom GSON TypeAdapter. The changes are well-executed, including updating the SecurityScheme hierarchy with a type() discriminator, renaming constants for better clarity, and refactoring other type adapters for improved maintainability. Additionally, enforcing GSON for the agent card endpoint ensures consistent serialization behavior. I've included a couple of suggestions to enhance the robustness of the new and existing type adapters by making the parsing logic stricter.

[jmesnil]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively addresses the JSON serialization issue for SecurityScheme objects by introducing a custom GSON TypeAdapter for polymorphic handling. The changes are well-structured, introducing a type() discriminator in the SecurityScheme hierarchy and updating related code and tests. I've provided a few suggestions to improve consistency and robustness, particularly regarding the naming of a type identifier and the initialization of a delegate GSON instance. Overall, this is a solid fix.

[jmesnil]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a robust JSON serialization mechanism for SecurityScheme and other polymorphic types by leveraging custom Gson TypeAdapters. The changes are a significant improvement in correctness and maintainability. Key enhancements include implementing polymorphic serialization for the SecurityScheme hierarchy, adding a custom adapter for APIKeySecurityScheme.Location enums, and refactoring existing adapters for better robustness. My review identifies two high-severity issues related to missing null checks in the new TypeAdapter implementations, which could lead to NullPointerExceptions during serialization. With these fixes, the pull request will be in excellent shape.

[jmesnil]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively addresses the JSON serialization of SecurityScheme objects by introducing a custom TypeAdapter for polymorphic handling. The changes are well-structured, with updates to the spec to support the new type() discriminator, and refactoring in JsonUtil to use this new adapter. The simplifications in other type adapters are also a welcome improvement to maintainability. The overall implementation is robust and clean. I have one minor suggestion to restore a suppressed warning that was lost during refactoring.

[jmesnil]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively addresses the JSON serialization issues for SecurityScheme by introducing a custom SecuritySchemeTypeAdapter for polymorphic serialization with Gson. The changes are well-implemented across the board, from updating the SecurityScheme interface and its implementations to registering the new type adapter and ensuring its use in the agent card endpoint. The addition of SecuritySchemeSerializationTest provides good test coverage for the new serialization logic.

I've found one issue in JsonUtil.java regarding the initialization of a delegate Gson instance which could lead to serialization errors for FileContent. Please see the specific comment for details.

Overall, this is a solid fix that improves the robustness and correctness of JSON serialization in the project.

[jmesnil]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively addresses JSON serialization issues for SecurityScheme by introducing a custom Gson TypeAdapter. The implementation is robust, handling polymorphic serialization and deserialization correctly. The changes also include beneficial refactorings in other type adapters, improving code quality and maintainability. The addition of a comprehensive test suite for SecurityScheme serialization is a great way to ensure the fix is working as expected and prevent future regressions. I have one suggestion to clean up unused imports in the new test file.

[jmesnil]

/gemini summary

[gemini-code-assist]

Summary of Changes

This pull request addresses and resolves several JSON serialization issues within the system, primarily focusing on the SecurityScheme hierarchy and the Agent Card endpoint. By introducing custom GSON TypeAdapters, standardizing type identification for security schemes, and refining existing serialization logic for Part and StreamingEventKind objects, the changes ensure that all related JSON outputs are consistent, correctly formatted, and robust against malformed inputs. This significantly improves the reliability of data exchange for security configurations and agent information.

Highlights

• Polymorphic Security Scheme Serialization: Implemented a custom Gson TypeAdapter (SecuritySchemeTypeAdapter) to correctly serialize and deserialize SecurityScheme and its sealed implementations, using a type discriminator field in the JSON output. This ensures proper handling of various security scheme types like API Key, HTTP Auth, OAuth2, OpenID Connect, and Mutual TLS.
• Standardized Security Scheme Type Identification: Replaced specific constants (e.g., API_KEY, OAUTH2) with a generic TYPE constant and introduced a type() method in the SecurityScheme interface and its implementations for consistent identification during serialization and deserialization.
• Improved JSON Serialization for Agent Card Endpoint: Modified the /well-known/agent-card.json endpoint to explicitly use JsonUtil.toJson() for serializing the AgentCard object, ensuring consistent and proper JSON output for agent configuration.
• Refactored Part and StreamingEventKind Serialization: Simplified PartTypeAdapter and StreamingEventKindTypeAdapter by using constants for discriminator fields and value.getClass() for delegate serialization, reducing boilerplate and improving maintainability. The PartTypeAdapter now also enforces that a Part object must have exactly one discriminator key.
• Enhanced API Key Location Serialization: Added APIKeyLocationTypeAdapter to handle the serialization and deserialization of the APIKeySecurityScheme.Location enum to its wire format string representation, ensuring correct JSON representation for API key locations.

Changelog

• client/transport/spi/src/test/java/io/a2a/client/transport/spi/interceptors/auth/AuthInterceptorTest.java
  • Updated AuthTestCase instantiations to use the new TYPE constant from APIKeySecurityScheme, OAuth2SecurityScheme, and OpenIdConnectSecurityScheme.
• jsonrpc-common/src/main/java/io/a2a/jsonrpc/common/json/JsonUtil.java
  • Added imports for various SecurityScheme related classes and constants (DATA, FILE, TEXT, format, Set).
  • Registered SecuritySchemeTypeAdapter with the OBJECT_MAPPER for polymorphic serialization.
  • Modified PartTypeAdapter to use constants for discriminator fields and added validation to ensure Part objects have exactly one key.
  • Simplified StreamingEventKindTypeAdapter to use value.getClass() for delegate serialization.
  • Updated FileContentTypeAdapter to initialize its delegate Gson instance using createBaseGsonBuilder() and simplified its serialization logic.
  • Introduced APIKeyLocationTypeAdapter for proper serialization of APIKeySecurityScheme.Location enum.
  • Added SecuritySchemeTypeAdapter to handle polymorphic serialization and deserialization of SecurityScheme implementations.
• jsonrpc-common/src/test/java/io/a2a/jsonrpc/common/json/SecuritySchemeSerializationTest.java
  • Added a new test file to comprehensively test the serialization and deserialization of all SecurityScheme implementations (HTTPAuth, APIKey, OAuth2, OpenIdConnect, MutualTLS) using Gson.
• reference/jsonrpc/src/main/java/io/a2a/server/apps/quarkus/A2AServerRoutes.java
  • Modified the getAgentCard() method to explicitly serialize the AgentCard object to JSON using JsonUtil.toJson() before returning it.
• spec/src/main/java/io/a2a/spec/APIKeySecurityScheme.java
  • Renamed the API_KEY constant to TYPE and implemented the type() method from the SecurityScheme interface.
• spec/src/main/java/io/a2a/spec/HTTPAuthSecurityScheme.java
  • Renamed the HTTP constant to TYPE and implemented the type() method from the SecurityScheme interface.
• spec/src/main/java/io/a2a/spec/MutualTLSSecurityScheme.java
  • Renamed the MUTUAL_TLS constant to TYPE and implemented the type() method from the SecurityScheme interface.
• spec/src/main/java/io/a2a/spec/OAuth2SecurityScheme.java
  • Renamed the OAUTH2 constant to TYPE and implemented the type() method from the SecurityScheme interface.
• spec/src/main/java/io/a2a/spec/OpenIdConnectSecurityScheme.java
  • Renamed the OPENID_CONNECT constant to TYPE and implemented the type() method from the SecurityScheme interface.
• spec/src/main/java/io/a2a/spec/SecurityScheme.java
  • Added a new type() method to the SecurityScheme sealed interface to provide a consistent way to identify the scheme type.

Activity

• An initial summary of changes was provided by gemini-code-assist[bot].
• The author jmesnil requested multiple /gemini review commands throughout the PR's lifecycle.
• gemini-code-assist[bot] identified an unused import of java.lang.InternalError in JsonUtil.java.
• A suggestion was made by gemini-code-assist[bot] to restore the @SuppressWarnings("unchecked") annotation for Map<String, Object> deserialization in PartTypeAdapter.
• gemini-code-assist[bot] recommended making VALID_KEYS a private static final constant in SecuritySchemeTypeAdapter.
• A suggestion to rename mtlsSecurityScheme to mutualTlsSecurityScheme was made by gemini-code-assist[bot], which was later retracted after jmesnil clarified that the naming is mandated by the specification.
• gemini-code-assist[bot] provided a detailed suggestion to refactor PartTypeAdapter deserialization to enforce exactly one discriminator key using a switch expression.
• A similar suggestion was made by gemini-code-assist[bot] to refactor SecuritySchemeTypeAdapter deserialization to enforce exactly one discriminator key using a switch expression.
• gemini-code-assist[bot] corrected a misleading comment in PartTypeAdapter regarding the reachability of the default case.
• A suggestion was made by gemini-code-assist[bot] to initialize delegateGson in SecuritySchemeTypeAdapter using createBaseGsonBuilder() for consistency.
• gemini-code-assist[bot] reiterated a suggestion to rename the mtlsSecurityScheme constant to mutualTLSSecurityScheme for consistency.
• High priority feedback from gemini-code-assist[bot] advised adding a null check in the APIKeyLocationTypeAdapter.write method.
• High priority feedback from gemini-code-assist[bot] advised adding a null check in the SecuritySchemeTypeAdapter.write method.
• gemini-code-assist[bot] suggested restoring the @SuppressWarnings("unchecked") annotation in PartTypeAdapter for the dataMap variable.
• High priority feedback from gemini-code-assist[bot] indicated that the delegateGson in FileContentTypeAdapter should be initialized using createBaseGsonBuilder().
• gemini-code-assist[bot] recommended removing numerous unused imports from the newly added SecuritySchemeSerializationTest.java file.
• The author jmesnil requested a final /gemini summary.