Skip to content

Commit

Permalink
Make SteamClient scoped so it's disposed after every job (#91)
Browse files Browse the repository at this point in the history 
* Make SteamClient scoped so it's disposed after every job

* Add tests for scopes

* Prevent attempts to connect to Steam multiple times

* Fix log message appearing even if client didn't connect
  • Loading branch information
@3Mydlo3
3Mydlo3 authored Nov 15, 2023
1 parent 02b9068 commit 660d59a
Show file tree
Hide file tree
Showing 11 changed files with 118 additions and 102 deletions.
4 changes: 2 additions & 2 deletions ArmaForces.Arma.Server.Tests/Helpers/TestSettings.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ public class TestSettings : ISettings
public string? ServerDirectory { get; set; } = MockUnixSupport.Path("C:\\Arma");
public string? ServerExecutable => MockUnixSupport.Path($"{ServerDirectory}\\{ServerExecutableName}");
public string ServerExecutableName { get; set; } = "arma3.exe";
public string? SteamUser { get; set; }
public string? SteamPassword { get; set; }
public string? SteamUser { get; set; } = "TEST_USER";
public string? SteamPassword { get; set; } = "TEST_PASSWORD";

public Result LoadSettings() => throw new System.NotImplementedException();

Expand Down
16 changes: 1 addition & 15 deletions ArmaForces.ArmaServerManager.Tests/Features/Mods/ModsManagerUnitTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
using ArmaForces.Arma.Server.Tests.Helpers;
using ArmaForces.ArmaServerManager.Extensions;
using ArmaForces.ArmaServerManager.Features.Mods;
using ArmaForces.ArmaServerManager.Features.Steam;
using ArmaForces.ArmaServerManager.Features.Steam.Content;
using ArmaForces.ArmaServerManager.Features.Steam.Content.DTOs;
using AutoFixture;
using CSharpFunctionalExtensions;
using FluentAssertions;
using Microsoft.Extensions.Logging.Abstractions;
using Moq;
using Xunit;
Expand All @@ -24,20 +24,17 @@ public class ModsManagerUnitTests
private readonly Mock<IModsCache> _modsCacheMock;
private readonly Mock<IContentVerifier> _contentVerifierMock;
private readonly Mock<IContentDownloader> _downloaderMock;
private readonly Mock<ISteamClient> _steamClientMock;
private readonly ModsManager _modsManager;

public ModsManagerUnitTests()
{
_modsCacheMock = CreateModsCacheMock();
_contentVerifierMock = CreateContentVerifierMock();
_downloaderMock = CreateContentDownloaderMock();
_steamClientMock = CreateSteamClientMock();
_modsManager = new ModsManager(
_downloaderMock.Object,
_contentVerifierMock.Object,
_modsCacheMock.Object,
_steamClientMock.Object,
new NullLogger<ModsManager>());
}

Expand Down Expand Up @@ -206,16 +203,5 @@ private Mock<IContentDownloader> CreateContentDownloaderMock()

return mock;
}

private Mock<ISteamClient> CreateSteamClientMock()
{
var mock = new Mock<ISteamClient>();

mock
.Setup(x => x.EnsureConnected(It.IsAny<CancellationToken>()))
.Returns(Task.CompletedTask);

return mock;
}
}
}
39 changes: 39 additions & 0 deletions ArmaForces.ArmaServerManager.Tests/Features/Steam/SteamClientTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
using System.Threading;
using System.Threading.Tasks;
using ArmaForces.Arma.Server.Config;
using ArmaForces.Arma.Server.Tests.Helpers;
using ArmaForces.ArmaServerManager.Features.Mods.DependencyInjection;
using ArmaForces.ArmaServerManager.Features.Steam;
using FluentAssertions;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging.Abstractions;
using Moq;
using Xunit;
Expand Down Expand Up @@ -38,6 +41,42 @@ public void Connect_InvalidCredentials_ThrowsInvalidCredentialsException()
action.Should().ThrowAsync<InvalidCredentialException>("Invalid Steam Credentials");
}

