diff --git a/app/library/Utils.py b/app/library/Utils.py index d534299e..d815a2b8 100644 --- a/app/library/Utils.py +++ b/app/library/Utils.py @@ -824,6 +824,79 @@ def rename_file(old_path: Path, new_name: str) -> tuple[Path, list[tuple[Path, P raise +def move_file(old_path: Path, target_dir: Path) -> tuple[Path, list[tuple[Path, Path]]]: + """ + Move a file along with all its sidecar files to a target directory. + + Args: + old_path (Path): The original file path. + target_dir (Path): The target directory to move the file to. + + Returns: + tuple[Path, list[tuple[Path, Path]]]: Tuple of (new_path, list of (old_sidecar_path, new_sidecar_path) tuples). + + Raises: + OSError: If moving fails. + ValueError: If target directory doesn't exist or if destination would cause collision. + + """ + if not target_dir.exists(): + msg: str = f"Target directory '{target_dir}' does not exist" + raise ValueError(msg) + + if not target_dir.is_dir(): + msg = f"Target '{target_dir}' is not a directory" + raise ValueError(msg) + + new_path: Path = target_dir / old_path.name + + if new_path.exists(): + msg = f"Destination '{new_path}' already exists" + raise ValueError(msg) + + sidecar_data: dict[str, dict] = get_file_sidecar(old_path) + sidecar_files: list[Path] = [] + + for category in sidecar_data.values(): + sidecar_files.extend([item["file"] for item in category if "file" in item and isinstance(item["file"], Path)]) + + renamed_sidecars: list[tuple[Path, Path]] = [] + move_operations: list[tuple[Path, Path]] = [] + + for sidecar in sidecar_files: + new_sidecar_path: Path = target_dir / sidecar.name + if new_sidecar_path.exists(): + msg = f"Sidecar destination '{new_sidecar_path}' already exists" + raise ValueError(msg) + + move_operations.append((sidecar, new_sidecar_path)) + + try: + moved_main: Path = old_path.rename(new_path) + for old_sidecar, new_sidecar in move_operations: + try: + moved_sidecar: Path = old_sidecar.rename(new_sidecar) + renamed_sidecars.append((old_sidecar, moved_sidecar)) + except OSError as e: + LOG.error(f"Failed to move sidecar '{old_sidecar}': {e}") + try: + moved_main.rename(old_path) + # Rollback any sidecars that were already moved + for rolled_back_old, rolled_back_new in renamed_sidecars: + try: + rolled_back_new.rename(rolled_back_old) + except OSError: + LOG.error(f"Failed to rollback sidecar move from '{rolled_back_new}' to '{rolled_back_old}'") + except OSError: + LOG.error(f"Failed to rollback main file move from '{moved_main}' to '{old_path}'") + raise + + return moved_main, renamed_sidecars + except OSError as e: + LOG.error(f"Failed to move '{old_path}' to '{new_path}': {e}") + raise + + def get_possible_images(dir: str) -> list[dict]: images: list = [] diff --git a/app/routes/api/browser.py b/app/routes/api/browser.py index e0626974..4066f0dd 100644 --- a/app/routes/api/browser.py +++ b/app/routes/api/browser.py @@ -14,7 +14,7 @@ from app.library.encoder import Encoder from app.library.Events import EventBus, Events from app.library.ffprobe import ffprobe from app.library.router import route -from app.library.Utils import delete_dir, get_file, get_file_sidecar, get_files, get_mime_type, rename_file +from app.library.Utils import delete_dir, get_file, get_file_sidecar, get_files, get_mime_type, move_file, rename_file LOG: logging.Logger = logging.getLogger(__name__) @@ -364,15 +364,19 @@ async def path_actions(request: Request, config: Config, queue: DownloadQueue, n continue try: - # Rename main file and all sidecar files - renamed, renamed_sidecars = rename_file(path, new_name) - - sidecar_count: int = len(renamed_sidecars) - sidecar_info: str = ( - f" (with {sidecar_count} sidecar file{'s' if sidecar_count != 1 else ''})" - if sidecar_count > 0 - else "" - ) + sidecar_count: int = 0 + sidecar_info: str = "" + sidecar_renamed: list[tuple[Path, Path]] = [] + if path.is_dir(): + renamed: Path = path.rename(new_path) + else: + renamed, sidecar_renamed = rename_file(path, new_name) + sidecar_count: int = len(sidecar_renamed) + sidecar_info: str = ( + f" (with {sidecar_count} sidecar file{'s' if sidecar_count != 1 else ''})" + if sidecar_count > 0 + else "" + ) LOG.info( f"Renamed '{path.relative_to(config.download_path)}' to '{renamed.relative_to(config.download_path)}'{sidecar_info}" @@ -386,21 +390,16 @@ async def path_actions(request: Request, config: Config, queue: DownloadQueue, n continue else: extra_info: dict[str, Path] = {"new_path": renamed} - if renamed_sidecars: - extra_info["sidecar_count"] = len(renamed_sidecars) + if sidecar_renamed: + extra_info["sidecar_count"] = len(sidecar_renamed) record(path, ok=True, action=action, extra=extra_info) - for old_sidecar, new_sidecar in renamed_sidecars: - record( - old_sidecar, - ok=True, - action="rename_sidecar", - extra={"new_path": new_sidecar, "parent": str(path.relative_to(config.download_path))}, - ) + for old_sidecar, new_sidecar in sidecar_renamed: + record(old_sidecar, ok=True, action=action, extra={"new_path": new_sidecar}) if item := queue.done.get_item(filename=str(path.relative_to(config.download_path))): item.info.filename = str(renamed.relative_to(config.download_path)) - if len(renamed_sidecars) > 0: + if sidecar_renamed: item.info.get_file_sidecar() queue.done.put(item) @@ -432,7 +431,7 @@ async def path_actions(request: Request, config: Config, queue: DownloadQueue, n continue raw_new: str = unquote_plus(str(new_path)).strip() - target_dir = ( + target_dir: Path = ( Path(config.download_path) if not raw_new or raw_new in ("/", ".") else Path(config.download_path).joinpath(raw_new.lstrip("/")) @@ -446,7 +445,7 @@ async def path_actions(request: Request, config: Config, queue: DownloadQueue, n ) continue - root_real = Path(config.download_path).resolve() + root_real: Path = Path(config.download_path).resolve() if not target_dir.exists() or not target_dir.is_dir(): record( path, ok=False, error="Destination path is invalid.", action=action, extra={"new_path": target_dir} @@ -468,14 +467,49 @@ async def path_actions(request: Request, config: Config, queue: DownloadQueue, n continue try: - dest = target_dir.joinpath(path.name) - path.rename(dest) + sidecar_count: int = 0 + sidecar_moved: list[tuple[Path, Path]] = [] + sidecar_info: str = "" + + if path.is_dir(): + dest: Path = target_dir.joinpath(path.name) + moved: Path = path.rename(dest) + else: + moved, sidecar_moved = move_file(path, target_dir) + sidecar_count: int = len(sidecar_moved) + sidecar_info: str = ( + f" (with {sidecar_count} sidecar file{'s' if sidecar_count != 1 else ''})" + if sidecar_count > 0 + else "" + ) + + LOG.info( + f"Moved '{path.relative_to(config.download_path)}' to '{moved.relative_to(config.download_path)}'{sidecar_info}" + ) except OSError as e: LOG.exception(e) record(path, ok=False, error=str(e), action=action, extra={"item": params}) continue + except ValueError as e: + record(path, ok=False, error=str(e), action=action, extra={"item": params}) + continue else: - record(path, ok=True, action=action, extra={"new_path": dest}) + extra_info: dict[str, Path] = {"new_path": moved} + if sidecar_moved: + extra_info["sidecar_count"] = len(sidecar_moved) + + record(path, ok=True, action=action, extra=extra_info) + + for old_sidecar, new_sidecar in sidecar_moved: + record(old_sidecar, ok=True, action=action, extra={"new_path": new_sidecar}) + + if item := queue.done.get_item(filename=str(path.relative_to(config.download_path))): + item.info.filename = str(moved.relative_to(config.download_path)) + if sidecar_moved: + item.info.get_file_sidecar() + + queue.done.put(item) + notify.emit(Events.ITEM_UPDATED, data=item.info) return web.json_response(data=operations_status, status=web.HTTPOk.status_code) diff --git a/app/tests/test_utils.py b/app/tests/test_utils.py index f41feda4..5b4893dd 100644 --- a/app/tests/test_utils.py +++ b/app/tests/test_utils.py @@ -39,6 +39,7 @@ from app.library.Utils import ( load_cookies, load_modules, merge_dict, + move_file, parse_tags, read_logfile, rename_file, @@ -2189,3 +2190,189 @@ class TestRenameFile: sidecar_names = {new_sidecar.name for old_sidecar, new_sidecar in sidecars} assert "renamed.en-US.ass" in sidecar_names assert "renamed.thumb.jpg" in sidecar_names + + +class TestMoveFile: + """Test move_file function.""" + + def test_move_single_file_no_sidecars(self, tmp_path: Path): + """Test moving a single file without sidecar files.""" + # Create test file + source_dir = tmp_path / "source" + source_dir.mkdir() + test_file = source_dir / "video.mp4" + test_file.write_text("test content") + + target_dir = tmp_path / "target" + target_dir.mkdir() + + # Move file + new_path, sidecars = move_file(test_file, target_dir) + + # Assertions + assert new_path.exists() + assert "video.mp4" == new_path.name + assert new_path.parent == target_dir + assert not test_file.exists() + assert 0 == len(sidecars) + + def test_move_file_with_subtitle_sidecar(self, tmp_path: Path): + """Test moving a file with subtitle sidecar.""" + # Create test files + source_dir = tmp_path / "source" + source_dir.mkdir() + test_file = source_dir / "video.mp4" + test_file.write_text("test video") + + subtitle_file = source_dir / "video.en.srt" + subtitle_file.write_text("test subtitle") + + target_dir = tmp_path / "target" + target_dir.mkdir() + + # Move file + new_path, sidecars = move_file(test_file, target_dir) + + # Assertions + assert new_path.exists() + assert "video.mp4" == new_path.name + assert new_path.parent == target_dir + assert not test_file.exists() + + assert 1 == len(sidecars) + old_sidecar, new_sidecar = sidecars[0] + assert new_sidecar.exists() + assert "video.en.srt" == new_sidecar.name + assert new_sidecar.parent == target_dir + assert old_sidecar == subtitle_file + assert not subtitle_file.exists() + + def test_move_file_with_multiple_sidecars(self, tmp_path: Path): + """Test moving a file with multiple sidecar files.""" + # Create test files + source_dir = tmp_path / "source" + source_dir.mkdir() + test_file = source_dir / "video.mp4" + test_file.write_text("test video") + + subtitle_en = source_dir / "video.en.srt" + subtitle_en.write_text("english subtitle") + + subtitle_fr = source_dir / "video.fr.srt" + subtitle_fr.write_text("french subtitle") + + info_file = source_dir / "video.info.json" + info_file.write_text('{"title": "test"}') + + target_dir = tmp_path / "target" + target_dir.mkdir() + + # Move file + new_path, sidecars = move_file(test_file, target_dir) + + # Assertions + assert new_path.exists() + assert "video.mp4" == new_path.name + assert new_path.parent == target_dir + assert not test_file.exists() + + assert 3 == len(sidecars) + + # Check all sidecars were moved + sidecar_names = {new_sidecar.name for old_sidecar, new_sidecar in sidecars} + assert "video.en.srt" in sidecar_names + assert "video.fr.srt" in sidecar_names + assert "video.info.json" in sidecar_names + + # Check all are in target directory + for _old_sidecar, new_sidecar in sidecars: + assert new_sidecar.parent == target_dir + + # Check old files don't exist + assert not subtitle_en.exists() + assert not subtitle_fr.exists() + assert not info_file.exists() + + def test_move_file_destination_exists(self, tmp_path: Path): + """Test moving a file when destination already exists.""" + # Create test files + source_dir = tmp_path / "source" + source_dir.mkdir() + test_file = source_dir / "video.mp4" + test_file.write_text("test content") + + target_dir = tmp_path / "target" + target_dir.mkdir() + existing_file = target_dir / "video.mp4" + existing_file.write_text("existing content") + + # Should raise ValueError + with pytest.raises(ValueError, match="already exists"): + move_file(test_file, target_dir) + + # Original files should still exist + assert test_file.exists() + assert existing_file.exists() + + def test_move_file_sidecar_destination_exists(self, tmp_path: Path): + """Test moving when sidecar destination already exists.""" + # Create test files + source_dir = tmp_path / "source" + source_dir.mkdir() + test_file = source_dir / "video.mp4" + test_file.write_text("test video") + + subtitle_file = source_dir / "video.en.srt" + subtitle_file.write_text("test subtitle") + + target_dir = tmp_path / "target" + target_dir.mkdir() + + # Create conflicting sidecar destination + conflicting_sidecar = target_dir / "video.en.srt" + conflicting_sidecar.write_text("existing subtitle") + + # Should raise ValueError + with pytest.raises(ValueError, match=r"Sidecar destination.*already exists"): + move_file(test_file, target_dir) + + # Original files should still exist + assert test_file.exists() + assert subtitle_file.exists() + assert conflicting_sidecar.exists() + + def test_move_file_target_not_directory(self, tmp_path: Path): + """Test moving when target is not a directory.""" + # Create test file + source_dir = tmp_path / "source" + source_dir.mkdir() + test_file = source_dir / "video.mp4" + test_file.write_text("test content") + + # Create a file (not directory) as target + target_file = tmp_path / "target.txt" + target_file.write_text("not a directory") + + # Should raise ValueError + with pytest.raises(ValueError, match="not a directory"): + move_file(test_file, target_file) + + # Original file should still exist + assert test_file.exists() + + def test_move_file_target_does_not_exist(self, tmp_path: Path): + """Test moving when target directory doesn't exist.""" + # Create test file + source_dir = tmp_path / "source" + source_dir.mkdir() + test_file = source_dir / "video.mp4" + test_file.write_text("test content") + + target_dir = tmp_path / "nonexistent" + + # Should raise ValueError + with pytest.raises(ValueError, match="does not exist"): + move_file(test_file, target_dir) + + # Original file should still exist + assert test_file.exists() diff --git a/ui/app/pages/browser/[...slug].vue b/ui/app/pages/browser/[...slug].vue index f3e30990..3cb6e827 100644 --- a/ui/app/pages/browser/[...slug].vue +++ b/ui/app/pages/browser/[...slug].vue @@ -726,9 +726,10 @@ const handleAction = async (action: string, item: FileItem): Promise => { } if ('rename' === action) { + const moveSideCars = 'file' === item.type ? ' (and its sidecars)' : '' const { status, value: newName } = await dialog.promptDialog({ title: 'Rename Item', - message: `Enter new name for '${item.name}' Sidecars will be renamed as well:`, + message: `Enter new name for '${item.name}'${moveSideCars}:`, initial: item.name, confirmText: 'Rename', cancelText: 'Cancel', @@ -775,9 +776,10 @@ const handleAction = async (action: string, item: FileItem): Promise => { } if ('move' === action) { + const moveSideCars = 'file' === item.type ? ' (and its sidecars)' : '' const { status, value: newPath } = await dialog.promptDialog({ title: 'Move Item', - message: `Enter new path for '${item.name}':`, + message: `Enter new path for '${item.name}'${moveSideCars}:`, initial: item.path.replace(/[^/]+$/, '') || '/', confirmText: 'Move', cancelText: 'Cancel', @@ -792,10 +794,12 @@ const handleAction = async (action: string, item: FileItem): Promise => { return } - await actionRequest(item, 'move', { new_path: new_path }, (item, _, data) => { - items.value = items.value.filter(i => i.path !== item.path) - toast.success(`Moved '${item.name}' to '${data.new_path}'.`) - }) + await actionRequest(item, 'move', { new_path: new_path }, (item, _, data, source) => { + if (item.path === source.path) { + toast.success(`Moved '${item.name}' to '${data.new_path}'.`) + } + items.value = items.value.filter(i => i.path !== data.path) + }, true) return }