Implement P6: cache system with FusionCache integration#4
Conversation
Add CacheHandler DelegatingHandler with FusionCache-backed caching for GET/HEAD requests. Includes ETag/If-None-Match revalidation, per-request cache duration overrides, user-isolated cache keys, mutation invalidation, SWR via FactorySoftTimeout, and fail-safe for stale-if-error scenarios. Source files: - Cache/CacheKeyBuilder: shared, user-isolated, and no-cache key strategies - Cache/CachedResponse: serializable record for cached HTTP responses - Cache/IAcdcCacheManager + AcdcCacheManager: cache clearing by URL/user - Configuration/AcdcCacheOptions: duration, fail-safe, SWR, ETag settings - Handlers/CacheHandler: DelegatingHandler with FusionCache GetOrSetAsync Test files (42 tests): - CacheKeyBuilderTests: key format for all strategies - CacheHandlerTests: hit/miss, metadata headers, skip-cache, user isolation - CacheHandlerETagTests: ETag storage, If-None-Match, 304 resolution - CacheHandlerSwrTests: SWR timeout, stale-if-error, fail-safe Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a FusionCache-backed HTTP caching layer to CSharpAcdc, including cache key strategies, ETag revalidation, stale-while-revalidate / fail-safe behavior, and programmatic invalidation support.
Changes:
- Introduce
CacheHandlerdelegating handler to cache GET/HEAD responses and support ETag-based 304 revalidation. - Add cache key strategy helper + cached response model, plus
AcdcCacheManager/IAcdcCacheManagerfor cache clearing/invalidation. - Add a comprehensive new test suite covering cache hits/misses, key strategies, invalidation, ETag behavior, and SWR/fail-safe.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 46 comments.
Show a summary per file
| File | Description |
|---|---|
| src/CSharpAcdc/Handlers/CacheHandler.cs | Core caching handler with FusionCache integration, ETag revalidation, and mutation invalidation logic. |
| src/CSharpAcdc/Configuration/AcdcCacheOptions.cs | Options record controlling TTL, SWR soft timeout, fail-safe window, key strategy, and ETag enablement. |
| src/CSharpAcdc/Cache/CacheKeyBuilder.cs | Cache key building logic supporting shared/user-isolated/no-cache strategies. |
| src/CSharpAcdc/Cache/CachedResponse.cs | Serializable response shape for storing cached HTTP responses. |
| src/CSharpAcdc/Cache/IAcdcCacheManager.cs | Public cache manager interface for clearing cache by scope. |
| src/CSharpAcdc/Cache/AcdcCacheManager.cs | Tracks cache keys per base URL and removes cache entries by URL/user/all. |
| tests/CSharpAcdc.Tests/Handlers/CacheHandlerTests.cs | Tests for basic caching, headers, bypass flags, user isolation, invalidation, and HEAD caching. |
| tests/CSharpAcdc.Tests/Handlers/CacheHandlerETagTests.cs | Tests for ETag storage, If-None-Match, and 304 cached resolution. |
| tests/CSharpAcdc.Tests/Handlers/CacheHandlerSwrTests.cs | Tests for SWR soft timeout behavior and fail-safe stale responses. |
| tests/CSharpAcdc.Tests/Cache/CacheKeyBuilderTests.cs | Tests for cache key formats across strategies and methods. |
| tests/CSharpAcdc.Tests/Cache/AcdcCacheManagerTests.cs | Tests for clearing/invalidation behavior by URL/user/all. |
| tests/CSharpAcdc.Tests/CSharpAcdc.Tests.csproj | Adds required test dependencies (FusionCache, logging abstractions, options). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| catch (Exception ex) when (ex is not AcdcCacheException) | ||
| { | ||
| _logger.LogWarning(ex, "Cache operation failed for key {CacheKey}, falling through to downstream", cacheKey); | ||
| var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); | ||
| return response; | ||
| } |
There was a problem hiding this comment.
The catch (Exception ex) when (ex is not AcdcCacheException) fallback calls base.SendAsync again. If the exception originated from the downstream call inside the FusionCache factory (e.g., HttpRequestException), this results in an unintended implicit retry / duplicate request. Consider only falling back when the failure is cache-layer related, and rethrow downstream exceptions instead of re-sending the request.
| // Add If-None-Match header if we have a stored ETag | ||
| if (_options.ETagEnabled && storedETag is not null) | ||
| { | ||
| request.Headers.IfNoneMatch.Clear(); | ||
| request.Headers.IfNoneMatch.Add( | ||
| new System.Net.Http.Headers.EntityTagHeaderValue($"\"{storedETag}\"")); | ||
| } |
There was a problem hiding this comment.
The FusionCache factory captures and mutates the caller’s HttpRequestMessage (clearing/setting If-None-Match). With AllowTimedOutFactoryBackgroundCompletion enabled, the factory may continue after the original call returns, so reusing the same request instance can race with caller disposal/mutation and can also leak headers into subsequent uses. Consider cloning the request message for the downstream send (and avoid overwriting caller-specified If-None-Match).
| private async Task InvalidateRelatedCacheEntriesAsync( | ||
| HttpRequestMessage request, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| if (_cacheManager is null) | ||
| return; | ||
|
|
||
| var baseUrl = GetBaseUrl(request.RequestUri); | ||
|
|
||
| try | ||
| { | ||
| await _cacheManager.InvalidateForBaseUrlAsync(baseUrl, cancellationToken).ConfigureAwait(false); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogWarning(ex, "Failed to invalidate cache entries for base URL {BaseUrl}", baseUrl); | ||
| } | ||
| } |
There was a problem hiding this comment.
Cache invalidation only removes FusionCache entries via _cacheManager, but _etagStore is never cleared. After a mutation invalidates a URL, a subsequent GET can still send an old ETag and, on a 304, return lastKnownResponse even though the cache was invalidated. Clear the corresponding _etagStore entry when invalidating (or store ETag/lastKnownResponse in the cache with the same invalidation path).
| private readonly Func<HttpRequestMessage, string?>? _userIdProvider; | ||
| private readonly AcdcCacheManager? _cacheManager; | ||
|
|
||
| // ETags and last responses survive cache expiration for 304 revalidation | ||
| private readonly ConcurrentDictionary<string, (string ETag, CachedResponse Response)> _etagStore = new(); | ||
|
|
||
| private static readonly HttpRequestOptionsKey<string> CacheSourceKey = new("acdc_source"); | ||
|
|
||
| public CacheHandler( | ||
| IFusionCache cache, | ||
| IOptions<AcdcCacheOptions> options, | ||
| ILogger<CacheHandler> logger, | ||
| Func<HttpRequestMessage, string?>? userIdProvider = null, | ||
| AcdcCacheManager? cacheManager = null) | ||
| { |
There was a problem hiding this comment.
CacheHandler depends on the concrete AcdcCacheManager type even though an IAcdcCacheManager interface exists. This makes it harder to mock/extend and also hides InvalidateForBaseUrlAsync (not on the interface). Consider updating IAcdcCacheManager to include the invalidation method and accept the interface here instead of the concrete class.
| { | ||
| Task ClearCacheAsync(CancellationToken ct = default); | ||
| Task ClearCacheForUrlAsync(string url, CancellationToken ct = default); | ||
| Task ClearCacheForUserAsync(string userId, CancellationToken ct = default); |
There was a problem hiding this comment.
IAcdcCacheManager is missing the InvalidateForBaseUrlAsync method that CacheHandler relies on, forcing the handler to take a concrete AcdcCacheManager. Add InvalidateForBaseUrlAsync (or rename/align methods) so the handler can depend on the interface.
| Task ClearCacheForUserAsync(string userId, CancellationToken ct = default); | |
| Task ClearCacheForUserAsync(string userId, CancellationToken ct = default); | |
| Task InvalidateForBaseUrlAsync(string baseUrl, CancellationToken ct = default); |
| return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) | ||
| { | ||
| Content = new StringContent($"data-for-call-{callCount}"), | ||
| }); |
There was a problem hiding this comment.
Disposable 'HttpResponseMessage' is created but not disposed.
| return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) | ||
| { | ||
| Content = new StringContent("user-data"), | ||
| }); |
There was a problem hiding this comment.
Disposable 'HttpResponseMessage' is created but not disposed.
| return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) | ||
| { | ||
| Content = new StringContent($"response-{callCount}"), | ||
| }); |
There was a problem hiding this comment.
Disposable 'HttpResponseMessage' is created but not disposed.
| return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) | ||
| { | ||
| Content = new StringContent("data"), | ||
| }); |
There was a problem hiding this comment.
Disposable 'HttpResponseMessage' is created but not disposed.
| var (_, client) = CreatePipeline((_, _) => | ||
| { | ||
| Interlocked.Increment(ref callCount); | ||
| return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK)); |
There was a problem hiding this comment.
Disposable 'HttpResponseMessage' is created but not disposed.
1 participant
Summary
CacheHandlerDelegatingHandler with FusionCache-backed caching for GET/HEAD requestsFactorySoftTimeoutand fail-safe viaFailSafeMaxDurationCacheKeyBuilderAcdcCacheManagerfor programmatic cache clearing by URL or userAcdcRequestOptions.CacheMaxAgeFiles added
Source (6 files):
src/CSharpAcdc/Cache/CacheKeyBuilder.cs- Cache key strategies (Shared, UserIsolated, NoCache)src/CSharpAcdc/Cache/CachedResponse.cs- Serializable record for cached HTTP responsessrc/CSharpAcdc/Cache/IAcdcCacheManager.cs- Interface for cache managementsrc/CSharpAcdc/Cache/AcdcCacheManager.cs- Cache clearing by URL, user, or all entriessrc/CSharpAcdc/Configuration/AcdcCacheOptions.cs- Configuration record (duration, fail-safe, SWR, ETag)src/CSharpAcdc/Handlers/CacheHandler.cs- Core DelegatingHandler with FusionCache integrationTests (5 files, 42 tests):
tests/CSharpAcdc.Tests/Cache/CacheKeyBuilderTests.cs- Key format for all strategiestests/CSharpAcdc.Tests/Cache/AcdcCacheManagerTests.cs- Clear all, by URL, by usertests/CSharpAcdc.Tests/Handlers/CacheHandlerTests.cs- Cache hit/miss, headers, skip, user isolation, invalidationtests/CSharpAcdc.Tests/Handlers/CacheHandlerETagTests.cs- ETag storage, If-None-Match, 304 resolutiontests/CSharpAcdc.Tests/Handlers/CacheHandlerSwrTests.cs- SWR timeout, stale-if-error, fail-safeTest plan
dotnet buildpasses with zero warnings (TreatWarningsAsErrors enabled)🤖 Generated with Claude Code