refactor(storage): use merged options for checksum validation and simplify logic

Author: v-pratap

google/cloud/storage/examples/storage_async_samples.cc

@@ -750,8 +750,8 @@ void CreateAndWriteAppendableObjectWithChecksum(
 
     // Set the expected CRC32C checksum in the current options scope
     // just before calling Finalize().
-    // Note: 548262564U is the pre-computed CRC32C checksum for the string "Some data\n".
-    // If the data changes, this checksum must be updated to match.
+    // Note: 548262564U is the pre-computed CRC32C checksum for the string "Some
+    // data\n". If the data changes, this checksum must be updated to match.
     google::cloud::internal::OptionsSpan span(
         google::cloud::Options{}.set<gcs::UseCrc32cValueOption>(548262564U));
     co_return (co_await writer.Finalize(std::move(token))).value();

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

@@ -13,14 +13,14 @@
 // limitations under the License.
 
 #include "google/cloud/storage/internal/async/writer_connection_impl.h"
+#include "google/cloud/storage/async/options.h"
+#include "google/cloud/storage/hashing_options.h"
 #include "google/cloud/storage/internal/async/handle_redirect_error.h"
 #include "google/cloud/storage/internal/async/partial_upload.h"
 #include "google/cloud/storage/internal/async/write_payload_impl.h"
 #include "google/cloud/storage/internal/grpc/ctype_cord_workaround.h"
 #include "google/cloud/storage/internal/grpc/object_metadata_parser.h"
 #include "google/cloud/storage/internal/grpc/object_request_parser.h"
-#include "google/cloud/storage/async/options.h"
-#include "google/cloud/storage/hashing_options.h"
 #include "google/cloud/internal/make_status.h"
 
 namespace google {
@@ -147,19 +147,18 @@ AsyncWriterConnectionImpl::Finalize(storage::WritePayload payload) {
   auto merged = google::cloud::internal::MergeOptions(
       current_options, options_ ? *options_ : google::cloud::Options{});
 
-  // Default to letting the internal hash function compute and send the checksum.
+  // Default to letting the internal hash function compute and send the
+  // checksum.
   auto action = PartialUpload::kFinalizeWithChecksum;
 
-  if (is_append || current_options.has<google::cloud::storage::UseCrc32cValueOption>()) {
+  if (is_append || merged.has<google::cloud::storage::UseCrc32cValueOption>()) {
+    // If it's an appendable upload, or the user manually specified an expected
+    // checksum, we manually inject it and use `kFinalize` so the internal hash
+    // function doesn't compute and overwrite it with a partial or local hash.
     if (merged.has<google::cloud::storage::UseCrc32cValueOption>()) {
       write.mutable_object_checksums()->set_crc32c(
           merged.get<google::cloud::storage::UseCrc32cValueOption>());
     }
-    // For appendable uploads, the internal hash function only sees the chunks uploaded
-    // in this stream, not the full object. We use `kFinalize` to avoid sending this
-    // partial hash, which would otherwise fail validation.
-    // If the user specified a manual expected checksum in `current_options`, we must
-    // also use `kFinalize` so the internal hash function doesn't overwrite it.
     action = PartialUpload::kFinalize;
   }
 

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

@@ -757,8 +757,7 @@ TEST(AsyncWriterConnectionTest, FinalizeAppendableWithExpectedChecksum) {
         EXPECT_EQ(request.common_object_request_params().encryption_algorithm(),
                   "test-only-algo");
         EXPECT_TRUE(request.has_object_checksums());
-        EXPECT_EQ(request.object_checksums().crc32c(),
-                  123456);
+        EXPECT_EQ(request.object_checksums().crc32c(), 123456);
         return sequencer.PushBack("Write");
       });
   EXPECT_CALL(*mock, Read).WillOnce([&]() {