Pull request #652: RED-6497

Merge in RED/persistence-service from RED-6497 to release/1.363.x

* commit '3fc9dc5132b5eaeb1d8bd609acb57bacc62237d5':
  RED-6497: Removed alternate file size check because it offers less error reporting capabilities
  RED-6497: Fixed issue where the file-size of download was saved incorrectly.
  RED-6497: Changed code to use a different method of getting a file size to have better error reporting
  RED-6497: Removed explicit variable initialization because its marked as a checkstyle violation.
  RED-6497: Corrected handling of the stream, temp-file and exceptions.
This commit is contained in:
Viktor Seifert 2023-03-31 09:47:47 +02:00
commit 9742afe175
8 changed files with 236 additions and 126 deletions

View File

@ -73,9 +73,11 @@ public class DownloadStatusPersistenceService {
@Transactional
public void updateStatus(String storageId, DownloadStatusValue status, long fileSize) {
public void updateStatus(DownloadStatusEntity entity, DownloadStatusValue statusValue, long fileSize) {
downloadStatusRepository.updateStatus(storageId, status, fileSize);
entity.setStatus(statusValue);
entity.setFileSize(fileSize);
downloadStatusRepository.save(entity);
}

View File

@ -18,12 +18,7 @@ public interface DownloadStatusRepository extends JpaRepository<DownloadStatusEn
@Modifying
@Query("update DownloadStatusEntity ds set ds.status = :status where ds.storageId = :storageId")
void updateStatus(String storageId, DownloadStatusValue status);
@Modifying
@Query("update DownloadStatusEntity ds set ds.status = :status, ds.fileSize = :fileSize where ds.storageId = :storageId")
void updateStatus(String storageId, DownloadStatusValue status, long fileSize);
@Modifying
@Query("update DownloadStatusEntity ds set ds.lastDownload = :lastDownload where ds.storageId = :storageId")

View File

@ -163,10 +163,7 @@ public class DownloadPreparationService {
private void updateStatusToReady(DownloadStatusEntity downloadStatus, FileSystemBackedArchiver fileSystemBackedArchiver) {
downloadStatusPersistenceService.updateStatus(downloadStatus.getStorageId(), DownloadStatusValue.READY, fileSystemBackedArchiver.getContentLength());
if (!Objects.equals(downloadStatus.getStatus(), DownloadStatusValue.READY)) {
downloadStatus.setStatus(DownloadStatusValue.READY);
}
downloadStatusPersistenceService.updateStatus(downloadStatus, DownloadStatusValue.READY, fileSystemBackedArchiver.getContentLength());
}

View File

@ -208,7 +208,7 @@ public class DossierTemplateExportService {
}
storeZipFile(downloadStatus.getStorageId(), fileSystemBackedArchiver);
downloadStatusPersistenceService.updateStatus(downloadStatus.getStorageId(), DownloadStatusValue.READY, fileSystemBackedArchiver.getContentLength());
downloadStatusPersistenceService.updateStatus(downloadStatus, DownloadStatusValue.READY, fileSystemBackedArchiver.getContentLength());
} catch (JsonProcessingException e) {
log.debug("fail ", e);

View File

@ -6,6 +6,8 @@ import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
@ -21,14 +23,31 @@ import lombok.extern.slf4j.Slf4j;
@Slf4j
public class FileSystemBackedArchiver implements AutoCloseable {
private final boolean rethrowExceptions;
private final Set<String> createdFolders = new HashSet<>();
private final File tempFile;
private final ZipOutputStream zipOutputStream;
private long tempFileLength;
@SneakyThrows
public FileSystemBackedArchiver() {
this(false);
}
/**
* Controls whether exceptions are re-thrown. Mostly meant for testing.
*
* @param rethrowExceptions If true exceptions caught when handling streams and files will be re-thrown.
*/
@SneakyThrows
FileSystemBackedArchiver(boolean rethrowExceptions) {
this.rethrowExceptions = rethrowExceptions;
tempFile = FileUtils.createTempFile("archive", ".zip");
zipOutputStream = new ZipOutputStream(new FileOutputStream(tempFile));
}
@ -50,11 +69,7 @@ public class FileSystemBackedArchiver implements AutoCloseable {
@SneakyThrows
public InputStream toInputStream() {
try {
zipOutputStream.close();
} catch (IOException e) {
log.debug(e.getMessage());
}
closeStreamAndStoreTempFileLength();
return new BufferedInputStream(new FileInputStream(tempFile));
}
@ -80,26 +95,50 @@ public class FileSystemBackedArchiver implements AutoCloseable {
@Override
public void close() {
closeStreamAndStoreTempFileLength();
try {
boolean res = tempFile.delete();
if (!res) {
log.warn("Failed to delete temp file");
Files.delete(tempFile.toPath());
} catch (IOException e) {
log.warn("Failed to delete temp-file", e);
if (rethrowExceptions) {
throw new RuntimeException(e);
}
zipOutputStream.close();
} catch (Exception e) {
log.debug("Failed to close FileSystemBackedArchiver");
}
}
public long getContentLength() {
closeStreamAndStoreTempFileLength();
return tempFileLength;
}
private void closeStreamAndStoreTempFileLength() {
try {
zipOutputStream.close();
} catch (IOException e) {
log.debug(e.getMessage());
log.warn("Failed to close temp-file stream", e);
if (rethrowExceptions) {
throw new RuntimeException(e);
}
}
if (tempFile.exists()) {
try {
var basicFileAttributes = Files.readAttributes(tempFile.toPath(), BasicFileAttributes.class);
tempFileLength = basicFileAttributes.size();
} catch (IOException e) {
if (rethrowExceptions) {
throw new RuntimeException(e);
}
}
} else {
log.warn("The temp file {} was deleted before it was completely processed", tempFile);
}
return tempFile.length();
}

View File

@ -25,20 +25,27 @@ import com.iqser.red.service.peristence.v1.server.integration.utils.AbstractPers
import com.iqser.red.service.peristence.v1.server.service.download.DownloadReportMessageReceiver;
import com.iqser.red.service.peristence.v1.server.service.download.RedactionResultMessageReceiver;
import com.iqser.red.service.persistence.management.v1.processor.utils.multitenancy.TenantContext;
import com.iqser.red.service.persistence.service.v1.api.model.dossiertemplate.DossierTemplate;
import com.iqser.red.service.persistence.service.v1.api.model.dossiertemplate.ReportTemplate;
import com.iqser.red.service.persistence.service.v1.api.model.dossiertemplate.ReportTemplateUploadRequest;
import com.iqser.red.service.persistence.service.v1.api.model.dossiertemplate.dossier.CreateOrUpdateDossierRequest;
import com.iqser.red.service.persistence.service.v1.api.model.dossiertemplate.dossier.Dossier;
import com.iqser.red.service.persistence.service.v1.api.model.dossiertemplate.dossier.file.FileModel;
import com.iqser.red.service.persistence.service.v1.api.model.dossiertemplate.dossier.file.WorkflowStatus;
import com.iqser.red.service.persistence.service.v1.api.model.download.DownloadStatus;
import com.iqser.red.service.persistence.service.v1.api.model.download.DownloadStatusValue;
import com.iqser.red.service.persistence.service.v1.api.model.download.DownloadWithOptionRequest;
import com.iqser.red.service.redaction.report.v1.api.model.ReportResultMessage;
import com.iqser.red.service.redaction.report.v1.api.model.StoredFileInformation;
import com.iqser.red.storage.commons.service.StorageService;
import lombok.AccessLevel;
import lombok.SneakyThrows;
import lombok.experimental.FieldDefaults;
public class DownloadPreparationTest extends AbstractPersistenceServerServiceTest {
public static final String USER_ID = "1";
@Autowired
private DownloadReportMessageReceiver downloadReportMessageReceiver;
@ -81,71 +88,48 @@ public class DownloadPreparationTest extends AbstractPersistenceServerServiceTes
@SneakyThrows
public void testReceiveDownloadPackage() {
String userId = "1";
var testData = new TestData();
var dossierTemplate = dossierTemplateTesterAndProvider.provideTestTemplate();
var dossier = dossierTesterAndProvider.provideTestDossier(dossierTemplate);
var file = fileTesterAndProvider.testAndProvideFile(dossier);
fileClient.setStatusApproved(dossier.getId(), file.getId(), file.getUploader());
var file11 = fileClient.getFileStatus(dossier.getId(), file.getId());
fileClient.setStatusApproved(testData.dossier.getId(), testData.file.getId(), testData.file.getUploader());
var file11 = fileClient.getFileStatus(testData.dossier.getId(), testData.file.getId());
assertThat(file11.getWorkflowStatus()).isEqualTo(WorkflowStatus.APPROVED);
reportTemplateClient.uploadTemplate(ReportTemplateUploadRequest.builder()
.activeByDefault(true)
.dossierTemplateId(dossierTemplate.getId())
.multiFileReport(true)
.fileName("test.docx")
.template(new byte[]{1, 2, 3, 4})
.build());
var availableTemplates = reportTemplateClient.getAvailableReportTemplates(dossierTemplate.getId());
assertThat(availableTemplates).isNotEmpty();
dossierClient.updateDossier(CreateOrUpdateDossierRequest.builder()
.dossierName(dossier.getDossierName())
.description(dossier.getDescription())
.ownerId(dossier.getOwnerId())
.memberIds(dossier.getMemberIds())
.approverIds(dossier.getApproverIds())
.downloadFileTypes(dossier.getDownloadFileTypes())
.watermarkId(dossier.getWatermarkId())
.dueDate(dossier.getDueDate())
.dossierTemplateId(dossier.getDossierTemplateId())
.reportTemplateIds(availableTemplates.stream().map(ReportTemplate::getTemplateId).collect(Collectors.toList()))
.build(), dossier.getId());
var updatedDossier = dossierClient.getDossierById(dossier.getId(), false, false);
var updatedDossier = dossierClient.getDossierById(testData.dossier.getId(), false, false);
assertThat(updatedDossier.getReportTemplateIds()).isNotEmpty();
downloadClient.prepareDownload(DownloadWithOptionRequest.builder()
.userId(userId)
.dossierId(dossier.getId())
.fileIds(Collections.singletonList(file.getId()))
.userId(USER_ID)
.dossierId(testData.dossier.getId())
.fileIds(Collections.singletonList(testData.file.getId()))
.redactionPreviewColor("#aaaaaa")
.build());
var statuses = downloadClient.getDownloadStatus(userId);
var statuses = downloadClient.getDownloadStatus(USER_ID);
assertThat(statuses).isNotEmpty();
assertThat(statuses.iterator().next().getLastDownload()).isNull();
DownloadStatus firstStatus = statuses.get(0);
assertThat(firstStatus.getLastDownload()).isNull();
String downloadId = statuses.iterator().next().getStorageId();
String downloadId = firstStatus.getStorageId();
addStoredFileInformationToStorage(file, availableTemplates, downloadId);
addStoredFileInformationToStorage(testData.file, testData.availableTemplates, downloadId);
ReportResultMessage reportResultMessage = new ReportResultMessage();
reportResultMessage.setUserId(userId);
reportResultMessage.setUserId(USER_ID);
reportResultMessage.setDownloadId(downloadId);
downloadReportMessageReceiver.receive(reportResultMessage);
redactionResultMessageReceiver.receive(RedactionResultMessage.builder()
.downloadId(downloadId)
.dossierId(dossier.getId())
.dossierId(testData.dossier.getId())
.redactionResultDetails(Collections.emptyList())
.build());
List<DownloadStatus> finalDownloadStatuses = downloadClient.getDownloadStatus(USER_ID);
assertThat(finalDownloadStatuses).hasSize(1);
DownloadStatus finalDownloadStatus = finalDownloadStatuses.get(0);
assertThat(finalDownloadStatus.getStatus()).isEqualTo(DownloadStatusValue.READY);
assertThat(finalDownloadStatus.getFileSize()).isGreaterThan(0);
}
@ -167,4 +151,46 @@ public class DownloadPreparationTest extends AbstractPersistenceServerServiceTes
storageService.storeObject(reportStorageId, new ByteArrayInputStream(new byte[]{1, 2, 3, 4}));
}
@FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE)
private class TestData {
DossierTemplate dossierTemplate = dossierTemplateTesterAndProvider.provideTestTemplate();
Dossier dossier = dossierTesterAndProvider.provideTestDossier(dossierTemplate);
FileModel file = fileTesterAndProvider.testAndProvideFile(dossier);
List<ReportTemplate> availableTemplates;
private TestData() {
reportTemplateClient.uploadTemplate(ReportTemplateUploadRequest.builder()
.activeByDefault(true)
.dossierTemplateId(dossierTemplate.getId())
.multiFileReport(true)
.fileName("test.docx")
.template(new byte[]{1, 2, 3, 4})
.build());
availableTemplates = reportTemplateClient.getAvailableReportTemplates(dossierTemplate.getId());
assertThat(availableTemplates).isNotEmpty();
dossierClient.updateDossier(CreateOrUpdateDossierRequest.builder()
.dossierName(dossier.getDossierName())
.description(dossier.getDescription())
.ownerId(dossier.getOwnerId())
.memberIds(dossier.getMemberIds())
.approverIds(dossier.getApproverIds())
.downloadFileTypes(dossier.getDownloadFileTypes())
.watermarkId(dossier.getWatermarkId())
.dueDate(dossier.getDueDate())
.dossierTemplateId(dossier.getDossierTemplateId())
.reportTemplateIds(availableTemplates.stream().map(ReportTemplate::getTemplateId).collect(Collectors.toList()))
.build(), dossier.getId());
}
}
}

View File

@ -1,57 +0,0 @@
package com.iqser.red.service.peristence.v1.server.utils;
import static org.assertj.core.api.Assertions.assertThat;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.ObjectOutputStream;
import java.util.SplittableRandom;
import org.apache.commons.io.IOUtils;
import org.junit.Test;
import lombok.SneakyThrows;
import lombok.extern.slf4j.Slf4j;
@Slf4j
public class FileSystemBackArchiverTest {
@Test
@SneakyThrows
public void testFileSystemBackedArchiver() {
try (var fsba = new FileSystemBackedArchiver()) {
SplittableRandom sr = new SplittableRandom();
var data = sr.doubles().limit(1024 * 1024).toArray();
for (int i = 0; i < 10; i++) {
log.info("At entry: {}, using {}MB of memory", i, (Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory()) / (1024 * 1024));
ByteArrayOutputStream bos = new ByteArrayOutputStream();
ObjectOutputStream oos = new ObjectOutputStream(bos);
oos.writeObject(data);
byte[] bytes = bos.toByteArray();
bos.close();
var entry = new FileSystemBackedArchiver.ArchiveModel("folder-" + i, "file-" + i, bytes);
fsba.addEntry(entry);
}
File f = File.createTempFile("test", ".zip");
var contentSize = fsba.getContentLength();
try (FileOutputStream fos = new FileOutputStream(f)) {
IOUtils.copy(fsba.toInputStream(), fos);
log.info("File: {}", f.getAbsolutePath());
assertThat(f.length()).isEqualTo(contentSize);
}
log.info("Total File Size: {}MB", f.length() / (1024 * 1024));
}
}
}

View File

@ -0,0 +1,108 @@
package com.iqser.red.service.peristence.v1.server.utils;
import static org.assertj.core.api.Assertions.assertThat;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.InputStream;
import java.io.ObjectOutputStream;
import java.nio.file.Files;
import java.nio.file.StandardCopyOption;
import java.util.SplittableRandom;
import org.junit.jupiter.api.Test;
import lombok.SneakyThrows;
import lombok.extern.slf4j.Slf4j;
@Slf4j
public class FileSystemBackedArchiverTest {
private final static byte[] dummyFileContent = new byte[]{1, 2};
@Test
@SneakyThrows
public void testFileSystemBackedArchiver() {
try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true)) {
SplittableRandom sr = new SplittableRandom();
var data = sr.doubles().limit(1024 * 1024).toArray();
for (int i = 0; i < 10; i++) {
log.info("At entry: {}, using {}MB of memory", i, (Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory()) / (1024 * 1024));
try (ByteArrayOutputStream bos = new ByteArrayOutputStream(); ObjectOutputStream oos = new ObjectOutputStream(bos)) {
oos.writeObject(data);
byte[] bytes = bos.toByteArray();
var entry = new FileSystemBackedArchiver.ArchiveModel("folder-" + i, "file-" + i, bytes);
fileSystemBackedArchiver.addEntry(entry);
}
}
File tempFile = File.createTempFile("test", ".zip");
var contentSize = fileSystemBackedArchiver.getContentLength();
try (InputStream inputStream = fileSystemBackedArchiver.toInputStream()) {
Files.copy(inputStream, tempFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
log.info("File: {}", tempFile.getAbsolutePath());
}
assertThat(tempFile.length()).isEqualTo(contentSize);
log.info("Total File Size: {}MB", tempFile.length() / (1024 * 1024));
Files.delete(tempFile.toPath());
}
}
@Test
public void testContentLengthForTwoEntries() {
try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true)) {
fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent));
fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent));
assertThat(fileSystemBackedArchiver.getContentLength()).isGreaterThan(0);
}
}
@Test
@SneakyThrows
public void testContentLengthForTwoEntriesAndStream() {
try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true)) {
fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent));
fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent));
try (InputStream stream = fileSystemBackedArchiver.toInputStream()) {
// Dummy statement to just have the code do something with the stream
//noinspection ResultOfMethodCallIgnored
stream.getClass();
}
assertThat(fileSystemBackedArchiver.getContentLength()).isGreaterThan(0);
}
}
@Test
public void testContentLengthForTwoEntriesWithClosing() {
// deliberately do not use try-with-resources to see if the content-length is available after temp file deletion
var fileSystemBackedArchiver = new FileSystemBackedArchiver(true);
fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent));
fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent));
fileSystemBackedArchiver.close();
assertThat(fileSystemBackedArchiver.getContentLength()).isGreaterThan(0);
}
}