From 4f24ee0422ca7dfc38f3fb9fbb17adb63646bec5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Cie=C5=9Blak?= <michalcieslak@status.im>
Date: Wed, 19 Jun 2024 15:29:42 +0200
Subject: [PATCH] fix(ObjectProxyModel): source data change handling improved

- source dataChanged rage is checked and skipped if malformed
- dataChange is not propagated to delegate for rows not accessed so far
---
 ui/StatusQ/src/objectproxymodel.cpp       | 19 ++++---
 ui/StatusQ/tests/tst_ObjectProxyModel.cpp | 69 ++++++++++++++++++++++-
 2 files changed, 79 insertions(+), 9 deletions(-)

diff --git a/ui/StatusQ/src/objectproxymodel.cpp b/ui/StatusQ/src/objectproxymodel.cpp
index 3c04e03204..fec5da5b33 100644
--- a/ui/StatusQ/src/objectproxymodel.cpp
+++ b/ui/StatusQ/src/objectproxymodel.cpp
@@ -61,20 +61,25 @@ void ObjectProxyModel::setSourceModel(QAbstractItemModel* model)
     });
 
     connect(model, &QAbstractItemModel::dataChanged, this,
-            [this](const QModelIndex& topLeft, const QModelIndex& bottomRight,
-            const QVector<int>& roles)
+            [this, model](const QModelIndex& topLeft,
+                const QModelIndex& bottomRight, const QVector<int>& roles)
     {
+        if (!topLeft.isValid() || !bottomRight.isValid())
+            return;
+
         auto first = topLeft.row();
         auto last = bottomRight.row();
 
-        auto model = sourceModel();
-
         for (auto idx = first; idx <= last; idx++) {
             auto rowData = m_container[idx].rowData;
 
+            if (rowData == nullptr)
+                continue;
+
             QHashIterator i(m_expectedRoleNames);
             while (i.hasNext()) {
                 i.next();
+
                 rowData->insert(i.value(),
                                 model->data(model->index(idx, 0), i.key()));
             }
@@ -232,12 +237,12 @@ void ObjectProxyModel::emitAllDataChanged()
     if (count == 0)
         return;
 
+    if (m_expectedRoles.isEmpty())
+        return;
+
     QVector<int> roles(m_exposedRolesSet.cbegin(),
                        m_exposedRolesSet.cend());
 
-    if (roles.empty())
-        return;
-
     emit this->dataChanged(index(0, 0), index(count - 1, 0), roles);
 }
 
diff --git a/ui/StatusQ/tests/tst_ObjectProxyModel.cpp b/ui/StatusQ/tests/tst_ObjectProxyModel.cpp
index d065d2a6ea..439017a6a5 100644
--- a/ui/StatusQ/tests/tst_ObjectProxyModel.cpp
+++ b/ui/StatusQ/tests/tst_ObjectProxyModel.cpp
@@ -313,7 +313,6 @@ private slots:
 
         ModelSignalsSpy signalsSpy(&model);
 
-
         QObject* proxy = model.proxyObject(0);
         proxy->setProperty("extraValue", 42);
 
@@ -376,7 +375,6 @@ private slots:
 
         QObject* proxy = model.proxyObject(0);
 
-
         // dataChanged signal emission is scheduled to event loop, not called
         // immediately. In the meantime the source may be cleared and then no
         // dataChanged event should be emited.
@@ -681,6 +679,73 @@ private slots:
         QCOMPARE(model.rowCount(), 0);
         QCOMPARE(model.roleNames().size(), 0);
     }
+
+    void sourceModelDataChangeTest() {
+        QQmlEngine engine;
+        QQmlComponent delegate(&engine);
+
+        auto delegateData = R"(
+            import QtQml 2.15
+            QtObject {
+                readonly property int doubledBalance: model.balance * 2
+            }
+        )";
+
+        delegate.setData(delegateData, QUrl());
+
+        ObjectProxyModel model;
+
+        auto source = R"([
+            { balance: 4 },
+            { balance: 10 }
+        ])";
+
+        ListModelWrapper sourceModel(engine, source);
+
+        model.setSourceModel(sourceModel);
+        model.setDelegate(&delegate);
+
+        model.setExpectedRoles(QStringList({ QStringLiteral("balance") }));
+        model.setExposedRoles({ QStringLiteral("doubledBalance")});
+
+        ModelSignalsSpy signalsSpy(&model);
+
+        sourceModel.setProperty(0, "balance", 42);
+        sourceModel.setProperty(1, "balance", 0);
+
+        {
+            ListModelWrapper expected(engine, R"([
+                { balance: 42, doubledBalance: 84 },
+                { balance: 0, doubledBalance: 0 }
+            ])");
+
+            QVERIFY(isSame(&model, expected));
+            QCOMPARE(signalsSpy.count(), 2);
+            QCOMPARE(signalsSpy.dataChangedSpy.count(), 2);
+        }
+
+        QTest::qWait(100);
+        QCOMPARE(signalsSpy.count(), 2);
+        QCOMPARE(signalsSpy.dataChangedSpy.count(), 2);
+
+        sourceModel.setProperty(0, "balance", 1);
+        sourceModel.setProperty(1, "balance", 2);
+
+        {
+            ListModelWrapper expected(engine, R"([
+                { balance: 1, doubledBalance: 2 },
+                { balance: 2, doubledBalance: 4 }
+            ])");
+
+            QVERIFY(isSame(&model, expected));
+            QCOMPARE(signalsSpy.count(), 4);
+            QCOMPARE(signalsSpy.dataChangedSpy.count(), 4);
+        }
+
+        QTest::qWait(100);
+        QCOMPARE(signalsSpy.count(), 5);
+        QCOMPARE(signalsSpy.dataChangedSpy.count(), 5);
+    }
 };
 
 QTEST_MAIN(TestObjectProxyModel)