[Fact]
public void GetSteamClient_TwoScopes_DifferentInstancesReturned()
{
var serviceProvider = new ServiceCollection()
.AddSingleton<ISettings, TestSettings>()
.AddMods()
.BuildServiceProvider();

ISteamClient CreateClientFromNewScope()
{
using var scope = serviceProvider.CreateScope();
return scope.ServiceProvider.GetRequiredService<ISteamClient>();
}

var firstScopeClient = CreateClientFromNewScope();
var secondScopeClient = CreateClientFromNewScope();

firstScopeClient.Should().NotBe(secondScopeClient, because: "Different instance should be created for each scope.");
}

[Fact]
public void GetSteamClient_OneScope_SameInstanceReturned()
{
var serviceProvider = new ServiceCollection()
.AddSingleton<ISettings, TestSettings>()
.AddMods()
.BuildServiceProvider();

using var scope = serviceProvider.CreateScope();

var firstScopeClient = scope.ServiceProvider.GetRequiredService<ISteamClient>();
var secondScopeClient = scope.ServiceProvider.GetRequiredService<ISteamClient>();

firstScopeClient.Should().Be(secondScopeClient, because: "Same instance should be used across the scope.");
}

private static SteamClient CreateSteamClient()
=> new SteamClient(PrepareMockedSettings(), NullLogger<SteamClient>.Instance);

Expand Down
1 change: 1 addition & 0 deletions ArmaForces.ArmaServerManager/ArmaForces.ArmaServerManager.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<PackageReference Include="Hangfire.LiteDB" Version="0.4.1" />
<PackageReference Include="Hangfire.MaximumConcurrentExecutions" Version="1.1.0" />
<PackageReference Include="Microsoft.Extensions.Hosting" Version="6.0.1" />
<PackageReference Include="Polly" Version="6.1.2" />
<PackageReference Include="Swashbuckle.AspNetCore.Annotations" Version="6.3.1" />
<PackageReference Include="Swashbuckle.AspNetCore.ReDoc" Version="6.3.1" />
</ItemGroup>
Expand Down
12 changes: 6 additions & 6 deletions ...es.ArmaServerManager/Features/Mods/DependencyInjection/ModsServiceCollectionExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public static IServiceCollection AddMods(this IServiceCollection services)
return services
.AddSingleton<ModsCache>()
.AddSingleton<IModsCache, ModsCache>()
.AddSingleton<IModsManager, ModsManager>()
.AddScoped<IModsManager, ModsManager>()
.AddContent()
.AddSingleton<IWebModsetMapper, ModsCache>()
.AddModsetsApiClient()
Expand All @@ -22,10 +22,10 @@ public static IServiceCollection AddMods(this IServiceCollection services)

private static IServiceCollection AddContent(this IServiceCollection services)
=> services
.AddSingleton<ISteamClient, SteamClient>()
.AddSingleton<IManifestDownloader, ManifestDownloader>()
.AddSingleton<IContentDownloader, ContentDownloader>()
.AddSingleton<IContentVerifier, ContentVerifier>()
.AddSingleton<IContentFileVerifier, ContentFileVerifier>();
.AddScoped<ISteamClient, SteamClient>()
.AddScoped<IManifestDownloader, ManifestDownloader>()
.AddScoped<IContentDownloader, ContentDownloader>()
.AddScoped<IContentVerifier, ContentVerifier>()
.AddScoped<IContentFileVerifier, ContentFileVerifier>();
}
}
7 changes: 0 additions & 7 deletions ArmaForces.ArmaServerManager/Features/Mods/ModsManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using ArmaForces.Arma.Server.Features.Mods;
using ArmaForces.Arma.Server.Features.Modsets;
using ArmaForces.ArmaServerManager.Extensions;
using ArmaForces.ArmaServerManager.Features.Steam;
using ArmaForces.ArmaServerManager.Features.Steam.Content;
using CSharpFunctionalExtensions;
using Microsoft.Extensions.Logging;
Expand All @@ -20,26 +19,22 @@ internal class ModsManager : IModsManager
private readonly IContentDownloader _contentDownloader;
private readonly IContentVerifier _contentVerifier;
private readonly IModsCache _modsCache;
private readonly ISteamClient _steamClient;
private readonly ILogger<ModsManager> _logger;

