Skip to content

Commit f32b972

Browse files
authored
refactor: ScopedDeleter doesn't require metadata (#3380)
Before the change, `ScopedDeleter` kept `ObjectMetadata` of all object to delete. This was unnecessary - object names and generation numbers are enough, so this change stores them instead of the metadata. This is useful when the caller has the information but doesn't have the full metadata.
1 parent 864b463 commit f32b972

File tree

5 files changed

+30
-21
lines changed

5 files changed

+30
-21
lines changed
0 Bytes
Binary file not shown.
943 Bytes
Binary file not shown.

google/cloud/storage/client.cc

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -344,17 +344,23 @@ std::string CreateRandomPrefixName(std::string const& prefix) {
344344

345345
namespace internal {
346346

347-
ScopedDeleter::ScopedDeleter(std::function<Status(ObjectMetadata)> delete_fun)
347+
ScopedDeleter::ScopedDeleter(
348+
std::function<Status(std::string, std::int64_t)> delete_fun)
348349
: delete_fun_(std::move(delete_fun)) {}
349350

350351
ScopedDeleter::~ScopedDeleter() { ExecuteDelete(); }
351352

352353
void ScopedDeleter::Add(ObjectMetadata object) {
353-
object_list_.emplace_back(std::move(object));
354+
auto generation = object.generation();
355+
Add(std::move(object).name(), generation);
356+
}
357+
358+
void ScopedDeleter::Add(std::string object_name, std::int64_t generation) {
359+
object_list_.emplace_back(std::move(object_name), generation);
354360
}
355361

356362
Status ScopedDeleter::ExecuteDelete() {
357-
std::vector<ObjectMetadata> object_list;
363+
std::vector<std::pair<std::string, std::int64_t>> object_list;
358364
// make sure the dtor will not do this again
359365
object_list.swap(object_list_);
360366

@@ -363,7 +369,7 @@ Status ScopedDeleter::ExecuteDelete() {
363369
// last.
364370
for (auto object_it = object_list.rbegin(); object_it != object_list.rend();
365371
++object_it) {
366-
Status status = delete_fun_(std::move(*object_it));
372+
Status status = delete_fun_(std::move(object_it->first), object_it->second);
367373
// Fail on first error. If the service is unavailable, every deletion
368374
// would potentially keep retrying until the timeout passes - this would
369375
// take way too much time and would be pointless.

google/cloud/storage/client.h

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3197,20 +3197,23 @@ class ScopedDeleter {
31973197
public:
31983198
// The actual deletion depends on local's types in a very non-trivial way,
31993199
// so we abstract this away by providing the function to delete one object.
3200-
ScopedDeleter(std::function<Status(ObjectMetadata)> delete_fun);
3200+
ScopedDeleter(std::function<Status(std::string, std::int64_t)> delete_fun);
32013201
ScopedDeleter(ScopedDeleter const&) = delete;
32023202
ScopedDeleter& operator=(ScopedDeleter const&) = delete;
32033203
~ScopedDeleter();
32043204

32053205
/// Defer object's deletion to this objects destruction (or ExecuteDelete())
32063206
void Add(ObjectMetadata object);
32073207

3208+
/// Defer object's deletion to this objects destruction (or ExecuteDelete())
3209+
void Add(std::string object_name, std::int64_t generation);
3210+
32083211
/// Execute all the deferred deletions now.
32093212
Status ExecuteDelete();
32103213

32113214
private:
3212-
std::function<Status(ObjectMetadata)> delete_fun_;
3213-
std::vector<ObjectMetadata> object_list_;
3215+
std::function<Status(std::string, std::int64_t)> delete_fun_;
3216+
std::vector<std::pair<std::string, std::int64_t>> object_list_;
32143217
};
32153218

32163219
} // namespace internal
@@ -3288,14 +3291,15 @@ StatusOr<ObjectMetadata> ComposeMany(
32883291
"EncryptionKey, IfGenerationMatch, IfMetagenerationMatch, KmsKeyName, "
32893292
"QuotaUser, UserIp, UserProject or WithObjectMetadata.");
32903293

3291-
internal::ScopedDeleter deleter([&](ObjectMetadata const& object) {
3292-
return google::cloud::internal::apply(
3293-
internal::DeleteApplyHelper{client, bucket_name, object.name()},
3294-
std::tuple_cat(
3295-
std::make_tuple(IfGenerationMatch(object.generation())),
3296-
StaticTupleFilter<Among<QuotaUser, UserProject, UserIp>::TPred>(
3297-
all_options)));
3298-
});
3294+
internal::ScopedDeleter deleter(
3295+
[&](std::string const& object_name, std::int64_t generation) {
3296+
return google::cloud::internal::apply(
3297+
internal::DeleteApplyHelper{client, bucket_name, object_name},
3298+
std::tuple_cat(
3299+
std::make_tuple(IfGenerationMatch(generation)),
3300+
StaticTupleFilter<Among<QuotaUser, UserProject, UserIp>::TPred>(
3301+
all_options)));
3302+
});
32993303

33003304
auto lock = internal::LockPrefix(client, bucket_name, prefix, "",
33013305
std::make_tuple(options...));

google/cloud/storage/parallel_upload.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -276,13 +276,12 @@ NonResumableParallelUploadState::Create(Client client,
276276
StaticTupleFilter<Among<QuotaUser, UserProject, UserIp>::TPred>(
277277
all_options);
278278
auto deleter = google::cloud::internal::make_unique<ScopedDeleter>(
279-
[client, bucket_name,
280-
delete_options](ObjectMetadata const& object) mutable {
279+
[client, bucket_name, delete_options](std::string const& object_name,
280+
std::int64_t generation) mutable {
281281
return google::cloud::internal::apply(
282-
DeleteApplyHelper{client, std::move(bucket_name), object.name()},
283-
std::tuple_cat(
284-
std::make_tuple(IfGenerationMatch(object.generation())),
285-
std::move(delete_options)));
282+
DeleteApplyHelper{client, std::move(bucket_name), object_name},
283+
std::tuple_cat(std::make_tuple(IfGenerationMatch(generation)),
284+
std::move(delete_options)));
286285
});
287286

288287
auto compose_options = StaticTupleFilter<

0 commit comments

Comments
 (0)