diff --git a/crates/engine/src/cmd/track.rs b/crates/engine/src/cmd/track.rs index f76664a0..9300a295 100644 --- a/crates/engine/src/cmd/track.rs +++ b/crates/engine/src/cmd/track.rs @@ -704,15 +704,26 @@ fn delete(e: &mut Engine, p: &Value) -> Result { fn rename(e: &mut Engine, p: &Value) -> Result { let t = track_param(e, "track.rename", p, "track")?.ok_or_else(|| bad("track.rename", "`track` required"))?; let name = str_param(p, "name").map(str::trim).filter(|n| !n.is_empty()).ok_or_else(|| bad("track.rename", "`name` required"))?; - let s = e.session_mut(); + // Validate the stored name, including its existing 64-character limit. + let name: String = name.chars().take(64).collect(); + let s = e.session(); if s.tracks.iter().any(|x| x.name == name && x.id != t) { return Err(bad("track.rename", format!("a track named `{name}` already exists"))); } - let name: String = name.chars().take(64).collect(); let bus = s.track(t).and_then(|track| match (track.kind, &track.mixer.input) { (TrackKind::Folder, Route::Bus(bus)) => Some(*bus), _ => None, }); + let track = s.track(t).ok_or_else(|| bad("track.rename", "unknown track"))?; + if track.name == name + && bus.is_none_or(|id| { + let expected = unique_folder_bus_name(s, &format!("{name} Bus"), Some(id)); + s.bus(id).is_none_or(|bus| bus.name == expected) + }) + { + return Ok(json!({"name": name})); + } + let s = e.session_mut(); if let Some(track) = s.track_mut(t) { track.name.clone_from(&name); } diff --git a/crates/engine/tests/track_name_collisions.rs b/crates/engine/tests/track_name_collisions.rs new file mode 100644 index 00000000..cff5b277 --- /dev/null +++ b/crates/engine/tests/track_name_collisions.rs @@ -0,0 +1,173 @@ +use serde_json::json; +use soundcraft_engine::Engine; +use soundcraft_model::{ChannelFormat, Session, TrackKind}; +use std::sync::Arc; + +#[test] +fn truncated_ascii_and_unicode_names_cannot_collide_with_another_track() { + for character in ['A', '界'] { + let existing: String = std::iter::repeat_n(character, 64).collect(); + let mut session = Session::default(); + let occupied = session.add_track(TrackKind::Audio, ChannelFormat::Stereo, Some(&existing)); + let target = session.add_track(TrackKind::Aux, ChannelFormat::Mono, Some("Target")); + let mut engine = Engine::new(session); + let before = engine.session_arc(); + let revision = engine.revision; + + let result = engine.execute("track.rename", &json!({"track": target.0, "name": format!(" {existing} extra ")})); + assert!(result.is_err()); + assert!(Arc::ptr_eq(&before, &engine.session_arc())); + assert_eq!(engine.revision, revision); + assert_eq!(engine.session().track(occupied).unwrap().name, existing); + assert_eq!(engine.session().track(target).unwrap().name, "Target"); + assert!(!engine.is_dirty()); + assert!(!engine.can_undo()); + } +} + +#[test] +fn rename_returns_the_stored_unicode_name_and_undo_restores_the_document() { + let mut session = Session::default(); + let track = session.add_track(TrackKind::Audio, ChannelFormat::Surround51, Some("Original")); + session.track_mut(track).unwrap().comments = "Retain this comment".into(); + let mut engine = Engine::new(session); + let before = engine.session().to_json().unwrap(); + let stored = "é".repeat(64); + + let result = engine.execute("track.rename", &json!({"track": track.0, "name": format!(" {stored}extra ")})).unwrap(); + assert_eq!(result["name"], stored); + let renamed = engine.session().track(track).unwrap(); + assert_eq!(renamed.name, stored); + assert_eq!(renamed.name.chars().count(), 64); + assert_eq!(renamed.id, track); + assert_eq!(renamed.format, ChannelFormat::Surround51); + assert_eq!(renamed.comments, "Retain this comment"); + assert_eq!(engine.undo_history(), ["Rename Track"]); + assert!(engine.undo()); + assert_eq!(engine.session().to_json().unwrap(), before); + assert!(engine.redo()); + assert_eq!(engine.session().track(track).unwrap().name, stored); +} + +#[test] +fn unchanged_name_after_truncation_does_not_make_an_undo_step() { + let name = "A".repeat(64); + let mut session = Session::default(); + let track = session.add_track(TrackKind::Audio, ChannelFormat::Mono, Some(&name)); + let mut engine = Engine::new(session); + let before = engine.session_arc(); + let revision = engine.revision; + + let result = engine.execute("track.rename", &json!({"track": track.0, "name": format!("{name} suffix")})).unwrap(); + assert_eq!(result["name"], name); + assert!(Arc::ptr_eq(&before, &engine.session_arc())); + assert_eq!(engine.revision, revision); + assert!(!engine.is_dirty()); + assert!(!engine.can_undo()); +} + +#[test] +fn routing_folder_rename_preserves_unique_owned_bus_routes_and_history() { + use soundcraft_model::{BusId, Route, TrackId}; + + let stored = "界".repeat(64); + let first_name = format!("{stored} Bus"); + let second_name = format!("{stored} Bus 2"); + let mut session = Session::default(); + let member = session.add_track(TrackKind::Audio, ChannelFormat::Stereo, Some("Member")); + let user_bus = session.add_bus(&first_name, ChannelFormat::Stereo); + let second_user_bus = session.add_bus(&second_name, ChannelFormat::Stereo); + let mut engine = Engine::new(session); + let unrelated = engine.execute("track.new", &json!({"kind": "folder", "name": "Unrelated"})).unwrap(); + let unrelated = TrackId(unrelated["tracks"][0].as_u64().unwrap()); + let unrelated_route = engine.session().track(unrelated).unwrap().mixer.input.clone(); + let created = engine.execute("track.move_to_new_folder", &json!({"tracks": [member.0], "name": "Initial"})).unwrap(); + let folder = TrackId(created["folder"].as_u64().unwrap()); + let owned_bus = BusId(created["bus"].as_u64().unwrap()); + let before = engine.session().to_json().unwrap(); + let history = engine.undo_history(); + let bus_count = engine.session().busses.len(); + + let result = engine.execute("track.rename", &json!({"track": folder.0, "name": format!(" {stored} suffix ")})).unwrap(); + assert_eq!(result["name"], stored); + assert_eq!(engine.session().track(folder).unwrap().name, stored); + assert_eq!(engine.session().bus(owned_bus).unwrap().name, format!("{stored} Bus 3")); + assert_eq!(engine.session().bus(user_bus).unwrap().name, first_name); + assert_eq!(engine.session().bus(second_user_bus).unwrap().name, second_name); + assert_eq!(engine.session().track(unrelated).unwrap().mixer.input, unrelated_route); + assert_eq!(engine.session().track(folder).unwrap().mixer.input, Route::Bus(owned_bus)); + assert_eq!(engine.session().track(member).unwrap().mixer.output, Route::Bus(owned_bus)); + assert_eq!(engine.session().track(member).unwrap().folder, Some(folder)); + assert_eq!(engine.session().busses.len(), bus_count); + assert_eq!(engine.undo_history().len(), history.len() + 1); + assert_eq!(engine.undo_label(), Some("Rename Track")); + let renamed = engine.session().to_json().unwrap(); + + let document = engine.session_arc(); + let revision = engine.revision; + let history = engine.undo_history(); + engine.execute("track.rename", &json!({"track": folder.0, "name": format!("{stored} more")})).unwrap(); + assert!(Arc::ptr_eq(&document, &engine.session_arc())); + assert_eq!(engine.revision, revision); + assert_eq!(engine.undo_history(), history); + assert!(engine.undo()); + assert_eq!(engine.session().to_json().unwrap(), before); + assert!(engine.redo()); + assert_eq!(engine.session().to_json().unwrap(), renamed); + + // Current-main same-name folder rename also repairs the folder's own bus, without taking + // either colliding user bus or changing its members' bus identity. + engine.session_mut().busses.iter_mut().find(|bus| bus.id == owned_bus).unwrap().name = "Manually named owned bus".into(); + let manual = engine.session().to_json().unwrap(); + engine.execute("track.rename", &json!({"track": folder.0, "name": stored})).unwrap(); + assert_eq!(engine.session().bus(owned_bus).unwrap().name, format!("{stored} Bus 3")); + assert_eq!(engine.session().bus(user_bus).unwrap().name, first_name); + assert_eq!(engine.session().bus(second_user_bus).unwrap().name, second_name); + assert_eq!(engine.session().track(member).unwrap().mixer.output, Route::Bus(owned_bus)); + assert!(engine.undo()); + assert_eq!(engine.session().to_json().unwrap(), manual); + assert!(engine.redo()); + assert_eq!(engine.session().to_json().unwrap(), renamed); +} + +#[test] +fn truncated_folder_collision_refuses_bus_and_document_changes_with_redo_pending() { + use soundcraft_model::{Route, TrackId}; + + for character in ['B', '界'] { + let occupied_name: String = std::iter::repeat_n(character, 64).collect(); + let mut session = Session::default(); + let occupied = session.add_track(TrackKind::Audio, ChannelFormat::Stereo, Some(&occupied_name)); + let mut engine = Engine::new(session); + let created = engine.execute("track.new", &json!({"kind": "folder", "name": "Target"})).unwrap(); + let folder = TrackId(created["tracks"][0].as_u64().unwrap()); + let owned_bus = match engine.session().track(folder).unwrap().mixer.input { + Route::Bus(bus) => bus, + _ => panic!("the routing folder must own a bus"), + }; + engine.execute("mix.volume", &json!({"track": folder.0, "db": -3})).unwrap(); + assert!(engine.undo()); + engine.mark_clean(); + let document = engine.session_arc(); + let before = engine.session().to_json().unwrap(); + let revision = engine.revision; + let journal = engine.journal.clone(); + let undo = engine.undo_history(); + let redo = engine.redo_label().map(str::to_owned); + assert!(redo.is_some()); + + let result = engine.execute("track.rename", &json!({"track": folder.0, "name": format!(" {occupied_name} suffix ")})); + assert!(result.unwrap_err().to_string().contains(&occupied_name)); + assert!(Arc::ptr_eq(&document, &engine.session_arc())); + assert_eq!(engine.session().to_json().unwrap(), before); + assert_eq!(engine.revision, revision); + assert_eq!(engine.journal, journal); + assert_eq!(engine.undo_history(), undo); + assert_eq!(engine.redo_label(), redo.as_deref()); + assert!(!engine.is_dirty()); + assert_eq!(engine.session().track(occupied).unwrap().name, occupied_name); + assert_eq!(engine.session().track(folder).unwrap().name, "Target"); + assert_eq!(engine.session().bus(owned_bus).unwrap().name, "Target Bus"); + assert_eq!(engine.session().track(folder).unwrap().mixer.input, Route::Bus(owned_bus)); + } +}