fix(watch): fix path format and add watch for newly created directories

[re2zero]

Ensure directories exist before adding watches and fix path format consistency to match getDesktopFileDirs(). Dynamically add watch for newly created application directories.

修复目录路径格式问题，并在添加监听前确保目录存在。为动态创建的应用目录添加监听。

Log: 修复目录监听问题
PMS: BUG-340395
Influence: 修复了路径格式不一致导致的目录监听失败问题，确保新创建的应用目录能被正确监听，避免应用更新后无法及时刷新。

Summary by Sourcery

Ensure application directories are consistently watched by normalizing paths, creating missing directories, and dynamically adding watches for newly created ones.

Bug Fixes:

• Fix failures to watch application directories caused by inconsistent directory path formats and missing directories before watch registration.

Enhancements:

• Automatically create application directories before registering filesystem watches and add watches dynamically when new application directories are created.

[sourcery-ai]

Reviewer's Guide

Ensures filesystem watcher paths match the format from getDesktopFileDirs(), guarantees watched directories exist (creating them if needed), and dynamically adds a watcher when the user applications directory is created during addUserApplication().

Sequence diagram for updated directory watch initialization in initService

sequenceDiagram
    participant AMS as ApplicationManager1Service
    participant FSW as QFileSystemWatcher_m_watcher
    participant Util as getDesktopFileDirs
    participant QD as QDir
    participant FS as FileSystem

    AMS->>Util: getDesktopFileDirs()
    Util-->>AMS: desktopDirs

    loop ensure_each_directory_exists
        AMS->>QD: QDir(dirPath)
        AMS->>QD: exists()
        alt directory_missing
            AMS->>QD: mkpath(dirPath)
            alt mkpath_success
                QD-->>AMS: true
                AMS->>FS: create_directory(dirPath)
            else mkpath_failure
                QD-->>AMS: false
                AMS-->>AMS: log_warning_failed_to_create
            end
        else directory_exists
            QD-->>AMS: true
        end
    end

    AMS->>FSW: addPaths(desktopDirs)
    FSW-->>AMS: unhandled_paths

    loop log_unhandled_paths
        AMS-->>AMS: log_critical_could_not_watch
    end

    AMS->>FSW: connect(directoryChanged, ReloadApplications)

Loading

Sequence diagram for updated addUserApplication directory creation and watch

sequenceDiagram
    participant Client as D-Bus_Client
    participant AMS as ApplicationManager1Service
    participant Env as getXDGDataHome
    participant QD as QDir
    participant FSW as QFileSystemWatcher_m_watcher
    participant FS as FileSystem

    Client->>AMS: addUserApplication(desktopInfo)

    AMS->>Env: getXDGDataHome()
    Env-->>AMS: xdgDataHomePath

    AMS-->>AMS: dir = xdgDataHomePath
    AMS-->>AMS: ensure_trailing_separator(dir)
    AMS-->>AMS: dir.append("applications")

    AMS->>QD: QDir(dir)
    AMS->>QD: exists()
    QD-->>AMS: dirExisted

    AMS->>QD: mkpath(dir)
    alt mkpath_failure
        QD-->>AMS: false
        AMS-->>Client: sendErrorReply(could_not_create_directory)
        AMS-->>AMS: return
    else mkpath_success
        QD-->>AMS: true

        alt directory_newly_created
            AMS->>FSW: directories()
            FSW-->>AMS: watchedDirs
            AMS-->>AMS: check !dirExisted && !watchedDirs.contains(dir)
            alt needs_watch
                AMS->>FSW: addPath(dir)
                alt watch_added
                    FSW-->>AMS: true
                    AMS-->>AMS: log_info_added_watch
                else watch_failed
                    FSW-->>AMS: false
                    AMS-->>AMS: log_warning_failed_to_add_watch
                end
            else already_watched_or_existed
                AMS-->>AMS: no_additional_watch
            end
        else directory_previously_existed
            AMS-->>AMS: no_additional_watch
        end

        AMS-->>AMS: continue_to_create_or_update_desktop_file
        AMS-->>Client: return_desktop_file_path_or_id
    end

Loading

Updated class diagram for ApplicationManager1Service directory watching logic

classDiagram
    class ApplicationManager1Service {
        - QFileSystemWatcher m_watcher
        + void initService(QDBusConnection connection)
        + QString addUserApplication(QVariantMap desktopInfo)
    }

    class QFileSystemWatcher {
        + QStringList directories()
        + QStringList addPaths(QStringList paths)
        + bool addPath(QString path)
        + void directoryChanged(QString path)
    }

    class QDir {
        + QDir(QString path)
        + bool exists()
        + bool mkpath(QString dirPath)
        + void setPath(QString path)
        + QString filePath(QString fileName)
    }

    class QDBusConnection {
    }

    class Helpers {
        + QStringList getDesktopFileDirs()
        + QString getXDGDataHome()
    }

    ApplicationManager1Service --> QFileSystemWatcher : uses_m_watcher
    ApplicationManager1Service --> QDBusConnection : uses
    ApplicationManager1Service --> QDir : uses
    ApplicationManager1Service --> Helpers : calls
    QFileSystemWatcher --> ApplicationManager1Service : emits_directoryChanged_to_ReloadApplications

