Skip to content

feat: Refactor download size query logic and optimize display#330

Open
electricface wants to merge 1 commit intolinuxdeepin:masterfrom
electricface:swt/dev-download-size
Open

feat: Refactor download size query logic and optimize display#330
electricface wants to merge 1 commit intolinuxdeepin:masterfrom
electricface:swt/dev-download-size

Conversation

@electricface
Copy link
Member

@electricface electricface commented Mar 13, 2026

- Remove IsIncrementalUpdateCached function, use IncrementalUpdate
flag to select backend
- Implement queryDownloadSizeViaApt: Fetch download size via apt-get
dist-upgrade command
- Implement queryDownloadSizeForIncrementalUpdate: Fetch download size via
deepin-immutable-ctl interface
- QuerySourceDownloadSize automatically selects the method based
on the IncrementalUpdate flag
- Load IncrementalUpdate flag from config on startup and apply to
system modules
- Use FormatSize function for log output to standardize display
format
- Remove redundant checks from PackagesDownloadSize and
updateModeStatusBySize

feat: 重构下载大小查询逻辑并优化显示

  • 移除 IsIncrementalUpdateCached 函数,改用 IncrementalUpdate 标志选择查询后端
  • 实现 queryDownloadSizeViaApt: 基于 apt-get dist-upgrade 命令获取下载大小
  • 实现 queryDownloadSizeForIncrementalUpdate: 基于 deepin-immutable-ctl 接口获取下载大小
  • QuerySourceDownloadSize 根据 IncrementalUpdate 标志自动选择查询方法
  • 启动时从配置加载 IncrementalUpdate 标志并应用到系统模块
  • 日志输出使用 FormatSize 函数格式化,统一显示格式
  • 移除 PackagesDownloadSize 和 updateModeStatusBySize 中的冗余检查

@deepin-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: electricface

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

@electricface electricface force-pushed the swt/dev-download-size branch 3 times, most recently from fce958c to b49f0f6 Compare March 14, 2026 07:23
@deepin-bot
Copy link
Contributor

deepin-bot bot commented Mar 14, 2026

TAG Bot

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

@electricface electricface force-pushed the swt/dev-download-size branch from b49f0f6 to 660fe12 Compare March 14, 2026 09:45
@electricface electricface changed the title feat(system): 支持增量更新及下载大小统计 feat: Refactor download size query logic and optimize display Mar 14, 2026
@electricface electricface force-pushed the swt/dev-download-size branch 2 times, most recently from e7f1894 to 136d378 Compare March 16, 2026 05:16
- Remove IsIncrementalUpdateCached function, use IncrementalUpdate
flag to select backend
- Implement queryDownloadSizeViaApt: Fetch download size via apt-get
dist-upgrade command
- Implement queryDownloadSizeForIncrementalUpdate: Fetch download size via
deepin-immutable-ctl interface
- QuerySourceDownloadSize automatically selects the method based
on the IncrementalUpdate flag
- Load IncrementalUpdate flag from config on startup and apply to
system modules
- Use FormatSize function for log output to standardize display
format
- Remove redundant checks from PackagesDownloadSize and
updateModeStatusBySize

Task: https://pms.uniontech.com/task-view-384185.html
Change-Id: I427ccdd0a8d1151e48e2c305ff3008d5ba4cb25e
@electricface electricface force-pushed the swt/dev-download-size branch from 136d378 to c337e7d Compare March 17, 2026 09:27
@electricface electricface marked this pull request as ready for review March 18, 2026 08:11
@deepin-ci-robot
Copy link

deepin pr auto review

Git Diff 代码审查报告

总体概述

该代码变更主要涉及增量更新功能的改进,主要包括:

  1. 将增量更新检查逻辑从 apt.go 移至 system_apt.go
  2. 新增了对 deepin-immutable-ctl 工具的支持,通过 JSON 解析获取更新大小信息
  3. 添加了字节大小格式化工具函数 FormatSize
  4. 改进了日志输出,使用格式化后的字节大小

详细审查

1. debian/rules

-	dh $@
+	dh $@ --buildsystem=makefile

审查意见

  • 明确指定构建系统为 makefile 是好的实践,可以避免 dh 自动检测可能带来的不确定性
  • 无安全问题

2. src/internal/system/apt/apt.go

删除了 IsIncrementalUpdateCached 函数,该功能被移至 system_apt.go

审查意见

  • 代码组织更合理,将系统相关的功能集中在 system_apt.go
  • 删除了未使用的 strconv 导入,代码更整洁

3. src/internal/system/system.go

var IncrementalUpdate bool

审查意见

  • 新增全局变量 IncrementalUpdate,在 main.go 中初始化
  • 建议添加注释说明该变量的用途和初始化时机
  • 考虑使用配置管理器统一管理此类全局变量,避免分散在多个文件中

4. src/internal/system/system_apt.go

新增函数:queryDownloadSizeViaApt

