Improve tracker entries handling

PR #19468.

* Use QHash to map tracker endpoints
* Don't clear numPeers unexpectedly
* Remove outdated tracker entry endpoints
* Move presentation logic from Core to GUI code
* Show all endpoints per tracker in tree structure

---------

Co-authored-by: Kacper Michajłow <kasper93@gmail.com>

src/base/bittorrent/sessionimpl.cpp

@@ -5920,7 +5920,7 @@ void SessionImpl::handleTrackerAlert(const lt::tracker_alert *a)
     if (!torrent)
         return;
 
-    QMap<TrackerEntry::Endpoint, int> &updateInfo = m_updatedTrackerEntries[torrent->nativeHandle()][std::string(a->tracker_url())];
+    QHash<TrackerEntry::Endpoint, int> &updateInfo = m_updatedTrackerEntries[torrent->nativeHandle()][std::string(a->tracker_url())];
 
     if (a->type() == lt::tracker_reply_alert::alert_type)
     {
@@ -6004,7 +6004,7 @@ void SessionImpl::processTrackerStatuses()
                         if (updatedTrackersIter == updatedTrackers.end())
                             continue;
 
-                        const QMap<TrackerEntry::Endpoint, int> &updateInfo = updatedTrackersIter.value();
+                        const QHash<TrackerEntry::Endpoint, int> &updateInfo = updatedTrackersIter.value();
                         TrackerEntry trackerEntry = torrent->updateTrackerEntry(announceEntry, updateInfo);
                         const QString url = trackerEntry.url;
                         updatedTrackerEntries.emplace(url, std::move(trackerEntry));

src/base/bittorrent/sessionimpl.h

@@ -746,7 +746,7 @@ namespace BitTorrent
 
         // This field holds amounts of peers reported by trackers in their responses to announces
         // (torrent.tracker_name.tracker_local_endpoint.num_peers)
-        QHash<lt::torrent_handle, QHash<std::string, QMap<TrackerEntry::Endpoint, int>>> m_updatedTrackerEntries;
+        QHash<lt::torrent_handle, QHash<std::string, QHash<TrackerEntry::Endpoint, int>>> m_updatedTrackerEntries;
 
         // I/O errored torrents
         QSet<TorrentID> m_recentErroredTorrents;

src/base/bittorrent/torrentimpl.cpp

@@ -80,16 +80,26 @@ namespace
 
 #ifdef QBT_USES_LIBTORRENT2
     void updateTrackerEntry(TrackerEntry &trackerEntry, const lt::announce_entry &nativeEntry
-            , const lt::info_hash_t &hashes, const QMap<TrackerEntry::Endpoint, int> &updateInfo)
+            , const lt::info_hash_t &hashes, const QHash<TrackerEntry::Endpoint, int> &updateInfo)
 #else
     void updateTrackerEntry(TrackerEntry &trackerEntry, const lt::announce_entry &nativeEntry
-            , const QMap<TrackerEntry::Endpoint, int> &updateInfo)
+            , const QHash<TrackerEntry::Endpoint, int> &updateInfo)
 #endif
     {
         Q_ASSERT(trackerEntry.url == QString::fromStdString(nativeEntry.url));
 
         trackerEntry.tier = nativeEntry.tier;
 
+        // remove outdated endpoints
+        trackerEntry.stats.removeIf([&nativeEntry](const decltype(trackerEntry.stats)::iterator &iter)
+        {
+            return std::none_of(nativeEntry.endpoints.cbegin(), nativeEntry.endpoints.cend()
+                    , [&endpoint = iter.key()](const auto &existingEndpoint)
+            {
+                return (endpoint == existingEndpoint.local_endpoint);
+            });
+        });
+
         int numUpdating = 0;
         int numWorking = 0;
         int numNotWorking = 0;
@@ -99,13 +109,17 @@ namespace
         const auto numEndpoints = static_cast<qsizetype>(nativeEntry.endpoints.size()) * ((hashes.has_v1() && hashes.has_v2()) ? 2 : 1);
         for (const lt::announce_endpoint &endpoint : nativeEntry.endpoints)
         {
+            const auto endpointName = QString::fromStdString((std::stringstream() << endpoint.local_endpoint).str());
+
             for (const auto protocolVersion : {lt::protocol_version::V1, lt::protocol_version::V2})
             {
                 if (hashes.has(protocolVersion))
                 {
                     const lt::announce_infohash &infoHash = endpoint.info_hashes[protocolVersion];
 
-                    TrackerEntry::EndpointStats trackerEndpoint;
+                    TrackerEntry::EndpointStats &trackerEndpoint = trackerEntry.stats[endpoint.local_endpoint][(protocolVersion == lt::protocol_version::V1) ? 1 : 2];
+
+                    trackerEndpoint.name = endpointName;
                     trackerEndpoint.numPeers = updateInfo.value(endpoint.local_endpoint, trackerEndpoint.numPeers);
                     trackerEndpoint.numSeeds = infoHash.scrape_complete;
                     trackerEndpoint.numLeeches = infoHash.scrape_incomplete;
@@ -131,20 +145,18 @@ namespace
                         trackerEndpoint.status = TrackerEntry::NotContacted;
                     }
 
-                    const QString trackerMessage = QString::fromStdString(infoHash.message);
-                    const QString errorMessage = QString::fromLocal8Bit(infoHash.last_error.message().c_str());
-                    trackerEndpoint.message = (!trackerMessage.isEmpty() ? trackerMessage : errorMessage);
-
-                    trackerEntry.stats[endpoint.local_endpoint][(protocolVersion == lt::protocol_version::V1) ? 1 : 2] = trackerEndpoint;
-                    trackerEntry.numPeers = std::max(trackerEntry.numPeers, trackerEndpoint.numPeers);
-                    trackerEntry.numSeeds = std::max(trackerEntry.numSeeds, trackerEndpoint.numSeeds);
-                    trackerEntry.numLeeches = std::max(trackerEntry.numLeeches, trackerEndpoint.numLeeches);
-                    trackerEntry.numDownloaded = std::max(trackerEntry.numDownloaded, trackerEndpoint.numDownloaded);
-
-                    if (firstTrackerMessage.isEmpty())
-                        firstTrackerMessage = trackerMessage;
-                    if (firstErrorMessage.isEmpty())
-                        firstErrorMessage = errorMessage;
+                    if (!infoHash.message.empty())
+                    {
+                        trackerEndpoint.message = QString::fromStdString(infoHash.message);
+                        if (firstTrackerMessage.isEmpty())
+                            firstTrackerMessage = trackerEndpoint.message;
+                    }
+                    else if (infoHash.last_error)
+                    {
+                        trackerEndpoint.message = QString::fromLocal8Bit(infoHash.last_error.message());
+                        if (firstErrorMessage.isEmpty())
+                            firstErrorMessage = trackerEndpoint.message;
+                    }
                 }
             }
         }
