chore: factor out commandid from the server lib (#1253)

Also, move command registry command logic to main_service
where it should reside.

Signed-off-by: Roman Gershman <roman@dragonflydb.io>

src/facade/command_id.h

@@ -0,0 +1,71 @@
+// Copyright 2023, DragonflyDB authors.  All rights reserved.
+// See LICENSE for licensing terms.
+//
+
+#pragma once
+
+#include "facade/facade_types.h"
+
+namespace facade {
+
+class CommandId {
+ public:
+  using CmdArgList = facade::CmdArgList;
+
+  /**
+   * @brief Construct a new Command Id object
+   *
+   * When creating a new command use the https://github.com/redis/redis/tree/unstable/src/commands
+   * files to find the right arguments.
+   *
+   * @param name
+   * @param mask
+   * @param arity -     positive if command has fixed number of required arguments
+   *                    negative if command has minimum number of required arguments, but may have
+   * more.
+   * @param first_key - position of first key in argument list
+   * @param last_key  - position of last key in argument list,
+   *                    -1 means the last key index is (arg_length - 1), -2 means that the last key
+   * index is (arg_length - 2).
+   * @param step -      step count for locating repeating keys
+   */
+  CommandId(const char* name, uint32_t mask, int8_t arity, int8_t first_key, int8_t last_key,
+            int8_t step);
+
+  std::string_view name() const {
+    return name_;
+  }
+
+  int arity() const {
+    return arity_;
+  }
+
+  uint32_t opt_mask() const {
+    return opt_mask_;
+  }
+
+  int8_t first_key_pos() const {
+    return first_key_;
+  }
+
+  int8_t last_key_pos() const {
+    return last_key_;
+  }
+
+  int8_t key_arg_step() const {
+    return step_key_;
+  }
+
+  static uint32_t OptCount(uint32_t mask);
+
+ protected:
+  std::string_view name_;
+
+  uint32_t opt_mask_;
+  int8_t arity_;
+  int8_t first_key_;
+  int8_t last_key_;
+  int8_t step_key_;
+};
+
+}  // namespace facade

src/facade/facade.cc

@@ -6,10 +6,10 @@
 #include <absl/strings/str_cat.h>
 
 #include "base/logging.h"
+#include "facade/command_id.h"
 #include "facade/conn_context.h"
 #include "facade/dragonfly_connection.h"
 #include "facade/error.h"
-#include "facade/facade_types.h"
 
 namespace facade {
 
@@ -130,6 +130,16 @@ RedisReplyBuilder* ConnectionContext::operator->() {
   return static_cast<RedisReplyBuilder*>(rbuilder_.get());
 }
 
+CommandId::CommandId(const char* name, uint32_t mask, int8_t arity, int8_t first_key,
+                     int8_t last_key, int8_t step)
+    : name_(name), opt_mask_(mask), arity_(arity), first_key_(first_key), last_key_(last_key),
+      step_key_(step) {
+}
+
+uint32_t CommandId::OptCount(uint32_t mask) {
+  return absl::popcount(mask);
+}
+
 }  // namespace facade
 
 namespace std {

src/server/command_registry.cc

@@ -11,6 +11,7 @@
 #include "server/conn_context.h"
 
 namespace dfly {
+
 using namespace facade;
 using namespace std;
 
@@ -19,8 +20,7 @@ using absl::StrCat;
 
 CommandId::CommandId(const char* name, uint32_t mask, int8_t arity, int8_t first_key,
                      int8_t last_key, int8_t step)
-    : name_(name), opt_mask_(mask), arity_(arity), first_key_(first_key), last_key_(last_key),
-      step_key_(step) {
+    : facade::CommandId(name, mask, arity, first_key, last_key, step) {
   if (mask & CO::ADMIN)
     opt_mask_ |= CO::NOSCRIPT;
 
@@ -38,65 +38,6 @@ bool CommandId::IsTransactional() const {
   return false;
 }
 
-uint32_t CommandId::OptCount(uint32_t mask) {
-  return absl::popcount(mask);
-}
-
-CommandRegistry::CommandRegistry() {
-  static const char kCMD[] = "COMMAND";
-  CommandId cd(kCMD, CO::LOADING | CO::NOSCRIPT, -1, 0, 0, 0);
-
-  cd.SetHandler([this](const auto& args, auto* cntx) { return Command(args, cntx); });
-
-  cmd_map_.emplace(kCMD, std::move(cd));
-}
-
-void CommandRegistry::Command(CmdArgList args, ConnectionContext* cntx) {
-  unsigned cmd_cnt = 0;
-  for (const auto& val : cmd_map_) {
-    const CommandId& cd = val.second;
-    if (cd.opt_mask() & CO::HIDDEN)
-      continue;
-
-    ++cmd_cnt;
-  }
-
-  if (args.size() > 0) {
-    ToUpper(&args[0]);
-    string_view subcmd = ArgS(args, 0);
-    if (subcmd == "COUNT") {
-      return (*cntx)->SendLong(cmd_cnt);
-    } else {
-      return (*cntx)->SendError(kSyntaxErr, kSyntaxErrType);
-    }
-  }
-
-  (*cntx)->StartArray(cmd_cnt);
-
-  for (const auto& val : cmd_map_) {
-    const CommandId& cd = val.second;
-    if (cd.opt_mask() & CO::HIDDEN)
-      continue;
-
-    (*cntx)->StartArray(6);
-    (*cntx)->SendSimpleString(cd.name());
-    (*cntx)->SendLong(cd.arity());
-    (*cntx)->StartArray(CommandId::OptCount(cd.opt_mask()));
-
-    for (uint32_t i = 0; i < 32; ++i) {
-      unsigned obit = (1u << i);
-      if (cd.opt_mask() & obit) {
-        const char* name = CO::OptName(CO::CommandOpt{obit});
-        (*cntx)->SendSimpleString(name);
-      }
-    }
-
-    (*cntx)->SendLong(cd.first_key_pos());
-    (*cntx)->SendLong(cd.last_key_pos());
-    (*cntx)->SendLong(cd.key_arg_step());
-  }
-}
-
 CommandRegistry& CommandRegistry::operator<<(CommandId cmd) {
   string_view k = cmd.name();
   CHECK(cmd_map_.emplace(k, std::move(cmd)).second) << k;

src/server/command_registry.h

@@ -10,7 +10,7 @@
 #include <functional>
 
 #include "base/function2.hpp"
-#include "server/common.h"
+#include "facade/command_id.h"
 
 namespace dfly {
 
@@ -53,8 +53,11 @@ static_assert(!IsEvalKind(""));
 
 };  // namespace CO
 
-class CommandId {
+class CommandId : public facade::CommandId {
  public:
+  CommandId(const char* name, uint32_t mask, int8_t arity, int8_t first_key, int8_t last_key,
+            int8_t step);
+
   using Handler =
       fu2::function_base<true /*owns*/, true /*copyable*/, fu2::capacity_default,
                          false /* non-throwing*/, false /* strong exceptions guarantees*/,
@@ -63,54 +66,10 @@ class CommandId {
   using ArgValidator = fu2::function_base<true, true, fu2::capacity_default, false, false,
                                           bool(CmdArgList, ConnectionContext*) const>;
 
-  /**
-   * @brief Construct a new Command Id object
-   *
-   * When creating a new command use the https://github.com/redis/redis/tree/unstable/src/commands
-   * files to find the right arguments.
-   *
-   * @param name
-   * @param mask
-   * @param arity -     positive if command has fixed number of required arguments
-   *                    negative if command has minimum number of required arguments, but may have
-   * more.
-   * @param first_key - position of first key in argument list
-   * @param last_key  - position of last key in argument list,
-   *                    -1 means the last key index is (arg_length - 1), -2 means that the last key
-   * index is (arg_length - 2).
-   * @param step -      step count for locating repeating keys
-   */
-  CommandId(const char* name, uint32_t mask, int8_t arity, int8_t first_key, int8_t last_key,
-            int8_t step);
-
-  std::string_view name() const {
-    return name_;
-  }
-
-  int arity() const {
-    return arity_;
-  }
-
-  uint32_t opt_mask() const {
-    return opt_mask_;
-  }
-
-  int8_t first_key_pos() const {
-    return first_key_;
-  }
-
-  int8_t last_key_pos() const {
-    return last_key_;
-  }
-
   bool is_multi_key() const {
     return (last_key_ != first_key_) || (opt_mask_ & CO::VARIADIC_KEYS);
   }
 
-  int8_t key_arg_step() const {
-    return step_key_;
-  }
-
   CommandId& SetHandler(Handler f) {
     handler_ = std::move(f);
     return *this;
@@ -126,25 +85,16 @@ class CommandId {
     handler_(std::move(args), cntx);
   }
 
-  bool IsTransactional() const;
-
   // Returns true if validation succeeded.
   bool Validate(CmdArgList args, ConnectionContext* cntx) const {
     return !validator_ || validator_(std::move(args), cntx);
   }
 
+  bool IsTransactional() const;
+
   static const char* OptName(CO::CommandOpt fl);
-  static uint32_t OptCount(uint32_t mask);
 
  private:
-  std::string_view name_;
-
-  uint32_t opt_mask_;
-  int8_t arity_;
-  int8_t first_key_;
-  int8_t last_key_;
-  int8_t step_key_;
-
   Handler handler_;
   ArgValidator validator_;
 };
@@ -153,7 +103,7 @@ class CommandRegistry {
   absl::flat_hash_map<std::string_view, CommandId> cmd_map_;
 
  public:
-  CommandRegistry();
+  CommandRegistry() = default;
 
   CommandRegistry& operator<<(CommandId cmd);
 
@@ -174,10 +124,6 @@ class CommandRegistry {
       cb(k_v.first, k_v.second);
     }
   }
-
- private:
-  // Implements COMMAND functionality.
-  void Command(CmdArgList args, ConnectionContext* cntx);
 };
 
 }  // namespace dfly

src/server/main_service.cc

@@ -1718,6 +1718,48 @@ void Service::Pubsub(CmdArgList args, ConnectionContext* cntx) {
   }
 }
 
+void Service::Command(CmdArgList args, ConnectionContext* cntx) {
+  unsigned cmd_cnt = 0;
+  registry_.Traverse([&](string_view name, const CommandId& cd) {
+    if ((cd.opt_mask() & CO::HIDDEN) == 0) {
+      ++cmd_cnt;
+    }
+  });
+
+  if (args.size() > 0) {
+    ToUpper(&args[0]);
+    string_view subcmd = ArgS(args, 0);
+    if (subcmd == "COUNT") {
+      return (*cntx)->SendLong(cmd_cnt);
+    } else {
+      return (*cntx)->SendError(kSyntaxErr, kSyntaxErrType);
+    }
+  }
+
+  (*cntx)->StartArray(cmd_cnt);
+
+  registry_.Traverse([&](string_view name, const CommandId& cd) {
+    if (cd.opt_mask() & CO::HIDDEN)
+      return;
+    (*cntx)->StartArray(6);
+    (*cntx)->SendSimpleString(cd.name());
+    (*cntx)->SendLong(cd.arity());
+    (*cntx)->StartArray(CommandId::OptCount(cd.opt_mask()));
+
+    for (uint32_t i = 0; i < 32; ++i) {
+      unsigned obit = (1u << i);
+      if (cd.opt_mask() & obit) {
+        const char* name = CO::OptName(CO::CommandOpt{obit});
+        (*cntx)->SendSimpleString(name);
+      }
+    }
+
+    (*cntx)->SendLong(cd.first_key_pos());
+    (*cntx)->SendLong(cd.last_key_pos());
+    (*cntx)->SendLong(cd.key_arg_step());
+  });
+}
+
 VarzValue::Map Service::GetVarzStats() {
   VarzValue::Map res;
 
@@ -1822,7 +1864,8 @@ void Service::RegisterCommands() {
       << CI{"PUNSUBSCRIBE", CO::NOSCRIPT | CO::LOADING, -1, 0, 0, 0}.MFUNC(PUnsubscribe)
       << CI{"FUNCTION", CO::NOSCRIPT, 2, 0, 0, 0}.MFUNC(Function)
       << CI{"MONITOR", CO::ADMIN, 1, 0, 0, 0}.MFUNC(Monitor)
-      << CI{"PUBSUB", CO::LOADING | CO::FAST, -1, 0, 0, 0}.MFUNC(Pubsub);
+      << CI{"PUBSUB", CO::LOADING | CO::FAST, -1, 0, 0, 0}.MFUNC(Pubsub)
+      << CI{"COMMAND", CO::LOADING | CO::NOSCRIPT, -1, 0, 0, 0}.MFUNC(Command);
 
   StreamFamily::Register(&registry_);
   StringFamily::Register(&registry_);

src/server/main_service.h

@@ -108,6 +108,8 @@ class Service : public facade::ServiceInterface {
   void Function(CmdArgList args, ConnectionContext* cntx);
   void Monitor(CmdArgList args, ConnectionContext* cntx);
   void Pubsub(CmdArgList args, ConnectionContext* cntx);
+  void Command(CmdArgList args, ConnectionContext* cntx);
+
   void PubsubChannels(std::string_view pattern, ConnectionContext* cntx);
   void PubsubPatterns(ConnectionContext* cntx);