<fix>[volume]: <description #3123
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
总体说明引入了两个新的消息类（UploadFileToBackupStorageHostMsg 和 GetFileDownloadProgressMsg）及其对应的回复类，用于支持文件上传至备份存储和下载进度查询。在 Ceph 备份存储中实现了完整的处理逻辑，包括 HTTP 调用和 URL 解析；在 SFTP、模拟器和外部备份存储中添加了"不支持"的异常处理。 Changes
序列图sequenceDiagram
participant Client
participant CephBS as Ceph<br/>BackupStorage
participant Agent as Backup<br/>Storage Agent
rect rgb(240, 248, 255)
Note over Client,Agent: 文件上传流程
Client->>CephBS: UploadFileToBackupStorageHostMsg<br/>(upload:// URL)
CephBS->>CephBS: 解析 upload:// URL
CephBS->>CephBS: 创建 UploadFileCmd
CephBS->>Agent: HTTP 调用 FILE_UPLOAD_PATH
Agent->>Agent: 上传文件
Agent-->>CephBS: UploadFileResponse<br/>(md5sum, size)
CephBS-->>Client: UploadFileToBackupStorageHostReply
end
rect rgb(240, 255, 240)
Note over Client,Agent: 下载进度查询流程
Client->>CephBS: GetFileDownloadProgressMsg
CephBS->>CephBS: 创建 DownloadFileCmd
CephBS->>Agent: HTTP 调用 FILE_DOWNLOAD_PATH
Agent->>Agent: 获取下载进度
Agent-->>CephBS: DownloadFileResponse<br/>(progress, size)
CephBS-->>Client: GetFileDownloadProgressReply
end
评估代码审查工作量🎯 3 (中等) | ⏱️ ~20 分钟 诗歌
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin/sftpBackupStorage/src/main/java/org/zstack/storage/backup/sftp/SftpBackupStorage.java (1)
692-696: 潜在缺陷：reply 未发送
handle(CalculateImageHashOnBackupStorageMsg)方法创建了reply并设置了错误，但未通过bus.reply(msg, reply)发送。这会导致调用方永远收不到响应，可能造成超时。🔎 建议修复
@Override protected void handle(CalculateImageHashOnBackupStorageMsg msg) { CalculateImageHashOnBackupStorageReply reply = new CalculateImageHashOnBackupStorageReply(); reply.setError(operr("sftp backup storage do not support calculate image hash")); + bus.reply(msg, reply); }
🧹 Nitpick comments (3)
header/src/main/java/org/zstack/header/volume/APICreateDataVolumeMsg.java (1)
3-3: 多余的空行此处新增的空行看起来是无意义的格式变更。如果这不是代码格式化工具的要求，建议移除以保持代码整洁。
simulator/simulatorImpl/src/main/java/org/zstack/simulator/storage/backup/SimulatorBackupStorage.java (1)
141-150: 实现可行，但与部分现有处理器风格不一致新增的处理方法使用
bus.replyErrorByMessageType()返回错误，这是更安全的方式。但请注意，该类中部分现有处理器（如handle(SyncImageSizeOnBackupStorageMsg)在 Lines 100-102）使用的是throw new CloudRuntimeException("not supported yet")。建议保持一致性，或者在后续统一将抛异常的方式改为消息回复方式。
plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java (1)
379-423: 新增命令类使用 public 字段，与现有类风格不一致。当前文件中其他命令类（如
GetDownloadProgressCmd、DownloadCmd等）使用 private 字段配合 getter/setter 方法，而新增的DownloadFileCmd、UploadFileCmd、GetDownloadFileProgressCmd及其响应类直接使用 public 字段。建议保持代码风格一致性。🔎 建议修改示例（以 DownloadFileCmd 为例）
public static class DownloadFileCmd extends AgentCommand implements HasThreadContext, Serializable { - public String taskUuid; - public String installPath; + private String taskUuid; + private String installPath; @NoLogging(type = NoLogging.Type.Uri) - public String url; + private String url; @NoLogging(type = NoLogging.Type.Uri) - public String urlScheme; - public long timeout; + private String urlScheme; + private long timeout; @NoLogging(type = NoLogging.Type.Uri) - public String sendCommandUrl; + private String sendCommandUrl; + + // Add getters and setters... }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pom.xmlis excluded by!**/*.xml
📒 Files selected for processing (10)
header/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressMsg.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressReply.javaheader/src/main/java/org/zstack/header/volume/APICreateDataVolumeMsg.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.javaplugin/sftpBackupStorage/src/main/java/org/zstack/storage/backup/sftp/SftpBackupStorage.javasimulator/simulatorImpl/src/main/java/org/zstack/simulator/storage/backup/SimulatorBackupStorage.javastorage/src/main/java/org/zstack/storage/addon/backup/ExternalBackupStorage.javastorage/src/main/java/org/zstack/storage/backup/BackupStorageBase.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:
header/src/main/java/org/zstack/header/volume/APICreateDataVolumeMsg.javastorage/src/main/java/org/zstack/storage/addon/backup/ExternalBackupStorage.javaheader/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostReply.javasimulator/simulatorImpl/src/main/java/org/zstack/simulator/storage/backup/SimulatorBackupStorage.javaplugin/sftpBackupStorage/src/main/java/org/zstack/storage/backup/sftp/SftpBackupStorage.javaheader/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostMsg.javastorage/src/main/java/org/zstack/storage/backup/BackupStorageBase.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressMsg.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressReply.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java
🧬 Code graph analysis (3)
storage/src/main/java/org/zstack/storage/addon/backup/ExternalBackupStorage.java (1)
header/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostMsg.java (1)
UploadFileToBackupStorageHostMsg(7-46)
simulator/simulatorImpl/src/main/java/org/zstack/simulator/storage/backup/SimulatorBackupStorage.java (1)
header/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostMsg.java (1)
UploadFileToBackupStorageHostMsg(7-46)
storage/src/main/java/org/zstack/storage/backup/BackupStorageBase.java (1)
header/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostMsg.java (1)
UploadFileToBackupStorageHostMsg(7-46)
🔇 Additional comments (9)
storage/src/main/java/org/zstack/storage/addon/backup/ExternalBackupStorage.java (1)
166-175: 实现符合既有模式新增的两个处理方法与该类中其他
handle方法的实现模式一致，均返回 "not supported" 错误。这符合 ExternalBackupStorage 作为外部备份存储桥接层的定位。plugin/sftpBackupStorage/src/main/java/org/zstack/storage/backup/sftp/SftpBackupStorage.java (1)
708-717: 新增处理方法实现正确新增的两个处理方法正确实现了抽象基类要求，返回 "not supported" 错误。
header/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostReply.java (1)
1-35: 新增回复类结构良好
UploadFileToBackupStorageHostReply类设计合理：
- 正确继承
MessageReply@NoLogging(type = NoLogging.Type.Uri)注解正确应用于directUploadUrl，防止敏感 URL 被记录到日志- 字段命名清晰，getter/setter 符合规范
header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressMsg.java (1)
1-34: 消息类结构正确
GetFileDownloadProgressMsg类设计符合框架规范：
- 正确继承
NeedReplyMessage并实现BackupStorageMessage接口getBackupStorageUuid()方法正确覆盖接口要求- 字段设计合理，用于查询下载进度
header/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostMsg.java (1)
1-46: 消息类设计良好
UploadFileToBackupStorageHostMsg类设计符合框架规范：
- 正确继承
NeedReplyMessage并实现BackupStorageMessage接口@NoLogging(type = NoLogging.Type.Uri)注解正确应用于url字段，防止敏感信息泄露- 字段设计完整，包含上传所需的任务标识、URL 和安装路径
plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java (2)
100-101: LGTM!新增的
ApiTimeoutManager注入和端点常量遵循现有代码规范。Also applies to: 737-739
2083-2146: LGTM!该方法正确处理了
upload://协议和其他 URL 的不同逻辑，URI 解析错误处理完善，使用了 Early Return 模式提高可读性。header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressReply.java (2)
49-51:isDownloadComplete()方法逻辑可能存在语义问题。当前实现
actualSize > 0 && actualSize == downloadSize，但字段命名可能引起混淆：
actualSize通常表示文件的实际大小downloadSize通常表示已下载的大小如果语义如此，判断下载完成的条件应该是
downloadSize == size或downloadSize >= actualSize。请确认字段语义是否正确，或考虑添加注释说明字段含义。
17-91: LGTM!Getter/setter 方法遵循 Java Bean 规范，实现正确。
Comment on lines
+5
to
+15
| public class GetFileDownloadProgressReply extends MessageReply { | ||
| private boolean completed; | ||
| private int progress; | ||
|
|
||
| private long size; | ||
| private long actualSize; | ||
| private long downloadSize; | ||
| private String installPath; | ||
| private String format; | ||
| private long lastOpTime; | ||
| private boolean supportSuspend; |
There was a problem hiding this comment.
缺少 md5sum 字段。
在 CephBackupStorageBase.java 第 2171 行调用了 reply.setMd5sum(rsp.md5sum)，但此类未定义 md5sum 字段，会导致编译错误。
🔎 建议添加 md5sum 字段
public class GetFileDownloadProgressReply extends MessageReply {
private boolean completed;
private int progress;
private long size;
private long actualSize;
private long downloadSize;
private String installPath;
private String format;
private long lastOpTime;
private boolean supportSuspend;
+ private String md5sum;
+
+ public String getMd5sum() {
+ return md5sum;
+ }
+
+ public void setMd5sum(String md5sum) {
+ this.md5sum = md5sum;
+ }🤖 Prompt for AI Agents
In
header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressReply.java
around lines 5 to 15, the class is missing the md5sum field referenced by
CephBackupStorageBase at line 2171; add a private String md5sum field and
provide its public getter and setter (getMd5sum and setMd5sum) so the
reply.setMd5sum(...) call compiles correctly and follows existing Java bean
style used in this class.
Comment on lines
+2148
to
+2176
| @Override | ||
| protected void handle(GetFileDownloadProgressMsg msg) { | ||
| GetFileDownloadProgressReply reply = new GetFileDownloadProgressReply(); | ||
|
|
||
| GetDownloadFileProgressCmd cmd = new GetDownloadFileProgressCmd(); | ||
| cmd.taskUuid = msg.getTaskUuid(); | ||
|
|
||
| httpCall(FILE_UPLOAD_PROGRESS_PATH, cmd, GetDownloadFileProgressResponse.class, new ReturnValueCompletion<GetDownloadFileProgressResponse>(msg) { | ||
| @Override | ||
| public void fail(ErrorCode err) { | ||
| reply.setError(err); | ||
| bus.reply(msg, reply); | ||
| } | ||
|
|
||
| @Override | ||
| public void success(GetDownloadFileProgressResponse rsp) { | ||
| reply.setCompleted(rsp.completed); | ||
| reply.setProgress(rsp.progress); | ||
| reply.setActualSize(rsp.actualSize); | ||
| reply.setSize(rsp.size); | ||
| reply.setInstallPath(rsp.installPath); | ||
| reply.setDownloadSize(rsp.downloadSize); | ||
| reply.setLastOpTime(rsp.lastOpTime); | ||
| reply.setMd5sum(rsp.md5sum); | ||
| reply.setSupportSuspend(rsp.supportSuspend); | ||
| bus.reply(msg, reply); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
编译错误：GetFileDownloadProgressReply 缺少 md5sum 字段。
第 2171 行调用 reply.setMd5sum(rsp.md5sum)，但 GetFileDownloadProgressReply 类中未定义 md5sum 字段和对应的 setter 方法，这将导致编译失败。
需要在 GetFileDownloadProgressReply 类中添加 md5sum 字段。
🤖 Prompt for AI Agents
In
plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java
around lines 2148 to 2176, the code calls reply.setMd5sum(rsp.md5sum) but
GetFileDownloadProgressReply lacks an md5sum field and setter; modify the
GetFileDownloadProgressReply class to add a private String md5sum field plus
public void setMd5sum(String md5sum) and public String getMd5sum() (or matching
types used by rsp.md5sum), then recompile to ensure the reference is resolved.
Comment on lines
+125
to
+127
| abstract protected void handle(UploadFileToBackupStorageHostMsg msg); | ||
|
|
||
| abstract protected void handle(GetFileDownloadProgressMsg msg); |
There was a problem hiding this comment.
缺少消息路由处理
新增了抽象方法 handle(UploadFileToBackupStorageHostMsg msg) 和 handle(GetFileDownloadProgressMsg msg)，但在 handleLocalMessage 方法（Lines 247-289）中未添加对应的消息路由分支。
这将导致这两种消息无法被正确分发到 handle 方法，而是进入 bus.dealWithUnknownMessage(msg) 分支。
🔎 建议在 handleLocalMessage 中添加路由
} else if (msg instanceof GetBackupStorageManagerHostnameMsg) {
handle((GetBackupStorageManagerHostnameMsg) msg);
+ } else if (msg instanceof UploadFileToBackupStorageHostMsg) {
+ handle((UploadFileToBackupStorageHostMsg) msg);
+ } else if (msg instanceof GetFileDownloadProgressMsg) {
+ handle((GetFileDownloadProgressMsg) msg);
} else {
bus.dealWithUnknownMessage(msg);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In storage/src/main/java/org/zstack/storage/backup/BackupStorageBase.java around
lines 125-127 (new abstract methods) and handleLocalMessage around lines
247-289, the two new abstract handlers UploadFileToBackupStorageHostMsg and
GetFileDownloadProgressMsg are not routed and will hit
bus.dealWithUnknownMessage(msg). Add branches in handleLocalMessage that check
for those message types (instanceof UploadFileToBackupStorageHostMsg and
instanceof GetFileDownloadProgressMsg), invoke the corresponding abstract
handle(...) methods, and return after handling so they do not fall through to
the unknown-message branch; also ensure the message classes are imported and any
reply or exception handling follows the existing pattern used for other message
branches.
MatheMatrix
force-pushed
the
sync/tao.gan/anyun-ZSV-1@@4
branch
from
fe16e65 to
ba36a3e
Compare
APIImpact Resolves: ZSV-1 Change-Id: I656979726c736267776c7262716969726874716d
MatheMatrix
force-pushed
the
sync/tao.gan/anyun-ZSV-1@@4
branch
from
ba36a3e to
a36dcfb
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java (1)
2148-2176: 编译错误：GetFileDownloadProgressReply缺少md5sum字段。Line 2171 调用
reply.setMd5sum(rsp.md5sum)，但GetFileDownloadProgressReply类（位于header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressReply.java）中未定义md5sum字段和对应的 setter 方法，这将导致编译失败。需要在
GetFileDownloadProgressReply类中添加md5sum字段及其 getter/setter 方法。🔎 修复建议
在
header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressReply.java文件中添加以下代码：private String md5sum; public String getMd5sum() { return md5sum; } public void setMd5sum(String md5sum) { this.md5sum = md5sum; }基于往期评审意见。
header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressReply.java (1)
5-15: 编译错误：缺少md5sum字段。在
CephBackupStorageBase.javaLine 2171 调用了reply.setMd5sum(rsp.md5sum)，但此类未定义md5sum字段及其 getter/setter 方法，会导致编译错误。该字段对于文件下载进度查询是必需的，用于返回文件的 MD5 校验和。🔎 建议添加 md5sum 字段
public class GetFileDownloadProgressReply extends MessageReply { private boolean completed; private int progress; private long size; private long actualSize; private long downloadSize; private String installPath; private String format; private long lastOpTime; private boolean supportSuspend; + private String md5sum; + + public String getMd5sum() { + return md5sum; + } + + public void setMd5sum(String md5sum) { + this.md5sum = md5sum; + }基于往期评审意见。
🧹 Nitpick comments (4)
plugin/sftpBackupStorage/src/main/java/org/zstack/storage/backup/sftp/SftpBackupStorage.java (1)
709-717: 建议使用更具描述性的错误消息这两个方法都返回通用的 "not supported" 错误消息。根据编码规范中关于编写自解释代码的要求，建议提供更详细的错误信息，帮助调用方理解具体是什么操作不被支持以及原因。
🔎 建议的改进
@Override protected void handle(UploadFileToBackupStorageHostMsg msg) { - bus.replyErrorByMessageType(msg, "not supported"); + bus.replyErrorByMessageType(msg, "SFTP backup storage does not support direct file upload to host"); } @Override protected void handle(GetFileDownloadProgressMsg msg) { - bus.replyErrorByMessageType(msg, "not supported"); + bus.replyErrorByMessageType(msg, "SFTP backup storage does not support querying file download progress"); }plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java (2)
43-44: 建议移除未使用的导入。Lines 43-44 导入了
KVMAgentCommands和KVMHost，但在本文件的可见代码中未找到使用这些类的地方。建议移除未使用的导入以保持代码整洁。
379-423: 建议考虑使用私有字段和访问器方法。新增的命令和响应类（
DownloadFileCmd、DownloadFileResponse等）使用了 public 字段而不是私有字段配合 getter/setter 方法。虽然这种模式与本文件中现有的命令类（如 Lines 300-338 的DownloadCmd）保持一致，但不符合标准的 Java Bean 规范和编码指南中关于封装的最佳实践。如果这是该代码库中序列化命令对象的既定模式，可以保持现状；否则建议后续重构时统一改为使用私有字段和访问器方法，以提高封装性和可维护性。
header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressReply.java (1)
49-51: 考虑澄清isDownloadComplete()与completed字段的关系。该类同时提供了
completed字段（Line 6）和isDownloadComplete()方法。isDownloadComplete()的逻辑是actualSize > 0 && actualSize == downloadSize，这与completed字段可能存在语义上的重叠。建议：
- 如果
completed表示下载任务结束（成功或失败），而isDownloadComplete()表示下载成功且大小匹配，请在方法上添加 Javadoc 注释说明区别- 如果两者表示相同含义，考虑移除重复的逻辑以避免混淆
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
header/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressMsg.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressReply.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.javaplugin/sftpBackupStorage/src/main/java/org/zstack/storage/backup/sftp/SftpBackupStorage.javasimulator/simulatorImpl/src/main/java/org/zstack/simulator/storage/backup/SimulatorBackupStorage.javastorage/src/main/java/org/zstack/storage/addon/backup/ExternalBackupStorage.javastorage/src/main/java/org/zstack/storage/backup/BackupStorageBase.java
🚧 Files skipped from review as they are similar to previous changes (4)
- storage/src/main/java/org/zstack/storage/backup/BackupStorageBase.java
- header/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostMsg.java
- header/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostReply.java
- header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressMsg.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:
simulator/simulatorImpl/src/main/java/org/zstack/simulator/storage/backup/SimulatorBackupStorage.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.javaplugin/sftpBackupStorage/src/main/java/org/zstack/storage/backup/sftp/SftpBackupStorage.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressReply.javastorage/src/main/java/org/zstack/storage/addon/backup/ExternalBackupStorage.java
🧬 Code graph analysis (2)
simulator/simulatorImpl/src/main/java/org/zstack/simulator/storage/backup/SimulatorBackupStorage.java (1)
header/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostMsg.java (1)
UploadFileToBackupStorageHostMsg(7-46)
storage/src/main/java/org/zstack/storage/addon/backup/ExternalBackupStorage.java (1)
header/src/main/java/org/zstack/header/image/UploadFileToBackupStorageHostMsg.java (1)
UploadFileToBackupStorageHostMsg(7-46)
🔇 Additional comments (3)
simulator/simulatorImpl/src/main/java/org/zstack/simulator/storage/backup/SimulatorBackupStorage.java (1)
9-9: LGTM!模拟器后端正确地为新消息类型添加了处理器，返回"不支持"错误。这与模拟器中其他处理器的模式一致（例如 Lines 100-107 的
SyncImageSizeOnBackupStorageMsg和GetLocalFileSizeOnBackupStorageMsg）。导入和方法命名符合编码规范。Also applies to: 142-150
storage/src/main/java/org/zstack/storage/addon/backup/ExternalBackupStorage.java (1)
10-10: LGTM!外部备份存储正确地为新消息类型添加了处理器，使用
bus.replyErrorByMessageType返回"不支持"错误。这与该类中其他未支持操作的处理模式一致（例如 Lines 92, 129, 134 等）。实现符合编码规范。Also applies to: 167-175
plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java (1)
2083-2146: LGTM!
handle(UploadFileToBackupStorageHostMsg)方法的实现逻辑清晰：
- 正确区分
upload://和普通下载 URL 的处理路径- 适当地集成了超时管理（Lines 2091, 2112）
- URI 解析异常处理完善（Lines 2117-2124）
- URL scheme 验证合理（Lines 2125-2129）
- 错误处理规范，所有错误情况都正确设置到 reply 并回复
实现符合编码规范要求。
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
APIImpact
Resolves: ZSV-1
Change-Id: I656979726c736267776c7262716969726874716d
sync from gitlab !8937