@@ -152,7 +164,9 @@ namespace
         const auto numEndpoints = static_cast<qsizetype>(nativeEntry.endpoints.size());
         for (const lt::announce_endpoint &endpoint : nativeEntry.endpoints)
         {
-            TrackerEntry::EndpointStats trackerEndpoint;
+            TrackerEntry::EndpointStats &trackerEndpoint = trackerEntry.stats[endpoint.local_endpoint][1];
+
+            trackerEndpoint.name = QString::fromStdString((std::stringstream() << endpoint.local_endpoint).str());
             trackerEndpoint.numPeers = updateInfo.value(endpoint.local_endpoint, trackerEndpoint.numPeers);
             trackerEndpoint.numSeeds = endpoint.scrape_complete;
             trackerEndpoint.numLeeches = endpoint.scrape_incomplete;
@@ -178,20 +192,18 @@ namespace
                 trackerEndpoint.status = TrackerEntry::NotContacted;
             }
 
-            const QString trackerMessage = QString::fromStdString(endpoint.message);
-            const QString errorMessage = QString::fromLocal8Bit(endpoint.last_error.message().c_str());
-            trackerEndpoint.message = (!trackerMessage.isEmpty() ? trackerMessage : errorMessage);
-
-            trackerEntry.stats[endpoint.local_endpoint][1] = trackerEndpoint;
-            trackerEntry.numPeers = std::max(trackerEntry.numPeers, trackerEndpoint.numPeers);
-            trackerEntry.numSeeds = std::max(trackerEntry.numSeeds, trackerEndpoint.numSeeds);
-            trackerEntry.numLeeches = std::max(trackerEntry.numLeeches, trackerEndpoint.numLeeches);
-            trackerEntry.numDownloaded = std::max(trackerEntry.numDownloaded, trackerEndpoint.numDownloaded);
-
-            if (firstTrackerMessage.isEmpty())
-                firstTrackerMessage = trackerMessage;
-            if (firstErrorMessage.isEmpty())
-                firstErrorMessage = errorMessage;
+            if (!endpoint.message.empty())
+            {
+                trackerEndpoint.message = QString::fromStdString(endpoint.message);
+                if (firstTrackerMessage.isEmpty())
+                    firstTrackerMessage = trackerEndpoint.message;
+            }
+            else if (endpoint.last_error)
+            {
+                trackerEndpoint.message = QString::fromLocal8Bit(endpoint.last_error.message());
+                if (firstErrorMessage.isEmpty())
+                    firstErrorMessage = trackerEndpoint.message;
+            }
         }
 #endif
 
@@ -1603,7 +1615,7 @@ void TorrentImpl::fileSearchFinished(const Path &savePath, const PathList &fileN
         endReceivedMetadataHandling(savePath, fileNames);
 }
 
-TrackerEntry TorrentImpl::updateTrackerEntry(const lt::announce_entry &announceEntry, const QMap<TrackerEntry::Endpoint, int> &updateInfo)
+TrackerEntry TorrentImpl::updateTrackerEntry(const lt::announce_entry &announceEntry, const QHash<TrackerEntry::Endpoint, int> &updateInfo)
 {
     const auto it = std::find_if(m_trackerEntries.begin(), m_trackerEntries.end()
             , [&announceEntry](const TrackerEntry &trackerEntry)

src/base/bittorrent/torrentimpl.h

@@ -264,7 +264,7 @@ namespace BitTorrent
         void saveResumeData(lt::resume_data_flags_t flags = {});
         void handleMoveStorageJobFinished(const Path &path, MoveStorageContext context, bool hasOutstandingJob);
         void fileSearchFinished(const Path &savePath, const PathList &fileNames);
-        TrackerEntry updateTrackerEntry(const lt::announce_entry &announceEntry, const QMap<TrackerEntry::Endpoint, int> &updateInfo);
+        TrackerEntry updateTrackerEntry(const lt::announce_entry &announceEntry, const QHash<TrackerEntry::Endpoint, int> &updateInfo);
 
     private:
         using EventTrigger = std::function<void ()>;

src/base/bittorrent/trackerentry.h

@@ -57,20 +57,15 @@ namespace BitTorrent
             int numLeeches = -1;
             int numDownloaded = -1;
             QString message {};
+            QString name {};
         };
 
         QString url {};
         int tier = 0;
+        Status status = NotContacted;
+        QString message {};
 
         QHash<Endpoint, QHash<int, EndpointStats>> stats {};
-
-        // Deprecated fields
-        Status status = NotContacted;
-        int numPeers = -1;
-        int numSeeds = -1;
-        int numLeeches = -1;
-        int numDownloaded = -1;
-        QString message {};
     };
 
     QVector<TrackerEntry> parseTrackerEntries(QStringView str);

src/gui/properties/trackerlistwidget.cpp

@@ -60,15 +60,17 @@
 TrackerListWidget::TrackerListWidget(PropertiesWidget *properties)
     : m_properties(properties)
 {
+#ifdef QBT_USES_LIBTORRENT2
+    setColumnHidden(COL_PROTOCOL, true); // Must be set before calling loadSettings()
+#endif
+
     // Set header
     // Must be set before calling loadSettings() otherwise the header is reset on restart
     setHeaderLabels(headerLabels());
     // Load settings
     loadSettings();
     // Graphical settings
-    setRootIsDecorated(false);
     setAllColumnsShowFocus(true);
-    setItemsExpandable(false);
     setSelectionMode(QAbstractItemView::ExtendedSelection);
     header()->setFirstSectionMovable(true);
     header()->setStretchLastSection(false); // Must be set after loadSettings() in order to work
@@ -80,8 +82,10 @@ TrackerListWidget::TrackerListWidget(PropertiesWidget *properties)
     // its size is 0, because explicitly 'showing' the column isn't enough
     // in the above scenario.
     for (int i = 0; i < COL_COUNT; ++i)
+    {
         if ((columnWidth(i) <= 0) && !isColumnHidden(i))
             resizeColumnToContents(i);
+    }
     // Context menu
     setContextMenuPolicy(Qt::CustomContextMenu);
     connect(this, &QWidget::customContextMenuRequested, this, &TrackerListWidget::showTrackerListMenu);
@@ -93,13 +97,13 @@ TrackerListWidget::TrackerListWidget(PropertiesWidget *properties)
     connect(header(), &QHeaderView::sortIndicatorChanged, this, &TrackerListWidget::saveSettings);
 
     // Set DHT, PeX, LSD items
-    m_DHTItem = new QTreeWidgetItem({ u""_s,  u"** [DHT] **"_s, u""_s, u"0"_s, u""_s, u""_s, u"0"_s });
+    m_DHTItem = new QTreeWidgetItem({ u"** [DHT] **"_s });
     insertTopLevelItem(0, m_DHTItem);
     setRowColor(0, QColorConstants::Svg::grey);
-    m_PEXItem = new QTreeWidgetItem({ u""_s,  u"** [PeX] **"_s, u""_s, u"0"_s, u""_s, u""_s, u"0"_s });
+    m_PEXItem = new QTreeWidgetItem({ u"** [PeX] **"_s });
     insertTopLevelItem(1, m_PEXItem);
     setRowColor(1, QColorConstants::Svg::grey);
-    m_LSDItem = new QTreeWidgetItem({ u""_s,  u"** [LSD] **"_s, u""_s, u"0"_s, u""_s, u""_s, u"0"_s });
+    m_LSDItem = new QTreeWidgetItem({ u"** [LSD] **"_s });
     insertTopLevelItem(2, m_LSDItem);
     setRowColor(2, QColorConstants::Svg::grey);
 
@@ -134,6 +138,22 @@ TrackerListWidget::TrackerListWidget(PropertiesWidget *properties)
     connect(copyHotkey, &QShortcut::activated, this, &TrackerListWidget::copyTrackerUrl);
 
     connect(this, &QAbstractItemView::doubleClicked, this, &TrackerListWidget::editSelectedTracker);
+    connect(this, &QTreeWidget::itemExpanded, this, [](QTreeWidgetItem *item)
+    {
+        item->setText(COL_PEERS, QString());
+        item->setText(COL_SEEDS, QString());
+        item->setText(COL_LEECHES, QString());
+        item->setText(COL_TIMES_DOWNLOADED, QString());
+        item->setText(COL_MSG, QString());
+    });
+    connect(this, &QTreeWidget::itemCollapsed, this, [](QTreeWidgetItem *item)
+    {
+        item->setText(COL_PEERS, item->data(COL_PEERS, Qt::UserRole).toString());
+        item->setText(COL_SEEDS, item->data(COL_SEEDS, Qt::UserRole).toString());
+        item->setText(COL_LEECHES, item->data(COL_LEECHES, Qt::UserRole).toString());
+        item->setText(COL_TIMES_DOWNLOADED, item->data(COL_TIMES_DOWNLOADED, Qt::UserRole).toString());
+        item->setText(COL_MSG, item->data(COL_MSG, Qt::UserRole).toString());
+    });
 }
 
 TrackerListWidget::~TrackerListWidget()
@@ -364,6 +384,33 @@ void TrackerListWidget::loadTrackers()
 
     loadStickyItems(torrent);
 
