From 3157e893a5b1502009b24683c325d590d80c98fc Mon Sep 17 00:00:00 2001
From: Sasidharan Gopal <sasidharan.gopal@freshworks.com>
Date: Fri, 30 Aug 2024 11:04:16 +0530
Subject: [PATCH] pr review fix

---
 .../network/common/redis/supported_commands.h |  4 ++--
 .../redis_proxy/command_splitter_impl.cc      | 19 +++++--------------
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/source/extensions/filters/network/common/redis/supported_commands.h b/source/extensions/filters/network/common/redis/supported_commands.h
index 34a27982ae6c..81299f565495 100644
--- a/source/extensions/filters/network/common/redis/supported_commands.h
+++ b/source/extensions/filters/network/common/redis/supported_commands.h
@@ -35,7 +35,7 @@ struct SupportedCommands {
         "zscore", "rpoplpush", "smove", "sunion", "sdiff", "sinter", "sinterstore", "zunionstore", 
         "zinterstore", "pfmerge", "georadius", "georadiusbymember", "rename", "getex", "sort", "zmscore", "sdiffstore", "msetnx", "substr",
         "zrangestore", "zunion", "echo", "zdiff", "sunionstore", "smismember",
-        "hrandfield", "geosearchstore", "zdiffstore", "geosearch", "randomkey", "zinter", "zrandmember",
+        "hrandfield", "geosearchstore", "zdiffstore", "geosearch", "zinter", "zrandmember",
         "bitop", "lpos", "renamenx","xread_simple_command");
   }
 
@@ -106,7 +106,7 @@ struct SupportedCommands {
    * @return commands which handle Redis commands without keys.
    */
   static const absl::flat_hash_set<std::string>& adminNokeyCommands() {
-    CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set<std::string>, "script", "flushall", "flushdb", "publish","pubsub", "keys", "slowlog", "config","client","info","cluster","select", "unwatch");
+    CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set<std::string>, "script", "flushall", "flushdb", "publish","pubsub", "keys", "slowlog", "config","client","info","cluster","select", "unwatch", "randomkey");
   }
 
   /**
diff --git a/source/extensions/filters/network/redis_proxy/command_splitter_impl.cc b/source/extensions/filters/network/redis_proxy/command_splitter_impl.cc
index 37ea703c6bdf..3126dde1ebdc 100644
--- a/source/extensions/filters/network/redis_proxy/command_splitter_impl.cc
+++ b/source/extensions/filters/network/redis_proxy/command_splitter_impl.cc
@@ -63,6 +63,7 @@ AdminRespHandlerType getresponseHandlerType(const std::string& command_name) {
         {"flushdb", AdminRespHandlerType::allresponses_mustbe_same},
         {"rename", AdminRespHandlerType::singleshardresponse},
         {"unwatch", AdminRespHandlerType::allresponses_mustbe_same},
+        {"randomkey", AdminRespHandlerType::singleshardresponse},
         // Add more mappings as needed
     };
 
@@ -269,21 +270,11 @@ SplitRequestPtr SimpleRequest::create(Router& router,
       callbacks.onResponse(Common::Redis::Utility::makeError(fmt::format("unexpected command format")));
        return nullptr;
   }
-  std::string key;
-  if (Common::Redis::SupportedCommands::noArgCommands().count(command_name) == 0) {
-    key =incoming_request->asArray()[shardKeyIndex].asString();;
-  }else{
-    key = std::string();
-  }
+  std::string key =incoming_request->asArray()[shardKeyIndex].asString();;
   std::unique_ptr<SimpleRequest> request_ptr{
       new SimpleRequest(callbacks, command_stats, time_source, delay_command_latency)};
-  
-  RouteSharedPtr route;
-  if (Common::Redis::SupportedCommands::noArgCommands().count(command_name) == 0) {
-    route = router.upstreamPool(incoming_request->asArray()[shardKeyIndex].asString(), stream_info);
-  }else{
-    route = router.upstreamPool(key, stream_info);
-  }
+
+  const auto route = router.upstreamPool(incoming_request->asArray()[shardKeyIndex].asString(), stream_info);
   if (route) {
     Common::Redis::RespValueSharedPtr base_request = std::move(incoming_request);
     request_ptr->handle_ = makeSingleServerRequest(
@@ -1702,7 +1693,7 @@ SplitRequestPtr TransactionRequest::create(Router& router,
 
   RouteSharedPtr route;
   if (transaction.key_.empty()) {
-    transaction.key_ = incoming_request->asArray()[1].asString();
+      transaction.key_ = incoming_request->asArray()[1].asString();
     route = router.upstreamPool(transaction.key_, stream_info);
     Common::Redis::RespValueSharedPtr multi_request =
         std::make_shared<Common::Redis::Client::MultiRequest>();