diff --git a/project_docs/API_GOLDEN.md b/project_docs/API_GOLDEN.md index 71e727e..0fc7452 100644 --- a/project_docs/API_GOLDEN.md +++ b/project_docs/API_GOLDEN.md @@ -129,7 +129,7 @@ Response shape: } ``` -Voor task-based file-actions `copy`, `move` en `duplicate` betekenen progressvelden: +Voor task-based file-actions `copy`, `move`, `duplicate` en `delete` betekenen progressvelden: - `done_items`: aantal volledig verwerkte bestanden - `total_items`: exact aantal te verwerken bestanden in de hele task - `current_item`: taakrelatief bestandspad als beschikbaar, anders bestandsnaam @@ -138,6 +138,10 @@ Voor `move` geldt een expliciete uitzondering: - file-gebaseerde move-paden rapporteren file-progress - same-root directory moves behouden directe rename-semantiek en rapporteren daarom grovere item-progress per directory-operatie +Voor `delete` geldt: +- recursive delete van directorytrees rapporteert file-progress per verwijderd bestand +- lege mappen of directory-only deletes houden `done_items = 0`, `total_items = 0` en gebruiken geen kunstmatige file-teller + ### `POST /api/tasks/{task_id}/cancel` Success for cancellable file-action task: ```json diff --git a/webui/backend/app/__pycache__/tasks_runner.cpython-313.pyc b/webui/backend/app/__pycache__/tasks_runner.cpython-313.pyc index bb13cfc..e924dbf 100644 Binary files a/webui/backend/app/__pycache__/tasks_runner.cpython-313.pyc and b/webui/backend/app/__pycache__/tasks_runner.cpython-313.pyc differ diff --git a/webui/backend/app/services/__pycache__/delete_task_service.cpython-313.pyc b/webui/backend/app/services/__pycache__/delete_task_service.cpython-313.pyc index f585653..c85e51e 100644 Binary files a/webui/backend/app/services/__pycache__/delete_task_service.cpython-313.pyc and b/webui/backend/app/services/__pycache__/delete_task_service.cpython-313.pyc differ diff --git a/webui/backend/app/services/delete_task_service.py b/webui/backend/app/services/delete_task_service.py index b80bc05..9dc3308 100644 --- a/webui/backend/app/services/delete_task_service.py +++ b/webui/backend/app/services/delete_task_service.py @@ -2,6 +2,7 @@ from __future__ import annotations import uuid from datetime import datetime, timezone +from pathlib import Path from backend.app.api.errors import AppError from backend.app.api.schemas import TaskCreateResponse @@ -26,31 +27,12 @@ class DeleteTaskService: def create_delete_task(self, path: str, recursive: bool = False) -> TaskCreateResponse: try: - resolved_target = self._path_guard.resolve_existing_path(path) - - if resolved_target.absolute.is_file(): - kind = "file" - elif resolved_target.absolute.is_dir(): - kind = "directory" - if not recursive and any(resolved_target.absolute.iterdir()): - raise AppError( - code="directory_not_empty", - message="Directory is not empty", - status_code=409, - details={"path": resolved_target.relative}, - ) - else: - raise AppError( - code="type_conflict", - message="Unsupported path type for delete", - status_code=409, - details={"path": resolved_target.relative}, - ) + item = self._build_delete_item(path=path, recursive=recursive) task_id = str(uuid.uuid4()) task = self._repository.create_task( operation="delete", - source=resolved_target.relative, + source=item["relative_path"], destination="", task_id=task_id, ) @@ -58,14 +40,9 @@ class DeleteTaskService: entry_id=task_id, operation="delete", status="queued", - path=resolved_target.relative, - ) - self._runner.enqueue_delete_path( - task_id=task["id"], - target=str(resolved_target.absolute), - kind=kind, - recursive=recursive, + path=item["relative_path"], ) + self._runner.enqueue_delete_path(task_id=task["id"], item=item) return TaskCreateResponse(task_id=task["id"], status=task["status"]) except AppError as exc: self._record_history( @@ -94,6 +71,66 @@ class DeleteTaskService: ) raise error + def _build_delete_item(self, path: str, recursive: bool) -> dict: + resolved_target = self._path_guard.resolve_existing_path(path) + + if resolved_target.absolute.is_file(): + files = [{"path": str(resolved_target.absolute), "label": resolved_target.absolute.name}] + directories: list[str] = [] + kind = "file" + elif resolved_target.absolute.is_dir(): + kind = "directory" + if not recursive and any(resolved_target.absolute.iterdir()): + raise AppError( + code="directory_not_empty", + message="Directory is not empty", + status_code=409, + details={"path": resolved_target.relative}, + ) + if recursive: + files, directories = self._build_recursive_delete_plan(resolved_target.absolute) + else: + files = [] + directories = [str(resolved_target.absolute)] + else: + raise AppError( + code="type_conflict", + message="Unsupported path type for delete", + status_code=409, + details={"path": resolved_target.relative}, + ) + + return { + "target": str(resolved_target.absolute), + "relative_path": resolved_target.relative, + "kind": kind, + "recursive": recursive, + "files": files, + "directories": directories, + "progress_total_items": len(files), + "progress_label": files[0]["label"] if files else None, + } + + def _build_recursive_delete_plan(self, root: Path) -> tuple[list[dict[str, str]], list[str]]: + files: list[dict[str, str]] = [] + directories: list[str] = [] + + def walk(path: Path, relative_prefix: Path) -> None: + for entry in sorted(path.iterdir(), key=lambda child: child.name.lower()): + relative_path = relative_prefix / entry.name + if entry.is_symlink(): + files.append({"path": str(entry), "label": relative_path.as_posix()}) + continue + if entry.is_dir(): + walk(entry, relative_path) + directories.append(str(entry)) + continue + files.append({"path": str(entry), "label": relative_path.as_posix()}) + + walk(root, Path()) + directories.append(str(root)) + return files, directories + def _record_history(self, **kwargs) -> None: if self._history_repository: self._history_repository.create_entry(**kwargs) diff --git a/webui/backend/app/tasks_runner.py b/webui/backend/app/tasks_runner.py index 9d424c6..538193a 100644 --- a/webui/backend/app/tasks_runner.py +++ b/webui/backend/app/tasks_runner.py @@ -80,10 +80,10 @@ class TaskRunner: ) thread.start() - def enqueue_delete_path(self, task_id: str, target: str, kind: str, recursive: bool) -> None: + def enqueue_delete_path(self, task_id: str, item: dict[str, object]) -> None: thread = threading.Thread( target=self._run_delete_path, - args=(task_id, target, kind, recursive), + args=(task_id, item), daemon=True, ) thread.start() @@ -429,39 +429,57 @@ class TaskRunner: total_items=total_items, ) - def _run_delete_path(self, task_id: str, target: str, kind: str, recursive: bool) -> None: + def _run_delete_path(self, task_id: str, item: dict[str, object]) -> None: + target = str(item["target"]) + kind = str(item["kind"]) + recursive = bool(item["recursive"]) + files = list(item.get("files", [])) # type: ignore[arg-type] + directories = list(item.get("directories", [])) # type: ignore[arg-type] + total_items = int(item.get("progress_total_items", len(files))) + current_item = str(item.get("progress_label")) if item.get("progress_label") else None if not self._repository.mark_running( task_id=task_id, done_items=0, - total_items=1, - current_item=target, + total_items=total_items, + current_item=current_item, ): - self._finalize_if_already_cancelled(task_id, done_items=0, total_items=1) + self._finalize_if_already_cancelled(task_id, done_items=0, total_items=total_items) return + completed_items = 0 try: - path = Path(target) if kind == "file": - self._filesystem.delete_file(path) + file_entry = files[0] + completed_items = self._delete_planned_file(task_id, file_entry, completed_items, total_items) elif recursive: - self._filesystem.delete_directory_recursive(path) + for file_entry in files: + if self._is_cancel_requested(task_id): + self._finalize_cancelled(task_id, done_items=completed_items, total_items=total_items) + return + completed_items = self._delete_planned_file(task_id, file_entry, completed_items, total_items) + if self._is_cancel_requested(task_id): + self._finalize_cancelled(task_id, done_items=completed_items, total_items=total_items) + return + for directory in directories: + self._filesystem.delete_empty_directory(Path(directory)) else: - self._filesystem.delete_empty_directory(path) + self._filesystem.delete_empty_directory(Path(target)) self._complete_or_cancel_item_task( task_id=task_id, - done_items=1, - total_items=1, + done_items=completed_items, + total_items=total_items, ) except OSError as exc: + task = self._repository.get_task(task_id) self._repository.mark_failed( task_id=task_id, error_code="io_error", error_message=str(exc), - failed_item=target, + failed_item=(task.get("current_item") if task else None) or target, done_bytes=None, total_bytes=None, - done_items=0, - total_items=1, + done_items=completed_items, + total_items=total_items, ) self._update_history_failed(task_id, str(exc)) @@ -630,6 +648,29 @@ class TaskRunner: ) return completed_items + def _delete_planned_file( + self, + task_id: str, + file_entry: dict[str, str], + completed_items: int, + total_items: int, + ) -> int: + self._repository.update_progress( + task_id=task_id, + done_items=completed_items, + total_items=total_items, + current_item=file_entry["label"], + ) + self._filesystem.delete_file(Path(file_entry["path"])) + completed_items += 1 + self._repository.update_progress( + task_id=task_id, + done_items=completed_items, + total_items=total_items, + current_item=self._next_item_label_after_completion(completed_items, total_items, file_entry["label"]), + ) + return completed_items + def _move_directory_item( self, task_id: str, diff --git a/webui/backend/data/tasks.db b/webui/backend/data/tasks.db index 835af55..100c233 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_file_ops_golden.cpython-313.pyc b/webui/backend/tests/golden/__pycache__/test_api_file_ops_golden.cpython-313.pyc index 48b2102..9ad4f63 100644 Binary files a/webui/backend/tests/golden/__pycache__/test_api_file_ops_golden.cpython-313.pyc and b/webui/backend/tests/golden/__pycache__/test_api_file_ops_golden.cpython-313.pyc differ diff --git a/webui/backend/tests/golden/__pycache__/test_api_tasks_golden.cpython-313.pyc b/webui/backend/tests/golden/__pycache__/test_api_tasks_golden.cpython-313.pyc index 00cad5d..a0c278a 100644 Binary files a/webui/backend/tests/golden/__pycache__/test_api_tasks_golden.cpython-313.pyc and b/webui/backend/tests/golden/__pycache__/test_api_tasks_golden.cpython-313.pyc differ diff --git a/webui/backend/tests/golden/test_api_file_ops_golden.py b/webui/backend/tests/golden/test_api_file_ops_golden.py index 23f5ac6..fb45249 100644 --- a/webui/backend/tests/golden/test_api_file_ops_golden.py +++ b/webui/backend/tests/golden/test_api_file_ops_golden.py @@ -293,6 +293,9 @@ class FileOpsApiGoldenTest(unittest.TestCase): detail = self._wait_task(body["task_id"]) self.assertEqual(detail["operation"], "delete") self.assertEqual(detail["status"], "completed") + self.assertEqual(detail["done_items"], 1) + self.assertEqual(detail["total_items"], 1) + self.assertIsNone(detail["current_item"]) self.assertFalse(target.exists()) def test_delete_file_cancelled_after_current_delete_finishes(self) -> None: @@ -330,7 +333,7 @@ class FileOpsApiGoldenTest(unittest.TestCase): task_id = response.json()["task_id"] self.assertTrue(blocking_fs.entered.wait(timeout=2.0)) running = self._wait_for_status(task_id, {"running"}) - self.assertEqual(running["current_item"], str(self.scope / "delete_later.txt")) + self.assertEqual(running["current_item"], "delete_later.txt") cancel_response = self._post(f"/api/tasks/{task_id}/cancel", {}) self.assertEqual(cancel_response.status_code, 200) @@ -358,6 +361,9 @@ class FileOpsApiGoldenTest(unittest.TestCase): detail = self._wait_task(body["task_id"]) self.assertEqual(detail["operation"], "delete") self.assertEqual(detail["status"], "completed") + self.assertEqual(detail["done_items"], 0) + self.assertEqual(detail["total_items"], 0) + self.assertIsNone(detail["current_item"]) self.assertFalse(target.exists()) def test_delete_not_found(self) -> None: @@ -424,6 +430,7 @@ class FileOpsApiGoldenTest(unittest.TestCase): nested = target / "nested" nested.mkdir() (nested / "a.txt").write_text("a", encoding="utf-8") + (target / "b.txt").write_text("b", encoding="utf-8") response = self._post( "/api/files/delete", @@ -436,8 +443,67 @@ class FileOpsApiGoldenTest(unittest.TestCase): detail = self._wait_task(body["task_id"]) self.assertEqual(detail["operation"], "delete") self.assertEqual(detail["status"], "completed") + self.assertEqual(detail["done_items"], 2) + self.assertEqual(detail["total_items"], 2) + self.assertIsNone(detail["current_item"]) self.assertFalse(target.exists()) + def test_delete_non_empty_directory_recursive_cancelled_after_current_file_finishes(self) -> None: + blocking_fs = BlockingDeleteFilesystemAdapter() + path_guard = PathGuard({"storage1": str(self.root)}) + service = FileOpsService(path_guard=path_guard, filesystem=blocking_fs) + delete_service = DeleteTaskService( + path_guard=path_guard, + repository=self.repo, + runner=TaskRunner(repository=self.repo, filesystem=blocking_fs), + ) + task_service = TaskService(repository=self.repo) + + async def _override_file_ops_service() -> FileOpsService: + return service + + async def _override_delete_task_service() -> DeleteTaskService: + return delete_service + + async def _override_task_service() -> TaskService: + return task_service + + app.dependency_overrides[get_file_ops_service] = _override_file_ops_service + app.dependency_overrides[get_delete_task_service] = _override_delete_task_service + app.dependency_overrides[get_task_service] = _override_task_service + + target = self.scope / "delete_recursive_later" + target.mkdir() + nested = target / "nested" + nested.mkdir() + (target / "a.txt").write_text("a", encoding="utf-8") + (nested / "b.txt").write_text("b", encoding="utf-8") + + response = self._post( + "/api/files/delete", + {"path": "storage1/scope/delete_recursive_later", "recursive": True}, + ) + + task_id = response.json()["task_id"] + self.assertTrue(blocking_fs.entered.wait(timeout=2.0)) + running = self._wait_for_status(task_id, {"running"}) + self.assertEqual(running["current_item"], "a.txt") + self.assertEqual(running["done_items"], 0) + self.assertEqual(running["total_items"], 2) + + cancel_response = self._post(f"/api/tasks/{task_id}/cancel", {}) + self.assertEqual(cancel_response.status_code, 200) + self.assertEqual(cancel_response.json()["status"], "cancelling") + + blocking_fs.release.set() + detail = self._wait_task(task_id) + self.assertEqual(detail["status"], "cancelled") + self.assertEqual(detail["done_items"], 1) + self.assertEqual(detail["total_items"], 2) + self.assertFalse(target.joinpath("a.txt").exists()) + self.assertTrue(target.joinpath("nested", "b.txt").exists()) + self.assertTrue(target.exists()) + def test_delete_invalid_path(self) -> None: response = self._post( "/api/files/delete", diff --git a/webui/backend/tests/golden/test_api_tasks_golden.py b/webui/backend/tests/golden/test_api_tasks_golden.py index d84ffc4..5eaa0b0 100644 --- a/webui/backend/tests/golden/test_api_tasks_golden.py +++ b/webui/backend/tests/golden/test_api_tasks_golden.py @@ -260,7 +260,7 @@ class TasksApiGoldenTest(unittest.TestCase): started_at="2026-03-10T10:00:01Z", done_items=0, total_items=1, - current_item="storage1/trash.txt", + current_item="trash.txt", ) response = self._get("/api/tasks/task-delete") @@ -271,7 +271,7 @@ class TasksApiGoldenTest(unittest.TestCase): self.assertEqual(body["status"], "running") self.assertEqual(body["done_items"], 0) self.assertEqual(body["total_items"], 1) - self.assertEqual(body["current_item"], "storage1/trash.txt") + self.assertEqual(body["current_item"], "trash.txt") def test_cancel_running_delete_task_returns_cancelling(self) -> None: self._insert_task( @@ -284,7 +284,7 @@ class TasksApiGoldenTest(unittest.TestCase): started_at="2026-03-10T10:00:01Z", done_items=0, total_items=1, - current_item="storage1/trash.txt", + current_item="trash.txt", ) response = self._post("/api/tasks/task-delete/cancel") @@ -293,7 +293,7 @@ class TasksApiGoldenTest(unittest.TestCase): body = response.json() self.assertEqual(body["operation"], "delete") self.assertEqual(body["status"], "cancelling") - self.assertEqual(body["current_item"], "storage1/trash.txt") + self.assertEqual(body["current_item"], "trash.txt") def test_cancel_completed_task_rejected(self) -> None: self._insert_task(