diff --git a/crates/mountmate-app/src/main.rs b/crates/mountmate-app/src/main.rs index ee19647..c51d7dc 100644 --- a/crates/mountmate-app/src/main.rs +++ b/crates/mountmate-app/src/main.rs @@ -4180,13 +4180,20 @@ impl App { let Some(id) = draft.editing_id.clone() else { return Task::none(); }; - let tags = match validated_connection_tags(&draft.tags, self.locale()) { - Ok(tags) => tags, - Err(error) => { - self.status = error; - return Task::none(); - } - }; + let existing_tags = self + .servers + .iter() + .find(|server| server.id == id) + .map(|server| server.tags.as_slice()); + let tags = + match validated_connection_tags_for_existing(&draft.tags, existing_tags, self.locale()) + { + Ok(tags) => tags, + Err(error) => { + self.status = error; + return Task::none(); + } + }; let auto_mount_at_login = draft.auto_mount_at_login && draft.connection_method != ConnectionMethod::Interactive; self.editor_saving = true; @@ -8344,19 +8351,54 @@ fn connection_preference_updates( } fn validated_connection_tags(tags: &[String], locale: Locale) -> Result, String> { + validated_connection_tags_for_existing(tags, None, locale) +} + +fn validated_connection_tags_for_existing( + tags: &[String], + existing: Option<&[String]>, + locale: Locale, +) -> Result, String> { let mut normalized = Vec::new(); for tag in tags { - let tag = normalized_tag_name(tag, locale)?; + let tag = match normalized_tag_name(tag, locale) { + Ok(tag) => tag, + Err(error) => { + let tag = tag.trim(); + let is_existing_overlong = tag.chars().count() > MAX_TAG_CHARS + && !tag.chars().any(char::is_control) + && !tag.contains(',') + && !tag.contains('，') + && existing.is_some_and(|existing| existing.iter().any(|item| item == tag)); + if !is_existing_overlong { + return Err(error); + } + tag.to_owned() + } + }; if !normalized.iter().any(|candidate| candidate == &tag) { normalized.push(tag); } } - if normalized.len() > MAX_CONNECTION_TAGS { + let preserves_existing = existing.is_some_and(|existing| { + mountmate_core::model::tag_update_only_preserves_existing(&normalized, existing) + }); + if normalized.len() > MAX_CONNECTION_TAGS && !preserves_existing { return Err(match locale { Locale::English => format!("A connection may have at most {MAX_CONNECTION_TAGS} tags"), Locale::Chinese => format!("一个连接最多只能有 {MAX_CONNECTION_TAGS} 个标签"), }); } + if normalized + .iter() + .any(|tag| tag.chars().count() > MAX_TAG_CHARS) + && !preserves_existing + { + return Err(match locale { + Locale::English => format!("A tag must be at most {MAX_TAG_CHARS} characters"), + Locale::Chinese => format!("标签最多只能有 {MAX_TAG_CHARS} 个字符"), + }); + } Ok(normalized) } @@ -11385,6 +11427,25 @@ mod localization_tests { ) .is_err() ); + + let legacy_long = "界".repeat(MAX_TAG_CHARS + 1); + assert_eq!( + validated_connection_tags_for_existing( + std::slice::from_ref(&legacy_long), + Some(std::slice::from_ref(&legacy_long)), + Locale::English, + ) + .unwrap(), + vec![legacy_long.clone()] + ); + assert!( + validated_connection_tags_for_existing( + &[legacy_long, "new-tag".into()], + Some(std::slice::from_ref(&"界".repeat(MAX_TAG_CHARS + 1))), + Locale::English, + ) + .is_err() + ); } #[test] diff --git a/crates/mountmate-core/src/connection.rs b/crates/mountmate-core/src/connection.rs index 18b2009..b9f1955 100644 --- a/crates/mountmate-core/src/connection.rs +++ b/crates/mountmate-core/src/connection.rs @@ -5,6 +5,7 @@ use thiserror::Error; use crate::model::{ MAX_CONNECTION_TAGS, MAX_TAG_CHARS, normalize_port, normalize_tags, sanitize_id, + tag_update_only_preserves_existing, }; use crate::mountpoint::HOME_MOUNTPOINT_VALUE; use crate::{AuthMethod, ConnectionMethod, ServerConfig}; @@ -306,7 +307,13 @@ impl ConnectionDraft { pub fn validate(&self, servers: &[ServerConfig]) -> Result { let requirements = self.requirements(); let name = required_display_name(&self.name)?; - let tags = validate_tags(&self.tags, &self.folder)?; + let tags = validate_tags( + &self.tags, + &self.folder, + self.existing + .as_ref() + .map(|server| (server.tags.as_slice(), server.folder.as_str())), + )?; let folder = tags.first().cloned().unwrap_or_default(); let host = required_scalar(&self.host, "IP/Host")?; let user = required_scalar(&self.user, "User")?; @@ -575,7 +582,11 @@ fn required_display_name(value: &str) -> Result { Ok(value.into()) } -fn validate_tags(tags: &[String], legacy_folder: &str) -> Result, DraftError> { +fn validate_tags( + tags: &[String], + legacy_folder: &str, + existing: Option<(&[String], &str)>, +) -> Result, DraftError> { if tags.iter().any(|tag| tag.chars().any(char::is_control)) || legacy_folder.chars().any(char::is_control) { @@ -583,12 +594,21 @@ fn validate_tags(tags: &[String], legacy_folder: &str) -> Result, Dr } let mut normalized = tags.to_vec(); normalize_tags(&mut normalized, legacy_folder); - if normalized.len() > MAX_CONNECTION_TAGS { + let existing_normalized = existing.map(|(tags, folder)| { + let mut tags = tags.to_vec(); + normalize_tags(&mut tags, folder); + tags + }); + let preserves_existing = existing_normalized + .as_ref() + .is_some_and(|existing| tag_update_only_preserves_existing(&normalized, existing)); + if normalized.len() > MAX_CONNECTION_TAGS && !preserves_existing { return Err(DraftError::TooManyTags(MAX_CONNECTION_TAGS)); } if normalized .iter() .any(|tag| tag.chars().count() > MAX_TAG_CHARS) + && !preserves_existing { return Err(DraftError::TagTooLong(MAX_TAG_CHARS)); } @@ -1232,16 +1252,48 @@ mod tests { .map(|index| format!("tag-{index}")) .collect::>(); assert_eq!( - validate_tags(&too_many, ""), + validate_tags(&too_many, "", None), Err(DraftError::TooManyTags(crate::model::MAX_CONNECTION_TAGS)) ); let too_long = vec!["界".repeat(crate::model::MAX_TAG_CHARS + 1)]; assert_eq!( - validate_tags(&too_long, ""), + validate_tags(&too_long, "", None), Err(DraftError::TagTooLong(crate::model::MAX_TAG_CHARS)) ); } + #[test] + fn legacy_tag_limits_allow_unrelated_edits_and_progressive_cleanup() { + let mut existing = password_server(); + existing.tags = (0..=crate::model::MAX_CONNECTION_TAGS) + .map(|index| format!("tag-{index}")) + .collect(); + existing.folder = existing.tags[0].clone(); + + let unchanged = ConnectionDraft::from_server(&existing); + assert!(unchanged.validate(std::slice::from_ref(&existing)).is_ok()); + + let mut cleanup = ConnectionDraft::from_server(&existing); + cleanup.tags.pop(); + assert!(cleanup.validate(std::slice::from_ref(&existing)).is_ok()); + + let mut addition = ConnectionDraft::from_server(&existing); + addition.tags.push("new-tag".into()); + assert_eq!( + addition.validate(std::slice::from_ref(&existing)), + Err(DraftError::TooManyTags(crate::model::MAX_CONNECTION_TAGS)) + ); + + let mut overlong = password_server(); + overlong.tags = vec!["界".repeat(crate::model::MAX_TAG_CHARS + 1)]; + overlong.folder = overlong.tags[0].clone(); + assert!( + ConnectionDraft::from_server(&overlong) + .validate(std::slice::from_ref(&overlong)) + .is_ok() + ); + } + #[test] fn refreshing_an_ssh_import_preserves_user_folder() { let mut draft = ConnectionDraft { diff --git a/crates/mountmate-core/src/model.rs b/crates/mountmate-core/src/model.rs index 7c9f1f2..ace6b97 100644 --- a/crates/mountmate-core/src/model.rs +++ b/crates/mountmate-core/src/model.rs @@ -11,6 +11,10 @@ pub const MAX_VFS_UPLOAD_TRANSFERS: u16 = 32; pub const MAX_CONNECTION_TAGS: usize = 8; pub const MAX_TAG_CHARS: usize = 24; +pub fn tag_update_only_preserves_existing(candidate: &[String], existing: &[String]) -> bool { + candidate.iter().all(|tag| existing.contains(tag)) +} + fn default_port() -> String { "22".into() } diff --git a/crates/mountmate-core/src/storage.rs b/crates/mountmate-core/src/storage.rs index f0a3f37..f6a509e 100644 --- a/crates/mountmate-core/src/storage.rs +++ b/crates/mountmate-core/src/storage.rs @@ -201,6 +201,10 @@ pub fn update_server_preferences_batch( } } for update in updates { + let server = servers + .iter_mut() + .find(|server| server.id == update.id) + .expect("validated connection ID"); if update .tags .as_ref() @@ -214,7 +218,11 @@ pub fn update_server_preferences_batch( if let Some(tags) = &update.tags { let mut normalized_tags = tags.clone(); crate::model::normalize_tags(&mut normalized_tags, ""); - if normalized_tags.len() > crate::model::MAX_CONNECTION_TAGS { + let mut existing_tags = server.tags.clone(); + crate::model::normalize_tags(&mut existing_tags, &server.folder); + let preserves_existing = + crate::model::tag_update_only_preserves_existing(&normalized_tags, &existing_tags); + if normalized_tags.len() > crate::model::MAX_CONNECTION_TAGS && !preserves_existing { return Err(StorageError::InvalidPreferenceUpdate(format!( "connection {} may have at most {} tags", update.id, @@ -224,6 +232,7 @@ pub fn update_server_preferences_batch( if normalized_tags .iter() .any(|tag| tag.chars().count() > crate::model::MAX_TAG_CHARS) + && !preserves_existing { return Err(StorageError::InvalidPreferenceUpdate(format!( "tags for connection {} must be at most {} Unicode characters each", @@ -232,10 +241,6 @@ pub fn update_server_preferences_batch( ))); } } - let server = servers - .iter_mut() - .find(|server| server.id == update.id) - .expect("validated connection ID"); if let Some(mut tags) = update.tags.clone() { crate::model::normalize_tags(&mut tags, ""); server.tags = tags; @@ -1125,6 +1130,56 @@ mod tests { ); } + #[test] + fn batch_preferences_allow_progressive_cleanup_of_legacy_tag_limits() { + let temp = tempdir().unwrap(); + let paths = AppPaths { + config_dir: temp.path().join("config"), + cache_dir: temp.path().join("cache"), + state_dir: temp.path().join("state"), + data_dir: temp.path().join("data"), + }; + let legacy_tags = (0..=crate::model::MAX_CONNECTION_TAGS) + .map(|index| format!("tag-{index}")) + .collect::>(); + save_servers( + &paths, + &[ServerConfig { + id: "alpha".into(), + folder: legacy_tags[0].clone(), + tags: legacy_tags.clone(), + ..ServerConfig::default() + }], + ) + .unwrap(); + + let reduced = legacy_tags[..legacy_tags.len() - 1].to_vec(); + let updated = update_server_preferences_batch( + &paths, + &[ServerPreferenceUpdate { + id: "alpha".into(), + tags: Some(reduced.clone()), + auto_mount_at_login: None, + }], + ) + .unwrap(); + assert_eq!(updated[0].tags, reduced); + + let mut invalid_addition = updated[0].tags.clone(); + invalid_addition.push("new-tag".into()); + assert!( + update_server_preferences_batch( + &paths, + &[ServerPreferenceUpdate { + id: "alpha".into(), + tags: Some(invalid_addition), + auto_mount_at_login: None, + }] + ) + .is_err() + ); + } + #[test] fn batch_remove_requires_existing_unique_ids_and_preserves_order() { let temp = tempdir().unwrap();