feat: B3 uit voor veilige archive-downloads - cancel knop toegevoegd
This commit is contained in:
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
@@ -37,11 +37,26 @@ class BlockingArchiveFileOpsService(FileOpsService):
|
||||
|
||||
|
||||
class FailingArchiveFileOpsService(FileOpsService):
|
||||
def _write_download_target_to_zip(self, archive: zipfile.ZipFile, resolved_target) -> None:
|
||||
def _write_download_target_to_zip(self, archive: zipfile.ZipFile, resolved_target, on_each_item=None) -> None:
|
||||
archive.writestr("partial.txt", b"partial")
|
||||
raise OSError("forced archive failure")
|
||||
|
||||
|
||||
class BlockingArchiveBuildFileOpsService(FileOpsService):
|
||||
def __init__(self, *args, entered: threading.Event, release: threading.Event, **kwargs):
|
||||
super().__init__(*args, **kwargs)
|
||||
self._entered = entered
|
||||
self._release = release
|
||||
|
||||
def _write_download_target_to_zip(self, archive: zipfile.ZipFile, resolved_target, on_each_item=None) -> None:
|
||||
archive.writestr("partial.txt", b"partial")
|
||||
self._entered.set()
|
||||
self._release.wait(timeout=2.0)
|
||||
if on_each_item:
|
||||
on_each_item()
|
||||
super()._write_download_target_to_zip(archive, resolved_target, on_each_item=on_each_item)
|
||||
|
||||
|
||||
class DownloadApiGoldenTest(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self.temp_dir = tempfile.TemporaryDirectory()
|
||||
@@ -221,6 +236,71 @@ class DownloadApiGoldenTest(unittest.TestCase):
|
||||
self.assertEqual(task["error_code"], "io_error")
|
||||
self.assertEqual(list(self.artifact_root.glob("*")), [])
|
||||
|
||||
def test_archive_cancel_during_preparing_sets_cancelled_and_removes_partial_artifact(self) -> None:
|
||||
entered = threading.Event()
|
||||
release = threading.Event()
|
||||
file_ops_service = BlockingArchiveBuildFileOpsService(
|
||||
path_guard=self.path_guard,
|
||||
filesystem=self.filesystem,
|
||||
history_repository=self.history_repo,
|
||||
zip_download_preflight_limits=ZipDownloadPreflightLimits(),
|
||||
entered=entered,
|
||||
release=release,
|
||||
)
|
||||
self._override_services(file_ops_service=file_ops_service)
|
||||
(self.root / "docs").mkdir()
|
||||
(self.root / "docs" / "a.txt").write_text("a", encoding="utf-8")
|
||||
|
||||
created = self._request("POST", "/api/files/download/archive-prepare", {"paths": ["storage1/docs"]})
|
||||
|
||||
self.assertEqual(created.status_code, 202)
|
||||
self.assertTrue(entered.wait(timeout=2.0))
|
||||
response = self._request("POST", f"/api/files/download/archive/{created.json()['task_id']}/cancel")
|
||||
release.set()
|
||||
task = self._wait_for_task_status(created.json()["task_id"], {"cancelled"})
|
||||
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertEqual(response.json()["status"], "cancelled")
|
||||
self.assertEqual(task["status"], "cancelled")
|
||||
self.assertEqual(list(self.artifact_root.glob("*")), [])
|
||||
|
||||
def test_archive_retrieval_for_cancelled_task_rejected(self) -> None:
|
||||
entered = threading.Event()
|
||||
release = threading.Event()
|
||||
file_ops_service = BlockingArchiveBuildFileOpsService(
|
||||
path_guard=self.path_guard,
|
||||
filesystem=self.filesystem,
|
||||
history_repository=self.history_repo,
|
||||
zip_download_preflight_limits=ZipDownloadPreflightLimits(),
|
||||
entered=entered,
|
||||
release=release,
|
||||
)
|
||||
self._override_services(file_ops_service=file_ops_service)
|
||||
(self.root / "docs").mkdir()
|
||||
(self.root / "docs" / "a.txt").write_text("a", encoding="utf-8")
|
||||
|
||||
created = self._request("POST", "/api/files/download/archive-prepare", {"paths": ["storage1/docs"]})
|
||||
|
||||
self.assertTrue(entered.wait(timeout=2.0))
|
||||
cancel_response = self._request("POST", f"/api/files/download/archive/{created.json()['task_id']}/cancel")
|
||||
release.set()
|
||||
response = self._request("GET", f"/api/files/download/archive/{created.json()['task_id']}")
|
||||
|
||||
self.assertEqual(cancel_response.status_code, 200)
|
||||
self.assertEqual(response.status_code, 409)
|
||||
self.assertEqual(response.json()["error"]["code"], "download_cancelled")
|
||||
|
||||
def test_archive_cancel_after_ready_rejected(self) -> None:
|
||||
(self.root / "docs").mkdir()
|
||||
(self.root / "docs" / "a.txt").write_text("a", encoding="utf-8")
|
||||
created = self._request("POST", "/api/files/download/archive-prepare", {"paths": ["storage1/docs"]})
|
||||
task = self._wait_for_task_status(created.json()["task_id"], {"ready"})
|
||||
|
||||
response = self._request("POST", f"/api/files/download/archive/{task['id']}/cancel")
|
||||
|
||||
self.assertEqual(response.status_code, 409)
|
||||
self.assertEqual(response.json()["error"]["code"], "download_not_cancellable")
|
||||
|
||||
def test_expired_artifact_rejected_and_removed(self) -> None:
|
||||
(self.root / "docs").mkdir()
|
||||
(self.root / "docs" / "a.txt").write_text("a", encoding="utf-8")
|
||||
|
||||
@@ -3,6 +3,7 @@ from __future__ import annotations
|
||||
import asyncio
|
||||
import sys
|
||||
import tempfile
|
||||
import threading
|
||||
import time
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
@@ -11,12 +12,13 @@ import httpx
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[3]))
|
||||
|
||||
from backend.app.dependencies import get_copy_task_service, get_file_ops_service, get_history_service, get_move_task_service, get_task_service
|
||||
from backend.app.dependencies import get_archive_download_task_service, get_copy_task_service, get_file_ops_service, get_history_service, get_move_task_service, get_task_service
|
||||
from backend.app.db.history_repository import HistoryRepository
|
||||
from backend.app.db.task_repository import TaskRepository
|
||||
from backend.app.fs.filesystem_adapter import FilesystemAdapter
|
||||
from backend.app.main import app
|
||||
from backend.app.security.path_guard import PathGuard
|
||||
from backend.app.services.archive_download_task_service import ArchiveDownloadTaskService
|
||||
from backend.app.services.copy_task_service import CopyTaskService
|
||||
from backend.app.services.file_ops_service import FileOpsService
|
||||
from backend.app.services.history_service import HistoryService
|
||||
@@ -30,6 +32,21 @@ class FailingCopyFilesystemAdapter(FilesystemAdapter):
|
||||
raise OSError('forced copy failure')
|
||||
|
||||
|
||||
class BlockingArchiveBuildFileOpsService(FileOpsService):
|
||||
def __init__(self, *args, entered: threading.Event, release: threading.Event, **kwargs):
|
||||
super().__init__(*args, **kwargs)
|
||||
self._entered = entered
|
||||
self._release = release
|
||||
|
||||
def _write_download_target_to_zip(self, archive, resolved_target, on_each_item=None) -> None:
|
||||
archive.writestr("partial.txt", b"partial")
|
||||
self._entered.set()
|
||||
self._release.wait(timeout=2.0)
|
||||
if on_each_item:
|
||||
on_each_item()
|
||||
super()._write_download_target_to_zip(archive, resolved_target, on_each_item=on_each_item)
|
||||
|
||||
|
||||
class HistoryApiGoldenTest(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self.temp_dir = tempfile.TemporaryDirectory()
|
||||
@@ -38,6 +55,7 @@ class HistoryApiGoldenTest(unittest.TestCase):
|
||||
self.root1.mkdir(parents=True, exist_ok=True)
|
||||
self.root2.mkdir(parents=True, exist_ok=True)
|
||||
db_path = str(Path(self.temp_dir.name) / 'tasks.db')
|
||||
self.artifact_root = Path(self.temp_dir.name) / "archive_tmp"
|
||||
self.task_repo = TaskRepository(db_path)
|
||||
self.history_repo = HistoryRepository(db_path)
|
||||
self.path_guard = PathGuard({'storage1': str(self.root1), 'storage2': str(self.root2)})
|
||||
@@ -47,9 +65,17 @@ class HistoryApiGoldenTest(unittest.TestCase):
|
||||
app.dependency_overrides.clear()
|
||||
self.temp_dir.cleanup()
|
||||
|
||||
def _set_services(self, filesystem: FilesystemAdapter) -> None:
|
||||
def _set_services(self, filesystem: FilesystemAdapter, file_ops_service: FileOpsService | None = None) -> None:
|
||||
runner = TaskRunner(repository=self.task_repo, filesystem=filesystem, history_repository=self.history_repo)
|
||||
file_ops_service = FileOpsService(path_guard=self.path_guard, filesystem=filesystem, history_repository=self.history_repo)
|
||||
file_ops_service = file_ops_service or FileOpsService(path_guard=self.path_guard, filesystem=filesystem, history_repository=self.history_repo)
|
||||
archive_service = ArchiveDownloadTaskService(
|
||||
path_guard=self.path_guard,
|
||||
repository=self.task_repo,
|
||||
runner=runner,
|
||||
history_repository=self.history_repo,
|
||||
file_ops_service=file_ops_service,
|
||||
artifact_root=self.artifact_root,
|
||||
)
|
||||
copy_service = CopyTaskService(path_guard=self.path_guard, repository=self.task_repo, runner=runner, history_repository=self.history_repo)
|
||||
move_service = MoveTaskService(path_guard=self.path_guard, repository=self.task_repo, runner=runner, history_repository=self.history_repo)
|
||||
task_service = TaskService(repository=self.task_repo)
|
||||
@@ -58,6 +84,9 @@ class HistoryApiGoldenTest(unittest.TestCase):
|
||||
async def _override_file_ops_service() -> FileOpsService:
|
||||
return file_ops_service
|
||||
|
||||
async def _override_archive_service() -> ArchiveDownloadTaskService:
|
||||
return archive_service
|
||||
|
||||
async def _override_copy_service() -> CopyTaskService:
|
||||
return copy_service
|
||||
|
||||
@@ -71,6 +100,7 @@ class HistoryApiGoldenTest(unittest.TestCase):
|
||||
return history_service
|
||||
|
||||
app.dependency_overrides[get_file_ops_service] = _override_file_ops_service
|
||||
app.dependency_overrides[get_archive_download_task_service] = _override_archive_service
|
||||
app.dependency_overrides[get_copy_task_service] = _override_copy_service
|
||||
app.dependency_overrides[get_move_task_service] = _override_move_service
|
||||
app.dependency_overrides[get_task_service] = _override_task_service
|
||||
@@ -91,7 +121,7 @@ class HistoryApiGoldenTest(unittest.TestCase):
|
||||
while time.time() < deadline:
|
||||
response = self._request('GET', f'/api/tasks/{task_id}')
|
||||
body = response.json()
|
||||
if body['status'] in {'completed', 'failed', 'ready'}:
|
||||
if body['status'] in {'completed', 'failed', 'ready', 'cancelled'}:
|
||||
return body
|
||||
time.sleep(0.02)
|
||||
self.fail('task did not reach terminal state in time')
|
||||
@@ -244,6 +274,38 @@ class HistoryApiGoldenTest(unittest.TestCase):
|
||||
self.assertEqual(history[0]['error_code'], 'download_preflight_failed')
|
||||
self.assertEqual(history[0]['error_message'], 'Zip download preflight failed')
|
||||
|
||||
def test_download_cancellation_writes_cancelled_history_item(self) -> None:
|
||||
entered = threading.Event()
|
||||
release = threading.Event()
|
||||
file_ops_service = BlockingArchiveBuildFileOpsService(
|
||||
path_guard=self.path_guard,
|
||||
filesystem=FilesystemAdapter(),
|
||||
history_repository=self.history_repo,
|
||||
entered=entered,
|
||||
release=release,
|
||||
)
|
||||
self._set_services(FilesystemAdapter(), file_ops_service=file_ops_service)
|
||||
(self.root1 / 'docs').mkdir()
|
||||
(self.root1 / 'docs' / 'a.txt').write_text('A', encoding='utf-8')
|
||||
|
||||
response = self._request('POST', '/api/files/download/archive-prepare', {'paths': ['storage1/docs']})
|
||||
|
||||
self.assertEqual(response.status_code, 202)
|
||||
self.assertTrue(entered.wait(timeout=2.0))
|
||||
cancel = self._request('POST', f"/api/files/download/archive/{response.json()['task_id']}/cancel")
|
||||
release.set()
|
||||
self._wait_task(response.json()['task_id'])
|
||||
history = self._request('GET', '/api/history').json()['items']
|
||||
|
||||
self.assertEqual(cancel.status_code, 200)
|
||||
self.assertEqual(history[0]['operation'], 'download')
|
||||
self.assertEqual(history[0]['status'], 'cancelled')
|
||||
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'], None)
|
||||
self.assertEqual(history[0]['error_message'], None)
|
||||
|
||||
def test_download_history_uses_server_certain_statuses_only(self) -> None:
|
||||
(self.root1 / 'report.txt').write_text('hello download', encoding='utf-8')
|
||||
|
||||
@@ -251,5 +313,5 @@ class HistoryApiGoldenTest(unittest.TestCase):
|
||||
|
||||
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.assertIn(history[0]['status'], {'requested', 'ready', 'preflight_failed', 'failed', 'cancelled'})
|
||||
self.assertNotIn(history[0]['status'], {'completed', 'downloaded', 'saved'})
|
||||
|
||||
@@ -263,6 +263,28 @@ class TasksApiGoldenTest(unittest.TestCase):
|
||||
self.assertEqual(body["status"], "ready")
|
||||
self.assertEqual(body["destination"], "docs.zip")
|
||||
|
||||
def test_get_task_detail_cancelled_archive_download(self) -> None:
|
||||
self._insert_task(
|
||||
task_id="task-download-cancelled",
|
||||
operation="download",
|
||||
status="cancelled",
|
||||
source="storage1/docs",
|
||||
destination="docs.zip",
|
||||
created_at="2026-03-10T10:00:00Z",
|
||||
started_at="2026-03-10T10:00:01Z",
|
||||
finished_at="2026-03-10T10:00:03Z",
|
||||
done_items=0,
|
||||
total_items=1,
|
||||
)
|
||||
|
||||
response = self._get("/api/tasks/task-download-cancelled")
|
||||
|
||||
self.assertEqual(response.status_code, 200)
|
||||
body = response.json()
|
||||
self.assertEqual(body["operation"], "download")
|
||||
self.assertEqual(body["status"], "cancelled")
|
||||
self.assertEqual(body["destination"], "docs.zip")
|
||||
|
||||
def test_get_task_not_found(self) -> None:
|
||||
response = self._get("/api/tasks/task-missing")
|
||||
|
||||
|
||||
@@ -74,6 +74,7 @@ class UiSmokeGoldenTest(unittest.TestCase):
|
||||
self.assertIn('id="download-modal-progress-bar"', body)
|
||||
self.assertIn('id="download-modal-count"', body)
|
||||
self.assertIn('id="download-modal-status"', body)
|
||||
self.assertIn('id="download-modal-cancel-btn"', body)
|
||||
self.assertIn('id="download-modal-close-btn"', body)
|
||||
self.assertIn('id="context-menu"', body)
|
||||
self.assertIn('id="context-menu-scope"', body)
|
||||
@@ -231,11 +232,14 @@ class UiSmokeGoldenTest(unittest.TestCase):
|
||||
self.assertIn('function openZipDownloadModal(selectedItems)', app_js)
|
||||
self.assertIn('function markZipDownloadReady(fileName)', app_js)
|
||||
self.assertIn('function markZipDownloadFailed(err)', app_js)
|
||||
self.assertIn('function markZipDownloadCancelled()', app_js)
|
||||
self.assertIn('function closeDownloadModal()', app_js)
|
||||
self.assertIn('function zipDownloadRequestKey(paths)', app_js)
|
||||
self.assertIn('async function createArchiveDownloadTask(paths)', app_js)
|
||||
self.assertIn('async function getTaskRequest(taskId)', app_js)
|
||||
self.assertIn('async function cancelArchiveDownloadTask(taskId)', app_js)
|
||||
self.assertIn('function startArchiveDownload(taskId, fileName)', app_js)
|
||||
self.assertIn('async function requestArchiveDownloadCancel()', app_js)
|
||||
self.assertIn('async function waitForArchiveDownloadReady(taskId)', app_js)
|
||||
self.assertIn('function contextMenuElements()', app_js)
|
||||
self.assertIn('function openContextMenu(pane, entry, event)', app_js)
|
||||
@@ -251,12 +255,15 @@ class UiSmokeGoldenTest(unittest.TestCase):
|
||||
self.assertIn('statusText: "Download started"', app_js)
|
||||
self.assertIn('countText: "Browser download started"', app_js)
|
||||
self.assertIn('countText: "Zip download failed"', app_js)
|
||||
self.assertIn('countText: "Zip download cancelled"', app_js)
|
||||
self.assertIn('statusText: "Cancelling download..."', app_js)
|
||||
self.assertIn('statusText: err.message || "Download failed"', app_js)
|
||||
self.assertIn('downloadProgressState.requestKey === requestKey', app_js)
|
||||
self.assertIn('setStatus("Preparing download...");', app_js)
|
||||
self.assertIn('"/api/files/download/archive-prepare"', app_js)
|
||||
self.assertIn('`/api/tasks/${encodeURIComponent(taskId)}`', app_js)
|
||||
self.assertIn('`/api/files/download/archive/${encodeURIComponent(taskId)}`', app_js)
|
||||
self.assertIn('`/api/files/download/archive/${encodeURIComponent(taskId)}/cancel`', app_js)
|
||||
self.assertIn('function applyContextMenuSelection()', app_js)
|
||||
self.assertIn('function startContextMenuOpen()', app_js)
|
||||
self.assertIn('function startContextMenuEdit()', app_js)
|
||||
|
||||
Binary file not shown.
@@ -80,6 +80,21 @@ class TaskRepositoryTest(unittest.TestCase):
|
||||
self.assertEqual(task["status"], "requested")
|
||||
self.assertEqual(artifact["file_name"], "docs.zip")
|
||||
|
||||
def test_mark_cancelled_transitions_requested_download_task(self) -> None:
|
||||
created = self.repo.create_task(
|
||||
operation="download",
|
||||
source="storage1/docs",
|
||||
destination="docs.zip",
|
||||
status="requested",
|
||||
)
|
||||
|
||||
changed = self.repo.mark_cancelled(created["id"])
|
||||
task = self.repo.get_task(created["id"])
|
||||
|
||||
self.assertTrue(changed)
|
||||
self.assertEqual(task["status"], "cancelled")
|
||||
self.assertIsNotNone(task["finished_at"])
|
||||
|
||||
def test_migrates_legacy_tasks_schema_missing_source_destination(self) -> None:
|
||||
legacy_db_path = Path(self.temp_dir.name) / "legacy.db"
|
||||
conn = sqlite3.connect(legacy_db_path)
|
||||
|
||||
Reference in New Issue
Block a user