Repository navigation
fix: possible proxy model desync bug on model reset - #1695
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideImplementationRebuild RoleCombineModel’s index map and emit modelReset when the major source resets, preventing stale mappings from surviving a reset.
Adjust the rowsRemoved iteration after deleting an empty group so a subsequent group is not skipped when handling a multi-row removal.
TestsAdd reset helpers and tests covering RoleCombineModel behavior after major and minor source model resets.
Add coverage for range-based row removal and child-level dataChanged indexes in RoleGroupModel.
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
TAG Bot New tag: 2.0.53 |
|
TAG Bot New tag: 2.0.54 |
|
TAG Bot New tag: 2.0.55 |
|
TAG Bot New tag: 2.0.56 |
|
TAG Bot New tag: 2.0.57 |
|
TAG Bot New tag: 2.0.58 |
|
TAG Bot New tag: 2.0.59 |
|
TAG Bot New tag: 2.0.60 |
|
TAG Bot New tag: 2.0.61 |
Bug 1 (RoleCombineModel 缺少 modelReset 转发) — 可触发 AbstractWindowMonitor::clearTrackedWindows() 会调用 beginResetModel() / endResetModel()，在以下时机被调用： treelandwindowmonitor.cpp:105 和 x11windowmonitor.cpp:94 — 当显示服务器重连或 compositor 重启时，窗口监视器会清空所有跟踪窗口并复位模型 这个 modelReset 会穿过 BoolFilterModel 到达 RoleCombineModel 的 major model。修复前，复位后 m_indexMap 残留旧映射且下游收不到 reset 信号，导致新旧数据冲突。这是一个生产环境可触发的 bug。 Bug 2 (RoleGroupModel rowsRemoved 跳过后续分组) — 当前不可触发，属于防御性修复 RoleGroupModel 的源模型是 DockGlobalElementModel，其所有删除路径（destroyWindow 单窗口关闭、DockGlobalElementModel 内部循环）都是逐个调用 beginRemoveRows(pos, pos) / endRemoveRows()，每次只有 first == last。因此 range 分支在当前代码中永远不会被执行，--i 缺失不会造成问题。但 handler 的接口签名支持 first..last 范围，LargeDataMemoryStabilityTest 测试也直接传入了范围，若未来有代码路径直接传入多行范围，就会触发。这是一个防御性修复。 Log:
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 语法正确，逻辑清晰 ✅ 通过 潜在问题： 建议： ['无'] 2. 代码质量 ✅评价： 代码结构清晰，注释基本完整，存在少量测试注释问题 ✅ 通过 潜在问题： 建议： ["更新 rolecombinemodeltests.cpp 第139行注释，移除'BUG: 当前实现不会发出此信号，此断言会 FAIL'，改为说明修复后的预期行为", '增强 MinorModelResetForwarding 测试：要么在实现中添加 minor modelReset 连接并用 QSignalSpy 验证，要么使用不同索引的数据使测试能真正暴露 stale mapping 问题', '考虑将测试注释翻译为英文以保持代码库风格一致'] 3. 代码性能 ✅评价： 性能良好，资源使用合理 ✅ 通过 潜在问题： 建议： ['无'] 4. 代码安全 🔒评价： 存在0个安全漏洞 ✅ 通过
安全漏洞详情： 建议： ['无'] 💡 改进建议代码示例// 修复 1: 更新过时的 BUG 注释 (rolecombinemodeltests.cpp)
// 将第139行:
// // BUG: 当前实现不会发出此信号，此断言会 FAIL
// EXPECT_EQ(resetSpy.count(), 1) << "FAIL: RoleCombineModel did not forward modelReset from major source";
// 改为:
// // 修复后: RoleCombineModel 现在会转发 major 源的 modelReset 信号
// EXPECT_EQ(resetSpy.count(), 1) << "RoleCombineModel should forward modelReset from major source";
// 修复 2: 增强 MinorModelResetForwarding 测试，使用不同索引暴露 stale mapping
TEST(RoleCombineModel, MinorModelResetForwarding)
{
TestModelA modelA;
TestModelB modelB;
auto combineFunc = [](QVariant data, QAbstractItemModel *model) -> QModelIndex {
auto matches = model->match(model->index(0, 0), TestModelB::idRole, data);
return matches.isEmpty() ? QModelIndex() : matches.first();
};
RoleCombineModel model(&modelA, &modelB, TestModelA::idRole, combineFunc);
modelA.addData(new DataA(0, "a0", &modelA));
modelA.addData(new DataA(1, "a1", &modelA));
modelB.addData(new DataB(0, "b0", &modelB));
modelB.addData(new DataB(1, "b1", &modelB));
ASSERT_EQ(model.rowCount(), 2);
// 清空 minor 后用不同数据重新填充，暴露 stale mapping
modelB.clear();
modelB.addData(new DataB(0, "b0_new", &modelB));
modelB.addData(new DataB(1, "b1_new", &modelB));
auto roleNames = model.roleNames();
auto roleNamesB = modelB.roleNames();
QHash<QByteArray, int> names2Role;
for (auto it = roleNames.constBegin(); it != roleNames.constEnd(); ++it) {
names2Role.insert(it.value(), it.key());
}
int bDataRole = names2Role.value(roleNamesB.value(TestModelB::dataRole));
// 若 minor modelReset 未连接，m_indexMap 保留旧映射，
// 新数据可能无法正确映射——此断言会暴露问题
EXPECT_EQ(model.index(0, 0).data(bDataRole).toString(), "b0_new");
EXPECT_EQ(model.index(1, 0).data(bDataRole).toString(), "b1_new");
}本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, wjyrich 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 |
Bug 1 (RoleCombineModel 缺少 modelReset 转发) — 可触发
AbstractWindowMonitor::clearTrackedWindows() 会调用 beginResetModel() / endResetModel()，在以下时机被调用：
treelandwindowmonitor.cpp:105 和 x11windowmonitor.cpp:94 — 当显示服务器重连或 compositor 重启时，窗口监视器会清空所有跟踪窗口并复位模型 这个 modelReset 会穿过 BoolFilterModel 到达 RoleCombineModel 的 major model。修复前，复位后 m_indexMap 残留旧映射且下游收不到 reset 信号，导致新旧数据冲突。这是一个生产环境可触发的 bug。
Bug 2 (RoleGroupModel rowsRemoved 跳过后续分组) — 当前不可触发，属于防御性修复
RoleGroupModel 的源模型是 DockGlobalElementModel，其所有删除路径（destroyWindow 单窗口关闭、DockGlobalElementModel 内部循环）都是逐个调用 beginRemoveRows(pos, pos) / endRemoveRows()，每次只有 first == last。因此 range 分支在当前代码中永远不会被执行，--i 缺失不会造成问题。但 handler 的接口签名支持 first..last 范围，LargeDataMemoryStabilityTest 测试也直接传入了范围，若未来有代码路径直接传入多行范围，就会触发。这是一个防御性修复。
Log:
Summary by Sourcery
Keep proxy models synchronized across source resets and grouped-row removals.
Bug Fixes:
Tests: