Improve move storage handling

PR #18857.
Closes #18795.

src/base/bittorrent/sessionimpl.cpp

@@ -1,6 +1,6 @@
 /*
  * Bittorrent Client using Qt and libtorrent.
- * Copyright (C) 2015-2022  Vladimir Golovnev <glassez@yandex.ru>
+ * Copyright (C) 2015-2023  Vladimir Golovnev <glassez@yandex.ru>
  * Copyright (C) 2006  Christophe Dumez <chris@qbittorrent.org>
  *
  * This program is free software; you can redistribute it and/or
@@ -72,6 +72,7 @@
 #endif
 #include <QNetworkInterface>
 #include <QRegularExpression>
+#include <QScopeGuard>
 #include <QString>
 #include <QThread>
 #include <QThreadPool>
@@ -4859,17 +4860,18 @@ void SessionImpl::handleTorrentInfoHashChanged(TorrentImpl *torrent, const InfoH
     }
 }
 
-bool SessionImpl::addMoveTorrentStorageJob(TorrentImpl *torrent, const Path &newPath, const MoveStorageMode mode)
+bool SessionImpl::addMoveTorrentStorageJob(TorrentImpl *torrent, const Path &newPath, const MoveStorageMode mode, const MoveStorageContext context)
 {
     Q_ASSERT(torrent);
 
     const lt::torrent_handle torrentHandle = torrent->nativeHandle();
     const Path currentLocation = torrent->actualStorageLocation();
+    const bool torrentHasActiveJob = !m_moveStorageQueue.isEmpty() && (m_moveStorageQueue.first().torrentHandle == torrentHandle);
 
     if (m_moveStorageQueue.size() > 1)
     {
         auto iter = std::find_if((m_moveStorageQueue.begin() + 1), m_moveStorageQueue.end()
-                                 , [&torrentHandle](const MoveStorageJob &job)
+                , [&torrentHandle](const MoveStorageJob &job)
         {
             return job.torrentHandle == torrentHandle;
         });
@@ -4877,20 +4879,13 @@ bool SessionImpl::addMoveTorrentStorageJob(TorrentImpl *torrent, const Path &new
         if (iter != m_moveStorageQueue.end())
         {
             // remove existing inactive job
+            torrent->handleMoveStorageJobFinished(currentLocation, iter->context, torrentHasActiveJob);
             LogMsg(tr("Torrent move canceled. Torrent: \"%1\". Source: \"%2\". Destination: \"%3\"").arg(torrent->name(), currentLocation.toString(), iter->path.toString()));
-            iter = m_moveStorageQueue.erase(iter);
-
-            iter = std::find_if(iter, m_moveStorageQueue.end(), [&torrentHandle](const MoveStorageJob &job)
-            {
-                return job.torrentHandle == torrentHandle;
-            });
-
-            const bool torrentHasOutstandingJob = (iter != m_moveStorageQueue.end());
-            torrent->handleMoveStorageJobFinished(currentLocation, torrentHasOutstandingJob);
+            m_moveStorageQueue.erase(iter);
         }
     }
 
-    if (!m_moveStorageQueue.isEmpty() && (m_moveStorageQueue.first().torrentHandle == torrentHandle))
+    if (torrentHasActiveJob)
     {
         // if there is active job for this torrent prevent creating meaningless
         // job that will move torrent to the same location as current one
@@ -4911,7 +4906,7 @@ bool SessionImpl::addMoveTorrentStorageJob(TorrentImpl *torrent, const Path &new
         }
     }
 
-    const MoveStorageJob moveStorageJob {torrentHandle, newPath, mode};
+    const MoveStorageJob moveStorageJob {torrentHandle, newPath, mode, context};
     m_moveStorageQueue << moveStorageJob;
     LogMsg(tr("Enqueued torrent move. Torrent: \"%1\". Source: \"%2\". Destination: \"%3\"").arg(torrent->name(), currentLocation.toString(), newPath.toString()));
 
@@ -4942,7 +4937,7 @@ void SessionImpl::handleMoveTorrentStorageJobFinished(const Path &newPath)
         moveTorrentStorage(m_moveStorageQueue.first());
 
     const auto iter = std::find_if(m_moveStorageQueue.cbegin(), m_moveStorageQueue.cend()
-                                   , [&finishedJob](const MoveStorageJob &job)
+            , [&finishedJob](const MoveStorageJob &job)
     {
         return job.torrentHandle == finishedJob.torrentHandle;
     });
@@ -4952,7 +4947,7 @@ void SessionImpl::handleMoveTorrentStorageJobFinished(const Path &newPath)
     TorrentImpl *torrent = m_torrents.value(finishedJob.torrentHandle.info_hash());
     if (torrent)
     {
-        torrent->handleMoveStorageJobFinished(newPath, torrentHasOutstandingJob);
+        torrent->handleMoveStorageJobFinished(newPath, finishedJob.context, torrentHasOutstandingJob);
     }
     else if (!torrentHasOutstandingJob)
     {
@@ -5839,7 +5834,7 @@ void SessionImpl::handleStorageMovedFailedAlert(const lt::storage_moved_failed_a
     TorrentImpl *torrent = m_torrents.value(id);
     const QString torrentName = (torrent ? torrent->name() : id.toString());
     const Path currentLocation = (torrent ? torrent->actualStorageLocation()
-                                          : Path(p->handle.status(lt::torrent_handle::query_save_path).save_path));
+            : Path(p->handle.status(lt::torrent_handle::query_save_path).save_path));
     const QString errorMessage = QString::fromStdString(p->message());
     LogMsg(tr("Failed to move torrent. Torrent: \"%1\". Source: \"%2\". Destination: \"%3\". Reason: \"%4\"")
            .arg(torrentName, currentLocation.toString(), currentJob.path.toString(), errorMessage), Log::WARNING);

src/base/bittorrent/sessionimpl.h

@@ -87,6 +87,7 @@ namespace BitTorrent
     struct LoadTorrentParams;
 
     enum class MoveStorageMode;
+    enum class MoveStorageContext;
 
     struct SessionMetricIndices
     {
@@ -439,7 +440,7 @@ namespace BitTorrent
         void handleTorrentResumeDataReady(TorrentImpl *const torrent, const LoadTorrentParams &data);
         void handleTorrentInfoHashChanged(TorrentImpl *torrent, const InfoHash &prevInfoHash);
 
-        bool addMoveTorrentStorageJob(TorrentImpl *torrent, const Path &newPath, MoveStorageMode mode);
+        bool addMoveTorrentStorageJob(TorrentImpl *torrent, const Path &newPath, MoveStorageMode mode, MoveStorageContext context);
 
         void findIncompleteFiles(const TorrentInfo &torrentInfo, const Path &savePath
                                  , const Path &downloadPath, const PathList &filePaths = {}) const;
@@ -482,6 +483,7 @@ namespace BitTorrent
             lt::torrent_handle torrentHandle;
             Path path;
             MoveStorageMode mode;
+            MoveStorageContext context;
         };
 
         struct RemovingTorrentData

src/base/bittorrent/torrentimpl.cpp

@@ -431,12 +431,16 @@ void TorrentImpl::setSavePath(const Path &path)
     if (resolvedPath == savePath())
         return;
 
-    m_savePath = resolvedPath;
-
-    m_session->handleTorrentNeedSaveResumeData(this);
-
     if (isFinished() || m_hasFinishedStatus || downloadPath().isEmpty())
-        moveStorage(savePath(), MoveStorageMode::KeepExistingFiles);
+    {
+        moveStorage(resolvedPath, MoveStorageContext::ChangeSavePath);
+    }
+    else
+    {
+        m_savePath = resolvedPath;
+        m_session->handleTorrentSavePathChanged(this);
+        m_session->handleTorrentNeedSaveResumeData(this);
+    }
 }
 
 Path TorrentImpl::downloadPath() const
@@ -454,13 +458,17 @@ void TorrentImpl::setDownloadPath(const Path &path)
     if (resolvedPath == m_downloadPath)
         return;
 
-    m_downloadPath = resolvedPath;
-
-    m_session->handleTorrentNeedSaveResumeData(this);
-
     const bool isIncomplete = !(isFinished() || m_hasFinishedStatus);
     if (isIncomplete)
-        moveStorage((m_downloadPath.isEmpty() ? savePath() : m_downloadPath), MoveStorageMode::KeepExistingFiles);
+    {
+        moveStorage((resolvedPath.isEmpty() ? savePath() : resolvedPath), MoveStorageContext::ChangeDownloadPath);
+    }
+    else
+    {
+        m_downloadPath = resolvedPath;
+        m_session->handleTorrentSavePathChanged(this);
+        m_session->handleTorrentNeedSaveResumeData(this);
+    }
 }
 
 Path TorrentImpl::rootPath() const
@@ -1754,7 +1762,7 @@ void TorrentImpl::resume(const TorrentOperatingMode mode)
     }
 }
 
-void TorrentImpl::moveStorage(const Path &newPath, const MoveStorageMode mode)
+void TorrentImpl::moveStorage(const Path &newPath, const MoveStorageContext context)
 {
     if (!hasMetadata())
     {
@@ -1762,7 +1770,9 @@ void TorrentImpl::moveStorage(const Path &newPath, const MoveStorageMode mode)
         return;
     }
 
-    if (m_session->addMoveTorrentStorageJob(this, newPath, mode))
+    const auto mode = (context == MoveStorageContext::AdjustCurrentLocation)
+            ? MoveStorageMode::Overwrite : MoveStorageMode::KeepExistingFiles;
+    if (m_session->addMoveTorrentStorageJob(this, newPath, mode, context))
     {
         m_storageIsMoving = true;
         updateState();
@@ -1784,16 +1794,17 @@ void TorrentImpl::handleStateUpdate(const lt::torrent_status &nativeStatus)
     updateStatus(nativeStatus);
 }
 
-void TorrentImpl::handleMoveStorageJobFinished(const Path &path, const bool hasOutstandingJob)
+void TorrentImpl::handleMoveStorageJobFinished(const Path &path, const MoveStorageContext context, const bool hasOutstandingJob)
 {
-    m_session->handleTorrentNeedSaveResumeData(this);
+    if (context == MoveStorageContext::ChangeSavePath)
+        m_savePath = path;
+    else if (context == MoveStorageContext::ChangeDownloadPath)
+        m_downloadPath = path;
     m_storageIsMoving = hasOutstandingJob;
+    m_nativeStatus.save_path = path.toString().toStdString();
 
-    if (actualStorageLocation() != path)
-    {
-        m_nativeStatus.save_path = path.toString().toStdString();
-        m_session->handleTorrentSavePathChanged(this);
-    }
+    m_session->handleTorrentSavePathChanged(this);
+    m_session->handleTorrentNeedSaveResumeData(this);
 
     if (!m_storageIsMoving)
     {
@@ -2221,7 +2232,7 @@ void TorrentImpl::adjustStorageLocation()
     const Path targetPath = ((isFinished() || m_hasFinishedStatus || downloadPath.isEmpty()) ? savePath() : downloadPath);
 
     if ((targetPath != actualStorageLocation()) || isMoveInProgress())
-        moveStorage(targetPath, MoveStorageMode::Overwrite);
+        moveStorage(targetPath, MoveStorageContext::AdjustCurrentLocation);
 }
 
 void TorrentImpl::doRenameFile(int index, const Path &path)

src/base/bittorrent/torrentimpl.h

@@ -68,6 +68,13 @@ namespace BitTorrent
         Overwrite
     };
 
+    enum class MoveStorageContext
+    {
+        AdjustCurrentLocation,
+        ChangeSavePath,
+        ChangeDownloadPath
+    };
+
     enum class MaintenanceJob
     {
         None,
@@ -252,7 +259,7 @@ namespace BitTorrent
         void handleCategoryOptionsChanged();
         void handleAppendExtensionToggled();
         void saveResumeData(lt::resume_data_flags_t flags = {});
-        void handleMoveStorageJobFinished(const Path &path, bool hasOutstandingJob);
+        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);
 
@@ -289,7 +296,7 @@ namespace BitTorrent
         Path wantedActualPath(int index, const Path &path) const;
         void adjustStorageLocation();
         void doRenameFile(int index, const Path &path);
-        void moveStorage(const Path &newPath, MoveStorageMode mode);
+        void moveStorage(const Path &newPath, MoveStorageContext context);
         void manageIncompleteFiles();
         void applyFirstLastPiecePriority(bool enabled);