Don't use output parameters for error handling

This commit is contained in:
Vladimir Golovnev (Glassez) 2021-10-06 21:45:37 +03:00
parent 4d480b8761
commit 41fc0fd084
No known key found for this signature in database
GPG Key ID: 52A2C7DEE2DFA6F7
12 changed files with 116 additions and 165 deletions

View File

@ -1698,7 +1698,7 @@ void Session::handleDownloadFinished(const Net::DownloadResult &result)
{
case Net::DownloadStatus::Success:
emit downloadFromUrlFinished(result.url);
addTorrent(TorrentInfo::load(result.data), m_downloadedTorrents.take(result.url));
addTorrent(TorrentInfo::load(result.data).value_or(TorrentInfo()), m_downloadedTorrents.take(result.url));
break;
case Net::DownloadStatus::RedirectedToMagnet:
emit downloadFromUrlFinished(result.url);
@ -2028,7 +2028,7 @@ bool Session::addTorrent(const QString &source, const AddTorrentParams &params)
return addTorrent(magnetUri, params);
TorrentFileGuard guard {source};
if (addTorrent(TorrentInfo::loadFromFile(source), params))
if (addTorrent(TorrentInfo::loadFromFile(source).value_or(TorrentInfo()), params))
{
guard.markAsAddedToSession();
return true;
@ -3938,8 +3938,7 @@ void Session::handleTorrentFinished(TorrentImpl *const torrent)
qDebug("Found possible recursive torrent download.");
const QString torrentFullpath = torrent->savePath(true) + '/' + torrentRelpath;
qDebug("Full subtorrent path is %s", qUtf8Printable(torrentFullpath));
TorrentInfo torrentInfo = TorrentInfo::loadFromFile(torrentFullpath);
if (torrentInfo.isValid())
if (TorrentInfo::loadFromFile(torrentFullpath))
{
qDebug("emitting recursiveTorrentDownloadPossible()");
emit recursiveTorrentDownloadPossible(torrent);
@ -4167,7 +4166,7 @@ void Session::recursiveTorrentDownload(const TorrentID &id)
AddTorrentParams params;
// Passing the save path along to the sub torrent file
params.savePath = torrent->savePath();
addTorrent(TorrentInfo::loadFromFile(torrentFullpath), params);
addTorrent(TorrentInfo::loadFromFile(torrentFullpath).value_or(TorrentInfo()), params);
}
}
}

View File

@ -106,7 +106,7 @@ TorrentInfo &TorrentInfo::operator=(const TorrentInfo &other)
return *this;
}
TorrentInfo TorrentInfo::load(const QByteArray &data, QString *error) noexcept
nonstd::expected<TorrentInfo, QString> TorrentInfo::load(const QByteArray &data) noexcept
{
// 2-step construction to overcome default limits of `depth_limit` & `token_limit` which are
// used in `torrent_info()` constructor
@ -117,42 +117,23 @@ TorrentInfo TorrentInfo::load(const QByteArray &data, QString *error) noexcept
const lt::bdecode_node node = lt::bdecode(data, ec
, nullptr, depthLimit, tokenLimit);
if (ec)
{
if (error)
*error = QString::fromStdString(ec.message());
return TorrentInfo();
}
return nonstd::make_unexpected(QString::fromStdString(ec.message()));
TorrentInfo info {std::shared_ptr<lt::torrent_info>(new lt::torrent_info(node, ec))};
if (ec)
{
if (error)
*error = QString::fromStdString(ec.message());
return TorrentInfo();
}
return nonstd::make_unexpected(QString::fromStdString(ec.message()));
return info;
}
TorrentInfo TorrentInfo::loadFromFile(const QString &path, QString *error) noexcept
nonstd::expected<TorrentInfo, QString> TorrentInfo::loadFromFile(const QString &path) noexcept
{
if (error)
error->clear();
QFile file {path};
if (!file.open(QIODevice::ReadOnly))
{
if (error)
*error = file.errorString();
return TorrentInfo();
}
return nonstd::make_unexpected(file.errorString());
if (file.size() > MAX_TORRENT_SIZE)
{
if (error)
*error = tr("File size exceeds max limit %1").arg(Utils::Misc::friendlyUnit(MAX_TORRENT_SIZE));
return TorrentInfo();
}
return nonstd::make_unexpected(tr("File size exceeds max limit %1").arg(Utils::Misc::friendlyUnit(MAX_TORRENT_SIZE)));
QByteArray data;
try
@ -161,20 +142,15 @@ TorrentInfo TorrentInfo::loadFromFile(const QString &path, QString *error) noexc
}
catch (const std::bad_alloc &e)
{
if (error)
*error = tr("Torrent file read error: %1").arg(e.what());
return TorrentInfo();
return nonstd::make_unexpected(tr("Torrent file read error: %1").arg(e.what()));
}
if (data.size() != file.size())
{
if (error)
*error = tr("Torrent file read error: size mismatch");
return TorrentInfo();
}
return nonstd::make_unexpected(tr("Torrent file read error: size mismatch"));
file.close();
return load(data, error);
return load(data);
}
void TorrentInfo::saveToFile(const QString &path) const

