Skip to content

Commit 8480f80

Browse files
coryancoryan
authored
cleanup(GCS+gRPC): protos for AsyncDeleteObject() (#13978)
1 parent 8652972 commit 8480f80

File tree

13 files changed

+216
-115
lines changed

13 files changed

+216
-115
lines changed

‎google/cloud/storage/async/client.cc‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,33 @@ AsyncClient::AsyncClient(Options options) {
3838
AsyncClient::AsyncClient(std::shared_ptr<AsyncConnection> connection)
3939
: connection_(std::move(connection)) {}
4040

41+
future<Status> AsyncClient::DeleteObject(BucketName const& bucket_name,
42+
std::string object_name,
43+
Options opts) {
44+
google::storage::v2::DeleteObjectRequest request;
45+
request.set_bucket(bucket_name.FullName());
46+
request.set_object(object_name);
47+
return DeleteObject(std::move(request), std::move(opts));
48+
}
49+
50+
future<Status> AsyncClient::DeleteObject(BucketName const& bucket_name,
51+
std::string object_name,
52+
std::int64_t generation,
53+
Options opts) {
54+
google::storage::v2::DeleteObjectRequest request;
55+
request.set_bucket(bucket_name.FullName());
56+
request.set_object(object_name);
57+
request.set_generation(generation);
58+
return DeleteObject(std::move(request), std::move(opts));
59+
}
60+
61+
future<Status> AsyncClient::DeleteObject(
62+
google::storage::v2::DeleteObjectRequest request, Options opts) {
63+
return connection_->DeleteObject(
64+
{std::move(request),
65+
internal::MergeOptions(std::move(opts), connection_->options())});
66+
}
67+
4168
std::pair<AsyncRewriter, AsyncToken> AsyncClient::StartRewrite(
4269
BucketName const& source_bucket, std::string source_object_name,
4370
BucketName const& destination_bucket, std::string destination_object_name,

‎google/cloud/storage/async/client.h‎

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -535,34 +535,58 @@ class AsyncClient {
535535
std::move(options)});
536536
}
537537

538+
/*
539+
[delete-object-common]
540+
@par Example
541+
@snippet storage_async_samples.cc delete-object
542+
543+
@par Idempotency
544+
This operation is only idempotent if:
545+
- restricted by pre-conditions, in this case, `IfGenerationMatch`
546+
- or, if it applies to only one object version via `Generation`.
547+
[delete-object-common]
548+
*/
538549
/**
539-
* Deletes an object.
550+
* @brief Deletes an object
551+
*
552+
* @snippet{doc} async/client.h delete-object-common
540553
*
541554
* @param bucket_name the name of the bucket that contains the object.
542-
* @param object_name the name of the object to be deleted.
543-
* @param args a list of optional query parameters and/or request headers.
544-
* Valid types for this operation include `Generation`,
545-
* `IfGenerationMatch`, `IfGenerationNotMatch`, `IfMetagenerationMatch`,
546-
* `IfMetagenerationNotMatch`, and `UserProject`.
547-
* See the class description for common request options.
555+
* @param object_name the name of the object to delete.
556+
* @param opts options controlling the behavior of this RPC, for example
557+
* the application may change the retry policy.
558+
*/
559+
future<Status> DeleteObject(BucketName const& bucket_name,
560+
std::string object_name, Options opts = {});
561+
562+
/**
563+
* @brief Deletes an object
548564
*
549-
* @par Idempotency
550-
* This operation is only idempotent if:
551-
* - restricted by pre-conditions, in this case, `IfGenerationMatch`
552-
* - or, if it applies to only one object version via `Generation`.
565+
* @snippet{doc} async/client.h delete-object-common
553566
*
554-
* @par Example
555-
* @snippet storage_async_samples.cc delete-object
567+
* @param bucket_name the name of the bucket that contains the object.
568+
* @param object_name the name of the object to delete.
569+
* @param generation the object generation to delete.
570+
* @param opts options controlling the behavior of this RPC, for example
571+
* the application may change the retry policy.
556572
*/
557-
template <typename... Args>
558-
future<Status> DeleteObject(std::string bucket_name, std::string object_name,
559-
Args&&... args) {
560-
auto options = SpanOptions(std::forward<Args>(args)...);
561-
return connection_->DeleteObject(
562-
{DeleteObjectRequest(std::move(bucket_name), std::move(object_name))
563-
.set_multiple_options(std::forward<Args>(args)...),
564-
/*.options=*/std::move(options)});
565-
}
573+
future<Status> DeleteObject(BucketName const& bucket_name,
574+
std::string object_name, std::int64_t generation,
575+
Options opts = {});
576+
577+
/**
578+
* @brief Deletes an object
579+
*
580+
* @snippet{doc} async/client.h delete-object-common
581+
*
582+
* @param request the full request describing what object to delete. It may
583+
* also include any preconditions the object must satisfy, and other
584+
* parameters that are necessary to complete the RPC.
585+
* @param opts options controlling the behavior of this RPC, for example
586+
* the application may change the retry policy.
587+
*/
588+
future<Status> DeleteObject(google::storage::v2::DeleteObjectRequest request,
589+
Options opts = {});
566590

567591
/*
568592
[start-rewrite-common]

‎google/cloud/storage/async/client_test.cc‎

Lines changed: 78 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ TEST(AsyncClient, ComposeObject) {
334334
EXPECT_THAT(response, IsOkAndHolds(TestObject()));
335335
}
336336

337-
TEST(AsyncClient, DeleteObject) {
337+
TEST(AsyncClient, DeleteObject1) {
338338
auto mock = std::make_shared<MockAsyncConnection>();
339339
EXPECT_CALL(*mock, options)
340340
.WillRepeatedly(
@@ -345,17 +345,88 @@ TEST(AsyncClient, DeleteObject) {
345345
EXPECT_THAT(p.options.get<TestOption<0>>(), "O0");
346346
EXPECT_THAT(p.options.get<TestOption<1>>(), "O1-function");
347347
EXPECT_THAT(p.options.get<TestOption<2>>(), "O2-function");
348-
EXPECT_EQ(p.request.bucket_name(), "test-bucket");
349-
EXPECT_EQ(p.request.object_name(), "test-object");
350-
EXPECT_EQ(p.request.GetOption<storage::IfGenerationMatch>().value_or(0),
351-
42);
348+
auto constexpr kExpected = R"pb(
349+
bucket: "projects/_/buckets/test-bucket"
350+
object: "test-object"
351+
)pb";
352+
google::storage::v2::DeleteObjectRequest expected;
353+
EXPECT_TRUE(TextFormat::ParseFromString(kExpected, &expected));
354+
EXPECT_THAT(p.request, IsProtoEqual(expected));
352355
return make_ready_future(Status{});
353356
});
354357

355358
auto client = AsyncClient(mock);
356359
auto response = client
357-
.DeleteObject("test-bucket", "test-object",
358-
storage::IfGenerationMatch(42),
360+
.DeleteObject(BucketName("test-bucket"), "test-object",
361+
Options{}
362+
.set<TestOption<1>>("O1-function")
363+
.set<TestOption<2>>("O2-function"))
364+
.get();
365+
EXPECT_THAT(response, IsOk());
366+
}
367+
368+
TEST(AsyncClient, DeleteObject2) {
369+
auto mock = std::make_shared<MockAsyncConnection>();
370+
EXPECT_CALL(*mock, options)
371+
.WillRepeatedly(
372+
Return(Options{}.set<TestOption<0>>("O0").set<TestOption<1>>("O1")));
373+
374+
EXPECT_CALL(*mock, DeleteObject)
375+
.WillOnce([](AsyncConnection::DeleteObjectParams const& p) {
376+
EXPECT_THAT(p.options.get<TestOption<0>>(), "O0");
377+
EXPECT_THAT(p.options.get<TestOption<1>>(), "O1-function");
378+
EXPECT_THAT(p.options.get<TestOption<2>>(), "O2-function");
379+
auto constexpr kExpected = R"pb(
380+
bucket: "projects/_/buckets/test-bucket"
381+
object: "test-object"
382+
generation: 12345
383+
)pb";
384+
google::storage::v2::DeleteObjectRequest expected;
385+
EXPECT_TRUE(TextFormat::ParseFromString(kExpected, &expected));
386+
EXPECT_THAT(p.request, IsProtoEqual(expected));
387+
return make_ready_future(Status{});
388+
});
389+
390+
auto client = AsyncClient(mock);
391+
auto response =
392+
client
393+
.DeleteObject(BucketName("test-bucket"), "test-object", 12345,
394+
Options{}
395+
.set<TestOption<1>>("O1-function")
396+
.set<TestOption<2>>("O2-function"))
397+
.get();
398+
EXPECT_THAT(response, IsOk());
399+
}
400+
401+
TEST(AsyncClient, DeleteObject3) {
402+
auto mock = std::make_shared<MockAsyncConnection>();
403+
EXPECT_CALL(*mock, options)
404+
.WillRepeatedly(
405+
Return(Options{}.set<TestOption<0>>("O0").set<TestOption<1>>("O1")));
406+
407+
EXPECT_CALL(*mock, DeleteObject)
408+
.WillOnce([](AsyncConnection::DeleteObjectParams const& p) {
409+
EXPECT_THAT(p.options.get<TestOption<0>>(), "O0");
410+
EXPECT_THAT(p.options.get<TestOption<1>>(), "O1-function");
411+
EXPECT_THAT(p.options.get<TestOption<2>>(), "O2-function");
412+
auto constexpr kExpected = R"pb(
413+
bucket: "test-only-invalid"
414+
object: "test-object"
415+
if_generation_match: 42
416+
)pb";
417+
google::storage::v2::DeleteObjectRequest expected;
418+
EXPECT_TRUE(TextFormat::ParseFromString(kExpected, &expected));
419+
EXPECT_THAT(p.request, IsProtoEqual(expected));
420+
return make_ready_future(Status{});
421+
});
422+
423+
auto client = AsyncClient(mock);
424+
google::storage::v2::DeleteObjectRequest request;
425+
request.set_bucket("test-only-invalid");
426+
request.set_object("test-object");
427+
request.set_if_generation_match(42);
428+
auto response = client
429+
.DeleteObject(std::move(request),
359430
Options{}
360431
.set<TestOption<1>>("O1-function")
361432
.set<TestOption<2>>("O2-function"))

‎google/cloud/storage/async/connection.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ class AsyncConnection {
153153
struct DeleteObjectParams {
154154
/// The bucket and object name for the object to be deleted. Including
155155
/// pre-conditions on the object and other optional parameters.
156-
DeleteObjectRequest request;
156+
google::storage::v2::DeleteObjectRequest request;
157157
/// Any options modifying the RPC behavior, including per-client and
158158
/// per-connection options.
159159
Options options;

‎google/cloud/storage/async/object_requests.h‎

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -218,45 +218,6 @@ class ReadObjectRequest {
218218
storage::internal::ReadObjectRangeRequest impl_;
219219
};
220220

221-
/**
222-
* A request to delete an object.
223-
*
224-
* This class can hold all the mandatory and optional parameters to delete an
225-
* object. This class is in the public API because it is required for mocking.
226-
*/
227-
class DeleteObjectRequest {
228-
public:
229-
DeleteObjectRequest() = default;
230-
DeleteObjectRequest(std::string bucket_name, std::string object_name)
231-
: impl_(std::move(bucket_name), std::move(object_name)) {}
232-
233-
std::string const& bucket_name() const { return impl_.bucket_name(); }
234-
std::string const& object_name() const { return impl_.object_name(); }
235-
236-
template <typename... T>
237-
DeleteObjectRequest& set_multiple_options(T&&... o) & {
238-
impl_.set_multiple_options(std::forward<T>(o)...);
239-
return *this;
240-
}
241-
template <typename... T>
242-
DeleteObjectRequest&& set_multiple_options(T&&... o) && {
243-
return std::move(set_multiple_options(std::forward<T>(o)...));
244-
}
245-
246-
template <typename T>
247-
bool HasOption() const {
248-
return impl_.HasOption<T>();
249-
}
250-
template <typename T>
251-
T GetOption() const {
252-
return impl_.GetOption<T>();
253-
}
254-
255-
protected:
256-
friend class storage_internal::AsyncConnectionImpl;
257-
storage::internal::DeleteObjectRequest impl_;
258-
};
259-
260221
/**
261222
* A request to compose multiple objects into a single object.
262223
*

‎google/cloud/storage/benchmarks/async_throughput_benchmark.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -760,8 +760,9 @@ void RunBenchmark(Configuration const& cfg) {
760760
std::vector<std::string> names) -> g::future<void> {
761761
std::vector<g::future<g::Status>> pending(names.size());
762762
std::transform(
763-
names.begin(), names.end(), pending.begin(),
764-
[&](auto const& name) { return client.DeleteObject(bucket, name); });
763+
names.begin(), names.end(), pending.begin(), [&](auto const& name) {
764+
return client.DeleteObject(gcs_ex::BucketName(bucket), name);
765+
});
765766
names.clear();
766767
for (auto& p : pending) co_await std::move(p);
767768
};

‎google/cloud/storage/examples/storage_async_mock_samples.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ TEST(StorageAsyncMockingSamples, MockDeleteObject) {
3737
.WillOnce(Return(ByMove(gc::make_ready_future(gc::Status{}))));
3838

3939
auto client = gcs_ex::AsyncClient(mock);
40-
auto actual = client.DeleteObject("test-bucket", "test-object").get();
40+
auto actual =
41+
client.DeleteObject(gcs_ex::BucketName("test-bucket"), "test-object")
42+
.get();
4143
EXPECT_TRUE(actual.ok());
4244
}
4345
//! [mock-async-delete-object]

‎google/cloud/storage/examples/storage_async_samples.cc‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,9 @@ void AsyncDeleteObject(google::cloud::storage_experimental::AsyncClient& client,
488488
namespace gcs_ex = google::cloud::storage_experimental;
489489
[](gcs_ex::AsyncClient& client, std::string bucket_name,
490490
std::string object_name) {
491-
client.DeleteObject(std::move(bucket_name), std::move(object_name))
491+
client
492+
.DeleteObject(gcs_ex::BucketName(std::move(bucket_name)),
493+
std::move(object_name))
492494
.then([](auto f) {
493495
auto status = f.get();
494496
if (!status.ok()) throw g::Status(std::move(status));
@@ -621,14 +623,15 @@ void AutoRun(std::vector<std::string> const& argv) {
621623

622624
namespace g = ::google::cloud;
623625
std::vector<g::future<g::Status>> pending;
624-
pending.push_back(client.DeleteObject(bucket_name, o1));
625-
pending.push_back(client.DeleteObject(bucket_name, o2));
626-
pending.push_back(client.DeleteObject(bucket_name, o3));
627-
pending.push_back(client.DeleteObject(bucket_name, o4));
628-
pending.push_back(client.DeleteObject(bucket_name, o5));
629-
pending.push_back(client.DeleteObject(bucket_name, o6));
630-
pending.push_back(client.DeleteObject(bucket_name, o7));
631-
pending.push_back(client.DeleteObject(bucket_name, o8));
626+
auto bucket = google::cloud::storage_experimental::BucketName(bucket_name);
627+
pending.push_back(client.DeleteObject(bucket, o1));
628+
pending.push_back(client.DeleteObject(bucket, o2));
629+
pending.push_back(client.DeleteObject(bucket, o3));
630+
pending.push_back(client.DeleteObject(bucket, o4));
631+
pending.push_back(client.DeleteObject(bucket, o5));
632+
pending.push_back(client.DeleteObject(bucket, o6));
633+
pending.push_back(client.DeleteObject(bucket, o7));
634+
pending.push_back(client.DeleteObject(bucket, o8));
632635
for (auto& f : pending) (void)f.get();
633636
}
634637

‎google/cloud/storage/examples/storage_otel_samples.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ void InstrumentedClient(std::vector<std::string> const& argv) {
6969
}
7070
std::cout << "Counted " << count << " 7's in the GCS object\n";
7171

72-
auto deleted = co_await client.DeleteObject(bucket_name, object_name);
72+
auto deleted = co_await client.DeleteObject(
73+
gcs_ex::BucketName(bucket_name), object_name);
7374
if (!deleted.ok()) throw gc::Status(std::move(deleted));
7475

7576
co_return;

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414

1515
#include "google/cloud/storage/internal/async/connection_impl.h"
16+
#include "google/cloud/storage/async/idempotency_policy.h"
1617
#include "google/cloud/storage/async/resume_policy.h"
1718
#include "google/cloud/storage/internal/async/accumulate_read_object.h"
1819
#include "google/cloud/storage/internal/async/default_options.h"
@@ -65,6 +66,11 @@ inline std::unique_ptr<storage::IdempotencyPolicy> legacy_idempotency_policy(
6566
return options.get<storage::IdempotencyPolicyOption>()->clone();
6667
}
6768

69+
inline std::unique_ptr<storage_experimental::IdempotencyPolicy>
70+
idempotency_policy(Options const& options) {
71+
return options.get<storage_experimental::IdempotencyPolicyOption>()();
72+
}
73+
6874
} // namespace
6975

7076
AsyncConnectionImpl::AsyncConnectionImpl(
@@ -269,24 +275,20 @@ future<StatusOr<storage::ObjectMetadata>> AsyncConnectionImpl::ComposeObject(
269275

270276
future<Status> AsyncConnectionImpl::DeleteObject(DeleteObjectParams p) {
271277
auto current = internal::MakeImmutableOptions(std::move(p.options));
272-
auto proto = ToProto(p.request.impl_);
273278
auto const idempotency =
274-
legacy_idempotency_policy(*current)->IsIdempotent(p.request.impl_)
275-
? Idempotency::kIdempotent
276-
: Idempotency::kNonIdempotent;
279+
idempotency_policy(*current)->DeleteObject(p.request);
277280
auto retry = retry_policy(*current);
278281
auto backoff = backoff_policy(*current);
279282
return google::cloud::internal::AsyncRetryLoop(
280283
std::move(retry), std::move(backoff), idempotency, cq_,
281-
[stub = stub_, request = std::move(p.request)](
282-
CompletionQueue& cq, std::shared_ptr<grpc::ClientContext> context,
283-
google::cloud::internal::ImmutableOptions options,
284-
google::storage::v2::DeleteObjectRequest const& proto) {
285-
ApplyQueryParameters(*context, *options, request);
284+
[stub = stub_](CompletionQueue& cq,
285+
std::shared_ptr<grpc::ClientContext> context,
286+
google::cloud::internal::ImmutableOptions options,
287+
google::storage::v2::DeleteObjectRequest const& proto) {
286288
return stub->AsyncDeleteObject(cq, std::move(context),
287289
std::move(options), proto);
288290
},
289-
std::move(current), proto, __func__);
291+
std::move(current), std::move(p.request), __func__);
290292
}
291293

292294
std::shared_ptr<storage_experimental::AsyncRewriterConnection>

0 commit comments

Comments
 (0)