docs(05-09): complete cloud document access fixes plan — PATCH endpoint, cloud-aware re-analyze, authenticated preview
This commit is contained in:
curo1305
@@ -0,0 +1,119 @@
|
||||
---
|
||||
phase: 05-cloud-storage-backends
|
||||
plan: "09"
|
||||
subsystem: cloud-documents
|
||||
tags: [cloud, documents, patch, celery, frontend, blob-url, authentication]
|
||||
dependency_graph:
|
||||
requires: [05-06]
|
||||
provides: [PATCH /api/documents/{id}, cloud-aware re-analyze, authenticated-preview]
|
||||
affects: [backend/api/documents.py, backend/tasks/document_tasks.py, frontend/src/api/client.js, frontend/src/components/documents/DocumentPreviewModal.vue, frontend/src/views/DocumentView.vue]
|
||||
tech_stack:
|
||||
added: []
|
||||
patterns: [fetch-blob-url, model_fields_set, cloud-aware-task-routing, tdd-red-green]
|
||||
key_files:
|
||||
created: []
|
||||
modified:
|
||||
- backend/api/documents.py
|
||||
- backend/tasks/document_tasks.py
|
||||
- backend/tests/test_cloud.py
|
||||
- frontend/src/api/client.js
|
||||
- frontend/src/components/documents/DocumentPreviewModal.vue
|
||||
- frontend/src/views/DocumentView.vue
|
||||
decisions:
|
||||
- "Test 3 patches storage module (not tasks module): deferred import pattern means get_storage_backend_for_document is not a module-level attribute of document_tasks — patching at storage module level is correct"
|
||||
- "Test 3 is a pure unit test (no db_session): _run() opens its own AsyncSessionLocal which requires PostgreSQL; mocking the session manager keeps the test fast and infrastructure-free"
|
||||
- "node_modules symlinked into worktree frontend for build verification: worktree does not have its own node_modules; symlink to main repo preserves isolation while enabling build check"
|
||||
metrics:
|
||||
duration: "~25 minutes"
|
||||
completed: "2026-05-30"
|
||||
tasks_completed: 2
|
||||
files_modified: 6
|
||||
---
|
||||
|
||||
# Phase 5 Plan 09: Cloud Document Access Fixes Summary
|
||||
|
||||
Fixed three independent root causes that blocked cloud document use: unauthenticated iframe preview, hardcoded MinIO in Celery re-analyze, and missing PATCH endpoint for document metadata.
|
||||
|
||||
## Tasks Completed
|
||||
|
||||
| Task | Name | Commit | Files |
|
||||
|------|------|--------|-------|
|
||||
| 1 | PATCH /documents/{id} + cloud-aware Celery re-analyze | 6d094d1 | backend/api/documents.py, backend/tasks/document_tasks.py, backend/tests/test_cloud.py |
|
||||
| TDD RED | Failing tests for Task 1 | 9bc0561 | backend/tests/test_cloud.py |
|
||||
| 2 | Authenticated document preview — fetch + Blob URL | 4a42cce | frontend/src/api/client.js, frontend/src/components/documents/DocumentPreviewModal.vue, frontend/src/views/DocumentView.vue |
|
||||
|
||||
## What Was Built
|
||||
|
||||
**Task 1: Backend fixes**
|
||||
|
||||
- `DocumentPatch` Pydantic model with `Optional[str] filename` and `Optional[uuid.UUID] folder_id`; uses `model_fields_set` to distinguish "not provided" from "explicitly set to null"
|
||||
- `PATCH /api/documents/{doc_id}` endpoint with ownership guard (non-owner → 404), admin guard (get_regular_user → 403), empty body guard (422), and `storage.get_metadata()` whitelist response
|
||||
- `_run()` in `document_tasks.py` updated: MinIO path unchanged; non-MinIO path calls `get_storage_backend_for_document(doc, user, session)`; missing user returns `missing_user` status; `CloudConnectionError` returns `extract_failed` with generic "cloud backend error" message (no provider details)
|
||||
|
||||
**Task 2: Frontend fixes**
|
||||
|
||||
- `fetchDocumentContent(docId)` in `client.js`: authenticated fetch returning raw `Response` (not parsed JSON); single 401 → refresh → retry pattern; mirrors `request()` auth logic without `res.json()`
|
||||
- `DocumentPreviewModal.vue`: replaced unauthenticated `iframe :src="proxyUrl"` with authenticated fetch → blob → `URL.createObjectURL()`; loading spinner, error state, `URL.revokeObjectURL` on unmount
|
||||
- `DocumentView.vue` `openPdf()`: replaced `window.open(rawUrl)` with fetch → blob → objectUrl → `window.open(objectUrl)`; 60-second auto-revoke
|
||||
|
||||
## Test Results
|
||||
|
||||
- 3 new tests in `test_cloud.py`: all pass
|
||||
- `test_patch_document_filename` — PATCH 200 with updated filename
|
||||
- `test_patch_document_wrong_owner` — PATCH 404 for non-owner (IDOR guard)
|
||||
- `test_reanalyze_cloud_document_routes_to_cloud_backend` — cloud backend called, MinIO not called
|
||||
- Full `test_cloud.py` suite: **23 passed, 0 failed** (no regressions)
|
||||
- Frontend build: **zero errors** (`vite build` exits 0)
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
### Auto-adjusted Issues
|
||||
|
||||
**1. [Rule 1 - Bug] Test patching at storage module level, not tasks module level**
|
||||
- **Found during:** Task 1 GREEN phase
|
||||
- **Issue:** Plan specified patching `tasks.document_tasks.get_storage_backend_for_document`, but that attribute does not exist at module level — the import is inside `_run()` (deferred import pattern). `unittest.mock.patch` raises `AttributeError` on absent attributes.
|
||||
- **Fix:** Test patches `storage.get_storage_backend_for_document` and `storage.get_storage_backend` — the canonical source of both functions. Behavior under test is identical.
|
||||
- **Files modified:** `backend/tests/test_cloud.py`
|
||||
|
||||
**2. [Rule 1 - Bug] Test 3 changed to pure unit test (no db_session fixture)**
|
||||
- **Found during:** Task 1 GREEN phase — second attempt
|
||||
- **Issue:** `_run()` opens its own `AsyncSessionLocal()` internally which requires a live PostgreSQL connection. Using `db_session` fixture in the test doesn't affect `_run()`'s internal session. Tests run without Docker → connection refused.
|
||||
- **Fix:** Test mocks `db.session.AsyncSessionLocal` with a fake async context manager that returns a mock session with `session.get()` returning pre-built `MagicMock` Document and User objects. Removed `db_session` from test signature.
|
||||
- **Files modified:** `backend/tests/test_cloud.py`
|
||||
|
||||
**3. [Rule 3 - Blocking] node_modules symlink needed for worktree build**
|
||||
- **Found during:** Task 2 verification
|
||||
- **Issue:** Worktree's `frontend/` has no `node_modules` (npm install runs in main repo only). `vite build` failed with `ERR_MODULE_NOT_FOUND`.
|
||||
- **Fix:** Created a symlink `worktree/frontend/node_modules → main/frontend/node_modules`. Build succeeded.
|
||||
- **Files modified:** `frontend/node_modules` (symlink, not tracked in git)
|
||||
|
||||
## Security Analysis (T-05-09 Threat Register)
|
||||
|
||||
| Threat ID | Status | Notes |
|
||||
|-----------|--------|-------|
|
||||
| T-05-09-01 | Mitigated | `get_regular_user` enforced on PATCH; admin → 403; wrong owner → 404 |
|
||||
| T-05-09-02 | Mitigated | `storage.get_metadata()` response whitelist via `_doc_to_dict()` — no `credentials_enc` |
|
||||
| T-05-09-03 | Mitigated | Credentials loaded inside Celery task's own DB session via `get_storage_backend_for_document` |
|
||||
| T-05-09-04 | Accepted | Blob URL same-origin, revoked on unmount — no persistent exposure |
|
||||
| T-05-09-SC | N/A | No new packages installed |
|
||||
|
||||
## Known Stubs
|
||||
|
||||
None — all plan features fully implemented and wired.
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
All created/modified files confirmed present on disk. All task commits verified in git log.
|
||||
|
||||
| Item | Status |
|
||||
|------|--------|
|
||||
| backend/api/documents.py | FOUND |
|
||||
| backend/tasks/document_tasks.py | FOUND |
|
||||
| backend/tests/test_cloud.py | FOUND |
|
||||
| frontend/src/api/client.js | FOUND |
|
||||
| frontend/src/components/documents/DocumentPreviewModal.vue | FOUND |
|
||||
| frontend/src/views/DocumentView.vue | FOUND |
|
||||
| .planning/phases/05-cloud-storage-backends/05-09-SUMMARY.md | FOUND |
|
||||
| Commit 9bc0561 (RED tests) | FOUND |
|
||||
| Commit 6d094d1 (GREEN implementation) | FOUND |
|
||||
| Commit 4a42cce (frontend auth preview) | FOUND |
|
||||
Reference in New Issue
Block a user