QAbstractProxyModel: port to new property system

The biggest trick here is the getter (QAbstractProxyModel::sourceModel),
which is returning nullptr, while internally using a global
staticEmptyModel() instance.
This lead to inconsistency while binding to a proxy model without
source model. The bound object would point to staticEmptyModel()
instance, while sourceModel() getter returns nullptr.
To solve this issue a custom QBindableInterface is implemented.

Task-number: QTBUG-85520
Change-Id: I597df891c7e425d51b55f50ccbacabdfe935cbac
Reviewed-by: Sona Kurazyan <sona.kurazyan@qt.io>

src/corelib/itemmodels/qabstractproxymodel.cpp

@@ -128,17 +128,22 @@ QAbstractProxyModel::~QAbstractProxyModel()
 void QAbstractProxyModel::setSourceModel(QAbstractItemModel *sourceModel)
 {
     Q_D(QAbstractProxyModel);
+    d->model.removeBindingUnlessInWrapper();
+    // Special case to handle nullptr models. Otherwise we will have unwanted
+    // notifications.
+    if (!sourceModel && d->model == QAbstractItemModelPrivate::staticEmptyModel())
+        return;
     if (sourceModel != d->model) {
         if (d->model)
             disconnect(d->model, SIGNAL(destroyed()), this, SLOT(_q_sourceModelDestroyed()));
 
         if (sourceModel) {
-            d->model = sourceModel;
+            d->model.setValueBypassingBindings(sourceModel);
             connect(d->model, SIGNAL(destroyed()), this, SLOT(_q_sourceModelDestroyed()));
         } else {
-            d->model = QAbstractItemModelPrivate::staticEmptyModel();
+            d->model.setValueBypassingBindings(QAbstractItemModelPrivate::staticEmptyModel());
         }
-        emit sourceModelChanged(QPrivateSignal());
+        d->model.notify();
     }
 }
 
@@ -153,6 +158,13 @@ QAbstractItemModel *QAbstractProxyModel::sourceModel() const
     return d->model;
 }
 
+QBindable<QAbstractItemModel *> QAbstractProxyModel::bindableSourceModel()
+{
+    Q_D(QAbstractProxyModel);
+    return QBindable<QAbstractItemModel *>(QAbstractProxyModelBindable(
+            &d->model, &QtPrivate::QBindableInterfaceForSourceModel::iface));
+}
+
 /*!
     \reimp
  */

src/corelib/itemmodels/qabstractproxymodel.h

