Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ public enum BulkUploadErrorCode {
EMPTY_FILE("ADM-BLK-006", "Empty file is not acceptable please provide valid file"),
NO_FILE("ADM-BLK-007", "No file uploaded"),
ENTRY_EXISTS_SAME_IDENTIFIER("ADM-BLK-008", "Entry found with same primary key values"),
BATCH_ERROR("ADM-BLK-009", "Failed to process entry");
BATCH_ERROR("ADM-BLK-009", "Failed to process entry"),
INVALID_DELIMITER("ADM-BLK-010","Invalid delimiter used");


private final String errorCode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
import io.mosip.admin.packetstatusupdater.util.AuditUtil;
import io.mosip.admin.packetstatusupdater.util.EventEnum;
import io.mosip.kernel.core.util.EmptyCheckUtils;
import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityManagerFactory;
import jakarta.validation.Validator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.batch.core.Job;
Expand Down Expand Up @@ -57,11 +60,9 @@
import org.springframework.web.client.RestTemplate;
import org.springframework.web.multipart.MultipartFile;

import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityManagerFactory;
import javax.sql.DataSource;
import jakarta.validation.Validator;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.sql.Timestamp;
import java.time.LocalDate;
import java.time.LocalDateTime;
Expand Down Expand Up @@ -252,16 +253,30 @@ private BulkDataResponseDto importDataFromFile(String tableName, String operatio
try {
logger.info("Is file empty ? {}, file-size :{}", file.isEmpty(), file.getSize());
if (file.isEmpty()) {
bulkUploadTranscation.setStatusCode("FAILED");
updateBulkUploadTransaction(bulkUploadTranscation);
throw new RequestException(BulkUploadErrorCode.EMPTY_FILE.getErrorCode(),
BulkUploadErrorCode.EMPTY_FILE.getErrorMessage());
}

if (!file.getOriginalFilename().endsWith(".csv") && !file.getOriginalFilename().endsWith(".xls") && !file.getOriginalFilename().endsWith(".xlsx")) {
bulkUploadTranscation.setStatusCode("FAILED");
updateBulkUploadTransaction(bulkUploadTranscation);
throw new RequestException(BulkUploadErrorCode.INVALID_FILE_FORMAT.getErrorCode(),
BulkUploadErrorCode.INVALID_FILE_FORMAT.getErrorMessage());
}

Boolean isCSV = file.getOriginalFilename().endsWith(".csv");

try (BufferedReader reader = new BufferedReader(new InputStreamReader(file.getInputStream()))) {
String firstLine = reader.readLine();
if (firstLine != null && !firstLine.contains(lineDelimiter)) {
bulkUploadTranscation.setStatusCode("FAILED");
updateBulkUploadTransaction(bulkUploadTranscation);
throw new RequestException(BulkUploadErrorCode.INVALID_DELIMITER.getErrorCode(),
"Invalid delimiter used. Expected delimiter: " + lineDelimiter);
Comment on lines +276 to +277
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Minor: Error message deviates from enum message.

Unlike other error handling in this method (lines 258-259, 265-266), this code appends additional context to the enum's error message. While the extra information about the expected delimiter is helpful for debugging, it's inconsistent with the established pattern.

Consider either:

  1. Using only the enum message for consistency: BulkUploadErrorCode.INVALID_DELIMITER.getErrorMessage()
  2. Updating the enum to include a placeholder: INVALID_DELIMITER("ADM-BLK-010","Invalid delimiter used. Expected: %s")
🤖 Prompt for AI Agents
In
admin/admin-service/src/main/java/io/mosip/admin/bulkdataupload/service/impl/BulkDataUploadServiceImpl.java
around lines 276 to 277, the thrown RequestException appends extra context to
the enum message causing inconsistency with other error handling; change it to
use the enum message directly or update the enum to support a formatted message.
Either replace the concatenated string with
BulkUploadErrorCode.INVALID_DELIMITER.getErrorMessage() to match existing
pattern, or modify the enum to add a placeholder (e.g., "Invalid delimiter used.
Expected: %s") and format that enum message with the expected delimiter before
throwing the RequestException.

}
}
Comment on lines 269 to +279
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Delimiter validation incorrectly applied to Excel files.

The delimiter validation (lines 271-279) executes for ALL file types, including Excel (.xls, .xlsx), which don't use text delimiters. This contradicts the past review discussion where it was confirmed that delimiter checking is only for CSV files.

Excel files will either:

  • Fail validation incorrectly (if the binary content doesn't contain the delimiter character)
  • Pass validation incorrectly (if the binary content happens to contain the delimiter character)

Apply this diff to restrict validation to CSV files only:

 Boolean isCSV = file.getOriginalFilename().endsWith(".csv");
-
-try (BufferedReader reader = new BufferedReader(new InputStreamReader(file.getInputStream()))) {
-    String firstLine = reader.readLine();
-    if (firstLine != null && !firstLine.contains(lineDelimiter)) {
-        bulkUploadTranscation.setStatusCode("FAILED");
-        updateBulkUploadTransaction(bulkUploadTranscation);
-        throw new RequestException(BulkUploadErrorCode.INVALID_DELIMITER.getErrorCode(),
-                "Invalid delimiter used. Expected delimiter: " + lineDelimiter);
-    }
-}
+
+if (isCSV) {
+    try (BufferedReader reader = new BufferedReader(new InputStreamReader(file.getInputStream()))) {
+        String firstLine = reader.readLine();
+        if (firstLine != null && !firstLine.contains(lineDelimiter)) {
+            bulkUploadTranscation.setStatusCode("FAILED");
+            updateBulkUploadTransaction(bulkUploadTranscation);
+            throw new RequestException(BulkUploadErrorCode.INVALID_DELIMITER.getErrorCode(),
+                    "Invalid delimiter used. Expected delimiter: " + lineDelimiter);
+        }
+    }
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Boolean isCSV = file.getOriginalFilename().endsWith(".csv");
try (BufferedReader reader = new BufferedReader(new InputStreamReader(file.getInputStream()))) {
String firstLine = reader.readLine();
if (firstLine != null && !firstLine.contains(lineDelimiter)) {
bulkUploadTranscation.setStatusCode("FAILED");
updateBulkUploadTransaction(bulkUploadTranscation);
throw new RequestException(BulkUploadErrorCode.INVALID_DELIMITER.getErrorCode(),
"Invalid delimiter used. Expected delimiter: " + lineDelimiter);
}
}
Boolean isCSV = file.getOriginalFilename().endsWith(".csv");
if (isCSV) {
try (BufferedReader reader = new BufferedReader(new InputStreamReader(file.getInputStream()))) {
String firstLine = reader.readLine();
if (firstLine != null && !firstLine.contains(lineDelimiter)) {
bulkUploadTranscation.setStatusCode("FAILED");
updateBulkUploadTransaction(bulkUploadTranscation);
throw new RequestException(BulkUploadErrorCode.INVALID_DELIMITER.getErrorCode(),
"Invalid delimiter used. Expected delimiter: " + lineDelimiter);
}
}
}
🤖 Prompt for AI Agents
In
admin/admin-service/src/main/java/io/mosip/admin/bulkdataupload/service/impl/BulkDataUploadServiceImpl.java
around lines 269 to 279, the delimiter validation is currently applied to all
uploaded files; restrict this validation to CSV files only by using the existing
isCSV boolean. Wrap the BufferedReader/firstLine delimiter check in an if
(isCSV) { ... } block (or guard the specific validation logic with if (isCSV))
so Excel files (.xls, .xlsx) skip this text-delimiter check; keep the rest of
the error handling (setting status to FAILED, updateBulkUploadTransaction and
throwing RequestException) unchanged when the validation runs for CSVs.


auditUtil.setAuditRequestDto(EventEnum.getEventEnumWithValue(EventEnum.BULKDATA_UPLOAD_CSV,
operation + " from " + file.getOriginalFilename()),null);
Expand Down Expand Up @@ -318,7 +333,9 @@ public BulkDataResponseDto bulkDataOperation(String tableName, String operation,


private void updateBulkUploadTransaction(BulkUploadTranscation bulkUploadTranscation) {
bulkUploadTranscation.setStatusCode("COMPLETED");
if (bulkUploadTranscation.getStatusCode() == null || !bulkUploadTranscation.getStatusCode().equalsIgnoreCase("FAILED")) {
bulkUploadTranscation.setStatusCode("COMPLETED");
}
bulkUploadTranscation.setUploadedDateTime(Timestamp.valueOf(LocalDateTime.now(ZoneId.of("UTC"))));
bulkUploadTranscation.setUpdatedBy("JOB");
bulkTranscationRepo.save(bulkUploadTranscation);
Expand Down