审查意见

  • 代码逻辑清晰,将原有逻辑提取为独立函数
  • 使用 #nosec G204 注释标记已知的命令注入风险,但实际参数来源可控
  • 建议添加函数注释说明参数和返回值的含义

新增函数:queryDownloadSizeForIncrementalUpdate

审查意见

  • 使用 JSON 解析替代了原来的字符串解析,更健壮
  • 错误处理完善,包括命令执行失败和 JSON 解析失败的情况
  • 特殊情况处理得当:当 needDownloadtotalSize 都为 0 但有缓存包时,返回非零值防止 UI 卡住
  • 建议添加对 DeepinImmutableCtlPath 常量的定义和使用

修改函数:QuerySourceDownloadSize

审查意见

  • 根据配置选择不同的查询函数,代码结构清晰
  • 添加了更详细的日志输出,便于调试
  • 建议在函数注释中说明两种查询方式的区别和适用场景

5. src/internal/utils/utils.go

新增函数:FormatSize

审查意见

  • 实现了字节大小的格式化,使用二进制单位(KiB, MiB 等)
  • 边界条件处理正确,包括负数返回 "unknown"
  • 代码简洁高效

改进建议

  • 可以考虑添加对超大数值(超过 YiB)的处理
  • 可以考虑添加对零值的特殊处理,如 "0 B" 而不是 "0.00 B"

6. src/internal/utils/utils_test.go

新增测试:TestFormatSize

审查意见

  • 测试用例全面,覆盖了各种边界条件
  • 包括了负数处理测试
  • 测试数据合理,验证了不同单位间的转换

改进建议

  • 可以添加对超大数值(TiB 以上)的测试用例
  • 可以添加对浮点精度边界的测试

7. src/lastore-daemon/main.go

logger.Info("incremental update:", config.IncrementalUpdate)
system.IncrementalUpdate = config.IncrementalUpdate

审查意见

  • 初始化逻辑清晰,与 IntranetUpdate 保持一致
  • 建议添加对配置文件中 IncrementalUpdate 字段的验证

8. src/lastore-daemon/manager_ifc.go

-	if m.config.IncrementalUpdate && size > 0 && apt.IsIncrementalUpdateCached("") {
-		size = 0.0
-	}

审查意见

  • 删除了旧的增量更新检查逻辑,现在统一在 system_apt.go 中处理
  • 改进了日志输出,使用格式化后的字节大小

9. src/lastore-daemon/update_status.go

-				sourceList, ok := system.GetCategorySourceMap()[typ]
-				sourceArgs := ""
-				if ok && sourceList != "" {
-					if utils.IsDir(sourceList) {
-						sourceArgs = "-o Dir::Etc::sourcelist=/dev/null -o Dir::Etc::SourceParts=" + sourceList
-					} else {
-						sourceArgs = "-o Dir::Etc::sourcelist=" + sourceList + " -o Dir::Etc::SourceParts=/dev/null"
-					}
-				}
-				if m.lsConfig.IncrementalUpdate && needDownloadSize > 0 && apt.IsIncrementalUpdateCached(sourceArgs) {
-					needDownloadSize = 0.0
-				}

审查意见

  • 删除了冗余的增量更新检查逻辑,现在统一在 system_apt.go 中处理
  • 改进了日志输出,使用格式化后的字节大小
  • 代码更简洁,逻辑更集中

安全性评估

  1. 命令注入风险

    • queryDownloadSizeViaAptqueryDownloadSizeForIncrementalUpdate 中使用 exec.Command 构建命令
    • 使用 #nosec G204 注释标记了已知风险
    • 参数来源可控,风险较低
  2. JSON 解析风险

    • queryDownloadSizeForIncrementalUpdate 中解析 JSON 输出
    • 有完善的错误处理,风险较低

性能评估

  1. 命令执行

    • 新增的 queryDownloadSizeForIncrementalUpdate 函数需要执行外部命令
    • 建议考虑缓存结果,避免频繁调用
  2. 字符串处理

    • 新增的 FormatSize 函数性能良好,循环次数有限

总结与建议

  1. 代码质量

    • 代码结构更清晰,功能划分更合理
    • 错误处理完善
    • 日志输出更详细
  2. 改进建议

    • 添加更多函数注释,特别是新增的函数
    • 考虑添加对 DeepinImmutableCtlPath 常量的定义
    • 考虑缓存 queryDownloadSizeForIncrementalUpdate 的结果
    • 统一配置管理,避免全局变量分散
  3. 测试建议

    • 添加对 queryDownloadSizeForIncrementalUpdate 的单元测试
    • 添加对 QuerySourceDownloadSize 的集成测试

总体而言,这是一次高质量的代码变更,改进了增量更新功能的实现,提高了代码的可维护性和可读性。

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Mar 21, 2026

TAG Bot

New tag: 6.2.51
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #337

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.

2 participants