diff --git a/server/RdtClient.Service.Test/Services/Downloaders/DownloadStationDownloaderTest.cs b/server/RdtClient.Service.Test/Services/Downloaders/DownloadStationDownloaderTest.cs index fde1e4d..1723166 100644 --- a/server/RdtClient.Service.Test/Services/Downloaders/DownloadStationDownloaderTest.cs +++ b/server/RdtClient.Service.Test/Services/Downloaders/DownloadStationDownloaderTest.cs @@ -3,11 +3,13 @@ using Moq; using RdtClient.Service.Helpers; using RdtClient.Service.Services.Downloaders; using Synology.Api.Client; +using Synology.Api.Client.ApiDescription; using Synology.Api.Client.Apis.DownloadStation; using Synology.Api.Client.Apis.DownloadStation.Task.Models; using Synology.Api.Client.Apis.FileStation; using Synology.Api.Client.Apis.FileStation.CreateFolder; using Synology.Api.Client.Apis.FileStation.CreateFolder.Models; +using Synology.Api.Client.Exceptions; namespace RdtClient.Service.Test.Services.Downloaders; @@ -162,6 +164,45 @@ public class DownloadStationDownloaderTest mocks.CreateFolderEndpointMock.Verify(e => e.CreateAsync(It.Is(p => p.Single() == "/Media/Downloads/Torrents/MyTorrent"), true), Times.Once); } + [Fact] + public async Task Download_WhenSynologyReportsSessionError119_ReAuthenticatesAndRetries() + { + // Arrange + var mocks = new Mocks(); + mocks.TaskEndpointMock.Setup(t => t.GetInfoAsync(mocks.Gid)).ThrowsAsync(new()); + mocks.TaskEndpointMock.Setup(t => t.ListAsync()).ReturnsAsync(new DownloadStationTaskListResponse { Total = 0, Offset = 0 }); + mocks.TaskEndpointMock.Setup(t => t.CreateAsync(It.IsAny())) + .ReturnsAsync(new DownloadStationTaskCreateResponse { TaskId = [mocks.Gid] }); + + // The first folder-create fails with DSM error 119 ("SID not found"); after re-auth the retry succeeds. + mocks.CreateFolderEndpointMock.SetupSequence(e => e.CreateAsync(It.IsAny(), It.IsAny())) + .ThrowsAsync(new SynologyApiException(new Mock().Object, "FileStation.CreateFolder", 119, "SID not found")) + .ReturnsAsync(new FileStationCreateFolderCreateResponse()); + + var reacquired = 0; + + var downloadStationDownloader = new DownloadStationDownloader(mocks.Gid, + "https://fake.url/file.txt", + "/Media/Downloads/Torrents/MyTorrent/file.txt", + "/path/to/file.txt", + "download-path", + mocks.SynologyClientMock.Object, + reacquireClient: () => + { + reacquired++; + + return Task.FromResult(mocks.SynologyClientMock.Object); + }); + + // Act + var result = await downloadStationDownloader.Download(); + + // Assert + Assert.Equal(mocks.Gid, result); + Assert.Equal(1, reacquired); + mocks.CreateFolderEndpointMock.Verify(e => e.CreateAsync(It.IsAny(), It.IsAny()), Times.Exactly(2)); + } + [Fact] public async Task Download_After5Tries_Throws() { diff --git a/server/RdtClient.Service.Test/Services/Downloaders/SynologyClientProviderTest.cs b/server/RdtClient.Service.Test/Services/Downloaders/SynologyClientProviderTest.cs new file mode 100644 index 0000000..2cd4337 --- /dev/null +++ b/server/RdtClient.Service.Test/Services/Downloaders/SynologyClientProviderTest.cs @@ -0,0 +1,108 @@ +using Moq; +using RdtClient.Service.Services.Downloaders; +using Synology.Api.Client; + +namespace RdtClient.Service.Test.Services.Downloaders; + +public class SynologyClientProviderTest +{ + public SynologyClientProviderTest() + { + // Static cache is shared across the process; start every test from a clean slate. + SynologyClientProvider.Reset(); + } + + [Fact] + public async Task GetClientAsync_ConcurrentCalls_AuthenticatesOnceAndSharesOneClient() + { + var logins = 0; + var shared = new Mock().Object; + + SynologyClientProvider.ClientFactory = (_, _, _) => + { + Interlocked.Increment(ref logins); + + return Task.FromResult(shared); + }; + + var clients = await Task.WhenAll(Enumerable.Range(0, 50).Select(_ => SynologyClientProvider.GetClientAsync("http://nas:5000", "user", "pass"))); + + Assert.Equal(1, logins); + Assert.All(clients, c => Assert.Same(shared, c)); + + SynologyClientProvider.Reset(); + } + + [Fact] + public async Task GetClientAsync_SameCredentials_ReusesClient_DifferentCredentials_ReAuthenticates() + { + var logins = 0; + + SynologyClientProvider.ClientFactory = (_, _, _) => + { + Interlocked.Increment(ref logins); + + return Task.FromResult(new Mock().Object); + }; + + await SynologyClientProvider.GetClientAsync("http://nas:5000", "user", "pass"); + await SynologyClientProvider.GetClientAsync("http://nas:5000", "user", "pass"); + await SynologyClientProvider.GetClientAsync("http://nas:5000", "user", "changed"); + + Assert.Equal(2, logins); + + SynologyClientProvider.Reset(); + } + + [Fact] + public async Task InvalidateAsync_ThenGet_AuthenticatesAgainAndLogsOutTheStaleSession() + { + var logins = 0; + var firstMock = new Mock(); + firstMock.Setup(c => c.LogoutAsync()).Returns(Task.CompletedTask); + + var clients = new Queue([firstMock.Object, new Mock().Object]); + + SynologyClientProvider.ClientFactory = (_, _, _) => + { + Interlocked.Increment(ref logins); + + return Task.FromResult(clients.Dequeue()); + }; + + var first = await SynologyClientProvider.GetClientAsync("http://nas:5000", "user", "pass"); + await SynologyClientProvider.InvalidateAsync(first); + var second = await SynologyClientProvider.GetClientAsync("http://nas:5000", "user", "pass"); + + Assert.Equal(2, logins); + Assert.NotSame(first, second); + firstMock.Verify(c => c.LogoutAsync(), Times.Once); + + SynologyClientProvider.Reset(); + } + + [Fact] + public async Task InvalidateAsync_WithAlreadyReplacedClient_DoesNotDropTheCurrentSession() + { + var logins = 0; + + SynologyClientProvider.ClientFactory = (_, _, _) => + { + Interlocked.Increment(ref logins); + + return Task.FromResult(new Mock().Object); + }; + + var current = await SynologyClientProvider.GetClientAsync("http://nas:5000", "user", "pass"); + + // A late failure referencing a session that was already rotated out must not invalidate the live one. + await SynologyClientProvider.InvalidateAsync(new Mock().Object); + + var afterStaleInvalidate = await SynologyClientProvider.GetClientAsync("http://nas:5000", "user", "pass"); + + Assert.Same(current, afterStaleInvalidate); + Assert.Equal(1, logins); + + SynologyClientProvider.Reset(); + } +} diff --git a/server/RdtClient.Service/Services/Downloaders/DownloadStationDownloader.cs b/server/RdtClient.Service/Services/Downloaders/DownloadStationDownloader.cs index 1f377ef..366b508 100644 --- a/server/RdtClient.Service/Services/Downloaders/DownloadStationDownloader.cs +++ b/server/RdtClient.Service/Services/Downloaders/DownloadStationDownloader.cs @@ -27,9 +27,13 @@ public class DownloadStationDownloader : IDownloader private readonly ILogger _logger; private readonly String? _remotePath; - private readonly ISynologyClient _synologyClient; private readonly String _uri; + // The Synology client is shared across downloads and can be swapped out by WithSessionRetry after a session + // expires, so it is not readonly. + private ISynologyClient _synologyClient; + private readonly Func> _reacquireClient; + private String? _gid; public DownloadStationDownloader(String? gid, @@ -39,7 +43,8 @@ public class DownloadStationDownloader : IDownloader String downloadPath, ISynologyClient synologyClient, IDelayProvider? delayProvider = null, - IFileSystem? fileSystem = null) + IFileSystem? fileSystem = null, + Func>? reacquireClient = null) { _logger = Log.ForContext(); _logger.Debug($"Instantiated new DownloadStation Downloader for URI {uri} to filePath {filePath} (remote {remotePath}) and downloadPath {downloadPath} and GID {gid}"); @@ -51,6 +56,32 @@ public class DownloadStationDownloader : IDownloader _synologyClient = synologyClient; _delayProvider = delayProvider ?? new DelayProvider(); _fileSystem = fileSystem ?? new FileSystem(); + + // When the caller doesn't supply a way to re-authenticate (e.g. unit tests with a mock client), session + // retry just reuses the current client. + _reacquireClient = reacquireClient ?? (() => Task.FromResult(_synologyClient)); + } + + /// + /// Runs a Synology API call, and if DownloadStation reports a session error (e.g. error 119 "SID not found", + /// 106 timeout, 107 duplicate login) drops the shared session, re-authenticates once and retries. With a single + /// shared login these are rare, but a session can still expire between downloads. + /// + private async Task WithSessionRetry(Func> op) + { + try + { + return await op(_synologyClient); + } + catch (SynologyApiException ex) when (ex.ErrorCode is 119 or 106 or 107) + { + _logger.Debug($"DownloadStation session error {ex.ErrorCode} ({ex.ErrorDescription}); re-authenticating and retrying once"); + + await SynologyClientProvider.InvalidateAsync(_synologyClient); + _synologyClient = await _reacquireClient(); + + return await op(_synologyClient); + } } public event EventHandler? DownloadComplete; @@ -68,18 +99,20 @@ public class DownloadStationDownloader : IDownloader _logger.Debug($"Remove download {_uri} {_gid} from Synology DownloadStation"); + var gid = _gid; + try { - await _synologyClient.DownloadStationApi() - .TaskEndpoint() - .DeleteAsync(new() - { - Ids = - [ - _gid - ], - ForceComplete = false - }); + await WithSessionRetry(c => c.DownloadStationApi() + .TaskEndpoint() + .DeleteAsync(new() + { + Ids = + [ + gid + ], + ForceComplete = false + })); } catch (Exception ex) { @@ -134,10 +167,9 @@ public class DownloadStationDownloader : IDownloader // `path` is the share-relative destination folder with a leading slash, e.g. /Media/Downloads/Torrents/. await EnsureRemoteFolderExists(path); - var createResult = await _synologyClient - .DownloadStationApi() - .TaskEndpoint() - .CreateAsync(new(_uri, path[1..])); + var createResult = await WithSessionRetry(c => c.DownloadStationApi() + .TaskEndpoint() + .CreateAsync(new(_uri, path[1..]))); _gid = createResult.TaskId?.FirstOrDefault(); _logger.Debug($"Added download to DownloadStation, received ID {_gid}"); @@ -217,15 +249,21 @@ public class DownloadStationDownloader : IDownloader throw new("No username/password specified for Synology download station"); } - var synologyClient = new SynologyClient(Settings.Get.DownloadClient.DownloadStationUrl); + var url = Settings.Get.DownloadClient.DownloadStationUrl!; + var username = Settings.Get.DownloadClient.DownloadStationUsername!; + var password = Settings.Get.DownloadClient.DownloadStationPassword!; + + ISynologyClient synologyClient; try { - await synologyClient.LoginAsync(Settings.Get.DownloadClient.DownloadStationUsername, Settings.Get.DownloadClient.DownloadStationPassword); + // One authenticated client is shared across all downloads. Logging in per download made DSM invalidate + // older sessions under high file-count concurrency, surfacing as error 119 "SID not found". + synologyClient = await SynologyClientProvider.GetClientAsync(url, username, password); } catch (Exception ex) { - throw new($"DownloadStation login failed for user '{Settings.Get.DownloadClient.DownloadStationUsername}': {ex.Message}. " + + throw new($"DownloadStation login failed for user '{username}': {ex.Message}. " + $"If 2-step verification is enabled on this Synology account, use a dedicated service account without 2FA."); } @@ -258,7 +296,18 @@ public class DownloadStationDownloader : IDownloader } } - return new(gid, uri, remotePath, filePath, downloadPath, synologyClient); + // Re-acquire reads the credentials fresh from settings (not the values captured above) so that editing and + // saving the DownloadStation URL/credentials takes effect live: an in-flight download that has to re-authenticate + // converges on the current settings instead of flipping the shared session back to stale credentials. + return new(gid, + uri, + remotePath, + filePath, + downloadPath, + synologyClient, + reacquireClient: () => SynologyClientProvider.GetClientAsync(Settings.Get.DownloadClient.DownloadStationUrl!, + Settings.Get.DownloadClient.DownloadStationUsername!, + Settings.Get.DownloadClient.DownloadStationPassword!)); } /// @@ -312,9 +361,9 @@ public class DownloadStationDownloader : IDownloader { try { - await _synologyClient.FileStationApi() - .CreateFolderEndpoint() - .CreateAsync([folderPath], true); + await WithSessionRetry(c => c.FileStationApi() + .CreateFolderEndpoint() + .CreateAsync([folderPath], true)); _logger.Debug($"Ensured DownloadStation destination folder exists: {folderPath}"); } @@ -334,7 +383,7 @@ public class DownloadStationDownloader : IDownloader { try { - var tasks = await _synologyClient.DownloadStationApi().TaskEndpoint().ListAsync(); + var tasks = await WithSessionRetry(c => c.DownloadStationApi().TaskEndpoint().ListAsync()); return tasks.Task?.FirstOrDefault(t => t.Additional?.Detail?.Uri == _uri)?.Id; } @@ -456,12 +505,14 @@ public class DownloadStationDownloader : IDownloader { try { - if (_gid == null) + var gid = _gid; + + if (gid == null) { return null; } - return await _synologyClient.DownloadStationApi().TaskEndpoint().GetInfoAsync(_gid); + return await WithSessionRetry(c => c.DownloadStationApi().TaskEndpoint().GetInfoAsync(gid)); } catch (Exception ex) { diff --git a/server/RdtClient.Service/Services/Downloaders/SynologyClientProvider.cs b/server/RdtClient.Service/Services/Downloaders/SynologyClientProvider.cs new file mode 100644 index 0000000..fbde66f --- /dev/null +++ b/server/RdtClient.Service/Services/Downloaders/SynologyClientProvider.cs @@ -0,0 +1,124 @@ +using Synology.Api.Client; + +namespace RdtClient.Service.Services.Downloaders; + +/// +/// Holds a single authenticated shared across all Download Station downloads. +/// Logging in once instead of once per download avoids the concurrent-login session churn that made DSM return +/// error 119 "SID not found" under high file-count concurrency. The session is created on demand, reused while +/// the credentials are unchanged, and dropped (via ) when a download hits a session +/// error so the next caller logs in again. +/// +public static class SynologyClientProvider +{ + private static readonly Func> DefaultClientFactory = async (url, username, password) => + { + var client = new SynologyClient(url); + await client.LoginAsync(username, password); + + return client; + }; + + private static readonly SemaphoreSlim Gate = new(1, 1); + + private static ISynologyClient? _client; + private static (String Url, String Username, String Password)? _key; + + /// + /// Builds an authenticated client. Overridable so tests can supply a fake without a live DSM. Restored by + /// . + /// + public static Func> ClientFactory { get; set; } = DefaultClientFactory; + + /// + /// Returns the shared authenticated client for these credentials, logging in once. Concurrent first calls + /// collapse to a single login; later calls reuse the cached client. + /// + public static async Task GetClientAsync(String url, String username, String password) + { + var key = (url, username, password); + + // Fast path: a client for these exact credentials already exists. + var current = _client; + + if (current != null && _key == key) + { + return current; + } + + await Gate.WaitAsync(); + + try + { + if (_client != null && _key == key) + { + return _client; + } + + // No client yet, or the credentials changed: drop any stale session and authenticate fresh. + await TryLogoutAsync(_client); + + _client = await ClientFactory(url, username, password); + _key = key; + + return _client; + } + finally + { + Gate.Release(); + } + } + + /// + /// Drops the cached session so the next authenticates again. Pass the client that + /// just failed so a session another caller has already refreshed is not thrown away. + /// + public static async Task InvalidateAsync(ISynologyClient? failedClient = null) + { + await Gate.WaitAsync(); + + try + { + if (failedClient != null && !ReferenceEquals(_client, failedClient)) + { + return; + } + + await TryLogoutAsync(_client); + + _client = null; + _key = null; + } + finally + { + Gate.Release(); + } + } + + /// + /// Clears the cached session and restores the default factory. For test isolation. + /// + public static void Reset() + { + _client = null; + _key = null; + ClientFactory = DefaultClientFactory; + } + + private static async Task TryLogoutAsync(ISynologyClient? client) + { + if (client == null) + { + return; + } + + try + { + await client.LogoutAsync(); + } + catch + { + // Best-effort: an expired or dead session cannot be logged out, and that is fine. + } + } +}