fix: reset intranet update settings after unregister

[zhaohuiw42]

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

[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

[deepin-ci-robot]

deepin pr auto review

Git Diff 代码审查报告

总体评价

这段代码主要进行了以下修改：

1. 将多个配置键常量从小写(dSettingsKey*)改为大写(DSettingsKey*)以符合导出规范
2. 重构了速度限制配置的重置逻辑，提取为独立函数
3. 添加了单元测试以验证新功能
4. 调整了IPFS配置更新的调用顺序

详细审查

1. 语法逻辑

优点：

• 常量命名从dSettingsKey*改为DSettingsKey*符合Go语言导出标识符的命名规范
• 新增的resetIntranetUpdateSettingsAfterUnregister()函数逻辑清晰，将相关操作集中在一起
• 测试用例覆盖了新增的功能

问题与建议：

1. 在message_report.go中，setIPFSRateLimit变量被定义为包级变量：

   var setIPFSRateLimit = ratelimit.SetIPFSRateLimit

   这种做法虽然便于测试，但会降低代码的可读性和维护性。建议：

   • 考虑使用接口而不是直接依赖具体实现
   • 如果必须使用这种方式，应添加注释说明其用途

2. 在resetSpeedLimitConfigToDefaults函数中，所有配置都被硬编码为相同的默认值：

   c.DeliveryRemoteDownloadGlobalLimit = defaultDeliveryRateLimitConfig
   c.DeliveryRemoteUploadGlobalLimit = defaultDeliveryRateLimitConfig
   // ... 多个类似赋值

   建议考虑这些配置是否真的应该全部使用相同的默认值，或者是否需要不同的默认值。

2. 代码质量

优点：

• 代码结构更加清晰，相关操作被封装到独立函数中
• 添加了全面的单元测试，提高了代码的可测试性
• 使用了常量来定义默认配置，便于维护

问题与建议：

1. 在tryToUnRegisterConsole函数中，原来的TODO注释被移除，但新的实现可能没有完全解决TODO中提到的问题：

   // TODO: 此处只是重置dsettings，合理的做法是恢复到上一次公网的情况

   建议确认新实现是否真正解决了这个问题，或者添加新的TODO说明后续工作。

2. 在resetIntranetUpdateSettingsAfterUnregister函数中，错误处理只是记录警告：

   if err := m.config.SetLastCheckTime(time.Unix(0, 0)); err != nil {
       logger.Warningf("failed to set last check time: %v", err)
   }

   考虑这些操作的重要性，建议评估是否需要更严格的错误处理。

3. 测试代码中的setIPFSRateLimit替换模式可以进一步封装为测试辅助函数，提高代码复用性。

3. 代码性能

优点：

• 没有明显的性能问题
• 重构后的代码没有引入额外的性能开销

建议：

1. 在resetIntranetUpdateSettingsAfterUnregister函数中，循环调用ResetDSettings可能会产生多次系统调用：

   for _, key := range [...] {
       if err := m.config.ResetDSettings(key); err != nil {
           logger.Warningf("failed to reset %s: %v", key, err)
       }
   }

   如果ResetDSettings操作比较耗时，考虑是否可以批量重置多个设置。

4. 代码安全

优点：

• 添加了nil检查：

  if m == nil || m.config == nil {
      return
  }

问题与建议：

1. 在UpdateDeliverySpeedLimit函数中，对IPFSConfig的检查：

   if m.IPFSConfig.UploadLimit == nil || m.IPFSConfig.DownloadLimit == nil {
       return nil
   }

   建议添加日志记录，说明为什么在这种情况下直接返回，便于调试。

2. 在resetSpeedLimitConfigToDefaults函数中，直接修改配置对象：

   c.UpgradeDeliveryEnabled = true
   c.DownloadSpeedLimitConfig = defaultDownloadSpeedLimitConfig
   // ...

   确保这些操作是线程安全的，如果配置可能在多个goroutine中访问。

3. 在测试代码中，直接修改包级变量setIPFSRateLimit：

   oldSetIPFSRateLimit := setIPFSRateLimit
   setIPFSRateLimit = func(...) { ... }
   defer func() {
       setIPFSRateLimit = oldSetIPFSRateLimit
   }()

   这种方式在并发测试中可能导致竞态条件，建议考虑使用更安全的测试隔离方式。

总结

这段代码总体质量良好，主要改进了配置键的命名规范和速度限制配置的重置逻辑。主要建议集中在：

1. 考虑使用接口而不是直接依赖具体实现
2. 评估错误处理策略是否足够严格
3. 确保配置修改的线程安全性
4. 考虑测试代码的并发安全性

这些改进将有助于提高代码的可维护性、可靠性和安全性。