Skip to content

fix: support unupgraded daemon compatibility - #163

Merged
18202781743 merged 1 commit into
linuxdeepin:masterfrom
18202781743:master
Sep 20, 2026
Merged

18202781743 merged 1 commit into
linuxdeepin:masterfrom
18202781743:master

Conversation

@18202781743

@18202781743 18202781743 commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor
  1. Add duplicate directory scanning prevention in subpathsForResource by
    tracking scanned directories with QSet
  2. Skip empty directories early to avoid unnecessary QDirIterator
    initialization
  3. Add hasInternalInterface check before calling daemon D-Bus interface
    to detect if daemon supports the Internal interface
  4. Fall back to local configuration scanning when the daemon does not
    expose the Internal interface
  5. Cache fallback results by setting m_configurationsValid to true,
    avoiding repeated failed D-Bus calls

Influence:

  1. Test behavior when dde-dconfig-daemon is not upgraded and lacks the
    Internal interface
  2. Verify fallback to local configuration directories works correctly
  3. Test duplicate resource paths are not scanned multiple times
  4. Verify empty directory optimization does not skip valid resources
  5. Test configuration retrieval after daemon upgrade to ensure D-Bus
    path still works
  6. Verify caching behavior prevents repeated fallback calls

fix: 兼容daemon未升级重启的场景

  1. 在subpathsForResource中通过QSet记录已扫描目录,防止重复扫描相同路径
  2. 提前跳过空目录,避免不必要的QDirIterator初始化
  3. 在调用daemon D-Bus接口前增加hasInternalInterface检查,检测daemon是否
    支持Internal接口
  4. 当daemon未暴露Internal接口时,回退到本地配置目录扫描
  5. 通过设置m_configurationsValid为true缓存回退结果,避免重复失败的D-Bus
    调用

Influence:

  1. 测试dde-dconfig-daemon未升级且缺少Internal接口时的行为
  2. 验证回退到本地配置目录的功能是否正常
  3. 测试重复的资源路径不会被多次扫描
  4. 验证空目录优化不会跳过有效资源
  5. 测试daemon升级后配置获取功能,确保D-Bus路径仍然可用
  6. 验证缓存机制可防止重复的回退调用

Summary by Sourcery

Support configuration discovery when the daemon is not upgraded while improving resource path scanning efficiency.

Bug Fixes:

  • Maintain compatibility with daemons that do not expose the Internal D-Bus interface by falling back to local configuration discovery and caching the result.

Enhancements:

  • Avoid redundant resource-directory scans and skip empty directories during subpath discovery.

1. Add duplicate directory scanning prevention in subpathsForResource by
tracking scanned directories with QSet
2. Skip empty directories early to avoid unnecessary QDirIterator
initialization
3. Add hasInternalInterface check before calling daemon D-Bus interface
to detect if daemon supports the Internal interface
4. Fall back to local configuration scanning when the daemon does not
expose the Internal interface
5. Cache fallback results by setting m_configurationsValid to true,
avoiding repeated failed D-Bus calls

Influence:
1. Test behavior when dde-dconfig-daemon is not upgraded and lacks the
Internal interface
2. Verify fallback to local configuration directories works correctly
3. Test duplicate resource paths are not scanned multiple times
4. Verify empty directory optimization does not skip valid resources
5. Test configuration retrieval after daemon upgrade to ensure D-Bus
path still works
6. Verify caching behavior prevents repeated fallback calls

fix: 兼容daemon未升级重启的场景

1. 在subpathsForResource中通过QSet记录已扫描目录,防止重复扫描相同路径
2. 提前跳过空目录,避免不必要的QDirIterator初始化
3. 在调用daemon D-Bus接口前增加hasInternalInterface检查,检测daemon是否
支持Internal接口
4. 当daemon未暴露Internal接口时,回退到本地配置目录扫描
5. 通过设置m_configurationsValid为true缓存回退结果,避免重复失败的D-Bus
调用

