From eb01f6bfcef846a7b6b2850fc2a3ea96961b48f0 Mon Sep 17 00:00:00 2001 From: jbob0 Date: Mon, 31 Aug 2026 12:10:15 -0400 Subject: [PATCH] Retarget cross-provider duplicate book rows to the grabbed book on shared edition identity RetargetSameWorkMatchesToGrabbedBook only retargeted an import decision back to the grabbed book row when the two rows shared a work-level provider ID (WorkIdMatcher.WorkProviderIdMatches). Two rows created for the same real book via different metadata providers (e.g. one Hardcover-sourced, one Goodreads-sourced) never share a work ID, so a completed, tracked download could permanently fail to import if the matcher happened to resolve to the wrong (unmonitored duplicate) row - no retry ever succeeds, since the matcher makes the same choice every time. Add a second, independent signal: if the matched edition already exists under the grabbed book row via a shared edition-level identifier (ISBN, ASIN, Audible ASIN, or a provider edition ID), that's sufficient proof the two rows represent the same book, since two different books cannot share an ISBN/ASIN. The retarget still only ever lands on a concrete edition the grabbed row already owns - no cloning or synthesis. The existing same-author and same-media-type guards are unchanged. Fixes #111 --- .../DownloadedBooksImportServiceFixture.cs | 237 ++++++++++++++++++ .../DownloadedBooksImportService.cs | 40 ++- 2 files changed, 274 insertions(+), 3 deletions(-) diff --git a/src/Chaptarr.Core.Test/MediaFiles/DownloadedBooksImportServiceFixture.cs b/src/Chaptarr.Core.Test/MediaFiles/DownloadedBooksImportServiceFixture.cs index e2755785..a8c8cf9f 100644 --- a/src/Chaptarr.Core.Test/MediaFiles/DownloadedBooksImportServiceFixture.cs +++ b/src/Chaptarr.Core.Test/MediaFiles/DownloadedBooksImportServiceFixture.cs @@ -1826,6 +1826,243 @@ public void should_reject_same_work_sibling_match_when_grabbed_book_has_no_equiv } } + // Issue #111: an author-bibliography re-discovery re-added the same real book under a different + // metadata provider, so the duplicate row's work ID (gr:...) never intersects the grabbed row's + // work ID (hc:...). Both rows still carry the same physical edition (same Audible ASIN), which is + // globally unique, so the download must be retargeted onto the row that actually owns the grab. + [Test] + public void should_retarget_cross_provider_duplicate_row_match_to_grabbed_book_when_edition_identity_matches() + { + var tempDir = Path.Combine(Path.GetTempPath(), "chaptarr-tests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(tempDir); + var filePath = Path.Combine(tempDir, "Fate Hollow Academy Term 1.m4b"); + File.WriteAllBytes(filePath, new byte[] { 1, 2, 3, 4 }); + + try + { + var diskProvider = new StubDiskProvider(); + var diskScanService = new StubDiskScanService(); + var tagsService = new StubMetadataTagService(); + + var matchingService = new StubFileMatchingService + { + InitialResult = new FileMatchResult + { + MatchedFiles = new[] + { + new FileMatch + { + File = new DiscoveredFileWithMetadata { Path = filePath, Size = 4, Modified = DateTime.UtcNow, AllTags = tagsService.Tags }, + AuthorId = 7, + AuthorName = "Test Author", + BookId = 29100, + BookTitle = "Fate Hollow Academy", + EditionId = 30 + } + }, + UnmatchedFiles = Array.Empty() + } + }; + + var author = new Author { Id = 7, Name = "Test Author" }; + + // Never-monitored duplicate row, re-discovered from Goodreads. + var matchedBook = new Book { Id = 29100, AuthorId = 7, Author = author, Title = "Fate Hollow Academy", AnyEditionOk = true, MediaType = BookMediaType.Audiobook, GoodreadsWorkId = "gr:200474660" }; + + // The row that was actually grabbed, sourced from Hardcover. No work-ID overlap with the duplicate. + var targetBook = new Book { Id = 29083, AuthorId = 7, Author = author, Title = "Fate Hollow Academy: Term 1", AnyEditionOk = true, MediaType = BookMediaType.Audiobook, HardcoverBookId = "hc:585413" }; + + var matchedEdition = new Edition { Id = 30, BookId = 29100, Title = "Fate Hollow Academy", AudibleASIN = "B0DXYZ1234", ReadingFormatId = 2, Monitored = false, Book = matchedBook }; + var targetEdition = new Edition { Id = 40, BookId = 29083, Title = "Fate Hollow Academy: Term 1", AudibleASIN = "B0DXYZ1234", ReadingFormatId = 2, Monitored = false, Book = targetBook }; + matchedBook.Editions = new List { matchedEdition }; + targetBook.Editions = new List { targetEdition }; + + var importApproved = new RecordingImportApprovedBooks(); + + var bookService = DispatchProxy.Create(); + var bookProxy = (BookServiceProxy)(object)bookService; + bookProxy.BooksById[29100] = matchedBook; + bookProxy.BooksById[29083] = targetBook; + + var authorService = DispatchProxy.Create(); + ((AuthorServiceProxy)(object)authorService).Author = author; + + var editionService = DispatchProxy.Create(); + var editionProxy = (EditionServiceProxy)(object)editionService; + editionProxy.EditionsById[30] = matchedEdition; + editionProxy.EditionsById[40] = targetEdition; + editionProxy.EditionsByBookId[29100] = new List { matchedEdition }; + editionProxy.EditionsByBookId[29083] = new List { targetEdition }; + + var importOrchestrator = DispatchProxy.Create>(); + var configService = ConfigServiceTestProxy.Create(); + var historyService = DispatchProxy.Create(); + var eventAggregator = DispatchProxy.Create>(); + var runtimeInfo = DispatchProxy.Create>(); + + var service = new DownloadedBooksImportService( + diskProvider, + diskScanService, + matchingService, + tagsService, + importApproved, + bookService, + authorService, + editionService, + importOrchestrator, + new StubAuthorLibraryService(), + new StubRootFolderService(), + configService, + historyService, + eventAggregator, + runtimeInfo, + DispatchProxy.Create>(), + LogManager.GetCurrentClassLogger()); + + var remoteBook = new RemoteBook + { + Author = author, + Books = new List { new Book { Id = 29083, AuthorId = 7, Title = "Fate Hollow Academy: Term 1", AnyEditionOk = true, MediaType = BookMediaType.Audiobook, HardcoverBookId = "hc:585413" } } + }; + + var downloadClientItem = new DownloadClientItem + { + DownloadId = "DOWNLOAD-CROSS-PROVIDER-DUPLICATE", + CanMoveFiles = false, + DownloadClientInfo = new DownloadClientItemClientInfo { Id = 1, Name = "qBittorrent", Type = "qBittorrent" } + }; + + _ = service.ProcessPath(filePath, ImportMode.Auto, author, downloadClientItem, remoteBook); + + Assert.That(importApproved.Decisions, Has.Count.EqualTo(1)); + Assert.That(importApproved.Decisions[0].Approved, Is.True); + Assert.That(importApproved.Decisions[0].Item.Book.Id, Is.EqualTo(29083)); + Assert.That(importApproved.Decisions[0].Item.Edition.Id, Is.EqualTo(40)); + Assert.That(importApproved.Decisions[0].Item.Edition.Book, Is.SameAs(targetBook)); + } + finally + { + try { Directory.Delete(tempDir, recursive: true); } catch { } + } + } + + // Safety boundary for issue #111: same author and media type, but the two rows share no work-level + // ID and no edition-level ID. Nothing proves they are the same book, so the file must NOT be moved + // onto the grabbed row -- the download is rejected and the matched row is left untouched. + [Test] + public void should_not_retarget_cross_provider_row_match_when_no_shared_work_or_edition_identity() + { + var tempDir = Path.Combine(Path.GetTempPath(), "chaptarr-tests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(tempDir); + var filePath = Path.Combine(tempDir, "Some Other Book.m4b"); + File.WriteAllBytes(filePath, new byte[] { 1, 2, 3, 4 }); + + try + { + var diskProvider = new StubDiskProvider(); + var diskScanService = new StubDiskScanService(); + var tagsService = new StubMetadataTagService(); + + var matchingService = new StubFileMatchingService + { + InitialResult = new FileMatchResult + { + MatchedFiles = new[] + { + new FileMatch + { + File = new DiscoveredFileWithMetadata { Path = filePath, Size = 4, Modified = DateTime.UtcNow, AllTags = tagsService.Tags }, + AuthorId = 7, + AuthorName = "Test Author", + BookId = 29100, + BookTitle = "A Different Book", + EditionId = 30 + } + }, + UnmatchedFiles = Array.Empty() + } + }; + + var author = new Author { Id = 7, Name = "Test Author" }; + var matchedBook = new Book { Id = 29100, AuthorId = 7, Author = author, Title = "A Different Book", AnyEditionOk = true, MediaType = BookMediaType.Audiobook, GoodreadsWorkId = "gr:200474660" }; + var targetBook = new Book { Id = 29083, AuthorId = 7, Author = author, Title = "Fate Hollow Academy: Term 1", AnyEditionOk = true, MediaType = BookMediaType.Audiobook, HardcoverBookId = "hc:585413" }; + + var matchedEdition = new Edition { Id = 30, BookId = 29100, Title = "A Different Book", AudibleASIN = "B0DAAAAAAA", ReadingFormatId = 2, Monitored = false, Book = matchedBook }; + var targetEdition = new Edition { Id = 40, BookId = 29083, Title = "Fate Hollow Academy: Term 1", AudibleASIN = "B0DZZZZZZZ", ReadingFormatId = 2, Monitored = false, Book = targetBook }; + matchedBook.Editions = new List { matchedEdition }; + targetBook.Editions = new List { targetEdition }; + + var importApproved = new RecordingImportApprovedBooks(); + + var bookService = DispatchProxy.Create(); + var bookProxy = (BookServiceProxy)(object)bookService; + bookProxy.BooksById[29100] = matchedBook; + bookProxy.BooksById[29083] = targetBook; + + var authorService = DispatchProxy.Create(); + ((AuthorServiceProxy)(object)authorService).Author = author; + + var editionService = DispatchProxy.Create(); + var editionProxy = (EditionServiceProxy)(object)editionService; + editionProxy.EditionsById[30] = matchedEdition; + editionProxy.EditionsById[40] = targetEdition; + editionProxy.EditionsByBookId[29100] = new List { matchedEdition }; + editionProxy.EditionsByBookId[29083] = new List { targetEdition }; + + var importOrchestrator = DispatchProxy.Create>(); + var configService = ConfigServiceTestProxy.Create(); + var historyService = DispatchProxy.Create(); + var eventAggregator = DispatchProxy.Create>(); + var runtimeInfo = DispatchProxy.Create>(); + + var service = new DownloadedBooksImportService( + diskProvider, + diskScanService, + matchingService, + tagsService, + importApproved, + bookService, + authorService, + editionService, + importOrchestrator, + new StubAuthorLibraryService(), + new StubRootFolderService(), + configService, + historyService, + eventAggregator, + runtimeInfo, + DispatchProxy.Create>(), + LogManager.GetCurrentClassLogger()); + + var remoteBook = new RemoteBook + { + Author = author, + Books = new List { new Book { Id = 29083, AuthorId = 7, Title = "Fate Hollow Academy: Term 1", AnyEditionOk = true, MediaType = BookMediaType.Audiobook, HardcoverBookId = "hc:585413" } } + }; + + var downloadClientItem = new DownloadClientItem + { + DownloadId = "DOWNLOAD-CROSS-PROVIDER-UNRELATED", + CanMoveFiles = false, + DownloadClientInfo = new DownloadClientItemClientInfo { Id = 1, Name = "qBittorrent", Type = "qBittorrent" } + }; + + _ = service.ProcessPath(filePath, ImportMode.Auto, author, downloadClientItem, remoteBook); + + Assert.That(importApproved.Decisions, Has.Count.EqualTo(1)); + Assert.That(importApproved.Decisions[0].Approved, Is.False); + + // The file must stay attached to the row the matcher chose; retargeting it would misfile it. + Assert.That(importApproved.Decisions[0].Item.Book.Id, Is.EqualTo(29100)); + Assert.That(importApproved.Decisions[0].Rejections.Select(r => r.Reason), Has.Some.Contains("but import matched")); + Assert.That(importApproved.Decisions[0].Rejections.Select(r => r.Reason), Has.None.Contains("no equivalent edition")); + } + finally + { + try { Directory.Delete(tempDir, recursive: true); } catch { } + } + } + [Test] public void should_reject_completed_download_when_latest_grabbed_edition_is_strict_and_match_hits_different_sibling() { diff --git a/src/NzbDrone.Core/MediaFiles/DownloadedBooksImportService.cs b/src/NzbDrone.Core/MediaFiles/DownloadedBooksImportService.cs index 123fe68d..bb157fe6 100644 --- a/src/NzbDrone.Core/MediaFiles/DownloadedBooksImportService.cs +++ b/src/NzbDrone.Core/MediaFiles/DownloadedBooksImportService.cs @@ -725,15 +725,44 @@ private void RetargetSameWorkMatchesToGrabbedBook(List { var localBook = decision.Item; var matchedBook = localBook?.Book; - if (!CanRetargetSameWorkMatch(targetBook, matchedBook)) + if (!SharesRetargetableRowIdentity(targetBook, matchedBook)) { continue; } var matchedEdition = localBook.Edition; + + // Two independent same-book signals are accepted here: + // + // 1. Work-level provider IDs intersect (hc/gr/ol work IDs). This is the original signal and + // covers duplicate rows catalogued through the same metadata provider. + // + // 2. The matched edition itself also exists under the grabbed book row, identified by an + // edition-level provider ID (ISBN-10/13, ASIN/Audible ASIN, or a Goodreads/Hardcover/ + // OpenLibrary/Google Books edition ID). Duplicate rows created by author-bibliography + // re-discovery are frequently sourced from a *different* provider than the row that was + // grabbed, so their work-level ID sets never intersect even though they describe the same + // title. Signal 2 recognises those without weakening the guarantee: edition-level provider + // IDs are globally unique per physical edition, so a shared one means both rows carry the + // very same edition. Combined with the same-author and same-media-type guards in + // SharesRetargetableRowIdentity, this cannot collide two different books that merely share + // a title -- which is exactly why title similarity is deliberately NOT used as a signal. + // + // Both signals still have to land on a concrete equivalent edition below before anything is + // retargeted, so a match on either one can only ever move the file to an edition the grabbed + // book row already owns. + var sameWorkProviderId = WorkIdMatcher.WorkProviderIdMatches(targetBook, matchedBook); var targetEdition = FindEquivalentEditionForTargetBook(targetBook, matchedEdition); + if (targetEdition == null) { + if (!sameWorkProviderId) + { + // Neither signal fired: nothing establishes that these two rows are the same book. + // Leave the decision untouched so the caller's expected-book check rejects it. + continue; + } + decision.Reject(new Rejection( $"Completed download was grabbed for {FormatBookLabel(targetBook)} and matched same-work sibling {FormatBookLabel(matchedBook)}, but no equivalent edition exists under the grabbed book row. Refresh metadata and retry.")); continue; @@ -780,7 +809,12 @@ private Book HydrateExpectedBook(Book book) } } - private static bool CanRetargetSameWorkMatch(Book targetBook, Book matchedBook) + /// + /// Baseline guards every retarget must clear, independent of how "same book" is established: + /// two distinct persisted rows, the same media type, and the same (known) author. These are + /// hard requirements -- retargeting across authors or across media types would misfile the download. + /// + private static bool SharesRetargetableRowIdentity(Book targetBook, Book matchedBook) { if (targetBook == null || matchedBook == null || @@ -803,7 +837,7 @@ private static bool CanRetargetSameWorkMatch(Book targetBook, Book matchedBook) return false; } - return WorkIdMatcher.WorkProviderIdMatches(targetBook, matchedBook); + return true; } private Edition FindEquivalentEditionForTargetBook(Book targetBook, Edition matchedEdition)