Skip to content

Fix data loss when importing a book matched to the wrong file. - #185

Open
nightah wants to merge 4 commits into
pennydreadful:developfrom
nightah:fix/identifier-validation-and-import-delete-guard
Open

nightah wants to merge 4 commits into
pennydreadful:developfrom
nightah:fix/identifier-validation-and-import-delete-guard

Conversation

@nightah

@nightah nightah commented Sep 21, 2026

Copy link
Copy Markdown

Database Migration

NO

Description

Fixes a data-loss path: importing a book can permanently delete an unrelated book's file. Reported in #173, reproduced here with debug/trace logs, and fixed in four independent commits (each stands alone, split them if you prefer).

What happens. A file whose tags carry an unusable identifier is matched to the wrong book, and the next import for that book deletes it as an "upgrade":

bookFileImported bookId=3 | Ana Huang - Twisted Games
bookFileDeleted  bookId=3 | /books/Ana Huang/Twisted Lies/Twisted Lies - Ana Huang.azw3 | reason=Upgrade

With no Recycle Bin configured RecycleBinProvider deletes permanently, which is the loss reported in #173.

Why the file was mis-matched. Calibre writes its internal UUID into the MOBI/AZW3 EXTH ASIN record (113). DistanceCalculator reads it as an ASIN and compares it to the edition ASIN at weight 10.0, while a wrong title costs only 3.0 and a missing identifier 0.1. The correct book therefore scores worse than a different book by the same author that has no ASIN at all:

[30913880][Twisted Lies]   book: exact match      asin: UUID vs B09TVV9NH2 -> 0.3855
[30704937][Twisted Games]  book: MISMATCH         asin: UUID vs ''         -> 0.0644   <- chosen

8 of 24 Kindle-format files in the library I reproduced this on carry such a UUID, produced by five different Calibre versions, so this is not a one-off.

Commits

  1. fix(identification): ignore malformed ASIN/ISBN values from file tags - validate the identifier from file tags before using it; a malformed value is treated like a missing one.
  2. fix(identification): treat a differing ISBN/ASIN as unknown, not as a mismatch - editions of the same work legitimately carry different identifiers, and the metadata source often holds one the file does not. Reward a match rather than punish a mismatch, so an identifier can promote the right edition but can no longer select a different book.
  3. fix(import): never delete an existing file outside the destination folder - the actual data-loss guard. UpgradeBookFile deleted every BookFile row attached to the book, wherever it lived. Now only files in the folder the incoming file will occupy are replaced; anything else is left on disk, detached from the book and warned about, and the next scan picks it up as an unmapped file. If the destination cannot be calculated the old behaviour is kept, so imports are never blocked. Calibre libraries are unaffected.
  4. fix(manualimport): guard null ImportItem when cleaning up download folders - ManualImport threw a NullReferenceException after importing successfully when the tracked download came from the client's history rather than a grab.

Todos

  • Tests
  • Translation Keys - not needed, no user-facing strings added
  • Wiki Updates - not needed

Notes for reviewers

  • Behaviour change in commit 3. If a book's folder is renamed upstream (a title change), an existing file in the old folder is no longer deleted on the next import; it is detached and left on disk, then re-imported by the next scan as an unmapped file. That seemed the right trade against deleting an unrelated book. Happy to gate it behind a config option instead.
  • replaceExistingFiles is dead. The flag is plumbed from the manual import API through ImportApprovedBooks but its only consumer, RemoveExistingTrackFiles, is commented out at ImportApprovedBooks.cs:128-131. Passing false does not prevent existing files from being deleted. Not touched here - it deserves its own decision: implement it or remove it from the API.

Issues Fixed or Closed by this PR

File tags are not a trusted source of identifiers. Calibre writes its internal
UUID into the MOBI/AZW3 EXTH ASIN record (113), so a converted file carries a
value such as "a2f97540-d315-4ee6-a025-5325c852d261" in the field the importer
reads as an ASIN.

BookDistance compared that UUID against the edition ASIN and counted it as a
mismatch, which is weighted at 10.0 against the book title's 3.0. An edition
that has an ASIN was therefore penalised far more heavily than a wrong-title
edition with no ASIN at all (weight 0.1), so a different book by the same
author could win the match.

Validate the local identifier against the expected format before using it and
ignore it otherwise, so a malformed value behaves the same as a missing one.

Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
… mismatch

A matching identifier is strong evidence that an edition is the right one, but
a differing identifier is not evidence that it is the wrong book. Editions of
the same work legitimately carry different ISBNs and ASINs, and the metadata
source frequently holds an identifier the file does not.

Scoring a mismatch at weight 10.0 meant the correct book scored worse than an
unrelated book by the same author that simply had no identifier to compare
(weight 0.1), because a wrong title only costs 3.0. Observed with a Calibre
converted file whose valid ASIN did not appear on the correct book's editions:

  [Twisted Lies]  title exact, asin differs  -> 0.3855
  [Twisted Games] title differs, no asin     -> 0.0644  (chosen)

Reward a match instead of punishing a mismatch, so an identifier can promote
the right edition but can no longer select a different book.

Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
…lder

UpgradeBookFile deleted every BookFile attached to the book being imported,
identified only by the path stored in the database. When a file has been
matched to the wrong book - which the metadata distance calculation can do for
files carrying an unusable identifier - importing the real file for that book
deletes the unrelated one:

  bookFileImported bookId=3 | Ana Huang - Twisted Games
  bookFileDeleted  bookId=3 | /books/Ana Huang/Twisted Lies/Twisted Lies.azw3 | reason=Upgrade

With no Recycle Bin configured, RecycleBinProvider deletes permanently, so the
unrelated book is gone. Reported in pennydreadful#173.

Work out where the incoming file will land and only replace existing files in
that folder. Anything else is not a copy of this book being upgraded, so leave
it on disk, detach it from the book and warn; the next scan picks it up as an
unmapped file. If the destination cannot be calculated the previous behaviour
is kept, so imports are never blocked by this check.

Calibre libraries are unaffected: files there are addressed by CalibreId.

Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
…lders

A tracked download adopted from the download client's history has no import
item, so the post-import cleanup threw and the whole ManualImport command was
reported as failed even though every file had already been imported:

  System.NullReferenceException: Object reference not set to an instance of an object.
     at NzbDrone.Core.MediaFiles.BookImport.Manual.ManualImportService.Execute(ManualImportCommand message)

Skip the folder cleanup when there is no import item; there is no output
folder to remove in that case.

Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Import of one book deletes unrelated book's file (no recycle bin fallback = permanent data loss)

1 participant