From ce721ced9032e7f6a4ccd04ac5ef8fdafe14094c Mon Sep 17 00:00:00 2001
From: Roman Gershman <romange@gmail.com>
Date: Thu, 3 Feb 2022 08:45:41 +0200
Subject: [PATCH] Get rid of SendRespBlob in RedisReplyBuilder

---
 server/command_registry.cc | 22 +++++++++++-----------
 server/generic_family.cc   |  9 ++++-----
 server/main_service.cc     | 13 +++++++------
 server/reply_builder.cc    |  6 ++++--
 server/reply_builder.h     |  5 +----
 5 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/server/command_registry.cc b/server/command_registry.cc
index d40bee8..8707c5d 100644
--- a/server/command_registry.cc
+++ b/server/command_registry.cc
@@ -38,29 +38,29 @@ CommandRegistry::CommandRegistry() {
 }
 
 void CommandRegistry::Command(CmdArgList args, ConnectionContext* cntx) {
-  size_t sz = cmd_map_.size();
-  string resp = StrCat("*", sz, "\r\n");
+  size_t len = cmd_map_.size();
+
+  (*cntx)->StartArray(len);
 
   for (const auto& val : cmd_map_) {
     const CommandId& cd = val.second;
-    StrAppend(&resp, "*6\r\n$", strlen(cd.name()), "\r\n", cd.name(), "\r\n");
-    StrAppend(&resp, ":", int(cd.arity()), "\r\n");
-    StrAppend(&resp, "*", CommandId::OptCount(cd.opt_mask()), "\r\n");
+    (*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});
-        StrAppend(&resp, "+", name, "\r\n");
+        (*cntx)->SendSimpleString(name);
       }
     }
 
-    StrAppend(&resp, ":", cd.first_key_pos(), "\r\n");
-    StrAppend(&resp, ":", cd.last_key_pos(), "\r\n");
-    StrAppend(&resp, ":", cd.key_arg_step(), "\r\n");
+    (*cntx)->SendLong(cd.first_key_pos());
+    (*cntx)->SendLong(cd.last_key_pos());
+    (*cntx)->SendLong(cd.key_arg_step());
   }
-
-  (*cntx)->SendRespBlob(resp);
 }
 
 CommandRegistry& CommandRegistry::operator<<(CommandId cmd) {
diff --git a/server/generic_family.cc b/server/generic_family.cc
index 59caf99..072fc3d 100644
--- a/server/generic_family.cc
+++ b/server/generic_family.cc
@@ -406,14 +406,13 @@ void GenericFamily::Scan(CmdArgList args, ConnectionContext* cntx) {
     DCHECK_EQ(0u, cursor);
   }
 
+  (*cntx)->StartArray(2);
   string res("*2\r\n$");
-  string curs_str = absl::StrCat(cursor);
-  absl::StrAppend(&res, curs_str.size(), "\r\n", curs_str, "\r\n*", keys.size(), "\r\n");
+  (*cntx)->SendSimpleString(absl::StrCat(cursor));
+  (*cntx)->StartArray(keys.size());
   for (const auto& k : keys) {
-    absl::StrAppend(&res, "$", k.size(), "\r\n", k, "\r\n");
+    (*cntx)->SendBulkString(k);
   }
-
-  return (*cntx)->SendRespBlob(res);
 }
 
 
diff --git a/server/main_service.cc b/server/main_service.cc
index 07e4f03..96dfa30 100644
--- a/server/main_service.cc
+++ b/server/main_service.cc
@@ -380,19 +380,20 @@ void Service::Eval(CmdArgList args, ConnectionContext* cntx) {
 }
 
 void Service::Exec(CmdArgList args, ConnectionContext* cntx) {
+  RedisReplyBuilder* rb = (*cntx).operator->();
+
   if (cntx->conn_state.exec_state == ConnectionState::EXEC_INACTIVE) {
-    return (*cntx)->SendError("EXEC without MULTI");
+    return rb->SendError("EXEC without MULTI");
   }
 
   if (cntx->conn_state.exec_state == ConnectionState::EXEC_ERROR) {
     cntx->conn_state.exec_state = ConnectionState::EXEC_INACTIVE;
     cntx->conn_state.exec_body.clear();
-    return (*cntx)->SendError("-EXECABORT Transaction discarded because of previous errors");
+    return rb->SendError("-EXECABORT Transaction discarded because of previous errors");
   }
 
-  (*cntx)->SendRespBlob(absl::StrCat("*", cntx->conn_state.exec_body.size(), "\r\n"));
-
-  if (!(*cntx)->GetError() && !cntx->conn_state.exec_body.empty()) {
+  rb->StartArray(cntx->conn_state.exec_body.size());
+  if (!cntx->conn_state.exec_body.empty()) {
     CmdArgVec str_list;
 
     for (auto& scmd : cntx->conn_state.exec_body) {
@@ -406,7 +407,7 @@ void Service::Exec(CmdArgList args, ConnectionContext* cntx) {
       CmdArgList cmd_arg_list{str_list.data(), str_list.size()};
       cntx->transaction->InitByArgs(cntx->conn_state.db_index, cmd_arg_list);
       scmd.descr->Invoke(cmd_arg_list, cntx);
-      if ((*cntx)->GetError())
+      if (rb->GetError())
         break;
     }
 
diff --git a/server/reply_builder.cc b/server/reply_builder.cc
index f7958bd..37cf972 100644
--- a/server/reply_builder.cc
+++ b/server/reply_builder.cc
@@ -78,14 +78,12 @@ void MCReplyBuilder::SendStored() {
   SendDirect("STORED\r\n");
 }
 
-
 void MCReplyBuilder::SendGetReply(std::string_view key, uint32_t flags, std::string_view value) {
   string first = absl::StrCat("VALUE ", key, " ", flags, " ", value.size(), "\r\n");
   iovec v[] = {IoVec(first), IoVec(value), IoVec(kCRLF)};
   Send(v, ABSL_ARRAYSIZE(v));
 }
 
-
 void MCReplyBuilder::SendError(string_view str) {
   SendDirect("ERROR\r\n");
 }
@@ -211,6 +209,10 @@ void RedisReplyBuilder::SendStringArr(absl::Span<const std::string_view> arr) {
   SendDirect(res);
 }
 
+void RedisReplyBuilder::StartArray(unsigned len) {
+  SendDirect(absl::StrCat("*", len, kCRLF));
+}
+
 void ReqSerializer::SendCommand(std::string_view str) {
   VLOG(1) << "SendCommand: " << str;
 
diff --git a/server/reply_builder.h b/server/reply_builder.h
index 6f7504f..6d75385 100644
--- a/server/reply_builder.h
+++ b/server/reply_builder.h
@@ -120,10 +120,7 @@ class RedisReplyBuilder : public SinkReplyBuilder {
 
   virtual void SendBulkString(std::string_view str);
 
-  // TODO: to get rid of it. We should only send high-level data.
-  void SendRespBlob(std::string_view str) {
-    SendDirect(str);
-  }
+  virtual void StartArray(unsigned len);
 
  private:
 };