This is an automated email from the ASF dual-hosted git repository.
twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new a453d384 Refactor command utilities to class methods (#1858)
a453d384 is described below
commit a453d384254f767ae7b7d2a2cb1a6a5f038ce121
Author: Twice <[email protected]>
AuthorDate: Sat Oct 28 21:33:45 2023 +0900
Refactor command utilities to class methods (#1858)
---
src/cluster/cluster.cc | 2 +-
src/commands/cmd_cluster.cc | 2 +-
src/commands/cmd_server.cc | 14 ++++++------
src/commands/commander.cc | 38 +++++++++++++++----------------
src/commands/commander.h | 52 +++++++++++++++++++++++--------------------
src/config/config.cc | 4 ++--
src/server/server.cc | 4 ++--
src/storage/scripting.cc | 2 +-
tests/cppunit/cluster_test.cc | 24 ++++++++++----------
tests/cppunit/config_test.cc | 6 ++---
10 files changed, 76 insertions(+), 72 deletions(-)
diff --git a/src/cluster/cluster.cc b/src/cluster/cluster.cc
index 7fa4e22d..6c19f11b 100644
--- a/src/cluster/cluster.cc
+++ b/src/cluster/cluster.cc
@@ -748,7 +748,7 @@ bool Cluster::IsWriteForbiddenSlot(int slot) { return
svr_->slot_migrator->GetFo
Status Cluster::CanExecByMySelf(const redis::CommandAttributes *attributes,
const std::vector<std::string> &cmd_tokens,
redis::Connection *conn) {
std::vector<int> keys_indexes;
- auto s = redis::GetKeysFromCommand(attributes, cmd_tokens, &keys_indexes);
+ auto s = redis::CommandTable::GetKeysFromCommand(attributes, cmd_tokens,
&keys_indexes);
// No keys
if (!s.IsOK()) return Status::OK();
diff --git a/src/commands/cmd_cluster.cc b/src/commands/cmd_cluster.cc
index 03e3bee8..1ecb01b9 100644
--- a/src/commands/cmd_cluster.cc
+++ b/src/commands/cmd_cluster.cc
@@ -182,7 +182,7 @@ class CommandClusterX : public Commander {
// CLUSTERX SETSLOT $SLOT_ID NODE $NODE_ID $VERSION
if (subcommand_ == "setslot" && args_.size() == 6) {
- Status s = CommanderHelper::ParseSlotRanges(args_[2], slot_ranges_);
+ Status s = CommandTable::ParseSlotRanges(args_[2], slot_ranges_);
if (!s.IsOK()) {
return s;
}
diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc
index f43f8937..80854799 100644
--- a/src/commands/cmd_server.cc
+++ b/src/commands/cmd_server.cc
@@ -609,7 +609,7 @@ class CommandCommand : public Commander {
public:
Status Execute(Server *svr, Connection *conn, std::string *output) override {
if (args_.size() == 1) {
- GetAllCommandsInfo(output);
+ CommandTable::GetAllCommandsInfo(output);
} else {
std::string sub_command = util::ToLower(args_[1]);
if ((sub_command == "count" && args_.size() != 2) || (sub_command ==
"getkeys" && args_.size() < 3) ||
@@ -618,18 +618,18 @@ class CommandCommand : public Commander {
}
if (sub_command == "count") {
- *output = redis::Integer(GetCommandNum());
+ *output = redis::Integer(CommandTable::Size());
} else if (sub_command == "info") {
- GetCommandsInfo(output, std::vector<std::string>(args_.begin() + 2,
args_.end()));
+ CommandTable::GetCommandsInfo(output,
std::vector<std::string>(args_.begin() + 2, args_.end()));
} else if (sub_command == "getkeys") {
- auto cmd_iter =
command_details::original_commands.find(util::ToLower(args_[2]));
- if (cmd_iter == command_details::original_commands.end()) {
+ auto cmd_iter =
CommandTable::GetOriginal()->find(util::ToLower(args_[2]));
+ if (cmd_iter == CommandTable::GetOriginal()->end()) {
return {Status::RedisUnknownCmd, "Invalid command specified"};
}
std::vector<int> keys_indexes;
- auto s = GetKeysFromCommand(cmd_iter->second,
std::vector<std::string>(args_.begin() + 2, args_.end()),
- &keys_indexes);
+ auto s = CommandTable::GetKeysFromCommand(
+ cmd_iter->second, std::vector<std::string>(args_.begin() + 2,
args_.end()), &keys_indexes);
if (!s.IsOK()) return s;
if (keys_indexes.size() == 0) {
diff --git a/src/commands/commander.cc b/src/commands/commander.cc
index b767b017..024db614 100644
--- a/src/commands/commander.cc
+++ b/src/commands/commander.cc
@@ -26,21 +26,21 @@ namespace redis {
RegisterToCommandTable::RegisterToCommandTable(std::initializer_list<CommandAttributes>
list) {
for (const auto &attr : list) {
- command_details::redis_command_table.emplace_back(attr);
- command_details::original_commands[attr.name] =
&command_details::redis_command_table.back();
- command_details::commands[attr.name] =
&command_details::redis_command_table.back();
+ CommandTable::redis_command_table.emplace_back(attr);
+ CommandTable::original_commands[attr.name] =
&CommandTable::redis_command_table.back();
+ CommandTable::commands[attr.name] =
&CommandTable::redis_command_table.back();
}
}
-size_t GetCommandNum() { return command_details::redis_command_table.size(); }
+size_t CommandTable::Size() { return redis_command_table.size(); }
-const CommandMap *GetOriginalCommands() { return
&command_details::original_commands; }
+const CommandMap *CommandTable::GetOriginal() { return &original_commands; }
-CommandMap *GetCommands() { return &command_details::commands; }
+CommandMap *CommandTable::Get() { return &commands; }
-void ResetCommands() { command_details::commands =
command_details::original_commands; }
+void CommandTable::Reset() { commands = original_commands; }
-std::string GetCommandInfo(const CommandAttributes *command_attributes) {
+std::string CommandTable::GetCommandInfo(const CommandAttributes
*command_attributes) {
std::string command, command_flags;
command.append(redis::MultiLen(6));
command.append(redis::BulkString(command_attributes->name));
@@ -54,20 +54,20 @@ std::string GetCommandInfo(const CommandAttributes
*command_attributes) {
return command;
}
-void GetAllCommandsInfo(std::string *info) {
- info->append(redis::MultiLen(command_details::original_commands.size()));
- for (const auto &iter : command_details::original_commands) {
+void CommandTable::GetAllCommandsInfo(std::string *info) {
+ info->append(redis::MultiLen(original_commands.size()));
+ for (const auto &iter : original_commands) {
auto command_attribute = iter.second;
auto command_info = GetCommandInfo(command_attribute);
info->append(command_info);
}
}
-void GetCommandsInfo(std::string *info, const std::vector<std::string>
&cmd_names) {
+void CommandTable::GetCommandsInfo(std::string *info, const
std::vector<std::string> &cmd_names) {
info->append(redis::MultiLen(cmd_names.size()));
for (const auto &cmd_name : cmd_names) {
- auto cmd_iter =
command_details::original_commands.find(util::ToLower(cmd_name));
- if (cmd_iter == command_details::original_commands.end()) {
+ auto cmd_iter = original_commands.find(util::ToLower(cmd_name));
+ if (cmd_iter == original_commands.end()) {
info->append(redis::NilString());
} else {
auto command_attribute = cmd_iter->second;
@@ -86,8 +86,8 @@ void DumpKeyRange(std::vector<int> &keys_index, int argc,
const CommandKeyRange
}
}
-Status GetKeysFromCommand(const CommandAttributes *attributes, const
std::vector<std::string> &cmd_tokens,
- std::vector<int> *keys_index) {
+Status CommandTable::GetKeysFromCommand(const CommandAttributes *attributes,
const std::vector<std::string> &cmd_tokens,
+ std::vector<int> *keys_index) {
if (attributes->key_range.first_key == 0) {
return {Status::NotOK, "The command has no key arguments"};
}
@@ -113,11 +113,11 @@ Status GetKeysFromCommand(const CommandAttributes
*attributes, const std::vector
return Status::OK();
}
-bool IsCommandExists(const std::string &name) {
- return command_details::original_commands.find(util::ToLower(name)) !=
command_details::original_commands.end();
+bool CommandTable::IsExists(const std::string &name) {
+ return original_commands.find(util::ToLower(name)) !=
original_commands.end();
}
-Status CommanderHelper::ParseSlotRanges(const std::string &slots_str,
std::vector<SlotRange> &slots) {
+Status CommandTable::ParseSlotRanges(const std::string &slots_str,
std::vector<SlotRange> &slots) {
if (slots_str.empty()) {
return {Status::NotOK, "No slots to parse."};
}
diff --git a/src/commands/commander.h b/src/commands/commander.h
index f6caad24..758cf993 100644
--- a/src/commands/commander.h
+++ b/src/commands/commander.h
@@ -89,11 +89,6 @@ class CommanderWithParseMove : Commander {
virtual Status ParseMove(std::vector<std::string> &&args) { return
Status::OK(); }
};
-class CommanderHelper {
- public:
- static Status ParseSlotRanges(const std::string &slots_str,
std::vector<SlotRange> &slots);
-};
-
using CommanderFactory = std::function<std::unique_ptr<Commander>()>;
struct CommandKeyRange {
@@ -250,16 +245,36 @@ struct RegisterToCommandTable {
RegisterToCommandTable(std::initializer_list<CommandAttributes> list);
};
-// these variables cannot be put into source files (to ensure init order for
multiple TUs)
-namespace command_details {
-inline std::deque<CommandAttributes> redis_command_table;
+struct CommandTable {
+ public:
+ CommandTable() = delete;
+
+ static CommandMap *Get();
+ static const CommandMap *GetOriginal();
+ static void Reset();
+
+ static void GetAllCommandsInfo(std::string *info);
+ static void GetCommandsInfo(std::string *info, const
std::vector<std::string> &cmd_names);
+ static std::string GetCommandInfo(const CommandAttributes
*command_attributes);
+ static Status GetKeysFromCommand(const CommandAttributes *attributes, const
std::vector<std::string> &cmd_tokens,
+ std::vector<int> *keys_indexes);
-// Original Command table before rename-command directive
-inline CommandMap original_commands;
+ static size_t Size();
+ static bool IsExists(const std::string &name);
-// Command table after rename-command directive
-inline CommandMap commands;
-} // namespace command_details
+ static Status ParseSlotRanges(const std::string &slots_str,
std::vector<SlotRange> &slots);
+
+ private:
+ static inline std::deque<CommandAttributes> redis_command_table;
+
+ // Original Command table before rename-command directive
+ static inline CommandMap original_commands;
+
+ // Command table after rename-command directive
+ static inline CommandMap commands;
+
+ friend struct RegisterToCommandTable;
+};
#define KVROCKS_CONCAT(a, b) a##b // NOLINT
#define KVROCKS_CONCAT2(a, b) KVROCKS_CONCAT(a, b) // NOLINT
@@ -268,15 +283,4 @@ inline CommandMap commands;
#define REDIS_REGISTER_COMMANDS(...) \
static RegisterToCommandTable KVROCKS_CONCAT2(register_to_command_table_,
__LINE__){__VA_ARGS__};
-size_t GetCommandNum();
-CommandMap *GetCommands();
-void ResetCommands();
-const CommandMap *GetOriginalCommands();
-void GetAllCommandsInfo(std::string *info);
-void GetCommandsInfo(std::string *info, const std::vector<std::string>
&cmd_names);
-std::string GetCommandInfo(const CommandAttributes *command_attributes);
-Status GetKeysFromCommand(const CommandAttributes *attributes, const
std::vector<std::string> &cmd_tokens,
- std::vector<int> *keys_indexes);
-bool IsCommandExists(const std::string &name);
-
} // namespace redis
diff --git a/src/config/config.cc b/src/config/config.cc
index d7ee57e4..3313eaba 100644
--- a/src/config/config.cc
+++ b/src/config/config.cc
@@ -286,7 +286,7 @@ void Config::initFieldValidator() {
if (args.size() != 2) {
return {Status::NotOK, "Invalid rename-command format"};
}
- auto commands = redis::GetCommands();
+ auto commands = redis::CommandTable::Get();
auto cmd_iter = commands->find(util::ToLower(args[0]));
if (cmd_iter == commands->end()) {
return {Status::NotOK, "No such command in rename-command"};
@@ -436,7 +436,7 @@ void Config::initFieldCallback() {
profiling_sample_commands.clear();
return Status::OK();
}
- if (!redis::IsCommandExists(cmd)) {
+ if (!redis::CommandTable::IsExists(cmd)) {
return {Status::NotOK, cmd + " is not Kvrocks supported command"};
}
// profiling_sample_commands use command's original name,
regardless of rename-command directive
diff --git a/src/server/server.cc b/src/server/server.cc
index 412e7967..89c121a2 100644
--- a/src/server/server.cc
+++ b/src/server/server.cc
@@ -56,7 +56,7 @@ constexpr const char *REDIS_VERSION = "4.0.0";
Server::Server(engine::Storage *storage, Config *config)
: storage(storage), start_time_(util::GetTimeStamp()), config_(config),
namespace_(storage) {
// init commands stats here to prevent concurrent insert, and cause core
- auto commands = redis::GetOriginalCommands();
+ auto commands = redis::CommandTable::GetOriginal();
for (const auto &iter : *commands) {
stats.commands_stats[iter.first].calls = 0;
stats.commands_stats[iter.first].latency = 0;
@@ -1509,7 +1509,7 @@ ReplState Server::GetReplicationState() {
Status Server::LookupAndCreateCommand(const std::string &cmd_name,
std::unique_ptr<redis::Commander> *cmd) {
if (cmd_name.empty()) return {Status::RedisUnknownCmd};
- auto commands = redis::GetCommands();
+ auto commands = redis::CommandTable::Get();
auto cmd_iter = commands->find(util::ToLower(cmd_name));
if (cmd_iter == commands->end()) {
return {Status::RedisUnknownCmd};
diff --git a/src/storage/scripting.cc b/src/storage/scripting.cc
index b0d63eb5..54aac26d 100644
--- a/src/storage/scripting.cc
+++ b/src/storage/scripting.cc
@@ -704,7 +704,7 @@ int RedisGenericCommand(lua_State *lua, int raise_error) {
}
}
- auto commands = redis::GetCommands();
+ auto commands = redis::CommandTable::Get();
auto cmd_iter = commands->find(util::ToLower(args[0]));
if (cmd_iter == commands->end()) {
PushError(lua, "Unknown Redis command called from Lua script");
diff --git a/tests/cppunit/cluster_test.cc b/tests/cppunit/cluster_test.cc
index bdbf20f2..a19d05e0 100644
--- a/tests/cppunit/cluster_test.cc
+++ b/tests/cppunit/cluster_test.cc
@@ -217,7 +217,7 @@ TEST(Cluster, ClusterParseSlotRanges) {
std::vector<SlotRange> slots;
const std::string t_single_slot = "1234";
- s = redis::CommanderHelper::ParseSlotRanges(t_single_slot, slots);
+ s = redis::CommandTable::ParseSlotRanges(t_single_slot, slots);
ASSERT_TRUE(s.IsOK());
s = cluster.SetSlotRanges(slots, node_id, version);
ASSERT_TRUE(s.IsOK());
@@ -225,7 +225,7 @@ TEST(Cluster, ClusterParseSlotRanges) {
slots.clear();
const std::string t_single_ranges = "1234-1236";
- s = redis::CommanderHelper::ParseSlotRanges(t_single_ranges, slots);
+ s = redis::CommandTable::ParseSlotRanges(t_single_ranges, slots);
ASSERT_TRUE(s.IsOK());
s = cluster.SetSlotRanges(slots, node_id, version);
ASSERT_TRUE(s.IsOK());
@@ -233,7 +233,7 @@ TEST(Cluster, ClusterParseSlotRanges) {
slots.clear();
const std::string t_mixed_slot = "10229 16301 4710 3557-8559 ";
- s = redis::CommanderHelper::ParseSlotRanges(t_mixed_slot, slots);
+ s = redis::CommandTable::ParseSlotRanges(t_mixed_slot, slots);
ASSERT_TRUE(s.IsOK());
s = cluster.SetSlotRanges(slots, node_id, version);
ASSERT_TRUE(s.IsOK());
@@ -241,13 +241,13 @@ TEST(Cluster, ClusterParseSlotRanges) {
slots.clear();
std::string empty_slots;
- s = redis::CommanderHelper::ParseSlotRanges(empty_slots, slots);
+ s = redis::CommandTable::ParseSlotRanges(empty_slots, slots);
ASSERT_FALSE(s.IsOK());
ASSERT_TRUE(s.Msg() == "No slots to parse.");
slots.clear();
std::string space_slots = " ";
- s = redis::CommanderHelper::ParseSlotRanges(space_slots, slots);
+ s = redis::CommandTable::ParseSlotRanges(space_slots, slots);
ASSERT_FALSE(s.IsOK());
ASSERT_TRUE(s.Msg() == fmt::format("Invalid slots: `{}`. No slots to parse. "
"Please use spaces to separate slots.",
@@ -278,44 +278,44 @@ TEST(Cluster, ClusterParseSlotRanges) {
}
}
- s = redis::CommanderHelper::ParseSlotRanges(error_slots[0], slots);
+ s = redis::CommandTable::ParseSlotRanges(error_slots[0], slots);
ASSERT_FALSE(s.IsOK());
ASSERT_TRUE(s.Msg() == "Invalid slot id: encounter non-integer
characters");
slots.clear();
- s = redis::CommanderHelper::ParseSlotRanges(error_slots[1], slots);
+ s = redis::CommandTable::ParseSlotRanges(error_slots[1], slots);
ASSERT_FALSE(s.IsOK());
ASSERT_TRUE(s.Msg() == "Invalid slot id: out of numeric range");
slots.clear();
- s = redis::CommanderHelper::ParseSlotRanges(error_slots[2], slots);
+ s = redis::CommandTable::ParseSlotRanges(error_slots[2], slots);
ASSERT_FALSE(s.IsOK());
ASSERT_TRUE(s.Msg() == fmt::format("Invalid slot range: `{}`. The
character '-' can't appear "
"in the first or last position.",
front_slot_ranges));
slots.clear();
- s = redis::CommanderHelper::ParseSlotRanges(error_slots[3], slots);
+ s = redis::CommandTable::ParseSlotRanges(error_slots[3], slots);
ASSERT_FALSE(s.IsOK());
ASSERT_TRUE(s.Msg() ==
fmt::format("Invalid slot range: `{}`. The character '-' can't
appear in the first or last position.",
back_slot_ranges));
slots.clear();
- s = redis::CommanderHelper::ParseSlotRanges(error_slots[4], slots);
+ s = redis::CommandTable::ParseSlotRanges(error_slots[4], slots);
ASSERT_FALSE(s.IsOK());
ASSERT_TRUE(s.Msg() ==
fmt::format("Invalid slot range: `{}`. The character '-' can't
appear in the first or last position.",
f_single_slot));
slots.clear();
- s = redis::CommanderHelper::ParseSlotRanges(error_slots[5], slots);
+ s = redis::CommandTable::ParseSlotRanges(error_slots[5], slots);
ASSERT_FALSE(s.IsOK());
ASSERT_TRUE(s.Msg() == fmt::format("Invalid slot range: `{}`. The slot
range should be of the form `int1-int2`.",
overmuch_slot_ranges));
slots.clear();
- s = redis::CommanderHelper::ParseSlotRanges(error_slots[6], slots);
+ s = redis::CommandTable::ParseSlotRanges(error_slots[6], slots);
ASSERT_FALSE(s.IsOK());
ASSERT_TRUE(
s.Msg() ==
diff --git a/tests/cppunit/config_test.cc b/tests/cppunit/config_test.cc
index d62dc615..a54868c7 100644
--- a/tests/cppunit/config_test.cc
+++ b/tests/cppunit/config_test.cc
@@ -145,7 +145,7 @@ TEST(Config, GetRenameCommand) {
output_file << "rename-command SET SET_NEW"
<< "\n";
output_file.close();
- redis::ResetCommands();
+ redis::CommandTable::Reset();
Config config;
ASSERT_TRUE(config.Load(CLIOptions(path)).IsOK());
std::vector<std::string> values;
@@ -171,12 +171,12 @@ TEST(Config, Rewrite) {
<< "\n";
output_file.close();
- redis::ResetCommands();
+ redis::CommandTable::Reset();
Config config;
ASSERT_TRUE(config.Load(CLIOptions(path)).IsOK());
ASSERT_TRUE(config.Rewrite({}).IsOK());
// Need to re-populate the command table since it has renamed by the previous
- redis::ResetCommands();
+ redis::CommandTable::Reset();
Config new_config;
ASSERT_TRUE(new_config.Load(CLIOptions(path)).IsOK());
unlink(path);