View File

@ -33,6 +33,7 @@
#include <QCoreApplication>
#include <QtContainerFwd>
#include "base/3rdparty/expected.hpp"
#include "base/indexrange.h"
#include "abstractfilestorage.h"
#include "torrentcontentlayout.h"
@ -55,8 +56,8 @@ namespace BitTorrent
explicit TorrentInfo(std::shared_ptr<const lt::torrent_info> nativeInfo = {});
TorrentInfo(const TorrentInfo &other);
static TorrentInfo load(const QByteArray &data, QString *error = nullptr) noexcept;
static TorrentInfo loadFromFile(const QString &path, QString *error = nullptr) noexcept;
static nonstd::expected<TorrentInfo, QString> load(const QByteArray &data) noexcept;
static nonstd::expected<TorrentInfo, QString> loadFromFile(const QString &path) noexcept;
void saveToFile(const QString &path) const;
TorrentInfo &operator=(const TorrentInfo &other);

View File

@ -142,67 +142,59 @@ Session *Session::instance()
return m_instance;
}
bool Session::addFolder(const QString &path, QString *error)
nonstd::expected<void, QString> Session::addFolder(const QString &path)
{
Folder *destFolder = prepareItemDest(path, error);
if (!destFolder)
return false;
const nonstd::expected<Folder *, QString> result = prepareItemDest(path);
if (!result)
return result.get_unexpected();
const auto destFolder = result.value();
addItem(new Folder(path), destFolder);
store();
return true;
return {};
}
bool Session::addFeed(const QString &url, const QString &path, QString *error)
nonstd::expected<void, QString> Session::addFeed(const QString &url, const QString &path)
{
if (m_feedsByURL.contains(url))
{
if (error)
*error = tr("RSS feed with given URL already exists: %1.").arg(url);
return false;
}
return nonstd::make_unexpected(tr("RSS feed with given URL already exists: %1.").arg(url));
Folder *destFolder = prepareItemDest(path, error);
if (!destFolder)
return false;
const nonstd::expected<Folder *, QString> result = prepareItemDest(path);
if (!result)
return result.get_unexpected();
const auto destFolder = result.value();
addItem(new Feed(generateUID(), url, path, this), destFolder);
store();
if (m_processingEnabled)
feedByURL(url)->refresh();
return true;
return {};
}
bool Session::moveItem(const QString &itemPath, const QString &destPath, QString *error)
nonstd::expected<void, QString> Session::moveItem(const QString &itemPath, const QString &destPath)
{
if (itemPath.isEmpty())
{
if (error)
*error = tr("Cannot move root folder.");
return false;
}
return nonstd::make_unexpected(tr("Cannot move root folder."));
auto item = m_itemsByPath.value(itemPath);
if (!item)
{
if (error)
*error = tr("Item doesn't exist: %1.").arg(itemPath);
return false;
}
return nonstd::make_unexpected(tr("Item doesn't exist: %1.").arg(itemPath));
return moveItem(item, destPath, error);
return moveItem(item, destPath);
}
bool Session::moveItem(Item *item, const QString &destPath, QString *error)
nonstd::expected<void, QString> Session::moveItem(Item *item, const QString &destPath)
{
Q_ASSERT(item);
Q_ASSERT(item != rootFolder());
Folder *destFolder = prepareItemDest(destPath, error);
if (!destFolder)
return false;
const nonstd::expected<Folder *, QString> result = prepareItemDest(destPath);
if (!result)
return result.get_unexpected();
auto srcFolder = static_cast<Folder *>(m_itemsByPath.value(Item::parentPath(item->path())));
const auto destFolder = result.value();
if (srcFolder != destFolder)
{
srcFolder->removeItem(item);
@ -211,25 +203,17 @@ bool Session::moveItem(Item *item, const QString &destPath, QString *error)
m_itemsByPath.insert(destPath, m_itemsByPath.take(item->path()));
item->setPath(destPath);
store();
return true;
return {};
}
bool Session::removeItem(const QString &itemPath, QString *error)
nonstd::expected<void, QString> Session::removeItem(const QString &itemPath)
{
if (itemPath.isEmpty())
{
if (error)
*error = tr("Cannot delete root folder.");
return false;
}
return nonstd::make_unexpected(tr("Cannot delete root folder."));
auto item = m_itemsByPath.value(itemPath);
auto *item = m_itemsByPath.value(itemPath);
if (!item)
{
if (error)
*error = tr("Item doesn't exist: %1.").arg(itemPath);
return false;
}
return nonstd::make_unexpected(tr("Item doesn't exist: %1.").arg(itemPath));
emit itemAboutToBeRemoved(item);
item->cleanup();
@ -238,7 +222,7 @@ bool Session::removeItem(const QString &itemPath, QString *error)
folder->removeItem(item);
delete item;
store();
return true;
return {};
}
QList<Item *> Session::items() const
@ -395,30 +379,18 @@ void Session::store()
m_confFileStorage->store(FeedsFileName, QJsonDocument(rootFolder()->toJsonValue().toObject()).toJson());
}
Folder *Session::prepareItemDest(const QString &path, QString *error)
nonstd::expected<Folder *, QString> Session::prepareItemDest(const QString &path)
{
if (!Item::isValidPath(path))
{
if (error)
*error = tr("Incorrect RSS Item path: %1.").arg(path);
return nullptr;
}
return nonstd::make_unexpected(tr("Incorrect RSS Item path: %1.").arg(path));
if (m_itemsByPath.contains(path))
{
if (error)
*error = tr("RSS item with given path already exists: %1.").arg(path);
return nullptr;
}
return nonstd::make_unexpected(tr("RSS item with given path already exists: %1.").arg(path));
const QString destFolderPath = Item::parentPath(path);
auto destFolder = qobject_cast<Folder *>(m_itemsByPath.value(destFolderPath));
const auto destFolder = qobject_cast<Folder *>(m_itemsByPath.value(destFolderPath));
if (!destFolder)
{
if (error)
*error = tr("Parent folder doesn't exist: %1.").arg(destFolderPath);
return nullptr;
}
return nonstd::make_unexpected(tr("Parent folder doesn't exist: %1.").arg(destFolderPath));
return destFolder;
}

