From f777764839e716f99f2848b1a5b5168c52f8a5c8 Mon Sep 17 00:00:00 2001 From: fabi Date: Wed, 29 Jul 2026 19:41:51 +0200 Subject: [PATCH] fix(maintenance): reclaim the media of deliberately deleted uploads The quota stopped bounding the disk. `soft_delete_in_event` stamps `deleted_at` and refunds `total_upload_bytes`, but nothing ever removed the bytes, and the hourly sweep reached only `compression_status = 'failed'`. Upload 500 MB, delete, quota back to zero, upload another 500 MB. Not an attack -- a guest curating their camera roll, which is what people do. The host then sees guests hitting "Du hast dein Upload-Limit erreicht" while the admin widget shows a disk full of files no upload row points at, and the quota message is actively misleading because the space really is gone, just not to anyone the accounting can name. Two retention windows, because the two deletes mean different things. A compression failure keeps its 14 days: the guest didn't ask for it and may not be able to retake the photo. A deliberate removal gets 24 hours -- 14 days outlives the whole event, so a deliberate delete would never reclaim anything while it mattered, and a day still covers a mis-tap. Wider than reported: ALL FOUR paths are reclaimed, not just the original. Preview, display and thumbnail are each a separate file, none counted in `original_size_bytes`, and nothing ever removed them either. That was invisible while the sweep only saw failed compressions (which produce no derivatives) and becomes three leaked files per upload the moment it reaches a successful one. A row is re-selected until every path is cleared, and the columns are cleared only once every file for that upload is gone -- clearing after a partial success would strand the survivors in exactly the unowned state this drains. `backfill_stale_derivatives` selects on `display_path IS NULL AND preview_path IS NOT NULL`, which is close enough to the post-sweep state to be worth pinning: it is guarded on `deleted_at IS NULL`, so it cannot re-decode an original that is no longer on disk. Covered. Residual, deliberately: within the 24h window the bytes are still spent and still unaccounted, so delete-and-re-upload through an eight-hour event can outrun the sweep. Bounding that means holding the quota until the file is reclaimed rather than refunding at `deleted_at`. The low-disk warning is the net under it. Tests: 6 DB-backed, replacing 3. The one asserting an owner-deleted upload IS reclaimed is the exact inverse of what this file used to assert. Co-Authored-By: Claude Opus 5 --- backend/src/services/maintenance.rs | 137 ++++++--- backend/tests/failed_original_sweep.rs | 409 +++++++++++++++++-------- 2 files changed, 373 insertions(+), 173 deletions(-) diff --git a/backend/src/services/maintenance.rs b/backend/src/services/maintenance.rs index 2cd07eb..b59b046 100644 --- a/backend/src/services/maintenance.rs +++ b/backend/src/services/maintenance.rs @@ -11,8 +11,9 @@ //! 2. **Periodic tasks** — pruning that should happen "every hour" rather than per //! request: expired sessions (otherwise the table grows unboundedly), the //! rate-limiter's in-memory windows (so keys for IPs that left long ago don't -//! accumulate), and the originals of uploads whose compression permanently failed -//! (which are deliberately retained for a recovery window, then reclaimed). +//! accumulate), and the media of soft-deleted uploads — both the ones whose compression +//! permanently failed and the ones a guest or host deliberately removed — which are +//! retained for a recovery window and then reclaimed. use std::path::PathBuf; use std::time::Duration; @@ -37,6 +38,27 @@ use crate::services::sse_tickets::SseTicketStore; /// failed upload still has the file, while the leak stays bounded. const FAILED_ORIGINAL_RETENTION_DAYS: i64 = 14; +/// How long a DELIBERATELY deleted upload's files are kept before they are reclaimed. +/// +/// The same leak, reached by the ordinary path rather than the exceptional one. +/// `soft_delete_in_event` stamps `deleted_at` and refunds `total_upload_bytes`, but nothing ever +/// removed the bytes — so the quota stopped bounding the disk. Upload 500 MB, delete, quota is back +/// to zero, upload another 500 MB: not an attack, just a guest curating their camera roll, which is +/// what people do. The host then sees guests hitting "Du hast dein Upload-Limit erreicht" while the +/// admin widget shows a disk full of files no upload row points at, and the quota message is +/// actively misleading because the space really is gone — just not to anyone the accounting can +/// name. +/// +/// Much shorter than the failure window on purpose. Fourteen days outlives the whole event, so a +/// deliberate delete would never reclaim anything while it mattered. A day still gives an operator +/// a recovery window for a mis-tap. +/// +/// NOTE what this does NOT do: within the window the bytes are still spent and still unaccounted, +/// so a guest deleting and re-uploading through an eight-hour event can outrun the sweep. Bounding +/// that would mean holding the quota until the file is actually reclaimed rather than refunding at +/// `deleted_at` — a deliberate trade, and the reason the low-disk warning exists. +const DELETED_UPLOAD_RETENTION_HOURS: i64 = 24; + /// Reset rows left in flight by a previous crashed instance. Run once on startup, /// before the HTTP server starts taking requests, so users never observe the /// half-state. @@ -117,38 +139,59 @@ pub fn spawn_periodic_tasks( loop { tick.tick().await; cleanup_sessions(&pool).await; - cleanup_failed_originals(&pool, &media_path).await; + cleanup_deleted_media(&pool, &media_path).await; rate_limiter.prune(); sse_tickets.prune(); } }); } -/// Reclaim the originals of uploads whose compression permanently failed, once they are -/// past [`FAILED_ORIGINAL_RETENTION_DAYS`]. +/// Reclaim the media of soft-deleted uploads once they are past their retention window. /// -/// Deliberately narrow. It only touches rows that are BOTH `compression_status = 'failed'` -/// AND soft-deleted — i.e. the exact state the compression worker's give-up path leaves -/// behind — so it can never reach a live upload or one whose preview works. `original_path` -/// is cleared in the same pass, which makes the sweep idempotent and stops a later run -/// re-reporting a file that is already gone. The row itself is kept: it is the audit trail -/// for the failure, and it costs a few hundred bytes. -async fn cleanup_failed_originals(pool: &PgPool, media_path: &std::path::Path) { - let rows = sqlx::query_as::<_, (uuid::Uuid, String)>( - "SELECT id, original_path FROM upload - WHERE compression_status = 'failed' - AND deleted_at IS NOT NULL - AND deleted_at < NOW() - ($1 || ' days')::interval - AND original_path <> ''", +/// ONLY ever touches rows with `deleted_at IS NOT NULL`, so it can never reach a live upload. Two +/// classes, two windows, because the two deletes mean different things: +/// +/// - a compression failure the guest didn't ask for and may want investigated — +/// [`FAILED_ORIGINAL_RETENTION_DAYS`]; +/// - a deliberate removal by the guest or the host — [`DELETED_UPLOAD_RETENTION_HOURS`]. +/// +/// ALL FOUR paths are reclaimed, not just the original. The previous version cleared +/// `original_path` alone, which was right for its only case (a failed compression produces no +/// derivatives) but wrong the moment the sweep reaches a successfully processed upload: preview, +/// display and thumbnail are each a separate file on disk, none of them counted in +/// `original_size_bytes`, and nothing else ever removed them. +/// +/// Every column is cleared in the same pass, which makes the sweep idempotent and stops a later run +/// re-reporting files that are already gone. The ROW is kept: it is the audit trail, it costs a few +/// hundred bytes, and `backfill_stale_derivatives` is guarded on `deleted_at IS NULL` so a nulled +/// `preview_path` can never make it regenerate what was just reclaimed. +async fn cleanup_deleted_media(pool: &PgPool, media_path: &std::path::Path) { + type Row = ( + uuid::Uuid, + String, + Option, + Option, + Option, + ); + let rows = sqlx::query_as::<_, Row>( + "SELECT id, original_path, preview_path, display_path, thumbnail_path FROM upload + WHERE deleted_at IS NOT NULL + AND CASE WHEN compression_status = 'failed' + THEN deleted_at < NOW() - ($1 || ' days')::interval + ELSE deleted_at < NOW() - ($2 || ' hours')::interval + END + AND (original_path <> '' OR preview_path IS NOT NULL + OR display_path IS NOT NULL OR thumbnail_path IS NOT NULL)", ) .bind(FAILED_ORIGINAL_RETENTION_DAYS.to_string()) + .bind(DELETED_UPLOAD_RETENTION_HOURS.to_string()) .fetch_all(pool) .await; let rows = match rows { Ok(r) => r, Err(e) => { - tracing::warn!(error = ?e, "failed-original sweep query failed"); + tracing::warn!(error = ?e, "deleted-media sweep query failed"); return; } }; @@ -157,32 +200,52 @@ async fn cleanup_failed_originals(pool: &PgPool, media_path: &std::path::Path) { } let mut reclaimed = 0usize; - for (id, original_path) in rows { - let absolute = media_path.join(&original_path); - match tokio::fs::remove_file(&absolute).await { - Ok(()) => reclaimed += 1, - // Already gone (manual cleanup, restored backup) — still clear the column so - // the row stops being re-selected every hour. - Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} - Err(e) => { - tracing::warn!(error = ?e, %id, path = %absolute.display(), - "could not reclaim failed original; leaving the row for the next sweep"); - continue; + for (id, original, preview, display, thumbnail) in rows { + let paths: Vec = std::iter::once(original) + .filter(|p| !p.is_empty()) + .chain([preview, display, thumbnail].into_iter().flatten()) + .collect(); + + // All-or-nothing per row: the columns are only cleared once every file for that upload is + // gone. Clearing after a partial success would strand the survivors with nothing pointing + // at them — the same unowned-bytes state this sweep exists to drain. + let mut all_gone = true; + for rel in &paths { + let absolute = media_path.join(rel); + match tokio::fs::remove_file(&absolute).await { + Ok(()) => reclaimed += 1, + // Already gone (manual cleanup, restored backup) — still counts as reclaimed for + // the purpose of clearing the columns, or the row is re-selected every hour forever. + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => { + tracing::warn!(error = ?e, %id, path = %absolute.display(), + "could not reclaim deleted media; leaving the row for the next sweep"); + all_gone = false; + } } } - if let Err(e) = sqlx::query("UPDATE upload SET original_path = '' WHERE id = $1") - .bind(id) - .execute(pool) - .await + if !all_gone { + continue; + } + + if let Err(e) = sqlx::query( + "UPDATE upload SET original_path = '', preview_path = NULL, + display_path = NULL, thumbnail_path = NULL + WHERE id = $1", + ) + .bind(id) + .execute(pool) + .await { - tracing::warn!(error = ?e, %id, "reclaimed the file but could not clear original_path"); + tracing::warn!(error = ?e, %id, "reclaimed the files but could not clear the paths"); } } if reclaimed > 0 { tracing::info!( - "reclaimed {reclaimed} original(s) from uploads that failed compression more than \ - {FAILED_ORIGINAL_RETENTION_DAYS} days ago" + "reclaimed {reclaimed} file(s) from soft-deleted uploads (deliberate deletes after \ + {DELETED_UPLOAD_RETENTION_HOURS}h, compression failures after \ + {FAILED_ORIGINAL_RETENTION_DAYS}d)" ); } } diff --git a/backend/tests/failed_original_sweep.rs b/backend/tests/failed_original_sweep.rs index 96840df..16fcf02 100644 --- a/backend/tests/failed_original_sweep.rs +++ b/backend/tests/failed_original_sweep.rs @@ -1,19 +1,27 @@ -//! DB-backed tests for the failed-original sweep (`services/maintenance.rs`). +//! DB-backed tests for the deleted-media sweep (`services/maintenance.rs`). //! -//! Context: the compression worker deliberately no longer deletes an upload's original when -//! its transcode fails — a transient ENOSPC or a codec panic must never destroy the only -//! copy of a photo a guest cannot retake. But the row is soft-deleted and the uploader's -//! quota IS refunded, so those bytes become invisible, unowned and free. A guest hitting a -//! reproducible codec failure could accumulate orphans at no personal cost, and since -//! `active_uploaders` counts only users with non-deleted uploads, dropping out of that count -//! actually RAISES everyone's per-user ceiling while the disk fills. +//! Context, in two halves. //! -//! The sweep reclaims them after a retention window. Its selection predicate is the whole -//! safety argument — it must reach the give-up path's leftovers and nothing else — so that -//! is what these tests pin, following the same "reproduce the SQL verbatim" pattern as -//! `upload_concurrency.rs`. +//! The compression worker deliberately no longer deletes an upload's original when its transcode +//! fails — a transient ENOSPC or a codec panic must never destroy the only copy of a photo a guest +//! cannot retake. But the row is soft-deleted and the uploader's quota IS refunded, so those bytes +//! become invisible, unowned and free. //! -//! `#[sqlx::test]` gives each test a fresh database with the real migrations applied. +//! The SAME hole was reachable by the ordinary path, and that one is not an edge case at all: +//! `soft_delete_in_event` refunds `total_upload_bytes` on every guest or host delete and nothing +//! removed the files, so the quota stopped bounding the disk. Upload 500 MB, delete, quota back to +//! zero, upload another 500 MB — a guest curating their camera roll, which is what people do. The +//! sweep used to reach only `compression_status = 'failed'`, so it never touched this case; the +//! test below that now asserts an owner-deleted upload IS reclaimed is the one that used to assert +//! the opposite. +//! +//! Two windows, because the two deletes mean different things: 14 days for a failure an operator +//! may want to investigate, 24 hours for a removal someone asked for (14 days outlives the whole +//! event, so a deliberate delete would never reclaim anything while it mattered). +//! +//! The selection predicate is the whole safety argument — it must reach both leftovers and never a +//! live upload — so that is what these pin, following the same "reproduce the SQL verbatim" pattern +//! as `upload_concurrency.rs`. `#[sqlx::test]` gives each test a fresh, migrated database. mod common; @@ -21,195 +29,324 @@ use common::*; use sqlx::PgPool; use uuid::Uuid; -/// SRC: `services/maintenance.rs::cleanup_failed_originals` — the selection, verbatim. -async fn sweep_selects(pool: &PgPool, retention_days: i64) -> Vec { - sqlx::query_as::<_, (Uuid, String)>( - "SELECT id, original_path FROM upload - WHERE compression_status = 'failed' - AND deleted_at IS NOT NULL - AND deleted_at < NOW() - ($1 || ' days')::interval - AND original_path <> ''", +const FAILED_DAYS: i64 = 14; +const DELETED_HOURS: i64 = 24; + +/// SRC: `services/maintenance.rs::cleanup_deleted_media` — the selection, verbatim. +async fn sweep_selects(pool: &PgPool, failed_days: i64, deleted_hours: i64) -> Vec { + type Row = (Uuid, String, Option, Option, Option); + sqlx::query_as::<_, Row>( + "SELECT id, original_path, preview_path, display_path, thumbnail_path FROM upload + WHERE deleted_at IS NOT NULL + AND CASE WHEN compression_status = 'failed' + THEN deleted_at < NOW() - ($1 || ' days')::interval + ELSE deleted_at < NOW() - ($2 || ' hours')::interval + END + AND (original_path <> '' OR preview_path IS NOT NULL + OR display_path IS NOT NULL OR thumbnail_path IS NOT NULL)", ) - .bind(retention_days.to_string()) + .bind(failed_days.to_string()) + .bind(deleted_hours.to_string()) .fetch_all(pool) .await .expect("sweep query") .into_iter() - .map(|(id, _)| id) + .map(|(id, ..)| id) .collect() } -#[allow(clippy::too_many_arguments)] -async fn seed_upload( +/// Seed an upload aged `deleted_hours_ago` (None = live), with optional derivative paths. +async fn seed_aged_upload( pool: &PgPool, event_id: Uuid, user_id: Uuid, status: &str, - deleted_days_ago: Option, + deleted_hours_ago: Option, original_path: &str, + derivatives: bool, ) -> Uuid { - let id: Uuid = sqlx::query_scalar( + sqlx::query_scalar( "INSERT INTO upload (event_id, user_id, original_path, mime_type, original_size_bytes, - compression_status, deleted_at) + compression_status, deleted_at, + preview_path, display_path, thumbnail_path) VALUES ($1, $2, $3, 'image/jpeg', 1000, $4, CASE WHEN $5::bigint IS NULL THEN NULL - ELSE NOW() - ($5::text || ' days')::interval END) + ELSE NOW() - ($5::text || ' hours')::interval END, + CASE WHEN $6 THEN 'previews/p.jpg' END, + CASE WHEN $6 THEN 'displays/d.jpg' END, + CASE WHEN $6 THEN 'thumbs/t.jpg' END) RETURNING id", ) .bind(event_id) .bind(user_id) .bind(original_path) .bind(status) - .bind(deleted_days_ago) + .bind(deleted_hours_ago) + .bind(derivatives) .fetch_one(pool) .await - .expect("seed upload"); - id + .expect("seed upload") } +/// A live upload is untouchable no matter how the windows are configured. +/// +/// PREVENTS: the catastrophic loosening. Everything else here is about reclaiming more; this is the +/// one assertion that must never bend. #[sqlx::test] -async fn sweeps_only_long_failed_soft_deleted_uploads(pool: PgPool) { - let event_id = seed_event(&pool, "sweep-event").await; +async fn a_live_upload_is_never_selected(pool: PgPool) { + let event_id = seed_event(&pool, "sweep-live").await; let user_id = seed_user(&pool, event_id, "Sweeper").await; - // The one and only thing the sweep may touch: the exact state the compression worker's - // give-up path leaves behind, aged past the window. - let target = seed_upload( - &pool, - event_id, - user_id, - "failed", - Some(30), - "originals/e/target.jpg", - ) - .await; - - // Everything below is a near-miss that must survive. - // A healthy live upload — the catastrophic case if the predicate were ever loosened. - let live = seed_upload( - &pool, - event_id, - user_id, - "done", - None, - "originals/e/live.jpg", - ) - .await; - // Failed but still inside the retention window: the recovery window is the entire point - // of keeping the file, so reclaiming it early would defeat the fix it protects. - let recent = seed_upload( - &pool, - event_id, - user_id, - "failed", - Some(1), - "originals/e/recent.jpg", - ) - .await; - // Failed but NOT soft-deleted — not the give-up path; something else set this status. - let failed_live = seed_upload( - &pool, - event_id, - user_id, - "failed", - None, - "originals/e/failed-live.jpg", - ) - .await; - // Soft-deleted by the OWNER, compression fine. Its file is already gone; this row must - // never be re-processed. - let owner_deleted = seed_upload( - &pool, - event_id, - user_id, - "done", - Some(30), - "originals/e/owner.jpg", - ) - .await; - // Already swept: `original_path` cleared. Re-selecting it every hour would log a - // phantom reclaim forever. - let already_swept = seed_upload(&pool, event_id, user_id, "failed", Some(30), "").await; - - let selected = sweep_selects(&pool, 14).await; - - assert_eq!( - selected, - vec![target], - "the sweep must select exactly the aged give-up-path leftovers" - ); - for (id, what) in [ - (live, "a live upload"), - (recent, "a failure still inside the retention window"), - (failed_live, "a failed but not soft-deleted upload"), - (owner_deleted, "an owner-deleted upload"), - (already_swept, "an already-swept row"), - ] { - assert!(!selected.contains(&id), "the sweep must not touch {what}"); + for status in ["done", "failed", "processing", "pending"] { + let live = seed_aged_upload( + &pool, + event_id, + user_id, + status, + None, + "originals/e/live.jpg", + true, + ) + .await; + assert!( + !sweep_selects(&pool, FAILED_DAYS, DELETED_HOURS) + .await + .contains(&live), + "a non-deleted upload with status {status} must never be swept" + ); } } +/// THE FIX. An upload a guest or host deliberately deleted is reclaimed once past 24 hours. +/// +/// PREVENTS: the regression back to a sweep scoped to `compression_status = 'failed'`, which is +/// what let the quota stop bounding the disk. This assertion is the inverse of the one this file +/// used to make. #[sqlx::test] -async fn retention_window_is_honoured_at_the_boundary(pool: PgPool) { - let event_id = seed_event(&pool, "sweep-boundary").await; - let user_id = seed_user(&pool, event_id, "Boundary").await; +async fn a_deliberately_deleted_upload_is_reclaimed_after_a_day(pool: PgPool) { + let event_id = seed_event(&pool, "sweep-deleted").await; + let user_id = seed_user(&pool, event_id, "Curator").await; - let inside = seed_upload( + let deleted = seed_aged_upload( &pool, event_id, user_id, - "failed", - Some(13), - "originals/e/inside.jpg", + "done", + Some(48), + "originals/e/owner.jpg", + true, ) .await; - let outside = seed_upload( + // Still inside the window — a mis-tap is recoverable for a day. + let recent = seed_aged_upload( &pool, event_id, user_id, - "failed", - Some(15), - "originals/e/outside.jpg", + "done", + Some(2), + "originals/e/recent.jpg", + true, ) .await; - let selected = sweep_selects(&pool, 14).await; + let selected = sweep_selects(&pool, FAILED_DAYS, DELETED_HOURS).await; assert!( - selected.contains(&outside), - "15 days old must be past a 14-day window" + selected.contains(&deleted), + "a deliberate delete past the window must be reclaimed — this is the leak" ); assert!( - !selected.contains(&inside), - "13 days old must still be retained" + !selected.contains(&recent), + "a delete inside the window keeps its recovery grace" ); } +/// The two windows are independent: a failure is retained far longer than a deliberate delete. +/// +/// PREVENTS: collapsing them into one. Applying 24h to failures would destroy the recovery window +/// the retained-original fix exists to provide; applying 14 days to deliberate deletes would mean +/// nothing is ever reclaimed during an event. #[sqlx::test] -async fn clearing_original_path_makes_the_sweep_idempotent(pool: PgPool) { - // The sweep clears `original_path` after reclaiming the file. Without that, a row whose - // file is already gone is re-selected on every hourly tick forever. - let event_id = seed_event(&pool, "sweep-idempotent").await; - let user_id = seed_user(&pool, event_id, "Idem").await; - let id = seed_upload( +async fn the_two_retention_windows_do_not_bleed_into_each_other(pool: PgPool) { + let event_id = seed_event(&pool, "sweep-windows").await; + let user_id = seed_user(&pool, event_id, "Windows").await; + + // 48h old: past the deliberate window, nowhere near the failure window. + let failed_recent = seed_aged_upload( &pool, event_id, user_id, "failed", - Some(30), - "originals/e/once.jpg", + Some(48), + "originals/e/f-recent.jpg", + false, + ) + .await; + let deleted_same_age = seed_aged_upload( + &pool, + event_id, + user_id, + "done", + Some(48), + "originals/e/d-same.jpg", + false, + ) + .await; + // 30 days old: past both. + let failed_old = seed_aged_upload( + &pool, + event_id, + user_id, + "failed", + Some(30 * 24), + "originals/e/f-old.jpg", + false, ) .await; - assert_eq!(sweep_selects(&pool, 14).await, vec![id]); + let selected = sweep_selects(&pool, FAILED_DAYS, DELETED_HOURS).await; + assert!( + !selected.contains(&failed_recent), + "a 2-day-old compression failure is still inside its 14-day recovery window" + ); + assert!( + selected.contains(&deleted_same_age), + "a deliberate delete of the same age is past its 24-hour window" + ); + assert!( + selected.contains(&failed_old), + "a 30-day-old failure is past both windows" + ); +} +/// Boundary behaviour on both windows. +#[sqlx::test] +async fn retention_windows_are_honoured_at_the_boundary(pool: PgPool) { + let event_id = seed_event(&pool, "sweep-boundary").await; + let user_id = seed_user(&pool, event_id, "Boundary").await; + + let cases = [ + ("failed", 13 * 24, false, "13 days"), + ("failed", 15 * 24, true, "15 days"), + ("done", 23, false, "23 hours"), + ("done", 25, true, "25 hours"), + ]; + for (status, hours, expected, label) in cases { + let id = seed_aged_upload( + &pool, + event_id, + user_id, + status, + Some(hours), + "originals/e/b.jpg", + false, + ) + .await; + assert_eq!( + sweep_selects(&pool, FAILED_DAYS, DELETED_HOURS) + .await + .contains(&id), + expected, + "a {status} upload deleted {label} ago: expected swept={expected}" + ); + sqlx::query("DELETE FROM upload WHERE id = $1") + .bind(id) + .execute(&pool) + .await + .expect("clean up"); + } +} + +/// A row is re-selected until EVERY one of its paths is cleared. +/// +/// PREVENTS: two failures at once. The sweep used to clear `original_path` alone, which was right +/// for its only case (a failed compression produces no derivatives) but leaves preview, display and +/// thumbnail on disk the moment it reaches a successfully processed upload — three files per +/// upload, none of them counted in `original_size_bytes`, that nothing else ever removes. And a row +/// whose paths are all cleared must stop coming back, or every hourly tick logs a phantom reclaim +/// forever. +#[sqlx::test] +async fn a_row_is_reselected_until_every_path_is_cleared(pool: PgPool) { + let event_id = seed_event(&pool, "sweep-idempotent").await; + let user_id = seed_user(&pool, event_id, "Idem").await; + let id = seed_aged_upload( + &pool, + event_id, + user_id, + "done", + Some(48), + "originals/e/once.jpg", + true, + ) + .await; + + assert_eq!(sweep_selects(&pool, FAILED_DAYS, DELETED_HOURS).await, [id]); + + // Clearing only the original is NOT enough — the derivatives are still on disk. sqlx::query("UPDATE upload SET original_path = '' WHERE id = $1") .bind(id) .execute(&pool) .await - .expect("clear path"); + .expect("clear original"); + assert_eq!( + sweep_selects(&pool, FAILED_DAYS, DELETED_HOURS).await, + [id], + "derivatives left behind must keep the row selected" + ); + sqlx::query( + "UPDATE upload SET preview_path = NULL, display_path = NULL, thumbnail_path = NULL + WHERE id = $1", + ) + .bind(id) + .execute(&pool) + .await + .expect("clear derivatives"); assert!( - sweep_selects(&pool, 14).await.is_empty(), - "a swept row must not come back" + sweep_selects(&pool, FAILED_DAYS, DELETED_HOURS) + .await + .is_empty(), + "a fully swept row must not come back" + ); +} + +/// The derivative backfill must never resurrect what the sweep just reclaimed. +/// +/// PREVENTS: an interaction, not a bug in either piece. The sweep nulls `preview_path`, and +/// `backfill_stale_derivatives` selects on `display_path IS NULL AND preview_path IS NOT NULL` — +/// close enough that a future edit to either could have the backfill re-decode an original that is +/// no longer on disk, on every boot. `deleted_at IS NULL` is what keeps them apart. +#[sqlx::test] +async fn the_backfill_ignores_swept_rows(pool: PgPool) { + let event_id = seed_event(&pool, "sweep-backfill").await; + let user_id = seed_user(&pool, event_id, "Backfill").await; + seed_aged_upload( + &pool, + event_id, + user_id, + "done", + Some(48), + "originals/e/gone.jpg", + true, + ) + .await; + + // SRC: `services/compression.rs::backfill_stale_derivatives` — the selection, verbatim. + let backfilled: Vec<(Uuid, String, String)> = sqlx::query_as( + "SELECT id, original_path, mime_type FROM upload + WHERE deleted_at IS NULL AND mime_type LIKE 'image/%' + AND original_path IS NOT NULL + AND ( + (display_path IS NULL AND preview_path IS NOT NULL) + OR derivatives_rev < $1 + )", + ) + .bind(1i16) + .fetch_all(&pool) + .await + .expect("backfill query"); + + assert!( + backfilled.is_empty(), + "a soft-deleted row must be invisible to the backfill, before or after sweeping" ); }