[Phelecks]

…figuration updates

[Copilot]

Pull request overview

This PR refactors the DistributedLockManager library to improve dependency injection patterns and resource management. The main enhancement is the introduction of IDistributedLockFactory as a central factory registered in the DI container, replacing the previous pattern where each service instance created its own RedLockFactory.

Changes:

• Centralized RedLockFactory creation in Configuration.cs with singleton registration and logger support
• Refactored DistributedLockService classes to use constructor injection with IDistributedLockFactory
• Fixed critical resource management bug in integration tests where ConnectionMultiplexer was incorrectly disposed immediately after creation

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
src/DistributedLockManager/Configuration.cs	Added singleton IDistributedLockFactory registration with RedLockFactory implementation, integrated ILoggerFactory support
src/DistributedLockManager/Services/DistributedLockService.cs	Refactored both service classes to use primary constructors with injected IDistributedLockFactory instead of creating RedLockFactory instances
tests/DistributedLockManager.Tests/DistributedLockServiceIntegrationTests.cs	Fixed ConnectionMultiplexer disposal issue, added wait strategy, logging support, and proper resource cleanup in teardown

Comments suppressed due to low confidence (1)

src/DistributedLockManager/Services/DistributedLockService.cs:16

• 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 function, 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.

    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 distributedLockFactory.CreateLockAsync(key, TimeSpan.FromSeconds(expiryInSecond), TimeSpan.FromSeconds(waitInSecond),
            TimeSpan.FromSeconds(retryInSecond), cancellationToken);

        if (redLock.IsAcquired)
            await func();
    }

[Copilot]

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]

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]

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]

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);