View File

@ -73,6 +73,8 @@
#include <QPointer>
#include <QTimer>
#include "base/3rdparty/expected.hpp"
class QThread;
class Application;
@ -110,12 +112,11 @@ namespace RSS
int refreshInterval() const;
void setRefreshInterval(int refreshInterval);
bool addFolder(const QString &path, QString *error = nullptr);
bool addFeed(const QString &url, const QString &path, QString *error = nullptr);
bool moveItem(const QString &itemPath, const QString &destPath
, QString *error = nullptr);
bool moveItem(Item *item, const QString &destPath, QString *error = nullptr);
bool removeItem(const QString &itemPath, QString *error = nullptr);
nonstd::expected<void, QString> addFolder(const QString &path);
nonstd::expected<void, QString> addFeed(const QString &url, const QString &path);
nonstd::expected<void, QString> moveItem(const QString &itemPath, const QString &destPath);
nonstd::expected<void, QString> moveItem(Item *item, const QString &destPath);
nonstd::expected<void, QString> removeItem(const QString &itemPath);
QList<Item *> items() const;
Item *itemByPath(const QString &path) const;
@ -146,7 +147,7 @@ namespace RSS
void loadFolder(const QJsonObject &jsonObj, Folder *folder);
void loadLegacy();
void store();
Folder *prepareItemDest(const QString &path, QString *error);
nonstd::expected<Folder *, QString> prepareItemDest(const QString &path);
Folder *addSubfolder(const QString &name, Folder *parentFolder);
Feed *addFeedToFolder(const QUuid &uid, const QString &url, const QString &name, Folder *parentFolder);
void addItem(Item *item, Folder *destFolder);

