Chore: refine code based on PR feedback

openraft/src/engine/leader_log_ids.rs

@@ -1,53 +1,54 @@
 use std::fmt;
+use std::ops::RangeInclusive;
 
 use crate::type_config::alias::LogIdOf;
 use crate::RaftTypeConfig;
 
 /// The first and the last log id belonging to a Leader.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub(crate) struct LeaderLogIds<C: RaftTypeConfig> {
-    first_last: Option<(LogIdOf<C>, LogIdOf<C>)>,
+    log_id_range: Option<RangeInclusive<LogIdOf<C>>>,
 }
 
 impl<C> fmt::Display for LeaderLogIds<C>
 where C: RaftTypeConfig
 {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match &self.first_last {
+        match &self.log_id_range {
             None => write!(f, "None"),
-            Some((first, last)) => write!(f, "({}, {})", first, last),
+            Some(rng) => write!(f, "({}, {})", rng.start(), rng.end()),
         }
     }
 }
 
 impl<C> LeaderLogIds<C>
 where C: RaftTypeConfig
 {
-    pub(crate) fn new(log_ids: Option<(LogIdOf<C>, LogIdOf<C>)>) -> Self {
-        Self { first_last: log_ids }
+    pub(crate) fn new(log_id_range: Option<RangeInclusive<LogIdOf<C>>>) -> Self {
+        Self { log_id_range }
     }
 
     /// Used only in tests
     #[allow(dead_code)]
-    pub(crate) fn new1(log_id: LogIdOf<C>) -> Self {
+    pub(crate) fn new_single(log_id: LogIdOf<C>) -> Self {
         Self {
-            first_last: Some((log_id.clone(), log_id)),
+            log_id_range: Some(log_id.clone()..=log_id),
         }
     }
 
     /// Used only in tests
     #[allow(dead_code)]
-    pub(crate) fn new2(first: LogIdOf<C>, last: LogIdOf<C>) -> Self {
+    pub(crate) fn new_start_end(first: LogIdOf<C>, last: LogIdOf<C>) -> Self {
         Self {
-            first_last: Some((first, last)),
+            log_id_range: Some(first..=last),
         }
     }
 
     pub(crate) fn first(&self) -> Option<&LogIdOf<C>> {
-        self.first_last.as_ref().map(|x| &x.0)
+        self.log_id_range.as_ref().map(|x| x.start())
     }
 
     pub(crate) fn last(&self) -> Option<&LogIdOf<C>> {
-        self.first_last.as_ref().map(|x| &x.1)
+        self.log_id_range.as_ref().map(|x| x.end())
     }
 }

openraft/src/engine/log_id_list.rs

@@ -1,3 +1,5 @@
+use std::ops::RangeInclusive;
+
 use crate::engine::leader_log_ids::LeaderLogIds;
 use crate::log_id::RaftLogId;
 use crate::storage::RaftLogReaderExt;
@@ -47,13 +49,15 @@ where C: RaftTypeConfig
     /// A-------C-------C : find(A,C)
     /// ```
     pub(crate) async fn get_key_log_ids<LR>(
-        first: LogId<C::NodeId>,
-        last: LogId<C::NodeId>,
+        range: RangeInclusive<LogId<C::NodeId>>,
         sto: &mut LR,
     ) -> Result<Vec<LogIdOf<C>>, StorageError<C>>
     where
         LR: RaftLogReader<C> + ?Sized,
     {
+        let first = range.start().clone();
+        let last = range.end().clone();
+
         let mut res: Vec<LogIdOf<C>> = vec![];
 
         // Recursion stack
@@ -312,15 +316,15 @@ where C: RaftTypeConfig
         let l = ks.len();
         if l < 2 {
             let last = self.last();
-            return LeaderLogIds::new(last.map(|x| (x.clone(), x.clone())));
+            return LeaderLogIds::new(last.map(|x| x.clone()..=x.clone()));
         }
 
         // There are at most two(adjacent) key log ids with the same leader_id
         if ks[l - 1].leader_id() == ks[l - 2].leader_id() {
-            LeaderLogIds::new(Some((ks[l - 2].clone(), ks[l - 1].clone())))
+            LeaderLogIds::new_start_end(ks[l - 2].clone(), ks[l - 1].clone())
         } else {
             let last = self.last().cloned().unwrap();
-            LeaderLogIds::new(Some((last.clone(), last)))
+            LeaderLogIds::new_single(last)
         }
     }
 }

openraft/src/engine/tests/log_id_list_test.rs

@@ -362,23 +362,23 @@ fn test_log_id_list_by_last_leader() -> anyhow::Result<()> {
 
     // len == 1
     let ids = LogIdList::<UTConfig>::new([log_id(1, 1, 1)]);
-    assert_eq!(LeaderLogIds::new1(log_id(1, 1, 1)), ids.by_last_leader());
+    assert_eq!(LeaderLogIds::new_single(log_id(1, 1, 1)), ids.by_last_leader());
 
     // len == 2, the last leader has only one log
     let ids = LogIdList::<UTConfig>::new([log_id(1, 1, 1), log_id(3, 1, 3)]);
-    assert_eq!(LeaderLogIds::new1(log_id(3, 1, 3)), ids.by_last_leader());
+    assert_eq!(LeaderLogIds::new_single(log_id(3, 1, 3)), ids.by_last_leader());
 
     // len == 2, the last leader has two logs
     let ids = LogIdList::<UTConfig>::new([log_id(1, 1, 1), log_id(1, 1, 3)]);
     assert_eq!(
-        LeaderLogIds::new2(log_id(1, 1, 1), log_id(1, 1, 3)),
+        LeaderLogIds::new_start_end(log_id(1, 1, 1), log_id(1, 1, 3)),
         ids.by_last_leader()
     );
 
     // len > 2, the last leader has only more than one logs
     let ids = LogIdList::<UTConfig>::new([log_id(1, 1, 1), log_id(7, 1, 8), log_id(7, 1, 10)]);
     assert_eq!(
-        LeaderLogIds::new2(log_id(7, 1, 8), log_id(7, 1, 10)),
+        LeaderLogIds::new_start_end(log_id(7, 1, 8), log_id(7, 1, 10)),
         ids.by_last_leader()
     );
 

openraft/src/proposer/candidate.rs

@@ -116,7 +116,7 @@ where
         //       Thus the first() is ignored.
         //       But we should not fake the first() there.
         let last = self.last_log_id();
-        let last_leader_log_ids = LeaderLogIds::new(last.map(|last| (last.clone(), last.clone())));
+        let last_leader_log_ids = LeaderLogIds::new(last.map(|last| last.clone()..=last.clone()));
 
         Leader::new(vote, self.quorum_set.clone(), self.learner_ids, last_leader_log_ids)
     }

openraft/src/proposer/leader.rs

@@ -243,7 +243,7 @@ mod tests {
                 vote,
                 vec![1, 2, 3],
                 vec![],
-                LeaderLogIds::new2(log_id(1, 2, 1), log_id(1, 2, 3)),
+                LeaderLogIds::new_start_end(log_id(1, 2, 1), log_id(1, 2, 3)),
             );
 
             assert_eq!(leader.noop_log_id(), Some(&log_id(2, 2, 4)));
@@ -257,7 +257,7 @@ mod tests {
                 vote,
                 vec![1, 2, 3],
                 vec![],
-                LeaderLogIds::new2(log_id(1, 2, 1), log_id(1, 2, 3)),
+                LeaderLogIds::new_start_end(log_id(1, 2, 1), log_id(1, 2, 3)),
             );
 
             assert_eq!(leader.noop_log_id(), Some(&log_id(1, 2, 1)));
@@ -267,7 +267,8 @@ mod tests {
         tracing::info!("--- vote equals last log id, reuse noop_log_id, last_leader_log_id.len()==1");
         {
             let vote = Vote::new(1, 2).into_committed();
-            let leader = Leader::<UTConfig, _>::new(vote, vec![1, 2, 3], vec![], LeaderLogIds::new1(log_id(1, 2, 3)));
+            let leader =
+                Leader::<UTConfig, _>::new(vote, vec![1, 2, 3], vec![], LeaderLogIds::new_single(log_id(1, 2, 3)));
 
             assert_eq!(leader.noop_log_id(), Some(&log_id(1, 2, 3)));
             assert_eq!(leader.last_log_id(), Some(&log_id(1, 2, 3)));
@@ -286,7 +287,8 @@ mod tests {
     #[test]
     fn test_leader_established() {
         let vote = Vote::new(2, 2).into_committed();
-        let mut leader = Leader::<UTConfig, _>::new(vote, vec![1, 2, 3], vec![], LeaderLogIds::new1(log_id(1, 2, 3)));
+        let mut leader =
+            Leader::<UTConfig, _>::new(vote, vec![1, 2, 3], vec![], LeaderLogIds::new_single(log_id(1, 2, 3)));
 
         let mut entries = vec![Entry::<UTConfig>::new_blank(log_id(5, 5, 2))];
         leader.assign_log_ids(&mut entries);
@@ -314,7 +316,8 @@ mod tests {
     #[test]
     fn test_no_entries_provided() {
         let vote = Vote::new(2, 2).into_committed();
-        let mut leading = Leader::<UTConfig, _>::new(vote, vec![1, 2, 3], vec![], LeaderLogIds::new1(log_id(1, 1, 8)));
+        let mut leading =
+            Leader::<UTConfig, _>::new(vote, vec![1, 2, 3], vec![], LeaderLogIds::new_single(log_id(1, 1, 8)));
 
         let mut entries: Vec<Entry<UTConfig>> = vec![];
         leading.assign_log_ids(&mut entries);
@@ -324,7 +327,8 @@ mod tests {
     #[test]
     fn test_multiple_entries() {
         let vote = Vote::new(2, 2).into_committed();
-        let mut leading = Leader::<UTConfig, _>::new(vote, vec![1, 2, 3], [], LeaderLogIds::new1(log_id(1, 1, 8)));
+        let mut leading =
+            Leader::<UTConfig, _>::new(vote, vec![1, 2, 3], [], LeaderLogIds::new_single(log_id(1, 1, 8)));
 
         let mut entries: Vec<Entry<UTConfig>> = vec![blank_ent(1, 1, 1), blank_ent(1, 1, 1), blank_ent(1, 1, 1)];
 

openraft/src/storage/helper.rs

@@ -335,7 +335,7 @@ where
 
         let first = log_reader.get_log_id(purged.next_index()).await?;
 
-        let mut log_ids = log_reader.get_key_log_ids(first, last).await?;
+        let mut log_ids = log_reader.get_key_log_ids(first..=last).await?;
 
         if !log_ids.is_empty() {
             if let Some(purged) = purged {

openraft/src/storage/v2/raft_log_reader.rs

@@ -1,5 +1,6 @@
 use std::fmt::Debug;
 use std::ops::RangeBounds;
+use std::ops::RangeInclusive;
 
 use openraft_macros::add_async_trait;
 use openraft_macros::since;
@@ -95,8 +96,7 @@ where C: RaftTypeConfig
     ///
     /// # Arguments
     ///
-    /// - `first`: the first log id to return.
-    /// - `last`: the last log id to return.
+    /// - `range`: range of the log id to return, inclusive. Such as `(1, 10)..=(2, 20)`.
     ///
     /// # Returns
     ///
@@ -106,9 +106,8 @@ where C: RaftTypeConfig
     #[since(version = "0.10.0")]
     async fn get_key_log_ids(
         &mut self,
-        first: LogId<C::NodeId>,
-        last: LogId<C::NodeId>,
+        range: RangeInclusive<LogId<C::NodeId>>,
     ) -> Result<Vec<LogId<C::NodeId>>, StorageError<C>> {
-        LogIdList::get_key_log_ids(first, last, self).await
+        LogIdList::get_key_log_ids(range, self).await
     }
 }