fix: improve generation of S3 ID

Avoid converting `Path` to/from `String` and `Path` to `File` by using `Path` everywhere. Avoid using regex replacement to create the ID by using the path name directly.
project-ncl · Feb 18, 2025 · 7ba174e · 7ba174e
1 parent 04cd15e
commit 7ba174e
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 40 deletions.
diff --git a/service/src/main/java/org/jboss/sbomer/service/feature/s3/S3ClientFacade.java b/service/src/main/java/org/jboss/sbomer/service/feature/s3/S3ClientFacade.java
@@ -174,12 +174,12 @@ public boolean doesObjectExists(String key) {
         }
     }
 
-    public void upload(String path, String key) {
+    public void upload(Path path, String key) {
         log.debug("Uploading '{}' file as '{}'...", path, key);
 
         try {
             PutObjectRequest request = PutObjectRequest.builder().key(key).bucket(bucketName()).build();
-            client.putObject(request, Path.of(path));
+            client.putObject(request, path);
         } catch (SdkException e) {
             throw new ApplicationException("An error occurred when uploading '{}' file to S3", path, e);
         }

diff --git a/service/src/main/java/org/jboss/sbomer/service/feature/s3/S3StorageHandler.java b/service/src/main/java/org/jboss/sbomer/service/feature/s3/S3StorageHandler.java
@@ -19,9 +19,12 @@
 package org.jboss.sbomer.service.feature.s3;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.stream.Stream;
 
 import org.jboss.sbomer.core.errors.ApplicationException;
 import org.jboss.sbomer.core.errors.NotFoundException;
@@ -58,10 +61,10 @@ public class S3StorageHandler {
      * @param rootDirectory
      * @return List of paths to all files.
      */
-    private List<String> getFilePaths(File rootDirectory) {
-        List<String> filePaths = new ArrayList<>();
+    private List<Path> getFilePaths(Path rootDirectory) {
+        List<Path> filePaths = new ArrayList<>();
 
-        if (rootDirectory.exists() && rootDirectory.isDirectory()) {
+        if (Files.isDirectory(rootDirectory)) {
             S3StorageHandler.getAllFileNamesRecursive(rootDirectory, filePaths);
         } else {
             log.error(
@@ -75,32 +78,30 @@ private List<String> getFilePaths(File rootDirectory) {
     /**
      * Populates the {@code filePaths} with absolute paths to files located under the {@code directory}.
      *
-     * @param directory
-     * @param filePaths
+     * @param directory the directory
+     * @param filePaths the list of file paths
      */
-    private static void getAllFileNamesRecursive(File directory, List<String> filePaths) {
-        log.debug("Handling '{}' directory...", directory.getAbsolutePath());
-
-        File[] files = directory.listFiles();
-
-        if (files == null) {
-            log.debug("Provided directory is empty or unable to read files, ignoring");
-            return;
-        }
-
-        for (File file : files) {
-            log.debug("Examining path '{}'...", file.getName());
-
-            if (file.getName().startsWith(".")) {
-                log.debug("Skipping '{}', because it's a hidden file", file.getName());
-                continue;
-            }
-
-            if (file.isDirectory()) {
-                S3StorageHandler.getAllFileNamesRecursive(file, filePaths);
-            } else {
-                filePaths.add(file.getAbsolutePath());
-            }
+    private static void getAllFileNamesRecursive(Path directory, List<Path> filePaths) {
+        log.debug("Handling '{}' directory...", directory.toAbsolutePath());
+
+        try (Stream<Path> stream = Files.list(directory)) {
+            stream.forEach(path -> {
+                Path fileName = path.getFileName();
+
+                log.debug("Examining path '{}'...", fileName);
+
+                if (fileName.startsWith(".")) {
+                    log.debug("Skipping '{}', because it's a hidden file", fileName);
+                } else {
+                    if (Files.isDirectory(path)) {
+                        S3StorageHandler.getAllFileNamesRecursive(path, filePaths);
+                    } else {
+                        filePaths.add(path.toAbsolutePath());
+                    }
+                }
+            });
+        } catch (IOException e) {
+            log.debug("Unable to read files in directory '" + directory + "', ignoring");
         }
     }
 
@@ -126,17 +127,17 @@ public void storeFiles(GenerationRequest generationRequest) {
 
         log.info("Storing data in S3 for Generation request '{}'", generationRequest.getId());
 
-        File generationRootDir = Path.of(controllerConfig.sbomDir(), generationRequest.getMetadata().getName())
-                .toFile();
+        Path generationRootDir = Path.of(controllerConfig.sbomDir(), generationRequest.getMetadata().getName());
 
-        log.debug("Using '{}' directory to scan for files to be uploaded to S3", generationRootDir.getAbsolutePath());
+        log.debug("Using '{}' directory to scan for files to be uploaded to S3", generationRootDir.toAbsolutePath());
 
-        List<String> filePaths = getFilePaths(generationRootDir);
+        List<Path> filePaths = getFilePaths(generationRootDir);
 
         log.debug("Found {} files: {}", filePaths.size(), filePaths);
 
         filePaths.forEach(path -> {
-            String key = path.replaceFirst(generationRootDir.getAbsolutePath(), generationRequest.getId());
+            String key = String.join("/", generationRequest.getId(), generationRootDir.relativize(path).toString());
+
             if (!client.doesObjectExists(key)) {
                 client.upload(path, key);
             }

diff --git a/service/src/test/java/org/jboss/sbomer/service/test/integ/feature/s3/S3FeatureTest.java b/service/src/test/java/org/jboss/sbomer/service/test/integ/feature/s3/S3FeatureTest.java
@@ -123,19 +123,19 @@ void testStoreFiles(@TempDir Path tempDir) throws IOException {
         Path logFile = logsDir.resolve("init.log");
         Files.write(logFile, "Some log".getBytes());
 
-        // doNothing().when(clientFacade).upload(file.toFile().getAbsolutePath(),
+        // doNothing().when(clientFacade).upload(file.toAbsolutePath(),
         // "bucket-name/bucket-name/bom.json");
         when(clientFacade.doesObjectExists("AABBCC/bom.json")).thenReturn(false);
         when(clientFacade.doesObjectExists("AABBCC/logs/init.log")).thenReturn(true);
-        doNothing().when(clientFacade).upload(file.toFile().getAbsolutePath(), "AABBCC/bom.json");
-        doNothing().when(clientFacade).upload(logFile.toFile().getAbsolutePath(), "AABBCC/logs/init.log");
+        doNothing().when(clientFacade).upload(file.toAbsolutePath(), "AABBCC/bom.json");
+        doNothing().when(clientFacade).upload(logFile.toAbsolutePath(), "AABBCC/logs/init.log");
 
         storageHandler.storeFiles(generationRequest);
 
         verify(clientFacade, times(1)).doesObjectExists("AABBCC/bom.json");
-        verify(clientFacade, times(1)).upload(file.toFile().getAbsolutePath(), "AABBCC/bom.json");
+        verify(clientFacade, times(1)).upload(file.toAbsolutePath(), "AABBCC/bom.json");
         verify(clientFacade, times(1)).doesObjectExists("AABBCC/logs/init.log");
-        verify(clientFacade, times(0)).upload(logFile.toFile().getAbsolutePath(), "AABBCC/logs/init.log");
+        verify(clientFacade, times(0)).upload(logFile.toAbsolutePath(), "AABBCC/logs/init.log");
     }
 
     @Test