fix: reset DownloadLimitOnChanging when preparing dist upgrade

[qiuzhiqian]

Bug: https://pms.uniontech.com/bug-view-359121.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.

xml seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You can retrigger this bot by commenting recheck in this Pull Request}

[deepin-ci-robot]

deepin pr auto review

这段代码的修改主要是为了在系统更新过程中添加对 DownloadLimitOnChanging 属性的日志记录和状态重置功能。下面是对这段 diff 的详细审查和改进建议：

1. 语法与逻辑审查

语法正确性：

• 代码符合 Go 语言语法规范，没有明显的语法错误。
• logger.Infof 和 setAfterHooks 的使用方式正确。

逻辑正确性：

• 在 setPropDownloadLimitOnChanging 中添加日志是合理的，有助于调试和追踪属性变化。
• 在 prepareDistUpgrade 中添加 setAfterHooks，在任务状态变为 RunningStatus 时重置 DownloadLimitOnChanging，逻辑上是合理的，因为一旦任务开始运行，可能需要重置之前的下载限制状态。

潜在逻辑问题：

• 在 setAfterHooks 中直接修改 m.jobManager.DownloadLimitOnChanging 和调用 m.setPropDownloadLimitOnChanging(false) 可能存在冗余。如果 setPropDownloadLimitOnChanging 内部已经负责了属性值的修改和信号发送，那么直接修改 m.jobManager.DownloadLimitOnChanging 可能是不必要的，或者可能导致双重修改。

2. 代码质量

可读性：

• 代码结构清晰，注释说明了重置 DownloadLimitOnChanging 的目的。
• 日志格式清晰，能够明确展示属性的前后变化。

一致性：

• 建议检查项目中其他类似的属性设置逻辑，确保日志记录和状态修改的方式保持一致。

3. 代码性能

• 添加日志和属性重置操作对性能的影响微乎其微，可以忽略不计。
• 如果 setPropDownloadLimitOnChanging 内部已经处理了属性值的修改，那么直接修改 m.jobManager.DownloadLimitOnChanging 是多余的，虽然不会造成性能问题，但会增加代码维护成本。

4. 代码安全

• 直接修改 m.jobManager.DownloadLimitOnChanging 可能绕过了某些内部检查或信号发送机制，建议确保属性修改的统一性。
• 日志中记录了属性值的变化，但未对敏感信息进行过滤。虽然 DownloadLimitOnChanging 是布尔值，不涉及敏感信息，但建议在未来的日志记录中注意敏感信息的保护。

改进建议

1. 避免双重修改：

   • 如果 setPropDownloadLimitOnChanging 内部已经修改了 DownloadLimitOnChanging 的值，那么在 setAfterHooks 中直接修改 m.jobManager.DownloadLimitOnChanging 是不必要的。建议只调用 m.setPropDownloadLimitOnChanging(false)。

   改进后的代码：

   j.setAfterHooks(map[string]func() error{
       string(system.RunningStatus): func() error {
           // reset DownloadLimitOnChanging
           m.setPropDownloadLimitOnChanging(false)
           return nil
       },
   })

2. 日志级别：

   • 如果属性变化是高频操作，建议评估是否需要 Info 级别的日志，或者是否可以降低为 Debug 级别以避免日志过多。

3. 错误处理：

   • 虽然 setPropDownloadLimitOnChanging 返回错误，但在 setAfterHooks 中未处理错误。建议检查是否需要处理错误。

   改进后的代码：

   j.setAfterHooks(map[string]func() error{
       string(system.RunningStatus): func() error {
           // reset DownloadLimitOnChanging
           if err := m.setPropDownloadLimitOnChanging(false); err != nil {
               logger.Warningf("Failed to reset DownloadLimitOnChanging: %v", err)
               return err
           }
           return nil
       },
   })

4. 常量使用：

   • 如果 RunningStatus 是一个常量，建议直接使用常量而不是通过 string() 转换，除非 setAfterHooks 的接口明确要求字符串键。

总结

这段代码的修改在语法和逻辑上是正确的，但可以通过避免双重修改、优化日志级别、增强错误处理和改进常量使用来进一步提升代码质量和安全性。

[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