From 2dfb61fe1cd9100e019c3b0894d444b6b6a53ec2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Cie=C5=9Blak?= <michalcieslak@status.im>
Date: Thu, 23 May 2024 21:57:40 +0200
Subject: [PATCH] fix(SubmodelProxyModel): Proxy model made non-intrusive

Now SubmodelProxyModel doesn't affect source model therefore it's
possible to safely use multiple SubmodelProxyModels over the same source
model.

Closes: #14550
---
 .../include/StatusQ/submodelproxymodel.h      |  4 +++
 ui/StatusQ/src/submodelproxymodel.cpp         | 36 ++++++++++---------
 ui/StatusQ/tests/tst_SubmodelProxyModel.cpp   |  3 --
 3 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/ui/StatusQ/include/StatusQ/submodelproxymodel.h b/ui/StatusQ/include/StatusQ/submodelproxymodel.h
index ebf2f2a1c8..8f72882a60 100644
--- a/ui/StatusQ/include/StatusQ/submodelproxymodel.h
+++ b/ui/StatusQ/include/StatusQ/submodelproxymodel.h
@@ -6,6 +6,8 @@
 #include <limits>
 #include <optional>
 
+#include "modelsyncedcontainer.h"
+
 class QQmlComponent;
 class QQmlEngine;
 
@@ -69,4 +71,6 @@ private:
     QHash<int, QByteArray> m_roleNames;
     QHash<QString, int> m_additionalRolesMap;
     int m_additionalRolesOffset = std::numeric_limits<int>::max();
+
+    mutable ModelSyncedContainer<std::unique_ptr<QObject>> m_container;
 };
diff --git a/ui/StatusQ/src/submodelproxymodel.cpp b/ui/StatusQ/src/submodelproxymodel.cpp
index 47ffe6430a..5fc039d957 100644
--- a/ui/StatusQ/src/submodelproxymodel.cpp
+++ b/ui/StatusQ/src/submodelproxymodel.cpp
@@ -26,8 +26,6 @@ SubmodelProxyModel::SubmodelProxyModel(QObject* parent)
 
 QVariant SubmodelProxyModel::data(const QModelIndex& index, int role) const
 {
-    static constexpr auto attachementPropertyName = "_attachement";
-
     if (!checkIndex(index, CheckIndexOption::IndexIsValid))
         return {};
 
@@ -41,31 +39,34 @@ QVariant SubmodelProxyModel::data(const QModelIndex& index, int role) const
             return submodel;
         }
 
-        QVariant attachement = submodelObj->property(attachementPropertyName);
+        QVariant proxyVariant;
 
-        if (attachement.isValid())
-            return attachement;
+        auto& entry = m_container[index.row()];
 
-        // Make sure that wrapper is destroyed before it receives signal related
-        // to submodel's destruction. Otherwise injected context property may
-        // be cleared causing warnings related to accessing null from qml.
-        connect(submodelObj, &QObject::destroyed, this, [](auto obj) {
-            QVariant attachement = obj->property(attachementPropertyName);
+        if (entry) {
+            proxyVariant = QVariant::fromValue(entry.get());
+        }
 
-            if (attachement.isValid())
-                delete attachement.value<QObject*>();
-        });
+        if (proxyVariant.isValid())
+            return proxyVariant;
 
         auto creationContext = m_delegateModel->creationContext();
         auto parentContext = creationContext
                 ? creationContext : m_delegateModel->engine()->rootContext();
 
         auto context = new QQmlContext(parentContext, submodelObj);
+
+        // Make sure that wrapper is destroyed before it receives signal related
+        // to submodel's destruction. Otherwise injected context property may
+        // be cleared causing warnings related to accessing null from qml.
+        connect(submodelObj, &QObject::destroyed, this,
+                [context = QPointer(context)](auto obj) {
+            delete context.data();
+        });
+
         context->setContextProperty(QStringLiteral("submodel"), submodel);
 
         QObject* instance = m_delegateModel->create(context);
-        instance->setParent(submodelObj);
-
         QVariant wrappedInstance = QVariant::fromValue(instance);
 
         if (m_additionalRolesMap.size()) {
@@ -77,8 +78,7 @@ QVariant SubmodelProxyModel::data(const QModelIndex& index, int role) const
                     this, SLOT(onCustomRoleChanged(QObject*,int)));
         }
 
-        submodelObj->setProperty(attachementPropertyName, wrappedInstance);
-
+        m_container[index.row()].reset(instance);
         return wrappedInstance;
     }
 
@@ -110,6 +110,8 @@ void SubmodelProxyModel::setSourceModel(QAbstractItemModel* model)
     if (sourceModel() == model)
         return;
 
+    m_container.setModel(model);
+
     // Workaround for QTBUG-57971
     if (model->roleNames().isEmpty())
         connect(model, &QAbstractItemModel::rowsInserted,
diff --git a/ui/StatusQ/tests/tst_SubmodelProxyModel.cpp b/ui/StatusQ/tests/tst_SubmodelProxyModel.cpp
index 3e1e10554d..0af4864c14 100644
--- a/ui/StatusQ/tests/tst_SubmodelProxyModel.cpp
+++ b/ui/StatusQ/tests/tst_SubmodelProxyModel.cpp
@@ -663,9 +663,6 @@ private slots:
     }
 
     void multipleProxiesTest() {
-        QSKIP("Not implemented yet. The goal is to make the proxy fully "
-              "non-intrusive what will fix the isse pointed in this test.");
-
         QQmlEngine engine;
         auto delegate1 = std::make_unique<QQmlComponent>(&engine);