Editions: Refactor feature resolution to use an intermediate message.
This places all of the tricky reflection logic of feature resolution into a single location that outputs a portable protobuf message. With this message, feature resolution becomes drastically simpler and easy to reimplement in other languages.
Follow-up changes will provide:
- An API to specify generator features from the CodeGenerator class for C++ plugins
- A CLI for building these default mappings
- Bazel/CMake rules to help embed these mappings in target languages (i.e. for runtimes and non-C++ plugins)
PiperOrigin-RevId: 559615720
diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel
index a89ef2e..4c2db3f 100644
--- a/src/google/protobuf/BUILD.bazel
+++ b/src/google/protobuf/BUILD.bazel
@@ -1109,6 +1109,9 @@
"//src/google/protobuf/compiler:importer",
"//src/google/protobuf/stubs",
"//src/google/protobuf/testing",
+ "@com_google_absl//absl/log:absl_check",
+ "@com_google_absl//absl/log:absl_log",
+ "@com_google_absl//absl/log:die_if_null",
"@com_google_googletest//:gtest",
"@com_google_googletest//:gtest_main",
],
diff --git a/src/google/protobuf/compiler/code_generator_unittest.cc b/src/google/protobuf/compiler/code_generator_unittest.cc
index e0277f6..eea9d48 100644
--- a/src/google/protobuf/compiler/code_generator_unittest.cc
+++ b/src/google/protobuf/compiler/code_generator_unittest.cc
@@ -177,7 +177,8 @@
EXPECT_EQ(features.field_presence(), FeatureSet::EXPLICIT);
EXPECT_EQ(features.enum_type(), FeatureSet::CLOSED);
- EXPECT_TRUE(ext.has_int_message_feature());
+ // TODO(b/296638633) Flip this once generators can specify their feature sets.
+ EXPECT_FALSE(ext.has_int_message_feature());
EXPECT_EQ(ext.int_file_feature(), 8);
EXPECT_EQ(ext.string_source_feature(), "file");
}
diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc
index 1d6d288..c133916 100644
--- a/src/google/protobuf/compiler/command_line_interface_unittest.cc
+++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc
@@ -1353,7 +1353,7 @@
TEST_F(CommandLineInterfaceTest, EditionsAreNotAllowed) {
CreateTempFile("foo.proto",
- "edition = \"very-cool\";\n"
+ "edition = \"2023\";\n"
"message FooRequest {}\n");
Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir foo.proto");
@@ -1365,7 +1365,7 @@
TEST_F(CommandLineInterfaceTest, EditionsFlagExplicitTrue) {
CreateTempFile("foo.proto",
- "edition = \"very-cool\";\n"
+ "edition = \"2023\";\n"
"message FooRequest {}\n");
Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir "
@@ -1477,14 +1477,16 @@
import "features.proto";
message Foo {
int32 bar = 1;
- int32 baz = 2 [features.(pb.test).int_feature = 5];
+ int32 baz = 2 [features.(pb.test).repeated_feature = 5];
})schema");
Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir "
"--experimental_editions foo.proto");
- ExpectErrorSubstring(
- "Feature field pb.TestFeatures.repeated_feature is an unsupported "
- "repeated field");
+ // TODO(b/296638633) Flip this once generators can specify their feature sets.
+ ExpectNoErrors();
+ // ExpectErrorSubstring(
+ // "Feature field pb.TestFeatures.repeated_feature is an unsupported "
+ // "repeated field");
}
TEST_F(CommandLineInterfaceTest, Plugin_LegacyFeatures) {
diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc
index 54e6b7f..100d35e 100644
--- a/src/google/protobuf/descriptor.cc
+++ b/src/google/protobuf/descriptor.cc
@@ -44,6 +44,7 @@
#include <sstream>
#include <string>
#include <type_traits>
+#include <utility>
#include <vector>
#include "google/protobuf/stubs/common.h"
@@ -57,6 +58,7 @@
#include "absl/hash/hash.h"
#include "absl/log/absl_check.h"
#include "absl/log/absl_log.h"
+#include "absl/memory/memory.h"
#include "absl/status/statusor.h"
#include "absl/strings/ascii.h"
#include "absl/strings/escaping.h"
@@ -1111,6 +1113,18 @@
allowed_proto3_extendees->end();
}
+const FeatureSetDefaults& GetCppFeatureSetDefaults() {
+ static const FeatureSetDefaults* default_spec = [] {
+ auto default_spec = FeatureResolver::CompileDefaults(
+ FeatureSet::descriptor(),
+ // TODO(b/297261063) Move this range to a central location.
+ {pb::CppFeatures::descriptor()->file()->extension(0)}, "2023", "2023");
+ ABSL_CHECK(default_spec.ok()) << default_spec.status();
+ return new FeatureSetDefaults(std::move(default_spec).value());
+ }();
+ return *default_spec;
+}
+
// Create global defaults for proto2/proto3 compatibility.
FeatureSet* CreateProto2DefaultFeatures() {
FeatureSet* features = new FeatureSet();
@@ -4577,14 +4591,7 @@
const FileDescriptor* DescriptorPool::BuildFile(
const FileDescriptorProto& proto) {
- ABSL_CHECK(fallback_database_ == nullptr)
- << "Cannot call BuildFile on a DescriptorPool that uses a "
- "DescriptorDatabase. You must instead find a way to get your file "
- "into the underlying database.";
- ABSL_CHECK(mutex_ == nullptr); // Implied by the above ABSL_CHECK.
- tables_->known_bad_symbols_.clear();
- tables_->known_bad_files_.clear();
- return DescriptorBuilder::New(this, tables_.get(), nullptr)->BuildFile(proto);
+ return BuildFileCollectingErrors(proto, nullptr);
}
const FileDescriptor* DescriptorPool::BuildFileCollectingErrors(
@@ -4596,6 +4603,7 @@
ABSL_CHECK(mutex_ == nullptr); // Implied by the above ABSL_CHECK.
tables_->known_bad_symbols_.clear();
tables_->known_bad_files_.clear();
+ build_started_ = true;
return DescriptorBuilder::New(this, tables_.get(), error_collector)
->BuildFile(proto);
}
@@ -4603,6 +4611,7 @@
const FileDescriptor* DescriptorPool::BuildFileFromDatabase(
const FileDescriptorProto& proto) const {
mutex_->AssertHeld();
+ build_started_ = true;
if (tables_->known_bad_files_.contains(proto.name())) {
return nullptr;
}
@@ -4615,6 +4624,14 @@
return result;
}
+void DescriptorPool::SetFeatureSetDefaults(FeatureSetDefaults spec) {
+ ABSL_CHECK(!build_started_)
+ << "Feature set defaults can't be changed once the pool has started "
+ "building.";
+ feature_set_defaults_spec_ =
+ absl::make_unique<FeatureSetDefaults>(std::move(spec));
+}
+
DescriptorBuilder::DescriptorBuilder(
const DescriptorPool* pool, DescriptorPool::Tables* tables,
DescriptorPool::ErrorCollector* error_collector)
@@ -5699,8 +5716,13 @@
}
ABSL_CHECK(descriptor);
+ const FeatureSetDefaults& defaults =
+ pool_->feature_set_defaults_spec_ == nullptr
+ ? GetCppFeatureSetDefaults()
+ : *pool_->feature_set_defaults_spec_;
+
absl::StatusOr<FeatureResolver> feature_resolver =
- FeatureResolver::Create(proto.edition(), descriptor);
+ FeatureResolver::Create(proto.edition(), defaults);
if (!feature_resolver.ok()) {
AddError(
proto.name(), proto, DescriptorPool::ErrorCollector::EDITIONS,
@@ -5816,16 +5838,6 @@
return nullptr;
}
- // Look for feature extensions in regular imports.
- if (feature_resolver_.has_value() && dependency != nullptr) {
- absl::Status status = feature_resolver_->RegisterExtensions(*dependency);
- if (!status.ok()) {
- AddError(dependency->name(), proto,
- DescriptorPool::ErrorCollector::EDITIONS,
- [&] { return std::string(status.message()); });
- }
- }
-
if (dependency == nullptr) {
if (!pool_->lazily_build_dependencies_) {
if (pool_->allow_unknown_ ||
diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h
index 15e0057..2c6a779 100644
--- a/src/google/protobuf/descriptor.h
+++ b/src/google/protobuf/descriptor.h
@@ -122,6 +122,7 @@
class FileOptions;
class UninterpretedOption;
class FeatureSet;
+class FeatureSetDefaults;
class SourceCodeInfo;
// Defined in message_lite.h
@@ -2289,6 +2290,13 @@
// DescriptorPool will report a import not found error.
void EnforceWeakDependencies(bool enforce) { enforce_weak_ = enforce; }
+ // Sets the default feature mappings used during the build. If this function
+ // isn't called, the C++ feature set defaults are used. If this function is
+ // called, these defaults will be used instead.
+ // FeatureSetDefaults includes a minimum/maximum supported edition, which will
+ // be enforced while building proto files.
+ void SetFeatureSetDefaults(FeatureSetDefaults spec);
+
// Toggles enforcement of extension declarations.
// This enforcement is disabled by default because it requires full
// descriptors with source-retention options, which are generally not
@@ -2469,11 +2477,16 @@
bool enforce_extension_declarations_;
bool disallow_enforce_utf8_;
bool deprecated_legacy_json_field_conflicts_;
+ mutable bool build_started_ = false;
// Set of files to track for unused imports. The bool value when true means
// unused imports are treated as errors (and as warnings when false).
absl::flat_hash_map<std::string, bool> unused_import_track_files_;
+ // Specification of defaults to use for feature resolution. This defaults to
+ // just the global and C++ features, but can be overridden for other runtimes.
+ std::unique_ptr<FeatureSetDefaults> feature_set_defaults_spec_;
+
// Returns true if the field extends an option message of descriptor.proto.
bool IsExtendingDescriptor(const FieldDescriptor& field) const;
diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc
index 10a69cb..43ef3fa 100644
--- a/src/google/protobuf/descriptor_unittest.cc
+++ b/src/google/protobuf/descriptor_unittest.cc
@@ -41,6 +41,7 @@
#include <memory>
#include <string>
#include <tuple>
+#include <utility>
#include <vector>
#include "google/protobuf/stubs/common.h"
@@ -67,6 +68,7 @@
#include "google/protobuf/descriptor_database.h"
#include "google/protobuf/descriptor_legacy.h"
#include "google/protobuf/dynamic_message.h"
+#include "google/protobuf/feature_resolver.h"
#include "google/protobuf/io/tokenizer.h"
#include "google/protobuf/io/zero_copy_stream_impl_lite.h"
#include "google/protobuf/test_textproto.h"
@@ -514,17 +516,17 @@
}
{
proto.set_syntax("editions");
- proto.set_edition("very-cool");
+ proto.set_edition("2023");
DescriptorPool pool;
const FileDescriptor* file = pool.BuildFile(proto);
ASSERT_TRUE(file != nullptr);
EXPECT_EQ(FileDescriptorLegacy::Syntax::SYNTAX_EDITIONS,
FileDescriptorLegacy(file).syntax());
- EXPECT_EQ("very-cool", file->edition());
+ EXPECT_EQ("2023", file->edition());
FileDescriptorProto other;
file->CopyTo(&other);
EXPECT_EQ("editions", other.syntax());
- EXPECT_EQ("very-cool", other.edition());
+ EXPECT_EQ("2023", other.edition());
}
}
@@ -552,7 +554,7 @@
EXPECT_EQ(&other.options().features(), &FeatureSet::default_instance());
{
proto.set_syntax("editions");
- proto.set_edition("very-cool");
+ proto.set_edition("2023");
DescriptorPool pool;
const FileDescriptor* file = pool.BuildFile(proto);
@@ -563,7 +565,7 @@
EXPECT_EQ(other.name(), "foo.proto");
EXPECT_EQ(other.package(), "foo.bar.baz");
EXPECT_EQ(other.syntax(), "editions");
- EXPECT_EQ(other.edition(), "very-cool");
+ EXPECT_EQ(other.edition(), "2023");
EXPECT_EQ(other.options().java_package(), "foo.bar.baz");
EXPECT_TRUE(other.message_type().empty());
EXPECT_EQ(&other.options().features(), &FeatureSet::default_instance());
@@ -7235,7 +7237,24 @@
"foo.proto: Foo.foo: EXTENDEE: \"Baz\" is not defined.\n");
}
-using FeaturesTest = ValidationErrorTest;
+using FeaturesBaseTest = ValidationErrorTest;
+
+class FeaturesTest : public FeaturesBaseTest {
+ protected:
+ void SetUp() override {
+ ValidationErrorTest::SetUp();
+
+ auto default_spec = FeatureResolver::CompileDefaults(
+ FeatureSet::descriptor(),
+ {pb::CppFeatures::descriptor()->file()->extension(0),
+ pb::TestFeatures::descriptor()->file()->extension(0),
+ pb::TestMessage::descriptor()->extension(0),
+ pb::TestMessage::Nested::descriptor()->extension(0)},
+ "2023", "2025");
+ ASSERT_OK(default_spec);
+ pool_.SetFeatureSetDefaults(std::move(default_spec).value());
+ }
+};
template <typename T>
const FeatureSet& GetFeatures(const T* descriptor) {
@@ -7544,12 +7563,36 @@
[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.
+ // Since pb::test is registered in the pool, 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(FeaturesBaseTest, DefaultEdition2023Defaults) {
+ BuildDescriptorMessagesInTestPool();
+ BuildFileInTestPool(pb::TestFeatures::descriptor()->file());
+ const FileDescriptor* file = BuildFile(R"pb(
+ name: "foo.proto"
+ syntax: "editions"
+ edition: "2023"
+ dependency: "google/protobuf/unittest_features.proto"
+ )pb");
+ ASSERT_NE(file, nullptr);
+
+ EXPECT_THAT(file->options(), EqualsProto(""));
+ EXPECT_THAT(
+ GetFeatures(file), EqualsProto(R"pb(
+ field_presence: EXPLICIT
+ enum_type: OPEN
+ repeated_field_encoding: PACKED
+ message_encoding: LENGTH_PREFIXED
+ json_format: ALLOW
+ [pb.cpp] { legacy_closed_enum: false utf8_validation: VERIFY_PARSE }
+ )pb"));
+ EXPECT_FALSE(GetFeatures(file).HasExtension(pb::test));
+}
+
TEST_F(FeaturesTest, ClearsOptions) {
BuildDescriptorMessagesInTestPool();
const FileDescriptor* file = BuildFile(R"pb(
@@ -7842,9 +7885,8 @@
R"pb(
name: "foo.proto" syntax: "editions" edition: "2022"
)pb",
- "foo.proto: foo.proto: EDITIONS: No valid default found for edition 2022 "
- "in "
- "feature field google.protobuf.FeatureSet.field_presence\n");
+ "foo.proto: foo.proto: EDITIONS: Edition 2022 is earlier than the "
+ "minimum supported edition 2023\n");
}
TEST_F(FeaturesTest, FileFeatures) {
@@ -9110,48 +9152,6 @@
"editions.\n");
}
-TEST_F(FeaturesTest, InvalidExtensionNonMessage) {
- BuildDescriptorMessagesInTestPool();
- 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: "unittest_invalid_features.proto"
- options {
- 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: 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) {
BuildDescriptorMessagesInTestPool();
BuildFileWithErrors(
diff --git a/src/google/protobuf/feature_resolver.cc b/src/google/protobuf/feature_resolver.cc
index 46b3beb..aa4858b 100644
--- a/src/google/protobuf/feature_resolver.cc
+++ b/src/google/protobuf/feature_resolver.cc
@@ -31,6 +31,7 @@
#include "google/protobuf/feature_resolver.h"
#include <algorithm>
+#include <cstddef>
#include <iterator>
#include <memory>
#include <string>
@@ -38,13 +39,17 @@
#include <vector>
#include "absl/algorithm/container.h"
+#include "absl/container/btree_set.h"
+#include "absl/container/flat_hash_set.h"
#include "absl/log/absl_check.h"
#include "absl/memory/memory.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/str_cat.h"
+#include "absl/strings/str_join.h"
#include "absl/strings/str_split.h"
#include "absl/strings/string_view.h"
+#include "absl/types/span.h"
#include "google/protobuf/cpp_features.pb.h"
#include "google/protobuf/descriptor.h"
#include "google/protobuf/descriptor.pb.h"
@@ -71,24 +76,25 @@
return absl::FailedPreconditionError(absl::StrCat(args...));
}
-bool EditionsLessThan(absl::string_view a, absl::string_view b) {
- std::vector<absl::string_view> as = absl::StrSplit(a, '.');
- std::vector<absl::string_view> bs = absl::StrSplit(b, '.');
- size_t min_size = std::min(as.size(), bs.size());
- for (size_t i = 0; i < min_size; ++i) {
- if (as[i].size() != bs[i].size()) {
- return as[i].size() < bs[i].size();
- } else if (as[i] != bs[i]) {
- return as[i] < bs[i];
+struct EditionsLessThan {
+ bool operator()(absl::string_view a, absl::string_view b) const {
+ std::vector<absl::string_view> as = absl::StrSplit(a, '.');
+ std::vector<absl::string_view> bs = absl::StrSplit(b, '.');
+ size_t min_size = std::min(as.size(), bs.size());
+ for (size_t i = 0; i < min_size; ++i) {
+ if (as[i].size() != bs[i].size()) {
+ return as[i].size() < bs[i].size();
+ } else if (as[i] != bs[i]) {
+ return as[i] < bs[i];
+ }
}
+ // Both strings are equal up until an extra element, which makes that string
+ // more recent.
+ return as.size() < bs.size();
}
- // Both strings are equal up until an extra element, which makes that string
- // more recent.
- return as.size() < bs.size();
-}
+};
-absl::Status ValidateDescriptor(absl::string_view edition,
- const Descriptor& descriptor) {
+absl::Status ValidateDescriptor(const Descriptor& descriptor) {
if (descriptor.oneof_decl_count() > 0) {
return Error("Type ", descriptor.full_name(),
" contains unsupported oneof feature fields.");
@@ -113,35 +119,57 @@
return absl::OkStatus();
}
-absl::Status ValidateExtension(const FieldDescriptor& extension) {
- if (extension.message_type() == nullptr) {
- return Error("FeatureSet extension ", extension.full_name(),
+absl::Status ValidateExtension(const Descriptor& feature_set,
+ const FieldDescriptor* extension) {
+ if (extension == nullptr) {
+ return Error("Unknown extension of ", feature_set.full_name(), ".");
+ }
+
+ if (extension->containing_type() != &feature_set) {
+ return Error("Extension ", extension->full_name(),
+ " is not an extension of ", feature_set.full_name(), ".");
+ }
+
+ 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()) {
+ if (extension->is_repeated()) {
return Error(
"Only singular features extensions are supported. Found "
"repeated extension ",
- extension.full_name());
+ extension->full_name());
}
- if (extension.message_type()->extension_count() > 0 ||
- extension.message_type()->extension_range_count() > 0) {
+ 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.");
+ extension->full_name(), " are not supported.");
}
return absl::OkStatus();
}
+void CollectEditions(const Descriptor& descriptor,
+ absl::string_view minimum_edition,
+ absl::string_view maximum_edition,
+ absl::btree_set<std::string, EditionsLessThan>& editions) {
+ for (int i = 0; i < descriptor.field_count(); ++i) {
+ for (const auto& def : descriptor.field(i)->options().edition_defaults()) {
+ if (EditionsLessThan()(maximum_edition, def.edition())) continue;
+ editions.insert(def.edition());
+ }
+ }
+}
+
absl::Status FillDefaults(absl::string_view edition, Message& msg) {
const Descriptor& descriptor = *msg.GetDescriptor();
auto comparator = [](const FieldOptions::EditionDefault& a,
const FieldOptions::EditionDefault& b) {
- return EditionsLessThan(a.edition(), b.edition());
+ return EditionsLessThan()(a.edition(), b.edition());
};
FieldOptions::EditionDefault edition_lookup;
edition_lookup.set_edition(edition);
@@ -206,124 +234,100 @@
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<FeatureSet> 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<const FieldDescriptor*> 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> FeatureResolver::Create(
- absl::string_view edition, const Descriptor* descriptor,
- const DescriptorPool* generated_pool) {
- if (descriptor == nullptr) {
+absl::StatusOr<FeatureSetDefaults> FeatureResolver::CompileDefaults(
+ const Descriptor* feature_set,
+ absl::Span<const FieldDescriptor* const> extensions,
+ absl::string_view minimum_edition, absl::string_view maximum_edition) {
+ // Find and validate the FeatureSet in the pool.
+ if (feature_set == nullptr) {
return Error(
"Unable to find definition of google.protobuf.FeatureSet in descriptor pool.");
}
+ RETURN_IF_ERROR(ValidateDescriptor(*feature_set));
- RETURN_IF_ERROR(ValidateDescriptor(edition, *descriptor));
+ // Collect and validate all the FeatureSet extensions.
+ for (const auto* extension : extensions) {
+ RETURN_IF_ERROR(ValidateExtension(*feature_set, extension));
+ RETURN_IF_ERROR(ValidateDescriptor(*extension->message_type()));
+ }
+ // Collect all the editions with unique defaults.
+ absl::btree_set<std::string, EditionsLessThan> editions;
+ CollectEditions(*feature_set, minimum_edition, maximum_edition, editions);
+ for (const auto* extension : extensions) {
+ CollectEditions(*extension->message_type(), minimum_edition,
+ maximum_edition, editions);
+ }
+
+ // Fill the default spec.
+ FeatureSetDefaults defaults;
+ defaults.set_minimum_edition(minimum_edition);
+ defaults.set_maximum_edition(maximum_edition);
auto message_factory = absl::make_unique<DynamicMessageFactory>();
- auto defaults =
- absl::WrapUnique(message_factory->GetPrototype(descriptor)->New());
-
- 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<FeatureSet> defaults =
- FillGeneratedDefaults(edition, generated_pool);
- RETURN_IF_ERROR(defaults.status());
- generated_defaults = std::move(defaults).value();
+ for (const auto& edition : editions) {
+ auto defaults_dynamic =
+ absl::WrapUnique(message_factory->GetPrototype(feature_set)->New());
+ RETURN_IF_ERROR(FillDefaults(edition, *defaults_dynamic));
+ for (const auto* extension : extensions) {
+ RETURN_IF_ERROR(FillDefaults(
+ edition, *defaults_dynamic->GetReflection()->MutableMessage(
+ defaults_dynamic.get(), extension)));
+ }
+ auto* edition_defaults = defaults.mutable_defaults()->Add();
+ edition_defaults->set_edition(edition);
+ edition_defaults->mutable_features()->MergeFromString(
+ defaults_dynamic->SerializeAsString());
}
-
- return FeatureResolver(edition, *descriptor, std::move(message_factory),
- std::move(defaults), std::move(generated_defaults));
+ return defaults;
}
-absl::Status FeatureResolver::RegisterExtension(
- const FieldDescriptor& extension) {
- if (!extension.is_extension() ||
- extension.containing_type() != &descriptor_ ||
- extensions_.contains(&extension)) {
- // These are valid but irrelevant extensions, just return ok.
- return absl::OkStatus();
+absl::StatusOr<FeatureResolver> FeatureResolver::Create(
+ absl::string_view edition, const FeatureSetDefaults& compiled_defaults) {
+ if (EditionsLessThan()(edition, compiled_defaults.minimum_edition())) {
+ return Error("Edition ", edition,
+ " is earlier than the minimum supported edition ",
+ compiled_defaults.minimum_edition());
+ }
+ if (EditionsLessThan()(compiled_defaults.maximum_edition(), edition)) {
+ return Error("Edition ", edition,
+ " is later than the maximum supported edition ",
+ compiled_defaults.maximum_edition());
}
- ABSL_CHECK(descriptor_.IsExtensionNumber(extension.number()));
-
- RETURN_IF_ERROR(ValidateExtension(extension));
-
- RETURN_IF_ERROR(ValidateDescriptor(edition_, *extension.message_type()));
-
- Message* msg =
- defaults_->GetReflection()->MutableMessage(defaults_.get(), &extension);
- ABSL_CHECK(msg != nullptr);
- RETURN_IF_ERROR(FillDefaults(edition_, *msg));
-
- extensions_.insert(&extension);
- return absl::OkStatus();
-}
-
-absl::Status FeatureResolver::RegisterExtensions(const FileDescriptor& file) {
- for (int i = 0; i < file.extension_count(); ++i) {
- RETURN_IF_ERROR(RegisterExtension(*file.extension(i)));
+ // Validate compiled defaults.
+ absl::string_view prev_edition;
+ for (const auto& edition_default : compiled_defaults.defaults()) {
+ if (!prev_edition.empty()) {
+ if (!EditionsLessThan()(prev_edition, edition_default.edition())) {
+ return Error(
+ "Feature set defaults are not strictly increasing. Edition ",
+ prev_edition, " is greater than or equal to edition ",
+ edition_default.edition(), ".");
+ }
+ }
+ prev_edition = edition_default.edition();
}
- for (int i = 0; i < file.message_type_count(); ++i) {
- RETURN_IF_ERROR(RegisterExtensions(*file.message_type(i)));
- }
- return absl::OkStatus();
-}
-absl::Status FeatureResolver::RegisterExtensions(const Descriptor& message) {
- for (int i = 0; i < message.extension_count(); ++i) {
- RETURN_IF_ERROR(RegisterExtension(*message.extension(i)));
+ // Select the matching edition defaults.
+ auto comparator = [](const auto& a, const auto& b) {
+ return EditionsLessThan()(a.edition(), b.edition());
+ };
+ FeatureSetDefaults::FeatureSetEditionDefault search;
+ search.set_edition(edition);
+ auto first_nonmatch =
+ absl::c_upper_bound(compiled_defaults.defaults(), search, comparator);
+ if (first_nonmatch == compiled_defaults.defaults().begin()) {
+ return Error("No valid default found for edition ", edition);
}
- for (int i = 0; i < message.nested_type_count(); ++i) {
- RETURN_IF_ERROR(RegisterExtensions(*message.nested_type(i)));
- }
- return absl::OkStatus();
+
+ return FeatureResolver(std::prev(first_nonmatch)->features());
}
absl::StatusOr<FeatureSet> FeatureResolver::MergeFeatures(
const FeatureSet& merged_parent, const FeatureSet& unmerged_child) const {
- 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()));
+ FeatureSet merged = defaults_;
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 0d27ee1..6b4ca27 100644
--- a/src/google/protobuf/feature_resolver.h
+++ b/src/google/protobuf/feature_resolver.h
@@ -39,6 +39,7 @@
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/string_view.h"
+#include "absl/types/span.h"
#include "google/protobuf/descriptor.h"
#include "google/protobuf/descriptor.pb.h"
#include "google/protobuf/dynamic_message.h"
@@ -56,48 +57,32 @@
FeatureResolver(FeatureResolver&&) = default;
FeatureResolver& operator=(FeatureResolver&&) = delete;
- // 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<FeatureResolver> Create(
- absl::string_view edition, const Descriptor* descriptor,
- const DescriptorPool* generated_pool =
- DescriptorPool::internal_generated_pool());
+ // Compiles a set of FeatureSet extensions into a mapping of edition to unique
+ // defaults. This is the most complicated part of feature resolution, and by
+ // abstracting this out into an intermediate message, we can make feature
+ // resolution significantly more portable.
+ static absl::StatusOr<FeatureSetDefaults> CompileDefaults(
+ const Descriptor* feature_set,
+ absl::Span<const FieldDescriptor* const> extensions,
+ absl::string_view minimum_edition, absl::string_view maximum_edition);
- // Registers a potential extension of the FeatureSet proto. Any visible
- // extensions will be used during merging. Returns an error if the extension
- // is a feature extension but fails validation.
- absl::Status RegisterExtensions(const FileDescriptor& file);
+ // Creates a new FeatureResolver at a specific edition. This calculates the
+ // default feature set for that edition, using the output of CompileDefaults.
+ static absl::StatusOr<FeatureResolver> Create(
+ absl::string_view edition, const FeatureSetDefaults& defaults);
// 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).
+ // to be a fully merged one. The returned FeatureSet will be fully resolved
+ // for any extensions that were used to construct the defaults.
absl::StatusOr<FeatureSet> MergeFeatures(
const FeatureSet& merged_parent, const FeatureSet& unmerged_child) const;
private:
- FeatureResolver(absl::string_view edition, const Descriptor& descriptor,
- std::unique_ptr<DynamicMessageFactory> message_factory,
- std::unique_ptr<Message> defaults,
- FeatureSet generated_defaults)
- : edition_(edition),
- descriptor_(descriptor),
- message_factory_(std::move(message_factory)),
- defaults_(std::move(defaults)),
- generated_defaults_(std::move(generated_defaults)) {}
+ explicit FeatureResolver(FeatureSet defaults)
+ : defaults_(std::move(defaults)) {}
- absl::Status RegisterExtensions(const Descriptor& message);
- absl::Status RegisterExtension(const FieldDescriptor& extension);
-
- std::string edition_;
- const Descriptor& descriptor_;
- absl::flat_hash_set<const FieldDescriptor*> extensions_;
- std::unique_ptr<DynamicMessageFactory> message_factory_;
- std::unique_ptr<Message> defaults_;
- FeatureSet generated_defaults_;
+ FeatureSet defaults_;
};
} // namespace protobuf
diff --git a/src/google/protobuf/feature_resolver_test.cc b/src/google/protobuf/feature_resolver_test.cc
index d42c491..aa49bfb 100644
--- a/src/google/protobuf/feature_resolver_test.cc
+++ b/src/google/protobuf/feature_resolver_test.cc
@@ -30,10 +30,14 @@
#include "google/protobuf/feature_resolver.h"
+#include <utility>
#include <vector>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include "absl/log/absl_check.h"
+#include "absl/log/absl_log.h"
+#include "absl/log/die_if_null.h"
#include "absl/memory/memory.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
@@ -43,6 +47,7 @@
#include "google/protobuf/descriptor.h"
#include "google/protobuf/descriptor.pb.h"
#include "google/protobuf/io/tokenizer.h"
+#include "google/protobuf/io/zero_copy_stream_impl_lite.h"
#include "google/protobuf/test_textproto.h"
#include "google/protobuf/text_format.h"
#include "google/protobuf/unittest_custom_options.pb.h"
@@ -61,7 +66,6 @@
using ::testing::HasSubstr;
// TODO(b/234474291): Use the gtest versions once that's available in OSS.
-absl::Status GetStatus(const absl::Status& s) { return s; }
template <typename T>
absl::Status GetStatus(const absl::StatusOr<T>& s) {
return s.status();
@@ -81,45 +85,28 @@
#define ASSERT_OK(x) ASSERT_THAT(x, StatusIs(absl::StatusCode::kOk))
template <typename ExtensionT>
-const FileDescriptor& GetExtensionFile(
+const FieldDescriptor* GetExtension(
const ExtensionT& ext,
const Descriptor* descriptor = FeatureSet::descriptor()) {
- return *DescriptorPool::generated_pool()
- ->FindExtensionByNumber(descriptor, ext.number())
- ->file();
+ return ABSL_DIE_IF_NULL(descriptor->file()->pool()->FindExtensionByNumber(
+ descriptor, ext.number()));
}
-const DescriptorPool* EmptyPool() {
- static DescriptorPool* empty_pool = new DescriptorPool();
- return empty_pool;
-}
-
-template <typename... Extensions>
-absl::StatusOr<FeatureResolver> 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<absl::Status> results{
- resolver->RegisterExtensions(GetExtensionFile(extensions))...};
- for (absl::Status result : results) {
- RETURN_IF_ERROR(result);
- }
- return resolver;
-}
template <typename... Extensions>
absl::StatusOr<FeatureResolver> SetupFeatureResolver(absl::string_view edition,
Extensions... extensions) {
- return SetupFeatureResolverImpl(edition, EmptyPool(), extensions...);
+ absl::StatusOr<FeatureSetDefaults> defaults =
+ FeatureResolver::CompileDefaults(FeatureSet::descriptor(),
+ {GetExtension(extensions)...}, "2023",
+ "2023");
+ RETURN_IF_ERROR(defaults.status());
+ return FeatureResolver::Create(edition, *defaults);
}
-template <typename... Extensions>
-absl::StatusOr<FeatureSet> GetDefaultsImpl(absl::string_view edition,
- const DescriptorPool* pool,
- Extensions... extensions) {
+absl::StatusOr<FeatureSet> GetDefaults(absl::string_view edition,
+ const FeatureSetDefaults& defaults) {
absl::StatusOr<FeatureResolver> resolver =
- SetupFeatureResolverImpl(edition, pool, extensions...);
+ FeatureResolver::Create(edition, defaults);
RETURN_IF_ERROR(resolver.status());
FeatureSet parent, child;
return resolver->MergeFeatures(parent, child);
@@ -128,7 +115,12 @@
template <typename... Extensions>
absl::StatusOr<FeatureSet> GetDefaults(absl::string_view edition,
Extensions... extensions) {
- return GetDefaultsImpl(edition, EmptyPool(), extensions...);
+ absl::StatusOr<FeatureSetDefaults> defaults =
+ FeatureResolver::CompileDefaults(FeatureSet::descriptor(),
+ {GetExtension(extensions)...}, "0",
+ "99999");
+ RETURN_IF_ERROR(defaults.status());
+ return GetDefaults(edition, *defaults);
}
FileDescriptorProto GetProto(const FileDescriptor* file) {
@@ -184,8 +176,10 @@
EXPECT_EQ(merged->enum_type(), FeatureSet::OPEN);
EXPECT_EQ(merged->repeated_field_encoding(), FeatureSet::PACKED);
EXPECT_EQ(merged->message_encoding(), FeatureSet::LENGTH_PREFIXED);
+ EXPECT_FALSE(merged->HasExtension(pb::test));
- const pb::TestFeatures& ext = merged->GetExtension(pb::test);
+ const pb::TestFeatures& ext =
+ merged->GetExtension(pb::TestMessage::test_message);
EXPECT_EQ(ext.int_file_feature(), 1);
EXPECT_EQ(ext.int_extension_range_feature(), 1);
EXPECT_EQ(ext.int_message_feature(), 1);
@@ -212,8 +206,10 @@
EXPECT_EQ(merged->enum_type(), FeatureSet::OPEN);
EXPECT_EQ(merged->repeated_field_encoding(), FeatureSet::PACKED);
EXPECT_EQ(merged->message_encoding(), FeatureSet::LENGTH_PREFIXED);
+ EXPECT_FALSE(merged->HasExtension(pb::test));
- const pb::TestFeatures& ext = merged->GetExtension(pb::test);
+ const pb::TestFeatures& ext =
+ merged->GetExtension(pb::TestMessage::Nested::test_nested);
EXPECT_EQ(ext.int_file_feature(), 1);
EXPECT_EQ(ext.int_extension_range_feature(), 1);
EXPECT_EQ(ext.int_message_feature(), 1);
@@ -238,25 +234,19 @@
nullptr);
ASSERT_NE(pool.BuildFile(GetProto(pb::TestFeatures::descriptor()->file())),
nullptr);
- absl::StatusOr<FeatureSet> merged = GetDefaultsImpl("2023", &pool);
- ASSERT_OK(merged);
+ absl::StatusOr<FeatureSetDefaults> defaults =
+ FeatureResolver::CompileDefaults(
+ pool.FindMessageTypeByName("google.protobuf.FeatureSet"),
+ {pool.FindExtensionByName("pb.test")}, "2023", "2023");
+ ASSERT_OK(defaults);
+ ASSERT_EQ(defaults->defaults().size(), 1);
+ ASSERT_EQ(defaults->defaults().at(0).edition(), "2023");
+ FeatureSet merged = defaults->defaults().at(0).features();
- 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<FeatureSet> 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);
+ 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, DefaultsTooEarly) {
@@ -328,28 +318,48 @@
}
}
-TEST(FeatureResolverTest, InitializePoolMissingDescriptor) {
- DescriptorPool pool;
- EXPECT_THAT(FeatureResolver::Create("2023", nullptr, EmptyPool()),
+TEST(FeatureResolverTest, CreateFromUnsortedDefaults) {
+ FeatureSetDefaults defaults = ParseTextOrDie(R"pb(
+ minimum_edition: "2022"
+ maximum_edition: "2024"
+ defaults {
+ edition: "2022"
+ features {}
+ }
+ defaults {
+ edition: "2024"
+ features {}
+ }
+ defaults {
+ edition: "2023"
+ features {}
+ }
+ )pb");
+ EXPECT_THAT(
+ FeatureResolver::Create("2023", defaults),
+ HasError(AllOf(
+ HasSubstr("not strictly increasing."),
+ HasSubstr("Edition 2024 is greater than or equal to edition 2023"))));
+}
+
+TEST(FeatureResolverTest, CompileDefaultsMissingDescriptor) {
+ EXPECT_THAT(FeatureResolver::CompileDefaults(nullptr, {}, "2023", "2023"),
HasError(HasSubstr("find definition of google.protobuf.FeatureSet")));
}
-TEST(FeatureResolverTest, RegisterExtensionDifferentContainingType) {
- auto resolver =
- FeatureResolver::Create("2023", FeatureSet::descriptor(), EmptyPool());
- ASSERT_OK(resolver);
-
- EXPECT_OK(resolver->RegisterExtensions(
- GetExtensionFile(protobuf_unittest::file_opt1, FileOptions::descriptor())));
+TEST(FeatureResolverTest, CompileDefaultsMissingExtension) {
+ EXPECT_THAT(FeatureResolver::CompileDefaults(FeatureSet::descriptor(),
+ {nullptr}, "2023", "2023"),
+ HasError(HasSubstr("Unknown extension")));
}
-TEST(FeatureResolverTest, RegisterExtensionTwice) {
- auto resolver =
- FeatureResolver::Create("2023", FeatureSet::descriptor(), EmptyPool());
- ASSERT_OK(resolver);
-
- EXPECT_OK(resolver->RegisterExtensions(GetExtensionFile(pb::test)));
- EXPECT_OK(resolver->RegisterExtensions(GetExtensionFile(pb::test)));
+TEST(FeatureResolverTest, CompileDefaultsInvalidExtension) {
+ EXPECT_THAT(
+ FeatureResolver::CompileDefaults(
+ FeatureSet::descriptor(),
+ {GetExtension(protobuf_unittest::file_opt1, FileOptions::descriptor())},
+ "2023", "2023"),
+ HasError(HasSubstr("is not an extension of")));
}
TEST(FeatureResolverTest, MergeFeaturesChildOverrideCore) {
@@ -508,13 +518,17 @@
pb::TestFeatures::TEST_ENUM_FEATURE_UNKNOWN);
}
-// TODO(b/273611177) Move the following tests to descriptor_unittest.cc once
-// FeatureResolver is hooked up. These will all fail during the descriptor
-// build, invalidating the test setup.
-//
-// Note: We should make sure to keep coverage over the custom DescriptorPool
-// cases. These are the only tests covering issues there (the ones above use
-// the generated descriptor pool).
+TEST(FeatureResolverTest, MergeFeaturesDistantPast) {
+ EXPECT_THAT(SetupFeatureResolver("1000"),
+ HasError(AllOf(HasSubstr("Edition 1000"),
+ HasSubstr("minimum supported edition 2023"))));
+}
+
+TEST(FeatureResolverTest, MergeFeaturesDistantFuture) {
+ EXPECT_THAT(SetupFeatureResolver("9999"),
+ HasError(AllOf(HasSubstr("Edition 9999"),
+ HasSubstr("maximum supported edition 2023"))));
+}
class FakeErrorCollector : public io::ErrorCollector {
public:
@@ -534,6 +548,12 @@
FileDescriptorProto file;
FileDescriptorProto::GetDescriptor()->file()->CopyTo(&file);
ASSERT_NE(pool_.BuildFile(file), nullptr);
+ feature_set_ = pool_.FindMessageTypeByName("google.protobuf.FeatureSet");
+ ASSERT_NE(feature_set_, nullptr);
+ auto defaults =
+ FeatureResolver::CompileDefaults(feature_set_, {}, "2023", "2023");
+ ASSERT_OK(defaults);
+ defaults_ = std::move(defaults).value();
}
const FileDescriptor* ParseSchema(absl::string_view schema) {
@@ -552,9 +572,11 @@
DescriptorPool pool_;
FileDescriptorProto file_proto_;
+ const Descriptor* feature_set_;
+ FeatureSetDefaults defaults_;
};
-TEST_F(FeatureResolverPoolTest, RegisterExtensionNonMessage) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidNonMessage) {
const FileDescriptor* file = ParseSchema(R"schema(
syntax = "proto2";
package test;
@@ -567,16 +589,14 @@
)schema");
ASSERT_NE(file, nullptr);
- auto resolver = FeatureResolver::Create(
- "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
- ASSERT_OK(resolver);
-
- EXPECT_THAT(resolver->RegisterExtensions(*file),
- HasError(AllOf(HasSubstr("test.bar"),
- HasSubstr("is not of message type"))));
+ const FieldDescriptor* ext = file->extension(0);
+ EXPECT_THAT(
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
+ HasError(
+ AllOf(HasSubstr("test.bar"), HasSubstr("is not of message type"))));
}
-TEST_F(FeatureResolverPoolTest, RegisterExtensionRepeated) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidRepeated) {
const FileDescriptor* file = ParseSchema(R"schema(
syntax = "proto2";
package test;
@@ -589,16 +609,13 @@
)schema");
ASSERT_NE(file, nullptr);
- auto resolver = FeatureResolver::Create(
- "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
- ASSERT_OK(resolver);
-
+ const FieldDescriptor* ext = file->extension(0);
EXPECT_THAT(
- resolver->RegisterExtensions(*file),
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
HasError(AllOf(HasSubstr("test.bar"), HasSubstr("repeated extension"))));
}
-TEST_F(FeatureResolverPoolTest, RegisterExtensionWithExtensions) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidWithExtensions) {
const FileDescriptor* file = ParseSchema(R"schema(
syntax = "proto2";
package test;
@@ -619,16 +636,13 @@
)schema");
ASSERT_NE(file, nullptr);
- auto resolver = FeatureResolver::Create(
- "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
- ASSERT_OK(resolver);
-
+ const FieldDescriptor* ext = file->extension(0);
EXPECT_THAT(
- resolver->RegisterExtensions(*file),
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
HasError(AllOf(HasSubstr("test.bar"), HasSubstr("Nested extensions"))));
}
-TEST_F(FeatureResolverPoolTest, RegisterExtensionWithOneof) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidWithOneof) {
const FileDescriptor* file = ParseSchema(R"schema(
syntax = "proto2";
package test;
@@ -652,16 +666,14 @@
)schema");
ASSERT_NE(file, nullptr);
- auto resolver = FeatureResolver::Create(
- "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
- ASSERT_OK(resolver);
-
- EXPECT_THAT(resolver->RegisterExtensions(*file),
- HasError(AllOf(HasSubstr("test.Foo"),
- HasSubstr("oneof feature fields"))));
+ const FieldDescriptor* ext = file->extension(0);
+ EXPECT_THAT(
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
+ HasError(
+ AllOf(HasSubstr("test.Foo"), HasSubstr("oneof feature fields"))));
}
-TEST_F(FeatureResolverPoolTest, RegisterExtensionWithRequired) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidWithRequired) {
const FileDescriptor* file = ParseSchema(R"schema(
syntax = "proto2";
package test;
@@ -679,16 +691,14 @@
)schema");
ASSERT_NE(file, nullptr);
- auto resolver = FeatureResolver::Create(
- "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
- ASSERT_OK(resolver);
-
- EXPECT_THAT(resolver->RegisterExtensions(*file),
- HasError(AllOf(HasSubstr("test.Foo.required_field"),
- HasSubstr("required field"))));
+ const FieldDescriptor* ext = file->extension(0);
+ EXPECT_THAT(
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
+ HasError(AllOf(HasSubstr("test.Foo.required_field"),
+ HasSubstr("required field"))));
}
-TEST_F(FeatureResolverPoolTest, RegisterExtensionWithRepeated) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidWithRepeated) {
const FileDescriptor* file = ParseSchema(R"schema(
syntax = "proto2";
package test;
@@ -706,16 +716,14 @@
)schema");
ASSERT_NE(file, nullptr);
- auto resolver = FeatureResolver::Create(
- "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
- ASSERT_OK(resolver);
-
- EXPECT_THAT(resolver->RegisterExtensions(*file),
- HasError(AllOf(HasSubstr("test.Foo.repeated_field"),
- HasSubstr("repeated field"))));
+ const FieldDescriptor* ext = file->extension(0);
+ EXPECT_THAT(
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
+ HasError(AllOf(HasSubstr("test.Foo.repeated_field"),
+ HasSubstr("repeated field"))));
}
-TEST_F(FeatureResolverPoolTest, RegisterExtensionWithMissingTarget) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidWithMissingTarget) {
const FileDescriptor* file = ParseSchema(R"schema(
syntax = "proto2";
package test;
@@ -732,16 +740,15 @@
)schema");
ASSERT_NE(file, nullptr);
- auto resolver = FeatureResolver::Create(
- "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
- ASSERT_OK(resolver);
-
- EXPECT_THAT(resolver->RegisterExtensions(*file),
- HasError(AllOf(HasSubstr("test.Foo.int_field"),
- HasSubstr("no target specified"))));
+ const FieldDescriptor* ext = file->extension(0);
+ EXPECT_THAT(
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
+ HasError(AllOf(HasSubstr("test.Foo.int_field"),
+ HasSubstr("no target specified"))));
}
-TEST_F(FeatureResolverPoolTest, RegisterExtensionDefaultsMessageParsingError) {
+TEST_F(FeatureResolverPoolTest,
+ CompileDefaultsInvalidDefaultsMessageParsingError) {
const FileDescriptor* file = ParseSchema(R"schema(
syntax = "proto2";
package test;
@@ -762,16 +769,14 @@
)schema");
ASSERT_NE(file, nullptr);
- auto resolver = FeatureResolver::Create(
- "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
- ASSERT_OK(resolver);
+ const FieldDescriptor* ext = file->extension(0);
EXPECT_THAT(
- resolver->RegisterExtensions(*file),
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
HasError(AllOf(HasSubstr("in edition_defaults"), HasSubstr("9987"))));
}
TEST_F(FeatureResolverPoolTest,
- RegisterExtensionDefaultsMessageParsingErrorMerged) {
+ CompileDefaultsInvalidDefaultsMessageParsingErrorMerged) {
const FileDescriptor* file = ParseSchema(R"schema(
syntax = "proto2";
package test;
@@ -794,16 +799,14 @@
)schema");
ASSERT_NE(file, nullptr);
- auto resolver = FeatureResolver::Create(
- "2024", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
- ASSERT_OK(resolver);
+ const FieldDescriptor* ext = file->extension(0);
EXPECT_THAT(
- resolver->RegisterExtensions(*file),
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2025"),
HasError(AllOf(HasSubstr("in edition_defaults"), HasSubstr("9987"))));
}
TEST_F(FeatureResolverPoolTest,
- RegisterExtensionDefaultsMessageParsingErrorSkipped) {
+ CompileDefaultsInvalidDefaultsMessageParsingErrorSkipped) {
const FileDescriptor* file = ParseSchema(R"schema(
syntax = "proto2";
package test;
@@ -826,15 +829,19 @@
)schema");
ASSERT_NE(file, nullptr);
- auto resolver = FeatureResolver::Create(
- "2024", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
+ const FieldDescriptor* ext = file->extension(0);
+ auto defaults =
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2024");
+ ASSERT_OK(defaults);
+
+ auto resolver = FeatureResolver::Create("2023", *defaults);
ASSERT_OK(resolver);
- EXPECT_OK(resolver->RegisterExtensions(*file));
FeatureSet parent, child;
EXPECT_OK(resolver->MergeFeatures(parent, child));
}
-TEST_F(FeatureResolverPoolTest, RegisterExtensionDefaultsScalarParsingError) {
+TEST_F(FeatureResolverPoolTest,
+ CompileDefaultsInvalidDefaultsScalarParsingError) {
const FileDescriptor* file = ParseSchema(R"schema(
syntax = "proto2";
package test;
@@ -852,16 +859,14 @@
)schema");
ASSERT_NE(file, nullptr);
- auto resolver = FeatureResolver::Create(
- "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
- ASSERT_OK(resolver);
+ const FieldDescriptor* ext = file->extension(0);
EXPECT_THAT(
- resolver->RegisterExtensions(*file),
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
HasError(AllOf(HasSubstr("in edition_defaults"), HasSubstr("1.23"))));
}
TEST_F(FeatureResolverPoolTest,
- RegisterExtensionDefaultsScalarParsingErrorSkipped) {
+ CompileDefaultsInvalidDefaultsScalarParsingErrorSkipped) {
const FileDescriptor* file = ParseSchema(R"schema(
syntax = "proto2";
package test;
@@ -880,41 +885,39 @@
)schema");
ASSERT_NE(file, nullptr);
- auto resolver = FeatureResolver::Create(
- "2023", pool_.FindMessageTypeByName("google.protobuf.FeatureSet"), EmptyPool());
+ const FieldDescriptor* ext = file->extension(0);
+ auto defaults =
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023");
+ ASSERT_OK(defaults);
+
+ auto resolver = FeatureResolver::Create("2023", *defaults);
ASSERT_OK(resolver);
- EXPECT_OK(resolver->RegisterExtensions(*file));
FeatureSet parent, child;
EXPECT_OK(resolver->MergeFeatures(parent, child));
}
-TEST_F(FeatureResolverPoolTest, GeneratedPoolInvalidExtension) {
+TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidDefaultsTooEarly) {
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;
+ optional Foo bar = 9999;
+ }
+ message Foo {
+ optional int32 int_field_feature = 12 [
+ targets = TARGET_TYPE_FIELD,
+ edition_defaults = { edition: "2022", value: "1" }
+ ];
}
)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);
+ const FieldDescriptor* ext = file->extension(0);
EXPECT_THAT(
- GetDefaultsImpl("2022", &pool_),
- HasError(AllOf(HasSubstr("No valid default found"), HasSubstr("2022"))));
+ FeatureResolver::CompileDefaults(feature_set_, {ext}, "2023", "2023"),
+ HasError(HasSubstr("No valid default found for edition 2022")));
}
} // namespace
