Skip to content

Commit 1e61e4d

Browse files
committed
add more tests and fix ci failures
1 parent 5116181 commit 1e61e4d

File tree

9 files changed

+256
-16
lines changed

9 files changed

+256
-16
lines changed

google/cloud/storage/examples/storage_bucket_samples.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ void ListBuckets(google::cloud::storage::Client client,
5151

5252
void ListBucketsPartial(google::cloud::storage::Client client,
5353
std::vector<std::string> const& /*argv*/) {
54-
//! [list buckets partial result] [START list_buckets_partial]
54+
//! [list buckets partial result] [START storage_list_buckets_partial_success]
5555
namespace gcs = ::google::cloud::storage;
5656
using ::google::cloud::StatusOr;
5757
[](gcs::Client client) {
@@ -73,7 +73,7 @@ void ListBucketsPartial(google::cloud::storage::Client client,
7373
std::cout << "No buckets in default project\n";
7474
}
7575
}
76-
//! [list buckets partial result] [END list_buckets_partial]
76+
//! [list buckets partial result] [END storage_list_buckets_partial_success]
7777
(std::move(client));
7878
}
7979

google/cloud/storage/google_cloud_cpp_storage.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,7 @@ if (BUILD_TESTING)
507507
internal/tuple_filter_test.cc
508508
internal/unified_rest_credentials_test.cc
509509
lifecycle_rule_test.cc
510+
list_buckets_partial_reader_test.cc
510511
list_buckets_reader_test.cc
511512
list_hmac_keys_reader_test.cc
512513
list_objects_and_prefixes_reader_test.cc

google/cloud/storage/internal/bucket_requests.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ class ListBucketsRequest
4646

4747
std::string const& project_id() const { return project_id_; }
4848
std::string const& page_token() const { return page_token_; }
49-
bool const& return_partial_success() const { return return_partial_success_; }
49+
bool return_partial_success() const {
50+
return GetOption<ReturnPartialSuccess>().value_or(false);
51+
}
5052
ListBucketsRequest& set_page_token(std::string page_token) {
5153
page_token_ = std::move(page_token);
5254
return *this;
@@ -55,7 +57,6 @@ class ListBucketsRequest
5557
private:
5658
std::string project_id_;
5759
std::string page_token_;
58-
bool return_partial_success_;
5960
};
6061

6162
std::ostream& operator<<(std::ostream& os, ListBucketsRequest const& r);

google/cloud/storage/internal/grpc/bucket_request_parser.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,9 @@ google::storage::v2::ListBucketsRequest ToProto(
407407
}
408408
result.set_page_token(request.page_token());
409409
result.set_prefix(request.GetOption<storage::Prefix>().value_or(""));
410+
if (request.return_partial_success()) {
411+
result.set_return_partial_success(true);
412+
}
410413
if (request.GetOption<storage::Projection>().value_or("") == "full") {
411414
result.mutable_read_mask()->add_paths("*");
412415
}

google/cloud/storage/internal/grpc/bucket_request_parser_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ TEST(GrpcBucketRequestParser, ListBucketsRequestAllOptions) {
187187
page_size: 123
188188
page_token: "test-token"
189189
prefix: "test-prefix"
190-
return_partial_success: false
190+
return_partial_success: true
191191
read_mask { paths: [ "*" ] }
192192
)pb",
193193
&expected));

google/cloud/storage/internal/grpc/stub_test.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,32 @@ TEST_F(GrpcClientTest, ListBuckets) {
278278
EXPECT_EQ(response.status(), PermanentError());
279279
}
280280

