From 6b303dc259fd03bffdde5ce28c439c76680d306d Mon Sep 17 00:00:00 2001 From: Andrea Morabito <78792023+and-mora@users.noreply.github.com> Date: Fri, 12 May 2023 09:52:08 +0200 Subject: [PATCH] fix: [RTD-1669] fix conditional flow after ack list retrieve fails (#360) * [RTD-1669] fix conditional flow on ade-ack step failure * [RTD-1669] improve failure handling and logging --- .../batch/TransactionFilterBatch.java | 2 +- .../batch/TransactionFilterBatchTest.java | 24 ++++++++ .../connector/SenderAdeAckRestClientImpl.java | 55 +++++++++++++------ .../connector/SenderAdeAckRestClientTest.java | 22 ++++++++ 4 files changed, 86 insertions(+), 17 deletions(-) diff --git a/api/batch/src/main/java/it/gov/pagopa/rtd/transaction_filter/batch/TransactionFilterBatch.java b/api/batch/src/main/java/it/gov/pagopa/rtd/transaction_filter/batch/TransactionFilterBatch.java index b350341e..d201a815 100644 --- a/api/batch/src/main/java/it/gov/pagopa/rtd/transaction_filter/batch/TransactionFilterBatch.java +++ b/api/batch/src/main/java/it/gov/pagopa/rtd/transaction_filter/batch/TransactionFilterBatch.java @@ -333,7 +333,7 @@ public FlowJobBuilder transactionJobBuilder() { .on(FAILED).to(fileManagementTask()) .from(transactionFilterStep.transactionSenderRtdMasterStep(this.hpanConnectorService, this.storeService)) .on("*").to(senderAdeAckFilesRecoveryTask()) - .next(pendingStepDecider(sendPendingFilesStepEnabled)) + .on("*").to(pendingStepDecider(sendPendingFilesStepEnabled)) .on(TRUE).to(transactionFilterStep.transactionSenderPendingMasterStep(this.hpanConnectorService)) .from(pendingStepDecider(sendPendingFilesStepEnabled)) .on(FALSE).to(fileManagementTask()) diff --git a/api/batch/src/test/java/it/gov/pagopa/rtd/transaction_filter/batch/TransactionFilterBatchTest.java b/api/batch/src/test/java/it/gov/pagopa/rtd/transaction_filter/batch/TransactionFilterBatchTest.java index e033cd75..179ca2d9 100644 --- a/api/batch/src/test/java/it/gov/pagopa/rtd/transaction_filter/batch/TransactionFilterBatchTest.java +++ b/api/batch/src/test/java/it/gov/pagopa/rtd/transaction_filter/batch/TransactionFilterBatchTest.java @@ -47,6 +47,7 @@ import org.junit.runner.RunWith; import org.mockito.BDDMockito; import org.mockito.Mockito; +import org.springframework.batch.core.BatchStatus; import org.springframework.batch.core.ExitStatus; import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobParameters; @@ -348,6 +349,29 @@ public void whenSenderAdeAckTaskletIsSetThenMatchExpectedOutputFiles() { .hasContent("7890123;stringdefault"); } + @SneakyThrows + @Test + public void whenSenderAdeAckTaskletFailsThenExecuteFileManagementStep() { + String publicKey = createPublicKey(); + createPanPGP(); + createTrnOutputFile(); + createAdeOutputFile(); + BDDMockito.doReturn(publicKey).when(storeServiceSpy).getKey("pagopa"); + BDDMockito.when(senderAdeAckRestClient.getSenderAdeAckFiles()).thenThrow(RuntimeException.class); + + JobExecution jobExecution = jobLauncherTestUtils.launchJob(new JobParametersBuilder() + .addDate("startDateTime", new Date()) + .toJobParameters()); + assertThat(jobExecution.getExitStatus()).isEqualTo(ExitStatus.FAILED); + assertThat(jobExecution.getStatus()).isEqualTo(BatchStatus.COMPLETED); + + Collection stepNamesExecuted = jobExecution.getStepExecutions().stream() + .map(StepExecution::getStepName) + .collect(Collectors.toSet()); + + assertThat(stepNamesExecuted).contains("transaction-filter-file-management-step"); + } + @SneakyThrows @Test public void jobExecutionFails() { diff --git a/integration/rest/src/main/java/it/gov/pagopa/rtd/transaction_filter/connector/SenderAdeAckRestClientImpl.java b/integration/rest/src/main/java/it/gov/pagopa/rtd/transaction_filter/connector/SenderAdeAckRestClientImpl.java index 8c4549d7..2a96daa3 100644 --- a/integration/rest/src/main/java/it/gov/pagopa/rtd/transaction_filter/connector/SenderAdeAckRestClientImpl.java +++ b/integration/rest/src/main/java/it/gov/pagopa/rtd/transaction_filter/connector/SenderAdeAckRestClientImpl.java @@ -12,6 +12,8 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.Optional; +import javax.validation.ValidationException; import javax.validation.constraints.NotNull; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -39,13 +41,26 @@ public class SenderAdeAckRestClientImpl implements SenderAdeAckRestClient { @Override public List getSenderAdeAckFiles() throws IOException { - ResponseEntity senderAdeAckResponse = hpanRestConnector.getSenderAdeAckList( - apiKey); + SenderAdeAckList adeAckList = retrieveAdeAckList(); - senderAdeAckValidator.validate(senderAdeAckResponse); + return downloadFiles(adeAckList.getFileNameList()); + } - SenderAdeAckList senderAdeAckDto = Objects.requireNonNull(senderAdeAckResponse.getBody()); - return downloadFiles(senderAdeAckDto.getFileNameList()); + private SenderAdeAckList retrieveAdeAckList() { + SenderAdeAckList senderAdeAckList; + try { + ResponseEntity adeAckResponse = hpanRestConnector.getSenderAdeAckList( + apiKey); + senderAdeAckValidator.validate(adeAckResponse); + + senderAdeAckList = adeAckResponse.getBody(); + } catch (FeignException | ResponseStatusException | ValidationException ex) { + log.warn("Failed to download ade ack list! It will be downloaded on the next run."); + // returns an empty list + return new SenderAdeAckList(); + } + + return Objects.requireNonNull(senderAdeAckList); } private List downloadFiles(List fileNameList) throws IOException { @@ -55,20 +70,14 @@ private List downloadFiles(List fileNameList) throws IOException { Path temporaryDirectory = Files.createTempDirectory(tempDir.toPath(), "senderAdeAck"); for (String fileName : fileNameList) { - ResponseEntity resourceResponseEntity = hpanRestConnector.getSenderAdeAckFile( - apiKey, fileName); - - resourceValidator.validateStatus(resourceResponseEntity.getStatusCode()); - - Resource resource = resourceResponseEntity.getBody(); - if (resource == null) { - log.warn("received empty file"); + Optional resource = downloadAdeAck(fileName); + if (!resource.isPresent()) { // skip to next file continue; } File tempFile = createTempFile(fileName, temporaryDirectory); - copyFromResourceToFile(resource, tempFile); + copyFromResourceToFile(resource.get(), tempFile); boolean isDownloadConfirmed = sendAckReceivedConfirmation(fileName); if (isDownloadConfirmed) { @@ -87,6 +96,21 @@ private List downloadFiles(List fileNameList) throws IOException { return filesDownloaded; } + private Optional downloadAdeAck(String fileName) { + Resource adeAckResource = null; + try { + ResponseEntity resourceResponseEntity = hpanRestConnector.getSenderAdeAckFile( + apiKey, fileName); + resourceValidator.validateStatus(resourceResponseEntity.getStatusCode()); + + adeAckResource = resourceResponseEntity.getBody(); + } catch (FeignException | ResponseStatusException ex) { + log.warn("Failed to download the ack: {}! It will be downloaded on the next run.", fileName); + } + + return Optional.ofNullable(adeAckResource); + } + private File createTempFile(@NotNull String filename, @NotNull Path directory) throws IOException { return Files.createFile(Paths.get(directory.toString() @@ -96,8 +120,7 @@ private File createTempFile(@NotNull String filename, @NotNull Path directory) private void copyFromResourceToFile(@NotNull Resource resource, @NotNull File file) throws IOException { - try ( - FileOutputStream tempFileFOS = new FileOutputStream(file); + try (FileOutputStream tempFileFOS = new FileOutputStream(file); InputStream inputStream = resource.getInputStream()) { StreamUtils.copy(inputStream, tempFileFOS); } diff --git a/integration/rest/src/test/java/it/gov/pagopa/rtd/transaction_filter/connector/SenderAdeAckRestClientTest.java b/integration/rest/src/test/java/it/gov/pagopa/rtd/transaction_filter/connector/SenderAdeAckRestClientTest.java index d110ee06..fec3afd3 100644 --- a/integration/rest/src/test/java/it/gov/pagopa/rtd/transaction_filter/connector/SenderAdeAckRestClientTest.java +++ b/integration/rest/src/test/java/it/gov/pagopa/rtd/transaction_filter/connector/SenderAdeAckRestClientTest.java @@ -142,6 +142,17 @@ public void whenDownloadFileIsNotFoundThenDoNotSaveTheFile() { assertThat(files).isNotNull().isEmpty(); } + @SneakyThrows + @Test + public void whenDownloadFileReturns404ThenDoNotSaveTheFile() { + wireMockRule.stubFor(get(urlPathMatching("/ade/.*")) + .willReturn(aResponse().withStatus(404))); + + List files = restClient.getSenderAdeAckFiles(); + + assertThat(files).isNotNull().isEmpty(); + } + @SneakyThrows @Test public void whenAdeAckListApiReturnsEmptyBodyThenRaisesException() { @@ -152,6 +163,17 @@ public void whenAdeAckListApiReturnsEmptyBodyThenRaisesException() { NullPointerException.class); } + @SneakyThrows + @Test + public void whenAdeAckListReturns404ThenReturnsEmptyList() { + wireMockRule.stubFor(get("/rtd/file-register/sender-ade-ack") + .willReturn(aResponse().withStatus(404))); + + List files = restClient.getSenderAdeAckFiles(); + + assertThat(files).isEmpty(); + } + @SneakyThrows @Test public void whenPutAckReceivedReturns404ThenReturnsNoFilesAndDeleteTempOnes() {