From 961d4d1d6f14a14f797926619e902652079e4083 Mon Sep 17 00:00:00 2001 From: Vitolo-Andrea Date: Wed, 13 Nov 2024 09:45:02 +0100 Subject: [PATCH] Tpp Connector Error Fix --- .../citizen/service/CitizenServiceImpl.java | 33 +++++++------------ .../CitizenConsentValidationService.java | 2 -- .../CitizenConsentValidationServiceImpl.java | 20 ++++------- .../citizen/service/CitizenServiceTest.java | 5 ++- ...tizenConsentValidationServiceImplTest.java | 32 +++++++++--------- 5 files changed, 37 insertions(+), 55 deletions(-) diff --git a/src/main/java/it/gov/pagopa/onboarding/citizen/service/CitizenServiceImpl.java b/src/main/java/it/gov/pagopa/onboarding/citizen/service/CitizenServiceImpl.java index e997977..c0c45a5 100644 --- a/src/main/java/it/gov/pagopa/onboarding/citizen/service/CitizenServiceImpl.java +++ b/src/main/java/it/gov/pagopa/onboarding/citizen/service/CitizenServiceImpl.java @@ -68,28 +68,17 @@ public Mono updateTppState(String fiscalCode, String tppId, b Utils.createSHA256(fiscalCode), inputSanify(tppId), tppState); return tppConnector.get(tppId) - .switchIfEmpty(Mono.error(exceptionMap.throwException(ExceptionName.TPP_NOT_FOUND, ExceptionMessage.TPP_NOT_FOUND))) - .flatMap(tppResponse -> { - if (!validationService.isTppValid(tppResponse)) { - return Mono.error(exceptionMap.throwException(ExceptionName.TPP_NOT_FOUND, ExceptionMessage.TPP_NOT_FOUND)); - } - - return citizenRepository.findByFiscalCodeAndTppId(fiscalCode, tppId) - .switchIfEmpty(Mono.error(exceptionMap.throwException - (ExceptionName.CITIZEN_NOT_ONBOARDED, "Citizen consent not founded during update state process"))) - .flatMap(citizenConsent -> { - ConsentDetails consentDetails = citizenConsent.getConsents().get(tppId); - if (consentDetails != null) { - consentDetails.setTppState(tppState); - } else { - return Mono.error(exceptionMap.throwException - (ExceptionName.CITIZEN_NOT_ONBOARDED, "ConsentDetails is null for this tppId")); - } - return citizenRepository.save(citizenConsent); - }) - .map(mapperToDTO::map) - .doOnSuccess(savedConsent -> log.info("[EMD][CITIZEN][UPDATE-CHANNEL-STATE] Updated state")); - }); + .onErrorMap(error ->exceptionMap.throwException(ExceptionName.TPP_NOT_FOUND, ExceptionMessage.TPP_NOT_FOUND)) + .flatMap(tppResponse -> citizenRepository.findByFiscalCodeAndTppId(fiscalCode, tppId) + .switchIfEmpty(Mono.error(exceptionMap.throwException + (ExceptionName.CITIZEN_NOT_ONBOARDED, "Citizen consent not founded during update state process"))) + .flatMap(citizenConsent -> { + ConsentDetails consentDetails = citizenConsent.getConsents().get(tppId); + consentDetails.setTppState(tppState); + return citizenRepository.save(citizenConsent); + }) + .map(mapperToDTO::map) + .doOnSuccess(savedConsent -> log.info("[EMD][CITIZEN][UPDATE-CHANNEL-STATE] Updated state"))); } @Override diff --git a/src/main/java/it/gov/pagopa/onboarding/citizen/validation/CitizenConsentValidationService.java b/src/main/java/it/gov/pagopa/onboarding/citizen/validation/CitizenConsentValidationService.java index 46830e9..8cbabed 100644 --- a/src/main/java/it/gov/pagopa/onboarding/citizen/validation/CitizenConsentValidationService.java +++ b/src/main/java/it/gov/pagopa/onboarding/citizen/validation/CitizenConsentValidationService.java @@ -1,7 +1,6 @@ package it.gov.pagopa.onboarding.citizen.validation; import it.gov.pagopa.onboarding.citizen.dto.CitizenConsentDTO; -import it.gov.pagopa.onboarding.citizen.dto.TppDTO; import it.gov.pagopa.onboarding.citizen.model.CitizenConsent; import reactor.core.publisher.Mono; @@ -11,5 +10,4 @@ public interface CitizenConsentValidationService { Mono validateTppAndSaveConsent(String fiscalCode, String tppId, CitizenConsent citizenConsent); - boolean isTppValid(TppDTO tppResponse); } diff --git a/src/main/java/it/gov/pagopa/onboarding/citizen/validation/CitizenConsentValidationServiceImpl.java b/src/main/java/it/gov/pagopa/onboarding/citizen/validation/CitizenConsentValidationServiceImpl.java index 8d04bf2..d2e8fd5 100644 --- a/src/main/java/it/gov/pagopa/onboarding/citizen/validation/CitizenConsentValidationServiceImpl.java +++ b/src/main/java/it/gov/pagopa/onboarding/citizen/validation/CitizenConsentValidationServiceImpl.java @@ -3,9 +3,9 @@ import it.gov.pagopa.common.utils.Utils; import it.gov.pagopa.onboarding.citizen.configuration.ExceptionMap; import it.gov.pagopa.onboarding.citizen.connector.tpp.TppConnectorImpl; +import it.gov.pagopa.onboarding.citizen.constants.CitizenConstants; import it.gov.pagopa.onboarding.citizen.constants.CitizenConstants.ExceptionName; import it.gov.pagopa.onboarding.citizen.dto.CitizenConsentDTO; -import it.gov.pagopa.onboarding.citizen.dto.TppDTO; import it.gov.pagopa.onboarding.citizen.dto.mapper.CitizenConsentObjectToDTOMapper; import it.gov.pagopa.onboarding.citizen.model.CitizenConsent; import it.gov.pagopa.onboarding.citizen.repository.CitizenRepository; @@ -47,36 +47,30 @@ public Mono handleExistingConsent(CitizenConsent existingCons @Override public Mono validateTppAndSaveConsent(String fiscalCode, String tppId, CitizenConsent citizenConsent) { return tppConnector.get(tppId) + .onErrorMap(error -> exceptionMap.throwException(ExceptionName.TPP_NOT_FOUND, CitizenConstants.ExceptionMessage.TPP_NOT_FOUND)) .flatMap(tppResponse -> { - if (isTppValid(tppResponse)) { + if (Boolean.TRUE.equals(citizenConsent.getConsents().get(tppId).getTppState())) { return citizenRepository.save(citizenConsent) .doOnSuccess(savedConsent -> { - log.info("[EMD][CREATE-CITIZEN-CONSENT] Created new citizen consent for fiscal code: {}", Utils.createSHA256(fiscalCode)); + log.info("[EMD][CREATE-CITIZEN-CONSENT] Created new citizen consent for fiscal code: {}", Utils.createSHA256(fiscalCode)); bloomFilterService.add(fiscalCode); }) - .flatMap(savedConsent -> Mono.just(mapperToDTO.map(savedConsent))); + .map(mapperToDTO::map); } else { - return Mono.error(exceptionMap.throwException(ExceptionName.TPP_NOT_FOUND, "TPP does not exist or is not active")); + return Mono.error(exceptionMap.throwException(ExceptionName.TPP_NOT_FOUND, "TPP is not active or is invalid")); } }); } - @Override - public boolean isTppValid(TppDTO tppResponse) { - return tppResponse != null && Boolean.TRUE.equals(tppResponse.getState()); - } private Mono validateTppAndUpdateConsent(CitizenConsent existingConsent, String tppId, CitizenConsent citizenConsent) { return tppConnector.get(tppId) + .onErrorMap(error -> exceptionMap.throwException(ExceptionName.TPP_NOT_FOUND, CitizenConstants.ExceptionMessage.TPP_NOT_FOUND)) .flatMap(tppResponse -> { - if (isTppValid(tppResponse)) { existingConsent.getConsents().put(tppId, citizenConsent.getConsents().get(tppId)); return citizenRepository.save(existingConsent) .doOnSuccess(savedConsent -> log.info("[EMD][CREATE-CITIZEN-CONSENT] Updated citizen consent for TPP: {}", tppId)) .flatMap(savedConsent -> Mono.just(mapperToDTO.map(savedConsent))); - } else { - return Mono.error(exceptionMap.throwException(ExceptionName.TPP_NOT_FOUND, "TPP does not exist or is not active")); - } }); } } diff --git a/src/test/java/it/gov/pagopa/onboarding/citizen/service/CitizenServiceTest.java b/src/test/java/it/gov/pagopa/onboarding/citizen/service/CitizenServiceTest.java index 4dac064..21ffa7a 100644 --- a/src/test/java/it/gov/pagopa/onboarding/citizen/service/CitizenServiceTest.java +++ b/src/test/java/it/gov/pagopa/onboarding/citizen/service/CitizenServiceTest.java @@ -170,7 +170,6 @@ void updateChannelState_Ok() { when(citizenRepository.save(any())) .thenReturn(Mono.just(CITIZEN_CONSENT)); - when(validationService.isTppValid(mockTppDTO)).thenReturn(true); CitizenConsentDTO response = citizenService.updateTppState(FISCAL_CODE, TPP_ID, TPP_STATE).block(); @@ -191,7 +190,7 @@ void updateChannelState_Ko_CitizenNotOnboarded() { when(citizenRepository.findByFiscalCodeAndTppId(FISCAL_CODE, TPP_ID)) .thenReturn(Mono.empty()); - when(validationService.isTppValid(mockTppDTO)).thenReturn(true); + Executable executable = () -> citizenService.updateTppState(FISCAL_CODE, TPP_ID, true).block(); ClientExceptionWithBody exception = assertThrows(ClientExceptionWithBody.class, executable); @@ -217,7 +216,7 @@ void updateChannelState_Ok_ConsentDetailsIsNull() { when(citizenRepository.save(any())) .thenReturn(Mono.just(citizenConsentWithConsentDetailNull)); - when(validationService.isTppValid(mockTppDTO)).thenReturn(true); + Executable executable = () -> citizenService.updateTppState(FISCAL_CODE, TPP_ID, TPP_STATE).block(); ClientExceptionWithBody exception = assertThrows(ClientExceptionWithBody.class, executable); diff --git a/src/test/java/it/gov/pagopa/onboarding/citizen/validation/CitizenConsentValidationServiceImplTest.java b/src/test/java/it/gov/pagopa/onboarding/citizen/validation/CitizenConsentValidationServiceImplTest.java index c1985fd..e4b2af1 100644 --- a/src/test/java/it/gov/pagopa/onboarding/citizen/validation/CitizenConsentValidationServiceImplTest.java +++ b/src/test/java/it/gov/pagopa/onboarding/citizen/validation/CitizenConsentValidationServiceImplTest.java @@ -117,7 +117,7 @@ void validateTppAndSaveConsent_TppInvalid() { StepVerifier.create(validationService.validateTppAndSaveConsent(fiscalCode, tppId, CITIZEN_CONSENT)) .expectErrorMatches(throwable -> throwable instanceof ClientExceptionWithBody && - "TPP does not exist or is not active".equals(throwable.getMessage()) && + "TPP is not active or is invalid".equals(throwable.getMessage()) && "TPP_NOT_FOUND".equals(((ClientExceptionWithBody) throwable).getCode())) .verify(); @@ -126,21 +126,23 @@ void validateTppAndSaveConsent_TppInvalid() { } @Test - void isTppValid_TrueWhenActive() { - TppDTO activeTpp = TPP_DTO; - activeTpp.setState(true); - assertTrue(validationService.isTppValid(activeTpp)); - } + void validateTppAndSaveConsent_TppNotFound() { + String fiscalCode = CITIZEN_CONSENT.getFiscalCode(); + String tppId = "inactiveTppId"; - @Test - void isTppValid_FalseWhenInactive() { - TppDTO inactiveTpp = TPP_DTO; - inactiveTpp.setState(false); - assertFalse(validationService.isTppValid(inactiveTpp)); - } + TppDTO inactiveTppDTO = TPP_DTO; + inactiveTppDTO.setState(false); - @Test - void isTppValid_FalseWhenNull() { - assertFalse(validationService.isTppValid(null)); + when(tppConnector.get(anyString())).thenReturn(Mono.just(inactiveTppDTO)); + + StepVerifier.create(validationService.validateTppAndSaveConsent(fiscalCode, tppId, CITIZEN_CONSENT)) + .expectErrorMatches(throwable -> throwable instanceof ClientExceptionWithBody && + "TPP does not exist or is not active".equals(throwable.getMessage()) && + "TPP_NOT_FOUND".equals(((ClientExceptionWithBody) throwable).getCode())) + .verify(); + + verify(citizenRepository, never()).save(any()); + verify(bloomFilterService, never()).add(fiscalCode); } + }