fix: enable upgrade delivery by default

[zhaohuiw42]

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

_{You can retrigger this bot by commenting recheck in this Pull Request}

[deepin-ci-robot]

deepin pr auto review

代码审查意见

总体评价

这段代码主要实现了 P2P 交付服务(Upgrade Delivery Service)与平台仓库的集成，增加了对平台仓库源的处理逻辑，包括仓库源的匹配、替换和交付服务的启用控制。代码整体结构清晰，测试覆盖较好，但存在一些可以改进的地方。

1. 语法逻辑

1.1 replaceRepoSchemeWithDelivery 函数逻辑

问题：在处理 delivery:// 前缀时直接返回原行，但可能存在行中既有 delivery:// 又有其他 URL 的情况。

case strings.HasPrefix(fields[i], "delivery://"):
    return line

建议：考虑是否需要检查整个行是否已经被处理过，或者是否需要处理行中多个 URL 的情况。

1.2 shouldEnableUpgradeDeliveryService 函数逻辑

问题：函数逻辑中，当 UpgradeDeliveryEnabled 为 true 时直接返回 true，忽略了其他条件。

if cfg.UpgradeDeliveryEnabled {
    return true
}

建议：考虑是否需要添加额外的条件检查，比如在内网环境下是否需要满足其他条件。

2. 代码质量

2.1 错误处理

问题：在 UpdateP2pDefaultSourceDir 函数中，文件写入失败后只是记录警告，没有返回错误。

_, err = p2pSource.Write([]byte(newContent))
if err != nil {
    logger.Warningf("Error writing file: %v\n", err)
}

建议：考虑将错误返回给调用者，以便上层可以处理错误情况。

2.2 变量命名

问题：replaceMatchedReposWithDelivery 函数中的 matchedRepoSet 变量名可以更清晰。

matchedRepoSet := make(map[string]struct{})

建议：可以改为 platformRepoSet 或 targetRepoSet 以更明确表示其用途。

2.3 代码重复

问题：在 replaceRepoSchemeWithDelivery 函数中，处理 http 和 https 的代码重复。

case strings.HasPrefix(fields[i], "https://"):
    fields[i] = "delivery://" + strings.TrimPrefix(fields[i], "https://")
    return strings.Join(fields, " ")
case strings.HasPrefix(fields[i], "http://"):
    fields[i] = "delivery://" + strings.TrimPrefix(fields[i], "http://")
    return strings.Join(fields, " ")

建议：可以提取公共逻辑：

func replaceRepoSchemeWithDelivery(line string) string {
    fields := strings.Fields(line)
    if len(fields) < 2 || fields[0] != "deb" {
        return line
    }
    for i := 1; i < len(fields); i++ {
        if strings.HasPrefix(fields[i], "[") {
            continue
        }
        if strings.HasPrefix(fields[i], "delivery://") {
            return line
        }
        for _, prefix := range []string{"https://", "http://"} {
            if strings.HasPrefix(fields[i], prefix) {
                fields[i] = "delivery://" + strings.TrimPrefix(fields[i], prefix)
                return strings.Join(fields, " ")
            }
        }
    }
    return line
}

3. 代码性能

3.1 字符串操作

问题：在 replaceMatchedReposWithDelivery 函数中，对每一行都进行 strings.TrimSpace 和 map 查找。

for _, line := range strings.Split(content, "\n") {
    if _, ok := matchedRepoSet[strings.TrimSpace(line)]; ok {
        lines = append(lines, replaceRepoSchemeWithDelivery(line))
        continue
    }
    lines = append(lines, line)
}

建议：如果平台仓库列表较大，可以考虑使用更高效的数据结构或算法，如 Trie 树或正则表达式预编译。

3.2 内存分配

问题：replaceMatchedReposWithDelivery 函数中，lines 切片可能多次重新分配内存。

var lines []string
for _, line := range strings.Split(content, "\n") {
    // ...
    lines = append(lines, line)
}

建议：可以预先分配足够大的切片：

lines := make([]string, 0, len(strings.Split(content, "\n")))

4. 代码安全

4.1 输入验证

问题：在 GetPlatformRepoSources 函数中，没有对 repo.Uri 和 repo.CodeName 进行验证。

repos = append(repos, fmt.Sprintf("deb %s %s %s", repo.Uri, repo.CodeName, suffix))

建议：添加输入验证，防止注入攻击或格式错误的仓库源。

4.2 文件操作

问题：在 UpdateP2pDefaultSourceDir 函数中，文件写入操作没有使用临时文件和原子重命名。

_, err = p2pSource.Write([]byte(newContent))
if err != nil {
    logger.Warningf("Error writing file: %v\n", err)
}

建议：使用临时文件写入，然后原子重命名：

tmpFile := p2pSource.Name() + ".tmp"
err := os.WriteFile(tmpFile, []byte(newContent), 0644)
if err != nil {
    logger.Warningf("Error writing temp file: %v\n", err)
    return
}
err = os.Rename(tmpFile, p2pSource.Name())
if err != nil {
    logger.Warningf("Error renaming file: %v\n", err)
    return
}

5. 测试覆盖

5.1 边界条件

问题：测试用例没有覆盖所有边界条件，如空字符串、特殊字符等。
建议：添加更多测试用例，特别是：

1. 空仓库列表
2. 包含特殊字符的仓库源
3. 多个 URL 在同一行的情况
4. 不同格式的仓库源（带选项、注释等）

5.2 并发测试

问题：没有测试并发访问的情况。
建议：如果这些函数可能被并发调用，添加并发测试用例。

6. 其他建议

1. 文档注释：为新添加的函数添加详细的文档注释，说明其用途、参数和返回值。

2. 日志级别：考虑使用更合适的日志级别，如某些警告可以改为 debug 或 info。

3. 配置默认值：在配置文件中，将 upgrade-delivery-enabled 的默认值改为 true，需要确保这是预期的行为，并考虑向后兼容性。

4. 错误恢复：在文件写入失败时，考虑是否需要恢复原始内容或保留备份。

5. 性能监控：对于可能被频繁调用的函数，如 replaceMatchedReposWithDelivery，考虑添加性能监控点。

总结

这段代码实现了所需的功能，但在错误处理、性能优化和安全性方面还有一些改进空间。建议在合并前进行更全面的测试，特别是边界条件和并发场景。同时，考虑添加更多的文档注释和日志，以便于后续维护。

[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