From eb8d84e55490007f8366eb20bac20e584b4284b1 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Thu, 14 Jul 2022 21:09:25 +0000 Subject: [PATCH 1/2] Updated CHANGES.txt. --- CHANGES.txt | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index fc30d1ae22153..1b831847597e5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,19 +1,18 @@ 2022-07-01 Unreleased version - * Add TransferAllMessages() util function - * Break ZeroCopyOutputByteSink into its own file. - * Explicitly instantiate InternalMetadata members at UnknownFieldSet. - * Support kdoc for Kotlin - * Delete unused function: IsProto3Field. + C++ + * Reduced .pb.o object file size slightly by explicitly instantiating + InternalMetadata templates in the runtime. * Add C++20 keywords guarded by PROTOBUF_FUTURE_CPP20_KEYWORDS - * Escape Kotlin keywords in package names in proto generated code - * Fix StrAppend crashing on empty strings. - -2022-06-27 Unreleased version - * Handle reflection for message splitting. - * make metadata fields lazy. - * Extend visibility of plugin library to upb - * Modernize conformance_cpp.cc. - * Don't request 64-byte alignment unless the toolchain supports it. + * Fixed crash in ThreadLocalStorage for pre-C++17 compilers on 32-bit ARM. + * Clarified that JSON API non-OK statuses are not a stable API. + + Kotlin + * Kotlin generated code comments now use kdoc format instead of javadoc. + * Escape keywords in package names in proto generated code + + Java + * Performance improvement for repeated use of FieldMaskUtil#merge by caching + constructed FieldMaskTrees. 2022-06-27 version 21.2 (C++/Java/Python/PHP/Objective-C/C#/Ruby) C++ From a0e3399cd5ab048a62136e60508a0e2379ec654d Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Thu, 14 Jul 2022 22:13:36 +0000 Subject: [PATCH 2/2] Sync from Piper @461060848 PROTOBUF_SYNC_PIPER --- build_files_updated_unittest.sh | 62 ---------- cmake/update_file_lists.sh | 8 ++ src/google/protobuf/util/json_util_test.cc | 133 ++++++++++----------- 3 files changed, 72 insertions(+), 131 deletions(-) delete mode 100755 build_files_updated_unittest.sh create mode 100755 cmake/update_file_lists.sh diff --git a/build_files_updated_unittest.sh b/build_files_updated_unittest.sh deleted file mode 100755 index 87541c35faa2d..0000000000000 --- a/build_files_updated_unittest.sh +++ /dev/null @@ -1,62 +0,0 @@ -#!/bin/bash - -# This script verifies that BUILD files and cmake files are in sync with src/Makefile.am - -set -eo pipefail - -if [ "$(uname)" != "Linux" ]; then - echo "build_files_updated_unittest only supported on Linux. Skipping..." - exit 0 -fi - -# Keep in sync with files needed by update_file_lists.sh -generated_files=( - "BUILD" - "cmake/extract_includes.bat.in" - "cmake/libprotobuf-lite.cmake" - "cmake/libprotobuf.cmake" - "cmake/libprotoc.cmake" - "cmake/tests.cmake" - "src/Makefile.am" -) - -# If we're running in Bazel, use the Bazel-provided temp-dir. -if [ -n "${TEST_TMPDIR}" ]; then - # Env-var TEST_TMPDIR is set, assume that this is Bazel. - # Bazel may have opinions whether we are allowed to delete TEST_TMPDIR. - test_root="${TEST_TMPDIR}/build_files_updated_unittest" - mkdir "${test_root}" -else - # Seems like we're not executed by Bazel. - test_root=$(mktemp -d) -fi - -# From now on, fail if there are any unbound variables. -set -u - -# Remove artifacts after test is finished. -function cleanup { - rm -rf "${test_root}" -} -trap cleanup EXIT - -# Create golden dir and add snapshot of current state. -golden_dir="${test_root}/golden" -mkdir -p "${golden_dir}/cmake" "${golden_dir}/src" -for file in ${generated_files[@]}; do - cp "${file}" "${golden_dir}/${file}" -done - -# Create test dir, copy current state into it, and execute update script. -test_dir="${test_root}/test" -cp -R "${golden_dir}" "${test_dir}" - -cp "update_file_lists.sh" "${test_dir}/update_file_lists.sh" -chmod +x "${test_dir}/update_file_lists.sh" -cd "${test_root}/test" -bash "${test_dir}/update_file_lists.sh" - -# Test whether there are any differences -for file in ${generated_files[@]}; do - diff -du "${golden_dir}/${file}" "${test_dir}/${file}" -done diff --git a/cmake/update_file_lists.sh b/cmake/update_file_lists.sh new file mode 100755 index 0000000000000..a08bdf313806e --- /dev/null +++ b/cmake/update_file_lists.sh @@ -0,0 +1,8 @@ +#!/bin/bash -u + +# This script generates file lists from Bazel for CMake. + +set -e + +bazel build //pkg:gen_src_file_lists +cp -v bazel-bin/pkg/src_file_lists.cmake src/file_lists.cmake diff --git a/src/google/protobuf/util/json_util_test.cc b/src/google/protobuf/util/json_util_test.cc index 9401d418d7b89..1c7207217e10c 100644 --- a/src/google/protobuf/util/json_util_test.cc +++ b/src/google/protobuf/util/json_util_test.cc @@ -74,6 +74,8 @@ using ::proto3::TestMessage; using ::proto3::TestOneof; using ::proto3::TestWrapper; using ::proto_util_converter::testing::MapIn; +using ::testing::ElementsAre; +using ::testing::SizeIs; // TODO(b/234474291): Use the gtest versions once that's available in OSS. MATCHER_P(IsOkAndHolds, inner, @@ -301,9 +303,8 @@ TEST_P(JsonTest, TestAlwaysPrintEnumsAsInts) { ASSERT_OK(parsed); EXPECT_EQ(parsed->enum_value(), proto3::BAR); - EXPECT_EQ(parsed->repeated_enum_value_size(), 2); - EXPECT_EQ(parsed->repeated_enum_value(0), proto3::FOO); - EXPECT_EQ(parsed->repeated_enum_value(1), proto3::BAR); + EXPECT_THAT(parsed->repeated_enum_value(), + ElementsAre(proto3::FOO, proto3::BAR)); } TEST_P(JsonTest, TestPrintEnumsAsIntsWithDefaultValue) { @@ -398,31 +399,14 @@ TEST_P(JsonTest, ParseMessage) { EXPECT_EQ(m->enum_value(), proto3::EnumType::BAR); EXPECT_EQ(m->message_value().value(), 2048); - ASSERT_EQ(m->repeated_bool_value_size(), 1); - EXPECT_TRUE(m->repeated_bool_value(0)); + EXPECT_THAT(m->repeated_bool_value(), ElementsAre(true)); + EXPECT_THAT(m->repeated_int32_value(), ElementsAre(0, -42)); + EXPECT_THAT(m->repeated_uint64_value(), ElementsAre(1, 2)); + EXPECT_THAT(m->repeated_double_value(), ElementsAre(1.5, -2)); + EXPECT_THAT(m->repeated_string_value(), ElementsAre("foo", "bar ", "")); + EXPECT_THAT(m->repeated_enum_value(), ElementsAre(proto3::BAR, proto3::FOO)); - ASSERT_EQ(m->repeated_int32_value_size(), 2); - EXPECT_EQ(m->repeated_int32_value(0), 0); - EXPECT_EQ(m->repeated_int32_value(1), -42); - - ASSERT_EQ(m->repeated_uint64_value_size(), 2); - EXPECT_EQ(m->repeated_uint64_value(0), 1); - EXPECT_EQ(m->repeated_uint64_value(1), 2); - - ASSERT_EQ(m->repeated_double_value_size(), 2); - EXPECT_EQ(m->repeated_double_value(0), 1.5); - EXPECT_EQ(m->repeated_double_value(1), -2); - - ASSERT_EQ(m->repeated_string_value_size(), 3); - EXPECT_EQ(m->repeated_string_value(0), "foo"); - EXPECT_EQ(m->repeated_string_value(1), "bar "); - EXPECT_EQ(m->repeated_string_value(2), ""); - - ASSERT_EQ(m->repeated_enum_value_size(), 2); - EXPECT_EQ(m->repeated_enum_value(0), proto3::EnumType::BAR); - EXPECT_EQ(m->repeated_enum_value(1), proto3::EnumType::FOO); - - ASSERT_EQ(m->repeated_message_value_size(), 2); + ASSERT_THAT(m->repeated_message_value(), SizeIs(2)); EXPECT_EQ(m->repeated_message_value(0).value(), 40); EXPECT_EQ(m->repeated_message_value(1).value(), 96); @@ -445,19 +429,9 @@ TEST_P(JsonTest, CurseOfAtob) { } )json"); ASSERT_OK(m); - EXPECT_EQ(m->repeated_bool_value_size(), 10); - - EXPECT_FALSE(m->repeated_bool_value(0)); - EXPECT_FALSE(m->repeated_bool_value(2)); - EXPECT_FALSE(m->repeated_bool_value(4)); - EXPECT_FALSE(m->repeated_bool_value(6)); - EXPECT_FALSE(m->repeated_bool_value(8)); - - EXPECT_TRUE(m->repeated_bool_value(1)); - EXPECT_TRUE(m->repeated_bool_value(3)); - EXPECT_TRUE(m->repeated_bool_value(5)); - EXPECT_TRUE(m->repeated_bool_value(7)); - EXPECT_TRUE(m->repeated_bool_value(9)); + EXPECT_THAT(m->repeated_bool_value(), + ElementsAre(false, true, false, true, false, true, false, true, + false, true)); } TEST_P(JsonTest, ParseLegacySingleRepeatedField) { @@ -469,16 +443,11 @@ TEST_P(JsonTest, ParseLegacySingleRepeatedField) { })json"); ASSERT_OK(m); - ASSERT_EQ(m->repeated_int32_value_size(), 1); - EXPECT_EQ(m->repeated_int32_value(0), 1997); + EXPECT_THAT(m->repeated_int32_value(), ElementsAre(1997)); + EXPECT_THAT(m->repeated_string_value(), ElementsAre("oh no")); + EXPECT_THAT(m->repeated_enum_value(), ElementsAre(proto3::EnumType::BAR)); - ASSERT_EQ(m->repeated_string_value_size(), 1); - EXPECT_EQ(m->repeated_string_value(0), "oh no"); - - ASSERT_EQ(m->repeated_enum_value_size(), 1); - EXPECT_EQ(m->repeated_enum_value(0), proto3::EnumType::BAR); - - ASSERT_EQ(m->repeated_message_value_size(), 1); + ASSERT_THAT(m->repeated_message_value(), SizeIs(1)); EXPECT_EQ(m->repeated_message_value(0).value(), -1); EXPECT_THAT(ToJson(*m), @@ -499,6 +468,13 @@ TEST_P(JsonTest, ParseMap) { EXPECT_EQ(other->DebugString(), message.DebugString()); } +TEST_P(JsonTest, RepeatedMapKey) { + EXPECT_THAT(ToProto(R"json({ + "twiceKey": 0, + "twiceKey": 1 + })json"), StatusIs(util::StatusCode::kInvalidArgument)); +} + TEST_P(JsonTest, ParsePrimitiveMapIn) { MapIn message; JsonPrintOptions print_options; @@ -535,6 +511,32 @@ TEST_P(JsonTest, ParseOverOneof) { EXPECT_EQ(m.oneof_int32_value(), 5); } +TEST_P(JsonTest, RepeatedSingularKeys) { + auto m = ToProto(R"json({ + "int32Value": 1, + "int32Value": 2 + })json"); + EXPECT_OK(m); + EXPECT_EQ(m->int32_value(), 2); +} + +TEST_P(JsonTest, RepeatedRepeatedKeys) { + auto m = ToProto(R"json({ + "repeatedInt32Value": [1], + "repeatedInt32Value": [2, 3] + })json"); + EXPECT_OK(m); + EXPECT_THAT(m->repeated_int32_value(), ElementsAre(1, 2, 3)); +} + +TEST_P(JsonTest, RepeatedOneofKeys) { + EXPECT_THAT(ToProto(R"json({ + "oneofInt32Value": 1, + "oneofStringValue": "foo" + })json"), + StatusIs(util::StatusCode::kInvalidArgument)); +} + TEST_P(JsonTest, TestParseIgnoreUnknownFields) { JsonParseOptions options; options.ignore_unknown_fields = true; @@ -585,9 +587,8 @@ TEST_P(JsonTest, TestDynamicMessage) { EXPECT_TRUE(generated.ParseFromString(message->SerializeAsString())); EXPECT_EQ(generated.int32_value(), 1024); - ASSERT_EQ(generated.repeated_int32_value_size(), 2); - EXPECT_EQ(generated.repeated_int32_value(0), 1); - EXPECT_EQ(generated.repeated_int32_value(1), 2); + EXPECT_THAT(generated.repeated_int32_value(), ElementsAre(1, 2)); + EXPECT_EQ(generated.message_value().value(), 2048); ASSERT_EQ(generated.repeated_message_value_size(), 2); EXPECT_EQ(generated.repeated_message_value(0).value(), 40); @@ -832,11 +833,11 @@ TEST_P(JsonTest, TestParsingEnumCaseSensitive) { EXPECT_EQ(m.enum_value(), proto3::FOO); } - TEST_P(JsonTest, TestParsingEnumLowercase) { JsonParseOptions options; options.case_insensitive_enum_parsing = true; - auto m = ToProto(R"json({"enum_value": "TLSv1_2"})json", options); + auto m = + ToProto(R"json({"enum_value": "TLSv1_2"})json", options); ASSERT_OK(m); EXPECT_THAT(m->enum_value(), proto3::TLSv1_2); } @@ -858,10 +859,7 @@ TEST_P(JsonTest, TestOverwriteRepeated) { m.add_repeated_int32_value(5); ASSERT_OK(ToProto(m, R"json({"repeated_int32_value": [1, 2, 3]})json")); - EXPECT_EQ(m.repeated_int32_value_size(), 3); - EXPECT_EQ(m.repeated_int32_value(0), 1); - EXPECT_EQ(m.repeated_int32_value(1), 2); - EXPECT_EQ(m.repeated_int32_value(2), 3); + EXPECT_THAT(m.repeated_int32_value(), ElementsAre(1, 2, 3)); } @@ -876,7 +874,8 @@ TEST_P(JsonTest, TestDuration) { EXPECT_EQ(m->value().seconds(), 123456); EXPECT_EQ(m->value().nanos(), 789000000); - EXPECT_EQ(m->repeated_value().size(), 2); + + EXPECT_THAT(m->repeated_value(), SizeIs(2)); EXPECT_EQ(m->repeated_value(0).seconds(), 0); EXPECT_EQ(m->repeated_value(0).nanos(), 100000000); EXPECT_EQ(m->repeated_value(1).seconds(), 999); @@ -911,7 +910,7 @@ TEST_P(JsonTest, TestTimestamp) { EXPECT_EQ(m->value().seconds(), 825422400); EXPECT_EQ(m->value().nanos(), 0); - EXPECT_EQ(m->repeated_value().size(), 1); + EXPECT_THAT(m->repeated_value(), SizeIs(1)); EXPECT_EQ(m->repeated_value(0).seconds(), 253402300799); EXPECT_EQ(m->repeated_value(0).nanos(), 0); @@ -961,10 +960,7 @@ TEST_P(JsonTest, TestFieldMask) { )json"); ASSERT_OK(m); - EXPECT_EQ(m->value().paths_size(), 2); - EXPECT_EQ(m->value().paths(0), "foo"); - EXPECT_EQ(m->value().paths(1), "bar.baz_baz"); - + EXPECT_THAT(m->value().paths(), ElementsAre("foo", "bar.baz_baz")); EXPECT_THAT(ToJson(*m), IsOkAndHolds(R"({"value":"foo,bar.bazBaz"})")); auto m2 = ToProto(R"json( @@ -976,8 +972,7 @@ TEST_P(JsonTest, TestFieldMask) { )json"); ASSERT_OK(m2); - EXPECT_EQ(m2->value().paths_size(), 1); - EXPECT_EQ(m2->value().paths(0), "yep.really"); + EXPECT_THAT(m2->value().paths(), ElementsAre("yep.really")); } TEST_P(JsonTest, TestLegalNullsInArray) { @@ -986,15 +981,15 @@ TEST_P(JsonTest, TestLegalNullsInArray) { })json"); ASSERT_OK(m); - EXPECT_EQ(m->repeated_null_value_size(), 1); - EXPECT_EQ(m->repeated_null_value(0), google::protobuf::NULL_VALUE); + EXPECT_THAT(m->repeated_null_value(), + ElementsAre(google::protobuf::NULL_VALUE)); auto m2 = ToProto(R"json({ "repeatedValue": [null] })json"); ASSERT_OK(m2); - EXPECT_EQ(m2->repeated_value_size(), 1); + ASSERT_THAT(m2->repeated_value(), SizeIs(1)); EXPECT_TRUE(m2->repeated_value(0).has_null_value()); }