diff --git a/app/library/Utils.py b/app/library/Utils.py index 1313f99f..d534299e 100644 --- a/app/library/Utils.py +++ b/app/library/Utils.py @@ -763,6 +763,67 @@ def get_file_sidecar(file: Path | None = None) -> dict[dict]: return files +def rename_file(old_path: Path, new_name: str) -> tuple[Path, list[tuple[Path, Path]]]: + """ + Rename a file along with all its sidecar files. + + Args: + old_path (Path): The original file path. + new_name (str): The new name for the file (not full path, just the name). + + Returns: + tuple[Path, list[tuple[Path, Path]]]: Tuple of (new_path, list of (old_sidecar_path, new_sidecar_path) tuples). + + Raises: + OSError: If renaming fails. + ValueError: If new_name would cause a collision with existing files. + + """ + new_path: Path = old_path.parent / new_name + + if new_path.exists(): + msg: str = f"Destination '{new_name}' 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]] = [] + new_stem: str = new_path.stem + rename_operations: list[tuple[Path, Path]] = [] + + for sidecar in sidecar_files: + sidecar_suffix: str = sidecar.name[len(old_path.stem) :] # Everything after the original stem + new_sidecar_path: Path = old_path.parent / f"{new_stem}{sidecar_suffix}" + if new_sidecar_path.exists(): + msg = f"Sidecar destination '{new_sidecar_path.name}' already exists" + raise ValueError(msg) + + rename_operations.append((sidecar, new_sidecar_path)) + + try: + renamed_main: Path = old_path.rename(new_path) + for old_sidecar, new_sidecar in rename_operations: + try: + renamed_sidecar: Path = old_sidecar.rename(new_sidecar) + renamed_sidecars.append((old_sidecar, renamed_sidecar)) + except OSError as e: + LOG.error(f"Failed to rename sidecar '{old_sidecar}': {e}") + try: + renamed_main.rename(old_path) + except OSError: + LOG.error(f"Failed to rollback main file rename from '{renamed_main}' to '{old_path}'") + raise + + return renamed_main, renamed_sidecars + except OSError as e: + LOG.error(f"Failed to rename '{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 2c47962c..e0626974 100644 --- a/app/routes/api/browser.py +++ b/app/routes/api/browser.py @@ -9,10 +9,12 @@ from aiohttp.web import Request, Response from app.library.cache import Cache from app.library.config import Config +from app.library.DownloadQueue import DownloadQueue 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 +from app.library.Utils import delete_dir, get_file, get_file_sidecar, get_files, get_mime_type, rename_file LOG: logging.Logger = logging.getLogger(__name__) @@ -175,13 +177,15 @@ async def file_browser(request: Request, config: Config, encoder: Encoder) -> Re @route("POST", "api/file/actions", "browser.file.actions") -async def path_actions(request: Request, config: Config) -> Response: +async def path_actions(request: Request, config: Config, queue: DownloadQueue, notify: EventBus) -> Response: """ Browser actions. Args: request (Request): The request object. config (Config): The configuration object. + queue (DownloadQueue): The download queue instance. + notify (EventBus): The event bus instance. Returns: Response: The response object. @@ -360,16 +364,47 @@ async def path_actions(request: Request, config: Config) -> Response: continue try: - renamed = path.rename(new_path) + # 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 "" + ) + LOG.info( - f"Renamed '{path.relative_to(config.download_path)}' to '{renamed.relative_to(config.download_path)}'" + f"Renamed '{path.relative_to(config.download_path)}' to '{renamed.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": renamed}) + extra_info: dict[str, Path] = {"new_path": renamed} + if renamed_sidecars: + extra_info["sidecar_count"] = len(renamed_sidecars) + 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))}, + ) + + 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: + item.info.get_file_sidecar() + + queue.done.put(item) + notify.emit(Events.ITEM_UPDATED, data=item.info) if "delete" == action: try: diff --git a/app/tests/test_utils.py b/app/tests/test_utils.py index 536b8ca8..f41feda4 100644 --- a/app/tests/test_utils.py +++ b/app/tests/test_utils.py @@ -41,6 +41,7 @@ from app.library.Utils import ( merge_dict, parse_tags, read_logfile, + rename_file, str_to_dt, strip_newline, tail_log, @@ -1731,9 +1732,7 @@ class TestGetChannelImages: """Test extracting poster image from portrait ratio thumbnail.""" from app.library.Utils import get_channel_images - thumbnails = [ - {"url": "http://example.com/poster.jpg", "width": 200, "height": 300, "id": "some_id"} - ] + thumbnails = [{"url": "http://example.com/poster.jpg", "width": 200, "height": 300, "id": "some_id"}] result = get_channel_images(thumbnails) @@ -2049,3 +2048,144 @@ class TestCreateCookiesFile: assert result == cookie_path assert cookie_path.read_text() == "new_data" + +class TestRenameFile: + """Test rename_file function.""" + + def test_rename_single_file_no_sidecars(self, tmp_path: Path): + """Test renaming a single file without sidecar files.""" + # Create test file + test_file = tmp_path / "video.mp4" + test_file.write_text("test content") + + # Rename file + new_path, sidecars = rename_file(test_file, "renamed_video.mp4") + + # Assertions + assert new_path.exists() + assert "renamed_video.mp4" == new_path.name + assert not test_file.exists() + assert 0 == len(sidecars) + + def test_rename_file_with_subtitle_sidecar(self, tmp_path: Path): + """Test renaming a file with subtitle sidecar.""" + # Create test files + test_file = tmp_path / "video.mp4" + test_file.write_text("test video") + + subtitle_file = tmp_path / "video.en.srt" + subtitle_file.write_text("test subtitle") + + # Rename file + new_path, sidecars = rename_file(test_file, "renamed_video.mp4") + + # Assertions + assert new_path.exists() + assert "renamed_video.mp4" == new_path.name + assert not test_file.exists() + + assert 1 == len(sidecars) + old_sidecar, new_sidecar = sidecars[0] + assert new_sidecar.exists() + assert "renamed_video.en.srt" == new_sidecar.name + assert old_sidecar == subtitle_file + assert not subtitle_file.exists() + + def test_rename_file_with_multiple_sidecars(self, tmp_path: Path): + """Test renaming a file with multiple sidecar files.""" + # Create test files + test_file = tmp_path / "video.mp4" + test_file.write_text("test video") + + subtitle_en = tmp_path / "video.en.srt" + subtitle_en.write_text("english subtitle") + + subtitle_fr = tmp_path / "video.fr.srt" + subtitle_fr.write_text("french subtitle") + + info_file = tmp_path / "video.info.json" + info_file.write_text('{"title": "test"}') + + # Rename file + new_path, sidecars = rename_file(test_file, "renamed_video.mp4") + + # Assertions + assert new_path.exists() + assert "renamed_video.mp4" == new_path.name + assert not test_file.exists() + + assert 3 == len(sidecars) + + # Check all sidecars were renamed + sidecar_names = {new_sidecar.name for old_sidecar, new_sidecar in sidecars} + assert "renamed_video.en.srt" in sidecar_names + assert "renamed_video.fr.srt" in sidecar_names + assert "renamed_video.info.json" in sidecar_names + + # Check old files don't exist + assert not subtitle_en.exists() + assert not subtitle_fr.exists() + assert not info_file.exists() + + def test_rename_file_destination_exists(self, tmp_path: Path): + """Test renaming a file when destination already exists.""" + # Create test files + test_file = tmp_path / "video.mp4" + test_file.write_text("test content") + + existing_file = tmp_path / "renamed_video.mp4" + existing_file.write_text("existing content") + + # Should raise ValueError + with pytest.raises(ValueError, match="already exists"): + rename_file(test_file, "renamed_video.mp4") + + # Original files should still exist + assert test_file.exists() + assert existing_file.exists() + + def test_rename_file_sidecar_destination_exists(self, tmp_path: Path): + """Test renaming when sidecar destination already exists.""" + # Create test files + test_file = tmp_path / "video.mp4" + test_file.write_text("test video") + + subtitle_file = tmp_path / "video.en.srt" + subtitle_file.write_text("test subtitle") + + # Create conflicting sidecar destination + conflicting_sidecar = tmp_path / "renamed_video.en.srt" + conflicting_sidecar.write_text("existing subtitle") + + # Should raise ValueError + with pytest.raises(ValueError, match=r"Sidecar destination.*already exists"): + rename_file(test_file, "renamed_video.mp4") + + # Original files should still exist + assert test_file.exists() + assert subtitle_file.exists() + assert conflicting_sidecar.exists() + + def test_rename_preserves_sidecar_extensions(self, tmp_path: Path): + """Test that rename preserves complex sidecar extensions.""" + # Create test files with complex extensions + test_file = tmp_path / "video.mp4" + test_file.write_text("test video") + + subtitle_file = tmp_path / "video.en-US.ass" + subtitle_file.write_text("test subtitle") + + thumb_file = tmp_path / "video.thumb.jpg" + thumb_file.write_text("test thumb") + + # Rename file + new_path, sidecars = rename_file(test_file, "renamed.mp4") + + # Assertions + assert new_path.exists() + assert "renamed.mp4" == new_path.name + + assert 2 == len(sidecars) + 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 diff --git a/ui/app/composables/useNotification.ts b/ui/app/composables/useNotification.ts index e9d5cc5d..7d54820b 100644 --- a/ui/app/composables/useNotification.ts +++ b/ui/app/composables/useNotification.ts @@ -51,13 +51,6 @@ const sendMessage = (type: notificationType, id: string, message: string, opts?: const useToastNotification = !window.isSecureContext || 'toast' === toastTarget.value || !('Notification' in window) || 'granted' !== Notification.permission; - console.log('useToastNotification', useToastNotification,{ - windowIsSecureContext: window.isSecureContext, - notificationTarget: toastTarget.value, - notificationInWindow: 'Notification' in window, - notificationPermission: Notification.permission - }); - if (useToastNotification) { switch (type) { case 'info': diff --git a/ui/app/pages/browser/[...slug].vue b/ui/app/pages/browser/[...slug].vue index f089eefa..f3e30990 100644 --- a/ui/app/pages/browser/[...slug].vue +++ b/ui/app/pages/browser/[...slug].vue @@ -728,7 +728,7 @@ const handleAction = async (action: string, item: FileItem): Promise => { if ('rename' === action) { const { status, value: newName } = await dialog.promptDialog({ title: 'Rename Item', - message: `Enter new name for '${item.name}':`, + message: `Enter new name for '${item.name}' Sidecars will be renamed as well:`, initial: item.name, confirmText: 'Rename', cancelText: 'Cancel', @@ -742,11 +742,13 @@ const handleAction = async (action: string, item: FileItem): Promise => { return } - await actionRequest(item, 'rename', { new_name: new_name }, (item, _, data) => { - item.name = data.new_name - item.path = item.path.replace(/[^/]+$/, data.new_name) - toast.success(`Renamed '${item.name}'.`) - }) + await actionRequest(item, 'rename', { new_name: new_name }, (item, _, data, source) => { + if (item.path === source.path) { + toast.success(`Renamed '${item.name}'.`) + } + item.name = basename(data.new_path) + item.path = data.new_path + }, true) return } @@ -790,7 +792,7 @@ const handleAction = async (action: string, item: FileItem): Promise => { return } - await actionRequest(item, 'move', { new_path: new_path }, (item, action, data) => { + 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}'.`) }) @@ -802,25 +804,22 @@ const handleAction = async (action: string, item: FileItem): Promise => { const actionRequest = async ( item: FileItem, action: string, - data: Record, - cb: (item: FileItem, action: string, data: any) => void + req: Record, + cb: (item: FileItem, action: string, data: any, source: FileItem, req: any) => void, + multiple: boolean = false ): Promise => { if (!config.app.browser_control_enabled) { return } - if (!item || !action || !data) { + if (!item || !action || !req) { return } try { const response = await request(`/api/file/actions`, { method: 'POST', - body: JSON.stringify([{ - path: item.path, - action: action, - ...data - }]), + body: JSON.stringify([{ path: item.path, action: action, ...req }]), }) if (!response.ok) { @@ -831,17 +830,24 @@ const actionRequest = async ( const json = await response.json() as Array<{ path: string, status: boolean, error?: string }> json.forEach(i => { - if (i.path !== item.path) { + if (false === multiple && i.path !== item.path) { return } - if (true !== i.status) { + if (false === multiple && true !== i.status) { toast.error(`Failed to perform action: ${i.error || 'Unknown error'}`) return } if (cb && typeof cb === 'function') { - cb(item, action, data) + if (false === multiple) { + return cb(item, action, req, item, req) + } + items.value.forEach(it => { + if (it.path === i.path) { + return cb(it, action, i, toRaw(item), req) + } + }) } });