feat: pdf viewer toegevoegd

2026-03-12 15:08:30 +01:00
parent ed34d6202f
commit fc22550e91
12 changed files with 382 additions and 1 deletions
@@ -0,0 +1,127 @@
+# PDF Viewer v1
+
+## 1. Doel
+PDF-viewing voegt nu directe waarde toe omdat PDF-bestanden in de huidige file manager al zichtbaar en selecteerbaar zijn, maar nog niet in de webui bekeken kunnen worden zonder externe tooling of downloadstap. Dat past logisch naast de bestaande text-viewer en video-viewer: elk veelvoorkomend bestandstype krijgt een passende read-only viewer, terwijl de dual-pane workflow intact blijft.
+
+## 2. Startgedrag
+Aanbevolen v1-gedrag:
+- `F3` opent de PDF viewer voor exact 1 geselecteerd PDF-bestand.
+- De bestaande `View`-knop hergebruikt dezelfde centrale view-dispatch en opent bij een PDF dezelfde PDF viewer.
+- Dit geldt alleen bij exact 1 geselecteerd item met extensie `.pdf`.
+- Bij geen selectie, multi-select, of niet-PDF blijft `F3` het bestaande view-gedrag respecteren of niets doen als `View` disabled zou zijn.
+- Gewone `Enter`-semantiek blijft intact en wordt niet gewijzigd voor PDF.
+
+## 3. Scope
+In scope voor v1:
+- alleen PDF
+- read-only
+- bekijken in modal
+- browser-native scrollen binnen de viewer
+- sluiten via `X` en `Escape`
+
+Niet in scope:
+- edit
+- annotatie
+- OCR
+- losse downloadfeature
+- page thumbnails
+- search in PDF-documenten
+- print-specifieke UI
+
+## 4. Viewer-richting
+Aanbevolen v1-richting: browser-native embedded PDF-view in een aparte modal.
+
+Concreet:
+- frontend opent een aparte PDF-modal
+- modal bevat een embedded viewer via `iframe` of `embed` naar een read-only backend endpoint
+- paginaweergave en scrollen worden aan de browser-PDF-viewer overgelaten
+- sluiten werkt via `X`, `Escape`, en eventueel overlay-click als dat consistent is met bestaande modals
+
+Waarom deze richting:
+- laag regressierisico
+- geen extra dependencies nodig
+- geen custom PDF rendering stack nodig
+- sluit aan op het bestaande modalmodel
+
+Niet aanbevolen voor v1:
+- eigen PDF-rendering met `pdf.js` of vergelijkbaar, tenzij later blijkt dat browser-native gedrag onvoldoende is
+- dat zou extra assets, integratie en regressierisico introduceren
+
+## 5. Backend-impact
+Aanbevolen: een apart read-only PDF endpoint, analoog aan video/view.
+
+Voorstel:
+- `GET /api/files/pdf?path=...`
+
+Eisen:
+- padvalidatie via bestaande `path_guard`
+- alleen files
+- directory -> bestaande `type_conflict`
+- path not found -> bestaande not-found fout
+- traversal / invalid root alias / outside whitelist -> bestaande securityfouten
+- `Content-Type: application/pdf`
+- streaming/read-only gedrag zonder onnodige buffering
+
+Waarom een apart endpoint:
+- expliciete content-type-afhandeling voor PDF
+- scheiding van concerns ten opzichte van tekst-view en video-streaming
+- eenvoudige frontend-integratie in een PDF-modal
+
+## 6. Frontend-impact
+Aanbevolen:
+- aparte PDF-modal, niet hergebruik van text-view modal
+- wel hergebruik van bestaande modalstijl en focusregels
+
+Gedrag:
+- `F3` en `View` dispatchen op bestandstype
+- tekstbestanden blijven naar text viewer gaan
+- video blijft naar video viewer gaan
+- PDF gaat naar PDF viewer
+- editorflow blijft ongemoeid
+
+Dat voorkomt regressie op bestaande viewers en houdt de semantiek per bestandstype helder.
+
+## 7. Risico's
+Belangrijkste risico's:
+- browserverschillen in ingebouwde PDF-weergave
+- sommige browsers of embedded contexten kunnen PDF anders tonen of beperkter ondersteunen
+- grote PDF-bestanden kunnen trager laden, maar browser-native streaming is nog steeds lichter dan custom rendering
+- security moet strikt read-only blijven via bestaande whitelist/path-guards
+- regressierisico in bestaande `F3`/`View` dispatchlogica als filetype-routing rommelig wordt geïmplementeerd
+
+Beperking voor v1:
+- geen garantie op identieke UX in elke browser
+- wel een veilige, kleine baseline voor moderne browsers met ingebouwde PDF-viewer
+
+## 8. Teststrategie
+Backend golden tests:
+- PDF endpoint success
+- directory -> `type_conflict`
+- path not found
+- traversal blocked
+- invalid root alias
+- non-pdf blocked of routed naar duidelijke fout
+
+UI smoke/regressietests:
+- PDF-modal container aanwezig
+- `F3` / `View` wiring aanwezig voor PDF-flow
+- bestaande text/video modal containers blijven aanwezig
+- geen regressie op huidige view-dispatch
+
+Handmatige validatie:
+- PDF openen via `F3`
+- PDF openen via `View`
+- sluiten via `X` en `Escape`
+- gewone `Enter` blijft ongewijzigd
+- text/video viewers blijven correct openen
+- groot PDF-bestand laadt zonder de UI te blokkeren
+
+## 9. Aanbeveling
+Aanbevolen v1-richting met laag regressierisico:
+- gebruik een apart read-only backend endpoint voor PDF
+- gebruik browser-native embedded PDF-weergave in een aparte modal
+- laat `F3` en `View` dezelfde type-dispatch gebruiken
+- houd PDF strikt read-only
+- voeg geen externe PDF-library of rendering stack toe in v1
+
+Dit levert de kleinste veilige stap op: bruikbare PDF-viewing, minimale architectuurimpact, en geen onnodige verbreding van scope.
@@ -66,6 +66,19 @@ async def video(
    )


