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

google/cloud/storage/async/options.h

@@ -29,8 +29,13 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
  * checksum of an object during uploads and downloads.
  *
  * The option has no effect if the service does not return a CRC32C hash.
+ *
+ * @deprecated Use `UploadChecksumValidationOption` and
+ * `DownloadChecksumValidationOption` instead.
  */
-struct EnableCrc32cValidationOption {
+struct [[deprecated(
+    "Use UploadChecksumValidationOption and DownloadChecksumValidationOption "
+    "instead")]] EnableCrc32cValidationOption {
   using Type = bool;
 };
 
@@ -54,8 +59,13 @@ struct UseCrc32cValueOption {
  *
  * The option has no effect for partial downloads or any other circumstance
  * where the service does not return a MD5 hash.
+ *
+ * @deprecated Use `UploadChecksumValidationOption` and
+ * `DownloadChecksumValidationOption` instead.
  */
-struct EnableMD5ValidationOption {
+struct [[deprecated(
+    "Use UploadChecksumValidationOption and DownloadChecksumValidationOption "
+    "instead")]] EnableMD5ValidationOption {
   using Type = bool;
 };
 

google/cloud/storage/google_cloud_cpp_storage_grpc.bzl

@@ -38,6 +38,7 @@ google_cloud_cpp_storage_grpc_hdrs = [
     "async/writer_connection.h",
     "grpc_plugin.h",
     "internal/async/connection_fwd.h",
+    "internal/async/checksum_helpers.h",
     "internal/async/connection_impl.h",
     "internal/async/connection_tracing.h",
     "internal/async/default_options.h",

google/cloud/storage/google_cloud_cpp_storage_grpc.cmake

@@ -99,6 +99,7 @@ add_library(
     async/writer_connection.h
     grpc_plugin.cc
     grpc_plugin.h
+    internal/async/checksum_helpers.h
     internal/async/connection_fwd.h
     internal/async/connection_impl.cc
     internal/async/connection_impl.h

google/cloud/storage/hashing_options.h

@@ -15,8 +15,10 @@
 #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_HASHING_OPTIONS_H
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_HASHING_OPTIONS_H
 
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
 #include "google/cloud/storage/internal/complex_option.h"
 #include "google/cloud/storage/version.h"
+#include "google/cloud/version.h"
 #include "absl/strings/string_view.h"
 #include <string>
 
@@ -70,8 +72,14 @@ inline std::string ComputeMD5Hash(char const* payload) {
  *   for most applications.  Disabling CRC32C checksums while MD5 hashes remain
  *   disabled exposes your application to data corruption. We recommend that all
  *   uploads to GCS and downloads from GCS use CRC32C checksums.
+ *
+ * @deprecated Use `UploadChecksumValidationOption` and
+ * `DownloadChecksumValidationOption` instead.
  */
-struct DisableMD5Hash : public internal::ComplexOption<DisableMD5Hash, bool> {
+struct [[deprecated(
+    "Use UploadChecksumValidationOption and DownloadChecksumValidationOption "
+    "instead")]] DisableMD5Hash
+    : public internal::ComplexOption<DisableMD5Hash, bool> {
   using ComplexOption<DisableMD5Hash, bool>::ComplexOption;
   // GCC <= 7.0 does not use the inherited default constructor, redeclare it
   // explicitly
@@ -134,8 +142,13 @@ inline std::string ComputeCrc32cChecksum(char const* payload) {
  *   for most applications.  Disabling CRC32C checksums while MD5 hashes remain
  *   disabled exposes your application to data corruption. We recommend that all
  *   uploads to GCS and downloads from GCS use CRC32C checksums.
+ *
+ * @deprecated Use `UploadChecksumValidationOption` and
+ * `DownloadChecksumValidationOption` instead.
  */
-struct DisableCrc32cChecksum
+struct [[deprecated(
+    "Use UploadChecksumValidationOption and DownloadChecksumValidationOption "
+    "instead")]] DisableCrc32cChecksum
     : public internal::ComplexOption<DisableCrc32cChecksum, bool> {
   using ComplexOption<DisableCrc32cChecksum, bool>::ComplexOption;
   // GCC <= 7.0 does not use the inherited default constructor, redeclare it
@@ -149,4 +162,5 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace cloud
 }  // namespace google
 
+#include "google/cloud/internal/diagnostics_pop.inc"
 #endif  // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_HASHING_OPTIONS_H

google/cloud/storage/hashing_options_test.cc

@@ -13,6 +13,8 @@
 // limitations under the License.
 
 #include "google/cloud/storage/hashing_options.h"
+#include "google/cloud/storage/options.h"
+#include "google/cloud/options.h"
 #include <gmock/gmock.h>
 #include <string>
 
@@ -54,6 +56,23 @@ TEST(ComputeCrc32cChecksumTest, Simple) {
   EXPECT_EQ("ImIEBA==", actual);
 }
 
+TEST(ChecksumOptionsTest, SetAndGet) {
+  Options options;
+  EXPECT_FALSE(options.has<UploadChecksumValidationOption>());
+  EXPECT_FALSE(options.has<DownloadChecksumValidationOption>());
+
+  options.set<UploadChecksumValidationOption>(ChecksumAlgorithm::kMD5);
+  options.set<DownloadChecksumValidationOption>(ChecksumAlgorithm::kCrc32c);
+
+  EXPECT_TRUE(options.has<UploadChecksumValidationOption>());
+  EXPECT_EQ(ChecksumAlgorithm::kMD5,
+            options.get<UploadChecksumValidationOption>());
+
+  EXPECT_TRUE(options.has<DownloadChecksumValidationOption>());
+  EXPECT_EQ(ChecksumAlgorithm::kCrc32c,
+            options.get<DownloadChecksumValidationOption>());
+}
+
 }  // namespace
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace storage

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

@@ -0,0 +1,59 @@
+// 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_ASYNC_CHECKSUM_HELPERS_H
+#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ASYNC_CHECKSUM_HELPERS_H
+
+#include "google/cloud/internal/disable_deprecation_warnings.inc"
+#include "google/cloud/storage/async/options.h"
+#include "google/cloud/storage/options.h"
+#include "google/cloud/options.h"
+
+namespace google {
+namespace cloud {
+namespace storage_internal {
+GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
+
+struct ChecksumSettings {
+  bool enable_crc32c;
+  bool enable_md5;
+};
+
+inline ChecksumSettings GetDownloadChecksumSettings(Options const& options) {
+  if (options.has<storage::DownloadChecksumValidationOption>()) {
+    auto const algo = options.get<storage::DownloadChecksumValidationOption>();
+    return {algo == storage::ChecksumAlgorithm::kCrc32c,
+            algo == storage::ChecksumAlgorithm::kMD5};
+  }
+  return {options.get<storage::EnableCrc32cValidationOption>(),
+          options.get<storage::EnableMD5ValidationOption>()};
+}
+
+inline ChecksumSettings GetUploadChecksumSettings(Options const& options) {
+  if (options.has<storage::UploadChecksumValidationOption>()) {
+    auto const algo = options.get<storage::UploadChecksumValidationOption>();
+    return {algo == storage::ChecksumAlgorithm::kCrc32c,
+            algo == storage::ChecksumAlgorithm::kMD5};
+  }
+  return {options.get<storage::EnableCrc32cValidationOption>(),
+          options.get<storage::EnableMD5ValidationOption>()};
+}
+
+GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
+}  // namespace storage_internal
+}  // namespace cloud
+}  // namespace google
+
+#include "google/cloud/internal/diagnostics_pop.inc"
+#endif  // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ASYNC_CHECKSUM_HELPERS_H

google/cloud/storage/internal/async/connection_impl.cc

@@ -19,6 +19,7 @@
 #include "google/cloud/storage/async/reader.h"
 #include "google/cloud/storage/async/resume_policy.h"
 #include "google/cloud/storage/async/retry_policy.h"
+#include "google/cloud/storage/internal/async/checksum_helpers.h"
 #include "google/cloud/storage/internal/async/default_options.h"
 #include "google/cloud/storage/internal/async/handle_redirect_error.h"
 #include "google/cloud/storage/internal/async/insert_object.h"
@@ -80,14 +81,18 @@ inline std::unique_ptr<storage::AsyncIdempotencyPolicy> idempotency_policy(
 }
 
 std::unique_ptr<storage::internal::HashFunction> CreateHashFunction(
-    Options const& options) {
+    Options const& options,
+    storage_internal::ChecksumSettings const& settings) {
   auto crc32c = std::unique_ptr<storage::internal::HashFunction>();
+  auto const enable_crc32c = settings.enable_crc32c;
+  auto const enable_md5 = settings.enable_md5;
+
   if (options.has<storage::UseCrc32cValueOption>()) {
     crc32c = std::make_unique<storage::internal::PrecomputedHashFunction>(
         storage::internal::HashValues{
             Crc32cFromProto(options.get<storage::UseCrc32cValueOption>()),
             /*.md5=*/{}});
-  } else if (options.get<storage::EnableCrc32cValidationOption>()) {
+  } else if (enable_crc32c) {
     crc32c = std::make_unique<storage::internal::Crc32cHashFunction>();
   }
 
@@ -97,7 +102,7 @@ std::unique_ptr<storage::internal::HashFunction> CreateHashFunction(
         storage::internal::HashValues{
             /*.crc32=*/{},
             MD5FromProto(options.get<storage::UseMD5ValueOption>())});
-  } else if (options.get<storage::EnableMD5ValidationOption>()) {
+  } else if (enable_md5) {
     md5 = storage::internal::MD5HashFunction::Create();
   }
 
@@ -117,9 +122,9 @@ std::unique_ptr<storage::internal::HashValidator> CreateHashValidator(
       request.read_limit() != 0 || request.read_offset() != 0;
   if (is_ranged_read) return storage::internal::CreateNullHashValidator();
 
-  auto const enable_crc32c =
-      options.get<storage::EnableCrc32cValidationOption>();
-  auto const enable_md5 = options.get<storage::EnableMD5ValidationOption>();
+  auto const settings = GetDownloadChecksumSettings(options);
+  auto const enable_crc32c = settings.enable_crc32c;
+  auto const enable_md5 = settings.enable_md5;
 
   if (enable_crc32c && enable_md5) {
     return std::make_unique<storage::internal::CompositeValidator>(
@@ -167,7 +172,8 @@ future<StatusOr<google::storage::v2::Object>> AsyncConnectionImpl::InsertObject(
         options->get<storage::TransferStallMinimumRateOption>(),
         google::storage::v2::ServiceConstants::MAX_WRITE_CHUNK_BYTES);
 
-    auto hash_function = CreateHashFunction(*options);
+    auto hash_function =
+        CreateHashFunction(*options, GetUploadChecksumSettings(*options));
     ApplyRoutingHeaders(*context, request.write_object_spec());
     AddIdempotencyToken(*context, id);
     auto rpc = stub->AsyncWriteObject(cq, std::move(context), options);
@@ -271,9 +277,10 @@ AsyncConnectionImpl::ReadObject(ReadObjectParams p) {
   // Create the hash function and validator based on the original request. Note
   // that p.request will be moved-from, so we have to do it relatively early in
   // this function.
+  auto const settings = GetDownloadChecksumSettings(*current);
   auto hash_function =
       std::make_shared<storage::internal::Crc32cMessageHashFunction>(
-          CreateHashFunction(*current));
+          CreateHashFunction(*current, settings));
   auto hash_validator = CreateHashValidator(p.request, *current);
 
   absl::optional<std::int64_t> requested_length;
@@ -323,7 +330,7 @@ AsyncConnectionImpl::AppendableObjectUploadImpl(AppendableUploadParams p) {
   auto current = internal::MakeImmutableOptions(std::move(p.options));
   auto request = p.request;
   std::shared_ptr<storage::internal::HashFunction> hash_function =
-      CreateHashFunction(*current);
+      CreateHashFunction(*current, GetUploadChecksumSettings(*current));
   auto retry =
       std::shared_ptr<storage::AsyncRetryPolicy>(retry_policy(*current));
   auto backoff =
@@ -703,7 +710,8 @@ AsyncConnectionImpl::StartUnbufferedUploadImpl(
         StatusOr<std::unique_ptr<storage::AsyncWriterConnection>>(
             std::move(response).status()));
   }
-  auto hash_function = CreateHashFunction(*current);
+  auto hash_function =
+      CreateHashFunction(*current, GetUploadChecksumSettings(*current));
   auto configure =
       [current, upload = response->upload_id()](grpc::ClientContext& context) {
         ApplyResumableUploadRoutingHeader(context, upload);
@@ -741,7 +749,8 @@ AsyncConnectionImpl::ResumeUnbufferedUploadImpl(
   // the upload resumes from the beginning of the file.
   auto hash_function = storage::internal::CreateNullHashFunction();
   if (response->persisted_size() == 0) {
-    hash_function = CreateHashFunction(*current);
+    hash_function =
+        CreateHashFunction(*current, GetUploadChecksumSettings(*current));
   }
   auto configure =
       [current, upload_id = query.upload_id()](grpc::ClientContext& context) {