fix: Adjust the conditions for reporting environment check results.

[qiuzhiqian]

Task: https://pms.uniontech.com/task-view-387367.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-bot]

TAG Bot

New tag: 6.2.50
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #331

[deepin-ci-robot]

deepin pr auto review

这份代码变更主要涉及系统更新平台的事件上报逻辑，增加了多个检查点的事件类型，并优化了事件上报的代码结构。以下是对代码的详细审查和改进建议：

1. 语法逻辑

优点：

• 代码结构清晰，新增的检查点事件类型定义明确
• 统一了事件上报的方式，使用 PostProcessEventMessage 方法
• 在 PostProcessEventMessage 方法中增加了 ExecAct 字段的时间戳自动填充逻辑

问题和改进建议：

1. ExecAct 字段初始化逻辑

   if body.ExecAct == 0 {
       body.ExecAct = time.Now().Unix()
   }

   • 问题：使用 time.Now().Unix() 生成时间戳可能存在精度问题，且在不同时区环境下可能表现不一致
   • 建议：使用 time.Now().UnixNano() 获取更高精度的时间戳，并考虑使用 UTC 时间

   if body.ExecAct == 0 {
       body.ExecAct = time.Now().UTC().UnixNano() / 1e6 // 毫秒级时间戳
   }

2. 事件类型常量定义

   const (
       // ...原有常量
       PreUpdateCheck    int = 101
       PostUpdateCheck   int = 102
       // ...
   )

   • 问题：新增常量与原有常量之间没有明确的分隔和注释
   • 建议：添加分组注释，提高可读性

   const (
       // 基础事件类型 (1-100)
       StartDownload     int = 1
       // ...
       
       // 检查点事件类型 (101-200)
       PreUpdateCheck    int = 101
       PostUpdateCheck   int = 102
       // ...
   )

2. 代码质量

优点：

• 消除了重复代码，统一了事件上报方式
• 使用 go 关键字进行异步上报，避免阻塞主流程

问题和改进建议：

1. 错误处理不一致

   if systemErr := dut.CheckSystem(checkType, nil); systemErr != nil {
       logger.Warning(systemErr)
       go func(err *system.JobError) {
           // 上报错误
       }(systemErr)
   } else {
       go m.updatePlatform.PostProcessEventMessage(...)
   }

   • 问题：成功和失败分支都使用了 go 关键字，但错误处理中还有额外的日志记录
   • 建议：统一错误处理逻辑，可以考虑封装一个辅助函数

   func (m *Manager) reportCheckResult(checkType int, err error) {
       if err != nil {
           logger.Warning(err)
           go m.updatePlatform.PostProcessEventMessage(updateplatform.ProcessEvent{
               TaskID:       1,
               EventType:    checkType,
               EventStatus:  false,
               EventContent: err.(*system.JobError).ErrDetail,
           })
       } else {
           go m.updatePlatform.PostProcessEventMessage(updateplatform.ProcessEvent{
               TaskID:       1,
               EventType:    checkType,
               EventStatus:  true,
               EventContent: fmt.Sprintf("%v success", checkType),
           })
       }
   }

2. 魔法数字

   TaskID: 1,

   • 问题：TaskID 硬编码为 1，缺乏灵活性
   • 建议：定义常量或从配置中获取

   const (
       DefaultTaskID = 1
   )
   // 使用时
   TaskID: m.taskID, // 如果有实例变量
   // 或
   TaskID: DefaultTaskID,

3. 代码性能

问题和改进建议：

1. 频繁创建 goroutine

   • 问题：每个事件上报都创建一个新的 goroutine，在高并发场景下可能导致资源浪费
   • 建议：使用工作池模式管理 goroutine

   var eventWorkerPool = make(chan struct{}, 10) // 限制并发数为10
   
   func (m *Manager) postEventAsync(event updateplatform.ProcessEvent) {
       eventWorkerPool <- struct{}{} // 获取令牌
       go func() {
           defer func() { <-eventWorkerPool }() // 释放令牌
           m.updatePlatform.PostProcessEventMessage(event)
       }()
   }

2. 字符串格式化

   EventContent: fmt.Sprintf("%v success", checkType),

   • 问题：频繁使用 fmt.Sprintf 可能影响性能
   • 建议：对于简单的字符串拼接，可以使用 + 操作符或 strings.Builder

   EventContent: strconv.Itoa(checkType) + " success",

4. 代码安全

问题和改进建议：

1. 敏感信息泄露

   EventContent: err.Error(),

   • 问题：直接将错误信息上报可能泄露系统敏感信息
   • 建议：对错误信息进行过滤或脱敏

   func sanitizeError(err error) string {
       if err == nil {
           return ""
       }
       // 这里可以添加过滤逻辑
       return strings.ReplaceAll(err.Error(), "/path/to/sensitive", "***")
   }

2. 并发安全

   • 问题：ProcessEvent 结构体中的字段在并发场景下可能存在竞态条件
   • 建议：如果需要并发访问，考虑添加互斥锁或使用原子操作

   type ProcessEvent struct {
       // ...
       ExecAct int64 `json:"execAct"`
       // ...
   }
   
   func (e *ProcessEvent) SetExecAct(t int64) {
       atomic.StoreInt64(&e.ExecAct, t)
   }
   
   func (e *ProcessEvent) GetExecAct() int64 {
       return atomic.LoadInt64(&e.ExecAct)
   }

3. 输入验证

   • 问题：PostProcessEventMessage 方法没有对输入参数进行验证
   • 建议：添加参数验证逻辑

   func (m *UpdatePlatformManager) PostProcessEventMessage(body ProcessEvent) error {
       if body.EventType <= 0 {
           return errors.New("invalid event type")
       }
       if body.TaskID <= 0 {
           return errors.New("invalid task ID")
       }
       // 其他验证...
       
       // 原有逻辑...
       return nil
   }

5. 其他改进建议

1. 文档完善

   • 为新增的事件类型添加详细注释，说明每个事件的触发时机和含义
   • 为 ProcessEvent 结构体和 PostProcessEventMessage 方法添加文档注释

2. 测试覆盖

   • 为新增的事件上报逻辑添加单元测试
   • 测试各种边界情况，如错误处理、并发场景等

3. 日志记录

   • 在关键步骤添加更详细的日志记录，便于问题排查

   logger.Debugf("Posting process event: %+v", body)

4. 配置化

   • 将事件上报的 URL、超时时间等参数配置化，提高灵活性

总结来说，这次代码变更在整体结构上是合理的，主要改进了事件上报的统一性和完整性。但在细节处理上，如错误处理、并发安全、性能优化等方面还有改进空间。建议逐步实施上述改进建议，以提高代码的健壮性和可维护性。

[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