From 440860c4a9d1ee23d0c0fb30065fb695dfabba9b Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Tue, 4 Jun 2019 20:46:41 +0800 Subject: [PATCH 1/5] Properly remove empty leftover folders after rename TorrentInfo::origFilePath will return the very original path from .torrent file, not the most recent file path before the rename operation and thus the code would not be working as we expected. --- src/base/bittorrent/torrenthandle.cpp | 59 +++++++++++++++++---------- src/base/bittorrent/torrenthandle.h | 10 +++++ src/base/utils/string.cpp | 11 +++++ src/base/utils/string.h | 4 ++ src/gui/torrentcontenttreeview.cpp | 9 ---- 5 files changed, 63 insertions(+), 30 deletions(-) diff --git a/src/base/bittorrent/torrenthandle.cpp b/src/base/bittorrent/torrenthandle.cpp index 41a1a0a05..7061b2987 100644 --- a/src/base/bittorrent/torrenthandle.cpp +++ b/src/base/bittorrent/torrenthandle.cpp @@ -59,6 +59,7 @@ #include "base/profile.h" #include "base/tristatebool.h" #include "base/utils/fs.h" +#include "base/utils/string.h" #include "downloadpriority.h" #include "peerinfo.h" #include "session.h" @@ -1481,6 +1482,7 @@ void TorrentHandle::moveStorage(const QString &newPath, bool overwrite) void TorrentHandle::renameFile(const int index, const QString &name) { + m_oldPath[LTFileIndex {index}].push_back(filePath(index)); ++m_renameCount; m_nativeHandle.rename_file(index, Utils::Fs::toNativePath(name).toStdString()); } @@ -1744,29 +1746,42 @@ void TorrentHandle::handleFastResumeRejectedAlert(const lt::fastresume_rejected_ void TorrentHandle::handleFileRenamedAlert(const lt::file_renamed_alert *p) { - const QString newName = Utils::Fs::fromNativePath(p->new_name()); - - // TODO: Check this! - if (filesCount() > 1) { - // Check if folders were renamed - QStringList oldPathParts = m_torrentInfo.origFilePath(p->index).split('/'); - oldPathParts.removeLast(); - QString oldPath = oldPathParts.join('/'); - QStringList newPathParts = newName.split('/'); - newPathParts.removeLast(); - const QString newPath = newPathParts.join('/'); - if (!newPathParts.isEmpty() && (oldPath != newPath)) { - qDebug("oldPath(%s) != newPath(%s)", qUtf8Printable(oldPath), qUtf8Printable(newPath)); - oldPath = QString("%1/%2").arg(savePath(true), oldPath); - qDebug("Detected folder renaming, attempt to delete old folder: %s", qUtf8Printable(oldPath)); - QDir().rmpath(oldPath); - } - } - // We don't really need to call updateStatus() in this place. // All we need to do is make sure we have a valid instance of the TorrentInfo object. m_torrentInfo = TorrentInfo {m_nativeHandle.torrent_file()}; + // remove empty leftover folders + // for example renaming "a/b/c" to "d/b/c", then folders "a/b" and "a" will + // be removed if they are empty + const QString oldFilePath = m_oldPath[LTFileIndex {p->index}].takeFirst(); + const QString newFilePath = Utils::Fs::fromNativePath(p->new_name()); + + if (m_oldPath[LTFileIndex {p->index}].isEmpty()) + m_oldPath.remove(LTFileIndex {p->index}); + + QVector oldPathParts = oldFilePath.splitRef('/', QString::SkipEmptyParts); + oldPathParts.removeLast(); // drop file name part + QVector newPathParts = newFilePath.splitRef('/', QString::SkipEmptyParts); + newPathParts.removeLast(); // drop file name part + +#if defined(Q_OS_WIN) + const Qt::CaseSensitivity caseSensitivity = Qt::CaseInsensitive; +#else + const Qt::CaseSensitivity caseSensitivity = Qt::CaseSensitive; +#endif + + int pathIdx = 0; + while ((pathIdx < oldPathParts.size()) && (pathIdx < newPathParts.size())) { + if (oldPathParts[pathIdx].compare(newPathParts[pathIdx], caseSensitivity) != 0) + break; + ++pathIdx; + } + + for (int i = (oldPathParts.size() - 1); i >= pathIdx; --i) { + QDir().rmdir(savePath() + Utils::String::join(oldPathParts, QLatin1String("/"))); + oldPathParts.removeLast(); + } + --m_renameCount; while (!isMoveInProgress() && (m_renameCount == 0) && !m_moveFinishedTriggers.isEmpty()) m_moveFinishedTriggers.takeFirst()(); @@ -1774,12 +1789,14 @@ void TorrentHandle::handleFileRenamedAlert(const lt::file_renamed_alert *p) void TorrentHandle::handleFileRenameFailedAlert(const lt::file_rename_failed_alert *p) { - Q_UNUSED(p); - LogMsg(tr("File rename failed. Torrent: \"%1\", file: \"%2\", reason: \"%3\"") .arg(name(), filePath(p->index) , QString::fromStdString(p->error.message())), Log::WARNING); + m_oldPath[LTFileIndex {p->index}].removeFirst(); + if (m_oldPath[LTFileIndex {p->index}].isEmpty()) + m_oldPath.remove(LTFileIndex {p->index}); + --m_renameCount; while (!isMoveInProgress() && (m_renameCount == 0) && !m_moveFinishedTriggers.isEmpty()) m_moveFinishedTriggers.takeFirst()(); diff --git a/src/base/bittorrent/torrenthandle.h b/src/base/bittorrent/torrenthandle.h index 4513d805e..7471957c3 100644 --- a/src/base/bittorrent/torrenthandle.h +++ b/src/base/bittorrent/torrenthandle.h @@ -355,6 +355,12 @@ namespace BitTorrent private: typedef std::function EventTrigger; +#if (LIBTORRENT_VERSION_NUM < 10200) + using LTFileIndex = int; +#else + using LTFileIndex = lt::file_index_t; +#endif + void updateStatus(); void updateStatus(const lt::torrent_status &nativeStatus); void updateState(); @@ -417,6 +423,10 @@ namespace BitTorrent QQueue m_moveFinishedTriggers; int m_renameCount; + // Until libtorrent provide an "old_name" field in `file_renamed_alert` + // we will rely on this workaround to remove empty leftover folders + QHash> m_oldPath; + bool m_useAutoTMM; // Persistent data diff --git a/src/base/utils/string.cpp b/src/base/utils/string.cpp index f1412b23e..78dd080df 100644 --- a/src/base/utils/string.cpp +++ b/src/base/utils/string.cpp @@ -198,3 +198,14 @@ TriStateBool Utils::String::parseTriStateBool(const QString &string) return TriStateBool::False; return TriStateBool::Undefined; } + +QString Utils::String::join(const QVector &strings, const QString &separator) +{ + if (strings.empty()) + return {}; + + QString ret = strings[0].toString(); + for (int i = 1; i < strings.count(); ++i) + ret += (separator + strings[i]); + return ret; +} diff --git a/src/base/utils/string.h b/src/base/utils/string.h index a04ac6fc3..53e6392da 100644 --- a/src/base/utils/string.h +++ b/src/base/utils/string.h @@ -31,8 +31,10 @@ #define UTILS_STRING_H #include +#include class QString; +class QStringRef; class TriStateBool; @@ -66,6 +68,8 @@ namespace Utils bool parseBool(const QString &string, bool defaultValue); TriStateBool parseTriStateBool(const QString &string); + + QString join(const QVector &strings, const QString &separator); } } diff --git a/src/gui/torrentcontenttreeview.cpp b/src/gui/torrentcontenttreeview.cpp index 1a0761a23..af38e34d3 100644 --- a/src/gui/torrentcontenttreeview.cpp +++ b/src/gui/torrentcontenttreeview.cpp @@ -204,15 +204,6 @@ void TorrentContentTreeView::renameSelectedFile(BitTorrent::TorrentHandle *torre if (needForceRecheck) torrent->forceRecheck(); - // Remove old folder - const QString oldFullPath = torrent->savePath(true) + oldPath; - int timeout = 10; - while (!QDir().rmpath(oldFullPath) && (timeout > 0)) { - // FIXME: We should not sleep here (freezes the UI for 1 second) - QThread::msleep(100); - --timeout; - } - model->setData(modelIndex, newName); } } From 4e87aebf557c1225adddf2408f48cb1bdd7e936f Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Tue, 11 Jun 2019 00:15:06 +0800 Subject: [PATCH 2/5] Don't remove parent directories QDir::rmpath removes *all* parent directories while QDir::rmdir removes the specified directory. --- src/base/bittorrent/session.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/base/bittorrent/session.cpp b/src/base/bittorrent/session.cpp index a857c1df2..55910b37a 100644 --- a/src/base/bittorrent/session.cpp +++ b/src/base/bittorrent/session.cpp @@ -1657,7 +1657,7 @@ bool Session::deleteTorrent(const QString &hash, const bool deleteLocalFiles) Utils::Fs::forceRemove(unwantedFile); const QString parentFolder = Utils::Fs::branchPath(unwantedFile); qDebug("Attempt to remove parent folder (if empty): %s", qUtf8Printable(parentFolder)); - QDir().rmpath(parentFolder); + QDir().rmdir(parentFolder); } } From 7d860b6c243c7171f291cfaf6f28d78371253d00 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Tue, 11 Jun 2019 02:11:28 +0800 Subject: [PATCH 3/5] Log save_resume_data_failed_alert --- src/base/bittorrent/torrenthandle.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/base/bittorrent/torrenthandle.cpp b/src/base/bittorrent/torrenthandle.cpp index 7061b2987..f325ee75c 100644 --- a/src/base/bittorrent/torrenthandle.cpp +++ b/src/base/bittorrent/torrenthandle.cpp @@ -1725,10 +1725,14 @@ void TorrentHandle::handleSaveResumeDataFailedAlert(const lt::save_resume_data_f { // if torrent has no metadata we should save dummy fastresume data // containing Magnet URI and qBittorrent own resume data only - if (p->error.value() == lt::errors::no_metadata) + if (p->error.value() == lt::errors::no_metadata) { handleSaveResumeDataAlert(nullptr); - else + } + else { + LogMsg(tr("Save resume data failed. Torrent: \"%1\", error: \"%2\"") + .arg(name(), QString::fromStdString(p->error.message())), Log::CRITICAL); m_session->handleTorrentResumeDataFailed(this); + } } void TorrentHandle::handleFastResumeRejectedAlert(const lt::fastresume_rejected_alert *p) From 9893a415c0408d65cf2ec7b5a9eda11c6fe9eaf6 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Tue, 11 Jun 2019 11:59:08 +0800 Subject: [PATCH 4/5] Fix updated save path not saved for paused torrents --- src/base/bittorrent/torrenthandle.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/base/bittorrent/torrenthandle.cpp b/src/base/bittorrent/torrenthandle.cpp index f325ee75c..f89bd5ada 100644 --- a/src/base/bittorrent/torrenthandle.cpp +++ b/src/base/bittorrent/torrenthandle.cpp @@ -1789,6 +1789,9 @@ void TorrentHandle::handleFileRenamedAlert(const lt::file_renamed_alert *p) --m_renameCount; while (!isMoveInProgress() && (m_renameCount == 0) && !m_moveFinishedTriggers.isEmpty()) m_moveFinishedTriggers.takeFirst()(); + + if (isPaused() && (m_renameCount == 0)) + saveResumeData(); // otherwise the new path will not be saved } void TorrentHandle::handleFileRenameFailedAlert(const lt::file_rename_failed_alert *p) @@ -1804,6 +1807,9 @@ void TorrentHandle::handleFileRenameFailedAlert(const lt::file_rename_failed_ale --m_renameCount; while (!isMoveInProgress() && (m_renameCount == 0) && !m_moveFinishedTriggers.isEmpty()) m_moveFinishedTriggers.takeFirst()(); + + if (isPaused() && (m_renameCount == 0)) + saveResumeData(); // otherwise the new path will not be saved } void TorrentHandle::handleFileCompletedAlert(const lt::file_completed_alert *p) From a64f3bbc6a6dab171bf819ea4a8cae53aaab52ef Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Wed, 12 Jun 2019 10:54:43 +0800 Subject: [PATCH 5/5] Reorder if conditions slightly --- src/base/bittorrent/session.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/base/bittorrent/session.cpp b/src/base/bittorrent/session.cpp index 55910b37a..62c42e5ad 100644 --- a/src/base/bittorrent/session.cpp +++ b/src/base/bittorrent/session.cpp @@ -2131,9 +2131,13 @@ void Session::generateResumeData(const bool final) { for (TorrentHandle *const torrent : asConst(m_torrents)) { if (!torrent->isValid()) continue; - if (torrent->isChecking() || torrent->isPaused()) continue; + if (!final && !torrent->needSaveResumeData()) continue; - if (torrent->hasMissingFiles() || torrent->hasError()) continue; + if (torrent->isChecking() + || torrent->isPaused() + || torrent->hasError() + || torrent->hasMissingFiles()) + continue; saveTorrentResumeData(torrent); }