Skip to content

Commit

Permalink
Change: remove Clone from trait AppData
Browse files Browse the repository at this point in the history
Application data `AppData` does not have to be `Clone`.

Upgrade tip:

Nothing needs to be done.
The default `RaftEntry` implementation `Entry` provided by openraft is
still `Clone`, if the AppData in it is `Clone`.
  • Loading branch information
drmingdrmer committed Mar 26, 2023
1 parent 19c94f4 commit 6e9d357
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 13 deletions.
14 changes: 13 additions & 1 deletion openraft/src/entry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ pub use traits::RaftEntry;
pub use traits::RaftPayload;

/// A Raft log entry.
#[derive(Clone)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize), serde(bound = ""))]
pub struct Entry<C>
where C: RaftTypeConfig
Expand All @@ -27,6 +26,19 @@ where C: RaftTypeConfig
pub payload: EntryPayload<C>,
}

impl<C> Clone for Entry<C>
where
C: RaftTypeConfig,
C::D: Clone,
{
fn clone(&self) -> Self {
Self {
log_id: self.log_id,
payload: self.payload.clone(),
}
}
}

impl<C> Debug for Entry<C>
where C: RaftTypeConfig
{
Expand Down
16 changes: 15 additions & 1 deletion openraft/src/entry/payload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::MessageSummary;
use crate::RaftTypeConfig;

/// Log entry payload variants.
#[derive(Clone, PartialEq)]
#[derive(PartialEq)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
pub enum EntryPayload<C: RaftTypeConfig> {
/// An empty payload committed by a new cluster leader.
Expand All @@ -19,6 +19,20 @@ pub enum EntryPayload<C: RaftTypeConfig> {
Membership(Membership<C::NodeId, C::Node>),
}

impl<C> Clone for EntryPayload<C>
where
C: RaftTypeConfig,
C::D: Clone,
{
fn clone(&self) -> Self {
match self {
EntryPayload::Blank => EntryPayload::Blank,
EntryPayload::Normal(n) => EntryPayload::Normal(n.clone()),
EntryPayload::Membership(m) => EntryPayload::Membership(m.clone()),
}
}
}

impl<C: RaftTypeConfig> fmt::Debug for EntryPayload<C> {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
Expand Down
4 changes: 2 additions & 2 deletions openraft/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,9 @@ impl<T> OptionalSerde for T {}
/// ## Note
///
/// The trait is automatically implemented for all types which satisfy its supertraits.
pub trait AppData: Clone + Send + Sync + 'static + OptionalSerde {}
pub trait AppData: Send + Sync + 'static + OptionalSerde {}

impl<T> AppData for T where T: Clone + Send + Sync + 'static + OptionalSerde {}
impl<T> AppData for T where T: Send + Sync + 'static + OptionalSerde {}

/// A trait defining application specific response data.
///
Expand Down
12 changes: 6 additions & 6 deletions tests/tests/append_entries/t20_append_conflicts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ async fn append_conflicts() -> Result<()> {
leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)),
};

let resp = r0.append_entries(req.clone()).await?;
let resp = r0.append_entries(req).await?;

assert!(resp.is_success());
assert!(!resp.is_conflict());
Expand All @@ -65,7 +65,7 @@ async fn append_conflicts() -> Result<()> {
leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)),
};

let resp = r0.append_entries(req.clone()).await?;
let resp = r0.append_entries(req).await?;
assert!(resp.is_success());
assert!(!resp.is_conflict());

Expand All @@ -78,31 +78,31 @@ async fn append_conflicts() -> Result<()> {
leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)),
};

let resp = r0.append_entries(req.clone()).await?;
let resp = r0.append_entries(req).await?;
assert!(resp.is_success());
assert!(!resp.is_conflict());

check_logs(&mut sto0, vec![0]).await?;

tracing::info!("--- case 0: prev_log_id.index == 0, ");

let req = AppendEntriesRequest {
let req = || AppendEntriesRequest {
vote: Vote::new_committed(1, 2),
prev_log_id: Some(LogId::new(CommittedLeaderId::new(0, 0), 0)),
entries: vec![blank(1, 1), blank(1, 2), blank(1, 3), blank(1, 4)],
// this set the last_applied to 2
leader_commit: Some(LogId::new(CommittedLeaderId::new(1, 0), 2)),
};

let resp = r0.append_entries(req.clone()).await?;
let resp = r0.append_entries(req()).await?;
assert!(resp.is_success());
assert!(!resp.is_conflict());

check_logs(&mut sto0, vec![0, 1, 1, 1, 1]).await?;

tracing::info!("--- case 0: prev_log_id.index == 0, last_log_id mismatch");

let resp = r0.append_entries(req.clone()).await?;
let resp = r0.append_entries(req()).await?;
assert!(resp.is_success());
assert!(!resp.is_conflict());

Expand Down
4 changes: 2 additions & 2 deletions tests/tests/append_entries/t40_append_updates_membership.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ async fn append_updates_membership() -> Result<()> {
leader_commit: Some(LogId::new(CommittedLeaderId::new(0, 0), 0)),
};

let resp = r0.append_entries(req.clone()).await?;
let resp = r0.append_entries(req).await?;
assert!(resp.is_success());
assert!(!resp.is_conflict());

Expand All @@ -80,7 +80,7 @@ async fn append_updates_membership() -> Result<()> {
leader_commit: Some(LogId::new(CommittedLeaderId::new(0, 0), 0)),
};

let resp = r0.append_entries(req.clone()).await?;
let resp = r0.append_entries(req).await?;
assert!(resp.is_success());
assert!(!resp.is_conflict());

Expand Down
2 changes: 1 addition & 1 deletion tests/tests/life_cycle/t20_initialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ async fn initialization() -> anyhow::Result<()> {

for i in [0, 1, 2] {
let mut sto = router.get_storage_handle(&1)?;
let first = StorageHelper::new(&mut sto).get_log_entries(0..2).await?.first().cloned();
let first = StorageHelper::new(&mut sto).get_log_entries(0..2).await?.into_iter().next();

tracing::info!("--- check membership is replicated: id: {}, first log: {:?}", i, first);
let mem = match first.unwrap().payload {
Expand Down

0 comments on commit 6e9d357

Please sign in to comment.