feat(task): persist task timestamps across restarts - #2914
Open
fryeggs wants to merge 1 commit into
Open
Conversation
Co-Authored-By: OpenAI Codex <noreply@openai.com>
21 tasks
PIKACHUIM
reviewed
Aug 11, 2026
PIKACHUIM
left a comment
Member
There was a problem hiding this comment.
🙏 感谢贡献
感谢 @fryeggs 提交此PR!我已完成代码评审,以下是评审结果。
📖 PR背景与需求
PR标题:feat(task): persist task timestamps across restarts
需求说明:将任务开始/结束时间纳入持久化任务表示,使复制任务在正常重启或容器重建后能够恢复可见的时间信息。之前这些时间戳是私有字段,重启后会丢失,导致任务列表中看不到任务的实际执行时间。
预期目标:
- 任务的
start_time和end_time字段能够被序列化到 JSON 持久化存储 - 容器重启或正常重启后,这些时间戳能从存储中恢复
- 保持向后兼容,旧的持久化数据仍然可读
📋 问题摘要
- ✅ 功能性:功能设计合理,解决了实际问题
- ✅ 代码质量:代码简洁清晰,测试覆盖充分
- 💡 改进建议:无重大问题,有1处可优化点
📂 逐文件分析
internal/task/base.go
改动意图:将 startTime 和 endTime 从私有字段改为公开字段,并添加 JSON 序列化标签,使其能够被持久化。
代码修改逻辑:
- 将
startTime *time.Time改为StartTime *time.Time(首字母大写,变为公开字段) - 将
endTime *time.Time改为EndTime *time.Time(首字母大写,变为公开字段) - 添加 JSON 标签:
json:"start_time,omitempty"和json:"end_time,omitempty" - 更新所有访问这两个字段的方法(
SetStartTime、GetStartTime、SetEndTime、GetEndTime、ClearEndTime),使其引用新的公开字段名
合理性评估:
-
✅ 优点:
- 改动最小化,仅涉及字段重命名和标签添加
- 使用
omitempty标签,未设置时间戳的任务不会序列化这些字段,保持 JSON 简洁 - 所有访问器方法保持不变,对外部调用者透明
- 向后兼容:旧的 JSON 数据没有这些字段时,会被解析为 nil,不会报错
-
⚠️ 疑问:- 字段变为公开后,是否会破坏封装性?理论上外部代码可以直接访问
task.StartTime而绕过访问器方法。
- 字段变为公开后,是否会破坏封装性?理论上外部代码可以直接访问
-
💡 建议:
- 如果担心封装性,可以考虑使用自定义 JSON 序列化(实现
MarshalJSON和UnmarshalJSON),保持字段私有。但考虑到当前改动简洁且团队内部代码可控,现有方案是合理的。
- 如果担心封装性,可以考虑使用自定义 JSON 序列化(实现
详细建议:
无需修改,当前实现已经非常简洁高效。
internal/fs/copy_queue_persistence_test.go
改动意图:添加回归测试,验证任务从持久化 JSON 中反序列化后,所有字段(包括新增的时间戳字段)都能正确恢复。
代码修改逻辑:
- 构造一个完整的任务 JSON(包含
start_time、end_time、Creator、TaskType、路径等字段) - 反序列化为
FileTransferTask对象 - 验证所有关键字段:
- 基础字段(ID、状态)
- 创建者信息(
Creator.Username) - 时间戳字段(
GetStartTime()、GetEndTime()不为 nil) - 任务类型(
TaskType == copy) - 路径字段(
SrcActualPath、DstActualPath) - 重试初始化延迟(
maxRetry初始为 0,调用SetRetry后为 2) - 任务分组(
groupID根据目标路径重建)
合理性评估:
-
✅ 优点:
- 测试覆盖全面,验证了迁移场景下的所有关键字段恢复
- 测试名称清晰(
TestMigratedCopyTaskRecoversNativeFields),准确描述测试意图 - 使用真实的 JSON 格式,模拟实际持久化数据
- 验证了重试初始化的延迟行为(迁移时不初始化,首次调用
SetRetry时才初始化) - 恢复原始配置(
t.Cleanup),避免测试污染全局状态
-
💡 可选优化:
- 可以添加一个测试用例验证旧格式(没有
start_time/end_time字段)的 JSON 也能正常解析(向后兼容性测试) - 可以验证序列化后的 JSON 是否包含正确的字段(测试双向转换)
- 可以添加一个测试用例验证旧格式(没有
详细建议:
当前测试已经非常完善。如果希望进一步加强,可以补充如下测试:
func TestLegacyTaskFormatWithoutTimestamps(t *testing.T) {
previousConf := conf.Conf
conf.Conf = &conf.Config{}
t.Cleanup(func() { conf.Conf = previousConf })
// 旧格式 JSON,没有 start_time 和 end_time 字段
raw := []byte(`{
"id":"legacy-task",
"state":0,
"Creator":{"id":1,"username":"admin","password":"","base_path":"/","role":2,"disabled":false,"permission":511,"sso_id":"","allow_ldap":true},
"TotalBytes":42,
"src_path":"/folder/file",
"dst_path":"/backup",
"TaskType":0
}`)
var task FileTransferTask
if err := json.Unmarshal(raw, &task); err != nil {
t.Fatalf("unmarshal legacy task: %v", err)
}
// 验证时间戳为 nil(向后兼容)
if task.GetStartTime() != nil || task.GetEndTime() != nil {
t.Fatal("legacy task should not have timestamps")
}
// 验证其他字段正常
if task.GetID() != "legacy-task" || task.GetCreator().Username != "admin" {
t.Fatal("legacy task fields were not recovered")
}
}但这不是必需的,因为 omitempty 标签已经隐含了向后兼容性。
🎯 总体评价
功能性:⭐⭐⭐⭐⭐ - 功能设计合理,精准解决了任务时间戳丢失的问题
安全性:⭐⭐⭐⭐⭐ - 无安全隐患,向后兼容
代码质量:⭐⭐⭐⭐⭐ - 代码简洁清晰,测试覆盖充分,改动最小化
实现方案:⭐⭐⭐⭐⭐ - 最优实现方案,改动最小,效果最好
建议操作:
- ✅ Approve(建议合并)
- 🔄 Request Changes(需要修改)
- ❌ Close(建议关闭)
理由:此 PR 改动精准、测试完善、向后兼容,可以安全合并。字段公开化在当前上下文中是合理的权衡(简洁性 > 封装性)。
Next Steps / 后续建议:
- 合并后,可以考虑在文档中说明任务持久化格式的演进历史
- 如果未来有更多字段需要持久化,可以考虑建立统一的迁移测试套件
再次感谢你的贡献!👏
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
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.
Summary / 摘要
This PR makes task start/end timestamps part of the persisted task representation so copy tasks can recover their visible timing information after a normal restart or container recreation. It also adds a regression test covering native task unmarshalling and recovery fields.
本 PR 将任务开始/结束时间纳入持久化任务表示,使复制任务在正常重启或容器重建后能够恢复可见的时间信息,并增加原生任务反序列化与恢复字段的回归测试。
start_time/end_timefields; older payloads remain readable.Related repository PRs / 关联仓库 PR:
Testing / 测试
go test ./internal/fs -run '^TestMigratedCopyTaskRecoversNativeFields$' -count=1go test ./...— the current upstream baseline still has unrelated failures in several drivers under the current Go toolchain, an environment-dependentinternal/nettransport assertion, and aria2 RPC tests when no local aria2 service is running.Checklist / 检查清单
gofmtorgo fmt.AI Disclosure / AI 使用声明
Tools used / 使用工具:
Usage scope / 使用范围:
Code generation / 代码生成
Refactoring / 重构
Tests / 测试
Review assistance / 审查辅助
I have reviewed and validated all AI-assisted content included in this PR.
I have ensured that this AI-assisted commit includes
Co-Authored-Byattribution.I can reproduce the checked behavior from the committed source and test commands without relying on hidden runtime state.