From df1dc5812cd5c928570cf957d5efc69753986149 Mon Sep 17 00:00:00 2001 From: Yat Ho Date: Sun, 7 Jan 2024 00:40:23 +0800 Subject: [PATCH] fix: restore the `tag` key in RPC response (#6492) * fix: restore the `tag` key in RPC response * chore: `tr_rpc_request_exec()` housekeeping * feat: new tests for RPC tag --- libtransmission/rpcimpl.cc | 52 +++++++++----------- tests/libtransmission/rpc-test.cc | 80 +++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 30 deletions(-) diff --git a/libtransmission/rpcimpl.cc b/libtransmission/rpcimpl.cc index e8effbb04..75716dc05 100644 --- a/libtransmission/rpcimpl.cc +++ b/libtransmission/rpcimpl.cc @@ -2117,56 +2117,36 @@ void tr_rpc_request_exec(tr_session* session, tr_variant const& request, tr_rpc_ auto const* const request_map = request.get_if(); - if (callback == nullptr) + if (!callback) { callback = noop_response_callback; } - // find the args auto const empty_args = tr_variant::Map{}; auto const* args_in = &empty_args; + auto method_name = std::string_view{}; if (request_map != nullptr) { + // find the args if (auto const* val = request_map->find_if(TR_KEY_arguments); val != nullptr) { args_in = val; } - } - auto const tag = args_in->value_if(TR_KEY_tag); - - // find the requested method - auto method_name = std::string_view{}; - if (request_map != nullptr) - { + // find the requested method if (auto const* val = request_map->find_if(TR_KEY_method); val != nullptr) { method_name = *val; } } + auto const tag = request_map->value_if(TR_KEY_tag); + auto const test = [method_name](auto const& handler) { return handler.first == method_name; }; - if (auto const end = std::end(SyncHandlers), handler = std::find_if(std::begin(SyncHandlers), end, test); handler != end) - { - auto args_out = tr_variant::Map{}; - char const* const result = (*handler->second)(session, *args_in, args_out); - - auto response = tr_variant::Map{ 3U }; - response.try_emplace(TR_KEY_arguments, std::move(args_out)); - response.try_emplace(TR_KEY_result, result != nullptr ? result : "success"); - if (tag.has_value()) - { - response.try_emplace(TR_KEY_tag, *tag); - } - - (callback)(session, tr_variant{ std::move(response) }); - return; - } - if (auto const end = std::end(AsyncHandlers), handler = std::find_if(std::begin(AsyncHandlers), end, test); handler != end) { auto* const data = new tr_rpc_idle_data{}; @@ -2181,16 +2161,28 @@ void tr_rpc_request_exec(tr_session* session, tr_variant const& request, tr_rpc_ return; } - // couldn't find a handler auto response = tr_variant::Map{ 3U }; - response.try_emplace(TR_KEY_arguments, 0); - response.try_emplace(TR_KEY_result, "no method name"); if (tag.has_value()) { response.try_emplace(TR_KEY_tag, *tag); } - (callback)(session, tr_variant{ std::move(response) }); + if (auto const end = std::end(SyncHandlers), handler = std::find_if(std::begin(SyncHandlers), end, test); handler != end) + { + auto args_out = tr_variant::Map{}; + char const* const result = (handler->second)(session, *args_in, args_out); + + response.try_emplace(TR_KEY_arguments, std::move(args_out)); + response.try_emplace(TR_KEY_result, result != nullptr ? result : "success"); + } + else + { + // couldn't find a handler + response.try_emplace(TR_KEY_arguments, 0); + response.try_emplace(TR_KEY_result, "no method name"); + } + + callback(session, tr_variant{ std::move(response) }); } /** diff --git a/tests/libtransmission/rpc-test.cc b/tests/libtransmission/rpc-test.cc index 28de7d38f..ad87e96ed 100644 --- a/tests/libtransmission/rpc-test.cc +++ b/tests/libtransmission/rpc-test.cc @@ -7,6 +7,7 @@ #include #include // size_t #include // int64_t +#include #include // std::inserter #include #include @@ -65,6 +66,85 @@ TEST_F(RpcTest, list) EXPECT_EQ(5, i); } +TEST_F(RpcTest, tagSync) +{ + auto request_map = tr_variant::Map{ 2U }; + request_map.try_emplace(TR_KEY_method, "session-stats"); + request_map.try_emplace(TR_KEY_tag, 12345); + + auto response = tr_variant{}; + tr_rpc_request_exec( + session_, + tr_variant{ std::move(request_map) }, + [&response](tr_session* /*session*/, tr_variant&& resp) { response = std::move(resp); }); + + auto const* const response_map = response.get_if(); + ASSERT_NE(response_map, nullptr); + auto const result = response_map->value_if(TR_KEY_result); + ASSERT_TRUE(result); + EXPECT_EQ(*result, "success"sv); + auto const tag = response_map->value_if(TR_KEY_tag); + ASSERT_TRUE(tag); + EXPECT_EQ(*tag, 12345); +} + +TEST_F(RpcTest, tagAsync) +{ + auto* tor = zeroTorrentInit(ZeroTorrentState::Complete); + EXPECT_NE(nullptr, tor); + + auto request_map = tr_variant::Map{ 3U }; + request_map.try_emplace(TR_KEY_method, "torrent-rename-path"); + request_map.try_emplace(TR_KEY_tag, 12345); + + auto arguments_map = tr_variant::Map{ 2U }; + arguments_map.try_emplace(TR_KEY_path, "files-filled-with-zeroes/512"); + arguments_map.try_emplace(TR_KEY_name, "512_test"); + request_map.try_emplace(TR_KEY_arguments, std::move(arguments_map)); + + auto promise = std::promise{}; + auto future = promise.get_future(); + tr_rpc_request_exec( + session_, + tr_variant{ std::move(request_map) }, + [&promise](tr_session* /*session*/, tr_variant&& resp) { promise.set_value(std::move(resp)); }); + auto const response = future.get(); + + auto const* const response_map = response.get_if(); + ASSERT_NE(response_map, nullptr); + auto const result = response_map->value_if(TR_KEY_result); + ASSERT_TRUE(result); + EXPECT_EQ(*result, "success"sv); + auto const tag = response_map->value_if(TR_KEY_tag); + ASSERT_TRUE(tag); + EXPECT_EQ(*tag, 12345); + + // cleanup + tr_torrentRemove(tor, false, nullptr, nullptr); +} + +TEST_F(RpcTest, tagNoHandler) +{ + auto request_map = tr_variant::Map{ 2U }; + request_map.try_emplace(TR_KEY_method, "sdgdhsgg"); + request_map.try_emplace(TR_KEY_tag, 12345); + + auto response = tr_variant{}; + tr_rpc_request_exec( + session_, + tr_variant{ std::move(request_map) }, + [&response](tr_session* /*session*/, tr_variant&& resp) { response = std::move(resp); }); + + auto const* const response_map = response.get_if(); + ASSERT_NE(response_map, nullptr); + auto const result = response_map->value_if(TR_KEY_result); + ASSERT_TRUE(result); + EXPECT_EQ(*result, "no method name"sv); + auto const tag = response_map->value_if(TR_KEY_tag); + ASSERT_TRUE(tag); + EXPECT_EQ(*tag, 12345); +} + /*** **** ***/