From 5622d472f3fd79cdd48061327d1d94a88f56d527 Mon Sep 17 00:00:00 2001 From: Kevin Heneveld <1192102+kevinheneveld@users.noreply.github.com> Date: Mon, 8 Jun 2026 16:40:31 -0800 Subject: [PATCH] fix(scan): normalize paths when deduplicating audiobook file rows ExistsAtPathAsync matched stored file rows by raw exact string, so a row left by an older scan under a bare/relative path (e.g. "Elantris.mp3") never matched the absolute path a current scan resolves for the same file. The scan then inserted a duplicate absolute-path row; the bare row resolved relative to the process working directory and 404'd on playback. Keep the exact match as a fast path and, only on a miss, resolve the book's rows to a canonical absolute form (anchoring relative paths on the audiobook BasePath) before comparing. A book has few file rows, so the fallback is cheap and never runs for the common all-absolute case. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 1 + .../Repositories/EfAudiobookFileRepository.cs | 42 +++++++- ...diobookFileRepository_ExistsAtPathTests.cs | 100 ++++++++++++++++++ 3 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 tests/Features/Api/Repositories/EfAudiobookFileRepository_ExistsAtPathTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 982b7c6df..fc0a182c1 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 +- **Library scan no longer creates duplicate file rows for a book that has a legacy relative-path entry.** `EfAudiobookFileRepository.ExistsAtPathAsync` deduplicated by raw exact-string match (`f.Path == path`), so a row stored by an older scan under a bare/relative path (e.g. `Elantris.mp3`) never matched the absolute path a current scan finds for the same physical file — and the scan inserted a second, absolute-path row. The result was two rows for one file: the absolute one streamed fine while the bare one resolved relative to the process working directory, failed the stream endpoint's root-folder guard, and 404'd on playback. The lookup now keeps the exact match as a fast path and, only when it misses, resolves the book's existing rows to a canonical absolute form (anchoring relative paths on the audiobook's `BasePath` via `FileUtils.CombineWithOptionalBase` + `NormalizeStoredPath`) before comparing, so the same file is recognised regardless of stored path shape. A book has only a handful of file rows, so the fallback stays cheap and never runs for the common all-absolute case. - **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.infrastructure/Persistence/Repositories/EfAudiobookFileRepository.cs b/listenarr.infrastructure/Persistence/Repositories/EfAudiobookFileRepository.cs index eb018e471..7d47c89dc 100644 --- a/listenarr.infrastructure/Persistence/Repositories/EfAudiobookFileRepository.cs +++ b/listenarr.infrastructure/Persistence/Repositories/EfAudiobookFileRepository.cs @@ -17,6 +17,7 @@ */ using Listenarr.Application.Interfaces.Repositories; using Listenarr.Application.Audiobooks; +using Listenarr.Domain.Common; using Listenarr.Domain.Models; using Microsoft.EntityFrameworkCore; @@ -86,14 +87,53 @@ public async Task DeleteAsync(int id, CancellationToken ct = default) public async Task ExistsAtPathAsync(int audiobookId, string path, CancellationToken ct = default) { - return await _db.AudiobookFiles.AnyAsync(f => f.AudiobookId == audiobookId && f.Path == path, ct); + // Fast path: exact string match. Scans and imports store canonical absolute paths, so the + // incoming path normally string-matches the stored one — an index-friendly lookup that + // covers the overwhelming majority of calls. + if (await _db.AudiobookFiles.AnyAsync(f => f.AudiobookId == audiobookId && f.Path == path, ct)) + { + return true; + } + + // Robust path: a stored row may represent the *same* file under a different path shape — + // most commonly a legacy bare/relative path (e.g. "Elantris.mp3") left by an older scan, + // which never string-matches today's absolute path. Without this, a rescan re-registers + // the file and spawns a duplicate row. Resolve this book's rows to a canonical absolute + // form (anchoring relative paths on its BasePath) and compare. A book has only a handful + // of file rows, so this stays cheap and only runs when the exact match misses. + var storedPaths = await _db.AudiobookFiles + .Where(f => f.AudiobookId == audiobookId && f.Path != null) + .Select(f => f.Path!) + .ToListAsync(ct); + + if (storedPaths.Count == 0) + { + return false; + } + + var basePath = await _db.Audiobooks + .Where(a => a.Id == audiobookId) + .Select(a => a.BasePath) + .FirstOrDefaultAsync(ct); + + var target = CanonicalizePath(path, basePath); + return storedPaths.Any(stored => + string.Equals(CanonicalizePath(stored, basePath), target, StringComparison.OrdinalIgnoreCase)); } public async Task IsPathUsedByOtherAsync(int audiobookId, string path, CancellationToken ct = default) { + // Cross-book guard: exact match is sufficient here because stored paths are canonical + // absolute by convention, and a file physically lives under exactly one book's folder + // (resolving every other book's relative paths would require each book's BasePath). return await _db.AudiobookFiles.AnyAsync(f => f.AudiobookId != audiobookId && f.Path == path, ct); } + // Resolve a stored or incoming path to a canonical absolute form so the same file is + // recognised regardless of shape (legacy relative vs. absolute, separator/`..` differences). + private static string CanonicalizePath(string path, string? basePath) + => FileUtils.NormalizeStoredPath(FileUtils.CombineWithOptionalBase(basePath, path)); + public async Task> GetAllFilePathsAsync(CancellationToken ct = default) { return await _db.AudiobookFiles diff --git a/tests/Features/Api/Repositories/EfAudiobookFileRepository_ExistsAtPathTests.cs b/tests/Features/Api/Repositories/EfAudiobookFileRepository_ExistsAtPathTests.cs new file mode 100644 index 000000000..8149a38a1 --- /dev/null +++ b/tests/Features/Api/Repositories/EfAudiobookFileRepository_ExistsAtPathTests.cs @@ -0,0 +1,100 @@ +/* + * Listenarr - Audiobook Management System + * Copyright (C) 2024-2026 Listenarr Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +using Microsoft.EntityFrameworkCore; +using Xunit; +using Listenarr.Domain.Models; +using Listenarr.Infrastructure.Persistence; +using Listenarr.Infrastructure.Persistence.Repositories; + +namespace Listenarr.Tests.Features.Api.Repositories +{ + public class EfAudiobookFileRepository_ExistsAtPathTests + { + private static ListenArrDbContext NewDb() => + new(new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options); + + [Fact] + public async Task ExistsAtPath_ExactMatch_ReturnsTrue() + { + using var db = NewDb(); + var book = new Audiobook { Title = "Elantris", BasePath = "/audiobooks/Brandon Sanderson/Elantris/Jack Garrett" }; + db.Audiobooks.Add(book); + await db.SaveChangesAsync(); + db.AudiobookFiles.Add(new AudiobookFile + { + AudiobookId = book.Id, + Path = "/audiobooks/Brandon Sanderson/Elantris/Jack Garrett/Elantris.mp3", + }); + await db.SaveChangesAsync(); + + var repo = new EfAudiobookFileRepository(db); + + Assert.True(await repo.ExistsAtPathAsync(book.Id, + "/audiobooks/Brandon Sanderson/Elantris/Jack Garrett/Elantris.mp3")); + } + + [Fact] + public async Task ExistsAtPath_StoredBareRelative_MatchesIncomingAbsolute() + { + // The regression: an older scan stored a bare filename ("Elantris.mp3"). A later scan finds + // the file at its absolute path and must recognise it as already-registered — not duplicate it. + using var db = NewDb(); + var book = new Audiobook { Title = "Elantris", BasePath = "/audiobooks/Brandon Sanderson/Elantris/Jack Garrett" }; + db.Audiobooks.Add(book); + await db.SaveChangesAsync(); + db.AudiobookFiles.Add(new AudiobookFile { AudiobookId = book.Id, Path = "Elantris.mp3" }); + await db.SaveChangesAsync(); + + var repo = new EfAudiobookFileRepository(db); + + Assert.True(await repo.ExistsAtPathAsync(book.Id, + "/audiobooks/Brandon Sanderson/Elantris/Jack Garrett/Elantris.mp3")); + } + + [Fact] + public async Task ExistsAtPath_DifferentFile_ReturnsFalse() + { + using var db = NewDb(); + var book = new Audiobook { Title = "Elantris", BasePath = "/audiobooks/Brandon Sanderson/Elantris/Jack Garrett" }; + db.Audiobooks.Add(book); + await db.SaveChangesAsync(); + db.AudiobookFiles.Add(new AudiobookFile { AudiobookId = book.Id, Path = "Elantris.mp3" }); + await db.SaveChangesAsync(); + + var repo = new EfAudiobookFileRepository(db); + + Assert.False(await repo.ExistsAtPathAsync(book.Id, + "/audiobooks/Brandon Sanderson/Elantris/Jack Garrett/Hope-of-Elantris.mp3")); + } + + [Fact] + public async Task ExistsAtPath_NoFileRows_ReturnsFalse() + { + using var db = NewDb(); + var book = new Audiobook { Title = "Empty", BasePath = "/audiobooks/x" }; + db.Audiobooks.Add(book); + await db.SaveChangesAsync(); + + var repo = new EfAudiobookFileRepository(db); + + Assert.False(await repo.ExistsAtPathAsync(book.Id, "/audiobooks/x/anything.mp3")); + } + } +}