Revise tag related implementations

Fix #12690.

src/app/application.cpp

@@ -350,11 +350,7 @@ void Application::runExternalProgram(const BitTorrent::Torrent *torrent) const
 #endif
             break;
         case u'G':
-            {
+            program.replace(i, 2, torrent->tags().join(QLatin1String(",")));
-                QStringList tags = torrent->tags().values();
-                std::sort(tags.begin(), tags.end(), Utils::Compare::NaturalLessThan<Qt::CaseInsensitive>());
-                program.replace(i, 2, tags.join(','));
-            }
             break;
         case u'I':
             program.replace(i, 2, torrent->id().toString());

src/base/CMakeLists.txt

@@ -58,6 +58,7 @@ add_library(qbt_base STATIC
     net/proxyconfigurationmanager.h
     net/reverseresolution.h
     net/smtp.h
+    orderedset.h
     preferences.h
     profile.h
     profile_p.h
@@ -74,6 +75,7 @@ add_library(qbt_base STATIC
     search/searchhandler.h
     search/searchpluginmanager.h
     settingsstorage.h
+    tagset.h
     torrentfileguard.h
     torrentfilter.h
     types.h
@@ -152,6 +154,7 @@ add_library(qbt_base STATIC
     search/searchhandler.cpp
     search/searchpluginmanager.cpp
     settingsstorage.cpp
+    tagset.cpp
     torrentfileguard.cpp
     torrentfilter.cpp
     utils/bytearray.cpp

src/base/base.pri

@@ -57,6 +57,7 @@ HEADERS += \
     $$PWD/net/proxyconfigurationmanager.h \
     $$PWD/net/reverseresolution.h \
     $$PWD/net/smtp.h \
+    $$PWD/orderedset.h \
     $$PWD/preferences.h \
     $$PWD/profile.h \
     $$PWD/profile_p.h \
@@ -74,6 +75,7 @@ HEADERS += \
     $$PWD/search/searchpluginmanager.h \
     $$PWD/settingsstorage.h \
     $$PWD/settingvalue.h \
+    $$PWD/tagset.h \
     $$PWD/torrentfileguard.h \
     $$PWD/torrentfilter.h \
     $$PWD/types.h \
@@ -152,6 +154,7 @@ SOURCES += \
     $$PWD/search/searchhandler.cpp \
     $$PWD/search/searchpluginmanager.cpp \
     $$PWD/settingsstorage.cpp \
+    $$PWD/tagset.cpp \
     $$PWD/torrentfileguard.cpp \
     $$PWD/torrentfilter.cpp \
     $$PWD/utils/bytearray.cpp \

src/base/bittorrent/addtorrentparams.h

@@ -30,10 +30,10 @@
 
 #include <optional>
 
-#include <QSet>
 #include <QString>
 #include <QVector>
 
+#include "base/tagset.h"
 #include "torrent.h"
 #include "torrentcontentlayout.h"
 
@@ -45,7 +45,7 @@ namespace BitTorrent
     {
         QString name;
         QString category;
-        QSet<QString> tags;
+        TagSet tags;
         QString savePath;
         bool disableTempPath = false; // e.g. for imported torrents
         bool sequential = false;

src/base/bittorrent/bencoderesumedatastorage.cpp

@@ -45,6 +45,7 @@
 #include "base/global.h"
 #include "base/logger.h"
 #include "base/profile.h"
+#include "base/tagset.h"
 #include "base/utils/fs.h"
 #include "base/utils/io.h"
 #include "base/utils/string.h"
@@ -82,7 +83,7 @@ namespace
 
     using ListType = lt::entry::list_type;
 
-    ListType setToEntryList(const QSet<QString> &input)
+    ListType setToEntryList(const TagSet &input)
     {
         ListType entryList;
         entryList.reserve(input.size());

src/base/bittorrent/loadtorrentparams.h

@@ -30,9 +30,9 @@
 
 #include <libtorrent/add_torrent_params.hpp>
 
-#include <QSet>
 #include <QString>
 
+#include "base/tagset.h"
 #include "torrent.h"
 #include "torrentcontentlayout.h"
 
@@ -44,7 +44,7 @@ namespace BitTorrent
 
         QString name;
         QString category;
-        QSet<QString> tags;
+        TagSet tags;
         QString savePath;
         TorrentContentLayout contentLayout = TorrentContentLayout::Original;
         TorrentOperatingMode operatingMode = TorrentOperatingMode::AutoManaged;

src/base/bittorrent/torrent.h

@@ -33,6 +33,7 @@
 #include <QString>
 #include <QtContainerFwd>
 
+#include "base/tagset.h"
 #include "abstractfilestorage.h"
 
 class QBitArray;
@@ -168,7 +169,7 @@ namespace BitTorrent
         virtual bool belongsToCategory(const QString &category) const = 0;
         virtual bool setCategory(const QString &category) = 0;
 
-        virtual QSet<QString> tags() const = 0;
+        virtual TagSet tags() const = 0;
         virtual bool hasTag(const QString &tag) const = 0;
         virtual bool addTag(const QString &tag) = 0;
         virtual bool removeTag(const QString &tag) = 0;

src/base/bittorrent/torrentimpl.cpp

@@ -703,7 +703,7 @@ bool TorrentImpl::belongsToCategory(const QString &category) const
     return false;
 }
 
-QSet<QString> TorrentImpl::tags() const
+TagSet TorrentImpl::tags() const
 {
     return m_tags;
 }
@@ -717,19 +717,19 @@ bool TorrentImpl::addTag(const QString &tag)
 {
     if (!Session::isValidTag(tag))
         return false;
+    if (hasTag(tag))
+        return false;
 
-    if (!hasTag(tag))
-    {
     if (!m_session->hasTag(tag))
+    {
         if (!m_session->addTag(tag))
             return false;
+    }
     m_tags.insert(tag);
     m_session->handleTorrentNeedSaveResumeData(this);
     m_session->handleTorrentTagAdded(this, tag);
     return true;
 }
-    return false;
-}
 
 bool TorrentImpl::removeTag(const QString &tag)
 {

src/base/bittorrent/torrentimpl.h

@@ -42,10 +42,10 @@
 #include <QMap>
 #include <QObject>
 #include <QQueue>
-#include <QSet>
 #include <QString>
 #include <QVector>
 
+#include "base/tagset.h"
 #include "infohash.h"
 #include "speedmonitor.h"
 #include "torrent.h"
@@ -115,7 +115,7 @@ namespace BitTorrent
         bool belongsToCategory(const QString &category) const override;
         bool setCategory(const QString &category) override;
 
-        QSet<QString> tags() const override;
+        TagSet tags() const override;
         bool hasTag(const QString &tag) const override;
         bool addTag(const QString &tag) override;
         bool removeTag(const QString &tag) override;
@@ -247,7 +247,7 @@ namespace BitTorrent
         QString actualStorageLocation() const;
 
     private:
-        typedef std::function<void ()> EventTrigger;
+        using EventTrigger = std::function<void ()>;
 
         void updateStatus();
         void updateStatus(const lt::torrent_status &nativeStatus);
@@ -315,7 +315,7 @@ namespace BitTorrent
         QString m_name;
         QString m_savePath;
         QString m_category;
-        QSet<QString> m_tags;
+        TagSet m_tags;
         qreal m_ratioLimit;
         int m_seedingTimeLimit;
         TorrentOperatingMode m_operatingMode;

src/base/orderedset.h

@@ -0,0 +1,108 @@
+/*
+ * Bittorrent Client using Qt and libtorrent.
+ * Copyright (C) 2021  Mike Tzou (Chocobo1)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * In addition, as a special exception, the copyright holders give permission to
+ * link this program with the OpenSSL project's "OpenSSL" library (or with
+ * modified versions of it that use the same license as the "OpenSSL" library),
+ * and distribute the linked executables. You must obey the GNU General Public
+ * License in all respects for all of the code used other than "OpenSSL".  If you
+ * modify file(s), you may extend this exception to your version of the file(s),
+ * but you are not obligated to do so. If you do not wish to do so, delete this
+ * exception statement from your version.
+ */
+
+#pragma once
+
+#include <functional>
+#include <set>
+#include <type_traits>
+
+#include "algorithm.h"
+
+template <typename T, typename Compare = std::less<T>>
+class OrderedSet : public std::set<T, Compare>
+{
+    using ThisType = OrderedSet<T, Compare>;
+
+public:
+    using BaseType = std::set<T, Compare>;
+
+    using key_type = typename BaseType::key_type;
+    using value_type = typename BaseType::value_type;
+
+    using BaseType::BaseType;
+    using BaseType::operator=;
+
+    // The following are custom functions that are in line with Qt API interface, such as `QSet`
+
+#if __cplusplus < 202002L
+    bool contains(const key_type &value) const
+    {
+        return (BaseType::find(value) != BaseType::cend());
+    }
+#endif
+
+    int count() const
+    {
+        return static_cast<int>(BaseType::size());
+    }
+
+    ThisType &intersect(const ThisType &other)
+    {
+        Algorithm::removeIf(*this, [&other](const value_type &value) -> bool
+        {
+            return !other.contains(value);
+        });
+        return *this;
+    }
+
+    bool isEmpty() const
+    {
+        return BaseType::empty();
+    }
+
+    template <typename std::enable_if_t<std::is_same_v<value_type, QString>, int> = 0>
+    QString join(const QString &separator) const
+    {
+        auto iter = BaseType::cbegin();
+        if (iter == BaseType::cend())
+            return {};
+
+        QString ret = *iter;
+        ++iter;
+
+        while (iter != BaseType::cend())
+        {
+            ret.push_back(separator + *iter);
+            ++iter;
+        }
+
+        return ret;
+    }
+
+    bool remove(const key_type &value)
+    {
+        return (BaseType::erase(value) > 0);
+    }
+
+    ThisType &unite(const ThisType &other)
+    {
+        BaseType::insert(other.cbegin(), other.cend());
+        return *this;
+    }
+};

src/base/tagset.cpp

@@ -0,0 +1,37 @@
+/*
+ * Bittorrent Client using Qt and libtorrent.
+ * Copyright (C) 2021  Mike Tzou (Chocobo1)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * In addition, as a special exception, the copyright holders give permission to
+ * link this program with the OpenSSL project's "OpenSSL" library (or with
+ * modified versions of it that use the same license as the "OpenSSL" library),
+ * and distribute the linked executables. You must obey the GNU General Public
+ * License in all respects for all of the code used other than "OpenSSL".  If you
+ * modify file(s), you may extend this exception to your version of the file(s),
+ * but you are not obligated to do so. If you do not wish to do so, delete this
+ * exception statement from your version.
+ */
+
+#include "tagset.h"
+
+bool TagLessThan::operator()(const QString &left, const QString &right) const
+{
+    const int result = m_compare(left, right);
+    if (result != 0)
+        return (result < 0);
+    return (m_subCompare(left, right) < 0);
+}

src/base/tagset.h

@@ -0,0 +1,49 @@
+/*
+ * Bittorrent Client using Qt and libtorrent.
+ * Copyright (C) 2021  Mike Tzou (Chocobo1)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * In addition, as a special exception, the copyright holders give permission to
+ * link this program with the OpenSSL project's "OpenSSL" library (or with
+ * modified versions of it that use the same license as the "OpenSSL" library),
+ * and distribute the linked executables. You must obey the GNU General Public
+ * License in all respects for all of the code used other than "OpenSSL".  If you
+ * modify file(s), you may extend this exception to your version of the file(s),
+ * but you are not obligated to do so. If you do not wish to do so, delete this
+ * exception statement from your version.
+ */
+
+#pragma once
+
+#include <QMetaType>
+#include <QString>
+
+#include "base/orderedset.h"
+#include "base/utils/compare.h"
+
+class TagLessThan
+{
+public:
+    bool operator()(const QString &left, const QString &right) const;
+
+private:
+    Utils::Compare::NaturalCompare<Qt::CaseInsensitive> m_compare;
+    Utils::Compare::NaturalCompare<Qt::CaseSensitive> m_subCompare;
+};
+
+using TagSet = OrderedSet<QString, TagLessThan>;
+
+Q_DECLARE_METATYPE(TagSet)

src/gui/tagfiltermodel.cpp

@@ -30,7 +30,6 @@
 
 #include <QDebug>
 #include <QIcon>
-#include <QSet>
 #include <QVector>
 
 #include "base/bittorrent/session.h"
@@ -315,7 +314,7 @@ TagModelItem *TagFilterModel::findItem(const QString &tag)
     return &m_tagItems[row];
 }
 
-QVector<TagModelItem *> TagFilterModel::findItems(const QSet<QString> &tags)
+QVector<TagModelItem *> TagFilterModel::findItems(const TagSet &tags)
 {
     QVector<TagModelItem *> items;
     items.reserve(tags.size());

src/gui/tagfiltermodel.h

@@ -31,6 +31,8 @@
 #include <QAbstractListModel>
 #include <QtContainerFwd>
 
+#include "base/tagset.h"
+
 class QModelIndex;
 
 class TagModelItem;
@@ -76,7 +78,7 @@ private:
     bool isValidRow(int row) const;
     int findRow(const QString &tag) const;
     TagModelItem *findItem(const QString &tag);
-    QVector<TagModelItem *> findItems(const QSet<QString> &tags);
+    QVector<TagModelItem *> findItems(const TagSet &tags);
     TagModelItem *allTagsItem();
     TagModelItem *untaggedItem();
 

src/gui/transferlistmodel.cpp

@@ -323,13 +323,6 @@ QString TransferListModel::displayValue(const BitTorrent::Torrent *torrent, cons
                      , Utils::Misc::userFriendlyDuration(seedingTime));
     };
 
-    const auto tagsString = [](const QSet<QString> &tags) -> QString
-    {
-        QStringList tagsList = tags.values();
-        tagsList.sort();
-        return tagsList.join(", ");
-    };
-
     const auto progressString = [](const qreal progress) -> QString
     {
         return (progress >= 1)
@@ -373,7 +366,7 @@ QString TransferListModel::displayValue(const BitTorrent::Torrent *torrent, cons
     case TR_CATEGORY:
         return torrent->category();
     case TR_TAGS:
-        return tagsString(torrent->tags());
+        return torrent->tags().join(QLatin1String(", "));
     case TR_ADD_DATE:
         return QLocale().toString(torrent->addedTime().toLocalTime(), QLocale::ShortFormat);
     case TR_SEED_DATE:
@@ -442,7 +435,7 @@ QVariant TransferListModel::internalValue(const BitTorrent::Torrent *torrent, co
     case TR_CATEGORY:
         return torrent->category();
     case TR_TAGS:
-        return QStringList {torrent->tags().values()};
+        return QVariant::fromValue(torrent->tags());
     case TR_ADD_DATE:
         return torrent->addedTime();
     case TR_SEED_DATE:

src/gui/transferlistsortmodel.cpp

@@ -58,6 +58,19 @@ namespace
         return isLeftValid ? -1 : 1;
     }
 
+    int customCompare(const TagSet &left, const TagSet &right, const Utils::Compare::NaturalCompare<Qt::CaseInsensitive> &compare)
+    {
+        for (auto leftIter = left.cbegin(), rightIter = right.cbegin();
+            (leftIter != left.cend()) && (rightIter != right.cend());
+            ++leftIter, ++rightIter)
+        {
+            const int result = compare(*leftIter, *rightIter);
+            if (result != 0)
+                return result;
+        }
+        return threeWayCompare(left.size(), right.size());
+    }
+
     template <typename T>
     int customCompare(const T left, const T right)
     {
@@ -140,10 +153,12 @@ int TransferListSortModel::compare(const QModelIndex &left, const QModelIndex &r
     case TransferListModel::TR_CATEGORY:
     case TransferListModel::TR_NAME:
     case TransferListModel::TR_SAVE_PATH:
-    case TransferListModel::TR_TAGS:
     case TransferListModel::TR_TRACKER:
         return m_naturalCompare(leftValue.toString(), rightValue.toString());
 
+    case TransferListModel::TR_TAGS:
+        return customCompare(leftValue.value<TagSet>(), rightValue.value<TagSet>(), m_naturalCompare);
+
     case TransferListModel::TR_AMOUNT_DOWNLOADED:
     case TransferListModel::TR_AMOUNT_DOWNLOADED_SESSION:
     case TransferListModel::TR_AMOUNT_LEFT:

src/gui/transferlistwidget.cpp

@@ -858,8 +858,8 @@ void TransferListWidget::displayListMenu(const QPoint &)
     bool firstAutoTMM = false;
     QString firstCategory;
     bool first = true;
-    QSet<QString> tagsInAny;
+    TagSet tagsInAny;
-    QSet<QString> tagsInAll;
+    TagSet tagsInAll;
 
     for (const QModelIndex &index : selectedIndexes)
     {
@@ -873,16 +873,17 @@ void TransferListWidget::displayListMenu(const QPoint &)
         if (firstCategory != torrent->category())
             allSameCategory = false;
 
-        tagsInAny.unite(torrent->tags());
+        const TagSet torrentTags = torrent->tags();
+        tagsInAny.unite(torrentTags);
 
         if (first)
         {
             firstAutoTMM = torrent->isAutoTMMEnabled();
-            tagsInAll = torrent->tags();
+            tagsInAll = torrentTags;
         }
         else
         {
-            tagsInAll.intersect(torrent->tags());
+            tagsInAll.intersect(torrentTags);
         }
 
         if (firstAutoTMM != torrent->isAutoTMMEnabled())
@@ -1011,8 +1012,7 @@ void TransferListWidget::displayListMenu(const QPoint &)
         action->setCloseOnTriggered(false);
 
         const Qt::CheckState initialState = tagsInAll.contains(tag) ? Qt::Checked
-                                            : tagsInAny.contains(tag) ? Qt::PartiallyChecked
+                                            : tagsInAny.contains(tag) ? Qt::PartiallyChecked : Qt::Unchecked;
-                                            : Qt::Unchecked;
         action->setCheckState(initialState);
 
         connect(action, &QAction::triggered, this, [this, tag](const bool checked)

src/webui/api/serialize/serialize_torrent.cpp

@@ -29,12 +29,12 @@
 #include "serialize_torrent.h"
 
 #include <QDateTime>
-#include <QSet>
 #include <QVector>
 
 #include "base/bittorrent/infohash.h"
 #include "base/bittorrent/torrent.h"
 #include "base/bittorrent/trackerentry.h"
+#include "base/tagset.h"
 #include "base/utils/fs.h"
 
 namespace
@@ -116,7 +116,7 @@ QVariantMap serialize(const BitTorrent::Torrent &torrent)
         {KEY_TORRENT_FIRST_LAST_PIECE_PRIO, torrent.hasFirstLastPiecePriority()},
 
         {KEY_TORRENT_CATEGORY, torrent.category()},
-        {KEY_TORRENT_TAGS, torrent.tags().values().join(", ")},
+        {KEY_TORRENT_TAGS, torrent.tags().join(QLatin1String(", "))},
         {KEY_TORRENT_SUPER_SEEDING, torrent.superSeeding()},
         {KEY_TORRENT_FORCE_START, torrent.isForced()},
         {KEY_TORRENT_SAVE_PATH, Utils::Fs::toNativePath(torrent.savePath())},

src/webui/api/serialize/serialize_torrent.h

@@ -28,7 +28,7 @@
 
 #pragma once
 
-#include <QVariantMap>
+#include <QVariant>
 
 namespace BitTorrent
 {

src/webui/api/torrentscontroller.cpp

@@ -637,7 +637,7 @@ void TorrentsController::addAction()
     const std::optional<bool> addPaused = parseBool(params()["paused"]);
     const QString savepath = params()["savepath"].trimmed();
     const QString category = params()["category"];
-    const QSet<QString> tags = List::toSet(params()["tags"].split(',', QString::SkipEmptyParts));
+    const QStringList tags = params()["tags"].split(',', QString::SkipEmptyParts);
     const QString torrentName = params()["rename"].trimmed();
     const int upLimit = parseInt(params()["upLimit"]).value_or(-1);
     const int dlLimit = parseInt(params()["dlLimit"]).value_or(-1);
@@ -676,7 +676,7 @@ void TorrentsController::addAction()
     addTorrentParams.contentLayout = contentLayout;
     addTorrentParams.savePath = savepath;
     addTorrentParams.category = category;
-    addTorrentParams.tags = tags;
+    addTorrentParams.tags.insert(tags.cbegin(), tags.cend());
     addTorrentParams.name = torrentName;
     addTorrentParams.uploadLimit = upLimit;
     addTorrentParams.downloadLimit = dlLimit;