Skip to content

[Architecture] Responsability of download client adapters #599

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.

Sign up for GitHub

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

Open
T4g1 wants to merge 5 commits into
base: canary
Choose a base branch
from
Open

[Architecture] Responsability of download client adapters #599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b6ef9bc
[enh] Harmonize queue updates on download clients
T4g1 May 17, 2026
b6cb634
[enh] Harmonized adapter queue updates
T4g1 May 17, 2026
b97809c
[enh] Allows adapters to handle multiple protocols
T4g1 May 17, 2026
4d21079
[fix] Cleaned warning on build
T4g1 May 17, 2026
0fd6574
[fix] Removed trim on perfectly valid space characters
T4g1 May 18, 2026
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
4 changes: 3 additions & 1 deletion Directory.Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
<CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
<EFVersion>10.0.7</EFVersion>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<NoWarn>$(NoWarn);1591</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand All @@ -26,7 +28,7 @@
<PackageVersion Include="Polly" Version="8.6.6" />
<PackageVersion Include="Serilog.AspNetCore" Version="10.0.0" />
<PackageVersion Include="Serilog.Sinks.File" Version="7.0.0" />
<PackageVersion Include="SharpCompress" Version="0.47.4" />
<PackageVersion Include="SharpCompress" Version="0.48.1" />
<PackageVersion Include="TagLibSharp" Version="2.3.0" />
<PackageVersion Include="SixLabors.ImageSharp" Version="3.1.12" />
<PackageVersion Include="Swashbuckle.AspNetCore" Version="10.1.7" />
Expand Down
32 changes: 3 additions & 29 deletions listenarr.api/Controllers/ManualImportController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ private async Task<string> GenerateManualImportPathAsync(Audiobook audiobook, Au

relativePath = string.IsNullOrWhiteSpace(folderRelative)
? fileRelative
: CombineWithOptionalBase(folderRelative, fileRelative);
: FileUtils.CombineWithOptionalBase(folderRelative, fileRelative);
}

if ((string.IsNullOrWhiteSpace(folderPattern) || isCustomBasePath)
Expand All @@ -591,33 +591,7 @@ private async Task<string> GenerateManualImportPathAsync(Audiobook audiobook, Au

return string.IsNullOrWhiteSpace(basePath)
? relativePath
: CombineWithOptionalBase(basePath, relativePath);
}

private static string CombineWithOptionalBase(string? basePath, string candidatePath)
{
var normalizedPath = candidatePath.Trim();

if (string.IsNullOrEmpty(normalizedPath))
{
return normalizedPath;
}

if (Path.IsPathRooted(normalizedPath) || string.IsNullOrWhiteSpace(basePath))
{
return normalizedPath;
}

var relativePath = normalizedPath.TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
if (Path.IsPathRooted(relativePath))
{
return relativePath;
}

var normalizedBasePath = basePath.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
return string.IsNullOrEmpty(normalizedBasePath)
? relativePath
: normalizedBasePath + Path.DirectorySeparatorChar + relativePath;
: FileUtils.CombineWithOptionalBase(basePath, relativePath);
}

private static List<ManualImportItemDto> BuildOrderedItems(IEnumerable<ManualImportItemDto> items)
Expand Down Expand Up @@ -751,7 +725,7 @@ private async Task<int> ImportCompanionFilesAsync(
continue;
}

var destinationPath = CombineWithOptionalBase(destinationRoot, relativePath);
var destinationPath = FileUtils.CombineWithOptionalBase(destinationRoot, relativePath);

var success = await _fileMover.PerformActionOn(request.Action, companionFile, destinationPath);
if (success)
Expand Down
12 changes: 2 additions & 10 deletions listenarr.application/Audiobooks/RenameService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,14 +424,14 @@ private string BuildExpectedPath(Audiobook audiobook, PreviewFileEntry file, App
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);
relativePath = string.IsNullOrWhiteSpace(folderRelative) ? fileRelative : FileUtils.CombineWithOptionalBase(folderRelative, fileRelative);
}

if ((string.IsNullOrWhiteSpace(folderPattern) || isCustomBasePath) && isMultiFile && !patternHasNumberTokens)
relativePath = FileUtils.AppendSequenceSuffix(relativePath, file.SequenceNumber);
if (!relativePath.EndsWith(file.Extension, StringComparison.OrdinalIgnoreCase)) relativePath += file.Extension;

return string.IsNullOrWhiteSpace(basePath) ? NormalizePath(relativePath) : NormalizePath(CombineWithOptionalBase(basePath, relativePath));
return string.IsNullOrWhiteSpace(basePath) ? NormalizePath(relativePath) : NormalizePath(FileUtils.CombineWithOptionalBase(basePath, relativePath));
}

