diff --git a/src/Chaptarr.Core.Test/Extras/CleanExtraFileServiceFixture.cs b/src/Chaptarr.Core.Test/Extras/CleanExtraFileServiceFixture.cs new file mode 100644 index 00000000..cb9c84d7 --- /dev/null +++ b/src/Chaptarr.Core.Test/Extras/CleanExtraFileServiceFixture.cs @@ -0,0 +1,218 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using NLog; +using NUnit.Framework; +using NzbDrone.Common.Disk; +using NzbDrone.Core.Books; +using NzbDrone.Core.Extras.Metadata.Files; +using NzbDrone.Core.MediaFiles; + +namespace Chaptarr.Core.Test.Extras +{ + [TestFixture] + public class CleanExtraFileServiceFixture + { + private const string AudioBase = "/mnt/Media/Audio Books/Some Author"; + private const string EbookBase = "/mnt/Media/Books/Some Author"; + + private class DiskProxy : DispatchProxy + { + public HashSet ExistingFiles { get; } = new HashSet(); + public List Probes { get; } = new List(); + + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + if (targetMethod?.Name == nameof(IDiskProvider.FileExists) && args?.Length == 1 && args[0] is string path) + { + Probes.Add(path); + return ExistingFiles.Contains(path); + } + + throw new NotImplementedException($"Test proxy does not implement IDiskProvider.{targetMethod?.Name}"); + } + } + + private class MetadataFileServiceProxy : DispatchProxy + { + public List Files { get; set; } = new List(); + public List> DeleteManyCalls { get; } = new List>(); + public List DeleteSingleCalls { get; } = new List(); + + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + switch (targetMethod?.Name) + { + case nameof(IMetadataFileService.GetFilesByAuthor): + return Files; + case nameof(IMetadataFileService.DeleteMany): + DeleteManyCalls.Add(((IEnumerable)args[0]).ToList()); + return null; + case nameof(IMetadataFileService.Delete): + DeleteSingleCalls.Add((int)args[0]); + return null; + } + + throw new NotImplementedException($"Test proxy does not implement IMetadataFileService.{targetMethod?.Name}"); + } + } + + private class MediaFileServiceProxy : DispatchProxy + { + public List Files { get; set; } = new List(); + public int GetFilesByAuthorCalls { get; private set; } + + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + if (targetMethod?.Name == nameof(IMediaFileService.GetFilesByAuthor) && args?.Length == 1 && args[0] is int) + { + GetFilesByAuthorCalls++; + return Files; + } + + throw new NotImplementedException($"Test proxy does not implement IMediaFileService.{targetMethod?.Name}"); + } + } + + private DiskProxy _disk; + private MetadataFileServiceProxy _metadata; + private MediaFileServiceProxy _media; + private CleanExtraFileService _sut; + private Author _author; + + [SetUp] + public void SetUp() + { + var disk = DispatchProxy.Create(); + var metadata = DispatchProxy.Create(); + var media = DispatchProxy.Create(); + _disk = (DiskProxy)(object)disk; + _metadata = (MetadataFileServiceProxy)(object)metadata; + _media = (MediaFileServiceProxy)(object)media; + _sut = new CleanExtraFileService(metadata, disk, LogManager.GetCurrentClassLogger(), media); + _author = new Author { Id = 1, Name = "Some Author", AudiobookPath = AudioBase, EbookPath = EbookBase, Path = AudioBase }; + } + + private static MetadataFile Row(int id, string relative, int? bookFileId = null) => + new MetadataFile { Id = id, AuthorId = 1, RelativePath = relative, BookFileId = bookFileId }; + + [Test] + public void should_delete_every_missing_row_in_a_single_batch_not_one_delete_per_row() + { + _metadata.Files = new List { Row(1, "A/cover.jpg"), Row(2, "B/cover.jpg"), Row(3, "C/cover.jpg") }; + _disk.ExistingFiles.Add(EbookBase + "/B/cover.jpg"); + + _sut.Clean(_author); + + Assert.That(_metadata.DeleteManyCalls, Has.Count.EqualTo(1), "one batched delete"); + Assert.That(_metadata.DeleteManyCalls[0], Is.EquivalentTo(new[] { 1, 3 })); + Assert.That(_metadata.DeleteSingleCalls, Is.Empty, "no per-row deletes"); + } + + [Test] + public void should_not_delete_anything_when_every_file_exists() + { + _metadata.Files = new List { Row(1, "A/cover.jpg"), Row(2, "B/cover.jpg") }; + _disk.ExistingFiles.Add(AudioBase + "/A/cover.jpg"); + _disk.ExistingFiles.Add(EbookBase + "/B/cover.jpg"); + + _sut.Clean(_author); + + Assert.That(_metadata.DeleteManyCalls, Is.Empty); + Assert.That(_metadata.DeleteSingleCalls, Is.Empty); + } + + [Test] + public void should_probe_the_base_path_the_linked_book_file_lives_under_first() + { + // An ebook sidecar: the ebook folder is the right place, the audiobook folder would be a guaranteed miss. + _media.Files = new List { new BookFile { Id = 50, Path = EbookBase + "/Book/Book.epub", MediaType = "ebook" } }; + _metadata.Files = new List { Row(1, "Book/cover.jpg", 50) }; + _disk.ExistingFiles.Add(EbookBase + "/Book/cover.jpg"); + + _sut.Clean(_author); + + Assert.That(_disk.Probes, Is.EqualTo(new[] { EbookBase + "/Book/cover.jpg" }), "found on the first probe, the audiobook folder is never touched"); + Assert.That(_metadata.DeleteManyCalls, Is.Empty); + } + + [Test] + public void should_still_find_a_file_under_another_base_path_when_the_preferred_one_misses() + { + _media.Files = new List { new BookFile { Id = 50, Path = EbookBase + "/Book/Book.epub", MediaType = "ebook" } }; + _metadata.Files = new List { Row(1, "Book/cover.jpg", 50) }; + _disk.ExistingFiles.Add(AudioBase + "/Book/cover.jpg"); // exists only under the other format's folder + + _sut.Clean(_author); + + Assert.That(_metadata.DeleteManyCalls, Is.Empty, "a file that exists somewhere must be kept"); + Assert.That(_disk.Probes.First(), Is.EqualTo(EbookBase + "/Book/cover.jpg"), "preferred base is tried first"); + Assert.That(_disk.Probes, Does.Contain(AudioBase + "/Book/cover.jpg")); + } + + [Test] + public void should_keep_the_default_probe_order_for_rows_that_have_no_book_file() + { + _metadata.Files = new List { Row(1, "poster.jpg") }; + _disk.ExistingFiles.Add(EbookBase + "/poster.jpg"); + + _sut.Clean(_author); + + Assert.That(_disk.Probes, Is.EqualTo(new[] { AudioBase + "/poster.jpg", EbookBase + "/poster.jpg" }), "audiobook, then ebook (legacy path equals audiobook and is de-duplicated)"); + Assert.That(_metadata.DeleteManyCalls, Is.Empty); + } + + [Test] + public void should_fall_back_to_the_default_order_when_the_linked_book_file_no_longer_exists() + { + // BookFileId 99 is not among the author's book files (deleted, or never mapped). + _media.Files = new List { new BookFile { Id = 50, Path = EbookBase + "/Other/Other.epub", MediaType = "ebook" } }; + _metadata.Files = new List { Row(1, "Book/cover.jpg", 99) }; + _disk.ExistingFiles.Add(EbookBase + "/Book/cover.jpg"); + + _sut.Clean(_author); + + Assert.That(_disk.Probes, Is.EqualTo(new[] { AudioBase + "/Book/cover.jpg", EbookBase + "/Book/cover.jpg" }), "default audiobook -> ebook order"); + Assert.That(_metadata.DeleteManyCalls, Is.Empty, "the file exists under the ebook folder, so the row is kept"); + } + + [Test] + public void should_ignore_rows_without_a_relative_path_instead_of_throwing() + { + _metadata.Files = new List { Row(1, null), Row(2, " "), Row(3, "gone.jpg") }; + + Assert.DoesNotThrow(() => _sut.Clean(_author)); + + Assert.That(_metadata.DeleteManyCalls.Single(), Is.EqualTo(new[] { 3 }), "only the row that really is missing is deleted"); + } + + [Test] + public void should_do_no_disk_or_media_file_work_for_an_author_with_no_metadata_files() + { + _metadata.Files = new List(); + + _sut.Clean(_author); + + Assert.That(_disk.Probes, Is.Empty); + Assert.That(_media.GetFilesByAuthorCalls, Is.EqualTo(0)); + Assert.That(_metadata.DeleteManyCalls, Is.Empty); + } + + [Test] + public void should_work_without_a_media_file_service_using_the_default_probe_order() + { + var disk = DispatchProxy.Create(); + var metadata = DispatchProxy.Create(); + var diskProxy = (DiskProxy)(object)disk; + var metadataProxy = (MetadataFileServiceProxy)(object)metadata; + metadataProxy.Files = new List { Row(1, "Book/cover.jpg", 50), Row(2, "gone.jpg") }; + diskProxy.ExistingFiles.Add(EbookBase + "/Book/cover.jpg"); + var sut = new CleanExtraFileService(metadata, disk, LogManager.GetCurrentClassLogger()); + + Assert.DoesNotThrow(() => sut.Clean(_author)); + + Assert.That(metadataProxy.DeleteManyCalls.Single(), Is.EqualTo(new[] { 2 }), "the existing file is kept, the missing one is deleted"); + } + } +} diff --git a/src/NzbDrone.Core/Extras/ExtraFilePathHelper.cs b/src/NzbDrone.Core/Extras/ExtraFilePathHelper.cs index 43b08d0d..4393e925 100644 --- a/src/NzbDrone.Core/Extras/ExtraFilePathHelper.cs +++ b/src/NzbDrone.Core/Extras/ExtraFilePathHelper.cs @@ -26,6 +26,17 @@ public static List GetAuthorBasePaths(Author author) return paths; } + // The author's base paths with `preferredBasePath` (if any) probed first. Extras are written under + // GetPreferredBasePath(author, bookFile), so looking there first finds an existing file in one + // probe instead of missing under the other formats' folders first. + public static List GetAuthorBasePaths(Author author, string preferredBasePath) + { + var paths = GetAuthorBasePaths(author); + PreferBase(paths, preferredBasePath); + + return paths; + } + public static string GetPreferredBasePath(Author author, BookFile bookFile) { if (author == null) diff --git a/src/NzbDrone.Core/Extras/Metadata/Files/CleanMetadataFileService.cs b/src/NzbDrone.Core/Extras/Metadata/Files/CleanMetadataFileService.cs index c4a7818d..d0d7ce78 100644 --- a/src/NzbDrone.Core/Extras/Metadata/Files/CleanMetadataFileService.cs +++ b/src/NzbDrone.Core/Extras/Metadata/Files/CleanMetadataFileService.cs @@ -1,9 +1,11 @@ +using System.Collections.Generic; using System.IO; using System.Linq; using NLog; using NzbDrone.Common.Disk; +using NzbDrone.Common.Extensions; using NzbDrone.Core.Books; -using NzbDrone.Core.Extras; +using NzbDrone.Core.MediaFiles; namespace NzbDrone.Core.Extras.Metadata.Files { @@ -17,14 +19,17 @@ public class CleanExtraFileService : ICleanMetadataService private readonly IMetadataFileService _metadataFileService; private readonly IDiskProvider _diskProvider; private readonly Logger _logger; + private readonly IMediaFileService _mediaFileService; public CleanExtraFileService(IMetadataFileService metadataFileService, IDiskProvider diskProvider, - Logger logger) + Logger logger, + IMediaFileService mediaFileService = null) { _metadataFileService = metadataFileService; _diskProvider = diskProvider; _logger = logger; + _mediaFileService = mediaFileService; } public void Clean(Author author) @@ -32,19 +37,60 @@ public void Clean(Author author) _logger.Debug("Cleaning missing metadata files for author: {0}", author.Name); var metadataFiles = _metadataFileService.GetFilesByAuthor(author.Id); + if (metadataFiles.Count == 0) + { + return; + } + + // Sidecar files are written under ExtraFilePathHelper.GetPreferredBasePath(author, bookFile), so + // for rows that point at a book file, probe that base path first. Probing a fixed audiobook -> + // ebook -> legacy order made every ebook sidecar miss under the audiobook folder first, and a + // miss is the expensive case (FileExists falls back to listing directories). + var bookFilesById = _mediaFileService == null + ? new Dictionary() + : (_mediaFileService.GetFilesByAuthor(author.Id) ?? new List()) + .GroupBy(file => file.Id) + .ToDictionary(group => group.Key, group => group.First()); + + var basePathsByPreferred = new Dictionary>(); + var missingIds = new List(); foreach (var metadataFile in metadataFiles) { - var exists = ExtraFilePathHelper.GetAuthorBasePaths(author) + if (metadataFile.RelativePath.IsNullOrWhiteSpace()) + { + continue; + } + + string preferredBasePath = null; + if (metadataFile.BookFileId.HasValue && bookFilesById.TryGetValue(metadataFile.BookFileId.Value, out var bookFile)) + { + preferredBasePath = ExtraFilePathHelper.GetPreferredBasePath(author, bookFile); + } + + var cacheKey = preferredBasePath ?? string.Empty; + if (!basePathsByPreferred.TryGetValue(cacheKey, out var basePaths)) + { + basePaths = ExtraFilePathHelper.GetAuthorBasePaths(author, preferredBasePath); + basePathsByPreferred[cacheKey] = basePaths; + } + + var exists = basePaths .Select(p => Path.Combine(p, metadataFile.RelativePath)) .Any(_diskProvider.FileExists); if (!exists) { _logger.Debug("Deleting metadata file from database: {0}", metadataFile.RelativePath); - _metadataFileService.Delete(metadataFile.Id); + missingIds.Add(metadataFile.Id); } } + + // One statement for all missing rows instead of one DELETE per row. + if (missingIds.Count > 0) + { + _metadataFileService.DeleteMany(missingIds); + } } } }