Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/main/java/me/itzg/helpers/files/ReactiveFileUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
}
30 changes: 1 addition & 29 deletions src/main/java/me/itzg/helpers/get/GetCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down Expand Up @@ -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) -> {
Expand All @@ -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) {
Expand Down Expand Up @@ -500,10 +477,5 @@ private static URI removeUserInfo(URI uri) throws URISyntaxException {
);
}

private Path getSaveToFile() {
return useTempFile ?
Paths.get(outputFile.toString() + ".download") :
outputFile;
}

}
2 changes: 1 addition & 1 deletion src/main/java/me/itzg/helpers/http/SharedFetch.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) ->
Expand Down Expand Up @@ -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);
Expand All @@ -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()))
);
}
Expand Down
119 changes: 0 additions & 119 deletions src/test/java/me/itzg/helpers/get/OutputToFileTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

}