From 4dfde5a1bffdddab781acb252cd7c3458dc958a4 Mon Sep 17 00:00:00 2001 From: Geoff Bourne Date: Mon, 8 Sep 2025 20:30:35 -0500 Subject: [PATCH 1/4] fetch: download specific file to adjacent temp --- .../java/me/itzg/helpers/files/ReactiveFileUtils.java | 11 +++++++++++ .../itzg/helpers/http/SpecificFileFetchBuilder.java | 6 ++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/main/java/me/itzg/helpers/files/ReactiveFileUtils.java b/src/main/java/me/itzg/helpers/files/ReactiveFileUtils.java index f2deae7b..d80d351d 100644 --- a/src/main/java/me/itzg/helpers/files/ReactiveFileUtils.java +++ b/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 Mono removeFailedDownload(Throwable throwable, Path outputF }) .subscribeOn(Schedulers.boundedElastic()); } + + public static Function> 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()); + } } diff --git a/src/main/java/me/itzg/helpers/http/SpecificFileFetchBuilder.java b/src/main/java/me/itzg/helpers/http/SpecificFileFetchBuilder.java index 09bc3389..29df24c7 100644 --- a/src/main/java/me/itzg/helpers/http/SpecificFileFetchBuilder.java +++ b/src/main/java/me/itzg/helpers/http/SpecificFileFetchBuilder.java @@ -64,6 +64,7 @@ public Mono 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 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 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())) ); } From 7a3fb493a9f8cd0a102430f4a9dccdfde0e0ffb6 Mon Sep 17 00:00:00 2001 From: Geoff Bourne Date: Tue, 9 Sep 2025 21:38:48 -0500 Subject: [PATCH 2/4] get: also benefits from temp download mechanism --- .../java/me/itzg/helpers/get/GetCommand.java | 30 +------------------ 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/src/main/java/me/itzg/helpers/get/GetCommand.java b/src/main/java/me/itzg/helpers/get/GetCommand.java index 7dc8d637..1b613d5b 100644 --- a/src/main/java/me/itzg/helpers/get/GetCommand.java +++ b/src/main/java/me/itzg/helpers/get/GetCommand.java @@ -134,11 +134,6 @@ public class GetCommand implements Callable { @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; - } } From ba3b1a279b77eaa180b7ee28e07a0a4354796e90 Mon Sep 17 00:00:00 2001 From: Geoff Bourne Date: Tue, 9 Sep 2025 21:43:31 -0500 Subject: [PATCH 3/4] get: also benefits from temp download mechanism --- .../me/itzg/helpers/get/OutputToFileTest.java | 119 ------------------ 1 file changed, 119 deletions(-) diff --git a/src/test/java/me/itzg/helpers/get/OutputToFileTest.java b/src/test/java/me/itzg/helpers/get/OutputToFileTest.java index 18c0dd74..d9e192bd 100644 --- a/src/test/java/me/itzg/helpers/get/OutputToFileTest.java +++ b/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(); - } - } From e0633a95bde4c341c42ce0735b40fcbef1b4e32e Mon Sep 17 00:00:00 2001 From: Geoff Bourne Date: Wed, 10 Sep 2025 17:38:09 -0500 Subject: [PATCH 4/4] http: use HTTP/1.1 as default in options builder --- src/main/java/me/itzg/helpers/http/SharedFetch.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/me/itzg/helpers/http/SharedFetch.java b/src/main/java/me/itzg/helpers/http/SharedFetch.java index 14b28672..131133ff 100644 --- a/src/main/java/me/itzg/helpers/http/SharedFetch.java +++ b/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;