Download via .download temp file then rename (#627)

* Support optional --use-temp-dir in get to download to temp dir and copy, to avoid partial jar if interrupted

* Change implementation to use adjacent temp file instead of temp directory

* Implement adjacent temporary file download with proper error handling and comprehensive tests

---------

Co-authored-by: Tom Abnett <[email protected]>

Author: pgcudahy

src/main/java/me/itzg/helpers/get/GetCommand.java

@@ -134,6 +134,11 @@ public class GetCommand implements Callable<Integer> {
     @Option(names = "--retry-delay", description = "in seconds", defaultValue = "2")
     int retryDelay;
 
+    @Option(names = {"--use-temp-file"},
+        description = "Download to a temporary file in the same directory with .download extension, then rename to the final destination when complete",
+        defaultValue = "false")
+    boolean useTempFile;
+
     @Parameters(split = OPTION_SPLIT_COMMAS, paramLabel = "URI",
         description = "The URI of the resource to retrieve. When the output is a directory,"
             + " more than one URI can be requested.",
@@ -209,18 +214,36 @@ null, new JsonPathOutputHandler(
                         stdout.println(outputFile);
                     }
                 } else {
-                    final Path file = fetch(uris.get(0))
-                        .toFile(outputFile)
-                        .skipUpToDate(skipUpToDate)
-                        .acceptContentTypes(acceptContentTypes)
-                        .handleDownloaded((uri, f, contentSizeBytes) -> {
-                            if (logProgressEach) {
-                                log.info("Downloaded {}", f);
+                    final Path saveToFile = getSaveToFile();
+                    try {
+                        final Path file = fetch(uris.get(0))
+                            .toFile(saveToFile)
+                            .skipUpToDate(skipUpToDate)
+                            .acceptContentTypes(acceptContentTypes)
+                            .handleDownloaded((uri, f, contentSizeBytes) -> {
+                                if (logProgressEach) {
+                                    log.info("Downloaded {}", f);
+                                }
+                            })
+                            .execute();
+                        if (useTempFile) {
+                            Files.move(saveToFile, outputFile);
+                        }
+                        if (this.outputFilename) {
+                            stdout.println(file);
+                        }
+                    } catch (Exception e) {
+                        // Clean up temp file if download fails
+                        if (useTempFile && Files.exists(saveToFile)) {
+                            try {
+                                Files.delete(saveToFile);
+                                log.debug("Cleaned up temporary file {} after failed download", saveToFile);
+                            } catch (IOException cleanupEx) {
+                                log.warn("Failed to clean up temporary file {} after failed download", 
+                                    saveToFile, cleanupEx);
                             }
-                        })
-                        .execute();
-                    if (this.outputFilename) {
-                        stdout.println(file);
+                        }
+                        throw e; // Re-throw the original exception
                     }
                 }
             }
@@ -477,5 +500,10 @@ private static URI removeUserInfo(URI uri) throws URISyntaxException {
         );
     }
 
+    private Path getSaveToFile() {
+        return useTempFile ? 
+            Paths.get(outputFile.toString() + ".download") : 
+            outputFile;
+    }
 
 }

src/test/java/me/itzg/helpers/get/OutputToFileTest.java

@@ -230,4 +230,123 @@ void skipsUpToDate_butDownloadsWhenAbsent(@TempDir Path tempDir) throws IOExcept
     assertThat(fileToDownload).hasContent("New content");
   }
 