+@router.get("/pdf")
+async def pdf(
+    path: str,
+    service: FileOpsService = Depends(get_file_ops_service),
+) -> StreamingResponse:
+    prepared = service.prepare_pdf_stream(path=path)
+    return StreamingResponse(
+        prepared["content"],
+        headers=prepared["headers"],
+        media_type=prepared["content_type"],
+    )
+
+
@router.get("/thumbnail")
 async def thumbnail(
    path: str,
@@ -35,6 +35,9 @@ VIDEO_CONTENT_TYPES = {
    ".mp4": "video/mp4",
    ".mkv": "video/x-matroska",
 }
+PDF_CONTENT_TYPES = {
+    ".pdf": "application/pdf",
+}


 class FileOpsService:
@@ -409,6 +412,39 @@ class FileOpsService:
            "content": self._filesystem.stream_file(resolved_target.absolute),
        }

+    def prepare_pdf_stream(self, path: str) -> dict:
+        resolved_target = self._path_guard.resolve_existing_path(path)
+
+        if resolved_target.absolute.is_dir():
+            raise AppError(
+                code="type_conflict",
+                message="Source must be a file",
+                status_code=409,
+                details={"path": resolved_target.relative},
+            )
+        if not resolved_target.absolute.is_file():
+            raise AppError(
+                code="type_conflict",
+                message="Unsupported path type for pdf",
+                status_code=409,
+                details={"path": resolved_target.relative},
+            )
+
+        content_type = self._pdf_content_type_for(resolved_target.absolute)
+        if content_type is None:
+            raise AppError(
+                code="unsupported_type",
+                message="File type is not supported for pdf viewing",
+                status_code=409,
+                details={"path": resolved_target.relative},
+            )
+
+        return {
+            "headers": {"Content-Length": str(int(resolved_target.absolute.stat().st_size))},
+            "content_type": content_type,
+            "content": self._filesystem.stream_file(resolved_target.absolute),
+        }
+
    @staticmethod
    def _join_relative(base: str, name: str) -> str:
        return f"{base}/{name}" if base else name
@@ -428,6 +464,10 @@ class FileOpsService:
    def _thumbnail_content_type_for(path: Path) -> str | None:
        return THUMBNAIL_CONTENT_TYPES.get(path.suffix.lower())

