Repository navigation
feat: add config catalog query and client - #162
Merged
Merged
Conversation
1. Add ConfigInfo struct and D-Bus metatype registration for (appId, resourceId, subpath) triples 2. Add DSGConfigCatalog on the daemon side to enumerate local configuration files across generic meta directories and per-app directories 3. Add ResourceCatalogClient to query the daemon over the system bus and fall back to local directory scanning when the daemon is unreachable 4. Add the Internal D-Bus interface XML exposing configurations() method and configurationsChanged signal 5. Provide helper methods on the client for querying applications, resources per app, all-app resources, subpaths, and existence checks Log: Added configuration catalog querying with D-Bus interface and local fallback Influence: 1. Verify ConfigInfo serialization and deserialization over D-Bus 2. Test DSGConfigCatalog enumeration with multiple app directories and nested subpaths 3. Verify configurationsChanged signal triggers client cache refresh 4. Test ResourceCatalogClient fallback to local directories when the daemon is unavailable 5. Validate resource filtering for NoAppId and per-app queries 6. Test subpath enumeration and exclusion of the root configuration 7. Verify availableResourcesForApp merges per-app and all-app resources without duplicates feat: 添加配置目录查询与客户端 1. 新增 ConfigInfo 结构体及 D-Bus 元类型注册，用于 (appId, resourceId, subpath) 三元组 2. 在守护进程侧新增 DSGConfigCatalog，用于遍历通用元目录及各应用目录下的 本地配置文件 3. 新增 ResourceCatalogClient，通过系统总线查询守护进程，在守护进程不可 用时回退到本地目录扫描 4. 新增 Internal D-Bus 接口 XML，暴露 configurations() 方法及 configurationsChanged 信号 5. 客户端提供查询应用列表、应用资源、全局资源、子路径及存在性检查的辅助 方法 Log: 新增配置目录查询功能，含 D-Bus 接口与本地回退 Influence: 1. 验证 ConfigInfo 在 D-Bus 上的序列化与反序列化 2. 测试 DSGConfigCatalog 对多应用目录及嵌套子路径的遍历 3. 验证 configurationsChanged 信号能触发客户端缓存刷新 4. 测试守护进程不可用时 ResourceCatalogClient 回退到本地目录的逻辑 5. 验证 NoAppId 与各应用查询的资源过滤 6. 测试子路径遍历并确认根配置被排除 7. 验证 availableResourcesForApp 合并各应用与全局资源且无重复
Reviewer's GuideThe PR adds an end-to-end configuration catalog: a shared D-Bus-serializable descriptor, daemon-side filesystem enumeration, an internally exposed D-Bus method and change signal, and a cached client API that falls back to local scanning when the daemon cannot be queried. Sequence diagram for catalog queries and fallbacksequenceDiagram
participant Client as ResourceCatalogClient
participant DBus as DSGConfigInternal
participant Daemon as DSGConfigCatalog
participant FS as Configuration directories
Client->>Client: configurations()
alt daemon available
Client->>DBus: configurations()
DBus->>Daemon: configurations()
Daemon->>FS: enumerate configuration files
FS-->>Daemon: ConfigInfoList
Daemon-->>DBus: ConfigInfoList
DBus-->>Client: ConfigInfoList
Client->>Client: cache configurations
else daemon unavailable
Client->>Client: localConfigurations()
Client->>FS: applications()
Client->>FS: resourcesForApp()
Client->>FS: subpathsForResource()
FS-->>Client: ConfigInfoList
end
Daemon-->>DBus: configurationsChanged()
DBus-->>Client: configurationsChanged()
Client->>Client: refresh()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
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="43-48" />
<code_context>
+{
+ ConfigInfoList result;
+ for (const QString &app : ::applications()) {
+ const QStringList resources = ::resourcesForApp(app);
+ for (const QString &resource : resources) {
+ result.append({app, resource, QString()});
+ const QStringList subpaths = ::subpathsForResource(app, resource);
+ for (const QString &subpath : subpaths)
+ result.append({app, resource, subpath});
+ }
+ }
</code_context>
<issue_to_address>
**issue (broader_impact):** The local fallback and daemon catalog disagree on generic directories containing nested subdirectories: the fallback's `applications()` helper treats each nested generic directory as an application and reports its files with that directory name as `appId`, while `appendConfigurations` deliberately does not recurse for `NoAppId`. The same generic nested configuration therefore receives a different identity, or disappears entirely, when the daemon is unavailable.
**Triggers:** When a generic configuration directory contains nested subpath directories and the daemon is unavailable.
**Suggested fix:** Use the same generic-directory traversal and `NoAppId`/subpath interpretation in the fallback as in `DSGConfigCatalog`, rather than deriving applications from generic directory children.
</issue_to_address>
### Comment 2
<location path="dconfig-center/common/resourcecatalogclient.cpp" line_range="85" />
<code_context>
+ if (m_configurationsValid)
+ 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.";
+ return localConfigurations();
+ }
+
</code_context>
<issue_to_address>
**issue (performance):** A failed D-Bus query returns `localConfigurations()` without setting `m_configurations` or `m_configurationsValid`, so every subsequent helper call synchronously retries the unavailable system-bus request and rescans all local directories. Calls such as `applications()`, `resourcesForApp()`, and `availableResourcesForApp()` consequently repeat the fallback work instead of using the catalog cache.
**Triggers:** When the daemon is unreachable and a caller performs multiple catalog queries.
**Suggested fix:** Cache the fallback result and mark it valid, while invalidating it when a later daemon/signal event indicates that a refresh is needed.
```suggestion
m_configurations = localConfigurations();
m_configurationsValid = true;
return m_configurations;
```
</issue_to_address>
Comment on lines
+43
to
+48
| const QStringList resources = ::resourcesForApp(app); | ||
| for (const QString &resource : resources) { | ||
| result.append({app, resource, QString()}); | ||
| const QStringList subpaths = ::subpathsForResource(app, resource); | ||
| for (const QString &subpath : subpaths) | ||
| result.append({app, resource, subpath}); |
There was a problem hiding this comment.
issue (broader_impact): The local fallback and daemon catalog disagree on generic directories containing nested subdirectories: the fallback's applications() helper treats each nested generic directory as an application and reports its files with that directory name as appId, while appendConfigurations deliberately does not recurse for NoAppId. The same generic nested configuration therefore receives a different identity, or disappears entirely, when the daemon is unavailable.
Triggers: When a generic configuration directory contains nested subpath directories and the daemon is unavailable.
Suggested fix: Use the same generic-directory traversal and NoAppId/subpath interpretation in the fallback as in DSGConfigCatalog, rather than deriving applications from generic directory children.
| if (reply.isError()) { | ||
| qWarning() << "Failed to query configurations from dde-dconfig-daemon:" | ||
| << reply.error().message() << ", fallback to local directories."; | ||
| return localConfigurations(); |
There was a problem hiding this comment.
issue (performance): A failed D-Bus query returns localConfigurations() without setting m_configurations or m_configurationsValid, so every subsequent helper call synchronously retries the unavailable system-bus request and rescans all local directories. Calls such as applications(), resourcesForApp(), and availableResourcesForApp() consequently repeat the fallback work instead of using the catalog cache.
Triggers: When the daemon is unreachable and a caller performs multiple catalog queries.
Suggested fix: Cache the fallback result and mark it valid, while invalidating it when a later daemon/signal event indicates that a refresh is needed.
Suggested change
| return localConfigurations(); | |
| m_configurations = localConfigurations(); | |
| m_configurationsValid = true; | |
| return m_configurations; |
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 语法正确，逻辑清晰，边界处理完善。建议对registerMetaType()使用QMutex或Q_GLOBAL_STATIC保证线程安全；reply.waitForFinished()阻塞调用在当前客户端场景下可接受，但建议在UI线程使用时改用异步调用模式。 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： 代码结构清晰，注释完整。建议为公开API方法添加Doxygen格式文档注释，说明参数含义、返回值和使用场景，提升代码可维护性。 3. 代码性能 ✅评价： 良好 ✅ 通过 潜在问题：
建议： 性能良好，资源使用合理。建议DSGConfigCatalog::configurations()增加缓存机制，在configurationsChanged信号触发时刷新缓存；existResource()可优化为单次遍历直接判断，避免创建临时列表。 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 安全合规，无安全风险。D-Bus通信使用系统总线标准接口，文件系统操作使用Qt安全API，无用户输入直接拼接路径的风险。建议保持当前安全编码实践。 💡 改进建议代码示例// 建议为 DSGConfigCatalog::configurations() 添加缓存机制
class DSGConfigCatalog : public QObject
{
Q_OBJECT
public:
explicit DSGConfigCatalog(QObject *parent = nullptr);
void setLocalPrefix(const QString &localPrefix);
public Q_SLOTS:
ConfigInfoList configurations() const;
Q_SIGNALS:
void configurationsChanged();
private:
QString m_localPrefix;
// 新增缓存成员
mutable ConfigInfoList m_cachedConfigurations;
mutable bool m_cacheValid = false;
};
ConfigInfoList DSGConfigCatalog::configurations() const
{
if (m_cacheValid)
return m_cachedConfigurations;
ConfigInfoList result;
QSet<QString> uniqueKeys;
// ... 原有遍历逻辑 ...
m_cachedConfigurations = result;
m_cacheValid = true;
return result;
}
void DSGConfigCatalog::setLocalPrefix(const QString &localPrefix)
{
if (m_localPrefix == localPrefix)
return;
m_localPrefix = localPrefix;
m_cacheValid = false; // 失效缓存
Q_EMIT configurationsChanged();
}
// 建议优化 existResource() 避免冗余迭代
bool ResourceCatalogClient::existResource(const QString &appid, const QString &resourceId)
{
const auto configs = configurations();
for (const auto &config : configs) {
if (config.resourceId == resourceId &&
(config.appId == appid || config.appId.isEmpty()))
return true;
}
return false;
}本报告由 AI 代码审查工具自动生成 |
MyLeeJiEun
approved these changes
Sep 17, 2026
mhduiy
approved these changes
Sep 17, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, mhduiy, MyLeeJiEun 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resourceId, subpath) triples
configuration files across generic meta directories and per-app
directories
fall back to local directory scanning when the daemon is unreachable
and configurationsChanged signal
resources per app, all-app resources, subpaths, and existence checks
Log: Added configuration catalog querying with D-Bus interface and local
fallback
Influence:
nested subpaths
daemon is unavailable
without duplicates
feat: 添加配置目录查询与客户端
subpath) 三元组
本地配置文件
用时回退到本地目录扫描
configurationsChanged 信号
方法
Log: 新增配置目录查询功能，含 D-Bus 接口与本地回退
Influence:
Summary by Sourcery
Add configuration catalog discovery with daemon-side enumeration, D-Bus access, and a local fallback.
New Features:
Bug Fixes:
Enhancements: