Share one authenticated Synology session across downloads
Init() created a new SynologyClient and logged in for every download. A many-file download opened dozens of near-simultaneous sessions for one DSM account; DSM invalidates older session IDs, so in-flight File Station calls failed with error 119 "SID not found". Add SynologyClientProvider: a thread-safe cache that authenticates once and shares the client across downloads (concurrent first calls collapse to a single login; later calls reuse it; a credential change re-authenticates). Wrap every Synology call in a session-retry that drops the session and re-authenticates once on a session error (119/106/107) so an expired SID self-heals, reading credentials fresh from settings so edits take effect live. Adds provider unit tests and a downloader test covering the 119 re-auth path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c8313361a2
commit
643a2e3d63
4 changed files with 350 additions and 26 deletions
|
|
@ -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<String[]>(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<DownloadStationTaskCreateRequest>()))
|
||||
.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<String[]>(), It.IsAny<Boolean>()))
|
||||
.ThrowsAsync(new SynologyApiException(new Mock<IApiInfo>().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<String[]>(), It.IsAny<Boolean>()), Times.Exactly(2));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Download_After5Tries_Throws()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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<ISynologyClient>().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<ISynologyClient>().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<ISynologyClient>();
|
||||
firstMock.Setup(c => c.LogoutAsync()).Returns(Task.CompletedTask);
|
||||
|
||||
var clients = new Queue<ISynologyClient>([firstMock.Object, new Mock<ISynologyClient>().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<ISynologyClient>().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<ISynologyClient>().Object);
|
||||
|
||||
var afterStaleInvalidate = await SynologyClientProvider.GetClientAsync("http://nas:5000", "user", "pass");
|
||||
|
||||
Assert.Same(current, afterStaleInvalidate);
|
||||
Assert.Equal(1, logins);
|
||||
|
||||
SynologyClientProvider.Reset();
|
||||
}
|
||||
}
|
||||
|
|
@ -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<Task<ISynologyClient>> _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<Task<ISynologyClient>>? reacquireClient = null)
|
||||
{
|
||||
_logger = Log.ForContext<DownloadStationDownloader>();
|
||||
_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));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
private async Task<T> WithSessionRetry<T>(Func<ISynologyClient, Task<T>> 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<DownloadCompleteEventArgs>? 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/<name>.
|
||||
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!));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
@ -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)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -0,0 +1,124 @@
|
|||
using Synology.Api.Client;
|
||||
|
||||
namespace RdtClient.Service.Services.Downloaders;
|
||||
|
||||
/// <summary>
|
||||
/// Holds a single authenticated <see cref="ISynologyClient" /> 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 <see cref="InvalidateAsync" />) when a download hits a session
|
||||
/// error so the next caller logs in again.
|
||||
/// </summary>
|
||||
public static class SynologyClientProvider
|
||||
{
|
||||
private static readonly Func<String, String, String, Task<ISynologyClient>> 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;
|
||||
|
||||
/// <summary>
|
||||
/// Builds an authenticated client. Overridable so tests can supply a fake without a live DSM. Restored by
|
||||
/// <see cref="Reset" />.
|
||||
/// </summary>
|
||||
public static Func<String, String, String, Task<ISynologyClient>> ClientFactory { get; set; } = DefaultClientFactory;
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
public static async Task<ISynologyClient> 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();
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Drops the cached session so the next <see cref="GetClientAsync" /> authenticates again. Pass the client that
|
||||
/// just failed so a session another caller has already refreshed is not thrown away.
|
||||
/// </summary>
|
||||
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();
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Clears the cached session and restores the default factory. For test isolation.
|
||||
/// </summary>
|
||||
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.
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue