Skip to content

Commit 3361c0f

Browse files
pulimsrpulimsr
committed
using existing tempFile object
1 parent 5fd263f commit 3361c0f

File tree

2 files changed

+27
-85
lines changed

2 files changed

+27
-85
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 & 70 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;
@@ -35,7 +29,6 @@ class MockS3Client : public S3Client {
3529
GetObjectResult result;
3630

3731
if (request.RangeHasBeenSet()) {
38-
// Always return mismatched range to trigger validation failure
3932
result.SetContentRange("bytes 1024-2047/2048");
4033
}
4134

@@ -48,7 +41,7 @@ class MockS3Client : public S3Client {
4841

4942
class MockMultipartS3Client : public S3Client {
5043
public:
51-
Aws::String FULL_OBJECT_CHECKSUM; //"SBi/K+1ooBg="
44+
Aws::String FULL_OBJECT_CHECKSUM;
5245
MockMultipartS3Client(Aws::String expected_checksum) : S3Client() {
5346
FULL_OBJECT_CHECKSUM = expected_checksum;
5447
};
@@ -57,8 +50,8 @@ class MockMultipartS3Client : public S3Client {
5750
HeadObjectResult result;
5851
result.SetContentLength(78643200);
5952
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
53+
result.SetChecksumType(Aws::S3::Model::ChecksumType::FULL_OBJECT);
54+
result.SetETag("\"test-etag-12345\"");
6255
return HeadObjectOutcome(std::move(result));
6356
}
6457

@@ -67,11 +60,6 @@ class MockMultipartS3Client : public S3Client {
6760

6861
const uint64_t totalSize = 78643200;
6962
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-
};
7563

7664
if (request.RangeHasBeenSet()) {
7765
auto range = request.GetRange();
@@ -83,33 +71,37 @@ class MockMultipartS3Client : public S3Client {
8371
int partNum = static_cast<int>(start) / partSize;
8472
if (partNum < 15) {
8573
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]);
8774
result.SetContentLength(size);
8875
result.SetETag(Aws::String("\"part-etag-") + Aws::String(std::to_string(partNum).c_str()) + "\"");
8976

90-
// Call the response stream factory if provided
9177
if (request.GetResponseStreamFactory()) {
9278
auto responseStream = request.GetResponseStreamFactory()();
9379

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);
80+
// Generate the same pattern as BigTest: "S3 MultiPart upload Test File " repeated
81+
const char* testString = "S3 MultiPart upload Test File ";
82+
const uint32_t testStrLen = static_cast<uint32_t>(strlen(testString));
83+
84+
for (uint64_t i = 0; i < size; i += testStrLen) {
85+
uint64_t writeLen = std::min(testStrLen, static_cast<uint32_t>(size - i));
86+
responseStream->write(testString, writeLen);
9887
}
9988
responseStream->flush();
10089

101-
// Simulate data received callback to track bytes transferred
10290
if (request.GetDataReceivedEventHandler()) {
10391
request.GetDataReceivedEventHandler()(nullptr, nullptr, size);
10492
}
10593

10694
result.ReplaceBody(responseStream);
10795
} else {
108-
// Fallback for non-factory requests
10996
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);
97+
98+
// Generate the same pattern as BigTest: "S3 MultiPart upload Test File " repeated
99+
const char* testString = "S3 MultiPart upload Test File ";
100+
const uint32_t testStrLen = static_cast<uint32_t>(strlen(testString));
101+
102+
for (uint64_t i = 0; i < size; i += testStrLen) {
103+
uint64_t writeLen = std::min(testStrLen, static_cast<uint32_t>(size - i));
104+
stream->write(testString, writeLen);
113105
}
114106
stream->seekg(0, std::ios::beg);
115107
result.ReplaceBody(stream);
@@ -126,7 +118,7 @@ class TransferUnitTest : public testing::Test {
126118
void SetUp() override {
127119
executor = Aws::MakeShared<PooledThreadExecutor>(ALLOCATION_TAG, 1);
128120
mockS3Client = Aws::MakeShared<MockS3Client>(ALLOCATION_TAG);
129-
mockMultipartS3Client = Aws::MakeShared<MockMultipartS3Client>(ALLOCATION_TAG, "SBi/K+1ooBg=");
121+
mockMultipartS3Client = Aws::MakeShared<MockMultipartS3Client>(ALLOCATION_TAG, "u5EvMU0xv5s=");
130122
}
131123

132124
static void SetUpTestSuite() {
@@ -167,35 +159,17 @@ TEST_F(TransferUnitTest, MultipartDownloadTest) {
167159
config.bufferSize = 5242880; // 5MB to ensure multipart
168160
auto transferManager = TransferManager::Create(config);
169161

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-
};
162+
Utils::TempFile tempFile(std::ios_base::out | std::ios_base::in | std::ios_base::binary | std::ios_base::trunc);
184163

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

189-
// Test multipart download functionality - should PASS with correct checksum
190167
EXPECT_TRUE(handle->IsMultipart());
191168
EXPECT_EQ(78643200u, handle->GetBytesTotalSize());
192169
EXPECT_EQ(15u, handle->GetCompletedParts().size());
193170
EXPECT_EQ(0u, handle->GetFailedParts().size());
194171
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());
172+
EXPECT_EQ(TransferStatus::COMPLETED, handle->GetStatus());
199173
}
200174

201175
TEST_F(TransferUnitTest, MultipartDownloadTest_Fail) {
@@ -205,34 +179,16 @@ TEST_F(TransferUnitTest, MultipartDownloadTest_Fail) {
205179
config.bufferSize = 5242880; // 5MB to ensure multipart
206180
auto transferManager = TransferManager::Create(config);
207181

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-
};
182+
Utils::TempFile tempFile{std::ios_base::out | std::ios_base::in | std::ios_base::binary | std::ios_base::trunc};
222183

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

227-
// Test multipart download functionality - should FAIL with wrong checksum
228187
EXPECT_TRUE(handle->IsMultipart());
229188
EXPECT_EQ(78643200u, handle->GetBytesTotalSize());
230189
EXPECT_EQ(15u, handle->GetCompletedParts().size());
231190
EXPECT_EQ(0u, handle->GetFailedParts().size());
232191
EXPECT_EQ(0u, handle->GetPendingParts().size());
233-
EXPECT_EQ(TransferStatus::FAILED, handle->GetStatus()); // Should FAIL due to wrong checksum
192+
EXPECT_EQ(TransferStatus::FAILED, handle->GetStatus());
234193
EXPECT_EQ("Full-object checksum validation failed", handle->GetLastError().GetMessage());
235-
236-
// Clean up
237-
std::remove(tempFile.c_str());
238194
}

0 commit comments

Comments
 (0)