Skip to content

Commit bef77d7

Browse files
pulimsrpulimsr
committed
Add temporary file handling methods and integration test for Transfer Manager durability compliance
1 parent 916a261 commit bef77d7

File tree

3 files changed

+108
-14
lines changed

3 files changed

+108
-14
lines changed

‎src/aws-cpp-sdk-transfer/include/aws/transfer/TransferManager.h‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,21 @@ namespace Aws
345345
*/
346346
Aws::String CombinePartChecksums(const std::shared_ptr<TransferHandle>& handle);
347347

348+
/**
349+
* Generates a temporary file name following SEP pattern: {finalFile}.s3tmp.{uniqueId}
350+
* @param finalFileName The final destination file name.
351+
* @return The temporary file name.
352+
*/
353+
Aws::String GenerateTempFileName(const Aws::String& finalFileName);
354+
355+
/**
356+
* Renames temporary file to final file after successful validation.
357+
* @param tempFile The temporary file path.
358+
* @param finalFile The final file path.
359+
* @return True if rename was successful, false otherwise.
360+
*/
361+
bool RenameTempFileToFinal(const Aws::String& tempFile, const Aws::String& finalFile);
362+
348363
static Aws::String DetermineFilePath(const Aws::String& directory, const Aws::String& prefix, const Aws::String& keyName);
349364

350365
Aws::Utils::ExclusiveOwnershipResourceManager<unsigned char*> m_bufferManager;

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,7 @@ namespace Aws
878878
getObjectOutcome.GetResult().GetBody().flush();
879879

880880
// Validate full object checksum if available
881+
bool checksumValid = true;
881882
if (!handle->GetChecksum().empty()) {
882883
const auto& getResult = getObjectOutcome.GetResult();
883884
Aws::String actualChecksum;
@@ -896,6 +897,7 @@ namespace Aws
896897
}
897898

898899
if (!actualChecksum.empty() && actualChecksum != handle->GetChecksum()) {
900+
checksumValid = false;
899901
Aws::Client::AWSError<Aws::S3::S3Errors> checksumError(
900902
Aws::S3::S3Errors::INTERNAL_FAILURE,
901903
"ChecksumMismatch",
@@ -913,6 +915,32 @@ namespace Aws
913915
}
914916
}
915917

918+
// Rename temp file to final file if using temporary file pattern
919+
if (!handle->GetTargetFilePath().empty() && checksumValid) {
920+
Aws::String tempFile = GenerateTempFileName(handle->GetTargetFilePath());
921+
if (handle->GetTargetFilePath().find(".s3tmp.") != Aws::String::npos) {
922+
// This is a temp file, rename to final
923+
Aws::String finalFile = handle->GetTargetFilePath();
924+
size_t pos = finalFile.find(".s3tmp.");
925+
if (pos != Aws::String::npos) {
926+
finalFile = finalFile.substr(0, pos);
927+
if (!RenameTempFileToFinal(handle->GetTargetFilePath(), finalFile)) {
928+
Aws::Client::AWSError<Aws::S3::S3Errors> renameError(
929+
Aws::S3::S3Errors::INTERNAL_FAILURE,
930+
"FileRenameFailure",
931+
"Failed to rename temporary file to final destination",
932+
false);
933+
handle->ChangePartToFailed(partState);
934+
handle->UpdateStatus(TransferStatus::FAILED);
935+
handle->SetError(renameError);
936+
TriggerErrorCallback(handle, renameError);
937+
TriggerTransferStatusUpdatedCallback(handle);
938+
return;
939+
}
940+
}
941+
}
942+
}
943+
916944
handle->UpdateStatus(TransferStatus::COMPLETED);
917945
}
918946
else
@@ -1224,11 +1252,13 @@ namespace Aws
12241252
outcome.GetResult().GetBody().flush();
12251253

12261254
// Validate full object checksum if available
1255+
bool checksumValid = true;
12271256
if (!handle->GetChecksum().empty()) {
12281257
// Combine part checksums to calculate full object checksum
12291258
Aws::String calculatedChecksum = CombinePartChecksums(handle);
12301259

12311260
if (!calculatedChecksum.empty() && calculatedChecksum != handle->GetChecksum()) {
1261+
checksumValid = false;
12321262
Aws::Client::AWSError<Aws::S3::S3Errors> checksumError(
12331263
Aws::S3::S3Errors::INTERNAL_FAILURE,
12341264
"ChecksumMismatch",
@@ -1245,6 +1275,30 @@ namespace Aws
12451275
}
12461276
}
12471277

1278+
// Rename temp file to final file if using temporary file pattern
1279+
if (!handle->GetTargetFilePath().empty() && checksumValid) {
1280+
if (handle->GetTargetFilePath().find(".s3tmp.") != Aws::String::npos) {
1281+
// This is a temp file, rename to final
1282+
Aws::String finalFile = handle->GetTargetFilePath();
1283+
size_t pos = finalFile.find(".s3tmp.");
1284+
if (pos != Aws::String::npos) {
1285+
finalFile = finalFile.substr(0, pos);
1286+
if (!RenameTempFileToFinal(handle->GetTargetFilePath(), finalFile)) {
1287+
Aws::Client::AWSError<Aws::S3::S3Errors> renameError(
1288+
Aws::S3::S3Errors::INTERNAL_FAILURE,
1289+
"FileRenameFailure",
1290+
"Failed to rename temporary file to final destination",
1291+
false);
1292+
handle->UpdateStatus(TransferStatus::FAILED);
1293+
handle->SetError(renameError);
1294+
TriggerErrorCallback(handle, renameError);
1295+
TriggerTransferStatusUpdatedCallback(handle);
1296+
return;
1297+
}
1298+
}
1299+
}
1300+
}
1301+
12481302
handle->UpdateStatus(TransferStatus::COMPLETED);
12491303
}
12501304
else
@@ -1640,5 +1694,30 @@ namespace Aws
16401694
ss << std::hex << combinedCrc;
16411695
return ss.str();
16421696
}
1697+
1698+
Aws::String TransferManager::GenerateTempFileName(const Aws::String& finalFileName) {
1699+
// Generate unique identifier (max 8 chars as per SEP)
1700+
Aws::String uuid = Aws::Utils::UUID::RandomUUID();
1701+
Aws::String uniqueId = uuid.length() > 8 ? uuid.substr(0, 8) : uuid;
1702+
return finalFileName + ".s3tmp." + uniqueId;
1703+
}
1704+
1705+
bool TransferManager::RenameTempFileToFinal(const Aws::String& tempFile, const Aws::String& finalFile) {
1706+
// Use atomic rename if supported by platform
1707+
if (std::rename(tempFile.c_str(), finalFile.c_str()) == 0) {
1708+
return true;
1709+
}
1710+
1711+
// Fallback strategy for platforms without atomic rename
1712+
if (Aws::FileSystem::RemoveFileIfExists(finalFile.c_str())) {
1713+
if (std::rename(tempFile.c_str(), finalFile.c_str()) == 0) {
1714+
return true;
1715+
}
1716+
}
1717+
1718+
// Clean up temp file on failure
1719+
Aws::FileSystem::RemoveFileIfExists(tempFile.c_str());
1720+
return false;
1721+
}
16431722
}
16441723
}

‎tests/aws-cpp-sdk-transfer-tests/TransferTests.cpp‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2328,41 +2328,41 @@ TEST_P(TransferTests, TransferManager_TestRelativePrefix)
23282328
}
23292329
}
23302330

2331-
// Test checksum validation during download
2332-
TEST_P(TransferTests, TransferManager_DownloadChecksumValidationTest)
2331+
TEST_P(TransferTests, TransferManager_DownloadWithChecksumValidationIntegrationTest)
23332332
{
23342333
const Aws::String RandomFileName = Aws::Utils::UUID::RandomUUID();
23352334
Aws::String testFilePath = MakeFilePath(RandomFileName.c_str());
2336-
ScopedTestFile testFile(testFilePath, SMALL_TEST_SIZE, testString);
2335+
ScopedTestFile testFile(testFilePath, MEDIUM_TEST_SIZE, testString);
23372336

23382337
TransferManagerConfiguration transferManagerConfig(m_executor.get());
23392338
transferManagerConfig.s3Client = m_s3Clients[GetParam()];
23402339
transferManagerConfig.bufferSize = MB5;
23412340
transferManagerConfig.transferBufferMaxHeapSize = MB5 * 10;
2341+
transferManagerConfig.checksumAlgorithm = Aws::S3::Model::ChecksumAlgorithm::CRC32;
23422342

23432343
auto transferManager = TransferManager::Create(transferManagerConfig);
23442344

2345-
// Upload file first
2346-
auto requestPtr = transferManager->UploadFile(testFilePath, GetTestBucketName(), RandomFileName,
2347-
"text/plain", Aws::Map<Aws::String, Aws::String>());
2348-
2349-
ASSERT_EQ(true, requestPtr->ShouldContinue());
2350-
ASSERT_EQ(TransferDirection::UPLOAD, requestPtr->GetTransferDirection());
2351-
requestPtr->WaitUntilFinished();
2352-
ASSERT_EQ(TransferStatus::COMPLETED, requestPtr->GetStatus());
2345+
// Upload file with checksum enabled
2346+
auto uploadPtr = transferManager->UploadFile(testFilePath, GetTestBucketName(), RandomFileName,
2347+
"text/plain", Aws::Map<Aws::String, Aws::String>());
23532348

2349+
uploadPtr->WaitUntilFinished();
2350+
ASSERT_EQ(TransferStatus::COMPLETED, uploadPtr->GetStatus());
23542351
ASSERT_TRUE(WaitForObjectToPropagate(GetTestBucketName(), RandomFileName.c_str()));
23552352

2356-
// Download file and verify checksum validation works
2353+
// Download file - should validate checksum successfully
23572354
Aws::String downloadFilePath = MakeFilePath((RandomFileName + "Download").c_str());
23582355
auto downloadPtr = transferManager->DownloadFile(GetTestBucketName(), RandomFileName, downloadFilePath);
23592356

2360-
ASSERT_EQ(true, downloadPtr->ShouldContinue());
2361-
ASSERT_EQ(TransferDirection::DOWNLOAD, downloadPtr->GetTransferDirection());
23622357
downloadPtr->WaitUntilFinished();
23632358

23642359
// Should complete successfully with valid checksum
23652360
ASSERT_EQ(TransferStatus::COMPLETED, downloadPtr->GetStatus());
2361+
2362+
// Verify the downloaded file exists by trying to open it
2363+
std::ifstream file(downloadFilePath.c_str());
2364+
ASSERT_TRUE(file.good());
2365+
file.close();
23662366
}
23672367

23682368
INSTANTIATE_TEST_SUITE_P(Https, TransferTests, testing::Values(TestType::Https));

0 commit comments

Comments
 (0)