feat(storage): thumb_attached_import drains its sidecars too

Completes step 10d. Same `?repair=true` opt-in and the same
readback-before-unlink as the derived half, and the check matters more
here: these sidecars hold the bytes that CANNOT be regenerated — a
client-uploaded PDF preview has no server-side render path — so the read
is the only thing between a migration and permanent loss, not
belt-and-braces.

verify_and_unlink is shared rather than copied. Two versions of "only
delete after proving the replacement is readable" would be two chances
to weaken one, and it is the rule the whole deletion step rests on.

Deletion covers the already-imported branch as well as fresh imports,
for the same reason as the derived job: a run without `repair` leaves
the sidecar behind, and a later run with it would otherwise see "already
imported" and never drain. Import first, enable deletion after, is the
expected operator sequence, so that branch is the common path.

Orphaned sidecars stay untouched — this job imports, it does not
reclaim, and a destructive default on a migration is what no-silent-
auto-repair forbids. Unverifiable ones are kept and reported, so the
next run retries.

With both halves draining, `.thumbnails/` can now actually empty and the
directory removal in the derived job can succeed — though not stably
until dual-write stops, since any render or upload recreates it.
This commit is contained in:
Edouard Vanbelle
2026-08-27 23:31:31 +02:00
parent ae9ff0d8b3
commit df619a7ed9
2 changed files with 97 additions and 9 deletions
@@ -123,11 +123,18 @@ impl ThumbDerivedImport {
/// Returns whether the file was removed. A failed verification leaves the
/// sidecar in place — the run reports it and the next one retries, which
/// is the safe direction.
async fn verify_and_unlink(&self, derived_hash: &str, path: &std::path::Path) -> bool {
/// Shared with `thumb_attached_import` rather than copied into it: both
/// jobs delete a sidecar only after proving its replacement is readable,
/// and two copies of that rule would be two chances to weaken one.
pub(crate) async fn verify_and_unlink(
dedup: &DedupService,
stored_hash: &str,
path: &std::path::Path,
) -> bool {
let Ok(meta) = fs::metadata(path).await else {
return false;
};
let Ok(stored) = self.dedup.read_blob_bytes(derived_hash).await else {
let Ok(stored) = dedup.read_blob_bytes(stored_hash).await else {
return false;
};
if stored.is_empty() || stored.len() as u64 != meta.len() {
@@ -277,7 +284,7 @@ impl RecoverableJobHandler for ThumbDerivedImport {
already += 1;
if delete_imported {
let path = self.thumbnails_root.join(dir_name).join(&name);
if self.verify_and_unlink(&existing.blob_hash, &path).await {
if Self::verify_and_unlink(&self.dedup, &existing.blob_hash, &path).await {
deleted += 1;
} else {
unverified += 1;
@@ -314,7 +321,13 @@ impl RecoverableJobHandler for ThumbDerivedImport {
Ok(derived_hash) => {
imported += 1;
if delete_imported {
if self.verify_and_unlink(&derived_hash, &path).await {
if Self::verify_and_unlink(
&self.dedup,
&derived_hash,
&path,
)
.await
{
deleted += 1;
} else {
unverified += 1;