From c726e40fb1f16bbdb4a1973c3918f44d2711ffab Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 31 Mar 2022 08:22:51 +0000 Subject: [PATCH 01/24] Add file change --- common/configdb.cpp | 181 ++++++++++++++++++++++++++++++-------------- configure.ac | 1 + 2 files changed, 125 insertions(+), 57 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index 6d8747bf3..f4b885fa4 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -5,9 +5,133 @@ #include "pubsub.h" #include "converter.h" +#include +#include +#include +#include +#include +#include +#include + using namespace std; using namespace swss; +map table_name_to_data_mapping; + +struct ly_ctx * CreateCtxAndLoadAllModules(char* module_path) +{ + struct ly_ctx *ctx = ly_ctx_new(module_path, LY_CTX_ALLIMPLEMENTED); + DIR *module_dir = opendir(module_path); + if (module_dir) + { + struct dirent *sub_dir; + while ((sub_dir = readdir(module_dir)) != NULL) + { + if (sub_dir->d_type == DT_REG) + { + string file_name(sub_dir->d_name); + printf("file_name: %s\n", file_name.c_str()); + auto pos = file_name.find(".yang"); + string module_name = file_name.substr(0, pos); + printf("%s\n", module_name.c_str()); + + // load module + const struct lys_module *module = ly_ctx_load_module(ctx, module_name.c_str(), ""); + if (module->data) + { + // every module should always has a top level container same with module name + // https://github.com/Azure/SONiC/blob/eeebbf6f8a32e5d989595b0816849e8ef0d15141/doc/mgmt/SONiC_YANG_Model_Guidelines.md + struct lys_node *data = module->data; + printf("root node name: %s\n",data->name); + data = data->child; + string container_name(data->name); + printf("root container name: %s\n",data->name); + table_name_to_data_mapping[container_name] = data; + } + } + } + closedir(module_dir); + } + + return ctx; +} + +void LoadAndAppendDefaultValue(string module_name, map>& data) +{ + CreateCtxAndLoadAllModules("/home/liuh/yang/yang-models/"); + + auto module_root_container = table_name_to_data_mapping.find(module_name); + if (module_root_container == table_name_to_data_mapping.end()) + { + return; + } + + // found yang model for the table + auto next_child = module_root_container->second->child; + while (next_child) + { + printf("Child name: %s\n",next_child->name); + switch (next_child->nodetype) + { + case LYS_LIST: + { + // Mapping tables in Redis are defined using nested 'list'. Use 'sonic-ext:map-list "true";' to indicate that the 'list' is used for mapping table. The outer 'list' is used for multiple instances of mapping. The inner 'list' is used for mapping entries for each outer list instance. + // check if the list if for mapping ConfigDbTable + printf("Ext count: %d\n",next_child->ext_size); + + auto table_column = next_child->child; + while (table_column) + { + struct lys_node_leaf *leafnode = (struct lys_node_leaf*)table_column; + string column_name(table_column->name); + printf(" table column name: %s, default: %s\n",table_column->name, leafnode->dflt); + + // assign default value to every row + if (leafnode->dflt) + { + string default_value(leafnode->dflt); + for (auto row_iterator = data.begin(); row_iterator != data.end(); ++row_iterator) + { + printf("assign default data to: %s\n", row_iterator->first.c_str()); + auto column = row_iterator->second.find(column_name); + if (column == row_iterator->second.end()) + { + printf("assigned default data to %s: %s\n", column_name.c_str(), default_value.c_str()); + // when no data from config DB, assign default value to result + row_iterator->second[column_name] = default_value; + } + } + } + + table_column = table_column->next; + } + } + break; + case LYS_UNKNOWN: + case LYS_CONTAINER: + case LYS_CHOICE: + case LYS_LEAF: + case LYS_LEAFLIST: + case LYS_ANYXML: + case LYS_CASE: + case LYS_NOTIF: + case LYS_RPC: + case LYS_INPUT: + case LYS_OUTPUT: + case LYS_GROUPING: + case LYS_USES: + case LYS_AUGMENT: + case LYS_ACTION: + case LYS_ANYDATA: + case LYS_EXT: + default: + break; + } + + next_child = next_child->next; + } +} + ConfigDBConnector_Native::ConfigDBConnector_Native(bool use_unix_socket_path, const char *netns) : SonicV2Connector_Native(use_unix_socket_path, netns) , m_table_name_separator("|") @@ -326,63 +450,6 @@ void ConfigDBPipeConnector_Native::_delete_table(DBConnector& client, RedisTrans } } -// Write a table entry to config db -// Remove extra fields in the db which are not in the data -// Args: -// table: Table name -// key: Key of table entry, or a tuple of keys if it is a multi-key table -// data: Table row data in a form of dictionary {'column_key': 'value', ...} -// Pass {} as data will delete the entry -void ConfigDBPipeConnector_Native::set_entry(string table, string key, const map& data) -{ - auto& client = get_redis_client(m_db_name); - DBConnector clientPipe(client); - RedisTransactioner pipe(&clientPipe); - - pipe.multi(); - _set_entry(pipe, table, key, data); - pipe.exec(); -} - -// Write a table entry to config db -// Remove extra fields in the db which are not in the data -// Args: -// pipe: Redis DB pipe -// table: Table name -// key: Key of table entry, or a tuple of keys if it is a multi-key table -// data: Table row data in a form of dictionary {'column_key': 'value', ...} -// Pass {} as data will delete the entry -void ConfigDBPipeConnector_Native::_set_entry(RedisTransactioner& pipe, std::string table, std::string key, const std::map& data) -{ - string _hash = to_upper(table) + m_table_name_separator + key; - if (data.empty()) - { - RedisCommand sdel; - sdel.format("DEL %s", _hash.c_str()); - pipe.enqueue(sdel, REDIS_REPLY_INTEGER); - } - else - { - auto original = get_entry(table, key); - - RedisCommand shmset; - shmset.formatHMSET(_hash, data.begin(), data.end()); - pipe.enqueue(shmset, REDIS_REPLY_STATUS); - - for (auto& it: original) - { - auto& k = it.first; - bool found = data.find(k) != data.end(); - if (!found) - { - RedisCommand shdel; - shdel.formatHDEL(_hash, k); - pipe.enqueue(shdel, REDIS_REPLY_INTEGER); - } - } - } -} - // Modify a table entry to config db. // Args: // table: Table name. diff --git a/configure.ac b/configure.ac index ff909b1cc..3c2e05055 100644 --- a/configure.ac +++ b/configure.ac @@ -48,6 +48,7 @@ fi AC_PATH_PROGS(SWIG, [swig4.0 swig3.0 swig]) CFLAGS_COMMON="" +CFLAGS_COMMON+=" -lyang" CFLAGS_COMMON+=" -ansi" CFLAGS_COMMON+=" -fPIC" CFLAGS_COMMON+=" -std=c++11" From a0824646e050c3a97a969d2b79323f5a2ae89675 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 31 Mar 2022 08:26:53 +0000 Subject: [PATCH 02/24] Add file change --- common/configdb.cpp | 57 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/common/configdb.cpp b/common/configdb.cpp index f4b885fa4..61af40f77 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -450,6 +450,63 @@ void ConfigDBPipeConnector_Native::_delete_table(DBConnector& client, RedisTrans } } +// Write a table entry to config db +// Remove extra fields in the db which are not in the data +// Args: +// table: Table name +// key: Key of table entry, or a tuple of keys if it is a multi-key table +// data: Table row data in a form of dictionary {'column_key': 'value', ...} +// Pass {} as data will delete the entry +void ConfigDBPipeConnector_Native::set_entry(string table, string key, const map& data) +{ + auto& client = get_redis_client(m_db_name); + DBConnector clientPipe(client); + RedisTransactioner pipe(&clientPipe); + + pipe.multi(); + _set_entry(pipe, table, key, data); + pipe.exec(); +} + +// Write a table entry to config db +// Remove extra fields in the db which are not in the data +// Args: +// pipe: Redis DB pipe +// table: Table name +// key: Key of table entry, or a tuple of keys if it is a multi-key table +// data: Table row data in a form of dictionary {'column_key': 'value', ...} +// Pass {} as data will delete the entry +void ConfigDBPipeConnector_Native::_set_entry(RedisTransactioner& pipe, std::string table, std::string key, const std::map& data) +{ + string _hash = to_upper(table) + m_table_name_separator + key; + if (data.empty()) + { + RedisCommand sdel; + sdel.format("DEL %s", _hash.c_str()); + pipe.enqueue(sdel, REDIS_REPLY_INTEGER); + } + else + { + auto original = get_entry(table, key); + + RedisCommand shmset; + shmset.formatHMSET(_hash, data.begin(), data.end()); + pipe.enqueue(shmset, REDIS_REPLY_STATUS); + + for (auto& it: original) + { + auto& k = it.first; + bool found = data.find(k) != data.end(); + if (!found) + { + RedisCommand shdel; + shdel.formatHDEL(_hash, k); + pipe.enqueue(shdel, REDIS_REPLY_INTEGER); + } + } + } +} + // Modify a table entry to config db. // Args: // table: Table name. From 63f6225ef9cd534d2cbf43d96988fca0df4517ff Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 31 Mar 2022 08:32:20 +0000 Subject: [PATCH 03/24] Add file change --- common/configdb.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/configdb.cpp b/common/configdb.cpp index 61af40f77..b1ca0ccd0 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -311,6 +311,8 @@ map> ConfigDBConnector_Native::get_table(string tabl row = key.substr(pos + 1); data[row] = entry; } + + LoadAndAppendDefaultValue(table, data); return data; } From 88cf2d198c3e02e887d19501aaaa310a40c77bf8 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Fri, 1 Apr 2022 07:25:04 +0000 Subject: [PATCH 04/24] POC code --- common/configdb.cpp | 57 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index b1ca0ccd0..244b59790 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -17,10 +17,16 @@ using namespace std; using namespace swss; map table_name_to_data_mapping; +bool module_loaded = false; struct ly_ctx * CreateCtxAndLoadAllModules(char* module_path) { struct ly_ctx *ctx = ly_ctx_new(module_path, LY_CTX_ALLIMPLEMENTED); + if (module_loaded) + { + return ctx; + } + DIR *module_dir = opendir(module_path); if (module_dir) { @@ -30,10 +36,10 @@ struct ly_ctx * CreateCtxAndLoadAllModules(char* module_path) if (sub_dir->d_type == DT_REG) { string file_name(sub_dir->d_name); - printf("file_name: %s\n", file_name.c_str()); + //printf("file_name: %s\n", file_name.c_str()); auto pos = file_name.find(".yang"); string module_name = file_name.substr(0, pos); - printf("%s\n", module_name.c_str()); + //printf("%s\n", module_name.c_str()); // load module const struct lys_module *module = ly_ctx_load_module(ctx, module_name.c_str(), ""); @@ -42,10 +48,10 @@ struct ly_ctx * CreateCtxAndLoadAllModules(char* module_path) // every module should always has a top level container same with module name // https://github.com/Azure/SONiC/blob/eeebbf6f8a32e5d989595b0816849e8ef0d15141/doc/mgmt/SONiC_YANG_Model_Guidelines.md struct lys_node *data = module->data; - printf("root node name: %s\n",data->name); + //printf("root node name: %s\n",data->name); data = data->child; string container_name(data->name); - printf("root container name: %s\n",data->name); + //printf("root container name: %s\n",data->name); table_name_to_data_mapping[container_name] = data; } } @@ -53,20 +59,31 @@ struct ly_ctx * CreateCtxAndLoadAllModules(char* module_path) closedir(module_dir); } + module_loaded = true; return ctx; } void LoadAndAppendDefaultValue(string module_name, map>& data) { - CreateCtxAndLoadAllModules("/home/liuh/yang/yang-models/"); + const char* show_default = getenv("CONFIG_DB_DEFAULT_VALUE"); + if (strcmp(show_default, "TRUE") != 0) + { + // enable feature with "export CONFIG_DB_DEFAULT_VALUE=TRUE" + printf("CONFIG_DB_DEFAULT_VALUE need set to TRUE to show default value.\n"); + return; + } + + CreateCtxAndLoadAllModules("/usr/local/yang-models/"); auto module_root_container = table_name_to_data_mapping.find(module_name); if (module_root_container == table_name_to_data_mapping.end()) { + printf("Not found module_name: %s\n",module_name.c_str()); return; } // found yang model for the table + printf("Found module_name: %s\n",module_name.c_str()); auto next_child = module_root_container->second->child; while (next_child) { @@ -77,11 +94,19 @@ void LoadAndAppendDefaultValue(string module_name, mapext_size); + //printf("Ext count: %d\n",next_child->ext_size); auto table_column = next_child->child; while (table_column) { + if (table_column->nodetype != LYS_LEAF) + { + // TODO: handle uses case here, for example "uses bgpcmn:sonic-bgp-cmn-neigh" + printf("Ignore none leaf child node: %s\n",table_column->name); + table_column = table_column->next; + continue; + } + struct lys_node_leaf *leafnode = (struct lys_node_leaf*)table_column; string column_name(table_column->name); printf(" table column name: %s, default: %s\n",table_column->name, leafnode->dflt); @@ -92,14 +117,17 @@ void LoadAndAppendDefaultValue(string module_name, mapdflt); for (auto row_iterator = data.begin(); row_iterator != data.end(); ++row_iterator) { - printf("assign default data to: %s\n", row_iterator->first.c_str()); auto column = row_iterator->second.find(column_name); if (column == row_iterator->second.end()) { - printf("assigned default data to %s: %s\n", column_name.c_str(), default_value.c_str()); + printf("assigned default data to row: %s column: %s value: %s\n", row_iterator->first.c_str(), column_name.c_str(), default_value.c_str()); // when no data from config DB, assign default value to result row_iterator->second[column_name] = default_value; } + else + { + printf("Row: %s already has data, column: %s value: %s\n", row_iterator->first.c_str(), column_name.c_str(), row_iterator->second[column_name].c_str()); + } } } @@ -387,6 +415,13 @@ map>> ConfigDBConnector_Native::get_conf data[table_name][row] = entry; } } + + // merge default value to config + for (auto& table : data) + { + LoadAndAppendDefaultValue(table.first, table.second); + } + return data; } @@ -633,5 +668,11 @@ map>> ConfigDBPipeConnector_Native::get_ cur = _get_config(client, pipe, data, cur); } + // merge default value to config + for (auto& table : data) + { + LoadAndAppendDefaultValue(table.first, table.second); + } + return data; } From 84aa16ff04511787fad675e8a5fca1b397cde460 Mon Sep 17 00:00:00 2001 From: liuh-80 <58683130+liuh-80@users.noreply.github.com> Date: Tue, 12 Apr 2022 17:50:47 +0800 Subject: [PATCH 05/24] Add default value provide class --- common/defaultvalueprovider.cpp | 319 ++++++++++++++++++++++++++++++++ common/defaultvalueprovider.h | 102 ++++++++++ 2 files changed, 421 insertions(+) create mode 100644 common/defaultvalueprovider.cpp create mode 100644 common/defaultvalueprovider.h diff --git a/common/defaultvalueprovider.cpp b/common/defaultvalueprovider.cpp new file mode 100644 index 000000000..19edd8e57 --- /dev/null +++ b/common/defaultvalueprovider.cpp @@ -0,0 +1,319 @@ + +#include +#include + +#include + +#include "defaultvalueprovider.h" +#include "logger.h" + +using namespace std; +using namespace swss; + +void ThrowRunTimeError(string &message) +{ + SWSS_LOG_ERROR(message); + throw std::runtime_error(message); +} + +TableInfoBase::TableInfoBase() +{ + // C++ need this empty ctor +} + +bool TableInfoBase::TryAppendDefaultValues(string key, map& target_values) +{ + map *field_mapping_ptr; + if (!FindFieldMappingByKey(key, &field_mapping_ptr)) { + return false; + } + + for (auto &default_value : *field_mapping_ptr) + { + auto field_data = target_values.find(default_value.first); + if (field_data != target_values.end()) + { + // ignore when a field already has value in config DB + continue; + } + + target_values[default_value.first] = default_value.second; + } + + return true; +} + +TableInfoDict::TableInfoDict(KeyInfoToDefaultValueInfoMapping &field_info_mapping) +{ + for (auto& field_mapping : field_info_mapping) + { + string fieldName = std::get<0>(field_mapping.first); + this->defaultValueMapping[fieldName] = field_mapping.second; + } +} + +bool TableInfoDict::FindFieldMappingByKey(string key, map ** founded_mapping_ptr) +{ + auto key_result = this->defaultValueMapping.find(key); + *founded_mapping_ptr = &(key_result->second); + return key_result == this->defaultValueMapping.end(); +} + +TableInfoSingleList::TableInfoSingleList(KeyInfoToDefaultValueInfoMapping &field_info_mapping) +{ + this->defaultValueMapping = field_info_mapping.begin()->second; +} + +bool TableInfoSingleList::FindFieldMappingByKey(string key, map ** founded_mapping_ptr) +{ + *founded_mapping_ptr = &(this->defaultValueMapping); + return true; +} + +TableInfoMultipleList::TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &field_info_mapping) +{ + for (auto& field_mapping : field_info_mapping) + { + unsigned int fieldCount = std::get<1>(field_mapping.first); + this->defaultValueMapping[fieldCount] = field_mapping.second; + } +} + +bool TableInfoMultipleList::FindFieldMappingByKey(string key, map ** founded_mapping_ptr) +{ + unsigned int key_field_count = std::count(key.begin(), key.end(), '|') + 1; + auto key_result = this->defaultValueMapping.find(key_field_count); + *founded_mapping_ptr = &(key_result->second); + return key_result == this->defaultValueMapping.end(); +} + +DefaultValueProvider& DefaultValueProvider::Instance() +{ + static DefaultValueProvider instance; + return instance; +} + +shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string table) +{ + auto find_result = this->default_value_mapping.find(table); + if (find_result == this->default_value_mapping.end()) + { + SWSS_LOG_WARN("Not found default value info for table: %s\n", table.c_str()); + return shared_ptr(nullptr); + } + + return find_result->second; +} + +void DefaultValueProvider::AppendDefaultValues(string table, map >& values) +{ + auto default_value_info = this->FindDefaultValueInfo(table); + if (default_value_info.get() == nullptr) + { + return; + } + + for (auto& row : data) + { + default_value_info->TryAppendDefaultValues(row.first, row.second); + } +} + +void DefaultValueProvider::AppendDefaultValues(string table, string key, map& values) +{ + auto default_value_info = this->FindDefaultValueInfo(table); + if (default_value_info.get() == nullptr) + { + return; + } + + default_value_info->TryAppendDefaultValues(key, values); +} + +DefaultValueProvider::~DefaultValueProvider() +{ + if (this->context) + { + ly_ctx_destroy(this->context); + } +} + +void DefaultValueProvider::Initialize(char* module_path = DEFAULT_YANG_MODULE_PATH) +{ + if (this->context) + { + ThrowRunTimeError("DefaultValueProvider already initialized"); + } + + this->context = ly_ctx_new(module_path, LY_CTX_ALLIMPLEMENTED); + DIR *module_dir = opendir(module_path); + if (!module_dir) + { + ThrowRunTimeError("Open Yang model path " + string(module_path) + " failed"); + } + + struct dirent *sub_dir; + while ((sub_dir = readdir(module_dir)) != NULL) + { + if (sub_dir->d_type == DT_REG) + { + SWSS_LOG_DEBUG("file_name: %s\n", sub_dir->d_name); + string file_name(sub_dir->d_name); + unsigned int pos = file_name.find(".yang"); + string module_name = file_name.substr(0, pos); + + const struct lys_module *module = ly_ctx_load_module( + this->context, + module_name.c_str(), + EMPTY_STR); // Use EMPTY_STR to revision to load the latest revision + if (module->data == NULL) + { + // Every yang file should contains yang model + ThrowRunTimeError("Yang file " + file_name + " does not contains any model.\n"); + } + + struct lys_node *top_level_node = module->data; + while (top_level_node) + { + if (top_level_node->nodetype != LYS_CONTAINER) + { + // Config DB table schema is defined by top level container + top_level_node = top_level_node->next; + continue; + } + + SWSS_LOG_DEBUG("top level container: %s\n",top_level_node->name); + auto container = top_level_node->child; + while (container) + { + SWSS_LOG_DEBUG("container name: %s\n",container->name); + + AppendTableInfoToMapping(container); + container = container->next; + } + + top_level_node = top_level_node->next; + } + } + } + closedir(module_dir); +} + +std::shared_ptr DefaultValueProvider::GetKeyInfo(struct lys_node* table_child_node) +{ + unsigned int key_field_count = 0; + string key_value = ""; + if (next_child->nodetype == LYS_LIST) + { + SWSS_LOG_DEBUG("child list: %s\n",next_child->name); + child_list_count++; + + // when a top level container contains list, the key defined by the 'keys' field. + struct lys_node_list *list_node = (struct lys_node_list*)next_child; + string key(list_node->keys_str); + key_field_count = std::count(key.begin(), key.end(), '|') + 1; + } + else if (next_child->nodetype == LYS_CONTAINER) + { + SWSS_LOG_DEBUG("child container name: %s\n",next_child->name); + + // when a top level container not contains any list, the key is child container name + key_value = string(next_child->name); + } + else + { + return make_shared(nullptr) + } + + return make_shared(key_value, key_field_count) +} + +std::shared_ptr DefaultValueProvider::GetDefaultValueInfo(struct lys_node* table_child_node) +{ + auto field = table_child_node->child; + auto field_mapping = make_shared(); + while (field) + { + if (field->nodetype == LYS_LEAF) + { + struct lys_node_leaf *leaf_node = (struct lys_node_leaf*)field; + if (leaf_node->dflt) + { + SWSS_LOG_DEBUG("field: %s, default: %s\n",leaf_node->name, leaf_node->dflt); + field_mapping->[string(leaf_node->name)] = string(leaf_node->dflt); + } + } + else if (field->nodetype == LYS_CHOICE) + { + struct lys_node_choice *choice_node = (struct lys_node_choice *)field; + if (choice_node->dflt) + { + // TODO: convert choice default value to string + SWSS_LOG_DEBUG("choice field: %s, default: TBD\n",choice_node->name); + } + } + else if (field->nodetype == LYS_LEAFLIST) + { + struct lys_node_leaflist *list_node = (struct lys_node_leaflist *)field; + if (list_node->dflt) + { + // TODO: convert list default value to json string + SWSS_LOG_DEBUG("list field: %s, default: TBD\n",list_node->name); + } + } + + field = field->next; + } + + return field_mapping; +} + +unsigned int DefaultValueProvider::BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping &field_info_mapping) +{ + unsigned child_list_count = 0; + + auto next_child = table->child; + while (next_child) + { + // get key from schema + shared_ptr keyInfo = this->GetKeyInfo(next_child); + if (keyInfo.get() == nullptr) + { + next_child = next_child->next; + continue; + } + + // get field name to default value mappings from schema + field_info_mapping[*keyInfo] = this->GetDefaultValueInfo(next_child); + + next_child = next_child->next; + } + + return child_list_count; +} + +// Load default value info from yang model and append to default value mapping +void DefaultValueProvider::AppendTableInfoToMapping(struct lys_node* table) +{ + SWSS_LOG_DEBUG("table name: %s\n",table->name); + KeyInfoToDefaultValueInfoMapping field_info_mapping; + unsigned list_count = this->BuildFieldMappingList(table, field_info_mapping); + + // create container data by list count + TableInfoBase* table_info_ptr = nullptr; + if (list_count == 0) + { + table_info_ptr = new TableInfoDict(field_info_mapping); + } + else if (list_count == 1) + { + table_info_ptr = new TableInfoSingleList(field_info_mapping); + } + else + { + table_info_ptr = new TableInfoMultipleList(field_info_mapping); + } + + string table_name(table->name); + default_value_mapping[table_name] = shared_ptr(table_info_ptr); +} \ No newline at end of file diff --git a/common/defaultvalueprovider.h b/common/defaultvalueprovider.h new file mode 100644 index 000000000..68a9e3b35 --- /dev/null +++ b/common/defaultvalueprovider.h @@ -0,0 +1,102 @@ +#ifndef __DEFAULTVALUEPROVIDER__ +#define __DEFAULTVALUEPROVIDER__ + +#define DEFAULT_YANG_MODULE_PATH "/usr/local/yang-models" +#define EMPTY_STR "" + + +// Key information +typedef std::tuple KeyInfo; + +// Key information +typedef std::map DefaultValueInfo; + +// Key info to default value info mapping +typedef std::map KeyInfoToDefaultValueInfoMapping; + +namespace swss { + +class TableInfoBase; +struct ly_ctx; + +class TableInfoBase +{ +public: + TableInfoBase(); + + bool TryAppendDefaultValues(std::string key, std::map& target_values); + +protected: + virtual bool FindFieldMappingByKey(std::string key, std::map ** founded_mapping_ptr) = 0; +}; + +class TableInfoDict : public TableInfoBase +{ +public: + TableInfoDict(KeyInfoToDefaultValueInfoMapping &field_info_mapping); + +private: + // Mapping: key value -> field -> default + std::map > defaultValueMapping; + + bool FindFieldMappingByKey(std::string key, std::map ** founded_mapping_ptr); +}; + +class TableInfoSingleList : public TableInfoBase +{ +public: + TableInfoSingleList(KeyInfoToDefaultValueInfoMapping &field_info_mapping); + +private: + // Mapping: field -> default + std::map defaultValueMapping; + + bool FindFieldMappingByKey(std::string key, std::map ** founded_mapping_ptr); +}; + +struct TableInfoMultipleList : public TableInfoBase +{ +public: + TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &field_info_mapping); + +private: + // Mapping: key field count -> field -> default + std::map > defaultValueMapping; + + bool FindFieldMappingByKey(std::string key, std::map ** founded_mapping_ptr); +}; + +class DefaultValueProvider +{ +public: + DefaultValueProvider& Instance(); + + void AppendDefaultValues(std::string table, std::map >& values); + + void AppendDefaultValues(std::string table, std::string key, std::map& values); + +private: + DefaultValueProvider() {}; + ~DefaultValueProvider(); + + // libyang context + struct ly_ctx *context = NULL; + + // The table name to table default value info mapping + std::map > default_value_mapping; + + void Initialize(char* module_path = DEFAULT_YANG_MODULE_PATH); + + // Load default value info from yang model and append to default value mapping + void AppendTableInfoToMapping(struct lys_node* table); + + std::shared_ptr FindDefaultValueInfo(std::string table); + + unsigned int BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping& field_mapping_list); + + std::shared_ptr GetKeyInfo(struct lys_node* table_child_node); + std::shared_ptr GetDefaultValueInfo(struct lys_node* table_child_node); +} + +} +#endif \ No newline at end of file From 1879bb2cc46c38c80a0a5f560b2e4cf5583b9bba Mon Sep 17 00:00:00 2001 From: liuh-80 <58683130+liuh-80@users.noreply.github.com> Date: Wed, 13 Apr 2022 14:55:27 +0800 Subject: [PATCH 06/24] Fix code build issue. --- common/Makefile.am | 1 + common/defaultvalueprovider.cpp | 69 +++++++++++++++++++-------------- common/defaultvalueprovider.h | 34 ++++++++-------- 3 files changed, 58 insertions(+), 46 deletions(-) diff --git a/common/Makefile.am b/common/Makefile.am index 4c400556d..78c90e182 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -33,6 +33,7 @@ libswsscommon_la_SOURCES = \ configdb.cpp \ dbconnector.cpp \ dbinterface.cpp \ + defaultvalueprovider.cpp \ sonicv2connector.cpp \ table.cpp \ json.cpp \ diff --git a/common/defaultvalueprovider.cpp b/common/defaultvalueprovider.cpp index 19edd8e57..9991e6d22 100644 --- a/common/defaultvalueprovider.cpp +++ b/common/defaultvalueprovider.cpp @@ -1,3 +1,8 @@ +#include +#include +#include +#include +#include #include #include @@ -10,7 +15,7 @@ using namespace std; using namespace swss; -void ThrowRunTimeError(string &message) +void ThrowRunTimeError(string message) { SWSS_LOG_ERROR(message); throw std::runtime_error(message); @@ -23,7 +28,7 @@ TableInfoBase::TableInfoBase() bool TableInfoBase::TryAppendDefaultValues(string key, map& target_values) { - map *field_mapping_ptr; + FieldDefaultValueMapping *field_mapping_ptr; if (!FindFieldMappingByKey(key, &field_mapping_ptr)) { return false; } @@ -52,10 +57,10 @@ TableInfoDict::TableInfoDict(KeyInfoToDefaultValueInfoMapping &field_info_mappin } } -bool TableInfoDict::FindFieldMappingByKey(string key, map ** founded_mapping_ptr) +bool TableInfoDict::FindFieldMappingByKey(string key, FieldDefaultValueMapping ** founded_mapping_ptr) { auto key_result = this->defaultValueMapping.find(key); - *founded_mapping_ptr = &(key_result->second); + *founded_mapping_ptr = key_result->second.get(); return key_result == this->defaultValueMapping.end(); } @@ -64,9 +69,9 @@ TableInfoSingleList::TableInfoSingleList(KeyInfoToDefaultValueInfoMapping &field this->defaultValueMapping = field_info_mapping.begin()->second; } -bool TableInfoSingleList::FindFieldMappingByKey(string key, map ** founded_mapping_ptr) +bool TableInfoSingleList::FindFieldMappingByKey(string key, FieldDefaultValueMapping ** founded_mapping_ptr) { - *founded_mapping_ptr = &(this->defaultValueMapping); + *founded_mapping_ptr = this->defaultValueMapping.get(); return true; } @@ -79,11 +84,11 @@ TableInfoMultipleList::TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &f } } -bool TableInfoMultipleList::FindFieldMappingByKey(string key, map ** founded_mapping_ptr) +bool TableInfoMultipleList::FindFieldMappingByKey(string key, FieldDefaultValueMapping ** founded_mapping_ptr) { unsigned int key_field_count = std::count(key.begin(), key.end(), '|') + 1; auto key_result = this->defaultValueMapping.find(key_field_count); - *founded_mapping_ptr = &(key_result->second); + *founded_mapping_ptr = key_result->second.get(); return key_result == this->defaultValueMapping.end(); } @@ -99,7 +104,7 @@ shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string if (find_result == this->default_value_mapping.end()) { SWSS_LOG_WARN("Not found default value info for table: %s\n", table.c_str()); - return shared_ptr(nullptr); + return nullptr; } return find_result->second; @@ -108,12 +113,12 @@ shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string void DefaultValueProvider::AppendDefaultValues(string table, map >& values) { auto default_value_info = this->FindDefaultValueInfo(table); - if (default_value_info.get() == nullptr) + if (default_value_info == nullptr) { return; } - for (auto& row : data) + for (auto& row : values) { default_value_info->TryAppendDefaultValues(row.first, row.second); } @@ -122,7 +127,7 @@ void DefaultValueProvider::AppendDefaultValues(string table, map& values) { auto default_value_info = this->FindDefaultValueInfo(table); - if (default_value_info.get() == nullptr) + if (default_value_info == nullptr) { return; } @@ -134,11 +139,12 @@ DefaultValueProvider::~DefaultValueProvider() { if (this->context) { - ly_ctx_destroy(this->context); + // set private_destructor to NULL because no any private data + ly_ctx_destroy(this->context, NULL); } } -void DefaultValueProvider::Initialize(char* module_path = DEFAULT_YANG_MODULE_PATH) +void DefaultValueProvider::Initialize(char* module_path) { if (this->context) { @@ -169,7 +175,8 @@ void DefaultValueProvider::Initialize(char* module_path = DEFAULT_YANG_MODULE_PA if (module->data == NULL) { // Every yang file should contains yang model - ThrowRunTimeError("Yang file " + file_name + " does not contains any model.\n"); + SWSS_LOG_WARN("Yang file " + file_name + " does not contains any model.\n"); + continue; } struct lys_node *top_level_node = module->data; @@ -203,35 +210,34 @@ std::shared_ptr DefaultValueProvider::GetKeyInfo(struct lys_node* table { unsigned int key_field_count = 0; string key_value = ""; - if (next_child->nodetype == LYS_LIST) + if (table_child_node->nodetype == LYS_LIST) { - SWSS_LOG_DEBUG("child list: %s\n",next_child->name); - child_list_count++; + SWSS_LOG_DEBUG("child list: %s\n",table_child_node->name); // when a top level container contains list, the key defined by the 'keys' field. - struct lys_node_list *list_node = (struct lys_node_list*)next_child; + struct lys_node_list *list_node = (struct lys_node_list*)table_child_node; string key(list_node->keys_str); key_field_count = std::count(key.begin(), key.end(), '|') + 1; } - else if (next_child->nodetype == LYS_CONTAINER) + else if (table_child_node->nodetype == LYS_CONTAINER) { - SWSS_LOG_DEBUG("child container name: %s\n",next_child->name); + SWSS_LOG_DEBUG("child container name: %s\n",table_child_node->name); // when a top level container not contains any list, the key is child container name - key_value = string(next_child->name); + key_value = string(table_child_node->name); } else { - return make_shared(nullptr) + return nullptr; } - return make_shared(key_value, key_field_count) + return make_shared(key_value, key_field_count); } -std::shared_ptr DefaultValueProvider::GetDefaultValueInfo(struct lys_node* table_child_node) +FieldDefaultValueMappingPtr DefaultValueProvider::GetDefaultValueInfo(struct lys_node* table_child_node) { auto field = table_child_node->child; - auto field_mapping = make_shared(); + auto field_mapping = make_shared(); while (field) { if (field->nodetype == LYS_LEAF) @@ -240,7 +246,7 @@ std::shared_ptr DefaultValueProvider::GetDefaultValueInfo(stru if (leaf_node->dflt) { SWSS_LOG_DEBUG("field: %s, default: %s\n",leaf_node->name, leaf_node->dflt); - field_mapping->[string(leaf_node->name)] = string(leaf_node->dflt); + (*field_mapping)[string(leaf_node->name)] = string(leaf_node->dflt); } } else if (field->nodetype == LYS_CHOICE) @@ -276,12 +282,17 @@ unsigned int DefaultValueProvider::BuildFieldMappingList(struct lys_node* table, while (next_child) { // get key from schema - shared_ptr keyInfo = this->GetKeyInfo(next_child); - if (keyInfo.get() == nullptr) + auto keyInfo = this->GetKeyInfo(next_child); + if (keyInfo == nullptr) { next_child = next_child->next; continue; } + else if (std::get<1>(*keyInfo) != 0) + { + // when key field count not 0, it's a list node. + child_list_count++; + } // get field name to default value mappings from schema field_info_mapping[*keyInfo] = this->GetDefaultValueInfo(next_child); diff --git a/common/defaultvalueprovider.h b/common/defaultvalueprovider.h index 68a9e3b35..949363939 100644 --- a/common/defaultvalueprovider.h +++ b/common/defaultvalueprovider.h @@ -4,21 +4,20 @@ #define DEFAULT_YANG_MODULE_PATH "/usr/local/yang-models" #define EMPTY_STR "" +struct ly_ctx; // Key information typedef std::tuple KeyInfo; -// Key information -typedef std::map DefaultValueInfo; +// Field name to default value mapping +typedef std::map FieldDefaultValueMapping; +typedef std::shared_ptr FieldDefaultValueMappingPtr; // Key info to default value info mapping -typedef std::map KeyInfoToDefaultValueInfoMapping; +typedef std::map KeyInfoToDefaultValueInfoMapping; namespace swss { -class TableInfoBase; -struct ly_ctx; - class TableInfoBase { public: @@ -27,7 +26,7 @@ class TableInfoBase bool TryAppendDefaultValues(std::string key, std::map& target_values); protected: - virtual bool FindFieldMappingByKey(std::string key, std::map ** founded_mapping_ptr) = 0; + virtual bool FindFieldMappingByKey(std::string key, FieldDefaultValueMapping ** founded_mapping_ptr) = 0; }; class TableInfoDict : public TableInfoBase @@ -37,9 +36,9 @@ class TableInfoDict : public TableInfoBase private: // Mapping: key value -> field -> default - std::map > defaultValueMapping; + std::map defaultValueMapping; - bool FindFieldMappingByKey(std::string key, std::map ** founded_mapping_ptr); + bool FindFieldMappingByKey(std::string key, FieldDefaultValueMapping ** founded_mapping_ptr); }; class TableInfoSingleList : public TableInfoBase @@ -49,9 +48,9 @@ class TableInfoSingleList : public TableInfoBase private: // Mapping: field -> default - std::map defaultValueMapping; + FieldDefaultValueMappingPtr defaultValueMapping; - bool FindFieldMappingByKey(std::string key, std::map ** founded_mapping_ptr); + bool FindFieldMappingByKey(std::string key, FieldDefaultValueMapping ** founded_mapping_ptr); }; struct TableInfoMultipleList : public TableInfoBase @@ -61,7 +60,7 @@ struct TableInfoMultipleList : public TableInfoBase private: // Mapping: key field count -> field -> default - std::map > defaultValueMapping; + std::map defaultValueMapping; bool FindFieldMappingByKey(std::string key, std::map ** founded_mapping_ptr); }; @@ -69,7 +68,9 @@ struct TableInfoMultipleList : public TableInfoBase class DefaultValueProvider { public: - DefaultValueProvider& Instance(); + static DefaultValueProvider& Instance(); + + void Initialize(char* module_path); void AppendDefaultValues(std::string table, std::map >& values); @@ -80,12 +81,11 @@ class DefaultValueProvider ~DefaultValueProvider(); // libyang context - struct ly_ctx *context = NULL; + struct ly_ctx *context = nullptr; // The table name to table default value info mapping std::map > default_value_mapping; - void Initialize(char* module_path = DEFAULT_YANG_MODULE_PATH); // Load default value info from yang model and append to default value mapping void AppendTableInfoToMapping(struct lys_node* table); @@ -95,8 +95,8 @@ class DefaultValueProvider unsigned int BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping& field_mapping_list); std::shared_ptr GetKeyInfo(struct lys_node* table_child_node); - std::shared_ptr GetDefaultValueInfo(struct lys_node* table_child_node); -} + FieldDefaultValueMappingPtr GetDefaultValueInfo(struct lys_node* table_child_node); +}; } #endif \ No newline at end of file From 8b9c2885e63aab4d906c6588dd84ac380a39ad14 Mon Sep 17 00:00:00 2001 From: liuh-80 <58683130+liuh-80@users.noreply.github.com> Date: Wed, 13 Apr 2022 15:30:23 +0800 Subject: [PATCH 07/24] Improve code. --- common/configdb.cpp | 162 +------------------------------- common/defaultvalueprovider.cpp | 10 ++ common/defaultvalueprovider.h | 3 +- 3 files changed, 16 insertions(+), 159 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index 244b59790..fa3913ade 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -4,162 +4,11 @@ #include "configdb.h" #include "pubsub.h" #include "converter.h" - -#include -#include -#include -#include -#include -#include -#include +#include "defaultvalueprovider.h" using namespace std; using namespace swss; -map table_name_to_data_mapping; -bool module_loaded = false; - -struct ly_ctx * CreateCtxAndLoadAllModules(char* module_path) -{ - struct ly_ctx *ctx = ly_ctx_new(module_path, LY_CTX_ALLIMPLEMENTED); - if (module_loaded) - { - return ctx; - } - - DIR *module_dir = opendir(module_path); - if (module_dir) - { - struct dirent *sub_dir; - while ((sub_dir = readdir(module_dir)) != NULL) - { - if (sub_dir->d_type == DT_REG) - { - string file_name(sub_dir->d_name); - //printf("file_name: %s\n", file_name.c_str()); - auto pos = file_name.find(".yang"); - string module_name = file_name.substr(0, pos); - //printf("%s\n", module_name.c_str()); - - // load module - const struct lys_module *module = ly_ctx_load_module(ctx, module_name.c_str(), ""); - if (module->data) - { - // every module should always has a top level container same with module name - // https://github.com/Azure/SONiC/blob/eeebbf6f8a32e5d989595b0816849e8ef0d15141/doc/mgmt/SONiC_YANG_Model_Guidelines.md - struct lys_node *data = module->data; - //printf("root node name: %s\n",data->name); - data = data->child; - string container_name(data->name); - //printf("root container name: %s\n",data->name); - table_name_to_data_mapping[container_name] = data; - } - } - } - closedir(module_dir); - } - - module_loaded = true; - return ctx; -} - -void LoadAndAppendDefaultValue(string module_name, map>& data) -{ - const char* show_default = getenv("CONFIG_DB_DEFAULT_VALUE"); - if (strcmp(show_default, "TRUE") != 0) - { - // enable feature with "export CONFIG_DB_DEFAULT_VALUE=TRUE" - printf("CONFIG_DB_DEFAULT_VALUE need set to TRUE to show default value.\n"); - return; - } - - CreateCtxAndLoadAllModules("/usr/local/yang-models/"); - - auto module_root_container = table_name_to_data_mapping.find(module_name); - if (module_root_container == table_name_to_data_mapping.end()) - { - printf("Not found module_name: %s\n",module_name.c_str()); - return; - } - - // found yang model for the table - printf("Found module_name: %s\n",module_name.c_str()); - auto next_child = module_root_container->second->child; - while (next_child) - { - printf("Child name: %s\n",next_child->name); - switch (next_child->nodetype) - { - case LYS_LIST: - { - // Mapping tables in Redis are defined using nested 'list'. Use 'sonic-ext:map-list "true";' to indicate that the 'list' is used for mapping table. The outer 'list' is used for multiple instances of mapping. The inner 'list' is used for mapping entries for each outer list instance. - // check if the list if for mapping ConfigDbTable - //printf("Ext count: %d\n",next_child->ext_size); - - auto table_column = next_child->child; - while (table_column) - { - if (table_column->nodetype != LYS_LEAF) - { - // TODO: handle uses case here, for example "uses bgpcmn:sonic-bgp-cmn-neigh" - printf("Ignore none leaf child node: %s\n",table_column->name); - table_column = table_column->next; - continue; - } - - struct lys_node_leaf *leafnode = (struct lys_node_leaf*)table_column; - string column_name(table_column->name); - printf(" table column name: %s, default: %s\n",table_column->name, leafnode->dflt); - - // assign default value to every row - if (leafnode->dflt) - { - string default_value(leafnode->dflt); - for (auto row_iterator = data.begin(); row_iterator != data.end(); ++row_iterator) - { - auto column = row_iterator->second.find(column_name); - if (column == row_iterator->second.end()) - { - printf("assigned default data to row: %s column: %s value: %s\n", row_iterator->first.c_str(), column_name.c_str(), default_value.c_str()); - // when no data from config DB, assign default value to result - row_iterator->second[column_name] = default_value; - } - else - { - printf("Row: %s already has data, column: %s value: %s\n", row_iterator->first.c_str(), column_name.c_str(), row_iterator->second[column_name].c_str()); - } - } - } - - table_column = table_column->next; - } - } - break; - case LYS_UNKNOWN: - case LYS_CONTAINER: - case LYS_CHOICE: - case LYS_LEAF: - case LYS_LEAFLIST: - case LYS_ANYXML: - case LYS_CASE: - case LYS_NOTIF: - case LYS_RPC: - case LYS_INPUT: - case LYS_OUTPUT: - case LYS_GROUPING: - case LYS_USES: - case LYS_AUGMENT: - case LYS_ACTION: - case LYS_ANYDATA: - case LYS_EXT: - default: - break; - } - - next_child = next_child->next; - } -} - ConfigDBConnector_Native::ConfigDBConnector_Native(bool use_unix_socket_path, const char *netns) : SonicV2Connector_Native(use_unix_socket_path, netns) , m_table_name_separator("|") @@ -339,8 +188,8 @@ map> ConfigDBConnector_Native::get_table(string tabl row = key.substr(pos + 1); data[row] = entry; } - - LoadAndAppendDefaultValue(table, data); + + DefaultValueProvider::Instance().AppendDefaultValues(table, data); return data; } @@ -419,9 +268,8 @@ map>> ConfigDBConnector_Native::get_conf // merge default value to config for (auto& table : data) { - LoadAndAppendDefaultValue(table.first, table.second); + DefaultValueProvider::Instance().AppendDefaultValues(table.first, table.second); } - return data; } @@ -671,7 +519,7 @@ map>> ConfigDBPipeConnector_Native::get_ // merge default value to config for (auto& table : data) { - LoadAndAppendDefaultValue(table.first, table.second); + DefaultValueProvider::Instance().AppendDefaultValues(table.first, table.second); } return data; diff --git a/common/defaultvalueprovider.cpp b/common/defaultvalueprovider.cpp index 9991e6d22..ee69b7045 100644 --- a/common/defaultvalueprovider.cpp +++ b/common/defaultvalueprovider.cpp @@ -112,6 +112,11 @@ shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string void DefaultValueProvider::AppendDefaultValues(string table, map >& values) { + if (this->context == nullptr) + { + this->Initialize(); + } + auto default_value_info = this->FindDefaultValueInfo(table); if (default_value_info == nullptr) { @@ -126,6 +131,11 @@ void DefaultValueProvider::AppendDefaultValues(string table, map& values) { + if (this->context == nullptr) + { + this->Initialize(); + } + auto default_value_info = this->FindDefaultValueInfo(table); if (default_value_info == nullptr) { diff --git a/common/defaultvalueprovider.h b/common/defaultvalueprovider.h index 949363939..5b618ff3b 100644 --- a/common/defaultvalueprovider.h +++ b/common/defaultvalueprovider.h @@ -70,8 +70,6 @@ class DefaultValueProvider public: static DefaultValueProvider& Instance(); - void Initialize(char* module_path); - void AppendDefaultValues(std::string table, std::map >& values); void AppendDefaultValues(std::string table, std::string key, std::map& values); @@ -86,6 +84,7 @@ class DefaultValueProvider // The table name to table default value info mapping std::map > default_value_mapping; + void Initialize(char* module_path = DEFAULT_YANG_MODULE_PATH); // Load default value info from yang model and append to default value mapping void AppendTableInfoToMapping(struct lys_node* table); From 2165f92917b75555f7f6ab37ead9c7b375c2ce91 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 13 Apr 2022 09:00:49 +0000 Subject: [PATCH 08/24] Fix build error --- common/defaultvalueprovider.cpp | 23 ++++++++++++----------- common/defaultvalueprovider.h | 6 +++--- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/common/defaultvalueprovider.cpp b/common/defaultvalueprovider.cpp index ee69b7045..765b4a0b7 100644 --- a/common/defaultvalueprovider.cpp +++ b/common/defaultvalueprovider.cpp @@ -15,9 +15,10 @@ using namespace std; using namespace swss; -void ThrowRunTimeError(string message) + +[[noreturn]] void ThrowRunTimeError(string message) { - SWSS_LOG_ERROR(message); + SWSS_LOG_ERROR("DefaultValueProvider: %s", message.c_str()); throw std::runtime_error(message); } @@ -79,14 +80,14 @@ TableInfoMultipleList::TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &f { for (auto& field_mapping : field_info_mapping) { - unsigned int fieldCount = std::get<1>(field_mapping.first); + int fieldCount = std::get<1>(field_mapping.first); this->defaultValueMapping[fieldCount] = field_mapping.second; } } bool TableInfoMultipleList::FindFieldMappingByKey(string key, FieldDefaultValueMapping ** founded_mapping_ptr) { - unsigned int key_field_count = std::count(key.begin(), key.end(), '|') + 1; + int key_field_count = (int)std::count(key.begin(), key.end(), '|') + 1; auto key_result = this->defaultValueMapping.find(key_field_count); *founded_mapping_ptr = key_result->second.get(); return key_result == this->defaultValueMapping.end(); @@ -175,7 +176,7 @@ void DefaultValueProvider::Initialize(char* module_path) { SWSS_LOG_DEBUG("file_name: %s\n", sub_dir->d_name); string file_name(sub_dir->d_name); - unsigned int pos = file_name.find(".yang"); + int pos = (int)file_name.find(".yang"); string module_name = file_name.substr(0, pos); const struct lys_module *module = ly_ctx_load_module( @@ -185,7 +186,7 @@ void DefaultValueProvider::Initialize(char* module_path) if (module->data == NULL) { // Every yang file should contains yang model - SWSS_LOG_WARN("Yang file " + file_name + " does not contains any model.\n"); + SWSS_LOG_WARN("Yang file %s does not contains any model.\n", sub_dir->d_name); continue; } @@ -218,7 +219,7 @@ void DefaultValueProvider::Initialize(char* module_path) std::shared_ptr DefaultValueProvider::GetKeyInfo(struct lys_node* table_child_node) { - unsigned int key_field_count = 0; + int key_field_count = 0; string key_value = ""; if (table_child_node->nodetype == LYS_LIST) { @@ -227,7 +228,7 @@ std::shared_ptr DefaultValueProvider::GetKeyInfo(struct lys_node* table // when a top level container contains list, the key defined by the 'keys' field. struct lys_node_list *list_node = (struct lys_node_list*)table_child_node; string key(list_node->keys_str); - key_field_count = std::count(key.begin(), key.end(), '|') + 1; + key_field_count = (int)std::count(key.begin(), key.end(), '|') + 1; } else if (table_child_node->nodetype == LYS_CONTAINER) { @@ -284,9 +285,9 @@ FieldDefaultValueMappingPtr DefaultValueProvider::GetDefaultValueInfo(struct lys return field_mapping; } -unsigned int DefaultValueProvider::BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping &field_info_mapping) +int DefaultValueProvider::BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping &field_info_mapping) { - unsigned child_list_count = 0; + int child_list_count = 0; auto next_child = table->child; while (next_child) @@ -318,7 +319,7 @@ void DefaultValueProvider::AppendTableInfoToMapping(struct lys_node* table) { SWSS_LOG_DEBUG("table name: %s\n",table->name); KeyInfoToDefaultValueInfoMapping field_info_mapping; - unsigned list_count = this->BuildFieldMappingList(table, field_info_mapping); + int list_count = this->BuildFieldMappingList(table, field_info_mapping); // create container data by list count TableInfoBase* table_info_ptr = nullptr; diff --git a/common/defaultvalueprovider.h b/common/defaultvalueprovider.h index 5b618ff3b..e192879b0 100644 --- a/common/defaultvalueprovider.h +++ b/common/defaultvalueprovider.h @@ -7,7 +7,7 @@ struct ly_ctx; // Key information -typedef std::tuple KeyInfo; +typedef std::tuple KeyInfo; // Field name to default value mapping typedef std::map FieldDefaultValueMapping; @@ -60,7 +60,7 @@ struct TableInfoMultipleList : public TableInfoBase private: // Mapping: key field count -> field -> default - std::map defaultValueMapping; + std::map defaultValueMapping; bool FindFieldMappingByKey(std::string key, std::map ** founded_mapping_ptr); }; @@ -91,7 +91,7 @@ class DefaultValueProvider std::shared_ptr FindDefaultValueInfo(std::string table); - unsigned int BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping& field_mapping_list); + int BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping& field_mapping_list); std::shared_ptr GetKeyInfo(struct lys_node* table_child_node); FieldDefaultValueMappingPtr GetDefaultValueInfo(struct lys_node* table_child_node); From 5f182681c4cba71c02637de9ab645c25bd4d6754 Mon Sep 17 00:00:00 2001 From: liuh-80 <58683130+liuh-80@users.noreply.github.com> Date: Thu, 14 Apr 2022 13:01:36 +0800 Subject: [PATCH 09/24] Change get all entry methods. --- common/configdb.cpp | 12 ++++++------ common/dbconnector.h | 23 ++++++++++++++++++++++- common/table.cpp | 23 +++++++++++++++++++++-- common/table.h | 6 ++++-- 4 files changed, 53 insertions(+), 11 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index fa3913ade..618cfbc59 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -499,6 +499,12 @@ int ConfigDBPipeConnector_Native::_get_config(DBConnector& client, RedisTransact string value = r.getChild(i+1)->str; dataentry.emplace(field, value); } + + // merge default value to config + for (auto& table : data) + { + DefaultValueProvider::Instance().AppendDefaultValues(table_name, dataentry); + } } return cur; } @@ -516,11 +522,5 @@ map>> ConfigDBPipeConnector_Native::get_ cur = _get_config(client, pipe, data, cur); } - // merge default value to config - for (auto& table : data) - { - DefaultValueProvider::Instance().AppendDefaultValues(table.first, table.second); - } - return data; } diff --git a/common/dbconnector.h b/common/dbconnector.h index bdd410f55..116e1d6d7 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -271,9 +271,30 @@ void DBConnector::hgetall(const std::string &key, OutputIterator result) auto ctx = r.getContext(); + if (this->getDbId() != CONFIG_DB) + { + for (unsigned int i = 0; i < ctx->elements; i += 2) + { + *result = std::make_pair(ctx->element[i]->str, ctx->element[i+1]->str); + ++result; + } + return; + } + + // When DB ID is CONFIG_DB, append default value to config DB result. + size_t pos = key.find("|"); + string table_name = key.substr(0, pos); + map data; for (unsigned int i = 0; i < ctx->elements; i += 2) { - *result = std::make_pair(ctx->element[i]->str, ctx->element[i+1]->str); + data[ctx->element[i]->str] = ctx->element[i+1]->str; + } + + DefaultValueProvider::Instance().AppendDefaultValues(table_name, data); + + for (auto& field_value_pair : data) + { + *result = std::make_pair(field_value_pair.first, field_value_pair.second); ++result; } } diff --git a/common/table.cpp b/common/table.cpp index cc2234d07..d269c0045 100644 --- a/common/table.cpp +++ b/common/table.cpp @@ -66,6 +66,7 @@ void Table::flush() bool Table::get(const string &key, vector &values) { + // [Hua] TODO: code here dupe with DBConnector::hgetall, check if can reuse it. RedisCommand hgetall_key; hgetall_key.format("HGETALL %s", getKeyName(key).c_str()); RedisReply r = m_pipe->push(hgetall_key, REDIS_REPLY_ARRAY); @@ -79,10 +80,28 @@ bool Table::get(const string &key, vector &values) throw system_error(make_error_code(errc::address_not_available), "Unable to connect netlink socket"); + if (this->getDbId() != CONFIG_DB) + { + for (unsigned int i = 0; i < reply->elements; i += 2) + { + values.emplace_back(stripSpecialSym(reply->element[i]->str), + reply->element[i + 1]->str); + } + return true; + } + + // When DB ID is CONFIG_DB, append default value to config DB result. + map data; for (unsigned int i = 0; i < reply->elements; i += 2) { - values.emplace_back(stripSpecialSym(reply->element[i]->str), - reply->element[i + 1]->str); + data[stripSpecialSym(reply->element[i]->str)] = reply->element[i + 1]->str; + } + + DefaultValueProvider::Instance().AppendDefaultValues(m_tableName, data); + + for (auto& field_value_pair : data) + { + values.emplace_back(field_value_pair.first, field_value_pair.second); } return true; diff --git a/common/table.h b/common/table.h index 04f25ee0e..f2f5f3b33 100644 --- a/common/table.h +++ b/common/table.h @@ -38,7 +38,7 @@ class TableBase { __attribute__((deprecated)) #endif TableBase(int dbId, const std::string &tableName) - : m_tableName(tableName) + : m_tableName(tableName), m_dbId(dbId) { /* Look up table separator for the provided DB */ auto it = tableNameSeparatorMap.find(dbId); @@ -55,7 +55,7 @@ class TableBase { } TableBase(const std::string &tableName, const std::string &tableSeparator) - : m_tableName(tableName), m_tableSeparator(tableSeparator) + : m_tableName(tableName), m_tableSeparator(tableSeparator), m_dbId(dbId) { static const std::string legalSeparators = ":|"; if (legalSeparators.find(tableSeparator) == std::string::npos) @@ -63,6 +63,7 @@ class TableBase { } std::string getTableName() const { return m_tableName; } + int getDbId() const { return m_dbId; } /* Return the actual key name as a combination of tableNamekey */ std::string getKeyName(const std::string &key) @@ -97,6 +98,7 @@ class TableBase { std::string m_tableName; std::string m_tableSeparator; + int m_dbId; }; class TableEntryWritable { From 0629be41d891cd82eadddeddb0e30a0521ab4f32 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 14 Apr 2022 06:13:40 +0000 Subject: [PATCH 10/24] Fix build issue --- common/configdb.cpp | 5 +---- common/dbconnector.h | 20 +++++++++++++++----- common/defaultvalueprovider.cpp | 22 +++++++++++----------- common/defaultvalueprovider.h | 10 ++++++---- common/table.cpp | 12 ++++++++++-- common/table.h | 6 ++---- 6 files changed, 45 insertions(+), 30 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index 618cfbc59..da3ab0022 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -501,10 +501,7 @@ int ConfigDBPipeConnector_Native::_get_config(DBConnector& client, RedisTransact } // merge default value to config - for (auto& table : data) - { - DefaultValueProvider::Instance().AppendDefaultValues(table_name, dataentry); - } + DefaultValueProvider::Instance().AppendDefaultValues(table_name, row, dataentry); } return cur; } diff --git a/common/dbconnector.h b/common/dbconnector.h index 116e1d6d7..621caccd6 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -5,12 +5,16 @@ #include #include #include +#include #include #include #include +#include "defaultvalueprovider.h" #include "rediscommand.h" #include "redisreply.h" +#include "schema.h" +#include "logger.h" #define EMPTY_NAMESPACE std::string() namespace swss { @@ -271,7 +275,13 @@ void DBConnector::hgetall(const std::string &key, OutputIterator result) auto ctx = r.getContext(); - if (this->getDbId() != CONFIG_DB) + size_t pos = key.find("|"); + if (this->getDbId() == CONFIG_DB && pos == std::string::npos) + { + SWSS_LOG_WARN("Table::get key for config DB is %s, can't find a sepreator\n", key.c_str()); + } + + if (this->getDbId() != CONFIG_DB || pos == std::string::npos) { for (unsigned int i = 0; i < ctx->elements; i += 2) { @@ -282,15 +292,15 @@ void DBConnector::hgetall(const std::string &key, OutputIterator result) } // When DB ID is CONFIG_DB, append default value to config DB result. - size_t pos = key.find("|"); - string table_name = key.substr(0, pos); - map data; + std::string table = key.substr(0, pos); + std::string row = key.substr(pos + 1); + std::map data; for (unsigned int i = 0; i < ctx->elements; i += 2) { data[ctx->element[i]->str] = ctx->element[i+1]->str; } - DefaultValueProvider::Instance().AppendDefaultValues(table_name, data); + DefaultValueProvider::Instance().AppendDefaultValues(table, row, data); for (auto& field_value_pair : data) { diff --git a/common/defaultvalueprovider.cpp b/common/defaultvalueprovider.cpp index 765b4a0b7..8eee8930d 100644 --- a/common/defaultvalueprovider.cpp +++ b/common/defaultvalueprovider.cpp @@ -54,25 +54,25 @@ TableInfoDict::TableInfoDict(KeyInfoToDefaultValueInfoMapping &field_info_mappin for (auto& field_mapping : field_info_mapping) { string fieldName = std::get<0>(field_mapping.first); - this->defaultValueMapping[fieldName] = field_mapping.second; + m_default_value_mapping[fieldName] = field_mapping.second; } } bool TableInfoDict::FindFieldMappingByKey(string key, FieldDefaultValueMapping ** founded_mapping_ptr) { - auto key_result = this->defaultValueMapping.find(key); + auto key_result = m_default_value_mapping.find(key); *founded_mapping_ptr = key_result->second.get(); - return key_result == this->defaultValueMapping.end(); + return key_result == m_default_value_mapping.end(); } TableInfoSingleList::TableInfoSingleList(KeyInfoToDefaultValueInfoMapping &field_info_mapping) { - this->defaultValueMapping = field_info_mapping.begin()->second; + m_default_value_mapping = field_info_mapping.begin()->second; } bool TableInfoSingleList::FindFieldMappingByKey(string key, FieldDefaultValueMapping ** founded_mapping_ptr) { - *founded_mapping_ptr = this->defaultValueMapping.get(); + *founded_mapping_ptr = m_default_value_mapping.get(); return true; } @@ -81,16 +81,16 @@ TableInfoMultipleList::TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &f for (auto& field_mapping : field_info_mapping) { int fieldCount = std::get<1>(field_mapping.first); - this->defaultValueMapping[fieldCount] = field_mapping.second; + m_default_value_mapping[fieldCount] = field_mapping.second; } } bool TableInfoMultipleList::FindFieldMappingByKey(string key, FieldDefaultValueMapping ** founded_mapping_ptr) { int key_field_count = (int)std::count(key.begin(), key.end(), '|') + 1; - auto key_result = this->defaultValueMapping.find(key_field_count); + auto key_result = m_default_value_mapping.find(key_field_count); *founded_mapping_ptr = key_result->second.get(); - return key_result == this->defaultValueMapping.end(); + return key_result == m_default_value_mapping.end(); } DefaultValueProvider& DefaultValueProvider::Instance() @@ -101,8 +101,8 @@ DefaultValueProvider& DefaultValueProvider::Instance() shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string table) { - auto find_result = this->default_value_mapping.find(table); - if (find_result == this->default_value_mapping.end()) + auto find_result = m_default_value_mapping.find(table); + if (find_result == m_default_value_mapping.end()) { SWSS_LOG_WARN("Not found default value info for table: %s\n", table.c_str()); return nullptr; @@ -337,5 +337,5 @@ void DefaultValueProvider::AppendTableInfoToMapping(struct lys_node* table) } string table_name(table->name); - default_value_mapping[table_name] = shared_ptr(table_info_ptr); + m_default_value_mapping[table_name] = shared_ptr(table_info_ptr); } \ No newline at end of file diff --git a/common/defaultvalueprovider.h b/common/defaultvalueprovider.h index e192879b0..905f3a8d0 100644 --- a/common/defaultvalueprovider.h +++ b/common/defaultvalueprovider.h @@ -1,5 +1,7 @@ #ifndef __DEFAULTVALUEPROVIDER__ #define __DEFAULTVALUEPROVIDER__ +#include +#include #define DEFAULT_YANG_MODULE_PATH "/usr/local/yang-models" #define EMPTY_STR "" @@ -36,7 +38,7 @@ class TableInfoDict : public TableInfoBase private: // Mapping: key value -> field -> default - std::map defaultValueMapping; + std::map m_default_value_mapping; bool FindFieldMappingByKey(std::string key, FieldDefaultValueMapping ** founded_mapping_ptr); }; @@ -48,7 +50,7 @@ class TableInfoSingleList : public TableInfoBase private: // Mapping: field -> default - FieldDefaultValueMappingPtr defaultValueMapping; + FieldDefaultValueMappingPtr m_default_value_mapping; bool FindFieldMappingByKey(std::string key, FieldDefaultValueMapping ** founded_mapping_ptr); }; @@ -60,7 +62,7 @@ struct TableInfoMultipleList : public TableInfoBase private: // Mapping: key field count -> field -> default - std::map defaultValueMapping; + std::map m_default_value_mapping; bool FindFieldMappingByKey(std::string key, std::map ** founded_mapping_ptr); }; @@ -82,7 +84,7 @@ class DefaultValueProvider struct ly_ctx *context = nullptr; // The table name to table default value info mapping - std::map > default_value_mapping; + std::map > m_default_value_mapping; void Initialize(char* module_path = DEFAULT_YANG_MODULE_PATH); diff --git a/common/table.cpp b/common/table.cpp index d269c0045..5e92b4709 100644 --- a/common/table.cpp +++ b/common/table.cpp @@ -80,7 +80,13 @@ bool Table::get(const string &key, vector &values) throw system_error(make_error_code(errc::address_not_available), "Unable to connect netlink socket"); - if (this->getDbId() != CONFIG_DB) + size_t pos = key.find("|"); + if (m_pipe->getDbId() == CONFIG_DB && pos == std::string::npos) + { + SWSS_LOG_WARN("Table::get key for config DB is %s, can't find a sepreator\n", key.c_str()); + } + + if (m_pipe->getDbId() != CONFIG_DB || pos == std::string::npos) { for (unsigned int i = 0; i < reply->elements; i += 2) { @@ -91,13 +97,15 @@ bool Table::get(const string &key, vector &values) } // When DB ID is CONFIG_DB, append default value to config DB result. + std::string table = key.substr(0, pos); + std::string row = key.substr(pos + 1); map data; for (unsigned int i = 0; i < reply->elements; i += 2) { data[stripSpecialSym(reply->element[i]->str)] = reply->element[i + 1]->str; } - DefaultValueProvider::Instance().AppendDefaultValues(m_tableName, data); + DefaultValueProvider::Instance().AppendDefaultValues(table, row, data); for (auto& field_value_pair : data) { diff --git a/common/table.h b/common/table.h index f2f5f3b33..04f25ee0e 100644 --- a/common/table.h +++ b/common/table.h @@ -38,7 +38,7 @@ class TableBase { __attribute__((deprecated)) #endif TableBase(int dbId, const std::string &tableName) - : m_tableName(tableName), m_dbId(dbId) + : m_tableName(tableName) { /* Look up table separator for the provided DB */ auto it = tableNameSeparatorMap.find(dbId); @@ -55,7 +55,7 @@ class TableBase { } TableBase(const std::string &tableName, const std::string &tableSeparator) - : m_tableName(tableName), m_tableSeparator(tableSeparator), m_dbId(dbId) + : m_tableName(tableName), m_tableSeparator(tableSeparator) { static const std::string legalSeparators = ":|"; if (legalSeparators.find(tableSeparator) == std::string::npos) @@ -63,7 +63,6 @@ class TableBase { } std::string getTableName() const { return m_tableName; } - int getDbId() const { return m_dbId; } /* Return the actual key name as a combination of tableNamekey */ std::string getKeyName(const std::string &key) @@ -98,7 +97,6 @@ class TableBase { std::string m_tableName; std::string m_tableSeparator; - int m_dbId; }; class TableEntryWritable { From 69a2af95ea1555423b9bf1d066138bcf3d06c4d8 Mon Sep 17 00:00:00 2001 From: liuh-80 <58683130+liuh-80@users.noreply.github.com> Date: Fri, 15 Apr 2022 13:58:49 +0800 Subject: [PATCH 11/24] Fix file issue --- common/configdb.cpp | 11 +- common/dbconnector.cpp | 16 +- common/defaultvalueprovider.cpp | 272 +++++++++++++++++++++++++------- common/defaultvalueprovider.h | 22 +-- common/table.cpp | 37 ++--- configure.ac | 2 + 6 files changed, 263 insertions(+), 97 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index da3ab0022..304b09ca2 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -186,10 +186,12 @@ map> ConfigDBConnector_Native::get_table(string tabl continue; } row = key.substr(pos + 1); + + DefaultValueProvider::Instance().AppendDefaultValues(table, row, const_cast& >(entry)); + data[row] = entry; } - DefaultValueProvider::Instance().AppendDefaultValues(table, data); return data; } @@ -261,15 +263,12 @@ map>> ConfigDBConnector_Native::get_conf if (!entry.empty()) { + DefaultValueProvider::Instance().AppendDefaultValues(table_name, row, const_cast& >(entry)); + data[table_name][row] = entry; } } - // merge default value to config - for (auto& table : data) - { - DefaultValueProvider::Instance().AppendDefaultValues(table.first, table.second); - } return data; } diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index 13f90df96..4ef07ab75 100644 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -780,7 +780,21 @@ shared_ptr DBConnector::hget(const string &key, const string &field) if (reply->type == REDIS_REPLY_NIL) { - return shared_ptr(NULL); + if (this->getDbId() != CONFIG_DB) + { + return shared_ptr(NULL); + } + + size_t pos = key.find("|"); + if (pos == std::string::npos) + { + SWSS_LOG_WARN("Table::get key for config DB is %s, can't find a sepreator\n", key.c_str()); + return shared_ptr(NULL); + } + + std::string table = key.substr(0, pos); + std::string row = key.substr(pos + 1); + return DefaultValueProvider::Instance().GetDefaultValue(table, row, field); } if (reply->type == REDIS_REPLY_STRING) diff --git a/common/defaultvalueprovider.cpp b/common/defaultvalueprovider.cpp index 8eee8930d..c08f6d1ce 100644 --- a/common/defaultvalueprovider.cpp +++ b/common/defaultvalueprovider.cpp @@ -1,8 +1,9 @@ #include -#include -#include +#include #include +#include #include +#include #include #include @@ -15,6 +16,23 @@ using namespace std; using namespace swss; +#ifdef DEBUG +#include +#include +#define TRACE_STACK_SIZE 30 +[[noreturn]] void sigv_handler(int sig) { + void *stack_info[TRACE_STACK_SIZE]; + size_t stack_info_size; + + // get void*'s for all entries on the stack + stack_info_size = backtrace(stack_info, TRACE_STACK_SIZE); + + // print out all the frames to stderr + cerr << "Error: signal " << sig << ":\n" >> endl; + backtrace_symbols_fd(array, (int)size, STDERR_FILENO); + exit(1); +} +#endif [[noreturn]] void ThrowRunTimeError(string message) { @@ -27,40 +45,72 @@ TableInfoBase::TableInfoBase() // C++ need this empty ctor } -bool TableInfoBase::TryAppendDefaultValues(string key, map& target_values) +std::shared_ptr TableInfoBase::GetDefaultValue(std::string row, std::string field) { + assert(!string::empty(row)); + assert(!string::empty(field)); + + SWSS_LOG_DEBUG("TableInfoBase::GetDefaultValue %s %s\n", row.c_str(), field.c_str()); FieldDefaultValueMapping *field_mapping_ptr; - if (!FindFieldMappingByKey(key, &field_mapping_ptr)) { - return false; + if (!FindFieldMappingByKey(row, &field_mapping_ptr)) { + SWSS_LOG_DEBUG("Can't found default value mapping for row %s\n", row.c_str()); + return nullptr; + } + + auto field_data = field_mapping_ptr->find(field); + if (field_data == field_mapping_ptr->end()) + { + SWSS_LOG_DEBUG("Can't found default value for field %s\n", field.c_str()); + return nullptr; + } + + SWSS_LOG_DEBUG("Found default value for field %s=%s\n", field.c_str(), field_data->second.c_str()); + return std::make_shared(field_data->second); +} + +// existed_values and target_values can be same container. +void TableInfoBase::AppendDefaultValues(string row, std::map& existed_values, map& target_values) +{ + assert(!string::empty(row)); + + SWSS_LOG_DEBUG("TableInfoBase::AppendDefaultValues %s\n", row.c_str()); + FieldDefaultValueMapping *field_mapping_ptr; + if (!FindFieldMappingByKey(row, &field_mapping_ptr)) { + SWSS_LOG_DEBUG("Can't found default value mapping for row %s\n", row.c_str()); + return; } for (auto &default_value : *field_mapping_ptr) { - auto field_data = target_values.find(default_value.first); - if (field_data != target_values.end()) + auto field_data = existed_values.find(default_value.first); + if (field_data != existed_values.end()) { - // ignore when a field already has value in config DB + // ignore when a field already has value in existed_values continue; } - target_values[default_value.first] = default_value.second; + SWSS_LOG_DEBUG("Append default value: %s=%s\n",default_value.first.c_str(), default_value.second.c_str()); + target_values.emplace(default_value.first, default_value.second); } - - return true; } TableInfoDict::TableInfoDict(KeyInfoToDefaultValueInfoMapping &field_info_mapping) { for (auto& field_mapping : field_info_mapping) { - string fieldName = std::get<0>(field_mapping.first); - m_default_value_mapping[fieldName] = field_mapping.second; + // KeyInfo.first is key value + string key_value = field_mapping.first.first; + m_default_value_mapping.emplace(key_value, field_mapping.second); } } -bool TableInfoDict::FindFieldMappingByKey(string key, FieldDefaultValueMapping ** founded_mapping_ptr) +bool TableInfoDict::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** founded_mapping_ptr) { - auto key_result = m_default_value_mapping.find(key); + assert(!string::empty(row)); + assert(founded_mapping_ptr != nullptr); + + SWSS_LOG_DEBUG("TableInfoDict::FindFieldMappingByKey %s\n", row.c_str()); + auto key_result = m_default_value_mapping.find(row); *founded_mapping_ptr = key_result->second.get(); return key_result == m_default_value_mapping.end(); } @@ -70,8 +120,12 @@ TableInfoSingleList::TableInfoSingleList(KeyInfoToDefaultValueInfoMapping &field m_default_value_mapping = field_info_mapping.begin()->second; } -bool TableInfoSingleList::FindFieldMappingByKey(string key, FieldDefaultValueMapping ** founded_mapping_ptr) +bool TableInfoSingleList::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** founded_mapping_ptr) { + assert(!string::empty(row)); + assert(founded_mapping_ptr != nullptr); + + SWSS_LOG_DEBUG("TableInfoSingleList::FindFieldMappingByKey %s\n", row.c_str()); *founded_mapping_ptr = m_default_value_mapping.get(); return true; } @@ -80,95 +134,166 @@ TableInfoMultipleList::TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &f { for (auto& field_mapping : field_info_mapping) { - int fieldCount = std::get<1>(field_mapping.first); - m_default_value_mapping[fieldCount] = field_mapping.second; + // KeyInfo.second is field count + int field_count = field_mapping.first.second; + m_default_value_mapping.emplace(field_count, field_mapping.second); } } -bool TableInfoMultipleList::FindFieldMappingByKey(string key, FieldDefaultValueMapping ** founded_mapping_ptr) +bool TableInfoMultipleList::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** founded_mapping_ptr) { - int key_field_count = (int)std::count(key.begin(), key.end(), '|') + 1; - auto key_result = m_default_value_mapping.find(key_field_count); - *founded_mapping_ptr = key_result->second.get(); - return key_result == m_default_value_mapping.end(); + assert(!string::empty(row)); + assert(founded_mapping_ptr != nullptr); + + SWSS_LOG_DEBUG("TableInfoMultipleList::FindFieldMappingByKey %s\n", row.c_str()); + int field_count = (int)std::count(row.begin(), row.end(), '|') + 1; + auto key_info = m_default_value_mapping.find(field_count); + + // when not found, key_info still a valied iterator + *founded_mapping_ptr = key_info->second.get(); + + // return false when not found + return key_info != m_default_value_mapping.end(); } DefaultValueProvider& DefaultValueProvider::Instance() { static DefaultValueProvider instance; + if (instance.context == nullptr) + { + instance.Initialize(); + } + return instance; } shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string table) { + assert(!string::empty(table)); + + SWSS_LOG_DEBUG("DefaultValueProvider::FindDefaultValueInfo %s\n", table.c_str()); auto find_result = m_default_value_mapping.find(table); if (find_result == m_default_value_mapping.end()) { - SWSS_LOG_WARN("Not found default value info for table: %s\n", table.c_str()); + SWSS_LOG_DEBUG("Not found default value info for %s\n", table.c_str()); return nullptr; } return find_result->second; } -void DefaultValueProvider::AppendDefaultValues(string table, map >& values) +std::shared_ptr DefaultValueProvider::GetDefaultValue(std::string table, std::string row, std::string field) +{ + assert(!string::empty(table)); + assert(!string::empty(row)); + assert(!string::empty(field)); + + SWSS_LOG_DEBUG("DefaultValueProvider::GetDefaultValue %s %s %s\n", table.c_str(), row.c_str(), field.c_str()); +#ifdef DEBUG + if (!FeatureEnabledByEnvironmentVariable()) + { + return nullptr; + } +#endif + + auto default_value_info = FindDefaultValueInfo(table); + if (default_value_info == nullptr) + { + SWSS_LOG_DEBUG("Not found default value info for %s\n", table.c_str()); + return nullptr; + } + + return default_value_info->GetDefaultValue(row, field); +} + +void DefaultValueProvider::AppendDefaultValues(std::string table, std::string row, std::vector > &values) { - if (this->context == nullptr) + assert(!string::empty(table)); + assert(!string::empty(row)); + + SWSS_LOG_DEBUG("DefaultValueProvider::AppendDefaultValues %s %s\n", table.c_str(), row.c_str()); +#ifdef DEBUG + if (!FeatureEnabledByEnvironmentVariable()) { - this->Initialize(); + return; } +#endif - auto default_value_info = this->FindDefaultValueInfo(table); + auto default_value_info = FindDefaultValueInfo(table); if (default_value_info == nullptr) { + SWSS_LOG_DEBUG("Not found default value info for %s\n", table.c_str()); return; } + + map existed_values; + map default_values; + for (auto& field_value_pair : values) + { + existed_values.emplace(field_value_pair.first, field_value_pair.second); + } + + default_value_info->AppendDefaultValues(row, existed_values, default_values); - for (auto& row : values) + for (auto& field_value_pair : default_values) { - default_value_info->TryAppendDefaultValues(row.first, row.second); + values.emplace_back(field_value_pair.first, field_value_pair.second); } } -void DefaultValueProvider::AppendDefaultValues(string table, string key, map& values) +void DefaultValueProvider::AppendDefaultValues(string table, string row, map& values) { - if (this->context == nullptr) + assert(!string::empty(table)); + assert(!string::empty(row)); + + SWSS_LOG_DEBUG("DefaultValueProvider::AppendDefaultValues %s %s\n", table.c_str(), row.c_str()); +#ifdef DEBUG + if (!FeatureEnabledByEnvironmentVariable()) { - this->Initialize(); + return; } +#endif - auto default_value_info = this->FindDefaultValueInfo(table); + auto default_value_info = FindDefaultValueInfo(table); if (default_value_info == nullptr) { + SWSS_LOG_DEBUG("Not found default value info for %s\n", table.c_str()); return; } - default_value_info->TryAppendDefaultValues(key, values); + default_value_info->AppendDefaultValues(row, values, values); } +DefaultValueProvider::DefaultValueProvider() +{ +#ifdef DEBUG + // initialize crash event handler for debug. + signal(SIGSEGV, sigv_handler); +#endif +} + + DefaultValueProvider::~DefaultValueProvider() { - if (this->context) + if (context) { // set private_destructor to NULL because no any private data - ly_ctx_destroy(this->context, NULL); + ly_ctx_destroy(context, NULL); } } void DefaultValueProvider::Initialize(char* module_path) { - if (this->context) - { - ThrowRunTimeError("DefaultValueProvider already initialized"); - } - - this->context = ly_ctx_new(module_path, LY_CTX_ALLIMPLEMENTED); + assert(module_path != nullptr && !string::empty(module_path)); + assert(context == nullptr); + DIR *module_dir = opendir(module_path); if (!module_dir) { ThrowRunTimeError("Open Yang model path " + string(module_path) + " failed"); } + context = ly_ctx_new(module_path, LY_CTX_ALLIMPLEMENTED); struct dirent *sub_dir; while ((sub_dir = readdir(module_dir)) != NULL) { @@ -180,13 +305,13 @@ void DefaultValueProvider::Initialize(char* module_path) string module_name = file_name.substr(0, pos); const struct lys_module *module = ly_ctx_load_module( - this->context, + context, module_name.c_str(), EMPTY_STR); // Use EMPTY_STR to revision to load the latest revision if (module->data == NULL) { - // Every yang file should contains yang model - SWSS_LOG_WARN("Yang file %s does not contains any model.\n", sub_dir->d_name); + // Not every yang file should contains yang model + SWSS_LOG_WARN("Yang file %s does not contains model %s.\n", sub_dir->d_name, module_name.c_str()); continue; } @@ -195,6 +320,7 @@ void DefaultValueProvider::Initialize(char* module_path) { if (top_level_node->nodetype != LYS_CONTAINER) { + SWSS_LOG_DEBUG("ignore top level element %s, tyoe %d\n",top_level_node->name, top_level_node->nodetype); // Config DB table schema is defined by top level container top_level_node = top_level_node->next; continue; @@ -219,11 +345,14 @@ void DefaultValueProvider::Initialize(char* module_path) std::shared_ptr DefaultValueProvider::GetKeyInfo(struct lys_node* table_child_node) { + assert(table_child_node != nullptr); + SWSS_LOG_DEBUG("DefaultValueProvider::GetKeyInfo %s\n",table_child_node->name); + int key_field_count = 0; string key_value = ""; if (table_child_node->nodetype == LYS_LIST) { - SWSS_LOG_DEBUG("child list: %s\n",table_child_node->name); + SWSS_LOG_DEBUG("Child list: %s\n",table_child_node->name); // when a top level container contains list, the key defined by the 'keys' field. struct lys_node_list *list_node = (struct lys_node_list*)table_child_node; @@ -232,13 +361,14 @@ std::shared_ptr DefaultValueProvider::GetKeyInfo(struct lys_node* table } else if (table_child_node->nodetype == LYS_CONTAINER) { - SWSS_LOG_DEBUG("child container name: %s\n",table_child_node->name); + SWSS_LOG_DEBUG("Child container name: %s\n",table_child_node->name); // when a top level container not contains any list, the key is child container name key_value = string(table_child_node->name); } else { + SWSS_LOG_DEBUG("Ignore child element: %s\n",table_child_node->name); return nullptr; } @@ -247,12 +377,16 @@ std::shared_ptr DefaultValueProvider::GetKeyInfo(struct lys_node* table FieldDefaultValueMappingPtr DefaultValueProvider::GetDefaultValueInfo(struct lys_node* table_child_node) { + assert(table_child_node != nullptr); + SWSS_LOG_DEBUG("DefaultValueProvider::GetDefaultValueInfo %s\n",table_child_node->name); + auto field = table_child_node->child; auto field_mapping = make_shared(); while (field) { if (field->nodetype == LYS_LEAF) { + SWSS_LOG_DEBUG("leaf field: %s\n",field->name); struct lys_node_leaf *leaf_node = (struct lys_node_leaf*)field; if (leaf_node->dflt) { @@ -262,6 +396,7 @@ FieldDefaultValueMappingPtr DefaultValueProvider::GetDefaultValueInfo(struct lys } else if (field->nodetype == LYS_CHOICE) { + SWSS_LOG_DEBUG("choice field: %s\n",field->name); struct lys_node_choice *choice_node = (struct lys_node_choice *)field; if (choice_node->dflt) { @@ -271,6 +406,7 @@ FieldDefaultValueMappingPtr DefaultValueProvider::GetDefaultValueInfo(struct lys } else if (field->nodetype == LYS_LEAFLIST) { + SWSS_LOG_DEBUG("list field: %s\n",field->name); struct lys_node_leaflist *list_node = (struct lys_node_leaflist *)field; if (list_node->dflt) { @@ -278,6 +414,12 @@ FieldDefaultValueMappingPtr DefaultValueProvider::GetDefaultValueInfo(struct lys SWSS_LOG_DEBUG("list field: %s, default: TBD\n",list_node->name); } } +#ifdef DEBUG + else + { + SWSS_LOG_DEBUG("Field %s with type %d does not support default value\n",field->name, field->nodetype); + } +#endif field = field->next; } @@ -287,26 +429,27 @@ FieldDefaultValueMappingPtr DefaultValueProvider::GetDefaultValueInfo(struct lys int DefaultValueProvider::BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping &field_info_mapping) { - int child_list_count = 0; + assert(table != nullptr); + int child_list_count = 0; auto next_child = table->child; while (next_child) { // get key from schema - auto keyInfo = this->GetKeyInfo(next_child); + auto keyInfo = GetKeyInfo(next_child); if (keyInfo == nullptr) { next_child = next_child->next; continue; } - else if (std::get<1>(*keyInfo) != 0) + else if (keyInfo->second != 0) { // when key field count not 0, it's a list node. child_list_count++; } // get field name to default value mappings from schema - field_info_mapping[*keyInfo] = this->GetDefaultValueInfo(next_child); + field_info_mapping.emplace(*keyInfo, GetDefaultValueInfo(next_child)); next_child = next_child->next; } @@ -317,9 +460,11 @@ int DefaultValueProvider::BuildFieldMappingList(struct lys_node* table, KeyInfoT // Load default value info from yang model and append to default value mapping void DefaultValueProvider::AppendTableInfoToMapping(struct lys_node* table) { - SWSS_LOG_DEBUG("table name: %s\n",table->name); + assert(table != nullptr); + + SWSS_LOG_DEBUG("DefaultValueProvider::AppendTableInfoToMapping table name: %s\n",table->name); KeyInfoToDefaultValueInfoMapping field_info_mapping; - int list_count = this->BuildFieldMappingList(table, field_info_mapping); + int list_count = BuildFieldMappingList(table, field_info_mapping); // create container data by list count TableInfoBase* table_info_ptr = nullptr; @@ -337,5 +482,20 @@ void DefaultValueProvider::AppendTableInfoToMapping(struct lys_node* table) } string table_name(table->name); - m_default_value_mapping[table_name] = shared_ptr(table_info_ptr); -} \ No newline at end of file + m_default_value_mapping.emplace(table_name, shared_ptr(table_info_ptr)); +} + +#ifdef DEBUG +bool DefaultValueProvider::FeatureEnabledByEnvironmentVariable() +{ + const char* show_default = getenv("CONFIG_DB_DEFAULT_VALUE"); + if (show_default == nullptr || strcmp(show_default, "TRUE") != 0) + { + // enable feature with "export CONFIG_DB_DEFAULT_VALUE=TRUE" + SWSS_LOG_DEBUG("enable feature with \"export CONFIG_DB_DEFAULT_VALUE=TRUE\"\n"); + return false; + } + + return true; +} +#endif \ No newline at end of file diff --git a/common/defaultvalueprovider.h b/common/defaultvalueprovider.h index 905f3a8d0..b75018f41 100644 --- a/common/defaultvalueprovider.h +++ b/common/defaultvalueprovider.h @@ -9,7 +9,7 @@ struct ly_ctx; // Key information -typedef std::tuple KeyInfo; +typedef std::pair KeyInfo; // Field name to default value mapping typedef std::map FieldDefaultValueMapping; @@ -25,10 +25,12 @@ class TableInfoBase public: TableInfoBase(); - bool TryAppendDefaultValues(std::string key, std::map& target_values); + void AppendDefaultValues(std::string row, std::map& source_values, std::map& target_values); + + std::shared_ptr GetDefaultValue(std::string row, std::string field); protected: - virtual bool FindFieldMappingByKey(std::string key, FieldDefaultValueMapping ** founded_mapping_ptr) = 0; + virtual bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** founded_mapping_ptr) = 0; }; class TableInfoDict : public TableInfoBase @@ -40,7 +42,7 @@ class TableInfoDict : public TableInfoBase // Mapping: key value -> field -> default std::map m_default_value_mapping; - bool FindFieldMappingByKey(std::string key, FieldDefaultValueMapping ** founded_mapping_ptr); + bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** founded_mapping_ptr); }; class TableInfoSingleList : public TableInfoBase @@ -52,7 +54,7 @@ class TableInfoSingleList : public TableInfoBase // Mapping: field -> default FieldDefaultValueMappingPtr m_default_value_mapping; - bool FindFieldMappingByKey(std::string key, FieldDefaultValueMapping ** founded_mapping_ptr); + bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** founded_mapping_ptr); }; struct TableInfoMultipleList : public TableInfoBase @@ -64,7 +66,7 @@ struct TableInfoMultipleList : public TableInfoBase // Mapping: key field count -> field -> default std::map m_default_value_mapping; - bool FindFieldMappingByKey(std::string key, std::map ** founded_mapping_ptr); + bool FindFieldMappingByKey(std::string row, std::map ** founded_mapping_ptr); }; class DefaultValueProvider @@ -72,12 +74,14 @@ class DefaultValueProvider public: static DefaultValueProvider& Instance(); - void AppendDefaultValues(std::string table, std::map >& values); + void AppendDefaultValues(std::string table, std::string row, std::map& values); + + void AppendDefaultValues(std::string table, std::string row, std::vector > &values); - void AppendDefaultValues(std::string table, std::string key, std::map& values); + std::shared_ptr GetDefaultValue(std::string table, std::string row, std::string field); private: - DefaultValueProvider() {}; + DefaultValueProvider(); ~DefaultValueProvider(); // libyang context diff --git a/common/table.cpp b/common/table.cpp index 5e92b4709..0f1974c62 100644 --- a/common/table.cpp +++ b/common/table.cpp @@ -80,36 +80,23 @@ bool Table::get(const string &key, vector &values) throw system_error(make_error_code(errc::address_not_available), "Unable to connect netlink socket"); - size_t pos = key.find("|"); - if (m_pipe->getDbId() == CONFIG_DB && pos == std::string::npos) - { - SWSS_LOG_WARN("Table::get key for config DB is %s, can't find a sepreator\n", key.c_str()); - } - - if (m_pipe->getDbId() != CONFIG_DB || pos == std::string::npos) - { - for (unsigned int i = 0; i < reply->elements; i += 2) - { - values.emplace_back(stripSpecialSym(reply->element[i]->str), - reply->element[i + 1]->str); - } - return true; - } - - // When DB ID is CONFIG_DB, append default value to config DB result. - std::string table = key.substr(0, pos); - std::string row = key.substr(pos + 1); - map data; for (unsigned int i = 0; i < reply->elements; i += 2) { - data[stripSpecialSym(reply->element[i]->str)] = reply->element[i + 1]->str; + values.emplace_back(stripSpecialSym(reply->element[i]->str), + reply->element[i + 1]->str); } - DefaultValueProvider::Instance().AppendDefaultValues(table, row, data); - - for (auto& field_value_pair : data) + if (m_pipe->getDbId() == CONFIG_DB) { - values.emplace_back(field_value_pair.first, field_value_pair.second); + size_t pos = key.find("|"); + if (pos == std::string::npos) + { + SWSS_LOG_WARN("Table::get key for config DB is %s, can't find a sepreator\n", key.c_str()); + } + else + { + DefaultValueProvider::Instance().AppendDefaultValues(table, row, values); + } } return true; diff --git a/configure.ac b/configure.ac index 3c2e05055..783789da3 100644 --- a/configure.ac +++ b/configure.ac @@ -91,6 +91,8 @@ CFLAGS_COMMON+=" -Wno-write-strings" CFLAGS_COMMON+=" -Wno-missing-format-attribute" CFLAGS_COMMON+=" -Wno-long-long" +CFLAGS_COMMON+=" -g -rdynamic" + AC_SUBST(CFLAGS_COMMON) AC_CONFIG_FILES([ From cc4ca1554268324b09f2a10bbfa44a1e87cd25d6 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Fri, 15 Apr 2022 07:09:59 +0000 Subject: [PATCH 12/24] Improve code --- common/dbconnector.h | 51 ++++++++++++++++++++++++-------------------- common/table.cpp | 2 ++ 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/common/dbconnector.h b/common/dbconnector.h index 621caccd6..8c3057404 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -275,37 +275,42 @@ void DBConnector::hgetall(const std::string &key, OutputIterator result) auto ctx = r.getContext(); - size_t pos = key.find("|"); - if (this->getDbId() == CONFIG_DB && pos == std::string::npos) + for (unsigned int i = 0; i < ctx->elements; i += 2) { - SWSS_LOG_WARN("Table::get key for config DB is %s, can't find a sepreator\n", key.c_str()); + *result = std::make_pair(ctx->element[i]->str, ctx->element[i+1]->str); + ++result; } - - if (this->getDbId() != CONFIG_DB || pos == std::string::npos) + + if (this->getDbId() == CONFIG_DB) { - for (unsigned int i = 0; i < ctx->elements; i += 2) + size_t pos = key.find("|"); + if (pos == std::string::npos) { - *result = std::make_pair(ctx->element[i]->str, ctx->element[i+1]->str); - ++result; + SWSS_LOG_WARN("Table::get key for config DB is %s, can't find a sepreator\n", key.c_str()); } - return; - } - // When DB ID is CONFIG_DB, append default value to config DB result. - std::string table = key.substr(0, pos); - std::string row = key.substr(pos + 1); - std::map data; - for (unsigned int i = 0; i < ctx->elements; i += 2) - { - data[ctx->element[i]->str] = ctx->element[i+1]->str; - } + // When DB ID is CONFIG_DB, append default value to config DB result. + std::string table = key.substr(0, pos); + std::string row = key.substr(pos + 1); + std::map values_with_default; + std::map existed_values; + for (unsigned int i = 0; i < ctx->elements; i += 2) + { + existed_values[ctx->element[i]->str] = ctx->element[i+1]->str; + values_with_default[ctx->element[i]->str] = ctx->element[i+1]->str; + } - DefaultValueProvider::Instance().AppendDefaultValues(table, row, data); + DefaultValueProvider::Instance().AppendDefaultValues(table, row, values_with_default); - for (auto& field_value_pair : data) - { - *result = std::make_pair(field_value_pair.first, field_value_pair.second); - ++result; + for (auto& field_value_pair : values_with_default) + { + auto find_result = existed_values.find(field_value_pair.first); + if (find_result == existed_values.end()) + { + *result = std::make_pair(field_value_pair.first, field_value_pair.second); + ++result; + } + } } } #endif diff --git a/common/table.cpp b/common/table.cpp index 0f1974c62..45bfaf1f7 100644 --- a/common/table.cpp +++ b/common/table.cpp @@ -95,6 +95,8 @@ bool Table::get(const string &key, vector &values) } else { + std::string table = key.substr(0, pos); + std::string row = key.substr(pos + 1); DefaultValueProvider::Instance().AppendDefaultValues(table, row, values); } } From 379b0933e4492aec5a500e0432616a786f8c6e1c Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Fri, 15 Apr 2022 07:15:40 +0000 Subject: [PATCH 13/24] Improve build flag --- common/Makefile.am | 2 +- configure.ac | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/common/Makefile.am b/common/Makefile.am index 78c90e182..49273b209 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -22,7 +22,7 @@ dist_swsscommon_DATA = $(EXTRA_CONF_DIST) bin_PROGRAMS = swssloglevel if DEBUG -DBGFLAGS = -ggdb -DDEBUG +DBGFLAGS = -ggdb -DDEBUG -g -rdynamic else DBGFLAGS = -g -DNDEBUG endif diff --git a/configure.ac b/configure.ac index 783789da3..3c2e05055 100644 --- a/configure.ac +++ b/configure.ac @@ -91,8 +91,6 @@ CFLAGS_COMMON+=" -Wno-write-strings" CFLAGS_COMMON+=" -Wno-missing-format-attribute" CFLAGS_COMMON+=" -Wno-long-long" -CFLAGS_COMMON+=" -g -rdynamic" - AC_SUBST(CFLAGS_COMMON) AC_CONFIG_FILES([ From ba5dc9a5c7db74426eafad660eb27e3aec328cb8 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 18 Apr 2022 03:20:14 +0000 Subject: [PATCH 14/24] Save code change --- common/configdb.cpp | 29 +-- common/configdb.h | 12 +- common/dbconnector.cpp | 4 +- common/dbconnector.h | 12 +- common/dbinterface.cpp | 8 +- common/dbinterface.h | 4 +- common/defaultvalueprovider.cpp | 308 ++++++++++++++++---------------- common/defaultvalueprovider.h | 35 ++-- common/logger.cpp | 4 +- common/loglevel.cpp | 2 +- common/redisclient.h | 4 +- common/sonicv2connector.cpp | 8 +- common/sonicv2connector.h | 4 +- tests/fdb_flush.cpp | 2 +- tests/logger_ut.cpp | 2 +- tests/redis_state_ut.cpp | 2 +- tests/redis_ut.cpp | 14 +- 17 files changed, 231 insertions(+), 223 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index 304b09ca2..1f0223de3 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -80,7 +80,7 @@ void ConfigDBConnector_Native::set_entry(string table, string key, const map ConfigDBConnector_Native::get_entry(string table, string key) +map ConfigDBConnector_Native::get_entry(string table, string key, bool withDefaultValue) { auto& client = get_redis_client(m_db_name); string _hash = to_upper(table) + m_table_name_separator + key; - return client.hgetall>(_hash); + return client.hgetall>(_hash, withDefaultValue); } // Read all keys of a table from config db. @@ -170,7 +170,7 @@ vector ConfigDBConnector_Native::get_keys(string table, bool split) // { 'row_key': {'column_key': value, ...}, ...} // or { ('l1_key', 'l2_key', ...): {'column_key': value, ...}, ...} for a multi-key table. // Empty dictionary if table does not exist. -map> ConfigDBConnector_Native::get_table(string table) +map> ConfigDBConnector_Native::get_table(string table, bool withDefaultValue) { auto& client = get_redis_client(m_db_name); string pattern = to_upper(table) + m_table_name_separator + "*"; @@ -178,7 +178,7 @@ map> ConfigDBConnector_Native::get_table(string tabl map> data; for (auto& key: keys) { - auto const& entry = client.hgetall>(key); + auto const& entry = client.hgetall>(key, withDefaultValue); size_t pos = key.find(m_table_name_separator); string row; if (pos == string::npos) @@ -246,7 +246,7 @@ void ConfigDBConnector_Native::mod_config(const map>> ConfigDBConnector_Native::get_config() +map>> ConfigDBConnector_Native::get_config(bool withDefaultValue) { auto& client = get_redis_client(m_db_name); auto const& keys = client.keys("*"); @@ -259,7 +259,7 @@ map>> ConfigDBConnector_Native::get_conf } string table_name = key.substr(0, pos); string row = key.substr(pos + 1); - auto const& entry = client.hgetall>(key); + auto const& entry = client.hgetall>(key, withDefaultValue); if (!entry.empty()) { @@ -371,7 +371,7 @@ void ConfigDBPipeConnector_Native::_set_entry(RedisTransactioner& pipe, std::str } else { - auto original = get_entry(table, key); + auto original = get_entry(table, key, false); RedisCommand shmset; shmset.formatHMSET(_hash, data.begin(), data.end()); @@ -458,7 +458,7 @@ void ConfigDBPipeConnector_Native::mod_config(const map>>& data, int cursor) +int ConfigDBPipeConnector_Native::_get_config(DBConnector& client, RedisTransactioner& pipe, map>>& data, int cursor, bool withDefaultValue) { auto const& rc = client.scan(cursor, "*", REDIS_SCAN_BATCH_SIZE); auto cur = rc.first; @@ -500,22 +500,25 @@ int ConfigDBPipeConnector_Native::_get_config(DBConnector& client, RedisTransact } // merge default value to config - DefaultValueProvider::Instance().AppendDefaultValues(table_name, row, dataentry); + if (withDefaultValue) + { + DefaultValueProvider::Instance().AppendDefaultValues(table_name, row, dataentry); + } } return cur; } -map>> ConfigDBPipeConnector_Native::get_config() +map>> ConfigDBPipeConnector_Native::get_config(bool withDefaultValue) { auto& client = get_redis_client(m_db_name); DBConnector clientPipe(client); RedisTransactioner pipe(&clientPipe); map>> data; - auto cur = _get_config(client, pipe, data, 0); + auto cur = _get_config(client, pipe, data, 0, withDefaultValue); while (cur != 0) { - cur = _get_config(client, pipe, data, cur); + cur = _get_config(client, pipe, data, cur, withDefaultValue); } return data; diff --git a/common/configdb.h b/common/configdb.h index d41a49cec..22666053c 100644 --- a/common/configdb.h +++ b/common/configdb.h @@ -19,12 +19,12 @@ class ConfigDBConnector_Native : public SonicV2Connector_Native virtual void set_entry(std::string table, std::string key, const std::map& data); virtual void mod_entry(std::string table, std::string key, const std::map& data); - std::map get_entry(std::string table, std::string key); + std::map get_entry(std::string table, std::string key, bool withDefaultValue); std::vector get_keys(std::string table, bool split = true); - std::map> get_table(std::string table); + std::map> get_table(std::string table, bool withDefaultValue); void delete_table(std::string table); virtual void mod_config(const std::map>>& data); - virtual std::map>> get_config(); + virtual std::map>> get_config(bool withDefaultValue); std::string getKeySeparator() const; std::string getTableNameSeparator() const; @@ -72,7 +72,7 @@ class ConfigDBConnector_Native : public SonicV2Connector_Native return self.getDbName() ## Note: callback is difficult to implement by SWIG C++, so keep in python - def listen(self, init_data_handler=None): + def listen(self, init_data_handler=None, with_default_value=False): ## Start listen Redis keyspace event. Pass a callback function to `init` to handle initial table data. self.pubsub = self.get_redis_client(self.db_name).pubsub() self.pubsub.psubscribe("__keyspace@{}__:*".format(self.get_dbid(self.db_name))) @@ -246,7 +246,7 @@ class ConfigDBPipeConnector_Native: public ConfigDBConnector_Native void set_entry(std::string table, std::string key, const std::map& data) override; void mod_config(const std::map>>& data) override; - std::map>> get_config() override; + std::map>> get_config(bool withDefaultValue) override; private: static const int64_t REDIS_SCAN_BATCH_SIZE = 30; @@ -255,7 +255,7 @@ class ConfigDBPipeConnector_Native: public ConfigDBConnector_Native void _delete_table(DBConnector& client, RedisTransactioner& pipe, std::string table); void _set_entry(RedisTransactioner& pipe, std::string table, std::string key, const std::map& data); void _mod_entry(RedisTransactioner& pipe, std::string table, std::string key, const std::map& data); - int _get_config(DBConnector& client, RedisTransactioner& pipe, std::map>>& data, int cursor); + int _get_config(DBConnector& client, RedisTransactioner& pipe, std::map>>& data, int cursor, bool withDefaultValue); }; #ifdef SWIG diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index 4ef07ab75..eb02d56f7 100644 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -771,7 +771,7 @@ shared_ptr DBConnector::get(const string &key) throw runtime_error("GET failed, memory exception"); } -shared_ptr DBConnector::hget(const string &key, const string &field) +shared_ptr DBConnector::hget(const string &key, const string &field, bool withDefaultValue) { RedisCommand shget; shget.format("HGET %s %s", key.c_str(), field.c_str()); @@ -780,7 +780,7 @@ shared_ptr DBConnector::hget(const string &key, const string &field) if (reply->type == REDIS_REPLY_NIL) { - if (this->getDbId() != CONFIG_DB) + if (!withDefaultValue || this->getDbId() != CONFIG_DB) { return shared_ptr(NULL); } diff --git a/common/dbconnector.h b/common/dbconnector.h index 8c3057404..988fccfa8 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -200,11 +200,11 @@ class DBConnector : public RedisContext void del(const std::vector& keys); template > - ReturnType hgetall(const std::string &key); + ReturnType hgetall(const std::string &key, bool withDefaultValue); #ifndef SWIG template - void hgetall(const std::string &key, OutputIterator result); + void hgetall(const std::string &key, OutputIterator result, bool withDefaultValue); #endif std::vector keys(const std::string &key); @@ -223,7 +223,7 @@ class DBConnector : public RedisContext std::shared_ptr get(const std::string &key); - std::shared_ptr hget(const std::string &key, const std::string &field); + std::shared_ptr hget(const std::string &key, const std::string &field, bool withDefaultValue); bool hexists(const std::string &key, const std::string &field); @@ -258,16 +258,16 @@ class DBConnector : public RedisContext }; template -ReturnType DBConnector::hgetall(const std::string &key) +ReturnType DBConnector::hgetall(const std::string &key, bool withDefaultValue) { ReturnType map; - hgetall(key, std::inserter(map, map.end())); + hgetall(key, std::inserter(map, map.end()), withDefaultValue); return map; } #ifndef SWIG template -void DBConnector::hgetall(const std::string &key, OutputIterator result) +void DBConnector::hgetall(const std::string &key, OutputIterator result, bool withDefaultValue) { RedisCommand shgetall; shgetall.format("HGETALL %s", key.c_str()); diff --git a/common/dbinterface.cpp b/common/dbinterface.cpp index 0043fb4c5..fd3962ee2 100644 --- a/common/dbinterface.cpp +++ b/common/dbinterface.cpp @@ -56,11 +56,11 @@ bool DBInterface::exists(const string& dbName, const std::string& key) return m_redisClient.at(dbName).exists(key); } -std::shared_ptr DBInterface::get(const std::string& dbName, const std::string& hash, const std::string& key, bool blocking) +std::shared_ptr DBInterface::get(const std::string& dbName, const std::string& hash, const std::string& key, bool withDefaultValue, bool blocking) { auto innerfunc = [&] { - auto pvalue = m_redisClient.at(dbName).hget(hash, key); + auto pvalue = m_redisClient.at(dbName).hget(hash, key, withDefaultValue); if (!pvalue) { std::string message = "Key '" + hash + "' field '" + key + "' unavailable in database '" + dbName + "'"; @@ -77,12 +77,12 @@ bool DBInterface::hexists(const std::string& dbName, const std::string& hash, co return m_redisClient.at(dbName).hexists(hash, key); } -std::map DBInterface::get_all(const std::string& dbName, const std::string& hash, bool blocking) +std::map DBInterface::get_all(const std::string& dbName, const std::string& hash, bool withDefaultValue, bool blocking) { auto innerfunc = [&] { std::map map; - m_redisClient.at(dbName).hgetall(hash, std::inserter(map, map.end())); + m_redisClient.at(dbName).hgetall(hash, std::inserter(map, map.end()), withDefaultValue); if (map.empty()) { diff --git a/common/dbinterface.h b/common/dbinterface.h index a1fcf2a2b..e3967f301 100644 --- a/common/dbinterface.h +++ b/common/dbinterface.h @@ -38,9 +38,9 @@ class DBInterface // Delete all keys which match %pattern from DB void delete_all_by_pattern(const std::string& dbName, const std::string& pattern); bool exists(const std::string& dbName, const std::string& key); - std::shared_ptr get(const std::string& dbName, const std::string& hash, const std::string& key, bool blocking = false); + std::shared_ptr get(const std::string& dbName, const std::string& hash, const std::string& key, bool withDefaultValue, bool blocking); // blocking = false bool hexists(const std::string& dbName, const std::string& hash, const std::string& key); - std::map get_all(const std::string& dbName, const std::string& hash, bool blocking = false); + std::map get_all(const std::string& dbName, const std::string& hash, bool withDefaultValue, bool blocking);//blocking = false std::vector keys(const std::string& dbName, const char *pattern = "*", bool blocking = false); std::pair> scan(const std::string& db_name, int cursor, const char *match, uint32_t count); int64_t publish(const std::string& dbName, const std::string& channel, const std::string& message); diff --git a/common/defaultvalueprovider.cpp b/common/defaultvalueprovider.cpp index c08f6d1ce..9460c77ca 100644 --- a/common/defaultvalueprovider.cpp +++ b/common/defaultvalueprovider.cpp @@ -21,15 +21,12 @@ using namespace swss; #include #define TRACE_STACK_SIZE 30 [[noreturn]] void sigv_handler(int sig) { - void *stack_info[TRACE_STACK_SIZE]; - size_t stack_info_size; - - // get void*'s for all entries on the stack - stack_info_size = backtrace(stack_info, TRACE_STACK_SIZE); + void *stackInfo[TRACE_STACK_SIZE]; + size_t stackInfoSize = backtrace(stackInfo, TRACE_STACK_SIZE); // print out all the frames to stderr cerr << "Error: signal " << sig << ":\n" >> endl; - backtrace_symbols_fd(array, (int)size, STDERR_FILENO); + backtrace_symbols_fd(stackInfo, (int)stackInfoSize, STDERR_FILENO); exit(1); } #endif @@ -51,115 +48,115 @@ std::shared_ptr TableInfoBase::GetDefaultValue(std::string row, std assert(!string::empty(field)); SWSS_LOG_DEBUG("TableInfoBase::GetDefaultValue %s %s\n", row.c_str(), field.c_str()); - FieldDefaultValueMapping *field_mapping_ptr; - if (!FindFieldMappingByKey(row, &field_mapping_ptr)) { + FieldDefaultValueMapping *fieldMappingPtr; + if (!FindFieldMappingByKey(row, &fieldMappingPtr)) { SWSS_LOG_DEBUG("Can't found default value mapping for row %s\n", row.c_str()); return nullptr; } - auto field_data = field_mapping_ptr->find(field); - if (field_data == field_mapping_ptr->end()) + auto fieldData = fieldMappingPtr->find(field); + if (fieldData == fieldMappingPtr->end()) { SWSS_LOG_DEBUG("Can't found default value for field %s\n", field.c_str()); return nullptr; } - SWSS_LOG_DEBUG("Found default value for field %s=%s\n", field.c_str(), field_data->second.c_str()); - return std::make_shared(field_data->second); + SWSS_LOG_DEBUG("Found default value for field %s=%s\n", field.c_str(), fieldData->second.c_str()); + return std::make_shared(fieldData->second); } -// existed_values and target_values can be same container. -void TableInfoBase::AppendDefaultValues(string row, std::map& existed_values, map& target_values) +// existedValues and targetValues can be same container. +void TableInfoBase::AppendDefaultValues(string row, FieldValueMapping& existedValues, FieldValueMapping& targetValues) { assert(!string::empty(row)); SWSS_LOG_DEBUG("TableInfoBase::AppendDefaultValues %s\n", row.c_str()); - FieldDefaultValueMapping *field_mapping_ptr; - if (!FindFieldMappingByKey(row, &field_mapping_ptr)) { + FieldDefaultValueMapping *fieldMappingPtr; + if (!FindFieldMappingByKey(row, &fieldMappingPtr)) { SWSS_LOG_DEBUG("Can't found default value mapping for row %s\n", row.c_str()); return; } - for (auto &default_value : *field_mapping_ptr) + for (auto &defaultValue : *fieldMappingPtr) { - auto field_data = existed_values.find(default_value.first); - if (field_data != existed_values.end()) + auto fieldData = existedValues.find(defaultValue.first); + if (fieldData != existedValues.end()) { - // ignore when a field already has value in existed_values + // ignore when a field already has value in existedValues continue; } - SWSS_LOG_DEBUG("Append default value: %s=%s\n",default_value.first.c_str(), default_value.second.c_str()); - target_values.emplace(default_value.first, default_value.second); + SWSS_LOG_DEBUG("Append default value: %s=%s\n",defaultValue.first.c_str(), defaultValue.second.c_str()); + targetValues.emplace(defaultValue.first, defaultValue.second); } } -TableInfoDict::TableInfoDict(KeyInfoToDefaultValueInfoMapping &field_info_mapping) +TableInfoDict::TableInfoDict(KeyInfoToDefaultValueInfoMapping &fieldInfoMapping) { - for (auto& field_mapping : field_info_mapping) + for (auto& fieldMapping : fieldInfoMapping) { // KeyInfo.first is key value - string key_value = field_mapping.first.first; - m_default_value_mapping.emplace(key_value, field_mapping.second); + string keyValue = fieldMapping.first.first; + m_defaultValueMapping.emplace(keyValue, fieldMapping.second); } } -bool TableInfoDict::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** founded_mapping_ptr) +bool TableInfoDict::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** foundedMappingPtr) { assert(!string::empty(row)); - assert(founded_mapping_ptr != nullptr); + assert(foundedMappingPtr != nullptr); SWSS_LOG_DEBUG("TableInfoDict::FindFieldMappingByKey %s\n", row.c_str()); - auto key_result = m_default_value_mapping.find(row); - *founded_mapping_ptr = key_result->second.get(); - return key_result == m_default_value_mapping.end(); + auto keyResult = m_defaultValueMapping.find(row); + *foundedMappingPtr = keyResult->second.get(); + return keyResult == m_defaultValueMapping.end(); } -TableInfoSingleList::TableInfoSingleList(KeyInfoToDefaultValueInfoMapping &field_info_mapping) +TableInfoSingleList::TableInfoSingleList(KeyInfoToDefaultValueInfoMapping &fieldInfoMapping) { - m_default_value_mapping = field_info_mapping.begin()->second; + m_defaultValueMapping = fieldInfoMapping.begin()->second; } -bool TableInfoSingleList::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** founded_mapping_ptr) +bool TableInfoSingleList::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** foundedMappingPtr) { assert(!string::empty(row)); - assert(founded_mapping_ptr != nullptr); + assert(foundedMappingPtr != nullptr); SWSS_LOG_DEBUG("TableInfoSingleList::FindFieldMappingByKey %s\n", row.c_str()); - *founded_mapping_ptr = m_default_value_mapping.get(); + *foundedMappingPtr = m_defaultValueMapping.get(); return true; } -TableInfoMultipleList::TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &field_info_mapping) +TableInfoMultipleList::TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &fieldInfoMapping) { - for (auto& field_mapping : field_info_mapping) + for (auto& fieldMapping : fieldInfoMapping) { // KeyInfo.second is field count - int field_count = field_mapping.first.second; - m_default_value_mapping.emplace(field_count, field_mapping.second); + int fieldCount = fieldMapping.first.second; + m_defaultValueMapping.emplace(fieldCount, fieldMapping.second); } } -bool TableInfoMultipleList::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** founded_mapping_ptr) +bool TableInfoMultipleList::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** foundedMappingPtr) { assert(!string::empty(row)); - assert(founded_mapping_ptr != nullptr); + assert(foundedMappingPtr != nullptr); SWSS_LOG_DEBUG("TableInfoMultipleList::FindFieldMappingByKey %s\n", row.c_str()); - int field_count = (int)std::count(row.begin(), row.end(), '|') + 1; - auto key_info = m_default_value_mapping.find(field_count); + int fieldCount = (int)std::count(row.begin(), row.end(), '|') + 1; + auto keyInfo = m_defaultValueMapping.find(fieldCount); // when not found, key_info still a valied iterator - *founded_mapping_ptr = key_info->second.get(); + *foundedMappingPtr = keyInfo->second.get(); // return false when not found - return key_info != m_default_value_mapping.end(); + return keyInfo != m_defaultValueMapping.end(); } DefaultValueProvider& DefaultValueProvider::Instance() { static DefaultValueProvider instance; - if (instance.context == nullptr) + if (instance.m_context == nullptr) { instance.Initialize(); } @@ -172,14 +169,14 @@ shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string assert(!string::empty(table)); SWSS_LOG_DEBUG("DefaultValueProvider::FindDefaultValueInfo %s\n", table.c_str()); - auto find_result = m_default_value_mapping.find(table); - if (find_result == m_default_value_mapping.end()) + auto findResult = m_defaultValueMapping.find(table); + if (findResult == m_defaultValueMapping.end()) { SWSS_LOG_DEBUG("Not found default value info for %s\n", table.c_str()); return nullptr; } - return find_result->second; + return findResult->second; } std::shared_ptr DefaultValueProvider::GetDefaultValue(std::string table, std::string row, std::string field) @@ -196,14 +193,14 @@ std::shared_ptr DefaultValueProvider::GetDefaultValue(std::string t } #endif - auto default_value_info = FindDefaultValueInfo(table); - if (default_value_info == nullptr) + auto defaultValueInfo = FindDefaultValueInfo(table); + if (defaultValueInfo == nullptr) { SWSS_LOG_DEBUG("Not found default value info for %s\n", table.c_str()); return nullptr; } - return default_value_info->GetDefaultValue(row, field); + return defaultValueInfo->GetDefaultValue(row, field); } void DefaultValueProvider::AppendDefaultValues(std::string table, std::string row, std::vector > &values) @@ -219,29 +216,29 @@ void DefaultValueProvider::AppendDefaultValues(std::string table, std::string ro } #endif - auto default_value_info = FindDefaultValueInfo(table); - if (default_value_info == nullptr) + auto defaultValueInfo = FindDefaultValueInfo(table); + if (defaultValueInfo == nullptr) { SWSS_LOG_DEBUG("Not found default value info for %s\n", table.c_str()); return; } - map existed_values; - map default_values; - for (auto& field_value_pair : values) + map existedValues; + map defaultValues; + for (auto& fieldValuePair : values) { - existed_values.emplace(field_value_pair.first, field_value_pair.second); + existedValues.emplace(fieldValuePair.first, fieldValuePair.second); } - default_value_info->AppendDefaultValues(row, existed_values, default_values); + defaultValueInfo->AppendDefaultValues(row, existedValues, defaultValues); - for (auto& field_value_pair : default_values) + for (auto& fieldValuePair : defaultValues) { - values.emplace_back(field_value_pair.first, field_value_pair.second); + values.emplace_back(fieldValuePair.first, fieldValuePair.second); } } -void DefaultValueProvider::AppendDefaultValues(string table, string row, map& values) +void DefaultValueProvider::AppendDefaultValues(string table, string row, FieldValueMapping& values) { assert(!string::empty(table)); assert(!string::empty(row)); @@ -254,14 +251,14 @@ void DefaultValueProvider::AppendDefaultValues(string table, string row, mapAppendDefaultValues(row, values, values); + defaultValueInfo->AppendDefaultValues(row, values, values); } DefaultValueProvider::DefaultValueProvider() @@ -275,59 +272,59 @@ DefaultValueProvider::DefaultValueProvider() DefaultValueProvider::~DefaultValueProvider() { - if (context) + if (m_context) { // set private_destructor to NULL because no any private data - ly_ctx_destroy(context, NULL); + ly_ctx_destroy(m_context, NULL); } } -void DefaultValueProvider::Initialize(char* module_path) +void DefaultValueProvider::Initialize(char* modulePath) { - assert(module_path != nullptr && !string::empty(module_path)); - assert(context == nullptr); + assert(modulePath != nullptr && !string::empty(modulePath)); + assert(m_context == nullptr); - DIR *module_dir = opendir(module_path); - if (!module_dir) + DIR *moduleDir = opendir(modulePath); + if (!moduleDir) { - ThrowRunTimeError("Open Yang model path " + string(module_path) + " failed"); + ThrowRunTimeError("Open Yang model path " + string(modulePath) + " failed"); } - context = ly_ctx_new(module_path, LY_CTX_ALLIMPLEMENTED); - struct dirent *sub_dir; - while ((sub_dir = readdir(module_dir)) != NULL) + m_context = ly_ctx_new(modulePath, LY_CTX_ALLIMPLEMENTED); + struct dirent *subDir; + while ((subDir = readdir(moduleDir)) != NULL) { - if (sub_dir->d_type == DT_REG) + if (subDir->d_type == DT_REG) { - SWSS_LOG_DEBUG("file_name: %s\n", sub_dir->d_name); - string file_name(sub_dir->d_name); - int pos = (int)file_name.find(".yang"); - string module_name = file_name.substr(0, pos); + SWSS_LOG_DEBUG("file name: %s\n", subDir->d_name); + string fileName(subDir->d_name); + int pos = (int)fileName.find(".yang"); + string moduleName = fileName.substr(0, pos); const struct lys_module *module = ly_ctx_load_module( - context, - module_name.c_str(), + m_context, + moduleName.c_str(), EMPTY_STR); // Use EMPTY_STR to revision to load the latest revision if (module->data == NULL) { // Not every yang file should contains yang model - SWSS_LOG_WARN("Yang file %s does not contains model %s.\n", sub_dir->d_name, module_name.c_str()); + SWSS_LOG_WARN("Yang file %s does not contains model %s.\n", subDir->d_name, moduleName.c_str()); continue; } - struct lys_node *top_level_node = module->data; - while (top_level_node) + struct lys_node *topLevelNode = module->data; + while (topLevelNode) { - if (top_level_node->nodetype != LYS_CONTAINER) + if (topLevelNode->nodetype != LYS_CONTAINER) { - SWSS_LOG_DEBUG("ignore top level element %s, tyoe %d\n",top_level_node->name, top_level_node->nodetype); + SWSS_LOG_DEBUG("ignore top level element %s, tyoe %d\n",topLevelNode->name, topLevelNode->nodetype); // Config DB table schema is defined by top level container - top_level_node = top_level_node->next; + topLevelNode = topLevelNode->next; continue; } - SWSS_LOG_DEBUG("top level container: %s\n",top_level_node->name); - auto container = top_level_node->child; + SWSS_LOG_DEBUG("top level container: %s\n",topLevelNode->name); + auto container = topLevelNode->child; while (container) { SWSS_LOG_DEBUG("container name: %s\n",container->name); @@ -336,82 +333,82 @@ void DefaultValueProvider::Initialize(char* module_path) container = container->next; } - top_level_node = top_level_node->next; + topLevelNode = topLevelNode->next; } } } - closedir(module_dir); + closedir(moduleDir); } -std::shared_ptr DefaultValueProvider::GetKeyInfo(struct lys_node* table_child_node) +std::shared_ptr DefaultValueProvider::GetKeyInfo(struct lys_node* tableChildNode) { - assert(table_child_node != nullptr); - SWSS_LOG_DEBUG("DefaultValueProvider::GetKeyInfo %s\n",table_child_node->name); + assert(tableChildNode != nullptr); + SWSS_LOG_DEBUG("DefaultValueProvider::GetKeyInfo %s\n",tableChildNode->name); - int key_field_count = 0; - string key_value = ""; - if (table_child_node->nodetype == LYS_LIST) + int keyFieldCount = 0; + string keyValue = ""; + if (tableChildNode->nodetype == LYS_LIST) { - SWSS_LOG_DEBUG("Child list: %s\n",table_child_node->name); + SWSS_LOG_DEBUG("Child list: %s\n",tableChildNode->name); // when a top level container contains list, the key defined by the 'keys' field. - struct lys_node_list *list_node = (struct lys_node_list*)table_child_node; - string key(list_node->keys_str); - key_field_count = (int)std::count(key.begin(), key.end(), '|') + 1; + struct lys_node_list *listNode = (struct lys_node_list*)tableChildNode; + string key(listNode->keys_str); + keyFieldCount = (int)std::count(key.begin(), key.end(), '|') + 1; } - else if (table_child_node->nodetype == LYS_CONTAINER) + else if (tableChildNode->nodetype == LYS_CONTAINER) { - SWSS_LOG_DEBUG("Child container name: %s\n",table_child_node->name); + SWSS_LOG_DEBUG("Child container name: %s\n",tableChildNode->name); // when a top level container not contains any list, the key is child container name - key_value = string(table_child_node->name); + keyValue = string(tableChildNode->name); } else { - SWSS_LOG_DEBUG("Ignore child element: %s\n",table_child_node->name); + SWSS_LOG_DEBUG("Ignore child element: %s\n",tableChildNode->name); return nullptr; } - return make_shared(key_value, key_field_count); + return make_shared(keyValue, keyFieldCount); } -FieldDefaultValueMappingPtr DefaultValueProvider::GetDefaultValueInfo(struct lys_node* table_child_node) +FieldDefaultValueMappingPtr DefaultValueProvider::GetDefaultValueInfo(struct lys_node* tableChildNode) { - assert(table_child_node != nullptr); - SWSS_LOG_DEBUG("DefaultValueProvider::GetDefaultValueInfo %s\n",table_child_node->name); + assert(tableChildNode != nullptr); + SWSS_LOG_DEBUG("DefaultValueProvider::GetDefaultValueInfo %s\n",tableChildNode->name); - auto field = table_child_node->child; - auto field_mapping = make_shared(); + auto field = tableChildNode->child; + auto fieldMapping = make_shared(); while (field) { if (field->nodetype == LYS_LEAF) { SWSS_LOG_DEBUG("leaf field: %s\n",field->name); - struct lys_node_leaf *leaf_node = (struct lys_node_leaf*)field; - if (leaf_node->dflt) + struct lys_node_leaf *leafNode = (struct lys_node_leaf*)field; + if (leafNode->dflt) { - SWSS_LOG_DEBUG("field: %s, default: %s\n",leaf_node->name, leaf_node->dflt); - (*field_mapping)[string(leaf_node->name)] = string(leaf_node->dflt); + SWSS_LOG_DEBUG("field: %s, default: %s\n",leafNode->name, leafNode->dflt); + fieldMapping->emplace(string(leafNode->name), string(leafNode->dflt)); } } else if (field->nodetype == LYS_CHOICE) { SWSS_LOG_DEBUG("choice field: %s\n",field->name); - struct lys_node_choice *choice_node = (struct lys_node_choice *)field; - if (choice_node->dflt) + struct lys_node_choice *choiceNode = (struct lys_node_choice *)field; + if (choiceNode->dflt) { // TODO: convert choice default value to string - SWSS_LOG_DEBUG("choice field: %s, default: TBD\n",choice_node->name); + SWSS_LOG_DEBUG("choice field: %s, default: TBD\n",choiceNode->name); } } else if (field->nodetype == LYS_LEAFLIST) { SWSS_LOG_DEBUG("list field: %s\n",field->name); - struct lys_node_leaflist *list_node = (struct lys_node_leaflist *)field; - if (list_node->dflt) + struct lys_node_leaflist *listNode = (struct lys_node_leaflist *)field; + if (listNode->dflt) { // TODO: convert list default value to json string - SWSS_LOG_DEBUG("list field: %s, default: TBD\n",list_node->name); + SWSS_LOG_DEBUG("list field: %s, default: TBD\n",listNode->name); } } #ifdef DEBUG @@ -424,37 +421,37 @@ FieldDefaultValueMappingPtr DefaultValueProvider::GetDefaultValueInfo(struct lys field = field->next; } - return field_mapping; + return fieldMapping; } -int DefaultValueProvider::BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping &field_info_mapping) +int DefaultValueProvider::BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping &fieldInfoMapping) { assert(table != nullptr); - int child_list_count = 0; - auto next_child = table->child; - while (next_child) + int childListCount = 0; + auto nextChild = table->child; + while (nextChild) { // get key from schema - auto keyInfo = GetKeyInfo(next_child); + auto keyInfo = GetKeyInfo(nextChild); if (keyInfo == nullptr) { - next_child = next_child->next; + nextChild = nextChild->next; continue; } else if (keyInfo->second != 0) { // when key field count not 0, it's a list node. - child_list_count++; + childListCount++; } // get field name to default value mappings from schema - field_info_mapping.emplace(*keyInfo, GetDefaultValueInfo(next_child)); + fieldInfoMapping.emplace(*keyInfo, GetDefaultValueInfo(nextChild)); - next_child = next_child->next; + nextChild = nextChild->next; } - return child_list_count; + return childListCount; } // Load default value info from yang model and append to default value mapping @@ -463,33 +460,40 @@ void DefaultValueProvider::AppendTableInfoToMapping(struct lys_node* table) assert(table != nullptr); SWSS_LOG_DEBUG("DefaultValueProvider::AppendTableInfoToMapping table name: %s\n",table->name); - KeyInfoToDefaultValueInfoMapping field_info_mapping; - int list_count = BuildFieldMappingList(table, field_info_mapping); + KeyInfoToDefaultValueInfoMapping fieldInfoMapping; + int listCount = BuildFieldMappingList(table, fieldInfoMapping); // create container data by list count - TableInfoBase* table_info_ptr = nullptr; - if (list_count == 0) - { - table_info_ptr = new TableInfoDict(field_info_mapping); - } - else if (list_count == 1) + TableInfoBase* tableInfoPtr = nullptr; + switch (listCount) { - table_info_ptr = new TableInfoSingleList(field_info_mapping); - } - else - { - table_info_ptr = new TableInfoMultipleList(field_info_mapping); + case 0: + { + tableInfoPtr = new TableInfoDict(fieldInfoMapping); + } + break; + + case 1: + { + tableInfoPtr = new TableInfoSingleList(fieldInfoMapping); + } + break; + + default: + { + tableInfoPtr = new TableInfoMultipleList(fieldInfoMapping); + } + break; } - string table_name(table->name); - m_default_value_mapping.emplace(table_name, shared_ptr(table_info_ptr)); + m_defaultValueMapping.emplace(string(table->name), shared_ptr(tableInfoPtr)); } #ifdef DEBUG bool DefaultValueProvider::FeatureEnabledByEnvironmentVariable() { - const char* show_default = getenv("CONFIG_DB_DEFAULT_VALUE"); - if (show_default == nullptr || strcmp(show_default, "TRUE") != 0) + const char* showDefault = getenv("CONFIG_DB_DEFAULT_VALUE"); + if (showDefault == nullptr || strcmp(showDefault, "TRUE") != 0) { // enable feature with "export CONFIG_DB_DEFAULT_VALUE=TRUE" SWSS_LOG_DEBUG("enable feature with \"export CONFIG_DB_DEFAULT_VALUE=TRUE\"\n"); diff --git a/common/defaultvalueprovider.h b/common/defaultvalueprovider.h index b75018f41..e80ad9d0f 100644 --- a/common/defaultvalueprovider.h +++ b/common/defaultvalueprovider.h @@ -13,6 +13,7 @@ typedef std::pair KeyInfo; // Field name to default value mapping typedef std::map FieldDefaultValueMapping; +typedef std::map FieldValueMapping; typedef std::shared_ptr FieldDefaultValueMappingPtr; // Key info to default value info mapping @@ -25,48 +26,48 @@ class TableInfoBase public: TableInfoBase(); - void AppendDefaultValues(std::string row, std::map& source_values, std::map& target_values); + void AppendDefaultValues(std::string row, FieldValueMapping& sourceValues, FieldValueMapping& targetValues); std::shared_ptr GetDefaultValue(std::string row, std::string field); protected: - virtual bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** founded_mapping_ptr) = 0; + virtual bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** foundedMappingPtr) = 0; }; class TableInfoDict : public TableInfoBase { public: - TableInfoDict(KeyInfoToDefaultValueInfoMapping &field_info_mapping); + TableInfoDict(KeyInfoToDefaultValueInfoMapping &fieldInfoMapping); private: // Mapping: key value -> field -> default - std::map m_default_value_mapping; + std::map m_defaultValueMapping; - bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** founded_mapping_ptr); + bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** foundedMappingPtr); }; class TableInfoSingleList : public TableInfoBase { public: - TableInfoSingleList(KeyInfoToDefaultValueInfoMapping &field_info_mapping); + TableInfoSingleList(KeyInfoToDefaultValueInfoMapping &fieldInfoMapping); private: // Mapping: field -> default - FieldDefaultValueMappingPtr m_default_value_mapping; + FieldDefaultValueMappingPtr m_defaultValueMapping; - bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** founded_mapping_ptr); + bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** foundedMappingPtr); }; struct TableInfoMultipleList : public TableInfoBase { public: - TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &field_info_mapping); + TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &fieldInfoMapping); private: // Mapping: key field count -> field -> default - std::map m_default_value_mapping; + std::map m_defaultValueMapping; - bool FindFieldMappingByKey(std::string row, std::map ** founded_mapping_ptr); + bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** foundedMappingPtr); }; class DefaultValueProvider @@ -74,7 +75,7 @@ class DefaultValueProvider public: static DefaultValueProvider& Instance(); - void AppendDefaultValues(std::string table, std::string row, std::map& values); + void AppendDefaultValues(std::string table, std::string row, FieldValueMapping& values); void AppendDefaultValues(std::string table, std::string row, std::vector > &values); @@ -85,22 +86,22 @@ class DefaultValueProvider ~DefaultValueProvider(); // libyang context - struct ly_ctx *context = nullptr; + struct ly_ctx *m_context = nullptr; // The table name to table default value info mapping - std::map > m_default_value_mapping; + std::map > m_defaultValueMapping; - void Initialize(char* module_path = DEFAULT_YANG_MODULE_PATH); + void Initialize(char* modulePath = DEFAULT_YANG_MODULE_PATH); // Load default value info from yang model and append to default value mapping void AppendTableInfoToMapping(struct lys_node* table); std::shared_ptr FindDefaultValueInfo(std::string table); - int BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping& field_mapping_list); + int BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping& fieldMappingList); std::shared_ptr GetKeyInfo(struct lys_node* table_child_node); - FieldDefaultValueMappingPtr GetDefaultValueInfo(struct lys_node* table_child_node); + FieldDefaultValueMappingPtr GetDefaultValueInfo(struct lys_node* tableChildNode); }; } diff --git a/common/logger.cpp b/common/logger.cpp index b27cbed93..cf0af00ac 100644 --- a/common/logger.cpp +++ b/common/logger.cpp @@ -125,8 +125,8 @@ void Logger::linkToDbWithOutput( std::string key = dbName + ":" + dbName; std::string prio, output; bool doUpdate = false; - auto prioPtr = db.hget(key, DAEMON_LOGLEVEL); - auto outputPtr = db.hget(key, DAEMON_LOGOUTPUT); + auto prioPtr = db.hget(key, DAEMON_LOGLEVEL, false); + auto outputPtr = db.hget(key, DAEMON_LOGOUTPUT, false); if (prioPtr == nullptr) { diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 1be85c099..e18b03d08 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -134,7 +134,7 @@ int main(int argc, char **argv) for (const auto& key : keys) { const auto redis_key = std::string(key).append(":").append(key); - auto level = db.hget(redis_key, DAEMON_LOGLEVEL); + auto level = db.hget(redis_key, DAEMON_LOGLEVEL, false); if (nullptr == level) { std::cerr << std::left << std::setw(30) << key << "Unknown log level" << std::endl; diff --git a/common/redisclient.h b/common/redisclient.h index e95d68396..4898e537e 100644 --- a/common/redisclient.h +++ b/common/redisclient.h @@ -32,7 +32,7 @@ class RedisClient int64_t hdel(const std::string &key, const std::vector &fields) { return m_db->hdel(key, fields); } - std::unordered_map hgetall(const std::string &key) { return m_db->hgetall(key); } + std::unordered_map hgetall(const std::string &key, bool withDefaultValue) { return m_db->hgetall(key, withDefaultValue); } template void hgetall(const std::string &key, OutputIterator result) { return m_db->hgetall(key, result); } @@ -48,7 +48,7 @@ class RedisClient std::shared_ptr get(const std::string &key) { return m_db->get(key); } - std::shared_ptr hget(const std::string &key, const std::string &field) { return m_db->hget(key, field); } + std::shared_ptr hget(const std::string &key, const std::string &field, bool withDefaultValue) { return m_db->hget(key, field, withDefaultValue); } int64_t incr(const std::string &key) { return m_db->incr(key); } diff --git a/common/sonicv2connector.cpp b/common/sonicv2connector.cpp index 59552cde2..33d4ee1bf 100644 --- a/common/sonicv2connector.cpp +++ b/common/sonicv2connector.cpp @@ -74,9 +74,9 @@ std::pair> SonicV2Connector_Native::scan(const std return m_dbintf.scan(db_name, cursor, match, count); } -std::shared_ptr SonicV2Connector_Native::get(const std::string& db_name, const std::string& _hash, const std::string& key, bool blocking) +std::shared_ptr SonicV2Connector_Native::get(const std::string& db_name, const std::string& _hash, const std::string& key, bool withDefaultValue, bool blocking) { - return m_dbintf.get(db_name, _hash, key, blocking); + return m_dbintf.get(db_name, _hash, key, withDefaultValue, blocking); } bool SonicV2Connector_Native::hexists(const std::string& db_name, const std::string& _hash, const std::string& key) @@ -84,9 +84,9 @@ bool SonicV2Connector_Native::hexists(const std::string& db_name, const std::str return m_dbintf.hexists(db_name, _hash, key); } -std::map SonicV2Connector_Native::get_all(const std::string& db_name, const std::string& _hash, bool blocking) +std::map SonicV2Connector_Native::get_all(const std::string& db_name, const std::string& _hash, bool withDefaultValue, bool blocking) { - return m_dbintf.get_all(db_name, _hash, blocking); + return m_dbintf.get_all(db_name, _hash, withDefaultValue, blocking); } void SonicV2Connector_Native::hmset(const std::string& db_name, const std::string &key, const std::map &values) diff --git a/common/sonicv2connector.h b/common/sonicv2connector.h index 541b5835f..79caf56dd 100644 --- a/common/sonicv2connector.h +++ b/common/sonicv2connector.h @@ -35,11 +35,11 @@ class SonicV2Connector_Native std::pair> scan(const std::string& db_name, int cursor = 0, const char *match = "", uint32_t count = 10); - std::shared_ptr get(const std::string& db_name, const std::string& _hash, const std::string& key, bool blocking=false); + std::shared_ptr get(const std::string& db_name, const std::string& _hash, const std::string& key, bool withDefaultValue, bool blocking);//blocking=false bool hexists(const std::string& db_name, const std::string& _hash, const std::string& key); - std::map get_all(const std::string& db_name, const std::string& _hash, bool blocking=false); + std::map get_all(const std::string& db_name, const std::string& _hash, bool withDefaultValue, bool blocking);//blocking=false void hmset(const std::string& db_name, const std::string &key, const std::map &values); diff --git a/tests/fdb_flush.cpp b/tests/fdb_flush.cpp index 7ba981c0d..0797b0917 100644 --- a/tests/fdb_flush.cpp +++ b/tests/fdb_flush.cpp @@ -80,7 +80,7 @@ void print() { printf("K %s\n", k.c_str()); - auto hash = db.hgetall(k); + auto hash = db.hgetall(k, false); for (auto&h: hash) { diff --git a/tests/logger_ut.cpp b/tests/logger_ut.cpp index c17c9c262..7b6e4acc8 100644 --- a/tests/logger_ut.cpp +++ b/tests/logger_ut.cpp @@ -32,7 +32,7 @@ void prioNotify(const string &component, const string &prioStr) void checkLoglevel(DBConnector& db, const string& key, const string& loglevel) { string redis_key = key + ":" + key; - auto level = db.hget(redis_key, DAEMON_LOGLEVEL); + auto level = db.hget(redis_key, DAEMON_LOGLEVEL, false); EXPECT_FALSE(level == nullptr); if (level != nullptr) { diff --git a/tests/redis_state_ut.cpp b/tests/redis_state_ut.cpp index 3d965103b..46f07844a 100644 --- a/tests/redis_state_ut.cpp +++ b/tests/redis_state_ut.cpp @@ -122,7 +122,7 @@ static void consumerWorker(int index) for (auto fv : kfvFieldsValues(kco)) { - string val = *db.hget(tableName + ":" + kfvKey(kco), fvField(fv)); + string val = *db.hget(tableName + ":" + kfvKey(kco), fvField(fv), false); EXPECT_EQ(val, fvValue(fv)); } } else if (kfvOp(kco) == "DEL") diff --git a/tests/redis_ut.cpp b/tests/redis_ut.cpp index dd6292f0c..482f305e2 100644 --- a/tests/redis_ut.cpp +++ b/tests/redis_ut.cpp @@ -322,7 +322,7 @@ TEST(DBConnector, DBInterface) SonicV2Connector_Native db; db.connect("TEST_DB"); db.set("TEST_DB", "key0", "field1", "value2"); - auto fvs = db.get_all("TEST_DB", "key0"); + auto fvs = db.get_all("TEST_DB", "key0", false, false); auto rc = fvs.find("field1"); EXPECT_NE(rc, fvs.end()); EXPECT_EQ(rc->second, "value2"); @@ -368,7 +368,7 @@ TEST(DBConnector, RedisClient) for (auto k : keys) { cout << "Get key [" << k << "]" << flush; - auto fvs = db.hgetall(k); + auto fvs = db.hgetall(k, false); unsigned int size_v = 3; EXPECT_EQ(fvs.size(), size_v); for (auto fv: fvs) @@ -392,7 +392,7 @@ TEST(DBConnector, RedisClient) int64_t rval = db.hdel(key_1, "field_2"); EXPECT_EQ(rval, 1); - auto fvs = db.hgetall(key_1); + auto fvs = db.hgetall(key_1, false); EXPECT_EQ(fvs.size(), 2); for (auto fv: fvs) { @@ -417,9 +417,9 @@ TEST(DBConnector, RedisClient) cout << "- Step 6. GET" << endl; cout << "Get key [a] and key [b]" << endl; - fvs = db.hgetall(key_1); + fvs = db.hgetall(key_1, false); EXPECT_TRUE(fvs.empty()); - fvs = db.hgetall(key_2); + fvs = db.hgetall(key_2, false); cout << "Get key [b]" << flush; for (auto fv: fvs) @@ -442,7 +442,7 @@ TEST(DBConnector, RedisClient) rval = db.hdel(key_2, vector({"field_2", "field_3"})); EXPECT_EQ(rval, 2); - fvs = db.hgetall(key_2); + fvs = db.hgetall(key_2, false); EXPECT_EQ(fvs.size(), 1); for (auto fv: fvs) { @@ -461,7 +461,7 @@ TEST(DBConnector, RedisClient) cout << "- Step 8. DEL and GET_TABLE_CONTENT" << endl; cout << "Delete key [b]" << endl; db.del(key_2); - fvs = db.hgetall(key_2); + fvs = db.hgetall(key_2, false); EXPECT_TRUE(fvs.empty()); From f19c55746d55db0e60225ef1d5bcab47581f1d2b Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 18 Apr 2022 05:01:28 +0000 Subject: [PATCH 15/24] Fix all API with new parameter --- common/configdb.cpp | 16 ++++++++++++++-- common/dbconnector.h | 20 ++++++++++---------- common/subscriberstatetable.cpp | 7 ++++--- common/subscriberstatetable.h | 3 ++- common/table.cpp | 8 ++++---- common/table.h | 6 +++--- tests/redis_multi_ns_ut.cpp | 8 ++++---- tests/redis_piped_ut.cpp | 8 ++++---- tests/redis_state_ut.cpp | 2 +- tests/redis_subscriber_state_ut.cpp | 16 ++++++++-------- tests/redis_ut.cpp | 8 ++++---- tests/selectable_priority.cpp | 2 +- 12 files changed, 59 insertions(+), 45 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index 1f0223de3..6e7992db8 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -187,7 +187,13 @@ map> ConfigDBConnector_Native::get_table(string tabl } row = key.substr(pos + 1); - DefaultValueProvider::Instance().AppendDefaultValues(table, row, const_cast& >(entry)); + /* + if (withDefaultValue) + { + // TODO: [Hua] check if following code can be removed because hgetall already return default values. + DefaultValueProvider::Instance().AppendDefaultValues(table, row, const_cast& >(entry)); + } + */ data[row] = entry; } @@ -263,7 +269,13 @@ map>> ConfigDBConnector_Native::get_conf if (!entry.empty()) { - DefaultValueProvider::Instance().AppendDefaultValues(table_name, row, const_cast& >(entry)); + /* + if (withDefaultValue) + { + // TODO: [Hua] check if code can remove because hgetall already return default values. + DefaultValueProvider::Instance().AppendDefaultValues(table_name, row, const_cast& >(entry)); + } + */ data[table_name][row] = entry; } diff --git a/common/dbconnector.h b/common/dbconnector.h index 988fccfa8..70c0e1249 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -281,7 +281,7 @@ void DBConnector::hgetall(const std::string &key, OutputIterator result, bool wi ++result; } - if (this->getDbId() == CONFIG_DB) + if (withDefaultValue && this->getDbId() == CONFIG_DB) { size_t pos = key.find("|"); if (pos == std::string::npos) @@ -292,22 +292,22 @@ void DBConnector::hgetall(const std::string &key, OutputIterator result, bool wi // When DB ID is CONFIG_DB, append default value to config DB result. std::string table = key.substr(0, pos); std::string row = key.substr(pos + 1); - std::map values_with_default; - std::map existed_values; + std::map valuesWithDefault; + std::map existedValues; for (unsigned int i = 0; i < ctx->elements; i += 2) { - existed_values[ctx->element[i]->str] = ctx->element[i+1]->str; - values_with_default[ctx->element[i]->str] = ctx->element[i+1]->str; + existedValues[ctx->element[i]->str] = ctx->element[i+1]->str; + valuesWithDefault[ctx->element[i]->str] = ctx->element[i+1]->str; } - DefaultValueProvider::Instance().AppendDefaultValues(table, row, values_with_default); + DefaultValueProvider::Instance().AppendDefaultValues(table, row, valuesWithDefault); - for (auto& field_value_pair : values_with_default) + for (auto& fieldValuePair : valuesWithDefault) { - auto find_result = existed_values.find(field_value_pair.first); - if (find_result == existed_values.end()) + auto findResult = existedValues.find(fieldValuePair.first); + if (findResult == existedValues.end()) { - *result = std::make_pair(field_value_pair.first, field_value_pair.second); + *result = std::make_pair(fieldValuePair.first, fieldValuePair.second); ++result; } } diff --git a/common/subscriberstatetable.cpp b/common/subscriberstatetable.cpp index b80f24ef5..8db50d8e8 100644 --- a/common/subscriberstatetable.cpp +++ b/common/subscriberstatetable.cpp @@ -14,10 +14,11 @@ using namespace std; namespace swss { -SubscriberStateTable::SubscriberStateTable(DBConnector *db, const string &tableName, int popBatchSize, int pri) +SubscriberStateTable::SubscriberStateTable(DBConnector *db, const string &tableName, bool withDefaultValue, int popBatchSize, int pri) : ConsumerTableBase(db, tableName, popBatchSize, pri), m_table(db, tableName) { m_keyspace = "__keyspace@"; + m_withDefaultValue = withDefaultValue; m_keyspace += to_string(db->getDbId()) + "__:" + tableName + m_table.getTableNameSeparator() + "*"; @@ -33,7 +34,7 @@ SubscriberStateTable::SubscriberStateTable(DBConnector *db, const string &tableN kfvKey(kco) = key; kfvOp(kco) = SET_COMMAND; - if (!m_table.get(key, kfvFieldsValues(kco))) + if (!m_table.get(key, kfvFieldsValues(kco), m_withDefaultValue)) { continue; } @@ -147,7 +148,7 @@ void SubscriberStateTable::pops(deque &vkco, const strin } else { - if (!m_table.get(key, kfvFieldsValues(kco))) + if (!m_table.get(key, kfvFieldsValues(kco), m_withDefaultValue)) { SWSS_LOG_NOTICE("Miss table key %s, possibly outdated", table_entry.c_str()); continue; diff --git a/common/subscriberstatetable.h b/common/subscriberstatetable.h index 432dc90fb..5da8ea951 100644 --- a/common/subscriberstatetable.h +++ b/common/subscriberstatetable.h @@ -11,7 +11,7 @@ namespace swss { class SubscriberStateTable : public ConsumerTableBase { public: - SubscriberStateTable(DBConnector *db, const std::string &tableName, int popBatchSize = DEFAULT_POP_BATCH_SIZE, int pri = 0); + SubscriberStateTable(DBConnector *db, const std::string &tableName, bool withDefaultValue, int popBatchSize = DEFAULT_POP_BATCH_SIZE, int pri = 0); /* Get all elements available */ void pops(std::deque &vkco, const std::string &prefix = EMPTY_PREFIX); @@ -33,6 +33,7 @@ class SubscriberStateTable : public ConsumerTableBase std::deque> m_keyspace_event_buffer; Table m_table; + bool m_withDefaultValue; }; } diff --git a/common/table.cpp b/common/table.cpp index 45bfaf1f7..98a499a4c 100644 --- a/common/table.cpp +++ b/common/table.cpp @@ -64,7 +64,7 @@ void Table::flush() m_pipe->flush(); } -bool Table::get(const string &key, vector &values) +bool Table::get(const string &key, vector &values, bool withDefaultValue) { // [Hua] TODO: code here dupe with DBConnector::hgetall, check if can reuse it. RedisCommand hgetall_key; @@ -86,7 +86,7 @@ bool Table::get(const string &key, vector &values) reply->element[i + 1]->str); } - if (m_pipe->getDbId() == CONFIG_DB) + if (withDefaultValue && m_pipe->getDbId() == CONFIG_DB) { size_t pos = key.find("|"); if (pos == std::string::npos) @@ -204,7 +204,7 @@ void Table::hdel(const string &key, const string &field, const string& /* op */, m_pipe->push(cmd, REDIS_REPLY_INTEGER); } -void TableEntryEnumerable::getContent(vector &tuples) +void TableEntryEnumerable::getContent(vector &tuples, bool withDefaultValue) { vector keys; getKeys(keys); @@ -216,7 +216,7 @@ void TableEntryEnumerable::getContent(vector &tuples) vector values; string op = ""; - get(key, values); + get(key, values, withDefaultValue); tuples.emplace_back(key, op, values); } } diff --git a/common/table.h b/common/table.h index 04f25ee0e..e163da176 100644 --- a/common/table.h +++ b/common/table.h @@ -166,14 +166,14 @@ class TableEntryEnumerable { virtual ~TableEntryEnumerable() = default; /* Get all the field-value tuple of the table entry with the key */ - virtual bool get(const std::string &key, std::vector &values) = 0; + virtual bool get(const std::string &key, std::vector &values, bool withDefaultValue) = 0; /* get all the keys in the table */ virtual void getKeys(std::vector &keys) = 0; /* Read the whole table content from the DB directly */ /* NOTE: Not an atomic function */ - void getContent(std::vector &tuples); + void getContent(std::vector &tuples, bool withDefaultValue); }; /* The default time to live for a DB entry is infinite */ @@ -222,7 +222,7 @@ class Table : public TableBase, public TableEntryEnumerable { const std::string &prefix = EMPTY_PREFIX); /* Read a value from the DB directly */ /* Returns false if the key doesn't exists */ - virtual bool get(const std::string &key, std::vector &ovalues); + virtual bool get(const std::string &key, std::vector &ovalues, bool withDefaultValue); virtual bool hget(const std::string &key, const std::string &field, std::string &value); virtual void hset(const std::string &key, diff --git a/tests/redis_multi_ns_ut.cpp b/tests/redis_multi_ns_ut.cpp index 7c7cf2c08..d5cd2f77f 100644 --- a/tests/redis_multi_ns_ut.cpp +++ b/tests/redis_multi_ns_ut.cpp @@ -166,7 +166,7 @@ void TableNetnsTest(string tableName) cout << "- Step 3. GET_TABLE_CONTENT" << endl; vector tuples; - t.getContent(tuples); + t.getContent(tuples, false); cout << "Get total " << tuples.size() << " number of entries" << endl; EXPECT_EQ(tuples.size(), (size_t)2); @@ -198,8 +198,8 @@ void TableNetnsTest(string tableName) cout << "- Step 5. GET" << endl; cout << "Get key [a] and key [b]" << endl; - EXPECT_EQ(t.get(key_1, values), false); - t.get(key_2, values); + EXPECT_EQ(t.get(key_1, values, false), false); + t.get(key_2, values, false); cout << "Get key [b]" << flush; for (auto fv: values) @@ -220,7 +220,7 @@ void TableNetnsTest(string tableName) cout << "- Step 6. DEL and GET_TABLE_CONTENT" << endl; cout << "Delete key [b]" << endl; t.del(key_2); - t.getContent(tuples); + t.getContent(tuples, false); EXPECT_EQ(tuples.size(), unsigned(0)); diff --git a/tests/redis_piped_ut.cpp b/tests/redis_piped_ut.cpp index 29978cdff..80c7c22c1 100644 --- a/tests/redis_piped_ut.cpp +++ b/tests/redis_piped_ut.cpp @@ -349,7 +349,7 @@ TEST(Table, piped_test) cout << "- Step 2. GET_TABLE_CONTENT" << endl; vector tuples; - t.getContent(tuples); + t.getContent(tuples, false); cout << "Get total " << tuples.size() << " number of entries" << endl; EXPECT_EQ(tuples.size(), (size_t)2); @@ -381,8 +381,8 @@ TEST(Table, piped_test) cout << "- Step 4. GET" << endl; cout << "Get key [a] and key [b]" << endl; - EXPECT_EQ(t.get(key_1, values), false); - t.get(key_2, values); + EXPECT_EQ(t.get(key_1, values, false), false); + t.get(key_2, values, false); cout << "Get key [b]" << flush; for (auto fv: values) @@ -403,7 +403,7 @@ TEST(Table, piped_test) cout << "- Step 5. DEL and GET_TABLE_CONTENT" << endl; cout << "Delete key [b]" << endl; t.del(key_2); - t.getContent(tuples); + t.getContent(tuples, false); EXPECT_EQ(tuples.size(), unsigned(0)); diff --git a/tests/redis_state_ut.cpp b/tests/redis_state_ut.cpp index 46f07844a..b74bf8625 100644 --- a/tests/redis_state_ut.cpp +++ b/tests/redis_state_ut.cpp @@ -472,7 +472,7 @@ TEST(ConsumerStateTable, set_pop_del_set_pop_get) /* Get data directly from table in redis DB*/ Table t(&db, tableName); vector values; - t.get(key, values); + t.get(key, values, false); /* size of values should be maxNumOfFields, no "field 1" left from first set */ EXPECT_EQ(values.size(), (unsigned int)maxNumOfFields); diff --git a/tests/redis_subscriber_state_ut.cpp b/tests/redis_subscriber_state_ut.cpp index 34db9d48a..7425bd47a 100644 --- a/tests/redis_subscriber_state_ut.cpp +++ b/tests/redis_subscriber_state_ut.cpp @@ -123,7 +123,7 @@ static void producerWorker(int index) static void subscriberWorker(int index, int *status) { DBConnector db("TEST_DB", 0, true); - SubscriberStateTable c(&db, testTableName); + SubscriberStateTable c(&db, testTableName, false); Select cs; Selectable *selectcs; int numberOfKeysSet = 0; @@ -181,7 +181,7 @@ TEST(SubscriberStateTable, set) int maxNumOfFields = 2; /* Prepare subscriber */ - SubscriberStateTable c(&db, testTableName); + SubscriberStateTable c(&db, testTableName, false); Select cs; Selectable *selectcs; cs.addSelectable(&c); @@ -232,7 +232,7 @@ TEST(SubscriberStateTable, set2_pop1_set1_pop1) int maxNumOfFields = 2; /* Prepare subscriber */ - SubscriberStateTable c(&db, testTableName); + SubscriberStateTable c(&db, testTableName, false); Select cs; Selectable *selectcs; cs.addSelectable(&c); @@ -363,7 +363,7 @@ TEST(SubscriberStateTable, pops_intial) } /* Prepare subscriber */ - SubscriberStateTable c(&db, testTableName); + SubscriberStateTable c(&db, testTableName, false); Select cs; cs.addSelectable(&c); std::deque entries; @@ -408,7 +408,7 @@ TEST(SubscriberStateTable, del) int maxNumOfFields = 2; /* Prepare subscriber */ - SubscriberStateTable c(&db, testTableName); + SubscriberStateTable c(&db, testTableName, false); Select cs; Selectable *selectcs; cs.addSelectable(&c); @@ -475,7 +475,7 @@ TEST(SubscriberStateTable, table_state) } /* Prepare subscriber */ - SubscriberStateTable c(&db, testTableName); + SubscriberStateTable c(&db, testTableName, false); Select cs; Selectable *selectcs; int ret, i = 0; @@ -583,8 +583,8 @@ TEST(SubscriberStateTable, cachedData) } /* Prepare subscriber */ - SubscriberStateTable c1(&db, testTableName); - SubscriberStateTable c2(&db, testTableName2); + SubscriberStateTable c1(&db, testTableName, false); + SubscriberStateTable c2(&db, testTableName2, false); Select cs; Selectable *selectcs; cs.addSelectable(&c1); diff --git a/tests/redis_ut.cpp b/tests/redis_ut.cpp index 482f305e2..e5eac7f8e 100644 --- a/tests/redis_ut.cpp +++ b/tests/redis_ut.cpp @@ -202,7 +202,7 @@ void TableBasicTest(string tableName, bool useDbId = false) cout << "- Step 3. GET_TABLE_CONTENT" << endl; vector tuples; - t.getContent(tuples); + t.getContent(tuples, false); cout << "Get total " << tuples.size() << " number of entries" << endl; EXPECT_EQ(tuples.size(), (size_t)2); @@ -234,8 +234,8 @@ void TableBasicTest(string tableName, bool useDbId = false) cout << "- Step 5. GET" << endl; cout << "Get key [a] and key [b]" << endl; - EXPECT_EQ(t.get(key_1, values), false); - t.get(key_2, values); + EXPECT_EQ(t.get(key_1, values, false), false); + t.get(key_2, values, false); cout << "Get key [b]" << flush; for (auto fv: values) @@ -256,7 +256,7 @@ void TableBasicTest(string tableName, bool useDbId = false) cout << "- Step 6. DEL and GET_TABLE_CONTENT" << endl; cout << "Delete key [b]" << endl; t.del(key_2); - t.getContent(tuples); + t.getContent(tuples, false); EXPECT_EQ(tuples.size(), unsigned(0)); diff --git a/tests/selectable_priority.cpp b/tests/selectable_priority.cpp index 24fbd3760..778514558 100644 --- a/tests/selectable_priority.cpp +++ b/tests/selectable_priority.cpp @@ -32,7 +32,7 @@ TEST(Priority, default_pri_values) RedisSelect rs; SelectableEvent se; SelectableTimer st(interval); - SubscriberStateTable sst(&db, tableName); + SubscriberStateTable sst(&db, tableName, false); EXPECT_EQ(nl.getPri(), 0); EXPECT_EQ(cst.getPri(), 0); From 17bc7d119b3bba062b65afd40bd25db891f97e23 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 18 Apr 2022 06:38:51 +0000 Subject: [PATCH 16/24] Add deprecated attribute --- common/configdb.cpp | 41 ++++++++++++++++++++------------- common/configdb.h | 20 ++++++++++++++++ common/dbconnector.cpp | 5 ++++ common/dbconnector.h | 22 ++++++++++++++++++ common/dbinterface.cpp | 10 ++++++++ common/dbinterface.h | 12 ++++++++-- common/redisclient.h | 10 ++++++++ common/sonicv2connector.cpp | 10 ++++++++ common/sonicv2connector.h | 14 +++++++++-- common/subscriberstatetable.cpp | 5 ++++ common/subscriberstatetable.h | 4 ++++ common/table.cpp | 10 ++++++++ common/table.h | 12 ++++++++++ tests/selectable_priority.cpp | 2 +- 14 files changed, 156 insertions(+), 21 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index 6e7992db8..4140fcc75 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -122,6 +122,11 @@ void ConfigDBConnector_Native::mod_entry(string table, string key, const map ConfigDBConnector_Native::get_entry(string table, string key) +{ + return get_entry(table, key, false); +} + map ConfigDBConnector_Native::get_entry(string table, string key, bool withDefaultValue) { auto& client = get_redis_client(m_db_name); @@ -170,6 +175,11 @@ vector ConfigDBConnector_Native::get_keys(string table, bool split) // { 'row_key': {'column_key': value, ...}, ...} // or { ('l1_key', 'l2_key', ...): {'column_key': value, ...}, ...} for a multi-key table. // Empty dictionary if table does not exist. +map> ConfigDBConnector_Native::get_table(string table) +{ + return get_table(table, false); +} + map> ConfigDBConnector_Native::get_table(string table, bool withDefaultValue) { auto& client = get_redis_client(m_db_name); @@ -187,14 +197,6 @@ map> ConfigDBConnector_Native::get_table(string tabl } row = key.substr(pos + 1); - /* - if (withDefaultValue) - { - // TODO: [Hua] check if following code can be removed because hgetall already return default values. - DefaultValueProvider::Instance().AppendDefaultValues(table, row, const_cast& >(entry)); - } - */ - data[row] = entry; } @@ -252,6 +254,11 @@ void ConfigDBConnector_Native::mod_config(const map>> ConfigDBConnector_Native::get_config() +{ + return get_config(false); +} + map>> ConfigDBConnector_Native::get_config(bool withDefaultValue) { auto& client = get_redis_client(m_db_name); @@ -269,14 +276,6 @@ map>> ConfigDBConnector_Native::get_conf if (!entry.empty()) { - /* - if (withDefaultValue) - { - // TODO: [Hua] check if code can remove because hgetall already return default values. - DefaultValueProvider::Instance().AppendDefaultValues(table_name, row, const_cast& >(entry)); - } - */ - data[table_name][row] = entry; } } @@ -470,6 +469,11 @@ void ConfigDBPipeConnector_Native::mod_config(const map>>& data, int cursor) +{ + return _get_config(client, pipe, data, cursor, false); +} + int ConfigDBPipeConnector_Native::_get_config(DBConnector& client, RedisTransactioner& pipe, map>>& data, int cursor, bool withDefaultValue) { auto const& rc = client.scan(cursor, "*", REDIS_SCAN_BATCH_SIZE); @@ -520,6 +524,11 @@ int ConfigDBPipeConnector_Native::_get_config(DBConnector& client, RedisTransact return cur; } +map>> ConfigDBPipeConnector_Native::get_config() +{ + return get_config(false); +} + map>> ConfigDBPipeConnector_Native::get_config(bool withDefaultValue) { auto& client = get_redis_client(m_db_name); diff --git a/common/configdb.h b/common/configdb.h index 22666053c..ff2e8a641 100644 --- a/common/configdb.h +++ b/common/configdb.h @@ -19,11 +19,23 @@ class ConfigDBConnector_Native : public SonicV2Connector_Native virtual void set_entry(std::string table, std::string key, const std::map& data); virtual void mod_entry(std::string table, std::string key, const std::map& data); +#ifndef SWIG + [[deprecated("Please use get_entry(std::string table, std::string key, bool withDefaultValue) instead.")]] +#endif + std::map get_entry(std::string table, std::string key); std::map get_entry(std::string table, std::string key, bool withDefaultValue); std::vector get_keys(std::string table, bool split = true); +#ifndef SWIG + [[deprecated("Please use get_table(std::string table, bool withDefaultValue) instead.")]] +#endif + std::map> get_table(std::string table); std::map> get_table(std::string table, bool withDefaultValue); void delete_table(std::string table); virtual void mod_config(const std::map>>& data); +#ifndef SWIG + [[deprecated("Please use get_config(bool withDefaultValue) instead.")]] +#endif + virtual std::map>> get_config(); virtual std::map>> get_config(bool withDefaultValue); std::string getKeySeparator() const; @@ -246,6 +258,10 @@ class ConfigDBPipeConnector_Native: public ConfigDBConnector_Native void set_entry(std::string table, std::string key, const std::map& data) override; void mod_config(const std::map>>& data) override; +#ifndef SWIG + [[deprecated("Please use get_config(bool withDefaultValue) instead.")]] +#endif + std::map>> get_config() override; std::map>> get_config(bool withDefaultValue) override; private: @@ -255,6 +271,10 @@ class ConfigDBPipeConnector_Native: public ConfigDBConnector_Native void _delete_table(DBConnector& client, RedisTransactioner& pipe, std::string table); void _set_entry(RedisTransactioner& pipe, std::string table, std::string key, const std::map& data); void _mod_entry(RedisTransactioner& pipe, std::string table, std::string key, const std::map& data); +#ifndef SWIG + [[deprecated("Please use _get_config(DBConnector& client, RedisTransactioner& pipe, std::map>>& data, int cursor, bool withDefaultValue) instead.")]] +#endif + int _get_config(DBConnector& client, RedisTransactioner& pipe, std::map>>& data, int cursor); int _get_config(DBConnector& client, RedisTransactioner& pipe, std::map>>& data, int cursor, bool withDefaultValue); }; diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index eb02d56f7..a788c1008 100644 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -771,6 +771,11 @@ shared_ptr DBConnector::get(const string &key) throw runtime_error("GET failed, memory exception"); } +shared_ptr DBConnector::hget(const string &key, const string &field) +{ + return hget(key, field, false); +} + shared_ptr DBConnector::hget(const string &key, const string &field, bool withDefaultValue) { RedisCommand shget; diff --git a/common/dbconnector.h b/common/dbconnector.h index 70c0e1249..bdea5a723 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -199,10 +199,18 @@ class DBConnector : public RedisContext void del(const std::vector& keys); + // TODO: [Hua] check if we can remove this method, because in a library, template method could not be export. + template > + ReturnType hgetall(const std::string &key); + template > ReturnType hgetall(const std::string &key, bool withDefaultValue); #ifndef SWIG + // TODO: [Hua] check if we can remove this method, because in a library, template method could not be export. + template + void hgetall(const std::string &key, OutputIterator result); + template void hgetall(const std::string &key, OutputIterator result, bool withDefaultValue); #endif @@ -223,6 +231,8 @@ class DBConnector : public RedisContext std::shared_ptr get(const std::string &key); + std::shared_ptr hget(const std::string &key, const std::string &field); + std::shared_ptr hget(const std::string &key, const std::string &field, bool withDefaultValue); bool hexists(const std::string &key, const std::string &field); @@ -257,6 +267,12 @@ class DBConnector : public RedisContext std::string m_shaRedisMulti; }; +template +ReturnType DBConnector::hgetall(const std::string &key) +{ + return hgetall(key, false); +} + template ReturnType DBConnector::hgetall(const std::string &key, bool withDefaultValue) { @@ -266,6 +282,12 @@ ReturnType DBConnector::hgetall(const std::string &key, bool withDefaultValue) } #ifndef SWIG +template +void DBConnector::hgetall(const std::string &key, OutputIterator result) +{ + hgetall(key, result, false); +} + template void DBConnector::hgetall(const std::string &key, OutputIterator result, bool withDefaultValue) { diff --git a/common/dbinterface.cpp b/common/dbinterface.cpp index fd3962ee2..ea8a987cd 100644 --- a/common/dbinterface.cpp +++ b/common/dbinterface.cpp @@ -56,6 +56,11 @@ bool DBInterface::exists(const string& dbName, const std::string& key) return m_redisClient.at(dbName).exists(key); } +std::shared_ptr DBInterface::get(const std::string& dbName, const std::string& hash, const std::string& key, bool blocking) +{ + return get(dbName, hash, key, false, blocking); +} + std::shared_ptr DBInterface::get(const std::string& dbName, const std::string& hash, const std::string& key, bool withDefaultValue, bool blocking) { auto innerfunc = [&] @@ -77,6 +82,11 @@ bool DBInterface::hexists(const std::string& dbName, const std::string& hash, co return m_redisClient.at(dbName).hexists(hash, key); } +std::map DBInterface::get_all(const std::string& dbName, const std::string& hash, bool blocking) +{ + return get_all(dbName, hash, false, blocking); +} + std::map DBInterface::get_all(const std::string& dbName, const std::string& hash, bool withDefaultValue, bool blocking) { auto innerfunc = [&] diff --git a/common/dbinterface.h b/common/dbinterface.h index e3967f301..0d9c2c2ea 100644 --- a/common/dbinterface.h +++ b/common/dbinterface.h @@ -38,9 +38,17 @@ class DBInterface // Delete all keys which match %pattern from DB void delete_all_by_pattern(const std::string& dbName, const std::string& pattern); bool exists(const std::string& dbName, const std::string& key); - std::shared_ptr get(const std::string& dbName, const std::string& hash, const std::string& key, bool withDefaultValue, bool blocking); // blocking = false +#ifndef SWIG + [[deprecated("Please use get(const std::string& dbName, const std::string& hash, const std::string& key, bool withDefaultValue, bool blocking) instead.")]] +#endif + std::shared_ptr get(const std::string& dbName, const std::string& hash, const std::string& key, bool blocking = false); + std::shared_ptr get(const std::string& dbName, const std::string& hash, const std::string& key, bool withDefaultValue, bool blocking); bool hexists(const std::string& dbName, const std::string& hash, const std::string& key); - std::map get_all(const std::string& dbName, const std::string& hash, bool withDefaultValue, bool blocking);//blocking = false +#ifndef SWIG + [[deprecated("Please use get_all(const std::string& dbName, const std::string& hash, bool withDefaultValue, bool blocking) instead.")]] +#endif + std::map get_all(const std::string& dbName, const std::string& hash, bool blocking = false); + std::map get_all(const std::string& dbName, const std::string& hash, bool withDefaultValue, bool blocking); std::vector keys(const std::string& dbName, const char *pattern = "*", bool blocking = false); std::pair> scan(const std::string& db_name, int cursor, const char *match, uint32_t count); int64_t publish(const std::string& dbName, const std::string& channel, const std::string& message); diff --git a/common/redisclient.h b/common/redisclient.h index 4898e537e..85a9f6f76 100644 --- a/common/redisclient.h +++ b/common/redisclient.h @@ -32,6 +32,11 @@ class RedisClient int64_t hdel(const std::string &key, const std::vector &fields) { return m_db->hdel(key, fields); } +#ifndef SWIG + [[deprecated("Please use hgetall(const std::string &key, bool withDefaultValue) instead.")]] +#endif + std::unordered_map hgetall(const std::string &key) { return hgetall(key, false); } + std::unordered_map hgetall(const std::string &key, bool withDefaultValue) { return m_db->hgetall(key, withDefaultValue); } template @@ -48,6 +53,11 @@ class RedisClient std::shared_ptr get(const std::string &key) { return m_db->get(key); } +#ifndef SWIG + [[deprecated("Please use hget(const std::string &key, const std::string &field, bool withDefaultValue) instead.")]] +#endif + std::shared_ptr hget(const std::string &key, const std::string &field) { return hget(key, field, false); } + std::shared_ptr hget(const std::string &key, const std::string &field, bool withDefaultValue) { return m_db->hget(key, field, withDefaultValue); } int64_t incr(const std::string &key) { return m_db->incr(key); } diff --git a/common/sonicv2connector.cpp b/common/sonicv2connector.cpp index 33d4ee1bf..b143e5df8 100644 --- a/common/sonicv2connector.cpp +++ b/common/sonicv2connector.cpp @@ -74,6 +74,11 @@ std::pair> SonicV2Connector_Native::scan(const std return m_dbintf.scan(db_name, cursor, match, count); } +std::shared_ptr SonicV2Connector_Native::get(const std::string& db_name, const std::string& _hash, const std::string& key, bool blocking) +{ + return get(db_name, _hash, key, false, blocking); +} + std::shared_ptr SonicV2Connector_Native::get(const std::string& db_name, const std::string& _hash, const std::string& key, bool withDefaultValue, bool blocking) { return m_dbintf.get(db_name, _hash, key, withDefaultValue, blocking); @@ -84,6 +89,11 @@ bool SonicV2Connector_Native::hexists(const std::string& db_name, const std::str return m_dbintf.hexists(db_name, _hash, key); } +std::map SonicV2Connector_Native::get_all(const std::string& db_name, const std::string& _hash, bool blocking) +{ + return get_all(db_name, _hash, false, blocking); +} + std::map SonicV2Connector_Native::get_all(const std::string& db_name, const std::string& _hash, bool withDefaultValue, bool blocking) { return m_dbintf.get_all(db_name, _hash, withDefaultValue, blocking); diff --git a/common/sonicv2connector.h b/common/sonicv2connector.h index 79caf56dd..f354e164f 100644 --- a/common/sonicv2connector.h +++ b/common/sonicv2connector.h @@ -35,11 +35,21 @@ class SonicV2Connector_Native std::pair> scan(const std::string& db_name, int cursor = 0, const char *match = "", uint32_t count = 10); - std::shared_ptr get(const std::string& db_name, const std::string& _hash, const std::string& key, bool withDefaultValue, bool blocking);//blocking=false +#ifndef SWIG + [[deprecated("Please use get(const std::string& db_name, const std::string& _hash, const std::string& key, bool withDefaultValue, bool blocking) instead.")]] +#endif + std::shared_ptr get(const std::string& db_name, const std::string& _hash, const std::string& key, bool blocking=false); + + std::shared_ptr get(const std::string& db_name, const std::string& _hash, const std::string& key, bool withDefaultValue, bool blocking); bool hexists(const std::string& db_name, const std::string& _hash, const std::string& key); - std::map get_all(const std::string& db_name, const std::string& _hash, bool withDefaultValue, bool blocking);//blocking=false +#ifndef SWIG + [[deprecated("Please use get_all(const std::string& db_name, const std::string& _hash, bool withDefaultValue, bool blocking) instead.")]] +#endif + std::map get_all(const std::string& db_name, const std::string& _hash, bool blocking=false); + + std::map get_all(const std::string& db_name, const std::string& _hash, bool withDefaultValue, bool blocking); void hmset(const std::string& db_name, const std::string &key, const std::map &values); diff --git a/common/subscriberstatetable.cpp b/common/subscriberstatetable.cpp index 8db50d8e8..dc68572d7 100644 --- a/common/subscriberstatetable.cpp +++ b/common/subscriberstatetable.cpp @@ -14,6 +14,11 @@ using namespace std; namespace swss { +SubscriberStateTable::SubscriberStateTable(DBConnector *db, const string &tableName, int popBatchSize, int pri) + : SubscriberStateTable(db, tableName, false, popBatchSize, pri) +{ +} + SubscriberStateTable::SubscriberStateTable(DBConnector *db, const string &tableName, bool withDefaultValue, int popBatchSize, int pri) : ConsumerTableBase(db, tableName, popBatchSize, pri), m_table(db, tableName) { diff --git a/common/subscriberstatetable.h b/common/subscriberstatetable.h index 5da8ea951..d4c0700f3 100644 --- a/common/subscriberstatetable.h +++ b/common/subscriberstatetable.h @@ -11,6 +11,10 @@ namespace swss { class SubscriberStateTable : public ConsumerTableBase { public: +#ifndef SWIG + [[deprecated("Please use SubscriberStateTable(DBConnector *db, const std::string &tableName, bool withDefaultValue, int popBatchSize = DEFAULT_POP_BATCH_SIZE, int pri = 0) instead.")]] +#endif + SubscriberStateTable(DBConnector *db, const std::string &tableName, int popBatchSize = DEFAULT_POP_BATCH_SIZE, int pri = 0); SubscriberStateTable(DBConnector *db, const std::string &tableName, bool withDefaultValue, int popBatchSize = DEFAULT_POP_BATCH_SIZE, int pri = 0); /* Get all elements available */ diff --git a/common/table.cpp b/common/table.cpp index 98a499a4c..1cc7f3d79 100644 --- a/common/table.cpp +++ b/common/table.cpp @@ -64,6 +64,11 @@ void Table::flush() m_pipe->flush(); } +bool Table::get(const string &key, vector &values) +{ + return get(key, values, false); +} + bool Table::get(const string &key, vector &values, bool withDefaultValue) { // [Hua] TODO: code here dupe with DBConnector::hgetall, check if can reuse it. @@ -204,6 +209,11 @@ void Table::hdel(const string &key, const string &field, const string& /* op */, m_pipe->push(cmd, REDIS_REPLY_INTEGER); } +void TableEntryEnumerable::getContent(vector &tuples) +{ + return getContent(tuples, false); +} + void TableEntryEnumerable::getContent(vector &tuples, bool withDefaultValue) { vector keys; diff --git a/common/table.h b/common/table.h index e163da176..5b1771338 100644 --- a/common/table.h +++ b/common/table.h @@ -166,6 +166,10 @@ class TableEntryEnumerable { virtual ~TableEntryEnumerable() = default; /* Get all the field-value tuple of the table entry with the key */ +#ifndef SWIG + [[deprecated("Please use get(const std::string &key, std::vector &values, bool withDefaultValue) instead.")]] +#endif + virtual bool get(const std::string &key, std::vector &values) = 0; virtual bool get(const std::string &key, std::vector &values, bool withDefaultValue) = 0; /* get all the keys in the table */ @@ -173,6 +177,10 @@ class TableEntryEnumerable { /* Read the whole table content from the DB directly */ /* NOTE: Not an atomic function */ +#ifndef SWIG + [[deprecated("Please use getContent(std::vector &tuples, bool withDefaultValue) instead.")]] +#endif + void getContent(std::vector &tuples); void getContent(std::vector &tuples, bool withDefaultValue); }; @@ -222,6 +230,10 @@ class Table : public TableBase, public TableEntryEnumerable { const std::string &prefix = EMPTY_PREFIX); /* Read a value from the DB directly */ /* Returns false if the key doesn't exists */ +#ifndef SWIG + [[deprecated("Please use get(const std::string &key, std::vector &ovalues, bool withDefaultValue) instead.")]] +#endif + virtual bool get(const std::string &key, std::vector &ovalues); virtual bool get(const std::string &key, std::vector &ovalues, bool withDefaultValue); virtual bool hget(const std::string &key, const std::string &field, std::string &value); diff --git a/tests/selectable_priority.cpp b/tests/selectable_priority.cpp index 778514558..38512edfe 100644 --- a/tests/selectable_priority.cpp +++ b/tests/selectable_priority.cpp @@ -59,7 +59,7 @@ TEST(Priority, set_pri_values) RedisSelect rs(105); SelectableEvent se(106); SelectableTimer st(interval, 107); - SubscriberStateTable sst(&db, tableName, DEFAULT_POP_BATCH_SIZE, 108); + SubscriberStateTable sst(&db, tableName, false, DEFAULT_POP_BATCH_SIZE, 108); EXPECT_EQ(nl.getPri(), 101); EXPECT_EQ(cst.getPri(), 102); From f4348613cfeff408933ab85dece45116ed05e07a Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 18 Apr 2022 07:32:20 +0000 Subject: [PATCH 17/24] Fix code bug --- common/configdb.cpp | 6 ++++++ common/defaultvalueprovider.cpp | 32 ++++++++++++++++---------------- common/defaultvalueprovider.h | 4 ++++ 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index 4140fcc75..62b53fe11 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -535,6 +535,12 @@ map>> ConfigDBPipeConnector_Native::get_ DBConnector clientPipe(client); RedisTransactioner pipe(&clientPipe); + // TODO: [Hua] remove this because only for demo + if (DefaultValueProvider::FeatureEnabledByEnvironmentVariable()) + { + withDefaultValue = true; + } + map>> data; auto cur = _get_config(client, pipe, data, 0, withDefaultValue); while (cur != 0) diff --git a/common/defaultvalueprovider.cpp b/common/defaultvalueprovider.cpp index 9460c77ca..7ca0cc3ea 100644 --- a/common/defaultvalueprovider.cpp +++ b/common/defaultvalueprovider.cpp @@ -25,7 +25,7 @@ using namespace swss; size_t stackInfoSize = backtrace(stackInfo, TRACE_STACK_SIZE); // print out all the frames to stderr - cerr << "Error: signal " << sig << ":\n" >> endl; + cerr << "Error: signal " << sig << ":\n" << endl; backtrace_symbols_fd(stackInfo, (int)stackInfoSize, STDERR_FILENO); exit(1); } @@ -44,8 +44,8 @@ TableInfoBase::TableInfoBase() std::shared_ptr TableInfoBase::GetDefaultValue(std::string row, std::string field) { - assert(!string::empty(row)); - assert(!string::empty(field)); + assert(!row.empty()); + assert(!field.empty()); SWSS_LOG_DEBUG("TableInfoBase::GetDefaultValue %s %s\n", row.c_str(), field.c_str()); FieldDefaultValueMapping *fieldMappingPtr; @@ -68,7 +68,7 @@ std::shared_ptr TableInfoBase::GetDefaultValue(std::string row, std // existedValues and targetValues can be same container. void TableInfoBase::AppendDefaultValues(string row, FieldValueMapping& existedValues, FieldValueMapping& targetValues) { - assert(!string::empty(row)); + assert(!row.empty()); SWSS_LOG_DEBUG("TableInfoBase::AppendDefaultValues %s\n", row.c_str()); FieldDefaultValueMapping *fieldMappingPtr; @@ -103,7 +103,7 @@ TableInfoDict::TableInfoDict(KeyInfoToDefaultValueInfoMapping &fieldInfoMapping) bool TableInfoDict::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** foundedMappingPtr) { - assert(!string::empty(row)); + assert(!row.empty()); assert(foundedMappingPtr != nullptr); SWSS_LOG_DEBUG("TableInfoDict::FindFieldMappingByKey %s\n", row.c_str()); @@ -119,7 +119,7 @@ TableInfoSingleList::TableInfoSingleList(KeyInfoToDefaultValueInfoMapping &field bool TableInfoSingleList::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** foundedMappingPtr) { - assert(!string::empty(row)); + assert(!row.empty()); assert(foundedMappingPtr != nullptr); SWSS_LOG_DEBUG("TableInfoSingleList::FindFieldMappingByKey %s\n", row.c_str()); @@ -139,7 +139,7 @@ TableInfoMultipleList::TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &f bool TableInfoMultipleList::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** foundedMappingPtr) { - assert(!string::empty(row)); + assert(!row.empty()); assert(foundedMappingPtr != nullptr); SWSS_LOG_DEBUG("TableInfoMultipleList::FindFieldMappingByKey %s\n", row.c_str()); @@ -166,7 +166,7 @@ DefaultValueProvider& DefaultValueProvider::Instance() shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string table) { - assert(!string::empty(table)); + assert(!table.empty()); SWSS_LOG_DEBUG("DefaultValueProvider::FindDefaultValueInfo %s\n", table.c_str()); auto findResult = m_defaultValueMapping.find(table); @@ -181,9 +181,9 @@ shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string std::shared_ptr DefaultValueProvider::GetDefaultValue(std::string table, std::string row, std::string field) { - assert(!string::empty(table)); - assert(!string::empty(row)); - assert(!string::empty(field)); + assert(!table.empty()); + assert(!row.empty()); + assert(!field.empty()); SWSS_LOG_DEBUG("DefaultValueProvider::GetDefaultValue %s %s %s\n", table.c_str(), row.c_str(), field.c_str()); #ifdef DEBUG @@ -205,8 +205,8 @@ std::shared_ptr DefaultValueProvider::GetDefaultValue(std::string t void DefaultValueProvider::AppendDefaultValues(std::string table, std::string row, std::vector > &values) { - assert(!string::empty(table)); - assert(!string::empty(row)); + assert(!table.empty()); + assert(!row.empty()); SWSS_LOG_DEBUG("DefaultValueProvider::AppendDefaultValues %s %s\n", table.c_str(), row.c_str()); #ifdef DEBUG @@ -240,8 +240,8 @@ void DefaultValueProvider::AppendDefaultValues(std::string table, std::string ro void DefaultValueProvider::AppendDefaultValues(string table, string row, FieldValueMapping& values) { - assert(!string::empty(table)); - assert(!string::empty(row)); + assert(!table.empty()); + assert(!row.empty()); SWSS_LOG_DEBUG("DefaultValueProvider::AppendDefaultValues %s %s\n", table.c_str(), row.c_str()); #ifdef DEBUG @@ -281,7 +281,7 @@ DefaultValueProvider::~DefaultValueProvider() void DefaultValueProvider::Initialize(char* modulePath) { - assert(modulePath != nullptr && !string::empty(modulePath)); + assert(modulePath != nullptr && strlen(modulePath) != 0); assert(m_context == nullptr); DIR *moduleDir = opendir(modulePath); diff --git a/common/defaultvalueprovider.h b/common/defaultvalueprovider.h index e80ad9d0f..73948a61b 100644 --- a/common/defaultvalueprovider.h +++ b/common/defaultvalueprovider.h @@ -81,6 +81,10 @@ class DefaultValueProvider std::shared_ptr GetDefaultValue(std::string table, std::string row, std::string field); +#ifdef DEBUG + static bool FeatureEnabledByEnvironmentVariable(); +#endif + private: DefaultValueProvider(); ~DefaultValueProvider(); From b169338c6264136b89b6ca687382cff81e173748 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 25 Apr 2022 04:36:10 +0000 Subject: [PATCH 18/24] Save file change --- common/defaultvalueprovider.cpp | 20 ++++++++++---------- common/defaultvalueprovider.h | 20 ++++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/common/defaultvalueprovider.cpp b/common/defaultvalueprovider.cpp index 7ca0cc3ea..1008d10d6 100644 --- a/common/defaultvalueprovider.cpp +++ b/common/defaultvalueprovider.cpp @@ -31,7 +31,7 @@ using namespace swss; } #endif -[[noreturn]] void ThrowRunTimeError(string message) +[[noreturn]] void ThrowRunTimeError(string &message) { SWSS_LOG_ERROR("DefaultValueProvider: %s", message.c_str()); throw std::runtime_error(message); @@ -42,7 +42,7 @@ TableInfoBase::TableInfoBase() // C++ need this empty ctor } -std::shared_ptr TableInfoBase::GetDefaultValue(std::string row, std::string field) +std::shared_ptr TableInfoBase::GetDefaultValue(std::string &row, std::string &field) { assert(!row.empty()); assert(!field.empty()); @@ -66,7 +66,7 @@ std::shared_ptr TableInfoBase::GetDefaultValue(std::string row, std } // existedValues and targetValues can be same container. -void TableInfoBase::AppendDefaultValues(string row, FieldValueMapping& existedValues, FieldValueMapping& targetValues) +void TableInfoBase::AppendDefaultValues(string &row, FieldValueMapping& existedValues, FieldValueMapping& targetValues) { assert(!row.empty()); @@ -101,7 +101,7 @@ TableInfoDict::TableInfoDict(KeyInfoToDefaultValueInfoMapping &fieldInfoMapping) } } -bool TableInfoDict::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** foundedMappingPtr) +bool TableInfoDict::FindFieldMappingByKey(string &row, FieldDefaultValueMapping ** foundedMappingPtr) { assert(!row.empty()); assert(foundedMappingPtr != nullptr); @@ -117,7 +117,7 @@ TableInfoSingleList::TableInfoSingleList(KeyInfoToDefaultValueInfoMapping &field m_defaultValueMapping = fieldInfoMapping.begin()->second; } -bool TableInfoSingleList::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** foundedMappingPtr) +bool TableInfoSingleList::FindFieldMappingByKey(string &row, FieldDefaultValueMapping ** foundedMappingPtr) { assert(!row.empty()); assert(foundedMappingPtr != nullptr); @@ -137,7 +137,7 @@ TableInfoMultipleList::TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &f } } -bool TableInfoMultipleList::FindFieldMappingByKey(string row, FieldDefaultValueMapping ** foundedMappingPtr) +bool TableInfoMultipleList::FindFieldMappingByKey(string &row, FieldDefaultValueMapping ** foundedMappingPtr) { assert(!row.empty()); assert(foundedMappingPtr != nullptr); @@ -164,7 +164,7 @@ DefaultValueProvider& DefaultValueProvider::Instance() return instance; } -shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string table) +shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string &table) { assert(!table.empty()); @@ -179,7 +179,7 @@ shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string return findResult->second; } -std::shared_ptr DefaultValueProvider::GetDefaultValue(std::string table, std::string row, std::string field) +std::shared_ptr DefaultValueProvider::GetDefaultValue(std::string &table, std::string &row, std::string &field) { assert(!table.empty()); assert(!row.empty()); @@ -203,7 +203,7 @@ std::shared_ptr DefaultValueProvider::GetDefaultValue(std::string t return defaultValueInfo->GetDefaultValue(row, field); } -void DefaultValueProvider::AppendDefaultValues(std::string table, std::string row, std::vector > &values) +void DefaultValueProvider::AppendDefaultValues(std::string &table, std::string &row, std::vector > &values) { assert(!table.empty()); assert(!row.empty()); @@ -238,7 +238,7 @@ void DefaultValueProvider::AppendDefaultValues(std::string table, std::string ro } } -void DefaultValueProvider::AppendDefaultValues(string table, string row, FieldValueMapping& values) +void DefaultValueProvider::AppendDefaultValues(string &table, string &row, FieldValueMapping& values) { assert(!table.empty()); assert(!row.empty()); diff --git a/common/defaultvalueprovider.h b/common/defaultvalueprovider.h index 73948a61b..abf342b43 100644 --- a/common/defaultvalueprovider.h +++ b/common/defaultvalueprovider.h @@ -26,12 +26,12 @@ class TableInfoBase public: TableInfoBase(); - void AppendDefaultValues(std::string row, FieldValueMapping& sourceValues, FieldValueMapping& targetValues); + void AppendDefaultValues(std::string &row, FieldValueMapping& sourceValues, FieldValueMapping& targetValues); - std::shared_ptr GetDefaultValue(std::string row, std::string field); + std::shared_ptr GetDefaultValue(std::string &row, std::string &field); protected: - virtual bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** foundedMappingPtr) = 0; + virtual bool FindFieldMappingByKey(std::string &row, FieldDefaultValueMapping ** foundedMappingPtr) = 0; }; class TableInfoDict : public TableInfoBase @@ -43,7 +43,7 @@ class TableInfoDict : public TableInfoBase // Mapping: key value -> field -> default std::map m_defaultValueMapping; - bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** foundedMappingPtr); + bool FindFieldMappingByKey(std::string &row, FieldDefaultValueMapping ** foundedMappingPtr); }; class TableInfoSingleList : public TableInfoBase @@ -55,7 +55,7 @@ class TableInfoSingleList : public TableInfoBase // Mapping: field -> default FieldDefaultValueMappingPtr m_defaultValueMapping; - bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** foundedMappingPtr); + bool FindFieldMappingByKey(std::string &row, FieldDefaultValueMapping ** foundedMappingPtr); }; struct TableInfoMultipleList : public TableInfoBase @@ -67,7 +67,7 @@ struct TableInfoMultipleList : public TableInfoBase // Mapping: key field count -> field -> default std::map m_defaultValueMapping; - bool FindFieldMappingByKey(std::string row, FieldDefaultValueMapping ** foundedMappingPtr); + bool FindFieldMappingByKey(std::string &row, FieldDefaultValueMapping ** foundedMappingPtr); }; class DefaultValueProvider @@ -75,11 +75,11 @@ class DefaultValueProvider public: static DefaultValueProvider& Instance(); - void AppendDefaultValues(std::string table, std::string row, FieldValueMapping& values); + void AppendDefaultValues(std::string &table, std::string &row, FieldValueMapping& values); - void AppendDefaultValues(std::string table, std::string row, std::vector > &values); + void AppendDefaultValues(std::string &table, std::string &row, std::vector > &values); - std::shared_ptr GetDefaultValue(std::string table, std::string row, std::string field); + std::shared_ptr GetDefaultValue(std::string &table, std::string &row, std::string &field); #ifdef DEBUG static bool FeatureEnabledByEnvironmentVariable(); @@ -100,7 +100,7 @@ class DefaultValueProvider // Load default value info from yang model and append to default value mapping void AppendTableInfoToMapping(struct lys_node* table); - std::shared_ptr FindDefaultValueInfo(std::string table); + std::shared_ptr FindDefaultValueInfo(std::string &table); int BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping& fieldMappingList); From 7d7e6bd2ce76098f96448d0556ea0ac2b3b620d7 Mon Sep 17 00:00:00 2001 From: azureuser Date: Wed, 27 Apr 2022 07:21:12 +0000 Subject: [PATCH 19/24] Update code by PR comments --- common/Makefile.am | 1 + common/configdb.cpp | 86 +++++++++++------------ common/configdb.h | 42 ++++++------ common/dbconnector.cpp | 101 +++++++++++++++++++++++---- common/dbconnector.h | 87 ++++++++++------------- common/dbdecorator.cpp | 118 ++++++++++++++++++++++++++++++++ common/dbdecorator.h | 50 ++++++++++++++ common/dbinterface.cpp | 14 +--- common/dbinterface.h | 8 --- common/defaultvalueprovider.cpp | 23 ++++--- common/defaultvalueprovider.h | 28 ++++---- common/logger.cpp | 4 +- common/loglevel.cpp | 2 +- common/producerstatetable.cpp | 2 +- common/redisclient.h | 14 +--- common/sonicv2connector.cpp | 14 +--- common/sonicv2connector.h | 12 +--- common/subscriberstatetable.cpp | 10 +-- common/subscriberstatetable.h | 5 -- common/table.cpp | 39 ++++------- common/table.h | 18 ++--- pyext/swsscommon.i | 5 ++ tests/fdb_flush.cpp | 2 +- tests/logger_ut.cpp | 2 +- tests/redis_multi_ns_ut.cpp | 8 +-- tests/redis_piped_ut.cpp | 10 +-- tests/redis_state_ut.cpp | 4 +- tests/redis_ut.cpp | 24 +++---- tests/selectable_priority.cpp | 2 +- 29 files changed, 443 insertions(+), 292 deletions(-) create mode 100644 common/dbdecorator.cpp create mode 100644 common/dbdecorator.h diff --git a/common/Makefile.am b/common/Makefile.am index 49273b209..9b7b39e73 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -32,6 +32,7 @@ libswsscommon_la_SOURCES = \ redisreply.cpp \ configdb.cpp \ dbconnector.cpp \ + dbdecorator.cpp \ dbinterface.cpp \ defaultvalueprovider.cpp \ sonicv2connector.cpp \ diff --git a/common/configdb.cpp b/common/configdb.cpp index 62b53fe11..01b103372 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -10,10 +10,28 @@ using namespace std; using namespace swss; ConfigDBConnector_Native::ConfigDBConnector_Native(bool use_unix_socket_path, const char *netns) + : ConfigDBConnector_Native(false, use_unix_socket_path, netns) +{ +} + +ConfigDBConnector_Native::ConfigDBConnector_Native(bool get_default_value, bool use_unix_socket_path, const char *netns) : SonicV2Connector_Native(use_unix_socket_path, netns) , m_table_name_separator("|") , m_key_separator("|") + , m_get_default_value(get_default_value) { + // TODO: [Hua] remove this because only for demo#ifdef DEBUG +#ifdef DEBUG + if (DefaultValueProvider::FeatureEnabledByEnvironmentVariable()) + { + m_get_default_value = true; + } +#endif + + if (m_get_default_value) + { + m_db_decorator = ConfigDBDecorator::Create(m_table_name_separator); + } } void ConfigDBConnector_Native::db_connect(string db_name, bool wait_for_init, bool retry_on) @@ -80,7 +98,7 @@ void ConfigDBConnector_Native::set_entry(string table, string key, const map ConfigDBConnector_Native::get_entry(string table, string key) -{ - return get_entry(table, key, false); -} - -map ConfigDBConnector_Native::get_entry(string table, string key, bool withDefaultValue) { auto& client = get_redis_client(m_db_name); string _hash = to_upper(table) + m_table_name_separator + key; - return client.hgetall>(_hash, withDefaultValue); + return client.hgetall>(_hash); } // Read all keys of a table from config db. @@ -176,11 +189,6 @@ vector ConfigDBConnector_Native::get_keys(string table, bool split) // or { ('l1_key', 'l2_key', ...): {'column_key': value, ...}, ...} for a multi-key table. // Empty dictionary if table does not exist. map> ConfigDBConnector_Native::get_table(string table) -{ - return get_table(table, false); -} - -map> ConfigDBConnector_Native::get_table(string table, bool withDefaultValue) { auto& client = get_redis_client(m_db_name); string pattern = to_upper(table) + m_table_name_separator + "*"; @@ -188,7 +196,7 @@ map> ConfigDBConnector_Native::get_table(string tabl map> data; for (auto& key: keys) { - auto const& entry = client.hgetall>(key, withDefaultValue); + auto const& entry = client.hgetall>(key); size_t pos = key.find(m_table_name_separator); string row; if (pos == string::npos) @@ -255,11 +263,6 @@ void ConfigDBConnector_Native::mod_config(const map>> ConfigDBConnector_Native::get_config() -{ - return get_config(false); -} - -map>> ConfigDBConnector_Native::get_config(bool withDefaultValue) { auto& client = get_redis_client(m_db_name); auto const& keys = client.keys("*"); @@ -272,7 +275,7 @@ map>> ConfigDBConnector_Native::get_conf } string table_name = key.substr(0, pos); string row = key.substr(pos + 1); - auto const& entry = client.hgetall>(key, withDefaultValue); + auto const& entry = client.hgetall>(key); if (!entry.empty()) { @@ -298,8 +301,21 @@ std::string ConfigDBConnector_Native::getDbName() const return m_db_name; } + +DBConnector& ConfigDBConnector_Native::get_redis_client(const std::string& db_name) +{ + auto& result = SonicV2Connector_Native::get_redis_client(db_name); + result.setDBDecortor(m_db_decorator); + return result; +} + ConfigDBPipeConnector_Native::ConfigDBPipeConnector_Native(bool use_unix_socket_path, const char *netns) - : ConfigDBConnector_Native(use_unix_socket_path, netns) + : ConfigDBConnector_Native(false, use_unix_socket_path, netns) +{ +} + +ConfigDBPipeConnector_Native::ConfigDBPipeConnector_Native(bool get_default_value, bool use_unix_socket_path, const char *netns) + : ConfigDBConnector_Native(get_default_value, use_unix_socket_path, netns) { } @@ -382,7 +398,7 @@ void ConfigDBPipeConnector_Native::_set_entry(RedisTransactioner& pipe, std::str } else { - auto original = get_entry(table, key, false); + auto original = get_entry(table, key); RedisCommand shmset; shmset.formatHMSET(_hash, data.begin(), data.end()); @@ -470,11 +486,6 @@ void ConfigDBPipeConnector_Native::mod_config(const map>>& data, int cursor) -{ - return _get_config(client, pipe, data, cursor, false); -} - -int ConfigDBPipeConnector_Native::_get_config(DBConnector& client, RedisTransactioner& pipe, map>>& data, int cursor, bool withDefaultValue) { auto const& rc = client.scan(cursor, "*", REDIS_SCAN_BATCH_SIZE); auto cur = rc.first; @@ -515,38 +526,27 @@ int ConfigDBPipeConnector_Native::_get_config(DBConnector& client, RedisTransact dataentry.emplace(field, value); } - // merge default value to config - if (withDefaultValue) + // Because run Redis command with pipe not use DBConnector, so need decorate result here. + if (m_db_decorator) { - DefaultValueProvider::Instance().AppendDefaultValues(table_name, row, dataentry); + m_db_decorator->decorate(key, dataentry); } } return cur; } map>> ConfigDBPipeConnector_Native::get_config() -{ - return get_config(false); -} - -map>> ConfigDBPipeConnector_Native::get_config(bool withDefaultValue) { auto& client = get_redis_client(m_db_name); DBConnector clientPipe(client); RedisTransactioner pipe(&clientPipe); - // TODO: [Hua] remove this because only for demo - if (DefaultValueProvider::FeatureEnabledByEnvironmentVariable()) - { - withDefaultValue = true; - } - map>> data; - auto cur = _get_config(client, pipe, data, 0, withDefaultValue); + auto cur = _get_config(client, pipe, data, 0); while (cur != 0) { - cur = _get_config(client, pipe, data, cur, withDefaultValue); + cur = _get_config(client, pipe, data, cur); } return data; -} +} \ No newline at end of file diff --git a/common/configdb.h b/common/configdb.h index ff2e8a641..1287f884b 100644 --- a/common/configdb.h +++ b/common/configdb.h @@ -2,6 +2,7 @@ #include #include +#include #include "sonicv2connector.h" #include "redistran.h" @@ -12,41 +13,37 @@ class ConfigDBConnector_Native : public SonicV2Connector_Native public: static constexpr const char *INIT_INDICATOR = "CONFIG_DB_INITIALIZED"; +#ifndef SWIG + [[deprecated("Please use ConfigDBConnector_Native(bool use_unix_socket_path = false, const char *netns = "", bool get_default_value = false) instead.")]] +#endif ConfigDBConnector_Native(bool use_unix_socket_path = false, const char *netns = ""); + ConfigDBConnector_Native(bool get_default_value, bool use_unix_socket_path = false, const char *netns = ""); void db_connect(std::string db_name, bool wait_for_init = false, bool retry_on = false); void connect(bool wait_for_init = true, bool retry_on = false); virtual void set_entry(std::string table, std::string key, const std::map& data); virtual void mod_entry(std::string table, std::string key, const std::map& data); -#ifndef SWIG - [[deprecated("Please use get_entry(std::string table, std::string key, bool withDefaultValue) instead.")]] -#endif std::map get_entry(std::string table, std::string key); - std::map get_entry(std::string table, std::string key, bool withDefaultValue); std::vector get_keys(std::string table, bool split = true); -#ifndef SWIG - [[deprecated("Please use get_table(std::string table, bool withDefaultValue) instead.")]] -#endif std::map> get_table(std::string table); - std::map> get_table(std::string table, bool withDefaultValue); void delete_table(std::string table); virtual void mod_config(const std::map>>& data); -#ifndef SWIG - [[deprecated("Please use get_config(bool withDefaultValue) instead.")]] -#endif virtual std::map>> get_config(); - virtual std::map>> get_config(bool withDefaultValue); std::string getKeySeparator() const; std::string getTableNameSeparator() const; std::string getDbName() const; + DBConnector& get_redis_client(const std::string& db_name) override; + protected: std::string m_table_name_separator = "|"; std::string m_key_separator = "|"; std::string m_db_name; + bool m_get_default_value; + std::shared_ptr m_db_decorator; }; #ifdef SWIG @@ -60,7 +57,12 @@ class ConfigDBConnector_Native : public SonicV2Connector_Native raise ValueError('decode_responses must be True if specified, False is not supported') if namespace is None: namespace = '' - super(ConfigDBConnector, self).__init__(use_unix_socket_path = use_unix_socket_path, namespace = namespace) + + get_default_value = False + if 'get_default_value' in kwargs and kwargs.pop('get_default_value') == True: + get_default_value = True + + super(ConfigDBConnector, self).__init__(get_default_value = get_default_value, use_unix_socket_path = use_unix_socket_path, namespace = namespace) # Trick: to achieve static/instance method "overload", we must use initize the function in ctor # ref: https://stackoverflow.com/a/28766809/2514803 @@ -84,7 +86,7 @@ class ConfigDBConnector_Native : public SonicV2Connector_Native return self.getDbName() ## Note: callback is difficult to implement by SWIG C++, so keep in python - def listen(self, init_data_handler=None, with_default_value=False): + def listen(self, init_data_handler=None): ## Start listen Redis keyspace event. Pass a callback function to `init` to handle initial table data. self.pubsub = self.get_redis_client(self.db_name).pubsub() self.pubsub.psubscribe("__keyspace@{}__:*".format(self.get_dbid(self.db_name))) @@ -254,15 +256,15 @@ class ConfigDBConnector_Native : public SonicV2Connector_Native class ConfigDBPipeConnector_Native: public ConfigDBConnector_Native { public: +#ifndef SWIG + [[deprecated("Please use ConfigDBPipeConnector_Native(bool get_default_value, bool use_unix_socket_path = false, const char *netns = "") instead.")]] +#endif ConfigDBPipeConnector_Native(bool use_unix_socket_path = false, const char *netns = ""); + ConfigDBPipeConnector_Native(bool get_default_value, bool use_unix_socket_path = false, const char *netns = ""); void set_entry(std::string table, std::string key, const std::map& data) override; void mod_config(const std::map>>& data) override; -#ifndef SWIG - [[deprecated("Please use get_config(bool withDefaultValue) instead.")]] -#endif std::map>> get_config() override; - std::map>> get_config(bool withDefaultValue) override; private: static const int64_t REDIS_SCAN_BATCH_SIZE = 30; @@ -271,11 +273,7 @@ class ConfigDBPipeConnector_Native: public ConfigDBConnector_Native void _delete_table(DBConnector& client, RedisTransactioner& pipe, std::string table); void _set_entry(RedisTransactioner& pipe, std::string table, std::string key, const std::map& data); void _mod_entry(RedisTransactioner& pipe, std::string table, std::string key, const std::map& data); -#ifndef SWIG - [[deprecated("Please use _get_config(DBConnector& client, RedisTransactioner& pipe, std::map>>& data, int cursor, bool withDefaultValue) instead.")]] -#endif int _get_config(DBConnector& client, RedisTransactioner& pipe, std::map>>& data, int cursor); - int _get_config(DBConnector& client, RedisTransactioner& pipe, std::map>>& data, int cursor, bool withDefaultValue); }; #ifdef SWIG diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index a788c1008..4798683b5 100644 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -9,6 +9,7 @@ #include "logger.h" #include "common/dbconnector.h" +#include "common/dbdecorator.h" #include "common/redisreply.h" #include "common/redisapi.h" #include "common/pubsub.h" @@ -512,6 +513,7 @@ DBConnector::DBConnector(const DBConnector &other) , m_dbId(other.m_dbId) , m_namespace(other.m_namespace) { + m_db_decorator = nullptr; select(this); } @@ -519,6 +521,7 @@ DBConnector::DBConnector(int dbId, const RedisContext& ctx) : RedisContext(ctx) , m_dbId(dbId) , m_namespace(EMPTY_NAMESPACE) + , m_db_decorator(nullptr) { select(this); } @@ -527,6 +530,7 @@ DBConnector::DBConnector(int dbId, const string& hostname, int port, unsigned int timeout) : m_dbId(dbId) , m_namespace(EMPTY_NAMESPACE) + , m_db_decorator(nullptr) { struct timeval tv = {0, (suseconds_t)timeout * 1000}; struct timeval *ptv = timeout ? &tv : NULL; @@ -538,6 +542,7 @@ DBConnector::DBConnector(int dbId, const string& hostname, int port, DBConnector::DBConnector(int dbId, const string& unixPath, unsigned int timeout) : m_dbId(dbId) , m_namespace(EMPTY_NAMESPACE) + , m_db_decorator(nullptr) { struct timeval tv = {0, (suseconds_t)timeout * 1000}; struct timeval *ptv = timeout ? &tv : NULL; @@ -550,6 +555,7 @@ DBConnector::DBConnector(const string& dbName, unsigned int timeout, bool isTcpC : m_dbId(SonicDBConfig::getDbId(dbName, netns)) , m_dbName(dbName) , m_namespace(netns) + , m_db_decorator(nullptr) { struct timeval tv = {0, (suseconds_t)timeout * 1000}; struct timeval *ptv = timeout ? &tv : NULL; @@ -571,6 +577,10 @@ DBConnector::DBConnector(const string& dbName, unsigned int timeout, bool isTcpC // Empty constructor } +DBConnector::~DBConnector() +{ +} + int DBConnector::getDbId() const { return m_dbId; @@ -772,11 +782,6 @@ shared_ptr DBConnector::get(const string &key) } shared_ptr DBConnector::hget(const string &key, const string &field) -{ - return hget(key, field, false); -} - -shared_ptr DBConnector::hget(const string &key, const string &field, bool withDefaultValue) { RedisCommand shget; shget.format("HGET %s %s", key.c_str(), field.c_str()); @@ -785,21 +790,15 @@ shared_ptr DBConnector::hget(const string &key, const string &field, boo if (reply->type == REDIS_REPLY_NIL) { - if (!withDefaultValue || this->getDbId() != CONFIG_DB) + auto dbdecortor = this->getDBDecortor(); + if (dbdecortor) { - return shared_ptr(NULL); + return dbdecortor->decorate(key, field); } - - size_t pos = key.find("|"); - if (pos == std::string::npos) + else { - SWSS_LOG_WARN("Table::get key for config DB is %s, can't find a sepreator\n", key.c_str()); return shared_ptr(NULL); } - - std::string table = key.substr(0, pos); - std::string row = key.substr(pos + 1); - return DefaultValueProvider::Instance().GetDefaultValue(table, row, field); } if (reply->type == REDIS_REPLY_STRING) @@ -934,3 +933,75 @@ void DBConnector::del(const std::vector& keys) RedisReply r(this, command, REDIS_REPLY_NIL); } + + +void DBConnector::setDBDecortor(std::shared_ptr &db_decorator) +{ + m_db_decorator = db_decorator; +} + +const std::shared_ptr &DBConnector::getDBDecortor() const +{ + return m_db_decorator; +} + +/* +CfgDBConnector::CfgDBConnector(const DBConnector &other, bool getDefaultValue) + : DBConnector(other) + , m_get_default_value(getDefaultValue) +{ + if (getDefaultValue) + { + m_db_decorator = make_shared(); + } +} + +CfgDBConnector::CfgDBConnector(int dbId, const RedisContext &ctx, bool getDefaultValue) + : DBConnector(dbId, ctx) + , m_get_default_value(getDefaultValue) +{ + if (getDefaultValue) + { + m_db_decorator = make_shared(); + } +} + +CfgDBConnector::CfgDBConnector(int dbId, const std::string &hostname, int port, unsigned int timeout, bool getDefaultValue) + : DBConnector(dbId, hostname, port, timeout) + , m_get_default_value(getDefaultValue) +{ + if (getDefaultValue) + { + m_db_decorator = make_shared(); + } +} + +CfgDBConnector::CfgDBConnector(const std::string &dbName, unsigned int timeout, bool getDefaultValue, bool isTcpConn) + : DBConnector(dbName, timeout, isTcpConn) + , m_get_default_value(getDefaultValue) +{ + if (getDefaultValue) + { + m_db_decorator = make_shared(); + } +} + +CfgDBConnector::CfgDBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn, const std::string &netns, bool getDefaultValue) + : DBConnector(dbName, timeout, isTcpConn, netns) + , m_get_default_value(getDefaultValue) +{ + if (getDefaultValue) + { + m_db_decorator = make_shared(); + } +} + +CfgDBConnector::~CfgDBConnector() +{ +} + +std::shared_ptr CfgDBConnector::getDBDecortor() const +{ + return m_db_decorator; +} +*/ diff --git a/common/dbconnector.h b/common/dbconnector.h index bdea5a723..7bbf9ce64 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -10,6 +10,7 @@ #include #include +#include "dbdecorator.h" #include "defaultvalueprovider.h" #include "rediscommand.h" #include "redisreply.h" @@ -160,6 +161,7 @@ class DBConnector : public RedisContext DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn = false); DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn, const std::string &netns); DBConnector& operator=(const DBConnector&) = delete; + virtual ~DBConnector(); int getDbId() const; std::string getDbName() const; @@ -199,20 +201,12 @@ class DBConnector : public RedisContext void del(const std::vector& keys); - // TODO: [Hua] check if we can remove this method, because in a library, template method could not be export. template > ReturnType hgetall(const std::string &key); - template > - ReturnType hgetall(const std::string &key, bool withDefaultValue); - #ifndef SWIG - // TODO: [Hua] check if we can remove this method, because in a library, template method could not be export. template void hgetall(const std::string &key, OutputIterator result); - - template - void hgetall(const std::string &key, OutputIterator result, bool withDefaultValue); #endif std::vector keys(const std::string &key); @@ -233,8 +227,6 @@ class DBConnector : public RedisContext std::shared_ptr hget(const std::string &key, const std::string &field); - std::shared_ptr hget(const std::string &key, const std::string &field, bool withDefaultValue); - bool hexists(const std::string &key, const std::string &field); int64_t incr(const std::string &key); @@ -257,6 +249,10 @@ class DBConnector : public RedisContext bool flushdb(); + void setDBDecortor(std::shared_ptr &db_decorator); + + const std::shared_ptr &getDBDecortor() const; + private: void setNamespace(const std::string &netns); @@ -265,31 +261,21 @@ class DBConnector : public RedisContext std::string m_namespace; std::string m_shaRedisMulti; + + std::shared_ptr m_db_decorator; }; template ReturnType DBConnector::hgetall(const std::string &key) -{ - return hgetall(key, false); -} - -template -ReturnType DBConnector::hgetall(const std::string &key, bool withDefaultValue) { ReturnType map; - hgetall(key, std::inserter(map, map.end()), withDefaultValue); + hgetall(key, std::inserter(map, map.end())); return map; } #ifndef SWIG template void DBConnector::hgetall(const std::string &key, OutputIterator result) -{ - hgetall(key, result, false); -} - -template -void DBConnector::hgetall(const std::string &key, OutputIterator result, bool withDefaultValue) { RedisCommand shgetall; shgetall.format("HGETALL %s", key.c_str()); @@ -303,36 +289,10 @@ void DBConnector::hgetall(const std::string &key, OutputIterator result, bool wi ++result; } - if (withDefaultValue && this->getDbId() == CONFIG_DB) + auto dbdecortor = this->getDBDecortor(); + if (dbdecortor) { - size_t pos = key.find("|"); - if (pos == std::string::npos) - { - SWSS_LOG_WARN("Table::get key for config DB is %s, can't find a sepreator\n", key.c_str()); - } - - // When DB ID is CONFIG_DB, append default value to config DB result. - std::string table = key.substr(0, pos); - std::string row = key.substr(pos + 1); - std::map valuesWithDefault; - std::map existedValues; - for (unsigned int i = 0; i < ctx->elements; i += 2) - { - existedValues[ctx->element[i]->str] = ctx->element[i+1]->str; - valuesWithDefault[ctx->element[i]->str] = ctx->element[i+1]->str; - } - - DefaultValueProvider::Instance().AppendDefaultValues(table, row, valuesWithDefault); - - for (auto& fieldValuePair : valuesWithDefault) - { - auto findResult = existedValues.find(fieldValuePair.first); - if (findResult == existedValues.end()) - { - *result = std::make_pair(fieldValuePair.first, fieldValuePair.second); - ++result; - } - } + dbdecortor->decorate(key, ctx, result); } } #endif @@ -345,5 +305,28 @@ void DBConnector::hmset(const std::string &key, InputIterator start, InputIterat RedisReply r(this, shmset, REDIS_REPLY_STATUS); } +/* +// TODO: need more discussion about the API design, use may both want get/not get default vaule when running time +class CfgDBConnector : public DBConnector +{ + explicit CfgDBConnector(const DBConnector &other, bool getDefaultValue); + CfgDBConnector(int dbId, const RedisContext &ctx, bool getDefaultValue); + CfgDBConnector(int dbId, const std::string &hostname, int port, unsigned int timeout, bool getDefaultValue); + CfgDBConnector(int dbId, const std::string &unixPath, unsigned int timeout, bool getDefaultValue); + CfgDBConnector(const std::string &dbName, unsigned int timeout, bool getDefaultValue, bool isTcpConn = false); + CfgDBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn, const std::string &netns, bool getDefaultValue); + CfgDBConnector& operator=(const DBConnector&) = delete; + CfgDBConnector& operator=(const CfgDBConnector&) = delete; + + ~CfgDBConnector(); + + std::shared_ptr getDBDecortor() const override; + +private: + bool m_get_default_value; + std::shared_ptr m_db_decorator; +}; +*/ + } #endif diff --git a/common/dbdecorator.cpp b/common/dbdecorator.cpp new file mode 100644 index 000000000..e66b21467 --- /dev/null +++ b/common/dbdecorator.cpp @@ -0,0 +1,118 @@ +#include +#include +#include + +#include "defaultvalueprovider.h" +#include "dbdecorator.h" +#include "logger.h" +#include "table.h" + +using namespace std; +using namespace swss; + +#define POS(TUPLE) get<0>(TUPLE) +#define TABLE(TUPLE) get<1>(TUPLE) +#define ROW(TUPLE) get<2>(TUPLE) + +ConfigDBDecorator::ConfigDBDecorator(string separator) + :m_separator(separator) +{ +} + + +template +void ConfigDBDecorator::decorateInternal(const std::string &key, OutputIterator &result) +{ + auto tableAndRow = getTableAndRow(key); + if (POS(tableAndRow) == string::npos) + { + return; + } + + DefaultValueProvider::Instance().AppendDefaultValues(TABLE(tableAndRow), ROW(tableAndRow), result); +} + +void ConfigDBDecorator::decorate(const string &key, vector &result) +{ + decorateInternal(key, result); +} + +void ConfigDBDecorator::decorate(const string &key, map &result) +{ + decorateInternal(key, result); +} + +template +void ConfigDBDecorator::decorateInternal(const std::string &key, redisReply *&ctx, OutputIterator &result) +{ + auto tableAndRow = getTableAndRow(key); + if (POS(tableAndRow) == string::npos) + { + return; + } + + // When DB ID is CONFIG_DB, append default value to config DB result. + map valuesWithDefault; + map existedValues; + for (unsigned int i = 0; i < ctx->elements; i += 2) + { + existedValues[ctx->element[i]->str] = ctx->element[i+1]->str; + valuesWithDefault[ctx->element[i]->str] = ctx->element[i+1]->str; + } + + DefaultValueProvider::Instance().AppendDefaultValues(TABLE(tableAndRow), ROW(tableAndRow), valuesWithDefault); + + for (auto& fieldValuePair : valuesWithDefault) + { + auto findResult = existedValues.find(fieldValuePair.first); + if (findResult == existedValues.end()) + { + *result = make_pair(fieldValuePair.first, fieldValuePair.second); + ++result; + } + } +} + +void ConfigDBDecorator::decorate(const string &key, redisReply *&ctx, insert_iterator > &result) +{ + decorateInternal(key, ctx, result); +} + +void ConfigDBDecorator::decorate(const string &key, redisReply *&ctx, insert_iterator > &result) +{ + decorateInternal(key, ctx, result); +} + +shared_ptr ConfigDBDecorator::decorate(const string &key, const string &field) +{ + auto tableAndRow = getTableAndRow(key); + if (POS(tableAndRow) == string::npos) + { + return shared_ptr(NULL); + } + + return DefaultValueProvider::Instance().GetDefaultValue(TABLE(tableAndRow), ROW(tableAndRow), field); +} + +tuple ConfigDBDecorator::getTableAndRow(const string &key) +{ + string table; + string row; + size_t pos = key.find(m_separator); + if (pos == string::npos) + { + SWSS_LOG_WARN("Table::get key for config DB is %s, can't find a sepreator\n", key.c_str()); + } + else + { + table = key.substr(0, pos); + row = key.substr(pos + 1); + } + + return tuple(pos, table, row); +} + +shared_ptr ConfigDBDecorator::Create(string separator) +{ + return shared_ptr(new ConfigDBDecorator(separator)); +} \ No newline at end of file diff --git a/common/dbdecorator.h b/common/dbdecorator.h new file mode 100644 index 000000000..9ca2ed74d --- /dev/null +++ b/common/dbdecorator.h @@ -0,0 +1,50 @@ +#pragma once + +#include +#include +#include +#include +#include +#include + +class redisReply; + +namespace swss { + +class DBDecorator +{ +public: + virtual void decorate(const std::string &key, std::vector > &result) = 0; + virtual void decorate(const std::string &key, std::map &result) = 0; + virtual void decorate(const std::string &key, redisReply *&ctx, std::insert_iterator > &result) = 0; + virtual void decorate(const std::string &key, redisReply *&ctx, std::insert_iterator > &result) = 0; + virtual std::shared_ptr decorate(const std::string &key, const std::string &field) = 0; +}; + +class ConfigDBDecorator : public DBDecorator +{ +public: + static std::shared_ptr Create(std::string separator); + + void decorate(const std::string &key, std::vector > &result) override; + void decorate(const std::string &key, std::map &result) override; + void decorate(const std::string &key, redisReply *&ctx, std::insert_iterator > &result) override; + void decorate(const std::string &key, redisReply *&ctx, std::insert_iterator > &result) override; + std::shared_ptr decorate(const std::string &key, const std::string &field) override; + +private: + // Because derived class shared_ptr issue, not allow create ConfigDBDecorator, please use Create() + ConfigDBDecorator(std::string separator); + std::tuple getTableAndRow(const std::string &key); + + // decorate will be invoke in template method, but virtual method not support template, so create internal method for common code. + template + void decorateInternal(const std::string &key, redisReply *&ctx, OutputIterator &result); + + template + void decorateInternal(const std::string &key, OutputIterator &result); + + std::string m_separator; +}; + +} \ No newline at end of file diff --git a/common/dbinterface.cpp b/common/dbinterface.cpp index ea8a987cd..0043fb4c5 100644 --- a/common/dbinterface.cpp +++ b/common/dbinterface.cpp @@ -57,15 +57,10 @@ bool DBInterface::exists(const string& dbName, const std::string& key) } std::shared_ptr DBInterface::get(const std::string& dbName, const std::string& hash, const std::string& key, bool blocking) -{ - return get(dbName, hash, key, false, blocking); -} - -std::shared_ptr DBInterface::get(const std::string& dbName, const std::string& hash, const std::string& key, bool withDefaultValue, bool blocking) { auto innerfunc = [&] { - auto pvalue = m_redisClient.at(dbName).hget(hash, key, withDefaultValue); + auto pvalue = m_redisClient.at(dbName).hget(hash, key); if (!pvalue) { std::string message = "Key '" + hash + "' field '" + key + "' unavailable in database '" + dbName + "'"; @@ -83,16 +78,11 @@ bool DBInterface::hexists(const std::string& dbName, const std::string& hash, co } std::map DBInterface::get_all(const std::string& dbName, const std::string& hash, bool blocking) -{ - return get_all(dbName, hash, false, blocking); -} - -std::map DBInterface::get_all(const std::string& dbName, const std::string& hash, bool withDefaultValue, bool blocking) { auto innerfunc = [&] { std::map map; - m_redisClient.at(dbName).hgetall(hash, std::inserter(map, map.end()), withDefaultValue); + m_redisClient.at(dbName).hgetall(hash, std::inserter(map, map.end())); if (map.empty()) { diff --git a/common/dbinterface.h b/common/dbinterface.h index 0d9c2c2ea..a1fcf2a2b 100644 --- a/common/dbinterface.h +++ b/common/dbinterface.h @@ -38,17 +38,9 @@ class DBInterface // Delete all keys which match %pattern from DB void delete_all_by_pattern(const std::string& dbName, const std::string& pattern); bool exists(const std::string& dbName, const std::string& key); -#ifndef SWIG - [[deprecated("Please use get(const std::string& dbName, const std::string& hash, const std::string& key, bool withDefaultValue, bool blocking) instead.")]] -#endif std::shared_ptr get(const std::string& dbName, const std::string& hash, const std::string& key, bool blocking = false); - std::shared_ptr get(const std::string& dbName, const std::string& hash, const std::string& key, bool withDefaultValue, bool blocking); bool hexists(const std::string& dbName, const std::string& hash, const std::string& key); -#ifndef SWIG - [[deprecated("Please use get_all(const std::string& dbName, const std::string& hash, bool withDefaultValue, bool blocking) instead.")]] -#endif std::map get_all(const std::string& dbName, const std::string& hash, bool blocking = false); - std::map get_all(const std::string& dbName, const std::string& hash, bool withDefaultValue, bool blocking); std::vector keys(const std::string& dbName, const char *pattern = "*", bool blocking = false); std::pair> scan(const std::string& db_name, int cursor, const char *match, uint32_t count); int64_t publish(const std::string& dbName, const std::string& channel, const std::string& message); diff --git a/common/defaultvalueprovider.cpp b/common/defaultvalueprovider.cpp index 1008d10d6..68a081815 100644 --- a/common/defaultvalueprovider.cpp +++ b/common/defaultvalueprovider.cpp @@ -12,6 +12,7 @@ #include "defaultvalueprovider.h" #include "logger.h" +#include "table.h" using namespace std; using namespace swss; @@ -31,7 +32,7 @@ using namespace swss; } #endif -[[noreturn]] void ThrowRunTimeError(string &message) +[[noreturn]] void ThrowRunTimeError(string message) { SWSS_LOG_ERROR("DefaultValueProvider: %s", message.c_str()); throw std::runtime_error(message); @@ -42,7 +43,7 @@ TableInfoBase::TableInfoBase() // C++ need this empty ctor } -std::shared_ptr TableInfoBase::GetDefaultValue(std::string &row, std::string &field) +std::shared_ptr TableInfoBase::GetDefaultValue(const std::string &row, const std::string &field) { assert(!row.empty()); assert(!field.empty()); @@ -66,7 +67,7 @@ std::shared_ptr TableInfoBase::GetDefaultValue(std::string &row, st } // existedValues and targetValues can be same container. -void TableInfoBase::AppendDefaultValues(string &row, FieldValueMapping& existedValues, FieldValueMapping& targetValues) +void TableInfoBase::AppendDefaultValues(const string &row, FieldValueMapping& existedValues, FieldValueMapping& targetValues) { assert(!row.empty()); @@ -101,7 +102,7 @@ TableInfoDict::TableInfoDict(KeyInfoToDefaultValueInfoMapping &fieldInfoMapping) } } -bool TableInfoDict::FindFieldMappingByKey(string &row, FieldDefaultValueMapping ** foundedMappingPtr) +bool TableInfoDict::FindFieldMappingByKey(const string &row, FieldDefaultValueMapping ** foundedMappingPtr) { assert(!row.empty()); assert(foundedMappingPtr != nullptr); @@ -117,7 +118,7 @@ TableInfoSingleList::TableInfoSingleList(KeyInfoToDefaultValueInfoMapping &field m_defaultValueMapping = fieldInfoMapping.begin()->second; } -bool TableInfoSingleList::FindFieldMappingByKey(string &row, FieldDefaultValueMapping ** foundedMappingPtr) +bool TableInfoSingleList::FindFieldMappingByKey(const string &row, FieldDefaultValueMapping ** foundedMappingPtr) { assert(!row.empty()); assert(foundedMappingPtr != nullptr); @@ -137,7 +138,7 @@ TableInfoMultipleList::TableInfoMultipleList(KeyInfoToDefaultValueInfoMapping &f } } -bool TableInfoMultipleList::FindFieldMappingByKey(string &row, FieldDefaultValueMapping ** foundedMappingPtr) +bool TableInfoMultipleList::FindFieldMappingByKey(const string &row, FieldDefaultValueMapping ** foundedMappingPtr) { assert(!row.empty()); assert(foundedMappingPtr != nullptr); @@ -164,7 +165,7 @@ DefaultValueProvider& DefaultValueProvider::Instance() return instance; } -shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string &table) +shared_ptr DefaultValueProvider::FindDefaultValueInfo(const std::string &table) { assert(!table.empty()); @@ -179,7 +180,7 @@ shared_ptr DefaultValueProvider::FindDefaultValueInfo(std::string return findResult->second; } -std::shared_ptr DefaultValueProvider::GetDefaultValue(std::string &table, std::string &row, std::string &field) +std::shared_ptr DefaultValueProvider::GetDefaultValue(const std::string &table, const std::string &row, std::string field) { assert(!table.empty()); assert(!row.empty()); @@ -203,7 +204,7 @@ std::shared_ptr DefaultValueProvider::GetDefaultValue(std::string & return defaultValueInfo->GetDefaultValue(row, field); } -void DefaultValueProvider::AppendDefaultValues(std::string &table, std::string &row, std::vector > &values) +void DefaultValueProvider::AppendDefaultValues(const std::string &table, const std::string &row, std::vector > &values) { assert(!table.empty()); assert(!row.empty()); @@ -238,7 +239,7 @@ void DefaultValueProvider::AppendDefaultValues(std::string &table, std::string & } } -void DefaultValueProvider::AppendDefaultValues(string &table, string &row, FieldValueMapping& values) +void DefaultValueProvider::AppendDefaultValues(const string &table, const string &row, FieldValueMapping& values) { assert(!table.empty()); assert(!row.empty()); @@ -502,4 +503,4 @@ bool DefaultValueProvider::FeatureEnabledByEnvironmentVariable() return true; } -#endif \ No newline at end of file +#endif diff --git a/common/defaultvalueprovider.h b/common/defaultvalueprovider.h index abf342b43..3617d57ef 100644 --- a/common/defaultvalueprovider.h +++ b/common/defaultvalueprovider.h @@ -1,7 +1,8 @@ -#ifndef __DEFAULTVALUEPROVIDER__ -#define __DEFAULTVALUEPROVIDER__ +#pragma once + #include #include +#include #define DEFAULT_YANG_MODULE_PATH "/usr/local/yang-models" #define EMPTY_STR "" @@ -26,12 +27,12 @@ class TableInfoBase public: TableInfoBase(); - void AppendDefaultValues(std::string &row, FieldValueMapping& sourceValues, FieldValueMapping& targetValues); + void AppendDefaultValues(const std::string &row, FieldValueMapping& sourceValues, FieldValueMapping& targetValues); - std::shared_ptr GetDefaultValue(std::string &row, std::string &field); + std::shared_ptr GetDefaultValue(const std::string &row, const std::string &field); protected: - virtual bool FindFieldMappingByKey(std::string &row, FieldDefaultValueMapping ** foundedMappingPtr) = 0; + virtual bool FindFieldMappingByKey(const std::string &row, FieldDefaultValueMapping ** foundedMappingPtr) = 0; }; class TableInfoDict : public TableInfoBase @@ -43,7 +44,7 @@ class TableInfoDict : public TableInfoBase // Mapping: key value -> field -> default std::map m_defaultValueMapping; - bool FindFieldMappingByKey(std::string &row, FieldDefaultValueMapping ** foundedMappingPtr); + bool FindFieldMappingByKey(const std::string &row, FieldDefaultValueMapping ** foundedMappingPtr); }; class TableInfoSingleList : public TableInfoBase @@ -55,7 +56,7 @@ class TableInfoSingleList : public TableInfoBase // Mapping: field -> default FieldDefaultValueMappingPtr m_defaultValueMapping; - bool FindFieldMappingByKey(std::string &row, FieldDefaultValueMapping ** foundedMappingPtr); + bool FindFieldMappingByKey(const std::string &row, FieldDefaultValueMapping ** foundedMappingPtr); }; struct TableInfoMultipleList : public TableInfoBase @@ -67,7 +68,7 @@ struct TableInfoMultipleList : public TableInfoBase // Mapping: key field count -> field -> default std::map m_defaultValueMapping; - bool FindFieldMappingByKey(std::string &row, FieldDefaultValueMapping ** foundedMappingPtr); + bool FindFieldMappingByKey(const std::string &row, FieldDefaultValueMapping ** foundedMappingPtr); }; class DefaultValueProvider @@ -75,11 +76,11 @@ class DefaultValueProvider public: static DefaultValueProvider& Instance(); - void AppendDefaultValues(std::string &table, std::string &row, FieldValueMapping& values); + void AppendDefaultValues(const std::string &table, const std::string &row, FieldValueMapping& values); - void AppendDefaultValues(std::string &table, std::string &row, std::vector > &values); + void AppendDefaultValues(const std::string &table, const std::string &row, std::vector > &values); - std::shared_ptr GetDefaultValue(std::string &table, std::string &row, std::string &field); + std::shared_ptr GetDefaultValue(const std::string &table, const std::string &row, std::string field); #ifdef DEBUG static bool FeatureEnabledByEnvironmentVariable(); @@ -100,7 +101,7 @@ class DefaultValueProvider // Load default value info from yang model and append to default value mapping void AppendTableInfoToMapping(struct lys_node* table); - std::shared_ptr FindDefaultValueInfo(std::string &table); + std::shared_ptr FindDefaultValueInfo(const std::string &table); int BuildFieldMappingList(struct lys_node* table, KeyInfoToDefaultValueInfoMapping& fieldMappingList); @@ -108,5 +109,4 @@ class DefaultValueProvider FieldDefaultValueMappingPtr GetDefaultValueInfo(struct lys_node* tableChildNode); }; -} -#endif \ No newline at end of file +} \ No newline at end of file diff --git a/common/logger.cpp b/common/logger.cpp index cf0af00ac..b27cbed93 100644 --- a/common/logger.cpp +++ b/common/logger.cpp @@ -125,8 +125,8 @@ void Logger::linkToDbWithOutput( std::string key = dbName + ":" + dbName; std::string prio, output; bool doUpdate = false; - auto prioPtr = db.hget(key, DAEMON_LOGLEVEL, false); - auto outputPtr = db.hget(key, DAEMON_LOGOUTPUT, false); + auto prioPtr = db.hget(key, DAEMON_LOGLEVEL); + auto outputPtr = db.hget(key, DAEMON_LOGOUTPUT); if (prioPtr == nullptr) { diff --git a/common/loglevel.cpp b/common/loglevel.cpp index e18b03d08..1be85c099 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -134,7 +134,7 @@ int main(int argc, char **argv) for (const auto& key : keys) { const auto redis_key = std::string(key).append(":").append(key); - auto level = db.hget(redis_key, DAEMON_LOGLEVEL, false); + auto level = db.hget(redis_key, DAEMON_LOGLEVEL); if (nullptr == level) { std::cerr << std::left << std::setw(30) << key << "Unknown log level" << std::endl; diff --git a/common/producerstatetable.cpp b/common/producerstatetable.cpp index 8c0b38b20..7a8b11fcd 100644 --- a/common/producerstatetable.cpp +++ b/common/producerstatetable.cpp @@ -217,7 +217,7 @@ void ProducerStateTable::apply_temp_view() TableDump currentState; { - Table mainTable(m_pipe, getTableName(), false); + Table mainTable(m_pipe, getTableName(), false, nullptr); mainTable.dump(currentState); } diff --git a/common/redisclient.h b/common/redisclient.h index 85a9f6f76..e95d68396 100644 --- a/common/redisclient.h +++ b/common/redisclient.h @@ -32,12 +32,7 @@ class RedisClient int64_t hdel(const std::string &key, const std::vector &fields) { return m_db->hdel(key, fields); } -#ifndef SWIG - [[deprecated("Please use hgetall(const std::string &key, bool withDefaultValue) instead.")]] -#endif - std::unordered_map hgetall(const std::string &key) { return hgetall(key, false); } - - std::unordered_map hgetall(const std::string &key, bool withDefaultValue) { return m_db->hgetall(key, withDefaultValue); } + std::unordered_map hgetall(const std::string &key) { return m_db->hgetall(key); } template void hgetall(const std::string &key, OutputIterator result) { return m_db->hgetall(key, result); } @@ -53,12 +48,7 @@ class RedisClient std::shared_ptr get(const std::string &key) { return m_db->get(key); } -#ifndef SWIG - [[deprecated("Please use hget(const std::string &key, const std::string &field, bool withDefaultValue) instead.")]] -#endif - std::shared_ptr hget(const std::string &key, const std::string &field) { return hget(key, field, false); } - - std::shared_ptr hget(const std::string &key, const std::string &field, bool withDefaultValue) { return m_db->hget(key, field, withDefaultValue); } + std::shared_ptr hget(const std::string &key, const std::string &field) { return m_db->hget(key, field); } int64_t incr(const std::string &key) { return m_db->incr(key); } diff --git a/common/sonicv2connector.cpp b/common/sonicv2connector.cpp index b143e5df8..59552cde2 100644 --- a/common/sonicv2connector.cpp +++ b/common/sonicv2connector.cpp @@ -76,12 +76,7 @@ std::pair> SonicV2Connector_Native::scan(const std std::shared_ptr SonicV2Connector_Native::get(const std::string& db_name, const std::string& _hash, const std::string& key, bool blocking) { - return get(db_name, _hash, key, false, blocking); -} - -std::shared_ptr SonicV2Connector_Native::get(const std::string& db_name, const std::string& _hash, const std::string& key, bool withDefaultValue, bool blocking) -{ - return m_dbintf.get(db_name, _hash, key, withDefaultValue, blocking); + return m_dbintf.get(db_name, _hash, key, blocking); } bool SonicV2Connector_Native::hexists(const std::string& db_name, const std::string& _hash, const std::string& key) @@ -91,12 +86,7 @@ bool SonicV2Connector_Native::hexists(const std::string& db_name, const std::str std::map SonicV2Connector_Native::get_all(const std::string& db_name, const std::string& _hash, bool blocking) { - return get_all(db_name, _hash, false, blocking); -} - -std::map SonicV2Connector_Native::get_all(const std::string& db_name, const std::string& _hash, bool withDefaultValue, bool blocking) -{ - return m_dbintf.get_all(db_name, _hash, withDefaultValue, blocking); + return m_dbintf.get_all(db_name, _hash, blocking); } void SonicV2Connector_Native::hmset(const std::string& db_name, const std::string &key, const std::map &values) diff --git a/common/sonicv2connector.h b/common/sonicv2connector.h index f354e164f..9ebc2d861 100644 --- a/common/sonicv2connector.h +++ b/common/sonicv2connector.h @@ -25,7 +25,7 @@ class SonicV2Connector_Native std::string get_db_separator(const std::string& db_name); - DBConnector& get_redis_client(const std::string& db_name); + virtual DBConnector& get_redis_client(const std::string& db_name); int64_t publish(const std::string& db_name, const std::string& channel, const std::string& message); @@ -35,22 +35,12 @@ class SonicV2Connector_Native std::pair> scan(const std::string& db_name, int cursor = 0, const char *match = "", uint32_t count = 10); -#ifndef SWIG - [[deprecated("Please use get(const std::string& db_name, const std::string& _hash, const std::string& key, bool withDefaultValue, bool blocking) instead.")]] -#endif std::shared_ptr get(const std::string& db_name, const std::string& _hash, const std::string& key, bool blocking=false); - std::shared_ptr get(const std::string& db_name, const std::string& _hash, const std::string& key, bool withDefaultValue, bool blocking); - bool hexists(const std::string& db_name, const std::string& _hash, const std::string& key); -#ifndef SWIG - [[deprecated("Please use get_all(const std::string& db_name, const std::string& _hash, bool withDefaultValue, bool blocking) instead.")]] -#endif std::map get_all(const std::string& db_name, const std::string& _hash, bool blocking=false); - std::map get_all(const std::string& db_name, const std::string& _hash, bool withDefaultValue, bool blocking); - void hmset(const std::string& db_name, const std::string &key, const std::map &values); int64_t set(const std::string& db_name, const std::string& _hash, const std::string& key, const std::string& val, bool blocking=false); diff --git a/common/subscriberstatetable.cpp b/common/subscriberstatetable.cpp index dc68572d7..b80f24ef5 100644 --- a/common/subscriberstatetable.cpp +++ b/common/subscriberstatetable.cpp @@ -15,15 +15,9 @@ using namespace std; namespace swss { SubscriberStateTable::SubscriberStateTable(DBConnector *db, const string &tableName, int popBatchSize, int pri) - : SubscriberStateTable(db, tableName, false, popBatchSize, pri) -{ -} - -SubscriberStateTable::SubscriberStateTable(DBConnector *db, const string &tableName, bool withDefaultValue, int popBatchSize, int pri) : ConsumerTableBase(db, tableName, popBatchSize, pri), m_table(db, tableName) { m_keyspace = "__keyspace@"; - m_withDefaultValue = withDefaultValue; m_keyspace += to_string(db->getDbId()) + "__:" + tableName + m_table.getTableNameSeparator() + "*"; @@ -39,7 +33,7 @@ SubscriberStateTable::SubscriberStateTable(DBConnector *db, const string &tableN kfvKey(kco) = key; kfvOp(kco) = SET_COMMAND; - if (!m_table.get(key, kfvFieldsValues(kco), m_withDefaultValue)) + if (!m_table.get(key, kfvFieldsValues(kco))) { continue; } @@ -153,7 +147,7 @@ void SubscriberStateTable::pops(deque &vkco, const strin } else { - if (!m_table.get(key, kfvFieldsValues(kco), m_withDefaultValue)) + if (!m_table.get(key, kfvFieldsValues(kco))) { SWSS_LOG_NOTICE("Miss table key %s, possibly outdated", table_entry.c_str()); continue; diff --git a/common/subscriberstatetable.h b/common/subscriberstatetable.h index d4c0700f3..432dc90fb 100644 --- a/common/subscriberstatetable.h +++ b/common/subscriberstatetable.h @@ -11,11 +11,7 @@ namespace swss { class SubscriberStateTable : public ConsumerTableBase { public: -#ifndef SWIG - [[deprecated("Please use SubscriberStateTable(DBConnector *db, const std::string &tableName, bool withDefaultValue, int popBatchSize = DEFAULT_POP_BATCH_SIZE, int pri = 0) instead.")]] -#endif SubscriberStateTable(DBConnector *db, const std::string &tableName, int popBatchSize = DEFAULT_POP_BATCH_SIZE, int pri = 0); - SubscriberStateTable(DBConnector *db, const std::string &tableName, bool withDefaultValue, int popBatchSize = DEFAULT_POP_BATCH_SIZE, int pri = 0); /* Get all elements available */ void pops(std::deque &vkco, const std::string &prefix = EMPTY_PREFIX); @@ -37,7 +33,6 @@ class SubscriberStateTable : public ConsumerTableBase std::deque> m_keyspace_event_buffer; Table m_table; - bool m_withDefaultValue; }; } diff --git a/common/table.cpp b/common/table.cpp index 1cc7f3d79..11945aeac 100644 --- a/common/table.cpp +++ b/common/table.cpp @@ -1,6 +1,7 @@ #include #include +#include "common/dbdecorator.h" #include "common/table.h" #include "common/logger.h" #include "common/redisreply.h" @@ -33,16 +34,24 @@ const TableNameSeparatorMap TableBase::tableNameSeparatorMap = { }; Table::Table(const DBConnector *db, const string &tableName) - : Table(new RedisPipeline(db, 1), tableName, false) + : Table(new RedisPipeline(db, 1), tableName, false, nullptr) { m_pipeowned = true; + m_db_decorator = db->getDBDecortor(); } Table::Table(RedisPipeline *pipeline, const string &tableName, bool buffered) + : Table(pipeline, tableName, buffered, nullptr) +{ + // TODO: remove this deprecated ctor +} + +Table::Table(RedisPipeline *pipeline, const string &tableName, bool buffered, std::shared_ptr db_decorator) : TableBase(tableName, SonicDBConfig::getSeparator(pipeline->getDBConnector())) , m_buffered(buffered) , m_pipeowned(false) , m_pipe(pipeline) + , m_db_decorator(db_decorator) { } @@ -66,12 +75,6 @@ void Table::flush() bool Table::get(const string &key, vector &values) { - return get(key, values, false); -} - -bool Table::get(const string &key, vector &values, bool withDefaultValue) -{ - // [Hua] TODO: code here dupe with DBConnector::hgetall, check if can reuse it. RedisCommand hgetall_key; hgetall_key.format("HGETALL %s", getKeyName(key).c_str()); RedisReply r = m_pipe->push(hgetall_key, REDIS_REPLY_ARRAY); @@ -91,19 +94,10 @@ bool Table::get(const string &key, vector &values, bool withDef reply->element[i + 1]->str); } - if (withDefaultValue && m_pipe->getDbId() == CONFIG_DB) + // Decorate result because result is not from DBConnection, so it's not decorated + if (m_db_decorator) { - size_t pos = key.find("|"); - if (pos == std::string::npos) - { - SWSS_LOG_WARN("Table::get key for config DB is %s, can't find a sepreator\n", key.c_str()); - } - else - { - std::string table = key.substr(0, pos); - std::string row = key.substr(pos + 1); - DefaultValueProvider::Instance().AppendDefaultValues(table, row, values); - } + m_db_decorator->decorate(key, values); } return true; @@ -210,11 +204,6 @@ void Table::hdel(const string &key, const string &field, const string& /* op */, } void TableEntryEnumerable::getContent(vector &tuples) -{ - return getContent(tuples, false); -} - -void TableEntryEnumerable::getContent(vector &tuples, bool withDefaultValue) { vector keys; getKeys(keys); @@ -226,7 +215,7 @@ void TableEntryEnumerable::getContent(vector &tuples, bo vector values; string op = ""; - get(key, values, withDefaultValue); + get(key, values); tuples.emplace_back(key, op, values); } } diff --git a/common/table.h b/common/table.h index 5b1771338..665812a2b 100644 --- a/common/table.h +++ b/common/table.h @@ -10,6 +10,7 @@ #include #include "hiredis/hiredis.h" #include "dbconnector.h" +#include "dbdecorator.h" #include "redisreply.h" #include "redisselect.h" #include "redispipeline.h" @@ -166,22 +167,14 @@ class TableEntryEnumerable { virtual ~TableEntryEnumerable() = default; /* Get all the field-value tuple of the table entry with the key */ -#ifndef SWIG - [[deprecated("Please use get(const std::string &key, std::vector &values, bool withDefaultValue) instead.")]] -#endif virtual bool get(const std::string &key, std::vector &values) = 0; - virtual bool get(const std::string &key, std::vector &values, bool withDefaultValue) = 0; /* get all the keys in the table */ virtual void getKeys(std::vector &keys) = 0; /* Read the whole table content from the DB directly */ /* NOTE: Not an atomic function */ -#ifndef SWIG - [[deprecated("Please use getContent(std::vector &tuples, bool withDefaultValue) instead.")]] -#endif void getContent(std::vector &tuples); - void getContent(std::vector &tuples, bool withDefaultValue); }; /* The default time to live for a DB entry is infinite */ @@ -190,7 +183,11 @@ static constexpr int64_t DEFAULT_DB_TTL = -1; class Table : public TableBase, public TableEntryEnumerable { public: Table(const DBConnector *db, const std::string &tableName); +#ifndef SWIG + [[deprecated("Please use Table(RedisPipeline *pipeline, const std::string &tableName, bool buffered, std::shared_ptr m_db_decorator instead.")]] +#endif Table(RedisPipeline *pipeline, const std::string &tableName, bool buffered); + Table(RedisPipeline *pipeline, const std::string &tableName, bool buffered, std::shared_ptr m_db_decorator); ~Table() override; /* Set an entry in the DB directly (op not in use) */ @@ -230,11 +227,7 @@ class Table : public TableBase, public TableEntryEnumerable { const std::string &prefix = EMPTY_PREFIX); /* Read a value from the DB directly */ /* Returns false if the key doesn't exists */ -#ifndef SWIG - [[deprecated("Please use get(const std::string &key, std::vector &ovalues, bool withDefaultValue) instead.")]] -#endif virtual bool get(const std::string &key, std::vector &ovalues); - virtual bool get(const std::string &key, std::vector &ovalues, bool withDefaultValue); virtual bool hget(const std::string &key, const std::string &field, std::string &value); virtual void hset(const std::string &key, @@ -266,6 +259,7 @@ class Table : public TableBase, public TableEntryEnumerable { * */ std::string stripSpecialSym(const std::string &key); std::string m_shaDump; + std::shared_ptr m_db_decorator; }; class TableName_KeyValueOpQueues { diff --git a/pyext/swsscommon.i b/pyext/swsscommon.i index ebf3e65fd..69180f5c0 100644 --- a/pyext/swsscommon.i +++ b/pyext/swsscommon.i @@ -14,6 +14,7 @@ #include "schema.h" #include "dbconnector.h" +#include "dbdecorator.h" #include "dbinterface.h" #include "sonicv2connector.h" #include "pubsub.h" @@ -47,6 +48,9 @@ %include %include +%shared_ptr(swss::DBDecorator); +%shared_ptr(swss::ConfigDBDecorator); + %template(FieldValuePair) std::pair; %template(FieldValuePairs) std::vector>; %template(FieldValueMap) std::map; @@ -150,6 +154,7 @@ T castSelectableObj(swss::Selectable *temp) %include "schema.h" %include "dbconnector.h" +%include "dbdecorator.h" %include "sonicv2connector.h" %include "pubsub.h" %include "selectable.h" diff --git a/tests/fdb_flush.cpp b/tests/fdb_flush.cpp index 0797b0917..7ba981c0d 100644 --- a/tests/fdb_flush.cpp +++ b/tests/fdb_flush.cpp @@ -80,7 +80,7 @@ void print() { printf("K %s\n", k.c_str()); - auto hash = db.hgetall(k, false); + auto hash = db.hgetall(k); for (auto&h: hash) { diff --git a/tests/logger_ut.cpp b/tests/logger_ut.cpp index 7b6e4acc8..c17c9c262 100644 --- a/tests/logger_ut.cpp +++ b/tests/logger_ut.cpp @@ -32,7 +32,7 @@ void prioNotify(const string &component, const string &prioStr) void checkLoglevel(DBConnector& db, const string& key, const string& loglevel) { string redis_key = key + ":" + key; - auto level = db.hget(redis_key, DAEMON_LOGLEVEL, false); + auto level = db.hget(redis_key, DAEMON_LOGLEVEL); EXPECT_FALSE(level == nullptr); if (level != nullptr) { diff --git a/tests/redis_multi_ns_ut.cpp b/tests/redis_multi_ns_ut.cpp index d5cd2f77f..7c7cf2c08 100644 --- a/tests/redis_multi_ns_ut.cpp +++ b/tests/redis_multi_ns_ut.cpp @@ -166,7 +166,7 @@ void TableNetnsTest(string tableName) cout << "- Step 3. GET_TABLE_CONTENT" << endl; vector tuples; - t.getContent(tuples, false); + t.getContent(tuples); cout << "Get total " << tuples.size() << " number of entries" << endl; EXPECT_EQ(tuples.size(), (size_t)2); @@ -198,8 +198,8 @@ void TableNetnsTest(string tableName) cout << "- Step 5. GET" << endl; cout << "Get key [a] and key [b]" << endl; - EXPECT_EQ(t.get(key_1, values, false), false); - t.get(key_2, values, false); + EXPECT_EQ(t.get(key_1, values), false); + t.get(key_2, values); cout << "Get key [b]" << flush; for (auto fv: values) @@ -220,7 +220,7 @@ void TableNetnsTest(string tableName) cout << "- Step 6. DEL and GET_TABLE_CONTENT" << endl; cout << "Delete key [b]" << endl; t.del(key_2); - t.getContent(tuples, false); + t.getContent(tuples); EXPECT_EQ(tuples.size(), unsigned(0)); diff --git a/tests/redis_piped_ut.cpp b/tests/redis_piped_ut.cpp index 80c7c22c1..a86d70167 100644 --- a/tests/redis_piped_ut.cpp +++ b/tests/redis_piped_ut.cpp @@ -323,7 +323,7 @@ TEST(Table, piped_test) string tableName = "TABLE_UT_TEST"; DBConnector db("TEST_DB", 0, true); RedisPipeline pipeline(&db); - Table t(&pipeline, tableName, true); + Table t(&pipeline, tableName, true, nullptr); clearDB(); cout << "Starting table manipulations" << endl; @@ -349,7 +349,7 @@ TEST(Table, piped_test) cout << "- Step 2. GET_TABLE_CONTENT" << endl; vector tuples; - t.getContent(tuples, false); + t.getContent(tuples); cout << "Get total " << tuples.size() << " number of entries" << endl; EXPECT_EQ(tuples.size(), (size_t)2); @@ -381,8 +381,8 @@ TEST(Table, piped_test) cout << "- Step 4. GET" << endl; cout << "Get key [a] and key [b]" << endl; - EXPECT_EQ(t.get(key_1, values, false), false); - t.get(key_2, values, false); + EXPECT_EQ(t.get(key_1, values), false); + t.get(key_2, values); cout << "Get key [b]" << flush; for (auto fv: values) @@ -403,7 +403,7 @@ TEST(Table, piped_test) cout << "- Step 5. DEL and GET_TABLE_CONTENT" << endl; cout << "Delete key [b]" << endl; t.del(key_2); - t.getContent(tuples, false); + t.getContent(tuples); EXPECT_EQ(tuples.size(), unsigned(0)); diff --git a/tests/redis_state_ut.cpp b/tests/redis_state_ut.cpp index b74bf8625..3d965103b 100644 --- a/tests/redis_state_ut.cpp +++ b/tests/redis_state_ut.cpp @@ -122,7 +122,7 @@ static void consumerWorker(int index) for (auto fv : kfvFieldsValues(kco)) { - string val = *db.hget(tableName + ":" + kfvKey(kco), fvField(fv), false); + string val = *db.hget(tableName + ":" + kfvKey(kco), fvField(fv)); EXPECT_EQ(val, fvValue(fv)); } } else if (kfvOp(kco) == "DEL") @@ -472,7 +472,7 @@ TEST(ConsumerStateTable, set_pop_del_set_pop_get) /* Get data directly from table in redis DB*/ Table t(&db, tableName); vector values; - t.get(key, values, false); + t.get(key, values); /* size of values should be maxNumOfFields, no "field 1" left from first set */ EXPECT_EQ(values.size(), (unsigned int)maxNumOfFields); diff --git a/tests/redis_ut.cpp b/tests/redis_ut.cpp index e5eac7f8e..4871848fa 100644 --- a/tests/redis_ut.cpp +++ b/tests/redis_ut.cpp @@ -202,7 +202,7 @@ void TableBasicTest(string tableName, bool useDbId = false) cout << "- Step 3. GET_TABLE_CONTENT" << endl; vector tuples; - t.getContent(tuples, false); + t.getContent(tuples); cout << "Get total " << tuples.size() << " number of entries" << endl; EXPECT_EQ(tuples.size(), (size_t)2); @@ -234,8 +234,8 @@ void TableBasicTest(string tableName, bool useDbId = false) cout << "- Step 5. GET" << endl; cout << "Get key [a] and key [b]" << endl; - EXPECT_EQ(t.get(key_1, values, false), false); - t.get(key_2, values, false); + EXPECT_EQ(t.get(key_1, values), false); + t.get(key_2, values); cout << "Get key [b]" << flush; for (auto fv: values) @@ -256,7 +256,7 @@ void TableBasicTest(string tableName, bool useDbId = false) cout << "- Step 6. DEL and GET_TABLE_CONTENT" << endl; cout << "Delete key [b]" << endl; t.del(key_2); - t.getContent(tuples, false); + t.getContent(tuples); EXPECT_EQ(tuples.size(), unsigned(0)); @@ -322,7 +322,7 @@ TEST(DBConnector, DBInterface) SonicV2Connector_Native db; db.connect("TEST_DB"); db.set("TEST_DB", "key0", "field1", "value2"); - auto fvs = db.get_all("TEST_DB", "key0", false, false); + auto fvs = db.get_all("TEST_DB", "key0", false); auto rc = fvs.find("field1"); EXPECT_NE(rc, fvs.end()); EXPECT_EQ(rc->second, "value2"); @@ -368,7 +368,7 @@ TEST(DBConnector, RedisClient) for (auto k : keys) { cout << "Get key [" << k << "]" << flush; - auto fvs = db.hgetall(k, false); + auto fvs = db.hgetall(k); unsigned int size_v = 3; EXPECT_EQ(fvs.size(), size_v); for (auto fv: fvs) @@ -392,7 +392,7 @@ TEST(DBConnector, RedisClient) int64_t rval = db.hdel(key_1, "field_2"); EXPECT_EQ(rval, 1); - auto fvs = db.hgetall(key_1, false); + auto fvs = db.hgetall(key_1); EXPECT_EQ(fvs.size(), 2); for (auto fv: fvs) { @@ -417,9 +417,9 @@ TEST(DBConnector, RedisClient) cout << "- Step 6. GET" << endl; cout << "Get key [a] and key [b]" << endl; - fvs = db.hgetall(key_1, false); + fvs = db.hgetall(key_1); EXPECT_TRUE(fvs.empty()); - fvs = db.hgetall(key_2, false); + fvs = db.hgetall(key_2); cout << "Get key [b]" << flush; for (auto fv: fvs) @@ -442,7 +442,7 @@ TEST(DBConnector, RedisClient) rval = db.hdel(key_2, vector({"field_2", "field_3"})); EXPECT_EQ(rval, 2); - fvs = db.hgetall(key_2, false); + fvs = db.hgetall(key_2); EXPECT_EQ(fvs.size(), 1); for (auto fv: fvs) { @@ -461,7 +461,7 @@ TEST(DBConnector, RedisClient) cout << "- Step 8. DEL and GET_TABLE_CONTENT" << endl; cout << "Delete key [b]" << endl; db.del(key_2); - fvs = db.hgetall(key_2, false); + fvs = db.hgetall(key_2); EXPECT_TRUE(fvs.empty()); @@ -714,7 +714,7 @@ TEST(Table, ttl_test) string tableName = "TABLE_UT_TEST"; DBConnector db("TEST_DB", 0, true); RedisPipeline pipeline(&db); - Table t(&pipeline, tableName, true); + Table t(&pipeline, tableName, true, nullptr); clearDB(); cout << "Starting table manipulations" << endl; diff --git a/tests/selectable_priority.cpp b/tests/selectable_priority.cpp index 38512edfe..778514558 100644 --- a/tests/selectable_priority.cpp +++ b/tests/selectable_priority.cpp @@ -59,7 +59,7 @@ TEST(Priority, set_pri_values) RedisSelect rs(105); SelectableEvent se(106); SelectableTimer st(interval, 107); - SubscriberStateTable sst(&db, tableName, false, DEFAULT_POP_BATCH_SIZE, 108); + SubscriberStateTable sst(&db, tableName, DEFAULT_POP_BATCH_SIZE, 108); EXPECT_EQ(nl.getPri(), 101); EXPECT_EQ(cst.getPri(), 102); From 73f3e52d55ada15c9da2d4f36e9c7fafdca309d2 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 27 Apr 2022 08:34:23 +0000 Subject: [PATCH 20/24] Improve code --- common/configdb.cpp | 7 ++----- common/dbconnector.h | 7 +------ common/dbdecorator.h | 2 +- common/defaultvalueprovider.h | 2 +- common/producerstatetable.cpp | 2 +- common/table.cpp | 11 ++--------- common/table.h | 4 ---- tests/redis_piped_ut.cpp | 2 +- tests/redis_subscriber_state_ut.cpp | 14 +++++++------- tests/redis_ut.cpp | 4 ++-- tests/selectable_priority.cpp | 2 +- 11 files changed, 19 insertions(+), 38 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index 01b103372..55464f02e 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -4,7 +4,6 @@ #include "configdb.h" #include "pubsub.h" #include "converter.h" -#include "defaultvalueprovider.h" using namespace std; using namespace swss; @@ -204,10 +203,8 @@ map> ConfigDBConnector_Native::get_table(string tabl continue; } row = key.substr(pos + 1); - data[row] = entry; } - return data; } @@ -282,7 +279,6 @@ map>> ConfigDBConnector_Native::get_conf data[table_name][row] = entry; } } - return data; } @@ -305,6 +301,7 @@ std::string ConfigDBConnector_Native::getDbName() const DBConnector& ConfigDBConnector_Native::get_redis_client(const std::string& db_name) { auto& result = SonicV2Connector_Native::get_redis_client(db_name); + // TODO: find a better way to attach decorator, because attach decorator here will impact other connector which share same dbconnection, also performance issue. result.setDBDecortor(m_db_decorator); return result; } @@ -549,4 +546,4 @@ map>> ConfigDBPipeConnector_Native::get_ } return data; -} \ No newline at end of file +} diff --git a/common/dbconnector.h b/common/dbconnector.h index 7bbf9ce64..e5a5b5006 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -5,17 +5,13 @@ #include #include #include -#include #include #include #include #include "dbdecorator.h" -#include "defaultvalueprovider.h" #include "rediscommand.h" #include "redisreply.h" -#include "schema.h" -#include "logger.h" #define EMPTY_NAMESPACE std::string() namespace swss { @@ -161,7 +157,6 @@ class DBConnector : public RedisContext DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn = false); DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn, const std::string &netns); DBConnector& operator=(const DBConnector&) = delete; - virtual ~DBConnector(); int getDbId() const; std::string getDbName() const; @@ -262,7 +257,7 @@ class DBConnector : public RedisContext std::string m_shaRedisMulti; - std::shared_ptr m_db_decorator; + std::shared_ptr m_db_decorator = nullptr; }; template diff --git a/common/dbdecorator.h b/common/dbdecorator.h index 9ca2ed74d..59253a240 100644 --- a/common/dbdecorator.h +++ b/common/dbdecorator.h @@ -47,4 +47,4 @@ class ConfigDBDecorator : public DBDecorator std::string m_separator; }; -} \ No newline at end of file +} diff --git a/common/defaultvalueprovider.h b/common/defaultvalueprovider.h index 3617d57ef..b6fe89568 100644 --- a/common/defaultvalueprovider.h +++ b/common/defaultvalueprovider.h @@ -109,4 +109,4 @@ class DefaultValueProvider FieldDefaultValueMappingPtr GetDefaultValueInfo(struct lys_node* tableChildNode); }; -} \ No newline at end of file +} diff --git a/common/producerstatetable.cpp b/common/producerstatetable.cpp index 7a8b11fcd..8c0b38b20 100644 --- a/common/producerstatetable.cpp +++ b/common/producerstatetable.cpp @@ -217,7 +217,7 @@ void ProducerStateTable::apply_temp_view() TableDump currentState; { - Table mainTable(m_pipe, getTableName(), false, nullptr); + Table mainTable(m_pipe, getTableName(), false); mainTable.dump(currentState); } diff --git a/common/table.cpp b/common/table.cpp index 11945aeac..b4eb03c5c 100644 --- a/common/table.cpp +++ b/common/table.cpp @@ -34,24 +34,17 @@ const TableNameSeparatorMap TableBase::tableNameSeparatorMap = { }; Table::Table(const DBConnector *db, const string &tableName) - : Table(new RedisPipeline(db, 1), tableName, false, nullptr) + : Table(new RedisPipeline(db, 1), tableName, false) { m_pipeowned = true; - m_db_decorator = db->getDBDecortor(); } Table::Table(RedisPipeline *pipeline, const string &tableName, bool buffered) - : Table(pipeline, tableName, buffered, nullptr) -{ - // TODO: remove this deprecated ctor -} - -Table::Table(RedisPipeline *pipeline, const string &tableName, bool buffered, std::shared_ptr db_decorator) : TableBase(tableName, SonicDBConfig::getSeparator(pipeline->getDBConnector())) , m_buffered(buffered) , m_pipeowned(false) , m_pipe(pipeline) - , m_db_decorator(db_decorator) + , m_db_decorator(pipeline->getDBConnector()->getDBDecortor()) { } diff --git a/common/table.h b/common/table.h index 665812a2b..f60770120 100644 --- a/common/table.h +++ b/common/table.h @@ -183,11 +183,7 @@ static constexpr int64_t DEFAULT_DB_TTL = -1; class Table : public TableBase, public TableEntryEnumerable { public: Table(const DBConnector *db, const std::string &tableName); -#ifndef SWIG - [[deprecated("Please use Table(RedisPipeline *pipeline, const std::string &tableName, bool buffered, std::shared_ptr m_db_decorator instead.")]] -#endif Table(RedisPipeline *pipeline, const std::string &tableName, bool buffered); - Table(RedisPipeline *pipeline, const std::string &tableName, bool buffered, std::shared_ptr m_db_decorator); ~Table() override; /* Set an entry in the DB directly (op not in use) */ diff --git a/tests/redis_piped_ut.cpp b/tests/redis_piped_ut.cpp index a86d70167..29978cdff 100644 --- a/tests/redis_piped_ut.cpp +++ b/tests/redis_piped_ut.cpp @@ -323,7 +323,7 @@ TEST(Table, piped_test) string tableName = "TABLE_UT_TEST"; DBConnector db("TEST_DB", 0, true); RedisPipeline pipeline(&db); - Table t(&pipeline, tableName, true, nullptr); + Table t(&pipeline, tableName, true); clearDB(); cout << "Starting table manipulations" << endl; diff --git a/tests/redis_subscriber_state_ut.cpp b/tests/redis_subscriber_state_ut.cpp index 7425bd47a..50722b81f 100644 --- a/tests/redis_subscriber_state_ut.cpp +++ b/tests/redis_subscriber_state_ut.cpp @@ -181,7 +181,7 @@ TEST(SubscriberStateTable, set) int maxNumOfFields = 2; /* Prepare subscriber */ - SubscriberStateTable c(&db, testTableName, false); + SubscriberStateTable c(&db, testTableName); Select cs; Selectable *selectcs; cs.addSelectable(&c); @@ -232,7 +232,7 @@ TEST(SubscriberStateTable, set2_pop1_set1_pop1) int maxNumOfFields = 2; /* Prepare subscriber */ - SubscriberStateTable c(&db, testTableName, false); + SubscriberStateTable c(&db, testTableName); Select cs; Selectable *selectcs; cs.addSelectable(&c); @@ -363,7 +363,7 @@ TEST(SubscriberStateTable, pops_intial) } /* Prepare subscriber */ - SubscriberStateTable c(&db, testTableName, false); + SubscriberStateTable c(&db, testTableName); Select cs; cs.addSelectable(&c); std::deque entries; @@ -408,7 +408,7 @@ TEST(SubscriberStateTable, del) int maxNumOfFields = 2; /* Prepare subscriber */ - SubscriberStateTable c(&db, testTableName, false); + SubscriberStateTable c(&db, testTableName); Select cs; Selectable *selectcs; cs.addSelectable(&c); @@ -475,7 +475,7 @@ TEST(SubscriberStateTable, table_state) } /* Prepare subscriber */ - SubscriberStateTable c(&db, testTableName, false); + SubscriberStateTable c(&db, testTableName); Select cs; Selectable *selectcs; int ret, i = 0; @@ -583,8 +583,8 @@ TEST(SubscriberStateTable, cachedData) } /* Prepare subscriber */ - SubscriberStateTable c1(&db, testTableName, false); - SubscriberStateTable c2(&db, testTableName2, false); + SubscriberStateTable c1(&db, testTableName); + SubscriberStateTable c2(&db, testTableName2); Select cs; Selectable *selectcs; cs.addSelectable(&c1); diff --git a/tests/redis_ut.cpp b/tests/redis_ut.cpp index 4871848fa..dd6292f0c 100644 --- a/tests/redis_ut.cpp +++ b/tests/redis_ut.cpp @@ -322,7 +322,7 @@ TEST(DBConnector, DBInterface) SonicV2Connector_Native db; db.connect("TEST_DB"); db.set("TEST_DB", "key0", "field1", "value2"); - auto fvs = db.get_all("TEST_DB", "key0", false); + auto fvs = db.get_all("TEST_DB", "key0"); auto rc = fvs.find("field1"); EXPECT_NE(rc, fvs.end()); EXPECT_EQ(rc->second, "value2"); @@ -714,7 +714,7 @@ TEST(Table, ttl_test) string tableName = "TABLE_UT_TEST"; DBConnector db("TEST_DB", 0, true); RedisPipeline pipeline(&db); - Table t(&pipeline, tableName, true, nullptr); + Table t(&pipeline, tableName, true); clearDB(); cout << "Starting table manipulations" << endl; diff --git a/tests/selectable_priority.cpp b/tests/selectable_priority.cpp index 778514558..24fbd3760 100644 --- a/tests/selectable_priority.cpp +++ b/tests/selectable_priority.cpp @@ -32,7 +32,7 @@ TEST(Priority, default_pri_values) RedisSelect rs; SelectableEvent se; SelectableTimer st(interval); - SubscriberStateTable sst(&db, tableName, false); + SubscriberStateTable sst(&db, tableName); EXPECT_EQ(nl.getPri(), 0); EXPECT_EQ(cst.getPri(), 0); From c0279c76cebf64ed896fc15023503cd266ca0512 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 27 Apr 2022 09:11:14 +0000 Subject: [PATCH 21/24] Improve code and fix typo --- common/configdb.cpp | 28 +++++++++++----------------- common/configdb.h | 3 --- common/dbconnector.cpp | 18 ++++++------------ common/dbconnector.h | 6 +++--- common/sonicv2connector.h | 2 +- common/table.cpp | 6 +++--- common/table.h | 1 - tests/redis_subscriber_state_ut.cpp | 2 +- 8 files changed, 25 insertions(+), 41 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index 55464f02e..f625a81dc 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -26,11 +26,6 @@ ConfigDBConnector_Native::ConfigDBConnector_Native(bool get_default_value, bool m_get_default_value = true; } #endif - - if (m_get_default_value) - { - m_db_decorator = ConfigDBDecorator::Create(m_table_name_separator); - } } void ConfigDBConnector_Native::db_connect(string db_name, bool wait_for_init, bool retry_on) @@ -38,10 +33,17 @@ void ConfigDBConnector_Native::db_connect(string db_name, bool wait_for_init, bo m_db_name = db_name; m_key_separator = m_table_name_separator = get_db_separator(db_name); SonicV2Connector_Native::connect(m_db_name, retry_on); + + // attach decorator to client + auto& client = get_redis_client(m_db_name); + if (m_get_default_value) + { + auto decorator = ConfigDBDecorator::Create(m_table_name_separator); + client.setDBDecorator(decorator); + } if (wait_for_init) { - auto& client = get_redis_client(m_db_name); auto pubsub = client.pubsub(); auto initialized = client.get(INIT_INDICATOR); if (!initialized || initialized->empty()) @@ -297,15 +299,6 @@ std::string ConfigDBConnector_Native::getDbName() const return m_db_name; } - -DBConnector& ConfigDBConnector_Native::get_redis_client(const std::string& db_name) -{ - auto& result = SonicV2Connector_Native::get_redis_client(db_name); - // TODO: find a better way to attach decorator, because attach decorator here will impact other connector which share same dbconnection, also performance issue. - result.setDBDecortor(m_db_decorator); - return result; -} - ConfigDBPipeConnector_Native::ConfigDBPipeConnector_Native(bool use_unix_socket_path, const char *netns) : ConfigDBConnector_Native(false, use_unix_socket_path, netns) { @@ -524,9 +517,10 @@ int ConfigDBPipeConnector_Native::_get_config(DBConnector& client, RedisTransact } // Because run Redis command with pipe not use DBConnector, so need decorate result here. - if (m_db_decorator) + auto& db_decorator = client.getDBDecorator(); + if (db_decorator != nullptr) { - m_db_decorator->decorate(key, dataentry); + db_decorator->decorate(key, dataentry); } } return cur; diff --git a/common/configdb.h b/common/configdb.h index 1287f884b..666e97f98 100644 --- a/common/configdb.h +++ b/common/configdb.h @@ -35,15 +35,12 @@ class ConfigDBConnector_Native : public SonicV2Connector_Native std::string getTableNameSeparator() const; std::string getDbName() const; - DBConnector& get_redis_client(const std::string& db_name) override; - protected: std::string m_table_name_separator = "|"; std::string m_key_separator = "|"; std::string m_db_name; bool m_get_default_value; - std::shared_ptr m_db_decorator; }; #ifdef SWIG diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index 4798683b5..4bc63050e 100644 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -512,8 +512,8 @@ DBConnector::DBConnector(const DBConnector &other) : RedisContext(other) , m_dbId(other.m_dbId) , m_namespace(other.m_namespace) + , m_db_decorator(other.m_db_decorator) { - m_db_decorator = nullptr; select(this); } @@ -521,7 +521,6 @@ DBConnector::DBConnector(int dbId, const RedisContext& ctx) : RedisContext(ctx) , m_dbId(dbId) , m_namespace(EMPTY_NAMESPACE) - , m_db_decorator(nullptr) { select(this); } @@ -530,7 +529,6 @@ DBConnector::DBConnector(int dbId, const string& hostname, int port, unsigned int timeout) : m_dbId(dbId) , m_namespace(EMPTY_NAMESPACE) - , m_db_decorator(nullptr) { struct timeval tv = {0, (suseconds_t)timeout * 1000}; struct timeval *ptv = timeout ? &tv : NULL; @@ -542,7 +540,6 @@ DBConnector::DBConnector(int dbId, const string& hostname, int port, DBConnector::DBConnector(int dbId, const string& unixPath, unsigned int timeout) : m_dbId(dbId) , m_namespace(EMPTY_NAMESPACE) - , m_db_decorator(nullptr) { struct timeval tv = {0, (suseconds_t)timeout * 1000}; struct timeval *ptv = timeout ? &tv : NULL; @@ -555,7 +552,6 @@ DBConnector::DBConnector(const string& dbName, unsigned int timeout, bool isTcpC : m_dbId(SonicDBConfig::getDbId(dbName, netns)) , m_dbName(dbName) , m_namespace(netns) - , m_db_decorator(nullptr) { struct timeval tv = {0, (suseconds_t)timeout * 1000}; struct timeval *ptv = timeout ? &tv : NULL; @@ -577,10 +573,6 @@ DBConnector::DBConnector(const string& dbName, unsigned int timeout, bool isTcpC // Empty constructor } -DBConnector::~DBConnector() -{ -} - int DBConnector::getDbId() const { return m_dbId; @@ -617,6 +609,7 @@ DBConnector *DBConnector::newConnector(unsigned int timeout) const ret->m_dbName = m_dbName; ret->setNamespace(getNamespace()); + ret->m_db_decorator = m_db_decorator; return ret; } @@ -790,7 +783,7 @@ shared_ptr DBConnector::hget(const string &key, const string &field) if (reply->type == REDIS_REPLY_NIL) { - auto dbdecortor = this->getDBDecortor(); + auto dbdecortor = this->getDBDecorator(); if (dbdecortor) { return dbdecortor->decorate(key, field); @@ -935,16 +928,17 @@ void DBConnector::del(const std::vector& keys) } -void DBConnector::setDBDecortor(std::shared_ptr &db_decorator) +void DBConnector::setDBDecorator(std::shared_ptr &db_decorator) { m_db_decorator = db_decorator; } -const std::shared_ptr &DBConnector::getDBDecortor() const +const std::shared_ptr &DBConnector::getDBDecorator() const { return m_db_decorator; } +// TODO: Need discussion connector design, remove following code after discussion. /* CfgDBConnector::CfgDBConnector(const DBConnector &other, bool getDefaultValue) : DBConnector(other) diff --git a/common/dbconnector.h b/common/dbconnector.h index e5a5b5006..69db4338e 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -244,9 +244,9 @@ class DBConnector : public RedisContext bool flushdb(); - void setDBDecortor(std::shared_ptr &db_decorator); + void setDBDecorator(std::shared_ptr &db_decorator); - const std::shared_ptr &getDBDecortor() const; + const std::shared_ptr &getDBDecorator() const; private: void setNamespace(const std::string &netns); @@ -284,7 +284,7 @@ void DBConnector::hgetall(const std::string &key, OutputIterator result) ++result; } - auto dbdecortor = this->getDBDecortor(); + auto dbdecortor = this->getDBDecorator(); if (dbdecortor) { dbdecortor->decorate(key, ctx, result); diff --git a/common/sonicv2connector.h b/common/sonicv2connector.h index 9ebc2d861..541b5835f 100644 --- a/common/sonicv2connector.h +++ b/common/sonicv2connector.h @@ -25,7 +25,7 @@ class SonicV2Connector_Native std::string get_db_separator(const std::string& db_name); - virtual DBConnector& get_redis_client(const std::string& db_name); + DBConnector& get_redis_client(const std::string& db_name); int64_t publish(const std::string& db_name, const std::string& channel, const std::string& message); diff --git a/common/table.cpp b/common/table.cpp index b4eb03c5c..35a2d30ae 100644 --- a/common/table.cpp +++ b/common/table.cpp @@ -44,7 +44,6 @@ Table::Table(RedisPipeline *pipeline, const string &tableName, bool buffered) , m_buffered(buffered) , m_pipeowned(false) , m_pipe(pipeline) - , m_db_decorator(pipeline->getDBConnector()->getDBDecortor()) { } @@ -88,9 +87,10 @@ bool Table::get(const string &key, vector &values) } // Decorate result because result is not from DBConnection, so it's not decorated - if (m_db_decorator) + auto& db_decorator = m_pipe->getDBConnector()->getDBDecorator(); + if (db_decorator != nullptr) { - m_db_decorator->decorate(key, values); + db_decorator->decorate(key, values); } return true; diff --git a/common/table.h b/common/table.h index f60770120..f73bba16b 100644 --- a/common/table.h +++ b/common/table.h @@ -255,7 +255,6 @@ class Table : public TableBase, public TableEntryEnumerable { * */ std::string stripSpecialSym(const std::string &key); std::string m_shaDump; - std::shared_ptr m_db_decorator; }; class TableName_KeyValueOpQueues { diff --git a/tests/redis_subscriber_state_ut.cpp b/tests/redis_subscriber_state_ut.cpp index 50722b81f..34db9d48a 100644 --- a/tests/redis_subscriber_state_ut.cpp +++ b/tests/redis_subscriber_state_ut.cpp @@ -123,7 +123,7 @@ static void producerWorker(int index) static void subscriberWorker(int index, int *status) { DBConnector db("TEST_DB", 0, true); - SubscriberStateTable c(&db, testTableName, false); + SubscriberStateTable c(&db, testTableName); Select cs; Selectable *selectcs; int numberOfKeysSet = 0; From 28398d7ff5641a0fe3b325e3f141e32b0f77a748 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 27 Apr 2022 09:18:39 +0000 Subject: [PATCH 22/24] Improve code --- common/table.h | 1 - 1 file changed, 1 deletion(-) diff --git a/common/table.h b/common/table.h index f73bba16b..04f25ee0e 100644 --- a/common/table.h +++ b/common/table.h @@ -10,7 +10,6 @@ #include #include "hiredis/hiredis.h" #include "dbconnector.h" -#include "dbdecorator.h" #include "redisreply.h" #include "redisselect.h" #include "redispipeline.h" From 9f6e9be326c9bfb6032d60db2005d033c56f4d42 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 27 Apr 2022 09:45:11 +0000 Subject: [PATCH 23/24] Improve code --- common/configdb.h | 1 - common/dbdecorator.cpp | 12 ++++++------ common/dbdecorator.h | 4 ++-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/common/configdb.h b/common/configdb.h index 666e97f98..43302ad20 100644 --- a/common/configdb.h +++ b/common/configdb.h @@ -2,7 +2,6 @@ #include #include -#include #include "sonicv2connector.h" #include "redistran.h" diff --git a/common/dbdecorator.cpp b/common/dbdecorator.cpp index e66b21467..f03c9d83d 100644 --- a/common/dbdecorator.cpp +++ b/common/dbdecorator.cpp @@ -21,7 +21,7 @@ ConfigDBDecorator::ConfigDBDecorator(string separator) template -void ConfigDBDecorator::decorateInternal(const std::string &key, OutputIterator &result) +void ConfigDBDecorator::_decorate(const std::string &key, OutputIterator &result) { auto tableAndRow = getTableAndRow(key); if (POS(tableAndRow) == string::npos) @@ -34,16 +34,16 @@ void ConfigDBDecorator::decorateInternal(const std::string &key, OutputIterator void ConfigDBDecorator::decorate(const string &key, vector &result) { - decorateInternal(key, result); + _decorate(key, result); } void ConfigDBDecorator::decorate(const string &key, map &result) { - decorateInternal(key, result); + _decorate(key, result); } template -void ConfigDBDecorator::decorateInternal(const std::string &key, redisReply *&ctx, OutputIterator &result) +void ConfigDBDecorator::_decorate(const std::string &key, redisReply *&ctx, OutputIterator &result) { auto tableAndRow = getTableAndRow(key); if (POS(tableAndRow) == string::npos) @@ -75,12 +75,12 @@ void ConfigDBDecorator::decorateInternal(const std::string &key, redisReply *&ct void ConfigDBDecorator::decorate(const string &key, redisReply *&ctx, insert_iterator > &result) { - decorateInternal(key, ctx, result); + _decorate(key, ctx, result); } void ConfigDBDecorator::decorate(const string &key, redisReply *&ctx, insert_iterator > &result) { - decorateInternal(key, ctx, result); + _decorate(key, ctx, result); } shared_ptr ConfigDBDecorator::decorate(const string &key, const string &field) diff --git a/common/dbdecorator.h b/common/dbdecorator.h index 59253a240..19ee862d4 100644 --- a/common/dbdecorator.h +++ b/common/dbdecorator.h @@ -39,10 +39,10 @@ class ConfigDBDecorator : public DBDecorator // decorate will be invoke in template method, but virtual method not support template, so create internal method for common code. template - void decorateInternal(const std::string &key, redisReply *&ctx, OutputIterator &result); + void _decorate(const std::string &key, redisReply *&ctx, OutputIterator &result); template - void decorateInternal(const std::string &key, OutputIterator &result); + void _decorate(const std::string &key, OutputIterator &result); std::string m_separator; }; From b17f958bd157a2a7248c9bbb5ea7b87678555c1b Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 28 Apr 2022 05:41:39 +0000 Subject: [PATCH 24/24] Improve support for multiple decorator --- common/configdb.cpp | 4 ++-- common/dbconnector.cpp | 28 +++++++++++++++++++++------- common/dbconnector.h | 13 +++++++++---- common/dbdecorator.cpp | 26 +++++++++++++++----------- common/dbdecorator.h | 16 +++++++++++++--- common/table.cpp | 2 +- pyext/swsscommon.i | 2 +- 7 files changed, 62 insertions(+), 29 deletions(-) diff --git a/common/configdb.cpp b/common/configdb.cpp index f625a81dc..84906c40c 100644 --- a/common/configdb.cpp +++ b/common/configdb.cpp @@ -38,7 +38,7 @@ void ConfigDBConnector_Native::db_connect(string db_name, bool wait_for_init, bo auto& client = get_redis_client(m_db_name); if (m_get_default_value) { - auto decorator = ConfigDBDecorator::Create(m_table_name_separator); + auto decorator = ConfigDBReadDecorator::Create(m_table_name_separator); client.setDBDecorator(decorator); } @@ -517,7 +517,7 @@ int ConfigDBPipeConnector_Native::_get_config(DBConnector& client, RedisTransact } // Because run Redis command with pipe not use DBConnector, so need decorate result here. - auto& db_decorator = client.getDBDecorator(); + auto& db_decorator = client.getDBDecorator(ReadDecorator); if (db_decorator != nullptr) { db_decorator->decorate(key, dataentry); diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index 4bc63050e..f9a771dd4 100644 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -512,7 +512,7 @@ DBConnector::DBConnector(const DBConnector &other) : RedisContext(other) , m_dbId(other.m_dbId) , m_namespace(other.m_namespace) - , m_db_decorator(other.m_db_decorator) + , m_db_decorators(other.m_db_decorators) { select(this); } @@ -609,7 +609,7 @@ DBConnector *DBConnector::newConnector(unsigned int timeout) const ret->m_dbName = m_dbName; ret->setNamespace(getNamespace()); - ret->m_db_decorator = m_db_decorator; + ret->m_db_decorators = m_db_decorators; return ret; } @@ -783,7 +783,7 @@ shared_ptr DBConnector::hget(const string &key, const string &field) if (reply->type == REDIS_REPLY_NIL) { - auto dbdecortor = this->getDBDecorator(); + auto dbdecortor = this->getDBDecorator(ReadDecorator); if (dbdecortor) { return dbdecortor->decorate(key, field); @@ -928,14 +928,28 @@ void DBConnector::del(const std::vector& keys) } -void DBConnector::setDBDecorator(std::shared_ptr &db_decorator) +const std::shared_ptr DBConnector::setDBDecorator(std::shared_ptr &db_decorator) { - m_db_decorator = db_decorator; + auto type = db_decorator->type(); + auto existed = getDBDecorator(type); + m_db_decorators[type] = db_decorator; + return existed; } -const std::shared_ptr &DBConnector::getDBDecorator() const +const std::shared_ptr DBConnector::getDBDecorator(swss::DBDecoratorType type) const { - return m_db_decorator; + auto existed = m_db_decorators.find(type); + std::shared_ptr existedDecorator = nullptr; + if (existed != m_db_decorators.end()) { + existedDecorator = existed->second; + } + + return existedDecorator; +} + +const DecoratorMapping &DBConnector::getDBDecorators() const +{ + return m_db_decorators; } // TODO: Need discussion connector design, remove following code after discussion. diff --git a/common/dbconnector.h b/common/dbconnector.h index 69db4338e..462684714 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -19,6 +19,8 @@ namespace swss { class DBConnector; class PubSub; +typedef std::map > DecoratorMapping; + class RedisInstInfo { public: @@ -244,9 +246,12 @@ class DBConnector : public RedisContext bool flushdb(); - void setDBDecorator(std::shared_ptr &db_decorator); + // every DBDecoratorType can ony have 1 decorator, set new one will return old one. + const std::shared_ptr setDBDecorator(std::shared_ptr &db_decorator); + + const std::shared_ptr getDBDecorator(swss::DBDecoratorType type) const; - const std::shared_ptr &getDBDecorator() const; + const DecoratorMapping &getDBDecorators() const; private: void setNamespace(const std::string &netns); @@ -257,7 +262,7 @@ class DBConnector : public RedisContext std::string m_shaRedisMulti; - std::shared_ptr m_db_decorator = nullptr; + DecoratorMapping m_db_decorators; }; template @@ -284,7 +289,7 @@ void DBConnector::hgetall(const std::string &key, OutputIterator result) ++result; } - auto dbdecortor = this->getDBDecorator(); + auto dbdecortor = this->getDBDecorator(ReadDecorator); if (dbdecortor) { dbdecortor->decorate(key, ctx, result); diff --git a/common/dbdecorator.cpp b/common/dbdecorator.cpp index f03c9d83d..304e34e01 100644 --- a/common/dbdecorator.cpp +++ b/common/dbdecorator.cpp @@ -14,14 +14,18 @@ using namespace swss; #define TABLE(TUPLE) get<1>(TUPLE) #define ROW(TUPLE) get<2>(TUPLE) -ConfigDBDecorator::ConfigDBDecorator(string separator) +ConfigDBReadDecorator::ConfigDBReadDecorator(string separator) :m_separator(separator) { } +DBDecoratorType ConfigDBReadDecorator::type() +{ + return ReadDecorator; +} template -void ConfigDBDecorator::_decorate(const std::string &key, OutputIterator &result) +void ConfigDBReadDecorator::_decorate(const std::string &key, OutputIterator &result) { auto tableAndRow = getTableAndRow(key); if (POS(tableAndRow) == string::npos) @@ -32,18 +36,18 @@ void ConfigDBDecorator::_decorate(const std::string &key, OutputIterator &result DefaultValueProvider::Instance().AppendDefaultValues(TABLE(tableAndRow), ROW(tableAndRow), result); } -void ConfigDBDecorator::decorate(const string &key, vector &result) +void ConfigDBReadDecorator::decorate(const string &key, vector &result) { _decorate(key, result); } -void ConfigDBDecorator::decorate(const string &key, map &result) +void ConfigDBReadDecorator::decorate(const string &key, map &result) { _decorate(key, result); } template -void ConfigDBDecorator::_decorate(const std::string &key, redisReply *&ctx, OutputIterator &result) +void ConfigDBReadDecorator::_decorate(const std::string &key, redisReply *&ctx, OutputIterator &result) { auto tableAndRow = getTableAndRow(key); if (POS(tableAndRow) == string::npos) @@ -73,17 +77,17 @@ void ConfigDBDecorator::_decorate(const std::string &key, redisReply *&ctx, Outp } } -void ConfigDBDecorator::decorate(const string &key, redisReply *&ctx, insert_iterator > &result) +void ConfigDBReadDecorator::decorate(const string &key, redisReply *&ctx, insert_iterator > &result) { _decorate(key, ctx, result); } -void ConfigDBDecorator::decorate(const string &key, redisReply *&ctx, insert_iterator > &result) +void ConfigDBReadDecorator::decorate(const string &key, redisReply *&ctx, insert_iterator > &result) { _decorate(key, ctx, result); } -shared_ptr ConfigDBDecorator::decorate(const string &key, const string &field) +shared_ptr ConfigDBReadDecorator::decorate(const string &key, const string &field) { auto tableAndRow = getTableAndRow(key); if (POS(tableAndRow) == string::npos) @@ -94,7 +98,7 @@ shared_ptr ConfigDBDecorator::decorate(const string &key, const string & return DefaultValueProvider::Instance().GetDefaultValue(TABLE(tableAndRow), ROW(tableAndRow), field); } -tuple ConfigDBDecorator::getTableAndRow(const string &key) +tuple ConfigDBReadDecorator::getTableAndRow(const string &key) { string table; string row; @@ -112,7 +116,7 @@ tuple ConfigDBDecorator::getTableAndRow(const string &ke return tuple(pos, table, row); } -shared_ptr ConfigDBDecorator::Create(string separator) +shared_ptr ConfigDBReadDecorator::Create(string separator) { - return shared_ptr(new ConfigDBDecorator(separator)); + return shared_ptr(new ConfigDBReadDecorator(separator)); } \ No newline at end of file diff --git a/common/dbdecorator.h b/common/dbdecorator.h index 19ee862d4..3c90b02fb 100644 --- a/common/dbdecorator.h +++ b/common/dbdecorator.h @@ -11,9 +11,18 @@ class redisReply; namespace swss { +// Decorator only designed for swss-common developer, not for user of swss-common. +// Suggest every decorator type only have 1 decorator class. +// And in DBConnector, every decorator type can only attach 1 instance, attach new instance will return old one. +enum DBDecoratorType +{ + ReadDecorator +}; + class DBDecorator { public: + virtual DBDecoratorType type() = 0; virtual void decorate(const std::string &key, std::vector > &result) = 0; virtual void decorate(const std::string &key, std::map &result) = 0; virtual void decorate(const std::string &key, redisReply *&ctx, std::insert_iterator > &result) = 0; @@ -21,11 +30,12 @@ class DBDecorator virtual std::shared_ptr decorate(const std::string &key, const std::string &field) = 0; }; -class ConfigDBDecorator : public DBDecorator +class ConfigDBReadDecorator : public DBDecorator { public: static std::shared_ptr Create(std::string separator); + DBDecoratorType type() override; void decorate(const std::string &key, std::vector > &result) override; void decorate(const std::string &key, std::map &result) override; void decorate(const std::string &key, redisReply *&ctx, std::insert_iterator > &result) override; @@ -33,8 +43,8 @@ class ConfigDBDecorator : public DBDecorator std::shared_ptr decorate(const std::string &key, const std::string &field) override; private: - // Because derived class shared_ptr issue, not allow create ConfigDBDecorator, please use Create() - ConfigDBDecorator(std::string separator); + // Because derived class shared_ptr issue, not allow create ConfigDBReadDecorator, please use Create() + ConfigDBReadDecorator(std::string separator); std::tuple getTableAndRow(const std::string &key); // decorate will be invoke in template method, but virtual method not support template, so create internal method for common code. diff --git a/common/table.cpp b/common/table.cpp index 35a2d30ae..9535ae8de 100644 --- a/common/table.cpp +++ b/common/table.cpp @@ -87,7 +87,7 @@ bool Table::get(const string &key, vector &values) } // Decorate result because result is not from DBConnection, so it's not decorated - auto& db_decorator = m_pipe->getDBConnector()->getDBDecorator(); + auto& db_decorator = m_pipe->getDBConnector()->getDBDecorator(ReadDecorator); if (db_decorator != nullptr) { db_decorator->decorate(key, values); diff --git a/pyext/swsscommon.i b/pyext/swsscommon.i index 69180f5c0..0e9cf3fc7 100644 --- a/pyext/swsscommon.i +++ b/pyext/swsscommon.i @@ -49,7 +49,7 @@ %include %shared_ptr(swss::DBDecorator); -%shared_ptr(swss::ConfigDBDecorator); +%shared_ptr(swss::ConfigDBReadDecorator); %template(FieldValuePair) std::pair; %template(FieldValuePairs) std::vector>;