Skip to content

refactor: optimize AM appitem property update handling - #1690

Merged
wjyrich merged 1 commit into
linuxdeepin:masterfrom
wjyrich:task-393709
Aug 11, 2026
Merged

refactor: optimize AM appitem property update handling#1690
wjyrich merged 1 commit into
linuxdeepin:masterfrom
wjyrich:task-393709

Conversation

@wjyrich

@wjyrich wjyrich commented Aug 5, 2026

Copy link
Copy Markdown
Contributor
  1. Refactor onPropertyChanged to handle individual property changes
    instead of fetching all properties on every D-Bus signal
  2. Add vendor and genericName data roles to AppItem with corresponding
    accessors
  3. Introduce Environ role for storing application environment data
  4. Remove QtConcurrent dependency by simplifying async loading logic
  5. Fix isOnDesktop property name mismatch (was OnDesktop)
  6. Optimize duplicate check during managed objects loading
  7. Replace ready property with proper readyChanged signal emission
  8. Expose DDE categories as a QVariantMap property for QML usage
  9. Clean up updateActions to properly handle empty action lists

Log: Optimized app item property updates and added vendor/genericName/
Environ data roles

Influence:

  1. Verify application display names and generic names still display
    correctly in launcher
  2. Test that application icons update properly when changed at runtime
  3. Verify app model readiness signal works correctly with QML bindings
  4. Test category filtering still functions with new DDE categories
    property
  5. Verify duplicate desktop entries are properly filtered during loading
  6. Test action menus still work correctly for applications with defined
    actions
  7. Verify apps are correctly filtered by vendor (deepin vs other)

refactor: 优化AM应用项属性更新处理

  1. 重构onPropertyChanged,针对单个属性变化进行处理,而不是每次D-Bus信号
    都获取所有属性
  2. 为AppItem添加vendor和genericName数据角色及对应的访问器
  3. 新增Environ角色用于存储应用环境数据
  4. 简化异步加载逻辑,移除QtConcurrent依赖
  5. 修复isOnDesktop属性名称不匹配问题(原为OnDesktop)
  6. 优化加载托管对象时的重复检查逻辑
  7. 用proper readyChanged信号替代原ready属性
  8. 将DDE分类暴露为QVariantMap属性供QML使用
  9. 清理updateActions以正确处理空操作列表

Log: 优化应用项属性更新,新增vendor/genericName/Environ数据角色

Influence:

  1. 验证启动器中应用显示名称和通用名称是否仍然正确显示
  2. 测试应用图标在运行时更改后是否能正确更新
  3. 验证应用模型就绪信号与QML绑定的正常工作
  4. 测试分类过滤功能在新DDE分类属性下是否正常
  5. 验证加载过程中重复桌面条目的过滤是否正常
  6. 测试已定义动作的应用的右键菜单是否正常工作
  7. 验证应用是否能按供应商正确筛选(deepin与其他)

PMS: TASK-393709

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @wjyrich, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Reviewer's Guide

Refactors application manager integration to perform targeted AMAppItem property updates, enrich AppItem metadata, introduce a GIO-based TrashMonitor for dynamic trash icon state, extend AppsApplet with app management APIs, fix AMAppItemModel initialization/ready signaling (including trash icon updates and deduplication), and propagate app model data changes into the dock's global element model.

Sequence diagram for AMAppItem targeted property updates

sequenceDiagram
    participant AM_Application as ApplicationManager_Application
    participant DBus as DBus
    participant AMAppItem as AMAppItem
    participant AppItem as AppItem
    participant AppItemModel as AppItemModel

    AM_Application->>DBus: PropertiesChanged
    DBus->>AMAppItem: onPropertyChanged(QDBusMessage)
    AMAppItem->>AMAppItem: qdbus_cast QVariantMap changedProperties
    alt [Name/GenericName/X_Deepin_Vendor changed]
        AMAppItem->>AppItem: setGenericName()
        AMAppItem->>AppItem: setVendor()
        AMAppItem->>AppItem: setAppName()
    end
    alt [Icons changed]
        AMAppItem->>AppItem: setAppIconName()
    end
    alt [Categories changed]
        AMAppItem->>AppItem: setDDECategories()
        AMAppItem->>AppItem: setCategories()
    end
    alt [Actions or ActionName changed]
        AMAppItem->>AMAppItem: updateActions()
        AMAppItem->>AppItem: setActions()
    end
    AppItem-->>AppItemModel: dataChanged(...) emits
Loading

Sequence diagram for TrashMonitor-driven trash icon updates

sequenceDiagram
    participant FS as GIO_trash
    participant GIO as GFileMonitor
    participant TrashMonitor as TrashMonitor
    participant AMAppItemModel as AMAppItemModel
    participant TrashItem as AMAppItem_dde_trash

    FS-->>GIO: file event
    GIO->>TrashMonitor: onTrashChanged(...)
    TrashMonitor->>TrashMonitor: updateState()
    TrashMonitor-->>AMAppItemModel: emptyChanged(bool)
    AMAppItemModel->>AMAppItemModel: updateTrashIcon()
    AMAppItemModel->>TrashItem: setAppIconName(user-trash | user-trash-full)