private static Dictionary<string, object> BuildNamingVariables(Audiobook audiobook, string? folderPattern, string? filePattern, int sequenceNumber, bool isMultiFile)
Expand Down Expand Up @@ -527,14 +527,6 @@ private static string ComputeCommonBasePath(IEnumerable<string> paths)
return string.IsNullOrWhiteSpace(common) ? string.Empty : NormalizePath(common);
}

private static string CombineWithOptionalBase(string basePath, string relativePath)
{
var safeRelative = relativePath ?? string.Empty;
if (string.IsNullOrWhiteSpace(basePath)) return safeRelative;
if (Path.IsPathRooted(safeRelative)) return safeRelative;
return Path.Join(basePath, safeRelative);
}

private static string CombineRelativePath(string basePath, string relativePath)
{
var safeRelative = relativePath ?? string.Empty;
Expand Down
31 changes: 3 additions & 28 deletions listenarr.application/Common/FileNamingService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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 Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -137,7 +138,7 @@ public async Task<string> GenerateFilePathAsync(

relativePath = string.IsNullOrWhiteSpace(folderRelative)
? fileRelative
: CombineWithOptionalBase(folderRelative, fileRelative);
: FileUtils.CombineWithOptionalBase(folderRelative, fileRelative);
}

// Ensure it has the correct extension
Expand All @@ -149,7 +150,7 @@ public async Task<string> GenerateFilePathAsync(
// Combine with the provided output path
var fullPath = string.IsNullOrWhiteSpace(outputPath)
? relativePath
: CombineWithOptionalBase(outputPath, relativePath);
: FileUtils.CombineWithOptionalBase(outputPath, relativePath);

fullPath = EnsurePathWithinLimits(fullPath);

Expand Down Expand Up @@ -559,32 +560,6 @@ public string EnsurePathWithinLimits(string fullPath)

return result;
}

private static string CombineWithOptionalBase(string? basePath, string candidatePath)
{
var normalizedPath = candidatePath.Trim();

if (string.IsNullOrEmpty(normalizedPath))
{
return normalizedPath;
}

if (Path.IsPathRooted(normalizedPath) || string.IsNullOrWhiteSpace(basePath))
{
return normalizedPath;
}

var relativePath = normalizedPath.TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
if (Path.IsPathRooted(relativePath))
{
return relativePath;
}

var normalizedBasePath = basePath.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
return string.IsNullOrEmpty(normalizedBasePath)
? relativePath
: normalizedBasePath + Path.DirectorySeparatorChar + relativePath;
}
}
}

91 changes: 53 additions & 38 deletions listenarr.application/Downloads/DownloadClientGateway.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
using Listenarr.Application.Interfaces;
using Listenarr.Application.Interfaces.Repositories;
using Listenarr.Application.Mapping;
using Listenarr.Application.Security;
using Listenarr.Domain.Common;
using Listenarr.Domain.Models;
Expand All @@ -32,31 +34,21 @@ namespace Listenarr.Application.Downloads
public class DownloadClientGateway(
IRemotePathMappingService remotePathMappingService,
IDownloadClientAdapterFactory factory,
IDownloadRepository downloadRepository,
ILogger<DownloadClientGateway> logger) : IDownloadClientGateway
{
internal IDownloadClientAdapter ResolveAdapter(DownloadClientConfiguration client)
{
if (client == null)
{
throw new ArgumentNullException(nameof(client));
}
ArgumentNullException.ThrowIfNull(client);

var attemptedKeys = new List<string?> { client.Id, client.Type };
foreach (var key in attemptedKeys)
if (!string.IsNullOrWhiteSpace(client.Type))
{
if (string.IsNullOrWhiteSpace(key))
{
continue;
}

try
{
return factory.GetByIdOrType(key);
return factory.GetByType(client.Type);
}
catch (InvalidOperationException)
{
// Try the next key.
continue;
}
}

Expand Down Expand Up @@ -90,21 +82,13 @@ public Task<bool> RemoveAsync(DownloadClientConfiguration client, string id, boo

public async Task<List<QueueItem>> GetQueueAsync(DownloadClientConfiguration client, CancellationToken ct = default)
{
var adapter = ResolveAdapter(client);
var results = await adapter.GetQueueAsync(client, ct);

List<QueueItem> translatedResults = [];
foreach (QueueItem result in results)
{
translatedResults.Add(await TranslateQueueItemPathsAsync(client, result));
}
return translatedResults;
}
var downloads = await downloadRepository.GetByClientAsync(client.Id);
var ids = GetExternalIds(downloads);

public Task<List<(string Id, string Name)>> GetRecentHistoryAsync(DownloadClientConfiguration client, int limit = 100, CancellationToken ct = default)
{
var adapter = ResolveAdapter(client);
return adapter.GetRecentHistoryAsync(client, limit, ct);
var items = await adapter.GetQueueAsync(client, ids, ct);
Copy link
Copy Markdown
Collaborator

@therobbiedavis therobbiedavis May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this changes the queue endpoint fetching the client queue to fetching only downloads already tracked in our DB. That drops untracked queue items and completed external downloads before DownloadQueueService can reconcile or display them.

Copy link
Copy Markdown
Contributor Author

@T4g1 T4g1 May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reasoning is the following: If all adapters have to return an external ID when we add a download: It means that every Listenarr Download has an external ID. So, any Download without an external ID should be dropped and any queue item without external ID should be ignored.

By fetching the whole queue and deciding what to use or not in the application, we gather informations that are not related to Listenarr and should probably never leave the adapter layer (by design, it solves some issues I witnessed where all queue items are shown for an adapter which is not expected)

var tasks = items.Select(item => TranslateQueueItemPathsAsync(client, item));
return [.. await Task.WhenAll(tasks)];
}

public async Task<bool> MarkItemAsImportedAsync(DownloadClientConfiguration client, Download download, CancellationToken ct = default)
Expand All @@ -131,6 +115,48 @@ public async Task<QueueItem> GetQueueItemAsync(
return await TranslateQueueItemPathsAsync(client, item);
}

public async Task<List<Download>> FetchDownloadsAsync(DownloadClientConfiguration client, List<Download> downloads, CancellationToken ct = default)
{
var ids = GetExternalIds(downloads);
Copy link
Copy Markdown
Collaborator

@therobbiedavis therobbiedavis May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path now only polls downloads that already have a stored external ID. If a legacy (or hopefully not a new record) is missing ClientDownloadId/TorrentHash, the adapter gets an empty ID list and the download can stay stuck without progress updates. Probably a reasonable tradeoff though, but maybe we should include a view for users to see "orphaned" records? wdyt

Copy link
Copy Markdown
Contributor Author

@T4g1 T4g1 May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe other *arr silently remove the download in case it becomes orphaned (no external ID or external ID no longer found in the download client), this seems like a good idea as in that case, automatic search will probably pick it up again later or the user can re-start a manual search (audiobook would be shown in the wanted list with no active download on it).
Also, I expect the majority of orphaned download to come from, either migration to this version or later (one time issue and the software is in canary phase anyway) or from download client manualy removing the queue item from their end (probably due to user input or configuration)
So, if we remove them directly (or even after retrying some times to get it from the adapter), by design we no longer have orphaned download and thus the view is probably not useful :p But if we dont go in that direction, then yes, we could bring a new oprhaned status meaning the download can no longer be linked with anything on the adapter side and the user could then remove only or remove and re-search them

I can see an issue with removing orphaned download: If the adapter is no longer reachable or experience technical issues, then it will send an empty list of queue items and we may delete them by error in those case. We can mitigate that by always checking if the adapter is up and running (using the TestAdapter method) first


var adapter = ResolveAdapter(client);
var items = await adapter.GetQueueAsync(client, ids!, ct);
var tasks = items.Select(item => TranslateQueueItemPathsAsync(client, item));
items = [.. await Task.WhenAll(tasks)];

foreach (QueueItem item in items)
{
var download = downloads.FirstOrDefault(d => d.GetExternalId() == item.Id);
if (download == null)
{
continue;
}

logger.LogDebug($"Found matching qBittorrent torrent for {download.Id}: {item.Title} (Hash: {item.Id}, Status: {item.Status}, Progress: {item.Progress:P2}, LocalPath: {item.LocalPath}, ContentPath: {item.ContentPath})");

// DIAGNOSTIC: Log detailed completion check values
logger.LogInformation($"Completion diagnostic for {download.Id}: Progress={item.Progress:F4} (>= 1.0? {item.Progress >= 1.0}), AmountLeft={item.Size - item.Downloaded} (== 0? {item.Size - item.Downloaded <= 0}), Status={item.Status}");

download = QueueItemConverter.UpdateFromQueueItem(download, item);
}

return downloads;
}

/// <summary>
/// Give the list of external IDs from a list of download
/// </summary>
/// <param name="downloads"></param>
/// <returns></returns>
private List<string> GetExternalIds(List<Download> downloads)
{
return downloads
.Select(d => d.GetExternalId())
.Where(id => id != null)
.ToHashSet()
.ToList()!;
}

/// <summary>
/// Handles path mapping of queue item
/// Make sure all path are localy accessible after processing and
Expand Down Expand Up @@ -201,16 +227,5 @@ private async Task<QueueItem> TranslateQueueItemPathsAsync(DownloadClientConfigu

return item;
}

public async Task<List<Download>> FetchDownloadsAsync(DownloadClientConfiguration client, List<Download> downloads, CancellationToken ct = default)
{
var adapter = ResolveAdapter(client);
downloads = await adapter.FetchDownloadsAsync(client, downloads, ct);
foreach (Download download in downloads)
{
download.DownloadPath = await remotePathMappingService.TranslatePathAsync(client, download.DownloadPath);
}
return downloads;
}
}
}
Loading
Loading