Merge pull request #13151 from glassez/move-storage

Improve "move torrent storage" handling

src/base/bittorrent/session.cpp

@@ -3805,6 +3805,7 @@ bool Session::addMoveTorrentStorageJob(TorrentHandleImpl *torrent, const QString
     Q_ASSERT(torrent);
 
     const lt::torrent_handle torrentHandle = torrent->nativeHandle();
+    const QString currentLocation = torrent->actualStorageLocation();
 
     if (m_moveStorageQueue.size() > 1) {
         const auto iter = std::find_if(m_moveStorageQueue.begin() + 1, m_moveStorageQueue.end()
@@ -3816,22 +3817,30 @@ bool Session::addMoveTorrentStorageJob(TorrentHandleImpl *torrent, const QString
         if (iter != m_moveStorageQueue.end()) {
             // remove existing inactive job
             m_moveStorageQueue.erase(iter);
+            LogMsg(tr("Cancelled moving \"%1\" from \"%2\" to \"%3\".").arg(torrent->name(), currentLocation, iter->path));
         }
     }
 
-    QString currentLocation = torrent->actualStorageLocation();
     if (!m_moveStorageQueue.isEmpty() && (m_moveStorageQueue.first().torrentHandle == torrentHandle)) {
-        // if there is active job for this torrent consider its target path as current location
-        // of this torrent to prevent creating meaningless job that will do nothing
-        currentLocation = m_moveStorageQueue.first().path;
+        // if there is active job for this torrent prevent creating meaningless
+        // job that will move torrent to the same location as current one
+        if (QDir {m_moveStorageQueue.first().path} == QDir {newPath}) {
+            LogMsg(tr("Couldn't enqueue move of \"%1\" to \"%2\". Torrent is currently moving to the same destination location.")
+                   .arg(torrent->name(), newPath));
+            return false;
+        }
+    }
+    else {
+        if (QDir {currentLocation} == QDir {newPath}) {
+            LogMsg(tr("Couldn't enqueue move of \"%1\" from \"%2\" to \"%3\". Both paths point to the same location.")
+                   .arg(torrent->name(), currentLocation, newPath));
+            return false;
+        }
     }
-
-    if (QDir {currentLocation} == QDir {newPath})
-        return false;
 
     const MoveStorageJob moveStorageJob {torrentHandle, newPath, mode};
     m_moveStorageQueue << moveStorageJob;
-    qDebug("Move storage from \"%s\" to \"%s\" is enqueued.", qUtf8Printable(currentLocation), qUtf8Printable(newPath));
+    LogMsg(tr("Enqueued to move \"%1\" from \"%2\" to \"%3\".").arg(torrent->name(), currentLocation, newPath));
 
     if (m_moveStorageQueue.size() == 1)
         moveTorrentStorage(moveStorageJob);
@@ -3841,31 +3850,34 @@ bool Session::addMoveTorrentStorageJob(TorrentHandleImpl *torrent, const QString
 
 void Session::moveTorrentStorage(const MoveStorageJob &job) const
 {
-    qDebug("Moving torrent storage to \"%s\"...", qUtf8Printable(job.path));
+    const InfoHash infoHash = job.torrentHandle.info_hash();
+    const TorrentHandleImpl *torrent = m_torrents.value(infoHash);
+    const QString torrentName = (torrent ? torrent->name() : QString {infoHash});
+    LogMsg(tr("Moving \"%1\" to \"%2\"...").arg(torrentName, job.path));
 
     job.torrentHandle.move_storage(job.path.toUtf8().constData()
                             , ((job.mode == MoveStorageMode::Overwrite)
                             ? lt::move_flags_t::always_replace_files : lt::move_flags_t::dont_replace));
 }
 
-void Session::handleMoveTorrentStorageJobFinished(const QString &errorMessage)
+void Session::handleMoveTorrentStorageJobFinished()
 {
     const MoveStorageJob finishedJob = m_moveStorageQueue.takeFirst();
-    if (!m_moveStorageQueue.isEmpty())
-        moveTorrentStorage(m_moveStorageQueue.first());
 
     const auto iter = std::find_if(m_moveStorageQueue.cbegin(), m_moveStorageQueue.cend()
                                    , [&finishedJob](const MoveStorageJob &job)
     {
         return job.torrentHandle == finishedJob.torrentHandle;
     });
-    if (iter == m_moveStorageQueue.cend()) {
-        TorrentHandleImpl *torrent = m_torrents.value(finishedJob.torrentHandle.info_hash());
-        if (torrent) {
-            // There is no more job for this torrent
-            torrent->handleStorageMoved(finishedJob.path, errorMessage);
-        }
-        else {
+
+    const bool torrentHasOutstandingJob = (iter != m_moveStorageQueue.cend());
+
+    TorrentHandleImpl *torrent = m_torrents.value(finishedJob.torrentHandle.info_hash());
+    if (torrent)
+        torrent->handleMoveStorageJobFinished(torrentHasOutstandingJob);
+
+    if (!torrentHasOutstandingJob) {
+        if (!torrent) {
             // Last job is completed for torrent that being removing, so actually remove it
             const lt::torrent_handle nativeHandle {finishedJob.torrentHandle};
             const RemovingTorrentData &removingTorrentData = m_removingTorrents[nativeHandle.info_hash()];
@@ -3873,6 +3885,9 @@ void Session::handleMoveTorrentStorageJobFinished(const QString &errorMessage)
                 m_nativeSession->remove_torrent(nativeHandle, lt::session::delete_partfile);
         }
     }
+
+    if (!m_moveStorageQueue.isEmpty())
+        moveTorrentStorage(m_moveStorageQueue.first());
 }
 
 void Session::handleTorrentTrackerWarning(TorrentHandleImpl *const torrent, const QString &trackerUrl)
@@ -4684,15 +4699,29 @@ void Session::handleStorageMovedAlert(const lt::storage_moved_alert *p)
     const QString newPath {p->storage_path()};
     Q_ASSERT(newPath == currentJob.path);
 
+    const InfoHash infoHash = currentJob.torrentHandle.info_hash();
+    const TorrentHandleImpl *torrent = m_torrents.value(infoHash);
+    const QString torrentName = (torrent ? torrent->name() : QString {infoHash});
+    LogMsg(tr("\"%1\" is successfully moved to \"%2\".").arg(torrentName, newPath));
+
     handleMoveTorrentStorageJobFinished();
 }
 
 void Session::handleStorageMovedFailedAlert(const lt::storage_moved_failed_alert *p)
 {
     Q_ASSERT(!m_moveStorageQueue.isEmpty());
-    Q_ASSERT(m_moveStorageQueue.first().torrentHandle == p->handle);
 
-    handleMoveTorrentStorageJobFinished(QString::fromStdString(p->message()));
+    const MoveStorageJob &currentJob = m_moveStorageQueue.first();
+    Q_ASSERT(currentJob.torrentHandle == p->handle);
+
+    const InfoHash infoHash = currentJob.torrentHandle.info_hash();
+    const TorrentHandleImpl *torrent = m_torrents.value(infoHash);
+    const QString torrentName = (torrent ? torrent->name() : QString {infoHash});
+    const QString currentLocation = QString::fromStdString(p->handle.status(lt::torrent_handle::query_save_path).save_path);
+    LogMsg(tr("Failed to move \"%1\" from \"%2\" to \"%3\". Reason: %4.")
+           .arg(torrentName, currentLocation, currentJob.path, QString::fromStdString(p->message())), Log::CRITICAL);
+
+    handleMoveTorrentStorageJobFinished();
 }
 
 void Session::handleStateUpdateAlert(const lt::state_update_alert *p)

src/base/bittorrent/session.h

@@ -617,7 +617,7 @@ namespace BitTorrent
         std::vector<lt::alert *> getPendingAlerts(lt::time_duration time = lt::time_duration::zero()) const;
 
         void moveTorrentStorage(const MoveStorageJob &job) const;
-        void handleMoveTorrentStorageJobFinished(const QString &errorMessage = {});
+        void handleMoveTorrentStorageJobFinished();
 
         // BitTorrent
         lt::session *m_nativeSession = nullptr;

src/base/bittorrent/torrenthandleimpl.cpp

@@ -1237,11 +1237,13 @@ void TorrentHandleImpl::move_impl(QString path, const MoveStorageMode mode)
     if (path == savePath()) return;
     path = Utils::Fs::toNativePath(path);
 
-    if (!useTempPath())
+    if (!useTempPath()) {
         moveStorage(path, mode);
-
-    m_savePath = path;
-    m_session->handleTorrentSavePathChanged(this);
+    }
+    else {
+        m_savePath = path;
+        m_session->handleTorrentSavePathChanged(this);
+    }
 }
 
 void TorrentHandleImpl::forceReannounce(int index)
@@ -1373,16 +1375,17 @@ void TorrentHandleImpl::handleStateUpdate(const lt::torrent_status &nativeStatus
     updateStatus(nativeStatus);
 }
 
-void TorrentHandleImpl::handleStorageMoved(const QString &newPath, const QString &errorMessage)
+void TorrentHandleImpl::handleMoveStorageJobFinished(const bool hasOutstandingJob)
 {
-    m_storageIsMoving = false;
-
-    if (!errorMessage.isEmpty())
-        LogMsg(tr("Could not move torrent: %1. Reason: %2").arg(name(), errorMessage), Log::CRITICAL);
-    else
-        LogMsg(tr("Successfully moved torrent: %1. New path: %2").arg(name(), newPath));
+    m_storageIsMoving = hasOutstandingJob;
 
     updateStatus();
+    const QString newPath = QString::fromStdString(m_nativeStatus.save_path);
+    if (!useTempPath() && (newPath != m_savePath)) {
+        m_savePath = newPath;
+        m_session->handleTorrentSavePathChanged(this);
+    }
+
     saveResumeData();
 
     while ((m_renameCount == 0) && !m_moveFinishedTriggers.isEmpty())

src/base/bittorrent/torrenthandleimpl.h

@@ -252,7 +252,7 @@ namespace BitTorrent
         void handleCategorySavePathChanged();
         void handleAppendExtensionToggled();
         void saveResumeData();
-        void handleStorageMoved(const QString &newPath, const QString &errorMessage);
+        void handleMoveStorageJobFinished(bool hasOutstandingJob);
 
         QString actualStorageLocation() const;
 

src/gui/transferlistwidget.cpp

@@ -332,13 +332,8 @@ void TransferListWidget::setSelectedTorrentsLocation()
     if (newLocation.isEmpty() || !QDir(newLocation).exists()) return;
 
     // Actually move storage
-    for (BitTorrent::TorrentHandle *const torrent : torrents) {
-        Logger::instance()->addMessage(tr("Set location: moving \"%1\", from \"%2\" to \"%3\""
-            , "Set location: moving \"ubuntu_16_04.iso\", from \"/home/dir1\" to \"/home/dir2\"")
-            .arg(torrent->name(), Utils::Fs::toNativePath(torrent->savePath())
-                , Utils::Fs::toNativePath(newLocation)));
+    for (BitTorrent::TorrentHandle *const torrent : torrents)
         torrent->move(Utils::Fs::expandPathAbs(newLocation));
-    }
 }
 
 void TransferListWidget::pauseAllTorrents()