Tree Model 2 Electric Boogaloo

This draws heavily on what @carlschwan did in network/neochat!1579 but I found it easier to start again and grab the bits as I needed them plus some other copying from what I did in the Space tree model.

From my current limited testing this seems to work nicely try and break it.

src/CMakeLists.txt

@@ -172,6 +172,8 @@ add_library(neochat STATIC
     models/statekeysmodel.h
     sharehandler.cpp
     sharehandler.h
+    models/roomtreeitem.cpp
+    models/roomtreeitem.h
 )
 
 set_source_files_properties(qml/OsmLocationPlugin.qml PROPERTIES

src/enums/neochatroomtype.h

@@ -29,6 +29,7 @@ public:
         Deprioritized, /**< The room is set as low priority. */
         Space, /**< The room is a space. */
         AddDirect, /**< So we can show the add friend delegate. */
+        TypesCount, /**< Number of different types (this should always be last). */
     };
     Q_ENUM(Types);
 

src/models/roomtreeitem.cpp

@@ -0,0 +1,100 @@
+// SPDX-FileCopyrightText: 2024 Carl Schwan <carl@carlschwan.eu>
+// SPDX-FileCopyrightText: 2024 James Graham <james.h.graham@protonmail.com>
+// SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
+
+#include "roomtreeitem.h"
+
+RoomTreeItem::RoomTreeItem(TreeData data, RoomTreeItem *parent)
+    : m_parentItem(parent)
+    , m_data(data)
+{
+}
+
+bool RoomTreeItem::operator==(const RoomTreeItem &other) const
+{
+    if (std::holds_alternative<NeoChatRoomType::Types>(m_data) && std::holds_alternative<NeoChatRoomType::Types>(other.data())) {
+        return std::get<NeoChatRoomType::Types>(m_data) == std::get<NeoChatRoomType::Types>(m_data);
+    }
+    if (std::holds_alternative<NeoChatRoom *>(m_data) && std::holds_alternative<NeoChatRoom *>(other.data())) {
+        return std::get<NeoChatRoom *>(m_data)->id() == std::get<NeoChatRoom *>(m_data)->id();
+    }
+    return false;
+}
+
+RoomTreeItem *RoomTreeItem::child(int row)
+{
+    return row >= 0 && row < childCount() ? m_children.at(row).get() : nullptr;
+}
+
+int RoomTreeItem::childCount() const
+{
+    return int(m_children.size());
+}
+
+bool RoomTreeItem::insertChild(std::unique_ptr<RoomTreeItem> newChild)
+{
+    if (newChild == nullptr) {
+        return false;
+    }
+
+    for (auto it = m_children.begin(), end = m_children.end(); it != end; ++it) {
+        if (*it == newChild) {
+            *it = std::move(newChild);
+            return true;
+        }
+    }
+
+    m_children.push_back(std::move(newChild));
+    return true;
+}
+
+bool RoomTreeItem::removeChild(int row)
+{
+    if (row < 0 || row >= childCount()) {
+        return false;
+    }
+    m_children.erase(m_children.begin() + row);
+    return true;
+}
+
+int RoomTreeItem::row() const
+{
+    if (m_parentItem == nullptr) {
+        return 0;
+    }
+
+    const auto it = std::find_if(m_parentItem->m_children.cbegin(), m_parentItem->m_children.cend(), [this](const std::unique_ptr<RoomTreeItem> &treeItem) {
+        return treeItem.get() == this;
+    });
+
+    if (it != m_parentItem->m_children.cend()) {
+        return std::distance(m_parentItem->m_children.cbegin(), it);
+    }
+    Q_ASSERT(false); // should not happen
+    return -1;
+}
+
+RoomTreeItem *RoomTreeItem::parentItem() const
+{
+    return m_parentItem;
+}
+
+RoomTreeItem::TreeData RoomTreeItem::data() const
+{
+    return m_data;
+}
+
+std::optional<int> RoomTreeItem::rowForRoom(Quotient::Room *room) const
+{
+    Q_ASSERT_X(std::holds_alternative<NeoChatRoomType::Types>(m_data), __FUNCTION__, "rowForRoom only works items for rooms not categories");
+
+    int i = 0;
+    for (const auto &child : m_children) {
+        if (std::get<NeoChatRoom *>(child->data()) == room) {
+            return i;
+        }
+        i++;
+    }
+
+    return std::nullopt;
+}

src/models/roomtreeitem.h

@@ -0,0 +1,78 @@
+// SPDX-FileCopyrightText: 2024 Carl Schwan <carl@carlschwan.eu>
+// SPDX-FileCopyrightText: 2024 James Graham <james.h.graham@protonmail.com>
+// SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
+
+#include "enums/neochatroomtype.h"
+
+class NeoChatRoom;
+
+/**
+ * @class RoomTreeItem
+ *
+ * This class defines an item in the space tree hierarchy model.
+ *
+ * @note This is separate from Quotient::Room and NeoChatRoom because we don't have
+ *       full room information for any room/space the user hasn't joined and we
+ *       don't want to create one for ever possible child in a space as that would
+ *       be expensive.
+ *
+ * @sa Quotient::Room, NeoChatRoom
+ */
+class RoomTreeItem
+{
+public:
+    using TreeData = std::variant<NeoChatRoom *, NeoChatRoomType::Types>;
+
+    explicit RoomTreeItem(TreeData data, RoomTreeItem *parent = nullptr);
+
+    bool operator==(const RoomTreeItem &other) const;
+
+    /**
+     * @brief Return the child at the given row number.
+     *
+     * Nullptr is returned if there is no child at the given row number.
+     */
+    RoomTreeItem *child(int row);
+
+    /**
+     * @brief The number of children this item has.
+     */
+    int childCount() const;
+
+    /**
+     * @brief Insert the given child.
+     */
+    bool insertChild(std::unique_ptr<RoomTreeItem> newChild);
+
+    /**
+     * @brief Remove the child at the given row number.
+     *
+     * @return True if a child was removed, false if the given row isn't valid.
+     */
+    bool removeChild(int row);
+
+    /**
+     * @brief Return this item's parent.
+     */
+    RoomTreeItem *parentItem() const;
+
+    /**
+     * @brief Return the row number for this child relative to the parent.
+     *
+     * @return The row value if the child has a parent, 0 otherwise.
+     */
+    int row() const;
+
+    /**
+     * @brief Return this item's data.
+     */
+    TreeData data() const;
+
+    std::optional<int> rowForRoom(Quotient::Room *room) const;
+
+private:
+    std::vector<std::unique_ptr<RoomTreeItem>> m_children;
+    RoomTreeItem *m_parentItem;
+
+    TreeData m_data;
+};

src/models/roomtreemodel.cpp

@@ -15,21 +15,47 @@ using namespace Quotient;
 
 RoomTreeModel::RoomTreeModel(QObject *parent)
     : QAbstractItemModel(parent)
+    , m_rootItem(new RoomTreeItem(nullptr))
 {
-    initializeCategories();
 }
 
