fix(data-transfer): improve file path handling in setting helper#722
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Apr 22, 2026
Merged
fix(data-transfer): improve file path handling in setting helper#722deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefines how data transfer settings resolve and clean up file paths by using Qt path utilities for platform-safe JSON file handling and more robust filename extraction, while also adding SPDX licensing headers. Sequence diagram for updated handleDataConfiguration file handlingsequenceDiagram
participant SettingHelper
participant QDir
participant ParseJson
participant QFile
participant Logger
SettingHelper->>QDir: QDir(path)
QDir-->>SettingHelper: absolutePath()
SettingHelper->>QDir: QDir(filepath)
QDir-->>SettingHelper: filePath(transfer.json)
SettingHelper->>ParseJson: ParseJson(transfer_json_path)
ParseJson-->>SettingHelper: QJsonObject jsonObj
SettingHelper->>SettingHelper: validate jsonObj
alt jsonObj is empty
SettingHelper->>SettingHelper: addTaskcounter(-1)
SettingHelper->>Logger: log transfer.json is invaild
SettingHelper-->>SettingHelper: return false
else jsonObj valid
SettingHelper->>SettingHelper: process jsonObj
SettingHelper->>SettingHelper: addTaskcounter(-1)
SettingHelper->>QDir: QDir(filepath)
QDir-->>SettingHelper: filePath(transfer.json)
SettingHelper->>QFile: remove(jsonFile)
alt remove fails
SettingHelper->>Logger: log Failed to remove transfer.json
end
SettingHelper-->>SettingHelper: return true
end
Updated class diagram for SettingHelper path handling methodsclassDiagram
class SettingHelper {
+bool handleDataConfiguration(QString path)
+bool setFile(QJsonObject jsonObj, QString filepath)
-void addTaskcounter(int delta)
}
class QDir {
+QDir(QString path)
+QString absolutePath()
+QString filePath(QString fileName)
+bool exists()
}
class QFileInfo {
+QFileInfo(QString file)
+QString fileName()
+QDir dir()
}
class QFile {
+static bool remove(QString fileName)
}
class QJsonObject {
}
class QJsonArray {
}
class QFilePathUsage {
+QString filepath
+QString jsonFile
+QString filename
+QString file
+QString targetFile
}
SettingHelper ..> QDir : uses
SettingHelper ..> QFileInfo : uses
SettingHelper ..> QFile : uses
SettingHelper ..> QJsonObject : uses
SettingHelper ..> QJsonArray : uses
QFilePathUsage <.. SettingHelper : local variables
%% setFile logic (conceptual structure)
class SettingHelper {
+bool setFile(QJsonObject jsonObj, QString filepath)
}
class SetFileLogic {
+void iterateUserFileArray(QJsonArray userFileArray, QString filepath)
+QString extractFileName(QString filename)
+QString buildSourcePath(QString filepath, QString baseName)
+QString buildTargetPath(QString homePath, QString filename)
+bool ensureTargetDirExists(QFileInfo info)
}
SettingHelper ..> SetFileLogic : uses
SetFileLogic ..> QFileInfo : uses
SetFileLogic ..> QDir : uses
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 1 issue, and left some high level feedback:
- When constructing
fileinsetFile, concatenatingfilepathwithQFileInfo(filename).fileName()may omit a path separator iffilepathdoesn’t end with one; consider usingQDir(filepath).filePath(QFileInfo(filename).fileName())instead of raw string concatenation. - In
handleDataConfiguration, you now only deletetransfer.jsoninstead of removing the whole directory; if the intention is still to clean up temporary directories, consider whether additional directory removal is needed or explicitly documenting that only the JSON file is cleaned up.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When constructing `file` in `setFile`, concatenating `filepath` with `QFileInfo(filename).fileName()` may omit a path separator if `filepath` doesn’t end with one; consider using `QDir(filepath).filePath(QFileInfo(filename).fileName())` instead of raw string concatenation.
- In `handleDataConfiguration`, you now only delete `transfer.json` instead of removing the whole directory; if the intention is still to clean up temporary directories, consider whether additional directory removal is needed or explicitly documenting that only the JSON file is cleaned up.
## Individual Comments
### Comment 1
<location path="src/lib/data-transfer/core/utils/settinghepler.cpp" line_range="78-81" />
<code_context>
QDir pdir(path);
QString filepath = pdir.absolutePath() + "/";
- QJsonObject jsonObj = ParseJson(filepath + "transfer.json");
+ QJsonObject jsonObj = ParseJson(QDir(filepath).filePath("transfer.json"));
if (jsonObj.isEmpty()) {
addTaskcounter(-1);
</code_context>
<issue_to_address>
**suggestion:** Avoid mixing manual path concatenation with QDir, and consider using QDir consistently for `filepath` as well.
`filepath` is built with `pdir.absolutePath() + "/"` and then immediately re-wrapped in `QDir(filepath)` just to resolve `transfer.json`. This is redundant and can be simplified by using `pdir.filePath("transfer.json")` directly, which also avoids any trailing-slash handling.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- Extract only the filename part from the full path to support both relative and absolute path formats - Use QFileInfo to properly handle path separators across different platforms - Replace complex string manipulation with more robust file name extraction logic
deepin pr auto review这段代码的 diff 主要包含版权声明更新、路径处理优化以及文件删除逻辑的修改。以下是对代码的详细审查意见： 1. 版权声明（新增部分）// SPDX-FileCopyrightText: 2023 - 2026 UnionTech Software Technology Co., Ltd.
// SPDX-License-Identifier: GPL-3.0-or-later改进意见：
2. 路径处理优化修改前：QString filepath = pdir.absolutePath() + "/";
QJsonObject jsonObj = ParseJson(filepath + "transfer.json");修改后：QJsonObject jsonObj = ParseJson(pdir.filePath("transfer.json"));改进意见：
3. 文件删除逻辑优化修改前：// remove dir
pdir.removeRecursively();修改后：// remove transfer.json
const QString jsonFile = pdir.filePath("transfer.json");
if (!QFile::remove(jsonFile))
WLOG << "Failed to remove transfer.json: " << jsonFile.toStdString();改进意见：
4. 文件名处理优化修改前：QString targetFile = QDir::homePath() + "/" + filename;
QString file = filepath + filename.mid(filename.indexOf('/') + 1);修改后：// 只保留文件名部分，兼容相对路径和绝对路径格式
QString file = filepath + QFileInfo(filename).fileName();改进意见：
5. 日志记录优化修改前：WLOG << "transfer.json is invaild";修改后：WLOG << "Failed to remove transfer.json: " << jsonFile.toStdString();改进意见：
6. 其他建议
总结
改进后的代码整体质量较高，符合 Qt 的最佳实践。 |
re2zero
approved these changes
Apr 22, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pppanghu77, re2zero 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 |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: unstable) |
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.
Summary by Sourcery
Improve data transfer setting helper path handling and cleanup behavior.
Bug Fixes:
Enhancements: