Feature: add err: NotAMembershipEntry, NotInMembers

- Change: fix error usage for `initialize()`: new error to return:
  NotAMembershipEntry and NotInMembers;

  MissingNodeInfo should not be returned when the node to initialize is
  not a member.

- Refactor: move engine to dir engine/

- Test: add engine test for `Engine::initialize()`

- part of #292

openraft/src/engine.rs → openraft/src/engine/engine.rs

@@ -6,8 +6,9 @@ use maplit::btreeset;
 
 use crate::entry::RaftEntry;
 use crate::error::InitializeError;
-use crate::error::MissingNodeInfo;
+use crate::error::NotAMembershipEntry;
 use crate::error::NotAllowed;
+use crate::error::NotInMembers;
 use crate::storage::InitialState;
 use crate::EffectiveMembership;
 use crate::LogId;
@@ -140,7 +141,7 @@ impl<NID: NodeId> Engine<NID> {
         if let Some(m) = entry.get_membership() {
             self.check_members_contain_me(m)?;
         } else {
-            panic!("Initialization log has to be a membership config entry");
+            Err(NotAMembershipEntry {})?;
         }
         self.try_update_membership(entry);
 
@@ -245,11 +246,11 @@ impl<NID: NodeId> Engine<NID> {
     }
 
     /// When initialize, the node that accept initialize request has to be a member of the initial config.
-    fn check_members_contain_me(&self, m: &Membership<NID>) -> Result<(), MissingNodeInfo<NID>> {
+    fn check_members_contain_me(&self, m: &Membership<NID>) -> Result<(), NotInMembers<NID>> {
         if !m.is_member(&self.id) {
-            let e = MissingNodeInfo {
+            let e = NotInMembers {
                 node_id: self.id,
-                reason: "target should be a member".to_string(),
+                membership: m.clone(),
             };
             Err(e)
         } else {

openraft/src/engine/initialize_test.rs

@@ -0,0 +1,120 @@
+use maplit::btreeset;
+
+use crate::engine::Command;
+use crate::engine::Engine;
+use crate::entry::EntryRef;
+use crate::error::InitializeError;
+use crate::error::NotAMembershipEntry;
+use crate::error::NotAllowed;
+use crate::error::NotInMembers;
+use crate::EntryPayload;
+use crate::LeaderId;
+use crate::LogId;
+use crate::Membership;
+use crate::MetricsChangeFlags;
+use crate::Vote;
+
+#[derive(Clone, serde::Serialize, serde::Deserialize)]
+pub(crate) struct Req {}
+#[derive(Clone, serde::Serialize, serde::Deserialize)]
+pub(crate) struct Resp {}
+
+crate::declare_raft_types!(
+   pub(crate) Config: D = Req, R = Resp, NodeId = u64
+);
+
+#[test]
+fn test_initialize() -> anyhow::Result<()> {
+    let eng = Engine::<u64>::default;
+
+    let log_id0 = LogId {
+        leader_id: LeaderId::new(0, 0),
+        index: 0,
+    };
+    let vote0 = Vote::new(0, 0);
+
+    let m12 = || Membership::<u64>::new(vec![btreeset! {1,2}], None);
+    let payload = EntryPayload::<Config>::Membership(m12());
+    let mut entries = [EntryRef::new(&payload)];
+
+    tracing::info!("--- ok");
+    {
+        let mut eng = eng();
+        eng.id = 1;
+
+        eng.initialize(&mut entries)?;
+        assert_eq!(Some(log_id0), eng.state.last_log_id);
+        assert_eq!(
+            MetricsChangeFlags {
+                leader: false,
+                other_metrics: true
+            },
+            eng.metrics_flags
+        );
+        assert_eq!(m12(), eng.state.effective_membership.membership);
+        assert_eq!(
+            vec![
+                Command::AppendInputEntries { range: 0..1 },
+                Command::UpdateMembership { membership: m12() },
+                Command::MoveInputCursorBy { n: 1 }
+            ],
+            eng.commands
+        );
+    }
+
+    tracing::info!("--- not allowed because of last_log_id");
+    {
+        let mut eng = eng();
+        eng.state.last_log_id = Some(log_id0);
+
+        assert_eq!(
+            Err(InitializeError::NotAllowed(NotAllowed {
+                last_log_id: Some(log_id0),
+                vote: vote0,
+            })),
+            eng.initialize(&mut entries)
+        );
+    }
+
+    tracing::info!("--- not allowed because of vote");
+    {
+        let mut eng = eng();
+        eng.state.vote = Vote::new(0, 1);
+
+        assert_eq!(
+            Err(InitializeError::NotAllowed(NotAllowed {
+                last_log_id: None,
+                vote: Vote::new(0, 1),
+            })),
+            eng.initialize(&mut entries)
+        );
+    }
+
+    tracing::info!("--- node id 0 is not in membership");
+    {
+        let mut eng = eng();
+
+        assert_eq!(
+            Err(InitializeError::NotInMembers(NotInMembers {
+                node_id: 0,
+                membership: m12()
+            })),
+            eng.initialize(&mut entries)
+        );
+    }
+
+    tracing::info!("--- log entry is not a membership entry");
+    {
+        let mut eng = eng();
+
+        let payload = EntryPayload::<Config>::Blank;
+        let mut entries = [EntryRef::new(&payload)];
+
+        assert_eq!(
+            Err(InitializeError::NotAMembershipEntry(NotAMembershipEntry {})),
+            eng.initialize(&mut entries)
+        );
+    }
+
+    Ok(())
+}

openraft/src/engine/mod.rs

@@ -0,0 +1,8 @@
+#[allow(clippy::module_inception)]
+mod engine;
+
+#[cfg(test)]
+mod initialize_test;
+
+pub(crate) use engine::Command;
+pub(crate) use engine::Engine;

openraft/src/error.rs

@@ -11,6 +11,7 @@ use serde::Serialize;
 
 use crate::raft_types::SnapshotSegmentId;
 use crate::LogId;
+use crate::Membership;
 use crate::Node;
 use crate::NodeId;
 use crate::RPCTypes;
@@ -161,6 +162,12 @@ pub enum InitializeError<NID: NodeId> {
     #[error(transparent)]
     NotAllowed(#[from] NotAllowed<NID>),
 
+    #[error(transparent)]
+    NotInMembers(#[from] NotInMembers<NID>),
+
+    #[error(transparent)]
+    NotAMembershipEntry(#[from] NotAMembershipEntry),
+
     #[error(transparent)]
     MissingNodeInfo(#[from] MissingNodeInfo<NID>),
 
@@ -386,6 +393,19 @@ pub struct MissingNodeInfo<NID: NodeId> {
     pub reason: String,
 }
 
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, thiserror::Error)]
+#[serde(bound = "")]
+#[error("node {node_id} has to be a member. membership:{membership:?}")]
+pub struct NotInMembers<NID: NodeId> {
+    pub node_id: NID,
+    pub membership: Membership<NID>,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, thiserror::Error)]
+#[serde(bound = "")]
+#[error("initializing log entry has to be a membership config entry")]
+pub struct NotAMembershipEntry {}
+
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, thiserror::Error)]
 #[error("new membership can not be empty")]
 pub struct EmptyMembership {}

openraft/src/raft_types.rs

@@ -145,7 +145,7 @@ pub enum Update<T> {
 }
 
 /// Describes the need to update some aspect of the metrics.
-#[derive(Debug, Clone, Default)]
+#[derive(Debug, Clone, Default, PartialEq, Eq)]
 pub(crate) struct MetricsChangeFlags {
     pub leader: bool,
     // TODO: split other_metrics into data metrics and cluster metrics

openraft/tests/initialize/t20_initialization.rs

@@ -3,8 +3,8 @@ use std::time::Duration;
 
 use maplit::btreeset;
 use openraft::error::InitializeError;
-use openraft::error::MissingNodeInfo;
 use openraft::error::NotAllowed;
+use openraft::error::NotInMembers;
 use openraft::Config;
 use openraft::EffectiveMembership;
 use openraft::EntryPayload;
@@ -152,9 +152,9 @@ async fn initialize_err_target_not_include_target() -> anyhow::Result<()> {
         let err = res.unwrap_err();
 
         assert_eq!(
-            InitializeError::MissingNodeInfo(MissingNodeInfo {
+            InitializeError::NotInMembers(NotInMembers {
                 node_id,
-                reason: "target should be a member".to_string()
+                membership: Membership::new(vec![btreeset! {9   }], None)
             }),
             err
         );