-void RoomTreeModel::initializeCategories()
+RoomTreeItem *RoomTreeModel::getItem(const QModelIndex &index) const
 {
-    for (const auto &key : m_rooms.keys()) {
+    if (index.isValid()) {
-        for (const auto &room : m_rooms[key]) {
+        RoomTreeItem *item = static_cast<RoomTreeItem *>(index.internalPointer());
-            room->disconnect(this);
+        if (item) {
+            return item;
         }
     }
-    m_rooms.clear();
+    return m_rootItem.get();
-    for (int i = 0; i < 8; i++) {
+}
-        m_rooms[NeoChatRoomType::Types(i)] = {};
+
+void RoomTreeModel::resetModel()
+{
+    if (m_connection == nullptr) {
+        beginResetModel();
+        m_rootItem.reset();
+        endResetModel();
+        return;
     }
+
+    beginResetModel();
+    m_rootItem.reset(new RoomTreeItem(nullptr));
+
+    for (int i = 0; i < NeoChatRoomType::TypesCount; i++) {
+        m_rootItem->insertChild(std::make_unique<RoomTreeItem>(NeoChatRoomType::Types(i), m_rootItem.get()));
+    }
+
+    for (const auto &r : m_connection->allRooms()) {
+        const auto room = dynamic_cast<NeoChatRoom *>(r);
+        const auto type = NeoChatRoomType::typeForRoom(room);
+        const auto categoryItem = m_rootItem->child(type);
+        if (categoryItem->insertChild(std::make_unique<RoomTreeItem>(room, categoryItem))) {
+            connectRoomSignals(room);
+        }
+    }
+
+    endResetModel();
 }
 
 void RoomTreeModel::setConnection(NeoChatConnection *connection)
@@ -41,16 +67,13 @@ void RoomTreeModel::setConnection(NeoChatConnection *connection)
         disconnect(m_connection.get(), nullptr, this, nullptr);
     }
     m_connection = connection;
-    beginResetModel();
+
-    initializeCategories();
+    resetModel();
-    endResetModel();
+
     connect(connection, &Connection::newRoom, this, &RoomTreeModel::newRoom);
     connect(connection, &Connection::leftRoom, this, &RoomTreeModel::leftRoom);
     connect(connection, &Connection::aboutToDeleteRoom, this, &RoomTreeModel::leftRoom);
 
-    for (const auto &room : m_connection->allRooms()) {
-        newRoom(dynamic_cast<NeoChatRoom *>(room));
-    }
     Q_EMIT connectionChanged();
 }
 
@@ -68,23 +91,28 @@ void RoomTreeModel::newRoom(Room *r)
         return;
     }
 
-    beginInsertRows(index(type, 0), m_rooms[type].size(), m_rooms[type].size());
+    const auto parentItem = m_rootItem->child(type);
-    m_rooms[type].append(room);
+    beginInsertRows(index(parentItem->row(), 0), parentItem->childCount(), parentItem->childCount());
+    parentItem->insertChild(std::make_unique<RoomTreeItem>(room, parentItem));
     connectRoomSignals(room);
     endInsertRows();
