From 3cf7e7423aeb5812b1d31e775abe4404e64a5da4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Thu, 25 Apr 2024 11:43:50 +0200 Subject: [PATCH] feat: Prioritize directories over files during private directory merge --- wnfs/src/private/directory.rs | 41 +++++++++++++++++++++++++++++++++-- wnfs/src/private/link.rs | 11 ---------- wnfs/src/private/node/node.rs | 7 ++++-- 3 files changed, 44 insertions(+), 15 deletions(-) diff --git a/wnfs/src/private/directory.rs b/wnfs/src/private/directory.rs index f1b90968..51e33bc3 100644 --- a/wnfs/src/private/directory.rs +++ b/wnfs/src/private/directory.rs @@ -10,6 +10,7 @@ use chrono::{DateTime, Utc}; use libipld_core::cid::Cid; use rand_core::CryptoRngCore; use std::{ + cmp::Ordering, collections::{btree_map::Entry, BTreeMap, BTreeSet}, fmt::Debug, }; @@ -1358,12 +1359,14 @@ impl PrivateDirectory { } /// TODO(matheus23): DOCS - pub(crate) fn merge( + pub(crate) async fn merge( self: &mut Arc, target_header: PrivateNodeHeader, our_cid: Cid, other: &Arc, other_cid: Cid, + forest: &impl PrivateForest, + store: &impl BlockStore, ) -> Result<()> { if our_cid == other_cid { return Ok(()); @@ -1371,6 +1374,8 @@ impl PrivateDirectory { let other_ratchet_diff = target_header.ratchet_diff_for_merge(&other.header)?; + let parent_name = Some(self.header.name.clone()); + let our = self.prepare_next_merge(our_cid, target_header)?; if our.content.previous.len() > 1 { @@ -1403,13 +1408,45 @@ impl PrivateDirectory { } Entry::Occupied(mut occupied) => { let our_link = occupied.get_mut(); + // We just tie-break on the content cid. // It's assumed both links have been resolved to their // PrivateRef before, and we can tie-break on their content_cid. // Otherwise, how would we have gotten `our_cid` and `other_cid` // in this context? Both of these were gotten from `.store()`ing the // nodes, which includes resolving the children to `PrivateRef`s. - our_link.tie_break_with(other_link)?; + let our_content_hash = our_link.crdt_tiebreaker()?; + let other_content_hash = other_link.crdt_tiebreaker()?; + + let ord = our_content_hash.cmp(&other_content_hash); + if ord == Ordering::Equal { + // there's nothing for us to do, they're equal + } else { + let our_node = our_link + .resolve_node_mut(forest, store, parent_name.clone()) + .await?; + + let other_node = other_link + .resolve_node(forest, store, parent_name.clone()) + .await?; + + match (our_node, other_node) { + (PrivateNode::Dir(_), PrivateNode::File(_)) => { + // our node wins, we don't need to do anything. + } + (PrivateNode::File(_), PrivateNode::Dir(_)) => { + // a directory wins over a file + *our_link = other_link.clone(); + } + // file vs. file and dir vs. dir cases + _ => { + // We tie-break as usual + if ord == Ordering::Greater { + *our_link = other_link.clone(); + } + } + } + } } } } diff --git a/wnfs/src/private/link.rs b/wnfs/src/private/link.rs index 1c06d87f..fa3dcf7c 100644 --- a/wnfs/src/private/link.rs +++ b/wnfs/src/private/link.rs @@ -162,17 +162,6 @@ impl PrivateLink { pub(crate) fn crdt_tiebreaker(&self) -> Result> { Ok(*self.get_content_cid().ok_or_else(|| anyhow!("Impossible case: CRDT tiebreaker needed on node wasn't persisted before tie breaking"))?.hash()) } - - pub(crate) fn tie_break_with(&mut self, other_link: &PrivateLink) -> Result<()> { - let our_hash = self.crdt_tiebreaker()?; - let other_hash = other_link.crdt_tiebreaker()?; - - if other_hash.digest() < our_hash.digest() { - *self = other_link.clone(); - } - - Ok(()) - } } impl PartialEq for PrivateLink { diff --git a/wnfs/src/private/node/node.rs b/wnfs/src/private/node/node.rs index 5f4acf52..58e34e1b 100644 --- a/wnfs/src/private/node/node.rs +++ b/wnfs/src/private/node/node.rs @@ -410,7 +410,7 @@ impl PrivateNode { Ok(head) } else { // We need to create a merge node - Self::merge(header, (cid, head), unmerged_heads).await + Self::merge(header, (cid, head), unmerged_heads, forest, store).await } } else { // If None, then there's nothing to merge in (and this node was never stored) @@ -423,6 +423,8 @@ impl PrivateNode { header: PrivateNodeHeader, (cid, node): (Cid, PrivateNode), nodes: BTreeMap, + forest: &impl PrivateForest, + store: &impl BlockStore, ) -> Result { match node { PrivateNode::File(mut file) => { @@ -449,7 +451,8 @@ impl PrivateNode { // Need to pass in rng & mutable forest access // for the cases where we haven't yet written a node to // the forest, but need its hash for tie-breaking. - dir.merge(header.clone(), cid, &other_dir, other_cid)?; + dir.merge(header.clone(), cid, &other_dir, other_cid, forest, store) + .await?; } Ok(PrivateNode::Dir(dir))