From cdd5ac6dc0b5a6e18cdc886339ffb40f5e51ed25 Mon Sep 17 00:00:00 2001 From: Tony Liao Date: Fri, 9 Aug 2024 15:38:22 -0700 Subject: [PATCH] Enable testing of more serialization codepaths for implicit presence fields. SerializeToString and SerializeToCord, for example, exercise codepaths that are actually quite different, but a user would be reasonable to expect that strings and cords are largely plug-and-play substitutable. The templating around TYPED_TEST can eventually make it easy to add coverage for other types as well, for example streams. PiperOrigin-RevId: 661427989 --- src/google/protobuf/BUILD.bazel | 1 + src/google/protobuf/no_field_presence_test.cc | 284 +++++++++++------- 2 files changed, 174 insertions(+), 111 deletions(-) diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index a161315ecb88..e1a6487192fa 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -1659,6 +1659,7 @@ cc_test( ":protobuf", "@com_google_absl//absl/log:absl_check", "@com_google_absl//absl/memory", + "@com_google_absl//absl/strings:cord", "@com_google_absl//absl/strings:string_view", "@com_google_googletest//:gtest", "@com_google_googletest//:gtest_main", diff --git a/src/google/protobuf/no_field_presence_test.cc b/src/google/protobuf/no_field_presence_test.cc index cd556703568f..e57b11b029be 100644 --- a/src/google/protobuf/no_field_presence_test.cc +++ b/src/google/protobuf/no_field_presence_test.cc @@ -8,12 +8,14 @@ #include #include #include +#include #include "google/protobuf/descriptor.pb.h" #include #include #include "absl/log/absl_check.h" #include "absl/memory/memory.h" +#include "absl/strings/cord.h" #include "absl/strings/string_view.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/unittest.pb.h" @@ -401,99 +403,6 @@ TEST(NoFieldPresenceTest, HasFieldOneofsTest) { EXPECT_EQ(false, r->HasField(message, desc_oneof_string)); } -TEST(NoFieldPresenceTest, DontSerializeDefaultValuesTest) { - // check that serialized data contains only non-zero numeric fields/non-empty - // string/byte fields. - proto2_nofieldpresence_unittest::TestAllTypes message; - std::string output; - - // All default values -> no output. - message.SerializeToString(&output); - EXPECT_EQ(0, output.size()); - - // Zero values -> still no output. - message.set_optional_int32(0); - message.set_optional_int64(0); - message.set_optional_uint32(0); - message.set_optional_uint64(0); - message.set_optional_sint32(0); - message.set_optional_sint64(0); - message.set_optional_fixed32(0); - message.set_optional_fixed64(0); - message.set_optional_sfixed32(0); - message.set_optional_sfixed64(0); - message.set_optional_float(0); - message.set_optional_double(0); - message.set_optional_bool(0); - message.set_optional_string(""); - message.set_optional_bytes(""); - message.set_optional_nested_enum( - proto2_nofieldpresence_unittest::TestAllTypes::FOO); // first enum entry - message.set_optional_foreign_enum( - proto2_nofieldpresence_unittest::FOREIGN_FOO); // first enum entry - - message.SerializeToString(&output); - EXPECT_EQ(0, output.size()); - - message.set_optional_int32(1); - message.SerializeToString(&output); - EXPECT_EQ(2, output.size()); - EXPECT_EQ("\x08\x01", output); - - message.set_optional_int32(0); - message.SerializeToString(&output); - EXPECT_EQ(0, output.size()); -} - -TEST(NoFieldPresenceTest, NullMutableSerializesEmpty) { - // Check that, if mutable_foo() was called, but fields were not modified, - // nothing is serialized on the wire. - proto2_nofieldpresence_unittest::TestAllTypes message; - std::string output; - - // All default values -> no output. - ASSERT_TRUE(message.SerializeToString(&output)); - EXPECT_TRUE(output.empty()); - - // No-op mutable calls -> no output. - message.mutable_optional_string(); - message.mutable_optional_bytes(); - ASSERT_TRUE(message.SerializeToString(&output)); - EXPECT_TRUE(output.empty()); - - // Assign to nonempty string -> some output. - *message.mutable_optional_bytes() = "bar"; - ASSERT_TRUE(message.SerializeToString(&output)); - EXPECT_THAT(output.size(), Gt(3)); // 3-byte-long string + tag/value + len -} - -TEST(NoFieldPresenceTest, SetAllocatedAndReleaseTest) { - // Check that setting an empty string via set_allocated_foo behaves properly; - // Check that serializing after release_foo does not generate output for foo. - proto2_nofieldpresence_unittest::TestAllTypes message; - std::string output; - - // All default values -> no output. - ASSERT_TRUE(message.SerializeToString(&output)); - EXPECT_TRUE(output.empty()); - - auto allocated_bytes = std::make_unique("test"); - message.set_allocated_optional_bytes(allocated_bytes.release()); - ASSERT_TRUE(message.SerializeToString(&output)); - EXPECT_THAT(output.size(), Gt(4)); // 4-byte-long string + tag/value + len - - size_t former_output_size = output.size(); - - auto allocated_string = std::make_unique(""); - message.set_allocated_optional_string(allocated_string.release()); - ASSERT_TRUE(message.SerializeToString(&output)); - EXPECT_EQ(former_output_size, output.size()); // empty string not serialized. - - auto bytes_ptr = absl::WrapUnique(message.release_optional_bytes()); - ASSERT_TRUE(message.SerializeToString(&output)); - EXPECT_TRUE(output.empty()); // released fields are not serialized. -} - TEST(NoFieldPresenceTest, MergeFromIfNonzeroTest) { // check that MergeFrom copies if nonzero/nondefault only. proto2_nofieldpresence_unittest::TestAllTypes source; @@ -596,7 +505,160 @@ TEST(NoFieldPresenceTest, IsInitializedTest) { EXPECT_EQ(true, message.IsInitialized()); } -TEST(NoFieldPresenceTest, LazyMessageFieldHasBit) { +// TODO: b/358616816 - `if constexpr` can be used here once C++17 is baseline. +template +bool TestSerialize(const MessageLite& message, T* output); + +template <> +bool TestSerialize(const MessageLite& message, + std::string* output) { + return message.SerializeToString(output); +} + +template <> +bool TestSerialize(const MessageLite& message, absl::Cord* output) { + return message.SerializeToCord(output); +} + +template +class NoFieldPresenceSerializeTest : public testing::Test { + public: + T& GetOutputSinkRef() { return value_; } + std::string GetOutput() { return std::string{value_}; } + + protected: + // Cargo-culted from: + // https://google.github.io/googletest/reference/testing.html#TYPED_TEST_SUITE + T value_; +}; + +using SerializableOutputTypes = ::testing::Types; + +// TODO: b/358616816 - `if constexpr` can be used here once C++17 is baseline. +// https://google.github.io/googletest/reference/testing.html#TYPED_TEST_SUITE +#ifdef __cpp_if_constexpr +// Providing the NameGenerator produces slightly more readable output in the +// test invocation summary (type names are displayed instead of numbers). +class NameGenerator { + public: + template + static std::string GetName(int) { + if constexpr (std::is_same_v) { + return "string"; + } else if constexpr (std::is_same_v) { + return "Cord"; + } else { + static_assert( + std::is_same_v || std::is_same_v, + "unsupported type"); + } + } +}; + +TYPED_TEST_SUITE(NoFieldPresenceSerializeTest, SerializableOutputTypes, + NameGenerator); +#else +TYPED_TEST_SUITE(NoFieldPresenceSerializeTest, SerializableOutputTypes); +#endif + +TYPED_TEST(NoFieldPresenceSerializeTest, DontSerializeDefaultValuesTest) { + // check that serialized data contains only non-zero numeric fields/non-empty + // string/byte fields. + proto2_nofieldpresence_unittest::TestAllTypes message; + TypeParam& output_sink = this->GetOutputSinkRef(); + + // All default values -> no output. + ASSERT_TRUE(TestSerialize(message, &output_sink)); + EXPECT_EQ(0, this->GetOutput().size()); + + // Zero values -> still no output. + message.set_optional_int32(0); + message.set_optional_int64(0); + message.set_optional_uint32(0); + message.set_optional_uint64(0); + message.set_optional_sint32(0); + message.set_optional_sint64(0); + message.set_optional_fixed32(0); + message.set_optional_fixed64(0); + message.set_optional_sfixed32(0); + message.set_optional_sfixed64(0); + message.set_optional_float(0); + message.set_optional_double(0); + message.set_optional_bool(false); + message.set_optional_string(""); + message.set_optional_bytes(""); + message.set_optional_nested_enum( + proto2_nofieldpresence_unittest::TestAllTypes::FOO); // first enum entry + message.set_optional_foreign_enum( + proto2_nofieldpresence_unittest::FOREIGN_FOO); // first enum entry + + ASSERT_TRUE(TestSerialize(message, &output_sink)); + EXPECT_EQ(0, this->GetOutput().size()); + + message.set_optional_int32(1); + ASSERT_TRUE(TestSerialize(message, &output_sink)); + EXPECT_EQ(2, this->GetOutput().size()); + EXPECT_EQ("\x08\x01", this->GetOutput()); + + message.set_optional_int32(0); + ASSERT_TRUE(TestSerialize(message, &output_sink)); + EXPECT_EQ(0, this->GetOutput().size()); +} + +TYPED_TEST(NoFieldPresenceSerializeTest, NullMutableSerializesEmpty) { + // Check that, if mutable_foo() was called, but fields were not modified, + // nothing is serialized on the wire. + proto2_nofieldpresence_unittest::TestAllTypes message; + TypeParam& output_sink = this->GetOutputSinkRef(); + + // All default values -> no output. + ASSERT_TRUE(TestSerialize(message, &output_sink)); + EXPECT_TRUE(this->GetOutput().empty()); + + // No-op mutable calls -> no output. + message.mutable_optional_string(); + message.mutable_optional_bytes(); + ASSERT_TRUE(TestSerialize(message, &output_sink)); + EXPECT_TRUE(this->GetOutput().empty()); + + // Assign to nonempty string -> some output. + *message.mutable_optional_bytes() = "bar"; + ASSERT_TRUE(TestSerialize(message, &output_sink)); + EXPECT_THAT(this->GetOutput().size(), + Gt(3)); // 3-byte-long string + tag/value + len +} + +TYPED_TEST(NoFieldPresenceSerializeTest, SetAllocatedAndReleaseTest) { + // Check that setting an empty string via set_allocated_foo behaves properly; + // Check that serializing after release_foo does not generate output for foo. + proto2_nofieldpresence_unittest::TestAllTypes message; + TypeParam& output_sink = this->GetOutputSinkRef(); + + // All default values -> no output. + ASSERT_TRUE(TestSerialize(message, &output_sink)); + EXPECT_TRUE(this->GetOutput().empty()); + + auto allocated_bytes = std::make_unique("test"); + message.set_allocated_optional_bytes(allocated_bytes.release()); + ASSERT_TRUE(TestSerialize(message, &output_sink)); + EXPECT_THAT(this->GetOutput().size(), + Gt(4)); // 4-byte-long string + tag/value + len + + size_t former_output_size = this->GetOutput().size(); + + auto allocated_string = std::make_unique(""); + message.set_allocated_optional_string(allocated_string.release()); + ASSERT_TRUE(TestSerialize(message, &output_sink)); + // empty string not serialized. + EXPECT_EQ(former_output_size, this->GetOutput().size()); + + auto bytes_ptr = absl::WrapUnique(message.release_optional_bytes()); + ASSERT_TRUE(TestSerialize(message, &output_sink)); + EXPECT_TRUE( + this->GetOutput().empty()); // released fields are not serialized. +} + +TYPED_TEST(NoFieldPresenceSerializeTest, LazyMessageFieldHasBit) { // Check that has-bit interaction with lazy message works (has-bit before and // after lazy decode). proto2_nofieldpresence_unittest::TestAllTypes message; @@ -614,10 +676,10 @@ TEST(NoFieldPresenceTest, LazyMessageFieldHasBit) { // Serialize and parse with a new message object so that lazy field on new // object is in unparsed state. - std::string output; - message.SerializeToString(&output); + TypeParam& output_sink = this->GetOutputSinkRef(); + ASSERT_TRUE(TestSerialize(message, &output_sink)); proto2_nofieldpresence_unittest::TestAllTypes message2; - message2.ParseFromString(output); + message2.ParseFromString(this->GetOutput()); EXPECT_EQ(true, message2.has_optional_lazy_message()); EXPECT_EQ(true, r->HasField(message2, field)); @@ -628,32 +690,32 @@ TEST(NoFieldPresenceTest, LazyMessageFieldHasBit) { EXPECT_EQ(true, r->HasField(message2, field)); } -TEST(NoFieldPresenceTest, OneofPresence) { +TYPED_TEST(NoFieldPresenceSerializeTest, OneofPresence) { proto2_nofieldpresence_unittest::TestAllTypes message; // oneof fields still have field presence -- ensure that this goes on the wire // even though its value is the empty string. message.set_oneof_string(""); - std::string serialized; - message.SerializeToString(&serialized); + TypeParam& output_sink = this->GetOutputSinkRef(); + ASSERT_TRUE(TestSerialize(message, &output_sink)); // Tag: 113 --> tag is (113 << 3) | 2 (length delimited) = 906 // varint: 0x8a 0x07 // Length: 0x00 - EXPECT_EQ(3, serialized.size()); - EXPECT_EQ(static_cast(0x8a), serialized.at(0)); - EXPECT_EQ(static_cast(0x07), serialized.at(1)); - EXPECT_EQ(static_cast(0x00), serialized.at(2)); + EXPECT_EQ(3, this->GetOutput().size()); + EXPECT_EQ(static_cast(0x8a), this->GetOutput().at(0)); + EXPECT_EQ(static_cast(0x07), this->GetOutput().at(1)); + EXPECT_EQ(static_cast(0x00), this->GetOutput().at(2)); message.Clear(); - EXPECT_TRUE(message.ParseFromString(serialized)); + EXPECT_TRUE(message.ParseFromString(this->GetOutput())); EXPECT_EQ(proto2_nofieldpresence_unittest::TestAllTypes::kOneofString, message.oneof_field_case()); // Also test int32 and enum fields. message.Clear(); message.set_oneof_uint32(0); // would not go on wire if ordinary field. - message.SerializeToString(&serialized); - EXPECT_EQ(3, serialized.size()); - EXPECT_TRUE(message.ParseFromString(serialized)); + ASSERT_TRUE(TestSerialize(message, &output_sink)); + EXPECT_EQ(3, this->GetOutput().size()); + EXPECT_TRUE(message.ParseFromString(this->GetOutput())); EXPECT_EQ(proto2_nofieldpresence_unittest::TestAllTypes::kOneofUint32, message.oneof_field_case()); @@ -661,9 +723,9 @@ TEST(NoFieldPresenceTest, OneofPresence) { message.set_oneof_enum( proto2_nofieldpresence_unittest::TestAllTypes::FOO); // default // value. - message.SerializeToString(&serialized); - EXPECT_EQ(3, serialized.size()); - EXPECT_TRUE(message.ParseFromString(serialized)); + ASSERT_TRUE(TestSerialize(message, &output_sink)); + EXPECT_EQ(3, this->GetOutput().size()); + EXPECT_TRUE(message.ParseFromString(this->GetOutput())); EXPECT_EQ(proto2_nofieldpresence_unittest::TestAllTypes::kOneofEnum, message.oneof_field_case());