From 2868d22a3a3b70ac7c91794a8bc869ed7c98b815 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Cie=C5=9Blak?= <michalcieslak@status.im>
Date: Wed, 5 Jun 2024 10:12:50 +0200
Subject: [PATCH] fix(ModelSyncedContainer): Proper disconnection from
 previously used model

---
 .../include/StatusQ/modelsyncedcontainer.h    | 28 +++++++++------
 ui/StatusQ/tests/tst_ModelSyncedContainer.cpp | 36 +++++++++++++++++++
 2 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/ui/StatusQ/include/StatusQ/modelsyncedcontainer.h b/ui/StatusQ/include/StatusQ/modelsyncedcontainer.h
index 0d6bd4e58d..7943654dd2 100644
--- a/ui/StatusQ/include/StatusQ/modelsyncedcontainer.h
+++ b/ui/StatusQ/include/StatusQ/modelsyncedcontainer.h
@@ -4,21 +4,26 @@
 #include <QObject>
 #include <QVariant>
 
+#include <memory>
+
 template<typename T>
 class ModelSyncedContainer
 {
-
 public:
     void setModel(QAbstractItemModel* model)
     {
         m_container.clear();
 
+        // create context object for connections, disconnect from previous model
+        // by destroying previous context object if present
+        m_ctx = std::make_unique<QObject>();
+
         if (model == nullptr)
             return;
 
         m_container.resize(model->rowCount());
 
-        QObject::connect(model, &QAbstractItemModel::rowsRemoved, &m_ctx,
+        QObject::connect(model, &QAbstractItemModel::rowsRemoved, m_ctx.get(),
                          [this] (const QModelIndex& parent, int first, int last)
         {
             if (parent.isValid())
@@ -28,7 +33,7 @@ public:
                               m_container.cbegin() + last + 1);
         });
 
-        QObject::connect(model, &QAbstractItemModel::rowsInserted, &m_ctx,
+        QObject::connect(model, &QAbstractItemModel::rowsInserted, m_ctx.get(),
                          [this] (const QModelIndex& parent, int first, int last)
         {
             if (parent.isValid())
@@ -45,8 +50,8 @@ public:
                                std::make_move_iterator(toBeInserted.end()));
         });
 
-        QObject::connect(model, &QAbstractItemModel::rowsAboutToBeMoved, &m_ctx,
-                         [this, model] (const QModelIndex& parent)
+        QObject::connect(model, &QAbstractItemModel::rowsAboutToBeMoved,
+                         m_ctx.get(), [this, model] (const QModelIndex& parent)
         {
             if (parent.isValid())
                 return;
@@ -55,7 +60,7 @@ public:
         });
 
         QObject::connect(model, &QAbstractItemModel::rowsMoved,
-                         &m_ctx, [this] (const QModelIndex& parent)
+                         m_ctx.get(), [this] (const QModelIndex& parent)
         {
             if (parent.isValid())
                 return;
@@ -67,26 +72,26 @@ public:
         });
 
         QObject::connect(model, &QAbstractItemModel::layoutAboutToBeChanged,
-                         &m_ctx, [this, model]
+                         m_ctx.get(), [this, model]
         {
             storePersistentIndexes(model);
         });
 
         QObject::connect(model, &QAbstractItemModel::layoutChanged,
-                         &m_ctx, [this]
+                         m_ctx.get(), [this]
         {
             updateFromPersistentIndexes();
         });
 
         QObject::connect(model, &QAbstractItemModel::modelReset,
-                         &m_ctx, [this, model]
+                         m_ctx.get(), [this, model]
         {
             m_container.clear();
             m_container.resize(model->rowCount());
         });
 
         QObject::connect(model, &QAbstractItemModel::destroyed,
-                         &m_ctx, [this, model]
+                         m_ctx.get(), [this, model]
         {
             m_container.clear();
         });
@@ -144,5 +149,6 @@ private:
 
     QList<QPersistentModelIndex> m_persistentIndexes;
     std::vector<T> m_container;
-    QObject m_ctx;
+
+    std::unique_ptr<QObject> m_ctx;
 };
diff --git a/ui/StatusQ/tests/tst_ModelSyncedContainer.cpp b/ui/StatusQ/tests/tst_ModelSyncedContainer.cpp
index f564665de8..4edc2034ff 100644
--- a/ui/StatusQ/tests/tst_ModelSyncedContainer.cpp
+++ b/ui/StatusQ/tests/tst_ModelSyncedContainer.cpp
@@ -59,6 +59,42 @@ private slots:
         QCOMPARE(container.size(), 0);
     }
 
+    void modelChangeDisconnectionTest()
+    {
+        struct Model : public QIdentityProxyModel
+        {
+            void connectNotify(const QMetaMethod&) override
+            {
+                connectionsCount++;
+            }
+
+            void disconnectNotify(const QMetaMethod&) override
+            {
+                connectionsCount--;
+            }
+
+            int connectionsCount = 0;
+        };
+
+        Model model1, model2;
+        ModelSyncedContainer<int> container;
+
+        QCOMPARE(model1.connectionsCount, 0);
+
+        container.setModel(&model1);
+        QVERIFY(model1.connectionsCount > 0);
+
+        container.setModel(nullptr);
+        QCOMPARE(model1.connectionsCount, 0);
+
+        container.setModel(&model1);
+        QVERIFY(model1.connectionsCount > 0);
+
+        container.setModel(&model2);
+        QCOMPARE(model1.connectionsCount, 0);
+        QVERIFY(model2.connectionsCount > 0);
+    }
+
     void appendTest()
     {
         QQmlEngine engine;