From 0ed7c8863131f39e471db95671372ef16c75bedd Mon Sep 17 00:00:00 2001 From: Philipp Zehnder Date: Sun, 7 Apr 2024 12:31:57 +0200 Subject: [PATCH 1/7] refactor(#2708): Make all methods of FileManager none static --- .../generator/ExportPackageGenerator.java | 2 +- .../streampipes/manager/file/FileManager.java | 32 ++++++++--------- .../manager/file/TestFileManager.java | 34 ++++++++++++------- .../streampipes/rest/ResetManagement.java | 5 +-- .../rest/impl/AssetDashboardResource.java | 6 ++-- .../rest/impl/PipelineElementFile.java | 18 ++++++---- 6 files changed, 56 insertions(+), 41 deletions(-) diff --git a/streampipes-data-export/src/main/java/org/apache/streampipes/export/generator/ExportPackageGenerator.java b/streampipes-data-export/src/main/java/org/apache/streampipes/export/generator/ExportPackageGenerator.java index fc3663929f..13abd92077 100644 --- a/streampipes-data-export/src/main/java/org/apache/streampipes/export/generator/ExportPackageGenerator.java +++ b/streampipes-data-export/src/main/java/org/apache/streampipes/export/generator/ExportPackageGenerator.java @@ -124,7 +124,7 @@ public byte[] generateExportPackage() throws IOException { String filename = fileResolver.findDocument(item.getResourceId()).getFilename(); addDoc(builder, item, new FileResolver(), manifest::addFile); try { - builder.addBinary(filename, Files.readAllBytes(FileManager.getFile(filename).toPath())); + builder.addBinary(filename, Files.readAllBytes(new FileManager().getFile(filename).toPath())); } catch (IOException e) { e.printStackTrace(); } 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 5038ec3af0..bfd7518b86 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 @@ -34,16 +34,16 @@ public class FileManager { - public static List getAllFiles() { + public List getAllFiles() { return getAllFiles(null); } - public static List getAllFiles(String filetypes) { + public List getAllFiles(String filetypes) { List allFiles = getFileMetadataStorage().getAllFileMetadataDescriptions(); return filetypes != null ? filterFiletypes(allFiles, filetypes) : allFiles; } - public static File getFile(String filename) { + public File getFile(String filename) { return new FileHandler().getFile(filename); } @@ -56,7 +56,7 @@ public static File getFile(String filename) { * @param fileInputStream content of file * @return metadata of file */ - public static FileMetadata storeFile(String user, + public FileMetadata storeFile(String user, String filename, InputStream fileInputStream) throws IOException { @@ -72,13 +72,13 @@ public static FileMetadata storeFile(String user, } - public static void deleteFile(String id) { + public void deleteFile(String id) { FileMetadata fileMetadata = getFileMetadataStorage().getMetadataById(id); new FileHandler().deleteFile(fileMetadata.getFilename()); getFileMetadataStorage().deleteFileMetadata(id); } - private static InputStream validateFileNameAndCleanFile(String filename, + private InputStream validateFileNameAndCleanFile(String filename, String filetype, InputStream fileInputStream) { if (!validateFileType(filename)) { @@ -95,7 +95,7 @@ private static InputStream validateFileNameAndCleanFile(String filename, * @param filetype file of type * @return input stream without BOM */ - protected static InputStream cleanFile(InputStream fileInputStream, String filetype) { + protected InputStream cleanFile(InputStream fileInputStream, String filetype) { if (Filetypes.CSV.getFileExtensions().contains(filetype.toLowerCase())) { fileInputStream = new BOMInputStream(fileInputStream); } @@ -103,34 +103,34 @@ protected static InputStream cleanFile(InputStream fileInputStream, String filet return fileInputStream; } - public static boolean checkFileContentChanged(String filename, String hash) throws IOException { + public boolean checkFileContentChanged(String filename, String hash) throws IOException { var fileHash = FileHasher.hash(getFile(filename)); return !fileHash.equals(hash); } - public static String sanitizeFilename(String filename) { + public String sanitizeFilename(String filename) { return filename.replaceAll("[^a-zA-Z0-9.\\-]", "_"); } - public static boolean validateFileType(String filename) { + public boolean validateFileType(String filename) { return Filetypes.getAllFileExtensions() .stream() .anyMatch(filename::endsWith); } - protected static void writeToFile(String sanitizedFilename, InputStream fileInputStream) throws IOException { + protected void writeToFile(String sanitizedFilename, InputStream fileInputStream) throws IOException { new FileHandler().storeFile(sanitizedFilename, fileInputStream); } - private static IFileMetadataStorage getFileMetadataStorage() { + private IFileMetadataStorage getFileMetadataStorage() { return StorageDispatcher .INSTANCE .getNoSqlStore() .getFileMetadataStorage(); } - protected static FileMetadata makeAndStoreFileMetadata(String user, + protected FileMetadata makeAndStoreFileMetadata(String user, String sanitizedFilename, String filetype) { var fileMetadata = makeFileMetadata(user, sanitizedFilename, filetype); @@ -139,7 +139,7 @@ protected static FileMetadata makeAndStoreFileMetadata(String user, return fileMetadata; } - private static FileMetadata makeFileMetadata(String user, + private FileMetadata makeFileMetadata(String user, String filename, String filetype) { @@ -152,12 +152,12 @@ private static FileMetadata makeFileMetadata(String user, return fileMetadata; } - private static void storeFileMetadata(FileMetadata fileMetadata) { + private void storeFileMetadata(FileMetadata fileMetadata) { getFileMetadataStorage().addFileMetadata(fileMetadata); } - private static List filterFiletypes(List allFiles, String filetypes) { + private List filterFiletypes(List allFiles, String filetypes) { return allFiles .stream() .filter(fileMetadata -> Arrays 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 a02a309fa3..21f8d9dac3 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 @@ -18,6 +18,7 @@ package org.apache.streampipes.manager.file; import org.apache.commons.io.IOUtils; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.io.IOException; @@ -32,19 +33,26 @@ public class TestFileManager { + private FileManager fileManager; + + @BeforeEach + public void setup() { + fileManager = new FileManager(); + } + @Test public void storeFile_throwsExceptionForInvalidFileType() { var filename = "testFile.invalid"; assertThrows(IllegalArgumentException.class, () -> - FileManager.storeFile("", filename, mock(InputStream.class))); + fileManager.storeFile("", filename, mock(InputStream.class))); } @Test public void testCleanFileWithoutBom() throws IOException { var expected = "test"; var inputStream = IOUtils.toInputStream(expected, StandardCharsets.UTF_8); - var resultStream = FileManager.cleanFile(inputStream, "CSV"); + var resultStream = fileManager.cleanFile(inputStream, "CSV"); var resultString = IOUtils.toString(resultStream, StandardCharsets.UTF_8); assertEquals(expected, resultString); @@ -56,7 +64,7 @@ public void testCleanFileWithBom() throws IOException { var utf8Bom = "\uFEFF"; var inputString = utf8Bom + expected; var inputStream = IOUtils.toInputStream(inputString, StandardCharsets.UTF_8); - var resultStream = FileManager.cleanFile(inputStream, "CSV"); + var resultStream = fileManager.cleanFile(inputStream, "CSV"); var resultString = IOUtils.toString(resultStream, StandardCharsets.UTF_8); assertEquals(expected, resultString); @@ -68,7 +76,7 @@ public void testCleanFileWithBomAndUmlauts() throws IOException { var utf8Bom = "\uFEFF"; var inputString = utf8Bom + expected; var inputStream = IOUtils.toInputStream(inputString, StandardCharsets.UTF_8); - var resultStream = FileManager.cleanFile(inputStream, "CSV"); + var resultStream = fileManager.cleanFile(inputStream, "CSV"); var resultString = IOUtils.toString(resultStream, StandardCharsets.UTF_8); assertEquals(expected, resultString); @@ -77,57 +85,57 @@ public void testCleanFileWithBomAndUmlauts() throws IOException { @Test public void sanitizeFilename_replacesNonAlphanumericCharactersWithUnderscore() { var filename = "file@name#with$special%characters"; - var sanitizedFilename = FileManager.sanitizeFilename(filename); + var sanitizedFilename = fileManager.sanitizeFilename(filename); assertEquals("file_name_with_special_characters", sanitizedFilename); } @Test public void sanitizeFilename_keepsAlphanumericAndDotAndHyphenCharacters() { var filename = "file.name-with_alphanumeric123"; - var sanitizedFilename = FileManager.sanitizeFilename(filename); + var sanitizedFilename = fileManager.sanitizeFilename(filename); assertEquals(filename, sanitizedFilename); } @Test public void sanitizeFilename_returnsUnderscoreForFilenameWithAllSpecialCharacters() { var filename = "@#$%^&*()"; - var sanitizedFilename = FileManager.sanitizeFilename(filename); + var sanitizedFilename = fileManager.sanitizeFilename(filename); assertEquals("_________", sanitizedFilename); } @Test public void sanitizeFilename_returnsEmptyStringForEmptyFilename() { var filename = ""; - var sanitizedFilename = FileManager.sanitizeFilename(filename); + var sanitizedFilename = fileManager.sanitizeFilename(filename); assertEquals("", sanitizedFilename); } @Test public void sanitizeFilename_removesSingleParentDirectory() { var filename = "../file.csv"; - var sanitizedFilename = FileManager.sanitizeFilename(filename); + var sanitizedFilename = fileManager.sanitizeFilename(filename); assertEquals(".._file.csv", sanitizedFilename); } @Test public void sanitizeFilename_removesDoubleParentDirectoryy() { var filename = "../../file"; - var sanitizedFilename = FileManager.sanitizeFilename(filename); + var sanitizedFilename = fileManager.sanitizeFilename(filename); assertEquals(".._.._file", sanitizedFilename); } @Test public void validateFileName_returnsTrueForCsv() { - assertTrue(FileManager.validateFileType("file.csv")); + assertTrue(fileManager.validateFileType("file.csv")); } @Test public void validateFileName_returnsTrueForJson() { - assertTrue(FileManager.validateFileType("file.json")); + assertTrue(fileManager.validateFileType("file.json")); } @Test public void validateFileName_returnsFalseForSh() { - assertFalse(FileManager.validateFileType("file.sh")); + assertFalse(fileManager.validateFileType("file.sh")); } } \ No newline at end of file diff --git a/streampipes-rest/src/main/java/org/apache/streampipes/rest/ResetManagement.java b/streampipes-rest/src/main/java/org/apache/streampipes/rest/ResetManagement.java index 8319d0a4ee..d5e89b9671 100644 --- a/streampipes-rest/src/main/java/org/apache/streampipes/rest/ResetManagement.java +++ b/streampipes-rest/src/main/java/org/apache/streampipes/rest/ResetManagement.java @@ -131,8 +131,9 @@ private static void stopAndDeleteAllAdapters() { } private static void deleteAllFiles() { - List allFiles = FileManager.getAllFiles(); - allFiles.forEach(fileMetadata -> FileManager.deleteFile(fileMetadata.getFileId())); + var fileManager = new FileManager(); + List allFiles = fileManager.getAllFiles(); + allFiles.forEach(fileMetadata -> fileManager.deleteFile(fileMetadata.getFileId())); } private static void removeAllDataInDataLake() { diff --git a/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/AssetDashboardResource.java b/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/AssetDashboardResource.java index 4433d51ae4..4687f7c962 100644 --- a/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/AssetDashboardResource.java +++ b/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/AssetDashboardResource.java @@ -106,7 +106,7 @@ public ResponseEntity deleteAssetDashboard(@PathVariable("dashboardId") St @GetMapping(path = "/images/{imageName}") public ResponseEntity getDashboardImage(@PathVariable("imageName") String imageName) { try { - var sanitizedFileName = FileManager.sanitizeFilename(imageName); + var sanitizedFileName = new FileManager().sanitizeFilename(imageName); java.nio.file.Path path = Paths.get(getTargetFile(sanitizedFileName)); File file = new File(path.toString()); FileNameMap fileNameMap = URLConnection.getFileNameMap(); @@ -128,7 +128,7 @@ public ResponseEntity storeDashboardImage(@RequestPart("file_upload") Mult try { var fileName = fileDetail.getName(); - if (!FileManager.validateFileType(fileName)) { + if (!new FileManager().validateFileType(fileName)) { LOG.error("File type of file " + fileName + " is not supported"); fail(); } @@ -148,7 +148,7 @@ private void storeFile(String fileName, InputStream fileInputStream) { targetDirectory.mkdirs(); } - var sanitizedFileName = FileManager.sanitizeFilename(fileName); + var sanitizedFileName = new FileManager().sanitizeFilename(fileName); var targetFile = new File(getTargetFile(sanitizedFileName)); diff --git a/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/PipelineElementFile.java b/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/PipelineElementFile.java index 89373cce05..006f3f955d 100644 --- a/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/PipelineElementFile.java +++ b/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/PipelineElementFile.java @@ -53,6 +53,12 @@ @RequestMapping("/api/v2/files") public class PipelineElementFile extends AbstractAuthGuardedRestResource { + private final FileManager fileManager; + + public PipelineElementFile() { + this.fileManager = new FileManager(); + } + @PostMapping( consumes = MediaType.MULTIPART_FORM_DATA_VALUE, produces = MediaType.APPLICATION_JSON_VALUE) @@ -60,7 +66,7 @@ public class PipelineElementFile extends AbstractAuthGuardedRestResource { public ResponseEntity storeFile(@RequestPart("file_upload") MultipartFile fileDetail) { try { FileMetadata metadata = - FileManager.storeFile( + fileManager.storeFile( getAuthenticatedUsername(), fileDetail.getOriginalFilename(), fileDetail.getInputStream() @@ -74,7 +80,7 @@ public ResponseEntity storeFile(@RequestPart("file_upload") MultipartFile fil @DeleteMapping(path = "{fileId}") @PreAuthorize(AuthConstants.IS_ADMIN_ROLE) public ResponseEntity deleteFile(@PathVariable("fileId") String fileId) { - FileManager.deleteFile(fileId); + fileManager.deleteFile(fileId); return ok(); } @@ -83,7 +89,7 @@ public ResponseEntity deleteFile(@PathVariable("fileId") String fileId) { public ResponseEntity> getFileInfo( @RequestParam(value = "filetypes", required = false) String filetypes ) { - return ok(FileManager.getAllFiles(filetypes)); + return ok(fileManager.getAllFiles(filetypes)); } @GetMapping(path = "/{filename}", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE) @@ -109,7 +115,7 @@ public ResponseEntity getFile( @PathVariable("filename") String filename ) { try { - return ok(getFileContents(FileManager.getFile(filename))); + return ok(getFileContents(fileManager.getFile(filename))); } catch (IOException e) { throw new SpMessageException( NOT_FOUND, @@ -125,7 +131,7 @@ private byte[] getFileContents(File file) throws IOException { @GetMapping(path = "/allFilenames", produces = MediaType.APPLICATION_JSON_VALUE) @PreAuthorize(AuthConstants.HAS_READ_FILE_PRIVILEGE) public ResponseEntity> getAllOriginalFilenames() { - return ok(FileManager.getAllFiles() + return ok(fileManager.getAllFiles() .stream() .map(fileMetadata -> fileMetadata.getFilename() .toLowerCase()) @@ -141,7 +147,7 @@ public ResponseEntity checkFileContentChanged( @PathVariable(value = "hash") String hash ) { try { - return ok(FileManager.checkFileContentChanged(filename, hash)); + return ok(fileManager.checkFileContentChanged(filename, hash)); } catch (IOException e) { throw new SpMessageException( NOT_FOUND, From 024ff2adbb8069691f68514a8e6f90182e0794d9 Mon Sep 17 00:00:00 2001 From: Philipp Zehnder Date: Sun, 7 Apr 2024 12:53:51 +0200 Subject: [PATCH 2/7] refactor(#2708): Add unit tests for getAllFiles --- .../streampipes/manager/file/FileManager.java | 16 ++++- .../manager/file/TestFileManager.java | 61 ++++++++++++++++++- 2 files changed, 75 insertions(+), 2 deletions(-) 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 bfd7518b86..b6371ded8a 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 @@ -34,12 +34,25 @@ public class FileManager { + private final IFileMetadataStorage fileMetadataStorage; + + public FileManager(IFileMetadataStorage fileMetadataStorage) { + this.fileMetadataStorage = fileMetadataStorage; + } + + public FileManager() { + this.fileMetadataStorage = StorageDispatcher + .INSTANCE + .getNoSqlStore() + .getFileMetadataStorage(); + } + public List getAllFiles() { return getAllFiles(null); } public List getAllFiles(String filetypes) { - List allFiles = getFileMetadataStorage().getAllFileMetadataDescriptions(); + List allFiles = fileMetadataStorage.getAllFileMetadataDescriptions(); return filetypes != null ? filterFiletypes(allFiles, filetypes) : allFiles; } @@ -123,6 +136,7 @@ protected void writeToFile(String sanitizedFilename, InputStream fileInputStream } + @Deprecated private IFileMetadataStorage getFileMetadataStorage() { return StorageDispatcher .INSTANCE 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 21f8d9dac3..1bc387764d 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 @@ -17,6 +17,9 @@ */ package org.apache.streampipes.manager.file; +import org.apache.streampipes.model.file.FileMetadata; +import org.apache.streampipes.storage.api.IFileMetadataStorage; + import org.apache.commons.io.IOUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -24,22 +27,78 @@ import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class TestFileManager { private FileManager fileManager; + private IFileMetadataStorage fileMetadataStorage; @BeforeEach public void setup() { - fileManager = new FileManager(); + fileMetadataStorage = mock(IFileMetadataStorage.class); + fileManager = new FileManager(fileMetadataStorage); + } + + @Test + public void getAllFiles_returnsAllFiles() { + var expected = prepareFileMetadataStorageWithTwoSampleFiles(); + + var result = fileManager.getAllFiles(); + + assertEquals(expected, result); + } + + @Test + public void getAllFiles_returnsAllFilesWhenFiletypesIsNull() { + var expected = prepareFileMetadataStorageWithTwoSampleFiles(); + + var result = fileManager.getAllFiles(null); + + assertEquals(expected, result); } + @Test + public void getAllFiles_returnsFilteredFilesWhenFiletypesIsNotNull() { + var files = prepareFileMetadataStorageWithTwoSampleFiles(); + + List result = fileManager.getAllFiles("csv"); + + assertEquals(1, result.size()); + assertEquals(files.get(0), result.get(0)); + } + + @Test + public void getAllFiles_returnsEmptyListWhenNoMatchingFiletypes() { + prepareFileMetadataStorageWithTwoSampleFiles(); + + var result = fileManager.getAllFiles("xml"); + + assertEquals(0, result.size()); + } + + private List prepareFileMetadataStorageWithTwoSampleFiles() { + List allFiles = Arrays.asList(createFile("csv"), createFile("json")); + when(fileMetadataStorage.getAllFileMetadataDescriptions()).thenReturn(allFiles); + + return allFiles; + } + + private FileMetadata createFile(String fileType) { + FileMetadata file = new FileMetadata(); + file.setFiletype(fileType); + return file; + } + + @Test public void storeFile_throwsExceptionForInvalidFileType() { var filename = "testFile.invalid"; From 7688dd139f602b850972dc5cdfd50cd6f838b2b0 Mon Sep 17 00:00:00 2001 From: Philipp Zehnder Date: Sun, 7 Apr 2024 13:00:52 +0200 Subject: [PATCH 3/7] refactor(#2708): Add unit tests for getFile --- .../streampipes/manager/file/FileManager.java | 12 +++++--- .../manager/file/TestFileManager.java | 28 +++++++++++++++++-- 2 files changed, 33 insertions(+), 7 deletions(-) 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 b6371ded8a..e984402649 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 @@ -35,9 +35,12 @@ public class FileManager { private final IFileMetadataStorage fileMetadataStorage; + private final FileHandler fileHandler; - public FileManager(IFileMetadataStorage fileMetadataStorage) { + public FileManager(IFileMetadataStorage fileMetadataStorage, + FileHandler fileHandler) { this.fileMetadataStorage = fileMetadataStorage; + this.fileHandler = fileHandler; } public FileManager() { @@ -45,6 +48,7 @@ public FileManager() { .INSTANCE .getNoSqlStore() .getFileMetadataStorage(); + this.fileHandler = new FileHandler(); } public List getAllFiles() { @@ -57,7 +61,7 @@ public List getAllFiles(String filetypes) { } public File getFile(String filename) { - return new FileHandler().getFile(filename); + return fileHandler.getFile(filename); } /** @@ -87,7 +91,7 @@ public FileMetadata storeFile(String user, public void deleteFile(String id) { FileMetadata fileMetadata = getFileMetadataStorage().getMetadataById(id); - new FileHandler().deleteFile(fileMetadata.getFilename()); + fileHandler.deleteFile(fileMetadata.getFilename()); getFileMetadataStorage().deleteFileMetadata(id); } @@ -132,7 +136,7 @@ public boolean validateFileType(String filename) { } protected void writeToFile(String sanitizedFilename, InputStream fileInputStream) throws IOException { - new FileHandler().storeFile(sanitizedFilename, fileInputStream); + fileHandler.storeFile(sanitizedFilename, fileInputStream); } 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 1bc387764d..c779b63096 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 @@ -24,6 +24,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; @@ -32,6 +33,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +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.Mockito.mock; @@ -41,11 +43,13 @@ public class TestFileManager { private FileManager fileManager; private IFileMetadataStorage fileMetadataStorage; + private FileHandler fileHandler; @BeforeEach public void setup() { fileMetadataStorage = mock(IFileMetadataStorage.class); - fileManager = new FileManager(fileMetadataStorage); + fileHandler = mock(FileHandler.class); + fileManager = new FileManager(fileMetadataStorage, fileHandler); } @Test @@ -86,18 +90,36 @@ public void getAllFiles_returnsEmptyListWhenNoMatchingFiletypes() { } private List prepareFileMetadataStorageWithTwoSampleFiles() { - List allFiles = Arrays.asList(createFile("csv"), createFile("json")); + List allFiles = Arrays.asList(createFileMetadata("csv"), createFileMetadata("json")); when(fileMetadataStorage.getAllFileMetadataDescriptions()).thenReturn(allFiles); return allFiles; } - private FileMetadata createFile(String fileType) { + private FileMetadata createFileMetadata(String fileType) { FileMetadata file = new FileMetadata(); file.setFiletype(fileType); return file; } + @Test + public void getFile_returnsExistingFile() { + var filename = "existingFile.txt"; + var expectedFile = new File(filename); + when(fileHandler.getFile(filename)).thenReturn(expectedFile); + + var result = fileManager.getFile(filename); + + assertEquals(expectedFile, result); + } + + @Test + public void getFile_returnsNullForNonExistingFile() { + var filename = "fileDoesNotExist.txt"; + when(fileHandler.getFile(filename)).thenReturn(null); + + assertNull(fileManager.getFile(filename)); + } @Test public void storeFile_throwsExceptionForInvalidFileType() { From f4c04b9fd3327f5f7fd9af8f5c5a046d6baf1cb9 Mon Sep 17 00:00:00 2001 From: Philipp Zehnder Date: Mon, 8 Apr 2024 08:34:32 +0200 Subject: [PATCH 4/7] 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"; From 49bfdeddd4b106f94c46dc3074b11557b4e5e945 Mon Sep 17 00:00:00 2001 From: Philipp Zehnder Date: Mon, 8 Apr 2024 08:43:56 +0200 Subject: [PATCH 5/7] refactor(#2708): Make FileHasher none static and add tests --- .../streampipes/commons/file/FileHasher.java | 7 ++- .../commons/file/FileHasherTest.java | 55 +++++++++++++++++++ .../connect/iiot/utils/FileProtocolUtils.java | 2 +- .../streampipes/manager/file/FileManager.java | 2 +- 4 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 streampipes-commons/src/test/java/org/apache/streampipes/commons/file/FileHasherTest.java diff --git a/streampipes-commons/src/main/java/org/apache/streampipes/commons/file/FileHasher.java b/streampipes-commons/src/main/java/org/apache/streampipes/commons/file/FileHasher.java index 8aac95a126..7b8c30eb62 100644 --- a/streampipes-commons/src/main/java/org/apache/streampipes/commons/file/FileHasher.java +++ b/streampipes-commons/src/main/java/org/apache/streampipes/commons/file/FileHasher.java @@ -28,7 +28,12 @@ public class FileHasher { - public static String hash(File file) throws IOException { + public String hash(File file) throws IOException { + + if (file == null) { + throw new IOException("Input file is null. Please provide a valid file to calculate the hash."); + } + try (InputStream is = Files.newInputStream(Paths.get(file.toURI()))) { return DigestUtils.md5Hex(is); } diff --git a/streampipes-commons/src/test/java/org/apache/streampipes/commons/file/FileHasherTest.java b/streampipes-commons/src/test/java/org/apache/streampipes/commons/file/FileHasherTest.java new file mode 100644 index 0000000000..7b8d1b981d --- /dev/null +++ b/streampipes-commons/src/test/java/org/apache/streampipes/commons/file/FileHasherTest.java @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.streampipes.commons.file; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.File; +import java.io.IOException; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class FileHasherTest { + + private FileHasher fileHasher; + + @BeforeEach + public void setup() { + this.fileHasher = new FileHasher(); + } + + @Test + void hash_returnsCorrectHashForFile() throws IOException { + var file = new File("src/test/resources/test.txt"); + assertEquals("6df4d50a41a5d20bc4faad8a6f09aa8f", fileHasher.hash(file)); + } + + @Test + void hash_throwsIOExceptionForNonExistingFile() { + var file = new File("src/test/resources/nonExistingFile.txt"); + assertThrows(IOException.class, () -> fileHasher.hash(file)); + } + + @Test + void hash_throwsIOExceptionForNullFile() { + assertThrows(IOException.class, () -> fileHasher.hash(null)); + } +} \ No newline at end of file diff --git a/streampipes-extensions/streampipes-connect-adapters-iiot/src/main/java/org/apache/streampipes/connect/iiot/utils/FileProtocolUtils.java b/streampipes-extensions/streampipes-connect-adapters-iiot/src/main/java/org/apache/streampipes/connect/iiot/utils/FileProtocolUtils.java index 91f0867a11..6c7ece4c44 100644 --- a/streampipes-extensions/streampipes-connect-adapters-iiot/src/main/java/org/apache/streampipes/connect/iiot/utils/FileProtocolUtils.java +++ b/streampipes-extensions/streampipes-connect-adapters-iiot/src/main/java/org/apache/streampipes/connect/iiot/utils/FileProtocolUtils.java @@ -56,7 +56,7 @@ public static InputStream getFileInputStream(String selectedFilename) throws Fil */ private static boolean checkIfFileChanged(String selectedFilename) { try { - var hash = FileHasher.hash(getFile(selectedFilename)); + var hash = new FileHasher().hash(getFile(selectedFilename)); StreamPipesClient client = getStreamPipesClientInstance(); return client.fileApi() .checkFileContentChanged(selectedFilename, hash); 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 41b1dcb724..0259716d50 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 @@ -123,7 +123,7 @@ protected InputStream cleanFile(InputStream fileInputStream, String filetype) { } public boolean checkFileContentChanged(String filename, String hash) throws IOException { - var fileHash = FileHasher.hash(getFile(filename)); + var fileHash = new FileHasher().hash(getFile(filename)); return !fileHash.equals(hash); } From 875a1f4dc7c356b86422f10cbffe8369f863dea7 Mon Sep 17 00:00:00 2001 From: Philipp Zehnder Date: Mon, 8 Apr 2024 10:54:33 +0200 Subject: [PATCH 6/7] refactor(#2708): Add unit test for checkFileContentChanged in FileManager --- .../streampipes/manager/file/FileManager.java | 8 +++-- .../manager/file/TestFileManager.java | 34 ++++++++++++++++++- 2 files changed, 39 insertions(+), 3 deletions(-) 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 0259716d50..84bb7f47b7 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 @@ -36,11 +36,14 @@ public class FileManager { private final IFileMetadataStorage fileMetadataStorage; private final FileHandler fileHandler; + private final FileHasher fileHasher; public FileManager(IFileMetadataStorage fileMetadataStorage, - FileHandler fileHandler) { + FileHandler fileHandler, + FileHasher fileHasher) { this.fileMetadataStorage = fileMetadataStorage; this.fileHandler = fileHandler; + this.fileHasher = fileHasher; } public FileManager() { @@ -49,6 +52,7 @@ public FileManager() { .getNoSqlStore() .getFileMetadataStorage(); this.fileHandler = new FileHandler(); + this.fileHasher = new FileHasher(); } public List getAllFiles() { @@ -123,7 +127,7 @@ protected InputStream cleanFile(InputStream fileInputStream, String filetype) { } public boolean checkFileContentChanged(String filename, String hash) throws IOException { - var fileHash = new FileHasher().hash(getFile(filename)); + var fileHash = fileHasher.hash(getFile(filename)); return !fileHash.equals(hash); } 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 f62fb9a888..da62437bc6 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 @@ -17,6 +17,7 @@ */ package org.apache.streampipes.manager.file; +import org.apache.streampipes.commons.file.FileHasher; import org.apache.streampipes.model.file.FileMetadata; import org.apache.streampipes.storage.api.IFileMetadataStorage; @@ -51,6 +52,8 @@ public class TestFileManager { private FileManager fileManager; private IFileMetadataStorage fileMetadataStorage; private FileHandler fileHandler; + private FileHasher fileHasher; + private static final String TEST_USER = "testUser"; @@ -58,7 +61,8 @@ public class TestFileManager { public void setup() { fileMetadataStorage = mock(IFileMetadataStorage.class); fileHandler = mock(FileHandler.class); - fileManager = new FileManager(fileMetadataStorage, fileHandler); + fileHasher = mock(FileHasher.class); + fileManager = new FileManager(fileMetadataStorage, fileHandler, fileHasher); } @Test @@ -303,4 +307,32 @@ public void validateFileName_returnsTrueForJson() { public void validateFileName_returnsFalseForSh() { assertFalse(fileManager.validateFileType("file.sh")); } + + @Test + public void checkFileContentChanged_returnsTrueWhenContentHasChanged() throws IOException { + var filename = "testFile.csv"; + var originalHash = "originalHash"; + var changedHash = "changedHash"; + + when(fileHandler.getFile(filename)).thenReturn(new File(filename)); + when(fileHasher.hash(any(File.class))).thenReturn(changedHash); + + var result = fileManager.checkFileContentChanged(filename, originalHash); + + assertTrue(result); + } + + @Test + public void checkFileContentChanged_returnsFalseWhenContentHasNotChanged() throws IOException { + var filename = "testFile.csv"; + var originalHash = "originalHash"; + + when(fileHandler.getFile(filename)).thenReturn(new File(filename)); + when(fileHasher.hash(any(File.class))).thenReturn(originalHash); + + var result = fileManager.checkFileContentChanged(filename, originalHash); + + assertFalse(result); + } + } \ No newline at end of file From 1ea746446f8d4509299622ae59943219ceb5a345 Mon Sep 17 00:00:00 2001 From: Philipp Zehnder Date: Mon, 8 Apr 2024 10:59:27 +0200 Subject: [PATCH 7/7] refactor(#2708): Remove hash code from FileMetadata --- .../streampipes/model/file/FileMetadata.java | 20 ------------------- 1 file changed, 20 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 a6b1fe7922..4696179903 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,8 +21,6 @@ import com.google.gson.annotations.SerializedName; -import java.util.Objects; - @TsModel public class FileMetadata { @@ -94,22 +92,4 @@ 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); - } }