Loading

File-Level Changes

Change Details Files
Extend AppsApplet to expose app model readiness, category metadata, and rich application management operations for launch, autostart, scaling, and desktop shortcuts.
  • Expose appModel, groupModel, appModelReady, and ddeCategories via Q_PROPERTY and helper accessors
  • Add a private application() helper to resolve AMAppItem by desktopId while normalizing .desktop suffix
  • Implement launchApp() using QProcess to invoke dde-am with optional activation token and error logging
  • Add autoStart()/setAutoStart() and disableScale()/setDisableScale() that manipulate AMAppItem D-Bus properties and environment variables
  • Add isOnDesktop(), sendToDesktop(), and removeFromDesktop() wrappers around AMAppItem D-Bus methods with error-checked replies
applets/dde-apps/appsapplet.cpp
applets/dde-apps/appsapplet.h
Refine AMAppItem to initialize vendor/genericName and handle D-Bus property change signals by updating only changed AppItem fields.
  • Connect to org.freedesktop.DBus.Properties::PropertiesChanged on each AM application object
  • Initialize AppItem::genericName and AppItem::vendor from AM metadata and adjust isOnDesktop key
  • In onPropertyChanged(), filter by Application interface, parse the changed-properties map, and selectively update AppItem fields (names, icons, flags, categories, timestamps, execs, desktop source path, actions, etc.)
  • Ensure actions JSON is cleared when no actions exist instead of retaining stale data
applets/dde-apps/amappitem.cpp
Fix AMAppItemModel initialization, introduce trash monitoring, and ensure trash icon reflects actual trash state without duplicate applications.
  • Create and own a TrashMonitor instance and hook its emptyChanged signal to updateTrashIcon()
  • Enhance GetManagedObjects() completion handler to skip empty paths, deduplicate by DesktopIdRole, and properly set m_ready and emit readyChanged
  • Update AMAppItemModel constructor wiring to append new AMAppItem rows and refresh trash icon on additions and on relevant dataChanged events
  • Implement updateTrashIcon() to toggle dde-trash's icon between user-trash and user-trash-full based on TrashMonitor::isEmpty()
applets/dde-apps/amappitemmodel.cpp
applets/dde-apps/amappitemmodel.h
applets/dde-apps/trashmonitor.cpp
applets/dde-apps/trashmonitor.h
applets/dde-apps/CMakeLists.txt
Extend AppItem/AppItemModel roles to carry vendor and genericName metadata into the model layer.
  • Add VendorRole and GenericNameRole to AppItemModel::Roles and expose them in roleNames()
  • Implement AppItem::vendor/genericName getters and setters that store data under the new roles
applets/dde-apps/appitem.cpp
applets/dde-apps/appitem.h
applets/dde-apps/appitemmodel.cpp
applets/dde-apps/appitemmodel.h
Propagate app model data changes into DockGlobalElementModel so dock entries stay in sync with application metadata.
  • Connect to the apps model's dataChanged signal and, for each affected source row present in m_data, emit corresponding dataChanged from the dock model
  • When DesktopIdRole changes, also mark ItemIdRole as changed so consumers relying on item IDs get notified
panels/dock/taskmanager/dockglobalelementmodel.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@wjyrich

wjyrich commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

/test github-pr-review-ci

Comment thread applets/dde-apps/appsapplet.cpp Outdated
Comment thread applets/dde-apps/appsapplet.cpp Outdated
Comment thread applets/dde-apps/appsapplet.h Outdated
Comment thread applets/dde-apps/trashmonitor.cpp Outdated
@wjyrich
wjyrich force-pushed the task-393709 branch 2 times, most recently from 79769f4 to 5e3d4d0 Compare August 7, 2026 06:22
@wjyrich wjyrich changed the title fix: optimize app properties and trash monitoring refactor: optimize AM appitem property update handling Aug 7, 2026
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, wjyrich

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

1. Refactor onPropertyChanged to handle individual property changes
instead of fetching all properties on every D-Bus signal
2. Add vendor and genericName data roles to AppItem with corresponding
accessors
3. Introduce Environ role for storing application environment data
4. Remove QtConcurrent dependency by simplifying async loading logic
5. Fix isOnDesktop property name mismatch (was OnDesktop)
6. Optimize duplicate check during managed objects loading
7. Replace ready property with proper readyChanged signal emission
8. Expose DDE categories as a QVariantMap property for QML usage
9. Clean up updateActions to properly handle empty action lists

Log: Optimized app item property updates and added vendor/genericName/
Environ data roles

