From f4c04b9fd3327f5f7fd9af8f5c5a046d6baf1cb9 Mon Sep 17 00:00:00 2001 From: Philipp Zehnder Date: Mon, 8 Apr 2024 08:34:32 +0200 Subject: [PATCH] refactor(#2708): Add tests in FileManager for storeFile and deleteFile --- .../streampipes/model/file/FileMetadata.java | 21 +++++ .../streampipes/manager/file/FileManager.java | 19 ++--- .../manager/file/TestFileManager.java | 84 +++++++++++++++++++ 3 files changed, 111 insertions(+), 13 deletions(-) diff --git a/streampipes-model/src/main/java/org/apache/streampipes/model/file/FileMetadata.java b/streampipes-model/src/main/java/org/apache/streampipes/model/file/FileMetadata.java index f4ee123ecc..a6b1fe7922 100644 --- a/streampipes-model/src/main/java/org/apache/streampipes/model/file/FileMetadata.java +++ b/streampipes-model/src/main/java/org/apache/streampipes/model/file/FileMetadata.java @@ -21,6 +21,8 @@ import com.google.gson.annotations.SerializedName; +import java.util.Objects; + @TsModel public class FileMetadata { @@ -91,4 +93,23 @@ public String getFiletype() { public void setFiletype(String filetype) { this.filetype = filetype; } + + @Override + public boolean equals(Object o) { + if (this == o) {return true;} + if (o == null || getClass() != o.getClass()) {return false;} + FileMetadata that = (FileMetadata) o; + return createdAt == that.createdAt && lastModified == that.lastModified && Objects.equals( + fileId, + that.fileId + ) && Objects.equals(rev, that.rev) && Objects.equals( + filename, + that.filename + ) && Objects.equals(filetype, that.filetype) && Objects.equals(createdByUser, that.createdByUser); + } + + @Override + public int hashCode() { + return Objects.hash(fileId, rev, filename, filetype, createdAt, lastModified, createdByUser); + } } diff --git a/streampipes-pipeline-management/src/main/java/org/apache/streampipes/manager/file/FileManager.java b/streampipes-pipeline-management/src/main/java/org/apache/streampipes/manager/file/FileManager.java index e984402649..41b1dcb724 100644 --- a/streampipes-pipeline-management/src/main/java/org/apache/streampipes/manager/file/FileManager.java +++ b/streampipes-pipeline-management/src/main/java/org/apache/streampipes/manager/file/FileManager.java @@ -90,9 +90,11 @@ public FileMetadata storeFile(String user, public void deleteFile(String id) { - FileMetadata fileMetadata = getFileMetadataStorage().getMetadataById(id); - fileHandler.deleteFile(fileMetadata.getFilename()); - getFileMetadataStorage().deleteFileMetadata(id); + var fileMetadata = fileMetadataStorage.getMetadataById(id); + if (fileMetadata != null) { + fileHandler.deleteFile(fileMetadata.getFilename()); + fileMetadataStorage.deleteFileMetadata(id); + } } private InputStream validateFileNameAndCleanFile(String filename, @@ -139,15 +141,6 @@ protected void writeToFile(String sanitizedFilename, InputStream fileInputStream fileHandler.storeFile(sanitizedFilename, fileInputStream); } - - @Deprecated - private IFileMetadataStorage getFileMetadataStorage() { - return StorageDispatcher - .INSTANCE - .getNoSqlStore() - .getFileMetadataStorage(); - } - protected FileMetadata makeAndStoreFileMetadata(String user, String sanitizedFilename, String filetype) { @@ -171,7 +164,7 @@ private FileMetadata makeFileMetadata(String user, } private void storeFileMetadata(FileMetadata fileMetadata) { - getFileMetadataStorage().addFileMetadata(fileMetadata); + fileMetadataStorage.addFileMetadata(fileMetadata); } diff --git a/streampipes-pipeline-management/src/test/java/org/apache/streampipes/manager/file/TestFileManager.java b/streampipes-pipeline-management/src/test/java/org/apache/streampipes/manager/file/TestFileManager.java index c779b63096..f62fb9a888 100644 --- a/streampipes-pipeline-management/src/test/java/org/apache/streampipes/manager/file/TestFileManager.java +++ b/streampipes-pipeline-management/src/test/java/org/apache/streampipes/manager/file/TestFileManager.java @@ -23,7 +23,9 @@ import org.apache.commons.io.IOUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -36,7 +38,12 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class TestFileManager { @@ -45,6 +52,8 @@ public class TestFileManager { private IFileMetadataStorage fileMetadataStorage; private FileHandler fileHandler; + private static final String TEST_USER = "testUser"; + @BeforeEach public void setup() { fileMetadataStorage = mock(IFileMetadataStorage.class); @@ -129,6 +138,81 @@ public void storeFile_throwsExceptionForInvalidFileType() { fileManager.storeFile("", filename, mock(InputStream.class))); } + @Test + public void storeFile_storesFileWithValidInput() throws IOException { + var filename = "testFile.csv"; + + var fileMetadata = fileManager.storeFile(TEST_USER, filename, mock(InputStream.class)); + + assertEquals(TEST_USER, fileMetadata.getCreatedByUser()); + assertEquals(filename, fileMetadata.getFilename()); + assertEquals("csv", fileMetadata.getFiletype()); + verify(fileHandler, times(1)).storeFile(eq(filename), any(InputStream.class)); + verify(fileMetadataStorage, times(1)).addFileMetadata(any(FileMetadata.class)); + } + + @Test + public void storeFile_sanitizesFilename() throws IOException { + var filename = "test@File.csv"; + var expectedSanitizedFilename = "test_File.csv"; + + var fileMetadata = fileManager.storeFile(TEST_USER, filename, mock(InputStream.class)); + + assertEquals(expectedSanitizedFilename, fileMetadata.getFilename()); + verify(fileHandler, times(1)).storeFile(eq(expectedSanitizedFilename), any(InputStream.class)); + verify(fileMetadataStorage, times(1)).addFileMetadata(any(FileMetadata.class)); + } + + /** + * This test validates that the storeFile method removes the BOM from the file before it is stored. + */ + @Test + public void storeFile_removesBom() throws IOException { + var expectedContent = "test content"; + // prepare input stream with BOM + var fileInputStream = new ByteArrayInputStream(("\uFEFF" + expectedContent) + .getBytes(StandardCharsets.UTF_8)); + + fileManager.storeFile(TEST_USER, "testfile.csv", fileInputStream); + + var inputStreamCaptor = ArgumentCaptor.forClass(InputStream.class); + verify(fileHandler, times(1)).storeFile(any(), inputStreamCaptor.capture()); + + // Convert the captured InputStream to a String + var capturedInputStream = inputStreamCaptor.getValue(); + var capturedContent = IOUtils.toString(capturedInputStream, StandardCharsets.UTF_8); + + // Assert that the captured content is equal to the expected content + assertEquals(expectedContent, capturedContent); + } + + + @Test + public void deleteFile_removesExistingFile() { + var id = "existingFileId"; + var fileMetadata = new FileMetadata(); + fileMetadata.setFilename("existingFile.txt"); + + when(fileMetadataStorage.getMetadataById(id)).thenReturn(fileMetadata); + + fileManager.deleteFile(id); + + verify(fileHandler, times(1)).deleteFile(fileMetadata.getFilename()); + verify(fileMetadataStorage, times(1)).deleteFileMetadata(id); + } + + @Test + public void deleteFile_doesNothingForNonExistingFile() { + var id = "nonExistingFileId"; + + when(fileMetadataStorage.getMetadataById(id)).thenReturn(null); + + fileManager.deleteFile(id); + + verify(fileHandler, times(0)).deleteFile(anyString()); + verify(fileMetadataStorage, times(0)).deleteFileMetadata(id); + } + @Test public void testCleanFileWithoutBom() throws IOException { var expected = "test";