Skip to content

Commit

Permalink
Merge pull request #55 from companieshouse/IDVA5-1788-ImproveErrorLog…
Browse files Browse the repository at this point in the history
…ging

improve error logging
  • Loading branch information
sardhani-ch authored Feb 18, 2025
2 parents 1ce96b8 + 8c37ee5 commit d1b3260
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 19 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
<dependency>
<groupId>software.amazon.awssdk</groupId>
<artifactId>s3</artifactId>
<version>2.29.31</version>
<version>2.30.21</version>
<exclusions>
<exclusion>
<groupId>io.netty</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,16 @@ public CsvValidationController( final FileTransferService fileTransferService, T
@Override
public ResponseEntity<FileUploadResponse> uploadFile(MultipartFile file, @Valid String metadata){
try {
String fileType = tika.detect(file.getInputStream(), file.getOriginalFilename());
if (!fileType.equals("text/csv")){
throw new BadRequestRuntimeException("Please upload a valid CSV file");
}

var objectMapper = new ObjectMapper();
var fileMetaData = objectMapper.readValue(metadata, FileMetaData.class);
if(StringUtils.isEmpty(fileMetaData.getFileName()) || StringUtils.isEmpty(fileMetaData.getFromLocation()) || StringUtils.isEmpty(fileMetaData.getToLocation())){
throw new BadRequestRuntimeException("Please provide a valid metadata: " + fileMetaData);
}
String fileType = tika.detect(file.getInputStream(), file.getOriginalFilename());
if (!fileType.equals("text/csv")){
throw new BadRequestRuntimeException(String.format("Please upload a valid CSV file. fileName: %s, amlBodyName: %s", fileMetaData.getFileName(), fileMetaData.getFromLocation()));
}
String id = fileTransferService.upload(file, fileMetaData);
return ResponseEntity.created(URI.create(Constants.UPLOAD_URI_PATTERN)).body(new FileUploadResponse().id(id));
} catch (FileUploadException | IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.io.Reader;
import java.util.Iterator;
import java.util.List;
import java.util.stream.Collectors;

import org.apache.commons.csv.CSVFormat;
import org.apache.commons.csv.CSVRecord;
Expand Down Expand Up @@ -46,7 +47,7 @@ public void parseRecords(byte[] bytesToParse) {
CSVRecord record = it.next();

if (!NUMBER_OF_COLUMNS.equals(record.size())) {
throw new CSVDataValidationException("Incorrect number of columns");
throw new CSVDataValidationException(String.format("Incorrect number of columns. Received: %s Expected: %s", record.size(), NUMBER_OF_COLUMNS ));
}
CsvRecordValidator.validateUniqueId(record.get(INDEX_OF_UNIQUE_ID));
CsvRecordValidator.validateRegisteredCompanyName(record.get(INDEX_OF_COMPANY_NAME));
Expand Down Expand Up @@ -82,8 +83,10 @@ private void isValidFieldHeaders(CSVRecord csvRecord) {
return trimmed.toLowerCase();
})
.toList();
if (!actualHeaders.equals(VALID_HEADERS)) {
throw new CSVDataValidationException("Headers did not match expected headers");
List<String> mismatchedHeaders = VALID_HEADERS.stream()
.filter(element -> !actualHeaders.contains(element)).collect(Collectors.toList());
if (!mismatchedHeaders.isEmpty()) {
throw new CSVDataValidationException(String.format("Headers did not match expected headers, following headers are missing: %s", mismatchedHeaders));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,15 @@ public void processFiles() {
recordsToProcess.forEach(recordToProcess -> {
Optional<FileApi> downloadedFile = Optional.empty();
try {
LOGGER.info("Processing record with id: " + recordToProcess.getId());
LOGGER.info(String.format("Processing record with id: %s, fileName: %s, amlBodyName: %s", recordToProcess.getId(), recordToProcess.getFileName(), recordToProcess.getFromLocation()));
fileValidationRepository.updateStatusAndRemoveErrorMessageById(recordToProcess.getId(), FileStatus.IN_PROGRESS.getLabel(), LocalDateTime.now(), SYSTEM);
downloadedFile = fileTransferService.get(recordToProcess.getFileId());
csvProcessor.parseRecords(downloadedFile.get().getBody());
s3UploadClient.uploadFile(downloadedFile.get().getBody(),
recordToProcess.getFileName(),
recordToProcess.getToLocation());
fileValidationRepository.updateStatusById(recordToProcess.getId(), FileStatus.COMPLETED.getLabel(), LocalDateTime.now(), SYSTEM);
LOGGER.info(String.format("Processing finished for record with id: %s, fileName: %s, amlBodyName: %s", recordToProcess.getId(), recordToProcess.getFileName(), recordToProcess.getFromLocation()));
} catch (FileDownloadException e) {
var errorMessage = String.format("Failed to download file: %s with message %s", recordToProcess.getId(), e.getMessage());
LOGGER.error(errorMessage);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ void testUploadIncorrectMetaDataReturnsStatus400() throws IOException {
String metaData = "{\"fileName\":\"Test file\",\"toLocation\":\"S3:abc\"}";

// When
when(tika.detect(any(InputStream.class), any(String.class))).thenReturn("text/csv");
var objectMapper = new ObjectMapper();
var fileMetaData = objectMapper.readValue(metaData, FileMetaData.class);

Expand All @@ -93,16 +92,17 @@ void testUploadIncorrectMetaDataReturnsStatus400() throws IOException {
void testUploadInvalidFileContentReturnsStatus400() throws IOException {
// Given
MultipartFile file = new MockMultipartFile("abc", "test.png", "", "Hello world".getBytes() );
String metaData = "{\"fileName\":\"Test file\",\"fromLocation\":\"abc\",\"toLocation\":\"S3:abc\"}";
String metaData = "{\"fileName\":\"Test file\",\"fromLocation\":\"amlBodyName\",\"toLocation\":\"S3:abc\"}";

when(tika.detect(any(InputStream.class), any(String.class))).thenReturn("image/png");

// Then
Exception thrown = assertThrows(
Exception exception = assertThrows(
BadRequestRuntimeException.class,
() -> csvValidationController.uploadFile(file, metaData)
);
assertTrue(thrown.getMessage().contains("Please upload a valid CSV file"));
String msg = "Please upload a valid CSV file. fileName: Test file, amlBodyName: amlBodyName";
assertEquals(msg, exception.getMessage());
}

@Test
Expand All @@ -113,13 +113,13 @@ void testUploadInvalidFileReturnsStatus500() throws IOException {

// When
when(tika.detect(any(InputStream.class), any(String.class))).thenReturn("text/csv");
when(fileTransferService.upload(any(),any())).thenThrow(new FileUploadException("Please upload a valid CSV file"));
when(fileTransferService.upload(any(),any())).thenThrow(new FileUploadException("server error"));

// Then
Exception thrown = assertThrows(
InternalServerErrorRuntimeException.class,
() -> csvValidationController.uploadFile(file, metaData)
);
assertTrue(thrown.getMessage().contains("Please upload a valid CSV file"));
assertTrue(thrown.getMessage().contains("server error"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import static org.mockito.Mockito.*;

import java.io.*;
import java.io.File;
import java.io.IOException;

import org.apache.commons.csv.CSVFormat;
import org.apache.commons.csv.CSVParser;
Expand All @@ -20,6 +22,8 @@
import org.mockito.junit.jupiter.MockitoExtension;
import uk.gov.companieshouse.filevalidationservice.exception.CSVDataValidationException;

import static org.junit.jupiter.api.Assertions.*;


@ExtendWith(MockitoExtension.class)
class CsvProcessorTest {
Expand All @@ -43,7 +47,9 @@ void csvFileWithTooFewHeadersMustFailToParse() throws IOException {
File file = new File("src/test/resources/tooFewHeaders.csv");
byte[] bytes = FileUtils.readFileToByteArray(file);

assertThrows(CSVDataValidationException.class, () -> csvProcessor.parseRecords(bytes));
Exception exception = assertThrows(CSVDataValidationException.class, () -> csvProcessor.parseRecords(bytes));
String msg = "Data validation exception: Headers did not match expected headers, following headers are missing: [registered company name, company number] at row number 0";
assertEquals(msg, exception.getMessage());
}

@Test
Expand All @@ -59,7 +65,9 @@ void csvRecordWithTooFewColumnsMustFailToParse() throws IOException {
File file = new File("src/test/resources/tooFewColumns.csv");
byte[] bytes = FileUtils.readFileToByteArray(file);

assertThrows(CSVDataValidationException.class, () -> csvProcessor.parseRecords(bytes));
Exception exception = assertThrows(CSVDataValidationException.class, () -> csvProcessor.parseRecords(bytes));
String msg = "Data validation exception: Incorrect number of columns. Received: 6 Expected: 13 at row number 1";
assertEquals(msg, exception.getMessage());
}

@Test
Expand Down
1 change: 1 addition & 0 deletions src/test/resources/tooFewColumns.csv
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
Unique ID,Registered Company Name,Company Number,Trading Name,First Name,Last Name,Date of Birth,Property Name or Number,Address Line 1,Address Line 2,City or Town,Postcode,Country
569857697,,,JS Accounting,John,Smith
569857697,,,JS Accounting,John,Smith,29091970,123,Stoke Road,,Stoke-on-Trent,ST6 3LJ
3 changes: 2 additions & 1 deletion src/test/resources/tooFewHeaders.csv
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
Unique ID,Registered Company Name,Company Number,Trading Name,First Name
Unique ID,Trading Name,First Name,Last Name,Date of Birth,Property Name or Number,Address Line 1,Address Line 2,City or Town,Postcode,Country
569857697,,,JS Accounting,John,Smith,29091970,123,Stoke Road,,Stoke-on-Trent,ST6 3LJ,United Kingdom
10 changes: 9 additions & 1 deletion suppress.xml
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<suppressions xmlns="https://jeremylong.github.io/DependencyCheck/dependency-suppression.1.3.xsd">

<suppress until="2025-03-03Z">
<notes><![CDATA[
file name: logback-core-1.5.12.jar
1.5.12 version of logback-core has vulnerability, All spring till 2025-01-03 have version 1.5.12 , So there is no fix till today , This will be fixed when version will be updated from Spring boot
]]></notes>
<packageUrl regex="true">^pkg:maven/ch\.qos\.logback/logback\-core@.*$</packageUrl>
<vulnerabilityName>CVE-2024-12798</vulnerabilityName>
<cve>CVE-2024-12798</cve>
</suppress>
</suppressions>

0 comments on commit d1b3260

Please sign in to comment.