From 4b7005976ac880e883d18a394846f53034a5b976 Mon Sep 17 00:00:00 2001 From: Jan Olszak Date: Fri, 31 Oct 2014 16:54:25 +0100 Subject: [PATCH 01/16] Macro for creating empty Config structures [Bug/Feature] N/A [Cause] N/A [Solution] N/A [Verification] Build, install, run tests Change-Id: I1e2c468f2d05855b71938b20e7b63bb5aa112d54 --- src/config/fields.hpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/config/fields.hpp b/src/config/fields.hpp index 3269856..d5443b7 100644 --- a/src/config/fields.hpp +++ b/src/config/fields.hpp @@ -55,6 +55,15 @@ * ) * }; */ + +#define CONFIG_REGISTER_EMPTY \ + template \ + void accept(Visitor ) { \ + } \ + template \ + void accept(Visitor ) const { \ + } \ + #define CONFIG_REGISTER(...) \ template \ void accept(Visitor v) { \ -- 2.7.4 From d555a457fcf245abf8972e5f9ab3317e811ff58f Mon Sep 17 00:00:00 2001 From: Jan Olszak Date: Wed, 19 Nov 2014 10:28:41 +0100 Subject: [PATCH 02/16] Timeout in read and write to a FDStore [Bug/Feature] N/A [Cause] N/A [Solution] N/A [Verification] Build, install, run tests Change-Id: I42a358ce87d1dd6e02030a0e0e2431838ddae29c --- src/config/fdstore.cpp | 112 +++++++++++++++++++++++++++++++++++++------------ src/config/fdstore.hpp | 6 ++- 2 files changed, 89 insertions(+), 29 deletions(-) diff --git a/src/config/fdstore.cpp b/src/config/fdstore.cpp index 050e99f..7934893 100644 --- a/src/config/fdstore.cpp +++ b/src/config/fdstore.cpp @@ -30,9 +30,52 @@ #include #include #include +#include +#include namespace config { +namespace { + +void waitForEvent(int fd, + short event, + const std::chrono::high_resolution_clock::time_point deadline) +{ + // Wait for the rest of the data + struct pollfd fds[1]; + fds[0].fd = fd; + fds[0].events = event | POLLHUP; + + for (;;) { + std::chrono::milliseconds timeoutMS = + std::chrono::duration_cast(deadline - std::chrono::high_resolution_clock::now()); + if (timeoutMS.count() < 0) { + throw ConfigException("Timeout"); + } + + int ret = ::poll(fds, 1 /*fds size*/, timeoutMS.count()); + + if (ret == -1) { + if (errno == EINTR) { + continue; + } + throw ConfigException("Error in poll: " + std::string(strerror(errno))); + } + + if (ret == 0) { + throw ConfigException("Timeout"); + } + + if (fds[0].revents & POLLHUP) { + throw ConfigException("Peer disconnected"); + } + + // Here Comes the Sun + break; + } +} + +} // namespace FDStore::FDStore(int fd) : mFD(fd) @@ -48,42 +91,57 @@ FDStore::~FDStore() { } -void FDStore::write(const void* bufferPtr, const size_t size) +void FDStore::write(const void* bufferPtr, const size_t size, const unsigned int timeoutMS) { + std::chrono::high_resolution_clock::time_point deadline = std::chrono::high_resolution_clock::now() + + std::chrono::milliseconds(timeoutMS); + size_t nTotal = 0; - int n; + for (;;) { + int n = ::write(mFD, + reinterpret_cast(bufferPtr) + nTotal, + size - nTotal); + if (n > 0) { + nTotal += n; + } else if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) { + // Neglected errors + } else { + throw ConfigException("Error during reading: " + std::string(strerror(errno))); + } - do { - n = ::write(mFD, - reinterpret_cast(bufferPtr) + nTotal, - size - nTotal); - if (n < 0) { - if (errno == EINTR) { - continue; - } - throw ConfigException("Error during witting: " + std::string(strerror(errno))); + if (nTotal >= size) { + // All data is written, break loop + break; + } else { + waitForEvent(mFD, POLLOUT, deadline); } - nTotal += n; - } while (nTotal < size); + } } -void FDStore::read(void* bufferPtr, const size_t size) +void FDStore::read(void* bufferPtr, const size_t size, const unsigned int timeoutMS) { - size_t nTotal = 0; - int n; + std::chrono::high_resolution_clock::time_point deadline = std::chrono::high_resolution_clock::now() + + std::chrono::milliseconds(timeoutMS); - do { - n = ::read(mFD, - reinterpret_cast(bufferPtr) + nTotal, - size - nTotal); - if (n < 0) { - if (errno == EINTR) { - continue; - } + size_t nTotal = 0; + for (;;) { + int n = ::read(mFD, + reinterpret_cast(bufferPtr) + nTotal, + size - nTotal); + if (n > 0) { + nTotal += n; + } else if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) { + // Neglected errors + } else { throw ConfigException("Error during reading: " + std::string(strerror(errno))); } - nTotal += n; - } while (nTotal < size); -} + if (nTotal >= size) { + // All data is read, break loop + break; + } else { + waitForEvent(mFD, POLLIN, deadline); + } + } +} } // namespace config diff --git a/src/config/fdstore.hpp b/src/config/fdstore.hpp index d99971d..d34ea14 100644 --- a/src/config/fdstore.hpp +++ b/src/config/fdstore.hpp @@ -48,16 +48,18 @@ public: * * @param bufferPtr buffer with the data * @param size size of the buffer + * @param timeoutMS timeout in milliseconds */ - void write(const void* bufferPtr, const size_t size); + void write(const void* bufferPtr, const size_t size, const unsigned int timeoutMS = 500); /** * Reads a value of the given type. * * @param bufferPtr buffer with the data * @param size size of the buffer + * @param timeoutMS timeout in milliseconds */ - void read(void* bufferPtr, const size_t size); + void read(void* bufferPtr, const size_t size, const unsigned int timeoutMS = 500); private: int mFD; -- 2.7.4 From bb674f609e25e5f7a216f9c30676c4381c2f99c9 Mon Sep 17 00:00:00 2001 From: Mateusz Malicki Date: Wed, 26 Nov 2014 15:11:51 +0100 Subject: [PATCH 03/16] Added possibility to declare union in config file [Bug/Feature] Possibility to declare something like union in config file [Cause] Necessity to create vector with element of more than one type [Solution] Added macro CONFIG_DECLARE_OPTIONS [Verification] Build, install, run scs tests Change-Id: I094eb63f5cda6435836b3d76674d3cad5f89fe14 --- src/config/fields-union.hpp | 158 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 src/config/fields-union.hpp diff --git a/src/config/fields-union.hpp b/src/config/fields-union.hpp new file mode 100644 index 0000000..8f4d997 --- /dev/null +++ b/src/config/fields-union.hpp @@ -0,0 +1,158 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Mateusz Malicki (m.malicki2@samsung.com) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +/** + * @file + * @author Mateusz Malicki (m.malicki2@samsung.com) + * @brief Macros for declare configuration fields + */ + +#ifndef CONFIG_FIELDS_UNION_HPP +#define CONFIG_FIELDS_UNION_HPP + +#include "config/fields.hpp" +#include +#include + +/** + * Use this macro to declare and register config fields + * + * Example: + * struct Foo { + * std::string bar; + * + * CONFIG_REGISTER + * ( + * bar, + * ) + * }; + * + * struct Config + * { + * CONFIG_DECLARE_UNION + * ( + * Foo, + * int + * ) + * }; + * + * Example of valid configuration: + * 1. { + * "type": "Foo", + * "value": { "bar": "some string" } + * } + * 2. { + * "type": "int", + * "value": 1 + * } + * + * + * Usage: + * Config config; + * if (config.is()) { + * Foo& foo = config.as(); + * // ... + * } + * if (config.is())) { + * int field = config.as(); + * // ... + * } + */ + + +#define CONFIG_DECLARE_UNION(...) \ + struct TypeWrapperBase \ + { \ + virtual ~TypeWrapperBase() {} \ + }; \ + \ + template \ + struct TypeWrapper : TypeWrapperBase \ + { \ + Class value; \ + ~TypeWrapper() {} \ + }; \ + \ + std::unique_ptr mConfigDeclareField__; \ + \ + template \ + void accept(Visitor v) { \ + std::string name; \ + v.visit("type", name); \ + visitOption(v, name); \ + } \ + \ + template \ + void accept(Visitor v) const { \ + const std::string name = getOptionName(); \ + if (!name.empty()) { \ + v.visit("type", name); \ + visitOption(v, name); \ + } else { \ + /* Unsupported type in config file */ \ + } \ + } \ + \ + template \ + void visitOption(Visitor& v, const std::string& name) { \ + GENERATE_CODE(GENERATE_UNION_VISIT__, __VA_ARGS__) \ + } \ + template \ + void visitOption(Visitor& v, const std::string& name) const { \ + GENERATE_CODE(GENERATE_UNION_VISIT_CONST__, __VA_ARGS__) \ + } \ + std::string getOptionName() const { \ + GENERATE_CODE(GENERATE_UNION_NAME__, __VA_ARGS__) \ + return std::string(); \ + } \ + \ + template \ + bool is() const { \ + return dynamic_cast*>(mConfigDeclareField__.get()) != NULL; \ + } \ + template \ + Type& as() { \ + assert(mConfigDeclareField__.get()); \ + return dynamic_cast&>(*mConfigDeclareField__.get()).value; \ + } \ + template \ + const Type& as() const { \ + assert(mConfigDeclareField__.get()); \ + return dynamic_cast&>(*mConfigDeclareField__.get()).value; \ + } + +#define GENERATE_CODE(MACRO, ...) \ + BOOST_PP_LIST_FOR_EACH(MACRO, _, BOOST_PP_VARIADIC_TO_LIST(__VA_ARGS__)) + +#define GENERATE_UNION_VISIT__(r, _, TYPE_) \ + if (#TYPE_ == name) { \ + mConfigDeclareField__.reset(new TypeWrapper()); \ + v.visit("value", as()); \ + } + +#define GENERATE_UNION_VISIT_CONST__(r, _, TYPE_) \ + if (#TYPE_ == name) { \ + v.visit("value", as()); \ + } + +#define GENERATE_UNION_NAME__(r, _, TYPE_) \ + if (is()) { \ + return #TYPE_; \ + } + +#endif /* CONFIG_FIELDS_UNION_HPP */ -- 2.7.4 From 24abd8462353da9c7f0865fdfc268713c7402fd0 Mon Sep 17 00:00:00 2001 From: Mateusz Malicki Date: Tue, 2 Dec 2014 15:01:32 +0100 Subject: [PATCH 04/16] Ability to copy union and to set union type [Feature] Ability to copy union and to set union type [Cause] Need to copy and add new union elements [Solution] Use of boost::any class; add isSet and set methods [Verification] Build, install, run scs tests Change-Id: I61e3211c27e62af93b36a5309e50ebaf90ab70ad --- src/config/fields-union.hpp | 91 ++++++++++++++++++++++++--------------------- 1 file changed, 49 insertions(+), 42 deletions(-) diff --git a/src/config/fields-union.hpp b/src/config/fields-union.hpp index 8f4d997..57e54a0 100644 --- a/src/config/fields-union.hpp +++ b/src/config/fields-union.hpp @@ -26,7 +26,8 @@ #define CONFIG_FIELDS_UNION_HPP #include "config/fields.hpp" -#include +#include +#include #include /** @@ -64,31 +65,43 @@ * * Usage: * Config config; - * if (config.is()) { - * Foo& foo = config.as(); - * // ... - * } - * if (config.is())) { - * int field = config.as(); - * // ... + * // ... + * if (config.isSet()) { + * if (config.is()) { + * Foo& foo = config.as(); + * // ... + * } + * if (config.is())) { + * int field = config.as(); + * // ... + * } + * } else { + * // Config is not set + * Foo foo({"some string"}); + * config.set(foo); + * config.set(std::move(foo)); + * config.set(Foo({"some string"})); * } */ #define CONFIG_DECLARE_UNION(...) \ - struct TypeWrapperBase \ - { \ - virtual ~TypeWrapperBase() {} \ - }; \ +private: \ + boost::any mConfigDeclareField__; \ \ - template \ - struct TypeWrapper : TypeWrapperBase \ - { \ - Class value; \ - ~TypeWrapper() {} \ - }; \ - \ - std::unique_ptr mConfigDeclareField__; \ + template \ + void visitOption(Visitor& v, const std::string& name) { \ + GENERATE_CODE(GENERATE_UNION_VISIT__, __VA_ARGS__) \ + } \ + template \ + void visitOption(Visitor& v, const std::string& name) const { \ + GENERATE_CODE(GENERATE_UNION_VISIT_CONST__, __VA_ARGS__) \ + } \ + std::string getOptionName() const { \ + GENERATE_CODE(GENERATE_UNION_NAME__, __VA_ARGS__) \ + return std::string(); \ + } \ +public: \ \ template \ void accept(Visitor v) { \ @@ -104,45 +117,39 @@ v.visit("type", name); \ visitOption(v, name); \ } else { \ - /* Unsupported type in config file */ \ + /* Unsupported type */ \ } \ } \ \ - template \ - void visitOption(Visitor& v, const std::string& name) { \ - GENERATE_CODE(GENERATE_UNION_VISIT__, __VA_ARGS__) \ - } \ - template \ - void visitOption(Visitor& v, const std::string& name) const { \ - GENERATE_CODE(GENERATE_UNION_VISIT_CONST__, __VA_ARGS__) \ - } \ - std::string getOptionName() const { \ - GENERATE_CODE(GENERATE_UNION_NAME__, __VA_ARGS__) \ - return std::string(); \ - } \ - \ template \ bool is() const { \ - return dynamic_cast*>(mConfigDeclareField__.get()) != NULL; \ + return boost::any_cast(&mConfigDeclareField__) != NULL; \ } \ template \ Type& as() { \ - assert(mConfigDeclareField__.get()); \ - return dynamic_cast&>(*mConfigDeclareField__.get()).value; \ + assert(!mConfigDeclareField__.empty()); \ + return boost::any_cast(mConfigDeclareField__); \ } \ template \ const Type& as() const { \ - assert(mConfigDeclareField__.get()); \ - return dynamic_cast&>(*mConfigDeclareField__.get()).value; \ - } + assert(!mConfigDeclareField__.empty()); \ + return boost::any_cast(mConfigDeclareField__); \ + } \ + bool isSet() { \ + return !getOptionName().empty(); \ + } \ + template \ + Type& set(Type&& src) { \ + mConfigDeclareField__ = std::forward(src); \ + return as(); \ + } \ #define GENERATE_CODE(MACRO, ...) \ BOOST_PP_LIST_FOR_EACH(MACRO, _, BOOST_PP_VARIADIC_TO_LIST(__VA_ARGS__)) #define GENERATE_UNION_VISIT__(r, _, TYPE_) \ if (#TYPE_ == name) { \ - mConfigDeclareField__.reset(new TypeWrapper()); \ - v.visit("value", as()); \ + v.visit("value", set(std::move(TYPE_()))); \ } #define GENERATE_UNION_VISIT_CONST__(r, _, TYPE_) \ -- 2.7.4 From 715efc8dc52494fd06faca5dcddb43817c08a4d2 Mon Sep 17 00:00:00 2001 From: Mateusz Malicki Date: Wed, 10 Dec 2014 18:07:09 +0100 Subject: [PATCH 05/16] Disable rvalue references [Bug] Boost any rvalue support is optional [Cause] rvalues is provided in 1.54 and can be disabled [Solution] Disable rvalue references [Verification] N/A Change-Id: I395adec94bbd07e33aa8bf703c0732ffa3b69ea6 --- src/config/fields-union.hpp | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/src/config/fields-union.hpp b/src/config/fields-union.hpp index 57e54a0..53620ca 100644 --- a/src/config/fields-union.hpp +++ b/src/config/fields-union.hpp @@ -79,15 +79,27 @@ * // Config is not set * Foo foo({"some string"}); * config.set(foo); - * config.set(std::move(foo)); + * config.set(std::move(foo)); //< copy sic! * config.set(Foo({"some string"})); * } */ +class DisbaleMoveAnyWrapper : public boost::any +{ + public: + DisbaleMoveAnyWrapper() {} + DisbaleMoveAnyWrapper(const DisbaleMoveAnyWrapper& any) + : boost::any(static_cast(any)) {}; + DisbaleMoveAnyWrapper& operator=(DisbaleMoveAnyWrapper&& any) = delete; + DisbaleMoveAnyWrapper& operator=(const DisbaleMoveAnyWrapper& any) { + static_cast(*this) = static_cast(any); + return *this; + } +}; #define CONFIG_DECLARE_UNION(...) \ private: \ - boost::any mConfigDeclareField__; \ + DisbaleMoveAnyWrapper mConfigDeclareField; \ \ template \ void visitOption(Visitor& v, const std::string& name) { \ @@ -101,6 +113,12 @@ private: GENERATE_CODE(GENERATE_UNION_NAME__, __VA_ARGS__) \ return std::string(); \ } \ + boost::any& getHolder() { \ + return static_cast(mConfigDeclareField); \ + } \ + const boost::any& getHolder() const { \ + return static_cast(mConfigDeclareField); \ + } \ public: \ \ template \ @@ -123,24 +141,24 @@ public: \ template \ bool is() const { \ - return boost::any_cast(&mConfigDeclareField__) != NULL; \ + return boost::any_cast(&getHolder()) != NULL; \ } \ template \ - Type& as() { \ - assert(!mConfigDeclareField__.empty()); \ - return boost::any_cast(mConfigDeclareField__); \ + typename std::enable_if::value, Type>::type& as() { \ + assert(!getHolder().empty()); \ + return boost::any_cast(getHolder()); \ } \ template \ const Type& as() const { \ - assert(!mConfigDeclareField__.empty()); \ - return boost::any_cast(mConfigDeclareField__); \ + assert(!getHolder().empty()); \ + return boost::any_cast(getHolder()); \ } \ bool isSet() { \ return !getOptionName().empty(); \ } \ template \ - Type& set(Type&& src) { \ - mConfigDeclareField__ = std::forward(src); \ + Type& set(const Type& src) { \ + getHolder() = std::forward(src); \ return as(); \ } \ @@ -154,7 +172,7 @@ public: #define GENERATE_UNION_VISIT_CONST__(r, _, TYPE_) \ if (#TYPE_ == name) { \ - v.visit("value", as()); \ + v.visit("value", as()); \ } #define GENERATE_UNION_NAME__(r, _, TYPE_) \ -- 2.7.4 From 8aa153f871f2f7ba005458dde3104abc191570ef Mon Sep 17 00:00:00 2001 From: Krzysztof Dynowski Date: Sat, 6 Dec 2014 12:53:07 +0100 Subject: [PATCH 06/16] config (dynamic): implemented kvjson visitor and loading methods [Bug/Feature] load config from kvstore with defaults from json [Cause] N/A [Solution] N/A [Verification] Build, install Change-Id: I58c70c0089102b8c4ec1c659937e43d698cd6197 --- src/config/from-kvjson-visitor.hpp | 215 +++++++++++++++++++++++++++++++++++++ src/config/fs-utils.hpp | 4 + src/config/kvstore.cpp | 6 +- src/config/manager.hpp | 39 +++++++ 4 files changed, 262 insertions(+), 2 deletions(-) create mode 100644 src/config/from-kvjson-visitor.hpp diff --git a/src/config/from-kvjson-visitor.hpp b/src/config/from-kvjson-visitor.hpp new file mode 100644 index 0000000..174bbad --- /dev/null +++ b/src/config/from-kvjson-visitor.hpp @@ -0,0 +1,215 @@ +#ifndef CONFIG_FROM_KVJSON_VISITOR_HPP +#define CONFIG_FROM_KVJSON_VISITOR_HPP + +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Krzysztof Dynowski (k.dynowski@samsumg.com) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + + +/** + * @file + * @author Krzysztof Dynowski (k.dynowski@samsumg.com) + * @brief KVStore visitor with defaults values from json + */ + +#include "config/manager.hpp" + + +namespace config { + +class FromKVJsonVisitor { +public: + FromKVJsonVisitor(const std::string& db, const std::string& json) : + mStorePtr(new KVStore(db)) + { + mObject = json_tokener_parse(json.c_str()); + if (mObject == nullptr) { + throw ConfigException("Json parsing error"); + } + } + + ~FromKVJsonVisitor() { + json_object_put(mObject); + } + + FromKVJsonVisitor(const FromKVJsonVisitor& v) : + mObject(json_object_get(v.mObject)), + mStorePtr(v.mStorePtr), + mKeyPrefix(v.mKeyPrefix) + { + } + FromKVJsonVisitor& operator=(const FromKVJsonVisitor&) = delete; + + template + void visit(const std::string& name, T& value) { + getValue(name, value); + } + +private: + std::shared_ptr mStorePtr; + std::string mKeyPrefix; + json_object* mObject; + + // create visitor for field object (visitable object) + FromKVJsonVisitor(const FromKVJsonVisitor& v, const std::string& name) : + mStorePtr(v.mStorePtr) + { + json_object* object; + if (!json_object_object_get_ex(v.mObject, name.c_str(), &object)) { + throw ConfigException("Missing json field " + key(mKeyPrefix, name)); + } + mObject = json_object_get(object); + mKeyPrefix = key(v.mKeyPrefix, name); + } + + // create visitor for vector i-th element (visitable object) + FromKVJsonVisitor(const FromKVJsonVisitor& v, int i) : + mStorePtr(v.mStorePtr) + { + json_object* object = json_object_array_get_idx(v.mObject, i); + checkType(object, json_type_object); + mObject = json_object_get(object); + mKeyPrefix = key(v.mKeyPrefix, std::to_string(i)); + } + + template::value, int>::type = 0> + void getValue(const std::string& name, T& t) + { + json_object* object; + if (!json_object_object_get_ex(mObject, name.c_str(), &object)) { + throw ConfigException("Missing json field " + key(mKeyPrefix, name)); + } + std::string k = key(mKeyPrefix, name); + if (mStorePtr->exists(k)) { + t = mStorePtr->get(k); + } + else { + fromJsonObject(object, t); + } + } + + template::value, int>::type = 0> + void getValue(const std::string& name, T& t) + { + FromKVJsonVisitor visitor(*this, name); + t.accept(visitor); + } + + int getArraySize(std::string& name, json_object* object) + { + int length = -1, jlength = json_object_array_length(object); + if (mStorePtr->exists(name)) { + length = mStorePtr->get(name); + } + return length != jlength ? jlength : length; + } + + template + void getValue(const std::string& name, std::vector& value) + { + json_object* object; + if (!json_object_object_get_ex(mObject, name.c_str(), &object)) { + throw ConfigException("Missing json field " + key(mKeyPrefix, name)); + } + checkType(object, json_type_array); + + std::string k = key(mKeyPrefix, name); + int length = getArraySize(k, object); + value.resize(static_cast(length)); + FromKVJsonVisitor visitor(*this, name); + for (int i = 0; i < length; ++i) { + visitor.getValue(i, value[i]); + } + } + + template::value, int>::type = 0> + void getValue(int i, T& t) + { + json_object* object = json_object_array_get_idx(mObject, i); + std::string k = key(mKeyPrefix, std::to_string(i)); + if (mStorePtr->exists(k)) { + t = mStorePtr->get(k); + } + else { + fromJsonObject(object, t); + } + } + + template::value, int>::type = 0> + void getValue(int i, T& t) + { + FromKVJsonVisitor visitor(*this, i); + t.accept(visitor); + } + + template + void getValue(int i, std::vector& value) + { + std::string k = key(mKeyPrefix, std::to_string(i)); + int length = getArraySize(k, mObject); + value.resize(static_cast(length)); + FromKVJsonVisitor visitor(*this, i); + for (int i = 0; i < length; ++i) { + visitor.getValue(i, value[i]); + } + } + + static void checkType(json_object* object, json_type type) + { + if (type != json_object_get_type(object)) { + throw ConfigException("Invalid field type " + std::to_string(type)); + } + } + + static void fromJsonObject(json_object* object, int& value) + { + checkType(object, json_type_int); + std::int64_t value64 = json_object_get_int64(object); + if (value64 > INT32_MAX || value64 < INT32_MIN) { + throw ConfigException("Value out of range"); + } + value = static_cast(value64); + } + + static void fromJsonObject(json_object* object, std::int64_t& value) + { + checkType(object, json_type_int); + value = json_object_get_int64(object); + } + + static void fromJsonObject(json_object* object, bool& value) + { + checkType(object, json_type_boolean); + value = json_object_get_boolean(object); + } + + static void fromJsonObject(json_object* object, double& value) + { + checkType(object, json_type_double); + value = json_object_get_double(object); + } + + static void fromJsonObject(json_object* object, std::string& value) + { + checkType(object, json_type_string); + value = json_object_get_string(object); + } +}; + +} // namespace config + +#endif // CONFIG_FROM_KVJSON_VISITOR_HPP diff --git a/src/config/fs-utils.hpp b/src/config/fs-utils.hpp index 924a642..82b85f8 100644 --- a/src/config/fs-utils.hpp +++ b/src/config/fs-utils.hpp @@ -33,6 +33,10 @@ namespace fsutils { bool readFileContent(const std::string& path, std::string& result); bool saveFileContent(const std::string& path, const std::string& content); +inline std::string readFileContent(const std::string& path) { + std::string content; + return readFileContent(path, content) ? content : std::string(); +} } // namespace fsutils } // namespace config diff --git a/src/config/kvstore.cpp b/src/config/kvstore.cpp index 621f7f6..d6223fc 100644 --- a/src/config/kvstore.cpp +++ b/src/config/kvstore.cpp @@ -165,7 +165,9 @@ void KVStore::prepareStatements() mGetValueStmt.reset( new sqlite3::Statement(mConn, "SELECT value FROM data WHERE key = ? LIMIT 1")); mGetKeyExistsStmt.reset( - new sqlite3::Statement(mConn, "SELECT 1 FROM data WHERE key = ?1 OR key GLOB escapeStr(?1) || '.*' LIMIT 1")); + // following line left in comment to have example of any subkey matching + //new sqlite3::Statement(mConn, "SELECT 1 FROM data WHERE key = ?1 OR key GLOB escapeStr(?1) || '.*' LIMIT 1")); + new sqlite3::Statement(mConn, "SELECT 1 FROM data WHERE key = ?1 LIMIT 1")); mGetIsEmptyStmt.reset( new sqlite3::Statement(mConn, "SELECT 1 FROM data LIMIT 1")); mSetValueStmt.reset( @@ -280,7 +282,7 @@ std::string KVStore::getInternal(const std::string& key, std::string*) int ret = ::sqlite3_step(mGetValueStmt->get()); if (ret == SQLITE_DONE) { - throw ConfigException("No value corresponding to the key: " + key); + throw ConfigException("No value corresponding to the key: " + key + "@" + mPath); } if (ret != SQLITE_ROW) { throw ConfigException("Error during stepping: " + mConn.getErrorMessage()); diff --git a/src/config/manager.hpp b/src/config/manager.hpp index a2623ea..eb7c740 100644 --- a/src/config/manager.hpp +++ b/src/config/manager.hpp @@ -31,6 +31,7 @@ #include "config/from-json-visitor.hpp" #include "config/from-kvstore-visitor.hpp" #include "config/from-fdstore-visitor.hpp" +#include "config/from-kvjson-visitor.hpp" #include "config/is-visitable.hpp" #include "config/fs-utils.hpp" @@ -136,6 +137,44 @@ void saveToKVStore(const std::string& filename, const Config& config, const std: } /** + * Load the config from KVStore with defaults given in json + * + * @param kvfile path to the KVStore db + * @param jsonfile path to json file with defaults + * @param config visitable structure to save + */ +template +void loadFromKVStoreWithJson(const std::string& kvfile, const std::string& json, Config& config) +{ + static_assert(isVisitable::value, "Use CONFIG_REGISTER macro"); + + FromKVJsonVisitor visitor(kvfile, json); + config.accept(visitor); +} +/** + * Load the config from KVStore with defaults given in json file + * + * @param kvfile path to the KVStore db + * @param jsonfile path to json file with defaults + * @param config visitable structure to save + */ +template +void loadFromKVStoreWithJsonFile(const std::string& kvfile, const std::string& jsonfile, Config& config) +{ + static_assert(isVisitable::value, "Use CONFIG_REGISTER macro"); + + std::string content; + if (!fsutils::readFileContent(jsonfile, content)) { + throw ConfigException("Could not load " + jsonfile); + } + try { + loadFromKVStoreWithJson(kvfile, content, config); + } catch (ConfigException& e) { + throw ConfigException("Error in " + jsonfile + ": " + e.what()); + } +} + +/** * Load binary data from a file/socket/pipe represented by the fd * * @param fd file descriptor -- 2.7.4 From a16f0812ed167378199adeaa5c30f938d49df0bc Mon Sep 17 00:00:00 2001 From: Mateusz Malicki Date: Fri, 9 Jan 2015 16:23:19 +0100 Subject: [PATCH 07/16] Prevent for writing unset union [Bug] Unset can be written to file (can't be read) [Cause] N/A [Solution] Assert in set and accept function, throw config::ConfigException when unsupported type is read or writen [Verification] N/A Change-Id: I5e30d53fe64a9fb132178c1f191af5a5910336d4 --- src/config/fields-union.hpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/config/fields-union.hpp b/src/config/fields-union.hpp index 53620ca..ec4ea3c 100644 --- a/src/config/fields-union.hpp +++ b/src/config/fields-union.hpp @@ -26,6 +26,8 @@ #define CONFIG_FIELDS_UNION_HPP #include "config/fields.hpp" +#include "config/exception.hpp" + #include #include #include @@ -104,10 +106,12 @@ private: template \ void visitOption(Visitor& v, const std::string& name) { \ GENERATE_CODE(GENERATE_UNION_VISIT__, __VA_ARGS__) \ + throw config::ConfigException("Union type error. Unsupported type"); \ } \ template \ void visitOption(Visitor& v, const std::string& name) const { \ GENERATE_CODE(GENERATE_UNION_VISIT_CONST__, __VA_ARGS__) \ + throw config::ConfigException("Union type error. Unsupported type"); \ } \ std::string getOptionName() const { \ GENERATE_CODE(GENERATE_UNION_NAME__, __VA_ARGS__) \ @@ -131,12 +135,9 @@ public: template \ void accept(Visitor v) const { \ const std::string name = getOptionName(); \ - if (!name.empty()) { \ - v.visit("type", name); \ - visitOption(v, name); \ - } else { \ - /* Unsupported type */ \ - } \ + assert(!name.empty() && "Type is not set"); \ + v.visit("type", name); \ + visitOption(v, name); \ } \ \ template \ @@ -159,6 +160,7 @@ public: template \ Type& set(const Type& src) { \ getHolder() = std::forward(src); \ + assert(!getOptionName().empty() && "Unsupported type"); \ return as(); \ } \ @@ -168,11 +170,13 @@ public: #define GENERATE_UNION_VISIT__(r, _, TYPE_) \ if (#TYPE_ == name) { \ v.visit("value", set(std::move(TYPE_()))); \ + return; \ } #define GENERATE_UNION_VISIT_CONST__(r, _, TYPE_) \ if (#TYPE_ == name) { \ v.visit("value", as()); \ + return; \ } #define GENERATE_UNION_NAME__(r, _, TYPE_) \ -- 2.7.4 From e28d57b64695d9f9a4a5939a9e8b71459d1fb9ae Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Mon, 12 Jan 2015 16:34:00 +0100 Subject: [PATCH 08/16] Ability to list all keys from kv store [Bug/Feature] Add listKeys method. Mainly for debug purposes. [Cause] N/A [Solution] N/A [Verification] Build Change-Id: I1d760e047e0393e8ecdb22c242f3ef52e7fdf9d5 --- src/config/kvstore.cpp | 23 +++++++++++++++++++++++ src/config/kvstore.hpp | 6 ++++++ 2 files changed, 29 insertions(+) diff --git a/src/config/kvstore.cpp b/src/config/kvstore.cpp index d6223fc..a9d3fb6 100644 --- a/src/config/kvstore.cpp +++ b/src/config/kvstore.cpp @@ -174,6 +174,8 @@ void KVStore::prepareStatements() new sqlite3::Statement(mConn, "INSERT OR REPLACE INTO data (key, value) VALUES (?,?)")); mRemoveValuesStmt.reset( new sqlite3::Statement(mConn, "DELETE FROM data WHERE key = ?1 OR key GLOB escapeStr(?1) ||'.*' ")); + mGetKeysStmt.reset( + new sqlite3::Statement(mConn, "SELECT key FROM data")); } void KVStore::createFunctions() @@ -310,4 +312,25 @@ std::vector KVStore::getInternal(const std::string& key, std::vecto return values; } +std::vector KVStore::getKeys() +{ + Transaction transaction = getTransaction(); + ScopedReset scopedReset(mGetKeysStmt); + + std::vector result; + + for (;;) { + int ret = ::sqlite3_step(mGetKeysStmt->get()); + if (ret == SQLITE_DONE) { + return result; + } + if (ret != SQLITE_ROW) { + throw ConfigException("Error during stepping: " + mConn.getErrorMessage()); + } + const char* key = reinterpret_cast(sqlite3_column_text(mGetKeysStmt->get(), + FIRST_COLUMN)); + result.push_back(key); + } +} + } // namespace config diff --git a/src/config/kvstore.hpp b/src/config/kvstore.hpp index 44c0aa1..4dc978a 100644 --- a/src/config/kvstore.hpp +++ b/src/config/kvstore.hpp @@ -106,6 +106,11 @@ public: return getInternal(key, static_cast(nullptr)); } + /** + * Returns all stored keys. + */ + std::vector getKeys(); + KVStore::Transaction getTransaction(); private: @@ -139,6 +144,7 @@ private: std::unique_ptr mGetValueListStmt; std::unique_ptr mSetValueStmt; std::unique_ptr mRemoveValuesStmt; + std::unique_ptr mGetKeysStmt; void setupDb(); void prepareStatements(); -- 2.7.4 From c9099ead16ca369eec52a5d1fbfdca5df6019ed4 Mon Sep 17 00:00:00 2001 From: Mateusz Malicki Date: Tue, 13 Jan 2015 15:03:39 +0100 Subject: [PATCH 09/16] Fix: Crashing when writing unset union type [Bug] Assert instead exception [Cause] Assert [Solution] Assert was removed [Verification] Build in debug mode, run test Change-Id: I93e9a481d008ea50882b7886f0a63c9f6136bbd0 --- src/config/fields-union.hpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/config/fields-union.hpp b/src/config/fields-union.hpp index ec4ea3c..d37791d 100644 --- a/src/config/fields-union.hpp +++ b/src/config/fields-union.hpp @@ -30,7 +30,6 @@ #include #include -#include /** * Use this macro to declare and register config fields @@ -135,7 +134,9 @@ public: template \ void accept(Visitor v) const { \ const std::string name = getOptionName(); \ - assert(!name.empty() && "Type is not set"); \ + if (name.empty()) { \ + throw config::ConfigException("Type is not set"); \ + } \ v.visit("type", name); \ visitOption(v, name); \ } \ @@ -146,12 +147,16 @@ public: } \ template \ typename std::enable_if::value, Type>::type& as() { \ - assert(!getHolder().empty()); \ + if (getHolder().empty()) { \ + throw config::ConfigException("Type is not set"); \ + } \ return boost::any_cast(getHolder()); \ } \ template \ const Type& as() const { \ - assert(!getHolder().empty()); \ + if (getHolder().empty()) { \ + throw config::ConfigException("Type is not set"); \ + } \ return boost::any_cast(getHolder()); \ } \ bool isSet() { \ @@ -160,7 +165,9 @@ public: template \ Type& set(const Type& src) { \ getHolder() = std::forward(src); \ - assert(!getOptionName().empty() && "Unsupported type"); \ + if (getOptionName().empty()) { \ + throw config::ConfigException("Unsupported type"); \ + } \ return as(); \ } \ -- 2.7.4 From fbce303855c4dc63be84ee80dbce7d561bdd9f39 Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Thu, 15 Jan 2015 10:13:33 +0100 Subject: [PATCH 10/16] Fix arrays in kvjson visitor, prohibit empty db name [Bug/Feature] Arrays were not correctly handled. [Cause] N/A [Solution] N/A [Verification] Build, run tests Change-Id: Idd5c2c35baf602e09961ee8af80857dda313a5da --- src/config/from-kvjson-visitor.hpp | 5 ++--- src/config/sqlite3/connection.cpp | 5 +++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/config/from-kvjson-visitor.hpp b/src/config/from-kvjson-visitor.hpp index 174bbad..bbae07e 100644 --- a/src/config/from-kvjson-visitor.hpp +++ b/src/config/from-kvjson-visitor.hpp @@ -111,11 +111,10 @@ private: int getArraySize(std::string& name, json_object* object) { - int length = -1, jlength = json_object_array_length(object); if (mStorePtr->exists(name)) { - length = mStorePtr->get(name); + return mStorePtr->get(name); } - return length != jlength ? jlength : length; + return json_object_array_length(object); } template diff --git a/src/config/sqlite3/connection.cpp b/src/config/sqlite3/connection.cpp index e5d0552..cb25694 100644 --- a/src/config/sqlite3/connection.cpp +++ b/src/config/sqlite3/connection.cpp @@ -31,6 +31,11 @@ namespace sqlite3 { Connection::Connection(const std::string& path) { + if (path.empty()) { + // Sqlite creates temporary database in case of empty path + // but we want to forbid this. + throw ConfigException("Error opening the database: empty path"); + } if (::sqlite3_open_v2(path.c_str(), &mDbPtr, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, -- 2.7.4 From f181b5f2d77b6981c0c05f88af63f1bf99535749 Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Fri, 16 Jan 2015 14:50:09 +0100 Subject: [PATCH 11/16] Config manager api refinements [Bug/Feature] Always use prefix in db. Function renames. [Cause] N/A [Solution] N/A [Verification] Build, run tests Change-Id: Ia32747c15c904b3c67b00aeb7a6bf051c9591de9 --- src/config/from-kvjson-visitor.hpp | 5 +++-- src/config/from-kvstore-visitor.hpp | 18 +++++++----------- src/config/manager.hpp | 36 +++++++++++++++++++++--------------- src/config/to-kvstore-visitor.hpp | 18 +++++++----------- 4 files changed, 38 insertions(+), 39 deletions(-) diff --git a/src/config/from-kvjson-visitor.hpp b/src/config/from-kvjson-visitor.hpp index bbae07e..d2c1060 100644 --- a/src/config/from-kvjson-visitor.hpp +++ b/src/config/from-kvjson-visitor.hpp @@ -33,8 +33,9 @@ namespace config { class FromKVJsonVisitor { public: - FromKVJsonVisitor(const std::string& db, const std::string& json) : - mStorePtr(new KVStore(db)) + FromKVJsonVisitor(const std::string& db, const std::string& json, const std::string& prefix) + : mStorePtr(new KVStore(db)) + , mKeyPrefix(prefix) { mObject = json_tokener_parse(json.c_str()); if (mObject == nullptr) { diff --git a/src/config/from-kvstore-visitor.hpp b/src/config/from-kvstore-visitor.hpp index c7e3540..3981bfd 100644 --- a/src/config/from-kvstore-visitor.hpp +++ b/src/config/from-kvstore-visitor.hpp @@ -43,17 +43,6 @@ public: { } - FromKVStoreVisitor(const FromKVStoreVisitor& visitor, const std::string& prefix) - : mStorePtr(visitor.mStorePtr), - mKeyPrefix(prefix), - mTransaction(visitor.mTransaction) - { - } - - ~FromKVStoreVisitor() - { - } - FromKVStoreVisitor& operator=(const FromKVStoreVisitor&) = delete; template @@ -67,6 +56,13 @@ private: std::string mKeyPrefix; KVStore::Transaction mTransaction; + FromKVStoreVisitor(const FromKVStoreVisitor& visitor, const std::string& prefix) + : mStorePtr(visitor.mStorePtr), + mKeyPrefix(prefix), + mTransaction(visitor.mTransaction) + { + } + template::value, int>::type = 0> void getInternal(const std::string& name, T& value) { diff --git a/src/config/manager.hpp b/src/config/manager.hpp index eb7c740..0785094 100644 --- a/src/config/manager.hpp +++ b/src/config/manager.hpp @@ -45,7 +45,7 @@ namespace config { * @param config visitable structure to fill */ template -void loadFromString(const std::string& jsonString, Config& config) +void loadFromJsonString(const std::string& jsonString, Config& config) { static_assert(isVisitable::value, "Use CONFIG_REGISTER macro"); @@ -53,14 +53,13 @@ void loadFromString(const std::string& jsonString, Config& config) config.accept(visitor); } - /** * Creates a string representation of the configuration in json format * * @param config visitable structure to convert */ template -std::string saveToString(const Config& config) +std::string saveToJsonString(const Config& config) { static_assert(isVisitable::value, "Use CONFIG_REGISTER macro"); @@ -76,14 +75,14 @@ std::string saveToString(const Config& config) * @param config visitable structure to load */ template -void loadFromFile(const std::string& filename, Config& config) +void loadFromJsonFile(const std::string& filename, Config& config) { std::string content; if (!fsutils::readFileContent(filename, content)) { throw ConfigException("Could not load " + filename); } try { - loadFromString(content, config); + loadFromJsonString(content, config); } catch (ConfigException& e) { throw ConfigException("Error in " + filename + ": " + e.what()); } @@ -96,9 +95,9 @@ void loadFromFile(const std::string& filename, Config& config) * @param config visitable structure to save */ template -void saveToFile(const std::string& filename, const Config& config) +void saveToJsonFile(const std::string& filename, const Config& config) { - const std::string content = saveToString(config); + const std::string content = saveToJsonString(config); if (!fsutils::saveFileContent(filename, content)) { throw ConfigException("Could not save " + filename); } @@ -112,7 +111,7 @@ void saveToFile(const std::string& filename, const Config& config) * @param configName name of the configuration inside the KVStore db */ template -void loadFromKVStore(const std::string& filename, Config& config, const std::string& configName = "") +void loadFromKVStore(const std::string& filename, Config& config, const std::string& configName) { static_assert(isVisitable::value, "Use CONFIG_REGISTER macro"); @@ -128,7 +127,7 @@ void loadFromKVStore(const std::string& filename, Config& config, const std::str * @param configName name of the config inside the KVStore db */ template -void saveToKVStore(const std::string& filename, const Config& config, const std::string& configName = "") +void saveToKVStore(const std::string& filename, const Config& config, const std::string& configName) { static_assert(isVisitable::value, "Use CONFIG_REGISTER macro"); @@ -142,33 +141,40 @@ void saveToKVStore(const std::string& filename, const Config& config, const std: * @param kvfile path to the KVStore db * @param jsonfile path to json file with defaults * @param config visitable structure to save + * @param kvConfigName name of the config inside the KVStore db */ template -void loadFromKVStoreWithJson(const std::string& kvfile, const std::string& json, Config& config) +void loadFromKVStoreWithJson(const std::string& kvfile, + const std::string& json, + Config& config, + const std::string& kvConfigName) { static_assert(isVisitable::value, "Use CONFIG_REGISTER macro"); - FromKVJsonVisitor visitor(kvfile, json); + FromKVJsonVisitor visitor(kvfile, json, kvConfigName); config.accept(visitor); } + /** * Load the config from KVStore with defaults given in json file * * @param kvfile path to the KVStore db * @param jsonfile path to json file with defaults * @param config visitable structure to save + * @param kvConfigName name of the config inside the KVStore db */ template -void loadFromKVStoreWithJsonFile(const std::string& kvfile, const std::string& jsonfile, Config& config) +void loadFromKVStoreWithJsonFile(const std::string& kvfile, + const std::string& jsonfile, + Config& config, + const std::string& kvConfigName) { - static_assert(isVisitable::value, "Use CONFIG_REGISTER macro"); - std::string content; if (!fsutils::readFileContent(jsonfile, content)) { throw ConfigException("Could not load " + jsonfile); } try { - loadFromKVStoreWithJson(kvfile, content, config); + loadFromKVStoreWithJson(kvfile, content, config, kvConfigName); } catch (ConfigException& e) { throw ConfigException("Error in " + jsonfile + ": " + e.what()); } diff --git a/src/config/to-kvstore-visitor.hpp b/src/config/to-kvstore-visitor.hpp index 37b70a9..071dece 100644 --- a/src/config/to-kvstore-visitor.hpp +++ b/src/config/to-kvstore-visitor.hpp @@ -43,17 +43,6 @@ public: { } - ToKVStoreVisitor(const ToKVStoreVisitor& visitor, const std::string& prefix) - : mStorePtr(visitor.mStorePtr), - mKeyPrefix(prefix), - mTransaction(visitor.mTransaction) - { - } - - ~ToKVStoreVisitor() - { - } - ToKVStoreVisitor& operator=(const ToKVStoreVisitor&) = delete; template @@ -67,6 +56,13 @@ private: std::string mKeyPrefix; KVStore::Transaction mTransaction; + ToKVStoreVisitor(const ToKVStoreVisitor& visitor, const std::string& prefix) + : mStorePtr(visitor.mStorePtr), + mKeyPrefix(prefix), + mTransaction(visitor.mTransaction) + { + } + template::value, int>::type = 0> void setInternal(const std::string& name, const T& value) { -- 2.7.4 From 6d63eb3b5fa8ce01447fbb2ddc2ba2a50113ae0e Mon Sep 17 00:00:00 2001 From: Krzysztof Dynowski Date: Tue, 13 Jan 2015 13:45:30 +0100 Subject: [PATCH 12/16] implement generic method checker and is-union [Bug/Feature] N/A [Cause] N/A [Solution] N/A [Verification] Build, install Change-Id: Iaf8eb549729e095ed1d446bc6fd2cd71399e1019 --- src/config/from-kvjson-visitor.hpp | 102 ++++++++++++++++++++++++------------- src/config/is-union.hpp | 70 +++++++++++++++++++++++++ src/config/manager.hpp | 1 - 3 files changed, 138 insertions(+), 35 deletions(-) create mode 100644 src/config/is-union.hpp diff --git a/src/config/from-kvjson-visitor.hpp b/src/config/from-kvjson-visitor.hpp index d2c1060..e01774e 100644 --- a/src/config/from-kvjson-visitor.hpp +++ b/src/config/from-kvjson-visitor.hpp @@ -1,6 +1,3 @@ -#ifndef CONFIG_FROM_KVJSON_VISITOR_HPP -#define CONFIG_FROM_KVJSON_VISITOR_HPP - /* * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved * @@ -26,8 +23,12 @@ * @brief KVStore visitor with defaults values from json */ -#include "config/manager.hpp" +#ifndef CONFIG_FROM_KVJSON_VISITOR_HPP +#define CONFIG_FROM_KVJSON_VISITOR_HPP +#include "config/from-kvstore-visitor.hpp" +#include "config/is-union.hpp" +#include namespace config { @@ -36,6 +37,7 @@ public: FromKVJsonVisitor(const std::string& db, const std::string& json, const std::string& prefix) : mStorePtr(new KVStore(db)) , mKeyPrefix(prefix) + , mIsUnion(false) { mObject = json_tokener_parse(json.c_str()); if (mObject == nullptr) { @@ -43,14 +45,16 @@ public: } } + ~FromKVJsonVisitor() { json_object_put(mObject); } FromKVJsonVisitor(const FromKVJsonVisitor& v) : - mObject(json_object_get(v.mObject)), + mObject(v.mObject ? json_object_get(v.mObject) : nullptr), mStorePtr(v.mStorePtr), - mKeyPrefix(v.mKeyPrefix) + mKeyPrefix(v.mKeyPrefix), + mIsUnion(v.mIsUnion) { } FromKVJsonVisitor& operator=(const FromKVJsonVisitor&) = delete; @@ -64,49 +68,64 @@ private: std::shared_ptr mStorePtr; std::string mKeyPrefix; json_object* mObject; + bool mIsUnion; // create visitor for field object (visitable object) - FromKVJsonVisitor(const FromKVJsonVisitor& v, const std::string& name) : - mStorePtr(v.mStorePtr) + FromKVJsonVisitor(const FromKVJsonVisitor& v, const std::string& name, const bool isUnion) : + mStorePtr(v.mStorePtr), mIsUnion(isUnion || v.mIsUnion) { - json_object* object; - if (!json_object_object_get_ex(v.mObject, name.c_str(), &object)) { - throw ConfigException("Missing json field " + key(mKeyPrefix, name)); - } - mObject = json_object_get(object); mKeyPrefix = key(v.mKeyPrefix, name); + json_object* object = nullptr; + if (v.mObject && !json_object_object_get_ex(v.mObject, name.c_str(), &object)) { + if (!mIsUnion) + throw ConfigException("Missing json field " + mKeyPrefix); + } + mObject = object ? json_object_get(object) : nullptr; } // create visitor for vector i-th element (visitable object) - FromKVJsonVisitor(const FromKVJsonVisitor& v, int i) : - mStorePtr(v.mStorePtr) + FromKVJsonVisitor(const FromKVJsonVisitor& v, int i, const bool isUnion) : + mStorePtr(v.mStorePtr), mIsUnion(isUnion || v.mIsUnion) { - json_object* object = json_object_array_get_idx(v.mObject, i); - checkType(object, json_type_object); - mObject = json_object_get(object); mKeyPrefix = key(v.mKeyPrefix, std::to_string(i)); + json_object* object = nullptr; + if (v.mObject) { + object = json_object_array_get_idx(v.mObject, i); + checkType(object, json_type_object); + } + mObject = object ? json_object_get(object) : nullptr; } template::value, int>::type = 0> void getValue(const std::string& name, T& t) { - json_object* object; - if (!json_object_object_get_ex(mObject, name.c_str(), &object)) { - throw ConfigException("Missing json field " + key(mKeyPrefix, name)); - } std::string k = key(mKeyPrefix, name); if (mStorePtr->exists(k)) { t = mStorePtr->get(k); } else { + json_object* object = nullptr; + if (mObject) { + json_object_object_get_ex(mObject, name.c_str(), &object); + } + if (!object) { + throw ConfigException("Missing json field " + key(mKeyPrefix, name)); + } fromJsonObject(object, t); } } - template::value, int>::type = 0> + template::value, int>::type = 0> + void getValue(const std::string& name, T& t) + { + FromKVJsonVisitor visitor(*this, name, true); + t.accept(visitor); + } + + template::value && !isUnion::value, int>::type = 0> void getValue(const std::string& name, T& t) { - FromKVJsonVisitor visitor(*this, name); + FromKVJsonVisitor visitor(*this, name, false); t.accept(visitor); } @@ -115,22 +134,27 @@ private: if (mStorePtr->exists(name)) { return mStorePtr->get(name); } - return json_object_array_length(object); + if (object) { + return json_object_array_length(object); + } + return -1; } template void getValue(const std::string& name, std::vector& value) { - json_object* object; - if (!json_object_object_get_ex(mObject, name.c_str(), &object)) { - throw ConfigException("Missing json field " + key(mKeyPrefix, name)); + json_object* object = nullptr; + if (mObject && json_object_object_get_ex(mObject, name.c_str(), &object)) { + checkType(object, json_type_array); } - checkType(object, json_type_array); std::string k = key(mKeyPrefix, name); int length = getArraySize(k, object); + if (length < 0) { + throw ConfigException("Missing json field " + key(mKeyPrefix, name)); + } value.resize(static_cast(length)); - FromKVJsonVisitor visitor(*this, name); + FromKVJsonVisitor visitor(*this, name, false); for (int i = 0; i < length; ++i) { visitor.getValue(i, value[i]); } @@ -139,20 +163,30 @@ private: template::value, int>::type = 0> void getValue(int i, T& t) { - json_object* object = json_object_array_get_idx(mObject, i); std::string k = key(mKeyPrefix, std::to_string(i)); if (mStorePtr->exists(k)) { t = mStorePtr->get(k); } else { + json_object* object = mObject ? json_object_array_get_idx(mObject, i) : nullptr; + if (!object) { + throw ConfigException("Missing json array elem " + k); + } fromJsonObject(object, t); } } - template::value, int>::type = 0> + template::value, int>::type = 0> + void getValue(int i, T& t) + { + FromKVJsonVisitor visitor(*this, i, true); + t.accept(visitor); + } + + template::value && !isUnion::value, int>::type = 0> void getValue(int i, T& t) { - FromKVJsonVisitor visitor(*this, i); + FromKVJsonVisitor visitor(*this, i, false); t.accept(visitor); } @@ -162,7 +196,7 @@ private: std::string k = key(mKeyPrefix, std::to_string(i)); int length = getArraySize(k, mObject); value.resize(static_cast(length)); - FromKVJsonVisitor visitor(*this, i); + FromKVJsonVisitor visitor(*this, i, false); for (int i = 0; i < length; ++i) { visitor.getValue(i, value[i]); } diff --git a/src/config/is-union.hpp b/src/config/is-union.hpp new file mode 100644 index 0000000..f696b72 --- /dev/null +++ b/src/config/is-union.hpp @@ -0,0 +1,70 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Contact: Piotr Bartosiewicz (p.bartosiewi@partner.samsung.com) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +/** + * @file + * @author Krzysztof Dynowski (k.dynowski@samsumg.com) + * @brief Internal configuration helper + */ + +#ifndef CONFIG_IS_UNION_HPP +#define CONFIG_IS_UNION_HPP + +#include "config/is-visitable.hpp" + +namespace config { + +// generic member checker, start +template +struct has_member_impl { + template + static std::true_type check(typename F::template checker__* =0); + + template + static std::false_type check(...); + + static const bool value = std::is_same(0)), std::true_type>::value; +}; + +template +struct has_member : public std::integral_constant::value> {}; +// generic member checker, end + + +template +struct check_union : isVisitable { + template + struct checker__ {}; +}; +template +struct isUnion : has_member> {}; + +//Note: +// unfortunately, above generic has_member can't be used for isVisitable +// because Vistable need 'accept' OR 'accept const', while has_member make exect match +// e.g accept AND accept const + +} // namespace config + +#endif // CONFIG_FROM_KVJSON_VISITOR_HPP + diff --git a/src/config/manager.hpp b/src/config/manager.hpp index 0785094..7c50087 100644 --- a/src/config/manager.hpp +++ b/src/config/manager.hpp @@ -32,7 +32,6 @@ #include "config/from-kvstore-visitor.hpp" #include "config/from-fdstore-visitor.hpp" #include "config/from-kvjson-visitor.hpp" -#include "config/is-visitable.hpp" #include "config/fs-utils.hpp" -- 2.7.4 From c3c2ab9be50e3585a6c086b0e4c7098c7ebf4c6c Mon Sep 17 00:00:00 2001 From: Krzysztof Dynowski Date: Thu, 22 Jan 2015 11:29:14 +0100 Subject: [PATCH 13/16] fix loading array from kvstore if size diffs in json [Bug/Feature] bug reading array from kvstore [Cause] N/A [Solution] don't use json object, set json object to null [Verification] Build, install Change-Id: I6760075039e0fbae173e7329fbd14721f2710664 --- src/config/from-kvjson-visitor.hpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/config/from-kvjson-visitor.hpp b/src/config/from-kvjson-visitor.hpp index e01774e..c355fc2 100644 --- a/src/config/from-kvjson-visitor.hpp +++ b/src/config/from-kvjson-visitor.hpp @@ -151,10 +151,13 @@ private: std::string k = key(mKeyPrefix, name); int length = getArraySize(k, object); if (length < 0) { - throw ConfigException("Missing json field " + key(mKeyPrefix, name)); + throw ConfigException("Missing array length " + k); } value.resize(static_cast(length)); FromKVJsonVisitor visitor(*this, name, false); + if (mStorePtr->exists(k)) { + visitor.mObject = nullptr; + } for (int i = 0; i < length; ++i) { visitor.getValue(i, value[i]); } @@ -197,6 +200,9 @@ private: int length = getArraySize(k, mObject); value.resize(static_cast(length)); FromKVJsonVisitor visitor(*this, i, false); + if (mStorePtr->exists(k)) { + visitor.mObject = nullptr; + } for (int i = 0; i < length; ++i) { visitor.getValue(i, value[i]); } -- 2.7.4 From 1e80174c8f8e21ad245902f4ea89c0df2c0dd88e Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Fri, 23 Jan 2015 13:45:59 +0100 Subject: [PATCH 14/16] Fix potential deadlock; FromKVJsonVisitor use db transaction [Bug/Feature] Invalid mutexes lock sequence in kv store transaction. Missing db transaction in FromKVJsonVisitor. [Cause] N/A [Solution] N/A [Verification] Build, run tests Change-Id: Ie44dbfe5ad86c9cbf0e19a162a267676b05f1aed --- src/config/from-kvjson-visitor.hpp | 17 ++++++++++++----- src/config/kvstore.cpp | 33 ++++++++++++++------------------- src/config/kvstore.hpp | 11 ++++++----- 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/config/from-kvjson-visitor.hpp b/src/config/from-kvjson-visitor.hpp index c355fc2..2e386b5 100644 --- a/src/config/from-kvjson-visitor.hpp +++ b/src/config/from-kvjson-visitor.hpp @@ -38,6 +38,7 @@ public: : mStorePtr(new KVStore(db)) , mKeyPrefix(prefix) , mIsUnion(false) + , mTransaction(mStorePtr->getTransaction()) { mObject = json_tokener_parse(json.c_str()); if (mObject == nullptr) { @@ -54,7 +55,8 @@ public: mObject(v.mObject ? json_object_get(v.mObject) : nullptr), mStorePtr(v.mStorePtr), mKeyPrefix(v.mKeyPrefix), - mIsUnion(v.mIsUnion) + mIsUnion(v.mIsUnion), + mTransaction(v.mTransaction) { } FromKVJsonVisitor& operator=(const FromKVJsonVisitor&) = delete; @@ -69,12 +71,15 @@ private: 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), mIsUnion(isUnion || v.mIsUnion) + mStorePtr(v.mStorePtr), + mKeyPrefix(key(v.mKeyPrefix, name)), + mIsUnion(isUnion || v.mIsUnion), + mTransaction(v.mTransaction) { - mKeyPrefix = key(v.mKeyPrefix, name); json_object* object = nullptr; if (v.mObject && !json_object_object_get_ex(v.mObject, name.c_str(), &object)) { if (!mIsUnion) @@ -85,9 +90,11 @@ private: // create visitor for vector i-th element (visitable object) FromKVJsonVisitor(const FromKVJsonVisitor& v, int i, const bool isUnion) : - mStorePtr(v.mStorePtr), mIsUnion(isUnion || v.mIsUnion) + mStorePtr(v.mStorePtr), + mKeyPrefix(key(v.mKeyPrefix, std::to_string(i))), + mIsUnion(isUnion || v.mIsUnion), + mTransaction(v.mTransaction) { - mKeyPrefix = key(v.mKeyPrefix, std::to_string(i)); json_object* object = nullptr; if (v.mObject) { object = json_object_array_get_idx(v.mObject, i); diff --git a/src/config/kvstore.cpp b/src/config/kvstore.cpp index a9d3fb6..476d182 100644 --- a/src/config/kvstore.cpp +++ b/src/config/kvstore.cpp @@ -31,6 +31,7 @@ #include #include #include +#include namespace config { @@ -96,38 +97,39 @@ void sqliteEscapeStr(::sqlite3_context* context, int argc, ::sqlite3_value** val } // namespace struct KVStore::TransactionImpl { - TransactionImpl(KVStore* kvstorePtr) - : mKVStorePtr(kvstorePtr) + TransactionImpl(KVStore& kvStore) + : mLock(kvStore.mMutex) + , mKVStore(kvStore) { - mKVStorePtr->mConnMtx.lock(); - mKVStorePtr->mConn.exec("BEGIN EXCLUSIVE TRANSACTION"); + mKVStore.mConn.exec("BEGIN EXCLUSIVE TRANSACTION"); } ~TransactionImpl() { + // be aware of a bug in gcc with uncaught_exception and rethrow_exception if (std::uncaught_exception()) { try { - mKVStorePtr->mConn.exec("ROLLBACK TRANSACTION"); + mKVStore.mConn.exec("ROLLBACK TRANSACTION"); } catch (ConfigException&) {} } else { try { - mKVStorePtr->mConn.exec("COMMIT TRANSACTION"); + mKVStore.mConn.exec("COMMIT TRANSACTION"); } catch (ConfigException&) {} } - mKVStorePtr->mConnMtx.unlock(); } private: - config::KVStore* mKVStorePtr; + config::KVStore::Lock mLock; + config::KVStore& mKVStore; }; KVStore::Transaction KVStore::getTransaction() { - Lock lock(getTransactionMutex); + Lock lock(mMutex); auto t = mTransactionImplPtr.lock(); if (!t) { - t = std::make_shared(this); + t = std::make_shared(*this); mTransactionImplPtr = t; } return t; @@ -142,21 +144,14 @@ KVStore::KVStore(const std::string& path) prepareStatements(); } -KVStore::KVStore(const KVStore& store) - : mPath(store.mPath), - mConn(mPath) -{ - createFunctions(); - prepareStatements(); -} - KVStore::~KVStore() { + assert(!mTransactionImplPtr.lock()); } void KVStore::setupDb() { - Transaction transaction = getTransaction(); + // called only from ctor, transaction is not needed mConn.exec("CREATE TABLE IF NOT EXISTS data (key TEXT PRIMARY KEY, value TEXT NOT NULL)"); } diff --git a/src/config/kvstore.hpp b/src/config/kvstore.hpp index 4dc978a..1b24b36 100644 --- a/src/config/kvstore.hpp +++ b/src/config/kvstore.hpp @@ -50,10 +50,12 @@ public: /** * @param path configuration database file path */ - KVStore(const std::string& path); - KVStore(const KVStore& store); + explicit KVStore(const std::string& path); ~KVStore(); + KVStore(const KVStore&) = delete; + KVStore& operator=(const KVStore&) = delete; + /** * Clears all the stored data */ @@ -114,12 +116,11 @@ public: KVStore::Transaction getTransaction(); private: - typedef std::lock_guard Lock; + typedef std::lock_guard Lock; struct TransactionImpl; std::weak_ptr mTransactionImplPtr; - std::mutex getTransactionMutex; - std::mutex mConnMtx; + std::recursive_mutex mMutex; void setInternal(const std::string& key, const std::string& value); void setInternal(const std::string& key, const std::initializer_list& values); -- 2.7.4 From 67591e764a41fe283006533a6f5763badddc5163 Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Fri, 30 Jan 2015 15:43:15 +0100 Subject: [PATCH 15/16] Fix memleak in FromKVJsonVisitor [Bug/Feature] N/A [Cause] N/A [Solution] N/A [Verification] Run tests under valgrind Change-Id: Id3f4c18b9efa84ebf8cf15a52791a0d79bc46589 --- src/config/from-kvjson-visitor.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/config/from-kvjson-visitor.hpp b/src/config/from-kvjson-visitor.hpp index 2e386b5..2624ad3 100644 --- a/src/config/from-kvjson-visitor.hpp +++ b/src/config/from-kvjson-visitor.hpp @@ -163,6 +163,7 @@ private: value.resize(static_cast(length)); FromKVJsonVisitor visitor(*this, name, false); if (mStorePtr->exists(k)) { + json_object_put(visitor.mObject); visitor.mObject = nullptr; } for (int i = 0; i < length; ++i) { @@ -208,6 +209,7 @@ private: value.resize(static_cast(length)); FromKVJsonVisitor visitor(*this, i, false); if (mStorePtr->exists(k)) { + json_object_put(visitor.mObject); visitor.mObject = nullptr; } for (int i = 0; i < length; ++i) { -- 2.7.4 From d6e9bf90e396fef5f70b673503d72574e89db432 Mon Sep 17 00:00:00 2001 From: Piotr Bartosiewicz Date: Thu, 5 Feb 2015 14:28:22 +0100 Subject: [PATCH 16/16] 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 --- src/config/from-kvjson-visitor.hpp | 39 ++++++------- src/config/from-kvstore-visitor.hpp | 23 +++----- src/config/kvstore.cpp | 113 ++++++++++++++++++++---------------- src/config/kvstore.hpp | 21 +++++-- src/config/manager.hpp | 15 ++++- src/config/to-kvstore-visitor.hpp | 24 +++----- 6 files changed, 124 insertions(+), 111 deletions(-) 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]); } -- 2.7.4