fix: set download speed and prototype

[zhaohuiw42]

Bug: https://pms.uniontech.com/bug-view-355963.html

[github-actions]

CLA Assistant Lite bot:
提交邮箱中包含我们的合作伙伴，但您似乎并非合作伙伴的成员或对接人，请联系相关对接人将您添加至组织之中，或由其重新发起 Pull Request。
The commit email domain belongs to one of our partners, but it seems you are not yet a member of the current organization, please contact the contact person to add you to the organization or let them submit the Pull Request.

_{You can retrigger this bot by commenting recheck in this Pull Request}

[deepin-ci-robot]

deepin pr auto review

代码审查报告：job.go 和 job_test.go 的修改

总体评价

这次修改主要增强了下载速度和协议信息的处理逻辑，引入了更精细的状态管理和协议规范化。代码整体结构清晰，但在某些方面仍有优化空间。

详细审查意见

1. 语法逻辑

1.1 updateDeliveryDownloadInfo 函数

if info.Speed == 0 && info.Proto == "" {
    return
}

问题：这个逻辑可能过于严格。如果速度为0但协议不为空（或反之），仍然应该处理。
建议：考虑拆分条件或使用更宽松的判断：

if info.Speed == 0 && info.Proto == "" {
    return
}
// 或者
if info.Speed == 0 && info.Proto == "" && !j.hasDeliveryDownloadInfo {
    return
}

1.2 updateInfo 函数中的速度更新逻辑

if !j.hasDeliveryDownloadInfo {
    // ...处理逻辑
}

问题：当 hasDeliveryDownloadInfo 为 true 时，速度更新完全依赖 updateDeliveryDownloadInfo，但 updateInfo 中的进度更新可能仍然需要计算速度。
建议：确保两种更新路径不会导致速度计算不一致。

2. 代码质量

2.1 魔法值

DeliverySpeed: -1,

问题：-1 作为初始值没有明确含义。
建议：定义常量：

const (
    InvalidDeliverySpeed = -1
)

2.2 函数职责

updateDeliveryDownloadInfo 函数现在做了太多事情：

1. 验证输入
2. 更新速度
3. 规范化协议
4. 发送属性变更信号

建议：考虑拆分为更小的函数：

func (j *Job) updateDeliveryDownloadInfo(info system.JobDeliveryDownloadInfo) {
    if !j.shouldUpdateDeliveryInfo(info) {
        return
    }
    
    j.updateSpeed(info.Speed)
    j.updateProto(info.Proto)
    j.hasDeliveryDownloadInfo = true
}

func (j *Job) shouldUpdateDeliveryInfo(info system.JobDeliveryDownloadInfo) bool {
    return info.Speed != 0 || info.Proto != ""
}

func (j *Job) updateSpeed(speed int64) {
    // ...原有速度更新逻辑
}

func (j *Job) updateProto(proto string) {
    // ...原有协议更新逻辑
}

3. 代码性能

3.1 字符串处理

func normalizeDownloadProto(proto string) string {
    switch strings.ToLower(proto) {
    // ...
    }
}

问题：每次调用都会进行字符串小写转换，如果频繁调用会有性能开销。
建议：如果协议值有限且固定，可以考虑使用枚举或常量：

const (
    ProtoHTTP = "http"
    ProtoP2P = "p2p"
)

func normalizeDownloadProto(proto string) string {
    switch proto {
    case "http", "https":
        return ProtoHTTP
    case "p2p", "delivery":
        return ProtoP2P
    default:
        return proto
    }
}

或者缓存常用协议的小写形式。

3.2 锁粒度

func (j *Job) updateDeliveryDownloadInfo(info system.JobDeliveryDownloadInfo) {
    j.PropsMu.Lock()
    defer j.PropsMu.Unlock()
    // ...大量逻辑
}

问题：整个函数都在锁内执行，包括可能的字符串操作和信号发送。
建议：缩小锁的范围：

func (j *Job) updateDeliveryDownloadInfo(info system.JobDeliveryDownloadInfo) {
    if info.Speed == 0 && info.Proto == "" {
        return
    }
    
    speed := info.Speed
    if speed < 0 {
        speed = 0
    }
    
    proto := normalizeDownloadProto(info.Proto)
    
    j.PropsMu.Lock()
    defer j.PropsMu.Unlock()
    
    j.hasDeliveryDownloadInfo = true
    
    if speed != j.Speed {
        j.Speed = speed
        if j.service != nil {
            _ = j.emitPropChangedSpeed(speed)
        }
    }
    
    if proto != j.Proto {
        j.Proto = proto
        if j.service != nil {
            _ = j.emitPropChangedProto(proto)
        }
    }
}

4. 代码安全

4.1 输入验证

func (j *Job) updateDeliveryDownloadInfo(info system.JobDeliveryDownloadInfo) {
    // 没有验证 info.JobId 是否匹配 j.id
}

问题：没有验证传入的 JobDeliveryDownloadInfo 是否属于当前任务。
建议：添加验证：

func (j *Job) updateDeliveryDownloadInfo(info system.JobDeliveryDownloadInfo) {
    if info.JobId != j.id {
        return
    }
    // ...其余逻辑
}

4.2 错误处理

_ = j.emitPropChangedSpeed(speed)
_ = j.emitPropChangedProto(proto)

问题：忽略信号发送错误可能导致状态不同步。
建议：至少记录日志：

if err := j.emitPropChangedSpeed(speed); err != nil {
    log.Warningf("Failed to emit speed changed: %v", err)
}

4.3 竞态条件

if !j.hasDeliveryDownloadInfo {
    // ...读取和修改 j.Speed 和 j.Proto
}

问题：hasDeliveryDownloadInfo 的检查和后续操作之间可能有竞态条件。
建议：确保所有相关操作都在锁保护下进行，或使用原子操作。

5. 测试代码

5.1 测试覆盖

新增的测试用例很好，但可以增加：

1. 测试 hasDeliveryDownloadInfo 状态切换
2. 测试并发更新的安全性
3. 测试边界情况（如极大/极小的速度值）

5.2 测试辅助函数

func TestJobIgnoresEmptyDeliveryDownloadInfo(t *testing.T) {
    // ...大量设置代码
}

建议：提取测试辅助函数：

func setupTestJob(jobType string) *Job {
    job := NewJob(nil, "test-job", "test-job", nil, jobType, "", nil)
    job.DownloadSize = 1024 * 1024
    job.Status = system.RunningStatus
    job.Progress = 0.5
    job.speedMeter.SetDownloadSize(job.DownloadSize)
    past := time.Now().Add(-10 * time.Second)
    job.speedMeter.startTime = past
    job.speedMeter.updateTime = past
    return job
}

func TestJobIgnoresEmptyDeliveryDownloadInfo(t *testing.T) {
    job := setupTestJob(system.PrepareDistUpgradeJobType)
    // ...测试逻辑
}

总结建议

1. 重构 updateDeliveryDownloadInfo：拆分为更小的函数，提高可读性和可维护性
2. 优化锁粒度：减少锁内执行的操作，提高并发性能
3. 增强输入验证：确保传入数据的有效性
4. 改进错误处理：至少记录关键错误
5. 完善测试用例：增加边界条件和并发测试
6. 消除魔法值：使用命名常量提高代码可读性
7. 考虑使用枚举：对于协议类型等有限集合，使用枚举而非字符串

这些改进将使代码更健壮、更高效、更易于维护。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: qiuzhiqian, zhaohuiw42

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment