Skip to content

Commit 65776df

Browse files
authored
fix(storage): allow overwrites in parallel uploads (#6908)
In `ParallelUploadFile()` we were not allowing overwrites of the target object. In some cases applications may absolutely want to overwite the target (say because they are replacing its contents). In some applications they may not, in which case they can provide a pre-condition.
1 parent a95eda5 commit 65776df

File tree

2 files changed

+79
-12
lines changed

2 files changed

+79
-12
lines changed

google/cloud/storage/client.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3539,7 +3539,7 @@ StatusOr<ObjectMetadata> ComposeMany(
35393539
internal::ComposeApplyHelper{client, bucket_name,
35403540
std::move(compose_range),
35413541
std::move(destination_object_name)},
3542-
std::tuple_cat(std::make_tuple(IfGenerationMatch(0)), all_options));
3542+
all_options);
35433543
}
35443544
return google::cloud::internal::apply(
35453545
internal::ComposeApplyHelper{client, bucket_name,

google/cloud/storage/tests/object_parallel_upload_integration_test.cc

Lines changed: 78 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@
2121
#include "google/cloud/testing_util/expect_exception.h"
2222
#include "google/cloud/testing_util/status_matchers.h"
2323
#include <gmock/gmock.h>
24-
#include <sys/types.h>
2524
#include <algorithm>
26-
#include <cstring>
2725
#include <iostream>
2826
#include <memory>
2927
#include <string>
@@ -34,6 +32,10 @@ namespace storage {
3432
inline namespace STORAGE_CLIENT_NS {
3533
namespace {
3634

35+
using ::google::cloud::testing_util::StatusIs;
36+
using ::testing::AnyOf;
37+
using ::testing::ElementsAre;
38+
3739
using ObjectParallelUploadIntegrationTest =
3840
::google::cloud::storage::testing::ObjectIntegrationTest;
3941

@@ -56,23 +58,88 @@ TEST_F(ObjectParallelUploadIntegrationTest, ParallelUpload) {
5658
std::string actual(std::istreambuf_iterator<char>{stream}, {});
5759
EXPECT_EQ(LoremIpsum(), actual);
5860

59-
std::vector<ObjectMetadata> objects;
60-
for (auto& object : client->ListObjects(bucket_name_)) {
61-
ASSERT_STATUS_OK(object);
62-
if (object->name().substr(0, prefix.size()) == prefix &&
63-
object->name() != prefix) {
64-
objects.emplace_back(*std::move(object));
65-
}
61+
std::vector<std::string> names;
62+
for (auto& object : client->ListObjects(bucket_name_, Prefix(prefix))) {
63+
if (!object) break;
64+
names.push_back(object->name());
6665
}
67-
ASSERT_EQ(1U, objects.size());
68-
ASSERT_EQ(prefix + ".dest", objects[0].name());
66+
EXPECT_THAT(names, ElementsAre(dest_object_name));
6967

7068
auto deletion_status =
7169
client->DeleteObject(bucket_name_, dest_object_name,
7270
IfGenerationMatch(object_metadata->generation()));
7371
ASSERT_STATUS_OK(deletion_status);
7472
}
7573

74+
TEST_F(ObjectParallelUploadIntegrationTest, DefaultAllowOverwrites) {
75+
StatusOr<Client> client = MakeIntegrationTestClient();
76+
ASSERT_STATUS_OK(client);
77+
78+
// Create the local file, we overwrite its contents manually because creating
79+
// a large block is fairly tedious.
80+
auto const block = MakeRandomData(1024 * 1024);
81+
testing::TempFile temp_file(block);
82+
83+
auto prefix = CreateRandomPrefixName();
84+
auto const dest_object_name = prefix + ".dest";
85+
// First insert the object, verify that it did not exist before the upload.
86+
auto insert = client->InsertObject(bucket_name_, dest_object_name,
87+
LoremIpsum(), IfGenerationMatch(0));
88+
ASSERT_STATUS_OK(insert);
89+
90+
auto object_metadata = ParallelUploadFile(
91+
*client, temp_file.name(), bucket_name_, dest_object_name, prefix, false,
92+
MinStreamSize(0), MaxStreams(64));
93+
EXPECT_STATUS_OK(object_metadata);
94+
EXPECT_EQ(block.size(), object_metadata->size());
95+
96+
std::vector<std::string> names;
97+
for (auto& object : client->ListObjects(bucket_name_, Prefix(prefix))) {
98+
if (!object) break;
99+
names.push_back(object->name());
100+
}
101+
EXPECT_THAT(names, ElementsAre(dest_object_name));
102+
103+
// Delete both objects
104+
(void)client->DeleteObject(bucket_name_, dest_object_name,
105+
Generation(insert->generation()));
106+
(void)client->DeleteObject(bucket_name_, dest_object_name,
107+
Generation(object_metadata->generation()));
108+
}
109+
110+
TEST_F(ObjectParallelUploadIntegrationTest, PreconditionsPreventOverwrites) {
111+
StatusOr<Client> client = MakeIntegrationTestClient();
112+
ASSERT_STATUS_OK(client);
113+
114+
// Create the local file, we overwrite its contents manually because creating
115+
// a large block is fairly tedious.
116+
auto const block = MakeRandomData(1024 * 1024);
117+
testing::TempFile temp_file(block);
118+
119+
auto prefix = CreateRandomPrefixName();
120+
auto const dest_object_name = prefix + ".dest";
121+
// First insert the object, verify that it did not exist before the upload.
122+
auto insert = client->InsertObject(bucket_name_, dest_object_name,
123+
LoremIpsum(), IfGenerationMatch(0));
124+
ASSERT_STATUS_OK(insert);
125+
126+
auto object_metadata = ParallelUploadFile(
127+
*client, temp_file.name(), bucket_name_, dest_object_name, prefix, false,
128+
MinStreamSize(0), MaxStreams(64), IfGenerationMatch(0));
129+
EXPECT_THAT(object_metadata, StatusIs(AnyOf(StatusCode::kFailedPrecondition,
130+
StatusCode::kAborted)));
131+
132+
std::vector<std::string> names;
133+
for (auto& object : client->ListObjects(bucket_name_, Prefix(prefix))) {
134+
if (!object) break;
135+
names.push_back(object->name());
136+
}
137+
EXPECT_THAT(names, ElementsAre(dest_object_name));
138+
139+
(void)client->DeleteObject(bucket_name_, dest_object_name,
140+
Generation(insert->generation()));
141+
}
142+
76143
} // anonymous namespace
77144
} // namespace STORAGE_CLIENT_NS
78145
} // namespace storage

0 commit comments

Comments
 (0)