From b3eb3df255394fe3aa5712290d832f562a63f733 Mon Sep 17 00:00:00 2001 From: Ch3fUlrich <71930650+Ch3fUlrich@users.noreply.github.com> Date: Fri, 28 Aug 2026 11:03:41 +0000 Subject: [PATCH] fix(author): stop ApplyChanges wiping stored paths when the update omits them Author.ApplyChanges assigned Path, AudiobookRootFolderPath and EbookRootFolderPath unconditionally, so any caller passing an Author without them cleared the stored values. That is inconsistent with the fields immediately around them, which are all guarded -- the quality profiles, the metadata profiles and the tags each check before assigning, with the existing comment noting that "Quality profiles from metadata sources are always null/0". Paths reach the same method from the same two directions: * the API, where a PUT may legitimately omit fields it is not changing, and * metadata refresh, where the incoming Author is built from remote metadata and carries no local paths at all. Both wiped the stored path. Guarding the three assignments the same way the neighbouring fields are guarded fixes it. UseDbFieldsFrom is deliberately left alone: it is a full copy from the database, so assigning unconditionally there is correct. Tests: AuthorApplyChangesPathPreservationFixture covers preservation when the paths are omitted, normal overwrite when they are supplied, and independent preservation when only one of the three changes. Verified red/green -- 2 of the 3 fail without the production change. Full suite 2855 passed / 0 failed (2852 before, +3 new). --- ...thorApplyChangesPathPreservationFixture.cs | 72 +++++++++++++++++++ src/NzbDrone.Core/Books/Model/Author.cs | 22 +++++- 2 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 src/Chaptarr.Core.Test/Books/AuthorApplyChangesPathPreservationFixture.cs diff --git a/src/Chaptarr.Core.Test/Books/AuthorApplyChangesPathPreservationFixture.cs b/src/Chaptarr.Core.Test/Books/AuthorApplyChangesPathPreservationFixture.cs new file mode 100644 index 00000000..e0ba41d1 --- /dev/null +++ b/src/Chaptarr.Core.Test/Books/AuthorApplyChangesPathPreservationFixture.cs @@ -0,0 +1,72 @@ +using NUnit.Framework; +using NzbDrone.Core.Books; + +namespace Chaptarr.Core.Test.Books +{ + [TestFixture] + public class AuthorApplyChangesPathPreservationFixture + { + private static Author StoredAuthor() + { + return new Author + { + Path = "/library/authors/Stored Author", + AudiobookRootFolderPath = "/library/audiobooks", + EbookRootFolderPath = "/library/ebooks" + }; + } + + [Test] + public void should_preserve_stored_paths_when_incoming_author_omits_them() + { + var stored = StoredAuthor(); + + // An update that does not mention paths - e.g. a PUT that only changes + // monitoring, or an Author built from remote metadata during a refresh. + stored.ApplyChanges(new Author()); + + Assert.Multiple(() => + { + Assert.That(stored.Path, Is.EqualTo("/library/authors/Stored Author")); + Assert.That(stored.AudiobookRootFolderPath, Is.EqualTo("/library/audiobooks")); + Assert.That(stored.EbookRootFolderPath, Is.EqualTo("/library/ebooks")); + }); + } + + [Test] + public void should_apply_paths_when_incoming_author_supplies_them() + { + var stored = StoredAuthor(); + + stored.ApplyChanges(new Author + { + Path = "/library/authors/Moved Author", + AudiobookRootFolderPath = "/library/audiobooks-2", + EbookRootFolderPath = "/library/ebooks-2" + }); + + Assert.Multiple(() => + { + Assert.That(stored.Path, Is.EqualTo("/library/authors/Moved Author")); + Assert.That(stored.AudiobookRootFolderPath, Is.EqualTo("/library/audiobooks-2")); + Assert.That(stored.EbookRootFolderPath, Is.EqualTo("/library/ebooks-2")); + }); + } + + [Test] + public void should_preserve_each_path_independently() + { + var stored = StoredAuthor(); + + // Only the audiobook root is being changed; the other two must survive. + stored.ApplyChanges(new Author { AudiobookRootFolderPath = "/library/audiobooks-2" }); + + Assert.Multiple(() => + { + Assert.That(stored.AudiobookRootFolderPath, Is.EqualTo("/library/audiobooks-2")); + Assert.That(stored.Path, Is.EqualTo("/library/authors/Stored Author")); + Assert.That(stored.EbookRootFolderPath, Is.EqualTo("/library/ebooks")); + }); + } + } +} diff --git a/src/NzbDrone.Core/Books/Model/Author.cs b/src/NzbDrone.Core/Books/Model/Author.cs index 18658aa4..7d8381ff 100644 --- a/src/NzbDrone.Core/Books/Model/Author.cs +++ b/src/NzbDrone.Core/Books/Model/Author.cs @@ -271,7 +271,16 @@ public override void UseDbFieldsFrom(Author other) public override void ApplyChanges(Author other) { - Path = other.Path; + // Only overwrite when a value is supplied. ApplyChanges is called both from + // the API -- where a PUT may legitimately omit fields it is not changing -- + // and from metadata refresh, where the incoming Author is built from remote + // metadata and carries no local paths at all. That is the same reason the + // quality and metadata profiles below are guarded; these three were not, so + // an update that omitted them wiped the stored paths. + if (other.Path != null) + { + Path = other.Path; + } // Don't overwrite quality profiles if they're already set // Quality profiles from metadata sources are always null/0 @@ -326,8 +335,15 @@ public override void ApplyChanges(Author other) Tags = (AudiobookTags ?? new HashSet()).Concat(EbookTags ?? new HashSet()).ToHashSet(); AddOptions = other.AddOptions; - AudiobookRootFolderPath = other.AudiobookRootFolderPath; - EbookRootFolderPath = other.EbookRootFolderPath; + if (other.AudiobookRootFolderPath != null) + { + AudiobookRootFolderPath = other.AudiobookRootFolderPath; + } + + if (other.EbookRootFolderPath != null) + { + EbookRootFolderPath = other.EbookRootFolderPath; + } Monitored = other.Monitored; // TRI-STATE MONITORING SYSTEM - Copy only when explicitly provided (not null). // This prevents partial updates from wiping existing monitoring values.