From 1325913afd65b39c268e5c4101d6b82f32957ae9 Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Thu, 2 Feb 2023 17:05:12 -0800 Subject: [PATCH] Add Java support for retention attribute PiperOrigin-RevId: 506760540 --- .../com/google/protobuf/DescriptorsTest.java | 12 ++ src/google/protobuf/BUILD.bazel | 19 +-- src/google/protobuf/compiler/java/BUILD.bazel | 1 + .../compiler/java/shared_code_generator.cc | 4 +- src/google/protobuf/retention_test.cc | 110 +++++++++++------- src/google/protobuf/unittest_retention.proto | 3 +- 6 files changed, 87 insertions(+), 62 deletions(-) diff --git a/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java b/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java index ee119535b9f15..60dcac7178d7b 100644 --- a/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java +++ b/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java @@ -38,6 +38,7 @@ import com.google.protobuf.DescriptorProtos.EnumValueDescriptorProto; import com.google.protobuf.DescriptorProtos.FieldDescriptorProto; import com.google.protobuf.DescriptorProtos.FileDescriptorProto; +import com.google.protobuf.DescriptorProtos.FileOptions; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.DescriptorValidationException; import com.google.protobuf.Descriptors.EnumDescriptor; @@ -64,6 +65,7 @@ import protobuf_unittest.UnittestProto.TestReservedEnumFields; import protobuf_unittest.UnittestProto.TestReservedFields; import protobuf_unittest.UnittestProto.TestService; +import protobuf_unittest.UnittestRetention; import java.util.Collections; import java.util.List; import org.junit.Test; @@ -392,6 +394,16 @@ public void testCustomOptions() throws Exception { .isEqualTo(UnittestCustomOptions.MethodOpt1.METHODOPT1_VAL2); } + @Test + public void testOptionRetention() throws Exception { + // Verify that options with RETENTION_SOURCE are stripped from the + // generated descriptors. + FileOptions options = UnittestRetention.getDescriptor().getOptions(); + assertThat(options.hasExtension(UnittestRetention.plainOption)).isTrue(); + assertThat(options.hasExtension(UnittestRetention.runtimeRetentionOption)).isTrue(); + assertThat(options.hasExtension(UnittestRetention.sourceRetentionOption)).isFalse(); + } + /** Test that the FieldDescriptor.Type enum is the same as the WireFormat.FieldType enum. */ @Test public void testFieldTypeTablesMatch() throws Exception { diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index 901bcf91a2bc7..30a3382d87e32 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -607,6 +607,7 @@ proto_library( "unittest_proto3_arena_lite.proto", "unittest_proto3_lite.proto", "unittest_proto3_optional.proto", + "unittest_retention.proto", "unittest_well_known_types.proto", ], strip_import_prefix = "/src", @@ -1017,6 +1018,7 @@ cc_test( cc_test( name = "map_test", + timeout = "long", srcs = [ "map_test.cc", "map_test.inc", @@ -1027,7 +1029,6 @@ cc_test( "-Wno-deprecated-declarations", ], }), - timeout = "long", data = [":testdata"], deps = [ ":cc_test_protos", @@ -1278,26 +1279,12 @@ cc_test( ], ) -proto_library( - name = "unittest_retention_proto", - testonly = True, - srcs = ["unittest_retention.proto"], - strip_import_prefix = "/src", - deps = [":descriptor_proto"], -) - -cc_proto_library( - name = "unittest_retention_cc_proto", - testonly = True, - deps = ["unittest_retention_proto"], -) - cc_test( name = "retention_test", srcs = ["retention_test.cc"], deps = [ + ":cc_test_protos", ":protobuf", - ":unittest_retention_cc_proto", "//src/google/protobuf/compiler:retention", "@com_google_googletest//:gtest_main", ], diff --git a/src/google/protobuf/compiler/java/BUILD.bazel b/src/google/protobuf/compiler/java/BUILD.bazel index b93a423b6cb6f..a34bb8331108f 100644 --- a/src/google/protobuf/compiler/java/BUILD.bazel +++ b/src/google/protobuf/compiler/java/BUILD.bazel @@ -114,6 +114,7 @@ cc_library( ":names_internal", "//src/google/protobuf:protobuf_nowkt", "//src/google/protobuf/compiler:code_generator", + "//src/google/protobuf/compiler:retention", "@com_google_absl//absl/container:btree", "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/strings", diff --git a/src/google/protobuf/compiler/java/shared_code_generator.cc b/src/google/protobuf/compiler/java/shared_code_generator.cc index a203d513a72b4..7c416719ba5f3 100644 --- a/src/google/protobuf/compiler/java/shared_code_generator.cc +++ b/src/google/protobuf/compiler/java/shared_code_generator.cc @@ -39,6 +39,7 @@ #include "google/protobuf/compiler/java/helpers.h" #include "google/protobuf/compiler/java/name_resolver.h" #include "google/protobuf/compiler/java/names.h" +#include "google/protobuf/compiler/retention.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/io/printer.h" @@ -134,8 +135,7 @@ void SharedCodeGenerator::GenerateDescriptors(io::Printer* printer) { // This makes huge bytecode files and can easily hit the compiler's internal // code size limits (error "code to large"). String literals are apparently // embedded raw, which is what we want. - FileDescriptorProto file_proto; - file_->CopyTo(&file_proto); + FileDescriptorProto file_proto = StripSourceRetentionOptions(*file_); std::string file_data; file_proto.SerializeToString(&file_data); diff --git a/src/google/protobuf/retention_test.cc b/src/google/protobuf/retention_test.cc index 86116adaad03a..2b582d8fc39e6 100644 --- a/src/google/protobuf/retention_test.cc +++ b/src/google/protobuf/retention_test.cc @@ -51,16 +51,20 @@ namespace internal { namespace { TEST(RetentionTest, DirectOptions) { - const google::protobuf::FileOptions& file_options = - OptionsMessage::descriptor()->file()->options(); - EXPECT_EQ(file_options.GetExtension(plain_option), 1); - EXPECT_EQ(file_options.GetExtension(runtime_retention_option), 2); + const FileOptions& file_options = + protobuf_unittest::OptionsMessage::descriptor()->file()->options(); + EXPECT_EQ(file_options.GetExtension(protobuf_unittest::plain_option), 1); + EXPECT_EQ( + file_options.GetExtension(protobuf_unittest::runtime_retention_option), 2); // RETENTION_SOURCE option should be stripped. - EXPECT_FALSE(file_options.HasExtension(source_retention_option)); - EXPECT_EQ(file_options.GetExtension(source_retention_option), 0); + EXPECT_FALSE( + file_options.HasExtension(protobuf_unittest::source_retention_option)); + EXPECT_EQ(file_options.GetExtension(protobuf_unittest::source_retention_option), + 0); } -void CheckOptionsMessageIsStrippedCorrectly(const OptionsMessage& options) { +void CheckOptionsMessageIsStrippedCorrectly( + const protobuf_unittest::OptionsMessage& options) { EXPECT_EQ(options.plain_field(), 1); EXPECT_EQ(options.runtime_retention_field(), 2); // RETENTION_SOURCE field should be stripped. @@ -69,93 +73,113 @@ void CheckOptionsMessageIsStrippedCorrectly(const OptionsMessage& options) { } TEST(RetentionTest, FieldsNestedInRepeatedMessage) { - const google::protobuf::FileOptions& file_options = - OptionsMessage::descriptor()->file()->options(); - ASSERT_EQ(1, file_options.ExtensionSize(repeated_options)); - const OptionsMessage& options_message = - file_options.GetRepeatedExtension(repeated_options)[0]; + const FileOptions& file_options = + protobuf_unittest::OptionsMessage::descriptor()->file()->options(); + ASSERT_EQ(1, file_options.ExtensionSize(protobuf_unittest::repeated_options)); + const protobuf_unittest::OptionsMessage& options_message = + file_options.GetRepeatedExtension(protobuf_unittest::repeated_options)[0]; CheckOptionsMessageIsStrippedCorrectly(options_message); } TEST(RetentionTest, File) { CheckOptionsMessageIsStrippedCorrectly( - OptionsMessage::descriptor()->file()->options().GetExtension( - file_option)); + protobuf_unittest::OptionsMessage::descriptor() + ->file() + ->options() + .GetExtension(protobuf_unittest::file_option)); } TEST(RetentionTest, TopLevelMessage) { CheckOptionsMessageIsStrippedCorrectly( - TopLevelMessage::descriptor()->options().GetExtension(message_option)); + protobuf_unittest::TopLevelMessage::descriptor()->options().GetExtension( + protobuf_unittest::message_option)); } TEST(RetentionTest, NestedMessage) { CheckOptionsMessageIsStrippedCorrectly( - TopLevelMessage::NestedMessage::descriptor()->options().GetExtension( - message_option)); + protobuf_unittest::TopLevelMessage::NestedMessage::descriptor() + ->options() + .GetExtension(protobuf_unittest::message_option)); } TEST(RetentionTest, TopLevelEnum) { CheckOptionsMessageIsStrippedCorrectly( - TopLevelEnum_descriptor()->options().GetExtension(enum_option)); + protobuf_unittest::TopLevelEnum_descriptor()->options().GetExtension( + protobuf_unittest::enum_option)); } TEST(RetentionTest, NestedEnum) { CheckOptionsMessageIsStrippedCorrectly( - TopLevelMessage::NestedEnum_descriptor()->options().GetExtension( - enum_option)); + protobuf_unittest::TopLevelMessage::NestedEnum_descriptor() + ->options() + .GetExtension(protobuf_unittest::enum_option)); } TEST(RetentionTest, EnumEntry) { CheckOptionsMessageIsStrippedCorrectly( - TopLevelEnum_descriptor()->value(0)->options().GetExtension( - enum_entry_option)); + protobuf_unittest::TopLevelEnum_descriptor() + ->value(0) + ->options() + .GetExtension(protobuf_unittest::enum_entry_option)); } TEST(RetentionTest, TopLevelExtension) { - CheckOptionsMessageIsStrippedCorrectly(TopLevelMessage::descriptor() - ->file() - ->FindExtensionByName("i") - ->options() - .GetExtension(field_option)); + CheckOptionsMessageIsStrippedCorrectly( + protobuf_unittest::TopLevelMessage::descriptor() + ->file() + ->FindExtensionByName("i") + ->options() + .GetExtension(protobuf_unittest::field_option)); } TEST(RetentionTest, NestedExtension) { CheckOptionsMessageIsStrippedCorrectly( - TopLevelMessage::descriptor()->extension(0)->options().GetExtension( - field_option)); + protobuf_unittest::TopLevelMessage::descriptor() + ->extension(0) + ->options() + .GetExtension(protobuf_unittest::field_option)); } TEST(RetentionTest, Field) { CheckOptionsMessageIsStrippedCorrectly( - TopLevelMessage::descriptor()->field(0)->options().GetExtension( - field_option)); + protobuf_unittest::TopLevelMessage::descriptor() + ->field(0) + ->options() + .GetExtension(protobuf_unittest::field_option)); } TEST(RetentionTest, Oneof) { CheckOptionsMessageIsStrippedCorrectly( - TopLevelMessage::descriptor()->oneof_decl(0)->options().GetExtension( - oneof_option)); + protobuf_unittest::TopLevelMessage::descriptor() + ->oneof_decl(0) + ->options() + .GetExtension(protobuf_unittest::oneof_option)); } TEST(RetentionTest, ExtensionRange) { CheckOptionsMessageIsStrippedCorrectly( - TopLevelMessage::descriptor()->extension_range(0)->options_->GetExtension( - extension_range_option)); + protobuf_unittest::TopLevelMessage::descriptor() + ->extension_range(0) + ->options_->GetExtension(protobuf_unittest::extension_range_option)); } TEST(RetentionTest, Service) { CheckOptionsMessageIsStrippedCorrectly( - TopLevelMessage::descriptor()->file()->service(0)->options().GetExtension( - service_option)); + protobuf_unittest::TopLevelMessage::descriptor() + ->file() + ->service(0) + ->options() + .GetExtension(protobuf_unittest::service_option)); } TEST(RetentionTest, Method) { - CheckOptionsMessageIsStrippedCorrectly(TopLevelMessage::descriptor() - ->file() - ->service(0) - ->method(0) - ->options() - .GetExtension(method_option)); + CheckOptionsMessageIsStrippedCorrectly( + protobuf_unittest::TopLevelMessage::descriptor() + ->file() + ->service(0) + ->method(0) + ->options() + .GetExtension(protobuf_unittest::method_option)); } TEST(RetentionTest, StripSourceRetentionOptions) { diff --git a/src/google/protobuf/unittest_retention.proto b/src/google/protobuf/unittest_retention.proto index 232fe5ba064c5..e157bc452ea01 100644 --- a/src/google/protobuf/unittest_retention.proto +++ b/src/google/protobuf/unittest_retention.proto @@ -30,10 +30,11 @@ syntax = "proto2"; -package google.protobuf.internal; +package protobuf_unittest; import "google/protobuf/descriptor.proto"; + // Retention attributes set directly on custom options extend google.protobuf.FileOptions { optional int32 plain_option = 505092806;