+    const auto setAlignment = [](QTreeWidgetItem *item)
+    {
+        for (const TrackerListColumn col : {COL_TIER, COL_PROTOCOL, COL_PEERS, COL_SEEDS, COL_LEECHES, COL_TIMES_DOWNLOADED})
+            item->setTextAlignment(col, (Qt::AlignRight | Qt::AlignVCenter));
+    };
+
+    const auto prettyCount = [](const int val)
+    {
+        return (val > -1) ? QString::number(val) : tr("N/A");
+    };
+
+    const auto toString = [](const BitTorrent::TrackerEntry::Status status)
+    {
+        switch (status)
+        {
+        case BitTorrent::TrackerEntry::Status::Working:
+            return tr("Working");
+        case BitTorrent::TrackerEntry::Status::Updating:
+            return tr("Updating...");
+        case BitTorrent::TrackerEntry::Status::NotWorking:
+            return tr("Not working");
+        case BitTorrent::TrackerEntry::Status::NotContacted:
+            return tr("Not contacted yet");
+        }
+        return tr("Invalid status!");
+    };
+
     // Load actual trackers information
     QStringList oldTrackerURLs = m_trackerItems.keys();
 
@@ -385,45 +432,59 @@ void TrackerListWidget::loadTrackers()
             oldTrackerURLs.removeOne(trackerURL);
         }
 
-        item->setText(COL_TIER, QString::number(entry.tier));
+        int peersMax = -1;
+        int seedsMax = -1;
+        int leechesMax = -1;
+        int downloadedMax = -1;
 
-        switch (entry.status)
+        int index = 0;
+        for (const auto &endpoint : entry.stats)
         {
-        case BitTorrent::TrackerEntry::Working:
-            item->setText(COL_STATUS, tr("Working"));
-            break;
-        case BitTorrent::TrackerEntry::Updating:
-            item->setText(COL_STATUS, tr("Updating..."));
-            break;
-        case BitTorrent::TrackerEntry::NotWorking:
-            item->setText(COL_STATUS, tr("Not working"));
-            break;
-        case BitTorrent::TrackerEntry::NotContacted:
-            item->setText(COL_STATUS, tr("Not contacted yet"));
-            break;
+            for (auto it = endpoint.cbegin(), end = endpoint.cend(); it != end; ++it)
+            {
+                const int protocolVersion = it.key();
+                const BitTorrent::TrackerEntry::EndpointStats &protocolStats = it.value();
+
+                peersMax = std::max(peersMax, protocolStats.numPeers);
+                seedsMax = std::max(seedsMax, protocolStats.numSeeds);
+                leechesMax = std::max(leechesMax, protocolStats.numLeeches);
+                downloadedMax = std::max(downloadedMax, protocolStats.numDownloaded);
+
+                QTreeWidgetItem *child = (index < item->childCount()) ? item->child(index) : new QTreeWidgetItem(item);
+                child->setText(COL_URL, protocolStats.name);
+                child->setText(COL_PROTOCOL, tr("v%1").arg(protocolVersion));
+                child->setText(COL_STATUS, toString(protocolStats.status));
+                child->setText(COL_PEERS, prettyCount(protocolStats.numPeers));
+                child->setText(COL_SEEDS, prettyCount(protocolStats.numSeeds));
+                child->setText(COL_LEECHES, prettyCount(protocolStats.numLeeches));
+                child->setText(COL_TIMES_DOWNLOADED, prettyCount(protocolStats.numDownloaded));
+                child->setText(COL_MSG, protocolStats.message);
+                child->setToolTip(COL_MSG, protocolStats.message);
+                setAlignment(child);
+                ++index;
+            }
         }
 
-        item->setText(COL_MSG, entry.message);
-        item->setToolTip(COL_MSG, entry.message);
-        item->setText(COL_PEERS, ((entry.numPeers > -1)
-            ? QString::number(entry.numPeers)
-            : tr("N/A")));
-        item->setText(COL_SEEDS, ((entry.numSeeds > -1)
-            ? QString::number(entry.numSeeds)
-            : tr("N/A")));
-        item->setText(COL_LEECHES, ((entry.numLeeches > -1)
-            ? QString::number(entry.numLeeches)
-            : tr("N/A")));
-        item->setText(COL_TIMES_DOWNLOADED, ((entry.numDownloaded > -1)
-            ? QString::number(entry.numDownloaded)
-            : tr("N/A")));
+        while (item->childCount() != index)
+            delete item->takeChild(index);
 
