[MultiDB]:swsscommon replace old API with new APIs including tests (#324

) * [MultiDB]:swsscommon replace old API with new APIs including tests * update code format * move db setup into global setup
sonic-net · Dec 13, 2019 · 838d9b4 · 838d9b4
1 parent 29a2cdf
commit 838d9b4
Show file tree

Hide file tree

Showing 18 changed files with 154 additions and 136 deletions.
diff --git a/common/logger.cpp b/common/logger.cpp
@@ -91,7 +91,7 @@ void Logger::linkToDbWithOutput(const std::string &dbName, const PriorityChangeN
 
     // Initialize internal DB with observer
     logger.m_settingChangeObservers.insert(std::make_pair(dbName, std::make_pair(prioNotify, outputNotify)));
-    DBConnector db(LOGLEVEL_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
+    DBConnector db("LOGLEVEL_DB", 0);
     RedisClient redisClient(&db);
     auto keys = redisClient.keys("*");
 
@@ -169,7 +169,7 @@ Logger::Priority Logger::getMinPrio()
 [[ noreturn ]] void Logger::settingThread()
 {
     Select select;
-    DBConnector db(LOGLEVEL_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
+    DBConnector db("LOGLEVEL_DB", 0);
     std::vector<std::shared_ptr<ConsumerStateTable>> selectables(m_settingChangeObservers.size());
 
     for (const auto& i : m_settingChangeObservers)

diff --git a/common/loglevel.cpp b/common/loglevel.cpp
@@ -107,7 +107,7 @@ int main(int argc, char **argv)
         }
     }
 
-    DBConnector db(LOGLEVEL_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
+    DBConnector db("LOGLEVEL_DB", 0);
     RedisClient redisClient(&db);
     auto keys = redisClient.keys("*");
     for (auto& key : keys)

diff --git a/common/warm_restart.cpp b/common/warm_restart.cpp
@@ -32,8 +32,7 @@ WarmStart &WarmStart::getInstance(void)
 void WarmStart::initialize(const std::string &app_name,
                            const std::string &docker_name,
                            unsigned int db_timeout,
-                           const std::string &db_hostname,
-                           int db_port)
+                           bool isTcpConn)
 {
     auto& warmStart = getInstance();
 
@@ -43,20 +42,10 @@ void WarmStart::initialize(const std::string &app_name,
     }
 
     /* Use unix socket for db connection by default */
-    if(db_hostname.empty())
-    {
-        warmStart.m_stateDb =
-            std::make_shared<swss::DBConnector>(STATE_DB, swss::DBConnector::DEFAULT_UNIXSOCKET, db_timeout);
-        warmStart.m_cfgDb =
-            std::make_shared<swss::DBConnector>(CONFIG_DB, swss::DBConnector::DEFAULT_UNIXSOCKET, db_timeout);
-    }
-    else
-    {
-        warmStart.m_stateDb =
-            std::make_shared<swss::DBConnector>(STATE_DB, db_hostname, db_port, db_timeout);
-        warmStart.m_cfgDb =
-            std::make_shared<swss::DBConnector>(CONFIG_DB, db_hostname, db_port, db_timeout);
-    }
+    warmStart.m_stateDb =
+        std::make_shared<swss::DBConnector>("STATE_DB", db_timeout, isTcpConn);
+    warmStart.m_cfgDb =
+        std::make_shared<swss::DBConnector>("CONFIG_DB", db_timeout, isTcpConn);
 
     warmStart.m_stateWarmRestartEnableTable =
             std::unique_ptr<Table>(new Table(warmStart.m_stateDb.get(), STATE_WARM_RESTART_ENABLE_TABLE_NAME));

diff --git a/common/warm_restart.h b/common/warm_restart.h
@@ -43,8 +43,7 @@ class WarmStart
     static void initialize(const std::string &app_name,
                            const std::string &docker_name,
                            unsigned int db_timeout = 0,
-                           const std::string &db_hostname = "",
-                           int db_port = 6379);
+                           bool isTcpConn = false);
 
     static bool checkWarmStart(const std::string &app_name,
                                const std::string &docker_name,

diff --git a/tests/Makefile.am b/tests/Makefile.am
@@ -26,7 +26,8 @@ tests_SOURCES = redis_ut.cpp                \
                 redis_subscriber_state_ut.cpp \
                 selectable_priority.cpp       \
                 warm_restart_ut.cpp         \
-                redis_multi_db_ut.cpp
+                redis_multi_db_ut.cpp       \
+                main.cpp
 
 tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS)
 tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS)

diff --git a/tests/json_ut.cpp b/tests/json_ut.cpp
@@ -6,13 +6,12 @@ using namespace std;
 using namespace swss;
 using json = nlohmann::json;
 
-#define TEST_DB         (15) // Default Redis config supports 16 databases, max DB ID is 15
 #define TEST_DUMP_FILE  "ut_dump_file.txt"
 
 TEST(JSON, test)
 {
     /* Construct the file */
-    DBConnector db(TEST_DB, "localhost", 6379, 0);
+    DBConnector db("TEST_DB", 0, true);
     ProducerTable *p;
     p = new ProducerTable(&db, "UT_REDIS", TEST_DUMP_FILE);
     string separator = p->getTableNameSeparator();

diff --git a/tests/main.cpp b/tests/main.cpp
@@ -0,0 +1,46 @@
+#include "gtest/gtest.h"
+#include "common/dbconnector.h"
+#include <iostream>
+
+using namespace std;
+using namespace swss;
+
+string existing_file = "./tests/redis_multi_db_ut_config/database_config.json";
+string nonexisting_file = "./tests/redis_multi_db_ut_config/database_config_nonexisting.json";
+
+class SwsscommonEnvironment : public ::testing::Environment {
+public:
+    // Override this to define how to set up the environment.
+    void SetUp() override {
+        // by default , init should be false
+        cout<<"Default : isInit = "<<SonicDBConfig::isInit()<<endl;
+        EXPECT_FALSE(SonicDBConfig::isInit());
+
+        // load nonexisting file, should throw exception with NO file existing
+        try
+        {
+            cout<<"INIT: loading nonexisting db config file"<<endl;
+            SonicDBConfig::initialize(nonexisting_file);
+        }
+        catch (exception &e)
+        {
+            EXPECT_TRUE(strstr(e.what(), "Sonic database config file doesn't exist"));
+        }
+        EXPECT_FALSE(SonicDBConfig::isInit());
+
+        // load local config file, init should be true
+        SonicDBConfig::initialize(existing_file);
+        cout<<"INIT: load local db config file, isInit = "<<SonicDBConfig::isInit()<<endl;
+        EXPECT_TRUE(SonicDBConfig::isInit());
+    }
+};
+
+int main(int argc, char* argv[])
+{
+    testing::InitGoogleTest(&argc, argv);
+    // Registers a global test environment, and verifies that the
+    // registration function returns its argument.
+    SwsscommonEnvironment* const env = new SwsscommonEnvironment;
+    testing::AddGlobalTestEnvironment(env);
+    return RUN_ALL_TESTS();
+}
diff --git a/tests/ntf_ut.cpp b/tests/ntf_ut.cpp
@@ -58,7 +58,7 @@ TEST(Notifications, test)
 {
     SWSS_LOG_ENTER();
 
-    swss::DBConnector dbNtf(ASIC_DB, "localhost", 6379, 0);
+    swss::DBConnector dbNtf("ASIC_DB", 0, true);
     swss::NotificationConsumer nc(&dbNtf, "NOTIFICATIONS");
     notification_thread = std::make_shared<std::thread>(std::thread(ntf_thread, std::ref(nc)));
 
@@ -81,7 +81,7 @@ TEST(Notifications, pops)
 {
     SWSS_LOG_ENTER();
 
-    swss::DBConnector dbNtf(ASIC_DB, "localhost", 6379, 0);
+    swss::DBConnector dbNtf("ASIC_DB", 0, true);
     swss::NotificationConsumer nc(&dbNtf, "NOTIFICATIONS", 100, (size_t)messages);
     swss::NotificationProducer notifications(&dbNtf, "NOTIFICATIONS");
 
@@ -125,7 +125,7 @@ TEST(Notifications, peek)
 {
     SWSS_LOG_ENTER();
 
-    swss::DBConnector dbNtf(ASIC_DB, "localhost", 6379, 0);
+    swss::DBConnector dbNtf("ASIC_DB", 0, true);
     swss::NotificationConsumer nc(&dbNtf, "NOTIFICATIONS", 100, (size_t)10);
     swss::NotificationProducer notifications(&dbNtf, "NOTIFICATIONS");
 

diff --git a/tests/redis_multi_db_ut.cpp b/tests/redis_multi_db_ut.cpp
@@ -9,39 +9,15 @@ using namespace std;
 using namespace swss;
 using json = nlohmann::json;
 
+extern string existing_file;
 
 TEST(DBConnector, multi_db_test)
 {
-    string file = "./tests/redis_multi_db_ut_config/database_config.json";
-    string nonexisting_file = "./tests/redis_multi_db_ut_config/database_config_nonexisting.json";
-
-    // by default , init should be false
-    cout<<"Default : isInit = "<<SonicDBConfig::isInit()<<endl;
-    EXPECT_FALSE(SonicDBConfig::isInit());
-
-
-    // load nonexisting file, should throw exception with NO file existing
-    try
-    {
-        cout<<"INIT: loading nonexisting db config file"<<endl;
-        SonicDBConfig::initialize(nonexisting_file);
-    }
-    catch (exception &e)
-    {
-        EXPECT_TRUE(strstr(e.what(), "Sonic database config file doesn't exist"));
-    }
-    EXPECT_FALSE(SonicDBConfig::isInit());
-
-    // load local config file, init should be true
-    SonicDBConfig::initialize(file);
-    cout<<"INIT: load local db config file, isInit = "<<SonicDBConfig::isInit()<<endl;
-    EXPECT_TRUE(SonicDBConfig::isInit());
-
     // load local config file again, should throw exception with init already done
     try
     {
         cout<<"INIT: loading local config file again"<<endl;
-        SonicDBConfig::initialize(file);
+        SonicDBConfig::initialize(existing_file);
     }
     catch (exception &e)
     {
@@ -65,7 +41,7 @@ TEST(DBConnector, multi_db_test)
     EXPECT_THROW(SonicDBConfig::getDbPort("INVALID_DBNAME"), out_of_range);
 
     // parse config file
-    ifstream i(file);
+    ifstream i(existing_file);
     if (i.good())
     {
         json j;

diff --git a/tests/redis_multi_db_ut_config/database_config.json b/tests/redis_multi_db_ut_config/database_config.json
@@ -29,39 +29,53 @@
     "DATABASES" : {
         "APPL_DB" : {
             "id" : 0,
+            "separator": ":",
             "instance" : "redis"
         },
         "ASIC_DB" : {
             "id" : 1,
-            "instance" : "redis1"
+            "separator": ":",
+            "instance" : "redis"
         },
         "COUNTERS_DB" : {
             "id" : 2,
-            "instance" : "redis1"
+            "separator": ":",
+            "instance" : "redis"
         },
         "LOGLEVEL_DB" : {
             "id" : 3,
-            "instance" : "redis2"
+            "separator": ":",
+            "instance" : "redis"
         },
         "CONFIG_DB" : {
             "id" : 4,
-            "instance" : "redis2"
+            "separator": "|",
+            "instance" : "redis"
         },
         "PFC_WD_DB" : {
             "id" : 5,
-            "instance" : "redis3"
+            "separator": ":",
+            "instance" : "redis"
         },
         "FLEX_COUNTER_DB" : {
             "id" : 5,
-            "instance" : "redis3"
+            "separator": ":",
+            "instance" : "redis"
         },
         "STATE_DB" : {
             "id" : 6,
-            "instance" : "redis4"
+            "separator": "|",
+            "instance" : "redis"
         },
         "SNMP_OVERLAY_DB" : {
             "id" : 7,
-            "instance" : "redis4"
+            "separator": "|",
+            "instance" : "redis"
+        },
+        "TEST_DB" : {
+            "id" : 15,
+            "separator": ":",
+            "instance" : "redis"
         }
     },
     "VERSION" : "1.0"

diff --git a/tests/redis_piped_state_ut.cpp b/tests/redis_piped_state_ut.cpp
@@ -16,7 +16,7 @@
 using namespace std;
 using namespace swss;
 
-#define TEST_DB           APPL_DB // Need to test against a DB which uses a colon table name separator due to hardcoding in consumer_table_pops.lua
+#define TEST_DB           "APPL_DB" // Need to test against a DB which uses a colon table name separator due to hardcoding in consumer_table_pops.lua
 #define NUMBER_OF_THREADS    (64) // Spawning more than 256 threads causes libc++ to except
 #define NUMBER_OF_OPS      (1000)
 #define MAX_FIELDS_DIV       (30) // Testing up to 30 fields objects
@@ -76,7 +76,7 @@ static inline void validateFields(const string& key, const vector<FieldValueTupl
 static void producerWorker(int index)
 {
     string tableName = "UT_REDIS_THREAD_" + to_string(index);
-    DBConnector db(TEST_DB, "localhost", 6379, 0);
+    DBConnector db(TEST_DB, 0, true);
     RedisPipeline pipeline(&db);
     ProducerStateTable p(&pipeline, tableName, true);
 
@@ -104,7 +104,7 @@ static void producerWorker(int index)
 static void consumerWorker(int index)
 {
     string tableName = "UT_REDIS_THREAD_" + to_string(index);
-    DBConnector db(TEST_DB, "localhost", 6379, 0);
+    DBConnector db(TEST_DB, 0, true);
     ConsumerStateTable c(&db, tableName);
     Select cs;
     Selectable *selectcs;
@@ -139,7 +139,7 @@ static void consumerWorker(int index)
 
 static inline void clearDB()
 {
-    DBConnector db(TEST_DB, "localhost", 6379, 0);
+    DBConnector db(TEST_DB, 0, true);
     RedisReply r(&db, "FLUSHALL", REDIS_REPLY_STATUS);
     r.checkStatusOK();
 }
@@ -151,7 +151,7 @@ TEST(ConsumerStateTable, async_double_set)
     /* Prepare producer */
     int index = 0;
     string tableName = "UT_REDIS_THREAD_" + to_string(index);
-    DBConnector db(TEST_DB, "localhost", 6379, 0);
+    DBConnector db(TEST_DB, 0, true);
     RedisPipeline pipeline(&db);
     ProducerStateTable p(&pipeline, tableName, true);
     string key = "TheKey";
@@ -228,7 +228,7 @@ TEST(ConsumerStateTable, async_set_del)
     /* Prepare producer */
     int index = 0;
     string tableName = "UT_REDIS_THREAD_" + to_string(index);
-    DBConnector db(TEST_DB, "localhost", 6379, 0);
+    DBConnector db(TEST_DB, 0, true);
     RedisPipeline pipeline(&db);
     ProducerStateTable p(&pipeline, tableName, true);
     string key = "TheKey";
@@ -282,7 +282,7 @@ TEST(ConsumerStateTable, async_set_del_set)
     /* Prepare producer */
     int index = 0;
     string tableName = "UT_REDIS_THREAD_" + to_string(index);
-    DBConnector db(TEST_DB, "localhost", 6379, 0);
+    DBConnector db(TEST_DB, 0, true);
     RedisPipeline pipeline(&db);
     ProducerStateTable p(&pipeline, tableName, true);
     string key = "TheKey";
@@ -357,7 +357,7 @@ TEST(ConsumerStateTable, async_singlethread)
 
     int index = 0;
     string tableName = "UT_REDIS_THREAD_" + to_string(index);
-    DBConnector db(TEST_DB, "localhost", 6379, 0);
+    DBConnector db(TEST_DB, 0, true);
     RedisPipeline pipeline(&db);
     ProducerStateTable p(&pipeline, tableName, true);
 
@@ -521,7 +521,7 @@ TEST(ConsumerStateTable, async_test)
 
 TEST(ConsumerStateTable, async_multitable)
 {
-    DBConnector db(TEST_DB, "localhost", 6379, 0);
+    DBConnector db(TEST_DB, 0, true);
     ConsumerStateTable *consumers[NUMBER_OF_THREADS];
     thread *producerThreads[NUMBER_OF_THREADS];
     KeyOpFieldsValuesTuple kco;