From 8ef9a285b394df7fe333f8713f92de6ac5fd410d Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Wed, 29 Mar 2023 18:36:48 +0200 Subject: [PATCH 1/6] RED-6497: Removed alternate file size check because it offers less error reporting capabilities --- .../utils/FileSystemBackedArchiver.java | 71 +++++++---- .../utils/FileSystemBackedArchiverTest.java | 110 ++++++++++++++++++ 2 files changed, 161 insertions(+), 20 deletions(-) create mode 100644 persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java index 4b0d16d6c..7035319f3 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java @@ -6,6 +6,7 @@ import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Files; import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -21,14 +22,31 @@ import lombok.extern.slf4j.Slf4j; @Slf4j public class FileSystemBackedArchiver implements AutoCloseable { + private final boolean rethrowExceptions; private final Set createdFolders = new HashSet<>(); private final File tempFile; private final ZipOutputStream zipOutputStream; + private long tempFileLength = 0; + @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)); } @@ -47,6 +65,14 @@ public class FileSystemBackedArchiver implements AutoCloseable { } + @SneakyThrows + public InputStream toInputStream() { + + closeStreamAndStoreTempFileLength(); + return new BufferedInputStream(new FileInputStream(tempFile)); + } + + @SneakyThrows public void addEntry(ArchiveModel archiveModel) { // begin writing a new ZIP entry, positions the stream to the start of the entry entity @@ -65,41 +91,46 @@ public class FileSystemBackedArchiver implements AutoCloseable { } - @SneakyThrows - public InputStream toInputStream() { - - try { - zipOutputStream.close(); - } catch (IOException e) { - log.debug(e.getMessage()); - } - return new BufferedInputStream(new FileInputStream(tempFile)); - } - - @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()) { + tempFileLength = tempFile.length(); + } else { + log.warn("The temp file {} was deleted before it was completely processed", tempFile); } - return tempFile.length(); } diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java new file mode 100644 index 000000000..a3e1aac93 --- /dev/null +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java @@ -0,0 +1,110 @@ +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 com.iqser.red.service.persistence.management.v1.processor.utils.FileSystemBackedArchiver; + +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); + } + +} \ No newline at end of file From 412b770e80cdc80f82bc316dc4f81683ab21289d Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Thu, 30 Mar 2023 14:20:55 +0200 Subject: [PATCH 2/6] RED-6497: Removed explicit variable initialization because its marked as a checkstyle violation. --- .../management/v1/processor/utils/FileSystemBackedArchiver.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java index 7035319f3..c4ff9b0c9 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java @@ -27,7 +27,7 @@ public class FileSystemBackedArchiver implements AutoCloseable { private final File tempFile; private final ZipOutputStream zipOutputStream; - private long tempFileLength = 0; + private long tempFileLength; @SneakyThrows From 6e9bcd7bb185521067b32b14a1e3669a76d72714 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Thu, 30 Mar 2023 15:43:07 +0200 Subject: [PATCH 3/6] RED-6497: Changed code to use a different method of getting a file size to have better error reporting --- .../utils/FileSystemBackedArchiver.java | 20 ++++++++-- .../utils/FileSystemBackedArchiverTest.java | 37 +++++++++++++++++-- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java index c4ff9b0c9..1d1c4209e 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java @@ -7,6 +7,7 @@ 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; @@ -23,6 +24,7 @@ import lombok.extern.slf4j.Slf4j; public class FileSystemBackedArchiver implements AutoCloseable { private final boolean rethrowExceptions; + private final boolean useFileAttributes; private final Set createdFolders = new HashSet<>(); private final File tempFile; private final ZipOutputStream zipOutputStream; @@ -33,7 +35,7 @@ public class FileSystemBackedArchiver implements AutoCloseable { @SneakyThrows public FileSystemBackedArchiver() { - this(false); + this(false, true); } @@ -44,9 +46,10 @@ public class FileSystemBackedArchiver implements AutoCloseable { * @param rethrowExceptions If true exceptions caught when handling streams and files will be re-thrown. */ @SneakyThrows - FileSystemBackedArchiver(boolean rethrowExceptions) { + FileSystemBackedArchiver(boolean rethrowExceptions, boolean useFileAttributes) { this.rethrowExceptions = rethrowExceptions; + this.useFileAttributes = useFileAttributes; tempFile = FileUtils.createTempFile("archive", ".zip"); zipOutputStream = new ZipOutputStream(new FileOutputStream(tempFile)); } @@ -127,7 +130,18 @@ public class FileSystemBackedArchiver implements AutoCloseable { } if (tempFile.exists()) { - tempFileLength = tempFile.length(); + try { + if (useFileAttributes) { + var basicFileAttributes = Files.readAttributes(tempFile.toPath(), BasicFileAttributes.class); + tempFileLength = basicFileAttributes.size(); + } else { + tempFileLength = tempFile.length(); + } + } catch (IOException e) { + if (rethrowExceptions) { + throw new RuntimeException(e); + } + } } else { log.warn("The temp file {} was deleted before it was completely processed", tempFile); } diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java index a3e1aac93..56d6b846c 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java @@ -27,7 +27,7 @@ public class FileSystemBackedArchiverTest { @SneakyThrows public void testFileSystemBackedArchiver() { - try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true)) { + try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true, false)) { SplittableRandom sr = new SplittableRandom(); @@ -64,7 +64,7 @@ public class FileSystemBackedArchiverTest { @Test public void testContentLengthForTwoEntries() { - try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true)) { + try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true, false)) { fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent)); fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent)); @@ -74,11 +74,40 @@ public class FileSystemBackedArchiverTest { } + @Test + public void testContentLengthForTwoEntriesComparingFileSize() { + + long defaultFileLength; + try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true, false)) { + + fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent)); + fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent)); + + long contentLength = fileSystemBackedArchiver.getContentLength(); + assertThat(contentLength).isGreaterThan(0); + defaultFileLength = contentLength; + } + + long fileAttributesContentLength; + try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true, true)) { + + fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent)); + fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent)); + + long contentLength = fileSystemBackedArchiver.getContentLength(); + assertThat(contentLength).isGreaterThan(0); + fileAttributesContentLength = contentLength; + } + + assertThat(defaultFileLength).isEqualTo(fileAttributesContentLength); + } + + @Test @SneakyThrows public void testContentLengthForTwoEntriesAndStream() { - try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true)) { + try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true, false)) { fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent)); fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent)); @@ -97,7 +126,7 @@ public class FileSystemBackedArchiverTest { @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); + var fileSystemBackedArchiver = new FileSystemBackedArchiver(true, false); fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent)); fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent)); From 338b06079b1d6ae928466293d0ac00632c16f3c1 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Thu, 30 Mar 2023 17:30:16 +0200 Subject: [PATCH 4/6] RED-6497: Fixed issue where the file-size of download was saved incorrectly. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Changed the update to the DownloadStatusEntity to use the entity object instead of using a query on JPA-repo.  This prevents values from the object being incorrectly inserted otherwise. * Extended the DownloadPreparationTest to check the resulting download state and file size. --- .../download/DownloadPreparationService.java | 5 +---- .../export/DossierTemplateExportService.java | 2 +- .../DownloadStatusPersistenceService.java | 6 ++++-- .../repository/DownloadStatusRepository.java | 7 +------ .../integration/tests/DownloadPreparationTest.java | 13 ++++++++++--- 5 files changed, 17 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/download/DownloadPreparationService.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/download/DownloadPreparationService.java index d5b62aa22..aa9444050 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/download/DownloadPreparationService.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/download/DownloadPreparationService.java @@ -166,10 +166,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()); } diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/export/DossierTemplateExportService.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/export/DossierTemplateExportService.java index ab897652c..c5f57e614 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/export/DossierTemplateExportService.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/export/DossierTemplateExportService.java @@ -221,7 +221,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); diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/DownloadStatusPersistenceService.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/DownloadStatusPersistenceService.java index 6b461488b..538960fa3 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/DownloadStatusPersistenceService.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/DownloadStatusPersistenceService.java @@ -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); } diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/repository/DownloadStatusRepository.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/repository/DownloadStatusRepository.java index 385575f15..30a9352f1 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/repository/DownloadStatusRepository.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/service/persistence/repository/DownloadStatusRepository.java @@ -18,12 +18,7 @@ public interface DownloadStatusRepository extends JpaRepository downloadStatuses = downloadClient.getDownloadStatus().getDownloadStatus(); + assertThat(downloadStatuses).hasSize(1); - DownloadStatus firstDownloadStatus = statuses.getDownloadStatus().iterator().next(); + DownloadStatus firstDownloadStatus = downloadStatuses.get(0); assertThat(firstDownloadStatus.getLastDownload()).isNull(); String downloadId = firstDownloadStatus.getStorageId(); @@ -134,6 +135,12 @@ public class DownloadPreparationTest extends AbstractPersistenceServerServiceTes .redactionResultDetails(Collections.emptyList()) .build()); + List finalDownloadStatuses = downloadClient.getDownloadStatus().getDownloadStatus(); + assertThat(finalDownloadStatuses).hasSize(1); + DownloadStatus finalDownloadStatus = finalDownloadStatuses.get(0); + assertThat(finalDownloadStatus.getStatus()).isEqualTo(DownloadStatusValue.READY); + assertThat(finalDownloadStatus.getFileSize()).isGreaterThan(0); + clearTenantContext(); } From 4c25ac402dd01004c5a8eb71ce757219abeeada1 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Thu, 30 Mar 2023 17:37:18 +0200 Subject: [PATCH 5/6] RED-6497: Removed alternate file size check because it offers less error reporting capabilities --- .../utils/FileSystemBackedArchiver.java | 14 ++----- .../utils/FileSystemBackedArchiverTest.java | 37 ++----------------- 2 files changed, 8 insertions(+), 43 deletions(-) diff --git a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java index 1d1c4209e..7a2e90973 100644 --- a/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java +++ b/persistence-service-v1/persistence-service-processor-v1/src/main/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiver.java @@ -24,7 +24,6 @@ import lombok.extern.slf4j.Slf4j; public class FileSystemBackedArchiver implements AutoCloseable { private final boolean rethrowExceptions; - private final boolean useFileAttributes; private final Set createdFolders = new HashSet<>(); private final File tempFile; private final ZipOutputStream zipOutputStream; @@ -35,7 +34,7 @@ public class FileSystemBackedArchiver implements AutoCloseable { @SneakyThrows public FileSystemBackedArchiver() { - this(false, true); + this(false); } @@ -46,10 +45,9 @@ public class FileSystemBackedArchiver implements AutoCloseable { * @param rethrowExceptions If true exceptions caught when handling streams and files will be re-thrown. */ @SneakyThrows - FileSystemBackedArchiver(boolean rethrowExceptions, boolean useFileAttributes) { + FileSystemBackedArchiver(boolean rethrowExceptions) { this.rethrowExceptions = rethrowExceptions; - this.useFileAttributes = useFileAttributes; tempFile = FileUtils.createTempFile("archive", ".zip"); zipOutputStream = new ZipOutputStream(new FileOutputStream(tempFile)); } @@ -131,12 +129,8 @@ public class FileSystemBackedArchiver implements AutoCloseable { if (tempFile.exists()) { try { - if (useFileAttributes) { - var basicFileAttributes = Files.readAttributes(tempFile.toPath(), BasicFileAttributes.class); - tempFileLength = basicFileAttributes.size(); - } else { - tempFileLength = tempFile.length(); - } + var basicFileAttributes = Files.readAttributes(tempFile.toPath(), BasicFileAttributes.class); + tempFileLength = basicFileAttributes.size(); } catch (IOException e) { if (rethrowExceptions) { throw new RuntimeException(e); diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java index 56d6b846c..a3e1aac93 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java @@ -27,7 +27,7 @@ public class FileSystemBackedArchiverTest { @SneakyThrows public void testFileSystemBackedArchiver() { - try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true, false)) { + try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true)) { SplittableRandom sr = new SplittableRandom(); @@ -64,7 +64,7 @@ public class FileSystemBackedArchiverTest { @Test public void testContentLengthForTwoEntries() { - try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true, false)) { + try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true)) { fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent)); fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent)); @@ -74,40 +74,11 @@ public class FileSystemBackedArchiverTest { } - @Test - public void testContentLengthForTwoEntriesComparingFileSize() { - - long defaultFileLength; - try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true, false)) { - - fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent)); - fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent)); - - long contentLength = fileSystemBackedArchiver.getContentLength(); - assertThat(contentLength).isGreaterThan(0); - defaultFileLength = contentLength; - } - - long fileAttributesContentLength; - try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true, true)) { - - fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent)); - fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent)); - - long contentLength = fileSystemBackedArchiver.getContentLength(); - assertThat(contentLength).isGreaterThan(0); - fileAttributesContentLength = contentLength; - } - - assertThat(defaultFileLength).isEqualTo(fileAttributesContentLength); - } - - @Test @SneakyThrows public void testContentLengthForTwoEntriesAndStream() { - try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true, false)) { + try (var fileSystemBackedArchiver = new FileSystemBackedArchiver(true)) { fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent)); fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent)); @@ -126,7 +97,7 @@ public class FileSystemBackedArchiverTest { @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, false); + var fileSystemBackedArchiver = new FileSystemBackedArchiver(true); fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Original", "original", dummyFileContent)); fileSystemBackedArchiver.addEntry(new FileSystemBackedArchiver.ArchiveModel("Preview", "preview", dummyFileContent)); From 78758a2a418a816f4efe0f7d64647926377ad4f7 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Fri, 31 Mar 2023 12:03:40 +0200 Subject: [PATCH 6/6] RED-6497: Moved test to the same package as the class being tested --- .../v1/processor}/utils/FileSystemBackedArchiverTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) rename persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/{peristence/v1/server => persistence/management/v1/processor}/utils/FileSystemBackedArchiverTest.java (96%) diff --git a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiverTest.java similarity index 96% rename from persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java rename to persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiverTest.java index a3e1aac93..7808accb7 100644 --- a/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/peristence/v1/server/utils/FileSystemBackedArchiverTest.java +++ b/persistence-service-v1/persistence-service-server-v1/src/test/java/com/iqser/red/service/persistence/management/v1/processor/utils/FileSystemBackedArchiverTest.java @@ -1,4 +1,4 @@ -package com.iqser.red.service.peristence.v1.server.utils; +package com.iqser.red.service.persistence.management.v1.processor.utils; import static org.assertj.core.api.Assertions.assertThat; @@ -12,8 +12,6 @@ import java.util.SplittableRandom; import org.junit.jupiter.api.Test; -import com.iqser.red.service.persistence.management.v1.processor.utils.FileSystemBackedArchiver; - import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j;