From 06bc9ddcb3f9e5b70d3ca4bfaa0234db1a34ea45 Mon Sep 17 00:00:00 2001 From: fabi Date: Wed, 29 Jul 2026 20:50:04 +0200 Subject: [PATCH] refactor(export): share the visibility filter between the row query and the estimate `query_uploads` selects the rows the archives are built from; `estimate_export_bytes` sizes them for the disk preflight. They stated the same WHERE clause separately, and the direction of drift matters: an estimate that MISSES rows the archive writes under-reserves, which is precisely the ENOSPC the preflight exists to prevent. The integration test claimed to guard this and cannot. Both sides of `the_estimate_sums_exactly_the_rows_the_archive_will_contain` are `SRC:`-marked hand-copies in tests/common/mod.rs -- neither is production code -- so drift means production moved while both copies sat still, and the test goes on passing. The convention is sound for pinning behaviour; it is structurally incapable of detecting divergence from the thing it copies. So fix it where it can be fixed. One `export_visibility_where!()` fragment, `concat!`-ed into both queries at compile time (still `&'static str`, no allocation), with the `u`/`usr` alias contract stated. Divergence is now impossible by construction rather than watched for. The tests keep their value and lose the overclaim: the docstrings now say they pin WHICH uploads may be counted -- each excluded row in the fixture is excluded by a different predicate, so weakening any one of them still fails here -- and say plainly that they do not detect drift, with a pointer to what does. No behaviour change. The filters were verified identical before the hoist (`u.event_id = $1 AND u.deleted_at IS NULL AND usr.uploads_hidden = FALSE AND usr.is_banned = FALSE`); 99 backend tests still pass. Co-Authored-By: Claude Opus 5 --- backend/src/services/export.rs | 43 ++++++++++++++++++++++++------- backend/tests/common/mod.rs | 8 +++++- backend/tests/export_preflight.rs | 23 ++++++++++++----- 3 files changed, 57 insertions(+), 17 deletions(-) diff --git a/backend/src/services/export.rs b/backend/src/services/export.rs index 4cac9ab..f754c80 100644 --- a/backend/src/services/export.rs +++ b/backend/src/services/export.rs @@ -20,6 +20,29 @@ use crate::state::SseEvent; static VIEWER_DIR: Dir<'_> = include_dir!("$CARGO_MANIFEST_DIR/static/export-viewer"); +// ── Shared visibility filter ───────────────────────────────────────────────── + +/// The predicate that decides what lands in a keepsake, as ONE definition. +/// +/// Two queries have to agree on it: [`query_uploads`], which selects the rows the archives are +/// built from, and [`estimate_export_bytes`], which sizes them for the disk preflight. They used +/// to state it separately, and the direction of drift matters — an estimate that misses rows the +/// archive writes UNDER-reserves, which is the exact ENOSPC the preflight exists to prevent. +/// +/// A `SRC:`-marked copy in the integration tests cannot catch that: drift means production moved +/// and the copy didn't, so both sides of such a test sit still and it keeps passing. Sharing the +/// fragment removes the failure by construction instead, and leaves the test doing what it is +/// actually good at — pinning the behaviour. +/// +/// CONTRACT: callers must alias `upload` as `u` and join `"user"` as `usr`, and bind the event id +/// as `$1`. +macro_rules! export_visibility_where { + () => { + "WHERE u.event_id = $1 AND u.deleted_at IS NULL + AND usr.uploads_hidden = FALSE AND usr.is_banned = FALSE" + }; +} + // ── DB query rows ──────────────────────────────────────────────────────────── #[derive(sqlx::FromRow)] @@ -1051,7 +1074,7 @@ async fn run_html_export_inner( // ── DB helpers ─────────────────────────────────────────────────────────────── async fn query_uploads(pool: &PgPool, event_id: Uuid) -> Result> { - Ok(sqlx::query_as::<_, ExportUploadRow>( + Ok(sqlx::query_as::<_, ExportUploadRow>(concat!( "SELECT u.id, u.original_path, u.mime_type, u.caption, usr.display_name AS uploader_name, COUNT(DISTINCT l.user_id) AS like_count, @@ -1059,11 +1082,12 @@ async fn query_uploads(pool: &PgPool, event_id: Uuid) -> Result