ThreePIDModel Updates

There is no need for NeochatConnection to depend on ThreePIdModel and also this means it's not in memory when not needed.

Also a little cleanup to make sure only a single job can run at a time.

autotests/messagecontentmodeltest.cpp

@@ -12,6 +12,7 @@
 
 #include "models/messagecontentmodel.h"
 
+#include "neochatconnection.h"
 #include "testutils.h"
 
 using namespace Quotient;
@@ -32,7 +33,7 @@ private Q_SLOTS:
 
 void MessageContentModelTest::initTestCase()
 {
-    connection = Connection::makeMockConnection(u"@bob:kde.org"_s);
+    connection = new NeoChatConnection;
 }
 
 void MessageContentModelTest::missingEvent()

src/models/threepidmodel.cpp

@@ -13,13 +13,35 @@
 
 using namespace Qt::StringLiterals;
 
-ThreePIdModel::ThreePIdModel(NeoChatConnection *connection)
+ThreePIdModel::ThreePIdModel(QObject *parent)
-    : QAbstractListModel(connection)
+    : QAbstractListModel(parent)
 {
-    Q_ASSERT(connection);
+}
-    connect(connection, &NeoChatConnection::stateChanged, this, [this]() {
+
+NeoChatConnection *ThreePIdModel::connection() const
+{
+    return m_connection;
+}
+
+void ThreePIdModel::setConnection(NeoChatConnection *connection)
+{
+    if (m_connection == connection) {
+        return;
+    }
+
+    if (m_connection != nullptr) {
+        m_connection->disconnect(this);
+    }
+
+    m_connection = connection;
+    if (m_connection) {
+        connect(m_connection, &NeoChatConnection::stateChanged, this, [this]() {
+            refreshModel();
+        });
         refreshModel();
-    });
+    }
+
+    Q_EMIT connectionChanged();
 }
 
 QVariant ThreePIdModel::data(const QModelIndex &index, int role) const
