Skip to content

Commit 99c14f7

Browse files
zoewanggzoewangg
committed
Add file length check when checking if file has modified or not
1 parent 709ab98 commit 99c14f7

File tree

4 files changed

+69
-32
lines changed

4 files changed

+69
-32
lines changed

‎services-custom/s3-transfer-manager/src/it/java/software/amazon/awssdk/transfer/s3/S3TransferManagerDownloadPauseResumeIntegrationTest.java

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import static software.amazon.awssdk.transfer.s3.SizeConstant.MB;
2121

2222
import java.io.File;
23-
import java.io.IOException;
2423
import java.nio.charset.StandardCharsets;
2524
import java.nio.file.Files;
2625
import java.nio.file.Path;
@@ -105,34 +104,41 @@ void pauseAndResume_ObjectNotChanged_shouldResumeDownload() {
105104

106105
@Test
107106
void pauseAndResume_objectChanged_shouldStartFromBeginning() {
108-
Path path = RandomTempFile.randomUncreatedFile().toPath();
109-
DownloadFileRequest request = DownloadFileRequest.builder()
110-
.getObjectRequest(b -> b.bucket(BUCKET).key(KEY))
111-
.destination(path)
112-
.build();
113-
FileDownload download = tm.downloadFile(request);
114-
waitUntilFirstByteBufferDelivered(download);
115-
116-
ResumableFileDownload resumableFileDownload = download.pause();
117-
log.debug(() -> "Paused: " + resumableFileDownload);
118-
String newObject = RandomStringUtils.randomAlphanumeric(1000);
119-
120-
// Re-upload the S3 object
121-
s3.putObject(PutObjectRequest.builder()
122-
.bucket(BUCKET)
123-
.key(KEY)
124-
.build(), RequestBody.fromString(newObject));
125-
126-
log.debug(() -> "Resuming download ");
127-
FileDownload resumedFileDownload = tm.resumeDownloadFile(resumableFileDownload);
128-
resumedFileDownload.progress().snapshot();
129-
resumedFileDownload.completionFuture().join();
130-
assertThat(path.toFile()).hasContent(newObject);
131-
assertThat(resumedFileDownload.progress().snapshot().transferSizeInBytes()).hasValue((long) newObject.getBytes(StandardCharsets.UTF_8).length);
107+
try {
108+
Path path = RandomTempFile.randomUncreatedFile().toPath();
109+
DownloadFileRequest request = DownloadFileRequest.builder()
110+
.getObjectRequest(b -> b.bucket(BUCKET).key(KEY))
111+
.destination(path)
112+
.build();
113+
FileDownload download = tm.downloadFile(request);
114+
waitUntilFirstByteBufferDelivered(download);
115+
116+
ResumableFileDownload resumableFileDownload = download.pause();
117+
log.debug(() -> "Paused: " + resumableFileDownload);
118+
String newObject = RandomStringUtils.randomAlphanumeric(1000);
119+
120+
// Re-upload the S3 object
121+
s3.putObject(PutObjectRequest.builder()
122+
.bucket(BUCKET)
123+
.key(KEY)
124+
.build(), RequestBody.fromString(newObject));
125+
126+
log.debug(() -> "Resuming download ");
127+
FileDownload resumedFileDownload = tm.resumeDownloadFile(resumableFileDownload);
128+
resumedFileDownload.progress().snapshot();
129+
resumedFileDownload.completionFuture().join();
130+
assertThat(path.toFile()).hasContent(newObject);
131+
assertThat(resumedFileDownload.progress().snapshot().transferSizeInBytes()).hasValue((long) newObject.getBytes(StandardCharsets.UTF_8).length);
132+
} finally {
133+
s3.putObject(PutObjectRequest.builder()
134+
.bucket(BUCKET)
135+
.key(KEY)
136+
.build(), sourceFile.toPath());
137+
}
132138
}
133139

134140
@Test
135-
void pauseAndResume_fileChanged_shouldStartFromBeginning() throws IOException {
141+
void pauseAndResume_fileChanged_shouldStartFromBeginning() throws Exception {
136142
Path path = RandomTempFile.randomUncreatedFile().toPath();
137143
DownloadFileRequest request = DownloadFileRequest.builder()
138144
.getObjectRequest(b -> b.bucket(BUCKET).key(KEY))

‎services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/utils/ResumableRequestConverter.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
package software.amazon.awssdk.transfer.s3.internal.utils;
1717

18+
import java.io.File;
1819
import java.time.Instant;
1920
import software.amazon.awssdk.annotations.SdkInternalApi;
2021
import software.amazon.awssdk.core.FileTransformerConfiguration;
@@ -53,7 +54,8 @@ private ResumableRequestConverter() {
5354
.toFile()
5455
.lastModified());
5556
boolean s3ObjectNotModified = headObjectResponse.lastModified().equals(lastModified);
56-
boolean fileNotModified = resumableFileDownload.fileLastModified().equals(fileLastModified);
57+
boolean fileNotModified = fileNotModified(resumableFileDownload, originalDownloadRequest.destination().toFile(),
58+
fileLastModified);
5759

5860
if (fileNotModified && s3ObjectNotModified) {
5961
newDownloadFileRequest = resumedDownloadFileRequest(resumableFileDownload,
@@ -73,6 +75,14 @@ private ResumableRequestConverter() {
7375
return Pair.of(newDownloadFileRequest, responseTransformer);
7476
}
7577

78+
private static boolean fileNotModified(ResumableFileDownload resumableFileDownload,
79+
File destination, Instant fileLastModified) {
80+
// On certain platforms, File.lastModified() does not contain milliseconds precision, so we need to check the
81+
// file length as well https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8177809
82+
return resumableFileDownload.fileLastModified().equals(fileLastModified)
83+
&& resumableFileDownload.bytesTransferred() == destination.length();
84+
}
85+
7686
private static AsyncResponseTransformer<GetObjectResponse, GetObjectResponse> fileAsyncResponseTransformer(
7787
DownloadFileRequest newDownloadFileRequest,
7888
boolean shouldAppend) {

‎services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/S3TransferManagerPauseAndResumeTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class S3TransferManagerPauseAndResumeTest {
5757
@BeforeEach
5858
public void methodSetup() throws IOException {
5959
file = RandomTempFile.createTempFile("test", UUID.randomUUID().toString());
60-
Files.write(file.toPath(), RandomStringUtils.random(2000).getBytes(StandardCharsets.UTF_8));
60+
Files.write(file.toPath(), RandomStringUtils.randomAlphanumeric(1000).getBytes(StandardCharsets.UTF_8));
6161
mockS3Crt = mock(S3CrtAsyncClient.class);
6262
uploadDirectoryHelper = mock(UploadDirectoryHelper.class);
6363
configuration = mock(TransferManagerConfiguration.class);
@@ -90,7 +90,7 @@ void resumeDownloadFile_shouldSetRangeAccordingly() {
9090
when(mockS3Crt.headObject(any(Consumer.class)))
9191
.thenReturn(CompletableFuture.completedFuture(headObjectResponse));
9292

93-
CompletedFileDownload completedFileDownload = tm.resumeDownloadFile(r -> r.bytesTransferred(1000l)
93+
CompletedFileDownload completedFileDownload = tm.resumeDownloadFile(r -> r.bytesTransferred(file.length())
9494
.downloadFileRequest(downloadFileRequest)
9595
.fileLastModified(fileLastModified)
9696
.s3ObjectLastModified(s3ObjectLastModified))

‎services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/util/ResumableRequestConverterTest.java

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class ResumableRequestConverterTest {
4545
@BeforeEach
4646
public void methodSetup() throws IOException {
4747
file = RandomTempFile.createTempFile("test", UUID.randomUUID().toString());
48-
Files.write(file.toPath(), RandomStringUtils.random(2000).getBytes(StandardCharsets.UTF_8));
48+
Files.write(file.toPath(), RandomStringUtils.randomAlphanumeric(1000).getBytes(StandardCharsets.UTF_8));
4949
}
5050

5151
@AfterEach
@@ -63,7 +63,7 @@ void toDownloadFileAndTransformer_notModified_shouldSetRangeAccordingly() {
6363
.build();
6464
Instant fileLastModified = Instant.ofEpochMilli(file.lastModified());
6565
ResumableFileDownload resumableFileDownload = ResumableFileDownload.builder()
66-
.bytesTransferred(1000L)
66+
.bytesTransferred(file.length())
6767
.s3ObjectLastModified(s3ObjectLastModified)
6868
.fileLastModified(fileLastModified)
6969
.downloadFileRequest(downloadFileRequest)
@@ -96,7 +96,7 @@ void toDownloadFileAndTransformer_s3ObjectModified_shouldStartFromBeginning() {
9696
}
9797

9898
@Test
99-
void toDownloadFileAndTransformer_fileModified_shouldStartFromBeginning() throws IOException {
99+
void toDownloadFileAndTransformer_fileLastModifiedTimeChanged_shouldStartFromBeginning() throws IOException {
100100
Instant s3ObjectLastModified = Instant.now();
101101
GetObjectRequest getObjectRequest = getObjectRequest();
102102
DownloadFileRequest downloadFileRequest = DownloadFileRequest.builder()
@@ -116,6 +116,27 @@ void toDownloadFileAndTransformer_fileModified_shouldStartFromBeginning() throws
116116
verifyActualGetObjectRequest(getObjectRequest, actual.left().getObjectRequest(), null);
117117
}
118118

119+
@Test
120+
void toDownloadFileAndTransformer_fileLengthChanged_shouldStartFromBeginning() {
121+
Instant s3ObjectLastModified = Instant.now();
122+
GetObjectRequest getObjectRequest = getObjectRequest();
123+
DownloadFileRequest downloadFileRequest = DownloadFileRequest.builder()
124+
.getObjectRequest(getObjectRequest)
125+
.destination(file)
126+
.build();
127+
Instant fileLastModified = Instant.now().minusSeconds(10);
128+
ResumableFileDownload resumableFileDownload = ResumableFileDownload.builder()
129+
.bytesTransferred(1100L)
130+
.s3ObjectLastModified(s3ObjectLastModified)
131+
.fileLastModified(fileLastModified)
132+
.downloadFileRequest(downloadFileRequest)
133+
.build();
134+
Pair<DownloadFileRequest, AsyncResponseTransformer<GetObjectResponse, GetObjectResponse>> actual =
135+
toDownloadFileRequestAndTransformer(resumableFileDownload, headObjectResponse(s3ObjectLastModified),
136+
downloadFileRequest);
137+
verifyActualGetObjectRequest(getObjectRequest, actual.left().getObjectRequest(), null);
138+
}
139+
119140
private static void verifyActualGetObjectRequest(GetObjectRequest originalRequest, GetObjectRequest actualRequest,
120141
String range) {
121142
assertThat(actualRequest.bucket()).isEqualTo(originalRequest.bucket());

0 commit comments

Comments
 (0)