Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]

### Fixed
- **Author/series metadata cache was silently disabled on SQLite, forcing an Audible re-fetch on every author and series page load.** The four catalog-cache read paths in `AudiobookRepository` (`GetCachedAuthorByNameAsync`, `GetCachedAuthorByAsinAsync`, `GetCachedSeriesByNameAsync`, `GetCachedSeriesByAsinAsync`) ordered results by `entry.LastFetchedAt.GetValueOrDefault(entry.UpdatedAt)`. The SQLite EF Core provider cannot translate `Nullable<DateTime>.GetValueOrDefault`, so the query threw `InvalidOperationException` at execution time — and because each caller wraps the cache read in a best-effort `try/catch` that treats any failure as a cache miss, the exception was swallowed and the persisted cache never returned a hit. Every author and series page therefore re-fetched from Audible on each load. Switched the ordering to null-coalescing (`entry.LastFetchedAt ?? entry.UpdatedAt`), which translates to SQL `COALESCE`. Added regression coverage that exercises these reads against the real SQLite provider (the EF InMemory provider tolerates the untranslatable LINQ and so could not catch this).
- **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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,11 @@ public async Task<int> DeleteBulkAsync(List<int> ids)

return await _db.AuthorCacheEntries
.AsNoTracking()
.OrderByDescending(entry => entry.LastFetchedAt.GetValueOrDefault(entry.UpdatedAt))
// Order by recency with null-coalescing (translates to SQL COALESCE).
// Nullable.GetValueOrDefault is not translatable by the SQLite EF provider
// and throws at runtime — which the caller's best-effort catch swallows,
// silently disabling this cache and re-fetching from Audible on every load.
.OrderByDescending(entry => entry.LastFetchedAt ?? entry.UpdatedAt)
.FirstOrDefaultAsync(entry =>
entry.AuthorNameNormalized == normalizedName &&
entry.Region == normalizedRegion);
Expand All @@ -243,7 +247,11 @@ public async Task<int> DeleteBulkAsync(List<int> ids)

