Store NeochatRoomMember objects in the room

Store NeochatRoomMember objects in the room so we don't duplicate them unnecessarily. This also adds a visible property for a room which is set true when shown by MessageEventModel and false when not, triggering the deletion of member objects. This mechanism will be used for other object types in the future.

src/models/messagecontentmodel.cpp

@@ -135,14 +135,14 @@ void MessageContentModel::initializeModel()
     });
     connect(m_room, &Room::memberNameUpdated, this, [this](RoomMember member) {
         if (m_room != nullptr) {
-            if (m_eventSenderId.isEmpty() || m_eventSenderId == member.id()) {
+            if (senderId().isEmpty() || senderId() == member.id()) {
                 Q_EMIT dataChanged(index(0, 0), index(rowCount() - 1, 0), {AuthorRole});
             }
         }
     });
     connect(m_room, &Room::memberAvatarUpdated, this, [this](RoomMember member) {
         if (m_room != nullptr) {
-            if (m_eventSenderId.isEmpty() || m_eventSenderId == member.id()) {
+            if (senderId().isEmpty() || senderId() == member.id()) {
                 Q_EMIT dataChanged(index(0, 0), index(rowCount() - 1, 0), {AuthorRole});
             }
         }
@@ -178,16 +178,6 @@ void MessageContentModel::initializeEvent()
     } else {
         m_currentState = Available;
     }
-
-    if (m_eventSenderObject == nullptr) {
-        auto senderId = eventResult.first->senderId();
-        // A pending event might not have a sender ID set yet but in that case it must
-        // be the local member.
-        if (senderId.isEmpty()) {
-            senderId = m_room->localMember().id();
-        }
-        m_eventSenderObject = std::unique_ptr<NeochatRoomMember>(new NeochatRoomMember(m_room, senderId));
-    }
     Q_EMIT eventUpdated();
 }
 
@@ -218,6 +208,31 @@ void MessageContentModel::getEvent()
     m_room->downloadEventFromServer(m_eventId);
 }
 
+QString MessageContentModel::senderId() const
+{
+    const auto eventResult = m_room->getEvent(m_eventId);
+    if (eventResult.first == nullptr) {
+        return {};
+    }
+    auto senderId = eventResult.first->senderId();
+    if (senderId.isEmpty()) {
+        senderId = m_room->localMember().id();
+    }
+    return senderId;
+}
+
+NeochatRoomMember *MessageContentModel::senderObject() const
+{
+    const auto eventResult = m_room->getEvent(m_eventId);
+    if (eventResult.first == nullptr) {
+        return nullptr;
+    }
+    if (eventResult.first->senderId().isEmpty()) {
+        return m_room->qmlSafeMember(m_room->localMember().id());
+    }
+    return m_room->qmlSafeMember(eventResult.first->senderId());
+}
+
 bool MessageContentModel::showAuthor() const
 {
     return m_showAuthor;
@@ -230,7 +245,7 @@ void MessageContentModel::setShowAuthor(bool showAuthor)
     }
     m_showAuthor = showAuthor;
 
-    if (m_room->connection()->isIgnored(m_eventSenderId)) {
+    if (m_room->connection()->isIgnored(senderId())) {
         if (showAuthor) {
             beginInsertRows({}, 0, 0);
             m_components.prepend(MessageComponent{MessageComponentType::Author, QString(), {}});
@@ -275,7 +290,7 @@ QVariant MessageContentModel::data(const QModelIndex &index, int role) const
     }
 
     if (role == DisplayRole) {
-        if (m_currentState == UnAvailable || m_room->connection()->isIgnored(m_eventSenderId)) {
+        if (m_currentState == UnAvailable || m_room->connection()->isIgnored(senderId())) {
             Kirigami::Platform::PlatformTheme *theme =
                 static_cast<Kirigami::Platform::PlatformTheme *>(qmlAttachedPropertiesObject<Kirigami::Platform::PlatformTheme>(this, true));
 
@@ -317,7 +332,7 @@ QVariant MessageContentModel::data(const QModelIndex &index, int role) const
         return EventHandler::timeString(m_room, event.first, u"hh:mm"_s, m_currentState == Pending);
     }
     if (role == AuthorRole) {
-        return QVariant::fromValue<NeochatRoomMember *>(m_eventSenderObject.get());
+        return QVariant::fromValue<NeochatRoomMember *>(senderObject());
     }
     if (role == MediaInfoRole) {
         return EventHandler::mediaInfo(m_room, event.first);
@@ -406,7 +421,7 @@ void MessageContentModel::resetModel()
     beginResetModel();
     m_components.clear();
 
-    if (m_room->connection()->isIgnored(m_eventSenderId) || m_currentState == UnAvailable) {
+    if (m_room->connection()->isIgnored(senderId()) || m_currentState == UnAvailable) {
         m_components += MessageComponent{MessageComponentType::Text, QString(), {}};
         endResetModel();
         return;

src/models/messagecontentmodel.h

@@ -113,8 +113,8 @@ Q_SIGNALS:
 private:
     QPointer<NeoChatRoom> m_room;
     QString m_eventId;
-    QString m_eventSenderId;
-    std::unique_ptr<NeochatRoomMember> m_eventSenderObject = nullptr;
+    QString senderId() const;
+    NeochatRoomMember *senderObject() const;
 
     MessageState m_currentState = Unknown;
     bool m_showAuthor = true;

src/models/messagemodel.cpp

@@ -54,6 +54,9 @@ void MessageModel::setRoom(NeoChatRoom *room)
 
     beginResetModel();
     m_room = room;
+    if (m_room != nullptr) {
+        m_room->setVisible(true);
+    }
     Q_EMIT roomChanged();
     endResetModel();
 }
@@ -493,12 +496,17 @@ void MessageModel::createEventObjects(const Quotient::RoomEvent *event, bool isP
 void MessageModel::clearModel()
 {
     if (m_room) {
+        const auto oldRoom = m_room;
+
         // HACK: Reset the model to a null room first to make sure QML dismantles
         // last room's objects before the room is actually changed
         beginResetModel();
         m_room->disconnect(this);
         m_room = nullptr;
         endResetModel();
+
+        // Because we don't want any of the object deleted before the model is cleared.
+        oldRoom->setVisible(false);
     }
 
     // Don't clear the member objects until the model has been fully reset and all

src/neochatroom.cpp

@@ -44,6 +44,7 @@
 #include "events/pollevent.h"
 #include "filetransferpseudojob.h"
 #include "neochatconfig.h"
+#include "neochatroommember.h"
 #include "roomlastmessageprovider.h"
 #include "spacehierarchycache.h"
 #include "texthandler.h"
@@ -161,6 +162,20 @@ NeoChatRoom::NeoChatRoom(Connection *connection, QString roomId, JoinState joinS
     });
 }
 
+bool NeoChatRoom::visible() const
+{
+    return m_visible;
+}
+
+void NeoChatRoom::setVisible(bool visible)
+{
+    m_visible = visible;
+
+    if (!visible) {
+        m_memberObjects.clear();
+    }
+}
+
 int NeoChatRoom::contextAwareNotificationCount() const
 {
     // DOn't include spaces, rooms that the user hasn't joined and rooms where the user has joined the successor.
@@ -1726,4 +1741,13 @@ void NeoChatRoom::setRoomState(const QString &type, const QString &stateKey, con
     setState(type, stateKey, QJsonDocument::fromJson(content).object());
 }
 
+NeochatRoomMember *NeoChatRoom::qmlSafeMember(const QString &memberId)
+{
+    if (!m_memberObjects.contains(memberId)) {
+        return m_memberObjects.emplace(memberId, std::make_unique<NeochatRoomMember>(this, memberId)).first->second.get();
+    }
+
+    return m_memberObjects[memberId].get();
+}
+
 #include "moc_neochatroom.cpp"

src/neochatroom.h

@@ -211,6 +211,9 @@ class NeoChatRoom : public Quotient::Room
 public:
     explicit NeoChatRoom(Quotient::Connection *connection, QString roomId, Quotient::JoinState joinState = {});
 
+    bool visible() const;
+    void setVisible(bool visible);
+
     [[nodiscard]] QDateTime lastActiveTime();
 
     /**
@@ -590,7 +593,11 @@ public:
      */
     Quotient::FileTransferInfo cachedFileTransferInfo(const Quotient::RoomEvent *event) const;
 
+    NeochatRoomMember *qmlSafeMember(const QString &memberId);
+
 private:
+    bool m_visible = false;
+
     QSet<const Quotient::RoomEvent *> highlights;
 
     bool m_hasFileUploading = false;
@@ -619,6 +626,8 @@ private:
     void cleanupExtraEventRange(Quotient::RoomEventsRange events);
     void cleanupExtraEvent(const QString &eventId);
 
+    std::unordered_map<QString, std::unique_ptr<NeochatRoomMember>> m_memberObjects;
+
 private Q_SLOTS:
     void updatePushNotificationState(QString type);