From 5b2d20286734864662d0594460fc2b24c8d572e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Donoso?= <donosoc@google.com> Date: Thu, 2 May 2019 17:42:52 +0000 Subject: [PATCH] [debugger] Added valid options for SettingSchema This is the first step to implementing enums and enum-lists for the setting system. This is not hooked yet to the SettingStore, so it's possible to add strings even though the schema doesn't allow it. As no setting uses this functionality, that will be added in another CL for making reviewing easier. TEST=Added SettingSchema unit test. Change-Id: I8376d9266c2fb3591576c9b14b5404db140cbd0b --- .../debug/zxdb/client/setting_schema.cc | 65 ++++++++++++++++--- .../debug/zxdb/client/setting_schema.h | 35 +++++++--- .../zxdb/client/setting_schema_unittest.cc | 22 +++++++ .../debug/zxdb/client/setting_store.cc | 10 +-- .../zxdb/client/setting_store_unittest.cc | 6 +- .../debug/zxdb/console/verbs_settings.cc | 2 +- 6 files changed, 116 insertions(+), 24 deletions(-) diff --git a/src/developer/debug/zxdb/client/setting_schema.cc b/src/developer/debug/zxdb/client/setting_schema.cc index a62ee77f3bd..ed852094dec 100644 --- a/src/developer/debug/zxdb/client/setting_schema.cc +++ b/src/developer/debug/zxdb/client/setting_schema.cc @@ -3,10 +3,41 @@ // found in the LICENSE file. #include "src/developer/debug/zxdb/client/setting_schema.h" + +#include <algorithm> +#include <set> +#include <string> + #include "src/lib/fxl/strings/join_strings.h" +#include "src/lib/fxl/strings/trim.h" namespace zxdb { +namespace { + +void TrimAndLowerCase(std::vector<std::string>* strings) { + for (std::string& s : *strings) { + std::transform(s.begin(), s.end(), s.begin(), ::tolower); + s = fxl::TrimString(s, " ").ToString(); + } +} + +bool IsSuperSet(const std::vector<std::string>& super_set, + const std::vector<std::string>& set) { + std::set<std::string> sset; + sset.insert(super_set.begin(), super_set.end()); + for (auto& s : set) { + auto [_, inserted] = sset.insert(s); + // If we insert a new string, means that it's a new one. + if (inserted) + return false; + } + + return true; +} + +} // namespace + void SettingSchema::AddBool(std::string name, std::string description, bool v) { SettingInfo info{name, std::move(description)}; @@ -24,14 +55,27 @@ void SettingSchema::AddString(std::string name, std::string description, AddSetting(std::move(name), {std::move(info), SettingValue(v)}); } -void SettingSchema::AddList(std::string name, std::string description, - std::vector<std::string> v) { +bool SettingSchema::AddList(std::string name, std::string description, + std::vector<std::string> v, + std::vector<std::string> options) { + // Transform everything to lower case. + TrimAndLowerCase(&v); + TrimAndLowerCase(&options); + if (!options.empty() && !IsSuperSet(options, v)) + return false; + SettingInfo info{name, std::move(description)}; - AddSetting(std::move(name), {std::move(info), SettingValue(v)}); + AddSetting(std::move(name), {std::move(info), SettingValue(v)}, + std::move(options)); + + return true; } -void SettingSchema::AddSetting(const std::string& key, Setting item) { - settings_[key] = std::move(item); +void SettingSchema::AddSetting(const std::string& key, Setting setting, + std::vector<std::string> options) { + auto& schema_setting = settings_[key]; + schema_setting.setting = std::move(setting); + schema_setting.options = std::move(options); } bool SettingSchema::HasSetting(const std::string& key) { @@ -45,20 +89,23 @@ Err SettingSchema::ValidateSetting(const std::string& key, return Err("Setting \"%s\" not found in the given context.", key.data()); auto& setting = it->second; - if (setting.value.type != value.type) { + if (setting.setting.value.type != value.type) { return Err( "Setting \"%s\" expects a different type (expected: %s, given: %s).", key.data(), SettingTypeToString(value.type), - SettingTypeToString(setting.value.type)); + SettingTypeToString(setting.setting.value.type)); } return Err(); } -Setting SettingSchema::GetSetting(const std::string& name) const { +const SettingSchema::SchemaSetting& SettingSchema::GetSetting( + const std::string& name) const { + static SchemaSetting null_setting; + const auto& setting = settings_.find(name); if (setting == settings_.end()) - return Setting(); + return null_setting; return setting->second; } diff --git a/src/developer/debug/zxdb/client/setting_schema.h b/src/developer/debug/zxdb/client/setting_schema.h index e0e25dcb9c5..c9594908a20 100644 --- a/src/developer/debug/zxdb/client/setting_schema.h +++ b/src/developer/debug/zxdb/client/setting_schema.h @@ -17,11 +17,23 @@ namespace zxdb { // process, etc.). class SettingSchema : public fxl::RefCountedThreadSafe<SettingSchema> { public: + // The SchemaSetting holds the actual setting (the value that is stored and + // overriden by SettingStore) + some metadata useful for implementing more + // complex settings such as enums, by using the |options| field. + struct SchemaSetting { + Setting setting; + std::vector<std::string> options; // Used only for string lists. + }; + bool HasSetting(const std::string& key); + bool empty() const { return settings_.empty(); } + // Returns a null setting if |key| is not within the schema. - Setting GetSetting(const std::string& name) const; - void AddSetting(const std::string& key, Setting setting); + const SchemaSetting& GetSetting(const std::string& name) const; + const std::map<std::string, SchemaSetting>& settings() const { + return settings_; + } // Create new items for settings that only belong to this schema. // For inter-schema options, the easier way is to create the Setting @@ -30,17 +42,24 @@ class SettingSchema : public fxl::RefCountedThreadSafe<SettingSchema> { void AddInt(std::string name, std::string description, int value = 0); void AddString(std::string name, std::string description, std::string value = {}); - void AddList(std::string name, std::string description, - std::vector<std::string> list = {}); - Err ValidateSetting(const std::string& key, const SettingValue&) const; + // |valid_options| determines which options will be accepted when writing into + // a setting. They will be stored as lowercase and comparison will be in + // lowercase for simplicty. + // Will return false if the given list has a entry that is not within the + // valid options. + bool AddList(std::string name, std::string description, + std::vector<std::string> list = {}, + std::vector<std::string> valid_options = {}); - const std::map<std::string, Setting>& settings() const { return settings_; } + // |options| are used to implement enum-like strings/lists. + void AddSetting(const std::string& key, Setting setting, + std::vector<std::string> options = {}); - bool empty() const { return settings_.empty(); } + Err ValidateSetting(const std::string& key, const SettingValue&) const; private: - std::map<std::string, Setting> settings_; + std::map<std::string, SchemaSetting> settings_; }; } // namespace zxdb diff --git a/src/developer/debug/zxdb/client/setting_schema_unittest.cc b/src/developer/debug/zxdb/client/setting_schema_unittest.cc index bbb0631d3c3..c15a2bfe70c 100644 --- a/src/developer/debug/zxdb/client/setting_schema_unittest.cc +++ b/src/developer/debug/zxdb/client/setting_schema_unittest.cc @@ -113,4 +113,26 @@ TEST(SettingSchema, List) { EXPECT_FALSE(err.has_error()) << err.msg(); } +TEST(SettingSchema, ListWithOptions) { + auto schema = fxl::MakeRefCounted<SettingSchema>(); + std::vector<std::string> value = {"test", "vector"}; + std::vector<std::string> options = {"test", "vector", "another"}; + + ASSERT_TRUE(schema->AddList("valid", "description", value, options)); + { + auto& schema_setting = schema->GetSetting("valid"); + ASSERT_TRUE(schema_setting.setting.value.is_list()); + ASSERT_EQ(schema_setting.options.size(), 3u); + EXPECT_EQ(schema_setting.options[0], options[0]); + EXPECT_EQ(schema_setting.options[1], options[1]); + EXPECT_EQ(schema_setting.options[2], options[2]); + } + + ASSERT_FALSE(schema->AddList("invalid", "description", {"non"}, options)); + { + auto& schema_setting = schema->GetSetting("invalid"); + EXPECT_TRUE(schema_setting.setting.value.is_null()); + } +} + } // namespace zxdb diff --git a/src/developer/debug/zxdb/client/setting_store.cc b/src/developer/debug/zxdb/client/setting_store.cc index e0dbd60dfa0..ab801ac0df4 100644 --- a/src/developer/debug/zxdb/client/setting_store.cc +++ b/src/developer/debug/zxdb/client/setting_store.cc @@ -67,15 +67,17 @@ SettingValue SettingStore::GetValue(const std::string& key) const { Setting SettingStore::GetSetting(const std::string& key) const { // First check if it's in the schema. auto default_setting = schema_->GetSetting(key); - if (default_setting.value.is_null()) + if (default_setting.setting.value.is_null()) { + DEBUG_LOG(Setting) << "Store: " << name_ << ": Key not found: " << key; return Setting(); + } // Check if it already exists. If so, return it. auto it = values_.find(key); if (it != values_.end()) { DEBUG_LOG(Setting) << "Store " << name_ << ": stored value for " << key << ": " << it->second.ToDebugString(); - return {std::move(default_setting.info), it->second}; + return {std::move(default_setting.setting.info), it->second}; } // We check the fallback SettingStore to see if it has the setting. @@ -88,8 +90,8 @@ Setting SettingStore::GetSetting(const std::string& key) const { // No fallback has the schema, we return the default. DEBUG_LOG(Setting) << "Store: " << name_ << ": schema default for " << key - << ": " << default_setting.value.ToDebugString(); - return default_setting; + << ": " << default_setting.setting.value.ToDebugString(); + return default_setting.setting; } bool SettingStore::HasSetting(const std::string& key) const { diff --git a/src/developer/debug/zxdb/client/setting_store_unittest.cc b/src/developer/debug/zxdb/client/setting_store_unittest.cc index 6932be49c4e..61c03660a85 100644 --- a/src/developer/debug/zxdb/client/setting_store_unittest.cc +++ b/src/developer/debug/zxdb/client/setting_store_unittest.cc @@ -23,8 +23,10 @@ fxl::RefPtr<SettingSchema> GetSchema() { schema->AddBool("bool", "bool_option", true); schema->AddInt("int", "int_option", kDefaultInt); schema->AddString("string", "string_option", kDefaultString); - schema->AddList("list", "list_option", DefaultList()); - + if (!schema->AddList("list", "list_option", DefaultList())) { + FXL_NOTREACHED() << "Schema should be valid!"; + return nullptr; + } return schema; } diff --git a/src/developer/debug/zxdb/console/verbs_settings.cc b/src/developer/debug/zxdb/console/verbs_settings.cc index e545abb9c93..84b86c3525b 100644 --- a/src/developer/debug/zxdb/console/verbs_settings.cc +++ b/src/developer/debug/zxdb/console/verbs_settings.cc @@ -539,7 +539,7 @@ Err GetSetContext(const Command& cmd, const std::string& setting_name, out->target = target; out->thread = thread; out->store = store; - out->setting = store->schema()->GetSetting(setting_name); + out->setting = store->schema()->GetSetting(setting_name).setting; return Err(); } -- GitLab