From 64231c107617c5e66dbab67d86697ae57e68ac0a Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles@charleskerr.com>
Date: Tue, 9 Dec 2025 18:18:46 -0600
Subject: [PATCH] refactor: extract-method is_valid_id() in rpcimpl (#7907)

---
 libtransmission/rpcimpl.cc | 59 +++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/libtransmission/rpcimpl.cc b/libtransmission/rpcimpl.cc
index 4deddd352..02249b961 100644
--- a/libtransmission/rpcimpl.cc
+++ b/libtransmission/rpcimpl.cc
@@ -129,11 +129,29 @@ namespace
 
 namespace
 {
+[[nodiscard]] constexpr bool is_valid_id(tr_variant const& val)
+{
+    switch (val.index())
+    {
+    // https://www.jsonrpc.org/specification#request_object
+    // An identifier established by the Client that MUST contain a String,
+    // Number, or NULL value if included. .. The value SHOULD normally not
+    // be Null and Numbers SHOULD NOT contain fractional parts
+    case tr_variant::StringIndex:
+    case tr_variant::IntIndex:
+    case tr_variant::DoubleIndex:
+    case tr_variant::NullIndex:
+        return true;
+
+    default:
+        return false;
+    }
+}
+
 // https://www.jsonrpc.org/specification#response_object
 [[nodiscard]] tr_variant::Map build_response(Error::Code code, tr_variant id, tr_variant::Map&& body)
 {
-    if (id.index() != tr_variant::StringIndex && id.index() != tr_variant::IntIndex && id.index() != tr_variant::DoubleIndex &&
-        id.index() != tr_variant::NullIndex)
+    if (!is_valid_id(id))
     {
         TR_ASSERT(false);
         id = nullptr;
@@ -2893,34 +2911,23 @@ void tr_rpc_request_exec_impl(tr_session* session, tr_variant const& request, tr
 
     auto data = tr_rpc_idle_data{};
     data.session = session;
-    if (is_jsonrpc)
+
+    if (auto iter = map->find(is_jsonrpc ? TR_KEY_id : TR_KEY_tag); iter != std::end(*map))
     {
-        if (auto it_id = map->find(TR_KEY_id); it_id != std::end(*map))
+        tr_variant const& id = iter->second;
+        if (is_jsonrpc && !is_valid_id(id))
         {
-            auto const& id = it_id->second;
-            switch (id.index())
-            {
-            case tr_variant::StringIndex:
-            case tr_variant::IntIndex:
-            case tr_variant::DoubleIndex:
-            case tr_variant::NullIndex:
-                data.id.merge(id); // copy
-                break;
-            default:
-                callback(
-                    session,
-                    build_response(
-                        Error::INVALID_REQUEST,
-                        nullptr,
-                        Error::build_data("id type must be String, Number, or Null"sv, {})));
-                return;
-            }
+            callback(
+                session,
+                build_response(
+                    Error::INVALID_REQUEST,
+                    nullptr,
+                    Error::build_data("id type must be String, Number, or Null"sv, {})));
+            return;
         }
+        data.id.merge(id);
     }
-    else if (auto tag = map->value_if<int64_t>(TR_KEY_tag); tag)
-    {
-        data.id = *tag;
-    }
+
     data.callback = std::move(callback);
 
     auto const is_notification = is_jsonrpc && !data.id.has_value();