From 2c519f5b5f495a48ab7da8db64fa43c1583a1b6e Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Mon, 29 Aug 2022 15:25:00 +0200 Subject: [PATCH] node: Minimize inventory, fix tests Signed-off-by: Alexis Sellier --- node/src/protocol.rs | 5 ++--- node/src/storage.rs | 2 +- node/src/storage/git.rs | 32 ++++++++++---------------------- node/src/test/arbitrary.rs | 15 +++++++++++++++ node/src/test/fixtures.rs | 10 ++++++---- node/src/test/storage.rs | 2 +- 6 files changed, 35 insertions(+), 31 deletions(-) diff --git a/node/src/protocol.rs b/node/src/protocol.rs index 66ec1765..859195ba 100644 --- a/node/src/protocol.rs +++ b/node/src/protocol.rs @@ -73,7 +73,7 @@ pub enum Message { /// Send our inventory to a peer. Sent in response to [`Message::GetInventory`]. /// Nb. This should be the whole inventory, not a partial update. Inventory { - inv: Inventory, + inv: Vec, timestamp: Timestamp, /// Original peer this inventory came from. We don't set this when we /// are the originator, only when relaying. @@ -253,7 +253,6 @@ impl Protocol { .storage .inventory()? .into_iter() - .map(|(id, _)| id) .filter(|id| !blocked.contains(id)) .collect(), @@ -714,7 +713,7 @@ where /// Process a peer inventory announcement by updating our routing table. fn process_inventory(&mut self, inventory: &Inventory, from: PeerId) { - for (proj_id, _refs) in inventory { + for proj_id in inventory { let inventory = self .routing .entry(proj_id.clone()) diff --git a/node/src/storage.rs b/node/src/storage.rs index 0eb406dd..fd5be0a4 100644 --- a/node/src/storage.rs +++ b/node/src/storage.rs @@ -21,7 +21,7 @@ use crate::identity::{ProjId, ProjIdError, UserId}; pub static IDENTITY_PATH: Lazy<&Path> = Lazy::new(|| Path::new(".rad/identity.toml")); pub type BranchName = String; -pub type Inventory = Vec<(ProjId, HashMap>)>; +pub type Inventory = Vec; /// Storage error. #[derive(Error, Debug)] diff --git a/node/src/storage/git.rs b/node/src/storage/git.rs index ca8ccb67..0ff92554 100644 --- a/node/src/storage/git.rs +++ b/node/src/storage/git.rs @@ -46,20 +46,7 @@ impl ReadStorage for Storage { } fn inventory(&self) -> Result { - let projs = self.projects()?; - let mut inv = Vec::new(); - - for proj in projs { - let repo = self.repository(&proj)?; - let remotes = repo - .remotes()? - .into_iter() - .map(|(id, r)| (id.to_string(), r)) - .collect(); - - inv.push((proj, remotes)); - } - Ok(inv) + self.projects() } } @@ -226,7 +213,7 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let storage = fixtures::storage(dir.path()); let inv = storage.inventory().unwrap(); - let (proj, _) = inv.first().unwrap(); + let proj = inv.first().unwrap(); let refs = git::list_remotes(&Url { host: Some(dir.path().to_string_lossy().to_string()), scheme: git_url::Scheme::File, @@ -242,11 +229,12 @@ mod tests { #[test] fn test_fetch() { - let path = tempfile::tempdir().unwrap().into_path(); - let alice = fixtures::storage(path.join("alice")); - let bob = Storage::new(path.join("bob")); + let tmp = tempfile::tempdir().unwrap(); + let alice = fixtures::storage(tmp.path().join("alice")); + let bob = Storage::new(tmp.path().join("bob")); let inventory = alice.inventory().unwrap(); - let (proj, remotes) = inventory.first().unwrap(); + let proj = inventory.first().unwrap(); + let remotes = alice.repository(proj).unwrap().remotes().unwrap(); let refname = "refs/heads/master"; // Have Bob fetch Alice's refs. @@ -260,16 +248,16 @@ mod tests { }) .unwrap(); - for remote in remotes.values() { + for (id, _) in remotes.into_iter() { let alice_oid = alice .repository(proj) .unwrap() - .find_reference(&remote.id, refname) + .find_reference(&id, refname) .unwrap(); let bob_oid = bob .repository(proj) .unwrap() - .find_reference(&remote.id, refname) + .find_reference(&id, refname) .unwrap(); assert_eq!(alice_oid, bob_oid); diff --git a/node/src/test/arbitrary.rs b/node/src/test/arbitrary.rs index 9fc0b64e..aebca5a5 100644 --- a/node/src/test/arbitrary.rs +++ b/node/src/test/arbitrary.rs @@ -1,9 +1,24 @@ +use std::collections::HashSet; +use std::hash::Hash; +use std::ops::RangeBounds; + use crate::collections::HashMap; use crate::hash; use crate::identity::{ProjId, UserId}; use crate::storage; use crate::test::storage::MockStorage; +pub fn set(range: impl RangeBounds) -> HashSet { + let size = fastrand::usize(range); + let mut set = HashSet::with_capacity(size); + let mut g = quickcheck::Gen::new(size); + + while set.len() < size { + set.insert(T::arbitrary(&mut g)); + } + set +} + pub fn gen(size: usize) -> T { let mut gen = quickcheck::Gen::new(size); diff --git a/node/src/test/fixtures.rs b/node/src/test/fixtures.rs index 992a407d..cb24bab0 100644 --- a/node/src/test/fixtures.rs +++ b/node/src/test/fixtures.rs @@ -8,8 +8,10 @@ use crate::test::arbitrary; pub fn storage>(path: P) -> Storage { let path = path.as_ref(); let storage = Storage::new(path); - let proj_ids = arbitrary::gen::>(9); - let user_ids = arbitrary::gen::>(9); + let proj_ids = arbitrary::set::(3..5); + let user_ids = arbitrary::set::(1..3); + + crate::test::logger::init(log::Level::Debug); for proj in proj_ids.iter() { log::debug!("creating {}...", proj); @@ -70,8 +72,8 @@ mod tests { #[test] fn smoke() { - let path = tempfile::tempdir().unwrap().into_path(); + let tmp = tempfile::tempdir().unwrap(); - storage(&path); + storage(&tmp.path()); } } diff --git a/node/src/test/storage.rs b/node/src/test/storage.rs index ac2a602b..2f11c6b1 100644 --- a/node/src/test/storage.rs +++ b/node/src/test/storage.rs @@ -34,7 +34,7 @@ impl ReadStorage for MockStorage { let inventory = self .inventory .iter() - .map(|(id, remotes)| (id.clone(), remotes.clone().into())) + .map(|(id, _)| id.clone()) .collect::>(); Ok(inventory)