View File

@ -526,10 +526,10 @@ void TorrentFilesWatcher::Worker::processFolder(const QString &path, const QStri
}
else
{
const auto torrentInfo = BitTorrent::TorrentInfo::loadFromFile(filePath);
if (torrentInfo.isValid())
const nonstd::expected<BitTorrent::TorrentInfo, QString> result = BitTorrent::TorrentInfo::loadFromFile(filePath);
if (result)
{
emit torrentFound(torrentInfo, addTorrentParams);
emit torrentFound(result.value(), addTorrentParams);
Utils::Fs::forceRemove(filePath);
}
else
@ -567,8 +567,8 @@ void TorrentFilesWatcher::Worker::processFailedTorrents()
if (!QFile::exists(torrentPath))
return true;
const auto torrentInfo = BitTorrent::TorrentInfo::loadFromFile(torrentPath);
if (torrentInfo.isValid())
const nonstd::expected<BitTorrent::TorrentInfo, QString> result = BitTorrent::TorrentInfo::loadFromFile(torrentPath);
if (result)
{
BitTorrent::AddTorrentParams addTorrentParams = options.addTorrentParams;
const QString exactDirPath = QFileInfo(torrentPath).canonicalPath();
@ -578,7 +578,7 @@ void TorrentFilesWatcher::Worker::processFailedTorrents()
addTorrentParams.savePath = QDir(addTorrentParams.savePath).filePath(subdirPath);
}
emit torrentFound(torrentInfo, addTorrentParams);
emit torrentFound(result.value(), addTorrentParams);
Utils::Fs::forceRemove(torrentPath);
return true;

View File

@ -252,13 +252,13 @@ bool AddNewTorrentDialog::loadTorrentFile(const QString &torrentPath)
? QUrl::fromEncoded(torrentPath.toLocal8Bit()).toLocalFile()
: torrentPath;
QString error;
m_torrentInfo = BitTorrent::TorrentInfo::loadFromFile(decodedPath, &error);
if (!m_torrentInfo.isValid())
const nonstd::expected<BitTorrent::TorrentInfo, QString> result = BitTorrent::TorrentInfo::loadFromFile(decodedPath);
m_torrentInfo = result.value_or(BitTorrent::TorrentInfo());
if (!result)
{
RaisedMessageBox::critical(this, tr("Invalid torrent")
, tr("Failed to load the torrent: %1.\nError: %2", "Don't remove the '\n' characters. They insert a newline.")
.arg(Utils::Fs::toNativePath(decodedPath), error));
.arg(Utils::Fs::toNativePath(decodedPath), result.error()));
return false;
}
@ -746,36 +746,38 @@ void AddNewTorrentDialog::setupTreeview()
updateDiskSpaceLabel();
}
void AddNewTorrentDialog::handleDownloadFinished(const Net::DownloadResult &result)
void AddNewTorrentDialog::handleDownloadFinished(const Net::DownloadResult &downloadResult)
{
QString error;
switch (result.status)
switch (downloadResult.status)
{
case Net::DownloadStatus::Success:
m_torrentInfo = BitTorrent::TorrentInfo::load(result.data, &error);
if (!m_torrentInfo.isValid())
{
RaisedMessageBox::critical(this, tr("Invalid torrent"), tr("Failed to load from URL: %1.\nError: %2")
.arg(result.url, error));
return;
const nonstd::expected<BitTorrent::TorrentInfo, QString> result = BitTorrent::TorrentInfo::load(downloadResult.data);
m_torrentInfo = result.value_or(BitTorrent::TorrentInfo());
if (!result)
{
RaisedMessageBox::critical(this, tr("Invalid torrent"), tr("Failed to load from URL: %1.\nError: %2")
.arg(downloadResult.url, result.error()));
return;
}
m_torrentGuard = std::make_unique<TorrentFileGuard>();
if (loadTorrentImpl())
open();
else
deleteLater();
}
m_torrentGuard = std::make_unique<TorrentFileGuard>();
if (loadTorrentImpl())
open();
else
deleteLater();
break;
case Net::DownloadStatus::RedirectedToMagnet:
if (loadMagnet(BitTorrent::MagnetUri(result.magnet)))
if (loadMagnet(BitTorrent::MagnetUri(downloadResult.magnet)))
open();
else
deleteLater();
break;
default:
RaisedMessageBox::critical(this, tr("Download Error"),
tr("Cannot download '%1': %2").arg(result.url, result.errorString));
tr("Cannot download '%1': %2").arg(downloadResult.url, downloadResult.errorString));
deleteLater();
}
}

