feat(storage): Add check for writeHandle before reansforming write_object_spec to append_object_spec (#15224)

* Add check for writeHandle before reansforming write_object_spec to append_object_spec

* Correct the formatting

* rename the any variable to rpc_status_detail

* Format write_connection_impl.cc file

Author: v-pratap

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

@@ -360,9 +360,11 @@ AsyncConnectionImpl::AppendableObjectUploadImpl(AppendableUploadParams p) {
             open.reset();
             auto response = f.get();
             if (!response) {
-              EnsureFirstMessageAppendObjectSpec(request);
+              google::rpc::Status grpc_status =
+                  ExtractGrpcStatus(response.status());
+              EnsureFirstMessageAppendObjectSpec(request, grpc_status);
               ApplyWriteRedirectErrors(*request.mutable_append_object_spec(),
-                                       ExtractGrpcStatus(response.status()));
+                                       grpc_status);
             }
             return response;
           });

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

@@ -21,12 +21,23 @@ namespace storage_internal {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 
 void EnsureFirstMessageAppendObjectSpec(
-    google::storage::v2::BidiWriteObjectRequest& request) {
-  if (request.has_write_object_spec()) {
-    auto spec = request.write_object_spec();
-    auto& append_object_spec = *request.mutable_append_object_spec();
-    append_object_spec.set_bucket(spec.resource().bucket());
-    append_object_spec.set_object(spec.resource().name());
+    google::storage::v2::BidiWriteObjectRequest& request,
+    google::rpc::Status const& rpc_status) {
+  for (auto const& rpc_status_detail : rpc_status.details()) {
+    google::storage::v2::BidiWriteObjectRedirectedError error =
+        google::storage::v2::BidiWriteObjectRedirectedError{};
+    if (!rpc_status_detail.UnpackTo(&error)) continue;
+    if (!error.has_write_handle()) continue;
+    if (request.has_write_object_spec()) {
+      auto spec = request.write_object_spec();
+      auto& append_object_spec = *request.mutable_append_object_spec();
+      append_object_spec.set_bucket(spec.resource().bucket());
+      append_object_spec.set_object(spec.resource().name());
+      append_object_spec.set_if_metageneration_match(
+          spec.if_metageneration_match());
+      append_object_spec.set_if_metageneration_not_match(
+          spec.if_metageneration_not_match());
+    }
   }
 }
 

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

@@ -26,7 +26,8 @@ namespace storage_internal {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 
 void EnsureFirstMessageAppendObjectSpec(
-    google::storage::v2::BidiWriteObjectRequest& request);
+    google::storage::v2::BidiWriteObjectRequest& request,
+    google::rpc::Status const& rpc_status);
 
 google::rpc::Status ExtractGrpcStatus(Status const& status);
 

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

@@ -70,6 +70,60 @@ TEST(ApplyRedirectErrors, NoRedirect) {
   EXPECT_TRUE(spec.routing_token().empty());
 }
 
+TEST(EnsureFirstMessageAppendObjectSpec, Success) {
+  google::storage::v2::BidiWriteObjectRequest request;
+  ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
+      R"pb(
+        write_object_spec {
+          resource { bucket: "projects/_/buckets/b", name: "o" }
+          if_metageneration_match: 1
+          if_metageneration_not_match: 1
+        }
+      )pb",
+      &request));
+
+  google::rpc::Status rpc_status;
+  google::storage::v2::BidiWriteObjectRedirectedError redirect;
+  redirect.mutable_write_handle();
+  rpc_status.add_details()->PackFrom(redirect);
+
+  EnsureFirstMessageAppendObjectSpec(request, rpc_status);
+
+  EXPECT_FALSE(request.has_write_object_spec());
+  EXPECT_TRUE(request.has_append_object_spec());
+
+  auto const& append_spec = request.append_object_spec();
+  EXPECT_EQ(append_spec.bucket(), "projects/_/buckets/b");
+  EXPECT_EQ(append_spec.object(), "o");
+
+  EXPECT_FALSE(append_spec.has_write_handle());
+  EXPECT_TRUE(append_spec.routing_token().empty());
+  EXPECT_EQ(append_spec.if_metageneration_match(), 1);
+  EXPECT_EQ(append_spec.if_metageneration_not_match(), 1);
+  EXPECT_EQ(append_spec.generation(), 0);
+}
+
+TEST(EnsureFirstMessageAppendObjectSpec, WriteHandleIsNotSet) {
+  google::storage::v2::BidiWriteObjectRequest request;
+  ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
+      R"pb(
+        write_object_spec {
+          resource { bucket: "projects/_/buckets/b", name: "o" }
+        }
+      )pb",
+      &request));
+
+  google::rpc::Status rpc_status;
+  google::storage::v2::BidiWriteObjectRedirectedError redirect;
+  redirect.set_generation(1234);
+  rpc_status.add_details()->PackFrom(redirect);
+
+  EnsureFirstMessageAppendObjectSpec(request, rpc_status);
+
+  EXPECT_TRUE(request.has_write_object_spec());
+  EXPECT_FALSE(request.has_append_object_spec());
+}
+
 }  // namespace
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace storage_internal

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

@@ -234,9 +234,10 @@ future<StatusOr<std::int64_t>> AsyncWriterConnectionImpl::OnQuery(
             "Expected error in Finish() after non-ok Read()"))
         .then([this](auto g) {
           auto result = g.get();
-          EnsureFirstMessageAppendObjectSpec(request_);
+          google::rpc::Status grpc_status = ExtractGrpcStatus(result);
+          EnsureFirstMessageAppendObjectSpec(request_, grpc_status);
           ApplyWriteRedirectErrors(*request_.mutable_append_object_spec(),
-                                   ExtractGrpcStatus(result));
+                                   grpc_status);
           return StatusOr<std::int64_t>(std::move(result));
         });
   }