diff --git a/listenarr.application/Audiobooks/Quality/QualityProfileService.cs b/listenarr.application/Audiobooks/Quality/QualityProfileService.cs index 3097a5060..eaab906c0 100644 --- a/listenarr.application/Audiobooks/Quality/QualityProfileService.cs +++ b/listenarr.application/Audiobooks/Quality/QualityProfileService.cs @@ -190,9 +190,15 @@ private async Task UnsetAllDefaultsAsync() } } - public async Task ScoreSearchResult(SearchResult searchResult, QualityProfile profile) + public Task ScoreSearchResult(SearchResult searchResult, QualityProfile profile) => + ScoreSearchResult(searchResult, profile, resolvedIndexers: null); + + private async Task ScoreSearchResult( + SearchResult searchResult, + QualityProfile profile, + IReadOnlyDictionary? resolvedIndexers) { - var scorer = new SearchResultScorer(_indexerRepository, _logger); + var scorer = new SearchResultScorer(_indexerRepository, _logger, resolvedIndexers); var score = await scorer.Score(searchResult, profile); // Also calculate the Prowlarr-style composite (Smart) score so the UI @@ -287,7 +293,15 @@ private int GetQualityScore(string? quality) public async Task> ScoreSearchResults(List searchResults, QualityProfile profile) { - var scores = await Task.WhenAll(searchResults.Select(result => ScoreSearchResult(result, profile))); + // Resolve every indexer this batch refers to before fanning out, not inside it. + // Scoring runs in parallel and the scorer reads indexer retention per result, so a + // per-result lookup meant N concurrent queries against one scoped DbContext. EF + // rejects the overlap, the scorer catches it and logs at Debug, and the result keeps + // a retention of 0 with Usenet detection skipped. The scores come out quietly wrong + // rather than the request failing. + var resolvedIndexers = await ResolveIndexersAsync(searchResults); + var scores = await Task.WhenAll( + searchResults.Select(result => ScoreSearchResult(result, profile, resolvedIndexers))); // Ensure rejected results are ordered last regardless of numeric TotalScore return scores @@ -296,6 +310,43 @@ public async Task> ScoreSearchResults(List sear .ToList(); } + private async Task> ResolveIndexersAsync( + List searchResults) + { + var resolved = new Dictionary(); + if (_indexerRepository == null) + { + return resolved; + } + + // Sequential and de-duplicated: a batch usually refers to a handful of indexers even + // when it carries hundreds of results, so this is fewer queries than before as well as + // non-overlapping ones. + foreach (var indexerId in searchResults + .Where(result => result.IndexerId.HasValue) + .Select(result => result.IndexerId!.Value) + .Distinct()) + { + try + { + var indexer = await _indexerRepository.GetByIdAsync(indexerId); + if (indexer != null) + { + resolved[indexerId] = indexer; + } + } + catch (Exception ex) when (ex is not OperationCanceledException && ex is not OutOfMemoryException && ex is not StackOverflowException) + { + _logger.LogWarning( + ex, + "Failed to resolve indexer {IndexerId} while scoring a search batch; retention and Usenet detection will be skipped for its results", + indexerId); + } + } + + return resolved; + } + /// /// Checks if a quality string contains VBR preset indicators (v0, v1, v2). /// diff --git a/listenarr.application/Search/Scoring/SearchResultScorer.cs b/listenarr.application/Search/Scoring/SearchResultScorer.cs index bfb446143..7dc4a95bd 100644 --- a/listenarr.application/Search/Scoring/SearchResultScorer.cs +++ b/listenarr.application/Search/Scoring/SearchResultScorer.cs @@ -34,10 +34,24 @@ public class SearchResultScorer public int QualityNotAllowedPenalty { get; set; } = -20; public int ForbiddenWordRejectionFlag { get; set; } = -1; // sentinel for rejection + private readonly IReadOnlyDictionary? _resolvedIndexers; + public SearchResultScorer(IIndexerRepository? indexerRepository, ILogger logger) + : this(indexerRepository, logger, resolvedIndexers: null) + { + } + + // resolvedIndexers lets a caller scoring a whole batch resolve each indexer once up front + // and pass the results in. The repository is scoped, and so is the DbContext behind it, so + // results scored in parallel must not each run their own lookup. + public SearchResultScorer( + IIndexerRepository? indexerRepository, + ILogger logger, + IReadOnlyDictionary? resolvedIndexers) { _indexerRepository = indexerRepository; _logger = logger; + _resolvedIndexers = resolvedIndexers; } public async Task Score(SearchResult searchResult, QualityProfile profile) @@ -118,11 +132,16 @@ public async Task Score(SearchResult searchResult, QualityProfile // Age checks and indexer retention double ageDays = 0; int indexerRetention = 0; - if (searchResult.IndexerId.HasValue && _indexerRepository != null) + if (searchResult.IndexerId.HasValue + && (_resolvedIndexers != null || _indexerRepository != null)) { try { - var idx = await _indexerRepository.GetByIdAsync(searchResult.IndexerId.Value); + var idx = _resolvedIndexers != null + ? (_resolvedIndexers.TryGetValue(searchResult.IndexerId.Value, out var preresolved) + ? preresolved + : null) + : await _indexerRepository!.GetByIdAsync(searchResult.IndexerId.Value); if (idx != null) { indexerRetention = idx.Retention; diff --git a/tests/Features/Application/Audiobooks/Quality/QualityProfileScoringConcurrencyTests.cs b/tests/Features/Application/Audiobooks/Quality/QualityProfileScoringConcurrencyTests.cs new file mode 100644 index 000000000..e91d25264 --- /dev/null +++ b/tests/Features/Application/Audiobooks/Quality/QualityProfileScoringConcurrencyTests.cs @@ -0,0 +1,160 @@ +/* + * 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 Listenarr.Tests.Common; +using Microsoft.Extensions.Logging.Abstractions; + +namespace Listenarr.Tests.Features.Application.Audiobooks.Quality +{ + [Trait("Name", "QualityProfileScoringConcurrencyTests")] + [Trait("Category", "Unit")] + public class QualityProfileScoringConcurrencyTests : BaseTests + { + // IIndexerRepository is registered scoped and the ListenArrDbContext behind it is scoped + // too, so every repository in a scope shares one context. EF rejects a second operation + // started on a context while another is in flight. Asserting on the real exception would + // mean racing it, so this counts overlap directly: the stub records the highest number of + // calls it ever had in flight at once. Anything above one is the condition EF refuses. + private sealed class OverlapRecordingIndexerRepository(int retention = 1500) : IIndexerRepository + { + private int _inFlight; + public int MaxConcurrent { get; private set; } + public int CallCount { get; private set; } + + public async Task GetByIdAsync(int id, CancellationToken ct = default) + { + var now = Interlocked.Increment(ref _inFlight); + lock (this) + { + CallCount++; + if (now > MaxConcurrent) MaxConcurrent = now; + } + + // A real query is not instantaneous. Without this the tasks can complete one at a + // time by luck and the overlap the test exists to catch would go unobserved. + await Task.Delay(20); + + Interlocked.Decrement(ref _inFlight); + return new Indexer { Id = id, Name = $"indexer-{id}", Type = "Usenet", Retention = retention }; + } + + public Task> GetAllAsync(CancellationToken ct = default) => + Task.FromResult(new List()); + public Task> GetEnabledAsync(bool isAutomaticSearch, CancellationToken ct = default) => + Task.FromResult(new List()); + public Task GetByNameAsync(string name, CancellationToken ct = default) => + Task.FromResult(null); + public Task AddAsync(Indexer indexer, CancellationToken ct = default) => + Task.FromResult(indexer); + public Task UpdateAsync(Indexer indexer, CancellationToken ct = default) => Task.CompletedTask; + public Task DeleteAsync(int id, CancellationToken ct = default) => Task.CompletedTask; + } + + [Fact] + public async Task ScoreSearchResults_DoesNotQueryTheIndexerRepositoryConcurrently() + { + var indexerRepository = new OverlapRecordingIndexerRepository(); + var service = new QualityProfileService( + Mock.Of(), + NullLogger.Instance, + indexerRepository); + + // Twelve results across three indexers: enough to overlap, and enough to show that the + // batch does not need one query per result. + var searchResults = Enumerable.Range(0, 12) + .Select(i => new SearchResult + { + Id = $"result-{i}", + Title = $"A Book {i}", + IndexerId = (i % 3) + 1, + Format = "mp3", + Language = "English", + PublishedDate = DateTime.UtcNow.AddDays(-1).ToString("o") + }) + .ToList(); + + var profile = new QualityProfile + { + MinimumSize = 0, + MaximumSize = 0, + PreferredFormats = ["mp3"], + PreferredWords = [], + MustNotContain = [], + MustContain = [], + PreferredLanguages = ["English"], + MinimumSeeders = 0, + MaximumAge = 3650 + }; + + var scores = await service.ScoreSearchResults(searchResults, profile); + + Assert.Equal(searchResults.Count, scores.Count); + Assert.Equal(1, indexerRepository.MaxConcurrent); + // Three distinct indexers, so three lookups rather than one per result. + Assert.Equal(3, indexerRepository.CallCount); + } + + // The overlap assertion above passes whether or not the resolved indexers reach the + // scorer: a batch that looks each indexer up once and then throws the answers away makes + // exactly the same three non-overlapping calls. This asks the other half of the question. + // Retention is 10 days and the result is 100 days old, so the indexer's own retention has + // to be the thing that rejects it. A profile MaximumAge well beyond 100 days means nothing + // else in the scorer can produce that rejection, and an empty or ignored dictionary leaves + // retention at 0 and the result accepted. + [Fact] + public async Task ScoreSearchResults_AppliesTheResolvedIndexerRetention() + { + var indexerRepository = new OverlapRecordingIndexerRepository(retention: 10); + var service = new QualityProfileService( + Mock.Of(), + NullLogger.Instance, + indexerRepository); + + var searchResults = new List + { + new() + { + Id = "aged-result", + Title = "A Book", + IndexerId = 7, + Format = "mp3", + Language = "English", + PublishedDate = DateTime.UtcNow.AddDays(-100).ToString("o") + } + }; + + var profile = new QualityProfile + { + MinimumSize = 0, + MaximumSize = 0, + PreferredFormats = ["mp3"], + PreferredWords = [], + MustNotContain = [], + MustContain = [], + PreferredLanguages = ["English"], + MinimumSeeders = 0, + MaximumAge = 3650 + }; + + var scores = await service.ScoreSearchResults(searchResults, profile); + + var score = Assert.Single(scores); + Assert.True(score.IsRejected); + Assert.Contains(score.RejectionReasons, reason => reason.Contains("indexer retention 10 days")); + } + } +}