diff --git a/src/web/api.rs b/src/web/api.rs index 41007fd1..f278a063 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -159,6 +159,56 @@ pub struct ApiService { suppressed_paths: Arc>>, } +/// Derives the directory name from a standalone script's filesystem path. +/// +/// Uses `file_stem()` with script suffix stripping instead of the decoded +/// instance name, so that Windows-invalid character encoding (e.g., `%3F` +/// for `?`) is preserved. +/// +/// Examples: +/// - `What%3F.server.luau` → `What%3F` +/// - `MyModule.luau` → `MyModule` +/// - `Handler.client.lua` → `Handler` +fn dir_name_from_script_path(standalone_path: &Path) -> &str { + let file_stem = standalone_path + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or(""); + file_stem + .strip_suffix(".server") + .or_else(|| file_stem.strip_suffix(".client")) + .or_else(|| file_stem.strip_suffix(".plugin")) + .or_else(|| file_stem.strip_suffix(".local")) + .or_else(|| file_stem.strip_suffix(".legacy")) + .unwrap_or(file_stem) +} + +/// Derives the directory name from a standalone non-script instance's filesystem path. +/// +/// Strips compound extensions (`.model.json5`, `.model.json`) or falls back +/// to `file_stem()` for single-extension types (`.txt`, `.csv`, etc.). +/// +/// Examples: +/// - `What%3F.model.json5` → `What%3F` +/// - `MyPart.model.json` → `MyPart` +/// - `Greeting.txt` → `Greeting` +/// - `Translations.csv` → `Translations` +fn dir_name_from_instance_path(standalone_path: &Path) -> &str { + let file_name = standalone_path + .file_name() + .and_then(|f| f.to_str()) + .unwrap_or(""); + file_name + .strip_suffix(".model.json5") + .or_else(|| file_name.strip_suffix(".model.json")) + .unwrap_or_else(|| { + standalone_path + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or(file_name) + }) +} + impl ApiService { pub fn new(serve_session: Arc) -> Self { let suppressed_paths = serve_session.suppressed_paths(); @@ -921,7 +971,7 @@ impl ApiService { fn convert_standalone_script_to_directory( &self, standalone_path: &std::path::Path, - script_name: &str, + _script_name: &str, class_name: &str, containing_dir: &std::path::Path, ) -> anyhow::Result { @@ -934,8 +984,10 @@ impl ApiService { String::new() }; + let dir_name = dir_name_from_script_path(standalone_path); + // Create the directory - let new_dir = containing_dir.join(script_name); + let new_dir = containing_dir.join(dir_name); self.suppress_path(&new_dir); fs::create_dir_all(&new_dir).with_context(|| { format!( @@ -970,7 +1022,7 @@ impl ApiService { } // Move adjacent meta file into directory if it exists - let meta_path = containing_dir.join(format!("{}.meta.json5", script_name)); + let meta_path = containing_dir.join(format!("{}.meta.json5", dir_name)); if meta_path.exists() { let init_meta_path = new_dir.join("init.meta.json5"); self.suppress_path(&meta_path); @@ -1012,7 +1064,9 @@ impl ApiService { ) -> anyhow::Result { use anyhow::Context; - let new_dir = containing_dir.join(instance_name); + let dir_name = dir_name_from_instance_path(standalone_path); + + let new_dir = containing_dir.join(dir_name); self.suppress_path(&new_dir); fs::create_dir_all(&new_dir).with_context(|| { format!( @@ -5140,4 +5194,163 @@ mod tests { ); } } + + mod dir_name_from_script_path_tests { + use super::*; + + #[test] + fn module_script_luau() { + let path = Path::new("src/MyModule.luau"); + assert_eq!(dir_name_from_script_path(path), "MyModule"); + } + + #[test] + fn server_script_luau() { + let path = Path::new("src/Handler.server.luau"); + assert_eq!(dir_name_from_script_path(path), "Handler"); + } + + #[test] + fn client_script_luau() { + let path = Path::new("src/UI.client.luau"); + assert_eq!(dir_name_from_script_path(path), "UI"); + } + + #[test] + fn local_script_luau() { + let path = Path::new("src/Input.local.luau"); + assert_eq!(dir_name_from_script_path(path), "Input"); + } + + #[test] + fn plugin_script_luau() { + let path = Path::new("src/MyPlugin.plugin.luau"); + assert_eq!(dir_name_from_script_path(path), "MyPlugin"); + } + + #[test] + fn legacy_script_luau() { + let path = Path::new("src/OldCode.legacy.luau"); + assert_eq!(dir_name_from_script_path(path), "OldCode"); + } + + #[test] + fn legacy_lua_extension() { + let path = Path::new("src/OldModule.lua"); + assert_eq!(dir_name_from_script_path(path), "OldModule"); + } + + #[test] + fn legacy_server_lua() { + let path = Path::new("src/Main.server.lua"); + assert_eq!(dir_name_from_script_path(path), "Main"); + } + + #[test] + fn legacy_client_lua() { + let path = Path::new("src/Client.client.lua"); + assert_eq!(dir_name_from_script_path(path), "Client"); + } + + #[test] + fn encoded_windows_chars_module() { + // ? is invalid on Windows, encoded as %3F + let path = Path::new("src/What%3F.luau"); + assert_eq!(dir_name_from_script_path(path), "What%3F"); + } + + #[test] + fn encoded_windows_chars_server() { + let path = Path::new("src/What%3F.server.luau"); + assert_eq!(dir_name_from_script_path(path), "What%3F"); + } + + #[test] + fn encoded_colon_char() { + // : is invalid on Windows, encoded as %3A + let path = Path::new("src/Key%3AValue.client.luau"); + assert_eq!(dir_name_from_script_path(path), "Key%3AValue"); + } + + #[test] + fn multiple_dots_in_name() { + // Name like "my.module.luau" — file_stem is "my.module", no suffix match + let path = Path::new("src/my.module.luau"); + assert_eq!(dir_name_from_script_path(path), "my.module"); + } + + #[test] + fn name_ending_with_suffix_substring() { + // "MyServer" should NOT be stripped to "My" — ".server" stripping only + // applies to the suffix after file_stem strips ".luau" + let path = Path::new("src/MyServer.luau"); + assert_eq!(dir_name_from_script_path(path), "MyServer"); + } + } + + mod dir_name_from_instance_path_tests { + use super::*; + + #[test] + fn model_json5() { + let path = Path::new("src/MyPart.model.json5"); + assert_eq!(dir_name_from_instance_path(path), "MyPart"); + } + + #[test] + fn model_json_legacy() { + let path = Path::new("src/MyPart.model.json"); + assert_eq!(dir_name_from_instance_path(path), "MyPart"); + } + + #[test] + fn txt_file() { + let path = Path::new("src/Greeting.txt"); + assert_eq!(dir_name_from_instance_path(path), "Greeting"); + } + + #[test] + fn csv_file() { + let path = Path::new("src/Translations.csv"); + assert_eq!(dir_name_from_instance_path(path), "Translations"); + } + + #[test] + fn encoded_windows_chars_model() { + let path = Path::new("src/What%3F.model.json5"); + assert_eq!(dir_name_from_instance_path(path), "What%3F"); + } + + #[test] + fn encoded_windows_chars_txt() { + let path = Path::new("src/Ask%3F.txt"); + assert_eq!(dir_name_from_instance_path(path), "Ask%3F"); + } + + #[test] + fn encoded_colon_model() { + let path = Path::new("src/Key%3AValue.model.json5"); + assert_eq!(dir_name_from_instance_path(path), "Key%3AValue"); + } + + #[test] + fn encoded_colon_legacy_model() { + let path = Path::new("src/Key%3AValue.model.json"); + assert_eq!(dir_name_from_instance_path(path), "Key%3AValue"); + } + + #[test] + fn multiple_dots_in_name_model() { + // "my.part.model.json5" — should strip ".model.json5" + let path = Path::new("src/my.part.model.json5"); + assert_eq!(dir_name_from_instance_path(path), "my.part"); + } + + #[test] + fn unknown_extension_uses_file_stem() { + // Fallback to file_stem for unrecognized extensions + let path = Path::new("src/Something.toml"); + assert_eq!(dir_name_from_instance_path(path), "Something"); + } + } } diff --git a/tests/tests/two_way_sync.rs b/tests/tests/two_way_sync.rs index c2f98386..4e4d22b2 100644 --- a/tests/tests/two_way_sync.rs +++ b/tests/tests/two_way_sync.rs @@ -634,3 +634,435 @@ fn echo_suppression_prevents_redundant_patches() { ); }); } + +// --------------------------------------------------------------------------- +// Helpers (syncback_format_transitions fixture) +// --------------------------------------------------------------------------- + +/// Look up an instance by name under ReplicatedStorage in the format_transitions fixture. +fn get_format_transitions_instance( + session: &crate::rojo_test::serve_util::TestServeSession, + instance_name: &str, +) -> (librojo::SessionId, Ref) { + let info = session.get_api_rojo().unwrap(); + let root_read = session.get_api_read(info.root_instance_id).unwrap(); + let (rs_id, _) = find_by_class(&root_read.instances, "ReplicatedStorage"); + let rs_read = session.get_api_read(rs_id).unwrap(); + let (instance_id, _) = find_by_name(&rs_read.instances, instance_name); + (info.session_id, instance_id) +} + +// --------------------------------------------------------------------------- +// Tests: Directory-format operations +// --------------------------------------------------------------------------- + +/// Test 13: Renaming a directory-format script renames the parent directory, +/// not the init file inside it. Children should move with it. +#[test] +fn rename_directory_format_script() { + run_serve_test("syncback_format_transitions", |session, _redactions| { + let (session_id, dir_module_id) = + get_format_transitions_instance(&session, "DirModuleWithChildren"); + + let old_dir = session.path().join("src").join("DirModuleWithChildren"); + let new_dir = session.path().join("src").join("RenamedDirModule"); + + assert!(old_dir.is_dir(), "Old directory should exist before rename"); + + send_update( + &session, + &session_id, + InstanceUpdate { + id: dir_module_id, + changed_name: Some("RenamedDirModule".to_string()), + changed_class_name: None, + changed_properties: UstrMap::default(), + changed_metadata: None, + }, + ); + + assert!( + !old_dir.exists(), + "Old directory should be gone after rename: {}", + old_dir.display() + ); + assert!( + new_dir.is_dir(), + "New directory should exist after rename: {}", + new_dir.display() + ); + + // init.luau should be inside the renamed directory + let init_file = new_dir.join("init.luau"); + assert_file_exists(&init_file, "init.luau inside renamed directory"); + + // Children should also be present in the renamed directory + let child_a = new_dir.join("ChildA.luau"); + assert_file_exists(&child_a, "ChildA.luau inside renamed directory"); + }); +} + +/// Test 14: Changing ClassName on a directory-format ModuleScript to Script +/// renames the init file inside the directory (init.luau → init.server.luau). +#[test] +fn classname_change_directory_module_to_script() { + run_serve_test("syncback_format_transitions", |session, _redactions| { + let (session_id, dir_module_id) = + get_format_transitions_instance(&session, "DirModuleWithChildren"); + + let dir = session.path().join("src").join("DirModuleWithChildren"); + let old_init = dir.join("init.luau"); + let new_init = dir.join("init.server.luau"); + + assert_file_exists(&old_init, "init.luau before class change"); + assert_not_exists(&new_init, "init.server.luau before class change"); + + send_update( + &session, + &session_id, + InstanceUpdate { + id: dir_module_id, + changed_name: None, + changed_class_name: Some(ustr("Script")), + changed_properties: UstrMap::default(), + changed_metadata: None, + }, + ); + + assert_not_exists(&old_init, "init.luau after class change to Script"); + assert_file_exists(&new_init, "init.server.luau after class change to Script"); + + // Children should be unaffected + let child_a = dir.join("ChildA.luau"); + assert_file_exists(&child_a, "ChildA.luau unaffected by class change"); + }); +} + +// --------------------------------------------------------------------------- +// Tests: Reverse ClassName transitions (code paths added in these commits) +// --------------------------------------------------------------------------- + +/// Test 15: Script → ModuleScript ClassName transition (reverse of test 6). +/// Validates that the .server suffix is correctly stripped. +#[test] +fn classname_change_script_to_module() { + run_serve_test("syncback_format_transitions", |session, _redactions| { + let (session_id, script_id) = + get_format_transitions_instance(&session, "StandaloneScript"); + + let old_path = session + .path() + .join("src") + .join("StandaloneScript.server.luau"); + let new_path = session.path().join("src").join("StandaloneScript.luau"); + + assert_file_exists(&old_path, "Script file before class change"); + + send_update( + &session, + &session_id, + InstanceUpdate { + id: script_id, + changed_name: None, + changed_class_name: Some(ustr("ModuleScript")), + changed_properties: UstrMap::default(), + changed_metadata: None, + }, + ); + + assert_not_exists(&old_path, "Script file after class change to ModuleScript"); + assert_file_exists(&new_path, "ModuleScript file after class change"); + }); +} + +/// Test 16: Script → LocalScript ClassName transition. +/// Validates that .server is replaced with .local. +#[test] +fn classname_change_script_to_localscript() { + run_serve_test("syncback_format_transitions", |session, _redactions| { + let (session_id, script_id) = + get_format_transitions_instance(&session, "StandaloneScript"); + + let old_path = session + .path() + .join("src") + .join("StandaloneScript.server.luau"); + let new_path = session + .path() + .join("src") + .join("StandaloneScript.local.luau"); + + assert_file_exists(&old_path, "Script file before class change"); + + send_update( + &session, + &session_id, + InstanceUpdate { + id: script_id, + changed_name: None, + changed_class_name: Some(ustr("LocalScript")), + changed_properties: UstrMap::default(), + changed_metadata: None, + }, + ); + + assert_not_exists(&old_path, "Script file after class change to LocalScript"); + assert_file_exists(&new_path, "LocalScript file after class change"); + }); +} + +/// Test 17: LocalScript → Script ClassName transition. +/// Exercises .client suffix handling (the fixture uses .client.luau for LocalScript). +#[test] +fn classname_change_localscript_to_script() { + run_serve_test("syncback_format_transitions", |session, _redactions| { + let (session_id, local_id) = + get_format_transitions_instance(&session, "StandaloneLocalScript"); + + let old_path = session + .path() + .join("src") + .join("StandaloneLocalScript.client.luau"); + let new_path = session + .path() + .join("src") + .join("StandaloneLocalScript.server.luau"); + + assert_file_exists(&old_path, "LocalScript file before class change"); + + send_update( + &session, + &session_id, + InstanceUpdate { + id: local_id, + changed_name: None, + changed_class_name: Some(ustr("Script")), + changed_properties: UstrMap::default(), + changed_metadata: None, + }, + ); + + assert_not_exists(&old_path, "LocalScript file after class change to Script"); + assert_file_exists(&new_path, "Script file after class change"); + }); +} + +// --------------------------------------------------------------------------- +// Tests: Combined operations (overridden_source_path logic) +// --------------------------------------------------------------------------- + +/// Test 18: Combined rename + Source update in a single request. +/// Validates that Source is written to the RENAMED file, not the old location. +/// This exercises the `overridden_source_path` tracking in ChangeProcessor. +#[test] +fn combined_rename_and_source_update() { + run_serve_test("syncback_write", |session, _redactions| { + let (session_id, _rs_id, existing_id) = get_rs_and_existing(&session); + + let old_path = session.path().join("src").join("existing.luau"); + let new_path = session.path().join("src").join("CombinedRename.luau"); + + assert_file_exists(&old_path, "existing.luau before combined update"); + + let mut props = UstrMap::default(); + props.insert( + ustr("Source"), + Some(Variant::String("-- Written after rename".to_string())), + ); + send_update( + &session, + &session_id, + InstanceUpdate { + id: existing_id, + changed_name: Some("CombinedRename".to_string()), + changed_class_name: None, + changed_properties: props, + changed_metadata: None, + }, + ); + + assert_not_exists(&old_path, "Old file after combined rename+source"); + assert_file_exists(&new_path, "Renamed file after combined rename+source"); + + let content = fs::read_to_string(&new_path).unwrap(); + assert!( + content.contains("Written after rename"), + "Source should be written to the NEW file location, got: {}", + content + ); + }); +} + +/// Test 19: Combined ClassName change + Source update in a single request. +/// Validates that Source is written to the file with the NEW extension. +#[test] +fn combined_classname_and_source_update() { + run_serve_test("syncback_write", |session, _redactions| { + let (session_id, _rs_id, existing_id) = get_rs_and_existing(&session); + + let old_path = session.path().join("src").join("existing.luau"); + let new_path = session.path().join("src").join("existing.server.luau"); + + assert_file_exists(&old_path, "existing.luau before combined update"); + + let mut props = UstrMap::default(); + props.insert( + ustr("Source"), + Some(Variant::String( + "-- Source after class change".to_string(), + )), + ); + send_update( + &session, + &session_id, + InstanceUpdate { + id: existing_id, + changed_name: None, + changed_class_name: Some(ustr("Script")), + changed_properties: props, + changed_metadata: None, + }, + ); + + assert_not_exists(&old_path, "ModuleScript file after ClassName+Source"); + assert_file_exists(&new_path, "Script file after ClassName+Source"); + + let content = fs::read_to_string(&new_path).unwrap(); + assert!( + content.contains("Source after class change"), + "Source should be in the new file, got: {}", + content + ); + }); +} + +// --------------------------------------------------------------------------- +// Tests: Standalone → directory conversion & suffix-aware removal +// --------------------------------------------------------------------------- + +/// Test 20: Adding a child instance to a standalone script converts it to +/// directory format (e.g., StandaloneModule.luau → StandaloneModule/init.luau). +/// This exercises `convert_standalone_script_to_directory` in ApiService. +#[test] +fn add_child_converts_standalone_to_directory() { + run_serve_test("syncback_format_transitions", |session, _redactions| { + let info = session.get_api_rojo().unwrap(); + let root_read = session.get_api_read(info.root_instance_id).unwrap(); + let (rs_id, _) = find_by_class(&root_read.instances, "ReplicatedStorage"); + let rs_read = session.get_api_read(rs_id).unwrap(); + let (standalone_id, _) = find_by_name(&rs_read.instances, "StandaloneModule"); + + let standalone_file = session.path().join("src").join("StandaloneModule.luau"); + let dir_path = session.path().join("src").join("StandaloneModule"); + + assert_file_exists(&standalone_file, "Standalone file before child add"); + assert!( + !dir_path.exists(), + "Directory should NOT exist before child add" + ); + + // Add a child ModuleScript to StandaloneModule + let mut properties = HashMap::new(); + properties.insert( + "Source".to_string(), + Variant::String("-- New child module".to_string()), + ); + let added = AddedInstance { + parent: Some(standalone_id), + name: "NewChild".to_string(), + class_name: "ModuleScript".to_string(), + properties, + children: vec![], + }; + let mut added_map = HashMap::new(); + added_map.insert(Ref::new(), added); + let write_request = WriteRequest { + session_id: info.session_id, + removed: vec![], + added: added_map, + updated: vec![], + }; + session.post_api_write(&write_request).unwrap(); + thread::sleep(Duration::from_millis(500)); + + // Standalone file should be gone, replaced by directory + assert_not_exists( + &standalone_file, + "Standalone file after child add (converted to directory)", + ); + assert!( + dir_path.is_dir(), + "Directory should exist after conversion: {}", + dir_path.display() + ); + + // init.luau should contain the original source + let init_file = dir_path.join("init.luau"); + assert_file_exists(&init_file, "init.luau after standalone→directory conversion"); + + let init_content = fs::read_to_string(&init_file).unwrap(); + assert!( + init_content.contains("Standalone ModuleScript"), + "init.luau should contain original source, got: {}", + init_content + ); + + // Child should be created inside the directory + let child_file = dir_path.join("NewChild.luau"); + assert_file_exists(&child_file, "NewChild.luau inside converted directory"); + }); +} + +/// Test 21: Removing a Script (with .server suffix) properly cleans up +/// the adjacent meta file by stripping the script suffix to find the +/// correct meta file name (e.g., StandaloneScript.meta.json5). +#[test] +fn remove_script_with_suffix_cleans_meta() { + run_serve_test("syncback_format_transitions", |session, _redactions| { + let (session_id, script_id) = + get_format_transitions_instance(&session, "StandaloneScript"); + + let script_path = session + .path() + .join("src") + .join("StandaloneScript.server.luau"); + let meta_path = session + .path() + .join("src") + .join("StandaloneScript.meta.json5"); + + assert_file_exists(&script_path, "Script file before operations"); + + // First, create a meta file by sending a non-Source property update + let mut attrs = rbx_dom_weak::types::Attributes::new(); + attrs.insert("ScriptAttr".to_string(), Variant::Float64(7.0)); + let mut props = UstrMap::default(); + props.insert(ustr("Attributes"), Some(Variant::Attributes(attrs))); + send_update( + &session, + &session_id, + InstanceUpdate { + id: script_id, + changed_name: None, + changed_class_name: None, + changed_properties: props, + changed_metadata: None, + }, + ); + + assert_file_exists(&meta_path, "Meta file after property update"); + + // Re-read ID in case tree was rebuilt after the update + let (session_id, script_id) = + get_format_transitions_instance(&session, "StandaloneScript"); + + // Now remove the instance — both script and meta should be deleted + send_removal(&session, &session_id, vec![script_id]); + + assert_not_exists(&script_path, "Script file after removal"); + assert_not_exists( + &meta_path, + "Meta file after removal (suffix-stripped cleanup)", + ); + }); +}