Loading

File-Level Changes

Change	Details	Files
Ensure directories returned by getDesktopFileDirs() exist before registering them with QFileSystemWatcher.	Capture getDesktopFileDirs() output into a local desktopDirs variable before adding paths to the watcher. Iterate over each directory path, constructing a QDir and checking existence. Create non-existent directories via QDir::mkpath and log success or failure with qInfo/qWarning. Use the possibly newly created desktopDirs list when calling m_watcher.addPaths instead of calling getDesktopFileDirs() inline.	src/dbus/applicationmanager1service.cpp
Normalize user applications directory path formatting and dynamically register a watch when it is created.	Change the construction of the user applications directory from getXDGDataHome() + "/applications" to building from getXDGDataHome() with a guaranteed trailing separator followed by "applications". Record whether the applications directory existed before mkpath using QDir(dir).exists(). After successful mkpath, if the directory was newly created and not already watched, call m_watcher.addPath(dir) to add a watcher. Log success or failure of dynamically adding the watcher with qInfo/qWarning.	src/dbus/applicationmanager1service.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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• When ensuring directories exist in initService, consider using QDir().mkpath(dirPath) or dir.mkpath(".") instead of dir.mkpath(dirPath) to avoid the confusing pattern of a QDir constructed with dirPath then being asked to mkpath the same absolute path.
• To reduce subtle mismatches when comparing and watching paths (e.g., m_watcher.directories().contains(dir)), normalize directory strings with QDir::cleanPath (and possibly consistent trailing separator handling) before comparisons and addPath/addPaths calls.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- When ensuring directories exist in initService, consider using QDir().mkpath(dirPath) or dir.mkpath(".") instead of dir.mkpath(dirPath) to avoid the confusing pattern of a QDir constructed with dirPath then being asked to mkpath the same absolute path.
- To reduce subtle mismatches when comparing and watching paths (e.g., m_watcher.directories().contains(dir)), normalize directory strings with QDir::cleanPath (and possibly consistent trailing separator handling) before comparisons and addPath/addPaths calls.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[deepin-ci-robot]

deepin pr auto review

这份代码 diff 主要涉及对 ApplicationManager1Service 中文件系统监控逻辑的改进，旨在确保目录存在并动态添加监控。以下是对该 diff 的详细审查，包括语法逻辑、代码质量、代码性能和代码安全方面的改进意见：

1. 语法逻辑

• std::as_const 的使用：代码中正确使用了 std::as_const(desktopDirs) 和 std::as_const(unhandled)，这避免了在基于范围的 for 循环中对容器发生不必要的隐式共享 detach，是良好的 C++ 实践。
• 路径拼接逻辑：
  • 在 addUserApplication 中，手动拼接路径字符串并检查 QDir::separator() 的逻辑略显繁琐且容易出错（例如处理多个分隔符的情况）。
  • 改进建议：使用 QDir::cleanPath() 或者直接使用 QFileInfo(dir, "applications").absoluteFilePath()，或者更简单的 QDir(getXDGDataHome()).filePath("applications")。Qt 的路径处理函数通常比手动拼接字符串更健壮，能自动处理操作系统相关的分隔符问题。
  • 示例改进：

    // 替换手动拼接逻辑
    QString xdgDataHomePath = getXDGDataHome();
    QString dir = QDir(xdgDataHomePath).filePath("applications");
    // 或者
    QString dir = xdgDataHomePath + QDir::separator() + "applications";
    dir = QDir::cleanPath(dir); // 规范化路径

2. 代码质量

• 目录创建的副作用：
  • 在 initService 中，代码尝试 mkpath 所有的 desktop 目录。这可能会在系统层面产生副作用（例如修改文件系统结构），即使这些目录原本并不打算被当前进程创建。通常文件监控服务应该"监控"而非"创建"系统目录。
  • 改进建议：建议确认是否真的需要在初始化时强制创建这些目录。如果只是为了监控，应该过滤掉不存在的目录，而不是创建它们。如果必须创建，应确保有相应的权限和清理机制。
• 日志级别：
  • qCritical() << "couldn't watch directory:" << dir; 使用了 Critical 级别。如果某些目录是可选的或者非关键的，监控失败可能不应是 Critical 级别。
  • 改进建议：根据业务重要性评估日志级别。如果该目录缺失不影响核心功能，降级为 qWarning() 或 qDebug()。