@@ -52,7 +52,8 @@ class QItemSelection;
 class Q_CORE_EXPORT QAbstractProxyModel : public QAbstractItemModel
 {
     Q_OBJECT
-    Q_PROPERTY(QAbstractItemModel* sourceModel READ sourceModel WRITE setSourceModel NOTIFY sourceModelChanged)
+    Q_PROPERTY(QAbstractItemModel *sourceModel READ sourceModel WRITE setSourceModel NOTIFY
+                       sourceModelChanged BINDABLE bindableSourceModel)
 
 public:
     explicit QAbstractProxyModel(QObject *parent = nullptr);
@@ -60,6 +61,7 @@ public:
 
     virtual void setSourceModel(QAbstractItemModel *sourceModel);
     QAbstractItemModel *sourceModel() const;
+    QBindable<QAbstractItemModel *> bindableSourceModel();
 
     Q_INVOKABLE virtual QModelIndex mapToSource(const QModelIndex &proxyIndex) const = 0;
     Q_INVOKABLE virtual QModelIndex mapFromSource(const QModelIndex &sourceIndex) const = 0;

src/corelib/itemmodels/qabstractproxymodel_p.h

@@ -53,22 +53,98 @@
 //
 
 #include "private/qabstractitemmodel_p.h"
+#include "private/qproperty_p.h"
 
 QT_REQUIRE_CONFIG(proxymodel);
 
 QT_BEGIN_NAMESPACE
 
+class QAbstractProxyModelBindable : public QUntypedBindable
+{
+public:
+    explicit QAbstractProxyModelBindable(QUntypedPropertyData *d,
+                                         const QtPrivate::QBindableInterface *i)
+        : QUntypedBindable(d, i)
+    {
+    }
+};
+
 class Q_CORE_EXPORT QAbstractProxyModelPrivate : public QAbstractItemModelPrivate
 {
     Q_DECLARE_PUBLIC(QAbstractProxyModel)
 public:
-    QAbstractProxyModelPrivate() : QAbstractItemModelPrivate(), model(nullptr) {}
-    QAbstractItemModel *model;
+    QAbstractProxyModelPrivate() : QAbstractItemModelPrivate() { }
+    void setModelForwarder(QAbstractItemModel *sourceModel)
+    {
+        q_func()->setSourceModel(sourceModel);
+    }
+    void modelChangedForwarder()
+    {
+        Q_EMIT q_func()->sourceModelChanged(QAbstractProxyModel::QPrivateSignal());
+    }
+    Q_OBJECT_COMPAT_PROPERTY_WITH_ARGS(QAbstractProxyModelPrivate, QAbstractItemModel *, model,
+                                       &QAbstractProxyModelPrivate::setModelForwarder,
+                                       &QAbstractProxyModelPrivate::modelChangedForwarder, nullptr)
     virtual void _q_sourceModelDestroyed();
     void mapDropCoordinatesToSource(int row, int column, const QModelIndex &parent,
                                     int *source_row, int *source_column, QModelIndex *source_parent) const;
+
+    using ModelPropertyType = decltype(model);
 };
 
+namespace QtPrivate {
+
+/*!
+    The biggest trick for adding new QProperty binding support here is the
+    getter for the sourceModel property (QAbstractProxyModel::sourceModel),
+    which returns nullptr, while internally using a global staticEmptyModel()
+    instance.
+    This lead to inconsistency while binding to a proxy model without source
+    model. The bound object would point to staticEmptyModel() instance, while
+    sourceModel() getter returns nullptr.
+    To solve this issue we need to implement a custom QBindableInterface, with
+    custom getter and makeBinding methods, that would introduce the required
+    logic.
+*/
+
+inline QAbstractItemModel *normalizePotentiallyEmptyModel(QAbstractItemModel *model)
+{
+    if (model == QAbstractItemModelPrivate::staticEmptyModel())
+        return nullptr;
+    return model;
+}
+
+class QBindableInterfaceForSourceModel
+{
+    using PropertyType = QAbstractProxyModelPrivate::ModelPropertyType;
+    using Parent = QBindableInterfaceForProperty<PropertyType>;
+    using T = typename PropertyType::value_type;
+
+public:
+    static constexpr QBindableInterface iface = {
+        [](const QUntypedPropertyData *d, void *value) -> void {
+            const auto val = static_cast<const PropertyType *>(d)->value();
+            *static_cast<T *>(value) = normalizePotentiallyEmptyModel(val);
+        },
+        Parent::iface.setter,
+        Parent::iface.getBinding,
+        Parent::iface.setBinding,
+        [](const QUntypedPropertyData *d,
+           const QPropertyBindingSourceLocation &location) -> QUntypedPropertyBinding {
+            return Qt::makePropertyBinding(
+                    [d]() -> T {
+                        return normalizePotentiallyEmptyModel(
+                                static_cast<const PropertyType *>(d)->value());
+                    },
+                    location);
+        },
+        Parent::iface.setObserver,
+        Parent::iface.metaType
+    };
+};
+
+} // namespace QtPrivate
+
 QT_END_NAMESPACE
 
 #endif // QABSTRACTPROXYMODEL_P_H

tests/auto/corelib/itemmodels/qabstractproxymodel/CMakeLists.txt

@@ -9,6 +9,7 @@ qt_internal_add_test(tst_qabstractproxymodel
         tst_qabstractproxymodel.cpp
     DEFINES
         QT_DISABLE_DEPRECATED_BEFORE=0
-    PUBLIC_LIBRARIES
+    LIBRARIES
         Qt::Gui
+        Qt::TestPrivate
 )

tests/auto/corelib/itemmodels/qabstractproxymodel/tst_qabstractproxymodel.cpp

@@ -1,6 +1,6 @@
 /****************************************************************************
 **
-** Copyright (C) 2016 The Qt Company Ltd.
+** Copyright (C) 2021 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the test suite of the Qt Toolkit.
@@ -27,6 +27,7 @@
 ****************************************************************************/
 
 #include <QTest>
+#include <QtTest/private/qpropertytesthelper_p.h>
 #include <qabstractproxymodel.h>
 #include <QItemSelection>
 #include <qstandarditemmodel.h>
@@ -60,6 +61,7 @@ private slots:
     void testRoleNames();
     void testSwappingRowsProxy();
     void testDragAndDrop();
+    void sourceModelBinding();
 };
 
 // Subclass that exposes the protected functions.
@@ -500,6 +502,34 @@ void tst_QAbstractProxyModel::testDragAndDrop()
     QCOMPARE(proxy.supportedDropActions(), sourceModel.supportedDropActions());
 }
 
+void tst_QAbstractProxyModel::sourceModelBinding()
+{
+    SubQAbstractProxyModel proxy;
+    QStandardItemModel model1;
+    QStandardItemModel model2;
+    QTestPrivate::testReadWritePropertyBasics<QAbstractProxyModel, QAbstractItemModel *>(
+            proxy, &model1, &model2, "sourceModel");
+    if (QTest::currentTestFailed()) {
+        qDebug("Failed model - model test");
+        return;
+    }
+
+    proxy.setSourceModel(&model2);
+    QTestPrivate::testReadWritePropertyBasics<QAbstractProxyModel, QAbstractItemModel *>(
+            proxy, &model1, nullptr, "sourceModel");
+    if (QTest::currentTestFailed()) {
+        qDebug("Failed model - nullptr test");
+        return;
+    }
+
+    proxy.setSourceModel(&model1);
+    QTestPrivate::testReadWritePropertyBasics<QAbstractProxyModel, QAbstractItemModel *>(
+            proxy, nullptr, &model2, "sourceModel");
+    if (QTest::currentTestFailed()) {
+        qDebug("Failed nullptr - model test");
+        return;
+    }
+}
 
 QTEST_MAIN(tst_QAbstractProxyModel)
 #include "tst_qabstractproxymodel.moc"