Skip to content

Commit bf49011

Browse files
committed
feat: add response compression, health checks, traceId in errors, tag-based cache invalidation, per-version OpenAPI docs, and code quality improvements
- Add Brotli + Gzip response compression middleware - Add Redis and Hangfire health checks on /health/ready - Surface traceId and correlationId in ProblemDetails error responses - Add tag-based cache invalidation (SetItemAsync with tags, RemoveByTagAsync) to ICacheService - Generate per-version OpenAPI documents (configurable via OpenApiOptions.Versions) - Add AsNoTracking to read-only EF queries in Group handlers and UpdateGroupCommandHandler - Replace read-then-delete with ExecuteDeleteAsync in session cleanup - Add explanatory comments to all intentional broad catch blocks across caching, eventing, jobs, auditing, identity, and multitenancy - Fix MultitenancyOptionsTests to match actual default values
1 parent 83228df commit bf49011

File tree

24 files changed

+280
-46
lines changed

24 files changed

+280
-46
lines changed

src/BuildingBlocks/Caching/DistributedCacheService.cs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ public DistributedCacheService(
4747
if (bytes is null || bytes.Length == 0) return default;
4848
return JsonSerializer.Deserialize<T>(Utf8.GetString(bytes), JsonOpts);
4949
}
50+
// Graceful degradation: cache failures must not crash the caller — return default and log
5051
catch (Exception ex) when (ex is not OperationCanceledException)
5152
{
5253
_logger.LogWarning(ex, "Cache get failed for key (length={KeyLength})", key.Length);
@@ -64,16 +65,34 @@ public async Task SetItemAsync<T>(string key, T value, TimeSpan? sliding = defau
6465
await _cache.SetAsync(key, bytes, BuildEntryOptions(sliding), ct).ConfigureAwait(false);
6566
LogCached(key);
6667
}
68+
// Graceful degradation: cache failures must not crash the caller
6769
catch (Exception ex) when (ex is not OperationCanceledException)
6870
{
6971
_logger.LogWarning(ex, "Cache set failed for key (length={KeyLength})", key.Length);
7072
}
7173
}
7274

75+
/// <inheritdoc />
76+
public async Task SetItemAsync<T>(string key, T value, IReadOnlyList<string> tags, TimeSpan? sliding = default, CancellationToken ct = default)
77+
{
78+
ArgumentNullException.ThrowIfNull(tags);
79+
await SetItemAsync(key, value, sliding, ct).ConfigureAwait(false);
80+
81+
// Store the key in each tag's set so we can invalidate by tag later
82+
foreach (var tag in tags)
83+
{
84+
var tagKey = NormalizeTagKey(tag);
85+
var existing = await GetItemAsync<HashSet<string>>(tagKey, ct).ConfigureAwait(false) ?? [];
86+
existing.Add(Normalize(key));
87+
await SetItemAsync(tagKey, existing, sliding, ct).ConfigureAwait(false);
88+
}
89+
}
90+
7391
/// <inheritdoc />
7492
public async Task RemoveItemAsync(string key, CancellationToken ct = default)
7593
{
7694
key = Normalize(key);
95+
// Graceful degradation: cache failures must not crash the caller
7796
try { await _cache.RemoveAsync(key, ct).ConfigureAwait(false); }
7897
catch (Exception ex) when (ex is not OperationCanceledException)
7998
{ _logger.LogWarning(ex, "Cache remove failed for {Key}", key); }
@@ -88,10 +107,36 @@ public async Task RefreshItemAsync(string key, CancellationToken ct = default)
88107
await _cache.RefreshAsync(key, ct).ConfigureAwait(false);
89108
LogRefreshed(key);
90109
}
110+
// Graceful degradation: cache operations must not crash the caller
91111
catch (Exception ex) when (ex is not OperationCanceledException)
92112
{ _logger.LogWarning(ex, "Cache refresh failed for {Key}", key); }
93113
}
94114

