fix(StatusQ/ConcatModel): Handle source submodel reset properly, tests amended

Closes: #12974

ui/StatusQ/src/concatmodel.cpp

@@ -303,7 +303,7 @@ QVariant ConcatModel::data(const QModelIndex &index, int role) const
             if (model == nullptr)
                 return {};
 
-            auto& mapping = m_rolesMappingToSource.at(i);
+            auto& mapping = m_rolesMappingToSource[i];
             auto it = mapping.find(role);
 
             if (it == mapping.end())
@@ -517,7 +517,7 @@ void ConcatModel::initRolesMapping()
     m_rolesMappingInitializationFlags.resize(m_sources.size(), false);
 
     for (auto i = 0; i < m_sources.size(); ++i) {
-        auto sourceModelWrapper = m_sources.at(i);
+        auto sourceModelWrapper = m_sources[i];
         auto sourceModel = sourceModelWrapper->model();
 
         if (sourceModel == nullptr)
@@ -530,7 +530,7 @@ void ConcatModel::initRolesMapping()
 void ConcatModel::initAllModelsSlots()
 {
     for (auto sourceIndex = 0; sourceIndex < m_sources.size(); ++sourceIndex) {
-        auto sourceModelWrapper = m_sources.at(sourceIndex);
+        auto sourceModelWrapper = m_sources[sourceIndex];
         auto sourceModel = sourceModelWrapper->model();
 
         if (sourceModel)
@@ -556,7 +556,7 @@ void ConcatModel::connectModelSlots(int index, QAbstractItemModel *model)
             initRoles();
             initRolesMapping();
             m_initialized = true;
-        } else if (!m_rolesMappingInitializationFlags.at(index)) {
+        } else if (!m_rolesMappingInitializationFlags[index]) {
             initRolesMapping(index, model);
         }
 
@@ -603,17 +603,54 @@ void ConcatModel::connectModelSlots(int index, QAbstractItemModel *model)
         emit this->layoutChanged();
     });
 
