Let the naming scheme organize files into subfolders
- rename::rename_one now resolves new_name relative to the source file's
own directory, allowing / as a subdirectory separator (e.g. a naming
scheme like {show name}/{episode name}) instead of rejecting it. Missing
intermediate directories are created automatically via create_dir_all.
- New validate_relative_target() replaces the old is_plain_filename():
still rejects anything that could escape the source directory (empty
path segments - which also catches a leading-slash absolute path -
and . / .. segments) plus backslashes (Renamr always treats / as the
scheme's folder separator regardless of OS), but now allows a
legitimate multi-segment relative path through.
- The "Unchanged" check now compares the full resolved target path to the
source, not just the filename, since a computed name can now include
directory components.
- 6 new tests: organizing into a new subfolder, nested subfolders that
don't exist yet, conflict detection still works for organized targets,
and rejection of backslashes / doubled / leading / trailing slashes.
- Frontend: naming-scheme editor hint explains the / convention, and the
rename confirmation modal notes when a plan will create new subfolders.
- No changes needed in the naming-expression engine itself - it already
treats / between two {} scriptlets as ordinary literal text.
This commit is contained in:
@@ -1,20 +1,30 @@
|
||||
//! The on-disk rename operation: given a planned `old path -> new filename` mapping,
|
||||
//! actually renames the files.
|
||||
//! The on-disk rename operation: given a planned `old path -> new relative target`
|
||||
//! mapping, actually renames (and, when the target includes subfolders, moves) the
|
||||
//! files.
|
||||
//!
|
||||
//! Deliberately scoped for v1: same-directory renames only (no moving files into a
|
||||
//! different folder tree/organizing by naming scheme yet - that's a bigger feature with
|
||||
//! its own directory-creation and cross-filesystem-move concerns). Conflict policy is
|
||||
//! fixed at **skip**: if the computed target filename already exists and isn't the same
|
||||
//! underlying file, that item is left alone and reported as skipped rather than
|
||||
//! overwritten. There is no undo yet, so the UI is expected to show the caller a
|
||||
//! confirmation of the full plan before calling [`execute_renames`].
|
||||
//! Every rename stays rooted at the source file's own directory - `new_name` is always
|
||||
//! resolved relative to `source`'s parent, never as an absolute path, and can never
|
||||
//! escape that directory tree via `..`. Within that directory tree, though, `new_name`
|
||||
//! may contain `/` to organize files into subfolders as part of renaming (e.g. a naming
|
||||
//! scheme like `{show name}/{episode name}` - the naming-expression engine already
|
||||
//! treats `/` between two `{ }` scriptlets as ordinary literal text, so no changes were
|
||||
//! needed there; this is purely about the rename engine trusting a `/` in the computed
|
||||
//! name to mean "put it in a subfolder" instead of rejecting it outright). Any missing
|
||||
//! intermediate directories are created automatically. Conflict policy is fixed at
|
||||
//! **skip**: if the computed target already exists and isn't the same underlying file,
|
||||
//! that item is left alone and reported as skipped rather than overwritten. There is no
|
||||
//! undo yet, so the UI is expected to show the caller a confirmation of the full plan
|
||||
//! before calling [`execute_renames`].
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
|
||||
/// One planned rename: `source` is the file's current full path, `new_name` is the
|
||||
/// filename (including extension) it should have afterward, in the same directory.
|
||||
/// target it should have afterward, resolved relative to `source`'s own directory -
|
||||
/// just a filename for a same-directory rename, or a `/`-separated relative path (e.g.
|
||||
/// `"Breaking Bad/Ozymandias.mkv"`) to also move the file into a (possibly new)
|
||||
/// subfolder.
|
||||
#[derive(Debug, Clone, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct RenamePlanItem {
|
||||
@@ -42,16 +52,32 @@ pub struct RenameResult {
|
||||
pub outcome: RenameOutcome,
|
||||
}
|
||||
|
||||
/// Rejects filenames that could escape the source file's own directory (path
|
||||
/// separators, `.`/`..`) or are empty. The rename engine only ever renames a file
|
||||
/// within its existing directory, so a `new_name` that isn't a plain filename is a bug
|
||||
/// upstream (or a hostile input) rather than something to interpret as a move.
|
||||
fn is_plain_filename(name: &str) -> bool {
|
||||
!name.is_empty()
|
||||
&& name != "."
|
||||
&& name != ".."
|
||||
&& !name.contains('/')
|
||||
&& !name.contains('\\')
|
||||
/// Validates a computed target, relative to the source file's own directory. `/` is
|
||||
/// allowed as a subdirectory separator - that's what lets a naming scheme like
|
||||
/// `{show name}/{episode name}` organize files into subfolders - but every path
|
||||
/// component is checked to make sure the target can never escape that directory tree:
|
||||
/// no empty components (a leading, trailing, or doubled `/` - which also catches an
|
||||
/// absolute-looking `/etc/passwd`-style path, since splitting it on `/` yields a
|
||||
/// leading empty component), and no `.`/`..` components. Backslashes are rejected
|
||||
/// outright rather than treated as an alternate separator - Renamr always uses `/` for
|
||||
/// the naming scheme's folder separator regardless of OS, so a literal `\` in a
|
||||
/// computed name is almost certainly a mistake, not an intentional Windows path.
|
||||
fn validate_relative_target(name: &str) -> Result<(), &'static str> {
|
||||
if name.is_empty() {
|
||||
return Err("empty");
|
||||
}
|
||||
if name.contains('\\') {
|
||||
return Err("backslashes aren't allowed - use / to organize into subfolders");
|
||||
}
|
||||
for component in name.split('/') {
|
||||
if component.is_empty() {
|
||||
return Err("contains an empty path segment (e.g. a leading, trailing, or doubled '/')");
|
||||
}
|
||||
if component == "." || component == ".." {
|
||||
return Err("contains a '.' or '..' path segment");
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// True if `a` and `b` refer to the same underlying file (used to allow case-only
|
||||
@@ -65,25 +91,12 @@ fn same_file(a: &Path, b: &Path) -> bool {
|
||||
}
|
||||
|
||||
fn rename_one(source: &Path, new_name: &str) -> RenameOutcome {
|
||||
if !is_plain_filename(new_name) {
|
||||
if let Err(reason) = validate_relative_target(new_name) {
|
||||
return RenameOutcome::Failed {
|
||||
error: format!("\"{new_name}\" is not a valid filename"),
|
||||
error: format!("\"{new_name}\" is not a valid target: {reason}"),
|
||||
};
|
||||
}
|
||||
|
||||
let current_name = match source.file_name().and_then(|n| n.to_str()) {
|
||||
Some(n) => n,
|
||||
None => {
|
||||
return RenameOutcome::Failed {
|
||||
error: "source has no filename".to_string(),
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
if current_name == new_name {
|
||||
return RenameOutcome::Unchanged;
|
||||
}
|
||||
|
||||
let parent = match source.parent() {
|
||||
Some(p) => p,
|
||||
None => {
|
||||
@@ -94,12 +107,27 @@ fn rename_one(source: &Path, new_name: &str) -> RenameOutcome {
|
||||
};
|
||||
let target = parent.join(new_name);
|
||||
|
||||
if target == source {
|
||||
return RenameOutcome::Unchanged;
|
||||
}
|
||||
|
||||
if target.exists() && !same_file(&target, source) {
|
||||
return RenameOutcome::SkippedConflict {
|
||||
existing_path: target.to_string_lossy().to_string(),
|
||||
};
|
||||
}
|
||||
|
||||
// `new_name` may include subdirectory components (organizing files into folders) -
|
||||
// make sure the destination directory tree exists before attempting the actual
|
||||
// rename/move. `create_dir_all` is a no-op (Ok) when the directory already exists.
|
||||
if let Some(target_parent) = target.parent() {
|
||||
if let Err(e) = fs::create_dir_all(target_parent) {
|
||||
return RenameOutcome::Failed {
|
||||
error: format!("couldn't create destination folder: {e}"),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
match fs::rename(source, &target) {
|
||||
Ok(()) => RenameOutcome::Renamed {
|
||||
new_path: target.to_string_lossy().to_string(),
|
||||
@@ -246,4 +274,104 @@ mod tests {
|
||||
|
||||
fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn organizes_into_a_new_subfolder() {
|
||||
let (dir, file) = temp_file_setup("Breaking.Bad.S05E14.mkv", "video");
|
||||
let items = vec![RenamePlanItem {
|
||||
source: file.to_string_lossy().to_string(),
|
||||
new_name: "Breaking Bad/Ozymandias.mkv".to_string(),
|
||||
}];
|
||||
|
||||
let results = execute_renames(&items);
|
||||
match &results[0].outcome {
|
||||
RenameOutcome::Renamed { new_path } => {
|
||||
let expected = dir.join("Breaking Bad").join("Ozymandias.mkv");
|
||||
assert_eq!(Path::new(new_path), expected.as_path());
|
||||
assert!(expected.exists());
|
||||
assert!(!file.exists());
|
||||
}
|
||||
other => panic!("expected Renamed, got {other:?}"),
|
||||
}
|
||||
|
||||
fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn organizes_into_nested_subfolders_that_dont_exist_yet() {
|
||||
let (dir, file) = temp_file_setup("ep.mkv", "video");
|
||||
let items = vec![RenamePlanItem {
|
||||
source: file.to_string_lossy().to_string(),
|
||||
new_name: "Breaking Bad/Season 05/S05E14.mkv".to_string(),
|
||||
}];
|
||||
|
||||
let results = execute_renames(&items);
|
||||
let expected = dir.join("Breaking Bad").join("Season 05").join("S05E14.mkv");
|
||||
match &results[0].outcome {
|
||||
RenameOutcome::Renamed { new_path } => {
|
||||
assert_eq!(Path::new(new_path), expected.as_path());
|
||||
assert!(expected.exists());
|
||||
}
|
||||
other => panic!("expected Renamed, got {other:?}"),
|
||||
}
|
||||
|
||||
fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skips_when_the_organized_target_already_exists() {
|
||||
let (dir, file) = temp_file_setup("a.txt", "aaa");
|
||||
fs::create_dir_all(dir.join("Sub")).unwrap();
|
||||
fs::write(dir.join("Sub").join("b.txt"), "bbb").unwrap();
|
||||
|
||||
let items = vec![RenamePlanItem {
|
||||
source: file.to_string_lossy().to_string(),
|
||||
new_name: "Sub/b.txt".to_string(),
|
||||
}];
|
||||
|
||||
let results = execute_renames(&items);
|
||||
assert!(matches!(
|
||||
results[0].outcome,
|
||||
RenameOutcome::SkippedConflict { .. }
|
||||
));
|
||||
assert!(file.exists());
|
||||
assert_eq!(fs::read_to_string(dir.join("Sub").join("b.txt")).unwrap(), "bbb");
|
||||
|
||||
fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_backslashes_in_new_name() {
|
||||
let (dir, file) = temp_file_setup("a.txt", "aaa");
|
||||
let items = vec![RenamePlanItem {
|
||||
source: file.to_string_lossy().to_string(),
|
||||
new_name: "Sub\\b.txt".to_string(),
|
||||
}];
|
||||
|
||||
let results = execute_renames(&items);
|
||||
assert!(matches!(results[0].outcome, RenameOutcome::Failed { .. }));
|
||||
assert!(file.exists());
|
||||
|
||||
fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_doubled_and_leading_slashes() {
|
||||
let (dir, file) = temp_file_setup("a.txt", "aaa");
|
||||
|
||||
for bad_name in ["Sub//b.txt", "/etc/passwd", "Sub/b.txt/"] {
|
||||
let items = vec![RenamePlanItem {
|
||||
source: file.to_string_lossy().to_string(),
|
||||
new_name: bad_name.to_string(),
|
||||
}];
|
||||
let results = execute_renames(&items);
|
||||
assert!(
|
||||
matches!(results[0].outcome, RenameOutcome::Failed { .. }),
|
||||
"expected {bad_name:?} to be rejected"
|
||||
);
|
||||
}
|
||||
assert!(file.exists());
|
||||
|
||||
fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user