<fix>[conf]: <description#2746
<fix>[conf]: <description#2746zstack-robot-1 wants to merge 1 commit intofeature-zsv-4.10.20-UI-install-sdsfrom
Conversation
|
Warning Rate limit exceeded@MatheMatrix has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 58 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
概览向 ZStack SDK 中新增了两个插件安装 API 操作类:InstallZStoneAction 和 InstallZceXAction,以及对应的结果类。同时在测试库中添加了 installZceX 辅助方法,用于支持这些新操作。 变更
序列图sequenceDiagram
participant Client
participant ApiHelper
participant Action
participant ZSClient
participant API
Client->>ApiHelper: installZceX(closure)
ApiHelper->>Action: create InstallZceXAction
ApiHelper->>Action: configure parameters
alt 同步调用
ApiHelper->>Action: call()
Action->>ZSClient: call(this)
ZSClient->>API: POST /zce-x-plugin/install
API-->>ZSClient: ApiResult
ZSClient-->>Action: Result
Action-->>ApiHelper: Result
ApiHelper-->>Client: result
else 异步调用
ApiHelper->>Action: call(Completion)
Action->>ZSClient: call(this, InternalCompletion)
ZSClient->>API: POST /zce-x-plugin/install (async)
API-->>ZSClient: ApiResult (async)
ZSClient-->>Action: InternalCompletion.done()
Action-->>Client: 回调通知
end
代码审查工作量🎯 2 (简单) | ⏱️ ~10 分钟 变更遵循既有代码模式,两个新 API 操作类高度相似,具有重复性强的结构(参数、方法、REST 配置均一致)。修改范围局限于新文件添加和一个测试辅助方法,无复杂业务逻辑或跨模块影响。 诗歌
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Comment |
b7bbe7e to
e2697ae
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (3)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java (1)
5-7: Result类缺少必需的基类继承。与
CleanStoragePackageResult类似,此类也未继承APIReply或APIEvent,不符合API设计规范。应用以下修改:
-public class InstallZceXResult { +public class InstallZceXResult extends APIReply { }根据编码规范(基于规范)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java (1)
5-7: Result类缺少必需的基类继承。此类同样未继承
APIReply或APIEvent,与前面的Result类存在相同问题。应用以下修改:
-public class InstallZStoneResult { +public class InstallZStoneResult extends APIReply { }根据编码规范(基于规范)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageResult.java (1)
5-7: Result类缺少必需的基类继承。此类同样未继承
APIReply或APIEvent,与前面的Result类存在相同问题。应用以下修改:
-public class UploadStoragePackageResult { +public class UploadStoragePackageResult extends APIReply { }根据编码规范(基于规范)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (10)
conf/db/upgrade/V4.10.16__schema.sql(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageResult.java(1 hunks)testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
- 单个 if 代码块不宜超过一屏显示,以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则:
- 对于可以通过预检查避免的 RuntimeException(如
NullPointerException、IndexOutOfBoundsException等),不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况,不应将异常逻辑当作正常流程控制。
- 在必要时,应继续抛出异常,使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源,确保在 finally 块中正确关闭资源,并避免在 finally 中返回值。
...
Files:
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.javasdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageResult.javasdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageResult.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.javasdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.javasdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.javasdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java
**/*.sql
⚙️ CodeRabbit configuration file
**/*.sql: Review the SQL code, make sure has no errors and confirm that:
- Upgrading scene has been carefully handled
- Do not use
DEFAULT 0000-00-00 00:00:00, useDEFAULT CURRENT_TIMESTAMPinstead- When
NOT NULLexists, must usestored procedureor other functions to process historical data, this is very very important
Files:
conf/db/upgrade/V4.10.16__schema.sql
🧠 Learnings (2)
📚 Learning: 2025-08-03T04:10:21.683Z
Learnt from: zstack-robot-1
PR: MatheMatrix/zstack#0
File: :0-0
Timestamp: 2025-08-03T04:10:21.683Z
Learning: ZStack 数据库升级脚本 V4.10.16__schema.sql 中的 UPGRADE_VM_METADATA_TABLES_DIRECT 存储过程使用直接 RENAME TABLE 操作,不具备幂等性。需要通过 information_schema 检查表和约束的存在性来确保脚本可以安全重复执行。
Applied to files:
conf/db/upgrade/V4.10.16__schema.sql
📚 Learning: 2025-09-01T08:16:10.006Z
Learnt from: MatheMatrix
PR: MatheMatrix/zstack#2541
File: testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy:30889-30893
Timestamp: 2025-09-01T08:16:10.006Z
Learning: ApiHelper.groovy in testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy is auto-generated and should not be manually modified or receive code change suggestions.
Applied to files:
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
🔇 Additional comments (5)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageResult.java (1)
5-7: Result类缺少必需的基类继承。根据编码规范,API返回类必须继承
APIReply或APIEvent。当前类为空且未继承任何基类,不符合API设计要求。建议应用以下修改之一:
方案1:继承 APIReply
-public class CleanStoragePackageResult { +public class CleanStoragePackageResult extends APIReply { }方案2:继承 APIEvent
-public class CleanStoragePackageResult { +public class CleanStoragePackageResult extends APIEvent { }根据编码规范(基于规范)
请运行以下脚本验证其他相关Result类是否也存在同样的问题:
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (1)
45259-45283: 确认自动生成的代码符合预期模式。根据已有的学习记录,此文件是自动生成的,不应手动修改。新增的四个方法(
installZceX、installZStone、cleanStoragePackage和uploadStoragePackage)遵循了文件中现有方法的一致模式,包括会话绑定、闭包配置和 API 路径跟踪逻辑。代码结构正确,未发现关键问题。基于学习记录
Also applies to: 45479-45503, 45697-45748
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java (1)
9-11: 严重问题:静态字段未初始化静态字段
parameterMap和nonAPIParameterMap已声明但从未初始化。这将导致getParameterMap()和getNonAPIParameterMap()方法返回空的 HashMap,可能影响参数处理逻辑。建议初始化这些映射:
- private static final HashMap<String, Parameter> parameterMap = new HashMap<>(); + private static final HashMap<String, Parameter> parameterMap; - private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); + private static final HashMap<String, Parameter> nonAPIParameterMap; + + static { + parameterMap = new HashMap<>(); + // TODO: 根据 @Param 注解初始化 parameterMap + + nonAPIParameterMap = new HashMap<>(); + // TODO: 根据 @NonAPIParam 注解初始化 nonAPIParameterMap + }Likely an incorrect or invalid review comment.
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (1)
9-11: 严重问题:静态字段未初始化静态 HashMap 字段未初始化。这是所有新增 Action 类的共性问题,需要统一修复以避免运行时错误。
应用初始化修复:
- private static final HashMap<String, Parameter> parameterMap = new HashMap<>(); + private static final HashMap<String, Parameter> parameterMap; - private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); + private static final HashMap<String, Parameter> nonAPIParameterMap; + + static { + parameterMap = new HashMap<>(); + // TODO: 根据 @Param 注解初始化 parameterMap + + nonAPIParameterMap = new HashMap<>(); + // TODO: 根据 @NonAPIParam 注解初始化 nonAPIParameterMap + }Likely an incorrect or invalid review comment.
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (1)
9-11: 严重问题:静态字段未初始化静态 HashMap 字段未初始化,这是一个跨多个 Action 类的共性问题,建议统一修复。
应用相同的初始化模式:
- private static final HashMap<String, Parameter> parameterMap = new HashMap<>(); + private static final HashMap<String, Parameter> parameterMap; - private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); + private static final HashMap<String, Parameter> nonAPIParameterMap; + + static { + parameterMap = new HashMap<>(); + // TODO: 根据 @Param 注解初始化 parameterMap + + nonAPIParameterMap = new HashMap<>(); + // TODO: 根据 @NonAPIParam 注解初始化 nonAPIParameterMap + }Likely an incorrect or invalid review comment.
conf/db/upgrade/V4.10.16__schema.sql
Outdated
| CREATE TABLE IF NOT EXISTS `zstack`.`StoragePackageVO` ( | ||
| `uuid` char(32) NOT NULL UNIQUE, | ||
| `name` varchar(1024), | ||
| `hostUuid` varchar(1024), | ||
| `mnUuid` varchar(1024), | ||
| `installPath` varchar(1024), | ||
| `unzipInstallPath` varchar(1024), | ||
| `size` bigint unsigned, | ||
| `type` varchar(1024), | ||
| PRIMARY KEY (`uuid`) | ||
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8; |
There was a problem hiding this comment.
StoragePackageVO表缺少标准时间戳字段。
新创建的 StoragePackageVO 表缺少 createDate 和 lastOpDate 时间戳字段,这与同文件中其他VO表的标准模式不一致。
对比其他表的定义:
PluginDriverVO(行11-12):包含lastOpDate和createDate字段VolumeCbtBackupRecordVO(行26-27):包含lastOpDate和createDate字段ResourceAttributeKeyVO(行63-64):包含lastOpDate和createDate字段
缺少时间戳字段会导致:
- 无法追踪记录的创建和修改时间
- 与系统中其他VO表的设计不一致
- 可能影响审计和问题排查
建议应用以下修改:
CREATE TABLE IF NOT EXISTS `zstack`.`StoragePackageVO` (
`uuid` char(32) NOT NULL UNIQUE,
`name` varchar(1024),
`hostUuid` varchar(1024),
`mnUuid` varchar(1024),
`installPath` varchar(1024),
`unzipInstallPath` varchar(1024),
`size` bigint unsigned,
`type` varchar(1024),
+ `lastOpDate` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+ `createDate` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
PRIMARY KEY (`uuid`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;根据SQL编码规范(基于规范)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CREATE TABLE IF NOT EXISTS `zstack`.`StoragePackageVO` ( | |
| `uuid` char(32) NOT NULL UNIQUE, | |
| `name` varchar(1024), | |
| `hostUuid` varchar(1024), | |
| `mnUuid` varchar(1024), | |
| `installPath` varchar(1024), | |
| `unzipInstallPath` varchar(1024), | |
| `size` bigint unsigned, | |
| `type` varchar(1024), | |
| PRIMARY KEY (`uuid`) | |
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8; | |
| CREATE TABLE IF NOT EXISTS `zstack`.`StoragePackageVO` ( | |
| `uuid` char(32) NOT NULL UNIQUE, | |
| `name` varchar(1024), | |
| `hostUuid` varchar(1024), | |
| `mnUuid` varchar(1024), | |
| `installPath` varchar(1024), | |
| `unzipInstallPath` varchar(1024), | |
| `size` bigint unsigned, | |
| `type` varchar(1024), | |
| `lastOpDate` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, | |
| `createDate` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| PRIMARY KEY (`uuid`) | |
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8; |
🤖 Prompt for AI Agents
In conf/db/upgrade/V4.10.16__schema.sql around lines 278 to 288, the new
StoragePackageVO table is missing the standard timestamp columns used across VO
tables; add two columns named lastOpDate and createDate (both bigint unsigned
NOT NULL, matching the same types/order used by PluginDriverVO and others) into
the CREATE TABLE definition (place them with the other column definitions before
the PRIMARY KEY) so the table follows the repository schema convention and
supports creation/modification timestamps for auditing.
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java
Outdated
Show resolved
Hide resolved
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java
Outdated
Show resolved
Hide resolved
| public class CleanStoragePackageAction extends AbstractAction { | ||
|
|
||
| private static final HashMap<String, Parameter> parameterMap = new HashMap<>(); | ||
|
|
||
| private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); | ||
|
|
||
| public static class Result { | ||
| public ErrorCode error; | ||
| public org.zstack.sdk.zsv.storage.api.CleanStoragePackageResult value; | ||
|
|
||
| public Result throwExceptionIfError() { | ||
| if (error != null) { | ||
| throw new ApiException( | ||
| String.format("error[code: %s, description: %s, details: %s]", error.code, error.description, error.details) | ||
| ); | ||
| } | ||
|
|
||
| return this; | ||
| } | ||
| } | ||
|
|
||
| @Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false) | ||
| public java.lang.String uuid; | ||
|
|
||
| @Param(required = false) | ||
| public java.util.List systemTags; | ||
|
|
||
| @Param(required = false) | ||
| public java.util.List userTags; | ||
|
|
||
| @Param(required = false) | ||
| public String sessionId; | ||
|
|
||
| @Param(required = false) | ||
| public String accessKeyId; | ||
|
|
||
| @Param(required = false) | ||
| public String accessKeySecret; | ||
|
|
||
| @Param(required = false) | ||
| public String requestIp; | ||
|
|
||
| @NonAPIParam | ||
| public long timeout = -1; | ||
|
|
||
| @NonAPIParam | ||
| public long pollingInterval = -1; | ||
|
|
||
|
|
||
| private Result makeResult(ApiResult res) { | ||
| Result ret = new Result(); | ||
| if (res.error != null) { | ||
| ret.error = res.error; | ||
| return ret; | ||
| } | ||
|
|
||
| org.zstack.sdk.zsv.storage.api.CleanStoragePackageResult value = res.getResult(org.zstack.sdk.zsv.storage.api.CleanStoragePackageResult.class); | ||
| ret.value = value == null ? new org.zstack.sdk.zsv.storage.api.CleanStoragePackageResult() : value; | ||
|
|
||
| return ret; | ||
| } | ||
|
|
||
| public Result call() { | ||
| ApiResult res = ZSClient.call(this); | ||
| return makeResult(res); | ||
| } | ||
|
|
||
| public void call(final Completion<Result> completion) { | ||
| ZSClient.call(this, new InternalCompletion() { | ||
| @Override | ||
| public void complete(ApiResult res) { | ||
| completion.complete(makeResult(res)); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| protected Map<String, Parameter> getParameterMap() { | ||
| return parameterMap; | ||
| } | ||
|
|
||
| protected Map<String, Parameter> getNonAPIParameterMap() { | ||
| return nonAPIParameterMap; | ||
| } | ||
|
|
||
| protected RestInfo getRestInfo() { | ||
| RestInfo info = new RestInfo(); | ||
| info.httpMethod = "DELETE"; | ||
| info.path = "/zsv/storage/package/{uuid}/delete"; | ||
| info.needSession = true; | ||
| info.needPoll = true; | ||
| info.parameterName = "cleanStoragePackage"; | ||
| return info; | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
缺少 Javadoc 文档
根据编码规范要求,需要为此公共 API 类添加完整的文档注释,说明清理存储包操作的用途、参数要求和使用方式。
文档模板示例:
/**
* 清理存储包的 API 操作类
* <p>
* 用于删除指定 UUID 的存储包
* </p>
*/
public class CleanStoragePackageAction extends AbstractAction {
// ...
}4bc5335 to
6a0d808
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java (1)
7-107: Javadoc 文档仍然缺失此问题在之前的评审中已被标记,但尚未解决。根据编码规范第3节要求,公共 API 类及其方法必须配有有效的 Javadoc 注释。请为以下内容添加文档:
- 类
UploadStoragePackageAction的用途和使用场景- 内部类
Result的作用- 关键方法的说明
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java (1)
7-101: Javadoc 文档仍然缺失此问题在之前的评审中已被标记为需要修复,但当前代码中仍未添加文档注释。根据编码规范要求,公共 API 类必须配有完整的 Javadoc 说明,包括类的用途、参数要求和使用方式。
🧹 Nitpick comments (5)
conf/db/upgrade/V4.10.20__schema.sql (2)
3-9: 建议:评估 varchar(1024) 的合理性多个字段(
name、hostUuid、mnUuid、installPath、unzipInstallPath、type)使用了varchar(1024),这个长度可能过大。建议:
- UUID 字段(
hostUuid、mnUuid):通常 UUID 为 32 或 36 个字符,建议使用char(32)或char(36)。- type 字段:类型字段通常是枚举值,
varchar(1024)过大,建议使用varchar(64)或varchar(128)。- name 字段:如果是用户输入的名称,建议评估实际需求,可能
varchar(256)或varchar(512)更合理。- 路径字段(
installPath、unzipInstallPath):路径长度 1024 可能合理,但建议确认实际场景需求。应用以下优化建议:
CREATE TABLE IF NOT EXISTS `zstack`.`StoragePackageVO` ( `uuid` char(32) NOT NULL UNIQUE, - `name` varchar(1024), - `hostUuid` varchar(1024), - `mnUuid` varchar(1024), + `name` varchar(256), + `hostUuid` char(32), + `mnUuid` char(32), `installPath` varchar(1024), `unzipInstallPath` varchar(1024), `size` bigint unsigned, - `type` varchar(1024), + `type` varchar(64), PRIMARY KEY (`uuid`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
11-11: 建议:考虑使用 utf8mb4 字符集当前使用
utf8字符集,建议升级为utf8mb4以支持完整的 Unicode 字符(包括 emoji 等 4 字节字符)。这是 MySQL 的最佳实践。应用以下修改:
-) ENGINE=InnoDB DEFAULT CHARSET=utf8; +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageResult.java (1)
5-7: 建议:添加类级别文档说明空的结果类缺少 Javadoc 文档。根据编码规范,公共类应该有清晰的说明。建议添加类级别的文档,说明此结果类的用途和使用场景。
应用以下修改:
package org.zstack.sdk.zsv.storage.api; +/** + * 清理存储包操作的结果类 + * <p> + * 用于封装 CleanStoragePackage API 操作的返回结果。 + * 当前为空实现,可根据业务需求扩展添加返回字段。 + * </p> + */ public class CleanStoragePackageResult { }根据编码规范
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java (1)
5-7: 建议:添加类级别文档说明空的结果类缺少 Javadoc 文档。建议添加类级别的文档,说明此结果类的用途、适用场景以及与 InstallZStone API 的关系。
应用以下修改:
package org.zstack.sdk.zstone.api; +/** + * 安装 ZStone 组件操作的结果类 + * <p> + * 用于封装 InstallZStone API 操作的返回结果。 + * 当前为空实现,可根据业务需求扩展添加安装状态、错误信息等字段。 + * </p> + */ public class InstallZStoneResult { }根据编码规范
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageResult.java (1)
5-7: 建议:添加类级别文档说明空的结果类缺少 Javadoc 文档。建议添加类级别的文档,说明此结果类的用途、适用场景以及可能包含的返回信息(如上传进度、文件信息等)。
应用以下修改:
package org.zstack.sdk.zsv.storage.api; +/** + * 上传存储包操作的结果类 + * <p> + * 用于封装 UploadStoragePackage API 操作的返回结果。 + * 当前为空实现,未来可扩展添加上传状态、包信息、存储位置等字段。 + * </p> + */ public class UploadStoragePackageResult { }根据编码规范
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (10)
conf/db/upgrade/V4.10.20__schema.sql(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageResult.java(1 hunks)testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java
- sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java
- sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
- 单个 if 代码块不宜超过一屏显示,以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则:
- 对于可以通过预检查避免的 RuntimeException(如
NullPointerException、IndexOutOfBoundsException等),不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况,不应将异常逻辑当作正常流程控制。
- 在必要时,应继续抛出异常,使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源,确保在 finally 块中正确关闭资源,并避免在 finally 中返回值。
...
Files:
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageResult.javasdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageResult.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.javasdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.javasdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java
**/*.sql
⚙️ CodeRabbit configuration file
**/*.sql: Review the SQL code, make sure has no errors and confirm that:
- Upgrading scene has been carefully handled
- Do not use
DEFAULT 0000-00-00 00:00:00, useDEFAULT CURRENT_TIMESTAMPinstead- When
NOT NULLexists, must usestored procedureor other functions to process historical data, this is very very important
Files:
conf/db/upgrade/V4.10.20__schema.sql
🧬 Code graph analysis (2)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java (2)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java (4)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
🔇 Additional comments (2)
conf/db/upgrade/V4.10.20__schema.sql (1)
1-11: 确认 StoragePackageVO 是否全新引入
脚本搜索未在其他 SQL 升级脚本或 Java 代码中发现引用。若此表为全新创建,则NOT NULL约束无需历史数据迁移;如需迁移,请补充相应脚本。testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (1)
45479-45503: 实现符合现有Action Helper模式新增的
installZStonehelper 与文件里其它 Action helper 的处理流程保持一致,session 设置、闭包委派与 API 跟踪逻辑都匹配现行模式,看起来可以直接合入。
6a0d808 to
a9122d3
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java (1)
7-7: 仍然缺少类级别的 Javadoc 文档根据编码规范,公共 API 类必须添加完整的文档注释,说明清理存储包操作的用途、参数要求和使用方式。
建议添加如下文档:
+/** + * 清理存储包的 API 操作类 + * <p> + * 用于删除指定 UUID 的存储包,支持同步和异步调用方式 + * </p> + * + * <p>使用示例:</p> + * <pre> + * CleanStoragePackageAction action = new CleanStoragePackageAction(); + * action.uuid = "package-uuid"; + * action.sessionId = "session-id"; + * CleanStoragePackageAction.Result result = action.call(); + * </pre> + */ public class CleanStoragePackageAction extends AbstractAction {sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (2)
7-7: 仍然缺少类级别的 Javadoc 文档根据编码规范,公共 API 类必须添加完整的文档注释。请为 InstallZceXAction 类添加 Javadoc,说明 ZCE-X 插件安装操作的用途、参数说明和使用示例。
建议添加如下文档:
+/** + * 安装 ZCE-X 插件的 API 操作类 + * <p> + * 用于根据存储包 UUID 安装 ZCE-X 插件,支持可选的配置参数 + * </p> + * + * <p>使用示例:</p> + * <pre> + * InstallZceXAction action = new InstallZceXAction(); + * action.storagePackageUuid = "package-uuid"; + * action.config = "optional-config-json"; + * action.sessionId = "session-id"; + * InstallZceXAction.Result result = action.call(); + * </pre> + */ public class InstallZceXAction extends AbstractAction {
100-100: 严重问题:parameterName 设置为空字符串导致请求体格式错误parameterName 设置为空字符串
"",这会导致 POST 请求的 JSON 请求体格式为{"": {...params...}},与其他 Action 类不一致且可能导致服务端解析失败。根据 ZSClient 的实现逻辑,对于 POST 请求:
Map m = new HashMap(); m.put(info.parameterName, params); // 如果 parameterName 为 "",结果为 {"": {...}}应设置为有意义的参数名称:
- info.parameterName = ""; + info.parameterName = "installZceX";sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (2)
7-26: 缺少类的 Javadoc 文档根据编码规范第 3 节,公共 API 类必须配有有效的 Javadoc 注释。此问题在之前的评审中已被指出,但尚未修复。
请为该类添加完整的 Javadoc 文档,说明:
- 类的用途(安装 ZStone 插件)
- 主要参数说明(storagePackageUuid、config 等)
- 使用示例或注意事项
基于编码规范
建议的文档模板:
+/** + * 安装 ZStone 插件的 API 操作类 + * <p> + * 用于在系统中安装 ZStone 插件,需要提供存储包 UUID 和配置信息。 + * 此操作为异步操作,需要轮询结果。 + * </p> + * + * @see InstallZStoneResult + */ public class InstallZStoneAction extends AbstractAction {
94-102: 参数名称设置为空字符串第 100 行的
info.parameterName设置为空字符串,与项目中其他 Action 类(如UploadStoragePackageAction和CleanStoragePackageAction)的命名风格不一致。此问题在之前的评审中已被指出,但尚未修复。建议使用有意义的参数名称以保持 API 风格统一。
请运行以下脚本查看其他相关 Action 类使用的 parameterName:
#!/bin/bash # 查找其他存储包相关 Action 类的 parameterName 设置 ast-grep --pattern $'class $_ extends AbstractAction { $$$ RestInfo getRestInfo() { $$$ info.parameterName = $PARAM; $$$ } $$$ }' | rg -A 2 -B 2 "UploadStoragePackage|CleanStoragePackage|InstallZceX"根据查询结果,建议修改为:
- info.parameterName = ""; + info.parameterName = "installZStone";或使用与其他 Action 一致的命名。
🧹 Nitpick comments (1)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (1)
34-38: 建议为 List 类型添加泛型参数字段
systemTags和userTags使用了原始的List类型,缺少泛型参数。根据编码规范和 Java 最佳实践,应明确指定列表元素类型以提高类型安全性。建议修改:
- @Param(required = false) - public java.util.List systemTags; + @Param(required = false) + public java.util.List<String> systemTags; - @Param(required = false) - public java.util.List userTags; + @Param(required = false) + public java.util.List<String> userTags;请运行以下脚本验证项目中其他 Action 类的 List 类型使用规范:
#!/bin/bash # 查找其他 Action 类中的 systemTags 和 userTags 字段定义 rg -n -A 1 "systemTags|userTags" sdk/src/main/java/org/zstack/sdk/ --type java | head -30
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (10)
conf/db/upgrade/V4.10.20__schema.sql(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageResult.java(1 hunks)testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java
🚧 Files skipped from review as they are similar to previous changes (4)
- sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageResult.java
- sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageResult.java
- sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java
- conf/db/upgrade/V4.10.20__schema.sql
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
- 单个 if 代码块不宜超过一屏显示,以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则:
- 对于可以通过预检查避免的 RuntimeException(如
NullPointerException、IndexOutOfBoundsException等),不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况,不应将异常逻辑当作正常流程控制。
- 在必要时,应继续抛出异常,使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源,确保在 finally 块中正确关闭资源,并避免在 finally 中返回值。
...
Files:
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.javasdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.javasdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java
🧬 Code graph analysis (3)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (1)
sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (1)
sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java (2)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
🔇 Additional comments (10)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java (3)
13-26: LGTM!Result 内部类实现正确Result 类正确封装了错误码和结果值,throwExceptionIfError() 方法提供了便捷的错误处理机制,与其他 Action 类的实现保持一致。
56-67: LGTM!方法实现正确且符合规范makeResult、call 方法的实现逻辑清晰,正确处理了同步和异步调用场景,与框架约定一致。
Also applies to: 69-81
91-99: LGTM!REST 配置正确DELETE 方法配置正确,路径使用了 URL 变量
{uuid},parameterName 设置为有意义的 "cleanStoragePackage",符合 RESTful 约定。sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (2)
13-26: LGTM!核心逻辑实现正确Result 类、makeResult 方法和 call 方法的实现遵循了框架模式,正确处理了错误和结果映射。
Also applies to: 59-70, 72-84
94-99: LGTM!REST 路径和方法配置正确POST 方法和
/zce-x-plugin/install路径配置合理,needSession 和 needPoll 设置也符合安装操作的要求。testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (3)
45259-45283: 新增 installZceX 封装与既有模式一致完全沿用了现有 helper 的 session 注入与 ApiPathTracker 逻辑,便于测试侧调用。
45479-45503: installZStone 助手实现保持一致性实现方式与其它 API helper 对齐,确保 apipath 追踪与错误处理逻辑一致。
45697-45748: 存储包清理与上传接口封装得当延续统一的闭包委派与错误处理模式,便于后续测试脚本复用。
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (2)
59-70: LGTM!
makeResult方法实现正确,妥善处理了错误情况和空值情况。当结果为空时自动创建空的InstallZStoneResult对象,避免了 NPE 风险。
72-84: LGTM!同步和异步调用方法的实现符合 SDK 的标准模式,正确委托给
ZSClient.call进行实际的 API 调用和结果处理。
| @Param(required = true, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false) | ||
| public java.lang.String storagePackageUuid; |
There was a problem hiding this comment.
参数配置不一致:storagePackageUuid 应设置 nonempty = true
storagePackageUuid 参数标记为 required = true 但 nonempty = false,这意味着允许空字符串。对于必需的 UUID 参数,应当禁止空字符串以保证参数有效性。
建议修改为:
- @Param(required = true, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false)
+ @Param(required = true, maxLength = 255, nonempty = true, nullElements = false, emptyString = true, noTrim = false)
public java.lang.String storagePackageUuid;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Param(required = true, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false) | |
| public java.lang.String storagePackageUuid; | |
| @Param(required = true, maxLength = 255, nonempty = true, nullElements = false, emptyString = true, noTrim = false) | |
| public java.lang.String storagePackageUuid; |
🤖 Prompt for AI Agents
In sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java around lines
28-29, the storagePackageUuid field is marked required=true but nonempty=false
and emptyString=true allowing empty strings; change the annotation to disallow
empty strings for a required UUID by setting nonempty=true and emptyString=false
(keep other constraints like maxLength as-is) so the parameter validation
rejects empty string values.
| public class InstallZStoneResult { | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
缺少 Javadoc 文档和类实现
根据编码规范第 3 节,公共类必须配有有效的 Javadoc 注释。此外,该 Result 类完全为空,没有任何字段或方法。
请确认:
- 空类体是否为有意设计?如果是,请添加注释说明原因
- 是否缺少结果字段(如操作状态、返回数据等)?
建议添加 Javadoc 文档:
+/**
+ * ZStone 插件安装操作的结果类
+ * <p>
+ * 用于封装 ZStone 插件安装操作的返回结果
+ * </p>
+ */
public class InstallZStoneResult {
}如果需要添加结果字段,请参考类似的 Result 类实现模式。
请运行以下脚本验证其他 Result 类的实现模式:
🏁 Script executed:
#!/bin/bash
# 查找其他类似的 Result 类实现,以确认空类体是否符合项目规范
fd -e java "Result.java$" sdk/src/main/java/org/zstack/sdk/ --exec head -20 {} \;Length of output: 262417
添加公共类 Javadoc 注释
根据编码规范第 3 节,所有公共类必须配备有效的 Javadoc 注释。请为 InstallZStoneResult 添加类注释,如:
/**
* ZStone 插件安装操作的结果。
*/
public class InstallZStoneResult {
}🤖 Prompt for AI Agents
In sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java around
lines 5 to 7, the public class lacks a Javadoc class comment; add a standard
Javadoc block immediately above the class declaration (for example a concise
one-line description in Chinese like "ZStone 插件安装操作的结果。") following project
Javadoc style so the public class is properly documented.
| private static final HashMap<String, Parameter> parameterMap = new HashMap<>(); | ||
|
|
||
| private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); |
There was a problem hiding this comment.
❓ Verification inconclusive
验证静态参数映射表的初始化机制
静态 HashMap 字段已初始化,但代码中未见填充逻辑。请确认这些映射表是否通过 AbstractAction 基类的反射机制或框架代码自动填充 @param 和 @NonAPIParam 注解的参数。
如果框架未自动处理,需要添加静态初始化块:
static {
// 通过反射或手动方式填充 parameterMap 和 nonAPIParameterMap
}请运行以下脚本验证 AbstractAction 基类是否包含参数注册逻辑:
🏁 Script executed:
#!/bin/bash
# 描述:检查 AbstractAction 是否自动处理参数映射初始化
# 预期:找到反射或初始化相关代码
ast-grep --pattern $'class AbstractAction {
$$$
getParameterMap() {
$$$
}
$$$
}'
rg -nP --type=java 'class\s+AbstractAction' -A 50 | rg -P '(parameterMap|@Param|reflection|Field\.get)'Length of output: 0
添加参数映射表填充逻辑
未在 AbstractAction 基类中发现自动填充 parameterMap 与 nonAPIParameterMap 的实现;请在 CleanStoragePackageAction 中添加静态初始化块,通过反射或手动方式填充映射表,确保 @Param 与 @NonAPIParam 注解生效。
🤖 Prompt for AI Agents
In
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java
around lines 9 to 11, the static parameterMap and nonAPIParameterMap are
declared but never populated; add a static initialization block that reflects
over the action's declared fields, detects fields annotated with @Param and
@NonAPIParam, builds corresponding Parameter instances (name, type,
required/defaults as appropriate) and inserts them into parameterMap or
nonAPIParameterMap respectively; ensure the block runs once at class load time,
handles inherited fields if needed, and keeps the maps final and thread-safe
(populate local mutable maps then assign to the static finals) so the
annotations are honored at runtime.
| @Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false) | ||
| public java.lang.String uuid; |
There was a problem hiding this comment.
🧩 Analysis chain
验证 uuid 参数的 nonempty 约束
uuid 参数标记为 required = true 但 nonempty = true, emptyString = true,这意味着允许空字符串。对于 UUID 这种必须有实际值的参数,这样的配置可能不合理。
建议验证其他 Action 类中 UUID 参数的注解配置,确保一致性:
🏁 Script executed:
#!/bin/bash
# 描述:检查其他 Action 类中 uuid 参数的 @Param 配置
# 预期:找到一致的 nonempty 配置模式
rg -nP --type=java '@Param.*\n.*uuid' -A 1 | rg -B 1 'nonempty'Length of output: 0
🏁 Script executed:
#!/bin/bash
# 描述:检查其他 Action 类中 uuid 参数的 @Param 配置,排除当前 CleanStoragePackageAction
rg -n --type java 'public\s+.*String\s+uuid' -C3 -g '!sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java'Length of output: 263394
🏁 Script executed:
#!/bin/bash
# 描述:检查 sdk 下所有 *Action.java 中 uuid 参数的 @Param 注解配置
rg -nP -g '*Action.java' '@Param\([^)]*\)\s*\n\s*public\s+java\.lang\.String\s+uuid;' sdk/src/main/java/org/zstack/sdkLength of output: 0
修复 uuid 注解冲突
CleanStoragePackageAction.java 第28行:将
- @Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false)
+ @Param(required = true, nonempty = true, nullElements = false, emptyString = false, noTrim = false)以消除 nonempty=true 与 emptyString=true 的冲突。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false) | |
| public java.lang.String uuid; | |
| @Param(required = true, nonempty = true, nullElements = false, emptyString = false, noTrim = false) | |
| public java.lang.String uuid; |
🤖 Prompt for AI Agents
In
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java
around lines 28-29, the @Param annotation on uuid has conflicting attributes
nonempty=true and emptyString=true; update the annotation to remove the conflict
by setting emptyString=false (so it respects nonempty=true) or alternatively
remove nonempty=true and keep emptyString=true—prefer changing emptyString to
false to enforce non-empty UUID values.
a4a7710 to
341d470
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java (1)
5-7: 缺少公共类 Javadoc 注释根据编码规范第 3 节,所有公共类必须配备有效的 Javadoc 注释。此问题已在之前的评审中标记。
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (2)
7-104: 缺少公共 API 类 Javadoc 注释根据编码规范第 3 节,公共 API 类必须配有有效的 Javadoc 注释。此问题已在之前的评审中标记。
100-100: 参数命名不一致
parameterName设置为 "params",与UploadStoragePackageAction和CleanStoragePackageAction的命名风格不一致。此问题已在之前的评审中标记。sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java (3)
7-7: 为公共 API 类与关键方法补充 Javadoc便于生成文档与规范化使用(类、Result、call/throwExceptionIfError 等)。
示例:
/** * 清理存储包 * DELETE /zsv/storage/package/{uuid}/delete * 需要会话,支持轮询。 */ public class CleanStoragePackageAction extends AbstractAction { ... }As per coding guidelines
9-12: 确认参数映射表的初始化机制parameterMap/nonAPIParameterMap 未见填充逻辑;请确认框架是否通过反射自动处理。
建议运行脚本核查 AbstractAction/ZSClient 是否使用注解或反射构建参数校验:
#!/bin/bash # 查找 AbstractAction/ZSClient 中对 @Param/@NonAPIParam 的扫描与使用 rg -nP --type=java 'class\s+AbstractAction\b|class\s+ZSClient\b' -n -C3 rg -nP --type=java '@Param\(|@NonAPIParam' -C2 rg -nP --type=java '(getParameterMap|getNonAPIParameterMap)\s*\(' -C3
28-30: 修复 @param 注解冲突:禁止空字符串nonempty=true 与 emptyString=true 冲突,可能放过空字符串。
- @Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false) + @Param(required = true, nonempty = true, nullElements = false, emptyString = false, noTrim = false)As per coding guidelines
🧹 Nitpick comments (12)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java (2)
2-4: 建议移除多余的空行。包声明与类声明之间存在三个连续空行,建议保留一个空行即可,以提高代码整洁度。
应用此修改:
package org.zstack.sdk.zcex.api; - - public class InstallZceXResult {
5-7: 请确认空类体是否符合设计意图,并补充文档说明。该类完全为空,没有任何字段或方法。虽然这可能是有意为之(作为类型标记或占位符),但通常 Result 类应包含一些字段来承载返回数据。此外,缺少 Javadoc 注释会降低代码的可维护性。
建议:
- 确认该空类体是否是最终设计,还是后续需要添加字段
- 添加 Javadoc 注释说明该类的用途和设计意图
如需添加文档,可参考:
+/** + * InstallZceX 操作的结果类。 + * 用于封装 InstallZceXAction 的返回数据。 + */ public class InstallZceXResult {sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageResult.java (2)
2-4: 建议移除多余的空行。包声明与类声明之间存在三个连续空行,建议保留一个空行即可,以提高代码整洁度。
应用此修改:
package org.zstack.sdk.zsv.storage.api; - - public class CleanStoragePackageResult {
5-7: 请确认空类体是否符合设计意图,并补充文档说明。该类完全为空,没有任何字段或方法。虽然这可能是有意为之(作为类型标记或占位符),但通常 Result 类应包含一些字段来承载返回数据。此外,缺少 Javadoc 注释会降低代码的可维护性。
建议:
- 确认该空类体是否是最终设计,还是后续需要添加字段
- 添加 Javadoc 注释说明该类的用途和设计意图
如需添加文档,可参考:
+/** + * CleanStoragePackage 操作的结果类。 + * 用于封装 CleanStoragePackageAction 的返回数据。 + */ public class CleanStoragePackageResult {conf/db/upgrade/V4.10.20__schema.sql (4)
2-2: 去除冗余唯一约束以避免重复索引PRIMARY KEY 已保证唯一;无需额外 UNIQUE。
建议修改:
- `uuid` char(32) NOT NULL UNIQUE, + `uuid` char(32) NOT NULL,
4-5: 将 hostUuid/mnUuid 收敛为 char(32)与常规 UUID 存储一致且节省空间。
- `hostUuid` varchar(1024), - `mnUuid` varchar(1024), + `hostUuid` char(32), + `mnUuid` char(32),
12-12: 为常用查询列添加索引,避免后期全表扫描预计会按 mnUuid/hostUuid/name/type/createDate 查询或清理。
- PRIMARY KEY (`uuid`) + PRIMARY KEY (`uuid`), + KEY `idx_mnUuid` (`mnUuid`), + KEY `idx_hostUuid` (`hostUuid`), + KEY `idx_name` (`name`), + KEY `idx_type` (`type`), + KEY `idx_createDate` (`createDate`)说明:本文件为新表创建,放在 CREATE TABLE 内可确保幂等。As per coding guidelines
13-13: 可选:升级为 utf8mb4 字符集避免表情符号/四字节字符存储问题。
-) ENGINE=InnoDB DEFAULT CHARSET=utf8; +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java (1)
93-97: 可选:路径资源名建议使用复数形式REST 规范建议资源使用复数,例如 /zsv/storage/packages/{uuid}/delete;变更需与服务端一致。
As per coding guidelines
sdk/src/main/java/org/zstack/sdk/zsv/storage/entity/StoragePackageInventory.java (1)
3-4: 补充 Inventory 字段以匹配后端 VO空类将导致反序列化无可用字段;建议与 StoragePackageVO 对齐。
-public class StoragePackageInventory { -} +public class StoragePackageInventory { + public java.lang.String uuid; + public java.lang.String name; + public java.lang.String hostUuid; + public java.lang.String mnUuid; + public java.lang.String installPath; + public java.lang.String unzipInstallPath; + public java.lang.Long size; + public java.lang.String type; + public java.sql.Timestamp createDate; + public java.sql.Timestamp lastOpDate; +}如需,我可以补充 Javadoc 与示例。As per coding guidelines
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java (2)
7-26: 为公共 API 类及关键方法补充 Javadoc请为类、Result、throwExceptionIfError、call(…) 等添加简明 Javadoc 以利于文档生成与使用。
As per coding guidelines
101-104: 可选:资源路径建议使用复数REST 资源建议复数,如 /zsv/storage/packages/upload;需与后端保持一致后再调整。
As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (12)
conf/db/upgrade/V4.10.20__schema.sql(1 hunks)sdk/src/main/java/SourceClassMap.java(2 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zsv/storage/entity/StoragePackageInventory.java(1 hunks)testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- sdk/src/main/java/SourceClassMap.java
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
- 单个 if 代码块不宜超过一屏显示,以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则:
- 对于可以通过预检查避免的 RuntimeException(如
NullPointerException、IndexOutOfBoundsException等),不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况,不应将异常逻辑当作正常流程控制。
- 在必要时,应继续抛出异常,使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源,确保在 finally 块中正确关闭资源,并避免在 finally 中返回值。
...
Files:
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageResult.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.javasdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.javasdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageResult.javasdk/src/main/java/org/zstack/sdk/zsv/storage/entity/StoragePackageInventory.javasdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.javasdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java
**/*.sql
⚙️ CodeRabbit configuration file
**/*.sql: Review the SQL code, make sure has no errors and confirm that:
- Upgrading scene has been carefully handled
- Do not use
DEFAULT 0000-00-00 00:00:00, useDEFAULT CURRENT_TIMESTAMPinstead- When
NOT NULLexists, must usestored procedureor other functions to process historical data, this is very very important
Files:
conf/db/upgrade/V4.10.20__schema.sql
🧠 Learnings (1)
📚 Learning: 2025-09-01T03:42:45.051Z
Learnt from: MatheMatrix
PR: MatheMatrix/zstack#2541
File: sdk/src/main/java/org/zstack/sdk/guesttools/GuestToolsStateInventory.java:1-1
Timestamp: 2025-09-01T03:42:45.051Z
Learning: The GuestToolsStateInventory SDK class in org.zstack.sdk package is not referenced by external clients, so package migrations for this component don't require backward compatibility aliases.
Applied to files:
sdk/src/main/java/org/zstack/sdk/zsv/storage/entity/StoragePackageInventory.java
🧬 Code graph analysis (4)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java (2)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageResult.java (1)
sdk/src/main/java/org/zstack/sdk/zsv/storage/entity/StoragePackageInventory.java (1)
StoragePackageInventory(3-4)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java (2)
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (4)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/CleanStoragePackageAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
🔇 Additional comments (6)
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (3)
45259-45282: 实现保持与既有 ApiHelper 模式一致新封装完整设置 session、委托闭包并处理 API Path 追踪,与文件内其它 Action 封装保持一致,逻辑清晰,无需进一步调整。
45479-45502: ZStone 安装封装与现有模式一致整体流程(session 绑定、闭包代理、调用追踪)与 ApiHelper 其他方法统一,可直接复用,无额外问题。
45697-45748: 新增存储包清理/上传封装满足调用需求两段封装沿用统一的 action 模板,包含错误处理与 API 路径记录,符合预期用法。
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageResult.java (1)
5-12: LGTM结构简洁、符合现有 SDK Result 模式。
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java (2)
9-11: 确认参数映射表自动填充 AbstractAction.initializeParametersIfNot 会反射扫描字段上的 @Param/@NonAPIParam 注解并填充 parameterMap 与 nonAPIParameterMap,无需手动初始化。
31-33: 确认 url 字段在服务器端消息类中是否为必填 UploadStoragePackageAction.url 当前标记为required=false,但上传操作通常需要源地址,请检查服务端 APIMessage 类中@APIParam(required=…)的设置。
| @Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false) | ||
| public java.lang.String managementNodeUuid; | ||
|
|
||
| @Param(required = false) | ||
| public java.lang.String url; | ||
|
|
||
| @Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false) | ||
| public java.lang.String installPath; | ||
|
|
There was a problem hiding this comment.
修复 @param 注解冲突(必填字段禁止空串)
两个必填字段同时设置 nonempty=true 与 emptyString=true。
- @Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false)
+ @Param(required = true, nonempty = true, nullElements = false, emptyString = false, noTrim = false)
public java.lang.String managementNodeUuid;
@@
- @Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false)
+ @Param(required = true, nonempty = true, nullElements = false, emptyString = false, noTrim = false)
public java.lang.String installPath;As per coding guidelines
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false) | |
| public java.lang.String managementNodeUuid; | |
| @Param(required = false) | |
| public java.lang.String url; | |
| @Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false) | |
| public java.lang.String installPath; | |
| @Param(required = true, nonempty = true, nullElements = false, emptyString = false, noTrim = false) | |
| public java.lang.String managementNodeUuid; | |
| @Param(required = false) | |
| public java.lang.String url; | |
| @Param(required = true, nonempty = true, nullElements = false, emptyString = false, noTrim = false) | |
| public java.lang.String installPath; |
🤖 Prompt for AI Agents
In
sdk/src/main/java/org/zstack/sdk/zsv/storage/api/UploadStoragePackageAction.java
around lines 28 to 36, the @Param annotations for the required fields
managementNodeUuid and installPath have conflicting attributes (nonempty = true
together with emptyString = true); update those annotations so they forbid empty
strings (set emptyString = false or remove the emptyString=true) to be
consistent with nonempty=true and the coding guideline, leaving the optional
field url unchanged.
557d467 to
8c2a478
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (1)
28-29: 参数配置不一致:storagePackageUuid 应设置 nonempty = truestoragePackageUuid 参数标记为
required = true但nonempty = false,这意味着允许空字符串。对于必需的 UUID 参数,应当禁止空字符串以保证参数有效性。建议修改为:
- @Param(required = true, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false) + @Param(required = true, maxLength = 255, nonempty = true, nullElements = false, emptyString = false, noTrim = false) public java.lang.String storagePackageUuid;
🧹 Nitpick comments (1)
conf/db/upgrade/V4.10.20__schema.sql (1)
1-13: PRIMARY KEY 上的 UNIQUE 约束是冗余的在第 2 行,
uuid列同时被声明为NOT NULL UNIQUE和PRIMARY KEY(第 12 行)。PRIMARY KEY 本身已经强制唯一性,UNIQUE约束是多余的。建议移除冗余的
UNIQUE约束:CREATE TABLE IF NOT EXISTS `zstack`.`StoragePackageVO` ( - `uuid` char(32) NOT NULL UNIQUE, + `uuid` char(32) NOT NULL,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (6)
conf/db/upgrade/V4.10.20__schema.sql(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java(1 hunks)testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
- 单个 if 代码块不宜超过一屏显示,以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则:
- 对于可以通过预检查避免的 RuntimeException(如
NullPointerException、IndexOutOfBoundsException等),不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况,不应将异常逻辑当作正常流程控制。
- 在必要时,应继续抛出异常,使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源,确保在 finally 块中正确关闭资源,并避免在 finally 中返回值。
...
Files:
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java
**/*.sql
⚙️ CodeRabbit configuration file
**/*.sql: Review the SQL code, make sure has no errors and confirm that:
- Upgrading scene has been carefully handled
- Do not use
DEFAULT 0000-00-00 00:00:00, useDEFAULT CURRENT_TIMESTAMPinstead- When
NOT NULLexists, must usestored procedureor other functions to process historical data, this is very very important
Files:
conf/db/upgrade/V4.10.20__schema.sql
🧠 Learnings (1)
📚 Learning: 2025-09-01T08:16:10.006Z
Learnt from: MatheMatrix
PR: MatheMatrix/zstack#2541
File: testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy:30889-30893
Timestamp: 2025-09-01T08:16:10.006Z
Learning: ApiHelper.groovy in testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy is auto-generated and should not be manually modified or receive code change suggestions.
Applied to files:
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
🧬 Code graph analysis (2)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (2)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (2)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
🔇 Additional comments (9)
conf/db/upgrade/V4.10.20__schema.sql (1)
10-11: 时间戳列的默认值配置正确
lastOpDate和createDate列正确使用了DEFAULT CURRENT_TIMESTAMP,符合编码规范要求(不使用已废弃的0000-00-00 00:00:00)。testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (2)
45479-45503: installZStone 助手新增:实现与现有模式一致,LGTM
- 与同类助手一致的调用/追踪模式;命名与后续 queryZStone 对齐。
- 同样请确认该改动由生成器产出且 SDK 已包含 org.zstack.sdk.zstone.api.InstallZStoneAction/InstallZStoneResult。若需验证,参考上方脚本思路替换为 ZStone 对应文件名即可。
建议在仓库中同时检查:
- 文件存在性与包名:InstallZStoneAction.java、InstallZStoneResult.java 属于 org.zstack.sdk.zstone.api;
- CI 是否按需设置 -Dapipath 以收集并断言 Test.apiPaths。
Based on learnings
45259-45283: installZceX 助手与 SDK 类验证通过,代码符合现有模式SDK 侧相关类已确认存在:
InstallZceXAction位于org.zstack.sdk.zcex.api包 ✓InstallZceXResult位于org.zstack.sdk.zcex.api包 ✓实现逻辑与既有 API 助手保持一致,代码生成流程正常。
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (6)
13-26: Result 类实现正确内部类
Result的错误处理逻辑清晰,throwExceptionIfError()方法正确实现了异常抛出和链式调用,与 SDK 其他 Action 类的模式一致。
28-56: 参数声明规范所有参数字段的命名和注解配置符合规范:
- 必填参数
storagePackageUuid和config正确标注- 可选参数使用
required = false- 非 API 参数
timeout和pollingInterval使用@NonAPIParam注解,默认值 -1 将被 ZSClient 映射为配置的默认轮询超时和间隔- 命名遵循 lowerCamelCase 规范
59-70: makeResult 方法实现正确方法正确处理了错误和成功两种情况:
- 错误时直接复制 error 字段
- 成功时类型安全地提取结果,并对 null 值进行防护处理
72-84: 调用方法实现正确同步和异步调用方法均正确实现:
- 同步方法直接调用 ZSClient 并返回结果
- 异步方法使用回调机制,正确包装结果转换逻辑
- 两种调用方式清晰分离,符合 SDK 设计模式
86-92: 参数 Map 获取方法实现正确这两个方法正确返回静态参数 Map,但需要先解决 lines 9-11 中标记的静态初始化问题,确保这些 Map 被正确填充。
94-102: REST 配置正确REST 元数据配置合理:
- 使用 POST 方法进行安装操作,符合编码规范
needPoll = true表明这是长时间运行的异步操作parameterName = "params"已在之前的 commit 中修复(之前为空字符串)
| public class InstallZceXAction extends AbstractAction { | ||
|
|
||
| private static final HashMap<String, Parameter> parameterMap = new HashMap<>(); | ||
|
|
||
| private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); | ||
|
|
||
| public static class Result { | ||
| public ErrorCode error; | ||
| public org.zstack.sdk.zcex.api.InstallZceXResult value; | ||
|
|
||
| public Result throwExceptionIfError() { | ||
| if (error != null) { | ||
| throw new ApiException( | ||
| String.format("error[code: %s, description: %s, details: %s]", error.code, error.description, error.details) | ||
| ); | ||
| } | ||
|
|
||
| return this; | ||
| } | ||
| } | ||
|
|
||
| @Param(required = true, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false) | ||
| public java.lang.String storagePackageUuid; | ||
|
|
||
| @Param(required = false, nonempty = false, nullElements = false, emptyString = true, noTrim = false) | ||
| public java.lang.String config; | ||
|
|
||
| @Param(required = false) | ||
| public java.util.List systemTags; | ||
|
|
||
| @Param(required = false) | ||
| public java.util.List userTags; | ||
|
|
||
| @Param(required = false) | ||
| public String sessionId; | ||
|
|
||
| @Param(required = false) | ||
| public String accessKeyId; | ||
|
|
||
| @Param(required = false) | ||
| public String accessKeySecret; | ||
|
|
||
| @Param(required = false) | ||
| public String requestIp; | ||
|
|
||
| @NonAPIParam | ||
| public long timeout = -1; | ||
|
|
||
| @NonAPIParam | ||
| public long pollingInterval = -1; | ||
|
|
||
|
|
||
| private Result makeResult(ApiResult res) { | ||
| Result ret = new Result(); | ||
| if (res.error != null) { | ||
| ret.error = res.error; | ||
| return ret; | ||
| } | ||
|
|
||
| org.zstack.sdk.zcex.api.InstallZceXResult value = res.getResult(org.zstack.sdk.zcex.api.InstallZceXResult.class); | ||
| ret.value = value == null ? new org.zstack.sdk.zcex.api.InstallZceXResult() : value; | ||
|
|
||
| return ret; | ||
| } | ||
|
|
||
| public Result call() { | ||
| ApiResult res = ZSClient.call(this); | ||
| return makeResult(res); | ||
| } | ||
|
|
||
| public void call(final Completion<Result> completion) { | ||
| ZSClient.call(this, new InternalCompletion() { | ||
| @Override | ||
| public void complete(ApiResult res) { | ||
| completion.complete(makeResult(res)); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| protected Map<String, Parameter> getParameterMap() { | ||
| return parameterMap; | ||
| } | ||
|
|
||
| protected Map<String, Parameter> getNonAPIParameterMap() { | ||
| return nonAPIParameterMap; | ||
| } | ||
|
|
||
| protected RestInfo getRestInfo() { | ||
| RestInfo info = new RestInfo(); | ||
| info.httpMethod = "POST"; | ||
| info.path = "/zce-x-plugin/install"; | ||
| info.needSession = true; | ||
| info.needPoll = true; | ||
| info.parameterName = "params"; | ||
| return info; | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
缺少 example 方法
根据编码规范要求:"API 类需要实现 __example__ 方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。" 当前类缺少该方法。
建议添加 __example__ 方法以支持 API 文档生成。
As per coding guidelines.
🤖 Prompt for AI Agents
In sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java around lines
7 to 104, the class is missing the required static __example__() method used to
generate API docs and Groovy/Markdown templates; implement a public static
InstallZceXAction __example__() that constructs and returns a populated example
instance (set required fields like storagePackageUuid and any common optional
fields such as config/systemTags/userTags) following patterns from other Action
classes in the SDK, and place it within the class so documentation generators
can find it.
| private static final HashMap<String, Parameter> parameterMap = new HashMap<>(); | ||
|
|
||
| private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); |
There was a problem hiding this comment.
缺少静态初始化块注册参数
静态字段 parameterMap 和 nonAPIParameterMap 已声明但从未填充。这会导致 getParameterMap() 和 getNonAPIParameterMap() 返回空的 Map,从而使 ZSClient 无法正确构建 API 请求参数。
所有使用 @Param 注解的字段(如 storagePackageUuid、config、systemTags 等)以及使用 @NonAPIParam 注解的字段(如 timeout、pollingInterval)都需要在静态初始化块中注册。
建议添加静态初始化块:
private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>();
+
+ static {
+ parameterMap.put("storagePackageUuid", new Parameter("storagePackageUuid", "java.lang.String", true, false, false, false, true, false, new String[0], 255, false, false));
+ parameterMap.put("config", new Parameter("config", "java.lang.String", true, false, false, false, false, false, new String[0], 0, false, false));
+ parameterMap.put("systemTags", new Parameter("systemTags", "java.util.List", false, false, false, false, false, false, new String[0], 0, false, false));
+ parameterMap.put("userTags", new Parameter("userTags", "java.util.List", false, false, false, false, false, false, new String[0], 0, false, false));
+ parameterMap.put("sessionId", new Parameter("sessionId", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false));
+ parameterMap.put("accessKeyId", new Parameter("accessKeyId", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false));
+ parameterMap.put("accessKeySecret", new Parameter("accessKeySecret", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false));
+ parameterMap.put("requestIp", new Parameter("requestIp", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false));
+
+ nonAPIParameterMap.put("timeout", new Parameter("timeout", "long", false, false, false, false, false, false, new String[0], 0, false, false));
+ nonAPIParameterMap.put("pollingInterval", new Parameter("pollingInterval", "long", false, false, false, false, false, false, new String[0], 0, false, false));
+ }
public static class Result {注意:请参考项目中其他 SDK Action 类(如 InstallZceXAction)的完整实现来确保 Parameter 构造函数参数的准确性。
🤖 Prompt for AI Agents
In sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java around
lines 9 to 11, the static maps parameterMap and nonAPIParameterMap are declared
but never populated; add a static initialization block that registers every
field annotated with @Param (e.g. storagePackageUuid, config, systemTags,
userTags, sessionId, accessKeyId, accessKeySecret, regionId, zoneId,
accountUuid, resourceUuid, name, description, dryRun, managementNodeUuid,
affinityGroupUuid, createDate, lastOpDate) into parameterMap and every
@NonAPIParam field (e.g. timeout, pollingInterval, nonAPIParams if present) into
nonAPIParameterMap using Parameter constructors as used elsewhere; follow the
exact constructor argument order and validation flags from the existing
InstallZceXAction implementation, call parameterMap.put("fieldName", param) and
nonAPIParameterMap.put("fieldName", param), and ensure getParameterMap() /
getNonAPIParameterMap() will then return the populated maps.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (2)
7-104: 缺少 API 文档和示例方法根据编码规范,该公共 API 类存在以下问题:
- 缺少 Javadoc 文档:公共 API 类及其关键方法需要完整的文档注释,说明 ZCE-X 插件安装操作的用途、参数和行为
- 缺少
__example__方法:编码规范明确要求"API 类需要实现__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件"建议添加:
- 类级别 Javadoc 说明该 Action 的用途和 REST 端点
- 为
Result类及其throwExceptionIfError方法添加简要 Javadoc- 为公共参数字段添加文档,说明预期值和约束
- 为
call(),call(Completion),getRestInfo()和makeResult()方法添加文档- 实现
__example__方法用于 API 文档生成基于 coding guidelines
28-29: 参数验证配置不一致:storagePackageUuid 应禁止空字符串
storagePackageUuid参数标记为required = true,但nonempty = false且emptyString = true,这意味着允许传入空字符串。对于必需的 UUID 参数,应当禁止空字符串以保证参数有效性。建议修改为:
- @Param(required = true, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false) + @Param(required = true, maxLength = 255, nonempty = true, nullElements = false, emptyString = false, noTrim = false) public java.lang.String storagePackageUuid;
🧹 Nitpick comments (4)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (1)
66-67: 考虑优化空值处理逻辑Line 67 在
value为null时创建一个空的InstallZStoneResult对象作为回退。如果 API 预期应该返回结果,null可能表示存在问题。建议评估这种回退逻辑是否会掩盖潜在的 API 错误。sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java (1)
3-4: 移除多余的空行第 3-4 行存在不必要的空行,影响代码整洁性。
应用此修改:
package org.zstack.sdk.zcex.api; - - public class InstallZceXResult {testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (2)
45267-45283: 建议消除分支重复调用样式,统一跟踪开关,降低修改面当前在有/无 apipath 的分支中分别 return errorOut(a.call()),结构上重复。可用一个布尔开关统一流程,避免未来改动遗漏。示例改造如下(installZceX 段):
- if (System.getProperty("apipath") != null) { - if (a.apiId == null) { - a.apiId = Platform.uuid - } - - def tracker = new ApiPathTracker(a.apiId) - def out = errorOut(a.call()) - def path = tracker.getApiPath() - if (!path.isEmpty()) { - Test.apiPaths[a.class.name] = path.join(" --->\n") - } - - return out - } else { - return errorOut(a.call()) - } + def track = System.getProperty("apipath") != null + def tracker = null + if (track) { + if (a.apiId == null) { + a.apiId = Platform.uuid + } + tracker = new ApiPathTracker(a.apiId) + } + def out = errorOut(a.call()) + if (track) { + def path = tracker.getApiPath() + if (!path.isEmpty()) { + Test.apiPaths[a.class.name] = path.join(" --->\n") + } + } + return out同理可在 installZStone 段套用相同模式:
- if (System.getProperty("apipath") != null) { - if (a.apiId == null) { - a.apiId = Platform.uuid - } - def tracker = new ApiPathTracker(a.apiId) - def out = errorOut(a.call()) - def path = tracker.getApiPath() - if (!path.isEmpty()) { - Test.apiPaths[a.class.name] = path.join(" --->\n") - } - return out - } else { - return errorOut(a.call()) - } + def track = System.getProperty("apipath") != null + def tracker = null + if (track) { + if (a.apiId == null) { + a.apiId = Platform.uuid + } + tracker = new ApiPathTracker(a.apiId) + } + def out = errorOut(a.call()) + if (track) { + def path = tracker.getApiPath() + if (!path.isEmpty()) { + Test.apiPaths[a.class.name] = path.join(" --->\n") + } + } + return outAlso applies to: 45487-45503
45261-45261: 可选:为空 session 提前给出更友好的断言,便于定位问题当 Test.currentEnvSpec?.session?.uuid 为空时,后续调用大概率直接 401。测试场景下可提前断言,错误信息更直观(可只在测试开关启用)。
- a.sessionId = Test.currentEnvSpec?.session?.uuid + a.sessionId = Test.currentEnvSpec?.session?.uuid + assert a.sessionId : "ApiHelper.${this.&installZceX ? 'installZceX' : 'installZStone'}: missing session UUID; did you forget to login the env?"如不希望引入 assert,可考虑在 sessionId 为空时记录 WARN 日志即可。
Also applies to: 45481-45481
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (6)
conf/db/upgrade/V4.10.20__schema.sql(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java(1 hunks)testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java
- conf/db/upgrade/V4.10.20__schema.sql
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
- 单个 if 代码块不宜超过一屏显示,以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则:
- 对于可以通过预检查避免的 RuntimeException(如
NullPointerException、IndexOutOfBoundsException等),不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况,不应将异常逻辑当作正常流程控制。
- 在必要时,应继续抛出异常,使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源,确保在 finally 块中正确关闭资源,并避免在 finally 中返回值。
...
Files:
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.javasdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java
🧬 Code graph analysis (2)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (2)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (2)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
🔇 Additional comments (5)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java (1)
5-7: 空结果类的设计是合理且一致的验证确认
InstallZceXResult作为空类是有意设计,而非疏漏:
- 相同模式:
RemoveZceXResult同样为空类,用于仅执行操作而不返回数据的 API- 对比差异:
AddZceXResult包含ZceXInventory因为创建操作需要返回资源清单;GetZceXCapabilityResult包含多个视图对象因为查询操作需要返回数据- 设计一致性:安装和删除等操作只需信号成功/失败状态,不必返回额外数据结构
此设计符合 ZceX API 模块的既有模式,无需修改。
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (3)
45259-45266: 新增 installZceX 方法整体符合既有模式,逻辑清晰会话绑定、闭包委托与后续 apipath 跟踪入口衔接合理,无明显功能性问题。LGTM。
45479-45486: 新增 installZStone 方法与既有 ApiHelper 风格一致实现流程与其他 Action 一致,易于维护与复用。LGTM。
45262-45264: 核对结果:无需变更 — ApiHelper.groovy 中 resolveStrategy 风格一致为 Closure.OWNER_FIRST已检索文件;行 45262–45264 与 45482–45484 的 c.resolveStrategy = Closure.OWNER_FIRST 与文件中其它匹配项一致,原审查关于 OWNER_FIRST/DELEGATE_FIRST 不一致的担忧不成立。若要切换为 DELEGATE_FIRST,须在全文件统一替换并验证闭包语义变化。
Likely an incorrect or invalid review comment.
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (1)
94-102: REST 元数据配置正确
getRestInfo方法的配置合理:
- HTTP 方法为 POST,符合创建/安装操作的约定
parameterName = "params"已修正(之前评审中指出的空字符串问题已解决)needSession和needPoll设置符合异步操作的要求实现与
InstallZStoneAction等类似 Action 保持一致。
| public class InstallZStoneAction extends AbstractAction { | ||
|
|
||
| private static final HashMap<String, Parameter> parameterMap = new HashMap<>(); | ||
|
|
||
| private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); | ||
|
|
||
| public static class Result { | ||
| public ErrorCode error; | ||
| public org.zstack.sdk.zstone.api.InstallZStoneResult value; | ||
|
|
||
| public Result throwExceptionIfError() { | ||
| if (error != null) { | ||
| throw new ApiException( | ||
| String.format("error[code: %s, description: %s, details: %s]", error.code, error.description, error.details) | ||
| ); | ||
| } | ||
|
|
||
| return this; | ||
| } | ||
| } | ||
|
|
||
| @Param(required = true, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false) | ||
| public java.lang.String storagePackageUuid; | ||
|
|
||
| @Param(required = true, nonempty = false, nullElements = false, emptyString = false, noTrim = false) | ||
| public java.lang.String config; | ||
|
|
||
| @Param(required = false) | ||
| public java.util.List systemTags; | ||
|
|
||
| @Param(required = false) | ||
| public java.util.List userTags; | ||
|
|
||
| @Param(required = false) | ||
| public String sessionId; | ||
|
|
||
| @Param(required = false) | ||
| public String accessKeyId; | ||
|
|
||
| @Param(required = false) | ||
| public String accessKeySecret; | ||
|
|
||
| @Param(required = false) | ||
| public String requestIp; | ||
|
|
||
| @NonAPIParam | ||
| public long timeout = -1; | ||
|
|
||
| @NonAPIParam | ||
| public long pollingInterval = -1; | ||
|
|
||
|
|
||
| private Result makeResult(ApiResult res) { | ||
| Result ret = new Result(); | ||
| if (res.error != null) { | ||
| ret.error = res.error; | ||
| return ret; | ||
| } | ||
|
|
||
| org.zstack.sdk.zstone.api.InstallZStoneResult value = res.getResult(org.zstack.sdk.zstone.api.InstallZStoneResult.class); | ||
| ret.value = value == null ? new org.zstack.sdk.zstone.api.InstallZStoneResult() : value; | ||
|
|
||
| return ret; | ||
| } | ||
|
|
||
| public Result call() { | ||
| ApiResult res = ZSClient.call(this); | ||
| return makeResult(res); | ||
| } | ||
|
|
||
| public void call(final Completion<Result> completion) { | ||
| ZSClient.call(this, new InternalCompletion() { | ||
| @Override | ||
| public void complete(ApiResult res) { | ||
| completion.complete(makeResult(res)); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| protected Map<String, Parameter> getParameterMap() { | ||
| return parameterMap; | ||
| } | ||
|
|
||
| protected Map<String, Parameter> getNonAPIParameterMap() { | ||
| return nonAPIParameterMap; | ||
| } | ||
|
|
||
| protected RestInfo getRestInfo() { | ||
| RestInfo info = new RestInfo(); | ||
| info.httpMethod = "POST"; | ||
| info.path = "/zstone-plugin/install"; | ||
| info.needSession = true; | ||
| info.needPoll = true; | ||
| info.parameterName = "params"; | ||
| return info; | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
缺少 example 方法用于 API 文档生成
根据编码规范第 1 条"API 设计要求",API 类需要实现 __example__ 方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。当前类缺少此方法。
建议在类中添加 __example__ 方法:
public static InstallZStoneAction __example__() {
InstallZStoneAction action = new InstallZStoneAction();
action.storagePackageUuid = uuid();
action.config = "example-config-content";
action.systemTags = Arrays.asList("system-tag-example");
action.userTags = Arrays.asList("user-tag-example");
return action;
}| private static final HashMap<String, Parameter> parameterMap = new HashMap<>(); | ||
|
|
||
| private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); |
There was a problem hiding this comment.
缺少静态初始化块填充参数映射
声明了静态参数映射 parameterMap 和 nonAPIParameterMap,但未提供静态初始化块来填充它们。根据 SDK Action 的标准模式,需要在静态块中将所有 @Param 注解的字段注册到 parameterMap,将 @NonAPIParam 注解的字段注册到 nonAPIParameterMap。缺少这些初始化会导致参数元数据不可用,影响参数验证和请求构建。
建议添加静态初始化块:
private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>();
+
+ static {
+ parameterMap.put("storagePackageUuid", new Parameter("storagePackageUuid", "java.lang.String", true, 255, false, false, true, false));
+ parameterMap.put("config", new Parameter("config", "java.lang.String", true, 0, false, false, false, false));
+ parameterMap.put("systemTags", new Parameter("systemTags", "java.util.List", false, 0, false, false, false, false));
+ parameterMap.put("userTags", new Parameter("userTags", "java.util.List", false, 0, false, false, false, false));
+ parameterMap.put("sessionId", new Parameter("sessionId", "java.lang.String", false, 0, false, false, false, false));
+ parameterMap.put("accessKeyId", new Parameter("accessKeyId", "java.lang.String", false, 0, false, false, false, false));
+ parameterMap.put("accessKeySecret", new Parameter("accessKeySecret", "java.lang.String", false, 0, false, false, false, false));
+ parameterMap.put("requestIp", new Parameter("requestIp", "java.lang.String", false, 0, false, false, false, false));
+
+ nonAPIParameterMap.put("timeout", new Parameter("timeout", "long", false, 0, false, false, false, false));
+ nonAPIParameterMap.put("pollingInterval", new Parameter("pollingInterval", "long", false, 0, false, false, false, false));
+ }
public static class Result {🤖 Prompt for AI Agents
In sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java around
lines 9 to 11, the static HashMaps parameterMap and nonAPIParameterMap are
declared but not populated; add a static initialization block that registers all
fields annotated with @Param into parameterMap and all fields annotated with
@NonAPIParam into nonAPIParameterMap following the project’s SDK Action pattern
(use the same reflection/Parameter construction logic used in other Action
classes — create Parameter entries keyed by the field name and include metadata
such as required/defaults/types), so parameter metadata is available for
validation and request building.
8c2a478 to
4e370b0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (2)
9-11: 缺少静态初始化块填充参数映射声明了
parameterMap和nonAPIParameterMap但未填充,导致getParameterMap()和getNonAPIParameterMap()返回空 Map。这会使 ZSClient 无法正确构建 API 请求参数和读取非 API 参数(如 timeout、pollingInterval),从而导致运行时失败。建议添加静态初始化块注册所有参数:
private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); + + static { + parameterMap.put("storagePackageUuid", new Parameter("storagePackageUuid", "java.lang.String", true, false, false, false, true, false, new String[0], 255, false, false)); + parameterMap.put("config", new Parameter("config", "java.lang.String", true, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("systemTags", new Parameter("systemTags", "java.util.List", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("userTags", new Parameter("userTags", "java.util.List", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("sessionId", new Parameter("sessionId", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("accessKeyId", new Parameter("accessKeyId", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("accessKeySecret", new Parameter("accessKeySecret", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("requestIp", new Parameter("requestIp", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + + nonAPIParameterMap.put("timeout", new Parameter("timeout", "long", false, false, false, false, false, false, new String[0], 0, false, false)); + nonAPIParameterMap.put("pollingInterval", new Parameter("pollingInterval", "long", false, false, false, false, false, false, new String[0], 0, false, false)); + } public static class Result {注意:请参考项目中其他 SDK Action 类(如
InstallZceXAction)的完整实现,确保 Parameter 构造函数参数的准确性。
7-104: 缺少 example 方法用于 API 文档生成根据编码规范第 1 条"API 设计要求",API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。当前类缺少此方法。建议在类中添加
__example__方法:public static InstallZStoneAction __example__() { InstallZStoneAction action = new InstallZStoneAction(); action.storagePackageUuid = uuid(); action.config = "example-config-content"; action.systemTags = Arrays.asList("system-tag-example"); action.userTags = Arrays.asList("user-tag-example"); return action; }sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (2)
28-29: 参数验证配置仍不一致:storagePackageUuid 应禁止空字符串该参数标记为
required = true但nonempty = false且emptyString = true,这意味着允许空字符串。对于必需的 UUID 参数,应当禁止空字符串以保证参数有效性。建议修改为:
- @Param(required = true, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false) + @Param(required = true, maxLength = 255, nonempty = true, nullElements = false, emptyString = false, noTrim = false) public java.lang.String storagePackageUuid;
7-104: 仍缺少 example 方法根据编码规范明确要求:"API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。" 该类仍然缺少此方法。建议参考其他 Action 类(如
InstallZStoneAction)添加__example__方法:public static InstallZceXAction __example__() { InstallZceXAction action = new InstallZceXAction(); action.storagePackageUuid = "示例UUID"; action.config = "示例配置"; action.sessionId = "示例会话ID"; return action; }As per coding guidelines.
🧹 Nitpick comments (1)
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (1)
45479-45503: 实现正确,但存在显著的代码重复。
installZStone方法的实现与installZceX几乎完全相同,仅在 Action 类型上有所不同。这种重复模式在文件中普遍存在(例如queryZceX和queryZStone),表明这是代码库的既定架构。虽然当前实现正确且与现有模式一致,但建议未来考虑引入通用方法来减少重复代码,例如:
private def executeAction(Class actionClass, Closure c) { def a = actionClass.newInstance() a.sessionId = Test.currentEnvSpec?.session?.uuid c.resolveStrategy = Closure.OWNER_FIRST c.delegate = a c() if (System.getProperty("apipath") != null) { if (a.apiId == null) { a.apiId = Platform.uuid } def tracker = new ApiPathTracker(a.apiId) def out = errorOut(a.call()) def path = tracker.getApiPath() if (!path.isEmpty()) { Test.apiPaths[a.class.name] = path.join(" --->\n") } return out } else { return errorOut(a.call()) } }这样可以将重复逻辑提取到单一位置,提高可维护性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (6)
conf/db/upgrade/V4.10.20__schema.sql(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java(1 hunks)testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java
- conf/db/upgrade/V4.10.20__schema.sql
- sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
- 单个 if 代码块不宜超过一屏显示,以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则:
- 对于可以通过预检查避免的 RuntimeException(如
NullPointerException、IndexOutOfBoundsException等),不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况,不应将异常逻辑当作正常流程控制。
- 在必要时,应继续抛出异常,使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源,确保在 finally 块中正确关闭资源,并避免在 finally 中返回值。
...
Files:
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java
🧬 Code graph analysis (2)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (2)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (2)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
🔇 Additional comments (5)
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (1)
45259-45283: 实现正确,遵循既定模式。
installZceX方法的实现逻辑正确:
- 正确创建了
InstallZceXAction实例- 使用安全导航操作符设置 sessionId,避免空指针
- Closure 委托和解析策略设置正确
- API 路径跟踪逻辑完整
该方法与文件中现有的 API helper 方法(如
queryZceX)保持一致。sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (4)
7-26: 类结构和 Result 内部类实现正确类正确继承
AbstractAction,Result内部类遵循标准模式,包含error和value字段,throwExceptionIfError()方法正确地抛出ApiException。实现与其他 Action 类(如InstallZStoneAction)保持一致。
72-84: 同步和异步调用方法实现正确同步
call()和异步call(Completion<Result>)方法正确实现,遵循 SDK 标准模式。异步调用正确使用InternalCompletion包装用户提供的完成回调。
86-102: REST 元数据配置正确REST 元数据正确配置:
- HTTP 方法使用
POST,符合创建/安装操作的规范要求- 路径使用
/zce-x-plugin/install- 正确设置
needSession = true和needPoll = trueparameterName已正确设置为"params"(先前的问题已解决)
66-67: 删除此审查注释 - 此模式是 SDK 中的标准设计经过验证,代码的空值处理逻辑
ret.value = value == null ? new ... : value;是整个 SDK 中的系统性设计模式,而非代码问题。该模式在 600+ 个 Action 类中一致应用,包括所有主要和专用操作(Create、Update、Query、Delete 等)。这是 SDK 的防御性编程策略,确保返回对象永不为空,防止调用方出现
NullPointerException。此模式是有意而为之,不需要修改或额外验证。
4e370b0 to
5129570
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (7)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java (1)
5-7: 仍缺少 Javadoc 文档根据编码规范第 3 节,所有公共类必须配备有效的 Javadoc 注释。此问题在之前的评审中已被标记,但尚未解决。
请添加 Javadoc 文档:
+/** + * ZStone 插件安装操作的结果类。 + */ public class InstallZStoneResult { }验证类设计是否符合 API 规范
根据编码规范第 1 节,API 返回类必须继承
APIReply或APIEvent,并使用@RestResponse注解。请确认:
- 此 SDK Result 类是否需要继承
APIReply或APIEvent?- 空类体是否为有意设计的占位符?如果是,请在 Javadoc 中说明
运行以下脚本验证项目中其他类似 Result 类的实现模式:
#!/bin/bash # 查找 SDK 中其他 Result 类的实现模式,验证是否有继承 APIReply/APIEvent 的规范 fd -e java "Result.java$" sdk/src/main/java/org/zstack/sdk/ --exec sh -c 'echo "=== {} ==="; head -15 {}'#!/bin/bash # 搜索 APIReply 和 APIEvent 的使用情况,确认 Result 类是否应该继承它们 rg -n --type java -A 3 "class.*Result.*extends.*(APIReply|APIEvent)" sdk/src/main/java/org/zstack/sdk/sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (3)
9-11: 静态参数映射未初始化静态字段
parameterMap和nonAPIParameterMap已声明但从未填充,这会导致getParameterMap()和getNonAPIParameterMap()返回空的 Map,使 ZSClient 无法正确构建 API 请求参数。所有使用@Param和@NonAPIParam注解的字段都需要在静态初始化块中注册。需要添加静态初始化块:
private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); + + static { + parameterMap.put("storagePackageUuid", new Parameter("storagePackageUuid", "java.lang.String", true, false, false, false, true, false, new String[0], 255, false, false)); + parameterMap.put("config", new Parameter("config", "java.lang.String", false, false, false, false, true, false, new String[0], 0, false, false)); + parameterMap.put("systemTags", new Parameter("systemTags", "java.util.List", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("userTags", new Parameter("userTags", "java.util.List", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("sessionId", new Parameter("sessionId", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("accessKeyId", new Parameter("accessKeyId", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("accessKeySecret", new Parameter("accessKeySecret", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("requestIp", new Parameter("requestIp", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + + nonAPIParameterMap.put("timeout", new Parameter("timeout", "long", false, false, false, false, false, false, new String[0], 0, false, false)); + nonAPIParameterMap.put("pollingInterval", new Parameter("pollingInterval", "long", false, false, false, false, false, false, new String[0], 0, false, false)); + } public static class Result {
28-29: 必填 UUID 参数应禁止空字符串
storagePackageUuid参数标记为required = true但允许空字符串(nonempty = false, emptyString = true),这对于必填的 UUID 参数来说不够严格。建议禁止空字符串以确保参数有效性。应用以下修改:
- @Param(required = true, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false) + @Param(required = true, maxLength = 255, nonempty = true, nullElements = false, emptyString = false, noTrim = false) public java.lang.String storagePackageUuid;
7-104: 缺少 example 方法用于 API 文档生成根据编码规范,API 类需要实现
__example__方法以便生成 API 文档和对应的 Groovy API Template 与 API Markdown 文件。当前类缺少此方法。建议添加:
public static InstallZceXAction __example__() { InstallZceXAction action = new InstallZceXAction(); action.storagePackageUuid = uuid(); action.config = "example-config-content"; action.systemTags = Arrays.asList("system-tag-example"); action.userTags = Arrays.asList("user-tag-example"); return action; }基于编码规范。
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (3)
9-11: 静态参数映射未初始化静态字段
parameterMap和nonAPIParameterMap已声明但从未填充,导致参数元数据不可用,影响参数验证和请求构建。需要在静态初始化块中注册所有@Param和@NonAPIParam注解的字段。添加静态初始化块:
private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); + + static { + parameterMap.put("storagePackageUuid", new Parameter("storagePackageUuid", "java.lang.String", true, false, false, false, true, false, new String[0], 255, false, false)); + parameterMap.put("config", new Parameter("config", "java.lang.String", true, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("systemTags", new Parameter("systemTags", "java.util.List", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("userTags", new Parameter("userTags", "java.util.List", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("sessionId", new Parameter("sessionId", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("accessKeyId", new Parameter("accessKeyId", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("accessKeySecret", new Parameter("accessKeySecret", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("requestIp", new Parameter("requestIp", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + + nonAPIParameterMap.put("timeout", new Parameter("timeout", "long", false, false, false, false, false, false, new String[0], 0, false, false)); + nonAPIParameterMap.put("pollingInterval", new Parameter("pollingInterval", "long", false, false, false, false, false, false, new String[0], 0, false, false)); + } public static class Result {
28-29: 必填 UUID 参数应禁止空字符串
storagePackageUuid参数标记为required = true但允许空字符串(nonempty = false, emptyString = true)。对于必填的 UUID 参数,应当禁止空字符串以保证参数有效性。建议修改为:
- @Param(required = true, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false) + @Param(required = true, maxLength = 255, nonempty = true, nullElements = false, emptyString = false, noTrim = false) public java.lang.String storagePackageUuid;
7-104: 缺少 example 方法用于 API 文档生成根据编码规范,API 类需要实现
__example__方法以便生成 API 文档和对应的 Groovy API Template 与 API Markdown 文件。当前类缺少此方法。建议添加:
public static InstallZStoneAction __example__() { InstallZStoneAction action = new InstallZStoneAction(); action.storagePackageUuid = uuid(); action.config = "example-config-content"; action.systemTags = Arrays.asList("system-tag-example"); action.userTags = Arrays.asList("user-tag-example"); return action; }基于编码规范。
🧹 Nitpick comments (1)
conf/db/upgrade/V4.10.20__schema.sql (1)
11-11: 建议为 createDate 添加默认值以保持一致性
createDate字段当前为可空且没有默认值,而lastOpDate字段使用了DEFAULT CURRENT_TIMESTAMP。为了保持一致性并避免在插入新记录时createDate为 NULL 的情况,建议为createDate也添加默认值。应用以下修改:
- `createDate` timestamp, + `createDate` timestamp DEFAULT CURRENT_TIMESTAMP,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (6)
conf/db/upgrade/V4.10.20__schema.sql(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java(1 hunks)testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
- 单个 if 代码块不宜超过一屏显示,以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则:
- 对于可以通过预检查避免的 RuntimeException(如
NullPointerException、IndexOutOfBoundsException等),不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况,不应将异常逻辑当作正常流程控制。
- 在必要时,应继续抛出异常,使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源,确保在 finally 块中正确关闭资源,并避免在 finally 中返回值。
...
Files:
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.javasdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java
**/*.sql
⚙️ CodeRabbit configuration file
**/*.sql: Review the SQL code, make sure has no errors and confirm that:
- Upgrading scene has been carefully handled
- Do not use
DEFAULT 0000-00-00 00:00:00, useDEFAULT CURRENT_TIMESTAMPinstead- When
NOT NULLexists, must usestored procedureor other functions to process historical data, this is very very important
Files:
conf/db/upgrade/V4.10.20__schema.sql
🧠 Learnings (1)
📚 Learning: 2025-09-01T08:16:10.006Z
Learnt from: MatheMatrix
PR: MatheMatrix/zstack#2541
File: testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy:30889-30893
Timestamp: 2025-09-01T08:16:10.006Z
Learning: ApiHelper.groovy in testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy is auto-generated and should not be manually modified or receive code change suggestions.
Applied to files:
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
🔇 Additional comments (2)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java (1)
5-7: 缺少 Javadoc 文档根据编码规范第 3 节,所有公共类必须配备有效的 Javadoc 注释。
请添加 Javadoc 文档:
+/** + * ZceX 插件安装操作的结果类。 + */ public class InstallZceXResult { }验证类设计是否符合 API 规范
根据编码规范第 1 节,API 返回类必须继承
APIReply或APIEvent,并使用@RestResponse注解。请确认:
- 此 SDK Result 类是否需要继承
APIReply或APIEvent?- 空类体是否为有意设计的占位符?如果是,请在 Javadoc 中说明
运行以下脚本验证项目中其他类似 Result 类的实现模式:
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (1)
45259-45283: 已确认:自动生成的代码,符合预期模式。根据代码仓库的学习记录,此文件为自动生成文件,不应手动修改。新增的
installZceX和installZStone方法遵循了标准的 API 辅助方法模式:
- 实例化对应的 Action 对象
- 设置 sessionId
- 配置 Closure 委托
- 执行调用并处理 apipath 跟踪
这两个方法的结构与现有方法一致,代码生成正确。
基于学习记录。
Also applies to: 45479-45503
5129570 to
f6ac281
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java (1)
5-7: 缺少公共类 Javadoc 注释根据编码规范第 3 节,所有公共类必须配备有效的 Javadoc 注释。该问题已在之前的评审中标记。
建议添加类注释:
+/** + * ZStone 插件安装操作的结果类 + */ public class InstallZStoneResult { }sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (2)
9-11: 缺少静态初始化块填充参数映射静态字段
parameterMap和nonAPIParameterMap已声明但未填充。该问题已在之前的评审中标记为严重问题。这会导致getParameterMap()和getNonAPIParameterMap()返回空的 Map,从而使参数验证和请求构建失败。需要添加静态初始化块来注册所有带
@Param注解的字段到parameterMap,以及所有带@NonAPIParam注解的字段到nonAPIParameterMap。请参考InstallZceXAction等其他 SDK Action 类的实现模式。
7-104: 缺少__example__方法用于 API 文档生成根据编码规范第 1 条"API 设计要求",API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。该问题已在之前的评审中标记。建议添加示例方法:
public static InstallZStoneAction __example__() { InstallZStoneAction action = new InstallZStoneAction(); action.storagePackageUuid = uuid(); action.config = "example-config-content"; action.systemTags = Arrays.asList("system-tag-example"); action.userTags = Arrays.asList("user-tag-example"); return action; }
🧹 Nitpick comments (2)
conf/db/upgrade/V4.10.20__schema.sql (1)
11-11: 建议为createDate添加默认值
createDate字段缺少默认值,建议添加DEFAULT CURRENT_TIMESTAMP以确保新记录的创建时间自动设置,与lastOpDate的处理方式保持一致。应用此修改:
- `createDate` timestamp, + `createDate` timestamp DEFAULT CURRENT_TIMESTAMP,sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java (1)
1-7: 移除多余的空行,并考虑添加类文档。当前代码存在以下可优化点:
- 多余空行: 第 2-4 行有三个连续的空行,Java 代码规范通常在 package 声明后只保留一个空行。
- 缺少文档: 类缺少 Javadoc 注释,建议添加类级别的文档说明该结果类的用途和意图。
- 空类结构: 当前类完全为空,如果这是有意的占位符设计(用于未来扩展或作为标记类型),建议在文档中明确说明;否则,请确认是否需要添加字段来承载 API 返回数据。
应用以下 diff 来改进代码格式和文档:
package org.zstack.sdk.zcex.api; - - + +/** + * InstallZceX API 的结果类。 + * 当前为空实现,用作 InstallZceXAction 的返回类型占位符。 + */ public class InstallZceXResult { - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (6)
conf/db/upgrade/V4.10.20__schema.sql(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java(1 hunks)testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
- 单个 if 代码块不宜超过一屏显示,以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则:
- 对于可以通过预检查避免的 RuntimeException(如
NullPointerException、IndexOutOfBoundsException等),不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况,不应将异常逻辑当作正常流程控制。
- 在必要时,应继续抛出异常,使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源,确保在 finally 块中正确关闭资源,并避免在 finally 中返回值。
...
Files:
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java
**/*.sql
⚙️ CodeRabbit configuration file
**/*.sql: Review the SQL code, make sure has no errors and confirm that:
- Upgrading scene has been carefully handled
- Do not use
DEFAULT 0000-00-00 00:00:00, useDEFAULT CURRENT_TIMESTAMPinstead- When
NOT NULLexists, must usestored procedureor other functions to process historical data, this is very very important
Files:
conf/db/upgrade/V4.10.20__schema.sql
🧠 Learnings (1)
📚 Learning: 2025-09-01T08:16:10.006Z
Learnt from: MatheMatrix
PR: MatheMatrix/zstack#2541
File: testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy:30889-30893
Timestamp: 2025-09-01T08:16:10.006Z
Learning: ApiHelper.groovy in testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy is auto-generated and should not be manually modified or receive code change suggestions.
Applied to files:
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
🧬 Code graph analysis (1)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (2)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
🔇 Additional comments (4)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (2)
13-26: Result 内部类缺少 Javadoc 注释根据编码规范第 3 节,公共类必须配备有效的 Javadoc 注释。请为
Result内部类添加注释。建议添加:
+ /** + * ZStone 插件安装操作的结果封装类 + */ public static class Result {Likely an incorrect or invalid review comment.
7-7: 缺少类级别 Javadoc 注释根据编码规范第 3 节,公共类必须配备有效的 Javadoc 注释。请添加类注释说明此 Action 的用途。
建议添加:
+/** + * ZStone 插件安装 API Action + * <p> + * 用于执行 ZStone 插件的安装操作 + * </p> + */ public class InstallZStoneAction extends AbstractAction {Likely an incorrect or invalid review comment.
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (2)
45479-45503: 验证完成:InstallZStone助手实现无问题,生成文件一致性确认验证结果:
- ✅ 方法正确位置(ApiHelper.groovy 第45479行)
- ✅ SDK类完整(InstallZStoneAction、InstallZStoneResult均存在)
- ✅ REST端点已定义(
/zstone-plugin/install)- ✅ 命名与模式一致(与其他API helper保持同步)
代码生成文件一致,建议保持原状不手动修改。
45259-45283: 代码变更验证通过,无需修改脚本验证确认:
- 方法
installZceX在 ApiHelper.groovy 中仅出现一次(行 45259),无重复定义- SDK 类
InstallZceXAction和InstallZceXResult均存在- REST 元数据正确关联:
info.path = "/zce-x-plugin/install"- 所有组件(会话、委托、apipath 追踪、errorOut)均与既有模式对齐
作为自动生成文件,本次变更无需手动调整。
f6ac281 to
5bfce8d
Compare
Resolves: ZSV-9928 Change-Id: I6a776c74767a73766f79776f7861667564706178
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java (1)
5-7: 仍缺少 Javadoc 类注释根据编码规范第 3 节要求,所有公共类必须配备有效的 Javadoc 注释。此问题在之前的评审中已提出但尚未解决。
建议添加类文档注释:
+/** + * ZStone 插件安装操作的结果。 + */ public class InstallZStoneResult { }sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (2)
28-29: 参数验证配置不一致:UUID 参数应禁止空字符串
storagePackageUuid参数标记为required = true,但同时设置了nonempty = false和emptyString = true,这意味着允许空字符串。对于必需的 UUID 参数,应当禁止空字符串以确保参数有效性。此问题在之前的评审中已提出但尚未解决。建议修改为:
- @Param(required = true, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false) + @Param(required = true, maxLength = 255, nonempty = true, nullElements = false, emptyString = false, noTrim = false) public java.lang.String storagePackageUuid;
7-104: 缺少必需的 example 方法根据编码规范第 1 节要求:"API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。" 当前类缺少该方法。此问题在之前的评审中已提出但尚未解决。建议参考其他 Action 类的实现模式添加
__example__方法:public static InstallZceXAction __example__() { InstallZceXAction action = new InstallZceXAction(); action.storagePackageUuid = uuid(); action.config = "example config"; action.sessionId = uuid(); return action; }As per coding guidelines.
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (2)
9-11: 【严重】缺少静态初始化块填充参数映射静态字段
parameterMap和nonAPIParameterMap已声明但从未填充。这会导致getParameterMap()和getNonAPIParameterMap()返回空的 Map,从而使 ZSClient 在构建 API 请求时无法获取参数元数据,导致 API 调用失败。所有使用
@Param注解的字段(storagePackageUuid、config、systemTags、userTags、sessionId、accessKeyId、accessKeySecret、requestIp)以及使用@NonAPIParam注解的字段(timeout、pollingInterval)都需要在静态初始化块中注册。建议添加静态初始化块:
private static final HashMap<String, Parameter> nonAPIParameterMap = new HashMap<>(); + + static { + parameterMap.put("storagePackageUuid", new Parameter("storagePackageUuid", "java.lang.String", true, false, false, false, true, false, new String[0], 255, false, false)); + parameterMap.put("config", new Parameter("config", "java.lang.String", true, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("systemTags", new Parameter("systemTags", "java.util.List", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("userTags", new Parameter("userTags", "java.util.List", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("sessionId", new Parameter("sessionId", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("accessKeyId", new Parameter("accessKeyId", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("accessKeySecret", new Parameter("accessKeySecret", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + parameterMap.put("requestIp", new Parameter("requestIp", "java.lang.String", false, false, false, false, false, false, new String[0], 0, false, false)); + + nonAPIParameterMap.put("timeout", new Parameter("timeout", "long", false, false, false, false, false, false, new String[0], 0, false, false)); + nonAPIParameterMap.put("pollingInterval", new Parameter("pollingInterval", "long", false, false, false, false, false, false, new String[0], 0, false, false)); + } public static class Result {注意:请参考项目中其他 SDK Action 类(如
InstallZceXAction)的完整实现来确保 Parameter 构造函数参数的准确性。
7-104: 【重要】缺少 example 方法用于 API 文档生成根据编码规范第 1 条"API 设计要求",API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。当前类缺少此方法。建议在类中添加
__example__方法:public static InstallZStoneAction __example__() { InstallZStoneAction action = new InstallZStoneAction(); action.storagePackageUuid = uuid(); action.config = "example-config-content"; action.systemTags = Arrays.asList("system-tag-example"); action.userTags = Arrays.asList("user-tag-example"); return action; }As per coding guidelines
🧹 Nitpick comments (2)
testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy (2)
45259-45283: 考虑提取重复的逻辑以减少代码重复。该方法与下方的
installZStone方法几乎完全相同(仅在 Action 类型上有所不同)。这两个方法共享约 25 行相同的模式代码,包括会话设置、闭包配置、API 路径跟踪等逻辑。建议提取一个通用的辅助方法来处理这些共同的模式,以提高代码的可维护性。
可以考虑类似以下的重构方式:
private def executeApiAction(action, Closure c) { action.sessionId = Test.currentEnvSpec?.session?.uuid c.resolveStrategy = Closure.OWNER_FIRST c.delegate = action c() if (System.getProperty("apipath") != null) { if (action.apiId == null) { action.apiId = Platform.uuid } def tracker = new ApiPathTracker(action.apiId) def out = errorOut(action.call()) def path = tracker.getApiPath() if (!path.isEmpty()) { Test.apiPaths[action.class.name] = path.join(" --->\n") } return out } else { return errorOut(action.call()) } } def installZceX(@DelegatesTo(strategy = Closure.OWNER_FIRST, value = org.zstack.sdk.zcex.api.InstallZceXAction.class) Closure c) { def a = new org.zstack.sdk.zcex.api.InstallZceXAction() return executeApiAction(a, c) }
45479-45503: 与installZceX方法存在相同的代码重复问题。该方法与上方的
installZceX方法(第 45259-45283 行)几乎完全相同,仅在 Action 类型上有所不同。请参考对installZceX的评审建议,考虑统一重构这两个方法以减少重复代码。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java(1 hunks)sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.java(1 hunks)testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXResult.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
- 单个 if 代码块不宜超过一屏显示,以提高可读性和后续维护性。
5. 异常处理与日志
- 捕获异常的原则:
- 对于可以通过预检查避免的 RuntimeException(如
NullPointerException、IndexOutOfBoundsException等),不建议使用 try-catch 来进行处理。- 捕获异常应仅用于处理真正的意外情况,不应将异常逻辑当作正常流程控制。
- 在必要时,应继续抛出异常,使上层业务处理者可以转换为用户友好的错误提示。
- 使用 try-with-resources 语法管理资源,确保在 finally 块中正确关闭资源,并避免在 finally 中返回值。
...
Files:
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneResult.javasdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.javasdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java
🧬 Code graph analysis (2)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (2)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
sdk/src/main/java/org/zstack/sdk/zstone/api/InstallZStoneAction.java (2)
sdk/src/main/java/org/zstack/sdk/zcex/api/InstallZceXAction.java (1)
Result(13-26)sdk/src/main/java/org/zstack/sdk/ZSClient.java (1)
ZSClient(33-788)
5bfce8d to
1aaa004
Compare
Resolves: ZSV-9928
Change-Id: I6a776c74767a73766f79776f7861667564706178
sync from gitlab !8534