From 38802f3ad7a0d643bd8399ef212e248b126aec81 Mon Sep 17 00:00:00 2001 From: sydhds Date: Fri, 10 Mar 2023 15:29:43 +0100 Subject: [PATCH 1/2] MipStoreRaw::update_with now return enum --- massa-versioning-worker/src/lib.rs | 2 +- massa-versioning-worker/src/versioning.rs | 32 ++++++++++++++++------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/massa-versioning-worker/src/lib.rs b/massa-versioning-worker/src/lib.rs index 9de6e40a134..d5e03bdf568 100644 --- a/massa-versioning-worker/src/lib.rs +++ b/massa-versioning-worker/src/lib.rs @@ -10,7 +10,7 @@ //! # Note on MipInfo versions //! //! There is 2 different versions: -//! * version -> Network version -> This is the network version to announce and thus to store in block header +//! * version == Network version -> This is the network version to announce and thus to store in block header //! * component_version -> This is the version for the associated component and is used in VersioningFactory //! //! # Note on MipState diff --git a/massa-versioning-worker/src/versioning.rs b/massa-versioning-worker/src/versioning.rs index 63b1839e0ef..536a2ee514e 100644 --- a/massa-versioning-worker/src/versioning.rs +++ b/massa-versioning-worker/src/versioning.rs @@ -461,6 +461,12 @@ impl TryFrom<[(MipInfo, MipState); N]> for MipStore { } } +#[derive(Debug, PartialEq)] +pub enum UpdateWithResult { + Success(Vec, Vec), + Fail, +} + /// Store of all versioning info #[derive(Debug, Clone, Default)] pub struct MipStoreRaw(pub(crate) BTreeMap); @@ -468,11 +474,14 @@ pub struct MipStoreRaw(pub(crate) BTreeMap); impl MipStoreRaw { /// Update our store with another (usually after a bootstrap where we received another store) /// Return true if update is successful, false if something is wrong on given store raw - fn update_with(&mut self, store_raw: &MipStoreRaw) -> bool { + fn update_with(&mut self, store_raw: &MipStoreRaw) -> UpdateWithResult { // iter over items in given store: - // -> 2 cases: VersioningInfo is already in self store -> add to 'to_update' list - // VersioningInfo is not in self.store -> add to 'to_add' list - // + // -> 2 cases: + // * MipInfo is already in self store -> add to 'to_update' list + // * MipInfo is not in self.store -> We received a new MipInfo so we are running an out dated version + // of the software + // We then return the list of new MipInfo so we can warn and ask + // to update the software let mut component_versions: HashMap = self .0 @@ -551,11 +560,14 @@ impl MipStoreRaw { // Return false is there is nothing to merge - maybe we could make a distinction here? if should_merge && (!to_update.is_empty() || !to_add.is_empty()) { + let added = to_add.keys().cloned().collect(); + let updated = to_update.keys().cloned().collect(); + self.0.append(&mut to_update); self.0.append(&mut to_add); - true + UpdateWithResult::Success(updated, added) } else { - false + UpdateWithResult::Fail } } } @@ -572,8 +584,8 @@ impl TryFrom<[(MipInfo, MipState); N]> for MipStoreRaw { // Use update_with ensuring that we have no overlapping time range, unique names & ... match store.update_with(&other_store) { - true => Ok(store), - false => Err(()), + UpdateWithResult::Success(_, _) => Ok(store), + UpdateWithResult::Fail => Err(()), } } } @@ -1052,7 +1064,7 @@ mod test { ]); assert_eq!(_vs_raw_2_.is_err(), true); - assert_eq!(vs_raw_1.update_with(&vs_raw_2), false); + assert_eq!(vs_raw_1.update_with(&vs_raw_2), UpdateWithResult::Fail); assert_eq!(vs_raw_1.0.get(&vi_1).unwrap().inner, vs_1.inner); assert_eq!(vs_raw_1.0.get(&vi_2).unwrap().inner, vs_2.inner); @@ -1070,6 +1082,6 @@ mod test { (vi_2_2.clone(), vs_2_2.clone()), ])); - assert_eq!(vs_raw_1.update_with(&vs_raw_2), false); + assert_eq!(vs_raw_1.update_with(&vs_raw_2), UpdateWithResult::Fail); } } From 8e23fc3d373c05a2b2419dd63f8dde73b845e0d3 Mon Sep 17 00:00:00 2001 From: sydhds Date: Fri, 10 Mar 2023 16:29:33 +0100 Subject: [PATCH 2/2] Switch to a Result for return of MipStoreRaw::update_with --- massa-versioning-worker/src/versioning.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/massa-versioning-worker/src/versioning.rs b/massa-versioning-worker/src/versioning.rs index 536a2ee514e..287b19f4cf1 100644 --- a/massa-versioning-worker/src/versioning.rs +++ b/massa-versioning-worker/src/versioning.rs @@ -461,20 +461,14 @@ impl TryFrom<[(MipInfo, MipState); N]> for MipStore { } } -#[derive(Debug, PartialEq)] -pub enum UpdateWithResult { - Success(Vec, Vec), - Fail, -} - /// Store of all versioning info #[derive(Debug, Clone, Default)] pub struct MipStoreRaw(pub(crate) BTreeMap); impl MipStoreRaw { /// Update our store with another (usually after a bootstrap where we received another store) - /// Return true if update is successful, false if something is wrong on given store raw - fn update_with(&mut self, store_raw: &MipStoreRaw) -> UpdateWithResult { + /// Return list of updated / added if update is successful + fn update_with(&mut self, store_raw: &MipStoreRaw) -> Result<(Vec, Vec), ()> { // iter over items in given store: // -> 2 cases: // * MipInfo is already in self store -> add to 'to_update' list @@ -565,9 +559,9 @@ impl MipStoreRaw { self.0.append(&mut to_update); self.0.append(&mut to_add); - UpdateWithResult::Success(updated, added) + Ok((updated, added)) } else { - UpdateWithResult::Fail + Err(()) } } } @@ -584,8 +578,8 @@ impl TryFrom<[(MipInfo, MipState); N]> for MipStoreRaw { // Use update_with ensuring that we have no overlapping time range, unique names & ... match store.update_with(&other_store) { - UpdateWithResult::Success(_, _) => Ok(store), - UpdateWithResult::Fail => Err(()), + Ok(_) => Ok(store), + Err(_) => Err(()), } } } @@ -1064,7 +1058,7 @@ mod test { ]); assert_eq!(_vs_raw_2_.is_err(), true); - assert_eq!(vs_raw_1.update_with(&vs_raw_2), UpdateWithResult::Fail); + assert_eq!(vs_raw_1.update_with(&vs_raw_2), Err(())); assert_eq!(vs_raw_1.0.get(&vi_1).unwrap().inner, vs_1.inner); assert_eq!(vs_raw_1.0.get(&vi_2).unwrap().inner, vs_2.inner); @@ -1082,6 +1076,6 @@ mod test { (vi_2_2.clone(), vs_2_2.clone()), ])); - assert_eq!(vs_raw_1.update_with(&vs_raw_2), UpdateWithResult::Fail); + assert_eq!(vs_raw_1.update_with(&vs_raw_2), Err(())); } }