+    @staticmethod
+    def _pdf_content_type_for(path: Path) -> str | None:
+        return PDF_CONTENT_TYPES.get(path.suffix.lower())
+
    def _record_history(
        self,
        *,
@@ -0,0 +1,92 @@
+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
+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
+
+
+class PdfApiGoldenTest(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)
+        path_guard = PathGuard({"storage1": str(self.root)})
+        service = FileOpsService(path_guard=path_guard, filesystem=FilesystemAdapter())
+
+        async def _override_file_ops_service() -> FileOpsService:
+            return service
+
+        app.dependency_overrides[get_file_ops_service] = _override_file_ops_service
+
+    def tearDown(self) -> None:
+        app.dependency_overrides.clear()
+        self.temp_dir.cleanup()
+
+    def _request(self, path: str) -> 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.get("/api/files/pdf", params={"path": path})
+
+        return asyncio.run(_run())
+
+    def test_pdf_endpoint_success(self) -> None:
+        payload = b"%PDF-1.7\n1 0 obj\n<<>>\nendobj\n"
+        (self.root / "sample.pdf").write_bytes(payload)
+
+        response = self._request("storage1/sample.pdf")
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.headers["content-type"], "application/pdf")
+        self.assertEqual(response.headers["content-length"], str(len(payload)))
+        self.assertEqual(response.content, payload)
+
+    def test_pdf_directory_type_conflict(self) -> None:
+        (self.root / "docs").mkdir()
+
+        response = self._request("storage1/docs")
+
+        self.assertEqual(response.status_code, 409)
+        self.assertEqual(response.json()["error"]["code"], "type_conflict")
+
+    def test_pdf_path_not_found(self) -> None:
+        response = self._request("storage1/missing.pdf")
+
+        self.assertEqual(response.status_code, 404)
+        self.assertEqual(response.json()["error"]["code"], "path_not_found")
+
+    def test_pdf_traversal_blocked(self) -> None:
+        response = self._request("storage1/../etc/passwd")
+
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.json()["error"]["code"], "path_traversal_detected")
+
+    def test_pdf_invalid_root_alias(self) -> None:
+        response = self._request("unknown/sample.pdf")
+
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.json()["error"]["code"], "invalid_root_alias")
+
+    def test_pdf_non_pdf_blocked(self) -> None:
+        (self.root / "notes.txt").write_text("hello", encoding="utf-8")
+
+        response = self._request("storage1/notes.txt")
+
+        self.assertEqual(response.status_code, 409)
+        self.assertEqual(response.json()["error"]["code"], "unsupported_type")
+
+
+if __name__ == "__main__":
+    unittest.main()
@@ -51,6 +51,9 @@ class UiSmokeGoldenTest(unittest.TestCase):
        self.assertIn("F8", body)
        self.assertIn('id="viewer-modal"', body)
        self.assertIn('id="video-modal"', body)
+        self.assertIn('id="pdf-modal"', body)
+        self.assertIn('id="pdf-frame"', body)
+        self.assertIn('id="pdf-close-btn"', body)
        self.assertIn('id="video-player"', body)
        self.assertIn('id="video-close-btn"', body)
        self.assertIn('id="settings-modal"', body)
@@ -151,6 +154,10 @@ class UiSmokeGoldenTest(unittest.TestCase):
        self.assertIn('document.getElementById("rename-btn").onclick = openRenamePopup;', app_js)
        self.assertIn('return triggerActionButton("rename-btn");', app_js)
        self.assertIn('function openVideoViewer()', app_js)
+        self.assertIn('function openPdfViewer()', app_js)
+        self.assertIn('document.getElementById("pdf-modal")', app_js)
+        self.assertIn("`/api/files/pdf?", app_js)
+        self.assertIn('if (isPdfSelection(selected)) {', app_js)
        self.assertIn('video.player.src = streamUrl;', app_js)
        self.assertIn('document.getElementById("video-close-btn")', app_js)
        self.assertIn('row.ondblclick = (ev) => {', app_js)
@@ -162,6 +162,18 @@ function videoElements() {
  };
 }

