fix: remove FileHandler backend and add null checks#139
Merged
18202781743 merged 1 commit intolinuxdeepin:masterfrom Feb 5, 2026
Merged
fix: remove FileHandler backend and add null checks#13918202781743 merged 1 commit intolinuxdeepin:masterfrom
18202781743 merged 1 commit intolinuxdeepin:masterfrom
Conversation
The changes remove the FileHandler backend functionality from the value handler creation process and add null pointer checks for ConfigGetter instances. The FileHandler backend was causing issues with directory writing operations, leading to potential data corruption or configuration problems. By removing this backend, the system now relies solely on the DBusHandler for configuration management, ensuring consistent behavior across the application. Additionally, multiple null pointer checks have been added when creating ConfigGetter instances in various UI components (MainWindow, Content, HistoryDialog) to prevent crashes when manager creation fails. These checks provide graceful error handling with appropriate warning messages. Log: Fixed configuration management issues by removing problematic file backend Influence: 1. Test configuration value retrieval through the UI to ensure no regression 2. Verify that reset operations work correctly for configuration keys 3. Test context menu functionality in the configuration editor 4. Validate history dialog operations with various configuration resources 5. Ensure error cases are handled gracefully with appropriate warnings 6. Test configuration operations with both valid and invalid resource paths fix: 移除FileHandler后端并添加空指针检查 本次修改移除了值处理器创建过程中的FileHandler后端功能，并为ConfigGetter 实例添加了空指针检查。FileHandler后端会导致目录写入操作出现问题，可能引 发数据损坏或配置问题。通过移除此后端，系统现在仅依赖DBusHandler进行配置 管理，确保应用程序行为的一致性。 此外，在多个UI组件（MainWindow、Content、HistoryDialog）中创建 ConfigGetter实例时添加了空指针检查，以防止管理器创建失败时程序崩溃。这些 检查提供了优雅的错误处理机制，并输出适当的警告信息。 Log: 通过移除有问题的文件后端修复了配置管理问题 Influence: 1. 测试通过UI获取配置值，确保功能没有退化 2. 验证配置键的重置操作是否正常工作 3. 测试配置编辑器中的上下文菜单功能 4. 使用各种配置资源验证历史对话框操作 5. 确保错误情况得到优雅处理并显示适当的警告 6. 测试使用有效和无效资源路径的配置操作 PMS: BUG-312173 Change-Id: Iae5db6d3701fe579dc4b0af8e4b6a4ae0f7c9e44
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRemoves the FileHandler backend from ValueHandler::createManager so that only DBusHandler is used, and adds null checks around ConfigGetter creation in several UI components to avoid crashes when manager creation fails and to log clear warnings instead. Sequence diagram for ConfigGetter creation with null checks in UI componentssequenceDiagram
actor User
participant MainWindow
participant Content
participant HistoryDialog
participant ValueHandler
participant ConfigGetter
User->>MainWindow: trigger reset command
MainWindow->>ValueHandler: new ValueHandler(appid, resource, subpath)
MainWindow->>ValueHandler: createManager()
ValueHandler-->>MainWindow: ConfigGetter pointer
alt manager is null
MainWindow->>MainWindow: log warning Failed to create manager for reset command
MainWindow-->>User: no reset performed
else manager is valid
MainWindow->>ConfigGetter: keyList()
ConfigGetter-->>MainWindow: keys
MainWindow->>ConfigGetter: reset(key)
MainWindow-->>User: reset completed
end
User->>Content: open context menu
Content->>ValueHandler: new ValueHandler(appid, resource, subpath)
Content->>ValueHandler: createManager()
ValueHandler-->>Content: ConfigGetter pointer
alt manager is null
Content->>Content: log warning Failed to create manager for context menu
else manager is valid
Content->>ConfigGetter: value(key)
ConfigGetter-->>Content: value
Content->>ConfigGetter: description(key, language)
ConfigGetter-->>Content: description
end
User->>HistoryDialog: open history dialog
HistoryDialog->>ValueHandler: handler.createManager()
ValueHandler-->>HistoryDialog: ConfigGetter pointer
alt manager is null
HistoryDialog->>HistoryDialog: log warning Failed to create manager for history
else manager is valid
HistoryDialog->>ConfigGetter: value(key)
ConfigGetter-->>HistoryDialog: historical values
end
Updated class diagram for ValueHandler createManager backend selectionclassDiagram
class ValueHandler {
- QString appid
- QString resource
- QString subpath
+ ValueHandler(QString appid, QString resource, QString subpath)
+ ConfigGetter* createManager()
}
class ConfigGetter {
<<abstract>>
+ QStringList keyList()
+ QVariant value(QString key)
+ QString description(QString key, QString language)
+ void reset(QString key)
}
class DBusHandler {
+ bool createManager(QString appid, QString fileName, QString subpath)
}
class FileHandler {
+ bool createManager(QString appid, QString fileName, QString subpath)
}
ValueHandler ..> ConfigGetter : createManager
ConfigGetter <|-- DBusHandler
ConfigGetter <|-- FileHandler
%% Updated behavior: only DBusHandler is used in createManager
ValueHandler ..> DBusHandler : tries_and_on_success_returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review以下，以下是对代码diff的详细审查意见： 代码审查报告1. 语法逻辑审查问题1： - } else {
- auto tmp = new FileHandler(this);
- if (tmp->createManager(appid, fileName, subpath)) {
- qDebug() << QString("using FileHandler to get value for appid=%1, resource=%2, subpath=%3.").arg(appid, fileName, subpath);
- return tmp;
- }
+ delete tmp; // 创建失败，释放内存问题：
建议修改： if (tmp->createManager(appid, fileName, subpath)) {
return tmp;
}
delete tmp; // 创建失败，释放内存问题2： if (!manager) {
qWarning() << "Failed to create manager for history";
return;
}
if (manager) {问题：
建议修改： if (!manager) {
qWarning() << "Failed to create manager for history";
return;
}
// 直接使用manager，不需要额外的if检查2. 代码质量审查优点：
改进建议：
3. 代码性能审查潜在问题：
建议： // 在类成员中缓存manager
void Content::ensureManagerCreated() {
if (!m_manager) {
m_manager.reset(m_getter->createManager());
if (!m_manager) {
qWarning() << "Failed to create manager";
}
}
}4. 代码安全审查安全问题：
建议：
5. 综合改进建议建议的完整修改方案： // valuehandler.cpp
ConfigGetter* ValueHandler::createManager() {
auto tmp = new FileHandler(this);
if (tmp->createManager(appid, fileName, subpath)) {
qDebug() << QString("Using FileHandler for appid=%1, resource=%2, subpath=%3")
.arg(appid, fileName, subpath);
return tmp;
}
delete tmp;
qWarning() << QString("Failed to create value handler for appid=%1, resource=%2, subpath=%3")
.arg(appid, fileName, subpath);
return nullptr;
}
// mainwindow.cpp
void MainWindow::onCustomResourceMenuRequested(const QString &appid, const QString &resource, const QString &subpath) {
// ... 其他代码 ...
connect(resetCmdAction, &QAction::triggered, this, [this, appid, resource, subpath] {
QScopedPointer<ValueHandler> getter(new ValueHandler(appid, resource, subpath));
QScopedPointer<ConfigGetter> manager(getter->createManager());
if (!manager) {
qWarning() << QString("Failed to create manager for reset command (appid=%1, resource=%2, subpath=%3)")
.arg(appid, resource, subpath);
return;
}
// ... 使用manager ...
});
}总结
这些改进将提高代码的健壮性、可维护性和安全性。 |
![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 left some high level feedback:
- In
ValueHandler::createManager, consider using a RAII helper (e.g.std::unique_ptr/QScopedPointer) forDBusHandlerinstead of a raw pointer plus manualdeleteto make the lifetime and failure paths safer and easier to maintain. - In
HistoryDialog, after adding the earlyif (!manager) { ... return; }check, the subsequentif (manager) { ... }is now redundant and can be simplified to reduce nesting.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ValueHandler::createManager`, consider using a RAII helper (e.g. `std::unique_ptr`/`QScopedPointer`) for `DBusHandler` instead of a raw pointer plus manual `delete` to make the lifetime and failure paths safer and easier to maintain.
- In `HistoryDialog`, after adding the early `if (!manager) { ... return; }` check, the subsequent `if (manager) { ... }` is now redundant and can be simplified to reduce nesting.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
BLumia
approved these changes
Feb 5, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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
3 participants
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.
The changes remove the FileHandler backend functionality from the
value handler creation process and add null pointer checks for
ConfigGetter instances. The FileHandler backend was causing issues
with directory writing operations, leading to potential data corruption
or configuration problems. By removing this backend, the system now
relies solely on the DBusHandler for configuration management, ensuring
consistent behavior across the application.
Additionally, multiple null pointer checks have been added when
creating ConfigGetter instances in various UI components (MainWindow,
Content, HistoryDialog) to prevent crashes when manager creation fails.
These checks provide graceful error handling with appropriate warning
messages.
Log: Fixed configuration management issues by removing problematic file
backend
Influence:
regression
resources
paths
fix: 移除FileHandler后端并添加空指针检查
本次修改移除了值处理器创建过程中的FileHandler后端功能，并为ConfigGetter
实例添加了空指针检查。FileHandler后端会导致目录写入操作出现问题，可能引
发数据损坏或配置问题。通过移除此后端，系统现在仅依赖DBusHandler进行配置
管理，确保应用程序行为的一致性。
此外，在多个UI组件（MainWindow、Content、HistoryDialog）中创建
ConfigGetter实例时添加了空指针检查，以防止管理器创建失败时程序崩溃。这些
检查提供了优雅的错误处理机制，并输出适当的警告信息。
Log: 通过移除有问题的文件后端修复了配置管理问题
Influence:
PMS: BUG-312173
Change-Id: Iae5db6d3701fe579dc4b0af8e4b6a4ae0f7c9e44
Summary by Sourcery
Remove the file-based configuration backend and harden configuration access against null managers.
Bug Fixes:
Enhancements: