refactor: restrict remote-command to only one command name

src/utils/command_manager.cpp

@@ -25,11 +25,12 @@
  */
 
 #include <iostream>
-#include <thread>
 #include <sstream>
+#include <utility>
 
 #include <dsn/utility/utils.h>
 #include <dsn/tool-api/command_manager.h>
+#include <dsn/dist/fmt_logging.h>
 
 namespace dsn {
 
@@ -38,29 +39,19 @@ dsn_handle_t command_manager::register_command(const std::vector<std::string> &c
                                                const std::string &help_long,
                                                command_handler handler)
 {
-    utils::auto_write_lock l(_lock);
-    bool is_valid_cmd = false;
+    dcheck_eq(commands.size(), 1);
+    const std::string &cmd = commands[0];
+    dassert(!cmd.empty(), "should not register empty command");
 
-    for (const std::string &cmd : commands) {
-        if (!cmd.empty()) {
-            is_valid_cmd = true;
-            auto it = _handlers.find(cmd);
-            dassert(it == _handlers.end(), "command '%s' already regisered", cmd.c_str());
-        }
-    }
-    dassert(is_valid_cmd, "should not register empty command");
+    utils::auto_write_lock l(_lock);
 
-    command_instance *c = new command_instance();
+    auto c = new command_instance();
     c->commands = commands;
     c->help_long = help_long;
     c->help_short = help_one_line;
-    c->handler = handler;
+    c->handler = std::move(handler);
 
-    for (const std::string &cmd : commands) {
-        if (!cmd.empty()) {
-            _handlers[cmd] = c;
-        }
-    }
+    _handlers[cmd] = c;
     return c;
 }
 
@@ -97,15 +88,15 @@ bool command_manager::run_command(const std::string &cmd,
 
 command_manager::command_manager()
 {
-    register_command({"help", "h", "H", "Help"},
-                     "help|Help|h|H [command] - display help information",
+    register_command({"help"},
+                     "help - display help information",
                      "",
                      [this](const std::vector<std::string> &args) {
                          std::stringstream ss;
 
                          if (args.size() == 0) {
                              utils::auto_read_lock l(_lock);
-                             for (const auto &c : this->_handlers) {
+                             for (const auto &c : _handlers) {
                                  ss << c.second->help_short << std::endl;
                              }
                          } else {
@@ -123,52 +114,6 @@ command_manager::command_manager()
 
                          return ss.str();
                      });
-
-    register_command(
-        {"repeat", "r", "R", "Repeat"},
-        "repeat|Repeat|r|R interval_seconds max_count command - execute command periodically",
-        "repeat|Repeat|r|R interval_seconds max_count command - execute command every interval "
-        "seconds, to the max count as max_count (0 for infinite)",
-        [this](const std::vector<std::string> &args) {
-            std::stringstream ss;
-
-            if (args.size() < 3) {
-                return "insufficient arguments";
-            }
-
-            int interval_seconds = atoi(args[0].c_str());
-            if (interval_seconds <= 0) {
-                return "invalid interval argument";
-            }
-
-            int max_count = atoi(args[1].c_str());
-            if (max_count < 0) {
-                return "invalid max count";
-            }
-
-            if (max_count == 0) {
-                max_count = std::numeric_limits<int>::max();
-            }
-
-            std::string cmd = args[2];
-            std::vector<std::string> largs;
-            for (int i = 3; i < (int)args.size(); i++) {
-                largs.push_back(args[i]);
-            }
-
-            for (int i = 0; i < max_count; i++) {
-                std::string output;
-                auto r = this->run_command(cmd, largs, output);
-
-                if (!r) {
-                    break;
-                }
-
-                std::this_thread::sleep_for(std::chrono::seconds(interval_seconds));
-            }
-
-            return "repeat command completed";
-        });
 }
 
 command_manager::~command_manager() { _handlers.clear(); }

src/utils/test/command_manager.cpp → src/utils/test/command_manager_test.cpp

@@ -24,40 +24,39 @@
  * THE SOFTWARE.
  */
 
-/*
- * Description:
- *     Unit-test for command_manager.
- *
- * Revision history:
- *     Nov., 2015, @qinzuoyan (Zuoyan Qin), first version
- *     xxxx-xx-xx, author, fix bug about xxx
- */
-
 #include <dsn/tool-api/command_manager.h>
 #include <gtest/gtest.h>
 
-using namespace ::dsn;
+namespace dsn {
 
-void command_manager_module_init()
+struct command_manager_test : public ::testing::Test
 {
-    dsn::command_manager::instance().register_command(
-        {"test-cmd"},
-        "test-cmd - just for command_manager unit-test",
-        "test-cmd arg1 arg2 ...",
-        [](const std::vector<std::string> &args) {
-            std::stringstream ss;
-            ss << "test-cmd response: [";
-            for (size_t i = 0; i < args.size(); ++i) {
-                if (i != 0)
-                    ss << " ";
-                ss << args[i];
-            }
-            ss << "]";
-            return ss.str();
-        });
-}
+    command_manager_test()
+    {
+        _test_cmd = command_manager::instance().register_command(
+            {"test-cmd"},
+            "just for command_manager unit-test",
+            "test-cmd arg1 arg2 ...",
+            [](const std::vector<std::string> &args) {
+                std::stringstream ss;
+                ss << "test-cmd response: [";
+                for (size_t i = 0; i < args.size(); ++i) {
+                    if (i != 0)
+                        ss << " ";
+                    ss << args[i];
+                }
+                ss << "]";
+                return ss.str();
+            });
+    }
+
+    ~command_manager_test() override { command_manager::instance().deregister_command(_test_cmd); }
 
-TEST(command_manager, exist_command)
+private:
+    dsn_handle_t _test_cmd;
+};
+
+TEST_F(command_manager_test, exist_command)
 {
     const std::string cmd = "test-cmd";
     const std::vector<std::string> cmd_args{"this", "is", "test", "argument"};
@@ -68,7 +67,7 @@ TEST(command_manager, exist_command)
     ASSERT_EQ(output, expect_output);
 }
 
-TEST(command_manager, not_exist_command)
+TEST_F(command_manager_test, not_exist_command)
 {
     const std::string cmd = "not-exist-cmd";
     const std::vector<std::string> cmd_args{"arg1", "arg2"};
@@ -78,3 +77,5 @@ TEST(command_manager, not_exist_command)
     std::string expect_output = std::string("unknown command '") + cmd + "'";
     ASSERT_EQ(output, expect_output);
 }
+
+} // namespace dsn

src/utils/test/main.cpp

@@ -9,7 +9,6 @@ GTEST_API_ int main(int argc, char **argv)
 {
     testing::InitGoogleTest(&argc, argv);
 
-    command_manager_module_init();
     // init logging
     dsn_log_init("dsn::tools::simple_logger", "./", nullptr);