return await _db.AuthorCacheEntries
.AsNoTracking()
.OrderByDescending(entry => entry.LastFetchedAt.GetValueOrDefault(entry.UpdatedAt))
// Order by recency with null-coalescing (translates to SQL COALESCE).
// Nullable.GetValueOrDefault is not translatable by the SQLite EF provider
// and throws at runtime — which the caller's best-effort catch swallows,
// silently disabling this cache and re-fetching from Audible on every load.
.OrderByDescending(entry => entry.LastFetchedAt ?? entry.UpdatedAt)
.FirstOrDefaultAsync(entry =>
entry.AuthorAsin != null &&
entry.AuthorAsin.ToUpper() == normalizedAsin &&
Expand Down Expand Up @@ -328,7 +336,11 @@ public async Task<AuthorCacheEntry> UpsertCachedAuthorAsync(AuthorCacheEntry aut

return await _db.SeriesCacheEntries
.AsNoTracking()
.OrderByDescending(entry => entry.LastFetchedAt.GetValueOrDefault(entry.UpdatedAt))
// Order by recency with null-coalescing (translates to SQL COALESCE).
// Nullable.GetValueOrDefault is not translatable by the SQLite EF provider
// and throws at runtime — which the caller's best-effort catch swallows,
// silently disabling this cache and re-fetching from Audible on every load.
.OrderByDescending(entry => entry.LastFetchedAt ?? entry.UpdatedAt)
.FirstOrDefaultAsync(entry =>
entry.SeriesNameNormalized == normalizedName &&
entry.Region == normalizedRegion);
Expand All @@ -346,7 +358,11 @@ public async Task<AuthorCacheEntry> UpsertCachedAuthorAsync(AuthorCacheEntry aut

return await _db.SeriesCacheEntries
.AsNoTracking()
.OrderByDescending(entry => entry.LastFetchedAt.GetValueOrDefault(entry.UpdatedAt))
// Order by recency with null-coalescing (translates to SQL COALESCE).
// Nullable.GetValueOrDefault is not translatable by the SQLite EF provider
// and throws at runtime — which the caller's best-effort catch swallows,
// silently disabling this cache and re-fetching from Audible on every load.
.OrderByDescending(entry => entry.LastFetchedAt ?? entry.UpdatedAt)
.FirstOrDefaultAsync(entry =>
entry.SeriesAsin != null &&
entry.SeriesAsin.ToUpper() == normalizedAsin &&
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/*
* 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 <https://www.gnu.org/licenses/>.
*/
using Microsoft.Data.Sqlite;
using Microsoft.EntityFrameworkCore;
using Xunit;
using Listenarr.Domain.Models;
using Listenarr.Infrastructure.Persistence;
using Listenarr.Infrastructure.Persistence.Repositories;

namespace Listenarr.Tests.Features.Infrastructure.Repositories
{
// Regression for the author/series catalog cache read paths.
//
// These deliberately use the real SQLite provider (not the EF InMemory provider) so that
// query translation is actually exercised. The cache reads order by recency; using
// Nullable.GetValueOrDefault there is not translatable by the SQLite provider and throws
// at runtime, which the cache callers swallow as a best-effort miss — silently disabling
// the cache so every author/series page re-fetched from Audible. The InMemory provider
// tolerates that LINQ, so it cannot catch this class of bug.
public class AudiobookRepository_CatalogCacheReadTests
{
private sealed class TestDb : IDisposable
{
private readonly SqliteConnection _connection;
public ListenArrDbContext Db { get; }

public TestDb()
{
_connection = new SqliteConnection("DataSource=:memory:");
_connection.Open();
Db = new ListenArrDbContext(
new DbContextOptionsBuilder<ListenArrDbContext>().UseSqlite(_connection).Options);
Db.Database.EnsureCreated();
}

public void Dispose()
{
Db.Dispose();
_connection.Dispose();
}
}

[Fact]
public async Task GetCachedAuthorByName_ReadsBackUnderSqlite()
{
using var ctx = new TestDb();
var repository = new AudiobookRepository(ctx.Db);

await repository.UpsertCachedAuthorAsync(new AuthorCacheEntry
{
AuthorName = "Brandon Sanderson",
AuthorAsin = "B001IGFHW6",
Region = "us"
});

var byName = await repository.GetCachedAuthorByNameAsync("Brandon Sanderson", "us");
var byAsin = await repository.GetCachedAuthorByAsinAsync("B001IGFHW6", "us");

Assert.NotNull(byName);
Assert.Equal("B001IGFHW6", byName!.AuthorAsin);
Assert.NotNull(byAsin);
Assert.Equal("Brandon Sanderson", byAsin!.AuthorName);
}

[Fact]
public async Task GetCachedSeriesByName_ReadsBackUnderSqlite()
{
using var ctx = new TestDb();
var repository = new AudiobookRepository(ctx.Db);

await repository.UpsertCachedSeriesAsync(new SeriesCacheEntry
{
SeriesName = "The Stormlight Archive",
SeriesAsin = "B00INWST01",
Region = "us"
});

var byName = await repository.GetCachedSeriesByNameAsync("The Stormlight Archive", "us");
var byAsin = await repository.GetCachedSeriesByAsinAsync("B00INWST01", "us");

Assert.NotNull(byName);
Assert.Equal("The Stormlight Archive", byName!.SeriesName);
Assert.NotNull(byAsin);
Assert.Equal("The Stormlight Archive", byAsin!.SeriesName);
}

[Fact]
public async Task GetCachedSeriesByAsin_OrdersByRecency_WhenLastFetchedAtIsNull()
{
using var ctx = new TestDb();
var db = ctx.Db;
var repository = new AudiobookRepository(db);

// Two distinct series rows can legitimately share one ASIN (e.g. two name slugs
// resolved to the same Audible series), so the by-ASIN read orders by recency to
// return the freshest. Insert directly to control LastFetchedAt/UpdatedAt — the
// (SeriesNameNormalized, Region) uniqueness only allows this on differing names.
db.SeriesCacheEntries.Add(new SeriesCacheEntry
{
SeriesNameNormalized = "mistborn",
SeriesName = "Mistborn",
SeriesAsin = "B00SHARED1",
Region = "us",
LastFetchedAt = DateTime.UtcNow.AddDays(-2),
CreatedAt = DateTime.UtcNow.AddDays(-2),
UpdatedAt = DateTime.UtcNow.AddDays(-2)
});
// Newer row with no LastFetchedAt: recency must coalesce to UpdatedAt (the branch
// GetValueOrDefault could not translate), and the query must not throw.
db.SeriesCacheEntries.Add(new SeriesCacheEntry
{
SeriesNameNormalized = "the final empire",
SeriesName = "The Final Empire (newer)",
SeriesAsin = "B00SHARED1",
Region = "us",
LastFetchedAt = null,
CreatedAt = DateTime.UtcNow,
UpdatedAt = DateTime.UtcNow.AddDays(1)
});
await db.SaveChangesAsync();

var resolved = await repository.GetCachedSeriesByAsinAsync("B00SHARED1", "us");

Assert.NotNull(resolved);
Assert.Equal("The Final Empire (newer)", resolved!.SeriesName);
}
}
}
Loading