From 965a6e6772615b99cc5196108d46bf4c3e76072b Mon Sep 17 00:00:00 2001 From: Rik De Peuter Date: Wed, 4 Sep 2024 14:02:05 +0200 Subject: [PATCH] fix: use lifetimescope when dispatching multiple commands for municipality proposal --- .../Function.cs | 5 ++ ...eStreetNameForMunicipalityMergerHandler.cs | 27 ++++--- .../ScopedIdempotentCommandHandler.cs | 31 ++++++++ .../BackOffice/Lambda/BackOfficeLambdaTest.cs | 24 +++++++ ...roposeForMunicipalityMergerHandlerTests.cs | 72 +++++++++++++------ 5 files changed, 128 insertions(+), 31 deletions(-) create mode 100644 src/StreetNameRegistry.Api.BackOffice.Handlers.Lambda/ScopedIdempotentCommandHandler.cs diff --git a/src/StreetNameRegistry.Api.BackOffice.Handlers.Lambda/Function.cs b/src/StreetNameRegistry.Api.BackOffice.Handlers.Lambda/Function.cs index 4df1a6c98..738a374a4 100644 --- a/src/StreetNameRegistry.Api.BackOffice.Handlers.Lambda/Function.cs +++ b/src/StreetNameRegistry.Api.BackOffice.Handlers.Lambda/Function.cs @@ -80,6 +80,11 @@ protected override IServiceProvider ConfigureServices(IServiceCollection service .AsSelf() .InstancePerLifetimeScope(); + builder.RegisterType() + .As() + .AsSelf() + .InstancePerLifetimeScope(); + services.ConfigureIdempotency( configuration.GetSection(IdempotencyConfiguration.Section).Get() .ConnectionString, diff --git a/src/StreetNameRegistry.Api.BackOffice.Handlers.Lambda/Handlers/ProposeStreetNameForMunicipalityMergerHandler.cs b/src/StreetNameRegistry.Api.BackOffice.Handlers.Lambda/Handlers/ProposeStreetNameForMunicipalityMergerHandler.cs index c464df7e2..1957fff20 100644 --- a/src/StreetNameRegistry.Api.BackOffice.Handlers.Lambda/Handlers/ProposeStreetNameForMunicipalityMergerHandler.cs +++ b/src/StreetNameRegistry.Api.BackOffice.Handlers.Lambda/Handlers/ProposeStreetNameForMunicipalityMergerHandler.cs @@ -7,6 +7,7 @@ namespace StreetNameRegistry.Api.BackOffice.Handlers.Lambda.Handlers using Be.Vlaanderen.Basisregisters.Sqs.Lambda.Infrastructure; using Be.Vlaanderen.Basisregisters.Sqs.Responses; using Microsoft.Extensions.Configuration; + using Microsoft.Extensions.Logging; using Municipality; using Municipality.Commands; using Municipality.Exceptions; @@ -16,14 +17,17 @@ namespace StreetNameRegistry.Api.BackOffice.Handlers.Lambda.Handlers public sealed class ProposeStreetNameForMunicipalityMergerHandler : StreetNameLambdaHandler { private readonly BackOfficeContext _backOfficeContext; + private readonly ILogger _logger; public ProposeStreetNameForMunicipalityMergerHandler( IConfiguration configuration, ICustomRetryPolicy retryPolicy, ITicketing ticketing, - IIdempotentCommandHandler idempotentCommandHandler, + IScopedIdempotentCommandHandler idempotentCommandHandler, BackOfficeContext backOfficeContext, - IMunicipalities municipalities) + IMunicipalities municipalities, + ILoggerFactory loggerFactory + ) : base( configuration, retryPolicy, @@ -32,6 +36,7 @@ public ProposeStreetNameForMunicipalityMergerHandler( idempotentCommandHandler) { _backOfficeContext = backOfficeContext; + _logger = loggerFactory.CreateLogger(GetType()); } protected override async Task InnerHandle( @@ -40,20 +45,24 @@ protected override async Task InnerHandle( { var commands = await BuildCommands(request, cancellationToken); - try + foreach (var command in commands) { - foreach (var command in commands) + _logger.LogDebug($"Handling {command.GetType().FullName}"); + + try { await IdempotentCommandHandler.Dispatch( command.CreateCommandId(), command, request.Metadata!, - cancellationToken); + cancellationToken: cancellationToken); + _logger.LogDebug($"Handled {command.GetType().FullName}"); + } + catch (IdempotencyException) + { + // Idempotent: Do Nothing return last etag + _logger.LogDebug($"Skipped due to idempotency {command.GetType().FullName}"); } - } - catch (IdempotencyException) - { - // Idempotent: Do Nothing return last etag } var etagResponses = new List(); diff --git a/src/StreetNameRegistry.Api.BackOffice.Handlers.Lambda/ScopedIdempotentCommandHandler.cs b/src/StreetNameRegistry.Api.BackOffice.Handlers.Lambda/ScopedIdempotentCommandHandler.cs new file mode 100644 index 000000000..e852f0563 --- /dev/null +++ b/src/StreetNameRegistry.Api.BackOffice.Handlers.Lambda/ScopedIdempotentCommandHandler.cs @@ -0,0 +1,31 @@ +namespace StreetNameRegistry.Api.BackOffice.Handlers.Lambda +{ + using Autofac; + using Be.Vlaanderen.Basisregisters.CommandHandling.Idempotency; + + public interface IScopedIdempotentCommandHandler: IIdempotentCommandHandler + { + } + + public class ScopedIdempotentCommandHandler : IScopedIdempotentCommandHandler + { + private readonly ILifetimeScope _container; + + public ScopedIdempotentCommandHandler(ILifetimeScope container) + { + _container = container; + } + + public async Task Dispatch(Guid? commandId, object command, IDictionary metadata, CancellationToken cancellationToken) + { + await using var scope = _container.BeginLifetimeScope(); + + var resolver = scope.Resolve(); + return await resolver.Dispatch( + commandId, + command, + metadata, + cancellationToken); + } + } +} diff --git a/test/StreetNameRegistry.Tests/BackOffice/Lambda/BackOfficeLambdaTest.cs b/test/StreetNameRegistry.Tests/BackOffice/Lambda/BackOfficeLambdaTest.cs index 40461df22..506a11404 100644 --- a/test/StreetNameRegistry.Tests/BackOffice/Lambda/BackOfficeLambdaTest.cs +++ b/test/StreetNameRegistry.Tests/BackOffice/Lambda/BackOfficeLambdaTest.cs @@ -13,6 +13,8 @@ namespace StreetNameRegistry.Tests.BackOffice.Lambda using Municipality; using Municipality.Commands; using Newtonsoft.Json; + using StreetNameRegistry.Api.BackOffice.Handlers.Lambda; + using StreetNameRegistry.Api.BackOffice.Handlers.Lambda.Handlers; using Testing; using TicketingService.Abstractions; using Xunit.Abstractions; @@ -51,6 +53,28 @@ protected Mock MockExceptionIdempotentCommandHandler< return idempotentCommandHandler; } + protected Mock MockExceptionScopedIdempotentCommandHandler() + where TException : Exception, new() + { + var idempotentCommandHandler = new Mock(); + idempotentCommandHandler + .Setup(x => x.Dispatch(It.IsAny(), It.IsAny(), + It.IsAny>(), CancellationToken.None)) + .Throws(); + return idempotentCommandHandler; + } + + protected Mock MockExceptionScopedIdempotentCommandHandler(Func exceptionFactory) + where TException : Exception + { + var idempotentCommandHandler = new Mock(); + idempotentCommandHandler + .Setup(x => x.Dispatch(It.IsAny(), It.IsAny(), + It.IsAny>(), CancellationToken.None)) + .Throws(exceptionFactory()); + return idempotentCommandHandler; + } + protected Mock MockTicketing(Action ticketingCompleteCallback) { var ticketing = new Mock(); diff --git a/test/StreetNameRegistry.Tests/BackOffice/Lambda/WhenProposingStreetNameForMunicipalityMerger/SqsStreetNameProposeForMunicipalityMergerHandlerTests.cs b/test/StreetNameRegistry.Tests/BackOffice/Lambda/WhenProposingStreetNameForMunicipalityMerger/SqsStreetNameProposeForMunicipalityMergerHandlerTests.cs index c17d5e5e0..6df824cfc 100644 --- a/test/StreetNameRegistry.Tests/BackOffice/Lambda/WhenProposingStreetNameForMunicipalityMerger/SqsStreetNameProposeForMunicipalityMergerHandlerTests.cs +++ b/test/StreetNameRegistry.Tests/BackOffice/Lambda/WhenProposingStreetNameForMunicipalityMerger/SqsStreetNameProposeForMunicipalityMergerHandlerTests.cs @@ -13,15 +13,17 @@ namespace StreetNameRegistry.Tests.BackOffice.Lambda.WhenProposingStreetNameForM using FluentAssertions; using global::AutoFixture; using Microsoft.Extensions.Configuration; + using Microsoft.Extensions.Logging.Abstractions; using Moq; + using Municipality; + using Municipality.Exceptions; using Newtonsoft.Json; using SqlStreamStore; using SqlStreamStore.Streams; using StreetNameRegistry.Api.BackOffice.Abstractions.SqsRequests; + using StreetNameRegistry.Api.BackOffice.Handlers.Lambda; using StreetNameRegistry.Api.BackOffice.Handlers.Lambda.Handlers; using StreetNameRegistry.Api.BackOffice.Handlers.Lambda.Requests; - using Municipality; - using Municipality.Exceptions; using TicketingService.Abstractions; using Xunit; using Xunit.Abstractions; @@ -68,9 +70,10 @@ public async Task ThenTheStreetNameIsProposed() Container.Resolve(), new FakeRetryPolicy(), ticketing.Object, - new IdempotentCommandHandler(Container.Resolve(), _idempotencyContext), + new FakeScopedIdemponentCommandHandler(() => new IdempotentCommandHandler(Container.Resolve(), _idempotencyContext)), _backOfficeContext, - Container.Resolve()); + Container.Resolve(), + new NullLoggerFactory()); //Act await handler.Handle(new ProposeStreetNamesForMunicipalityMergerLambdaRequest(newMunicipalityId, @@ -120,9 +123,10 @@ public async Task WhenStreetNameNameAlreadyExistsException_ThenTicketingErrorIsE Container.Resolve(), new FakeRetryPolicy(), ticketing.Object, - MockExceptionIdempotentCommandHandler(() => new StreetNameNameAlreadyExistsException(streetname)).Object, + MockExceptionScopedIdempotentCommandHandler(() => new StreetNameNameAlreadyExistsException(streetname)).Object, _backOfficeContext, - Mock.Of()); + Mock.Of(), + new NullLoggerFactory()); await sut.Handle(new ProposeStreetNamesForMunicipalityMergerLambdaRequest(newMunicipalityId, new ProposeStreetNamesForMunicipalityMergerSqsRequest @@ -167,9 +171,10 @@ public async Task WhenMunicipalityHasInvalidStatusException_ThenTicketingErrorIs Container.Resolve(), new FakeRetryPolicy(), ticketing.Object, - MockExceptionIdempotentCommandHandler().Object, + MockExceptionScopedIdempotentCommandHandler().Object, _backOfficeContext, - Mock.Of()); + Mock.Of(), + new NullLoggerFactory()); await sut.Handle(new ProposeStreetNamesForMunicipalityMergerLambdaRequest(newMunicipalityId, new ProposeStreetNamesForMunicipalityMergerSqsRequest @@ -213,9 +218,10 @@ public async Task WhenStreetNameNameLanguageIsNotSupportedException_ThenTicketin Container.Resolve(), new FakeRetryPolicy(), ticketing.Object, - MockExceptionIdempotentCommandHandler().Object, + MockExceptionScopedIdempotentCommandHandler().Object, _backOfficeContext, - Mock.Of()); + Mock.Of(), + new NullLoggerFactory()); // Act var municipalityId = Guid.NewGuid().ToString(); @@ -262,9 +268,10 @@ public async Task WhenStreetNameIsMissingALanguageException_ThenTicketingErrorIs Container.Resolve(), new FakeRetryPolicy(), ticketing.Object, - MockExceptionIdempotentCommandHandler().Object, + MockExceptionScopedIdempotentCommandHandler().Object, _backOfficeContext, - Mock.Of()); + Mock.Of(), + new NullLoggerFactory()); // Act await sut.Handle(new ProposeStreetNamesForMunicipalityMergerLambdaRequest(newMunicipalityId, @@ -311,9 +318,10 @@ public async Task WhenMergedStreetNamePersistentLocalIdsAreMissingException_Then Container.Resolve(), new FakeRetryPolicy(), ticketing.Object, - MockExceptionIdempotentCommandHandler().Object, + MockExceptionScopedIdempotentCommandHandler().Object, _backOfficeContext, - Mock.Of()); + Mock.Of(), + new NullLoggerFactory()); // Act await sut.Handle(new ProposeStreetNamesForMunicipalityMergerLambdaRequest(newMunicipalityId, @@ -360,9 +368,10 @@ public async Task WhenMergedStreetNamePersistentLocalIdsAreNotUniqueException_Th Container.Resolve(), new FakeRetryPolicy(), ticketing.Object, - MockExceptionIdempotentCommandHandler().Object, + MockExceptionScopedIdempotentCommandHandler().Object, _backOfficeContext, - Mock.Of()); + Mock.Of(), + new NullLoggerFactory()); // Act await sut.Handle(new ProposeStreetNamesForMunicipalityMergerLambdaRequest(newMunicipalityId, @@ -409,9 +418,10 @@ public async Task WhenStreetNameHasInvalidDesiredStatusException_ThenTicketingEr Container.Resolve(), new FakeRetryPolicy(), ticketing.Object, - MockExceptionIdempotentCommandHandler().Object, + MockExceptionScopedIdempotentCommandHandler().Object, _backOfficeContext, - Mock.Of()); + Mock.Of(), + new NullLoggerFactory()); // Act await sut.Handle(new ProposeStreetNamesForMunicipalityMergerLambdaRequest(newMunicipalityId, @@ -479,9 +489,10 @@ public async Task WhenIdempotencyException_ThenTicketingCompleteIsExpected() Container.Resolve(), new FakeRetryPolicy(), ticketing.Object, - MockExceptionIdempotentCommandHandler(() => new IdempotencyException(string.Empty)).Object, + MockExceptionScopedIdempotentCommandHandler(() => new IdempotencyException(string.Empty)).Object, _backOfficeContext, - municipalities); + municipalities, + new NullLoggerFactory()); // Act await sut.Handle(new ProposeStreetNamesForMunicipalityMergerLambdaRequest(newMunicipalityId, @@ -545,9 +556,10 @@ public async Task GivenRetryingRequest_ThenTicketingCompleteIsExpected() Container.Resolve(), new FakeRetryPolicy(), ticketing.Object, - new IdempotentCommandHandler(Container.Resolve(), _idempotencyContext), + new FakeScopedIdemponentCommandHandler(() => new IdempotentCommandHandler(Container.Resolve(), _idempotencyContext)), _backOfficeContext, - municipalities); + municipalities, + new NullLoggerFactory()); var request = new ProposeStreetNamesForMunicipalityMergerLambdaRequest(newMunicipalityId, new ProposeStreetNamesForMunicipalityMergerSqsRequest @@ -589,5 +601,21 @@ public async Task GivenRetryingRequest_ThenTicketingCompleteIsExpected() }), CancellationToken.None)); } + + private class FakeScopedIdemponentCommandHandler : IScopedIdempotentCommandHandler + { + private readonly Func _idempotentCommandHandlerResolver; + + public FakeScopedIdemponentCommandHandler(Func idempotentCommandHandlerResolver) + { + _idempotentCommandHandlerResolver = idempotentCommandHandlerResolver; + } + + public Task Dispatch(Guid? commandId, object command, IDictionary metadata, CancellationToken cancellationToken) + { + var handler = _idempotentCommandHandlerResolver(); + return handler.Dispatch(commandId, command, metadata, cancellationToken); + } + } } }