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
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-7c3e9d1.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "AWS SDK for Java v2",
"contributor": "",
"description": "Prevent `AsyncResponseTransformer.toFile` from deleting a pre-existing destination when a request fails before the SDK opens the file. Calls using `CREATE_NEW` now preserve the existing file and surface `FileAlreadyExistsException` instead of allowing a retry to overwrite it."
}
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ private AsynchronousFileChannel createChannel(Path path) throws IOException {

@Override
public CompletableFuture<ResponseT> prepare() {
fileChannel = null;
cf = new CompletableFuture<>();
cf.whenComplete((r, t) -> {
if (t != null && fileChannel != null) {
Expand Down Expand Up @@ -169,14 +170,16 @@ public void onStream(SdkPublisher<ByteBuffer> publisher) {

@Override
public void exceptionOccurred(Throwable throwable) {
AsynchronousFileChannel currentFileChannel = fileChannel;
fileChannel = null;
try {
if (fileChannel != null) {
if (currentFileChannel != null) {
runAndLogError(log.logger(),
String.format("Failed to close the file %s, resource may be leaked", path),
() -> fileChannel.close());
currentFileChannel::close);
}
} finally {
if (configuration.failureBehavior() == FailureBehavior.DELETE) {
if (currentFileChannel != null && configuration.failureBehavior() == FailureBehavior.DELETE) {
runAndLogError(log.logger(),
String.format("Failed to delete the file %s", path),
() -> Files.deleteIfExists(path));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ public void synchronousPublisher_shouldNotHang() throws Exception {
@Test
void noConfiguration_fileAlreadyExists_shouldThrowException() throws Exception {
Path testPath = testFs.getPath("test_file.txt");
Files.write(testPath, RandomStringUtils.random(1000).getBytes(StandardCharsets.UTF_8));
String existingContent = RandomStringUtils.randomAlphanumeric(1000);
Files.write(testPath, existingContent.getBytes(StandardCharsets.UTF_8));
assertThat(testPath).exists();

String content = RandomStringUtils.randomAlphanumeric(30000);
Expand All @@ -131,6 +132,7 @@ void noConfiguration_fileAlreadyExists_shouldThrowException() throws Exception {
transformer.onResponse("foobar");
transformer.onStream(testPublisher(content));
assertThatThrownBy(() -> future.join()).hasRootCauseInstanceOf(FileAlreadyExistsException.class);
assertThat(testPath).hasContent(existingContent);
}

@Test
Expand Down Expand Up @@ -186,6 +188,73 @@ void createOrAppendExisting_fileExists_shouldAppend() throws Exception {
assertThat(testPath).hasContent(existingString + content);
}

@ParameterizedTest
@MethodSource("deleteConfigurations")
void exceptionOccurred_beforeFileOpened_shouldPreserveExistingFile(FileTransformerConfiguration configuration)
throws Exception {
Path testPath = testFs.getPath("test_file.txt");
String existingContent = RandomStringUtils.randomAlphanumeric(1000);
Files.write(testPath, existingContent.getBytes(StandardCharsets.UTF_8));

FileAsyncResponseTransformer<String> transformer = new FileAsyncResponseTransformer<>(testPath, configuration);
CompletableFuture<String> future = transformer.prepare();
RuntimeException exception = new RuntimeException("oops");
transformer.exceptionOccurred(exception);

assertThat(future).failsWithin(1, TimeUnit.SECONDS)
.withThrowableOfType(ExecutionException.class)
.withCause(exception);
assertThat(testPath).hasContent(existingContent);
}

@Test
void exceptionOccurred_beforeFileOpenedOnRetry_shouldPreserveExistingFile() throws Exception {
Path testPath = testFs.getPath("test_file.txt");
FileAsyncResponseTransformer<String> transformer = new FileAsyncResponseTransformer<>(testPath);

stubException(transformer);
assertThat(testPath).doesNotExist();

String existingContent = RandomStringUtils.randomAlphanumeric(1000);
Files.write(testPath, existingContent.getBytes(StandardCharsets.UTF_8));
CompletableFuture<String> future = transformer.prepare();
RuntimeException exception = new RuntimeException("oops");
transformer.exceptionOccurred(exception);

assertThat(future).failsWithin(1, TimeUnit.SECONDS)
.withThrowableOfType(ExecutionException.class)
.withCause(exception);
assertThat(testPath).hasContent(existingContent);
}

@Test
void exceptionOccurred_afterFileOpened_shouldAllowRetry() throws Exception {
Path testPath = testFs.getPath("test_file.txt");
FileAsyncResponseTransformer<String> transformer = new FileAsyncResponseTransformer<>(testPath);

stubException(transformer);
assertThat(testPath).doesNotExist();

String content = RandomStringUtils.randomAlphanumeric(1000);
stubSuccessfulStreaming(content, transformer);
assertThat(testPath).hasContent(content);
}

@Test
void exceptionOccurred_calledTwice_shouldNotDeleteReplacement() throws Exception {
Path testPath = testFs.getPath("test_file.txt");
FileAsyncResponseTransformer<String> transformer = new FileAsyncResponseTransformer<>(testPath);

stubException(transformer);
assertThat(testPath).doesNotExist();

String replacementContent = RandomStringUtils.randomAlphanumeric(1000);
Files.write(testPath, replacementContent.getBytes(StandardCharsets.UTF_8));
transformer.exceptionOccurred(new RuntimeException("second callback"));

assertThat(testPath).hasContent(replacementContent);
}

@ParameterizedTest
@MethodSource("configurations")
void exceptionOccurred_deleteFileBehavior(FileTransformerConfiguration configuration) throws Exception {
Expand All @@ -203,6 +272,17 @@ void exceptionOccurred_deleteFileBehavior(FileTransformerConfiguration configura
}
}

private static List<FileTransformerConfiguration> deleteConfigurations() {
List<FileTransformerConfiguration> conf = new ArrayList<>();
for (FileWriteOption fileWriteOption : FileWriteOption.values()) {
conf.add(FileTransformerConfiguration.builder()
.fileWriteOption(fileWriteOption)
.failureBehavior(DELETE)
.build());
}
return conf;
}

private static List<FileTransformerConfiguration> configurations() {
List<FileTransformerConfiguration> conf = new ArrayList<>();
conf.add(FileTransformerConfiguration.defaultCreateNew());
Expand Down
Loading