Skip to content

feat: enhance distributed lock service with Redis integration and con… #15

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

Merged
Phelecks merged 1 commit into from
Jan 21, 2026
Merged

feat: enhance distributed lock service with Redis integration and con… #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 13 additions & 0 deletions src/DistributedLockManager/Configuration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,26 @@
using DistributedLockManager.Interfaces;
using DistributedLockManager.Services;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using RedLockNet;
using RedLockNet.SERedis;
using RedLockNet.SERedis.Configuration;
using StackExchange.Redis;

namespace DistributedLockManager;

public static class CacheManagerConfiguration
{
public static void AddDistributedLockManager(this IServiceCollection services)
{
services.AddSingleton<IDistributedLockFactory, RedLockFactory>(sp =>
{
var connectionMultiplexer = sp.GetRequiredService<IConnectionMultiplexer>();
var loggerFactory = sp.GetRequiredService<ILoggerFactory>();

return RedLockFactory.Create([new RedLockMultiplexer(connectionMultiplexer)], loggerFactory);
});
Comment on lines +16 to +22

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

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

The factory registration requires ILoggerFactory but this dependency isn't documented. If ILoggerFactory is not registered in the DI container when AddDistributedLockManager is called, GetRequiredService will throw an exception. Consider either documenting this prerequisite in XML comments or making the logger optional by using GetService and providing a null logger to the factory if logging isn't configured.

Copilot uses AI. Check for mistakes.

services.AddScoped<IDistributedLockService, DistributedLockService>();
services.AddScoped(typeof(IDistributedLockService<>), typeof(DistributedLockService<>));
}
Expand Down
34 changes: 7 additions & 27 deletions src/DistributedLockManager/Services/DistributedLockService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,14 @@
using DistributedLockManager.Interfaces;
using RedLockNet.SERedis;
using RedLockNet.SERedis.Configuration;
using StackExchange.Redis;
using RedLockNet;

namespace DistributedLockManager.Services;

public class DistributedLockService : IDistributedLockService
public class DistributedLockService(IDistributedLockFactory distributedLockFactory) : IDistributedLockService
{
private readonly RedLockFactory _redLockFactory;
public DistributedLockService(IConnectionMultiplexer connectionMultiplexer)
{
var endPoints = connectionMultiplexer.GetEndPoints();
List<RedLockEndPoint> redLockEndPoints = [.. endPoints];

_redLockFactory = RedLockFactory.Create(redLockEndPoints);
}

public async Task RunWithLockAsync(Func<Task> func, string key, CancellationToken cancellationToken, int expiryInSecond = 30, int waitInSecond = 10, int retryInSecond = 1)
{
// blocks until acquired or 'wait' timeout
await using var redLock = await _redLockFactory.CreateLockAsync(key, TimeSpan.FromSeconds(expiryInSecond), TimeSpan.FromSeconds(waitInSecond),
await using var redLock = await distributedLockFactory.CreateLockAsync(key, TimeSpan.FromSeconds(expiryInSecond), TimeSpan.FromSeconds(waitInSecond),
TimeSpan.FromSeconds(retryInSecond), cancellationToken);

if (redLock.IsAcquired)
Expand All @@ -29,29 +18,20 @@ public async Task RunWithLockAsync(Func<Task> func, string key, CancellationToke
public async Task RunWithLockAsync(Task task, string key, CancellationToken cancellationToken, int expiryInSecond = 30, int waitInSecond = 10, int retryInSecond = 1)
{
// blocks until acquired or 'wait' timeout
await using var redLock = await _redLockFactory.CreateLockAsync(key, TimeSpan.FromSeconds(expiryInSecond), TimeSpan.FromSeconds(waitInSecond),
await using var redLock = await distributedLockFactory.CreateLockAsync(key, TimeSpan.FromSeconds(expiryInSecond), TimeSpan.FromSeconds(waitInSecond),
TimeSpan.FromSeconds(retryInSecond), cancellationToken);

if (redLock.IsAcquired)
await task;
}
Comment on lines 18 to 26

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

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

The void version of RunWithLockAsync doesn't throw an exception when the lock is not acquired, creating an inconsistent behavior with the generic version (IDistributedLockService). When the lock fails to be acquired, this method silently completes without executing the task, which could lead to data inconsistency or missed operations. Consider throwing an InvalidOperationException similar to the generic version to ensure consistent error handling across both service implementations.

Copilot uses AI. Check for mistakes.
}

public class DistributedLockService<TResult> : IDistributedLockService<TResult>
public class DistributedLockService<TResult>(IDistributedLockFactory distributedLockFactory) : IDistributedLockService<TResult>
{
private readonly RedLockFactory _redLockFactory;
public DistributedLockService(IConnectionMultiplexer connectionMultiplexer)
{
var endPoints = connectionMultiplexer.GetEndPoints();
List<RedLockEndPoint> redLockEndPoints = [.. endPoints];

_redLockFactory = RedLockFactory.Create(redLockEndPoints);
}

public async Task<TResult> RunWithLockAsync(Func<Task<TResult>> func, string key, CancellationToken cancellationToken, int expiryInSecond = 30, int waitInSecond = 10, int retryInSecond = 1)
{
// blocks until acquired or 'wait' timeout
await using var redLock = await _redLockFactory.CreateLockAsync(key,
await using var redLock = await distributedLockFactory.CreateLockAsync(key,
expiryTime: TimeSpan.FromSeconds(expiryInSecond),
waitTime: TimeSpan.FromSeconds(waitInSecond),
retryTime: TimeSpan.FromSeconds(retryInSecond), cancellationToken);
Expand All @@ -66,7 +46,7 @@ public async Task<TResult> RunWithLockAsync(Func<Task<TResult>> func, string key
public async Task<TResult> RunWithLockAsync(Task<TResult> task, string key, CancellationToken cancellationToken, int expiryInSecond = 30, int waitInSecond = 10, int retryInSecond = 1)
{
// blocks until acquired or 'wait' timeout
await using var redLock = await _redLockFactory.CreateLockAsync(key, TimeSpan.FromSeconds(expiryInSecond), TimeSpan.FromSeconds(waitInSecond),
await using var redLock = await distributedLockFactory.CreateLockAsync(key, TimeSpan.FromSeconds(expiryInSecond), TimeSpan.FromSeconds(waitInSecond),
TimeSpan.FromSeconds(retryInSecond), cancellationToken);

if (redLock.IsAcquired)
Expand Down
17 changes: 13 additions & 4 deletions tests/DistributedLockManager.Tests/DistributedLockServiceIntegrationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,33 +17,39 @@ public class DistributedLockServiceIntegrationTests
{
private IServiceScope? scope;
private IContainer? _redisContainer;
private IConnectionMultiplexer? _connectionMultiplexer;
private const string RedisImage = "redis:latest";
private const int RedisPort = 6379;

[OneTimeSetUp]
public async Task OneTimeSetUp()
{
_redisContainer = new ContainerBuilder()
.WithImage(RedisImage)
_redisContainer = new ContainerBuilder(RedisImage)
.WithCleanUp(true)
.WithName($"dtm-redis-{Guid.NewGuid():N}")
.WithPortBinding(RedisPort, true)
.WithWaitStrategy(Wait.ForUnixContainer().UntilExternalTcpPortIsAvailable(RedisPort))
.Build();

if (_redisContainer == null) Assert.Fail("Redis container not started");

await _redisContainer!.StartAsync();

// Wait a bit for Redis to be fully ready
await Task.Delay(1000);

Comment on lines +38 to +40

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

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

The hardcoded 1-second delay after container start is a brittle approach to waiting for Redis readiness. The test already uses WithWaitStrategy on line 31 which should handle this. If the wait strategy isn't sufficient, consider increasing its timeout or using a more reliable readiness check (e.g., attempting a Redis PING command with retry logic) instead of an arbitrary delay.

Suggested change
// Wait a bit for Redis to be fully ready
await Task.Delay(1000);

Copilot uses AI. Check for mistakes.
var container = _redisContainer!;
var host = container.Hostname;
var port = container.GetMappedPublicPort(RedisPort);
var endpoint = $"{host}:{port}";

// create the connection multiplexer and register it in DI
using var mux = await ConnectionMultiplexer.ConnectAsync(endpoint);
_connectionMultiplexer = await ConnectionMultiplexer.ConnectAsync(endpoint);

var services = new ServiceCollection();
services.AddLogging();
services.AddDistributedLockManager();
services.AddSingleton<IConnectionMultiplexer>(mux);
services.AddSingleton<IConnectionMultiplexer>(_connectionMultiplexer);

var provider = services.BuildServiceProvider();

Expand All @@ -53,6 +59,9 @@ public async Task OneTimeSetUp()
[OneTimeTearDown]
public async Task OneTimeTearDown()
{
scope?.Dispose();
_connectionMultiplexer?.Dispose();
Comment on lines +62 to +63

Copilot AI Jan 21, 2026

Copy link

Choose a reason for hiding this comment

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

The service provider should be disposed before disposing the ConnectionMultiplexer to ensure proper cleanup order. The scope's service provider may still hold references to services that depend on the ConnectionMultiplexer. Consider storing the ServiceProvider and disposing it explicitly before disposing the ConnectionMultiplexer to prevent potential resource cleanup issues.

Copilot uses AI. Check for mistakes.

if (_redisContainer is not null)
await _redisContainer.StopAsync();
}
Expand Down
Loading