Skip to content

Commit f5ad578

Browse files
fix: ResponseIdValidationService owns its IMemoryCache; remove shared SizeLimit; remove dead cleanupOldSessions JS
- ResponseIdValidationService now creates and owns a dedicated MemoryCache instance (parameterless DI ctor) rather than injecting the shared IMemoryCache. This prevents the SizeLimit constraint from forcing every other IMemoryCache caller in the app to call SetSize, which would cause runtime InvalidOperationExceptions in ASP.NET Core's built-in caches. - An IMemoryCache overload ctor is retained for unit test injection. - Remove SizeLimit from AddMemoryCache() in Program.cs; the dedicated cache in ResponseIdValidationService already carries SizeLimit=10_000. - Remove cleanupOldSessions() from chat-module.js: loadChatHistory() already enforces the 2-hour TTL on load, making the 7-day cleanup function unreachable dead code.
1 parent 9a50e7b commit f5ad578

3 files changed

Lines changed: 48 additions & 33 deletions

File tree

EssentialCSharp.Web/Program.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ private static void Main(string[] args)
259259
// MCP server — always enabled, authenticated via opaque DB-backed tokens.
260260
builder.Services.AddScoped<McpApiTokenService>();
261261

262-
builder.Services.AddMemoryCache(options => options.SizeLimit = 10_000);
262+
builder.Services.AddMemoryCache();
263263
builder.Services.AddSingleton<ResponseIdValidationService>();
264264

265265
builder.Services.AddAuthentication()

EssentialCSharp.Web/Services/ResponseIdValidationService.cs

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,53 @@ namespace EssentialCSharp.Web.Services;
1111
/// matching the OpenAI Responses API conversation window. This eliminates arbitrary eviction (no
1212
/// per-user <see cref="HashSet{T}"/> cap needed) and gives each ID its own natural TTL.
1313
///
14-
/// <para><b>Cache-miss = allow</b> (graceful degradation): in a single-instance deployment a cache
15-
/// miss occurs only after a server restart. In a multi-instance deployment the cache is in-process only,
16-
/// so a request routed to a different instance will always miss. In both scenarios the user is allowed
17-
/// to continue rather than being locked out. This is an accepted trade-off for a soft control on an
14+
/// <para>This service creates and owns a dedicated <see cref="MemoryCache"/> with a bounded
15+
/// <c>SizeLimit</c>, rather than using the shared <c>IMemoryCache</c>. This prevents the
16+
/// <c>SizeLimit</c> from applying globally (which would require every other cache user in the app to
17+
/// call <c>SetSize</c>) while still bounding memory use for this service.</para>
18+
///
19+
/// <para><b>Cache-miss = allow</b> (graceful degradation): a cache miss can occur after a server
20+
/// restart, when a sliding-expiration entry times out (2 h of inactivity), under memory pressure
21+
/// (OS or GC-triggered eviction), or when the configured <c>SizeLimit</c> forces eviction of the
22+
/// least-recently-used entries. In a multi-instance deployment the in-process cache is not shared, so
23+
/// requests routed to a different instance always miss. In all these scenarios the user is allowed to
24+
/// continue rather than being locked out. This is an accepted trade-off for a soft control on an
1825
/// already-authenticated, rate-limited endpoint. Upgrade to <c>IDistributedCache</c> (e.g., Redis) to
19-
/// close this gap in multi-instance deployments.</para>
26+
/// close this gap in multi-instance or high-eviction deployments.</para>
2027
/// </remarks>
21-
public sealed class ResponseIdValidationService(IMemoryCache cache)
28+
public sealed class ResponseIdValidationService : IDisposable
2229
{
30+
private const int CacheSizeLimit = 10_000;
31+
32+
private readonly IMemoryCache _cache;
33+
private readonly bool _ownsCache;
34+
35+
/// <summary>
36+
/// Production constructor. Creates and owns a dedicated <see cref="MemoryCache"/> with a bounded
37+
/// <see cref="MemoryCacheOptions.SizeLimit"/> so the app-wide shared <c>IMemoryCache</c> is unaffected.
38+
/// </summary>
39+
public ResponseIdValidationService()
40+
: this(new MemoryCache(new MemoryCacheOptions { SizeLimit = CacheSizeLimit }), ownsCache: true) { }
41+
42+
/// <summary>
43+
/// Testing constructor. The caller owns and disposes the supplied cache.
44+
/// </summary>
45+
public ResponseIdValidationService(IMemoryCache cache)
46+
: this(cache, ownsCache: false) { }
47+
48+
private ResponseIdValidationService(IMemoryCache cache, bool ownsCache)
49+
{
50+
_cache = cache;
51+
_ownsCache = ownsCache;
52+
}
53+
54+
/// <inheritdoc />
55+
public void Dispose()
56+
{
57+
if (_ownsCache && _cache is IDisposable disposable)
58+
disposable.Dispose();
59+
}
60+
2361
/// <summary>
2462
/// Records a newly issued response ID as belonging to the specified user.
2563
/// </summary>
@@ -33,11 +71,11 @@ public void RecordResponseId(string? userId, string? responseId)
3371
// Create a fresh options instance per call — MemoryCacheEntryOptions has mutable
3472
// list properties (ExpirationTokens, PostEvictionCallbacks) and sharing a static
3573
// instance would cause future additions to affect all entries.
36-
// Size = 1 unit per entry is required when AddMemoryCache sets a SizeLimit.
74+
// SetSize(1) accounts for this entry against the dedicated cache's SizeLimit.
3775
var entryOptions = new MemoryCacheEntryOptions()
3876
.SetSlidingExpiration(TimeSpan.FromHours(2))
3977
.SetSize(1);
40-
cache.Set(ResponseKey(responseId), userId, entryOptions);
78+
_cache.Set(ResponseKey(responseId), userId, entryOptions);
4179
}
4280

4381
/// <summary>
@@ -60,7 +98,7 @@ public bool ValidateResponseId(string? userId, string? responseId)
6098
return false;
6199
}
62100

63-
if (!cache.TryGetValue(ResponseKey(responseId), out string? ownerUserId))
101+
if (!_cache.TryGetValue(ResponseKey(responseId), out string? ownerUserId))
64102
{
65103
return true; // Cache miss — allow (graceful degradation; see class remarks)
66104
}

EssentialCSharp.Web/wwwroot/js/chat-module.js

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -379,29 +379,6 @@ export function useChatWidget() {
379379
}
380380
}
381381

382-
// Clean up old chat sessions (keep only last 7 days)
383-
function cleanupOldSessions() {
384-
try {
385-
const saved = localStorage.getItem('aiChatHistory');
386-
if (saved) {
387-
const data = JSON.parse(saved);
388-
const sevenDaysAgo = Date.now() - (7 * 24 * 60 * 60 * 1000);
389-
390-
if (data.timestamp && data.timestamp < sevenDaysAgo) {
391-
localStorage.removeItem('aiChatHistory');
392-
sessionStorage.removeItem('aiChatLastResponseId');
393-
chatMessages.value = [];
394-
lastResponseId.value = null;
395-
}
396-
}
397-
} catch (error) {
398-
console.warn('Failed to cleanup old sessions:', error);
399-
}
400-
}
401-
402-
// Run cleanup on initialization
403-
cleanupOldSessions();
404-
405382
return {
406383
// State
407384
isAuthenticated,

0 commit comments

Comments
 (0)