Skip to content

fix: report missing DConfig resource with a dedicated error name - #165

Draft
Ivy233 wants to merge 1 commit into
linuxdeepin:masterfrom
Ivy233:fix/dconfig-resource-not-found
Draft

Ivy233 wants to merge 1 commit into
linuxdeepin:masterfrom
Ivy233:fix/dconfig-resource-not-found

Conversation

@Ivy233

@Ivy233 Ivy233 commented Oct 14, 2026 •

Copy link
Copy Markdown

The daemon replied with the generic org.freedesktop.DBus.Error.Failed both when a configuration resource has no meta file installed (an expected case, e.g. an optional component's configuration is not packaged) and when a real load failure happened (e.g. an unparsable meta file). Clients could not tell the two apart, so the expected case was printed at warning level by dtkcore.

Reply with a dedicated org.desktopspec.ConfigManager.ResourceNotFound error when the meta file is simply not installed; real load failures keep the generic Failed error. isMetaFileMissing() reuses the same meta lookup as the resource loading path.

daemon 不再用通用 Failed 同时表示"meta 未安装"和"meta 在盘但加载失败":前者返回专用错误名 org.desktopspec.ConfigManager.ResourceNotFound,供客户端降级为 debug;后者保持 Failed,避免掩盖真实故障。新增 isMetaFileMissing() 复用与资源加载相同的 meta 查找逻辑。

配套客户端侧改动:linuxdeepin/dtkcore PR #588(优先按该错误名分流日志级别,本地 meta 查找保留为旧版 daemon 的回退路径;两侧可独立合入)。

PMS: TASK-394335

Summary by Sourcery

Differentiate absent configuration metadata from real resource load failures in daemon error responses.

New Features:

  • Return a dedicated ResourceNotFound D-Bus error when a configuration resource has no installed meta file.

Bug Fixes:

  • Distinguish missing configuration metadata from genuine metadata loading failures so clients can handle expected missing resources separately.

Enhancements:

  • Preserve the generic Failed error for installed but invalid or otherwise unloadable metadata.
  • Add coverage for installed, invalid, and missing metadata scenarios.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Ivy233

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

@sourcery-ai

sourcery-ai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Reviewer's Guide

The daemon now distinguishes an uninstalled DConfig meta file from genuine resource-loading failures: it reports the former as org.desktopspec.ConfigManager.ResourceNotFound and preserves the generic Failed error for the latter, with shared lookup logic and unit coverage validating the distinction.

Sequence diagram for DConfig resource acquisition errors

sequenceDiagram
    participant Client
    participant Server as DSGConfigServer
    participant Resource as DSGConfigResource
    participant DConfigFile

    Client->>Server: acquireManagerV2(uid, appid)
    Server->>Resource: load(appid)
    Resource->>DConfigFile: metaPath(m_localPrefix)
    DConfigFile-->>Resource: meta path or empty path
    Resource-->>Server: loadStatus
    alt load succeeds
        Server-->>Client: QDBusObjectPath
    else load fails
        Server->>Resource: isMetaFileMissing(innerAppid)
        Resource->>DConfigFile: metaPath(m_localPrefix)
        DConfigFile-->>Resource: empty or non-empty path
        alt meta path is empty
            Server-->>Client: ResourceNotFound
        else meta path exists
            Server-->>Client: org.freedesktop.DBus.Error.Failed
        end
    end
Loading

File-Level Changes

Change Details Files
Introduces a dedicated D-Bus error for resources whose meta file is not installed.
  • Defines the shared ResourceNotFound error name.
  • Adds a meta-path presence check using the same lookup inputs as resource loading.
  • Returns ResourceNotFound for missing metadata while retaining Failed for other load errors.
  • Adds coverage for installed, invalid, and absent meta-file cases.
dconfig-center/dde-dconfig-daemon/dconfig_global.h
dconfig-center/dde-dconfig-daemon/dconfigresource.cpp
dconfig-center/dde-dconfig-daemon/dconfigresource.h
dconfig-center/dde-dconfig-daemon/dconfigserver.cpp
dconfig-center/tests/ut_dconfigserver.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 reviewed your changes and they look great!


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

The daemon replied with the generic org.freedesktop.DBus.Error.Failed both
when a configuration resource has no meta file installed (an expected case,
e.g. an optional component's configuration is not packaged) and when a real
load failure happened (e.g. unparsable meta file). Clients could not tell
the two apart, so the expected case was printed at warning level by dtkcore.

Reply with a dedicated org.desktopspec.ConfigManager.ResourceNotFound
error when the meta file is simply not installed; real load failures keep
the generic Failed error. isMetaFileMissing() reuses the same meta lookup
as the resource loading path.

daemon 不再用通用 Failed 同时表示“meta 未安装”和“meta 在盘但加载失败”:
前者返回专用错误名 org.desktopspec.ConfigManager.ResourceNotFound,供
客户端降级为 debug;后者保持 Failed,避免掩盖真实故障。新增
isMetaFileMissing() 复用与资源加载相同的 meta 查找逻辑。

PMS: TASK-394335
@Ivy233
Ivy233 force-pushed the fix/dconfig-resource-not-found branch from 1a3bcae to 22095df Compare October 15, 2026 05:17
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

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

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 总体评分 100 分,大于 70 分通过阈值,代码质量优秀。本次提交为 DConfig 守护进程新增专用错误名 ResourceNotFound,用于区分 meta 文件未安装(预期情况)和 meta 文件加载失败(真实故障),实现合理,注释完整,测试覆盖充分。

🔍 详细分析

1. 语法逻辑 ✅

评价: 优秀 ✅ 通过

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

建议: 语法正确,逻辑清晰。isMetaFileMissing() 方法正确复用了 getOrCreateFile() 的 meta 查找逻辑,acquireManagerV2() 中的条件分支正确区分了 meta 未安装和加载失败两种情况。测试用例覆盖了三种场景:已安装(不缺失)、存在但不可解析(不缺失)、完全不存在(缺失)。


2. 代码质量 ✅

评价: 优秀 ✅ 通过

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

建议: 代码结构清晰,注释完整。所有新增代码均配有详细注释,解释了设计意图和与 dtkcore 的同步要求。新增的 ResourceNotFoundError 常量遵循了现有代码风格(与 VirtualInterAppId 一致)。isMetaFileMissing() 方法注释明确说明了与 load() 查找逻辑的一致性关系。测试用例注释清晰说明了每个场景的预期行为。


3. 代码性能 ✅

评价: 优秀 ✅ 通过

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

建议: 性能良好,资源使用合理。isMetaFileMissing() 仅在错误路径(load 失败后)调用,创建临时 DConfigFile 对象的开销可忽略。ResourceNotFoundError 使用 QStringLiteral 宏实现编译时优化。无需优化。


4. 代码安全 🔒

评价: 优秀 ✅ 通过

🔐 发现 0 个安全漏洞

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

建议: 存在0个安全漏洞。ResourceNotFoundError 为编译时常量字符串,无注入风险。错误消息使用 QString::arg() 安全构造。isMetaFileMissing() 仅检查路径是否为空,不涉及文件系统写入操作。无敏感信息泄露。


💡 改进建议代码示例

// 本次提交代码质量优秀,无需修改。
// isMetaFileMissing() 实现简洁正确:
//
// bool DSGConfigResource::isMetaFileMissing(const QString &appid) const
// {
//     DConfigFile file(innerAppidToOuter(appid), m_fileName, m_subpath);
//     return file.meta()->metaPath(m_localPrefix).isEmpty();
// }
//
// acquireManagerV2() 中的错误分流逻辑:
//
// if (resource->isMetaFileMissing(innerAppid))
//     sendErrorReply(ResourceNotFoundError, errorMsg);
// else
//     sendErrorReply(QDBusError::Failed, errorMsg);

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

@Ivy233

Ivy233 commented Oct 15, 2026

Copy link
Copy Markdown
Author

/retest

@deepin-bot

deepin-bot Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

TAG Bot

New tag: 1.0.47
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #167

@Ivy233
Ivy233 marked this pull request as draft October 19, 2026 09:23
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.

2 participants