OLMIS-8026 Improve csv files validation (#126)

* OLMIS-8026 Add validation for all files inside zip instead of skipping them * OLMIS-8026 Update changelog
OpenLMIS · Oct 30, 2024 · 599318a · 599318a
1 parent bb8b64c
commit 599318a
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@ Improvements:
 * [OLMIS-7989](https://openlmis.atlassian.net/browse/OLMIS-7989): Extension of import functionality by adding possibility to import 
 facilities and supported programs.
   * Improved performance. 
+  * Improved csv files validation
 * [OLMIS-7895](https://openlmis.atlassian.net/browse/OLMIS-7895): Add demo data for BUQ and TB Monthly
 * [OLMIS-7988](https://openlmis.atlassian.net/browse/OLMIS-7988): Add DHIS2 integration rights
 * [OLMIS-7953](https://openlmis.atlassian.net/browse/OLMIS-7953): Improve some API calls performance

diff --git a/src/main/java/org/openlmis/referencedata/service/export/DataImportService.java b/src/main/java/org/openlmis/referencedata/service/export/DataImportService.java
@@ -20,6 +20,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+
 import org.openlmis.referencedata.dto.BaseDto;
 import org.openlmis.referencedata.exception.ValidationMessageException;
 import org.openlmis.referencedata.util.FileHelper;
@@ -61,6 +62,11 @@ public List<BaseDto> importData(MultipartFile zipFile, Profiler profiler)
     profiler.start("CONVERT_TO_ZIP_FILE_MAP");
     final Map<String, InputStream> fileMap = fileHelper.convertMultipartFileToZipFileMap(zipFile);
 
+    profiler.start("VALIDATE_CSV_FILES");
+    for (String fileName : fileMap.keySet()) {
+      fileHelper.validateCsvFile(fileName, IMPORT_ORDER);
+    }
+
     final List<BaseDto> result = new ArrayList<>();
     for (String importFileName : IMPORT_ORDER) {
       final InputStream fileStream = fileMap.get(importFileName);
@@ -71,10 +77,6 @@ public List<BaseDto> importData(MultipartFile zipFile, Profiler profiler)
 
       try {
         final Profiler entryProfiler = profiler.startNested("IMPORT_ZIP_ENTRY: " + importFileName);
-
-        entryProfiler.start("VALIDATE_CSV_FILE");
-        fileHelper.validateCsvFile(importFileName);
-
         final DataImportPersister<?, ?, ? extends BaseDto> persister =
             beanFactory.getBean(importFileName, DataImportPersister.class);
         result.addAll(persister.processAndPersist(fileStream, entryProfiler));

diff --git a/src/main/java/org/openlmis/referencedata/util/FileHelper.java b/src/main/java/org/openlmis/referencedata/util/FileHelper.java
@@ -134,8 +134,16 @@ public void validateMultipartFile(MultipartFile multipartFile) {
    * @param fileName the name of CSV file
    * @throws ValidationMessageException if any of check fails
    */
-  public void validateCsvFile(String fileName) {
+  public void validateCsvFile(String fileName, List<String> expectedFileNames) {
     hasExpectedExtension(fileName, ".csv");
+    hasExpectedName(fileName, expectedFileNames);
+  }
+
+  private void hasExpectedName(String fileName, List<String> expectedFileNames) {
+    if (!expectedFileNames.contains(fileName)) {
+      throw new ValidationMessageException(
+              new Message(CsvUploadMessageKeys.ERROR_FILE_NAME_INVALID, fileName));
+    }
   }
 
   private void hasExpectedExtension(String fileName, String expectedExtension) {

diff --git a/src/test/java/org/openlmis/referencedata/util/FileHelperTest.java b/src/test/java/org/openlmis/referencedata/util/FileHelperTest.java
@@ -23,6 +23,8 @@
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipOutputStream;
@@ -38,6 +40,14 @@
 @RunWith(MockitoJUnitRunner.class)
 public class FileHelperTest {
 
+  private static final List<String> IMPORTABLE_FILES =
+          Arrays.asList(
+                  "facility.csv",
+                  "supportedProgram.csv",
+                  "orderable.csv",
+                  "programOrderable.csv",
+                  "tradeItem.csv");
+
   @InjectMocks
   private FileHelper fileHelper;
 
@@ -84,8 +94,14 @@ private byte[] createValidZipFileContent() throws IOException {
 
   @Test(expected = ValidationMessageException.class)
   public void shouldThrowExceptionWhenValidateCsvWithWrongExtension() {
-    String fileName = "test.txt";
-    fileHelper.validateCsvFile(fileName);
+    String fileName = "facility.txt";
+    fileHelper.validateCsvFile(fileName, IMPORTABLE_FILES);
+  }
+
+  @Test(expected = ValidationMessageException.class)
+  public void shouldThrowExceptionWhenValidateCsvWithWrongFileName() {
+    String fileName = "invalidName.csv";
+    fileHelper.validateCsvFile(fileName, IMPORTABLE_FILES);
   }
 
   @Test(expected = ValidationMessageException.class)