update parseSlotRanges

Signed-off-by: clundro <859287553@qq.com>

src/cluster/cluster.cc

@@ -94,8 +94,7 @@ Status Cluster::SetNodeId(const std::string &node_id) {
 }
 
 // Set the slots to the node if new version is current version +1. It is useful
-// when we scale cluster avoid too many big messages. By th way,there are 16384 slots
-// in our design.
+// when we scale cluster avoid too many big messages.
 // The reason why the new version MUST be +1 of current version is that,
 // the command changes topology based on specific topology (also means specific
 // version), we must guarantee current topology is exactly expected, otherwise,
@@ -124,7 +123,7 @@ Status Cluster::SetSlot(const std::string &slots_str, const std::string &node_id
   }
 
   std::vector<std::pair<int, int>> slots;
-  Status s = parseSlotRange(slots_str, slots);
+  Status s = parseSlotRanges(slots_str, slots);
   if (!s.OK()) {
     return s;
   }
@@ -650,48 +649,50 @@ Status Cluster::LoadClusterNodes(const std::string &file_path) {
 }
 
 // TODO: maybe it needs to use a more precise error type to represent `NotOk`.
-Status Cluster::parseSlotRange(const std::string &slots_str, std::vector<std::pair<int, int>> &slots) {
+Status Cluster::parseSlotRanges(const std::string &slots_str, std::vector<std::pair<int, int>> &slots) {
   if (slots_str.empty()) {
-    return {Status::NotOK, "Don't use empty slots."};
+    return {Status::NotOK, "No slots to parse."};
   }
   std::vector<std::string> slot_ranges = util::Split(slots_str, " ");
 
   if (slot_ranges.empty()) {
     return {Status::NotOK, fmt::format("Invalid slots: {}. Please use ' ' to space slots.", slots_str)};
   }
 
-  auto is_number = [&](const std::string &s) {
-#if __cplusplus >= 202002L
-    return std::ranges::all_of(s.begin(), s.end(), [](char c) { return isdigit(c) != 0; });
-#else
-    for (char c : s) {
-      if (isdigit(c) == 0) {
-        return false;
-      }
-    }
-    return true;
-#endif
-  };
-
+  auto valid_range = NumericRange<int>{0, kClusterSlots - 1};
   // Parse all slots(include slot ranges)
-  for (const auto &slot_range : slot_ranges) {
-    if (is_number(slot_range)) {
-      int s_start = stoi(slot_range);
-      assert(IsValidSlot(s_start));
-      slots.emplace_back(std::make_pair(s_start, s_start));
+  for (auto &slot_range : slot_ranges) {
+    if (slot_range.find('-') == std::string::npos) {
+      auto parse_result = ParseInt<int>(slot_range, valid_range, 10);
+      if (!parse_result) {
+        return {Status::NotOK, errInvalidSlotID};
+      }
+      int slot_start = *parse_result;
+      slots.emplace_back(std::make_pair(slot_start, slot_start));
       continue;
     }
 
     // parse slot range: "int1-int2" (satisfy: int1 <= int2 )
-    assert(slot_range.back() != '-');
+    if (slot_range.back() == '-') {
+      return {Status::NotOK,
+              fmt::format("Invalid slot range: {}. The character '-' can't appear in the last position.", slot_range)};
+    }
     std::vector<std::string> fields = util::Split(slot_range, "-");
-    assert(fields.size() == 2);
-    if (is_number(fields[0]) && is_number(fields[1])) {
-      int s_start = stoi(fields[0]), s_end = stoi(fields[1]);
-      assert((s_start <= s_end) && IsValidSlot(s_end));
-      slots.emplace_back(std::make_pair(s_start, s_end));
+    if (fields.size() != 2) {
+      return {Status::NotOK,
+              fmt::format("Invalid slot range: {}. The slot range should be of the form `int1-int2`.", slot_range)};
+    }
+    auto parse_start = ParseInt<int>(fields[0], valid_range, 10);
+    auto parse_end = ParseInt<int>(fields[1], valid_range, 10);
+    if (!parse_start || !parse_end || *parse_start > *parse_end) {
+      return {Status::NotOK,
+              fmt::format(
+                  "Invalid slot range: {}. The slot range `int1-int2` needs to satisfy the condition (int1 <= int2).",
+                  slot_range)};
     }
-    return {Status::NotOK, fmt::format("Invalid slot range: {}", slot_range)};
+    int slot_start = *parse_start;
+    int slot_end = *parse_end;
+    slots.emplace_back(std::make_pair(slot_start, slot_end));
   }
 
   return Status::OK();

src/cluster/cluster.h

@@ -104,7 +104,7 @@ class Cluster {
   std::string genNodesInfo();
   void updateSlotsInfo();
   SlotInfo genSlotNodeInfo(int start, int end, const std::shared_ptr<ClusterNode> &n);
-  static Status parseSlotRange(const std::string&slots_str,std::vector<std::pair<int,int>> &slots);
+  static Status parseSlotRanges(const std::string&slots_str,std::vector<std::pair<int,int>> &slots);
   static Status parseClusterNodes(const std::string &nodes_str, ClusterNodes *nodes,
                                   std::unordered_map<int, std::string> *slots_nodes);
   Server *svr_;