From ac9e688e7a0018757bb0431fa132e85d168b4dc6 Mon Sep 17 00:00:00 2001 From: Nils Schneider Date: Tue, 11 Nov 2025 02:19:39 +0100 Subject: [PATCH] /api/download --- DEVELOPMENT.md | 2 +- backend/src/documents/asset.rs | 2 +- backend/src/routes/documents.rs | 2 +- backend/src/routes/mod.rs | 2 +- backend/src/services/documents.rs | 38 +++++++++----- backend/src/workers/thumbnails.rs | 4 +- backend/tests/documents_flow.rs | 52 +++++++++++++++++-- backend/tests/download_flow.rs | 2 +- docs/api.txt | 4 +- .../hooks/documents/useDocumentMutations.js | 37 +++++++++++-- 10 files changed, 116 insertions(+), 29 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 78fd853..874afbe 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -67,7 +67,7 @@ The backend reads its settings from environment variables. In particular: - `DATABASE_MAX_POOL_SIZE` – optional override for the r2d2 connection pool size. Defaults to `2`; increase it in staging/production to match expected concurrency. - `PROXY_DOWNLOADS` – set to `true` when the object store is only reachable from - the backend network. When enabled, `/download/{token}` and asset-object fetches + the backend network. When enabled, `/api/download/{token}` and asset-object fetches stream bytes through the API instead of redirecting clients to S3/Hetzner. On startup each binary logs the effective configuration with secrets redacted diff --git a/backend/src/documents/asset.rs b/backend/src/documents/asset.rs index b5d2541..ab4087f 100644 --- a/backend/src/documents/asset.rs +++ b/backend/src/documents/asset.rs @@ -83,7 +83,7 @@ pub fn build_download_path( state .jwt .generate_download_token(document.id, version_id, user_id, document.tenant_id) - .map(|token| format!("/download/{token}")) + .map(|token| format!("/api/download/{token}")) .map_err(|err| { tracing::error!(error = ?err, "failed to generate download token"); AppError::internal("failed to generate download token") diff --git a/backend/src/routes/documents.rs b/backend/src/routes/documents.rs index cdd26da..237d10b 100644 --- a/backend/src/routes/documents.rs +++ b/backend/src/routes/documents.rs @@ -558,7 +558,7 @@ pub async fn get_document_version( #[utoipa::path( get, - path = "/download/{token}", + path = "/api/download/{token}", params(("token" = String, Path, description = "Download token")), responses((status = 200, description = "Proxied download stream or redirect")), tag = "Documents" diff --git a/backend/src/routes/mod.rs b/backend/src/routes/mod.rs index efad73e..3eac18e 100644 --- a/backend/src/routes/mod.rs +++ b/backend/src/routes/mod.rs @@ -210,7 +210,7 @@ pub fn create_router(state: AppState) -> Router<()> { ); let download_routes = - Router::new().route("/download/{token}", get(documents::download_with_token)); + Router::new().route("/api/download/{token}", get(documents::download_with_token)); let folders_routes = Router::new() .route( diff --git a/backend/src/services/documents.rs b/backend/src/services/documents.rs index 379b0b2..8b704ed 100644 --- a/backend/src/services/documents.rs +++ b/backend/src/services/documents.rs @@ -964,7 +964,7 @@ impl<'a> DocumentsService<'a> { error!(error = ?err, "failed to issue asset download token"); AppError::internal("failed to issue asset download token") })?; - (format!("/download/{token}"), None) + (format!("/api/download/{token}"), None) } else { let storage = storage .as_ref() @@ -1044,19 +1044,33 @@ impl<'a> DocumentsService<'a> { tenant_id: Uuid, document_id: Uuid, ) -> AppResult<()> { - let now = Utc::now().naive_utc(); - diesel::update( - documents::table + conn.transaction::<_, AppError, _>(|conn| { + let document: Document = documents::table .find(document_id) - .filter(documents::tenant_id.eq(tenant_id)), - ) - .set(( - documents::deleted_at.eq(Some(now)), - documents::updated_at.eq(now), - )) - .execute(conn)?; + .filter(documents::tenant_id.eq(tenant_id)) + .for_update() + .first(conn) + .optional()? + .ok_or_else(AppError::not_found)?; - Ok(()) + if document.deleted_at.is_some() { + return Err(AppError::conflict("document already trashed")); + } + + let now = Utc::now().naive_utc(); + diesel::update( + documents::table + .find(document_id) + .filter(documents::tenant_id.eq(tenant_id)), + ) + .set(( + documents::deleted_at.eq(Some(now)), + documents::updated_at.eq(now), + )) + .execute(conn)?; + + Ok(()) + }) } pub fn delete_document( diff --git a/backend/src/workers/thumbnails.rs b/backend/src/workers/thumbnails.rs index d5f5714..4e14a6e 100644 --- a/backend/src/workers/thumbnails.rs +++ b/backend/src/workers/thumbnails.rs @@ -24,9 +24,7 @@ use crate::{ use super::{ analyze::determine_thumbnail_support, - taskflow::{ - document::DocumentVersionTaskContext, Task, TaskContext, TaskError, TaskResult, - }, + taskflow::{document::DocumentVersionTaskContext, Task, TaskContext, TaskError, TaskResult}, }; pub const THUMBNAIL_WIDTH: u32 = 512; diff --git a/backend/tests/documents_flow.rs b/backend/tests/documents_flow.rs index 1fb78cd..6930270 100644 --- a/backend/tests/documents_flow.rs +++ b/backend/tests/documents_flow.rs @@ -199,7 +199,7 @@ async fn upload_and_list_document() -> Result<()> { .current_version .as_ref() .expect("current version detail"); - assert!(current_version.download_path.starts_with("/download/")); + assert!(current_version.download_path.starts_with("/api/download/")); assert_eq!(current_version.version_number, 1); assert_eq!(current_version.size_bytes, file_bytes.len() as i64); assert!(current_version.assets.is_empty()); @@ -231,7 +231,7 @@ async fn upload_and_list_document() -> Result<()> { .as_ref() .expect("list current version") .download_path - .starts_with("/download/")); + .starts_with("/api/download/")); let redirect = app.get(¤t_version.download_path, None).await?; assert_eq!(redirect.status(), StatusCode::TEMPORARY_REDIRECT); @@ -367,7 +367,7 @@ async fn asset_detail_uses_proxy_urls_when_configured() -> Result<()> { .url .as_deref() .ok_or_else(|| anyhow!("missing url"))?; - assert!(url.starts_with("/download/")); + assert!(url.starts_with("/api/download/")); assert!(object.expires_at.is_none()); app.cleanup().await?; @@ -1717,6 +1717,50 @@ async fn list_documents_by_status_filter() -> Result<()> { app.cleanup().await?; Ok(()) } + +#[tokio::test] +async fn trash_document_requires_active_state() -> Result<()> { + let _lock = acquire_db_lock().await; + let app = TestApp::new().await?; + + let password = "trashstate"; + app.insert_user("trashstate", TestUserRole::Owner).await?; + let token = app.login_token("trashstate", password).await?; + + let upload = app + .upload_document( + "/api/documents", + "trash-once.txt", + "text/plain", + b"trash", + None, + &token, + ) + .await?; + let body = body_to_vec(upload.into_body()).await?; + let detail: DocumentDetail = serde_json::from_slice(&body)?; + + let first = app + .post_json( + &format!("/api/documents/{}/trash", detail.document.id), + &json!({}), + Some(&token), + ) + .await?; + assert_eq!(first.status(), StatusCode::NO_CONTENT); + + let second = app + .post_json( + &format!("/api/documents/{}/trash", detail.document.id), + &json!({}), + Some(&token), + ) + .await?; + assert_eq!(second.status(), StatusCode::CONFLICT); + + app.cleanup().await?; + Ok(()) +} #[tokio::test] async fn purge_document_removes_data() -> Result<()> { let _lock = acquire_db_lock().await; @@ -2008,7 +2052,7 @@ async fn list_document_versions_and_fetch_detail() -> Result<()> { let detail_body = body_to_vec(detail_resp.into_body()).await?; let version_detail: DocumentVersionPayload = serde_json::from_slice(&detail_body)?; assert_eq!(version_detail.id, version_id); - assert!(version_detail.download_path.starts_with("/download/")); + assert!(version_detail.download_path.starts_with("/api/download/")); assert!(version_detail.assets.is_empty()); app.cleanup().await?; diff --git a/backend/tests/download_flow.rs b/backend/tests/download_flow.rs index e60a0c0..56df5fc 100644 --- a/backend/tests/download_flow.rs +++ b/backend/tests/download_flow.rs @@ -68,7 +68,7 @@ async fn download_with_invalid_token_is_rejected() -> Result<()> { let _lock = acquire_db_lock().await; let app = TestApp::new().await?; - let response = app.get("/download/not-a-token", None).await?; + let response = app.get("/api/download/not-a-token", None).await?; assert_eq!(response.status(), StatusCode::UNAUTHORIZED); app.cleanup().await?; diff --git a/docs/api.txt b/docs/api.txt index 94ca71b..7553f32 100644 --- a/docs/api.txt +++ b/docs/api.txt @@ -44,7 +44,7 @@ Document Assets Downloads --------- -- GET /download/:token - Follow a one-time download token; redirects to a pre-signed URL (public token required). +- GET /api/download/:token - Follow a one-time download token; redirects to a pre-signed URL (public token required). Folders ------- @@ -68,4 +68,4 @@ Correspondents - GET /api/correspondents - List correspondents with usage totals. - POST /api/correspondents - Create a correspondent (name + optional metadata JSON). - PATCH /api/correspondents/:id - Update name and/or metadata. -- DELETE /api/correspondents/:id - Remove a correspondent; fails with 400 if referenced by any document. \ No newline at end of file +- DELETE /api/correspondents/:id - Remove a correspondent; fails with 400 if referenced by any document. diff --git a/frontend/src/hooks/documents/useDocumentMutations.js b/frontend/src/hooks/documents/useDocumentMutations.js index 9725cd1..fa1527e 100644 --- a/frontend/src/hooks/documents/useDocumentMutations.js +++ b/frontend/src/hooks/documents/useDocumentMutations.js @@ -274,9 +274,39 @@ const useDocumentMutations = ({ } try { - await Promise.all( - documentIds.map((documentId) => api.post(`/documents/${documentId}/trash`)), - ); + const softDeleteTargets = []; + const hardDeleteTargets = []; + + documentIds.forEach((documentId) => { + const lookupDoc = + documentLookup && typeof documentLookup.get === 'function' + ? documentLookup.get(documentId) + : documentLookup?.[documentId]; + + if (lookupDoc && lookupDoc.deleted_at) { + hardDeleteTargets.push(documentId); + } else { + softDeleteTargets.push(documentId); + } + }); + + const operations = []; + if (softDeleteTargets.length) { + operations.push( + Promise.all( + softDeleteTargets.map((documentId) => api.post(`/documents/${documentId}/trash`)), + ), + ); + } + if (hardDeleteTargets.length) { + operations.push( + Promise.all( + hardDeleteTargets.map((documentId) => api.delete(`/documents/${documentId}`)), + ), + ); + } + + await Promise.all(operations); removeDocumentsFromCaches(documentIds); @@ -302,6 +332,7 @@ const useDocumentMutations = ({ [ api, token, + documentLookup, removeDocumentsFromCaches, previewDocumentId, closeDocumentPreview,