Merge to upstream master

[ut005973]

No description provided.

[xionglinlin]

Code review

Found 1 issue:

1. Memory leak in UpdateAssistant destructor - d_ptr is allocated with new in the constructor but never deleted. The destructor is empty, causing a memory leak every time an UpdateAssistant object is destroyed.

deepin-update-ui/src/common/dbus/updateassistant.cpp

Lines 8 to 15 in a760b17

	, d_ptr(new UpdateAssistantPrivate)
	{
	QDBusConnection::systemBus().connect("org.deepin.upgradedelivery", "/org/deepin/upgradedelivery", "org.freedesktop.DBus.Properties", "PropertiesChanged", this, SLOT(onPropertyChanged(QString, QVariantMap, QStringList)));
	}
	UpdateAssistant::~UpdateAssistant()
	{
	}

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[deepin-ci-robot]

deepin pr auto review

代码审查报告

总体评价

这次代码变更主要涉及系统更新功能的增强，包括：

1. 新增私有化更新支持（private-lastore-tray）
2. 更新传递优化功能（Delivery Optimization）
3. 更新UI和交互逻辑的改进
4. 多语言支持更新

整体代码结构清晰，功能划分合理，但存在一些可以改进的地方。

详细审查

1. 语法和逻辑问题

1.1 updateassistant.cpp 中的内存管理

UpdateAssistant::~UpdateAssistant()
{
    delete d_ptr;
    d_ptr = nullptr;  // 这行是多余的，因为对象即将被销毁
}

建议：删除 d_ptr = nullptr; 这行代码，因为对象即将被销毁，置空没有意义。

1.2 updateassistant.cpp 中的逻辑问题

void UpdateAssistant::onPendingCallFinished(QDBusPendingCallWatcher *w)
{
    if (!w)
        return;
    w->deleteLater();
    const auto callName = d_ptr->m_processingCalls.key(w);
    Q_ASSERT(!callName.isEmpty());
    if (callName.isEmpty())
        return;
    // ...
}

问题：使用 Q_ASSERT 后紧接着检查 callName.isEmpty() 是矛盾的，因为 Q_ASSERT 在发布版本中会被移除。
建议：将 Q_ASSERT 替换为普通的条件检查：

if (callName.isEmpty()) {
    qWarning() << "Failed to find call name for watcher";
    return;
}

1.3 updatemodel.cpp 中的逻辑问题

void UpdateModel::setSpeedLimitConfig(const QByteArray& config, bool isFromQml)
{
    qCDebug(logDccUpdatePlugin) << "Set speed limit config:" << config;
    if (DownloadSpeedLimitConfig::fromJson(m_speedLimitConfig).isOnlineSpeedLimit && isFromQml)
        return;
    if (m_speedLimitConfig == config)
        return;

    m_speedLimitConfig = config;
    Q_EMIT downloadSpeedLimitConfigChanged();
}

问题：当 isFromQml 为 true 且当前是在线限速时，直接返回而不更新配置，这可能导致配置不一致。
建议：添加日志说明为什么跳过更新，或者考虑是否真的需要跳过。

2. 代码质量改进

2.1 魔法数字

static constexpr int ShutdownUpdateStatus = 5;

建议：使用枚举或常量定义，并添加注释说明每个状态值的含义：

enum UpdateStatus {
    Default = 0,
    UpdatesAvailable = 1,
    Downloading = 2,
    DownloadPaused = 3,
    Downloaded = 4,
    ShutdownUpdate = 5,
    // ...
};

2.2 函数命名

inline QString transferDeliveryConfigToLastoreDeliveryConfig(const QString& deliveryConfig)

问题：函数名过长，且 transfer 和 to 语义重复。
建议：简化为 convertDeliveryConfig 或 transformDeliveryConfig。

2.3 代码重复

在 UpdateSetting.qml 中，下载和上传限速的设置逻辑高度重复：

D.LineEdit {
    id: downloadLineEdit
    // ... 大量重复代码
}

D.LineEdit {
    id: uploadLineEdit
    // ... 类似的重复代码
}

建议：提取为可重用组件：

component SpeedLimitLineEdit: D.LineEdit {
    // 公共属性和方法
}

3. 性能优化

3.1 频繁的 JSON 解析

bool UpdateModel::downloadSpeedLimitEnabled() const
{
    return DownloadSpeedLimitConfig::fromJson(m_speedLimitConfig).downloadSpeedLimitEnabled;
}

问题：每次调用都解析 JSON，效率较低。
建议：缓存解析结果，只在配置变化时重新解析。

3.2 不必要的信号连接

connect(m_updateAssistant, &UpdateAssistant::UploadLimitSpeedChanged, this, &UpdateModel::onUpdatePropertiesChanged);
connect(m_updateAssistant, &UpdateAssistant::DownloadLimitSpeedChanged, this, &UpdateModel::onUpdatePropertiesChanged);

建议：考虑使用信号映射或单一信号带参数的方式，减少连接数量。

4. 安全问题

4.1 输入验证

void UpdateWorker::setDownloadSpeedLimitSize(const QString& size)
{
    qCDebug(logDccUpdatePlugin) << "set download speed limit size" << size;
    auto config = m_model->speedLimitConfig();
    config.limitSpeed = size;
    setDownloadSpeedLimitConfig(config.toJson());
}

问题：没有验证输入 size 的有效性。
建议：添加输入验证：

bool ok;
int speed = size.toInt(&ok);
if (!ok || speed < 10 || speed > 999999) {
    qWarning() << "Invalid speed limit value:" << size;
    return;
}

4.2 D-Bus 调用错误处理

QDBusPendingReply<void> UpdateDBusProxy::SetUpgradeDeliveryEnable(bool enable)
{
    qCDebug(logCommon) << "Setting upgrade delivery enable :" << enable;
    QList<QVariant> argumentList;
    argumentList << QVariant::fromValue(enable);
    return m_updateInter->asyncCallWithArgumentList(QStringLiteral("SetP2PUpdateEnable"), argumentList);
}

问题：没有处理可能的 D-Bus 错误。
建议：添加错误处理回调或使用同步调用检查结果。

4.3 配置文件权限

m_dconfig(DConfig::create("org.deepin.dde.lastore", "org.deepin.dde.lastore", "", this))

建议：添加配置文件权限检查，确保只有授权用户可以修改。

5. 其他建议

5.1 资源管理

m_animLabel1->setPixmap(QPixmap(":resources/private-lastore-sleep_16px.svg"));
m_animLabel2->setPixmap(QPixmap(":resources/private-lastore-active_16px.svg"));

建议：考虑使用 SVG 渲染器直接渲染，而不是转换为位图，以获得更好的缩放效果。

5.2 线程安全

QMap<QString, QDBusPendingCallWatcher *> m_processingCalls;
QMap<QString, QList<QVariant>> m_waittingCalls;

问题：这些映射可能在多线程环境下使用，但没有同步保护。
建议：添加互斥锁保护这些共享数据结构。

5.3 国际化

text: qsTr("Delivery Optimization")

建议：为所有用户可见的文本添加翻译，并确保翻译键名一致。

总结

这次代码变更实现了重要的功能增强，但在以下几个方面需要改进：

1. 加强输入验证和错误处理
2. 优化性能，特别是频繁的 JSON 解析
3. 提高代码可维护性，减少重复
4. 增强线程安全性
5. 完善文档和注释

建议在合并前进行充分的测试，特别是边界条件和错误场景。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ut005973, xionglinlin

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