refactor rename_note: change callback result type, include check if nodename is available

diff --git a/application/backend/src/backend.rs b/application/backend/src/backend.rs
index 53d563988ebe677f1cc8c4cbd6ff8d24e9b16a41..33d98a758b002fe3d0d610eefde506511bd674f2 100644 (file)
--- a/application/backend/src/backend.rs
+++ b/application/backend/src/backend.rs
@@ -30,7 +30,7 @@ use std::future::Future;
 use std::io::Error;
 use std::path::{Path, PathBuf};
 use std::pin::Pin;
-use sea_orm::{ActiveModelTrait, Database, DatabaseConnection, EntityTrait, QueryFilter, ColumnTrait, Set, QueryOrder, DbErr, ExprTrait, UpdateResult, QuerySelect};
+use sea_orm::{ActiveModelTrait, Database, DatabaseConnection, EntityTrait, QueryFilter, ColumnTrait, Set, QueryOrder, DbErr, ExprTrait, UpdateResult, QuerySelect, TransactionTrait};
 use tokio::runtime::Runtime;
 use std::sync::{Arc};
 use std::thread::JoinHandle;
@@ -680,55 +680,99 @@ impl BackendHandle {
     }
 
     pub fn rename_note<F>(&self, note_id: i32, nodename: Option<String>, title: Option<String>, callback: F)
-    where F: FnOnce(Result<UpdateResult, DbErr>) + Send + 'static {
+    where F: FnOnce(RenameNoteRet) + Send + 'static {
         let bhandle = self.clone();
         let cmd = Box::pin(async move {
-            let local_path_res = get_note_storage_path(&bhandle.backend.db, note_id).await;
-            let local_path = match local_path_res {
-                Ok(path) => path,
-                Err(e) => {
-                    callback(Err(e));
-                    return;
+            let result: Result<RenameNoteRet, DbErr> = async {
+                // get the note's parent collection_id
+                let collection_id = note::Entity::find_by_id(note_id)
+                    .select_only().column(note::Column::CollectionId)
+                    .into_tuple::<i32>().one(&bhandle.backend.db).await?;
+                let Some(collection_id) = collection_id else {
+                    return Err(DbErr::RecordNotFound("Note not found".to_string()));
+                };
+
+                let local_path_res = get_note_storage_path(&bhandle.backend.db, note_id).await?;
+
+                // prepare update
+                let mut update = Note::update_many();
+
+                // rename?
+                if let Some(nodename) = &nodename {
+                    // check if another note with the new nodename exists
+                    let exists = note::Entity::find()
+                        .filter(note::Column::CollectionId.eq(collection_id))
+                        .filter(note::Column::Nodename.eq(nodename))
+                        .select_only()
+                        .column(note::Column::NoteId)
+                        .into_tuple::<i32>()
+                        .one(&bhandle.backend.db).await?;
+                    if exists.is_some() {
+                        return Ok(RenameNoteRet::NameAlreadyExists)
+                    }
+
+                    // if local storage for the note is enabled, check if a file with the
+                    // new name already exists
+                    if let Some(path) = &local_path_res {
+                        let p = Path::new(&path);
+                        if let Some(parent) = p.parent() {
+                            let new_path = parent.join(&nodename);
+                            let m = fs::metadata(Path::new(new_path.as_path())).await;
+                            if m.is_ok() {
+                                return Ok(RenameNoteRet::NameAlreadyExists) // file exists
+                            }
+                        }
+                    }
+
+                    // ok
+                    update = update.col_expr(Column::Nodename, Expr::value(nodename.clone()));
                 }
-            };
 
-            let mut update = Note::update_many();
-            if let Some(nodename ) = &nodename {
-                update = update.col_expr(Column::Nodename, Expr::value(nodename.clone()));
-            }
-            if let Some(title ) = &title {
-                update = update.col_expr(Column::Title, Expr::value(title)).col_expr(Column::FixedTitle, Expr::value(true));
-            }
+                if let Some(title ) = &title {
+                    update = update.col_expr(Column::Title, Expr::value(title)).col_expr(Column::FixedTitle, Expr::value(true));
+                }
 
-            let result = update
+                let tx = bhandle.backend.db.begin().await?;
+
+                let result = update
                     .filter(Column::NoteId.eq(note_id))
-                    .exec(&bhandle.backend.db).await;
+                    .exec(&bhandle.backend.db).await?;
+                if result.rows_affected != 1 {
+                    return Err(DbErr::Custom("Unexpected number of rows updated".to_string()))
+                }
 
-            if let Some(nodename) = nodename && let Some(path_str) = local_path {
-                let path = Path::new(&path_str);
-                let parent = path.parent();
-                if let Some(parent) = parent {
-                    let new_path = parent.join(nodename);
-                    let result = fs::rename(path, &new_path).await;
-                    if let Err(e) = result {
-                        callback(Err(DbErr::Custom(format!("rename failed: {}", e))));
-                        // TODO: rollback nodename update
-                        return;
-                    }
-                } // else: should not happen
-            }
+                if let Some(nodename) = nodename && let Some(path_str) = &local_path_res {
+                    let path = Path::new(&path_str);
+                    let parent = path.parent();
+                    if let Some(parent) = parent {
+                        let new_path = parent.join(nodename);
+                        let result = fs::rename(path, &new_path).await;
+                        if let Err(e) = result {
+                            return Ok(RenameNoteRet::FileError(e))
+                        }
+                    } // else: should not happen
+                }
+                
+                tx.commit().await?;
+                Ok(RenameNoteRet::Ok)
+            }.await;
 
-            callback(result);
+            let res = match result {
+                Ok(r) => r,
+                Err(e) => RenameNoteRet::Error(e)
+            };
+
+            callback(res);
         });
         let _ = self.tx.send(cmd);
     }
 
