diff --git a/collab/src/folder/folder.rs b/collab/src/folder/folder.rs index 385a8ce3a..b84e0ff58 100644 --- a/collab/src/folder/folder.rs +++ b/collab/src/folder/folder.rs @@ -16,7 +16,6 @@ use uuid::Uuid; use super::folder_observe::ViewChangeSender; use super::hierarchy_builder::{FlattedViews, ParentChildViews}; -use super::revision::RevisionMapping; use super::section::{Section, SectionItem, SectionMap}; use super::{ FolderData, ParentChildRelations, SectionChangeSender, SpacePermission, TrashInfo, View, @@ -57,7 +56,6 @@ const VIEWS: &str = "views"; const PARENT_CHILD_VIEW_RELATION: &str = "relation"; const CURRENT_VIEW: &str = "current_view"; const CURRENT_VIEW_FOR_USER: &str = "current_view_for_user"; -const REVISION_MAP: &str = "revision_map"; pub(crate) const FAVORITES_V1: &str = "favorites"; const SECTION: &str = "section"; @@ -1098,11 +1096,6 @@ impl Folder { } } - pub fn replace_view(&mut self, from: &ViewId, to: &ViewId, uid: i64) -> bool { - let mut txn = self.collab.transact_mut(); - self.body.replace_view(&mut txn, from, to, Some(uid)) - } - /// Get a view by id. When uid is provided, includes user-specific data like is_favorite. /// When uid is None, returns base view data without user-specific enrichment. pub fn get_view(&self, view_id: &ViewId, uid: Option) -> Option> { @@ -1244,9 +1237,6 @@ impl FolderBody { let parent_child_relations = Arc::new(ParentChildRelations::new( root.get_or_init(&mut txn, PARENT_CHILD_VIEW_RELATION), )); - let revision_map = Arc::new(RevisionMapping::new( - root.get_or_init(&mut txn, REVISION_MAP), - )); let section = Arc::new(SectionMap::create( &mut txn, @@ -1262,7 +1252,6 @@ impl FolderBody { .map(|notifier| notifier.view_change_tx.clone()), parent_child_relations, section.clone(), - revision_map, )); if let Some(folder_data) = folder_data { @@ -1612,18 +1601,6 @@ impl FolderBody { current_view_for_user.try_update(txn, uid.to_string(), view.to_string()); } } - - pub fn replace_view( - &self, - txn: &mut TransactionMut, - old_view_id: &ViewId, - new_view_id: &ViewId, - uid: Option, - ) -> bool { - uid - .map(|uid| self.views.replace_view(txn, old_view_id, new_view_id, uid)) - .unwrap_or(false) - } } pub fn default_folder_data(uid: i64, workspace_id: &str) -> FolderData { diff --git a/collab/src/folder/folder_observe.rs b/collab/src/folder/folder_observe.rs index f7bf31b44..e89abd565 100644 --- a/collab/src/folder/folder_observe.rs +++ b/collab/src/folder/folder_observe.rs @@ -6,7 +6,6 @@ use crate::preclude::{ }; use tokio::sync::broadcast; -use super::revision::RevisionMapping; use super::section::SectionMap; use super::view::FOLDER_VIEW_ID; use super::{ParentChildRelations, View, ViewId, view_from_map_ref}; @@ -27,7 +26,6 @@ pub(crate) fn subscribe_view_change( change_tx: ViewChangeSender, view_relations: Arc, section_map: Arc, - revision_mapping: Arc, uid: i64, ) -> Subscription { let r = root.clone(); @@ -45,16 +43,10 @@ pub(crate) fn subscribe_view_change( if let Some(view_id_str) = map_ref.get_with_txn::<_, String>(txn, FOLDER_VIEW_ID) { if let Ok(view_uuid) = uuid::Uuid::parse_str(&view_id_str) { - let (view_id, mappings) = revision_mapping.mappings(txn, view_uuid); - if let Some(YrsValue::YMap(map_ref)) = r.get(txn, &view_id.to_string()) { - if let Some(view) = view_from_map_ref( - &map_ref, - txn, - &view_relations, - §ion_map, - Some(uid), - mappings, - ) { + if let Some(YrsValue::YMap(map_ref)) = r.get(txn, &view_uuid.to_string()) { + if let Some(view) = + view_from_map_ref(&map_ref, txn, &view_relations, §ion_map, Some(uid)) + { deletion_cache.insert(view.id, Arc::new(view.clone())); // Send indexing view @@ -71,16 +63,10 @@ pub(crate) fn subscribe_view_change( .get_with_txn::<_, String>(txn, FOLDER_VIEW_ID) { if let Ok(view_uuid) = uuid::Uuid::parse_str(&view_id_str) { - let (view_id, mappings) = revision_mapping.mappings(txn, view_uuid); - if let Some(YrsValue::YMap(map_ref)) = r.get(txn, &view_id.to_string()) { - if let Some(view) = view_from_map_ref( - &map_ref, - txn, - &view_relations, - §ion_map, - Some(uid), - mappings, - ) { + if let Some(YrsValue::YMap(map_ref)) = r.get(txn, &view_uuid.to_string()) { + if let Some(view) = + view_from_map_ref(&map_ref, txn, &view_relations, §ion_map, Some(uid)) + { // Update deletion cache with the updated view deletion_cache.insert(view.id, Arc::new(view.clone())); let _ = change_tx.send(ViewChange::DidUpdate { view }); diff --git a/collab/src/folder/mod.rs b/collab/src/folder/mod.rs index e8631a0d9..a5caa1af8 100644 --- a/collab/src/folder/mod.rs +++ b/collab/src/folder/mod.rs @@ -18,7 +18,6 @@ mod folder_migration; mod folder_observe; pub mod hierarchy_builder; mod relation; -mod revision; mod section; pub mod space_info; mod view; diff --git a/collab/src/folder/revision.rs b/collab/src/folder/revision.rs deleted file mode 100644 index 3d4e110e3..000000000 --- a/collab/src/folder/revision.rs +++ /dev/null @@ -1,79 +0,0 @@ -use super::ViewId; -use crate::preclude::{Any, Map, MapRef, Out, ReadTxn, TransactionMut}; - -pub struct RevisionMapping { - container: MapRef, -} - -impl RevisionMapping { - /// The maximum number of jumps to prevent infinite loops in the revision map. - const REVISION_MAP_JUMP_LIMIT: usize = 1000; - - pub fn new(container: MapRef) -> Self { - Self { container } - } - - pub fn contains_key(&self, txn: &T, key: &str) -> bool { - self.container.contains_key(txn, key) - } - - pub fn replace_view(&self, txn: &mut TransactionMut, old_view_id: &ViewId, new_view_id: &ViewId) { - let uuid_old_view_id = old_view_id.to_string(); - let uuid_new_view_id = new_view_id.to_string(); - - if self.container.contains_key(txn, &uuid_new_view_id) { - // new view id should not already exist in the revision map, otherwise it could create a cycle - panic!( - "new view_id {} already exists in the revision map", - new_view_id - ); - } - - self - .container - .insert(txn, uuid_old_view_id, uuid_new_view_id); - } - - pub fn mappings(&self, txn: &impl ReadTxn, view_id: ViewId) -> (ViewId, Vec) { - let mut buf = Vec::new(); - let last_view_id = self.iter_mapping(txn, view_id, |view_id| { - buf.push(view_id); - }); - (last_view_id, buf) - } - - pub fn map(&self, txn: &T, view_id: ViewId) -> ViewId { - self.iter_mapping(txn, view_id, |_| {}) - } - - fn iter_mapping(&self, txn: &T, view_id: ViewId, mut f: F) -> ViewId - where - T: ReadTxn, - F: FnMut(ViewId), - { - let mut current_view_id = view_id; - let mut i = Self::REVISION_MAP_JUMP_LIMIT; - while i > 0 { - if let Some(Out::Any(Any::String(next_view_id_str))) = - self.container.get(txn, ¤t_view_id.to_string()) - { - if let Ok(next_view_id) = uuid::Uuid::parse_str(&next_view_id_str) { - let old_view_id = std::mem::replace(&mut current_view_id, next_view_id); - f(old_view_id); - i -= 1; - } else { - break; - } - } else { - break; - } - } - if i == 0 { - panic!( - "Infinite loop detected in revision map for view_id: {}", - current_view_id - ); - } - current_view_id - } -} diff --git a/collab/src/folder/view.rs b/collab/src/folder/view.rs index e45a63664..818bd5c03 100644 --- a/collab/src/folder/view.rs +++ b/collab/src/folder/view.rs @@ -1,4 +1,4 @@ -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::str::FromStr; use std::sync::Arc; @@ -15,7 +15,6 @@ use uuid::Uuid; use super::folder_observe::ViewChangeSender; use super::UserId; -use super::revision::RevisionMapping; use super::section::{Section, SectionItem, SectionMap}; use super::space_info::SpaceInfo; use super::{ParentChildRelations, RepeatedViewIdentifier, ViewIdentifier, subscribe_view_change}; @@ -44,7 +43,6 @@ pub struct ViewsMap { pub(crate) container: MapRef, pub(crate) parent_children_relation: Arc, pub(crate) section_map: Arc, - pub(crate) revision_map: Arc, // Minimal cache only for deletion notifications - stores basic view info deletion_cache: Arc>>, subscription: Mutex>, @@ -57,7 +55,6 @@ impl ViewsMap { change_tx: Option, view_relations: Arc, section_map: Arc, - revision_map: Arc, ) -> ViewsMap { trace!("Initializing ViewsMap with deletion cache for proper deletion notifications"); // Initialize deletion cache with existing views @@ -67,7 +64,6 @@ impl ViewsMap { change_tx, parent_children_relation: view_relations, section_map, - revision_map, deletion_cache: Arc::new(DashMap::new()), } } @@ -87,7 +83,6 @@ impl ViewsMap { change_tx.clone(), self.parent_children_relation.clone(), self.section_map.clone(), - self.revision_map.clone(), uid, ) }); @@ -174,10 +169,9 @@ impl ViewsMap { .iter() .flat_map(|child| { // Always load fresh from storage - let (child_id, mappings) = self.revision_map.mappings(txn, child.id); self .container - .get_with_txn(txn, &child_id.to_string()) + .get_with_txn(txn, &child.id.to_string()) .and_then(|map| { view_from_map_ref( &map, @@ -185,7 +179,6 @@ impl ViewsMap { &self.parent_children_relation, &self.section_map, uid, - mappings, ) }) .map(Arc::new) @@ -225,41 +218,19 @@ impl ViewsMap { /// Get all views. When uid is provided, includes user-specific enrichment like is_favorite. pub fn get_all_views(&self, txn: &T, uid: Option) -> Vec> { - // since views can be mapped through revisions, we need a map of final_view_id and its predecessors - - // first split the keys into ones that have existing mappings (roots) and ones that have not more mapping (leafs) - let (roots, leafs): (Vec<_>, Vec<_>) = self + self .container - .keys(txn) - .partition(|view_id| self.revision_map.contains_key(txn, view_id)); - - let mut mapped: HashMap<_, _> = leafs - .into_iter() - .map(|view_id| (view_id.to_string(), HashSet::new())) - .collect(); - - for view_id in roots { - let view_uuid = Uuid::parse_str(view_id).unwrap_or_else(|_| Uuid::nil()); - let (_, mappings) = self.revision_map.mappings(txn, view_uuid); - let values = mapped - .entry(view_id.to_string()) - .or_insert_with(HashSet::new); - values.extend(mappings); - } - - mapped - .into_iter() - .flat_map(|(final_id, predecessors)| { - let map_ref = self.container.get_with_txn(txn, &final_id)?; - view_from_map_ref( - &map_ref, + .iter(txn) + .flat_map(|(_, v)| match v { + yrs::Out::YMap(map) => view_from_map_ref( + &map, txn, &self.parent_children_relation, &self.section_map, uid, - predecessors, ) - .map(Arc::new) + .map(Arc::new), + _ => None, }) .collect() } @@ -300,7 +271,6 @@ impl ViewsMap { view_id: &ViewId, uid: Option, ) -> Option> { - let (view_id, mappings) = self.revision_map.mappings(txn, *view_id); let map_ref = self.container.get_with_txn(txn, &view_id.to_string())?; view_from_map_ref( &map_ref, @@ -308,7 +278,6 @@ impl ViewsMap { &self.parent_children_relation, &self.section_map, uid, - mappings, ) .map(Arc::new) } @@ -505,31 +474,6 @@ impl ViewsMap { timestamp } } - - pub fn replace_view( - &self, - txn: &mut TransactionMut, - old_view_id: &ViewId, - new_view_id: &ViewId, - uid: i64, - ) -> bool { - if let Some(old_view) = self.get_view(txn, old_view_id, Some(uid)) { - let mut new_view = (*old_view).clone(); - new_view.id = *new_view_id; - new_view.last_edited_by = Some(uid); - new_view.last_edited_time = timestamp(); - - self.insert(txn, new_view, None, uid); - - self - .revision_map - .replace_view(txn, old_view_id, new_view_id); - - true - } else { - false - } - } } pub(crate) fn view_from_map_ref( @@ -538,7 +482,6 @@ pub(crate) fn view_from_map_ref( view_relations: &Arc, section_map: &SectionMap, uid: Option, - mappings: impl IntoIterator, ) -> Option { let parent_view_id: String = map_ref.get_with_txn(txn, VIEW_PARENT_ID)?; let parent_view_id = if parent_view_id.is_empty() { @@ -558,23 +501,11 @@ pub(crate) fn view_from_map_ref( .get_with_txn::<_, i64>(txn, VIEW_LAYOUT) .and_then(|v| v.try_into().ok())?; - let mut children = view_relations + let children = view_relations .get_children_with_txn(txn, &id) .map(|array| array.get_children_with_txn(txn)) .unwrap_or_default(); - for view_id in mappings { - let ids = view_relations - .get_children_with_txn(txn, &view_id) - .map(|array| array.get_children_with_txn(txn)) - .unwrap_or_default(); - for view_id in ids.items { - if !children.items.contains(&view_id) { - children.items.push(view_id); - } - } - } - let icon = get_icon_from_view_map(map_ref, txn); let is_favorite = uid .and_then(|uid| { @@ -915,7 +846,6 @@ impl<'a, 'b, 'c> ViewUpdate<'a, 'b, 'c> { &self.children_map, self.section_map, Some(self.uid.as_i64()), - [], ) } } diff --git a/collab/tests/folder/main.rs b/collab/tests/folder/main.rs index 76b5cbeb2..6a3fb7e9b 100644 --- a/collab/tests/folder/main.rs +++ b/collab/tests/folder/main.rs @@ -5,7 +5,6 @@ mod custom_section; mod favorite_test; mod load_disk; // mod recent_views_test; -mod replace_view_test; mod serde_test; mod space_info_test; mod trash_test; diff --git a/collab/tests/folder/replace_view_test.rs b/collab/tests/folder/replace_view_test.rs deleted file mode 100644 index 5013baf3f..000000000 --- a/collab/tests/folder/replace_view_test.rs +++ /dev/null @@ -1,195 +0,0 @@ -use crate::util::{create_folder, make_test_view, parse_view_id}; -use collab::entity::uuid_validation::view_id_from_any_string; -use collab::folder::{Folder, UserId}; -use collab::preclude::updates::decoder::Decode; -use collab::preclude::{Collab, Update}; -use uuid::Uuid; - -#[test] -fn replace_view_get_view() { - let uid = 1i64; - let mut folder = create_folder(UserId::from(uid), view_id_from_any_string("w1")); - let workspace_id = folder.get_workspace_id().unwrap(); - - // Create initial views - let v1 = make_test_view("v1", workspace_id, vec![]); - let v21 = make_test_view("v2.1", workspace_id, vec![]); - let v22 = make_test_view("v2.2", workspace_id, vec![]); - let v2 = make_test_view( - "v2", - workspace_id, - vec![ - crate::util::test_uuid("v2.1"), - crate::util::test_uuid("v2.2"), - ], - ); - folder.insert_view(v1, None, uid); - folder.insert_view(v21, None, uid); - folder.insert_view(v22, None, uid); - folder.insert_view(v2, None, uid); - - let v2_id = crate::util::test_uuid("v2").to_string(); - let old = folder.get_view(&parse_view_id(&v2_id), Some(uid)).unwrap(); - assert_eq!( - old.id.to_string(), - uuid::Uuid::new_v5(&uuid::Uuid::NAMESPACE_OID, "v2".as_bytes()).to_string() - ); - - folder.replace_view( - &crate::util::test_uuid("v2"), - &crate::util::test_uuid("v3"), - uid, - ); - - // getting old view id should return new one - let new = folder.get_view(&parse_view_id(&v2_id), Some(uid)).unwrap(); - assert_eq!( - new.id.to_string(), - uuid::Uuid::new_v5(&uuid::Uuid::NAMESPACE_OID, "v3".as_bytes()).to_string() - ); - assert_eq!(new.name, old.name); - assert_eq!(new.parent_view_id, old.parent_view_id); - assert_eq!(new.children, old.children); - assert_eq!(new.layout, old.layout); - assert_eq!(new.icon, old.icon); -} - -#[test] -fn replace_view_get_view_concurrent_update() { - let uid1 = 1i64; - let uid2 = 1i64; - let mut f1 = create_folder(UserId::from(uid1), view_id_from_any_string("w1")); - let workspace_id = f1.get_workspace_id().unwrap(); - - // Create initial views - let v1 = make_test_view("v1", workspace_id, vec![]); - let v21 = make_test_view("v2.1", workspace_id, vec![]); - let v22 = make_test_view("v2.2", workspace_id, vec![]); - let v2 = make_test_view( - "v2", - workspace_id, - vec![ - crate::util::test_uuid("v2.1"), - crate::util::test_uuid("v2.2"), - ], - ); - f1.insert_view(v1, None, uid1); - f1.insert_view(v21, None, uid1); - f1.insert_view(v22, None, uid1); - f1.insert_view(v2, None, uid1); - - let mut collab = Collab::new(uid2, Uuid::new_v4(), "device-2", 2); - - // sync initial state between f1 and f2 - collab - .apply_update(Update::decode_v2(&f1.encode_collab_v2().doc_state).unwrap()) - .unwrap(); - let mut f2 = Folder::open(collab, None).unwrap(); - - assert_eq!(f1.to_json_value(), f2.to_json_value()); - - // concurrently replace view in f1 and add new child view in f2 - f1.replace_view( - &crate::util::test_uuid("v2"), - &crate::util::test_uuid("v3"), - uid1, - ); - - let mut v23 = make_test_view("v2.3", workspace_id, vec![]); - v23.parent_view_id = Some(collab::entity::uuid_validation::view_id_from_any_string( - "v2", - )); - f2.insert_view(v23, None, uid2); - - // cross-sync state between f1 and f2 - f2.apply_update(Update::decode_v2(&f1.encode_collab_v2().doc_state).unwrap()) - .unwrap(); - f1.apply_update(Update::decode_v2(&f2.encode_collab_v2().doc_state).unwrap()) - .unwrap(); - - let v2_id = crate::util::test_uuid("v2").to_string(); - let v1 = f1.get_view(&parse_view_id(&v2_id), Some(uid2)).unwrap(); - assert_eq!( - v1.id.to_string(), - uuid::Uuid::new_v5(&uuid::Uuid::NAMESPACE_OID, "v3".as_bytes()).to_string() - ); - assert_eq!( - v1.children - .iter() - .map(|c| c.id.to_string()) - .collect::>(), - vec![ - uuid::Uuid::new_v5(&uuid::Uuid::NAMESPACE_OID, "v2.1".as_bytes()).to_string(), - uuid::Uuid::new_v5(&uuid::Uuid::NAMESPACE_OID, "v2.2".as_bytes()).to_string(), - uuid::Uuid::new_v5(&uuid::Uuid::NAMESPACE_OID, "v2.3".as_bytes()).to_string() - ] - ); - - let v2 = f2.get_view(&parse_view_id(&v2_id), Some(uid1)).unwrap(); - assert_eq!(v1, v2); -} - -#[test] -fn replace_view_all_views_concurrent_update() { - let uid1 = 1i64; - let uid2 = 1i64; - let mut f1 = create_folder(UserId::from(uid1), view_id_from_any_string("w1")); - let workspace_id = f1.get_workspace_id().unwrap(); - - // Create initial views - let v1 = make_test_view("v1", workspace_id, vec![]); - let v21 = make_test_view("v2.1", workspace_id, vec![]); - let v22 = make_test_view("v2.2", workspace_id, vec![]); - let v2 = make_test_view( - "v2", - workspace_id, - vec![ - crate::util::test_uuid("v2.1"), - crate::util::test_uuid("v2.2"), - ], - ); - f1.insert_view(v1, None, uid1); - f1.insert_view(v21, None, uid1); - f1.insert_view(v22, None, uid1); - f1.insert_view(v2, None, uid1); - - let mut collab = Collab::new(uid2, Uuid::new_v4(), "device-2", 2); - - // sync initial state between f1 and f2 - collab - .apply_update(Update::decode_v2(&f1.encode_collab_v2().doc_state).unwrap()) - .unwrap(); - let mut f2 = Folder::open(collab, None).unwrap(); - - assert_eq!(f1.to_json_value(), f2.to_json_value()); - - // concurrently replace view in f1 and add new child view in f2 - f1.replace_view( - &crate::util::test_uuid("v2"), - &crate::util::test_uuid("v3"), - uid1, - ); - - let mut v23 = make_test_view("v2.3", workspace_id, vec![]); - v23.parent_view_id = Some(collab::entity::uuid_validation::view_id_from_any_string( - "v2", - )); - f2.insert_view(v23, None, uid2); - - // cross-sync state between f1 and f2 - f2.apply_update(Update::decode_v2(&f1.encode_collab_v2().doc_state).unwrap()) - .unwrap(); - f1.apply_update(Update::decode_v2(&f2.encode_collab_v2().doc_state).unwrap()) - .unwrap(); - - // check if both sides have the same views - assert_eq!(f1.to_json_value(), f2.to_json_value()); - - let mut v1 = f1.get_all_views(Some(uid1)); - let mut v2 = f2.get_all_views(Some(uid2)); - - v1.sort_by_key(|v| v.id.to_string()); - v2.sort_by_key(|v| v.id.to_string()); - - assert_eq!(v1, v2); -}