Repository navigation
fix: support unupgraded daemon compatibility - #163
Conversation
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. 验证缓存机制可防止重复的回退调用
Reviewer's GuideAdds 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 retrievalsequenceDiagram
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
Flow diagram for duplicate-safe resource directory scanningflowchart 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]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
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>
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 维持现有实现。补充说明:configurations() 中缓存回退结果后,若 daemon 升级恢复,configurationsChanged 信号会触发 refresh() 重置缓存(m_configurationsValid=false),下次调用将重新走 DBus 查询路径,状态流转正确,无死缓存风险。 2. 代码质量 ✅评价: 良好 ✅ 通过 潜在问题:
建议: 将回退逻辑提取为 lambda 或私有辅助方法(见下方代码示例),并为 hasInternalInterface() 补充一行用途注释,便于后续维护。 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题:
建议: helper.hpp 中 scannedDirs 去重与空目录提前跳过是明显的正向性能优化(避免对同一 resource 目录重复执行 QDirIterator 子树递归遍历),值得肯定。上述 2 点为轻微优化建议,可视情况采纳。 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: 本次变更无用户输入处理、无命令/路径拼接、无敏感信息。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 代码审查工具自动生成 |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
tracking scanned directories with QSet
initialization
to detect if daemon supports the Internal interface
expose the Internal interface
avoiding repeated failed D-Bus calls
Influence:
Internal interface
path still works
fix: 兼容daemon未升级重启的场景
支持Internal接口
调用
Influence:
Summary by Sourcery
Support configuration discovery when the daemon is not upgraded while improving resource path scanning efficiency.
Bug Fixes:
Enhancements: