From 80101576f398eda79c10d099befeb5430c29d325 Mon Sep 17 00:00:00 2001 From: Yohan Sanchez Date: Sat, 10 Oct 2026 20:30:25 +0200 Subject: [PATCH 1/2] Keep untracked sidecars with audiobooks when renaming folders Renaming moved audiobook files one by one and only carried the sidecars Chaptarr tracks. Untracked ones (AudioBookShelf cover.jpg/metadata.json, .opf, desc.txt...) stayed in the old folder, and a library watcher saw a new folder fill up file by file: AudioBookShelf created a new item and wrote its own sidecars there before Chaptarr moved the tracked ones, which then failed with "File already exists". When every audiobook file of a folder goes to the same new folder and nothing else lives there, the folder is now moved in one step. If the destination already exists, the sidecars are moved ahead of the audio files, never overwriting an existing file. Tracked metadata and extra files that a folder move already carried are re-pointed instead of failing to move. Co-Authored-By: Claude Opus 5.5 --- .../MetadataServiceSidecarOverwriteFixture.cs | 67 ++++- .../RenameBookFileServiceFixture.cs | 247 +++++++++++++++++- .../Extras/Files/ExtraFileManager.cs | 13 +- .../Extras/Metadata/MetadataService.cs | 15 +- .../MediaFiles/RenameBookFileService.cs | 170 +++++++++++- 5 files changed, 502 insertions(+), 10 deletions(-) diff --git a/src/Chaptarr.Core.Test/Extras/Metadata/MetadataServiceSidecarOverwriteFixture.cs b/src/Chaptarr.Core.Test/Extras/Metadata/MetadataServiceSidecarOverwriteFixture.cs index 8b7d4abb..e4d2ea4d 100644 --- a/src/Chaptarr.Core.Test/Extras/Metadata/MetadataServiceSidecarOverwriteFixture.cs +++ b/src/Chaptarr.Core.Test/Extras/Metadata/MetadataServiceSidecarOverwriteFixture.cs @@ -299,7 +299,33 @@ public void should_keep_image_extra_file_tracking_but_disable_overwrites_when_ex }); } - private static MetadataService CreateSubject(TestDisk disk) + [Test] + public void should_track_metadata_already_carried_by_an_audiobook_folder_move() + { + var disk = new TestDisk(); + disk.TextFiles["/library/A Author/A Book/metadata.json"] = "moved with the folder"; + var consumer = new TestMetadataConsumer(); + disk.TrackedMetadata.Add(new MetadataFile + { + Id = 70, + AuthorId = 10, + BookId = 20, + Consumer = consumer.GetType().Name, + Type = MetadataType.BookMetadata, + RelativePath = "Old Book/metadata.json" + }); + var subject = CreateSubject(disk, consumer); + + subject.MoveFilesAfterRename(BuildAuthor(), new List { BuildBookFile() }); + + Assert.Multiple(() => + { + Assert.That(disk.MovedFiles, Is.Empty); + Assert.That(disk.UpsertedMetadata.Single().RelativePath, Is.EqualTo("A Book/metadata.json")); + }); + } + + private static MetadataService CreateSubject(TestDisk disk, IMetadata consumer = null) { return new MetadataService( configService: null, @@ -307,7 +333,7 @@ private static MetadataService CreateSubject(TestDisk disk) diskTransferService: null, recycleBinProvider: null, otherExtraFileRenamer: NoOp(), - metadataFactory: null, + metadataFactory: consumer == null ? null : MetadataFactoryProxy.Create(consumer), cleanMetadataService: null, httpClient: Throwing(), mediaFileAttributeService: NoOp(), @@ -398,6 +424,8 @@ private class TestDisk public Dictionary BinaryFiles { get; } = new(); public List DeletedMetadataIds { get; } = new(); public List UpsertedMetadata { get; } = new(); + public List TrackedMetadata { get; } = new(); + public List MovedFiles { get; } = new(); public IDiskProvider DiskProvider { @@ -460,6 +488,12 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) return File.OpenRead(tempPath); } + if (targetMethod?.Name == nameof(IDiskProvider.MoveFile)) + { + Disk.MovedFiles.Add(TestDisk.NormalizePath((string)args[0])); + return null; + } + if (targetMethod?.Name == nameof(IDiskProvider.CopyFile)) { var source = TestDisk.NormalizePath((string)args[0]); @@ -484,6 +518,11 @@ private class MetadataFileServiceProxy : DispatchProxy protected override object Invoke(MethodInfo targetMethod, object[] args) { + if (targetMethod?.Name == nameof(IMetadataFileService.GetFilesByAuthor)) + { + return Disk.TrackedMetadata; + } + if (targetMethod?.Name == nameof(IMetadataFileService.Delete)) { Disk.DeletedMetadataIds.Add((int)args[0]); @@ -508,6 +547,28 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) } } + private class MetadataFactoryProxy : DispatchProxy + { + private IMetadata _consumer; + + public static IMetadataFactory Create(IMetadata consumer) + { + var proxy = DispatchProxy.Create(); + ((MetadataFactoryProxy)(object)proxy)._consumer = consumer; + return proxy; + } + + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + if (targetMethod?.Name == nameof(IMetadataFactory.GetAvailableProviders)) + { + return new List { _consumer }; + } + + throw new NotImplementedException($"Test proxy does not implement IMetadataFactory.{targetMethod?.Name}"); + } + } + private class NoOpProxy : DispatchProxy { protected override object Invoke(MethodInfo targetMethod, object[] args) @@ -556,7 +617,7 @@ public string GetFilenameAfterMove(Author author, BookFile bookFile, MetadataFil public string GetFilenameAfterMove(Author author, string bookPath, MetadataFile metadataFile) { - throw new NotImplementedException(); + return Path.Combine(bookPath, Path.GetFileName(metadataFile.RelativePath)); } public MetadataFile FindMetadataFile(Author author, string path) diff --git a/src/Chaptarr.Core.Test/MediaFiles/RenameBookFileServiceFixture.cs b/src/Chaptarr.Core.Test/MediaFiles/RenameBookFileServiceFixture.cs index b835b6af..6991b5a9 100644 --- a/src/Chaptarr.Core.Test/MediaFiles/RenameBookFileServiceFixture.cs +++ b/src/Chaptarr.Core.Test/MediaFiles/RenameBookFileServiceFixture.cs @@ -8,6 +8,7 @@ using NUnit.Framework; using NzbDrone.Common; using NzbDrone.Common.Disk; +using NzbDrone.Common.Extensions; using NzbDrone.Common.Messaging; using NzbDrone.Core.Books; using NzbDrone.Core.CustomFormats; @@ -106,6 +107,7 @@ private sealed class RecordingMoveBookFiles : IMoveBookFiles public Func PlanFactory { get; set; } public List CanonicalRequests { get; } = new(); public List ExecutedPlans { get; } = new(); + public DiskProviderProxy Disk { get; set; } public BookFileMovePlan GetOrganizeDestination(BookFile bookFile, Author author, bool moveToCanonicalAuthorFolder, RenameBatchContext renameBatchContext = null) { @@ -126,6 +128,7 @@ public BookFile MoveBookFile(BookFile bookFile, Author author, BookFileMovePlan var destination = plan.DestinationPath; if (destination != null) { + Disk?.MoveFileTo(bookFile.Path, destination, "MoveBookFile"); bookFile.Path = destination; } @@ -145,11 +148,39 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) } } + private sealed class NullRootFolderWatchingService : IRootFolderWatchingService + { + public void ReportFileSystemChangeBeginning(params string[] paths) + { + } + } + private class DiskProviderProxy : DispatchProxy { public HashSet ExistingFolders { get; } = new(PathEqualityComparer.Instance); + public HashSet ExistingFiles { get; } = new(PathEqualityComparer.Instance); public List RemovedEmptySubfolders { get; } = new(); public List DeletedFolders { get; } = new(); + public List Operations { get; } = new(); + public bool FailFolderMoves { get; set; } + + public void AddFile(string path) + { + ExistingFiles.Add(path); + for (var folder = Path.GetDirectoryName(path); !string.IsNullOrEmpty(folder); folder = Path.GetDirectoryName(folder)) + { + ExistingFolders.Add(folder); + } + } + + public void MoveFileTo(string source, string destination, string operation) + { + Operations.Add($"{operation} {Path.GetFileName(source)}"); + if (ExistingFiles.Remove(source)) + { + AddFile(destination); + } + } protected override object Invoke(MethodInfo targetMethod, object[] args) { @@ -158,6 +189,62 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) return ExistingFolders.Contains((string)args[0]); } + if (targetMethod?.Name == nameof(IDiskProvider.FileExists)) + { + return ExistingFiles.Contains((string)args[0]); + } + + if (targetMethod?.Name == nameof(IDiskProvider.GetDirectories)) + { + var parent = (string)args[0]; + return ExistingFolders.Where(f => Path.GetDirectoryName(f).PathEquals(parent)).ToList(); + } + + if (targetMethod?.Name == nameof(IDiskProvider.CreateFolder)) + { + var folder = (string)args[0]; + Operations.Add($"CreateFolder {folder}"); + for (; !string.IsNullOrEmpty(folder); folder = Path.GetDirectoryName(folder)) + { + ExistingFolders.Add(folder); + } + + return null; + } + + if (targetMethod?.Name == nameof(IDiskProvider.MoveFile)) + { + var destination = (string)args[1]; + if (ExistingFiles.Contains(destination)) + { + throw new FileAlreadyExistsException("File already exists", destination); + } + + MoveFileTo((string)args[0], destination, "MoveFile"); + return null; + } + + if (targetMethod?.Name == nameof(IDiskProvider.MoveFolder)) + { + var source = (string)args[0]; + var destination = (string)args[1]; + if (FailFolderMoves) + { + throw new IOException("Cross-device link"); + } + + Operations.Add($"MoveFolder {source} -> {destination}"); + foreach (var file in ExistingFiles.Where(f => source.IsParentPath(f)).ToList()) + { + ExistingFiles.Remove(file); + AddFile(Path.Combine(destination, Path.GetRelativePath(source, file))); + } + + ExistingFolders.RemoveWhere(f => f.PathEquals(source) || source.IsParentPath(f)); + ExistingFolders.Add(destination); + return null; + } + if (targetMethod?.Name == nameof(IDiskProvider.RemoveEmptySubfolders)) { RemovedEmptySubfolders.Add((string)args[0]); @@ -166,7 +253,11 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) if (targetMethod?.Name == nameof(IDiskProvider.GetFiles)) { - return Enumerable.Empty(); + var folder = (string)args[0]; + var recursive = (bool)args[1]; + return ExistingFiles + .Where(f => recursive ? folder.IsParentPath(f) : Path.GetDirectoryName(f).PathEquals(folder)) + .ToList(); } if (targetMethod?.Name == nameof(IDiskProvider.DeleteFolder)) @@ -207,6 +298,7 @@ private static RenameBookFileService CreateService( mover, eventAggregator, diskProvider ?? DispatchProxy.Create(), + new NullRootFolderWatchingService(), LogManager.GetCurrentClassLogger()); } @@ -451,6 +543,159 @@ public void should_remove_only_empty_directories_bounded_by_the_actual_source_au Assert.That(diskProxy.DeletedFolders, Does.Not.Contain(rootFolder)); } + private const string HobbFolder = "/audiobooks/Robin Hobb"; + private const string OldBookFolder = HobbFolder + "/L'Assassin Royal/L'apprenti assassin"; + private const string NewBookFolder = HobbFolder + "/L'Assassin royal/01 - L'apprenti assassin"; + + private static (RenameBookFileService Service, DiskProviderProxy Disk, RecordingMoveBookFiles Mover, RecordingEventAggregator Events, StubMediaFileService Files) + CreateAudiobookFolderScenario(List files, Func destinationFactory) + { + var author = new Author { Id = 1, Name = "Robin Hobb", Path = HobbFolder, AudiobookPath = HobbFolder }; + var mediaFileService = new StubMediaFileService { Files = files }; + var diskProvider = DispatchProxy.Create(); + var disk = (DiskProviderProxy)(object)diskProvider; + foreach (var file in files) + { + disk.AddFile(file.Path); + } + + var mover = new RecordingMoveBookFiles + { + Disk = disk, + PlanFactory = (file, _) => new BookFileMovePlan + { + CanOrganize = true, + SourceAuthorFolderPath = HobbFolder, + DestinationAuthorFolderPath = HobbFolder, + DestinationPath = destinationFactory(file) + } + }; + var events = new RecordingEventAggregator(); + + return (CreateService(author, mediaFileService, mover, events, diskProvider), disk, mover, events, mediaFileService); + } + + private static List TwoTrackAudiobook() + { + return new List + { + BookFile(1, OldBookFolder + "/01 Prologue.mp3", Quality.MP3, "audiobook"), + BookFile(2, OldBookFolder + "/02 Le nouveau.mp3", Quality.MP3, "audiobook") + }; + } + + [Test] + public void should_move_the_whole_audiobook_folder_so_untracked_sidecars_follow() + { + var files = TwoTrackAudiobook(); + var scenario = CreateAudiobookFolderScenario(files, f => Path.Combine(NewBookFolder, Path.GetFileName(f.Path))); + scenario.Disk.AddFile(OldBookFolder + "/cover.jpg"); + scenario.Disk.AddFile(OldBookFolder + "/metadata.json"); + + scenario.Service.Execute(new RenameFilesCommand(1, new List { 1, 2 })); + + Assert.That(scenario.Disk.Operations, Is.EqualTo(new[] { $"CreateFolder {HobbFolder}/L'Assassin royal", $"MoveFolder {OldBookFolder} -> {NewBookFolder}" })); + Assert.That(scenario.Mover.MovedFileIds, Is.Empty); + Assert.That(files.Select(f => f.Path), Is.EqualTo(new[] { NewBookFolder + "/01 Prologue.mp3", NewBookFolder + "/02 Le nouveau.mp3" })); + Assert.That(scenario.Disk.ExistingFiles, Does.Contain(NewBookFolder + "/cover.jpg")); + Assert.That(scenario.Disk.ExistingFiles, Does.Contain(NewBookFolder + "/metadata.json")); + Assert.That(scenario.Files.Updated.Select(f => f.Id), Is.EqualTo(new[] { 1, 2 })); + + var renamed = scenario.Events.Events.OfType().Single().RenamedFiles; + Assert.That(renamed.Select(r => r.PreviousPath), Is.EqualTo(new[] { OldBookFolder + "/01 Prologue.mp3", OldBookFolder + "/02 Le nouveau.mp3" })); + Assert.That(scenario.Events.Events.OfType().Count(), Is.EqualTo(2)); + } + + [Test] + public void should_still_rename_a_file_whose_name_changes_inside_the_moved_folder() + { + var files = TwoTrackAudiobook(); + var scenario = CreateAudiobookFolderScenario(files, f => Path.Combine(NewBookFolder, f.Id == 1 ? "01 - Prologue.mp3" : Path.GetFileName(f.Path))); + + scenario.Service.Execute(new RenameFilesCommand(1, new List { 1, 2 })); + + Assert.That(scenario.Disk.Operations, Does.Contain($"MoveFolder {OldBookFolder} -> {NewBookFolder}")); + Assert.That(scenario.Mover.MovedFileIds, Is.EqualTo(new[] { 1 })); + Assert.That(files[0].Path, Is.EqualTo(NewBookFolder + "/01 - Prologue.mp3")); + Assert.That(files[1].Path, Is.EqualTo(NewBookFolder + "/02 Le nouveau.mp3")); + Assert.That(scenario.Events.Events.OfType().Single().RenamedFiles.Single(r => r.BookFile.Id == 1).PreviousPath, + Is.EqualTo(OldBookFolder + "/01 Prologue.mp3")); + } + + [Test] + public void should_move_sidecars_before_audio_when_destination_folder_already_exists() + { + var files = TwoTrackAudiobook(); + var scenario = CreateAudiobookFolderScenario(files, f => Path.Combine(NewBookFolder, Path.GetFileName(f.Path))); + scenario.Disk.AddFile(OldBookFolder + "/cover.jpg"); + scenario.Disk.AddFile(OldBookFolder + "/metadata.json"); + scenario.Disk.AddFile(NewBookFolder + "/metadata.json"); + + scenario.Service.Execute(new RenameFilesCommand(1, new List { 1, 2 })); + + Assert.That(scenario.Disk.Operations, Is.EqualTo(new[] + { + "MoveFile cover.jpg", + "MoveBookFile 01 Prologue.mp3", + "MoveBookFile 02 Le nouveau.mp3" + })); + Assert.That(scenario.Disk.ExistingFiles, Does.Contain(NewBookFolder + "/cover.jpg")); + Assert.That(scenario.Disk.ExistingFiles, Does.Contain(OldBookFolder + "/metadata.json"), "an existing destination file is never overwritten"); + } + + [Test] + public void should_leave_the_folder_alone_when_another_book_file_stays_behind() + { + var files = TwoTrackAudiobook(); + var scenario = CreateAudiobookFolderScenario(files, f => Path.Combine(NewBookFolder, Path.GetFileName(f.Path))); + scenario.Disk.AddFile(OldBookFolder + "/cover.jpg"); + + scenario.Service.Execute(new RenameFilesCommand(1, new List { 1 })); + + Assert.That(scenario.Disk.Operations, Is.EqualTo(new[] { "MoveBookFile 01 Prologue.mp3" })); + Assert.That(scenario.Disk.ExistingFiles, Does.Contain(OldBookFolder + "/cover.jpg")); + } + + [Test] + public void should_leave_the_folder_alone_when_it_holds_untracked_media() + { + var files = TwoTrackAudiobook(); + var scenario = CreateAudiobookFolderScenario(files, f => Path.Combine(NewBookFolder, Path.GetFileName(f.Path))); + scenario.Disk.AddFile(OldBookFolder + "/bonus.mp3"); + scenario.Disk.AddFile(OldBookFolder + "/cover.jpg"); + + scenario.Service.Execute(new RenameFilesCommand(1, new List { 1, 2 })); + + Assert.That(scenario.Mover.MovedFileIds, Is.EqualTo(new[] { 1, 2 })); + Assert.That(scenario.Disk.Operations, Has.None.StartsWith("MoveFolder").And.None.StartsWith("MoveFile ")); + } + + [Test] + public void should_leave_the_folder_alone_when_files_split_across_destinations() + { + var files = TwoTrackAudiobook(); + var scenario = CreateAudiobookFolderScenario(files, f => Path.Combine(f.Id == 1 ? NewBookFolder : NewBookFolder + " (2)", Path.GetFileName(f.Path))); + scenario.Disk.AddFile(OldBookFolder + "/cover.jpg"); + + scenario.Service.Execute(new RenameFilesCommand(1, new List { 1, 2 })); + + Assert.That(scenario.Mover.MovedFileIds, Is.EqualTo(new[] { 1, 2 })); + Assert.That(scenario.Disk.ExistingFiles, Does.Contain(OldBookFolder + "/cover.jpg")); + } + + [Test] + public void should_fall_back_to_moving_files_one_by_one_when_the_folder_move_fails() + { + var files = TwoTrackAudiobook(); + var scenario = CreateAudiobookFolderScenario(files, f => Path.Combine(NewBookFolder, Path.GetFileName(f.Path))); + scenario.Disk.FailFolderMoves = true; + + scenario.Service.Execute(new RenameFilesCommand(1, new List { 1, 2 })); + + Assert.That(scenario.Mover.MovedFileIds, Is.EqualTo(new[] { 1, 2 })); + Assert.That(files.Select(f => f.Path), Is.EqualTo(new[] { NewBookFolder + "/01 Prologue.mp3", NewBookFolder + "/02 Le nouveau.mp3" })); + } + [Test] public void should_report_boundary_skips_separately() { diff --git a/src/NzbDrone.Core/Extras/Files/ExtraFileManager.cs b/src/NzbDrone.Core/Extras/Files/ExtraFileManager.cs index c5aaa979..8608cf01 100644 --- a/src/NzbDrone.Core/Extras/Files/ExtraFileManager.cs +++ b/src/NzbDrone.Core/Extras/Files/ExtraFileManager.cs @@ -111,7 +111,7 @@ protected TExtraFile MoveFile(Author author, BookFile bookFile, TExtraFile extra { _logger.Trace("Renaming extra file: {0} to {1}", extraFile, newFileName); - _diskProvider.MoveFile(existingFileName, newFileName); + MoveFileAfterRename(existingFileName, newFileName); if (!ExtraFilePathHelper.TryGetRelativePath(author, newFileName, out var relativePath, out _, preferredBase)) { @@ -158,6 +158,17 @@ protected TExtraFile MoveFile(Author author, BookFile bookFile, TExtraFile extra return null; } + // The file is already at its new path when the whole audiobook folder was moved during the rename. + private void MoveFileAfterRename(string existingFileName, string newFileName) + { + if (!_diskProvider.FileExists(existingFileName) && _diskProvider.FileExists(newFileName)) + { + return; + } + + _diskProvider.MoveFile(existingFileName, newFileName); + } + private string GetAvailableFilename(string destinationPath) { if (destinationPath.IsNullOrWhiteSpace()) diff --git a/src/NzbDrone.Core/Extras/Metadata/MetadataService.cs b/src/NzbDrone.Core/Extras/Metadata/MetadataService.cs index 6f3326b5..350953c0 100644 --- a/src/NzbDrone.Core/Extras/Metadata/MetadataService.cs +++ b/src/NzbDrone.Core/Extras/Metadata/MetadataService.cs @@ -160,7 +160,7 @@ public override IEnumerable MoveFilesAfterRename(Author author, List< { try { - _diskProvider.MoveFile(existingFileName, newFileName); + MoveFileAfterRename(existingFileName, newFileName); if (!ExtraFilePathHelper.TryGetRelativePath(author, newFileName, out var relativePath, out _, preferredBase)) { @@ -192,7 +192,7 @@ public override IEnumerable MoveFilesAfterRename(Author author, List< { try { - _diskProvider.MoveFile(existingFileName, newFileName); + MoveFileAfterRename(existingFileName, newFileName); if (!ExtraFilePathHelper.TryGetRelativePath(author, newFileName, out var relativePath, out _, preferredBase)) { @@ -221,6 +221,17 @@ public override ExtraFile Import(Author author, BookFile bookFile, string path, return null; } + // The file is already at its new path when the whole audiobook folder was moved during the rename. + private void MoveFileAfterRename(string existingFileName, string newFileName) + { + if (!_diskProvider.FileExists(existingFileName) && _diskProvider.FileExists(newFileName)) + { + return; + } + + _diskProvider.MoveFile(existingFileName, newFileName); + } + private List GetMetadataFilesForConsumer(IMetadata consumer, List authorMetadata) { return authorMetadata.Where(c => c.Consumer == consumer.GetType().Name).ToList(); diff --git a/src/NzbDrone.Core/MediaFiles/RenameBookFileService.cs b/src/NzbDrone.Core/MediaFiles/RenameBookFileService.cs index 0a3b6aee..ebfd9b68 100644 --- a/src/NzbDrone.Core/MediaFiles/RenameBookFileService.cs +++ b/src/NzbDrone.Core/MediaFiles/RenameBookFileService.cs @@ -40,6 +40,7 @@ public class RenameBookFileService : IRenameBookFileService, IExecute bookFiles, Author author, s .ToList(); var batchContext = new RenameBatchContext(); + var previousPaths = ordered.ToDictionary(f => f.Id, f => f.Path); + + // Audiobook destinations only depend on naming, so they can be planned up front; ebook plans + // still wait for the audiobook moves recorded in the batch context. + var audiobookPlans = ordered + .Where(IsAudiobook) + .ToDictionary(f => f.Id, f => _bookFileMover.GetOrganizeDestination(f, author, moveToCanonicalAuthorFolder, batchContext)); + var relocatedFileIds = RelocateAudiobookFolders(ordered, allFiles, audiobookPlans); foreach (var bookFile in ordered) { - var previousPath = bookFile.Path; - var plan = _bookFileMover.GetOrganizeDestination(bookFile, author, moveToCanonicalAuthorFolder, batchContext); + var previousPath = previousPaths[bookFile.Id]; + var plan = audiobookPlans.TryGetValue(bookFile.Id, out var audiobookPlan) + ? audiobookPlan + : _bookFileMover.GetOrganizeDestination(bookFile, author, moveToCanonicalAuthorFolder, batchContext); if (!plan.CanOrganize) { result.BoundarySkippedCount++; @@ -272,7 +285,12 @@ private RenameFilesResult RenameFiles(List bookFiles, Author author, s try { _logger.Debug("Organizing book file: {0}", bookFile); - _bookFileMover.MoveBookFile(bookFile, author, plan, batchContext); + + // A file carried by a folder move is already in place unless its own name changes too. + if (!relocatedFileIds.Contains(bookFile.Id) || bookFile.Path.PathNotEquals(plan.DestinationPath)) + { + _bookFileMover.MoveBookFile(bookFile, author, plan, batchContext); + } _mediaFileService.Update(bookFile); TrackAudiobookFolderMove(batchContext, bookFile, previousPath); @@ -335,6 +353,152 @@ private RenameFilesResult RenameFiles(List bookFiles, Author author, s return result; } + // When every file of an audiobook folder goes to the same new folder, the folder is moved as a whole: + // sidecars Chaptarr does not track (AudioBookShelf cover.jpg/metadata.json, .opf, desc.txt...) stay with + // the book, and library watchers see one rename instead of a new folder filling up file by file. If the + // folder cannot be moved in one step, the sidecars are moved ahead of the audio files instead. + private HashSet RelocateAudiobookFolders(List ordered, List allFiles, Dictionary plans) + { + var relocatedFileIds = new HashSet(); + + var groups = ordered + .Where(f => plans.TryGetValue(f.Id, out var plan) && + plan.CanOrganize && + plan.ReplicaPaths == null && + !plan.ShouldCleanupReplicas && + plan.DestinationPath.IsNotNullOrWhiteSpace() && + f.Path.IsNotNullOrWhiteSpace()) + .GroupBy(f => Path.GetDirectoryName(f.Path), PathEqualityComparer.Instance); + + foreach (var group in groups) + { + var oldFolder = group.Key; + var files = group.ToList(); + var newFolders = files + .Select(f => Path.GetDirectoryName(plans[f.Id].DestinationPath)) + .Distinct(PathEqualityComparer.Instance) + .ToList(); + + if (newFolders.Count != 1) + { + continue; + } + + var newFolder = newFolders[0]; + + try + { + if (!CanRelocateFolder(oldFolder, newFolder, plans[files[0].Id].SourceAuthorFolderPath, files, allFiles)) + { + continue; + } + + if (!_diskProvider.FolderExists(newFolder) && !_diskProvider.GetDirectories(oldFolder).Any()) + { + var parentFolder = Path.GetDirectoryName(newFolder); + if (parentFolder.IsNotNullOrWhiteSpace() && !_diskProvider.FolderExists(parentFolder)) + { + _diskProvider.CreateFolder(parentFolder); + } + + _rootFolderWatchingService.ReportFileSystemChangeBeginning(oldFolder, newFolder); + _diskProvider.MoveFolder(oldFolder, newFolder); + _logger.Debug("Moved audiobook folder {0} to {1}", oldFolder, newFolder); + + foreach (var file in files) + { + file.Path = Path.Combine(newFolder, Path.GetFileName(file.Path)); + relocatedFileIds.Add(file.Id); + } + } + else + { + MoveSidecarFiles(oldFolder, newFolder); + } + } + catch (Exception ex) + { + _logger.Warn(ex, "Unable to move audiobook folder {0} to {1} as a whole, moving its files one by one", oldFolder, newFolder); + } + } + + return relocatedFileIds; + } + + private bool CanRelocateFolder(string oldFolder, string newFolder, string sourceAuthorFolder, List files, List allFiles) + { + // Only book folders below the author folder, never the author folder itself or a case-only change. + if (sourceAuthorFolder.IsNullOrWhiteSpace() || + !sourceAuthorFolder.IsParentPath(oldFolder) || + string.Equals(oldFolder, newFolder, StringComparison.OrdinalIgnoreCase) || + oldFolder.IsParentPath(newFolder) || + newFolder.IsParentPath(oldFolder)) + { + return false; + } + + // Every known file in or below the folder has to be part of this move (no ebook or other book left behind). + var movingIds = files.Select(f => f.Id).ToHashSet(); + if (allFiles.Any(f => f.Path.IsNotNullOrWhiteSpace() && + !movingIds.Contains(f.Id) && + (Path.GetDirectoryName(f.Path).PathEquals(oldFolder) || oldFolder.IsParentPath(f.Path)))) + { + return false; + } + + if (!_diskProvider.FolderExists(oldFolder)) + { + return false; + } + + // The media files on disk must be exactly the ones being moved: an untracked one would be carried + // along silently, and a missing one means the database and the disk disagree. + var movingPaths = files.Select(f => f.Path).ToHashSet(PathEqualityComparer.Instance); + return movingPaths.SetEquals(_diskProvider.GetFiles(oldFolder, false).Where(IsMediaFile)); + } + + private void MoveSidecarFiles(string oldFolder, string newFolder) + { + var sidecars = _diskProvider.GetFiles(oldFolder, false) + .Where(path => !IsMediaFile(path)) + .ToList(); + + if (sidecars.Count == 0) + { + return; + } + + if (!_diskProvider.FolderExists(newFolder)) + { + _diskProvider.CreateFolder(newFolder); + } + + foreach (var sidecar in sidecars) + { + var destination = Path.Combine(newFolder, Path.GetFileName(sidecar)); + + if (_diskProvider.FileExists(destination)) + { + _logger.Debug("Keeping sidecar {0} in place, {1} already exists", sidecar, destination); + continue; + } + + _rootFolderWatchingService.ReportFileSystemChangeBeginning(sidecar, destination); + _diskProvider.MoveFile(sidecar, destination); + _logger.Debug("Moved sidecar {0} to {1}", sidecar, destination); + } + } + + private static bool IsMediaFile(string path) + { + return MediaFileExtensions.AllExtensions.Contains(Path.GetExtension(path)); + } + + private static bool IsAudiobook(BookFile file) + { + return string.Equals(GetEffectiveMediaType(file), "audiobook", StringComparison.OrdinalIgnoreCase); + } + private static void TrackAudiobookFolderMove(RenameBatchContext batchContext, BookFile bookFile, string previousPath) { if (batchContext == null || !string.Equals(GetEffectiveMediaType(bookFile), "audiobook", StringComparison.OrdinalIgnoreCase)) From 20358f7c2172a9673b5ed34e66068757c15ee5a9 Mon Sep 17 00:00:00 2001 From: Yohan Sanchez Date: Sat, 10 Oct 2026 20:45:09 +0200 Subject: [PATCH 2/2] Rename extras that a folder move carried under their old name Extras are renamed after the book file (e.g. ".jpg"). After a folder move, the tracked file sits in the new folder under its old name, so looking only for the new name failed with FileNotFoundException and left the extra's stored path stale. Look for the carried file under its old name and rename it in place when needed. Co-Authored-By: Claude Opus 5.5 --- .../Others/OtherExtraServiceRenameFixture.cs | 125 ++++++++++++++++++ .../Extras/Files/ExtraFileManager.cs | 23 +++- .../Extras/Metadata/MetadataService.cs | 21 ++- 3 files changed, 160 insertions(+), 9 deletions(-) create mode 100644 src/Chaptarr.Core.Test/Extras/Others/OtherExtraServiceRenameFixture.cs diff --git a/src/Chaptarr.Core.Test/Extras/Others/OtherExtraServiceRenameFixture.cs b/src/Chaptarr.Core.Test/Extras/Others/OtherExtraServiceRenameFixture.cs new file mode 100644 index 00000000..83163e24 --- /dev/null +++ b/src/Chaptarr.Core.Test/Extras/Others/OtherExtraServiceRenameFixture.cs @@ -0,0 +1,125 @@ +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.Others; +using NzbDrone.Core.MediaFiles; + +namespace Chaptarr.Core.Test.Extras.Others +{ + [TestFixture] + public class OtherExtraServiceRenameFixture + { + private const string AuthorFolder = "/library/Zogarth"; + private const string NewBookFolder = AuthorFolder + "/The Primal Hunter/01 - Primal Hunter 1"; + + [Test] + public void should_rename_an_extra_that_a_folder_move_carried_under_its_old_name() + { + var disk = new FakeDisk(); + disk.Files.Add(NewBookFolder + "/Primal Hunter 1.jpg"); + disk.Files.Add(NewBookFolder + "/Primal Hunter 1 - Tome 1.m4b"); + var extraFiles = new FakeOtherExtraFileService(); + extraFiles.Files.Add(new OtherExtraFile + { + Id = 5, + AuthorId = 1, + BookFileId = 40, + RelativePath = "Primal Hunter/Primal Hunter 1 - Tome1/Primal Hunter 1.jpg", + Extension = ".jpg" + }); + var subject = new OtherExtraService(null, disk.Proxy, null, extraFiles.Proxy, null, LogManager.GetCurrentClassLogger()); + var author = new Author { Id = 1, Name = "Zogarth", Path = AuthorFolder, AudiobookPath = AuthorFolder }; + var bookFile = new BookFile { Id = 40, Path = NewBookFolder + "/Primal Hunter 1 - Tome 1.m4b" }; + + subject.MoveFilesAfterRename(author, new List { bookFile }); + + Assert.Multiple(() => + { + Assert.That(disk.Moves, Is.EqualTo(new[] { $"{NewBookFolder}/Primal Hunter 1.jpg -> {NewBookFolder}/Primal Hunter 1 - Tome 1.jpg" })); + Assert.That(extraFiles.Upserted.Single().RelativePath, Is.EqualTo("The Primal Hunter/01 - Primal Hunter 1/Primal Hunter 1 - Tome 1.jpg")); + }); + } + + private sealed class FakeDisk + { + public HashSet Files { get; } = new(); + public List Moves { get; } = new(); + + public IDiskProvider Proxy + { + get + { + var proxy = DispatchProxy.Create(); + ((DiskProxy)(object)proxy).Disk = this; + return proxy; + } + } + } + + private class DiskProxy : DispatchProxy + { + public FakeDisk Disk { get; set; } + + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + switch (targetMethod?.Name) + { + case nameof(IDiskProvider.FileExists): + return Disk.Files.Contains((string)args[0]); + case nameof(IDiskProvider.MoveFile): + var source = (string)args[0]; + if (!Disk.Files.Remove(source)) + { + throw new System.IO.FileNotFoundException("File doesn't exist: " + source); + } + + Disk.Files.Add((string)args[1]); + Disk.Moves.Add($"{source} -> {args[1]}"); + return null; + default: + throw new NotImplementedException($"Test proxy does not implement IDiskProvider.{targetMethod?.Name}"); + } + } + } + + private sealed class FakeOtherExtraFileService + { + public List Files { get; } = new(); + public List Upserted { get; } = new(); + + public IOtherExtraFileService Proxy + { + get + { + var proxy = DispatchProxy.Create(); + ((ExtraFileServiceProxy)(object)proxy).Service = this; + return proxy; + } + } + } + + private class ExtraFileServiceProxy : DispatchProxy + { + public FakeOtherExtraFileService Service { get; set; } + + protected override object Invoke(MethodInfo targetMethod, object[] args) + { + switch (targetMethod?.Name) + { + case nameof(IOtherExtraFileService.GetFilesByAuthor): + return Service.Files; + case nameof(IOtherExtraFileService.Upsert) when args[0] is List files: + Service.Upserted.AddRange(files); + return null; + default: + throw new NotImplementedException($"Test proxy does not implement IOtherExtraFileService.{targetMethod?.Name}"); + } + } + } + } +} diff --git a/src/NzbDrone.Core/Extras/Files/ExtraFileManager.cs b/src/NzbDrone.Core/Extras/Files/ExtraFileManager.cs index 8608cf01..db911ae2 100644 --- a/src/NzbDrone.Core/Extras/Files/ExtraFileManager.cs +++ b/src/NzbDrone.Core/Extras/Files/ExtraFileManager.cs @@ -131,7 +131,7 @@ protected TExtraFile MoveFile(Author author, BookFile bookFile, TExtraFile extra var availableName = GetAvailableFilename(newFileName); _logger.Debug("Destination file exists, moving extra file to alternate name: {0}", availableName); - _diskProvider.MoveFile(existingFileName, availableName); + _diskProvider.MoveFile(ResolveCarriedFile(existingFileName, newFileName), availableName); if (!ExtraFilePathHelper.TryGetRelativePath(author, availableName, out var relativePath, out _, preferredBase)) { @@ -158,15 +158,28 @@ protected TExtraFile MoveFile(Author author, BookFile bookFile, TExtraFile extra return null; } - // The file is already at its new path when the whole audiobook folder was moved during the rename. + // When the whole audiobook folder was moved during the rename, the file already sits in the new folder + // under its old name: only rename it there if its name changes. private void MoveFileAfterRename(string existingFileName, string newFileName) { - if (!_diskProvider.FileExists(existingFileName) && _diskProvider.FileExists(newFileName)) + var source = ResolveCarriedFile(existingFileName, newFileName); + + if (source.PathNotEquals(newFileName)) { - return; + _diskProvider.MoveFile(source, newFileName); } + } + + private string ResolveCarriedFile(string existingFileName, string newFileName) + { + if (_diskProvider.FileExists(existingFileName)) + { + return existingFileName; + } + + var carried = Path.Combine(Path.GetDirectoryName(newFileName), Path.GetFileName(existingFileName)); - _diskProvider.MoveFile(existingFileName, newFileName); + return _diskProvider.FileExists(carried) ? carried : existingFileName; } private string GetAvailableFilename(string destinationPath) diff --git a/src/NzbDrone.Core/Extras/Metadata/MetadataService.cs b/src/NzbDrone.Core/Extras/Metadata/MetadataService.cs index 350953c0..55aab7ba 100644 --- a/src/NzbDrone.Core/Extras/Metadata/MetadataService.cs +++ b/src/NzbDrone.Core/Extras/Metadata/MetadataService.cs @@ -221,15 +221,28 @@ public override ExtraFile Import(Author author, BookFile bookFile, string path, return null; } - // The file is already at its new path when the whole audiobook folder was moved during the rename. + // When the whole audiobook folder was moved during the rename, the file already sits in the new folder + // under its old name: only rename it there if its name changes. private void MoveFileAfterRename(string existingFileName, string newFileName) { - if (!_diskProvider.FileExists(existingFileName) && _diskProvider.FileExists(newFileName)) + var source = ResolveCarriedFile(existingFileName, newFileName); + + if (source.PathNotEquals(newFileName)) { - return; + _diskProvider.MoveFile(source, newFileName); + } + } + + private string ResolveCarriedFile(string existingFileName, string newFileName) + { + if (_diskProvider.FileExists(existingFileName)) + { + return existingFileName; } - _diskProvider.MoveFile(existingFileName, newFileName); + var carried = Path.Combine(Path.GetDirectoryName(newFileName), Path.GetFileName(existingFileName)); + + return _diskProvider.FileExists(carried) ? carried : existingFileName; } private List GetMetadataFilesForConsumer(IMetadata consumer, List authorMetadata)