RED-6497: Corrected handling of the stream, temp-file and exceptions.
* Corrected the order of closing the steam and deleting the file. * Switched the output of exception to the WARN level, since the hint at potential problems. * Corrected the tests so that they clean-up resources. * Changed the tests to use an archiver that re-throws exceptions, so that problems are not swallowed in testing.
This commit is contained in:
Viktor Seifert
parent
9930430bcd
commit
d74b8bf4ba
@ -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<String> 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));
|
||||
}
|
||||
@ -50,11 +68,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 +94,43 @@ 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()) {
|
||||
tempFileLength = tempFile.length();
|
||||
} else {
|
||||
log.warn("The temp file {} was deleted before it was completely processed", tempFile);
|
||||
}
|
||||
return tempFile.length();
|
||||
}
|
||||
|
||||
|
||||
|
||||
@ -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));
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
@ -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);
|
||||
}
|
||||
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user