From 5a4e7d9518e91cf39b96f14b3b310fe5b9a27594 Mon Sep 17 00:00:00 2001
From: Tobias Markmann <tm@ayena.de>
Date: Thu, 25 Aug 2016 16:00:10 +0200
Subject: Alphabetically sort MUC search result

This also changes the classes around MUCSearchModel to C++11
smart pointer based memory management.

Test-Information:

Verified that dtors of MUCSearch*Items are called when old
search results are replaced by new search results. This
was not the case previously.

All unit tests and manual testing with an ASAN enabled build
succeeded.

Change-Id: I84d62f3b86138728401b98d3774f47c72fdf9a4c

diff --git a/Swift/Controllers/Chat/MUCSearchController.cpp b/Swift/Controllers/Chat/MUCSearchController.cpp
index 0893799..5db917a 100644
--- a/Swift/Controllers/Chat/MUCSearchController.cpp
+++ b/Swift/Controllers/Chat/MUCSearchController.cpp
@@ -13,7 +13,6 @@
 
 #include <Swiften/Base/Log.h>
 #include <Swiften/Base/String.h>
-#include <Swiften/Base/foreach.h>
 #include <Swiften/Client/NickResolver.h>
 #include <Swiften/Disco/DiscoServiceWalker.h>
 #include <Swiften/Disco/GetDiscoItemsRequest.h>
