Batch track removal that guarantees the database matches the disk #15

Merged
wokoman merged 1 commit from feat/batch-removal into main 2026-08-28 20:02:40 +02:00
Owner

A single batch-removal entry point that owns the rule every caller currently re-implements: the database on disk always matches the audio files on disk.

Nothing changes for users yet. The single-track removal stays public and every caller stays on it, so the tree stays green; the migration is #5.

Built to the interface settled in #4, with one deviation, which needs a look before this merges.

failed_id is Option<u32>, not u32. The pinned shape has no honest value for it when what fails is the database write that follows the removals — every removal succeeded, so no track's id is the answer, and picking the last one would name a track that is fine. None means "the write failed", which is a state a caller may well want to tell apart: the tracks are gone from disk, and the database on disk still lists them. Reversible either way, but it is a change to a shape the ticket asked to pin precisely, so say if you would rather have something else there.

Everything else is as specified. Stop at the first failed removal, deliberately unlike the copy direction — a failed copy is per-file, a failed delete is usually systemic and pressing on turns one stranded file into two hundred. That divergence is now documented on both sides, since on one side alone it reads like an accident. Write only when at least one removal succeeded, with the reasoning in a comment so it does not get deleted as dead weight. The callback fires before each removal, because the caller with a screen is single-threaded and has to repaint before it blocks. RemoveFailed gets a hand-written Display/Error, as anyhow::Error cannot be a derived source. Rustdoc has the # Errors section covering the stopping rule, the write guarantee, and that a write renumbers track IDs so held ids are meaningless afterwards.

Three tests, all with no device attached, one per acceptance criterion:

  • A whole batch returns its count, writes once, and the callback is asserted to have fired as (0,2) then (1,2) — before each removal, not after.
  • A failure partway stops there: three tracks, failing on the second, and the third is untouched in both the database and on disk. The reopened database has exactly the track that was never reached.
  • A batch whose first removal fails writes nothing, so the reopened database still lists both tracks — and both files are still there, so it is telling the truth.

Forcing the failure needs no permissions or root: putting a non-empty directory where the audio file was makes unlink refuse, on macOS and on Linux alike. The middle test also has to re-read every id after its setup write, which is the ID-renumbering rule the rustdoc warns about, demonstrated.

64 unit tests pass, cargo clippy --all-targets clean.

Refs #4.

A single batch-removal entry point that owns the rule every caller currently re-implements: the database on disk always matches the audio files on disk. Nothing changes for users yet. The single-track removal stays public and every caller stays on it, so the tree stays green; the migration is #5. Built to the interface settled in #4, with **one deviation, which needs a look before this merges**. `failed_id` is `Option<u32>`, not `u32`. The pinned shape has no honest value for it when what fails is the database *write* that follows the removals — every removal succeeded, so no track's id is the answer, and picking the last one would name a track that is fine. `None` means "the write failed", which is a state a caller may well want to tell apart: the tracks are gone from disk, and the database on disk still lists them. Reversible either way, but it is a change to a shape the ticket asked to pin precisely, so say if you would rather have something else there. Everything else is as specified. Stop at the first failed removal, deliberately unlike the copy direction — a failed copy is per-file, a failed delete is usually systemic and pressing on turns one stranded file into two hundred. That divergence is now documented on **both** sides, since on one side alone it reads like an accident. Write only when at least one removal succeeded, with the reasoning in a comment so it does not get deleted as dead weight. The callback fires *before* each removal, because the caller with a screen is single-threaded and has to repaint before it blocks. `RemoveFailed` gets a hand-written `Display`/`Error`, as `anyhow::Error` cannot be a derived source. Rustdoc has the `# Errors` section covering the stopping rule, the write guarantee, and that a write renumbers track IDs so held ids are meaningless afterwards. Three tests, all with no device attached, one per acceptance criterion: - A whole batch returns its count, writes once, and the callback is asserted to have fired as `(0,2)` then `(1,2)` — before each removal, not after. - A failure partway stops there: three tracks, failing on the second, and the third is untouched in both the database and on disk. The reopened database has exactly the track that was never reached. - A batch whose first removal fails writes nothing, so the reopened database still lists both tracks — and both files are still there, so it is telling the truth. Forcing the failure needs no permissions or root: putting a non-empty directory where the audio file was makes unlink refuse, on macOS and on Linux alike. The middle test also has to re-read every id after its setup write, which is the ID-renumbering rule the rustdoc warns about, demonstrated. 64 unit tests pass, `cargo clippy --all-targets` clean. Refs #4.
Three call sites each guess at the failure behaviour of "loop, then write
once", and one guesses wrong. Making the guarantee a property of the
module is what stops that recurring; the callers move over in #5.

Refs #4.
wokoman deleted branch feat/batch-removal 2026-08-28 20:02:40 +02:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
wokoman/ironpod!15
No description provided.