-        const Qt::Alignment alignment = (Qt::AlignRight | Qt::AlignVCenter);
-        item->setTextAlignment(COL_TIER, alignment);
-        item->setTextAlignment(COL_PEERS, alignment);
-        item->setTextAlignment(COL_SEEDS, alignment);
-        item->setTextAlignment(COL_LEECHES, alignment);
-        item->setTextAlignment(COL_TIMES_DOWNLOADED, alignment);
+        item->setText(COL_TIER, QString::number(entry.tier));
+        item->setText(COL_STATUS, toString(entry.status));
+
+        item->setData(COL_PEERS, Qt::UserRole, prettyCount(peersMax));
+        item->setData(COL_SEEDS, Qt::UserRole, prettyCount(seedsMax));
+        item->setData(COL_LEECHES, Qt::UserRole, prettyCount(leechesMax));
+        item->setData(COL_TIMES_DOWNLOADED, Qt::UserRole, prettyCount(downloadedMax));
+        item->setData(COL_MSG, Qt::UserRole, entry.message);
+        if (!item->isExpanded())
+        {
+            item->setText(COL_PEERS, item->data(COL_PEERS, Qt::UserRole).toString());
+            item->setText(COL_SEEDS, item->data(COL_SEEDS, Qt::UserRole).toString());
+            item->setText(COL_LEECHES, item->data(COL_LEECHES, Qt::UserRole).toString());
+            item->setText(COL_TIMES_DOWNLOADED, item->data(COL_TIMES_DOWNLOADED, Qt::UserRole).toString());
+            item->setText(COL_MSG, item->data(COL_MSG, Qt::UserRole).toString());
+        }
+        setAlignment(item);
     }
 
     // Remove old trackers
@@ -617,8 +678,9 @@ QStringList TrackerListWidget::headerLabels()
 {
     return
     {
-        tr("Tier")
-        , tr("URL")
+        tr("URL/Announce endpoint")
+        , tr("Tier")
+        , tr("Protocol")
         , tr("Status")
         , tr("Peers")
         , tr("Seeds")

src/gui/properties/trackerlistwidget.h

@@ -46,8 +46,9 @@ class TrackerListWidget : public QTreeWidget
 public:
     enum TrackerListColumn
     {
-        COL_TIER,
         COL_URL,
+        COL_TIER,
+        COL_PROTOCOL,
         COL_STATUS,
         COL_PEERS,
         COL_SEEDS,

src/webui/api/torrentscontroller.cpp

@@ -483,16 +483,31 @@ void TorrentsController::trackersAction()
 
     for (const BitTorrent::TrackerEntry &tracker : asConst(torrent->trackers()))
     {
+        int numPeers = -1;
+        int numSeeds = -1;
+        int numLeeches = -1;
+        int numDownloaded = -1;
+        for (const auto &endpoint : tracker.stats)
+        {
+            for (const auto &protocolStat : endpoint)
+            {
+                numPeers = std::max(numPeers, protocolStat.numPeers);
+                numSeeds = std::max(numSeeds, protocolStat.numSeeds);
+                numLeeches = std::max(numLeeches, protocolStat.numLeeches);
+                numDownloaded = std::max(numDownloaded, protocolStat.numDownloaded);
+            }
+        }
+
         trackerList << QJsonObject
         {
             {KEY_TRACKER_URL, tracker.url},
             {KEY_TRACKER_TIER, tracker.tier},
             {KEY_TRACKER_STATUS, static_cast<int>(tracker.status)},
             {KEY_TRACKER_MSG, tracker.message},
-            {KEY_TRACKER_PEERS_COUNT, tracker.numPeers},
-            {KEY_TRACKER_SEEDS_COUNT, tracker.numSeeds},
-            {KEY_TRACKER_LEECHES_COUNT, tracker.numLeeches},
-            {KEY_TRACKER_DOWNLOADED_COUNT, tracker.numDownloaded}
+            {KEY_TRACKER_PEERS_COUNT, numPeers},
+            {KEY_TRACKER_SEEDS_COUNT, numSeeds},
+            {KEY_TRACKER_LEECHES_COUNT, numLeeches},
+            {KEY_TRACKER_DOWNLOADED_COUNT, numDownloaded}
         };
     }