feat(storage): migrate sync client to unified checksum options

google/cloud/storage/benchmarks/throughput_experiment.cc

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
 #include "google/cloud/storage/benchmarks/throughput_experiment.h"
 #include "google/cloud/storage/benchmarks/benchmark_utils.h"
 #include "google/cloud/storage/client.h"
@@ -477,3 +478,4 @@ std::vector<std::unique_ptr<ThroughputExperiment>> CreateDownloadExperiments(
 }  // namespace storage_benchmarks
 }  // namespace cloud
 }  // namespace google
+#include "google/cloud/internal/diagnostics_pop.inc"

google/cloud/storage/client.cc

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
 #include "google/cloud/storage/client.h"
 #include "google/cloud/storage/idempotency_policy.h"
 #include "google/cloud/storage/internal/base64.h"
@@ -699,3 +700,5 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace storage
 }  // namespace cloud
 }  // namespace google
+
+#include "google/cloud/internal/diagnostics_pop.inc"

google/cloud/storage/client_object_test.cc

@@ -12,7 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
 #include "google/cloud/storage/client.h"
+#include "google/cloud/storage/internal/checksum_helpers.h"
 #include "google/cloud/storage/internal/object_metadata_parser.h"
 #include "google/cloud/storage/retry_policy.h"
 #include "google/cloud/storage/testing/canonical_errors.h"
@@ -91,6 +93,32 @@ TEST_F(ObjectTest, InsertObjectMedia) {
   EXPECT_EQ(expected, *actual);
 }
 