115+
/// <inheritdoc />
116+
public async Task RemoveByTagAsync(string tag, CancellationToken ct = default)
117+
{
118+
var tagKey = NormalizeTagKey(tag);
119+
var keys = await GetItemAsync<HashSet<string>>(tagKey, ct).ConfigureAwait(false);
120+
if (keys is null || keys.Count == 0) return;
121+
122+
foreach (var key in keys)
123+
{
124+
try
125+
{
126+
await _cache.RemoveAsync(key, ct).ConfigureAwait(false);
127+
}
128+
// Graceful degradation: best-effort removal per key
129+
catch (Exception ex) when (ex is not OperationCanceledException)
130+
{
131+
_logger.LogWarning(ex, "Cache remove by tag failed for {Key}", key);
132+
}
133+
}
134+
135+
await RemoveItemAsync(tagKey, ct).ConfigureAwait(false);
136+
}
137+
138+
private string NormalizeTagKey(string tag) => Normalize($"__tag:{tag}");
139+
95140
/// <summary>
96141
/// Builds cache entry options with configured expiration settings.
97142
/// </summary>

src/BuildingBlocks/Caching/HybridCacheService.cs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ public HybridCacheService(
8080

8181
return value;
8282
}
83+
// Graceful degradation: cache failures must not crash the caller — return default and log
8384
catch (Exception ex) when (ex is not OperationCanceledException)
8485
{
8586
_logger.LogWarning(ex, "Cache get failed for key (length={KeyLength})", key.Length);
@@ -105,12 +106,29 @@ public async Task SetItemAsync<T>(string key, T value, TimeSpan? sliding = defau
105106

106107
LogCachedBoth(key);
107108
}
109+
// Graceful degradation: cache failures must not crash the caller
108110
catch (Exception ex) when (ex is not OperationCanceledException)
109111
{
110112
_logger.LogWarning(ex, "Cache set failed for key (length={KeyLength})", key.Length);
111113
}
112114
}
113115

116+
/// <inheritdoc />
117+
public async Task SetItemAsync<T>(string key, T value, IReadOnlyList<string> tags, TimeSpan? sliding = default, CancellationToken ct = default)
118+
{
119+
ArgumentNullException.ThrowIfNull(tags);
120+
await SetItemAsync(key, value, sliding, ct).ConfigureAwait(false);
121+
122+
// Store the key in each tag's set so we can invalidate by tag later
123+
foreach (var tag in tags)
124+
{
125+
var tagKey = NormalizeTagKey(tag);
126+
var existing = await GetItemAsync<HashSet<string>>(tagKey, ct).ConfigureAwait(false) ?? [];
127+
existing.Add(Normalize(key));
128+
await SetItemAsync(tagKey, existing, sliding, ct).ConfigureAwait(false);
129+
}
130+
}
131+
114132
/// <inheritdoc />
115133
/// <remarks>
116134
/// Removes from both L1 memory cache and L2 distributed cache.
@@ -125,6 +143,7 @@ public async Task RemoveItemAsync(string key, CancellationToken ct = default)
125143
await _distributedCache.RemoveAsync(key, ct).ConfigureAwait(false);
126144
LogRemovedBoth(key);
127145
}
146+
// Graceful degradation: cache failures must not crash the caller
128147
catch (Exception ex) when (ex is not OperationCanceledException)
129148
{
130149
_logger.LogWarning(ex, "Cache remove failed for {Key}", key);
@@ -140,12 +159,40 @@ public async Task RefreshItemAsync(string key, CancellationToken ct = default)
140159
await _distributedCache.RefreshAsync(key, ct).ConfigureAwait(false);
141160
LogRefreshed(key);
142161
}
162+
// Graceful degradation: cache operations must not crash the caller
143163
catch (Exception ex) when (ex is not OperationCanceledException)
144164
{
145165
_logger.LogWarning(ex, "Cache refresh failed for {Key}", key);
146166
}
147167
}
148168

