Clarify packing errors, check existing file by checksum

pace-packaging/src/main/java/edu/colorado/cires/pace/packaging/FileUtils.java

@@ -31,7 +31,7 @@ public static void copyFile(Path source, Path target) throws IOException {
   public static void appendChecksumToManifest(FileWriter fileWriter, Path path, Path manifestParentDirectory) throws IOException {
     String checksum;
     try (InputStream inputStream = new FileInputStream(path.toFile())) {
-      checksum = DigestUtils.sha256Hex(inputStream);
+      checksum = computeChecksum(inputStream);
     }
 
     fileWriter.append(String.format(
@@ -49,9 +49,18 @@ public static boolean filterHidden(Path path) throws IOException {
     return Files.isRegularFile(path) && !Files.isHidden(path);
   }
 
-  public static boolean filterTimeSize(Path source, Path target) throws IOException {
-    return !Files.exists(target) || Files.getLastModifiedTime(source).toMillis() != Files.getLastModifiedTime(target).toMillis() ||
-        Files.size(source) != Files.size(target);
+  public static boolean filterByChecksum(Path source, Path target) throws IOException {
+    if (Files.exists(target)) {
+      try (InputStream sourceStream = new FileInputStream(source.toFile()); InputStream targetStream = new FileInputStream(target.toFile())) {
+        return !computeChecksum(sourceStream).equals(computeChecksum(targetStream));
+      }
+    }
+
+    return true;
+  }
+
+  private static String computeChecksum(InputStream inputStream) throws IOException {
+    return DigestUtils.sha256Hex(inputStream);
   }
 
   public static Path writeMetadata(DetailedPackage aPackage, Path targetDirectory) throws IOException {

pace-packaging/src/main/java/edu/colorado/cires/pace/packaging/PackageInstructionFactory.java

@@ -117,7 +117,7 @@ private static Stream<PackageInstruction> processPath(Supplier<Path> pathGetter,
             try {
               boolean regularFile = FileUtils.filterHidden(p);
               if (!regularFile) {
-                logger.warn("Filtered file will not be moved: {}", p);
+                logger.warn("Hidden file will not be moved: {}", p);
               }
               return regularFile;
             } catch (IOException e) {
@@ -131,9 +131,9 @@ private static Stream<PackageInstruction> processPath(Supplier<Path> pathGetter,
               )
           ).filter(packageInstruction -> {
             try {
-              boolean shouldMoveFile = FileUtils.filterTimeSize(packageInstruction.source(), packageInstruction.target());
+              boolean shouldMoveFile = FileUtils.filterByChecksum(packageInstruction.source(), packageInstruction.target());
               if (!shouldMoveFile) {
-                logger.warn("Filtered file will not be moved: {}", packageInstruction.source());
+                logger.warn("Identical file already exists: {}", packageInstruction.target());
               }
               return shouldMoveFile;
             } catch (IOException e) {

pace-packaging/src/main/java/edu/colorado/cires/pace/packaging/Packager.java

@@ -3,11 +3,9 @@
 import java.io.FileWriter;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
-import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Path;
 import java.time.LocalDate;
 import java.time.format.DateTimeFormatter;
-import java.util.Objects;
 import java.util.stream.Stream;
 import org.apache.logging.log4j.Logger;
 
@@ -63,38 +61,23 @@ protected static Path copyFilesAndWriteManifest(Stream<PackageInstruction> moveI
     Path outputFile = outputDir.resolve("manifest-sha256.txt");
 
     try (FileWriter writer = new FileWriter(outputFile.toFile(), StandardCharsets.UTF_8, true)) {
-      RuntimeException exception = moveInstructions
-          .map(packageInstruction -> {
+      moveInstructions
+          .forEach(packageInstruction -> {
             try {
               FileUtils.copyFile(packageInstruction.source(), packageInstruction.target());
               logger.info("Copied {} to {}", packageInstruction.source(), packageInstruction.target());
               FileUtils.appendChecksumToManifest(writer, packageInstruction.target(), outputDir);
               logger.info("Appended {} checksum to {}", packageInstruction.target(), outputFile);
 
               incrementProgressFn.run();
-              return null;
-            } catch (FileAlreadyExistsException e) {
-              return new RuntimeException(String.format(
-                  "Path already exists: %s", packageInstruction.target()
-              ));
             } catch (IOException e) {
-              return new RuntimeException(e);
+              throw new RuntimeException(e);
             }
-          }).filter(Objects::nonNull)
-          .reduce(new RuntimeException("Packaging failed"), (o1, o2) -> {
-            o1.addSuppressed(o2);
-            return o1;
           });
-      if (exception.getSuppressed().length != 0) {
-        throw new PackagingException(
-            exception.getMessage(),
-            exception.getSuppressed()
-        );
-      }
 
-    } catch (IOException e) {
+    } catch (IOException | RuntimeException e) {
       throw new PackagingException(String.format(
-          "Failed to write %s", outputFile
+          "Packaging failed: %s", e.getMessage()
       ), e);
     }
 

pace-packaging/src/main/java/edu/colorado/cires/pace/packaging/PackagingException.java

@@ -6,10 +6,4 @@ public PackagingException(String message, Throwable cause) {
     super(message, cause);
   }
 
-  public PackagingException(String message, Throwable[] causes) {
-    super(message);
-    for (Throwable cause : causes) {
-      super.addSuppressed(cause);
-    }
-  }
 }

pace-packaging/src/test/java/edu/colorado/cires/pace/packaging/FileUtilTest.java

@@ -46,23 +46,18 @@ void testFilterHidden() throws IOException {
   }
 
   @Test
-  void testFilterTimeSize() throws IOException, InterruptedException {
+  void testFilterByChecksum() throws IOException {
     Path path = TEST_PATH.resolve("test.txt");
     org.apache.commons.io.FileUtils.createParentDirectories(path.toFile());
     Files.createFile(path);
 
-    assertFalse(FileUtils.filterTimeSize(path, path)); // same file = fail
+    assertFalse(FileUtils.filterByChecksum(path, path)); // same file = fail
 
     Path path2 = TEST_PATH.resolve("test1.txt");
-    assertTrue(FileUtils.filterTimeSize(path, path2)); // target does not exist = pass
-
-    Thread.sleep(10);    
-    org.apache.commons.io.FileUtils.createParentDirectories(path2.toFile());
-    Files.createFile(path2);
-    assertTrue(FileUtils.filterTimeSize(path, path2)); // modified times not equal = pass
+    assertTrue(FileUtils.filterByChecksum(path, path2)); // target does not exist = pass
 
     org.apache.commons.io.FileUtils.writeStringToFile(path2.toFile(), "Hello World", StandardCharsets.UTF_8);
-    assertTrue(FileUtils.filterTimeSize(path, path2)); // size not equal = pass
+    assertTrue(FileUtils.filterByChecksum(path, path2)); // size not equal = pass
   }
 
 }

pace-packaging/src/test/java/edu/colorado/cires/pace/packaging/PackagerTest.java

@@ -206,20 +206,20 @@ void testWriteTagManifestFileDoesNotExist() throws PackagingException {
   void testFileProcessingFailure() throws IOException {
     try (MockedStatic<edu.colorado.cires.pace.packaging.FileUtils> mockedStatic = Mockito.mockStatic(
         edu.colorado.cires.pace.packaging.FileUtils.class)) {
+      Exception exception = new IOException("test file error");
       mockedStatic.when(() -> edu.colorado.cires.pace.packaging.FileUtils.appendChecksumToManifest(any(), any(), any())).thenThrow(
-          new IOException("test file error")
+          exception
       );
       mockedStatic.when(() -> edu.colorado.cires.pace.packaging.FileUtils.mkdir(any())).thenCallRealMethod();
 
       List<PackageInstruction> packageInstructions = getInstructionForSourceDir().toList();
 
       ProgressIndicator progressIndicator = mock(ProgressIndicator.class);
 
-      Exception exception = assertThrows(PackagingException.class, () -> Packager.run(packageInstructions.stream(), TARGET_DIR, LogManager.getLogger("test"), progressIndicator));
-      assertEquals("Packaging failed", exception.getMessage());
-      for (Throwable throwable : exception.getSuppressed()) {
-        assertEquals("java.io.IOException: test file error", throwable.getMessage());
-      }
+      Exception packagingException = assertThrows(PackagingException.class, () -> Packager.run(packageInstructions.stream(), TARGET_DIR, LogManager.getLogger("test"), progressIndicator));
+      assertEquals(String.format(
+          "Packaging failed: java.io.IOException: %s", exception.getMessage()
+      ), packagingException.getMessage());
 
       verify(progressIndicator, times(0)).incrementProcessedRecords();
     }
@@ -231,7 +231,7 @@ void testWriteManifestDirectoryDoesNotExist() {
 
     Exception exception = assertThrows(PackagingException.class, () -> Packager.copyFilesAndWriteManifest(Stream.empty(), TARGET_DIR, progressIndicator::incrementProcessedRecords, LogManager.getLogger("test")));
     assertEquals(String.format(
-        "Failed to write %s", TARGET_DIR.resolve("manifest-sha256.txt")
+        "Packaging failed: %s (No such file or directory)", TARGET_DIR.resolve("manifest-sha256.txt")
     ), exception.getMessage());
 
     verify(progressIndicator, times(0)).incrementProcessedRecords();