From afba508283e46c204fd7b53b9243b8dfc6f2faee Mon Sep 17 00:00:00 2001 From: Pietro Tota Date: Wed, 29 Nov 2023 23:33:28 +0100 Subject: [PATCH 1/6] feat(retry): disable retry for blocking exceptions --- pom.xml | 5 + src/main/resources/application.yml | 98 ++++- .../services/v1/CircuitBreakerTest.java | 345 +++++++++++++++++- 3 files changed, 433 insertions(+), 15 deletions(-) diff --git a/pom.xml b/pom.xml index 45aeb0c45..99b302a28 100644 --- a/pom.xml +++ b/pom.xml @@ -231,6 +231,11 @@ opentelemetry-semconv 1.27.0-alpha + + com.fasterxml.jackson.dataformat + jackson-dataformat-yaml + test + diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 8725764ae..4b9358255 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -1,29 +1,89 @@ resilience4j.retry: instances: - getPaymentRequestInfo: - maxAttempts: 3 - waitDuration: 2s - enableExponentialBackoff: false newTransaction: maxAttempts: 3 waitDuration: 2s enableExponentialBackoff: false + ignoreExceptions: + - it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException + - it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException + - it.pagopa.transactions.exceptions.TransactionNotFoundException + - it.pagopa.transactions.exceptions.AlreadyProcessedException + - it.pagopa.transactions.exceptions.NotImplementedException + - it.pagopa.transactions.exceptions.InvalidRequestException + - it.pagopa.transactions.exceptions.TransactionAmountMismatchException + - it.pagopa.transactions.exceptions.NodoErrorException + - it.pagopa.transactions.exceptions.InvalidNodoResponseException getTransactionInfo: maxAttempts: 3 waitDuration: 2s enableExponentialBackoff: false + ignoreExceptions: + - it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException + - it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException + - it.pagopa.transactions.exceptions.TransactionNotFoundException + - it.pagopa.transactions.exceptions.AlreadyProcessedException + - it.pagopa.transactions.exceptions.NotImplementedException + - it.pagopa.transactions.exceptions.InvalidRequestException + - it.pagopa.transactions.exceptions.TransactionAmountMismatchException + - it.pagopa.transactions.exceptions.NodoErrorException + - it.pagopa.transactions.exceptions.InvalidNodoResponseException requestTransactionAuthorization: maxAttempts: 3 waitDuration: 2s enableExponentialBackoff: false + ignoreExceptions: + - it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException + - it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException + - it.pagopa.transactions.exceptions.TransactionNotFoundException + - it.pagopa.transactions.exceptions.AlreadyProcessedException + - it.pagopa.transactions.exceptions.NotImplementedException + - it.pagopa.transactions.exceptions.InvalidRequestException + - it.pagopa.transactions.exceptions.TransactionAmountMismatchException + - it.pagopa.transactions.exceptions.NodoErrorException + - it.pagopa.transactions.exceptions.InvalidNodoResponseException updateTransactionAuthorization: maxAttempts: 3 waitDuration: 2s enableExponentialBackoff: false - activateTransaction: + ignoreExceptions: + - it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException + - it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException + - it.pagopa.transactions.exceptions.TransactionNotFoundException + - it.pagopa.transactions.exceptions.AlreadyProcessedException + - it.pagopa.transactions.exceptions.NotImplementedException + - it.pagopa.transactions.exceptions.InvalidRequestException + - it.pagopa.transactions.exceptions.TransactionAmountMismatchException + - it.pagopa.transactions.exceptions.NodoErrorException + - it.pagopa.transactions.exceptions.InvalidNodoResponseException + cancelTransaction: + maxAttempts: 3 + waitDuration: 2s + enableExponentialBackoff: false + ignoreExceptions: + - it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException + - it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException + - it.pagopa.transactions.exceptions.TransactionNotFoundException + - it.pagopa.transactions.exceptions.AlreadyProcessedException + - it.pagopa.transactions.exceptions.NotImplementedException + - it.pagopa.transactions.exceptions.InvalidRequestException + - it.pagopa.transactions.exceptions.TransactionAmountMismatchException + - it.pagopa.transactions.exceptions.NodoErrorException + - it.pagopa.transactions.exceptions.InvalidNodoResponseException + addUserReceipt: maxAttempts: 3 waitDuration: 2s enableExponentialBackoff: false + ignoreExceptions: + - it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException + - it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException + - it.pagopa.transactions.exceptions.TransactionNotFoundException + - it.pagopa.transactions.exceptions.AlreadyProcessedException + - it.pagopa.transactions.exceptions.NotImplementedException + - it.pagopa.transactions.exceptions.InvalidRequestException + - it.pagopa.transactions.exceptions.TransactionAmountMismatchException + - it.pagopa.transactions.exceptions.NodoErrorException + - it.pagopa.transactions.exceptions.InvalidNodoResponseException resilience4j.circuitbreaker: configs: @@ -37,10 +97,38 @@ resilience4j.circuitbreaker: instances: transactions-backend: baseConfig: default + ignoreExceptions: + - it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException + - it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException + - it.pagopa.transactions.exceptions.TransactionNotFoundException + - it.pagopa.transactions.exceptions.AlreadyProcessedException + - it.pagopa.transactions.exceptions.NotImplementedException + - it.pagopa.transactions.exceptions.InvalidRequestException + - it.pagopa.transactions.exceptions.TransactionAmountMismatchException + - it.pagopa.transactions.exceptions.NodoErrorException + - it.pagopa.transactions.exceptions.InvalidNodoResponseException node-backend: baseConfig: default ignoreExceptions: + - it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException + - it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException + - it.pagopa.transactions.exceptions.TransactionNotFoundException + - it.pagopa.transactions.exceptions.AlreadyProcessedException + - it.pagopa.transactions.exceptions.NotImplementedException + - it.pagopa.transactions.exceptions.InvalidRequestException + - it.pagopa.transactions.exceptions.TransactionAmountMismatchException - it.pagopa.transactions.exceptions.NodoErrorException + - it.pagopa.transactions.exceptions.InvalidNodoResponseException ecommerce-db: baseConfig: default + ignoreExceptions: + - it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException + - it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException + - it.pagopa.transactions.exceptions.TransactionNotFoundException + - it.pagopa.transactions.exceptions.AlreadyProcessedException + - it.pagopa.transactions.exceptions.NotImplementedException + - it.pagopa.transactions.exceptions.InvalidRequestException + - it.pagopa.transactions.exceptions.TransactionAmountMismatchException + - it.pagopa.transactions.exceptions.NodoErrorException + - it.pagopa.transactions.exceptions.InvalidNodoResponseException diff --git a/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java b/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java index 350257d21..073d7996c 100644 --- a/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java +++ b/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java @@ -1,28 +1,35 @@ package it.pagopa.transactions.services.v1; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import io.github.resilience4j.circuitbreaker.CallNotPermittedException; import io.github.resilience4j.circuitbreaker.CircuitBreaker; import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import io.github.resilience4j.retry.Retry; +import io.github.resilience4j.retry.RetryRegistry; import it.pagopa.ecommerce.commons.documents.PaymentNotice; import it.pagopa.ecommerce.commons.documents.PaymentTransferInformation; import it.pagopa.ecommerce.commons.documents.v1.TransactionActivatedData; import it.pagopa.ecommerce.commons.documents.v1.TransactionActivatedEvent; +import it.pagopa.ecommerce.commons.domain.PaymentToken; import it.pagopa.ecommerce.commons.domain.TransactionId; import it.pagopa.ecommerce.commons.v1.TransactionTestUtils; import it.pagopa.generated.transactions.model.CtFaultBean; -import it.pagopa.generated.transactions.server.model.ClientIdDto; -import it.pagopa.generated.transactions.server.model.NewTransactionRequestDto; -import it.pagopa.generated.transactions.server.model.PartyConfigurationFaultDto; -import it.pagopa.generated.transactions.server.model.PaymentNoticeInfoDto; +import it.pagopa.generated.transactions.server.model.*; import it.pagopa.transactions.commands.handlers.v1.TransactionActivateHandler; -import it.pagopa.transactions.exceptions.InvalidNodoResponseException; -import it.pagopa.transactions.exceptions.NodoErrorException; +import it.pagopa.transactions.exceptions.*; +import it.pagopa.transactions.repositories.TransactionsViewRepository; +import it.pagopa.transactions.utils.TransactionsUtils; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestMethodOrder; import org.junit.jupiter.api.parallel.Execution; import org.junit.jupiter.api.parallel.ExecutionMode; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; @@ -31,8 +38,11 @@ import reactor.core.publisher.Mono; import reactor.test.StepVerifier; -import java.util.List; -import java.util.UUID; +import java.io.File; +import java.io.IOException; +import java.util.*; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; import static it.pagopa.ecommerce.commons.v1.TransactionTestUtils.EMAIL_STRING; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -54,9 +64,315 @@ class CircuitBreakerTest { @MockBean private TransactionActivateHandler transactionActivateHandlerV1; + @MockBean + private TransactionsViewRepository transactionsViewRepository; + @MockBean + private TransactionsUtils transactionsUtils; + @Autowired private CircuitBreakerRegistry circuitBreakerRegistry; + @Autowired + private RetryRegistry retryRegistry; + + private static final JsonNode resilience4jConfiguration; + + private static final Map exceptionMapper = Map.of( + "it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException", + new UnsatisfiablePspRequestException( + new PaymentToken(""), + RequestAuthorizationRequestDto.LanguageEnum.IT, + 0 + ), + "it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException", + new PaymentNoticeAllCCPMismatchException("rptId", true, true), + "it.pagopa.transactions.exceptions.TransactionNotFoundException", + new TransactionNotFoundException(""), + "it.pagopa.transactions.exceptions.AlreadyProcessedException", + new AlreadyProcessedException(new TransactionId(TransactionTestUtils.TRANSACTION_ID)), + "it.pagopa.transactions.exceptions.NotImplementedException", + new NotImplementedException(""), + "it.pagopa.transactions.exceptions.InvalidRequestException", + new InvalidRequestException(""), + "it.pagopa.transactions.exceptions.TransactionAmountMismatchException", + new TransactionAmountMismatchException(10, 11), + "it.pagopa.transactions.exceptions.NodoErrorException", + new NodoErrorException(new CtFaultBean()), + "it.pagopa.transactions.exceptions.InvalidNodoResponseException", + new InvalidNodoResponseException("") + ); + + static { + ObjectMapper mapper = new ObjectMapper(new YAMLFactory()); + try { + resilience4jConfiguration = mapper.readTree(new File("./src/main/resources/application.yml")); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private static Stream getIgnoredExceptionsForRetry(String retryInstanceName) { + return StreamSupport.stream( + Spliterators.spliteratorUnknownSize( + resilience4jConfiguration + .get("resilience4j.retry") + .get("instances") + .get(retryInstanceName) + .get("ignoreExceptions") + .elements(), + Spliterator.ORDERED + ), + false + ) + .map(json -> { + String exceptionName = json.asText(); + return Arguments.of( + Optional + .ofNullable(exceptionMapper.get(exceptionName)) + .orElseThrow( + () -> new RuntimeException( + "No mapping found for exception: %s".formatted(exceptionName) + ) + ), + retryInstanceName + ); + } + + ); + } + + private static Stream getIgnoredExceptionForNewTransactionRetry() { + return getIgnoredExceptionsForRetry("newTransaction"); + } + + private static Stream getIgnoredExceptionForGetTransactionInfoRetry() { + return getIgnoredExceptionsForRetry("getTransactionInfo"); + } + + private static Stream getIgnoredExceptionForRequestTransactionAuthorizationRetry() { + return getIgnoredExceptionsForRetry("requestTransactionAuthorization"); + } + + private static Stream getIgnoredExceptionForUpdateTransactionAuthorizationRetry() { + return getIgnoredExceptionsForRetry("updateTransactionAuthorization"); + } + + private static Stream getIgnoredExceptionForCancelTransactionRetry() { + return getIgnoredExceptionsForRetry("cancelTransaction"); + } + + private static Stream getIgnoredExceptionForAddUserReceiptRetry() { + return getIgnoredExceptionsForRetry("addUserReceipt"); + } + + @ParameterizedTest + @MethodSource("getIgnoredExceptionForNewTransactionRetry") + @Order(0) + void shouldNotPerformRetryForExcludedException_newTransactionRetry( + Exception thrownException, + String retryInstanceName + ) { + Retry retry = retryRegistry.retry(retryInstanceName); + long expectedFailedCallsWithoutRetryAttempt = retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + + 1; + ClientIdDto clientIdDto = ClientIdDto.CHECKOUT; + UUID TEST_CCP = UUID.randomUUID(); + UUID TRANSACTION_ID = UUID.randomUUID(); + + NewTransactionRequestDto transactionRequestDto = new NewTransactionRequestDto() + .email(EMAIL_STRING) + .addPaymentNoticesItem(new PaymentNoticeInfoDto().rptId(TransactionTestUtils.RPT_ID).amount(10)); + + TransactionActivatedData transactionActivatedData = new TransactionActivatedData(); + transactionActivatedData.setEmail(TransactionTestUtils.EMAIL); + transactionActivatedData + .setPaymentNotices( + List.of( + new PaymentNotice( + TransactionTestUtils.PAYMENT_TOKEN, + null, + "desc", + 0, + TEST_CCP.toString(), + List.of(new PaymentTransferInformation("77777777777", false, 0, null)), + false + ) + ) + ); + + TransactionActivatedEvent transactionActivatedEvent = new TransactionActivatedEvent( + new TransactionId(TRANSACTION_ID).value(), + transactionActivatedData + ); + + /* + * Preconditions + */ + Mockito.when(transactionActivateHandlerV1.handle(any())).thenReturn(Mono.error(thrownException)); + StepVerifier + .create( + transactionsService.newTransaction( + transactionRequestDto, + clientIdDto, + new TransactionId(transactionActivatedEvent.getTransactionId()) + ) + ) + .expectError(thrownException.getClass()) + .verify(); + assertEquals( + expectedFailedCallsWithoutRetryAttempt, + retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + ); + } + + @ParameterizedTest + @MethodSource("getIgnoredExceptionForGetTransactionInfoRetry") + @Order(0) + void shouldNotPerformRetryForExcludedException_getTransactionInfoRetry( + Exception thrownException, + String retryInstanceName + ) { + Retry retry = retryRegistry.retry(retryInstanceName); + long expectedFailedCallsWithoutRetryAttempt = retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + + 1; + + /* + * Preconditions + */ + Mockito.when(transactionsViewRepository.findById(any(String.class))) + .thenReturn(Mono.error(thrownException)); + + StepVerifier + .create( + transactionsService.getTransactionInfo("transactionId") + ) + .expectError(thrownException.getClass()) + .verify(); + assertEquals( + expectedFailedCallsWithoutRetryAttempt, + retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + ); + } + + @ParameterizedTest + @MethodSource("getIgnoredExceptionForRequestTransactionAuthorizationRetry") + @Order(0) + void shouldNotPerformRetryForExcludedException_requestTransactionAuthorizationRetry( + Exception thrownException, + String retryInstanceName + ) { + Retry retry = retryRegistry.retry(retryInstanceName); + long expectedFailedCallsWithoutRetryAttempt = retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + + 1; + + /* + * Preconditions + */ + Mockito.when(transactionsViewRepository.findById(any(String.class))) + .thenReturn(Mono.error(thrownException)); + + StepVerifier + .create( + transactionsService.requestTransactionAuthorization( + "transactionId", + "", + new RequestAuthorizationRequestDto() + ) + ) + .expectError(thrownException.getClass()) + .verify(); + assertEquals( + expectedFailedCallsWithoutRetryAttempt, + retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + ); + } + + @ParameterizedTest + @MethodSource("getIgnoredExceptionForUpdateTransactionAuthorizationRetry") + @Order(0) + void shouldNotPerformRetryForExcludedException_updateTransactionAuthorizationRetry( + Exception thrownException, + String retryInstanceName + ) { + Retry retry = retryRegistry.retry(retryInstanceName); + long expectedFailedCallsWithoutRetryAttempt = retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + + 1; + + /* + * Preconditions + */ + Mockito.when(transactionsUtils.reduceEvents(any(), any(), any(), any())) + .thenReturn(Mono.error(thrownException)); + + StepVerifier + .create( + transactionsService + .updateTransactionAuthorization(UUID.randomUUID(), new UpdateAuthorizationRequestDto()) + ) + .expectError(thrownException.getClass()) + .verify(); + assertEquals( + expectedFailedCallsWithoutRetryAttempt, + retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + ); + } + + @ParameterizedTest + @MethodSource("getIgnoredExceptionForCancelTransactionRetry") + @Order(0) + void shouldNotPerformRetryForExcludedException_cancelTransactionRetry( + Exception thrownException, + String retryInstanceName + ) { + Retry retry = retryRegistry.retry(retryInstanceName); + long expectedFailedCallsWithoutRetryAttempt = retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + + 1; + + /* + * Preconditions + */ + Mockito.when(transactionsViewRepository.findById(any(String.class))).thenReturn(Mono.error(thrownException)); + + StepVerifier + .create( + transactionsService.cancelTransaction("") + ) + .expectError(thrownException.getClass()) + .verify(); + assertEquals( + expectedFailedCallsWithoutRetryAttempt, + retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + ); + } + + @ParameterizedTest + @MethodSource("getIgnoredExceptionForAddUserReceiptRetry") + @Order(0) + void shouldNotPerformRetryForExcludedException_AddUserReceiptRetry( + Exception thrownException, + String retryInstanceName + ) { + Retry retry = retryRegistry.retry(retryInstanceName); + long expectedFailedCallsWithoutRetryAttempt = retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + + 1; + + /* + * Preconditions + */ + Mockito.when(transactionsViewRepository.findById(any(String.class))).thenReturn(Mono.error(thrownException)); + + StepVerifier + .create( + transactionsService.addUserReceipt("", new AddUserReceiptRequestDto()) + ) + .expectError(thrownException.getClass()) + .verify(); + assertEquals( + expectedFailedCallsWithoutRetryAttempt, + retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + ); + } + @Test @Order(1) void shouldNotOpenCircuitBreakerForNodoErrorException() { @@ -116,7 +432,11 @@ void shouldNotOpenCircuitBreakerForNodoErrorException() { @Test @Order(2) - void shouldOpenCircuitBreakerForNotExcludedException() { + void shouldOpenCircuitBreakerForNotExcludedExceptionPerformingRetry() { + Retry retry = retryRegistry.retry("newTransaction"); + long expectedFailedCallsWithoutRetryAttempt = retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt(); + long expectedFailedCallsWithRetryAttempt = retry.getMetrics().getNumberOfFailedCallsWithRetryAttempt() + 1; + ClientIdDto clientIdDto = ClientIdDto.CHECKOUT; UUID TEST_CCP = UUID.randomUUID(); UUID TRANSACTION_ID = UUID.randomUUID(); @@ -151,7 +471,7 @@ void shouldOpenCircuitBreakerForNotExcludedException() { * Preconditions */ Mockito.when(transactionActivateHandlerV1.handle(any())) - .thenReturn(Mono.error(new InvalidNodoResponseException("Invalid response received"))); + .thenReturn(Mono.error(new RuntimeException("Invalid response received"))); StepVerifier .create( @@ -165,6 +485,11 @@ void shouldOpenCircuitBreakerForNotExcludedException() { .verify(); CircuitBreaker circuitBreaker = circuitBreakerRegistry.circuitBreaker("node-backend"); assertEquals(CircuitBreaker.State.OPEN, circuitBreaker.getState()); + assertEquals( + expectedFailedCallsWithoutRetryAttempt, + retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + ); + assertEquals(expectedFailedCallsWithRetryAttempt, retry.getMetrics().getNumberOfFailedCallsWithRetryAttempt()); } From 441e0885e7942d8b327c8e6d8ffee004c33a4059 Mon Sep 17 00:00:00 2001 From: Pietro Tota Date: Thu, 30 Nov 2023 11:00:44 +0100 Subject: [PATCH 2/6] fix: tests --- .../services/v2/CircuitBreakerTest.java | 172 ++++++++++++++++-- 1 file changed, 159 insertions(+), 13 deletions(-) diff --git a/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java b/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java index b226bd9ca..6ba61e35d 100644 --- a/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java +++ b/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java @@ -1,29 +1,37 @@ package it.pagopa.transactions.services.v2; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import io.github.resilience4j.circuitbreaker.CallNotPermittedException; import io.github.resilience4j.circuitbreaker.CircuitBreaker; import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import io.github.resilience4j.retry.Retry; +import io.github.resilience4j.retry.RetryRegistry; import it.pagopa.ecommerce.commons.documents.PaymentNotice; import it.pagopa.ecommerce.commons.documents.PaymentTransferInformation; -import it.pagopa.ecommerce.commons.documents.v1.TransactionActivatedData; -import it.pagopa.ecommerce.commons.documents.v1.TransactionActivatedEvent; +import it.pagopa.ecommerce.commons.documents.v2.TransactionActivatedData; +import it.pagopa.ecommerce.commons.documents.v2.TransactionActivatedEvent; +import it.pagopa.ecommerce.commons.domain.PaymentToken; import it.pagopa.ecommerce.commons.domain.TransactionId; -import it.pagopa.ecommerce.commons.v1.TransactionTestUtils; +import it.pagopa.ecommerce.commons.v2.TransactionTestUtils; import it.pagopa.generated.transactions.model.CtFaultBean; -import it.pagopa.generated.transactions.server.model.ClientIdDto; -import it.pagopa.generated.transactions.server.model.NewTransactionRequestDto; -import it.pagopa.generated.transactions.server.model.PartyConfigurationFaultDto; -import it.pagopa.generated.transactions.server.model.PaymentNoticeInfoDto; +import it.pagopa.generated.transactions.server.model.RequestAuthorizationRequestDto; +import it.pagopa.generated.transactions.v2.server.model.ClientIdDto; +import it.pagopa.generated.transactions.v2.server.model.NewTransactionRequestDto; +import it.pagopa.generated.transactions.v2.server.model.PartyConfigurationFaultDto; +import it.pagopa.generated.transactions.v2.server.model.PaymentNoticeInfoDto; import it.pagopa.transactions.commands.handlers.v2.TransactionActivateHandler; -import it.pagopa.transactions.exceptions.InvalidNodoResponseException; -import it.pagopa.transactions.exceptions.NodoErrorException; -import it.pagopa.transactions.services.v1.TransactionsService; +import it.pagopa.transactions.exceptions.*; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestMethodOrder; import org.junit.jupiter.api.parallel.Execution; import org.junit.jupiter.api.parallel.ExecutionMode; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; @@ -32,8 +40,11 @@ import reactor.core.publisher.Mono; import reactor.test.StepVerifier; -import java.util.List; -import java.util.UUID; +import java.io.File; +import java.io.IOException; +import java.util.*; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; import static it.pagopa.ecommerce.commons.v1.TransactionTestUtils.EMAIL_STRING; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -53,6 +64,141 @@ class CircuitBreakerTest { @Autowired private CircuitBreakerRegistry circuitBreakerRegistry; + @Autowired + private RetryRegistry retryRegistry; + + private static final JsonNode resilience4jConfiguration; + + private static final Map exceptionMapper = Map.of( + "it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException", + new UnsatisfiablePspRequestException( + new PaymentToken(""), + RequestAuthorizationRequestDto.LanguageEnum.IT, + 0 + ), + "it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException", + new PaymentNoticeAllCCPMismatchException("rptId", true, true), + "it.pagopa.transactions.exceptions.TransactionNotFoundException", + new TransactionNotFoundException(""), + "it.pagopa.transactions.exceptions.AlreadyProcessedException", + new AlreadyProcessedException( + new TransactionId(it.pagopa.ecommerce.commons.v1.TransactionTestUtils.TRANSACTION_ID) + ), + "it.pagopa.transactions.exceptions.NotImplementedException", + new NotImplementedException(""), + "it.pagopa.transactions.exceptions.InvalidRequestException", + new InvalidRequestException(""), + "it.pagopa.transactions.exceptions.TransactionAmountMismatchException", + new TransactionAmountMismatchException(10, 11), + "it.pagopa.transactions.exceptions.NodoErrorException", + new NodoErrorException(new CtFaultBean()), + "it.pagopa.transactions.exceptions.InvalidNodoResponseException", + new InvalidNodoResponseException("") + ); + + static { + ObjectMapper mapper = new ObjectMapper(new YAMLFactory()); + try { + resilience4jConfiguration = mapper.readTree(new File("./src/main/resources/application.yml")); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private static Stream getIgnoredExceptionsForRetry(String retryInstanceName) { + return StreamSupport.stream( + Spliterators.spliteratorUnknownSize( + resilience4jConfiguration + .get("resilience4j.retry") + .get("instances") + .get(retryInstanceName) + .get("ignoreExceptions") + .elements(), + Spliterator.ORDERED + ), + false + ) + .map(json -> { + String exceptionName = json.asText(); + return Arguments.of( + Optional + .ofNullable(exceptionMapper.get(exceptionName)) + .orElseThrow( + () -> new RuntimeException( + "No mapping found for exception: %s".formatted(exceptionName) + ) + ), + retryInstanceName + ); + } + + ); + } + + private static Stream getIgnoredExceptionForNewTransactionRetry() { + return getIgnoredExceptionsForRetry("newTransaction"); + } + + @ParameterizedTest + @MethodSource("getIgnoredExceptionForNewTransactionRetry") + @Order(0) + void shouldNotPerformRetryForExcludedException_newTransactionRetry( + Exception thrownException, + String retryInstanceName + ) { + Retry retry = retryRegistry.retry(retryInstanceName); + long expectedFailedCallsWithoutRetryAttempt = retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + + 1; + ClientIdDto clientIdDto = ClientIdDto.CHECKOUT; + UUID TEST_CCP = UUID.randomUUID(); + UUID TRANSACTION_ID = UUID.randomUUID(); + + NewTransactionRequestDto transactionRequestDto = new NewTransactionRequestDto() + .email(EMAIL_STRING) + .addPaymentNoticesItem(new PaymentNoticeInfoDto().rptId(TransactionTestUtils.RPT_ID).amount(10)); + + TransactionActivatedData transactionActivatedData = new TransactionActivatedData(); + transactionActivatedData.setEmail(it.pagopa.ecommerce.commons.v1.TransactionTestUtils.EMAIL); + transactionActivatedData + .setPaymentNotices( + List.of( + new PaymentNotice( + it.pagopa.ecommerce.commons.v1.TransactionTestUtils.PAYMENT_TOKEN, + null, + "desc", + 0, + TEST_CCP.toString(), + List.of(new PaymentTransferInformation("77777777777", false, 0, null)), + false + ) + ) + ); + + TransactionActivatedEvent transactionActivatedEvent = new TransactionActivatedEvent( + new TransactionId(TRANSACTION_ID).value(), + transactionActivatedData + ); + + /* + * Preconditions + */ + Mockito.when(transactionActivateHandlerV2.handle(any())).thenReturn(Mono.error(thrownException)); + StepVerifier + .create( + transactionsService.newTransaction( + transactionRequestDto, + clientIdDto, + new TransactionId(transactionActivatedEvent.getTransactionId()) + ) + ) + .expectError(thrownException.getClass()) + .verify(); + assertEquals( + expectedFailedCallsWithoutRetryAttempt, + retry.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt() + ); + } + @Test @Order(1) void shouldNotOpenCircuitBreakerForNodoErrorException() { @@ -147,7 +293,7 @@ void shouldOpenCircuitBreakerForNotExcludedException() { * Preconditions */ Mockito.when(transactionActivateHandlerV2.handle(any())) - .thenReturn(Mono.error(new InvalidNodoResponseException("Invalid response received"))); + .thenReturn(Mono.error(new RuntimeException("Invalid response received"))); StepVerifier .create( From d1f2161847856f1470f4bdf094739a9110ea0700 Mon Sep 17 00:00:00 2001 From: Pietro Tota <115724836+pietro-tota@users.noreply.github.com> Date: Thu, 30 Nov 2023 15:00:08 +0100 Subject: [PATCH 3/6] Update src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java Co-authored-by: Giovanni Berti --- .../it/pagopa/transactions/services/v1/CircuitBreakerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java b/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java index 073d7996c..6b392386f 100644 --- a/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java +++ b/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java @@ -131,7 +131,7 @@ private static Stream getIgnoredExceptionsForRetry(String retryInstan .ofNullable(exceptionMapper.get(exceptionName)) .orElseThrow( () -> new RuntimeException( - "No mapping found for exception: %s".formatted(exceptionName) + "Missing exception instance in test suite inside map `exceptionMapper` for class: %s".formatted(exceptionName) ) ), retryInstanceName From 670ebb1a7fea406c49827246dbf53643f7954fd1 Mon Sep 17 00:00:00 2001 From: Pietro Tota Date: Thu, 30 Nov 2023 15:03:31 +0100 Subject: [PATCH 4/6] fix: renaming local variable --- .../transactions/services/v1/CircuitBreakerTest.java | 7 ++++--- .../transactions/services/v2/CircuitBreakerTest.java | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java b/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java index 6b392386f..3cc38cdf7 100644 --- a/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java +++ b/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java @@ -124,14 +124,15 @@ private static Stream getIgnoredExceptionsForRetry(String retryInstan ), false ) - .map(json -> { - String exceptionName = json.asText(); + .map(ignoredException -> { + String exceptionName = ignoredException.asText(); return Arguments.of( Optional .ofNullable(exceptionMapper.get(exceptionName)) .orElseThrow( () -> new RuntimeException( - "Missing exception instance in test suite inside map `exceptionMapper` for class: %s".formatted(exceptionName) + "Missing exception instance in test suite inside map `exceptionMapper` for class: %s" + .formatted(exceptionName) ) ), retryInstanceName diff --git a/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java b/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java index 6ba61e35d..c06eb9778 100644 --- a/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java +++ b/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java @@ -118,8 +118,8 @@ private static Stream getIgnoredExceptionsForRetry(String retryInstan ), false ) - .map(json -> { - String exceptionName = json.asText(); + .map(ignoredException -> { + String exceptionName = ignoredException.asText(); return Arguments.of( Optional .ofNullable(exceptionMapper.get(exceptionName)) From 84942ffc696f5d9a21bdc6b7caeedff834b64d3e Mon Sep 17 00:00:00 2001 From: Pietro Tota Date: Thu, 30 Nov 2023 15:07:47 +0100 Subject: [PATCH 5/6] fix: code review --- .../services/v1/CircuitBreakerTest.java | 15 ++++----------- .../services/v2/CircuitBreakerTest.java | 15 ++++----------- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java b/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java index 3cc38cdf7..dd3523be7 100644 --- a/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java +++ b/src/test/java/it/pagopa/transactions/services/v1/CircuitBreakerTest.java @@ -41,6 +41,8 @@ import java.io.File; import java.io.IOException; import java.util.*; +import java.util.function.Function; +import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.stream.StreamSupport; @@ -77,30 +79,21 @@ class CircuitBreakerTest { private static final JsonNode resilience4jConfiguration; - private static final Map exceptionMapper = Map.of( - "it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException", + private static final Map exceptionMapper = Stream.of( new UnsatisfiablePspRequestException( new PaymentToken(""), RequestAuthorizationRequestDto.LanguageEnum.IT, 0 ), - "it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException", new PaymentNoticeAllCCPMismatchException("rptId", true, true), - "it.pagopa.transactions.exceptions.TransactionNotFoundException", new TransactionNotFoundException(""), - "it.pagopa.transactions.exceptions.AlreadyProcessedException", new AlreadyProcessedException(new TransactionId(TransactionTestUtils.TRANSACTION_ID)), - "it.pagopa.transactions.exceptions.NotImplementedException", new NotImplementedException(""), - "it.pagopa.transactions.exceptions.InvalidRequestException", new InvalidRequestException(""), - "it.pagopa.transactions.exceptions.TransactionAmountMismatchException", new TransactionAmountMismatchException(10, 11), - "it.pagopa.transactions.exceptions.NodoErrorException", new NodoErrorException(new CtFaultBean()), - "it.pagopa.transactions.exceptions.InvalidNodoResponseException", new InvalidNodoResponseException("") - ); + ).collect(Collectors.toMap(exception -> exception.getClass().getCanonicalName(), Function.identity())); static { ObjectMapper mapper = new ObjectMapper(new YAMLFactory()); diff --git a/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java b/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java index c06eb9778..fcbc8d576 100644 --- a/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java +++ b/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java @@ -43,6 +43,8 @@ import java.io.File; import java.io.IOException; import java.util.*; +import java.util.function.Function; +import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.stream.StreamSupport; @@ -69,32 +71,23 @@ class CircuitBreakerTest { private static final JsonNode resilience4jConfiguration; - private static final Map exceptionMapper = Map.of( - "it.pagopa.transactions.exceptions.UnsatisfiablePspRequestException", + private static final Map exceptionMapper = Stream.of( new UnsatisfiablePspRequestException( new PaymentToken(""), RequestAuthorizationRequestDto.LanguageEnum.IT, 0 ), - "it.pagopa.transactions.exceptions.PaymentNoticeAllCCPMismatchException", new PaymentNoticeAllCCPMismatchException("rptId", true, true), - "it.pagopa.transactions.exceptions.TransactionNotFoundException", new TransactionNotFoundException(""), - "it.pagopa.transactions.exceptions.AlreadyProcessedException", new AlreadyProcessedException( new TransactionId(it.pagopa.ecommerce.commons.v1.TransactionTestUtils.TRANSACTION_ID) ), - "it.pagopa.transactions.exceptions.NotImplementedException", new NotImplementedException(""), - "it.pagopa.transactions.exceptions.InvalidRequestException", new InvalidRequestException(""), - "it.pagopa.transactions.exceptions.TransactionAmountMismatchException", new TransactionAmountMismatchException(10, 11), - "it.pagopa.transactions.exceptions.NodoErrorException", new NodoErrorException(new CtFaultBean()), - "it.pagopa.transactions.exceptions.InvalidNodoResponseException", new InvalidNodoResponseException("") - ); + ).collect(Collectors.toMap(exception -> exception.getClass().getCanonicalName(), Function.identity())); static { ObjectMapper mapper = new ObjectMapper(new YAMLFactory()); From 0c550d54aeb0f3e19fcdd9cc32d22516bf539f9d Mon Sep 17 00:00:00 2001 From: Pietro Tota <115724836+pietro-tota@users.noreply.github.com> Date: Fri, 1 Dec 2023 10:43:40 +0100 Subject: [PATCH 6/6] Update src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java Co-authored-by: Giovanni Berti --- .../it/pagopa/transactions/services/v2/CircuitBreakerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java b/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java index fcbc8d576..4c7597f0e 100644 --- a/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java +++ b/src/test/java/it/pagopa/transactions/services/v2/CircuitBreakerTest.java @@ -118,7 +118,7 @@ private static Stream getIgnoredExceptionsForRetry(String retryInstan .ofNullable(exceptionMapper.get(exceptionName)) .orElseThrow( () -> new RuntimeException( - "No mapping found for exception: %s".formatted(exceptionName) + "Missing exception instance in test suite inside map `exceptionMapper` for class: %s" ) ), retryInstanceName