Influence:
1. 测试dde-dconfig-daemon未升级且缺少Internal接口时的行为
2. 验证回退到本地配置目录的功能是否正常
3. 测试重复的资源路径不会被多次扫描
4. 验证空目录优化不会跳过有效资源
5. 测试daemon升级后配置获取功能,确保D-Bus路径仍然可用
6. 验证缓存机制可防止重复的回退调用
@sourcery-ai

sourcery-ai Bot commented Sep 20, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adds compatibility for unupgraded daemons by detecting the Internal D-Bus interface and falling back to cached local configuration discovery, while reducing redundant and unnecessary directory scans during resource lookup.

Sequence diagram for daemon compatibility configuration retrieval

sequenceDiagram
    participant Client as ResourceCatalogClient
    participant DBus as ConfigManagerInternal
    participant Local as localConfigurations()
    participant Cache as configuration cache

    Client->>Cache: configurations()
    alt m_configurationsValid
        Cache-->>Client: cached configurations
    else cache miss
        Client->>DBus: hasInternalInterface()
        alt Internal interface available
            Client->>DBus: configurations()
            alt D-Bus call succeeds
                DBus-->>Client: configuration list
                Client->>Cache: set m_configurationsValid
            else D-Bus call fails
                Client->>Local: localConfigurations()
                Local-->>Client: configuration list
                Client->>Cache: set m_configurationsValid
            end
        else Internal interface unavailable
            Client->>Local: localConfigurations()
            Local-->>Client: configuration list
            Client->>Cache: set m_configurationsValid
        end
    end
    Client-->>Client: return configurations
Loading

Flow diagram for duplicate-safe resource directory scanning

flowchart TD
    A[resourcePathsForApp] --> B[resource path]
    B --> C[derive resource directory]
    C --> D{scannedDirs contains directory?}
    D -->|yes| E[skip directory]
    D -->|no| F[insert directory into scannedDirs]
    F --> G{entryList is empty?}
    G -->|yes| H[skip empty directory]
    G -->|no| I[QDirIterator subdirectories]
    I --> J[collect resource subpaths]
Loading

File-Level Changes

Change Details Files
Avoid redundant filesystem traversal when discovering resource subpaths.
  • Track processed resource directories with a QSet.
  • Skip directories with no child directories before creating a recursive iterator.
dconfig-center/common/helper.hpp
Support configuration retrieval from daemons that predate the Internal D-Bus interface.
  • Probe the Internal interface before issuing the configurations call.
  • Fall back to local configuration scanning when the interface is unavailable or the D-Bus call fails.
  • Mark fallback results valid so subsequent requests use the cached result instead of retrying D-Bus.
dconfig-center/common/resourcecatalogclient.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

@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.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="dconfig-center/common/resourcecatalogclient.cpp" line_range="89-103" />
<code_context>
     if (m_configurationsValid)
         return m_configurations;

+    if (!hasInternalInterface()) {
+        m_configurations = localConfigurations();
+        m_configurationsValid = true;
+        return m_configurations;
+    }
+
     auto reply = m_interface->configurations();
</code_context>
<issue_to_address>
**issue (broader_impact):** After the first fallback to localConfigurations(), m_configurationsValid remains true indefinitely, so configurations() never probes the daemon again. If the daemon is upgraded or temporarily unavailable and later recovers, the client continues serving the stale local snapshot and does not switch back to the D-Bus configuration source until an explicit refresh occurs.

**Triggers:** When the client first starts while the Internal interface is absent or a D-Bus request fails, then the daemon becomes available or configuration files change.

**Suggested fix:** Invalidate the cached fallback when the daemon becomes available, or distinguish fallback caching from a successful daemon cache and periodically/reliably retry the D-Bus path.