View File

@ -82,7 +82,7 @@ private slots:
void updateDiskSpaceLabel();
void onSavePathChanged(const QString &newPath);
void updateMetadata(const BitTorrent::TorrentInfo &metadata);
void handleDownloadFinished(const Net::DownloadResult &result);
void handleDownloadFinished(const Net::DownloadResult &downloadResult);
void TMMChanged(int index);
void categoryChanged(int index);
void doNotDeleteTorrentClicked(bool checked);

View File

@ -246,10 +246,10 @@ void RSSWidget::askNewFolder()
? RSS::Session::instance()->rootFolder()
: qobject_cast<RSS::Folder *>(m_feedListWidget->getRSSItem(destItem)));
QString error;
const QString newFolderPath = RSS::Item::joinPath(rssDestFolder->path(), newName);
if (!RSS::Session::instance()->addFolder(newFolderPath, &error))
QMessageBox::warning(this, "qBittorrent", error, QMessageBox::Ok);
const nonstd::expected<void, QString> result = RSS::Session::instance()->addFolder(newFolderPath);
if (!result)
QMessageBox::warning(this, "qBittorrent", result.error(), QMessageBox::Ok);
// Expand destination folder to display new feed
if (destItem && (destItem != m_feedListWidget->stickyUnreadItem()))
@ -287,11 +287,11 @@ void RSSWidget::on_newFeedButton_clicked()
? RSS::Session::instance()->rootFolder()
: qobject_cast<RSS::Folder *>(m_feedListWidget->getRSSItem(destItem)));
QString error;
// NOTE: We still add feed using legacy way (with URL as feed name)
const QString newFeedPath = RSS::Item::joinPath(rssDestFolder->path(), newURL);
if (!RSS::Session::instance()->addFeed(newURL, newFeedPath, &error))
QMessageBox::warning(this, "qBittorrent", error, QMessageBox::Ok);
const nonstd::expected<void, QString> result = RSS::Session::instance()->addFeed(newURL, newFeedPath);
if (!result)
QMessageBox::warning(this, "qBittorrent", result.error(), QMessageBox::Ok);
// Expand destination folder to display new feed
if (destItem && (destItem != m_feedListWidget->stickyUnreadItem()))
@ -411,10 +411,10 @@ void RSSWidget::renameSelectedRSSItem()
// Check if name is already taken
if (!ok) return;
QString error;
if (!RSS::Session::instance()->moveItem(rssItem, RSS::Item::joinPath(parentPath, newName), &error))
const nonstd::expected<void, QString> result = RSS::Session::instance()->moveItem(rssItem, RSS::Item::joinPath(parentPath, newName));
if (!result)
{
QMessageBox::warning(nullptr, tr("Rename failed"), error);
QMessageBox::warning(nullptr, tr("Rename failed"), result.error());
ok = false;
}
} while (!ok);

View File

