From 68951fbf6828bcb9119d0ffcd28588c17cefaefe Mon Sep 17 00:00:00 2001 From: Vitolo-Andrea Date: Fri, 8 Nov 2024 14:39:51 +0100 Subject: [PATCH] Clean code --- .../tpp/configuration/ExceptionMap.java | 2 +- .../pagopa/tpp/controller/TppController.java | 10 +- .../java/it/gov/pagopa/tpp/dto/TppIdList.java | 2 + .../pagopa/tpp/service/TppServiceImpl.java | 77 +++++++------ .../tpp/controller/TppControllerTest.java | 57 ++++----- .../it/gov/pagopa/tpp/faker/TppFaker.java | 22 ++++ .../pagopa/tpp/service/TppServiceTest.java | 108 ++++++++---------- 7 files changed, 136 insertions(+), 142 deletions(-) create mode 100644 src/test/java/it/gov/pagopa/tpp/faker/TppFaker.java diff --git a/src/main/java/it/gov/pagopa/tpp/configuration/ExceptionMap.java b/src/main/java/it/gov/pagopa/tpp/configuration/ExceptionMap.java index ea35b59..0d22092 100644 --- a/src/main/java/it/gov/pagopa/tpp/configuration/ExceptionMap.java +++ b/src/main/java/it/gov/pagopa/tpp/configuration/ExceptionMap.java @@ -32,7 +32,7 @@ public RuntimeException throwException(String exceptionKey, String message) { if (exceptions.containsKey(exceptionKey)) { return exceptions.get(exceptionKey).apply(message); } else { - log.error("Exception Name Not Found: {}", exceptionKey); + log.error("[EMP-TPP][EXCEPTION-MAP] Exception Name Not Found: {}", exceptionKey); return new RuntimeException(); } } diff --git a/src/main/java/it/gov/pagopa/tpp/controller/TppController.java b/src/main/java/it/gov/pagopa/tpp/controller/TppController.java index 8d952a2..eb6a141 100644 --- a/src/main/java/it/gov/pagopa/tpp/controller/TppController.java +++ b/src/main/java/it/gov/pagopa/tpp/controller/TppController.java @@ -13,10 +13,10 @@ public interface TppController { /** - * Get list of tpps + * Get list of tpp * * @param tppIdList whose data is to be retrieved - * @return outcome of retrieving the tpps + * @return outcome of retrieving the tpp */ @PostMapping("/list") Mono>> getEnabledList(@Valid @RequestBody TppIdList tppIdList); @@ -41,10 +41,10 @@ public interface TppController { Mono> upsert(@Valid @RequestBody TppDTO tppDTO); /** - * Save a tpp + * Get a tpp * - * @param tppId to save - * @return outcome of saving the tpp + * @param tppId to get + * @return outcome of getting tpp */ @GetMapping("/{tppId}") Mono> get(@Valid @PathVariable String tppId); diff --git a/src/main/java/it/gov/pagopa/tpp/dto/TppIdList.java b/src/main/java/it/gov/pagopa/tpp/dto/TppIdList.java index 459cbe0..51b9274 100644 --- a/src/main/java/it/gov/pagopa/tpp/dto/TppIdList.java +++ b/src/main/java/it/gov/pagopa/tpp/dto/TppIdList.java @@ -1,11 +1,13 @@ package it.gov.pagopa.tpp.dto; +import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; import java.util.List; @Data @NoArgsConstructor +@AllArgsConstructor public class TppIdList { List ids; } diff --git a/src/main/java/it/gov/pagopa/tpp/service/TppServiceImpl.java b/src/main/java/it/gov/pagopa/tpp/service/TppServiceImpl.java index eee6578..e5d7185 100644 --- a/src/main/java/it/gov/pagopa/tpp/service/TppServiceImpl.java +++ b/src/main/java/it/gov/pagopa/tpp/service/TppServiceImpl.java @@ -28,7 +28,8 @@ public class TppServiceImpl implements TppService { private final TppDTOToObjectMapper mapperToObject; private final ExceptionMap exceptionMap; - public TppServiceImpl(TppRepository tppRepository, TppObjectToDTOMapper mapperToDTO, TppDTOToObjectMapper mapperToObject, ExceptionMap exceptionMap) { + public TppServiceImpl(TppRepository tppRepository, TppObjectToDTOMapper mapperToDTO, + TppDTOToObjectMapper mapperToObject, ExceptionMap exceptionMap) { this.tppRepository = tppRepository; this.mapperToDTO = mapperToDTO; this.mapperToObject = mapperToObject; @@ -37,65 +38,67 @@ public TppServiceImpl(TppRepository tppRepository, TppObjectToDTOMapper mapperTo @Override public Mono> getEnabledList(List tppIdList) { - log.info("[EMD-TPP][GET-ENABLED] Received tppIdList: {}",(tppIdList)); + log.info("[EMD-TPP][GET-ENABLED] Received tppIdList: {}", tppIdList); return tppRepository.findByTppIdInAndStateTrue(tppIdList) .collectList() .map(tppList -> tppList.stream() .map(mapperToDTO::map) - .toList() - ) - .doOnSuccess(tppDTOList -> log.info("[EMD-TPP][GET-ENABLED] Tpps founded: {}",tppDTOList)) - .doOnError(error -> log.error("[EMD-TPP][GET-ENABLED] Error: {}", error.getMessage())); - } - + .toList()) + .doOnSuccess(tppDTOList -> log.info("[EMD-TPP][GET-ENABLED] Found TPPs: {}", tppDTOList)) + .doOnError(error -> log.error("[EMD-TPP][GET-ENABLED] Error: {}", error.getMessage())); + } @Override public Mono upsert(TppDTO tppDTO) { - log.info("[EMD-TPP][UPSERT] Received tppDTO: {}", inputSanify(tppDTO.toString())); - Tpp tppReceived = mapperToObject.map(tppDTO); + log.info("[EMD-TPP][UPSERT] Received tppDTO: {}", inputSanify(tppDTO.toString())); + Tpp tppToSave = mapperToObject.map(tppDTO); + return tppRepository.findByTppId(tppDTO.getTppId()) - .flatMap(tppDB -> { - log.info("[EMD-TPP][UPSERT] TPP with tppId:[{}] already exists",(tppDTO.getTppId())); - tppReceived.setId(tppDB.getId()); - tppReceived.setLastUpdateDate(LocalDateTime.now()); - return tppRepository.save(tppReceived) - .map(mapperToDTO::map) - .doOnSuccess(savedTpp -> log.info("[EMD-TPP][UPSERT] Updated existing TPP")); + .flatMap(existingTpp -> { + log.info("[EMD-TPP][UPSERT] TPP with tppId [{}] already exists", tppDTO.getTppId()); + tppToSave.setId(existingTpp.getId()); + tppToSave.setLastUpdateDate(LocalDateTime.now()); + return saveTpp(tppToSave, "[EMD-TPP][UPSERT] Updated existing TPP "); }) - .switchIfEmpty( - Mono.defer(() -> { - tppReceived.setTppId("%s_%d".formatted(UUID.randomUUID().toString(), System.currentTimeMillis())); - tppReceived.setCreationDate(LocalDateTime.now()); - tppReceived.setLastUpdateDate(LocalDateTime.now()); - return tppRepository.save(tppReceived) - .map(mapperToDTO::map) - .doOnSuccess(savedTpp -> log.info("[EMD-TPP][UPSERT] Created TPP")); - }) - ); + .switchIfEmpty(Mono.defer(() -> { + tppToSave.setTppId(generateTppId()); + tppToSave.setCreationDate(LocalDateTime.now()); + tppToSave.setLastUpdateDate(LocalDateTime.now()); + return saveTpp(tppToSave, "[EMD-TPP][UPSERT] Created new TPP"); + })); } @Override public Mono updateState(String tppId, Boolean state) { - log.info("[EMD-TPP][UPDATE] Received tppId: {}",inputSanify(tppId)); + log.info("[EMD-TPP][UPDATE-STATE] Received tppId: {}", inputSanify(tppId)); return tppRepository.findByTppId(tppId) - .switchIfEmpty(Mono.error(exceptionMap - .throwException(ExceptionName.TPP_NOT_ONBOARDED,"Tpp not founded during update state process"))) + .switchIfEmpty(Mono.error(exceptionMap.throwException(ExceptionName.TPP_NOT_ONBOARDED, + "Tpp not found during state update process"))) .flatMap(tpp -> { tpp.setState(state); return tppRepository.save(tpp); }) .map(mapperToDTO::map) - .doOnSuccess(updatedTpp -> log.info("[EMD][TPP][UPDATE-STATE] Updated")); + .doOnSuccess(updatedTpp -> log.info("[EMD-TPP][UPDATE-STATE] State updated for tppId: {}", tppId)); } - @Override public Mono get(String tppId) { - log.info("[EMD-TPP][GET] Received tppId: {}",inputSanify(tppId)); + log.info("[EMD-TPP][GET] Received tppId: {}", inputSanify(tppId)); return tppRepository.findByTppId(tppId) - .switchIfEmpty(Mono.error(exceptionMap - .throwException(ExceptionName.TPP_NOT_ONBOARDED,"Tpp not founded during get process"))) + .switchIfEmpty(Mono.error(exceptionMap.throwException(ExceptionName.TPP_NOT_ONBOARDED, + "Tpp not found during get process"))) .map(mapperToDTO::map) - .doOnSuccess(updatedTpp -> log.info("[EMD][TPP][GET] Founded")); + .doOnSuccess(tppDTO -> log.info("[EMD-TPP][GET] Found TPP with tppId: {}", tppId)); + } + + private Mono saveTpp(Tpp tppToSave, String successLogMessage) { + return tppRepository.save(tppToSave) + .map(mapperToDTO::map) + .doOnSuccess(savedTpp -> log.info(successLogMessage)); + } + + private String generateTppId() { + return String.format("%s_%d", UUID.randomUUID(), System.currentTimeMillis()); } -} +} \ No newline at end of file diff --git a/src/test/java/it/gov/pagopa/tpp/controller/TppControllerTest.java b/src/test/java/it/gov/pagopa/tpp/controller/TppControllerTest.java index 280633c..aa1ff49 100644 --- a/src/test/java/it/gov/pagopa/tpp/controller/TppControllerTest.java +++ b/src/test/java/it/gov/pagopa/tpp/controller/TppControllerTest.java @@ -15,7 +15,6 @@ import org.springframework.test.web.reactive.server.WebTestClient; import reactor.core.publisher.Mono; -import java.util.ArrayList; import java.util.List; @WebFluxTest(TppControllerImpl.class) @@ -30,98 +29,82 @@ class TppControllerTest { @Autowired private ObjectMapper objectMapper; + private static final TppDTO MOCK_TPP_DTO = TppDTOFaker.mockInstance(true); + + private static final List MOCK_TPP_DTO_LIST = List.of(MOCK_TPP_DTO); + private static final TppIdList MOCK_TPP_ID_LIST = new TppIdList(List.of(MOCK_TPP_DTO.getTppId())); + @Test void upsert_Ok() { - TppDTO mockTppDTO = TppDTOFaker.mockInstance(true); - Mockito.when(tppService.upsert(mockTppDTO)).thenReturn(Mono.just(mockTppDTO)); + Mockito.when(tppService.upsert(MOCK_TPP_DTO)).thenReturn(Mono.just(MOCK_TPP_DTO)); webClient.post() .uri("/emd/tpp") .contentType(MediaType.APPLICATION_JSON) - .bodyValue(mockTppDTO) + .bodyValue(MOCK_TPP_DTO) .exchange() .expectStatus().isOk() .expectBody(TppDTO.class) .consumeWith(response -> { TppDTO resultResponse = response.getResponseBody(); Assertions.assertNotNull(resultResponse); - Assertions.assertEquals(mockTppDTO, resultResponse); + Assertions.assertEquals(MOCK_TPP_DTO, resultResponse); }); } @Test void stateUpdate_Ok() { - TppDTO mockTppDTO = TppDTOFaker.mockInstance(true); - - Mockito.when(tppService.updateState(mockTppDTO.getTppId(), mockTppDTO.getState())) - .thenReturn(Mono.just(mockTppDTO)); + Mockito.when(tppService.updateState(MOCK_TPP_DTO.getTppId(), MOCK_TPP_DTO.getState())) + .thenReturn(Mono.just(MOCK_TPP_DTO)); webClient.put() .uri("/emd/tpp") .contentType(MediaType.APPLICATION_JSON) - .bodyValue(mockTppDTO) + .bodyValue(MOCK_TPP_DTO) .exchange() .expectStatus().isOk() .expectBody(TppDTO.class) .consumeWith(response -> { TppDTO resultResponse = response.getResponseBody(); Assertions.assertNotNull(resultResponse); - Assertions.assertEquals(resultResponse,mockTppDTO); + Assertions.assertEquals(MOCK_TPP_DTO,resultResponse); }); } @Test void get_Ok() { - TppDTO mockTppDTO = TppDTOFaker.mockInstance(true); - - Mockito.when(tppService.get(mockTppDTO.getTppId())) - .thenReturn(Mono.just(mockTppDTO)); + Mockito.when(tppService.get(MOCK_TPP_DTO.getTppId())) + .thenReturn(Mono.just(MOCK_TPP_DTO)); webClient.get() - .uri("/emd/tpp/{tppId}",mockTppDTO.getTppId()) + .uri("/emd/tpp/{tppId}",MOCK_TPP_DTO.getTppId()) .exchange() .expectStatus().isOk() .expectBody(TppDTO.class) .consumeWith(response -> { TppDTO resultResponse = response.getResponseBody(); Assertions.assertNotNull(resultResponse); - Assertions.assertEquals(resultResponse,mockTppDTO); + Assertions.assertEquals(MOCK_TPP_DTO,resultResponse); }); } @Test void getEnabled_Ok() { - TppIdList tppIdList = new TppIdList(); - ArrayList tppIds = new ArrayList<>(); - tppIds.add("1"); - tppIds.add("2"); - tppIds.add("3"); - tppIdList.setIds(tppIds); // Using ArrayList - - - TppDTO tpp1 = new TppDTO(); - TppDTO tpp2 = new TppDTO(); - - - ArrayList tppDTOList = new ArrayList<>(); - tppDTOList.add(tpp1); - tppDTOList.add(tpp2); - - Mockito.when(tppService.getEnabledList(tppIdList.getIds())).thenReturn(Mono.just(tppDTOList)); + Mockito.when(tppService.getEnabledList(MOCK_TPP_ID_LIST.getIds())).thenReturn(Mono.just(MOCK_TPP_DTO_LIST)); webClient.post() .uri("/emd/tpp/list") .contentType(MediaType.APPLICATION_JSON) - .bodyValue(tppIdList) // Body of the request + .bodyValue(MOCK_TPP_ID_LIST) // Body of the request .exchange() .expectStatus().isOk() .expectBodyList(TppDTO.class) .consumeWith(response -> { List resultResponse = response.getResponseBody(); Assertions.assertNotNull(resultResponse); - Assertions.assertEquals(tppDTOList.size(), resultResponse.size()); - Assertions.assertTrue(resultResponse.containsAll(tppDTOList)); + Assertions.assertEquals(MOCK_TPP_DTO_LIST.size(), resultResponse.size()); + Assertions.assertTrue(resultResponse.containsAll(MOCK_TPP_DTO_LIST)); }); } } \ No newline at end of file diff --git a/src/test/java/it/gov/pagopa/tpp/faker/TppFaker.java b/src/test/java/it/gov/pagopa/tpp/faker/TppFaker.java new file mode 100644 index 0000000..084f539 --- /dev/null +++ b/src/test/java/it/gov/pagopa/tpp/faker/TppFaker.java @@ -0,0 +1,22 @@ +package it.gov.pagopa.tpp.faker; + +import it.gov.pagopa.tpp.enums.AuthenticationType; +import it.gov.pagopa.tpp.model.Contact; +import it.gov.pagopa.tpp.model.Tpp; + +public class TppFaker { + + private TppFaker(){} + public static Tpp mockInstance(Boolean bias){ + return Tpp.builder() + .tppId("tppId") + .messageUrl("https://wwwmessageUrl.it") + .authenticationUrl("https://www.AuthenticationUrl.it") + .authenticationType(AuthenticationType.OAUTH2) + .state(bias) + .entityId("entityId01234567") + .businessName("businessName") + .contact(new Contact("name","number", "email")) + .build(); + } +} diff --git a/src/test/java/it/gov/pagopa/tpp/service/TppServiceTest.java b/src/test/java/it/gov/pagopa/tpp/service/TppServiceTest.java index a304f73..fa39124 100644 --- a/src/test/java/it/gov/pagopa/tpp/service/TppServiceTest.java +++ b/src/test/java/it/gov/pagopa/tpp/service/TppServiceTest.java @@ -5,6 +5,7 @@ import it.gov.pagopa.tpp.dto.mapper.TppObjectToDTOMapper; import it.gov.pagopa.tpp.configuration.ExceptionMap; import it.gov.pagopa.tpp.faker.TppDTOFaker; +import it.gov.pagopa.tpp.faker.TppFaker; import org.junit.jupiter.api.function.Executable; import it.gov.pagopa.tpp.model.Tpp; import it.gov.pagopa.tpp.model.mapper.TppDTOToObjectMapper; @@ -21,11 +22,9 @@ import java.util.List; +import static org.junit.jupiter.api.Assertions.*; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; - -@ExtendWith({SpringExtension.class}) +@ExtendWith(SpringExtension.class) @ContextConfiguration(classes = { TppServiceImpl.class, TppObjectToDTOMapper.class, @@ -33,119 +32,104 @@ ExceptionMap.class }) class TppServiceTest { + @Autowired - TppServiceImpl tppService; + private TppServiceImpl tppService; + @MockBean - TppRepository tppRepository; + private TppRepository tppRepository; + @Autowired - TppDTOToObjectMapper mapperToObject; + private TppDTOToObjectMapper mapperToObject; + + private static final TppDTO MOCK_TPP_DTO = TppDTOFaker.mockInstance(true); + private static final Tpp MOCK_TPP = TppFaker.mockInstance(true); + private static final List MOCK_TPP_DTO_LIST = List.of(MOCK_TPP_DTO); + private static final List MOCK_TPP_LIST = List.of(MOCK_TPP); + private static final List MOCK_TPP_ID_LIST = List.of(MOCK_TPP_DTO.getTppId()); @Test void getEnabled_Ok() { + Mockito.when(tppRepository.findByTppIdInAndStateTrue(MOCK_TPP_ID_LIST)) + .thenReturn(Flux.fromIterable(MOCK_TPP_LIST)); - TppDTO tppDTO = TppDTOFaker.mockInstance(true); - List tppDTOs =List.of(tppDTO); - List tpps =List.of(mapperToObject.map(tppDTO)); - List arrayList = List.of("1"); - - Mockito.when(tppRepository.findByTppIdInAndStateTrue(arrayList)) - .thenReturn(Flux.fromIterable(tpps)); - - List response = tppService.getEnabledList(arrayList).block(); - - assertEquals(tppDTOs, response); + List response = tppService.getEnabledList(MOCK_TPP_ID_LIST).block(); + assertNotNull(response); + assertEquals(MOCK_TPP_DTO_LIST, response); } @Test void createTpp_Ok() { - - TppDTO tppDTO = TppDTOFaker.mockInstance(true); - Tpp tpp = mapperToObject.map(tppDTO); - Mockito.when(tppRepository.findByTppId(Mockito.any())) .thenReturn(Mono.empty()); Mockito.when(tppRepository.save(Mockito.any())) - .thenReturn(Mono.just(tpp)); + .thenReturn(Mono.just(MOCK_TPP)); - TppDTO response = tppService.upsert(tppDTO).block(); + TppDTO response = tppService.upsert(MOCK_TPP_DTO).block(); - assertEquals(tppDTO, response); + assertNotNull(response); + assertEquals(MOCK_TPP_DTO, response); } @Test void updateTpp_Ok() { - - TppDTO tppDTO = TppDTOFaker.mockInstance(true); - Tpp tpp = mapperToObject.map(tppDTO); - Mockito.when(tppRepository.findByTppId(Mockito.any())) - .thenReturn(Mono.just(tpp)); + .thenReturn(Mono.just(MOCK_TPP)); Mockito.when(tppRepository.save(Mockito.any())) - .thenReturn(Mono.just(tpp)); + .thenReturn(Mono.just(MOCK_TPP)); - TppDTO response = tppService.upsert(tppDTO).block(); + TppDTO response = tppService.upsert(MOCK_TPP_DTO).block(); - assertEquals(tppDTO, response); + assertNotNull(response); + assertEquals(MOCK_TPP_DTO, response); } - - @Test void updateState_Ok() { - - TppDTO tppDTO = TppDTOFaker.mockInstance(true); - Tpp tpp = mapperToObject.map(tppDTO); - - Mockito.when(tppRepository.findByTppId(tppDTO.getTppId())) - .thenReturn(Mono.just(tpp)); + Mockito.when(tppRepository.findByTppId(MOCK_TPP_DTO.getTppId())) + .thenReturn(Mono.just(MOCK_TPP)); Mockito.when(tppRepository.save(Mockito.any())) - .thenReturn(Mono.just(tpp)); + .thenReturn(Mono.just(MOCK_TPP)); - TppDTO result = tppService.updateState(tppDTO.getTppId(),tppDTO.getState()).block(); + TppDTO result = tppService.updateState(MOCK_TPP_DTO.getTppId(), MOCK_TPP_DTO.getState()).block(); - assertEquals(result, tppDTO); + assertNotNull(result); + assertEquals(MOCK_TPP_DTO, result); } @Test void updateState_Ko_TppNotOnboarded() { - - TppDTO tppDTO = TppDTOFaker.mockInstance(true); - - Mockito.when(tppRepository.findByTppId(tppDTO.getTppId())) + Mockito.when(tppRepository.findByTppId(MOCK_TPP_DTO.getTppId())) .thenReturn(Mono.empty()); - Executable executable = () -> tppService.updateState(tppDTO.getTppId(),tppDTO.getState()).block(); + Executable executable = () -> tppService.updateState(MOCK_TPP_DTO.getTppId(), MOCK_TPP_DTO.getState()).block(); ClientExceptionWithBody exception = assertThrows(ClientExceptionWithBody.class, executable); + assertNotNull(exception); assertEquals("TPP_NOT_ONBOARDED", exception.getCode()); } @Test void get_Ok() { + Mockito.when(tppRepository.findByTppId(MOCK_TPP_DTO.getTppId())) + .thenReturn(Mono.just(MOCK_TPP)); - TppDTO tppDTO = TppDTOFaker.mockInstance(true); - Tpp tpp = mapperToObject.map(tppDTO); - - Mockito.when(tppRepository.findByTppId(tppDTO.getEntityId())) - .thenReturn(Mono.just(tpp)); - - TppDTO result = tppService.get(tppDTO.getEntityId()).block(); + TppDTO result = tppService.get(MOCK_TPP_DTO.getTppId()).block(); - assertEquals(result, tppDTO); + assertNotNull(result); + assertEquals(MOCK_TPP_DTO, result); } @Test void get_Ko_TppNotOnboarded() { - - TppDTO tppDTO = TppDTOFaker.mockInstance(true); - - Mockito.when(tppRepository.findByTppId(tppDTO.getEntityId())) + Mockito.when(tppRepository.findByTppId(MOCK_TPP_DTO.getTppId())) .thenReturn(Mono.empty()); - Executable executable = () -> tppService.get(tppDTO.getEntityId()).block(); + Executable executable = () -> tppService.get(MOCK_TPP_DTO.getTppId()).block(); ClientExceptionWithBody exception = assertThrows(ClientExceptionWithBody.class, executable); + assertNotNull(exception); assertEquals("TPP_NOT_ONBOARDED", exception.getCode()); } }