Skip to content

Commit e25511f

Browse files
committed
fix: resolve remaining antipatterns from Roslyn architecture audit
- Fix string concatenation in structured log calls (IdentityService) - Add AsNoTracking to read-only queries (GetUserGroupsQueryHandler) - Add explanatory comments to catch blocks (WebhookDeliveryService, GetTenantMigrationsQueryHandler) - Add ConfigureAwait to middleware delegate calls (AuditHttpMiddleware)
1 parent bf49011 commit e25511f

File tree

5 files changed

+9
-4
lines changed

5 files changed

+9
-4
lines changed

src/Modules/Auditing/Modules.Auditing/Infrastructure/Http/AuditHttpMiddleware.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public async Task InvokeAsync(HttpContext ctx)
2020

2121
if (ShouldSkip(ctx))
2222
{
23-
await _next(ctx);
23+
await _next(ctx).ConfigureAwait(false);
2424
return;
2525
}
2626

@@ -33,7 +33,7 @@ public async Task InvokeAsync(HttpContext ctx)
3333

3434
try
3535
{
36-
await _next(ctx);
36+
await _next(ctx).ConfigureAwait(false);
3737
sw.Stop();
3838

3939
await WriteSuccessAuditAsync(ctx, requestContext, responseBuffer, originalBody, sw);

src/Modules/Identity/Modules.Identity/Features/v1/Users/GetUserGroups/GetUserGroupsQueryHandler.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ public async ValueTask<IEnumerable<GroupDto>> Handle(GetUserGroupsQuery query, C
2020
{
2121
// Validate user exists
2222
var userExists = await _dbContext.Users
23+
.AsNoTracking()
2324
.AnyAsync(u => u.Id == query.UserId, cancellationToken);
2425

2526
if (!userExists)
@@ -47,6 +48,7 @@ public async ValueTask<IEnumerable<GroupDto>> Handle(GetUserGroupsQuery query, C
4748

4849
// Get member counts
4950
var memberCounts = await _dbContext.UserGroups
51+
.AsNoTracking()
5052
.Where(ug => groupIds.Contains(ug.GroupId))
5153
.GroupBy(ug => ug.GroupId)
5254
.Select(g => new { GroupId = g.Key, Count = g.Count() })

src/Modules/Identity/Modules.Identity/Services/IdentityService.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public async Task StoreRefreshTokenAsync(string subject, string refreshToken, Da
7878
{
7979
_logger.LogDebug(
8080
"Storing refresh token for user {UserId} in tenant {TenantId}. Token hash: {TokenHash}, Expires: {ExpiresAt}",
81-
subject, tenant.Id, hashedToken[..Math.Min(8, hashedToken.Length)] + "...", expiresAtUtc);
81+
subject, tenant.Id, hashedToken[..Math.Min(8, hashedToken.Length)], expiresAtUtc);
8282
}
8383

8484
var result = await _userManager.UpdateAsync(user);
@@ -122,7 +122,7 @@ private async Task<FshUser> FindUserByRefreshTokenAsync(string refreshToken, str
122122
{
123123
_logger.LogDebug(
124124
"Validating refresh token for tenant {TenantId}. Token hash: {TokenHash}",
125-
tenantId, hashedToken[..Math.Min(8, hashedToken.Length)] + "...");
125+
tenantId, hashedToken[..Math.Min(8, hashedToken.Length)]);
126126
}
127127

128128
var user = await _userManager.Users

src/Modules/Multitenancy/Modules.Multitenancy/Features/v1/GetTenantMigrations/GetTenantMigrationsQueryHandler.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ public async ValueTask<IReadOnlyCollection<TenantMigrationStatusDto>> Handle(
6363
tenantStatus.PendingMigrations = pendingMigrations.ToArray();
6464
tenantStatus.HasPendingMigrations = tenantStatus.PendingMigrations.Count > 0;
6565
}
66+
// Per-tenant failure must not stop reporting on other tenants
6667
catch (Exception ex)
6768
{
6869
tenantStatus.Error = ex.Message;

src/Modules/Webhooks/Modules.Webhooks/Services/WebhookDeliveryService.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ public async Task DeliverAsync(
4545
delivery.Id, url, (int)response.StatusCode);
4646
}
4747
}
48+
// Broad catch is intentional: delivery failures (DNS, timeout, HTTP errors) must be
49+
// recorded in the delivery log rather than crashing the caller.
4850
catch (Exception ex) when (ex is not OperationCanceledException)
4951
{
5052
delivery.RecordResult(0, false, ex.Message);

0 commit comments

Comments
 (0)