From 79ab9d85f3dd1c9f814f18b57225a4c3e9443f6d Mon Sep 17 00:00:00 2001 From: Vaibhav Pratap Date: Mon, 13 Jul 2026 06:31:41 +0000 Subject: [PATCH 1/4] fix(storage): allow providing expected CRC32C to Finalize() for appendable uploads --- .../storage/examples/storage_async_samples.cc | 42 +++++++ .../internal/async/writer_connection_impl.cc | 26 +++- .../async/writer_connection_impl_test.cc | 119 ++++++++++++++++++ 3 files changed, 183 insertions(+), 4 deletions(-) diff --git a/google/cloud/storage/examples/storage_async_samples.cc b/google/cloud/storage/examples/storage_async_samples.cc index da4f6c5f5fbf3..5483bffe7d52d 100644 --- a/google/cloud/storage/examples/storage_async_samples.cc +++ b/google/cloud/storage/examples/storage_async_samples.cc @@ -728,6 +728,40 @@ void CreateAndWriteAppendableObject(google::cloud::storage::AsyncClient& client, std::cout << "File successfully uploaded " << object.DebugString() << "\n"; } +void CreateAndWriteAppendableObjectWithChecksum( + google::cloud::storage::AsyncClient& client, + std::vector const& argv) { + //! [create-and-write-appendable-object-with-checksum] + // [START storage_create_and_write_appendable_object_with_checksum] + namespace gcs = google::cloud::storage; + auto coro = [](gcs::AsyncClient& client, std::string bucket_name, + std::string object_name) + -> google::cloud::future { + auto [writer, token] = + (co_await client.StartAppendableObjectUpload( + gcs::BucketName(std::move(bucket_name)), std::move(object_name))) + .value(); + std::cout << "Appendable upload started for object " << object_name << "\n"; + + token = (co_await writer.Write(std::move(token), + gcs::WritePayload("Some data\n"))) + .value(); + std::cout << "Wrote some data.\n"; + + // Set the expected CRC32C checksum in the current options scope + // just before calling Finalize(). + google::cloud::internal::OptionsSpan span( + google::cloud::Options{}.set(1848151177U)); + co_return (co_await writer.Finalize(std::move(token))).value(); + }; + // [END storage_create_and_write_appendable_object_with_checksum] + //! [create-and-write-appendable-object-with-checksum] + // The example is easier to test and run if we call the coroutine and block + // until it completes.. + auto const object = coro(client, argv.at(0), argv.at(1)).get(); + std::cout << "File successfully uploaded " << object.DebugString() << "\n"; +} + void PauseAndResumeAppendableUpload(google::cloud::storage::AsyncClient& client, std::vector const& argv) { //! [pause-and-resume-appendable-upload] @@ -1035,6 +1069,12 @@ void CreateAndWriteAppendableObject(google::cloud::storage::AsyncClient&, "coroutines\n"; } +void CreateAndWriteAppendableObjectWithChecksum( + google::cloud::storage::AsyncClient&, std::vector const&) { + std::cerr << "AsyncClient::CreateAndWriteAppendableObjectWithChecksum() " + "example requires coroutines\n"; +} + void PauseAndResumeAppendableUpload(google::cloud::storage::AsyncClient&, std::vector const&) { std::cerr << "AsyncClient::PauseAndResumeAppendableUpload() example requires " @@ -1505,6 +1545,8 @@ int main(int argc, char* argv[]) try { make_entry("create-and-write-appendable-object", {}, CreateAndWriteAppendableObject), + make_entry("create-and-write-appendable-object-with-checksum", {}, + CreateAndWriteAppendableObjectWithChecksum), make_entry("pause-and-resume-appendable-upload", {}, PauseAndResumeAppendableUpload), make_entry("finalize-appendable-object-upload", {}, diff --git a/google/cloud/storage/internal/async/writer_connection_impl.cc b/google/cloud/storage/internal/async/writer_connection_impl.cc index 3acafa12d7dd8..50dfa64023a89 100644 --- a/google/cloud/storage/internal/async/writer_connection_impl.cc +++ b/google/cloud/storage/internal/async/writer_connection_impl.cc @@ -18,6 +18,8 @@ #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/internal/make_status.h" namespace google { @@ -138,10 +140,26 @@ AsyncWriterConnectionImpl::Finalize(storage::WritePayload payload) { auto p = WritePayloadImpl::GetImpl(payload); auto size = p.size(); - auto action = request_.has_append_object_spec() || - request_.write_object_spec().appendable() - ? PartialUpload::kFinalize - : PartialUpload::kFinalizeWithChecksum; + auto is_append = request_.has_append_object_spec() || + request_.write_object_spec().appendable(); + auto current_options = google::cloud::internal::CurrentOptions(); + + // Default to letting the internal hash function compute and send the checksum. + auto action = PartialUpload::kFinalizeWithChecksum; + + if (current_options.has()) { + // The user provided a final CRC via OptionsSpan. We manually inject it into the + // request. We use `kFinalize` so the internal hash function doesn't overwrite it. + write.mutable_object_checksums()->set_crc32c( + current_options.get()); + action = PartialUpload::kFinalize; + } else if (is_append && !options_->has()) { + // 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 CRC, which would otherwise fail validation. + action = PartialUpload::kFinalize; + } + auto coro = PartialUpload::Call(impl_, hash_function_, std::move(write), std::move(p), std::move(action)); return coro->Start().then([coro, size, this](auto f) mutable { diff --git a/google/cloud/storage/internal/async/writer_connection_impl_test.cc b/google/cloud/storage/internal/async/writer_connection_impl_test.cc index 03ecf7699cc2c..f9e783b502030 100644 --- a/google/cloud/storage/internal/async/writer_connection_impl_test.cc +++ b/google/cloud/storage/internal/async/writer_connection_impl_test.cc @@ -14,9 +14,11 @@ #include "google/cloud/storage/internal/async/writer_connection_impl.h" #include "google/cloud/mocks/mock_async_streaming_read_write_rpc.h" +#include "google/cloud/storage/async/options.h" #include "google/cloud/storage/internal/async/write_object.h" #include "google/cloud/storage/internal/crc32c.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/hash_function_impl.h" #include "google/cloud/storage/options.h" #include "google/cloud/storage/testing/canonical_errors.h" @@ -744,6 +746,123 @@ TEST(AsyncWriterConnectionTest, FinalizeAppendableNoChecksum) { next.first.set_value(true); } +TEST(AsyncWriterConnectionTest, FinalizeAppendableWithExpectedChecksum) { + AsyncSequencer sequencer; + auto mock = std::make_unique(); + EXPECT_CALL(*mock, Cancel).Times(1); + EXPECT_CALL(*mock, Write) + .WillOnce([&](Request const& request, grpc::WriteOptions wopt) { + EXPECT_TRUE(request.finish_write()); + EXPECT_TRUE(wopt.is_last_message()); + 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); // wait, it might be an int in proto + return sequencer.PushBack("Write"); + }); + EXPECT_CALL(*mock, Read).WillOnce([&]() { + return sequencer.PushBack("Read").then([](auto f) { + if (!f.get()) return absl::optional(); + return absl::make_optional(MakeTestResponse()); + }); + }); + EXPECT_CALL(*mock, Finish).WillOnce([&] { + return sequencer.PushBack("Finish").then([](auto f) { + if (f.get()) return Status{}; + return PermanentError(); + }); + }); + auto hash = std::make_shared(); + EXPECT_CALL(*hash, Update(_, An(), _)).Times(1); + EXPECT_CALL(*hash, Finish) + .WillOnce(Return(storage::internal::HashValues{ + storage_internal::Crc32cFromProto(123456), {}})); + + auto request = MakeRequest(); + request.mutable_write_object_spec()->set_appendable(true); + + auto options = internal::MakeImmutableOptions( + Options{}.set(123456)); + + auto tested = std::make_unique( + options, std::move(request), std::move(mock), hash, 1024); + auto response = tested->Finalize(WritePayload{}); + + auto next = sequencer.PopFrontWithName(); + ASSERT_THAT(next.second, "Write"); + next.first.set_value(true); + next = sequencer.PopFrontWithName(); + ASSERT_THAT(next.second, "Read"); + next.first.set_value(true); + auto object = response.get(); + EXPECT_THAT(object, IsOkAndHolds(IsProtoEqual(MakeTestObject()))) + << "=" << object->DebugString(); + + tested = {}; + next = sequencer.PopFrontWithName(); + ASSERT_THAT(next.second, "Finish"); + next.first.set_value(true); +} + +TEST(AsyncWriterConnectionTest, + FinalizeAppendableWithExpectedChecksumFromCurrentOptions) { + AsyncSequencer sequencer; + auto mock = std::make_unique(); + EXPECT_CALL(*mock, Cancel).Times(1); + EXPECT_CALL(*mock, Write) + .WillOnce([&](Request const& request, grpc::WriteOptions wopt) { + EXPECT_TRUE(request.finish_write()); + EXPECT_TRUE(wopt.is_last_message()); + EXPECT_EQ(request.common_object_request_params().encryption_algorithm(), + "test-only-algo"); + EXPECT_TRUE(request.has_object_checksums()); + EXPECT_EQ(request.object_checksums().crc32c(), 654321); + return sequencer.PushBack("Write"); + }); + EXPECT_CALL(*mock, Read).WillOnce([&]() { + return sequencer.PushBack("Read").then([](auto f) { + if (!f.get()) return absl::optional(); + return absl::make_optional(MakeTestResponse()); + }); + }); + EXPECT_CALL(*mock, Finish).WillOnce([&] { + return sequencer.PushBack("Finish").then([](auto f) { + if (f.get()) return Status{}; + return PermanentError(); + }); + }); + auto hash = std::make_shared(); + EXPECT_CALL(*hash, Update(_, An(), _)).Times(1); + // It shouldn't call Finish() because we use kFinalize! + EXPECT_CALL(*hash, Finish).Times(0); + + auto request = MakeRequest(); + request.mutable_write_object_spec()->set_appendable(true); + + auto tested = std::make_unique( + TestOptions(), std::move(request), std::move(mock), hash, 1024); + + internal::OptionsSpan span( + Options{}.set(654321)); + auto response = tested->Finalize(WritePayload{}); + + auto next = sequencer.PopFrontWithName(); + ASSERT_THAT(next.second, "Write"); + next.first.set_value(true); + next = sequencer.PopFrontWithName(); + ASSERT_THAT(next.second, "Read"); + next.first.set_value(true); + auto object = response.get(); + EXPECT_THAT(object, IsOkAndHolds(IsProtoEqual(MakeTestObject()))) + << "=" << object->DebugString(); + + tested = {}; + next = sequencer.PopFrontWithName(); + ASSERT_THAT(next.second, "Finish"); + next.first.set_value(true); +} + TEST(AsyncWriterConnectionTest, ResumeWithHandle) { AsyncSequencer sequencer; auto mock = std::make_unique(); From 1d35e9604dd581bb6bfef4c9056c12ad99fd5818 Mon Sep 17 00:00:00 2001 From: Vaibhav Pratap Date: Mon, 13 Jul 2026 06:56:34 +0000 Subject: [PATCH 2/4] fix(storage): address reviewer comments on checksum logic --- .../storage/examples/storage_async_samples.cc | 2 +- .../internal/async/writer_connection_impl.cc | 22 +++++++++++++++++-- .../async/writer_connection_impl_test.cc | 4 +--- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/google/cloud/storage/examples/storage_async_samples.cc b/google/cloud/storage/examples/storage_async_samples.cc index 5483bffe7d52d..c2f884635949e 100644 --- a/google/cloud/storage/examples/storage_async_samples.cc +++ b/google/cloud/storage/examples/storage_async_samples.cc @@ -751,7 +751,7 @@ void CreateAndWriteAppendableObjectWithChecksum( // Set the expected CRC32C checksum in the current options scope // just before calling Finalize(). google::cloud::internal::OptionsSpan span( - google::cloud::Options{}.set(1848151177U)); + google::cloud::Options{}.set(548262564U)); co_return (co_await writer.Finalize(std::move(token))).value(); }; // [END storage_create_and_write_appendable_object_with_checksum] diff --git a/google/cloud/storage/internal/async/writer_connection_impl.cc b/google/cloud/storage/internal/async/writer_connection_impl.cc index 50dfa64023a89..5a972bcf74115 100644 --- a/google/cloud/storage/internal/async/writer_connection_impl.cc +++ b/google/cloud/storage/internal/async/writer_connection_impl.cc @@ -20,6 +20,7 @@ #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 { @@ -153,10 +154,27 @@ AsyncWriterConnectionImpl::Finalize(storage::WritePayload payload) { write.mutable_object_checksums()->set_crc32c( current_options.get()); action = PartialUpload::kFinalize; - } else if (is_append && !options_->has()) { + } else if (current_options.has()) { + auto as_proto = storage_internal::MD5ToProto( + current_options.get()); + if (as_proto) { + write.mutable_object_checksums()->set_md5_hash(*as_proto); + action = PartialUpload::kFinalize; + } + } else if (is_append) { + if (options_->has()) { + write.mutable_object_checksums()->set_crc32c( + options_->get()); + } else if (options_->has()) { + auto as_proto = storage_internal::MD5ToProto( + options_->get()); + if (as_proto) { + write.mutable_object_checksums()->set_md5_hash(*as_proto); + } + } // 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 CRC, which would otherwise fail validation. + // partial hash, which would otherwise fail validation. action = PartialUpload::kFinalize; } diff --git a/google/cloud/storage/internal/async/writer_connection_impl_test.cc b/google/cloud/storage/internal/async/writer_connection_impl_test.cc index f9e783b502030..4641f4e5dbdb2 100644 --- a/google/cloud/storage/internal/async/writer_connection_impl_test.cc +++ b/google/cloud/storage/internal/async/writer_connection_impl_test.cc @@ -775,9 +775,7 @@ TEST(AsyncWriterConnectionTest, FinalizeAppendableWithExpectedChecksum) { }); auto hash = std::make_shared(); EXPECT_CALL(*hash, Update(_, An(), _)).Times(1); - EXPECT_CALL(*hash, Finish) - .WillOnce(Return(storage::internal::HashValues{ - storage_internal::Crc32cFromProto(123456), {}})); + EXPECT_CALL(*hash, Finish).Times(0); auto request = MakeRequest(); request.mutable_write_object_spec()->set_appendable(true); From 8f48e9d7173086822902de25714e73607eaa7e00 Mon Sep 17 00:00:00 2001 From: Vaibhav Pratap Date: Mon, 13 Jul 2026 07:29:54 +0000 Subject: [PATCH 3/4] fix(storage): correctly merge options and handle MD5 for appendable uploads --- .../storage/examples/storage_async_samples.cc | 2 ++ .../internal/async/writer_connection_impl.cc | 31 +++++++------------ .../async/writer_connection_impl_test.cc | 2 +- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/google/cloud/storage/examples/storage_async_samples.cc b/google/cloud/storage/examples/storage_async_samples.cc index c2f884635949e..197acaec3fa22 100644 --- a/google/cloud/storage/examples/storage_async_samples.cc +++ b/google/cloud/storage/examples/storage_async_samples.cc @@ -750,6 +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. google::cloud::internal::OptionsSpan span( google::cloud::Options{}.set(548262564U)); co_return (co_await writer.Finalize(std::move(token))).value(); diff --git a/google/cloud/storage/internal/async/writer_connection_impl.cc b/google/cloud/storage/internal/async/writer_connection_impl.cc index 5a972bcf74115..30e91e386d245 100644 --- a/google/cloud/storage/internal/async/writer_connection_impl.cc +++ b/google/cloud/storage/internal/async/writer_connection_impl.cc @@ -144,30 +144,21 @@ AsyncWriterConnectionImpl::Finalize(storage::WritePayload payload) { auto is_append = request_.has_append_object_spec() || request_.write_object_spec().appendable(); auto current_options = google::cloud::internal::CurrentOptions(); - + 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. auto action = PartialUpload::kFinalizeWithChecksum; - if (current_options.has()) { - // The user provided a final CRC via OptionsSpan. We manually inject it into the - // request. We use `kFinalize` so the internal hash function doesn't overwrite it. - write.mutable_object_checksums()->set_crc32c( - current_options.get()); - action = PartialUpload::kFinalize; - } else if (current_options.has()) { - auto as_proto = storage_internal::MD5ToProto( - current_options.get()); - if (as_proto) { - write.mutable_object_checksums()->set_md5_hash(*as_proto); - action = PartialUpload::kFinalize; - } - } else if (is_append) { - if (options_->has()) { + if (is_append || current_options.has() || + current_options.has()) { + if (merged.has()) { write.mutable_object_checksums()->set_crc32c( - options_->get()); - } else if (options_->has()) { + merged.get()); + } + if (merged.has()) { auto as_proto = storage_internal::MD5ToProto( - options_->get()); + merged.get()); if (as_proto) { write.mutable_object_checksums()->set_md5_hash(*as_proto); } @@ -175,6 +166,8 @@ AsyncWriterConnectionImpl::Finalize(storage::WritePayload payload) { // 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; } diff --git a/google/cloud/storage/internal/async/writer_connection_impl_test.cc b/google/cloud/storage/internal/async/writer_connection_impl_test.cc index 4641f4e5dbdb2..9603158f933d1 100644 --- a/google/cloud/storage/internal/async/writer_connection_impl_test.cc +++ b/google/cloud/storage/internal/async/writer_connection_impl_test.cc @@ -758,7 +758,7 @@ TEST(AsyncWriterConnectionTest, FinalizeAppendableWithExpectedChecksum) { "test-only-algo"); EXPECT_TRUE(request.has_object_checksums()); EXPECT_EQ(request.object_checksums().crc32c(), - 123456); // wait, it might be an int in proto + 123456); return sequencer.PushBack("Write"); }); EXPECT_CALL(*mock, Read).WillOnce([&]() { From 76976b59faf0bec7df42765726b2af236cc26fbc Mon Sep 17 00:00:00 2001 From: Vaibhav Pratap Date: Mon, 13 Jul 2026 07:32:05 +0000 Subject: [PATCH 4/4] refactor(storage): clarify Finalize() checksum logic to avoid apparent redundancy --- .../storage/examples/storage_async_samples.cc | 4 +- .../internal/async/writer_connection_impl.cc | 41 +++--- .../async/writer_connection_impl_test.cc | 121 +++++++++++++++++- 3 files changed, 145 insertions(+), 21 deletions(-) diff --git a/google/cloud/storage/examples/storage_async_samples.cc b/google/cloud/storage/examples/storage_async_samples.cc index 197acaec3fa22..8bf4933c483b5 100644 --- a/google/cloud/storage/examples/storage_async_samples.cc +++ b/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(548262564U)); co_return (co_await writer.Finalize(std::move(token))).value(); diff --git a/google/cloud/storage/internal/async/writer_connection_impl.cc b/google/cloud/storage/internal/async/writer_connection_impl.cc index 30e91e386d245..88dd17fc9cf74 100644 --- a/google/cloud/storage/internal/async/writer_connection_impl.cc +++ b/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 { @@ -143,31 +143,38 @@ AsyncWriterConnectionImpl::Finalize(storage::WritePayload payload) { auto size = p.size(); auto is_append = request_.has_append_object_spec() || request_.write_object_spec().appendable(); - auto current_options = google::cloud::internal::CurrentOptions(); + auto const& current_options = google::cloud::internal::CurrentOptions(); 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() || - current_options.has()) { + if (is_append) { + // 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 (merged.has()) { write.mutable_object_checksums()->set_crc32c( merged.get()); } - if (merged.has()) { - auto as_proto = storage_internal::MD5ToProto( - merged.get()); - if (as_proto) { - write.mutable_object_checksums()->set_md5_hash(*as_proto); - } + action = PartialUpload::kFinalize; + } else if (current_options + .has() || + current_options.has()) { + // If the user specified a manual expected checksum dynamically at + // Finalize() via current_options, we manually inject it and use `kFinalize` + // so the internal hash function doesn't overwrite it with its own computed + // hash. + if (current_options.has()) { + write.mutable_object_checksums()->set_crc32c( + current_options.get()); + } + if (current_options.has()) { + write.mutable_object_checksums()->set_md5_hash( + current_options.get()); } - // 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; } diff --git a/google/cloud/storage/internal/async/writer_connection_impl_test.cc b/google/cloud/storage/internal/async/writer_connection_impl_test.cc index 9603158f933d1..37a4f9cd9a30c 100644 --- a/google/cloud/storage/internal/async/writer_connection_impl_test.cc +++ b/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([&]() { @@ -861,6 +860,124 @@ TEST(AsyncWriterConnectionTest, next.first.set_value(true); } +TEST(AsyncWriterConnectionTest, + FinalizeNonAppendableWithExpectedChecksumFromCurrentOptions) { + AsyncSequencer sequencer; + auto mock = std::make_unique(); + EXPECT_CALL(*mock, Cancel).Times(1); + EXPECT_CALL(*mock, Write) + .WillOnce([&](Request const& request, grpc::WriteOptions wopt) { + EXPECT_TRUE(request.finish_write()); + EXPECT_TRUE(wopt.is_last_message()); + EXPECT_EQ(request.common_object_request_params().encryption_algorithm(), + "test-only-algo"); + EXPECT_TRUE(request.has_object_checksums()); + EXPECT_EQ(request.object_checksums().crc32c(), 654321); + return sequencer.PushBack("Write"); + }); + EXPECT_CALL(*mock, Read).WillOnce([&]() { + return sequencer.PushBack("Read").then([](auto f) { + if (!f.get()) return absl::optional(); + return absl::make_optional(MakeTestResponse()); + }); + }); + EXPECT_CALL(*mock, Finish).WillOnce([&] { + return sequencer.PushBack("Finish").then([](auto f) { + if (f.get()) return Status{}; + return PermanentError(); + }); + }); + auto hash = std::make_shared(); + EXPECT_CALL(*hash, Update(_, An(), _)).Times(1); + // It shouldn't call Finish() because we use kFinalize! + EXPECT_CALL(*hash, Finish).Times(0); + + auto request = MakeRequest(); + // Ensure it's explicitly not appendable. + request.mutable_write_object_spec()->set_appendable(false); + + auto tested = std::make_unique( + TestOptions(), std::move(request), std::move(mock), hash, 1024); + + internal::OptionsSpan span( + Options{}.set(654321)); + auto response = tested->Finalize(WritePayload{}); + + auto next = sequencer.PopFrontWithName(); + ASSERT_THAT(next.second, "Write"); + next.first.set_value(true); + next = sequencer.PopFrontWithName(); + ASSERT_THAT(next.second, "Read"); + next.first.set_value(true); + auto object = response.get(); + EXPECT_THAT(object, IsOkAndHolds(IsProtoEqual(MakeTestObject()))) + << "=" << object->DebugString(); + + tested = {}; + next = sequencer.PopFrontWithName(); + ASSERT_THAT(next.second, "Finish"); + next.first.set_value(true); +} + +TEST(AsyncWriterConnectionTest, + FinalizeNonAppendableWithExpectedMD5FromCurrentOptions) { + AsyncSequencer sequencer; + auto mock = std::make_unique(); + EXPECT_CALL(*mock, Cancel).Times(1); + EXPECT_CALL(*mock, Write) + .WillOnce([&](Request const& request, grpc::WriteOptions wopt) { + EXPECT_TRUE(request.finish_write()); + EXPECT_TRUE(wopt.is_last_message()); + EXPECT_EQ(request.common_object_request_params().encryption_algorithm(), + "test-only-algo"); + EXPECT_TRUE(request.has_object_checksums()); + EXPECT_EQ(request.object_checksums().md5_hash(), "test-md5"); + return sequencer.PushBack("Write"); + }); + EXPECT_CALL(*mock, Read).WillOnce([&]() { + return sequencer.PushBack("Read").then([](auto f) { + if (!f.get()) return absl::optional(); + return absl::make_optional(MakeTestResponse()); + }); + }); + EXPECT_CALL(*mock, Finish).WillOnce([&] { + return sequencer.PushBack("Finish").then([](auto f) { + if (f.get()) return Status{}; + return PermanentError(); + }); + }); + auto hash = std::make_shared(); + EXPECT_CALL(*hash, Update(_, An(), _)).Times(1); + // It shouldn't call Finish() because we use kFinalize! + EXPECT_CALL(*hash, Finish).Times(0); + + auto request = MakeRequest(); + // Ensure it's explicitly not appendable. + request.mutable_write_object_spec()->set_appendable(false); + + auto tested = std::make_unique( + TestOptions(), std::move(request), std::move(mock), hash, 1024); + + internal::OptionsSpan span( + Options{}.set("test-md5")); + auto response = tested->Finalize(WritePayload{}); + + auto next = sequencer.PopFrontWithName(); + ASSERT_THAT(next.second, "Write"); + next.first.set_value(true); + next = sequencer.PopFrontWithName(); + ASSERT_THAT(next.second, "Read"); + next.first.set_value(true); + auto object = response.get(); + EXPECT_THAT(object, IsOkAndHolds(IsProtoEqual(MakeTestObject()))) + << "=" << object->DebugString(); + + tested = {}; + next = sequencer.PopFrontWithName(); + ASSERT_THAT(next.second, "Finish"); + next.first.set_value(true); +} + TEST(AsyncWriterConnectionTest, ResumeWithHandle) { AsyncSequencer sequencer; auto mock = std::make_unique();