From e35527cb4a44c22ddfe7dd6f61a09e4c972ad994 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 18 Dec 2025 10:29:08 -0600 Subject: [PATCH] refactor: minor cleanup in RpcClient.cc (#7951) * refactor: RpcClient::sendNetworkRequest() takes a QByteArray body arg * refactor: RpcClient::sendLocalRequest() takes a tr_variant const& req arg * refactor: move RpcClient::getNextTag() into an anonymous namespace * refactor: merge RpcClient::exec() and RpcClient::sendRequest() * refactor: remove unnecessary heap allocation in RpcClient::networkRequestFinished() --- qt/RpcClient.cc | 115 +++++++++++++++++++++++------------------------- qt/RpcClient.h | 7 +-- 2 files changed, 57 insertions(+), 65 deletions(-) diff --git a/qt/RpcClient.cc b/qt/RpcClient.cc index 422ab5ec3..78e1f29f1 100644 --- a/qt/RpcClient.cc +++ b/qt/RpcClient.cc @@ -4,6 +4,7 @@ // License text can be found in the licenses/ folder. #include +#include #include @@ -29,9 +30,33 @@ using ::trqt::variant_helpers::variantInit; namespace { -char constexpr const* const RequestDataPropertyKey{ "requestData" }; +char constexpr const* const RequestBodyKey{ "requestBody" }; char constexpr const* const RequestFutureinterfacePropertyKey{ "requestReplyFutureInterface" }; +[[nodiscard]] int64_t nextTag() +{ + static int64_t tag = {}; + return tag++; +} + +[[nodiscard]] std::pair buildRequest(tr_quark const method, tr_variant* args) +{ + auto const tag = nextTag(); + + // TODO(ckerr): JSON-RPCize + auto req = tr_variant::Map{ 3U }; + req.try_emplace(TR_KEY_method, tr_variant::unmanaged_string(method)); + req.try_emplace(TR_KEY_tag, tag); + if (args != nullptr) + { + auto tmp = tr_variant{}; + std::swap(tmp, *args); + req.try_emplace(TR_KEY_arguments, std::move(tmp)); + } + + return { std::move(req), tag }; +} + TrVariantPtr createVariant() { return std::make_shared(); @@ -71,35 +96,31 @@ void RpcClient::start(QUrl const& url) request_.reset(); } -RpcResponseFuture RpcClient::exec(tr_quark const method_key, tr_variant* args) +RpcResponseFuture RpcClient::exec(tr_quark const method, tr_variant* args) { - auto const method = tr_quark_get_string_view(method_key); + auto const [req, tag] = buildRequest(method, args); - TrVariantPtr const json = createVariant(); - tr_variantInitDict(json.get(), 3); - dictAdd(json.get(), TR_KEY_method, method); + auto promise = QFutureInterface{}; + promise.setExpectedResultCount(1); + promise.setProgressRange(0, 1); + promise.setProgressValue(0); + promise.reportStarted(); - if (args != nullptr) // if args were passed in, use them + if (session_ != nullptr) { - auto* child = tr_variantDictFind(json.get(), TR_KEY_arguments); - - if (child == nullptr) - { - child = tr_variantDictAddDict(json.get(), TR_KEY_arguments, 0); - } - - std::swap(*child, *args); + sendLocalRequest(req, promise, tag); + } + else if (!url_.isEmpty()) + { + auto const json = tr_variant_serde::json().compact().to_string(req); + auto const body = QByteArray::fromStdString(json); + sendNetworkRequest(body, promise); } - return sendRequest(json); + return promise.future(); } -int64_t RpcClient::getNextTag() -{ - return next_tag_++; -} - -void RpcClient::sendNetworkRequest(TrVariantPtr req, QFutureInterface const& promise) +void RpcClient::sendNetworkRequest(QByteArray const& body, QFutureInterface const& promise) { if (!request_) { @@ -117,9 +138,8 @@ void RpcClient::sendNetworkRequest(TrVariantPtr req, QFutureInterfacepost(*request_, json_data); - reply->setProperty(RequestDataPropertyKey, QVariant::fromValue(req)); + QNetworkReply* reply = networkAccessManager()->post(*request_, body); + reply->setProperty(RequestBodyKey, body); reply->setProperty(RequestFutureinterfacePropertyKey, QVariant::fromValue(promise)); connect(reply, &QNetworkReply::downloadProgress, this, &RpcClient::dataReadProgress); @@ -135,21 +155,21 @@ void RpcClient::sendNetworkRequest(TrVariantPtr req, QFutureInterface const& promise, int64_t tag) +void RpcClient::sendLocalRequest(tr_variant const& req, QFutureInterface const& promise, int64_t tag) { if (verbose_) { - fmt::print("{:s}:{:d} sending req:\n{:s}\n", __FILE__, __LINE__, tr_variant_serde::json().to_string(*req)); + fmt::print("{:s}:{:d} sending req:\n{:s}\n", __FILE__, __LINE__, tr_variant_serde::json().to_string(req)); } local_requests_.try_emplace(tag, promise); tr_rpc_request_exec( session_, - *req, + req, [this](tr_session* /*sesson*/, tr_variant&& response) { if (verbose_) @@ -166,29 +186,6 @@ void RpcClient::sendLocalRequest(TrVariantPtr req, QFutureInterface }); } -RpcResponseFuture RpcClient::sendRequest(TrVariantPtr json) -{ - int64_t const tag = getNextTag(); - dictAdd(json.get(), TR_KEY_tag, tag); - - QFutureInterface promise; - promise.setExpectedResultCount(1); - promise.setProgressRange(0, 1); - promise.setProgressValue(0); - promise.reportStarted(); - - if (session_ != nullptr) - { - sendLocalRequest(json, promise, tag); - } - else if (!url_.isEmpty()) - { - sendNetworkRequest(json, promise); - } - - return promise.future(); -} - QNetworkAccessManager* RpcClient::networkAccessManager() { if (nam_ == nullptr) @@ -230,7 +227,7 @@ void RpcClient::networkRequestFinished(QNetworkReply* reply) session_id_ = QString::fromUtf8(reply->rawHeader(TR_RPC_SESSION_ID_HEADER)); request_.reset(); - sendNetworkRequest(reply->property(RequestDataPropertyKey).value(), promise); + sendNetworkRequest(reply->property(RequestBodyKey).toByteArray(), promise); return; } @@ -246,18 +243,16 @@ void RpcClient::networkRequestFinished(QNetworkReply* reply) } else { - auto const json_data = reply->readAll().trimmed().toStdString(); - auto const json = createVariant(); - auto result = RpcResponse{}; + auto const json = reply->readAll().trimmed().toStdString(); + auto response = RpcResponse{}; - if (auto top = tr_variant_serde::json().parse(json_data); top) + if (auto var = tr_variant_serde::json().parse(json)) { - std::swap(*json, *top); - result = parseResponseData(*json); + response = parseResponseData(*var); } promise.setProgressValue(1); - promise.reportFinished(&result); + promise.reportFinished(&response); } } diff --git a/qt/RpcClient.h b/qt/RpcClient.h index 9c57be677..dd55a2365 100644 --- a/qt/RpcClient.h +++ b/qt/RpcClient.h @@ -85,12 +85,10 @@ private slots: void localRequestFinished(TrVariantPtr response); private: - RpcResponseFuture sendRequest(TrVariantPtr json); QNetworkAccessManager* networkAccessManager(); - int64_t getNextTag(); - void sendNetworkRequest(TrVariantPtr req, QFutureInterface const& promise); - void sendLocalRequest(TrVariantPtr req, QFutureInterface const& promise, int64_t tag); + void sendNetworkRequest(QByteArray const& body, QFutureInterface const& promise); + void sendLocalRequest(tr_variant const& req, QFutureInterface const& promise, int64_t tag); [[nodiscard]] int64_t parseResponseTag(tr_variant& response) const; [[nodiscard]] RpcResponse parseResponseData(tr_variant& response) const; @@ -101,7 +99,6 @@ private: QUrl url_; QNetworkAccessManager* nam_ = {}; std::unordered_map> local_requests_; - int64_t next_tag_ = {}; bool const verbose_ = qEnvironmentVariableIsSet("TR_RPC_VERBOSE"); bool url_is_loopback_ = false; };