From: Piotr Bartosiewicz Date: Thu, 5 Feb 2015 13:28:22 +0000 (+0100) Subject: Add 'commit' method to kv transaction X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d6e9bf90e396fef5f70b673503d72574e89db432;p=archive%2Fplatform%2Fcore%2Fsystem%2FlibConfig.git Add 'commit' method to kv transaction [Bug/Feature] Do no rely on std::uncaught_exception for two reasons: - there is a bug in gcc with rethrow_exception - it wasn't possible to update db during stack unwinding. [Cause] N/A [Solution] N/A [Verification] Build, install, run tests Change-Id: I8e12688efdebb3f2865e6a4512ed331756c69537 --- diff --git a/src/config/from-kvjson-visitor.hpp b/src/config/from-kvjson-visitor.hpp index 2624ad3..6b2f6a6 100644 --- a/src/config/from-kvjson-visitor.hpp +++ b/src/config/from-kvjson-visitor.hpp @@ -34,11 +34,10 @@ namespace config { class FromKVJsonVisitor { public: - FromKVJsonVisitor(const std::string& db, const std::string& json, const std::string& prefix) - : mStorePtr(new KVStore(db)) + FromKVJsonVisitor(KVStore& store, const std::string& json, const std::string& prefix) + : mStore(store) , mKeyPrefix(prefix) , mIsUnion(false) - , mTransaction(mStorePtr->getTransaction()) { mObject = json_tokener_parse(json.c_str()); if (mObject == nullptr) { @@ -53,10 +52,9 @@ public: FromKVJsonVisitor(const FromKVJsonVisitor& v) : mObject(v.mObject ? json_object_get(v.mObject) : nullptr), - mStorePtr(v.mStorePtr), + mStore(v.mStore), mKeyPrefix(v.mKeyPrefix), - mIsUnion(v.mIsUnion), - mTransaction(v.mTransaction) + mIsUnion(v.mIsUnion) { } FromKVJsonVisitor& operator=(const FromKVJsonVisitor&) = delete; @@ -67,18 +65,16 @@ public: } private: - std::shared_ptr mStorePtr; + KVStore& mStore; std::string mKeyPrefix; json_object* mObject; bool mIsUnion; - KVStore::Transaction mTransaction; // create visitor for field object (visitable object) FromKVJsonVisitor(const FromKVJsonVisitor& v, const std::string& name, const bool isUnion) : - mStorePtr(v.mStorePtr), + mStore(v.mStore), mKeyPrefix(key(v.mKeyPrefix, name)), - mIsUnion(isUnion || v.mIsUnion), - mTransaction(v.mTransaction) + mIsUnion(isUnion || v.mIsUnion) { json_object* object = nullptr; if (v.mObject && !json_object_object_get_ex(v.mObject, name.c_str(), &object)) { @@ -90,10 +86,9 @@ private: // create visitor for vector i-th element (visitable object) FromKVJsonVisitor(const FromKVJsonVisitor& v, int i, const bool isUnion) : - mStorePtr(v.mStorePtr), + mStore(v.mStore), mKeyPrefix(key(v.mKeyPrefix, std::to_string(i))), - mIsUnion(isUnion || v.mIsUnion), - mTransaction(v.mTransaction) + mIsUnion(isUnion || v.mIsUnion) { json_object* object = nullptr; if (v.mObject) { @@ -107,8 +102,8 @@ private: void getValue(const std::string& name, T& t) { std::string k = key(mKeyPrefix, name); - if (mStorePtr->exists(k)) { - t = mStorePtr->get(k); + if (mStore.exists(k)) { + t = mStore.get(k); } else { json_object* object = nullptr; @@ -138,8 +133,8 @@ private: int getArraySize(std::string& name, json_object* object) { - if (mStorePtr->exists(name)) { - return mStorePtr->get(name); + if (mStore.exists(name)) { + return mStore.get(name); } if (object) { return json_object_array_length(object); @@ -162,7 +157,7 @@ private: } value.resize(static_cast(length)); FromKVJsonVisitor visitor(*this, name, false); - if (mStorePtr->exists(k)) { + if (mStore.exists(k)) { json_object_put(visitor.mObject); visitor.mObject = nullptr; } @@ -175,8 +170,8 @@ private: void getValue(int i, T& t) { std::string k = key(mKeyPrefix, std::to_string(i)); - if (mStorePtr->exists(k)) { - t = mStorePtr->get(k); + if (mStore.exists(k)) { + t = mStore.get(k); } else { json_object* object = mObject ? json_object_array_get_idx(mObject, i) : nullptr; @@ -208,7 +203,7 @@ private: int length = getArraySize(k, mObject); value.resize(static_cast(length)); FromKVJsonVisitor visitor(*this, i, false); - if (mStorePtr->exists(k)) { + if (mStore.exists(k)) { json_object_put(visitor.mObject); visitor.mObject = nullptr; } diff --git a/src/config/from-kvstore-visitor.hpp b/src/config/from-kvstore-visitor.hpp index 3981bfd..8279c97 100644 --- a/src/config/from-kvstore-visitor.hpp +++ b/src/config/from-kvstore-visitor.hpp @@ -26,20 +26,15 @@ #define CONFIG_FROM_KVSTORE_VISITOR_HPP #include "config/is-visitable.hpp" -#include "config/exception.hpp" #include "config/kvstore.hpp" -#include -#include - namespace config { class FromKVStoreVisitor { public: - explicit FromKVStoreVisitor(const std::string& filePath, const std::string& prefix) - : mStorePtr(new KVStore(filePath)), - mKeyPrefix(prefix), - mTransaction(mStorePtr->getTransaction()) + FromKVStoreVisitor(KVStore& store, const std::string& prefix) + : mStore(store), + mKeyPrefix(prefix) { } @@ -52,21 +47,19 @@ public: } private: - std::shared_ptr mStorePtr; + KVStore& mStore; std::string mKeyPrefix; - KVStore::Transaction mTransaction; FromKVStoreVisitor(const FromKVStoreVisitor& visitor, const std::string& prefix) - : mStorePtr(visitor.mStorePtr), - mKeyPrefix(prefix), - mTransaction(visitor.mTransaction) + : mStore(visitor.mStore), + mKeyPrefix(prefix) { } template::value, int>::type = 0> void getInternal(const std::string& name, T& value) { - value = mStorePtr->get(name); + value = mStore.get(name); } template::value, int>::type = 0> @@ -81,7 +74,7 @@ private: { values.clear(); - size_t vectorSize = mStorePtr->get(name); + size_t vectorSize = mStore.get(name); if (vectorSize == 0) { return; } diff --git a/src/config/kvstore.cpp b/src/config/kvstore.cpp index 476d182..49656ae 100644 --- a/src/config/kvstore.cpp +++ b/src/config/kvstore.cpp @@ -96,47 +96,47 @@ void sqliteEscapeStr(::sqlite3_context* context, int argc, ::sqlite3_value** val } // namespace -struct KVStore::TransactionImpl { - TransactionImpl(KVStore& kvStore) - : mLock(kvStore.mMutex) - , mKVStore(kvStore) - { +KVStore::Transaction::Transaction(KVStore& kvStore) + : mLock(kvStore.mMutex) + , mKVStore(kvStore) + , mIsOuter(kvStore.mTransactionDepth == 0) +{ + if (mKVStore.mIsTransactionCommited) { + throw ConfigException("Previous transaction is not closed"); + } + if (mIsOuter) { mKVStore.mConn.exec("BEGIN EXCLUSIVE TRANSACTION"); } + ++mKVStore.mTransactionDepth; +} - ~TransactionImpl() - { - // be aware of a bug in gcc with uncaught_exception and rethrow_exception - if (std::uncaught_exception()) { - try { - mKVStore.mConn.exec("ROLLBACK TRANSACTION"); - } catch (ConfigException&) {} +KVStore::Transaction::~Transaction() +{ + --mKVStore.mTransactionDepth; + if (mIsOuter) { + if (mKVStore.mIsTransactionCommited) { + mKVStore.mIsTransactionCommited = false; } else { - try { - mKVStore.mConn.exec("COMMIT TRANSACTION"); - } catch (ConfigException&) {} + mKVStore.mConn.exec("ROLLBACK TRANSACTION"); } } +} -private: - config::KVStore::Lock mLock; - config::KVStore& mKVStore; -}; - -KVStore::Transaction KVStore::getTransaction() +void KVStore::Transaction::commit() { - Lock lock(mMutex); - - auto t = mTransactionImplPtr.lock(); - if (!t) { - t = std::make_shared(*this); - mTransactionImplPtr = t; + if (mKVStore.mIsTransactionCommited) { + throw ConfigException("Transaction already commited"); + } + if (mIsOuter) { + mKVStore.mConn.exec("COMMIT TRANSACTION"); + mKVStore.mIsTransactionCommited = true; } - return t; } KVStore::KVStore(const std::string& path) - : mPath(path), + : mTransactionDepth(0), + mIsTransactionCommited(false), + mPath(path), mConn(path) { setupDb(); @@ -146,7 +146,7 @@ KVStore::KVStore(const std::string& path) KVStore::~KVStore() { - assert(!mTransactionImplPtr.lock()); + assert(mTransactionDepth == 0); } void KVStore::setupDb() @@ -183,45 +183,44 @@ void KVStore::createFunctions() void KVStore::clear() { - Transaction transaction = getTransaction(); + Transaction transaction(*this); mConn.exec("DELETE FROM data"); + transaction.commit(); } bool KVStore::isEmpty() { - Transaction transaction = getTransaction(); + Transaction transaction(*this); ScopedReset scopedReset(mGetIsEmptyStmt); int ret = ::sqlite3_step(mGetIsEmptyStmt->get()); - if (ret == SQLITE_DONE) { - return true; - } else if (ret == SQLITE_ROW) { - return false; - } else { + if (ret != SQLITE_DONE && ret != SQLITE_ROW) { throw ConfigException("Error during stepping: " + mConn.getErrorMessage()); } + + transaction.commit(); + return ret == SQLITE_DONE; } bool KVStore::exists(const std::string& key) { - Transaction transaction = getTransaction(); + Transaction transaction(*this); ScopedReset scopedReset(mGetKeyExistsStmt); ::sqlite3_bind_text(mGetKeyExistsStmt->get(), 1, key.c_str(), AUTO_DETERM_SIZE, SQLITE_TRANSIENT); int ret = ::sqlite3_step(mGetKeyExistsStmt->get()); - if (ret == SQLITE_DONE) { - return false; - } else if (ret == SQLITE_ROW) { - return true; - } else { + if (ret != SQLITE_DONE && ret != SQLITE_ROW) { throw ConfigException("Error during stepping: " + mConn.getErrorMessage()); } + + transaction.commit(); + return ret == SQLITE_ROW; } void KVStore::remove(const std::string& key) { - Transaction transaction = getTransaction(); + Transaction transaction(*this); ScopedReset scopedReset(mRemoveValuesStmt); ::sqlite3_bind_text(mRemoveValuesStmt->get(), 1, key.c_str(), AUTO_DETERM_SIZE, SQLITE_STATIC); @@ -229,11 +228,12 @@ void KVStore::remove(const std::string& key) if (::sqlite3_step(mRemoveValuesStmt->get()) != SQLITE_DONE) { throw ConfigException("Error during stepping: " + mConn.getErrorMessage()); } + transaction.commit(); } void KVStore::setInternal(const std::string& key, const std::string& value) { - Transaction transaction = getTransaction(); + Transaction transaction(*this); ScopedReset scopedReset(mSetValueStmt); ::sqlite3_bind_text(mSetValueStmt->get(), 1, key.c_str(), AUTO_DETERM_SIZE, SQLITE_STATIC); @@ -243,6 +243,7 @@ void KVStore::setInternal(const std::string& key, const std::string& value) if (::sqlite3_step(mSetValueStmt->get()) != SQLITE_DONE) { throw ConfigException("Error during stepping: " + mConn.getErrorMessage()); } + transaction.commit(); } void KVStore::setInternal(const std::string& key, const std::initializer_list& values) @@ -256,7 +257,7 @@ void KVStore::setInternal(const std::string& key, const std::vector throw ConfigException("Too many values to insert"); } - Transaction transaction = getTransaction(); + Transaction transaction(*this); remove(key); @@ -268,11 +269,12 @@ void KVStore::setInternal(const std::string& key, const std::vector setInternal(config::key(key, std::to_string(i)), values[i]); } + transaction.commit(); } std::string KVStore::getInternal(const std::string& key, std::string*) { - Transaction transaction = getTransaction(); + Transaction transaction(*this); ScopedReset scopedReset(mGetValueStmt); ::sqlite3_bind_text(mGetValueStmt->get(), 1, key.c_str(), AUTO_DETERM_SIZE, SQLITE_TRANSIENT); @@ -285,16 +287,21 @@ std::string KVStore::getInternal(const std::string& key, std::string*) throw ConfigException("Error during stepping: " + mConn.getErrorMessage()); } - return reinterpret_cast(sqlite3_column_text(mGetValueStmt->get(), FIRST_COLUMN)); + std::string value = reinterpret_cast( + sqlite3_column_text(mGetValueStmt->get(), FIRST_COLUMN)); + + transaction.commit(); + return value; } std::vector KVStore::getInternal(const std::string& key, std::vector*) { - Transaction transaction = getTransaction(); + Transaction transaction(*this); unsigned int valuesSize = get(key); std::vector values(valuesSize); if (valuesSize == 0) { + transaction.commit(); return values; } @@ -304,12 +311,13 @@ std::vector KVStore::getInternal(const std::string& key, std::vecto } + transaction.commit(); return values; } std::vector KVStore::getKeys() { - Transaction transaction = getTransaction(); + Transaction transaction(*this); ScopedReset scopedReset(mGetKeysStmt); std::vector result; @@ -317,7 +325,7 @@ std::vector KVStore::getKeys() for (;;) { int ret = ::sqlite3_step(mGetKeysStmt->get()); if (ret == SQLITE_DONE) { - return result; + break; } if (ret != SQLITE_ROW) { throw ConfigException("Error during stepping: " + mConn.getErrorMessage()); @@ -326,6 +334,9 @@ std::vector KVStore::getKeys() FIRST_COLUMN)); result.push_back(key); } + + transaction.commit(); + return result; } } // namespace config diff --git a/src/config/kvstore.hpp b/src/config/kvstore.hpp index 1b24b36..0717665 100644 --- a/src/config/kvstore.hpp +++ b/src/config/kvstore.hpp @@ -45,7 +45,20 @@ public: /** * A guard struct for thread synchronization and transaction management. */ - typedef std::shared_ptr Transaction; + class Transaction { + public: + Transaction(KVStore& store); + ~Transaction(); + + Transaction(const Transaction&) = delete; + Transaction& operator=(const Transaction&) = delete; + + void commit(); + private: + std::unique_lock mLock; + KVStore& mKVStore; + bool mIsOuter; + }; /** * @param path configuration database file path @@ -113,14 +126,12 @@ public: */ std::vector getKeys(); - KVStore::Transaction getTransaction(); - private: typedef std::lock_guard Lock; - struct TransactionImpl; - std::weak_ptr mTransactionImplPtr; std::recursive_mutex mMutex; + size_t mTransactionDepth; + bool mIsTransactionCommited; void setInternal(const std::string& key, const std::string& value); void setInternal(const std::string& key, const std::initializer_list& values); diff --git a/src/config/manager.hpp b/src/config/manager.hpp index 7c50087..7afde48 100644 --- a/src/config/manager.hpp +++ b/src/config/manager.hpp @@ -114,8 +114,11 @@ void loadFromKVStore(const std::string& filename, Config& config, const std::str { static_assert(isVisitable::value, "Use CONFIG_REGISTER macro"); - FromKVStoreVisitor visitor(filename, configName); + KVStore store(filename); + KVStore::Transaction transaction(store); + FromKVStoreVisitor visitor(store, configName); config.accept(visitor); + transaction.commit(); } /** @@ -130,8 +133,11 @@ void saveToKVStore(const std::string& filename, const Config& config, const std: { static_assert(isVisitable::value, "Use CONFIG_REGISTER macro"); - ToKVStoreVisitor visitor(filename, configName); + KVStore store(filename); + KVStore::Transaction transaction(store); + ToKVStoreVisitor visitor(store, configName); config.accept(visitor); + transaction.commit(); } /** @@ -150,8 +156,11 @@ void loadFromKVStoreWithJson(const std::string& kvfile, { static_assert(isVisitable::value, "Use CONFIG_REGISTER macro"); - FromKVJsonVisitor visitor(kvfile, json, kvConfigName); + KVStore store(kvfile); + KVStore::Transaction transaction(store); + FromKVJsonVisitor visitor(store, json, kvConfigName); config.accept(visitor); + transaction.commit(); } /** diff --git a/src/config/to-kvstore-visitor.hpp b/src/config/to-kvstore-visitor.hpp index 071dece..5821a66 100644 --- a/src/config/to-kvstore-visitor.hpp +++ b/src/config/to-kvstore-visitor.hpp @@ -28,18 +28,14 @@ #include "config/is-visitable.hpp" #include "config/kvstore.hpp" -#include -#include - namespace config { class ToKVStoreVisitor { public: - ToKVStoreVisitor(const std::string& filePath, const std::string& prefix) - : mStorePtr(new KVStore(filePath)), - mKeyPrefix(prefix), - mTransaction(mStorePtr->getTransaction()) + ToKVStoreVisitor(KVStore& store, const std::string& prefix) + : mStore(store), + mKeyPrefix(prefix) { } @@ -52,21 +48,19 @@ public: } private: - std::shared_ptr mStorePtr; + KVStore& mStore; std::string mKeyPrefix; - KVStore::Transaction mTransaction; ToKVStoreVisitor(const ToKVStoreVisitor& visitor, const std::string& prefix) - : mStorePtr(visitor.mStorePtr), - mKeyPrefix(prefix), - mTransaction(visitor.mTransaction) + : mStore(visitor.mStore), + mKeyPrefix(prefix) { } template::value, int>::type = 0> void setInternal(const std::string& name, const T& value) { - mStorePtr->set(name, value); + mStore.set(name, value); } template::value, int>::type = 0> @@ -79,8 +73,8 @@ private: template::value, int>::type = 0> void setInternal(const std::string& name, const std::vector& values) { - mStorePtr->remove(name); - mStorePtr->set(name, values.size()); + mStore.remove(name); + mStore.set(name, values.size()); for (size_t i = 0; i < values.size(); ++i) { setInternal(key(name, std::to_string(i)), values[i]); }