Skip to content

Commit fb42587

Browse files
colinmoycolinmoy
committed
fix(generator): address maintainer review feedback and fix compilation across protobuf versions
1 parent 1e723fc commit fb42587

4 files changed

Lines changed: 31 additions & 35 deletions

File tree

‎generator/internal/discovery_to_proto.cc‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ std::set<std::string> FindAllTypesToImport(nlohmann::json const& json) {
360360
types_to_import.insert((*current)["$ref"]);
361361
}
362362

363-
if (current->contains("format") && (*current)["format"].is_string()) {
363+
if (current->contains("format")) {
364364
std::string const format = (*current)["format"];
365365
if (absl::StartsWith(format, "google.protobuf.")) {
366366
types_to_import.insert(format);
@@ -383,9 +383,8 @@ std::set<std::string> FindAllTypesToImport(nlohmann::json const& json) {
383383

384384
if (IsDiscoveryArrayType(*current)) {
385385
auto const& items = (*current)["items"];
386-
if (items.is_object() && items.contains("type") &&
387-
items["type"] == "object" && items.contains("additionalProperties") &&
388-
items["additionalProperties"].is_object() &&
386+
if (items.contains("type") && items["type"] == "object" &&
387+
items.contains("additionalProperties") &&
389388
items["additionalProperties"].value("type", "") == "any" &&
390389
!items.contains("format") &&
391390
!items["additionalProperties"].contains("format")) {
@@ -397,8 +396,7 @@ std::set<std::string> FindAllTypesToImport(nlohmann::json const& json) {
397396

398397
if (IsDiscoveryMapType(*current)) {
399398
auto const& additional_properties = (*current)["additionalProperties"];
400-
if (additional_properties.is_object() &&
401-
additional_properties.contains("type") &&
399+
if (additional_properties.contains("type") &&
402400
additional_properties["type"] == "any" &&
403401
!additional_properties.contains("format") &&
404402
!current->contains("format")) {

‎generator/internal/discovery_type_vertex.cc‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -48,20 +48,16 @@ std::optional<std::string> CheckForScalarType(nlohmann::json const& j) {
4848
bool IsStringOrBytes(nlohmann::json const& field_json) {
4949
std::string const type = field_json.value("type", "");
5050
if (type == "string" || type == "bytes") return true;
51-
if (type == "array" && field_json.contains("items") &&
52-
field_json["items"].is_object()) {
51+
if (type == "array" && field_json.contains("items")) {
5352
std::string const item_type = field_json["items"].value("type", "");
5453
if (item_type == "string" || item_type == "bytes") return true;
5554
}
5655
return false;
5756
}
5857

5958
bool ContainsKeyWord(std::string_view s) {
60-
for (std::size_t pos = s.find("key"); pos != std::string_view::npos;
61-
pos = s.find("key", pos + 1)) {
62-
bool const prefix_ok = (pos == 0 || s[pos - 1] == '_');
63-
bool const suffix_ok = (pos + 3 == s.size() || s[pos + 3] == '_');
64-
if (prefix_ok && suffix_ok) return true;
59+
for (auto const& token : absl::StrSplit(s, '_')) {
60+
if (token == "key") return true;
6561
}
6662
return false;
6763
}
@@ -133,7 +129,7 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v,
133129
}
134130

135131
if (type == "any") {
136-
if (v.contains("format") && v["format"].is_string()) {
132+
if (v.contains("format")) {
137133
type = v["format"];
138134
} else {
139135
type = "google.protobuf.Value";
@@ -174,11 +170,9 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v,
174170
properties_for_synthesis = &additional_properties;
175171
is_message = true;
176172
} else if (map_type == "any") {
177-
if (additional_properties.is_object() &&
178-
additional_properties.contains("format") &&
179-
additional_properties["format"].is_string()) {
173+
if (additional_properties.contains("format")) {
180174
map_type = additional_properties["format"];
181-
} else if (v.contains("format") && v["format"].is_string()) {
175+
} else if (v.contains("format")) {
182176
map_type = v["format"];
183177
} else {
184178
map_type = "google.protobuf.Struct";
@@ -219,26 +213,21 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v,
219213
if (scalar_type) {
220214
type = *scalar_type;
221215
} else if (type == "any") {
222-
if (items.is_object() && items.contains("format") &&
223-
items["format"].is_string()) {
216+
if (items.contains("format")) {
224217
type = items["format"];
225218
} else {
226219
type = "google.protobuf.Value";
227220
}
228221
return TypeInfo{type, compare_package_name, nullptr, false, false};
229-
} else if (type == "object" && items.is_object() &&
230-
items.contains("properties")) {
222+
} else if (type == "object" && items.contains("properties")) {
231223
// Synthesize a nested type for this array.
232224
type = CapitalizeFirstLetter(field_name + "Item");
233225
return TypeInfo{type, compare_package_name, &items, false, true};
234-
} else if (type == "object" && items.is_object() &&
235-
items.contains("additionalProperties") &&
236-
items["additionalProperties"].is_object() &&
237-
items["additionalProperties"].value("type", "") == "any") {
238-
if (items.contains("format") && items["format"].is_string()) {
226+
} else if (type == "object" && items.contains("additionalProperties") &&
227+
(items["additionalProperties"]).value("type", "") == "any") {
228+
if (items.contains("format")) {
239229
type = items["format"];
240-
} else if (items["additionalProperties"].contains("format") &&
241-
items["additionalProperties"]["format"].is_string()) {
230+
} else if (items["additionalProperties"].contains("format")) {
242231
type = items["additionalProperties"]["format"];
243232
} else {
244233
type = "google.protobuf.Struct";

‎google/cloud/internal/debug_string_protobuf.cc‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,22 @@ class TimestampMessagePrinter
6262
}
6363
};
6464

65+
template <typename Printer>
66+
auto SetRedact(Printer& p, int)
67+
-> decltype(p.SetRedactDebugString(true), void()) {
68+
p.SetRedactDebugString(true);
69+
}
70+
71+
template <typename Printer>
72+
void SetRedact(Printer&, ...) {}
73+
6574
} // namespace
6675

6776
std::string DebugString(google::protobuf::Message const& m,
6877
TracingOptions const& options) {
6978
std::string str;
7079
google::protobuf::TextFormat::Printer p;
71-
p.SetRedactDebugString(true);
80+
SetRedact(p, 0);
7281
p.SetSingleLineMode(options.single_line_mode());
7382
if (!options.single_line_mode()) p.SetInitialIndentLevel(1);
7483
p.SetUseShortRepeatedPrimitives(options.use_short_repeated_primitives());

‎google/cloud/internal/debug_string_protobuf_test.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ TEST(LogWrapperHelpers, DefaultOptions) {
5454
// clang-format off
5555
std::string const text =
5656
R"pb(google.iam.v1.Policy { )pb"
57-
R"pb(goo.gle/debugproto )pb"
57+
R"pb(goo.gle/debugonly )pb"
5858
R"pb(bindings { )pb"
5959
R"pb(role: "roles/viewer" )pb"
6060
R"pb(members: "user:user1@example.com" )pb"
@@ -76,7 +76,7 @@ TEST(LogWrapperHelpers, MultiLine) {
7676
tracing_options.SetOptions("single_line_mode=off");
7777
// clang-format off
7878
std::string const text = R"pb(google.iam.v1.Policy {
79-
goo.gle/debugproto
79+
goo.gle/debugonly
8080
bindings {
8181
role: "roles/viewer"
8282
members: "user:user1@example.com"
@@ -99,7 +99,7 @@ TEST(LogWrapperHelpers, Truncate) {
9999
// clang-format off
100100
std::string const text =
101101
R"pb(google.iam.v1.Policy { )pb"
102-
R"pb(goo.gle/debugproto )pb"
102+
R"pb(goo.gle/debugonly )pb"
103103
R"pb(bindings { )pb"
104104
R"pb(role: "roles/vi...<truncated>..." )pb"
105105
R"pb(members: "user:use...<truncated>..." )pb"
@@ -121,7 +121,7 @@ TEST(LogWrapperHelpers, Duration) {
121121
duration.set_seconds((11 * 60 + 22) * 60 + 33);
122122
duration.set_nanos(123456789);
123123
std::string const expected =
124-
R"(google.protobuf.Duration { goo.gle/debugproto "11h22m33.123456789s" })";
124+
R"(google.protobuf.Duration { goo.gle/debugonly "11h22m33.123456789s" })";
125125
EXPECT_EQ(expected, DebugString(duration, TracingOptions{}.SetOptions(
126126
"single_line_mode=on")));
127127
}
@@ -131,7 +131,7 @@ TEST(LogWrapperHelpers, Timestamp) {
131131
timestamp.set_seconds(1658470436);
132132
timestamp.set_nanos(123456789);
133133
std::string const expected = R"(google.protobuf.Timestamp {
134-
goo.gle/debugproto
134+
goo.gle/debugonly
135135
"2022-07-22T06:13:56.123456789Z"
136136
})";
137137
EXPECT_EQ(expected, DebugString(timestamp, TracingOptions{}.SetOptions(

0 commit comments

Comments
 (0)