@@ -48,7 +47,7 @@ void MUCSearchController::openSearchWindow() {
 
 void MUCSearchController::loadSavedServices() {
     savedServices_.clear();
-    foreach (std::string stringItem, String::split(settings_->getStringSetting(SEARCHED_SERVICES), '\n')) {
+    for (auto&& stringItem : String::split(settings_->getStringSetting(SEARCHED_SERVICES), '\n')) {
         savedServices_.push_back(JID(stringItem));
     }
 }
@@ -59,7 +58,7 @@ void MUCSearchController::addToSavedServices(const JID& jid) {
 
     std::string collapsed;
     int i = 0;
-    foreach (JID jidItem, savedServices_) {
+    for (auto&& jidItem : savedServices_) {
         if (i >= 15) {
             break;
         }
@@ -104,7 +103,7 @@ void MUCSearchController::handleSearchService(const JID& jid) {
 void MUCSearchController::handleDiscoServiceFound(const JID& jid, std::shared_ptr<DiscoInfo> info) {
     bool isMUC = false;
     std::string name;
-    foreach (DiscoInfo::Identity identity, info->getIdentities()) {
+    for (auto&& identity : info->getIdentities()) {
             if ((identity.getCategory() == "directory"
                 && identity.getType() == "chatroom")
                 || (identity.getCategory() == "conference"
@@ -152,7 +151,7 @@ void MUCSearchController::handleRoomsItemsResponse(std::shared_ptr<DiscoItems> i
         return;
     }
     serviceDetails_[jid].clearRooms();
-    foreach (DiscoItems::Item item, items->getItems()) {
+    for (auto&& item : items->getItems()) {
         serviceDetails_[jid].addRoom(MUCService::MUCRoom(item.getJID().getNode(), item.getName(), -1));
     }
     serviceDetails_[jid].setComplete(true);
@@ -166,7 +165,7 @@ void MUCSearchController::handleDiscoError(const JID& jid, ErrorPayload::ref err
 
 void MUCSearchController::refreshView() {
     window_->clearList();
-    foreach (JID jid, services_) {
+    for (auto&& jid : services_) {
         window_->addService(serviceDetails_[jid]);
     }
 }
diff --git a/Swift/QtUI/MUCSearch/MUCSearchEmptyItem.cpp b/Swift/QtUI/MUCSearch/MUCSearchEmptyItem.cpp
index d95682c..ce6e8f9 100644
--- a/Swift/QtUI/MUCSearch/MUCSearchEmptyItem.cpp
+++ b/Swift/QtUI/MUCSearch/MUCSearchEmptyItem.cpp
@@ -6,18 +6,23 @@
 
 #include <Swift/QtUI/MUCSearch/MUCSearchEmptyItem.h>
 
+#include <memory>
+
 #include <QColor>
 #include <QFont>
 
 #include <Swift/QtUI/MUCSearch/MUCSearchServiceItem.h>
 
 namespace Swift {
-MUCSearchEmptyItem::MUCSearchEmptyItem(MUCSearchServiceItem* parent) : parent(parent) {
-    parent->addRoom(this);
+MUCSearchEmptyItem::MUCSearchEmptyItem() {
+}
+
+void MUCSearchEmptyItem::setParent(std::shared_ptr<MUCSearchServiceItem> parent) {
+    parent_ = parent;
 }
 
-MUCSearchServiceItem* MUCSearchEmptyItem::getParent() {
-    return parent;
+std::shared_ptr<MUCSearchServiceItem> MUCSearchEmptyItem::getParent() {
+    return parent_.lock();
 }
 
 QVariant MUCSearchEmptyItem::data(int role) {
diff --git a/Swift/QtUI/MUCSearch/MUCSearchEmptyItem.h b/Swift/QtUI/MUCSearch/MUCSearchEmptyItem.h
index ca4b2b2..06c7c4e 100644
--- a/Swift/QtUI/MUCSearch/MUCSearchEmptyItem.h
+++ b/Swift/QtUI/MUCSearch/MUCSearchEmptyItem.h
@@ -1,11 +1,13 @@
 /*
- * Copyright (c) 2010 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #pragma once
 
+#include <memory>
+
 #include <Swift/QtUI/MUCSearch/MUCSearchItem.h>
 
 namespace Swift {
@@ -13,13 +15,14 @@ namespace Swift {
 
     class MUCSearchEmptyItem : public MUCSearchItem {
         public:
-            MUCSearchEmptyItem(MUCSearchServiceItem* parent);
+            MUCSearchEmptyItem();
 
-            MUCSearchServiceItem* getParent();
+            void setParent(std::shared_ptr<MUCSearchServiceItem> parent);
+            std::shared_ptr<MUCSearchServiceItem> getParent();
 
             QVariant data(int role);
 
         private:
-            MUCSearchServiceItem* parent;
+            std::weak_ptr<MUCSearchServiceItem> parent_;
     };
 }
diff --git a/Swift/QtUI/MUCSearch/MUCSearchItem.h b/Swift/QtUI/MUCSearch/MUCSearchItem.h
index c378247..d0e2a88 100644
--- a/Swift/QtUI/MUCSearch/MUCSearchItem.h
+++ b/Swift/QtUI/MUCSearch/MUCSearchItem.h
@@ -1,17 +1,23 @@
 /*
- * Copyright (c) 2010 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #pragma once
 
+#include <memory>
+
 #include <QVariant>
 
 namespace Swift {
-    class MUCSearchItem {
+
+class MUCSearchServiceItem;
+
+class MUCSearchItem {
         public:
             virtual ~MUCSearchItem() {}
+            virtual void setParent(std::shared_ptr<MUCSearchServiceItem>) { }
             virtual QVariant data(int role) = 0;
     };
 }
diff --git a/Swift/QtUI/MUCSearch/MUCSearchModel.cpp b/Swift/QtUI/MUCSearch/MUCSearchModel.cpp
index 2dbfc37..cc36f5f 100644
--- a/Swift/QtUI/MUCSearch/MUCSearchModel.cpp
+++ b/Swift/QtUI/MUCSearch/MUCSearchModel.cpp
@@ -6,6 +6,8 @@
 
 #include <Swift/QtUI/MUCSearch/MUCSearchModel.h>
 
+#include <memory>
+
 #include <Swift/QtUI/MUCSearch/MUCSearchEmptyItem.h>
 
 namespace Swift {
@@ -14,13 +16,19 @@ MUCSearchModel::MUCSearchModel() {
 }
 
 void MUCSearchModel::clear() {
-    emit layoutAboutToBeChanged();
+    // We need to reset the model, so that model indices containing raw pointers
+    // to MUCSearchServiceItems are invalaidated before we delete the
+    // MUCSearchServiceItems.
+    emit beginResetModel();
     services_.clear();
-    emit layoutChanged();
+    emit endResetModel();
 }
 
-void MUCSearchModel::addService(MUCSearchServiceItem* service) {
+void MUCSearchModel::addService(std::shared_ptr<MUCSearchServiceItem> service) {
     emit layoutAboutToBeChanged();
+    if (sortOrder_) {
+        service->setSorting(*sortOrder_);
+    }
     services_.push_back(service);
     emit layoutChanged();
 }
@@ -42,10 +50,8 @@ QModelIndex MUCSearchModel::index(int row, int column, const QModelIndex & paren
         MUCSearchServiceItem* parentItem = static_cast<MUCSearchServiceItem*>(parent.internalPointer());
         return row < parentItem->rowCount() ? createIndex(row, column, parentItem->getItem(row)) : QModelIndex();
     } else {
-        return row < services_.size() ? createIndex(row, column, services_[row]) : QModelIndex();
+        return row < services_.size() ? createIndex(row, column, services_[row].get()) : QModelIndex();
     }
-
-
 }
 
 QModelIndex MUCSearchModel::parent(const QModelIndex& index) const {
@@ -60,7 +66,7 @@ QModelIndex MUCSearchModel::parent(const QModelIndex& index) const {
         return QModelIndex();
     }
 
-    MUCSearchServiceItem* parent = nullptr;
+    std::shared_ptr<MUCSearchServiceItem> parent;
     if (MUCSearchRoomItem* roomItem = dynamic_cast<MUCSearchRoomItem*>(item)) {
         parent = roomItem->getParent();
     }
@@ -69,7 +75,7 @@ QModelIndex MUCSearchModel::parent(const QModelIndex& index) const {
     }
     if (parent) {
         int row = services_.indexOf(parent);
-        return createIndex(row, 1, parent);
+        return createIndex(row, 1, parent.get());
     }
     else {
         return QModelIndex();
@@ -88,4 +94,13 @@ int MUCSearchModel::rowCount(const QModelIndex& parentIndex) const {
     }
 }
 
+void MUCSearchModel::sort(int column, Qt::SortOrder order) {
+    sortOrder_ = order;
+    if (column == 0) {
+        for (auto&& serviceItem : services_) {
+            serviceItem->setSorting(*sortOrder_);
+        }
+    }
+}
+
 }
diff --git a/Swift/QtUI/MUCSearch/MUCSearchModel.h b/Swift/QtUI/MUCSearch/MUCSearchModel.h
index 2922ca6..f36d147 100644
--- a/Swift/QtUI/MUCSearch/MUCSearchModel.h
+++ b/Swift/QtUI/MUCSearch/MUCSearchModel.h
@@ -8,8 +8,10 @@
 
 #include <memory>
 
+#include <boost/optional.hpp>
+
 #include <QAbstractItemModel>
-#include <QList>
+#include <QVector>
 
 #include <Swift/QtUI/MUCSearch/MUCSearchServiceItem.h>
 
@@ -19,17 +21,17 @@ namespace Swift {
         public:
             MUCSearchModel();
             void clear();
-            void addService(MUCSearchServiceItem* service);
+            void addService(std::shared_ptr<MUCSearchServiceItem> service);
             int columnCount(const QModelIndex& parent = QModelIndex()) const;
             QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const;
             QModelIndex index(int row, int column, const QModelIndex & parent = QModelIndex()) const;
             QModelIndex parent(const QModelIndex& index) const;
             int rowCount(const QModelIndex& parent = QModelIndex()) const;
-//            ChatListItem* getItemForIndex(const QModelIndex& index) const;
+            virtual void sort(int column, Qt::SortOrder order = Qt::AscendingOrder);
+
         private:
-//            ChatListGroupItem* mucBookmarks_;
-//            ChatListGroupItem* root_;
-            QList<MUCSearchServiceItem*> services_;
+            QVector<std::shared_ptr<MUCSearchServiceItem>> services_;
+            boost::optional<Qt::SortOrder> sortOrder_;
     };
 
 }
diff --git a/Swift/QtUI/MUCSearch/MUCSearchRoomItem.cpp b/Swift/QtUI/MUCSearch/MUCSearchRoomItem.cpp
index cb7ee2d..3b786b6 100644
--- a/Swift/QtUI/MUCSearch/MUCSearchRoomItem.cpp
+++ b/Swift/QtUI/MUCSearch/MUCSearchRoomItem.cpp
@@ -6,16 +6,24 @@
 
 #include <Swift/QtUI/MUCSearch/MUCSearchRoomItem.h>
 
+#include <memory>
+
 #include <Swift/QtUI/MUCSearch/MUCSearchServiceItem.h>
 
 namespace Swift {
-MUCSearchRoomItem::MUCSearchRoomItem(const QString& node, MUCSearchServiceItem* parent) : parent_(parent), node_(node) {
-    parent_->addRoom(this);
+
+MUCSearchRoomItem::MUCSearchRoomItem(const QString& node) : node_(node) {
+
 }
 
-MUCSearchServiceItem* MUCSearchRoomItem::getParent() {
-    return parent_;
+void MUCSearchRoomItem::setParent(std::shared_ptr<MUCSearchServiceItem> parent) {
+    parent_ = parent;
 }
+
+std::shared_ptr<MUCSearchServiceItem> MUCSearchRoomItem::getParent() {
+    return parent_.lock();
+}
+
 QVariant MUCSearchRoomItem::data(int role) {
     switch (role) {
         case Qt::DisplayRole: return QVariant(node_);
diff --git a/Swift/QtUI/MUCSearch/MUCSearchRoomItem.h b/Swift/QtUI/MUCSearch/MUCSearchRoomItem.h
index d2b88bc..281b555 100644
--- a/Swift/QtUI/MUCSearch/MUCSearchRoomItem.h
+++ b/Swift/QtUI/MUCSearch/MUCSearchRoomItem.h
@@ -6,18 +6,22 @@
 
 #pragma once
 
+#include <memory>
+
 #include <Swift/QtUI/MUCSearch/MUCSearchItem.h>
 
 namespace Swift {
     class MUCSearchServiceItem;
     class MUCSearchRoomItem : public MUCSearchItem {
         public:
-            MUCSearchRoomItem(const QString& node, MUCSearchServiceItem* parent);
-            MUCSearchServiceItem* getParent();
+            MUCSearchRoomItem(const QString& node);
+            void setParent(std::shared_ptr<MUCSearchServiceItem> parent);
+            std::shared_ptr<MUCSearchServiceItem> getParent();
             QVariant data(int role);
             QString getNode() const {return node_;}
+
         private:
-            MUCSearchServiceItem* parent_;
+            std::weak_ptr<MUCSearchServiceItem> parent_;
             QString node_;
     };
 }
diff --git a/Swift/QtUI/MUCSearch/MUCSearchServiceItem.cpp b/Swift/QtUI/MUCSearch/MUCSearchServiceItem.cpp
new file mode 100644
index 0000000..57d5aac
--- /dev/null
+++ b/Swift/QtUI/MUCSearch/MUCSearchServiceItem.cpp
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2010-2016 Isode Limited.
+ * All rights reserved.
+ * See the COPYING file for more information.
+ */
+
+#include <Swift/QtUI/MUCSearch/MUCSearchServiceItem.h>
+
+#include <memory>
+
+#include <QString>
+#include <QVector>
+#include <QtAlgorithms>
+
+namespace Swift {
+
+MUCSearchServiceItem::MUCSearchServiceItem(const QString& jidString) : jidString_(jidString) {
+
+}
+
+void MUCSearchServiceItem::addRoom(std::shared_ptr<MUCSearchItem> room) {
+    room->setParent(shared_from_this());
+    rooms_.push_back(room);
+    if (sortOrder_) {
+        sort();
+    }
+}
+
+void MUCSearchServiceItem::addRooms(const std::vector<std::shared_ptr<MUCSearchItem> >& rooms) {
+    for (auto&& room: rooms) {
+        room->setParent(shared_from_this());
+        rooms_.push_back(room);
+    }
+    if (sortOrder_) {
+        sort();
+    }
+}
+
+int MUCSearchServiceItem::rowCount() {
+    return rooms_.count();
+}
+
+MUCSearchItem* MUCSearchServiceItem::getItem(int i) {
+    return rooms_[i].get();
+}
+
+QVariant MUCSearchServiceItem::data(int role) {
+    switch (role) {
+        case Qt::DisplayRole:
+            return QVariant(jidString_);
+        default:
+            return QVariant();
+    }
+}
+
+QString MUCSearchServiceItem::getHost() const {
+    return jidString_;
+}
+
+void MUCSearchServiceItem::setSorting(Qt::SortOrder sortOrder) {
+    sortOrder_ = sortOrder;
+    sort();
+}
+
+void MUCSearchServiceItem::sort() {
+    if (*sortOrder_ == Qt::AscendingOrder) {
+        qStableSort(rooms_.begin(), rooms_.end(), [](const std::shared_ptr<MUCSearchItem>& item1, const std::shared_ptr<MUCSearchItem>& item2) -> bool {
+            return QString::localeAwareCompare(item1->data(Qt::DisplayRole).toString(), item2->data(Qt::DisplayRole).toString()) < 0;
+        });
+    }
+    else {
+        qStableSort(rooms_.begin(), rooms_.end(), [](const std::shared_ptr<MUCSearchItem>& item1, const std::shared_ptr<MUCSearchItem>& item2) -> bool {
+            return QString::localeAwareCompare(item1->data(Qt::DisplayRole).toString(), item2->data(Qt::DisplayRole).toString()) > 0;
+        });
+    }
+}
+
+}
diff --git a/Swift/QtUI/MUCSearch/MUCSearchServiceItem.h b/Swift/QtUI/MUCSearch/MUCSearchServiceItem.h
index cfde071..9f5e000 100644
--- a/Swift/QtUI/MUCSearch/MUCSearchServiceItem.h
+++ b/Swift/QtUI/MUCSearch/MUCSearchServiceItem.h
@@ -6,27 +6,34 @@
 
 #pragma once
 
-#include <QList>
+#include <memory>
+
+#include <boost/optional.hpp>
+
 #include <QString>
+#include <QVector>
 
 #include <Swift/QtUI/MUCSearch/MUCSearchRoomItem.h>
 
 namespace Swift {
-    class MUCSearchServiceItem : public MUCSearchItem {
+    class MUCSearchServiceItem : public MUCSearchItem, public std::enable_shared_from_this<MUCSearchServiceItem> {
         public:
-            MUCSearchServiceItem(const QString& jidString) : jidString_(jidString) {}
-            void addRoom(MUCSearchItem* room) {rooms_.push_back(room);}
-            int rowCount() {return rooms_.count();}
-            MUCSearchItem* getItem(int i) {return rooms_[i];}
-            QVariant data(int role) {
-                switch (role) {
-                    case Qt::DisplayRole: return QVariant(jidString_);
-                    default: return QVariant();
-                }
-            }
-            QString getHost() const {return jidString_;}
+            MUCSearchServiceItem(const QString& jidString);
+
+            void addRoom(std::shared_ptr<MUCSearchItem> room);
+            void addRooms(const std::vector<std::shared_ptr<MUCSearchItem>>& rooms);
+            int rowCount();
+            MUCSearchItem* getItem(int i);
+            QVariant data(int role);
+            QString getHost() const;
+            void setSorting(Qt::SortOrder sortOrder);
+
+        private:
+            void sort();
+
         private:
-            QList<MUCSearchItem*> rooms_;
+            QVector<std::shared_ptr<MUCSearchItem>> rooms_;
             QString jidString_;
+            boost::optional<Qt::SortOrder> sortOrder_;
     };
 }
diff --git a/Swift/QtUI/MUCSearch/QtMUCSearchWindow.cpp b/Swift/QtUI/MUCSearch/QtMUCSearchWindow.cpp
index 8bef7e4..114ec1d 100644
--- a/Swift/QtUI/MUCSearch/QtMUCSearchWindow.cpp
+++ b/Swift/QtUI/MUCSearch/QtMUCSearchWindow.cpp
@@ -6,13 +6,14 @@
 
 #include <Swift/QtUI/MUCSearch/QtMUCSearchWindow.h>
 
+#include <memory>
+#include <vector>
+
 #include <QMovie>
 #include <QPushButton>
 #include <QScrollBar>
 #include <QTimer>
 
-#include <qdebug.h>
-
 #include <Swift/Controllers/UIEvents/AddMUCBookmarkUIEvent.h>
 #include <Swift/Controllers/UIEvents/JoinMUCUIEvent.h>
 
@@ -38,6 +39,8 @@ QtMUCSearchWindow::QtMUCSearchWindow() {
     ui_.results_->setRootIsDecorated(true);
     ui_.results_->setAnimated(true);
     ui_.results_->setAlternatingRowColors(true);
+    ui_.results_->setSortingEnabled(true);
+    ui_.results_->sortByColumn(0, Qt::AscendingOrder);
     connect(ui_.searchButton, SIGNAL(clicked()), this, SLOT(handleSearch()));
     connect(ui_.service_, SIGNAL(activated(const QString&)), this, SLOT(handleSearch(const QString&)));
     connect(ui_.results_->selectionModel(), SIGNAL(selectionChanged (const QItemSelection&, const QItemSelection&)), this, SLOT(handleSelectionChanged (const QItemSelection&, const QItemSelection&)));
@@ -86,7 +89,7 @@ void QtMUCSearchWindow::updateThrobberPosition() {
 
 void QtMUCSearchWindow::addSavedServices(const std::list<JID>& services) {
     ui_.service_->clear();
-    foreach (const JID& jid, services) {
+    for (const auto& jid : services) {
         ui_.service_->addItem(P2QSTRING(jid.toString()));
     }
     if (!services.empty()) {
@@ -127,14 +130,18 @@ void QtMUCSearchWindow::clearList() {
 
 void QtMUCSearchWindow::addService(const MUCService& service) {
     updateThrobberPosition();
-    MUCSearchServiceItem* serviceItem = new MUCSearchServiceItem(P2QSTRING(service.getJID().toString()));
+    auto serviceItem = std::make_shared<MUCSearchServiceItem>(P2QSTRING(service.getJID().toString()));
     if (service.getRooms().size() > 0) {
-        foreach (MUCService::MUCRoom room, service.getRooms()) {
-            new MUCSearchRoomItem(P2QSTRING(room.getNode()), serviceItem);
+        std::vector<std::shared_ptr<MUCSearchItem>> rooms;
+        for (auto&& room : service.getRooms()) {
+            if (!room.getNode().empty()) {
+                rooms.push_back(std::make_shared<MUCSearchRoomItem>(P2QSTRING(room.getNode())));
+            }
         }
+        serviceItem->addRooms(rooms);
     }
     else {
-        new MUCSearchEmptyItem(serviceItem);
+        serviceItem->addRoom(std::make_shared<MUCSearchEmptyItem>());
     }
     model_->addService(serviceItem);
     ui_.results_->expandAll();
diff --git a/Swift/QtUI/SConscript b/Swift/QtUI/SConscript
index 200bd58..1f15c15 100644
--- a/Swift/QtUI/SConscript
+++ b/Swift/QtUI/SConscript
@@ -142,7 +142,6 @@ sources = [
     "QtConnectionSettingsWindow.cpp",
     "Roster/RosterModel.cpp",
     "Roster/QtTreeWidget.cpp",
-#   "Roster/QtTreeWidgetItem.cpp",
     "Roster/RosterDelegate.cpp",
     "Roster/GroupItemDelegate.cpp",
     "Roster/DelegateCommons.cpp",
@@ -166,6 +165,7 @@ sources = [
     "MUCSearch/MUCSearchRoomItem.cpp",
     "MUCSearch/MUCSearchEmptyItem.cpp",
     "MUCSearch/MUCSearchDelegate.cpp",
+    "MUCSearch/MUCSearchServiceItem.cpp",
     "UserSearch/ContactListDelegate.cpp",
     "UserSearch/ContactListModel.cpp",
     "UserSearch/QtContactListWidget.cpp",
-- 
cgit v0.10.2-6-g49f6