Skip to content

fix: derive P2P update support from delivery repo prefix#372

Open
zhaohuiw42 wants to merge 1 commit intodevelop/intranet-updatefrom
setP2PSupport
Open

fix: derive P2P update support from delivery repo prefix#372
zhaohuiw42 wants to merge 1 commit intodevelop/intranet-updatefrom
setP2PSupport

Conversation

@zhaohuiw42
Copy link
Copy Markdown
Contributor

Set P2PUpdateSupport based on whether the update repository uses the delivery:// prefix returned by the update platform. Preserve the server repository protocol when generating platform sources, and refresh P2P support after updating platform repository data.

Bug: https://pms.uniontech.com/bug-view-356709.html

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 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

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 15, 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.

You can retrigger this bot by commenting recheck in this Pull Request

Set P2PUpdateSupport based on whether the update repository uses the
delivery:// prefix returned by the update platform. Preserve the server
repository protocol when generating platform sources, and refresh P2P
support after updating platform repository data.

Bug: https://pms.uniontech.com/bug-view-356709.html
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

代码审查报告

总体评价

这次代码改动主要涉及将平台仓库源的生成逻辑从UpdatePlatformManager中提取为独立函数,并增加了P2P更新支持的检测逻辑。整体代码结构清晰,增加了单元测试,提高了代码的可测试性和可维护性。

详细审查

1. 语法逻辑

1.1 message_report.go 中的改动

优点:

  • 将仓库源生成逻辑从genDepositoryFromPlatform方法中提取为独立函数genPlatformReposFromRepoInfos,提高了代码的可测试性和复用性
  • 参数传递清晰,将方法依赖的配置项作为参数传递

建议:

  • genDepositoryFromPlatform方法中,文件写入操作被移到了方法主体,但错误处理仅记录了警告日志,没有返回错误。建议考虑返回错误,让调用者决定如何处理。
func (m *UpdatePlatformManager) genDepositoryFromPlatform() error {
    repos := genPlatformReposFromRepoInfos(m.repoInfos, m.config.PlatformRepoComponents, m.config.UpgradeDeliveryEnabled, m.config.IntranetUpdate)
    err := os.WriteFile(system.PlatFormSourceFile, []byte(strings.Join(repos, "\n")), 0644)
    if err != nil {
        logger.Warningf("update source list file err: %v", err)
        return err
    }
    return nil
}

1.2 updater.go 中的改动

优点:

  • 新增updateSourceSupportsP2P函数用于检测源文件是否支持P2P更新
  • 使用utils2.IsFileExist检查文件是否存在,避免不必要的读取操作

建议:

  • refreshUpgradeDeliveryService方法中,TODO注释提到"公网更新传递是否一定开启PlatformUpdate",这个问题需要进一步明确和解决
  • updateSourceSupportsP2P函数的实现简单直接,但可以考虑添加更多的错误处理

2. 代码质量

2.1 函数职责

优点:

  • 函数职责单一,genPlatformReposFromRepoInfos只负责生成仓库源列表,不涉及文件操作
  • 函数命名清晰,能够准确表达函数的功能

建议:

  • genDepositoryFromPlatform方法现在主要负责调用genPlatformReposFromRepoInfos和写入文件,可以考虑重命名为更准确的名称,如updatePlatformSourceFile

2.2 错误处理

问题:

  • 文件写入错误只记录了警告日志,没有返回错误,可能导致调用者无法感知操作失败
  • genDepositoryFromPlatform中,即使文件写入失败,也不会影响程序继续执行,可能导致后续操作基于不完整的源列表

建议:

  • 考虑将错误返回给调用者,让调用者决定如何处理
  • 如果文件写入失败,可以考虑重试或使用备用源列表

3. 代码性能

优点:

  • 使用strings.Contains检查文件内容,简单高效
  • updateSourceSupportsP2P中先检查文件是否存在,避免不必要的文件读取

建议:

  • 对于大文件,可以考虑使用缓冲读取,而不是一次性读取整个文件
  • 如果源文件较大,可以考虑使用正则表达式或更高效的字符串匹配算法

4. 代码安全

问题:

  • 文件权限设置为0644,允许所有用户读取,对于包含敏感信息的源文件可能不够安全
  • genPlatformReposFromRepoInfos中,直接使用strings.Replace替换URL前缀,没有验证URL的合法性

建议:

  • 考虑使用更严格的文件权限,如0600,限制文件访问
  • 在替换URL前缀前,验证URL的合法性,防止注入攻击
  • 对于用户提供的配置参数,如platformRepoComponents,进行验证和清理

5. 测试覆盖

优点:

  • genPlatformReposFromRepoInfosupdateSourceSupportsP2P函数添加了单元测试
  • 测试用例覆盖了多种场景,包括P2P启用/禁用、内网/外网、不同URL前缀等

建议:

  • 考虑添加边界条件测试,如空输入、特殊字符等
  • genDepositoryFromPlatform方法添加测试,验证文件写入操作
  • 考虑添加集成测试,验证整个更新流程

6. 其他建议

  1. genPlatformReposFromRepoInfos函数中,可以考虑使用常量定义前缀字符串,避免重复定义:
const (
    httpPrefix    = "http://"
    httpsPrefix   = "https://"
    deliveryPrefix = "delivery://"
)
  1. updateSourceSupportsP2P函数中,可以考虑添加文件大小限制,防止读取过大的文件:
const maxSourceFileSize = 1 << 20 // 1MB

func updateSourceSupportsP2P(sourcePath string) bool {
    if !utils2.IsFileExist(sourcePath) {
        return false
    }
    
    info, err := os.Stat(sourcePath)
    if err != nil {
        return false
    }
    if info.Size() > maxSourceFileSize {
        return false
    }
    
    data, err := os.ReadFile(sourcePath)
    return err == nil && strings.Contains(string(data), "delivery://")
}
  1. refreshUpgradeDeliveryService方法中,TODO注释提到的问题需要进一步明确和解决,建议创建一个跟踪问题并定期跟进。

总结

这次代码改动整体质量较高,主要改进了代码结构和可测试性。建议进一步完善错误处理、安全性和测试覆盖,特别是文件操作和URL处理部分。对于TODO注释中提到的问题,建议尽快明确并解决。

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