diff --git a/.github/workflows/test_ruby.yml b/.github/workflows/test_ruby.yml index 39dfaa9de72c..46f6d12a9bf1 100644 --- a/.github/workflows/test_ruby.yml +++ b/.github/workflows/test_ruby.yml @@ -45,6 +45,39 @@ jobs: bazel-cache: ruby_linux/${{ matrix.ruby }}_${{ matrix.bazel }} bazel: test //ruby/... //ruby/tests:ruby_version --test_env=KOKORO_RUBY_VERSION --test_env=BAZEL=true ${{ matrix.ffi == 'FFI' && '--//ruby:ffi=enabled --test_env=PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION=FFI' || '' }} + linux-32bit: + name: Linux 32-bit + runs-on: ubuntu-latest + steps: + - name: Checkout pending changes + uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0 + with: + ref: ${{ inputs.safe-checkout }} + submodules: recursive + + - name: Cross compile protoc for i386 + id: cross-compile + uses: protocolbuffers/protobuf-ci/cross-compile-protoc@v2 + with: + image: us-docker.pkg.dev/protobuf-build/containers/common/linux/bazel:6.3.0-91a0ac83e968068672bc6001a4d474cfd9a50f1d + credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} + architecture: linux-i386 + + - name: Run tests + uses: protocolbuffers/protobuf-ci/docker@v2 + with: + image: i386/ruby:2.7.3-buster + skip-staleness-check: true + credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} + command: >- + /bin/bash -cex ' + gem install bundler; + cd /workspace/ruby; + bundle; + PROTOC=/workspace/${{ steps.cross-compile.outputs.protoc }} rake; + rake clobber_package gem; + PROTOC=/workspace/${{ steps.cross-compile.outputs.protoc }} rake test' + linux-aarch64: name: Linux aarch64 runs-on: ubuntu-latest diff --git a/ruby/ext/google/protobuf_c/protobuf.c b/ruby/ext/google/protobuf_c/protobuf.c index c9354db4e340..39062cfca65a 100644 --- a/ruby/ext/google/protobuf_c/protobuf.c +++ b/ruby/ext/google/protobuf_c/protobuf.c @@ -276,17 +276,25 @@ static void ObjectCache_Init(VALUE protobuf) { rb_const_set(protobuf, rb_intern("SIZEOF_VALUE"), INT2NUM(SIZEOF_VALUE)); } -VALUE ObjectCache_TryAdd(const void *key, VALUE val) { +static VALUE ObjectCache_GetKey(const void *key) { VALUE key_val = (VALUE)key; PBRUBY_ASSERT((key_val & 3) == 0); - return rb_funcall(weak_obj_cache, item_try_add, 2, LL2NUM(key_val), val); + // Ensure the key can be stored as a Fixnum since 1 bit is needed for + // FIXNUM_FLAG and 1 bit is needed for the sign bit. + VALUE new_key = LL2NUM(key_val >> 2); + PBRUBY_ASSERT(FIXNUM_P(new_key)); + return new_key; +} + +VALUE ObjectCache_TryAdd(const void *key, VALUE val) { + VALUE key_val = ObjectCache_GetKey(key); + return rb_funcall(weak_obj_cache, item_try_add, 2, key_val, val); } // Returns the cached object for this key, if any. Otherwise returns Qnil. VALUE ObjectCache_Get(const void *key) { - VALUE key_val = (VALUE)key; - PBRUBY_ASSERT((key_val & 3) == 0); - return rb_funcall(weak_obj_cache, item_get, 1, LL2NUM(key_val)); + VALUE key_val = ObjectCache_GetKey(key); + return rb_funcall(weak_obj_cache, item_get, 1, key_val); } /* diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index ae6012349aa8..9284b315eb5c 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -685,7 +685,6 @@ filegroup( "unittest_features.proto", "unittest_import.proto", "unittest_import_public.proto", - "unittest_invalid_features.proto", "unittest_lite_imports_nonlite.proto", "unittest_mset.proto", "unittest_mset_wire_format.proto", diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index 66e81f05237f..af99450f5069 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -1564,6 +1564,10 @@ TEST_F(CommandLineInterfaceTest, Plugin_RuntimeFeatures) { message_encoding: LENGTH_PREFIXED json_format: ALLOW raw_features { field_presence: IMPLICIT } + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + } )pb")); EXPECT_THAT(request.source_file_descriptors(0) .message_type(0) @@ -1577,6 +1581,10 @@ TEST_F(CommandLineInterfaceTest, Plugin_RuntimeFeatures) { message_encoding: LENGTH_PREFIXED json_format: ALLOW raw_features { field_presence: IMPLICIT } + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + } )pb")); } diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index f1430a8d659a..51e483cecd2a 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -69,7 +69,6 @@ #include "google/protobuf/unittest.pb.h" #include "google/protobuf/unittest_custom_options.pb.h" #include "google/protobuf/unittest_features.pb.h" -#include "google/protobuf/unittest_invalid_features.pb.h" #include "google/protobuf/unittest_lazy_dependencies.pb.h" #include "google/protobuf/unittest_lazy_dependencies_custom_option.pb.h" #include "google/protobuf/unittest_lazy_dependencies_enum.pb.h" @@ -7179,6 +7178,16 @@ const FeatureSet& GetFeatures(const T* descriptor) { return internal::InternalFeatureHelper::GetFeatures(*descriptor); } +template +FeatureSet GetCoreFeatures(const T* descriptor) { + FeatureSet features = GetFeatures(descriptor); + // Strip test features to avoid excessive brittleness. + features.ClearExtension(pb::test); + features.ClearExtension(pb::TestMessage::test_message); + features.ClearExtension(pb::TestMessage::Nested::test_nested); + return features; +} + TEST_F(FeaturesTest, InvalidProto2Features) { BuildDescriptorMessagesInTestPool(); BuildFileWithErrors( @@ -7440,6 +7449,32 @@ TEST_F(FeaturesTest, Proto3Extensions) { R"pb([bar_ext] { baz: 1 })pb")); } +TEST_F(FeaturesTest, Edition2023Defaults) { + FileDescriptorProto file_proto = + ParseTextOrDie(R"pb( + name: "foo.proto" syntax: "editions" edition: "2023" + )pb"); + + BuildDescriptorMessagesInTestPool(); + const FileDescriptor* file = ABSL_DIE_IF_NULL(pool_.BuildFile(file_proto)); + EXPECT_THAT(file->options(), EqualsProto("")); + EXPECT_THAT( + GetCoreFeatures(file), EqualsProto(R"pb( + field_presence: EXPLICIT + enum_type: OPEN + repeated_field_encoding: PACKED + string_field_validation: MANDATORY + message_encoding: LENGTH_PREFIXED + json_format: ALLOW + [pb.cpp] { legacy_closed_enum: false utf8_validation: VERIFY_PARSE } + )pb")); + + // Since pb::test is linked in, it should end up with defaults in our + // FeatureSet. + EXPECT_TRUE(GetFeatures(file).HasExtension(pb::test)); + EXPECT_EQ(GetFeatures(file).GetExtension(pb::test).int_file_feature(), 1); +} + TEST_F(FeaturesTest, ClearsOptions) { BuildDescriptorMessagesInTestPool(); const FileDescriptor* file = BuildFile(R"pb( @@ -7452,13 +7487,17 @@ TEST_F(FeaturesTest, ClearsOptions) { } )pb"); EXPECT_THAT(file->options(), EqualsProto("java_package: 'bar'")); - EXPECT_THAT(GetFeatures(file), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(file), EqualsProto(R"pb( field_presence: IMPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, RestoresOptionsRoundTrip) { @@ -7711,13 +7750,17 @@ TEST_F(FeaturesTest, NoOptions) { name: "foo.proto" syntax: "editions" edition: "2023" )pb"); EXPECT_EQ(&file->options(), &FileOptions::default_instance()); - EXPECT_THAT(GetFeatures(file), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(file), EqualsProto(R"pb( field_presence: EXPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, InvalidEdition) { @@ -7740,13 +7783,17 @@ TEST_F(FeaturesTest, FileFeatures) { options { features { field_presence: IMPLICIT } } )pb"); EXPECT_THAT(file->options(), EqualsProto("")); - EXPECT_THAT(GetFeatures(file), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(file), EqualsProto(R"pb( field_presence: IMPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, FileFeaturesExtension) { @@ -7761,7 +7808,7 @@ TEST_F(FeaturesTest, FileFeaturesExtension) { )pb"); EXPECT_THAT(file->options(), EqualsProto("")); EXPECT_EQ(GetFeatures(file).field_presence(), FeatureSet::IMPLICIT); - EXPECT_EQ(GetFeatures(file).string_field_validation(), FeatureSet::MANDATORY); + EXPECT_EQ(GetFeatures(file).enum_type(), FeatureSet::OPEN); EXPECT_EQ(GetFeatures(file).GetExtension(pb::test).int_file_feature(), 4); EXPECT_EQ(GetFeatures(file) .GetExtension(pb::TestMessage::test_message) @@ -7792,7 +7839,7 @@ TEST_F(FeaturesTest, FileFeaturesExtensionOverride) { )pb"); EXPECT_THAT(file->options(), EqualsProto("")); EXPECT_EQ(GetFeatures(file).field_presence(), FeatureSet::IMPLICIT); - EXPECT_EQ(GetFeatures(file).string_field_validation(), FeatureSet::MANDATORY); + EXPECT_EQ(GetFeatures(file).enum_type(), FeatureSet::OPEN); EXPECT_EQ(GetFeatures(file).GetExtension(pb::test).int_file_feature(), 9); EXPECT_EQ(GetFeatures(file) .GetExtension(pb::TestMessage::test_message) @@ -7814,13 +7861,17 @@ TEST_F(FeaturesTest, MessageFeaturesDefault) { )pb"); const Descriptor* message = file->message_type(0); EXPECT_THAT(message->options(), EqualsProto("")); - EXPECT_THAT(GetFeatures(message), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(message), EqualsProto(R"pb( field_presence: EXPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, MessageFeaturesInherit) { @@ -7918,13 +7969,17 @@ TEST_F(FeaturesTest, FieldFeaturesDefault) { )pb"); const FieldDescriptor* field = file->message_type(0)->field(0); EXPECT_THAT(field->options(), EqualsProto("")); - EXPECT_THAT(GetFeatures(field), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(field), EqualsProto(R"pb( field_presence: EXPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, FieldFeaturesInherit) { @@ -7968,7 +8023,7 @@ TEST_F(FeaturesTest, FieldFeaturesOverride) { dependency: "google/protobuf/unittest_features.proto" options { features { - string_field_validation: HINT + enum_type: CLOSED field_presence: IMPLICIT [pb.test] { int_multiple_feature: 2 } } @@ -7987,7 +8042,7 @@ TEST_F(FeaturesTest, FieldFeaturesOverride) { type: TYPE_STRING options { features { - string_field_validation: NONE + field_presence: EXPLICIT [pb.test] { int_multiple_feature: 9 } } } @@ -7996,8 +8051,8 @@ TEST_F(FeaturesTest, FieldFeaturesOverride) { )pb"); const FieldDescriptor* field = file->message_type(0)->field(0); EXPECT_THAT(field->options(), EqualsProto("")); - EXPECT_EQ(GetFeatures(field).field_presence(), FeatureSet::IMPLICIT); - EXPECT_EQ(GetFeatures(field).string_field_validation(), FeatureSet::NONE); + EXPECT_EQ(GetFeatures(field).field_presence(), FeatureSet::EXPLICIT); + EXPECT_EQ(GetFeatures(field).enum_type(), FeatureSet::CLOSED); EXPECT_EQ(GetFeatures(field).GetExtension(pb::test).int_multiple_feature(), 9); } @@ -8074,7 +8129,6 @@ TEST_F(FeaturesTest, OneofFieldFeaturesOverride) { type: TYPE_STRING options { features { - string_field_validation: NONE [pb.test] { int_multiple_feature: 9 } } } @@ -8213,7 +8267,7 @@ TEST_F(FeaturesTest, RootExtensionFeaturesOverride) { dependency: "google/protobuf/unittest_features.proto" options { features { - string_field_validation: HINT + enum_type: CLOSED field_presence: IMPLICIT [pb.test] { int_multiple_feature: 2 } } @@ -8225,7 +8279,7 @@ TEST_F(FeaturesTest, RootExtensionFeaturesOverride) { type: TYPE_STRING options { features { - string_field_validation: NONE + enum_type: OPEN [pb.test] { int_multiple_feature: 9 } } } @@ -8239,7 +8293,7 @@ TEST_F(FeaturesTest, RootExtensionFeaturesOverride) { const FieldDescriptor* field = file->extension(0); EXPECT_THAT(field->options(), EqualsProto("")); EXPECT_EQ(GetFeatures(field).field_presence(), FeatureSet::IMPLICIT); - EXPECT_EQ(GetFeatures(field).string_field_validation(), FeatureSet::NONE); + EXPECT_EQ(GetFeatures(field).enum_type(), FeatureSet::OPEN); EXPECT_EQ(GetFeatures(field).GetExtension(pb::test).int_multiple_feature(), 9); } @@ -8254,7 +8308,7 @@ TEST_F(FeaturesTest, MessageExtensionFeaturesOverride) { dependency: "google/protobuf/unittest_features.proto" options { features { - string_field_validation: HINT + enum_type: CLOSED field_presence: IMPLICIT [pb.test] { int_multiple_feature: 2 } } @@ -8271,7 +8325,7 @@ TEST_F(FeaturesTest, MessageExtensionFeaturesOverride) { number: 1 label: LABEL_OPTIONAL type: TYPE_STRING - options { features { string_field_validation: NONE } } + options { features { enum_type: OPEN } } extendee: "Foo2" } } @@ -8288,7 +8342,7 @@ TEST_F(FeaturesTest, MessageExtensionFeaturesOverride) { const FieldDescriptor* field = file->message_type(0)->extension(0); EXPECT_THAT(field->options(), EqualsProto("")); EXPECT_EQ(GetFeatures(field).field_presence(), FeatureSet::IMPLICIT); - EXPECT_EQ(GetFeatures(field).string_field_validation(), FeatureSet::NONE); + EXPECT_EQ(GetFeatures(field).enum_type(), FeatureSet::OPEN); EXPECT_EQ(GetFeatures(field).GetExtension(pb::test).int_multiple_feature(), 3); } @@ -8306,13 +8360,17 @@ TEST_F(FeaturesTest, EnumFeaturesDefault) { )pb"); const EnumDescriptor* enm = file->enum_type(0); EXPECT_THAT(enm->options(), EqualsProto("")); - EXPECT_THAT(GetFeatures(enm), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(enm), EqualsProto(R"pb( field_presence: EXPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, EnumFeaturesInherit) { @@ -8412,13 +8470,17 @@ TEST_F(FeaturesTest, EnumValueFeaturesDefault) { )pb"); const EnumValueDescriptor* value = file->enum_type(0)->value(0); EXPECT_THAT(value->options(), EqualsProto("")); - EXPECT_THAT(GetFeatures(value), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(value), EqualsProto(R"pb( field_presence: EXPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, EnumValueFeaturesInherit) { @@ -8503,13 +8565,17 @@ TEST_F(FeaturesTest, OneofFeaturesDefault) { )pb"); const OneofDescriptor* oneof = file->message_type(0)->oneof_decl(0); EXPECT_THAT(oneof->options(), EqualsProto("")); - EXPECT_THAT(GetFeatures(oneof), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(oneof), EqualsProto(R"pb( field_presence: EXPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, OneofFeaturesInherit) { @@ -8601,13 +8667,17 @@ TEST_F(FeaturesTest, ExtensionRangeFeaturesDefault) { const Descriptor::ExtensionRange* range = file->message_type(0)->extension_range(0); EXPECT_THAT(range->options(), EqualsProto("")); - EXPECT_THAT(GetFeatures(range), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(range), EqualsProto(R"pb( field_presence: EXPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, ExtensionRangeFeaturesInherit) { @@ -8684,13 +8754,17 @@ TEST_F(FeaturesTest, ServiceFeaturesDefault) { )pb"); const ServiceDescriptor* service = file->service(0); EXPECT_THAT(service->options(), EqualsProto("")); - EXPECT_THAT(GetFeatures(service), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(service), EqualsProto(R"pb( field_presence: EXPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, ServiceFeaturesInherit) { @@ -8749,13 +8823,17 @@ TEST_F(FeaturesTest, MethodFeaturesDefault) { )pb"); const MethodDescriptor* method = file->service(0)->method(0); EXPECT_THAT(method->options(), EqualsProto("")); - EXPECT_THAT(GetFeatures(method), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(method), EqualsProto(R"pb( field_presence: EXPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, MethodFeaturesInherit) { @@ -9006,23 +9084,44 @@ TEST_F(FeaturesTest, FeaturesOutsideEditions) { TEST_F(FeaturesTest, InvalidExtensionNonMessage) { BuildDescriptorMessagesInTestPool(); - BuildFileInTestPool(::pb::TestInvalid::descriptor()->file()); + ASSERT_NE(BuildFile(R"pb( + name: "unittest_invalid_features.proto" + syntax: "proto2" + package: "pb" + dependency: "google/protobuf/descriptor.proto" + message_type { + name: "TestInvalid" + extension { + name: "scalar_extension" + number: 9996 + label: LABEL_OPTIONAL + type: TYPE_STRING + extendee: ".google.protobuf.FeatureSet" + } + } + )pb"), + nullptr); BuildFileWithErrors( R"pb( name: "foo.proto" syntax: "editions" edition: "2023" - dependency: "google/protobuf/unittest_invalid_features.proto" + dependency: "unittest_invalid_features.proto" options { - features { - [pb.TestInvalid.scalar_extension]: "hello" + uninterpreted_option { + name { name_part: "features" is_extension: false } + name { + name_part: "pb.TestInvalid.scalar_extension" + is_extension: true + } + identifier_value: "hello" } } )pb", - "foo.proto: google/protobuf/unittest_invalid_features.proto: " - "EDITIONS: FeatureSet extension pb.TestInvalid.scalar_extension is not " - "of message type. Feature extensions should always use messages to " - "allow for evolution.\n"); + "foo.proto: unittest_invalid_features.proto: EDITIONS: FeatureSet " + "extension pb.TestInvalid.scalar_extension is not of message type. " + "Feature extensions should always use messages to allow for " + "evolution.\n"); } TEST_F(FeaturesTest, InvalidFieldImplicitDefault) { @@ -9416,13 +9515,17 @@ TEST_F(FeaturesTest, UninterpretedOptions) { } )pb"); EXPECT_THAT(file->options(), EqualsProto("")); - EXPECT_THAT(GetFeatures(file), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(file), EqualsProto(R"pb( field_presence: IMPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, UninterpretedOptionsMerge) { @@ -9434,8 +9537,8 @@ TEST_F(FeaturesTest, UninterpretedOptionsMerge) { options { uninterpreted_option { name { name_part: "features" is_extension: false } - name { name_part: "string_field_validation" is_extension: false } - identifier_value: "HINT" + name { name_part: "enum_type" is_extension: false } + identifier_value: "CLOSED" } } message_type { @@ -9448,8 +9551,8 @@ TEST_F(FeaturesTest, UninterpretedOptionsMerge) { options { uninterpreted_option { name { name_part: "features" is_extension: false } - name { name_part: "string_field_validation" is_extension: false } - identifier_value: "NONE" + name { name_part: "enum_type" is_extension: false } + identifier_value: "OPEN" } } } @@ -9458,8 +9561,8 @@ TEST_F(FeaturesTest, UninterpretedOptionsMerge) { const FieldDescriptor* field = file->message_type(0)->field(0); EXPECT_THAT(file->options(), EqualsProto("")); EXPECT_THAT(field->options(), EqualsProto("")); - EXPECT_EQ(GetFeatures(file).string_field_validation(), FeatureSet::HINT); - EXPECT_EQ(GetFeatures(field).string_field_validation(), FeatureSet::NONE); + EXPECT_EQ(GetFeatures(file).enum_type(), FeatureSet::CLOSED); + EXPECT_EQ(GetFeatures(field).enum_type(), FeatureSet::OPEN); } TEST_F(FeaturesTest, UninterpretedOptionsMergeExtension) { @@ -9540,13 +9643,17 @@ TEST_F(FeaturesTest, RawFeatures) { options { features { raw_features { field_presence: IMPLICIT } } } )pb"); EXPECT_THAT(file->options(), EqualsProto("")); - EXPECT_THAT(GetFeatures(file), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(file), EqualsProto(R"pb( field_presence: IMPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, RawFeaturesConflict) { @@ -9563,13 +9670,17 @@ TEST_F(FeaturesTest, RawFeaturesConflict) { } )pb"); EXPECT_THAT(file->options(), EqualsProto("")); - EXPECT_THAT(GetFeatures(file), EqualsProto(R"pb( + EXPECT_THAT(GetCoreFeatures(file), EqualsProto(R"pb( field_presence: IMPLICIT enum_type: OPEN repeated_field_encoding: PACKED string_field_validation: MANDATORY message_encoding: LENGTH_PREFIXED - json_format: ALLOW)pb")); + json_format: ALLOW + [pb.cpp] { + legacy_closed_enum: false + utf8_validation: VERIFY_PARSE + })pb")); } TEST_F(FeaturesTest, InvalidJsonUniquenessDefaultWarning) { diff --git a/src/google/protobuf/editions/BUILD b/src/google/protobuf/editions/BUILD index 9cf1f2530729..8b31ffda8197 100644 --- a/src/google/protobuf/editions/BUILD +++ b/src/google/protobuf/editions/BUILD @@ -7,6 +7,11 @@ proto_library( deps = ["//src/google/protobuf:cpp_features_proto"], ) +cc_proto_library( + name = "test_messages_proto2_cc_proto", + deps = [":test_messages_proto2_proto"], +) + proto_library( name = "test_messages_proto3_proto", srcs = ["golden/test_messages_proto3.proto"], @@ -23,19 +28,26 @@ proto_library( ) cc_proto_library( - name = "test_messages_proto2_cc_proto", - deps = [":test_messages_proto2_proto"], + name = "test_messages_proto3_cc_proto", + deps = [":test_messages_proto3_proto"], +) + +proto_library( + name = "editions_default_features_proto", + srcs = ["proto/editions_default_features.proto"], + strip_import_prefix = "/src", ) cc_proto_library( - name = "test_messages_proto3_cc_proto", - deps = [":test_messages_proto3_proto"], + name = "editions_default_features_cc_proto", + deps = [":editions_default_features_proto"], ) cc_test( name = "generated_files_test", srcs = ["generated_files_test.cc"], deps = [ + ":editions_default_features_cc_proto", ":test_messages_proto2_cc_proto", ":test_messages_proto3_cc_proto", "//src/google/protobuf", diff --git a/src/google/protobuf/editions/generated_files_test.cc b/src/google/protobuf/editions/generated_files_test.cc index c772230e2787..0f9ab590612e 100644 --- a/src/google/protobuf/editions/generated_files_test.cc +++ b/src/google/protobuf/editions/generated_files_test.cc @@ -33,6 +33,7 @@ #include "google/protobuf/descriptor.h" #include "google/protobuf/editions/golden/test_messages_proto2.pb.h" #include "google/protobuf/editions/golden/test_messages_proto3.pb.h" +#include "google/protobuf/editions/proto/editions_default_features.pb.h" #include "google/protobuf/test_textproto.h" // These tests provide some basic minimal coverage that protos work as expected. @@ -42,12 +43,13 @@ namespace google { namespace protobuf { namespace { +using ::protobuf_editions_test::EditionsDefaultMessage; using ::protobuf_test_messages::proto2::TestAllRequiredTypesProto2; using ::protobuf_test_messages::proto2::TestAllTypesProto2; using ::protobuf_test_messages::proto3::TestAllTypesProto3; using ::testing::NotNull; -TEST(Gemerated, Parsing) { +TEST(Generated, Parsing) { TestAllTypesProto2 message = ParseTextOrDie(R"pb( Data { group_int32: 9 } )pb"); @@ -55,21 +57,21 @@ TEST(Gemerated, Parsing) { EXPECT_EQ(message.data().group_int32(), 9); } -TEST(Gemerated, GeneratedMethods) { +TEST(Generated, GeneratedMethods) { TestAllTypesProto3 message; message.set_optional_int32(9); EXPECT_THAT(message, EqualsProto(R"pb(optional_int32: 9)pb")); } -TEST(Gemerated, ExplicitPresence) { +TEST(Generated, ExplicitPresence) { const FieldDescriptor* field = TestAllTypesProto2::descriptor()->FindFieldByName("default_int32"); ASSERT_THAT(field, NotNull()); EXPECT_TRUE(field->has_presence()); } -TEST(Gemerated, RequiredPresence) { +TEST(Generated, RequiredPresence) { const FieldDescriptor* field = TestAllRequiredTypesProto2::descriptor()->FindFieldByName( "required_int32"); @@ -79,63 +81,63 @@ TEST(Gemerated, RequiredPresence) { EXPECT_EQ(field->label(), FieldDescriptor::LABEL_REQUIRED); } -TEST(Gemerated, ImplicitPresence) { +TEST(Generated, ImplicitPresence) { const FieldDescriptor* field = TestAllTypesProto3::descriptor()->FindFieldByName("optional_int32"); ASSERT_THAT(field, NotNull()); EXPECT_FALSE(field->has_presence()); } -TEST(Gemerated, ClosedEnum) { +TEST(Generated, ClosedEnum) { const EnumDescriptor* enm = TestAllTypesProto2::descriptor()->FindEnumTypeByName("NestedEnum"); ASSERT_THAT(enm, NotNull()); EXPECT_TRUE(enm->is_closed()); } -TEST(Gemerated, OpenEnum) { +TEST(Generated, OpenEnum) { const EnumDescriptor* enm = TestAllTypesProto3::descriptor()->FindEnumTypeByName("NestedEnum"); ASSERT_THAT(enm, NotNull()); EXPECT_FALSE(enm->is_closed()); } -TEST(Gemerated, PackedRepeated) { +TEST(Generated, PackedRepeated) { const FieldDescriptor* field = TestAllTypesProto2::descriptor()->FindFieldByName("packed_int32"); ASSERT_THAT(field, NotNull()); EXPECT_TRUE(field->is_packed()); } -TEST(Gemerated, ExpandedRepeated) { +TEST(Generated, ExpandedRepeated) { const FieldDescriptor* field = TestAllTypesProto2::descriptor()->FindFieldByName("repeated_int32"); ASSERT_THAT(field, NotNull()); EXPECT_FALSE(field->is_packed()); } -TEST(Gemerated, DoesNotEnforceUtf8) { +TEST(Generated, DoesNotEnforceUtf8) { const FieldDescriptor* field = TestAllTypesProto2::descriptor()->FindFieldByName("optional_string"); ASSERT_THAT(field, NotNull()); EXPECT_FALSE(field->requires_utf8_validation()); } -TEST(Gemerated, EnforceUtf8) { +TEST(Generated, EnforceUtf8) { const FieldDescriptor* field = TestAllTypesProto3::descriptor()->FindFieldByName("optional_string"); ASSERT_THAT(field, NotNull()); EXPECT_TRUE(field->requires_utf8_validation()); } -TEST(Gemerated, DelimitedEncoding) { +TEST(Generated, DelimitedEncoding) { const FieldDescriptor* field = TestAllTypesProto2::descriptor()->FindFieldByName("data"); ASSERT_THAT(field, NotNull()); EXPECT_EQ(field->type(), FieldDescriptor::TYPE_GROUP); } -TEST(Gemerated, LengthPrefixedEncoding) { +TEST(Generated, LengthPrefixedEncoding) { const FieldDescriptor* field = TestAllTypesProto2::descriptor()->FindFieldByName( "optional_nested_message"); @@ -143,6 +145,34 @@ TEST(Gemerated, LengthPrefixedEncoding) { EXPECT_EQ(field->type(), FieldDescriptor::TYPE_MESSAGE); } +TEST(Generated, EditionDefaults2023) { + const Descriptor* desc = EditionsDefaultMessage::descriptor(); + EXPECT_TRUE(desc->FindFieldByName("int32_field")->has_presence()); + EXPECT_TRUE( + desc->FindFieldByName("string_field")->requires_utf8_validation()); + EXPECT_FALSE(desc->FindFieldByName("enum_field") + ->legacy_enum_field_treated_as_closed()); + EXPECT_FALSE(desc->FindFieldByName("enum_field")->enum_type()->is_closed()); + EXPECT_TRUE(desc->FindFieldByName("repeated_int32_field")->is_packed()); + EXPECT_EQ(desc->FindFieldByName("sub_message_field")->type(), + FieldDescriptor::TYPE_MESSAGE); +} + +TEST(Generated, EditionDefaults2023InternalFeatures) { + EXPECT_THAT( + internal::InternalFeatureHelper::GetFeatures( + *EditionsDefaultMessage::descriptor()), + google::protobuf::EqualsProto(R"pb( + field_presence: EXPLICIT + enum_type: OPEN + repeated_field_encoding: PACKED + string_field_validation: MANDATORY + message_encoding: LENGTH_PREFIXED + json_format: ALLOW + [pb.cpp] { legacy_closed_enum: false utf8_validation: VERIFY_PARSE } + )pb")); +} + } // namespace } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/unittest_invalid_features.proto b/src/google/protobuf/editions/proto/editions_default_features.proto similarity index 77% rename from src/google/protobuf/unittest_invalid_features.proto rename to src/google/protobuf/editions/proto/editions_default_features.proto index d19c8c438841..a5755f09ed74 100644 --- a/src/google/protobuf/unittest_invalid_features.proto +++ b/src/google/protobuf/editions/proto/editions_default_features.proto @@ -28,14 +28,26 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -syntax = "proto2"; +edition = "2023"; -package pb; +package protobuf_editions_test; -import "google/protobuf/descriptor.proto"; +// This file tests the default features in the absence of any dependencies. -message TestInvalid { - extend .google.protobuf.FeatureSet { - optional string scalar_extension = 9996; +enum EditionsDefaultEnum { + EDITIONS_DEFAULT_ENUM_UNKNOWN = 0; + EDITIONS_DEFAULT_ENUM_VALUE1 = 1; +} + +message EditionsDefaultMessage { + int32 int32_field = 1; + string string_field = 2; + EditionsDefaultEnum enum_field = 3; + + repeated int32 repeated_int32_field = 4; + + message SubMessage { + int32 nested_int32_field = 1; } + SubMessage sub_message_field = 6; } diff --git a/src/google/protobuf/feature_resolver.cc b/src/google/protobuf/feature_resolver.cc index feda73ed4447..1387d7e9503e 100644 --- a/src/google/protobuf/feature_resolver.cc +++ b/src/google/protobuf/feature_resolver.cc @@ -44,6 +44,8 @@ #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_split.h" +#include "absl/strings/string_view.h" +#include "google/protobuf/cpp_features.pb.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/dynamic_message.h" @@ -118,6 +120,29 @@ absl::Status ValidateDescriptor(absl::string_view edition, return absl::OkStatus(); } +absl::Status ValidateExtension(const FieldDescriptor& extension) { + if (extension.message_type() == nullptr) { + return Error("FeatureSet extension ", extension.full_name(), + " is not of message type. Feature extensions should " + "always use messages to allow for evolution."); + } + + if (extension.is_repeated()) { + return Error( + "Only singular features extensions are supported. Found " + "repeated extension ", + extension.full_name()); + } + + if (extension.message_type()->extension_count() > 0 || + extension.message_type()->extension_range_count() > 0) { + return Error("Nested extensions in feature extension ", + extension.full_name(), " are not supported."); + } + + return absl::OkStatus(); +} + absl::Status FillDefaults(absl::string_view edition, Message& msg) { const Descriptor& descriptor = *msg.GetDescriptor(); @@ -189,10 +214,41 @@ absl::Status ValidateMergedFeatures(const Message& msg) { return absl::OkStatus(); } +// Forces calculation of the defaults for any features from the generated pool +// that may be missed if the proto file doesn't import them, giving the final +// resolved FeatureSet object maximal coverage. +absl::StatusOr FillGeneratedDefaults(absl::string_view edition, + const DescriptorPool* pool) { + const Descriptor* desc = + pool->FindMessageTypeByName(FeatureSet::descriptor()->full_name()); + // If there's no FeatureSet, there's no extensions. + if (desc == nullptr) return FeatureSet(); + + DynamicMessageFactory message_factory; + auto defaults = absl::WrapUnique(message_factory.GetPrototype(desc)->New()); + std::vector extensions; + pool->FindAllExtensions(desc, &extensions); + for (const auto* extension : extensions) { + RETURN_IF_ERROR(ValidateExtension(*extension)); + Message* msg = + defaults->GetReflection()->MutableMessage(defaults.get(), extension); + ABSL_CHECK(msg != nullptr); + RETURN_IF_ERROR(FillDefaults(edition, *msg)); + } + + // Our defaults are reflectively built in a custom pool, while the + // returned object here is in the generated pool. We parse/serialize to + // convert from the potentially skewed FeatureSet descriptors. + FeatureSet features; + ABSL_CHECK(features.MergeFromString(defaults->SerializeAsString())); + return features; +} + } // namespace absl::StatusOr FeatureResolver::Create( - absl::string_view edition, const Descriptor* descriptor) { + absl::string_view edition, const Descriptor* descriptor, + const DescriptorPool* generated_pool) { if (descriptor == nullptr) { return Error( "Unable to find definition of google.protobuf.FeatureSet in descriptor pool."); @@ -206,8 +262,22 @@ absl::StatusOr FeatureResolver::Create( RETURN_IF_ERROR(FillDefaults(edition, *defaults)); + FeatureSet generated_defaults; + if (descriptor->file()->pool() == generated_pool) { + // If we're building the generated pool, the best we can do is load the C++ + // language features, which should always be present for code using the C++ + // runtime. + RETURN_IF_ERROR( + FillDefaults(edition, *generated_defaults.MutableExtension(pb::cpp))); + } else { + absl::StatusOr defaults = + FillGeneratedDefaults(edition, generated_pool); + RETURN_IF_ERROR(defaults.status()); + generated_defaults = std::move(defaults).value(); + } + return FeatureResolver(edition, *descriptor, std::move(message_factory), - std::move(defaults)); + std::move(defaults), std::move(generated_defaults)); } absl::Status FeatureResolver::RegisterExtension( @@ -221,24 +291,7 @@ absl::Status FeatureResolver::RegisterExtension( ABSL_CHECK(descriptor_.IsExtensionNumber(extension.number())); - if (extension.message_type() == nullptr) { - return Error("FeatureSet extension ", extension.full_name(), - " is not of message type. Feature extensions should " - "always use messages to allow for evolution."); - } - - if (extension.is_repeated()) { - return Error( - "Only singular features extensions are supported. Found " - "repeated extension ", - extension.full_name()); - } - - if (extension.message_type()->extension_count() > 0 || - extension.message_type()->extension_range_count() > 0) { - return Error("Nested extensions in feature extension ", - extension.full_name(), " are not supported."); - } + RETURN_IF_ERROR(ValidateExtension(extension)); RETURN_IF_ERROR(ValidateDescriptor(edition_, *extension.message_type())); @@ -273,8 +326,12 @@ absl::Status FeatureResolver::RegisterExtensions(const Descriptor& message) { absl::StatusOr FeatureResolver::MergeFeatures( const FeatureSet& merged_parent, const FeatureSet& unmerged_child) const { - FeatureSet merged; - ABSL_CHECK(merged.ParseFromString(defaults_->SerializeAsString())); + FeatureSet merged = generated_defaults_; + + // Our defaults are a dynamic message located in the build pool, while the + // returned object here is in the generated pool. We parse/serialize to + // convert from the potentially skewed FeatureSet descriptors. + ABSL_CHECK(merged.MergeFromString(defaults_->SerializeAsString())); merged.MergeFrom(merged_parent); merged.MergeFrom(unmerged_child); diff --git a/src/google/protobuf/feature_resolver.h b/src/google/protobuf/feature_resolver.h index 013dc65e8508..0d27ee1acf0c 100644 --- a/src/google/protobuf/feature_resolver.h +++ b/src/google/protobuf/feature_resolver.h @@ -37,6 +37,7 @@ #include "absl/container/flat_hash_set.h" #include "absl/status/status.h" +#include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" @@ -58,8 +59,10 @@ class PROTOBUF_EXPORT FeatureResolver { // Creates a new FeatureResolver at a specific edition. This validates the // built-in features for the given edition, and calculates the default feature // set. - static absl::StatusOr Create(absl::string_view edition, - const Descriptor* descriptor); + static absl::StatusOr Create( + absl::string_view edition, const Descriptor* descriptor, + const DescriptorPool* generated_pool = + DescriptorPool::internal_generated_pool()); // Registers a potential extension of the FeatureSet proto. Any visible // extensions will be used during merging. Returns an error if the extension @@ -69,17 +72,22 @@ class PROTOBUF_EXPORT FeatureResolver { // Creates a new feature set using inheritance and default behavior. This is // designed to be called recursively, and the parent feature set is expected // to be a fully merged one. + // The returned FeatureSet will be fully resolved for any extensions that were + // explicitly registered (in the custom pool) or linked into this binary (in + // the generated pool). absl::StatusOr MergeFeatures( const FeatureSet& merged_parent, const FeatureSet& unmerged_child) const; private: FeatureResolver(absl::string_view edition, const Descriptor& descriptor, std::unique_ptr message_factory, - std::unique_ptr defaults) + std::unique_ptr defaults, + FeatureSet generated_defaults) : edition_(edition), descriptor_(descriptor), message_factory_(std::move(message_factory)), - defaults_(std::move(defaults)) {} + defaults_(std::move(defaults)), + generated_defaults_(std::move(generated_defaults)) {} absl::Status RegisterExtensions(const Descriptor& message); absl::Status RegisterExtension(const FieldDescriptor& extension); @@ -89,6 +97,7 @@ class PROTOBUF_EXPORT FeatureResolver { absl::flat_hash_set extensions_; std::unique_ptr message_factory_; std::unique_ptr defaults_; + FeatureSet generated_defaults_; }; } // namespace protobuf diff --git a/src/google/protobuf/feature_resolver_test.cc b/src/google/protobuf/feature_resolver_test.cc index 8d1d7ce47bee..5c5ef41c8ffa 100644 --- a/src/google/protobuf/feature_resolver_test.cc +++ b/src/google/protobuf/feature_resolver_test.cc @@ -37,7 +37,9 @@ #include "absl/memory/memory.h" #include "absl/status/status.h" #include "absl/status/statusor.h" +#include "absl/strings/string_view.h" #include "google/protobuf/compiler/parser.h" +#include "google/protobuf/cpp_features.pb.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/io/tokenizer.h" @@ -87,10 +89,17 @@ const FileDescriptor& GetExtensionFile( ->file(); } +const DescriptorPool* EmptyPool() { + static DescriptorPool* empty_pool = new DescriptorPool(); + return empty_pool; +} + template -absl::StatusOr SetupFeatureResolver(absl::string_view edition, - Extensions... extensions) { - auto resolver = FeatureResolver::Create(edition, FeatureSet::descriptor()); +absl::StatusOr SetupFeatureResolverImpl( + absl::string_view edition, const DescriptorPool* pool, + Extensions... extensions) { + auto resolver = + FeatureResolver::Create(edition, FeatureSet::descriptor(), pool); RETURN_IF_ERROR(resolver.status()); std::vector results{ resolver->RegisterExtensions(GetExtensionFile(extensions))...}; @@ -99,17 +108,35 @@ absl::StatusOr SetupFeatureResolver(absl::string_view edition, } return resolver; } +template +absl::StatusOr SetupFeatureResolver(absl::string_view edition, + Extensions... extensions) { + return SetupFeatureResolverImpl(edition, EmptyPool(), extensions...); +} template -absl::StatusOr GetDefaults(absl::string_view edition, - Extensions... extensions) { +absl::StatusOr GetDefaultsImpl(absl::string_view edition, + const DescriptorPool* pool, + Extensions... extensions) { absl::StatusOr resolver = - SetupFeatureResolver(edition, extensions...); + SetupFeatureResolverImpl(edition, pool, extensions...); RETURN_IF_ERROR(resolver.status()); FeatureSet parent, child; return resolver->MergeFeatures(parent, child); } +template +absl::StatusOr GetDefaults(absl::string_view edition, + Extensions... extensions) { + return GetDefaultsImpl(edition, EmptyPool(), extensions...); +} + +FileDescriptorProto GetProto(const FileDescriptor* file) { + FileDescriptorProto proto; + file->CopyTo(&proto); + return proto; +} + TEST(FeatureResolverTest, DefaultsCore2023) { absl::StatusOr merged = GetDefaults("2023"); ASSERT_OK(merged); @@ -119,6 +146,7 @@ TEST(FeatureResolverTest, DefaultsCore2023) { EXPECT_EQ(merged->repeated_field_encoding(), FeatureSet::PACKED); EXPECT_EQ(merged->string_field_validation(), FeatureSet::MANDATORY); EXPECT_EQ(merged->message_encoding(), FeatureSet::LENGTH_PREFIXED); + EXPECT_FALSE(merged->HasExtension(pb::test)); } TEST(FeatureResolverTest, DefaultsTest2023) { @@ -207,6 +235,34 @@ TEST(FeatureResolverTest, DefaultsTestNestedExtension) { EXPECT_EQ(ext.enum_field_feature(), pb::TestFeatures::ENUM_VALUE1); } +TEST(FeatureResolverTest, DefaultsGeneratedPoolCustom) { + DescriptorPool pool; + ASSERT_NE( + pool.BuildFile(GetProto(google::protobuf::DescriptorProto::descriptor()->file())), + nullptr); + ASSERT_NE(pool.BuildFile(GetProto(pb::TestFeatures::descriptor()->file())), + nullptr); + absl::StatusOr merged = GetDefaultsImpl("2023", &pool); + ASSERT_OK(merged); + + EXPECT_EQ(merged->field_presence(), FeatureSet::EXPLICIT); + EXPECT_TRUE(merged->HasExtension(pb::test)); + EXPECT_EQ(merged->GetExtension(pb::test).int_file_feature(), 1); + EXPECT_FALSE(merged->HasExtension(pb::cpp)); +} + +TEST(FeatureResolverTest, DefaultsGeneratedPoolGenerated) { + absl::StatusOr merged = + GetDefaultsImpl("2023", DescriptorPool::generated_pool()); + ASSERT_OK(merged); + + EXPECT_EQ(merged->field_presence(), FeatureSet::EXPLICIT); + EXPECT_FALSE(merged->HasExtension(pb::test)); + EXPECT_TRUE(merged->HasExtension(pb::cpp)); + EXPECT_EQ(merged->GetExtension(pb::cpp).utf8_validation(), + pb::CppFeatures::VERIFY_PARSE); +} + TEST(FeatureResolverTest, DefaultsTooEarly) { absl::StatusOr merged = GetDefaults("2022", pb::test); EXPECT_THAT(merged, HasError(AllOf(HasSubstr("No valid default found"), @@ -278,12 +334,13 @@ TEST(FeatureResolverTest, DefaultsMessageMerge) { TEST(FeatureResolverTest, InitializePoolMissingDescriptor) { DescriptorPool pool; - EXPECT_THAT(FeatureResolver::Create("2023", nullptr), + EXPECT_THAT(FeatureResolver::Create("2023", nullptr, EmptyPool()), HasError(HasSubstr("find definition of google.protobuf.FeatureSet"))); } TEST(FeatureResolverTest, RegisterExtensionDifferentContainingType) { - auto resolver = FeatureResolver::Create("2023", FeatureSet::descriptor()); + auto resolver = + FeatureResolver::Create("2023", FeatureSet::descriptor(), EmptyPool()); ASSERT_OK(resolver); EXPECT_OK(resolver->RegisterExtensions( @@ -291,7 +348,8 @@ TEST(FeatureResolverTest, RegisterExtensionDifferentContainingType) { } TEST(FeatureResolverTest, RegisterExtensionTwice) { - auto resolver = FeatureResolver::Create("2023", FeatureSet::descriptor()); + auto resolver = + FeatureResolver::Create("2023", FeatureSet::descriptor(), EmptyPool()); ASSERT_OK(resolver); EXPECT_OK(resolver->RegisterExtensions(GetExtensionFile(pb::test))); @@ -530,7 +588,7 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionNonMessage) { ASSERT_NE(file, nullptr); auto resolver = FeatureResolver::Create( - "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet")); + "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool()); ASSERT_OK(resolver); EXPECT_THAT(resolver->RegisterExtensions(*file), @@ -552,7 +610,7 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionRepeated) { ASSERT_NE(file, nullptr); auto resolver = FeatureResolver::Create( - "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet")); + "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool()); ASSERT_OK(resolver); EXPECT_THAT( @@ -582,7 +640,7 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionWithExtensions) { ASSERT_NE(file, nullptr); auto resolver = FeatureResolver::Create( - "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet")); + "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool()); ASSERT_OK(resolver); EXPECT_THAT( @@ -615,7 +673,7 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionWithOneof) { ASSERT_NE(file, nullptr); auto resolver = FeatureResolver::Create( - "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet")); + "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool()); ASSERT_OK(resolver); EXPECT_THAT(resolver->RegisterExtensions(*file), @@ -642,7 +700,7 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionWithRequired) { ASSERT_NE(file, nullptr); auto resolver = FeatureResolver::Create( - "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet")); + "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool()); ASSERT_OK(resolver); EXPECT_THAT(resolver->RegisterExtensions(*file), @@ -669,7 +727,7 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionWithRepeated) { ASSERT_NE(file, nullptr); auto resolver = FeatureResolver::Create( - "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet")); + "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool()); ASSERT_OK(resolver); EXPECT_THAT(resolver->RegisterExtensions(*file), @@ -695,7 +753,7 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionWithMissingTarget) { ASSERT_NE(file, nullptr); auto resolver = FeatureResolver::Create( - "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet")); + "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool()); ASSERT_OK(resolver); EXPECT_THAT(resolver->RegisterExtensions(*file), @@ -725,7 +783,7 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionDefaultsMessageParsingError) { ASSERT_NE(file, nullptr); auto resolver = FeatureResolver::Create( - "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet")); + "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool()); ASSERT_OK(resolver); EXPECT_THAT( resolver->RegisterExtensions(*file), @@ -757,7 +815,7 @@ TEST_F(FeatureResolverPoolTest, ASSERT_NE(file, nullptr); auto resolver = FeatureResolver::Create( - "2024", pool_.FindMessageTypeByName("google.protobuf.FeatureSet")); + "2024", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool()); ASSERT_OK(resolver); EXPECT_THAT( resolver->RegisterExtensions(*file), @@ -789,7 +847,7 @@ TEST_F(FeatureResolverPoolTest, ASSERT_NE(file, nullptr); auto resolver = FeatureResolver::Create( - "2024", pool_.FindMessageTypeByName("google.protobuf.FeatureSet")); + "2024", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool()); ASSERT_OK(resolver); EXPECT_OK(resolver->RegisterExtensions(*file)); FeatureSet parent, child; @@ -815,7 +873,7 @@ TEST_F(FeatureResolverPoolTest, RegisterExtensionDefaultsScalarParsingError) { ASSERT_NE(file, nullptr); auto resolver = FeatureResolver::Create( - "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet")); + "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool()); ASSERT_OK(resolver); EXPECT_THAT( resolver->RegisterExtensions(*file), @@ -843,13 +901,42 @@ TEST_F(FeatureResolverPoolTest, ASSERT_NE(file, nullptr); auto resolver = FeatureResolver::Create( - "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet")); + "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool()); ASSERT_OK(resolver); EXPECT_OK(resolver->RegisterExtensions(*file)); FeatureSet parent, child; EXPECT_OK(resolver->MergeFeatures(parent, child)); } +TEST_F(FeatureResolverPoolTest, GeneratedPoolInvalidExtension) { + const FileDescriptor* file = ParseSchema(R"schema( + syntax = "proto2"; + package test; + import "google/protobuf/descriptor.proto"; + + message Foo {} + extend google.protobuf.FeatureSet { + optional string bar = 9999; + } + )schema"); + ASSERT_NE(file, nullptr); + + EXPECT_THAT(GetDefaultsImpl("2023", &pool_), + HasError(AllOf(HasSubstr("test.bar"), + HasSubstr("is not of message type")))); +} + +TEST_F(FeatureResolverPoolTest, GeneratedPoolDefaultsFailure) { + ASSERT_NE( + pool_.BuildFile(GetProto(google::protobuf::DescriptorProto::descriptor()->file())), + nullptr); + ASSERT_NE(pool_.BuildFile(GetProto(pb::TestFeatures::descriptor()->file())), + nullptr); + EXPECT_THAT( + GetDefaultsImpl("2022", &pool_), + HasError(AllOf(HasSubstr("No valid default found"), HasSubstr("2022")))); +} + } // namespace } // namespace protobuf } // namespace google