diff --git a/backend/sync_server/src/server/update_document.rs b/backend/sync_server/src/server/update_document.rs index 809a7ce..414180b 100644 --- a/backend/sync_server/src/server/update_document.rs +++ b/backend/sync_server/src/server/update_document.rs @@ -8,7 +8,6 @@ use axum_extra::{ use axum_jsonschema::Json; use chrono::{DateTime, Utc}; use log::info; -use regex::Regex; use schemars::JsonSchema; use serde::Deserialize; use sync_lib::{base64_to_bytes, is_file_type_mergable, merge}; @@ -20,12 +19,9 @@ use super::{ responses::DocumentUpdateResponse, }; use crate::{ - database::{ - self, Transaction, - models::{DocumentId, StoredDocumentVersion, VaultId, VaultUpdateId}, - }, + database::models::{DocumentId, StoredDocumentVersion, VaultId, VaultUpdateId}, errors::{SyncServerError, client_error, not_found_error, server_error}, - utils::sanitize_path, + utils::{deduped_file_paths, sanitize_path}, }; // This is required for aide to infer the path parameter types and names @@ -172,13 +168,21 @@ async fn internal_update_document( let new_relative_path = if parent_document.relative_path == latest_version.relative_path && latest_version.relative_path != sanitized_relative_path { - get_deduped_file_name( - &state.database, - &vault_id, - &mut transaction, - &sanitized_relative_path, - ) - .await? + let mut new_relative_path = Default::default(); + for candidate in deduped_file_paths(&sanitized_relative_path) { + if state + .database + .get_latest_document_by_path(&vault_id, &candidate, Some(&mut transaction)) + .await + .map_err(server_error)? + .is_none() + { + new_relative_path = candidate; + break; + } + } + + new_relative_path } else { latest_version.relative_path.clone() }; @@ -212,53 +216,3 @@ async fn internal_update_document( DocumentUpdateResponse::FastForwardUpdate(new_version.into()) })) } - -// Only a single file can be on the same path, so we need to dedup the path -// in case the client is trying to rename the file to an existing file's name -// that it's unaware of. -async fn get_deduped_file_name( - database: &database::Database, - vault_id: &VaultId, - transaction: &mut Transaction<'_>, - path: &str, -) -> Result { - let parts = path.rsplitn(2, '.').collect::>(); - let mut reverse_parts = parts.into_iter().rev(); - let (stem, extension) = match (reverse_parts.next(), reverse_parts.next()) { - (Some(stem), maybe_extension) => ( - stem, - maybe_extension - .map(|ext| format!(".{ext}")) - .unwrap_or_default(), - ), - _ => unreachable!("Path must have at least one part"), - }; - - let regex = Regex::new(r" \((\d+)\)$").unwrap(); - let start_number = regex - .captures(stem) - .and_then(|caps| caps.get(1)) - .and_then(|m| m.as_str().parse::().ok()) - .unwrap_or(0); - - let clean_stem = regex.replace(stem, "").to_string(); - - for dedup_number in start_number.. { - let proposed_path = if dedup_number == 0 { - format!("{clean_stem}{extension}") - } else { - format!("{clean_stem} ({dedup_number}){extension}") - }; - - if database - .get_latest_document_by_path(vault_id, &proposed_path, Some(transaction)) - .await - .map_err(server_error)? - .is_none() - { - return Ok(proposed_path.to_string()); - } - } - - unreachable!("Loop must always return a value"); -} diff --git a/backend/sync_server/src/utils.rs b/backend/sync_server/src/utils.rs index 8718944..03839c2 100644 --- a/backend/sync_server/src/utils.rs +++ b/backend/sync_server/src/utils.rs @@ -1,3 +1,5 @@ +use regex::Regex; + /// Sanitize the document's path to allow all clients to create the same path in /// their filesystem. If we didn't do this server-side, client's would need to /// deal with mapping invalid names to valid ones and then back. @@ -21,6 +23,45 @@ pub fn sanitize_path(path: &str) -> String { .join("/") } +pub fn deduped_file_paths(path: &str) -> impl Iterator { + let mut path_parts = path.split('/').collect::>(); + let file_name = path_parts.pop().unwrap().to_owned(); + + let mut directory = path_parts.join("/"); + if !directory.is_empty() { + directory.push('/'); + } + + let name_parts = file_name.rsplitn(2, '.').collect::>(); + let mut reverse_parts = name_parts.into_iter().rev(); + let (stem, extension) = match (reverse_parts.next(), reverse_parts.next()) { + (Some(stem), maybe_extension) => ( + stem.to_owned(), + maybe_extension + .map(|ext| format!(".{ext}")) + .unwrap_or_default(), + ), + _ => unreachable!("Path must have at least one part"), + }; + + let regex = Regex::new(r" \((\d+)\)$").unwrap(); + let start_number = regex + .captures(&stem) + .and_then(|caps| caps.get(1)) + .and_then(|m| m.as_str().parse::().ok()) + .unwrap_or(0); + + let clean_stem = regex.replace(&stem, "").to_string(); + + (start_number..).map(move |dedup_number| { + if dedup_number == 0 { + format!("{directory}{clean_stem}{extension}") + } else { + format!("{directory}{clean_stem} ({dedup_number}){extension}") + } + }) +} + #[cfg(test)] mod test { use super::*; @@ -30,4 +71,47 @@ mod test { assert_eq!(sanitize_path("/my/path/what?"), "/my/path/what"); assert_eq!(sanitize_path("/my/path/\\\\:?"), "/my/path/_"); } + + #[test] + fn test_deduped_file_paths() { + let mut deduped = deduped_file_paths("file.txt"); + assert_eq!(deduped.next(), Some("file.txt".to_owned())); + assert_eq!(deduped.next(), Some("file (1).txt".to_owned())); + assert_eq!(deduped.next(), Some("file (2).txt".to_owned())); + + let mut deduped = deduped_file_paths("file"); + assert_eq!(deduped.next(), Some("file".to_owned())); + assert_eq!(deduped.next(), Some("file (1)".to_owned())); + assert_eq!(deduped.next(), Some("file (2)".to_owned())); + + let mut deduped = deduped_file_paths("file (51).md"); + assert_eq!(deduped.next(), Some("file (51).md".to_owned())); + assert_eq!(deduped.next(), Some("file (52).md".to_owned())); + assert_eq!(deduped.next(), Some("file (53).md".to_owned())); + + let mut deduped = deduped_file_paths("file (5)"); + assert_eq!(deduped.next(), Some("file (5)".to_owned())); + assert_eq!(deduped.next(), Some("file (6)".to_owned())); + assert_eq!(deduped.next(), Some("file (7)".to_owned())); + + let mut deduped = deduped_file_paths("my/path.with.dots/file (5).md"); + assert_eq!( + deduped.next(), + Some("my/path.with.dots/file (5).md".to_owned()) + ); + assert_eq!( + deduped.next(), + Some("my/path.with.dots/file (6).md".to_owned()) + ); + + let mut deduped = deduped_file_paths("my/path.with.dots/file (5)"); + assert_eq!( + deduped.next(), + Some("my/path.with.dots/file (5)".to_owned()) + ); + assert_eq!( + deduped.next(), + Some("my/path.with.dots/file (6)".to_owned()) + ); + } }