Repository navigation
fix: mark index changed in removeFile - #293
deepin-bot[bot] merged 1 commit into
Conversation
1. Root cause: removeFile() deleted index documents but did not call reporter->markIndexChanged(), leaving indexChanged=false, causing taskmanager to skip saveLastUpdateTime after file deletion 2. Fix: add reporter->markIndexChanged() after deleteDocuments, before increment, matching updateFile and removeDirectoryIndex pattern 3. Impact: deleting files now correctly refreshes index update time; RemoveFileListHandler directory path is idempotent (already marked) Log: Fix index update time not refreshing after file deletion Influence: 1. Test file deletion triggers index status update time refresh 2. Test directory deletion still refreshes index update time 3. Verify no regression on batch file removal operations fix: 修复删除文件后索引更新时间未刷新 1. 根因:removeFile() 删除索引文档后未调用 markIndexChanged(), 导致 indexChanged 为 false,taskmanager 跳过 saveLastUpdateTime 2. 方案:在 deleteDocuments 之后、increment 之前添加 markIndexChanged() 调用,与 updateFile/removeDirectoryIndex 一致 3. 影响:删除文件后索引更新时间正确刷新;删除目录路径幂等无冲突 Log: 修复删除文件后文件索引更新时间不刷新的问题 Influence: 1. 测试删除文件后索引状态更新时间是否刷新 2. 测试删除目录后索引更新时间是否刷新 3. 验证批量删除文件操作无回归 PMS: BUG-380553
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Johnson-zs 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 |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds the missing index-change notification to removeFile(), ensuring file deletions trigger saveLastUpdateTime() while preserving existing directory and batch deletion behavior. Sequence diagram for file deletion index status updatesequenceDiagram
participant RemoveFileListHandler
participant removeFile
participant IndexWriter
participant Reporter
participant TaskManager
participant IndexStatus
RemoveFileListHandler->>removeFile: removeFile(context, path, writer, reporter)
removeFile->>IndexWriter: deleteDocuments(term)
removeFile->>Reporter: markIndexChanged()
removeFile->>Reporter: increment()
TaskManager->>Reporter: indexChanged()
alt index changed
TaskManager->>IndexStatus: saveLastUpdateTime()
end
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="src/index/task/taskhandler.cpp" line_range="468" />
<code_context>
TermPtr term = newLucene<Term>(context.profile().pathField(), path.toStdWString());
writer->deleteDocuments(term);
if (reporter) {
+ reporter->markIndexChanged();
reporter->increment();
}
</code_context>
<issue_to_address>
**No-op deletion advances timestamp**
When a removal request names a path with no indexed document, `removeFile()` calls `markIndexChanged()` after `deleteDocuments()` matches nothing, so `RemoveFileListHandler` returns `indexChanged=true` and `TaskManager::updateIndexStatusOnSuccess()` saves a new update time for an unchanged index.
Mark the index changed only when `deleteDocuments()` removes at least one document.
</issue_to_address>| TermPtr term = newLucene<Term>(context.profile().pathField(), path.toStdWString()); | ||
| writer->deleteDocuments(term); | ||
| if (reporter) { | ||
| reporter->markIndexChanged(); |
There was a problem hiding this comment.
🟡 Medium · No-op deletion advances timestamp
When a removal request names a path with no indexed document, removeFile() calls markIndexChanged() after deleteDocuments() matches nothing, so RemoveFileListHandler returns indexChanged=true and TaskManager::updateIndexStatusOnSuccess() saves a new update time for an unchanged index.
Mark the index changed only when deleteDocuments() removes at least one document.
Prompt for AI agents
In `src/index/task/taskhandler.cpp` at line 468:
**No-op deletion advances timestamp**
When a removal request names a path with no indexed document, `removeFile()` calls `markIndexChanged()` after `deleteDocuments()` matches nothing, so `RemoveFileListHandler` returns `indexChanged=true` and `TaskManager::updateIndexStatusOnSuccess()` saves a new update time for an unchanged index.
Mark the index changed only when `deleteDocuments()` removes at least one document.
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 语法正确,逻辑清晰 ✅ 通过 潜在问题: 建议: [] 2. 代码质量 ✅评价: 代码结构清晰,注释完整 ✅ 通过 潜在问题: 建议: [] 3. 代码性能 ✅评价: 性能良好,资源使用合理 ✅ 通过 潜在问题: 建议: [] 4. 代码安全 🔒评价: 存在0个安全漏洞 ✅ 通过
安全漏洞详情: 建议: [] 💡 改进建议代码示例本报告由 AI 代码审查工具自动生成 |
|
/forcemerge |
Root Cause Analysis
removeFile()insrc/index/task/taskhandler.cppdeleted index documents viawriter->deleteDocuments(term)but only calledreporter->increment(), missingreporter->markIndexChanged(). This leftindexChanged=false, soTaskManager::updateIndexStatusOnSuccess()skippedsaveLastUpdateTime()after file deletion, while theindexStatusChangedsignal was still emitted unconditionally — the UI refreshed but the index update time stayed unchanged. Sibling functionsupdateFile(),cleanupIndexs(), andremoveDirectoryIndex()all correctly callmarkIndexChanged();removeFile()is the only one that omits it (introduced missing in migration commit8d481bc). Key evidence:taskhandler.cpp:466-468(the omission),taskmanager.cpp:1050-1058(the skip logic), demo verification confirmedindexChanged=falsebefore fix andtrueafter.Fix
Add
reporter->markIndexChanged()inremoveFile(), afterwriter->deleteDocuments(term)and beforereporter->increment(), matching the pattern used byupdateFile()andremoveDirectoryIndex(). A single-line change, fully consistent with the stage-2 analysis report's recommended root-cause fix (section 10).Change Safety Assessment
Code Safety
markIndexChanged()call inside the existingif (reporter)block ofremoveFile().markIndexChanged()only setsm_indexChanged = true(taskhandler.cpp:201) with no side effects; no signature, public member, or call-site change.8d481bc(one-time migration), not a prior bug fix — this change does not revert any historical fix. Only 2 callers (taskhandler.cpp:1322,:1327), both ≤5 references, both compatible: the directory-deletion path already marksindexChangedviaremoveDirectoryIndex()(idempotent, no conflict); the file-deletion path is the intended fix target.Business Impact Scope
Affects the file index deletion module: deleting files/directories from the index and refreshing index status. Callers
RemoveFileListHandler(taskhandler.cpp:1322directory path,:1327file path) map to the user scenario where deleting a file in the desktop should refresh the dde-file-manager index "update time". After the fix, file deletion correctly triggerssaveLastUpdateTime()so the timestamp refreshes; directory deletion remains correct (already marked).Verification Suggestion
Verify: (1) delete a single file → index "update time" refreshes; (2) delete a directory → update time still refreshes (regression); (3) batch file deletion → no regression. The
autotests/index/test_taskhandler_withindex.cpp:189test covers theremoveFile()call path but only assertsSUCCEED()(noindexChangedassertion), so it will not break.根因分析
src/index/task/taskhandler.cpp中的removeFile()通过writer->deleteDocuments(term)删除索引文档,但只调用了reporter->increment(),遗漏了reporter->markIndexChanged()。这导致indexChanged=false,TaskManager::updateIndexStatusOnSuccess()在删除文件后跳过saveLastUpdateTime(),而indexStatusChanged信号仍无条件发射——界面刷新但索引更新时间不变。同文件updateFile()、cleanupIndexs()、removeDirectoryIndex()均正确调用了markIndexChanged(),唯独removeFile()遗漏(迁移 commit8d481bc引入时即缺失)。关键证据:taskhandler.cpp:466-468(遗漏点)、taskmanager.cpp:1050-1058(跳过逻辑)、demo 验证确认修复前indexChanged=false、修复后true。修复方案
在
removeFile()中,于writer->deleteDocuments(term)之后、reporter->increment()之前添加reporter->markIndexChanged(),与updateFile()、removeDirectoryIndex()保持一致。单行改动,与阶段 2 分析报告第十节推荐的根因层方案完全一致。改动安全评估
代码安全评估
removeFile()的if (reporter)块内添加一行markIndexChanged()调用。markIndexChanged()仅设置m_indexChanged=true(taskhandler.cpp:201),无副作用;不修改函数签名、公开成员或调用点。8d481bc(一次性迁移产物),非历史修复——本次改动不撤销任何已有修复。仅 2 处调用者(taskhandler.cpp:1322、:1327),均兼容:删除目录路径已通过removeDirectoryIndex()标记indexChanged(幂等无冲突);删除文件路径为修复目标。业务影响范围
影响文件索引删除模块:删除文件/目录时从索引移除文档并更新索引状态。调用者
RemoveFileListHandler(taskhandler.cpp:1322目录路径、:1327文件路径)对应用户在桌面删除文件后应刷新 dde-file-manager 索引"更新时间"的场景。修复后删除文件正确触发saveLastUpdateTime(),时间戳刷新;删除目录保持正确(已标记)。验证建议
验证:(1) 删除单个文件 → 索引"更新时间"刷新;(2) 删除目录 → 更新时间仍刷新(回归);(3) 批量删除文件 → 无回归。
autotests/index/test_taskhandler_withindex.cpp:189测试覆盖removeFile()调用路径,但仅断言SUCCEED()(无indexChanged断言),不会因本次改动失败。Summary by Sourcery
Bug Fixes: