Skip to content

fix: Remove CheckPolicyCron related logic#316

Merged
qiuzhiqian merged 1 commit intomasterfrom
fix_review
Feb 6, 2026
Merged

fix: Remove CheckPolicyCron related logic#316
qiuzhiqian merged 1 commit intomasterfrom
fix_review

Conversation

@qiuzhiqian
Copy link
Copy Markdown
Contributor

No description provided.

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: qiuzhiqian

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

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 6, 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
Copy Markdown

deepin pr auto review

这段代码 diff 主要展示了从配置系统中移除 CheckPolicyCron(对应配置键 check-policy-on-calendar)的相关代码,转而使用 CheckPolicyInterval。这是一个重构性质的变更。

以下是对该变更的审查意见,涵盖语法逻辑、代码质量、性能和安全方面:

1. 语法逻辑与代码质量

  • 清理彻底性(优点)

    • 代码变更在 src/internal/config/config.go 中不仅移除了 Config 结构体中的 CheckPolicyCron 字段,也同步移除了常量 dSettingsKeyCheckPolicyOnCalendar 以及 getConfigFromDSettings 函数中读取该配置的逻辑。
    • 同时,在 usr/share/dsg/configs/org.deepin.dde.lastore/org.deepin.dde.lastore.json 中也删除了对应的配置项定义。
    • 评价:这种“删库跑路”式的清理非常彻底,避免了遗留的无用代码,有助于保持代码库的整洁,逻辑上是一致的。
  • 数据类型转换(潜在风险点)

    • getConfigFromDSettings 中,原代码使用了 c.CheckPolicyCron = v.Value().(string)。虽然这段代码被删除了,但需要确认新的 CheckPolicyInterval(int 类型)是如何读取和转换的。通常 dsettings/gsettings 存储的是字符串或变体,读取 int 值时通常需要类型断言或转换(如 int(v.Value().(float64))strconv.Atoi)。
    • 建议:请确认 CheckPolicyInterval 的读取逻辑已经正确处理了类型转换,避免因类型不匹配导致运行时 panic。

2. 代码性能

  • 性能影响
    • 本次变更主要是删除代码,移除了对 dSettingsKeyCheckPolicyOnCalendar 的读取操作。这实际上略微减少了初始化配置时的系统调用(DBus 调用或文件读取)和内存占用。
    • 评价:对性能有微小的正面影响,可以忽略不计。

3. 代码安全

  • 向后兼容性(重点关注)
    • 风险:如果系统中有已经部署的旧版本配置文件(org.deepin.dde.lastore.json 或用户自定义的 dsettings 覆盖文件),其中包含 check-policy-on-calendar 的配置值。
    • 影响:新代码启动时会忽略这个旧配置键。如果业务逻辑强依赖于这个时间间隔(例如 systemd timer 的调度),仅仅删除读取逻辑而不做迁移,可能会导致旧配置失效,从而改变系统的更新检查行为(例如从原来的“每天特定时间”变成了“固定间隔”),这可能不符合用户预期。
    • 建议
      1. 数据迁移:在 getConfigFromDSettings 或类似的初始化逻辑中,检查是否存在旧的 check-policy-on-calendar 配置。如果存在且 CheckPolicyInterval 为默认值,尝试解析 Cron 表达式并转换为秒数赋值给 CheckPolicyInterval,或者至少记录一条警告日志提示用户配置已失效。
      2. 版本控制:确保配置文件(JSON schema)的版本号已更新,以便管理工具知道配置结构发生了变化。

4. 改进建议

  1. 确认 CheckPolicyInterval 的实现

    • 确保新增的 CheckPolicyInterval 字段在 getConfigFromDSettings 中有对应的读取逻辑(虽然 diff 没显示出来,但必须存在)。
    • 示例代码(仅供参考):
      v, err = c.dsLastoreManager.Value(0, dSettingsKeyCheckPolicyInterval) // 假设有这个常量
      if err != nil {
          logger.Warning(err)
      } else {
          // 注意类型断言的安全性
          if val, ok := v.Value().(float64); ok { // JSON数字通常解析为float64
              c.CheckPolicyInterval = int(val)
          }
      }
  2. 文档更新

    • 由于删除了 CheckPolicyCron(Cron 表达式)并引入了 CheckPolicyInterval(秒数),配置方式发生了根本变化。请务必更新相关的 API 文档、部署文档或用户手册,说明不再支持 Cron 表达式,改为秒数间隔。
  3. 逻辑一致性检查

    • 原注释提到 CheckPolicyCron 是 "systemd定时器里的OnCalendar参数"。这意味着底层可能使用了 systemd timer。
    • 如果底层机制依然是 systemd timer,仅仅将配置改为 int (秒) 可能不够,因为 systemd timer 更擅长处理日历时间(OnCalendar)而不是单调递增的间隔(OnUnitActiveSec)。请确认后端逻辑是否已经从使用 OnCalendar 切换到了使用 OnUnitActiveSec,或者是否在 Go 层面实现了定时器逻辑。

总结

该 diff 逻辑清晰,代码清理彻底。主要风险在于配置格式的变更可能导致旧配置失效以及底层调度机制(Cron vs Interval)的适配问题。建议重点检查向后兼容性以及后端定时任务的实现方式。

@qiuzhiqian qiuzhiqian merged commit ac64abd into master Feb 6, 2026
22 of 28 checks passed
@qiuzhiqian qiuzhiqian deleted the fix_review branch February 6, 2026 05:40
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