From 3d76a2e90ca45df547b1b650b2c3616d35f848da Mon Sep 17 00:00:00 2001 From: Alexey Rochev Date: Tue, 24 Dec 2024 22:39:48 +0300 Subject: [PATCH] Make sure that torrent file is deleted after sending torrent-add request (#563) Also do it on the background thread. --- src/fileutils.cpp | 31 +++++++------ src/fileutils.h | 4 +- src/rpc/rpc.cpp | 26 +++++++++-- src/rpc/rpc.h | 9 +++- .../screens/addtorrent/addtorrentdialog.cpp | 43 +++++++++++++++---- src/ui/screens/addtorrent/addtorrentdialog.h | 2 +- .../screens/addtorrent/addtorrenthelpers.cpp | 19 -------- src/ui/screens/addtorrent/addtorrenthelpers.h | 2 - .../mainwindow/mainwindowviewmodel.cpp | 9 ++-- 9 files changed, 90 insertions(+), 55 deletions(-) diff --git a/src/fileutils.cpp b/src/fileutils.cpp index 7a34dc59..8959b0b7 100644 --- a/src/fileutils.cpp +++ b/src/fileutils.cpp @@ -216,19 +216,25 @@ 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); @@ -236,9 +242,8 @@ namespace tremotesf { 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 index 93680791..c3c302fb 100644 --- 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 index 8e08f15f..c06b8910 100644 --- a/src/rpc/rpc.cpp +++ b/src/rpc/rpc.cpp @@ -239,7 +239,8 @@ namespace tremotesf { std::vector lowPriorityFiles, std::map 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 lowPriorityFiles, std::map renamedFiles, TorrentData::Priority bandwidthPriority, - bool start + bool start, + DeleteFileMode deleteFileMode ) { std::optional 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 index 1bfa9084..225ac950 100644 --- 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 lowPriorityFiles, std::map renamedFiles, TorrentData::Priority bandwidthPriority, - bool start + bool start, + DeleteFileMode deleteFileMode ); void addTorrentLinks( @@ -182,7 +185,8 @@ namespace tremotesf { std::vector lowPriorityFiles, std::map 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 index 8a3faa70..e9935f5c 100644 --- a/src/ui/screens/addtorrent/addtorrentdialog.cpp +++ b/src/ui/screens/addtorrent/addtorrentdialog.cpp @@ -28,6 +28,7 @@ #include #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 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(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(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 index 25ccd7b0..3edcc955 100644 --- 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 mParams; diff --git a/src/ui/screens/addtorrent/addtorrenthelpers.cpp b/src/ui/screens/addtorrent/addtorrenthelpers.cpp index 7cdb201a..4c5b0745 100644 --- 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> 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 index 400a5a45..490f5416 100644 --- 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> 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 index 0f0ea11a..5b33619e 100644 --- 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); - } } }