diff --git a/include/rocksdb/customizable.h b/include/rocksdb/customizable.h index d6bd07087..0a018eace 100644 --- a/include/rocksdb/customizable.h +++ b/include/rocksdb/customizable.h @@ -101,6 +101,20 @@ class Customizable : public Configurable { } } + const void* GetOptionsPtr(const std::string& name) const override { + const void* ptr = Configurable::GetOptionsPtr(name); + if (ptr != nullptr) { + return ptr; + } else { + const auto inner = Inner(); + if (inner != nullptr) { + return inner->GetOptionsPtr(name); + } else { + return nullptr; + } + } + } + // Returns the named instance of the Customizable as a T*, or nullptr if not // found. This method uses IsInstanceOf/Inner to find the appropriate class // instance and then casts it to the expected return type. diff --git a/options/configurable.cc b/options/configurable.cc index a98d2646b..3ce3c0ec8 100644 --- a/options/configurable.cc +++ b/options/configurable.cc @@ -43,21 +43,23 @@ Status Configurable::PrepareOptions(const ConfigOptions& opts) { Status status = Status::OK(); #ifndef ROCKSDB_LITE for (auto opt_iter : options_) { - for (auto map_iter : *(opt_iter.type_map)) { - auto& opt_info = map_iter.second; - if (!opt_info.IsDeprecated() && !opt_info.IsAlias() && - opt_info.IsConfigurable()) { - if (!opt_info.IsEnabled(OptionTypeFlags::kDontPrepare)) { - Configurable* config = - opt_info.AsRawPointer(opt_iter.opt_ptr); - if (config != nullptr) { - status = config->PrepareOptions(opts); - if (!status.ok()) { - return status; + if (opt_iter.type_map != nullptr) { + for (auto map_iter : *(opt_iter.type_map)) { + auto& opt_info = map_iter.second; + if (!opt_info.IsDeprecated() && !opt_info.IsAlias() && + opt_info.IsConfigurable()) { + if (!opt_info.IsEnabled(OptionTypeFlags::kDontPrepare)) { + Configurable* config = + opt_info.AsRawPointer(opt_iter.opt_ptr); + if (config != nullptr) { + status = config->PrepareOptions(opts); + if (!status.ok()) { + return status; + } + } else if (!opt_info.CanBeNull()) { + status = Status::NotFound("Missing configurable object", + map_iter.first); } - } else if (!opt_info.CanBeNull()) { - status = - Status::NotFound("Missing configurable object", map_iter.first); } } } @@ -74,20 +76,22 @@ Status Configurable::ValidateOptions(const DBOptions& db_opts, Status status; #ifndef ROCKSDB_LITE for (auto opt_iter : options_) { - for (auto map_iter : *(opt_iter.type_map)) { - auto& opt_info = map_iter.second; - if (!opt_info.IsDeprecated() && !opt_info.IsAlias()) { - if (opt_info.IsConfigurable()) { - const Configurable* config = - opt_info.AsRawPointer(opt_iter.opt_ptr); - if (config != nullptr) { - status = config->ValidateOptions(db_opts, cf_opts); - } else if (!opt_info.CanBeNull()) { - status = - Status::NotFound("Missing configurable object", map_iter.first); - } - if (!status.ok()) { - return status; + if (opt_iter.type_map != nullptr) { + for (auto map_iter : *(opt_iter.type_map)) { + auto& opt_info = map_iter.second; + if (!opt_info.IsDeprecated() && !opt_info.IsAlias()) { + if (opt_info.IsConfigurable()) { + const Configurable* config = + opt_info.AsRawPointer(opt_iter.opt_ptr); + if (config != nullptr) { + status = config->ValidateOptions(db_opts, cf_opts); + } else if (!opt_info.CanBeNull()) { + status = Status::NotFound("Missing configurable object", + map_iter.first); + } + if (!status.ok()) { + return status; + } } } } @@ -124,11 +128,13 @@ const OptionTypeInfo* ConfigurableHelper::FindOption( const std::vector& options, const std::string& short_name, std::string* opt_name, void** opt_ptr) { for (auto iter : options) { - const auto opt_info = - OptionTypeInfo::Find(short_name, *(iter.type_map), opt_name); - if (opt_info != nullptr) { - *opt_ptr = iter.opt_ptr; - return opt_info; + if (iter.type_map != nullptr) { + const auto opt_info = + OptionTypeInfo::Find(short_name, *(iter.type_map), opt_name); + if (opt_info != nullptr) { + *opt_ptr = iter.opt_ptr; + return opt_info; + } } } return nullptr; @@ -280,12 +286,14 @@ Status ConfigurableHelper::ConfigureOptions( if (!opts_map.empty()) { #ifndef ROCKSDB_LITE for (const auto& iter : configurable.options_) { - s = ConfigureSomeOptions(config_options, configurable, *(iter.type_map), - &remaining, iter.opt_ptr); - if (remaining.empty()) { // Are there more options left? - break; - } else if (!s.ok()) { - break; + if (iter.type_map != nullptr) { + s = ConfigureSomeOptions(config_options, configurable, *(iter.type_map), + &remaining, iter.opt_ptr); + if (remaining.empty()) { // Are there more options left? + break; + } else if (!s.ok()) { + break; + } } } #else @@ -573,36 +581,38 @@ Status ConfigurableHelper::SerializeOptions(const ConfigOptions& config_options, std::string* result) { assert(result); for (auto const& opt_iter : configurable.options_) { - for (const auto& map_iter : *(opt_iter.type_map)) { - const auto& opt_name = map_iter.first; - const auto& opt_info = map_iter.second; - if (opt_info.ShouldSerialize()) { - std::string value; - Status s; - if (!config_options.mutable_options_only) { - s = opt_info.Serialize(config_options, prefix + opt_name, - opt_iter.opt_ptr, &value); - } else if (opt_info.IsMutable()) { - ConfigOptions copy = config_options; - copy.mutable_options_only = false; - s = opt_info.Serialize(copy, prefix + opt_name, opt_iter.opt_ptr, - &value); - } else if (opt_info.IsConfigurable()) { - // If it is a Configurable and we are either printing all of the - // details or not printing only the name, this option should be - // included in the list - if (config_options.IsDetailed() || - !opt_info.IsEnabled(OptionTypeFlags::kStringNameOnly)) { + if (opt_iter.type_map != nullptr) { + for (const auto& map_iter : *(opt_iter.type_map)) { + const auto& opt_name = map_iter.first; + const auto& opt_info = map_iter.second; + if (opt_info.ShouldSerialize()) { + std::string value; + Status s; + if (!config_options.mutable_options_only) { s = opt_info.Serialize(config_options, prefix + opt_name, opt_iter.opt_ptr, &value); + } else if (opt_info.IsMutable()) { + ConfigOptions copy = config_options; + copy.mutable_options_only = false; + s = opt_info.Serialize(copy, prefix + opt_name, opt_iter.opt_ptr, + &value); + } else if (opt_info.IsConfigurable()) { + // If it is a Configurable and we are either printing all of the + // details or not printing only the name, this option should be + // included in the list + if (config_options.IsDetailed() || + !opt_info.IsEnabled(OptionTypeFlags::kStringNameOnly)) { + s = opt_info.Serialize(config_options, prefix + opt_name, + opt_iter.opt_ptr, &value); + } + } + if (!s.ok()) { + return s; + } else if (!value.empty()) { + // = + result->append(prefix + opt_name + "=" + value + + config_options.delimiter); } - } - if (!s.ok()) { - return s; - } else if (!value.empty()) { - // = - result->append(prefix + opt_name + "=" + value + - config_options.delimiter); } } } @@ -629,16 +639,18 @@ Status ConfigurableHelper::ListOptions( const std::string& prefix, std::unordered_set* result) { Status status; for (auto const& opt_iter : configurable.options_) { - for (const auto& map_iter : *(opt_iter.type_map)) { - const auto& opt_name = map_iter.first; - const auto& opt_info = map_iter.second; - // If the option is no longer used in rocksdb and marked as deprecated, - // we skip it in the serialization. - if (!opt_info.IsDeprecated() && !opt_info.IsAlias()) { - if (!config_options.mutable_options_only) { - result->emplace(prefix + opt_name); - } else if (opt_info.IsMutable()) { - result->emplace(prefix + opt_name); + if (opt_iter.type_map != nullptr) { + for (const auto& map_iter : *(opt_iter.type_map)) { + const auto& opt_name = map_iter.first; + const auto& opt_info = map_iter.second; + // If the option is no longer used in rocksdb and marked as deprecated, + // we skip it in the serialization. + if (!opt_info.IsDeprecated() && !opt_info.IsAlias()) { + if (!config_options.mutable_options_only) { + result->emplace(prefix + opt_name); + } else if (opt_info.IsMutable()) { + result->emplace(prefix + opt_name); + } } } } @@ -702,7 +714,7 @@ bool ConfigurableHelper::AreEquivalent(const ConfigOptions& config_options, if (this_offset != that_offset) { if (this_offset == nullptr || that_offset == nullptr) { return false; - } else { + } else if (o.type_map != nullptr) { for (const auto& map_iter : *(o.type_map)) { const auto& opt_info = map_iter.second; if (config_options.IsCheckEnabled(opt_info.GetSanityLevel())) { diff --git a/options/configurable_test.cc b/options/configurable_test.cc index fd412528f..6cc83b847 100644 --- a/options/configurable_test.cc +++ b/options/configurable_test.cc @@ -656,6 +656,30 @@ TEST_F(ConfigurableTest, TestNoCompare) { ASSERT_TRUE(base->AreEquivalent(config_options_, copy.get(), &mismatch)); ASSERT_FALSE(copy->AreEquivalent(config_options_, base.get(), &mismatch)); } + +TEST_F(ConfigurableTest, NullOptionMapTest) { + std::unique_ptr base; + std::unordered_set names; + std::string str; + + base.reset( + SimpleConfigurable::Create("c", TestConfigMode::kDefaultMode, nullptr)); + ASSERT_NOK(base->ConfigureFromString(config_options_, "int=10")); + ASSERT_NOK(base->ConfigureFromString(config_options_, "int=20")); + ASSERT_NOK(base->ConfigureOption(config_options_, "int", "20")); + ASSERT_NOK(base->GetOption(config_options_, "int", &str)); + ASSERT_NE(base->GetOptions("c"), nullptr); + ASSERT_OK(base->GetOptionNames(config_options_, &names)); + ASSERT_EQ(names.size(), 0UL); + ASSERT_OK(base->PrepareOptions(config_options_)); + ASSERT_OK(base->ValidateOptions(DBOptions(), ColumnFamilyOptions())); + std::unique_ptr copy; + copy.reset( + SimpleConfigurable::Create("c", TestConfigMode::kDefaultMode, nullptr)); + ASSERT_OK(base->GetOptionString(config_options_, &str)); + ASSERT_OK(copy->ConfigureFromString(config_options_, str)); + ASSERT_TRUE(base->AreEquivalent(config_options_, copy.get(), &str)); +} #endif static std::unordered_map TestFactories = { diff --git a/options/customizable_test.cc b/options/customizable_test.cc index 776393c8f..98e953e3e 100644 --- a/options/customizable_test.cc +++ b/options/customizable_test.cc @@ -612,11 +612,20 @@ static std::unordered_map inner_option_info = { #endif // ROCKSDB_LITE }; +struct InnerOptions { + static const char* kName() { return "InnerOptions"; } + std::shared_ptr inner; +}; + class InnerCustomizable : public Customizable { public: - explicit InnerCustomizable(const std::shared_ptr& w) - : inner_(w) {} + explicit InnerCustomizable(const std::shared_ptr& w) { + iopts_.inner = w; + RegisterOptions(&iopts_, &inner_option_info); + } static const char* kClassName() { return "Inner"; } + const char* Name() const override { return kClassName(); } + bool IsInstanceOf(const std::string& name) const override { if (name == kClassName()) { return true; @@ -626,26 +635,51 @@ class InnerCustomizable : public Customizable { } protected: - const Customizable* Inner() const override { return inner_.get(); } + const Customizable* Inner() const override { return iopts_.inner.get(); } private: - std::shared_ptr inner_; + InnerOptions iopts_; +}; + +struct WrappedOptions1 { + static const char* kName() { return "WrappedOptions1"; } + int i = 42; }; class WrappedCustomizable1 : public InnerCustomizable { public: explicit WrappedCustomizable1(const std::shared_ptr& w) - : InnerCustomizable(w) {} + : InnerCustomizable(w) { + RegisterOptions(&wopts_, nullptr); + } const char* Name() const override { return kClassName(); } static const char* kClassName() { return "Wrapped1"; } + + private: + WrappedOptions1 wopts_; }; +struct WrappedOptions2 { + static const char* kName() { return "WrappedOptions2"; } + std::string s = "42"; +}; class WrappedCustomizable2 : public InnerCustomizable { public: explicit WrappedCustomizable2(const std::shared_ptr& w) : InnerCustomizable(w) {} + const void* GetOptionsPtr(const std::string& name) const override { + if (name == WrappedOptions2::kName()) { + return &wopts_; + } else { + return InnerCustomizable::GetOptionsPtr(name); + } + } + const char* Name() const override { return kClassName(); } static const char* kClassName() { return "Wrapped2"; } + + private: + WrappedOptions2 wopts_; }; } // namespace @@ -677,6 +711,29 @@ TEST_F(CustomizableTest, WrappedInnerTest) { ASSERT_EQ(wc2->CheckedCast(), ac.get()); } +TEST_F(CustomizableTest, CustomizableInnerTest) { + std::shared_ptr c = + std::make_shared(std::make_shared("a")); + std::shared_ptr wc1 = std::make_shared(c); + std::shared_ptr wc2 = std::make_shared(c); + auto inner = c->GetOptions(); + ASSERT_NE(inner, nullptr); + + auto aopts = c->GetOptions(); + ASSERT_NE(aopts, nullptr); + ASSERT_EQ(aopts, wc1->GetOptions()); + ASSERT_EQ(aopts, wc2->GetOptions()); + auto w1opts = wc1->GetOptions(); + ASSERT_NE(w1opts, nullptr); + ASSERT_EQ(c->GetOptions(), nullptr); + ASSERT_EQ(wc2->GetOptions(), nullptr); + + auto w2opts = wc2->GetOptions(); + ASSERT_NE(w2opts, nullptr); + ASSERT_EQ(c->GetOptions(), nullptr); + ASSERT_EQ(wc1->GetOptions(), nullptr); +} + TEST_F(CustomizableTest, CopyObjectTest) { class CopyCustomizable : public Customizable { public: @@ -714,20 +771,9 @@ TEST_F(CustomizableTest, CopyObjectTest) { } TEST_F(CustomizableTest, TestStringDepth) { - class ShallowCustomizable : public Customizable { - public: - ShallowCustomizable() { - inner_ = std::make_shared("a"); - RegisterOptions("inner", &inner_, &inner_option_info); - } - static const char* kClassName() { return "shallow"; } - const char* Name() const override { return kClassName(); } - - private: - std::shared_ptr inner_; - }; ConfigOptions shallow = config_options_; - std::unique_ptr c(new ShallowCustomizable()); + std::unique_ptr c( + new InnerCustomizable(std::make_shared("a"))); std::string opt_str; shallow.depth = ConfigOptions::Depth::kDepthShallow; ASSERT_OK(c->GetOptionString(shallow, &opt_str));