/// <inheritdoc cref="ModsManager" />
/// <param name="contentDownloader">Client for mods download and updating.</param>
/// <param name="contentVerifier">Client for verifying whether mods are up to date and correct.</param>
/// <param name="modsCache">Installed mods cache.</param>
/// <param name="steamClient"></param>
/// <param name="logger">Logger.</param>
public ModsManager(
IContentDownloader contentDownloader,
IContentVerifier contentVerifier,
IModsCache modsCache,
ISteamClient steamClient,
ILogger<ModsManager> logger)
{
_contentDownloader = contentDownloader;
_contentVerifier = contentVerifier;
_modsCache = modsCache;
_steamClient = steamClient;
_logger = logger;
}

Expand Down Expand Up @@ -96,8 +91,6 @@ await _contentVerifier.ItemIsUpToDate(mod.AsContentItem(), cancellationToken)
/// <param name="cancellationToken"><see cref="CancellationToken" /> used for mods download safe cancelling.</param>
private async Task<Result> CheckUpdatesAndDownloadMods(IEnumerable<Mod> modsToDownload, CancellationToken cancellationToken)
{
await _steamClient.EnsureConnected(cancellationToken);

return await CheckModsUpdated(modsToDownload.ToList(), cancellationToken)
.Bind(modsMissingOrOutdated => DownloadMods(modsMissingOrOutdated, cancellationToken));
}
Expand Down
17 changes: 14 additions & 3 deletions ArmaForces.ArmaServerManager/Features/Steam/Content/ContentDownloader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,17 @@ public class ContentDownloader : IContentDownloader
private readonly ISteamClient _steamClient;
private readonly ILogger<ContentDownloader> _logger;

/// <inheritdoc />
/// <param name="settings">Application settings.</param>
/// <param name="steamClient">Client used for connection.</param>
/// <param name="logger">Logger</param>
// Used by DI
// ReSharper disable once UnusedMember.Global
public ContentDownloader(
ISettings settings,
ISteamClient steamClient,
ILogger<ContentDownloader> contentDownloader)
: this(steamClient, settings.ModsDirectory!,
contentDownloader)
ILogger<ContentDownloader> logger)
: this(steamClient, settings.ModsDirectory!, logger)
{
}

Expand All @@ -50,6 +55,12 @@ private ContentDownloader(
_modsDirectory = modsDirectory;
}

/// <summary>
/// Downloads or updates given <paramref name="mods"/> collection.
/// </summary>
/// <param name="mods">Collection of mods to update.</param>
/// <param name="cancellationToken"></param>
/// <returns></returns>
public async Task<List<Result<Mod>>> DownloadOrUpdateMods(
IReadOnlyCollection<Mod> mods,
CancellationToken cancellationToken)
Expand Down
91 changes: 37 additions & 54 deletions ArmaForces.ArmaServerManager/Features/Steam/Content/ManifestDownloader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using BytexDigital.Steam.ContentDelivery.Models;
using BytexDigital.Steam.Core.Structs;
using Microsoft.Extensions.Logging;
using Polly;
using SteamKit2;

namespace ArmaForces.ArmaServerManager.Features.Steam.Content
Expand All @@ -22,73 +23,55 @@ public ManifestDownloader(ISteamClient steamClient, ILogger<ManifestDownloader>
}

public async Task<Manifest> GetManifest(ContentItem contentItem, CancellationToken cancellationToken)
=> await _steamClient.ContentClient.GetManifestAsync(
{
await _steamClient.EnsureConnected(cancellationToken);

var cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
cancellationTokenSource.CancelAfter(TimeSpan.FromSeconds(15));

return await _steamClient.ContentClient.GetManifestAsync(
appId: SteamConstants.ArmaAppId,
depotId: SteamConstants.ArmaWorkshopDepotId,
manifestId: await GetManifestId(contentItem, cancellationToken),
cancellationToken: cancellationToken);

/// <summary>
/// TODO: Do it better
/// </summary>
manifestId: await GetManifestId(contentItem, cancellationTokenSource.Token),
cancellationToken: cancellationTokenSource.Token);
}

