fix: Refactor NettyServerConfig to use MessageCodecFactory directly#4134
fix: Refactor NettyServerConfig to use MessageCodecFactory directly#4134
Conversation
- Change NettyServerConfig to store MessageCodecFactory instead of PartialFunction - Add codecFactory getter that ensures withMapOutput is applied for JSON serialization - Update NettyResponseHandler to accept MessageCodecFactory directly - Centralize codec factory creation to avoid duplication between response handler and request dispatcher This improves maintainability by making the codec dependency more explicit and avoiding duplicated codec factory creation logic. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @xerial, 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 refactors the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 NettyServerConfig to use MessageCodecFactory directly, improving maintainability by making the codec dependency more explicit and centralizing its creation. The changes are well-aligned with this goal and represent a good improvement to the codebase. I have one suggestion to simplify one of the new methods for better clarity and consistency.
| def withCustomCodec(m: Map[Surface, MessageCodec[_]]): NettyServerConfig = { | ||
| this.copy(customCodec = customCodec.orElse { | ||
| withCustomCodec { | ||
| case s: Surface if m.contains(s) => m(s) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
The implementation of this withCustomCodec overload can be simplified by calling customCodecFactory.withCodecs(m) directly, similar to the other overload for PartialFunction. This avoids the indirection of calling the other withCustomCodec overload and creating a verbose and slightly inefficient anonymous PartialFunction. It also makes the two overloads more parallel in their implementation.
def withCustomCodec(m: Map[Surface, MessageCodec[_]]): NettyServerConfig = {
this.copy(customCodecFactory = customCodecFactory.withCodecs(m))
}Address Gemini review feedback 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
f9c7678 to
b9b6bc5
Compare
Summary
NettyServerConfigto storeMessageCodecFactoryinstead ofPartialFunctioncodecFactorygetter that ensureswithMapOutputis applied for JSON serializationNettyResponseHandlerto acceptMessageCodecFactorydirectlyThis follows the suggestion from Gemini code review on #4132 to pass
MessageCodecFactorydirectly instead ofPartialFunction, improving maintainability by making the codec dependency more explicit.Test plan
🤖 Generated with Claude Code