diff --git a/project_docs/LOCAL_UPLOAD_V1_DESIGN.md b/project_docs/LOCAL_UPLOAD_V1_DESIGN.md new file mode 100644 index 0000000..91a739c --- /dev/null +++ b/project_docs/LOCAL_UPLOAD_V1_DESIGN.md @@ -0,0 +1,233 @@ +# Local Upload v1 + +## 1. Doel +Local upload voegt nu direct waarde toe omdat de app al een bruikbare dual-pane bestandsworkflow heeft, maar nog geen ingang om bestanden vanaf de lokale machine de beheerde storage in te brengen. Dat gat is functioneel groot: browse, rename, move, copy en delete bestaan al, maar import ontbreekt. + +Binnen de dual-pane workflow is de meest natuurlijke semantiek: +- bron: lokale machine via de native browser file picker +- doel: `currentPath` van het actieve paneel + +Dat houdt het model eenvoudig en voorspelbaar. De gebruiker kiest eerst waar in de storage hij staat, en uploadt daarna naar die locatie. + +## 2. Scope +Aanbevolen scope voor v1: +- upload van lokale bestanden via browser naar storage +- target = `currentPath` van het actieve paneel +- native browser file picker gebruiken +- single-file upload +- multi-file upload +- geen folder upload in v1 +- geen drag & drop in v1 +- geen resumable upload +- geen chunked upload + +Motivatie: +- Multi-file upload via de native picker is klein en nuttig. +- Folder upload verhoogt de complexiteit direct sterk: recursie, conflictgedrag, voortgang, directory-creatie, mixed failures. +- Drag & drop is UX-matig aantrekkelijk, maar voegt event-complexiteit en extra foutpaden toe zonder dat het nodig is voor een eerste bruikbare versie. +- Chunking/resume is pas zinvol als gewone multipart upload aantoonbaar onvoldoende is. + +## 3. Startgedrag / UI +Voor v1: +- een `Upload` knop links van `F1 Settings` in de onderbalk/topactiezone waar die nu logisch past +- klik op `Upload` opent direct de native browser file picker +- de upload werkt altijd naar het actieve paneel +- de UI toont compact en expliciet: + - `Upload to: ` + +Aanbevolen flow: +1. gebruiker activeert een paneel +2. gebruiker klikt `Upload` +3. browser opent native file picker +4. gebruiker kiest 1 of meerdere bestanden +5. upload start naar `currentPath` van actief paneel +6. voortgang wordt zichtbaar +7. na afronding wordt het actieve paneel refreshed + +Belangrijk: +- de actieve-paneelcontext moet vooraf duidelijk zijn +- de knop hoeft niet disabled te zijn zolang een geldig `currentPath` bestaat +- als een modal open is, moet `Upload` niet tegelijk een nieuwe flow starten + +## 4. Voortgang +Aanbevolen v1-model: +- één compacte upload-progress UI per lopende uploadbatch +- globale voortgang over de batch +- daarnaast compacte status per huidig bestand indien nodig + +V1 hoeft niet meteen een volledige task-UI te hergebruiken. De eenvoudigste bruikbare richting is: +- één uploadstatusblok of kleine modal +- toont: + - totaal aantal bestanden + - huidig bestand + - globale voortgangsbalk of percentage + +Aanbevolen velden in de UI: +- `Uploading 3 files to /Volumes/...` +- `2/3 files` +- huidige bestandsnaam +- percentage of bytes-progress voor de actieve upload + +Dit is lichter dan de bestaande task-list volledig integreren in v1. + +## 5. Backend-impact +Er is zeer waarschijnlijk een nieuw upload-endpoint nodig, bijvoorbeeld: +- `POST /api/files/upload` + +Verwachte vorm: +- multipart/form-data +- target path als apart veld, bijvoorbeeld `target_path` +- één of meerdere file parts + +Veiligheidsmodel: +- `target_path` altijd via bestaande `path_guard` +- target moet binnen whitelist/toegestane roots vallen +- target moet bestaan +- target moet een directory zijn +- bestandsnamen niet vertrouwen vanuit clientpad-informatie +- alleen de basename van het gekozen lokale bestand gebruiken +- validatie van naam via bestaande naamregels (`validate_name` of equivalent) +- geen client-side padsegmenten overnemen + +Traversalpreventie: +- geen directorystructuur uit de browser aan serverzijde interpreteren in v1 +- geen relatieve paden uit multipart metadata vertrouwen +- ieder bestand wordt server-side gemapt naar: + - `target_path / validated_basename` + +## 6. Conflictgedrag +Ontwerp voor Engelstalige keuzes: +- `Overwrite` +- `Overwrite all` +- `Skip` +- `Cancel` + +Aanbevolen v1-gedrag: +- conflictcontrole gebeurt server-side per bestand +- bij conflict in een batch wordt de batch niet stil doorgezet +- de UI toont een compacte conflictmodal voor het huidige conflicterende bestand +- de gebruiker kiest één actie + +Semantiek: +- `Overwrite`: alleen huidig conflicterend bestand overschrijven +- `Overwrite all`: huidig en alle volgende conflicten automatisch overschrijven +- `Skip`: huidig conflicterend bestand overslaan en doorgaan +- `Cancel`: resterende batch stoppen + +Aanbevolen v1-realisatie: +- conflict afhandelen per bestand binnen de uploadbatch-flow +- geen complexe vooraf-scan van alle conflicten nodig +- geen rollback + +Belangrijk: +- ook directoryconflicten moeten duidelijk zijn +- als target al een directory met dezelfde naam bevat voor een file-upload, moet dat als conflict/typefout behandeld worden + +## 7. Grote bestanden / performance +Aanbevolen v1: +- gewone multipart upload +- geen chunking +- geen resumable upload + +Motivatie: +- technisch het eenvoudigst +- breed ondersteund door browser en backendstack +- voldoende voor een eerste bruikbare versie + +Risico: +- zeer grote bestanden kunnen lang duren of mislukken bij netwerkonderbreking +- dat risico moet in v1 geaccepteerd en netjes gecommuniceerd worden + +V1 hoeft daarom niet meer te doen dan: +- voortgang tonen +- foutmelding tonen bij mislukking +- geen herstart of resume bieden + +## 8. Relatie met tasks/history +Aanbevolen v1: +- upload opnemen in `history` +- upload niet meteen in het generieke `tasks` model stoppen + +Motivatie: +- upload heeft wel auditwaarde, dus history is logisch +- task-integratie maakt de slice groter: background execution, task persistence, progress mapping, polling-UI integratie +- voor een eerste bruikbare upload is een lichtere directe UI-flow met history-opslag pragmatischer + +History v1 voor upload zou moeten registreren: +- operation = `upload` +- status = `completed` / `failed` +- destination = doelpad +- path of source-naam waar nuttig +- error_code / error_message bij failure + +Als later blijkt dat uploads langlopend worden of meerdere gelijktijdige uploads normaal zijn, kan task-integratie in v2 logisch worden. + +## 9. Regressierisico +Belangrijkste risico's: +- security: onbetrouwbare bestandsnamen of target path misbruik +- grote bestanden: timeouts of langlopende requests +- foutafhandeling: deels geslaagde batch zonder duidelijke feedback +- UI-complexiteit: conflictflow kan snel onrustig worden +- actieve-paneelcontext: upload naar verkeerd paneel/pad als context niet duidelijk is +- conflictafhandeling: onduidelijke semantiek rond overwrite/skip + +Laag-regressierisico aanpak: +- target altijd expliciet koppelen aan actief paneel +- geen folder upload +- geen drag & drop +- geen chunking/resume +- compacte conflictmodal per bestand +- direct paneelrefresh na succesvolle upload(s) + +## 10. Teststrategie +Backend golden tests: +- upload single file success +- upload multi-file success +- target path not found +- target path is file -> type_conflict +- traversal blocked +- invalid root alias +- invalid filename blocked +- conflict -> already_exists of equivalent +- overwrite success +- skip/cancel flow indien servercontract dat nodig maakt + +UI smoke/regressietests: +- `Upload` knop aanwezig links van `F1 Settings` +- geen uploadstart als ongeldige UI-context aanwezig is +- targetpaneel-context zichtbaar in uploadflow +- progress UI verschijnt +- conflictkeuze-UI verschijnt met: + - `Overwrite` + - `Overwrite all` + - `Skip` + - `Cancel` + +Handmatige validatie: +- upload 1 klein bestand +- upload meerdere bestanden +- conflict op bestaand bestand +- overwrite all werkt over meerdere conflicten +- skip laat batch doorgaan +- cancel stopt batch +- actief paneel bepaalt doelpad correct +- history bevat upload-resultaten + +## 11. Aanbeveling +Aanbevolen v1-richting met laag regressierisico: +- native browser file picker +- single + multi-file upload +- target = `currentPath` van actief paneel +- geen folder upload +- geen drag & drop +- gewone multipart upload +- directe voortgangsweergave in lichte upload-UI +- conflictafhandeling per bestand met: + - `Overwrite` + - `Overwrite all` + - `Skip` + - `Cancel` +- wel history-integratie +- nog geen task-integratie + +Dit is de kleinste versie die echt bruikbaar is, zonder meteen te ontsporen in mediaserver- of synchronisatiecomplexiteit. diff --git a/webui/backend/app/api/__pycache__/routes_files.cpython-313.pyc b/webui/backend/app/api/__pycache__/routes_files.cpython-313.pyc index a3b0caa..bd83a8a 100644 Binary files a/webui/backend/app/api/__pycache__/routes_files.cpython-313.pyc and b/webui/backend/app/api/__pycache__/routes_files.cpython-313.pyc differ diff --git a/webui/backend/app/api/__pycache__/schemas.cpython-313.pyc b/webui/backend/app/api/__pycache__/schemas.cpython-313.pyc index c3eb33d..e9dce69 100644 Binary files a/webui/backend/app/api/__pycache__/schemas.cpython-313.pyc and b/webui/backend/app/api/__pycache__/schemas.cpython-313.pyc differ diff --git a/webui/backend/app/api/routes_files.py b/webui/backend/app/api/routes_files.py index 5738ed0..812b687 100644 --- a/webui/backend/app/api/routes_files.py +++ b/webui/backend/app/api/routes_files.py @@ -1,9 +1,9 @@ from __future__ import annotations -from fastapi import APIRouter, Depends, Request +from fastapi import APIRouter, Depends, File, Form, Request, UploadFile from fastapi.responses import StreamingResponse -from backend.app.api.schemas import DeleteRequest, DeleteResponse, FileInfoResponse, MkdirRequest, MkdirResponse, RenameRequest, RenameResponse, SaveRequest, SaveResponse, ViewResponse +from backend.app.api.schemas import DeleteRequest, DeleteResponse, FileInfoResponse, MkdirRequest, MkdirResponse, RenameRequest, RenameResponse, SaveRequest, SaveResponse, UploadResponse, ViewResponse from backend.app.dependencies import get_file_ops_service from backend.app.services.file_ops_service import FileOpsService @@ -34,6 +34,15 @@ async def delete( return service.delete(path=request.path) +@router.post("/upload", response_model=UploadResponse) +async def upload( + target_path: str = Form(...), + file: UploadFile = File(...), + service: FileOpsService = Depends(get_file_ops_service), +) -> UploadResponse: + return service.upload(target_path=target_path, upload_file=file) + + @router.get("/view", response_model=ViewResponse) async def view( path: str, diff --git a/webui/backend/app/api/schemas.py b/webui/backend/app/api/schemas.py index 074d014..52e1ad2 100644 --- a/webui/backend/app/api/schemas.py +++ b/webui/backend/app/api/schemas.py @@ -58,6 +58,12 @@ class DeleteResponse(BaseModel): path: str +class UploadResponse(BaseModel): + path: str + size: int + modified: str + + class ViewResponse(BaseModel): path: str name: str 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 31a6984..007e0fa 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 a9d32b0..2073f08 100644 --- a/webui/backend/app/db/history_repository.py +++ b/webui/backend/app/db/history_repository.py @@ -7,7 +7,7 @@ from datetime import datetime, timezone from pathlib import Path VALID_HISTORY_STATUSES = {"queued", "completed", "failed"} -VALID_HISTORY_OPERATIONS = {"mkdir", "rename", "delete", "copy", "move"} +VALID_HISTORY_OPERATIONS = {"mkdir", "rename", "delete", "copy", "move", "upload"} class HistoryRepository: diff --git a/webui/backend/app/fs/__pycache__/filesystem_adapter.cpython-313.pyc b/webui/backend/app/fs/__pycache__/filesystem_adapter.cpython-313.pyc index eb48fd5..faa5900 100644 Binary files a/webui/backend/app/fs/__pycache__/filesystem_adapter.cpython-313.pyc and b/webui/backend/app/fs/__pycache__/filesystem_adapter.cpython-313.pyc differ diff --git a/webui/backend/app/fs/filesystem_adapter.py b/webui/backend/app/fs/filesystem_adapter.py index 03ea6e6..ebc8938 100644 --- a/webui/backend/app/fs/filesystem_adapter.py +++ b/webui/backend/app/fs/filesystem_adapter.py @@ -140,6 +140,18 @@ class FilesystemAdapter: "modified": self.modified_iso(path), } + def write_uploaded_file(self, path: Path, file_stream, chunk_size: int = 1024 * 1024) -> dict: + with path.open("xb") as handle: + while True: + chunk = file_stream.read(chunk_size) + if not chunk: + break + handle.write(chunk) + return { + "size": int(path.stat().st_size), + "modified": self.modified_iso(path), + } + async def stream_file_range(self, path: Path, start: int, end: int, chunk_size: int = 1024 * 1024): with path.open("rb") as handle: handle.seek(start) 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 c9668da..a590fe0 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 03bf372..de185cb 100644 --- a/webui/backend/app/services/file_ops_service.py +++ b/webui/backend/app/services/file_ops_service.py @@ -3,7 +3,7 @@ from __future__ import annotations from pathlib import Path from backend.app.api.errors import AppError -from backend.app.api.schemas import DeleteResponse, FileInfoResponse, MkdirResponse, RenameResponse, SaveResponse, ViewResponse +from backend.app.api.schemas import DeleteResponse, FileInfoResponse, MkdirResponse, RenameResponse, SaveResponse, UploadResponse, ViewResponse from backend.app.db.history_repository import HistoryRepository from backend.app.fs.filesystem_adapter import FilesystemAdapter from backend.app.security.path_guard import PathGuard @@ -204,6 +204,61 @@ class FileOpsService: self._record_history_error(operation="delete", path=path, error=error) raise error + def upload(self, target_path: str, upload_file) -> UploadResponse: + destination_relative = None + history_path = target_path + try: + resolved_target = self._path_guard.resolve_directory_path(target_path) + filename = Path(upload_file.filename or "").name + safe_name = self._path_guard.validate_name(filename, field="name") + destination_relative = self._join_relative(resolved_target.relative, safe_name) + history_path = destination_relative + resolved_destination = self._path_guard.resolve_path(destination_relative) + + if resolved_destination.absolute.exists(): + raise AppError( + code="already_exists", + message="Target path already exists", + status_code=409, + details={"path": resolved_destination.relative}, + ) + + saved = self._filesystem.write_uploaded_file(resolved_destination.absolute, upload_file.file) + self._record_history( + operation="upload", + status="completed", + destination=resolved_destination.relative, + path=resolved_destination.relative, + finished_at=self._now_iso(), + ) + return UploadResponse( + path=resolved_destination.relative, + size=saved["size"], + modified=saved["modified"], + ) + except AppError as exc: + self._record_history_error( + operation="upload", + destination=destination_relative, + path=history_path, + error=exc, + ) + raise + except OSError as exc: + error = AppError( + code="io_error", + message="Filesystem operation failed", + status_code=500, + details={"reason": str(exc)}, + ) + self._record_history_error( + operation="upload", + destination=destination_relative, + path=history_path, + error=error, + ) + raise error + def view(self, path: str, for_edit: bool = False) -> ViewResponse: resolved_target = self._path_guard.resolve_existing_path(path) diff --git a/webui/backend/tests/golden/__pycache__/test_api_upload_golden.cpython-313.pyc b/webui/backend/tests/golden/__pycache__/test_api_upload_golden.cpython-313.pyc new file mode 100644 index 0000000..4bd7a18 Binary files /dev/null and b/webui/backend/tests/golden/__pycache__/test_api_upload_golden.cpython-313.pyc differ diff --git a/webui/backend/tests/golden/test_api_upload_golden.py b/webui/backend/tests/golden/test_api_upload_golden.py new file mode 100644 index 0000000..eaf817e --- /dev/null +++ b/webui/backend/tests/golden/test_api_upload_golden.py @@ -0,0 +1,186 @@ +from __future__ import annotations + +import asyncio +import sys +import tempfile +import unittest +from pathlib import Path + +import httpx + +sys.path.insert(0, str(Path(__file__).resolve().parents[3])) + +from backend.app.dependencies import get_file_ops_service, get_history_service +from backend.app.db.history_repository import HistoryRepository +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.file_ops_service import FileOpsService +from backend.app.services.history_service import HistoryService + + +class UploadApiGoldenTest(unittest.TestCase): + def setUp(self) -> None: + self.temp_dir = tempfile.TemporaryDirectory() + self.root = Path(self.temp_dir.name) / "root" + self.root.mkdir(parents=True, exist_ok=True) + self.uploads_dir = self.root / "uploads" + self.uploads_dir.mkdir(parents=True, exist_ok=True) + self.db_path = str(Path(self.temp_dir.name) / "history.db") + + history_repository = HistoryRepository(self.db_path) + file_ops_service = FileOpsService( + path_guard=PathGuard({"storage1": str(self.root)}), + filesystem=FilesystemAdapter(), + history_repository=history_repository, + ) + history_service = HistoryService(repository=history_repository) + + async def _override_file_ops_service() -> FileOpsService: + return file_ops_service + + async def _override_history_service() -> HistoryService: + return history_service + + app.dependency_overrides[get_file_ops_service] = _override_file_ops_service + app.dependency_overrides[get_history_service] = _override_history_service + + def tearDown(self) -> None: + app.dependency_overrides.clear() + self.temp_dir.cleanup() + + def _upload(self, *, target_path: str, filename: str, content: bytes) -> httpx.Response: + async def _run() -> httpx.Response: + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + return await client.post( + "/api/files/upload", + data={"target_path": target_path}, + files={"file": (filename, content, "application/octet-stream")}, + ) + + return asyncio.run(_run()) + + def _get_history(self) -> list[dict]: + async def _run() -> list[dict]: + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client: + response = await client.get("/api/history") + return response.json()["items"] + + return asyncio.run(_run()) + + def test_upload_single_file_success(self) -> None: + response = self._upload(target_path="storage1/uploads", filename="hello.txt", content=b"hello") + + self.assertEqual(response.status_code, 200) + body = response.json() + self.assertEqual(body["path"], "storage1/uploads/hello.txt") + self.assertEqual(body["size"], 5) + self.assertTrue((self.uploads_dir / "hello.txt").exists()) + + history = self._get_history() + self.assertEqual(history[0]["operation"], "upload") + self.assertEqual(history[0]["status"], "completed") + self.assertEqual(history[0]["destination"], "storage1/uploads/hello.txt") + + def test_upload_target_path_not_found(self) -> None: + response = self._upload(target_path="storage1/missing", filename="hello.txt", content=b"hello") + + self.assertEqual(response.status_code, 404) + self.assertEqual( + response.json(), + { + "error": { + "code": "path_not_found", + "message": "Requested path was not found", + "details": {"path": "storage1/missing"}, + } + }, + ) + + def test_upload_target_path_is_file(self) -> None: + target_file = self.root / "not_a_directory.txt" + target_file.write_text("x", encoding="utf-8") + + response = self._upload(target_path="storage1/not_a_directory.txt", filename="hello.txt", content=b"hello") + + self.assertEqual(response.status_code, 409) + self.assertEqual( + response.json(), + { + "error": { + "code": "path_type_conflict", + "message": "Requested path is not a directory", + "details": {"path": "storage1/not_a_directory.txt"}, + } + }, + ) + + def test_upload_traversal_blocked(self) -> None: + response = self._upload(target_path="storage1/../etc", filename="hello.txt", content=b"hello") + + self.assertEqual(response.status_code, 403) + self.assertEqual( + response.json(), + { + "error": { + "code": "path_traversal_detected", + "message": "Path traversal is not allowed", + "details": {"path": "storage1/../etc"}, + } + }, + ) + + def test_upload_invalid_root_alias(self) -> None: + response = self._upload(target_path="unknown/uploads", filename="hello.txt", content=b"hello") + + self.assertEqual(response.status_code, 403) + self.assertEqual( + response.json(), + { + "error": { + "code": "invalid_root_alias", + "message": "Unknown root alias", + "details": {"path": "unknown/uploads"}, + } + }, + ) + + def test_upload_invalid_filename_blocked(self) -> None: + response = self._upload(target_path="storage1/uploads", filename="..", content=b"hello") + + self.assertEqual(response.status_code, 400) + self.assertEqual( + response.json(), + { + "error": { + "code": "invalid_request", + "message": "Invalid name", + "details": {"name": ".."}, + } + }, + ) + + def test_upload_conflict_on_existing_file(self) -> None: + existing = self.uploads_dir / "hello.txt" + existing.write_text("existing", encoding="utf-8") + + response = self._upload(target_path="storage1/uploads", filename="hello.txt", content=b"hello") + + self.assertEqual(response.status_code, 409) + self.assertEqual( + response.json(), + { + "error": { + "code": "already_exists", + "message": "Target path already exists", + "details": {"path": "storage1/uploads/hello.txt"}, + } + }, + ) + + history = self._get_history() + self.assertEqual(history[0]["operation"], "upload") + self.assertEqual(history[0]["status"], "failed") + self.assertEqual(history[0]["error_code"], "already_exists")