From c57ee4fa08674441215981660bc22705ee7cdaaa Mon Sep 17 00:00:00 2001 From: Andrei Isvoran Date: Mon, 15 Jul 2024 09:51:37 +0200 Subject: [PATCH] RED-9622 - Introduce csv validation --- .../FileAttributesManagementService.java | 28 ++++++++- .../v1/server/integration/tests/FileTest.java | 59 ++++++++++++++----- 2 files changed, 71 insertions(+), 16 deletions(-) diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/FileAttributesManagementService.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/FileAttributesManagementService.java index 126491996..16b0e3e50 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/FileAttributesManagementService.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/FileAttributesManagementService.java @@ -20,8 +20,6 @@ import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; -import jakarta.transaction.Transactional; - import org.apache.commons.validator.routines.DateValidator; import org.springframework.stereotype.Service; @@ -45,7 +43,9 @@ import com.opencsv.CSVParserBuilder; import com.opencsv.CSVReader; import com.opencsv.CSVReaderBuilder; import com.opencsv.exceptions.CsvException; +import com.opencsv.exceptions.CsvValidationException; +import jakarta.transaction.Transactional; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -73,6 +73,8 @@ public class FileAttributesManagementService { FileAttributesGeneralConfigurationEntity generalConfiguration = fileAttributeConfigPersistenceService.getFileAttributesGeneralConfiguration(dossier.getDossierTemplateId()); List configuration = fileAttributeConfigPersistenceService.getFileAttributes(dossier.getDossierTemplateId()); + validateCsv(importCsvRequest.getCsvFile(), generalConfiguration.getDelimiter(), generalConfiguration.getEncoding()); + Map fileStatusByFilename = fileStatusService.getDossierStatus(dossierId) .stream() .filter(f -> !f.isSoftOrHardDeleted()) @@ -136,6 +138,28 @@ public class FileAttributesManagementService { } + @SuppressWarnings("checkstyle:all") + private void validateCsv(byte[] csvFileBytes, String delimiter, String encoding) { + + if (delimiter.length() != 1) { + throw new IllegalArgumentException("Delimiter must be a single character."); + } + char delimiterChar = delimiter.charAt(0); + Charset charset = Charset.forName(encoding); + + try (CSVReader csvReader = new CSVReaderBuilder(new InputStreamReader(new ByteArrayInputStream(csvFileBytes), charset)).withCSVParser(new CSVParserBuilder().withSeparator( + delimiterChar).build()).build()) { + + while (csvReader.readNext() != null) { + // Intentionally empty: reading lines for validation purposes + } + + } catch (IOException | CsvValidationException e) { + throw new BadRequestException("Invalid CSV file format: " + e.getMessage(), e); + } + } + + @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") private List> getCsvRecords(byte[] csv, String delimiter, String encoding) { diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/FileTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/FileTest.java index 67953922d..a5ca14b1b 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/FileTest.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/integration/tests/FileTest.java @@ -1,22 +1,28 @@ package com.iqser.red.service.peristence.v1.server.integration.tests; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.Mockito.*; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.any; import static org.mockito.Mockito.when; import java.io.ByteArrayInputStream; import java.nio.charset.StandardCharsets; import java.time.OffsetDateTime; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; +import org.apache.commons.codec.binary.Base64; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.mock.web.MockMultipartFile; import com.google.common.collect.Sets; @@ -38,8 +44,10 @@ import com.iqser.red.service.peristence.v1.server.integration.service.FileTester import com.iqser.red.service.peristence.v1.server.integration.service.TypeProvider; import com.iqser.red.service.peristence.v1.server.integration.service.UserProvider; import com.iqser.red.service.peristence.v1.server.integration.utils.AbstractPersistenceServerServiceTest; +import com.iqser.red.service.persistence.management.v1.processor.entity.configuration.FileAttributesGeneralConfigurationEntity; import com.iqser.red.service.persistence.management.v1.processor.service.FileManagementStorageService; import com.iqser.red.service.persistence.management.v1.processor.service.FileStatusService; +import com.iqser.red.service.persistence.management.v1.processor.service.persistence.FileAttributeConfigPersistenceService; import com.iqser.red.service.persistence.service.v1.api.shared.model.DossierTemplateModel; import com.iqser.red.service.persistence.service.v1.api.shared.model.FileAttributes; import com.iqser.red.service.persistence.service.v1.api.shared.model.FileAttributesConfig; @@ -122,7 +130,6 @@ public class FileTest extends AbstractPersistenceServerServiceTest { @Autowired private FileStatusService fileStatusService; - @Test public void testFileSoftDeleteReupload() { @@ -430,7 +437,13 @@ public class FileTest extends AbstractPersistenceServerServiceTest { .build())); manualRedactionClient.recategorizeBulk(dossierId, fileId, - Set.of(RecategorizationRequestModel.builder().annotationId(annotationId).comment("comment").type("new-type").legalBasis(null).section("section").build()), + Set.of(RecategorizationRequestModel.builder() + .annotationId(annotationId) + .comment("comment") + .type("new-type") + .legalBasis(null) + .section("section") + .build()), false); var loadedFile = fileClient.getFileStatus(dossierId, fileId); @@ -477,17 +490,17 @@ public class FileTest extends AbstractPersistenceServerServiceTest { fileClient.setStatusUnderReview(dossier.getId(), file.getId(), userId); manualRedactionClient.addRedactionBulk(dossierId, - fileId, - Set.of(AddRedactionRequestModel.builder() - .addToDictionary(true) - .addToAllDossiers(true) - .comment(new AddCommentRequestModel("comment")) - .type(type.getType()) - .reason("1") - .value("test") - .legalBasis("1") - .dictionaryEntryType(DictionaryEntryType.ENTRY) - .build())); + fileId, + Set.of(AddRedactionRequestModel.builder() + .addToDictionary(true) + .addToAllDossiers(true) + .comment(new AddCommentRequestModel("comment")) + .type(type.getType()) + .reason("1") + .value("test") + .legalBasis("1") + .dictionaryEntryType(DictionaryEntryType.ENTRY) + .build())); var loadedFile = fileClient.getFileStatus(dossier.getId(), file.getId()); @@ -665,4 +678,22 @@ public class FileTest extends AbstractPersistenceServerServiceTest { assertThat(fileStatusService.getFileName(file.getId())).isEqualTo(filename + ".pdf"); } + + @Test + public void testUploadWronglyFormattedCsv() { + + FileAttributeConfigPersistenceService fileAttributeConfigPersistenceService = Mockito.mock(FileAttributeConfigPersistenceService.class); + String fileName = "file"; + var dossier = dossierTesterAndProvider.provideTestDossier(); + String malformedCsvContent = "Path,\"Document Title\",\"Major Version Number\",\"Minor Version Number\",\"Vertebrate Study\"\n\"402Study.pdf\",\"My Title\",\"4.636.0,\"4.363.0\",\"4.363.0\""; + var fileId = Base64.encodeBase64String((dossier.getId() + fileName).getBytes(StandardCharsets.UTF_8)); + var malformedCsvFile = new MockMultipartFile(fileName + ".csv", fileName + ".csv", "text/csv", malformedCsvContent.getBytes()); + + + fileManagementStorageService.storeObject(dossier.getId(), fileId, FileType.UNTOUCHED, new ByteArrayInputStream("test".getBytes(StandardCharsets.UTF_8))); + when(fileAttributeConfigPersistenceService.getFileAttributesGeneralConfiguration(anyString())).thenReturn(FileAttributesGeneralConfigurationEntity.builder().delimiter(",").encoding("UTF-8").build()); + when(fileAttributeConfigPersistenceService.getFileAttributes(anyString())).thenReturn(Collections.emptyList()); + assertThrows(FeignException.class, () -> uploadClient.upload(malformedCsvFile, dossier.getId(), false)); + } + }