Skip to content

Commit 89c4819

Browse files
Horusiathappflowy
andauthored
chore: change parent view id to uuid (#416)
* chore: change parent view id to uuid * chore: fix clippy issues * chore: convert folder get workspace id to uuid * chore: fix clippy issues * chore: serialize parent view id into empty string if none * chore: clippy fix --------- Co-authored-by: Nathan <[email protected]>
1 parent 576cfb0 commit 89c4819

File tree

18 files changed

+304
-333
lines changed

18 files changed

+304
-333
lines changed

collab-folder/src/folder.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,9 @@ impl Folder {
173173
self.body.get_workspace_info(&txn, workspace_id, uid)
174174
}
175175

176-
pub fn get_workspace_id(&self) -> Option<String> {
176+
pub fn get_workspace_id(&self) -> Option<ViewId> {
177177
let txn = self.collab.transact();
178-
self.body.get_workspace_id(&txn)
178+
self.body.get_workspace_id(&txn)?.parse().ok()
179179
}
180180

181181
pub fn get_all_views(&self, uid: i64) -> Vec<Arc<View>> {
@@ -721,8 +721,8 @@ impl FolderBody {
721721
uid: i64,
722722
) -> Option<Arc<View>> {
723723
let view = self.views.get_view_with_txn(txn, view_id, uid)?;
724-
if let Ok(parent_uuid) = uuid::Uuid::parse_str(&view.parent_view_id) {
725-
self.views.move_child(txn, &parent_uuid, from, to);
724+
if let Some(parent_uuid) = &view.parent_view_id {
725+
self.views.move_child(txn, parent_uuid, from, to);
726726
}
727727
Some(view)
728728
}
@@ -751,10 +751,10 @@ impl FolderBody {
751751
}
752752

753753
// dissociate the child from its parent
754-
if let Ok(parent_uuid) = uuid::Uuid::parse_str(parent_id) {
754+
if let Some(parent_uuid) = parent_id {
755755
self
756756
.views
757-
.dissociate_parent_child_with_txn(txn, &parent_uuid, view_id);
757+
.dissociate_parent_child_with_txn(txn, parent_uuid, view_id);
758758
}
759759
// associate the child with its new parent and place it after the prev_view_id. If the prev_view_id is None,
760760
// place it as the first child.
@@ -893,23 +893,23 @@ mod tests {
893893
let collab = Collab::new_with_options(CollabOrigin::Empty, options).unwrap();
894894
let view_1 = View::new(
895895
Uuid::parse_str("00000000-0000-0000-0000-000000000002").unwrap(),
896-
workspace_id.to_string(),
896+
workspace_id,
897897
"View 1".to_string(),
898898
crate::ViewLayout::Document,
899899
Some(uid),
900900
);
901901
let view_1_id = view_1.id;
902902
let view_2 = View::new(
903903
Uuid::parse_str("00000000-0000-0000-0000-000000000003").unwrap(),
904-
workspace_id.to_string(),
904+
workspace_id,
905905
"View 2".to_string(),
906906
crate::ViewLayout::Document,
907907
Some(uid),
908908
);
909909
let view_2_id = view_2.id;
910910
let space_view = View {
911911
id: Uuid::parse_str("00000000-0000-0000-0000-000000000001").unwrap(),
912-
parent_view_id: workspace_id.to_string(),
912+
parent_view_id: Some(workspace_id),
913913
name: "Space 1".to_string(),
914914
children: RepeatedViewIdentifier::new(vec![
915915
ViewIdentifier::new(view_1_id),
@@ -969,7 +969,7 @@ mod tests {
969969
.map(|i| {
970970
View::new(
971971
Uuid::parse_str(&format!("00000000-0000-0000-0000-00000000001{}", i)).unwrap(),
972-
space_view_id.to_string(),
972+
space_view_id,
973973
format!("View {:?}", i),
974974
crate::ViewLayout::Document,
975975
Some(uid),
@@ -978,7 +978,7 @@ mod tests {
978978
.collect();
979979
let space_view = View {
980980
id: space_view_id,
981-
parent_view_id: "".to_string(),
981+
parent_view_id: None,
982982
name: "Space".to_string(),
983983
children: RepeatedViewIdentifier::new(
984984
views

collab-folder/src/hierarchy_builder.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ impl NestedChildViewBuilder {
203203
pub fn build(self) -> ParentChildViews {
204204
let view = View {
205205
id: self.view_id,
206-
parent_view_id: self.parent_view_id.to_string(),
206+
parent_view_id: Some(self.parent_view_id),
207207
name: self.name,
208208
created_at: timestamp(),
209209
is_favorite: self.is_favorite,
@@ -297,7 +297,7 @@ impl ParentChildViews {
297297
let indent = " ".repeat(indent_level);
298298
writeln!(
299299
f,
300-
"{}: {}, parent id: {}, layout: {:?}",
300+
"{}: {}, parent id: {:?}, layout: {:?}",
301301
indent, self.view.name, self.view.parent_view_id, self.view.layout,
302302
)?;
303303

collab-folder/src/view.rs

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ impl ViewsMap {
269269
None
270270
}
271271
})
272-
.filter(|view| view.parent_view_id == view.id.to_string())
272+
.filter(|view| view.parent_view_id == Some(view.id))
273273
.collect()
274274
}
275275

@@ -339,13 +339,15 @@ impl ViewsMap {
339339
pub fn insert(&self, txn: &mut TransactionMut, view: View, index: Option<u32>, uid: i64) {
340340
let time = timestamp();
341341

342-
if let Some(parent_map_ref) = self
343-
.container
344-
.get_with_txn::<_, MapRef>(txn, &view.parent_view_id.to_string())
345-
{
342+
if let Some(parent_map_ref) = self.container.get_with_txn::<_, MapRef>(
343+
txn,
344+
&view
345+
.parent_view_id
346+
.map(|v| v.to_string())
347+
.unwrap_or_default(),
348+
) {
346349
let view_identifier = ViewIdentifier { id: view.id };
347-
let parent_view_uuid =
348-
uuid::Uuid::parse_str(&view.parent_view_id).unwrap_or_else(|_| uuid::Uuid::nil());
350+
let parent_view_uuid = view.parent_view_id.unwrap_or_else(Uuid::nil);
349351
let updated_view = ViewUpdate::new(
350352
UserId::from(uid),
351353
&parent_view_uuid,
@@ -380,7 +382,12 @@ impl ViewsMap {
380382
let last_edited_time = self.normalize_timestamp(view.last_edited_time);
381383
update
382384
.set_name(view.name)
383-
.set_bid(&view.parent_view_id)
385+
.set_bid(
386+
view
387+
.parent_view_id
388+
.map(|v| v.to_string())
389+
.unwrap_or_default(),
390+
)
384391
.set_layout(view.layout)
385392
.set_created_at(created_at)
386393
.set_children(view.children)
@@ -513,6 +520,11 @@ pub(crate) fn view_from_map_ref<T: ReadTxn>(
513520
mappings: impl IntoIterator<Item = ViewId>,
514521
) -> Option<View> {
515522
let parent_view_id: String = map_ref.get_with_txn(txn, VIEW_PARENT_ID)?;
523+
let parent_view_id = if parent_view_id.is_empty() {
524+
None
525+
} else {
526+
Uuid::parse_str(&parent_view_id).ok()
527+
};
516528
let id_str: String = map_ref.get_with_txn(txn, FOLDER_VIEW_ID)?;
517529
let id = Uuid::parse_str(&id_str).ok()?;
518530
let name: String = map_ref
@@ -782,9 +794,10 @@ impl<'a, 'b, 'c> ViewUpdate<'a, 'b, 'c> {
782794
#[derive(Serialize, Deserialize, Clone, Debug, Eq, PartialEq)]
783795
pub struct View {
784796
/// The id of the view
785-
pub id: collab_entity::uuid_validation::ViewId,
786-
/// The id for given parent view
787-
pub parent_view_id: String,
797+
pub id: ViewId,
798+
/// The id for given parent view
799+
#[serde(with = "collab::preclude::serde_option_uuid")]
800+
pub parent_view_id: Option<ViewId>,
788801
/// The name that display on the left sidebar
789802
pub name: String,
790803
/// A list of ids, each of them is the id of other view
@@ -815,14 +828,14 @@ pub struct View {
815828
impl View {
816829
pub fn new(
817830
view_id: ViewId,
818-
parent_view_id: String,
831+
parent_view_id: ViewId,
819832
name: String,
820833
layout: ViewLayout,
821834
created_by: Option<i64>,
822835
) -> Self {
823836
Self {
824837
id: view_id,
825-
parent_view_id,
838+
parent_view_id: Some(parent_view_id),
826839
name,
827840
children: Default::default(),
828841
created_at: timestamp(),

collab-folder/src/workspace.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ impl From<Workspace> for View {
4646
fn from(value: Workspace) -> Self {
4747
Self {
4848
id: value.id,
49-
parent_view_id: "".to_string(),
49+
parent_view_id: None,
5050
name: value.name,
5151
children: value.child_views,
5252
created_at: value.created_at,

collab-folder/tests/folder_test/child_views_test.rs

Lines changed: 40 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
1+
use crate::util::{create_folder_with_workspace, make_test_view, parse_view_id};
12
use assert_json_diff::assert_json_include;
3+
use collab_entity::uuid_validation::view_id_from_any_string;
24
use collab_folder::{UserId, timestamp};
35
use serde_json::json;
46

5-
use crate::util::{create_folder_with_workspace, make_test_view, parse_view_id};
6-
77
#[test]
88
fn create_child_views_test() {
99
let uid = UserId::from(1);
10-
let workspace_id = "fake_w_1".to_string();
11-
let folder_test = create_folder_with_workspace(uid.clone(), &workspace_id);
12-
let v_1_1 = make_test_view("1_1", "1", vec![]);
13-
let v_1_2 = make_test_view("1_2", "1", vec![]);
14-
let v_1_2_1 = make_test_view("1_2_1", "1_2", vec![]);
15-
let v_1_2_2 = make_test_view("1_2_2", "1_2", vec![]);
16-
let v_1_3 = make_test_view("1_3", "1", vec![]);
17-
let v_1 = make_test_view("1", &workspace_id, vec![]);
10+
let workspace_id = view_id_from_any_string("fake_w_1");
11+
let folder_test = create_folder_with_workspace(uid.clone(), workspace_id);
12+
let v_1_1 = make_test_view("1_1", view_id_from_any_string("1"), vec![]);
13+
let v_1_2 = make_test_view("1_2", view_id_from_any_string("1"), vec![]);
14+
let v_1_2_1 = make_test_view("1_2_1", view_id_from_any_string("1_2"), vec![]);
15+
let v_1_2_2 = make_test_view("1_2_2", view_id_from_any_string("1_2"), vec![]);
16+
let v_1_3 = make_test_view("1_3", view_id_from_any_string("1"), vec![]);
17+
let v_1 = make_test_view("1", workspace_id, vec![]);
1818

1919
let mut folder = folder_test.folder;
2020
let mut txn = folder.collab.transact_mut();
@@ -57,8 +57,7 @@ fn create_child_views_test() {
5757
.get_views_belong_to(&txn, &v_1_2.id, uid.as_i64());
5858
assert_eq!(v_1_2_child_views.len(), 2);
5959

60-
let workspace_uuid_str =
61-
uuid::Uuid::new_v5(&uuid::Uuid::NAMESPACE_OID, workspace_id.as_bytes()).to_string();
60+
let workspace_uuid_str = workspace_id.to_string();
6261
let folder_data = folder
6362
.body
6463
.get_folder_data(&txn, &workspace_uuid_str, uid.as_i64())
@@ -187,11 +186,13 @@ fn create_child_views_test() {
187186
#[test]
188187
fn move_child_views_test() {
189188
let uid = UserId::from(1);
190-
let folder_test = create_folder_with_workspace(uid.clone(), "w1");
191-
let v_1_1 = make_test_view("1_1", "1", vec![]);
192-
let v_1_2 = make_test_view("1_2", "1", vec![]);
193-
let v_1_3 = make_test_view("1_3", "1", vec![]);
194-
let v_1 = make_test_view("1", "w1", vec![]);
189+
let workspace_id = view_id_from_any_string("w1");
190+
let v_1_id = view_id_from_any_string("1");
191+
let folder_test = create_folder_with_workspace(uid.clone(), workspace_id);
192+
let v_1_1 = make_test_view("1_1", v_1_id, vec![]);
193+
let v_1_2 = make_test_view("1_2", v_1_id, vec![]);
194+
let v_1_3 = make_test_view("1_3", v_1_id, vec![]);
195+
let v_1 = make_test_view("1", workspace_id, vec![]);
195196

196197
let mut folder = folder_test.folder;
197198
let mut txn = folder.collab.transact_mut();
@@ -255,11 +256,12 @@ fn move_child_views_test() {
255256
#[test]
256257
fn delete_view_test() {
257258
let uid = UserId::from(1);
258-
let folder_test = create_folder_with_workspace(uid.clone(), "w1");
259+
let workspace_id = view_id_from_any_string("w1");
260+
let folder_test = create_folder_with_workspace(uid.clone(), workspace_id);
259261
let workspace_id = folder_test.get_workspace_id().unwrap();
260-
let view_1 = make_test_view("1_1", "w1", vec![]);
261-
let view_2 = make_test_view("1_2", "w1", vec![]);
262-
let view_3 = make_test_view("1_3", "w1", vec![]);
262+
let view_1 = make_test_view("1_1", workspace_id, vec![]);
263+
let view_2 = make_test_view("1_2", workspace_id, vec![]);
264+
let view_3 = make_test_view("1_3", workspace_id, vec![]);
263265

264266
let mut folder = folder_test.folder;
265267
let mut txn = folder.collab.transact_mut();
@@ -277,15 +279,11 @@ fn delete_view_test() {
277279
.views
278280
.insert(&mut txn, view_3, None, uid.as_i64());
279281

280-
folder
282+
folder.body.views.remove_child(&mut txn, &workspace_id, 1);
283+
let w_1_child_views = folder
281284
.body
282285
.views
283-
.remove_child(&mut txn, &uuid::Uuid::parse_str(&workspace_id).unwrap(), 1);
284-
let w_1_child_views =
285-
folder
286-
.body
287-
.views
288-
.get_views_belong_to(&txn, &parse_view_id(&workspace_id), uid.as_i64());
286+
.get_views_belong_to(&txn, &workspace_id, uid.as_i64());
289287
assert_eq!(
290288
w_1_child_views[0].id.to_string(),
291289
uuid::Uuid::new_v5(&uuid::Uuid::NAMESPACE_OID, "1_1".as_bytes()).to_string()
@@ -299,12 +297,13 @@ fn delete_view_test() {
299297
#[test]
300298
fn delete_child_view_test() {
301299
let uid = UserId::from(1);
302-
let folder_test = create_folder_with_workspace(uid.clone(), "w1");
303-
let view_1 = make_test_view("v1", "w1", vec![]);
300+
let workspace_id = view_id_from_any_string("w1");
301+
let folder_test = create_folder_with_workspace(uid.clone(), workspace_id);
302+
let view_1 = make_test_view("v1", workspace_id, vec![]);
304303
let view_1_id = view_1.id.to_string();
305-
let view_1_1 = make_test_view("v1_1", "v1", vec![]);
304+
let view_1_1 = make_test_view("v1_1", view_id_from_any_string("v1"), vec![]);
306305
let view_1_1_id = view_1_1.id;
307-
let view_2 = make_test_view("v2", "w1", vec![]);
306+
let view_2 = make_test_view("v2", workspace_id, vec![]);
308307

309308
let mut folder = folder_test.folder;
310309
let mut txn = folder.collab.transact_mut();
@@ -339,12 +338,12 @@ fn delete_child_view_test() {
339338
#[test]
340339
fn create_orphan_child_views_test() {
341340
let uid = UserId::from(1);
342-
let workspace_id = "fake_w_1".to_string();
343-
let folder_test = create_folder_with_workspace(uid.clone(), &workspace_id);
344-
let view_1 = make_test_view("1", &workspace_id, vec![]);
341+
let workspace_id = view_id_from_any_string("fake_w_1");
342+
let folder_test = create_folder_with_workspace(uid.clone(), workspace_id);
343+
let view_1 = make_test_view("1", workspace_id, vec![]);
345344

346345
// The orphan view: the parent_view_id equal to the view_id
347-
let view_2 = make_test_view("2", "2", vec![]);
346+
let view_2 = make_test_view("2", view_id_from_any_string("2"), vec![]);
348347

349348
let mut folder = folder_test.folder;
350349
let mut txn = folder.collab.transact_mut();
@@ -358,13 +357,11 @@ fn create_orphan_child_views_test() {
358357
.views
359358
.insert(&mut txn, view_2.clone(), None, uid.as_i64());
360359

361-
let workspace_uuid_str =
362-
uuid::Uuid::new_v5(&uuid::Uuid::NAMESPACE_OID, workspace_id.as_bytes()).to_string();
363-
let child_views =
364-
folder
365-
.body
366-
.views
367-
.get_views_belong_to(&txn, &parse_view_id(&workspace_uuid_str), uid.as_i64());
360+
let workspace_uuid_str = workspace_id.to_string();
361+
let child_views = folder
362+
.body
363+
.views
364+
.get_views_belong_to(&txn, &workspace_id, uid.as_i64());
368365
assert_eq!(child_views.len(), 1);
369366

370367
let orphan_views = folder

collab-folder/tests/folder_test/custom_section.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
1+
use crate::util::create_folder_with_workspace;
12
use assert_json_diff::assert_json_include;
23
use collab::preclude::Any;
4+
use collab_entity::uuid_validation::view_id_from_any_string;
35
use collab_folder::{Section, SectionItem, UserId, timestamp};
46
use serde_json::json;
57
use std::collections::HashMap;
68
use std::sync::Arc;
79

8-
use crate::util::create_folder_with_workspace;
9-
1010
#[test]
1111
fn custom_section_test() {
1212
let uid = UserId::from(1);
13-
let folder_test = create_folder_with_workspace(uid.clone(), "w1");
13+
let folder_test = create_folder_with_workspace(uid.clone(), view_id_from_any_string("w1"));
1414

1515
let mut folder = folder_test.folder;
1616
let mut txn = folder.collab.transact_mut();

0 commit comments

Comments
 (0)