Skip to content

fix: Fix the issue of failed reporting to the platform.#336

Merged
qiuzhiqian merged 1 commit intomasterfrom
fix-platform
Mar 21, 2026
Merged

fix: Fix the issue of failed reporting to the platform.#336
qiuzhiqian merged 1 commit intomasterfrom
fix-platform

Conversation

@qiuzhiqian
Copy link

No description provided.

@github-actions
Copy link

github-actions bot commented Mar 21, 2026

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
Copy link

deepin pr auto review

这份代码变更涉及配置管理、测试代码、事件报告和升级管理等多个模块。以下是对各个部分的详细审查意见:

1. src/internal/config/config.go

变更内容:

-		variants = append(variants, dbus.MakeVariant(item))
+		variants = append(variants, dbus.MakeVariant(int64(item)))

审查意见:

  • 语法逻辑: 逻辑正确。将 int 显式转换为 int64 以满足 D-Bus 协议或后端存储的要求。
  • 代码质量: 良好。显式类型转换比依赖隐式转换更清晰,尤其是在跨语言/跨协议交互(如 D-Bus)时。
  • 代码安全: 改进。D-Bus 规范中整数类型通常对应 int32int64,Go 的 int 在 64 位系统上是 64 位,但在 32 位系统上是 32 位。显式转换为 int64 可以确保在不同架构下数据类型的一致性,避免潜在的精度丢失或类型不匹配问题。

2. src/internal/config/config_test.go

变更内容:
删除了 TestSetStartCheckRangeSavesDBusVariants 测试函数及其相关的 import (github.com/godbus/dbus/v5, ConfigManager, mock)。

审查意见:

  • 语法逻辑: 无问题。
  • 代码质量: 需要关注。删除测试通常意味着被测试的逻辑发生了重大变化,或者该测试不再适用。
    • 由于 config.go 中的实现逻辑从 MakeVariant(item) 变为了 MakeVariant(int64(item)),原有的测试逻辑 variant.Value().(int) 现在会失败(因为实际值变成了 int64)。
    • 建议: 虽然删除了旧的测试,但强烈建议补充一个新的测试用例,验证 SetStartCheckRange 现在正确地保存了 int64 类型的变体。如果删除该测试是为了简化测试结构(例如转向集成测试),请确保新的测试覆盖了此场景。如果仅仅是因为类型断言失败而删除,这是不恰当的,应该更新测试逻辑以匹配新的实现(例如将断言改为 variant.Value().(int64))。

3. src/internal/updateplatform/message_report.go

变更内容 1:

-	ExecAct      int64  `json:"execAct"`
+	ExecAt       int64  `json:"execAt"`

以及相关的逻辑修改:

-	if body.ExecAct == 0 {
-		body.ExecAct = time.Now().Unix()
+	if body.ExecAt == 0 {
+		body.ExecAt = time.Now().Unix()

审查意见:

  • 语法逻辑: 正确。这是一个字段重命名,逻辑保持不变。
  • 代码质量: 改进。ExecAt(Execute At)比 ExecAct(Execute Action 的缩写?)更能准确表达该字段存储的是"执行时间戳"这一含义,提高了代码可读性。
  • 代码安全: 涉及 JSON 序列化。由于 json tag 从 execAct 变更为 execAt,这是一个破坏性变更。如果该结构体用于与外部服务通信,或者数据存储在数据库/文件中,旧的数据将无法被正确反序列化到新字段,新字段的数据可能无法被旧版本读取。请确保:
    1. 消费此 JSON 的上游/下游服务已同步更新。
    2. 如果有持久化存储,需要考虑数据迁移方案。

变更内容 2:

-	if (m.config.PlatformDisabled & DisabledCurrentPkgLists) == 0 {
+	if (m.config.PlatformDisabled&DisabledCurrentPkgLists) == 0 && m.preBaseline != "" {

审查意见:

  • 语法逻辑: 正确。增加了一个前置条件检查。
  • 代码质量: 改进。增加了空字符串检查 m.preBaseline != "",防止在 preBaseline 未初始化或为空时执行 updateCurrentPreInstalledPkgMetaSync,这有助于避免潜在的空指针引用或无效逻辑执行。
  • 代码性能: 无影响。
  • 代码安全: 改进。增加了防御性编程,提高了程序的健壮性。

4. src/lastore-daemon/manager_download.go, manager_update.go, manager_upgrade.go

变更内容:
在多处错误处理的事件上报中,将 EventContent 从具体的错误详情 err.ErrDetail 修改为固定的字符串描述,如 "PreDownloadCheck failed"

审查意见:

  • 语法逻辑: 正确。
  • 代码质量: 有争议
    • 优点:统一了错误消息的格式,便于日志分析和监控。避免了将内部敏感的错误细节直接暴露给上层或日志系统,可能有助于信息脱敏。
    • 缺点丢失了关键的调试信息。当 PreDownloadCheck 失败时,开发人员和运维人员无法通过 EventContent 知晓具体原因(是网络问题?权限问题?还是依赖缺失?)。
  • 代码安全: 改进。如果 err.ErrDetail 包含文件路径、用户信息或其他敏感数据,将其替换为通用字符串可以防止信息泄露。
  • 改进建议:
    • 建议保留错误详情,但将其记录在 Debug 或 Info 级别的日志中,以便排查问题。
    • 或者,在 EventContent 中包含错误代码,同时将详细错误记录在日志中。
    • 如果必须脱敏,建议至少保留 err.Error() 的通用部分,或者定义一个 SanitizeError(err) 函数来提取安全的错误信息,而不是完全丢弃。

总结与建议

  1. 类型转换 (config.go):修改合理,增强了跨平台兼容性。
  2. 测试删除 (config_test.go)高风险。删除测试应谨慎,建议补充针对 int64 类型的测试用例。
  3. 字段重命名 (message_report.go):提高了可读性,但需注意 JSON 序列化的兼容性风险,确保上下游同步更新。
  4. 空值检查 (message_report.go):良好的防御性编程实践。
  5. 错误信息抹除 (manager_*.go):虽然可能出于安全或统一格式的考虑,但严重降低了可调试性。建议在日志中保留详细错误信息,或者采用更精细的错误处理策略。

@deepin-ci-robot
Copy link

[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:

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

@qiuzhiqian qiuzhiqian merged commit 7222e8a into master Mar 21, 2026
22 of 29 checks passed
@qiuzhiqian qiuzhiqian deleted the fix-platform branch March 21, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants