Skip to content

Commit 52f177f

Browse files
v-pratapv-pratap
committed
Fix integration tests overlapping options, fix ReadObject using Upload option, remove unused Request parameter from ChecksumHelpers
1 parent eddcfdf commit 52f177f

7 files changed

Lines changed: 17 additions & 29 deletions

File tree

‎google/cloud/storage/internal/checksum_helpers.h‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,7 @@ struct HashDisabled {
3131
bool crc32c;
3232
};
3333

34-
template <typename Request>
35-
HashDisabled GetDownloadChecksumSettings(Request const&,
36-
Options const& options) {
34+
inline HashDisabled GetDownloadChecksumSettings(Options const& options) {
3735
bool disable_md5 = true;
3836
bool disable_crc32c = false;
3937
if (options.has<DownloadChecksumValidationOption>()) {
@@ -48,9 +46,7 @@ HashDisabled GetDownloadChecksumSettings(Request const&,
4846
return {disable_md5, disable_crc32c};
4947
}
5048

51-
template <typename Request>
52-
HashDisabled GetUploadChecksumSettings(Request const&,
53-
Options const& options) {
49+
inline HashDisabled GetUploadChecksumSettings(Options const& options) {
5450
bool disable_md5 = true;
5551
bool disable_crc32c = false;
5652
if (options.has<UploadChecksumValidationOption>()) {

‎google/cloud/storage/internal/grpc/stub.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ StatusOr<storage::ObjectMetadata> GrpcStub::InsertObjectMedia(
349349
auto stream = stub_->WriteObject(std::move(ctx), options);
350350

351351
auto const settings =
352-
storage::internal::GetUploadChecksumSettings(request, options);
352+
storage::internal::GetUploadChecksumSettings(options);
353353
auto hash_function = storage::internal::CreateHashFunction(
354354
request.GetOption<storage::Crc32cChecksumValue>(), settings.crc32c,
355355
request.GetOption<storage::MD5HashValue>(), settings.md5);

‎google/cloud/storage/internal/hash_function.cc‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,7 @@ std::unique_ptr<HashFunction> CreateHashFunction(
6464
ReadObjectRangeRequest const& request) {
6565
if (request.RequiresRangeHeader()) return CreateNullHashFunction();
6666

67-
auto const settings = GetDownloadChecksumSettings(
68-
request, google::cloud::internal::CurrentOptions());
67+
auto const settings = GetDownloadChecksumSettings(google::cloud::internal::CurrentOptions());
6968
auto const disable_md5 = settings.md5;
7069
auto const disable_crc32c = settings.crc32c;
7170
if (disable_md5 && disable_crc32c) {
@@ -85,8 +84,7 @@ std::unique_ptr<HashFunction> CreateHashFunction(
8584
return CreateNullHashFunction();
8685
}
8786

88-
auto const settings = GetUploadChecksumSettings(
89-
request, google::cloud::internal::CurrentOptions());
87+
auto const settings = GetUploadChecksumSettings(google::cloud::internal::CurrentOptions());
9088
return CreateHashFunction(request.GetOption<Crc32cChecksumValue>(),
9189
settings.crc32c, request.GetOption<MD5HashValue>(),
9290
settings.md5);

‎google/cloud/storage/internal/hash_validator.cc‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,15 @@ std::unique_ptr<HashValidator> CreateHashValidator(
5757
ReadObjectRangeRequest const& request) {
5858
if (request.RequiresRangeHeader()) return CreateNullHashValidator();
5959

60-
auto const settings = GetDownloadChecksumSettings(
61-
request, google::cloud::internal::CurrentOptions());
60+
auto const settings = GetDownloadChecksumSettings(google::cloud::internal::CurrentOptions());
6261
auto const disable_md5 = settings.md5;
6362
auto const disable_crc32c = settings.crc32c;
6463
return CreateHashValidator(disable_md5, disable_crc32c);
6564
}
6665

6766
std::unique_ptr<HashValidator> CreateHashValidator(
6867
ResumableUploadRequest const& request) {
69-
auto const settings = GetUploadChecksumSettings(
70-
request, google::cloud::internal::CurrentOptions());
68+
auto const settings = GetUploadChecksumSettings(google::cloud::internal::CurrentOptions());
7169
auto const disable_md5 = settings.md5;
7270
auto const disable_crc32c = settings.crc32c;
7371
return CreateHashValidator(disable_md5, disable_crc32c);

‎google/cloud/storage/internal/rest/stub.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ StatusOr<ObjectMetadata> RestStub::InsertObjectMediaMultipart(
381381
}
382382

383383
auto const settings =
384-
storage::internal::GetUploadChecksumSettings(request, options);
384+
storage::internal::GetUploadChecksumSettings(options);
385385
auto hash_function = storage::internal::CreateHashFunction(
386386
request.GetOption<storage::Crc32cChecksumValue>(), settings.crc32c,
387387
request.GetOption<storage::MD5HashValue>(), settings.md5);
@@ -459,7 +459,7 @@ StatusOr<ObjectMetadata> RestStub::InsertObjectMedia(
459459
// computing the MD5 hash or CRC32C checksum, we need to use multipart
460460
// uploads.
461461
auto const settings =
462-
storage::internal::GetUploadChecksumSettings(request, options);
462+
storage::internal::GetUploadChecksumSettings(options);
463463
if (!settings.md5 || !settings.crc32c || request.HasOption<MD5HashValue>() ||
464464
request.HasOption<Crc32cChecksumValue>()) {
465465
return InsertObjectMediaMultipart(context, options, request);

‎google/cloud/storage/tests/object_checksum_integration_test.cc‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,7 @@ TEST_F(ObjectChecksumIntegrationTest, WriteObjectExplicitDisable) {
156156
auto object_name = MakeRandomObjectName();
157157

158158
auto os =
159-
client.WriteObject(bucket_name_, object_name, Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kMD5),
160-
Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kCrc32c), IfGenerationMatch(0));
159+
client.WriteObject(bucket_name_, object_name, Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kNone), IfGenerationMatch(0));
161160
os << LoremIpsum();
162161
os.Close();
163162
auto meta = os.metadata();
@@ -180,8 +179,7 @@ TEST_F(ObjectChecksumIntegrationTest, WriteObjectExplicitEnable) {
180179
auto client = MakeIntegrationTestClient();
181180
auto object_name = MakeRandomObjectName();
182181
auto os = client.WriteObject(bucket_name_, object_name,
183-
Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kCrc32cAndMD5),
184-
Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kCrc32c), IfGenerationMatch(0));
182+
Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kCrc32cAndMD5), IfGenerationMatch(0));
185183
os << LoremIpsum();
186184
os.Close();
187185
auto meta = os.metadata();
@@ -284,8 +282,7 @@ TEST_F(ObjectChecksumIntegrationTest, WriteObjectWithFullChecksumValidation) {
284282
auto expected_crc32c = ComputeCrc32cChecksum(content);
285283

286284
auto os = client.WriteObject(bucket_name_, object_name,
287-
Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kCrc32cAndMD5),
288-
Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kCrc32c), IfGenerationMatch(0));
285+
Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kCrc32cAndMD5), IfGenerationMatch(0));
289286
os << content;
290287
os.Close();
291288
auto meta = os.metadata();
@@ -361,7 +358,7 @@ TEST_F(ObjectChecksumIntegrationTest, ReadObjectCorruptedByServerGetc) {
361358
ScheduleForDelete(*meta);
362359

363360
auto stream = client.ReadObject(
364-
bucket_name_, object_name, Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kCrc32c),
361+
bucket_name_, object_name, Options{}.set<DownloadChecksumValidationOption>(ChecksumAlgorithm::kCrc32c),
365362
CustomHeader("x-goog-emulator-instructions", "return-corrupted-data"));
366363

367364
#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
@@ -401,7 +398,7 @@ TEST_F(ObjectChecksumIntegrationTest, ReadObjectCorruptedByServerRead) {
401398
ScheduleForDelete(*meta);
402399

403400
auto stream = client.ReadObject(
404-
bucket_name_, object_name, Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kCrc32c),
401+
bucket_name_, object_name, Options{}.set<DownloadChecksumValidationOption>(ChecksumAlgorithm::kCrc32c),
405402
CustomHeader("x-goog-emulator-instructions", "return-corrupted-data"));
406403

407404
// Create a buffer large enough to read the full contents.

‎google/cloud/storage/tests/object_hash_integration_test.cc‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,7 @@ TEST_F(ObjectHashIntegrationTest, InsertObjectExplicitDisable) {
7575
auto object_name = MakeRandomObjectName();
7676

7777
auto meta = client.InsertObject(
78-
bucket_name_, object_name, LoremIpsum(), Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kCrc32c),
79-
Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kMD5), IfGenerationMatch(0));
78+
bucket_name_, object_name, LoremIpsum(), Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kCrc32cAndMD5), IfGenerationMatch(0));
8079
ASSERT_STATUS_OK(meta);
8180
ScheduleForDelete(*meta);
8281

@@ -309,7 +308,7 @@ TEST_F(ObjectHashIntegrationTest, ReadObjectCorruptedByServerGetc) {
309308
ScheduleForDelete(*meta);
310309

311310
auto stream = client.ReadObject(
312-
bucket_name_, object_name, Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kMD5),
311+
bucket_name_, object_name, Options{}.set<DownloadChecksumValidationOption>(ChecksumAlgorithm::kMD5),
313312
CustomHeader("x-goog-emulator-instructions", "return-corrupted-data"));
314313

315314
#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
@@ -348,7 +347,7 @@ TEST_F(ObjectHashIntegrationTest, ReadObjectCorruptedByServerRead) {
348347
ScheduleForDelete(*meta);
349348

350349
auto stream = client.ReadObject(
351-
bucket_name_, object_name, Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kMD5),
350+
bucket_name_, object_name, Options{}.set<DownloadChecksumValidationOption>(ChecksumAlgorithm::kMD5),
352351
CustomHeader("x-goog-emulator-instructions", "return-corrupted-data"));
353352

354353
// Create a buffer large enough to read the full contents.

0 commit comments

Comments
 (0)