diff --git a/firewood/Cargo.toml b/firewood/Cargo.toml index 907368160..cf87264ed 100644 --- a/firewood/Cargo.toml +++ b/firewood/Cargo.toml @@ -43,6 +43,7 @@ rand = "0.8.5" triehash = "0.8.4" clap = { version = "4.5.0", features = ['derive'] } pprof = { version = "0.13.0", features = ["flamegraph"] } +tempfile = "3.12.0" [[bench]] name = "hashops" diff --git a/firewood/src/db.rs b/firewood/src/db.rs index 2652bdcfe..449770b22 100644 --- a/firewood/src/db.rs +++ b/firewood/src/db.rs @@ -248,8 +248,7 @@ where .manager .read() .expect("poisoned lock") - .latest_revision() - .expect("no latest revision"); + .current_revision(); let proposal = NodeStore::new(parent)?; let mut merkle = Merkle::from(proposal); for op in batch { @@ -375,3 +374,70 @@ impl<'a> api::Proposal for Proposal<'a> { } } } +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod test { + use std::{ + ops::{Deref, DerefMut}, + path::PathBuf, + }; + + use crate::{ + db::Db, + v2::api::{Db as _, Error, Proposal}, + }; + + use super::DbConfig; + + #[tokio::test] + async fn test_cloned_proposal_error() { + let mut db = testdb().await; + let proposal = db + .propose::, Vec>(Default::default()) + .await + .unwrap(); + let cloned = proposal.clone(); + + // attempt to commit the clone; this should fail + let result = cloned.commit().await; + assert!( + matches!(result, Err(Error::CannotCommitClonedProposal)), + "{result:?}" + ); + + // the prior attempt consumed the Arc though, so cloned is no longer valid + // that means the actual proposal can be committed + let result = proposal.commit().await; + assert!(matches!(result, Ok(())), "{result:?}"); + } + + // Testdb is a helper struct for testing the Db. Once it's dropped, the directory and file disappear + struct TestDb { + db: Db, + _tmpdir: tempfile::TempDir, + } + impl Deref for TestDb { + type Target = Db; + fn deref(&self) -> &Self::Target { + &self.db + } + } + impl DerefMut for TestDb { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.db + } + } + + async fn testdb() -> TestDb { + let tmpdir = tempfile::tempdir().unwrap(); + let dbpath: PathBuf = [tmpdir.path().to_path_buf(), PathBuf::from("testdb")] + .iter() + .collect(); + let dbconfig = DbConfig::builder().truncate(true).build(); + let db = Db::new(dbpath, dbconfig).await.unwrap(); + TestDb { + db, + _tmpdir: tmpdir, + } + } +} diff --git a/firewood/src/manager.rs b/firewood/src/manager.rs index b00bcd73f..b22196967 100644 --- a/firewood/src/manager.rs +++ b/firewood/src/manager.rs @@ -25,6 +25,9 @@ pub struct RevisionManagerConfig { node_cache_size: NonZero, } +type CommittedRevision = Arc>; +type ProposedRevision = Arc>; + #[derive(Debug)] pub(crate) struct RevisionManager { /// Maximum number of revisions to keep on disk @@ -35,13 +38,25 @@ pub(crate) struct RevisionManager { /// The list of revisions that are on disk; these point to the different roots /// stored in the filebacked storage. - historical: VecDeque>>, - proposals: Vec>>, + historical: VecDeque, + proposals: Vec, // committing_proposals: VecDeque>, - by_hash: HashMap>>, + by_hash: HashMap, // TODO: maintain root hash of the most recent commit } +#[derive(Debug, thiserror::Error)] +pub(crate) enum RevisionManagerError { + #[error("The proposal cannot be committed since a sibling was committed")] + SiblingCommitted, + #[error( + "The proposal cannot be committed since it is not a direct child of the most recent commit" + )] + NotLatest, + #[error("An IO error occurred during the commit")] + IO(#[from] std::io::Error), +} + impl RevisionManager { pub fn new( filename: PathBuf, @@ -61,24 +76,6 @@ impl RevisionManager { Ok(manager) } - pub fn latest_revision(&self) -> Option>> { - self.historical.back().cloned() - } -} - -#[derive(Debug, thiserror::Error)] -pub(crate) enum RevisionManagerError { - #[error("The proposal cannot be committed since a sibling was committed")] - SiblingCommitted, - #[error( - "The proposal cannot be committed since it is not a direct child of the most recent commit" - )] - NotLatest, - #[error("An IO error occurred during the commit")] - IO(#[from] std::io::Error), -} - -impl RevisionManager { /// Commit a proposal /// To commit a proposal involves a few steps: /// 1. Commit check. @@ -106,10 +103,7 @@ impl RevisionManager { /// Any other proposals that have this proposal as a parent should be reparented to the committed version. /// 8. Revision reaping. /// If more than the configurable number of revisions is available, the oldest revision can be forgotten. - pub fn commit( - &mut self, - proposal: Arc>, - ) -> Result<(), RevisionManagerError> { + pub fn commit(&mut self, proposal: ProposedRevision) -> Result<(), RevisionManagerError> { // 1. Commit check let current_revision = self.current_revision(); if !proposal @@ -122,7 +116,7 @@ impl RevisionManager { // TODO // 3. Set last committed revision - let committed: Arc> = proposal.as_committed().into(); + let committed: CommittedRevision = proposal.as_committed().into(); self.historical.push_back(committed.clone()); if let Some(hash) = committed.kind.root_hash() { self.by_hash.insert(hash, committed.clone()); @@ -157,14 +151,11 @@ impl RevisionManager { pub type NewProposalError = (); // TODO implement impl RevisionManager { - pub fn add_proposal(&mut self, proposal: Arc>) { + pub fn add_proposal(&mut self, proposal: ProposedRevision) { self.proposals.push(proposal); } - pub fn revision( - &self, - root_hash: HashKey, - ) -> Result>, RevisionManagerError> { + pub fn revision(&self, root_hash: HashKey) -> Result { self.by_hash .get(&root_hash) .cloned() @@ -185,7 +176,7 @@ impl RevisionManager { ))) } - fn current_revision(&self) -> Arc> { + pub fn current_revision(&self) -> CommittedRevision { self.historical .back() .expect("there is always one revision")