<feature>[host]: <description #2668
Closed
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
|
Warning Rate limit exceeded@MatheMatrix has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 27 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 (15)
Walkthrough为主机增加文件上传/下载与进度查询：新增 REST 消息与事件、主机侧消息/回复模型、KVM agent 命令与常量、HostManagerImpl 对 APIUploadFileMsg 的处理，以及 UploadFileTracker 周期性跟踪并上报进度（超时/重试控制）。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as 客户端
participant API as API/REST
participant HM as HostManagerImpl
participant Bus as CloudBus
participant KH as KVMHost
participant AG as KVM Agent
participant TR as UploadFileTracker
Client->>API: POST /host/{uuid}/upload-file (APIUploadFileMsg)
API->>HM: 派发 APIUploadFileMsg
HM->>Bus: 发送 UploadFileMsg(hostUuid, url, installPath)
Bus->>KH: Deliver UploadFileMsg
KH->>AG: DownloadFileCmd(url, installPath, taskUuid)
AG-->>KH: DownloadFileResponse(taskUuid)
KH->>TR: addTrackTask(taskUuid, hostname, url) / runTrackTask
loop 周期性轮询（内部定时）
TR->>Bus: GetFileDownloadProgressMsg(taskUuid, hostname, url)
Bus->>KH: 转发 GetFileDownloadProgressMsg
KH->>AG: GetDownloadFileProgressCmd(url, installPath)
AG-->>KH: GetDownloadFileProgressResponse(progress, completed, sizes...)
KH-->>Bus: GetFileDownloadProgressReply(...)
Bus-->>TR: Reply
TR-->>TR: 上报进度或结束（完成/失败）
end
KH-->>Bus: 下载完成或失败事件
Bus-->>HM: UploadFileMsg 回调（成功/错误）
HM-->>API: 发布 APIUploadFileEvent
API-->>Client: 返回结果
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Comment |
MatheMatrix
force-pushed
the
sync/tao.gan/ZSV-9928@@2
branch
2 times, most recently
from
1274f52 to
96b5f98
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
compute/src/main/java/org/zstack/compute/host/HostManagerImpl.java (1)
112-133: 遗漏 APIUploadFileMsg 的分发分支，导致自定义处理方法永远不会被调用当前 handleApiMessage 在判断
msg instanceof HostMessage之前未添加APIUploadFileMsg的分支，若APIUploadFileMsg实现了 HostMessage，会被直接 passThrough 到 Host，绕过新加的handle(APIUploadFileMsg)。应显式增加分支。[建议改动]
} else if (msg instanceof APIGetHostSensorsMsg){ handle((APIGetHostSensorsMsg) msg); + } else if (msg instanceof APIUploadFileMsg){ + handle((APIUploadFileMsg) msg); } else if (msg instanceof HostMessage) { HostMessage hmsg = (HostMessage) msg; passThrough(hmsg);
🧹 Nitpick comments (8)
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java (1)
4400-4424: 下载进度查询缺少幂等关联字段，部分字段语义需明确
- 仅靠 url+installPath 标识任务在并发/重复下载场景下可能冲突，建议在进度查询命令中携带 taskUuid 与 DownloadFileResponse 对齐。
- 建议明确 progress 的取值范围（0–100？）与 lastOpTime 的时间单位（毫秒/秒）并在返回注释或字段命名中体现，避免接口使用歧义。
[建议改动]
public static class GetDownloadFileProgressCmd extends AgentCommand implements HasThreadContext { - public String url; - public String installPath; + public String url; + public String installPath; + // 建议用于唯一关联一次下载任务，避免 url+installPath 冲突 + public String taskUuid; }请同时确认 KVM agent 侧实现对 progress（百分比/比率）、size/actualSize/downloadSize 的定义与单位一致。可附带接口注释或常量枚举说明。
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
17-17: 避免使用通配符导入，保持依赖显式可控。建议还原为按需导入，降低未来包内类重名或IDE自动优化带来的抖动。
header/src/main/java/org/zstack/header/host/GetFileDownloadProgressMsg.java (1)
5-43: 消息体字段完备，但缺少“唯一定位条件”的约束说明。
- 建议在类/字段 Javadoc 中明确：taskUuid 与 url 至少提供其一，用于唯一定位下载任务；并说明 hostname 的语义（管理IP/数据面IP）。
- 若可行，可在上层做参数校验，避免 host 端收到空标识。
是否有统一的 Validator 对消息进行非空/互斥校验？如无，我可补充示例实现。
header/src/main/java/org/zstack/header/host/GetFileDownloadProgressReply.java (1)
5-92: completed 与 isDownloadComplete 语义可能重复，需保持一致性来源。
- 当前同时暴露 completed 标志与基于 actualSize/downloadSize 的派生方法，来源不一致易引入歧义。
- 建议：在注释中声明 completed 的权威来源（来自 agent 还是由管理面计算）；或仅保留一种判定方式，另一种作为辅助字段供展示。
header/src/main/java/org/zstack/header/host/APIUploadFileMsg.java (1)
3-8: 修正 example：补充 url，移除未定义的 uuid() 使用当前示例调用 uuid() 但无静态导入；同时未设置 url。建议使用标准库生成 UUID，并提供示例 URL。
import org.springframework.http.HttpMethod; import org.zstack.header.log.NoLogging; import org.zstack.header.message.APIParam; -import org.zstack.header.message.APISyncCallMessage; +import org.zstack.header.message.APIMessage; import org.zstack.header.rest.RestRequest; +import java.util.UUID; @@ public static APIUploadFileMsg __example__() { APIUploadFileMsg msg = new APIUploadFileMsg(); - msg.setUuid(uuid()); + msg.setHostUuid(UUID.randomUUID().toString()); + msg.setUrl("http://127.0.0.1:8080/sample.img"); return msg; }Also applies to: 42-46
compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java (3)
60-61: ctxs 可能并发访问：建议使用 ConcurrentHashMap定时任务与调用方可能并行读写。
- Map<String, TrackContext> ctxs = new HashMap<>(); + Map<String, TrackContext> ctxs = new java.util.concurrent.ConcurrentHashMap<>();
40-50: 移除未使用依赖或补充用途说明DatabaseFacade、PluginRegistry、EventFacade 当前未被使用。建议移除以降低依赖面，或补充使用场景。
70-131: 可选：在完成/失败后取消周期任务并释放 ctx如需提前停止任务线程，可持有返回的句柄进行取消，或在 markCompletion/markFailure 中触发任务停止标记。
是否需要我基于 ThreadFacade 的返回类型补上取消逻辑示例？
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
compute/src/main/java/org/zstack/compute/host/HostManagerImpl.java(1 hunks)compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java(1 hunks)header/src/main/java/org/zstack/header/host/APIUploadFileEvent.java(1 hunks)header/src/main/java/org/zstack/header/host/APIUploadFileMsg.java(1 hunks)header/src/main/java/org/zstack/header/host/GetFileDownloadProgressMsg.java(1 hunks)header/src/main/java/org/zstack/header/host/GetFileDownloadProgressReply.java(1 hunks)header/src/main/java/org/zstack/header/host/UploadFileMsg.java(1 hunks)plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java(1 hunks)plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java(1 hunks)plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java(4 hunks)
🧰 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:
plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.javacompute/src/main/java/org/zstack/compute/host/HostManagerImpl.javaheader/src/main/java/org/zstack/header/host/APIUploadFileEvent.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaheader/src/main/java/org/zstack/header/host/GetFileDownloadProgressMsg.javaheader/src/main/java/org/zstack/header/host/GetFileDownloadProgressReply.javacompute/src/main/java/org/zstack/compute/host/UploadFileTracker.javaheader/src/main/java/org/zstack/header/host/APIUploadFileMsg.javaheader/src/main/java/org/zstack/header/host/UploadFileMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
🧬 Code graph analysis (1)
compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java (4)
core/src/main/java/org/zstack/core/db/Q.java (1)
Q(16-193)header/src/main/java/org/zstack/header/host/GetFileDownloadProgressMsg.java (1)
GetFileDownloadProgressMsg(5-43)header/src/main/java/org/zstack/header/host/GetFileDownloadProgressReply.java (1)
GetFileDownloadProgressReply(5-92)core/src/main/java/org/zstack/core/progress/ProgressReportService.java (1)
ProgressReportService(53-660)
🔇 Additional comments (7)
plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java (1)
121-123: 确认 kvmagent 已注册 /host/file/download 与 /host/file/download/progress 路由KVMConstant 已新增常量（plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java:121-122）；KVMHost 构建并调用了这两个 URI（plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java:462、466）；命令类在 KVMAgentCommands 中定义（DownloadFileCmd / GetDownloadFileProgressCmd，plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java:4400、4409）。仓库中未找到 agent 端将这两个 path 绑定到处理器的实现 —— 在 kvm agent（zstack-kvmagent）或运行时确认路由已绑定到 DownloadFileCmd / GetDownloadFileProgressCmd 的处理器。
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (4)
220-222: 新增下载与进度路径字段定义合理。命名清晰、与常量一致，后续被实际使用。
461-468: 初始化文件下载与进度路径正确。与其余路径初始化方式一致，基于 baseUrl 构造。
6888-6891: 方案判断简单明了。upload:// 前缀判定直观。
6877-6886: 在 handleLocalMessage 中注册 UploadFileMsg 与 GetFileDownloadProgressMsgrg 查询未返回结果，无法确认注册；检查 plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java 的 handleLocalMessage 分发分支，若缺失，追加：
- } else { + } else if (msg instanceof UploadFileMsg) { + handle((UploadFileMsg) msg); + } else if (msg instanceof GetFileDownloadProgressMsg) { + handle((GetFileDownloadProgressMsg) msg); + } else { super.handleLocalMessage(msg); }验证命令：
rg -nC2 -e 'instanceof\s+UploadFileMsg' -e 'instanceof\s+GetFileDownloadProgressMsg' plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaheader/src/main/java/org/zstack/header/host/APIUploadFileMsg.java (1)
9-13: 确认响应类已正确标注 @RestResponse 并生成文档请确认 APIUploadFileEvent 带有 @RestResponse，且已生成对应 Groovy Template 与 Markdown 文档。
compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java (1)
62-73: 结论：无需修改 — 签名未变更且调用处已匹配
UploadFileTracker.addTrackTask 仍为 addTrackTask(String taskUuid, String hostname, String url)，唯一调用点为 plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java（tracker.addTrackTask(rsp.taskUuid, self.getManagementIp(), cmd.url)）；runTrackTask 调用也匹配。
Comment on lines
+51
to
+53
| private final String apiId = ThreadContext.get(THREAD_CONTEXT_API); | ||
| private final boolean continuable = apiId != null && Q.New(LongJobVO.class).eq(LongJobVO_.apiId, apiId).isExists(); | ||
|
|
There was a problem hiding this comment.
不要在字段初始化读取 ThreadContext：apiId 会被错误固定
在 bean/实例创建时读取 THREAD_CONTEXT_API 会导致 apiId 固定为创建线程的值，后续任务均错误复用。应按任务保存或在运行时读取。
- private final String apiId = ThreadContext.get(THREAD_CONTEXT_API);
- private final boolean continuable = apiId != null && Q.New(LongJobVO.class).eq(LongJobVO_.apiId, apiId).isExists();
+ // apiId 按任务保存或在运行时从 ThreadContext 读取，避免固定为构造时的值📝 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.
Suggested change
| private final String apiId = ThreadContext.get(THREAD_CONTEXT_API); | |
| private final boolean continuable = apiId != null && Q.New(LongJobVO.class).eq(LongJobVO_.apiId, apiId).isExists(); | |
| // apiId 按任务保存或在运行时从 ThreadContext 读取，避免固定为构造时的值 |
🤖 Prompt for AI Agents
In compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java around
lines 51-53, the code reads ThreadContext during field initialization which
fixes apiId to the creating thread and causes later tasks to reuse it
incorrectly; remove ThreadContext access from field initializers and instead
obtain the apiId at task runtime or accept it as a constructor/runner parameter,
then compute continuable when needed (e.g., inside the method that starts the
upload or in a per-task init) by querying
Q.New(LongJobVO.class).eq(LongJobVO_.apiId, apiId).isExists() using that runtime
apiId; ensure fields are not final if they must be set later or replace them
with local variables computed at execution time.
Comment on lines
+54
to
+58
| public static class TrackContext { | ||
| String url; | ||
| String taskUuid; | ||
| String hostname; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
TrackContext 缺少 hostUuid/apiId，难以正确路由与打点
需要 hostUuid 进行总线路由（按资源 UUID），并保存 apiId 便于进度上报关联。
public static class TrackContext {
String url;
String taskUuid;
String hostname;
+ String hostUuid;
+ String apiId;
}📝 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.
Suggested change
| public static class TrackContext { | |
| String url; | |
| String taskUuid; | |
| String hostname; | |
| } | |
| public static class TrackContext { | |
| String url; | |
| String taskUuid; | |
| String hostname; | |
| String hostUuid; | |
| String apiId; | |
| } |
🤖 Prompt for AI Agents
In compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java around
lines 54 to 58, TrackContext is missing hostUuid and apiId which are required
for correct routing by resource UUID and for associating progress reports; add
two String fields hostUuid and apiId to TrackContext, provide appropriate
accessors/mutators (or make them package-private if consistent with surrounding
code), update any places that construct or populate TrackContext to set these
new fields, and ensure any serialization/logging/progress reporting paths read
hostUuid for routing and apiId for progress association.
Comment on lines
+62
to
+68
| public void addTrackTask(String taskUuid, String hostname, String url) { | ||
| TrackContext ctx = new TrackContext(); | ||
| ctx.url = url; | ||
| ctx.taskUuid = taskUuid; | ||
| ctx.hostname = hostname; | ||
| ctxs.put(taskUuid, ctx); | ||
| } |
There was a problem hiding this comment.
addTrackTask 需接收 hostUuid 与 apiId
保证正确路由与进度归属。若不便修改所有调用点，可新增重载。
-public void addTrackTask(String taskUuid, String hostname, String url) {
+public void addTrackTask(String taskUuid, String hostUuid, String hostname, String url, String apiId) {
TrackContext ctx = new TrackContext();
ctx.url = url;
ctx.taskUuid = taskUuid;
ctx.hostname = hostname;
+ ctx.hostUuid = hostUuid;
+ ctx.apiId = apiId != null ? apiId : ThreadContext.get(THREAD_CONTEXT_API);
ctxs.put(taskUuid, ctx);
}我可以另行提供一个保留旧签名的重载以降低改动面。
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java around
lines 62 to 68, add hostUuid and apiId to the addTrackTask signature so the
tracker records host identity and API caller for correct routing and progress
ownership; implement the method to set ctx.hostUuid and ctx.apiId (or equivalent
fields) before putting into ctxs, and add a new overloaded method with the
original three-argument signature that delegates to the new four-argument method
to preserve backward compatibility.
Comment on lines
+70
to
+73
| public void runTrackTask(String taskUuid, Completion completion) { | ||
| TrackContext ctx = ctxs.get(taskUuid); | ||
| trackUpload(ctx.taskUuid, ctx.hostname, ctx.url,completion); | ||
| } |
There was a problem hiding this comment.
runTrackTask 未判空，可能 NPE；同时建议改用 ctx 传参
避免空指针，并将上下文一次性传入后续流程。
public void runTrackTask(String taskUuid, Completion completion) {
TrackContext ctx = ctxs.get(taskUuid);
- trackUpload(ctx.taskUuid, ctx.hostname, ctx.url,completion);
+ if (ctx == null) {
+ completion.fail(operr("no track context found for taskUuid=%s", taskUuid));
+ return;
+ }
+ trackUpload(ctx, completion);
}📝 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.
Suggested change
| public void runTrackTask(String taskUuid, Completion completion) { | |
| TrackContext ctx = ctxs.get(taskUuid); | |
| trackUpload(ctx.taskUuid, ctx.hostname, ctx.url,completion); | |
| } | |
| public void runTrackTask(String taskUuid, Completion completion) { | |
| TrackContext ctx = ctxs.get(taskUuid); | |
| if (ctx == null) { | |
| completion.fail(operr("no track context found for taskUuid=%s", taskUuid)); | |
| return; | |
| } | |
| trackUpload(ctx, completion); | |
| } |
🤖 Prompt for AI Agents
In compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java around
lines 70 to 73, runTrackTask currently dereferences ctx without a null check and
passes individual fields to trackUpload; add a null-check for ctx and handle the
null case by calling completion.fail(...) or logging and returning, and refactor
to pass the whole TrackContext to trackUpload (or add an overload
trackUpload(TrackContext ctx, Completion completion)) so subsequent code uses
ctx.taskUuid, ctx.hostname and ctx.url from a non-null context; update
trackUpload signature/usage accordingly and keep completion invoked on error
paths.
Comment on lines
+15
to
+17
| @APIParam(nonempty = true, resourceType = HostVO.class) | ||
| private String uuid; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
重命名字段 uuid → hostUuid，避免语义歧义并对齐 HostMessage
“uuid” 含义不明确，建议改为 “hostUuid”，同时更新 getter/setter 和 HostMessage 返回值。
- @APIParam(nonempty = true, resourceType = HostVO.class)
- private String uuid;
+ @APIParam(nonempty = true, resourceType = HostVO.class)
+ private String hostUuid;
@@
- public String getUuid() {
- return uuid;
- }
-
- public void setUuid(String uuid) {
- this.uuid = uuid;
- }
+ public void setHostUuid(String hostUuid) {
+ this.hostUuid = hostUuid;
+ }
@@
@Override
public String getHostUuid() {
- return uuid;
+ return hostUuid;
}Also applies to: 21-27, 37-40
🤖 Prompt for AI Agents
In header/src/main/java/org/zstack/header/host/APIUploadFileMsg.java around
lines 15-17 (and also apply same changes to 21-27 and 37-40), rename the private
field "uuid" to "hostUuid" and update its @APIParam annotation usage if needed;
change the corresponding getter and setter names to getHostUuid() and
setHostUuid(String hostUuid); update any references in this class (including
HostMessage implementation/overrides) to return hostUuid instead of uuid; ensure
serialization/deserialization and any JSON/property names remain consistent with
the new field name.
Comment on lines
+18
to
+20
| @NoLogging(type = NoLogging.Type.Uri) | ||
| private String url; | ||
|
|
There was a problem hiding this comment.
为 url 增加必填校验，同时保留 NoLogging
上传 URL 应为必填，避免空值进入后续流程。
- @NoLogging(type = NoLogging.Type.Uri)
- private String url;
+ @APIParam(nonempty = true)
+ @NoLogging(type = NoLogging.Type.Uri)
+ private String url;Also applies to: 33-35
🤖 Prompt for AI Agents
In header/src/main/java/org/zstack/header/host/APIUploadFileMsg.java around
lines 18-20 (and similarly apply to lines 33-35), the url field is currently
annotated with @NoLogging but lacks a required validation; add the API parameter
validation annotation (e.g. @APIParam(required = true) or the project’s
equivalent non-null/required API validation) directly above the url field while
keeping the existing @NoLogging annotation so the URL becomes mandatory and
null/empty values are rejected before further processing.
Comment on lines
6892
to
6951
| private void getSensors1(UploadFileMsg msg, NoErrorCompletion completion) { | ||
| GetHostSensorsReply reply = new GetHostSensorsReply(); | ||
| DownloadFileCmd cmd = new DownloadFileCmd(); | ||
| UploadFileTracker tracker = new UploadFileTracker(); | ||
| new Http<>(fileDownloadPath, cmd, DownloadFileResponse.class).call(new ReturnValueCompletion<DownloadFileResponse>(msg) { | ||
| @Override | ||
| public void success(DownloadFileResponse rsp) { | ||
| if (isUpload(msg.getUrl())) { | ||
| tracker.addTrackTask(rsp.taskUuid, self.getManagementIp(), cmd.url); | ||
| tracker.runTrackTask(rsp.taskUuid, new Completion(msg) { | ||
| @Override | ||
| public void success() { | ||
| bus.reply(msg, reply); | ||
| completion.done(); | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode errorCode) { | ||
| reply.setError(operr("failed to upload file, because:%s", errorCode)); | ||
| bus.reply(msg, reply); | ||
| } | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| if (!rsp.isSuccess()) { | ||
| reply.setError(operr("failed to upload file, because:%s", rsp.getError())); | ||
| bus.reply(msg, reply); | ||
| completion.done(); | ||
| return; | ||
| } | ||
| bus.reply(msg, reply); | ||
| completion.done(); | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode errorCode) { | ||
| reply.setError(errorCode); | ||
| bus.reply(msg, reply); | ||
| completion.done(); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
上传实现存在多处功能性错误与潜在卡死。
- 方法名 getSensors1 与职责不符，违反“自解释命名”规范。
- 使用 GetHostSensorsReply 回复 UploadFileMsg，类型错误。
- DownloadFileCmd 未从 msg 填充 url/目标路径/格式等关键字段。
- 直接 new UploadFileTracker 破坏依赖注入，可能缺少 CloudBus/ProgressReport 等依赖。
- 追踪分支 fail() 未调用 completion.done()，会导致队列卡死。
- addTrackTask 第3参使用了 cmd.url（未赋值），应使用 msg.getUrl()。
建议按下述修正（仅示例，字段名请以实际定义为准）：
- private void handle(UploadFileMsg msg) {
+ private void handle(UploadFileMsg msg) {
inQueue().name(String.format("upload-file-to-host-%s", self.getUuid())).asyncBackup(msg)
- .run(chain -> getSensors1(msg, new NoErrorCompletion(chain) {
+ .run(chain -> uploadFile(msg, new NoErrorCompletion(chain) {
@Override
public void done() {
chain.next();
}
}));
}
- private void getSensors1(UploadFileMsg msg, NoErrorCompletion completion) {
- GetHostSensorsReply reply = new GetHostSensorsReply();
+ private void uploadFile(UploadFileMsg msg, NoErrorCompletion completion) {
+ MessageReply reply = new MessageReply();
DownloadFileCmd cmd = new DownloadFileCmd();
- UploadFileTracker tracker = new UploadFileTracker();
+ // 必填：从消息填充命令参数（字段名以实际类为准）
+ cmd.url = msg.getUrl();
+ cmd.installPath = msg.getInstallPath();
+ cmd.format = msg.getFormat();
+ UploadFileTracker tracker = uploadFileTracker;
new Http<>(fileDownloadPath, cmd, DownloadFileResponse.class).call(new ReturnValueCompletion<DownloadFileResponse>(msg) {
@Override
public void success(DownloadFileResponse rsp) {
if (isUpload(msg.getUrl())) {
- tracker.addTrackTask(rsp.taskUuid, self.getManagementIp(), cmd.url);
+ tracker.addTrackTask(rsp.taskUuid, self.getManagementIp(), msg.getUrl());
tracker.runTrackTask(rsp.taskUuid, new Completion(msg) {
@Override
public void success() {
bus.reply(msg, reply);
completion.done();
}
@Override
public void fail(ErrorCode errorCode) {
- reply.setError(operr("failed to upload file, because:%s", errorCode));
+ reply.setError(operr("failed to upload file, because:%s", errorCode.getDetails()));
bus.reply(msg, reply);
+ completion.done();
}
});
return;
}
if (!rsp.isSuccess()) {
reply.setError(operr("failed to upload file, because:%s", rsp.getError()));
bus.reply(msg, reply);
completion.done();
return;
}
bus.reply(msg, reply);
completion.done();
}
@Override
public void fail(ErrorCode errorCode) {
reply.setError(errorCode);
bus.reply(msg, reply);
completion.done();
}
});
}此外，需要在类字段区注入 tracker（变更发生在当前片段之外）：
@Autowired
private UploadFileTracker uploadFileTracker;🤖 Prompt for AI Agents
In plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java around lines 6892 to
6934, this method has multiple functional bugs: rename getSensors1 to a
self-explanatory name for upload handling; replace GetHostSensorsReply with the
correct UploadFileReply (or appropriate reply type matching UploadFileMsg);
populate DownloadFileCmd fields from msg (url, destination path, format, etc.)
before issuing the HTTP call; stop instantiating UploadFileTracker with new and
use an injected UploadFileTracker field (add @Autowired private
UploadFileTracker uploadFileTracker to the class); in the upload tracking branch
call addTrackTask with msg.getUrl() (not cmd.url) and use the injected tracker;
ensure every fail() path (including the tracker fail callback) calls
completion.done() after bus.reply(msg, reply) to avoid deadlocks; and keep error
messages consistent by wrapping and setting reply errors from real error
objects.
Comment on lines
+6936
to
+6982
| private void handle(GetFileDownloadProgressMsg msg) { | ||
| GetFileDownloadProgressReply r = new GetFileDownloadProgressReply(); | ||
| if (CoreGlobalProperty.UNIT_TEST_ON) { | ||
| bus.reply(msg, r); | ||
| return; | ||
| } | ||
|
|
||
| GetDownloadFileProgressCmd cmd = new GetDownloadFileProgressCmd(); | ||
| new Http<>(fileDownloadProgressPath, cmd, GetDownloadFileProgressResponse.class).call(new ReturnValueCompletion<GetDownloadFileProgressResponse>(msg) { | ||
| @Override | ||
| public void success(GetDownloadFileProgressResponse resp) { | ||
| r.setCompleted(resp.completed); | ||
| r.setProgress(resp.progress); | ||
| r.setActualSize(resp.actualSize); | ||
| r.setSize(resp.size); | ||
| r.setInstallPath(resp.installPath); | ||
| r.setFormat(resp.format); | ||
| r.setDownloadSize(resp.downloadSize); | ||
| r.setLastOpTime(resp.lastOpTime); | ||
| r.setSupportSuspend(true); | ||
| bus.reply(msg, r); | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode errorCode) { | ||
| r.setError(errorCode); | ||
| bus.reply(msg, r); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
进度查询未携带标识且未校验返回成功标志。
- GetDownloadFileProgressCmd 未设置 taskUuid/url，后端无法定位任务。
- 未检查 resp.isSuccess()，直接填充进度可能掩盖错误。
- supportSuspend 被硬编码为 true，应以响应为准（若有）。
建议修正如下（字段名以实际类为准）：
private void handle(GetFileDownloadProgressMsg msg) {
GetFileDownloadProgressReply r = new GetFileDownloadProgressReply();
if (CoreGlobalProperty.UNIT_TEST_ON) {
bus.reply(msg, r);
return;
}
GetDownloadFileProgressCmd cmd = new GetDownloadFileProgressCmd();
+ // 进度查询的唯一标识（两者至少其一）
+ cmd.taskUuid = msg.getTaskUuid();
+ cmd.url = msg.getUrl();
new Http<>(fileDownloadProgressPath, cmd, GetDownloadFileProgressResponse.class).call(new ReturnValueCompletion<GetDownloadFileProgressResponse>(msg) {
@Override
public void success(GetDownloadFileProgressResponse resp) {
- r.setCompleted(resp.completed);
- r.setProgress(resp.progress);
- r.setActualSize(resp.actualSize);
- r.setSize(resp.size);
- r.setInstallPath(resp.installPath);
- r.setFormat(resp.format);
- r.setDownloadSize(resp.downloadSize);
- r.setLastOpTime(resp.lastOpTime);
- r.setSupportSuspend(true);
+ if (resp.isSuccess()) {
+ r.setCompleted(resp.completed);
+ r.setProgress(resp.progress);
+ r.setActualSize(resp.actualSize);
+ r.setSize(resp.size);
+ r.setInstallPath(resp.installPath);
+ r.setFormat(resp.format);
+ r.setDownloadSize(resp.downloadSize);
+ r.setLastOpTime(resp.lastOpTime);
+ r.setSupportSuspend(resp.supportSuspend);
+ } else {
+ r.setError(operr("failed to get file download progress, because:%s", resp.getError()));
+ }
bus.reply(msg, r);
}
@Override
public void fail(ErrorCode errorCode) {
r.setError(errorCode);
bus.reply(msg, r);
}
});
}📝 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.
Suggested change
| private void handle(GetFileDownloadProgressMsg msg) { | |
| GetFileDownloadProgressReply r = new GetFileDownloadProgressReply(); | |
| if (CoreGlobalProperty.UNIT_TEST_ON) { | |
| bus.reply(msg, r); | |
| return; | |
| } | |
| GetDownloadFileProgressCmd cmd = new GetDownloadFileProgressCmd(); | |
| new Http<>(fileDownloadProgressPath, cmd, GetDownloadFileProgressResponse.class).call(new ReturnValueCompletion<GetDownloadFileProgressResponse>(msg) { | |
| @Override | |
| public void success(GetDownloadFileProgressResponse resp) { | |
| r.setCompleted(resp.completed); | |
| r.setProgress(resp.progress); | |
| r.setActualSize(resp.actualSize); | |
| r.setSize(resp.size); | |
| r.setInstallPath(resp.installPath); | |
| r.setFormat(resp.format); | |
| r.setDownloadSize(resp.downloadSize); | |
| r.setLastOpTime(resp.lastOpTime); | |
| r.setSupportSuspend(true); | |
| bus.reply(msg, r); | |
| } | |
| @Override | |
| public void fail(ErrorCode errorCode) { | |
| r.setError(errorCode); | |
| bus.reply(msg, r); | |
| } | |
| }); | |
| } | |
| private void handle(GetFileDownloadProgressMsg msg) { | |
| GetFileDownloadProgressReply r = new GetFileDownloadProgressReply(); | |
| if (CoreGlobalProperty.UNIT_TEST_ON) { | |
| bus.reply(msg, r); | |
| return; | |
| } | |
| GetDownloadFileProgressCmd cmd = new GetDownloadFileProgressCmd(); | |
| // 进度查询的唯一标识（两者至少其一） | |
| cmd.taskUuid = msg.getTaskUuid(); | |
| cmd.url = msg.getUrl(); | |
| new Http<>(fileDownloadProgressPath, cmd, GetDownloadFileProgressResponse.class).call(new ReturnValueCompletion<GetDownloadFileProgressResponse>(msg) { | |
| @Override | |
| public void success(GetDownloadFileProgressResponse resp) { | |
| if (resp.isSuccess()) { | |
| r.setCompleted(resp.completed); | |
| r.setProgress(resp.progress); | |
| r.setActualSize(resp.actualSize); | |
| r.setSize(resp.size); | |
| r.setInstallPath(resp.installPath); | |
| r.setFormat(resp.format); | |
| r.setDownloadSize(resp.downloadSize); | |
| r.setLastOpTime(resp.lastOpTime); | |
| r.setSupportSuspend(resp.supportSuspend); | |
| } else { | |
| r.setError(operr("failed to get file download progress, because:%s", resp.getError())); | |
| } | |
| bus.reply(msg, r); | |
| } | |
| @Override | |
| public void fail(ErrorCode errorCode) { | |
| r.setError(errorCode); | |
| bus.reply(msg, r); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java around lines 6936 to
6965, the GetFileDownloadProgressCmd is sent without identifying the task and
the response is assumed successful; update the cmd to set task identifiers
(taskUuid and/or url) from the incoming msg so the backend can locate the task,
null-check the response and verify resp.isSuccess() before copying progress
fields (on failure populate r.setError(...) with an appropriate ErrorCode and
reply), and set r.setSupportSuspend(...) from the response value instead of
hardcoding true; ensure all branches reply to the msg.
96b5f98 to
5466be4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java (1)
60-61: 并发安全：ctxs 使用 ConcurrentHashMap避免并发访问导致的不一致。
- Map<String, TrackContext> ctxs = new HashMap<>(); + Map<String, TrackContext> ctxs = new ConcurrentHashMap<>();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
compute/src/main/java/org/zstack/compute/host/HostManagerImpl.java(2 hunks)compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java(1 hunks)header/src/main/java/org/zstack/header/host/APIUploadFileEvent.java(1 hunks)header/src/main/java/org/zstack/header/host/APIUploadFileMsg.java(1 hunks)header/src/main/java/org/zstack/header/host/GetFileDownloadProgressMsg.java(1 hunks)header/src/main/java/org/zstack/header/host/GetFileDownloadProgressReply.java(1 hunks)header/src/main/java/org/zstack/header/host/UploadFileMsg.java(1 hunks)plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java(1 hunks)plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java(1 hunks)plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
- header/src/main/java/org/zstack/header/host/GetFileDownloadProgressMsg.java
- compute/src/main/java/org/zstack/compute/host/HostManagerImpl.java
- header/src/main/java/org/zstack/header/host/GetFileDownloadProgressReply.java
- header/src/main/java/org/zstack/header/host/APIUploadFileMsg.java
- plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.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:
compute/src/main/java/org/zstack/compute/host/UploadFileTracker.javaheader/src/main/java/org/zstack/header/host/APIUploadFileEvent.javaheader/src/main/java/org/zstack/header/host/UploadFileMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
🧬 Code graph analysis (1)
compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java (4)
core/src/main/java/org/zstack/core/db/Q.java (1)
Q(16-193)header/src/main/java/org/zstack/header/host/GetFileDownloadProgressMsg.java (1)
GetFileDownloadProgressMsg(5-43)header/src/main/java/org/zstack/header/host/GetFileDownloadProgressReply.java (1)
GetFileDownloadProgressReply(5-92)core/src/main/java/org/zstack/core/progress/ProgressReportService.java (1)
ProgressReportService(53-660)
🔇 Additional comments (21)
header/src/main/java/org/zstack/header/host/APIUploadFileEvent.java (2)
6-12: 补齐事件构造函数，支持按约定传入 apiId多数事件类支持
new APIUploadFileEvent(msg.getId())。建议新增无参与带apiId的构造函数，以便正确关联 API 请求 ID。应用此补丁：
@RestResponse(fieldsTo = {"all"}) public class APIUploadFileEvent extends APIEvent { + public APIUploadFileEvent() { + } + + public APIUploadFileEvent(String apiId) { + super(apiId); + } public static APIUploadFileEvent __example__() { APIUploadFileEvent event = new APIUploadFileEvent(); return event; } }
6-11: RestResponse 与 example 实现到位事件类注解与示例方法符合文档生成与返回体约定。
header/src/main/java/org/zstack/header/host/UploadFileMsg.java (2)
6-36: 消息体设计简洁清晰，符合 HostMessage 约定字段与访问器命名规范，
getHostUuid()覆盖正确。
8-10: 确认：NoLogging.Type 枚举为 Uri；需排查并处理其它直接打印 url 的日志点NoLogging.Type 包含 Simple、Tag、Uri（header/src/main/java/org/zstack/header/log/NoLogging.java）。扫描结果（发现直接包含或格式化打印 url 的日志点），请逐一处理（加 @nologging 或避免打印）：
- storage/src/main/java/org/zstack/storage/backup/BackupStorageBase.java:193
- storage/src/main/java/org/zstack/storage/primary/PrimaryStorageManagerImpl.java:364, 369-370
- storage/src/main/java/org/zstack/storage/primary/ImageCacheGarbageCollector.java:130
- plugin/sugonSdnController/src/main/java/org/zstack/sugonSdnController/controller/api/TfHttpClient.java:182
- image/src/main/java/org/zstack/image/ImageQuotaUtil.java:105, 117
- core/src/main/java/org/zstack/core/Platform.java:328, 342
- core/src/main/java/org/zstack/core/webhook/WebhookCaller.java:48
- header/src/main/java/org/zstack/header/rest/TimeoutRestTemplate.java:48-49
- core/src/main/java/org/zstack/core/rest/RESTFacadeImpl.java:153, 423, 649
- compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java:84
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (8)
17-17: 导入变更 OK。通配导入在本文件规模下可接受，无功能风险。
96-98: 新增 URI 相关导入 OK。为解析上传 URL 提供所需依赖。
222-224: 新增下载/进度路径字段 OK。命名清晰，与构造器中的初始化相匹配。
463-470: 构造器中新增路径初始化 OK。与 KVMConstant 常量配套，风格与其他路径一致。
720-721: 未在 handleLocalMessage 注册进度查询消息，导致无法处理进度请求。已新增 GetFileDownloadProgressMsg 的处理方法，但未在分发处挂接，消息将落到 super.handleLocalMessage 并大概率无法处理。
请按下述方式补充分发分支：
} else if (msg instanceof UpdateHostNqnMsg) { handle((UpdateHostNqnMsg) msg); -} else if (msg instanceof UploadFileMsg) { +} else if (msg instanceof UploadFileMsg) { handle((UploadFileMsg) msg); +} else if (msg instanceof GetFileDownloadProgressMsg) { + handle((GetFileDownloadProgressMsg) msg); } else { super.handleLocalMessage(msg); }
6882-6891: 方法命名与职责不符：getSensors1 应重命名为 uploadFile，并修正调用处。当前名称与“文件上传”业务不相干，违反自解释命名规范。
请同步修改调用点：
- .run(chain -> getSensors1(msg, new NoErrorCompletion(chain) { + .run(chain -> uploadFile(msg, new NoErrorCompletion(chain) {
6896-6951: 上传实现存在类型错误、依赖注入缺失及可能卡死。
- 使用 GetHostSensorsReply 回复 UploadFileMsg，类型错误。
- new UploadFileTracker() 破坏依赖注入。
- 追踪失败分支未调用 completion.done()，会卡死队列（严重）。
- 不应将 cmd.taskUuid 设为 host UUID；应由后端生成或来自 msg。
- 建议将错误详情使用 errorCode.getDetails()，便于排查。
请按下述修正（字段名以实际为准）：
- private void getSensors1(UploadFileMsg msg, NoErrorCompletion completion) { - GetHostSensorsReply reply = new GetHostSensorsReply(); + private void uploadFile(UploadFileMsg msg, NoErrorCompletion completion) { + MessageReply reply = new MessageReply(); // 或替换为与 UploadFileMsg 匹配的 Reply 类型 String scheme; try { URI uri = new URI(msg.getUrl()); scheme = uri.getScheme(); } catch (URISyntaxException e) { throw new CloudRuntimeException(e); } DownloadFileCmd cmd = new DownloadFileCmd(); - cmd.taskUuid =self.getUuid(); cmd.url = msg.getUrl(); cmd.urlScheme = scheme; cmd.timeout = timeoutManager.getTimeout(); cmd.installPath = msg.getInstallPath(); + // 可选：若协议需要，补充格式等字段 + // cmd.format = msg.getFormat(); - UploadFileTracker tracker = new UploadFileTracker(); + UploadFileTracker tracker = uploadFileTracker; new Http<>(fileDownloadPath, cmd, DownloadFileResponse.class).call(new ReturnValueCompletion<DownloadFileResponse>(msg) { @Override public void success(DownloadFileResponse rsp) { if (isUpload(msg.getUrl())) { - tracker.addTrackTask(rsp.taskUuid, self.getManagementIp(), cmd.url); + tracker.addTrackTask(rsp.taskUuid, self.getManagementIp(), msg.getUrl()); tracker.runTrackTask(rsp.taskUuid, new Completion(msg) { @Override public void success() { bus.reply(msg, reply); completion.done(); } @Override public void fail(ErrorCode errorCode) { - reply.setError(operr("failed to upload file, because:%s", errorCode)); + reply.setError(operr("failed to upload file, because:%s", errorCode.getDetails())); bus.reply(msg, reply); + completion.done(); } }); return; } if (!rsp.isSuccess()) { reply.setError(operr("failed to upload file, because:%s", rsp.getError())); bus.reply(msg, reply); completion.done(); return; } bus.reply(msg, reply); completion.done(); } @Override public void fail(ErrorCode errorCode) { reply.setError(errorCode); bus.reply(msg, reply); completion.done(); } }); }并在类字段区注入 Tracker（变更发生在当前片段之外）：
@Autowired private UploadFileTracker uploadFileTracker;
6953-6982: 进度查询缺少任务标识且未校验返回成功标志。
- 未设置 taskUuid/url，后端无法定位任务。
- 未判断 resp.isSuccess()，硬写入进度字段会掩盖错误。
- supportSuspend 不应硬编码为 true。
请修正如下：
GetDownloadFileProgressCmd cmd = new GetDownloadFileProgressCmd(); +cmd.taskUuid = msg.getTaskUuid(); +cmd.url = msg.getUrl(); new Http<>(fileDownloadProgressPath, cmd, GetDownloadFileProgressResponse.class).call(new ReturnValueCompletion<GetDownloadFileProgressResponse>(msg) { @Override public void success(GetDownloadFileProgressResponse resp) { - r.setCompleted(resp.completed); - r.setProgress(resp.progress); - r.setActualSize(resp.actualSize); - r.setSize(resp.size); - r.setInstallPath(resp.installPath); - r.setFormat(resp.format); - r.setDownloadSize(resp.downloadSize); - r.setLastOpTime(resp.lastOpTime); - r.setSupportSuspend(true); + if (resp != null && resp.isSuccess()) { + r.setCompleted(resp.completed); + r.setProgress(resp.progress); + r.setActualSize(resp.actualSize); + r.setSize(resp.size); + r.setInstallPath(resp.installPath); + r.setFormat(resp.format); + r.setDownloadSize(resp.downloadSize); + r.setLastOpTime(resp.lastOpTime); + r.setSupportSuspend(resp.supportSuspend); + } else { + r.setError(operr("failed to get file download progress, because:%s", + resp == null ? "null response" : resp.getError())); + } bus.reply(msg, r); }compute/src/main/java/org/zstack/compute/host/UploadFileTracker.java (9)
26-29: 并发安全：为 ctxs 使用 ConcurrentHashMap 需添加导入[ suggest_recommended_refactor ]
+import java.util.concurrent.ConcurrentHashMap;
21-23: 修正消息路由与错误导入（应路由 Host 服务，移除备份存储相关类）当前导入了备份存储常量与错误的 Reply 类型，导致路由与类型错配。
-import org.zstack.header.storage.backup.BackupStorageConstant; -import org.zstack.header.storage.backup.GetImageDownloadProgressReply; +import org.zstack.header.host.HostConstant;
140-153: 更正进度查询消息的构造与路由（携带 hostUuid/url/taskUuid，并路由 Host 服务）避免将 apiId 误当 taskUuid，且需按 hostUuid 精准路由。
- public GetFileDownloadProgressReply getImageDownloadProgress(String hostname) { - final GetFileDownloadProgressMsg dmsg = new GetFileDownloadProgressMsg(); - dmsg.setTaskUuid(apiId); - dmsg.setHostname(hostname); - bus.makeLocalServiceId(dmsg, BackupStorageConstant.SERVICE_ID); + public GetFileDownloadProgressReply getFileDownloadProgress(TrackContext ctx) { + final GetFileDownloadProgressMsg dmsg = new GetFileDownloadProgressMsg(); + dmsg.setTaskUuid(ctx.taskUuid); + dmsg.setHostname(ctx.hostname); + dmsg.setHostUuid(ctx.hostUuid); + dmsg.setUrl(ctx.url); + bus.makeTargetServiceIdByResourceUuid(dmsg, HostConstant.SERVICE_ID, ctx.hostUuid); final MessageReply reply = bus.call(dmsg); if (reply.isSuccess()) { - return reply.castReply(); + return reply.castReply(); } else { GetFileDownloadProgressReply r = new GetFileDownloadProgressReply(); r.setError(reply.getError()); return r; } }
51-53: 不要在字段初始化读取 ThreadContext（apiId/continuable 会被错误固定）移除这两行，改为“按任务在运行时获取/传递”。
- private final String apiId = ThreadContext.get(THREAD_CONTEXT_API); - private final boolean continuable = apiId != null && Q.New(LongJobVO.class).eq(LongJobVO_.apiId, apiId).isExists();
54-58: TrackContext 缺少 hostUuid/apiId用于精确路由与进度归属。
public static class TrackContext { String url; String taskUuid; String hostname; + String hostUuid; + String apiId; }
62-68: addTrackTask 需接收 hostUuid 与 apiId（否则无法路由与正确打点）这是必要签名变更；请同步更新调用点。
- public void addTrackTask(String taskUuid, String hostname, String url) { + public void addTrackTask(String taskUuid, String hostUuid, String hostname, String url, String apiId) { TrackContext ctx = new TrackContext(); ctx.url = url; ctx.taskUuid = taskUuid; ctx.hostname = hostname; + ctx.hostUuid = hostUuid; + ctx.apiId = apiId != null ? apiId : ThreadContext.get(THREAD_CONTEXT_API); ctxs.put(taskUuid, ctx); }建议同时保留一个向后兼容的三参重载（插入到同一类中任意合适位置）：
public void addTrackTask(String taskUuid, String hostname, String url) { addTrackTask(taskUuid, null, hostname, url, null); }验证调用点（请在仓库根目录执行）：
#!/bin/bash rg -nP '\baddTrackTask\s*\(' -g '!**/target/**'
70-73: runTrackTask 需判空并改为传递 ctx（避免 NPE）public void runTrackTask(String taskUuid, Completion completion) { TrackContext ctx = ctxs.get(taskUuid); - trackUpload(ctx.taskUuid, ctx.hostname, ctx.url,completion); + if (ctx == null) { + completion.fail(operr("no track context found for taskUuid=%s", taskUuid)); + return; + } + trackUpload(ctx, completion); }
75-131: 轮询逻辑错误：首次成功即停止、超时判断错误、未回调 completion重写为：用 reply.lastOpTime 判定 idle；仅在 completed 时停止；在失败/完成时调用 completion；并上报进度。
- void trackUpload(String taskUuid, String hostname,String url,Completion completion) { + void trackUpload(TrackContext ctx, Completion completion) { final int maxNumOfFailure = 3; final long maxIdleSecond = 10; thdf.submitCancelablePeriodicTask(new CancelablePeriodicTask() { - private long numError = 0; - private long createdTime = System.currentTimeMillis(); + private long numError = 0; + private final long createdTime = System.currentTimeMillis(); - private void markCompletion(final GetImageDownloadProgressReply dr) { - logger.debug(String.format("added file [name: %s, taskUuid: %s]", url, taskUuid)); + private void markCompletion(final GetFileDownloadProgressReply dr) { + logger.debug(String.format("upload completed [url: %s, taskUuid: %s, progress: %s%%]", + ctx.url, ctx.taskUuid, dr.getProgress())); + ctxs.remove(ctx.taskUuid); + completion.success(); } private void markFailure(ErrorCode reason) { + logger.warn(String.format("upload failed [url: %s, taskUuid: %s], reason: %s", + ctx.url, ctx.taskUuid, reason)); + ctxs.remove(ctx.taskUuid); + completion.fail(reason); } private boolean overMaxIdleTime(long lastOpTimeInMills) { - long latestTime = Long.max(lastOpTimeInMills, createdTime); + long latestTime = Math.max(lastOpTimeInMills, createdTime); return System.currentTimeMillis() - latestTime > TimeUnit.SECONDS.toMillis(maxIdleSecond); } @Override public boolean run() { - if (overMaxIdleTime(createdTime)) { - markFailure(operr("upload session expired")); - return true; - } - - final GetFileDownloadProgressReply reply = getImageDownloadProgress(hostname); + final String apiId = ctx.apiId != null ? ctx.apiId : ThreadContext.get(THREAD_CONTEXT_API); + final GetFileDownloadProgressReply reply = getFileDownloadProgress(ctx); if (!reply.isSuccess()) { if (++numError <= maxNumOfFailure) { return false; } markFailure(reply.getError()); return true; } - return true; + if (overMaxIdleTime(reply.getLastOpTime())) { + markFailure(operr("upload session expired (idle > %ss)", maxIdleSecond)); + return true; + } + + if (reply.isCompleted()) { + markCompletion(reply); + return true; + } + + doReportProgress(getName(), reply.getProgress(), apiId); + return false; } @Override public TimeUnit getTimeUnit() { return TimeUnit.SECONDS; } @@ @Override public String getName() { - return String.format("tracking upload file [name: %s, taskUuid: %s]", url, taskUuid); + return String.format("tracking upload file [url: %s, taskUuid: %s]", ctx.url, ctx.taskUuid); } }); }
134-138: ThreadContext 设置后未清理，易污染线程池线程用 try/finally 清理，并将 apiId 作为参数传入。
- private void doReportProgress(String taskName, long progress) { - ThreadContext.put(THREAD_CONTEXT_API, apiId); - ThreadContext.put(THREAD_CONTEXT_TASK_NAME, taskName); - reportProgress(String.valueOf(progress)); - } + private void doReportProgress(String taskName, long progress, String apiId) { + ThreadContext.put(THREAD_CONTEXT_API, apiId); + ThreadContext.put(THREAD_CONTEXT_TASK_NAME, taskName); + try { + reportProgress(String.valueOf(progress)); + } finally { + ThreadContext.remove(THREAD_CONTEXT_API); + ThreadContext.remove(THREAD_CONTEXT_TASK_NAME); + } + }
APIImpact Resolves: ZSV-9928 Change-Id: I6465616177776e656c6b6a61696b696a6f75716f
MatheMatrix
force-pushed
the
sync/tao.gan/ZSV-9928@@2
branch
2 times, most recently
from
316dc8b to
d19e135
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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-9928
Change-Id: I6465616177776e656c6b6a61696b696a6f75716f
sync from gitlab !8456