Make sure that torrent file is deleted after sending torrent-add requ…

…est (#563) Also do it on the background thread.
equeim · Dec 25, 2024 · 3d76a2e · 3d76a2e
1 parent 28f6d05
commit 3d76a2e
Show file tree

Hide file tree

Showing 9 changed files with 90 additions and 55 deletions.
diff --git a/src/fileutils.cpp b/src/fileutils.cpp
@@ -216,29 +216,34 @@ namespace tremotesf {
         return data;
     }
 
-    void deleteFile(const QString& path) {
-        info().log("Deleting file {}", path);
-        QFile file(path);
-        if (file.remove()) {
-            info().log("Succesfully deleted file");
-        } else {
-            throw QFileError(fmt::format("Failed to delete {}: {}", fileDescription(file), errorDescription(file)));
+    namespace {
+        void deleteFileImpl(QFile& file) {
+            info().log("Deleting {}", fileDescription(file));
+            if (file.remove()) {
+                info().log("Succesfully deleted file");
+            } else {
+                throw QFileError(fmt::format("Failed to delete {}: {}", fileDescription(file), errorDescription(file)));
+            }
         }
     }
 
-    void moveFileToTrash(const QString& path) {
-        info().log("Moving file {} to trash", path);
-        QFile file(path);
+    void deleteFile(const QString& filePath) {
+        QFile file(filePath);
+        deleteFileImpl(file);
+    }
+
+    void moveFileToTrashOrDelete(const QString& filePath) {
+        QFile file(filePath);
+        info().log("Moving {} to trash", fileDescription(file));
         if (file.moveToTrash()) {
             if (const auto newPath = file.fileName(); !newPath.isEmpty()) {
                 info().log("Successfully moved file to trash, new path is {}", newPath);
             } else {
                 info().log("Successfully moved file to trash");
             }
         } else {
-            throw QFileError(
-                fmt::format("Failed to move {} to trash: {}", fileDescription(file), errorDescription(file))
-            );
+            warning().log("Failed to move {} to trash: {}", fileDescription(file), errorDescription(file));
+            deleteFileImpl(file);
         }
     }
 

diff --git a/src/fileutils.h b/src/fileutils.h
@@ -29,8 +29,8 @@ namespace tremotesf {
 
     [[nodiscard]] QByteArray readFile(const QString& path);
 
-    void deleteFile(const QString& path);
-    void moveFileToTrash(const QString& path);
+    void deleteFile(const QString &filePath);
+    void moveFileToTrashOrDelete(const QString &filePath);
 
     [[maybe_unused]]
     QString resolveExternalBundledResourcesPath(QLatin1String path);

diff --git a/src/rpc/rpc.cpp b/src/rpc/rpc.cpp
@@ -239,7 +239,8 @@ namespace tremotesf {
         std::vector<int> lowPriorityFiles,
         std::map<QString, QString> renamedFiles,
         TorrentData::Priority bandwidthPriority,
-        bool start
+        bool start,
+        DeleteFileMode deleteFileMode
     ) {
         if (isConnected()) {
             mBackgroundRequestsCoroutineScope.launch(addTorrentFileImpl(
@@ -250,7 +251,8 @@ namespace tremotesf {
                 std::move(lowPriorityFiles),
                 std::move(renamedFiles),
                 bandwidthPriority,
-                start
+                start,
+                deleteFileMode
             ));
         }
     }
@@ -285,6 +287,20 @@ namespace tremotesf {
                  {"paused"_l1, !start}}
             );
         }
+
+        Coroutine<> deleteTorrentFile(QString filePath, bool moveToTrash) {
+            co_await runOnThreadPool([moveToTrash, filePath = std::move(filePath)] {
+                try {
+                    if (moveToTrash) {
+                        moveFileToTrashOrDelete(filePath);
+                    } else {
+                        deleteFile(filePath);
+                    }
+                } catch (const QFileError& e) {
+                    warning().logWithException(e, "Failed to delete torrent file");
+                }
+            });
+        }
     }
 
     Coroutine<> Rpc::addTorrentFileImpl(
@@ -295,7 +311,8 @@ namespace tremotesf {
         std::vector<int> lowPriorityFiles,
         std::map<QString, QString> renamedFiles,
         TorrentData::Priority bandwidthPriority,
-        bool start
+        bool start,
+        DeleteFileMode deleteFileMode
     ) {
         std::optional<QByteArray> requestData = co_await runOnThreadPool(
             makeAddTorrentFileRequestData,
@@ -311,6 +328,9 @@ namespace tremotesf {
             emit torrentAddError(filePath);
             co_return;
         }
+        if (deleteFileMode != DeleteFileMode::No) {
+            mDeletingFilesCoroutineScope.launch(deleteTorrentFile(filePath, deleteFileMode == DeleteFileMode::MoveToTrash));
+        }
         if (!isConnected()) co_return;
         const auto response = co_await mRequestRouter->postRequest("torrent-add"_l1, std::move(requestData).value());
         if (response.arguments.contains(torrentDuplicateKey)) {

diff --git a/src/rpc/rpc.h b/src/rpc/rpc.h
@@ -126,6 +126,8 @@ namespace tremotesf {
         void connect();
         void disconnect();
 
+        enum class DeleteFileMode { No, Delete, MoveToTrash };
+
         void addTorrentFile(
             QString filePath,
             QString downloadDirectory,
@@ -134,7 +136,8 @@ namespace tremotesf {
             std::vector<int> lowPriorityFiles,
             std::map<QString, QString> renamedFiles,
             TorrentData::Priority bandwidthPriority,
-            bool start
+            bool start,
+            DeleteFileMode deleteFileMode
         );
 
         void addTorrentLinks(
@@ -182,7 +185,8 @@ namespace tremotesf {
             std::vector<int> lowPriorityFiles,
             std::map<QString, QString> renamedFiles,
             TorrentData::Priority bandwidthPriority,
-            bool start
+            bool start,
+            DeleteFileMode deleteFileMode
         );
         Coroutine<> addTorrentLinksImpl(
             QStringList links, QString downloadDirectory, TorrentData::Priority bandwidthPriority, bool start
@@ -211,6 +215,7 @@ namespace tremotesf {
         impl::RequestRouter* mRequestRouter{};
         CoroutineScope mBackgroundRequestsCoroutineScope{};
         CoroutineScope mAutoReconnectCoroutineScope{};
+        CoroutineScope mDeletingFilesCoroutineScope{};
 
         bool mAutoReconnectEnabled{};
 

diff --git a/src/ui/screens/addtorrent/addtorrentdialog.cpp b/src/ui/screens/addtorrent/addtorrentdialog.cpp
@@ -28,6 +28,7 @@
 #include <fmt/format.h>
 
 #include "addtorrenthelpers.h"
+#include "fileutils.h"
 #include "formatutils.h"
 #include "magnetlinkparser.h"
 #include "settings.h"
@@ -64,6 +65,19 @@ namespace tremotesf {
             }
             return row;
         }
+
+        Rpc::DeleteFileMode determineDeleteFileMode(const AddTorrentDialog::AddTorrentParametersWidgets& widgets) {
+            if (!widgets.deleteTorrentFileGroupBox || !widgets.moveTorrentFileToTrashCheckBox) {
+                return Rpc::DeleteFileMode::No;
+            }
+            if (!widgets.deleteTorrentFileGroupBox->isChecked()) {
+                return Rpc::DeleteFileMode::No;
+            }
+            if (widgets.moveTorrentFileToTrashCheckBox->isChecked()) {
+                return Rpc::DeleteFileMode::MoveToTrash;
+            }
+            return Rpc::DeleteFileMode::Delete;
+        }
     }
 
     AddTorrentDialog::AddTorrentDialog(Rpc* rpc, std::variant<FileParams, UrlParams> params, QWidget* parent)
@@ -95,7 +109,8 @@ namespace tremotesf {
                     mFilesModel->lowPriorityFiles(),
                     mFilesModel->renamedFiles(),
                     priorityFromComboBoxIndex(mAddTorrentParametersWidgets.priorityComboBox->currentIndex()),
-                    mAddTorrentParametersWidgets.startTorrentCheckBox->isChecked()
+                    mAddTorrentParametersWidgets.startTorrentCheckBox->isChecked(),
+                    determineDeleteFileMode(mAddTorrentParametersWidgets)
                 );
             }
         } else {
@@ -117,7 +132,6 @@ namespace tremotesf {
         if (settings->rememberAddTorrentParameters()) {
             mAddTorrentParametersWidgets.saveToSettings();
         }
-        deleteTorrentFileIfEnabled();
         QDialog::accept();
     }
 
@@ -349,7 +363,9 @@ namespace tremotesf {
             mTorrentFileInfoHashAndTrackers =
                 std::pair{std::move(torrentFile.infoHashV1), std::move(torrentFile.trackers)};
             if (checkIfTorrentFileExists()) {
-                deleteTorrentFileIfEnabled();
+                if (isAddingFile()) {
+                    co_await deleteTorrentFileIfEnabled();
+                }
                 close();
                 co_return;
             }
@@ -407,13 +423,22 @@ namespace tremotesf {
         return false;
     }
 
-    void AddTorrentDialog::deleteTorrentFileIfEnabled() {
-        if (isAddingFile() && mAddTorrentParametersWidgets.deleteTorrentFileGroupBox->isChecked()) {
-            deleteTorrentFile(
-                std::get<FileParams>(mParams).filePath,
-                mAddTorrentParametersWidgets.moveTorrentFileToTrashCheckBox->isChecked()
-            );
+    Coroutine<> AddTorrentDialog::deleteTorrentFileIfEnabled() {
+        const auto deleteFileMode = determineDeleteFileMode(mAddTorrentParametersWidgets);
+        if (deleteFileMode == Rpc::DeleteFileMode::No) {
+            co_return;
         }
+        co_await runOnThreadPool([deleteFileMode, filePath = std::get<FileParams>(mParams).filePath] {
+            try {
+                if (deleteFileMode == Rpc::DeleteFileMode::MoveToTrash) {
+                    moveFileToTrashOrDelete(filePath);
+                } else {
+                    deleteFile(filePath);
+                }
+            } catch (const QFileError& e) {
+                warning().logWithException(e, "Failed to delete torrent file");
+            }
+        });
     }
 
     void AddTorrentDialog::AddTorrentParametersWidgets::reset(Rpc* rpc) const {

diff --git a/src/ui/screens/addtorrent/addtorrentdialog.h b/src/ui/screens/addtorrent/addtorrentdialog.h
@@ -81,7 +81,7 @@ namespace tremotesf {
         void parseMagnetLinksAndCheckIfTorrentsExist(QStringList& urls);
         bool checkIfTorrentFileExists();
 
-        void deleteTorrentFileIfEnabled();
+        Coroutine<> deleteTorrentFileIfEnabled();
 
         Rpc* mRpc;
         std::variant<FileParams, UrlParams> mParams;

diff --git a/src/ui/screens/addtorrent/addtorrenthelpers.cpp b/src/ui/screens/addtorrent/addtorrenthelpers.cpp
@@ -9,9 +9,7 @@
 
 #include "addtorrenthelpers.h"
 
-#include "fileutils.h"
 #include "settings.h"
-#include "log/log.h"
 #include "rpc/rpc.h"
 #include "rpc/servers.h"
 #include "rpc/serversettings.h"
@@ -44,23 +42,6 @@ namespace tremotesf {
         };
     }
 
-    void deleteTorrentFile(const QString& filePath, bool moveToTrash) {
-        try {
-            if (moveToTrash) {
-                try {
-                    moveFileToTrash(filePath);
-                } catch (const QFileError& e) {
-                    warning().logWithException(e, "Failed to move torrent file to trash");
-                    deleteFile(filePath);
-                }
-            } else {
-                deleteFile(filePath);
-            }
-        } catch (const QFileError& e) {
-            warning().logWithException(e, "Failed to delete torrent file");
-        }
-    }
-
     QDialog* askForMergingTrackers(Torrent* torrent, std::vector<std::set<QString>> trackers, QWidget* parent) {
         auto* const settings = Settings::instance();
         QMessageBox* messageBox{};

diff --git a/src/ui/screens/addtorrent/addtorrenthelpers.h b/src/ui/screens/addtorrent/addtorrenthelpers.h
@@ -27,8 +27,6 @@ namespace tremotesf {
     AddTorrentParameters getAddTorrentParameters(Rpc* rpc);
     AddTorrentParameters getInitialAddTorrentParameters(Rpc* rpc);
 
-    void deleteTorrentFile(const QString& filePath, bool moveToTrash);
-
     QDialog* askForMergingTrackers(Torrent* torrent, std::vector<std::set<QString>> trackers, QWidget* parent);
 
     QString bencodeErrorString(bencode::Error::Type type);

diff --git a/src/ui/screens/mainwindow/mainwindowviewmodel.cpp b/src/ui/screens/mainwindow/mainwindowviewmodel.cpp
@@ -255,11 +255,12 @@ namespace tremotesf {
                 {},
                 {},
                 parameters.priority,
-                parameters.startAfterAdding
+                parameters.startAfterAdding,
+                parameters.deleteTorrentFile
+                    ? (parameters.moveTorrentFileToTrash ? Rpc::DeleteFileMode::MoveToTrash : Rpc::DeleteFileMode::Delete)
+                    : Rpc::DeleteFileMode::No
+
             );
-            if (parameters.deleteTorrentFile) {
-                deleteTorrentFile(filePath, parameters.moveTorrentFileToTrash);
-            }
         }
     }