Refactor file renaming logic in JobThreadContext to improve path encoding

- Updated the renaming logic to utilize encoded path names for both directories and files, ensuring compatibility with encoded names and improving accuracy in file handling.
- Enhanced comments for clarity on the naming resolution process, particularly in relation to script suffixes and metadata file management.
- Improved the overall robustness of file path operations during instance updates, addressing potential issues with name resolution.
This commit is contained in:
Joe 2026-02-05 20:32:41 -06:00
parent 033d8c6892
commit e33cf7ae77

View file

@ -11,6 +11,7 @@ use std::{
use crate::{ use crate::{
message_queue::MessageQueue, message_queue::MessageQueue,
path_encoding::encode_path_name,
snapshot::{ snapshot::{
apply_patch_set, compute_patch_set, AppliedPatchSet, InstigatingSource, PatchSet, RojoTree, apply_patch_set, compute_patch_set, AppliedPatchSet, InstigatingSource, PatchSet, RojoTree,
}, },
@ -499,7 +500,6 @@ impl JobThreadContext {
match instigating_source { match instigating_source {
InstigatingSource::Path(path) => { InstigatingSource::Path(path) => {
if path.exists() { if path.exists() {
let old_name = instance.name();
let file_name = let file_name =
path.file_name().and_then(|f| f.to_str()).unwrap_or(""); path.file_name().and_then(|f| f.to_str()).unwrap_or("");
@ -512,13 +512,18 @@ impl JobThreadContext {
if is_init_file { if is_init_file {
let dir_path = path.parent().unwrap(); let dir_path = path.parent().unwrap();
if let Some(grandparent) = dir_path.parent() { if let Some(grandparent) = dir_path.parent() {
// Use the encoded dir name from the filesystem
// for the old meta lookup, and encode the new
// name for the new path. Using instance.name()
// (decoded) would fail for encoded names.
let dir_name = dir_path let dir_name = dir_path
.file_name() .file_name()
.and_then(|f| f.to_str()) .and_then(|f| f.to_str())
.unwrap_or(""); .unwrap_or("");
let new_dir_name = let encoded_new_name =
dir_name.replacen(old_name, new_name, 1); encode_path_name(new_name);
let new_dir_path = grandparent.join(&new_dir_name); let new_dir_path =
grandparent.join(&encoded_new_name);
if new_dir_path != dir_path { if new_dir_path != dir_path {
log::info!( log::info!(
@ -541,11 +546,14 @@ impl JobThreadContext {
Some(new_dir_path.join(file_name)); Some(new_dir_path.join(file_name));
let old_meta = grandparent.join(format!( let old_meta = grandparent.join(format!(
"{}.meta.json5", "{}.meta.json5",
old_name dir_name
)); ));
if old_meta.exists() { if old_meta.exists() {
let new_meta = grandparent.join( let new_meta = grandparent.join(
format!("{}.meta.json5", new_name), format!(
"{}.meta.json5",
encoded_new_name
),
); );
let _ = let _ =
fs::rename(&old_meta, &new_meta); fs::rename(&old_meta, &new_meta);
@ -554,8 +562,48 @@ impl JobThreadContext {
} }
} }
} else if let Some(parent) = path.parent() { } else if let Some(parent) = path.parent() {
let new_file_name = // Derive the suffix from the existing filename
file_name.replacen(old_name, new_name, 1); // rather than using replacen with the decoded
// instance name, which fails for encoded names
// (e.g., What%QUESTION%.server.luau).
let extension = path
.extension()
.and_then(|e| e.to_str())
.unwrap_or("");
let stem = path
.file_stem()
.and_then(|s| s.to_str())
.unwrap_or("");
let known_suffixes = [
".server", ".client", ".plugin", ".local",
".legacy",
];
let script_suffix = known_suffixes
.iter()
.find(|s| stem.ends_with(*s))
.copied()
.unwrap_or("");
let old_base = if script_suffix.is_empty() {
stem
} else {
&stem[..stem.len() - script_suffix.len()]
};
let encoded_new_name =
encode_path_name(new_name);
let new_file_name = if extension.is_empty() {
format!(
"{}{}",
encoded_new_name, script_suffix
)
} else {
format!(
"{}{}.{}",
encoded_new_name,
script_suffix,
extension
)
};
let new_path = parent.join(&new_file_name); let new_path = parent.join(&new_file_name);
if new_path != *path { if new_path != *path {
@ -573,12 +621,14 @@ impl JobThreadContext {
); );
} else { } else {
overridden_source_path = Some(new_path.clone()); overridden_source_path = Some(new_path.clone());
let old_meta = parent let old_meta = parent.join(format!(
.join(format!("{}.meta.json5", old_name)); "{}.meta.json5",
old_base
));
if old_meta.exists() { if old_meta.exists() {
let new_meta = parent.join(format!( let new_meta = parent.join(format!(
"{}.meta.json5", "{}.meta.json5",
new_name encoded_new_name
)); ));
let _ = fs::rename(&old_meta, &new_meta); let _ = fs::rename(&old_meta, &new_meta);
} }
@ -807,10 +857,34 @@ fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: Ref) -> Option<
// file/folder in the first place. // file/folder in the first place.
match instigating_source { match instigating_source {
InstigatingSource::Path(path) => { InstigatingSource::Path(path) => {
// For directory-format scripts, the instigating_source is the
// init file (e.g., init.luau) so that two-way sync writes to the
// correct file. However, snapshot_from_vfs returns None for init
// files because they are handled as part of the parent directory
// snapshot. Detect this case and snapshot the parent directory.
let is_init_file = path
.file_name()
.and_then(|f| f.to_str())
.map(|f| f.starts_with("init."))
.unwrap_or(false);
let snapshot_path = if is_init_file {
path.parent().unwrap_or(path.as_path())
} else {
path.as_path()
};
log::info!( log::info!(
"compute_and_apply_changes: checking path {} for instance {:?}", "compute_and_apply_changes: checking path {} for instance {:?}{}",
path.display(), path.display(),
id id,
if is_init_file {
format!(
" (init file, will snapshot parent dir {})",
snapshot_path.display()
)
} else {
String::new()
}
); );
match vfs.metadata(path).with_not_found() { match vfs.metadata(path).with_not_found() {
@ -820,16 +894,17 @@ fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: Ref) -> Option<
// that path and use it as the source for our patch. // that path and use it as the source for our patch.
log::info!( log::info!(
"compute_and_apply_changes: path EXISTS via VFS, re-snapshotting {}", "compute_and_apply_changes: path EXISTS via VFS, re-snapshotting {}",
path.display() snapshot_path.display()
); );
let snapshot = match snapshot_from_vfs(&metadata.context, vfs, path) { let snapshot =
Ok(snapshot) => snapshot, match snapshot_from_vfs(&metadata.context, vfs, snapshot_path) {
Err(err) => { Ok(snapshot) => snapshot,
log::error!("Snapshot error: {:?}", err); Err(err) => {
return None; log::error!("Snapshot error: {:?}", err);
} return None;
}; }
};
let patch_set = compute_patch_set(snapshot, tree, id); let patch_set = compute_patch_set(snapshot, tree, id);
let applied = apply_patch_set(tree, patch_set); let applied = apply_patch_set(tree, patch_set);
@ -852,17 +927,18 @@ fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: Ref) -> Option<
path.display() path.display()
); );
let snapshot = match snapshot_from_vfs(&metadata.context, vfs, path) { let snapshot =
Ok(snapshot) => snapshot, match snapshot_from_vfs(&metadata.context, vfs, snapshot_path) {
Err(err) => { Ok(snapshot) => snapshot,
log::error!( Err(err) => {
"Recovery snapshot error for {}: {:?}", log::error!(
path.display(), "Recovery snapshot error for {}: {:?}",
err snapshot_path.display(),
); err
return None; );
} return None;
}; }
};
let patch_set = compute_patch_set(snapshot, tree, id); let patch_set = compute_patch_set(snapshot, tree, id);
let applied = apply_patch_set(tree, patch_set); let applied = apply_patch_set(tree, patch_set);