diff --git a/migrations/20261021000000_file_attached_blobs_decrement_trigger.sql b/migrations/20261021000000_file_attached_blobs_decrement_trigger.sql new file mode 100644 index 00000000..a1d8c368 --- /dev/null +++ b/migrations/20261021000000_file_attached_blobs_decrement_trigger.sql @@ -0,0 +1,28 @@ +-- Release the blob reference when an attachment row goes away. +-- +-- `storage.file_attached_blobs.file_id` is `ON DELETE CASCADE`, so deleting a +-- file removes its attachment rows inside the database — invisible to Rust. +-- The lifecycle hook cannot cover this: `on_file_deleted` fires AFTER +-- `delete_file`, by which point the cascade has already run and there is +-- nothing left to read. The references would survive with no row behind them, +-- and `dedup_gc` would see a positive count forever — bytes pinned for good. +-- +-- `storage.decrement_blob_ref()` already exists for exactly this, on +-- `storage.files`. It keys off `OLD.blob_hash` and is otherwise +-- table-agnostic, so it applies verbatim — and reusing it keeps the +-- manifest-first decrement contract defined in one place rather than +-- transcribed into a second trigger that can drift. +-- +-- Only DELETE. Replacing a preview updates `blob_hash` in place +-- (`store_attached_blob` is ON CONFLICT DO UPDATE), and the reference to the +-- superseded blob is released there, in Rust. Adding UPDATE here would +-- double-decrement it. + +CREATE OR REPLACE TRIGGER trg_file_attached_blobs_decrement_blob_ref + AFTER DELETE ON storage.file_attached_blobs + FOR EACH ROW + EXECUTE FUNCTION storage.decrement_blob_ref(); + +COMMENT ON TRIGGER trg_file_attached_blobs_decrement_blob_ref + ON storage.file_attached_blobs IS + 'Releases the blob reference held by an attachment row. Needed because file_id is ON DELETE CASCADE, so rows vanish inside the DB where the Rust lifecycle hooks cannot see them.'; diff --git a/src/infrastructure/services/dedup_service.rs b/src/infrastructure/services/dedup_service.rs index c4826648..c0d1d15f 100644 --- a/src/infrastructure/services/dedup_service.rs +++ b/src/infrastructure/services/dedup_service.rs @@ -576,6 +576,106 @@ impl DedupService { /// `docs/plan/derived-blobs.md`. /// /// Returns the derived blob hash. + /// Attach user-supplied bytes to a FILE — the file-keyed twin of + /// [`Self::store_derived_blob`]. + /// + /// Same storage path (the bytes are still content-addressed and still + /// deduplicated), different mapping: the row is keyed by `file_id`, so + /// two files holding identical attached bytes get two rows and two + /// references. Sharing the mapping is what must not happen — a + /// content-keyed client preview would let one user's upload be served + /// for another user's file. + /// + /// `ON CONFLICT … DO UPDATE`, unlike the derived twin: re-uploading a + /// preview for the same `(file_id, kind, variant)` is a deliberate + /// replacement, whereas a re-derived thumbnail is the same bytes again. + /// The reference held by the row being replaced is released. + pub async fn store_attached_blob( + &self, + file_id: &str, + kind: &str, + variant: &str, + content_type: &str, + bytes: Bytes, + uploaded_by: uuid::Uuid, + ) -> Result { + let stored = self + .store_from_stream( + stream::once(async move { Ok::(bytes) }), + Some(content_type.to_string()), + ) + .await?; + let attached_hash = stored.hash().to_string(); + + // `previous` is the hash this row pointed at before, when it existed + // and differed — the reference to release once the row no longer + // holds it. + let previous: Option<(Option,)> = sqlx::query_as( + "INSERT INTO storage.file_attached_blobs + (file_id, kind, variant, blob_hash, content_type, uploaded_by) + VALUES ($1::uuid, $2, $3, $4, $5, $6) + ON CONFLICT (file_id, kind, variant) DO UPDATE + SET blob_hash = EXCLUDED.blob_hash, + content_type = EXCLUDED.content_type, + uploaded_by = EXCLUDED.uploaded_by, + created_at = now() + RETURNING NULLIF(storage.file_attached_blobs.blob_hash, EXCLUDED.blob_hash)", + ) + .bind(file_id) + .bind(kind) + .bind(variant) + .bind(&attached_hash) + .bind(content_type) + .bind(uploaded_by) + .fetch_optional(self.pool.as_ref()) + .await + .map_err(|e| DomainError::internal_error("Dedup", format!("record attached blob: {e}")))?; + + // A replaced row's old blob loses its only reference from here. Not + // releasing it would pin those bytes forever — nothing else points at + // a superseded preview. + if let Some((Some(old_hash),)) = previous + && old_hash != attached_hash + && let Err(e) = self.remove_reference(&old_hash).await + { + tracing::warn!( + target: "oxicloud::dedup", + error = %e, + "failed to release replaced attached-blob reference for {}", + &old_hash[..old_hash.len().min(12)], + ); + } + + Ok(attached_hash) + } + + /// Look up bytes attached to a file. File-keyed counterpart of + /// [`Self::find_derived_blob`]. + pub async fn find_attached_blob( + &self, + file_id: &str, + kind: &str, + variant: &str, + ) -> Option { + sqlx::query_as::<_, (String, String)>( + "SELECT blob_hash, content_type FROM storage.file_attached_blobs + WHERE file_id = $1::uuid AND kind = $2 AND variant = $3", + ) + .bind(file_id) + .bind(kind) + .bind(variant) + .fetch_optional(self.pool.as_ref()) + .await + .ok() + .flatten() + .map(|(blob_hash, content_type)| { + crate::application::ports::dedup_ports::DerivedBlobRef { + blob_hash, + content_type, + } + }) + } + pub async fn store_derived_blob( &self, source_hash: &str, diff --git a/src/infrastructure/services/thumbnail_service.rs b/src/infrastructure/services/thumbnail_service.rs index 3da76cbe..92f91c07 100644 --- a/src/infrastructure/services/thumbnail_service.rs +++ b/src/infrastructure/services/thumbnail_service.rs @@ -505,6 +505,39 @@ impl ThumbnailService { /// `blob_hash` is used to locate the file on disk (dedup-aware). /// If `None`, only the in-memory cache is checked (used for video /// thumbnails where blob_hash is not yet resolved). + /// Drain a blob through the dedup stack into memory. + /// + /// Shared by the attached and derived tiers — the only difference between + /// them is which table produced the hash, so the read itself belongs in + /// one place. Returns `None` on a read fault rather than propagating: a + /// missing satellite must degrade to the next tier, never break a gallery. + async fn read_blob_to_bytes( + dedup: &DedupService, + blob_hash: &str, + file_id: &str, + size: ThumbnailSize, + ) -> Option { + use futures::StreamExt; + let mut stream = dedup.read_blob_stream(blob_hash).await.ok()?; + let mut buf = Vec::new(); + while let Some(chunk) = stream.next().await { + match chunk { + Ok(part) => buf.extend_from_slice(&part), + Err(e) => { + tracing::warn!( + target: "oxicloud::dedup", + error = %e, + "thumbnail blob read failed for {} {:?}", + file_id, + size, + ); + return None; + } + } + } + Some(Bytes::from(buf)) + } + pub async fn get_cached_thumbnail( &self, file_id: &str, @@ -553,6 +586,32 @@ impl ThumbnailService { return Some(bytes); } + // 2b. Bytes the USER attached to this file, if any. + // + // Ahead of every content-derived tier below on purpose: an uploaded + // preview is an explicit choice about THIS file and must beat + // anything the server would render from its content. It is also the + // only tier a copy can inherit — the `ext-` sidecar above is keyed by + // file_id and is not copied, so without this branch a copied file + // silently falls back to a rendered thumbnail, or to none at all for + // a PDF that has no server-side render path. + if let Some(dedup) = dedup + && let Some(attached) = dedup + .find_attached_blob(file_id, "preview", size.dir_name()) + .await + && let Some(bytes) = + Self::read_blob_to_bytes(dedup, &attached.blob_hash, file_id, size).await + { + // Cached under the per-file key: these bytes belong to this file, + // not to its content, so a content key would leak them to every + // other file sharing that content — the poisoning the file-keyed + // table exists to prevent. + self.cache + .insert(ThumbnailCacheKey::external(file_id, size), bytes.clone()) + .await; + return Some(bytes); + } + // 3. Check disk for blob-hash thumbnails (needs blob_hash to locate) let hash = blob_hash?; let thumb_path = self.get_thumbnail_path(hash, size, format); @@ -580,25 +639,7 @@ impl ThumbnailService { let derived = dedup .find_derived_blob(hash, "thumbnail", size.dir_name()) .await?; - use futures::StreamExt; - let mut stream = dedup.read_blob_stream(&derived.blob_hash).await.ok()?; - let mut buf = Vec::new(); - while let Some(chunk) = stream.next().await { - match chunk { - Ok(part) => buf.extend_from_slice(&part), - Err(e) => { - tracing::warn!( - target: "oxicloud::dedup", - error = %e, - "derived thumbnail read failed for {} {:?}", - file_id, - size, - ); - return None; - } - } - } - let bytes = Bytes::from(buf); + let bytes = Self::read_blob_to_bytes(dedup, &derived.blob_hash, file_id, size).await?; self.cache .insert( ThumbnailCacheKey::content(hash, size, format), diff --git a/src/interfaces/api/handlers/file_handler.rs b/src/interfaces/api/handlers/file_handler.rs index 7d2e7f71..7fdf95be 100644 --- a/src/interfaces/api/handlers/file_handler.rs +++ b/src/interfaces/api/handlers/file_handler.rs @@ -726,15 +726,57 @@ impl FileHandler { return AppError::from(err).into_response(); } - // Validate, re-encode to WebP, and store - match thumbnail_service + // Validate, re-encode, and store the per-file sidecar. + let stored = match thumbnail_service .store_external_thumbnail(&id, thumb_size.into(), body) .await { - Ok(_) => StatusCode::CREATED.into_response(), - Err(err) => AppError::internal_error(format!("Failed to store thumbnail: {}", err)) - .into_response(), + Ok(bytes) => bytes, + Err(err) => { + return AppError::internal_error(format!("Failed to store thumbnail: {}", err)) + .into_response(); + } + }; + + // Also record it as a file-keyed attachment. + // + // The sidecar above is `ext-{file_id}.jpg` on local disk, which no + // copy path duplicates and no other instance can see. Without this + // row a copied file loses the preview its owner uploaded — falling + // back to a rendered thumbnail, or to nothing at all for a PDF, which + // has no server-side render path. `copy_file_satellites` duplicates + // the row, so the copy inherits the bytes. + // + // File-keyed, never content-keyed: these bytes are the uploader's + // claim about THIS file, and sharing them across files with identical + // content is the poisoning vector `storage.file_attached_blobs` + // exists to prevent. + // + // Best-effort: the sidecar already succeeded, so the user has their + // thumbnail. Failing the request here would report an error for an + // operation that visibly worked. + if let Err(e) = state + .core + .dedup_service + .store_attached_blob( + &id, + "preview", + thumb_size.dir_name(), + "image/jpeg", + stored, + auth_user.id, + ) + .await + { + tracing::warn!( + target: "oxicloud::dedup", + error = %e, + file_id = %id, + "failed to record attached thumbnail; sidecar written, copies will not inherit it" + ); } + + StatusCode::CREATED.into_response() } // ═══════════════════════════════════════════════════════════════════════ diff --git a/tests/api/attached_thumbnail_copy.hurl b/tests/api/attached_thumbnail_copy.hurl new file mode 100644 index 00000000..bf4831b1 --- /dev/null +++ b/tests/api/attached_thumbnail_copy.hurl @@ -0,0 +1,275 @@ +# ============================================================= +# OxiCloud – An UPLOADED thumbnail survives both copy paths +# ============================================================= +# A user-supplied preview is not derivable from the file's content, so +# nothing can regenerate it. If a copy loses it, it is gone — and the loss +# is silent, because the server quietly falls back to rendering one from +# the source (or to 204 for a PDF, which has no render path at all). +# +# That was the behaviour before `storage.file_attached_blobs`: the PUT +# wrote `ext-{file_id}.jpg`, keyed by file id, which no copy path +# duplicates and no other instance can see. +# +# The test distinguishes "preserved" from "re-rendered" by making the two +# visibly different: the FILE is red-image.png, the uploaded thumbnail is +# derived from green-image.png. A server-side render of the file could +# only ever produce the red one. So byte-equality with the post-upload +# bytes proves the copy served the ATTACHMENT, not a fresh render. +# +# Step 4 is what makes that airtight — it captures the rendered thumbnail +# BEFORE the upload and requires the upload to change it. Without that, +# byte-equality across copies could be satisfied by three identical +# renders. +# +# Prerequisites: setup.hurl must have run (admin user exists). +# ============================================================= + + +# ───────────────────────────────────────────────────────────── +# Step 1 – Login +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ + "username": "{{username}}", + "password": "{{password}}" +} + +HTTP 200 +[Captures] +token: jsonpath "$.access_token" + + +# ───────────────────────────────────────────────────────────── +# Step 2 – Source and destination folders +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/folders +Authorization: Bearer {{token}} +Content-Type: application/json +{ + "name": "hurl-attach-src" +} + +HTTP 201 +[Captures] +src_folder_id: jsonpath "$.id" + + +POST {{base_url}}/api/folders +Authorization: Bearer {{token}} +Content-Type: application/json +{ + "name": "hurl-attach-dst" +} + +HTTP 201 +[Captures] +dst_folder_id: jsonpath "$.id" + + +# ───────────────────────────────────────────────────────────── +# Step 3 – Upload the file (RED) +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/files/upload +Authorization: Bearer {{token}} +[MultipartFormData] +folder_id: {{src_folder_id}} +file: file,fixtures/red-image.png; image/png + +HTTP 201 +[Captures] +orig_file_id: jsonpath "$.id" +orig_file_name: jsonpath "$.name" + + +# ───────────────────────────────────────────────────────────── +# Step 4 – The server-rendered thumbnail, before any upload. +# Captured so the upload can be shown to have replaced it. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/files/{{orig_file_id}}/thumbnail/preview +Authorization: Bearer {{token}} + +HTTP 200 +[Captures] +rendered_thumb: bytes + + +# ───────────────────────────────────────────────────────────── +# Step 5 – Upload a custom thumbnail (GREEN) for that file +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/api/files/{{orig_file_id}}/thumbnail/preview +Authorization: Bearer {{token}} +Content-Type: image/png +file,fixtures/green-image.png; + +HTTP 201 + + +# It must now serve the upload, not the render. +GET {{base_url}}/api/files/{{orig_file_id}}/thumbnail/preview +Authorization: Bearer {{token}} + +HTTP 200 +[Captures] +uploaded_thumb: bytes +[Asserts] +bytes != {{rendered_thumb}} + + +# ───────────────────────────────────────────────────────────── +# Step 6 – Single-file copy → the attachment comes with it. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/batch/files/copy +Authorization: Bearer {{token}} +Content-Type: application/json +{ + "file_ids": ["{{orig_file_id}}"], + "target_folder_id": "{{dst_folder_id}}" +} + +HTTP 200 +[Captures] +file_copy_id: jsonpath "$.successful[0].id" +[Asserts] +jsonpath "$.successful[0].id" != "{{orig_file_id}}" + + +GET {{base_url}}/api/files/{{file_copy_id}}/thumbnail/preview +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +bytes == {{uploaded_thumb}} +bytes != {{rendered_thumb}} + + +# ───────────────────────────────────────────────────────────── +# Step 7 – Folder copy → same, through storage.copy_folder_tree. +# +# The other copy path. It reaches the attachment through the same +# `copy_file_satellites` call, and this is the leg that would break if +# the tree path ever grew its own fan-out again. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/batch/folders/copy +Authorization: Bearer {{token}} +Content-Type: application/json +{ + "folder_ids": ["{{src_folder_id}}"], + "target_folder_id": "{{dst_folder_id}}" +} + +HTTP 200 +[Captures] +tree_root_id: jsonpath "$.successful[0].new_root_folder_id" +[Asserts] +jsonpath "$.stats.failed" == 0 + + +GET {{base_url}}/api/files?folder_id={{tree_root_id}} +Authorization: Bearer {{token}} + +HTTP 200 +[Captures] +tree_copy_id: jsonpath "$[0].id" +[Asserts] +jsonpath "$" count == 1 +jsonpath "$[0].name" == "{{orig_file_name}}" +jsonpath "$[0].id" != "{{orig_file_id}}" + + +GET {{base_url}}/api/files/{{tree_copy_id}}/thumbnail/preview +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +bytes == {{uploaded_thumb}} +bytes != {{rendered_thumb}} + + +# ───────────────────────────────────────────────────────────── +# Step 8 – Delete the ORIGINAL, run GC, and require both copies to keep +# serving the upload. +# +# Each copy holds its own reference on the attached blob — the rows are +# duplicated, not shared, because the table is file-keyed. If the copy +# had failed to take one, deleting the original would walk the count to +# zero and GC would reap bytes that cannot be regenerated. +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/api/files/{{orig_file_id}} +Authorization: Bearer {{token}} + +HTTP 204 + + +GET {{base_url}}/api/trash/resources +Authorization: Bearer {{token}} + +HTTP 200 +[Captures] +trash_orig_id: jsonpath "$.items[?(@.resource.id == '{{orig_file_id}}')].resource.id" + + +DELETE {{base_url}}/api/trash/{{trash_orig_id}} +Authorization: Bearer {{token}} + +HTTP 200 + + +POST {{base_url}}/api/admin/jobs/dedup_gc/trigger +Authorization: Bearer {{token}} +[Options] +delay: 500ms + +HTTP 200 + + +GET {{base_url}}/api/files/{{file_copy_id}}/thumbnail/preview +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +bytes == {{uploaded_thumb}} + + +GET {{base_url}}/api/files/{{tree_copy_id}}/thumbnail/preview +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +bytes == {{uploaded_thumb}} + + +# ───────────────────────────────────────────────────────────── +# Step 9 – Teardown. Hurl files share one database within run.sh. +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/api/folders/{{src_folder_id}} +Authorization: Bearer {{token}} + +HTTP 204 + + +DELETE {{base_url}}/api/folders/{{dst_folder_id}} +Authorization: Bearer {{token}} + +HTTP 204 + + +GET {{base_url}}/api/trash/resources +Authorization: Bearer {{token}} + +HTTP 200 +[Captures] +trash_src_id: jsonpath "$.items[?(@.resource.id == '{{src_folder_id}}')].resource.id" +trash_dst_id: jsonpath "$.items[?(@.resource.id == '{{dst_folder_id}}')].resource.id" + + +DELETE {{base_url}}/api/trash/{{trash_src_id}} +Authorization: Bearer {{token}} + +HTTP 200 + + +DELETE {{base_url}}/api/trash/{{trash_dst_id}} +Authorization: Bearer {{token}} + +HTTP 200 diff --git a/tests/api/run.sh b/tests/api/run.sh index d644513b..3d17c303 100755 --- a/tests/api/run.sh +++ b/tests/api/run.sh @@ -168,6 +168,7 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test "$API_DIR/dedup_blob_cleanup.hurl" \ "$API_DIR/derived_blob_copy.hurl" \ "$API_DIR/thumbnail_etag_content_keyed.hurl" \ + "$API_DIR/attached_thumbnail_copy.hurl" \ "$API_DIR/dedup_admin_gate.hurl" \ "$API_DIR/admin_jobs.hurl" \ "$API_DIR/recoverable_jobs.hurl" \