private async Task<ManifestId> GetManifestId(ContentItem contentItem, CancellationToken cancellationToken)
{
_logger.LogDebug("Downloading ManifestId for item {ContentItemId}", contentItem.Id);
var asyncJobFailedPolicy = Policy<ulong>
.Handle<AsyncJobFailedException>()
.WaitAndRetryAsync(
retryCount: SteamContentConstants.MaximumRetryCount,
sleepDurationProvider: _ => TimeSpan.FromSeconds(5),
onRetry: (result, _, _) => LogManifestIdDownloadFailure(result.Exception, contentItem));

var errors = 0;
var taskCanceledPolicy = Policy<ulong>
.Handle<TaskCanceledException>()
.FallbackAsync(Task.FromCanceled<ulong>);

// TODO: Use Polly
while (true)
{
try
{
return (await _steamClient.ContentClient.GetPublishedFileDetailsAsync(contentItem.Id, cancellationToken))
.hcontent_file;
}
catch (TaskCanceledException exception)
{
errors++;
LogManifestIdDownloadFailure(contentItem, exception, errors);

if (errors >= SteamContentConstants.MaximumRetryCount)
{
throw CreateManifestDownloadException(errors, contentItem, exception);
}

await Task.Delay(TimeSpan.FromSeconds(5), cancellationToken);
}
catch (AsyncJobFailedException exception)
{
errors++;
LogManifestIdDownloadFailure(contentItem, exception, errors);
var policy = Policy.WrapAsync(asyncJobFailedPolicy, taskCanceledPolicy);

if (errors >= SteamContentConstants.MaximumRetryCount)
{
throw CreateManifestDownloadException(errors, contentItem, exception);
}
_logger.LogDebug("Downloading ManifestId for item {ContentItemId}", contentItem.Id);

var result = await policy.ExecuteAndCaptureAsync(async token =>
(await _steamClient.ContentClient.GetPublishedFileDetailsAsync(contentItem.Id, token)).hcontent_file,
cancellationToken);

await Task.Delay(TimeSpan.FromSeconds(5), cancellationToken);
}
}
return result.Outcome == OutcomeType.Successful
? result.Result
: throw CreateManifestDownloadException(contentItem, result.FinalException);
}

private void LogManifestIdDownloadFailure(
ContentItem contentItem,
Exception exception,
int errors)
private void LogManifestIdDownloadFailure(Exception exception, ContentItem contentItem)
=> _logger.LogTrace(
exception,
"Failed to download ManifestId for item {ContentItemId}. Errors = {Number}",
contentItem.Id,
errors);
"Failed to download ManifestId for item {ContentItemId}",
contentItem.Id);

private Exception CreateManifestDownloadException(
int errors,
ContentItem contentItem,
Exception? innerException = null)
private Exception CreateManifestDownloadException(ContentItem contentItem, Exception? innerException = null)
{
var newException = new Exception(
$"{errors} errors while attempting to download manifest for {contentItem.Id}",
$"Failed while attempting to download manifest for {contentItem.Id}",
innerException);

if (innerException is null)
Expand All @@ -101,10 +84,10 @@ private Exception CreateManifestDownloadException(
else
{
_logger.LogError(
newException,
innerException,
"Could not download ManifestId for item {ContentItemId}, error message {Message}",
contentItem.Id,
innerException.Message);
newException.Message);
}

return newException;
Expand Down
5 changes: 0 additions & 5 deletions ArmaForces.ArmaServerManager/Features/Steam/ISteamClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,5 @@ public interface ISteamClient
/// <param name="cancellationToken"><see cref="CancellationToken" /> used for safe connection aborting.</param>
/// <returns>Awaitable <see cref="Task" /></returns>
Task EnsureConnected(CancellationToken cancellationToken);

/// <summary>
/// Disconnects from Steam Servers.
/// </summary>
void Disconnect();
}
}
Loading

0 comments on commit 660d59a

Please sign in to comment.