From 5cbddff02b8665958cb0db0530ad034d041f9b7a Mon Sep 17 00:00:00 2001 From: Viachaslau_Khandramai Date: Sun, 26 Jan 2025 01:18:33 +0100 Subject: [PATCH 1/5] MODBULKOPS-451 - Rework errors preview --- descriptors/ModuleDescriptor-template.json | 2 +- .../folio/bulkops/client/BulkEditClient.java | 6 -- .../controller/BulkOperationController.java | 4 +- .../folio/bulkops/service/ErrorService.java | 40 ++++----- .../swagger.api/bulk-operations.yaml | 6 ++ .../bulkops/service/ErrorServiceTest.java | 86 +++++++------------ src/test/resources/files/errors.csv | 1 + 7 files changed, 59 insertions(+), 86 deletions(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 78da7ece3..5b6bb2ab5 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -4,7 +4,7 @@ "provides": [ { "id": "bulk-operations", - "version": "1.5", + "version": "1.6", "handlers": [ { "methods": [ "POST" ], diff --git a/src/main/java/org/folio/bulkops/client/BulkEditClient.java b/src/main/java/org/folio/bulkops/client/BulkEditClient.java index 24fc09aa1..1813b6d6b 100644 --- a/src/main/java/org/folio/bulkops/client/BulkEditClient.java +++ b/src/main/java/org/folio/bulkops/client/BulkEditClient.java @@ -3,13 +3,10 @@ import java.util.UUID; import org.folio.bulkops.configs.FeignClientConfiguration; -import org.folio.bulkops.domain.dto.Errors; import org.springframework.cloud.openfeign.FeignClient; import org.springframework.http.MediaType; -import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; -import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RequestPart; import org.springframework.web.multipart.MultipartFile; @@ -20,7 +17,4 @@ public interface BulkEditClient { @PostMapping(value = "/{jobId}/start") void startJob(@PathVariable UUID jobId); - - @GetMapping(value = "/{jobId}/errors") - Errors getErrorsPreview(@PathVariable UUID jobId, @RequestParam int limit); } diff --git a/src/main/java/org/folio/bulkops/controller/BulkOperationController.java b/src/main/java/org/folio/bulkops/controller/BulkOperationController.java index 567f48411..3418710a8 100644 --- a/src/main/java/org/folio/bulkops/controller/BulkOperationController.java +++ b/src/main/java/org/folio/bulkops/controller/BulkOperationController.java @@ -92,8 +92,8 @@ public ResponseEntity getBulkOperationCollection(String } @Override - public ResponseEntity getErrorsPreviewByOperationId(UUID operationId, Integer limit) { - return new ResponseEntity<>(errorService.getErrorsPreviewByBulkOperationId(operationId, limit), HttpStatus.OK); + public ResponseEntity getErrorsPreviewByOperationId(UUID operationId, Integer limit, Integer offset) { + return new ResponseEntity<>(errorService.getErrorsPreviewByBulkOperationId(operationId, limit, offset), HttpStatus.OK); } @Override diff --git a/src/main/java/org/folio/bulkops/service/ErrorService.java b/src/main/java/org/folio/bulkops/service/ErrorService.java index 9ad47a822..7e9b4780b 100644 --- a/src/main/java/org/folio/bulkops/service/ErrorService.java +++ b/src/main/java/org/folio/bulkops/service/ErrorService.java @@ -2,6 +2,7 @@ import static java.util.Objects.isNull; import static java.util.Objects.nonNull; +import static java.util.stream.Collectors.toList; import static org.apache.commons.lang3.StringUtils.EMPTY; import static org.apache.commons.lang3.StringUtils.LF; import static org.folio.bulkops.domain.dto.OperationStatusType.COMPLETED; @@ -12,7 +13,9 @@ import static org.folio.bulkops.util.Constants.DATA_IMPORT_ERROR_DISCARDED; import static org.folio.bulkops.util.Constants.MSG_NO_CHANGE_REQUIRED; +import java.io.BufferedReader; import java.io.ByteArrayInputStream; +import java.io.InputStreamReader; import java.time.LocalDate; import java.util.ArrayList; import java.util.List; @@ -29,6 +32,7 @@ import org.folio.bulkops.domain.bean.JobLogEntry; import org.folio.bulkops.domain.bean.StateType; import org.folio.bulkops.domain.dto.Error; +import org.folio.bulkops.domain.dto.ErrorType; import org.folio.bulkops.domain.dto.Errors; import org.folio.bulkops.domain.dto.IdentifierType; import org.folio.bulkops.domain.dto.Parameter; @@ -56,7 +60,6 @@ public class ErrorService { private final BulkOperationRepository operationRepository; private final RemoteFileSystemClient remoteFileSystemClient; private final BulkOperationExecutionContentRepository executionContentRepository; - private final BulkEditClient bulkEditClient; private final MetadataProviderClient metadataProviderClient; public void saveError(UUID bulkOperationId, String identifier, String errorMessage, String uiErrorMessage, String link) { @@ -87,17 +90,23 @@ public void deleteErrorsByBulkOperationId(UUID bulkOperationId) { log.info("Errors deleted for bulk operation {}", bulkOperationId); } - public Errors getErrorsPreviewByBulkOperationId(UUID bulkOperationId, int limit) { + public Errors getErrorsPreviewByBulkOperationId(UUID bulkOperationId, int limit, int offset) { var bulkOperation = operationRepository.findById(bulkOperationId) .orElseThrow(() -> new NotFoundException("BulkOperation was not found by id=" + bulkOperationId)); if (Set.of(DATA_MODIFICATION, REVIEW_CHANGES, REVIEWED_NO_MARC_RECORDS).contains(bulkOperation.getStatus()) || COMPLETED_WITH_ERRORS == bulkOperation.getStatus() && noCommittedErrors(bulkOperation)) { - var errors = bulkEditClient.getErrorsPreview(bulkOperation.getDataExportJobId(), limit); - return new Errors().errors(errors.getErrors().stream() - .map(this::prepareInternalErrorRepresentation) - .toList()) - .totalRecords(errors.getTotalRecords()); + var errors = new BufferedReader(new InputStreamReader(remoteFileSystemClient.get(bulkOperation.getLinkToMatchedRecordsErrorsCsvFile()))) + .lines() + .skip(offset) + .limit(limit) + .map(message -> { + var error = message.split(Constants.COMMA_DELIMETER); + return new Error().message(error[1]).parameters(List.of(new Parameter().key(IDENTIFIER).value(error[0]))); + }) + .collect(toList()); + return new Errors().errors(errors) + .totalRecords(remoteFileSystemClient.getNumOfLines(bulkOperation.getLinkToMatchedRecordsErrorsCsvFile())); } else if (COMPLETED == bulkOperation.getStatus() || COMPLETED_WITH_ERRORS == bulkOperation.getStatus()) { - return getExecutionErrors(bulkOperationId, limit); + return getExecutionErrors(bulkOperationId, limit, offset); } else { throw new NotFoundException("Errors preview is not available"); } @@ -139,19 +148,8 @@ private boolean noCommittedErrors(BulkOperation bulkOperation) { return isNull(bulkOperation.getCommittedNumOfErrors()) || bulkOperation.getCommittedNumOfErrors() == 0; } - private Error prepareInternalErrorRepresentation(Error e) { - var error= e.getMessage().split(Constants.COMMA_DELIMETER); - return new Error().message(error[1]).parameters(List.of(new Parameter().key(IDENTIFIER).value(error[0]))); - } - - public String getErrorsCsvByBulkOperationId(UUID bulkOperationId) { - return getErrorsPreviewByBulkOperationId(bulkOperationId, Integer.MAX_VALUE).getErrors().stream() - .map(error -> String.join(Constants.COMMA_DELIMETER, ObjectUtils.isEmpty(error.getParameters()) ? EMPTY : error.getParameters().get(0).getValue(), error.getMessage())) - .collect(Collectors.joining(Constants.NEW_LINE_SEPARATOR)); - } - - private Errors getExecutionErrors(UUID bulkOperationId, int limit) { - var errorPage = executionContentRepository.findByBulkOperationIdAndErrorMessageIsNotNull(bulkOperationId, OffsetRequest.of(0, limit)); + private Errors getExecutionErrors(UUID bulkOperationId, int limit, int offset) { + var errorPage = executionContentRepository.findByBulkOperationIdAndErrorMessageIsNotNull(bulkOperationId, OffsetRequest.of(offset, limit)); var errors = errorPage.toList().stream() .map(this::executionContentToFolioError) .toList(); diff --git a/src/main/resources/swagger.api/bulk-operations.yaml b/src/main/resources/swagger.api/bulk-operations.yaml index 19cf25ac7..ab5ff663a 100644 --- a/src/main/resources/swagger.api/bulk-operations.yaml +++ b/src/main/resources/swagger.api/bulk-operations.yaml @@ -312,6 +312,12 @@ paths: schema: type: integer description: The numbers of errors to return + - in: query + name: offset + required: true + schema: + type: integer + description: The query offset responses: '200': description: Collection of errors for preview diff --git a/src/test/java/org/folio/bulkops/service/ErrorServiceTest.java b/src/test/java/org/folio/bulkops/service/ErrorServiceTest.java index 243b6101d..efd705bab 100644 --- a/src/test/java/org/folio/bulkops/service/ErrorServiceTest.java +++ b/src/test/java/org/folio/bulkops/service/ErrorServiceTest.java @@ -18,10 +18,16 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static wiremock.org.hamcrest.MatcherAssert.assertThat; +import static wiremock.org.hamcrest.Matchers.anyOf; +import static wiremock.org.hamcrest.Matchers.contains; import static wiremock.org.hamcrest.Matchers.equalTo; +import static wiremock.org.hamcrest.Matchers.hasItems; import static wiremock.org.hamcrest.Matchers.hasSize; +import java.io.IOException; import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Paths; import java.time.LocalDate; import java.util.Arrays; import java.util.List; @@ -155,19 +161,15 @@ void shouldUploadErrorsAndReturnLinkToFile() { @ParameterizedTest @EnumSource(value = OperationStatusType.class, names = { "DATA_MODIFICATION", "COMPLETED" }, mode = EnumSource.Mode.INCLUDE) - void shouldGetErrorsPreviewByBulkOperationId(OperationStatusType statusType) { + void shouldGetErrorsPreviewByBulkOperationId(OperationStatusType statusType) throws IOException { try (var context = new FolioExecutionContextSetter(folioExecutionContext)) { var operationId = bulkOperationRepository.save(BulkOperation.builder().id(UUID.randomUUID()).dataExportJobId(UUID.randomUUID()).status(statusType).build()).getId(); - var expected = List.of( - new Error().message("No match found").parameters(List.of(new Parameter().key("IDENTIFIER").value("123"))), - new Error().message("Invalid format").parameters(List.of(new Parameter().key("IDENTIFIER").value("456"))) - ); - mockErrorsData(statusType, operationId); - var actual = errorService.getErrorsPreviewByBulkOperationId(operationId, 2); - assertThat(actual.getErrors(), hasSize(2)); + var actualWithOffset = errorService.getErrorsPreviewByBulkOperationId(operationId, 2, 1); + assertThat(actualWithOffset.getErrors(), hasSize(1)); + assertThat(actualWithOffset.getTotalRecords(), equalTo(2)); bulkOperationRepository.deleteById(operationId); } @@ -179,40 +181,7 @@ void shouldRejectErrorsPreviewOnWrongOperationStatus(OperationStatusType statusT try (var context = new FolioExecutionContextSetter(folioExecutionContext)) { var operationId = bulkOperationRepository.save(BulkOperation.builder().id(UUID.randomUUID()).status(statusType).build()).getId(); - assertThrows(NotFoundException.class, () -> errorService.getErrorsPreviewByBulkOperationId(operationId, 10)); - - bulkOperationRepository.deleteById(operationId); - } - } - - @ParameterizedTest - @EnumSource(value = OperationStatusType.class, names = { "DATA_MODIFICATION", "COMPLETED" }, mode = EnumSource.Mode.INCLUDE) - void shouldGetErrorsCsvByBulkOperationId(OperationStatusType statusType) { - try (var context = new FolioExecutionContextSetter(folioExecutionContext)) { - var operationId = bulkOperationRepository.save(BulkOperation.builder().id(UUID.randomUUID()).dataExportJobId(UUID.randomUUID()).status(statusType).build()).getId(); - - var expected = "123,No match found\n456,Invalid format".split(LF); - - mockErrorsData(statusType, operationId); - - var actual = errorService.getErrorsCsvByBulkOperationId(operationId).split(LF); - Arrays.sort(expected); - Arrays.sort(actual); - - assertArrayEquals(expected, actual); - assertThat(actual.length, equalTo(2)); - - bulkOperationRepository.deleteById(operationId); - } - } - - @ParameterizedTest - @EnumSource(value = OperationStatusType.class, names = { "DATA_MODIFICATION", "REVIEW_CHANGES", "COMPLETED", "COMPLETED_WITH_ERRORS", "REVIEWED_NO_MARC_RECORDS" }, mode = EnumSource.Mode.EXCLUDE) - void shouldRejectErrorsCsvOnWrongOperationStatus(OperationStatusType statusType) { - try (var context = new FolioExecutionContextSetter(folioExecutionContext)) { - var operationId = bulkOperationRepository.save(BulkOperation.builder().id(UUID.randomUUID()).status(statusType).build()).getId(); - - assertThrows(NotFoundException.class, () -> errorService.getErrorsCsvByBulkOperationId(operationId)); + assertThrows(NotFoundException.class, () -> errorService.getErrorsPreviewByBulkOperationId(operationId, 10, 1)); bulkOperationRepository.deleteById(operationId); } @@ -220,7 +189,7 @@ void shouldRejectErrorsCsvOnWrongOperationStatus(OperationStatusType statusType) @ParameterizedTest @ValueSource(ints = {0, 1}) - void shouldReturnErrorsPreviewOnCompletedWithErrors(int committedErrors) { + void shouldReturnErrorsPreviewOnCompletedWithErrors(int committedErrors) throws IOException { try (var context = new FolioExecutionContextSetter(folioExecutionContext)) { var jobId = UUID.randomUUID(); @@ -247,9 +216,10 @@ void shouldReturnErrorsPreviewOnCompletedWithErrors(int committedErrors) { .build()); } - var errors = errorService.getErrorsPreviewByBulkOperationId(operationId, 10); + var errors = errorService.getErrorsPreviewByBulkOperationId(operationId, 10, 0); assertThat(errors.getErrors(), hasSize(2)); + assertThat(errors.getTotalRecords(), equalTo(2)); } } @@ -276,7 +246,7 @@ void testOptimisticLockErrorProcessing() { .linkToFailedEntity(link) .build()); - var errors = errorService.getErrorsPreviewByBulkOperationId(operationId, 1); + var errors = errorService.getErrorsPreviewByBulkOperationId(operationId, 1, 0); assertThat(errors.getErrors(), hasSize(1)); assertThat(errors.getErrors().get(0).getParameters(), hasSize(2)); @@ -304,7 +274,10 @@ void testSaveErrorsFromDataImport(IdentifierType identifierType, boolean related final var instanceInfo = new RelatedInstanceInfo().withIdList(List.of(instanceId)).withHridList(List.of("instance HRID")); when(metadataProviderClient.getJobLogEntries(dataImportJobId.toString(), Integer.MAX_VALUE)) .thenReturn(new JobLogEntryCollection().withEntries(List.of(new JobLogEntry() - .withError("some MARC error").withSourceRecordId(sourceRecordId).withRelatedInstanceInfo( + .withError("some MARC error #1").withSourceRecordId(sourceRecordId).withRelatedInstanceInfo( + relatedInstanceInfo ? instanceInfo : new RelatedInstanceInfo().withIdList(List.of()).withHridList(List.of()) + ), new JobLogEntry() + .withError("some MARC error #2").withSourceRecordId(sourceRecordId).withRelatedInstanceInfo( relatedInstanceInfo ? instanceInfo : new RelatedInstanceInfo().withIdList(List.of()).withHridList(List.of()) )))); when(srsClient.getSrsRecordById(sourceRecordId)).thenReturn(new SrsRecord().withExternalIdsHolder( @@ -315,15 +288,16 @@ void testSaveErrorsFromDataImport(IdentifierType identifierType, boolean related .id(UUID.randomUUID()) .status(COMPLETED_WITH_ERRORS) .identifierType(identifierType) - .committedNumOfErrors(1) + .committedNumOfErrors(2) .dataExportJobId(dataExportJobId) .build()) .getId(); errorService.saveErrorsFromDataImport(operationId, dataImportJobId); - var errors = errorService.getErrorsPreviewByBulkOperationId(operationId, 1); + var errors = errorService.getErrorsPreviewByBulkOperationId(operationId, 2, 1); assertThat(errors.getErrors(), hasSize(1)); - assertEquals("some MARC error", errors.getErrors().get(0).getMessage()); + assertThat(errors.getTotalRecords(), equalTo(2)); + assertThat(errors.getErrors().stream().map(Error::getMessage).toList(), anyOf(contains("some MARC error #1"), contains("some MARC error #2"))); } } @@ -346,7 +320,7 @@ void shouldNotSaveError_IfErrorFromDataImportIsEmpty() { .build()) .getId(); errorService.saveErrorsFromDataImport(operationId, dataImportJobId); - var errors = errorService.getErrorsPreviewByBulkOperationId(operationId, 1); + var errors = errorService.getErrorsPreviewByBulkOperationId(operationId, 1, 1); assertThat(errors.getErrors(), hasSize(0)); } @@ -375,7 +349,7 @@ void testSaveErrorsFromDataImport_whenDiscardedAndNoMessage() { .build()) .getId(); errorService.saveErrorsFromDataImport(operationId, dataImportJobId); - var errors = errorService.getErrorsPreviewByBulkOperationId(operationId, 1); + var errors = errorService.getErrorsPreviewByBulkOperationId(operationId, 1, 0); assertThat(errors.getErrors(), hasSize(1)); assertEquals(DATA_IMPORT_ERROR_DISCARDED, errors.getErrors().get(0).getMessage()); @@ -404,12 +378,12 @@ void testDataImportException() { } } - private void mockErrorsData(OperationStatusType statusType, UUID operationId) { + private void mockErrorsData(OperationStatusType statusType, UUID operationId) throws IOException { if (DATA_MODIFICATION == statusType || COMPLETED_WITH_ERRORS == statusType) { - when(bulkEditClient.getErrorsPreview(any(UUID.class), anyInt())) - .thenReturn(new Errors() - .errors(List.of(new Error().type(ErrorType.ERROR).message("123,No match found"), - new Error().type(ErrorType.ERROR).message("456,Invalid format")))); + when(remoteFileSystemClient.get(any())) + .thenReturn(Files.newInputStream(Paths.get("src/test/resources/files/errors.csv"))); + when(remoteFileSystemClient.getNumOfLines(any())) + .thenReturn(2); } else { executionContentRepository.save(BulkOperationExecutionContent.builder() .bulkOperationId(operationId) diff --git a/src/test/resources/files/errors.csv b/src/test/resources/files/errors.csv index c61fb6beb..f174cb030 100644 --- a/src/test/resources/files/errors.csv +++ b/src/test/resources/files/errors.csv @@ -1 +1,2 @@ 12345678,Not Found +987654321,Bad data From abbe070ca375a1a68659e837aac0acc5cb13563d Mon Sep 17 00:00:00 2001 From: Viachaslau_Khandramai Date: Sun, 26 Jan 2025 02:06:40 +0100 Subject: [PATCH 2/5] Merge branch 'master' into MODBULKOPS-451 # Conflicts: # src/main/java/org/folio/bulkops/controller/BulkOperationController.java # src/main/java/org/folio/bulkops/service/ErrorService.java # src/main/resources/swagger.api/bulk-operations.yaml # src/test/java/org/folio/bulkops/service/ErrorServiceTest.java --- .../folio/bulkops/service/ErrorService.java | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/folio/bulkops/service/ErrorService.java b/src/main/java/org/folio/bulkops/service/ErrorService.java index 32e809375..5bcc86880 100644 --- a/src/main/java/org/folio/bulkops/service/ErrorService.java +++ b/src/main/java/org/folio/bulkops/service/ErrorService.java @@ -94,21 +94,19 @@ public void deleteErrorsByBulkOperationId(UUID bulkOperationId) { public Errors getErrorsPreviewByBulkOperationId(UUID bulkOperationId, int limit, int offset, ErrorType errorType) { var bulkOperation = operationRepository.findById(bulkOperationId) .orElseThrow(() -> new NotFoundException("BulkOperation was not found by id=" + bulkOperationId)); - if (Set.of(DATA_MODIFICATION, REVIEW_CHANGES, REVIEWED_NO_MARC_RECORDS).contains(bulkOperation.getStatus()) || COMPLETED_WITH_ERRORS == bulkOperation.getStatus() - && noCommittedErrors(bulkOperation) - && noCommittedWarnings(bulkOperation)) { - var errors = new BufferedReader(new InputStreamReader(remoteFileSystemClient.get(bulkOperation.getLinkToMatchedRecordsErrorsCsvFile()))) - .lines() - .skip(offset) - .limit(limit) - .map(message -> { - var error = message.split(Constants.COMMA_DELIMETER); - return new Error().message(error[1]).parameters(List.of(new Parameter().key(IDENTIFIER).value(error[0]))); - }) - .collect(toList()); - return new Errors().errors(errors) - .totalRecords(remoteFileSystemClient.getNumOfLines(bulkOperation.getLinkToMatchedRecordsErrorsCsvFile())); - } else if (COMPLETED == bulkOperation.getStatus() || COMPLETED_WITH_ERRORS == bulkOperation.getStatus()) { + if (Set.of(DATA_MODIFICATION, REVIEW_CHANGES, REVIEWED_NO_MARC_RECORDS).contains(bulkOperation.getStatus()) || COMPLETED_WITH_ERRORS == bulkOperation.getStatus() && noCommittedErrors(bulkOperation) && noCommittedWarnings(bulkOperation)) { + var errors = new BufferedReader(new InputStreamReader(remoteFileSystemClient.get(bulkOperation.getLinkToMatchedRecordsErrorsCsvFile()))) + .lines() + .skip(offset) + .limit(limit) + .map(message -> { + var error = message.split(Constants.COMMA_DELIMETER); + return new Error().message(error[1]).parameters(List.of(new Parameter().key(IDENTIFIER).value(error[0]))); + }) + .collect(toList()); + return new Errors().errors(errors) + .totalRecords(remoteFileSystemClient.getNumOfLines(bulkOperation.getLinkToMatchedRecordsErrorsCsvFile())); + } else if (COMPLETED == bulkOperation.getStatus() || COMPLETED_WITH_ERRORS == bulkOperation.getStatus()) { return getExecutionErrors(bulkOperationId, limit, offset, errorType); } else { throw new NotFoundException("Errors preview is not available"); From 58936d79b2f7baada05a077e098eca47b50dfeb9 Mon Sep 17 00:00:00 2001 From: Viachaslau_Khandramai Date: Wed, 29 Jan 2025 14:07:20 +0100 Subject: [PATCH 3/5] MODBULKOPS-451 - Rework errors preview --- .../BulkOperationExecutionContentRepository.java | 4 +++- src/main/java/org/folio/bulkops/service/ErrorService.java | 8 +++++++- src/main/resources/swagger.api/bulk-operations.yaml | 4 ++++ src/main/resources/swagger.api/schemas/errors.json | 2 +- 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/folio/bulkops/repository/BulkOperationExecutionContentRepository.java b/src/main/java/org/folio/bulkops/repository/BulkOperationExecutionContentRepository.java index d17bcfbab..ed2312e0e 100644 --- a/src/main/java/org/folio/bulkops/repository/BulkOperationExecutionContentRepository.java +++ b/src/main/java/org/folio/bulkops/repository/BulkOperationExecutionContentRepository.java @@ -8,6 +8,7 @@ import org.folio.spring.data.OffsetRequest; import org.springframework.data.domain.Page; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; import org.springframework.stereotype.Repository; @Repository @@ -16,6 +17,7 @@ public interface BulkOperationExecutionContentRepository extends JpaRepository findByBulkOperationIdAndErrorMessageIsNotNullAndErrorTypeIsOrderByErrorType(UUID bulkOperationId, OffsetRequest offsetRequest, ErrorType errorType); Optional findFirstByBulkOperationIdAndIdentifier(UUID bulkOperationId, String identifier); int countAllByBulkOperationIdAndErrorMessageIsNotNullAndErrorTypeIs(UUID bulkOperationId, ErrorType errorType); - + @Query("SELECT COUNT(i) FROM BulkOperationExecutionContent i WHERE i.bulkOperationId = :bulkOperationId") + long countByBulkOperationId(UUID bulkOperationId); void deleteByBulkOperationId(UUID bulkOperationId); } diff --git a/src/main/java/org/folio/bulkops/service/ErrorService.java b/src/main/java/org/folio/bulkops/service/ErrorService.java index 5bcc86880..4a1f7d38c 100644 --- a/src/main/java/org/folio/bulkops/service/ErrorService.java +++ b/src/main/java/org/folio/bulkops/service/ErrorService.java @@ -160,6 +160,12 @@ public String getErrorsCsvByBulkOperationId(UUID bulkOperationId, int offset, Er } private Errors getExecutionErrors(UUID bulkOperationId, int limit, int offset, ErrorType errorType) { + var totalRecords = (int) executionContentRepository.countByBulkOperationId(bulkOperationId); + if (limit == 0) { + return new Errors() + .errors(List.of()) + .totalRecords(totalRecords); + } Page errorPage; if (isNull(errorType)) { errorPage = executionContentRepository.findByBulkOperationIdAndErrorMessageIsNotNullOrderByErrorType(bulkOperationId, OffsetRequest.of(offset, limit)); @@ -171,7 +177,7 @@ private Errors getExecutionErrors(UUID bulkOperationId, int limit, int offset, E .toList(); return new Errors() .errors(errors) - .totalRecords((int) errorPage.getTotalElements()); + .totalRecords(totalRecords); } /** diff --git a/src/main/resources/swagger.api/bulk-operations.yaml b/src/main/resources/swagger.api/bulk-operations.yaml index e4f1d14d4..8800bcbcd 100644 --- a/src/main/resources/swagger.api/bulk-operations.yaml +++ b/src/main/resources/swagger.api/bulk-operations.yaml @@ -311,12 +311,16 @@ paths: required: true schema: type: integer + default: 10 + minimum: 0 description: The numbers of errors to return - in: query name: offset required: false schema: type: integer + default: 0 + minimum: 0 description: Query offset - in: query name: errorType diff --git a/src/main/resources/swagger.api/schemas/errors.json b/src/main/resources/swagger.api/schemas/errors.json index 262434968..6bc3c82f8 100644 --- a/src/main/resources/swagger.api/schemas/errors.json +++ b/src/main/resources/swagger.api/schemas/errors.json @@ -11,7 +11,7 @@ "$ref": "error.json" } }, - "total_records": { + "totalRecords": { "description": "Total number of errors", "type": "integer" } From 798664fe233eae443831995dd9693df60a49486c Mon Sep 17 00:00:00 2001 From: Viachaslau_Khandramai Date: Wed, 29 Jan 2025 14:43:51 +0100 Subject: [PATCH 4/5] MODBULKOPS-451 - Rework errors preview --- .../bulkops/service/ErrorServiceTest.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/test/java/org/folio/bulkops/service/ErrorServiceTest.java b/src/test/java/org/folio/bulkops/service/ErrorServiceTest.java index c5cf9d4f4..2c0f8f913 100644 --- a/src/test/java/org/folio/bulkops/service/ErrorServiceTest.java +++ b/src/test/java/org/folio/bulkops/service/ErrorServiceTest.java @@ -252,6 +252,42 @@ void shouldReturnErrorsPreviewOnCompletedWithErrors(int committedErrors) throws } } + @ParameterizedTest + @ValueSource(ints = {0, 1}) + void shouldReturnOnlyErrorsTotalNumberPreviewOnCompletedWithErrors(int committedErrors) throws IOException { + try (var context = new FolioExecutionContextSetter(folioExecutionContext)) { + var jobId = UUID.randomUUID(); + + var operationId = bulkOperationRepository.save(BulkOperation.builder() + .id(UUID.randomUUID()) + .status(COMPLETED_WITH_ERRORS) + .committedNumOfErrors(committedErrors) + .dataExportJobId(jobId) + .build()) + .getId(); + + mockErrorsData(COMPLETED_WITH_ERRORS, operationId); + + if (committedErrors == 1) { + executionContentRepository.save(BulkOperationExecutionContent.builder() + .bulkOperationId(operationId) + .identifier("123") + .errorMessage("No match found") + .build()); + executionContentRepository.save(BulkOperationExecutionContent.builder() + .bulkOperationId(operationId) + .identifier("456") + .errorMessage("Invalid format") + .build()); + } + + var errors = errorService.getErrorsPreviewByBulkOperationId(operationId, 0, 0, null); + + assertThat(errors.getErrors(), hasSize(0)); + assertThat(errors.getTotalRecords(), equalTo(2)); + } + } + @Test void testOptimisticLockErrorProcessing() { From de6a98bbbe7664b1be1e04656c283638e506a91d Mon Sep 17 00:00:00 2001 From: Viachaslau_Khandramai Date: Wed, 29 Jan 2025 14:51:47 +0100 Subject: [PATCH 5/5] MODBULKOPS-451 - Rework errors preview --- src/main/java/org/folio/bulkops/service/ErrorService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/folio/bulkops/service/ErrorService.java b/src/main/java/org/folio/bulkops/service/ErrorService.java index 517b2129f..7dda034a4 100644 --- a/src/main/java/org/folio/bulkops/service/ErrorService.java +++ b/src/main/java/org/folio/bulkops/service/ErrorService.java @@ -106,7 +106,7 @@ public Errors getErrorsPreviewByBulkOperationId(UUID bulkOperationId, int limit, var error = message.split(Constants.COMMA_DELIMETER); return new Error().message(error[1]).parameters(List.of(new Parameter().key(IDENTIFIER).value(error[0]))); }) - .collect(toList()); + .toList(); return new Errors().errors(errors) .totalRecords(remoteFileSystemClient.getNumOfLines(bulkOperation.getLinkToMatchedRecordsErrorsCsvFile())); } else if (COMPLETED == bulkOperation.getStatus() || COMPLETED_WITH_ERRORS == bulkOperation.getStatus()) {