-    connect(model, &QAbstractItemModel::modelAboutToBeReset, this, [this]
+    connect(model, &QAbstractItemModel::modelAboutToBeReset, this, [this, index]
     {
-        this->beginResetModel();
+        if (!m_initialized)
+            return;
+
+        auto currentCount = m_rowCounts[index];
+
+        if (currentCount) {
+            auto prefix = this->countPrefix(index);
+            this->beginRemoveRows({}, prefix, prefix + currentCount - 1);
+        }
     });
 
     connect(model, &QAbstractItemModel::modelReset, this, [this, model, index]
     {
-        m_rowCounts[index] = model->rowCount();
+        auto count = model->rowCount();
-        m_rolesMappingInitializationFlags[index] = false;
 
-        this->endResetModel();
+        if (!m_initialized) {
+            if (count) {
+                this->beginInsertRows({}, 0, count - 1);
+
+                initRoles();
+                initRolesMapping();
+                m_initialized = true;
+
+                m_rowCounts[index] = count;
+
+                this->endInsertRows();
+            }
+        } else {
+            auto previousCount = m_rowCounts[index];
+
+            if (previousCount) {
+                m_rowCounts[index] = 0;
+                this->endRemoveRows();
+            }
+
+            initRolesMapping(index, model);
+
+            if (count) {
+                auto prefix = this->countPrefix(index);
+                this->beginInsertRows({}, prefix, prefix + count - 1);
+
+                m_rowCounts[index] = count;
+
+                this->endInsertRows();
+            }
+        }
     });
 
     connect(model, &QAbstractItemModel::dataChanged, this,
@@ -655,7 +692,7 @@ void ConcatModel::fetchRowCounts()
     m_rowCounts.resize(m_sources.size());
 
     for (auto i = 0; i < m_sources.size(); ++i) {
-        auto sourceModelWrapper = m_sources.at(i);
+        auto sourceModelWrapper = m_sources[i];
         auto sourceModel = sourceModelWrapper->model();
 
         m_rowCounts[i] = (sourceModel == nullptr) ? 0 : sourceModel->rowCount();

ui/StatusQ/tests/tst_ConcatModel.cpp

@@ -1,3 +1,4 @@
+#include <QIdentityProxyModel>
 #include <QSignalSpy>
 #include <QTest>
 
@@ -17,6 +18,16 @@
 
 namespace {
 
+// Workaround for a bug in QIdentityProxyModel (returning roleNames improperly)
+class IdentityModel : public QIdentityProxyModel {
+public:
+    QHash<int,QByteArray>  roleNames() const override {
+        if (sourceModel())
+            return sourceModel()->roleNames();
+        return {};
+    }
+};
+
 class ListModelWrapper {
 
 public:
@@ -103,7 +114,7 @@ public:
     }
 
     void clear() {
-        runExpression(QString("append()"));
+        runExpression(QString("clear()"));
     }
 
     void remove(int index, int count = 1) {
@@ -1609,9 +1620,285 @@ private slots:
         QCOMPARE(layoutChangedSpy.count(), 1);
     }
 
-    void layoutResetTest()
+    void modelResetWhenEmptyTest()
     {
-        // TODO
+        QQmlEngine engine;
+        ConcatModel model;
+
+        ListModelWrapper sourceModel1(engine);
+        ListModelWrapper sourceModel2(engine);
+        ListModelWrapper sourceModel3(engine);
+        ListModelWrapper sourceModel4(engine);
+        ListModelWrapper sourceModel5(engine, QJsonArray {
+            QJsonObject {{ "key", 1}, { "color", "red" }},
+            QJsonObject {{ "key", 2}, { "color", "blue" }}
+        });
+
+        QQmlListProperty<SourceModel> sources = model.sources();
+
+        SourceModel source1, source2, source3;
+
+        IdentityModel proxy1, proxy2, proxy3;
+
+        proxy1.setSourceModel(sourceModel1.model());
+        proxy2.setSourceModel(sourceModel2.model());
+        proxy3.setSourceModel(sourceModel3.model());
+
+        source1.setModel(&proxy1);
+        source2.setModel(&proxy2);
+        source3.setModel(&proxy3);
+
+        sources.append(&sources, &source1);
+        sources.append(&sources, &source2);
+        sources.append(&sources, &source3);
+
+        QCOMPARE(model.rowCount(), 0);
+        QCOMPARE(model.roleNames(), {});
+        QCOMPARE(model.index(0, 0).isValid(), false);
+
+        model.componentComplete();
+
+        QCOMPARE(model.rowCount(), 0);
+        QCOMPARE(model.roleNames(), {});
+
+        {
+            QSignalSpy modelAboutToBeResetSpy(&model, &ConcatModel::modelAboutToBeReset);
+            QSignalSpy modelResetSpy(&model, &ConcatModel::modelReset);
+
+            QSignalSpy rowsAboutToBeInsertedSpy(&model, &ConcatModel::rowsAboutToBeInserted);
+            QSignalSpy rowsInsertedSpy(&model, &ConcatModel::rowsInserted);
+
+            proxy2.setSourceModel(sourceModel4.model());
+
+            QCOMPARE(modelAboutToBeResetSpy.count(), 0);
+            QCOMPARE(modelResetSpy.count(), 0);
+
+            QCOMPARE(rowsAboutToBeInsertedSpy.count(), 0);
+            QCOMPARE(rowsInsertedSpy.count(), 0);
+
+            QCOMPARE(model.rowCount(), 0);
+            QCOMPARE(model.roleNames(), {});
+        }
+        {
+            QSignalSpy modelAboutToBeResetSpy(&model, &ConcatModel::modelAboutToBeReset);
+            QSignalSpy modelResetSpy(&model, &ConcatModel::modelReset);
+
+            QSignalSpy rowsAboutToBeInsertedSpy(&model, &ConcatModel::rowsAboutToBeInserted);
+            QSignalSpy rowsInsertedSpy(&model, &ConcatModel::rowsInserted);
+
+            // checking validity inside rowsAboutToBeInserted signal
+            {
+                QObject context;
+                connect(&model, &ConcatModel::rowsAboutToBeInserted, &context,
+                        [&model] { QCOMPARE(model.rowCount(), 0); });
+
+                proxy2.setSourceModel(sourceModel5.model());
+            }
+
+            QCOMPARE(modelAboutToBeResetSpy.count(), 0);
+            QCOMPARE(modelResetSpy.count(), 0);
+
+            QCOMPARE(rowsAboutToBeInsertedSpy.count(), 1);
+            QCOMPARE(rowsAboutToBeInsertedSpy.at(0).at(0), QModelIndex{});
+            QCOMPARE(rowsAboutToBeInsertedSpy.at(0).at(1), 0);
+            QCOMPARE(rowsAboutToBeInsertedSpy.at(0).at(2), 1);
+
+            QCOMPARE(rowsInsertedSpy.count(), 1);
+            QCOMPARE(rowsInsertedSpy.at(0).at(0), QModelIndex{});
+            QCOMPARE(rowsInsertedSpy.at(0).at(1), 0);
+            QCOMPARE(rowsInsertedSpy.at(0).at(2), 1);
+
+            auto roles = model.roleNames();
+
+            QCOMPARE(model.rowCount(), 2);
+            QCOMPARE(roles.count(), 3);
+
+            QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "key")), 1);
+            QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "key")), 2);
+            QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "color")), "red");
+            QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "color")), "blue");
+        }
+    }
+
+    void modelResetWhenNotEmptyTest()
+    {
+        QQmlEngine engine;
+        ConcatModel model;
+
+        ListModelWrapper sourceModel1(engine, QJsonArray {
+            QJsonObject {{ "key", 1}, { "color", "red" }},
+            QJsonObject {{ "key", 2}, { "color", "blue" }}
+        });
+        ListModelWrapper sourceModel2(engine, QJsonArray {
+            QJsonObject {{ "key", 3}},
+            QJsonObject {{ "key", 4}},
+            QJsonObject {{ "key", 5}}
+        });
+        ListModelWrapper sourceModel3(engine);
+
+        ListModelWrapper sourceModel4(engine);
+        ListModelWrapper sourceModel5(engine, QJsonArray {
+            QJsonObject {{ "color", "red" }, { "name", "a" }, { "key", 11}},
+            QJsonObject {{ "color", "blue" }, { "name", "b" }, { "key", 12}},
+            QJsonObject {{ "color", "pink" }, { "name", "c" }, { "key", 13}}
+        });
+
+        QQmlListProperty<SourceModel> sources = model.sources();
+
+        SourceModel source1, source2, source3;
+
+        IdentityModel proxy1, proxy2, proxy3;
+
+        proxy1.setSourceModel(sourceModel1.model());
+        proxy2.setSourceModel(sourceModel2.model());
+        proxy3.setSourceModel(sourceModel3.model());
+
+        source1.setModel(&proxy1);
+        source2.setModel(&proxy2);
+        source3.setModel(&proxy3);
+
+        sources.append(&sources, &source1);
+        sources.append(&sources, &source2);
+        sources.append(&sources, &source3);
+
+        QCOMPARE(model.rowCount(), 0);
+        QCOMPARE(model.roleNames(), {});
+        QCOMPARE(model.index(0, 0).isValid(), false);
+
+        model.componentComplete();
+
+        auto roles = model.roleNames();
+
+        QCOMPARE(model.rowCount(), 5);
+        QCOMPARE(roles.count(), 3);
+
+        // reset to empty model
+        {
+            QSignalSpy modelAboutToBeResetSpy(&model, &ConcatModel::modelAboutToBeReset);
+            QSignalSpy modelResetSpy(&model, &ConcatModel::modelReset);
+
+            QSignalSpy rowsAboutToBeInsertedSpy(&model, &ConcatModel::rowsAboutToBeInserted);
+            QSignalSpy rowsInsertedSpy(&model, &ConcatModel::rowsInserted);
+
+            QSignalSpy rowsAboutToBeRemovedSpy(&model, &ConcatModel::rowsAboutToBeRemoved);
+            QSignalSpy rowsRemovedSpy(&model, &ConcatModel::rowsRemoved);
+
+            // checking validity inside rowsAboutToBeRemoved signal
+            {
+                QObject context;
+                connect(&model, &ConcatModel::rowsAboutToBeRemoved, &context,
+                        [this, &model, &roles] {
+                    QCOMPARE(model.rowCount(), 5);
+
+                    QCOMPARE(model.data(model.index(3, 0), roleForName(roles, "key")), 4);
+                    QCOMPARE(model.data(model.index(3, 0), roleForName(roles, "color")), {});
+                });
+
+                proxy2.setSourceModel(sourceModel4.model());
+            }
+
+            QCOMPARE(modelAboutToBeResetSpy.count(), 0);
+            QCOMPARE(modelResetSpy.count(), 0);
+
+            QCOMPARE(rowsAboutToBeInsertedSpy.count(), 0);
+            QCOMPARE(rowsInsertedSpy.count(), 0);
+
+            QCOMPARE(rowsAboutToBeRemovedSpy.count(), 1);
+            QCOMPARE(rowsRemovedSpy.count(), 1);
+            QCOMPARE(rowsAboutToBeRemovedSpy.at(0).at(0), QModelIndex{});
+            QCOMPARE(rowsAboutToBeRemovedSpy.at(0).at(1), 2);
+            QCOMPARE(rowsAboutToBeRemovedSpy.at(0).at(2), 4);
+
+            QCOMPARE(model.rowCount(), 2);
+
+            QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "key")), 1);
+            QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "key")), 2);
+            QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "color")), "red");
+            QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "color")), "blue");
+
+            // insert some data to check if roles are re-initialized properly
+            sourceModel4.append(QJsonArray {
+                QJsonObject {{ "color", "purple"}, { "key", 3} },
+                QJsonObject {{ "color", "green" }, { "key", 4}}
+            });
+
+            QCOMPARE(model.rowCount(), 4);
+
+            QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "key")), 1);
+            QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "key")), 2);
+            QCOMPARE(model.data(model.index(2, 0), roleForName(roles, "key")), 3);
+            QCOMPARE(model.data(model.index(3, 0), roleForName(roles, "key")), 4);
+            QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "color")), "red");
+            QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "color")), "blue");
+            QCOMPARE(model.data(model.index(2, 0), roleForName(roles, "color")), "purple");
+            QCOMPARE(model.data(model.index(3, 0), roleForName(roles, "color")), "green");
+
+            sourceModel4.clear();
+        }
+        // reset to not empty model
+        {
+            QSignalSpy modelAboutToBeResetSpy(&model, &ConcatModel::modelAboutToBeReset);
+            QSignalSpy modelResetSpy(&model, &ConcatModel::modelReset);
+
+            QSignalSpy rowsAboutToBeRemovedSpy(&model, &ConcatModel::rowsAboutToBeRemoved);
+            QSignalSpy rowsRemovedSpy(&model, &ConcatModel::rowsRemoved);
+
+            QSignalSpy rowsAboutToBeInsertedSpy(&model, &ConcatModel::rowsAboutToBeInserted);
+            QSignalSpy rowsInsertedSpy(&model, &ConcatModel::rowsInserted);
+
+            // checking validity inside rowsAboutToBeRemoved, rowsRemoved and
+            // rowsAboutToBeInserted signals
+            {
+                QObject context;
+                connect(&model, &ConcatModel::rowsAboutToBeRemoved, &context,
+                        [this, &model, &roles] {
+                    QCOMPARE(model.rowCount(), 2);
+
+                    QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "key")), 1);
+                    QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "key")), 2);
+                    QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "color")), "red");
+                    QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "color")), "blue");
+                });
+
+                connect(&model, &ConcatModel::rowsRemoved, &context,
+                        [this, &model, &roles] {
+                    QCOMPARE(model.rowCount(), 0);
+                });
+
+                connect(&model, &ConcatModel::rowsAboutToBeInserted, &context,
+                        [this, &model, &roles] {
+                    QCOMPARE(model.rowCount(), 0);
+                });
+
+                proxy1.setSourceModel(sourceModel5.model());
+            }
+
+            QCOMPARE(modelAboutToBeResetSpy.count(), 0);
+            QCOMPARE(modelResetSpy.count(), 0);
+
+            QCOMPARE(rowsAboutToBeRemovedSpy.count(), 1);
+            QCOMPARE(rowsAboutToBeRemovedSpy.at(0).at(0), QModelIndex{});
+            QCOMPARE(rowsAboutToBeRemovedSpy.at(0).at(1), 0);
+            QCOMPARE(rowsAboutToBeRemovedSpy.at(0).at(2), 1);
+
+            QCOMPARE(rowsRemovedSpy.count(), 1);
+
+            QCOMPARE(rowsAboutToBeInsertedSpy.count(), 1);
+            QCOMPARE(rowsAboutToBeInsertedSpy.at(0).at(0), QModelIndex{});
+            QCOMPARE(rowsAboutToBeInsertedSpy.at(0).at(1), 0);
+            QCOMPARE(rowsAboutToBeInsertedSpy.at(0).at(2), 2);
+
+            QCOMPARE(rowsInsertedSpy.count(), 1);
+
+            QCOMPARE(model.rowCount(), 3);
+
+            QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "key")), 11);
+            QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "key")), 12);
+            QCOMPARE(model.data(model.index(2, 0), roleForName(roles, "key")), 13);
+            QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "color")), "red");
+            QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "color")), "blue");
+            QCOMPARE(model.data(model.index(2, 0), roleForName(roles, "color")), "pink");
+        }
     }
 
     void sameModelUsedMultipleTimesTest()