-
Notifications
You must be signed in to change notification settings - Fork 459
feat(generator): add debug_redact to key fields and enable redact in DebugString #16398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
2509790
214eb62
c7f71dc
1e723fc
fb42587
672d0fc
83c0b09
c74ae87
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -360,7 +360,7 @@ std::set<std::string> FindAllTypesToImport(nlohmann::json const& json) { | |
| types_to_import.insert((*current)["$ref"]); | ||
| } | ||
|
|
||
| if (current->contains("format")) { | ||
| if (current->contains("format") && (*current)["format"].is_string()) { | ||
| std::string const format = (*current)["format"]; | ||
| if (absl::StartsWith(format, "google.protobuf.")) { | ||
| types_to_import.insert(format); | ||
|
|
@@ -383,8 +383,9 @@ std::set<std::string> FindAllTypesToImport(nlohmann::json const& json) { | |
|
|
||
| if (IsDiscoveryArrayType(*current)) { | ||
| auto const& items = (*current)["items"]; | ||
| if (items.contains("type") && items["type"] == "object" && | ||
| items.contains("additionalProperties") && | ||
| if (items.is_object() && items.contains("type") && | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly, if this isn't an object, then the discovery document is invalid. Emit an error and exit. Same below. If checks to verify the structure of the document fail, these are non-recoverable. |
||
| items["type"] == "object" && items.contains("additionalProperties") && | ||
| items["additionalProperties"].is_object() && | ||
| items["additionalProperties"].value("type", "") == "any" && | ||
| !items.contains("format") && | ||
| !items["additionalProperties"].contains("format")) { | ||
|
|
@@ -396,7 +397,8 @@ std::set<std::string> FindAllTypesToImport(nlohmann::json const& json) { | |
|
|
||
| if (IsDiscoveryMapType(*current)) { | ||
| auto const& additional_properties = (*current)["additionalProperties"]; | ||
| if (additional_properties.contains("type") && | ||
| if (additional_properties.is_object() && | ||
| additional_properties.contains("type") && | ||
| additional_properties["type"] == "any" && | ||
| !additional_properties.contains("format") && | ||
| !current->contains("format")) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,8 @@ | |
| #include "google/cloud/internal/algorithm.h" | ||
| #include "google/cloud/internal/make_status.h" | ||
| #include "google/cloud/log.h" | ||
| #include "absl/strings/ascii.h" | ||
| #include "absl/strings/match.h" | ||
| #include "absl/strings/str_format.h" | ||
| #include "absl/strings/str_join.h" | ||
| #include "absl/strings/str_replace.h" | ||
|
|
@@ -43,6 +45,27 @@ std::optional<std::string> CheckForScalarType(nlohmann::json const& j) { | |
| return std::nullopt; | ||
| } | ||
|
|
||
| bool IsStringOrBytes(nlohmann::json const& field_json) { | ||
| std::string const type = field_json.value("type", ""); | ||
| if (type == "string" || type == "bytes") return true; | ||
| if (type == "array" && field_json.contains("items") && | ||
| field_json["items"].is_object()) { | ||
| std::string const item_type = field_json["items"].value("type", ""); | ||
| if (item_type == "string" || item_type == "bytes") return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| bool ContainsKeyWord(std::string_view s) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer using string matching functions from absl (e.g. |
||
| for (std::size_t pos = s.find("key"); pos != std::string_view::npos; | ||
| pos = s.find("key", pos + 1)) { | ||
| bool const prefix_ok = (pos == 0 || s[pos - 1] == '_'); | ||
| bool const suffix_ok = (pos + 3 == s.size() || s[pos + 3] == '_'); | ||
| if (prefix_ok && suffix_ok) return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| DiscoveryTypeVertex::DiscoveryTypeVertex( | ||
|
|
@@ -110,7 +133,7 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v, | |
| } | ||
|
|
||
| if (type == "any") { | ||
| if (v.contains("format")) { | ||
| if (v.contains("format") && v["format"].is_string()) { | ||
| type = v["format"]; | ||
| } else { | ||
| type = "google.protobuf.Value"; | ||
|
|
@@ -151,9 +174,11 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v, | |
| properties_for_synthesis = &additional_properties; | ||
| is_message = true; | ||
| } else if (map_type == "any") { | ||
| if (additional_properties.contains("format")) { | ||
| if (additional_properties.is_object() && | ||
| additional_properties.contains("format") && | ||
| additional_properties["format"].is_string()) { | ||
| map_type = additional_properties["format"]; | ||
| } else if (v.contains("format")) { | ||
| } else if (v.contains("format") && v["format"].is_string()) { | ||
| map_type = v["format"]; | ||
| } else { | ||
| map_type = "google.protobuf.Struct"; | ||
|
|
@@ -194,21 +219,26 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v, | |
| if (scalar_type) { | ||
| type = *scalar_type; | ||
| } else if (type == "any") { | ||
| if (items.contains("format")) { | ||
| if (items.is_object() && items.contains("format") && | ||
| items["format"].is_string()) { | ||
| type = items["format"]; | ||
| } else { | ||
| type = "google.protobuf.Value"; | ||
| } | ||
| return TypeInfo{type, compare_package_name, nullptr, false, false}; | ||
| } else if (type == "object" && items.contains("properties")) { | ||
| } else if (type == "object" && items.is_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") { | ||
| if (items.contains("format")) { | ||
| } else if (type == "object" && items.is_object() && | ||
| items.contains("additionalProperties") && | ||
| items["additionalProperties"].is_object() && | ||
| items["additionalProperties"].value("type", "") == "any") { | ||
| if (items.contains("format") && items["format"].is_string()) { | ||
| type = items["format"]; | ||
| } else if (items["additionalProperties"].contains("format")) { | ||
| } else if (items["additionalProperties"].contains("format") && | ||
| items["additionalProperties"]["format"].is_string()) { | ||
| type = items["additionalProperties"]["format"]; | ||
| } else { | ||
| type = "google.protobuf.Struct"; | ||
|
|
@@ -495,6 +525,10 @@ std::string DiscoveryTypeVertex::FormatFieldOptions( | |
| absl::StrCat("\"", field_name, "\"")); | ||
| } | ||
|
|
||
| if (IsStringOrBytes(field_json) && ContainsKeyWord(field_name)) { | ||
| field_options.emplace_back("debug_redact", "true"); | ||
| } | ||
|
|
||
| // Discovery doc defined field names that are not always in strict | ||
| // camelCase, leading to translation issue between json and protobuf. Thus, | ||
| // the emitted proto fields need to have their name as it appears in the | ||
|
|
@@ -512,6 +546,8 @@ std::string DiscoveryTypeVertex::FormatFieldOptions( | |
| std::pair<std::string, std::string> const& p) { | ||
| if (p.first == "json_name") { | ||
| *s += absl::StrFormat("%s=\"%s\"", p.first, p.second); | ||
| } else if (p.first == "debug_redact") { | ||
| *s += absl::StrFormat("%s = %s", p.first, p.second); | ||
| } else { | ||
| *s += absl::StrFormat("(%s) = %s", p.first, p.second); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the discovery document contains a
formatfield that is not a string, this is a fundamental problem that the generator cannot recover from. We should emit an error and exit.