diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs index b539536a5b..220735786e 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs @@ -182,7 +182,7 @@ public virtual IServiceCollection RegisterServices(IServiceCollection services) { return new VmrInfo(string.Empty, string.Empty); }); - services.TryAddSingleton(); + services.TryAddSingleton(); return services; } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/IRedisCacheClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/IDistributedCacheClient.cs similarity index 63% rename from src/Microsoft.DotNet.Darc/DarcLib/IRedisCacheClient.cs rename to src/Microsoft.DotNet.Darc/DarcLib/IDistributedCacheClient.cs index fdc5871458..470186bcbe 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/IRedisCacheClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/IDistributedCacheClient.cs @@ -7,15 +7,22 @@ #nullable enable namespace Microsoft.DotNet.DarcLib; -public interface IRedisCacheClient +/// +/// Generic interface for caching POCOs. Implementations should create keys based on the provided key and +/// object type to avoid collisions. +/// +public interface IDistributedCacheClient { Task TrySetAsync(string key, T value, TimeSpan? expiration = null) where T : class; Task TryGetAsync(string key) where T : class; Task DeleteAsync(string key); } -// This no-op redis client is used when DarcLib is invoked through CLI operations where redis is not available. -public class NoOpRedisClient : IRedisCacheClient +/// +/// This no-op cache client the default implementation of IDistributedCacheClient in DarcLib. Caching is not mandatory +/// and requires explicit implemnetation if used. +/// +public class NoOpCacheClient : IDistributedCacheClient { public Task TrySetAsync(string key, T value, TimeSpan? expiration = null) where T : class { diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Remote.cs b/src/Microsoft.DotNet.Darc/DarcLib/Remote.cs index 6ba32349b7..007e5c365f 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Remote.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Remote.cs @@ -26,7 +26,7 @@ public sealed class Remote : IRemote private readonly ISourceMappingParser _sourceMappingParser; private readonly IRemoteFactory _remoteFactory; private readonly IAssetLocationResolver _locationResolver; - private readonly IRedisCacheClient _cache; + private readonly IDistributedCacheClient _cache; private readonly ILogger _logger; //[DependencyUpdate]: <> (Begin) @@ -43,7 +43,7 @@ public Remote( ISourceMappingParser sourceMappingParser, IRemoteFactory remoteFactory, IAssetLocationResolver locationResolver, - IRedisCacheClient cacheClient, + IDistributedCacheClient cacheClient, ILogger logger) { _logger = logger; @@ -53,7 +53,9 @@ public Remote( _remoteFactory = remoteFactory; _locationResolver = locationResolver; _fileManager = new DependencyFileManager(remoteGitClient, _versionDetailsParser, _logger); - _cache = cacheClient; + _cache = cacheClient ?? throw new ArgumentNullException( + nameof(cacheClient), + "The cache client must not be null. Please use the no-op implementation of IDistributedCacheClient provided in this package."); } public async Task CreateNewBranchAsync(string repoUri, string baseBranch, string newBranch) diff --git a/src/ProductConstructionService/ProductConstructionService.Common/ProductConstructionServiceExtension.cs b/src/ProductConstructionService/ProductConstructionService.Common/ProductConstructionServiceExtension.cs index fdce6ef3cb..3217073b4a 100644 --- a/src/ProductConstructionService/ProductConstructionService.Common/ProductConstructionServiceExtension.cs +++ b/src/ProductConstructionService/ProductConstructionService.Common/ProductConstructionServiceExtension.cs @@ -77,7 +77,7 @@ public static async Task AddRedisCache( builder.Services.AddSingleton(redisConfig); builder.Services.AddSingleton(); - builder.Services.AddSingleton(); + builder.Services.AddSingleton(); } public static void AddMetricRecorder(this IHostApplicationBuilder builder) diff --git a/src/ProductConstructionService/ProductConstructionService.Common/RedisCacheClient.cs b/src/ProductConstructionService/ProductConstructionService.Common/RedisCacheClient.cs index 4c8aaa82b2..f578a1eb44 100644 --- a/src/ProductConstructionService/ProductConstructionService.Common/RedisCacheClient.cs +++ b/src/ProductConstructionService/ProductConstructionService.Common/RedisCacheClient.cs @@ -7,11 +7,12 @@ namespace ProductConstructionService.Common; /// -/// This class acts as a delegate for RedisCache and RedisCacheFactory. -/// It is needed because DarcLib does not depend on ProductConstructionService and can only access caching through a delegate. +/// This class acts as a delegate for RedisCache and RedisCacheFactory. It is needed because DarcLib +/// does not depend on ProductConstructionService and can only implement caching there through a delegate. /// -internal class RedisCacheClient : IRedisCacheClient +internal class RedisCacheClient : IDistributedCacheClient { + private static readonly TimeSpan DefaultExpiration = TimeSpan.FromDays(15); private readonly IRedisCacheFactory _factory; private readonly ILogger _logger; @@ -26,11 +27,11 @@ public RedisCacheClient(IRedisCacheFactory factory, ILogger lo return await _factory.Create(key).TryGetStateAsync(); } - public async Task TrySetAsync(string key, T value, TimeSpan? expiration = null) where T : class + public async Task TrySetAsync(string key, T value, TimeSpan? expiration) where T : class { try { - await _factory.Create(key).SetAsync(value, expiration); + await _factory.Create(key).SetAsync(value, expiration ?? DefaultExpiration); } catch (Exception ex) { diff --git a/test/Microsoft.DotNet.DarcLib.Tests/RemoteTests.cs b/test/Microsoft.DotNet.DarcLib.Tests/RemoteTests.cs index 559774f3db..5226e730e0 100644 --- a/test/Microsoft.DotNet.DarcLib.Tests/RemoteTests.cs +++ b/test/Microsoft.DotNet.DarcLib.Tests/RemoteTests.cs @@ -101,7 +101,7 @@ See [Dependency Description Format](https://github.com/dotnet/arcade/blob/main/D sourceMappingParser.Object, Mock.Of(), new AssetLocationResolver(barClient.Object), - new NoOpRedisClient(), + new NoOpCacheClient(), logger); await remote.MergeDependencyPullRequestAsync("https://github.com/test/test2", mergePullRequest); diff --git a/test/ProductConstructionService.DependencyFlow.Tests/PullRequestPolicyFailureNotifierTests.cs b/test/ProductConstructionService.DependencyFlow.Tests/PullRequestPolicyFailureNotifierTests.cs index e38170c890..524cb9769d 100644 --- a/test/ProductConstructionService.DependencyFlow.Tests/PullRequestPolicyFailureNotifierTests.cs +++ b/test/ProductConstructionService.DependencyFlow.Tests/PullRequestPolicyFailureNotifierTests.cs @@ -101,7 +101,7 @@ where subscription.Id.Equals(subscriptionToFind) SourceMappingParser.Object, RemoteFactory.Object, new AssetLocationResolver(BarClient.Object), - new NoOpRedisClient(), + new NoOpCacheClient(), NullLogger.Instance); RemoteFactory.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(MockRemote);