fix(import): reject an automatic import onto an occupied destination instead of throwing - #216
Open
jordanfelle wants to merge 6 commits into
Open
jordanfelle wants to merge 6 commits into
jordanfelle wants to merge 6 commits into
Conversation
A book group's files are imported one at a time, but the "which existing files does this replace" query is re-run from the database for every file in the group, and the old rows are not removed until the whole group is committed. When the new release writes its parts to the same destination paths as the old release (the normal case for a multi-part audiobook), the second file of the group saw the first file's freshly written destination as an old file to displace, renamed it to a .chaptarr-upgrade~ backup, and FinalizePendingFileCommits then recycled it. For a 173-part set only the last part survived. Track the destination paths written and the rows displaced by the batch in progress, exclude those from replacement selection, and never stage a path the same batch already imported into.
A file whose destination path was already tracked took one of two paths in ResolveBookFilePathConflict: queue that row for the group's atomic swap, or adopt it in place. Both lost the row when another file of the same batch had already queued it for replacement, because the group-level ReplaceMany then deleted the very row the adoption had just repointed at the new file. The file stayed on disk with no BookFiles row, and history recorded an 'Upgrade' deletion for a path the batch had just imported. Always reuse the row found at the destination, keep the batch's adopted row ids out of GetDatabaseRowsToReplace, and keep the pending commit registered so its staged backups are still recycled and are never rolled back once the row is persisted.
…ing files ImportApprovedBooks.ImportFile built filesToReplace from every existing file of the matched edition and, for an automatic import (replaceExisting: true from DownloadedBooksImportService), staged, replaced and recycled them whenever the new copy was not a strictly lower *revision* (ImportApprovedBooks.cs:1104-1109). A second copy of an already-imported release therefore churned the library: on the live host 2,525 editions were re-imported in the same format as "Upgrade". An automatic import (not IsManualImport, not DownloadForced) now only displaces existing files when the shared upgrade rule says the release is a real upgrade for the edition's quality profile: IUpgradableSpecification.IsUpgradable plus IsUpgradeAllowed, which is the same profile-preference comparison (quality rank, revision, custom-format score, UpgradeAllowed) that search, queue and history use. Equal or worse against any file it would replace is reported as "Already imported: this edition already has files of equal or better quality" and nothing is moved, deleted or recycled. Rows whose file is missing on disk are excluded, so stale/aborted-import rows and the converted-destination reuse path are unaffected. Manual imports and forced grabs keep their existing explicit-replace behaviour untouched. The rejection surfaces through the normal "nothing imported" path: the tracked download ends up importBlocked with that message. No queue-completion mechanism was added - marking it Imported would fabricate history rows, and there is no established "recognised duplicate, leave the queue" state in this codebase.
jordanfelle
added a commit
to jordanfelle/chaptarr
that referenced
this pull request
Sep 26, 2026
…ready-imported duplicate The equal-quality guard compared each incoming file with the edition's surviving files only. An edition that lost parts (its rows still declare PartCount N but fewer than N files remain, as after the same-path upgrade deletion) was therefore rejected as 'already has files of equal or better quality' when the complete set arrived, so the automatic import could no longer repair it. When the surviving rows declare more parts than are present, let the import proceed and replace the partial set. Also adds a regression test that a multi-file upgrade parses each stored file's custom formats once per batch.
…rowing An automatic import whose computed destination path already held a file that filesToReplace did not cover reached CopyBookFile/MoveBookFile, where ClearTargetPath throws DestinationAlreadyExistsException and the queue item ends as IMPORT_EXCEPTION. filesToReplace only covers rows of the same edition at a different path, so a destination occupied by an untracked file or by a row tracked for another edition of the same book was never caught. ImportFile now checks the computed destination before any staging or transfer, for automatic imports only: a row tracked for this edition gives the existing AlreadyImportedRejectionReason, a row for another edition or an untracked file gives a clear rejection naming the destination. Nothing is moved or deleted. Manual imports, forced downloads, relocations and genuine upgrades (whose destination row is in filesToReplace) are unaffected.
jordanfelle
force-pushed
the
fix-auto-import-occupied-destination
branch
from
September 26, 2026 21:52
42d65b0 to
7dd17ca
Compare
jordanfelle
added a commit
to jordanfelle/chaptarr
that referenced
this pull request
Sep 26, 2026
…not exist DiskProvider.FileExists on a missing path falls back to enumerating each directory along the path for a case/normalisation match. The occupied-destination pre-check ran that on every automatic import, including the ordinary case of a new book whose folder does not exist yet, where nothing can occupy the destination. Check the parent folder first and only probe the file when it exists; the transfer keeps its own check. Adds a regression test that records FileExists probes.
jordanfelle
added a commit
to jordanfelle/chaptarr
that referenced
this pull request
Sep 26, 2026
jordanfelle
added a commit
to jordanfelle/chaptarr
that referenced
this pull request
Sep 27, 2026
jordanfelle
added a commit
to jordanfelle/chaptarr
that referenced
this pull request
Sep 27, 2026
jordanfelle
added a commit
to jordanfelle/chaptarr
that referenced
this pull request
Sep 27, 2026
jordanfelle
added a commit
to jordanfelle/chaptarr
that referenced
this pull request
Sep 27, 2026
jordanfelle
added a commit
to jordanfelle/chaptarr
that referenced
this pull request
Sep 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #200 and on the equal-quality guard PR (
fix-equal-quality-duplicate-guard); stacked on that branch.Problem
An automatic import whose computed destination path already held a file that
filesToReplacedid not cover reachedCopyBookFile/MoveBookFile, whereClearTargetPaththrowsDestinationAlreadyExistsExceptionand the queue item ends asIMPORT_EXCEPTION(about 2,700DestinationAlreadyExistserrors logged in three days on a live instance).filesToReplaceonly covers rows of the same edition at a different path, so a destination occupied by an untracked file or by a row tracked for another edition of the same book was never caught.Fix
ImportFilechecks the computed destination before any staging or transfer, for automatic imports only: a row tracked for this edition gives the existing already-imported rejection, a row for another edition or an untracked file gives a clear rejection naming the destination. Nothing is moved or deleted. Manual imports, forced downloads, relocations and genuine upgrades are unaffected.Verification
ImportApprovedBooksAdditionalCopyFixture.IMPORT_EXCEPTION.The destination check is skipped when the destination folder does not exist (nothing can occupy it), so ordinary imports do not pay for
FileExists' path-resolution fallback, which lists every directory along a missing path. The transfer keeps its own check.automatic_import_into_a_folder_that_does_not_exist_should_not_probe_the_destination_file(recordsFileExistscalls; failed with 1 probe, passes with 0) (3,038 core tests pass).