From 50125f465957800f772cb6993598bb17ac97d070 Mon Sep 17 00:00:00 2001 From: Nils Schneider Date: Mon, 27 Oct 2025 22:06:33 +0100 Subject: [PATCH] document updates --- backend/src/openapi.rs | 12 + backend/src/routes/documents.rs | 210 ++++++++++++++--- backend/tests/documents_flow.rs | 403 ++++++++++++++++++++++++++++++++ 3 files changed, 588 insertions(+), 37 deletions(-) diff --git a/backend/src/openapi.rs b/backend/src/openapi.rs index ff9d2f3..f71434a 100644 --- a/backend/src/openapi.rs +++ b/backend/src/openapi.rs @@ -687,10 +687,22 @@ pub mod schemas { pub size_bytes: i64, } + #[derive(Serialize, Deserialize, ToSchema)] + pub struct DocumentMetadataUpdate { + pub value: Value, + #[serde(default)] + #[schema(default = false)] + pub replace: bool, + } + #[derive(Serialize, Deserialize, ToSchema)] pub struct UpdateDocumentRequest { #[schema(nullable)] pub title: Option, + #[schema(nullable, value_type = Option)] + pub issued_at: Option, + #[schema(nullable)] + pub metadata: Option, } #[derive(Serialize, Deserialize, ToSchema)] diff --git a/backend/src/routes/documents.rs b/backend/src/routes/documents.rs index e679401..82ef4d8 100644 --- a/backend/src/routes/documents.rs +++ b/backend/src/routes/documents.rs @@ -11,7 +11,7 @@ use diesel::dsl::exists; use diesel::{prelude::*, result::DatabaseErrorKind, select, PgConnection}; use reqwest::Client; use serde::{Deserialize, Serialize}; -use serde_json::{json, Value}; +use serde_json::{json, map::Entry, Map, Value}; use sha2::{Digest, Sha256}; use tracing::{debug, error, info, warn}; use utoipa::{IntoParams, ToSchema}; @@ -34,6 +34,7 @@ use crate::state::AppState; use crate::utils::{ db::{no_content, validate_bulk_ids, IntoJsonResponse}, http::inline_content_disposition, + json::{classify_nullable, NullableValue}, storage_paths::document_version_object_key, time::to_iso, validation::ensure_exists, @@ -225,9 +226,24 @@ pub struct BulkMoveRequest { pub folder_id: Option, } +#[derive(Deserialize, ToSchema)] +pub struct DocumentMetadataUpdate { + pub value: Value, + #[serde(default)] + #[schema(default = false)] + pub replace: bool, +} + #[derive(Deserialize, ToSchema)] pub struct UpdateDocumentRequest { + #[serde(default)] pub title: Option, + #[serde(default)] + #[schema(nullable, value_type = Option)] + pub issued_at: Option, + #[serde(default)] + #[schema(nullable)] + pub metadata: Option, } #[derive(Serialize, ToSchema)] @@ -274,6 +290,16 @@ pub struct AssignCorrespondentsRequest { pub replace: bool, } +#[derive(Default, AsChangeset)] +#[diesel(table_name = documents)] +struct DocumentUpdateChangeset { + title: Option, + filename: Option, + issued_at: Option>, + metadata: Option, + updated_at: Option, +} + #[derive(Deserialize, Copy, Clone, PartialEq, Eq, ToSchema)] #[serde(rename_all = "lowercase")] pub enum BulkCorrespondentAction { @@ -1220,7 +1246,7 @@ pub async fn update_document( user_id, .. }: TenantScopedConn, - Json(payload): Json, + Json(payload): Json, ) -> AppResult> { let mut document: Document = documents::table .find(document_id) @@ -1230,52 +1256,113 @@ pub async fn update_document( return Err(AppError::not_found()); } - let new_title = match payload.title { - Some(ref title) => { - let trimmed = title.trim(); - if trimmed.is_empty() { - return Err(AppError::bad_request("title must not be empty")); - } - Some(trimmed.to_string()) - } - None => None, + let payload_obj = payload + .as_object() + .ok_or_else(|| AppError::bad_request("request body must be a JSON object"))?; + + let title = match payload_obj.get("title") { + None | Some(Value::Null) => None, + Some(Value::String(value)) => Some(value.clone()), + Some(_) => return Err(AppError::bad_request("title must be a string")), }; - if new_title.is_none() { + let issued_at_class = + classify_nullable(payload_obj.get("issued_at")).map_err(AppError::bad_request)?; + + let metadata = match payload_obj.get("metadata") { + None | Some(Value::Null) => None, + Some(value) => Some( + serde_json::from_value::(value.clone()) + .map_err(|err| AppError::bad_request(format!("invalid metadata payload: {err}")))?, + ), + }; + + let mut changes = DocumentUpdateChangeset::default(); + let mut has_changes = false; + + if let Some(ref candidate) = title { + let trimmed = candidate.trim(); + if trimmed.is_empty() { + return Err(AppError::bad_request("title must not be empty")); + } + if trimmed != document.title { + let new_title = trimmed.to_string(); + let new_filename = filename_with_retained_extension(&new_title, &document.filename); + changes.title = Some(new_title); + if new_filename != document.filename { + changes.filename = Some(new_filename); + } + has_changes = true; + } + } + + match issued_at_class { + NullableValue::Omitted => {} + NullableValue::Null => { + if document.issued_at.is_some() { + changes.issued_at = Some(None); + has_changes = true; + } + } + NullableValue::String(raw) => { + let trimmed = raw.trim(); + if trimmed.is_empty() { + return Err(AppError::bad_request("issued_at must not be empty")); + } + let parsed = DateTime::parse_from_rfc3339(trimmed).map_err(|err| { + let msg = format!("issued_at must be an RFC3339 timestamp: {err}"); + AppError::bad_request(msg) + })?; + let normalized = Some(parsed.naive_utc()); + if document.issued_at != normalized { + changes.issued_at = Some(normalized); + has_changes = true; + } + } + } + + if let Some(metadata_update) = metadata { + let next_metadata = if metadata_update.replace { + metadata_update.value + } else { + merge_document_metadata(document.metadata.clone(), metadata_update.value)? + }; + + if document.metadata != next_metadata { + changes.metadata = Some(next_metadata); + has_changes = true; + } + } + + if !has_changes { return Err(AppError::bad_request("no changes provided")); } - if let Some(title) = new_title { - let now = Utc::now().naive_utc(); - let new_filename = filename_with_retained_extension(&title, &document.filename); + let now = Utc::now().naive_utc(); + changes.updated_at = Some(now); - let target = documents::table - .find(document_id) - .filter(documents::tenant_id.eq(tenant_id)); + let target = documents::table + .find(document_id) + .filter(documents::tenant_id.eq(tenant_id)); - let update_result = diesel::update(target).set(( - documents::title.eq(&title), - documents::filename.eq(&new_filename), - documents::updated_at.eq(now), - )); + let update_result = diesel::update(target).set(&changes); - match update_result.execute(&mut conn) { - Ok(_) => {} - Err(diesel::result::Error::DatabaseError(DatabaseErrorKind::UniqueViolation, _)) => { - return Err(AppError::conflict( - "another document in this folder already uses that filename", - ) - .with_code("duplicate_filename")); - } - Err(err) => return Err(AppError::from(err)), + match update_result.execute(&mut conn) { + Ok(_) => {} + Err(diesel::result::Error::DatabaseError(DatabaseErrorKind::UniqueViolation, _)) => { + return Err(AppError::conflict( + "another document in this folder already uses that filename", + ) + .with_code("duplicate_filename")); } - - document = documents::table - .find(document_id) - .filter(documents::tenant_id.eq(tenant_id)) - .first(&mut conn)?; + Err(err) => return Err(AppError::from(err)), } + document = documents::table + .find(document_id) + .filter(documents::tenant_id.eq(tenant_id)) + .first(&mut conn)?; + let current_version: DocumentVersion = document_versions::table .find(document.current_version_id) .first(&mut conn)?; @@ -1300,6 +1387,55 @@ pub async fn update_document( })) } +fn merge_document_metadata(existing: Value, updates: Value) -> AppResult { + let mut base = match existing { + Value::Object(map) => map, + Value::Null => Map::new(), + _ => { + return Err(AppError::bad_request( + "existing metadata is not an object; set replace=true to overwrite", + )); + } + }; + + let incoming = match updates { + Value::Object(map) => map, + _ => { + return Err(AppError::bad_request( + "metadata value must be a JSON object when replace is false", + )); + } + }; + + merge_metadata_maps(&mut base, incoming); + Ok(Value::Object(base)) +} + +fn merge_metadata_maps(target: &mut Map, updates: Map) { + for (key, value) in updates { + match target.entry(key) { + Entry::Occupied(mut entry) => { + let existing = entry.get_mut(); + match value { + Value::Object(update_map) => { + if let Value::Object(existing_map) = existing { + merge_metadata_maps(existing_map, update_map); + } else { + *existing = Value::Object(update_map); + } + } + other => { + *existing = other; + } + } + } + Entry::Vacant(entry) => { + entry.insert(value); + } + } + } +} + pub async fn move_document( Path(document_id): Path, TenantScopedConn { diff --git a/backend/tests/documents_flow.rs b/backend/tests/documents_flow.rs index c74154a..4b530da 100644 --- a/backend/tests/documents_flow.rs +++ b/backend/tests/documents_flow.rs @@ -4,6 +4,7 @@ use anyhow::Result; use axum::http::StatusCode; use common::{acquire_db_lock, body_to_vec, TestApp, UploadExtras}; use serde::{Deserialize, Serialize}; +use serde_json::{json, Value}; use uuid::Uuid; #[derive(Deserialize)] @@ -19,6 +20,7 @@ struct DocumentInfo { original_name: String, deleted_at: Option, issued_at: Option, + metadata: Value, tags: Vec, #[serde(default)] correspondents: Vec, @@ -103,6 +105,13 @@ struct AnalyzeJobPayload { force: bool, } +#[derive(Deserialize)] +struct ErrorResponse { + error: String, + #[serde(default)] + code: Option, +} + #[derive(Deserialize)] struct FolderResponse { folder: FolderInfo, @@ -1229,3 +1238,397 @@ async fn bulk_reanalyze_selected_documents() -> Result<()> { app.cleanup().await?; Ok(()) } + +#[tokio::test] +async fn patch_document_updates_title_and_handles_conflict() -> Result<()> { + let _lock = acquire_db_lock().await; + let app = TestApp::new().await?; + + let password = "patch-title"; + app.insert_user("editor", password, "admin").await?; + let token = app.login_token("editor", password).await?; + + let first_upload = app + .upload_document( + "/api/documents", + "report.pdf", + "application/pdf", + b"fake pdf contents", + None, + &token, + ) + .await?; + let first_body = body_to_vec(first_upload.into_body()).await?; + let mut first_detail: DocumentDetail = serde_json::from_slice(&first_body)?; + + let update = app + .patch_json( + &format!("/api/documents/{}", first_detail.document.id), + &json!({ + "title": "Quarterly Summary" + }), + Some(&token), + ) + .await?; + assert_eq!(update.status(), StatusCode::OK); + let update_body = body_to_vec(update.into_body()).await?; + first_detail = serde_json::from_slice(&update_body)?; + assert_eq!(first_detail.document.title, "Quarterly Summary"); + assert_eq!(first_detail.document.filename, "Quarterly Summary.pdf"); + + let second_upload = app + .upload_document( + "/api/documents", + "notes.pdf", + "application/pdf", + b"other pdf", + None, + &token, + ) + .await?; + let second_body = body_to_vec(second_upload.into_body()).await?; + let second_detail: DocumentDetail = serde_json::from_slice(&second_body)?; + + let conflict = app + .patch_json( + &format!("/api/documents/{}", second_detail.document.id), + &json!({ + "title": "Quarterly Summary" + }), + Some(&token), + ) + .await?; + assert_eq!(conflict.status(), StatusCode::CONFLICT); + let conflict_body = body_to_vec(conflict.into_body()).await?; + let conflict_json: ErrorResponse = serde_json::from_slice(&conflict_body)?; + assert_eq!(conflict_json.code.as_deref(), Some("duplicate_filename")); + + app.cleanup().await?; + Ok(()) +} + +#[tokio::test] +async fn patch_document_updates_and_clears_issued_at() -> Result<()> { + let _lock = acquire_db_lock().await; + let app = TestApp::new().await?; + + let password = "patch-issued"; + app.insert_user("scheduler", password, "admin").await?; + let token = app.login_token("scheduler", password).await?; + + let upload = app + .upload_document( + "/api/documents", + "invoice.txt", + "text/plain", + b"invoice contents", + None, + &token, + ) + .await?; + let body = body_to_vec(upload.into_body()).await?; + let detail: DocumentDetail = serde_json::from_slice(&body)?; + assert!(detail.document.issued_at.is_none()); + + let issued_at = "2021-02-03T04:05:06Z"; + let response = app + .patch_json( + &format!("/api/documents/{}", detail.document.id), + &json!({ + "issued_at": issued_at + }), + Some(&token), + ) + .await?; + assert_eq!(response.status(), StatusCode::OK); + let body = body_to_vec(response.into_body()).await?; + let patched: DocumentDetail = serde_json::from_slice(&body)?; + assert_eq!( + patched.document.issued_at.as_deref(), + Some("2021-02-03T04:05:06+00:00") + ); + + let cleared = app + .patch_json( + &format!("/api/documents/{}", detail.document.id), + &json!({ + "issued_at": null + }), + Some(&token), + ) + .await?; + let cleared_status = cleared.status(); + let cleared_body = body_to_vec(cleared.into_body()).await?; + assert!( + cleared_status == StatusCode::OK, + "clear issued_at failed status {} body {}", + cleared_status, + String::from_utf8_lossy(&cleared_body) + ); + let cleared_detail: DocumentDetail = serde_json::from_slice(&cleared_body)?; + assert!(cleared_detail.document.issued_at.is_none()); + + app.cleanup().await?; + Ok(()) +} + +#[tokio::test] +async fn patch_document_metadata_merge_and_replace() -> Result<()> { + let _lock = acquire_db_lock().await; + let app = TestApp::new().await?; + + let password = "patch-meta"; + app.insert_user("curator", password, "admin").await?; + let token = app.login_token("curator", password).await?; + + let initial_metadata = r#"{"existing":{"keep":true},"other":1}"#; + let upload = app + .upload_document_with_options( + "/api/documents", + "meta.txt", + "text/plain", + b"meta", + None, + None, + Some(initial_metadata), + &token, + ) + .await?; + let upload_body = body_to_vec(upload.into_body()).await?; + let detail: DocumentDetail = serde_json::from_slice(&upload_body)?; + assert_eq!( + detail.document.metadata, + json!({ + "existing": {"keep": true}, + "other": 1 + }) + ); + + let merge_response = app + .patch_json( + &format!("/api/documents/{}", detail.document.id), + &json!({ + "metadata": { + "value": { + "existing": {"update": 5}, + "added": "new" + } + } + }), + Some(&token), + ) + .await?; + assert_eq!(merge_response.status(), StatusCode::OK); + let merge_body = body_to_vec(merge_response.into_body()).await?; + let merged: DocumentDetail = serde_json::from_slice(&merge_body)?; + assert_eq!( + merged.document.metadata, + json!({ + "existing": {"keep": true, "update": 5}, + "other": 1, + "added": "new" + }) + ); + + let replace_response = app + .patch_json( + &format!("/api/documents/{}", detail.document.id), + &json!({ + "metadata": { + "replace": true, + "value": {"fresh": true} + } + }), + Some(&token), + ) + .await?; + assert_eq!(replace_response.status(), StatusCode::OK); + let replace_body = body_to_vec(replace_response.into_body()).await?; + let replaced: DocumentDetail = serde_json::from_slice(&replace_body)?; + assert_eq!(replaced.document.metadata, json!({"fresh": true})); + + app.cleanup().await?; + Ok(()) +} + +#[tokio::test] +async fn patch_document_validation_errors() -> Result<()> { + let _lock = acquire_db_lock().await; + let app = TestApp::new().await?; + + let password = "patch-errors"; + app.insert_user("auditor", password, "admin").await?; + let token = app.login_token("auditor", password).await?; + + let upload = app + .upload_document( + "/api/documents", + "errors.txt", + "text/plain", + b"errors", + None, + &token, + ) + .await?; + let body = body_to_vec(upload.into_body()).await?; + let detail: DocumentDetail = serde_json::from_slice(&body)?; + + let empty_title = app + .patch_json( + &format!("/api/documents/{}", detail.document.id), + &json!({ "title": " " }), + Some(&token), + ) + .await?; + assert_eq!(empty_title.status(), StatusCode::BAD_REQUEST); + let title_body = body_to_vec(empty_title.into_body()).await?; + let title_error: ErrorResponse = serde_json::from_slice(&title_body)?; + assert_eq!(title_error.error, "title must not be empty"); + + let empty_issued = app + .patch_json( + &format!("/api/documents/{}", detail.document.id), + &json!({ "issued_at": "" }), + Some(&token), + ) + .await?; + assert_eq!(empty_issued.status(), StatusCode::BAD_REQUEST); + let issued_body = body_to_vec(empty_issued.into_body()).await?; + let issued_error: ErrorResponse = serde_json::from_slice(&issued_body)?; + assert_eq!(issued_error.error, "issued_at must not be empty"); + + let invalid_merge = app + .patch_json( + &format!("/api/documents/{}", detail.document.id), + &json!({ + "metadata": { + "value": 5 + } + }), + Some(&token), + ) + .await?; + assert_eq!(invalid_merge.status(), StatusCode::BAD_REQUEST); + let merge_body = body_to_vec(invalid_merge.into_body()).await?; + let merge_error: ErrorResponse = serde_json::from_slice(&merge_body)?; + assert_eq!( + merge_error.error, + "metadata value must be a JSON object when replace is false" + ); + + let replace_scalar = app + .patch_json( + &format!("/api/documents/{}", detail.document.id), + &json!({ + "metadata": { + "replace": true, + "value": 5 + } + }), + Some(&token), + ) + .await?; + assert_eq!(replace_scalar.status(), StatusCode::OK); + let replace_body = body_to_vec(replace_scalar.into_body()).await?; + let replace_detail: DocumentDetail = serde_json::from_slice(&replace_body)?; + assert_eq!(replace_detail.document.metadata, json!(5)); + + let merge_after_scalar = app + .patch_json( + &format!("/api/documents/{}", detail.document.id), + &json!({ + "metadata": { + "value": { "new": 1 } + } + }), + Some(&token), + ) + .await?; + assert_eq!(merge_after_scalar.status(), StatusCode::BAD_REQUEST); + let merge_after_body = body_to_vec(merge_after_scalar.into_body()).await?; + let merge_after_error: ErrorResponse = serde_json::from_slice(&merge_after_body)?; + assert_eq!( + merge_after_error.error, + "existing metadata is not an object; set replace=true to overwrite" + ); + + let malformed_timestamp = app + .patch_json( + &format!("/api/documents/{}", detail.document.id), + &json!({ "issued_at": "not-a-timestamp" }), + Some(&token), + ) + .await?; + assert_eq!(malformed_timestamp.status(), StatusCode::BAD_REQUEST); + let malformed_body = body_to_vec(malformed_timestamp.into_body()).await?; + let malformed_error: ErrorResponse = serde_json::from_slice(&malformed_body)?; + assert!( + malformed_error + .error + .starts_with("issued_at must be an RFC3339 timestamp"), + "unexpected error: {}", + malformed_error.error + ); + + app.cleanup().await?; + Ok(()) +} + +#[tokio::test] +async fn patch_document_updates_multiple_fields() -> Result<()> { + let _lock = acquire_db_lock().await; + let app = TestApp::new().await?; + + let password = "patch-multi"; + app.insert_user("planner", password, "admin").await?; + let token = app.login_token("planner", password).await?; + + let upload = app + .upload_document( + "/api/documents", + "multi.pdf", + "application/pdf", + b"multi", + None, + &token, + ) + .await?; + let body = body_to_vec(upload.into_body()).await?; + let detail: DocumentDetail = serde_json::from_slice(&body)?; + + let response = app + .patch_json( + &format!("/api/documents/{}", detail.document.id), + &json!({ + "title": "Annual Report", + "issued_at": "2022-05-01T12:00:00Z", + "metadata": { + "value": { + "department": "finance", + "year": 2022 + } + } + }), + Some(&token), + ) + .await?; + assert_eq!(response.status(), StatusCode::OK); + let response_body = body_to_vec(response.into_body()).await?; + let updated: DocumentDetail = serde_json::from_slice(&response_body)?; + assert_eq!(updated.document.title, "Annual Report"); + assert_eq!(updated.document.filename, "Annual Report.pdf"); + assert_eq!( + updated.document.issued_at.as_deref(), + Some("2022-05-01T12:00:00+00:00") + ); + assert_eq!( + updated.document.metadata, + json!({ + "department": "finance", + "year": 2022 + }) + ); + + app.cleanup().await?; + Ok(()) +}