169+
/// <inheritdoc />
170+
public async Task RemoveByTagAsync(string tag, CancellationToken ct = default)
171+
{
172+
var tagKey = NormalizeTagKey(tag);
173+
var keys = await GetItemAsync<HashSet<string>>(tagKey, ct).ConfigureAwait(false);
174+
if (keys is null || keys.Count == 0) return;
175+
176+
foreach (var key in keys)
177+
{
178+
try
179+
{
180+
_memoryCache.Remove(key);
181+
await _distributedCache.RemoveAsync(key, ct).ConfigureAwait(false);
182+
}
183+
// Graceful degradation: best-effort removal per key
184+
catch (Exception ex) when (ex is not OperationCanceledException)
185+
{
186+
_logger.LogWarning(ex, "Cache remove by tag failed for {Key}", key);
187+
}
188+
}
189+
190+
_memoryCache.Remove(tagKey);
191+
await _distributedCache.RemoveAsync(tagKey, ct).ConfigureAwait(false);
192+
}
193+
194+
private string NormalizeTagKey(string tag) => Normalize($"__tag:{tag}");
195+
149196
/// <summary>
150197
/// Builds distributed cache entry options with configured expiration settings.
151198
/// </summary>

src/BuildingBlocks/Caching/ICacheService.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,17 @@ public interface ICacheService
2525
/// <param name="ct">Cancellation token for the operation.</param>
2626
Task SetItemAsync<T>(string key, T value, TimeSpan? sliding = default, CancellationToken ct = default);
2727

28+
/// <summary>
29+
/// Asynchronously stores an item in the cache with associated tags for group invalidation.
30+
/// </summary>
31+
/// <typeparam name="T">The type of the item to cache.</typeparam>
32+
/// <param name="key">The unique cache key.</param>
33+
/// <param name="value">The value to cache.</param>
34+
/// <param name="tags">Tags to associate with this cache entry for group invalidation.</param>
35+
/// <param name="sliding">Optional sliding expiration. Uses default if not specified.</param>
36+
/// <param name="ct">Cancellation token for the operation.</param>
37+
Task SetItemAsync<T>(string key, T value, IReadOnlyList<string> tags, TimeSpan? sliding = default, CancellationToken ct = default);
38+
2839
/// <summary>
2940
/// Asynchronously removes an item from the cache.
3041
/// </summary>
@@ -39,4 +50,10 @@ public interface ICacheService
3950
/// <param name="ct">Cancellation token for the operation.</param>
4051
Task RefreshItemAsync(string key, CancellationToken ct = default);
4152

53+
/// <summary>
54+
/// Asynchronously removes all cache entries associated with the specified tag.
55+
/// </summary>
56+
/// <param name="tag">The tag whose associated entries should be removed.</param>
57+
/// <param name="ct">Cancellation token for the operation.</param>
58+
Task RemoveByTagAsync(string tag, CancellationToken ct = default);
4259
}

