feat:fixed Zpopmaxbugs etc. bugs (OpenAtomFoundation#2188)

* feat:unstable branch * feat:handle merge * feat:add lastsave cmd * feat:update lastsave cmd * feat:update lastsave cmd * feat:fixed lastsave cmd * feat:fixed server_test.go * feat:fixed server_test.go * feat:fixed server_test.go * feat:fixed server_test.go * feat:update modifies * feat:fixed zpopmax etc. bug * feat:add zpopmax ut * feat:fixed ut bugs * feat:fixed ut * feat:update code * feat:update ut
bigdaronlee163 · Dec 21, 2023 · b71964b · b71964b
1 parent 110d227
commit b71964b
Show file tree

Hide file tree

Showing 9 changed files with 57 additions and 43 deletions.
diff --git a/src/pika_command.cc b/src/pika_command.cc
@@ -32,7 +32,6 @@ extern PikaServer* g_pika_server;
 extern std::unique_ptr<PikaReplicaManager> g_pika_rm;
 extern std::unique_ptr<PikaCmdTableManager> g_pika_cmd_table_manager;
 
-
 void InitCmdTable(CmdTable* cmd_table) {
   // Admin
   ////Slaveof
@@ -420,7 +419,7 @@ void InitCmdTable(CmdTable* cmd_table) {
       std::make_unique<LPushCmd>(kCmdNameLPush, -3, kCmdFlagsWrite | kCmdFlagsSingleSlot | kCmdFlagsList | kCmdFlagsDoThroughDB | kCmdFlagsUpdateCache);
   cmd_table->insert(std::pair<std::string, std::unique_ptr<Cmd>>(kCmdNameLPush, std::move(lpushptr)));
   std::unique_ptr<Cmd> lpushxptr =
-
+      
       std::make_unique<LPushxCmd>(kCmdNameLPushx, -3, kCmdFlagsWrite | kCmdFlagsSingleSlot | kCmdFlagsList | kCmdFlagsDoThroughDB | kCmdFlagsUpdateCache);
   std::make_unique<LPushxCmd>(kCmdNameLPushx, 3, kCmdFlagsWrite | kCmdFlagsSingleSlot | kCmdFlagsList);
   cmd_table->insert(std::pair<std::string, std::unique_ptr<Cmd>>(kCmdNameLPushx, std::move(lpushxptr)));

diff --git a/src/pika_list.cc b/src/pika_list.cc
@@ -401,10 +401,11 @@ void LPopCmd::DoInitial() {
   }
   key_ = argv_[1];
   size_t argc = argv_.size();
-  size_t index = 2;
-  if (index < argc) {
-    if (pstd::string2int(argv_[index].data(), argv_[index].size(), &count_) == 0) {
-      res_.SetRes(CmdRes::kWrongNum, kCmdNameLPop);
+  if (argc > 3) {
+    res_.SetRes(CmdRes::kWrongNum, kCmdNameLPop);
+  } else if (argc == 3) {
+    if (pstd::string2int(argv_[2].data(), argv_[2].size(), &count_) == 0) {
+      res_.SetRes(CmdRes::kErrOther, kCmdNameLPop);
       return;
     }
     if (count_ < 0) {
@@ -728,10 +729,11 @@ void RPopCmd::DoInitial() {
     return;
   }
   key_ = argv_[1];
-  size_t index = 2;
-  if (index < argv_.size()) {
-    if (pstd::string2int(argv_[index].data(), argv_[index].size(), &count_) == 0) {
-      res_.SetRes(CmdRes::kWrongNum, kCmdNameRPop);
+  if (argv_.size() > 3) {
+    res_.SetRes(CmdRes::kWrongNum, kCmdNameRPop);
+  } else if (argv_.size() == 3) {
+    if (pstd::string2int(argv_[2].data(), argv_[2].size(), &count_) == 0) {
+      res_.SetRes(CmdRes::kErrOther, kCmdNameRPop);
       return;
     }
     if (count_ < 0) {

diff --git a/src/pika_set.cc b/src/pika_set.cc
@@ -46,17 +46,16 @@ void SAddCmd::DoUpdateCache(std::shared_ptr<Slot> slot) {
 
 void SPopCmd::DoInitial() {
   size_t argc = argv_.size();
-  size_t index = 2;
   if (!CheckArg(argc)) {
     res_.SetRes(CmdRes::kWrongNum, kCmdNameSPop);
     return;
   }
-
-  key_ = argv_[1];
   count_ = 1;
-
-  if (index < argc) {
-    if (pstd::string2int(argv_[index].data(), argv_[index].size(), &count_) == 0) {
+  key_ = argv_[1];
+  if (argc > 3) {
+    res_.SetRes(CmdRes::kWrongNum, kCmdNameSPop);
+  } else if (argc == 3) {
+    if (pstd::string2int(argv_[2].data(), argv_[2].size(), &count_) == 0) {
       res_.SetRes(CmdRes::kErrOther, kCmdNameSPop);
       return;
     }

diff --git a/src/pika_slot.cc b/src/pika_slot.cc
@@ -390,6 +390,7 @@ bool Slot::RunBgsaveEngine() {
     return false;
   }
   LOG(INFO) << slot_name_ << " create new backup finished.";
+
   return true;
 }
 

diff --git a/src/pika_zset.cc b/src/pika_zset.cc
@@ -1424,13 +1424,13 @@ void ZPopmaxCmd::DoInitial() {
     return;
   }
   key_ = argv_[1];
-  if (argv_.size() == 2) {
-    count_ = 1;
-    return;
-  }
-  if (pstd::string2int(argv_[2].data(), argv_[2].size(), static_cast<int64_t *>(&count_)) == 0) {
-    res_.SetRes(CmdRes::kInvalidInt);
-    return;
+  count_ = 1;
+  if (argv_.size() > 3) {
+    res_.SetRes(CmdRes::kWrongNum, kCmdNameZPopmax);
+  } else if (argv_.size() == 3) {
+    if (pstd::string2int(argv_[2].data(), argv_[2].size(), static_cast<int64_t *>(&count_)) == 0) {
+      res_.SetRes(CmdRes::kInvalidInt);
+    }
   }
 }
 
@@ -1458,13 +1458,13 @@ void ZPopminCmd::DoInitial() {
     return;
   }
   key_ = argv_[1];
-  if (argv_.size() == 2) {
-    count_ = 1;
-    return;
-  }
-  if (pstd::string2int(argv_[2].data(), argv_[2].size(), static_cast<int64_t *>(&count_)) == 0) {
-    res_.SetRes(CmdRes::kInvalidInt);
-    return;
+  count_ = 1;
+  if (argv_.size() > 3) {
+    res_.SetRes(CmdRes::kWrongNum, kCmdNameZPopmin);
+  } else if (argv_.size() == 3) {
+    if (pstd::string2int(argv_[2].data(), argv_[2].size(), static_cast<int64_t *>(&count_)) == 0) {
+      res_.SetRes(CmdRes::kInvalidInt);
+    }
   }
 }
 

diff --git a/tests/integration/list_test.go b/tests/integration/list_test.go
@@ -915,6 +915,9 @@ var _ = Describe("List Commands", func() {
 			lRange := client.LRange(ctx, "list", 0, -1)
 			Expect(lRange.Err()).NotTo(HaveOccurred())
 			Expect(lRange.Val()).To(Equal([]string{"two", "three"}))
+
+            err := client.Do(ctx, "LPOP", "list", 1, 2).Err()
+            Expect(err).To(MatchError(ContainSubstring("ERR wrong number of arguments for 'lpop' command")))
 		})
 
 		It("should LPopCount", func() {
@@ -1157,6 +1160,9 @@ var _ = Describe("List Commands", func() {
 			lRange := client.LRange(ctx, "list", 0, -1)
 			Expect(lRange.Err()).NotTo(HaveOccurred())
 			Expect(lRange.Val()).To(Equal([]string{"one", "two"}))
+
+			err := client.Do(ctx, "RPOP", "list", 1, 2).Err()
+            Expect(err).To(MatchError(ContainSubstring("ERR wrong number of arguments for 'rpop' command")))
 		})
 
 		It("should RPopCount", func() {

diff --git a/tests/integration/server_test.go b/tests/integration/server_test.go
@@ -393,19 +393,19 @@ var _ = Describe("Server", func() {
 		It("should LastSave", func() {
 			lastSave := client.LastSave(ctx)
 			Expect(lastSave.Err()).NotTo(HaveOccurred())
-// 			Expect(lastSave.Val()).To(Equal(int64(0)))
-
-            bgSaveTime1 := time.Now().Unix()
-            bgSave, err := client.BgSave(ctx).Result()
-            Expect(err).NotTo(HaveOccurred())
-            Expect(bgSave).To(ContainSubstring("Background saving started"))
-            time.Sleep(1 * time.Second)
-            bgSaveTime2 := time.Now().Unix()
-
-            lastSave = client.LastSave(ctx)
-            Expect(lastSave.Err()).NotTo(HaveOccurred())
-            Expect(lastSave.Val()).To(BeNumerically(">=", bgSaveTime1))
-            Expect(lastSave.Val()).To(BeNumerically("<=", bgSaveTime2))
+			//Expect(lastSave.Val()).To(Equal(int64(0)))
+
+			bgSaveTime1 := time.Now().Unix()
+			bgSave, err := client.BgSave(ctx).Result()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(bgSave).To(ContainSubstring("Background saving started"))
+			time.Sleep(1 * time.Second)
+			bgSaveTime2 := time.Now().Unix()
+
+			lastSave = client.LastSave(ctx)
+			Expect(lastSave.Err()).NotTo(HaveOccurred())
+			Expect(lastSave.Val()).To(BeNumerically(">=", bgSaveTime1))
+			Expect(lastSave.Val()).To(BeNumerically("<=", bgSaveTime2))
 
 		})
 

diff --git a/tests/integration/set_test.go b/tests/integration/set_test.go
@@ -275,6 +275,9 @@ var _ = Describe("Set Commands", func() {
 			sMembers := client.SMembers(ctx, "set")
 			Expect(sMembers.Err()).NotTo(HaveOccurred())
 			Expect(sMembers.Val()).To(HaveLen(3))
+
+            err := client.Do(ctx, "SPOP", "set", 1, 2).Err()
+            Expect(err).To(MatchError(ContainSubstring("ERR wrong number of arguments for 'spop' command")))
 		})
 
 		It("should SPopN", func() {

diff --git a/tests/integration/zset_test.go b/tests/integration/zset_test.go
@@ -1016,6 +1016,8 @@ var _ = Describe("Zset Commands", func() {
 			Score:  1,
 			Member: "one",
 		}}))
+		err = client.Do(ctx, "ZPOPMAX", "zset", 1, 2).Err()
+		Expect(err).To(MatchError(ContainSubstring("ERR wrong number of arguments for 'zpopmax' command")))
 	})
 
 	It("should ZPopMin", func() {
@@ -1083,6 +1085,8 @@ var _ = Describe("Zset Commands", func() {
 			Score:  3,
 			Member: "three",
 		}}))
+		err = client.Do(ctx, "ZPOPMIN", "zset", 1, 2).Err()
+        Expect(err).To(MatchError(ContainSubstring("ERR wrong number of arguments for 'zpopmin' command")))
 	})
 
 	It("should ZRange", func() {