Skip to content

Commit c03c99b

Browse files
authored
fix(storage): Add start appendable condition for bypassing full object checksum (#15786)
* fix(storage): Add start appendable condition for bypassing full object checksum * Fix the format * add test
1 parent 4ac3fe1 commit c03c99b

File tree

2 files changed

+52
-1
lines changed

2 files changed

+52
-1
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ AsyncWriterConnectionImpl::Finalize(
133133

134134
auto p = WritePayloadImpl::GetImpl(payload);
135135
auto size = p.size();
136-
auto action = request_.has_append_object_spec()
136+
auto action = request_.has_append_object_spec() ||
137+
request_.write_object_spec().appendable()
137138
? PartialUpload::kFinalize
138139
: PartialUpload::kFinalizeWithChecksum;
139140
auto coro = PartialUpload::Call(impl_, hash_function_, std::move(write),

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,56 @@ TEST(AsyncWriterConnectionTest, UnexpectedQueryFailsWithoutError) {
648648
EXPECT_THAT(query.get(), StatusIs(StatusCode::kInternal));
649649
}
650650

651+
TEST(AsyncWriterConnectionTest, FinalizeAppendableNoChecksum) {
652+
AsyncSequencer<bool> sequencer;
653+
auto mock = std::make_unique<MockStream>();
654+
EXPECT_CALL(*mock, Cancel).Times(1);
655+
EXPECT_CALL(*mock, Write)
656+
.WillOnce([&](Request const& request, grpc::WriteOptions wopt) {
657+
EXPECT_TRUE(request.finish_write());
658+
EXPECT_TRUE(wopt.is_last_message());
659+
EXPECT_EQ(request.common_object_request_params().encryption_algorithm(),
660+
"test-only-algo");
661+
EXPECT_FALSE(request.has_object_checksums());
662+
return sequencer.PushBack("Write");
663+
});
664+
EXPECT_CALL(*mock, Read).WillOnce([&]() {
665+
return sequencer.PushBack("Read").then([](auto f) {
666+
if (!f.get()) return absl::optional<Response>();
667+
return absl::make_optional(MakeTestResponse());
668+
});
669+
});
670+
EXPECT_CALL(*mock, Finish).WillOnce([&] {
671+
return sequencer.PushBack("Finish").then([](auto f) {
672+
if (f.get()) return Status{};
673+
return PermanentError();
674+
});
675+
});
676+
auto hash = std::make_shared<MockHashFunction>();
677+
EXPECT_CALL(*hash, Update(_, An<absl::Cord const&>(), _)).Times(1);
678+
EXPECT_CALL(*hash, Finish).Times(0);
679+
680+
auto request = MakeRequest();
681+
request.mutable_write_object_spec()->set_appendable(true);
682+
auto tested = std::make_unique<AsyncWriterConnectionImpl>(
683+
TestOptions(), std::move(request), std::move(mock), hash, 1024);
684+
auto response = tested->Finalize(WritePayload{});
685+
auto next = sequencer.PopFrontWithName();
686+
ASSERT_THAT(next.second, "Write");
687+
next.first.set_value(true);
688+
next = sequencer.PopFrontWithName();
689+
ASSERT_THAT(next.second, "Read");
690+
next.first.set_value(true);
691+
auto object = response.get();
692+
EXPECT_THAT(object, IsOkAndHolds(IsProtoEqual(MakeTestObject())))
693+
<< "=" << object->DebugString();
694+
695+
tested = {};
696+
next = sequencer.PopFrontWithName();
697+
ASSERT_THAT(next.second, "Finish");
698+
next.first.set_value(true);
699+
}
700+
651701
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
652702
} // namespace storage_internal
653703
} // namespace cloud

0 commit comments

Comments
 (0)