Skip to content

Commit

Permalink
Bugfix for reflection-based Proto2Json: Always write default values i…
Browse files Browse the repository at this point in the history
…n maps
  • Loading branch information
vadim-xd committed Dec 22, 2023
1 parent d16b312 commit 002a753
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 8 deletions.
10 changes: 7 additions & 3 deletions library/cpp/protobuf/json/proto2json_printer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,9 @@ namespace NProtobufJson {
using namespace google::protobuf;
auto type = proto.GetDescriptor()->well_known_type();

// XXX static_cast will cause UB if used with dynamic messages
// (can be created by a DynamicMessageFactory with SetDelegateToGeneratedFactory(false). Unlikely, but still possible).
// See workaround with CopyFrom in JsonString2Duration, JsonString2Timestamp (json2proto.cpp)
if (type == Descriptor::WellKnownType::WELLKNOWNTYPE_DURATION) {
const auto& duration = static_cast<const Duration&>(proto);
json.Write(util::TimeUtil::ToString(duration));
Expand All @@ -210,7 +213,8 @@ namespace NProtobufJson {
void TProto2JsonPrinter::PrintSingleField(const Message& proto,
const FieldDescriptor& field,
IJsonOutput& json,
TStringBuf key) {
TStringBuf key,
bool inProtoMap) {
Y_ABORT_UNLESS(!field.is_repeated(), "field is repeated.");

if (!key) {
Expand All @@ -236,7 +240,7 @@ namespace NProtobufJson {

const Reflection* reflection = proto.GetReflection();

bool shouldPrintField = reflection->HasField(proto, &field);
bool shouldPrintField = inProtoMap || reflection->HasField(proto, &field);
if (!shouldPrintField && GetConfig().MissingSingleKeyMode == TProto2JsonConfig::MissingKeyExplicitDefaultThrowRequired) {
if (field.has_default_value()) {
shouldPrintField = true;
Expand Down Expand Up @@ -409,7 +413,7 @@ namespace NProtobufJson {
TString key = MakeKey(proto, *keyField);
const FieldDescriptor* valueField = proto.GetDescriptor()->FindFieldByName("value");
Y_ABORT_UNLESS(valueField, "Map entry value field not found.");
PrintField(proto, *valueField, json, key);
PrintSingleField(proto, *valueField, json, key, true);
}

TString TProto2JsonPrinter::MakeKey(const NProtoBuf::Message& proto,
Expand Down
2 changes: 1 addition & 1 deletion library/cpp/protobuf/json/proto2json_printer.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ namespace NProtobufJson {
void PrintSingleField(const NProtoBuf::Message& proto,
const NProtoBuf::FieldDescriptor& field,
IJsonOutput& json,
TStringBuf key = {});
TStringBuf key = {}, bool inProtoMap = false);

void PrintKeyValue(const NProtoBuf::Message& proto,
IJsonOutput& json);
Expand Down
44 changes: 42 additions & 2 deletions library/cpp/protobuf/json/ut/proto2json_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -981,7 +981,7 @@ Y_UNIT_TEST(TestMapAsObject) {
UNIT_ASSERT_JSON_STRINGS_EQUAL(jsonStr.Str(), modelStr);
} // TestMapAsObject

Y_UNIT_TEST(TestMapWTF) {
Y_UNIT_TEST(TestMapUsingGeneratedAsJSON) {
TMapType proto;

auto& items = *proto.MutableItems();
Expand All @@ -995,7 +995,47 @@ Y_UNIT_TEST(TestMapWTF) {
UNIT_ASSERT_NO_EXCEPTION(Proto2Json(proto, jsonStr));

UNIT_ASSERT_JSON_STRINGS_EQUAL(jsonStr.Str(), modelStr);
} // TestMapWTF
} // TestMapUsingGeneratedAsJSON

Y_UNIT_TEST(TestMapDefaultValue) {
TMapType proto;

auto& items = *proto.MutableItems();
items["key1"] = "";

TString modelStr(R"_({"Items":{"key1":""}})_");

TStringStream jsonStr;

TProto2JsonConfig config;
config.MapAsObject = true;
UNIT_ASSERT_NO_EXCEPTION(Proto2Json(proto, jsonStr, config));
UNIT_ASSERT_JSON_STRINGS_EQUAL(jsonStr.Str(), modelStr);

jsonStr.Clear();
UNIT_ASSERT_NO_EXCEPTION(Proto2Json(proto, jsonStr));
UNIT_ASSERT_JSON_STRINGS_EQUAL(jsonStr.Str(), modelStr);
} // TestMapDefaultValue

Y_UNIT_TEST(TestMapDefaultMessageValue) {
TComplexMapType proto;

auto& map = *proto.MutableNested();
map["key1"]; // Creates an empty nested message

TString modelStr(R"_({"Nested":{"key1":{}}})_");

TStringStream jsonStr;

TProto2JsonConfig config;
config.MapAsObject = true;
UNIT_ASSERT_NO_EXCEPTION(Proto2Json(proto, jsonStr, config));
UNIT_ASSERT_JSON_STRINGS_EQUAL(jsonStr.Str(), modelStr);

jsonStr.Clear();
UNIT_ASSERT_NO_EXCEPTION(Proto2Json(proto, jsonStr));
UNIT_ASSERT_JSON_STRINGS_EQUAL(jsonStr.Str(), modelStr);
} // TestMapDefaultMessageValue

Y_UNIT_TEST(TestStringifyNumbers) {
#define TEST_SINGLE(flag, field, value, expectString) \
Expand Down
2 changes: 0 additions & 2 deletions library/cpp/protobuf/json/ut/string_transform_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ Y_UNIT_TEST_SUITE(TDoubleEscapeTransform) {
TString s;
s = "aba\\ca\"ba";
transform.Transform(s);
Cerr << "###" << s << Endl;
UNIT_ASSERT_EQUAL(s, "aba\\\\\\\\ca\\\\\\\"ba");
}
}
Expand All @@ -52,7 +51,6 @@ Y_UNIT_TEST_SUITE(TDoubleUnescapeTransform) {
TString s;
s = "abacaba";
transform.Transform(s);
Cerr << "###" << s << Endl;
UNIT_ASSERT_EQUAL("abacaba", s);
}

Expand Down

0 comments on commit 002a753

Please sign in to comment.