diff --git a/docs/web/docker.md b/docs/web/docker.md index a097d10f..ef05717b 100644 --- a/docs/web/docker.md +++ b/docs/web/docker.md @@ -98,7 +98,7 @@ sqlserver # Test database database.migrations # Fresh schema database.seed # Test data api.specs # Reqnroll BDD tests -unit.tests # All Features.*.Tests unit test projects +unit.tests # All Features.*.Tests and Shared.*.Tests unit test projects frontend.tests # Storybook Vitest + Playwright suites (own Playwright image) ``` diff --git a/docs/web/testing.md b/docs/web/testing.md index 18a299ab..9f50e28d 100644 --- a/docs/web/testing.md +++ b/docs/web/testing.md @@ -76,10 +76,11 @@ docker compose --env-file web/.env.test -f web/docker-compose.test.yaml down -v You can run individual test projects locally without Docker: -### Feature slice unit tests +### Feature slice and shared unit tests Each feature slice has its own test project, covering its command/query -handlers: +handlers. Shared cross-cutting code (e.g. MediatR pipeline behaviors) has +its own test project under `Shared/`: ```bash cd web/backend @@ -87,9 +88,10 @@ dotnet test Features/Features.Users.Tests/Features.Users.Tests.csproj dotnet test Features/Features.Breweries.Tests/Features.Breweries.Tests.csproj dotnet test Features/Features.Emails.Tests/Features.Emails.Tests.csproj dotnet test Features/Features.PhotoUpload.Tests/Features.PhotoUpload.Tests.csproj +dotnet test Shared/Shared.Application.Tests/Shared.Application.Tests.csproj # Or run all of them at once via the solution: -for proj in Features/*.Tests; do dotnet test "$proj"; done +for proj in Features/*.Tests Shared/*.Tests; do dotnet test "$proj"; done ``` **Requirements**: diff --git a/web/backend/API/API.Core/Dockerfile b/web/backend/API/API.Core/Dockerfile index f1af52e9..3973aa04 100644 --- a/web/backend/API/API.Core/Dockerfile +++ b/web/backend/API/API.Core/Dockerfile @@ -35,6 +35,7 @@ COPY ["Features/Features.Users/Features.Users.csproj", "Features/Features.Users/ COPY ["Features/Features.Users.Tests/Features.Users.Tests.csproj", "Features/Features.Users.Tests/"] COPY ["Configuration/Configuration.csproj", "Configuration/"] COPY ["Shared/Shared.Application/Shared.Application.csproj", "Shared/Shared.Application/"] +COPY ["Shared/Shared.Application.Tests/Shared.Application.Tests.csproj", "Shared/Shared.Application.Tests/"] COPY ["Shared/Shared.Contracts/Shared.Contracts.csproj", "Shared/Shared.Contracts/"] RUN dotnet restore "Biergarten.API.slnx" COPY . . diff --git a/web/backend/API/API.Core/ServiceCollectionExtensions.cs b/web/backend/API/API.Core/ServiceCollectionExtensions.cs index 3f168f89..f6fdcb7f 100644 --- a/web/backend/API/API.Core/ServiceCollectionExtensions.cs +++ b/web/backend/API/API.Core/ServiceCollectionExtensions.cs @@ -46,6 +46,7 @@ internal static class ServiceCollectionExtensions cfg.RegisterServicesFromAssembly(typeof(UploadPhotoCommand).Assembly); cfg.RegisterServicesFromAssembly(typeof(IEmailDispatcher).Assembly); cfg.RegisterServicesFromAssembly(typeof(CityController).Assembly); + cfg.AddOpenBehavior(typeof(UnhandledExceptionLoggingBehavior<,>)); cfg.AddOpenBehavior(typeof(ValidationBehavior<,>)); }); diff --git a/web/backend/API/API.Specs/Dockerfile b/web/backend/API/API.Specs/Dockerfile index d303ea1d..cb962d08 100644 --- a/web/backend/API/API.Specs/Dockerfile +++ b/web/backend/API/API.Specs/Dockerfile @@ -25,6 +25,7 @@ COPY ["Features/Features.Users/Features.Users.csproj", "Features/Features.Users/ COPY ["Features/Features.Users.Tests/Features.Users.Tests.csproj", "Features/Features.Users.Tests/"] COPY ["Configuration/Configuration.csproj", "Configuration/"] COPY ["Shared/Shared.Application/Shared.Application.csproj", "Shared/Shared.Application/"] +COPY ["Shared/Shared.Application.Tests/Shared.Application.Tests.csproj", "Shared/Shared.Application.Tests/"] COPY ["Shared/Shared.Contracts/Shared.Contracts.csproj", "Shared/Shared.Contracts/"] RUN dotnet restore "Biergarten.API.slnx" COPY . . diff --git a/web/backend/Biergarten.API.slnx b/web/backend/Biergarten.API.slnx index b2734ffb..6d5e7d17 100644 --- a/web/backend/Biergarten.API.slnx +++ b/web/backend/Biergarten.API.slnx @@ -47,5 +47,6 @@ + diff --git a/web/backend/Dockerfile.tests b/web/backend/Dockerfile.tests index bab9c101..55c661b5 100644 --- a/web/backend/Dockerfile.tests +++ b/web/backend/Dockerfile.tests @@ -5,11 +5,11 @@ COPY . . RUN dotnet restore "Biergarten.API.slnx" RUN dotnet build "Biergarten.API.slnx" -c $BUILD_CONFIGURATION -# Runs every Features.*.Tests project (unit + DbMocker-backed repository tests). API.Specs is -# excluded here since it's a separate BDD/integration suite, run by its own compose service -# against the live, seeded database. +# Runs every Features.*.Tests and Shared.*.Tests project (unit + DbMocker-backed repository +# tests). API.Specs is excluded here since it's a separate BDD/integration suite, run by its own +# compose service against the live, seeded database. FROM build AS final ARG BUILD_CONFIGURATION=Release WORKDIR /src RUN mkdir -p /app/test-results -ENTRYPOINT ["sh", "-c", "set -e; for proj in Features/*.Tests; do dotnet test \"$proj\" -c ${BUILD_CONFIGURATION:-Release} --logger \"trx;LogFileName=/app/test-results/$(basename \"$proj\").trx\"; done"] +ENTRYPOINT ["sh", "-c", "set -e; for proj in Features/*.Tests Shared/*.Tests; do dotnet test \"$proj\" -c ${BUILD_CONFIGURATION:-Release} --logger \"trx;LogFileName=/app/test-results/$(basename \"$proj\").trx\"; done"] diff --git a/web/backend/Shared/Shared.Application.Tests/Behaviors/UnhandledExceptionLoggingBehaviorTests.cs b/web/backend/Shared/Shared.Application.Tests/Behaviors/UnhandledExceptionLoggingBehaviorTests.cs new file mode 100644 index 00000000..5ccbb47a --- /dev/null +++ b/web/backend/Shared/Shared.Application.Tests/Behaviors/UnhandledExceptionLoggingBehaviorTests.cs @@ -0,0 +1,111 @@ +using FluentAssertions; +using FluentValidation; +using FluentValidation.Results; +using MediatR; +using Microsoft.Extensions.Logging; +using Moq; +using Shared.Application.Behaviors; + +namespace Shared.Application.Tests.Behaviors; + +public class UnhandledExceptionLoggingBehaviorTests +{ + private readonly Mock< + ILogger> + > _loggerMock = new(); + + private UnhandledExceptionLoggingBehavior CreateBehavior() => + new(_loggerMock.Object); + + [Fact] + public async Task Handle_WhenHandlerThrowsGenericException_LogsOnceAndRethrows() + { + UnhandledExceptionLoggingBehavior behavior = CreateBehavior(); + TestRequest request = new("aaron", "hunter2"); + InvalidOperationException thrown = new("boom"); + RequestHandlerDelegate next = () => throw thrown; + + Func act = async () => await behavior.Handle(request, next, CancellationToken.None); + + (await act.Should().ThrowAsync()) + .Which.Should() + .BeSameAs(thrown); + VerifyLogError(Times.Once()); + } + + [Fact] + public async Task Handle_WhenValidationExceptionThrown_DoesNotLogAndRethrows() + { + UnhandledExceptionLoggingBehavior behavior = CreateBehavior(); + TestRequest request = new("aaron", "hunter2"); + ValidationException thrown = new([new ValidationFailure("Username", "is required")]); + RequestHandlerDelegate next = () => throw thrown; + + Func act = async () => await behavior.Handle(request, next, CancellationToken.None); + + (await act.Should().ThrowAsync()).Which.Should().BeSameAs(thrown); + VerifyLogError(Times.Never()); + } + + [Fact] + public async Task Handle_WhenHandlerSucceeds_DoesNotLogAndReturnsResponse() + { + UnhandledExceptionLoggingBehavior behavior = CreateBehavior(); + TestRequest request = new("aaron", "hunter2"); + TestResponse response = new("ok"); + RequestHandlerDelegate next = () => Task.FromResult(response); + + TestResponse result = await behavior.Handle(request, next, CancellationToken.None); + + result.Should().BeSameAs(response); + VerifyLogError(Times.Never()); + } + + [Fact] + public async Task Handle_WhenHandlerThrows_RedactsSensitiveFieldsInLoggedPayload() + { + UnhandledExceptionLoggingBehavior behavior = CreateBehavior(); + TestRequest request = new("aaron", "hunter2"); + RequestHandlerDelegate next = () => + throw new InvalidOperationException("boom"); + + Func act = async () => await behavior.Handle(request, next, CancellationToken.None); + await act.Should().ThrowAsync(); + + string loggedMessage = CaptureLoggedMessage(); + loggedMessage.Should().Contain("aaron").And.NotContain("hunter2"); + } + + private void VerifyLogError(Times times) => + _loggerMock.Verify( + l => + l.Log( + LogLevel.Error, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>() + ), + times + ); + + private string CaptureLoggedMessage() + { + var 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 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/Shared.Application.Tests.csproj b/web/backend/Shared/Shared.Application.Tests/Shared.Application.Tests.csproj new file mode 100644 index 00000000..35a05cfc --- /dev/null +++ b/web/backend/Shared/Shared.Application.Tests/Shared.Application.Tests.csproj @@ -0,0 +1,26 @@ + + + net10.0 + enable + enable + false + Shared.Application.Tests + + + + + + + + + + + + + + + + + + + diff --git a/web/backend/Shared/Shared.Application/Behaviors/UnhandledExceptionLoggingBehavior.cs b/web/backend/Shared/Shared.Application/Behaviors/UnhandledExceptionLoggingBehavior.cs new file mode 100644 index 00000000..08721878 --- /dev/null +++ b/web/backend/Shared/Shared.Application/Behaviors/UnhandledExceptionLoggingBehavior.cs @@ -0,0 +1,86 @@ +using System.Text.Json; +using System.Text.Json.Serialization.Metadata; +using FluentValidation; +using MediatR; +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 +{ + private static readonly string[] SensitiveFieldNames = ["password", "token"]; + + private static readonly JsonSerializerOptions SerializerOptions = new() + { + TypeInfoResolver = new DefaultJsonTypeInfoResolver + { + Modifiers = { RedactSensitiveProperties }, + }, + }; + + /// + 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) + return; + + foreach (JsonPropertyInfo property in typeInfo.Properties) + { + if (property.PropertyType != typeof(string)) + continue; + + if ( + SensitiveFieldNames.Any(name => + property.Name.Contains(name, StringComparison.OrdinalIgnoreCase) + ) + ) + property.Get = static _ => "***REDACTED***"; + } + } +} diff --git a/web/backend/Shared/Shared.Application/Shared.Application.csproj b/web/backend/Shared/Shared.Application/Shared.Application.csproj index be50cf0f..305d1df0 100644 --- a/web/backend/Shared/Shared.Application/Shared.Application.csproj +++ b/web/backend/Shared/Shared.Application/Shared.Application.csproj @@ -9,5 +9,6 @@ +