update kvs build to avoid reopen opened instance.

- avoid openening same instance if from builder.

Fixes #63

Author: Ahmed-Elsaka-JC

src/cpp/src/kvs.cpp

@@ -17,7 +17,6 @@
 #include "kvs.hpp"
 
 //TODO Default Value Handling TBD
-//TODO Add Score Logging
 //TODO Replace std libs with baselibs
 //TODO String Handling in set_value TBD: (e.g. set_value("key", "value") is not correct, since KvsValue() expects a string ptr and not a string)
 

src/cpp/src/kvsbuilder.cpp

@@ -14,13 +14,31 @@
 
 namespace score::mw::per::kvs {
 
+//Definition of static member variables
+std::unordered_map<size_t, std::shared_ptr<Kvs>> KvsBuilder::kvs_instances;
+KvsBuilder* KvsBuilder::latest_instance = nullptr;
+std::mutex                  KvsBuilder::kvs_instances_mutex;                 ///< Mutex for synchronizing access to KVS instances
+std::mutex                  KvsBuilder::latest_instance_mutex;               ///< Mutex for synchronizing access to KVS Builder
+
 /*********************** KVS Builder Implementation *********************/
 KvsBuilder::KvsBuilder(const InstanceId& instance_id)
     : instance_id(instance_id)
     , need_defaults(false)
     , need_kvs(false)
     , directory("./data_folder/") /* Default Directory */
-{}
+{
+    std::lock_guard<std::mutex> lock(latest_instance_mutex);
+    latest_instance = this;
+}
+
+KvsBuilder::~KvsBuilder() {
+    std::lock(latest_instance_mutex, kvs_instances_mutex);
+    std::lock_guard<std::mutex> latest_lock(latest_instance_mutex, std::adopt_lock);
+    std::lock_guard<std::mutex> kvs_instances_lock(kvs_instances_mutex, std::adopt_lock);
+    if (latest_instance == this) {
+       kvs_instances.clear();
+    }
+}
 
 KvsBuilder& KvsBuilder::need_defaults_flag(bool flag) {
     need_defaults = flag;
@@ -38,22 +56,38 @@ KvsBuilder& KvsBuilder::dir(std::string&& dir_path) {
 }
 
 
-score::Result<Kvs> KvsBuilder::build() {
-    score::Result<Kvs> result = score::MakeUnexpected(ErrorCode::UnmappedError);
+score::Result<std::shared_ptr<Kvs>> KvsBuilder::build() {
 
     /* Use current directory if empty */
-    if ("" == directory) {
+    if (directory.empty()) {
         directory = "./";
     }
 
-    result = Kvs::open(
+    //Lock the mutex before accessing the cache
+    std::lock_guard<std::mutex> lock(kvs_instances_mutex);
+
+    auto it = kvs_instances.find(instance_id.id);
+    if(it != kvs_instances.end()) {
+        /* Return existing instance */
+        return score::Result<std::shared_ptr<Kvs>>(it->second);
+    }
+
+    auto opened_kvs = Kvs::open(
         instance_id,
         need_defaults ? OpenNeedDefaults::Required : OpenNeedDefaults::Optional,
         need_kvs      ? OpenNeedKvs::Required      : OpenNeedKvs::Optional,
         std::move(directory)
     );
 
-    return result;
+    if(opened_kvs.has_value()) {
+        auto kvs_ptr = std::make_shared<Kvs>(std::move(opened_kvs.value()));
+        kvs_instances.insert({instance_id.id, kvs_ptr});
+        return score::Result<std::shared_ptr<Kvs>>(kvs_ptr);
+    }
+    else {
+        return score::Result<std::shared_ptr<Kvs>>(score::Unexpected(opened_kvs.error()));
+    }
+
 }
 
 } /* namespace score::mw::per::kvs */

src/cpp/src/kvsbuilder.hpp

@@ -14,8 +14,9 @@
 #define SCORE_LIB_KVS_KVSBUILDER_HPP
 
 #include <string>
+#include <unordered_map>
+#include <mutex>
 #include "kvs.hpp"
-
 namespace score::mw::per::kvs {
 
 /**
@@ -64,6 +65,11 @@ class KvsBuilder final {
      */
     explicit KvsBuilder(const InstanceId& instance_id);
 
+    /**
+     * @brief Destroys the KvsBuilder instance.
+     */
+    ~KvsBuilder();
+
     /**
      * @brief Specify whether default values must be loaded.
      * @param flag True to require default values; false to make them optional.
@@ -94,13 +100,18 @@ class KvsBuilder final {
      *
      * @return A score::Result<Kvs> containing the opened store or an ErrorCode.
      */
-    score::Result<Kvs> build();
+    score::Result<std::shared_ptr<Kvs>> build();
 
 private:
-    InstanceId                         instance_id;   ///< ID of the KVS instance
-    bool                               need_defaults; ///< Whether default values are required
-    bool                               need_kvs;      ///< Whether an existing KVS is required
-    std::string                        directory;     ///< Directory where to store the KVS Files
+    InstanceId                         instance_id;                         ///< ID of the KVS instance
+    bool                               need_defaults;                       ///< Whether default values are required
+    bool                               need_kvs;                            ///< Whether an existing KVS is required
+    std::string                        directory;                           ///< Directory where to store the KVS Files
+    static std::mutex                  kvs_instances_mutex;                 ///< Mutex for synchronizing access to KVS instances
+    static std::mutex                  latest_instance_mutex;               ///< Mutex for synchronizing access to KVS Builder
+    static std::unordered_map<size_t, std::shared_ptr<Kvs>> kvs_instances;  ///< Map of KVS instances
+    static KvsBuilder* latest_instance;
+    
 };
 
 } /* namespace score::mw::per::kvs */

src/cpp/tests/test_kvs_builder.cpp

@@ -43,27 +43,33 @@ TEST(kvs_kvsbuilder, kvsbuilder_build) {
     builder.need_kvs_flag(false);
     result_build = builder.build();
     EXPECT_TRUE(result_build);
-    result_build.value().flush_on_exit = false;
-    EXPECT_EQ(result_build.value().filename_prefix.CStr(), "./kvsbuilder/kvs_"+std::to_string(instance_id.id));
+    result_build.value()->flush_on_exit = false;
+    EXPECT_EQ(result_build.value()->filename_prefix.CStr(), "./kvsbuilder/kvs_"+std::to_string(instance_id.id));
 }
 
 TEST(kvs_kvsbuilder, kvsbuilder_directory_check) {
 
     /* Test the KvsBuilder with all configurations for the current working directory */
-    KvsBuilder builder(instance_id);
+    InstanceId id(10);
+    KvsBuilder builder(id);
     builder.dir("");
     auto result_build = builder.build();
     EXPECT_TRUE(result_build);
-    EXPECT_EQ(result_build.value().filename_prefix.CStr(), "./kvs_"+std::to_string(instance_id.id));
+    EXPECT_EQ(result_build.value()->filename_prefix.CStr(), "./kvs_"+std::to_string(id.id));
+
 
+    InstanceId id1(20);
     builder.dir("./");
+    builder.instance_id = id1;
     result_build = builder.build();
     EXPECT_TRUE(result_build);
-    EXPECT_EQ(result_build.value().filename_prefix.CStr(), "./kvs_"+std::to_string(instance_id.id));
+    EXPECT_EQ(result_build.value()->filename_prefix.CStr(), "./kvs_"+std::to_string(id1.id));
 
+    InstanceId id2(30);
     builder.dir(".");
+    builder.instance_id = id2;
     result_build = builder.build();
     EXPECT_TRUE(result_build);
-    EXPECT_EQ(result_build.value().filename_prefix.CStr(), "./kvs_"+std::to_string(instance_id.id));
+    EXPECT_EQ(result_build.value()->filename_prefix.CStr(), "./kvs_"+std::to_string(id2.id));
 
 }