diff --git a/CHANGELOG.md b/CHANGELOG.md index 420c45dd1..2e85a8777 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,7 @@ - Dev: Refactor Args to be less of a singleton. (#5041) - Dev: Channels without any animated elements on screen will skip updates from the GIF timer. (#5042, #5043, #5045) - Dev: Autogenerate docs/plugin-meta.lua. (#5055) +- Dev: Refactor `NetworkPrivate`. (#5063) - Dev: Removed duplicate scale in settings dialog. (#5069) - Dev: Fix `NotebookTab` emitting updates for every message. (#5068) - Dev: Added benchmark for parsing and building recent messages. (#5071) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 2b83ee2a8..e649d312a 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -50,6 +50,9 @@ set(SOURCE_FILES common/enums/MessageOverflow.hpp + common/network/NetworkTask.hpp + common/network/NetworkTask.cpp + controllers/accounts/Account.cpp controllers/accounts/Account.hpp controllers/accounts/AccountController.cpp @@ -457,6 +460,7 @@ set(SOURCE_FILES singletons/helper/LoggingChannel.cpp singletons/helper/LoggingChannel.hpp + util/AbandonObject.hpp util/AttachToConsole.cpp util/AttachToConsole.hpp util/CancellationToken.hpp @@ -931,6 +935,7 @@ target_compile_definitions(${LIBRARY_PROJECT} PUBLIC IRC_STATIC IRC_NAMESPACE=Communi ) + if (USE_SYSTEM_QTKEYCHAIN) target_compile_definitions(${LIBRARY_PROJECT} PUBLIC CMAKE_BUILD diff --git a/src/common/NetworkCommon.hpp b/src/common/NetworkCommon.hpp index 41cf2e85e..a5a44430e 100644 --- a/src/common/NetworkCommon.hpp +++ b/src/common/NetworkCommon.hpp @@ -13,7 +13,6 @@ class NetworkResult; using NetworkSuccessCallback = std::function; using NetworkErrorCallback = std::function; -using NetworkReplyCreatedCallback = std::function; using NetworkFinallyCallback = std::function; enum class NetworkRequestType { @@ -23,13 +22,6 @@ enum class NetworkRequestType { Delete, Patch, }; -const static std::vector networkRequestTypes{ - "GET", // - "POST", // - "PUT", // - "DELETE", // - "PATCH", // -}; // parseHeaderList takes a list of headers in string form, // where each header pair is separated by semicolons (;) and the header name and value is divided by a colon (:) diff --git a/src/common/NetworkPrivate.cpp b/src/common/NetworkPrivate.cpp index 04f0c4b0b..02b0ecede 100644 --- a/src/common/NetworkPrivate.cpp +++ b/src/common/NetworkPrivate.cpp @@ -1,46 +1,104 @@ #include "common/NetworkPrivate.hpp" +#include "common/network/NetworkTask.hpp" #include "common/NetworkManager.hpp" #include "common/NetworkResult.hpp" #include "common/QLogging.hpp" -#include "debug/AssertInGuiThread.hpp" #include "singletons/Paths.hpp" +#include "util/AbandonObject.hpp" #include "util/DebugCount.hpp" #include "util/PostToThread.hpp" +#include #include +#include #include #include #include +#ifdef NDEBUG +constexpr qsizetype SLOW_HTTP_THRESHOLD = 30; +#else +constexpr qsizetype SLOW_HTTP_THRESHOLD = 90; +#endif + +using namespace chatterino::network::detail; + +namespace { + +using namespace chatterino; + +void runCallback(bool concurrent, auto &&fn) +{ + if (concurrent) + { + std::ignore = QtConcurrent::run(std::forward(fn)); + } + else + { + runInGuiThread(std::forward(fn)); + } +} + +void loadUncached(std::shared_ptr &&data) +{ + DebugCount::increase("http request started"); + + NetworkRequester requester; + auto *worker = new NetworkTask(std::move(data)); + + worker->moveToThread(&NetworkManager::workerThread); + + QObject::connect(&requester, &NetworkRequester::requestUrl, worker, + &NetworkTask::run); + + emit requester.requestUrl(); +} + +void loadCached(std::shared_ptr &&data) +{ + QFile cachedFile(getPaths()->cacheDirectory() + "/" + data->getHash()); + + if (!cachedFile.exists() || !cachedFile.open(QIODevice::ReadOnly)) + { + loadUncached(std::move(data)); + return; + } + + // XXX: check if bytes is empty? + QByteArray bytes = cachedFile.readAll(); + + qCDebug(chatterinoHTTP).noquote() << data->typeString() << "[CACHED] 200" + << data->request.url().toString(); + + data->emitSuccess( + {NetworkResult::NetworkError::NoError, QVariant(200), bytes}); + data->emitFinally(); +} + +} // namespace + namespace chatterino { NetworkData::NetworkData() - : lifetimeManager_(new QObject) { DebugCount::increase("NetworkData"); } NetworkData::~NetworkData() { - this->lifetimeManager_->deleteLater(); - DebugCount::decrease("NetworkData"); } QString NetworkData::getHash() { - static std::mutex mu; - - std::lock_guard lock(mu); - if (this->hash_.isEmpty()) { QByteArray bytes; - bytes.append(this->request_.url().toString().toUtf8()); + bytes.append(this->request.url().toString().toUtf8()); - for (const auto &header : this->request_.rawHeaderList()) + for (const auto &header : this->request.rawHeaderList()) { bytes.append(header); } @@ -54,353 +112,85 @@ QString NetworkData::getHash() return this->hash_; } -void writeToCache(const std::shared_ptr &data, - const QByteArray &bytes) +void NetworkData::emitSuccess(NetworkResult &&result) { - if (data->cache_) + if (!this->onSuccess) { - QtConcurrent::run([data, bytes] { - QFile cachedFile(getPaths()->cacheDirectory() + "/" + - data->getHash()); - - if (cachedFile.open(QIODevice::WriteOnly)) - { - cachedFile.write(bytes); - } - }); - } -} - -void loadUncached(std::shared_ptr &&data) -{ - DebugCount::increase("http request started"); - - NetworkRequester requester; - auto *worker = new NetworkWorker; - - worker->moveToThread(&NetworkManager::workerThread); - - auto onUrlRequested = [data, worker]() mutable { - if (data->hasTimeout_) - { - data->timer_ = new QTimer(); - data->timer_->setSingleShot(true); - data->timer_->start(data->timeoutMS_); - } - - auto *reply = [&]() -> QNetworkReply * { - switch (data->requestType_) - { - case NetworkRequestType::Get: - return NetworkManager::accessManager.get(data->request_); - - case NetworkRequestType::Put: - return NetworkManager::accessManager.put(data->request_, - data->payload_); - - case NetworkRequestType::Delete: - return NetworkManager::accessManager.deleteResource( - data->request_); - - case NetworkRequestType::Post: - if (data->multiPartPayload_) - { - assert(data->payload_.isNull()); - - return NetworkManager::accessManager.post( - data->request_, data->multiPartPayload_); - } - else - { - return NetworkManager::accessManager.post( - data->request_, data->payload_); - } - case NetworkRequestType::Patch: - if (data->multiPartPayload_) - { - assert(data->payload_.isNull()); - - return NetworkManager::accessManager.sendCustomRequest( - data->request_, "PATCH", data->multiPartPayload_); - } - else - { - return NetworkManager::accessManager.sendCustomRequest( - data->request_, "PATCH", data->payload_); - } - } - return nullptr; - }(); - - if (reply == nullptr) - { - qCDebug(chatterinoCommon) << "Unhandled request type"; - return; - } - - if (data->timer_ != nullptr && data->timer_->isActive()) - { - QObject::connect( - data->timer_, &QTimer::timeout, worker, [reply, data]() { - qCDebug(chatterinoCommon) << "Aborted!"; - reply->abort(); - qCDebug(chatterinoHTTP) - << QString("%1 [timed out] %2") - .arg(networkRequestTypes.at( - int(data->requestType_)), - data->request_.url().toString()); - - if (data->onError_) - { - postToThread([data] { - data->onError_(NetworkResult( - NetworkResult::NetworkError::TimeoutError, {}, - {})); - }); - } - - if (data->finally_) - { - postToThread([data] { - data->finally_(); - }); - } - }); - } - - if (data->onReplyCreated_) - { - data->onReplyCreated_(reply); - } - - auto handleReply = [data, reply]() mutable { - if (data->hasCaller_ && data->caller_.isNull()) - { - return; - } - - // TODO(pajlada): A reply was received, kill the timeout timer - if (reply->error() != QNetworkReply::NetworkError::NoError) - { - if (reply->error() == - QNetworkReply::NetworkError::OperationCanceledError) - { - // Operation cancelled, most likely timed out - qCDebug(chatterinoHTTP) - << QString("%1 [cancelled] %2") - .arg(networkRequestTypes.at( - int(data->requestType_)), - data->request_.url().toString()); - return; - } - - if (data->onError_) - { - auto status = reply->attribute( - QNetworkRequest::HttpStatusCodeAttribute); - if (data->requestType_ == NetworkRequestType::Get) - { - qCDebug(chatterinoHTTP) - << QString("%1 %2 %3") - .arg(networkRequestTypes.at( - int(data->requestType_)), - QString::number(status.toInt()), - data->request_.url().toString()); - } - else - { - qCDebug(chatterinoHTTP) - << QString("%1 %2 %3 %4") - .arg(networkRequestTypes.at( - int(data->requestType_)), - QString::number(status.toInt()), - data->request_.url().toString(), - QString(data->payload_)); - } - // TODO: Should this always be run on the GUI thread? - postToThread([data, status, reply] { - data->onError_(NetworkResult(reply->error(), status, - reply->readAll())); - }); - } - - if (data->finally_) - { - postToThread([data] { - data->finally_(); - }); - } - return; - } - - QByteArray bytes = reply->readAll(); - writeToCache(data, bytes); - - auto status = - reply->attribute(QNetworkRequest::HttpStatusCodeAttribute); - - NetworkResult result(reply->error(), status, bytes); - - DebugCount::increase("http request success"); - // log("starting {}", data->request_.url().toString()); - if (data->onSuccess_) - { - if (data->executeConcurrently_) - { - QtConcurrent::run([onSuccess = std::move(data->onSuccess_), - result = std::move(result)] { - onSuccess(result); - }); - } - else - { - data->onSuccess_(result); - } - } - // log("finished {}", data->request_.url().toString()); - - reply->deleteLater(); - - if (data->requestType_ == NetworkRequestType::Get) - { - qCDebug(chatterinoHTTP) - << QString("%1 %2 %3") - .arg(networkRequestTypes.at(int(data->requestType_)), - QString::number(status.toInt()), - data->request_.url().toString()); - } - else - { - qCDebug(chatterinoHTTP) - << QString("%1 %3 %2 %4") - .arg(networkRequestTypes.at(int(data->requestType_)), - data->request_.url().toString(), - QString::number(status.toInt()), - QString(data->payload_)); - } - if (data->finally_) - { - if (data->executeConcurrently_) - { - QtConcurrent::run([finally = std::move(data->finally_)] { - finally(); - }); - } - else - { - data->finally_(); - } - } - }; - - if (data->timer_ != nullptr) - { - QObject::connect(reply, &QNetworkReply::finished, data->timer_, - &QObject::deleteLater); - } - - QObject::connect( - reply, &QNetworkReply::finished, worker, - [data, handleReply, worker]() mutable { - if (data->executeConcurrently_ || isGuiThread()) - { - handleReply(); - delete worker; - } - else - { - postToThread( - [worker, cb = std::move(handleReply)]() mutable { - cb(); - delete worker; - }); - } - }); - }; - - QObject::connect(&requester, &NetworkRequester::requestUrl, worker, - onUrlRequested); - - emit requester.requestUrl(); -} - -// First tried to load cached, then uncached. -void loadCached(std::shared_ptr &&data) -{ - QFile cachedFile(getPaths()->cacheDirectory() + "/" + data->getHash()); - - if (!cachedFile.exists() || !cachedFile.open(QIODevice::ReadOnly)) - { - // File didn't exist OR File could not be opened - loadUncached(std::move(data)); return; } - // XXX: check if bytes is empty? - QByteArray bytes = cachedFile.readAll(); - NetworkResult result(NetworkResult::NetworkError::NoError, QVariant(200), - bytes); + runCallback(this->executeConcurrently, + [cb = std::move(this->onSuccess), result = std::move(result), + url = this->request.url(), hasCaller = this->hasCaller, + caller = this->caller]() { + if (hasCaller && caller.isNull()) + { + return; + } - qCDebug(chatterinoHTTP) - << QString("%1 [CACHED] 200 %2") - .arg(networkRequestTypes.at(int(data->requestType_)), - data->request_.url().toString()); - if (data->onSuccess_) + QElapsedTimer timer; + timer.start(); + cb(result); + if (timer.elapsed() > SLOW_HTTP_THRESHOLD) + { + qCWarning(chatterinoHTTP) + << "Slow HTTP success handler for" << url.toString() + << timer.elapsed() + << "ms (threshold:" << SLOW_HTTP_THRESHOLD << "ms)"; + } + }); +} + +void NetworkData::emitError(NetworkResult &&result) +{ + if (!this->onError) { - if (data->executeConcurrently_ || isGuiThread()) - { - // XXX: If outcome is Failure, we should invalidate the cache file - // somehow/somewhere - /*auto outcome =*/ - if (data->hasCaller_ && data->caller_.isNull()) - { - return; - } - data->onSuccess_(result); - } - else - { - postToThread([data, result]() { - if (data->hasCaller_ && data->caller_.isNull()) - { - return; - } - - data->onSuccess_(result); - }); - } + return; } - if (data->finally_) + runCallback(this->executeConcurrently, + [cb = std::move(this->onError), result = std::move(result), + hasCaller = this->hasCaller, caller = this->caller]() { + if (hasCaller && caller.isNull()) + { + return; + } + + cb(result); + }); +} + +void NetworkData::emitFinally() +{ + if (!this->finally) { - if (data->executeConcurrently_ || isGuiThread()) - { - if (data->hasCaller_ && data->caller_.isNull()) - { - return; - } - - data->finally_(); - } - else - { - postToThread([data]() { - if (data->hasCaller_ && data->caller_.isNull()) - { - return; - } - - data->finally_(); - }); - } + return; } + + runCallback(this->executeConcurrently, + [cb = std::move(this->finally), hasCaller = this->hasCaller, + caller = this->caller]() { + if (hasCaller && caller.isNull()) + { + return; + } + + cb(); + }); +} + +QLatin1String NetworkData::typeString() const +{ + auto view = magic_enum::enum_name(this->requestType); + return QLatin1String{view.data(), + static_cast(view.size())}; } void load(std::shared_ptr &&data) { - if (data->cache_) + if (data->cache) { - QtConcurrent::run([data = std::move(data)]() mutable { + std::ignore = QtConcurrent::run([data = std::move(data)]() mutable { loadCached(std::move(data)); }); } diff --git a/src/common/NetworkPrivate.hpp b/src/common/NetworkPrivate.hpp index d48000aeb..85fa5c149 100644 --- a/src/common/NetworkPrivate.hpp +++ b/src/common/NetworkPrivate.hpp @@ -1,5 +1,6 @@ #pragma once +#include "common/Common.hpp" #include "common/NetworkCommon.hpp" #include @@ -7,8 +8,8 @@ #include #include -#include #include +#include class QNetworkReply; @@ -24,46 +25,43 @@ signals: void requestUrl(); }; -class NetworkWorker : public QObject +class NetworkData { - Q_OBJECT - -signals: - void doneUrl(); -}; - -struct NetworkData { +public: NetworkData(); ~NetworkData(); + NetworkData(const NetworkData &) = delete; + NetworkData(NetworkData &&) = delete; + NetworkData &operator=(const NetworkData &) = delete; + NetworkData &operator=(NetworkData &&) = delete; - QNetworkRequest request_; - bool hasCaller_{}; - QPointer caller_; - bool cache_{}; - bool executeConcurrently_{}; + QNetworkRequest request; + bool hasCaller{}; + QPointer caller; + bool cache{}; + bool executeConcurrently{}; - NetworkReplyCreatedCallback onReplyCreated_; - NetworkErrorCallback onError_; - NetworkSuccessCallback onSuccess_; - NetworkFinallyCallback finally_; + NetworkSuccessCallback onSuccess; + NetworkErrorCallback onError; + NetworkFinallyCallback finally; - NetworkRequestType requestType_ = NetworkRequestType::Get; + NetworkRequestType requestType = NetworkRequestType::Get; - QByteArray payload_; - // lifetime secured by lifetimeManager_ - QHttpMultiPart *multiPartPayload_{}; + QByteArray payload; + std::unique_ptr multiPartPayload; - // Timer that tracks the timeout - // By default, there's no explicit timeout for the request - // to enable the timer, the "setTimeout" function needs to be called before - // execute is called - bool hasTimeout_{}; - int timeoutMS_{}; - QTimer *timer_ = nullptr; - QObject *lifetimeManager_; + /// By default, there's no explicit timeout for the request. + /// To set a timeout, use NetworkRequest's timeout method + std::optional timeout{}; QString getHash(); + void emitSuccess(NetworkResult &&result); + void emitError(NetworkResult &&result); + void emitFinally(); + + QLatin1String typeString() const; + private: QString hash_; }; diff --git a/src/common/NetworkRequest.cpp b/src/common/NetworkRequest.cpp index b4dc8cc46..f8c9c8f53 100644 --- a/src/common/NetworkRequest.cpp +++ b/src/common/NetworkRequest.cpp @@ -16,8 +16,8 @@ NetworkRequest::NetworkRequest(const std::string &url, NetworkRequestType requestType) : data(new NetworkData) { - this->data->request_.setUrl(QUrl(QString::fromStdString(url))); - this->data->requestType_ = requestType; + this->data->request.setUrl(QUrl(QString::fromStdString(url))); + this->data->requestType = requestType; this->initializeDefaultValues(); } @@ -25,8 +25,8 @@ NetworkRequest::NetworkRequest(const std::string &url, NetworkRequest::NetworkRequest(const QUrl &url, NetworkRequestType requestType) : data(new NetworkData) { - this->data->request_.setUrl(url); - this->data->requestType_ = requestType; + this->data->request.setUrl(url); + this->data->requestType = requestType; this->initializeDefaultValues(); } @@ -35,7 +35,7 @@ NetworkRequest::~NetworkRequest() = default; NetworkRequest NetworkRequest::type(NetworkRequestType newRequestType) && { - this->data->requestType_ = newRequestType; + this->data->requestType = newRequestType; return std::move(*this); } @@ -46,61 +46,55 @@ NetworkRequest NetworkRequest::caller(const QObject *caller) && // Caller must be in gui thread assert(caller->thread() == qApp->thread()); - this->data->caller_ = const_cast(caller); - this->data->hasCaller_ = true; + this->data->caller = const_cast(caller); + this->data->hasCaller = true; } return std::move(*this); } -NetworkRequest NetworkRequest::onReplyCreated(NetworkReplyCreatedCallback cb) && -{ - this->data->onReplyCreated_ = std::move(cb); - return std::move(*this); -} - NetworkRequest NetworkRequest::onError(NetworkErrorCallback cb) && { - this->data->onError_ = std::move(cb); + this->data->onError = std::move(cb); return std::move(*this); } NetworkRequest NetworkRequest::onSuccess(NetworkSuccessCallback cb) && { - this->data->onSuccess_ = std::move(cb); + this->data->onSuccess = std::move(cb); return std::move(*this); } NetworkRequest NetworkRequest::finally(NetworkFinallyCallback cb) && { - this->data->finally_ = std::move(cb); + this->data->finally = std::move(cb); return std::move(*this); } NetworkRequest NetworkRequest::header(const char *headerName, const char *value) && { - this->data->request_.setRawHeader(headerName, value); + this->data->request.setRawHeader(headerName, value); return std::move(*this); } NetworkRequest NetworkRequest::header(const char *headerName, const QByteArray &value) && { - this->data->request_.setRawHeader(headerName, value); + this->data->request.setRawHeader(headerName, value); return std::move(*this); } NetworkRequest NetworkRequest::header(const char *headerName, const QString &value) && { - this->data->request_.setRawHeader(headerName, value.toUtf8()); + this->data->request.setRawHeader(headerName, value.toUtf8()); return std::move(*this); } NetworkRequest NetworkRequest::header(QNetworkRequest::KnownHeaders header, const QVariant &value) && { - this->data->request_.setHeader(header, value); + this->data->request.setHeader(header, value); return std::move(*this); } @@ -109,28 +103,26 @@ NetworkRequest NetworkRequest::headerList( { for (const auto &[headerName, headerValue] : headers) { - this->data->request_.setRawHeader(headerName, headerValue); + this->data->request.setRawHeader(headerName, headerValue); } return std::move(*this); } NetworkRequest NetworkRequest::timeout(int ms) && { - this->data->hasTimeout_ = true; - this->data->timeoutMS_ = ms; + this->data->timeout = std::chrono::milliseconds(ms); return std::move(*this); } NetworkRequest NetworkRequest::concurrent() && { - this->data->executeConcurrently_ = true; + this->data->executeConcurrently = true; return std::move(*this); } NetworkRequest NetworkRequest::multiPart(QHttpMultiPart *payload) && { - payload->setParent(this->data->lifetimeManager_); - this->data->multiPartPayload_ = payload; + this->data->multiPartPayload = {payload, {}}; return std::move(*this); } @@ -138,13 +130,13 @@ NetworkRequest NetworkRequest::followRedirects(bool on) && { if (on) { - this->data->request_.setAttribute( + this->data->request.setAttribute( QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::NoLessSafeRedirectPolicy); } else { - this->data->request_.setAttribute( + this->data->request.setAttribute( QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::ManualRedirectPolicy); } @@ -154,13 +146,13 @@ NetworkRequest NetworkRequest::followRedirects(bool on) && NetworkRequest NetworkRequest::payload(const QByteArray &payload) && { - this->data->payload_ = payload; + this->data->payload = payload; return std::move(*this); } NetworkRequest NetworkRequest::cache() && { - this->data->cache_ = true; + this->data->cache = true; return std::move(*this); } @@ -169,15 +161,14 @@ void NetworkRequest::execute() this->executed_ = true; // Only allow caching for GET request - if (this->data->cache_ && - this->data->requestType_ != NetworkRequestType::Get) + if (this->data->cache && this->data->requestType != NetworkRequestType::Get) { qCDebug(chatterinoCommon) << "Can only cache GET requests!"; - this->data->cache_ = false; + this->data->cache = false; } // Can not have a caller and be concurrent at the same time. - assert(!(this->data->caller_ && this->data->executeConcurrently_)); + assert(!(this->data->caller && this->data->executeConcurrently)); load(std::move(this->data)); } @@ -189,7 +180,7 @@ void NetworkRequest::initializeDefaultValues() Version::instance().commitHash()) .toUtf8(); - this->data->request_.setRawHeader("User-Agent", userAgent); + this->data->request.setRawHeader("User-Agent", userAgent); } NetworkRequest NetworkRequest::json(const QJsonArray &root) && diff --git a/src/common/NetworkRequest.hpp b/src/common/NetworkRequest.hpp index 9d55cf1b7..edbe9881f 100644 --- a/src/common/NetworkRequest.hpp +++ b/src/common/NetworkRequest.hpp @@ -12,7 +12,7 @@ class QJsonDocument; namespace chatterino { -struct NetworkData; +class NetworkData; class NetworkRequest final { @@ -43,7 +43,6 @@ public: NetworkRequest type(NetworkRequestType newRequestType) &&; - NetworkRequest onReplyCreated(NetworkReplyCreatedCallback cb) &&; NetworkRequest onError(NetworkErrorCallback cb) &&; NetworkRequest onSuccess(NetworkSuccessCallback cb) &&; NetworkRequest finally(NetworkFinallyCallback cb) &&; diff --git a/src/common/network/NetworkTask.cpp b/src/common/network/NetworkTask.cpp new file mode 100644 index 000000000..f990dd5de --- /dev/null +++ b/src/common/network/NetworkTask.cpp @@ -0,0 +1,189 @@ +#include "common/network/NetworkTask.hpp" + +#include "common/NetworkManager.hpp" +#include "common/NetworkPrivate.hpp" +#include "common/NetworkResult.hpp" +#include "common/QLogging.hpp" +#include "singletons/Paths.hpp" +#include "util/AbandonObject.hpp" +#include "util/DebugCount.hpp" + +#include +#include +#include + +namespace chatterino::network::detail { + +NetworkTask::NetworkTask(std::shared_ptr &&data) + : data_(std::move(data)) +{ +} + +NetworkTask::~NetworkTask() +{ + if (this->reply_) + { + this->reply_->deleteLater(); + } +} + +void NetworkTask::run() +{ + const auto &timeout = this->data_->timeout; + if (timeout.has_value()) + { + this->timer_ = new QTimer(this); + this->timer_->setSingleShot(true); + this->timer_->start(timeout.value()); + QObject::connect(this->timer_, &QTimer::timeout, this, + &NetworkTask::timeout); + } + + this->reply_ = this->createReply(); + if (!this->reply_) + { + this->deleteLater(); + return; + } + QObject::connect(this->reply_, &QNetworkReply::finished, this, + &NetworkTask::finished); +} + +QNetworkReply *NetworkTask::createReply() +{ + const auto &data = this->data_; + const auto &request = this->data_->request; + auto &accessManager = NetworkManager::accessManager; + switch (this->data_->requestType) + { + case NetworkRequestType::Get: + return accessManager.get(request); + + case NetworkRequestType::Put: + return accessManager.put(request, data->payload); + + case NetworkRequestType::Delete: + return accessManager.deleteResource(data->request); + + case NetworkRequestType::Post: + if (data->multiPartPayload) + { + assert(data->payload.isNull()); + + return accessManager.post(request, + data->multiPartPayload.get()); + } + else + { + return accessManager.post(request, data->payload); + } + case NetworkRequestType::Patch: + if (data->multiPartPayload) + { + assert(data->payload.isNull()); + + return accessManager.sendCustomRequest( + request, "PATCH", data->multiPartPayload.get()); + } + else + { + return NetworkManager::accessManager.sendCustomRequest( + request, "PATCH", data->payload); + } + } + return nullptr; +} + +void NetworkTask::logReply() +{ + auto status = + this->reply_->attribute(QNetworkRequest::HttpStatusCodeAttribute) + .toInt(); + if (this->data_->requestType == NetworkRequestType::Get) + { + qCDebug(chatterinoHTTP).noquote() + << this->data_->typeString() << status + << this->data_->request.url().toString(); + } + else + { + qCDebug(chatterinoHTTP).noquote() + << this->data_->typeString() + << this->data_->request.url().toString() << status + << QString(this->data_->payload); + } +} + +void NetworkTask::writeToCache(const QByteArray &bytes) const +{ + std::ignore = QtConcurrent::run([data = this->data_, bytes] { + QFile cachedFile(getPaths()->cacheDirectory() + "/" + data->getHash()); + + if (cachedFile.open(QIODevice::WriteOnly)) + { + cachedFile.write(bytes); + } + }); +} + +void NetworkTask::timeout() +{ + AbandonObject guard(this); + + // prevent abort() from calling finished() + QObject::disconnect(this->reply_, &QNetworkReply::finished, this, + &NetworkTask::finished); + this->reply_->abort(); + + qCDebug(chatterinoHTTP).noquote() + << this->data_->typeString() << "[timed out]" + << this->data_->request.url().toString(); + + this->data_->emitError({NetworkResult::NetworkError::TimeoutError, {}, {}}); + this->data_->emitFinally(); +} + +void NetworkTask::finished() +{ + AbandonObject guard(this); + + if (this->timer_) + { + this->timer_->stop(); + } + + auto *reply = this->reply_; + auto status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute); + + if (reply->error() == QNetworkReply::OperationCanceledError) + { + // Operation cancelled, most likely timed out + qCDebug(chatterinoHTTP).noquote() + << this->data_->typeString() << "[cancelled]" + << this->data_->request.url().toString(); + return; + } + + if (reply->error() != QNetworkReply::NoError) + { + this->logReply(); + this->data_->emitError({reply->error(), status, reply->readAll()}); + this->data_->emitFinally(); + + return; + } + + QByteArray bytes = reply->readAll(); + + if (this->data_->cache) + { + this->writeToCache(bytes); + } + + DebugCount::increase("http request success"); + this->logReply(); + this->data_->emitSuccess({reply->error(), status, bytes}); + this->data_->emitFinally(); +} + +} // namespace chatterino::network::detail diff --git a/src/common/network/NetworkTask.hpp b/src/common/network/NetworkTask.hpp new file mode 100644 index 000000000..d88d0a113 --- /dev/null +++ b/src/common/network/NetworkTask.hpp @@ -0,0 +1,51 @@ +#pragma once + +#include +#include + +#include + +class QNetworkReply; + +namespace chatterino { + +class NetworkData; + +} // namespace chatterino + +namespace chatterino::network::detail { + +class NetworkTask : public QObject +{ + Q_OBJECT + +public: + NetworkTask(std::shared_ptr &&data); + ~NetworkTask() override; + + NetworkTask(const NetworkTask &) = delete; + NetworkTask(NetworkTask &&) = delete; + NetworkTask &operator=(const NetworkTask &) = delete; + NetworkTask &operator=(NetworkTask &&) = delete; + + // NOLINTNEXTLINE(readability-redundant-access-specifiers) +public slots: + void run(); + +private: + QNetworkReply *createReply(); + + void logReply(); + void writeToCache(const QByteArray &bytes) const; + + std::shared_ptr data_; + QNetworkReply *reply_{}; // parent: default (accessManager) + QTimer *timer_{}; // parent: this + + // NOLINTNEXTLINE(readability-redundant-access-specifiers) +private slots: + void timeout(); + void finished(); +}; + +} // namespace chatterino::network::detail diff --git a/src/singletons/ImageUploader.cpp b/src/singletons/ImageUploader.cpp index d08067e26..7e63c3528 100644 --- a/src/singletons/ImageUploader.cpp +++ b/src/singletons/ImageUploader.cpp @@ -127,9 +127,6 @@ void ImageUploader::sendImageUploadRequest(RawImageData imageData, ChannelPtr channel, QPointer textEdit) { - const static char *const boundary = "thisistheboudaryasd"; - const static QString contentType = - QString("multipart/form-data; boundary=%1").arg(boundary); QUrl url(getSettings()->imageUploaderUrl.getValue().isEmpty() ? getSettings()->imageUploaderUrl.getDefaultValue() : getSettings()->imageUploaderUrl); @@ -152,11 +149,9 @@ void ImageUploader::sendImageUploadRequest(RawImageData imageData, QString("form-data; name=\"%1\"; filename=\"control_v.%2\"") .arg(formField) .arg(imageData.format)); - payload->setBoundary(boundary); payload->append(part); NetworkRequest(url, NetworkRequestType::Post) - .header("Content-Type", contentType) .headerList(extraHeaders) .multiPart(payload) .onSuccess( diff --git a/src/util/AbandonObject.hpp b/src/util/AbandonObject.hpp new file mode 100644 index 000000000..b0588449a --- /dev/null +++ b/src/util/AbandonObject.hpp @@ -0,0 +1,33 @@ +#pragma once + +#include + +namespace chatterino { + +/// Guard to call `deleteLater` on a QObject when destroyed. +class AbandonObject +{ +public: + AbandonObject(QObject *obj) + : obj_(obj) + { + } + + ~AbandonObject() + { + if (this->obj_) + { + this->obj_->deleteLater(); + } + } + + AbandonObject(const AbandonObject &) = delete; + AbandonObject(AbandonObject &&) = delete; + AbandonObject &operator=(const AbandonObject &) = delete; + AbandonObject &operator=(AbandonObject &&) = delete; + +private: + QObject *obj_; +}; + +} // namespace chatterino