+TEST_F(ObjectTest, InsertObjectUploadChecksumMD5) {
+  std::string text = R"""({
+      "name": "test-bucket-name/test-object-name/1"
+})""";
+  auto expected =
+      storage::internal::ObjectMetadataParser::FromString(text).value();
+
+  EXPECT_CALL(*mock_, InsertObjectMedia)
+      .WillOnce([&expected](internal::InsertObjectMediaRequest const& request) {
+        EXPECT_EQ("test-bucket-name", request.bucket_name());
+        EXPECT_EQ("test-object-name", request.object_name());
+        EXPECT_EQ("test object contents", request.payload());
+        EXPECT_TRUE(CurrentOptions().has<UploadChecksumValidationOption>());
+        EXPECT_EQ(CurrentOptions().get<UploadChecksumValidationOption>(),
+                  ChecksumAlgorithm::kMD5);
+        return make_status_or(expected);
+      });
+
+  auto client = ClientForMock();
+  auto actual = client.InsertObject(
+      "test-bucket-name", "test-object-name", "test object contents",
+      Options{}.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kMD5));
+  ASSERT_STATUS_OK(actual);
+  EXPECT_EQ(expected, *actual);
+}
+
 TEST_F(ObjectTest, GetObjectMetadata) {
   std::string text = R"""({
       "bucket": "test-bucket-name",
@@ -206,6 +234,65 @@ TEST_F(ObjectTest, ReadObject) {
   EXPECT_EQ(actual.gcount(), 1024);
 }
 
+TEST_F(ObjectTest, ReadObjectChecksumPrecedence) {
+  EXPECT_CALL(*mock_, ReadObject)
+      .WillOnce([](internal::ReadObjectRangeRequest const& r) {
+        EXPECT_TRUE(r.HasOption<DisableMD5Hash>());
+        EXPECT_FALSE(r.GetOption<DisableMD5Hash>().value());
+
+        auto settings =
+            internal::GetDownloadChecksumSettings(r, CurrentOptions());
+        // Verify MD5 is enabled (disable_md5 = false) and CRC32C is disabled
+        // (disable_crc32c = true)
+        EXPECT_FALSE(settings.md5);
+        EXPECT_TRUE(settings.crc32c);
+
+        auto read_source = std::make_unique<testing::MockObjectReadSource>();
+        EXPECT_CALL(*read_source, IsOpen()).WillRepeatedly(Return(true));
+        EXPECT_CALL(*read_source, Read)
+            .WillOnce(Return(internal::ReadSourceResult{1024, {}}));
+        EXPECT_CALL(*read_source, Close).Times(1);
+        return StatusOr<std::unique_ptr<internal::ObjectReadSource>>(
+            std::move(read_source));
+      });
+  auto client = ClientForMock();
+  auto actual = client.ReadObject(
+      "test-bucket-name", "test-object-name", DisableMD5Hash(false),
+      Options{}.set<DownloadChecksumValidationOption>(
+          ChecksumAlgorithm::kNone));
+  ASSERT_STATUS_OK(actual.status());
+  std::vector<char> v(1024);
+  actual.read(v.data(), v.size());
+  EXPECT_EQ(actual.gcount(), 1024);
+}
+
+TEST_F(ObjectTest, InsertObjectChecksumPrecedence) {
+  EXPECT_CALL(*mock_, InsertObjectMedia)
+      .WillOnce([](internal::InsertObjectMediaRequest const& r) {
+        EXPECT_TRUE(r.HasOption<DisableCrc32cChecksum>());
+        EXPECT_TRUE(r.GetOption<DisableCrc32cChecksum>().value());
+
+        auto settings =
+            internal::GetUploadChecksumSettings(r, CurrentOptions());
+        // Verify CRC32C is disabled (disable_crc32c = true) and MD5 remains
+        // enabled (disable_md5 = false)
+        EXPECT_TRUE(settings.crc32c);
+        EXPECT_FALSE(settings.md5);
+
+        return make_status_or(
+            storage::internal::ObjectMetadataParser::FromString(
+                R"({"name": "test-object-name"})")
+                .value());
+      });
+  auto client = ClientForMock();
+  auto actual =
+      client.InsertObject("test-bucket-name", "test-object-name", "payload",
+                          DisableCrc32cChecksum(true),
+                          Options{}.set<UploadChecksumValidationOption>(
+                              ChecksumAlgorithm::kCrc32cAndMD5));
+  ASSERT_STATUS_OK(actual);
+}
+
 TEST_F(ObjectTest, WriteObject) {
   EXPECT_CALL(*mock_, CreateResumableUpload)
       .WillOnce(Return(TransientError()))
@@ -483,3 +570,5 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace storage
 }  // namespace cloud
 }  // namespace google
+
+#include "google/cloud/internal/diagnostics_pop.inc"

google/cloud/storage/google_cloud_cpp_storage.bzl

@@ -51,6 +51,7 @@ google_cloud_cpp_storage_hdrs = [
     "internal/bucket_acl_requests.h",
     "internal/bucket_metadata_parser.h",
     "internal/bucket_requests.h",
+    "internal/checksum_helpers.h",
     "internal/complex_option.h",
     "internal/compute_engine_util.h",
     "internal/connection_factory.h",

google/cloud/storage/google_cloud_cpp_storage.cmake

@@ -78,6 +78,7 @@ add_library(
     internal/bucket_metadata_parser.h
     internal/bucket_requests.cc
     internal/bucket_requests.h
+    internal/checksum_helpers.h
     internal/complex_option.h
     internal/compute_engine_util.cc
     internal/compute_engine_util.h

google/cloud/storage/hashing_options_test.cc

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
 #include "google/cloud/storage/hashing_options.h"
 #include "google/cloud/storage/options.h"
 #include "google/cloud/options.h"
@@ -78,3 +79,4 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace storage
 }  // namespace cloud
 }  // namespace google
+#include "google/cloud/internal/diagnostics_pop.inc"

google/cloud/storage/internal/checksum_helpers.h

@@ -0,0 +1,107 @@
+// Copyright 2024 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_CHECKSUM_HELPERS_H
+#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_CHECKSUM_HELPERS_H
+
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
+#include "google/cloud/storage/hashing_options.h"
+#include "google/cloud/storage/options.h"
+#include "google/cloud/options.h"
+
+namespace google {
+namespace cloud {
+namespace storage {
+GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
+namespace internal {
+
+struct HashDisabled {
+  bool md5;
+  bool crc32c;
+};
+
+template <typename Request>
+HashDisabled GetDownloadChecksumSettings(Request const& request,
+                                         Options const& options) {
+  bool disable_md5 = false;
+  bool disable_crc32c = false;
+  bool use_new_algo = false;
+  if (options.has<DownloadChecksumValidationOption>()) {
+    auto const algo = options.get<DownloadChecksumValidationOption>();
+    disable_md5 = (algo != ChecksumAlgorithm::kMD5 &&
+                   algo != ChecksumAlgorithm::kCrc32cAndMD5);
+    disable_crc32c = (algo != ChecksumAlgorithm::kCrc32c &&
+                      algo != ChecksumAlgorithm::kCrc32cAndMD5);
+    use_new_algo = true;
+  }
+
+  auto const md5 = request.template GetOption<DisableMD5Hash>();
+  if (md5.has_value()) {
+    // DisableMD5Hash defaults to true. We only override the new option if the
+    // legacy option was explicitly set to false, or if the new option was not
+    // provided at all.
+    if (!use_new_algo || md5.value() != true) {
+      disable_md5 = md5.value();
+    }
+  }
+  auto const crc32c = request.template GetOption<DisableCrc32cChecksum>();
+  if (crc32c.has_value()) {
+    // DisableCrc32cChecksum defaults to std::nullopt, so if it has a value, it
+    // was explicitly set.
+    disable_crc32c = crc32c.value();
+  }
+  return {disable_md5, disable_crc32c};
+}
+
+template <typename Request>
+HashDisabled GetUploadChecksumSettings(Request const& request,
+                                       Options const& options) {
+  bool disable_md5 = false;
+  bool disable_crc32c = false;
+  bool use_new_algo = false;
+  if (options.has<UploadChecksumValidationOption>()) {
+    auto const algo = options.get<UploadChecksumValidationOption>();
+    disable_md5 = (algo != ChecksumAlgorithm::kMD5 &&
+                   algo != ChecksumAlgorithm::kCrc32cAndMD5);
+    disable_crc32c = (algo != ChecksumAlgorithm::kCrc32c &&
+                      algo != ChecksumAlgorithm::kCrc32cAndMD5);
+    use_new_algo = true;
+  }
+
+  auto const md5 = request.template GetOption<DisableMD5Hash>();
+  if (md5.has_value()) {
+    // DisableMD5Hash defaults to true. We only override the new option if the
+    // legacy option was explicitly set to false, or if the new option was not
+    // provided at all.
+    if (!use_new_algo || md5.value() != true) {
+      disable_md5 = md5.value();
+    }
+  }
+  auto const crc32c = request.template GetOption<DisableCrc32cChecksum>();
+  if (crc32c.has_value()) {
+    // DisableCrc32cChecksum defaults to std::nullopt, so if it has a value, it
+    // was explicitly set.
+    disable_crc32c = crc32c.value();
+  }
+  return {disable_md5, disable_crc32c};
+}
+
+}  // namespace internal
+GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
+}  // namespace storage
+}  // namespace cloud
+}  // namespace google
+
+#include "google/cloud/internal/diagnostics_pop.inc"
+#endif  // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_CHECKSUM_HELPERS_H

google/cloud/storage/internal/connection_impl.cc

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
 #include "google/cloud/storage/internal/connection_impl.h"
 #include "google/cloud/storage/internal/retry_object_read_source.h"
 #include "google/cloud/storage/parallel_upload.h"
@@ -1392,3 +1393,4 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace storage
 }  // namespace cloud
 }  // namespace google
+#include "google/cloud/internal/diagnostics_pop.inc"

google/cloud/storage/internal/grpc/object_request_parser.cc

@@ -936,16 +936,6 @@ Status Finalize(google::storage::v2::BidiWriteObjectRequest& write_request,
                            Merge(std::move(hashes), hash_function.Finish()));
 }
 
-// If this is the last `Write()` call of the last `InsertObjectMedia()` set the
-// flags to finalize the request
-Status MaybeFinalize(google::storage::v2::WriteObjectRequest& write_request,
-                     grpc::WriteOptions& options,
-                     storage::internal::InsertObjectMediaRequest const& request,
-                     bool chunk_has_more) {
-  if (chunk_has_more) return {};
-  return Finalize(write_request, options, request.hash_function());
-}
-
 // If this is the last `Write()` call of the last `UploadChunk()` set the flags
 // to finalize the request
 Status MaybeFinalize(google::storage::v2::WriteObjectRequest& write_request,

google/cloud/storage/internal/grpc/object_request_parser.h

@@ -96,11 +96,6 @@ Status Finalize(google::storage::v2::BidiWriteObjectRequest& write_request,
                 storage::internal::HashFunction& hash_function,
                 storage::internal::HashValues = {});
 
-Status MaybeFinalize(google::storage::v2::WriteObjectRequest& write_request,
-                     grpc::WriteOptions& options,
-                     storage::internal::InsertObjectMediaRequest const& request,
-                     bool chunk_has_more);
-
 Status MaybeFinalize(google::storage::v2::WriteObjectRequest& write_request,
                      grpc::WriteOptions& options,
                      storage::internal::UploadChunkRequest const& request,