@@ -62,12 +84,14 @@ QHash<int, QByteArray> ThreePIdModel::roleNames() const
 
 void ThreePIdModel::refreshModel()
 {
-    const auto connection = dynamic_cast<NeoChatConnection *>(this->parent());
+    if (m_connection != nullptr && m_connection->isLoggedIn()) {
-    if (connection != nullptr && connection->isLoggedIn()) {
+        if (m_job.isRunning()) {
-        const auto threePIdJob = connection->callApi<Quotient::GetAccount3PIDsJob>();
+            m_job.cancel();
-        connect(threePIdJob, &Quotient::BaseJob::success, this, [this, threePIdJob]() {
+        }
+        m_job = m_connection->callApi<Quotient::GetAccount3PIDsJob>();
+        connect(m_job, &Quotient::BaseJob::success, this, [this]() {
             beginResetModel();
-            m_threePIds = threePIdJob->threepids();
+            m_threePIds = m_job->threepids();
             endResetModel();
 
             refreshBindStatus();
@@ -77,25 +101,24 @@ void ThreePIdModel::refreshModel()
 
 void ThreePIdModel::refreshBindStatus()
 {
-    const auto connection = dynamic_cast<NeoChatConnection *>(this->parent());
+    if (m_connection == nullptr || !m_connection->hasIdentityServer()) {
-    if (connection == nullptr || !connection->hasIdentityServer()) {
         return;
     }
 
-    const auto openIdJob = connection->callApi<Quotient::RequestOpenIdTokenJob>(connection->userId());
+    const auto openIdJob = m_connection->callApi<Quotient::RequestOpenIdTokenJob>(m_connection->userId());
-    connect(openIdJob, &Quotient::BaseJob::success, this, [this, connection, openIdJob]() {
+    connect(openIdJob, &Quotient::BaseJob::success, this, [this, openIdJob]() {
-        const auto requestUrl = QUrl(connection->identityServer().toString() + u"/_matrix/identity/v2/account/register"_s);
+        const auto requestUrl = QUrl(m_connection->identityServer().toString() + u"/_matrix/identity/v2/account/register"_s);
         if (!(requestUrl.scheme() == u"https"_s || requestUrl.scheme() == u"http"_s)) {
             return;
         }
 
         QNetworkRequest request(requestUrl);
         auto newRequest = Quotient::NetworkAccessManager::instance()->post(request, QJsonDocument(openIdJob->jsonData()).toJson());
-        connect(newRequest, &QNetworkReply::finished, this, [this, connection, newRequest]() {
+        connect(newRequest, &QNetworkReply::finished, this, [this, newRequest]() {
             QJsonObject replyJson = QJsonDocument::fromJson(newRequest->readAll()).object();
             const auto identityServerToken = replyJson["token"_L1].toString();
 
-            const auto requestUrl = QUrl(connection->identityServer().toString() + u"/_matrix/identity/v2/hash_details"_s);
+            const auto requestUrl = QUrl(m_connection->identityServer().toString() + u"/_matrix/identity/v2/hash_details"_s);
             if (!(requestUrl.scheme() == u"https"_s || requestUrl.scheme() == u"http"_s)) {
                 return;
             }
@@ -104,11 +127,11 @@ void ThreePIdModel::refreshBindStatus()
             hashRequest.setRawHeader("Authorization", "Bearer " + identityServerToken.toLatin1());
 
             auto hashReply = Quotient::NetworkAccessManager::instance()->get(hashRequest);
-            connect(hashReply, &QNetworkReply::finished, this, [this, connection, identityServerToken, hashReply]() {
+            connect(hashReply, &QNetworkReply::finished, this, [this, identityServerToken, hashReply]() {
                 QJsonObject replyJson = QJsonDocument::fromJson(hashReply->readAll()).object();
                 const auto lookupPepper = replyJson["lookup_pepper"_L1].toString();
 
-                const auto requestUrl = QUrl(connection->identityServer().toString() + u"/_matrix/identity/v2/lookup"_s);
+                const auto requestUrl = QUrl(m_connection->identityServer().toString() + u"/_matrix/identity/v2/lookup"_s);
                 if (!(requestUrl.scheme() == u"https"_s || requestUrl.scheme() == u"http"_s)) {
                     return;
                 }
@@ -127,13 +150,13 @@ void ThreePIdModel::refreshBindStatus()
                 requestData["addresses"_L1] = idLookups;
 
                 auto lookupReply = Quotient::NetworkAccessManager::instance()->post(lookupRequest, QJsonDocument(requestData).toJson(QJsonDocument::Compact));
-                connect(lookupReply, &QNetworkReply::finished, this, [this, connection, lookupReply]() {
+                connect(lookupReply, &QNetworkReply::finished, this, [this, lookupReply]() {
                     beginResetModel();
                     m_bindings.clear();
 
                     QJsonObject mappings = QJsonDocument::fromJson(lookupReply->readAll()).object()["mappings"_L1].toObject();
                     for (const auto &id : mappings.keys()) {
-                        if (mappings[id] == connection->userId()) {
+                        if (mappings[id] == m_connection->userId()) {
                             m_bindings += id.section(u' ', 0, 0);
                         }
                     }

src/models/threepidmodel.h

@@ -7,6 +7,7 @@
 #include <QQmlEngine>
 
 #include <Quotient/csapi/administrative_contact.h>
+#include <Quotient/jobs/jobhandle.h>
 
 class NeoChatConnection;
 
@@ -19,19 +20,27 @@ class ThreePIdModel : public QAbstractListModel
 {
     Q_OBJECT
     QML_ELEMENT
-    QML_UNCREATABLE("")
+
+    /**
+     * @brief The current connection for the model to use.
+     */
+    Q_PROPERTY(NeoChatConnection *connection READ connection WRITE setConnection NOTIFY connectionChanged)
 
 public:
     /**
      * @brief Defines the model roles.
      */
-    enum EventRoles {
+    enum Roles {
         AddressRole = Qt::DisplayRole, /**< The third-party identifier address. */
         MediumRole, /**< The medium of the third-party identifier. One of: [email, msisdn]. */
         IsBoundRole, /**< Whether the 3PID is bound to the current identity server. */
     };
+    Q_ENUM(Roles)
 
-    explicit ThreePIdModel(NeoChatConnection *parent);
+    explicit ThreePIdModel(QObject *parent = nullptr);
+
+    [[nodiscard]] NeoChatConnection *connection() const;
+    void setConnection(NeoChatConnection *connection);
 
     /**
      * @brief Get the given role value at the given index.
@@ -56,9 +65,15 @@ public:
 
     Q_INVOKABLE void refreshModel();
 
+Q_SIGNALS:
+    void connectionChanged();
+
 private:
+    QPointer<NeoChatConnection> m_connection;
     QVector<Quotient::GetAccount3PIDsJob::ThirdPartyIdentifier> m_threePIds;
 
+    Quotient::JobHandle<Quotient::GetAccount3PIDsJob> m_job;
+
     QList<QString> m_bindings;
 
     void refreshBindStatus();

src/neochatconnection.cpp

@@ -37,7 +37,6 @@ using namespace Qt::StringLiterals;
 
 NeoChatConnection::NeoChatConnection(QObject *parent)
     : Connection(parent)
-    , m_threePIdModel(new ThreePIdModel(this))
 {
     m_linkPreviewers.setMaxCost(20);
     connectSignals();
@@ -45,7 +44,6 @@ NeoChatConnection::NeoChatConnection(QObject *parent)
 
 NeoChatConnection::NeoChatConnection(const QUrl &server, QObject *parent)
     : Connection(server, parent)
-    , m_threePIdModel(new ThreePIdModel(this))
 {
     m_linkPreviewers.setMaxCost(20);
     connectSignals();
@@ -299,11 +297,6 @@ void NeoChatConnection::deactivateAccount(const QString &password, const bool er
     });
 }
 
-ThreePIdModel *NeoChatConnection::threePIdModel() const
-{
-    return m_threePIdModel;
-}
-
 bool NeoChatConnection::hasIdentityServer() const
 {
     if (!hasAccountData(u"m.identity_server"_s)) {

src/neochatconnection.h

@@ -14,7 +14,6 @@
 
 #include "enums/messagetype.h"
 #include "linkpreviewer.h"
-#include "models/threepidmodel.h"
 
 class NeoChatConnection : public Quotient::Connection
 {
@@ -37,11 +36,6 @@ class NeoChatConnection : public Quotient::Connection
      */
     Q_PROPERTY(bool globalUrlPreviewEnabled READ globalUrlPreviewEnabled WRITE setGlobalUrlPreviewEnabled NOTIFY globalUrlPreviewEnabledChanged)
 
-    /**
-     * @brief The model with the account's 3PIDs.
-     */
-    Q_PROPERTY(ThreePIdModel *threePIdModel READ threePIdModel CONSTANT)
-
     /**
      * @brief Whether an identity server is configured.
      */
@@ -144,8 +138,6 @@ public:
 
     Q_INVOKABLE void deactivateAccount(const QString &password, bool erase);
 
-    ThreePIdModel *threePIdModel() const;
-
     bool hasIdentityServer() const;
 
     /**
@@ -234,8 +226,6 @@ private:
     bool m_isOnline = true;
     void setIsOnline(bool isOnline);
 
-    ThreePIdModel *m_threePIdModel;
-
     void connectSignals();
 
     int m_badgeNotificationCount = 0;

src/settings/ThreePIdCard.qml

@@ -28,7 +28,10 @@ ColumnLayout {
         Repeater {
             id: deviceRepeater
             model: KSortFilterProxyModel {
-                sourceModel: root.connection.threePIdModel
+                sourceModel: ThreePIdModel {
+                    id: threePIdModel
+                    connection: root.connection
+                }
                 filterRoleName: "medium"
                 filterString: root.medium
             }
@@ -110,6 +113,9 @@ ColumnLayout {
                     connection: root.connection
                     newId: threePIdDelegate.address
                     medium: threePIdDelegate.medium
+
+                    onThreePIdBound: threePIdModel.refreshModel()
+                    onThreePIdUnbound: threePIdModel.refreshModel()
                 }
             }
 
@@ -130,7 +136,7 @@ ColumnLayout {
             label: i18nc("@label:textbox", "Country Code for new phone number")
 
             Connections {
-                target: root.connection.threePIdModel
+                target: threePIdModel
 
                 function onModelReset() {
                     newCountryCode.text = ""
@@ -170,7 +176,7 @@ ColumnLayout {
             onAccepted: _private.openPasswordSheet()
 
             Connections {
-                target: root.connection.threePIdModel
+                target: threePIdModel
 
                 function onModelReset() {
                     newId.text = ""
@@ -198,6 +204,10 @@ ColumnLayout {
         connection: root.connection
         medium: root.medium
         newId: newId.text
+
+        onThreePIdAdded: threePIdModel.refreshModel()
+        onThreePIdRemoved: threePIdModel.refreshModel()
+        onThreePIdUnbound: threePIdModel.refreshModel()
     }
 
     QtObject {

src/threepidaddhelper.cpp

@@ -154,11 +154,11 @@ void ThreePIdAddHelper::finalizeNewIdAdd(const QString &password)
             authData.authInfo["identifier"_L1] = QJsonObject{{"type"_L1, "m.id.user"_L1}, {"user"_L1, m_connection->userId()}};
             const auto innerJob = m_connection->callApi<Add3PIDJob>(m_newIdSecret, m_newIdSid, authData);
             connect(innerJob, &Quotient::BaseJob::success, this, [this]() {
-                m_connection->threePIdModel()->refreshModel();
                 m_newIdSecret.clear();
                 m_newIdSid.clear();
                 m_newIdStatus = Success;
                 Q_EMIT newIdStatusChanged();
+                Q_EMIT threePIdAdded();
             });
             connect(innerJob, &Quotient::BaseJob::failure, this, [innerJob, this]() {
                 if (innerJob->jsonData()["errcode"_L1] == "M_FORBIDDEN"_L1) {
@@ -180,7 +180,7 @@ void ThreePIdAddHelper::remove3PId(const QString &threePId, const QString &type)
 {
     const auto job = m_connection->callApi<Quotient::Delete3pidFromAccountJob>(type, threePId);
     connect(job, &Quotient::BaseJob::success, this, [this]() {
-        m_connection->threePIdModel()->refreshModel();
+        Q_EMIT threePIdRemoved();
     });
 }
 
@@ -188,7 +188,7 @@ void ThreePIdAddHelper::unbind3PId(const QString &threePId, const QString &type)
 {
     const auto job = m_connection->callApi<Quotient::Unbind3pidFromAccountJob>(type, threePId);
     connect(job, &Quotient::BaseJob::success, this, [this]() {
-        m_connection->threePIdModel()->refreshModel();
+        Q_EMIT threePIdUnbound();
     });
 }
 

src/threepidaddhelper.h

@@ -121,6 +121,21 @@ Q_SIGNALS:
     void newEmailSessionStartedChanged();
     void newIdStatusChanged();
 
+    /**
+     * @brief A 3PID has been added.
+     */
+    void threePIdAdded();
+
+    /**
+     * @brief A 3PID has been removed.
+     */
+    void threePIdRemoved();
+
+    /**
+     * @brief A 3PID has been unbound.
+     */
+    void threePIdUnbound();
+
 private:
     QPointer<NeoChatConnection> m_connection;
     QString m_medium = QString();

src/threepidbindhelper.cpp

@@ -6,6 +6,7 @@
 #include <QNetworkReply>
 
 #include <Quotient/converters.h>
+#include <Quotient/csapi/administrative_contact.h>
 #include <Quotient/csapi/definitions/auth_data.h>
 #include <Quotient/csapi/definitions/request_msisdn_validation.h>
 #include <Quotient/csapi/openid.h>
@@ -193,7 +194,7 @@ void ThreePIdBindHelper::finalizeNewIdBind()
     connect(job, &Quotient::BaseJob::success, this, [this] {
         m_bindStatus = Success;
         Q_EMIT bindStatusChanged();
-        m_connection->threePIdModel()->refreshModel();
+        Q_EMIT threePIdBound();
     });
     connect(job, &Quotient::BaseJob::failure, this, [this, job]() {
         if (job->jsonData()["errcode"_L1] == "M_SESSION_NOT_VALIDATED"_L1) {
@@ -211,7 +212,7 @@ void ThreePIdBindHelper::unbind3PId(const QString &threePId, const QString &type
     const auto job = m_connection->callApi<Quotient::Unbind3pidFromAccountJob>(type, threePId);
     connect(job, &Quotient::BaseJob::success, this, [this]() {
         cancel();
-        m_connection->threePIdModel()->refreshModel();
+        Q_EMIT threePIdUnbound();
     });
 }
 

src/threepidbindhelper.h

@@ -127,6 +127,16 @@ Q_SIGNALS:
     void newEmailSessionStartedChanged();
     void bindStatusChanged();
 
+    /**
+     * @brief A 3PID has been unbound.
+     */
+    void threePIdBound();
+
+    /**
+     * @brief A 3PID has been unbound.
+     */
+    void threePIdUnbound();
+
 private:
     QPointer<NeoChatConnection> m_connection;
     QString m_medium = QString();