Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit76c0a0e

Browse files
authored
fix(storage): append object spec should only be set in the first request (#15831)
1 parentcaff0e1 commit76c0a0e

File tree

2 files changed

+38
-89
lines changed

2 files changed

+38
-89
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,16 +172,16 @@ AsyncWriterConnectionImpl::MakeRequest() {
172172
auto request = request_;
173173
if (first_request_) {
174174
first_request_ =false;
175+
if (latest_write_handle_.has_value()) {
176+
*request.mutable_append_object_spec()->mutable_write_handle() =
177+
*latest_write_handle_;
178+
}
175179
}else {
176180
request.clear_upload_id();
177181
request.clear_write_object_spec();
178182
request.clear_append_object_spec();
179183
}
180184
request.set_write_offset(offset_);
181-
if (latest_write_handle_.has_value()) {
182-
*request.mutable_append_object_spec()->mutable_write_handle() =
183-
*latest_write_handle_;
184-
}
185185
return request;
186186
}
187187

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

Lines changed: 34 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -699,13 +699,13 @@ TEST(AsyncWriterConnectionTest, FinalizeAppendableNoChecksum) {
699699
next.first.set_value(true);
700700
}
701701

702-
TEST(AsyncWriterConnectionTest,WriteHandleIsUpdatedAfterQuery) {
702+
TEST(AsyncWriterConnectionTest,ResumeWithHandle) {
703703
AsyncSequencer<bool> sequencer;
704704
auto mock = std::make_unique<MockStream>();
705705
std::vector<std::string> seen_handles;
706706

707707
EXPECT_CALL(*mock, Write)
708-
.Times(3)
708+
.Times(1)
709709
.WillRepeatedly([&](Requestconst& req, grpc::WriteOptions) {
710710
EXPECT_TRUE(req.has_append_object_spec());
711711
EXPECT_TRUE(req.append_object_spec().has_write_handle());
@@ -714,74 +714,40 @@ TEST(AsyncWriterConnectionTest, WriteHandleIsUpdatedAfterQuery) {
714714
return sequencer.PushBack("Write");
715715
});
716716

717-
int read_call_count =0;
718-
EXPECT_CALL(*mock, Read).Times(2).WillRepeatedly([&]() {
719-
Response resp;
720-
if (read_call_count ==0) {
721-
resp.mutable_write_handle()->set_handle("handle1");
722-
resp.set_persisted_size(42);
723-
}else {
724-
resp.mutable_write_handle()->set_handle("handle2");
725-
resp.set_persisted_size(43);
726-
}
727-
++read_call_count;
728-
returnmake_ready_future(absl::make_optional(std::move(resp)));
729-
});
730-
731-
auto hash = std::make_shared<MockHashFunction>();
732717
EXPECT_CALL(*mock, Cancel).Times(1);
733718
EXPECT_CALL(*mock, Finish).WillOnce([] {
734719
returnmake_ready_future(Status{});
735720
});
736-
EXPECT_CALL(*hash,Update(_, An<absl::Cordconst&>(), _)).Times(3);
721+
722+
auto hash = std::make_shared<MockHashFunction>();
723+
EXPECT_CALL(*hash,Update(_, An<absl::Cordconst&>(), _)).Times(1);
724+
EXPECT_CALL(*hash, Finish).Times(0);
737725

738726
google::storage::v2::BidiWriteObjectRequest req;
739727
req.mutable_append_object_spec()->set_bucket("bucket");
740728
req.mutable_append_object_spec()->set_object("object");
729+
req.mutable_append_object_spec()->mutable_write_handle()->set_handle(
730+
"test-handle");
741731

742732
auto tested = std::make_unique<AsyncWriterConnectionImpl>(
743733
TestOptions(), req,std::move(mock), hash,0);
744734

745-
// First Query sets handle1.
746-
EXPECT_THAT(tested->Query().get(),IsOkAndHolds(42));
735+
auto result = tested->Write(WritePayload("payload"));
736+
auto next = sequencer.PopFrontWithName();
737+
ASSERT_THAT(next.second,"Write");
738+
next.first.set_value(true);
739+
EXPECT_STATUS_OK(result.get());
747740

748-
// First Write uses handle1.
749-
auto result1 = tested->Write(WritePayload("payload1"));
750-
auto next1 = sequencer.PopFrontWithName();
751-
ASSERT_THAT(next1.second,"Write");
752-
next1.first.set_value(true);
753-
EXPECT_STATUS_OK(result1.get());
754-
755-
// Second Query sets handle2.
756-
EXPECT_THAT(tested->Query().get(),IsOkAndHolds(43));
757-
758-
// Second Write uses handle2.
759-
auto result2 = tested->Write(WritePayload("payload2"));
760-
auto next2 = sequencer.PopFrontWithName();
761-
ASSERT_THAT(next2.second,"Write");
762-
next2.first.set_value(true);
763-
EXPECT_STATUS_OK(result2.get());
764-
765-
// Third Write also uses handle2.
766-
auto result3 = tested->Write(WritePayload("payload3"));
767-
auto next3 = sequencer.PopFrontWithName();
768-
ASSERT_THAT(next3.second,"Write");
769-
next3.first.set_value(true);
770-
EXPECT_STATUS_OK(result3.get());
771-
772-
ASSERT_EQ(seen_handles.size(),3);
773-
EXPECT_EQ(seen_handles[0],"handle1");
774-
EXPECT_EQ(seen_handles[1],"handle2");
775-
EXPECT_EQ(seen_handles[2],"handle2");
741+
ASSERT_EQ(seen_handles.size(),1);
742+
EXPECT_EQ(seen_handles[0],"test-handle");
776743
}
777-
778-
TEST(AsyncWriterConnectionTest, WriteHandleIsUpdatedAfterResume) {
744+
TEST(AsyncWriterConnectionTest, QueryUpdatesHandle) {
779745
AsyncSequencer<bool> sequencer;
780746
auto mock = std::make_unique<MockStream>();
781747
std::vector<std::string> seen_handles;
782748

783749
EXPECT_CALL(*mock, Write)
784-
.Times(2)
750+
.Times(1)
785751
.WillRepeatedly([&](Requestconst& req, grpc::WriteOptions) {
786752
EXPECT_TRUE(req.has_append_object_spec());
787753
EXPECT_TRUE(req.append_object_spec().has_write_handle());
@@ -790,27 +756,20 @@ TEST(AsyncWriterConnectionTest, WriteHandleIsUpdatedAfterResume) {
790756
return sequencer.PushBack("Write");
791757
});
792758

793-
EXPECT_CALL(*mock, Read)
794-
.WillOnce([&]() {
795-
Response resp;
796-
resp.mutable_write_handle()->set_handle("handle1");
797-
resp.set_persisted_size(42);
798-
returnmake_ready_future(absl::make_optional(std::move(resp)));
799-
})
800-
.WillOnce([&]() {
801-
Response resp;
802-
resp.mutable_write_handle()->set_handle("handle2");
803-
resp.set_persisted_size(43);
804-
returnmake_ready_future(absl::make_optional(std::move(resp)));
805-
});
759+
EXPECT_CALL(*mock, Read).WillOnce([&]() {
760+
Response resp;
761+
resp.mutable_write_handle()->set_handle("queried-handle");
762+
resp.set_persisted_size(42);
763+
returnmake_ready_future(absl::make_optional(std::move(resp)));
764+
});
806765

807766
EXPECT_CALL(*mock, Cancel).Times(1);
808767
EXPECT_CALL(*mock, Finish).WillOnce([] {
809768
returnmake_ready_future(Status{});
810769
});
811770

812771
auto hash = std::make_shared<MockHashFunction>();
813-
EXPECT_CALL(*hash,Update(_, An<absl::Cordconst&>(), _)).Times(2);
772+
EXPECT_CALL(*hash,Update(_, An<absl::Cordconst&>(), _)).Times(1);
814773

815774
google::storage::v2::BidiWriteObjectRequest req;
816775
req.mutable_append_object_spec()->set_bucket("bucket");
@@ -819,28 +778,18 @@ TEST(AsyncWriterConnectionTest, WriteHandleIsUpdatedAfterResume) {
819778
auto tested = std::make_unique<AsyncWriterConnectionImpl>(
820779
TestOptions(), req,std::move(mock), hash,0);
821780

822-
//FirstQuerysets handle1.
781+
// Queryshould update the internal handle.
823782
EXPECT_THAT(tested->Query().get(),IsOkAndHolds(42));
824783

825-
// First Write uses handle1 but fails.
826-
auto result1 = tested->Write(WritePayload("payload1"));
827-
auto next1 = sequencer.PopFrontWithName();
828-
ASSERT_THAT(next1.second,"Write");
829-
next1.first.set_value(false);
830-
831-
// Simulate resume by calling Query again which returns handle2.
832-
EXPECT_THAT(tested->Query().get(),IsOkAndHolds(43));
833-
834-
// Second Write should use handle2.
835-
auto result2 = tested->Write(WritePayload("payload2"));
836-
auto next2 = sequencer.PopFrontWithName();
837-
ASSERT_THAT(next2.second,"Write");
838-
next2.first.set_value(true);
839-
EXPECT_STATUS_OK(result2.get());
840-
841-
ASSERT_EQ(seen_handles.size(),2);
842-
EXPECT_EQ(seen_handles[0],"handle1");
843-
EXPECT_EQ(seen_handles[1],"handle2");
784+
// Write should now use the handle from the Query response.
785+
auto result = tested->Write(WritePayload("payload"));
786+
auto next = sequencer.PopFrontWithName();
787+
ASSERT_THAT(next.second,"Write");
788+
next.first.set_value(true);
789+
EXPECT_STATUS_OK(result.get());
790+
791+
ASSERT_EQ(seen_handles.size(),1);
792+
EXPECT_EQ(seen_handles[0],"queried-handle");
844793
}
845794

846795
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp