Refactor: move MembershipState to raft_state/

Because `MembershipState` is a sub state of `RaftState`.

And also move ChangeHandler to raft_state/membership_state; Move tests out of mod file.

openraft/src/lib.rs

@@ -89,7 +89,6 @@ pub use crate::entry::Entry;
 pub use crate::entry::EntryPayload;
 pub use crate::membership::EffectiveMembership;
 pub use crate::membership::Membership;
-pub use crate::membership::MembershipState;
 pub use crate::membership::StoredMembership;
 pub use crate::metrics::RaftMetrics;
 pub use crate::network::RPCTypes;
@@ -101,6 +100,7 @@ pub use crate::node::Node;
 pub use crate::node::NodeId;
 pub use crate::raft::Raft;
 pub use crate::raft::RaftTypeConfig;
+pub use crate::raft_state::MembershipState;
 pub use crate::raft_state::RaftState;
 pub use crate::raft_types::LogId;
 pub use crate::raft_types::LogIdOptionExt;

openraft/src/membership/mod.rs

@@ -1,21 +1,16 @@
-mod change_handler;
 mod effective_membership;
 mod into_nodes;
 #[allow(clippy::module_inception)] mod membership;
-mod membership_state;
 mod stored_membership;
 
 #[cfg(feature = "bench")]
 #[cfg(test)]
 mod bench;
 
 #[cfg(test)] mod effective_membership_test;
-#[cfg(test)] mod membership_state_test;
 #[cfg(test)] mod membership_test;
 
-pub(crate) use change_handler::ChangeHandler;
 pub use effective_membership::EffectiveMembership;
 pub use into_nodes::IntoNodes;
 pub use membership::Membership;
-pub use membership_state::MembershipState;
 pub use stored_membership::StoredMembership;

openraft/src/raft_state/membership_state/change_handler.rs

@@ -0,0 +1,65 @@
+use crate::error::ChangeMembershipError;
+use crate::error::InProgress;
+use crate::ChangeMembers;
+use crate::Membership;
+use crate::MembershipState;
+use crate::Node;
+use crate::NodeId;
+
+/// This struct handles change-membership requests, validating them and applying the changes if
+/// the necessary conditions are met. It operates at the `Engine` and `RaftState` level, and
+/// serves as the outermost API for a consensus engine.
+pub(crate) struct ChangeHandler<'m, NID, N>
+where
+    NID: NodeId,
+    N: Node,
+{
+    pub(crate) state: &'m MembershipState<NID, N>,
+}
+
+impl<'m, NID, N> ChangeHandler<'m, NID, N>
+where
+    NID: NodeId,
+    N: Node,
+{
+    /// Builds a new membership configuration by applying changes to the current configuration.
+    ///
+    /// * `changes`: The changes to apply to the current membership configuration.
+    /// * `retain` specifies whether to retain the removed voters as a learners, i.e., nodes that
+    ///   continue to receive log replication from the leader.
+    ///
+    /// A Result containing the new membership configuration if the operation succeeds, or a
+    /// `ChangeMembershipError` if an error occurs.
+    ///
+    /// This function ensures that the cluster will have at least one voter in the new membership
+    /// configuration.
+    pub(crate) fn apply(
+        &self,
+        change: ChangeMembers<NID, N>,
+        retain: bool,
+    ) -> Result<Membership<NID, N>, ChangeMembershipError<NID>> {
+        self.ensure_committed()?;
+
+        let new_membership = self.state.effective().membership().clone().change(change, retain)?;
+        Ok(new_membership)
+    }
+
+    /// Ensures that the latest membership has been committed.
+    ///
+    /// Returns Ok if the last membership is committed, or an InProgress error
+    /// otherwise, to indicate a change-membership request should be rejected.
+    pub(crate) fn ensure_committed(&self) -> Result<(), InProgress<NID>> {
+        let effective = self.state.effective();
+        let committed = self.state.committed();
+
+        if effective.log_id() == committed.log_id() {
+            // Ok: last membership(effective) is committed
+            Ok(())
+        } else {
+            Err(InProgress {
+                committed: *committed.log_id(),
+                membership_log_id: *effective.log_id(),
+            })
+        }
+    }
+}

openraft/src/raft_state/membership_state/change_handler_test.rs

@@ -0,0 +1,95 @@
+use std::sync::Arc;
+
+use maplit::btreemap;
+use maplit::btreeset;
+
+use crate::error::ChangeMembershipError;
+use crate::error::EmptyMembership;
+use crate::error::InProgress;
+use crate::error::LearnerNotFound;
+use crate::testing::log_id;
+use crate::ChangeMembers;
+use crate::EffectiveMembership;
+use crate::Membership;
+use crate::MembershipState;
+
+/// Create an Arc<EffectiveMembership>
+fn effmem(term: u64, index: u64, m: Membership<u64, ()>) -> Arc<EffectiveMembership<u64, ()>> {
+    let lid = Some(log_id(term, index));
+    Arc::new(EffectiveMembership::new(lid, m))
+}
+
+fn m1() -> Membership<u64, ()> {
+    Membership::new(vec![btreeset! {1}], None)
+}
+
+fn m12() -> Membership<u64, ()> {
+    Membership::new(vec![btreeset! {1,2}], None)
+}
+
+fn m123_345() -> Membership<u64, ()> {
+    Membership::new(vec![btreeset! {1,2,3}, btreeset! {3,4,5}], None)
+}
+
+#[test]
+fn test_apply_not_committed() -> anyhow::Result<()> {
+    let new = || MembershipState::new(effmem(2, 2, m1()), effmem(3, 4, m123_345()));
+    let res = new().change_handler().apply(ChangeMembers::AddVoterIds(btreeset! {1}), false);
+
+    assert_eq!(
+        Err(ChangeMembershipError::InProgress(InProgress {
+            committed: Some(log_id(2, 2)),
+            membership_log_id: Some(log_id(3, 4))
+        })),
+        res
+    );
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_empty_voters() -> anyhow::Result<()> {
+    let new = || MembershipState::new(effmem(3, 4, m1()), effmem(3, 4, m1()));
+    let res = new().change_handler().apply(ChangeMembers::RemoveVoters(btreeset! {1}), false);
+
+    assert_eq!(Err(ChangeMembershipError::EmptyMembership(EmptyMembership {})), res);
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_learner_not_found() -> anyhow::Result<()> {
+    let new = || MembershipState::new(effmem(3, 4, m1()), effmem(3, 4, m1()));
+    let res = new().change_handler().apply(ChangeMembers::AddVoterIds(btreeset! {2}), false);
+
+    assert_eq!(
+        Err(ChangeMembershipError::LearnerNotFound(LearnerNotFound { node_id: 2 })),
+        res
+    );
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_retain_learner() -> anyhow::Result<()> {
+    let new = || MembershipState::new(effmem(3, 4, m12()), effmem(3, 4, m123_345()));
+
+    // Do not leave removed voters as learner
+    let res = new().change_handler().apply(ChangeMembers::RemoveVoters(btreeset! {1,2}), false);
+    assert_eq!(
+        Ok(Membership::new(vec![btreeset! {3,4,5}], btreemap! {3=>(),4=>(),5=>()})),
+        res
+    );
+
+    // Leave removed voters as learner
+    let res = new().change_handler().apply(ChangeMembers::RemoveVoters(btreeset! {1,2}), true);
+    assert_eq!(
+        Ok(Membership::new(
+            vec![btreeset! {3,4,5}],
+            btreemap! {1=>(),2=>(),3=>(),4=>(),5=>()}
+        )),
+        res
+    );
+
+    Ok(())
+}

openraft/src/membership/membership_state.rs → openraft/src/raft_state/membership_state/mod.rs

@@ -2,15 +2,20 @@ use std::error::Error;
 use std::sync::Arc;
 
 use crate::less_equal;
-use crate::membership::ChangeHandler;
-use crate::node::Node;
 use crate::validate::Validate;
 use crate::EffectiveMembership;
 use crate::LogId;
 use crate::LogIdOptionExt;
 use crate::MessageSummary;
+use crate::Node;
 use crate::NodeId;
 
+mod change_handler;
+#[cfg(test)] mod change_handler_test;
+#[cfg(test)] mod membership_state_test;
+
+pub(crate) use change_handler::ChangeHandler;
+
 /// The state of membership configs a raft node needs to know.
 ///
 /// A raft node needs to store at most 2 membership config log: