test(storage): cover the sidecar walk for both import jobs

Both imports run over ONE directory, where the two legacy shapes sit
side by side, so the property worth asserting spans them: together they
must claim every real sidecar exactly once, and neither may take the
other's. A job that drifted into the other's shape would content-key
user-supplied bytes — sharing one user's uploaded preview onto every
file with identical content — and no per-job test in isolation would
notice.

So the fixture is shared. `legacy_tree` builds a directory holding a
content-keyed .webp pair, an ext- upload, and a stray README, and both
test modules walk it: derived claims exactly the two hashes in sorted
order, attached claims exactly the ext- file, the two sets are disjoint,
and between them they account for all three real sidecars.

`sidecar_names` became an associated function taking the root instead of
reading `self`, which is what makes this testable at all — the walk is
the half that decides which files a job claims, and it needed no pool to
verify. Sorting is asserted rather than assumed, since the cursor
resumes by skipping everything at or before it and a stable order is the
only thing that makes that correct.

A missing size directory is covered too: normal on a fresh install, and
it must yield no work rather than abort the walk.

Not covered here, and it needs a pooled fixture that does not exist: the
round trip itself — store the blob, write the row, and confirm a COPY
inherits the preview. That belongs in the API-level harness, where the
legacy state can be manufactured through the real write path and then
stripped.
This commit is contained in:
Edouard Vanbelle
2026-08-26 00:39:17 +02:00
parent 7a2ebe0fdc
commit 49e7bb15a6
2 changed files with 131 additions and 9 deletions
@@ -106,8 +106,12 @@ impl ThumbAttachedImport {
/// ///
/// Sorted because the cursor resumes by skipping everything at or before /// Sorted because the cursor resumes by skipping everything at or before
/// it, which only works over a stable order. /// it, which only works over a stable order.
async fn sidecar_names(&self, size: ThumbnailSize) -> Vec<String> { ///
let dir = self.thumbnails_root.join(size.dir_name()); /// Takes the root rather than reading `self`, so the walk — the half that
/// decides which files this job claims, and therefore which keying they
/// get — is testable against a temp directory with no database in sight.
async fn sidecar_names(root: &std::path::Path, size: ThumbnailSize) -> Vec<String> {
let dir = root.join(size.dir_name());
let Ok(mut entries) = fs::read_dir(&dir).await else { let Ok(mut entries) = fs::read_dir(&dir).await else {
return Vec::new(); return Vec::new();
}; };
@@ -146,7 +150,9 @@ impl RecoverableJobHandler for ThumbAttachedImport {
async fn count_total(&self) -> Option<u64> { async fn count_total(&self) -> Option<u64> {
let mut total = 0u64; let mut total = 0u64;
for size in ThumbnailSize::all() { for size in ThumbnailSize::all() {
total += self.sidecar_names(*size).await.len() as u64; total += Self::sidecar_names(&self.thumbnails_root, *size)
.await
.len() as u64;
} }
Some(total) Some(total)
} }
@@ -181,7 +187,7 @@ impl RecoverableJobHandler for ThumbAttachedImport {
for size in ThumbnailSize::all() { for size in ThumbnailSize::all() {
let dir_name = size.dir_name().to_string(); let dir_name = size.dir_name().to_string();
for name in self.sidecar_names(*size).await { for name in Self::sidecar_names(&self.thumbnails_root, *size).await {
let position = format!("{dir_name}/{name}"); let position = format!("{dir_name}/{name}");
if let Some(c) = &cursor if let Some(c) = &cursor
@@ -341,6 +347,43 @@ mod tests {
); );
} }
/// The other half of the partition. Reuses the same legacy tree as
/// `thumb_derived_import`'s test on purpose: the two jobs run over one
/// directory, so the property that matters is that together they claim
/// every real sidecar exactly once, and neither takes the other's.
#[tokio::test]
async fn walk_claims_only_uploaded_previews() {
use crate::infrastructure::services::thumb_derived_import_service::ThumbDerivedImport;
let tmp =
crate::infrastructure::services::thumb_derived_import_service::tests::legacy_tree()
.await;
let attached = ThumbAttachedImport::sidecar_names(tmp.path(), ThumbnailSize::Preview).await;
let derived = ThumbDerivedImport::sidecar_names(tmp.path(), ThumbnailSize::Preview).await;
assert_eq!(
attached,
vec!["ext-3f2b1c00-1111-2222-3333-444455556666.jpg".to_string()],
"must claim the uploaded preview and nothing else"
);
// Disjoint: no file is imported under both keyings, which would take
// two references and — worse — content-key user-supplied bytes.
for a in &attached {
assert!(
!derived.contains(a),
"both jobs claimed {a}; keying would be ambiguous"
);
}
// And nothing real is dropped: README.txt is the only unclaimed file.
assert_eq!(
attached.len() + derived.len(),
3,
"the three real sidecars must be claimed exactly once between them"
);
}
/// The content-keyed sidecars belong to `thumb_derived_import`. Importing /// The content-keyed sidecars belong to `thumb_derived_import`. Importing
/// one here would file-key bytes that are shared across every file with /// one here would file-key bytes that are shared across every file with
/// the same content, so each such file would take its own reference to /// the same content, so each such file would take its own reference to
@@ -93,8 +93,12 @@ impl ThumbDerivedImport {
/// ///
/// Sorted so the cursor is meaningful: resume skips everything at or /// Sorted so the cursor is meaningful: resume skips everything at or
/// before it, which only works over a stable order. /// before it, which only works over a stable order.
async fn sidecar_names(&self, size: ThumbnailSize) -> Vec<String> { ///
let dir = self.thumbnails_root.join(size.dir_name()); /// Takes the root rather than reading `self`, so the walk — the half that
/// decides which files this job claims, and therefore which keying they
/// get — is testable against a temp directory with no database in sight.
pub(crate) async fn sidecar_names(root: &std::path::Path, size: ThumbnailSize) -> Vec<String> {
let dir = root.join(size.dir_name());
let Ok(mut entries) = fs::read_dir(&dir).await else { let Ok(mut entries) = fs::read_dir(&dir).await else {
return Vec::new(); return Vec::new();
}; };
@@ -120,7 +124,9 @@ impl RecoverableJobHandler for ThumbDerivedImport {
async fn count_total(&self) -> Option<u64> { async fn count_total(&self) -> Option<u64> {
let mut total = 0u64; let mut total = 0u64;
for size in ThumbnailSize::all() { for size in ThumbnailSize::all() {
total += self.sidecar_names(*size).await.len() as u64; total += Self::sidecar_names(&self.thumbnails_root, *size)
.await
.len() as u64;
} }
Some(total) Some(total)
} }
@@ -155,7 +161,7 @@ impl RecoverableJobHandler for ThumbDerivedImport {
for size in ThumbnailSize::all() { for size in ThumbnailSize::all() {
let dir_name = variant_of(*size); let dir_name = variant_of(*size);
for name in self.sidecar_names(*size).await { for name in Self::sidecar_names(&self.thumbnails_root, *size).await {
let position = format!("{dir_name}/{name}"); let position = format!("{dir_name}/{name}");
// Resume: everything at or before the cursor is done. // Resume: everything at or before the cursor is done.
@@ -277,7 +283,11 @@ impl RecoverableJobHandler for ThumbDerivedImport {
} }
#[cfg(test)] #[cfg(test)]
mod tests { // `pub(crate)` so the attached import's test can reuse `legacy_tree`. Both
// jobs walk ONE directory, so the property worth asserting spans them — that
// together they claim every sidecar exactly once — and that needs a shared
// fixture rather than two that can drift apart.
pub(crate) mod tests {
use super::*; use super::*;
const H: &str = "0a1b2c3d4e5f60718293a4b5c6d7e8f90a1b2c3d4e5f60718293a4b5c6d7e8f9"; const H: &str = "0a1b2c3d4e5f60718293a4b5c6d7e8f90a1b2c3d4e5f60718293a4b5c6d7e8f9";
@@ -290,6 +300,75 @@ mod tests {
); );
} }
/// A legacy `.thumbnails` tree as it exists before the migration: both
/// sidecar shapes side by side in the same size directory, which is
/// exactly how they are written today.
///
/// Returns the temp dir — the caller must hold it, or the directory is
/// removed while the test is still reading it.
pub(crate) async fn legacy_tree() -> tempfile::TempDir {
let tmp = tempfile::tempdir().expect("create temp dir");
for size in ThumbnailSize::all() {
let dir = tmp.path().join(size.dir_name());
tokio::fs::create_dir_all(&dir).await.unwrap();
// Server-rendered, content-keyed. `b` sorts after `0a…`, so the
// pair also proves the listing is ordered rather than incidental.
tokio::fs::write(dir.join(format!("{H}.webp")), b"webp")
.await
.unwrap();
tokio::fs::write(
dir.join("b111111111111111111111111111111111111111111111111111111111111111.webp"),
b"webp2",
)
.await
.unwrap();
// User-uploaded, file-keyed.
tokio::fs::write(
dir.join("ext-3f2b1c00-1111-2222-3333-444455556666.jpg"),
b"jpeg",
)
.await
.unwrap();
// Neither: a stray file that must be claimed by no one.
tokio::fs::write(dir.join("README.txt"), b"nope")
.await
.unwrap();
}
tmp
}
/// The migration's core invariant: this job claims the content-keyed
/// sidecars and *only* those, leaving the uploaded previews for
/// `thumb_attached_import`. Getting this wrong content-keys user-supplied
/// bytes, which shares one user's preview onto every file with identical
/// content.
#[tokio::test]
async fn walk_claims_only_content_keyed_sidecars_in_sorted_order() {
let tmp = legacy_tree().await;
let names = ThumbDerivedImport::sidecar_names(tmp.path(), ThumbnailSize::Preview).await;
assert_eq!(
names,
vec![
format!("{H}.webp"),
"b111111111111111111111111111111111111111111111111111111111111111.webp".to_string(),
],
"must claim both content-keyed sidecars, sorted, and nothing else"
);
}
/// A missing size directory is normal on a fresh install and must not
/// abort the walk — the job simply has nothing to import.
#[tokio::test]
async fn missing_size_directory_yields_no_work() {
let tmp = tempfile::tempdir().expect("create temp dir");
assert!(
ThumbDerivedImport::sidecar_names(tmp.path(), ThumbnailSize::Icon)
.await
.is_empty()
);
}
/// `ext-` files are user-supplied and file-keyed. Importing one here /// `ext-` files are user-supplied and file-keyed. Importing one here
/// would content-key it and share it across every file with identical /// would content-key it and share it across every file with identical
/// content — the exact poisoning the table split prevents. /// content — the exact poisoning the table split prevents.