From 20206ec92a1fed1c3a8c4075ffcd619847ce6e76 Mon Sep 17 00:00:00 2001 From: "Vladimir Golovnev (Glassez)" Date: Fri, 14 Aug 2020 16:32:09 +0300 Subject: [PATCH 1/2] Improve adding torrent using Magnet URI Closes #13249. --- src/base/bittorrent/session.cpp | 81 ++++++++++----------------------- src/base/bittorrent/session.h | 7 +-- src/gui/addnewtorrentdialog.cpp | 8 ++-- src/gui/addnewtorrentdialog.h | 2 +- 4 files changed, 31 insertions(+), 67 deletions(-) diff --git a/src/base/bittorrent/session.cpp b/src/base/bittorrent/session.cpp index 05ee5eff6..38b23c992 100644 --- a/src/base/bittorrent/session.cpp +++ b/src/base/bittorrent/session.cpp @@ -1728,18 +1728,9 @@ bool Session::cancelLoadMetadata(const InfoHash &hash) if (loadedMetadataIter == m_loadedMetadata.end()) return false; m_loadedMetadata.erase(loadedMetadataIter); - const lt::torrent_handle torrent = m_nativeSession->find_torrent(hash); - if (!torrent.is_valid()) return false; - - if (!torrent.status(lt::status_flags_t {0}).has_metadata) { - // if hidden torrent is still loading metadata... - --m_extraLimit; - adjustLimits(); - } - - // Remove it from session - m_nativeSession->remove_torrent(torrent, lt::session::delete_files); - qDebug("Preloaded torrent deleted."); + --m_extraLimit; + adjustLimits(); + m_nativeSession->remove_torrent(m_nativeSession->find_torrent(hash), lt::session::delete_files); return true; } @@ -1787,7 +1778,7 @@ void Session::decreaseTorrentsQueuePos(const QVector &hashes) } for (auto i = m_loadedMetadata.cbegin(); i != m_loadedMetadata.cend(); ++i) - torrentQueuePositionBottom(m_nativeSession->find_torrent(i.key())); + torrentQueuePositionBottom(m_nativeSession->find_torrent(*i)); saveTorrentsQueue(); } @@ -1836,7 +1827,7 @@ void Session::bottomTorrentsQueuePos(const QVector &hashes) } for (auto i = m_loadedMetadata.cbegin(); i != m_loadedMetadata.cend(); ++i) - torrentQueuePositionBottom(m_nativeSession->find_torrent(i.key())); + torrentQueuePositionBottom(m_nativeSession->find_torrent(*i)); saveTorrentsQueue(); } @@ -1891,42 +1882,14 @@ bool Session::addTorrent(const MagnetUri &magnetUri, const AddTorrentParams &par const auto it = m_loadedMetadata.constFind(hash); if (it != m_loadedMetadata.constEnd()) { - // Adding preloaded torrent... - const TorrentInfo metadata = it->metadata; - if (metadata.isValid()) { - // Metadata is received and torrent_handle is being deleted - // so we can't reuse it. Just add torrent using its metadata. - return addTorrent(metadata, params); - } - - // Reuse existing torrent_handle - lt::torrent_handle handle = m_nativeSession->find_torrent(hash); - - // Preloaded torrent is in "Upload mode" so we need to disable it - // otherwise the torrent never be downloaded (until application restart) - handle.unset_flags(lt::torrent_flags::upload_mode); - - LoadTorrentParams createTorrentParams = initLoadTorrentParams(params); - createTorrentParams.ltAddTorrentParams = it->ltAddTorrentParams; - - if (createTorrentParams.paused) { - // Preloaded torrent isn't auto managed already - handle.pause(); - } - else if (!createTorrentParams.forced) { - handle.set_flags(lt::torrent_flags::auto_managed); - handle.pause(); - } - - m_loadedMetadata.remove(hash); - m_loadingTorrents.insert(hash, createTorrentParams); - + // It looks illogical that we don't just use an existing handle, + // but as previous experience has shown, it actually creates unnecessary + // problems and unwanted behavior due to the fact that it was originally + // added with parameters other than those provided by the user. + m_loadedMetadata.erase(it); --m_extraLimit; adjustLimits(); - - // use common last step of torrent loading - createTorrentHandle(handle); - return true; + m_nativeSession->remove_torrent(m_nativeSession->find_torrent(hash), lt::session::delete_files); } return addTorrent_impl(params, magnetUri); @@ -2034,6 +1997,10 @@ bool Session::addTorrent_impl(const AddTorrentParams &addTorrentParams, const Ma } else { p = magnetUri.addTorrentParams(); + + if (loadTorrentParams.name.isEmpty() && !p.name.empty()) + loadTorrentParams.name = QString::fromStdString(p.name); + if (isTempPathEnabled()) actualSavePath = tempPath(); } @@ -2164,7 +2131,7 @@ bool Session::loadMetadata(const MagnetUri &magnetUri) if (ec) return false; // waiting for metadata... - m_loadedMetadata.insert(h.info_hash(), {p, TorrentInfo {}}); + m_loadedMetadata.insert(h.info_hash()); ++m_extraLimit; adjustLimits(); @@ -3953,6 +3920,10 @@ bool Session::loadTorrentResumeData(const QByteArray &data, const TorrentInfo &m const lt::bdecode_node addedTimeNode = root.dict_find("qBt-addedTime"); if (addedTimeNode.type() == lt::bdecode_node::int_t) p.added_time = addedTimeNode.int_value(); + + if (torrentParams.name.isEmpty() && !p.name.empty()) + torrentParams.name = QString::fromStdString(p.name); + } // === END DEPRECATED CODE === // else { @@ -4291,12 +4262,6 @@ void Session::handleTorrentRemovedAlert(const lt::torrent_removed_alert *p) { const InfoHash infoHash {p->info_hash}; - const auto loadedMetadataIter = m_loadedMetadata.find(infoHash); - if (loadedMetadataIter != m_loadedMetadata.end()) { - emit metadataLoaded(loadedMetadataIter->metadata); - m_loadedMetadata.erase(loadedMetadataIter); - } - const auto removingTorrentDataIter = m_removingTorrents.find(infoHash); if (removingTorrentDataIter != m_removingTorrents.end()) { if (removingTorrentDataIter->deleteOption == Torrent) { @@ -4348,10 +4313,14 @@ void Session::handleMetadataReceivedAlert(const lt::metadata_received_alert *p) const auto loadedMetadataIter = m_loadedMetadata.find(hash); if (loadedMetadataIter != m_loadedMetadata.end()) { + TorrentInfo metadata {p->handle.torrent_file()}; + + m_loadedMetadata.erase(loadedMetadataIter); --m_extraLimit; adjustLimits(); - loadedMetadataIter->metadata = TorrentInfo {p->handle.torrent_file()}; m_nativeSession->remove_torrent(p->handle, lt::session::delete_files); + + emit metadataLoaded(metadata); } } diff --git a/src/base/bittorrent/session.h b/src/base/bittorrent/session.h index ee0bfce46..406f3998f 100644 --- a/src/base/bittorrent/session.h +++ b/src/base/bittorrent/session.h @@ -746,12 +746,7 @@ namespace BitTorrent QThread *m_ioThread = nullptr; ResumeDataSavingManager *m_resumeDataSavingManager = nullptr; - struct LoadedMetadataHandle - { - lt::add_torrent_params ltAddTorrentParams {}; - TorrentInfo metadata; - }; - QHash m_loadedMetadata; + QSet m_loadedMetadata; QHash m_torrents; QHash m_loadingTorrents; diff --git a/src/gui/addnewtorrentdialog.cpp b/src/gui/addnewtorrentdialog.cpp index 68271296e..595693bdf 100644 --- a/src/gui/addnewtorrentdialog.cpp +++ b/src/gui/addnewtorrentdialog.cpp @@ -591,20 +591,20 @@ void AddNewTorrentDialog::reject() QDialog::reject(); } -void AddNewTorrentDialog::updateMetadata(const BitTorrent::TorrentInfo &info) +void AddNewTorrentDialog::updateMetadata(const BitTorrent::TorrentInfo &metadata) { - if (info.hash() != m_hash) return; + if (metadata.hash() != m_magnetURI.hash()) return; disconnect(BitTorrent::Session::instance(), &BitTorrent::Session::metadataLoaded, this, &AddNewTorrentDialog::updateMetadata); - if (!info.isValid()) { + if (!metadata.isValid()) { RaisedMessageBox::critical(this, tr("I/O Error"), ("Invalid metadata.")); setMetadataProgressIndicator(false, tr("Invalid metadata")); return; } // Good to go - m_torrentInfo = info; + m_torrentInfo = metadata; m_hasMetadata = true; setMetadataProgressIndicator(true, tr("Parsing metadata...")); diff --git a/src/gui/addnewtorrentdialog.h b/src/gui/addnewtorrentdialog.h index eed1fc1d0..9a638a9df 100644 --- a/src/gui/addnewtorrentdialog.h +++ b/src/gui/addnewtorrentdialog.h @@ -82,7 +82,7 @@ private slots: void displayContentTreeMenu(const QPoint &); void updateDiskSpaceLabel(); void onSavePathChanged(const QString &newPath); - void updateMetadata(const BitTorrent::TorrentInfo &info); + void updateMetadata(const BitTorrent::TorrentInfo &metadata); void handleDownloadFinished(const Net::DownloadResult &result); void TMMChanged(int index); void categoryChanged(int index); From 5727fcb001002e470f4b95576a6b4604b8c6792c Mon Sep 17 00:00:00 2001 From: "Vladimir Golovnev (Glassez)" Date: Mon, 7 Sep 2020 18:05:55 +0300 Subject: [PATCH 2/2] Store and use full magnet URI instead of hash --- src/gui/addnewtorrentdialog.cpp | 25 ++++++++++++++----------- src/gui/addnewtorrentdialog.h | 6 +++--- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/gui/addnewtorrentdialog.cpp b/src/gui/addnewtorrentdialog.cpp index 595693bdf..61f575e93 100644 --- a/src/gui/addnewtorrentdialog.cpp +++ b/src/gui/addnewtorrentdialog.cpp @@ -39,6 +39,7 @@ #include #include "base/bittorrent/downloadpriority.h" +#include "base/bittorrent/infohash.h" #include "base/bittorrent/magneturi.h" #include "base/bittorrent/session.h" #include "base/bittorrent/torrenthandle.h" @@ -276,11 +277,11 @@ bool AddNewTorrentDialog::loadTorrentFile(const QString &torrentPath) bool AddNewTorrentDialog::loadTorrentImpl() { m_hasMetadata = true; - m_hash = m_torrentInfo.hash(); + const BitTorrent::InfoHash infoHash = m_torrentInfo.hash(); // Prevent showing the dialog if download is already present - if (BitTorrent::Session::instance()->isKnownTorrent(m_hash)) { - BitTorrent::TorrentHandle *const torrent = BitTorrent::Session::instance()->findTorrent(m_hash); + if (BitTorrent::Session::instance()->isKnownTorrent(infoHash)) { + BitTorrent::TorrentHandle *const torrent = BitTorrent::Session::instance()->findTorrent(infoHash); if (torrent) { if (torrent->isPrivate() || m_torrentInfo.isPrivate()) { RaisedMessageBox::warning(this, tr("Torrent is already present"), tr("Torrent '%1' is already in the transfer list. Trackers haven't been merged because it is a private torrent.").arg(torrent->name()), QMessageBox::Ok); @@ -297,7 +298,7 @@ bool AddNewTorrentDialog::loadTorrentImpl() return false; } - m_ui->labelHashData->setText(m_hash); + m_ui->labelHashData->setText(infoHash); setupTreeview(); TMMChanged(m_ui->comboTTM->currentIndex()); m_ui->keepTopLevelFolderCheckBox->setEnabled(m_torrentInfo.hasRootFolder()); @@ -312,10 +313,11 @@ bool AddNewTorrentDialog::loadMagnet(const BitTorrent::MagnetUri &magnetUri) } m_torrentGuard = std::make_unique(); - m_hash = magnetUri.hash(); + + const BitTorrent::InfoHash infoHash = magnetUri.hash(); // Prevent showing the dialog if download is already present - if (BitTorrent::Session::instance()->isKnownTorrent(m_hash)) { - BitTorrent::TorrentHandle *const torrent = BitTorrent::Session::instance()->findTorrent(m_hash); + if (BitTorrent::Session::instance()->isKnownTorrent(infoHash)) { + BitTorrent::TorrentHandle *const torrent = BitTorrent::Session::instance()->findTorrent(infoHash); if (torrent) { if (torrent->isPrivate()) { RaisedMessageBox::warning(this, tr("Torrent is already present"), tr("Torrent '%1' is already in the transfer list. Trackers haven't been merged because it is a private torrent.").arg(torrent->name()), QMessageBox::Ok); @@ -335,7 +337,7 @@ bool AddNewTorrentDialog::loadMagnet(const BitTorrent::MagnetUri &magnetUri) connect(BitTorrent::Session::instance(), &BitTorrent::Session::metadataLoaded, this, &AddNewTorrentDialog::updateMetadata); // Set dialog title - QString torrentName = magnetUri.name(); + const QString torrentName = magnetUri.name(); setWindowTitle(torrentName.isEmpty() ? tr("Magnet link") : torrentName); setupTreeview(); @@ -343,8 +345,9 @@ bool AddNewTorrentDialog::loadMagnet(const BitTorrent::MagnetUri &magnetUri) BitTorrent::Session::instance()->loadMetadata(magnetUri); setMetadataProgressIndicator(true, tr("Retrieving metadata...")); - m_ui->labelHashData->setText(m_hash); + m_ui->labelHashData->setText(infoHash); + m_magnetURI = magnetUri; return true; } @@ -573,7 +576,7 @@ void AddNewTorrentDialog::accept() // Add torrent if (!m_hasMetadata) - BitTorrent::Session::instance()->addTorrent(m_hash, m_torrentParams); + BitTorrent::Session::instance()->addTorrent(m_magnetURI, m_torrentParams); else BitTorrent::Session::instance()->addTorrent(m_torrentInfo, m_torrentParams); @@ -585,7 +588,7 @@ void AddNewTorrentDialog::reject() { if (!m_hasMetadata) { setMetadataProgressIndicator(false); - BitTorrent::Session::instance()->cancelLoadMetadata(m_hash); + BitTorrent::Session::instance()->cancelLoadMetadata(m_magnetURI.hash()); } QDialog::reject(); diff --git a/src/gui/addnewtorrentdialog.h b/src/gui/addnewtorrentdialog.h index 9a638a9df..dda15330d 100644 --- a/src/gui/addnewtorrentdialog.h +++ b/src/gui/addnewtorrentdialog.h @@ -34,13 +34,13 @@ #include #include "base/bittorrent/addtorrentparams.h" -#include "base/bittorrent/infohash.h" +#include "base/bittorrent/magneturi.h" #include "base/bittorrent/torrentinfo.h" #include "base/settingvalue.h" namespace BitTorrent { - class MagnetUri; + class InfoHash; } namespace Net @@ -112,7 +112,7 @@ private: TorrentContentFilterModel *m_contentModel; PropListDelegate *m_contentDelegate; bool m_hasMetadata; - BitTorrent::InfoHash m_hash; + BitTorrent::MagnetUri m_magnetURI; BitTorrent::TorrentInfo m_torrentInfo; QByteArray m_headerState; int m_oldIndex;