Skip to content

feat: return transaction for workspace database operation #342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
appflowy merged 1 commit into from
Dec 5, 2024
Merged

feat: return transaction for workspace database operation #342

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions collab-database/src/workspace_database/body.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl WorkspaceDatabase {
/// Create a new [DatabaseMeta] for the given database id and view id
/// use [Self::update_database] to attach more views to the existing database.
///
pub fn add_database(&mut self, database_id: &str, view_ids: Vec<String>) {
pub fn add_database(&mut self, database_id: &str, view_ids: Vec<String>) -> TransactionMut {
Copy link
Collaborator

@Horusiath Horusiath Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to move add_database logic to body and provide transaction from outside. What if you want to call this method, while you already had transaction acquired? This would cause a deadlock.

let mut txn = self.collab.transact_mut();
let linked_views: HashSet<String> = view_ids.into_iter().collect();
let record = DatabaseMeta {
Expand All @@ -65,6 +65,7 @@ impl WorkspaceDatabase {
linked_views: linked_views.into_iter().collect(),
};
self.body.push_back(&mut txn, record);
txn
}

pub fn batch_add_database(
Expand All @@ -85,17 +86,21 @@ impl WorkspaceDatabase {
}

/// Update the database by the given id
pub fn update_database(&mut self, database_id: &str, f: impl FnMut(&mut DatabaseMeta)) {
self
.body
.update_database(&mut self.collab.transact_mut(), database_id, f);
pub fn update_database(
&mut self,
database_id: &str,
f: impl FnMut(&mut DatabaseMeta),
) -> TransactionMut {
let mut txn = self.collab.transact_mut();
self.body.update_database(&mut txn, database_id, f);
txn
}

/// Delete the database by the given id
pub fn delete_database(&mut self, database_id: &str) {
self
.body
.delete_database(&mut self.collab.transact_mut(), database_id);
pub fn delete_database(&mut self, database_id: &str) -> TransactionMut {
let mut txn = self.collab.transact_mut();
self.body.delete_database(&mut txn, database_id);
Copy link
Collaborator

@Horusiath Horusiath Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can see here, you don't need to return transaction. You can just call self.collab.transact_mut + self.body.delete_database. Because both self.collab and self.body are split in every collab type, creating transaction should never be a problem.

txn
}

/// Test if the database with the given id exists
Expand Down
Loading