Skip to content

Commit 8f8a410

Browse files
pulimsrpulimsr
committed
using existing tempFile object
1 parent 5fd263f commit 8f8a410

File tree

2 files changed

+27
-84
lines changed

2 files changed

+27
-84
lines changed

‎src/aws-cpp-sdk-transfer/source/transfer/TransferManager.cpp‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,7 +1099,6 @@ namespace Aws
10991099
getObjectRangeRequest.SetRange(FormatRangeSpecifier(rangeStart, rangeEnd));
11001100
getObjectRangeRequest.SetResponseStreamFactory(responseStreamFunction);
11011101
getObjectRangeRequest.SetIfMatch(handle->GetEtag());
1102-
getObjectRangeRequest.SetChecksumMode(Aws::S3::Model::ChecksumMode::ENABLED);
11031102
if(handle->GetVersionId().size() > 0)
11041103
{
11051104
getObjectRangeRequest.SetVersionId(handle->GetVersionId());
@@ -1222,19 +1221,7 @@ namespace Aws
12221221

12231222
Aws::String errMsg{handle->WritePartToDownloadStream(bufferStream, partState->GetRangeBegin())};
12241223
if (errMsg.empty()) {
1225-
if (!outcome.GetResult().GetChecksumCRC32().empty()) {
1226-
partState->SetChecksum(outcome.GetResult().GetChecksumCRC32());
1227-
} else if (!outcome.GetResult().GetChecksumCRC32C().empty()) {
1228-
partState->SetChecksum(outcome.GetResult().GetChecksumCRC32C());
1229-
} else if (!outcome.GetResult().GetChecksumCRC64NVME().empty()) {
1230-
partState->SetChecksum(outcome.GetResult().GetChecksumCRC64NVME());
1231-
} else if (!outcome.GetResult().GetChecksumSHA1().empty()) {
1232-
partState->SetChecksum(outcome.GetResult().GetChecksumSHA1());
1233-
} else if (!outcome.GetResult().GetChecksumSHA256().empty()) {
1234-
partState->SetChecksum(outcome.GetResult().GetChecksumSHA256());
1235-
} else {
1236-
if (m_transferConfig.validateChecksums) { handle->AddChecksumForPart(bufferStream, partState); }
1237-
}
1224+
if (m_transferConfig.validateChecksums) { handle->AddChecksumForPart(bufferStream, partState); }
12381225
handle->ChangePartToCompleted(partState, outcome.GetResult().GetETag());
12391226
} else {
12401227
Aws::Client::AWSError<Aws::S3::S3Errors> error(Aws::S3::S3Errors::INTERNAL_FAILURE,
@@ -1323,7 +1310,6 @@ namespace Aws
13231310
std::memcpy(checksumBuffer.GetUnderlyingData(), &be, sizeof(uint32_t));
13241311
}
13251312
Aws::String combinedChecksumStr = Aws::Utils::HashingUtils::Base64Encode(checksumBuffer);
1326-
13271313
if (combinedChecksumStr != handle->GetChecksum()) {
13281314
AWS_LOGSTREAM_ERROR(CLASS_TAG, "Transfer handle [" << handle->GetId()
13291315
<< "] Full-object checksum mismatch. Expected: " << handle->GetChecksum()

‎tests/aws-cpp-sdk-transfer-unit-tests/TransferUnitTests.cpp‎

Lines changed: 26 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,3 @@
1-
#ifdef _WIN32
2-
#ifndef NOMINMAX
3-
#define NOMINMAX
4-
#endif
5-
#include <windows.h>
6-
#endif
7-
81
#include <gtest/gtest.h>
92
#include <aws/core/Aws.h>
103
#include <aws/core/utils/threading/PooledThreadExecutor.h>
@@ -18,6 +11,7 @@
1811
#include <aws/testing/MemoryTesting.h>
1912
#include <sstream>
2013
#include <fstream>
14+
#include <aws/core/utils/FileSystemUtils.h>
2115

2216
using namespace Aws;
2317
using namespace Aws::S3;
@@ -48,7 +42,7 @@ class MockS3Client : public S3Client {
4842

4943
class MockMultipartS3Client : public S3Client {
5044
public:
51-
Aws::String FULL_OBJECT_CHECKSUM; //"SBi/K+1ooBg="
45+
Aws::String FULL_OBJECT_CHECKSUM;
5246
MockMultipartS3Client(Aws::String expected_checksum) : S3Client() {
5347
FULL_OBJECT_CHECKSUM = expected_checksum;
5448
};
@@ -57,8 +51,8 @@ class MockMultipartS3Client : public S3Client {
5751
HeadObjectResult result;
5852
result.SetContentLength(78643200);
5953
result.SetChecksumCRC64NVME(FULL_OBJECT_CHECKSUM);
60-
result.SetChecksumType(Aws::S3::Model::ChecksumType::FULL_OBJECT); // This is key!
61-
result.SetETag("\"test-etag-12345\""); // Add ETag
54+
result.SetChecksumType(Aws::S3::Model::ChecksumType::FULL_OBJECT);
55+
result.SetETag("\"test-etag-12345\"");
6256
return HeadObjectOutcome(std::move(result));
6357
}
6458

@@ -67,11 +61,6 @@ class MockMultipartS3Client : public S3Client {
6761

6862
const uint64_t totalSize = 78643200;
6963
const uint64_t partSize = 5242880;
70-
const std::vector<Aws::String> checksums = {
71-
"wAQOkgd/LJk=", "zfmsUj6AZfs=", "oyENjcGDHcY=", "wAQOkgd/LJk=", "zfmsUj6AZfs=",
72-
"oyENjcGDHcY=", "wAQOkgd/LJk=", "zfmsUj6AZfs=", "oyENjcGDHcY=", "wAQOkgd/LJk=",
73-
"zfmsUj6AZfs=", "oyENjcGDHcY=", "wAQOkgd/LJk=", "zfmsUj6AZfs=", "oyENjcGDHcY="
74-
};
7564

7665
if (request.RangeHasBeenSet()) {
7766
auto range = request.GetRange();
@@ -83,33 +72,37 @@ class MockMultipartS3Client : public S3Client {
8372
int partNum = static_cast<int>(start) / partSize;
8473
if (partNum < 15) {
8574
result.SetContentRange(Aws::String("bytes ") + Aws::String(std::to_string(start).c_str()) + "-" + Aws::String(std::to_string(end).c_str()) + "/" + Aws::String(std::to_string(totalSize).c_str()));
86-
result.SetChecksumCRC64NVME(checksums[partNum]);
8775
result.SetContentLength(size);
8876
result.SetETag(Aws::String("\"part-etag-") + Aws::String(std::to_string(partNum).c_str()) + "\"");
8977

90-
// Call the response stream factory if provided
9178
if (request.GetResponseStreamFactory()) {
9279
auto responseStream = request.GetResponseStreamFactory()();
9380

94-
// Write part-specific data to the response stream
95-
char partChar = 'A' + (partNum % 3);
96-
for (uint64_t i = 0; i < size; ++i) {
97-
responseStream->put(partChar);
81+
// Generate the same pattern as BigTest: "S3 MultiPart upload Test File " repeated
82+
const char* testString = "S3 MultiPart upload Test File ";
83+
const uint32_t testStrLen = static_cast<uint32_t>(strlen(testString));
84+
85+
for (uint64_t i = 0; i < size; i += testStrLen) {
86+
uint64_t writeLen = std::min(testStrLen, static_cast<uint32_t>(size - i));
87+
responseStream->write(testString, writeLen);
9888
}
9989
responseStream->flush();
10090

101-
// Simulate data received callback to track bytes transferred
10291
if (request.GetDataReceivedEventHandler()) {
10392
request.GetDataReceivedEventHandler()(nullptr, nullptr, size);
10493
}
10594

10695
result.ReplaceBody(responseStream);
10796
} else {
108-
// Fallback for non-factory requests
10997
auto stream = Aws::New<std::stringstream>(ALLOCATION_TAG);
110-
char partChar = 'A' + (partNum % 3);
111-
for (uint64_t i = 0; i < size; ++i) {
112-
stream->put(partChar);
98+
99+
// Generate the same pattern as BigTest: "S3 MultiPart upload Test File " repeated
100+
const char* testString = "S3 MultiPart upload Test File ";
101+
const uint32_t testStrLen = static_cast<uint32_t>(strlen(testString));
102+
103+
for (uint64_t i = 0; i < size; i += testStrLen) {
104+
uint64_t writeLen = std::min(testStrLen, static_cast<uint32_t>(size - i));
105+
stream->write(testString, writeLen);
113106
}
114107
stream->seekg(0, std::ios::beg);
115108
result.ReplaceBody(stream);
@@ -126,7 +119,7 @@ class TransferUnitTest : public testing::Test {
126119
void SetUp() override {
127120
executor = Aws::MakeShared<PooledThreadExecutor>(ALLOCATION_TAG, 1);
128121
mockS3Client = Aws::MakeShared<MockS3Client>(ALLOCATION_TAG);
129-
mockMultipartS3Client = Aws::MakeShared<MockMultipartS3Client>(ALLOCATION_TAG, "SBi/K+1ooBg=");
122+
mockMultipartS3Client = Aws::MakeShared<MockMultipartS3Client>(ALLOCATION_TAG, "u5EvMU0xv5s=");
130123
}
131124

132125
static void SetUpTestSuite() {
@@ -167,35 +160,17 @@ TEST_F(TransferUnitTest, MultipartDownloadTest) {
167160
config.bufferSize = 5242880; // 5MB to ensure multipart
168161
auto transferManager = TransferManager::Create(config);
169162

170-
// Create a temporary file for download since multipart needs seekable stream
171-
std::string tempFile;
172-
#ifdef _WIN32
173-
char tempPath[MAX_PATH];
174-
GetTempPathA(MAX_PATH, tempPath);
175-
tempFile = std::string(tempPath) + "test_download_" + std::to_string(rand());
176-
#else
177-
tempFile = "/tmp/test_download_" + std::to_string(rand());
178-
#endif
179-
auto createStreamFn = [tempFile]() -> Aws::IOStream* {
180-
return Aws::New<Aws::FStream>(ALLOCATION_TAG, tempFile.c_str(),
181-
std::ios_base::out | std::ios_base::in |
182-
std::ios_base::binary | std::ios_base::trunc);
183-
};
163+
Utils::TempFile tempFile(std::ios_base::out | std::ios_base::in | std::ios_base::binary | std::ios_base::trunc);
184164

185-
// Download the full 78MB file
186-
auto handle = transferManager->DownloadFile("test-bucket", "test-key", createStreamFn);
165+
auto handle = transferManager->DownloadFile("test-bucket", "test-key", tempFile.GetFileName());
187166
handle->WaitUntilFinished();
188167

189-
// Test multipart download functionality - should PASS with correct checksum
190168
EXPECT_TRUE(handle->IsMultipart());
191169
EXPECT_EQ(78643200u, handle->GetBytesTotalSize());
192170
EXPECT_EQ(15u, handle->GetCompletedParts().size());
193171
EXPECT_EQ(0u, handle->GetFailedParts().size());
194172
EXPECT_EQ(0u, handle->GetPendingParts().size());
195-
EXPECT_EQ(TransferStatus::COMPLETED, handle->GetStatus()); // Should PASS
196-
197-
// Clean up
198-
std::remove(tempFile.c_str());
173+
EXPECT_EQ(TransferStatus::COMPLETED, handle->GetStatus());
199174
}
200175

201176
TEST_F(TransferUnitTest, MultipartDownloadTest_Fail) {
@@ -205,34 +180,16 @@ TEST_F(TransferUnitTest, MultipartDownloadTest_Fail) {
205180
config.bufferSize = 5242880; // 5MB to ensure multipart
206181
auto transferManager = TransferManager::Create(config);
207182

208-
// Create a temporary file for download since multipart needs seekable stream
209-
std::string tempFile;
210-
#ifdef _WIN32
211-
char tempPath[MAX_PATH];
212-
GetTempPathA(MAX_PATH, tempPath);
213-
tempFile = std::string(tempPath) + "test_download_" + std::to_string(rand());
214-
#else
215-
tempFile = "/tmp/test_download_" + std::to_string(rand());
216-
#endif
217-
auto createStreamFn = [tempFile]() -> Aws::IOStream* {
218-
return Aws::New<Aws::FStream>(ALLOCATION_TAG, tempFile.c_str(),
219-
std::ios_base::out | std::ios_base::in |
220-
std::ios_base::binary | std::ios_base::trunc);
221-
};
183+
Utils::TempFile tempFile{std::ios_base::out | std::ios_base::in | std::ios_base::binary | std::ios_base::trunc};
222184

223-
// Download the full 78MB file
224-
auto handle = transferManager->DownloadFile("test-bucket", "test-key", createStreamFn);
185+
auto handle = transferManager->DownloadFile("test-bucket", "test-key", tempFile.GetFileName());
225186
handle->WaitUntilFinished();
226187

227-
// Test multipart download functionality - should FAIL with wrong checksum
228188
EXPECT_TRUE(handle->IsMultipart());
229189
EXPECT_EQ(78643200u, handle->GetBytesTotalSize());
230190
EXPECT_EQ(15u, handle->GetCompletedParts().size());
231191
EXPECT_EQ(0u, handle->GetFailedParts().size());
232192
EXPECT_EQ(0u, handle->GetPendingParts().size());
233-
EXPECT_EQ(TransferStatus::FAILED, handle->GetStatus()); // Should FAIL due to wrong checksum
193+
EXPECT_EQ(TransferStatus::FAILED, handle->GetStatus());
234194
EXPECT_EQ("Full-object checksum validation failed", handle->GetLastError().GetMessage());
235-
236-
// Clean up
237-
std::remove(tempFile.c_str());
238195
}

0 commit comments

Comments
 (0)