281+
TEST_F(GrpcClientTest, ListBucketsWithPartialSuccess) {
282+
auto mock = std::make_shared<MockStorageStub>();
283+
EXPECT_CALL(*mock, ListBuckets)
284+
.WillOnce([this](grpc::ClientContext& context, Options const&,
285+
google::storage::v2::ListBucketsRequest const& request) {
286+
auto metadata = GetMetadata(context);
287+
EXPECT_THAT(metadata,
288+
UnorderedElementsAre(
289+
Pair(kIdempotencyTokenHeader, "test-token-1234"),
290+
Pair("x-goog-quota-user", "test-quota-user"),
291+
Pair("x-goog-fieldmask", "field1,field2")));
292+
EXPECT_THAT(request.parent(), "projects/test-project");
293+
EXPECT_TRUE(request.return_partial_success());
294+
return PermanentError();
295+
});
296+
auto client = CreateTestClient(mock);
297+
auto context = TestContext();
298+
auto response = client->ListBuckets(
299+
context, TestOptions(),
300+
storage::internal::ListBucketsRequest("test-project")
301+
.set_multiple_options(Fields("field1,field2"),
302+
QuotaUser("test-quota-user"),
303+
storage::ReturnPartialSuccess(true)));
304+
EXPECT_EQ(response.status(), PermanentError());
305+
}
306+
281307
TEST_F(GrpcClientTest, LockBucketRetentionPolicy) {
282308
auto mock = std::make_shared<MockStorageStub>();
283309
EXPECT_CALL(*mock, LockBucketRetentionPolicy)
Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
// Copyright 2025 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#include "google/cloud/storage/list_buckets_partial_reader.h"
16+
#include "google/cloud/storage/internal/bucket_metadata_parser.h"
17+
#include "google/cloud/storage/testing/canonical_errors.h"
18+
#include "google/cloud/storage/testing/mock_client.h"
19+
#include "google/cloud/testing_util/status_matchers.h"
20+
#include <gmock/gmock.h>
21+
#include <nlohmann/json.hpp>
22+
#include <vector>
23+
24+
namespace google {
25+
namespace cloud {
26+
namespace storage {
27+
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
28+
namespace {
29+
30+
using ::google::cloud::storage::internal::ListBucketsRequest;
31+
using ::google::cloud::storage::internal::ListBucketsResponse;
32+
using ::google::cloud::storage::testing::MockClient;
33+
using ::google::cloud::storage::testing::canonical_errors::PermanentError;
34+
using ::testing::ContainerEq;
35+
using ::testing::Return;
36+
37+
BucketMetadata CreateElement(int index) {
38+
std::string id = "bucket-" + std::to_string(index);
39+
std::string name = id;
40+
std::string link = "https://storage.googleapis.com/storage/v1/b/" + id;
41+
nlohmann::json metadata{
42+
{"id", id},
43+
{"name", name},
44+
{"selfLink", link},
45+
{"kind", "storage#bucket"},
46+
};
47+
return internal::BucketMetadataParser::FromJson(metadata).value();
48+
}
49+
50+
TEST(ListBucketsPartialReaderTest, Basic) {
51+
// We will have 3 pages.
52+
// Page 0: 2 buckets, 1 unreachable
53+
// Page 1: 1 bucket, 0 unreachable
54+
// Page 2: 0 buckets, 1 unreachable
55+
56+
std::vector<BucketsPartial> expected;
57+
58+
// Page 0
59+
expected.push_back({{CreateElement(0), CreateElement(1)}, {"region-a"}});
60+
61+
// Page 1
62+
expected.push_back({{CreateElement(2)}, {}});
63+
64+
// Page 2
65+
expected.push_back({{}, {"region-b"}});
66+
67+
auto create_mock = [&](int i) {
68+
ListBucketsResponse response;
69+
if (i < 2) {
70+
response.next_page_token = "page-" + std::to_string(i + 1);
71+
}
72+
response.items = expected[i].buckets;
73+
response.unreachable = expected[i].unreachable;
74+
75+
return [response](ListBucketsRequest const&) {
76+
return make_status_or(response);
77+
};
78+
};
79+
80+
auto mock = std::make_shared<MockClient>();
81+
EXPECT_CALL(*mock, ListBuckets)
82+
.WillOnce(create_mock(0))
83+
.WillOnce(create_mock(1))
84+
.WillOnce(create_mock(2));
85+
86+
auto reader =
87+
google::cloud::internal::MakePaginationRange<ListBucketsPartialReader>(
88+
ListBucketsRequest("test-project"),
89+
[mock](ListBucketsRequest const& r) { return mock->ListBuckets(r); },
90+
[](internal::ListBucketsResponse r) {
91+
std::vector<BucketsPartial> result;
92+
if (r.items.empty() && r.unreachable.empty()) return result;
93+
result.push_back(
94+
BucketsPartial{std::move(r.items), std::move(r.unreachable)});
95+
return result;
96+
});
97+
98+
std::vector<BucketsPartial> actual;
99+
for (auto&& page : reader) {
100+
ASSERT_STATUS_OK(page);
101+
actual.push_back(*std::move(page));
102+
}
103+
104+
ASSERT_EQ(actual.size(), expected.size());
105+
for (size_t i = 0; i < actual.size(); ++i) {
106+
EXPECT_THAT(actual[i].buckets, ContainerEq(expected[i].buckets));
107+
EXPECT_THAT(actual[i].unreachable, ContainerEq(expected[i].unreachable));
108+
}
109+
}
110+
111+
TEST(ListBucketsPartialReaderTest, Empty) {
112+
auto mock = std::make_shared<MockClient>();
113+
EXPECT_CALL(*mock, ListBuckets)
114+
.WillOnce(Return(make_status_or(ListBucketsResponse())));
115+
116+
auto reader =
117+
google::cloud::internal::MakePaginationRange<ListBucketsPartialReader>(
118+
ListBucketsRequest("test-project"),
119+
[mock](ListBucketsRequest const& r) { return mock->ListBuckets(r); },
120+
[](internal::ListBucketsResponse r) {
121+
std::vector<BucketsPartial> result;
122+
if (r.items.empty() && r.unreachable.empty()) return result;
123+
result.push_back(
124+
BucketsPartial{std::move(r.items), std::move(r.unreachable)});
125+
return result;
126+
});
127+
128+
auto count = std::distance(reader.begin(), reader.end());
129+
EXPECT_EQ(0, count);
130+
}
131+
132+
TEST(ListBucketsPartialReaderTest, PermanentFailure) {
133+
// Create a synthetic list of BucketsPartial elements.
134+
std::vector<BucketsPartial> expected;
135+
136+
// Page 0
137+
expected.push_back({{CreateElement(0), CreateElement(1)}, {"region-a"}});
138+
// Page 1
139+
expected.push_back({{CreateElement(2)}, {}});
140+
141+
auto create_mock = [&](int i) {
142+
ListBucketsResponse response;
143+
response.next_page_token = "page-" + std::to_string(i + 1);
144+
response.items = expected[i].buckets;
145+
response.unreachable = expected[i].unreachable;
146+
return [response](ListBucketsRequest const&) {
147+
return StatusOr<ListBucketsResponse>(response);
148+
};
149+
};
150+
151+
auto mock = std::make_shared<MockClient>();
152+
EXPECT_CALL(*mock, ListBuckets)
153+
.WillOnce(create_mock(0))
154+
.WillOnce(create_mock(1))
155+
.WillOnce([](ListBucketsRequest const&) {
156+
return StatusOr<ListBucketsResponse>(PermanentError());
157+
});
158+
159+
auto reader =
160+
google::cloud::internal::MakePaginationRange<ListBucketsPartialReader>(
161+
ListBucketsRequest("test-project"),
162+
[mock](ListBucketsRequest const& r) { return mock->ListBuckets(r); },
163+
[](internal::ListBucketsResponse r) {
164+
std::vector<BucketsPartial> result;
165+
if (r.items.empty() && r.unreachable.empty()) return result;
166+
result.push_back(
167+
BucketsPartial{std::move(r.items), std::move(r.unreachable)});
168+
return result;
169+
});
170+
std::vector<BucketsPartial> actual;
171+
bool has_status_or_error = false;
172+
for (auto&& page : reader) {
173+
if (page.ok()) {
174+
actual.emplace_back(*std::move(page));
175+
continue;
176+
}
177+
// The iteration should fail only once, an error should reset the iterator
178+
// to `end()`.
179+
EXPECT_FALSE(has_status_or_error);
180+
has_status_or_error = true;
181+
// Verify the error is what we expect.
182+
Status status = std::move(page).status();
183+
EXPECT_EQ(PermanentError().code(), status.code());
184+
EXPECT_EQ(PermanentError().message(), status.message());
185+
}
186+
// The iteration should have returned an error at least once.
187+
EXPECT_TRUE(has_status_or_error);
188+
189+
// The iteration should have returned all the elements prior to the error.
190+
ASSERT_EQ(actual.size(), expected.size());
191+
for (size_t i = 0; i < actual.size(); ++i) {
192+
EXPECT_THAT(actual[i].buckets, ContainerEq(expected[i].buckets));
193+
EXPECT_THAT(actual[i].unreachable, ContainerEq(expected[i].unreachable));
194+
}
195+
}
196+
197+
} // namespace
198+
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
199+
} // namespace storage
200+
} // namespace cloud
201+
} // namespace google

google/cloud/storage/storage_client_unit_tests.bzl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ storage_client_unit_tests = [
9797
"internal/tuple_filter_test.cc",
9898
"internal/unified_rest_credentials_test.cc",
9999
"lifecycle_rule_test.cc",
100+
"list_buckets_partial_reader_test.cc",
100101
"list_buckets_reader_test.cc",
101102
"list_hmac_keys_reader_test.cc",
102103
"list_objects_and_prefixes_reader_test.cc",

google/cloud/storage/tests/bucket_integration_test.cc

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -685,25 +685,32 @@ TEST_F(BucketIntegrationTest, ListFailure) {
685685
TEST_F(BucketIntegrationTest, ListPartialSuccess) {
686686
auto client = MakeIntegrationTestClient();
687687
std::string bucket_name = MakeRandomBucketName();
688+
std::string unreachable_bucket_name = MakeRandomBucketName() + "-unreachable";
688689

689690
auto meta =
690691
client.CreateBucketForProject(bucket_name, project_id_, BucketMetadata{});
691692
ASSERT_STATUS_OK(meta);
692693
ScheduleForDelete(*meta);
693694

694-
auto list_buckets_result = [&client] {
695-
std::vector<std::string> names;
696-
for (auto& r : client.ListBucketsPartial()) {
697-
EXPECT_STATUS_OK(r);
698-
if (!r) break;
699-
for (auto const& b : r->buckets) {
700-
names.push_back(b.name());
701-
}
695+
auto meta_unreachable = client.CreateBucketForProject(
696+
unreachable_bucket_name, project_id_, BucketMetadata{});
697+
ASSERT_STATUS_OK(meta_unreachable);
698+
ScheduleForDelete(*meta_unreachable);
699+
700+
std::vector<std::string> names;
701+
std::vector<std::string> unreachable;
702+
for (auto& r : client.ListBucketsPartial()) {
703+
EXPECT_STATUS_OK(r);
704+
if (!r) break;
705+
for (auto const& b : r->buckets) {
706+
names.push_back(b.name());
702707
}
703-
return names;
704-
};
708+
unreachable.insert(unreachable.end(), r->unreachable.begin(),
709+
r->unreachable.end());
710+
}
705711

706-
ASSERT_THAT(list_buckets_result(), Contains(bucket_name));
712+
EXPECT_THAT(names, Contains(bucket_name));
713+
EXPECT_THAT(unreachable, Contains(unreachable_bucket_name));
707714
}
708715

709716
TEST_F(BucketIntegrationTest, CreateFailure) {

0 commit comments

Comments
 (0)