Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 40 additions & 6 deletions generator/internal/discovery_to_proto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,10 @@ StatusOr<std::string> GetImportForProtobufType(
std::string const& protobuf_type) {
static auto const* const kProtobufTypeImports =
new std::unordered_map<std::string, std::string>{
{"google.protobuf.Any", "google/protobuf/any.proto"}};
{"google.protobuf.Any", "google/protobuf/any.proto"},
{"google.protobuf.ListValue", "google/protobuf/struct.proto"},
{"google.protobuf.Struct", "google/protobuf/struct.proto"},
{"google.protobuf.Value", "google/protobuf/struct.proto"}};

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

if (current->contains("type") && (*current)["type"] == "any") {
types_to_import.insert("google.protobuf.Any");
}
if (current->contains("$ref")) {
types_to_import.insert((*current)["$ref"]);
}

if (current->contains("format")) {
std::string const format = (*current)["format"];
if (absl::StartsWith(format, "google.protobuf.")) {
types_to_import.insert(format);
// This node is resolved as a protobuf message (e.g. google.protobuf.Any
// on Status.details items), so do not traverse into its internal
// additionalProperties or properties.
continue;
}
}

if (current->contains("type") && (*current)["type"] == "any") {
types_to_import.insert("google.protobuf.Value");
}

if (IsDiscoveryNestedType(*current) || current->contains("properties")) {
for (auto const& f : (*current)["properties"]) {
worklist.push_back(&f);
}
}

if (IsDiscoveryArrayType(*current)) {
worklist.push_back(&(*current)["items"]);
auto const& items = (*current)["items"];
if (items.contains("type") && items["type"] == "object" &&
items.contains("additionalProperties") &&
items["additionalProperties"].value("type", "") == "any" &&
!items.contains("format") &&
!items["additionalProperties"].contains("format")) {
types_to_import.insert("google.protobuf.Struct");
} else {
worklist.push_back(&items);
}
}

if (IsDiscoveryMapType(*current)) {
worklist.push_back(&(*current)["additionalProperties"]);
auto const& additional_properties = (*current)["additionalProperties"];
if (additional_properties.contains("type") &&
additional_properties["type"] == "any" &&
!additional_properties.contains("format") &&
!current->contains("format")) {
types_to_import.insert("google.protobuf.Struct");
} else {
worklist.push_back(&additional_properties);
}
}
}

Expand Down
65 changes: 63 additions & 2 deletions generator/internal/discovery_to_proto_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1276,7 +1276,7 @@ TEST(FindAllTypesToImportTest, SimpleAnyField) {
auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false);
ASSERT_TRUE(parsed_json.is_object());
auto result = FindAllTypesToImport(parsed_json);
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Any"));
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Value"));
}

TEST(FindAllTypesToImportTest, MultipleSimpleRefFields) {
Expand Down Expand Up @@ -1345,7 +1345,7 @@ TEST(FindAllTypesToImportTest, ArrayRefAnyFields) {
auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false);
ASSERT_TRUE(parsed_json.is_object());
auto result = FindAllTypesToImport(parsed_json);
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Any", "Bar"));
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Struct", "Bar"));
}

TEST(FindAllTypesToImportTest, MapRefFields) {
Expand Down Expand Up @@ -1384,12 +1384,72 @@ TEST(FindAllTypesToImportTest, MapAnyFields) {
}
})""";

auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false);
ASSERT_TRUE(parsed_json.is_object());
auto result = FindAllTypesToImport(parsed_json);
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Struct"));
}

TEST(FindAllTypesToImportTest, StatusDetailsField) {
auto constexpr kTypeJson = R"""({
"properties": {
"details": {
"type": "array",
"items": {
"format": "google.protobuf.Any",
"type": "object",
"additionalProperties": {
"type": "any"
}
}
}
}
})""";

auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false);
ASSERT_TRUE(parsed_json.is_object());
auto result = FindAllTypesToImport(parsed_json);
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Any"));
}

TEST(FindAllTypesToImportTest, ArrayAnyWithFormat) {
auto constexpr kTypeJson = R"""({
"properties": {
"field_1": {
"type": "array",
"items": {
"type": "any",
"format": "google.protobuf.Struct"
}
}
}
})""";

auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false);
ASSERT_TRUE(parsed_json.is_object());
auto result = FindAllTypesToImport(parsed_json);
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Struct"));
}

TEST(FindAllTypesToImportTest, MapAnyWithFormat) {
auto constexpr kTypeJson = R"""({
"properties": {
"map_1": {
"type": "object",
"additionalProperties": {
"type": "any",
"format": "google.protobuf.Value"
}
}
}
})""";

auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false);
ASSERT_TRUE(parsed_json.is_object());
auto result = FindAllTypesToImport(parsed_json);
EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Value"));
}

TEST(FindAllTypesToImportTest, SingleNestedRefField) {
auto constexpr kTypeJson = R"""({
"properties": {
Expand Down Expand Up @@ -2586,6 +2646,7 @@ TEST_F(AssignResourcesAndTypesToFilesTest, ResourceAndCommonFilesWithImports) {
"properties": {
"permissions": {
"items": {
"format": "google.protobuf.Any",
"type": "object",
"additionalProperties": {
"type": "any"
Expand Down
40 changes: 36 additions & 4 deletions generator/internal/discovery_type_vertex.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,13 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v,
}

if (type == "any") {
return TypeInfo{"google.protobuf.Any", compare_package_name,
properties_for_synthesis, false, false};
if (v.contains("format")) {
type = v["format"];
} else {
type = "google.protobuf.Value";
}
return TypeInfo{type, compare_package_name, properties_for_synthesis, false,
false};
}

if (type == "object" &&
Expand Down Expand Up @@ -146,7 +151,14 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v,
properties_for_synthesis = &additional_properties;
is_message = true;
} else if (map_type == "any") {
return TypeInfo{"google.protobuf.Struct", compare_package_name,
if (additional_properties.contains("format")) {
map_type = additional_properties["format"];
} else if (v.contains("format")) {
map_type = v["format"];
} else {
map_type = "google.protobuf.Struct";
}
return TypeInfo{map_type, compare_package_name,
properties_for_synthesis, true, is_message};
} else {
return internal::InvalidArgumentError(
Expand Down Expand Up @@ -181,13 +193,26 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v,
scalar_type = CheckForScalarType(items);
if (scalar_type) {
type = *scalar_type;
} else if (type == "any") {
if (items.contains("format")) {
type = items["format"];
} else {
type = "google.protobuf.Value";
}
return TypeInfo{type, compare_package_name, nullptr, false, false};
} else if (type == "object" && items.contains("properties")) {
// Synthesize a nested type for this array.
type = CapitalizeFirstLetter(field_name + "Item");
return TypeInfo{type, compare_package_name, &items, false, true};
} else if (type == "object" && items.contains("additionalProperties") &&
(items["additionalProperties"]).value("type", "") == "any") {
type = "google.protobuf.Any";
if (items.contains("format")) {
type = items["format"];
} else if (items["additionalProperties"].contains("format")) {
type = items["additionalProperties"]["format"];
} else {
type = "google.protobuf.Struct";
}
return TypeInfo{type, compare_package_name, nullptr, false, false};
} else {
return internal::InvalidArgumentError(
Expand Down Expand Up @@ -544,6 +569,13 @@ StatusOr<int> DiscoveryTypeVertex::GetFieldNumber(
}

if (field_descriptor->name() == field_name && type_name != field_type) {
// Allow migration of google.protobuf.Any to google.protobuf.Struct or
// google.protobuf.Value.
if (absl::StrContains(type_name, "google.protobuf.Any") &&
(absl::StrContains(field_type, "google.protobuf.Struct") ||
absl::StrContains(field_type, "google.protobuf.Value"))) {
return field_descriptor->number();
}
// Existing field type has changed. This is a breaking change.
return internal::InvalidArgumentError(absl::StrFormat(
"Message: %s has field: %s whose type has changed "
Expand Down
52 changes: 50 additions & 2 deletions generator/internal/discovery_type_vertex_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,16 @@ INSTANTIATE_TEST_SUITE_P(
DetermineTypesSuccess{"string", R"""({"type":"string"})""", "string",
true, false, false, false},
DetermineTypesSuccess{"any", R"""({"type":"any"})""",
"google.protobuf.Any", true, false, false, false},
"google.protobuf.Value", true, false, false,
false},
DetermineTypesSuccess{
"any_with_format",
R"""({"type":"any","format":"google.protobuf.Value"})""",
"google.protobuf.Value", true, false, false, false},
DetermineTypesSuccess{
"any_with_struct_format",
R"""({"type":"any","format":"google.protobuf.Struct"})""",
"google.protobuf.Struct", true, false, false, false},
DetermineTypesSuccess{"boolean", R"""({"type":"boolean"})""", "bool",
true, false, false, false},
DetermineTypesSuccess{"integer_no_format", R"""({"type":"integer"})""",
Expand All @@ -246,6 +255,21 @@ INSTANTIATE_TEST_SUITE_P(
DetermineTypesSuccess{
"array_any",
R"""({"type":"array","items":{"type":"object","additionalProperties":{"type":"any"}}})""",
"google.protobuf.Struct", true, false, false, false},
DetermineTypesSuccess{
"array_any_with_format",
R"""({"type":"array","items":{"type":"object","additionalProperties":{"type":"any","format":"google.protobuf.Value"}}})""",
"google.protobuf.Value", true, false, false, false},
DetermineTypesSuccess{
"array_items_any", R"""({"type":"array","items":{"type":"any"}})""",
"google.protobuf.Value", true, false, false, false},
DetermineTypesSuccess{
"array_items_any_with_format",
R"""({"type":"array","items":{"type":"any","format":"google.protobuf.Struct"}})""",
"google.protobuf.Struct", true, false, false, false},
DetermineTypesSuccess{
"status_details_any",
R"""({"type":"array","items":{"type":"object","format":"google.protobuf.Any","additionalProperties":{"type":"any"}}})""",
"google.protobuf.Any", true, false, false, false},
DetermineTypesSuccess{
"array_nested_message",
Expand All @@ -266,6 +290,14 @@ INSTANTIATE_TEST_SUITE_P(
"any_to_struct",
R"""({"type":"object","additionalProperties":{"type":"any"}})""",
"google.protobuf.Struct", true, true, false, false},
DetermineTypesSuccess{
"map_any_with_format",
R"""({"type":"object","additionalProperties":{"type":"any","format":"google.protobuf.Value"}})""",
"google.protobuf.Value", true, true, false, false},
DetermineTypesSuccess{
"map_any_with_outer_format",
R"""({"type":"object","format":"google.protobuf.Value","additionalProperties":{"type":"any"}})""",
"google.protobuf.Value", true, true, false, false},
DetermineTypesSuccess{
"map_nested_message",
R"""({"type":"object","additionalProperties":{"type":"object", "properties":{}}})""",
Expand Down Expand Up @@ -846,6 +878,7 @@ message Bar {}
syntax = "proto3";
package generator.test;

import "google/protobuf/any.proto";
import "imported.proto";

message Foo {}
Expand All @@ -857,6 +890,8 @@ message FieldsOnly {
int32 field4 = 4;
repeated generator.imported.Bar field5 = 5;
map<string, generator.imported.Bar> field6 = 6;
google.protobuf.Any field7 = 7;
google.protobuf.Any field8 = 8;
}

)""";
Expand All @@ -878,7 +913,7 @@ message FieldsOnly {
ASSERT_STATUS_OK(field_number);
EXPECT_THAT(*field_number, Eq(1));

int const candidate_field_number = 7;
int const candidate_field_number = 9;
message_descriptor = file_descriptor->FindMessageTypeByName("FieldsOnly");

ASSERT_THAT(message_descriptor, NotNull());
Expand Down Expand Up @@ -923,6 +958,19 @@ message FieldsOnly {
ASSERT_STATUS_OK(existing_map_different_package_field_number);
EXPECT_THAT(*existing_map_different_package_field_number, Eq(6));

auto existing_any_to_struct_field_number =
DiscoveryTypeVertex::GetFieldNumber(message_descriptor, "field7",
"google.protobuf.Struct",
candidate_field_number);
ASSERT_STATUS_OK(existing_any_to_struct_field_number);
EXPECT_THAT(*existing_any_to_struct_field_number, Eq(7));

auto existing_any_to_value_field_number = DiscoveryTypeVertex::GetFieldNumber(
message_descriptor, "field8", "google.protobuf.Value",
candidate_field_number);
ASSERT_STATUS_OK(existing_any_to_value_field_number);
EXPECT_THAT(*existing_any_to_value_field_number, Eq(8));

auto field_type_changed = DiscoveryTypeVertex::GetFieldNumber(
message_descriptor, "field6", "map<string, generator.test.Bar>",
candidate_field_number);
Expand Down
Loading