Skip to content

Commit 139dd7e

Browse files
authored
fix(storage): Set the idempotency token for async rewrites (googleapis#16114)
1 parent 3cda322 commit 139dd7e

3 files changed

Lines changed: 64 additions & 1 deletion

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,12 @@ RewriterConnectionImpl::Iterate() {
4949
current_->get<storage::RetryPolicyOption>()->clone(),
5050
current_->get<storage::BackoffPolicyOption>()->clone(),
5151
policy->RewriteObject(request_), cq_,
52-
[stub = stub_](
52+
[stub = stub_, id = invocation_id_generator_.MakeInvocationId()](
5353
CompletionQueue& cq,
5454
std::shared_ptr<grpc::ClientContext> context,
5555
google::cloud::internal::ImmutableOptions options,
5656
google::storage::v2::RewriteObjectRequest const& proto) {
57+
AddIdempotencyToken(*context, id);
5758
return stub->AsyncRewriteObject(cq, std::move(context),
5859
std::move(options), proto);
5960
},

google/cloud/storage/internal/async/rewriter_connection_impl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "google/cloud/storage/options.h"
2121
#include "google/cloud/completion_queue.h"
2222
#include "google/cloud/future.h"
23+
#include "google/cloud/internal/invocation_id_generator.h"
2324
#include "google/cloud/status_or.h"
2425
#include "google/cloud/version.h"
2526
#include "google/storage/v2/storage.pb.h"
@@ -53,6 +54,7 @@ class RewriterConnectionImpl
5354
std::shared_ptr<StorageStub> stub_;
5455
google::cloud::internal::ImmutableOptions current_;
5556
google::storage::v2::RewriteObjectRequest request_;
57+
google::cloud::internal::InvocationIdGenerator invocation_id_generator_;
5658
};
5759

5860
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "google/cloud/internal/background_threads_impl.h"
2424
#include "google/cloud/testing_util/async_sequencer.h"
2525
#include "google/cloud/testing_util/status_matchers.h"
26+
#include "google/cloud/testing_util/validate_metadata.h"
2627
#include <gmock/gmock.h>
2728

2829
namespace google {
@@ -238,6 +239,65 @@ TEST(RewriterConnectionImplTest, TooManyTransients) {
238239
EXPECT_THAT(r1.get(), StatusIs(TransientError().status().code()));
239240
}
240241

242+
TEST(RewriterConnectionImplTest, IterateReusesIdempotencyTokenOnRetry) {
243+
google::cloud::testing_util::ValidateMetadataFixture
244+
validate_metadata_fixture;
245+
AsyncSequencer<bool> sequencer;
246+
auto mock = std::make_shared<MockStorageStub>();
247+
std::string first_token;
248+
249+
EXPECT_CALL(*mock, AsyncRewriteObject)
250+
.WillOnce([&](auto&, auto const& context, auto const&,
251+
google::storage::v2::RewriteObjectRequest const&) {
252+
auto metadata = validate_metadata_fixture.GetMetadata(*context);
253+
auto l = metadata.find("x-goog-gcs-idempotency-token");
254+
EXPECT_NE(l, metadata.end());
255+
if (l != metadata.end()) {
256+
first_token = l->second;
257+
EXPECT_FALSE(first_token.empty());
258+
}
259+
260+
return sequencer.PushBack("RewriteObject(1)").then([](auto) {
261+
return TransientError();
262+
});
263+
})
264+
.WillOnce([&](auto&, auto const& context, auto const&,
265+
google::storage::v2::RewriteObjectRequest const&) {
266+
auto metadata = validate_metadata_fixture.GetMetadata(*context);
267+
auto l = metadata.find("x-goog-gcs-idempotency-token");
268+
EXPECT_NE(l, metadata.end());
269+
if (l != metadata.end()) {
270+
EXPECT_EQ(l->second, first_token);
271+
}
272+
273+
return sequencer.PushBack("RewriteObject(2)").then([](auto) {
274+
google::storage::v2::RewriteResponse response;
275+
response.set_total_bytes_rewritten(1000);
276+
response.set_object_size(3000);
277+
response.set_rewrite_token("test-rewrite-token");
278+
return make_status_or(response);
279+
});
280+
});
281+
282+
internal::AutomaticallyCreatedBackgroundThreads pool(1);
283+
auto connection = std::make_shared<RewriterConnectionImpl>(
284+
pool.cq(), std::move(mock), TestOptions(), MakeRequest());
285+
286+
auto r1 = connection->Iterate();
287+
auto next = sequencer.PopFrontWithName();
288+
EXPECT_EQ(next.second, "RewriteObject(1)");
289+
next.first.set_value(true);
290+
next = sequencer.PopFrontWithName();
291+
EXPECT_EQ(next.second, "RewriteObject(2)");
292+
next.first.set_value(true);
293+
EXPECT_THAT(
294+
r1.get(),
295+
IsOkAndHolds(ResultOf(
296+
"total bytes",
297+
[](RewriteResponse const& v) { return v.total_bytes_rewritten(); },
298+
1000)));
299+
}
300+
241301
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
242302
} // namespace storage_internal
243303
} // namespace cloud

0 commit comments

Comments
 (0)