Skip to content

Commit 581514e

Browse files
colinmoycolinmoy
authored
feat(generator): handle Any fields with Value or Struct (#16377)
1 parent 5c916cf commit 581514e

4 files changed

Lines changed: 189 additions & 14 deletions

File tree

‎generator/internal/discovery_to_proto.cc‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,10 @@ StatusOr<std::string> GetImportForProtobufType(
108108
std::string const& protobuf_type) {
109109
static auto const* const kProtobufTypeImports =
110110
new std::unordered_map<std::string, std::string>{
111-
{"google.protobuf.Any", "google/protobuf/any.proto"}};
111+
{"google.protobuf.Any", "google/protobuf/any.proto"},
112+
{"google.protobuf.ListValue", "google/protobuf/struct.proto"},
113+
{"google.protobuf.Struct", "google/protobuf/struct.proto"},
114+
{"google.protobuf.Value", "google/protobuf/struct.proto"}};
112115

113116
auto iter = kProtobufTypeImports->find(protobuf_type);
114117
if (iter == kProtobufTypeImports->end()) {
@@ -353,23 +356,54 @@ std::set<std::string> FindAllTypesToImport(nlohmann::json const& json) {
353356
auto const* current = worklist.back();
354357
worklist.pop_back();
355358

356-
if (current->contains("type") && (*current)["type"] == "any") {
357-
types_to_import.insert("google.protobuf.Any");
358-
}
359359
if (current->contains("$ref")) {
360360
types_to_import.insert((*current)["$ref"]);
361361
}
362362

363+
if (current->contains("format")) {
364+
std::string const format = (*current)["format"];
365+
if (absl::StartsWith(format, "google.protobuf.")) {
366+
types_to_import.insert(format);
367+
// This node is resolved as a protobuf message (e.g. google.protobuf.Any
368+
// on Status.details items), so do not traverse into its internal
369+
// additionalProperties or properties.
370+
continue;
371+
}
372+
}
373+
374+
if (current->contains("type") && (*current)["type"] == "any") {
375+
types_to_import.insert("google.protobuf.Value");
376+
}
377+
363378
if (IsDiscoveryNestedType(*current) || current->contains("properties")) {
364379
for (auto const& f : (*current)["properties"]) {
365380
worklist.push_back(&f);
366381
}
367382
}
383+
368384
if (IsDiscoveryArrayType(*current)) {
369-
worklist.push_back(&(*current)["items"]);
385+
auto const& items = (*current)["items"];
386+
if (items.contains("type") && items["type"] == "object" &&
387+
items.contains("additionalProperties") &&
388+
items["additionalProperties"].value("type", "") == "any" &&
389+
!items.contains("format") &&
390+
!items["additionalProperties"].contains("format")) {
391+
types_to_import.insert("google.protobuf.Struct");
392+
} else {
393+
worklist.push_back(&items);
394+
}
370395
}
396+
371397
if (IsDiscoveryMapType(*current)) {
372-
worklist.push_back(&(*current)["additionalProperties"]);
398+
auto const& additional_properties = (*current)["additionalProperties"];
399+
if (additional_properties.contains("type") &&
400+
additional_properties["type"] == "any" &&
401+
!additional_properties.contains("format") &&
402+
!current->contains("format")) {
403+
types_to_import.insert("google.protobuf.Struct");
404+
} else {
405+
worklist.push_back(&additional_properties);
406+
}
373407
}
374408
}
375409

‎generator/internal/discovery_to_proto_test.cc‎

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,7 +1276,7 @@ TEST(FindAllTypesToImportTest, SimpleAnyField) {
12761276
auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false);
12771277
ASSERT_TRUE(parsed_json.is_object());
12781278
auto result = FindAllTypesToImport(parsed_json);
1279-
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Any"));
1279+
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Value"));
12801280
}
12811281

12821282
TEST(FindAllTypesToImportTest, MultipleSimpleRefFields) {
@@ -1345,7 +1345,7 @@ TEST(FindAllTypesToImportTest, ArrayRefAnyFields) {
13451345
auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false);
13461346
ASSERT_TRUE(parsed_json.is_object());
13471347
auto result = FindAllTypesToImport(parsed_json);
1348-
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Any", "Bar"));
1348+
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Struct", "Bar"));
13491349
}
13501350

13511351
TEST(FindAllTypesToImportTest, MapRefFields) {
@@ -1384,12 +1384,72 @@ TEST(FindAllTypesToImportTest, MapAnyFields) {
13841384
}
13851385
})""";
13861386

1387+
auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false);
1388+
ASSERT_TRUE(parsed_json.is_object());
1389+
auto result = FindAllTypesToImport(parsed_json);
1390+
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Struct"));
1391+
}
1392+
1393+
TEST(FindAllTypesToImportTest, StatusDetailsField) {
1394+
auto constexpr kTypeJson = R"""({
1395+
"properties": {
1396+
"details": {
1397+
"type": "array",
1398+
"items": {
1399+
"format": "google.protobuf.Any",
1400+
"type": "object",
1401+
"additionalProperties": {
1402+
"type": "any"
1403+
}
1404+
}
1405+
}
1406+
}
1407+
})""";
1408+
13871409
auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false);
13881410
ASSERT_TRUE(parsed_json.is_object());
13891411
auto result = FindAllTypesToImport(parsed_json);
13901412
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Any"));
13911413
}
13921414

1415+
TEST(FindAllTypesToImportTest, ArrayAnyWithFormat) {
1416+
auto constexpr kTypeJson = R"""({
1417+
"properties": {
1418+
"field_1": {
1419+
"type": "array",
1420+
"items": {
1421+
"type": "any",
1422+
"format": "google.protobuf.Struct"
1423+
}
1424+
}
1425+
}
1426+
})""";
1427+
1428+
auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false);
1429+
ASSERT_TRUE(parsed_json.is_object());
1430+
auto result = FindAllTypesToImport(parsed_json);
1431+
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Struct"));
1432+
}
1433+
1434+
TEST(FindAllTypesToImportTest, MapAnyWithFormat) {
1435+
auto constexpr kTypeJson = R"""({
1436+
"properties": {
1437+
"map_1": {
1438+
"type": "object",
1439+
"additionalProperties": {
1440+
"type": "any",
1441+
"format": "google.protobuf.Value"
1442+
}
1443+
}
1444+
}
1445+
})""";
1446+
1447+
auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false);
1448+
ASSERT_TRUE(parsed_json.is_object());
1449+
auto result = FindAllTypesToImport(parsed_json);
1450+
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Value"));
1451+
}
1452+
13931453
TEST(FindAllTypesToImportTest, SingleNestedRefField) {
13941454
auto constexpr kTypeJson = R"""({
13951455
"properties": {
@@ -2586,6 +2646,7 @@ TEST_F(AssignResourcesAndTypesToFilesTest, ResourceAndCommonFilesWithImports) {
25862646
"properties": {
25872647
"permissions": {
25882648
"items": {
2649+
"format": "google.protobuf.Any",
25892650
"type": "object",
25902651
"additionalProperties": {
25912652
"type": "any"

‎generator/internal/discovery_type_vertex.cc‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,13 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v,
110110
}
111111

112112
if (type == "any") {
113-
return TypeInfo{"google.protobuf.Any", compare_package_name,
114-
properties_for_synthesis, false, false};
113+
if (v.contains("format")) {
114+
type = v["format"];
115+
} else {
116+
type = "google.protobuf.Value";
117+
}
118+
return TypeInfo{type, compare_package_name, properties_for_synthesis, false,
119+
false};
115120
}
116121

117122
if (type == "object" &&
@@ -146,7 +151,14 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v,
146151
properties_for_synthesis = &additional_properties;
147152
is_message = true;
148153
} else if (map_type == "any") {
149-
return TypeInfo{"google.protobuf.Struct", compare_package_name,
154+
if (additional_properties.contains("format")) {
155+
map_type = additional_properties["format"];
156+
} else if (v.contains("format")) {
157+
map_type = v["format"];
158+
} else {
159+
map_type = "google.protobuf.Struct";
160+
}
161+
return TypeInfo{map_type, compare_package_name,
150162
properties_for_synthesis, true, is_message};
151163
} else {
152164
return internal::InvalidArgumentError(
@@ -181,13 +193,26 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v,
181193
scalar_type = CheckForScalarType(items);
182194
if (scalar_type) {
183195
type = *scalar_type;
196+
} else if (type == "any") {
197+
if (items.contains("format")) {
198+
type = items["format"];
199+
} else {
200+
type = "google.protobuf.Value";
201+
}
202+
return TypeInfo{type, compare_package_name, nullptr, false, false};
184203
} else if (type == "object" && items.contains("properties")) {
185204
// Synthesize a nested type for this array.
186205
type = CapitalizeFirstLetter(field_name + "Item");
187206
return TypeInfo{type, compare_package_name, &items, false, true};
188207
} else if (type == "object" && items.contains("additionalProperties") &&
189208
(items["additionalProperties"]).value("type", "") == "any") {
190-
type = "google.protobuf.Any";
209+
if (items.contains("format")) {
210+
type = items["format"];
211+
} else if (items["additionalProperties"].contains("format")) {
212+
type = items["additionalProperties"]["format"];
213+
} else {
214+
type = "google.protobuf.Struct";
215+
}
191216
return TypeInfo{type, compare_package_name, nullptr, false, false};
192217
} else {
193218
return internal::InvalidArgumentError(
@@ -544,6 +569,13 @@ StatusOr<int> DiscoveryTypeVertex::GetFieldNumber(
544569
}
545570

546571
if (field_descriptor->name() == field_name && type_name != field_type) {
572+
// Allow migration of google.protobuf.Any to google.protobuf.Struct or
573+
// google.protobuf.Value.
574+
if (absl::StrContains(type_name, "google.protobuf.Any") &&
575+
(absl::StrContains(field_type, "google.protobuf.Struct") ||
576+
absl::StrContains(field_type, "google.protobuf.Value"))) {
577+
return field_descriptor->number();
578+
}
547579
// Existing field type has changed. This is a breaking change.
548580
return internal::InvalidArgumentError(absl::StrFormat(
549581
"Message: %s has field: %s whose type has changed "

‎generator/internal/discovery_type_vertex_test.cc‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,16 @@ INSTANTIATE_TEST_SUITE_P(
220220
DetermineTypesSuccess{"string", R"""({"type":"string"})""", "string",
221221
true, false, false, false},
222222
DetermineTypesSuccess{"any", R"""({"type":"any"})""",
223-
"google.protobuf.Any", true, false, false, false},
223+
"google.protobuf.Value", true, false, false,
224+
false},
225+
DetermineTypesSuccess{
226+
"any_with_format",
227+
R"""({"type":"any","format":"google.protobuf.Value"})""",
228+
"google.protobuf.Value", true, false, false, false},
229+
DetermineTypesSuccess{
230+
"any_with_struct_format",
231+
R"""({"type":"any","format":"google.protobuf.Struct"})""",
232+
"google.protobuf.Struct", true, false, false, false},
224233
DetermineTypesSuccess{"boolean", R"""({"type":"boolean"})""", "bool",
225234
true, false, false, false},
226235
DetermineTypesSuccess{"integer_no_format", R"""({"type":"integer"})""",
@@ -246,6 +255,21 @@ INSTANTIATE_TEST_SUITE_P(
246255
DetermineTypesSuccess{
247256
"array_any",
248257
R"""({"type":"array","items":{"type":"object","additionalProperties":{"type":"any"}}})""",
258+
"google.protobuf.Struct", true, false, false, false},
259+
DetermineTypesSuccess{
260+
"array_any_with_format",
261+
R"""({"type":"array","items":{"type":"object","additionalProperties":{"type":"any","format":"google.protobuf.Value"}}})""",
262+
"google.protobuf.Value", true, false, false, false},
263+
DetermineTypesSuccess{
264+
"array_items_any", R"""({"type":"array","items":{"type":"any"}})""",
265+
"google.protobuf.Value", true, false, false, false},
266+
DetermineTypesSuccess{
267+
"array_items_any_with_format",
268+
R"""({"type":"array","items":{"type":"any","format":"google.protobuf.Struct"}})""",
269+
"google.protobuf.Struct", true, false, false, false},
270+
DetermineTypesSuccess{
271+
"status_details_any",
272+
R"""({"type":"array","items":{"type":"object","format":"google.protobuf.Any","additionalProperties":{"type":"any"}}})""",
249273
"google.protobuf.Any", true, false, false, false},
250274
DetermineTypesSuccess{
251275
"array_nested_message",
@@ -266,6 +290,14 @@ INSTANTIATE_TEST_SUITE_P(
266290
"any_to_struct",
267291
R"""({"type":"object","additionalProperties":{"type":"any"}})""",
268292
"google.protobuf.Struct", true, true, false, false},
293+
DetermineTypesSuccess{
294+
"map_any_with_format",
295+
R"""({"type":"object","additionalProperties":{"type":"any","format":"google.protobuf.Value"}})""",
296+
"google.protobuf.Value", true, true, false, false},
297+
DetermineTypesSuccess{
298+
"map_any_with_outer_format",
299+
R"""({"type":"object","format":"google.protobuf.Value","additionalProperties":{"type":"any"}})""",
300+
"google.protobuf.Value", true, true, false, false},
269301
DetermineTypesSuccess{
270302
"map_nested_message",
271303
R"""({"type":"object","additionalProperties":{"type":"object", "properties":{}}})""",
@@ -846,6 +878,7 @@ message Bar {}
846878
syntax = "proto3";
847879
package generator.test;
848880
881+
import "google/protobuf/any.proto";
849882
import "imported.proto";
850883
851884
message Foo {}
@@ -857,6 +890,8 @@ message FieldsOnly {
857890
int32 field4 = 4;
858891
repeated generator.imported.Bar field5 = 5;
859892
map<string, generator.imported.Bar> field6 = 6;
893+
google.protobuf.Any field7 = 7;
894+
google.protobuf.Any field8 = 8;
860895
}
861896
862897
)""";
@@ -878,7 +913,7 @@ message FieldsOnly {
878913
ASSERT_STATUS_OK(field_number);
879914
EXPECT_THAT(*field_number, Eq(1));
880915

881-
int const candidate_field_number = 7;
916+
int const candidate_field_number = 9;
882917
message_descriptor = file_descriptor->FindMessageTypeByName("FieldsOnly");
883918

884919
ASSERT_THAT(message_descriptor, NotNull());
@@ -923,6 +958,19 @@ message FieldsOnly {
923958
ASSERT_STATUS_OK(existing_map_different_package_field_number);
924959
EXPECT_THAT(*existing_map_different_package_field_number, Eq(6));
925960

961+
auto existing_any_to_struct_field_number =
962+
DiscoveryTypeVertex::GetFieldNumber(message_descriptor, "field7",
963+
"google.protobuf.Struct",
964+
candidate_field_number);
965+
ASSERT_STATUS_OK(existing_any_to_struct_field_number);
966+
EXPECT_THAT(*existing_any_to_struct_field_number, Eq(7));
967+
968+
auto existing_any_to_value_field_number = DiscoveryTypeVertex::GetFieldNumber(
969+
message_descriptor, "field8", "google.protobuf.Value",
970+
candidate_field_number);
971+
ASSERT_STATUS_OK(existing_any_to_value_field_number);
972+
EXPECT_THAT(*existing_any_to_value_field_number, Eq(8));
973+
926974
auto field_type_changed = DiscoveryTypeVertex::GetFieldNumber(
927975
message_descriptor, "field6", "map<string, generator.test.Bar>",
928976
candidate_field_number);

0 commit comments

Comments
 (0)