+function pdfElements() {
+  return {
+    overlay: document.getElementById("pdf-modal"),
+    title: document.getElementById("pdf-title"),
+    fileName: document.getElementById("pdf-file-name"),
+    filePath: document.getElementById("pdf-file-path"),
+    error: document.getElementById("pdf-error"),
+    frame: document.getElementById("pdf-frame"),
+    closeButton: document.getElementById("pdf-close-btn"),
+  };
+}
+
 function moveElements() {
  return {
    overlay: document.getElementById("move-popup"),
@@ -468,6 +480,13 @@ function isVideoSelection(item) {
  return lower.endsWith(".mp4") || lower.endsWith(".mkv");
 }

+function isPdfSelection(item) {
+  if (!item || item.kind !== "file") {
+    return false;
+  }
+  return (item.name || "").toLowerCase().endsWith(".pdf");
+}
+
 function currentParentPath(path) {
  const normalized = (path || "").trim();
  if (!normalized) {
@@ -774,6 +793,15 @@ function renderPaneItems(pane) {
        renderPaneItems(pane);
        openVideoViewer();
      };
+    } else if (entry.kind === "file" && isPdfSelection({ path: entry.path, name: entry.name, kind: entry.kind })) {
+      row.ondblclick = (ev) => {
+        ev.stopPropagation();
+        setActivePane(pane);
+        model.currentRowIndex = index;
+        setSingleSelectionAtIndex(pane, { path: entry.path, name: entry.name, kind: entry.kind }, index);
+        renderPaneItems(pane);
+        openPdfViewer();
+      };
    }
    items.append(row);
  });
@@ -1480,6 +1508,17 @@ function closeVideoViewer() {
  video.player.load();
 }

+function isPdfOpen() {
+  return !pdfElements().overlay.classList.contains("hidden");
+}
+
+function closePdfViewer() {
+  const pdf = pdfElements();
+  pdf.overlay.classList.add("hidden");
+  pdf.error.textContent = "";
+  pdf.frame.removeAttribute("src");
+}
+
 function isInfoOpen() {
  return !infoElements().overlay.classList.contains("hidden");
 }
@@ -1760,7 +1799,7 @@ function closeEditor() {
  resetEditorState();
 }

-async function openViewer() {
+async function openTextViewer() {
  const selectedItems = activePaneState().selectedItems;
  if (selectedItems.length !== 1 || selectedItems[0].kind !== "file") {
    return;
@@ -1787,6 +1826,22 @@ async function openViewer() {
  }
 }

+async function openPdfViewer() {
+  const selectedItems = activePaneState().selectedItems;
+  if (selectedItems.length !== 1 || !isPdfSelection(selectedItems[0])) {
+    return;
+  }
+  const selected = selectedItems[0];
+  const pdf = pdfElements();
+  const pdfUrl = `/api/files/pdf?${new URLSearchParams({ path: selected.path }).toString()}`;
+  pdf.overlay.classList.remove("hidden");
+  pdf.title.textContent = "PDF";
+  pdf.fileName.textContent = selected.name;
+  pdf.filePath.textContent = selected.path;
+  pdf.error.textContent = "";
+  pdf.frame.src = pdfUrl;
+}
+
 function videoPlaybackMessage(item) {
  const lower = (item.name || "").toLowerCase();
  if (lower.endsWith(".mkv")) {
@@ -1845,6 +1900,23 @@ async function openEditor() {
  }
 }

+function openViewer() {
+  const selectedItems = activePaneState().selectedItems;
+  if (selectedItems.length !== 1 || selectedItems[0].kind !== "file") {
+    return;
+  }
+  const selected = selectedItems[0];
+  if (isVideoSelection(selected)) {
+    openVideoViewer();
+    return;
+  }
+  if (isPdfSelection(selected)) {
+    openPdfViewer();
+    return;
+  }
+  openTextViewer();
+}
+
 async function saveEditor() {
  if (!editorState.path) {
    return;
@@ -2031,6 +2103,13 @@ function handleKeyboardShortcuts(event) {
    }
    return;
  }
+  if (isPdfOpen()) {
+    if (event.key === "Escape") {
+      event.preventDefault();
+      closePdfViewer();
+    }
+    return;
+  }
  if (isViewerOpen()) {
    if (event.key === "Escape") {
      event.preventDefault();
@@ -2210,6 +2289,9 @@ function setupEvents() {
    }
  };

+  const pdf = pdfElements();
+  pdf.closeButton.onclick = closePdfViewer;
+
  const wildcard = wildcardPopupElements();
  wildcard.cancelButton.onclick = closeWildcardPopup;
  wildcard.applyButton.onclick = submitWildcardPopup;
@@ -214,6 +214,17 @@
    </div>
  </div>

+  <div id="pdf-modal" class="popup-overlay hidden" role="dialog" aria-modal="true" aria-labelledby="pdf-title">
+    <div class="popup-card viewer-card">
+      <button id="pdf-close-btn" class="viewer-close" type="button" aria-label="Close pdf">X</button>
+      <h3 id="pdf-title">PDF</h3>
+      <div id="pdf-file-name" class="popup-meta"></div>
+      <div id="pdf-file-path" class="popup-meta"></div>
+      <div id="pdf-error" class="error"></div>
+      <iframe id="pdf-frame" class="pdf-frame" title="PDF viewer"></iframe>
+    </div>
+  </div>
+
  <div id="editor-modal" class="popup-overlay hidden" role="dialog" aria-modal="true" aria-labelledby="editor-title">
    <div class="popup-card viewer-card">
      <button id="editor-close-btn" class="viewer-close" type="button" aria-label="Close editor">X</button>
@@ -632,6 +632,15 @@ button:disabled {
  border: 1px solid var(--color-border);
 }

+.pdf-frame {
+  width: 100%;
+  height: calc(100vh - 180px);
+  min-height: 420px;
+  background: #fff;
+  border-radius: var(--radius-sm);
+  border: 1px solid var(--color-border);
+}
+
 .search-card {
  width: min(680px, calc(100vw - 32px));
 }