From a11c871f60b05ff72a4568b9422172372e704e4e Mon Sep 17 00:00:00 2001 From: m4bard <304653687+m4bard@users.noreply.github.com> Date: Tue, 14 Jul 2026 15:05:33 -0500 Subject: [PATCH] fix(scan): don't attribute every book by an author to whichever one is scanned ScanFileDiscovery.Matches accepted "the path contains the author" as sufficient to attribute a file to a specific audiobook. In the usual {Author}/... layout every file beneath an author's folder contains that author's name, so the clause was true for all of them. FindMatchingAudioFiles takes every file in any directory group where any file matches, so each of the author's book folders was claimed whole, and ScanPathPlanner.CalculateBasePath then reduced the result to its common parent -- the author folder. That became the audiobook's BasePath, which is what rename and move operate on. This is reached whenever an audiobook has no BasePath, because the scan root then falls back to the configured OutputPath and the whole library becomes candidates. A BasePath is empty in normal use: it is only assigned on add when a destination path is supplied, and ScanJobProcessor sets it to null when the stored folder no longer exists. The author names a shelf, not a book. Attribution by path now requires the title, in the filename or in the containing directory. A path carrying neither the title nor anything else identifying is left unmatched rather than attached to an arbitrary book by the same author -- a miss is recoverable and visible, a wrong link is neither. Layouts that encode neither title nor author in the path go from wrongly matched to unmatched. Those are what an embedded-tag pass should claim. No existing test depended on author-only matching. Fixes #765 --- .../Library/Scanning/ScanFileDiscovery.cs | 39 ++-- .../Scanning/ScanFileDiscoveryReproTests.cs | 166 ++++++++++++++++++ 2 files changed, 193 insertions(+), 12 deletions(-) create mode 100644 tests/Features/Infrastructure/Library/Scanning/ScanFileDiscoveryReproTests.cs diff --git a/listenarr.infrastructure/Library/Scanning/ScanFileDiscovery.cs b/listenarr.infrastructure/Library/Scanning/ScanFileDiscovery.cs index 7ce228c6f..5effa4128 100644 --- a/listenarr.infrastructure/Library/Scanning/ScanFileDiscovery.cs +++ b/listenarr.infrastructure/Library/Scanning/ScanFileDiscovery.cs @@ -33,8 +33,7 @@ public static List FindMatchingAudioFiles( foreach (var group in candidates.GroupBy(file => Path.GetDirectoryName(file) ?? string.Empty)) { var directoryName = Path.GetFileName(group.Key) ?? string.Empty; - var groupHasMatch = group.Any(file => - Matches(file, directoryName, titleToken, authorToken)); + var groupHasMatch = group.Any(file => Matches(file, directoryName, titleToken)); if (groupHasMatch) { foundFiles.AddRange(group.Where(unique.Add)); @@ -42,7 +41,7 @@ public static List FindMatchingAudioFiles( } foreach (var file in group.Where(file => - Matches(file, directoryName: string.Empty, titleToken, authorToken))) + Matches(file, directoryName: string.Empty, titleToken))) { if (unique.Add(file)) { @@ -103,20 +102,36 @@ private static List CollectCandidates(string scanRoot, Guid jobId, ILogg return candidates; } + /// + /// Decides whether a file can be attributed to a specific audiobook from its path alone. + /// + /// Only the TITLE can do this. The author names a shelf, not a book: in the usual + /// {Author}/... layout every file beneath an author's folder contains that author's + /// name, so accepting "the path contains the author" as a match attributes every book by + /// that author to whichever one is being scanned -- and the resulting common parent is the + /// author folder, which then becomes the audiobook's BasePath. + /// + /// + /// A path that carries neither the title nor anything else identifying is simply not + /// attributable by path, and is left for the embedded-tag pass to claim. Leaving a file + /// unmatched is recoverable; attaching it to the wrong book is not. + /// + /// private static bool Matches( string file, string directoryName, - string titleToken, - string authorToken) + string titleToken) { - var fileNameMatchesTitle = !string.IsNullOrEmpty(titleToken) - && Path.GetFileNameWithoutExtension(file) - .Contains(titleToken, StringComparison.OrdinalIgnoreCase); - var filePathMatchesAuthor = !string.IsNullOrEmpty(authorToken) - && file.Contains(authorToken, StringComparison.OrdinalIgnoreCase); + if (string.IsNullOrEmpty(titleToken)) + { + return false; + } + + var fileNameMatchesTitle = Path.GetFileNameWithoutExtension(file) + .Contains(titleToken, StringComparison.OrdinalIgnoreCase); var directoryMatchesTitle = !string.IsNullOrEmpty(directoryName) - && !string.IsNullOrEmpty(titleToken) && directoryName.Contains(titleToken, StringComparison.OrdinalIgnoreCase); - return fileNameMatchesTitle || filePathMatchesAuthor || directoryMatchesTitle; + + return fileNameMatchesTitle || directoryMatchesTitle; } } diff --git a/tests/Features/Infrastructure/Library/Scanning/ScanFileDiscoveryReproTests.cs b/tests/Features/Infrastructure/Library/Scanning/ScanFileDiscoveryReproTests.cs new file mode 100644 index 000000000..707afd38d --- /dev/null +++ b/tests/Features/Infrastructure/Library/Scanning/ScanFileDiscoveryReproTests.cs @@ -0,0 +1,166 @@ +/* + * Listenarr - Audiobook Management System + * Copyright (C) 2024-2026 Listenarr Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace Listenarr.Tests.Features.Infrastructure.Library.Scanning +{ + /// + /// Path attribution in ScanFileDiscovery, exercised against real directory trees. + /// + /// When an audiobook has no BasePath the scan root falls back to the whole library, so every + /// audio file under it becomes a candidate and attribution rests entirely on the path. + /// Books used below are public domain (H. Rider Haggard, Jules Verne). + /// + public class ScanFileDiscoveryReproTests : IDisposable + { + private readonly string _root; + + public ScanFileDiscoveryReproTests() + { + _root = Path.Combine(Path.GetTempPath(), "listenarr-scan-" + Guid.NewGuid().ToString("N")); + } + + public void Dispose() + { + try { Directory.Delete(_root, recursive: true); } catch { /* best effort */ } + GC.SuppressFinalize(this); + } + + private void AddBook(string relativeFolder, params string[] fileNames) + { + var dir = Path.Combine(_root, relativeFolder); + Directory.CreateDirectory(dir); + foreach (var name in fileNames) + { + File.WriteAllText(Path.Combine(dir, name), "not really audio"); + } + } + + private static Audiobook Book(string title, string author) => new() + { + Title = title, + Authors = new List { author }, + }; + + private List Scan(Audiobook audiobook) => + ScanFileDiscovery.FindMatchingAudioFiles(_root, audiobook, Guid.NewGuid(), NullLogger.Instance); + + private static List Names(IEnumerable paths) => + paths.Select(Path.GetFileName).OrderBy(n => n, StringComparer.Ordinal).ToList()!; + + // ----------------------------------------------------------------- + // The bug: the author names a shelf, not a book. + // ----------------------------------------------------------------- + + [Fact] + public void ScanningOneBook_DoesNotClaimTheOtherBooksByTheSameAuthor() + { + AddBook("H. Rider Haggard/H. Rider Haggard - She", "She.m4b"); + AddBook("H. Rider Haggard/H. Rider Haggard - Allan Quatermain", "Allan Quatermain.m4b"); + AddBook("H. Rider Haggard/H. Rider Haggard - King Solomon's Mines", "King Solomon's Mines.m4b"); + + var found = Scan(Book("She", "H. Rider Haggard")); + + Assert.Equal(new List { "She.m4b" }, Names(found)); + } + + [Fact] + public void BasePath_IsTheBookFolder_NotTheAuthorFolder() + { + AddBook("H. Rider Haggard/H. Rider Haggard - She", "She.m4b"); + AddBook("H. Rider Haggard/H. Rider Haggard - Allan Quatermain", "Allan Quatermain.m4b"); + + var basePath = ScanPathPlanner.CalculateBasePath(Scan(Book("She", "H. Rider Haggard"))); + + // If the author counted as a match, the common parent of every Haggard file would be + // the AUTHOR folder -- and that would be stored as this book's BasePath, which is + // what rename and move then operate on. + Assert.EndsWith("H. Rider Haggard - She", basePath); + } + + // ----------------------------------------------------------------- + // Layouts that must keep working. + // ----------------------------------------------------------------- + + [Fact] + public void TitleInTheFolderName_Matches() + { + // The common Audnex/Plex shape: {Author}/{Author} - {Series} - {Title}/ + AddBook("Jules Verne/Jules Verne - Captain Nemo - Twenty Thousand Leagues Under the Sea", + "Twenty Thousand Leagues Under the Sea.m4b"); + + var found = Scan(Book("Twenty Thousand Leagues Under the Sea", "Jules Verne")); + + Assert.Single(found); + } + + [Fact] + public void TitleInTheFileName_Matches_EvenWhenTheFolderDoesNotCarryIt() + { + AddBook("Jules Verne/Audiobooks", "Around the World in Eighty Days.mp3"); + + var found = Scan(Book("Around the World in Eighty Days", "Jules Verne")); + + Assert.Single(found); + } + + [Fact] + public void EveryFileInAMatchedBookFolder_IsClaimed() + { + // A multi-part book: the folder identifies it, so all its parts belong to it -- + // including parts whose own filenames do not repeat the title. + AddBook("Jules Verne/1870 - Twenty Thousand Leagues Under the Sea", + "Part 01.mp3", "Part 02.mp3", "Part 03.mp3"); + + var found = Scan(Book("Twenty Thousand Leagues Under the Sea", "Jules Verne")); + + Assert.Equal(3, found.Count); + } + + [Fact] + public void ASiblingBookInTheSameSeriesFolder_IsNotClaimed() + { + AddBook("Jules Verne/Captain Nemo/1870 - Twenty Thousand Leagues Under the Sea", "book.m4b"); + AddBook("Jules Verne/Captain Nemo/1874 - The Mysterious Island", "book.m4b"); + + var found = Scan(Book("The Mysterious Island", "Jules Verne")); + + Assert.Single(found); + Assert.Contains("The Mysterious Island", found[0]); + } + + // ----------------------------------------------------------------- + // The trade-off, stated explicitly. + // ----------------------------------------------------------------- + + [Fact] + public void APathCarryingNeitherTitleNorAnythingIdentifying_IsLeftUnmatched() + { + // Previously this matched -- on the author alone -- and so did every other book by + // the same author. Unmatched is the correct outcome for a path that cannot identify + // a book: a miss is recoverable, a wrong link is not. Embedded tags are what should + // claim these. + AddBook("Elfie Donnelly/Bibi und Tina/61", "61 - track.mp3"); + + var found = Scan(Book("Retten die Biber", "Elfie Donnelly")); + + Assert.Empty(found); + } + } +}