Return as Map instead of Array

git-hulk · Aug 8, 2024 · 2e0e1fa · 2e0e1fa
1 parent 4298457
commit 2e0e1fa
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 31 deletions.
diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc
@@ -1291,24 +1291,21 @@ class CommandPollUpdates : public Commander {
   }
 
   Status Execute(Server *srv, Connection *conn, std::string *output) override {
+    uint64_t next_sequence = sequence_;
     // sequence + 1 is for excluding the current sequence to avoid getting duplicate updates
     auto batches = GET_OR_RET(srv->PollUpdates(sequence_ + 1, max_, is_strict_));
-
-    *output = redis::MultiLen(8);
-    *output += redis::BulkString("latest_sequence");
-    *output += redis::Integer(srv->storage->LatestSeqNumber());
-    *output += redis::BulkString("format");
-    *output += redis::BulkString("RAW");
-    *output += redis::BulkString("updates");
-    *output += redis::MultiLen(batches.size());
-    uint64_t next_sequence = sequence_;
+    std::string updates = redis::MultiLen(batches.size());
     for (const auto &batch : batches) {
-      *output += redis::BulkString(util::StringToHex(batch.writeBatchPtr->Data()));
+      updates += redis::BulkString(util::StringToHex(batch.writeBatchPtr->Data()));
       // It might contain more than one sequence in a batch
       next_sequence = batch.sequence + batch.writeBatchPtr->Count() - 1;
     }
-    *output += redis::BulkString("next_sequence");
-    *output += redis::Integer(next_sequence);
+
+    *output = conn->Map(std::map<std::string, std::string>{
+        {redis::BulkString("latest_sequence"), redis::Integer(srv->storage->LatestSeqNumber())},
+        {redis::BulkString("updates"), std::move(updates)},
+        {redis::BulkString("next_sequence"), redis::Integer(next_sequence)},
+    });
     return Status::OK();
   }
 

diff --git a/src/server/redis_connection.h b/src/server/redis_connection.h
@@ -96,6 +96,7 @@ class Connection : public EvbufCallbackBase<Connection> {
   std::string MapOfBulkStrings(const std::vector<std::string> &elems) const {
     return redis::MapOfBulkStrings(protocol_version_, elems);
   }
+  std::string Map(const std::map<std::string, std::string> &map) const { return redis::Map(protocol_version_, map); }
   template <typename T, std::enable_if_t<std::is_integral_v<T>, int> = 0>
   std::string HeaderOfAttribute(T len) const {
     return redis::HeaderOfAttribute(len);

diff --git a/src/server/redis_reply.cc b/src/server/redis_reply.cc
@@ -124,4 +124,13 @@ std::string MapOfBulkStrings(RESP ver, const std::vector<std::string> &elems) {
   return result;
 }
 
+std::string Map(RESP ver, const std::map<std::string, std::string> &map) {
+  std::string result = HeaderOfMap(ver, map.size());
+  for (const auto &pair : map) {
+    result += pair.first;
+    result += pair.second;
+  }
+  return result;
+}
+
 }  // namespace redis
diff --git a/src/server/redis_reply.h b/src/server/redis_reply.h
@@ -22,6 +22,7 @@
 
 #include <event2/buffer.h>
 
+#include <map>
 #include <string>
 #include <vector>
 
@@ -98,6 +99,7 @@ template <typename T, std::enable_if_t<std::is_integral_v<T>, int> = 0>
 std::string HeaderOfMap(RESP ver, T len) {
   return ver == RESP::v3 ? "%" + std::to_string(len) + CRLF : MultiLen(len * 2);
 }
+std::string Map(RESP ver, const std::map<std::string, std::string> &map);
 std::string MapOfBulkStrings(RESP ver, const std::vector<std::string> &elems);
 template <typename T, std::enable_if_t<std::is_integral_v<T>, int> = 0>
 std::string HeaderOfAttribute(T len) {

diff --git a/tests/gocase/unit/server/poll_updates_test.go b/tests/gocase/unit/server/poll_updates_test.go
@@ -37,30 +37,30 @@ type PollUpdatesResult struct {
 }
 
 func sliceToPollUpdatesResult(t *testing.T, slice []interface{}) *PollUpdatesResult {
-	require.Len(t, slice, 8)
+	itemCount := 6
+	require.Len(t, slice, itemCount)
+	var latestSeq, nextSeq int64
 
-	require.Equal(t, "latest_sequence", slice[0])
-	latestSeq, ok := slice[1].(int64)
-	require.True(t, ok)
-
-	require.Equal(t, "format", slice[2])
-	require.Equal(t, "RAW", slice[3])
-	require.Equal(t, "updates", slice[4])
 	updates := make([]string, 0)
-	if slice[5] != nil {
-		fields, ok := slice[5].([]interface{})
-		require.True(t, ok)
-		for _, field := range fields {
-			str, ok := field.(string)
-			require.True(t, ok)
-			updates = append(updates, str)
+	for i := 0; i < itemCount; i += 2 {
+		key := slice[i].(string)
+		switch key {
+		case "latest_sequence":
+			latestSeq = slice[i+1].(int64)
+		case "next_sequence":
+			nextSeq = slice[i+1].(int64)
+		case "updates":
+			fields := slice[i+1].([]interface{})
+			for _, field := range fields {
+				str, ok := field.(string)
+				require.True(t, ok)
+				updates = append(updates, str)
+			}
+		default:
+			require.Fail(t, fmt.Sprintf("unknown key: %s", key))
 		}
 	}
 
-	require.Equal(t, "next_sequence", slice[6])
-	nextSeq, ok := slice[7].(int64)
-	require.True(t, ok)
-
 	return &PollUpdatesResult{
 		LatestSeq: latestSeq,
 		NextSeq:   nextSeq,