Skip to content

Commit bff6db6

Browse files
coryancoryan
authored
cleanup(storage): more cognitive complexity reduction (#9432)
This time, I used clang-tidy to find functions with high cognitive complexity in the gRPC plugin.
1 parent 27950e4 commit bff6db6

File tree

2 files changed

+258
-206
lines changed

2 files changed

+258
-206
lines changed

‎google/cloud/storage/internal/grpc_bucket_request_parser.cc‎

Lines changed: 188 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,177 @@ namespace cloud {
2828
namespace storage {
2929
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
3030
namespace internal {
31+
namespace {
32+
33+
using ::google::storage::v2::Bucket;
34+
35+
Status PatchAcl(Bucket& b, nlohmann::json const& patch) {
36+
for (auto const& a : patch) {
37+
auto& acl = *b.add_acl();
38+
acl.set_entity(a.value("entity", ""));
39+
acl.set_role(a.value("role", ""));
40+
}
41+
return Status{};
42+
}
43+
44+
Status PatchDefaultObjectAcl(Bucket& b, nlohmann::json const& patch) {
45+
for (auto const& a : patch) {
46+
auto& acl = *b.add_default_object_acl();
47+
acl.set_entity(a.value("entity", ""));
48+
acl.set_role(a.value("role", ""));
49+
}
50+
return Status{};
51+
}
52+
53+
Status PatchLifecycle(Bucket& b, nlohmann::json const& patch) {
54+
auto& lifecycle = *b.mutable_lifecycle();
55+
// By construction, the PatchBuilder always includes the "rule"
56+
// subobject.
57+
for (auto const& r : patch["rule"]) {
58+
auto lf = LifecycleRuleParser::FromJson(r);
59+
if (!lf) return std::move(lf).status();
60+
*lifecycle.add_rule() = GrpcBucketMetadataParser::ToProto(*lf);
61+
}
62+
return Status{};
63+
}
64+
65+
Status PatchCors(Bucket& b, nlohmann::json const& patch) {
66+
for (auto const& c : patch) {
67+
auto& cors = *b.add_cors();
68+
cors.set_max_age_seconds(c.value("maxAgeSeconds", std::int32_t{0}));
69+
if (c.contains("origin")) {
70+
for (auto const& o : c["origin"]) {
71+
cors.add_origin(o.get<std::string>());
72+
}
73+
}
74+
if (c.contains("method")) {
75+
for (auto const& m : c["method"]) {
76+
cors.add_method(m.get<std::string>());
77+
}
78+
}
79+
if (c.contains("responseHeader")) {
80+
for (auto const& h : c["responseHeader"]) {
81+
cors.add_response_header(h.get<std::string>());
82+
}
83+
}
84+
}
85+
return Status{};
86+
}
87+
88+
Status PatchIamConfig(Bucket& b, nlohmann::json const& i) {
89+
auto& iam_config = *b.mutable_iam_config();
90+
if (i.contains("uniformBucketLevelAccess")) {
91+
iam_config.mutable_uniform_bucket_level_access()->set_enabled(
92+
i["uniformBucketLevelAccess"].value("enabled", false));
93+
}
94+
if (i.contains("publicAccessPrevention")) {
95+
auto pap = i.value("publicAccessPrevention", "");
96+
iam_config.set_public_access_prevention(pap);
97+
}
98+
return Status{};
99+
}
100+
101+
void UpdateAcl(Bucket& bucket, BucketMetadata const& metadata) {
102+
for (auto const& a : metadata.acl()) {
103+
auto& acl = *bucket.add_acl();
104+
acl.set_entity(a.entity());
105+
acl.set_role(a.role());
106+
}
107+
}
108+
109+
void UpdateDefaultObjectAcl(Bucket& bucket, BucketMetadata const& metadata) {
110+
for (auto const& a : metadata.default_acl()) {
111+
auto& acl = *bucket.add_default_object_acl();
112+
acl.set_entity(a.entity());
113+
acl.set_role(a.role());
114+
}
115+
}
116+
117+
void UpdateLifecycle(Bucket& bucket, BucketMetadata const& metadata) {
118+
if (!metadata.has_lifecycle()) return;
119+
auto& lifecycle = *bucket.mutable_lifecycle();
120+
// By construction, the PatchBuilder always includes the "rule"
121+
// subobject.
122+
for (auto const& r : metadata.lifecycle().rule) {
123+
*lifecycle.add_rule() = GrpcBucketMetadataParser::ToProto(r);
124+
}
125+
}
126+
127+
void UpdateCors(Bucket& bucket, BucketMetadata const& metadata) {
128+
for (auto const& c : metadata.cors()) {
129+
auto& cors = *bucket.add_cors();
130+
cors.set_max_age_seconds(
131+
static_cast<std::int32_t>(c.max_age_seconds.value_or(0)));
132+
for (auto const& o : c.origin) {
133+
cors.add_origin(o);
134+
}
135+
for (auto const& m : c.method) {
136+
cors.add_method(m);
137+
}
138+
for (auto const& h : c.response_header) {
139+
cors.add_response_header(h);
140+
}
141+
}
142+
}
143+
144+
void UpdateLabels(Bucket& bucket, BucketMetadata const& metadata) {
145+
for (auto const& kv : metadata.labels()) {
146+
(*bucket.mutable_labels())[kv.first] = kv.second;
147+
}
148+
}
149+
150+
void UpdateWebsite(Bucket& bucket, BucketMetadata const& metadata) {
151+
if (!metadata.has_website()) return;
152+
auto const& w = metadata.website();
153+
bucket.mutable_website()->set_main_page_suffix(w.main_page_suffix);
154+
bucket.mutable_website()->set_not_found_page(w.not_found_page);
155+
}
156+
157+
void UpdateVersioning(Bucket& bucket, BucketMetadata const& metadata) {
158+
if (!metadata.has_versioning()) return;
159+
bucket.mutable_versioning()->set_enabled(metadata.versioning()->enabled);
160+
}
161+
162+
void UpdateLogging(Bucket& bucket, BucketMetadata const& metadata) {
163+
if (!metadata.has_logging()) return;
164+
bucket.mutable_logging()->set_log_bucket(metadata.logging().log_bucket);
165+
bucket.mutable_logging()->set_log_object_prefix(
166+
metadata.logging().log_object_prefix);
167+
}
168+
169+
void UpdateEncryption(Bucket& bucket, BucketMetadata const& metadata) {
170+
if (!metadata.has_encryption()) return;
171+
bucket.mutable_encryption()->set_default_kms_key(
172+
metadata.encryption().default_kms_key_name);
173+
}
174+
175+
void UpdateBilling(Bucket& bucket, BucketMetadata const& metadata) {
176+
if (!metadata.has_billing()) return;
177+
bucket.mutable_billing()->set_requester_pays(
178+
metadata.billing().requester_pays);
179+
}
180+
181+
void UpdateRetentionPolicy(Bucket& bucket, BucketMetadata const& metadata) {
182+
if (!metadata.has_retention_policy()) return;
183+
bucket.mutable_retention_policy()->set_retention_period(
184+
metadata.retention_policy().retention_period.count());
185+
}
186+
187+
void UpdateIamConfig(Bucket& bucket, BucketMetadata const& metadata) {
188+
if (!metadata.has_iam_configuration()) return;
189+
auto& iam_config = *bucket.mutable_iam_config();
190+
auto const& i = metadata.iam_configuration();
191+
if (i.uniform_bucket_level_access.has_value()) {
192+
iam_config.mutable_uniform_bucket_level_access()->set_enabled(
193+
i.uniform_bucket_level_access->enabled);
194+
}
195+
if (i.public_access_prevention.has_value()) {
196+
auto pap = i.public_access_prevention.value();
197+
iam_config.set_public_access_prevention(pap);
198+
}
199+
}
200+
201+
} // namespace
31202

32203
google::storage::v2::DeleteBucketRequest GrpcBucketRequestParser::ToProto(
33204
DeleteBucketRequest const& request) {
@@ -228,7 +399,6 @@ GrpcBucketRequestParser::ToProto(PatchBucketRequest const& request) {
228399

229400
// This struct and the vector refactors some common code to create patches for
230401
// each field.
231-
using google::storage::v2::Bucket;
232402
struct Field {
233403
std::string name;
234404
std::string rename;
@@ -246,59 +416,10 @@ GrpcBucketRequestParser::ToProto(PatchBucketRequest const& request) {
246416
b.set_rpo(patch.get<std::string>());
247417
return Status{};
248418
}},
249-
{"acl", "",
250-
[](Bucket& b, nlohmann::json const& patch) {
251-
for (auto const& a : patch) {
252-
auto& acl = *b.add_acl();
253-
acl.set_entity(a.value("entity", ""));
254-
acl.set_role(a.value("role", ""));
255-
}
256-
return Status{};
257-
}},
258-
{"defaultObjectAcl", "default_object_acl",
259-
[](Bucket& b, nlohmann::json const& patch) {
260-
for (auto const& a : patch) {
261-
auto& acl = *b.add_default_object_acl();
262-
acl.set_entity(a.value("entity", ""));
263-
acl.set_role(a.value("role", ""));
264-
}
265-
return Status{};
266-
}},
267-
{"lifecycle", "",
268-
[](Bucket& b, nlohmann::json const& patch) {
269-
auto& lifecycle = *b.mutable_lifecycle();
270-
// By construction, the PatchBuilder always includes the "rule"
271-
// subobject.
272-
for (auto const& r : patch["rule"]) {
273-
auto lf = LifecycleRuleParser::FromJson(r);
274-
if (!lf) return std::move(lf).status();
275-
*lifecycle.add_rule() = GrpcBucketMetadataParser::ToProto(*lf);
276-
}
277-
return Status{};
278-
}},
279-
{"cors", "",
280-
[](Bucket& b, nlohmann::json const& patch) {
281-
for (auto const& c : patch) {
282-
auto& cors = *b.add_cors();
283-
cors.set_max_age_seconds(c.value("maxAgeSeconds", std::int32_t{0}));
284-
if (c.contains("origin")) {
285-
for (auto const& o : c["origin"]) {
286-
cors.add_origin(o.get<std::string>());
287-
}
288-
}
289-
if (c.contains("method")) {
290-
for (auto const& m : c["method"]) {
291-
cors.add_method(m.get<std::string>());
292-
}
293-
}
294-
if (c.contains("responseHeader")) {
295-
for (auto const& h : c["responseHeader"]) {
296-
cors.add_response_header(h.get<std::string>());
297-
}
298-
}
299-
}
300-
return Status{};
301-
}},
419+
{"acl", "", PatchAcl},
420+
{"defaultObjectAcl", "default_object_acl", PatchDefaultObjectAcl},
421+
{"lifecycle", "", PatchLifecycle},
422+
{"cors", "", PatchCors},
302423
{"defaultEventBasedHold", "default_event_based_hold",
303424
[](Bucket& b, nlohmann::json const& patch) {
304425
b.set_default_event_based_hold(patch.get<bool>());
@@ -341,19 +462,7 @@ GrpcBucketRequestParser::ToProto(PatchBucketRequest const& request) {
341462
r.value("retentionPeriod", int64_t{0}));
342463
return Status{};
343464
}},
344-
{"iamConfiguration", "iam_config",
345-
[](Bucket& b, nlohmann::json const& i) {
346-
auto& iam_config = *b.mutable_iam_config();
347-
if (i.contains("uniformBucketLevelAccess")) {
348-
iam_config.mutable_uniform_bucket_level_access()->set_enabled(
349-
i["uniformBucketLevelAccess"].value("enabled", false));
350-
}
351-
if (i.contains("publicAccessPrevention")) {
352-
auto pap = i.value("publicAccessPrevention", "");
353-
iam_config.set_public_access_prevention(pap);
354-
}
355-
return Status{};
356-
}},
465+
{"iamConfiguration", "iam_config", PatchIamConfig},
357466
};
358467

359468
for (auto const& field : fields) {
@@ -399,91 +508,31 @@ google::storage::v2::UpdateBucketRequest GrpcBucketRequestParser::ToProto(
399508
result.mutable_update_mask()->add_paths("rpo");
400509
bucket.set_rpo(metadata.rpo());
401510
result.mutable_update_mask()->add_paths("acl");
402-
for (auto const& a : metadata.acl()) {
403-
auto& acl = *bucket.add_acl();
404-
acl.set_entity(a.entity());
405-
acl.set_role(a.role());
406-
}
511+
UpdateAcl(bucket, metadata);
407512
result.mutable_update_mask()->add_paths("default_object_acl");
408-
for (auto const& a : metadata.default_acl()) {
409-
auto& acl = *bucket.add_default_object_acl();
410-
acl.set_entity(a.entity());
411-
acl.set_role(a.role());
412-
}
513+
UpdateDefaultObjectAcl(bucket, metadata);
413514
result.mutable_update_mask()->add_paths("lifecycle");
414-
if (metadata.has_lifecycle()) {
415-
auto& lifecycle = *bucket.mutable_lifecycle();
416-
// By construction, the PatchBuilder always includes the "rule"
417-
// subobject.
418-
for (auto const& r : metadata.lifecycle().rule) {
419-
*lifecycle.add_rule() = GrpcBucketMetadataParser::ToProto(r);
420-
}
421-
}
515+
UpdateLifecycle(bucket, metadata);
422516
result.mutable_update_mask()->add_paths("cors");
423-
for (auto const& c : metadata.cors()) {
424-
auto& cors = *bucket.add_cors();
425-
cors.set_max_age_seconds(
426-
static_cast<std::int32_t>(c.max_age_seconds.value_or(0)));
427-
for (auto const& o : c.origin) {
428-
cors.add_origin(o);
429-
}
430-
for (auto const& m : c.method) {
431-
cors.add_method(m);
432-
}
433-
for (auto const& h : c.response_header) {
434-
cors.add_response_header(h);
435-
}
436-
}
517+
UpdateCors(bucket, metadata);
437518
result.mutable_update_mask()->add_paths("default_event_based_hold");
438519
bucket.set_default_event_based_hold(metadata.default_event_based_hold());
439520
result.mutable_update_mask()->add_paths("labels");
440-
for (auto const& kv : metadata.labels()) {
441-
(*bucket.mutable_labels())[kv.first] = kv.second;
442-
}
521+
UpdateLabels(bucket, metadata);
443522
result.mutable_update_mask()->add_paths("website");
444-
if (metadata.has_website()) {
445-
auto const& w = metadata.website();
446-
bucket.mutable_website()->set_main_page_suffix(w.main_page_suffix);
447-
bucket.mutable_website()->set_not_found_page(w.not_found_page);
448-
}
523+
UpdateWebsite(bucket, metadata);
449524
result.mutable_update_mask()->add_paths("versioning");
450-
if (metadata.has_versioning()) {
451-
bucket.mutable_versioning()->set_enabled(metadata.versioning()->enabled);
452-
}
525+
UpdateVersioning(bucket, metadata);
453526
result.mutable_update_mask()->add_paths("logging");
454-
if (metadata.has_logging()) {
455-
bucket.mutable_logging()->set_log_bucket(metadata.logging().log_bucket);
456-
bucket.mutable_logging()->set_log_object_prefix(
457-
metadata.logging().log_object_prefix);
458-
}
527+
UpdateLogging(bucket, metadata);
459528
result.mutable_update_mask()->add_paths("encryption");
460-
if (metadata.has_encryption()) {
461-
bucket.mutable_encryption()->set_default_kms_key(
462-
metadata.encryption().default_kms_key_name);
463-
}
529+
UpdateEncryption(bucket, metadata);
464530
result.mutable_update_mask()->add_paths("billing");
465-
if (metadata.has_billing()) {
466-
bucket.mutable_billing()->set_requester_pays(
467-
metadata.billing().requester_pays);
468-
}
531+
UpdateBilling(bucket, metadata);
469532
result.mutable_update_mask()->add_paths("retention_policy");
470-
if (metadata.has_retention_policy()) {
471-
bucket.mutable_retention_policy()->set_retention_period(
472-
metadata.retention_policy().retention_period.count());
473-
}
533+
UpdateRetentionPolicy(bucket, metadata);
474534
result.mutable_update_mask()->add_paths("iam_config");
475-
if (metadata.has_iam_configuration()) {
476-
auto& iam_config = *bucket.mutable_iam_config();
477-
auto const& i = metadata.iam_configuration();
478-
if (i.uniform_bucket_level_access.has_value()) {
479-
iam_config.mutable_uniform_bucket_level_access()->set_enabled(
480-
i.uniform_bucket_level_access->enabled);
481-
}
482-
if (i.public_access_prevention.has_value()) {
483-
auto pap = i.public_access_prevention.value();
484-
iam_config.set_public_access_prevention(pap);
485-
}
486-
}
535+
UpdateIamConfig(bucket, metadata);
487536

488537
if (request.HasOption<IfMetagenerationMatch>()) {
489538
result.set_if_metageneration_match(

0 commit comments

Comments
 (0)