Skip to content

Commit

Permalink
fix: Drop keyGroupOutput objects that do not have keyValues
Browse files Browse the repository at this point in the history
Bug: 388567854
Fixed: 388567854
Change-Id: Ie93d6302f05ebd6afb45f59d3212eb5742d300c8
GitOrigin-RevId: c05e21e4a2b8971a0d0b04b5264e26d6d9ca2f51
  • Loading branch information
lusayaa authored and copybara-github committed Jan 9, 2025
1 parent 21899f9 commit f8fa8de
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 0 deletions.
3 changes: 3 additions & 0 deletions components/data/converters/cbor_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ absl::StatusOr<cbor_item_t*> EncodePartitionOutput(
cbor_new_definite_array(partition_output.key_group_outputs().size());
for (auto& key_group_output :
*(partition_output.mutable_key_group_outputs())) {
if (key_group_output.key_values().empty()) {
continue;
}
PS_ASSIGN_OR_RETURN(auto* serialized_key_group_output,
EncodeKeyGroupOutput(key_group_output));
if (!cbor_array_push(serialized_key_group_outputs,
Expand Down
53 changes: 53 additions & 0 deletions components/data/converters/cbor_converter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,59 @@ TEST(CborConverterTest, PartitionOutputsCborEncodeSuccess) {
EXPECT_EQ(json_etalon, ordered_json::from_cbor(*cbor_encoded_proto_maybe));
}

TEST(CborConverterTest,
PartitionOutputsCborEncodeIgnoresKeyGroupOutputWithoutKVsSuccess) {
ordered_json json_etalon = ordered_json::parse(R"(
[
{
"id": 0,
"keyGroupOutputs": []
},
{
"id": 1,
"keyGroupOutputs": [
{
"tags": [
"custom",
"keys"
],
"keyValues": {
"hello2": {
"value": "world2"
}
}
}
]
}
]
)");

application_pa::V2CompressionGroup compression_group;
TextFormat::ParseFromString(
R"pb(partition_outputs {
id: 0
key_group_outputs { tags: "custom" tags: "keys" }
}
partition_outputs {
id: 1
key_group_outputs {
tags: "custom"
tags: "keys"
key_values {
key: "hello2"
value { value { string_value: "world2" } }
}
}
})pb",
&compression_group);
absl::StatusOr<std::string> cbor_encoded_proto_maybe =
PartitionOutputsCborEncode(
*compression_group.mutable_partition_outputs());
ASSERT_TRUE(cbor_encoded_proto_maybe.ok())
<< cbor_encoded_proto_maybe.status();
EXPECT_EQ(json_etalon, ordered_json::from_cbor(*cbor_encoded_proto_maybe));
}

TEST(CborConverterTest, PartitionOutputsCborEncodeEmptyKeyGroupOutputsSuccess) {
ordered_json json_etalon = nlohmann::ordered_json::parse(R"(
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,56 @@ TEST(CborEncoderTest, EncodePartitionOutputsSuccess) {
EXPECT_EQ(expected_partition_outputs, json::from_cbor(*maybe_cbor_content));
}

TEST(CborEncoderTest,
EncodePartitionOutputsIgnoresKeyGroupOutputWithoutKVsSuccess) {
InitMetricsContextMap();
json json_partition_output1 = R"(
{
"keyGroupOutputs": [
{
"keyValues": {
},
"tags": [
"custom",
"keys"
]
}
]
})"_json;
json json_partition_output2 = R"(
{
"keyGroupOutputs": [
{
"tags": [
"custom",
"keys"
]
}
]
})"_json;
std::vector<std::pair<int32_t, std::string>> partition_output_pairs = {
{1, json_partition_output1.dump()}, {2, json_partition_output2.dump()}};

auto request_context_factory = std::make_unique<RequestContextFactory>();
CborV2EncoderDecoder encoder;
const auto maybe_cbor_content = encoder.EncodePartitionOutputs(
partition_output_pairs, *request_context_factory);

json expected_empty_json_output = R"(
{
"keyGroupOutputs": []
})"_json;

json expected_partition_output1 = {{"id", 1}};
expected_partition_output1.update(expected_empty_json_output);
json expected_partition_output2 = {{"id", 2}};
expected_partition_output2.update(expected_empty_json_output);
json expected_partition_outputs = {expected_partition_output1,
expected_partition_output2};
ASSERT_TRUE(maybe_cbor_content.ok()) << maybe_cbor_content.status();
EXPECT_EQ(expected_partition_outputs, json::from_cbor(*maybe_cbor_content));
}

TEST(CborEncoderTest, EncodePartitionOutputsEmptyKeyGroupOutputSuccess) {
InitMetricsContextMap();
json json_partition_output = R"(
Expand Down

0 comments on commit f8fa8de

Please sign in to comment.