From 6a1efeeed3722b60dbe0ccaa9df912e016ba2cd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rasmus=20H=C3=B8llund=20Kristensen?= Date: Thu, 11 Dec 2025 19:07:11 +0100 Subject: [PATCH 1/2] Decided to add ILogger to all audit log failures and file deletion error places --- .../DeleteAccountCommandHandlerTests.cs | 9 +++++++-- .../EditUser/EditUserCommandHandlerTests.cs | 6 +++++- .../DeleteAccountCommandHandler.cs | 12 ++++++++---- .../EditUser/EditUserCommandHandler.cs | 19 ++++++++++--------- .../UploadAvatarCommandHandler.cs | 17 +++++++++++------ .../Services/LocalFileStorageService.cs | 9 ++++++--- 6 files changed, 47 insertions(+), 25 deletions(-) diff --git a/AutoStack.Application.Tests/Features/Users/Commands/DeleteAccount/DeleteAccountCommandHandlerTests.cs b/AutoStack.Application.Tests/Features/Users/Commands/DeleteAccount/DeleteAccountCommandHandlerTests.cs index 06e795d..57d7822 100644 --- a/AutoStack.Application.Tests/Features/Users/Commands/DeleteAccount/DeleteAccountCommandHandlerTests.cs +++ b/AutoStack.Application.Tests/Features/Users/Commands/DeleteAccount/DeleteAccountCommandHandlerTests.cs @@ -5,6 +5,7 @@ using AutoStack.Domain.Entities; using AutoStack.Domain.Enums; using FluentAssertions; +using Microsoft.Extensions.Logging; using Moq; using Xunit; @@ -13,13 +14,17 @@ namespace AutoStack.Application.Tests.Features.Users.Commands.DeleteAccount; public class DeleteAccountCommandHandlerTests : CommandHandlerTestBase { private readonly DeleteAccountCommandHandler _handler; + private readonly Mock> _mockLogger; public DeleteAccountCommandHandlerTests() { + _mockLogger = new Mock>(); + _handler = new DeleteAccountCommandHandler( MockUserRepository.Object, MockUnitOfWork.Object, - MockAuditLog.Object + MockAuditLog.Object, + _mockLogger.Object ); } @@ -71,7 +76,7 @@ public async Task Handle_WithExistingUserId_ShouldReturnSuccess() MockUnitOfWork.Verify(r => r.SaveChangesAsync(It.IsAny()), Times.Once); MockAuditLog.Verify(a => a.LogAsync( It.Is(req => - req.Level == LogLevel.Information && + req.Level == Domain.Enums.LogLevel.Information && req.Category == LogCategory.User && req.Message == "Account deleted" && req.UserIdOverride == user.Id && diff --git a/AutoStack.Application.Tests/Features/Users/Commands/EditUser/EditUserCommandHandlerTests.cs b/AutoStack.Application.Tests/Features/Users/Commands/EditUser/EditUserCommandHandlerTests.cs index dd4c7d8..9868034 100644 --- a/AutoStack.Application.Tests/Features/Users/Commands/EditUser/EditUserCommandHandlerTests.cs +++ b/AutoStack.Application.Tests/Features/Users/Commands/EditUser/EditUserCommandHandlerTests.cs @@ -6,6 +6,7 @@ using AutoStack.Application.Tests.Common; using AutoStack.Domain.Entities; using FluentAssertions; +using Microsoft.Extensions.Logging; using Moq; using Xunit; @@ -15,16 +16,19 @@ public class EditUserCommandHandlerTests : CommandHandlerTestBase { private readonly Mock _mockPasswordHasher; private readonly EditUserCommandHandler _handler; + private readonly Mock> _mockLogger; public EditUserCommandHandlerTests() { _mockPasswordHasher = new Mock(); + _mockLogger = new Mock>(); _handler = new EditUserCommandHandler( MockUserRepository.Object, MockUnitOfWork.Object, _mockPasswordHasher.Object, - MockAuditLog.Object + MockAuditLog.Object, + _mockLogger.Object ); } diff --git a/AutoStack.Application/Features/Users/Commands/DeleteAccount/DeleteAccountCommandHandler.cs b/AutoStack.Application/Features/Users/Commands/DeleteAccount/DeleteAccountCommandHandler.cs index 47afa07..7715eba 100644 --- a/AutoStack.Application/Features/Users/Commands/DeleteAccount/DeleteAccountCommandHandler.cs +++ b/AutoStack.Application/Features/Users/Commands/DeleteAccount/DeleteAccountCommandHandler.cs @@ -3,6 +3,7 @@ using AutoStack.Application.Common.Models; using AutoStack.Domain.Enums; using AutoStack.Domain.Repositories; +using Microsoft.Extensions.Logging; namespace AutoStack.Application.Features.Users.Commands.DeleteAccount; @@ -11,11 +12,14 @@ public class DeleteAccountCommandHandler : ICommandHandler _logger; + + public DeleteAccountCommandHandler(IUserRepository userRepository, IUnitOfWork unitOfWork, IAuditLogService auditLogService, ILogger logger) { _userRepository = userRepository; _unitOfWork = unitOfWork; _auditLogService = auditLogService; + _logger = logger; } public async Task> Handle(DeleteAccountCommand request, CancellationToken cancellationToken) @@ -41,16 +45,16 @@ public async Task> Handle(DeleteAccountCommand request, Cancellatio { await _auditLogService.LogAsync(new AuditLogRequest { - Level = LogLevel.Information, + Level = Domain.Enums.LogLevel.Information, Category = LogCategory.User, Message = "Account deleted", UserIdOverride = user.Id, UsernameOverride = user.Username }, cancellationToken); } - catch + catch (Exception ex) { - // Ignore logging failures + _logger.LogError(ex, "Failed to log account deletion audit for user {UserId}", user.Id); } return Result.Success(true); diff --git a/AutoStack.Application/Features/Users/Commands/EditUser/EditUserCommandHandler.cs b/AutoStack.Application/Features/Users/Commands/EditUser/EditUserCommandHandler.cs index a540cf2..1db5faf 100644 --- a/AutoStack.Application/Features/Users/Commands/EditUser/EditUserCommandHandler.cs +++ b/AutoStack.Application/Features/Users/Commands/EditUser/EditUserCommandHandler.cs @@ -6,6 +6,7 @@ using AutoStack.Domain.Entities; using AutoStack.Domain.Enums; using AutoStack.Domain.Repositories; +using Microsoft.Extensions.Logging; namespace AutoStack.Application.Features.Users.Commands.EditUser; @@ -15,13 +16,15 @@ public class EditUserCommandHandler : ICommandHandler _logger; - public EditUserCommandHandler(IUserRepository userRepository, IUnitOfWork unitOfWork, IPasswordHasher passwordHasher, IAuditLogService auditLogService) + public EditUserCommandHandler(IUserRepository userRepository, IUnitOfWork unitOfWork, IPasswordHasher passwordHasher, IAuditLogService auditLogService, ILogger logger) { _userRepository = userRepository; _unitOfWork = unitOfWork; _passwordHasher = passwordHasher; _auditLogService = auditLogService; + _logger = logger; } public async Task> Handle(EditUserCommand request, CancellationToken cancellationToken) @@ -68,21 +71,20 @@ public async Task> Handle(EditUserCommand request, Cancella var isPasswordValid = _passwordHasher.VerifyPassword(request.CurrentPassword, user.PasswordHash); if (!isPasswordValid) { - // Log failed password change attempt try { await _auditLogService.LogAsync(new AuditLogRequest { - Level = LogLevel.Warning, + Level = Domain.Enums.LogLevel.Warning, Category = LogCategory.Security, Message = "Failed password change attempt - incorrect current password", UserIdOverride = user.Id, UsernameOverride = user.Username }, cancellationToken); } - catch + catch (Exception ex) { - // Ignore logging failures + _logger.LogError(ex, "Failed to log password change attempt for user {UserId}", user.Id); } return Result.Failure("Current password is invalid"); @@ -101,7 +103,6 @@ await _auditLogService.LogAsync(new AuditLogRequest await _userRepository.UpdateAsync(user, cancellationToken); await _unitOfWork.SaveChangesAsync(cancellationToken); - // Log successful profile update if (changedFields.Count > 0) { try @@ -109,7 +110,7 @@ await _auditLogService.LogAsync(new AuditLogRequest additionalData["ChangedFields"] = changedFields; await _auditLogService.LogAsync(new AuditLogRequest { - Level = LogLevel.Information, + Level = Domain.Enums.LogLevel.Information, Category = LogCategory.User, Message = $"User profile updated: {string.Join(", ", changedFields)}", UserIdOverride = user.Id, @@ -117,9 +118,9 @@ await _auditLogService.LogAsync(new AuditLogRequest AdditionalData = additionalData }, cancellationToken); } - catch + catch (Exception ex) { - // Ignore logging failures + _logger.LogError(ex, "Failed to log profile update audit for user {UserId}", user.Id); } } diff --git a/AutoStack.Application/Features/Users/Commands/UploadAvatar/UploadAvatarCommandHandler.cs b/AutoStack.Application/Features/Users/Commands/UploadAvatar/UploadAvatarCommandHandler.cs index eeb1b9d..601af54 100644 --- a/AutoStack.Application/Features/Users/Commands/UploadAvatar/UploadAvatarCommandHandler.cs +++ b/AutoStack.Application/Features/Users/Commands/UploadAvatar/UploadAvatarCommandHandler.cs @@ -4,6 +4,7 @@ using AutoStack.Application.DTOs.Users; using AutoStack.Domain.Enums; using AutoStack.Domain.Repositories; +using Microsoft.Extensions.Logging; namespace AutoStack.Application.Features.Users.Commands.UploadAvatar; @@ -13,17 +14,20 @@ public class UploadAvatarCommandHandler : ICommandHandler _logger; public UploadAvatarCommandHandler( IUserRepository userRepository, IUnitOfWork unitOfWork, IFileStorageService fileStorageService, - IAuditLogService auditLogService) + IAuditLogService auditLogService, + ILogger logger) { _userRepository = userRepository; _unitOfWork = unitOfWork; _fileStorageService = fileStorageService; _auditLogService = auditLogService; + _logger = logger; } public async Task> Handle(UploadAvatarCommand request, CancellationToken cancellationToken) @@ -62,9 +66,10 @@ public async Task> Handle(UploadAvatarCommand request, Canc { await _fileStorageService.DeleteAvatarAsync(oldAvatarUrl, cancellationToken); } - catch + catch (Exception ex) { - // Ignore deletion errors + _logger.LogWarning(ex, "Failed to delete old avatar {OldAvatarUrl} for user {UserId}", oldAvatarUrl, user.Id); + // Continue - deletion failure is non-critical } } @@ -72,7 +77,7 @@ public async Task> Handle(UploadAvatarCommand request, Canc { await _auditLogService.LogAsync(new AuditLogRequest { - Level = LogLevel.Information, + Level = Domain.Enums.LogLevel.Information, Category = LogCategory.User, Message = "User avatar uploaded", UserIdOverride = user.Id, @@ -86,9 +91,9 @@ await _auditLogService.LogAsync(new AuditLogRequest } }, cancellationToken); } - catch + catch (Exception ex) { - // Ignore logging failures + _logger.LogError(ex, "Failed to log avatar upload audit for user {UserId}", user.Id); } var userResponse = new UserResponse(user.Id, user.Email, user.Username, user.AvatarUrl, user.EmailVerified); diff --git a/AutoStack.Infrastructure/Services/LocalFileStorageService.cs b/AutoStack.Infrastructure/Services/LocalFileStorageService.cs index 8fcdc29..52fd861 100644 --- a/AutoStack.Infrastructure/Services/LocalFileStorageService.cs +++ b/AutoStack.Infrastructure/Services/LocalFileStorageService.cs @@ -1,5 +1,6 @@ using AutoStack.Application.Common.Interfaces; using AutoStack.Infrastructure.Options; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace AutoStack.Infrastructure.Services; @@ -7,10 +8,12 @@ namespace AutoStack.Infrastructure.Services; public class LocalFileStorageService : IFileStorageService { private readonly FileStorageOptions _options; + private readonly ILogger _logger; - public LocalFileStorageService(IOptions options) + public LocalFileStorageService(IOptions options, ILogger logger) { _options = options.Value; + _logger = logger; if (string.IsNullOrWhiteSpace(_options.BaseUrl)) { @@ -58,9 +61,9 @@ public Task DeleteAvatarAsync(string fileUrl, CancellationToken cancellationToke File.Delete(filePath); } } - catch + catch (Exception ex) { - // Ignore deletion errors for now + _logger.LogWarning(ex, "Failed to delete avatar file from URL {FileUrl}", fileUrl); } return Task.CompletedTask; From 8951eaa53fde4db6aff2496d8227f14e6d6bd9b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rasmus=20H=C3=B8llund=20Kristensen?= Date: Thu, 11 Dec 2025 19:45:13 +0100 Subject: [PATCH 2/2] Fixed reflection on every request on validation behaviour and comments that is not needed anymore --- .../Common/Behaviours/ValidationBehaviour.cs | 68 ++++++++++++------- 1 file changed, 45 insertions(+), 23 deletions(-) diff --git a/AutoStack.Application/Common/Behaviours/ValidationBehaviour.cs b/AutoStack.Application/Common/Behaviours/ValidationBehaviour.cs index 588b7f0..234f7a6 100644 --- a/AutoStack.Application/Common/Behaviours/ValidationBehaviour.cs +++ b/AutoStack.Application/Common/Behaviours/ValidationBehaviour.cs @@ -1,4 +1,6 @@ -using AutoStack.Application.Common.Models; +using System.Collections.Concurrent; +using System.Linq.Expressions; +using AutoStack.Application.Common.Models; using FluentValidation; using MediatR; @@ -9,11 +11,11 @@ namespace AutoStack.Application.Common.Behaviours; /// /// The request type (Command or Query) /// The response type (must be a Result) -public class ValidationBehaviour : IPipelineBehavior +public class ValidationBehaviour : IPipelineBehavior where TRequest : notnull - where TResponse : Result { private readonly IEnumerable> _validators; + private static readonly ConcurrentDictionary, object>> FailureFactoryCache = new(); /// /// Constructor - MediatR DI automatically injects all validators for TRequest. @@ -35,29 +37,24 @@ public async Task Handle( RequestHandlerDelegate next, CancellationToken cancellationToken) { - // If no validators are registered for this request type, skip validation if (!_validators.Any()) { return await next(cancellationToken); } - + var context = new ValidationContext(request); - // Run ALL validators for this request type in parallel var validationResults = await Task.WhenAll( _validators.Select(v => v.ValidateAsync(context, cancellationToken))); - // Collect all validation failures from all validators var failures = validationResults - .Where(r => !r.IsValid) // Only failed validations - .SelectMany(r => r.Errors) // Get all error messages + .Where(r => !r.IsValid) + .SelectMany(r => r.Errors) .ToList(); - // If there are any validation failures, return error result - if (failures.Count == 0) return await next(cancellationToken); - - // Group errors by property name for structured response - // Example: { "Email": ["Email is required", "Invalid format"], "Password": ["Too short"] } + if (failures.Count == 0) + return await next(cancellationToken); + var errors = failures .GroupBy(e => e.PropertyName) .ToDictionary( @@ -65,17 +62,42 @@ public async Task Handle( g => g.Select(e => e.ErrorMessage).ToArray() ); - var resultType = typeof(TResponse); - var failureMethod = resultType.GetMethod( - nameof(Result.Failure), - [typeof(Dictionary)] - ); + return CreateValidationFailureResult(errors); + } + + private static TResult CreateValidationFailureResult(Dictionary errors) + { + var resultType = typeof(TResult); - if (failureMethod != null) + var factory = FailureFactoryCache.GetOrAdd(resultType, type => { - return (TResponse)failureMethod.Invoke(null, [errors])!; - } + if (type == typeof(Result)) + { + return errorsDict => Result.Failure(errorsDict); + } + + if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Result<>)) + { + var failureMethod = type.GetMethod(nameof(Result.Failure), new[] { typeof(Dictionary) }); + + if (failureMethod == null) + { + throw new InvalidOperationException($"Unable to find Failure method for type {type.Name}"); + } + + var errorsParam = Expression.Parameter(typeof(Dictionary), "errors"); + var callExpression = Expression.Call(failureMethod, errorsParam); + var lambda = Expression.Lambda, object>>( + Expression.Convert(callExpression, typeof(object)), + errorsParam + ); + + return lambda.Compile(); + } + + throw new InvalidOperationException($"Unable to create validation failure for type {type.Name}"); + }); - return await next(cancellationToken); + return (TResult)factory(errors); } } \ No newline at end of file