+    qWarning() << "adding room" << type << "new count" << parentItem->childCount();
 }
 
 void RoomTreeModel::leftRoom(Room *r)
 {
     const auto room = dynamic_cast<NeoChatRoom *>(r);
-    const auto type = NeoChatRoomType::typeForRoom(room);
+    auto index = indexForRoom(room);
-    auto row = m_rooms[type].indexOf(room);
+    if (!index.isValid()) {
-    if (row == -1) {
         return;
     }
-    beginRemoveRows(index(type, 0), row, row);
+
-    m_rooms[type][row]->disconnect(this);
+    const auto parentItem = getItem(index.parent());
-    m_rooms[type].removeAt(row);
+    Q_ASSERT(parentItem);
+
+    beginRemoveRows(index.parent(), index.row(), index.row());
+    parentItem->removeChild(index.row());
+    room->disconnect(this);
     endRemoveRows();
 }
 
@@ -94,30 +122,41 @@ void RoomTreeModel::moveRoom(Quotient::Room *room)
     // NeoChatRoomType::typeForRoom doesn't match it's current location. So find the room.
     NeoChatRoomType::Types oldType;
     int oldRow = -1;
-    for (const auto &key : m_rooms.keys()) {
+    for (int i = 0; i < NeoChatRoomType::TypesCount; i++) {
-        if (m_rooms[key].contains(room)) {
+        const auto categoryItem = m_rootItem->child(i);
-            oldType = key;
+        const auto row = categoryItem->rowForRoom(room);
-            oldRow = m_rooms[key].indexOf(room);
+        if (row) {
+            oldType = static_cast<NeoChatRoomType::Types>(i);
+            oldRow = *row;
         }
     }
 
     if (oldRow == -1) {
         return;
     }
-    const auto newType = NeoChatRoomType::typeForRoom(dynamic_cast<NeoChatRoom *>(room));
+    auto neochatRoom = dynamic_cast<NeoChatRoom *>(room);
+    const auto newType = NeoChatRoomType::typeForRoom(neochatRoom);
     if (newType == oldType) {
         return;
     }
 
     const auto oldParent = index(oldType, 0, {});
+    auto oldParentItem = getItem(oldParent);
+    Q_ASSERT(oldParentItem);
+
     const auto newParent = index(newType, 0, {});
+    auto newParentItem = getItem(newParent);
+    Q_ASSERT(newParentItem);
+
     // HACK: We're doing this as a remove then insert because  moving doesn't work
     // properly with DelegateChooser for whatever reason.
+    Q_ASSERT(checkIndex(index(oldRow, 0, oldParent), QAbstractItemModel::CheckIndexOption::IndexIsValid));
     beginRemoveRows(oldParent, oldRow, oldRow);
-    m_rooms[oldType].removeAt(oldRow);
+    const bool success = oldParentItem->removeChild(oldRow);
+    Q_ASSERT(success);
     endRemoveRows();
-    beginInsertRows(newParent, m_rooms[newType].size(), m_rooms[newType].size());
+    beginInsertRows(newParent, newParentItem->childCount(), newParentItem->childCount());
-    m_rooms[newType].append(dynamic_cast<NeoChatRoom *>(room));
+    newParentItem->insertChild(std::make_unique<RoomTreeItem>(neochatRoom, newParentItem));
     endInsertRows();
 }
 
@@ -151,15 +190,12 @@ void RoomTreeModel::connectRoomSignals(NeoChatRoom *room)
 
 void RoomTreeModel::refreshRoomRoles(NeoChatRoom *room, const QList<int> &roles)
 {
-    const auto roomType = NeoChatRoomType::typeForRoom(room);
+    const auto index = indexForRoom(room);
-    const auto it = std::find(m_rooms[roomType].begin(), m_rooms[roomType].end(), room);
+    if (!index.isValid()) {
-    if (it == m_rooms[roomType].end()) {
         qCritical() << "Room" << room->id() << "not found in the room list";
         return;
     }
-    const auto parentIndex = index(roomType, 0, {});
+    Q_EMIT dataChanged(index, index, roles);
-    const auto idx = index(it - m_rooms[roomType].begin(), 0, parentIndex);
-    Q_EMIT dataChanged(idx, idx, roles);
 }
 
 NeoChatConnection *RoomTreeModel::connection() const
@@ -175,32 +211,55 @@ int RoomTreeModel::columnCount(const QModelIndex &parent) const
 
 int RoomTreeModel::rowCount(const QModelIndex &parent) const
 {
+    RoomTreeItem *parentItem;
+    if (parent.column() > 0) {
+        return 0;
+    }
+
     if (!parent.isValid()) {
-        return m_rooms.keys().size();
+        parentItem = m_rootItem.get();
+    } else {
+        parentItem = static_cast<RoomTreeItem *>(parent.internalPointer());
     }
-    if (!parent.parent().isValid()) {
+
-        return m_rooms.values()[parent.row()].size();
+    return parentItem->childCount();
-    }
-    return 0;
 }
 
 QModelIndex RoomTreeModel::parent(const QModelIndex &index) const
 {
-    if (!index.internalPointer()) {
+    if (!index.isValid()) {
-        return {};
+        return QModelIndex();
     }
-    return this->index(NeoChatRoomType::typeForRoom(static_cast<NeoChatRoom *>(index.internalPointer())), 0, QModelIndex());
+
+    RoomTreeItem *childItem = static_cast<RoomTreeItem *>(index.internalPointer());
+    if (!childItem) {
+        return QModelIndex();
+    }
+    RoomTreeItem *parentItem = childItem->parentItem();
+
+    if (parentItem == m_rootItem.get()) {
+        return QModelIndex();
+    }
+
+    return createIndex(parentItem->row(), 0, parentItem);
 }
 
 QModelIndex RoomTreeModel::index(int row, int column, const QModelIndex &parent) const
 {
-    if (!parent.isValid()) {
+    if (!hasIndex(row, column, parent)) {
-        return createIndex(row, column, nullptr);
+        return QModelIndex();
     }
-    if (row >= rowCount(parent)) {
+
-        return {};
+    RoomTreeItem *parentItem = getItem(parent);
+    if (!parentItem) {
+        return QModelIndex();
     }
-    return createIndex(row, column, m_rooms[NeoChatRoomType::Types(parent.row())][row]);
+
+    RoomTreeItem *childItem = parentItem->child(row);
+    if (childItem) {
+        return createIndex(row, column, childItem);
+    }
+    return QModelIndex();
 }
 
 QHash<int, QByteArray> RoomTreeModel::roleNames() const
@@ -235,7 +294,8 @@ QVariant RoomTreeModel::data(const QModelIndex &index, int role) const
         return QVariant();
     }
 
-    if (!index.parent().isValid()) {
+    RoomTreeItem *child = getItem(index);
+    if (std::holds_alternative<NeoChatRoomType::Types>(child->data())) {
         if (role == DisplayNameRole) {
             return NeoChatRoomType::typeName(index.row());
         }
@@ -256,7 +316,8 @@ QVariant RoomTreeModel::data(const QModelIndex &index, int role) const
         }
         return {};
     }
-    const auto room = m_rooms.values()[index.parent().row()][index.row()].get();
+
+    const auto room = std::get<NeoChatRoom *>(child->data());
     Q_ASSERT(room);
 
     if (role == DisplayNameRole) {
@@ -338,16 +399,20 @@ QModelIndex RoomTreeModel::indexForRoom(NeoChatRoom *room) const
 
     // Try and find by checking type.
     const auto type = NeoChatRoomType::typeForRoom(room);
-    auto row = m_rooms[type].indexOf(room);
+    const auto parentItem = m_rootItem->child(type);
-    if (row >= 0) {
+    const auto row = parentItem->rowForRoom(room);
-        return index(row, 0, index(type, 0));
+    if (row) {
+        return index(*row, 0, index(type, 0));
     }
     // Double check that the room isn't in the wrong category.
-    for (const auto &key : m_rooms.keys()) {
+    for (int i = 0; i < NeoChatRoomType::TypesCount; i++) {
-        if (m_rooms[key].contains(room)) {
+        const auto parentItem = m_rootItem->child(i);
-            return index(m_rooms[key].indexOf(room), 0, index(key, 0));
+        const auto row = parentItem->rowForRoom(room);
+        if (row) {
+            return index(*row, 0, index(i, 0));
         }
     }
+
     return {};
 }
 

src/models/roomtreemodel.h

@@ -7,6 +7,7 @@
 #include <QPointer>
 
 #include "enums/neochatroomtype.h"
+#include "roomtreeitem.h"
 
 namespace Quotient
 {
@@ -83,9 +84,11 @@ Q_SIGNALS:
 
 private:
     QPointer<NeoChatConnection> m_connection;
-    QMap<NeoChatRoomType::Types, QList<QPointer<NeoChatRoom>>> m_rooms;
+    std::unique_ptr<RoomTreeItem> m_rootItem;
 
-    void initializeCategories();
+    RoomTreeItem *getItem(const QModelIndex &index) const;
+
+    void resetModel();
     void connectRoomSignals(NeoChatRoom *room);
 
     void newRoom(Quotient::Room *room);

src/models/sortfilterroomtreemodel.cpp

@@ -24,7 +24,6 @@ SortFilterRoomTreeModel::SortFilterRoomTreeModel(RoomTreeModel *sourceModel, QOb
 
     setRecursiveFilteringEnabled(true);
     sort(0);
-    invalidateFilter();
     connect(this, &SortFilterRoomTreeModel::filterTextChanged, this, &SortFilterRoomTreeModel::invalidateFilter);
     connect(this, &SortFilterRoomTreeModel::sourceModelChanged, this, [this]() {
         this->sourceModel()->disconnect(this);