fetch: download specific file to adjacent temp (#629)

* get: also benefits from temp download mechanism
* http: use HTTP/1.1 as default in options builder

Author: itzg

src/main/java/me/itzg/helpers/files/ReactiveFileUtils.java

@@ -5,8 +5,10 @@
 import java.nio.channels.FileChannel;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
 import java.nio.file.StandardOpenOption;
 import java.time.Instant;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import lombok.extern.slf4j.Slf4j;
 import reactor.core.publisher.Mono;
@@ -107,4 +109,13 @@ public static <T> Mono<T> removeFailedDownload(Throwable throwable, Path outputF
             })
             .subscribeOn(Schedulers.boundedElastic());
     }
+
+    public static Function<Path, Mono<Path>> moveTo(Path to) {
+        return from -> Mono.fromCallable(() -> {
+                log.debug("Moving {} to {}", from, to);
+                Files.move(from, to, StandardCopyOption.REPLACE_EXISTING);
+                return to;
+            })
+            .subscribeOn(Schedulers.boundedElastic());
+    }
 }

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

@@ -134,11 +134,6 @@ 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.",
@@ -214,10 +209,8 @@ null, new JsonPathOutputHandler(
                         stdout.println(outputFile);
                     }
                 } else {
-                    final Path saveToFile = getSaveToFile();
-                    try {
                         final Path file = fetch(uris.get(0))
-                            .toFile(saveToFile)
+                        .toFile(outputFile)
                             .skipUpToDate(skipUpToDate)
                             .acceptContentTypes(acceptContentTypes)
                             .handleDownloaded((uri, f, contentSizeBytes) -> {
@@ -226,25 +219,9 @@ null, new JsonPathOutputHandler(
                                 }
                             })
                             .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);
-                            }
-                        }
-                        throw e; // Re-throw the original exception
-                    }
                 }
             }
         } catch (URISyntaxException e) {
@@ -500,10 +477,5 @@ private static URI removeUserInfo(URI uri) throws URISyntaxException {
         );
     }
 
-    private Path getSaveToFile() {
-        return useTempFile ? 
-            Paths.get(outputFile.toString() + ".download") : 
-            outputFile;
-    }
 
 }

src/main/java/me/itzg/helpers/http/SharedFetch.java

@@ -163,7 +163,7 @@ public static class Options {
         private final URI filesViaUrl;
 
         @Default
-        private final boolean useHttp2 = true;
+        private final boolean useHttp2 = false;
 
         private final boolean wiretap;
 

src/main/java/me/itzg/helpers/http/SpecificFileFetchBuilder.java

@@ -64,6 +64,7 @@ public Mono<Path> assemble() {
 
         final boolean useIfModifiedSince = skipUpToDate && Files.exists(file);
 
+        final Path tempDownloadFile = file.getParent().resolve(file.getFileName() + ".download");
         return useReactiveClient(client ->
             client
                 .doOnRequest((httpClientRequest, connection) ->
@@ -107,7 +108,7 @@ public Mono<Path> assemble() {
                         return failedContentTypeMono(resp);
                     }
 
-                    return ReactiveFileUtils.writeByteBufFluxToFile(byteBufFlux, file)
+                    return ReactiveFileUtils.writeByteBufFluxToFile(byteBufFlux, tempDownloadFile)
                         .flatMap(fileSize -> {
                             statusHandler.call(FileDownloadStatus.DOWNLOADED, uri, file);
                             downloadedHandler.call(uri, file, fileSize);
@@ -120,12 +121,13 @@ public Mono<Path> assemble() {
                                             uri, formatDuration(durationMillis), transferRate(durationMillis, fileSize)
                                         );
                                     }
-                                    return Mono.just(file);
+                                    return Mono.just(tempDownloadFile);
                                 });
                         });
 
                 })
                 .last()
+                .flatMap(ReactiveFileUtils.moveTo(file))
                 .contextWrite(context -> context.put("downloadStart", currentTimeMillis()))
         );
     }

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

@@ -230,123 +230,4 @@ 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();
-  }
-
 }