src/BuildingBlocks/Eventing/RabbitMq/RabbitMqEventBus.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ private async Task DisposeConnectionAsync()
198198
await _channel.CloseAsync().ConfigureAwait(false);
199199
_channel.Dispose();
200200
}
201+
// Cleanup must not throw: connection may already be closed or faulted
201202
catch (Exception ex)
202203
{
203204
_logger.LogDebug(ex, "Error closing RabbitMQ channel during cleanup");
@@ -213,6 +214,7 @@ private async Task DisposeConnectionAsync()
213214
await _connection.CloseAsync().ConfigureAwait(false);
214215
_connection.Dispose();
215216
}
217+
// Cleanup must not throw: connection may already be closed or faulted
216218
catch (Exception ex)
217219
{
218220
_logger.LogDebug(ex, "Error closing RabbitMQ connection during cleanup");

src/BuildingBlocks/Jobs/HangfireStaleLockCleanupService.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
5050
_logger.LogWarning("Cleaned up {Count} stale Hangfire locks", deleted);
5151
}
5252
}
53+
// Best-effort cleanup: table may not exist yet on first startup, or DB may be temporarily unreachable
5354
catch (Exception ex) when (ex is not OperationCanceledException)
5455
{
5556
_logger.LogDebug(ex, "Could not cleanup stale Hangfire locks (table may not exist yet)");

src/BuildingBlocks/Web/Exceptions/GlobalExceptionHandler.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using FSH.Framework.Core.Exceptions;
1+
using System.Diagnostics;
2+
using FSH.Framework.Core.Exceptions;
23
using Microsoft.AspNetCore.Diagnostics;
34
using Microsoft.AspNetCore.Http;
45
using Microsoft.AspNetCore.Mvc;
@@ -62,6 +63,14 @@ public async ValueTask<bool> TryHandleAsync(HttpContext httpContext, Exception e
6263

6364
httpContext.Response.StatusCode = statusCode;
6465

66+
// Surface trace and correlation IDs so clients/support can correlate errors to traces
67+
var traceId = Activity.Current?.TraceId.ToString() ?? httpContext.TraceIdentifier;
68+
problemDetails.Extensions["traceId"] = traceId;
69+
70+
var correlationId = httpContext.Request.Headers["X-Correlation-ID"].FirstOrDefault()
71+
?? httpContext.TraceIdentifier;
72+
problemDetails.Extensions["correlationId"] = correlationId;
73+
6574
LogContext.PushProperty("exception_title", problemDetails.Title);
6675
LogContext.PushProperty("exception_detail", problemDetails.Detail);
6776
LogContext.PushProperty("exception_statusCode", problemDetails.Status);

src/BuildingBlocks/Web/Extensions.cs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@
1818
using FSH.Framework.Web.RateLimiting;
1919
using FSH.Framework.Web.Security;
2020
using FSH.Framework.Web.Versioning;
21-
using Mediator;
2221
using Microsoft.AspNetCore.Builder;
22+
using Microsoft.AspNetCore.ResponseCompression;
23+
using Microsoft.Extensions.Caching.Distributed;
2324
using Microsoft.Extensions.Configuration;
2425
using Microsoft.Extensions.DependencyInjection;
2526
using Microsoft.Extensions.Diagnostics.HealthChecks;
2627
using Microsoft.Extensions.Hosting;
28+
using Mediator;
2729

2830
namespace FSH.Framework.Web;
2931

@@ -38,6 +40,17 @@ public static IHostApplicationBuilder AddHeroPlatform(this IHostApplicationBuild
3840

3941
builder.Services.AddScoped<CurrentUserMiddleware>();
4042

43+
builder.Services.AddResponseCompression(options =>
44+
{
45+
options.EnableForHttps = true;
46+
options.Providers.Add<BrotliCompressionProvider>();
47+
options.Providers.Add<GzipCompressionProvider>();
48+
});
49+
builder.Services.Configure<BrotliCompressionProviderOptions>(options =>
50+
{
51+
options.Level = System.IO.Compression.CompressionLevel.Fastest;
52+
});
53+
4154
builder.AddHeroLogging();
4255
if (options.EnableOpenTelemetry)
4356
{
@@ -68,6 +81,7 @@ public static IHostApplicationBuilder AddHeroPlatform(this IHostApplicationBuild
6881
if (options.EnableJobs)
6982
{
7083
builder.Services.AddHeroJobs();
84+
builder.Services.AddHealthChecks().AddCheck<HangfireHealthCheck>("hangfire");
7185
}
7286

7387
if (options.EnableMailing)
@@ -78,6 +92,11 @@ public static IHostApplicationBuilder AddHeroPlatform(this IHostApplicationBuild
7892
if (options.EnableCaching)
7993
{
8094
builder.Services.AddHeroCaching(builder.Configuration);
95+
var cacheConfig = builder.Configuration.GetSection(nameof(CachingOptions)).Get<CachingOptions>();
96+
if (cacheConfig is not null && !string.IsNullOrEmpty(cacheConfig.Redis))
97+
{
98+
builder.Services.AddHealthChecks().AddCheck<RedisHealthCheck>("redis");
99+
}
81100
}
82101

83102
if (options.EnableFeatureFlags)
@@ -116,6 +135,7 @@ public static WebApplication UseHeroPlatform(this WebApplication app, Action<Fsh
116135
var openApiEnabled = options.UseOpenApi && IsOpenApiEnabled(app.Configuration);
117136

118137
app.UseExceptionHandler();
138+
app.UseResponseCompression();
119139
app.UseHttpsRedirection();
120140

121141
app.UseHeroSecurityHeaders();
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
using Hangfire;
2+
using Microsoft.Extensions.Diagnostics.HealthChecks;
3+
4+
namespace FSH.Framework.Web.Health;
5+
6+
/// <summary>
7+
/// Health check that verifies Hangfire storage is accessible.
8+
/// </summary>
9+
public sealed class HangfireHealthCheck : IHealthCheck
10+
{
11+
public Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default)
12+
{
13+
try
14+
{
15+
var storage = JobStorage.Current;
16+
using var connection = storage.GetConnection();
17+
return Task.FromResult(HealthCheckResult.Healthy("Hangfire storage is accessible."));
18+
}
19+
#pragma warning disable CA1031 // Health checks must catch all exceptions to report degraded/unhealthy
20+
catch (Exception ex)
21+
{
22+
return Task.FromResult(HealthCheckResult.Unhealthy("Hangfire storage is not accessible.", ex));
23+
}
24+
#pragma warning restore CA1031
25+
}
26+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
using Microsoft.Extensions.Caching.Distributed;
2+
using Microsoft.Extensions.Diagnostics.HealthChecks;
3+
4+
namespace FSH.Framework.Web.Health;
5+
6+
/// <summary>
7+
/// Health check that verifies Redis connectivity by performing a round-trip set/remove.
8+
/// </summary>
9+
public sealed class RedisHealthCheck : IHealthCheck
10+
{
11+
private readonly IDistributedCache _cache;
12+
13+
public RedisHealthCheck(IDistributedCache cache)
14+
{
15+
_cache = cache;
16+
}
17+
18+
public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default)
19+
{
20+
try
21+
{
22+
const string key = "__health_check__";
23+
var options = new DistributedCacheEntryOptions
24+
{
25+
AbsoluteExpirationRelativeToNow = TimeSpan.FromSeconds(5)
26+
};
27+
await _cache.SetStringAsync(key, "ok", options, cancellationToken).ConfigureAwait(false);
28+
await _cache.RemoveAsync(key, cancellationToken).ConfigureAwait(false);
29+
return HealthCheckResult.Healthy("Redis is accessible.");
30+
}
31+
#pragma warning disable CA1031 // Health checks must catch all exceptions to report degraded/unhealthy
32+
catch (Exception ex)
33+
{
34+
return HealthCheckResult.Unhealthy("Redis is not accessible.", ex);
35+
}
36+
#pragma warning restore CA1031
37+
}
38+
}

src/BuildingBlocks/Web/Idempotency/IdempotencyEndpointFilter.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ public sealed class IdempotencyEndpointFilter : IEndpointFilter
8585

8686
await cache.SetItemAsync(cacheKey, responseToCache, options.DefaultTtl, httpContext.RequestAborted).ConfigureAwait(false);
8787
}
88+
// Best-effort caching: idempotency replay is a convenience, not a correctness requirement
8889
catch (Exception ex) when (ex is not OperationCanceledException)
8990
{
9091
logger.LogWarning(ex, "Failed to cache idempotent response for key {KeyHash}", HashKey(idempotencyKey));

0 commit comments

Comments
 (0)