add transport protocol for service ref and fix some ui bugs#13702
add transport protocol for service ref and fix some ui bugs#13702KomachiSion merged 1 commit intoalibaba:developfrom
Conversation
|
Thanks for your this PR. 🙏 感谢您提交的PR。 🙏 |
支持服务引用传输协议并修复UI问题变更概述
变更文件
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
|
![lingma-agents[bot]](https://avatars.githubusercontent.com/in/1198481?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔎 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 0 | 阻断性问题，需立即修复。例如：系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 0 | 严重问题，高优先级修复。例如：核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 1 | 主要问题，建议修复。例如：非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 1 | 次要问题，酬情优化。例如：代码格式不规范或注释缺失。 |
总计: 2 个问题
📋 评审意见详情
💡 代码实现建议
以下是文件级别的代码建议，聚焦于代码的可读性、可维护性和潜在问题。
☕ ai/src/main/java/com/alibaba/nacos/ai/constant/Constants.java (1 💬)
- 修复常量名称中的拼写错误以提高代码可读性。 (L54)
📱 console-ui/src/pages/AI/McpDetail/McpDetail.js (1 💬)
- 增加协议字段的有效性校验以防止无效协议导致的错误。 (L1501)
🚀 架构设计建议
以下是对代码架构和设计的综合分析，聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍1. 传输协议字段命名不一致导致潜在混淆
在 Constants.java 中定义的 MCP_BACKEND_ISTANCE_PROTOCOL_KEY 常量名称存在拼写错误（'ISTANCE' 应为 'INSTANCE'），这可能导致代码可读性和维护性问题。虽然已有文件级建议指出该拼写错误，但从系统层面来看，这个常量在整个项目中被多处引用，若未统一修正可能引发理解上的歧义或未来扩展时的不一致性。
📌 关键代码
public static final String MCP_BACKEND_ISTANCE_PROTOCOL_KEY = "transportProtocol";可能引起开发人员对字段含义的误解，影响代码可维护性和一致性；若后续功能依赖此常量名进行反射或其他动态处理，可能引发运行时错误。
🔍2. 前端 UI 对数组 items 节点处理逻辑分散且重复
在 CreateTools 和 ShowTools 组件中都出现了针对数组 items 节点的特殊处理逻辑，包括判断是否为 items 节点、隐藏描述字段等操作。这些逻辑存在重复实现，并且分布在多个地方，增加了维护成本和出错风险。建议将这部分通用逻辑抽象成独立的工具函数或组件以提高代码复用性和可维护性。
📌 关键代码
// items 节点不需要描述
if (!String(element.key || '').endsWith('@@items')) {
arg.description = element.description;
} else if (arg && arg.description !== undefined) {
delete arg.description;
}// 递归构建数组项的子树
const buildArrayItemSubtree = (itemDef, itemKey) => {
...
}重复代码增加维护难度，容易出现修改一处而遗漏另一处的问题；逻辑分散不利于统一行为控制和调试排查。
🔍3. 服务引用中的协议信息传递链路缺乏明确校验机制
从 UI 层接收协议配置到后端服务处理的过程中，虽然加入了协议字段的设置与传递，但整体链路中缺少对该字段有效性的集中校验机制。例如，在 McpServerOperationService 中直接使用了来自 serviceRef 的 transportProtocol 字段，但没有验证其是否符合预期值（如 http/https）或是否为空。这种缺失可能在未来引入非法协议导致服务异常。
📌 关键代码
String protocol = null;
McpServiceRef serviceRef = detailInfo.getRemoteServerConfig().getServiceRef();
if (Objects.nonNull(serviceRef)) {
protocol = serviceRef.getTransportProtocol();
}
List<McpEndpointInfo> backendEndpoints = transferToMcpEndpointInfo(instances,
detailInfo.getRemoteServerConfig().getExportPath(),
protocol);非法协议值可能导致生成错误的服务地址或连接失败；缺少校验会降低系统的健壮性和安全性。
🔍4. 新增传输协议字段未在所有相关测试用例中覆盖
尽管在主流程中增加了 transportProtocol 字段的支持，但在单元测试 McpServerOperationServiceTest 中并未全面更新所有涉及服务引用的测试场景来验证该字段的行为。部分测试方法仅设置了基础字段，忽略了新加入的协议字段，这可能导致测试覆盖率不足，无法充分保证新功能的正确性和稳定性。
📌 关键代码
serviceRef.setNamespaceId(Constants.MCP_SERVER_ENDPOINT_GROUP);
serviceRef.setServiceName("mcpName");
remoteServiceConfig.setServiceRef(serviceRef);
remoteServiceConfig.setExportPath("/nacos");
mockStorageInfo.setRemoteServerConfig(remoteServiceConfig);测试覆盖不全可能导致潜在缺陷未被发现，影响产品质量；特别是涉及协议选择的关键路径需要更严格的测试保障。
审查详情
📒 文件清单 （8 个文件）
📝 变更: 8 个文件
📝 变更文件:
ai/src/main/java/com/alibaba/nacos/ai/constant/Constants.javaai/src/main/java/com/alibaba/nacos/ai/service/McpServerOperationService.javaai/src/test/java/com/alibaba/nacos/ai/service/McpServerOperationServiceTest.javaapi/src/main/java/com/alibaba/nacos/api/ai/model/mcp/McpServiceRef.javaconsole-ui/src/pages/AI/McpDetail/CreateTools/index.jsconsole-ui/src/pages/AI/McpDetail/McpDetail.jsconsole-ui/src/pages/AI/McpDetail/ShowTools.jsconsole-ui/src/pages/AI/NewMcpServer/NewMcpServer.js
💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论，lingma-agents 将自动处理您的请求。例如：
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案，并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如：
-
@lingma-agents 分析这个方法的性能瓶颈并提供优化建议。
-
@lingma-agents 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如：
-
@lingma-agents 请总结上述讨论并提出解决方案。
-
@lingma-agents 请根据讨论内容生成优化代码。
|
|
||
| public static final String MCP_SERVER_ENDPOINT_CLUSTER = com.alibaba.nacos.api.common.Constants.DEFAULT_CLUSTER_NAME; | ||
|
|
||
| public static final String MCP_BACKEND_ISTANCE_PROTOCOL_KEY = "transportProtocol"; |
There was a problem hiding this comment.
修复常量名称中的拼写错误以提高代码可读性。
🟢 Minor | 🧹 Code Smells
📋 问题详情
常量 MCP_BACKEND_ISTANCE_PROTOCOL_KEY 中存在拼写错误（'ISTANCE' 应为 'INSTANCE'），这会导致代码含义不清晰，影响可维护性。
💡 解决方案
将常量名称中的拼写错误修正为正确的英文单词。
- public static final String MCP_BACKEND_ISTANCE_PROTOCOL_KEY = "transportProtocol";
+ public static final String MCP_BACKEND_INSTANCE_PROTOCOL_KEY = "transportProtocol";您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
| // 根据 protocol 字段判断使用 https 还是 http 前缀 | ||
| const protocol = this.state.serverConfig?.protocol || ''; | ||
| const protocolPrefix = protocol.includes('https') ? 'https://' : 'http://'; | ||
| const protocolPrefix = item.protocol + '://'; |
There was a problem hiding this comment.
增加协议字段的有效性校验以防止无效协议导致的错误。
🟡 Major | 🐞 Bugs
📋 问题详情
在构建 endpoint URL 时，直接使用 item.protocol 字段作为协议前缀，如果该字段为空或无效，可能会导致生成错误的 URL。应添加对协议字段的校验。
💡 解决方案
添加默认协议值，防止协议字段为空时生成无效 URL。
- const protocolPrefix = item.protocol + '://';
+ const protocolPrefix = (item.protocol || 'http') + '://';您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
KomachiSion
added
area/Nacos console
|
|
3 participants
Please do not create a Pull Request without creating an issue first.
What is the purpose of the change
Brief changelog
XX
Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
[ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=trueto make sure basic checks pass. Runmvn clean install -DskipITsto make sure unit-test pass. Runmvn clean test-compile failsafe:integration-testto make sure integration-test pass.