@ -234,8 +234,8 @@ void TorrentCreatorDialog::handleCreationSuccess(const QString &path, const QStr
if (m_ui->checkStartSeeding->isChecked())
{
// Create save path temp data
const BitTorrent::TorrentInfo info = BitTorrent::TorrentInfo::loadFromFile(Utils::Fs::toNativePath(path));
if (!info.isValid())
const nonstd::expected<BitTorrent::TorrentInfo, QString> result = BitTorrent::TorrentInfo::loadFromFile(Utils::Fs::toNativePath(path));
if (!result)
{
QMessageBox::critical(this, tr("Torrent creation failed"), tr("Reason: Created torrent is invalid. It won't be added to download list."));
return;
@ -251,7 +251,7 @@ void TorrentCreatorDialog::handleCreationSuccess(const QString &path, const QStr
}
params.useAutoTMM = false; // otherwise if it is on by default, it will overwrite `savePath` to the default save path
BitTorrent::Session::instance()->addTorrent(info, params);
BitTorrent::Session::instance()->addTorrent(result.value(), params);
}
QMessageBox::information(this, tr("Torrent creator")
, QString::fromLatin1("%1\n%2").arg(tr("Torrent created:"), Utils::Fs::toNativePath(path)));

View File

@ -50,9 +50,9 @@ void RSSController::addFolderAction()
requireParams({"path"});
const QString path = params()["path"].trimmed();
QString error;
if (!RSS::Session::instance()->addFolder(path, &error))
throw APIError(APIErrorType::Conflict, error);
const nonstd::expected<void, QString> result = RSS::Session::instance()->addFolder(path);
if (!result)
throw APIError(APIErrorType::Conflict, result.error());
}
void RSSController::addFeedAction()
@ -61,9 +61,9 @@ void RSSController::addFeedAction()
const QString url = params()["url"].trimmed();
const QString path = params()["path"].trimmed();
QString error;
if (!RSS::Session::instance()->addFeed(url, (path.isEmpty() ? url : path), &error))
throw APIError(APIErrorType::Conflict, error);
const nonstd::expected<void, QString> result = RSS::Session::instance()->addFeed(url, (path.isEmpty() ? url : path));
if (!result)
throw APIError(APIErrorType::Conflict, result.error());
}
void RSSController::removeItemAction()
@ -71,9 +71,9 @@ void RSSController::removeItemAction()
requireParams({"path"});
const QString path = params()["path"].trimmed();
QString error;
if (!RSS::Session::instance()->removeItem(path, &error))
throw APIError(APIErrorType::Conflict, error);
const nonstd::expected<void, QString> result = RSS::Session::instance()->removeItem(path);
if (!result)
throw APIError(APIErrorType::Conflict, result.error());
}
void RSSController::moveItemAction()
@ -82,9 +82,9 @@ void RSSController::moveItemAction()
const QString itemPath = params()["itemPath"].trimmed();
const QString destPath = params()["destPath"].trimmed();
QString error;
if (!RSS::Session::instance()->moveItem(itemPath, destPath, &error))
throw APIError(APIErrorType::Conflict, error);
const nonstd::expected<void, QString> result = RSS::Session::instance()->moveItem(itemPath, destPath);
if (!result)
throw APIError(APIErrorType::Conflict, result.error());
}
void RSSController::itemsAction()

View File

@ -701,14 +701,14 @@ void TorrentsController::addAction()
for (auto it = data().constBegin(); it != data().constEnd(); ++it)
{
const BitTorrent::TorrentInfo torrentInfo = BitTorrent::TorrentInfo::load(it.value());
if (!torrentInfo.isValid())
const nonstd::expected<BitTorrent::TorrentInfo, QString> result = BitTorrent::TorrentInfo::load(it.value());
if (!result)
{
throw APIError(APIErrorType::BadData
, tr("Error: '%1' is not a valid torrent file.").arg(it.key()));
}
partialSuccess |= BitTorrent::Session::instance()->addTorrent(torrentInfo, addTorrentParams);
partialSuccess |= BitTorrent::Session::instance()->addTorrent(result.value(), addTorrentParams);
}
if (partialSuccess)