• 代码复用：
  • initService 中有遍历目录创建和添加监控的逻辑，addUserApplication 中也有类似的逻辑（检查存在、创建、添加监控）。
  • 改进建议：可以提取一个私有辅助函数，例如 ensureDirectoryWatched(const QString &path)，封装"检查存在 -> 创建 -> 添加监控"的流程，减少重复代码。

3. 代码性能

• 文件系统操作：
  • dir.exists() 和 dir.mkpath() 都涉及磁盘 I/O。在 initService 中对 desktopDirs 进行循环调用，如果列表较长，可能会阻塞初始化过程。
  • 改进建议：如果初始化在主线程，且目录较多，需注意耗时。不过考虑到通常 desktop 目录数量很少（通常只有几个），这目前不是严重瓶颈，但值得注意。
• 字符串操作：
  • dir.endsWith(QDir::separator()) 和 dir.append(...) 涉及字符串分配和复制。
  • 改进建议：如前所述，使用 QDir 的静态辅助方法处理路径拼接通常效率更高且更安全。

4. 代码安全

• 路径注入与权限：
  • getDesktopFileDirs() 返回的路径如果包含不可信输入，直接进行 mkpath 可能存在安全隐患（例如创建大量嵌套目录消耗磁盘空间，或在敏感位置创建文件）。
  • 改进建议：确保 getDesktopFileDirs() 返回的路径是经过验证的、受控的系统路径（如 /usr/share/applications, ~/.local/share/applications 等），不要接受任意用户输入作为监控路径。
• TOCTOU (Time-of-check to time-of-use)：
  • 代码中存在典型的 if (!dir.exists()) { dir.mkpath(...) } 模式。虽然在单线程应用中通常没问题，但在多线程或多进程环境下，目录状态可能在检查和创建之间发生变化。
  • 改进建议：直接调用 mkpath。QDir::mkpath 内部通常会检查是否存在，如果已存在则返回 true（成功）。这样可以简化逻辑并避免竞态条件。
  • 示例改进：

    // 简化逻辑，直接创建，无需预先检查 exists
    QDir dir(dirPath);
    if (!dir.exists()) { // 这里其实可以去掉，mkpath 会处理
        if (!dir.mkpath(dirPath)) {
             qWarning() << "Failed to create directory:" << dirPath;
        }
    }
    // 改为：
    if (!QDir().mkpath(dirPath)) {
         qWarning() << "Failed to ensure directory exists:" << dirPath;
    }

• 错误处理：
  • safe_sendErrorReply 表明这是一个 DBus 服务。如果 mkpath 失败，直接返回空字符串 {} 是合理的。但需要确保调用方能够正确处理空返回值，避免后续空指针解引用或无效文件操作。

总结与修改建议代码

以下是结合上述建议的优化版本代码片段：

修改 initService 部分：

    connect(&m_watcher, &QFileSystemWatcher::directoryChanged, this, &ApplicationManager1Service::ReloadApplications);

    auto desktopDirs = getDesktopFileDirs();
    for (const auto &dirPath : std::as_const(desktopDirs)) {
        // 安全性：假设 getDesktopFileDirs 返回的是受控路径
        // 性能与逻辑：直接尝试 mkpath，减少一次 exists 检查（mkpath 内部已处理）
        // 注意：根据需求决定是否真的需要创建目录。如果只需监控存在的目录，应改为：
        // if (!QDir(dirPath).exists()) continue;
        
        if (!QDir().mkpath(dirPath)) {
            qWarning() << "Failed to ensure directory exists for watching:" << dirPath;
        }
    }

    auto unhandled = m_watcher.addPaths(desktopDirs);
    for (const auto &dir : std::as_const(unhandled)) {
        // 质量改进：根据业务需求调整日志级别
        qWarning() << "Couldn't watch directory:" << dir;
    }

修改 addUserApplication 部分：

    // 使用 Qt 原生路径处理，更安全且跨平台
    QString xdgDataHomePath = getXDGDataHome();
    QString dir = QDir(xdgDataHomePath).filePath("applications");
    
    // 规范化路径，确保与 getDesktopFileDirs() 格式一致（去除多余分隔符等）
    dir = QDir::cleanPath(dir);

    const QDir appDir(dir);
    const bool dirExisted = appDir.exists();

    // 直接创建，无需预先检查
    if (!QDir().mkpath(dir)) {
        safe_sendErrorReply(QDBusError::Failed, "couldn't create directory of user applications.");
        return {};
    }

    // 动态添加监控
    if (!dirExisted && !m_watcher.directories().contains(dir)) {
        if (!m_watcher.addPath(dir)) {
            qWarning() << "Failed to add watch for newly created directory:" << dir;
        }
    }

    // 后续逻辑...
    xdgDataHome.setPath(dir);
    // ...

这些修改提高了代码的健壮性、可读性和安全性，同时保持了原有功能。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mhduiy, re2zero

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

[re2zero]

/merge