```suggestion
    if (!hasInternalInterface()) {
        m_configurations = localConfigurations();
        m_configurationsValid = false;
        return m_configurations;
    }

    auto reply = m_interface->configurations();
    reply.waitForFinished();
    if (reply.isError()) {
        qWarning() << "Failed to query configurations from dde-dconfig-daemon:"
                   << reply.error().message() << ", fallback to local directories.";
        m_configurations = localConfigurations();
        m_configurationsValid = false;
        return m_configurations;
    }
```
</issue_to_address>

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

Comment thread dconfig-center/common/resourcecatalogclient.cpp
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 96 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 总体评分 96 分,大于 70 分通过阈值。本次变更目的为兼容未升级的 dde-dconfig-daemon(DBUS Internal 接口不存在时回退本地目录扫描并缓存),实现与 commit 目的完全一致,无安全漏洞,仅有少量可维护性与性能层面的轻微改进建议。

🔍 详细分析

1. 语法逻辑 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: 维持现有实现。补充说明:configurations() 中缓存回退结果后,若 daemon 升级恢复,configurationsChanged 信号会触发 refresh() 重置缓存(m_configurationsValid=false),下次调用将重新走 DBus 查询路径,状态流转正确,无死缓存风险。


2. 代码质量 ✅

评价: 良好 ✅ 通过

潜在问题:

  1. dconfig-center/common/resourcecatalogclient.cpp:89 - localConfigurations() 回退逻辑在 !hasInternalInterface() 路径与 reply.isError() 路径重复出现 2 处(OCR 与 AI 审计共同发现,已合并去重),建议提取为 lambda/辅助方法(重复代码块 -3 分)
  2. dconfig-center/common/resourcecatalogclient.cpp:21 - 新增的 hasInternalInterface() 函数缺少用途注释(轻微,未单独扣分)

建议: 将回退逻辑提取为 lambda 或私有辅助方法(见下方代码示例),并为 hasInternalInterface() 补充一行用途注释,便于后续维护。


3. 代码性能 ✅

评价: 优秀 ✅ 通过

潜在问题:

  1. dconfig-center/common/resourcecatalogclient.cpp:21 - hasInternalInterface() 缓存失效时同步构造 QDBusInterface 触发一次同步 introspection 往返,有 m_configurationsValid 缓存保护,影响很小(轻微建议 -1 分)
  2. dconfig-center/common/helper.hpp:162 - entryList(filters) 与 setFilter(filters) 调用存在少量冗余(功能正确,轻微建议)

建议: helper.hpp 中 scannedDirs 去重与空目录提前跳过是明显的正向性能优化(避免对同一 resource 目录重复执行 QDirIterator 子树递归遍历),值得肯定。上述 2 点为轻微优化建议,可视情况采纳。


4. 代码安全 🔒

评价: 优秀 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: 本次变更无用户输入处理、无命令/路径拼接、无敏感信息。DBus 服务名 org.desktopspec.ConfigManager、对象路径 /Internal、接口名均为编译期常量,无注入或路径遍历风险,安全合规。


💡 改进建议代码示例

// resourcecatalogclient.cpp - configurations() 消除回退逻辑重复(OCR 建议 + AI 审计一致)
ConfigInfoList ResourceCatalogClient::configurations()
{
    if (m_configurationsValid)
        return m_configurations;

    // 探测旧版本 daemon:无 Internal 接口时回退本地目录扫描并缓存
    auto fallbackToLocal = [this]() {
        m_configurations = localConfigurations();
        m_configurationsValid = true;
        return m_configurations;
    };

    if (!hasInternalInterface())
        return fallbackToLocal();

    auto reply = m_interface->configurations();
    reply.waitForFinished();
    if (reply.isError()) {
        qWarning() << "Failed to query configurations from dde-dconfig-daemon:"
                   << reply.error().message() << ", fallback to local directories.";
        return fallbackToLocal();
    }

    m_configurations = reply.value();
    m_configurationsValid = true;
    return m_configurations;
}

本报告由 AI 代码审查工具自动生成

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@18202781743
18202781743 merged commit 37cb64d into linuxdeepin:master Sep 20, 2026
15 of 16 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