+  @Test
+  void successfulWithTemporaryFile(@TempDir Path tempDir) throws MalformedURLException, IOException {
+    mock.expectRequest("GET", "/downloadsToFile.txt",
+        response()
+            .withBody("Response content to file", MediaType.TEXT_PLAIN)
+    );
+
+    final Path expectedFile = tempDir.resolve("out.txt");
+
+    final int status =
+        new CommandLine(new GetCommand())
+            .execute(
+                "-o",
+                expectedFile.toString(),
+                "--use-temp-file",
+                mock.buildMockedUrl("/downloadsToFile.txt").toString()
+            );
+
+    assertThat(status).isEqualTo(0);
+    assertThat(expectedFile).exists();
+    assertThat(expectedFile).hasContent("Response content to file");
+    // The temporary file with .download extension should no longer exist after successful download
+    assertThat(tempDir.resolve("out.txt.download")).doesNotExist();
+  }
+  
+  @Test
+  void handlesExistingDownloadFile(@TempDir Path tempDir) throws MalformedURLException, IOException {
+    mock.expectRequest("GET", "/downloadsToFile.txt",
+        response()
+            .withBody("New content", MediaType.TEXT_PLAIN)
+    );
+
+    final Path expectedFile = tempDir.resolve("out.txt");
+    final Path downloadFile = tempDir.resolve("out.txt.download");
+    
+    // Create a pre-existing .download file with different content
+    Files.writeString(downloadFile, "Partial old content");
+    
+    final int status =
+        new CommandLine(new GetCommand())
+            .execute(
+                "-o",
+                expectedFile.toString(),
+                "--use-temp-file",
+                mock.buildMockedUrl("/downloadsToFile.txt").toString()
+            );
+
+    assertThat(status).isEqualTo(0);
+    assertThat(expectedFile).exists();
+    assertThat(expectedFile).hasContent("New content");
+    // The temporary file should be gone
+    assertThat(downloadFile).doesNotExist();
+  }
+  
+  @Test
+  void preservesOriginalWhenErrorOccurs(@TempDir Path tempDir) throws MalformedURLException, IOException {
+    mock.expectRequest("GET", "/errorFile.txt",
+        response()
+            .withStatusCode(500)
+            .withBody("Server error", MediaType.TEXT_PLAIN)
+    );
+
+    final Path expectedFile = tempDir.resolve("out.txt");
+    final String originalContent = "Original content that should be preserved";
+    
+    // Create the original file with content that should remain untouched
+    Files.writeString(expectedFile, originalContent);
+    
+    final int status =
+        new CommandLine(new GetCommand())
+            .execute(
+                "-o",
+                expectedFile.toString(),
+                "--use-temp-file",
+                mock.buildMockedUrl("/errorFile.txt").toString()
+            );
+
+    // Should fail with non-zero status
+    assertThat(status).isNotEqualTo(0);
+    // Original file should still exist with unchanged content
+    assertThat(expectedFile).exists();
+    assertThat(expectedFile).hasContent(originalContent);
+    // Any temporary download file should be cleaned up
+    assertThat(tempDir.resolve("out.txt.download")).doesNotExist();
+  }
+  
+  @Test
+  void preservesOriginalWhenDownloadHasInvalidContent(@TempDir Path tempDir) throws MalformedURLException, IOException {
+    // Set up a request that will result in an error during processing
+    // We'll return content with a valid Content-Length but corrupted/truncated data
+    mock.expectRequest("GET", "/interruptedFile.txt",
+        response()
+            .withHeader("Content-Length", "1000")  // Much larger than actual content
+            .withBody("This is only part of the expected content...")  // Truncated content
+    );
+
+    final Path expectedFile = tempDir.resolve("out.txt");
+    final String originalContent = "Original content that should remain intact";
+    
+    // Create the original file with content that should remain untouched
+    Files.writeString(expectedFile, originalContent);
+    
+    final int status =
+        new CommandLine(new GetCommand())
+            .execute(
+                "-o",
+                expectedFile.toString(),
+                "--use-temp-file",
+                mock.buildMockedUrl("/interruptedFile.txt").toString()
+            );
+    
+    // Original file should still exist with unchanged content
+    assertThat(expectedFile).exists();
+    assertThat(expectedFile).hasContent(originalContent);
+    
+    // Any temporary download file should be cleaned up
+    assertThat(tempDir.resolve("out.txt.download")).doesNotExist();
+  }
+
 }