From 0c98bda5d8d5bbc48228fbbb710350f6180f3b7f Mon Sep 17 00:00:00 2001 From: s3ntin3l8 <58235613+s3ntin3l8@users.noreply.github.com> Date: Sun, 7 Jun 2026 15:29:44 +0000 Subject: [PATCH 01/12] fix: apply configured naming patterns exactly, without implicit Series/Subtitle (#571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ComputeAudiobookBaseDirectoryFromPattern injected a {Series} directory level into the configured folder pattern whenever the audiobook had series metadata, even when the user deliberately omitted {Series}. The injected path drives the Add-book destination preview and the bulk root-folder change, and is persisted as the audiobook BasePath — so imported files really landed in an unconfigured series folder. RenameService and ManualImportController similarly rewrote the {Title} variable to "Title: Subtitle" when the patterns lacked {Subtitle} (sanitized to "Title - Subtitle" in paths). Patterns are now applied verbatim; users who want series folders or subtitles add the {Series}/{Subtitle} tokens explicitly. The default folder pattern already contains {Series}, so default configurations are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + .../Controllers/LibraryController.cs | 14 ---- .../Controllers/ManualImportController.cs | 15 +--- .../Audiobooks/RenameService.cs | 14 +--- tests/Builders/AudiobookBuilder.cs | 6 ++ .../LibraryController_BasePathTests.cs | 49 +++++++++++++ .../ManualImportControllerTests.cs | 42 +++++++++++ .../Api/Services/RenameServiceTests.cs | 70 +++++++++++++++++++ 8 files changed, 173 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 982b7c6df..9b5676c0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- **Naming patterns are applied exactly as configured — no implicit `{Series}` folder or subtitle.** The Add-book destination preview and bulk root-folder change (`LibraryController.ComputeAudiobookBaseDirectoryFromPattern`) no longer inject a `{Series}` directory level when the configured folder pattern omits `{Series}` — previously the injected path was persisted as the audiobook's `BasePath`, so imported files really did land in an unconfigured series folder. Likewise, rename/organize previews (`RenameService`) and manual imports (`ManualImportController`) no longer rewrite `{Title}` to `"Title: Subtitle"` when the pattern lacks `{Subtitle}`. Add the `{Series}`/`{Subtitle}` tokens explicitly to include them; the default folder pattern already contains `{Series}`, so default configurations are unaffected. ([#571](https://github.com/Listenarrs/Listenarr/issues/571)) - **Authentication settings: startup-config save no longer offers a downloadable `config.json` fallback when the backend refuses the save as invalid.** `SettingsView.saveSettings()` previously wrapped `apiService.saveStartupConfig` in a bare `catch {}` and treated every failure as a disk-persistence problem — offering the user a downloadable `config.json` containing the *server-rejected* values so they could save it manually. That bypasses the new backend admin-existence guard entirely: a user who tries to enable the login screen with no admin user gets the backend's 400, the FE catches it, and the FE offers a download of the same `AuthenticationRequired=true` config the server just refused. The catch now inspects the thrown error's `status`: 4xx responses are validation refusals and surface as a hard error toast (no download offered); 5xx and network failures fall through to the existing download fallback, which is the right escape hatch for "server wants to save but can't write to disk." - **Authentication settings: enabling the login screen now refuses to persist when no admin user exists.** `ConfigurationService.SaveStartupConfigAsync` queries `IUserService.GetAdminUsersAsync` whenever the incoming save *transitions* `AuthenticationRequired` from disabled to enabled, and throws if the admin user list is empty. This closes the carveout left by the credential-visibility and admin-provisioning fixes below: the settings DTO clears blank fields before save, so a user who flips "Enable login screen" with empty (or username-only) admin credentials silently skipped provisioning entirely and still reached the startup-config write, locking themselves out of an admin-less instance (recoverable by editing `config/config.json` back to `"AuthenticationRequired": "false"`, but a confusing first-time-setup trap). The check is scoped to the transition: subsequent saves while auth is already on (API key regenerations, port changes, log-level tweaks) don't re-query the admin list, and the common "just updating other startup fields with auth off" path stays unaffected. The admin block in `SaveApplicationSettings` runs before the startup-config write in the same save flow, so the typical "supply credentials and enable login in the same save" sequence has the admin row in place by the time the check runs. - **Authentication settings: admin provisioning failures no longer silently let the auth-required toggle proceed.** `ConfigurationService.SaveApplicationSettingsAsync` previously caught any exception from `CreateUserAsync` / `UpdatePasswordAsync`, logged it, and returned successfully — so when admin credentials were supplied but the user-service rejected them (password policy violation, repo I/O error, concurrent-write race), `SettingsView.saveSettings()` would still go on to persist `AuthenticationRequired=true` on its second request. The result was an instance that required login but had no working admin account — exactly the lockout shape the credential-visibility fix below was meant to prevent. The catch now re-throws the failure so the caller aborts before the auth-toggle write. The settings row itself is still saved before the admin block (non-admin changes like notification triggers and webhooks shouldn't disappear because admin provisioning failed), and the no-credentials path remains an unchanged silent skip. diff --git a/listenarr.api/Controllers/LibraryController.cs b/listenarr.api/Controllers/LibraryController.cs index ed1d665b1..b2892afa7 100644 --- a/listenarr.api/Controllers/LibraryController.cs +++ b/listenarr.api/Controllers/LibraryController.cs @@ -3491,20 +3491,6 @@ private string ComputeAudiobookBaseDirectoryFromPattern(Audiobook audiobook, str directoryPattern = "{Author}/{Title}"; } - // For series books, ensure we include the series in the directory structure - if (!string.IsNullOrWhiteSpace(audiobook.Series) && !directoryPattern.Contains("{Series}")) - { - // Insert series between author and title if not already present - if (directoryPattern.Contains("{Author}/{Title}")) - { - directoryPattern = directoryPattern.Replace("{Author}/{Title}", "{Author}/{Series}/{Title}"); - } - else if (directoryPattern.Contains("{Author}/")) - { - directoryPattern = directoryPattern.Replace("{Author}/", "{Author}/{Series}/"); - } - } - // If the audiobook has no Series, remove any {Series} tokens from the directory pattern // Tests expect the controller to strip the Series token when series metadata is missing. if (string.IsNullOrWhiteSpace(audiobook.Series)) diff --git a/listenarr.api/Controllers/ManualImportController.cs b/listenarr.api/Controllers/ManualImportController.cs index 37dccd6b9..9afea0661 100644 --- a/listenarr.api/Controllers/ManualImportController.cs +++ b/listenarr.api/Controllers/ManualImportController.cs @@ -486,19 +486,8 @@ private async Task GenerateManualImportPathAsync(Audiobook audiobook, Au if (!string.IsNullOrWhiteSpace(audiobook.Edition)) variables["Edition"] = audiobook.Edition; - // Preserve the older title+subtitle uniqueness behavior unless the user explicitly uses {Subtitle}. - // (e.g. "The Land" + "Founding" → "The Land: Founding") - var usesSubtitleToken = (!string.IsNullOrWhiteSpace(folderPattern) && folderPattern.IndexOf("Subtitle", StringComparison.OrdinalIgnoreCase) >= 0) - || (!string.IsNullOrWhiteSpace(filePattern) && filePattern.IndexOf("Subtitle", StringComparison.OrdinalIgnoreCase) >= 0); - - var titleFull = !usesSubtitleToken - && !string.IsNullOrWhiteSpace(audiobook.Subtitle) - && !string.IsNullOrWhiteSpace(audiobook.Title) - && !audiobook.Title.Contains(audiobook.Subtitle, StringComparison.OrdinalIgnoreCase) - ? $"{audiobook.Title}: {audiobook.Subtitle}" - : audiobook.Title; - variables["Title"] = !string.IsNullOrWhiteSpace(titleFull) - ? titleFull + variables["Title"] = !string.IsNullOrWhiteSpace(audiobook.Title) + ? audiobook.Title : "Unknown Title"; // Title is required as fallback if (!string.IsNullOrWhiteSpace(audiobook.Series)) diff --git a/listenarr.application/Audiobooks/RenameService.cs b/listenarr.application/Audiobooks/RenameService.cs index 3791266d8..c1ed3ea75 100644 --- a/listenarr.application/Audiobooks/RenameService.cs +++ b/listenarr.application/Audiobooks/RenameService.cs @@ -403,7 +403,7 @@ private string BuildExpectedPath(Audiobook audiobook, PreviewFileEntry file, App { var folderPattern = settings.FolderNamingPattern; var filePattern = isMultiFile ? settings.MultiFileNamingPattern : settings.FileNamingPattern; - var variables = BuildNamingVariables(audiobook, folderPattern, filePattern, file.SequenceNumber, isMultiFile); + var variables = BuildNamingVariables(audiobook, file.SequenceNumber, isMultiFile); var patternHasNumberTokens = !string.IsNullOrWhiteSpace(filePattern) && (filePattern.IndexOf("DiskNumber", StringComparison.OrdinalIgnoreCase) >= 0 || filePattern.IndexOf("ChapterNumber", StringComparison.OrdinalIgnoreCase) >= 0); @@ -434,23 +434,15 @@ private string BuildExpectedPath(Audiobook audiobook, PreviewFileEntry file, App return string.IsNullOrWhiteSpace(basePath) ? NormalizePath(relativePath) : NormalizePath(CombineWithOptionalBase(basePath, relativePath)); } - private static Dictionary BuildNamingVariables(Audiobook audiobook, string? folderPattern, string? filePattern, int sequenceNumber, bool isMultiFile) + private static Dictionary BuildNamingVariables(Audiobook audiobook, int sequenceNumber, bool isMultiFile) { - var usesSubtitleToken = (!string.IsNullOrWhiteSpace(folderPattern) && folderPattern.IndexOf("Subtitle", StringComparison.OrdinalIgnoreCase) >= 0) - || (!string.IsNullOrWhiteSpace(filePattern) && filePattern.IndexOf("Subtitle", StringComparison.OrdinalIgnoreCase) >= 0); - var combinedTitle = !usesSubtitleToken - && !string.IsNullOrWhiteSpace(audiobook.Subtitle) - && !string.IsNullOrWhiteSpace(audiobook.Title) - && !audiobook.Title.Contains(audiobook.Subtitle, StringComparison.OrdinalIgnoreCase) - ? $"{audiobook.Title}: {audiobook.Subtitle}" - : audiobook.Title; var narrator = audiobook.Narrators != null ? string.Join(", ", audiobook.Narrators.Where(n => !string.IsNullOrWhiteSpace(n))) : string.Empty; return new Dictionary(StringComparer.OrdinalIgnoreCase) { { "Author", audiobook.Authors?.FirstOrDefault(n => !string.IsNullOrWhiteSpace(n)) ?? "Unknown Author" }, { "Series", audiobook.Series ?? string.Empty }, - { "Title", string.IsNullOrWhiteSpace(combinedTitle) ? "Unknown Title" : combinedTitle }, + { "Title", string.IsNullOrWhiteSpace(audiobook.Title) ? "Unknown Title" : audiobook.Title }, { "Subtitle", audiobook.Subtitle ?? string.Empty }, { "Edition", audiobook.Edition ?? string.Empty }, { "Narrator", narrator }, diff --git a/tests/Builders/AudiobookBuilder.cs b/tests/Builders/AudiobookBuilder.cs index 36e3bbe41..df7bcb82e 100644 --- a/tests/Builders/AudiobookBuilder.cs +++ b/tests/Builders/AudiobookBuilder.cs @@ -119,6 +119,12 @@ public AudiobookBuilder WithAuthorAsin(string value) return this; } + public AudiobookBuilder WithAsin(string value) + { + _audiobook.Asin = value; + return this; + } + public AudiobookBuilder WithQualityProfile(QualityProfile value) { _audiobook.QualityProfile = value; diff --git a/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs b/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs index af7faf9a7..9501f9f63 100644 --- a/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs +++ b/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs @@ -79,5 +79,54 @@ public void ComputeAudiobookBaseDirectoryFromPattern_SeriesBook_ReturnsCorrectPa // Then Assert.Equal(Path.Join(RootPath, "Stephen King", "The Dark Tower", "The Gunslinger"), result); } + + [Fact] + [Trait("Method", "ComputeAudiobookBaseDirectoryFromPattern")] + [Trait("Scenario", "SeriesBook_PatternWithoutSeries_OmitsSeriesFolder")] + public void ComputeAudiobookBaseDirectoryFromPattern_SeriesBook_PatternWithoutSeries_OmitsSeriesFolder() + { + // Given a series book and a configured pattern that deliberately omits {Series} + var audiobook = new AudiobookBuilder() + .WithTitle("The Gunslinger") + .WithAuthor("Stephen King") + .WithYear("1982") + .WithSeries("The Dark Tower") + .WithSeriesNumber("1") + .Build(); + + var controller = _provider.GetRequiredService(); + + // When + var result = (string)ComputeBaseDirectoryMethod.Invoke(controller, new object[] { audiobook, RootPath, "{Author}/{Title}" }); + + // Then the pattern is applied exactly — no series folder is injected + Assert.Equal(Path.Join(RootPath, "Stephen King", "The Gunslinger"), result); + Assert.DoesNotContain("The Dark Tower", result); + } + + [Fact] + [Trait("Method", "ComputeAudiobookBaseDirectoryFromPattern")] + [Trait("Scenario", "SeriesBook_PatternWithYearAndAsin_OmitsSeriesFolder")] + public void ComputeAudiobookBaseDirectoryFromPattern_SeriesBook_PatternWithYearAndAsin_OmitsSeriesFolder() + { + // Given a series book and a pattern with extra tokens but no {Series} + var audiobook = new AudiobookBuilder() + .WithTitle("Executive Orders") + .WithAuthor("Tom Clancy") + .WithYear("2010") + .WithSeries("A Jack Ryan Novel (chronological order)") + .WithSeriesNumber("8") + .WithAsin("B004ESTSSO") + .Build(); + + var controller = _provider.GetRequiredService(); + + // When + var result = (string)ComputeBaseDirectoryMethod.Invoke(controller, new object[] { audiobook, RootPath, "{Author}/{Title} ({Year}) [{Asin}]" }); + + // Then the remaining tokens still render and no series folder is injected + Assert.Equal(Path.Join(RootPath, "Tom Clancy", "Executive Orders (2010) [B004ESTSSO]"), result); + Assert.DoesNotContain("Jack Ryan", result); + } } } diff --git a/tests/Features/Api/Controllers/ManualImportControllerTests.cs b/tests/Features/Api/Controllers/ManualImportControllerTests.cs index 0b479c691..dcaab854c 100644 --- a/tests/Features/Api/Controllers/ManualImportControllerTests.cs +++ b/tests/Features/Api/Controllers/ManualImportControllerTests.cs @@ -170,6 +170,48 @@ public async Task InteractiveManualImport_MultipleFiles_ResolvesCollisionsWithin Assert.True(diskFiles.Count >= 2, "Expected at least two files in destination (one suffixed for the collision)"); } + [Fact] + public async Task InteractiveManualImport_PatternWithoutSubtitle_DoesNotCombineSubtitleIntoFilename() + { + // Given a book with a subtitle and a file pattern that deliberately omits {Subtitle} + var basePath = CreateTempDirectory("listenarr-manual-subtitle"); + var srcDir = CreateTempDirectory("listenarr-manual-subtitle-src"); + + var book = new Audiobook { Id = 99, Title = "The Land", Subtitle = "Founding", BasePath = basePath }; + + var src = Path.Join(srcDir, "one.mp3"); + await File.WriteAllTextAsync(src, "one"); + + var request = new ManualImportRequestDto + { + Path = srcDir, + Mode = "interactive", + Action = FileAction.Copy, + Items = + [ + new ManualImportItemDto { FullPath = src, MatchedAudiobookId = book.Id } + ] + }; + + var controller = GetController(book, new ApplicationSettings + { + OutputPath = basePath, + FolderNamingPattern = "{Author}", + FileNamingPattern = "{Title}" + }); + + // When + await controller.Start(request); + + // Then the configured pattern is applied exactly — the subtitle is not folded into the filename + var diskFiles = Directory.GetFiles(basePath, "*", SearchOption.AllDirectories) + .Select(Path.GetFileName) + .ToList(); + + Assert.Contains("The Land.mp3", diskFiles); + Assert.DoesNotContain(diskFiles, f => f!.Contains("Founding", StringComparison.OrdinalIgnoreCase)); + } + [Fact] public async Task InteractiveManualImport_MultipartFiles_UsesStableNaturalOrderAndNumbering() { diff --git a/tests/Features/Api/Services/RenameServiceTests.cs b/tests/Features/Api/Services/RenameServiceTests.cs index 48a2be5f2..508d19583 100644 --- a/tests/Features/Api/Services/RenameServiceTests.cs +++ b/tests/Features/Api/Services/RenameServiceTests.cs @@ -100,6 +100,76 @@ public async Task PreviewRename_UsesExtendedMetadataVariables() Assert.Contains("B000TEST", preview.NewFolderPath); } + [Fact] + public async Task PreviewRename_PatternWithoutSubtitle_DoesNotCombineSubtitleIntoTitle() + { + // Given a book with a subtitle and patterns that deliberately omit {Subtitle} + var settings = new ApplicationSettings + { + OutputPath = _tempRoot, + FolderNamingPattern = "{Author}/{Title}", + FileNamingPattern = "{Title}" + }; + + var (service, db, _) = BuildService(settings); + db.Audiobooks.Add(new Audiobook + { + Id = 7, + Title = "The Land", + Subtitle = "Founding", + Authors = new List { "Aleron Kong" }, + BasePath = Path.Join(_tempRoot, "Wrong", "Folder"), + Files = new List + { + new() { Id = 71, AudiobookId = 7, Path = Path.Join(_tempRoot, "Wrong", "Folder", "old-name.m4b"), Format = "m4b" } + } + }); + await db.SaveChangesAsync(); + + // When + var previews = await service.PreviewRenameAsync(new[] { 7 }); + + // Then the configured patterns are applied exactly — the subtitle is not folded into the title + var preview = Assert.Single(previews); + Assert.Contains("The Land", preview.NewFolderPath); + Assert.DoesNotContain("Founding", preview.NewFolderPath); + Assert.All(preview.FileRenames, file => Assert.DoesNotContain("Founding", file.NewPath)); + } + + [Fact] + public async Task PreviewRename_ExplicitSubtitleToken_IncludesSubtitle() + { + // Given a book with a subtitle and a file pattern that explicitly uses {Subtitle} + var settings = new ApplicationSettings + { + OutputPath = _tempRoot, + FolderNamingPattern = "{Author}/{Title}", + FileNamingPattern = "{Title} - {Subtitle}" + }; + + var (service, db, _) = BuildService(settings); + db.Audiobooks.Add(new Audiobook + { + Id = 8, + Title = "The Land", + Subtitle = "Founding", + Authors = new List { "Aleron Kong" }, + BasePath = Path.Join(_tempRoot, "Wrong", "Folder"), + Files = new List + { + new() { Id = 81, AudiobookId = 8, Path = Path.Join(_tempRoot, "Wrong", "Folder", "old-name.m4b"), Format = "m4b" } + } + }); + await db.SaveChangesAsync(); + + // When + var previews = await service.PreviewRenameAsync(new[] { 8 }); + + // Then the explicit token still renders the subtitle + var preview = Assert.Single(previews); + Assert.Contains(preview.FileRenames, file => file.NewPath!.Contains("The Land - Founding")); + } + [Fact] public async Task PreviewRename_PreservesCustomBasePath() { From 5cc3b75c9bb864f84d0cafd7ea2c775930a967b2 Mon Sep 17 00:00:00 2001 From: s3ntin3l8 <58235613+s3ntin3l8@users.noreply.github.com> Date: Sun, 7 Jun 2026 15:35:15 +0000 Subject: [PATCH 02/12] fix: apply slash-less folder patterns exactly in base-directory computation (#571) A deliberately flat folder pattern (e.g. "{Title}") was silently replaced with "{Author}/{Title}" when computing an audiobook's base directory for the destination preview and bulk root-folder change. The import pipeline (FileNamingService) already applies such patterns exactly, so the preview did not match where files actually went. The "{Author}/{Title}" fallback now only applies when no pattern is configured at all (or token stripping leaves it empty). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + .../Controllers/LibraryController.cs | 5 +++-- .../LibraryController_BasePathTests.cs | 22 +++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b5676c0f..013b9b8aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **Naming patterns are applied exactly as configured — no implicit `{Series}` folder or subtitle.** The Add-book destination preview and bulk root-folder change (`LibraryController.ComputeAudiobookBaseDirectoryFromPattern`) no longer inject a `{Series}` directory level when the configured folder pattern omits `{Series}` — previously the injected path was persisted as the audiobook's `BasePath`, so imported files really did land in an unconfigured series folder. Likewise, rename/organize previews (`RenameService`) and manual imports (`ManualImportController`) no longer rewrite `{Title}` to `"Title: Subtitle"` when the pattern lacks `{Subtitle}`. Add the `{Series}`/`{Subtitle}` tokens explicitly to include them; the default folder pattern already contains `{Series}`, so default configurations are unaffected. ([#571](https://github.com/Listenarrs/Listenarr/issues/571)) +- **Flat folder patterns are respected in the destination preview.** A deliberately slash-less folder pattern (e.g. `{Title}`) is no longer silently overridden with `{Author}/{Title}` when computing an audiobook's base directory — matching how the import pipeline (`FileNamingService`) already applies such patterns. The `{Author}/{Title}` default still applies when no pattern is configured at all. ([#571](https://github.com/Listenarrs/Listenarr/issues/571)) - **Authentication settings: startup-config save no longer offers a downloadable `config.json` fallback when the backend refuses the save as invalid.** `SettingsView.saveSettings()` previously wrapped `apiService.saveStartupConfig` in a bare `catch {}` and treated every failure as a disk-persistence problem — offering the user a downloadable `config.json` containing the *server-rejected* values so they could save it manually. That bypasses the new backend admin-existence guard entirely: a user who tries to enable the login screen with no admin user gets the backend's 400, the FE catches it, and the FE offers a download of the same `AuthenticationRequired=true` config the server just refused. The catch now inspects the thrown error's `status`: 4xx responses are validation refusals and surface as a hard error toast (no download offered); 5xx and network failures fall through to the existing download fallback, which is the right escape hatch for "server wants to save but can't write to disk." - **Authentication settings: enabling the login screen now refuses to persist when no admin user exists.** `ConfigurationService.SaveStartupConfigAsync` queries `IUserService.GetAdminUsersAsync` whenever the incoming save *transitions* `AuthenticationRequired` from disabled to enabled, and throws if the admin user list is empty. This closes the carveout left by the credential-visibility and admin-provisioning fixes below: the settings DTO clears blank fields before save, so a user who flips "Enable login screen" with empty (or username-only) admin credentials silently skipped provisioning entirely and still reached the startup-config write, locking themselves out of an admin-less instance (recoverable by editing `config/config.json` back to `"AuthenticationRequired": "false"`, but a confusing first-time-setup trap). The check is scoped to the transition: subsequent saves while auth is already on (API key regenerations, port changes, log-level tweaks) don't re-query the admin list, and the common "just updating other startup fields with auth off" path stays unaffected. The admin block in `SaveApplicationSettings` runs before the startup-config write in the same save flow, so the typical "supply credentials and enable login in the same save" sequence has the admin row in place by the time the check runs. - **Authentication settings: admin provisioning failures no longer silently let the auth-required toggle proceed.** `ConfigurationService.SaveApplicationSettingsAsync` previously caught any exception from `CreateUserAsync` / `UpdatePasswordAsync`, logged it, and returned successfully — so when admin credentials were supplied but the user-service rejected them (password policy violation, repo I/O error, concurrent-write race), `SettingsView.saveSettings()` would still go on to persist `AuthenticationRequired=true` on its second request. The result was an instance that required login but had no working admin account — exactly the lockout shape the credential-visibility fix below was meant to prevent. The catch now re-throws the failure so the caller aborts before the auth-toggle write. The settings row itself is still saved before the admin block (non-admin changes like notification triggers and webhooks shouldn't disappear because admin provisioning failed), and the no-credentials path remains an unchanged silent skip. diff --git a/listenarr.api/Controllers/LibraryController.cs b/listenarr.api/Controllers/LibraryController.cs index b2892afa7..168165c11 100644 --- a/listenarr.api/Controllers/LibraryController.cs +++ b/listenarr.api/Controllers/LibraryController.cs @@ -3479,8 +3479,9 @@ private string ComputeAudiobookBaseDirectoryFromPattern(Audiobook audiobook, str directoryPattern = Regex.Replace(directoryPattern, @"^\s*[\\/]", ""); directoryPattern = Regex.Replace(directoryPattern, @"[\\/]\s*$", ""); - // If the pattern is now empty or doesn't contain directory separators, use a fallback - if (string.IsNullOrWhiteSpace(directoryPattern) || !directoryPattern.Contains("/")) + // If the pattern is now empty, use a fallback; deliberately flat (slash-less) + // patterns are applied exactly as configured + if (string.IsNullOrWhiteSpace(directoryPattern)) { directoryPattern = "{Author}/{Title}"; } diff --git a/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs b/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs index 9501f9f63..cfbb02a34 100644 --- a/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs +++ b/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs @@ -128,5 +128,27 @@ public void ComputeAudiobookBaseDirectoryFromPattern_SeriesBook_PatternWithYearA Assert.Equal(Path.Join(RootPath, "Tom Clancy", "Executive Orders (2010) [B004ESTSSO]"), result); Assert.DoesNotContain("Jack Ryan", result); } + + [Fact] + [Trait("Method", "ComputeAudiobookBaseDirectoryFromPattern")] + [Trait("Scenario", "SlashlessPattern_AppliedExactly")] + public void ComputeAudiobookBaseDirectoryFromPattern_SlashlessPattern_AppliedExactly() + { + // Given a deliberately flat folder pattern without directory separators + var audiobook = new AudiobookBuilder() + .WithTitle("The Buffalo Hunter Hunter") + .WithAuthor("Stephen Graham Jones") + .WithYear("2025") + .Build(); + + var controller = _provider.GetRequiredService(); + + // When + var result = (string)ComputeBaseDirectoryMethod.Invoke(controller, new object[] { audiobook, RootPath, "{Title}" }); + + // Then the pattern is applied exactly — no implicit {Author} folder is prepended + Assert.Equal(Path.Join(RootPath, "The Buffalo Hunter Hunter"), result); + Assert.DoesNotContain("Stephen Graham Jones", result); + } } } From 57b898e2f22b5e77d3e71e9c01d44d9669404e71 Mon Sep 17 00:00:00 2001 From: s3ntin3l8 <58235613+s3ntin3l8@users.noreply.github.com> Date: Sun, 7 Jun 2026 19:48:51 +0000 Subject: [PATCH 03/12] fix: don't strip {SeriesNumber} when an audiobook has no series (#571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "remove {Series} when the book has no series" cleanup in ComputeAudiobookBaseDirectoryFromPattern used the regex \{Series[^}]*\}, which also matched {SeriesNumber}/{SeriesNumber:00} — so a configured pattern like {Author}/{SeriesNumber}/{Title} lost its number for non-series books. Remove the textual strip block entirely and let ApplyNamingPattern handle the empty {Series} value (it already substitutes empty tokens and collapses the surrounding separators — the single cleanup path the rest of this change relies on). {SeriesNumber} is now preserved; the existing non-series test ({Author}/{Series}/{Title} -> Author/Title) still passes. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Controllers/LibraryController.cs | 16 ++++++-------- .../LibraryController_BasePathTests.cs | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/listenarr.api/Controllers/LibraryController.cs b/listenarr.api/Controllers/LibraryController.cs index 168165c11..397de791a 100644 --- a/listenarr.api/Controllers/LibraryController.cs +++ b/listenarr.api/Controllers/LibraryController.cs @@ -3492,16 +3492,12 @@ private string ComputeAudiobookBaseDirectoryFromPattern(Audiobook audiobook, str directoryPattern = "{Author}/{Title}"; } - // If the audiobook has no Series, remove any {Series} tokens from the directory pattern - // Tests expect the controller to strip the Series token when series metadata is missing. - if (string.IsNullOrWhiteSpace(audiobook.Series)) - { - directoryPattern = Regex.Replace(directoryPattern, @"\{Series[^}]*\}", string.Empty, RegexOptions.IgnoreCase); - // Clean up any resulting duplicate separators or empty parts again - directoryPattern = Regex.Replace(directoryPattern, @"[\\/]\s*[\\/]", "/"); - directoryPattern = Regex.Replace(directoryPattern, @"^\s*[\\/]", ""); - directoryPattern = Regex.Replace(directoryPattern, @"[\\/]\s*$", ""); - } + // An empty {Series} (no series metadata) is handled by ApplyNamingPattern below, which + // substitutes empty tokens and collapses the surrounding separators. We deliberately do + // not strip {Series} textually here: a regex like \{Series[^}]*\} also matches + // {SeriesNumber}/{SeriesNumber:00}, which would drop those tokens from patterns that use + // them. Applying the pattern exactly and letting the sentinel cleanup remove empties keeps + // {SeriesNumber} intact. // Build variables for naming pattern using audiobook-level metadata var variables = new Dictionary diff --git a/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs b/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs index cfbb02a34..a01929dce 100644 --- a/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs +++ b/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs @@ -150,5 +150,26 @@ public void ComputeAudiobookBaseDirectoryFromPattern_SlashlessPattern_AppliedExa Assert.Equal(Path.Join(RootPath, "The Buffalo Hunter Hunter"), result); Assert.DoesNotContain("Stephen Graham Jones", result); } + + [Fact] + [Trait("Method", "ComputeAudiobookBaseDirectoryFromPattern")] + [Trait("Scenario", "NonSeriesBook_KeepsSeriesNumberToken")] + public void ComputeAudiobookBaseDirectoryFromPattern_NonSeriesBook_KeepsSeriesNumberToken() + { + // Given a book with no series title but a series number, and a pattern using {SeriesNumber} + var audiobook = new AudiobookBuilder() + .WithTitle("The Gunslinger") + .WithAuthor("Stephen King") + .WithSeriesNumber("3") + .Build(); + + var controller = _provider.GetRequiredService(); + + // When + var result = (string)ComputeBaseDirectoryMethod.Invoke(controller, new object[] { audiobook, RootPath, "{Author}/{SeriesNumber}/{Title}" }); + + // Then {SeriesNumber} is preserved — it must not be stripped along with an empty {Series} + Assert.Equal(Path.Join(RootPath, "Stephen King", "3", "The Gunslinger"), result); + } } } From 7e16c8f1f38b3434fcb3badd03773f9b3fcbd282 Mon Sep 17 00:00:00 2001 From: s3ntin3l8 <58235613+s3ntin3l8@users.noreply.github.com> Date: Tue, 9 Jun 2026 21:22:42 +0000 Subject: [PATCH 04/12] refactor: add unified NamingContext + naming orchestrator (#571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a single path-assembly orchestrator in FileNamingService — BuildDirectory/BuildPath over a normalized NamingContext — so the per-flow naming duplication can be consolidated. NamingContext.From() factories map Audiobook/AudioMetadata/AudibleBookMetadata into one shape; source-specific quirks (noisy-tag author selection) live in the mappers, while sanitization, token substitution, the folder/file/legacy branch and path-length limits live once in the service. GenerateFilePathAsync now delegates to BuildPath, so its existing ~40 tests exercise the orchestrator. No behavior change yet — the individual flows are rerouted onto BuildDirectory/BuildPath in follow-up commits. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Common/FileNamingService.cs | 165 +++++++++++------- .../Interfaces/IFileNamingService.cs | 41 +++++ .../Models/Naming/NamingContext.cs | 137 +++++++++++++++ 3 files changed, 284 insertions(+), 59 deletions(-) create mode 100644 listenarr.domain/Models/Naming/NamingContext.cs diff --git a/listenarr.application/Common/FileNamingService.cs b/listenarr.application/Common/FileNamingService.cs index 2fd97f5e1..eac7f859a 100644 --- a/listenarr.application/Common/FileNamingService.cs +++ b/listenarr.application/Common/FileNamingService.cs @@ -20,8 +20,10 @@ using System.Text; using System.Text.RegularExpressions; using Listenarr.Application.Interfaces; +using Listenarr.Domain.Common; using Listenarr.Domain.Models; using Listenarr.Domain.Models.Configurations; +using Listenarr.Domain.Models.Naming; using Microsoft.Extensions.Logging; namespace Listenarr.Application.Common @@ -62,99 +64,115 @@ public async Task GenerateFilePathAsync( string originalExtension = ".m4b") { var settings = await _configService.GetApplicationSettingsAsync() ?? new ApplicationSettings(); - var folderPattern = settings.FolderNamingPattern; - - // Determine if this is a multi-file import (has disk or chapter number) - bool isMultiFile = metadata.DiscNumber.HasValue || metadata.TrackNumber.HasValue; - var filePattern = isMultiFile - ? settings.MultiFileNamingPattern - : settings.FileNamingPattern; - var effectiveFolderPattern = folderPattern; - try - { - if (!string.IsNullOrWhiteSpace(outputPath) && !string.IsNullOrWhiteSpace(settings.OutputPath)) - { - var requestedRoot = Path.GetFullPath(outputPath); - var configuredRoot = Path.GetFullPath(settings.OutputPath); - if (!string.Equals(requestedRoot, configuredRoot, StringComparison.OrdinalIgnoreCase)) - { - // Caller provided a custom base path (e.g., audiobook BasePath) -> skip folder pattern - effectiveFolderPattern = string.Empty; - } - } - } - catch (Exception caughtEx_2) when (caughtEx_2 is not OperationCanceledException && caughtEx_2 is not OutOfMemoryException && caughtEx_2 is not StackOverflowException) + var options = new NamingOptions { - // If paths are invalid, fall back to configured folder pattern - System.Diagnostics.Debug.WriteLine("Suppressed non-fatal exception in catch block."); - } + OutputRoot = string.IsNullOrWhiteSpace(outputPath) ? settings.OutputPath : outputPath, + // A custom output root means the destination folder is already chosen -> file pattern only. + IsCustomBasePath = IsCustomOutputRoot(outputPath, settings.OutputPath), + IsMultiFile = metadata.DiscNumber.HasValue || metadata.TrackNumber.HasValue, + SequenceNumber = null, + Extension = originalExtension, + }; - var variables = BuildVariables(metadata); + var result = BuildPath(NamingContext.From(metadata), settings, options); + _logger.LogInformation("Generated file path: {FilePath}", result.FullPath); + return result.FullPath; + } - // Diagnostic logging: record the variables used for pattern replacement - try - { - var dbg = string.Join(", ", variables.Select(kv => $"{kv.Key}='{kv.Value}'")); - _logger.LogInformation("FileNamingService variables: {Vars}", dbg); - } - catch (Exception caughtEx_3) when (caughtEx_3 is not OperationCanceledException && caughtEx_3 is not OutOfMemoryException && caughtEx_3 is not StackOverflowException) - { - // ignore logging errors - System.Diagnostics.Debug.WriteLine("Suppressed non-fatal exception in catch block."); - } + public string BuildDirectory(NamingContext context, ApplicationSettings settings) + { + // Folder-only computation (the audiobook's BasePath). Fall back to the file pattern when no + // folder pattern is configured, then rely on ApplyNamingPattern for empty-token cleanup. + var folderPattern = string.IsNullOrWhiteSpace(settings.FolderNamingPattern) + ? settings.FileNamingPattern + : settings.FolderNamingPattern; + var variables = BuildVariables(context); + return ApplyNamingPattern(folderPattern ?? string.Empty, variables, treatAsFilename: false); + } + + public NamingResult BuildPath(NamingContext context, ApplicationSettings settings, NamingOptions options) + { + var variables = BuildVariables(context); + var folderPattern = settings.FolderNamingPattern; + var filePattern = options.IsMultiFile ? settings.MultiFileNamingPattern : settings.FileNamingPattern; + + var patternHasNumberTokens = !string.IsNullOrWhiteSpace(filePattern) + && (filePattern.IndexOf("DiskNumber", StringComparison.OrdinalIgnoreCase) >= 0 + || filePattern.IndexOf("ChapterNumber", StringComparison.OrdinalIgnoreCase) >= 0); string relativePath; - if (string.IsNullOrWhiteSpace(effectiveFolderPattern)) + if (string.IsNullOrWhiteSpace(folderPattern)) { - // Legacy behavior: use FileNamingPattern as the full relative path pattern + // Legacy behavior: use the file naming pattern as the full relative path. var legacyPattern = string.IsNullOrWhiteSpace(filePattern) ? "{Author}/{Series}/{Title}" : filePattern; - - relativePath = ApplyNamingPattern(legacyPattern, variables); + relativePath = ApplyNamingPattern(legacyPattern, variables, treatAsFilename: false); + } + else if (options.IsCustomBasePath) + { + // The destination folder is already decided; apply the file pattern only. + var effectiveFilePattern = string.IsNullOrWhiteSpace(filePattern) ? "{Title}" : filePattern; + relativePath = ApplyNamingPattern(effectiveFilePattern, variables, treatAsFilename: !PatternAllowsSubfolders(effectiveFilePattern)); } else { - // New behavior: separate folder and file patterns + // Separate folder and file patterns. var effectiveFilePattern = string.IsNullOrWhiteSpace(filePattern) ? "{Title}" : filePattern; - var folderRelative = ApplyNamingPattern(effectiveFolderPattern, variables, treatAsFilename: false); - - // Normalize path separators to platform-specific ones + var folderRelative = ApplyNamingPattern(folderPattern, variables, treatAsFilename: false); if (!string.IsNullOrWhiteSpace(folderRelative)) { folderRelative = folderRelative.Replace('/', Path.DirectorySeparatorChar) .Replace('\\', Path.DirectorySeparatorChar); } - var patternAllowsSubfolders = effectiveFilePattern.IndexOf("DiskNumber", StringComparison.OrdinalIgnoreCase) >= 0 - || effectiveFilePattern.IndexOf("ChapterNumber", StringComparison.OrdinalIgnoreCase) >= 0 - || effectiveFilePattern.IndexOf('/') >= 0 - || effectiveFilePattern.IndexOf('\\') >= 0; - - var fileRelative = ApplyNamingPattern(effectiveFilePattern, variables, treatAsFilename: !patternAllowsSubfolders); + var fileRelative = ApplyNamingPattern(effectiveFilePattern, variables, treatAsFilename: !PatternAllowsSubfolders(effectiveFilePattern)); + if (options.IsMultiFile && !patternHasNumberTokens && options.SequenceNumber.HasValue) + fileRelative = FileUtils.AppendSequenceSuffix(fileRelative, options.SequenceNumber.Value); relativePath = string.IsNullOrWhiteSpace(folderRelative) ? fileRelative : CombineWithOptionalBase(folderRelative, fileRelative); } - // Ensure it has the correct extension - if (!relativePath.EndsWith(originalExtension, StringComparison.OrdinalIgnoreCase)) + // For the legacy and custom-base branches the multi-file suffix is appended to the whole path. + if ((string.IsNullOrWhiteSpace(folderPattern) || options.IsCustomBasePath) + && options.IsMultiFile && !patternHasNumberTokens && options.SequenceNumber.HasValue) { - relativePath += originalExtension; + relativePath = FileUtils.AppendSequenceSuffix(relativePath, options.SequenceNumber.Value); } - // Combine with the provided output path - var fullPath = string.IsNullOrWhiteSpace(outputPath) + if (!relativePath.EndsWith(options.Extension, StringComparison.OrdinalIgnoreCase)) + relativePath += options.Extension; + + var fullPath = string.IsNullOrWhiteSpace(options.OutputRoot) ? relativePath - : CombineWithOptionalBase(outputPath, relativePath); + : CombineWithOptionalBase(options.OutputRoot, relativePath); fullPath = EnsurePathWithinLimits(fullPath); + return new NamingResult(relativePath, fullPath); + } - _logger.LogInformation("Generated file path: {FilePath}", fullPath); - return fullPath; + private static bool PatternAllowsSubfolders(string pattern) + => pattern.IndexOf("DiskNumber", StringComparison.OrdinalIgnoreCase) >= 0 + || pattern.IndexOf("ChapterNumber", StringComparison.OrdinalIgnoreCase) >= 0 + || pattern.IndexOf('/') >= 0 + || pattern.IndexOf('\\') >= 0; + + private static bool IsCustomOutputRoot(string? outputPath, string? configuredOutput) + { + if (string.IsNullOrWhiteSpace(outputPath) || string.IsNullOrWhiteSpace(configuredOutput)) + return false; + try + { + return !string.Equals(Path.GetFullPath(outputPath), Path.GetFullPath(configuredOutput), StringComparison.OrdinalIgnoreCase); + } + catch (Exception ex) when (ex is not OperationCanceledException && ex is not OutOfMemoryException && ex is not StackOverflowException) + { + return false; + } } public string ApplyNamingPattern(string pattern, Dictionary variables, bool treatAsFilename = false) @@ -393,6 +411,35 @@ private Dictionary BuildVariables(AudibleBookMetadata metadata) }; } + // Unified variable builder used by the orchestrator (BuildDirectory/BuildPath). All flows map their + // source type into a NamingContext, so token sanitization and empty-handling live in one place. + // The dictionary is case-insensitive so patterns like {author} resolve as well as {Author}. + private Dictionary BuildVariables(NamingContext context) + { + var author = context.Authors?.FirstOrDefault(a => !string.IsNullOrWhiteSpace(a)); + var narrator = context.Narrators != null + ? string.Join(", ", context.Narrators.Where(n => !string.IsNullOrWhiteSpace(n))) + : string.Empty; + + return new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "Author", SanitizePathComponent(FirstNonEmpty(author, "Unknown Author")) }, + { "Series", string.IsNullOrWhiteSpace(context.Series) ? string.Empty : SanitizePathComponent(context.Series) }, + { "Title", SanitizePathComponent(FirstNonEmpty(context.Title, "Unknown Title")) }, + { "Subtitle", string.IsNullOrWhiteSpace(context.Subtitle) ? string.Empty : SanitizePathComponent(context.Subtitle) }, + { "Edition", string.IsNullOrWhiteSpace(context.Edition) ? string.Empty : SanitizePathComponent(context.Edition) }, + { "Narrator", string.IsNullOrWhiteSpace(narrator) ? string.Empty : SanitizePathComponent(narrator) }, + { "Publisher", string.IsNullOrWhiteSpace(context.Publisher) ? string.Empty : SanitizePathComponent(context.Publisher) }, + { "Language", string.IsNullOrWhiteSpace(context.Language) ? string.Empty : SanitizePathComponent(context.Language) }, + { "Asin", string.IsNullOrWhiteSpace(context.Asin) ? string.Empty : SanitizePathComponent(context.Asin) }, + { "SeriesNumber", context.SeriesNumber ?? string.Empty }, + { "Year", context.Year ?? string.Empty }, + { "Quality", context.Quality ?? string.Empty }, + { "DiskNumber", context.DiskNumber?.ToString() ?? string.Empty }, + { "ChapterNumber", context.ChapterNumber?.ToString() ?? string.Empty } + }; + } + private static string FirstNonEmpty(params string?[] candidates) { return candidates.FirstOrDefault(c => !string.IsNullOrWhiteSpace(c)) ?? string.Empty; diff --git a/listenarr.application/Interfaces/IFileNamingService.cs b/listenarr.application/Interfaces/IFileNamingService.cs index 4faba4acf..1e7320834 100644 --- a/listenarr.application/Interfaces/IFileNamingService.cs +++ b/listenarr.application/Interfaces/IFileNamingService.cs @@ -1,12 +1,53 @@ using Listenarr.Domain.Models; +using Listenarr.Domain.Models.Configurations; +using Listenarr.Domain.Models.Naming; namespace Listenarr.Application.Interfaces { + /// + /// Options controlling how assembles a path. + /// + public sealed record NamingOptions + { + /// Root the relative path is combined with (e.g. the configured output path or a root folder). + public string OutputRoot { get; init; } = string.Empty; + + /// + /// When true, the destination folder is already decided (the audiobook has a custom BasePath), so only + /// the file naming pattern is applied — the folder pattern is skipped. + /// + public bool IsCustomBasePath { get; init; } + + /// Whether this file is part of a multi-file audiobook (selects the multi-file naming pattern). + public bool IsMultiFile { get; init; } + + /// Sequence number used to disambiguate multi-file names when the pattern has no Disk/Chapter token. + public int? SequenceNumber { get; init; } + + /// File extension to ensure on the result (e.g. ".m4b"). + public string Extension { get; init; } = ".m4b"; + } + + /// Result of : both the relative path and the full path. + public sealed record NamingResult(string RelativePath, string FullPath); + /// /// Generates file paths using configured naming patterns /// public interface IFileNamingService { + /// + /// Build the relative directory for an audiobook from the configured folder pattern only (no filename). + /// Used when computing an audiobook's BasePath. + /// + string BuildDirectory(NamingContext context, ApplicationSettings settings); + + /// + /// Build the relative and full path for a single audiobook file, applying the folder + file naming + /// patterns, multi-file handling, sanitization and platform path-length limits. + /// + NamingResult BuildPath(NamingContext context, ApplicationSettings settings, NamingOptions options); + /// /// Apply the configured file naming pattern to generate the final file path /// diff --git a/listenarr.domain/Models/Naming/NamingContext.cs b/listenarr.domain/Models/Naming/NamingContext.cs new file mode 100644 index 000000000..d57fad93a --- /dev/null +++ b/listenarr.domain/Models/Naming/NamingContext.cs @@ -0,0 +1,137 @@ +/* + * 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 System.Globalization; + +namespace Listenarr.Domain.Models.Naming +{ + /// + /// Normalized, source-agnostic metadata used to build audiobook file/folder names. + /// The various import/rename flows map their own source type (, + /// , ) into a single + /// so that token substitution, sanitization and path assembly + /// live in exactly one place (the file naming service). Source-specific quirks (e.g. picking an + /// author from noisy file tags) belong in the From(...) factories; processing belongs in + /// the naming service. + /// + public sealed record NamingContext + { + public IReadOnlyList Authors { get; init; } = []; + public IReadOnlyList Narrators { get; init; } = []; + public string? Series { get; init; } + public string? SeriesNumber { get; init; } + public string? Title { get; init; } + public string? Subtitle { get; init; } + public string? Edition { get; init; } + public string? Publisher { get; init; } + public string? Language { get; init; } + public string? Asin { get; init; } + public string? Year { get; init; } + public string? Quality { get; init; } + public int? DiskNumber { get; init; } + public int? ChapterNumber { get; init; } + + /// Build from a library entity (rename, library add, base-path compute). + public static NamingContext From(Audiobook audiobook) => new() + { + Authors = audiobook.Authors ?? [], + Narrators = audiobook.Narrators ?? [], + Series = audiobook.Series, + SeriesNumber = audiobook.SeriesNumber, + Title = audiobook.Title, + Subtitle = audiobook.Subtitle, + Edition = audiobook.Edition, + Publisher = audiobook.Publisher, + Language = audiobook.Language, + Asin = audiobook.Asin, + Year = audiobook.PublishYear, + Quality = audiobook.Quality, + DiskNumber = null, + ChapterNumber = null, + }; + + /// + /// Build from file-extracted (import after download, manual import). + /// Resolves the author from potentially-noisy tags here (an artist/album-artist tag may actually + /// be the narrator or the title), so the naming service can treat as clean. + /// + public static NamingContext From(AudioMetadata metadata) + { + var author = ChooseAuthorFromTags(metadata); + return new NamingContext + { + Authors = string.IsNullOrWhiteSpace(author) ? [] : [author], + Narrators = string.IsNullOrWhiteSpace(metadata.Narrator) ? [] : [metadata.Narrator!], + Series = metadata.Series, + // Match the historical AudioMetadata behavior: fall back to the track number when no + // explicit series position is present. + SeriesNumber = FirstNonEmpty( + metadata.SeriesPosition?.ToString(CultureInfo.InvariantCulture), + metadata.TrackNumber?.ToString(CultureInfo.InvariantCulture)), + Title = metadata.Title, + Subtitle = metadata.Subtitle, + Edition = metadata.Edition, + Publisher = metadata.Publisher, + Language = metadata.Language, + Asin = metadata.Asin, + Year = metadata.Year?.ToString(CultureInfo.InvariantCulture), + Quality = FirstNonEmpty(metadata.BitRate.HasValue ? metadata.BitRate + "kbps" : null, metadata.Format), + DiskNumber = metadata.DiscNumber, + ChapterNumber = metadata.TrackNumber, + }; + } + + /// Build from provider (library add, path preview). + public static NamingContext From(AudibleBookMetadata metadata) => From(metadata.ToAudiobook()); + + private static string? FirstNonEmpty(params string?[] candidates) + => candidates.FirstOrDefault(c => !string.IsNullOrWhiteSpace(c)); + + // Sometimes a file's Artist tag is noisy and actually holds the narrator, the title or the + // series. Prefer the album-artist in that case, otherwise leave it empty so the naming service + // falls back to "Unknown Author". + private static string ChooseAuthorFromTags(AudioMetadata metadata) + { + var primary = NonNarratorCandidate(metadata.Artist, metadata.Narrator); + var alternate = NonNarratorCandidate(metadata.AlbumArtist, metadata.Narrator); + + if (string.IsNullOrWhiteSpace(primary)) + return alternate; + + if (!string.IsNullOrWhiteSpace(metadata.Title) && + (primary.IndexOf(metadata.Title, StringComparison.OrdinalIgnoreCase) >= 0 || + (!string.IsNullOrWhiteSpace(metadata.Series) && string.Equals(primary, metadata.Series, StringComparison.OrdinalIgnoreCase)) || + string.Equals(primary, metadata.Title, StringComparison.OrdinalIgnoreCase))) + return !string.IsNullOrWhiteSpace(alternate) ? alternate : primary; + + return primary; + } + + private static string NonNarratorCandidate(string? candidate, string? narrator) + { + if (string.IsNullOrWhiteSpace(candidate)) + return string.Empty; + + var trimmed = candidate.Trim(); + if (!string.IsNullOrWhiteSpace(narrator) && + string.Equals(trimmed, narrator.Trim(), StringComparison.OrdinalIgnoreCase)) + return string.Empty; + + return trimmed; + } + } +} From f8539d80ef929fc50a2afc0125555d79013cdbd6 Mon Sep 17 00:00:00 2001 From: s3ntin3l8 <58235613+s3ntin3l8@users.noreply.github.com> Date: Tue, 9 Jun 2026 21:46:35 +0000 Subject: [PATCH 05/12] refactor: route all naming flows through the orchestrator (#571) Replace the per-flow variable builders and the duplicated folder/file/legacy branch with the single NamingContext + BuildDirectory/BuildPath orchestrator: - LibraryAddService + LibraryController.ComputeAudiobookBaseDirectoryFromPattern -> BuildDirectory / ApplyNamingPattern(NamingContext) - RenameService.BuildExpectedPath -> BuildPath - ManualImportController -> BuildPath - DownloadImportService -> ApplyNamingPattern(NamingContext), dropping its private invalid-char sanitizer (the shared sanitizer covers it) Delete the now-dead duplicates: SanitizeDirectoryName, per-flow BuildNamingVariables/PatternAllowsSubfolders copies, and the unused AudioMetadata/AudibleBookMetadata ApplyNamingPattern + BuildVariables overloads and ChooseAuthor heuristic (folded into NamingContext). Canonical behavior is now uniform across flows: one strong SanitizePathComponent (":" -> " - "), case-insensitive tokens, "Unknown Author" fallback, and consistent legacy/custom-base/folder+file handling. Manual import now supports {SeriesNumber}/{Quality} and uses the "Unknown Author" folder for author-less books (its multi-file ordering test updated to match). Full backend suite green (705 tests). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Controllers/LibraryController.cs | 40 +----- .../Controllers/ManualImportController.cs | 121 ++---------------- .../Audiobooks/LibraryAddService.cs | 3 +- .../Audiobooks/RenameService.cs | 72 +++-------- .../Common/FileNamingService.cs | 101 +-------------- .../Downloads/DownloadImportService.cs | 56 ++------ .../Interfaces/IFileNamingService.cs | 3 +- .../ManualImportControllerTests.cs | 10 +- 8 files changed, 58 insertions(+), 348 deletions(-) diff --git a/listenarr.api/Controllers/LibraryController.cs b/listenarr.api/Controllers/LibraryController.cs index 397de791a..2712e7bce 100644 --- a/listenarr.api/Controllers/LibraryController.cs +++ b/listenarr.api/Controllers/LibraryController.cs @@ -27,6 +27,7 @@ using Listenarr.Domain.Common; using Listenarr.Application.Interfaces; using Listenarr.Domain.Models.Configurations; +using Listenarr.Domain.Models.Naming; using Listenarr.Application.Interfaces.Repositories; using Listenarr.Application.Notification; using Listenarr.Application.Security; @@ -3499,27 +3500,8 @@ private string ComputeAudiobookBaseDirectoryFromPattern(Audiobook audiobook, str // them. Applying the pattern exactly and letting the sentinel cleanup remove empties keeps // {SeriesNumber} intact. - // Build variables for naming pattern using audiobook-level metadata - var variables = new Dictionary - { - { "Author", SanitizeDirectoryName(audiobook.Authors?.FirstOrDefault() ?? "Unknown Author") }, - { "Series", SanitizeDirectoryName(!string.IsNullOrWhiteSpace(audiobook.Series) ? audiobook.Series! : string.Empty) }, - { "Title", SanitizeDirectoryName(audiobook.Title ?? "Unknown Title") }, - { "Subtitle", SanitizeDirectoryName(audiobook.Subtitle ?? string.Empty) }, - { "Edition", SanitizeDirectoryName(audiobook.Edition ?? string.Empty) }, - { "Narrator", SanitizeDirectoryName((audiobook.Narrators != null && audiobook.Narrators.Any()) ? string.Join(", ", audiobook.Narrators.Where(n => !string.IsNullOrWhiteSpace(n))) : string.Empty) }, - { "Publisher", SanitizeDirectoryName(audiobook.Publisher ?? string.Empty) }, - { "Language", SanitizeDirectoryName(audiobook.Language ?? string.Empty) }, - { "Asin", SanitizeDirectoryName(audiobook.Asin ?? string.Empty) }, - { "SeriesNumber", audiobook.SeriesNumber ?? string.Empty }, - { "Year", audiobook.PublishYear ?? string.Empty }, - { "Quality", string.Empty }, - { "DiskNumber", string.Empty }, - { "ChapterNumber", string.Empty } - }; - - // Apply the directory pattern to get the relative directory path - var relative = _fileNamingService.ApplyNamingPattern(directoryPattern, variables, false); + // Apply the directory pattern using the unified naming variables (single sanitizer + token engine). + var relative = _fileNamingService.ApplyNamingPattern(directoryPattern, NamingContext.From(audiobook), false); // Combine with root path var combined = ResolvePathWithOptionalBase(rootPath, relative); @@ -3629,22 +3611,6 @@ private string GetCommonPath(List paths) return commonPath; } - private string SanitizeDirectoryName(string name) - { - // Remove or replace characters that are invalid in directory names - var invalidChars = Path.GetInvalidFileNameChars(); - foreach (var c in invalidChars) - { - name = name.Replace(c, '_'); - } - - // Also replace some additional characters that might cause issues - name = name.Replace(":", "_").Replace("*", "_").Replace("?", "_").Replace("\"", "_").Replace("<", "_").Replace(">", "_").Replace("|", "_"); - - // Trim whitespace and return - return name.Trim(); - } - private static string ComputeShortHash(string? input) { if (string.IsNullOrEmpty(input)) diff --git a/listenarr.api/Controllers/ManualImportController.cs b/listenarr.api/Controllers/ManualImportController.cs index 9afea0661..13c0f3d49 100644 --- a/listenarr.api/Controllers/ManualImportController.cs +++ b/listenarr.api/Controllers/ManualImportController.cs @@ -21,6 +21,7 @@ using Listenarr.Api.Dtos.ManualImport; using Listenarr.Domain.Models.Enumerations; using Listenarr.Domain.Models.Configurations; +using Listenarr.Domain.Models.Naming; using Listenarr.Application.Interfaces.Repositories; using Listenarr.Domain.Models; @@ -408,9 +409,6 @@ private async Task PersistAudiobookBasePathAsync(Audiobook audiobook, string? ba private async Task GenerateManualImportPathAsync(Audiobook audiobook, AudioMetadata metadata, ManualImportItemDto item, List rootFolders, ApplicationSettings settings, bool isMultiFile = false) { var sourceFilePath = item.FullPath ?? string.Empty; - // Get the configured folder/file naming patterns from settings - var folderPattern = settings.FolderNamingPattern; - var filePattern = isMultiFile ? settings.MultiFileNamingPattern : settings.FileNamingPattern; // If a custom BasePath is set (different from configured OutputPath AND not a known // root folder), store directly under that path using file-only naming. @@ -457,45 +455,6 @@ private async Task GenerateManualImportPathAsync(Audiobook audiobook, Au extension = ".m4b"; // Fallback if no extension } - // Build variables for the pattern - only include non-empty values - var variables = new Dictionary(); - - // Get first author from Authors list - var author = audiobook.Authors?.FirstOrDefault(); - if (!string.IsNullOrWhiteSpace(author)) - variables["Author"] = author; - - var narrator = audiobook.Narrators != null - ? string.Join(", ", audiobook.Narrators.Where(n => !string.IsNullOrWhiteSpace(n))) - : string.Empty; - if (!string.IsNullOrWhiteSpace(narrator)) - variables["Narrator"] = narrator; - - if (!string.IsNullOrWhiteSpace(audiobook.Publisher)) - variables["Publisher"] = audiobook.Publisher; - - if (!string.IsNullOrWhiteSpace(audiobook.Language)) - variables["Language"] = audiobook.Language; - - if (!string.IsNullOrWhiteSpace(audiobook.Asin)) - variables["Asin"] = audiobook.Asin; - - if (!string.IsNullOrWhiteSpace(audiobook.Subtitle)) - variables["Subtitle"] = audiobook.Subtitle; - - if (!string.IsNullOrWhiteSpace(audiobook.Edition)) - variables["Edition"] = audiobook.Edition; - - variables["Title"] = !string.IsNullOrWhiteSpace(audiobook.Title) - ? audiobook.Title - : "Unknown Title"; // Title is required as fallback - - if (!string.IsNullOrWhiteSpace(audiobook.Series)) - variables["Series"] = audiobook.Series; - - if (!string.IsNullOrWhiteSpace(audiobook.PublishYear)) - variables["Year"] = audiobook.PublishYear; - var effectiveDiskNumber = item.DiskNumberHint ?? (metadata.DiscNumber.HasValue && metadata.DiscNumber.Value > 0 ? metadata.DiscNumber.Value : null); var effectiveChapterNumber = item.ChapterNumberHint @@ -507,76 +466,24 @@ private async Task GenerateManualImportPathAsync(Audiobook audiobook, Au effectiveChapterNumber ??= effectiveDiskNumber; } - if (effectiveDiskNumber.HasValue && effectiveDiskNumber.Value > 0) - variables["DiskNumber"] = effectiveDiskNumber.Value; - - if (effectiveChapterNumber.HasValue && effectiveChapterNumber.Value > 0) - variables["ChapterNumber"] = effectiveChapterNumber.Value; - var stableSuffixNumber = effectiveChapterNumber ?? effectiveDiskNumber ?? item.SequenceNumberHint; - string relativePath; - var patternHasNumberTokens = !string.IsNullOrWhiteSpace(filePattern) - && (filePattern.IndexOf("DiskNumber", StringComparison.OrdinalIgnoreCase) >= 0 - || filePattern.IndexOf("ChapterNumber", StringComparison.OrdinalIgnoreCase) >= 0); - - if (string.IsNullOrWhiteSpace(folderPattern)) + var context = NamingContext.From(audiobook) with { - // Legacy behavior: use FileNamingPattern as the full relative path pattern - var legacyPattern = string.IsNullOrWhiteSpace(filePattern) - ? "{Author}/{Title}/{Title}" - : filePattern; + DiskNumber = effectiveDiskNumber.HasValue && effectiveDiskNumber.Value > 0 ? effectiveDiskNumber : null, + ChapterNumber = effectiveChapterNumber.HasValue && effectiveChapterNumber.Value > 0 ? effectiveChapterNumber : null, + }; - relativePath = _fileNamingService.ApplyNamingPattern(legacyPattern, variables, treatAsFilename: false); - } - else if (isCustomBasePath) + // OutputRoot is empty so the relative path is combined with the resolved basePath below. + var result = _fileNamingService.BuildPath(context, settings, new NamingOptions { - // Custom base path: only apply file naming pattern, not folder pattern - // (the BasePath already represents the folder location) - var effectiveFilePattern = string.IsNullOrWhiteSpace(filePattern) ? "{Title}" : filePattern; - - var patternAllowsSubfolders = effectiveFilePattern.IndexOf("DiskNumber", StringComparison.OrdinalIgnoreCase) >= 0 - || effectiveFilePattern.IndexOf("ChapterNumber", StringComparison.OrdinalIgnoreCase) >= 0 - || effectiveFilePattern.IndexOf('/') >= 0 - || effectiveFilePattern.IndexOf('\\') >= 0; - - relativePath = _fileNamingService.ApplyNamingPattern(effectiveFilePattern, variables, treatAsFilename: !patternAllowsSubfolders); - } - else - { - // New behavior: separate folder and file patterns - var effectiveFilePattern = string.IsNullOrWhiteSpace(filePattern) ? "{Title}" : filePattern; - - var folderRelative = _fileNamingService.ApplyNamingPattern(folderPattern, variables, treatAsFilename: false); - - var patternAllowsSubfolders = effectiveFilePattern.IndexOf("DiskNumber", StringComparison.OrdinalIgnoreCase) >= 0 - || effectiveFilePattern.IndexOf("ChapterNumber", StringComparison.OrdinalIgnoreCase) >= 0 - || effectiveFilePattern.IndexOf('/') >= 0 - || effectiveFilePattern.IndexOf('\\') >= 0; - - var fileRelative = _fileNamingService.ApplyNamingPattern(effectiveFilePattern, variables, treatAsFilename: !patternAllowsSubfolders); - - if (isMultiFile && !patternHasNumberTokens && stableSuffixNumber.HasValue) - fileRelative = FileUtils.AppendSequenceSuffix(fileRelative, stableSuffixNumber.Value); - - relativePath = string.IsNullOrWhiteSpace(folderRelative) - ? fileRelative - : CombineWithOptionalBase(folderRelative, fileRelative); - } - - if ((string.IsNullOrWhiteSpace(folderPattern) || isCustomBasePath) - && isMultiFile - && !patternHasNumberTokens - && stableSuffixNumber.HasValue) - { - relativePath = FileUtils.AppendSequenceSuffix(relativePath, stableSuffixNumber.Value); - } - - // Ensure it has the correct extension - if (!relativePath.EndsWith(extension, StringComparison.OrdinalIgnoreCase)) - { - relativePath += extension; - } + OutputRoot = string.Empty, + IsCustomBasePath = isCustomBasePath, + IsMultiFile = isMultiFile, + SequenceNumber = stableSuffixNumber, + Extension = extension, + }); + var relativePath = result.RelativePath; return string.IsNullOrWhiteSpace(basePath) ? relativePath diff --git a/listenarr.application/Audiobooks/LibraryAddService.cs b/listenarr.application/Audiobooks/LibraryAddService.cs index 2a8505c04..1badef6bf 100644 --- a/listenarr.application/Audiobooks/LibraryAddService.cs +++ b/listenarr.application/Audiobooks/LibraryAddService.cs @@ -21,6 +21,7 @@ using Listenarr.Application.Interfaces.Repositories; using Listenarr.Application.Metadata; using Listenarr.Domain.Models; +using Listenarr.Domain.Models.Naming; using Microsoft.Extensions.Logging; namespace Listenarr.Application.Audiobooks @@ -172,7 +173,7 @@ public async Task AddToLibraryAsync( var rootFolder = await _rootFolderService.GetDefaultAsync(); baseDirectory = rootFolder != null ? rootFolder.Path : settings.OutputPath; - audiobook.BasePath = Path.Join(baseDirectory, _fileNamingService.ApplyNamingPattern(settings.FolderNamingPattern, metadata)); + audiobook.BasePath = Path.Join(baseDirectory, _fileNamingService.BuildDirectory(NamingContext.From(audiobook), settings)); } else { diff --git a/listenarr.application/Audiobooks/RenameService.cs b/listenarr.application/Audiobooks/RenameService.cs index c1ed3ea75..105eeb97f 100644 --- a/listenarr.application/Audiobooks/RenameService.cs +++ b/listenarr.application/Audiobooks/RenameService.cs @@ -21,6 +21,7 @@ using Listenarr.Domain.Models; using Listenarr.Domain.Models.Configurations; using Listenarr.Domain.Models.Enumerations; +using Listenarr.Domain.Models.Naming; using Microsoft.Extensions.Logging; namespace Listenarr.Application.Audiobooks @@ -401,62 +402,25 @@ private static List GetFileEntries(Audiobook audiobook) private string BuildExpectedPath(Audiobook audiobook, PreviewFileEntry file, ApplicationSettings settings, string basePath, bool isCustomBasePath, bool isMultiFile) { - var folderPattern = settings.FolderNamingPattern; - var filePattern = isMultiFile ? settings.MultiFileNamingPattern : settings.FileNamingPattern; - var variables = BuildNamingVariables(audiobook, file.SequenceNumber, isMultiFile); - var patternHasNumberTokens = !string.IsNullOrWhiteSpace(filePattern) - && (filePattern.IndexOf("DiskNumber", StringComparison.OrdinalIgnoreCase) >= 0 || filePattern.IndexOf("ChapterNumber", StringComparison.OrdinalIgnoreCase) >= 0); - - string relativePath; - if (string.IsNullOrWhiteSpace(folderPattern)) - { - var legacyPattern = string.IsNullOrWhiteSpace(filePattern) ? "{Author}/{Title}/{Title}" : filePattern; - relativePath = _fileNamingService.ApplyNamingPattern(legacyPattern, variables, false); - } - else if (isCustomBasePath) - { - var effectiveFilePattern = string.IsNullOrWhiteSpace(filePattern) ? "{Title}" : filePattern; - relativePath = _fileNamingService.ApplyNamingPattern(effectiveFilePattern, variables, !PatternAllowsSubfolders(effectiveFilePattern)); - } - else - { - var effectiveFilePattern = string.IsNullOrWhiteSpace(filePattern) ? "{Title}" : filePattern; - var folderRelative = _fileNamingService.ApplyNamingPattern(folderPattern, variables, false); - var fileRelative = _fileNamingService.ApplyNamingPattern(effectiveFilePattern, variables, !PatternAllowsSubfolders(effectiveFilePattern)); - if (isMultiFile && !patternHasNumberTokens) fileRelative = FileUtils.AppendSequenceSuffix(fileRelative, file.SequenceNumber); - relativePath = string.IsNullOrWhiteSpace(folderRelative) ? fileRelative : CombineWithOptionalBase(folderRelative, fileRelative); - } + var context = NamingContext.From(audiobook); + if (isMultiFile) + context = context with { DiskNumber = file.SequenceNumber, ChapterNumber = file.SequenceNumber }; - if ((string.IsNullOrWhiteSpace(folderPattern) || isCustomBasePath) && isMultiFile && !patternHasNumberTokens) - relativePath = FileUtils.AppendSequenceSuffix(relativePath, file.SequenceNumber); - if (!relativePath.EndsWith(file.Extension, StringComparison.OrdinalIgnoreCase)) relativePath += file.Extension; + // OutputRoot is left empty so the relative path is combined with the resolved naming base below, + // preserving this flow's NormalizePath behavior. + var result = _fileNamingService.BuildPath(context, settings, new NamingOptions + { + OutputRoot = string.Empty, + IsCustomBasePath = isCustomBasePath, + IsMultiFile = isMultiFile, + SequenceNumber = isMultiFile ? file.SequenceNumber : null, + Extension = file.Extension, + }); + var relativePath = result.RelativePath; return string.IsNullOrWhiteSpace(basePath) ? NormalizePath(relativePath) : NormalizePath(CombineWithOptionalBase(basePath, relativePath)); } - private static Dictionary BuildNamingVariables(Audiobook audiobook, int sequenceNumber, bool isMultiFile) - { - var narrator = audiobook.Narrators != null ? string.Join(", ", audiobook.Narrators.Where(n => !string.IsNullOrWhiteSpace(n))) : string.Empty; - - return new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { "Author", audiobook.Authors?.FirstOrDefault(n => !string.IsNullOrWhiteSpace(n)) ?? "Unknown Author" }, - { "Series", audiobook.Series ?? string.Empty }, - { "Title", string.IsNullOrWhiteSpace(audiobook.Title) ? "Unknown Title" : audiobook.Title }, - { "Subtitle", audiobook.Subtitle ?? string.Empty }, - { "Edition", audiobook.Edition ?? string.Empty }, - { "Narrator", narrator }, - { "Publisher", audiobook.Publisher ?? string.Empty }, - { "Language", audiobook.Language ?? string.Empty }, - { "Asin", audiobook.Asin ?? string.Empty }, - { "SeriesNumber", audiobook.SeriesNumber ?? string.Empty }, - { "Year", audiobook.PublishYear ?? string.Empty }, - { "Quality", audiobook.Quality ?? string.Empty }, - { "DiskNumber", isMultiFile ? sequenceNumber : string.Empty }, - { "ChapterNumber", isMultiFile ? sequenceNumber : string.Empty } - }; - } - private async Task> LoadRootFoldersAsync() { if (_rootFolderService == null) return new(); @@ -562,12 +526,6 @@ private static bool PathsEqual(string? left, string? right) private static bool IsSamePathOrWithin(string childPath, string rootPath) => PathsEqual(childPath, rootPath) || FileUtils.IsPathInsideOf(childPath, rootPath); - private static bool PatternAllowsSubfolders(string pattern) - => pattern.IndexOf("DiskNumber", StringComparison.OrdinalIgnoreCase) >= 0 - || pattern.IndexOf("ChapterNumber", StringComparison.OrdinalIgnoreCase) >= 0 - || pattern.IndexOf('/') >= 0 - || pattern.IndexOf('\\') >= 0; - private sealed record PreviewFileEntry(int FileId, string CurrentPath, string Extension, int SequenceNumber); } } diff --git a/listenarr.application/Common/FileNamingService.cs b/listenarr.application/Common/FileNamingService.cs index eac7f859a..760605793 100644 --- a/listenarr.application/Common/FileNamingService.cs +++ b/listenarr.application/Common/FileNamingService.cs @@ -15,7 +15,6 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . */ -using System.Globalization; using System.Runtime.InteropServices; using System.Text; using System.Text.RegularExpressions; @@ -289,15 +288,9 @@ public string ApplyNamingPattern(string pattern, Dictionary vari return result; } - public string ApplyNamingPattern(string pattern, AudioMetadata metadata, bool treatAsFilename = false) + public string ApplyNamingPattern(string pattern, NamingContext context, bool treatAsFilename = false) { - var variables = BuildVariables(metadata); - return ApplyNamingPattern(pattern, variables, treatAsFilename); - } - - public string ApplyNamingPattern(string pattern, AudibleBookMetadata metadata, bool treatAsFilename = false) - { - var variables = BuildVariables(metadata); + var variables = BuildVariables(context); return ApplyNamingPattern(pattern, variables, treatAsFilename); } @@ -359,58 +352,6 @@ private string SanitizePathComponent(string pathComponent) return result; } - private Dictionary BuildVariables(AudioMetadata metadata) - { - return new Dictionary - { - // Keep multi-word author names as a single folder name (e.g. "Jane Austen") - { "Author", SanitizePathComponent(FirstNonEmpty(ChooseAuthor(metadata), "Unknown Author")) }, - // For Series we must not fallback to Album or Title - when Series is blank we want - // the variable to be empty so ApplyNamingPattern can remove any adjacent separators - { "Series", string.IsNullOrWhiteSpace(metadata.Series) ? string.Empty : SanitizePathComponent(metadata.Series) }, - { "Title", SanitizePathComponent(FirstNonEmpty(metadata.Title, "Unknown Title")) }, - { "Subtitle", string.IsNullOrWhiteSpace(metadata.Subtitle) ? string.Empty : SanitizePathComponent(metadata.Subtitle) }, - { "Edition", string.IsNullOrWhiteSpace(metadata.Edition) ? string.Empty : SanitizePathComponent(metadata.Edition) }, - { "Narrator", string.IsNullOrWhiteSpace(metadata.Narrator) ? string.Empty : SanitizePathComponent(metadata.Narrator) }, - { "Publisher", string.IsNullOrWhiteSpace(metadata.Publisher) ? string.Empty : SanitizePathComponent(metadata.Publisher) }, - { "Language", string.IsNullOrWhiteSpace(metadata.Language) ? string.Empty : SanitizePathComponent(metadata.Language) }, - { "Asin", string.IsNullOrWhiteSpace(metadata.Asin) ? string.Empty : SanitizePathComponent(metadata.Asin) }, - { "SeriesNumber", FirstNonEmpty(metadata.SeriesPosition?.ToString(CultureInfo.InvariantCulture), metadata.TrackNumber?.ToString()) }, - { "Year", FirstNonEmpty(metadata.Year?.ToString()) }, - { "Quality", FirstNonEmpty(metadata.BitRate.HasValue ? metadata.BitRate + "kbps" : null, metadata.Format) }, - { "DiskNumber", metadata.DiscNumber?.ToString() ?? string.Empty }, - { "ChapterNumber", metadata.TrackNumber?.ToString() ?? string.Empty } - }; - } - - private Dictionary BuildVariables(AudibleBookMetadata metadata) - { - var author = metadata.Author ?? "Unknown Author"; - if (metadata.Authors != null && metadata.Authors.Count > 0) - { - // Assume first one is the main author - author = metadata.Authors.First(); - } - - return new Dictionary - { - { "Author", SanitizePathComponent(author) }, - { "Series", string.IsNullOrWhiteSpace(metadata.Series) ? string.Empty : SanitizePathComponent(metadata.Series) }, - { "Title", SanitizePathComponent(FirstNonEmpty(metadata.Title, "Unknown Title")) }, - { "Subtitle", string.IsNullOrWhiteSpace(metadata.Subtitle) ? string.Empty : SanitizePathComponent(metadata.Subtitle) }, - { "Edition", string.IsNullOrWhiteSpace(metadata.Edition) ? string.Empty : SanitizePathComponent(metadata.Edition) }, - { "Narrator", string.IsNullOrWhiteSpace(metadata.Narrator) ? string.Empty : SanitizePathComponent(metadata.Narrator) }, - { "Publisher", string.IsNullOrWhiteSpace(metadata.Publisher) ? string.Empty : SanitizePathComponent(metadata.Publisher) }, - { "Language", string.IsNullOrWhiteSpace(metadata.Language) ? string.Empty : SanitizePathComponent(metadata.Language) }, - { "Asin", string.IsNullOrWhiteSpace(metadata.Asin) ? string.Empty : SanitizePathComponent(metadata.Asin) }, - { "SeriesNumber", metadata.SeriesNumber?.ToString() ?? string.Empty }, - { "Year", metadata.PublishYear?.ToString() ?? string.Empty }, - { "Quality", string.Empty }, - { "DiskNumber", string.Empty }, - { "ChapterNumber", string.Empty } - }; - } - // Unified variable builder used by the orchestrator (BuildDirectory/BuildPath). All flows map their // source type into a NamingContext, so token sanitization and empty-handling live in one place. // The dictionary is case-insensitive so patterns like {author} resolve as well as {Author}. @@ -445,44 +386,6 @@ private static string FirstNonEmpty(params string?[] candidates) return candidates.FirstOrDefault(c => !string.IsNullOrWhiteSpace(c)) ?? string.Empty; } - // Heuristic: sometimes metadata.Artist can contain the title/series (noisy tags). - // Prefer an AlbumArtist or alternate artist value if the primary artist looks like the title/series. - private static string ChooseAuthor(AudioMetadata metadata) - { - var primary = NonNarratorAuthorCandidate(metadata.Artist, metadata.Narrator); - var alternate = NonNarratorAuthorCandidate(metadata.AlbumArtist, metadata.Narrator); - - if (string.IsNullOrWhiteSpace(primary)) - { - return alternate; - } - - if (!string.IsNullOrWhiteSpace(metadata.Title) && - (primary.IndexOf(metadata.Title, StringComparison.OrdinalIgnoreCase) >= 0 || - (!string.IsNullOrWhiteSpace(metadata.Series) && string.Equals(primary, metadata.Series, StringComparison.OrdinalIgnoreCase)) || - string.Equals(primary, metadata.Title, StringComparison.OrdinalIgnoreCase))) - return !string.IsNullOrWhiteSpace(alternate) ? alternate : primary; - - return string.IsNullOrWhiteSpace(primary) ? alternate : primary; - } - - private static string NonNarratorAuthorCandidate(string? candidate, string? narrator) - { - if (string.IsNullOrWhiteSpace(candidate)) - { - return string.Empty; - } - - var trimmedCandidate = candidate.Trim(); - if (!string.IsNullOrWhiteSpace(narrator) && - string.Equals(trimmedCandidate, narrator.Trim(), StringComparison.OrdinalIgnoreCase)) - { - return string.Empty; - } - - return trimmedCandidate; - } - private static HashSet BuildPortableInvalidFileNameChars() { var invalidChars = new HashSet(Path.GetInvalidFileNameChars()); diff --git a/listenarr.application/Downloads/DownloadImportService.cs b/listenarr.application/Downloads/DownloadImportService.cs index 7b3386e6a..252a18ba5 100644 --- a/listenarr.application/Downloads/DownloadImportService.cs +++ b/listenarr.application/Downloads/DownloadImportService.cs @@ -20,6 +20,7 @@ using Listenarr.Domain.Common; using Listenarr.Domain.Models; using Listenarr.Domain.Models.Enumerations; +using Listenarr.Domain.Models.Naming; using Microsoft.Extensions.Logging; namespace Listenarr.Application.Downloads @@ -207,28 +208,19 @@ public async Task> ImportDownloadFilesAsync(Audiobook audiobo effectiveDiskNumber ??= effectiveChapterNumber; effectiveChapterNumber ??= effectiveDiskNumber; } - var stableSuffixNumber = effectiveChapterNumber ?? effectiveDiskNumber ?? plan?.SequenceNumber; - // Build variables for naming patterns (used for both folder and file patterns) - var variablesForFile = new Dictionary + // Map the file's naming metadata into the unified NamingContext. The Title + // falls back to the source filename, and disk/chapter use the effective values + // derived from hints above. + var context = NamingContext.From(namingMetadata) with { - { "Author", namingMetadata.Artist ?? "Unknown Author" }, - { "Series", string.IsNullOrWhiteSpace(namingMetadata.Series) ? string.Empty : namingMetadata.Series }, - { "Title", namingMetadata.Title ?? Path.GetFileNameWithoutExtension(file) }, - { "Subtitle", string.IsNullOrWhiteSpace(namingMetadata.Subtitle) ? string.Empty : namingMetadata.Subtitle }, - { "Edition", string.IsNullOrWhiteSpace(namingMetadata.Edition) ? string.Empty : namingMetadata.Edition }, - { "Narrator", string.IsNullOrWhiteSpace(namingMetadata.Narrator) ? string.Empty : namingMetadata.Narrator }, - { "Publisher", string.IsNullOrWhiteSpace(namingMetadata.Publisher) ? string.Empty : namingMetadata.Publisher }, - { "Language", string.IsNullOrWhiteSpace(namingMetadata.Language) ? string.Empty : namingMetadata.Language }, - { "Asin", string.IsNullOrWhiteSpace(namingMetadata.Asin) ? string.Empty : namingMetadata.Asin }, - { "SeriesNumber", namingMetadata.SeriesPosition?.ToString() ?? effectiveChapterNumber?.ToString() ?? string.Empty }, - { "Year", namingMetadata.Year?.ToString() ?? string.Empty }, - { "Quality", (namingMetadata.BitRate.HasValue ? $"{namingMetadata.BitRate}kbps" : null) ?? namingMetadata.Format ?? string.Empty }, - { "DiskNumber", effectiveDiskNumber?.ToString() ?? string.Empty }, - { "ChapterNumber", effectiveChapterNumber?.ToString() ?? string.Empty } + Title = !string.IsNullOrWhiteSpace(namingMetadata.Title) ? namingMetadata.Title : Path.GetFileNameWithoutExtension(file), + SeriesNumber = namingMetadata.SeriesPosition?.ToString() ?? effectiveChapterNumber?.ToString(), + DiskNumber = effectiveDiskNumber, + ChapterNumber = effectiveChapterNumber, }; - var folderRelative = fileNamingService.ApplyNamingPattern(folderPattern, variablesForFile, treatAsFilename: false); + var folderRelative = fileNamingService.ApplyNamingPattern(folderPattern, context, treatAsFilename: false); if (string.IsNullOrEmpty(audiobook.BasePath) && !string.IsNullOrWhiteSpace(folderRelative)) { destDirForFile = CombineWithOptionalBase(destDirForFile, folderRelative); @@ -237,9 +229,6 @@ public async Task> ImportDownloadFilesAsync(Audiobook audiobo var baseFilePattern = isMultiFileBatch ? settings.MultiFileNamingPattern : settings.FileNamingPattern; var ext = Path.GetExtension(file); - var patternHasNumberTokens = !string.IsNullOrWhiteSpace(baseFilePattern) - && (baseFilePattern.IndexOf("DiskNumber", StringComparison.OrdinalIgnoreCase) >= 0 - || baseFilePattern.IndexOf("ChapterNumber", StringComparison.OrdinalIgnoreCase) >= 0); var patternAllowsSubfolders = baseFilePattern.IndexOf("DiskNumber", StringComparison.OrdinalIgnoreCase) >= 0 || baseFilePattern.Contains("ChapterNumber", StringComparison.OrdinalIgnoreCase) @@ -247,27 +236,10 @@ public async Task> ImportDownloadFilesAsync(Audiobook audiobo || baseFilePattern.Contains('\\'); var treatAsFilename = !patternAllowsSubfolders; - var filename = fileNamingService.ApplyNamingPattern(baseFilePattern, variablesForFile, treatAsFilename); - if (!filename.EndsWith(ext, StringComparison.OrdinalIgnoreCase)) filename += ext; // FIXME: Should be in ApplyNamingPattern - - if (!patternAllowsSubfolders) - { - try - { - var forced = Path.GetFileName(filename); - var invalid = Path.GetInvalidFileNameChars(); - var sb = new System.Text.StringBuilder(); - foreach (var c in forced) - { - sb.Append(invalid.Contains(c) ? '_' : c); - } - filename = sb.ToString(); - } - catch (Exception exception) when (exception is not (OperationCanceledException or OutOfMemoryException or StackOverflowException)) - { - filename = Path.GetFileName(filename); - } - } + // ApplyNamingPattern already sanitizes (and, for filename mode, strips any + // directory separators) via the shared SanitizePathComponent path. + var filename = fileNamingService.ApplyNamingPattern(baseFilePattern, context, treatAsFilename); + if (!filename.EndsWith(ext, StringComparison.OrdinalIgnoreCase)) filename += ext; var destination = CombineWithOptionalBase(destDirForFile, filename); diff --git a/listenarr.application/Interfaces/IFileNamingService.cs b/listenarr.application/Interfaces/IFileNamingService.cs index 1e7320834..e448d373a 100644 --- a/listenarr.application/Interfaces/IFileNamingService.cs +++ b/listenarr.application/Interfaces/IFileNamingService.cs @@ -73,7 +73,6 @@ public interface IFileNamingService /// Whether to treat as filename (sanitize invalid chars) /// Final path with variables replaced string ApplyNamingPattern(string pattern, Dictionary variables, bool treatAsFilename = false); // FIXME: Should be private - string ApplyNamingPattern(string pattern, AudioMetadata metadata, bool treatAsFilename = false); - string ApplyNamingPattern(string pattern, AudibleBookMetadata metadata, bool treatAsFilename = false); + string ApplyNamingPattern(string pattern, NamingContext context, bool treatAsFilename = false); } } diff --git a/tests/Features/Api/Controllers/ManualImportControllerTests.cs b/tests/Features/Api/Controllers/ManualImportControllerTests.cs index dcaab854c..0bff7d56d 100644 --- a/tests/Features/Api/Controllers/ManualImportControllerTests.cs +++ b/tests/Features/Api/Controllers/ManualImportControllerTests.cs @@ -255,12 +255,16 @@ public async Task InteractiveManualImport_MultipartFiles_UsesStableNaturalOrderA .Select(Path.GetFileName) .ToList(); + // The book has no author, so the unified naming applies the "{Author}" folder pattern as + // "Unknown Author" (the convention shared by every flow). Manual import previously omitted the + // empty author folder; it now matches the other flows. + var authorDir = Path.Join(basePath, "Unknown Author"); Assert.Contains("Ordered Book-01.mp3", diskFiles); Assert.Contains("Ordered Book-02.mp3", diskFiles); Assert.Contains("Ordered Book-10.mp3", diskFiles); - Assert.Equal("one", await File.ReadAllTextAsync(Path.Join(basePath, "Ordered Book-01.mp3"))); - Assert.Equal("two", await File.ReadAllTextAsync(Path.Join(basePath, "Ordered Book-02.mp3"))); - Assert.Equal("ten", await File.ReadAllTextAsync(Path.Join(basePath, "Ordered Book-10.mp3"))); + Assert.Equal("one", await File.ReadAllTextAsync(Path.Join(authorDir, "Ordered Book-01.mp3"))); + Assert.Equal("two", await File.ReadAllTextAsync(Path.Join(authorDir, "Ordered Book-02.mp3"))); + Assert.Equal("ten", await File.ReadAllTextAsync(Path.Join(authorDir, "Ordered Book-10.mp3"))); } [Fact] From 1c9fdde110a68ec47632590639676d82b2b5a7a1 Mon Sep 17 00:00:00 2001 From: s3ntin3l8 <58235613+s3ntin3l8@users.noreply.github.com> Date: Tue, 9 Jun 2026 21:49:10 +0000 Subject: [PATCH 06/12] test: lock in unified naming orchestrator behavior (#571) Direct BuildDirectory/BuildPath tests covering the canonical decisions: case-insensitive tokens, ":" -> " - " sanitization, "Unknown Author" fallback, empty-{Series} separator collapse, multi-file sequence suffix, and custom-base-path file-only naming. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../FileNamingService_OrchestratorTests.cs | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 tests/Features/Api/Services/FileNamingService_OrchestratorTests.cs diff --git a/tests/Features/Api/Services/FileNamingService_OrchestratorTests.cs b/tests/Features/Api/Services/FileNamingService_OrchestratorTests.cs new file mode 100644 index 000000000..1761dfbb8 --- /dev/null +++ b/tests/Features/Api/Services/FileNamingService_OrchestratorTests.cs @@ -0,0 +1,125 @@ +/* + * 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 Xunit; +using Moq; +using Microsoft.Extensions.Logging; +using Listenarr.Application.Common; +using Listenarr.Application.Interfaces; +using Listenarr.Domain.Models; +using Listenarr.Domain.Models.Configurations; +using Listenarr.Domain.Models.Naming; + +namespace Listenarr.Tests.Features.Api.Services +{ + /// + /// Tests for the unified naming orchestrator (BuildDirectory/BuildPath) that every flow now routes + /// through. These lock in the canonical behavior chosen during the naming-pattern consolidation. + /// + [Trait("Category", "FileNamingService")] + public class FileNamingService_OrchestratorTests + { + private readonly FileNamingService _service = + new(new Mock().Object, new Mock>().Object); + + [Fact] + public void BuildDirectory_LowercaseTokens_ResolveCaseInsensitively() + { + // Given a pattern using lowercase tokens + var settings = new ApplicationSettings { FolderNamingPattern = "{author}/{title}" }; + var ctx = NamingContext.From(new Audiobook { Title = "The Gunslinger", Authors = new() { "Stephen King" } }); + + // When / Then the case-insensitive variable dictionary still resolves them + Assert.Equal(Path.Join("Stephen King", "The Gunslinger"), _service.BuildDirectory(ctx, settings)); + } + + [Fact] + public void BuildDirectory_TitleWithColon_UsesStrongSanitizer() + { + // Given a title containing a colon + var settings = new ApplicationSettings { FolderNamingPattern = "{Author}/{Title}" }; + var ctx = NamingContext.From(new Audiobook { Title = "Vol 1: The Beginning", Authors = new() { "Jane Doe" } }); + + // When / Then the single canonical sanitizer renders ":" as " - " (not "_") + Assert.Equal(Path.Join("Jane Doe", "Vol 1 - The Beginning"), _service.BuildDirectory(ctx, settings)); + } + + [Fact] + public void BuildDirectory_NoAuthor_FallsBackToUnknownAuthor() + { + var settings = new ApplicationSettings { FolderNamingPattern = "{Author}/{Title}" }; + var ctx = NamingContext.From(new Audiobook { Title = "Orphan" }); + + Assert.Equal(Path.Join("Unknown Author", "Orphan"), _service.BuildDirectory(ctx, settings)); + } + + [Fact] + public void BuildDirectory_EmptySeries_CollapsesSeparators() + { + var settings = new ApplicationSettings { FolderNamingPattern = "{Author}/{Series}/{Title}" }; + var ctx = NamingContext.From(new Audiobook { Title = "Standalone", Authors = new() { "Jane Doe" } }); + + // Empty {Series} is removed along with its surrounding separator. + Assert.Equal(Path.Join("Jane Doe", "Standalone"), _service.BuildDirectory(ctx, settings)); + } + + [Fact] + public void BuildPath_MultiFile_NoNumberToken_AppendsSequenceSuffix() + { + var settings = new ApplicationSettings + { + OutputPath = "/audiobooks", + FolderNamingPattern = "{Author}", + FileNamingPattern = "{Title}", + MultiFileNamingPattern = "{Title}", // deliberately no Disk/Chapter token + }; + var ctx = NamingContext.From(new Audiobook { Title = "Book", Authors = new() { "Author" } }); + + var result = _service.BuildPath(ctx, settings, new NamingOptions + { + OutputRoot = "/audiobooks", + IsMultiFile = true, + SequenceNumber = 3, + Extension = ".mp3", + }); + + Assert.EndsWith("Book-03.mp3", result.RelativePath); + } + + [Fact] + public void BuildPath_CustomBasePath_AppliesFileOnly() + { + var settings = new ApplicationSettings + { + OutputPath = "/audiobooks", + FolderNamingPattern = "{Author}/{Series}", + FileNamingPattern = "{Title}", + }; + var ctx = NamingContext.From(new Audiobook { Title = "Book", Authors = new() { "Author" }, Series = "Series" }); + + var result = _service.BuildPath(ctx, settings, new NamingOptions + { + OutputRoot = "/custom/base", + IsCustomBasePath = true, + Extension = ".m4b", + }); + + // The folder pattern is skipped under a custom base path — only the file name is produced. + Assert.Equal("Book.m4b", result.RelativePath); + } + } +} From d653cb6470ceec06804f6e009a5d011d9014aa62 Mon Sep 17 00:00:00 2001 From: s3ntin3l8 <58235613+s3ntin3l8@users.noreply.github.com> Date: Tue, 9 Jun 2026 21:58:19 +0000 Subject: [PATCH 07/12] fix: keep author from Artist tag for author-narrated imports (#571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The consolidation routed DownloadImportService through NamingContext.From(AudioMetadata), which had folded in the narrator-avoidance heuristic from the (dead-in-prod) GenerateFilePathAsync. That heuristic drops the author when Artist == Narrator — common for self-narrated memoirs — sending those imports to "Unknown Author/". Take the author straight from the Artist tag (matching the historical auto-import behavior) and drop the heuristic. Adds a regression test for the author-narrated case. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Models/Naming/NamingContext.cs | 41 ++----------------- .../FileNamingService_OrchestratorTests.cs | 11 +++++ 2 files changed, 15 insertions(+), 37 deletions(-) diff --git a/listenarr.domain/Models/Naming/NamingContext.cs b/listenarr.domain/Models/Naming/NamingContext.cs index d57fad93a..70cc00dcf 100644 --- a/listenarr.domain/Models/Naming/NamingContext.cs +++ b/listenarr.domain/Models/Naming/NamingContext.cs @@ -66,15 +66,15 @@ public sealed record NamingContext /// /// Build from file-extracted (import after download, manual import). - /// Resolves the author from potentially-noisy tags here (an artist/album-artist tag may actually - /// be the narrator or the title), so the naming service can treat as clean. + /// The author is taken straight from the Artist tag — matching the historical import behavior — + /// so a book whose author also narrates it (Artist == Narrator) keeps the author rather than + /// collapsing to "Unknown Author". /// public static NamingContext From(AudioMetadata metadata) { - var author = ChooseAuthorFromTags(metadata); return new NamingContext { - Authors = string.IsNullOrWhiteSpace(author) ? [] : [author], + Authors = string.IsNullOrWhiteSpace(metadata.Artist) ? [] : [metadata.Artist], Narrators = string.IsNullOrWhiteSpace(metadata.Narrator) ? [] : [metadata.Narrator!], Series = metadata.Series, // Match the historical AudioMetadata behavior: fall back to the track number when no @@ -100,38 +100,5 @@ public static NamingContext From(AudioMetadata metadata) private static string? FirstNonEmpty(params string?[] candidates) => candidates.FirstOrDefault(c => !string.IsNullOrWhiteSpace(c)); - - // Sometimes a file's Artist tag is noisy and actually holds the narrator, the title or the - // series. Prefer the album-artist in that case, otherwise leave it empty so the naming service - // falls back to "Unknown Author". - private static string ChooseAuthorFromTags(AudioMetadata metadata) - { - var primary = NonNarratorCandidate(metadata.Artist, metadata.Narrator); - var alternate = NonNarratorCandidate(metadata.AlbumArtist, metadata.Narrator); - - if (string.IsNullOrWhiteSpace(primary)) - return alternate; - - if (!string.IsNullOrWhiteSpace(metadata.Title) && - (primary.IndexOf(metadata.Title, StringComparison.OrdinalIgnoreCase) >= 0 || - (!string.IsNullOrWhiteSpace(metadata.Series) && string.Equals(primary, metadata.Series, StringComparison.OrdinalIgnoreCase)) || - string.Equals(primary, metadata.Title, StringComparison.OrdinalIgnoreCase))) - return !string.IsNullOrWhiteSpace(alternate) ? alternate : primary; - - return primary; - } - - private static string NonNarratorCandidate(string? candidate, string? narrator) - { - if (string.IsNullOrWhiteSpace(candidate)) - return string.Empty; - - var trimmed = candidate.Trim(); - if (!string.IsNullOrWhiteSpace(narrator) && - string.Equals(trimmed, narrator.Trim(), StringComparison.OrdinalIgnoreCase)) - return string.Empty; - - return trimmed; - } } } diff --git a/tests/Features/Api/Services/FileNamingService_OrchestratorTests.cs b/tests/Features/Api/Services/FileNamingService_OrchestratorTests.cs index 1761dfbb8..6f8f4c573 100644 --- a/tests/Features/Api/Services/FileNamingService_OrchestratorTests.cs +++ b/tests/Features/Api/Services/FileNamingService_OrchestratorTests.cs @@ -58,6 +58,17 @@ public void BuildDirectory_TitleWithColon_UsesStrongSanitizer() Assert.Equal(Path.Join("Jane Doe", "Vol 1 - The Beginning"), _service.BuildDirectory(ctx, settings)); } + [Fact] + public void From_AudioMetadata_AuthorNarratedBook_KeepsAuthor() + { + // A memoir whose author also narrates it has Artist == Narrator. The author must be kept, + // not collapsed to "Unknown Author" (the auto-import path takes Artist directly). + var settings = new ApplicationSettings { FolderNamingPattern = "{Author}/{Title}" }; + var ctx = NamingContext.From(new AudioMetadata { Title = "Bossypants", Artist = "Tina Fey", Narrator = "Tina Fey" }); + + Assert.Equal(Path.Join("Tina Fey", "Bossypants"), _service.BuildDirectory(ctx, settings)); + } + [Fact] public void BuildDirectory_NoAuthor_FallsBackToUnknownAuthor() { From 60b403a9722fa8ddd542633b978c7c338ec58ece Mon Sep 17 00:00:00 2001 From: s3ntin3l8 <58235613+s3ntin3l8@users.noreply.github.com> Date: Tue, 9 Jun 2026 22:50:28 +0000 Subject: [PATCH 08/12] refactor: route DownloadImportService through BuildPath (#571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review finding #1: DownloadImport had consolidated its variable building onto NamingContext but still assembled the destination with direct ApplyNamingPattern calls, bypassing the shared orchestrator. Route it through BuildPath like the other flows — a set BasePath means file-only naming (IsCustomBasePath), an empty BasePath applies the folder pattern. This also brings EnsurePathWithinLimits to the auto-import path and adopts the multi-file sequence suffix, fixing a latent overwrite when a multi-file download uses a pattern without {DiskNumber}/{ChapterNumber}. Remove the now-dead folderPattern/destDirForFile/baseFilePattern/ patternAllowsSubfolders/treatAsFilename locals. Add a regression test for the suffix collision fix, and a doc note on the AudibleBookMetadata -> ToAudiobook coupling (review finding #5). Full backend suite green (713 tests). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Downloads/DownloadImportService.cs | 38 ++++++----------- .../Models/Naming/NamingContext.cs | 5 ++- .../Downloads/DownloadImportServiceTests.cs | 41 +++++++++++++++++++ 3 files changed, 58 insertions(+), 26 deletions(-) diff --git a/listenarr.application/Downloads/DownloadImportService.cs b/listenarr.application/Downloads/DownloadImportService.cs index 252a18ba5..68747c9f8 100644 --- a/listenarr.application/Downloads/DownloadImportService.cs +++ b/listenarr.application/Downloads/DownloadImportService.cs @@ -71,7 +71,6 @@ public async Task> ImportDownloadFilesAsync(Audiobook audiobo } var results = new List(); - var folderPattern = settings.FolderNamingPattern; var sourceFiles = files .Where(file => !FileUtils.IsBlacklistedFile(file, settings.ImportBlacklistExtensions)) .Distinct(StringComparer.OrdinalIgnoreCase) @@ -196,9 +195,6 @@ public async Task> ImportDownloadFilesAsync(Audiobook audiobo logger.LogDebug(exception, $"ImportFilesFromDirectory: Failed to evaluate quality for multi-file import {file}"); } - // Determine destination directory (prefer audiobook basepath) - string destDirForFile = audiobook.BasePath; - // Build naming metadata: prefer audiobook metadata when available, otherwise use extracted candidate metadata var namingMetadata = BuildNamingMetadata(audiobook, candidateMetadata, Path.GetFileNameWithoutExtension(file)); var effectiveDiskNumber = namingDiskNumber > 0 ? namingDiskNumber : (namingMetadata.DiscNumber ?? plan?.DiskNumberHint); @@ -208,6 +204,7 @@ public async Task> ImportDownloadFilesAsync(Audiobook audiobo effectiveDiskNumber ??= effectiveChapterNumber; effectiveChapterNumber ??= effectiveDiskNumber; } + var stableSuffixNumber = effectiveChapterNumber ?? effectiveDiskNumber ?? plan?.SequenceNumber; // Map the file's naming metadata into the unified NamingContext. The Title // falls back to the source filename, and disk/chapter use the effective values @@ -220,28 +217,19 @@ public async Task> ImportDownloadFilesAsync(Audiobook audiobo ChapterNumber = effectiveChapterNumber, }; - var folderRelative = fileNamingService.ApplyNamingPattern(folderPattern, context, treatAsFilename: false); - if (string.IsNullOrEmpty(audiobook.BasePath) && !string.IsNullOrWhiteSpace(folderRelative)) + // Route through the shared orchestrator. A set BasePath means the destination + // folder is already chosen (file-only naming); an empty BasePath applies the + // folder pattern. BuildPath also enforces path-length limits and appends the + // multi-file sequence suffix when the pattern has no Disk/Chapter token. + var result = fileNamingService.BuildPath(context, settings, new NamingOptions { - destDirForFile = CombineWithOptionalBase(destDirForFile, folderRelative); - } - - var baseFilePattern = isMultiFileBatch ? settings.MultiFileNamingPattern : settings.FileNamingPattern; - - var ext = Path.GetExtension(file); - - var patternAllowsSubfolders = baseFilePattern.IndexOf("DiskNumber", StringComparison.OrdinalIgnoreCase) >= 0 - || baseFilePattern.Contains("ChapterNumber", StringComparison.OrdinalIgnoreCase) - || baseFilePattern.Contains('/') - || baseFilePattern.Contains('\\'); - var treatAsFilename = !patternAllowsSubfolders; - - // ApplyNamingPattern already sanitizes (and, for filename mode, strips any - // directory separators) via the shared SanitizePathComponent path. - var filename = fileNamingService.ApplyNamingPattern(baseFilePattern, context, treatAsFilename); - if (!filename.EndsWith(ext, StringComparison.OrdinalIgnoreCase)) filename += ext; - - var destination = CombineWithOptionalBase(destDirForFile, filename); + OutputRoot = audiobook.BasePath ?? string.Empty, + IsCustomBasePath = !string.IsNullOrEmpty(audiobook.BasePath), + IsMultiFile = isMultiFileBatch, + SequenceNumber = stableSuffixNumber, + Extension = Path.GetExtension(file), + }); + var destination = result.FullPath; if (!await fileMover.PerformActionOn(completedFileAction, file, destination)) { diff --git a/listenarr.domain/Models/Naming/NamingContext.cs b/listenarr.domain/Models/Naming/NamingContext.cs index 70cc00dcf..4daa85a1b 100644 --- a/listenarr.domain/Models/Naming/NamingContext.cs +++ b/listenarr.domain/Models/Naming/NamingContext.cs @@ -95,7 +95,10 @@ public static NamingContext From(AudioMetadata metadata) }; } - /// Build from provider (library add, path preview). + /// + /// Build from provider (library add, path preview). Field coverage + /// follows — a field it does not map is absent here. + /// public static NamingContext From(AudibleBookMetadata metadata) => From(metadata.ToAudiobook()); private static string? FirstNonEmpty(params string?[] candidates) diff --git a/tests/Features/Application/Downloads/DownloadImportServiceTests.cs b/tests/Features/Application/Downloads/DownloadImportServiceTests.cs index 2ce966677..90e134cbc 100644 --- a/tests/Features/Application/Downloads/DownloadImportServiceTests.cs +++ b/tests/Features/Application/Downloads/DownloadImportServiceTests.cs @@ -381,6 +381,47 @@ public async Task ImportDownloadFilesAsync_MultipartFiles_KeepNaturalOrderWhenRe Assert.Equal("ten", await File.ReadAllTextAsync(mapped[part10])); } + [Fact] + public async Task ImportDownloadFilesAsync_MultiFile_NoNumberToken_AppendsSuffixToAvoidCollision() + { + // Multi-file download whose pattern has no {DiskNumber}/{ChapterNumber} token: every file would + // otherwise resolve to the same name and overwrite. Routing through BuildPath now appends a + // stable -NN sequence suffix, matching rename/manual-import behavior. + var outputDir = FileService.GetTempDirectory("listenarr-import-nosuffix"); + + var srcDir = FileService.GetTempDirectory("listenarr-import-nosuffix-src"); + var part2 = await FileService.GetFileAsync(srcDir, "Part 2.mp3", "two"); + var part1 = await FileService.GetFileAsync(srcDir, "Part 1.mp3", "one"); + + metadataServiceMock.AddMetadata(@"\.mp3$", new AudioMetadata { Title = "No Token Book", Format = "mp3", BitRate = 128000 }); + + var audiobook = await _audiobookRepository.AddAsync(new AudiobookBuilder() + .WithBasePath(outputDir) + .Build()); + + await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() + .WithOutputPath(outputDir) + .WithMetadataProcessing() + .WithCopyFileOnCompleted() + .WithFolderNamingPattern("") + .WithFileNamingPattern("{Title}") + .WithMultiFileNamingPattern("{Title}") // no Disk/Chapter token -> would collide without a suffix + .Build()); + + var downloadImportService = _provider.GetRequiredService(); + var results = await downloadImportService.ImportDownloadFilesAsync(audiobook, [part2, part1]); + + var mapped = results + .Where(r => r.Success && !string.IsNullOrWhiteSpace(r.FinalPath) && !string.IsNullOrWhiteSpace(r.SourcePath)) + .ToDictionary(r => r.SourcePath!, r => r.FinalPath!, StringComparer.OrdinalIgnoreCase); + + // Distinct, suffixed names — no overwrite — with content preserved per source file. + Assert.Equal(Path.Join(outputDir, "No Token Book-01.mp3"), mapped[part1]); + Assert.Equal(Path.Join(outputDir, "No Token Book-02.mp3"), mapped[part2]); + Assert.Equal("one", await File.ReadAllTextAsync(mapped[part1])); + Assert.Equal("two", await File.ReadAllTextAsync(mapped[part2])); + } + [Fact] public async Task ImportDownloadFilesAsync_SameNumberOfResult_ThanNumberOfFiles() { From 23a29a5de18e7b46bcbe6e36fca2f6100e90293d Mon Sep 17 00:00:00 2001 From: s3ntin3l8 <58235613+s3ntin3l8@users.noreply.github.com> Date: Tue, 9 Jun 2026 23:07:32 +0000 Subject: [PATCH 09/12] fix: align BuildDirectory with BuildPath when no folder pattern (#571) Review item 7: with an empty FolderNamingPattern, BuildDirectory rendered the whole FileNamingPattern as a directory (over-nesting the filename segment as a folder), while BuildPath's legacy branch treats it as a full relative path. Derive the directory portion in BuildDirectory's fallback so the two agree: BuildDirectory(ctx) == Path.GetDirectoryName(BuildPath(ctx).RelativePath) Only LibraryAddService uses BuildDirectory, and only the empty-pattern edge case changes. Adds tests for the consistency invariant plus the review's flagged gaps: BuildPath legacy fallback, multi-file + {DiskNumber} token, From(Authors=null), and From(AudibleBookMetadata) mapping. Full suite green (719 tests; Category=FileNamingService = 36). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Common/FileNamingService.cs | 19 ++-- .../FileNamingService_OrchestratorTests.cs | 97 +++++++++++++++++++ 2 files changed, 110 insertions(+), 6 deletions(-) diff --git a/listenarr.application/Common/FileNamingService.cs b/listenarr.application/Common/FileNamingService.cs index 760605793..b173aa79e 100644 --- a/listenarr.application/Common/FileNamingService.cs +++ b/listenarr.application/Common/FileNamingService.cs @@ -81,13 +81,20 @@ public async Task GenerateFilePathAsync( public string BuildDirectory(NamingContext context, ApplicationSettings settings) { - // Folder-only computation (the audiobook's BasePath). Fall back to the file pattern when no - // folder pattern is configured, then rely on ApplyNamingPattern for empty-token cleanup. - var folderPattern = string.IsNullOrWhiteSpace(settings.FolderNamingPattern) - ? settings.FileNamingPattern - : settings.FolderNamingPattern; + // Folder-only computation (the audiobook's BasePath). var variables = BuildVariables(context); - return ApplyNamingPattern(folderPattern ?? string.Empty, variables, treatAsFilename: false); + + if (!string.IsNullOrWhiteSpace(settings.FolderNamingPattern)) + return ApplyNamingPattern(settings.FolderNamingPattern, variables, treatAsFilename: false); + + // No folder pattern: FileNamingPattern is the full relative path (folder structure + filename) + // — the same legacy rule BuildPath uses — so the directory is everything except the final + // (file) segment. This keeps BuildDirectory consistent with BuildPath's legacy branch. + var legacyPattern = string.IsNullOrWhiteSpace(settings.FileNamingPattern) + ? "{Author}/{Series}/{Title}" + : settings.FileNamingPattern; + var full = ApplyNamingPattern(legacyPattern, variables, treatAsFilename: false); + return Path.GetDirectoryName(full) ?? string.Empty; } public NamingResult BuildPath(NamingContext context, ApplicationSettings settings, NamingOptions options) diff --git a/tests/Features/Api/Services/FileNamingService_OrchestratorTests.cs b/tests/Features/Api/Services/FileNamingService_OrchestratorTests.cs index 6f8f4c573..06091305a 100644 --- a/tests/Features/Api/Services/FileNamingService_OrchestratorTests.cs +++ b/tests/Features/Api/Services/FileNamingService_OrchestratorTests.cs @@ -132,5 +132,102 @@ public void BuildPath_CustomBasePath_AppliesFileOnly() // The folder pattern is skipped under a custom base path — only the file name is produced. Assert.Equal("Book.m4b", result.RelativePath); } + + [Fact] + public void BuildDirectory_AgreesWithBuildPathDirectory_AcrossPatternConfigs() + { + // Review item 7: BuildDirectory (folder) must agree with BuildPath's directory portion, + // including when only a (multi-segment) FileNamingPattern is set. + var ctx = NamingContext.From(new Audiobook + { + Title = "The Gunslinger", + Authors = new() { "Stephen King" }, + Series = "The Dark Tower", + SeriesNumber = "1", + }); + + var configs = new[] + { + new ApplicationSettings { FolderNamingPattern = "{Author}/{Series}/{Title}", FileNamingPattern = "{Title}" }, + new ApplicationSettings { FolderNamingPattern = "", FileNamingPattern = "{Author}/{Series}/{Title}" }, + }; + + foreach (var settings in configs) + { + var dir = _service.BuildDirectory(ctx, settings); + var path = _service.BuildPath(ctx, settings, new NamingOptions { OutputRoot = string.Empty, Extension = ".m4b" }); + Assert.Equal(Path.GetDirectoryName(path.RelativePath) ?? string.Empty, dir); + } + } + + [Fact] + public void BuildPath_EmptyFolderPattern_UsesFilePatternAsFullRelativePath() + { + var ctx = NamingContext.From(new Audiobook { Title = "The Gunslinger", Authors = new() { "Stephen King" }, Series = "The Dark Tower" }); + var settings = new ApplicationSettings { FolderNamingPattern = "", FileNamingPattern = "{Author}/{Series}/{Title}" }; + + var result = _service.BuildPath(ctx, settings, new NamingOptions { OutputRoot = string.Empty, Extension = ".m4b" }); + + Assert.Equal(Path.Join("Stephen King", "The Dark Tower", "The Gunslinger") + ".m4b", result.RelativePath); + } + + [Fact] + public void BuildPath_EmptyFolderAndFilePattern_FallsBackToAuthorSeriesTitle() + { + var ctx = NamingContext.From(new Audiobook { Title = "The Gunslinger", Authors = new() { "Stephen King" }, Series = "The Dark Tower" }); + var settings = new ApplicationSettings { FolderNamingPattern = "", FileNamingPattern = "" }; + + var result = _service.BuildPath(ctx, settings, new NamingOptions { OutputRoot = string.Empty, Extension = ".m4b" }); + + Assert.Equal(Path.Join("Stephen King", "The Dark Tower", "The Gunslinger") + ".m4b", result.RelativePath); + } + + [Fact] + public void BuildPath_MultiFile_WithDiskNumberToken_RendersNumberWithoutExtraSuffix() + { + var ctx = NamingContext.From(new Audiobook { Title = "Book", Authors = new() { "Author" } }) with { DiskNumber = 3 }; + var settings = new ApplicationSettings + { + OutputPath = "/audiobooks", + FolderNamingPattern = "{Author}", + FileNamingPattern = "{Title}", + MultiFileNamingPattern = "{Title}-{DiskNumber:00}", + }; + + var result = _service.BuildPath(ctx, settings, new NamingOptions + { + OutputRoot = "/audiobooks", + IsMultiFile = true, + SequenceNumber = 3, + Extension = ".mp3", + }); + + Assert.EndsWith("Book-03.mp3", result.RelativePath); + Assert.DoesNotContain("Book-03-03", result.RelativePath); // token renders; no extra suffix appended + } + + [Fact] + public void From_AudiobookWithNullAuthors_DoesNotThrow_AndUsesUnknownAuthor() + { + var settings = new ApplicationSettings { FolderNamingPattern = "{Author}/{Title}" }; + var ctx = NamingContext.From(new Audiobook { Title = "Orphan", Authors = null }); + + Assert.Equal(Path.Join("Unknown Author", "Orphan"), _service.BuildDirectory(ctx, settings)); + } + + [Fact] + public void From_AudibleBookMetadata_MapsAuthorSeriesTitle() + { + // LibraryAdd source: the provider metadata (via ToAudiobook) drives the name. + var settings = new ApplicationSettings { FolderNamingPattern = "{Author}/{Series}/{Title}" }; + var ctx = NamingContext.From(new AudibleBookMetadata + { + Title = "The Gunslinger", + Authors = new() { "Stephen King" }, + Series = "The Dark Tower", + }); + + Assert.Equal(Path.Join("Stephen King", "The Dark Tower", "The Gunslinger"), _service.BuildDirectory(ctx, settings)); + } } } From e5f535c5952f6c8aa0cb0518548faebde061676b Mon Sep 17 00:00:00 2001 From: s3ntin3l8 <58235613+s3ntin3l8@users.noreply.github.com> Date: Tue, 9 Jun 2026 23:33:25 +0000 Subject: [PATCH 10/12] chore: drop CHANGELOG entries; align no-pattern fallback (#571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove the two #571 CHANGELOG entries — changelog entries are no longer wanted on upstream PRs (maintainer confirmed on #659/#660); they only cause [Unreleased] conflicts. Keeps the existing Authentication entries. - Unify LibraryController.ComputeAudiobookBaseDirectoryFromPattern's no-pattern fallback from {Author}/{Title} to {Author}/{Series}/{Title}, matching the orchestrator's legacy default (review follow-up). Reachable only when no folder pattern is configured at all; empty {Series} collapses away, so non-series books are unaffected. 719 backend tests green. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 -- listenarr.api/Controllers/LibraryController.cs | 9 +++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 013b9b8aa..982b7c6df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed -- **Naming patterns are applied exactly as configured — no implicit `{Series}` folder or subtitle.** The Add-book destination preview and bulk root-folder change (`LibraryController.ComputeAudiobookBaseDirectoryFromPattern`) no longer inject a `{Series}` directory level when the configured folder pattern omits `{Series}` — previously the injected path was persisted as the audiobook's `BasePath`, so imported files really did land in an unconfigured series folder. Likewise, rename/organize previews (`RenameService`) and manual imports (`ManualImportController`) no longer rewrite `{Title}` to `"Title: Subtitle"` when the pattern lacks `{Subtitle}`. Add the `{Series}`/`{Subtitle}` tokens explicitly to include them; the default folder pattern already contains `{Series}`, so default configurations are unaffected. ([#571](https://github.com/Listenarrs/Listenarr/issues/571)) -- **Flat folder patterns are respected in the destination preview.** A deliberately slash-less folder pattern (e.g. `{Title}`) is no longer silently overridden with `{Author}/{Title}` when computing an audiobook's base directory — matching how the import pipeline (`FileNamingService`) already applies such patterns. The `{Author}/{Title}` default still applies when no pattern is configured at all. ([#571](https://github.com/Listenarrs/Listenarr/issues/571)) - **Authentication settings: startup-config save no longer offers a downloadable `config.json` fallback when the backend refuses the save as invalid.** `SettingsView.saveSettings()` previously wrapped `apiService.saveStartupConfig` in a bare `catch {}` and treated every failure as a disk-persistence problem — offering the user a downloadable `config.json` containing the *server-rejected* values so they could save it manually. That bypasses the new backend admin-existence guard entirely: a user who tries to enable the login screen with no admin user gets the backend's 400, the FE catches it, and the FE offers a download of the same `AuthenticationRequired=true` config the server just refused. The catch now inspects the thrown error's `status`: 4xx responses are validation refusals and surface as a hard error toast (no download offered); 5xx and network failures fall through to the existing download fallback, which is the right escape hatch for "server wants to save but can't write to disk." - **Authentication settings: enabling the login screen now refuses to persist when no admin user exists.** `ConfigurationService.SaveStartupConfigAsync` queries `IUserService.GetAdminUsersAsync` whenever the incoming save *transitions* `AuthenticationRequired` from disabled to enabled, and throws if the admin user list is empty. This closes the carveout left by the credential-visibility and admin-provisioning fixes below: the settings DTO clears blank fields before save, so a user who flips "Enable login screen" with empty (or username-only) admin credentials silently skipped provisioning entirely and still reached the startup-config write, locking themselves out of an admin-less instance (recoverable by editing `config/config.json` back to `"AuthenticationRequired": "false"`, but a confusing first-time-setup trap). The check is scoped to the transition: subsequent saves while auth is already on (API key regenerations, port changes, log-level tweaks) don't re-query the admin list, and the common "just updating other startup fields with auth off" path stays unaffected. The admin block in `SaveApplicationSettings` runs before the startup-config write in the same save flow, so the typical "supply credentials and enable login in the same save" sequence has the admin row in place by the time the check runs. - **Authentication settings: admin provisioning failures no longer silently let the auth-required toggle proceed.** `ConfigurationService.SaveApplicationSettingsAsync` previously caught any exception from `CreateUserAsync` / `UpdatePasswordAsync`, logged it, and returned successfully — so when admin credentials were supplied but the user-service rejected them (password policy violation, repo I/O error, concurrent-write race), `SettingsView.saveSettings()` would still go on to persist `AuthenticationRequired=true` on its second request. The result was an instance that required login but had no working admin account — exactly the lockout shape the credential-visibility fix below was meant to prevent. The catch now re-throws the failure so the caller aborts before the auth-toggle write. The settings row itself is still saved before the admin block (non-admin changes like notification triggers and webhooks shouldn't disappear because admin provisioning failed), and the no-credentials path remains an unchanged silent skip. diff --git a/listenarr.api/Controllers/LibraryController.cs b/listenarr.api/Controllers/LibraryController.cs index 2712e7bce..43c095815 100644 --- a/listenarr.api/Controllers/LibraryController.cs +++ b/listenarr.api/Controllers/LibraryController.cs @@ -3481,16 +3481,17 @@ private string ComputeAudiobookBaseDirectoryFromPattern(Audiobook audiobook, str directoryPattern = Regex.Replace(directoryPattern, @"[\\/]\s*$", ""); // If the pattern is now empty, use a fallback; deliberately flat (slash-less) - // patterns are applied exactly as configured + // patterns are applied exactly as configured. Matches the orchestrator's legacy default + // (FileNamingService.BuildPath/BuildDirectory); empty {Series} is collapsed away. if (string.IsNullOrWhiteSpace(directoryPattern)) { - directoryPattern = "{Author}/{Title}"; + directoryPattern = "{Author}/{Series}/{Title}"; } } else { - // Fallback to default directory pattern - directoryPattern = "{Author}/{Title}"; + // Fallback to default directory pattern (aligned with the orchestrator's legacy default). + directoryPattern = "{Author}/{Series}/{Title}"; } // An empty {Series} (no series metadata) is handled by ApplyNamingPattern below, which From d3d2afb3458e26217881b75b8a806d55e671b816 Mon Sep 17 00:00:00 2001 From: s3ntin3l8 <58235613+s3ntin3l8@users.noreply.github.com> Date: Wed, 10 Jun 2026 00:12:23 +0000 Subject: [PATCH 11/12] test: cover no-pattern fallback and empty-folder-pattern BasePath (#571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups: the {Author}/{Series}/{Title} no-pattern fallback in ComputeAudiobookBaseDirectoryFromPattern (unified in the previous commit) had no test — every existing case passed an explicit pattern. Also adds a service-level test for LibraryAddService deriving BasePath from the file naming pattern's directory portion when no folder pattern is configured (BuildDirectory's legacy branch, previously only covered at unit level). Co-Authored-By: Claude Fable 5 --- .../LibraryController_AddToLibraryTests.cs | 33 +++++++++++++++++++ .../LibraryController_BasePathTests.cs | 26 +++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/tests/Features/Api/Controllers/LibraryController_AddToLibraryTests.cs b/tests/Features/Api/Controllers/LibraryController_AddToLibraryTests.cs index b21b3bd48..73e90e83b 100644 --- a/tests/Features/Api/Controllers/LibraryController_AddToLibraryTests.cs +++ b/tests/Features/Api/Controllers/LibraryController_AddToLibraryTests.cs @@ -201,6 +201,39 @@ public async Task AddToLibrary_WithoutAsin_UsesDerivedKey_AndMovesImageToLibrary imageCacheServiceMock.Verify(m => m.MoveToLibraryStorageAsync(It.IsAny(), imageUrl2), Times.Once); } + [Fact] + public async Task AddToLibrary_EmptyFolderPattern_DerivesBasePathFromFileNamingPattern() + { + // Given no folder pattern: the file naming pattern acts as the full relative path + // (legacy rule), so the BasePath is its directory portion — BuildDirectory's fallback. + await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() + .WithFolderNamingPattern("") + .WithFileNamingPattern("{Author}/{Series}/{Title}") + .Build()); + + var controller = _provider.GetRequiredService(); + + var request = new LibraryController.AddToLibraryRequest + { + Metadata = new AudibleBookMetadata + { + Title = "The Gunslinger", + Authors = new List { "Stephen King" }, + Series = "The Dark Tower" + }, + Monitored = true + }; + + // Act + var actionResult = await controller.AddToLibrary(request); + + // Assert + Assert.IsType(actionResult); + + var stored = (await _audiobookRepository.GetAllAsync()).First(); + Assert.Equal(Path.Join(tempRoot, "Stephen King", "The Dark Tower"), stored.BasePath); + } + [Fact] public async Task AddToLibrary_WithCustomPath_StoresCustomPathAsBasePath() { diff --git a/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs b/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs index a01929dce..ec15125a6 100644 --- a/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs +++ b/tests/Features/Api/Controllers/LibraryController_BasePathTests.cs @@ -151,6 +151,32 @@ public void ComputeAudiobookBaseDirectoryFromPattern_SlashlessPattern_AppliedExa Assert.DoesNotContain("Stephen Graham Jones", result); } + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [Trait("Method", "ComputeAudiobookBaseDirectoryFromPattern")] + [Trait("Scenario", "NoPatternConfigured_FallsBackToAuthorSeriesTitle")] + public void ComputeAudiobookBaseDirectoryFromPattern_NoPatternConfigured_FallsBackToAuthorSeriesTitle(string? pattern) + { + // Given a series book and no configured pattern at all + var audiobook = new AudiobookBuilder() + .WithTitle("The Gunslinger") + .WithAuthor("Stephen King") + .WithSeries("The Dark Tower") + .WithSeriesNumber("1") + .Build(); + + var controller = _provider.GetRequiredService(); + + // When + var result = (string)ComputeBaseDirectoryMethod.Invoke(controller, new object?[] { audiobook, RootPath, pattern })!; + + // Then the legacy default {Author}/{Series}/{Title} applies — aligned with the + // orchestrator's no-pattern fallback (FileNamingService.BuildPath/BuildDirectory) + Assert.Equal(Path.Join(RootPath, "Stephen King", "The Dark Tower", "The Gunslinger"), result); + } + [Fact] [Trait("Method", "ComputeAudiobookBaseDirectoryFromPattern")] [Trait("Scenario", "NonSeriesBook_KeepsSeriesNumberToken")] From fd6301dc92eee7e1c39d1350c5311187e22628ff Mon Sep 17 00:00:00 2001 From: s3ntin3l8 <58235613+s3ntin3l8@users.noreply.github.com> Date: Wed, 10 Jun 2026 00:12:50 +0000 Subject: [PATCH 12/12] fix: keep UNC roots intact in Windows path-limit handling; make it testable (#571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pull the MAX_PATH truncation logic behind a testable seam (EnforceWindowsPathLimits) that uses explicit Windows path semantics — separators and drive/UNC root parsing — instead of the platform-dependent System.IO.Path helpers. The previously self-skipping path-length tests now run on any OS (including the ubuntu-only CI), and UNC (NAS share) inputs are covered. Writing the UNC coverage exposed a real bug: on Windows, Path.GetPathRoot(@"\\server\share\dir") returns the root without a trailing separator, and the function always returns the rebuilt root + join(parts) string — so every UNC path it touched came back as \\server\sharedir\..., even when under the limit. This was dormant while EnsurePathWithinLimits had no production callers, but routing manual import, rename, and download import through BuildPath made it reachable; fixed by including the separator after the share in the parsed root. Co-Authored-By: Claude Fable 5 --- .../Common/FileNamingService.cs | 68 +++++++++- .../FileNamingService_PathLengthTests.cs | 121 ++++++++++++------ 2 files changed, 142 insertions(+), 47 deletions(-) diff --git a/listenarr.application/Common/FileNamingService.cs b/listenarr.application/Common/FileNamingService.cs index b173aa79e..9d3cef597 100644 --- a/listenarr.application/Common/FileNamingService.cs +++ b/listenarr.application/Common/FileNamingService.cs @@ -435,14 +435,30 @@ public string EnsurePathWithinLimits(string fullPath) if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) return fullPath; + return EnforceWindowsPathLimits(fullPath); + } + + /// + /// Windows MAX_PATH enforcement, implemented with explicit Windows path semantics + /// (separators, drive and UNC roots) rather than the platform-dependent System.IO.Path + /// root helpers, so the behavior is identical — and testable — on any OS. + /// + internal string EnforceWindowsPathLimits(string fullPath) + { + if (string.IsNullOrWhiteSpace(fullPath)) + return fullPath; + var originalPath = fullPath; - // Split into root (e.g. "D:\") and component parts - var root = Path.GetPathRoot(fullPath) ?? string.Empty; + // Split into root (e.g. "D:\" or "\\server\share\") and component parts. + // The root — including a UNC server/share — is never truncated. + var root = GetWindowsPathRoot(fullPath); var withoutRoot = fullPath.Substring(root.Length); - var parts = withoutRoot.Split(new[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }, StringSplitOptions.RemoveEmptyEntries) + var parts = withoutRoot.Split(WindowsSeparators, StringSplitOptions.RemoveEmptyEntries) .ToList(); + string Rebuild() => root + string.Join('\\', parts); + if (parts.Count == 0) return fullPath; @@ -466,7 +482,7 @@ public string EnsurePathWithinLimits(string fullPath) const int maxIterations = 50; // safety valve for (int iter = 0; iter < maxIterations; iter++) { - var currentPath = root + string.Join(Path.DirectorySeparatorChar.ToString(), parts); + var currentPath = Rebuild(); if (currentPath.Length <= WindowsMaxPath) break; @@ -506,7 +522,7 @@ public string EnsurePathWithinLimits(string fullPath) : nameWithoutExt.Substring(0, newLen).TrimEnd(); } - var result = root + string.Join(Path.DirectorySeparatorChar.ToString(), parts); + var result = Rebuild(); if (result != originalPath) { @@ -517,6 +533,48 @@ public string EnsurePathWithinLimits(string fullPath) return result; } + private static readonly char[] WindowsSeparators = { '\\', '/' }; + + /// + /// Windows-semantics equivalent of for the path shapes + /// this service produces: drive paths ("C:\", drive-relative "C:"), UNC shares + /// ("\\server\share\"), and rooted paths ("\" or "/"). Unlike Path.GetPathRoot on Windows, + /// a UNC root includes the separator after the share, so root + components rebuilds a valid + /// path. (\\?\ device syntax is not produced by this code base and gets no special handling.) + /// + internal static string GetWindowsPathRoot(string path) + { + static bool IsSep(char c) => c == '\\' || c == '/'; + + if (string.IsNullOrEmpty(path)) + return string.Empty; + + if (path.Length >= 2 && IsSep(path[0]) && IsSep(path[1])) + { + // UNC: \\server\share[\...] — the root runs through the share name and the + // separator that follows it. + var separatorsSeen = 0; + for (var i = 2; i < path.Length; i++) + { + if (!IsSep(path[i])) + continue; + separatorsSeen++; + if (separatorsSeen == 2) + return path.Substring(0, i + 1); + } + // "\\server" or "\\server\share" with nothing after it — the whole path is root. + return path; + } + + if (path.Length >= 2 && char.IsAsciiLetter(path[0]) && path[1] == ':') + { + // Drive path "C:\..." (or drive-relative "C:foo", whose root is just "C:"). + return path.Length >= 3 && IsSep(path[2]) ? path.Substring(0, 3) : path.Substring(0, 2); + } + + return IsSep(path[0]) ? path.Substring(0, 1) : string.Empty; + } + private static string CombineWithOptionalBase(string? basePath, string candidatePath) { var normalizedPath = candidatePath.Trim(); diff --git a/tests/Features/Api/Services/FileNamingService_PathLengthTests.cs b/tests/Features/Api/Services/FileNamingService_PathLengthTests.cs index 6e8e6d635..29469551e 100644 --- a/tests/Features/Api/Services/FileNamingService_PathLengthTests.cs +++ b/tests/Features/Api/Services/FileNamingService_PathLengthTests.cs @@ -25,7 +25,10 @@ namespace Listenarr.Tests.Features.Api.Services { /// - /// Tests for FileNamingService Windows path length enforcement (MAX_PATH / per-component limits) + /// Tests for FileNamingService Windows path length enforcement (MAX_PATH / per-component limits). + /// The limit logic lives behind a testable seam (EnforceWindowsPathLimits) that uses explicit + /// Windows path semantics, so these tests exercise the real Windows behavior on any OS — including + /// the Linux CI runners. Only the public wrapper's platform gate is OS-dependent. /// [Trait("Category", "FileNamingService")] public class FileNamingService_PathLengthTests @@ -39,24 +42,38 @@ public FileNamingService_PathLengthTests() _service = new FileNamingService(mockConfig.Object, mockLogger.Object); } + // ---------- Public wrapper: platform gate ---------- + [Fact] public void EnsurePathWithinLimits_ShortPath_ReturnsUnchanged() { + // Under the limit on Windows; not enforced at all elsewhere — unchanged either way. var path = @"D:\Audiobooks\Author\Title\Book.m4b"; - var result = _service.EnsurePathWithinLimits(path); + Assert.Equal(path, _service.EnsurePathWithinLimits(path)); + } + [Fact] + public void EnsurePathWithinLimits_NonWindows_DoesNotTruncate() + { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - Assert.Equal(path, result); - } + return; // The gate itself is what's under test here + + var path = $"/audiobooks/{new string('A', 300)}/{new string('T', 300)}.m4b"; + Assert.Equal(path, _service.EnsurePathWithinLimits(path)); } [Fact] - public void EnsurePathWithinLimits_PathExceeding260Chars_IsTruncated() + public void EnsurePathWithinLimits_EmptyOrNull_ReturnsAsIs() { - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - return; // Limit only enforced on Windows + Assert.Equal("", _service.EnsurePathWithinLimits("")); + Assert.Null(_service.EnsurePathWithinLimits(null!)); + } + // ---------- Seam: Windows limit semantics, runnable on any OS ---------- + + [Fact] + public void EnforceWindowsPathLimits_PathExceeding260Chars_IsTruncated() + { // Build a path well over 260 characters var longAuthor = new string('A', 100); var longTitle = new string('T', 200); @@ -64,44 +81,35 @@ public void EnsurePathWithinLimits_PathExceeding260Chars_IsTruncated() Assert.True(path.Length > 259, $"Test path should exceed 259 chars, was {path.Length}"); - var result = _service.EnsurePathWithinLimits(path); + var result = _service.EnforceWindowsPathLimits(path); Assert.True(result.Length <= 259, $"Result path should be ≤ 259 chars, was {result.Length}"); Assert.EndsWith(".m4b", result); } [Fact] - public void EnsurePathWithinLimits_PreservesExtension() + public void EnforceWindowsPathLimits_PreservesExtension() { - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - return; - var longTitle = new string('T', 300); var path = $@"D:\Audiobooks\Author\{longTitle}.mp3"; - var result = _service.EnsurePathWithinLimits(path); + var result = _service.EnforceWindowsPathLimits(path); Assert.True(result.Length <= 259); Assert.EndsWith(".mp3", result); } [Fact] - public void EnsurePathWithinLimits_ComponentExceeding255Chars_IsTruncated() + public void EnforceWindowsPathLimits_ComponentExceeding255Chars_IsTruncated() { - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - return; - - // Single component over 255 chars but total path under 260 - // Not realistic on Windows (260 total means components can't be that long with a root) - // but test the per-component logic directly + // Single component over 255 chars; the per-component limit applies independently var longFolder = new string('F', 256); var path = $@"D:\{longFolder}\Book.m4b"; - var result = _service.EnsurePathWithinLimits(path); + var result = _service.EnforceWindowsPathLimits(path); - // Each component should be ≤ 255 - var parts = result.Substring(Path.GetPathRoot(result)!.Length) - .Split(Path.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries); + var parts = result.Substring(FileNamingService.GetWindowsPathRoot(result).Length) + .Split(new[] { '\\', '/' }, StringSplitOptions.RemoveEmptyEntries); foreach (var part in parts) { Assert.True(part.Length <= 255, $"Component '{part.Substring(0, Math.Min(30, part.Length))}...' is {part.Length} chars, exceeds 255"); @@ -109,18 +117,15 @@ public void EnsurePathWithinLimits_ComponentExceeding255Chars_IsTruncated() } [Fact] - public void EnsurePathWithinLimits_TruncatesLongestComponentFirst() + public void EnforceWindowsPathLimits_TruncatesLongestComponentFirst() { - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - return; - // Create a path where the title folder is much longer than the author var shortAuthor = "Author"; var longTitle = new string('T', 200); var filename = "Book.m4b"; var path = $@"D:\Audiobooks\{shortAuthor}\{longTitle}\{filename}"; - var result = _service.EnsurePathWithinLimits(path); + var result = _service.EnforceWindowsPathLimits(path); Assert.True(result.Length <= 259); // Author should be preserved since it's short; the long title should be truncated @@ -129,18 +134,8 @@ public void EnsurePathWithinLimits_TruncatesLongestComponentFirst() } [Fact] - public void EnsurePathWithinLimits_EmptyOrNull_ReturnsAsIs() - { - Assert.Equal("", _service.EnsurePathWithinLimits("")); - Assert.Null(_service.EnsurePathWithinLimits(null!)); - } - - [Fact] - public void EnsurePathWithinLimits_ExactlyAtLimit_ReturnsUnchanged() + public void EnforceWindowsPathLimits_ExactlyAtLimit_ReturnsUnchanged() { - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - return; - // Build a path that's exactly 259 chars var root = @"D:\"; var remaining = 259 - root.Length - ".m4b".Length - 1; // -1 for separator before filename @@ -151,8 +146,50 @@ public void EnsurePathWithinLimits_ExactlyAtLimit_ReturnsUnchanged() // Verify our test setup Assert.Equal(259, path.Length); - var result = _service.EnsurePathWithinLimits(path); + var result = _service.EnforceWindowsPathLimits(path); Assert.Equal(path, result); } + + // ---------- UNC (NAS shares) ---------- + + [Fact] + public void EnforceWindowsPathLimits_UncPathUnderLimit_ReturnsUnchanged() + { + // Regression: the rebuild used Path.GetPathRoot, whose UNC root has no trailing + // separator, producing "\\nas\audiobooksAuthor\..." even for in-limit paths. + var path = @"\\nas\audiobooks\Author\Series\Title\Book.m4b"; + Assert.Equal(path, _service.EnforceWindowsPathLimits(path)); + } + + [Fact] + public void EnforceWindowsPathLimits_UncPathOverLimit_PreservesShareRootAndExtension() + { + var longTitle = new string('T', 300); + var path = $@"\\nas\audiobooks\Author\{longTitle}\Book.m4b"; + + var result = _service.EnforceWindowsPathLimits(path); + + // The server/share root is never truncated; only components after it are. + Assert.StartsWith(@"\\nas\audiobooks\Author\", result); + Assert.True(result.Length <= 259, $"Result path should be ≤ 259 chars, was {result.Length}"); + Assert.EndsWith(".m4b", result); + } + + // ---------- Root parsing ---------- + + [Theory] + [InlineData(@"C:\Books\file.m4b", @"C:\")] + [InlineData("C:/Books/file.m4b", "C:/")] + [InlineData("C:relative", "C:")] + [InlineData(@"\\server\share\dir\file.m4b", @"\\server\share\")] + [InlineData(@"\\server\share", @"\\server\share")] + [InlineData(@"\dir\file.m4b", @"\")] + [InlineData("/dir/file.m4b", "/")] + [InlineData(@"relative\dir\file.m4b", "")] + [InlineData("", "")] + public void GetWindowsPathRoot_ParsesWindowsRootShapes(string path, string expectedRoot) + { + Assert.Equal(expectedRoot, FileNamingService.GetWindowsPathRoot(path)); + } } }