Influence:
1. Verify application display names and generic names still display
correctly in launcher
2. Test that application icons update properly when changed at runtime
3. Verify app model readiness signal works correctly with QML bindings
4. Test category filtering still functions with new DDE categories
property
5. Verify duplicate desktop entries are properly filtered during loading
6. Test action menus still work correctly for applications with defined
actions
7. Verify apps are correctly filtered by vendor (deepin vs other)

refactor: 优化AM应用项属性更新处理

1. 重构onPropertyChanged,针对单个属性变化进行处理,而不是每次D-Bus信号
都获取所有属性
2. 为AppItem添加vendor和genericName数据角色及对应的访问器
3. 新增Environ角色用于存储应用环境数据
4. 简化异步加载逻辑,移除QtConcurrent依赖
5. 修复isOnDesktop属性名称不匹配问题(原为OnDesktop)
6. 优化加载托管对象时的重复检查逻辑
7. 用proper readyChanged信号替代原ready属性
8. 将DDE分类暴露为QVariantMap属性供QML使用
9. 清理updateActions以正确处理空操作列表

Log: 优化应用项属性更新,新增vendor/genericName/Environ数据角色

Influence:
1. 验证启动器中应用显示名称和通用名称是否仍然正确显示
2. 测试应用图标在运行时更改后是否能正确更新
3. 验证应用模型就绪信号与QML绑定的正常工作
4. 测试分类过滤功能在新DDE分类属性下是否正常
5. 验证加载过程中重复桌面条目的过滤是否正常
6. 测试已定义动作的应用的右键菜单是否正常工作
7. 验证应用是否能按供应商正确筛选(deepin与其他)

PMS: TASK-393709
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:95分

■ 【总体评价】

代码优化了DBus属性变更处理逻辑并修复了状态同步问题,整体质量良好
逻辑正确且性能提升明显,因初始化时的查重操作存在轻微性能隐患扣5分

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

amappitem.cpp 中的 onPropertyChanged 函数修复了原先全量更新导致的问题,改为根据 DBus 返回的变更字段按需更新特定属性,逻辑正确。amappitemmodel.cpp 中将 watcher->deleteLater() 移至获取 reply 之后,确保了异步调用结果的安全获取,避免了潜在的悬空指针问题。
潜在问题:在 AMAppItemModel 构造函数的 lambda 中,每次添加新应用都调用 match 函数进行全表搜索查重,若应用数量极多可能导致初始化变慢。
建议:使用 QSet 或 QHash 记录已存在的 desktopId 以优化查重性能,将时间复杂度从 O(N^2) 降至 O(N)。

  • 2.代码质量(良好)✓

移除了未使用的 QtConcurrent 依赖、load() 函数和 m_appModelReady 成员变量,清理了冗余代码。在 AppItemModel 中补充了 Q_OBJECT 宏,确保元对象系统正常工作。新增的 vendorgenericName 属性封装规范,符合 Qt 的属性定义标准。
潜在问题:onPropertyChanged 中存在大量重复的 if (contains(...)) 结构,略显冗长。
建议:可以考虑使用宏或模板函数进一步简化重复的属性更新代码,但当前可读性已可接受。

  • 3.代码性能(高效)✓

onPropertyChanged 从原先的每次属性变更都拉取并更新所有属性,优化为仅更新发生变化的属性,大幅减少了不必要的 DBus 调用和 UI 刷新,性能提升显著。
潜在问题:无
建议:无需额外优化。

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码主要涉及 DBus 通信和本地数据模型更新,输入来源于系统 DBus 服务,不存在外部不可信输入注入的风险。类型转换使用了 Qt 提供的安全接口。

  • 建议:保持现有的安全编码规范。

■ 【改进建议代码示例】

// amappitemmodel.cpp
auto watcher = new QDBusPendingCallWatcher(m_manager->GetManagedObjects(), this);
connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, watcher]() {
    QDBusPendingReply<ObjectMap> reply = *watcher;
    watcher->deleteLater();
    if (reply.isError()) {
        qCWarning(appsLog) << "Failed to load applications from ApplicationManager:" << reply.error();
        return;
    }

    QSet<QString> existingIds;
    for (int i = 0; i < rowCount(); ++i) {
        existingIds.insert(item(i)->data(AppItemModel::DesktopIdRole).toString());
    }

    const auto apps = reply.value();
    for (auto app = apps.cbegin(); app != apps.cend(); ++app) {
        const auto path = app.key();
        if (path.path().isEmpty())
            continue;

        const auto desktopId = DUtil::unescapeFromObjectPath(path.path().split('/').last());
        if (existingIds.contains(desktopId))
            continue;
        
        existingIds.insert(desktopId);
        appendRow(new AMAppItem(path, app.value()));
    }

    m_ready = true;
    Q_EMIT readyChanged(true);
    qCDebug(appsLog) << "AMAppItemModel is now ready with apps counts:" << rowCount();
});

@wjyrich
wjyrich merged commit 26d7804 into linuxdeepin:master Aug 11, 2026
12 of 13 checks passed
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