Skip to content

Commit b9e74bb

Browse files
authored
fix(storage): consume server responses before finishing stream in AsyncWriter Close/Finalize (#16270)
1 parent d1c37cc commit b9e74bb

6 files changed

Lines changed: 671 additions & 262 deletions

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

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,12 @@ std::unique_ptr<MockAsyncBidiWriteObjectStream> MakeCommonAppendStream(
104104
response.mutable_resource()->set_size(persisted_size + 1024);
105105
return std::make_optional(std::move(response));
106106
});
107+
})
108+
// The third `Read()` call returns EOF.
109+
.WillOnce([&] {
110+
return sequencer.PushBack("Read(EOF)").then([](auto) {
111+
return std::optional<google::storage::v2::BidiWriteObjectResponse>();
112+
});
107113
});
108114

109115
EXPECT_CALL(*stream, Cancel).Times(1);
@@ -304,6 +310,12 @@ TEST_F(AsyncConnectionImplAppendableTest, StartAppendableObjectUploadSuccess) {
304310
next = sequencer.PopFrontWithName();
305311
EXPECT_EQ(next.second, "Read(FinalObject)");
306312
next.first.set_value(true);
313+
next = sequencer.PopFrontWithName();
314+
EXPECT_EQ(next.second, "Read(EOF)");
315+
next.first.set_value(true);
316+
next = sequencer.PopFrontWithName();
317+
EXPECT_EQ(next.second, "Finish");
318+
next.first.set_value(true);
307319

308320
auto response = w2.get();
309321
ASSERT_STATUS_OK(response);
@@ -312,9 +324,6 @@ TEST_F(AsyncConnectionImplAppendableTest, StartAppendableObjectUploadSuccess) {
312324
EXPECT_EQ(response->size(), 1024);
313325

314326
writer.reset();
315-
next = sequencer.PopFrontWithName();
316-
EXPECT_EQ(next.second, "Finish");
317-
next.first.set_value(true);
318327
}
319328

320329
TEST_F(AsyncConnectionImplAppendableTest, ResumeAppendableObjectUploadSuccess) {
@@ -370,15 +379,18 @@ TEST_F(AsyncConnectionImplAppendableTest, ResumeAppendableObjectUploadSuccess) {
370379
next = sequencer.PopFrontWithName();
371380
EXPECT_EQ(next.second, "Read(FinalObject)");
372381
next.first.set_value(true);
382+
next = sequencer.PopFrontWithName();
383+
EXPECT_EQ(next.second, "Read(EOF)");
384+
next.first.set_value(true);
385+
next = sequencer.PopFrontWithName();
386+
EXPECT_EQ(next.second, "Finish");
387+
next.first.set_value(true);
373388

374389
auto response = w2.get();
375390
ASSERT_STATUS_OK(response);
376391
EXPECT_EQ(response->size(), kPersistedSize + 1024);
377392

378393
writer.reset();
379-
next = sequencer.PopFrontWithName();
380-
EXPECT_EQ(next.second, "Finish");
381-
next.first.set_value(true);
382394
}
383395

384396
TEST_F(AsyncConnectionImplAppendableTest, AppendableUploadTooManyTransients) {
@@ -517,6 +529,12 @@ TEST_F(AsyncConnectionImplAppendableTest, AppendableUploadRedirect) {
517529
next = sequencer.PopFrontWithName();
518530
EXPECT_EQ(next.second, "Read(FinalObject)");
519531
next.first.set_value(true);
532+
next = sequencer.PopFrontWithName();
533+
EXPECT_EQ(next.second, "Read(EOF)");
534+
next.first.set_value(true);
535+
next = sequencer.PopFrontWithName();
536+
EXPECT_EQ(next.second, "Finish");
537+
next.first.set_value(true);
520538

521539
auto response = w2.get();
522540
ASSERT_STATUS_OK(response);
@@ -525,9 +543,6 @@ TEST_F(AsyncConnectionImplAppendableTest, AppendableUploadRedirect) {
525543
EXPECT_EQ(response->size(), 1024 + 1024);
526544

527545
writer.reset();
528-
next = sequencer.PopFrontWithName();
529-
EXPECT_EQ(next.second, "Finish");
530-
next.first.set_value(true);
531546
}
532547

533548
TEST_F(AsyncConnectionImplAppendableTest, AppendableUploadRedirectNoHandle) {
@@ -614,6 +629,12 @@ TEST_F(AsyncConnectionImplAppendableTest, AppendableUploadRedirectNoHandle) {
614629
next = sequencer.PopFrontWithName();
615630
EXPECT_EQ(next.second, "Read(FinalObject)");
616631
next.first.set_value(true);
632+
next = sequencer.PopFrontWithName();
633+
EXPECT_EQ(next.second, "Read(EOF)");
634+
next.first.set_value(true);
635+
next = sequencer.PopFrontWithName();
636+
EXPECT_EQ(next.second, "Finish");
637+
next.first.set_value(true);
617638

618639
auto response = w2.get();
619640
ASSERT_STATUS_OK(response);
@@ -622,9 +643,6 @@ TEST_F(AsyncConnectionImplAppendableTest, AppendableUploadRedirectNoHandle) {
622643
EXPECT_EQ(response->size(), 1024 + 1024);
623644

624645
writer.reset();
625-
next = sequencer.PopFrontWithName();
626-
EXPECT_EQ(next.second, "Finish");
627-
next.first.set_value(true);
628646
}
629647
} // namespace
630648
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END

0 commit comments

Comments
 (0)