Skip to content

Commit 9a3a1d5

Browse files
test: DRY up ResponseIdValidationServiceTests (#1078)
## What changed Refactored `ResponseIdValidationServiceTests` to eliminate repetitive setup boilerplate across all 9 tests. **Before:** Each test created its own `MemoryCache` and `ResponseIdValidationService` via two static factory helpers, resulting in a 2-line setup block in every test body. **After:** A single `_service` field is initialized once using the production parameterless constructor (which already creates a bounded `MemoryCache` with `SizeLimit = 10_000`). Cleanup is handled via `[After(Test)]` (TUnit-idiomatic) plus `IDisposable`/`GC.SuppressFinalize` to satisfy CA1001/CA1816 analyzers. ## Why - Removes ~30 lines of repeated setup boilerplate - Drops the `using Microsoft.Extensions.Caching.Memory` import entirely - tests no longer need to know cache configuration details - Uses the production constructor, so the `SizeLimit` is inherently exercised without duplication - All 11 tests continue to pass
1 parent 0d2cb75 commit 9a3a1d5

2 files changed

Lines changed: 42 additions & 53 deletions

File tree

Lines changed: 33 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,24 @@
11
using EssentialCSharp.Web.Services;
2-
using Microsoft.Extensions.Caching.Memory;
32

43
namespace EssentialCSharp.Web.Tests;
54

6-
public class ResponseIdValidationServiceTests
5+
public class ResponseIdValidationServiceTests : IDisposable
76
{
87
// Match production SizeLimit so SetSize(1) is exercised in tests, not silently ignored.
9-
private static MemoryCache CreateCache() => new(new MemoryCacheOptions { SizeLimit = 10_000 });
8+
private readonly ResponseIdValidationService _service = new();
109

11-
private static ResponseIdValidationService CreateService(MemoryCache cache) => new(cache);
10+
public void Dispose()
11+
{
12+
_service.Dispose();
13+
GC.SuppressFinalize(this);
14+
}
1215

1316
[Test]
1417
[Arguments(null)]
1518
[Arguments("")]
1619
public async Task ValidateResponseId_BlankResponseId_AllowsNewConversation(string? responseId)
1720
{
18-
using var cache = CreateCache();
19-
var service = CreateService(cache);
20-
21-
bool result = service.ValidateResponseId("user1", responseId);
21+
bool result = _service.ValidateResponseId("user1", responseId);
2222

2323
await Assert.That(result).IsTrue();
2424
}
@@ -28,103 +28,83 @@ public async Task ValidateResponseId_BlankResponseId_AllowsNewConversation(strin
2828
[Arguments("")]
2929
public async Task ValidateResponseId_BlankUserId_Rejects(string? userId)
3030
{
31-
using var cache = CreateCache();
32-
var service = CreateService(cache);
33-
34-
bool result = service.ValidateResponseId(userId, "resp_123");
31+
bool result = _service.ValidateResponseId(userId, "resp_123");
3532

3633
await Assert.That(result).IsFalse();
3734
}
3835

3936
[Test]
4037
public async Task ValidateResponseId_CacheMiss_AllowsGracefulDegradation()
4138
{
42-
using var cache = CreateCache();
43-
var service = CreateService(cache);
4439
// No RecordResponseId call — simulate server restart / different instance
45-
46-
bool result = service.ValidateResponseId("user1", "resp_unknown");
40+
bool result = _service.ValidateResponseId("user1", "resp_unknown");
4741

4842
await Assert.That(result).IsTrue();
4943
}
5044

5145
[Test]
5246
public async Task ValidateResponseId_RecordedByOwner_Validates()
5347
{
54-
using var cache = CreateCache();
55-
var service = CreateService(cache);
56-
service.RecordResponseId("user1", "resp_abc");
48+
_service.RecordResponseId("user1", "resp_abc");
5749

58-
bool result = service.ValidateResponseId("user1", "resp_abc");
50+
bool result = _service.ValidateResponseId("user1", "resp_abc");
5951

6052
await Assert.That(result).IsTrue();
6153
}
6254

6355
[Test]
6456
public async Task ValidateResponseId_RecordedByDifferentUser_Rejects()
6557
{
66-
using var cache = CreateCache();
67-
var service = CreateService(cache);
68-
service.RecordResponseId("user1", "resp_abc");
58+
_service.RecordResponseId("user1", "resp_abc");
6959

70-
bool result = service.ValidateResponseId("user2", "resp_abc");
60+
bool result = _service.ValidateResponseId("user2", "resp_abc");
7161

7262
await Assert.That(result).IsFalse();
7363
}
7464

7565
[Test]
7666
public async Task RecordResponseId_NullInputs_DoesNotThrow()
7767
{
78-
using var cache = CreateCache();
79-
var service = CreateService(cache);
80-
81-
service.RecordResponseId(null, "resp_abc");
82-
service.RecordResponseId("user1", null);
83-
service.RecordResponseId(null, null);
68+
_service.RecordResponseId(null, "resp_abc");
69+
_service.RecordResponseId("user1", null);
70+
_service.RecordResponseId(null, null);
8471

8572
// Verify the service is still functional after no-op calls
86-
service.RecordResponseId("user1", "resp_abc");
87-
await Assert.That(service.ValidateResponseId("user1", "resp_abc")).IsTrue();
73+
_service.RecordResponseId("user1", "resp_abc");
74+
await Assert.That(_service.ValidateResponseId("user1", "resp_abc")).IsTrue();
8875
}
8976

9077
[Test]
9178
public async Task ValidateResponseId_MultipleResponseIds_EachValidatedIndependently()
9279
{
93-
using var cache = CreateCache();
94-
var service = CreateService(cache);
95-
service.RecordResponseId("user1", "resp_001");
96-
service.RecordResponseId("user1", "resp_002");
80+
_service.RecordResponseId("user1", "resp_001");
81+
_service.RecordResponseId("user1", "resp_002");
9782

98-
await Assert.That(service.ValidateResponseId("user1", "resp_001")).IsTrue();
99-
await Assert.That(service.ValidateResponseId("user1", "resp_002")).IsTrue();
83+
await Assert.That(_service.ValidateResponseId("user1", "resp_001")).IsTrue();
84+
await Assert.That(_service.ValidateResponseId("user1", "resp_002")).IsTrue();
10085
// Unrecorded ID for same user → cache miss → allow
101-
await Assert.That(service.ValidateResponseId("user1", "resp_003")).IsTrue();
86+
await Assert.That(_service.ValidateResponseId("user1", "resp_003")).IsTrue();
10287
}
10388

10489
[Test]
10590
public async Task ValidateResponseId_TwoUsers_IsolatedFromEachOther()
10691
{
107-
using var cache = CreateCache();
108-
var service = CreateService(cache);
109-
service.RecordResponseId("user1", "resp_A");
110-
service.RecordResponseId("user2", "resp_B");
111-
112-
await Assert.That(service.ValidateResponseId("user1", "resp_A")).IsTrue();
113-
await Assert.That(service.ValidateResponseId("user2", "resp_B")).IsTrue();
114-
await Assert.That(service.ValidateResponseId("user2", "resp_A")).IsFalse();
115-
await Assert.That(service.ValidateResponseId("user1", "resp_B")).IsFalse();
92+
_service.RecordResponseId("user1", "resp_A");
93+
_service.RecordResponseId("user2", "resp_B");
94+
95+
await Assert.That(_service.ValidateResponseId("user1", "resp_A")).IsTrue();
96+
await Assert.That(_service.ValidateResponseId("user2", "resp_B")).IsTrue();
97+
await Assert.That(_service.ValidateResponseId("user2", "resp_A")).IsFalse();
98+
await Assert.That(_service.ValidateResponseId("user1", "resp_B")).IsFalse();
11699
}
117100

118101
[Test]
119102
public async Task RecordResponseId_SizeLimitEnforced_EntryCountedInCache()
120103
{
121-
using var cache = CreateCache();
122-
var service = CreateService(cache);
123-
124104
// Record an entry — with SizeLimit set, SetSize(1) should count toward the cache size.
125-
service.RecordResponseId("user1", "resp_size_test");
105+
_service.RecordResponseId("user1", "resp_size_test");
126106

127107
// Verify it was recorded (i.e., not silently evicted due to misconfiguration).
128-
await Assert.That(service.ValidateResponseId("user1", "resp_size_test")).IsTrue();
108+
await Assert.That(_service.ValidateResponseId("user1", "resp_size_test")).IsTrue();
129109
}
130110
}

EssentialCSharp.Web/Services/ResponseIdValidationService.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public sealed class ResponseIdValidationService : IDisposable
3131

3232
private readonly IMemoryCache _cache;
3333
private readonly bool _ownsCache;
34+
private bool _disposed;
3435

3536
/// <summary>
3637
/// Production constructor. Creates and owns a dedicated <see cref="MemoryCache"/> with a bounded
@@ -54,8 +55,16 @@ private ResponseIdValidationService(IMemoryCache cache, bool ownsCache)
5455
/// <inheritdoc />
5556
public void Dispose()
5657
{
58+
if (_disposed)
59+
{
60+
return;
61+
}
62+
_disposed = true;
63+
5764
if (_ownsCache && _cache is IDisposable disposable)
65+
{
5866
disposable.Dispose();
67+
}
5968
}
6069

6170
/// <summary>

0 commit comments

Comments
 (0)