diff --git a/webui/backend/app/db/__pycache__/history_repository.cpython-313.pyc b/webui/backend/app/db/__pycache__/history_repository.cpython-313.pyc index 007e0fa..fc5a4d6 100644 Binary files a/webui/backend/app/db/__pycache__/history_repository.cpython-313.pyc and b/webui/backend/app/db/__pycache__/history_repository.cpython-313.pyc differ diff --git a/webui/backend/app/db/history_repository.py b/webui/backend/app/db/history_repository.py index 2073f08..38c9363 100644 --- a/webui/backend/app/db/history_repository.py +++ b/webui/backend/app/db/history_repository.py @@ -6,8 +6,8 @@ from contextlib import contextmanager from datetime import datetime, timezone from pathlib import Path -VALID_HISTORY_STATUSES = {"queued", "completed", "failed"} -VALID_HISTORY_OPERATIONS = {"mkdir", "rename", "delete", "copy", "move", "upload"} +VALID_HISTORY_STATUSES = {"queued", "completed", "failed", "requested", "ready", "preflight_failed"} +VALID_HISTORY_OPERATIONS = {"mkdir", "rename", "delete", "copy", "move", "upload", "download"} class HistoryRepository: diff --git a/webui/backend/app/services/__pycache__/file_ops_service.cpython-313.pyc b/webui/backend/app/services/__pycache__/file_ops_service.cpython-313.pyc index 9aa3599..3d2523d 100644 Binary files a/webui/backend/app/services/__pycache__/file_ops_service.cpython-313.pyc and b/webui/backend/app/services/__pycache__/file_ops_service.cpython-313.pyc differ diff --git a/webui/backend/app/services/file_ops_service.py b/webui/backend/app/services/file_ops_service.py index 0b399d8..bccfe9c 100644 --- a/webui/backend/app/services/file_ops_service.py +++ b/webui/backend/app/services/file_ops_service.py @@ -387,17 +387,74 @@ class FileOpsService: ) def prepare_download(self, paths: list[str]) -> dict: + history_entry_id: str | None = None + history_mode = self._download_mode_from_request_paths(paths) + history_path = self._summarize_download_targets(paths) + history_download_name: str | None = None if not paths: - raise AppError( + error = AppError( code="invalid_request", message="At least one path is required", status_code=400, ) + self._record_download_failure( + mode=history_mode, + path_summary=history_path, + download_name=None, + error=error, + history_entry_id=None, + ) + raise error - resolved_targets = [self._path_guard.resolve_existing_path(path) for path in paths] - if len(resolved_targets) == 1 and resolved_targets[0].absolute.is_file(): - return self._prepare_single_file_download(resolved_targets[0]) - return self._prepare_zip_download(resolved_targets) + try: + resolved_targets = [self._path_guard.resolve_existing_path(path) for path in paths] + history_mode = self._download_mode_from_resolved_targets(resolved_targets) + history_path = self._summarize_download_targets([target.relative for target in resolved_targets]) + history_download_name = self._download_name_for_targets(resolved_targets) + history_entry_id = self._record_download_status( + status="requested", + mode=history_mode, + path_summary=history_path, + download_name=history_download_name, + ) + + if len(resolved_targets) == 1 and resolved_targets[0].absolute.is_file(): + prepared = self._prepare_single_file_download(resolved_targets[0]) + else: + prepared = self._prepare_zip_download(resolved_targets, history_download_name) + + self._record_download_status( + status="ready", + mode=history_mode, + path_summary=history_path, + download_name=history_download_name, + history_entry_id=history_entry_id, + ) + return prepared + except AppError as error: + self._record_download_failure( + mode=history_mode, + path_summary=history_path, + download_name=history_download_name, + error=error, + history_entry_id=history_entry_id, + ) + raise + except OSError as exc: + error = AppError( + code="io_error", + message="Filesystem operation failed", + status_code=500, + details={"reason": str(exc)}, + ) + self._record_download_failure( + mode=history_mode, + path_summary=history_path, + download_name=history_download_name, + error=error, + history_entry_id=history_entry_id, + ) + raise error def save(self, path: str, content: str, expected_modified: str) -> SaveResponse: resolved_target = self._path_guard.resolve_existing_path(path) @@ -699,7 +756,7 @@ class FileOpsService: "content_type": self._content_type_for(resolved_target.absolute) or "application/octet-stream", } - def _prepare_zip_download(self, resolved_targets: list) -> dict: + def _prepare_zip_download(self, resolved_targets: list, download_name: str) -> dict: archive_names: set[str] = set() for resolved_target in resolved_targets: archive_name = resolved_target.absolute.name @@ -712,11 +769,6 @@ class FileOpsService: archive_names.add(archive_name) self._run_zip_download_preflight(resolved_targets) - if len(resolved_targets) == 1 and resolved_targets[0].absolute.is_dir(): - download_name = f"{resolved_targets[0].absolute.name}.zip" - else: - download_name = f"kodidownload-{datetime.now(timezone.utc).strftime('%Y%m%d-%H%M%S')}.zip" - buffer = BytesIO() with zipfile.ZipFile(buffer, "w", compression=zipfile.ZIP_DEFLATED) as archive: for resolved_target in resolved_targets: @@ -734,6 +786,97 @@ class FileOpsService: "content_type": "application/zip", } + def _download_name_for_targets(self, resolved_targets: list) -> str: + if len(resolved_targets) == 1 and resolved_targets[0].absolute.is_file(): + return resolved_targets[0].absolute.name + if len(resolved_targets) == 1 and resolved_targets[0].absolute.is_dir(): + return f"{resolved_targets[0].absolute.name}.zip" + return f"kodidownload-{datetime.now(timezone.utc).strftime('%Y%m%d-%H%M%S')}.zip" + + @staticmethod + def _download_mode_from_request_paths(paths: list[str]) -> str: + return "multi_zip" if len(paths) > 1 else "single_file" + + @staticmethod + def _download_mode_from_resolved_targets(resolved_targets: list) -> str: + if len(resolved_targets) == 1 and resolved_targets[0].absolute.is_file(): + return "single_file" + if len(resolved_targets) == 1 and resolved_targets[0].absolute.is_dir(): + return "single_directory_zip" + return "multi_zip" + + @staticmethod + def _summarize_download_targets(paths: list[str]) -> str: + if not paths: + return "-" + if len(paths) == 1: + return paths[0] + if len(paths) == 2: + return f"{paths[0]}, {paths[1]}" + return f"{paths[0]}, {paths[1]}, +{len(paths) - 2} more" + + def _record_download_status( + self, + *, + status: str, + mode: str, + path_summary: str, + download_name: str | None, + history_entry_id: str | None = None, + ) -> str | None: + if not self._history_repository: + return history_entry_id + if history_entry_id: + self._history_repository.update_entry( + entry_id=history_entry_id, + status=status, + error_code=None, + error_message=None, + finished_at=self._now_iso(), + ) + return history_entry_id + created = self._history_repository.create_entry( + operation="download", + status=status, + source=mode, + destination=download_name, + path=path_summary, + finished_at=self._now_iso() if status != "requested" else None, + ) + return created["id"] + + def _record_download_failure( + self, + *, + mode: str, + path_summary: str, + download_name: str | None, + error: AppError, + history_entry_id: str | None, + ) -> None: + if not self._history_repository: + return + failure_status = "preflight_failed" if error.code == "download_preflight_failed" else "failed" + if history_entry_id: + self._history_repository.update_entry( + entry_id=history_entry_id, + status=failure_status, + error_code=error.code, + error_message=error.message, + finished_at=self._now_iso(), + ) + return + self._history_repository.create_entry( + operation="download", + status=failure_status, + source=mode, + destination=download_name, + path=path_summary, + error_code=error.code, + error_message=error.message, + finished_at=self._now_iso(), + ) + def _run_zip_download_preflight(self, resolved_targets: list) -> None: started_at = self._monotonic() state = ZipDownloadPreflightState() diff --git a/webui/backend/data/tasks.db b/webui/backend/data/tasks.db index b66e458..f6e4791 100644 Binary files a/webui/backend/data/tasks.db and b/webui/backend/data/tasks.db differ diff --git a/webui/backend/tests/golden/__pycache__/test_api_history_golden.cpython-313.pyc b/webui/backend/tests/golden/__pycache__/test_api_history_golden.cpython-313.pyc index 428e159..2edd151 100644 Binary files a/webui/backend/tests/golden/__pycache__/test_api_history_golden.cpython-313.pyc and b/webui/backend/tests/golden/__pycache__/test_api_history_golden.cpython-313.pyc differ diff --git a/webui/backend/tests/golden/test_api_history_golden.py b/webui/backend/tests/golden/test_api_history_golden.py index 6adfc8e..32f0d73 100644 --- a/webui/backend/tests/golden/test_api_history_golden.py +++ b/webui/backend/tests/golden/test_api_history_golden.py @@ -178,3 +178,75 @@ class HistoryApiGoldenTest(unittest.TestCase): self.assertEqual(history[0]['operation'], 'move') self.assertEqual(history[0]['status'], 'failed') self.assertEqual(history[0]['error_code'], 'io_error') + + def test_single_file_download_writes_ready_history_item(self) -> None: + (self.root1 / 'report.txt').write_text('hello download', encoding='utf-8') + + response = self._request('GET', '/api/files/download?path=storage1/report.txt') + + self.assertEqual(response.status_code, 200) + history = self._request('GET', '/api/history').json()['items'] + self.assertEqual(history[0]['operation'], 'download') + self.assertEqual(history[0]['status'], 'ready') + self.assertEqual(history[0]['source'], 'single_file') + self.assertEqual(history[0]['path'], 'storage1/report.txt') + self.assertEqual(history[0]['destination'], 'report.txt') + self.assertEqual(history[0]['error_code'], None) + self.assertEqual(history[0]['error_message'], None) + + def test_single_directory_zip_download_writes_ready_history_item(self) -> None: + (self.root1 / 'docs').mkdir() + (self.root1 / 'docs' / 'a.txt').write_text('A', encoding='utf-8') + + response = self._request('GET', '/api/files/download?path=storage1/docs') + + self.assertEqual(response.status_code, 200) + history = self._request('GET', '/api/history').json()['items'] + self.assertEqual(history[0]['operation'], 'download') + self.assertEqual(history[0]['status'], 'ready') + self.assertEqual(history[0]['source'], 'single_directory_zip') + self.assertEqual(history[0]['path'], 'storage1/docs') + self.assertEqual(history[0]['destination'], 'docs.zip') + + def test_multi_mixed_zip_download_writes_ready_history_item(self) -> None: + (self.root1 / 'readme.txt').write_text('R', encoding='utf-8') + (self.root1 / 'photos').mkdir() + (self.root1 / 'photos' / 'img.txt').write_text('P', encoding='utf-8') + + response = self._request('GET', '/api/files/download?path=storage1/readme.txt&path=storage1/photos') + + self.assertEqual(response.status_code, 200) + history = self._request('GET', '/api/history').json()['items'] + self.assertEqual(history[0]['operation'], 'download') + self.assertEqual(history[0]['status'], 'ready') + self.assertEqual(history[0]['source'], 'multi_zip') + self.assertEqual(history[0]['path'], 'storage1/readme.txt, storage1/photos') + self.assertRegex(history[0]['destination'], r'^kodidownload-\d{8}-\d{6}\.zip$') + + def test_download_preflight_failure_writes_preflight_failed_history_item(self) -> None: + target = self.root1 / 'real.txt' + target.write_text('x', encoding='utf-8') + (self.root1 / 'docs').mkdir() + (self.root1 / 'docs' / 'link.txt').symlink_to(target) + + response = self._request('GET', '/api/files/download?path=storage1/docs') + + self.assertEqual(response.status_code, 409) + history = self._request('GET', '/api/history').json()['items'] + self.assertEqual(history[0]['operation'], 'download') + self.assertEqual(history[0]['status'], 'preflight_failed') + self.assertEqual(history[0]['source'], 'single_directory_zip') + self.assertEqual(history[0]['path'], 'storage1/docs') + self.assertEqual(history[0]['destination'], 'docs.zip') + self.assertEqual(history[0]['error_code'], 'download_preflight_failed') + self.assertEqual(history[0]['error_message'], 'Zip download preflight failed') + + def test_download_history_uses_server_certain_statuses_only(self) -> None: + (self.root1 / 'report.txt').write_text('hello download', encoding='utf-8') + + response = self._request('GET', '/api/files/download?path=storage1/report.txt') + + self.assertEqual(response.status_code, 200) + history = self._request('GET', '/api/history').json()['items'] + self.assertIn(history[0]['status'], {'requested', 'ready', 'preflight_failed', 'failed'}) + self.assertNotIn(history[0]['status'], {'completed', 'downloaded', 'saved'}) diff --git a/webui/backend/tests/unit/__pycache__/test_history_repository.cpython-313.pyc b/webui/backend/tests/unit/__pycache__/test_history_repository.cpython-313.pyc index b030f13..02ac3e2 100644 Binary files a/webui/backend/tests/unit/__pycache__/test_history_repository.cpython-313.pyc and b/webui/backend/tests/unit/__pycache__/test_history_repository.cpython-313.pyc differ