-
-
Notifications
You must be signed in to change notification settings - Fork 23
refactor(indexers): extract test logic into search providers #490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: canary
Are you sure you want to change the base?
Changes from 3 commits
c2b1c92
f0d8744
de85e96
4d432ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| /* | ||
| * Listenarr - Audiobook Management System | ||
| * Copyright (C) 2024-2025 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 System.Text.Json; | ||
| using Listenarr.Api.Services; | ||
| using Listenarr.Domain.Models; | ||
|
|
||
| namespace Listenarr.Api.Services.Search.Providers | ||
| { | ||
| /// <summary> | ||
| /// Base class for indexer providers, providing shared constants, helpers, | ||
| /// and exception handling for connection tests. | ||
| /// </summary> | ||
| public abstract class IndexerProviderBase : IIndexerSearchProvider | ||
| { | ||
| protected readonly ILogger _logger; | ||
|
|
||
| protected IndexerProviderBase(ILogger logger) | ||
| { | ||
| _logger = logger ?? throw new ArgumentNullException(nameof(logger)); | ||
| } | ||
|
|
||
| public abstract string IndexerType { get; } | ||
|
|
||
| /// <summary>Listenarr user-agent for standard indexer requests.</summary> | ||
| protected static string ListenarrUserAgent { get; } = | ||
| $"Listenarr/{typeof(IndexerProviderBase).Assembly.GetName().Version?.ToString() ?? "0.0.0"} (+https://github.com/listenarrs/listenarr)"; | ||
|
|
||
| /// <summary>Browser-like user-agent for indexers that require it (e.g. MyAnonamouse).</summary> | ||
| protected const string BrowserUserAgent = | ||
| "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36"; | ||
|
|
||
| /// <inheritdoc /> | ||
| public abstract Task<List<IndexerSearchResult>> SearchAsync( | ||
| Indexer indexer, | ||
| string query, | ||
| string? category = null, | ||
| Listenarr.Api.Models.SearchRequest? request = null); | ||
|
|
||
| /// <summary> | ||
| /// Implement the actual test logic. Exceptions are caught by <see cref="TestAsync"/>. | ||
| /// </summary> | ||
| protected abstract Task<IndexerTestResult> ExecuteTestAsync(Indexer indexer); | ||
|
|
||
| /// <summary> | ||
| /// Wraps <see cref="ExecuteTestAsync"/> in shared exception handling | ||
| /// so individual providers only implement the happy path. | ||
| /// </summary> | ||
| public async Task<IndexerTestResult> TestAsync(Indexer indexer) | ||
| { | ||
| try | ||
| { | ||
| return await ExecuteTestAsync(indexer); | ||
| } | ||
| catch (Exception ex) when (ex is HttpRequestException | ||
| or TaskCanceledException | ||
| or JsonException | ||
| or UriFormatException | ||
| or System.Net.CookieException | ||
| or InvalidOperationException) | ||
| { | ||
| return LogAndFail(indexer, ex); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Parse a string value from the indexer's AdditionalSettings JSON. | ||
| /// </summary> | ||
| protected string? GetAdditionalSetting(Indexer indexer, string key) | ||
|
lzinga marked this conversation as resolved.
Outdated
|
||
| { | ||
| if (string.IsNullOrEmpty(indexer.AdditionalSettings)) | ||
| return null; | ||
|
|
||
| try | ||
| { | ||
| using var doc = JsonDocument.Parse(indexer.AdditionalSettings); | ||
| if (doc.RootElement.TryGetProperty(key, out var prop)) | ||
| return prop.GetString(); | ||
| } | ||
| catch (JsonException ex) | ||
| { | ||
| _logger.LogWarning(ex, "Failed to parse AdditionalSettings for indexer '{Name}'", | ||
| LogRedaction.SanitizeText(indexer.Name)); | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| private IndexerTestResult LogAndFail(Indexer indexer, Exception ex) | ||
| { | ||
| _logger.LogWarning(ex, "{IndexerType} indexer '{Name}' test failed", | ||
| IndexerType, LogRedaction.SanitizeText(indexer.Name)); | ||
| return IndexerTestResult.Failed($"{IndexerType} test failed", ex.Message); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| /* | ||
| * Listenarr - Audiobook Management System | ||
| * Copyright (C) 2024-2025 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/>. | ||
| */ | ||
|
|
||
| namespace Listenarr.Api.Services.Search.Providers | ||
| { | ||
| /// <summary> | ||
| /// Represents the outcome of an indexer connection test, including | ||
| /// whether the test succeeded and any diagnostic messages. | ||
| /// </summary> | ||
| public record IndexerTestResult | ||
| { | ||
| /// <summary>Whether the connection test passed.</summary> | ||
| public bool Success { get; init; } | ||
|
|
||
| /// <summary>Human-readable summary shown to the user (e.g. "Indexer authentication successful").</summary> | ||
| public string? Message { get; init; } | ||
|
|
||
| /// <summary>Error detail when <see cref="Success"/> is <c>false</c>; <c>null</c> on success.</summary> | ||
| public string? Error { get; init; } | ||
|
|
||
| /// <summary>Create a successful test result.</summary> | ||
| public static IndexerTestResult Succeeded(string message) => | ||
| new() { Success = true, Message = message }; | ||
|
|
||
| /// <summary>Create a failed test result.</summary> | ||
| public static IndexerTestResult Failed(string message, string error) => | ||
| new() { Success = false, Message = message, Error = error }; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| /* | ||
| * Listenarr - Audiobook Management System | ||
| * Copyright (C) 2024-2025 Robbie Davis | ||
| * Copyright (C) 2024-2025 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 | ||
|
|
@@ -23,25 +23,68 @@ | |
| namespace Listenarr.Api.Services.Search.Providers; | ||
|
|
||
| /// <summary> | ||
| /// Search provider for Internet Archive (archive.org) | ||
| /// Searches public domain audiobooks from collections like LibriVox | ||
| /// Search provider for Internet Archive (archive.org). | ||
| /// Searches public domain audiobooks from collections like LibriVox. | ||
| /// </summary> | ||
| public class InternetArchiveSearchProvider : IIndexerSearchProvider | ||
| public class InternetArchiveSearchProvider : IndexerProviderBase | ||
| { | ||
| private readonly HttpClient _httpClient; | ||
| private readonly ILogger<InternetArchiveSearchProvider> _logger; | ||
|
|
||
| public string IndexerType => "InternetArchive"; | ||
| private const string BaseUrl = "https://archive.org"; | ||
| private const string DefaultCollection = "librivoxaudio"; | ||
| private const string AdvancedSearchPath = "/advancedsearch.php"; | ||
|
|
||
| public override string IndexerType => "InternetArchive"; | ||
|
|
||
| public InternetArchiveSearchProvider( | ||
| HttpClient httpClient, | ||
| ILogger<InternetArchiveSearchProvider> logger) | ||
| ILogger<InternetArchiveSearchProvider> logger, | ||
| HttpClient httpClient) | ||
| : base(logger) | ||
| { | ||
| _httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient)); | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| protected override async Task<IndexerTestResult> ExecuteTestAsync(Indexer indexer) | ||
| { | ||
| _httpClient = httpClient; | ||
| _logger = logger; | ||
| var collection = GetAdditionalSetting(indexer, "collection") ?? DefaultCollection; | ||
|
|
||
| var testUrl = $"{BaseUrl}{AdvancedSearchPath}?q=collection:{Uri.EscapeDataString(collection)}&rows=1&output=json"; | ||
|
|
||
| _logger.LogInformation("Testing Internet Archive indexer '{Name}' with collection '{Collection}'", | ||
| LogRedaction.SanitizeText(indexer.Name), LogRedaction.SanitizeText(collection)); | ||
|
|
||
| var uri = new Uri(testUrl); | ||
| var (response, _) = await OutboundRequestSecurity.SendWithValidatedRedirectsAsync( | ||
| currentUri => new HttpRequestMessage(HttpMethod.Get, currentUri), | ||
| uri, | ||
| _httpClient, | ||
| _logger, | ||
| allowPrivateTargets: true); | ||
|
|
||
| using (response) | ||
| { | ||
| response.EnsureSuccessStatusCode(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the IA provider has a logic inconsistency from the other two providers. This works, but the other providers return a IndexerTestResult.Failed() explicitly while this uses exceptions as a control flow. Can we refactor this to be more in line with the other providers for consistency? |
||
|
|
||
| var content = await response.Content.ReadAsStringAsync(); | ||
| using var jsonDoc = JsonDocument.Parse(content); | ||
|
|
||
| if (!jsonDoc.RootElement.TryGetProperty("response", out var responseProperty)) | ||
| throw new InvalidOperationException("Invalid response format: missing 'response' property"); | ||
|
|
||
| if (!responseProperty.TryGetProperty("docs", out _)) | ||
| throw new InvalidOperationException("Invalid response format: missing 'docs' property"); | ||
| } | ||
|
|
||
| _logger.LogInformation("Internet Archive indexer '{Name}' test succeeded for collection '{Collection}'", | ||
| LogRedaction.SanitizeText(indexer.Name), LogRedaction.SanitizeText(collection)); | ||
|
|
||
| return IndexerTestResult.Succeeded( | ||
| $"Internet Archive connection successful for collection '{collection}'"); | ||
| } | ||
|
|
||
| public async Task<List<IndexerSearchResult>> SearchAsync( | ||
| /// <inheritdoc /> | ||
| public override async Task<List<IndexerSearchResult>> SearchAsync( | ||
| Indexer indexer, | ||
| string query, | ||
| string? category = null, | ||
|
|
@@ -75,7 +118,7 @@ public async Task<List<IndexerSearchResult>> SearchAsync( | |
|
|
||
| // Build search query - search in title and creator (author) fields | ||
| var searchQuery = $"collection:{collection} AND (title:({query}) OR creator:({query}))"; | ||
| var searchUrl = $"https://archive.org/advancedsearch.php?q={Uri.EscapeDataString(searchQuery)}&fl=identifier,title,creator,date,downloads,item_size,description&rows=100&output=json"; | ||
| var searchUrl = $"{BaseUrl}{AdvancedSearchPath}?q={Uri.EscapeDataString(searchQuery)}&fl=identifier,title,creator,date,downloads,item_size,description&rows=100&output=json"; | ||
|
|
||
| _logger.LogInformation("Internet Archive search URL: {Url}", searchUrl); | ||
|
|
||
|
|
@@ -152,7 +195,7 @@ private async Task<List<IndexerSearchResult>> ParseInternetArchiveSearchResponse | |
| _logger.LogDebug("Fetching metadata for {Identifier}", identifier); | ||
|
|
||
| // Fetch detailed metadata to get file information | ||
| var metadataUrl = $"https://archive.org/metadata/{identifier}"; | ||
| var metadataUrl = $"{BaseUrl}/metadata/{identifier}"; | ||
| var metadataResponse = await _httpClient.GetAsync(metadataUrl); | ||
|
|
||
| if (!metadataResponse.IsSuccessStatusCode) | ||
|
|
@@ -171,7 +214,7 @@ private async Task<List<IndexerSearchResult>> ParseInternetArchiveSearchResponse | |
| } | ||
|
|
||
| // Build download URL | ||
| var downloadUrl = $"https://archive.org/download/{identifier}/{audioFile.FileName}"; | ||
| var downloadUrl = $"{BaseUrl}/download/{identifier}/{audioFile.FileName}"; | ||
|
|
||
| _logger.LogDebug("Found audio file for {Title}: {FileName} ({Format}, {Size} bytes)", | ||
| title, audioFile.FileName, audioFile.Format, audioFile.Size); | ||
|
|
@@ -188,7 +231,7 @@ private async Task<List<IndexerSearchResult>> ParseInternetArchiveSearchResponse | |
| Leechers = 0, // N/A for direct downloads | ||
| TorrentUrl = downloadUrl, // Using TorrentUrl field for direct download URL | ||
| // Internet Archive item page | ||
| ResultUrl = !string.IsNullOrEmpty(identifier) ? $"https://archive.org/details/{identifier}" : null, | ||
| ResultUrl = !string.IsNullOrEmpty(identifier) ? $"{BaseUrl}/details/{identifier}" : null, | ||
| DownloadType = "DDL", // Direct Download Link | ||
| Format = audioFile.Format, | ||
| Quality = DetectQualityFromFormat(audioFile.Format), | ||
|
|
@@ -201,7 +244,7 @@ private async Task<List<IndexerSearchResult>> ParseInternetArchiveSearchResponse | |
| // Ensure ResultUrl is present (fallback to item page or archive details) | ||
| if (string.IsNullOrEmpty(iaResult.ResultUrl) && !string.IsNullOrEmpty(identifier)) | ||
| { | ||
| iaResult.ResultUrl = $"https://archive.org/details/{identifier}"; | ||
| iaResult.ResultUrl = $"{BaseUrl}/details/{identifier}"; | ||
| } | ||
|
|
||
| try | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is so broad and could throw errors for unrelated bugs. What about propagate those to other handlers and use a dedicated exception type for provider validation failures?