From bf66b21e279eae09af67aeca53301251c0111e40 Mon Sep 17 00:00:00 2001 From: Jon Truland Date: Sat, 3 Oct 2026 12:53:22 -0400 Subject: [PATCH 1/3] Add downloaded ebooks through Calibre when the root folder is a Calibre library ImportApprovedBooks always moved or copied the file with the book file mover, so a download imported into a root folder marked as a Calibre library bypassed the content server: the book never reached metadata.db and Calibre/Calibre-Web could not see it (issue #205). When the import destination for an ebook resolves to a Calibre library root with settings, hand the file to CalibreProxy.AddAndConvert instead, as UpgradeMediaFileService already does. Calibre picks the folder and SetFields records its path on the BookFile. Tag writing and release extras are skipped for these files because Calibre owns the folder. Relocations of already-tracked files keep the existing path. Co-Authored-By: Claude Opus 5.5 --- ...tApprovedBooksCalibreDestinationFixture.cs | 121 ++++++++++++++++++ .../BookImport/ImportApprovedBooks.cs | 65 +++++++++- 2 files changed, 181 insertions(+), 5 deletions(-) create mode 100644 src/Chaptarr.Core.Test/MediaFiles/ImportApprovedBooksCalibreDestinationFixture.cs diff --git a/src/Chaptarr.Core.Test/MediaFiles/ImportApprovedBooksCalibreDestinationFixture.cs b/src/Chaptarr.Core.Test/MediaFiles/ImportApprovedBooksCalibreDestinationFixture.cs new file mode 100644 index 00000000..1c29d145 --- /dev/null +++ b/src/Chaptarr.Core.Test/MediaFiles/ImportApprovedBooksCalibreDestinationFixture.cs @@ -0,0 +1,121 @@ +using System; +using System.Collections.Generic; +using System.Reflection; +using NLog; +using NUnit.Framework; +using NzbDrone.Core.Books; +using NzbDrone.Core.Books.Calibre; +using NzbDrone.Core.MediaFiles; +using NzbDrone.Core.MediaFiles.BookImport; +using NzbDrone.Core.Parser.Model; +using NzbDrone.Core.RootFolders; + +namespace Chaptarr.Core.Test.MediaFiles +{ + [TestFixture] + public class ImportApprovedBooksCalibreDestinationFixture + { + private static readonly MethodInfo GetSettingsMethod = typeof(ImportApprovedBooks).GetMethod( + "GetCalibreDestinationSettings", + BindingFlags.NonPublic | BindingFlags.Instance); + + private class StubProxy : DispatchProxy + where T : class + { + public Dictionary> Handlers { get; } = new(); + + public static T Create(Dictionary> handlers = null) + { + var proxy = DispatchProxy.Create>(); + var state = (StubProxy)(object)proxy; + foreach (var handler in handlers ?? new Dictionary>()) + { + state.Handlers[handler.Key] = handler.Value; + } + + return proxy; + } + + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + if (Handlers.TryGetValue(targetMethod.Name, out var handler)) + { + return handler(args); + } + + throw new NotImplementedException($"Test proxy does not implement {typeof(T).Name}.{targetMethod.Name}"); + } + } + + private const string CalibreRoot = "/data/calibre/library"; + private const string PlainRoot = "/data/books"; + + [Test] + public void should_route_ebook_into_calibre_library_root_through_calibre() + { + var settings = new CalibreSettings { Host = "calibre", Port = 8181 }; + + var result = GetSettings(BuildFile("ebook"), CalibreRoot + "/Author/Book/Book.epub", + new RootFolder { Path = CalibreRoot, IsCalibreLibrary = true, CalibreSettings = settings }); + + Assert.That(result, Is.SameAs(settings)); + } + + [Test] + public void should_use_normal_transfer_for_plain_root_folder() + { + var result = GetSettings(BuildFile("ebook"), PlainRoot + "/Author/Book/Book.epub", + new RootFolder { Path = PlainRoot, IsCalibreLibrary = false }); + + Assert.That(result, Is.Null); + } + + [Test] + public void should_use_normal_transfer_when_calibre_root_has_no_settings() + { + var result = GetSettings(BuildFile("ebook"), CalibreRoot + "/Author/Book/Book.epub", + new RootFolder { Path = CalibreRoot, IsCalibreLibrary = true, CalibreSettings = null }); + + Assert.That(result, Is.Null); + } + + [Test] + public void should_never_send_audiobooks_to_calibre() + { + var result = GetSettings(BuildFile("audiobook"), CalibreRoot + "/Author/Book/Book.m4b", + new RootFolder { Path = CalibreRoot, IsCalibreLibrary = true, CalibreSettings = new CalibreSettings() }); + + Assert.That(result, Is.Null); + } + + private static BookFile BuildFile(string mediaType) + { + return new BookFile { Path = "/downloads/Book.epub", MediaType = mediaType }; + } + + private static CalibreSettings GetSettings(BookFile bookFile, string destination, RootFolder rootFolder) + { + var mover = StubProxy.Create(new Dictionary> + { + [nameof(IMoveBookFiles.GetImportDestinationPath)] = _ => destination + }); + var rootFolders = StubProxy.Create(new Dictionary> + { + [nameof(IRootFolderService.GetBestRootFolder)] = args => + { + Assert.That(args[0], Is.EqualTo(destination)); + return rootFolder; + } + }); + + var subject = new ImportApprovedBooks( + null, null, null, null, null, null, null, null, mover, null, null, + null, null, null, null, null, null, + LogManager.GetLogger(nameof(ImportApprovedBooksCalibreDestinationFixture)), + rootFolderService: rootFolders, + calibre: StubProxy.Create()); + + return (CalibreSettings)GetSettingsMethod.Invoke(subject, new object[] { bookFile, new LocalBook { Path = bookFile.Path } }); + } + } +} diff --git a/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs b/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs index 9a955cfa..101438a7 100644 --- a/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs +++ b/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs @@ -17,6 +17,7 @@ using NzbDrone.Common.Instrumentation.Extensions; using NzbDrone.Core.Books; using NzbDrone.Core.Books.Calibre; +using NzbDrone.Core.RootFolders; using NzbDrone.Core.Books.Commands; using NzbDrone.Core.Configuration; using NzbDrone.Core.CustomFormats; @@ -90,6 +91,8 @@ private sealed class PendingFileCommit private readonly IContainmentValidator _containmentValidator; private readonly IMapCoversToLocal _coverMapper; private readonly ICustomFormatCalculationService _customFormatCalculationService; + private readonly IRootFolderService _rootFolderService; + private readonly ICalibreProxy _calibre; private readonly Logger _logger; public ImportApprovedBooks( @@ -117,7 +120,9 @@ public ImportApprovedBooks( IContainmentValidator containmentValidator = null, IMapCoversToLocal coverMapper = null, ICustomFormatCalculationService customFormatCalculationService = null, - IConversionJobService conversionJobService = null) + IConversionJobService conversionJobService = null, + IRootFolderService rootFolderService = null, + ICalibreProxy calibre = null) { _mediaFileService = mediaFileService; _metadataTagService = metadataTagService; @@ -143,6 +148,8 @@ public ImportApprovedBooks( _containmentValidator = containmentValidator; _coverMapper = coverMapper; _customFormatCalculationService = customFormatCalculationService; + _rootFolderService = rootFolderService; + _calibre = calibre; _logger = logger; } @@ -1315,10 +1322,28 @@ private void CleanupNewBookInstanceIfEmpty(bool createdNewBookInstance, Book new _logger.Debug("[PERFORMANCE] Old file staging took {0}ms", stageStopwatch.ElapsedMilliseconds); } + var calibreSettings = relocateExistingFile == null + ? GetCalibreDestinationSettings(bookFile, localBook) + : null; + try { var transferStopwatch = Stopwatch.StartNew(); - if (copyOnly) + if (calibreSettings != null) + { + // A Calibre library root is owned by Calibre: add the book through the + // content server so it lands in metadata.db and Calibre picks the folder. + // AddAndConvert reads the source and sets Path to Calibre's copy. + var source = localBook.Path; + bookFile.Path = source; + bookFile = _calibre.AddAndConvert(bookFile, calibreSettings); + + if (!copyOnly) + { + _diskProvider.DeleteFile(source); + } + } + else if (copyOnly) { bookFile = _bookFileMover.CopyBookFile(bookFile, localBook); } @@ -1365,17 +1390,22 @@ private void CleanupNewBookInstanceIfEmpty(bool createdNewBookInstance, Book new { writeTagsForRelocatedFileAfterPersistence = true; } - else + else if (calibreSettings == null) { + // Calibre already wrote metadata into its own copy. TryWriteTags(bookFile, true, "TRANSFER"); } } - // Import extras (best-effort; do not fail the core import if extras post-processing fails) + // Import extras (best-effort; do not fail the core import if extras post-processing fails). + // Calibre manages its own folders, so release extras are not copied into them. var extrasStopwatch = Stopwatch.StartNew(); try { - _extraService.ImportTrack(localBook, bookFile, copyOnly); + if (calibreSettings == null) + { + _extraService.ImportTrack(localBook, bookFile, copyOnly); + } } catch (Exception ex) { @@ -3389,6 +3419,31 @@ private void TryWriteTags(BookFile bookFile, bool newDownload, string route) // Transfer logic is handled by IMoveBookFiles (BookFileMovingService) + // Returns the Calibre settings when this ebook's import destination is inside a root folder + // marked as a Calibre library; null means the normal move/copy path applies. + private CalibreSettings GetCalibreDestinationSettings(BookFile bookFile, LocalBook localBook) + { + if (_rootFolderService == null || _calibre == null || !string.Equals(bookFile.MediaType, "ebook", StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + var destination = _bookFileMover.GetImportDestinationPath(bookFile, localBook); + if (destination.IsNullOrWhiteSpace()) + { + return null; + } + + var rootFolder = _rootFolderService.GetBestRootFolder(destination); + if (rootFolder?.IsCalibreLibrary == true && rootFolder.CalibreSettings != null) + { + _logger.Debug("[CALIBRE-IMPORT] Destination {0} is in Calibre library root {1}; adding through Calibre", destination, rootFolder.Path); + return rootFolder.CalibreSettings; + } + + return null; + } + private bool ShouldMoveFile(LocalBook localBook, Author author) { // Only used for new downloads. If file already resides under the configured root From 10a758cc34ee9dea4352a9409cc14051f9829b7f Mon Sep 17 00:00:00 2001 From: Jon Truland Date: Sat, 3 Oct 2026 13:00:46 -0400 Subject: [PATCH 2/3] Keep a Calibre import when the source file cannot be removed Once AddAndConvert succeeds the book is in Calibre. Treating a failed delete of the download as a failed import left the book untracked, and a retry would add a duplicate to Calibre. Log a warning instead. Co-Authored-By: Claude Opus 5.5 --- .../MediaFiles/BookImport/ImportApprovedBooks.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs b/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs index 101438a7..f44a1f71 100644 --- a/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs +++ b/src/NzbDrone.Core/MediaFiles/BookImport/ImportApprovedBooks.cs @@ -1340,7 +1340,16 @@ private void CleanupNewBookInstanceIfEmpty(bool createdNewBookInstance, Book new if (!copyOnly) { - _diskProvider.DeleteFile(source); + // Calibre now holds the book. Failing the import here would leave it + // untracked and a retry would add it to Calibre a second time. + try + { + _diskProvider.DeleteFile(source); + } + catch (Exception ex) + { + _logger.Warn(ex, "[CALIBRE-IMPORT] Added to Calibre but could not remove source file: {0}", source); + } } } else if (copyOnly) From 6258b9ff6f95c440c56fe49af065cd4a1d8857ef Mon Sep 17 00:00:00 2001 From: Jon Truland Date: Sat, 3 Oct 2026 13:14:03 -0400 Subject: [PATCH 3/3] Do not let missing metadata abort a Calibre metadata push CalibreProxy.SetFields runs after the book is already in Calibre. It dereferenced series links, edition images, genres and ratings without null checks, so a book with no genres or series links threw ArgumentNullException at OrderBy and failed the whole import, leaving the book in Calibre with no Chaptarr record (a retry then added it twice). Treat absent metadata as empty. Co-Authored-By: Claude Opus 5.5 --- .../Books/CalibreProxySetFieldsFixture.cs | 65 +++++++++++++++++++ .../Books/Calibre/CalibreProxy.cs | 11 ++-- 2 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 src/Chaptarr.Core.Test/Books/CalibreProxySetFieldsFixture.cs diff --git a/src/Chaptarr.Core.Test/Books/CalibreProxySetFieldsFixture.cs b/src/Chaptarr.Core.Test/Books/CalibreProxySetFieldsFixture.cs new file mode 100644 index 00000000..5cbc25b1 --- /dev/null +++ b/src/Chaptarr.Core.Test/Books/CalibreProxySetFieldsFixture.cs @@ -0,0 +1,65 @@ +using System; +using System.Collections.Generic; +using System.Reflection; +using NLog; +using NUnit.Framework; +using NzbDrone.Common.Cache; +using NzbDrone.Common.Http; +using NzbDrone.Core.Books; +using NzbDrone.Core.Books.Calibre; +using NzbDrone.Core.MediaFiles; + +namespace Chaptarr.Core.Test.Books +{ + [TestFixture] + public class CalibreProxySetFieldsFixture + { + private sealed class ReachedHttpException : Exception + { + } + + // Any HTTP call means the payload was built; stop there. + private class HttpClientProxy : DispatchProxy + { + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + throw new ReachedHttpException(); + } + } + + [Test] + public void should_build_payload_when_provider_metadata_is_missing() + { + var book = new Book + { + Id = 1, + Title = "City of Lost Souls", + Genres = null, + LazySeriesLinks = null + }; + var edition = new Edition + { + Id = 2, + BookId = book.Id, + Book = book, + Title = book.Title, + Images = null, + Ratings = null + }; + var file = new BookFile + { + CalibreId = 2766, + Path = "/data/calibre/library/Cassandra Clare/City of Lost Souls (2766)/City of Lost Souls.epub", + Edition = edition, + Author = new Author { Name = "Cassandra Clare" } + }; + + var subject = new CalibreProxy( + DispatchProxy.Create(), + null, null, null, null, null, new CacheManager(), + LogManager.GetLogger(nameof(CalibreProxySetFieldsFixture))); + + Assert.Throws(() => subject.SetFields(file, new CalibreSettings { Host = "calibre", Port = 8181 }, false)); + } + } +} diff --git a/src/NzbDrone.Core/Books/Calibre/CalibreProxy.cs b/src/NzbDrone.Core/Books/Calibre/CalibreProxy.cs index 4c0de116..82879eba 100644 --- a/src/NzbDrone.Core/Books/Calibre/CalibreProxy.cs +++ b/src/NzbDrone.Core/Books/Calibre/CalibreProxy.cs @@ -213,7 +213,10 @@ public void SetFields(BookFile file, CalibreSettings settings, bool updateCover { var edition = file.Edition; var book = edition.Book; - var serieslink = book.SeriesLinks.OrderBy(x => x.SeriesPosition).FirstOrDefault(x => x.Series.Value.Title.IsNotNullOrWhiteSpace()); + // Provider metadata can leave series links, images, genres and ratings unset; none of + // them may abort the push, because the book is already in Calibre by the time this runs. + var seriesLinks = book.SeriesLinks ?? new List(); + var serieslink = seriesLinks.OrderBy(x => x.SeriesPosition).FirstOrDefault(x => x.Series?.Value?.Title.IsNotNullOrWhiteSpace() == true); var series = serieslink?.Series.Value; double? seriesIndex = null; @@ -225,7 +228,7 @@ public void SetFields(BookFile file, CalibreSettings settings, bool updateCover _logger.Trace("Book: {0} Series: {1}, Position: {2}", book, series?.Title, seriesIndex); - var cover = edition.Images.FirstOrDefault(x => x.CoverType == MediaCoverTypes.Cover); + var cover = (edition.Images ?? new List()).FirstOrDefault(x => x.CoverType == MediaCoverTypes.Cover); string image = null; if (cover != null) { @@ -242,7 +245,7 @@ public void SetFields(BookFile file, CalibreSettings settings, bool updateCover } var textInfo = CultureInfo.InvariantCulture.TextInfo; - var genres = book.Genres.Select(x => textInfo.ToTitleCase(x.Replace('-', ' '))).ToList(); + var genres = (book.Genres ?? new List()).Where(x => x.IsNotNullOrWhiteSpace()).Select(x => textInfo.ToTitleCase(x.Replace('-', ' '))).ToList(); var payload = new CalibreChangesPayload { @@ -257,7 +260,7 @@ public void SetFields(BookFile file, CalibreSettings settings, bool updateCover Languages = edition.Language.CanonicalizeLanguage(), Tags = genres, Comments = edition.Overview, - Rating = (int)(edition.Ratings.Value * 2), + Rating = (int)((edition.Ratings?.Value ?? 0) * 2), Identifiers = new Dictionary { { "isbn", edition.Isbn13 },