-    pub fn check_nodename<F>(&self, collection_id: i32, note_id: i32, nodename: &str, callback: F)
+    pub fn check_nodename<F>(&self, note_id: i32, nodename: &str, callback: F)
     where F: FnOnce(Result<bool, DbErr>) + Send + 'static {
         let bhandle = self.clone();
         let name = nodename.to_string();
         let cmd = Box::pin(async move {
-            let result = check_free_nodename(&bhandle.backend.db, collection_id, note_id, name.as_str()).await;
+            let result = check_free_nodename(&bhandle.backend.db, note_id, name.as_str()).await;
             callback(result);
         });
         let _ = self.tx.send(cmd);

diff --git a/application/backend/src/storage.rs b/application/backend/src/storage.rs
index 681fe855ff0d18c31e48aa67e43df1b710d95f10..59694a708ce63152e47ad5335370ad453aa2a7c2 100644 (file)
--- a/application/backend/src/storage.rs
+++ b/application/backend/src/storage.rs
@@ -137,7 +137,15 @@ pub async fn write_tmp_file(name: &str, data: Vec<u8>) -> std::io::Result<std::p
     Ok(path)
 }
 
-pub async fn check_free_nodename(db: &DatabaseConnection, collection_id: i32, note_id: i32, nodename: &str) -> Result<bool, DbErr> {
+pub async fn check_free_nodename(db: &DatabaseConnection, note_id: i32, nodename: &str) -> Result<bool, DbErr> {
+    let collection_id = note::Entity::find_by_id(note_id)
+        .select_only().column(note::Column::CollectionId)
+        .into_tuple::<i32>().one(db).await?;
+
+    let Some(collection_id) = collection_id else {
+        return Err(DbErr::RecordNotFound("Note not found".to_string()));
+    };
+
     let exists = note::Entity::find()
             .filter(note::Column::CollectionId.eq(collection_id))
             .filter(note::Column::Nodename.eq(nodename))

diff --git a/application/note/src/note.rs b/application/note/src/note.rs
index d71d7d3689d560c6c537b571aef544cff7358854..2ba7f84d66d184171213ad60b8e1aa3dba88a342 100644 (file)
--- a/application/note/src/note.rs
+++ b/application/note/src/note.rs
@@ -31,7 +31,7 @@ use std::sync::atomic::{AtomicU64, Ordering};
 use std::time::SystemTime;
 use sea_orm::{NotSet, Set};
 use sea_orm::sea_query::prelude::Utc;
-use backend::backend::{BackendHandle, BroadcastMessage, NoteContentRet, NoteId, NoteTitleUpdate, SaveNoteResult};
+use backend::backend::{BackendHandle, BroadcastMessage, NoteContentRet, NoteId, NoteTitleUpdate, RenameNoteRet, SaveNoteResult};
 use backend::lockmanager::NoteLock;
 use entity::note::NoteType;
 use ui_rs::{action, dialog, doc_cast, ui_actions, UiModel};
@@ -489,7 +489,7 @@ impl Note {
                     // Rename OK
                     // check if the new nodename is available
                     let new_nodename = new_name.clone();
-                    backend.check_nodename(collection_id, note_id, new_name.as_str(), move|result|{
+                    backend.check_nodename(note_id, new_name.as_str(), move|result|{
                         proxy.call_mainthread(move|doc, note| {
                             match result {
                                 Ok(available) => {
@@ -498,15 +498,17 @@ impl Note {
                                         note.backend.rename_note(note_id, Some(new_nodename.clone()), None, |result|{
                                             proxy.call_mainthread(|_doc, note|{
                                                 match result {
-                                                    Ok(updated) => {
-                                                        if updated.rows_affected != 1 {
-                                                            eprintln!("rename_note: unexpected number of rows affected");
-                                                        } else {
-                                                            note.nodename = Some(new_nodename);
-                                                        }
+                                                    RenameNoteRet::Ok => {
+                                                        note.nodename = Some(new_nodename);
                                                     },
-                                                    Err(e) => {
+                                                    RenameNoteRet::NameAlreadyExists => {
+                                                        // TODO
+                                                    },
+                                                    RenameNoteRet::Error(e) => {
                                                         eprintln!("note update failed: {:?}", e);
+                                                    },
+                                                    RenameNoteRet::FileError(e) => {
+                                                        eprintln!("cannot rename file: {:?}", e);
                                                     }
                                                 }
                                             });