From a6a904645790e3746504a8ddf0b171f67976e419 Mon Sep 17 00:00:00 2001 From: Aaron Po Date: Sat, 12 Sep 2026 22:14:55 -0400 Subject: [PATCH] Fix CA1873 warning and consolidate test fixtures (#313) --- .../Behaviors/TestMediatorMessages.cs | 9 ++ .../UnhandledExceptionLoggingBehaviorTests.cs | 29 +++-- .../Behaviors/ValidationBehaviorTests.cs | 93 +++++++------- .../Shared.Application.Tests.csproj | 16 +-- .../UnhandledExceptionLoggingBehavior.cs | 119 ++++++++++-------- .../Behaviors/ValidationBehavior.cs | 5 +- 6 files changed, 150 insertions(+), 121 deletions(-) create mode 100644 web/backend/Shared/Shared.Application.Tests/Behaviors/TestMediatorMessages.cs diff --git a/web/backend/Shared/Shared.Application.Tests/Behaviors/TestMediatorMessages.cs b/web/backend/Shared/Shared.Application.Tests/Behaviors/TestMediatorMessages.cs new file mode 100644 index 00000000..e8bd26ef --- /dev/null +++ b/web/backend/Shared/Shared.Application.Tests/Behaviors/TestMediatorMessages.cs @@ -0,0 +1,9 @@ +using MediatR; + +namespace Shared.Application.Tests.Behaviors; + +// ReSharper disable NotAccessedPositionalProperty.Global +public sealed record TestRequest(string Username, string Password) : IRequest; + +public sealed record TestResponse(string Value); +// ReSharper restore NotAccessedPositionalProperty.Global diff --git a/web/backend/Shared/Shared.Application.Tests/Behaviors/UnhandledExceptionLoggingBehaviorTests.cs b/web/backend/Shared/Shared.Application.Tests/Behaviors/UnhandledExceptionLoggingBehaviorTests.cs index 5ccbb47a..5514233a 100644 --- a/web/backend/Shared/Shared.Application.Tests/Behaviors/UnhandledExceptionLoggingBehaviorTests.cs +++ b/web/backend/Shared/Shared.Application.Tests/Behaviors/UnhandledExceptionLoggingBehaviorTests.cs @@ -14,15 +14,18 @@ public class UnhandledExceptionLoggingBehaviorTests ILogger> > _loggerMock = new(); - private UnhandledExceptionLoggingBehavior CreateBehavior() => - new(_loggerMock.Object); + private UnhandledExceptionLoggingBehavior CreateBehavior() + { + _loggerMock.Setup(l => l.IsEnabled(LogLevel.Error)).Returns(true); + return new(_loggerMock.Object); + } [Fact] public async Task Handle_WhenHandlerThrowsGenericException_LogsOnceAndRethrows() { UnhandledExceptionLoggingBehavior behavior = CreateBehavior(); - TestRequest request = new("aaron", "hunter2"); - InvalidOperationException thrown = new("boom"); + TestRequest request = new("aaron", "pa$$word123"); + InvalidOperationException thrown = new("Something went wrong."); RequestHandlerDelegate next = () => throw thrown; Func act = async () => await behavior.Handle(request, next, CancellationToken.None); @@ -37,7 +40,7 @@ public class UnhandledExceptionLoggingBehaviorTests public async Task Handle_WhenValidationExceptionThrown_DoesNotLogAndRethrows() { UnhandledExceptionLoggingBehavior behavior = CreateBehavior(); - TestRequest request = new("aaron", "hunter2"); + TestRequest request = new("aaron", "pa$$word123"); ValidationException thrown = new([new ValidationFailure("Username", "is required")]); RequestHandlerDelegate next = () => throw thrown; @@ -51,7 +54,7 @@ public class UnhandledExceptionLoggingBehaviorTests public async Task Handle_WhenHandlerSucceeds_DoesNotLogAndReturnsResponse() { UnhandledExceptionLoggingBehavior behavior = CreateBehavior(); - TestRequest request = new("aaron", "hunter2"); + TestRequest request = new("aaron", "pa$$word123"); TestResponse response = new("ok"); RequestHandlerDelegate next = () => Task.FromResult(response); @@ -65,15 +68,15 @@ public class UnhandledExceptionLoggingBehaviorTests public async Task Handle_WhenHandlerThrows_RedactsSensitiveFieldsInLoggedPayload() { UnhandledExceptionLoggingBehavior behavior = CreateBehavior(); - TestRequest request = new("aaron", "hunter2"); + TestRequest request = new("aaron", "pa$$word123"); RequestHandlerDelegate next = () => - throw new InvalidOperationException("boom"); + throw new InvalidOperationException("Something went wrong."); Func act = async () => await behavior.Handle(request, next, CancellationToken.None); await act.Should().ThrowAsync(); string loggedMessage = CaptureLoggedMessage(); - loggedMessage.Should().Contain("aaron").And.NotContain("hunter2"); + loggedMessage.Should().Contain("aaron").And.NotContain("pa$$word123"); } private void VerifyLogError(Times times) => @@ -91,21 +94,17 @@ public class UnhandledExceptionLoggingBehaviorTests private string CaptureLoggedMessage() { - var invocation = _loggerMock + IInvocation? invocation = _loggerMock .Invocations.Where(i => i.Method.Name == nameof(ILogger.Log)) .Should() .ContainSingle() .Subject; object state = invocation.Arguments[2]; - object? exception = invocation.Arguments[3]; + object exception = invocation.Arguments[3]; object formatter = invocation.Arguments[4]; System.Reflection.MethodInfo invokeMethod = formatter.GetType().GetMethod("Invoke")!; return (string)invokeMethod.Invoke(formatter, [state, exception])!; } } - -public record TestRequest(string Username, string Password) : IRequest; - -public record TestResponse(string Value); diff --git a/web/backend/Shared/Shared.Application.Tests/Behaviors/ValidationBehaviorTests.cs b/web/backend/Shared/Shared.Application.Tests/Behaviors/ValidationBehaviorTests.cs index db91f2da..71919190 100644 --- a/web/backend/Shared/Shared.Application.Tests/Behaviors/ValidationBehaviorTests.cs +++ b/web/backend/Shared/Shared.Application.Tests/Behaviors/ValidationBehaviorTests.cs @@ -9,35 +9,35 @@ namespace Shared.Application.Tests.Behaviors; public class ValidationBehaviorTests { - private static ValidationBehavior CreateBehavior( - params IValidator[] validators + private static ValidationBehavior CreateBehavior( + params IValidator[] validators ) => new(validators); [Fact] public async Task Handle_WhenRequestPassesAllValidators_InvokesNextUnchanged() { - Mock> validatorMock = new(); + Mock> validatorMock = new(); validatorMock .Setup(v => v.ValidateAsync( - It.IsAny>(), + It.IsAny>(), It.IsAny() ) ) .ReturnsAsync(new ValidationResult()); - ValidationBehavior behavior = CreateBehavior( + ValidationBehavior behavior = CreateBehavior( validatorMock.Object ); - ValidationTestRequest request = new("aaron"); - ValidationTestResponse response = new("ok"); + TestRequest request = new("aaron", "pa$$word123"); + TestResponse response = new("ok"); bool nextInvoked = false; - RequestHandlerDelegate next = () => + RequestHandlerDelegate next = () => { nextInvoked = true; return Task.FromResult(response); }; - ValidationTestResponse result = await behavior.Handle( + TestResponse result = await behavior.Handle( request, next, CancellationToken.None @@ -50,30 +50,31 @@ public class ValidationBehaviorTests [Fact] public async Task Handle_WhenValidatorReportsFailures_ThrowsValidationExceptionAndNeverInvokesNext() { - ValidationFailure failure = new("Name", "is required"); - Mock> validatorMock = new(); + ValidationFailure failure = new("Username", "is required"); + Mock> validatorMock = new(); validatorMock .Setup(v => v.ValidateAsync( - It.IsAny>(), + It.IsAny>(), It.IsAny() ) ) .ReturnsAsync(new ValidationResult([failure])); - ValidationBehavior behavior = CreateBehavior( + ValidationBehavior behavior = CreateBehavior( validatorMock.Object ); - ValidationTestRequest request = new(""); + TestRequest request = new("", "pa$$word123"); bool nextInvoked = false; - RequestHandlerDelegate next = () => + RequestHandlerDelegate next = () => { nextInvoked = true; - return Task.FromResult(new ValidationTestResponse("ok")); + return Task.FromResult(new TestResponse("ok")); }; Func act = async () => await behavior.Handle(request, next, CancellationToken.None); - (await act.Should().ThrowAsync()).Which.Errors.Should() + (await act.Should().ThrowAsync()) + .Which.Errors.Should() .ContainSingle() .Which.Should() .BeSameAs(failure); @@ -83,38 +84,38 @@ public class ValidationBehaviorTests [Fact] public async Task Handle_WhenMultipleValidatorsAreRegistered_RunsAllOfThem() { - Mock> firstValidatorMock = new(); + Mock> firstValidatorMock = new(); firstValidatorMock .Setup(v => v.ValidateAsync( - It.IsAny>(), + It.IsAny>(), It.IsAny() ) ) .ReturnsAsync(new ValidationResult()); - Mock> secondValidatorMock = new(); + Mock> secondValidatorMock = new(); secondValidatorMock .Setup(v => v.ValidateAsync( - It.IsAny>(), + It.IsAny>(), It.IsAny() ) ) .ReturnsAsync(new ValidationResult()); - ValidationBehavior behavior = CreateBehavior( + ValidationBehavior behavior = CreateBehavior( firstValidatorMock.Object, secondValidatorMock.Object ); - ValidationTestRequest request = new("aaron"); - RequestHandlerDelegate next = () => - Task.FromResult(new ValidationTestResponse("ok")); + TestRequest request = new("aaron", "pa$$word123"); + RequestHandlerDelegate next = () => + Task.FromResult(new TestResponse("ok")); await behavior.Handle(request, next, CancellationToken.None); firstValidatorMock.Verify( v => v.ValidateAsync( - It.IsAny>(), + It.IsAny>(), It.IsAny() ), Times.Once() @@ -122,7 +123,7 @@ public class ValidationBehaviorTests secondValidatorMock.Verify( v => v.ValidateAsync( - It.IsAny>(), + It.IsAny>(), It.IsAny() ), Times.Once() @@ -132,49 +133,51 @@ public class ValidationBehaviorTests [Fact] public async Task Handle_WhenMultipleValidatorsReportFailures_AggregatesErrorsFromAll() { - ValidationFailure firstFailure = new("Name", "is required"); - ValidationFailure secondFailure = new("Name", "must be at least 3 characters"); - Mock> firstValidatorMock = new(); + ValidationFailure firstFailure = new("Username", "is required"); + ValidationFailure secondFailure = new("Username", "must be at least 3 characters"); + Mock> firstValidatorMock = new(); firstValidatorMock .Setup(v => v.ValidateAsync( - It.IsAny>(), + It.IsAny>(), It.IsAny() ) ) .ReturnsAsync(new ValidationResult([firstFailure])); - Mock> secondValidatorMock = new(); + Mock> secondValidatorMock = new(); secondValidatorMock .Setup(v => v.ValidateAsync( - It.IsAny>(), + It.IsAny>(), It.IsAny() ) ) .ReturnsAsync(new ValidationResult([secondFailure])); - ValidationBehavior behavior = CreateBehavior( + ValidationBehavior behavior = CreateBehavior( firstValidatorMock.Object, secondValidatorMock.Object ); - ValidationTestRequest request = new(""); - RequestHandlerDelegate next = () => - Task.FromResult(new ValidationTestResponse("ok")); + TestRequest request = new("", "pa$$word123"); + RequestHandlerDelegate next = () => + Task.FromResult(new TestResponse("ok")); Func act = async () => await behavior.Handle(request, next, CancellationToken.None); - (await act.Should().ThrowAsync()).Which.Errors.Should() + (await act.Should().ThrowAsync()) + .Which.Errors.Should() .BeEquivalentTo([firstFailure, secondFailure]); } [Fact] public async Task Handle_WhenNoValidatorsAreRegistered_InvokesNext() { - ValidationBehavior behavior = CreateBehavior(); - ValidationTestRequest request = new("aaron"); - ValidationTestResponse response = new("ok"); - RequestHandlerDelegate next = () => Task.FromResult(response); + ValidationBehavior behavior = + CreateBehavior(); + TestRequest request = new("aaron", "pa$$word123"); + TestResponse response = new("ok"); + RequestHandlerDelegate next = () => Task.FromResult(response); - ValidationTestResponse result = await behavior.Handle( + TestResponse result = await behavior.Handle( request, next, CancellationToken.None @@ -183,7 +186,3 @@ public class ValidationBehaviorTests result.Should().BeSameAs(response); } } - -public record ValidationTestRequest(string Name) : IRequest; - -public record ValidationTestResponse(string Value); diff --git a/web/backend/Shared/Shared.Application.Tests/Shared.Application.Tests.csproj b/web/backend/Shared/Shared.Application.Tests/Shared.Application.Tests.csproj index 35a05cfc..c7c3d8a4 100644 --- a/web/backend/Shared/Shared.Application.Tests/Shared.Application.Tests.csproj +++ b/web/backend/Shared/Shared.Application.Tests/Shared.Application.Tests.csproj @@ -8,19 +8,19 @@ - - - - - - + + + + + + - + - + diff --git a/web/backend/Shared/Shared.Application/Behaviors/UnhandledExceptionLoggingBehavior.cs b/web/backend/Shared/Shared.Application/Behaviors/UnhandledExceptionLoggingBehavior.cs index 08721878..0137b683 100644 --- a/web/backend/Shared/Shared.Application/Behaviors/UnhandledExceptionLoggingBehavior.cs +++ b/web/backend/Shared/Shared.Application/Behaviors/UnhandledExceptionLoggingBehavior.cs @@ -6,18 +6,11 @@ using Microsoft.Extensions.Logging; namespace Shared.Application.Behaviors; -/// -/// MediatR pipeline behavior that logs unhandled exceptions together with the request type and -/// payload, then rethrows them unchanged for GlobalExceptionFilter to map to a status code. -/// -public class UnhandledExceptionLoggingBehavior( - ILogger> logger -) : IPipelineBehavior - where TRequest : IRequest +public abstract class JsonLogger { private static readonly string[] SensitiveFieldNames = ["password", "token"]; - private static readonly JsonSerializerOptions SerializerOptions = new() + protected static readonly JsonSerializerOptions SerializerOptions = new() { TypeInfoResolver = new DefaultJsonTypeInfoResolver { @@ -25,46 +18,6 @@ public class UnhandledExceptionLoggingBehavior( }, }; - /// - public async Task Handle( - TRequest request, - RequestHandlerDelegate next, - CancellationToken cancellationToken - ) - { - try - { - return await next(); - } - catch (ValidationException) - { - // Expected: ValidationBehavior already reports these as request errors, not incidents. - throw; - } - catch (Exception ex) - { - logger.LogError( - ex, - "Unhandled exception while processing {RequestName}: {RequestPayload}", - typeof(TRequest).Name, - Serialize(request) - ); - throw; - } - } - - private static string Serialize(TRequest request) - { - try - { - return JsonSerializer.Serialize(request, SerializerOptions); - } - catch (Exception) - { - return ""; - } - } - private static void RedactSensitiveProperties(JsonTypeInfo typeInfo) { if (typeInfo.Kind != JsonTypeInfoKind.Object) @@ -84,3 +37,71 @@ public class UnhandledExceptionLoggingBehavior( } } } + +/// +/// MediatR pipeline behavior that logs unhandled exceptions together with the +/// request type and +/// payload, then rethrows them unchanged for GlobalExceptionFilter to +/// map to a status code. +/// +public partial class UnhandledExceptionLoggingBehavior + : JsonLogger, + IPipelineBehavior + where TRequest : IRequest +{ + private readonly ILogger> _logger; + + public UnhandledExceptionLoggingBehavior( + ILogger> logger + ) + { + _logger = logger; + } + + /// + public async Task Handle( + TRequest request, + RequestHandlerDelegate next, + CancellationToken cancellationToken + ) + { + try + { + return await next(); + } + catch (ValidationException) + { + // Expected: ValidationBehavior already reports these as request errors + throw; + } + catch (Exception ex) + { + if (_logger.IsEnabled(LogLevel.Error)) + LogUnhandledException(ex, typeof(TRequest).Name, Serialize(request)); + + throw; + } + } + + private static string Serialize(TRequest request) + { + try + { + return JsonSerializer.Serialize(request, SerializerOptions); + } + catch (Exception) + { + return ""; + } + } + + [LoggerMessage( + Level = LogLevel.Error, + Message = "Unhandled exception while processing {RequestName}: {RequestPayload}" + )] + private partial void LogUnhandledException( + Exception ex, + string requestName, + string requestPayload + ); +} diff --git a/web/backend/Shared/Shared.Application/Behaviors/ValidationBehavior.cs b/web/backend/Shared/Shared.Application/Behaviors/ValidationBehavior.cs index aabc964f..6c18690e 100644 --- a/web/backend/Shared/Shared.Application/Behaviors/ValidationBehavior.cs +++ b/web/backend/Shared/Shared.Application/Behaviors/ValidationBehavior.cs @@ -5,14 +5,15 @@ using MediatR; namespace Shared.Application.Behaviors; /// -/// MediatR pipeline behavior that runs registered FluentValidation validators before the handler, +/// MediatR pipeline behavior that runs registered FluentValidation validators +/// before the handler, /// throwing if any report failures. /// public class ValidationBehavior(IEnumerable> validators) : IPipelineBehavior where TRequest : IRequest { - /// + /// public async Task Handle( TRequest request, RequestHandlerDelegate next,